From 194c622b8a27f54f31447c64bbea654e89624e65 Mon Sep 17 00:00:00 2001 From: Maximo Guk <62088388+Maximo-Guk@users.noreply.github.com> Date: Thu, 20 Aug 2026 14:32:38 -0500 Subject: [PATCH 1/4] Govern sharing of restricted data by observer verification Rename prohibitAllSharing to containsRestrictedData: the flag now means "this observation carries data whose sharing is governed by observer verification" rather than a wholesale ban on sharing the workspace. The legacy flag is still read on persisted records and converted on load. Instead of refusing to share a flagged workspace outright, sharing is admitted when every collaborator is verified as an observer of the producing gatekeeper: - open() verifies each collaborator against every gatekeeper in their role's verification scope (ensureObserver), and the coverage guard (#assertSensitiveObservationCoverage) blocks a sensitive observation naming any current collaborator not yet verified against its producer. Coverage is held to each collaborator's role scope, since a "use" collaborator can never be verified against a gatekeeper no gadget binds. - Share-key redemption writes a *pending* edge that grants no interim authority: the redeeming open() verifies the recipient at the role the edge would grant, confirms only on success (capped at the verified role, denied if the connection/binding topology changed mid-verification, merged if a concurrent redemption landed first), and rolls the edge back on refusal or a null effective role. - Removing the producing connection no longer lifts the restriction for existing collaborators, and assertNewSharingAllowed refuses new grants inside the grant write's synchronous block. - The coverage-guard error reaches sandboxed gadget/agent output, so it names the collaborator but omits their profile id (the full email on OAuth/CF Access deployments). Three hardening rounds are folded in: - A failed live re-check scrubs the failed gatekeeper from the collaborator's persisted observer record synchronously with the failure determination (covering both failure sites and cancel-after-reprompt), and the terminal catch best-effort de-registers invalidated gatekeepers alongside newly-added ones (removeObserver is idempotent). Without this, a collaborator whose provider-side access was revoked kept admitting the producer's restricted observations to their still-live older session. The scrub is scoped to the failed gatekeeper; a repaired pass re-persists full coverage. Fail-closed by design: an operational failure (outage, expired credential) scrubs the same way, blocking that producer's restricted reads until the collaborator re-opens successfully. - confirmShareKeyRedemption re-asserts the redemption policy via the same optional assertGrantAllowed callback the other grant-writing mutators use, in the granting write's synchronous block: redemption is two-phase, and a restricted-data producer removed between the pending write and the confirm -- invisible to the topology fingerprint, since an unverifiable legacy producer's remove() skips the share-link guard -- must still refuse the grant. An already-confirmed edge skips it, matching redeemShareKey. - The accepted pending-edge re-add wart is documented precisely: a racing removal necessarily aimed at a previously confirmed edge (pending-only recipients are invisible to listCollaborators), the re-add carries no incremental authority (the recipient holds the live, manually re-redeemable link), and revoking the link is the durable exclusion. The accepted residuals (formerly-bound and never-bound producers outside "use" scope) are documented in docs/observers.md edge case 4 and docs/sharing.md. Co-Authored-By: Claude Fable 5 --- docs/observers.md | 184 +++++-- docs/sharing.md | 29 +- .../__tests__/restricted-data.test.ts | 39 ++ .../__tests__/sharing.test.ts | 444 ++++++++++++++-- packages/workshop-backend/src/overseer.ts | 503 +++++++++++++++--- packages/workshop-backend/src/sharing.ts | 259 ++++++--- packages/workshop-shared/src/api.ts | 18 +- packages/workshop-shared/src/gatekeeper.ts | 31 +- 8 files changed, 1243 insertions(+), 264 deletions(-) create mode 100644 packages/workshop-backend/__tests__/restricted-data.test.ts diff --git a/docs/observers.md b/docs/observers.md index d6c5534c9..1034abebf 100644 --- a/docs/observers.md +++ b/docs/observers.md @@ -30,14 +30,15 @@ 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." +The mechanism is a per-user, gatekeeper-mediated check — "this data may be shared, but only with +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 +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.) -This feature replaces that all-or-nothing posture with a per-user, gatekeeper-mediated check: +The check works as follows: - **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 @@ -56,7 +57,7 @@ This feature replaces that all-or-nothing posture with a per-user, gatekeeper-me `ObservationDescription.excludeObservers`. The overseer must then guarantee those observers never see it, or block the observation. -**The API is already committed** (commit `e2f1707`). The relevant interfaces are +**The API is already committed.** The relevant interfaces are `GatekeeperUser.getVerifier()`, `GatekeeperUserVerifier`, `Gatekeeper.addObserver()` / `removeObserver()`, and `ObservationDescription.excludeObservers`, all in `packages/workshop-shared/src/gatekeeper.ts`. @@ -68,8 +69,8 @@ This feature replaces that all-or-nothing posture with a per-user, gatekeeper-me - **Role-based breadth of verification:** - **`build`** collaborators (full access — chat + code + all bindings) must be verified against **every** gatekeeper the Gadget has. - - **`use`** collaborators (UI only, no chat access — see `UseOverseerInterface`, - `overseer.ts:2816`) must be verified only against **named bindings** (gatekeepers with a + - **`use`** collaborators (UI only, no chat access — see `UseOverseerInterface` in + `overseer.ts`) must be verified only against **named bindings** (gatekeepers with a `bindingName`), since that is all the UI can invoke. - **Account selection.** A collaborator must have their own connected account for each vendor the Gadget depends on. For ordinary bindings, they choose which account to use (e.g. work or personal @@ -90,18 +91,18 @@ This feature replaces that all-or-nothing posture with a per-user, gatekeeper-me | Concern | Location | |---|---| | Gatekeeper RPC API (the committed surface) | `packages/workshop-shared/src/gatekeeper.ts` | -| 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`) | -| 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` | -| Gatekeeper facet access | `overseer.ts:1079` `getGatekeeperFacet()` | -| Overseer storage collections | `overseer.ts:316` (`gatekeepers`, with `byBindingName` index — template for a new collection) | -| Connected accounts (User DO) | `packages/workshop-backend/src/user.ts:12` `ConnectedAccountRecord` (`account: Fetcher`, `vendorId`) | -| List connected accounts | `user.ts:890` `subscribeConnectedAccounts()`; subscriber type `api.ts:116` | -| Account → gatekeeper class | `user.ts:1136` `getGatekeeperClassFor()` | +| Overseer DO, `open()` auth entry point | `packages/workshop-backend/src/overseer.ts` | +| Server `openGadget` path | `packages/workshop-backend/src/server.ts` | +| Role resolution / permission graph | `packages/workshop-backend/src/sharing.ts` (`getEffectiveRole`, `computeEffectiveRoles`) | +| `containsRestrictedData` enforcement | `overseer.ts` (`authorizeObservation` coverage guard, `getWebFetchEnv`, `submitAction`) | +| Observation recording | `overseer.ts` `authorizeObservation()`; `ApprovalQueueImpl` | +| Gatekeeper storage record | `overseer.ts` `GatekeeperRecord` (has `creationSpec.vendorId`) | +| `GatekeeperCreationSpec` | `packages/workshop-shared/src/api.ts` | +| Gatekeeper facet access | `overseer.ts` `getGatekeeperFacet()` | +| Overseer storage collections | `overseer.ts` (`gatekeepers`, with `byBindingName` index — template for a new collection) | +| Connected accounts (User DO) | `packages/workshop-backend/src/user.ts` `ConnectedAccountRecord` (`account: Fetcher`, `vendorId`) | +| List connected accounts | `user.ts` `subscribeConnectedAccounts()`; subscriber type in `api.ts` | +| Account → gatekeeper class | `user.ts` `getGatekeeperClassFor()` | --- @@ -134,8 +135,8 @@ This feature replaces that all-or-nothing posture with a per-user, gatekeeper-me ### New overseer storage collection: `observers` -Add an `observers` collection to `OverseerStorage` (mirror the `gatekeepers` collection at -`overseer.ts:316`, including a secondary index for reverse lookup): +Add an `observers` collection to `OverseerStorage` (mirror the `gatekeepers` collection in +`overseer.ts`, including a secondary index for reverse lookup): ```ts type ObserverRecord = { @@ -167,7 +168,7 @@ log) lives inside each gatekeeper's own DO and is out of scope here. ### Step 1 — User DO: mint a verifier for a chosen account Add a method to the User DO (`packages/workshop-backend/src/user.ts`), near -`getGatekeeperClassFor` (`user.ts:1136`): +`getGatekeeperClassFor`: ```ts // Mint a verifier from one of THIS user's connected accounts, identified by accountId. @@ -197,7 +198,7 @@ invoked **only** when the opening user needs to configure gatekeeper accounts. I without an extra round trip. Add to the RPC API (`packages/workshop-shared/src/api.ts`) and thread through -`server.ts:206` → `overseer.open()` (`overseer.ts:2714`): +`server.ts` `openGadget` → `overseer.open()`: ```ts // Provided by the client when opening a gadget. Invoked by the overseer only if the opening @@ -227,11 +228,19 @@ 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. (Observer verification *is* the open()-time enforcement for +sensitive data; no `containsRestrictedData` check precedes it.) +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. +Role resolution plus verification is one shared gate, `OverseerImpl.authorizeCollaborator`, and +every non-owner entry point that can surface workspace data runs it — `open()` interactively, and +`receiveExternalMessage()` non-interactively (no configuration channel, so an unverified caller is +told to open the workspace, which is where verification happens). An agent reply on the external +path can surface anything the workspace already read, so it must not admit a collaborator with +less verification than `open()` would demand. + Add a private helper on `OverseerImpl`, roughly: ```ts @@ -306,7 +315,7 @@ Notes: Implement the `ObserverConfigCallback` on the client. When the overseer calls `configure(needs)`: 1. For each `ObserverBindingNeed`, find the user's candidate accounts by filtering the existing - `subscribeConnectedAccounts()` results (`user.ts:890`) by `need.vendorId`. + `subscribeConnectedAccounts()` results by `need.vendorId`. 2. If one or more accounts match, pre-select one arbitrarily as the default; let the user change it via a dropdown. (Most users have one account per vendor and will just click "OK".) 3. Include forced auto-provisioned accounts in the subscription. If **no** account matches, use @@ -322,7 +331,7 @@ you're allowed to see the data it uses." ### Step 5 — Overseer: forward exclusion in `authorizeObservation()` -Extend `authorizeObservation()` (`overseer.ts:1169`) to honor `description.excludeObservers`. +Extend `authorizeObservation()` (in `overseer.ts`) to honor `description.excludeObservers`. Because v1 has no per-thread hiding, the only case in which we can let an excluded-but-named observation proceed is when the named observer has *already lost access* in the sharing graph. @@ -401,12 +410,55 @@ already in the JSDoc in `gatekeeper.ts`; add anything missing there rather than throws and denies the open. 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. + model in `sharing.ts`. The denial also scrubs each failed gatekeeper from the collaborator's + persisted observer record (and best-effort de-registers them gatekeeper-side), so the + sensitive-observation coverage guard (edge case 4) blocks that producer's later restricted + observations — including to the collaborator's still-live sessions — until a successful + re-open re-persists coverage, or the collaborator is removed. The residual under the lazy + model: a collaborator who never re-opens keeps their record and any live session, but once + scrubbed they *block* that producer's restricted reads like any unverified collaborator. + An operational failure (vendor outage, expired credential) scrubs the same way — the + overseer cannot tell it from a settled denial, so coverage fails closed until a repaired + re-open. +4. **`containsRestrictedData` interaction** — a sensitive observation is admitted only if every + current collaborator *in whose role-scope the producing gatekeeper falls* holds an observer + record covering it + (`#assertSensitiveObservationCoverage` in `authorizeObservation`); otherwise it is blocked + with a message naming the unverified collaborator. Coverage is held to each collaborator's own + verification scope because `ensureObserver` can never verify beyond it: a `use` collaborator + can't be covered for a gatekeeper no gadget binds, so demanding that would block the read + permanently (an unverifiable gatekeeper — no vendor account, or a legacy record — blocks + 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. + `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 + stop requiring `use` collaborators' coverage — the same skip as a never-bound producer, + though the liveness argument above doesn't apply to it. Accepted because (i) `use` sessions + cannot read chat history or the action log, so the exposure is limited to state the gadget + persisted, served through the gadget's own UI or export; (ii) that data entered gadget + storage while the producer *was* bound, when every `use` collaborator was verified against + it or the read was blocked; (iii) the residual is `use` grants created after the unbind, + who view that persisted state unverified — and re-binding the connection restores their + verifiability at their next open. + The *never*-bound flavor of the same skip is broader: a producer reachable only through + chat bindings (an ambient singleton the agent reads in chat) was never in any `use` + collaborator's scope, so premise (ii) does not hold for it — the agent can persist its + restricted data into gadget code or storage without any `use` collaborator ever having + been verified against it, and there is no prior binding for "re-bind" to restore. + Accepted on the same grounds: coverage there is unverifiable by construction (the + liveness argument above), `use` sessions still cannot read chat history or the action + log, so the exposure is limited to what the agent chose to persist, and the forward + remedy is binding the producer to a gadget — that puts it in `use` scope, so every + collaborator is verified against it at their next open. 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). + resource (inherent to the security model). Adding a binding does not restart live sessions, + so an already-open collaborator is only verified against it at their next open; the coverage + guard (edge case 4) covers the interim, blocking the new connection's sensitive reads until + every collaborator has been verified against it. 6. **Performance** — `ensureObserver` does one `getVerifier` + one `addObserver` per in-scope gatekeeper per open. Parallelize with `Promise.all` and pipe the verifier promise straight into `addObserver`. Expensive gatekeepers cache on their side. @@ -414,6 +466,46 @@ already in the JSDoc in `gatekeeper.ts`; add anything missing there rather than named bindings, so they will never appear in `excludeObservers` from a non-named binding (the gatekeeper doesn't know their id). The Step 5 logic handles this naturally (unknown id → ignored). +8. **Removing a connection that read restricted data** — while the workspace is latched + (`containsRestrictedData`) *and* shared, `GatekeeperClient.remove()` refuses: the gatekeeper + record is what observer verification (and the coverage guard) runs against, and the restricted + data outlives the connection in chat history and storage, so deleting the record would let a + never-verified collaborator open unchecked. Outstanding (unrevoked) share links block removal + the same way: a link creates no collaborator state until redeemed, keys are multi-redeemable + and never expire, and redemption is gated at open() only while the record exists — so a link + redeemed after removal would grant the same unchecked access. The remedy is to remove + collaborators and revoke share links first. + The guard is scoped to the *producer* connections — those through which restricted data was + actually read, derived from the permanent action log (`restrictedProducerIds`) — since only + they anchor restricted-data verification; a non-producer connection stays removable while + shared. Unverifiable records (legacy, or no vendor account behind them) are exempt even as + producers — they anchor no verification, and removing one is itself a remedy. For a *legacy* + record that remedy is also an access-widening event for existing grants: while the record + exists, `#inScopeGatekeepers` throws on every non-owner open (a workspace-wide hard deny), + so removing it readmits existing collaborators — verified against only the surviving + connections, with the restricted data still in chat history. Accepted because the + alternative (requiring zero collaborators first, as for verifiable producers) would strand + the workspace: once the producer is gone, re-adding anyone is refused, so forced pre-removal + would make it permanently unshareable rather than usable by exactly the people who already + had grants. (The other unverifiable flavor — aiModel/agentSpawner, no vendor account — + widens nothing: it was never in any collaborator's verification scope, and its sensitive + reads were coverage-blocked whenever anyone was shared in.) Internal removals + (creation-failure rollback, ambient reconciliation) are unguarded. + The complementary rule: once latched, if any producer connection no longer exists (removed + while the workspace was unshared, or an exempt unverifiable producer), a new party could no + longer be verified for the data, so everything that would admit one refuses + (`assertNewSharingAllowed`): the grant-creating sharing mutators — `addCollaborator`, + `createShareLink`, `newShareLinkKey` — and `redeemShareKey` at open(). Gating redemption is + what makes the unverifiable-producer exemption above safe despite outstanding keys: once any + producer is gone, those keys are dead too — refused before a pending edge is written, and + before a not-yet-confirmed edge is settled. The grant-creating mutators check synchronously + with their storage write, so a concurrent connection removal cannot slip between the check + and the grant. Redemption is two-phase, so it checks twice: once synchronously with the + *pending* write (`redeemShareKey`), and again in the *confirm's* synchronous block + (`confirmShareKeyRedemption`, the granting write) — the two are separated by the redeeming + open()'s await windows, so a producer removed anywhere between redemption and confirm still + refuses the grant. Existing grants are untouched: a *confirmed* edge skips both checks, so a + collaborator re-opening with a retained key stays a no-op. --- @@ -428,6 +520,9 @@ already in the JSDoc in `gatekeeper.ts`; add anything missing there rather than opens do not (record covers them) but still re-run `addObserver`. - a thrown `addObserver` denies the open and triggers best-effort `removeObserver` rollback on bindings added in the same pass, and does not persist the record. + - a failure against an *already-covered* binding scrubs that binding from the persisted record, + so coverage fails closed after a revocation (edge case 3) instead of admitting the producer's + restricted reads on stale coverage. - missing account → binding reported as a need to the callback; callback rejection denies open. - **`authorizeObservation` exclusion:** observation naming a still-authorized observer throws; observation naming an observer who lost access proceeds and deletes that observer record (+ @@ -474,15 +569,17 @@ 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). + 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 + 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. `getVerifier()` mints a verifier exposing the observer's vendor identity (via the - "non-standard method on the verifier" pattern, `gatekeeper.ts:456-461`). `addObserver()` resolves - that identity and checks it against the bound resource's ACL, throwing on failure. Gatekeepers - should cache per-open to bound cost (`gatekeeper.ts:511-516`). No `excludeObservers` is needed: + "non-standard method on the verifier" pattern; see `GatekeeperUserVerifier` in `gatekeeper.ts`). + `addObserver()` resolves that identity and checks it against the bound resource's ACL, throwing + on failure. Gatekeepers should cache per-open to bound cost (see the note on `addObserver()`). + No `excludeObservers` is needed: the whole unit is covered up front, so nothing read later could be invisible to a verified observer. @@ -491,8 +588,9 @@ its resource types. plus the set of current observers. `addObserver()` verifies the observer against **every** logged set so far. When a later observation first touches a **new** set, the gatekeeper re-verifies all current observers and sets `excludeObservers` for any who fail (the overseer then blocks the - observation per `gatekeeper.ts:751-774`). `removeObserver()` drops the observer from the tracked - set. Each per-set check reuses the same ACL primitive the corresponding narrow (B) binding uses. + observation per the `excludeObservers` contract). `removeObserver()` drops the observer from + the tracked set. Each per-set check reuses the same ACL primitive the corresponding narrow (B) + binding uses. - **D — Low-stakes.** No information-flow tracking. `addObserver()` / `removeObserver()` are no-ops; any collaborator may observe. `getVerifier()` returns a trivial verifier (the overseer @@ -520,8 +618,8 @@ its resource types. | **linear** | Workspace | **C** | Track accessed teams; verify the observer against each (reusing the Team B check). | | **notion** | Page / Database | **B** | Check the observer's Notion access to the bound page/database. | | **notion** | Workspace | **C** | Track accessed pages/databases; verify the observer's access to each. | -| **supabase** | Project | **B** | Verify the observer's own `listProjects()` (`supabase-api.ts:306`) includes the bound project ref. Within a project, arbitrary read-only SQL spans the whole DB, so the project is the atomic unit (no per-table tracking). | -| **supabase** | Organization | **C** | Track accessed project refs (the org session reaches them via `openProject` / `listProjects`, `supabase.ts:1015`/`:1037`); verify the observer's `listProjects()` includes each, reusing the Project B check. | +| **supabase** | Project | **B** | Verify the observer's own `listProjects()` (`supabase-api.ts`) includes the bound project ref. Within a project, arbitrary read-only SQL spans the whole DB, so the project is the atomic unit (no per-table tracking). | +| **supabase** | Organization | **C** | Track accessed project refs (the org session reaches them via `openProject` / `listProjects` in `supabase.ts`); verify the observer's `listProjects()` includes each, reusing the Project B check. | | **confluence** | Site | **C** | Verify site access; track observed spaces and content because both can have narrower permissions. | | **confluence** | Space | **C** | Verify space access; track observed pages and blog posts because content restrictions may be narrower. | | **confluence** | Page / Blog Post | **C** | Verify bound-content access; track observed child pages because they may have stricter restrictions than their parent. | diff --git a/docs/sharing.md b/docs/sharing.md index 8bb4fc467..12018ee17 100644 --- a/docs/sharing.md +++ b/docs/sharing.md @@ -33,7 +33,7 @@ Authorization is capability-based: `open()` computes the caller's effective role There are two ways to grant someone collaborator access: -**Direct add.** The owner or an existing collaborator enters a username (email address) in the Share modal. The system looks up the corresponding user account; if it exists, a collaborator record is created. The target user does not receive an in-product notification -- the sharer is expected to send them a link or tell them out of band. +**Direct add.** The owner or an existing collaborator enters a username (an email address on OAuth/CF Access deployments; a normalized alphanumeric handle on password deployments) in the Share modal. The system looks up the corresponding user account; if it exists, a collaborator record is created. The target user does not receive an in-product notification -- the sharer is expected to send them a link or tell them out of band. **Share link.** Any collaborator (or the owner) can create a share link, which encodes a secret key in the URL as a `#share=` fragment. Anyone who opens this link is automatically added as a collaborator. A link is a durable handle that owns one or more keys: creating it mints its first key, and "copying" the link later mints another key for the same link. The raw key is shown to the creator only once at mint time and is never stored server-side, so re-copying can't reproduce an old key -- it mints a new one. Any of a link's keys can be redeemed by multiple people, or the same person multiple times, until the link is revoked, which invalidates every key minted for it. @@ -43,6 +43,8 @@ Storage shape: a link is its first key. The `shareKeys` table holds one row per Share key redemption and gadget opening happen atomically in a single RPC call (`openGadget(id, shareKey)`), which allows subsequent calls to be pipelined on the returned `Overseer` stub without waiting for a separate redemption step. +Redemption is **pending until verified**: redeeming a key writes the recipient's `shareKey` edge with a `pending` flag, which grants no authority to anyone -- the recipient is invisible to `listCollaborators`, to everyone's effective roles, and to the sensitive-observation coverage guard. Only the open() that performed the redemption counts the edge (to compute the role it must verify the recipient for); the edge is confirmed when that open's observer verification succeeds and severed when it fails, so a refused (or still-verifying) recipient never holds authority. A stale pending edge left by a crashed open is inert and settles on the next redemption of the same link. + ### Home page behavior A shared gadget does not appear on a collaborator's home page until they first open it. At that point, a record is created in the collaborator's user account (via `UserDurableObject.recordSharedGadgetOpen()`), storing a cached copy of the gadget's title and the owner's profile. The `lastActive` timestamp is updated each time they open the gadget. @@ -97,22 +99,21 @@ This does mean removed collaborators and revoked links accumulate in storage. Li ### Effective-role algorithm -The core is a **fixed-point role-propagation computation** implemented in `SharingManager.computeEffectiveRoles()`. It computes the effective role of every collaborator (given an optional hypothetical change), returning a map from profile ID to effective role (absence from the map means no access). It is the single source of truth: `open()`, `hasAnyShares()`, the listing RPCs, and the preview methods all derive from it. +The core is a **fixed-point role-propagation computation** implemented in `SharingManager.computeEffectiveRoles()`. It computes the effective role of every collaborator (given an optional hypothetical change), returning a map from profile ID to effective role (absence from the map means no access). It is the single source of truth: `open()`, the listing RPCs, and the preview methods all derive from it. -Inputs (all optional; used to model a hypothetical change in preview): -- `removedUser` -- a profile ID to treat as removed (excluded from the graph). +Inputs (all optional; used to model a hypothetical change): +- `removedUser` -- a profile ID to treat as removed (excluded from the graph). Used by preview. - `removedEdge` -- a single user edge (`{target, sharer}`) to treat as removed. Used to preview a non-owner removing only their own edge. -- `revokedLinkId` -- a share link ID to treat as revoked. -- `overrides` -- profile IDs pinned to at least a given role regardless of their edges. +- `revokedLinkId` -- a share link ID to treat as revoked. Used by preview. +- `assumePendingLink` -- a `{profileId, linkId}` pair whose pending edge is counted as though it were confirmed. Unlike the removal-shaped preview inputs, this models a hypothetical *grant*: the open() that just redeemed the link counting its own edge to compute the role it must verify for. Every other pending edge contributes nothing. The algorithm: 1. **Build the candidate set.** Load all collaborators except the (hypothetically) removed user. 2. **Collect share-link metadata.** Build a map from link ID to `{creator, role}`, skipping links that are `revoked` (or the hypothetical `revokedLinkId`). -3. **Initialize** the role map with any `overrides`. -4. **Iterate to fixed point.** Repeatedly scan all collaborators. For each edge, compute the role it grants -- `min(edge role, sharer's effective role)`, where the sharer (or share link creator) is the owner (always `build`) or another collaborator's current effective role -- and raise the collaborator's role to the maximum across their valid edges. Raising one collaborator's role may unlock or raise others on the next pass. -5. **Converge.** Roles only ever increase, so the loop terminates when a full pass changes nothing. -6. **Return the role map.** Collaborators absent from the map have no access; collaborators present with a lower role than before have been downgraded. +3. **Iterate to fixed point.** Repeatedly scan all collaborators. For each edge, compute the role it grants -- `min(edge role, sharer's effective role)`, where the sharer (or share link creator) is the owner (always `build`) or another collaborator's current effective role -- and raise the collaborator's role to the maximum across their valid edges. Pending `shareKey` edges are skipped (except the one named by `assumePendingLink`). Raising one collaborator's role may unlock or raise others on the next pass. +4. **Converge.** Roles only ever increase, so the loop terminates when a full pass changes nothing. +5. **Return the role map.** Collaborators absent from the map have no access; collaborators present with a lower role than before have been downgraded. This handles arbitrary graph shapes: diamonds (a user reachable via two independent paths), cycles (mutual adds), and deep chains. @@ -150,13 +151,17 @@ Authorization is enforced at `open()`: the method computes the caller's effectiv Because the role is recomputed from the graph on every `open()`, the live computation is the *sole* source of truth for access -- there is no eager cleanup whose bugs could grant access to an unreachable user. This is what makes lazy revocation safe: severing an edge is enough to deny access, even though the unreachable records linger in storage. +A share-key redemption goes through the same gate without ever granting interim authority: the redeemed edge is written pending (see "Adding collaborators"), the redeeming open() verifies the recipient against the role the edge *would* grant, and only a successful verification confirms the edge. After confirming, the role is re-derived from the live graph and re-checked -- which is what denies a redeemer whose link was revoked while their verification was in flight -- and capped at the role verification covered: a role raised mid-verification (say, an owner grant landing while the recipient sat on the configuration modal) takes effect at their next open, which verifies at the wider role's scope. Confirming additionally requires the workspace's connection/binding topology to be unchanged since verification began: any change (a connection or gadget binding added or removed while the recipient sat on the modal) denies the open and reverts the redemption, and a retry re-verifies against the new topology. + ### Terminating live sessions on revocation Authorization is only checked at `open()`, so a session that is *already* open is not re-checked per message. Without intervention, a collaborator who was just removed or downgraded could keep using their live session until something else disconnected them. To close this gap, `removeCollaborator`/`revokeShareLink` proactively restart the gadget's Overseer DO via `ctx.abort()` whenever the change actually removed or downgraded someone (i.e. the returned `AffectedCollaborator[]` is non-empty; pure no-op removals don't restart). Aborting forcibly disconnects every client; each reconnects and re-runs `open()`, which re-evaluates the now-changed permission graph -- sending removed users to the terminal access-denied page and handing downgraded users their reduced capability (the editor swaps to the `use` view automatically based on `metadata.role`). Since removals are rare (and DOs restart unpredictably anyway, so reconnects are already cheap), the disruption is acceptable. -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. +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. The client discards its retained share key on the first successful open, so this forced reconnect after a removal is keyless and lands the removed collaborator on the access-denied page rather than silently re-redeeming the still-active link (which would undo the removal and break the assumption stated above). The residual is unchanged: the *link* itself survives a collaborator removal under the lazy model, so a recipient who kept the URL can still re-redeem it manually until the owner revokes it -- the discard removes only the client's automatic re-grant. The same residual bounds the one write-side race in this area: a removal landing while a redemption is mid-verification can sever the pending edge, which the verifying open's confirm then re-adds. That race is narrower than it looks -- a pending-only recipient is invisible to `listCollaborators`, so the removal UI cannot target one mid-verification; a racing removal was necessarily aimed at an edge some earlier open had already confirmed. And the re-add grants no incremental authority: the recipient holds the live link and could simply re-redeem it. In every one of these cases, revoking the link is the durable exclusion (`computeEffectiveRoles` skips revoked links, so even a re-added or re-redeemed edge of a revoked link grants nothing). + +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 *in whose verification scope the producing gatekeeper falls* is already a verified observer of it (coverage is held to each collaborator's role scope; see docs/observers.md edge case 4), so no live session ever belongs to someone the flag would newly exclude. Verification, though, is a *live* check re-run at each open: a collaborator whose provider-side access is later revoked may still hold an older session opened while they passed. That is why a failed re-verification scrubs the failed gatekeeper from their persisted observer record (docs/observers.md edge case 3) — from that point the coverage guard blocks that producer's restricted observations, including to the stale session, until a successful re-open. -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. +One party the restart deliberately does not cover is a redeemer whose share link is revoked *while their first open is still verifying them*: their edge is pending, so it is invisible to the revocation's affected-set computation and no restart targets them. They are denied anyway -- after verification confirms the edge, `authorizeCollaborator` re-derives the role from the live graph, where the revoked link contributes nothing. ## Future work diff --git a/packages/workshop-backend/__tests__/restricted-data.test.ts b/packages/workshop-backend/__tests__/restricted-data.test.ts new file mode 100644 index 000000000..a74803539 --- /dev/null +++ b/packages/workshop-backend/__tests__/restricted-data.test.ts @@ -0,0 +1,39 @@ +// The restricted-data flag on persisted observation records must be readable under both its +// current name and its pre-rename one: records written before the rename (commit 6b778a18) carry +// `prohibitAllSharing`, are never rewritten, and anchor the producer-scoped removal guard and +// assertNewSharingAllowed -- a legacy record read as unflagged would let a legacy-latched +// workspace share past a removed producer. + +import { describe, it, expect } from "vitest"; +import { observationContainsRestrictedData } from "../src/overseer.js"; +import type { ObservationDescription } from "@gadgets/workshop-shared/gatekeeper"; + +function description(flags: Record): ObservationDescription { + return { title: "t", description: "d", ...flags } as ObservationDescription; +} + +describe("observationContainsRestrictedData", () => { + it("reads the current field name", () => { + expect(observationContainsRestrictedData(description({ containsRestrictedData: true }))) + .toBe(true); + }); + + it("reads the pre-rename field name on legacy records", () => { + expect(observationContainsRestrictedData(description({ prohibitAllSharing: true }))) + .toBe(true); + }); + + it("is false when neither name is present", () => { + expect(observationContainsRestrictedData(description({}))).toBe(false); + }); + + it("is true when both names are present", () => { + expect(observationContainsRestrictedData( + description({ containsRestrictedData: true, prohibitAllSharing: true }))).toBe(true); + }); + + it("is false for an explicit legacy false", () => { + expect(observationContainsRestrictedData(description({ prohibitAllSharing: false }))) + .toBe(false); + }); +}); diff --git a/packages/workshop-backend/__tests__/sharing.test.ts b/packages/workshop-backend/__tests__/sharing.test.ts index 720908f02..0cfca5d1a 100644 --- a/packages/workshop-backend/__tests__/sharing.test.ts +++ b/packages/workshop-backend/__tests__/sharing.test.ts @@ -44,6 +44,21 @@ function keyEdge(keyId: string, role: CollaboratorRole = "build"): PermissionEdg return { type: "shareKey", keyId, created: new Date(), role }; } +function pendingKeyEdge(keyId: string, role: CollaboratorRole = "build"): PermissionEdge { + return { type: "shareKey", keyId, created: new Date(), role, pending: true }; +} + +// Redeem `rawKey` for `profileId` and confirm the redemption, as a successful open() does. +// Redemption alone adds only a pending edge, which grants nothing. +async function redeemConfirmed( + mgr: SharingManager, rawKey: string, profileId: string): Promise { + let linkId = await mgr.redeemShareKey({ + rawKey, profileId, fetchProfile: async () => profile(profileId), + }); + if (linkId !== null) mgr.confirmShareKeyRedemption(profileId, linkId); + return linkId; +} + function seedCollaborator(storage: SharingStorage, id: string, addedBy: PermissionEdge[]) { storage.collaborators.put({ profile: profile(id), addedBy }); } @@ -96,27 +111,6 @@ describe("authorization", () => { expect(mgr.getEffectiveRole("a")).toBe("use"); expect(mgr.getEffectiveRole("b")).toBe("build"); }); - - it("hasAnyShares reflects current reachability, not table membership", () => { - let { storage, mgr } = makeManager(); - expect(mgr.hasAnyShares()).toBe(false); - - // An active share link counts as a share. - seedLink(storage, "k1", OWNER); - expect(mgr.hasAnyShares()).toBe(true); - - // A revoked link does not. - storage.shareKeys.put({ id: "k1", created: new Date(), createdBy: OWNER, revoked: true }); - expect(mgr.hasAnyShares()).toBe(false); - - // A reachable collaborator counts. - seedCollaborator(storage, "a", [userEdge(OWNER)]); - expect(mgr.hasAnyShares()).toBe(true); - - // A collaborator whose record lingers but is unreachable does not. - storage.collaborators.put({ profile: profile("a"), addedBy: [] }); - expect(mgr.hasAnyShares()).toBe(false); - }); }); describe("redeemShareKey", () => { @@ -140,10 +134,7 @@ describe("redeemShareKey", () => { let { storage, mgr } = makeManager(); let { key } = await mgr.createShareLink({ caller: owner, role: "use" }); - await mgr.redeemShareKey({ - rawKey: key, profileId: "a", - fetchProfile: async () => profile("a"), - }); + await redeemConfirmed(mgr, key, "a"); expect(storage.collaborators.get("a")!.addedBy) .toEqual([expect.objectContaining({ type: "shareKey", role: "use" })]); @@ -169,27 +160,369 @@ describe("redeemShareKey", () => { let { storage, mgr } = makeManager(); let { key } = await mgr.createShareLink({ caller: owner, role: "build" }); + await redeemConfirmed(mgr, key, "a"); + await redeemConfirmed(mgr, key, "a"); + + expect(storage.collaborators.get("a")!.addedBy).toHaveLength(1); + }); + + it("is a no-op for an unknown key", async () => { + let { storage, mgr } = makeManager(); + // A syntactically-valid raw key (hex) that was never created. await mgr.redeemShareKey({ + rawKey: "00112233445566778899aabbccddeeff", profileId: "a", + fetchProfile: async () => profile("a"), + }); + expect(storage.collaborators.get("a")).toBeUndefined(); + }); + + it("returns the link id while there is a pending edge to settle, null once confirmed", async () => { + let { mgr } = makeManager(); + let { key, linkId } = await mgr.createShareLink({ caller: owner, role: "build" }); + + let redeem = () => mgr.redeemShareKey({ rawKey: key, profileId: "a", fetchProfile: async () => profile("a"), }); - await mgr.redeemShareKey({ + await expect(redeem()).resolves.toBe(linkId); // added a pending edge + await expect(redeem()).resolves.toBe(linkId); // still pending -> this attempt settles it too + mgr.confirmShareKeyRedemption("a", linkId); + await expect(redeem()).resolves.toBeNull(); // confirmed edge -> nothing to settle + + await expect(mgr.redeemShareKey({ + rawKey: "00112233445566778899aabbccddeeff", profileId: "b", + fetchProfile: async () => profile("b"), + })).resolves.toBeNull(); // unknown key + }); + + it("revertShareKeyRedemption makes a brand-new recipient unreachable again", async () => { + let { storage, mgr } = makeManager(); + let { key } = await mgr.createShareLink({ caller: owner, role: "build" }); + + let redeemed = await mgr.redeemShareKey({ rawKey: key, profileId: "a", fetchProfile: async () => profile("a"), }); + mgr.revertShareKeyRedemption("a", redeemed!); + + // The record lingers edge-less (lazy model), granting nothing. + expect(storage.collaborators.get("a")!.addedBy).toEqual([]); + expect(mgr.getEffectiveRole("a")).toBeUndefined(); + expect(ids(mgr.listCollaborators())).toEqual([]); + }); + it("a redemption that raced a completed one merges instead of clobbering", async () => { + let { storage, mgr } = makeManager(); + let { key, linkId } = await mgr.createShareLink({ caller: owner, role: "build" }); + + // While A's redemption is fetching the profile (the one await), a concurrent redemption of + // the same key completes fully -- redeem plus confirm. A's re-read after the await must find + // that record and settle against it, not overwrite it with a fresh pending edge. + let result = await mgr.redeemShareKey({ + rawKey: key, profileId: "a", + fetchProfile: async () => { + await redeemConfirmed(mgr, key, "a"); + return profile("a"); + }, + }); + + // The confirmed edge means there is nothing for A to settle... + expect(result).toBeNull(); + let edges = storage.collaborators.get("a")!.addedBy; + expect(edges).toEqual([expect.objectContaining({ type: "shareKey", keyId: linkId })]); + expect(edges[0]).not.toHaveProperty("pending"); + + // ...and A's failed-verification revert (which only severs pending edges) leaves the + // concurrent open's grant intact. + mgr.revertShareKeyRedemption("a", linkId); expect(storage.collaborators.get("a")!.addedBy).toHaveLength(1); + expect(mgr.getEffectiveRole("a")).toBe("build"); }); - it("is a no-op for an unknown key", async () => { + it("a redemption that raced a still-pending one settles the shared edge", async () => { let { storage, mgr } = makeManager(); - // A syntactically-valid raw key (hex) that was never created. - await mgr.redeemShareKey({ - rawKey: "00112233445566778899aabbccddeeff", profileId: "a", + let { key, linkId } = await mgr.createShareLink({ caller: owner, role: "build" }); + + // The concurrent redemption is still mid-verification: its pending edge is found on the + // re-read, not duplicated, and A gets the link id back so it settles the edge itself. + let result = await mgr.redeemShareKey({ + rawKey: key, profileId: "a", + fetchProfile: async () => { + await mgr.redeemShareKey({ + rawKey: key, profileId: "a", fetchProfile: async () => profile("a"), + }); + return profile("a"); + }, + }); + + expect(result).toBe(linkId); + expect(storage.collaborators.get("a")!.addedBy).toEqual( + [expect.objectContaining({ type: "shareKey", keyId: linkId, pending: true })]); + }); + + it("revertShareKeyRedemption severs only the redeemed edge", async () => { + let { storage, mgr } = makeManager(); + let { key } = await mgr.createShareLink({ caller: owner, role: "build" }); + seedCollaborator(storage, "a", [userEdge(OWNER, "use")]); + + let redeemed = await mgr.redeemShareKey({ + rawKey: key, profileId: "a", fetchProfile: async () => profile("a"), }); + // The pending grant is visible only to the verifying open itself. + expect(mgr.getEffectiveRole("a")).toBe("use"); + expect(mgr.getEffectiveRole("a", redeemed!)).toBe("build"); + + mgr.revertShareKeyRedemption("a", redeemed!); + + // The pre-existing user edge survives; only the shareKey grant is gone. + expect(storage.collaborators.get("a")!.addedBy).toEqual([ + expect.objectContaining({ type: "user", sharer: OWNER, role: "use" }), + ]); + expect(mgr.getEffectiveRole("a")).toBe("use"); + }); + + it("a throwing assertGrantAllowed rejects a new recipient with nothing persisted", async () => { + let { storage, mgr } = makeManager(); + let { key } = await mgr.createShareLink({ caller: owner, role: "build" }); + + await expect(mgr.redeemShareKey({ + rawKey: key, profileId: "a", fetchProfile: async () => profile("a"), + assertGrantAllowed: () => { throw new Error("sharing is closed"); }, + })).rejects.toThrow(/sharing is closed/); + + // No collaborator record and no edge were written. expect(storage.collaborators.get("a")).toBeUndefined(); }); + + it("does not invoke assertGrantAllowed for an already-confirmed edge", async () => { + let { mgr } = makeManager(); + let { key } = await mgr.createShareLink({ caller: owner, role: "build" }); + await redeemConfirmed(mgr, key, "a"); + + // A confirmed edge is an existing grant, not a new one: the redemption stays a no-op even + // when policy forbids new sharing (a collaborator re-opening with a retained key). + await expect(mgr.redeemShareKey({ + rawKey: key, profileId: "a", fetchProfile: async () => profile("a"), + assertGrantAllowed: () => { throw new Error("sharing is closed"); }, + })).resolves.toBeNull(); + }); + + it("gates settling a still-pending edge, leaving it untouched on refusal", async () => { + let { storage, mgr } = makeManager(); + let { key, linkId } = await mgr.createShareLink({ caller: owner, role: "build" }); + await mgr.redeemShareKey({ + rawKey: key, profileId: "a", fetchProfile: async () => profile("a"), + }); + + // Settling a pending edge completes a new grant, so it is refused too. + await expect(mgr.redeemShareKey({ + rawKey: key, profileId: "a", fetchProfile: async () => profile("a"), + assertGrantAllowed: () => { throw new Error("sharing is closed"); }, + })).rejects.toThrow(/sharing is closed/); + + // The pre-existing pending edge survives for its own open to settle. + expect(storage.collaborators.get("a")!.addedBy).toEqual([ + expect.objectContaining({ type: "shareKey", keyId: linkId, pending: true }), + ]); + }); + + it("invokes a passing assertGrantAllowed once and writes the pending edge", async () => { + let { storage, mgr } = makeManager(); + let { key, linkId } = await mgr.createShareLink({ caller: owner, role: "build" }); + + let calls = 0; + await expect(mgr.redeemShareKey({ + rawKey: key, profileId: "a", fetchProfile: async () => profile("a"), + assertGrantAllowed: () => { calls++; }, + })).resolves.toBe(linkId); + + expect(calls).toBe(1); + expect(storage.collaborators.get("a")!.addedBy).toEqual([ + expect.objectContaining({ type: "shareKey", keyId: linkId, pending: true }), + ]); + }); +}); + +describe("pending redemptions", () => { + it("a pending redemption grants nothing until confirmed", async () => { + let { storage, mgr } = makeManager(); + let { key, linkId } = await mgr.createShareLink({ caller: owner, role: "build" }); + await mgr.redeemShareKey({ + rawKey: key, profileId: "a", fetchProfile: async () => profile("a"), + }); + + // The edge exists but is pending: invisible to roles, listings, and downstream grants. + expect(storage.collaborators.get("a")!.addedBy) + .toEqual([expect.objectContaining({ type: "shareKey", keyId: linkId, pending: true })]); + expect(mgr.getEffectiveRole("a")).toBeUndefined(); + expect(ids(mgr.listCollaborators())).toEqual([]); + seedCollaborator(storage, "b", [userEdge("a")]); + expect(mgr.getEffectiveRole("b")).toBeUndefined(); + + mgr.confirmShareKeyRedemption("a", linkId); + expect(mgr.getEffectiveRole("a")).toBe("build"); + expect(ids(mgr.listCollaborators())).toEqual(["a", "b"]); + }); + + it("assumePendingLink models the grant only for the verifying profile", async () => { + let { storage, mgr } = makeManager(); + let { key, linkId } = await mgr.createShareLink({ caller: owner, role: "build" }); + await mgr.redeemShareKey({ + rawKey: key, profileId: "c", fetchProfile: async () => profile("c"), + }); + + // The verifying open counts its own pending edge... + expect(mgr.getEffectiveRole("c", linkId)).toBe("build"); + // ...but the edge contributes nothing to anyone else's role, even in the same computation: + // a user downstream of c stays unreachable when the open verifying a different profile asks. + seedCollaborator(storage, "a", [userEdge("c")]); + expect(mgr.getEffectiveRole("a", linkId)).toBeUndefined(); + }); + + it("assumePendingLink is bounded by the link creator's effective role", () => { + let { storage, mgr } = makeManager(); + // "s" created a build link but is themselves only a "use" collaborator. + seedCollaborator(storage, "s", [userEdge(OWNER, "use")]); + seedLink(storage, "k1", "s", "build"); + seedCollaborator(storage, "a", [pendingKeyEdge("k1", "build")]); + expect(mgr.getEffectiveRole("a", "k1")).toBe("use"); + }); + + it("assumePendingLink grants nothing for a revoked link", async () => { + let { mgr } = makeManager(); + let { key, linkId } = await mgr.createShareLink({ caller: owner, role: "build" }); + await mgr.redeemShareKey({ + rawKey: key, profileId: "a", fetchProfile: async () => profile("a"), + }); + mgr.revokeShareLink(owner, linkId, []); + expect(mgr.getEffectiveRole("a", linkId)).toBeUndefined(); + }); + + it("confirmShareKeyRedemption clears the pending flag and is idempotent", async () => { + let { storage, mgr } = makeManager(); + let { key, linkId } = await mgr.createShareLink({ caller: owner, role: "use" }); + await mgr.redeemShareKey({ + rawKey: key, profileId: "a", fetchProfile: async () => profile("a"), + }); + + mgr.confirmShareKeyRedemption("a", linkId); + mgr.confirmShareKeyRedemption("a", linkId); + + let edges = storage.collaborators.get("a")!.addedBy; + expect(edges).toEqual([expect.objectContaining({ type: "shareKey", keyId: linkId })]); + expect(edges[0]).not.toHaveProperty("pending"); + expect(mgr.getEffectiveRole("a")).toBe("use"); + }); + + it("confirmShareKeyRedemption gates settling a pending edge, leaving it pending on refusal", + async () => { + let { storage, mgr } = makeManager(); + let { key, linkId } = await mgr.createShareLink({ caller: owner, role: "build" }); + await mgr.redeemShareKey({ + rawKey: key, profileId: "a", fetchProfile: async () => profile("a"), + }); + + // The redemption gate passed with the pending write, but policy changed before the confirm + // (the redeeming open's await windows): the confirm is the granting write, so it re-asserts. + expect(() => mgr.confirmShareKeyRedemption("a", linkId, + () => { throw new Error("sharing is closed"); })).toThrow(/sharing is closed/); + + // The edge is still pending, granting nothing. + expect(storage.collaborators.get("a")!.addedBy) + .toEqual([expect.objectContaining({ type: "shareKey", keyId: linkId, pending: true })]); + expect(mgr.getEffectiveRole("a")).toBeUndefined(); + expect(ids(mgr.listCollaborators())).toEqual([]); + }); + + it("confirmShareKeyRedemption gates the missing-edge re-add too", async () => { + let { storage, mgr } = makeManager(); + let { key, linkId } = await mgr.createShareLink({ caller: owner, role: "build" }); + await mgr.redeemShareKey({ + rawKey: key, profileId: "a", fetchProfile: async () => profile("a"), + }); + + // A concurrent revert (or a racing owner removal) severed the pending edge; the re-add is a + // granting write like any other, so the policy refuses it with nothing persisted. + mgr.revertShareKeyRedemption("a", linkId); + expect(() => mgr.confirmShareKeyRedemption("a", linkId, + () => { throw new Error("sharing is closed"); })).toThrow(/sharing is closed/); + + expect(storage.collaborators.get("a")!.addedBy).toEqual([]); + expect(mgr.getEffectiveRole("a")).toBeUndefined(); + }); + + it("confirmShareKeyRedemption does not invoke the gate for an already-confirmed edge", + async () => { + let { mgr } = makeManager(); + let { key, linkId } = await mgr.createShareLink({ caller: owner, role: "build" }); + await redeemConfirmed(mgr, key, "a"); + + // A confirmed edge is an existing grant, not a new one: re-confirming stays a no-op even + // when policy forbids new sharing. + expect(() => mgr.confirmShareKeyRedemption("a", linkId, + () => { throw new Error("sharing is closed"); })).not.toThrow(); + expect(mgr.getEffectiveRole("a")).toBe("build"); + }); + + it("confirmShareKeyRedemption re-adds an edge a concurrent revert severed", async () => { + let { storage, mgr } = makeManager(); + let { key, linkId } = await mgr.createShareLink({ caller: owner, role: "use" }); + await mgr.redeemShareKey({ + rawKey: key, profileId: "a", fetchProfile: async () => profile("a"), + }); + + // A parallel open of the same link failed verification and reverted while this open's + // verification was succeeding. The successful open's confirm must still land the grant. + mgr.revertShareKeyRedemption("a", linkId); + expect(storage.collaborators.get("a")!.addedBy).toEqual([]); + + mgr.confirmShareKeyRedemption("a", linkId); + expect(storage.collaborators.get("a")!.addedBy) + .toEqual([expect.objectContaining({ type: "shareKey", keyId: linkId, role: "use" })]); + expect(mgr.getEffectiveRole("a")).toBe("use"); + }); + + it("revertShareKeyRedemption leaves a confirmed edge alone", async () => { + let { storage, mgr } = makeManager(); + let { key, linkId } = await mgr.createShareLink({ caller: owner, role: "build" }); + await redeemConfirmed(mgr, key, "a"); + + // A parallel open of the same link failed verification after this one confirmed; its revert + // must not take away the grant the successful open established. + mgr.revertShareKeyRedemption("a", linkId); + + expect(storage.collaborators.get("a")!.addedBy).toHaveLength(1); + expect(mgr.getEffectiveRole("a")).toBe("build"); + }); + + it("a redemption confirmed after its link was revoked grants nothing", async () => { + let { mgr } = makeManager(); + let { key, linkId } = await mgr.createShareLink({ caller: owner, role: "build" }); + await mgr.redeemShareKey({ + rawKey: key, profileId: "a", fetchProfile: async () => profile("a"), + }); + mgr.revokeShareLink(owner, linkId, []); + + // Confirming cannot resurrect revoked authority: the confirmed edge is inert. + mgr.confirmShareKeyRedemption("a", linkId); + expect(mgr.getEffectiveRole("a")).toBeUndefined(); + expect(ids(mgr.listCollaborators())).toEqual([]); + }); + + it("a pending build edge does not raise the caller's sharing authority", async () => { + let { storage, mgr } = makeManager(); + seedCollaborator(storage, "a", [userEdge(OWNER, "use")]); + let { key } = await mgr.createShareLink({ caller: owner, role: "build" }); + await mgr.redeemShareKey({ + rawKey: key, profileId: "a", fetchProfile: async () => profile("a"), + }); + + // Sharing mutators read the live graph, where the pending edge is invisible: "a" is still + // only a "use" collaborator and cannot mint build-role grants. + await expect(mgr.createShareLink({ caller: collab("a"), role: "build" })) + .rejects.toThrow(/higher than your own/); + }); }); describe("addCollaborator", () => { @@ -469,10 +802,7 @@ describe("revokeShareLink", () => { mgr.revokeShareLink(owner, linkId, []); - await mgr.redeemShareKey({ - rawKey: key, profileId: "a", - fetchProfile: async () => profile("a"), - }); + await redeemConfirmed(mgr, key, "a"); expect(storage.collaborators.get("a")).toBeUndefined(); }); @@ -501,6 +831,26 @@ describe("createShareLink", () => { expect(() => mgr.createShareLink({ caller: collab("a"), role: "build" })) .rejects.toThrow(/higher than your own/); }); + + it("a throwing assertGrantAllowed aborts with nothing persisted", async () => { + let { storage, mgr } = makeManager(); + await expect(mgr.createShareLink({ + caller: owner, role: "build", + assertGrantAllowed: () => { throw new Error("sharing is closed"); }, + })).rejects.toThrow(/sharing is closed/); + // The minted key was discarded, never stored. + expect([...storage.shareKeys.list()]).toEqual([]); + }); + + it("invokes assertGrantAllowed once and persists the grant when it passes", async () => { + let { mgr } = makeManager(); + let calls = 0; + let { linkId } = await mgr.createShareLink({ + caller: owner, role: "use", assertGrantAllowed: () => { calls++; }, + }); + expect(calls).toBe(1); + expect(mgr.listShareLinkRecords().map(r => r.id)).toEqual([linkId]); + }); }); describe("newShareLinkKey", () => { @@ -521,8 +871,8 @@ describe("newShareLinkKey", () => { expect(listed[0].note).toBe("team"); // Both secrets redeem, and a user redeeming both gets a single (deduplicated) edge. - await mgr.redeemShareKey({ rawKey: key1, profileId: "a", fetchProfile: async () => profile("a") }); - await mgr.redeemShareKey({ rawKey: key2, profileId: "a", fetchProfile: async () => profile("a") }); + await redeemConfirmed(mgr, key1, "a"); + await redeemConfirmed(mgr, key2, "a"); expect(storage.collaborators.get("a")!.addedBy).toHaveLength(1); expect(mgr.getEffectiveRole("a")).toBe("use"); }); @@ -540,7 +890,7 @@ describe("newShareLinkKey", () => { // Neither the original nor the copied secret can be redeemed anymore. for (let rawKey of [key1, key2]) { - await mgr.redeemShareKey({ rawKey, profileId: "a", fetchProfile: async () => profile("a") }); + await redeemConfirmed(mgr, rawKey, "a"); } expect(storage.collaborators.get("a")).toBeUndefined(); }); @@ -562,6 +912,23 @@ describe("newShareLinkKey", () => { .rejects.toThrow(/higher than your own/); }); + it("a throwing assertGrantAllowed aborts the copy with nothing persisted", async () => { + let { storage, mgr } = makeManager(); + let { linkId } = await mgr.createShareLink({ caller: owner, role: "build" }); + + await expect(mgr.newShareLinkKey({ + caller: owner, linkId, + assertGrantAllowed: () => { throw new Error("sharing is closed"); }, + })).rejects.toThrow(/sharing is closed/); + // Only the original link record remains; the aborted copy's key was never stored. + expect([...storage.shareKeys.list()].map(r => r.id)).toEqual([linkId]); + + let calls = 0; + await mgr.newShareLinkKey({ caller: owner, linkId, assertGrantAllowed: () => { calls++; } }); + expect(calls).toBe(1); + expect([...storage.shareKeys.list()]).toHaveLength(2); + }); + it("cannot manage a link through the id of one of its copies", async () => { let { storage, mgr } = makeManager(); await mgr.createShareLink({ caller: owner, role: "build" }); @@ -595,8 +962,7 @@ describe("pre-copy share keys", () => { // Such a link can also be copied, and the copy grants the same access. let { key } = await mgr.newShareLinkKey({ caller: owner, linkId: "hash1" }); - await mgr.redeemShareKey( - { rawKey: key, profileId: "b", fetchProfile: async () => profile("b") }); + await redeemConfirmed(mgr, key, "b"); expect(mgr.getEffectiveRole("b")).toBe("use"); }); }); diff --git a/packages/workshop-backend/src/overseer.ts b/packages/workshop-backend/src/overseer.ts index 3b0723f3e..389b082e4 100644 --- a/packages/workshop-backend/src/overseer.ts +++ b/packages/workshop-backend/src/overseer.ts @@ -41,7 +41,7 @@ import type { ProductAnalyticsConnectionType, ProductAnalyticsGadgetInput } from 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 } from "./sharing"; +import { SharingManager, SharingCaller, CollaboratorRecord, ShareKeyRecord, roleRank } from "./sharing"; import { AutoApprovalDrainer } from "./auto-approval"; import { collectSlashCommands, invokeSlashCommand } from "./slash-commands"; import { createWorkshopLogger, obsContext, traced } from "./observability"; @@ -1009,8 +1009,13 @@ export 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 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 + // observed restricted data. prohibitAllSharing: false, }, @@ -1347,6 +1352,22 @@ export function sanitizeMessageFormatRefs( return accepted.toSorted((a, b) => a.position - b.position); } +// Action records written before commit 6b778a18 carry `containsRestrictedData` under its +// pre-rename name, `prohibitAllSharing`. Records are data at rest and are never rewritten, so +// this shape is permanent (matching the latch singleton, which kept its historical storage key) +// and the tolerance can never be removed. +type LegacyObservationDescription = ObservationDescription & { prohibitAllSharing?: boolean }; + +/** + * Whether a persisted observation description carries the restricted-data flag, under either its + * current name or the pre-rename one still present on older records. Exported for its unit test; + * every read of the flag off a persisted record must go through this. + */ +export function observationContainsRestrictedData(description: ObservationDescription): boolean { + let d: LegacyObservationDescription = description; + return (d.containsRestrictedData ?? d.prohibitAllSharing) === true; +} + class OverseerImpl implements AgentHooks { public storage: OverseerStorage; readonly logger: ReturnType; @@ -4356,15 +4377,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."); - } - - this.storage.prohibitAllSharing.put(true); + if (description.containsRestrictedData) { + await this.#assertSensitiveObservationCoverage(gatekeeperId); } // Forward exclusion: the gatekeeper may name observers who must not see this observation. Since @@ -4376,6 +4390,12 @@ class OverseerImpl implements AgentHooks { await this.#enforceExcludeObservers(description.excludeObservers); } + // Latch restricted mode only once every gate above has passed: a blocked observation delivers + // no data, so it must not leave the workspace restricted. + if (description.containsRestrictedData) { + this.storage.prohibitAllSharing.put(true); + } + let actionId = this.storage.nextActionId.get(); this.storage.nextActionId.put(actionId + 1); @@ -4490,6 +4510,119 @@ 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 when a collaborator enters (see authorizeCollaborator), 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. Coverage is held to each collaborator's own + // verification scope (#inScopeGatekeepers of their role): ensureObserver never verifies a "use" + // collaborator against a gatekeeper no gadget binds, so demanding coverage there would block the + // read permanently and make the error's remedy (re-open the workspace) a lie. Unredeemed share + // links do NOT block: redemption is gated at open(), where ensureObserver runs before the new + // collaborator sees anything -- and a redemption stays *pending* (invisible to effective roles, + // hence to listCollaborators here) until that verification succeeds, so neither a + // mid-verification nor a refused recipient ever counts here. A pending redeemer being + // invisible is safe only because authorizeCollaborator denies the redeeming open if the + // connection/binding topology changed while its verification was in flight: a redeemer is + // never confirmed against a scope narrower than what exists at confirm time, so anyone this + // guard can't see is either denied or was verified against the producer being checked. + 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. + } + } + // Computed from the gadget bindings directly rather than via #inScopeGatekeepers, which + // throws if *any* record is a legacy connection -- an unrelated legacy record must not make + // this gatekeeper's sensitive reads fail. + let inUseScope = + vendorId !== null && this.#gadgetBoundGatekeeperIds().has(gatekeeperId); + + for (let collaborator of collaborators) { + // A verifiable gatekeeper outside a "use" collaborator's scope is one the UI can't invoke + // and ensureObserver can't cover; only the unverifiable case above blocks regardless of + // role. The skip also covers a *formerly*-bound producer (unbinding shrinks use scope + // live) and a *never*-bound one reachable only through chat bindings, whose restricted + // data the agent may have persisted with no "use" collaborator ever verified against it + // -- both accepted residuals; see docs/observers.md edge case 4. An absent role means + // "build" (see CollaboratorInfo), which fails safe here. + if (vendorId && (collaborator.role ?? "build") === "use" && !inUseScope) continue; + let observer = vendorId ? this.storage.observers.get(collaborator.profile.id) : undefined; + if (!observer || !(gatekeeperId in observer.accountChoices)) { + // The message reaches sandboxed gadget code and agent output -- an audience that can't + // otherwise list collaborators -- so it names the collaborator but omits their profile + // id, which is the full email on OAuth/CF Access deployments. The name is never a full + // email on any path (password usernames are normalized alphanumeric handles, OAuth + // display names default to the email local-part), which is what makes keeping it + // acceptable for this audience. + throw new Error( + "This observation was blocked because it contains sensitive data, but this " + + `workspace is shared with ${collaborator.profile.name}, ` + + "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."); + } + } + } + + // The connection ids through which this workspace has read restricted data -- the producers the + // `prohibitAllSharing` latch guards. Derived by scanning the action log for observations whose + // description carries `containsRestrictedData` (under either of its names -- see + // observationContainsRestrictedData): action records are never deleted, so the set survives the + // producer connection's removal, and the latch is written in the same synchronous block that + // 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. + restrictedProducerIds(): Set { + let producers = new Set(); + for (let record of this.storage.actions.list()) { + if (record.type === "observation" && + observationContainsRestrictedData(record.description) && + record.gatekeeperId !== BUILTIN_TOOL_GATEKEEPER_ID) { + producers.add(record.gatekeeperId); + } + } + return producers; + } + + // Refuse a new sharing grant once the workspace has read restricted data through a connection + // that no longer exists. A new collaborator's verification anchors on the producer connection's + // record (ensureObserver and the coverage guard); with the record gone there is nothing to + // verify them against, while the restricted data persists in chat history and storage. Grants + // that predate the removal are untouched -- this guards only new ones, which includes share-key + // *redemption* (open() passes this as redeemShareKey's assertGrantAllowed), so outstanding keys + // die with the producer rather than staying redeemable. Removing a *verifiable* + // producer already requires zero collaborators and zero share links (see + // GatekeeperClientImpl.remove()), so this bites after any producer was removed while the + // workspace was unshared, or after an unverifiable producer -- exempt from the removal guard + // as its own remedy -- was removed while links were outstanding. + assertNewSharingAllowed(): void { + if (!this.storage.prohibitAllSharing.get()) return; + for (let id of this.restrictedProducerIds()) { + if (!this.storage.gatekeepers.get(id)) { + throw new Error( + "This workspace can no longer be shared: it read sensitive data through a connection " + + "that has since been removed, so new collaborators can no longer be verified for " + + "access to that data."); + } + } + } + // 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. @@ -7670,23 +7803,40 @@ class OverseerImpl implements AgentHooks { } } + // Gatekeeper ids bound by some non-provisional gadget -- everything the gadget UI can invoke, + // and therefore all of a "use" collaborator's verification scope. + #gadgetBoundGatekeeperIds(): Set { + let boundIds = new Set(); + for (let gadget of this.storage.gadgets.list()) { + // Provisional gadgets and binding edges aren't visible to "use" collaborators, so they + // don't bring gatekeepers into scope. + if (gadget.pending) continue; + for (let [, edge] of this.visibleBindings(gadget)) { + boundIds.add(edge.target); + } + } + return boundIds; + } + + // The raw inputs every role's verification scope is derived from (see #inScopeGatekeepers): + // the connection set and the gadget-bound subset, serialized for cheap equality. + // authorizeCollaborator compares this across a pending redemption's verification to detect a + // scope change mid-flight. Raw inputs rather than #inScopeGatekeepers output: that helper + // throws on legacy records, and a removal or bind-change must register just like an addition + // -- including of connections no verification could cover. + #verificationScopeFingerprint(): string { + let connectionIds = + [...this.storage.gatekeepers.list()].map(gk => gk.id).toSorted((a, b) => a - b); + let boundIds = [...this.#gadgetBoundGatekeeperIds()].toSorted((a, b) => a - b); + return JSON.stringify([connectionIds, boundIds]); + } + // Selects the gatekeepers a non-owner observer with the given `role` must be verified against: // - "build" collaborators (full access): every account-requiring gatekeeper. // - "use" collaborators (UI only): only account-requiring gatekeepers bound by some gadget, // since that is all the UI can invoke. #inScopeGatekeepers(role: CollaboratorRole): GatekeeperRecord[] { - let boundIds: Set | undefined; - if (role === "use") { - boundIds = new Set(); - for (let gadget of this.storage.gadgets.list()) { - // Provisional gadgets and binding edges aren't visible to "use" collaborators, so they - // don't bring gatekeepers into scope. - if (gadget.pending) continue; - for (let [, edge] of this.visibleBindings(gadget)) { - boundIds.add(edge.target); - } - } - } + let boundIds = role === "use" ? this.#gadgetBoundGatekeeperIds() : undefined; let result: GatekeeperRecord[] = []; for (let gk of this.storage.gatekeepers.list()) { @@ -7762,6 +7912,85 @@ class OverseerImpl implements AgentHooks { } } + // The authorization gate every non-owner entry point (open(), receiveExternalMessage()) must + // pass through: resolve the caller's effective role, then verify them as an observer of + // everything this workspace has read. Returns null for no access; verification failures throw. + // A caller that requires at least `requireRole` (e.g. receiveExternalMessage needs "build") + // passes it so an insufficient role is denied *before* verification runs -- otherwise the caller + // would be verified (real addObserver calls, a persisted observer record) only to be turned + // away, or worse, told to fix a verification failure that can never grant them access. + // `configureCb` is forwarded to ensureObserver to prompt for unconfigured account choices; + // without it, verification is non-interactive and an unconfigured binding denies access. + // `pendingLinkId` names a share-key redemption this same open() just performed (see + // redeemShareKey): the redeemed edge is pending -- it grants no authority to anyone -- so the + // role is computed as though it were confirmed, verification runs against that hypothetical + // role, and only on success is the edge confirmed for real. + async authorizeCollaborator( + profileId: string, + clientUser: DurableObjectStub, + opts: { + configureCb?: RpcStub; + requireRole?: CollaboratorRole; + pendingLinkId?: string; + } = {}): Promise { + let sharing = await this.getSharingManager(); + let role = sharing.getEffectiveRole(profileId, opts.pendingLinkId); + if (!role || (opts.requireRole && roleRank(role) < roleRank(opts.requireRole))) return null; + + // For a pending redemption, snapshot the verification-scope inputs in the same synchronous + // tick as ensureObserver's own #inScopeGatekeepers snapshot. ensureObserver may park + // unboundedly on the recipient's configuration modal, and a connection added (or a binding + // made) in that window would otherwise be invisible to the verification yet covered by the + // confirmed grant -- an already-confirmed collaborator in the same window is caught by the + // coverage guard, but a pending redeemer is invisible to it. + let scopeBefore = + opts.pendingLinkId !== undefined ? this.#verificationScopeFingerprint() : null; + + await this.ensureObserver(profileId, clientUser, role, opts.configureCb); + if (opts.pendingLinkId === undefined) return role; + + // Deny on *any* topology change -- additions, removals, and bind-changes alike -- rather + // than re-verifying: the throw lands in open()'s catch, which severs the pending edge, and + // the recipient's retained share key makes the retry re-redeem and re-verify against the + // full new topology (where the redemption policy gate refuses if a producer is now gone). + // This check, the confirm, and the role re-derivation below form one synchronous block, so + // any change landing after it sees the recipient in listCollaborators() and the coverage + // guard fail-closes -- the same protection ordinary opens have. + if (scopeBefore !== this.#verificationScopeFingerprint()) { + throw new Error( + "A connection or binding changed in this workspace while your access was being " + + "verified. Open the workspace again to retry."); + } + + // Re-assert the redemption policy in the same synchronous block as the granting write. The + // gate at redeemShareKey ran with the *pending* write, before this open's await windows + // (ensureCapsules, ensureObserver); a restricted-data producer removed *before* the + // fingerprint snapshot above was taken is absent from both fingerprints -- an unverifiable + // producer's remove() skips the share-link guard entirely -- so the scope check cannot catch + // it, and confirming would admit a recipient nobody can verify for the restricted data. The + // throw lands in open()'s catch, which severs the pending edge. + sharing.confirmShareKeyRedemption( + profileId, opts.pendingLinkId, () => this.assertNewSharingAllowed()); + // Re-derive the role from the live graph now that the edge is confirmed. Pending edges are + // invisible to revokeShareLink's affected-set computation, so no revocation restart aborts a + // mid-verification redeemer whose link was revoked while verification waited (possibly on the + // configuration modal) -- this re-check is what denies them: a revoked link contributes + // nothing, so the role collapses. The just-confirmed edge then lingers inert, like any edge + // of a revoked link under the lazy model. + let confirmed = sharing.getEffectiveRole(profileId); + if (!confirmed || + (opts.requireRole && roleRank(confirmed) < roleRank(opts.requireRole))) { + return null; + } + // Decreases pass through (this re-derivation is the revocation catch above), but an increase + // -- say an owner grant of "build" landing while verification waited on the configuration + // modal -- must not ride out on this open: ensureObserver verified the caller at `role`, and + // a wider role widens the gatekeeper scope that verification must cover. The raise takes + // effect at the caller's next open, exactly as it does for an ordinary (keyless) open, which + // returns the role it verified at. + return roleRank(confirmed) < roleRank(role) ? confirmed : role; + } + // Bring a non-owner `profileId` into compliance as an observer for their `role`, so that they may // open the Gadget. May invoke `configureCb` to ask the user to choose connected accounts for // gatekeeper bindings they haven't configured yet. Re-runs `addObserver` (re-verification) for @@ -7793,6 +8022,9 @@ class OverseerImpl implements AgentHooks { let observerId = record?.observerId ?? crypto.randomUUID(); // Gatekeepers we successfully registered the observer with during this call. let newlyAdded = new Set(); + // Gatekeepers that refused (or whose account was gone) during this call -- see fail() below, + // which scrubs each from the persisted observer record as the failure is determined. + let invalidated = new Set(); // Failures from the previous pass, keyed by gatekeeper id: an already-configured binding whose // chosen account was disconnected, or which the gatekeeper refused. @@ -7883,6 +8115,21 @@ class OverseerImpl implements AgentHooks { let fail = (reason: string, err?: unknown) => { failures.set(gk.id, {accountId, reason}); + // The coverage guard (#assertSensitiveObservationCoverage) reads the *persisted* + // record from other turns, so until this gatekeeper is scrubbed from it, the record + // keeps admitting this producer's restricted observations to the collaborator's + // still-live sessions -- even though the live check just refused them. Scrub it + // synchronously with the failure determination (the record is re-read because the + // awaits since load may have let a concurrent open update it; get/put are synchronous + // in this single-threaded DO, so nothing lands between the check and the write). + // Scoped to the failed gatekeeper: coverage elsewhere stays intact, and a repaired + // pass re-persists full coverage at step 6. + invalidated.add(gk.id); + let persisted = this.storage.observers.get(profileId); + if (persisted && gk.id in persisted.accountChoices) { + delete persisted.accountChoices[gk.id]; + this.storage.observers.put(persisted); + } this.logger.warn("observer verification failed", { event: "gatekeeper.observer.verify.failed", gatekeeperId: gk.id, vendorId, accountId, observerId, error: err, @@ -7936,9 +8183,12 @@ class OverseerImpl implements AgentHooks { break; } } catch (err) { - // Best-effort remove all the observers that were newly-added since we didn't persist the - // user's observer record. - await this.#removeObserverFromGatekeepers(observerId, [...newlyAdded]); + // Best-effort deregistration: the newly-added observers were never persisted, and a + // gatekeeper that refused this call may still hold a registration from an earlier + // successful open (its persisted coverage was already scrubbed in fail(), and + // removeObserver is idempotent per the interface contract). + await this.#removeObserverFromGatekeepers( + observerId, [...new Set([...newlyAdded, ...invalidated])]); throw err; } @@ -8231,46 +8481,71 @@ 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 - // appear in the collaborators table. + // appear in the collaborators table. Redemption adds only a *pending* edge, which grants + // nothing to anyone until observer verification below confirms it; the redeemed link id is + // kept so authorizeCollaborator can verify against the hypothetical grant and confirm it on + // success, and so a refusal severs the still-pending edge. + let redeemedLinkId: string | null = null; if (shareKey) { - await sharing.redeemShareKey({ + redeemedLinkId = await sharing.redeemShareKey({ rawKey: shareKey, profileId, fetchProfile: () => clientUser.whoami(), + // An outstanding key is a new grant vector, so redemption is policy-gated like the + // grant-creating mutators. Without this, keys minted before an exempted + // (unverifiable-producer) removal -- or on a legacy-latched workspace whose producer is + // gone -- would still admit unverified recipients. + assertGrantAllowed: () => this.impl.assertNewSharingAllowed(), }); } - // Check authorization. Compute the caller's effective role from the permission graph; this - // both authorizes the session and determines which capability we hand back. - // - // An unauthorized caller (no effective role -- never had access, or was removed) gets a - // distinct denial without workspace metadata. A removed collaborator who reconnects after - // their session is force-restarted lands here and sees the terminal access-denied page. - let effectiveRole = sharing.getEffectiveRole(profileId); - if (!effectiveRole) { - throw createOpenGadgetError(OPEN_GADGET_ERROR_CODES.workspaceAccessDenied); - } - role = effectiveRole; - // Ambient reconciliation may attach Gatekeepers after open() starts. Finish it before taking // the observer snapshot so every capability exposed to this collaborator has an observer. await ensureCapsules; - // 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. - await this.impl.ensureObserver(profileId, clientUser, role, configureObservers); + // Check authorization: compute the caller's effective role from the permission graph, then + // verify they may observe everything this Gadget has read through its in-scope gatekeepers, + // configuring their connected accounts if needed. Observer verification runs only after a + // valid role is confirmed, so it never reveals gatekeeper or resource metadata to an + // unauthorized user -- who instead gets a distinct denial without workspace metadata. (A + // removed collaborator who reconnects after their session is force-restarted lands there + // and sees the terminal access-denied page.) Verification 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. + let effectiveRole: CollaboratorRole | null; + try { + effectiveRole = await this.impl.authorizeCollaborator( + profileId, clientUser, + {configureCb: configureObservers, pendingLinkId: redeemedLinkId ?? undefined}); + } catch (err) { + // Verification refused the caller. If this open() had just redeemed a share key, sever + // the still-pending edge it added, so a refused recipient never persists in the sharing + // graph. (An edge a concurrent open of the same link already confirmed is left alone.) + // They can redeem the same key again once their access is fixed. (The revert persists + // despite the rethrow: DO storage is not rolled back when an RPC throws.) + if (redeemedLinkId) { + sharing.revertShareKeyRedemption(profileId, redeemedLinkId); + } + throw err; + } + if (!effectiveRole) { + // A null role means the redeemed link's creator is currently unreachable in the + // permission graph -- or the link itself was revoked while verification was in flight + // (authorizeCollaborator re-derives the role after confirming the edge). Sever any + // still-pending edge here too: otherwise the recipient persists as an inert collaborator + // who springs back -- unverified -- if the creator regains access. In the revoked-link + // case the edge was already confirmed, so this is a no-op and the confirmed edge lingers + // inert (lazy model). + if (redeemedLinkId) { + sharing.revertShareKeyRedemption(profileId, redeemedLinkId); + } + throw createOpenGadgetError(OPEN_GADGET_ERROR_CODES.workspaceAccessDenied); + } + role = effectiveRole; // Fire-and-forget a call to the collaborator's user DO so the gadget appears on // (or is refreshed on) their home page. @@ -8343,15 +8618,26 @@ export class OverseerDurableObject extends DurableObject { ownerId = callerId; } - // Caller must be the owner or a build collaborator. + // Caller must be the owner or a build collaborator. The agent's reply can surface anything + // the workspace has already read (chat history, gadget storage), so a collaborator passes the + // same authorization gate as open() -- but non-interactively: with no way to configure + // accounts here, an unverified caller is sent to open the workspace, which is where + // verification happens. Requiring "build" up front means a "use" collaborator gets the plain + // denial below rather than being verified (or told to fix a verification failure) for access + // this path can never grant them. if (ownerId !== callerId) { - if (this.impl.storage.prohibitAllSharing.get()) { + let role: CollaboratorRole | null; + try { + role = await this.impl.authorizeCollaborator( + callerProfile.id, caller, {requireRole: "build"}); + } catch (err) { return { accepted: false, - message: "This workspace has sharing disabled, so only its owner can access it.", + message: "Your access to the data this workspace has read could not be verified. Open " + + "the workspace in your browser to verify your access, then try again. " + + `(${stringifyError(err)})`, }; } - let role = (await this.impl.getSharingManager()).getEffectiveRole(callerProfile.id); if (role !== "build") { return { accepted: false, @@ -9029,7 +9315,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, }; @@ -9048,7 +9334,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, }; @@ -9070,9 +9356,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); } }; @@ -9080,13 +9366,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); @@ -10262,8 +10548,18 @@ 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 (authorizeCollaborator at every non-owner + // entry point, and the coverage guard in authorizeObservation), not by blocking sharing + // wholesale. The one exception: once latched, if a connection that read the sensitive data has + // since been removed, verification has lost its anchor, so the grant-creating mutators + // (addCollaborator, createShareLink, newShareLinkKey -- and share-key redemption in open()) + // refuse -- see assertNewSharingAllowed. + // The keepUsers re-rooting inside removeCollaborator/revokeShareLink needs no such check: it + // re-grants existing collaborators at no more than their prior role, so there is no new party + // and no new verification obligation. async listObserverRequirements( role: CollaboratorRole): Promise { @@ -10284,13 +10580,12 @@ 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({ + let sharing = await this.impl.getSharingManager(); + // Asserted in the same synchronous block as the grant's storage write (after every await): a + // check ahead of the awaits above could pass, a concurrent producer-connection removal land + // during the yield, and the grant still be written past it. + this.impl.assertNewSharingAllowed(); + return sharing.addCollaborator({ caller: this.#sharingCaller(), profile, role, @@ -10343,25 +10638,20 @@ 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 }); + return (await this.impl.getSharingManager()).createShareLink({ + caller: this.#sharingCaller(), role, note, + assertGrantAllowed: () => this.impl.assertNewSharingAllowed(), + }); } 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 }); + return (await this.impl.getSharingManager()).newShareLinkKey({ + caller: this.#sharingCaller(), linkId, + // A fresh key is a new grant vector even though the link already exists: it is reachable + // here when an unverifiable producer was removed while links were outstanding (which the + // removal guard deliberately allows as a remedy). + assertGrantAllowed: () => this.impl.assertNewSharingAllowed(), + }); } async listShareLinks(): Promise { @@ -11017,6 +11307,43 @@ class GatekeeperClientImpl> async remove(): Promise { let record = this.impl.storage.gatekeepers.get(this.id); + // A connection that has read restricted data is the anchor observer verification runs + // against: while the workspace is shared, deleting its record would let a never-verified + // collaborator open unchecked even though the data persists in chat history and storage. + // Outstanding share links count as shared too: redemption is gated at open() only while the + // record exists, so a link redeemed after removal would grant the same unchecked access. + // The guard applies only to the connections that themselves read restricted data (the + // producers, derived from the action log): the latch is workspace-wide, but a non-producer + // connection anchors no restricted-data verification, so it stays removable while shared. + // Unverifiable records (legacy, or no vendor account behind them) anchor no verification -- + // removing one is itself a remedy -- so they stay removable too. + if (record && this.impl.storage.prohibitAllSharing.get()) { + let vendorId: string | null = null; + try { + vendorId = observerVendorId(record); + } catch { + // Legacy connection with no creationSpec: unverifiable, exempt from the guard. + } + if (vendorId !== null) { + // An empty producer set with the latch set should be impossible: the latch and the action + // record are written in one synchronous block, built-in observations never latch, and + // records that predate the flag's rename still read correctly (see + // observationContainsRestrictedData). If it ever happens anyway, fall back to guarding + // every connection rather than none. + let producers = this.impl.restrictedProducerIds(); + if (producers.size === 0 || producers.has(this.id)) { + let sharing = await this.impl.getSharingManager(); + if (sharing.listCollaborators().length > 0 || + sharing.listShareLinkRecords().length > 0) { + throw new Error( + "This connection cannot be removed: it has read sensitive data into this " + + "workspace, and the workspace is shared. Collaborators are verified against this " + + "connection before they may see that data, so remove all collaborators and revoke " + + "all share links first."); + } + } + } + } this.impl.removeGatekeeper(this.id); this.impl.recordGadgetAnalytics({ event_name: "connection_removed", diff --git a/packages/workshop-backend/src/sharing.ts b/packages/workshop-backend/src/sharing.ts index ff0cb9e18..fad4ed49e 100644 --- a/packages/workshop-backend/src/sharing.ts +++ b/packages/workshop-backend/src/sharing.ts @@ -14,19 +14,26 @@ // denied at open() time. Because the graph is never destructively pruned, revocation is reversible: // 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.) +// A third graph state exists alongside live and unreachable: a *pending* shareKey edge (see +// `redeemShareKey`) grants no authority to anyone until the redeeming open()'s verification +// confirms it. // -// 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"; import { Collection, NonUniqueIndex } from "@gadgets/typed-storage"; -// Roles are totally ordered: build > use. Higher rank means strictly more access. -function roleRank(role: CollaboratorRole): number { +/** + * Roles are totally ordered: build > use. Higher rank means strictly more access. Exported so + * role comparisons elsewhere (e.g. the Overseer's `requireRole` floor) rank rather than + * string-compare, which stays correct if a role is ever added between the two. + */ +export function roleRank(role: CollaboratorRole): number { return role === "build" ? 2 : 1; } @@ -156,26 +163,6 @@ export class SharingManager { */ constructor(private storage: SharingStorage, private ownerProfileId: string) {} - // --------------------------------------------------------------------------------------- - // 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. - * - * Because removed collaborators and revoked links linger in storage (the lazy revocation model; - * see the module header and removeCollaborator/revokeShareLink), this must reflect *current* - * reachability, not mere table membership: a collaborator with a live path from the owner, or - * an un-revoked share link whose keys anyone could still redeem. - */ - hasAnyShares(): boolean { - if (this.computeEffectiveRoles().size > 0) return true; - for (let link of this.#listLinks()) { - if (!link.revoked) return true; - } - return false; - } - // Every share link, revoked or not. Aliases are skipped. *#listLinks(): Generator { for (let record of this.storage.shareKeys.list()) { @@ -199,68 +186,181 @@ export class SharingManager { /** * The effective role of `profileId` -- the maximum role reachable from the owner through valid * permission edges -- or undefined if the user has no access. The owner always has "build". + * + * `assumePendingLink` optionally names a link whose pending edge on this profile is counted as + * though it were confirmed. It is used by the open() that just performed a redemption (see + * `redeemShareKey`) to compute the role it must verify the recipient for; the pending edge + * still grants nothing to anyone else. */ - getEffectiveRole(profileId: string): CollaboratorRole | undefined { + getEffectiveRole(profileId: string, assumePendingLink?: string): CollaboratorRole | undefined { if (profileId === this.ownerProfileId) return "build"; - return this.computeEffectiveRoles().get(profileId); + return this.computeEffectiveRoles({ + assumePendingLink: assumePendingLink !== undefined + ? { profileId, linkId: assumePendingLink } : null, + }).get(profileId); } /** * Redeem a raw share key on behalf of a user opening the gadget. If the key exists, ensures the - * user is a collaborator with a `shareKey` edge for its link (adding the edge if missing, or - * creating the collaborator record if they're new). Does nothing if the key is unknown. + * user is a collaborator with a *pending* `shareKey` edge for its link (adding the edge if + * missing, or creating the collaborator record if they're new). Does nothing if the key is + * unknown. + * + * A pending edge grants no authority to anyone: the recipient is invisible to + * listCollaborators and to everyone's effective roles until the redeeming open()'s observer + * verification settles the redemption -- success confirms the edge + * (`confirmShareKeyRedemption`), failure severs it (`revertShareKeyRedemption`). Only the + * verifying open itself counts the edge, via `getEffectiveRole(profileId, linkId)`. * * The raw key is hashed internally; the plaintext is never stored. `fetchProfile` is invoked * (an RPC, in production) only when a brand-new collaborator must be created, so existing - * collaborators are redeemed without any RPC. + * collaborators are redeemed without any RPC. Because that fetch yields, the collaborator + * record is re-read after it resolves and any record a concurrent redemption wrote meanwhile + * is merged into rather than overwritten. * * A key whose link is revoked behaves like an unknown key (it cannot be redeemed). + * + * `assertGrantAllowed` is the same optional policy check createShareLink/newShareLinkKey take: + * redeeming a key admits a new party, so once policy forbids new sharing the callback refuses + * redemption too -- both writing a fresh pending edge and settling a not-yet-confirmed one + * (settling completes a new grant). It runs synchronously with the return of the link id, after + * every await, and a throw persists nothing. It is NOT invoked for an already-*confirmed* edge + * (an existing grant; re-opening with a retained key stays a no-op) nor for unknown/revoked + * keys. The policy itself stays out of this module, per the module header. + * + * Returns the redeemed link's id when there is now a pending edge for the caller to settle -- + * whether this call added it, or a concurrent (or crashed) redemption of the same link left one + * mid-verification; each such open verifies and confirms/reverts independently. Returns null + * when the call changed nothing: an unknown or revoked key, or an already-*confirmed* edge for + * this link (redeeming a second key of the same link is a no-op). */ async redeemShareKey(opts: { rawKey: string; profileId: string; fetchProfile: () => Promise; - }): Promise { + assertGrantAllowed?: () => void; + }): Promise { let hash = await hashShareKey(opts.rawKey); let keyRecord = this.storage.shareKeys.get(hash); - if (!keyRecord) return; + if (!keyRecord) return null; // Edges point at the link, not the individual key, so a link's keys collapse to one grant. // A copy of a link is an alias; follow it to the link that owns the metadata. let link = keyRecord.alias === undefined ? keyRecord : asLink(this.storage.shareKeys.get(keyRecord.alias)); - if (!link || link.revoked) return; + if (!link || link.revoked) return null; let linkId = link.id; let role = link.role ?? "build"; let existing = this.storage.collaborators.get(opts.profileId); - if (existing) { - // User is already a collaborator. Only add an edge if they don't already have one for this - // link (redeeming a second key of the same link is a no-op). - let alreadyHasEdge = existing.addedBy.some( - e => e.type === "shareKey" && e.keyId === linkId); - if (!alreadyHasEdge) { - existing.addedBy.push({ - type: "shareKey", - keyId: linkId, - created: new Date(), - role, - }); - this.storage.collaborators.put(existing); - } - } else { - // New collaborator -- need full profile from their user DO. + if (!existing) { + // New collaborator -- need full profile from their user DO. The fetch yields, so a + // concurrent redemption may have created the record meanwhile: re-read and merge rather + // than blindly put. A stale put would overwrite an edge the concurrent open already wrote + // (possibly confirmed), and this open's later failed verification would then revert it -- + // erasing a grant whose recipient holds a live capability, exactly the ghost the pending + // model exists to prevent. let profile = await opts.fetchProfile(); - this.storage.collaborators.put({ - profile, - addedBy: [{ - type: "shareKey", - keyId: linkId, - created: new Date(), - role, - }], - }); + existing = this.storage.collaborators.get(opts.profileId) ?? { profile, addedBy: [] }; } + + // A confirmed edge for this link means a prior redemption fully succeeded, so there is + // nothing to settle (and no new grant, so no policy check). A still-pending one means another + // redemption of this link is mid-verification (or crashed): leave it untouched and let this + // attempt verify and confirm/revert it independently -- but settling it would complete a new + // grant, so it is policy-gated like writing one. + for (let edge of existing.addedBy) { + if (edge.type === "shareKey" && edge.keyId === linkId) { + if (!edge.pending) return null; + opts.assertGrantAllowed?.(); + return linkId; + } + } + opts.assertGrantAllowed?.(); + existing.addedBy.push({ + type: "shareKey", + keyId: linkId, + created: new Date(), + role, + pending: true, + }); + this.storage.collaborators.put(existing); + return linkId; + } + + /** + * Confirm the pending `shareKey` edge a just-completed `redeemShareKey` added: the redeeming + * open()'s observer verification succeeded, so the grant becomes real and the recipient now + * counts in everyone's effective roles. Idempotent -- confirming an already-confirmed edge + * changes nothing. If a concurrent revert (a parallel open of the same link whose verification + * failed) severed the edge mid-flight, it is re-added confirmed at the link's current role. + * + * The missing-edge re-add also fires when the *owner's* removeCollaborator raced this + * verification: pending edges are invisible to the affected-set computation, so the removal + * severed the edge without a revocation restart, and this confirm quietly re-grants. Accepted + * wart: the still-live link is re-redeemable anyway, so the removal never durably excluded this + * recipient; a revoked link's re-added edge is inert (below); and a removal that visibly + * affects anyone restarts the DO within ~100ms, killing a parked open before it confirms. + * Note the race is narrower than it looks: a pending-only recipient is invisible to + * listCollaborators too, so the removal UI cannot even target one mid-verification -- a racing + * removal was necessarily aimed at an edge some earlier open had already confirmed. And the + * re-add grants no incremental authority: the recipient holds the live link and can re-redeem + * it manually whether or not this confirm lands. The durable exclusion is revoking the link + * (computeEffectiveRoles skips revoked links, which inerts every edge referencing one). + * + * Confirming cannot resurrect revoked authority: effective-role resolution already excludes + * revoked links, so an edge confirmed after its link was revoked grants nothing (it lingers + * inert, like any edge of a revoked link under the lazy model). + * + * `assertGrantAllowed` is the same optional policy check redeemShareKey takes, re-asserted + * here because redemption is two-phase: the gate at redeemShareKey ran synchronously with the + * *pending* write, but this confirm is the *granting* write, separated from it by the + * redeeming open()'s await windows (observer verification, capsule reconciliation). A policy + * change landing in that window -- a restricted-data producer removed, closing the workspace + * to new sharing -- must refuse the grant, so the check runs again in this synchronous block, + * before the pending flag is cleared or the missing edge re-added. A throw persists nothing. + * It is NOT invoked for an already-confirmed edge (an existing grant, not a new one -- + * matching redeemShareKey). + */ + confirmShareKeyRedemption( + profileId: string, linkId: string, assertGrantAllowed?: () => void): void { + let record = this.storage.collaborators.get(profileId); + if (!record) return; + for (let edge of record.addedBy) { + if (edge.type === "shareKey" && edge.keyId === linkId) { + if (edge.pending) { + assertGrantAllowed?.(); + delete edge.pending; + this.storage.collaborators.put(record); + } + return; + } + } + assertGrantAllowed?.(); + record.addedBy.push({ + type: "shareKey", + keyId: linkId, + created: new Date(), + role: asLink(this.storage.shareKeys.get(linkId))?.role ?? "build", + }); + this.storage.collaborators.put(record); + } + + /** + * Sever the still-pending `shareKey` edge a just-completed `redeemShareKey` added, restoring + * the graph to its pre-redemption state. Used when the redeeming user is refused *after* + * redemption, so a refused recipient never persists in the sharing graph. They can redeem the + * same key again once whatever refused them is fixed. An edge a concurrent open of the same + * link already *confirmed* is left alone -- that open verified this user, and its grant must + * survive this attempt's failure. Lazy like removeCollaborator: the collaborator record itself + * is retained, even if now edge-less. + */ + revertShareKeyRedemption(profileId: string, linkId: string): void { + let record = this.storage.collaborators.get(profileId); + if (!record) return; + record.addedBy = record.addedBy.filter( + e => !(e.type === "shareKey" && e.keyId === linkId && e.pending)); + this.storage.collaborators.put(record); } // --------------------------------------------------------------------------------------- @@ -288,8 +388,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; @@ -432,7 +532,18 @@ export class SharingManager { } async createShareLink( - opts: { caller: SharingCaller; role: CollaboratorRole; note?: string }) + opts: { + caller: SharingCaller; + role: CollaboratorRole; + note?: string; + /** + * Optional policy check invoked synchronously with the grant's storage write, after + * every await, so a policy change cannot slip between check and grant. A throw aborts + * the call with nothing persisted (the minted key is discarded, never stored). The + * policy itself stays out of this module, per the module header. + */ + assertGrantAllowed?: () => void; + }) : Promise<{ key: string; linkId: string }> { let callerRole = this.#requireCallerRole(opts.caller); if (roleRank(opts.role) > roleRank(callerRole)) { @@ -441,6 +552,7 @@ export class SharingManager { // The link is stored as its first key: the record is keyed by that key's hash. let { key, hash } = await this.#mintKey(); + opts.assertGrantAllowed?.(); this.storage.shareKeys.put({ id: hash, note: opts.note, @@ -452,7 +564,12 @@ export class SharingManager { } /** Mints another key for an existing link. */ - async newShareLinkKey(opts: { caller: SharingCaller; linkId: string }): Promise<{ key: string }> { + async newShareLinkKey(opts: { + caller: SharingCaller; + linkId: string; + /** See createShareLink: run synchronously with the put, a throw persists nothing. */ + assertGrantAllowed?: () => void; + }): Promise<{ key: string }> { let link = this.#requireLink(opts.linkId); if (link.revoked) { throw new Error("Share link not found."); @@ -467,6 +584,7 @@ export class SharingManager { } let { key, hash } = await this.#mintKey(); + opts.assertGrantAllowed?.(); this.storage.shareKeys.put({ id: hash, alias: link.id }); return { key }; } @@ -563,15 +681,21 @@ export class SharingManager { * - `removedUser`: a profileId treated as removed (excluded from the graph entirely). * - `removedEdge`: a single user edge (target ← sharer) treated as removed. * - `revokedLinkId`: a link treated as revoked (its edges contribute nothing). + * - `assumePendingLink`: the named profile's own pending edge for the named link is counted + * as though it were confirmed. Unlike the removal-shaped options above, this models a + * hypothetical *grant*: the open() that just redeemed the link counting its own edge to + * compute the role it must verify for. Every other pending edge still contributes nothing. */ computeEffectiveRoles(opts: { removedUser?: string | null; removedEdge?: { target: string; sharer: string } | null; revokedLinkId?: string | null; + assumePendingLink?: { profileId: string; linkId: string } | null; } = {}): Map { let removedUser = opts.removedUser ?? null; let removedEdge = opts.removedEdge ?? null; let revokedLinkId = opts.revokedLinkId ?? null; + let assumePendingLink = opts.assumePendingLink ?? null; // Map linkId → {creator, role}, excluding revoked links (the persisted `revoked` flag, and the // hypothetical `revokedLinkId` used by preview). @@ -608,6 +732,13 @@ export class SharingManager { for (let edge of record.addedBy) { let granted: CollaboratorRole | undefined; if (edge.type === "shareKey") { + // A pending edge grants nothing to anyone, except to the one open() currently + // verifying its redemption (modeled by assumePendingLink). + if (edge.pending && + !(assumePendingLink && id === assumePendingLink.profileId && + edge.keyId === assumePendingLink.linkId)) { + continue; + } let info = linkInfo.get(edge.keyId); if (!info) continue; // link revoked or no longer exists let creatorRole = sharerRole(info.creator); diff --git a/packages/workshop-shared/src/api.ts b/packages/workshop-shared/src/api.ts index c7d34b9e0..b6689b2be 100644 --- a/packages/workshop-shared/src/api.ts +++ b/packages/workshop-shared/src/api.ts @@ -1254,10 +1254,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, @@ -3966,6 +3968,16 @@ export type PermissionEdge = { * resolves to this id, so redeeming any of them yields this one edge. */ keyId: string; + + /** + * Present while the redeeming open()'s observer verification has not yet succeeded. A pending + * edge grants no authority to anyone -- only the open() that performed the redemption counts + * it, to compute the role it is verifying for. Verification success confirms the edge (clears + * the flag); failure severs it. A stale pending edge left behind by a crashed open() is inert + * and self-heals: the next redemption of the same link re-verifies and settles it. Edges + * written before this field existed lack it and read as confirmed. + */ + pending?: true; }); /** Information about a single collaborator, returned by list/add operations. */ diff --git a/packages/workshop-shared/src/gatekeeper.ts b/packages/workshop-shared/src/gatekeeper.ts index 48bcdd729..6190c75fe 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 From b1d7268202f01154bcb5d919700dc14c3b7f3c7f Mon Sep 17 00:00:00 2001 From: Maximo Guk <62088388+Maximo-Guk@users.noreply.github.com> Date: Thu, 20 Aug 2026 14:32:55 -0500 Subject: [PATCH 2/4] Add integration coverage for restricted-data sharing and observer role scope New sensitive-observations and observer-role-scope suites, with the harness, RPC client, and gatekeeper-test fixture support they need (the fixture gains a configurable observer hook and sensitive reads). The fixture's verify outcome can also target a single bound resource (a resource-specific key with a label-wide fallback), which the coverage-scrub test uses to prove the scrub is per-producer: after a failed re-verification, exactly the refused producer's restricted reads are blocked -- the sibling producer's keep flowing -- until a repaired re-open re-persists coverage. Co-Authored-By: Claude Fable 5 --- .../__tests__/observer-role-scope.test.ts | 191 +++++ .../__tests__/sensitive-observations.test.ts | 738 ++++++++++++++++++ .../fixtures/gatekeeper-test/src/env.d.ts | 11 + .../gatekeeper-test/src/test-gatekeeper.ts | 132 +++- .../fixtures/gatekeeper-test/wrangler.jsonc | 27 +- packages/integration-tests/src/harness.ts | 6 + packages/integration-tests/src/rpc-client.ts | 14 +- 7 files changed, 1101 insertions(+), 18 deletions(-) create mode 100644 packages/integration-tests/__tests__/observer-role-scope.test.ts create mode 100644 packages/integration-tests/__tests__/sensitive-observations.test.ts diff --git a/packages/integration-tests/__tests__/observer-role-scope.test.ts b/packages/integration-tests/__tests__/observer-role-scope.test.ts new file mode 100644 index 000000000..11652de62 --- /dev/null +++ b/packages/integration-tests/__tests__/observer-role-scope.test.ts @@ -0,0 +1,191 @@ +// Tests for role-scoped observer enforcement: both the sensitive-observation coverage guard and +// the external-message authorization gate hold a collaborator only to what their role's +// verification scope can actually cover ("use" collaborators are verified only against +// gadget-bound connections; see #inScopeGatekeepers in overseer.ts). +// +// These live in their own file -- with their own harness, like every suite here -- rather than in +// sensitive-observations.test.ts, because that suite includes a revocation-restart test whose DO +// abort makes the shared local harness briefly drop unrelated in-flight requests; its concurrent +// tests pass with their current timing, but growing that file re-rolls those dice. + +import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import type { RpcStub } from "capnweb"; +import type { AuthenticatedApi, Overseer, PublicApi } from "@gadgets/workshop-shared/api"; +import type { + SubmitExternalMessageResult, +} from "@gadgets/workshop-shared/external-message-gateway"; +import { + startTestGatekeeperHarness, TEST_GATEKEEPER_WORKER, TEST_VENDOR_ID, type Harness, +} from "../src/harness.js"; +import { + 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; + }); +} + +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. +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 }; +} + +/** + * Submit an external chat message as `callerEmail`, through the fixture worker's control surface + * (and so through the Workshop's real ExternalMessageGateway entrypoint). + */ +async function submitExternalMessage(input: { + callerEmail: string; gadgetKey: string; prompt: string; +}): Promise { + const res = await harness.fetchWorker( + TEST_GATEKEEPER_WORKER, "http://gatekeeper-test.test/control/submit-external-message", + { method: "POST", body: JSON.stringify({ + chatKey: `chat-${input.gadgetKey}`, messageKey: crypto.randomUUID(), + gadgetTitle: input.gadgetKey, ...input }) }); + if (res.status !== 200) { + throw new Error(`submit-external-message failed with ${res.status}: ${await res.text()}`); + } + return await res.json() as SubmitExternalMessageResult; +} + +/** The workspace id behind an external gadgetKey -- the DO id the gateway derives from it. */ +async function externalGadgetId(gadgetKey: string): Promise { + const res = await harness.fetchWorker( + TEST_GATEKEEPER_WORKER, "http://gatekeeper-test.test/control/external-gadget-id", + { method: "POST", body: JSON.stringify({ gadgetKey }) }); + if (res.status !== 200) { + throw new Error(`external-gadget-id failed with ${res.status}: ${await res.text()}`); + } + return (await res.json() as { gadgetId: string }).gadgetId; +} + +describe("role-scoped observer enforcement", () => { + it.concurrent("a use collaborator only blocks reads from connections in their scope", + async () => { + await withSession(async publicApi => { + const ws = await newWorkspace(publicApi, "use-scope"); + const [carol] = nextUsernames("carol"); + const carolApi = await signUp(publicApi, carol); + const carolAccount = await provisionAccount(carolApi); + const collaborator = await ws.overseer.addCollaborator(carol, "use"); + if (!collaborator) throw new Error(`Failed to share the gadget with ${carol}`); + + // No gadget binds the connection, so Carol's "use" verification scope is empty: her open + // must not prompt (the recorder has no queued responses, so an unexpected prompt throws). + const emptyCallback = stubFor(new ObserverConfigRecorder()); + try { + (await carolApi.openGadget(ws.gadgetId, undefined, emptyCallback))[Symbol.dispose](); + } finally { + emptyCallback[Symbol.dispose](); + } + + // ensureObserver can never verify Carol against an unbound connection, so she must not + // block its sensitive reads either -- demanding coverage her role can't gain would block + // them forever. + await expect(ws.session.readThing(true)).resolves.toContain("use-scope"); + + // Binding the connection to a gadget (pure storage writes; no gadget code runs) brings it + // into "use" scope. Carol is now in scope but uncovered, so the read blocks... + using gadget = await ws.overseer.createGadget("Test Gadget", undefined, "TEST_GADGET"); + await gadget.bind("TEST_THING", ws.gatekeeperId); + await expect(ws.session.readThing(true)).rejects.toThrow(/has not been verified/i); + + // ...and the error's remedy works: re-opening verifies her, which unblocks the read. + const callback = stubFor( + new ObserverConfigRecorder().alwaysChoose(carolAccount.id, MAX_OBSERVER_PROMPTS)); + try { + (await carolApi.openGadget(ws.gadgetId, undefined, callback))[Symbol.dispose](); + } finally { + callback[Symbol.dispose](); + } + await expect(ws.session.readThing(true)).resolves.toContain("use-scope"); + }); + }); + + it.concurrent("the external-message path denies a use collaborator by role, not verification", + async () => { + await withSession(async publicApi => { + const [alice, dave] = nextUsernames("alice", "dave"); + const aliceApi = await signUp(publicApi, alice); + const aliceAccount = await provisionAccount(aliceApi); + const gadgetKey = `external-use-${crypto.randomUUID()}`; + + // Alice creates the workspace through the external channel (the AI-model rejection means + // her submission passed the gate), then binds its connection to a gadget so it falls in + // "use" verification scope. + await expect(submitExternalMessage({ callerEmail: alice, gadgetKey, prompt: "hello" })) + .resolves.toMatchObject({ + accepted: false, message: expect.stringMatching(/AI model/i) }); + const gadgetId = await externalGadgetId(gadgetKey); + using overseer = await aliceApi.openGadget(gadgetId); + const gatekeeper = await overseer.newGatekeeper(aliceAccount.id, thingUrl("external-use")); + if (!gatekeeper) throw new Error("Failed to create the test connection"); + using gadget = await overseer.createGadget("Test Gadget", undefined, "TEST_GADGET"); + await gadget.bind("TEST_THING", await gatekeeper.getId()); + + // Dave is in verification scope and unverified, but this path can never grant a "use" + // collaborator agent access, so his role is checked before verification runs: he gets the + // plain denial, not a verification failure he has no reason to go fix. + await signUp(publicApi, dave); + await overseer.addCollaborator(dave, "use"); + await expect(submitExternalMessage({ callerEmail: dave, gadgetKey, prompt: "hi" })) + .resolves.toMatchObject({ + accepted: false, message: expect.stringMatching(/do not have access/i) }); + }); + }); +}); 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 000000000..15c76c76e --- /dev/null +++ b/packages/integration-tests/__tests__/sensitive-observations.test.ts @@ -0,0 +1,738 @@ +// Tests for the sensitive-data (`containsRestrictedData`) observation policy. +// +// 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). +// +// 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, ObserverAccountChoice, Overseer, PublicApi, +} from "@gadgets/workshop-shared/api"; +import type { + SubmitExternalMessageResult, +} from "@gadgets/workshop-shared/external-message-gateway"; +import { + startTestGatekeeperHarness, TEST_GATEKEEPER_WORKER, TEST_VENDOR_ID, type Harness, +} from "../src/harness.js"; +import { + accountLabel, connect, listConnectedAccounts, logIn, 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 -- everywhere, or (with + * `resourceUrl`) at one bound resource only, which wins over the account-wide outcome. + */ +async function setVerifyOutcome( + label: string, outcome: { allow: true } | { allow: false; reason: string }, + resourceUrl?: string): Promise { + const res = await harness.fetchWorker( + TEST_GATEKEEPER_WORKER, "http://gatekeeper-test.test/control/verify-outcome", + { method: "POST", body: JSON.stringify({ label, resourceUrl, ...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; + alice: string; + 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, alice, 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<{ + bob: string; + bobProfileId: string; + 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 { + bob, bobProfileId: collaborator.profile.id, 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(gadgetId: string, bobApi: RpcStub, + bobAccount: ConnectedAccount): Promise> { + const recorder = new ObserverConfigRecorder().alwaysChoose(bobAccount.id, MAX_OBSERVER_PROMPTS); + const callback = stubFor(recorder); + try { + return await bobApi.openGadget(gadgetId, undefined, callback); + } finally { + callback[Symbol.dispose](); + } +} + +/** + * Submit an external chat message as `callerEmail`, through the fixture worker's control surface + * (and so through the Workshop's real ExternalMessageGateway entrypoint). + */ +async function submitExternalMessage(input: { + callerEmail: string; gadgetKey: string; prompt: string; +}): Promise { + const res = await harness.fetchWorker( + TEST_GATEKEEPER_WORKER, "http://gatekeeper-test.test/control/submit-external-message", + { method: "POST", body: JSON.stringify({ + chatKey: `chat-${input.gadgetKey}`, messageKey: crypto.randomUUID(), + gadgetTitle: input.gadgetKey, ...input }) }); + if (res.status !== 200) { + throw new Error(`submit-external-message failed with ${res.status}: ${await res.text()}`); + } + return await res.json() as SubmitExternalMessageResult; +} + +/** The workspace id behind an external gadgetKey -- the DO id the gateway derives from it. */ +async function externalGadgetId(gadgetKey: string): Promise { + const res = await harness.fetchWorker( + TEST_GATEKEEPER_WORKER, "http://gatekeeper-test.test/control/external-gadget-id", + { method: "POST", body: JSON.stringify({ gadgetKey }) }); + if (res.status !== 200) { + throw new Error(`external-gadget-id failed with ${res.status}: ${await res.text()}`); + } + return (await res.json() as { gadgetId: string }).gadgetId; +} + +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"); + + // Sharing stays available after the latch, across every sharing RPC. + 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.gadgetId, 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.gadgetId, 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"); + + // Bob's open runs observer verification, which the fixture admits by default, so the latch + // does not shut him out. + const bob = await addBob(publicApi, ws); + using bobOverseer = await bobOpens(ws.gadgetId, 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(), so + // the user sees the gatekeeper's own message. + const error = await bobOpens(ws.gadgetId, 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); + }); + }); + + it.concurrent("a failed re-verification scrubs coverage for just the failed producer", + async () => { + await withSession(async publicApi => { + const ws = await newWorkspace(publicApi, "scrub"); + // A second producer, so the test can prove the scrub is scoped to the one that refused. + const accounts = await listConnectedAccounts(ws.aliceApi); + const account = accounts.find(a => a.vendorId === TEST_VENDOR_ID)!; + const second = await ws.overseer.newGatekeeper(account.id, thingUrl("scrub-2")); + if (!second) throw new Error("Failed to create the second test connection"); + const secondSession: any = await second.openSession(); + + // Bob verifies against both producers, so both restricted reads are admitted. + const bob = await addBob(publicApi, ws); + (await bobOpens(ws.gadgetId, bob.bobApi, bob.bobAccount))[Symbol.dispose](); + await expect(ws.session.readThing(true)).resolves.toContain("scrub"); + await expect(secondSession.readThing(true)).resolves.toContain("scrub-2"); + + // Bob's access to the first producer's resource is revoked; his next open is denied... + await setVerifyOutcome( + bob.bobLabel, { allow: false, reason: "Access revoked." }, thingUrl("scrub")); + await expect(bobOpens(ws.gadgetId, bob.bobApi, bob.bobAccount)) + .rejects.toThrow(/could not confirm/i); + + // ...and the failure scrubbed his persisted coverage for that producer, so its restricted + // reads fail closed against his older live session rather than keep flowing to it... + await expect(ws.session.readThing(true)).rejects.toThrow(/has not been verified/i); + // ...while the producer he still passes stays covered (the scrub is scoped). + await expect(secondSession.readThing(true)).resolves.toContain("scrub-2"); + + // A repaired re-open re-verifies him and re-persists full coverage. + await setVerifyOutcome(bob.bobLabel, { allow: true }, thingUrl("scrub")); + (await bobOpens(ws.gadgetId, bob.bobApi, bob.bobAccount))[Symbol.dispose](); + await expect(ws.session.readThing(true)).resolves.toContain("scrub"); + }); + }); + + it.concurrent("a refused share-link recipient is rolled back and does not block later reads", + async () => { + await withSession(async publicApi => { + const ws = await newWorkspace(publicApi, "refused-link"); + await expect(ws.session.readThing(true)).resolves.toContain("refused-link"); + + const { key } = await ws.overseer.createShareLink("build", "refused recipient"); + + const [dave] = nextUsernames("dave"); + const daveApi = await signUp(publicApi, dave); + const daveAccount = await provisionAccount(daveApi); + await setVerifyOutcome( + accountLabel(daveAccount), { allow: false, reason: "You do not have access." }); + + // Dave's open redeems the key, but observer verification then refuses him, which must roll + // the redemption back rather than leave him persisted as a collaborator. + const recorder = + new ObserverConfigRecorder().alwaysChoose(daveAccount.id, MAX_OBSERVER_PROMPTS); + const callback = stubFor(recorder); + try { + await expect(daveApi.openGadget(ws.gadgetId, key, callback)) + .rejects.toThrow(/could not confirm/i); + } finally { + callback[Symbol.dispose](); + } + + // He is not a collaborator, so he does not count against sensitive-observation coverage. + await expect(ws.overseer.listCollaborators()).resolves.toEqual([]); + await expect(ws.session.readThing(true)).resolves.toContain("refused-link"); + }); + }); + + it.concurrent("a pending share-key redemption grants no access to a concurrent open", + async () => { + await withSession(async publicApi => { + const ws = await newWorkspace(publicApi, "pending"); + await expect(ws.session.readThing(true)).resolves.toContain("pending"); + + const { key } = await ws.overseer.createShareLink("build", "pending recipient"); + + const [dave] = nextUsernames("dave"); + const daveApi = await signUp(publicApi, dave); + await provisionAccount(daveApi); + + // Gate Dave's observer-config prompt on a test-controlled deferred, holding his keyed open + // mid-verification -- the redemption exists but is still pending. + let refuseConfig!: (err: Error) => void; + const gate = new Promise((_, reject) => { refuseConfig = reject; }); + const recorder = new ObserverConfigRecorder().respondWith(() => gate); + const callback = stubFor(recorder); + try { + // Observe the eventual rejection immediately, so it can't surface as an unhandled + // rejection while the mid-verification assertions below run. + const gatedOpen = daveApi.openGadget(ws.gadgetId, key, callback).then( + overseer => { overseer[Symbol.dispose](); return null; }, + (err: unknown) => err as Error); + + await waitFor("Dave's open to reach the configuration prompt", + async () => recorder.callCount > 0 ? true : null); + + // While the redemption is pending it grants nothing: a concurrent keyless open is denied + // by role (not sent through verification), and Dave does not count against + // sensitive-observation coverage. + await expect(daveApi.openGadget(ws.gadgetId)).rejects.toThrow(/don't have access/i); + await expect(ws.session.readThing(true)).resolves.toContain("pending"); + + refuseConfig(new Error("test declined the configuration prompt")); + expect(await gatedOpen).not.toBeNull(); + } finally { + callback[Symbol.dispose](); + } + + // The refused redemption was severed: no collaborator, and reads stay unaffected. + await expect(ws.overseer.listCollaborators()).resolves.toEqual([]); + await expect(ws.session.readThing(true)).resolves.toContain("pending"); + }); + }); + + it.concurrent("concurrent redemptions of the same key both verify", async () => { + await withSession(async publicApi => { + const ws = await newWorkspace(publicApi, "raced"); + const { key } = await ws.overseer.createShareLink("build", "raced"); + + const [dave] = nextUsernames("dave"); + const daveApi = await signUp(publicApi, dave); + const daveAccount = await provisionAccount(daveApi); + + const callbacks = [0, 1].map(() => stubFor( + new ObserverConfigRecorder().alwaysChoose(daveAccount.id, MAX_OBSERVER_PROMPTS))); + try { + // Each open redeems the same key; the second finds the first's pending edge and settles + // it independently, so neither is turned away and the grants collapse to one edge. + const overseers = await Promise.all( + callbacks.map(cb => daveApi.openGadget(ws.gadgetId, key, cb))); + for (const overseer of overseers) overseer[Symbol.dispose](); + } finally { + for (const cb of callbacks) cb[Symbol.dispose](); + } + + const collaborators = await ws.overseer.listCollaborators(); + expect(collaborators).toHaveLength(1); + expect(collaborators[0].addedBy).toHaveLength(1); + expect(collaborators[0].addedBy[0]).not.toHaveProperty("pending"); + }); + }); + + it.concurrent("a role granted mid-verification is not returned by the redeeming open", + async () => { + await withSession(async publicApi => { + const ws = await newWorkspace(publicApi, "mid-grant"); + // Bind the connection to a gadget so it falls in "use" verification scope: Dave's keyed + // open then prompts for an account choice, which is where the test parks it. + using gadget = await ws.overseer.createGadget("Test Gadget", undefined, "TEST_GADGET"); + await gadget.bind("TEST_THING", ws.gatekeeperId); + + const { key } = await ws.overseer.createShareLink("use", "mid-grant"); + + const [dave] = nextUsernames("dave"); + const daveApi = await signUp(publicApi, dave); + const daveAccount = await provisionAccount(daveApi); + + // Park Dave's keyed open at the configuration prompt; while it waits, Alice grants him + // "build" directly. His verification covered only the "use" scope, so the open must hand + // back a "use" capability -- the raise takes effect at his next open. + let releaseConfig!: () => void; + const gate = new Promise(resolve => { releaseConfig = resolve; }); + const recorder = new ObserverConfigRecorder() + .respondWith(async needs => { + await gate; + return needs.map(n => ({ gatekeeperId: n.gatekeeperId, accountId: daveAccount.id })); + }) + .alwaysChoose(daveAccount.id, MAX_OBSERVER_PROMPTS - 1); + const callback = stubFor(recorder); + try { + const gatedOpen = daveApi.openGadget(ws.gadgetId, key, callback); + await waitFor("Dave's open to reach the configuration prompt", + async () => recorder.callCount > 0 ? true : null); + + await ws.overseer.addCollaborator(dave, "build"); + + releaseConfig(); + using daveOverseer = await gatedOpen; + await expect(daveOverseer.getMetadata()).resolves.toMatchObject({ role: "use" }); + } finally { + callback[Symbol.dispose](); + } + + // A fresh open verifies at the raised role and yields it. + using secondOpen = await bobOpens(ws.gadgetId, daveApi, daveAccount); + await expect(secondOpen.getMetadata()).resolves.toMatchObject({ role: "build" }); + }); + }); + + it.concurrent("a connection added mid-verification denies the redeeming open", async () => { + await withSession(async publicApi => { + const ws = await newWorkspace(publicApi, "mid-topology"); + await expect(ws.session.readThing(true)).resolves.toContain("mid-topology"); + + const { key } = await ws.overseer.createShareLink("build", "mid-topology"); + + const [dave] = nextUsernames("dave"); + const daveApi = await signUp(publicApi, dave); + const daveAccount = await provisionAccount(daveApi); + + // Park Dave's keyed open at the configuration prompt; while it waits, Alice adds a second + // connection. Dave's verification snapshot never covered it, so confirming him would admit + // a collaborator the new producer never verified -- the open must be denied instead. + let releaseConfig!: () => void; + const gate = new Promise(resolve => { releaseConfig = resolve; }); + const recorder = new ObserverConfigRecorder() + .respondWith(async needs => { + await gate; + return needs.map(n => ({ gatekeeperId: n.gatekeeperId, accountId: daveAccount.id })); + }) + .alwaysChoose(daveAccount.id, MAX_OBSERVER_PROMPTS - 1); + const callback = stubFor(recorder); + let added; + try { + const gatedOpen = daveApi.openGadget(ws.gadgetId, key, callback).then( + overseer => { overseer[Symbol.dispose](); return null; }, + (err: unknown) => err as Error); + + await waitFor("Dave's open to reach the configuration prompt", + async () => recorder.callCount > 0 ? true : null); + + const accounts = await listConnectedAccounts(ws.aliceApi); + const account = accounts.find(a => a.vendorId === TEST_VENDOR_ID)!; + added = await ws.overseer.newGatekeeper(account.id, thingUrl("mid-topology-late")); + if (!added) throw new Error("Failed to create the second test connection"); + + // Mid-window, Dave's redemption is still pending and grants nothing, so it does not + // block the first connection's sensitive reads (unchanged behavior). + await expect(ws.session.readThing(true)).resolves.toContain("mid-topology"); + + releaseConfig(); + const error = await gatedOpen; + expect(error).not.toBeNull(); + expect(error!.message).toMatch(/changed in this workspace while your access was being verified/i); + } finally { + callback[Symbol.dispose](); + } + + // The denied redemption was reverted: no collaborator persists. + await expect(ws.overseer.listCollaborators()).resolves.toEqual([]); + + // A fresh keyed open re-redeems and verifies against the full new topology, confirming + // Dave... + const retryCallback = stubFor( + new ObserverConfigRecorder().alwaysChoose(daveAccount.id, MAX_OBSERVER_PROMPTS)); + try { + (await daveApi.openGadget(ws.gadgetId, key, retryCallback))[Symbol.dispose](); + } finally { + retryCallback[Symbol.dispose](); + } + const collaborators = await ws.overseer.listCollaborators(); + expect(collaborators).toHaveLength(1); + expect(collaborators[0].addedBy[0]).not.toHaveProperty("pending"); + + // ...including against the added connection: its sensitive reads see him covered. + const lateSession: any = await added.openSession(); + await expect(lateSession.readThing(true)).resolves.toContain("mid-topology-late"); + }); + }); + + it.concurrent("a latched connection cannot be removed while the workspace is shared", + async () => { + await withSession(async publicApi => { + // Latched but unshared: removal proceeds. (The latch itself persists; there is nobody + // whose verification the record anchors.) + const solo = await newWorkspace(publicApi, "remove-solo"); + await expect(solo.session.readThing(true)).resolves.toContain("remove-solo"); + const soloGatekeeper = await solo.overseer.getGatekeeperById(solo.gatekeeperId); + await expect(soloGatekeeper.remove()).resolves.toBeUndefined(); + + // Latched and shared: the record is what Bob's verification runs against, so removing it + // would let him open unchecked while the restricted data persists. + const ws = await newWorkspace(publicApi, "remove-shared"); + await expect(ws.session.readThing(true)).resolves.toContain("remove-shared"); + await addBob(publicApi, ws); + const gatekeeper = await ws.overseer.getGatekeeperById(ws.gatekeeperId); + await expect(gatekeeper.remove()).rejects.toThrow(/remove all collaborators/i); + // The refused removal left the connection intact. + await expect(ws.session.readThing()).resolves.toContain("remove-shared"); + }); + }); + + it.concurrent("a latched connection cannot be removed while a share link is outstanding", + async () => { + await withSession(async publicApi => { + // An unredeemed link creates no collaborator state, but its keys are multi-redeemable and + // never expire: redemption is gated at open() only while the gatekeeper record exists, so + // removing the record now would let a later recipient open unchecked. + const ws = await newWorkspace(publicApi, "remove-linked"); + await expect(ws.session.readThing(true)).resolves.toContain("remove-linked"); + const { linkId } = await ws.overseer.createShareLink("build", "outstanding"); + + const gatekeeper = await ws.overseer.getGatekeeperById(ws.gatekeeperId); + await expect(gatekeeper.remove()).rejects.toThrow(/revoke all share links/i); + // The refused removal left the connection intact. + await expect(ws.session.readThing()).resolves.toContain("remove-linked"); + + // Nobody redeemed the link, so revoking it affects no collaborator (no revocation restart) + // and unblocks the removal. + await expect(ws.overseer.revokeShareLink(linkId, [])).resolves.toEqual([]); + await expect(gatekeeper.remove()).resolves.toBeUndefined(); + }); + }); + + it.concurrent("the removal guard is scoped to the connection that read the sensitive data", + async () => { + await withSession(async publicApi => { + const ws = await newWorkspace(publicApi, "scoped-producer"); + // A second connection that never reads anything sensitive. + const accounts = await listConnectedAccounts(ws.aliceApi); + const account = accounts.find(a => a.vendorId === TEST_VENDOR_ID)!; + const bystander = await ws.overseer.newGatekeeper(account.id, thingUrl("scoped-bystander")); + if (!bystander) throw new Error("Failed to create the second test connection"); + + // Only the first connection reads restricted data; share after the latch. + await expect(ws.session.readThing(true)).resolves.toContain("scoped-producer"); + await addBob(publicApi, ws); + + // The latch is workspace-wide, but only the producer anchors verification: the bystander + // stays removable while shared, the producer does not. + await expect(bystander.remove()).resolves.toBeUndefined(); + const producer = await ws.overseer.getGatekeeperById(ws.gatekeeperId); + await expect(producer.remove()).rejects.toThrow(/remove all collaborators/i); + await expect(ws.session.readThing()).resolves.toContain("scoped-producer"); + }); + }); + + it.concurrent("a workspace whose sensitive-data producer was removed can no longer be shared", + async () => { + await withSession(async publicApi => { + const ws = await newWorkspace(publicApi, "unshareable"); + await expect(ws.session.readThing(true)).resolves.toContain("unshareable"); + + // Unshared, so removal is allowed -- but the restricted data (and the latch) outlive it. + const gatekeeper = await ws.overseer.getGatekeeperById(ws.gatekeeperId); + await expect(gatekeeper.remove()).resolves.toBeUndefined(); + + // With the producer's record gone there is nothing to verify a new collaborator against, + // so the grant-creating mutators refuse. + const [carol] = nextUsernames("carol"); + await signUp(publicApi, carol); + await expect(ws.overseer.addCollaborator(carol, "build")) + .rejects.toThrow(/can no longer be shared/i); + await expect(ws.overseer.createShareLink("use", "too late")) + .rejects.toThrow(/can no longer be shared/i); + }); + }); + + it.concurrent("removal unblocks the observation and tears down the observer record", + async () => { + await withSession(async publicApi => { + const ws = await newWorkspace(publicApi, "removal"); + const bob = await addBob(publicApi, ws); + (await bobOpens(ws.gadgetId, bob.bobApi, bob.bobAccount))[Symbol.dispose](); + await expect(ws.session.readThing(true)).resolves.toContain("removal"); + + // Removing Bob triggers the revocation restart: the DO aborts shortly after this call + // returns, killing every stub from this connection. Everything past this point runs on a + // fresh connection, retried across the abort window. + await ws.overseer.removeCollaborator(bob.bobProfileId, []); + + const reopened = await waitFor("the workspace to come back after the revocation restart", + async () => { + const publicApi2 = connect(harness.url); + try { + const aliceApi = await logIn(publicApi2, ws.alice); + const overseer = await aliceApi.openGadget(ws.gadgetId); + const gatekeeper = await overseer.getGatekeeperById(ws.gatekeeperId); + const session: any = await gatekeeper.openSession(); + // Probe with a benign read, so a session felled by the abort retries here rather than + // failing an assertion below. + await session.readThing(); + return { publicApi2, overseer, session }; + } catch { + publicApi2[Symbol.dispose](); + return null; + } + }); + + try { + // Bob's collaborator record lingers in storage (lazy revocation), but he is no longer + // reachable from the owner, so he no longer blocks the observation. + await expect(reopened.session.readThing(true)).resolves.toContain("removal"); + + // Removal also tore down his observer record, so re-adding him must not silently restore + // his coverage: he blocks again until he re-opens (which re-verifies him). + await reopened.overseer.addCollaborator(bob.bob, "build"); + await expect(reopened.session.readThing(true)).rejects.toThrow(/has not been verified/i); + } finally { + reopened.publicApi2[Symbol.dispose](); + } + }); + }); + + it.concurrent("the external-message path verifies collaborators like open() does", async () => { + await withSession(async publicApi => { + const [alice, bob, carol] = nextUsernames("alice", "bob", "carol"); + const aliceApi = await signUp(publicApi, alice); + const aliceAccount = await provisionAccount(aliceApi); + const gadgetKey = `external-${crypto.randomUUID()}`; + + // Alice creates the workspace through the external channel. No test user has an AI model, + // so a submission that passes the authorization gate is rejected with the model message -- + // which is what tells "passed the gate" apart from a gate denial below. + await expect(submitExternalMessage({ callerEmail: alice, gadgetKey, prompt: "hello" })) + .resolves.toMatchObject({ + accepted: false, message: expect.stringMatching(/AI model/i) }); + + // Wire the workspace up over the web API: bind a Thing, latch restricted mode, add Bob. + const gadgetId = await externalGadgetId(gadgetKey); + using overseer = await aliceApi.openGadget(gadgetId); + const gatekeeper = await overseer.newGatekeeper(aliceAccount.id, thingUrl("external")); + if (!gatekeeper) throw new Error("Failed to create the test connection"); + const session: any = await gatekeeper.openSession(); + await expect(session.readThing(true)).resolves.toContain("external"); + + const bobApi = await signUp(publicApi, bob); + const bobAccount = await provisionAccount(bobApi); + await overseer.addCollaborator(bob, "build"); + + // A stranger is turned away by role, before verification is ever attempted. + await signUp(publicApi, carol); + await expect(submitExternalMessage({ callerEmail: carol, gadgetKey, prompt: "hi" })) + .resolves.toMatchObject({ + accepted: false, message: expect.stringMatching(/do not have access/i) }); + + // Bob has build access but has never opened, so he was never observer-verified. The agent's + // reply could surface the restricted data the workspace already read, so the external path + // must refuse him rather than fall through to the model check. + await expect(submitExternalMessage({ callerEmail: bob, gadgetKey, prompt: "hi" })) + .resolves.toMatchObject({ + accepted: false, message: expect.stringMatching(/could not be verified/i) }); + + // Opening the workspace verifies him; the same submission now passes the gate and fails + // only on the missing AI model, exactly like the owner's did. + (await bobOpens(gadgetId, bobApi, bobAccount))[Symbol.dispose](); + await expect(submitExternalMessage({ callerEmail: bob, gadgetKey, prompt: "hi" })) + .resolves.toMatchObject({ + accepted: false, message: expect.stringMatching(/AI model/i) }); + }); + }); +}); diff --git a/packages/integration-tests/fixtures/gatekeeper-test/src/env.d.ts b/packages/integration-tests/fixtures/gatekeeper-test/src/env.d.ts index b5b372a78..8a9715af0 100644 --- a/packages/integration-tests/fixtures/gatekeeper-test/src/env.d.ts +++ b/packages/integration-tests/fixtures/gatekeeper-test/src/env.d.ts @@ -12,6 +12,17 @@ declare namespace Cloudflare { // Storage classes exposed as DO namespaces on ctx.exports. durableNamespaces: "TestGatekeeper" | "TestControl"; } + + interface Env { + // The Workshop's external-message gateway entrypoint (see wrangler.jsonc). The contract + // interface is not entrypoint-branded (the shipping class implements it), so brand it here to + // satisfy Fetcher's constraint. + WORKSHOP_EXTERNAL_MESSAGES: Fetcher< + import("@gadgets/workshop-shared/external-message-gateway").ExternalMessageGateway & + Rpc.WorkerEntrypointBranded>; + // The Workshop's Overseer DO namespace (see wrangler.jsonc); used only to derive ids. + WORKSHOP_OVERSEER: DurableObjectNamespace; + } } interface ExecutionContext { 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 a279323c6..19a3f907c 100644 --- a/packages/integration-tests/fixtures/gatekeeper-test/src/test-gatekeeper.ts +++ b/packages/integration-tests/fixtures/gatekeeper-test/src/test-gatekeeper.ts @@ -20,12 +20,15 @@ // 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, SupportedResource, VendorDescription, } from "@gadgets/workshop-shared/gatekeeper"; +import type { + ChatGatewayRpcTarget, GadgetResponse, +} from "@gadgets/workshop-shared/external-message-gateway"; // Nothing but classes and the default handler may be exported from a Worker entry module: workerd // treats every named export as an entrypoint and rejects anything that isn't one. @@ -57,13 +60,22 @@ const AVATAR = { type VerifyOutcome = { allow: true } | { allow: false; reason: string }; export class TestControl extends DurableObject { - setVerifyOutcome(label: string, outcome: VerifyOutcome): void { - this.ctx.storage.kv.put(`outcome:${label}`, outcome); + /** + * An outcome may target one bound resource (`resourceUrl` present) or the whole account. The + * resource-specific entry wins, so a test can refuse an account at one producer while the same + * account keeps passing everywhere else. + */ + setVerifyOutcome(label: string, outcome: VerifyOutcome, resourceUrl?: string): void { + this.ctx.storage.kv.put( + resourceUrl !== undefined ? `outcome:${label}|${resourceUrl}` : `outcome:${label}`, + outcome); } - getVerifyOutcome(label: string): VerifyOutcome { + getVerifyOutcome(label: string, resourceUrl: string): VerifyOutcome { // Default to admitting: a collaborator's first open has to be able to succeed. - return this.ctx.storage.kv.get(`outcome:${label}`) ?? { allow: true }; + return this.ctx.storage.kv.get(`outcome:${label}|${resourceUrl}`) + ?? this.ctx.storage.kv.get(`outcome:${label}`) + ?? { allow: true }; } recordAmbientVerification(label: string): void { @@ -217,8 +229,46 @@ 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, + }); + } + + /** The session owns the queue stub dup'd in startSession(); release it with the session. */ + [Symbol.dispose]() { + this.#queue[Symbol.dispose](); + } +} export class TestGatekeeper extends DurableObject implements Gatekeeper { @@ -251,8 +301,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; + } } /** @@ -269,7 +326,8 @@ export class TestGatekeeper this.ctx.storage.kv.put(`observer:${id}`, label); return; } - const outcome = await control(this.ctx.exports).getVerifyOutcome(label); + const outcome = + await control(this.ctx.exports).getVerifyOutcome(label, this.ctx.props.resourceUrl); if (!outcome.allow) throw new Error(outcome.reason); this.ctx.storage.kv.put(`observer:${id}`, label); } @@ -309,8 +367,16 @@ function isNonEmptyString(value: unknown): value is string { return typeof value === "string" && value.length > 0; } +/** + * Discards Gadget responses. The control endpoint below only asserts on the submission result, + * and the rejection paths under test return before any response is produced. + */ +class DevNullChatGateway extends RpcTarget implements ChatGatewayRpcTarget { + async onGadgetResponse(_response: GadgetResponse): Promise {} +} + export default { - async fetch(req: Request, _env: Cloudflare.Env, ctx: ExecutionContext): Promise { + async fetch(req: Request, env: Cloudflare.Env, ctx: ExecutionContext): Promise { const url = new URL(req.url); let body: unknown; @@ -325,20 +391,24 @@ export default { } } - // Set what addObserver() should do for one account. - // Body: {"label": "...", "allow": false, "reason": "..."} + // Set what addObserver() should do for one account -- everywhere, or (with `resourceUrl`) at + // one bound resource only, which wins over the account-wide entry. + // Body: {"label": "...", "allow": false, "reason": "...", "resourceUrl": "..."} if (url.pathname === "/control/verify-outcome" && req.method === "POST") { - const { label, allow, reason } = body as Record; + const { label, allow, reason, resourceUrl } = body as Record; if (!isNonEmptyString(label)) return badRequest("`label` must be a non-empty string"); if (typeof allow !== "boolean") return badRequest("`allow` must be a boolean"); if (reason !== undefined && typeof reason !== "string") { return badRequest("`reason` must be a string when present"); } + if (resourceUrl !== undefined && !isNonEmptyString(resourceUrl)) { + return badRequest("`resourceUrl` must be a non-empty string when present"); + } const outcome: VerifyOutcome = allow ? { allow: true } : { allow: false, reason: reason ?? "The test gatekeeper refused this account." }; - await control(ctx.exports).setVerifyOutcome(label, outcome); + await control(ctx.exports).setVerifyOutcome(label, outcome, resourceUrl); return new Response(null, { status: 204 }); } @@ -348,6 +418,38 @@ export default { return Response.json({ count: await control(ctx.exports).getAmbientVerificationCount(label) }); } + // Submit an external chat message through the Workshop's ExternalMessageGateway entrypoint, + // the way a chat-integration worker would, so tests can drive receiveExternalMessage(). + // Body: {"callerEmail", "gadgetKey", "chatKey", "messageKey", "gadgetTitle", "prompt"} + // -> SubmitExternalMessageResult + if (url.pathname === "/control/submit-external-message" && req.method === "POST") { + const fields = + ["callerEmail", "gadgetKey", "chatKey", "messageKey", "gadgetTitle", "prompt"] as const; + const input = {} as Record<(typeof fields)[number], string>; + for (const field of fields) { + const value = (body as Record)[field]; + if (!isNonEmptyString(value)) return badRequest(`\`${field}\` must be a non-empty string`); + input[field] = value; + } + // The instance becomes a stub when it crosses the RPC boundary; the parameter type can only + // name the stub side of that. + const chatGatewayRpcTarget = + new DevNullChatGateway() as unknown as RpcStub; + return Response.json(await env.WORKSHOP_EXTERNAL_MESSAGES.submitExternalMessage( + { ...input, chatGatewayRpcTarget })); + } + + // Map an external gadgetKey to the Overseer id the gateway targets -- the DO named + // ":", where "test" is the `source` prop on WORKSHOP_EXTERNAL_MESSAGES -- + // so a test can open the same workspace over the web API, which addresses by DO id string. + // Body: {"gadgetKey": "..."} -> {"gadgetId": "..."} + if (url.pathname === "/control/external-gadget-id" && req.method === "POST") { + const { gadgetKey } = body as Record; + if (!isNonEmptyString(gadgetKey)) return badRequest("`gadgetKey` must be a non-empty string"); + return Response.json( + { gadgetId: env.WORKSHOP_OVERSEER.idFromName(`test:${gadgetKey}`).toString() }); + } + // Make this Worker issue a subrequest, so a test can prove that Worker-originated fetches really // do route through the interceptor rather than out to the internet. // diff --git a/packages/integration-tests/fixtures/gatekeeper-test/wrangler.jsonc b/packages/integration-tests/fixtures/gatekeeper-test/wrangler.jsonc index d4ca3ff2d..96167a8a4 100644 --- a/packages/integration-tests/fixtures/gatekeeper-test/wrangler.jsonc +++ b/packages/integration-tests/fixtures/gatekeeper-test/wrangler.jsonc @@ -12,7 +12,32 @@ "compatibility_date": "2026-02-02", "compatibility_flags": ["experimental", "allow_irrevocable_stub_storage"], - // DO classes are reached via ctx.exports; no durable_objects binding needed. + // Lets the control surface submit external chat messages through the Workshop's gateway + // entrypoint the way a real chat-integration worker (bound with its own `source` prop) would. + // The harness always boots workshop-backend as the primary worker, so the name resolves. + "services": [ + { + "binding": "WORKSHOP_EXTERNAL_MESSAGES", + "service": "workshop-backend", + "entrypoint": "ExternalMessageGateway", + "props": { "source": "test" } + } + ], + + // The Workshop's Overseer namespace, so the control surface can derive the DO id behind an + // external gadgetKey -- the same name-derived id the gateway targets -- for tests to open the + // workspace over the web API. The binding only derives ids; it never reaches an instance. + "durable_objects": { + "bindings": [ + { + "name": "WORKSHOP_OVERSEER", + "class_name": "OverseerDurableObject", + "script_name": "workshop-backend" + } + ] + }, + + // This worker's own DO classes are reached via ctx.exports; no durable_objects binding needed. "migrations": [ { "tag": "v0", diff --git a/packages/integration-tests/src/harness.ts b/packages/integration-tests/src/harness.ts index 199facdeb..da1df1514 100644 --- a/packages/integration-tests/src/harness.ts +++ b/packages/integration-tests/src/harness.ts @@ -76,6 +76,12 @@ function readWorkerConfig(dir: string): WorkerConfig { const config = parsed.data; config.build = { ...config.build, cwd: dir }; config.main = join(dir, config.main); + + // Local-dev var files (.dev.vars/.env at the harness root) must not leak into tests: a + // developer's local settings (say CF_AI_GATEWAY_*) would make suites behave differently on + // their machine than in CI -- up to sending real AI traffic. Declaring an empty required-secrets + // list makes wrangler exclude every such key that is not already a config var. + config.secrets = { required: [] }; return config; } diff --git a/packages/integration-tests/src/rpc-client.ts b/packages/integration-tests/src/rpc-client.ts index 83ec8da96..68911da2a 100644 --- a/packages/integration-tests/src/rpc-client.ts +++ b/packages/integration-tests/src/rpc-client.ts @@ -77,6 +77,14 @@ export async function signUp( return (await api.authenticate(token)) as unknown as RpcStub; } +/** Log back into an account created by signUp(), e.g. from a fresh connection. */ +export async function logIn( + api: RpcStub, username: string): Promise> { + const token = await api.login(username, passwordHashFor(username)); + if (!token) throw new Error(`Login failed for "${username}"`); + return (await api.authenticate(token)) as unknown as RpcStub; +} + export type ConnectedAccount = { id: number; vendorId: string; @@ -138,10 +146,12 @@ export const MAX_OBSERVER_PROMPTS = 2; */ export class ObserverConfigRecorder extends RpcTarget implements ObserverConfigCallback { readonly calls: ObserverBindingNeed[][] = []; - #responses: ((needs: ObserverBindingNeed[]) => ObserverAccountChoice[])[] = []; + #responses: ((needs: ObserverBindingNeed[]) + => ObserverAccountChoice[] | Promise)[] = []; /** Queue one response. The nth configure() call is answered by the nth queued responder. */ - respondWith(responder: (needs: ObserverBindingNeed[]) => ObserverAccountChoice[]): this { + respondWith(responder: (needs: ObserverBindingNeed[]) + => ObserverAccountChoice[] | Promise): this { this.#responses.push(responder); return this; } From fac71a1fd105895b8fb6fed0523735d1c1d14a7c Mon Sep 17 00:00:00 2001 From: Maximo Guk <62088388+Maximo-Guk@users.noreply.github.com> Date: Thu, 20 Aug 2026 14:32:55 -0500 Subject: [PATCH 3/4] Adopt the containsRestrictedData rename in gatekeepers and guidance Update gatekeeper-google to the renamed flag, and revise the gatekeeper-mcp README and write-gatekeeper skill guidance to describe observer-verified sharing rather than a wholesale sharing prohibition. Co-Authored-By: Claude Fable 5 --- .agents/skills/write-gatekeeper/SKILL.md | 2 +- packages/gatekeeper-google/README.md | 6 ++-- .../__tests__/bigquery-resource.test.ts | 4 +-- .../src/bigquery-resource.ts | 6 ++-- .../gatekeeper-google/src/bigquery-types.d.ts | 5 ++-- packages/gatekeeper-google/src/google.ts | 29 ++++++++++--------- packages/gatekeeper-mcp/README.md | 11 +++---- 7 files changed, 32 insertions(+), 31 deletions(-) diff --git a/.agents/skills/write-gatekeeper/SKILL.md b/.agents/skills/write-gatekeeper/SKILL.md index 7416bd12d..1e470f73c 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/packages/gatekeeper-google/README.md b/packages/gatekeeper-google/README.md index 70c5c9d65..c37ce5788 100644 --- a/packages/gatekeeper-google/README.md +++ b/packages/gatekeeper-google/README.md @@ -112,9 +112,9 @@ Such a connection names **two** projects, because a public project can be read b That split is the point. The session's scope is the *public* project, so the existing `referencedTables` check rejects any query touching the billing project: the connection spends the user's money but cannot read a byte of their data. Because it provably reads only data anyone -signed in to Google can already read, its observations do **not** set `prohibitAllSharing` — so a -workspace built on one stays shareable and can still perform actions, rather than dropping into the -lockdown mode that reading a private BigQuery dataset triggers. +signed in to Google can already read, its observations are **not** marked as restricted data — so a +workspace built on one stays shareable and can still fetch from the web, unlike one that has read a +private BigQuery dataset. Costs still apply: public tables are large and the queries bill the user's project. The default 100 GB `maximumBytesBilled` cap and the mandatory dry-run gate apply exactly as they do to a diff --git a/packages/gatekeeper-google/__tests__/bigquery-resource.test.ts b/packages/gatekeeper-google/__tests__/bigquery-resource.test.ts index a332b1df8..22007236f 100644 --- a/packages/gatekeeper-google/__tests__/bigquery-resource.test.ts +++ b/packages/gatekeeper-google/__tests__/bigquery-resource.test.ts @@ -93,8 +93,8 @@ describe("public resource URLs", () => { } }); - // The check the "reads only public data" label -- and so the sharing exemption -- rests on. - // Without it a binding could be pointed at a private project the user's token can reach. + // The check the "reads only public data" label -- and so the restricted-data exemption -- rests + // on. Without it a binding could be pointed at a private project the user's token can reach. it("rejects a data project outside the allowlist", () => { expect(() => parse(`https://${BIGQUERY_PUBLIC_HOST}/my-project/my-private-project`)) .toThrow(/not a known public BigQuery project/); diff --git a/packages/gatekeeper-google/src/bigquery-resource.ts b/packages/gatekeeper-google/src/bigquery-resource.ts index 66154b7f7..509341d47 100644 --- a/packages/gatekeeper-google/src/bigquery-resource.ts +++ b/packages/gatekeeper-google/src/bigquery-resource.ts @@ -44,9 +44,9 @@ export type PublicDataProject = { * The projects a public-data binding is allowed to read. * * An allowlist rather than free text, because it is what makes a public binding's "reads only - * public data" label true: that label is why such a binding is exempted from `prohibitAllSharing` - * (see `BigQuerySessionImpl`), and free text would let a binding titled "public data" read - * whatever private project the user's own token happens to reach. + * public data" label true: that label is why such a binding is exempted from + * `containsRestrictedData` (see `BigQuerySessionImpl`), and free text would let a binding titled + * "public data" read whatever private project the user's own token happens to reach. * * Every entry grants `roles/bigquery.dataViewer` to `allAuthenticatedUsers`. Adding one is a * one-line data change; removing one correctly disables existing bindings, since the allowlist is diff --git a/packages/gatekeeper-google/src/bigquery-types.d.ts b/packages/gatekeeper-google/src/bigquery-types.d.ts index 7993740b9..450b77697 100644 --- a/packages/gatekeeper-google/src/bigquery-types.d.ts +++ b/packages/gatekeeper-google/src/bigquery-types.d.ts @@ -131,9 +131,8 @@ export type BigQueryQueryOptions = { * projects (e.g. `bigquery-public-data`), which any Google account can read, and separately names * one of the user's own projects to bill the query jobs to. Only the public project is readable: * a query referencing a table in the billing project — or in any other project — is rejected, so a - * join between a public table and the user's own data is not possible through this connection. - * Reading through it does not lock the workspace down or prohibit sharing it, since anyone signed - * in to Google could read the same rows. + * join between a public table and the user's own data is not possible through this connection. Its + * results are not treated as restricted data, since anyone signed in to Google could read them. * * BigQuery uses Google Standard SQL by default. Legacy SQL is not supported. */ diff --git a/packages/gatekeeper-google/src/google.ts b/packages/gatekeeper-google/src/google.ts index fb76be7ea..126edd815 100644 --- a/packages/gatekeeper-google/src/google.ts +++ b/packages/gatekeeper-google/src/google.ts @@ -1710,8 +1710,9 @@ export class GmailGatekeeperImpl extends DurableObject): Promise { throw new Error( @@ -2967,8 +2968,8 @@ type BigQueryGatekeeperImplProps = { scopedDatasetId?: string; scopedTableId?: string; // Set iff `scopedProjectId` is an allowlisted public-data project. Such a session reads data - // every authenticated Google user can already read, so its observations neither prohibit sharing - // nor need a per-dataset IAM check on each observer. Re-verified against the allowlist when a + // every authenticated Google user can already read, so its observations are not restricted data + // and its observers need no per-dataset IAM check. Re-verified against the allowlist when a // session starts, so removing a project from the list disables existing bindings too. publicData?: true; }; @@ -3032,7 +3033,7 @@ export class BigQueryGatekeeperImpl // them, but the props outlive that call and are persisted with the binding, so this is what // makes turning the deployment flag off -- or dropping a project from PUBLIC_DATA_PROJECTS -- // disable the bindings that already exist, instead of leaving them reading a project we no - // longer vouch for as public and, on that basis, exempt from the sharing prohibition. + // longer vouch for as public and unmarked as restricted data on that basis. if (publicData) { if (!bigQueryPublicDataEnabled(this.env)) { throw new Error(BIGQUERY_PUBLIC_DATA_DISABLED_MESSAGE); @@ -3119,7 +3120,7 @@ class BigQuerySessionImpl extends RpcTarget implements BigQuerySession { #scopedDatasetId?: string; #scopedTableId?: string; // Whether #scopedProjectId is an allowlisted public-data project, in which case nothing this - // session returns needs to prohibit sharing the workspace. + // session returns is restricted data. #publicData: boolean; // Records the datasets an observation reveals and returns observers to exclude (see // BigQueryGatekeeperImpl.#prepareDatasetObservation). @@ -3303,7 +3304,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: !this.#publicData, + containsRestrictedData: !this.#publicData, }); let result = await this.#api.query(billingProject, sql, { @@ -3335,7 +3336,7 @@ class BigQuerySessionImpl extends RpcTarget implements BigQuerySession { description: `Estimated bytes processed: ${estimate.bytesProcessed.toLocaleString()}\n` + `Referenced tables: ${estimate.referencedTables.join(", ") || "(none)"}`, - prohibitAllSharing: !this.#publicData, + containsRestrictedData: !this.#publicData, }); return estimate; @@ -3347,7 +3348,7 @@ class BigQuerySessionImpl extends RpcTarget implements BigQuerySession { await this.#authorizeDatasets([], { title: "Get BigQuery project", description: `Returned the scoped project: \`${this.#scopedProjectId}\`.`, - prohibitAllSharing: !this.#publicData, + containsRestrictedData: !this.#publicData, }); return result; } @@ -3365,7 +3366,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: !this.#publicData, + containsRestrictedData: !this.#publicData, }); return [dataset]; } @@ -3375,7 +3376,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: !this.#publicData, + containsRestrictedData: !this.#publicData, }); return result; } @@ -3400,7 +3401,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: !this.#publicData, + containsRestrictedData: !this.#publicData, }); return [table]; } @@ -3409,7 +3410,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: !this.#publicData, + containsRestrictedData: !this.#publicData, }); return result; } @@ -3445,7 +3446,7 @@ class BigQuerySessionImpl extends RpcTarget implements BigQuerySession { title: `Describe ${p}.${d}.${t}`, description: `Described table \`${p}.${d}.${t}\` (${result.schema.length} columns).`, - prohibitAllSharing: !this.#publicData, + containsRestrictedData: !this.#publicData, }); return result; } diff --git a/packages/gatekeeper-mcp/README.md b/packages/gatekeeper-mcp/README.md index 93317ca88..1e993073d 100644 --- a/packages/gatekeeper-mcp/README.md +++ b/packages/gatekeeper-mcp/README.md @@ -177,8 +177,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 @@ -206,9 +206,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 From d120690a23f2b71397a5e152beeb0e897274e2e9 Mon Sep 17 00:00:00 2001 From: Maximo Guk <62088388+Maximo-Guk@users.noreply.github.com> Date: Thu, 20 Aug 2026 14:32:55 -0500 Subject: [PATCH 4/4] Let the Share modal share restricted-data workspaces behind a notice The modal no longer refuses to share a workspace with restricted data; it explains that collaborators must be verified against the producing connection at their next open. The share key is retained across failures and reloads (sessionStorage tier) so a recipient who is denied verification can retry, and is discarded at the first successful open so a later removal isn't undone by an automatic re-redemption. The sessionStorage tier lives in retainedShareKeys.ts (so useAuth does not import the workspace hook) and is bound to the user who captured the key, so it cannot cross users in a shared tab: - Entries are identity-stamped (JSON {key, userId}) with the capturing session's whoami, resolved from the same stub the open is issued on. The retained-read path honors an entry only when the current session's identity matches; a definite mismatch sweeps it, a transport failure leaves it but does not attach the key. The common keyless open stays fully pipelined -- identity is only resolved when a fragment key is captured (async, gated against the success-discard racing it) or a stored entry exists (rare: only after a reload mid-retry). Malformed or unstamped entries read as absent. - logout() sweeps the whole retention prefix, before the CF Access navigate-away. The in-memory ref tier needs no stamp: it is bounded by the editor's lifetime (logout unmounts it; CF Access logout navigates away). Residual: a reload before the identity stamp lands loses retention, recovered by re-clicking the invite link. Co-Authored-By: Claude Fable 5 --- packages/workshop-frontend/src/ShareModal.tsx | 45 ++-- .../src/retainedShareKeys.ts | 74 ++++++ .../workshop-frontend/src/useAuth.test.tsx | 19 ++ packages/workshop-frontend/src/useAuth.ts | 4 + .../src/useWorkspaceOpen.test.tsx | 213 +++++++++++++++++- .../workshop-frontend/src/useWorkspaceOpen.ts | 83 ++++++- 6 files changed, 406 insertions(+), 32 deletions(-) create mode 100644 packages/workshop-frontend/src/retainedShareKeys.ts diff --git a/packages/workshop-frontend/src/ShareModal.tsx b/packages/workshop-frontend/src/ShareModal.tsx index b6212442d..dba7d869b 100644 --- a/packages/workshop-frontend/src/ShareModal.tsx +++ b/packages/workshop-frontend/src/ShareModal.tsx @@ -372,7 +372,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 { @@ -559,7 +559,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) @@ -585,7 +585,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 { @@ -611,7 +611,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 { @@ -775,23 +775,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'} @@ -911,16 +903,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)}> @@ -932,7 +924,6 @@ export default function ShareModal({ open, onClose, overseer, metadata, currentU
)} - - )}
diff --git a/packages/workshop-frontend/src/retainedShareKeys.ts b/packages/workshop-frontend/src/retainedShareKeys.ts new file mode 100644 index 000000000..6fcef113f --- /dev/null +++ b/packages/workshop-frontend/src/retainedShareKeys.ts @@ -0,0 +1,74 @@ +// The sessionStorage tier of share-key retention (see useWorkspaceOpen's retainedShareKeyRef for +// the model and the security notes). Split into its own module so useAuth can sweep the entries +// on logout without importing the workspace hook. +// +// Entries are identity-stamped: each stores the userId of the session that captured the key, and +// readers only honor an entry whose stamp matches the current session's identity. That is what +// keeps a key from crossing users in a shared tab -- user A's retained key must not be silently +// re-redeemed under user B's account. Logout additionally sweeps the whole prefix +// (clearAllRetainedShareKeys), which also collects stale entries from older storage formats. +// +// All operations are best-effort: storage can be unavailable in restricted browser contexts, and +// a lost key only costs the user a re-visit of their invite link. + +const RETAINED_SHARE_KEY_PREFIX = 'gadgets:retained-share-key:' +const V2_PREFIX = `${RETAINED_SHARE_KEY_PREFIX}v2:` + +export type RetainedShareKey = { + key: string + /** The profile id of the user whose session captured the key. */ + userId: string +} + +function storageKey(workspaceId: string): string { + return `${V2_PREFIX}${workspaceId}` +} + +export function writeRetainedShareKey(workspaceId: string, entry: RetainedShareKey): void { + try { + window.sessionStorage.setItem(storageKey(workspaceId), JSON.stringify(entry)) + } catch { + // Best-effort; see above. + } +} + +export function readRetainedShareKey(workspaceId: string): RetainedShareKey | undefined { + try { + const raw = window.sessionStorage.getItem(storageKey(workspaceId)) + if (!raw) return undefined + const parsed: unknown = JSON.parse(raw) + if (typeof parsed !== 'object' || parsed === null) return undefined + const { key, userId } = parsed as { key?: unknown; userId?: unknown } + // Lenient bounds only -- the server is the validator of record for the key itself. A v1 + // (bare-string) or otherwise malformed entry fails the shape check and reads as absent. + if (typeof key === 'string' && key.length > 0 && key.length <= 128 && + typeof userId === 'string') { + return { key, userId } + } + } catch { + // Best-effort; see above (JSON.parse failure on a v1 entry lands here too). + } + return undefined +} + +export function clearRetainedShareKey(workspaceId: string): void { + try { + window.sessionStorage.removeItem(storageKey(workspaceId)) + } catch { + // Best-effort; see above. + } +} + +/** Sweep every retained share key, of any format version. Called on logout. */ +export function clearAllRetainedShareKeys(): void { + try { + const doomed: string[] = [] + for (let i = 0; i < window.sessionStorage.length; i++) { + const key = window.sessionStorage.key(i) + if (key?.startsWith(RETAINED_SHARE_KEY_PREFIX)) doomed.push(key) + } + for (const key of doomed) window.sessionStorage.removeItem(key) + } catch { + // Best-effort; see above. + } +} diff --git a/packages/workshop-frontend/src/useAuth.test.tsx b/packages/workshop-frontend/src/useAuth.test.tsx index 6b30bf47d..e66c29ecb 100644 --- a/packages/workshop-frontend/src/useAuth.test.tsx +++ b/packages/workshop-frontend/src/useAuth.test.tsx @@ -69,6 +69,7 @@ describe('useAuth error reporting identity', () => { containers.forEach(container => container.remove()) containers.length = 0 localStorage.clear() + sessionStorage.clear() vi.unstubAllEnvs() vi.clearAllMocks() }) @@ -148,6 +149,24 @@ describe('useAuth error reporting identity', () => { expect(setReportedUserId).toHaveBeenLastCalledWith(undefined) }) + it('sweeps retained share keys of every format on logout, leaving unrelated storage', async () => { + // The retained-key entries are per-user secrets; the next user of this tab must not inherit + // them. The sweep covers the whole prefix, so stale v1 (pre-identity-stamp) entries go too. + sessionStorage.setItem( + 'gadgets:retained-share-key:v2:workspace-1', + JSON.stringify({ key: 'cafe', userId: 'person@example.com' })) + sessionStorage.setItem('gadgets:retained-share-key:v1:workspace-2', 'deadbeef') + sessionStorage.setItem('gadgets:unrelated', 'kept') + localStorage.setItem('authToken', 'stored-token') + const { controls } = await mount(stubPublicApi(person)) + + act(() => controls.logout()) + + expect(sessionStorage.getItem('gadgets:retained-share-key:v2:workspace-1')).toBeNull() + expect(sessionStorage.getItem('gadgets:retained-share-key:v1:workspace-2')).toBeNull() + expect(sessionStorage.getItem('gadgets:unrelated')).toBe('kept') + }) + it('ignores a lookup that resolves after logout', async () => { localStorage.setItem('authToken', 'stored-token') const { api, release } = deferredPublicApi() diff --git a/packages/workshop-frontend/src/useAuth.ts b/packages/workshop-frontend/src/useAuth.ts index 1eeb23138..26ce76fea 100644 --- a/packages/workshop-frontend/src/useAuth.ts +++ b/packages/workshop-frontend/src/useAuth.ts @@ -2,6 +2,7 @@ import { useState, useEffect, useRef } from 'react' import { RpcStub } from 'capnweb' import { PublicApi, AuthenticatedApi } from '@gadgets/workshop-shared/api' import { setReportedUserId } from './errorReporting' +import { clearAllRetainedShareKeys } from './retainedShareKeys' const CF_ACCESS_MODE = import.meta.env.VITE_CF_ACCESS_MODE === 'true' @@ -124,6 +125,9 @@ export function useAuth(publicApi: RpcStub) { const logout = () => { setReportedUserId(undefined) + // Retained share keys belong to the session that captured them; the next user of this tab + // must not inherit them. Swept before the CF Access navigation below, which never returns. + clearAllRetainedShareKeys() if (CF_ACCESS_MODE) { window.location.assign('/cdn-cgi/access/logout') diff --git a/packages/workshop-frontend/src/useWorkspaceOpen.test.tsx b/packages/workshop-frontend/src/useWorkspaceOpen.test.tsx index 235cf264e..5b00f8cb8 100644 --- a/packages/workshop-frontend/src/useWorkspaceOpen.test.tsx +++ b/packages/workshop-frontend/src/useWorkspaceOpen.test.tsx @@ -33,8 +33,20 @@ function disposableStub(value: T, dispose = vi.fn<() => void>( return Object.assign(value, { [Symbol.dispose]: dispose }) as T & Disposable } +// The identity the mocked session reports, and the stamp share-key retention stores under it. +const WHOAMI_USER = { type: 'user', id: 'person@example.com', name: 'Person' } + function api(overseer: RpcStub): RpcStub { - return { openGadget: () => overseer } as unknown as RpcStub + return { + openGadget: () => overseer, + whoami: async () => WHOAMI_USER, + } as unknown as RpcStub +} + +const RETAINED_V2_KEY = 'gadgets:retained-share-key:v2:workspace-1' + +function retainedEntry(key: string, userId = WHOAMI_USER.id): string { + return JSON.stringify({ key, userId }) } const METADATA = { @@ -71,6 +83,8 @@ describe('useWorkspaceOpen', () => { act(() => root?.unmount()) container?.remove() document.title = '' + window.location.hash = '' + sessionStorage.clear() vi.restoreAllMocks() }) @@ -108,6 +122,203 @@ describe('useWorkspaceOpen', () => { expect(subscriptionDispose).toHaveBeenCalledOnce() }) + it('persists the fragment share key and re-sends it after a reload', async () => { + vi.spyOn(console, 'error').mockImplementation(() => {}) + window.location.hash = '#share=deadbeef' + const sentKeys: (string | undefined)[] = [] + const deniedOverseer = disposableStub({ + subscribeToMetadata: vi.fn<() => Promise>>(async () => { + throw createOpenGadgetError(OPEN_GADGET_ERROR_CODES.workspaceAccessDenied) + }), + }) as unknown as RpcStub + const authenticatedApi = { + openGadget: (_id: string, shareKey?: string) => { + sentKeys.push(shareKey) + return deniedOverseer + }, + whoami: async () => WHOAMI_USER, + } as unknown as RpcStub + + function Probe() { + useWorkspaceOpen({ + id: 'workspace-1', + authenticatedApi, + onInvalidShareKey: () => {}, + onMetadata: () => {}, + // The app strips the fragment before the open is issued; mirror that here so the + // second mount can only recover the key from storage. + onShareKeyConsumed: () => { window.location.hash = '' }, + }) + return null + } + + container = document.createElement('div') + document.body.append(container) + root = createRoot(container) + await act(async () => root!.render()) + expect(sentKeys).toEqual(['deadbeef']) + expect(window.location.hash).toBe('') + // The persisted entry is stamped with the capturing session's identity. + expect(sessionStorage.getItem(RETAINED_V2_KEY)).toBe(retainedEntry('deadbeef')) + + // A reload drops the hook's in-memory ref: unmount and mount a fresh root. The failed open + // never cleared the persisted key, so the fresh mount re-sends it instead of dead-ending on + // the access-denied page. + act(() => root!.unmount()) + root = createRoot(container) + await act(async () => root!.render()) + expect(sentKeys).toEqual(['deadbeef', 'deadbeef']) + }) + + it('clears the persisted key on a successful open', async () => { + window.location.hash = '#share=cafe' + const sentKeys: (string | undefined)[] = [] + const overseer = disposableStub({ + subscribeToMetadata: vi.fn< + (callback: (metadata: GadgetMetadata) => void) => Promise> + >(async callback => { + callback(METADATA) + return disposableStub({}) as RpcStub<{}> + }), + }) as unknown as RpcStub + const authenticatedApi = { + openGadget: (_id: string, shareKey?: string) => { + sentKeys.push(shareKey) + return overseer + }, + whoami: async () => WHOAMI_USER, + } as unknown as RpcStub + + let retry!: () => void + function Probe() { + const state = useWorkspaceOpen({ + id: 'workspace-1', + authenticatedApi, + onInvalidShareKey: () => {}, + onMetadata: () => {}, + onShareKeyConsumed: () => { window.location.hash = '' }, + }) + retry = state.retry + return null + } + + container = document.createElement('div') + document.body.append(container) + root = createRoot(container) + await act(async () => root!.render()) + expect(sentKeys).toEqual(['cafe']) + // The open succeeded, so the persisted secret is gone -- even though the identity stamp + // that writes it resolves asynchronously alongside the open... + expect(sessionStorage.getItem(RETAINED_V2_KEY)).toBeNull() + + // ...and so is the in-memory ref: a retry resolves keylessly from the confirmed edge. + await act(async () => retry()) + expect(sentKeys).toEqual(['cafe', undefined]) + }) + + it('a reconnect after a successful open retries keylessly', async () => { + // The owner-removal scenario: the revocation restart kills the WebSocket, useAuth swaps in + // a new authenticatedApi while the editor stays mounted, and the open effect re-runs. A + // retained key here would silently re-redeem the still-active link, undoing the removal. + window.location.hash = '#share=cafe' + const sentKeys: (string | undefined)[] = [] + const overseer = disposableStub({ + subscribeToMetadata: vi.fn< + (callback: (metadata: GadgetMetadata) => void) => Promise> + >(async callback => { + callback(METADATA) + return disposableStub({}) as RpcStub<{}> + }), + }) as unknown as RpcStub + // Each call yields a distinct stub identity, like a fresh post-reconnect connection. + const keyedApi = () => ({ + openGadget: (_id: string, shareKey?: string) => { + sentKeys.push(shareKey) + return overseer + }, + whoami: async () => WHOAMI_USER, + } as unknown as RpcStub) + + function Probe({ authenticatedApi }: { authenticatedApi: RpcStub }) { + useWorkspaceOpen({ + id: 'workspace-1', + authenticatedApi, + onInvalidShareKey: () => {}, + onMetadata: () => {}, + onShareKeyConsumed: () => { window.location.hash = '' }, + }) + return null + } + + container = document.createElement('div') + document.body.append(container) + root = createRoot(container) + await act(async () => root!.render()) + expect(sentKeys).toEqual(['cafe']) + + await act(async () => root!.render()) + expect(sentKeys).toEqual(['cafe', undefined]) + }) + + it('ignores and sweeps a retained key stamped by a different user', async () => { + // The shared-tab user switch: A's failed keyed open left a retained entry, A logged out + // without the sweep landing (or the entry predates it), and B opens the same workspace. The + // key must not be redeemed under B's account, and the stale entry goes away. + sessionStorage.setItem(RETAINED_V2_KEY, retainedEntry('cafe', 'someone-else@example.com')) + const sentKeys: (string | undefined)[] = [] + const overseer = disposableStub({ + subscribeToMetadata: vi.fn< + (callback: (metadata: GadgetMetadata) => void) => Promise> + >(async callback => { + callback(METADATA) + return disposableStub({}) as RpcStub<{}> + }), + }) as unknown as RpcStub + const authenticatedApi = { + openGadget: (_id: string, shareKey?: string) => { + sentKeys.push(shareKey) + return overseer + }, + whoami: async () => WHOAMI_USER, + } as unknown as RpcStub + + container = document.createElement('div') + document.body.append(container) + root = createRoot(container) + await act(async () => root!.render()) + + expect(sentKeys).toEqual([undefined]) + expect(sessionStorage.getItem(RETAINED_V2_KEY)).toBeNull() + }) + + it('neither attaches nor sweeps the retained key when identity cannot be resolved', async () => { + vi.spyOn(console, 'error').mockImplementation(() => {}) + // A transport failure leaves the identity unknown: the entry may well belong to this user, + // so it must survive for a later attempt, but the key must not be attached blind. + sessionStorage.setItem(RETAINED_V2_KEY, retainedEntry('cafe')) + const sentKeys: (string | undefined)[] = [] + const deniedOverseer = disposableStub({ + subscribeToMetadata: vi.fn<() => Promise>>(async () => { + throw createOpenGadgetError(OPEN_GADGET_ERROR_CODES.workspaceAccessDenied) + }), + }) as unknown as RpcStub + const authenticatedApi = { + openGadget: (_id: string, shareKey?: string) => { + sentKeys.push(shareKey) + return deniedOverseer + }, + whoami: async () => { throw new Error('connection lost') }, + } as unknown as RpcStub + + container = document.createElement('div') + document.body.append(container) + root = createRoot(container) + await act(async () => root!.render()) + + expect(sentKeys).toEqual([undefined]) + expect(sessionStorage.getItem(RETAINED_V2_KEY)).toBe(retainedEntry('cafe')) + }) + it('clears loaded metadata and title and disposes the failed stub after access is denied', async () => { vi.spyOn(console, 'error').mockImplementation(() => {}) document.title = 'outside' diff --git a/packages/workshop-frontend/src/useWorkspaceOpen.ts b/packages/workshop-frontend/src/useWorkspaceOpen.ts index 842fba8a5..331e2fe4c 100644 --- a/packages/workshop-frontend/src/useWorkspaceOpen.ts +++ b/packages/workshop-frontend/src/useWorkspaceOpen.ts @@ -10,6 +10,11 @@ import type { } from '@gadgets/workshop-shared/api' import { reportIssue } from './errorReporting' import { useDocumentTitle } from './useDocumentTitle' +import { + clearRetainedShareKey, + readRetainedShareKey, + writeRetainedShareKey, +} from './retainedShareKeys' import { classifyWorkspaceOpenFailure, type WorkspaceOpenFailureKind, @@ -49,6 +54,24 @@ export function useWorkspaceOpen({ const [observerConfig, setObserverConfig] = useState(null) const [reloadNonce, setReloadNonce] = useState(0) const openWorkspaceIdRef = useRef(undefined) + // The share key from the URL fragment, retained after the fragment is stripped. A failed or + // cancelled first open reverts the redemption server-side, so a retry (the retry button, or a + // reconnection) must re-send the key or it dead-ends on access-denied; never cleared on + // failure -- that is the point. Retention has two tiers: this in-memory ref, and a + // sessionStorage entry that also survives a reload (see retainedShareKeys.ts). Both are + // discarded at the *first successful open*: from then on the confirmed edge makes every retry + // resolvable keylessly, and a kept key would re-redeem the still-active link after an owner + // removal (see the success block below). The sessionStorage tier is identity-stamped with the + // capturing session's userId and honored only when the reading session's identity matches, so + // a key never crosses users in a shared tab (logout additionally sweeps all entries). The + // in-memory ref needs no stamp: it is bounded by this component's lifetime -- logout unmounts + // the editor, and CF Access logout navigates away. Residual: a reload before the async + // identity stamp lands loses retention, recovered by re-clicking the invite link. The secret + // never enters the URL or history -- the fragment is stripped before openGadget is even + // issued -- nor error reports (normalizePageLocation keeps origin+pathname only); + // sessionStorage is same-origin, per-tab, and dies with the tab, and gadget UIs run in + // opaque-origin frames that cannot read it. + const retainedShareKeyRef = useRef<{ id: string; key: string } | null>(null) const pendingObserverRejectRef = useRef<((error: unknown) => void) | null>(null) const callbacksRef = useRef({ onMetadata, onShareKeyConsumed, onInvalidShareKey }) callbacksRef.current = { onMetadata, onShareKeyConsumed, onInvalidShareKey } @@ -87,10 +110,53 @@ export function useWorkspaceOpen({ } if (!hadOpenWorkspace) setError(null) + // Set by the success path so the capture path's async identity stamp (below) cannot + // re-write a retained-key entry this attempt has already discarded. + let shareKeyDiscarded = false + try { const hash = window.location.hash - const shareKey = hash.startsWith('#share=') ? hash.slice('#share='.length) : undefined - if (shareKey) callbacksRef.current.onShareKeyConsumed() + let shareKey = hash.startsWith('#share=') ? hash.slice('#share='.length) : undefined + if (shareKey) { + retainedShareKeyRef.current = { id, key: shareKey } + // Stamp the sessionStorage tier with the capturing session's identity, resolved from + // the same stub the open is issued on (useAuth state can be stale across stub swaps). + // Async so the open itself stays pipelined; not gated on `cancelled`, since the stamp + // binds the key to whoever captured it regardless of how this attempt ends (a capture + // attempt cancelled by a remount must still leave the entry for a later reload). It is + // gated on this attempt's success-discard, so a stamp resolving late cannot resurrect + // an entry the successful open just retired. + const capturedKey = shareKey + authenticatedApi.whoami().then(info => { + if (info.type === 'user' && !shareKeyDiscarded) { + writeRetainedShareKey(id, { key: capturedKey, userId: info.id }) + } + }).catch(() => {}) + callbacksRef.current.onShareKeyConsumed() + } else if (retainedShareKeyRef.current?.id === id) { + shareKey = retainedShareKeyRef.current.key + } else { + // A reload lost the in-memory ref; the sessionStorage tier is what keeps a failed + // first open retryable across it. Rare path, so the identity round trip here does not + // cost the common keyless open its pipelining. + const retained = readRetainedShareKey(id) + if (retained) { + try { + const info = await authenticatedApi.whoami() + if (info.type === 'user' && info.id === retained.userId) { + shareKey = retained.key + retainedShareKeyRef.current = { id, key: retained.key } + } else { + // Definitely someone else's key (a same-tab user switch): sweep it rather than + // redeem it under the wrong account. + clearRetainedShareKey(id) + } + } catch { + // Transport failure: identity unknown, so neither attach the key nor discard an + // entry that may belong to this user. The open proceeds keylessly. + } + } + } const configureObserversTarget = new (class extends RpcTarget implements ObserverConfigCallback { configure(needs: ObserverBindingNeed[]): Promise { @@ -130,6 +196,16 @@ export function useWorkspaceOpen({ metadataSubscription = resolvedSubscription openWorkspaceIdRef.current = id + // The open succeeded, so the key's job is done: the redeemed edge is confirmed, and + // every later retry or reconnect resolves keylessly from the permission graph. Discard + // both retention tiers -- a *kept* key would silently re-redeem the still-active link + // after an owner removes this collaborator (the revocation restart reconnects with a + // new authenticatedApi, re-running this effect while the component stays mounted), + // undoing the removal. (`shareKeyDiscarded` keeps the capture path's still-in-flight + // identity stamp from re-writing the entry after this.) + shareKeyDiscarded = true + retainedShareKeyRef.current = null + clearRetainedShareKey(id) setError(null) if (connectionLost) setConnectionLost(false) } catch (caught) { @@ -149,7 +225,8 @@ export function useWorkspaceOpen({ }) } else if (message.includes('permitted to observe') || message.includes('no longer connected') || - message.includes('connect an account for every service')) { + message.includes('connect an account for every service') || + message.includes('while your access was being verified')) { showTerminalError({ kind: 'message', message }) } else { const failure = classifyWorkspaceOpenFailure(caught)