Skip to content

feat(mcp): expose tenant-scoped read-only resources - #631

Draft
adity982 wants to merge 2 commits into
HelpCode-ai:mainfrom
adity982:feat/mcp-resources-sep15
Draft

adity982 wants to merge 2 commits into
HelpCode-ai:mainfrom
adity982:feat/mcp-resources-sep15

Conversation

@adity982

Copy link
Copy Markdown

Summary\n\n- expose native MCP resources on per-server endpoints via

esources/list and
esources/read\n- publish composed server instructions, connector setup instructions, and persisted McpResource content\n- keep resource discovery tenant-scoped and fail closed for remote URL fetch configs to avoid SSRF\n- deduplicate resource URIs and document the stateful-session snapshot behavior\n\n## Validation\n\n- TypeScript parser smoke check passed for changed backend files\n- resource helper smoke check passed (serialization, registration, callback output)\n- git diff --check passed\n- Full backend Jest/build validation was not runnable because dependency installation cannot complete on this host (no
ode_modules/.bin/jest or TypeScript compiler available).\n\n## Certification\n\n- [x] This change follows the AnythingMCP contribution standards (https://github.com/HelpCode-ai/anythingmcp/blob/main/CONTRIBUTING.md)\n\nCloses #593

@github-actions

Copy link
Copy Markdown


Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.


I have read the CLA Document and I hereby sign the CLA


You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot.

@github-actions

Copy link
Copy Markdown

👋 Welcome, @adity982, and thanks for opening your first PR on AnythingMCP!

A few quick pointers:

  • Make sure CI is green before requesting review (Backend, Frontend, Playwright, CodeQL, Trivy).
  • If this is a new adapter, the parametrised catalog.spec.ts test will validate it automatically.
  • Sign off your commits if you can — it's not blocking, just nice to have.

Someone from the core team will look at this within ~48h. If you don't hear back, please ping us in Discussions / Q&A.

⭐ While you wait — if you find AnythingMCP useful, a star helps others discover it.

@keysersoft

Copy link
Copy Markdown
Contributor

Thanks for this, and for picking the one from #593 that has the clearer payoff. I approved the workflows (fork PRs need that here) and CI is green: backend lint, typecheck, tests and build all pass, so your "could not install dependencies" note is covered, you did not break anything.

I read the whole diff. The design is right and I want this feature. Two things before it can go in, one of them matters a lot.

1. Resources ignore role-based access. This is the blocker.

In handleMcpRequest the tool list goes through two filters: the server's assigned connectors, and then allowedToolIds from RolesService.getAllowedToolIds. Your planResources call only goes through the first one. So a caller whose role denies every tool of a connector still gets that connector's instructions listed and readable as a resource, and gets any persisted McpResource content attached to it.

That is not theoretical for us. We have a customer running a read-only role with 64 explicitly denied tools, and the whole point of that role is that the agent cannot see the rest. Resources have to respect the same boundary. Something like this, right after step 4 where allowedToolIds is computed:

const allowedConnectorIds = new Set(
  serverTools
    .filter((t) => !allowedToolIds || allowedToolIds.includes(t.id))
    .map((t) => t.connectorId),
);
const resources = this.planResources(
  serverId,
  resourceConnectors.filter((c) => allowedConnectorIds.has(c.id)),
  instructions,
);

Please also add a test for it. The rule I go by after an isolation bug we shipped earlier this year: an isolation test that does not assert the negative case is not a test. So: same server, a role that denies connector B's tools, assert connector B's URI is absent from the planned list.

2. Drop the data branch in contentFromFetchConfig.

fetchConfig is an operator-written JSON blob. Your text and content branches are fine because someone had to type prose in there, but 'data' in config will happily serialise whatever is in that object into the model's context, and a fetch config is exactly where someone stores a URL with a key in it. I would keep only explicit text/content and let everything else fall to the placeholder, which is the fail-closed behaviour you already wrote for URLs and correctly tested against the metadata IP.

Worth knowing: nothing in the codebase writes McpResource rows today. The model is in the schema and there is no API, UI or importer behind it, so the persisted branch cannot actually be exercised end to end yet. I am fine keeping the code path, but that is why point 2 costs you nothing.

Two smaller notes, neither blocking:

  • registerResources(server: any, ...) with a typeof register !== 'function' guard: type it as McpServer and let the compiler tell us when the SDK changes. We pin the SDK, so the runtime guard is defending against something that cannot happen silently.
  • Native MCP elicitation and resources #593 also names the knowledge graph as a resource, and that is the one I am most interested in. KgService is already injected in the controller. Happy to take it as a second PR.

And the only other blocker is out of my hands: the CLA check is red. Post the one line the bot asks for and I can merge as soon as the two points above are in.

Signed-off-by: Aditya Datta <crazyme07071996@gmail.com>
@adity982

Copy link
Copy Markdown
Author

Addressed both blockers in 66f35d7:

  • Resource planning now derives visible connector IDs from the same role-filtered server tool set, so a connector with no role-allowed tools contributes neither its instruction URI nor persisted resources. The new negative test covers one allowed and one denied connector on the same server.
  • contentFromFetchConfig now accepts only explicit string text/content; arbitrary data (including metadata that could contain credentials) falls through to the fail-closed placeholder.

I also typed registerResources to the pinned McpServer API and removed the unreachable runtime capability guard.

Validation on the pushed head:

  • focused backend Jest: 16 passed across the endpoint and resource registry suites
  • scoped ESLint: passed
  • backend build: passed after supplying a local dummy DATABASE_URL for Prisma config parsing (no database connection)
  • git diff --check: passed

I have not posted the CLA attestation; that remains a separate author decision.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants