diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f3b77d..b8a244e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,21 @@ package versions for a release. ## Unreleased +- **Breaking (MCP): `"prompt"`-policy consent is elicitation-only.** The Claude Code-specific + fallback is gone: `tools/list` no longer emits `_meta["anthropic/requiresUserInteraction"]`, and + the MCP server no longer sends `consent: "client"`. A `"prompt"` tool called from an MCP client + that doesn't declare the `elicitation` capability is now denied with `policy_denied` (reason + `no_consent_channel`), the same as the CLI. Current Claude Code declares elicitation, so it gets + the elicitation prompt instead; only a client that relied on the flag without supporting + elicitation loses access. To fix that, use a client that supports elicitation, or set the tool's + policy to `"allow"` in `config.json` — which removes the gate for every caller, including the + CLI — and restart the daemon (`appduct daemon stop`; the next command starts it again), since + `config.json` is read once at daemon start. + - A daemon with this change that receives `consent: "client"` from an older MCP server treats it + as no consent, so the call is denied and audited as `no_consent_channel`. + - `@appduct/shared`: `ToolsCallParams.consent` and the audit record's `consent` narrow to + `"elicitation"`. New audit records never carry `"client"`; existing audit files may. + - **`config.json`'s `wssPort` accepts `0`, meaning "bind an OS-assigned port".** The pinned-wss listener takes whatever ephemeral port the OS hands it, and everything that reports or advertises the port — `daemon.status`'s `wssPort`, a minted link's `endpoint.port`, and so the deep link and diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index bb0123b..6c5db96 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -27,10 +27,9 @@ Deliberately out of scope, so the boundaries of the design are explicit: - Arbitrary code execution inside the app — only pre-registered, named tools. - Anonymous or unauthenticated remote access. - Treating deep links as proof of authority. -- A general-purpose interactive consent UI. `policy: "prompt"` (§12) exists, but its only two - implemented gates are both MCP-only: an MCP client that declares the `elicitation` capability - (preferred) or one that enforces `_meta["anthropic/requiresUserInteraction"]` (fallback) — - every other caller (the CLI, an MCP client on neither channel) fails closed rather than getting +- A general-purpose interactive consent UI. `policy: "prompt"` (§12) exists, but its only + implemented gate is MCP elicitation: an MCP client that declares the `elicitation` capability — + every other caller (the CLI, an MCP client without elicitation) fails closed rather than getting a prompt of its own. - Remote relay to hosts outside the operator machine. - Pinning an offline anchor CA that signs short-lived leaf certs. The current model pins @@ -263,7 +262,7 @@ Methods: | `sessions.describe` | `{ selector? }` | full session detail incl. device metadata, state timestamps, tool count | | `sessions.revoke` | `{ selector? }` | `{ ok: true }` — closes socket (code 1000), frees alias | | `tools.list` | `{ selector?, filter?, limit?, offset? }` | `{ tools: ToolsListEntry[], total }` — `tools` is the registry sorted by `name` (code-point order), `filter`ed (case-insensitive substring match against name/description) and paged with `limit`/`offset`; each entry is a `ToolDescriptor` (full schema + annotations) plus the tool's effective `policy: "allow" \| "deny" \| "prompt"` (§12), resolved daemon-side. `total` is the filtered count *before* paging, so a caller can tell how much a page left out | -| `tools.call` | `{ selector?, name, args, timeoutMs?, caller?: "cli" \| "mcp", consent?: "client" \| "elicitation" }` | `{ result, callId }` on success — `callId` lets a caller with several in-flight calls match `tool_call_progress`/`tool_call_finished` events back to this call; JSON-RPC error with `data.type` preserving the wire error type on failure. `caller` attributes the audit record (§12); `consent` is the MCP server's evidence of a `"prompt"`-policy human gate (§12) — `"client"` (the flag-based gate) or `"elicitation"` (the elicitation-based gate), absent for the CLI. | +| `tools.call` | `{ selector?, name, args, timeoutMs?, caller?: "cli" \| "mcp", consent?: "elicitation" }` | `{ result, callId }` on success — `callId` lets a caller with several in-flight calls match `tool_call_progress`/`tool_call_finished` events back to this call; JSON-RPC error with `data.type` preserving the wire error type on failure. `caller` attributes the audit record (§12); `consent` is the MCP server's evidence of a `"prompt"`-policy human gate (§12) — `"elicitation"` after the client accepted an elicitation prompt, absent otherwise (including for the CLI). | | `tools.cancel` | `{ selector?, callId, reason? }` | `{ cancelled: boolean }` — sends `tool_cancel` (§7) to the app for a still-pending call; `false` for an unknown/already-finished `callId` or no active socket (a no-op, not an error) | | `events.subscribe` | `{ sessionSelector?, kinds? }` | `{ ok: true }`, then `event` notifications on this connection | | `events.since` | `{ selector?, since?, kinds?, limit? }` | `{ events: EventNotification[], cursor }` — pull counterpart to `events.subscribe`, draining the per-session retention buffer described below | @@ -467,20 +466,14 @@ proxies daemon RPC (auto-spawning the daemon like any client): `notifications/tools/list_changed`, so an agent's tool list tracks the device. - Tool calls, progress frames, errors (with their `type` preserved), and descriptor annotations all map through verbatim. Two semantics the MCP surface does add: - `"prompt"`-policy consent (§12) — two channels, preferred in this order per connection: - (1) elicitation (issue #10), whenever the client declared the `elicitation` capability at - `initialize`: a `"prompt"`-policy call sends one `elicitation/create` request naming the + `"prompt"`-policy consent (§12) — one channel, elicitation (issue #10), used whenever the + client declared the `elicitation` capability at `initialize`: a `"prompt"`-policy call sends one `elicitation/create` request naming the tool, the session alias, and the call's arguments, and an `action: "accept"` reply becomes `consent: "elicitation"` on `tools.call`; a decline/cancel/timeout short-circuits to an MCP tool result with `isError: true` without ever reaching the daemon; a failed request (declared but rejected as unsupported, a transport error) is treated as no consent, never as approval. - (2) the flag-based fallback (issue #14), only reachable when the client didn't declare - elicitation: `tools/list` emits `_meta["anthropic/requiresUserInteraction"]` for a tool whose - effective policy is `"prompt"`, gated on the connected client's `initialize` `clientInfo`, and - `tools/call` echoes that gate back to the daemon as `consent: "client"`, only for a tool this - same connection's most recent listing actually flagged. The two channels never both arm for - the same call: whenever elicitation is preferred, the flag is never emitted at all. And - cancellation: an MCP client's `notifications/cancelled` maps to `tools.cancel` (§5), only for + A client that didn't declare elicitation gets no consent marker, so the daemon denies its + `"prompt"` calls. And cancellation: an MCP client's `notifications/cancelled` maps to `tools.cancel` (§5), only for a call that requested progress (the SDK only assigns a `progressToken`, and only the progress-tracked path opens the dedicated connection that learns `callId` while the call is still in flight; a non-progress call has no `callId` to cancel by until it has already @@ -884,9 +877,9 @@ it. `policy.tools["/"]`, each `"allow" | "deny" | "prompt"`. `allow`/`deny` behave as before; denied calls return `policy_denied` and are audited. - `"prompt"` means "a human gate is required; if one cannot be guaranteed, deny" — it - fails closed rather than silently behaving like `allow`. Two gates are implemented today, - both MCP-only, tried in this order per connection (issues #10 and #14): - 1. **Elicitation** (issue #10): whenever the connected client declared the `elicitation` + fails closed rather than silently behaving like `allow`. One gate is implemented today, and + it is MCP-only: + - **Elicitation** (issue #10): whenever the connected client declared the `elicitation` capability at `initialize` (checked via the SDK Server's `getClientCapabilities()`), a `"prompt"`-policy call sends one `elicitation/create` request — message naming the tool, the session alias, and the call's arguments (JSON, truncated past a small bound) — and @@ -899,24 +892,15 @@ it. unsupported anyway, or a transport error — that is treated as **no consent obtained**, never as approval: the call falls through to the daemon with no `consent` marker, landing on the same `policy_denied`/`no_consent_channel` path as any other ungated caller. - 2. **The flag-based fallback** (issue #14), reached only when the client did **not** declare - elicitation: `tools/list` emits `_meta["anthropic/requiresUserInteraction"] = true` for a - `"prompt"` tool, per connection, only when the connected client's `initialize` - `clientInfo` is known to enforce it (Claude Code ≥ v2.1.199 — every other client ignores - the flag). `tools/call` then sets `consent: "client"` only for a tool this same - connection's most recent `tools/list` actually flagged that way — not merely a tool whose - live policy happens to be `"prompt"` on a client that happens to qualify — so a call can't - ride on a stale or hypothetical listing. - - The two channels are mutually exclusive per connection, never both armed for the same call: - whenever a client declares elicitation, `tools/list` never emits the `requiresUserInteraction` - flag for it, so a "prompt" tool can't trigger two consent prompts for one call. Every caller on - neither channel (the CLI, an MCP client that declares neither capability) is denied with - `policy_denied`, reason `no_consent_channel`. - - Elicitation is the stronger of the two: it carries an *observed decision* (the client's - reply to a specific request), where the flag is only evidence the client *armed itself* to - ask — the daemon never sees the client's own prompt UI either way. `clientInfo` and a - client's declared capabilities are self-reported, and `consent` is an ordinary RPC param on + + Every other caller (the CLI, an MCP client that doesn't declare elicitation) is denied with + `policy_denied`, reason `no_consent_channel`. An earlier Claude Code-specific fallback that + emitted `_meta["anthropic/requiresUserInteraction"]` on `tools/list` and sent + `consent: "client"` was removed: it was evidence only that a client armed itself to ask, not + an observed decision, and it tied consent to one client's self-reported `clientInfo`. + - Elicitation carries an *observed decision* (the client's reply to a specific request), but + the daemon never sees the client's own prompt UI. A client's declared capabilities are + self-reported, and `consent` is an ordinary RPC param on `daemon.sock` — any local process that can reach the socket (the CLI, or an agent with shell access, which is the typical Claude Code setup this feature targets) can set it directly, same as it could send any other RPC call. `"prompt"` guards against a compliant @@ -927,21 +911,22 @@ it. `"decline"` or `"cancel"` without ever really surfacing the prompt to a human (older Codex behavior at the time of writing). This fails closed — the tool is simply never callable through that client — which is the acceptable failure mode; it is not distinguishable from - a human genuinely saying no. - - Two client-observable behaviors worth documenting rather than filing as bugs: - non-interactive Claude Code (`--permission-prompt-tool`) converts an `allow` result - for a flagged tool into a denial (`MCP tool requires user interaction; not - supported via --permission-prompt-tool`) — that conversion is the client's, not - the daemon's. And `"prompt"` denies unconditionally in any unattended pipeline - (CI has no consent channel at all); pipelines that need a tool to run + a human genuinely saying no. Non-interactive Claude Code (`claude -p`) behaves this way: it + declares elicitation and answers every request with `"cancel"`. + - The opposite also exists: a client can be configured to accept elicitations automatically + (Claude Code's `Elicitation` hook can answer without showing a dialog). That approval is the + user's own client configuration, and the daemon cannot tell it apart from a person + accepting. + - `"prompt"` denies unconditionally in any unattended pipeline (CI has no consent channel at + all); pipelines that need a tool to run unattended must set `allow`/`deny` explicitly for it rather than `"prompt"`. - Audit: every `tools.call` appends one JSONL record to `audit/.jsonl`: `{ ts, sessionId, alias, tool, argsSha256, outcome: "ok"|"error"|"denied"|"cancelled", - errorType?, durationMs, caller: "cli"|"mcp"|"client", consent?: "client"|"elicitation" }`. - `consent` is set only when a `"prompt"` call proceeded on one of the two channels above, and - its value names which one — `"client"` for the flag-based gate, `"elicitation"` for an - observed accept — kept distinct from a plain `"ok"` since the daemon never observes either - channel's client-side behavior itself, only that the call arrived carrying this marker. Raw + errorType?, durationMs, caller: "cli"|"mcp"|"client", consent?: "elicitation" }`. + `consent` is set only when a `"prompt"` call proceeded after an elicitation accept, kept + distinct from a plain `"ok"` since the daemon never observes the client-side prompt itself, + only that the call arrived carrying this marker. Audit files written before the flag-based + fallback was removed may also contain `consent: "client"`. Raw args are never logged. Day files are pruned on the `auditRetentionDays` schedule described in §3, and `daemon status` surfaces the directory's file count, size, and failure counters. @@ -984,10 +969,9 @@ named-pipe path `\\.\pipe\appduct-` behind the same client API. ## 14. Current limitations -- A general-purpose interactive consent UI. `policy: "prompt"` (§12) has two implemented - MCP-only gates (elicitation, issue #10; the `requiresUserInteraction` flag, issue #14) — the - CLI, and an MCP client that declares neither channel, still fail closed with no prompt of - their own. +- A general-purpose interactive consent UI. `policy: "prompt"` (§12) has one implemented + MCP-only gate (elicitation, issue #10) — the CLI, and an MCP client that doesn't declare + elicitation, still fail closed with no prompt of their own. - Remote relay / hosts outside the operator machine. - Pinning an offline anchor CA that signs short-lived leaves (rotation uses overlapping pin sets; the anchor-CA design is a future option). diff --git a/docs/SECURITY.md b/docs/SECURITY.md index 767d80b..d468793 100644 --- a/docs/SECURITY.md +++ b/docs/SECURITY.md @@ -353,23 +353,24 @@ not as the mechanism that keeps a destructive tool out of reach of a hostile one `appduct/client` alike — is evaluated against this before it ever reaches the app; a denial returns `policy_denied` and never sends a `tool_call` frame. `"prompt"` requires a human gate and fails closed everywhere one can't be guaranteed: today the only implemented gate - is an MCP client that enforces `_meta["anthropic/requiresUserInteraction"]` (Claude - Code ≥ v2.1.199); the CLI and every other client are denied outright + is an MCP client that declares the `elicitation` capability, which receives an + `elicitation/create` prompt for each call; the CLI and every other client are denied outright (`policy_denied`, reason `no_consent_channel`) rather than silently treated as `"allow"`. That gate is *not* a defense within this feature's own trust boundary: the - daemon trusts the MCP server's `consent: "client"` param verbatim rather than + daemon trusts the MCP server's `consent: "elicitation"` param verbatim rather than re-deriving it, so any local process that can reach `daemon.sock` — including the CLI, or an agent with shell access, which is the typical Claude Code setup this feature targets — could send that param directly, the same way it could send any other RPC call. This is consistent with, not an exception to, the trust boundary above: anything that can reach the socket already has full daemon control. `"prompt"` guards against a compliant MCP client silently auto-approving on the caller's behalf, not against a - hostile process on the operator's own machine. `clientInfo` is also self-reported, so - it's not a defense against a hostile client claiming to be a compliant one either — - only against a compliant client's own auto-approval. Two behaviors worth knowing about - rather than filing as bugs: non-interactive Claude Code (`--permission-prompt-tool`) - converts an `allow` result for a flagged tool into a denial — that conversion is the - client's, not the daemon's; and `"prompt"` denies unconditionally in CI or any other + hostile process on the operator's own machine. The prompt reaches whatever the client + does with elicitations: a client configured to answer them automatically (Claude Code's + `Elicitation` hook, for example) approves without showing anything, and the daemon cannot + tell that apart from a person accepting. A client's declared capabilities are + also self-reported, so it's not a defense against a hostile client that answers the + prompt itself either — only against a compliant client's own auto-approval. Worth + knowing rather than filing as a bug: `"prompt"` denies unconditionally in CI or any other unattended pipeline (there is no consent channel there at all), so a pipeline that needs a tool to run unattended must set `allow`/`deny` for it explicitly. Until a non-MCP consent channel ships, `"deny"` remains the only way to hard-block a tool for @@ -377,7 +378,8 @@ not as the mechanism that keeps a destructive tool out of reach of a hostile one - **Audit.** Every `tools.call` attempt — regardless of outcome — appends one line to `audit/.jsonl`: timestamp, session, alias, tool name, a sha256 of the canonicalized args (never the raw args), outcome, error type if any, duration, caller - (`cli`/`mcp`/`client`), and — only for a `"prompt"` call that proceeded — `consent: "client"`. + (`cli`/`mcp`/`client`), and — only for a `"prompt"` call that proceeded — `consent: "elicitation"` + (files written by older versions may also contain `consent: "client"`). That marker is the weakest form of evidence recorded here: the daemon never observes the actual consent decision, only that the call arrived already gated, so it's kept distinct from a plain `"ok"` rather than folded into it. This is on unconditionally; diff --git a/packages/appduct/src/__tests__/call-timeouts.test.ts b/packages/appduct/src/__tests__/call-timeouts.test.ts index 9bde828..d53ca86 100644 --- a/packages/appduct/src/__tests__/call-timeouts.test.ts +++ b/packages/appduct/src/__tests__/call-timeouts.test.ts @@ -101,7 +101,7 @@ describe("toMcpTool", () => { const toMcpTool = createMcpToolMapper(() => {}); test("never emits a timeout on the MCP tool, even for a tool that declares one", () => { - const mapped = toMcpTool(namespacedTool(60_000), false); + const mapped = toMcpTool(namespacedTool(60_000)); // The deadline is a daemon-side scheduling hint, not part of the MCP `Tool` contract. This // guards against a future refactor swapping the explicit field mapping for a spread — under @@ -112,7 +112,7 @@ describe("toMcpTool", () => { }); test("maps a tool that declares one identically to a tool that does not", () => { - expect(toMcpTool(namespacedTool(60_000), false)).toEqual(toMcpTool(namespacedTool(), false)); + expect(toMcpTool(namespacedTool(60_000))).toEqual(toMcpTool(namespacedTool())); }); }); diff --git a/packages/appduct/src/__tests__/policy-and-audit.integration.test.ts b/packages/appduct/src/__tests__/policy-and-audit.integration.test.ts index 129ec4a..7880971 100644 --- a/packages/appduct/src/__tests__/policy-and-audit.integration.test.ts +++ b/packages/appduct/src/__tests__/policy-and-audit.integration.test.ts @@ -220,7 +220,7 @@ type AuditRecord = { deniedReason?: "policy" | "no_consent_channel"; durationMs: number; caller: "cli" | "mcp"; - consent?: "client" | "elicitation"; + consent?: "elicitation"; }; const readAuditRecords = async (stateDir: string): Promise => { @@ -344,66 +344,30 @@ describe("policy: config validation", () => { }); }); -describe("policy: prompt via MCP requiresUserInteraction", () => { - /** A minimal `Client` that reports `clientInfo` the daemon/MCP server should recognize as - * honoring `_meta["anthropic/requiresUserInteraction"]` (ARCHITECTURE.md §12 / issue #14). */ - const connectCompliantClient = async (mcpHandle: McpServerHandle): Promise => { +describe("policy: prompt without elicitation", () => { + /** A client that doesn't declare the `elicitation` capability — the only `"prompt"` consent + * channel (ARCHITECTURE.md §12) — so every `"prompt"` call from it must be denied. */ + const connectClientWithoutElicitation = async ( + mcpHandle: McpServerHandle, + clientInfo: { name: string; version: string } = { name: "some-other-client", version: "9.9.9" }, + ): Promise => { const [serverTransport, clientTransport] = InMemoryTransport.createLinkedPair(); await mcpHandle.connect(serverTransport); - const client = new Client({ name: "claude-code", version: "2.1.199" }); + const client = new Client(clientInfo); await client.connect(clientTransport); return client; }; - test('tools/list emits _meta["anthropic/requiresUserInteraction"] for a "prompt" tool only for a compliant client', async () => { - const { daemon, port, stateDir } = await startTestDaemon({ policy: { tools: { "pixel-8/echo": "prompt" } } }); - const app = await claimApp(daemon, port, "Pixel 8"); - await snapshotTools(daemon, app, [{ name: "echo" }, { name: "other" }]); - - const compliantHandle = await createMcpServer({ - stateDir, - spawn: () => { - throw new Error("must not auto-spawn"); - }, - }); - mcpHandles.push(compliantHandle); - const compliantClient = await connectCompliantClient(compliantHandle); - const compliantList = await compliantClient.request({ method: "tools/list", params: {} }, ListToolsResultSchema); - const compliantEcho = compliantList.tools.find((tool) => tool.name === "echo"); - expect(compliantEcho?._meta).toEqual({ "anthropic/requiresUserInteraction": true }); - const compliantOther = compliantList.tools.find((tool) => tool.name === "other"); - expect(compliantOther?._meta).toBeUndefined(); - - const nonCompliantHandle = await createMcpServer({ - stateDir, - spawn: () => { - throw new Error("must not auto-spawn"); - }, - }); - mcpHandles.push(nonCompliantHandle); - const [serverTransport, clientTransport] = InMemoryTransport.createLinkedPair(); - await nonCompliantHandle.connect(serverTransport); - const nonCompliantClient = new Client({ name: "some-other-client", version: "9.9.9" }); - await nonCompliantClient.connect(clientTransport); - const nonCompliantList = await nonCompliantClient.request( - { method: "tools/list", params: {} }, - ListToolsResultSchema, - ); - const nonCompliantEcho = nonCompliantList.tools.find((tool) => tool.name === "echo"); - expect(nonCompliantEcho?._meta).toBeUndefined(); - - app.socket.close(); - }); - - test('a "prompt" tool call from a compliant MCP client proceeds and is audited with consent: "client"', async () => { + test('Claude Code without elicitation gets no requiresUserInteraction flag, and its "prompt" call is denied with reason no_consent_channel', async () => { const { daemon, port, stateDir } = await startTestDaemon({ policy: { tools: { "pixel-8/echo": "prompt" } } }); const app = await claimApp(daemon, port, "Pixel 8"); await snapshotTools(daemon, app, [{ name: "echo" }]); + let toolCallFrames = 0; app.socket.on("message", (data) => { const msg = JSON.parse(data.toString("utf8")) as Record; if (msg.type === "tool_call") { - app.socket.send(JSON.stringify({ type: "tool_result", session_id: app.sessionId, id: msg.id, result: "ok" })); + toolCallFrames += 1; } }); @@ -414,48 +378,11 @@ describe("policy: prompt via MCP requiresUserInteraction", () => { }, }); mcpHandles.push(mcpHandle); - const client = await connectCompliantClient(mcpHandle); + // The version that used to qualify for the removed flag-based channel. + const client = await connectClientWithoutElicitation(mcpHandle, { name: "claude-code", version: "2.1.199" }); - // `consent: "client"` requires this connection to have actually listed the tool with - // `_meta["anthropic/requiresUserInteraction"]` set first — a call is never gated purely on the - // tool's live policy plus a qualifying `clientInfo` (see `server.ts`'s - // `emittedRequiresUserInteraction`). const listed = await client.request({ method: "tools/list", params: {} }, ListToolsResultSchema); - expect(listed.tools.find((tool) => tool.name === "echo")?._meta).toEqual({ - "anthropic/requiresUserInteraction": true, - }); - - const result = await client.request( - { method: "tools/call", params: { name: "echo", arguments: {} } }, - CallToolResultSchema, - ); - expect(result.isError).not.toBe(true); - - app.socket.close(); - await shutdownNow(daemon); - - const records = await readAuditRecords(stateDir); - const record = records.find((r) => r.tool === "echo" && r.caller === "mcp"); - expect(record?.outcome).toBe("ok"); - expect(record?.consent).toBe("client"); - }); - - test('a "prompt" tool call from a non-compliant MCP client is denied with reason no_consent_channel', async () => { - const { daemon, port, stateDir } = await startTestDaemon({ policy: { tools: { "pixel-8/echo": "prompt" } } }); - const app = await claimApp(daemon, port, "Pixel 8"); - await snapshotTools(daemon, app, [{ name: "echo" }]); - - const mcpHandle = await createMcpServer({ - stateDir, - spawn: () => { - throw new Error("must not auto-spawn"); - }, - }); - mcpHandles.push(mcpHandle); - const [serverTransport, clientTransport] = InMemoryTransport.createLinkedPair(); - await mcpHandle.connect(serverTransport); - const client = new Client({ name: "some-other-client", version: "9.9.9" }); - await client.connect(clientTransport); + expect(listed.tools.find((tool) => tool.name === "echo")?._meta).toBeUndefined(); const result = await client.request( { method: "tools/call", params: { name: "echo", arguments: {} } }, @@ -464,36 +391,62 @@ describe("policy: prompt via MCP requiresUserInteraction", () => { expect(result.isError).toBe(true); const text = (result.content[0] as { text: string }).text; expect(text).toContain("policy_denied"); + expect(text).toContain("no_consent_channel"); + expect(text).toContain("elicitation"); + expect(toolCallFrames).toBe(0); + app.socket.close(); await shutdownNow(daemon); const records = await readAuditRecords(stateDir); const record = records.find((r) => r.tool === "echo" && r.caller === "mcp"); expect(record?.outcome).toBe("denied"); + expect(record?.deniedReason).toBe("no_consent_channel"); expect(record?.consent).toBeUndefined(); - - app.socket.close(); }); - test('a "prompt" tool call from the CLI (no consent channel) is denied with reason no_consent_channel', async () => { + test('a legacy consent: "client" from an older MCP server is ignored: the call is denied and audited as no_consent_channel', async () => { const { daemon, port, stateDir } = await startTestDaemon({ policy: { tools: { "pixel-8/echo": "prompt" } } }); const app = await claimApp(daemon, port, "Pixel 8"); await snapshotTools(daemon, app, [{ name: "echo" }]); await expect( - rpcCall(daemon.paths.socketPath, "tools.call", { selector: app.alias, name: "echo", args: {} }), + rpcCall(daemon.paths.socketPath, "tools.call", { + selector: app.alias, + name: "echo", + args: {}, + caller: "mcp", + consent: "client", + }), ).rejects.toMatchObject({ data: { type: "policy_denied", details: { reason: "no_consent_channel" } } }); + app.socket.close(); await shutdownNow(daemon); const records = await readAuditRecords(stateDir); - const record = records.find((r) => r.tool === "echo" && r.caller === "cli"); + const record = records.find((r) => r.tool === "echo" && r.caller === "mcp"); expect(record?.outcome).toBe("denied"); expect(record?.deniedReason).toBe("no_consent_channel"); expect(record?.consent).toBeUndefined(); + }); + + test("an unknown consent value is still rejected as an invalid request", async () => { + const { daemon, port } = await startTestDaemon({ policy: { tools: { "pixel-8/echo": "prompt" } } }); + const app = await claimApp(daemon, port, "Pixel 8"); + await snapshotTools(daemon, app, [{ name: "echo" }]); + + await expect( + rpcCall(daemon.paths.socketPath, "tools.call", { + selector: app.alias, + name: "echo", + args: {}, + caller: "mcp", + consent: "yes", + }), + ).rejects.toMatchObject({ data: { type: "invalid_request" } }); app.socket.close(); }); - test('a client one patch version below the minimum ("2.1.198") is treated as non-compliant', async () => { + test('a "prompt" tool call from an MCP client without elicitation is denied with reason no_consent_channel', async () => { const { daemon, port, stateDir } = await startTestDaemon({ policy: { tools: { "pixel-8/echo": "prompt" } } }); const app = await claimApp(daemon, port, "Pixel 8"); await snapshotTools(daemon, app, [{ name: "echo" }]); @@ -505,56 +458,45 @@ describe("policy: prompt via MCP requiresUserInteraction", () => { }, }); mcpHandles.push(mcpHandle); - const [serverTransport, clientTransport] = InMemoryTransport.createLinkedPair(); - await mcpHandle.connect(serverTransport); - const client = new Client({ name: "claude-code", version: "2.1.198" }); - await client.connect(clientTransport); - - const listed = await client.request({ method: "tools/list", params: {} }, ListToolsResultSchema); - expect(listed.tools.find((tool) => tool.name === "echo")?._meta).toBeUndefined(); + const client = await connectClientWithoutElicitation(mcpHandle); const result = await client.request( { method: "tools/call", params: { name: "echo", arguments: {} } }, CallToolResultSchema, ); expect(result.isError).toBe(true); + const text = (result.content[0] as { text: string }).text; + expect(text).toContain("policy_denied"); + + await shutdownNow(daemon); + const records = await readAuditRecords(stateDir); + const record = records.find((r) => r.tool === "echo" && r.caller === "mcp"); + expect(record?.outcome).toBe("denied"); + expect(record?.consent).toBeUndefined(); app.socket.close(); }); - test("a compliant client that never called tools/list on this connection is still denied (no fabricated consent)", async () => { + test('a "prompt" tool call from the CLI (no consent channel) is denied with reason no_consent_channel', async () => { const { daemon, port, stateDir } = await startTestDaemon({ policy: { tools: { "pixel-8/echo": "prompt" } } }); const app = await claimApp(daemon, port, "Pixel 8"); await snapshotTools(daemon, app, [{ name: "echo" }]); - const mcpHandle = await createMcpServer({ - stateDir, - spawn: () => { - throw new Error("must not auto-spawn"); - }, - }); - mcpHandles.push(mcpHandle); - const client = await connectCompliantClient(mcpHandle); - - // Deliberately no `tools/list` call before `tools/call` — a real compliant client would never - // have been shown the `_meta` flag for "echo" on this connection, so no consent can exist yet. - const result = await client.request( - { method: "tools/call", params: { name: "echo", arguments: {} } }, - CallToolResultSchema, - ); - expect(result.isError).toBe(true); - const text = (result.content[0] as { text: string }).text; - expect(text).toContain("no_consent_channel"); + await expect( + rpcCall(daemon.paths.socketPath, "tools.call", { selector: app.alias, name: "echo", args: {} }), + ).rejects.toMatchObject({ data: { type: "policy_denied", details: { reason: "no_consent_channel" } } }); - app.socket.close(); await shutdownNow(daemon); const records = await readAuditRecords(stateDir); - const record = records.find((r) => r.tool === "echo" && r.caller === "mcp"); + const record = records.find((r) => r.tool === "echo" && r.caller === "cli"); expect(record?.outcome).toBe("denied"); + expect(record?.deniedReason).toBe("no_consent_channel"); expect(record?.consent).toBeUndefined(); + + app.socket.close(); }); - test('policy "deny" is still denied for a compliant client, and no _meta is emitted for a "deny" tool', async () => { + test('policy "deny" is still denied for an MCP client, and no _meta is emitted for a "deny" tool', async () => { const { daemon, port, stateDir } = await startTestDaemon({ policy: { tools: { "pixel-8/echo": "deny" } } }); const app = await claimApp(daemon, port, "Pixel 8"); await snapshotTools(daemon, app, [{ name: "echo" }]); @@ -566,7 +508,7 @@ describe("policy: prompt via MCP requiresUserInteraction", () => { }, }); mcpHandles.push(mcpHandle); - const client = await connectCompliantClient(mcpHandle); + const client = await connectClientWithoutElicitation(mcpHandle); const listed = await client.request({ method: "tools/list", params: {} }, ListToolsResultSchema); expect(listed.tools.find((tool) => tool.name === "echo")?._meta).toBeUndefined(); @@ -587,7 +529,7 @@ describe("policy: prompt via MCP requiresUserInteraction", () => { expect(record?.deniedReason).toBe("policy"); }); - test('policy "allow" for a compliant client never fabricates consent in the audit record', async () => { + test('policy "allow" for an MCP client never fabricates consent in the audit record', async () => { const { daemon, port, stateDir } = await startTestDaemon(); const app = await claimApp(daemon, port, "Pixel 8"); await snapshotTools(daemon, app, [{ name: "echo" }]); @@ -606,7 +548,7 @@ describe("policy: prompt via MCP requiresUserInteraction", () => { }, }); mcpHandles.push(mcpHandle); - const client = await connectCompliantClient(mcpHandle); + const client = await connectClientWithoutElicitation(mcpHandle); const listed = await client.request({ method: "tools/list", params: {} }, ListToolsResultSchema); expect(listed.tools.find((tool) => tool.name === "echo")?._meta).toBeUndefined(); @@ -624,55 +566,11 @@ describe("policy: prompt via MCP requiresUserInteraction", () => { expect(record?.outcome).toBe("ok"); expect(record?.consent).toBeUndefined(); }); - - test('a "prompt" call from a compliant, gated client that errors still records consent: "client"', async () => { - const { daemon, port, stateDir } = await startTestDaemon({ policy: { tools: { "pixel-8/boom": "prompt" } } }); - const app = await claimApp(daemon, port, "Pixel 8"); - await snapshotTools(daemon, app, [{ name: "boom" }]); - - app.socket.on("message", (data) => { - const msg = JSON.parse(data.toString("utf8")) as Record; - if (msg.type === "tool_call") { - app.socket.send( - JSON.stringify({ - type: "tool_error", - session_id: app.sessionId, - id: msg.id, - error: { type: "tool_execution_error", message: "boom" }, - }), - ); - } - }); - - const mcpHandle = await createMcpServer({ - stateDir, - spawn: () => { - throw new Error("must not auto-spawn"); - }, - }); - mcpHandles.push(mcpHandle); - const client = await connectCompliantClient(mcpHandle); - await client.request({ method: "tools/list", params: {} }, ListToolsResultSchema); - - const result = await client.request( - { method: "tools/call", params: { name: "boom", arguments: {} } }, - CallToolResultSchema, - ); - expect(result.isError).toBe(true); - - app.socket.close(); - await shutdownNow(daemon); - const records = await readAuditRecords(stateDir); - const record = records.find((r) => r.tool === "boom" && r.caller === "mcp"); - expect(record?.outcome).toBe("error"); - expect(record?.consent).toBe("client"); - }); }); describe("policy: prompt via MCP elicitation (issue #10)", () => { /** A client that declares the `elicitation` capability and answers every `elicitation/create` - * request with `onElicit` — the preferred consent channel (ARCHITECTURE.md §12), checked ahead of - * the `_meta["anthropic/requiresUserInteraction"]` flag whenever it's present. */ + * request with `onElicit` — the only consent channel (ARCHITECTURE.md §12). */ const connectElicitationClient = async ( mcpHandle: McpServerHandle, onElicit: (message: string) => { action: "accept" | "decline" | "cancel" }, @@ -685,7 +583,7 @@ describe("policy: prompt via MCP elicitation (issue #10)", () => { return client; }; - test('tools/list never emits _meta["anthropic/requiresUserInteraction"] for a client that declares elicitation, even for a "prompt" tool (channel preference)', async () => { + test('tools/list never emits _meta for a "prompt" tool — consent is asked at call time, not flagged at listing time', async () => { const { daemon, port, stateDir } = await startTestDaemon({ policy: { tools: { "pixel-8/echo": "prompt" } } }); const app = await claimApp(daemon, port, "Pixel 8"); await snapshotTools(daemon, app, [{ name: "echo" }]); @@ -741,6 +639,43 @@ describe("policy: prompt via MCP elicitation (issue #10)", () => { expect(record?.consent).toBe("elicitation"); }); + test('a "prompt" call accepted via elicitation that then errors still records consent: "elicitation"', async () => { + const { daemon, port, stateDir } = await startTestDaemon({ policy: { tools: { "pixel-8/boom": "prompt" } } }); + const app = await claimApp(daemon, port, "Pixel 8"); + await snapshotTools(daemon, app, [{ name: "boom" }]); + + app.socket.on("message", (data) => { + const msg = JSON.parse(data.toString("utf8")) as Record; + if (msg.type === "tool_call") { + app.socket.send( + JSON.stringify({ + type: "tool_error", + session_id: app.sessionId, + id: msg.id, + error: { type: "tool_execution_error", message: "boom" }, + }), + ); + } + }); + + const mcpHandle = await createMcpServer({ stateDir, spawn: () => { throw new Error("must not auto-spawn"); } }); + mcpHandles.push(mcpHandle); + const client = await connectElicitationClient(mcpHandle, () => ({ action: "accept" })); + + const result = await client.request( + { method: "tools/call", params: { name: "boom", arguments: {} } }, + CallToolResultSchema, + ); + expect(result.isError).toBe(true); + + app.socket.close(); + await shutdownNow(daemon); + const records = await readAuditRecords(stateDir); + const record = records.find((r) => r.tool === "boom" && r.caller === "mcp"); + expect(record?.outcome).toBe("error"); + expect(record?.consent).toBe("elicitation"); + }); + test.each(["decline", "cancel"] as const)( 'a "prompt" tool call %s\'d via elicitation never reaches the app, and the agent gets an isError result naming it', async (action) => { diff --git a/packages/appduct/src/__tests__/tool-mapping.test.ts b/packages/appduct/src/__tests__/tool-mapping.test.ts index d2a8b6b..d0fcf57 100644 --- a/packages/appduct/src/__tests__/tool-mapping.test.ts +++ b/packages/appduct/src/__tests__/tool-mapping.test.ts @@ -86,7 +86,7 @@ const mapperWithNotices = (): { map: McpToolMapper; notices: string[] } => { return { map: createMcpToolMapper((message) => notices.push(message)), notices }; }; -const map = (descriptor: Partial) => mapperWithNotices().map(namespacedTool(descriptor), false); +const map = (descriptor: Partial) => mapperWithNotices().map(namespacedTool(descriptor)); describe("the fixtures match what the pinned MCP SDK accepts", () => { test.each(ACCEPTED)("ToolSchema accepts a %s schema in both slots", (_label, schema) => { @@ -159,9 +159,9 @@ describe("degradation notices", () => { const { map: mapper, notices } = mapperWithNotices(); const tool = namespacedTool({ name: "list-todos", output_schema: ARRAY_SCHEMA }); - mapper(tool, false); - mapper(tool, false); - mapper(tool, false); + mapper(tool); + mapper(tool); + mapper(tool); expect(notices).toHaveLength(1); expect(notices[0]).toContain("list-todos"); @@ -171,15 +171,12 @@ describe("degradation notices", () => { test("names the tool as the agent sees it, and says why MCP rejected the schema", () => { const { map: mapper, notices } = mapperWithNotices(); - mapper( - { - mcpName: "pixel-8__lies", - selector: "pixel-8", - descriptor: { name: "lies", description: "d", output_schema: SCALAR_REQUIRED }, - policy: "allow", - }, - false, - ); + mapper({ + mcpName: "pixel-8__lies", + selector: "pixel-8", + descriptor: { name: "lies", description: "d", output_schema: SCALAR_REQUIRED }, + policy: "allow", + }); expect(notices[0]).toContain("pixel-8__lies"); expect(notices[0]).toContain("required"); @@ -188,8 +185,8 @@ describe("degradation notices", () => { test("two sessions exposing the same broken tool each get a notice", () => { const { map: mapper, notices } = mapperWithNotices(); - mapper(namespacedTool({ name: "list-todos", output_schema: ARRAY_SCHEMA }, "pixel-8"), false); - mapper(namespacedTool({ name: "list-todos", output_schema: ARRAY_SCHEMA }, "iphone-16"), false); + mapper(namespacedTool({ name: "list-todos", output_schema: ARRAY_SCHEMA }, "pixel-8")); + mapper(namespacedTool({ name: "list-todos", output_schema: ARRAY_SCHEMA }, "iphone-16")); expect(notices).toHaveLength(2); }); @@ -197,8 +194,8 @@ describe("degradation notices", () => { test("re-registering the same tool with a differently broken schema warns again", () => { const { map: mapper, notices } = mapperWithNotices(); - mapper(namespacedTool({ name: "list-todos", output_schema: ARRAY_SCHEMA }), false); - mapper(namespacedTool({ name: "list-todos", output_schema: STRING_SCHEMA }), false); + mapper(namespacedTool({ name: "list-todos", output_schema: ARRAY_SCHEMA })); + mapper(namespacedTool({ name: "list-todos", output_schema: STRING_SCHEMA })); expect(notices).toHaveLength(2); }); @@ -207,8 +204,8 @@ describe("degradation notices", () => { const { map: mapper, notices } = mapperWithNotices(); const descriptor = { name: "list-todos", description: "d", output_schema: ARRAY_SCHEMA }; - mapper({ mcpName: "list-todos", selector: "pixel-8", descriptor, policy: "allow" }, false); - mapper({ mcpName: "pixel-8__list-todos", selector: "pixel-8", descriptor, policy: "allow" }, false); + mapper({ mcpName: "list-todos", selector: "pixel-8", descriptor, policy: "allow" }); + mapper({ mcpName: "pixel-8__list-todos", selector: "pixel-8", descriptor, policy: "allow" }); expect(notices).toHaveLength(1); }); @@ -216,7 +213,7 @@ describe("degradation notices", () => { test("warns separately for the input and the output side of one tool", () => { const { map: mapper, notices } = mapperWithNotices(); - mapper(namespacedTool({ name: "a", input_schema: STRING_SCHEMA, output_schema: ARRAY_SCHEMA }), false); + mapper(namespacedTool({ name: "a", input_schema: STRING_SCHEMA, output_schema: ARRAY_SCHEMA })); expect(notices).toHaveLength(2); expect(notices.filter((notice) => notice.includes("input schema"))).toHaveLength(1); @@ -229,17 +226,17 @@ describe("degradation notices", () => { // Each iteration is a *distinct* broken schema for the same tool, which is what an app // building a schema from fetched rows would produce. Far past the 256-key cap. for (let index = 0; index < 400; index += 1) { - mapper(namespacedTool({ name: "from-live-data", output_schema: { type: "string", const: `v${index}` } }), false); + mapper(namespacedTool({ name: "from-live-data", output_schema: { type: "string", const: `v${index}` } })); } expect(notices).toHaveLength(400); // The oldest keys have been evicted, so the very first schema warns again... - mapper(namespacedTool({ name: "from-live-data", output_schema: { type: "string", const: "v0" } }), false); + mapper(namespacedTool({ name: "from-live-data", output_schema: { type: "string", const: "v0" } })); expect(notices).toHaveLength(401); // ...while a recent one is still remembered and stays quiet. - mapper(namespacedTool({ name: "from-live-data", output_schema: { type: "string", const: "v399" } }), false); + mapper(namespacedTool({ name: "from-live-data", output_schema: { type: "string", const: "v399" } })); expect(notices).toHaveLength(401); }); @@ -247,7 +244,7 @@ describe("degradation notices", () => { const stderr = vi.spyOn(console, "error").mockImplementation(() => {}); try { - createMcpToolMapper()(namespacedTool({ name: "list-todos", output_schema: ARRAY_SCHEMA }), false); + createMcpToolMapper()(namespacedTool({ name: "list-todos", output_schema: ARRAY_SCHEMA })); expect(stderr).toHaveBeenCalledTimes(1); expect(String(stderr.mock.calls[0]![0])).toMatch(/^appduct mcp: /); @@ -259,67 +256,31 @@ describe("degradation notices", () => { test("stays silent for accepted and absent schemas", () => { const { map: mapper, notices } = mapperWithNotices(); - mapper(namespacedTool({ name: "quiet", input_schema: OBJECT_SCHEMA, output_schema: RECORD_SCHEMA }), false); - mapper(namespacedTool({ name: "also-quiet" }), false); + mapper(namespacedTool({ name: "quiet", input_schema: OBJECT_SCHEMA, output_schema: RECORD_SCHEMA })); + mapper(namespacedTool({ name: "also-quiet" })); expect(notices).toEqual([]); }); }); describe("unrelated mapping is unchanged", () => { - test("annotations and the requiresUserInteraction meta still map alongside a dropped output schema", () => { + test('annotations still map alongside a dropped output schema, and a "prompt" tool carries no _meta', () => { const { map: mapper } = mapperWithNotices(); - const mapped = mapper( - { - mcpName: "pixel-8__list-todos", - selector: "pixel-8", - descriptor: { - name: "list-todos", - description: "Lists todos.", - output_schema: ARRAY_SCHEMA, - annotations: { readOnlyHint: true }, - }, - policy: "prompt", + const mapped = mapper({ + mcpName: "pixel-8__list-todos", + selector: "pixel-8", + descriptor: { + name: "list-todos", + description: "Lists todos.", + output_schema: ARRAY_SCHEMA, + annotations: { readOnlyHint: true }, }, - true, - ); + policy: "prompt", + }); expect(mapped.name).toBe("pixel-8__list-todos"); expect(mapped.annotations).toEqual({ readOnlyHint: true }); - expect(mapped._meta).toEqual({ "anthropic/requiresUserInteraction": true }); + expect(mapped).not.toHaveProperty("_meta"); expect(mapped.outputSchema).toBeUndefined(); }); }); - -describe("the requiresUserInteraction flag", () => { - /** - * `mcp/server.ts` folds the elicitation-channel preference into the mapper's boolean before - * calling it (never emit the flag once elicitation is preferred, so a "prompt" tool cannot arm - * both consent channels for one call — ARCHITECTURE.md §12 / issues #10 & #14). This exercises - * the mapper's side of that contract, without a daemon or an MCP client. - */ - const promptTool = (policy: NamespacedTool["policy"]): NamespacedTool => ({ - mcpName: "deleteAll", - selector: "pixel-8", - descriptor: { name: "deleteAll", description: "Deletes everything." }, - policy, - }); - - test('a "prompt" tool gets the flag when the caller says to emit it', () => { - expect(mapperWithNotices().map(promptTool("prompt"), true)._meta).toEqual({ - "anthropic/requiresUserInteraction": true, - }); - }); - - test('a "prompt" tool gets no flag when the caller says not to — e.g. elicitation was preferred for this connection (issue #10)', () => { - expect(mapperWithNotices().map(promptTool("prompt"), false)._meta).toBeUndefined(); - }); - - test('an "allow" tool never gets the flag, even when the caller would otherwise emit it', () => { - expect(mapperWithNotices().map(promptTool("allow"), true)._meta).toBeUndefined(); - }); - - test('a "deny" tool never gets the flag either', () => { - expect(mapperWithNotices().map(promptTool("deny"), true)._meta).toBeUndefined(); - }); -}); diff --git a/packages/appduct/src/daemon/audit.ts b/packages/appduct/src/daemon/audit.ts index c35be79..7456f28 100644 --- a/packages/appduct/src/daemon/audit.ts +++ b/packages/appduct/src/daemon/audit.ts @@ -52,17 +52,13 @@ export type AuditRecord = { /** * Set only when a `"prompt"`-policy call actually proceeded because the MCP server confirmed a * consent gate (ARCHITECTURE.md §12). Deliberately distinct from a plain `"ok"`: the daemon - * never observed either channel's client-side behavior itself, only that the call arrived - * carrying this marker. The value distinguishes which gate fired: - * - `"client"` (issue #14): the flag-based gate — the weaker of the two, evidence only that - * `_meta["anthropic/requiresUserInteraction"]` was emitted and the client is one known to - * enforce it, not that a human actually answered a prompt. - * - `"elicitation"` (issue #10): the client replied `action: "accept"` to a live - * `elicitation/create` request sent for this call — an observed decision, not merely an armed - * flag, though still not independently verifiable by the daemon (see the field-level trust - * caveat on `ToolsCallParams.consent` in `@appduct/shared`). + * never observed the client-side prompt itself, only that the call arrived carrying this marker. + * The only value is `"elicitation"` (issue #10): the client replied `action: "accept"` to a live + * `elicitation/create` request sent for this call — not independently verifiable by the daemon + * (see the field-level trust caveat on `ToolsCallParams.consent` in `@appduct/shared`). Audit + * files written before the flag-based channel was removed may still contain `"client"`. */ - consent?: "client" | "elicitation"; + consent?: "elicitation"; }; /** diff --git a/packages/appduct/src/daemon/config.ts b/packages/appduct/src/daemon/config.ts index 9ed75b7..4ac820e 100644 --- a/packages/appduct/src/daemon/config.ts +++ b/packages/appduct/src/daemon/config.ts @@ -14,13 +14,10 @@ export type PolicyDecision = "allow" | "deny" | "prompt"; * `config.json`'s `policy` shape (ARCHITECTURE.md §12): `default` applies to tools whose * descriptor has no `annotations.destructiveHint`; `destructive` applies when it is `true`; * `tools["/"]` overrides both for one specific tool on one specific session alias. - * `"prompt"` means "a human gate is required; if one cannot be guaranteed, deny" — today there are - * two such gates, both MCP-only (ARCHITECTURE.md §9/§12): an MCP client that declares the - * `elicitation` capability at `initialize` (issue #10, preferred whenever available) and an MCP - * client that honors `_meta["anthropic/requiresUserInteraction"]` (issue #14, Claude Code ≥ - * v2.1.199, used only as a fallback for a client that doesn't declare elicitation). Every other - * caller (CLI, an MCP client on neither channel) gets `policy_denied` with reason - * `no_consent_channel`. + * `"prompt"` means "a human gate is required; if one cannot be guaranteed, deny" — today the only + * such gate is an MCP client that declares the `elicitation` capability at `initialize` + * (ARCHITECTURE.md §9/§12, issue #10). Every other caller (CLI, an MCP client without + * elicitation) gets `policy_denied` with reason `no_consent_channel`. */ export type AppductPolicyConfig = { default: PolicyDecision; diff --git a/packages/appduct/src/daemon/daemon.ts b/packages/appduct/src/daemon/daemon.ts index 6b2a3b5..c2209a6 100644 --- a/packages/appduct/src/daemon/daemon.ts +++ b/packages/appduct/src/daemon/daemon.ts @@ -235,8 +235,12 @@ const asToolsCallParams = (params: unknown): ToolsCallParams => { const consent = record.consent; - if (consent !== undefined && consent !== "client" && consent !== "elicitation") { - throw new RpcApplicationError("invalid_request", '"consent" must be "client" or "elicitation".'); + // `"client"` was the removed flag-based channel's marker. An MCP server older than this daemon + // can still send it (a newer daemon serves older clients, ARCHITECTURE.md §4), so it is accepted + // and ignored rather than rejected: the call then lands on the audited `no_consent_channel` + // denial like any other ungated `"prompt"` call, instead of an unaudited `invalid_request`. + if (consent !== undefined && consent !== "elicitation" && consent !== "client") { + throw new RpcApplicationError("invalid_request", '"consent" must be "elicitation".'); } return { @@ -245,7 +249,7 @@ const asToolsCallParams = (params: unknown): ToolsCallParams => { args: args as Record, timeoutMs: timeoutMs as number | undefined, caller: caller as "cli" | "mcp" | "client" | undefined, - consent: consent as "client" | "elicitation" | undefined, + consent: consent === "elicitation" ? "elicitation" : undefined, }; }; @@ -585,8 +589,7 @@ export const startDaemon = async (options: DaemonOptions): Promise ({ ...descriptor, policy: evaluatePolicy(descriptor, { alias: resolved.alias }, config.policy), @@ -626,7 +629,7 @@ export const startDaemon = async (options: DaemonOptions): Promise { auditLogger.record({ @@ -659,24 +662,20 @@ export const startDaemon = async (options: DaemonOptions): Promise { return `a ${typeof value}`; }; -/** The minimum Claude Code version documented (ARCHITECTURE.md §12 / issue #14) to enforce - * `_meta["anthropic/requiresUserInteraction"]` on every call, in every permission mode, with no - * "don't ask again" option. Older Claude Code and every other client ignore the flag silently. */ -const REQUIRES_USER_INTERACTION_MIN_VERSION = [2, 1, 199] as const; - -/** Strict `\d+` per dot-separated part — rejects a pre-release/build suffix (`"2.1.199-beta.1"`, - * `"2.1.199rc"`) rather than letting `Number.parseInt`'s leading-digits-only parsing treat it as - * `2.1.199` and wrongly report a pre-release as version-compliant. */ -const parseVersionParts = (version: string): number[] | undefined => { - const rawParts = version.split("."); - - if (!rawParts.every((part) => /^\d+$/u.test(part))) { - return undefined; - } - - return rawParts.map((part) => Number.parseInt(part, 10)); -}; - -const isVersionAtLeast = (version: string, min: readonly number[]): boolean => { - const parts = parseVersionParts(version); - - if (!parts) { - return false; - } - - for (let i = 0; i < min.length; i++) { - const part = parts[i] ?? 0; - - if (part > min[i]!) { - return true; - } - - if (part < min[i]!) { - return false; - } - } - - return true; -}; - -/** - * Whether the connected MCP client is known to enforce `_meta["anthropic/requiresUserInteraction"]` - * (ARCHITECTURE.md §12 / issue #14). `clientInfo` is self-reported at `initialize` — a hostile - * client could claim to be Claude Code, but that is outside this feature's threat model - * (unattended automation, not a malicious client); it is documented as a known limitation. - */ -const clientHonorsRequiresUserInteraction = (server: Server): boolean => { - const clientInfo = server.getClientVersion(); - return clientInfo?.name === "claude-code" && isVersionAtLeast(clientInfo.version, REQUIRES_USER_INTERACTION_MIN_VERSION); -}; - /** * Whether the connected MCP client declared the `elicitation` capability at `initialize` - * (ARCHITECTURE.md §12 / issue #10) — the preferred `"prompt"`-policy consent channel, checked - * fresh on every call rather than cached (same principle as `clientHonorsRequiresUserInteraction` - * above: nothing about consent is trusted from a stale snapshot). A bare `elicitation: {}` from the - * client normalizes to `{ form: {} }` in the SDK's parsed capabilities (backwards-compat default), + * (ARCHITECTURE.md §12 / issue #10) — the only `"prompt"`-policy consent channel, checked fresh on + * every call rather than cached: nothing about consent is trusted from a stale snapshot. A bare + * `elicitation: {}` from the client normalizes to `{ form: {} }` in the SDK's parsed capabilities (backwards-compat default), * which is what `server.elicitInput`'s own form-mode request actually requires — this check only * needs to know the key is present at all, and lets `elicitInput` itself fail (caught below, and * treated as "no channel", never as approval) if the specific mode turns out unsupported. @@ -338,8 +285,8 @@ const proxiedToolResultContent = (tool: NamespacedTool, result: unknown): CallTo * How this server opens a daemon connection. Both the startup stream and each short-lived * progress stream go through it, so a caller can put something other than a real daemon on the * other end. The only production implementation is {@link openDaemonStream}; the seam exists so - * the behaviour that is purely this module's own — name mapping, schema degradation, consent - * flags, namespacing, `list_changed` — can be tested without a TLS listener, a pidfile and a + * the behaviour that is purely this module's own — name mapping, schema degradation, consent, + * namespacing, `list_changed` — can be tested without a TLS listener, a pidfile and a * subprocess, none of which those behaviours depend on. */ export type OpenDaemonStreamFn = (options: { @@ -416,25 +363,14 @@ export const createMcpServer = async (options: CreateMcpServerOptions): Promise< { capabilities: { tools: { listChanged: true }, resources: {} } }, ); - // The `mcpName`s this connection's *most recent* `tools/list` response actually emitted - // `_meta["anthropic/requiresUserInteraction"]` for — repopulated on every `tools/list` request, - // never on the internal `list_changed` refresh below (which doesn't answer a client request, so - // nothing was shown to a human). `resolveToolCallConsent` requires membership here in addition to - // recomputing the tool's live policy and the client check, so `consent: "client"` reflects an - // MCP `Tool` the client actually listed with the flag set on *this* connection, not merely a - // client that happens to qualify version-wise (ARCHITECTURE.md §12 / issue #14). Stays empty for - // any connection where elicitation is preferred (issue #10) — see the `tools/list` handler below. - const emittedRequiresUserInteraction = new Set(); - /** * Resolves the `consent` param for one `"prompt"`-policy `tools.call` (ARCHITECTURE.md §12), - * shared by both call paths below so the two channels' logic exists exactly once. Recomputes the - * tool's live policy's implications at call time — never trusts anything cached from a prior - * `tools/list` snapshot except membership in `emittedRequiresUserInteraction` above, which by - * construction can only be true for *this* connection's most recent listing. + * shared by both call paths below so the logic exists exactly once. Recomputes the tool's live + * policy's implications at call time — never trusts anything cached from a prior `tools/list` + * snapshot. * - * Channel preference (never both armed for one call): elicitation (issue #10) whenever the - * client declared the capability, else the flag-based gate (issue #14) as a fallback. An + * Elicitation (issue #10) is the only channel. A client that didn't declare the capability gets + * no consent marker, so the daemon denies the call with reason `no_consent_channel`. An * elicitation decline/cancel/timeout throws `ElicitationDeclinedError` — the caller must not * catch it, since it belongs in the outer `tools/call` handler's result path, not lumped in with * "no consent obtained". @@ -442,36 +378,30 @@ export const createMcpServer = async (options: CreateMcpServerOptions): Promise< const resolveToolCallConsent = async ( tool: NamespacedTool, args: Record, - ): Promise<"client" | "elicitation" | undefined> => { - if (tool.policy !== "prompt") { + ): Promise<"elicitation" | undefined> => { + if (tool.policy !== "prompt" || !clientSupportsElicitation(server)) { return undefined; } - if (clientSupportsElicitation(server)) { - const outcome = await requestElicitationConsent( - server, - tool, - args, - options.elicitationTimeoutMs ?? ELICITATION_TIMEOUT_MS, - ); - - if (outcome.type === "accepted") { - return "elicitation"; - } + const outcome = await requestElicitationConsent( + server, + tool, + args, + options.elicitationTimeoutMs ?? ELICITATION_TIMEOUT_MS, + ); - if (outcome.type === "declined") { - throw new ElicitationDeclinedError(outcome.message); - } + if (outcome.type === "accepted") { + return "elicitation"; + } - // "no-channel": never fabricate approval from a failed request — fall through with no - // consent so the daemon's own "prompt" gate denies the call exactly as it would for any - // other ungated caller (ARCHITECTURE.md §12). - return undefined; + if (outcome.type === "declined") { + throw new ElicitationDeclinedError(outcome.message); } - return clientHonorsRequiresUserInteraction(server) && emittedRequiresUserInteraction.has(tool.mcpName) - ? "client" - : undefined; + // "no-channel": never fabricate approval from a failed request — fall through with no + // consent so the daemon's own "prompt" gate denies the call exactly as it would for any + // other ungated caller (ARCHITECTURE.md §12). + return undefined; }; // One mapper per server: it owns the dedup for the "this schema had to be degraded" stderr @@ -606,20 +536,6 @@ export const createMcpServer = async (options: CreateMcpServerOptions): Promise< server.setRequestHandler(ListToolsRequestSchema, async () => { const tools = await fetchEffectiveTools(stream.call); - // Channel preference (ARCHITECTURE.md §12 / issues #10 & #14): elicitation always wins over the - // flag-based gate when the client declared it, so a "prompt" tool never arms two consent UIs - // for one call. When elicitation is preferred, the flag is suppressed entirely — this listing - // never sets `_meta["anthropic/requiresUserInteraction"]`, `emittedRequiresUserInteraction` - // stays empty, and `resolveToolCallConsent` above takes the elicitation branch at call time - // instead. - const emitRequiresUserInteractionFlag = !clientSupportsElicitation(server) && clientHonorsRequiresUserInteraction(server); - - emittedRequiresUserInteraction.clear(); - for (const tool of tools) { - if (tool.policy === "prompt" && emitRequiresUserInteractionFlag) { - emittedRequiresUserInteraction.add(tool.mcpName); - } - } return { tools: [ @@ -627,7 +543,7 @@ export const createMcpServer = async (options: CreateMcpServerOptions): Promise< WAIT_FOR_SESSION_TOOL_DESCRIPTOR, EVENTS_TOOL_DESCRIPTOR, WAIT_FOR_EVENT_TOOL_DESCRIPTOR, - ...tools.map((tool) => mapToMcpTool(tool, emitRequiresUserInteractionFlag)), + ...tools.map(mapToMcpTool), ], }; }); diff --git a/packages/appduct/src/mcp/tool-mapping.ts b/packages/appduct/src/mcp/tool-mapping.ts index 769157b..16e4438 100644 --- a/packages/appduct/src/mcp/tool-mapping.ts +++ b/packages/appduct/src/mcp/tool-mapping.ts @@ -12,15 +12,6 @@ * (the JSON Schema shorthand `properties: { a: true }` is rejected) and `required` must be an * array. Any of those makes a client reject the entire `tools/list` result, so the only safe * predicate is the one the client will actually apply. - * - * A tool whose effective policy is `"prompt"` (ARCHITECTURE.md §12) gets - * `_meta["anthropic/requiresUserInteraction"] = true` — but only when the caller says to - * (`emitRequiresUserInteractionFlag`), which `mcp/server.ts` decides is true only when the - * connected client is known to enforce the flag (issue #14) *and* it hasn't already preferred the - * elicitation channel instead (issue #10). Emitting the flag for a client that ignores it would - * create a false sense of security; emitting it *and* using elicitation for the same tool would - * arm two consent prompts for one call — so this function never decides that itself, only renders - * the decision it's handed. */ import { ToolSchema } from "@modelcontextprotocol/sdk/types.js"; @@ -39,7 +30,6 @@ export type McpToolSchema = { inputSchema: Record; outputSchema?: Record; annotations?: Record; - _meta?: Record; }; type SchemaSlot = "input" | "output"; @@ -96,7 +86,7 @@ const defaultWarn = (message: string): void => { */ const MAX_REMEMBERED_NOTICES = 256; -export type McpToolMapper = (tool: NamespacedTool, emitRequiresUserInteractionFlag: boolean) => McpToolSchema; +export type McpToolMapper = (tool: NamespacedTool) => McpToolSchema; /** * Builds a mapper with its own degradation-notice dedup, so nothing here is module state and no @@ -185,9 +175,8 @@ export const createMcpToolMapper = (warn: (message: string) => void = defaultWar return undefined; }; - return (tool, emitRequiresUserInteractionFlag) => { + return (tool) => { const { descriptor } = tool; - const requiresUserInteraction = tool.policy === "prompt" && emitRequiresUserInteractionFlag; const outputSchema = mapOutputSchema(tool); return { @@ -196,8 +185,6 @@ export const createMcpToolMapper = (warn: (message: string) => void = defaultWar inputSchema: mapInputSchema(tool), ...(outputSchema ? { outputSchema } : {}), ...(descriptor.annotations ? { annotations: { ...descriptor.annotations } } : {}), - // Must be the JSON boolean `true` literal — any other value is ignored by the client. - ...(requiresUserInteraction ? { _meta: { "anthropic/requiresUserInteraction": true } } : {}), }; }; }; diff --git a/packages/appduct/src/mcp/tool-namespace.ts b/packages/appduct/src/mcp/tool-namespace.ts index e7fe9bb..fbebcc1 100644 --- a/packages/appduct/src/mcp/tool-namespace.ts +++ b/packages/appduct/src/mcp/tool-namespace.ts @@ -23,9 +23,8 @@ export type NamespacedTool = { selector: string; descriptor: ToolDescriptor; /** The effective policy decision for this tool right now (ARCHITECTURE.md §12), as resolved by - * the daemon's `tools.list` — carried through so the MCP server can emit - * `_meta["anthropic/requiresUserInteraction"]` and set `consent` on `tools.call` without a - * second round trip (issue #14). */ + * the daemon's `tools.list` — carried through so the MCP server knows whether to ask for + * consent via elicitation before `tools.call`, without a second round trip. */ policy: EffectivePolicyDecision; }; diff --git a/packages/shared/src/domains/rpc.ts b/packages/shared/src/domains/rpc.ts index 51d132f..2597640 100644 --- a/packages/shared/src/domains/rpc.ts +++ b/packages/shared/src/domains/rpc.ts @@ -150,8 +150,8 @@ export type ToolsListParams = SessionSelectorParams & { /** A `tools.list` entry: the tool's descriptor plus the policy decision (ARCHITECTURE.md §12) * that would apply to it right now — resolved daemon-side (it needs `session.alias` and - * `config.policy`) so the MCP server can emit `_meta["anthropic/requiresUserInteraction"]` for - * `"prompt"` tools at `tools/list` time without a second round trip. */ + * `config.policy`) so the MCP server knows which calls need an elicitation prompt without a + * second round trip. */ export type ToolsListEntry = ToolDescriptor & { policy: EffectivePolicyDecision; }; @@ -179,27 +179,18 @@ export type ToolsCallParams = SessionSelectorParams & { /** * Set by this codebase's own MCP server as evidence a `"prompt"`-policy tool's human gate was * satisfied (ARCHITECTURE.md §12) — the daemon trusts this param verbatim once present, and - * everything else (CLI, an MCP client on neither channel) is denied (`policy_denied`, reason - * `no_consent_channel`). Two distinct channels set it, and the value tells the audit log which - * one fired (`daemon/audit.ts`): - * - `"client"` (issue #14): the server emitted `_meta["anthropic/requiresUserInteraction"]` for - * this tool in this connection's most recent `tools/list` response, and the connected client's - * `initialize` `clientInfo` is one known to enforce that flag (`name === "claude-code"`, - * version ≥ 2.1.199). This is evidence the flag was emitted, not that a human answered — the - * daemon never observes the client's own permission UI. - * - `"elicitation"` (issue #10): the server sent an `elicitation/create` request over this - * connection and the client's reply was `action: "accept"`. This is the stronger of the two — - * an observed decision, not merely an armed flag — and works on any client that declares the - * `elicitation` capability at `initialize`, not just Claude Code. The two channels never both - * arm for the same call: the MCP server prefers elicitation whenever the client declares it, - * and skips emitting the `requiresUserInteraction` flag entirely in that case. + * everything else (CLI, an MCP client without elicitation) is denied (`policy_denied`, reason + * `no_consent_channel`). The only value is `"elicitation"` (issue #10): the server sent an + * `elicitation/create` request over this connection and the client's reply was + * `action: "accept"`. It works on any client that declares the `elicitation` capability at + * `initialize`. * - * The daemon cannot itself re-verify either channel's client-side behavior, so this is not a - * defense against another local process (one with access to the same `daemon.sock`) sending this - * param directly — see `docs/SECURITY.md`'s threat model, which already treats socket access as - * full daemon control. `"prompt"` fails closed by design when neither channel applies. + * The daemon cannot itself re-verify the client-side prompt, so this is not a defense against + * another local process (one with access to the same `daemon.sock`) sending this param directly — + * see `docs/SECURITY.md`'s threat model, which already treats socket access as full daemon + * control. `"prompt"` fails closed by design when the client has no elicitation support. */ - consent?: "client" | "elicitation"; + consent?: "elicitation"; }; export type ToolsCallResult = {