diff --git a/packages/devframe/src/adapters/mcp/__tests__/mcp-server.test.ts b/packages/devframe/src/adapters/mcp/__tests__/mcp-server.test.ts index cf9f7b4b0..dea57b83b 100644 --- a/packages/devframe/src/adapters/mcp/__tests__/mcp-server.test.ts +++ b/packages/devframe/src/adapters/mcp/__tests__/mcp-server.test.ts @@ -12,13 +12,13 @@ function nullHost(): DevframeHost { } } -async function bootPair() { +async function bootPair(exposeSharedState: boolean | ((key: string) => boolean) = true) { const ctx = await createHostContext({ cwd: process.cwd(), mode: 'dev', host: nullHost() }) const server = buildMcpServerFromContext(ctx, { serverName: 'test', serverVersion: '0.0.0-test', - exposeSharedState: true, + exposeSharedState, }) const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair() @@ -312,40 +312,22 @@ describe('mcp adapter (in-memory)', () => { }) it('hides devframe:state:read when shared-state exposure is disabled', async () => { - const ctx = await createHostContext({ cwd: process.cwd(), mode: 'dev', host: nullHost() }) - const server = buildMcpServerFromContext(ctx, { - serverName: 'test', - serverVersion: '0.0.0-test', - exposeSharedState: false, - }) - const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair() - await server.connect(serverTransport) - const client = new Client({ name: 'test-client', version: '0.0.0' }) - await client.connect(clientTransport) + const { client, cleanup } = await bootPair(false) try { const listed = await client.listTools() expect(listed.tools.map(t => t.name)).not.toContain('devframe_state_read') } finally { - await client.close() - await server.close() + await cleanup() } }) it('respects the shared-state filter in devframe:state:read', async () => { - const ctx = await createHostContext({ cwd: process.cwd(), mode: 'dev', host: nullHost() }) - await ctx.rpc.sharedState.get('visible:key', { initialValue: { n: 1 } }) - await ctx.rpc.sharedState.get('hidden:key', { initialValue: { n: 2 } }) - const server = buildMcpServerFromContext(ctx, { - serverName: 'test', - serverVersion: '0.0.0-test', - exposeSharedState: key => key.startsWith('visible:'), - }) - const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair() - await server.connect(serverTransport) - const client = new Client({ name: 'test-client', version: '0.0.0' }) - await client.connect(clientTransport) + const { ctx, client, cleanup } = await bootPair(key => key.startsWith('visible:')) try { + await ctx.rpc.sharedState.get('visible:key', { initialValue: { n: 1 } }) + await ctx.rpc.sharedState.get('hidden:key', { initialValue: { n: 2 } }) + const keys = await client.callTool({ name: 'devframe_state_read', arguments: {} }) expect(keys.structuredContent).toEqual({ keys: ['visible:key'] }) @@ -353,8 +335,75 @@ describe('mcp adapter (in-memory)', () => { expect(hidden.isError).toBe(true) } finally { - await client.close() - await server.close() + await cleanup() + } + }) + + it('omits state resources and rejects a direct URI read when exposure is disabled', async () => { + const { ctx, client, cleanup } = await bootPair(false) + try { + await ctx.rpc.sharedState.get('alpha:key', { initialValue: { n: 1 } }) + + const listed = await client.listResources() + expect(listed.resources.some(r => r.uri.startsWith('devframe://state/'))).toBe(false) + + // A caller that knows the key must not bypass the policy by reading the + // URI directly. + await expect( + client.readResource({ uri: `devframe://state/${encodeURIComponent('alpha:key')}` }), + ).rejects.toThrow() + } + finally { + await cleanup() + } + }) + + it('applies the filter to direct URI reads, not only discovery', async () => { + const { ctx, client, cleanup } = await bootPair(key => key.startsWith('visible:')) + try { + await ctx.rpc.sharedState.get('visible:key', { initialValue: { n: 1 } }) + await ctx.rpc.sharedState.get('hidden:key', { initialValue: { n: 2 } }) + + const listed = await client.listResources() + const stateUris = listed.resources.filter(r => r.uri.startsWith('devframe://state/')).map(r => r.uri) + expect(stateUris).toEqual([`devframe://state/${encodeURIComponent('visible:key')}`]) + + const allowed = await client.readResource({ uri: `devframe://state/${encodeURIComponent('visible:key')}` }) + const c = allowed.contents[0] as { text: string } + expect(JSON.parse(c.text)).toEqual({ n: 1 }) + + // Known key, denied by predicate: rejected before storage access. + await expect( + client.readResource({ uri: `devframe://state/${encodeURIComponent('hidden:key')}` }), + ).rejects.toThrow() + } + finally { + await cleanup() + } + }) + + it('agrees between the state-read tool and the resource path for the same policy', async () => { + const { ctx, client, cleanup } = await bootPair(key => key.startsWith('visible:')) + try { + await ctx.rpc.sharedState.get('visible:key', { initialValue: { n: 1 } }) + await ctx.rpc.sharedState.get('hidden:key', { initialValue: { n: 2 } }) + + // Allowed key: both paths return the same value. + const toolValue = await client.callTool({ name: 'devframe_state_read', arguments: { key: 'visible:key' } }) + expect(toolValue.structuredContent).toEqual({ key: 'visible:key', value: { n: 1 } }) + const resourceValue = await client.readResource({ uri: `devframe://state/${encodeURIComponent('visible:key')}` }) + const c = resourceValue.contents[0] as { text: string } + expect(JSON.parse(c.text)).toEqual({ n: 1 }) + + // Denied key: both paths reject. + const toolHidden = await client.callTool({ name: 'devframe_state_read', arguments: { key: 'hidden:key' } }) + expect(toolHidden.isError).toBe(true) + await expect( + client.readResource({ uri: `devframe://state/${encodeURIComponent('hidden:key')}` }), + ).rejects.toThrow() + } + finally { + await cleanup() } }) }) diff --git a/packages/devframe/src/adapters/mcp/build-server.ts b/packages/devframe/src/adapters/mcp/build-server.ts index 237c8fb63..7c6c7425e 100644 --- a/packages/devframe/src/adapters/mcp/build-server.ts +++ b/packages/devframe/src/adapters/mcp/build-server.ts @@ -332,6 +332,8 @@ function registerResourceHandlers( ctx: DevframeNodeContext, exposeSharedState: boolean | ((key: string) => boolean), ): void { + const stateFilter = sharedStateFilter(exposeSharedState) + server.setRequestHandler('resources/list', async () => { const resources = ctx.agent.list().resources.map(resource => ({ uri: resource.uri, @@ -340,10 +342,9 @@ function registerResourceHandlers( mimeType: resource.mimeType, })) - if (exposeSharedState !== false) { - const filter = typeof exposeSharedState === 'function' ? exposeSharedState : () => true + if (stateFilter) { for (const key of ctx.rpc.sharedState.keys()) { - if (!filter(key)) + if (!stateFilter(key)) continue resources.push({ uri: `devframe://state/${encodeURIComponent(key)}`, @@ -375,6 +376,12 @@ function registerResourceHandlers( } if (parsed.kind === 'state') { + // Apply the exposure policy at the read, not only during discovery: + // a caller that knows a filtered key must not bypass it. Deny with the + // same DF0048 the built-in read tool uses, so a denied key is + // indistinguishable from a missing one. + if (!stateFilter || !stateFilter(parsed.key)) + throw diagnostics.DF0048({ key: parsed.key }) const state = await ctx.rpc.sharedState.get(parsed.key) return { contents: [ diff --git a/plans/README.md b/plans/README.md index 3def85aec..7d0470e7a 100644 --- a/plans/README.md +++ b/plans/README.md @@ -8,7 +8,7 @@ Generated by the improve skill on 2026-09-01 at commit `2d978f84`. Execute in th |---|---|---|---|---|---| | 001 | Pin privileged GitHub Actions dependencies | P1 | S | - | TODO | | 002 | Require authentication on route-based MCP | P1 | M | 001 | TODO | -| 003 | Enforce shared-state exposure policy on direct MCP reads | P1 | S | 002 | TODO | +| 003 | Enforce shared-state exposure policy on direct MCP reads | P1 | S | 002 | DONE | | 004 | Contain remote asset materialization | P1 | S | - | DONE | | 005 | Block Data Inspector prototype-chain writes | P1 | S | - | DONE | | 006 | Validate request-derived authentication-link origins | P1 | M | - | DONE |