From 415e71a7bb73caff0910c1203c18b0dd83149efc Mon Sep 17 00:00:00 2001 From: Maximo Guk <62088388+Maximo-Guk@users.noreply.github.com> Date: Sun, 23 Aug 2026 19:22:52 -0500 Subject: [PATCH 01/12] Allow writes-to-self in restricted mode, never auto-approved The restricted-data latch used to block every action once any sensitive (containsRestrictedData) observation occurred. Carve out writes-to-self: a latched workspace may still submit actions targeting the connections that produced its restricted data -- sending the data back where it came from reveals nothing new to that system -- while any other target, and all web fetches, stay blocked. Provenance is derived, not persisted: restrictedProducerIds() already scans the never-deleted action log for restricted observations (under either of the flag's names), with the invariant that the latch and the producing record are written in one synchronous block, so a latched workspace always yields a non-empty set. That makes the earlier design's persisted producer-set singleton and its backfill migration unnecessary -- the same scan IS the backfill, run on demand. If the set is ever empty under the latch anyway, every target fails the membership check and all actions are refused (conservative fallback). Latched actions are never auto-approved: the human approving each write is the interim mitigation for hidden-instruction injection riding in the restricted data. The auto-approval gate, previously duplicated between submitAction and the drainer, is factored into one exported predicate autoApprovalRule() -- author verdict + user rule + not latched. setAutoApprovedActionKind refuses to store a rule while latched (it would never fire) and listPreApprovableActions offers nothing. Accepted residual: with producers {A, B}, data observed from A may be written back to B -- the carve-out is set-based, not per-producer. Every such write is human-approved, and gatekeeper.ts's TODO(someday) already flags the restricted mode's bluntness. Co-Authored-By: Claude Fable 5 --- docs/observers.md | 6 +- .../__tests__/auto-approval.test.ts | 78 +++++++++- .../restricted-writes-to-self.test.ts | 145 ++++++++++++++++++ .../workshop-backend/src/auto-approval.ts | 40 ++++- packages/workshop-backend/src/overseer.ts | 47 ++++-- packages/workshop-shared/src/api.ts | 5 +- packages/workshop-shared/src/gatekeeper.ts | 7 +- 7 files changed, 301 insertions(+), 27 deletions(-) create mode 100644 packages/workshop-backend/__tests__/restricted-writes-to-self.test.ts diff --git a/docs/observers.md b/docs/observers.md index 54683f2f9..24c3114c8 100644 --- a/docs/observers.md +++ b/docs/observers.md @@ -34,7 +34,8 @@ The mechanism is a per-user, gatekeeper-mediated check — "this data may be sha people who *also* have access to it". (Maximally sensitive data gets an extra layer: an observation marked **`containsRestrictedData`** (`ObservationDescription.containsRestrictedData` in `packages/workshop-shared/src/gatekeeper.ts`) -latches the workspace into a restricted mode — no actions, no web fetches — and is admitted only +latches the workspace into a restricted mode — no web fetches, and actions only back to the +connections that produced the restricted data, each manually approved — and is admitted only if every current collaborator has been verified against the gatekeeper producing it; see the coverage guard, `#assertSensitiveObservationCoverage`, in `overseer.ts` and edge case 4 below.) @@ -441,7 +442,8 @@ already in the JSDoc in `gatekeeper.ts`; add anything missing there rather than on any collaborator regardless of role). At open() time, `ensureObserver` re-verifies each collaborator against every in-scope gatekeeper, which is what admits (or refuses) them for sensitive data. The flag also latches the workspace into a restricted mode that blocks - actions and web fetches. + web fetches and limits actions to the connections that produced the restricted data, each + requiring manual approval. `use` scope is *live* binding state, with a transition case in each direction. Adding a binding grows it, and edge case 5 covers the interim. Unbinding shrinks it with no guard: a formerly-bound producer drops out of `use` verification scope, so its sensitive reads diff --git a/packages/workshop-backend/__tests__/auto-approval.test.ts b/packages/workshop-backend/__tests__/auto-approval.test.ts index 0b4ce7389..1c37bb076 100644 --- a/packages/workshop-backend/__tests__/auto-approval.test.ts +++ b/packages/workshop-backend/__tests__/auto-approval.test.ts @@ -1,12 +1,18 @@ import { describe, it, expect } from "vitest"; import { createTypedStorage, collection } from "@gadgets/typed-storage"; -import { AutoApprovalDrainer, AutoApprovalStorage, ApplyPendingActionFn } from "../src/auto-approval.js"; +import { + AutoApprovalDrainer, AutoApprovalStorage, ApplyPendingActionFn, autoApprovalRule, +} from "../src/auto-approval.js"; import type { ActionRecord, AutoApproveTagRecord } from "../src/overseer.js"; import type { AiChatAuthorInfo } from "@gadgets/workshop-shared/api"; +import type { ActionDescription } from "@gadgets/workshop-shared/gatekeeper"; import { makeMockStorage } from "./mock-storage.js"; function makeStorage(): AutoApprovalStorage { return createTypedStorage(makeMockStorage(), { + singletons: { + prohibitAllSharing: false, + }, collections: { actions: collection()({ primaryKey: "id" }), autoApproveTags: collection()({ @@ -211,4 +217,74 @@ describe("AutoApprovalDrainer.drain", () => { expect(getAction(storage, 1).state).toBe("approved"); expect(getAction(storage, 2).state).toBe("approved"); }); + + it("applies nothing while the workspace is latched, even with a matching rule", async () => { + let storage = makeStorage(); + enableRule(storage); + putAction(storage, 1); + storage.prohibitAllSharing.put(true); + + let apply = makeImmediateApply(storage); + await new AutoApprovalDrainer(storage, apply.applyFn).drain(GK); + + expect(apply.calls).toEqual([]); + expect(getAction(storage, 1).state).toBe("pending"); + }); +}); + +// A description that passes every gate, for the predicate tests to knock single gates out of. +function eligibleDescription(overrides: Partial = {}): ActionDescription { + return { + title: "Edit the thing", + description: "Edits the thing.", + implementsRevert: true, + actionKind: { tag: "edit", label: "Edits" }, + autoApprovable: true, + ...overrides, + }; +} + +describe("autoApprovalRule", () => { + it("returns the enabling rule when every gate passes", () => { + let storage = makeStorage(); + enableRule(storage); + let rule = autoApprovalRule(storage, GK, eligibleDescription()); + expect(rule?.enabledBy).toEqual(ENABLER); + }); + + it("requires the author's autoApprovable verdict", () => { + let storage = makeStorage(); + enableRule(storage); + expect(autoApprovalRule(storage, GK, eligibleDescription({ autoApprovable: undefined }))) + .toBeUndefined(); + expect(autoApprovalRule(storage, GK, eligibleDescription({ autoApprovable: false }))) + .toBeUndefined(); + }); + + it("requires an actionKind", () => { + let storage = makeStorage(); + enableRule(storage); + expect(autoApprovalRule(storage, GK, eligibleDescription({ actionKind: undefined }))) + .toBeUndefined(); + }); + + it("requires a user-enabled rule for the kind on this gatekeeper", () => { + let storage = makeStorage(); + expect(autoApprovalRule(storage, GK, eligibleDescription())).toBeUndefined(); + enableRule(storage, "edit", GK + 1); // right tag, wrong gatekeeper + expect(autoApprovalRule(storage, GK, eligibleDescription())).toBeUndefined(); + enableRule(storage, "delete", GK); // right gatekeeper, wrong tag + expect(autoApprovalRule(storage, GK, eligibleDescription())).toBeUndefined(); + }); + + it("refuses every action while the restricted-data latch is set", () => { + let storage = makeStorage(); + enableRule(storage); + enableRule(storage, "edit", GK + 1); + storage.prohibitAllSharing.put(true); + // The latch is workspace-wide: even a write-to-self (which submitAction lets pend) must be + // read by a human, on every gatekeeper. + expect(autoApprovalRule(storage, GK, eligibleDescription())).toBeUndefined(); + expect(autoApprovalRule(storage, GK + 1, eligibleDescription())).toBeUndefined(); + }); }); diff --git a/packages/workshop-backend/__tests__/restricted-writes-to-self.test.ts b/packages/workshop-backend/__tests__/restricted-writes-to-self.test.ts new file mode 100644 index 000000000..ad3a9ea60 --- /dev/null +++ b/packages/workshop-backend/__tests__/restricted-writes-to-self.test.ts @@ -0,0 +1,145 @@ +// submitAction's writes-to-self carve-out: a latched workspace may still submit actions targeting +// the connections that produced its restricted data (sending the data back where it came from +// reveals nothing new), while any other target is refused before a record is written. Latched +// actions are never auto-approved -- even a write-to-self with a matching rule must pend for a +// human (see autoApprovalRule). +// +// Runs against a real OverseerDurableObject (the TEST_OVERSEER binding, like +// restricted-producer-removal.test.ts) so submitAction reads real storage; records are seeded +// directly through the impl. + +import { describe, expect, it } from "vitest"; +import { env } from "cloudflare:workers"; +import { runInDurableObject } from "cloudflare:test"; +import type { OverseerDurableObject } from "../src/overseer.js"; +import type { ActionDescription } from "@gadgets/workshop-shared/gatekeeper"; + +declare module "cloudflare:workers" { + interface ProvidedEnv { + TEST_OVERSEER: DurableObjectNamespace; + } +} + +const CALLER = { from: "user" } as const; + +function getImpl(instance: OverseerDurableObject): any { + return (instance as unknown as { impl: any }).impl; +} + +function seedGatekeeper(impl: any, id: number): void { + impl.storage.gatekeepers.put({ + id, + resourceTitle: `Connection ${id}`, + class: {} as any, + creationSpec: { + type: "gatekeeper", + vendorId: "testvendor", + resourceUrl: `https://example.com/${id}`, + typeUrlPattern: "https://*", + }, + }); +} + +// A restricted observation attributed to `gatekeeperId`, making it a producer +// (restrictedProducerIds scans the action log for exactly these), plus the latch the same +// authorizeObservation write would set. +function seedRestrictedObservation(impl: any, gatekeeperId: number, actionId: number): void { + impl.storage.actions.put({ + id: actionId, + gatekeeperId, + caller: CALLER, + createdAt: new Date(), + state: "approved", + type: "observation", + description: { + title: "Read a thing", + description: "The test read a thing.", + containsRestrictedData: true, + }, + }); + impl.storage.nextActionId.put(actionId + 1); + impl.storage.prohibitAllSharing.put(true); +} + +function pokeDescription(autoApprovable = false): ActionDescription { + return { + title: "Poke the thing", + description: "The test poked the thing.", + implementsRevert: false, + actionKind: { tag: "poke", label: "Pokes" }, + ...(autoApprovable ? { autoApprovable: true } : {}), + }; +} + +function actionStates(impl: any): Array<{ gatekeeperId: number; state: string }> { + return [...impl.storage.actions.list()] + .filter((rec: any) => rec.type === "action") + .map((rec: any) => ({ gatekeeperId: rec.gatekeeperId, state: rec.state })); +} + +describe("submitAction under the restricted-data latch", () => { + it("pends an unlatched action normally", async () => { + let stub = env.TEST_OVERSEER.getByName("writes-to-self-unlatched"); + await runInDurableObject(stub, async (instance: OverseerDurableObject) => { + let impl = getImpl(instance); + seedGatekeeper(impl, 1); + + await impl.submitAction(1, 0, pokeDescription(), CALLER); + expect(actionStates(impl)).toEqual([{ gatekeeperId: 1, state: "pending" }]); + }); + }); + + it("pends a latched write-to-self, and never auto-approves it", async () => { + let stub = env.TEST_OVERSEER.getByName("writes-to-self-producer"); + await runInDurableObject(stub, async (instance: OverseerDurableObject) => { + let impl = getImpl(instance); + seedGatekeeper(impl, 1); + seedRestrictedObservation(impl, 1, 100); + // A rule that would auto-approve this exact action were the workspace not latched. + impl.storage.autoApproveTags.put({ + gatekeeperId: 1, + actionKind: { tag: "poke", label: "Pokes" }, + enabledBy: { type: "user", id: "alice", name: "Alice" }, + }); + + await impl.submitAction(1, 0, pokeDescription(/* autoApprovable */ true), CALLER); + expect(actionStates(impl)).toEqual([{ gatekeeperId: 1, state: "pending" }]); + + // Not auto-approved: the submit never schedules a drain while latched, and even an explicit + // drain refuses (autoApprovalRule) -- the action stays a manual gate. + await impl.drainAutoApprovals(1); + expect(actionStates(impl)).toEqual([{ gatekeeperId: 1, state: "pending" }]); + }); + }); + + it("refuses a latched action on a non-producer, writing no record", async () => { + let stub = env.TEST_OVERSEER.getByName("writes-to-self-non-producer"); + await runInDurableObject(stub, async (instance: OverseerDurableObject) => { + let impl = getImpl(instance); + seedGatekeeper(impl, 1); + seedGatekeeper(impl, 2); + seedRestrictedObservation(impl, 1, 100); + let nextActionId = impl.storage.nextActionId.get(); + + await expect(impl.submitAction(2, 0, pokeDescription(), CALLER)) + .rejects.toThrow(/only perform actions on those same connections/i); + expect(actionStates(impl)).toEqual([]); + expect(impl.storage.nextActionId.get()).toBe(nextActionId); + }); + }); + + it("refuses everything when the latch is set with no derivable producer", async () => { + let stub = env.TEST_OVERSEER.getByName("writes-to-self-empty-producers"); + await runInDurableObject(stub, async (instance: OverseerDurableObject) => { + let impl = getImpl(instance); + seedGatekeeper(impl, 1); + // Should be impossible (the latch and its action record are written together), so fail + // closed: with no producer to match, every target is refused. + impl.storage.prohibitAllSharing.put(true); + + await expect(impl.submitAction(1, 0, pokeDescription(), CALLER)) + .rejects.toThrow(/only perform actions on those same connections/i); + expect(actionStates(impl)).toEqual([]); + }); + }); +}); diff --git a/packages/workshop-backend/src/auto-approval.ts b/packages/workshop-backend/src/auto-approval.ts index 0c1fa032f..40c1bba6b 100644 --- a/packages/workshop-backend/src/auto-approval.ts +++ b/packages/workshop-backend/src/auto-approval.ts @@ -3,8 +3,9 @@ // can't double-apply the same action. The apply is injected, keeping this constructible over a // mock storage in tests. -import type { Collection } from "@gadgets/typed-storage"; +import type { Collection, Singleton } from "@gadgets/typed-storage"; import type { AiChatAuthorInfo } from "@gadgets/workshop-shared/api"; +import type { ActionDescription } from "@gadgets/workshop-shared/gatekeeper"; import { createWorkshopLogger } from "./observability"; import type { ActionRecord, AutoApproveTagRecord } from "./overseer.js"; @@ -13,6 +14,32 @@ const logger = createWorkshopLogger("workshop.auto.approval"); export interface AutoApprovalStorage { actions: Collection; autoApproveTags: Collection; + + /** + * The restricted-data latch (see makeOverseerStorage; the key predates the flag's rename to + * `containsRestrictedData`). While set, no action is ever auto-approved: a latched workspace's + * only permissible actions are writes back to a restricted producer (submitAction's + * writes-to-self carve-out), and each of those must pend for a human. + */ + prohibitAllSharing: Singleton; +} + +/** + * The single authority on whether an action may be applied without a human. Returns the enabling + * rule iff ALL of: + * - the gatekeeper author marked this specific action `autoApprovable`, + * - the action carries an `actionKind` for which the user enabled a rule on this gatekeeper, + * - the workspace has not latched restricted mode (`prohibitAllSharing` above). + * Returns undefined otherwise: manual approval required. + */ +export function autoApprovalRule( + storage: AutoApprovalStorage, gatekeeperId: number, description: ActionDescription) + : AutoApproveTagRecord | undefined { + if (description.autoApprovable !== true) return undefined; + let tag = description.actionKind?.tag; + if (tag === undefined) return undefined; + if (storage.prohibitAllSharing.get()) return undefined; + return storage.autoApproveTags.get(`${gatekeeperId}:${tag}`); } /** @@ -55,8 +82,8 @@ export class AutoApprovalDrainer { // -- it is never skipped ahead of. This preserves in-order application and the invariant that // nothing is silently applied past a human gate. // - // Eligibility requires BOTH signals: the author's `autoApprovable` verdict on the action AND a - // user-enabled rule for the action's type on this gatekeeper. + // Eligibility is `autoApprovalRule()`: the author's `autoApprovable` verdict, a user-enabled + // rule for the action's kind, and no restricted-data latch. async #drainOnce(gatekeeperId: number): Promise { // Materialize a snapshot first: list() is a lazy generator over storage, and we mutate the // actions collection (via applyPendingAction) as we go. @@ -65,11 +92,8 @@ export class AutoApprovalDrainer { rec.gatekeeperId === gatekeeperId && rec.type === "action" && rec.state === "pending"); for (let record of pending) { - let tag = record.description.actionKind?.tag; - let rule = tag !== undefined - ? this.storage.autoApproveTags.get(`${gatekeeperId}:${tag}`) - : undefined; - if (record.description.autoApprovable !== true || rule === undefined) { + let rule = autoApprovalRule(this.storage, gatekeeperId, record.description); + if (rule === undefined) { // A manual gate. Stop rather than skipping ahead to any later auto-eligible action. break; } diff --git a/packages/workshop-backend/src/overseer.ts b/packages/workshop-backend/src/overseer.ts index b491dbc08..f3237c59a 100644 --- a/packages/workshop-backend/src/overseer.ts +++ b/packages/workshop-backend/src/overseer.ts @@ -42,7 +42,7 @@ import { checkUsageAndBalance } from "./ai-gateway-billing/limits/usage-checker" import { completeAgentCatalogSnapshot, normalizeAgentCatalog } from "./agent-catalog"; import { refreshCachedBalance } from "./ai-gateway-billing/cloudflare/connection-service"; import { SharingManager, SharingCaller, CollaboratorRecord, ShareKeyRecord, roleRank } from "./sharing"; -import { AutoApprovalDrainer } from "./auto-approval"; +import { AutoApprovalDrainer, autoApprovalRule } from "./auto-approval"; import { collectSlashCommands, invokeSlashCommand } from "./slash-commands"; import { createWorkshopLogger, obsContext, traced } from "./observability"; import { retryOnDoReset, wrapDoStubForTelemetry } from "./do-retry"; @@ -1015,8 +1015,10 @@ export function makeOverseerStorage(storage: DurableObjectStorage) { nextHookId: 0, // True if any past observation was authorized that had the `containsRestrictedData` flag - // set in its `ObservationDescription`. While set, the workspace may not perform actions or - // fetch from the public web. + // set in its `ObservationDescription`. While set, the workspace may not fetch from the + // public web, and actions are limited to the connections that produced the restricted data + // (the writes-to-self carve-out; see restrictedProducerIds and submitAction), each + // requiring manual approval (never auto-approved; see autoApprovalRule). // // NOTE: The property CANNOT be renamed to match the flag: the typed-storage key is the // property name, so a rename would silently unlatch every workspace that has already @@ -4620,8 +4622,9 @@ class OverseerImpl implements AgentHooks { // persists the record (authorizeObservation), so a latched workspace always yields a non-empty // set. Built-in tool observations are skipped: they name no connection, and the // BUILTIN_TOOL_GATEKEEPER_ID sentinel could never match a gatekeeper record (built-ins also - // never latch). Cold paths only (connection removal and sharing mutators), so the full scan is - // fine. + // never latch). The callers are cold paths (connection removal, sharing mutators) plus + // submitAction -- but the latter scans only while latched, and the auto-approval drainer + // already full-scans the log per drain, so the scan is fine where it runs. restrictedProducerIds(): Set { let producers = new Set(); for (let record of this.storage.actions.list()) { @@ -4794,10 +4797,19 @@ class OverseerImpl implements AgentHooks { async submitAction(gatekeeperId: number, action: number, description: ActionDescription, caller: GatekeeperCaller) : Promise { - if (this.storage.prohibitAllSharing.get()) { + // Writes-to-self carve-out: a latched workspace may still act on the connections that + // produced its restricted data -- sending the data back where it came from reveals nothing + // new to that system -- while any other target could leak it. (Every such action still + // requires manual human approval; see autoApprovalRule.) A latched workspace always has a + // non-empty producer set (the latch and its action record are written in one synchronous + // block; see restrictedProducerIds); if the set is ever empty anyway, the `has` check fails + // for every target and all actions are refused -- the conservative fallback. Refused before + // the id allocation below, so a blocked action leaves no record behind. + if (this.storage.prohibitAllSharing.get() && + !this.restrictedProducerIds().has(gatekeeperId)) { throw new Error( - "This workspace has observed sensitive data. To prevent leaks, the workspace is prohibited " + - "from performing actions."); + "This workspace has observed sensitive data from other connections. To prevent leaks, " + + "it may only perform actions on those same connections."); } let actionId = this.storage.nextActionId.get(); @@ -4821,10 +4833,9 @@ class OverseerImpl implements AgentHooks { this.storage.actions.put(record); this.#associateAction(caller, actionId); - // Same auto-approval gate as before, named because awaitDecision uses it too. The drain is - // deferred because applying calls back into the gatekeeper facet still awaiting submitAction. - let willAutoApprove = !!(description.autoApprovable && description.actionKind && - this.storage.autoApproveTags.get(`${gatekeeperId}:${description.actionKind.tag}`) !== undefined); + // Same auto-approval gate the drainer uses, named because awaitDecision uses it too. The drain + // is deferred because applying calls back into the gatekeeper facet still awaiting submitAction. + let willAutoApprove = autoApprovalRule(this.storage, gatekeeperId, description) !== undefined; // Only agent turns suspend on awaitDecision, and only when a manual decision is pending. // Auto-approved actions keep the seamless behavior the user opted into. @@ -10036,6 +10047,14 @@ class OverseerClientInterface extends RpcTarget implements Overseer { throw new Error(`No such gatekeeper: ${gatekeeperId}`); } + // A rule stored while the workspace is latched would never fire (see autoApprovalRule), so + // refuse to store one rather than let the UI suggest auto-approval is in effect. + if (this.impl.storage.prohibitAllSharing.get()) { + throw new Error( + "This workspace has observed sensitive data, so its actions always require manual " + + "approval and cannot be auto-approved."); + } + let profile = await this.#getClientProfile(); this.impl.storage.autoApproveTags.put({ gatekeeperId, @@ -10062,6 +10081,10 @@ class OverseerClientInterface extends RpcTarget implements Overseer { } async listPreApprovableActions(): Promise { + // A latched workspace can't have auto-approval rules (see setAutoApprovedActionKind), so + // offer nothing. + if (this.impl.storage.prohibitAllSharing.get()) return []; + // Surface actions from every gatekeeper bound by some gadget (the connections the UI shows). let boundIds = new Set(); for (let gadget of this.impl.storage.gadgets.list()) { diff --git a/packages/workshop-shared/src/api.ts b/packages/workshop-shared/src/api.ts index e36a52be0..cd5784900 100644 --- a/packages/workshop-shared/src/api.ts +++ b/packages/workshop-shared/src/api.ts @@ -1256,8 +1256,9 @@ export type GadgetMetadata = { /** * True when the gadget has observed data marked as containing restricted data (see * `ObservationDescription.containsRestrictedData`). Such gadgets can still be shared, but - * collaborators must be verified (per gatekeeper) to have access to the same data, and the - * workspace can no longer perform actions or fetch from the public web. + * collaborators must be verified (per gatekeeper) to have access to the same data; the + * workspace can no longer fetch from the public web, and actions are limited to the + * connections that produced the sensitive data (manual approval only). */ containsRestrictedData?: boolean; diff --git a/packages/workshop-shared/src/gatekeeper.ts b/packages/workshop-shared/src/gatekeeper.ts index c6728d128..b1df95b52 100644 --- a/packages/workshop-shared/src/gatekeeper.ts +++ b/packages/workshop-shared/src/gatekeeper.ts @@ -1077,8 +1077,11 @@ export type ObservationDescription = { * coverage guard). Collaborators are re-verified every time they open the gadget, so a * gatekeeper whose `addObserver()` always throws is unshareable once it has made one of * these observations. - * - Once observed, the gadget enters a restricted mode: no more actions or public-web fetches, - * only observations, so the gadget cannot leak the data through other gatekeepers. + * - Once observed, the gadget enters a restricted mode: it may no longer fetch from the + * public web, and it may only perform actions that target a gatekeeper that itself produced + * a sensitive observation (writes-to-self -- sending the data back where it came from + * reveals nothing new), each requiring manual human approval (never auto-approved). This + * prevents the gadget from leaking the data through other gatekeepers. * * TODO(someday): The restricted mode is a blunt instrument. It should be possible to perform * actions whose visibility is limited to people verified to have access to the same data, From 21151032a353ba6ba4120b6fa765d433c12dc188 Mon Sep 17 00:00:00 2001 From: Maximo Guk <62088388+Maximo-Guk@users.noreply.github.com> Date: Sun, 23 Aug 2026 19:24:59 -0500 Subject: [PATCH 02/12] Integration-test the writes-to-self carve-out and auto-approval gates The fixture gatekeeper's doThing() now tags its poke with an actionKind and takes a per-action autoApprovable verdict, getAutoApprovableActions offers the kind, and applyAction succeeds, so tests can drive the real submit -> auto-approve -> apply round trip. The post-latch action test flips by design: the actor IS the producer, so its doThing() now pends (writes-to-self) while a second connection on the same account is still refused. New auto-approval-policy tests cover the happy auto-approve path and that a pre-latch rule stops firing once the workspace reads sensitive data -- the action pends, the rule surface refuses, and manual approval remains the path through. Co-Authored-By: Claude Fable 5 --- .../__tests__/auto-approval-policy.test.ts | 136 ++++++++++++++++++ .../__tests__/sensitive-observations.test.ts | 22 ++- .../gatekeeper-test/src/test-gatekeeper.ts | 24 +++- 3 files changed, 172 insertions(+), 10 deletions(-) create mode 100644 packages/integration-tests/__tests__/auto-approval-policy.test.ts diff --git a/packages/integration-tests/__tests__/auto-approval-policy.test.ts b/packages/integration-tests/__tests__/auto-approval-policy.test.ts new file mode 100644 index 000000000..2432e1a98 --- /dev/null +++ b/packages/integration-tests/__tests__/auto-approval-policy.test.ts @@ -0,0 +1,136 @@ +// Tests for the auto-approval policy gate around sensitive data. +// +// Auto-approval requires the author's per-action `autoApprovable` verdict AND a user-enabled rule +// for the action's kind. The restricted-data latch must additionally force manual approval no +// matter what -- even when the rule was enabled before the data was read. (The web-fetch +// restriction has no client-reachable surface, so it is not asserted here.) +// +// The fixture gatekeeper's session drives this through the real ApprovalQueue funnel: `doThing()` +// submits a "poke" action with the given verdict, and `applyAction` succeeds, so the drain's +// submit -> auto-approve -> apply round trip is the real one. + +import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import type { RpcStub } from "capnweb"; +import type { + ActionLogEntry, AuthenticatedApi, Overseer, PublicApi, +} from "@gadgets/workshop-shared/api"; +import { startTestGatekeeperHarness, TEST_VENDOR_ID, type Harness } from "../src/harness.js"; +import { + connect, listConnectedAccounts, nextUsernames, signUp, waitFor, type ConnectedAccount, +} from "../src/rpc-client.js"; +import { NetworkInterceptor } from "../src/network-interceptor.js"; + +const POKE = { tag: "poke", label: "Pokes" }; + +let harness: Harness; +let interceptor: NetworkInterceptor; + +beforeAll(async () => { + interceptor = new NetworkInterceptor(); + interceptor.install(); + harness = await startTestGatekeeperHarness(); +}); + +afterAll(async () => { + const unmocked = interceptor.getUnmockedCalls(); + await harness?.server.close(); + interceptor.uninstall(); + interceptor.reset(); + expect(unmocked).toEqual([]); +}); + +async function withSession(body: (api: RpcStub) => Promise): Promise { + const publicApi = connect(harness.url); + try { + return await body(publicApi); + } finally { + publicApi[Symbol.dispose](); + } +} + +async function provisionAccount(api: RpcStub): Promise { + await api.provisionAmbientAccount(TEST_VENDOR_ID); + return waitFor("the test account to be provisioned", async () => { + const accounts = await listConnectedAccounts(api); + return accounts.find(a => a.vendorId === TEST_VENDOR_ID) ?? null; + }); +} + +type Workspace = { + overseer: RpcStub; + session: any; + gatekeeperId: number; +}; + +async function newWorkspace(publicApi: RpcStub, thingName: string): Promise { + const [alice] = nextUsernames("alice"); + const aliceApi = await signUp(publicApi, alice); + const account = await provisionAccount(aliceApi); + const overseer = await aliceApi.newGadget(); + const gatekeeper = await overseer.newGatekeeper( + account.id, `https://gadgets-test.example/things/${thingName}`); + if (!gatekeeper) throw new Error("Failed to create the test connection"); + return { + overseer, + session: await gatekeeper.openSession(), + gatekeeperId: await gatekeeper.getId(), + }; +} + +async function listPokes(ws: Workspace): Promise> { + const actions = await ws.overseer.listActions(); + return actions.filter( + (a): a is ActionLogEntry & { type: "action" } => + a.type === "action" && a.gatekeeperId === ws.gatekeeperId); +} + +// The drain runs via ctx.waitUntil after submit, so "did not auto-approve" needs a settle window. +// One further RPC round trip plus a beat is far beyond the drain's synchronous storage work. +async function settle(ws: Workspace): Promise { + await ws.overseer.listActions(); + await new Promise(resolve => setTimeout(resolve, 300)); +} + +describe("auto-approval policy", () => { + it.concurrent("auto-approves a rule-enabled, author-approvable action", async () => { + await withSession(async publicApi => { + const ws = await newWorkspace(publicApi, "auto-happy"); + await ws.overseer.setAutoApprovedActionKind(ws.gatekeeperId, POKE); + await ws.session.doThing({ autoApprovable: true }); + + const applied = await waitFor("the poke to be auto-approved", async () => { + const [poke] = await listPokes(ws); + return poke?.state === "approved" ? poke : null; + }); + expect(applied.autoApproved).toBe(true); + }); + }); + + it.concurrent("a pre-latch auto-approval rule stops firing once the workspace reads sensitive data", + async () => { + await withSession(async publicApi => { + const ws = await newWorkspace(publicApi, "pre-latch"); + await ws.overseer.setAutoApprovedActionKind(ws.gatekeeperId, POKE); + await ws.session.readThing(true); + + // Writes-to-self still pend, but the rule the user enabled before the latch must not fire. + await ws.session.doThing({ autoApprovable: true }); + await settle(ws); + const [poke] = await listPokes(ws); + expect(poke.state).toBe("pending"); + + // The rule surface refuses a latched workspace outright. + await expect(ws.overseer.setAutoApprovedActionKind(ws.gatekeeperId, POKE)) + .rejects.toThrow(/cannot be auto-approved/i); + await expect(ws.overseer.listPreApprovableActions()).resolves.toEqual([]); + + // Manual approval still works: the human is the intended path. + await ws.overseer.approveAction(poke.id); + const approved = await waitFor("the poke to be applied after manual approval", async () => { + const [fresh] = await listPokes(ws); + return fresh.state === "approved" ? fresh : null; + }); + expect(approved.autoApproved).toBeFalsy(); + }); + }); +}); diff --git a/packages/integration-tests/__tests__/sensitive-observations.test.ts b/packages/integration-tests/__tests__/sensitive-observations.test.ts index ec39a2d87..2d73867f3 100644 --- a/packages/integration-tests/__tests__/sensitive-observations.test.ts +++ b/packages/integration-tests/__tests__/sensitive-observations.test.ts @@ -3,8 +3,9 @@ // A sensitive observation is blocked only while some *current collaborator* has not been verified // (via `addObserver`) against the gatekeeper producing it; sharing itself stays available, since // recipients are verified when they open. The observation also latches the workspace into a -// restricted mode: once latched, the workspace may not perform actions (nor fetch from the web, -// which has no client-reachable surface to assert here). +// restricted mode: once latched, the workspace may only perform actions targeting the connections +// that produced the sensitive data -- the writes-to-self carve-out -- and may not fetch from the +// web (which has no client-reachable surface to assert here). // // The fixture gatekeeper's session drives all of this through the real ApprovalQueue funnel: // `readThing(true)` records a `containsRestrictedData` observation, `doThing()` submits an action. @@ -136,7 +137,8 @@ async function bobOpens(gadgetId: string, bobApi: RpcStub, } describe("sensitive observations", () => { - it.concurrent("latch restricted mode: actions are blocked and metadata reports it", async () => { + it.concurrent("latch restricted mode: only writes-to-self are allowed and metadata reports it", + async () => { await withSession(async publicApi => { const ws = await newWorkspace(publicApi, "latch"); @@ -147,7 +149,19 @@ describe("sensitive observations", () => { await expect(ws.session.readThing(true)).resolves.toContain("latch"); expect((await ws.overseer.getMetadata()).containsRestrictedData).toBe(true); - await expect(ws.session.doThing()).rejects.toThrow(/prohibited from performing actions/i); + + // Actions targeting the gatekeeper that produced the sensitive data still pend (the + // writes-to-self carve-out: the data came from there, so sending it back reveals nothing + // new). Actions on any other connection are blocked. + await expect(ws.session.doThing()).resolves.toBeUndefined(); + const accounts = await listConnectedAccounts(ws.aliceApi); + const account = accounts.find(a => a.vendorId === TEST_VENDOR_ID)!; + const other = await ws.overseer.newGatekeeper(account.id, thingUrl("latch-other")); + if (!other) throw new Error("Failed to create the second test connection"); + const otherSession: any = await other.openSession(); + await expect(otherSession.doThing()) + .rejects.toThrow(/only perform actions on those same connections/i); + // Reads -- sensitive or not -- keep working. await expect(ws.session.readThing()).resolves.toContain("latch"); await expect(ws.session.readThing(true)).resolves.toContain("latch"); diff --git a/packages/integration-tests/fixtures/gatekeeper-test/src/test-gatekeeper.ts b/packages/integration-tests/fixtures/gatekeeper-test/src/test-gatekeeper.ts index 19a3f907c..252e58559 100644 --- a/packages/integration-tests/fixtures/gatekeeper-test/src/test-gatekeeper.ts +++ b/packages/integration-tests/fixtures/gatekeeper-test/src/test-gatekeeper.ts @@ -235,7 +235,8 @@ export class TestVerifier * The two methods exist so tests can drive the overseer's observation/action policy through the * same `ApprovalQueue` funnel a shipping gatekeeper uses: `readThing()` records an observation * (optionally marked `containsRestrictedData`, to trip the sensitive-data coverage guard and the - * restricted-mode latch), and `doThing()` submits an action (which restricted mode blocks). + * restricted-mode latch), and `doThing()` submits an action (which restricted mode allows only + * back to the producing connections, never auto-approved). */ export class TestSession extends RpcTarget { #queue: RpcStub; @@ -256,11 +257,16 @@ export class TestSession extends RpcTarget { return `the contents of ${this.#title}`; } - async doThing(): Promise { + async doThing(opts?: { autoApprovable?: boolean }): Promise { await this.#queue.submitAction(0, { title: `Poke ${this.#title}`, description: `The test poked ${this.#title}.`, implementsRevert: false, + // Always tagged, so a test can enable an auto-approval rule for it; whether this specific + // poke is eligible is the per-action verdict below, exactly as a shipping gatekeeper would + // set it. + actionKind: POKE_ACTION_KIND, + ...(opts?.autoApprovable ? { autoApprovable: true } : {}), }); } @@ -270,6 +276,8 @@ export class TestSession extends RpcTarget { } } +const POKE_ACTION_KIND: ActionKind = { tag: "poke", label: "Pokes" }; + export class TestGatekeeper extends DurableObject implements Gatekeeper { async describe(): Promise { @@ -298,7 +306,7 @@ export class TestGatekeeper } async getAutoApprovableActions(): Promise { - return []; + return [POKE_ACTION_KIND]; } async startSession(approvalQueue: RpcStub): Promise { @@ -336,14 +344,18 @@ export class TestGatekeeper this.ctx.storage.kv.delete(`observer:${id}`); } - async applyAction(_action: number): Promise { - throw new Error("The test gatekeeper submits no actions."); + /** + * Applying succeeds and leaves a record, so approval-path tests (manual and auto) can drive a + * submit -> approve -> apply round trip against real machinery. + */ + async applyAction(action: number): Promise { + this.ctx.storage.kv.put(`applied:${action}`, true); } async rejectAction(_action: number): Promise {} async revertAction(_action: number): Promise { - throw new Error("The test gatekeeper submits no actions."); + throw new Error("The test gatekeeper does not implement revert."); } } From f3e3560471c0374f135dedb166e7367bd9ad20d1 Mon Sep 17 00:00:00 2001 From: Maximo Guk <62088388+Maximo-Guk@users.noreply.github.com> Date: Sun, 23 Aug 2026 19:59:23 -0500 Subject: [PATCH 03/12] Add ActionDescription.operatorWarnings: gatekeeper-authored approver warnings Gatekeeper-authored warnings addressed to the human approver, for context the action's own content can't show -- e.g. "this conversation has read data from other accounts, which could leak into this write." - New optional `ActionDescription.operatorWarnings: string[]`. - A warning exists precisely to be read by a human, so autoApprovalRule() refuses any action carrying one, even with the author's verdict and a matching user rule; the drainer therefore stops at a warned action without skipping ahead. - Both approval surfaces (chat action card and the Activity panel) render the warnings as a prominent warning strip ahead of the description, and suppress the always-approve affordance on warned actions. - Unit tests for the predicate and drainer gates; integration test drives a warned poke through the real submit -> pend -> manual approve -> drain round trip via the test gatekeeper fixture. Co-Authored-By: Claude Fable 5 --- .../__tests__/auto-approval-policy.test.ts | 40 ++++++++++++++++--- .../gatekeeper-test/src/test-gatekeeper.ts | 3 +- .../__tests__/auto-approval.test.ts | 28 ++++++++++++- .../workshop-backend/src/auto-approval.ts | 7 +++- packages/workshop-frontend/src/Activity.tsx | 36 ++++++++++++++++- .../workshop-frontend/src/ChatInterface.tsx | 25 +++++++++++- packages/workshop-shared/src/gatekeeper.ts | 11 +++++ 7 files changed, 138 insertions(+), 12 deletions(-) diff --git a/packages/integration-tests/__tests__/auto-approval-policy.test.ts b/packages/integration-tests/__tests__/auto-approval-policy.test.ts index 2432e1a98..129ac63c7 100644 --- a/packages/integration-tests/__tests__/auto-approval-policy.test.ts +++ b/packages/integration-tests/__tests__/auto-approval-policy.test.ts @@ -1,13 +1,14 @@ -// Tests for the auto-approval policy gate around sensitive data. +// Tests for the auto-approval policy gates around sensitive data and operator warnings. // // Auto-approval requires the author's per-action `autoApprovable` verdict AND a user-enabled rule -// for the action's kind. The restricted-data latch must additionally force manual approval no -// matter what -- even when the rule was enabled before the data was read. (The web-fetch -// restriction has no client-reachable surface, so it is not asserted here.) +// for the action's kind. Two things must additionally force manual approval no matter what: +// `operatorWarnings` on the action (a warning exists to be read by a human), and the +// restricted-data latch -- even when the rule was enabled before the data was read. (The +// web-fetch restriction has no client-reachable surface, so it is not asserted here.) // // The fixture gatekeeper's session drives this through the real ApprovalQueue funnel: `doThing()` -// submits a "poke" action with the given verdict, and `applyAction` succeeds, so the drain's -// submit -> auto-approve -> apply round trip is the real one. +// submits a "poke" action with the given verdict/warnings, and `applyAction` succeeds, so the +// drain's submit -> auto-approve -> apply round trip is the real one. import { afterAll, beforeAll, describe, expect, it } from "vitest"; import type { RpcStub } from "capnweb"; @@ -106,6 +107,33 @@ describe("auto-approval policy", () => { }); }); + it.concurrent("a warned action holds the queue until a human approves it", async () => { + await withSession(async publicApi => { + const ws = await newWorkspace(publicApi, "warned"); + await ws.overseer.setAutoApprovedActionKind(ws.gatekeeperId, POKE); + + // The warned action is fully rule-covered and author-approved -- only the warning stands + // between it and auto-application. A clean eligible action behind it must not be applied + // either (the drain never skips ahead of a manual gate). + await ws.session.doThing({ autoApprovable: true, warnings: ["Cross-account data risk."] }); + await ws.session.doThing({ autoApprovable: true }); + await settle(ws); + + const pokes = await listPokes(ws); + expect(pokes.map(p => p.state)).toEqual(["pending", "pending"]); + expect(pokes[0].description.operatorWarnings).toEqual(["Cross-account data risk."]); + + // A human approving the warned action clears the gate; the one behind it then auto-applies. + await ws.overseer.approveAction(pokes[0].id); + const drained = await waitFor("the queued poke to auto-apply", async () => { + const [first, second] = await listPokes(ws); + return first.state === "approved" && second?.state === "approved" ? [first, second] : null; + }); + expect(drained[0].autoApproved).toBeFalsy(); + expect(drained[1].autoApproved).toBe(true); + }); + }); + it.concurrent("a pre-latch auto-approval rule stops firing once the workspace reads sensitive data", async () => { await withSession(async publicApi => { diff --git a/packages/integration-tests/fixtures/gatekeeper-test/src/test-gatekeeper.ts b/packages/integration-tests/fixtures/gatekeeper-test/src/test-gatekeeper.ts index 252e58559..45b1d283b 100644 --- a/packages/integration-tests/fixtures/gatekeeper-test/src/test-gatekeeper.ts +++ b/packages/integration-tests/fixtures/gatekeeper-test/src/test-gatekeeper.ts @@ -257,7 +257,7 @@ export class TestSession extends RpcTarget { return `the contents of ${this.#title}`; } - async doThing(opts?: { autoApprovable?: boolean }): Promise { + async doThing(opts?: { autoApprovable?: boolean; warnings?: string[] }): Promise { await this.#queue.submitAction(0, { title: `Poke ${this.#title}`, description: `The test poked ${this.#title}.`, @@ -267,6 +267,7 @@ export class TestSession extends RpcTarget { // set it. actionKind: POKE_ACTION_KIND, ...(opts?.autoApprovable ? { autoApprovable: true } : {}), + ...(opts?.warnings ? { operatorWarnings: opts.warnings } : {}), }); } diff --git a/packages/workshop-backend/__tests__/auto-approval.test.ts b/packages/workshop-backend/__tests__/auto-approval.test.ts index 1c37bb076..73e2f4597 100644 --- a/packages/workshop-backend/__tests__/auto-approval.test.ts +++ b/packages/workshop-backend/__tests__/auto-approval.test.ts @@ -33,7 +33,7 @@ function enableRule(storage: AutoApprovalStorage, actionTag = "edit", gatekeeper function putAction( storage: AutoApprovalStorage, id: number, opts: { gatekeeperId?: number; actionTag?: string; autoApprovable?: boolean; - state?: ActionRecord["state"] } = {}) { + operatorWarnings?: string[]; state?: ActionRecord["state"] } = {}) { storage.actions.put({ id, gatekeeperId: opts.gatekeeperId ?? GK, @@ -48,6 +48,7 @@ function putAction( implementsRevert: true, actionKind: { tag: opts.actionTag ?? "edit", label: "Edits" }, autoApprovable: opts.autoApprovable ?? true, + ...(opts.operatorWarnings ? { operatorWarnings: opts.operatorWarnings } : {}), }, }); } @@ -218,6 +219,20 @@ describe("AutoApprovalDrainer.drain", () => { expect(getAction(storage, 2).state).toBe("approved"); }); + it("stops at a warned action without skipping ahead", async () => { + let storage = makeStorage(); + enableRule(storage); + putAction(storage, 1, { operatorWarnings: ["cross-account data risk"] }); + putAction(storage, 2); + + let apply = makeImmediateApply(storage); + await new AutoApprovalDrainer(storage, apply.applyFn).drain(GK); + + expect(apply.calls).toEqual([]); + expect(getAction(storage, 1).state).toBe("pending"); + expect(getAction(storage, 2).state).toBe("pending"); + }); + it("applies nothing while the workspace is latched, even with a matching rule", async () => { let storage = makeStorage(); enableRule(storage); @@ -277,6 +292,17 @@ describe("autoApprovalRule", () => { expect(autoApprovalRule(storage, GK, eligibleDescription())).toBeUndefined(); }); + it("refuses any action carrying operator warnings", () => { + let storage = makeStorage(); + enableRule(storage); + expect(autoApprovalRule( + storage, GK, eligibleDescription({ operatorWarnings: ["watch out"] }))) + .toBeUndefined(); + // An empty array carries no warning to read, so it does not disqualify. + expect(autoApprovalRule(storage, GK, eligibleDescription({ operatorWarnings: [] }))) + .toBeDefined(); + }); + it("refuses every action while the restricted-data latch is set", () => { let storage = makeStorage(); enableRule(storage); diff --git a/packages/workshop-backend/src/auto-approval.ts b/packages/workshop-backend/src/auto-approval.ts index 40c1bba6b..579d1066f 100644 --- a/packages/workshop-backend/src/auto-approval.ts +++ b/packages/workshop-backend/src/auto-approval.ts @@ -29,6 +29,8 @@ export interface AutoApprovalStorage { * rule iff ALL of: * - the gatekeeper author marked this specific action `autoApprovable`, * - the action carries an `actionKind` for which the user enabled a rule on this gatekeeper, + * - the action carries no `operatorWarnings` (a warning exists precisely to be read by the + * human approver, so it forces manual approval), * - the workspace has not latched restricted mode (`prohibitAllSharing` above). * Returns undefined otherwise: manual approval required. */ @@ -38,6 +40,9 @@ export function autoApprovalRule( if (description.autoApprovable !== true) return undefined; let tag = description.actionKind?.tag; if (tag === undefined) return undefined; + if (description.operatorWarnings !== undefined && description.operatorWarnings.length > 0) { + return undefined; + } if (storage.prohibitAllSharing.get()) return undefined; return storage.autoApproveTags.get(`${gatekeeperId}:${tag}`); } @@ -83,7 +88,7 @@ export class AutoApprovalDrainer { // nothing is silently applied past a human gate. // // Eligibility is `autoApprovalRule()`: the author's `autoApprovable` verdict, a user-enabled - // rule for the action's kind, and no restricted-data latch. + // rule for the action's kind, no operator warnings, and no restricted-data latch. async #drainOnce(gatekeeperId: number): Promise { // Materialize a snapshot first: list() is a lazy generator over storage, and we mutate the // actions collection (via applyPendingAction) as we go. diff --git a/packages/workshop-frontend/src/Activity.tsx b/packages/workshop-frontend/src/Activity.tsx index f4ab21e53..51746ec54 100644 --- a/packages/workshop-frontend/src/Activity.tsx +++ b/packages/workshop-frontend/src/Activity.tsx @@ -1,6 +1,6 @@ import { useEffect, useMemo, useRef, useState, type ReactNode } from 'react' import { Switch, useKumoToastManager } from '@cloudflare/kumo' -import { CaretRight, Check, Eye, Lightning, ShieldCheck } from '@phosphor-icons/react' +import { CaretRight, Check, Eye, Lightning, ShieldCheck, ShieldWarning } from '@phosphor-icons/react' import { RpcStub } from 'capnweb' import { ActionLogEntry, Overseer } from '@gadgets/workshop-shared/api' import { ActionKind } from '@gadgets/workshop-shared/gatekeeper' @@ -220,7 +220,10 @@ export default function Activity({ const autoApproveTarget = record.type === 'action' && record.gatekeeperId !== undefined && record.description.actionKind !== undefined && - record.description.autoApprovable === true + record.description.autoApprovable === true && + // A warned action is never auto-approved (see autoApprovalRule in the backend), + // so don't offer the rule from it. + (record.description.operatorWarnings?.length ?? 0) === 0 ? { actionId: record.id, gatekeeperId: record.gatekeeperId, @@ -505,6 +508,7 @@ function ReviewRequest({ onAlwaysApprove?: () => void }) { const resourceUrl = safeExternalUrl(record.resourceUrl) + const operatorWarnings = record.type === 'action' ? record.description.operatorWarnings ?? [] : [] return (
@@ -547,6 +551,20 @@ function ReviewRequest({
+ {operatorWarnings.length > 0 && ( +
+ {operatorWarnings.map((warning, i) => ( +
+ + {warning} +
+ ))} +
+ )} + {record.description.description && (

{record.description.description} @@ -572,6 +590,7 @@ function HistoryRow({ const resourceUrl = safeExternalUrl(record.resourceUrl) const resolvedBy = record.type === 'action' ? record.resolvedBy : undefined const autoApproved = record.type === 'action' && record.autoApproved === true + const operatorWarnings = record.type === 'action' ? record.description.operatorWarnings ?? [] : [] const at = record.appliedAt ?? record.createdAt const status = activityStatus(record) @@ -607,6 +626,19 @@ function HistoryRow({ {expanded && (

+ {operatorWarnings.length > 0 && ( +
+ {operatorWarnings.map((warning, i) => ( +
+ + {warning} +
+ ))} +
+ )} {record.description.description && (

{record.description.description} diff --git a/packages/workshop-frontend/src/ChatInterface.tsx b/packages/workshop-frontend/src/ChatInterface.tsx index c5ae6faff..30876d7af 100644 --- a/packages/workshop-frontend/src/ChatInterface.tsx +++ b/packages/workshop-frontend/src/ChatInterface.tsx @@ -49,6 +49,7 @@ import { PencilSimple, Brain, ShieldCheck, + ShieldWarning, Terminal, Globe, MagnifyingGlass, @@ -6822,6 +6823,23 @@ function ChatInterface({ // decision. Resolved actions are history, and collapse so a long thread stays scannable. const showDescription = isPending || open; const metadata = log.resourceTitle; + // Gatekeeper-authored warnings for the human approver: rendered prominently ahead of the + // description in both presentations. Their presence also suppresses the always-approve + // affordance below (the backend never auto-approves a warned action regardless). + const operatorWarnings = log.description.operatorWarnings ?? []; + const warningStrip = operatorWarnings.length > 0 ? ( +

+ {operatorWarnings.map((warning, i) => ( +
+ + {warning} +
+ ))} +
+ ) : null; const stateLabel = isApproved ? "Approved" : isRejected @@ -6836,7 +6854,10 @@ function ChatInterface({ // auto-approvable action with an existing rule wouldn't still be pending.) const autoApproveTarget = log.gatekeeperId !== undefined && log.description.actionKind !== undefined && - log.description.autoApprovable === true + log.description.autoApprovable === true && + // A warned action is never auto-approved (see autoApprovalRule in the backend), so don't + // offer the rule from it. + operatorWarnings.length === 0 ? { actionId: msg.actionId, gatekeeperId: log.gatekeeperId, @@ -6915,6 +6936,7 @@ function ChatInterface({ {resourceMeta}
+ {warningStrip}
@@ -6975,6 +6997,7 @@ function ChatInterface({ )} {showDescription && (
+ {warningStrip}
diff --git a/packages/workshop-shared/src/gatekeeper.ts b/packages/workshop-shared/src/gatekeeper.ts index b1df95b52..955adbad9 100644 --- a/packages/workshop-shared/src/gatekeeper.ts +++ b/packages/workshop-shared/src/gatekeeper.ts @@ -1184,6 +1184,17 @@ export type ActionDescription = { */ autoApprovable?: boolean; + /** + * Gatekeeper-authored warnings addressed to the human approver, rendered prominently on the + * approval card ahead of the description. Use these when the gatekeeper knows something about + * the *context* of the action that the action's own content can't show -- e.g. "this + * conversation has read data from other accounts, which could leak into this write." + * + * A warning exists precisely to be read by a human, so any action carrying one is never + * auto-approved, even if `autoApprovable` is set and a matching rule exists. + */ + operatorWarnings?: string[]; + // ---------------------------------------------------------------------------- // Policy hints // From 4f12ecba1d8a74cc83b2c3fbe05f0d61f84d6559 Mon Sep 17 00:00:00 2001 From: Maximo Guk <62088388+Maximo-Guk@users.noreply.github.com> Date: Mon, 24 Aug 2026 17:53:39 -0500 Subject: [PATCH 04/12] Bugfix: Refuse to persist an action whose connection no longer exists. An in-flight facet RPC can outlive removeGatekeeper (the same race the restricted-observation refusal in authorizeObservation guards), so submitAction could persist a pending action naming a removed connection. Such a record can never be resolved: approveAction and rejectAction both dereference it through getGatekeeperFacet, which throws, and an awaitDecision agent turn then suspends forever. Require a live gatekeeper record before any write (refused ahead of the id allocation, so no record is left behind). This also closes the latched case: restrictedProducerIds deliberately survives removal, so carve-out set membership alone must not admit a write to a dead connection. Co-Authored-By: Claude Fable 5 --- .../restricted-writes-to-self.test.ts | 37 ++++++++++++++++++- packages/workshop-backend/src/overseer.ts | 21 +++++++++-- 2 files changed, 53 insertions(+), 5 deletions(-) diff --git a/packages/workshop-backend/__tests__/restricted-writes-to-self.test.ts b/packages/workshop-backend/__tests__/restricted-writes-to-self.test.ts index ad3a9ea60..e6feb1324 100644 --- a/packages/workshop-backend/__tests__/restricted-writes-to-self.test.ts +++ b/packages/workshop-backend/__tests__/restricted-writes-to-self.test.ts @@ -2,7 +2,8 @@ // the connections that produced its restricted data (sending the data back where it came from // reveals nothing new), while any other target is refused before a record is written. Latched // actions are never auto-approved -- even a write-to-self with a matching rule must pend for a -// human (see autoApprovalRule). +// human (see autoApprovalRule). An action naming a removed connection is refused outright, +// latched or not: a pending action on a dead connection could never be approved or rejected. // // Runs against a real OverseerDurableObject (the TEST_OVERSEER binding, like // restricted-producer-removal.test.ts) so submitAction reads real storage; records are seeded @@ -128,6 +129,40 @@ describe("submitAction under the restricted-data latch", () => { }); }); + it("refuses a latched action on a removed producer, writing no record", async () => { + let stub = env.TEST_OVERSEER.getByName("writes-to-self-removed-producer"); + await runInDurableObject(stub, async (instance: OverseerDurableObject) => { + let impl = getImpl(instance); + seedGatekeeper(impl, 1); + seedRestrictedObservation(impl, 1, 100); + // The producer is removed, but restrictedProducerIds still contains it (it scans the + // never-deleted action log). Set membership must not admit the write: a pending action on a + // removed connection could never be approved or rejected. + impl.storage.gatekeepers.delete(1); + let nextActionId = impl.storage.nextActionId.get(); + + await expect(impl.submitAction(1, 0, pokeDescription(), CALLER)) + .rejects.toThrow(/has been removed from this workspace/i); + expect(actionStates(impl)).toEqual([]); + expect(impl.storage.nextActionId.get()).toBe(nextActionId); + }); + }); + + it("refuses an unlatched action on a removed connection, writing no record", async () => { + let stub = env.TEST_OVERSEER.getByName("writes-to-self-removed-unlatched"); + await runInDurableObject(stub, async (instance: OverseerDurableObject) => { + let impl = getImpl(instance); + seedGatekeeper(impl, 1); + impl.storage.gatekeepers.delete(1); + let nextActionId = impl.storage.nextActionId.get(); + + await expect(impl.submitAction(1, 0, pokeDescription(), CALLER)) + .rejects.toThrow(/has been removed from this workspace/i); + expect(actionStates(impl)).toEqual([]); + expect(impl.storage.nextActionId.get()).toBe(nextActionId); + }); + }); + it("refuses everything when the latch is set with no derivable producer", async () => { let stub = env.TEST_OVERSEER.getByName("writes-to-self-empty-producers"); await runInDurableObject(stub, async (instance: OverseerDurableObject) => { diff --git a/packages/workshop-backend/src/overseer.ts b/packages/workshop-backend/src/overseer.ts index f3237c59a..e6c8e675b 100644 --- a/packages/workshop-backend/src/overseer.ts +++ b/packages/workshop-backend/src/overseer.ts @@ -4797,6 +4797,21 @@ class OverseerImpl implements AgentHooks { async submitAction(gatekeeperId: number, action: number, description: ActionDescription, caller: GatekeeperCaller) : Promise { + // An in-flight facet RPC can outlive removeGatekeeper (cf. the restricted-observation refusal + // in authorizeObservation), so an action can arrive naming a connection this workspace no + // longer has. A pending action persisted on a removed connection could never be approved *or* + // rejected -- both paths dereference the record through getGatekeeperFacet -- and would + // suspend an awaitDecision agent turn forever, so refuse before any write. This also covers + // the latched case below: restrictedProducerIds deliberately survives removal (that is its + // point for removalBlockedByRestrictedData / assertNewSharingAllowed), so membership alone + // must not admit a write to a dead connection. + let gatekeeper = this.storage.gatekeepers.get(gatekeeperId); + if (!gatekeeper) { + throw new Error( + "This action was blocked because the connection it was submitted through has been " + + "removed from this workspace."); + } + // Writes-to-self carve-out: a latched workspace may still act on the connections that // produced its restricted data -- sending the data back where it came from reveals nothing // new to that system -- while any other target could leak it. (Every such action still @@ -4815,14 +4830,12 @@ class OverseerImpl implements AgentHooks { let actionId = this.storage.nextActionId.get(); this.storage.nextActionId.put(actionId + 1); - let gatekeeper = this.storage.gatekeepers.get(gatekeeperId); - let record: ActionRecord = { id: actionId, gatekeeperId, caller, - resourceTitle: gatekeeper?.resourceTitle, - resourceUrl: gatekeeper?.resourceUrl, + resourceTitle: gatekeeper.resourceTitle, + resourceUrl: gatekeeper.resourceUrl, action, createdAt: new Date(), state: "pending", From 0dbaed695e674502324db67e53388f541e44d199 Mon Sep 17 00:00:00 2001 From: Maximo Guk <62088388+Maximo-Guk@users.noreply.github.com> Date: Mon, 24 Aug 2026 17:54:14 -0500 Subject: [PATCH 05/12] Cleanup: Document the writes-to-self residual precisely. The carve-out's prior rationale ("sending the data back where it came from reveals nothing new") over-claims for the multi-producer and broad-connection cases: the check is per-connection set membership, not data provenance, so with producers {A, B} data observed from A may be written back to B, and one broad connection can reach audiences beyond where the data was read. State that residual in the containsRestrictedData JSDoc and at the carve-out itself, naming the mandatory human approval of every latched action as the interim mitigation. The existing TODO(someday) policy framework remains the full fix. Co-Authored-By: Claude Fable 5 --- packages/workshop-backend/src/overseer.ts | 7 ++++--- packages/workshop-shared/src/gatekeeper.ts | 10 +++++++--- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/packages/workshop-backend/src/overseer.ts b/packages/workshop-backend/src/overseer.ts index e6c8e675b..498402785 100644 --- a/packages/workshop-backend/src/overseer.ts +++ b/packages/workshop-backend/src/overseer.ts @@ -4813,9 +4813,10 @@ class OverseerImpl implements AgentHooks { } // Writes-to-self carve-out: a latched workspace may still act on the connections that - // produced its restricted data -- sending the data back where it came from reveals nothing - // new to that system -- while any other target could leak it. (Every such action still - // requires manual human approval; see autoApprovalRule.) A latched workspace always has a + // produced its restricted data, while any other target could leak it. (Every such action + // still requires manual human approval; see autoApprovalRule.) The check is set membership, + // not provenance -- the human approver is the check for cross-producer or broader-audience + // writes; see the containsRestrictedData doc. A latched workspace always has a // non-empty producer set (the latch and its action record are written in one synchronous // block; see restrictedProducerIds); if the set is ever empty anyway, the `has` check fails // for every target and all actions are refused -- the conservative fallback. Refused before diff --git a/packages/workshop-shared/src/gatekeeper.ts b/packages/workshop-shared/src/gatekeeper.ts index 955adbad9..233da0acd 100644 --- a/packages/workshop-shared/src/gatekeeper.ts +++ b/packages/workshop-shared/src/gatekeeper.ts @@ -1079,9 +1079,13 @@ export type ObservationDescription = { * these observations. * - Once observed, the gadget enters a restricted mode: it may no longer fetch from the * public web, and it may only perform actions that target a gatekeeper that itself produced - * a sensitive observation (writes-to-self -- sending the data back where it came from - * reveals nothing new), each requiring manual human approval (never auto-approved). This - * prevents the gadget from leaking the data through other gatekeepers. + * a sensitive observation (writes-to-self), each requiring manual human approval (never + * auto-approved). This prevents the gadget from leaking the data through other gatekeepers. + * Note the carve-out is per-connection set membership, not data provenance: with multiple + * restricted producers, data observed through one may be written back through another, and + * even a single connection can reach audiences beyond where the data was read (e.g. a broad + * account writing to a more public destination). The mandatory human approval of every such + * action is the interim mitigation for those cases. * * TODO(someday): The restricted mode is a blunt instrument. It should be possible to perform * actions whose visibility is limited to people verified to have access to the same data, From 6c0bab8f5658630569e08d55fdee1a18dfd4a4bd Mon Sep 17 00:00:00 2001 From: Maximo Guk <62088388+Maximo-Guk@users.noreply.github.com> Date: Mon, 24 Aug 2026 17:59:15 -0500 Subject: [PATCH 06/12] Cleanup: Reflect restricted mode in the approval surfaces. A latched workspace never auto-approves (autoApprovalRule refuses, setAutoApprovedActionKind throws, listPreApprovableActions returns nothing), but the UI still offered "Always approve" on pending actions -- which could only error -- and rendered pre-latch rules as applying. Thread the live GadgetMetadata.containsRestrictedData (already pushed by the metadata subscription) into ChatInterface and Activity: restricted workspaces no longer offer the always-approve affordance in either surface, and the auto-approvals panel annotates each rule as suspended ("won't apply -- actions always require manual approval") with enabling blocked while disabling stays possible, so a standing grant remains revocable. Presentation-only: the backend gates are unchanged, and rules are kept rather than cleared so revocation history stays intact. Co-Authored-By: Claude Fable 5 --- packages/workshop-frontend/src/Activity.tsx | 36 +++++++++++++++---- .../workshop-frontend/src/ChatInterface.tsx | 9 ++++- .../workshop-frontend/src/GadgetEditor.tsx | 2 ++ 3 files changed, 39 insertions(+), 8 deletions(-) diff --git a/packages/workshop-frontend/src/Activity.tsx b/packages/workshop-frontend/src/Activity.tsx index 51746ec54..b80d2fde1 100644 --- a/packages/workshop-frontend/src/Activity.tsx +++ b/packages/workshop-frontend/src/Activity.tsx @@ -26,6 +26,12 @@ const PANE_BAR = 'flex h-9 flex-shrink-0 items-center border-b border-kumo-line' interface ActivityProps { overseer: RpcStub + // True once the workspace has read restricted data (GadgetMetadata.containsRestrictedData, live + // via the metadata subscription). Latched actions are never auto-approved (the backend's + // setAutoApprovedActionKind also throws), so the review tab suppresses the always-approve + // affordance and the auto-approval panel annotates existing rules as suspended. Rules stay + // listed and disable-able: a standing grant must remain revocable. + restricted?: boolean view: ActivityView onViewChange: (view: ActivityView) => void onAutoApproveChange?: () => void @@ -111,6 +117,7 @@ function TypeIcon({ record, className }: { record: ActionLogEntry; className?: s export default function Activity({ overseer, + restricted, view, onViewChange, onAutoApproveChange, @@ -218,6 +225,9 @@ export default function Activity({
{pendingActions.map(record => { const autoApproveTarget = + // A restricted workspace never auto-approves, so don't offer a rule that could + // only error. + !restricted && record.type === 'action' && record.gatekeeperId !== undefined && record.description.actionKind !== undefined && record.description.autoApprovable === true && @@ -327,7 +337,11 @@ export default function Activity({ )} ) : ( - + )} {confirmAutoApprove && ( @@ -351,9 +365,11 @@ export default function Activity({ function AutoApprovalPanel({ overseer, + restricted, reloadTrigger, }: { overseer: RpcStub + restricted?: boolean reloadTrigger?: number }) { const { entries, isLoading, loadError, pending, refresh, setEnabled } = useAutoApproval(overseer) @@ -466,17 +482,23 @@ function AutoApprovalPanel({ {entry.actionKind.label} - {entry.orphaned - ? 'This connection no longer offers this action; the rule still applies.' - : entry.enabled - ? 'Applied without asking' - : 'Waits for your approval'} + {restricted + // Rules never apply while the workspace is restricted (autoApprovalRule + // refuses), so don't claim they do -- but keep them listed and revocable. + ? "Won't apply: this workspace has read sensitive data, so actions always require manual approval." + : entry.orphaned + ? 'This connection no longer offers this action; the rule still applies.' + : entry.enabled + ? 'Applied without asking' + : 'Waits for your approval'} void setEnabled(entry, enabled)} /> diff --git a/packages/workshop-frontend/src/ChatInterface.tsx b/packages/workshop-frontend/src/ChatInterface.tsx index 30876d7af..ca0a1493d 100644 --- a/packages/workshop-frontend/src/ChatInterface.tsx +++ b/packages/workshop-frontend/src/ChatInterface.tsx @@ -4409,6 +4409,10 @@ function fallbackToStoredModelSelection( interface ChatInterfaceProps { workspaceId: string | undefined; overseer: RpcStub; + // True once the workspace has read restricted data (GadgetMetadata.containsRestrictedData, live + // via the metadata subscription). Latched actions are never auto-approved (the backend's + // setAutoApprovedActionKind also throws), so the always-approve affordance is suppressed. + restricted?: boolean; selectedChatId: number | null; onNavigateToChat: ( chatId: number | null, @@ -4603,6 +4607,7 @@ function getOrCreateProvisionalToolCall( function ChatInterface({ workspaceId, overseer, + restricted, selectedChatId, onNavigateToChat, onChatChangesChange, @@ -6851,8 +6856,10 @@ function ChatInterface({ // Auto-approval target: offer "Always approve this type" only when enabling a rule would // actually apply this action -- a tagged action on a connection that the gatekeeper marked // auto-approvable. (A non-auto-approvable action stays a manual gate even with a rule; an - // auto-approvable action with an existing rule wouldn't still be pending.) + // auto-approvable action with an existing rule wouldn't still be pending.) A restricted + // workspace never auto-approves, so don't offer a rule that could only error. const autoApproveTarget = + !restricted && log.gatekeeperId !== undefined && log.description.actionKind !== undefined && log.description.autoApprovable === true && // A warned action is never auto-approved (see autoApprovalRule in the backend), so don't diff --git a/packages/workshop-frontend/src/GadgetEditor.tsx b/packages/workshop-frontend/src/GadgetEditor.tsx index 5339e0ef0..5a672615a 100644 --- a/packages/workshop-frontend/src/GadgetEditor.tsx +++ b/packages/workshop-frontend/src/GadgetEditor.tsx @@ -1663,6 +1663,7 @@ export default function GadgetEditor() { key={id} workspaceId={id} overseer={overseer.stub} + restricted={metadata?.containsRestrictedData === true} selectedChatId={effectiveSelectedChatId} onNavigateToChat={navigateToChat} onChatChangesChange={setChatChanges} @@ -1823,6 +1824,7 @@ export default function GadgetEditor() {
setAutoApproveReloadTrigger(t => t + 1)} From f3237f8dc50c7b81ec6753d3b00699f4eb3f4b11 Mon Sep 17 00:00:00 2001 From: Maximo Guk <62088388+Maximo-Guk@users.noreply.github.com> Date: Mon, 24 Aug 2026 19:45:45 -0500 Subject: [PATCH 07/12] Bugfix: Fetch the approver profile before the auto-approval gates. Co-Authored-By: Claude Fable 5 --- packages/workshop-backend/src/overseer.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/workshop-backend/src/overseer.ts b/packages/workshop-backend/src/overseer.ts index 498402785..a57667dec 100644 --- a/packages/workshop-backend/src/overseer.ts +++ b/packages/workshop-backend/src/overseer.ts @@ -10056,6 +10056,11 @@ class OverseerClientInterface extends RpcTarget implements Overseer { // actions that this newly unblocks. Auto-approval rules are workspace-wide per gatekeeper. async setAutoApprovedActionKind(gatekeeperId: WorkpieceId, actionKind: ActionKind) : Promise { + // Fetch the approver's profile before the gates below: awaiting between them and the put + // would let a concurrent restricted observation latch the workspace and still persist an + // inert rule (or a concurrent removeGatekeeper slip past the existence check). + let profile = await this.#getClientProfile(); + let gatekeeper = this.impl.storage.gatekeepers.get(gatekeeperId); if (!gatekeeper) { throw new Error(`No such gatekeeper: ${gatekeeperId}`); @@ -10069,7 +10074,6 @@ class OverseerClientInterface extends RpcTarget implements Overseer { "approval and cannot be auto-approved."); } - let profile = await this.#getClientProfile(); this.impl.storage.autoApproveTags.put({ gatekeeperId, actionKind, From b2c8f579a72e930b00c020e7dd2ea210d8210062 Mon Sep 17 00:00:00 2001 From: Maximo Guk <62088388+Maximo-Guk@users.noreply.github.com> Date: Mon, 24 Aug 2026 19:45:47 -0500 Subject: [PATCH 08/12] Cleanup: Dismiss the auto-approve confirm when restricted mode latches. Co-Authored-By: Claude Fable 5 --- packages/workshop-frontend/src/Activity.tsx | 7 +++++++ packages/workshop-frontend/src/ChatInterface.tsx | 7 +++++++ 2 files changed, 14 insertions(+) diff --git a/packages/workshop-frontend/src/Activity.tsx b/packages/workshop-frontend/src/Activity.tsx index b80d2fde1..afd6680e5 100644 --- a/packages/workshop-frontend/src/Activity.tsx +++ b/packages/workshop-frontend/src/Activity.tsx @@ -135,6 +135,13 @@ export default function Activity({ actionKind: ActionKind actionLabel: string } | null>(null) + + // Dismiss an open confirmation when the workspace latches restricted mode: the affordance that + // opened it is already suppressed, and confirming could only error (setAutoApprovedActionKind + // refuses while restricted). + useEffect(() => { + if (restricted) setConfirmAutoApprove(null) + }, [restricted]) const toasts = useKumoToastManager() const { pendingActions, historyGroups, historyTotal, historyShown } = useMemo(() => { diff --git a/packages/workshop-frontend/src/ChatInterface.tsx b/packages/workshop-frontend/src/ChatInterface.tsx index ca0a1493d..89e430a51 100644 --- a/packages/workshop-frontend/src/ChatInterface.tsx +++ b/packages/workshop-frontend/src/ChatInterface.tsx @@ -6249,6 +6249,13 @@ function ChatInterface({ actionKind: ActionKind; actionLabel: string } | null >(null); + // Dismiss an open confirmation when the workspace latches restricted mode: the affordance that + // opened it is already suppressed, and confirming could only error (setAutoApprovedActionKind + // refuses while restricted). + useEffect(() => { + if (restricted) setAutoApproveConfirm(null); + }, [restricted]); + // Enable auto-approval of an action tag on its connection (gated by the confirm dialog). The // server applies the now-eligible pending action(s) via its drain, and the action state flips to // "approved" through the actions subscription -- so we don't optimistically mutate it here. From 9abdfa13aceaf8f2444c5f2a6778eaa67be9417b Mon Sep 17 00:00:00 2001 From: Maximo Guk <62088388+Maximo-Guk@users.noreply.github.com> Date: Mon, 24 Aug 2026 19:45:47 -0500 Subject: [PATCH 09/12] Cleanup: Update the observers doc for the writes-to-self carve-out. Co-Authored-By: Claude Fable 5 --- docs/observers.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/observers.md b/docs/observers.md index 24c3114c8..61d439ac6 100644 --- a/docs/observers.md +++ b/docs/observers.md @@ -606,7 +606,8 @@ its resource types. - **A — Private-only.** Non-owner observers are refused: `addObserver()` unconditionally throws. For data that must additionally never leak back out, the `containsRestrictedData` restricted - mode (no actions, no web fetches) is available separately; combined with strategy A it makes + mode (actions only to the connections that produced the data, each manually approved; no web + fetches) is available separately; combined with strategy A it makes the workspace effectively private once sensitive data is observed. `getVerifier()` must still exist (the overseer mints one on every open) but is never consulted. From 8bef9d486a6dfd55ce10c0dfa46cd54e54166068 Mon Sep 17 00:00:00 2001 From: Maximo Guk <62088388+Maximo-Guk@users.noreply.github.com> Date: Tue, 25 Aug 2026 10:14:46 -0500 Subject: [PATCH 10/12] Bugfix: Re-check the writes-to-self carve-out when applying a pending action. An action can outlive the policy it was submitted under: queued to a non-producer connection while the workspace was unlatched, it stayed pending and manually approvable after a restricted observation latched the workspace. The auto-approval drain already refused via autoApprovalRule; now the apply chokepoint (applyPendingAction) re-checks the carve-out synchronously before the gatekeeper call, so such an action can be denied but never applied. rejectAction is deliberately not gated -- denying is how the user unsticks an agent turn suspended on awaitDecision. Co-Authored-By: Claude Fable 5 --- .../__tests__/sensitive-observations.test.ts | 36 +++++++++++++++++++ packages/workshop-backend/src/overseer.ts | 22 ++++++++++-- 2 files changed, 56 insertions(+), 2 deletions(-) diff --git a/packages/integration-tests/__tests__/sensitive-observations.test.ts b/packages/integration-tests/__tests__/sensitive-observations.test.ts index 2d73867f3..7b73bfbbb 100644 --- a/packages/integration-tests/__tests__/sensitive-observations.test.ts +++ b/packages/integration-tests/__tests__/sensitive-observations.test.ts @@ -168,6 +168,42 @@ describe("sensitive observations", () => { }); }); + it.concurrent("a pre-latch pending action on another connection cannot be approved after " + + "the latch", async () => { + await withSession(async publicApi => { + const ws = await newWorkspace(publicApi, "approve-after-latch"); + + // A second connection queues an action while the workspace is still unlatched, so + // submitAction's carve-out check admits it and it pends for manual approval. + const accounts = await listConnectedAccounts(ws.aliceApi); + const account = accounts.find(a => a.vendorId === TEST_VENDOR_ID)!; + const other = await ws.overseer.newGatekeeper( + account.id, thingUrl("approve-after-latch-other")); + if (!other) throw new Error("Failed to create the second test connection"); + const otherGatekeeperId = await other.getId(); + const otherSession: any = await other.openSession(); + await expect(otherSession.doThing()).resolves.toBeUndefined(); + + // The first connection latches the workspace. The second connection is not a producer, so + // its still-pending action now violates the writes-to-self carve-out. + await expect(ws.session.readThing(true)).resolves.toContain("approve-after-latch"); + + const actions = await ws.overseer.listActions(); + const pending = actions.find( + a => a.type === "action" && a.state === "pending" && + a.gatekeeperId === otherGatekeeperId); + if (!pending) throw new Error("The pre-latch action is not pending"); + + // Approval is refused at the apply chokepoint; the action stays pending, and denying it -- + // the way to unstick a suspended agent turn -- still works. + await expect(ws.overseer.approveAction(pending.id)) + .rejects.toThrow(/only perform actions on those same connections/i); + await expect(ws.overseer.rejectAction(pending.id)).resolves.toBeUndefined(); + const after = await ws.overseer.listActions(); + expect(after.find(a => a.id === pending.id)?.state).toBe("rejected"); + }); + }); + it.concurrent("an unredeemed share link does not block a sensitive observation", async () => { await withSession(async publicApi => { const ws = await newWorkspace(publicApi, "unredeemed"); diff --git a/packages/workshop-backend/src/overseer.ts b/packages/workshop-backend/src/overseer.ts index a57667dec..30ac2d8db 100644 --- a/packages/workshop-backend/src/overseer.ts +++ b/packages/workshop-backend/src/overseer.ts @@ -4222,6 +4222,23 @@ class OverseerImpl implements AgentHooks { // was applied automatically. For an auto-approval, `resolvedBy` is the user who enabled the rule. async applyPendingAction(record: ActionRecord & {type: "action"}, resolvedBy: AiChatAuthorInfo, autoApproved: boolean): Promise { + // Writes-to-self carve-out, re-checked at apply time: an action can outlive the policy it was + // submitted under (queued before a restricted observation latched the workspace, approved + // after), and the approval surfaces tell the user a latched workspace acts only on its + // restricted producers -- so the invariant must hold here, at the one place an action + // transitions to "approved", not just in submitAction. Checked synchronously before the facet + // call so a refused action is left untouched. The auto-approval drain already refuses via + // autoApprovalRule (nothing auto-applies while latched); this covers manual approval. + // rejectAction is deliberately NOT gated -- denying is how the user unsticks an agent turn + // suspended on awaitDecision. + if (this.storage.prohibitAllSharing.get() && + !this.restrictedProducerIds().has(record.gatekeeperId)) { + throw new Error( + "This workspace has observed sensitive data from other connections. To prevent leaks, " + + "it may only perform actions on those same connections; this pending action targets " + + "another connection, so it can only be denied."); + } + let gatekeeper = this.getGatekeeperFacet(record.gatekeeperId); await gatekeeper.applyAction(record.action); record.state = "approved"; @@ -4623,8 +4640,9 @@ class OverseerImpl implements AgentHooks { // set. Built-in tool observations are skipped: they name no connection, and the // BUILTIN_TOOL_GATEKEEPER_ID sentinel could never match a gatekeeper record (built-ins also // never latch). The callers are cold paths (connection removal, sharing mutators) plus - // submitAction -- but the latter scans only while latched, and the auto-approval drainer - // already full-scans the log per drain, so the scan is fine where it runs. + // submitAction and applyPendingAction -- but both scan only while latched (the unlatched path + // short-circuits before the scan), latched manual approvals are human-bounded, and the + // auto-approval drainer already full-scans the log per drain, so the scan is fine where it runs. restrictedProducerIds(): Set { let producers = new Set(); for (let record of this.storage.actions.list()) { From 07ebc1227bc1d96818c11916ec7b1a7ff955124b Mon Sep 17 00:00:00 2001 From: Maximo Guk <62088388+Maximo-Guk@users.noreply.github.com> Date: Tue, 25 Aug 2026 10:16:30 -0500 Subject: [PATCH 11/12] Bugfix: Refuse to remove a connection that has pending approval requests. A pending action is resolvable only through the gatekeeper facet that removeGatekeeper deletes -- approveAction (via applyPendingAction) and rejectAction both dereference it -- so removing the connection stranded the record "pending" forever and suspended an awaitDecision agent turn with it. The guard sits in removeGatekeeper itself, synchronous with the delete, so it covers GatekeeperClientImpl.remove() and any future caller; together with submitAction's existence check (which covers the submit side), a pending action's facet now always exists. The ambient-capsule reconcile skips (and later retries) a stale capsule with pending actions the same way it already skips a shared restricted producer, rather than throwing out of open(). Co-Authored-By: Claude Fable 5 --- .../__tests__/sensitive-observations.test.ts | 21 +++++++++++ packages/workshop-backend/src/overseer.ts | 35 +++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/packages/integration-tests/__tests__/sensitive-observations.test.ts b/packages/integration-tests/__tests__/sensitive-observations.test.ts index 7b73bfbbb..b5c5ba856 100644 --- a/packages/integration-tests/__tests__/sensitive-observations.test.ts +++ b/packages/integration-tests/__tests__/sensitive-observations.test.ts @@ -626,6 +626,27 @@ describe("sensitive observations", () => { }); }); + it.concurrent("a connection with pending approval requests cannot be removed", async () => { + await withSession(async publicApi => { + const ws = await newWorkspace(publicApi, "remove-pending"); + + // Queue an action; it pends for manual approval. Removing the connection now would delete + // the facet both approval and rejection resolve through, stranding the record forever. + await expect(ws.session.doThing()).resolves.toBeUndefined(); + const gatekeeper = await ws.overseer.getGatekeeperById(ws.gatekeeperId); + await expect(gatekeeper.remove()).rejects.toThrow(/pending approval requests/i); + // The refused removal left the connection intact. + await expect(ws.session.readThing()).resolves.toContain("remove-pending"); + + // Denying the action resolves it, which unblocks the removal. + const actions = await ws.overseer.listActions(); + const pending = actions.find(a => a.type === "action" && a.state === "pending"); + if (!pending) throw new Error("The submitted action is not pending"); + await ws.overseer.rejectAction(pending.id); + await expect(gatekeeper.remove()).resolves.toBeUndefined(); + }); + }); + it.concurrent("a latched connection cannot be removed while the workspace is shared", async () => { await withSession(async publicApi => { diff --git a/packages/workshop-backend/src/overseer.ts b/packages/workshop-backend/src/overseer.ts index 30ac2d8db..d3cf43119 100644 --- a/packages/workshop-backend/src/overseer.ts +++ b/packages/workshop-backend/src/overseer.ts @@ -4320,6 +4320,19 @@ class OverseerImpl implements AgentHooks { // no gadget's env retains a dangling entry. (This is distinct from merely unbinding it from one // gadget -- GadgetClient.unbind() -- which leaves the gatekeeper alive, possibly orphaned.) removeGatekeeper(id: number) { + // A pending action is resolvable only through the facet this method deletes -- both + // applyPendingAction and rejectAction dereference it -- so removal would strand the record + // "pending" forever and suspend an awaitDecision agent turn with it. Refuse before any + // mutation (binding edges are severed below); submitAction's existence check covers the + // submit side. Synchronous with the delete, like submitAction's check-and-put block, so one + // always sees the other. The addGatekeeper failure cleanup passes vacuously: no session has + // ever been handed out there, so no action can name the id. + if (this.hasPendingActions(id)) { + throw new Error( + "This connection cannot be removed while it has pending approval requests. Approve or " + + "deny them first."); + } + for (let gadget of Array.from(this.storage.gadgets.list())) { let names = Object.entries(gadget.bindings) .filter(([, edge]) => edge.target === id) @@ -4655,6 +4668,19 @@ class OverseerImpl implements AgentHooks { return producers; } + // True if any pending approval request names connection `id`. Scans the action log like + // restrictedProducerIds above, and for the same reason it's acceptable: the callers are cold + // paths (connection removal and the ambient reconcile). Used to refuse removing a connection + // whose pending actions could then never be resolved -- see removeGatekeeper. + hasPendingActions(id: WorkpieceId): boolean { + for (let record of this.storage.actions.list()) { + if (record.type === "action" && record.state === "pending" && record.gatekeeperId === id) { + return true; + } + } + return false; + } + // True if removing gatekeeper `id` is blocked because it anchors restricted-data verification: // the workspace is latched, `id` is a restricted producer (or the producer set is unexpectedly // empty -- see below), and the sharing graph still has collaborators or outstanding share @@ -6388,6 +6414,15 @@ class OverseerImpl implements AgentHooks { gatekeeperId: gk.id, vendorId: gk.creationSpec.vendorId, }); + } else if (this.hasPendingActions(gk.id)) { + // removeGatekeeper refuses while approval requests are pending (resolving them needs the + // facet it deletes), so defer this stale capsule to a later reconcile rather than throw + // out of open(). Not added to `bound`, same as the restricted-producer skip above. + this.logger.warn("skipping removal of stale ambient capsule with pending actions", { + event: "singleton.capsules.reconcile.pending.actions", + gatekeeperId: gk.id, + vendorId: gk.creationSpec.vendorId, + }); } else { this.removeGatekeeper(gk.id); } From 7ef69f6f1a89627ae25b46dcefc682900c0be69b Mon Sep 17 00:00:00 2001 From: Maximo Guk <62088388+Maximo-Guk@users.noreply.github.com> Date: Tue, 25 Aug 2026 10:18:30 -0500 Subject: [PATCH 12/12] Cleanup: Associate operator warnings with the approve/deny controls. Activity's review rows and the chat's inline pending row render the approve/deny buttons before the operator-warnings block in DOM order, so a screen reader reading linearly reached the decision before the warning meant to inform it. Give the warnings block an id and point both ResolveButtons' aria-describedby at it: the warning is announced as the buttons' description on focus, in every presentation (the association rides the chat surface's shared controls JSX, covering the blocking callout too -- which already rendered warnings first). The warnings aren't focusable, so tab order is unaffected; the visual design is unchanged. AlwaysApproveButton needs nothing: both surfaces gate it on the absence of warnings. Co-Authored-By: Claude Fable 5 --- packages/workshop-frontend/src/Activity.tsx | 14 ++++++++++---- packages/workshop-frontend/src/ChatInterface.tsx | 9 ++++++++- .../src/components/ResolveButton.tsx | 8 ++++++++ 3 files changed, 26 insertions(+), 5 deletions(-) diff --git a/packages/workshop-frontend/src/Activity.tsx b/packages/workshop-frontend/src/Activity.tsx index afd6680e5..135b5e794 100644 --- a/packages/workshop-frontend/src/Activity.tsx +++ b/packages/workshop-frontend/src/Activity.tsx @@ -1,4 +1,4 @@ -import { useEffect, useMemo, useRef, useState, type ReactNode } from 'react' +import { useEffect, useId, useMemo, useRef, useState, type ReactNode } from 'react' import { Switch, useKumoToastManager } from '@cloudflare/kumo' import { CaretRight, Check, Eye, Lightning, ShieldCheck, ShieldWarning } from '@phosphor-icons/react' import { RpcStub } from 'capnweb' @@ -538,6 +538,12 @@ function ReviewRequest({ }) { const resourceUrl = safeExternalUrl(record.resourceUrl) const operatorWarnings = record.type === 'action' ? record.description.operatorWarnings ?? [] : [] + // Referenced by the approve/deny buttons' aria-describedby: the warnings render below the + // controls, so screen readers wouldn't otherwise reach them before a decision. (useId, unlike + // the chat surface's action-id-derived ids, so the two never collide when both show the same + // action.) + const warningsDomId = useId() + const warningsId = operatorWarnings.length > 0 ? warningsDomId : undefined return (
@@ -575,13 +581,13 @@ function ReviewRequest({ {onAlwaysApprove && ( )} - - + +
{operatorWarnings.length > 0 && ( -
+
{operatorWarnings.map((warning, i) => (
0 ? `action-warnings-${msg.actionId}` : undefined; const warningStrip = operatorWarnings.length > 0 ? ( -
+
{operatorWarnings.map((warning, i) => (
void resolveAction(msg.actionId, "deny")} disabled={isProc} + describedBy={warningsId} /> void resolveAction(msg.actionId, "approve")} disabled={isProc} + describedBy={warningsId} /> ) : null; diff --git a/packages/workshop-frontend/src/components/ResolveButton.tsx b/packages/workshop-frontend/src/components/ResolveButton.tsx index 26b51cfa6..4be859386 100644 --- a/packages/workshop-frontend/src/components/ResolveButton.tsx +++ b/packages/workshop-frontend/src/components/ResolveButton.tsx @@ -5,11 +5,18 @@ export function ResolveButton({ variant = 'quiet', disabled, onClick, + describedBy, }: { tone: 'approve' | 'deny' variant?: 'quiet' | 'filled' disabled: boolean onClick: MouseEventHandler + /** + * Id of the action's operator-warnings block, when it has one, so screen readers announce the + * warning as the button's description on focus. The warnings render below the controls in some + * presentations, so DOM order alone doesn't reach them before a decision. + */ + describedBy?: string }) { const toneClassName = variant === 'filled' ? 'h-7 bg-kumo-brand px-3 text-white enabled:hover:opacity-90' @@ -22,6 +29,7 @@ export function ResolveButton({ type="button" onClick={onClick} disabled={disabled} + aria-describedby={describedBy} className={`flex cursor-pointer items-center rounded-md text-[12px] font-medium tracking-[-0.15px] transition-colors disabled:cursor-not-allowed disabled:opacity-40 ${toneClassName}`} > {tone === 'approve' ? 'Approve' : 'Deny'}