Skip to content

MCP custom resource reads (mcpResources) bypass row-level allowRead — confirmed data leak #1735

Description

@kriszyp

Summary

The MCP custom-content-resources hook shipped in #1609 (static mcpResources) unconditionally bypasses row-level allowRead. The natural, realistic way a component author uses this feature is to wrap real table data behind a friendly MCP URI (e.g. orders:///{+orderId} → a customer's own order) — exactly how #1609's own shipped example is written. When the underlying table has a row-level allowRead(user) hook, the same low-priv-user request that correctly 403s over REST returns 200 with the full forbidden row's content over MCP resources/read.

Positive controls confirm this isn't a broken handler or a config mistake: the user's own order reads fine, an admin reads both. It's also not an enumeration leak — resources/list/templates/list don't separately expose the forbidden id. It's specifically the read-time permission gate that's missing.

Root cause

readCustomResource (components/mcp/resources.ts) calls the author's read(params, { user, profile }) directly:

async function readCustomResource(uri, custom, user, profile) {
	const { def, params } = custom;
	const result = await def.read(params, { user, profile });
	...

No transaction(), no contextStorage.run() — so context.authorize is never set, and the internal tables.Order.get(id) call inside the author's read() skips allowRead entirely.

The same file's subscribeToResource does this correctly, right next to it:

const context = { user, authorize: true, request };
const result = await transaction(context, async () => ResourceClass.subscribe!(request, context));

The fix pattern already exists in the same module — it just wasn't applied to the new read path.

Pattern note

This is the third independent instance of this same shape in Harper's MCP layer:

  • MCP verb-tools had the identical missing-context-wiring gap, already fixed via buildContext+liveResource.
  • Relationship-expansion allowRead bypass (a different code path, same root-cause class) is tracked separately.
  • This one: MCP resource reads.

Might be worth a structural guardrail (e.g. a lint/test asserting any registered read/write handler threads an authorize-context) rather than fixing these one at a time as they're discovered — flagging for visibility, not proposing scope for this issue.

Repro

Local repro (not yet in the tracked suite): integrationTests/qa-scratch/qa515-mcp-resources-allowread.test.ts. A table with a row-level allowRead(user) hook, wrapped behind an mcpResources URI per #1609's own example; a low-priv user's own row reads fine over both REST and MCP; the SAME user's forbidden row correctly 403s over REST but returns 200 with full content over MCP resources/read.

— KrAIs 🤖 (exploratory QA, on Kris's behalf)

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:mcpModel Context Protocol (MCP) server: protocol, profiles, stdio CLIarea:securitySecurity, TLS/certs, authentication, authorizationbugSomething isn't working

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions