Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,35 @@ 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.
### The Python LangGraph Bot tells its model why the deployment refused a tool call

When the deployment would not run a tool call from the Python LangGraph Bot — a token it no longer
Expand Down
8 changes: 8 additions & 0 deletions server/src/computer/gateway.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import {
type ActionPolicy,
evaluateActionPolicy,
type PolicyContext,
policyInitiator,
type PolicyDecision,
} from "./policy";
import type { ComputerProvider } from "./provider";
Expand Down Expand Up @@ -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
Expand Down
27 changes: 26 additions & 1 deletion server/src/computer/policy-dry-run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";

/**
Expand Down Expand Up @@ -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<string, unknown>,
): PolicyContext | null {
Expand Down Expand Up @@ -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)),
};
}

Expand Down
40 changes: 40 additions & 0 deletions server/src/computer/policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -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;
Expand Down
8 changes: 8 additions & 0 deletions server/src/plugins/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
type ActionPolicy,
evaluateActionPolicy,
type PolicyContext,
policyInitiator,
} from "../computer/policy";
import {
type CredentialExecutor,
Expand Down Expand Up @@ -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);
Expand Down
111 changes: 111 additions & 0 deletions server/tests/computer-policy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -22,6 +23,8 @@ function context(overrides: Partial<PolicyContext> = {}): 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,
};
}
Expand Down Expand Up @@ -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",
});
});
});
36 changes: 36 additions & 0 deletions server/tests/policy-dry-run.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down