From 8acf37aed6e206565ffe25655c0e9ee04051a5f2 Mon Sep 17 00:00:00 2001 From: Vaibhav Zope Date: Mon, 24 Aug 2026 04:43:48 +0530 Subject: [PATCH 1/2] Record a failed action the same way it was decided --- server/src/computer/gateway.ts | 9 ++++ server/tests/computer-gateway.test.ts | 67 +++++++++++++++++++++++++++ 2 files changed, 76 insertions(+) diff --git a/server/src/computer/gateway.ts b/server/src/computer/gateway.ts index 96895210..bf6a8920 100644 --- a/server/src/computer/gateway.ts +++ b/server/src/computer/gateway.ts @@ -534,6 +534,13 @@ export function createComputerGateway( * Writing the decision before acting is still right, because an allowed action may have partial * effects before failing. The failure row records the outcome separately from the policy * decision. + * + * It carries the same subject the decision row did, and for the same reasons: a shell call + * that failed part-way is the row somebody most needs to name the command from, and a keypress + * that failed is still the difference between a submitted form and a typed letter. Leaving + * them off also chose the wrong element branch below, because "this action never had an + * element" is decided by the command and the file path — so a failed command claimed a + * snapshot lookup that never happened. */ await write(auditStore, { toolName, @@ -541,6 +548,8 @@ export function createComputerGateway( actor, element, ref, + ...(subject.key ? { key: subject.key } : {}), + ...(subject.command ? { command: subject.command } : {}), filePath, pageUrl, decision, diff --git a/server/tests/computer-gateway.test.ts b/server/tests/computer-gateway.test.ts index 494c2326..bdbfd7b4 100644 --- a/server/tests/computer-gateway.test.ts +++ b/server/tests/computer-gateway.test.ts @@ -201,6 +201,11 @@ async function gatewayWith( resetResult?: { cleared: boolean }; locations?: ComputerLocation[]; token?: string; + /** Endpoints the computer answers differently, for the calls that have to fail. */ + routes?: Record< + string, + (init?: RequestInit) => Response | Promise + >; }, ) { const { provider, fetchImpl, calls, addressedAs, requests } = @@ -444,6 +449,68 @@ describe("the computer gateway", () => { expect(rows[0]?.payload.command).toBe("cat secrets.txt"); }); + /** + * The two rows describe one action, so they have to describe the same one. + * + * Asserted as agreement rather than field by field on purpose: the failure row is a second + * hand-maintained argument list, and what goes wrong with one of those is not a particular field + * being wrong, it is a field being added to one and not the other. Comparing the payloads catches + * the next one too. + */ + test("a permitted action that fails is recorded the same way it was decided", async () => { + const failing = () => + Response.json({ error: "device or resource busy" }, { status: 500 }); + + for (const action of [ + { + what: "a command", + route: "/exec", + run: (gateway: Awaited>["gateway"]) => + gateway.runCommand("bot-1", ACTOR, { + command: "rm -rf /workspace/build", + }), + }, + { + what: "a keypress", + route: "/key", + run: (gateway: Awaited>["gateway"]) => + gateway.key("bot-1", ACTOR, { + ref: "e1", + snapshotId: 7, + key: "Enter", + }), + }, + { + what: "a file write", + route: "/files/write", + run: (gateway: Awaited>["gateway"]) => + gateway.writeFile("bot-1", ACTOR, { path: "notes.md", text: "kept" }), + }, + ]) { + const { gateway, rows } = await gatewayWith(PERMISSIVE, { + routes: { [action.route]: failing }, + }); + rows.length = 0; + + await expect(action.run(gateway)).rejects.toThrow(); + + const allowed = rows.find( + (row) => row.eventType === "computer.action_allowed", + ); + const failed = rows.find( + (row) => row.eventType === "computer.action_failed", + ); + expect(allowed, action.what).toBeDefined(); + expect(failed, action.what).toBeDefined(); + + // The outcome is the only thing the failure row adds. Everything describing what was attempted + // is the same action and reads the same on both rows. + const { failure, ...attempted } = failed?.payload ?? {}; + expect(failure, action.what).toBeString(); + expect(attempted, action.what).toEqual(allowed?.payload ?? {}); + } + }); + test("the computer is told WHICH Bot is asking", async () => { // Every per-Bot behaviour on the computer keys off this id: the profile it opens, the logins it // has, the proxy its traffic leaves through, and who holds its wheel. From 7fd486247409b3f2db0a5d15b51f8c3b13217a0f Mon Sep 17 00:00:00 2001 From: Vaibhav Zope Date: Mon, 24 Aug 2026 04:45:26 +0530 Subject: [PATCH 2/2] Record the audit row fix in the changelog --- CHANGELOG.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5e37bf41..cc08bd68 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,21 @@ the admin screen, which mints a token and points at the one it just made. If a d custom server pointing at a credential of another kind, adding it again will now be refused, and the answer is to give the server its own token. +### A failed action is recorded the same way it was decided + +An action the policy allowed and the computer then failed is recorded twice, once for the decision +and once for the outcome, so the trail can tell an action that happened from one that was permitted +and did not. The second row was leaving out the command and the key that the first one carried. + +A shell command that failed part-way therefore said a Bot had run something without saying what, in +the row somebody reading an incident reaches for first. The same omission picked the wrong element +branch, so that row also claimed the command had been looked for in the page snapshot and not found +— a page element a shell call never had. A failed file write kept its path throughout and is +unchanged. + +Both rows now carry the same subject. Nothing about the boundary moves: the policy decided on a +complete context before and after, and no action is permitted that was not permitted before. + ### Upgrading **A deployment that sets `AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS=true` with `NODE_ENV=production` no