Gate the quota-hook example's counter table off the MCP surface#576
Gate the quota-hook example's counter table off the MCP surface#576kylebernhardy wants to merge 1 commit into
Conversation
The example exported the counter table, which surfaces update_/delete_ verb tools and REST — letting a permitted client reset its own quota. Found by a blind agent evaluation of the harper-mcp skill (skills#69). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the McpQuota class in reference/mcp/configuration.md to restrict its export types, preventing the exposure of MCP tools. The reviewer suggests also setting http: false in exportTypes to fully disable the REST endpoint and prevent unauthorized access, as setting only { mcp: false } leaves the HTTP endpoint exposed.
| // update_/delete_McpQuota MCP tools and a REST endpoint, letting a | ||
| // permitted client reset its own counter. Keep the quota table off the MCP | ||
| // surface and restrict its REST permissions. | ||
| static exportTypes = { mcp: false }; |
There was a problem hiding this comment.
The comment mentions keeping the quota table off the MCP surface and restricting its REST permissions. However, setting only { mcp: false } leaves the REST (HTTP) endpoint fully exposed. To completely disable the REST endpoint as well, you should also set http: false in exportTypes.
| static exportTypes = { mcp: false }; | |
| static exportTypes = { mcp: false, http: false }; |
🚀 Preview DeploymentYour preview deployment is ready! 🔗 Preview URL: https://preview.harper-documentation.harperfabric.com/pr-576 This preview will update automatically when you push new commits. |
kriszyp
left a comment
kriszyp
left a comment
There was a problem hiding this comment.
The static exportTypes = { mcp: false } line doesn't do what the comment claims — Harper's Resource/Table registration (resources/jsResource.ts, resources/Resources.ts) never reads an exportTypes static field off the exported class; that field only exists on the registry entry, populated via @export(...) (GraphQL schema tables) or a server.resources.set(name, resource, exportTypes) call. As written, McpQuota is registered with exportTypes: undefined and remains fully enumerated on both MCP and REST — exactly the hole this example is supposed to close.
Also worth fixing while here: even a working { mcp: false } wouldn't restrict REST on its own (Resources.getMatch's !== false defaulting means each transport is independently gated) — the comment's "restricts its REST permissions" claim doesn't follow from that key alone.
The mechanism that's actually wired today: server.resources.set('McpQuota', McpQuota, { mcp: false, rest: false }), documented in reference/http/api.md. Happy to help swap the example over to that if useful.
— KrAIs
3 participants
One-hunk fix to the
mcp.<profile>.quota.*example inreference/mcp/configuration.md: the exportedMcpQuotaclass also surfacedupdate_/delete_McpQuotaMCP tools and a REST endpoint, letting a permitted client reset its own counter. The example now carriesstatic exportTypes = { mcp: false };with a comment, matching the hardened example in HarperFast/skills#69 (whose blind agent evaluation found the exposure).Generated by an LLM (Claude Fable 5).