diff --git a/.env.example b/.env.example index 89d8a3999..b47f26cc4 100644 --- a/.env.example +++ b/.env.example @@ -90,7 +90,17 @@ AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS=true # which Bot took it. A rule can still restrict a single Bot with `bot.id`. # # Attributes: tool.name, bot.id, actor.id, page.url, page.host, element.ref/role/name/type, -# key, file.path, file.name, file.extension. +# key, file.path, file.name, file.extension, repeat.count. +# +# repeat.count is how many times this Bot has just made this exact call, counting the one being +# decided. A stuck model retries, and each retry is a real action on somebody's live website that is +# perfectly reasonable on its own terms; only the count tells the thirtieth click apart from the +# first. `repeat.count >= 10` in `deny` stops a Bot going in circles. Two calls are the same call +# when the thing acted on is the same, whatever was typed into it, so ten searches typed into one box +# are ten repeats and a rule about repetition refuses the tenth: try one in `dry-run` first. The +# count is held in memory by the process that served the call, so a deployment running two API +# replicas splits every count and a rule about ten attempts fires at twenty or never, and calls to +# another server's tools over MCP are not counted at all. # # 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 @@ -101,6 +111,16 @@ AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS=true # # AGENT_COMPUTER_POLICY={"mode":"enforce","deny":["(intent == \"activate\" && contains(element.name, \"submit\")) || (tool.name == \"computer_key\" && key == \"Enter\")"],"allow":["true"]} +# How long two identical calls count as the same repetition, in ms. Three minutes unset, which +# assumes a retry loop is a model round trip apart: call the tool, read the failure, try again. +# Widen it for a deployment whose provider is slow or heavily queued, where genuine retries arrive +# minutes apart and every attempt would otherwise be counted as the first one. Widen it too far and +# honest work starts to accumulate: a Bot told to watch a dashboard all morning reloads the same page +# and is not stuck. Anything that is not a positive whole number stops the server rather than falling +# back to the default, because a rule about repetition that never fires looks exactly like a Bot +# behaving itself. +# COMPUTER_REPEAT_WINDOW_MS=180000 + # 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/audit.tsx b/app/src/routes/_authed/admin/audit.tsx index d6f44e585..182ec683e 100644 --- a/app/src/routes/_authed/admin/audit.tsx +++ b/app/src/routes/_authed/admin/audit.tsx @@ -39,6 +39,12 @@ const FILTERS = [ "?eventType=computer.action_refused,mcp.call_rejected,component.refused,component.function_refused", }, { label: "Did not happen", search: "?eventType=computer.action_failed" }, + { + // Its own filter rather than a place in "Blocked". A Bot repeating itself has not been stopped by + // anything, and putting it beside the refusals would make the refusals look less real. + label: "Going in circles", + search: "?eventType=computer.action_repeated", + }, ] as const; function AuditPage() { @@ -163,6 +169,10 @@ function Row({ ) : null} + ) : typeof payload.fingerprint === "string" ? ( + // A repeat row has no element and no file of its own: what it is about is the call, which + // the fingerprint names in full. + {payload.fingerprint} ) : typeof payload.file === "string" ? ( {payload.file} ) : typeof element === "object" && element?.name ? ( @@ -223,6 +233,12 @@ function Row({ , reported by the Bot itself ) : null} + {event.eventType === "computer.action_repeated" && + typeof payload.count === "number" ? ( +
+ {payload.count} times within a few minutes +
+ ) : null} {failed && typeof payload.failure === "string" ? (
{payload.failure} @@ -268,6 +284,8 @@ const DECISIONS: Record = { "computer.secret_supplied": "A person supplied a secret", "computer.reset": "The computer was reset", "computer.stopped": "A person pressed stop", + // Not "Blocked". Nothing refused this; the Bot did the same thing again and the trail is saying so. + "computer.action_repeated": "The Bot repeated itself", "component.granted": "Granted to this Bot", "component.revoked": "Taken away from this Bot", diff --git a/app/src/routes/_authed/admin/boundaries.tsx b/app/src/routes/_authed/admin/boundaries.tsx index dd920c2d5..e8e76297d 100644 --- a/app/src/routes/_authed/admin/boundaries.tsx +++ b/app/src/routes/_authed/admin/boundaries.tsx @@ -32,6 +32,12 @@ const PRESETS: { label: string; rule: string; cost?: string }[] = [ rule: 'intent == "type" && contains(element.name, "password")', cost: "A password box the page labels something else is not covered, the rule matches the label.", }, + { + label: "Stop a Bot repeating itself", + // The count includes the attempt being decided, so this refuses the tenth, not the eleventh. + rule: "repeat.count >= 10", + cost: "Two calls count as the same call when the thing acted on is the same, whatever was typed into it, so a Bot running ten searches from one box, or reading one file ten times, is refused on the tenth. It misses the other way too: a Bot slow enough to spread its attempts wider than a few minutes is never caught, one that changes a single argument each time is ten different calls, and calls to another server's tools are not counted at all. Worth adding while a match is recorded and allowed, before it starts refusing anybody's work.", + }, { label: "Stay off social media", rule: 'intent == "navigate" && (contains(page.host, "facebook.com") || contains(page.host, "x.com"))', diff --git a/docs/architecture.md b/docs/architecture.md index 90b52c14d..9e5f14010 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -56,6 +56,15 @@ Policy rules can inspect: - `key` - `file.path`, `file.name`, `file.extension` - `mcp.server`, `mcp.tool`, `mcp.effect` +- `repeat.count` + +`repeat.count` is how many times that Bot has just made that exact call, counting the one being +decided. The gateway keys it on the tool plus the ref, key, file path, or target URL, over a sliding +window that defaults to three minutes and is set by `COMPUTER_REPEAT_WINDOW_MS`. Crossing 3, 10, or +25 writes one `computer.action_repeated` row each; the detector itself never refuses anything, so +`repeat.count >= 10` in `deny` is what stops a Bot going in circles. The count is held in memory by +the process that served the call, so two API replicas split it, and it covers the browser and the +workspace only: a call to another server's tools over MCP always reports one. Rules use CEL expressions plus case-insensitive `contains()` and `matches()`. Deny rules are evaluated before allow rules. The policy engine fails closed: a diff --git a/server/src/audit.ts b/server/src/audit.ts index 0ff57945d..405ea43fa 100644 --- a/server/src/audit.ts +++ b/server/src/audit.ts @@ -50,6 +50,22 @@ export const auditEventTypes = [ // Permitted by policy, attempted, and did not succeed. Its own type because "allowed" reads as // "happened", and a trail that cannot tell those apart misleads exactly when it matters most. "computer.action_failed", + /** + * The same call, again, and again. + * + * The rows above record actions one at a time, which is the only way to record them and the reason + * a Bot stuck in a retry loop is invisible here: thirty identical rows look like thirty rows. This + * one says the thing the sequence cannot, that these are the same call, and how many times. + * + * It is not a refusal. Nothing was forbidden and nothing was stopped; a Bot did the same thing + * again, which is often merely a retry that is about to work. Filing it as a refusal would teach a + * reader to skim past the refusals that are real, so it is its own type and the audit page gives it + * its own words. + * + * Written when a count crosses a threshold rather than on every repeat, because a row per attempt + * would bury the attempts themselves under the observation that they kept happening. + */ + "computer.action_repeated", // A person taking the wheel and giving it back. Recorded as a period rather than as keystrokes: the // useful fact for an investigator is that a human drove this browser between these two times, and // logging every click a person made would bury it while telling nobody anything. diff --git a/server/src/computer/gateway.ts b/server/src/computer/gateway.ts index b2337be96..c435d7805 100644 --- a/server/src/computer/gateway.ts +++ b/server/src/computer/gateway.ts @@ -25,6 +25,7 @@ import { type PolicyContext, type PolicyDecision, } from "./policy"; +import { createRepeatDetector, type RepeatDetector } from "./repeat"; import type { ClickInput, KeyInput, @@ -76,6 +77,15 @@ export type ComputerGatewayOptions = { auditStore: AuditStore; /** Absent denies everything. See evaluateActionPolicy. */ policy: () => ActionPolicy | undefined; + /** + * Counts a Bot repeating itself, so that the policy can be told how many times. + * + * Absent, the gateway makes its own, which is what almost every deployment gets. Passed in only to + * widen the window for a slow provider, or to hand a test a clock it can move, because otherwise + * proving that a window expires means a test that waits three minutes, and a test that waits three + * minutes is a test somebody eventually deletes. + */ + repeat?: RepeatDetector; }; /** @@ -95,6 +105,7 @@ type CachedSnapshot = { export function createComputerGateway(options: ComputerGatewayOptions) { const { client, auditStore, supervisor } = options; const snapshots = new Map(); + const repeat = options.repeat ?? createRepeatDetector(); /** * The computer, addressed as the Bot that is asking. @@ -169,11 +180,29 @@ export function createComputerGateway(options: ComputerGatewayOptions) { const intent = intentOf(toolName, subject.key); + /* + * Counted before the policy is asked, so that a rule written against the count decides the very + * attempt that crossed the line rather than the one after it. Off by one here would mean a + * deployment forbidding a tenth identical click allows the tenth and refuses the eleventh, which + * is the kind of thing nobody notices until they are counting rows in an incident. + * + * Reading a page never reaches this function, so nothing counts a Bot looking at the same screen + * over and over. That is the cheapest thing it does and the one nobody minds. + */ + const repetition = repeat.observe(botId, { + tool: toolName, + ref, + key: subject.key, + filePath, + targetUrl: subject.targetUrl, + }); + const context: PolicyContext = { tool: { name: toolName }, bot: { id: botId }, actor: { id: actor.id }, page: { url: pageUrl, host: hostOf(pageUrl) }, + repeat: { count: repetition.count }, ...(intent ? { intent } : {}), ...(subject.key ? { key: subject.key } : {}), ...(element @@ -189,6 +218,44 @@ export function createComputerGateway(options: ComputerGatewayOptions) { ...(filePath ? { file: describeFile(filePath) } : {}), }; + if (repetition.threshold !== null && repetition.fingerprint) { + /* + * Ahead of the decision row, so the trail reads in the order the thing happened: this was the + * tenth identical attempt, and this is what the policy did about it. Filed the other way round + * a reader has to deduce the cause from a row written after its effect. + * + * Its failure is swallowed, which nothing else in this file does. This row is an observation, + * and an observation is not allowed to refuse anything: letting a lost insert throw from here + * would stop every third, tenth and twenty-fifth identical call before the policy had even + * been asked, so a deployment that permits an action would lose it to a moment's trouble at the + * audit store. Nothing is weakened by that. An action that was not recorded still does not + * happen, because the decision row goes to the same store a few lines below, and a store that + * is genuinely down refuses the action there. + */ + try { + await writeRepeat(auditStore, { + toolName, + botId, + actor, + computerId, + pageUrl, + filePath, + fingerprint: repetition.fingerprint, + count: repetition.count, + }); + } catch (error) { + console.error( + JSON.stringify({ + type: "computer-repeat-row-lost", + bot: botId, + fingerprint: repetition.fingerprint, + count: repetition.count, + error: String(error), + }), + ); + } + } + const decision = evaluateActionPolicy(options.policy(), context); await write(auditStore, { toolName, @@ -732,6 +799,49 @@ async function write( }); } +/** + * One row for a Bot going round in circles. + * + * Separate from `write` because there is no policy decision to record. This row is an observation + * about the call that is about to be decided, not the decision, and giving it a `decision` block + * would mean inventing an answer the policy was never asked for. It is also why it is not a refusal: + * nothing was forbidden here. + * + * The fingerprint goes in as written, which is why `repeat.ts` builds a readable one. A reader + * arriving at "the same call, 25 times" needs to be told which call in the row itself. + */ +async function writeRepeat( + auditStore: AuditStore, + entry: { + toolName: string; + botId: string; + actor: ActionActor; + computerId: string; + pageUrl: string; + filePath: string | undefined; + fingerprint: string; + count: number; + }, +) { + await recordAuditEvent(auditStore, { + eventType: "computer.action_repeated", + targetType: "computer", + targetId: entry.computerId, + ...(entry.actor.userId ? { actorUserId: entry.actor.userId } : {}), + payload: { + action: entry.toolName, + bot: entry.botId, + actor: entry.actor.id, + // The page, for a browser action only. A file call has nothing to do with whatever the browser + // happens to be showing, and naming a host on that row sends a reader somewhere irrelevant, the + // same trap `describeRefusal` avoids. + ...(entry.filePath ? {} : { page: entry.pageUrl }), + fingerprint: entry.fingerprint, + count: entry.count, + }, + }); +} + /** * The host a rule can match on, or empty. * diff --git a/server/src/computer/policy.ts b/server/src/computer/policy.ts index 7b18ceeb8..06ac3a97e 100644 --- a/server/src/computer/policy.ts +++ b/server/src/computer/policy.ts @@ -46,6 +46,34 @@ export type PolicyContext = { bot: { id: string }; page: { url: string; host: string }; actor: { id: string }; + /** + * How many times this Bot has just made this exact call, counting the one being decided. + * + * A stuck model retries, and every retry is a real action on somebody's live website. Each one is + * permitted on its own terms, because each one is: the rule that would refuse the thirtieth click + * on a button would refuse the first, and refusing the first is refusing the product. Only the + * count separates them, so the count is here, and a deployment that wants to stop a Bot going in + * circles writes `repeat.count >= 10` and nothing else changes. + * + * Always present, at one on a call the Bot has not made before, so that a rule mentioning it is + * evaluable on every action. An absent field would throw inside CEL, and a deny rule that throws + * denies, so an optional `repeat` would turn one rule about repetition into a deployment that + * refuses everything. + * + * It is wrong in both directions, and a rule written against it has to be worth both. Under, three + * ways: the window is time-based, so a Bot slow enough to spread its attempts wider than the window + * never trips this, and one that varies a single argument each time round is thirty calls; the + * count is held by the process that served the call, so a deployment behind two API replicas + * splits every count and a rule about ten attempts fires at twenty or never; and a call to another + * server's tools over MCP is not counted at all, because only the computer gateway counts. + * + * Over, once, and that one costs somebody their Bot rather than their evidence. Two calls are the + * same call when the thing acted on is the same, whatever was typed into it, so ten searches typed + * into one box and one file read ten times while a Bot works through it are both ten repeats, and + * `repeat.count >= 10` refuses the tenth. It is a backstop against the loop that actually happens, + * not a guarantee, which is the argument for trying a rule about it in `dry-run` first. + */ + repeat: { count: number }; element?: { ref: string; role: string; diff --git a/server/src/computer/repeat.ts b/server/src/computer/repeat.ts new file mode 100644 index 000000000..297a5235a --- /dev/null +++ b/server/src/computer/repeat.ts @@ -0,0 +1,336 @@ +/** + * How many times a Bot has just made this exact call. + * + * A model that cannot get something to work retries. It clicks the same button, reloads the same + * page, writes the same file, and every one of those attempts is a real action on somebody's live + * website and a real charge against somebody's model credit. The trail records all thirty of them, + * one row at a time, with nothing to say that they are the same row thirty times over, so nobody + * notices until the other side's rate limiter does. + * + * This counts, and it does nothing else with the answer. The count goes to the policy, which is the + * one thing in this codebase allowed to refuse an action. A detector that blocked on its own would + * be a second boundary with rules of its own, invisible on the Boundaries page, unanswerable to + * `dry-run`, and impossible to switch off for the one Bot whose job really is to poll something. + * One boundary, given better information. + * + * In memory, and per process, for the same reason the gateway's snapshot cache is: it describes what + * a Bot did in the last few minutes, and after a restart that question has no useful answer. The + * loop is over, because whatever was driving it is gone too. + */ + +/** + * How long a call stays counted. + * + * Time rather than "this turn", because the gateway has no idea where a turn begins or ends. It is + * handed one action at a time by whichever route is serving the model, and nothing in that path + * carries a turn boundary; inventing one would mean threading a conversation id through every acting + * call to get a worse answer than a clock gives. + * + * Three minutes. A retry loop is a model round trip each time round — call the tool, read the + * failure, decide to try again — which is seconds, not minutes, so ten identical attempts fit inside + * this comfortably. Much longer and honest repetition starts to accumulate: a Bot told to watch a + * dashboard reloads the same page all morning and is not stuck. Much shorter and a model that thinks + * for twenty seconds between attempts never trips anything. + */ +export const DEFAULT_REPEAT_WINDOW_MS = 3 * 60_000; + +/** + * The counts worth writing a row about. + * + * Three is "this is now a pattern rather than a retry", ten is "nobody is going to fix this by + * trying again", twenty-five is "somebody should look". Each fires once, so the trail gains three + * rows for a Bot stuck all afternoon rather than a row per attempt, which would bury the actions it + * was taking under the observation that it kept taking them. + */ +export const DEFAULT_REPEAT_THRESHOLDS: readonly number[] = [3, 10, 25]; + +/** + * How many distinct calls are remembered per Bot. + * + * A Bot doing genuinely varied work never repeats anything, so every call it makes is a new key, and + * without a cap the map grows for exactly the Bots this feature has nothing to say about. + * + * Full means full. A call the Bot has not made before is not counted, and nothing still inside the + * window is dropped to make room for it, because dropping something is a guess at which key will not + * come round again and the obvious guess is the one that fails hardest. Least recently seen is + * exactly the key a Bot going round a long loop is about to make next, so a loop of more than this + * many distinct calls would lose each key one step before it returned, and a Bot in a tight circle + * would be reported as making every one of its calls for the first time. + * + * The cost is the Bot whose first sixty-four distinct calls are honest work and which only then gets + * stuck: its loop is invisible until one of those sixty-four falls out of the window, which is at + * most one window away. A blind spot that clears itself is worth more than an eviction rule that can + * be wrong for as long as the loop lasts, and it buys the other half of the bargain, that a call + * already being counted can never be pushed out by a Bot doing other things in between. + */ +export const DEFAULT_REPEAT_KEYS_PER_BOT = 64; + +/** + * How many Bots are remembered at once. + * + * The id counted against is the one in the request path. It is checked against a session and against + * nothing else, because a Bot's computer answers to whatever it is addressed as and no acting route + * resolves the id to a row in `bots` first. So the number of ids this map can be asked to hold is + * not the number of Bots somebody wrote down, it is however many a signed-in caller cares to type, + * and an uncapped map of maps would grow for the life of the process on nothing more than a loop of + * requests naming a fresh id each time. + * + * Reclaimed the way the per-Bot cap is: a Bot that has gone quiet for a whole window gives its place + * up, and while every place is held by a Bot that is still working, one more Bot is not counted. + * Two hundred and fifty-six, which is far more Bots than a deployment has acting inside any three + * minutes, and small enough that the worst case is a few megabytes rather than the whole heap. + */ +export const DEFAULT_REPEAT_BOTS = 256; + +/** + * One governed call, in the terms the detector cares about. + * + * The same fields the gateway already assembles for the policy and the audit row. Nothing is added + * to a call site to support this. + */ +export type RepeatedCall = { + tool: string; + ref?: string | undefined; + key?: string | undefined; + filePath?: string | undefined; + targetUrl?: string | undefined; +}; + +export type RepeatObservation = { + /** + * Including the call being observed, so the first one counts as one. + * + * One is also the answer for a call the detector had no room to remember, and for one carrying + * nothing to identify it. It is the only number that cannot make anything happen: a rule about + * repetition reads it as a first attempt and stands aside. A count nobody can substantiate must + * never be the reason a Bot is refused. + */ + count: number; + /** + * The call's identity, in a form a person can read. + * + * Null when nothing distinguished it. See `fingerprintOf`: a bare tool name is not a call worth + * counting, and the honest count for one of those is a single occurrence. + */ + fingerprint: string | null; + /** + * The threshold this call has just reached, or null on the overwhelming majority of calls. + * + * Reported once per run of repetition rather than on every call past it. A run ends when the + * window empties completely, and a key that fills it again afterwards reports again, because that + * is a Bot that got stuck twice. A count that merely dips and climbs is the same run still going + * and says nothing further: one incident, one row per threshold, or a row per wobble would be a + * row per attempt under another name. + */ + threshold: number | null; +}; + +export type RepeatDetector = { + /** Records the call and answers with what it now knows. Never throws, never blocks. */ + observe: (botId: string, call: RepeatedCall) => RepeatObservation; +}; + +export type RepeatDetectorOptions = { + windowMs?: number; + thresholds?: readonly number[]; + maxKeysPerBot?: number; + maxBots?: number; + /** Injected so a test can move time without waiting for it. */ + now?: () => number; +}; + +/** One key's history, held only while it is still inside the window. */ +type Occurrences = { + /** When each counted call happened, oldest first. */ + at: number[]; + /** Which thresholds this run of repetition has already reported. */ + reported: Set; +}; + +/** + * One Bot's history, and when it was last heard from. + * + * The time is kept here as well as inside the keys because a Bot that has stopped acting has to give + * its place up without anybody walking its whole map to work out that it has. + */ +type BotHistory = { + calls: Map; + lastSeen: number; +}; + +export function createRepeatDetector( + options: RepeatDetectorOptions = {}, +): RepeatDetector { + const windowMs = options.windowMs ?? DEFAULT_REPEAT_WINDOW_MS; + const maxKeysPerBot = options.maxKeysPerBot ?? DEFAULT_REPEAT_KEYS_PER_BOT; + const maxBots = options.maxBots ?? DEFAULT_REPEAT_BOTS; + // Ascending, so the loop below ends on the highest threshold a call crossed rather than on + // whichever one the caller happened to list last. + const thresholds = [ + ...(options.thresholds ?? DEFAULT_REPEAT_THRESHOLDS), + ].sort((a, b) => a - b); + const clock = options.now ?? Date.now; + + /** + * Per Bot, then per call. + * + * Nested rather than keyed on a combined string so that one Bot's varied work cannot push another + * Bot's history out: the cap is per Bot, and a shared map would make it a race between them. + * + * Both levels are capped, and neither ever drops something that is still inside the window. What + * is in here is therefore the recent past and nothing else, which is the only claim about memory + * this module can honestly make: an id it has never usefully counted cannot be made to sit here + * for the life of the process. + */ + const perBot = new Map(); + + return { + observe(botId, call) { + const fingerprint = fingerprintOf(call); + if (!fingerprint) { + return { count: 1, fingerprint: null, threshold: null }; + } + + const now = clock(); + const cutoff = now - windowMs; + + let history = perBot.get(botId); + if (!history) { + if (perBot.size >= maxBots) forgetQuietBots(perBot, cutoff); + if (perBot.size >= maxBots) return untracked(fingerprint); + history = { calls: new Map(), lastSeen: now }; + perBot.set(botId, history); + } + // Whether or not the call itself is counted. A Bot making calls is a Bot at work, and one that + // has filled its keys with a long loop must not lose the loop by looking idle. + history.lastSeen = now; + const calls = history.calls; + + let entry = calls.get(fingerprint); + if (!entry) { + if (calls.size >= maxKeysPerBot) forgetExpiredCalls(calls, cutoff); + if (calls.size >= maxKeysPerBot) return untracked(fingerprint); + entry = { at: [], reported: new Set() }; + calls.set(fingerprint, entry); + } + + trimToWindow(entry.at, cutoff); + if (entry.at.length === 0) { + // Nothing survived the window, so whatever run of repetition this key was in has ended and + // its thresholds are free to report again. Without this a Bot that got stuck, recovered and + // got stuck again an hour later would leave one row for two incidents. + entry.reported.clear(); + } + entry.at.push(now); + + const count = entry.at.length; + let threshold: number | null = null; + for (const candidate of thresholds) { + if (count >= candidate && !entry.reported.has(candidate)) { + entry.reported.add(candidate); + threshold = candidate; + } + } + + return { count, fingerprint, threshold }; + }, + }; +} + +/** + * What makes two calls the same call. + * + * The tool name is not enough, and a detector keyed on it alone would be worse than none: five + * clicks may be five different buttons, so a Bot working steadily down a form would look exactly + * like one stuck on its first field, and the first person to see a rule fire on that would turn the + * feature off. So the key is the tool plus the argument saying WHICH thing it acted on — the ref, + * the key pressed, the file path, the address being opened — and a call carrying none of those is + * not counted at all. + * + * Deliberately absent: the text being typed. A Bot that fills the same field thirty times is worth + * catching, but what it filled it with is a password as often as it is anything else, and from here + * the fingerprint travels into an audit row. The cost is that thirty different values into one field + * read as thirty repeats, which is the direction to be wrong in. + * + * A ref only means anything against the snapshot it came from. A page that re-renders and hands back + * a different ref for the same button reads as a different call, so a Bot looping around a reload is + * undercounted. Keying on the element's label instead would follow the button across snapshots and + * merge two buttons that share a label, and a count that is sometimes low is easier to live with + * than one that is sometimes about the wrong thing. + * + * Readable, because it goes on the audit row as-is. An investigator reading "the same action 25 + * times" needs to be told which action without going and decoding a hash. + */ +export function fingerprintOf(call: RepeatedCall): string | null { + const parts: string[] = []; + const add = (label: string, value: string | undefined) => { + const normalized = normalize(value); + if (normalized) parts.push(`${label}=${normalized}`); + }; + + add("ref", call.ref); + add("key", call.key); + add("file", call.filePath); + add("url", call.targetUrl); + + if (parts.length === 0) return null; + return [normalize(call.tool) || call.tool, ...parts].join(" "); +} + +/** + * Whitespace collapsed and trimmed. + * + * A model reproducing an argument from its own earlier output does not always reproduce the spacing, + * and a Bot writing to `reports/q3.md` and to `reports/q3.md ` is doing one thing twice. Treating + * those as two calls would let a stuck Bot slip the count without changing anything about what it + * was actually doing. + */ +function normalize(value: string | undefined): string { + return (value ?? "").replaceAll(/\s+/g, " ").trim(); +} + +/** + * What a call is worth when there was no room to remember it. + * + * The fingerprint is still returned, because it is a fact about the call and costs nothing to say. + * The count is one and the threshold is null, so nothing downstream acts on a number this module + * could not stand behind. + */ +function untracked(fingerprint: string): RepeatObservation { + return { count: 1, fingerprint, threshold: null }; +} + +/** + * Timestamps outside the window, dropped in place. + * + * Oldest first, so the scan stops at the first survivor and costs what it actually removes rather + * than the length of the list. Rebuilding the list instead would make a Bot hammering one call pay + * for its own history on every attempt, which is the Bot this module exists to describe. + */ +function trimToWindow(at: number[], cutoff: number) { + const surviving = at.findIndex((time) => time > cutoff); + if (surviving === -1) { + at.length = 0; + return; + } + if (surviving > 0) at.splice(0, surviving); +} + +/** + * Keys whose last call has aged out, which are the only ones safe to forget. + * + * They carry no count any more: the next call on one of them would start from one whether it was + * held or not, so dropping it loses nothing and frees the place for a call that might. + */ +function forgetExpiredCalls(calls: Map, cutoff: number) { + for (const [key, entry] of calls) { + if ((entry.at.at(-1) ?? 0) <= cutoff) calls.delete(key); + } +} + +/** The same rule one level up: a Bot that has not acted for a whole window has nothing left to say. */ +function forgetQuietBots(perBot: Map, cutoff: number) { + for (const [botId, history] of perBot) { + if (history.lastSeen <= cutoff) perBot.delete(botId); + } +} diff --git a/server/src/config.ts b/server/src/config.ts index 7d2c1a752..d9f8bd9fd 100644 --- a/server/src/config.ts +++ b/server/src/config.ts @@ -73,6 +73,15 @@ export type DeploymentConfig = { * precedence, which is the only subtle thing about them, impossible to see. */ policy?: ActionPolicy; + /** + * How long two identical calls count as the same repetition, in milliseconds. + * + * Absent uses the built-in window, which assumes a retry loop is a model round trip apart. It is + * here because that assumption is about someone else's model: a deployment on a slow or heavily + * queued provider can have genuine retries minutes apart, and there the built-in window counts + * every attempt as the first one and a rule about repetition never fires at all. + */ + repeatWindowMs?: number; }; }; @@ -269,11 +278,13 @@ function computerConfig( const computerToken = optional(environment, "COMPUTER_TOKEN"); const supervisorUrl = url(environment, "COMPUTER_SUPERVISOR_URL"); const supervisorToken = optional(environment, "SUPERVISOR_TOKEN"); + const repeatWindowMs = milliseconds(environment, "COMPUTER_REPEAT_WINDOW_MS"); return { baseUrl, allowPrivateHosts: optional(environment, "AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS") === "true", ...(policy ? { policy } : {}), + ...(repeatWindowMs ? { repeatWindowMs } : {}), ...(computerToken ? { token: computerToken } : {}), ...(supervisorUrl ? { @@ -286,6 +297,29 @@ function computerConfig( }; } +/** + * A duration in milliseconds, or a refusal to start. + * + * Refused rather than quietly defaulted, for the same reason a malformed policy is. An operator who + * widened a window and typed `3m` would otherwise get a running deployment on the built-in value, + * and the only evidence would be a rule that never fires, which reads exactly like a Bot behaving + * itself. + */ +function milliseconds( + environment: Environment, + name: string, +): number | undefined { + const raw = optional(environment, name); + if (!raw) { + return undefined; + } + const value = Number(raw); + if (!Number.isSafeInteger(value) || value <= 0) { + throw new Error(`${name} must be a positive whole number of milliseconds`); + } + return value; +} + /** * The action policy, as JSON in one variable. * diff --git a/server/src/index.ts b/server/src/index.ts index 2d72bca9f..cb9f7f0f2 100644 --- a/server/src/index.ts +++ b/server/src/index.ts @@ -22,6 +22,7 @@ import { createPolicyStore, DEFAULT_ACTION_POLICY, } from "./computer/policy-store"; +import { createRepeatDetector } from "./computer/repeat"; import { createSupervisorClient } from "./computer/supervisor"; import { loadConfig } from "./config"; import { createConnectorAdminService } from "./connectors"; @@ -329,6 +330,15 @@ const app = createApp( policy: () => policyStore.get(), // Stop, reset and the listing act on containers when there are containers to act on. ...(supervisor ? { supervisor } : {}), + // Only when a deployment has said its Bots retry on a slower rhythm than the built-in window + // assumes. Otherwise the gateway makes its own and nobody has to know it exists. + ...(config.computer?.repeatWindowMs + ? { + repeat: createRepeatDetector({ + windowMs: config.computer.repeatWindowMs, + }), + } + : {}), }) : undefined, policyStore, diff --git a/server/src/plugins/store.ts b/server/src/plugins/store.ts index b4caf3950..66fc4b46c 100644 --- a/server/src/plugins/store.ts +++ b/server/src/plugins/store.ts @@ -799,6 +799,12 @@ export function createPluginStore(options: PluginStoreOptions) { bot: { id: input.botId }, actor: { id: input.actorId }, page: { url: "", host: "" }, + // One, for the same reason as the empty strings, and with a cost worth naming: repetition is + // counted by the computer gateway, and nothing counts a Bot calling the same MCP tool over + // and over. A rule about repetition is therefore false here rather than unevaluable, which + // keeps a browser rule from refusing every tool call, and leaves a Bot looping through + // somebody else's server as a gap this deployment cannot yet see. + repeat: { count: 1 }, element: { ref: "", role: "", name: "", type: "" }, key: "", file: { path: "", name: "", extension: "" }, diff --git a/server/tests/computer-gateway.test.ts b/server/tests/computer-gateway.test.ts index eff68c07c..db7576b1a 100644 --- a/server/tests/computer-gateway.test.ts +++ b/server/tests/computer-gateway.test.ts @@ -6,6 +6,10 @@ import { createComputerGateway, } from "../src/computer/gateway"; import type { ActionPolicy } from "../src/computer/policy"; +import { + createRepeatDetector, + type RepeatDetector, +} from "../src/computer/repeat"; import type { SnapshotResult } from "../src/computer/schema"; /** @@ -103,13 +107,18 @@ function fakeAudit() { const ACTOR = { id: "dev-local-user" }; const PERMISSIVE: ActionPolicy = { mode: "enforce", deny: [], allow: ["true"] }; -async function gatewayWith(policy: ActionPolicy | undefined) { +async function gatewayWith( + policy: ActionPolicy | undefined, + /** Only the repetition tests supply one; everything else gets the gateway's own. */ + repeat?: RepeatDetector, +) { const { client, calls } = fakeClient(); const { store, rows } = fakeAudit(); const gateway = createComputerGateway({ client, auditStore: store, policy: () => policy, + ...(repeat ? { repeat } : {}), }); // Every test acts on refs, so the server must hold a snapshot first, exactly as the real flow does. await gateway.snapshot("default"); @@ -397,3 +406,212 @@ describe("the computer gateway", () => { expect(rows[0]?.payload.element).toBe("not in the current snapshot"); }); }); + +/** + * The gateway is the only place that can count this, which is why it does. + * + * Every governed action already passes through one function that already writes a row for it, so + * counting is very nearly free here and impossible anywhere else. The part that matters is where the + * count goes: into the policy context, before the decision, so a deployment can act on it rather than + * read about it a week later. + */ +describe("a Bot going in circles", () => { + const click = { ref: "e9", snapshotId: 7 }; + + test("the count reaches the policy, and the rule refuses the attempt that crosses the line", async () => { + const { gateway, calls, rows } = await gatewayWith({ + ...PERMISSIVE, + deny: ["repeat.count >= 3"], + }); + + await gateway.click("default", "bot-1", ACTOR, click); + await gateway.click("default", "bot-1", ACTOR, click); + // The first two are the same action on the same button and nothing about them is objectionable. + expect(calls).toEqual(["click", "click"]); + + await expect( + gateway.click("default", "bot-1", ACTOR, click), + ).rejects.toThrow(ActionRefusedError); + // Counted before the decision, so the third attempt is the one refused rather than the fourth. + expect(calls).toEqual(["click", "click"]); + expect(rows.at(-1)?.eventType).toBe("computer.action_refused"); + }); + + test("a rule about repetition leaves a Bot doing varied work alone", async () => { + // The failure that would take this feature back out again: a Bot working steadily down a form + // refused for doing its job. + const { gateway, calls } = await gatewayWith({ + ...PERMISSIVE, + deny: ["repeat.count >= 3"], + }); + + await gateway.click("default", "bot-1", ACTOR, { + ref: "e1", + snapshotId: 7, + }); + await gateway.click("default", "bot-1", ACTOR, { + ref: "e9", + snapshotId: 7, + }); + await gateway.type("default", "bot-1", ACTOR, { + ref: "e1", + snapshotId: 7, + text: "Grace Hopper", + }); + + expect(calls).toEqual(["click", "click", "type"]); + }); + + test("crossing a threshold writes its own row, ahead of the decision it explains", async () => { + const { gateway, rows } = await gatewayWith( + PERMISSIVE, + createRepeatDetector({ thresholds: [3] }), + ); + + await gateway.click("default", "bot-1", ACTOR, click); + await gateway.click("default", "bot-1", ACTOR, click); + await gateway.click("default", "bot-1", ACTOR, click); + + // Two allowed actions, then the observation, then the third allowed action. Filed the other way + // round a reader has to deduce the cause from a row written after its effect. + expect(rows.map((row) => row.eventType)).toEqual([ + "computer.action_allowed", + "computer.action_allowed", + "computer.action_repeated", + "computer.action_allowed", + ]); + }); + + test("the row says which call, and how many times", async () => { + const { gateway, rows } = await gatewayWith( + PERMISSIVE, + createRepeatDetector({ thresholds: [2] }), + ); + + await gateway.click("default", "bot-1", ACTOR, click); + await gateway.click("default", "bot-1", ACTOR, click); + + const repeated = rows.find( + (row) => row.eventType === "computer.action_repeated", + ); + expect(repeated?.payload).toMatchObject({ + action: "computer_click", + bot: "bot-1", + fingerprint: "computer_click ref=e9", + count: 2, + }); + // Not a refusal, and it must not carry the furniture of one. A row with a `decision` block would + // read as the policy having answered a question nobody asked it. + expect(repeated?.payload.decision).toBeUndefined(); + }); + + test("a refused action is still counted, because the Bot still tried", async () => { + // A Bot hammering a button the policy forbids is going in circles as surely as one hammering a + // button that works, and it is the case an operator most wants to see. + const { gateway, rows } = await gatewayWith( + { ...PERMISSIVE, deny: ['contains(element.name, "submit")'] }, + createRepeatDetector({ thresholds: [2] }), + ); + + await gateway.click("default", "bot-1", ACTOR, click).catch(() => {}); + await gateway.click("default", "bot-1", ACTOR, click).catch(() => {}); + + expect(rows.map((row) => row.eventType)).toEqual([ + "computer.action_refused", + "computer.action_repeated", + "computer.action_refused", + ]); + }); + + test("two Bots on one gateway do not pool a count", async () => { + const { gateway, rows } = await gatewayWith( + PERMISSIVE, + createRepeatDetector({ thresholds: [2] }), + ); + + await gateway.click("default", "sales-bot", ACTOR, click); + await gateway.click("default", "research-bot", ACTOR, click); + + // One click each. A pooled count would file this against whichever Bot happened to go second. + expect( + rows.some((row) => row.eventType === "computer.action_repeated"), + ).toBe(false); + }); + + test("a call with nothing to distinguish it is never reported as a repeat", async () => { + // Scrolling names no element. Counting it by tool name alone would report a Bot reaching the + // bottom of a long page as one going in circles. + const { gateway, rows } = await gatewayWith( + PERMISSIVE, + createRepeatDetector({ thresholds: [2] }), + ); + + await gateway.scroll("default", "bot-1", ACTOR, { direction: "down" }); + await gateway.scroll("default", "bot-1", ACTOR, { direction: "down" }); + await gateway.scroll("default", "bot-1", ACTOR, { direction: "down" }); + + expect( + rows.every((row) => row.eventType === "computer.action_allowed"), + ).toBe(true); + }); + + test("a lost observation row does not refuse an action the policy allows", async () => { + // The detector observes and the policy decides, and that has to survive the audit store having a + // bad moment. Letting the observation row throw would refuse every third, tenth and twenty-fifth + // identical call, before the policy had been asked, on an action nothing objected to. + const { client, calls } = fakeClient(); + const rows: AuditEventInput[] = []; + const store: AuditStore = { + insert: async (event) => { + if (event.eventType === "computer.action_repeated") { + throw new Error("the audit store is unreachable"); + } + rows.push(event); + }, + }; + const gateway = createComputerGateway({ + client, + auditStore: store, + policy: () => PERMISSIVE, + repeat: createRepeatDetector({ thresholds: [2] }), + }); + await gateway.snapshot("default"); + + await gateway.click("default", "bot-1", ACTOR, click); + await gateway.click("default", "bot-1", ACTOR, click); + + // Both clicks happened and both decisions are on the record. What was lost is the note saying + // they were the same click twice, which is the only thing that may be lost here. + expect(calls).toEqual(["click", "click"]); + expect(rows.map((row) => row.eventType)).toEqual([ + "computer.action_allowed", + "computer.action_allowed", + ]); + }); + + test("a repeated file write names the path and not the browser's page", async () => { + // The workspace has nothing to do with whatever the browser happens to be showing, and naming a + // host on that row sends a reader somewhere irrelevant. + const { gateway, rows } = await gatewayWith( + PERMISSIVE, + createRepeatDetector({ thresholds: [2] }), + ); + + await gateway.writeFile("default", "bot-1", ACTOR, { + path: "notes.md", + contents: "again", + }); + await gateway.writeFile("default", "bot-1", ACTOR, { + path: "notes.md", + contents: "again", + }); + + const repeated = rows.find( + (row) => row.eventType === "computer.action_repeated", + ); + expect(repeated?.payload.fingerprint).toBe( + "computer_write_file file=notes.md", + ); + expect(repeated?.payload.page).toBeUndefined(); + }); +}); diff --git a/server/tests/computer-policy.test.ts b/server/tests/computer-policy.test.ts index 5191793cb..c2c8efe50 100644 --- a/server/tests/computer-policy.test.ts +++ b/server/tests/computer-policy.test.ts @@ -21,6 +21,10 @@ function context(overrides: Partial = {}): PolicyContext { bot: { id: "risk-analyst" }, actor: { id: "dev-local-user" }, page: { url: "https://example.com/order", host: "example.com" }, + // The first time this Bot has made this call, which is what a context with nothing to say about + // repetition means. Always present: an absent field throws inside CEL, and a throwing deny rule + // denies, so a rule about repetition would otherwise refuse everything. + repeat: { count: 1 }, element: { ref: "e13", role: "button", name: "Submit order" }, ...overrides, }; @@ -191,6 +195,7 @@ describe("the second door", () => { bot: { id: "sales" }, actor: { id: "someone" }, page: { url: "https://example.com/order", host: "example.com" }, + repeat: { count: 1 }, key: "Enter", }); expect(refused.allowed).toBe(false); @@ -201,6 +206,7 @@ describe("the second door", () => { bot: { id: "sales" }, actor: { id: "someone" }, page: { url: "https://example.com/order", host: "example.com" }, + repeat: { count: 1 }, key: "a", }); expect(allowed.allowed).toBe(true); @@ -217,6 +223,7 @@ describe("a rule written about what an action does", () => { bot: { id: "b" }, actor: { id: "a" }, page: { url: "https://example.com/", host: "example.com" }, + repeat: { count: 1 }, intent: "activate", ...extra, }); @@ -307,6 +314,7 @@ describe("a rule that names an identifier only some actions carry", () => { bot: { id: "b" }, actor: { id: "a" }, page: { url: "https://httpbin.org/forms/post", host: "httpbin.org" }, + repeat: { count: 1 }, intent: "navigate", }; @@ -350,3 +358,43 @@ describe("a rule that names an identifier only some actions carry", () => { expect(decision.allowed).toBe(false); }); }); + +/** + * The one attribute that separates the thirtieth click on a button from the first. + * + * Both are the same action on the same element, and any rule able to refuse the thirtieth by its + * shape would refuse the first as well. So the count is the whole of it, and these check that a rule + * written against it actually evaluates: `repeat` is a nested field like `page` and `element`, and a + * rule the engine cannot evaluate denies, which would turn one restriction into a Bot that can do + * nothing at all. + */ +describe("a rule about a Bot repeating itself", () => { + const repeating: ActionPolicy = { + mode: "enforce", + deny: ["repeat.count >= 10"], + allow: ["true"], + }; + + test("leaves the attempts below the line alone", () => { + expect( + evaluateActionPolicy(repeating, context({ repeat: { count: 9 } })) + .allowed, + ).toBe(true); + }); + + test("refuses the attempt that crosses it, not the one after", () => { + const decision = evaluateActionPolicy( + repeating, + context({ repeat: { count: 10 } }), + ); + expect(decision.allowed).toBe(false); + expect(decision.matched).toBe("repeat.count >= 10"); + }); + + test("goes on refusing past it", () => { + expect( + evaluateActionPolicy(repeating, context({ repeat: { count: 40 } })) + .allowed, + ).toBe(false); + }); +}); diff --git a/server/tests/computer-repeat.test.ts b/server/tests/computer-repeat.test.ts new file mode 100644 index 000000000..f45b5459b --- /dev/null +++ b/server/tests/computer-repeat.test.ts @@ -0,0 +1,393 @@ +import { describe, expect, test } from "bun:test"; +import { + createRepeatDetector, + DEFAULT_REPEAT_WINDOW_MS, + fingerprintOf, +} from "../src/computer/repeat"; + +/** + * What the detector must get right, and every one of them is a way of being wrong that looks fine. + * + * A detector that overcounts is worse than none: the first person to see a boundary refuse a Bot + * working steadily down a form turns the boundary off, and takes the real refusals with it. A + * detector that fires its thresholds again on every call past them buries the actions a Bot took + * under the observation that it kept taking them. And a detector two Bots share reports one Bot's + * loop against another Bot's name, which is the one thing an audit trail may never do. + * + * Time is injected. A test that waits three minutes to prove a window expires is a test somebody + * eventually deletes. + */ + +/** A clock a test drives by hand, so a window can expire in a microsecond. */ +function clock(start = 1_000_000) { + let time = start; + return { + now: () => time, + advance(ms: number) { + time += ms; + }, + }; +} + +describe("counting a Bot repeating itself", () => { + test("identical calls count up", () => { + const detector = createRepeatDetector({ now: clock().now }); + const call = { tool: "computer_click", ref: "e9" }; + + expect(detector.observe("sales-bot", call).count).toBe(1); + expect(detector.observe("sales-bot", call).count).toBe(2); + expect(detector.observe("sales-bot", call).count).toBe(3); + }); + + test("the count includes the call being observed", () => { + // The gateway counts before it asks the policy, so a rule saying `repeat.count >= 10` has to + // refuse the tenth attempt. Starting at zero would make it refuse the eleventh, and nobody would + // notice until they were counting rows in an incident. + const detector = createRepeatDetector({ now: clock().now }); + expect( + detector.observe("sales-bot", { tool: "computer_click", ref: "e1" }), + ).toMatchObject({ count: 1 }); + }); + + test("a different argument is a different call", () => { + const detector = createRepeatDetector({ now: clock().now }); + + detector.observe("sales-bot", { tool: "computer_click", ref: "e1" }); + detector.observe("sales-bot", { tool: "computer_click", ref: "e2" }); + const third = detector.observe("sales-bot", { + tool: "computer_click", + ref: "e3", + }); + + // Three clicks, three buttons. A Bot working down a form must not look like one stuck on its + // first field, or the feature gets switched off the first day somebody uses it. + expect(third.count).toBe(1); + }); + + test("a different tool on the same argument is a different call", () => { + const detector = createRepeatDetector({ now: clock().now }); + + detector.observe("sales-bot", { + tool: "computer_read_file", + filePath: "a", + }); + const written = detector.observe("sales-bot", { + tool: "computer_write_file", + filePath: "a", + }); + + expect(written.count).toBe(1); + }); + + test("whitespace around an argument does not buy a Bot a fresh count", () => { + // A model reproducing a path from its own earlier output does not always reproduce the spacing, + // and it is doing the same thing either way. + const detector = createRepeatDetector({ now: clock().now }); + + detector.observe("bot", { tool: "computer_write_file", filePath: "q3.md" }); + const spaced = detector.observe("bot", { + tool: "computer_write_file", + filePath: " q3.md ", + }); + + expect(spaced.count).toBe(2); + }); + + test("a call with nothing to distinguish it is not counted", () => { + // Scrolling is the only governed call that names nothing. Counting it by tool name alone would + // mean a Bot reading a long page trips a rule about repetition just by reaching the bottom. + const detector = createRepeatDetector({ now: clock().now }); + + for (let attempt = 0; attempt < 30; attempt++) { + const seen = detector.observe("bot", { tool: "computer_scroll" }); + expect(seen.count).toBe(1); + expect(seen.fingerprint).toBeNull(); + expect(seen.threshold).toBeNull(); + } + }); + + test("the window expires, and the count starts again", () => { + const time = clock(); + const detector = createRepeatDetector({ + now: time.now, + windowMs: 60_000, + }); + const call = { + tool: "computer_navigate", + targetUrl: "https://example.com", + }; + + detector.observe("bot", call); + time.advance(30_000); + expect(detector.observe("bot", call).count).toBe(2); + + // Far enough that the first two fall out of the window entirely. + time.advance(61_000); + expect(detector.observe("bot", call).count).toBe(1); + }); + + test("the window slides rather than resetting on a fixed tick", () => { + // A Bot pacing itself just inside the window still accumulates, which is the point: the window + // is about how close together attempts are, not about which minute they landed in. + const time = clock(); + const detector = createRepeatDetector({ now: time.now, windowMs: 60_000 }); + const call = { tool: "computer_click", ref: "e9" }; + + detector.observe("bot", call); + time.advance(50_000); + detector.observe("bot", call); + time.advance(50_000); + + // The first has aged out, the second has not. + expect(detector.observe("bot", call).count).toBe(2); + }); + + test("each threshold fires exactly once", () => { + const detector = createRepeatDetector({ + now: clock().now, + thresholds: [3, 10], + }); + const call = { tool: "computer_click", ref: "e9" }; + + const fired: number[] = []; + for (let attempt = 0; attempt < 15; attempt++) { + const seen = detector.observe("bot", call); + if (seen.threshold !== null) fired.push(seen.threshold); + } + + // A row per attempt past the line would bury the attempts themselves under the observation that + // they kept happening. + expect(fired).toEqual([3, 10]); + }); + + test("a threshold fires on the attempt that reaches it", () => { + const detector = createRepeatDetector({ + now: clock().now, + thresholds: [3], + }); + const call = { tool: "computer_click", ref: "e9" }; + + expect(detector.observe("bot", call).threshold).toBeNull(); + expect(detector.observe("bot", call).threshold).toBeNull(); + expect(detector.observe("bot", call)).toMatchObject({ + count: 3, + threshold: 3, + }); + }); + + test("a Bot that gets stuck twice is reported twice", () => { + // Once the window has emptied the run of repetition is over, so the next one is a new incident + // and deserves its own row. Holding the thresholds for the life of the process would leave one + // row for an afternoon of separate failures. + const time = clock(); + const detector = createRepeatDetector({ + now: time.now, + windowMs: 60_000, + thresholds: [3], + }); + const call = { tool: "computer_click", ref: "e9" }; + + detector.observe("bot", call); + detector.observe("bot", call); + expect(detector.observe("bot", call).threshold).toBe(3); + + time.advance(120_000); + detector.observe("bot", call); + detector.observe("bot", call); + expect(detector.observe("bot", call).threshold).toBe(3); + }); + + test("a run that dips without emptying is still the same run", () => { + // The run ends when the window is empty, not when the count falls back under a threshold. A row + // every time a stuck Bot's count wobbles past the line would be a row per attempt under another + // name, which is the thing the thresholds exist to avoid. + const time = clock(); + const detector = createRepeatDetector({ + now: time.now, + windowMs: 60_000, + thresholds: [2], + }); + const call = { tool: "computer_click", ref: "e9" }; + + detector.observe("bot", call); + time.advance(10_000); + expect(detector.observe("bot", call).threshold).toBe(2); + + // The first attempt ages out and the second does not, so the count falls to one and climbs + // straight back. The key never went quiet, so this is the same incident, already reported. + time.advance(55_000); + expect(detector.observe("bot", call)).toMatchObject({ + count: 2, + threshold: null, + }); + }); + + test("two Bots do not share a count", () => { + // The audit row names a Bot. A count pooled across Bots would report one Bot's loop against + // another Bot's name, which is the one thing a trail may never do. + const detector = createRepeatDetector({ now: clock().now }); + const call = { tool: "computer_click", ref: "e9" }; + + detector.observe("sales-bot", call); + detector.observe("sales-bot", call); + detector.observe("sales-bot", call); + + expect(detector.observe("research-bot", call).count).toBe(1); + expect(detector.observe("sales-bot", call).count).toBe(4); + }); + + test("the number of calls held per Bot is capped", () => { + // A Bot doing genuinely varied work never repeats anything, so every call it makes is a new key. + // Without a cap the map grows for exactly the Bots this has nothing to say about. + const detector = createRepeatDetector({ + now: clock().now, + maxKeysPerBot: 3, + }); + + detector.observe("bot", { tool: "computer_click", ref: "e1" }); + detector.observe("bot", { tool: "computer_click", ref: "e2" }); + detector.observe("bot", { tool: "computer_click", ref: "e3" }); + + // Full, so the fourth call is not counted. Nothing still inside the window is thrown out to make + // room for it, and a call nobody could count reports one, which no rule acts on. + expect( + detector.observe("bot", { tool: "computer_click", ref: "e4" }).count, + ).toBe(1); + expect( + detector.observe("bot", { tool: "computer_click", ref: "e4" }).count, + ).toBe(1); + // The half of the cap that matters: a live loop survives a Bot doing other things in between, + // however much of it there is. + expect( + detector.observe("bot", { tool: "computer_click", ref: "e1" }).count, + ).toBe(2); + }); + + test("a Bot going round a loop wider than the cap is still counted", () => { + // The failure that would make the whole feature ornamental. Dropping the least recently seen key + // to make room drops each key exactly one step before it comes round again, so a Bot circling + // all afternoon would report every call as its first and no rule about repetition would ever + // fire on the one thing this exists to catch. + const detector = createRepeatDetector({ + now: clock().now, + maxKeysPerBot: 3, + thresholds: [3], + }); + + const counts: number[] = []; + const fired: number[] = []; + for (let round = 0; round < 3; round++) { + for (const ref of ["e1", "e2", "e3", "e4"]) { + const seen = detector.observe("bot", { tool: "computer_click", ref }); + counts.push(seen.count); + if (seen.threshold !== null) fired.push(seen.threshold); + } + } + + // Three of the four keys fitted, and three times round is what the trail is told about. + expect(Math.max(...counts)).toBe(3); + expect(fired).toEqual([3, 3, 3]); + }); + + test("a call the cap turned away is counted once the window drains", () => { + // The cost of not evicting is a blind spot, and this is the thing that makes it bearable: it + // ends by itself within a window rather than lasting as long as the Bot does. + const time = clock(); + const detector = createRepeatDetector({ + now: time.now, + windowMs: 60_000, + maxKeysPerBot: 2, + }); + + detector.observe("bot", { tool: "computer_click", ref: "e1" }); + detector.observe("bot", { tool: "computer_click", ref: "e2" }); + expect( + detector.observe("bot", { tool: "computer_click", ref: "e3" }).count, + ).toBe(1); + + time.advance(61_000); + detector.observe("bot", { tool: "computer_click", ref: "e3" }); + expect( + detector.observe("bot", { tool: "computer_click", ref: "e3" }).count, + ).toBe(2); + }); + + test("the number of Bots held is capped", () => { + // The id counted against is the one in the request path, checked against a session and against + // nothing else. A caller naming a fresh Bot on every request would otherwise buy a map of its + // own each time, for the life of the process. + const detector = createRepeatDetector({ now: clock().now, maxBots: 2 }); + const call = { tool: "computer_click", ref: "e9" }; + + detector.observe("sales-bot", call); + detector.observe("research-bot", call); + for (let invented = 0; invented < 100; invented++) { + detector.observe(`bot-${invented}`, call); + } + + // The two that were really working keep their counts, and the hundred invented ones bought + // nothing at all. + expect(detector.observe("sales-bot", call).count).toBe(2); + expect(detector.observe("research-bot", call).count).toBe(2); + }); + + test("a Bot that has gone quiet gives its place up", () => { + const time = clock(); + const detector = createRepeatDetector({ + now: time.now, + windowMs: 60_000, + maxBots: 1, + }); + const call = { tool: "computer_click", ref: "e9" }; + + detector.observe("morning-bot", call); + // Still working, so it keeps its place and the second Bot is not counted. + expect(detector.observe("afternoon-bot", call).count).toBe(1); + + time.advance(61_000); + detector.observe("afternoon-bot", call); + expect(detector.observe("afternoon-bot", call).count).toBe(2); + }); + + test("one Bot's varied work does not evict another Bot's loop", () => { + const detector = createRepeatDetector({ + now: clock().now, + maxKeysPerBot: 2, + }); + const call = { tool: "computer_click", ref: "e9" }; + + detector.observe("stuck-bot", call); + for (let ref = 0; ref < 20; ref++) { + detector.observe("busy-bot", { tool: "computer_click", ref: `e${ref}` }); + } + + expect(detector.observe("stuck-bot", call).count).toBe(2); + }); + + test("the default window is a few minutes, not a few seconds", () => { + // A retry loop is a model round trip each time round. A window of seconds would count nothing at + // all, and the feature would look like it worked because it never fired. + expect(DEFAULT_REPEAT_WINDOW_MS).toBeGreaterThanOrEqual(60_000); + expect(DEFAULT_REPEAT_WINDOW_MS).toBeLessThanOrEqual(10 * 60_000); + }); +}); + +describe("the fingerprint an audit row carries", () => { + test("names the tool and the argument, in words", () => { + // It goes onto the row as written. An investigator reading "the same call, 25 times" has to be + // told which call without going and decoding anything. + expect(fingerprintOf({ tool: "computer_click", ref: "e9" })).toBe( + "computer_click ref=e9", + ); + expect( + fingerprintOf({ tool: "computer_write_file", filePath: "notes.md" }), + ).toBe("computer_write_file file=notes.md"); + expect( + fingerprintOf({ tool: "computer_key", ref: "e1", key: "Enter" }), + ).toBe("computer_key ref=e1 key=Enter"); + }); + + test("is null when the call named nothing", () => { + expect(fingerprintOf({ tool: "computer_scroll" })).toBeNull(); + }); +}); diff --git a/server/tests/config.test.ts b/server/tests/config.test.ts index 722a5c2f1..9e7b31261 100644 --- a/server/tests/config.test.ts +++ b/server/tests/config.test.ts @@ -153,4 +153,37 @@ describe("deployment configuration", () => { }), ).toThrow("Google authentication requires BETTER_AUTH_SECRET"); }); + + test("takes a widened repetition window, and leaves it absent when nobody set one", () => { + expect( + loadConfig({ + ...baseEnvironment, + AGENT_COMPUTER_URL: "http://localhost:4100", + COMPUTER_REPEAT_WINDOW_MS: "600000", + }).computer?.repeatWindowMs, + ).toBe(600_000); + + expect( + loadConfig({ + ...baseEnvironment, + AGENT_COMPUTER_URL: "http://localhost:4100", + }).computer?.repeatWindowMs, + ).toBeUndefined(); + }); + + // Refused rather than quietly defaulted, like a malformed policy. An operator who typed `3m` would + // otherwise get a deployment running the built-in window, and the only evidence would be a rule + // about repetition that never fires, which reads exactly like a Bot behaving itself. + test.each(["3m", "0", "-1", "180000.5"])( + "refuses to start on a repetition window of %p", + (value) => { + expect(() => + loadConfig({ + ...baseEnvironment, + AGENT_COMPUTER_URL: "http://localhost:4100", + COMPUTER_REPEAT_WINDOW_MS: value, + }), + ).toThrow("COMPUTER_REPEAT_WINDOW_MS"); + }, + ); });