Skip to content

Commit e8e0d34

Browse files
nizzlenitzclaude
authored andcommitted
fix(mcp): list verb tools for table-less Resources to any authenticated user (#1940)
`makeVisibleTo` returned false for a Resource with no databaseName/tableName, so every non-super user saw none of its verb tools in tools/list. That hid exactly the Resources #1920/#1921 taught to produce rich schemas, and hid nothing meaningful: tools/call already accepts these tools by name, so the gate cost discoverability without buying access control. Return true instead and let the Resource's own allow* predicates enforce at call time — the contract custom `mcpTools` have always had (they have never had a listing filter beyond authentication). The table-backed path is unchanged and still gates on per-table permissions. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0188G62J9fZQg4J9rVuqLzjy
1 parent 8811652 commit e8e0d34

2 files changed

Lines changed: 33 additions & 5 deletions

File tree

components/mcp/tools/application.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -708,11 +708,12 @@ function makeVisibleTo(
708708
return function visibleTo(user: AuthedUser): boolean {
709709
// Super-user sees everything.
710710
if (user?.role?.permission?.super_user === true) return true;
711-
// Non-table Resources have no static permission gate — runtime allow*
712-
// predicates enforce. Conservative default: don't list them for
713-
// non-super users; they can still call via tools/call if they know
714-
// the tool name (pass-through is documented behavior).
715-
if (!databaseName || !tableName) return false;
711+
// A Resource with no backing table has no static permission gate to consult, so listing is
712+
// open to any authenticated caller and the Resource's own `allow*` predicates enforce at call
713+
// time. This matches custom `mcpTools`, which have never had a listing filter beyond
714+
// authentication — and hiding these while still accepting `tools/call` on them bought no
715+
// security, only undiscoverable tools (#1940).
716+
if (!databaseName || !tableName) return true;
716717
const perm = getUserTablePermissions(user, databaseName, tableName);
717718
if (!perm) return false;
718719
if (mode === 'read') return perm.read === true || perm.describe === true;

unitTests/components/mcp/tools/application.test.js

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,33 @@ describe('mcp/tools/application — registration', () => {
331331
assert.equal(update.visibleTo(ALICE_WRITE), true);
332332
});
333333

334+
it('lists verb tools for a table-less Resource to any authenticated user (#1940)', () => {
335+
// No databaseName/tableName means no static permission gate to consult, so listing is open and
336+
// the Resource's own allow* predicates enforce at call time — the same contract custom
337+
// `mcpTools` have always had. Hiding these while still accepting tools/call bought no security.
338+
const Inventory = makeTableResource({
339+
primaryKey: 'sku',
340+
attributes: [{ name: 'sku', type: 'String', isPrimaryKey: true }],
341+
});
342+
_setResourcesForTest(makeRegistry([['Inventory', { Resource: Inventory }]]));
343+
registerApplicationTools();
344+
for (const name of ['get_Inventory', 'search_Inventory', 'create_Inventory', 'delete_Inventory']) {
345+
const tool = getTool(name);
346+
assert.ok(tool, `${name} registered`);
347+
assert.equal(tool.visibleTo(NOBODY), true, `${name} is listed for a user with no table grants`);
348+
assert.equal(tool.visibleTo(SUPER), true, `${name} is listed for super_user`);
349+
}
350+
});
351+
352+
it('still gates a table-backed Resource by table permissions after #1940', () => {
353+
// Guards against the #1940 change leaking into the table-backed path.
354+
const Product = makeTableResource({ databaseName: 'data', tableName: 'product' });
355+
_setResourcesForTest(makeRegistry([['Product', { Resource: Product }]]));
356+
registerApplicationTools();
357+
assert.equal(getTool('get_Product').visibleTo(NOBODY), false);
358+
assert.equal(getTool('delete_Product').visibleTo(ALICE_READ), false);
359+
});
360+
334361
it('flags delete_ tools as destructive and get_/search_ as readOnly', () => {
335362
const Product = makeTableResource({ databaseName: 'data', tableName: 'product' });
336363
_setResourcesForTest(makeRegistry([['Product', { Resource: Product }]]));

0 commit comments

Comments
 (0)