Skip to content

Commit 11481bf

Browse files
oratisclaude
andcommitted
fix(mcp): gate mcp serve through the tool dispatcher
`deepcode mcp serve` exposes Read/Write/Edit/Bash/Grep/Glob to whatever MCP client connects — usually another agent. It called `tool.execute` directly: no mode, no permission rules, no file contract, no PreToolUse hooks. The CLI did not pass a sandbox config either, so Bash ran unsandboxed as well. This is the same shape as the `runAgent` bypass fixed in #181, in an entry point that fix did not reach. DEVELOPMENT_PLAN §"风险" listed it — "`deepcode mcp serve` 反向暴露的线程/权限模型缺失", with the mitigation "M3 出独立 design doc". The design doc was never written and the feature shipped anyway. Every call now goes through `dispatchToolCall`. `gate` is a required field on `BuildMcpServerOpts` rather than an optional one, because AGENTS.md's rule is that safety must not depend on a host remembering an argument — optional is how this happened. Nobody is attached to that pipe, so `ask` is refused rather than granted; otherwise "whoever connected" becomes the authority on what may run. A permissive `permissions.defaultMode` is clamped to `default` through the same `resolveTriggerMode` a scheduled job uses, since `bypassPermissions` is a decision about sitting at a REPL. `--mode` is the explicit opt-in back out, and `--sandbox` now applies too. Directory trust gates project settings, so an untrusted checkout cannot widen the posture of the server serving it. This is breaking: a peer can now do what `permissions.allow` says and nothing else. That is the point, and the startup banner says which mode is in effect. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 7fa4611 commit 11481bf

9 files changed

Lines changed: 400 additions & 10 deletions

File tree

CHANGELOG.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### ⚠️ Breaking
11+
12+
- **`deepcode mcp serve` now applies your permission settings.** It executed
13+
Read/Write/Edit/Bash for any connected MCP peer with no mode, no permission
14+
rules, no file contract and no `PreToolUse` hooks — the same shape as the
15+
`runAgent` bypass fixed in #181, in a different entry point. Every call now
16+
goes through the central gate, a call that would need approval is **refused**
17+
(nobody is attached to that pipe to approve it), and a permissive
18+
`permissions.defaultMode` is clamped to `default` exactly as a scheduled job's
19+
is. A peer can now do what `permissions.allow` says it can and nothing else,
20+
so anyone relying on the old behaviour must add rules — or start the server
21+
with an explicit `--mode`. `--sandbox` also applies now; it did not before.
22+
1023
### 🔒 Security
1124

1225
- **A sub-agent did not inherit the file contract.** The `Task` delegation

apps/cli/src/cli.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,10 @@ async function main(): Promise<number> {
100100
cwd: process.cwd(),
101101
output: process.stdout,
102102
errOutput: process.stderr,
103+
// `mcp serve` has no attached user, so a permissive ambient mode is
104+
// clamped unless --mode says otherwise. Same rule as a scheduled job.
105+
mode: args.mode,
106+
sandbox: args.sandbox,
103107
});
104108
}
105109
if (args.positional[0] === 'app-server') {

apps/cli/src/mcp-cmd.test.ts

Lines changed: 98 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
1+
import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises';
2+
import { tmpdir } from 'node:os';
3+
import { join } from 'node:path';
14
import { Writable } from 'node:stream';
2-
import { describe, expect, it } from 'vitest';
5+
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
6+
import type { Mode, ServeMcpStdioOpts } from '@deepcode/core';
37
import { runMcpCommand } from './mcp-cmd.js';
48

59
function sink(): { stream: Writable; text: () => string } {
@@ -51,3 +55,96 @@ describe('runMcpCommand', () => {
5155
expect(err.text()).toMatch(/\[mcp\] ready: Read, Write/);
5256
});
5357
});
58+
59+
// The served tools are Read/Write/Edit/Bash in a real project, and nobody is on
60+
// the other end of the pipe to approve anything.
61+
describe('runMcpCommand serve — permission posture', () => {
62+
let home: string;
63+
let cwd: string;
64+
65+
beforeEach(async () => {
66+
home = await mkdtemp(join(tmpdir(), 'dc-mcp-home-'));
67+
cwd = await mkdtemp(join(tmpdir(), 'dc-mcp-cwd-'));
68+
});
69+
afterEach(async () => {
70+
await rm(home, { recursive: true, force: true });
71+
await rm(cwd, { recursive: true, force: true });
72+
});
73+
74+
async function serve(opts: { mode?: Mode } = {}): Promise<{
75+
err: string;
76+
captured: ServeMcpStdioOpts | undefined;
77+
}> {
78+
const out = sink();
79+
const err = sink();
80+
let captured: ServeMcpStdioOpts | undefined;
81+
await runMcpCommand(['serve'], {
82+
cwd,
83+
home,
84+
output: out.stream,
85+
errOutput: err.stream,
86+
mode: opts.mode,
87+
serve: async (o) => {
88+
captured = o;
89+
},
90+
});
91+
return { err: err.text(), captured };
92+
}
93+
94+
async function writeUserSettings(settings: Record<string, unknown>): Promise<void> {
95+
await mkdir(join(home, '.deepcode'), { recursive: true });
96+
await writeFile(join(home, '.deepcode', 'settings.json'), JSON.stringify(settings));
97+
}
98+
99+
it('passes a gate at all — the server cannot be built without one', async () => {
100+
const { captured } = await serve();
101+
expect(captured?.gate).toBeTypeOf('function');
102+
});
103+
104+
it('refuses a call that would need approval', async () => {
105+
const { captured } = await serve();
106+
const verdict = await captured!.gate({
107+
tool: 'Write',
108+
input: { file_path: join(cwd, 'x.txt'), content: 'x' },
109+
});
110+
expect(verdict.allowed).toBe(false);
111+
expect(verdict.reason).toMatch(/no attached user/);
112+
});
113+
114+
it('clamps a permissive ambient mode and says so', async () => {
115+
// `bypassPermissions` in settings.json is a choice about sitting at a REPL.
116+
// Inheriting it here would hand "never ask me" to whatever connected.
117+
await writeUserSettings({ permissions: { defaultMode: 'bypassPermissions' } });
118+
const { err, captured } = await serve();
119+
expect(err).toMatch(/was not applied to this unattended run/);
120+
expect(err).toMatch(/mode=default/);
121+
122+
const verdict = await captured!.gate({
123+
tool: 'Bash',
124+
input: { command: 'rm -rf /' },
125+
});
126+
expect(verdict.allowed).toBe(false);
127+
});
128+
129+
it('--mode is the explicit opt-in back out of the clamp', async () => {
130+
await writeUserSettings({ permissions: { defaultMode: 'bypassPermissions' } });
131+
const { err, captured } = await serve({ mode: 'bypassPermissions' });
132+
expect(err).not.toMatch(/was not applied/);
133+
expect(err).toMatch(/mode=bypassPermissions/);
134+
expect((await captured!.gate({ tool: 'Bash', input: { command: 'echo hi' } })).allowed).toBe(
135+
true,
136+
);
137+
});
138+
139+
it('honours permissions.allow without any mode change', async () => {
140+
await writeUserSettings({ permissions: { allow: ['Read'] } });
141+
const { captured } = await serve();
142+
expect(
143+
(await captured!.gate({ tool: 'Read', input: { file_path: join(cwd, 'a') } })).allowed,
144+
).toBe(true);
145+
expect(
146+
(await captured!.gate({ tool: 'Write', input: { file_path: join(cwd, 'a'), content: '' } }))
147+
.allowed,
148+
).toBe(false);
149+
});
150+
});

apps/cli/src/mcp-cmd.ts

Lines changed: 68 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,23 @@
66
// line goes to stderr, and nothing else may touch stdout.
77

88
import {
9+
HookDispatcher,
910
VERSION,
11+
buildMcpGate,
12+
describeClamp,
13+
gateUntrustedSettings,
14+
loadFileContract,
15+
loadSettings,
1016
mcpServableTools,
17+
resolveTriggerMode,
1118
serveMcpOverStdio,
19+
withSandboxMode,
20+
type Mode,
21+
type SandboxMode,
1222
type ServeMcpStdioOpts,
1323
} from '@deepcode/core';
1424
import type { Writable } from 'node:stream';
25+
import { TrustStore } from './trust.js';
1526

