From 5836e45679df80b458b85f6facdbdf43b6d9d69c Mon Sep 17 00:00:00 2001 From: kevin9327 Date: Mon, 31 Aug 2026 20:16:43 +0900 Subject: [PATCH] Record what a rule in dry-run would refuse a Bot's tools, not only its browser MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `dry-run` is how a boundary is measured against live traffic before it starts refusing anybody: `evaluateActionPolicy` returns `allowed: false` with `forward: true`, so the policy refuses and the mode lets the action through anyway. The value of the mode is entirely in the row it leaves behind. The browser gateway writes that row. It keys its event type on `decision.allowed`, so a matched action is recorded as `computer.action_refused` and then carried out. The connector path keyed its row on `verdict.forward` instead, so in dry-run it wrote nothing before the call and then wrote `mcp.call_succeeded` — the same decision, on the sibling surface, recorded as its opposite. So an administrator who wrote `deny: mcp.effect == "write"`, switched the mode to "Record it and allow it" exactly as the Boundaries page describes, and came back a week later found `Blocked` empty and every `eventType=mcp.call_rejected` query answering zero. The dry-run report could not compensate: its replay reads browser rows only. The rule looked inert, and enforcing it started refusing Bots with nothing in the trail that could have warned anybody. That is the outcome the mode exists to prevent. The refusal is now written on the policy's answer rather than on what the mode did with it, which is what the gateway has always done. `decision.carriedOut` tells the two rows apart: false is a call this deployment stopped, true is one dry-run recorded and let past. The outcome row is untouched, so a forwarded call still says separately whether the vendor answered, and an enforcing deployment writes exactly the rows it wrote before. Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 11 +++++ server/src/plugins/store.ts | 20 +++++++- server/tests/plugin-store.integration.test.ts | 48 +++++++++++++++++++ 3 files changed, 77 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c851fcd32..16ae5fcbd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,17 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged. ## Unreleased +### A rule tried in dry-run now says what it would have refused a Bot's tools + +`dry-run` exists so a boundary can be measured against live traffic before it starts refusing +anybody. It worked that way for the browser, and not for connectors: a tool call the rule matched was +recorded only as the call that then went out, so `Blocked` on the audit page — and any query behind +it — answered "this rule would have refused none of them" about calls it would have refused. A rule +about `mcp.server`, `mcp.tool` or `mcp.effect` therefore looked inert, and enforcing it started +refusing Bots with nothing in the trail to have warned anybody. A refused tool call is now recorded +whatever the mode does with it, carrying `carriedOut` so a reader can tell a call this deployment +stopped from one dry-run recorded and let past. Enforcing deployments behave exactly as before. + ### A Bot's shell can no longer reach the embedded database without a password In the all-in-one image the cluster was `trust`-auth on loopback, and the Bot's shell runs in the diff --git a/server/src/plugins/store.ts b/server/src/plugins/store.ts index 4164c39a6..1b8ca1bb1 100644 --- a/server/src/plugins/store.ts +++ b/server/src/plugins/store.ts @@ -2875,18 +2875,34 @@ export function createPluginStore(options: PluginStoreOptions) { }; /* - * A refusal is written here, because there is no attempt to wait for. + * A refusal is written on the POLICY's answer, not on whether the call was then let through. * * This deployment declining is the whole event, and it is recorded before the throw so that a * refusal cannot be lost by the caller's error handling. + * + * In `dry-run` the policy still refuses and the mode forwards anyway, which is the whole point + * of the mode: `evaluateActionPolicy` returns `allowed: false` with `forward: true` so a rule + * can be tried against live traffic before it starts refusing anybody. Writing this row on + * `forward` therefore recorded nothing at all on this surface for exactly the traffic an + * operator switched dry-run on to measure — the browser gateway keys its row on + * `decision.allowed` and does record it — so `Blocked` on the audit page, and every + * `eventType=mcp.call_rejected` query behind it, answered "this rule would refuse none of your + * tool calls" about calls it would refuse. The rule then looked inert, and enforcing it + * started refusing Bots with no warning in the trail. + * + * `decision.carriedOut` is what tells the two rows apart: false is a call this deployment + * stopped, true is one dry-run recorded and let past. The outcome row below is unchanged, so a + * forwarded call still says separately whether the vendor answered. */ - if (!verdict.forward) { + if (!verdict.allowed) { await recordAuditEvent(auditStore, { eventType: "mcp.call_rejected", targetType: "mcp_tool", targetId: input.ref, payload: decided, }); + } + if (!verdict.forward) { throw new PluginRefusedError(verdict.reason, verdict.matched); } diff --git a/server/tests/plugin-store.integration.test.ts b/server/tests/plugin-store.integration.test.ts index c82eb3d7f..f05914ffc 100644 --- a/server/tests/plugin-store.integration.test.ts +++ b/server/tests/plugin-store.integration.test.ts @@ -383,6 +383,54 @@ describe("the policy is asked as well as the grant", () => { expect((thrown as PluginRefusedError).rule).toBeNull(); expect((thrown as PluginRefusedError).message).toContain("connected"); }); + + test("a dry-run refusal is recorded, even though the call is let through", async () => { + await store.grant("mcp", ref, holderId, "admin@openbot.local"); + /* + * The mode an operator switches on to size a rule before enforcing it, and the only mode in + * which the policy refuses and the call still goes out. Its whole value is the row: without one + * the report reads "this rule would refuse nothing" about traffic it would refuse. + */ + const rule = `mcp.tool == "${toolName}"`; + policy = { mode: "dry-run", deny: [rule], allow: ["true"] }; + + try { + await store + .callTool({ + ref, + args: {}, + botId: holderId, + actorId: "someone@openbot.local", + }) + // Forwarded past the policy, so what happens next is the vendor's business and not this + // test's: nobody has connected an account, so it fails there. Swallowed deliberately. + .catch(() => undefined); + } finally { + policy = { mode: "enforce", deny: [], allow: ["true"] }; + } + + const rows = await auditRowsFor(ref); + const recorded = rows.filter( + (row) => + row.eventType === "mcp.call_rejected" && + (row.payload as { decision?: { rule?: string } }).decision?.rule === + rule, + ); + expect(recorded.length).toBeGreaterThan(0); + /* + * What tells this row apart from a call this deployment actually stopped. `allowed` is the + * policy's answer and `carriedOut` is what the mode did with it, so a reader counting what a + * rule would have refused finds this one, and a reader counting what was refused does not. + */ + const decision = ( + recorded[0].payload as { + decision?: { allowed?: boolean; mode?: string; carriedOut?: boolean }; + } + ).decision; + expect(decision?.allowed).toBe(false); + expect(decision?.mode).toBe("dry-run"); + expect(decision?.carriedOut).toBe(true); + }); }); describe("the trail says what happened, not what was permitted", () => {