diff --git a/docs/observers.md b/docs/observers.md index 54683f2f9..61d439ac6 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 @@ -604,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. 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..129ac63c7 --- /dev/null +++ b/packages/integration-tests/__tests__/auto-approval-policy.test.ts @@ -0,0 +1,164 @@ +// 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. 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/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"; +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 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 => { + 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..b5c5ba856 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,13 +149,61 @@ 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"); }); }); + 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"); @@ -576,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/integration-tests/fixtures/gatekeeper-test/src/test-gatekeeper.ts b/packages/integration-tests/fixtures/gatekeeper-test/src/test-gatekeeper.ts index 19a3f907c..45b1d283b 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,17 @@ export class TestSession extends RpcTarget { return `the contents of ${this.#title}`; } - async doThing(): Promise { + async doThing(opts?: { autoApprovable?: boolean; warnings?: string[] }): 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 } : {}), + ...(opts?.warnings ? { operatorWarnings: opts.warnings } : {}), }); } @@ -270,6 +277,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 +307,7 @@ export class TestGatekeeper } async getAutoApprovableActions(): Promise { - return []; + return [POKE_ACTION_KIND]; } async startSession(approvalQueue: RpcStub): Promise { @@ -336,14 +345,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."); } } diff --git a/packages/workshop-backend/__tests__/auto-approval.test.ts b/packages/workshop-backend/__tests__/auto-approval.test.ts index 0b4ce7389..73e2f4597 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()({ @@ -27,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, @@ -42,6 +48,7 @@ function putAction( implementsRevert: true, actionKind: { tag: opts.actionTag ?? "edit", label: "Edits" }, autoApprovable: opts.autoApprovable ?? true, + ...(opts.operatorWarnings ? { operatorWarnings: opts.operatorWarnings } : {}), }, }); } @@ -211,4 +218,99 @@ describe("AutoApprovalDrainer.drain", () => { expect(getAction(storage, 1).state).toBe("approved"); 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); + 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 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); + 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..e6feb1324 --- /dev/null +++ b/packages/workshop-backend/__tests__/restricted-writes-to-self.test.ts @@ -0,0 +1,180 @@ +// 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). 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 +// 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 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) => { + 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..579d1066f 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,37 @@ 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 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. + */ +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 (description.operatorWarnings !== undefined && description.operatorWarnings.length > 0) { + return undefined; + } + if (storage.prohibitAllSharing.get()) return undefined; + return storage.autoApproveTags.get(`${gatekeeperId}:${tag}`); } /** @@ -55,8 +87,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, 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. @@ -65,11 +97,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..d3cf43119 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 @@ -4220,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"; @@ -4301,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) @@ -4620,8 +4652,10 @@ 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 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()) { @@ -4634,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 @@ -4794,23 +4841,46 @@ class OverseerImpl implements AgentHooks { async submitAction(gatekeeperId: number, action: number, description: ActionDescription, caller: GatekeeperCaller) : Promise { - if (this.storage.prohibitAllSharing.get()) { + // 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 workspace has observed sensitive data. To prevent leaks, the workspace is prohibited " + - "from performing actions."); + "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, 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 + // 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 from other connections. To prevent leaks, " + + "it may only perform actions on those same connections."); } 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", @@ -4821,10 +4891,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. @@ -6345,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); } @@ -10031,12 +10109,24 @@ 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}`); } - let profile = await this.#getClientProfile(); + // 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."); + } + this.impl.storage.autoApproveTags.put({ gatekeeperId, actionKind, @@ -10062,6 +10152,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-frontend/src/Activity.tsx b/packages/workshop-frontend/src/Activity.tsx index f4ab21e53..135b5e794 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 { useEffect, useId, 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' @@ -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, @@ -128,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(() => { @@ -218,9 +232,15 @@ 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 + 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, @@ -324,7 +344,11 @@ export default function Activity({ )} ) : ( - + )} {confirmAutoApprove && ( @@ -348,9 +372,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) @@ -463,17 +489,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)} /> @@ -505,6 +537,13 @@ function ReviewRequest({ onAlwaysApprove?: () => void }) { 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 (
@@ -542,11 +581,25 @@ function ReviewRequest({ {onAlwaysApprove && ( )} - - + +
+ {operatorWarnings.length > 0 && ( +
+ {operatorWarnings.map((warning, i) => ( +
+ + {warning} +
+ ))} +
+ )} + {record.description.description && (

{record.description.description} @@ -572,6 +625,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 +661,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..e26313b12 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, @@ -4408,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, @@ -4602,6 +4607,7 @@ function getOrCreateProvisionalToolCall( function ChatInterface({ workspaceId, overseer, + restricted, selectedChatId, onNavigateToChat, onChatChangesChange, @@ -6243,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. @@ -6822,6 +6835,28 @@ 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 ?? []; + // Referenced by the approve/deny buttons' aria-describedby: the inline pending row renders + // the warnings below the controls, so screen readers wouldn't otherwise reach them before a + // decision. Deterministic (not useId -- this is a closure, not a component), keyed by the + // action id, which is unique within the page. + const warningsId = operatorWarnings.length > 0 ? `action-warnings-${msg.actionId}` : undefined; + const warningStrip = operatorWarnings.length > 0 ? ( +

+ {operatorWarnings.map((warning, i) => ( +
+ + {warning} +
+ ))} +
+ ) : null; const stateLabel = isApproved ? "Approved" : isRejected @@ -6833,10 +6868,15 @@ 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 + 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, @@ -6863,12 +6903,14 @@ function ChatInterface({ tone="deny" onClick={() => void resolveAction(msg.actionId, "deny")} disabled={isProc} + describedBy={warningsId} /> void resolveAction(msg.actionId, "approve")} disabled={isProc} + describedBy={warningsId} /> ) : null; @@ -6915,6 +6957,7 @@ function ChatInterface({ {resourceMeta}
+ {warningStrip}
@@ -6975,6 +7018,7 @@ function ChatInterface({ )} {showDescription && (
+ {warningStrip}
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)} 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'} 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..233da0acd 100644 --- a/packages/workshop-shared/src/gatekeeper.ts +++ b/packages/workshop-shared/src/gatekeeper.ts @@ -1077,8 +1077,15 @@ 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), 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, @@ -1181,6 +1188,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 //