1627
export interface McpCmdDeps {
1728
cwd: string;
@@ -23,6 +34,12 @@ export interface McpCmdDeps {
2334
signal?: AbortSignal;
2435
/** Serve implementation — injectable so tests don't grab the real stdio. */
2536
serve?: (opts: ServeMcpStdioOpts) => Promise<void>;
37+
/** Override `~/.deepcode` (tests). */
38+
home?: string;
39+
/** `--mode`: the explicit opt-in out of the unattended clamp. */
40+
mode?: Mode;
41+
/** `--sandbox`: tightens the sandbox for served commands. */
42+
sandbox?: SandboxMode;
2643
}
2744

2845
export async function runMcpCommand(sub: string[], deps: McpCmdDeps): Promise<number> {
@@ -32,13 +49,62 @@ export async function runMcpCommand(sub: string[], deps: McpCmdDeps): Promise<nu
3249

3350
if (cmd === 'serve') {
3451
const tools = mcpServableTools();
52+
const { cwd } = deps;
53+
54+
// The served tools are Read/Write/Edit/Bash in a real project. Which of
55+
// them a peer may call is a settings question, not a "you connected, so
56+
// you may" question — so load the same policy every other host loads,
57+
// including the directory trust gate that stops an untrusted checkout from
58+
// widening its own permissions.
59+
const loaded = await loadSettings({ cwd, home: deps.home });
60+
const trustStatus = await new TrustStore({ home: deps.home }).statusFor(cwd);
61+
const trustGate = gateUntrustedSettings(loaded, trustStatus);
62+
const settings = trustGate.settings;
63+
if (trustGate.gated.length > 0) {
64+
err.write(
65+
`[mcp] untrusted directory — ignoring project ${trustGate.gated.join(', ')}. ` +
66+
`Run \`deepcode trust\` to enable.\n`,
67+
);
68+
}
69+
70+
// Nobody is attached to this pipe, so a permissive `defaultMode` picked for
71+
// REPL convenience must not become the posture of whatever connects. Same
72+
// clamp, and the same explicit opt-in, that scheduled jobs use.
73+
const ambient = (settings.permissions?.defaultMode ?? 'default') as Mode;
74+
const resolved = resolveTriggerMode(deps.mode ? { mode: deps.mode } : undefined, ambient);
75+
const clampNote = describeClamp(resolved);
76+
if (clampNote) err.write(`[mcp] ${clampNote}\n`);
77+
78+
const contract = await loadFileContract({ cwd, home: deps.home });
79+
if (contract.status === 'invalid') {
80+
err.write(`[mcp] file contract could not be parsed: ${contract.error}\n`);
81+
}
82+
83+
const hooks = new HookDispatcher({
84+
hooks: settings.hooks,
85+
disableAllHooks: settings.disableAllHooks,
86+
allowedHttpHookUrls: settings.allowedHttpHookUrls,
87+
});
88+
3589
err.write(
36-
`DeepCode MCP server v${VERSION} — exposing ${tools.length} tools over stdio in ${deps.cwd}\n`,
90+
`DeepCode MCP server v${VERSION} — exposing ${tools.length} tools over stdio in ${cwd}\n`,
3791
);
92+
err.write(`[mcp] mode=${resolved.mode}; calls needing approval are refused, not granted\n`);
93+
3894
await (deps.serve ?? serveMcpOverStdio)({
39-
cwd: deps.cwd,
95+
cwd,
4096
version: VERSION,
4197
signal: deps.signal,
98+
gate: buildMcpGate({
99+
cwd,
100+
mode: resolved.mode,
101+
permissions: settings.permissions,
102+
contract: contract.contract,
103+
hooks,
104+
autoMode: settings.autoMode,
105+
}),
106+
contract: contract.contract,
107+
sandboxConfig: withSandboxMode(settings.sandbox, deps.sandbox),
42108
onReady: (names) => err.write(`[mcp] ready: ${names.join(', ')}\n`),
43109
});
44110
return 0;

docs/security-model.md

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,32 @@ decreasing order of operator severity:
2424
| 8 | Model reads an in-project secret (`.env`, `*.pem`) through a read tool | Partly mitigated | File contract `read: deny` — covers Read/Grep/Glob, **not Bash** (see below) |
2525
| 9 | Unattended job runs with a permissive mode inherited from interactive settings | Mitigated | Trigger profile clamp + `onApprovalRequired` |
2626
| 10 | User cannot audit or undo what the agent wrote | Mitigated | Change ledger + `deepcode ledger rollback` through the apply ceremony |
27+
| 11 | An MCP peer calls DeepCode's own tools without any policy | Mitigated | `mcp serve` routes every call through `dispatchToolCall`; `ask` is refused (see below) |
28+
29+
### `deepcode mcp serve` runs with nobody attached
30+
31+
`mcp serve` exposes Read / Write / Edit / Bash / Grep / Glob to whatever MCP
32+
client connects — typically another agent. It executed them directly, with no
33+
mode, no permission rules, no file contract and no `PreToolUse` hooks: the same
34+
shape as the `runAgent` bypass fixed in #181, in a different entry point. The
35+
original plan listed the missing permission model as a known risk and deferred
36+
the design document; the feature shipped without either.
37+
38+
Every call now goes through `dispatchToolCall`, and:
39+
40+
- **`ask` is refused, not granted.** There is no user on that pipe. Granting
41+
would make "whoever connected" the authority on what may run.
42+
- **A permissive `permissions.defaultMode` is clamped** to `default`, exactly as
43+
a scheduled job's is. `bypassPermissions` is a decision about sitting at a
44+
REPL; inheriting it here hands "never ask me" to a peer.
45+
- **`--mode` is the explicit opt-in** back out of that clamp, and `--sandbox`
46+
tightens the sandbox for served commands.
47+
- Directory trust still gates project settings, so an untrusted checkout cannot
48+
widen the posture the server runs under.
49+
50+
The practical consequence: a peer can do what `permissions.allow` says it can,
51+
and nothing else. That is a real reduction in capability for anyone who was
52+
relying on the old behaviour, and it is the point.
2753

2854
### Residual risk: the file contract is policy, not a boundary
2955

packages/core/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,7 @@ export {
276276
connectAllMcpServers,
277277
closeAllMcpServers,
278278
buildMcpServer,
279+
buildMcpGate,
279280
serveMcpOverStdio,
280281
mcpServableTools,
281282
MCP_SERVE_EXCLUDE,

packages/core/src/mcp/index.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,14 @@ export {
3333

3434
export {
3535
buildMcpServer,
36+
buildMcpGate,
3637
serveMcpOverStdio,
3738
mcpServableTools,
3839
MCP_SERVE_EXCLUDE,
3940
type BuildMcpServerOpts,
41+
type McpGateOptions,
42+
type McpGateVerdict,
43+
type McpToolGate,
4044
type ServeMcpStdioOpts,
4145
} from './serve.js';
4246

0 commit comments

Comments
 (0)