From 1f73a914789f6880703887577424d0aa92bcda6d Mon Sep 17 00:00:00 2001 From: beardthelion <56458543+beardthelion@users.noreply.github.com> Date: Fri, 21 Aug 2026 14:20:34 -0500 Subject: [PATCH] Let the policy see the Enter a type action presses A form has three doors and the policy could see two. `computer_type` takes a `submit` flag that presses Enter once the text is in, and the gateway passed no key for it, so an agent refused on clicking "Submit order" and refused again on pressing Enter typed into the field with `submit: true` and the order went through. The audit row said a field had been filled in, because it had no key to say otherwise. The deployment following the product's own advice was the one with the door open. Both shipped copies of the rule, the example in `.env.example` and the "Never submit a form" preset on the Boundaries page, named `computer_key` alone. Three changes, and the last is the one that closes it for anybody who has not written their own rule: - the gateway carries `key: "Enter"` for a type that submits, so a rule about Enter decides on it and the trail records it; - `intentOf` reports that action as `activate` rather than `type`, on the same reasoning the keypress path already used: what Enter does is press whatever has focus; - both shipped rules name `computer_type` alongside `computer_key`. The clause still guards `key` behind a tool name so it short-circuits on actions that have no keypress in them, which is what keeps a browser rule from refusing MCP calls. The preset's behaviour is now pinned by a test that runs the shipped string through the button, the keypress and the submit flag, and confirms a Bot can still fill a form in. Two strings in two files drift otherwise, and the drift is silent. --- .env.example | 5 +- app/src/routes/_authed/admin/boundaries.tsx | 7 +- server/src/computer/gateway.ts | 14 ++- server/src/computer/policy.ts | 10 +- server/tests/computer-gateway.test.ts | 128 ++++++++++++++++++++ 5 files changed, 154 insertions(+), 10 deletions(-) diff --git a/.env.example b/.env.example index 016e81df..05f4a2d3 100644 --- a/.env.example +++ b/.env.example @@ -162,12 +162,13 @@ AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS=true # # Name every route to the same effect. A form submits from a keypress in any of its fields, so a rule # that only blocks a Submit button does not block Enter from another field. The example below refuses -# Enter outright for that reason. +# Enter outright for that reason, from both tools that can press it: a key action, and a type action +# carrying `submit`. # Functions: contains(haystack, needle) and matches(value, pattern), both case-insensitive. # `enforce` blocks; `dry-run` decides and records but lets everything through, so a new rule can be # tried against real traffic before it starts refusing anybody's work. # -# AGENT_COMPUTER_POLICY={"mode":"enforce","deny":["(intent == \"activate\" && contains(element.name, \"submit\")) || (tool.name == \"computer_key\" && key == \"Enter\")"],"allow":["true"]} +# AGENT_COMPUTER_POLICY={"mode":"enforce","deny":["(intent == \"activate\" && contains(element.name, \"submit\")) || ((tool.name == \"computer_key\" || tool.name == \"computer_type\") && key == \"Enter\")"],"allow":["true"]} # How long one action waits for its element, in ms. Read by agent-computer, not the server. # ACTION_TIMEOUT_MS=10000 diff --git a/app/src/routes/_authed/admin/boundaries.tsx b/app/src/routes/_authed/admin/boundaries.tsx index 66644bd0..cb6faca6 100644 --- a/app/src/routes/_authed/admin/boundaries.tsx +++ b/app/src/routes/_authed/admin/boundaries.tsx @@ -23,8 +23,11 @@ import { Input } from "@/components/ui/input"; const PRESETS: { label: string; rule: string; cost?: string }[] = [ { label: "Never submit a form", - // `key` exists only on keypress actions; guard it by tool name to keep other actions evaluable. - rule: '(intent == "activate" && contains(element.name, "submit")) || (tool.name == "computer_key" && key == "Enter")', + // `key` is guarded by tool name so the clause short-circuits before it on actions that have no + // keypress in them. Both tools that can press Enter are named: `computer_type` takes a `submit` + // flag that presses it once the text is in, and a rule naming only `computer_key` left that door + // open. + rule: '(intent == "activate" && contains(element.name, "submit")) || ((tool.name == "computer_key" || tool.name == "computer_type") && key == "Enter")', cost: "Also stops the Bot pressing Enter for anything else, because a form submits from Enter in any of its fields.", }, { diff --git a/server/src/computer/gateway.ts b/server/src/computer/gateway.ts index be73249d..63b60984 100644 --- a/server/src/computer/gateway.ts +++ b/server/src/computer/gateway.ts @@ -674,6 +674,15 @@ export function createComputerGateway( { ref: input.ref, snapshotId: input.snapshotId, + /* + * `submit` presses Enter once the text is in, so this call is a keypress as well as a + * typing one and the policy has to see both halves. Without the key here, an agent refused + * on clicking the button and refused on pressing Enter types into the field with + * `submit: true` instead, and the form goes through against a rule written to stop exactly + * that. It is what the trail is missing too: a row with no key says a field was filled in, + * not that the form was sent. + */ + ...(input.submit ? { key: "Enter" } : {}), ...(signal ? { signal } : {}), }, () => post(botId, "/type", input, signal), @@ -832,9 +841,10 @@ function intentOf( case "computer_click": return "activate"; case "computer_key": - return key && ACTIVATING_KEYS.has(key) ? "activate" : "type"; + // A type carrying `submit` ends in Enter, and the same reasoning applies to it: what the + // keypress does is press whatever the form activates, whichever tool asked for it. case "computer_type": - return "type"; + return key && ACTIVATING_KEYS.has(key) ? "activate" : "type"; case "computer_navigate": return "navigate"; case "computer_read": diff --git a/server/src/computer/policy.ts b/server/src/computer/policy.ts index ec9d39e2..93b06137 100644 --- a/server/src/computer/policy.ts +++ b/server/src/computer/policy.ts @@ -60,9 +60,10 @@ export type PolicyContext = { * click is refused and audited, the keypress is allowed, because nothing in the context could tell * one keypress from another. * - * A form has two doors and the policy could only see one. Now a rule can say - * `tool.name == "computer_key" && key == "Enter"`, and an operator blocking a submit button knows - * to block both routes, which the deny example in `.env.example` now does. + * A form has three doors, and this is set for two of them. `computer_type` takes a `submit` flag + * that presses Enter once the text is in, so it carries the key as well; a rule naming only + * `computer_key` was refused at the button and at the keypress and let through the third way in. + * The deny example in `.env.example` and the Boundaries preset both name both tools. */ key?: string; /** @@ -78,7 +79,8 @@ export type PolicyContext = { * `read`, looking at the page or listing what is on it. * `write_file` / `read_file` / `list_files`, the workspace. * - * It still cannot see whether a keypress will submit a form. A browser submits + * It still cannot see whether a keypress will submit a form, only that one is coming: a type + * carrying `submit` reports `activate` because it ends in Enter, but a browser submits * from Enter in any field of it, and the element a keypress names is the field, not the form. The * gateway would need to know the page's structure at decision time, which it does not, refs are * held off-DOM by Playwright and the policy runs before the action reaches the browser. So a rule diff --git a/server/tests/computer-gateway.test.ts b/server/tests/computer-gateway.test.ts index d6d54ad6..d8413d08 100644 --- a/server/tests/computer-gateway.test.ts +++ b/server/tests/computer-gateway.test.ts @@ -726,6 +726,134 @@ describe("the computer gateway", () => { expect(calls).toEqual(["click"]); }); + test("a submit typed into a field is the same Enter, and the rule catches it", async () => { + /* + * The second door, reached through the first tool. + * + * `computer_type` takes `submit`, which presses Enter when the text is in. A rule written + * about Enter saw a keypress and a click and not this, so an agent refused at both doors + * typed into the field with `submit: true` and the form went through. The preset shipped in + * `.env.example` and on the Boundaries page is exactly this rule, so the deployment that + * followed the product's own advice was the one with the hole in it. + */ + const { gateway, calls, rows } = await gatewayWith({ + ...PERMISSIVE, + deny: ['key == "Enter"'], + }); + + await expect( + gateway.type("bot-1", ACTOR, { + ref: "e1", + snapshotId: 7, + text: "Ada", + submit: true, + }), + ).rejects.toThrow(); + expect(calls).toEqual([]); + expect(rows[0]?.eventType).toBe("computer.action_refused"); + // The key belongs in the trail for the same reason it belongs in the decision: without it the + // row says somebody filled in a field, and not that they submitted the form. + expect((rows[0]?.payload as { key?: string } | undefined)?.key).toBe( + "Enter", + ); + }); + + test("typing without a submit is still typing", async () => { + // The other direction. A rule about Enter must not start refusing ordinary text, which is what + // a fix that simply reported every type as a keypress would do. + const { gateway, calls } = await gatewayWith({ + ...PERMISSIVE, + deny: ['key == "Enter"'], + }); + + await gateway.type("bot-1", ACTOR, { + ref: "e1", + snapshotId: 7, + text: "Ada", + }); + + expect(calls).toEqual(["type"]); + }); + + test("a submit is an activation, so an intent rule catches it too", async () => { + // `intent` is the other way an operator writes this, and it is the one the preset leads with. + // Enter pressed in a field presses whatever the form activates, the same as Space or a click. + const { gateway, calls } = await gatewayWith({ + ...PERMISSIVE, + deny: ['intent == "activate"'], + }); + + await expect( + gateway.type("bot-1", ACTOR, { + ref: "e1", + snapshotId: 7, + text: "Ada", + submit: true, + }), + ).rejects.toThrow(); + expect(calls).toEqual([]); + }); + + /* + * The rule the product ships, run against every door it claims to close. + * + * Written out here rather than imported because the two places it lives, `.env.example` and the + * Boundaries preset list, are a string in a comment and a string in the browser bundle. What + * this pins is the behaviour an operator gets by taking the product's own advice, so a change to + * either copy that reopens a door fails here. + */ + const SHIPPED_NEVER_SUBMIT = + '(intent == "activate" && contains(element.name, "submit")) || ((tool.name == "computer_key" || tool.name == "computer_type") && key == "Enter")'; + + test("the shipped preset closes the button, the keypress and the submit flag", async () => { + const shipped = { ...PERMISSIVE, deny: [SHIPPED_NEVER_SUBMIT] }; + + const click = await gatewayWith(shipped); + await expect( + click.gateway.click("bot-1", ACTOR, { ref: "e9", snapshotId: 7 }), + ).rejects.toThrow(); + expect(click.calls).toEqual([]); + + const enter = await gatewayWith(shipped); + await expect( + enter.gateway.key("bot-1", ACTOR, { + ref: "e1", + snapshotId: 7, + key: "Enter", + }), + ).rejects.toThrow(); + expect(enter.calls).toEqual([]); + + const submit = await gatewayWith(shipped); + await expect( + submit.gateway.type("bot-1", ACTOR, { + ref: "e1", + snapshotId: 7, + text: "Ada", + submit: true, + }), + ).rejects.toThrow(); + expect(submit.calls).toEqual([]); + }); + + test("the shipped preset still lets the Bot fill the form in", async () => { + // A boundary that stopped a Bot typing its way through a form would be refused by the first + // deployment to try it, and an operator would take the whole rule off rather than narrow it. + const shipped = { ...PERMISSIVE, deny: [SHIPPED_NEVER_SUBMIT] }; + + const typing = await gatewayWith(shipped); + await typing.gateway.type("bot-1", ACTOR, { + ref: "e1", + snapshotId: 7, + text: "Ada", + }); + expect(typing.calls).toEqual(["type"]); + + const looking = await gatewayWith(shipped); + await looking.gateway.screenshot("bot-1", ACTOR, {}); + expect(looking.calls).toEqual(["screenshot"]); + }); + test("but the rule still refuses the action it is about", async () => { // The point is not that these rules stop working. A neutral value answers honestly; it does // not answer no.