From 4566e27f6d4e5ce6c1dd1345f36f79c56ae1ecbf Mon Sep 17 00:00:00 2001 From: hotragn Date: Mon, 14 Sep 2026 03:11:26 -0400 Subject: [PATCH] Let a boundary rule ask what started a run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A routine's turn goes through exactly the path a person's chat turn does, as the routine's owner: their grants, their connections, their thread. That is the right design, and it is why `actor.id` cannot tell a scheduled run at three in the morning from the same person typing. The trail already draws that distinction. `AuditInitiator` is signed into the run assertion and written onto the row, with the docstring "what caused a row, where `actorUserId` is only whose authority it borrowed". The boundary could not ask the same question, so this was unwritable: deny: initiator.kind == "routine" && intent == "run_command" `PolicyContext` now carries `initiator`, the kind and id the trail already records. The id is there too, so a deployment can name one routine rather than deciding about scheduled runs as a class, and `handoff` is its own kind. Required on the type rather than optional, and flattened to two always-present strings. cel-js throws on an unbound identifier and a throw fails closed, so a field that were sometimes absent would turn one rule about routines into a deployment that refused every ordinary click — the failure #115 exists to prevent. `id` is "" for the kinds that carry none, the neutral `mcp.effect` already uses. Set at all three places a context is built. `plugins/store.ts` is where it is real: a routine's tools run through there carrying the initiator its assertion was signed with. The computer gateway passes a person, and that is truthful rather than convenient — a Bot's computer is driven by frontend tools in the browser, so every action reaching that gateway came from somebody's session. #298 is the change that makes that untrue, and the comment says so. The history replay reads the initiator off the row when it is there. Nothing is refused that was not refused before. Ten tests: a routine refused by a rule naming it while the identical action with a person is allowed, one routine named without catching another, a rule naming the initiator leaving an ordinary click alone, and a replayed row with no initiator or an unrecognised one reading as a person. 52 pass in computer-policy, 13 in policy-dry-run, and computer-gateway's 60 unchanged. Answers the half of #484 that survived — the rest of that issue was my own misreading, corrected there. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 30 +++++++ server/src/computer/gateway.ts | 8 ++ server/src/computer/policy-dry-run.ts | 27 ++++++- server/src/computer/policy.ts | 40 ++++++++++ server/src/plugins/store.ts | 8 ++ server/tests/computer-policy.test.ts | 111 ++++++++++++++++++++++++++ server/tests/policy-dry-run.test.ts | 36 +++++++++ 7 files changed, 259 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f9131e34e..97487c1e5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,36 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged. ## Unreleased +### A boundary rule can ask what started a run, not only whose authority it carries + +A routine's turn goes through exactly the path a person's chat turn does, as the routine's owner: +their grants, their connections, their thread. That is the right design, and it is also why +`actor.id` cannot tell a scheduled run at three in the morning from the same person typing. The +trail already drew that distinction — `AuditInitiator` is signed into the run assertion and written +onto the row, so an investigator can see a routine caused something. A rule could not ask the same +question. + +The policy context now carries `initiator`, with the kind and id the trail already records, so this +is writable: + +``` +deny: initiator.kind == "routine" && intent == "run_command" +``` + +A deployment happy for a Bot to run a shell while somebody watches, and not happy for it to do so +unattended, can now say so. The id is there too, so a single routine can be named rather than +scheduled runs as a class. `handoff` is its own kind, for a Bot that hands work to another Bot. + +**Nothing is refused that was not refused before.** The field is neutral — `{kind: "person", id: ""}` +— everywhere a person is driving, which is every path that does not carry an initiator today, +including every action on a Bot'"'"'s computer: those are driven by the browser, so they really are +somebody'"'"'s session. It is required rather than optional for the reason #115 exists: cel-js throws on +an unbound identifier and a throw fails closed, so a field that were sometimes absent would turn one +rule about routines into a deployment that refused every ordinary click. + +Replaying a rule against history reads the initiator off the row when it is there and treats a row +that predates the field, or carries a shape this version does not recognise, as a person. + ## 0.0.10 ### The LangGraph Bot says a refused tool call was refused, not that it found nothing diff --git a/server/src/computer/gateway.ts b/server/src/computer/gateway.ts index ffe66ab2d..1d1226079 100644 --- a/server/src/computer/gateway.ts +++ b/server/src/computer/gateway.ts @@ -41,6 +41,7 @@ import { type ActionPolicy, evaluateActionPolicy, type PolicyContext, + policyInitiator, type PolicyDecision, } from "./policy"; import type { ComputerProvider } from "./provider"; @@ -513,6 +514,13 @@ export function createComputerGateway( ? describeFile(filePath) : { path: "", name: "", extension: "" }, command: subject.command ?? "", + /* + * A person, and truthfully so today: a Bot's computer is driven by frontend tools in the + * browser, so every action arriving here came from somebody's session rather than from a + * schedule. #298 is the change that would make that untrue, and it is the one that has to pass + * the run's own initiator through instead of inheriting this. + */ + initiator: policyInitiator(), // Neutral, like the fields above: this is not an MCP call, but a `deny: mcp.effect == "write"` // names `mcp`, and cel-js throws on an unbound identifier — which fails closed and would refuse // every browser action the moment an operator wrote a rule about their tools. Empty server and diff --git a/server/src/computer/policy-dry-run.ts b/server/src/computer/policy-dry-run.ts index 46f3347cc..8249a5f07 100644 --- a/server/src/computer/policy-dry-run.ts +++ b/server/src/computer/policy-dry-run.ts @@ -12,12 +12,13 @@ * behaves one way here and another way live would make this feature worse than absent. */ -import type { AuditEvent } from "../audit"; +import type { AuditEvent, AuditInitiator } from "../audit"; import { describeFile, hostOf, intentOf } from "./gateway"; import { type ActionPolicy, evaluateActionPolicy, type PolicyContext, + policyInitiator, } from "./policy"; /** @@ -77,6 +78,23 @@ const CHANGES_CAP = 50; * Null when the row does not carry enough to replay — a row from before a field existed, or a * hand-inserted one. Skipped rather than guessed at. */ +/** + * The initiator an audit payload carries, narrowed back to the union. + * + * Its own reader rather than the one in `callback-token.ts`: that one narrows a value this + * deployment signed and can trust the shape of, and this one reads a stored row that may predate the + * field, have been written by an older version, or been inserted by hand. Anything it does not + * recognise reads as absent, and `policyInitiator` turns that into a person. + */ +function initiatorFromPayload(value: unknown): AuditInitiator | undefined { + if (!value || typeof value !== "object") return undefined; + const kind = (value as { kind?: unknown }).kind; + if (kind === "person" || kind === "deployment") return { kind }; + if (kind !== "routine" && kind !== "handoff") return undefined; + const id = (value as { id?: unknown }).id; + return typeof id === "string" && id ? { kind, id } : undefined; +} + export function contextFromAuditPayload( payload: Record, ): PolicyContext | null { @@ -116,6 +134,13 @@ export function contextFromAuditPayload( file: file ? describeFile(file) : { path: "", name: "", extension: "" }, command: text(payload.command), mcp: { server: "", tool: "", effect: "" }, + /* + * Read off the row when it is there, neutral when it is not. The rows this replays are computer + * actions, which carry no initiator today, so in practice this is a person — but reading it + * rather than hardcoding it means a replay stays honest the day those rows do carry one, and a + * rule being tested against history is judged on what actually happened. + */ + initiator: policyInitiator(initiatorFromPayload(payload.initiator)), }; } diff --git a/server/src/computer/policy.ts b/server/src/computer/policy.ts index 1766ecc47..0318de503 100644 --- a/server/src/computer/policy.ts +++ b/server/src/computer/policy.ts @@ -15,6 +15,7 @@ * defeated by a broader rule that grants it, or a company cannot reason about what it has forbidden. */ import { evaluate } from "cel-js"; +import type { AuditInitiator, AuditInitiatorKind } from "../audit"; export type PolicyMode = "dry-run" | "enforce"; @@ -150,8 +151,47 @@ export type PolicyContext = { * and no list catches them all. The boundary is the container the command runs in. */ command?: string; + /** + * What caused this run, as distinct from whose authority it carries. + * + * `actor.id` answers "whose grants and connections is this spending", and for a routine that is + * its owner — asleep, at three in the morning, with the run going through exactly the path their + * own chat turn takes. That is the right design and it is also why `actor` cannot answer "was + * anybody there". The trail already draws the distinction: `AuditInitiator` is signed into the run + * assertion and written onto the row, with the docstring "what caused a row, where `actorUserId` + * is only whose authority it borrowed". A rule could not ask the same question. + * + * So `deny: initiator.kind == "routine" && intent == "run_command"` is now writable — a deployment + * that is happy for a Bot to run a shell while somebody watches, and not happy for it to do so + * unattended, can say so. + * + * REQUIRED, not optional, and flattened to two always-present strings. cel-js throws on an + * unbound identifier and a throw fails closed, so a rule naming this field would have refused + * every action built by a call site that forgot it — the failure #115 exists to prevent. `id` is + * `""` for `person` and `deployment`, which carry none, the same neutral `mcp.effect` uses. + */ + initiator: { kind: AuditInitiatorKind; id: string }; }; +/** + * The initiator as the policy sees it, defaulting to a person. + * + * A person is the honest default rather than a convenient one: every path that does not carry an + * initiator today is one a person drove. The computer gateway is the case worth naming — a Bot's + * computer is driven by frontend tools in the browser (`app/src/lib/copilot/computer-tools.tsx`), so + * every action reaching that gateway came from somebody's session. When that stops being true, the + * call site has to say so rather than inherit this. + */ +export function policyInitiator( + initiator?: AuditInitiator, +): PolicyContext["initiator"] { + if (!initiator) return { kind: "person", id: "" }; + return { + kind: initiator.kind, + id: "id" in initiator ? initiator.id : "", + }; +} + export type PolicyDecision = { allowed: boolean; mode: PolicyMode; diff --git a/server/src/plugins/store.ts b/server/src/plugins/store.ts index 63d611f34..852d82c90 100644 --- a/server/src/plugins/store.ts +++ b/server/src/plugins/store.ts @@ -8,6 +8,7 @@ import { type ActionPolicy, evaluateActionPolicy, type PolicyContext, + policyInitiator, } from "../computer/policy"; import { type CredentialExecutor, @@ -2954,6 +2955,13 @@ export function createPluginStore(options: PluginStoreOptions) { command: "", intent: effect === "write" ? "write_tool" : "read_tool", mcp: { server: serverId, tool: toolName, effect }, + /* + * The real one, and this is the path where it is not neutral. A routine's turn reaches its + * tools through here, carrying the initiator its run assertion was signed with, so this is + * where `initiator.kind == "routine"` becomes a rule a deployment can actually write. A + * chat turn arrives with none and reads as a person. + */ + initiator: policyInitiator(input.initiator), }; const verdict = evaluateActionPolicy(options.policy(), context); diff --git a/server/tests/computer-policy.test.ts b/server/tests/computer-policy.test.ts index 63f886c99..27ab58570 100644 --- a/server/tests/computer-policy.test.ts +++ b/server/tests/computer-policy.test.ts @@ -2,6 +2,7 @@ import { describe, expect, test } from "bun:test"; import { type ActionPolicy, evaluateActionPolicy, + policyInitiator, type PolicyContext, } from "../src/computer/policy"; import { parseActionPolicy } from "../src/computer/policy-store"; @@ -22,6 +23,8 @@ function context(overrides: Partial = {}): PolicyContext { actor: { id: "dev-local-user" }, page: { url: "https://example.com/order", host: "example.com" }, element: { ref: "e13", role: "button", name: "Submit order" }, + // A person, which is what every context the gateway builds carries today. + initiator: { kind: "person", id: "" }, ...overrides, }; } @@ -672,3 +675,111 @@ describe("refusal wording under the context the gateway actually builds", () => expect(decision.reason).toContain("search_notes on notes"); }); }); + +/** + * What started the run, as a thing a rule can ask about. + * + * `actor` says whose grants a run is spending, and for a routine that is its owner — which is + * correct and is also why it cannot answer "was anybody watching". The trail already separates the + * two through `AuditInitiator`; these pin that the boundary can now separate them too. + */ +describe("a rule about what started the run", () => { + const unattended = context({ + tool: { name: "mcp__jira__createJiraIssue" }, + intent: "write_tool", + initiator: { kind: "routine", id: "nightly-summary" }, + }); + + test("a routine is refused by a rule naming it, and a person is not", () => { + const policy: ActionPolicy = { + mode: "enforce", + deny: ['initiator.kind == "routine" && intent == "write_tool"'], + allow: ["true"], + }; + + expect(evaluateActionPolicy(policy, unattended).allowed).toBe(false); + + // The same action, with somebody in front of it. This is the whole point of the field: the rule + // separates when it happened from whose authority it carried, and `actor` is identical in both. + const watched = context({ + tool: { name: "mcp__jira__createJiraIssue" }, + intent: "write_tool", + initiator: { kind: "person", id: "" }, + }); + expect(evaluateActionPolicy(policy, watched).allowed).toBe(true); + }); + + test("one routine can be named without catching the others", () => { + // The id is on the context, so a deployment can exempt or target a single routine rather than + // being forced to decide about scheduled runs as a class. + const policy: ActionPolicy = { + mode: "enforce", + deny: ['initiator.id == "nightly-summary"'], + allow: ["true"], + }; + + expect(evaluateActionPolicy(policy, unattended).allowed).toBe(false); + expect( + evaluateActionPolicy( + policy, + context({ initiator: { kind: "routine", id: "weekly-digest" } }), + ).allowed, + ).toBe(true); + }); + + test("a rule naming the initiator does not refuse an action that has a person", () => { + /* + * The #115 property, for this field. cel-js throws on an unbound identifier and a throw fails + * closed, so a field that were optional-and-sometimes-absent would turn one rule about routines + * into a deployment that refuses every ordinary click. It is required on the type and neutral + * everywhere precisely so this stays true. + */ + const policy: ActionPolicy = { + mode: "enforce", + deny: ['initiator.kind == "routine"'], + allow: ["true"], + }; + + const decision = evaluateActionPolicy(policy, context()); + expect(decision.allowed).toBe(true); + expect(decision.source).toBe("allow"); + }); + + test("a handoff is its own kind, not a person and not a routine", () => { + // A Bot handing work to another Bot has somebody behind it somewhere and nobody watching that + // run. Naming it separately is what lets a deployment decide about the two differently. + const policy: ActionPolicy = { + mode: "enforce", + deny: ['initiator.kind == "handoff"'], + allow: ["true"], + }; + + expect( + evaluateActionPolicy( + policy, + context({ initiator: { kind: "handoff", id: "risk-analyst" } }), + ).allowed, + ).toBe(false); + expect(evaluateActionPolicy(policy, unattended).allowed).toBe(true); + }); +}); + +describe("policyInitiator", () => { + test("nothing is a person, because every path without one is driven by somebody", () => { + expect(policyInitiator()).toEqual({ kind: "person", id: "" }); + }); + + test("a kind that carries no id still gets one, so a rule naming id cannot throw", () => { + expect(policyInitiator({ kind: "deployment" })).toEqual({ + kind: "deployment", + id: "", + }); + }); + + test("a routine keeps its id", () => { + expect(policyInitiator({ kind: "routine", id: "nightly" })).toEqual({ + kind: "routine", + id: "nightly", + }); + }); +}); diff --git a/server/tests/policy-dry-run.test.ts b/server/tests/policy-dry-run.test.ts index e5933733d..42baff5d5 100644 --- a/server/tests/policy-dry-run.test.ts +++ b/server/tests/policy-dry-run.test.ts @@ -41,6 +41,42 @@ const CLICK_SUBMIT = { element: { role: "button", name: "Submit order" }, }; +describe("the initiator a replayed row carries", () => { + test("a row with no initiator replays as a person, not as an unbound identifier", () => { + // Every computer row written before this field existed. A rule naming the initiator has to judge + // them rather than throw, which fails closed and would report a boundary far stricter than the + // one being tested. + expect(contextFromAuditPayload(CLICK_SUBMIT)?.initiator).toEqual({ + kind: "person", + id: "", + }); + }); + + test("a row that names a routine replays as that routine", () => { + expect( + contextFromAuditPayload({ + ...CLICK_SUBMIT, + initiator: { kind: "routine", id: "nightly-summary" }, + })?.initiator, + ).toEqual({ kind: "routine", id: "nightly-summary" }); + }); + + test("a shape this version does not recognise reads as a person rather than as itself", () => { + // A row from a later version, or one inserted by hand. Guessing at it would replay a rule + // against a kind no branch here knows, so it is read as absent and neutralised. + for (const initiator of [ + { kind: "wat", id: "x" }, + { kind: "routine" }, + "routine", + null, + ]) { + expect( + contextFromAuditPayload({ ...CLICK_SUBMIT, initiator })?.initiator, + ).toEqual({ kind: "person", id: "" }); + } + }); +}); + describe("contextFromAuditPayload", () => { test("rebuilds the element with the ref that is stored beside it", () => { const context = contextFromAuditPayload(CLICK_SUBMIT);