diff --git a/.agents/skills/write-gatekeeper/SKILL.md b/.agents/skills/write-gatekeeper/SKILL.md index 7416bd12..1e470f73 100644 --- a/.agents/skills/write-gatekeeper/SKILL.md +++ b/.agents/skills/write-gatekeeper/SKILL.md @@ -243,7 +243,7 @@ async getVerifier(): Promise> { Strategy is chosen **per `Gatekeeper` DO class / binding**, not per package — one package may use several (e.g. Google: Gmail=A, Doc=B, BigQuery=C). -- **A — Private-only.** `addObserver()` always throws; `removeObserver()` is a no-op. `getVerifier()` must still exist (the overseer mints it) but is never consulted. Use when the resource is too sensitive to share and there is no per-observer access oracle (e.g. a personal Gmail mailbox). +- **A — Private-only.** `addObserver()` always throws; `removeObserver()` is a no-op. `getVerifier()` must still exist (the overseer mints it) but is never consulted. Use when the resource is too sensitive to share and there is no per-observer access oracle (e.g. a personal Gmail mailbox). For truly sensitive data, also mark each observation with `ObservationDescription.containsRestrictedData: true`: the workspace then refuses sensitive observations while any unverified collaborator has access (with strategy A that is every collaborator) and latches into a restricted mode that blocks all actions and public web fetches, so the data cannot leak back out through other gatekeepers. - **B — ACL check (single unit).** The binding is one atomic resource; sub-resources inherit its ACL. `addObserver()` calls a verifier method to confirm the observer can access it and throws otherwise; `removeObserver()` is a no-op; nothing is tracked and no `excludeObservers` is ever needed. Use for repo / document / page / team / single-project bindings. - **C — Data-set tracking.** The binding spans sub-resources with **distinct ACLs**, and there is a **per-observer access oracle** for each. The DO logs the data sets actually observed and the current observers; `addObserver()` verifies the observer against **every** logged set (plus a coarse membership baseline) and **stores their verifier**; each later observation that first touches a **new** set re-checks all stored observers and sets `excludeObservers` for any who fail. Use for workspace / organization / dataset-spanning bindings. - **D — Low-stakes.** `addObserver()` / `removeObserver()` are no-ops; `getVerifier()` returns a trivial verifier with a no-op public method such as `verify(): void {}` (an empty `WorkerEntrypoint` is not registered in `ctx.exports`). Use when any collaborator may observe (personal, low-stakes services). diff --git a/docs/observers.md b/docs/observers.md index d6c5534c..07fae70d 100644 --- a/docs/observers.md +++ b/docs/observers.md @@ -30,14 +30,19 @@ Gadgets enforce a core security invariant (see `overview.md` §"Security Model") > able to read that information will also be prohibited from interacting with the Gadget, > to prevent data leaks. -Today the only mechanism enforcing this is the blunt **`prohibitAllSharing`** flag -(`packages/workshop-shared/src/gatekeeper.ts`, `ObservationDescription.prohibitAllSharing`). -When a gatekeeper marks an observation as maximally sensitive, the Gadget can no longer be -shared with *anyone*, and it drops into "lockdown" (no further actions, no web fetches). This -is a deliberate stopgap — it cannot express "this data may be shared, but only with people who -*also* have access to it." - -This feature replaces that all-or-nothing posture with a per-user, gatekeeper-mediated check: +Before observers landed, the only mechanism enforcing this was the blunt +**`prohibitAllSharing`** flag (since renamed **`containsRestrictedData`**; +`packages/workshop-shared/src/gatekeeper.ts`, `ObservationDescription.containsRestrictedData`). +When a gatekeeper marked an observation as maximally sensitive, the Gadget could no longer be +shared with *anyone*, and it dropped into "lockdown" (no further actions, no web fetches). That +was a deliberate stopgap — it could not express "this data may be shared, but only with people +who *also* have access to it." Today the flag still latches the workspace into a restricted mode +(no actions, no web fetches), but sharing is governed by the per-user, gatekeeper-mediated check +described here: a sensitive observation is blocked only if some current collaborator has not been +verified (via `addObserver`) against the gatekeeper producing it — see the coverage guard, +`#assertSensitiveObservationCoverage`, in `overseer.ts`. + +This feature is that per-user, gatekeeper-mediated check: - **Observers.** Every non-owner who can see data the Gadget read is an *observer*. When a user becomes an observer, each relevant gatekeeper is asked — via `Gatekeeper.addObserver()` — to @@ -93,7 +98,7 @@ This feature replaces that all-or-nothing posture with a per-user, gatekeeper-me | Overseer DO, `open()` auth entry point | `packages/workshop-backend/src/overseer.ts:2714` | | Server `openGadget` path | `packages/workshop-backend/src/server.ts:206` | | Role resolution / permission graph | `packages/workshop-backend/src/sharing.ts` (`getEffectiveRole`, `computeEffectiveRoles`, `hasAnyShares`) | -| `prohibitAllSharing` enforcement | `overseer.ts:1171` (`authorizeObservation`), `:1207` (web fetch), `:1258` (`submitAction`) | +| `containsRestrictedData` enforcement | `overseer.ts` (`authorizeObservation` coverage guard, `getWebFetchEnv`, `submitAction`) | | Observation recording | `overseer.ts:1169` `authorizeObservation()`; `ApprovalQueueImpl` `overseer.ts:4856` | | Gatekeeper storage record | `overseer.ts:110` `GatekeeperRecord` (has `creationSpec.vendorId`) | | `GatekeeperCreationSpec` | `packages/workshop-shared/src/api.ts:1345` | @@ -227,8 +232,9 @@ type ObserverAccountChoice = { ### Step 3 — Overseer: observer configuration & re-verification at `open()` Hook into `open()` in the non-owner branch, after `effectiveRole` is confirmed and before -constructing the client interface. Keep the existing `prohibitAllSharing` short-circuit ahead of -this -- lockdown still wins. The `NeedsConnections` signal is produced only *after* a valid role is +constructing the client interface. (There is no longer a `containsRestrictedData` short-circuit +ahead of this: observer verification *is* the enforcement for sensitive data at open() time.) +The `NeedsConnections` signal is produced only *after* a valid role is confirmed, so it never reveals a workspace's gatekeeper or resource metadata to an unauthorized user. @@ -402,8 +408,13 @@ already in the JSDoc in `gatekeeper.ts`; add anything missing there rather than 3. **Underlying resource access revoked** — caught at the next open because `addObserver` re-runs the live check and throws; the open is denied. Consistent with the lazy-revocation model in `sharing.ts`. -4. **`prohibitAllSharing` interaction** — unchanged and still authoritative: if set, no non-owner - can open at all (`overseer.ts:2770`). Observer checks only matter when sharing is allowed. +4. **`containsRestrictedData` interaction** — no longer a wholesale block. A sensitive + observation is admitted only if every current collaborator holds an observer record covering + the producing gatekeeper (`#assertSensitiveObservationCoverage` in `authorizeObservation`); + otherwise it is blocked with a message naming the unverified collaborator. 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 still latches the workspace into a + restricted mode that blocks actions and web fetches. 5. **Owner adds a new binding after sharing** — existing observers see an incremental modal for just the new binding on their next open, and may be denied if they lack access to the new resource (inherent to the security model). @@ -474,8 +485,10 @@ gatekeeper package — a single package (e.g. `gatekeeper-google`) may use sever its resource types. - **A — Private-only.** Non-owner observers are refused: `addObserver()` unconditionally throws. - This is the replacement for today's reliance on `prohibitAllSharing` for these resources (the - `prohibitAllSharing` lockdown mechanism itself is unchanged and remains available separately). + This is the replacement for the old reliance on wholesale sharing prohibition for these + resources (the `containsRestrictedData` restricted mode -- no actions, no web fetches -- is + unchanged and remains 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. - **B — ACL check (single unit).** The resource is treated as one atomic unit. diff --git a/docs/sharing.md b/docs/sharing.md index 8bb4fc46..46360231 100644 --- a/docs/sharing.md +++ b/docs/sharing.md @@ -156,7 +156,7 @@ Authorization is only checked at `open()`, so a session that is *already* open i Two precautions surround the abort (`OverseerImpl.scheduleRevocationRestart`): the severed edge is flushed with `ctx.storage.sync()` first (because `ctx.abort()` does not respect the output gate, a restart could otherwise come back with the change lost), and the abort is delayed ~100ms so the triggering RPC's response reaches the caller -- typically the owner, who is also connected -- before their own connection drops. The disconnect reaches the browser through the existing `notifyClosed` plumbing: when the Overseer DO aborts, the per-session `notifyClosed` stub is disposed without being called, which `AuthenticatedApiImpl` treats as a lost connection and reacts to by killing the browser WebSocket, forcing a reconnect. -Note this is only needed for removals/downgrades. Granting or raising access never strands anyone, and `prohibitAllSharing` cannot strand a session either: an observation that would set that flag is *blocked* (rather than applied) if the gadget is already shared, so the flag only ever flips to true on a gadget with no other sessions to evict. +Note this is only needed for removals/downgrades. Granting or raising access never strands anyone, and `containsRestrictedData` cannot strand a session either: an observation carrying that flag is *blocked* (rather than applied) unless every current collaborator is already a verified observer of the producing gatekeeper, so no live session ever belongs to someone the flag would newly exclude. ## Future work diff --git a/packages/gatekeeper-google/src/google.ts b/packages/gatekeeper-google/src/google.ts index 280e38f6..6f66f3fc 100644 --- a/packages/gatekeeper-google/src/google.ts +++ b/packages/gatekeeper-google/src/google.ts @@ -1963,8 +1963,9 @@ export class GmailGatekeeperImpl extends DurableObject): Promise { throw new Error( @@ -3624,7 +3625,7 @@ class BigQuerySessionImpl extends RpcTarget implements BigQuerySession { `Referenced tables: ${estimate.referencedTables.join(", ")}\n` + `Estimated bytes processed: ${estimate.bytesProcessed.toLocaleString()}\n` + `Maximum bytes billed: ${maxBytes.toLocaleString()}.`, - prohibitAllSharing: true, + containsRestrictedData: true, }); let result = await this.#api.query(billingProject, sql, { @@ -3656,7 +3657,7 @@ class BigQuerySessionImpl extends RpcTarget implements BigQuerySession { description: `Estimated bytes processed: ${estimate.bytesProcessed.toLocaleString()}\n` + `Referenced tables: ${estimate.referencedTables.join(", ") || "(none)"}`, - prohibitAllSharing: true, + containsRestrictedData: true, }); return estimate; @@ -3668,7 +3669,7 @@ class BigQuerySessionImpl extends RpcTarget implements BigQuerySession { await this.#authorizeDatasets([], { title: "Get BigQuery project", description: `Returned the scoped project: \`${this.#scopedProjectId}\`.`, - prohibitAllSharing: true, + containsRestrictedData: true, }); return result; } @@ -3689,7 +3690,7 @@ class BigQuerySessionImpl extends RpcTarget implements BigQuerySession { await this.#authorizeDatasets([{ projectId: p, datasetId: this.#scopedDatasetId }], { title: `List datasets in ${p}`, description: `Returned scoped dataset \`${p}.${this.#scopedDatasetId}\` (1 dataset).`, - prohibitAllSharing: true, + containsRestrictedData: true, }); return [dataset]; } @@ -3699,7 +3700,7 @@ class BigQuerySessionImpl extends RpcTarget implements BigQuerySession { await this.#authorizeDatasets(result.map(ds => ({ projectId: p, datasetId: ds.datasetId })), { title: `List datasets in ${p}`, description: `Listed ${result.length} dataset(s) in \`${p}\`.`, - prohibitAllSharing: true, + containsRestrictedData: true, }); return result; } @@ -3725,7 +3726,7 @@ class BigQuerySessionImpl extends RpcTarget implements BigQuerySession { await this.#authorizeDatasets([{ projectId: p, datasetId: d }], { title: `List tables in ${p}.${d}`, description: `Returned scoped table \`${p}.${d}.${this.#scopedTableId}\` (1 table).`, - prohibitAllSharing: true, + containsRestrictedData: true, }); return [table]; } @@ -3734,7 +3735,7 @@ class BigQuerySessionImpl extends RpcTarget implements BigQuerySession { await this.#authorizeDatasets([{ projectId: p, datasetId: d }], { title: `List tables in ${p}.${d}`, description: `Listed ${result.length} table(s) in \`${p}.${d}\`.`, - prohibitAllSharing: true, + containsRestrictedData: true, }); return result; } @@ -3771,7 +3772,7 @@ class BigQuerySessionImpl extends RpcTarget implements BigQuerySession { title: `Describe ${p}.${d}.${t}`, description: `Described table \`${p}.${d}.${t}\` (${result.schema.length} columns).`, - prohibitAllSharing: true, + containsRestrictedData: true, }); return result; } diff --git a/packages/gatekeeper-mcp/README.md b/packages/gatekeeper-mcp/README.md index 794231cb..5b12aa32 100644 --- a/packages/gatekeeper-mcp/README.md +++ b/packages/gatekeeper-mcp/README.md @@ -171,8 +171,8 @@ rules. A Gadget bound to an MCP server can only be opened by its owner: `addObserver` refuses unconditionally. Being able to authenticate to a server is not evidence of being allowed to see what the *owner* read from it, and the Gadget runs on the owner's credentials throughout. Writes still -work — the alternative, marking every observation `prohibitAllSharing`, would latch a lockdown that -blocks every action for the rest of the session. See +work — the alternative, marking every observation `containsRestrictedData`, would latch a +restricted mode that blocks every action for the rest of the session. See [`sharing-policy.ts`](../mcp-shared/src/sharing-policy.ts). To share the work rather than the binding, publish the Gadget as a blueprint and let each person @@ -200,9 +200,10 @@ connect their own server. compatibility flag in `wrangler.jsonc`, which makes workerd reject reserved IP ranges after resolution on every request and redirect hop. It does not apply under `wrangler dev`, which is what keeps `MCP_ALLOW_INSECURE` usable locally. -- **Sharing UI reports late.** `GadgetMetadata.sharingProhibited` derives only from - `prohibitAllSharing`, so creating a share key appears to succeed and fails when the recipient - opens it. Fixing this needs a kernel change. +- **Sharing UI reports late.** `GadgetMetadata.containsRestrictedData` derives only from + `ObservationDescription.containsRestrictedData`, so creating a share key appears to succeed and + fails when the recipient opens it (their observer verification is refused). Fixing this needs a + kernel change. ## Layout diff --git a/packages/integration-tests/__tests__/sensitive-observations.test.ts b/packages/integration-tests/__tests__/sensitive-observations.test.ts new file mode 100644 index 00000000..d5c7c100 --- /dev/null +++ b/packages/integration-tests/__tests__/sensitive-observations.test.ts @@ -0,0 +1,254 @@ +// Tests for the sensitive-data (`containsRestrictedData`) observation policy. +// +// A sensitive observation used to be blocked outright whenever the workspace had any share at all, +// and latched a lockdown that also blocked all future sharing. Observer verification replaced the +// sharing side of that: a sensitive observation is now blocked only while some *current +// collaborator* has not been verified (via `addObserver`) against the gatekeeper producing it, and +// sharing stays available afterwards -- recipients are verified when they open. The restricted-mode +// half is unchanged: once latched, the workspace may not perform actions (nor 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. + +import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import type { RpcStub } from "capnweb"; +import type { AuthenticatedApi, Overseer, PublicApi } from "@gadgets/workshop-shared/api"; +import { + startTestGatekeeperHarness, TEST_GATEKEEPER_WORKER, TEST_VENDOR_ID, type Harness, +} from "../src/harness.js"; +import { + accountLabel, connect, listConnectedAccounts, MAX_OBSERVER_PROMPTS, nextUsernames, + ObserverConfigRecorder, signUp, stubFor, waitFor, type ConnectedAccount, +} from "../src/rpc-client.js"; +import { NetworkInterceptor } from "../src/network-interceptor.js"; + +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](); + } +} + +function thingUrl(name: string): string { + return `https://gadgets-test.example/things/${name}`; +} + +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; + }); +} + +/** Tell the fixture gatekeeper whether to admit `label` as an observer. */ +async function setVerifyOutcome( + label: string, outcome: { allow: true } | { allow: false; reason: string }): Promise { + const res = await harness.fetchWorker( + TEST_GATEKEEPER_WORKER, "http://gatekeeper-test.test/control/verify-outcome", + { method: "POST", body: JSON.stringify({ label, ...outcome }) }); + if (res.status !== 204) { + throw new Error(`Setting the verify outcome failed with ${res.status}: ${await res.text()}`); + } +} + +type Workspace = { + gadgetId: string; + overseer: RpcStub; + aliceApi: RpcStub; + /** The fixture session bound to the workspace's (first) gatekeeper. */ + session: any; + gatekeeperId: number; +}; + +// Alice creates a workspace bound to one Test Thing and opens a session on its gatekeeper. Every +// test starts here; collaborators and links are layered on per test. +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, thingUrl(thingName)); + if (!gatekeeper) throw new Error("Failed to create the test connection"); + const gatekeeperId = await gatekeeper.getId(); + const session = await gatekeeper.openSession(); + const { id: gadgetId } = await overseer.getMetadata(); + return { gadgetId, overseer, aliceApi, session, gatekeeperId }; +} + +// Sign Bob up, add him as a collaborator, and give him his own fixture account. +async function addBob(publicApi: RpcStub, ws: Workspace): Promise<{ + bobApi: RpcStub; + bobAccount: ConnectedAccount; + bobLabel: string; +}> { + const [bob] = nextUsernames("bob"); + const bobApi = await signUp(publicApi, bob); + const bobAccount = await provisionAccount(bobApi); + const collaborator = await ws.overseer.addCollaborator(bob, "build"); + if (!collaborator) throw new Error(`Failed to share the gadget with ${bob}`); + return { bobApi, bobAccount, bobLabel: accountLabel(bobAccount) }; +} + +// Bob opens the workspace, answering observer prompts with his own account. This is what writes +// his observer record, i.e. verifies him against every in-scope gatekeeper. +async function bobOpens(ws: Workspace, bobApi: RpcStub, + bobAccount: ConnectedAccount): Promise> { + const recorder = new ObserverConfigRecorder().alwaysChoose(bobAccount.id, MAX_OBSERVER_PROMPTS); + const callback = stubFor(recorder); + try { + return await bobApi.openGadget(ws.gadgetId, undefined, callback); + } finally { + callback[Symbol.dispose](); + } +} + +describe("sensitive observations", () => { + it.concurrent("latch restricted mode: actions are blocked and metadata reports it", async () => { + await withSession(async publicApi => { + const ws = await newWorkspace(publicApi, "latch"); + + // Before the latch, actions submit fine and metadata is clean. + await expect(ws.session.doThing()).resolves.toBeUndefined(); + expect((await ws.overseer.getMetadata()).containsRestrictedData).toBeFalsy(); + + 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); + // 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("an unredeemed share link does not block a sensitive observation", async () => { + await withSession(async publicApi => { + const ws = await newWorkspace(publicApi, "unredeemed"); + await ws.overseer.createShareLink("build", "never redeemed"); + + // Nobody has redeemed the link, so nobody unverified can be watching: the observation + // proceeds. (Redemption happens inside open(), where observer verification gates it.) + await expect(ws.session.readThing(true)).resolves.toContain("unredeemed"); + }); + }); + + it.concurrent("sharing stays available after the latch", async () => { + await withSession(async publicApi => { + const ws = await newWorkspace(publicApi, "share-after"); + await expect(ws.session.readThing(true)).resolves.toContain("share-after"); + + // All three sharing RPCs used to throw "the workspace cannot be shared" once latched. + const [carol] = nextUsernames("carol"); + await signUp(publicApi, carol); + await expect(ws.overseer.addCollaborator(carol, "build")).resolves.toMatchObject({ + profile: expect.objectContaining({ id: expect.any(String) }), + }); + const { linkId } = await ws.overseer.createShareLink("use", "post-latch"); + await expect(ws.overseer.newShareLinkKey(linkId)).resolves.toMatchObject({ + key: expect.any(String), + }); + }); + }); + + it.concurrent("an unverified collaborator blocks a sensitive observation", async () => { + await withSession(async publicApi => { + const ws = await newWorkspace(publicApi, "unverified"); + await addBob(publicApi, ws); + + // Bob has access but has never opened, so he holds no observer record for this gatekeeper. + // He may hold a live session the moment he does open, so the observation must not proceed. + await expect(ws.session.readThing(true)).rejects.toThrow(/has not been verified/i); + // Non-sensitive reads are unaffected. + await expect(ws.session.readThing()).resolves.toContain("unverified"); + }); + }); + + it.concurrent("a verified collaborator allows the sensitive observation through", async () => { + await withSession(async publicApi => { + const ws = await newWorkspace(publicApi, "verified"); + const bob = await addBob(publicApi, ws); + (await bobOpens(ws, bob.bobApi, bob.bobAccount))[Symbol.dispose](); + + await expect(ws.session.readThing(true)).resolves.toContain("verified"); + }); + }); + + it.concurrent("a verified collaborator does not cover a gatekeeper added later", async () => { + await withSession(async publicApi => { + const ws = await newWorkspace(publicApi, "covered"); + const bob = await addBob(publicApi, ws); + (await bobOpens(ws, bob.bobApi, bob.bobAccount))[Symbol.dispose](); + + // A second connection Bob has never been verified against. + const accounts = await listConnectedAccounts(ws.aliceApi); + const account = accounts.find(a => a.vendorId === TEST_VENDOR_ID)!; + const late = await ws.overseer.newGatekeeper(account.id, thingUrl("late")); + if (!late) throw new Error("Failed to create the second test connection"); + const lateSession: any = await late.openSession(); + + await expect(lateSession.readThing(true)).rejects.toThrow(/has not been verified/i); + // The gatekeeper Bob is verified against still reads fine. + await expect(ws.session.readThing(true)).resolves.toContain("covered"); + }); + }); + + it.concurrent("a collaborator can open a workspace that latched before they were added", + async () => { + await withSession(async publicApi => { + const ws = await newWorkspace(publicApi, "open-after"); + await expect(ws.session.readThing(true)).resolves.toContain("open-after"); + + // Adding Bob and having him open both used to be impossible post-latch. His open runs + // observer verification, which the fixture admits by default. + const bob = await addBob(publicApi, ws); + using bobOverseer = await bobOpens(ws, bob.bobApi, bob.bobAccount); + await expect(bobOverseer.getMetadata()).resolves.toMatchObject({ + id: ws.gadgetId, + containsRestrictedData: true, + }); + }); + }); + + it.concurrent("a collaborator the gatekeeper refuses is denied at open, with its reason", + async () => { + await withSession(async publicApi => { + const ws = await newWorkspace(publicApi, "refused"); + await expect(ws.session.readThing(true)).resolves.toContain("refused"); + + const bob = await addBob(publicApi, ws); + const reason = "You do not have access to this thing."; + await setVerifyOutcome(bob.bobLabel, { allow: false, reason }); + + // This is the strategy-A shape: enforcement lives in the gatekeeper's addObserver(), not in + // a wholesale sharing block, so the user sees the gatekeeper's own message. + const error = await bobOpens(ws, bob.bobApi, bob.bobAccount).then( + overseer => { overseer[Symbol.dispose](); return null; }, + (err: unknown) => err as Error); + expect(error).not.toBeNull(); + expect(error!.message).toMatch(/could not confirm/i); + expect(error!.message).toContain(reason); + }); + }); +}); 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 a279323c..a6c27db5 100644 --- a/packages/integration-tests/fixtures/gatekeeper-test/src/test-gatekeeper.ts +++ b/packages/integration-tests/fixtures/gatekeeper-test/src/test-gatekeeper.ts @@ -20,7 +20,7 @@ // is one control knob here, `allow`, and the reason string is what carries the distinction to the // user. Tests exercise both narratives by choosing reason text. -import { DurableObject, WorkerEntrypoint, type RpcStub } from "cloudflare:workers"; +import { DurableObject, RpcTarget, WorkerEntrypoint, type RpcStub } from "cloudflare:workers"; import type { AccountDescription, ActionKind, ApprovalQueue, Gatekeeper, GatekeeperConnectCallback, GatekeeperUser, GatekeeperUserVerifier, ResourceDescription, ResourceConfiguratorFrame, @@ -217,8 +217,41 @@ export class TestVerifier // --------------------------------------------------------------------------- // Gatekeeper (one per bound resource, running as a facet under the gadget's Overseer) -/** No operations: these tests never open a gadget's session, only verify observers. */ -export type TestSession = Record; +/** + * A live session against a Test Thing, opened via `GatekeeperClient.openSession()`. + * + * 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). + */ +export class TestSession extends RpcTarget { + #queue: RpcStub; + #title: string; + + constructor(queue: RpcStub, title: string) { + super(); + this.#queue = queue; + this.#title = title; + } + + async readThing(restricted?: boolean): Promise { + await this.#queue.authorizeObservation({ + title: `Read ${this.#title}`, + description: `The test read ${this.#title}.`, + ...(restricted ? { containsRestrictedData: true } : {}), + }); + return `the contents of ${this.#title}`; + } + + async doThing(): Promise { + await this.#queue.submitAction(0, { + title: `Poke ${this.#title}`, + description: `The test poked ${this.#title}.`, + implementsRevert: false, + }); + } +} export class TestGatekeeper extends DurableObject implements Gatekeeper { @@ -251,8 +284,15 @@ export class TestGatekeeper return []; } - async startSession(_approvalQueue: RpcStub): Promise { - return {}; + async startSession(approvalQueue: RpcStub): Promise { + // The session calls the queue after startSession() returns, so it owns a duplicate. + let queue = approvalQueue.dup(); + try { + return new TestSession(queue, (await this.describe()).title); + } catch (err) { + queue[Symbol.dispose]?.(); + throw err; + } } /** diff --git a/packages/workshop-backend/src/overseer.ts b/packages/workshop-backend/src/overseer.ts index 98c5f34b..62b9b14b 100644 --- a/packages/workshop-backend/src/overseer.ts +++ b/packages/workshop-backend/src/overseer.ts @@ -819,8 +819,13 @@ function makeOverseerStorage(storage: DurableObjectStorage) { nextChatId: 0, nextHookId: 0, - // True if any past observation was authorized that had the `prohibitAllSharing` flag set - // in its `ObservationDescription`. + // 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. + // + // NOTE: The name predates the flag's rename from `prohibitAllSharing`. It CANNOT be + // renamed: the typed-storage key is the property name, so a rename would silently unlatch + // every workspace that has already observed restricted data. prohibitAllSharing: false, }, @@ -2823,14 +2828,8 @@ class OverseerImpl implements AgentHooks { async authorizeObservation(gatekeeperId: number, description: ObservationDescription, caller: GatekeeperCaller): Promise { - if (description.prohibitAllSharing) { - if ((await this.getSharingManager()).hasAnyShares()) { - throw new Error( - "This observation was blocked because it contains sensitive data that must only be " + - "shown to the account owner, but this workspace is shared with other users. Try again " + - "from a workspace that is not shared."); - } - + if (description.containsRestrictedData) { + await this.#assertSensitiveObservationCoverage(gatekeeperId); this.storage.prohibitAllSharing.put(true); } @@ -2950,6 +2949,45 @@ class OverseerImpl implements AgentHooks { }); } + // Enforce an observation's `containsRestrictedData`: it may proceed only if every current + // collaborator has been verified to have access to the data source producing it, i.e. holds an + // observer record whose account choices cover this gatekeeper. Observer verification normally + // runs at open() (see ensureObserver), but that alone leaves a live-session gap: a collaborator + // added and opened before this gatekeeper existed (or before it read anything sensitive) may + // hold a session that was never verified against it, and must not watch sensitive observations + // arrive. Unredeemed share links do NOT block: redemption is gated at open(), where + // ensureObserver runs before the new collaborator sees anything. + async #assertSensitiveObservationCoverage(gatekeeperId: number): Promise { + let sharing = await this.getSharingManager(); + let collaborators = sharing.listCollaborators(); + if (collaborators.length === 0) return; + + // A gatekeeper that can't verify observers -- no vendor account behind it, or a legacy + // record with no creationSpec -- can never have covered anyone, so any current collaborator + // blocks the observation (conservative). + let gatekeeper = this.storage.gatekeepers.get(gatekeeperId); + let vendorId: string | null = null; + if (gatekeeper) { + try { + vendorId = observerVendorId(gatekeeper); + } catch { + // Legacy connection with no creationSpec: treat as unverifiable. + } + } + + for (let collaborator of collaborators) { + let observer = vendorId ? this.storage.observers.get(collaborator.profile.id) : undefined; + if (!observer || !(gatekeeperId in observer.accountChoices)) { + throw new Error( + "This observation was blocked because it contains sensitive data, but this " + + `workspace is shared with ${collaborator.profile.name} (${collaborator.profile.id}), ` + + "who has not been verified to have access to that data. They must re-open the " + + "workspace (which verifies their access) or be removed from it before this data " + + "can be read."); + } + } + } + // Enforce an observation's `excludeObservers`. For each named opaque observerId: // - Map it back to a profileId via the byObserverId index. An unknown id is not an active // observer (e.g. already torn down), so it is ignored. @@ -6637,13 +6675,6 @@ export class OverseerDurableObject extends DurableObject { let role: CollaboratorRole = "build"; if (!isOwner) { - if (this.impl.storage.prohibitAllSharing.get()) { - // `prohibitAllSharing` can only have been set when the gadget had no shares (see - // `authorizeObservation`), and no new shares can be created while it's set, so any - // non-owner reaching here is necessarily unauthorized. - throw createOpenGadgetError(OPEN_GADGET_ERROR_CODES.workspaceAccessDenied); - } - let sharing = await this.impl.getSharingManager(); // If a share key was provided, redeem it. The owner already has full access and should not @@ -6675,7 +6706,9 @@ export class OverseerDurableObject extends DurableObject { // Verify the caller may observe everything this Gadget has read through its in-scope // gatekeepers, configuring their connected accounts if needed. This runs only after a valid // role is confirmed, so it never reveals gatekeeper or resource metadata to an unauthorized - // user. The prohibitAllSharing short-circuit above still wins -- lockdown takes precedence. + // user. This is also what enforces sensitive (`containsRestrictedData`) data access: a + // gatekeeper that has read such data admits a collaborator only if addObserver() verifies + // them, and refuses everyone if it cannot verify anyone. await this.impl.ensureObserver(profileId, clientUser, role, configureObservers); // Fire-and-forget a call to the collaborator's user DO so the gadget appears on @@ -6751,12 +6784,6 @@ export class OverseerDurableObject extends DurableObject { // Caller must be the owner or a build collaborator. if (ownerId !== callerId) { - if (this.impl.storage.prohibitAllSharing.get()) { - return { - accepted: false, - message: "This workspace has sharing disabled, so only its owner can access it.", - }; - } let role = (await this.impl.getSharingManager()).getEffectiveRole(callerProfile.id); if (role !== "build") { return { @@ -7422,7 +7449,7 @@ class OverseerClientInterface extends RpcTarget implements Overseer { id: this.impl.ctx.id.toString(), title: this.impl.storage.title.get(), totalCost: this.impl.storage.totalCost.get(), - sharingProhibited: this.impl.storage.prohibitAllSharing.get(), + containsRestrictedData: this.impl.storage.prohibitAllSharing.get(), role: "build", defaultGadgetId: this.impl.defaultGadgetId, }; @@ -7441,7 +7468,7 @@ class OverseerClientInterface extends RpcTarget implements Overseer { id: this.impl.ctx.id.toString(), title: this.impl.storage.title.get(), totalCost: this.impl.storage.totalCost.get(), - sharingProhibited: this.impl.storage.prohibitAllSharing.get(), + containsRestrictedData: this.impl.storage.prohibitAllSharing.get(), role: "build", defaultGadgetId: this.impl.defaultGadgetId, }; @@ -7463,9 +7490,9 @@ class OverseerClientInterface extends RpcTarget implements Overseer { callback(metadata).catch(unsubscribe); } }; - let sharingProhibitedSubscriber = { + let restrictedDataSubscriber = { update(value: boolean | undefined) { - metadata.sharingProhibited = value; + metadata.containsRestrictedData = value; callback(metadata).catch(unsubscribe); } }; @@ -7473,13 +7500,13 @@ class OverseerClientInterface extends RpcTarget implements Overseer { let unsubscribe = () => { this.impl.storage.title.unsubscribe(titleSubscriber); this.impl.storage.totalCost.unsubscribe(costSubscriber); - this.impl.storage.prohibitAllSharing.unsubscribe(sharingProhibitedSubscriber); + this.impl.storage.prohibitAllSharing.unsubscribe(restrictedDataSubscriber); callback[Symbol.dispose](); }; this.impl.storage.title.subscribe(titleSubscriber); this.impl.storage.totalCost.subscribe(costSubscriber); - this.impl.storage.prohibitAllSharing.subscribe(sharingProhibitedSubscriber); + this.impl.storage.prohibitAllSharing.subscribe(restrictedDataSubscriber); callback(metadata).catch(unsubscribe); @@ -8875,8 +8902,11 @@ class OverseerClientInterface extends RpcTarget implements Overseer { // --- Collaborator management --- // // The sharing/permission logic lives in SharingManager (./sharing). These methods handle only - // the RPC-bound pieces (resolving profiles via User DOs, the `prohibitAllSharing` policy) and - // delegate the rest. + // the RPC-bound pieces (resolving profiles via User DOs) and delegate the rest. Note that + // sharing stays available even after the workspace observes sensitive data + // (`containsRestrictedData`): whether a given collaborator may actually see that data is + // enforced per-gatekeeper by observer verification (ensureObserver at open(), and the coverage + // guard in authorizeObservation), not by blocking sharing wholesale. async listObserverRequirements( role: CollaboratorRole): Promise { @@ -8897,12 +8927,6 @@ class OverseerClientInterface extends RpcTarget implements Overseer { return null; } - if (this.impl.storage.prohibitAllSharing.get()) { - throw new Error( - "This workspace has observed sensitive data. To prevent leaks, the workspace cannot be " + - "shared."); - } - return (await this.impl.getSharingManager()).addCollaborator({ caller: this.#sharingCaller(), profile, @@ -8956,23 +8980,11 @@ class OverseerClientInterface extends RpcTarget implements Overseer { async createShareLink(role: CollaboratorRole, note?: string) : Promise<{ key: string; linkId: string }> { - if (this.impl.storage.prohibitAllSharing.get()) { - throw new Error( - "This workspace has observed sensitive data. To prevent leaks, the workspace cannot be " + - "shared."); - } - return (await this.impl.getSharingManager()) .createShareLink({ caller: this.#sharingCaller(), role, note }); } async newShareLinkKey(linkId: string): Promise<{ key: string }> { - if (this.impl.storage.prohibitAllSharing.get()) { - throw new Error( - "This workspace has observed sensitive data. To prevent leaks, the workspace cannot be " + - "shared."); - } - return (await this.impl.getSharingManager()) .newShareLinkKey({ caller: this.#sharingCaller(), linkId }); } diff --git a/packages/workshop-backend/src/sharing.ts b/packages/workshop-backend/src/sharing.ts index ff0cb9e1..b75f6d84 100644 --- a/packages/workshop-backend/src/sharing.ts +++ b/packages/workshop-backend/src/sharing.ts @@ -15,11 +15,11 @@ // re-adding a removed collaborator restores them and, transitively, everyone they had shared with. // (Records and revoked keys accumulate in storage; a future GC could reclaim long-dead entries.) // -// NOTE: The `prohibitAllSharing` policy flag intentionally does NOT live here. It is a broader -// "is this gadget allowed to communicate with anyone other than the owner?" policy (it also -// gates gatekeeper writes and web fetches) and is expected to grow into a separate policy engine. -// The Overseer enforces that flag; this module only exposes `hasAnyShares()` so the policy can -// ask about the current sharing state. +// NOTE: The sensitive-data (`containsRestrictedData`) policy intentionally does NOT live here. +// It is a broader "what may this gadget do after reading restricted data?" policy (it gates +// gatekeeper writes and web fetches, and requires per-gatekeeper observer verification of +// collaborators) and is expected to grow into a separate policy engine. The Overseer enforces +// it; this module only answers questions about the sharing graph. import { AiChatAuthorInfo, CollaboratorInfo, PermissionEdge, CollaboratorRole, AffectedCollaborator } from "@gadgets/workshop-shared/api"; @@ -160,8 +160,7 @@ export class SharingManager { // Sharing-state queries /** - * True if anyone other than the owner can currently access the gadget. Used by the Overseer's - * `prohibitAllSharing` policy to decide whether a sensitive observation must be blocked. + * True if anyone other than the owner can currently access the gadget. * * Because removed collaborators and revoked links linger in storage (the lazy revocation model; * see the module header and removeCollaborator/revokeShareLink), this must reflect *current* @@ -288,8 +287,8 @@ export class SharingManager { /** * Add a collaborator with a `user` edge from the caller, granting `role`. The caller is - * responsible for resolving `profile` (via RPC) and for any policy checks (e.g. - * `prohibitAllSharing`). The caller may not grant a role higher than their own effective role. + * responsible for resolving `profile` (via RPC) and for any policy checks. The caller may not + * grant a role higher than their own effective role. */ addCollaborator(opts: { caller: SharingCaller; diff --git a/packages/workshop-frontend/src/ShareModal.tsx b/packages/workshop-frontend/src/ShareModal.tsx index d843ea04..a153d685 100644 --- a/packages/workshop-frontend/src/ShareModal.tsx +++ b/packages/workshop-frontend/src/ShareModal.tsx @@ -371,7 +371,7 @@ export default function ShareModal({ open, onClose, overseer, metadata, currentU }, []) const isOwner = !metadata.owner - const sharingProhibited = metadata.sharingProhibited === true + const containsRestrictedData = metadata.containsRestrictedData === true const loadData = useCallback(async () => { try { @@ -558,7 +558,7 @@ export default function ShareModal({ open, onClose, overseer, metadata, currentU const handleAddCollaborator = async () => { const username = addUsername.trim() - if (!username || sharingProhibited || addingRef.current) return + if (!username || addingRef.current) return addingRef.current = true setAdding(true) @@ -584,7 +584,7 @@ export default function ShareModal({ open, onClose, overseer, metadata, currentU } const handleCreateShareLink = async () => { - if (sharingProhibited || creatingLinkRef.current) return + if (creatingLinkRef.current) return creatingLinkRef.current = true setCreatingLink(true) try { @@ -610,7 +610,7 @@ export default function ShareModal({ open, onClose, overseer, metadata, currentU // Copy a share link again. Secrets are never stored, so the previously-shown URL can't be // re-displayed. We mint a new secret for the same logical link and copy that. const handleCopyShareLink = async (linkId: string) => { - if (sharingProhibited || copyingLinkRef.current) return + if (copyingLinkRef.current) return copyingLinkRef.current = true setCopyingLinkId(linkId) try { @@ -774,23 +774,17 @@ export default function ShareModal({ open, onClose, overseer, metadata, currentU className="chat-panel min-h-0 flex-1 overflow-y-auto overscroll-contain px-4 pb-6 sm:px-6" onScroll={(e) => setScrolled(e.currentTarget.scrollTop > 0)} > - {sharingProhibited ? ( -
-
- + {containsRestrictedData && ( +
+
+
-

- This workspace can’t be shared -

-

- It has observed sensitive data that can only be accessed by you, the owner. -

-

- To share something similar, create a blueprint from a gadget in this workspace, then use it to create a new workspace. +

+ This workspace has read sensitive data. People you invite must be verified to have + access to the same data — some may be unable to open it.

- ) : ( - <> + )}
{adding ? 'Inviting…' : 'Invite'} @@ -910,16 +902,16 @@ export default function ShareModal({ open, onClose, overseer, metadata, currentU placeholder="Name this link (optional)…" aria-label="Share link name (optional)" className="h-9 min-w-0 flex-1 border-0 bg-transparent p-0 text-[14px] leading-5 tracking-[-0.25px] text-kumo-default outline-none placeholder:text-kumo-inactive" - disabled={creatingLink || sharingProhibited} + disabled={creatingLink} /> - + {creatingLink ? 'Creating…' : 'Create link'} setShowLinkComposer(false)}> @@ -931,7 +923,6 @@ export default function ShareModal({ open, onClose, overseer, metadata, currentU
)} - - )}
diff --git a/packages/workshop-shared/src/api.ts b/packages/workshop-shared/src/api.ts index d858dd2d..c44b0b15 100644 --- a/packages/workshop-shared/src/api.ts +++ b/packages/workshop-shared/src/api.ts @@ -1250,10 +1250,12 @@ export type GadgetMetadata = { role?: CollaboratorRole; /** - * True when the gadget has observed data marked as share-prohibited. Such gadgets can no longer - * be shared with additional users or links. + * 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. */ - sharingProhibited?: boolean; + containsRestrictedData?: boolean; /** * Various objects in the API specify a gadgetId, but make the property optional. When omitted, diff --git a/packages/workshop-shared/src/gatekeeper.ts b/packages/workshop-shared/src/gatekeeper.ts index 48bcdd72..6190c75f 100644 --- a/packages/workshop-shared/src/gatekeeper.ts +++ b/packages/workshop-shared/src/gatekeeper.ts @@ -1070,21 +1070,22 @@ export type ObservationDescription = { // can help detect situations where the gadget could leak information. /** - * If true, then this observation contains sensitive information that MUST NOT be shared with - * ANYONE except the account owner. This means: - * - If the gadget is shared already, authorizeObservation() must throw an exception to block - * the observation. - * - All future sharing of the gadget is prohibited. - * - Once observed, the gadget goes into "lockdown mode" where it can no longer perform any - * actions, only make observations. This prevents the gadget from leaking data through other - * gatekeepers. - * - * TODO(someday): This was added as a stopgap in order to be able to make certain sensitive data - * sources available to internal users. In the longer-term, it should be possible to share - * sensitive data as long as the recipients also have access to that same data, but this - * requires a more complex policy framework to compute. - */ - prohibitAllSharing?: boolean; + * If true, then this observation contains sensitive information that must only be shown to + * people who are verified to have access to the same data. This means: + * - If the gadget is shared, authorizeObservation() throws unless every current collaborator + * is already a verified observer of this gatekeeper (via `addObserver()`; see the overseer's + * coverage guard). Collaborators are (re-)verified every time they open the gadget, so a + * gatekeeper whose `addObserver()` always throws is effectively unshareable once it has made + * one of these observations. + * - Once observed, the gadget goes into a restricted mode where it can no longer perform any + * actions or fetch from the public web, only make observations. This prevents the gadget + * from leaking the data through other gatekeepers. + * + * TODO(someday): The restricted mode is still a blunt instrument. It should be possible to + * perform actions whose visibility is limited to people verified to have access to the same + * data, but this requires a more complex policy framework to compute. + */ + containsRestrictedData?: boolean; /** * If present, then this observation includes data that must not be revealed to the given