diff --git a/.agents/skills/write-gatekeeper/SKILL.md b/.agents/skills/write-gatekeeper/SKILL.md index 7416bd12d..0d8a0b49b 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`: because nobody can open the workspace without passing `addObserver()`, and with strategy A nobody ever does, the first such observation makes the workspace effectively unshareable — and it latches into a restricted mode that blocks all actions and public web fetches, so the data cannot leak back out through other gatekeepers. - **B — ACL check (single unit).** The binding is one atomic resource; sub-resources inherit its ACL. `addObserver()` calls a verifier method to confirm the observer can access it and throws otherwise; `removeObserver()` is a no-op; nothing is tracked and no `excludeObservers` is ever needed. Use for repo / document / page / team / single-project bindings. - **C — Data-set tracking.** The binding spans sub-resources with **distinct ACLs**, and there is a **per-observer access oracle** for each. The DO logs the data sets actually observed and the current observers; `addObserver()` verifies the observer against **every** logged set (plus a coarse membership baseline) and **stores their verifier**; each later observation that first touches a **new** set re-checks all stored observers and sets `excludeObservers` for any who fail. Use for workspace / organization / dataset-spanning bindings. - **D — Low-stakes.** `addObserver()` / `removeObserver()` are no-ops; `getVerifier()` returns a trivial verifier with a no-op public method such as `verify(): void {}` (an empty `WorkerEntrypoint` is not registered in `ctx.exports`). Use when any collaborator may observe (personal, low-stakes services). diff --git a/docs/observers.md b/docs/observers.md index d6c5534c9..da7cd720f 100644 --- a/docs/observers.md +++ b/docs/observers.md @@ -30,14 +30,18 @@ Gadgets enforce a core security invariant (see `overview.md` §"Security Model") > able to read that information will also be prohibited from interacting with the Gadget, > to prevent data leaks. -Today the only mechanism enforcing this is the blunt **`prohibitAllSharing`** flag -(`packages/workshop-shared/src/gatekeeper.ts`, `ObservationDescription.prohibitAllSharing`). -When a gatekeeper marks an observation as maximally sensitive, the Gadget can no longer be -shared with *anyone*, and it drops into "lockdown" (no further actions, no web fetches). This -is a deliberate stopgap — it cannot express "this data may be shared, but only with people who -*also* have access to it." - -This feature replaces that all-or-nothing posture with a per-user, gatekeeper-mediated check: +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. Its coverage rests on +admission: nobody can open the workspace without being verified against the producing gatekeeper, +and anything that widens what they must be verified against restarts every live session. The one +producer admission cannot see — one with no vendor account behind it — is refused outright while +the workspace is shared; see `#assertUnverifiableProducerUnshared` in `overseer.ts` and edge case +4 below.) + +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 +60,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 +72,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 +94,19 @@ 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`'s `#assertUnverifiableProducerUnshared`, `getWebFetchEnv`, `submitAction`) | +| Session restart when verification scope widens | `overseer.ts` (`#restartIfShared`, `scheduleAccessRestart`) | +| 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 +139,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 +172,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 +202,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 +232,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 @@ -275,10 +288,17 @@ Logic: **throws** on a mismatch. This server-side check is what guarantees a gatekeeper only receives a verifier minted by its own vendor; filtering account choices in the client is only a user-interface convenience. - - If any `addObserver` **throws** (or `getVerifier` throws on vendor mismatch), the user is not - (or no longer) allowed: best-effort `removeObserver(record.observerId)` on the gatekeepers - added in *this* pass, do **not** persist the working record, and deny the open with a clear - message. + - If any `addObserver` **throws** (or `getVerifier` throws on vendor mismatch, or returns null + for a disconnected account), the user is not (or no longer) allowed. Every such failure goes + through one `fail()` path that synchronously scrubs the failed gatekeeper from the + *persisted* record, so the record stops claiming a verification that no longer holds, and the + user is offered a bounded number of re-prompts to repair (e.g. re-authenticate an expired + account). On terminal failure the open is denied with a message naming each refused binding, + and the registrations added by this call -- plus those it invalidated and no later pass + re-verified -- are best-effort-removed while no record is persisted. A terminal failure that + scrubbed a previously-persisted choice also restarts the workspace (see + "Restarting when verification scope changes" below), because the collaborator may hold other + sessions that opened while that choice still verified them. 6. **Persist the observer record** (with merged `accountChoices` and `observerId`) only after all `addObserver` calls succeed. Storing/creating the record is the canonical moment the user @@ -301,12 +321,43 @@ Notes: the owner added after this user last configured, or an ambient binding without a matching provided account). +#### Restarting when verification scope changes + +Verification runs at `open()` and nowhere else, so a live session is only ever as verified as the +scope that existed when it opened. When that scope **widens**, the overseer restarts the workspace +rather than trying to re-verify sessions in place: `#restartIfShared(reason)` delegates to +`scheduleAccessRestart(reason)` — the same DO abort used to revoke a collaborator (see +`docs/sharing.md`) — so every client's browser reconnects and re-runs +`authorizeCollaborator`/`ensureObserver` against the new scope. It is a no-op when the workspace +has no collaborators: the owner is never an observer, so there is nobody to re-verify. + +Four events trigger it: + +| Event | What grows | +|---|---| +| `addGatekeeper()` with a vendor-backed `creationSpec` | **build** scope — a live `build` session can `getGatekeeperById()`/`openSession()` on it with no observer check | +| `bindWorkpiece()` for a permanent (non-`chatId`) edge onto a vendor-backed connection | **use** scope — the gadget UI a `use` session drives can now invoke it | +| A merge whose promotions bring an account-requiring connection into `#gadgetBoundGatekeeperIds()` | **use** scope, same reason. The trigger compares the effective scope before and after rather than firing on any promotion, since most merges promote something and most of what they promote widens nothing — a gadget with no bindings, or an edge onto a vendorless connection nobody is verified against | +| A terminal `ensureObserver()` failure that scrubbed a previously-persisted account choice | Coverage *shrank*: the collaborator's other sessions still hold access the scrubbed choice used to justify. Scheduled when the failure becomes terminal, which a re-prompt the failing client never answers can defer (edge case 3) | + +Shrinking scope needs no restart (`unbindWorkpiece`, `removeGatekeeper`): `ensureObserver`'s prune +handles it at the next open, and a narrower scope can never under-verify. Role *rises* +(`addCollaborator`, share-key redemption) are deliberately not triggers either — a live session's +capability is fixed at open, so raising someone's graph role does not widen the session they +already hold. + +Enforcement is therefore at admission, within the ~100 ms abort delay of the moment the change is +determined, and held to the collaborator's role scope (edge case 4 below). For the three widening +triggers that moment is the change itself; for the scrub trigger it is the point at which the +failure becomes terminal, which the failing collaborator can defer by leaving a re-prompt +unanswered — worth no more to them than never re-opening at all (edge case 3). + ### Step 4 — Frontend: the configuration modal 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 +373,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. @@ -357,16 +408,17 @@ methods wrapping `SharingManager` mutations (`removeCollaborator`, `revokeShareL downgrades — see the matching methods on `OverseerClientInterface` and `SharingManager`): - After a mutation, use the returned `AffectedCollaborator[]` to find users who **lost access**. - For each who is now unreachable, if they have an observer record: best-effort - `removeObserver(record.observerId)` on **all** gatekeeper facets, then delete the observer - record. + For each who is now unreachable, if they have an observer record: delete the observer record, + then best-effort `removeObserver(record.observerId)` on **all** gatekeeper facets. - For a **`build` → `use` downgrade**, optionally `removeObserver` (and drop the corresponding `accountChoices` entries) for the now-out-of-scope bindings (those without a `bindingName`). Safe to defer — an over-broad observer set only ever errs toward stricter future checks — but it keeps gatekeeper state tidy. - All these calls are best-effort: log and continue on error. An orphaned observer entry only - causes superfluous future checks, never a data leak (the leak-relevant gate is - `authorizeObservation`, which keys off the live sharing graph). + causes superfluous future checks, never a data leak: a registration is what *admits* an open, + and every open re-runs `addObserver`, so a stale one grants nothing on its own — while + `authorizeObservation`'s exclusion gate re-checks the live sharing graph for any id a gatekeeper + still names. A record is only ever persisted for a party who passed full verification. > Multi-gatekeeper sequencing/atomicity is an overseer implementation detail, not part of the > shared interface. Because `addObserver` is re-run every open and `removeObserver` is idempotent, @@ -401,12 +453,77 @@ 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, so the record stops claiming a verification that no longer holds. + Because the collaborator may hold *other* sessions that opened while it did, a scrub also + restarts the workspace when the denial is determined (see "Restarting when verification scope + changes"), which forces every session on it to re-open and re-verify; whoever cannot is denied + at that open. "When determined" is later than the scrub itself: the failing client is offered a + re-prompt first, and one it never answers defers the restart for as long as it stays unanswered + — the same residual as never re-opening, below. + The gatekeeper-side registration is kept on a re-verification failure — de-registering it would be + fail-open, since the gatekeeper would stop naming that observer in `excludeObservers` and an + observation it would have excluded them from would be admitted with nothing left to block it, + while keeping it can only add exclusion names; the next successful open's `addObserver` + overwrites its verifier. Only a *first-ever* verification failure rolls its registrations back, + since that collaborator was never admitted and the minted id would otherwise linger + unresolvable. The residual under the lazy model is unchanged: a collaborator who never opens + again is never asked, so nothing detects their revocation and nothing severs the session they + already hold. + An operational failure (vendor outage, expired credential) is treated the same way — the + overseer cannot tell it from a settled denial, so it scrubs and restarts too, and the + collaborator gets back in as soon as a repaired open re-verifies them. +4. **`containsRestrictedData` interaction** — coverage is enforced at *admission*, not per + observation: `ensureObserver` re-verifies each collaborator against every in-scope gatekeeper + at every `open()`, so nobody can be in the workspace without having passed the producing + gatekeeper's `addObserver()`, and anything that widens what they must pass restarts every live + session (see "Restarting when verification scope changes"). The flag also latches the workspace + into a restricted mode that blocks actions and web fetches. + One producer admission structurally cannot cover: one with no vendor account behind it — an + `aiModel`/`agentSpawner` binding, or a legacy record with no `creationSpec`. + `#inScopeGatekeepers` skips those, so no collaborator is ever asked about them, and + `#assertUnverifiableProducerUnshared` in `authorizeObservation` therefore refuses their + restricted observations outright while the workspace is shared. (This matches + `assertNewSharingAllowed()`, which already treats the same case as unshareable, and the message + names no collaborator: it reaches sandboxed gadget code and agent output.) + Verification is also held to each collaborator's own role scope, because `ensureObserver` can + never verify beyond it: a `use` collaborator can't be covered for a gatekeeper no gadget binds. + `use` scope is *live* binding state, with a transition case in each direction. Adding a + binding grows it, which restarts every session (edge case 5). Unbinding shrinks it silently: + 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 could not open the workspace; (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. Stale coverage does not ride across the unbind/rebind: + `ensureObserver` prunes out-of-scope entries from the observer record at every open, so a + `use` collaborator who opened only during the unbound window (verifying nothing against the + producer) holds no entry for it, and the rebind restarts the workspace, so their forced + re-open asks them about the producer again rather than re-registering them off the choice they + made before it was unbound. + 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). Because that next open is what verifies them, the + addition restarts the workspace on a shared workspace (see "Restarting when verification scope + changes"): every client reconnects within ~100 ms and re-opens at the new scope, so no session + keeps watching a connection its holder was never verified against. A connection added *while a + collaborator's verification is parked* on an await (the modal, verifier RPCs) is covered by the + same restart: their committed record lacks an entry for the new connection, and the restart + forces the open that adds one. The residual is the ~100 ms window itself, which is inside the + revocation window the sharing model already accepts. 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 +531,30 @@ 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 for the + *producer* connections — those through which restricted data was actually read, derived from + the permanent action log (`restrictedProducerIds`); non-producers stay removable. The record + is what observer verification runs against, and the restricted data outlives it in chat + history and storage, so deleting it would let a never-verified collaborator open unchecked. + Outstanding share links block removal the same way: their keys never expire, and redemption + is gated at open() only while the record exists. The remedy is to remove collaborators and + revoke share links first. + Unverifiable producers (a legacy record with no creation spec, or an aiModel/agentSpawner + backed by no vendor account) are guarded the same way: a legacy record hard-denies every + non-owner open while it exists, so removing it while shared would readmit every existing + collaborator with the restricted history still in chat. It cannot be migrated (it never + persisted the vendor identity), so the recovery for an owner who wants to share such a + workspace is to start a new one. Internal removals need no guard: the creation-failure + rollback removes a record too new to be a producer, and ambient reconciliation skips — and + logs — a stale record the guard protects. + The complementary rule (`assertNewSharingAllowed`): once latched, if any producer is gone or + can never verify a collaborator, everything that would admit a new party refuses — the + grant-creating mutators (`addCollaborator`, `createShareLink`, `newShareLinkKey`) and + `redeemShareKey` at open() — leaving the workspace permanently owner-only. Each check runs + synchronously with its storage write, so a producer removed in any await window still refuses + the grant; a redemption whose edge already exists skips the check, so an existing + collaborator's re-open is untouched. --- @@ -428,6 +569,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 +618,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 +637,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 +667,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. | @@ -553,3 +700,16 @@ This is why the broad bindings split the way they do: - **Decomposition deliberately deferred → A:** Gmail Mailbox — could in principle decompose into mailing lists the observer belongs to, but that is the out-of-scope "advanced" case, so it stays fully private for now. + +--- + +## Known limitations + +Revocations and role changes take effect within seconds -- the revocation restart lands in +~100ms -- and read-side races inside that envelope are accepted by design: a guard earns its +place here only if its failure mode is *persistent* wrong state that outlives the window. + +The observer-side deferrals are ledgered in `plans/restricted-data-sharing.md`, each marked at +its site in the code by a matching `TODO` comment: observer verification is not serialized per +profile, and the exclusion gate's teardown deletes from a snapshot that can go stale across its +awaited fan-out. diff --git a/docs/sharing.md b/docs/sharing.md index 8bb4fc467..d0c702ba9 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 **one-step**: redeeming a key writes the recipient's `shareKey` edge immediately, making them a collaborator like any other before the redeeming open()'s observer verification runs. Redemption is policy-gated like every grant-creating mutator (`assertNewSharingAllowed` runs synchronously with the write; a re-redemption whose edge already exists is a no-op that skips the gate). A recipient whose verification then fails keeps the edge -- see Known limitations. + ### 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,20 @@ 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): +Inputs (all optional; used to model a hypothetical change): - `removedUser` -- a profile ID to treat as removed (excluded from the graph). - `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. 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. 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 +150,19 @@ 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. -### Terminating live sessions on revocation +A share-key redemption goes through the same gate: the redemption is a grant like any other, policy-gated by `assertNewSharingAllowed` synchronously with the edge write. The redeeming open() then verifies the recipient as an observer like any other collaborator; a recipient whose verification fails persists as an unverified collaborator until removed (see Known limitations). + +### Terminating live sessions on revocation or scope growth 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.scheduleAccessRestart`): 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 abort also lands later than the ~100ms delay alone suggests: the revocation handlers first await the observer teardown (`tearDownLostObservers`, a per-collaborator `removeObserver` fan-out) and the listing refresh (`refreshAffectedCollaboratorListings`, chunked cross-DO round trips), so the removed users' sessions stay live and watching for a window that scales with collaborator and gatekeeper count. -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. +Granting or raising access never strands anyone: a live session's capability is fixed at open, so a `use` collaborator promoted to `build` in the graph still holds `UseOverseerInterface` until they re-open, and nobody is newly excluded from anything. + +The same abort serves a second purpose, though, and there the trigger is a *grant*: observer verification also runs only at `open()`, so widening the set of gatekeepers a collaborator must be verified against leaves their live session holding access they were never verified for. `OverseerImpl.#restartIfShared` restarts the workspace whenever that happens -- a connection is added, one is bound into a gadget, a merge promotes such a binding, or a re-verification failure scrubs a previously-persisted account choice -- so every client re-opens and re-runs `ensureObserver` at the new scope. It is a no-op when the workspace has no collaborators, so a solo workspace is never disturbed. See docs/observers.md, "Restarting when verification scope changes", for the full trigger list and the reasoning about what deliberately does *not* trigger it. ## Future work @@ -167,3 +173,22 @@ Note this is only needed for removals/downgrades. Granting or raising access nev - **Un-revoking share links.** Revocation is non-destructive (the `revoked` flag), but there is no UI or RPC to list revoked links or clear the flag, so link revocation is currently one-way in practice. - **Garbage-collecting dead records.** Removed collaborators and revoked links accumulate in storage under the lazy model; a background sweep could reclaim entries that have been unreachable for a long time. - **Notifications.** Currently there are no in-product notifications for access grants or revocations. + +## Known limitations + +Revocations and role changes take effect within seconds -- the revocation restart lands in +~100ms -- and read-side races inside that envelope are accepted by design; only guards against +*persistent* wrong state remain. Each item below is marked at its site in the code by a matching +`TODO` comment. + +- **An unverified redeemer persists as a collaborator.** Redemption writes a real edge before the + redeeming open's observer verification runs, so from the moment a recipient clicks the link they + appear in `listCollaborators` whether or not they ever complete the open. They cannot reach the + workspace -- verification denies them at open -- but the workspace counts as *shared* for the + checks that ask only whether anyone else is on it: removing a restricted producer is blocked, and + an unverifiable producer's restricted reads are refused. Remedies: they verify (complete the + open), the owner removes them, or the link is revoked. Two-phase redemption (a pending edge + granting nothing until verification confirms it) is the planned fix. +- **A refused recipient persists.** A recipient whose verification is refused keeps their edge: + they appear in `listCollaborators` until the owner removes them (or revokes the link), with the + same consequences as the previous item, and covered by the same planned fix. diff --git a/packages/gatekeeper-google/__tests__/drive-session.test.ts b/packages/gatekeeper-google/__tests__/drive-session.test.ts index 2d0933809..934ccd902 100644 --- a/packages/gatekeeper-google/__tests__/drive-session.test.ts +++ b/packages/gatekeeper-google/__tests__/drive-session.test.ts @@ -121,7 +121,7 @@ describe("Drive session scope", () => { description: expect.stringContaining('name starts with "missing"'), excludeObservers: ["excluded"], })]); - expect(authorizations[0]).not.toHaveProperty("prohibitAllSharing"); + expect(authorizations[0]).not.toHaveProperty("containsRestrictedData"); expect(authorizations[0].description).not.toContain("0"); expect(events).toEqual(["authorize"]); }); diff --git a/packages/gatekeeper-google/src/google.ts b/packages/gatekeeper-google/src/google.ts index 7acef83f6..603675905 100644 --- a/packages/gatekeeper-google/src/google.ts +++ b/packages/gatekeeper-google/src/google.ts @@ -1735,7 +1735,7 @@ export class GmailGatekeeperImpl extends DurableObject): Promise { @@ -3396,7 +3396,7 @@ class BigQuerySessionImpl extends RpcTarget implements BigQuerySession { `Referenced tables: ${estimate.referencedTables.join(", ")}\n` + `Estimated bytes processed: ${estimate.bytesProcessed.toLocaleString()}\n` + `Maximum bytes billed: ${maxBytes.toLocaleString()}.`, - prohibitAllSharing: true, + containsRestrictedData: true, }); let result = await this.#api.query(billingProject, sql, { @@ -3428,7 +3428,7 @@ class BigQuerySessionImpl extends RpcTarget implements BigQuerySession { description: `Estimated bytes processed: ${estimate.bytesProcessed.toLocaleString()}\n` + `Referenced tables: ${estimate.referencedTables.join(", ") || "(none)"}`, - prohibitAllSharing: true, + containsRestrictedData: true, }); return estimate; @@ -3440,7 +3440,7 @@ class BigQuerySessionImpl extends RpcTarget implements BigQuerySession { await this.#authorizeDatasets([], { title: "Get BigQuery project", description: `Returned the scoped project: \`${this.#scopedProjectId}\`.`, - prohibitAllSharing: true, + containsRestrictedData: true, }); return result; } @@ -3461,7 +3461,7 @@ class BigQuerySessionImpl extends RpcTarget implements BigQuerySession { await this.#authorizeDatasets([{ projectId: p, datasetId: this.#scopedDatasetId }], { title: `List datasets in ${p}`, description: `Returned scoped dataset \`${p}.${this.#scopedDatasetId}\` (1 dataset).`, - prohibitAllSharing: true, + containsRestrictedData: true, }); return [dataset]; } @@ -3471,7 +3471,7 @@ class BigQuerySessionImpl extends RpcTarget implements BigQuerySession { await this.#authorizeDatasets(result.map(ds => ({ projectId: p, datasetId: ds.datasetId })), { title: `List datasets in ${p}`, description: `Listed ${result.length} dataset(s) in \`${p}\`.`, - prohibitAllSharing: true, + containsRestrictedData: true, }); return result; } @@ -3497,7 +3497,7 @@ class BigQuerySessionImpl extends RpcTarget implements BigQuerySession { await this.#authorizeDatasets([{ projectId: p, datasetId: d }], { title: `List tables in ${p}.${d}`, description: `Returned scoped table \`${p}.${d}.${this.#scopedTableId}\` (1 table).`, - prohibitAllSharing: true, + containsRestrictedData: true, }); return [table]; } @@ -3506,7 +3506,7 @@ class BigQuerySessionImpl extends RpcTarget implements BigQuerySession { await this.#authorizeDatasets([{ projectId: p, datasetId: d }], { title: `List tables in ${p}.${d}`, description: `Listed ${result.length} table(s) in \`${p}.${d}\`.`, - prohibitAllSharing: true, + containsRestrictedData: true, }); return result; } @@ -3543,7 +3543,7 @@ class BigQuerySessionImpl extends RpcTarget implements BigQuerySession { title: `Describe ${p}.${d}.${t}`, description: `Described table \`${p}.${d}.${t}\` (${result.schema.length} columns).`, - prohibitAllSharing: true, + containsRestrictedData: true, }); return result; } diff --git a/packages/gatekeeper-mcp/README.md b/packages/gatekeeper-mcp/README.md index 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 diff --git a/packages/integration-tests/__tests__/external-message-verification.test.ts b/packages/integration-tests/__tests__/external-message-verification.test.ts new file mode 100644 index 000000000..66aa3eafa --- /dev/null +++ b/packages/integration-tests/__tests__/external-message-verification.test.ts @@ -0,0 +1,205 @@ +// Tests for the external-message authorization gate (authorizeCollaborator in overseer.ts): +// receiveExternalMessage() must hold a collaborator to the same observer verification open() +// applies -- non-interactively, since this path has no way to prompt for account configuration -- +// and must deny an insufficient role *before* verification runs. +// +// These live in their own file -- with their own harness, like every suite here -- so the suite +// stays self-contained as the observer suites around it grow. + +import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import type { RpcStub } from "capnweb"; +import type { AuthenticatedApi, 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, MAX_OBSERVER_PROMPTS, nextUsernames, + ObserverConfigRecorder, signUp, stubFor, waitFor, type ConnectedAccount, +} from "../src/rpc-client.js"; +import { NetworkInterceptor } from "../src/network-interceptor.js"; + +// Reason text shaped like what a gatekeeper actually reports on a settled denial. Its appearance +// in the gateway's reply below is what proves a live verification round trip happened. +const DENIED_REASON = "You do not have access to this thing."; + +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; + }); +} + +/** + * 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; +} + +/** Tell the gatekeeper what to do the next time it's asked to admit `label` as an observer. */ +async function setVerifyOutcome( + label: string, outcome: { allow: true } | { allow: false; reason: string }): Promise { + const res = await harness.fetchWorker( + TEST_GATEKEEPER_WORKER, "http://gatekeeper-test.test/control/verify-outcome", + { method: "POST", body: JSON.stringify({ label, ...outcome }) }); + if (res.status !== 204) { + throw new Error(`Setting the verify outcome failed with ${res.status}: ${await res.text()}`); + } +} + +/** 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("external-message verification", () => { + 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: connect a Thing (an account-requiring connection, + // so collaborators must be observer-verified against it) and 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 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 -- and this + // path has no configuration channel to fix that. The agent's reply could surface anything + // the workspace has 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. + const callback = stubFor( + new ObserverConfigRecorder().alwaysChoose(bobAccount.id, MAX_OBSERVER_PROMPTS)); + try { + (await bobApi.openGadget(gadgetId, undefined, callback))[Symbol.dispose](); + } finally { + callback[Symbol.dispose](); + } + await expect(submitExternalMessage({ callerEmail: bob, gadgetKey, prompt: "hi" })) + .resolves.toMatchObject({ + accepted: false, message: expect.stringMatching(/AI model/i) }); + + // The gatekeeper now revokes Bob's underlying access. His persisted observer record is + // untouched, so only a live addObserver re-verification on this submission can notice -- + // and the gatekeeper's own refusal reason appearing in the reply is the proof that round + // trip happened, since nothing persisted in the Workshop contains it. An implementation + // that merely checked the record would keep accepting him here. + await setVerifyOutcome(accountLabel(bobAccount), { allow: false, reason: DENIED_REASON }); + const revoked = await submitExternalMessage({ callerEmail: bob, gadgetKey, prompt: "hi" }); + if (revoked.accepted) throw new Error("The revoked submission was accepted"); + expect(revoked.message).toMatch(/could not be verified/i); + expect(revoked.message).toContain(DENIED_REASON); + }); + }); + + 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); + if (!await overseer.addCollaborator(dave, "use")) { + throw new Error(`Failed to share the gadget with ${dave}`); + } + 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__/observer-reverification.test.ts b/packages/integration-tests/__tests__/observer-reverification.test.ts index 09b8b5d4a..57941325b 100644 --- a/packages/integration-tests/__tests__/observer-reverification.test.ts +++ b/packages/integration-tests/__tests__/observer-reverification.test.ts @@ -172,6 +172,15 @@ async function bobOpens( } } +// A denied re-verification scrubs the account choice it just failed against, so the overseer +// severs every session on the workspace -- Bob may hold others that opened while that choice still +// verified him. The sever is a ctx.abort() ~100ms after the open rejects, i.e. after the test body +// has returned. Wait it out before withSession() drops the connection: an abort that lands with no +// client left on the workspace crashes the local workerd, and these tests share one harness, so the +// crash fails whichever siblings are mid-flight rather than this test. +const RESTART_SETTLE_MS = 400; +const settleRestart = () => new Promise(resolve => setTimeout(resolve, RESTART_SETTLE_MS)); + /** Open once and answer the prompt, which is what persists Bob's account choice. */ async function bobOpensAndCloses(shared: SharedGadget): Promise { const recorder = @@ -272,6 +281,7 @@ describe("observer re-verification", () => { expect(need.failure).toBeDefined(); expect(need.failure!.accountId).toBe(shared.bobAccount.id); expect(need.failure!.reason).toContain(EXPIRED_REASON); + await settleRestart(); }); }); @@ -295,6 +305,7 @@ describe("observer re-verification", () => { expect(error!.message).toContain(EXPIRED_REASON); // One line per failed binding, so a single failure must not introduce stray newlines. expect(error!.message.split("\n").filter(l => l.includes(shared.bobLabel))).toHaveLength(1); + await settleRestart(); }); }); @@ -326,6 +337,7 @@ describe("observer re-verification", () => { expect(error!.message).toContain("Test Thing multi-a"); expect(error!.message).toContain("Test Thing multi-b"); expect(error!.message.split("\n").filter(l => l.includes(shared.bobLabel))).toHaveLength(2); + await settleRestart(); }); }); @@ -362,6 +374,7 @@ describe("observer re-verification", () => { expect(events.filter(e => e.type === "remove")).toEqual([]); // Both successful verifications registered it: the first open and the pass-2 repair. expect(events.filter(e => e.type === "add")).toHaveLength(2); + await settleRestart(); }); }); @@ -382,6 +395,7 @@ describe("observer re-verification", () => { expect(error).not.toBeNull(); expect(error!.message).toMatch(/could not confirm/i); expect(error!.message).toContain(DENIED_REASON); + await settleRestart(); }); }); }); 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..b6179c25f --- /dev/null +++ b/packages/integration-tests/__tests__/observer-role-scope.test.ts @@ -0,0 +1,182 @@ +// Tests for role-scoped observer enforcement: a collaborator is held 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). So binding a connection into +// a gadget widens every "use" collaborator's scope, and since sessions are verified only at open(), +// that widening restarts the workspace: each client's next open re-verifies against the new scope. +// The external-message gate's role scoping is covered by external-message-verification.test.ts. +// +// This lives in its own file -- with its own harness, like every suite here -- rather than in +// sensitive-observations.test.ts, because both suites restart the workspace, and a DO abort makes +// the shared local harness briefly drop unrelated in-flight requests; their concurrent tests pass +// with their current timing, but growing either 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 { + startTestGatekeeperHarness, TEST_VENDOR_ID, type Harness, +} from "../src/harness.js"; +import type { TestSession } from "../fixtures/gatekeeper-test/src/test-gatekeeper.js"; +import { + 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; + }); +} + +type Workspace = { + gadgetId: string; + overseer: RpcStub; + alice: string; + aliceApi: RpcStub; + /** The fixture session bound to the workspace's (first) gatekeeper. */ + session: RpcStub; + 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() as RpcStub; + const { id: gadgetId } = await overseer.getMetadata(); + return { gadgetId, overseer, alice, aliceApi, session, gatekeeperId }; +} + +// The owner's own reconnect after a restart, on a fresh connection: the abort fells every client of +// the workspace, so `ws`'s stubs -- and the whole session they came from -- are dead afterwards. +async function reopenAfterRestart(ws: Workspace): Promise<{ + publicApi: RpcStub; + session: RpcStub; +}> { + await waitFor("the restart to fell the old workspace instance", () => + ws.session.readThing().then(() => null, () => true)); + + return waitFor("the workspace to come back after the restart", async () => { + const publicApi = connect(harness.url); + try { + const aliceApi = await logIn(publicApi, ws.alice); + const overseer = await aliceApi.openGadget(ws.gadgetId); + const gatekeeper = await overseer.getGatekeeperById(ws.gatekeeperId); + const session = await gatekeeper.openSession() as RpcStub; + // Probe with a benign read, so a session felled by the abort retries here rather than + // failing an assertion below. + await session.readThing(); + return { publicApi, session }; + } catch { + publicApi[Symbol.dispose](); + return null; + } + }); +} + +// Carol's forced re-open, on the fresh connection her browser would reconnect with. +async function carolReopens( + ws: Workspace, carol: string, recorder: ObserverConfigRecorder): Promise { + const publicApi = connect(harness.url); + try { + const carolApi = await logIn(publicApi, carol); + const callback = stubFor(recorder); + try { + (await carolApi.openGadget(ws.gadgetId, undefined, callback))[Symbol.dispose](); + } finally { + callback[Symbol.dispose](); + } + } finally { + publicApi[Symbol.dispose](); + } +} + +describe("role-scoped observer enforcement", () => { + it.concurrent("a use collaborator is verified only against 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](); + } + + // Carol holds no coverage for the connection and never will while it stays unbound, but that + // is enforced against her open, not against the owner's reads: this restricted read goes + // through. + 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. That widens what Carol's live session must be verified against, and a + // live session is never re-verified in place -- so the workspace restarts instead. + using gadget = await ws.overseer.createGadget("Test Gadget", undefined, "TEST_GADGET"); + await gadget.bind("TEST_THING", ws.gatekeeperId); + + const reopened = await reopenAfterRestart(ws); + try { + // The owner's restricted read is undisturbed by the widening: nothing about Carol's + // coverage gates it. + await expect(reopened.session.readThing(true)).resolves.toContain("use-scope"); + + // Carol's forced re-open is where the newly in-scope connection gets verified, and she is + // asked about exactly it -- the one connection her role's scope just gained. + const recorder = + new ObserverConfigRecorder().alwaysChoose(carolAccount.id, MAX_OBSERVER_PROMPTS); + await carolReopens(ws, carol, recorder); + expect(recorder.callCount).toBe(1); + expect(recorder.calls[0].map(need => need.gatekeeperId)).toEqual([ws.gatekeeperId]); + } finally { + reopened.publicApi[Symbol.dispose](); + } + }); + }); +}); 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..aed892342 --- /dev/null +++ b/packages/integration-tests/__tests__/sensitive-observations.test.ts @@ -0,0 +1,632 @@ +// Tests for the sensitive-data (`containsRestrictedData`) observation policy. +// +// Coverage is enforced at admission, not at the read: every collaborator passes the producing +// gatekeeper's `addObserver` at their most recent open and cannot open without passing it, and +// anything that widens what they must pass restarts the workspace so every live session re-opens +// against the new scope. So sensitive observations are not blocked by an unverified collaborator, +// and sharing stays available. 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, Overseer, PublicApi, +} from "@gadgets/workshop-shared/api"; +import { + startTestGatekeeperHarness, TEST_GATEKEEPER_WORKER, TEST_VENDOR_ID, type Harness, +} from "../src/harness.js"; +import type { TestSession } from "../fixtures/gatekeeper-test/src/test-gatekeeper.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: RpcStub; + 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() as RpcStub; + const { id: gadgetId } = await overseer.getMetadata(); + return { gadgetId, overseer, alice, aliceApi, session, gatekeeperId }; +} + +type Bob = { + bob: string; + bobProfileId: string; + bobApi: RpcStub; + bobAccount: ConnectedAccount; + bobLabel: string; +}; + +// Sign Bob up, add him as a collaborator, and give him his own fixture account. +async function addBob(publicApi: RpcStub, ws: Workspace): Promise { + 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. Pass a `recorder` to +// assert *which* connections the open asked him about. +async function bobOpens(gadgetId: string, bobApi: RpcStub, + bobAccount: ConnectedAccount, + recorder?: ObserverConfigRecorder): Promise> { + const callback = stubFor( + recorder ?? new ObserverConfigRecorder().alwaysChoose(bobAccount.id, MAX_OBSERVER_PROMPTS)); + try { + return await bobApi.openGadget(gadgetId, undefined, callback); + } finally { + callback[Symbol.dispose](); + } +} + +// Wait out a restart and come back on a fresh connection, returning the owner's re-opened +// workspace and a session on `gatekeeperId`. +// +// A restart aborts the DO shortly after the triggering call returns, killing every stub from the +// connection that made it. A probe on a fresh connection can only detect a DO that is *already* +// dead -- never one about to die -- so a reopen attempted inside the pre-abort window can fully +// succeed against the doomed instance and then lose its session under the assertions that follow. +// Hence two steps: watch the pre-restart session die, then reopen with retries. +async function reopenAfterRestart(ws: Workspace, gatekeeperId = ws.gatekeeperId): Promise<{ + publicApi: RpcStub; + overseer: RpcStub; + session: RpcStub; +}> { + await waitFor("the restart to fell the old workspace instance", () => + ws.session.readThing().then(() => null, () => true)); + + return waitFor("the workspace to come back after the restart", async () => { + const publicApi = connect(harness.url); + try { + const aliceApi = await logIn(publicApi, ws.alice); + const overseer = await aliceApi.openGadget(ws.gadgetId); + const gatekeeper = await overseer.getGatekeeperById(gatekeeperId); + const session = await gatekeeper.openSession() as RpcStub; + // Probe with a benign read, so a session felled by the abort retries here rather than + // failing an assertion below. + await session.readThing(); + return { publicApi, overseer, session }; + } catch { + publicApi[Symbol.dispose](); + return null; + } + }); +} + +// Bob's forced re-open, on the fresh connection his browser would reconnect with. The restart +// killed the whole session his `bobApi` came from -- every client of the workspace loses its +// connection, not just its workspace stubs -- so reusing it here would fail on a dead socket +// rather than exercising the re-verification this asserts. +async function bobReopens( + ws: Workspace, bob: Bob, recorder: ObserverConfigRecorder): Promise { + const publicApi = connect(harness.url); + try { + const bobApi = await logIn(publicApi, bob.bob); + (await bobOpens(ws.gadgetId, bobApi, bob.bobAccount, recorder))[Symbol.dispose](); + } finally { + publicApi[Symbol.dispose](); + } +} + +describe("sensitive observations", () => { + it.concurrent("latch restricted mode: actions are blocked and metadata reports it", async () => { + await withSession(async publicApi => { + const ws = await newWorkspace(publicApi, "latch"); + + // Before the latch, actions submit fine and metadata is clean. + await expect(ws.session.doThing()).resolves.toBeUndefined(); + expect((await ws.overseer.getMetadata()).containsRestrictedData).toBeFalsy(); + + await expect(ws.session.readThing(true)).resolves.toContain("latch"); + + expect((await ws.overseer.getMetadata()).containsRestrictedData).toBe(true); + await expect(ws.session.doThing()).rejects.toThrow(/prohibited from performing actions/i); + // Reads -- sensitive or not -- keep working. + await expect(ws.session.readThing()).resolves.toContain("latch"); + await expect(ws.session.readThing(true)).resolves.toContain("latch"); + }); + }); + + it.concurrent("an unredeemed share link does not block a sensitive observation", async () => { + await withSession(async publicApi => { + const ws = await newWorkspace(publicApi, "unredeemed"); + await ws.overseer.createShareLink("build", "never redeemed"); + + // An outstanding link grants nobody anything until it is redeemed, and redemption happens + // inside open() -- where verification runs -- so the observation proceeds. + 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 does not block a sensitive observation", async () => { + await withSession(async publicApi => { + const ws = await newWorkspace(publicApi, "unverified"); + const bob = await addBob(publicApi, ws); + + // Bob has access but has never opened, so he holds no observer record for this gatekeeper + // -- and no session either, because verification is a precondition of getting one. There is + // nothing for the read to fail closed against. + await expect(ws.session.readThing(true)).resolves.toContain("unverified"); + + // Admission is where the coverage requirement bites: the gatekeeper refuses him, so his + // open is denied and he never reaches the workspace, let alone the observation. + await setVerifyOutcome(bob.bobLabel, { allow: false, reason: "You do not have access." }); + await expect(bobOpens(ws.gadgetId, bob.bobApi, bob.bobAccount)) + .rejects.toThrow(/could not confirm/i); + + // His refusal costs the owner nothing -- a first-ever failure scrubs no coverage, so + // nothing was severed and reads keep flowing. + await expect(ws.session.readThing(true)).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("adding a connection restarts the workspace so collaborators re-verify", + 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. It is in his verification scope + // the moment it exists -- a "build" session can open a session on it with no observer check + // -- and his live session was admitted without it, so adding it severs every session. + 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 lateId = await late.getId(); + + const reopened = await reopenAfterRestart(ws, lateId); + try { + // Nothing is blocked: the owner reads restricted data through the new connection... + await expect(reopened.session.readThing(true)).resolves.toContain("late"); + + // ...and Bob's forced re-open is where it gets verified. He is asked about exactly it, + // since his coverage for the connections that predate it survived. + const recorder = new ObserverConfigRecorder() + .alwaysChoose(bob.bobAccount.id, MAX_OBSERVER_PROMPTS); + await bobReopens(ws, bob, recorder); + expect(recorder.callCount).toBe(1); + expect(recorder.calls[0].map(need => need.gatekeeperId)).toEqual([lateId]); + } finally { + reopened.publicApi[Symbol.dispose](); + } + }); + }); + + 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. + // Added before Bob, so it widens nobody's scope and restarts nothing. + 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 = await second.openSession() as RpcStub; + + // Bob verifies against both producers. + 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 because the failure shrank what his record claims, it severs the sessions that + // claim admitted -- including the one he opened while it still covered that producer. + const reopened = await reopenAfterRestart(ws); + try { + // The owner's reads keep flowing throughout: Bob cannot be admitted again without + // re-verifying, which is the whole of the enforcement. + await expect(reopened.session.readThing(true)).resolves.toContain("scrub"); + + // A repaired re-open asks him about exactly the producer that refused: the scrub took + // his coverage for that one and left the other intact. + await setVerifyOutcome(bob.bobLabel, { allow: true }, thingUrl("scrub")); + const recorder = new ObserverConfigRecorder() + .alwaysChoose(bob.bobAccount.id, MAX_OBSERVER_PROMPTS); + await bobReopens(ws, bob, recorder); + expect(recorder.callCount).toBe(1); + expect(recorder.calls[0].map(need => need.gatekeeperId)).toEqual([ws.gatekeeperId]); + } finally { + reopened.publicApi[Symbol.dispose](); + } + }); + }); + + it.concurrent("a refused share-link recipient persists as a collaborator without blocking 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 -- writing a real edge -- and observer verification then + // refuses him. One-step redemption accepts the residue: he persists as an unverified + // collaborator (see the TODO on redeemShareKey). + 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](); + } + + // The residue is a collaborator row, not access: he never opened, and he cannot open + // without passing the same check. So the owner's reads are untouched -- and nothing was + // severed either, since a first-ever failure has no persisted coverage to scrub. + const collaborators = await ws.overseer.listCollaborators(); + expect(collaborators).toHaveLength(1); + await expect(ws.session.readThing(true)).resolves.toContain("refused-link"); + }); + }); + + 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 edges deduplicate, so neither open 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); + }); + }); + + 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 restarts the workspace 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 -- including the session Bob holds, + // which is the point. Everything past here runs on a fresh connection. + await ws.overseer.removeCollaborator(bob.bobProfileId, []); + const reopened = await reopenAfterRestart(ws); + + try { + // Bob's collaborator record lingers in storage (lazy revocation), and the owner's reads + // are unaffected either way. + 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: his next open has to name an account for the producer and pass + // addObserver again. + await reopened.overseer.addCollaborator(bob.bob, "build"); + const recorder = new ObserverConfigRecorder() + .alwaysChoose(bob.bobAccount.id, MAX_OBSERVER_PROMPTS); + await bobReopens(ws, bob, recorder); + expect(recorder.calls[0].map(need => need.gatekeeperId)).toContain(ws.gatekeeperId); + } finally { + reopened.publicApi[Symbol.dispose](); + } + }); + }); + + it.concurrent("ambient reconciliation preserves a shared restricted producer", async () => { + await withSession(async publicApi => { + const ws = await newWorkspace(publicApi, "ambient-reconcile"); + + // Ambient capsule records aren't published to clients, but their workpiece ids are small + // sequential integers, so probe for the one ensureAmbientCapsules provisioned at first open. + const findAmbientIds = async (overseer: RpcStub) => { + const found: number[] = []; + for (let id = 0; id < ws.gatekeeperId + 4; id++) { + try { + const gatekeeper = await overseer.getGatekeeperById(id); + if ((await gatekeeper.getTitle()) === "Test Ambient") found.push(id); + } catch { + // Not a gatekeeper workpiece. + } + } + return found; + }; + const [ambientId] = await findAmbientIds(ws.overseer); + expect(ambientId).toBeDefined(); + + // Latch through the ambient capsule, so it -- not the pasted connection -- is the producer. + const ambient = await ws.overseer.getGatekeeperById(ambientId); + const ambientSession = await ambient.openSession() as RpcStub; + await expect(ambientSession.readThing(true)).resolves.toContain("Test Ambient"); + await addBob(publicApi, ws); + + // Replace the owner's singleton account: disconnecting and re-provisioning mints a new + // accountId, so the existing capsule record is stale at the next reconcile. + const accounts = await listConnectedAccounts(ws.aliceApi); + const oldAccount = accounts.find(a => a.vendorId === TEST_VENDOR_ID)!; + await ws.aliceApi.disconnectAccount(oldAccount.id); + const newAccount = await provisionAccount(ws.aliceApi); + expect(newAccount.id).not.toBe(oldAccount.id); + + // Reopen. Later opens run the capsule reconcile in the background, and provisioning the + // replacement is itself a connection Bob has never been verified against -- so the + // reconcile restarts the workspace out from under this connection. Come back on a fresh + // one, then wait for the replacement's record to appear (proof the reconcile has run). + (await ws.aliceApi.openGadget(ws.gadgetId))[Symbol.dispose](); + const reopened = await reopenAfterRestart(ws); + try { + const ids = await waitFor("the replacement ambient capsule to be provisioned", async () => { + const found = await findAmbientIds(reopened.overseer); + return found.some(id => id !== ambientId) ? found : null; + }); + + // The stale record anchors Bob's verification, so the reconcile must have skipped it: + // the record survives, and sharing -- which refuses once any producer's record is gone -- + // still works. + expect(ids).toContain(ambientId); + await expect(reopened.overseer.createShareLink("build", "still shareable")) + .resolves.toMatchObject({ key: expect.any(String) }); + } finally { + reopened.publicApi[Symbol.dispose](); + } + }); + }); +}); 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 db72750eb..d054a42b8 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. @@ -243,8 +246,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 restricted-mode latch and the + * unverifiable-producer guard), 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 { @@ -277,8 +318,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; + } } /** @@ -338,8 +386,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; @@ -392,6 +448,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 b88b0c012..37934819a 100644 --- a/packages/integration-tests/src/rpc-client.ts +++ b/packages/integration-tests/src/rpc-client.ts @@ -79,6 +79,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; diff --git a/packages/typed-storage/__tests__/index.test.ts b/packages/typed-storage/__tests__/index.test.ts index 78d816dd3..fb98d81f0 100644 --- a/packages/typed-storage/__tests__/index.test.ts +++ b/packages/typed-storage/__tests__/index.test.ts @@ -1,5 +1,6 @@ import { expect, it, describe } from "vitest" -import { createTypedStorage, collection, UniqueIndex, NonUniqueIndex } from "../src/index.js"; +import { createTypedStorage, collection, singleton, UniqueIndex, NonUniqueIndex } + from "../src/index.js"; import { DurableObjectListOptions, DurableObjectStorage } from "@cloudflare/workers-types/experimental"; // We mock out DurableObjectStorage becaues otherwise we'd have to run the tests inside a @@ -137,6 +138,112 @@ describe("singletons", () => { storage.counter.put(555); expect(subscriber.lastValue).toStrictEqual(321); }); + + it("uses the property name as the storage key by default", () => { + let mockStorage = makeMockStorage(); + let storage = createTypedStorage(mockStorage, { + singletons: { + counter: singleton(0), + } + }); + + storage.counter.put(123); + + // Declaring a singleton with no options must be byte-identical on disk to a bare default. + expect(mockStorage.kv.get("counter")).toStrictEqual(123); + }); + + it("reads and writes a legacy storage key", () => { + let mockStorage = makeMockStorage(); + + // Data written by an earlier version of the schema, when the property was called `oldName`. + mockStorage.kv.put("oldName", 42); + + let storage = createTypedStorage(mockStorage, { + singletons: { + newName: singleton(0, {storageKey: "oldName"}), + } + }); + + expect(storage.newName.get()).toStrictEqual(42); + + storage.newName.put(43); + + expect(storage.newName.get()).toStrictEqual(43); + expect(mockStorage.kv.get("oldName")).toStrictEqual(43); + expect(mockStorage.kv.get("newName")).toBeUndefined(); + }); + + it("falls back to the default when the legacy key was never written", () => { + let mockStorage = makeMockStorage(); + let storage = createTypedStorage(mockStorage, { + singletons: { + newName: singleton(false, {storageKey: "oldName"}), + } + }); + + expect(storage.newName.get()).toStrictEqual(false); + + storage.newName.put(true); + + expect(mockStorage.kv.get("oldName")).toStrictEqual(true); + }); + + it("notifies subscribers for a legacy storage key", () => { + let mockStorage = makeMockStorage(); + let storage = createTypedStorage(mockStorage, { + singletons: { + newName: singleton(0, {storageKey: "oldName"}), + } + }); + + let subscriber = { + lastValue: -1, + update(value: number) { + this.lastValue = value; + } + }; + storage.newName.subscribe(subscriber); + + storage.newName.put(7); + + expect(subscriber.lastValue).toStrictEqual(7); + expect(mockStorage.kv.get("oldName")).toStrictEqual(7); + }); +}); + +describe("collections with a legacy storage name", () => { + it("stores records and indexes under the legacy prefix", () => { + let mockStorage = makeMockStorage(); + let storage = createTypedStorage(mockStorage, { + collections: { + people: collection()({ + storageName: "users", + primaryKey: "name", + uniqueIndexes: { + byUid: (user: User) => user.uid + }, + nonUniqueIndexes: { + byLevel: (user: User) => user.level + } + }) + } + }); + + storage.people.put(ALICE); + + expect(storage.people.get("alice")).toStrictEqual(ALICE); + expect(storage.people.byUid.get(45)).toStrictEqual(ALICE); + expect([...storage.people.byLevel.list(8)]).toStrictEqual([ALICE]); + + // Every key -- the record and both indexes -- lives under the legacy name, so a collection + // renamed in code reads data written before the rename. + let keys = [...mockStorage.kv.list({})].map(([key]) => key); + expect(keys.some(key => key.startsWith("users:"))).toStrictEqual(true); + expect(keys.some(key => key.startsWith("users.byUid:"))).toStrictEqual(true); + expect(keys.some(key => key.startsWith("users.byLevel:"))).toStrictEqual(true); + expect(keys.some(key => key.startsWith("people"))).toStrictEqual(false); + }); }); type User = { diff --git a/packages/typed-storage/src/index.ts b/packages/typed-storage/src/index.ts index 757a73d2c..f55be9f60 100644 --- a/packages/typed-storage/src/index.ts +++ b/packages/typed-storage/src/index.ts @@ -172,6 +172,7 @@ interface CollectionSchema< primaryKey: PrimaryKey; uniqueIndexes?: UniqueIndexes; nonUniqueIndexes?: NonUniqueIndexes; + storageName?: string; } export function collection() { @@ -182,12 +183,46 @@ export function collection() { primaryKey: PrimaryKey, uniqueIndexes?: UniqueIndexes, nonUniqueIndexes?: NonUniqueIndexes, + /** + * The name this collection's keys (records and indexes alike) are prefixed with, + * overriding the schema property name. Like `SingletonOptions.storageKey`, this lets the + * code be renamed without migrating what is already on disk. + */ + storageName?: string, }) : CollectionSchema { return options as (CollectionSchemaBrand & typeof options); } } +/** Options for a singleton slot declared with `singleton()` rather than a bare default value. */ +export interface SingletonOptions { + /** + * The KV key this slot lives under, overriding the schema property name. Renaming a schema + * property is otherwise a storage migration, since the property name *is* the key; declaring the + * old key here renames the code without touching what is already on disk. + */ + storageKey?: string; +} + +/** + * A singleton slot declared with options. Returned by `singleton()`; a class rather than a plain + * branded object so `createTypedStorage` can tell it apart at runtime from a default value that + * happens to be an object. + */ +export class SingletonSchema { + constructor(readonly defaultValue: T, readonly options: SingletonOptions) {} +} + +/** + * Declares a singleton slot that needs options. A bare default value stays the shorthand for the + * common case (`{singletons: {count: 0}}`) and behaves identically. + */ +export function singleton( + defaultValue: T, options: SingletonOptions = {}): SingletonSchema { + return new SingletonSchema(defaultValue, options); +} + // ======================================================================================= type CollectionImpl = TypedStorage ? CollectionImpl : never } & { - [K in keyof Singletons]: Singleton; + [K in keyof Singletons]: Singletons[K] extends SingletonSchema + ? Singleton : Singleton; }; export function keyString(key: Key): string { @@ -681,15 +717,19 @@ export function createTypedStoragecolSchema); + let storageName = (>colSchema).storageName; + result[colName] = createCollection(storage, storageName ?? colName, colSchema); } - for (let [key, defaultValue] of Object.entries(schema.singletons || {})) { + for (let [key, slotSchema] of Object.entries(schema.singletons || {})) { + let defaultValue = slotSchema instanceof SingletonSchema ? slotSchema.defaultValue : slotSchema; + let storageKey = slotSchema instanceof SingletonSchema + ? slotSchema.options.storageKey ?? key : key; let subscribers = new Set>(); - let singleton: Singleton = { + let slot: Singleton = { get(): any { - let result = storage.kv.get(key); + let result = storage.kv.get(storageKey); if (result === undefined) { result = defaultValue; } @@ -698,13 +738,13 @@ export function createTypedStorage { for (let subscriber of subscribers) { subscriber.update(value); } - storage.kv.put(key, value); + storage.kv.put(storageKey, value); }); } }, @@ -718,7 +758,7 @@ export function createTypedStorage () => {}, ensureObserver: async () => {}, syncOutputsTo: async () => {}, - getSharingManager: async () => ({ getEffectiveRole: () => role }), + // What open() consults for a non-owner's role: the permission-graph lookup and observer + // verification in one. The sharing manager is still reached, but only to redeem a share key, + // which these tests never pass. + authorizeCollaborator: async () => role, + getSharingManager: async () => ({}), ctx: { id: { toString: () => "workspace-id" }, exports: opts.exports ?? {} }, users: { idFromString: (id: string) => id, @@ -96,7 +100,7 @@ export async function openFakeOverseer( }), }, storage: Object.assign(storage, { - prohibitAllSharing: { get: () => false }, + containsRestrictedData: { get: () => false }, title: { get: () => "Test Workspace" }, }), }, diff --git a/packages/workshop-backend/__tests__/observer-coverage-scrub.test.ts b/packages/workshop-backend/__tests__/observer-coverage-scrub.test.ts new file mode 100644 index 000000000..4defc6494 --- /dev/null +++ b/packages/workshop-backend/__tests__/observer-coverage-scrub.test.ts @@ -0,0 +1,210 @@ +// A failed live check (a gatekeeper's addObserver refusing, or the verifier failing to resolve) +// must scrub that gatekeeper from the collaborator's *persisted* observer record synchronously +// with the failure determination: the record is the standing claim that this collaborator was +// verified for that producer, and this open is not going to renew it. Because the claim is what +// admitted them, the shrink also severs their still-live sessions (see observer-scope-restart). +// +// Runs against a real OverseerDurableObject (the TEST_OVERSEER binding, like +// git-migration-do.test.ts); the gatekeeper facet, the client's User DO, and the restart are the +// only fakes -- a real ctx.abort() would kill the test DO. + +import { describe, expect, it } from "vitest"; +import { env } from "cloudflare:workers"; +import { runInDurableObject } from "cloudflare:test"; +import type { OverseerDurableObject } from "../src/overseer.js"; + +declare module "cloudflare:workers" { + interface ProvidedEnv { + TEST_OVERSEER: DurableObjectNamespace; + } +} + +// Seed the owner profile id (so the sharing manager needs no User DO round trip) and record the +// restart a coverage shrink schedules instead of performing it. +function recordRestarts(impl: any): string[] { + impl.ownerProfileId = "owner"; + let restarts: string[] = []; + impl.scheduleAccessRestart = async (reason: string) => { restarts.push(reason); }; + return restarts; +} + +function seedGatekeepers(impl: any): void { + for (let id of [1, 2]) { + impl.storage.gatekeepers.put({ + id, + resourceTitle: `Connection ${id}`, + class: {} as any, + creationSpec: { + type: "gatekeeper", + vendorId: "testvendor", + resourceUrl: `https://example.com/${id}`, + typeUrlPattern: "https://*", + }, + }); + } +} + +// A client User DO that always has the account and always mints a verifier. +const fakeClientUser = { + getVerifier: async () => ({}), + describeConnectedAccount: async () => null, +} as any; + +describe("observer coverage scrub on a failed live check", () => { + it("a refused re-verification drops the entry and severs the collaborator's sessions", + async () => { + let stub = env.TEST_OVERSEER.getByName("observer-coverage-scrub-refused"); + await runInDurableObject(stub, async (instance: OverseerDurableObject) => { + let impl = (instance as unknown as { impl: any }).impl; + seedGatekeepers(impl); + let restarts = recordRestarts(impl); + // Alice is a reachable collaborator whose previous successful open left coverage for both + // gatekeepers. + impl.storage.collaborators.put({ + profile: { type: "user", id: "alice", name: "Alice" }, + addedBy: [{ type: "user", sharer: "owner", created: new Date(), role: "build" }], + }); + impl.storage.observers.put( + { profileId: "alice", observerId: "obs-1", accountChoices: { 1: 10, 2: 20 } }); + + impl.getGatekeeperFacet = (id: number) => ({ + addObserver: async () => { + if (id === 1) throw new Error("access revoked upstream"); + }, + removeObserver: async () => {}, + }); + + // No repair channel, so gatekeeper 1's refusal is terminal -- and descriptive. + await expect(impl.ensureObserver("alice", fakeClientUser, "build")) + .rejects.toThrow(/could not confirm/); + + // The refused gatekeeper's coverage is scrubbed; the other's survives. + let record = impl.storage.observers.get("alice"); + expect(1 in record.accountChoices).toBe(false); + expect(record.accountChoices[2]).toBe(20); + + // The point of the scrub: alice's coverage shrank, so every live session is severed and + // must re-open against what the record now claims. + await new Promise(resolve => setTimeout(resolve, 0)); + expect(restarts).toHaveLength(1); + + // Neither producer's restricted reads are blocked, though -- both are verifiable, so + // admission is the whole enforcement and nobody unverified can be watching. + let restricted = { title: "t", description: "d", containsRestrictedData: true }; + await expect(impl.authorizeObservation(1, restricted, { from: "user" })) + .resolves.toBeUndefined(); + await expect(impl.authorizeObservation(2, restricted, { from: "user" })) + .resolves.toBeUndefined(); + }); + }); + + it("a getVerifier rejection scrubs that gatekeeper's persisted coverage", async () => { + let stub = env.TEST_OVERSEER.getByName("observer-coverage-scrub-getverifier"); + await runInDurableObject(stub, async (instance: OverseerDurableObject) => { + let impl = (instance as unknown as { impl: any }).impl; + seedGatekeepers(impl); + recordRestarts(impl); + // Already-configured coverage for both gatekeepers, as a previous successful open left it. + impl.storage.observers.put( + { profileId: "alice", observerId: "obs-1", accountChoices: { 1: 10, 2: 20 } }); + + let removed: number[] = []; + impl.getGatekeeperFacet = (id: number) => ({ + addObserver: async () => {}, + removeObserver: async () => { removed.push(id); }, + }); + + // Gatekeeper 1's verifier never materializes: the client's User DO *rejects* (the + // deterministic vendor-mismatch throw, or any cross-worker transport failure) rather than + // returning null. + let failingClientUser = { + getVerifier: async (accountId: number) => { + if (accountId === 10) throw new Error("account is for a different vendor"); + return {}; + }, + describeConnectedAccount: async () => null, + } as any; + + // No repair channel, so the failure is terminal -- and descriptive, not the raw RPC error. + await expect(impl.ensureObserver("alice", failingClientUser, "build")) + .rejects.toThrow(/could not confirm/); + + // The rejection went through fail(): gatekeeper 1's persisted coverage is scrubbed -- so + // the record no longer claims this collaborator was verified for it -- while gatekeeper 2's + // survives. + let record = impl.storage.observers.get("alice"); + expect(1 in record.accountChoices).toBe(false); + expect(record.accountChoices[2]).toBe(20); + + // Alice was already an admitted observer, so the failure de-registers her from nothing: the + // registrations are what make gatekeepers name her in `excludeObservers`, and the scrub does + // not cover the same observations (it gates `containsRestrictedData` only). + expect(removed).toEqual([]); + }); + }); + + it("keeps a returning observer's registration so forward exclusion survives the failure", + async () => { + let stub = env.TEST_OVERSEER.getByName("observer-coverage-scrub-keeps-registration"); + await runInDurableObject(stub, async (instance: OverseerDurableObject) => { + let impl = (instance as unknown as { impl: any }).impl; + seedGatekeepers(impl); + recordRestarts(impl); + // Alice's previous open covered gatekeeper 1 only; gatekeeper 2 is a binding added since, + // which she has never been verified against. + impl.storage.observers.put( + { profileId: "alice", observerId: "obs-1", accountChoices: { 1: 10 } }); + + let removed: number[] = []; + impl.getGatekeeperFacet = (id: number) => ({ + // Gatekeeper 1 has revoked her access upstream: the binding she *was* admitted for is the + // one that now refuses, which is exactly the case that used to drop her registration. + addObserver: async () => { if (id === 1) throw new Error("access revoked upstream"); }, + removeObserver: async () => { removed.push(id); }, + }); + + let configureCb = { configure: async (needs: {gatekeeperId: number}[]) => + needs.map(need => ({ gatekeeperId: need.gatekeeperId, accountId: 20 })) } as any; + + await expect(impl.ensureObserver("alice", fakeClientUser, "build", configureCb)) + .rejects.toThrow(/could not confirm/); + + // The two registrations are treated differently, which is the whole point. Gatekeeper 1's + // predates this call, so it survives and keeps naming her in `excludeObservers`. Gatekeeper + // 2's was created by this call, so rolling it back merely restores the pre-call state -- + // there was no prior registration whose exclusions could be lost. + expect(removed).toEqual([2]); + // Coverage is still scrubbed regardless, so her next open must re-verify gatekeeper 1. + expect(1 in impl.storage.observers.get("alice").accountChoices).toBe(false); + }); + }); + + it("a first-ever verification failure still rolls its registrations back", async () => { + let stub = env.TEST_OVERSEER.getByName("observer-coverage-scrub-first-ever"); + await runInDurableObject(stub, async (instance: OverseerDurableObject) => { + let impl = (instance as unknown as { impl: any }).impl; + seedGatekeepers(impl); + recordRestarts(impl); + // No observer record: Alice has never been admitted, so the observerId minted for this call + // is discarded with the unpersisted record and anything registered under it would linger + // unresolvable. + let removed: number[] = []; + impl.getGatekeeperFacet = (id: number) => ({ + addObserver: async () => { if (id === 2) throw new Error("no access"); }, + removeObserver: async () => { removed.push(id); }, + }); + + let configureCb = { configure: async (needs: {gatekeeperId: number}[]) => + needs.map(need => ({ gatekeeperId: need.gatekeeperId, accountId: need.gatekeeperId * 10 })) + } as any; + + await expect(impl.ensureObserver("alice", fakeClientUser, "build", configureCb)) + .rejects.toThrow(/could not confirm/); + + // Both the one that verified and the one that refused are rolled back, and no record is + // persisted. + expect(removed.toSorted()).toEqual([1, 2]); + expect(impl.storage.observers.get("alice")).toBeUndefined(); + }); + }); +}); diff --git a/packages/workshop-backend/__tests__/observer-scope-prune.test.ts b/packages/workshop-backend/__tests__/observer-scope-prune.test.ts new file mode 100644 index 000000000..bcab9bd8e --- /dev/null +++ b/packages/workshop-backend/__tests__/observer-scope-prune.test.ts @@ -0,0 +1,172 @@ +// ensureObserver must prune out-of-scope account choices from the observer record at every open, +// keeping the record an accurate statement of what this collaborator's most recent open verified. +// Rebinding a connection keeps the same gatekeeper id, so a stale entry left from before an unbind +// would otherwise silently re-register them off an account choice made for a scope the workspace +// no longer has, instead of asking them again. +// +// Runs against a real OverseerDurableObject (the TEST_OVERSEER binding, like +// git-migration-do.test.ts); the gatekeeper facet, the client's User DO, and the restart are the +// only fakes -- a real ctx.abort() would kill the test DO. + +import { describe, expect, it } from "vitest"; +import { env } from "cloudflare:workers"; +import { runInDurableObject } from "cloudflare:test"; +import type { OverseerDurableObject } from "../src/overseer.js"; + +declare module "cloudflare:workers" { + interface ProvidedEnv { + TEST_OVERSEER: DurableObjectNamespace; + } +} + +function seedGatekeepers(impl: any): void { + for (let id of [1, 2]) { + impl.storage.gatekeepers.put({ + id, + resourceTitle: `Connection ${id}`, + class: {} as any, + creationSpec: { + type: "gatekeeper", + vendorId: "testvendor", + resourceUrl: `https://example.com/${id}`, + typeUrlPattern: "https://*", + }, + }); + } +} + +// A gadget that binds only gatekeeper 1, leaving gatekeeper 2 out of "use" scope. +function seedGadgetBindingGk1(impl: any): void { + impl.storage.gadgets.put({ + id: 100, + title: "G", + created: new Date(), + bindingName: "G", + bindings: { DB: { target: 1 } }, + }); +} + +// A client User DO that always has the account and always mints a verifier. +const fakeClientUser = { + getVerifier: async () => ({}), +} as any; + +describe("ensureObserver out-of-scope coverage pruning", () => { + it("prunes an unbound gatekeeper's entry at a use-role open", async () => { + let stub = env.TEST_OVERSEER.getByName("observer-scope-prune-use"); + await runInDurableObject(stub, async (instance: OverseerDurableObject) => { + let impl = (instance as unknown as { impl: any }).impl; + seedGatekeepers(impl); + seedGadgetBindingGk1(impl); + impl.ownerProfileId = "owner"; + impl.storage.observers.put( + { profileId: "alice", observerId: "obs-1", accountChoices: { 1: 10, 2: 20 } }); + + let verified: number[] = []; + impl.getGatekeeperFacet = (id: number) => ({ + addObserver: async () => { verified.push(id); }, + }); + + await impl.ensureObserver("alice", fakeClientUser, "use"); + + // Gatekeeper 2 is outside "use" scope: its stale entry is gone, and nothing re-verified it. + expect(verified).toEqual([1]); + expect(impl.storage.observers.get("alice").accountChoices).toEqual({ 1: 10 }); + }); + }); + + it("prunes everything at an empty-scope open, keeping the record", async () => { + let stub = env.TEST_OVERSEER.getByName("observer-scope-prune-empty"); + await runInDurableObject(stub, async (instance: OverseerDurableObject) => { + let impl = (instance as unknown as { impl: any }).impl; + seedGatekeepers(impl); + // No gadgets at all: a "use" collaborator's verification scope is empty. + impl.ownerProfileId = "owner"; + impl.storage.observers.put( + { profileId: "alice", observerId: "obs-1", accountChoices: { 1: 10, 2: 20 } }); + + // No configureCb: the open must still resolve (nothing in scope to configure), and it must + // still prune -- this is exactly the everything-unbound open the fix exists for. + await impl.ensureObserver("alice", fakeClientUser, "use"); + + let record = impl.storage.observers.get("alice"); + expect(record).toBeDefined(); + expect(record.accountChoices).toEqual({}); + }); + }); + + it("keeps unbound gatekeepers' entries at a build-role open", async () => { + let stub = env.TEST_OVERSEER.getByName("observer-scope-prune-build"); + await runInDurableObject(stub, async (instance: OverseerDurableObject) => { + let impl = (instance as unknown as { impl: any }).impl; + seedGatekeepers(impl); + seedGadgetBindingGk1(impl); + impl.ownerProfileId = "owner"; + impl.storage.observers.put( + { profileId: "alice", observerId: "obs-1", accountChoices: { 1: 10, 2: 20 } }); + + let verified: number[] = []; + impl.getGatekeeperFacet = (id: number) => ({ + addObserver: async () => { verified.push(id); }, + }); + + // "build" scope is every account-requiring gatekeeper regardless of gadget bindings, so + // both entries are in scope and nothing may be pruned (guards against over-pruning). + await impl.ensureObserver("alice", fakeClientUser, "build"); + + expect(verified.toSorted()).toEqual([1, 2]); + expect(impl.storage.observers.get("alice").accountChoices).toEqual({ 1: 10, 2: 20 }); + }); + }); + + it("restarts on a rebind, and the re-open re-verifies the pruned producer", async () => { + let stub = env.TEST_OVERSEER.getByName("observer-scope-prune-rebind"); + await runInDurableObject(stub, async (instance: OverseerDurableObject) => { + let impl = (instance as unknown as { impl: any }).impl; + seedGatekeepers(impl); + seedGadgetBindingGk1(impl); + impl.ownerProfileId = "owner"; + let restarts: string[] = []; + impl.scheduleAccessRestart = async (reason: string) => { restarts.push(reason); }; + // Alice is a "use" collaborator with stale coverage for gatekeeper 2, left over from before + // it was unbound from every gadget. + impl.storage.collaborators.put({ + profile: { type: "user", id: "alice", name: "Alice" }, + addedBy: [{ type: "user", sharer: "owner", created: new Date(), role: "use" }], + }); + impl.storage.observers.put( + { profileId: "alice", observerId: "obs-1", accountChoices: { 1: 10, 2: 20 } }); + + let verified: number[] = []; + impl.getGatekeeperFacet = (id: number) => ({ + addObserver: async () => { verified.push(id); }, + }); + + // Alice opens during the unbound window: gatekeeper 2 is out of her scope, so this open + // verifies nothing against it -- and prunes her stale entry. + await impl.ensureObserver("alice", fakeClientUser, "use"); + expect(verified).toEqual([1]); + expect(impl.storage.observers.get("alice").accountChoices).toEqual({ 1: 10 }); + + // Rebind gatekeeper 2 (same gatekeeper id -- only the gadget's binding edges change). That + // widens every "use" collaborator's scope, so it severs Alice's live session. + impl.bindWorkpiece(100, "DB2", 2); + await new Promise(resolve => setTimeout(resolve, 0)); + expect(restarts).toHaveLength(1); + + // Her forced re-open is where gatekeeper 2 gets verified again -- and since the prune left + // no entry to reuse, she is asked to choose an account for it rather than being re-registered + // off the choice she made before it was unbound. + let asked: number[] = []; + let configureCb = { configure: async (needs: { gatekeeperId: number }[]) => { + asked.push(...needs.map(need => need.gatekeeperId)); + return needs.map(need => ({ gatekeeperId: need.gatekeeperId, accountId: 30 })); + } } as any; + await impl.ensureObserver("alice", fakeClientUser, "use", configureCb); + + expect(asked).toEqual([2]); + expect(verified.toSorted()).toEqual([1, 1, 2]); + expect(impl.storage.observers.get("alice").accountChoices).toEqual({ 1: 10, 2: 30 }); + }); + }); +}); diff --git a/packages/workshop-backend/__tests__/observer-scope-restart.test.ts b/packages/workshop-backend/__tests__/observer-scope-restart.test.ts new file mode 100644 index 000000000..5d162ea33 --- /dev/null +++ b/packages/workshop-backend/__tests__/observer-scope-restart.test.ts @@ -0,0 +1,283 @@ +// Authorization and observer verification run only at open(), so widening what a collaborator must +// be verified against would otherwise leave their live session holding access nobody checked. Each +// widening restarts the workspace (scheduleAccessRestart), forcing every client to re-open and +// re-verify against the new scope -- and a workspace with no collaborators is never disturbed, +// since the owner is never an observer. +// +// Runs against a real OverseerDurableObject (the TEST_OVERSEER binding, like +// observer-coverage-scrub.test.ts). scheduleAccessRestart is replaced with a recorder: a real +// ctx.abort() would kill the test DO. + +import { describe, expect, it } from "vitest"; +import { env } from "cloudflare:workers"; +import { runInDurableObject } from "cloudflare:test"; +import type { AiChatAuthorInfo } from "@gadgets/workshop-shared/api"; +import type { OverseerDurableObject } from "../src/overseer.js"; +import { openFakeOverseer } from "./fixtures.js"; + +declare module "cloudflare:workers" { + interface ProvidedEnv { + TEST_OVERSEER: DurableObjectNamespace; + } +} + +const OWNER = "owner"; +const AGENT: AiChatAuthorInfo = { type: "agent", id: "some-model", name: "Agent" }; +const USER_META = { profile: { type: "user", id: OWNER, name: "Owner" } as AiChatAuthorInfo }; + +// #restartIfShared is fire-and-forget over an async getSharingManager(), so let its continuation +// run before asserting. +const settle = () => new Promise(resolve => setTimeout(resolve, 0)); + +let doCounter = 0; + +async function withImpl(fn: (impl: any, restarts: string[]) => Promise): Promise { + let stub = env.TEST_OVERSEER.getByName(`observer-scope-restart-${++doCounter}`); + await runInDurableObject(stub, async (instance: OverseerDurableObject) => { + let impl = (instance as unknown as { impl: any }).impl; + // Seed the cached owner profile id so the sharing manager needs no User DO round trip. + impl.ownerProfileId = OWNER; + let restarts: string[] = []; + impl.scheduleAccessRestart = async (reason: string) => { restarts.push(reason); }; + await fn(impl, restarts); + }); +} + +function addCollaborator(impl: any, role: "build" | "use" = "build"): void { + impl.storage.collaborators.put({ + profile: { type: "user", id: "alice", name: "Alice" }, + addedBy: [{ type: "user", sharer: OWNER, created: new Date(), role }], + }); +} + +function seedGatekeeper(impl: any, id: number): void { + impl.storage.gatekeepers.put({ + id, + resourceTitle: `Connection ${id}`, + class: {} as any, + creationSpec: { + type: "gatekeeper", + vendorId: "testvendor", + resourceUrl: `https://example.com/${id}`, + typeUrlPattern: "https://*", + }, + }); +} + +// A vendorless connection (an AI model), which no collaborator is ever verified against. +function seedVendorlessGatekeeper(impl: any, id: number): void { + impl.storage.gatekeepers.put({ + id, + resourceTitle: `Model ${id}`, + class: {} as any, + creationSpec: { + type: "aiModel", modelId: `m${id}`, provider: "anthropic", modelName: "claude", + }, + }); +} + +function seedGadget(impl: any, id: number): void { + impl.storage.gadgets.put( + { id, title: "G", created: new Date(0), bindingName: "G", bindings: {} }); +} + +// A facet that lets addGatekeeper's describe() succeed. +function stubFacets(impl: any): void { + impl.getGatekeeperFacet = () => ({ + describe: async () => ({ title: "Test", url: "https://example.com/new" }), + }); +} + +const CONNECTION_SPEC = { + type: "gatekeeper" as const, + vendorId: "testvendor", + resourceUrl: "https://example.com/new", + typeUrlPattern: "https://*", +}; + +describe("restarting sessions when verification scope widens", () => { + it("adding a connection restarts a shared workspace", () => withImpl(async (impl, restarts) => { + addCollaborator(impl); + stubFacets(impl); + + // A new account-requiring connection is immediately in every "build" collaborator's scope, + // and their live session was never verified against it. + await impl.addGatekeeper({} as any, CONNECTION_SPEC); + await settle(); + + expect(restarts).toHaveLength(1); + })); + + it("a connection under construction is unreachable until the restart is scheduled", + () => withImpl(async (impl, restarts) => { + addCollaborator(impl); + let releaseDescribe!: () => void; + let describing = new Promise(resolve => { releaseDescribe = resolve; }); + impl.getGatekeeperFacet = () => ({ + describe: async () => { + await describing; + return { title: "Test", url: "https://example.com/new" }; + }, + }); + // A "build" client interface over this DO's real gatekeeper table, which is all + // getGatekeeperById consults. + let client = await openFakeOverseer({ gatekeepers: impl.storage.gatekeepers }); + + // Ids are allocated sequentially, so a client can simply guess the next one. + let id = impl.storage.nextGatekeeperId.get(); + let added = impl.addGatekeeper({} as any, CONNECTION_SPEC); + await settle(); + + // The DO's input gate is open across describe(), so a live build session gets a turn here -- + // before #restartIfShared has severed it. Nothing is published for it to find. + expect(impl.storage.gatekeepers.get(id)).toBeUndefined(); + await expect(client.getGatekeeperById(id)).rejects.toThrow(/No such gatekeeper id/); + expect(restarts).toEqual([]); + + releaseDescribe(); + await added; + await settle(); + + expect(impl.storage.gatekeepers.get(id).resourceTitle).toBe("Test"); + expect(restarts).toHaveLength(1); + })); + + it("adding a connection to a solo workspace disturbs nobody", + () => withImpl(async (impl, restarts) => { + stubFacets(impl); + + // The owner is never an observer, so there is nobody to re-verify -- and the one session that + // exists is the one that asked for the connection. + await impl.addGatekeeper({} as any, CONNECTION_SPEC); + await settle(); + + expect(restarts).toEqual([]); + })); + + it("adding a vendorless connection widens nothing", () => withImpl(async (impl, restarts) => { + addCollaborator(impl); + stubFacets(impl); + + // #inScopeGatekeepers skips a spec with no vendorId, so no collaborator is ever verified + // against an AI model binding and adding one cannot leave anyone under-verified. + await impl.addGatekeeper({} as any, { + type: "aiModel", modelId: "m", provider: "anthropic", modelName: "claude", + }); + await settle(); + + expect(restarts).toEqual([]); + })); + + it("binding a connection into a gadget restarts a shared workspace", + () => withImpl(async (impl, restarts) => { + addCollaborator(impl, "use"); + seedGatekeeper(impl, 1); + seedGadget(impl, 100); + + // A permanent edge puts the connection into "use" scope: the gadget UI the collaborator drives + // can now invoke it. + impl.bindWorkpiece(100, "DB", 1); + await settle(); + + expect(restarts).toHaveLength(1); + })); + + it("a pending bind is invisible to collaborators, so it restarts nothing", + () => withImpl(async (impl, restarts) => { + addCollaborator(impl, "use"); + seedGatekeeper(impl, 1); + seedGadget(impl, 100); + + // An edge provisional to a chat isn't in #gadgetBoundGatekeeperIds until it's promoted, which + // is what restarts (see the merge case below). + impl.bindWorkpiece(100, "DB", 1, 7); + await settle(); + + expect(restarts).toEqual([]); + })); + + it("promoting a pending bind at merge restarts a shared workspace", + () => withImpl(async (impl, restarts) => { + addCollaborator(impl, "use"); + seedGatekeeper(impl, 1); + seedGadget(impl, 100); + impl.storage.chatMeta.put( + { id: 1, title: "Chat", started: new Date(0), lastActive: new Date(0) }); + + impl.bindWorkpiece(100, "DB", 1, 1); + await impl.commitAgentStep(1, AGENT, [{ type: "message", message: "bound a connection" }], { + changes: [], + createdGadgets: [], + addedBindings: [{ gadgetId: 100, name: "DB", target: 1 }], + }); + await settle(); + expect(restarts).toEqual([]); + + expect(await impl.mergeChanges(1, USER_META, "owner-user-do")) + .toEqual({ outcome: "merged" }); + await settle(); + + // Accepting the change is the moment the edge becomes visible to "use" collaborators. + expect(impl.storage.gadgets.get(100).bindings.DB.pending).toBeUndefined(); + expect(restarts).toHaveLength(1); + })); + + it("a merge that promotes only a vendorless edge restarts nothing", + () => withImpl(async (impl, restarts) => { + addCollaborator(impl, "use"); + seedVendorlessGatekeeper(impl, 1); + seedGadget(impl, 100); + impl.storage.chatMeta.put( + { id: 1, title: "Chat", started: new Date(0), lastActive: new Date(0) }); + + impl.bindWorkpiece(100, "MODEL", 1, 1); + await impl.commitAgentStep(1, AGENT, [{ type: "message", message: "bound a model" }], { + changes: [], + createdGadgets: [], + addedBindings: [{ gadgetId: 100, name: "MODEL", target: 1 }], + }); + + expect(await impl.mergeChanges(1, USER_META, "owner-user-do")) + .toEqual({ outcome: "merged" }); + await settle(); + + // The edge is promoted, but a vendorless connection is in nobody's verification scope, so the + // effective scope is unchanged and no collaborator's session is interrupted. (The trigger + // compares scopes rather than restarting on any promotion, which most merges are.) + expect(impl.storage.gadgets.get(100).bindings.MODEL.pending).toBeUndefined(); + expect(restarts).toEqual([]); + })); + + it("a failed re-verification severs the collaborator's other sessions", + () => withImpl(async (impl, restarts) => { + addCollaborator(impl); + seedGatekeeper(impl, 1); + // Alice's previous open left coverage that her still-live sessions rest on. + impl.storage.observers.put( + { profileId: "alice", observerId: "obs-1", accountChoices: { 1: 10 } }); + impl.getGatekeeperFacet = () => ({ + addObserver: async () => { throw new Error("access revoked upstream"); }, + removeObserver: async () => {}, + }); + let fakeClientUser = + { getVerifier: async () => ({}), describeConnectedAccount: async () => null } as any; + // Answers every prompt with the same account, so each attempt fails the same way. + let configureCb = { configure: async (needs: { gatekeeperId: number }[]) => + needs.map(need => ({ gatekeeperId: need.gatekeeperId, accountId: 10 })) } as any; + + await expect(impl.ensureObserver("alice", fakeClientUser, "build", configureCb)) + .rejects.toThrow(/could not confirm/); + await settle(); + + // The scrub only rewrites what the record claims; the restart is what reaches the sessions. + expect(1 in impl.storage.observers.get("alice").accountChoices).toBe(false); + expect(restarts).toHaveLength(1); + + // A second identical failure finds the entry already scrubbed, so nothing shrank and nothing + // restarts: the trigger cannot loop against a collaborator who simply keeps failing. + await expect(impl.ensureObserver("alice", fakeClientUser, "build", configureCb)) + .rejects.toThrow(/could not confirm/); + await settle(); + expect(restarts).toHaveLength(1); + })); +}); 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..233761fb3 --- /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 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__/restricted-observation-latch.test.ts b/packages/workshop-backend/__tests__/restricted-observation-latch.test.ts new file mode 100644 index 000000000..b6f390ab2 --- /dev/null +++ b/packages/workshop-backend/__tests__/restricted-observation-latch.test.ts @@ -0,0 +1,250 @@ +// authorizeObservation's restricted-data gates. The exclusion gate is decided before anything +// else: the restricted-mode latch is one-way, so an observation the exclusion blocks must leave +// no trace -- no latch, no record, sharing untouched. The decisions the delivery rests on (the +// removed-connection refusal, the unverifiable-producer refusal, the latch, the record) all run +// *after* the exclusion teardown's awaited cross-worker fan-out, in one synchronous block, so a +// removal landing mid-teardown refuses the observation rather than slipping past a pre-latched +// producer. And a restricted observation arriving through an already-removed connection (an +// in-flight facet RPC can outlive removeGatekeeper) is refused rather than latched: with zero +// collaborators nothing else would stop it, and latching a missing producer id would permanently +// brick sharing via assertNewSharingAllowed's missing-record branch. +// +// The last case covers the one producer admission cannot enforce: a connection with no vendor +// account behind it is in nobody's verification scope, so no collaborator is ever asked about it. +// +// Runs against a real OverseerDurableObject (the TEST_OVERSEER binding, like +// restricted-producer-removal.test.ts); the gatekeeper facet is the only fake. + +import { describe, expect, it } from "vitest"; +import { env } from "cloudflare:workers"; +import { runInDurableObject } from "cloudflare:test"; +import type { OverseerDurableObject } from "../src/overseer.js"; + +declare module "cloudflare:workers" { + interface ProvidedEnv { + TEST_OVERSEER: DurableObjectNamespace; + } +} + +const OWNER = "alice"; + +function deferred(): { promise: Promise; resolve: () => void } { + let resolve!: () => void; + let promise = new Promise(r => { resolve = r; }); + return { promise, resolve }; +} + +const tick = () => new Promise(resolve => setTimeout(resolve, 0)); + +function getImpl(instance: OverseerDurableObject): any { + let impl = (instance as unknown as { impl: any }).impl; + // The sharing manager resolves collaborator reachability from the owner; seed the cached + // profile id so no User DO round trip is attempted. + impl.ownerProfileId = OWNER; + return impl; +} + +function seedGatekeeper(impl: any, id: number): void { + impl.storage.gatekeepers.put({ + id, + resourceTitle: `Connection ${id}`, + class: {} as any, + creationSpec: { + type: "gatekeeper", + vendorId: "testvendor", + resourceUrl: `https://example.com/${id}`, + typeUrlPattern: "https://*", + }, + }); +} + +const RESTRICTED_EXCLUDING_MALLORY = { + title: "Read a thing", + description: "The test read a thing.", + containsRestrictedData: true, + excludeObservers: ["obs-m"], +}; + +describe("authorizeObservation's restricted-data gates", () => { + it("latches and records only after the exclusion teardown admits the observation", async () => { + let stub = env.TEST_OVERSEER.getByName("restricted-latch-teardown-window"); + await runInDurableObject(stub, async (instance: OverseerDurableObject) => { + let impl = getImpl(instance); + seedGatekeeper(impl, 1); + // An outstanding share link keeps the workspace "shared" for + // removalBlockedByRestrictedData. + impl.storage.shareKeys.put({ + id: "link-1", created: new Date(), createdBy: OWNER, role: "build", + }); + // Mallory holds an observer record but no reachable role: the named exclusion admits the + // observation and schedules her teardown. + impl.storage.observers.put( + { profileId: "mallory", observerId: "obs-m", accountChoices: { 1: 10 } }); + + // The cross-worker teardown parks, holding the observation mid-flight before any decision + // the delivery rests on has been made. + let held = deferred(); + impl.getGatekeeperFacet = () => ({ + removeObserver: async () => { await held.promise; }, + }); + + let observation = impl.authorizeObservation( + 1, RESTRICTED_EXCLUDING_MALLORY, { from: "user" }); + await tick(); + + // Nothing is delivered while the teardown is in flight, so nothing has latched: a teardown + // that ends in refusal must leave no trace. + expect(impl.storage.containsRestrictedData.get()).toBe(false); + + held.resolve(); + await expect(observation).resolves.toBeUndefined(); + + // Delivery: the latch and the record landed together, and everything keyed on the latch + // now holds. + expect(impl.storage.containsRestrictedData.get()).toBe(true); + expect(impl.removalBlockedByRestrictedData(1, await impl.getSharingManager())).toBe(true); + + // The teardown still ran (mallory is no longer set up to observe). + expect(impl.storage.observers.get("mallory")).toBeUndefined(); + let records = [...impl.storage.actions.list()]; + expect(records).toHaveLength(1); + expect(records[0].type).toBe("observation"); + }); + }); + + it("leaves no trace when the exclusion gate blocks the observation", async () => { + let stub = env.TEST_OVERSEER.getByName("restricted-latch-exclusion-blocked"); + await runInDurableObject(stub, async (instance: OverseerDurableObject) => { + let impl = getImpl(instance); + seedGatekeeper(impl, 1); + // Mallory is a current collaborator, verified against the producer: nothing else stands in + // this observation's way, so the exclusion gate is the only thing blocking it. + impl.storage.collaborators.put({ + profile: { id: "mallory", name: "Mallory" }, + addedBy: [{ type: "user", sharer: OWNER, created: new Date(), role: "build" }], + }); + impl.storage.observers.put( + { profileId: "mallory", observerId: "obs-m", accountChoices: { 1: 10 } }); + + await expect(impl.authorizeObservation( + 1, RESTRICTED_EXCLUDING_MALLORY, { from: "user" })) + .rejects.toThrow(/not permitted to see/); + + // The blocked observation delivered no data, so the workspace is not restricted: no latch, + // sharing still grantable, no action record -- and mallory, still authorized, was not torn + // down. + expect(impl.storage.containsRestrictedData.get()).toBe(false); + expect(() => impl.assertNewSharingAllowed()).not.toThrow(); + expect([...impl.storage.actions.list()]).toHaveLength(0); + expect(impl.storage.observers.get("mallory")).toBeDefined(); + }); + }); + + it("refuses the observation when the connection is removed mid-teardown", async () => { + let stub = env.TEST_OVERSEER.getByName("restricted-latch-removed-mid-teardown"); + await runInDurableObject(stub, async (instance: OverseerDurableObject) => { + let impl = getImpl(instance); + seedGatekeeper(impl, 1); + impl.storage.observers.put( + { profileId: "mallory", observerId: "obs-m", accountChoices: { 1: 10 } }); + + let held = deferred(); + impl.getGatekeeperFacet = () => ({ + removeObserver: async () => { await held.promise; }, + }); + + let observation = impl.authorizeObservation( + 1, RESTRICTED_EXCLUDING_MALLORY, { from: "user" }); + await tick(); + + // The latch isn't set during the teardown, so removalBlockedByRestrictedData doesn't + // protect the producer in this window; the connection is removed out from under the + // in-flight observation. + impl.storage.gatekeepers.delete(1); + + held.resolve(); + // The post-teardown re-read catches the removal: refused, and nothing latched or recorded. + await expect(observation).rejects.toThrow(/has been removed/); + expect(impl.storage.containsRestrictedData.get()).toBe(false); + expect([...impl.storage.actions.list()]).toHaveLength(0); + }); + }); + + it("refuses restricted data through a removed connection instead of bricking sharing", async () => { + let stub = env.TEST_OVERSEER.getByName("restricted-latch-missing-producer"); + await runInDurableObject(stub, async (instance: OverseerDurableObject) => { + let impl = getImpl(instance); + // No gatekeeper record: the in-flight facet RPC outlived removeGatekeeper. With zero + // collaborators nothing else refuses it, so only this guard stands between the observation + // and latching a missing producer id. + await expect(impl.authorizeObservation(1, { + title: "Read a thing", + description: "The test read a thing.", + containsRestrictedData: true, + }, { from: "user" })).rejects.toThrow(/has been removed/); + + // A blocked observation delivered no data: the workspace must not be left restricted -- + // and above all must not be left permanently unshareable by latching a missing producer. + expect(impl.storage.containsRestrictedData.get()).toBe(false); + expect(() => impl.assertNewSharingAllowed()).not.toThrow(); + }); + }); + + it("refuses an unverifiable producer's restricted data on a shared workspace", async () => { + let stub = env.TEST_OVERSEER.getByName("restricted-latch-unverifiable-shared"); + await runInDurableObject(stub, async (instance: OverseerDurableObject) => { + let impl = getImpl(instance); + // An AI model binding has no vendor account behind it, so #inScopeGatekeepers skips it and + // no collaborator is ever asked to verify against it -- the one producer admission cannot + // enforce, and the only one this check still refuses. + impl.storage.gatekeepers.put({ + id: 1, + resourceTitle: "Claude", + class: {} as any, + creationSpec: { + type: "aiModel", modelId: "m", provider: "anthropic", modelName: "claude", + }, + }); + impl.storage.collaborators.put({ + profile: { id: "mallory", name: "Mallory" }, + addedBy: [{ type: "user", sharer: OWNER, created: new Date(), role: "build" }], + }); + + await expect(impl.authorizeObservation(1, { + title: "Read a thing", + description: "The test read a thing.", + containsRestrictedData: true, + }, { from: "user" })).rejects.toThrow(/cannot verify anyone's access/); + + // Refused, so nothing latched: the owner can still unshare and read it. + expect(impl.storage.containsRestrictedData.get()).toBe(false); + expect([...impl.storage.actions.list()]).toHaveLength(0); + }); + }); + + it("admits an unverifiable producer's restricted data on a solo workspace", async () => { + let stub = env.TEST_OVERSEER.getByName("restricted-latch-unverifiable-solo"); + await runInDurableObject(stub, async (instance: OverseerDurableObject) => { + let impl = getImpl(instance); + impl.storage.gatekeepers.put({ + id: 1, + resourceTitle: "Claude", + class: {} as any, + creationSpec: { + type: "aiModel", modelId: "m", provider: "anthropic", modelName: "claude", + }, + }); + + // Nobody to under-verify: the owner reads their own data, and the latch is what keeps it + // that way. + await expect(impl.authorizeObservation(1, { + title: "Read a thing", + description: "The test read a thing.", + containsRestrictedData: true, + }, { from: "user" })).resolves.toBeUndefined(); + + expect(impl.storage.containsRestrictedData.get()).toBe(true); + expect(() => impl.assertNewSharingAllowed()).toThrow(); + }); + }); +}); diff --git a/packages/workshop-backend/__tests__/restricted-producer-removal.test.ts b/packages/workshop-backend/__tests__/restricted-producer-removal.test.ts new file mode 100644 index 000000000..8127507b6 --- /dev/null +++ b/packages/workshop-backend/__tests__/restricted-producer-removal.test.ts @@ -0,0 +1,367 @@ +// removalBlockedByRestrictedData() is the single predicate behind the producer-removal guard: +// GatekeeperClientImpl.remove() refuses on it, and ensureAmbientCapsules()'s reconciliation skips +// stale records on it. It must block exactly when deleting the record would readmit an unverified +// party -- the workspace is latched, the record is a restricted producer (verifiable or not), and +// the sharing graph still has collaborators or outstanding share links. +// +// Runs against a real OverseerDurableObject (the TEST_OVERSEER binding, like +// git-migration-do.test.ts) so the predicate reads real storage; records are seeded directly +// through the impl. + +import { describe, expect, it } from "vitest"; +import { env } from "cloudflare:workers"; +import { runInDurableObject } from "cloudflare:test"; +import type { OverseerDurableObject } from "../src/overseer.js"; + +declare module "cloudflare:workers" { + interface ProvidedEnv { + TEST_OVERSEER: DurableObjectNamespace; + } +} + +const OWNER = "alice"; + +function getImpl(instance: OverseerDurableObject): any { + let impl = (instance as unknown as { impl: any }).impl; + // The sharing manager resolves collaborator reachability from the owner; seed the cached + // profile id so no User DO round trip is attempted. + impl.ownerProfileId = OWNER; + return impl; +} + +// A verifiable connection record, or (without `creationSpec`) a legacy one -- unverifiable, and +// guarded all the same. +function seedGatekeeper(impl: any, id: number, creationSpec = true): void { + impl.storage.gatekeepers.put({ + id, + resourceTitle: `Connection ${id}`, + class: {} as any, + ...(creationSpec ? { + creationSpec: { + type: "gatekeeper", + vendorId: "testvendor", + resourceUrl: `https://example.com/${id}`, + typeUrlPattern: "https://*", + }, + } : {}), + }); +} + +// A restricted observation attributed to `gatekeeperId`, which is what makes it a producer +// (restrictedProducerIds scans the action log for exactly these). +function seedRestrictedObservation(impl: any, gatekeeperId: number, actionId: number): void { + impl.storage.actions.put({ + id: actionId, + gatekeeperId, + caller: { from: "user" }, + createdAt: new Date(), + state: "approved", + type: "observation", + description: { + title: "Read a thing", + description: "The test read a thing.", + containsRestrictedData: true, + }, + }); +} + +function seedCollaborator(impl: any): void { + impl.storage.collaborators.put({ + profile: { id: "bob", name: "Bob" }, + addedBy: [{ type: "user", sharer: OWNER, created: new Date(), role: "build" }], + }); +} + +function seedShareLink(impl: any): void { + impl.storage.shareKeys.put({ + id: "link-1", + created: new Date(), + createdBy: OWNER, + role: "build", + }); +} + +describe("removalBlockedByRestrictedData", () => { + it("does not block while the workspace is unlatched", async () => { + let stub = env.TEST_OVERSEER.getByName("producer-removal-unlatched"); + await runInDurableObject(stub, async (instance: OverseerDurableObject) => { + let impl = getImpl(instance); + seedGatekeeper(impl, 1); + seedRestrictedObservation(impl, 1, 100); + seedCollaborator(impl); + + expect(impl.removalBlockedByRestrictedData(1, await impl.getSharingManager())).toBe(false); + }); + }); + + it("does not block a latched non-producer, even while shared", async () => { + let stub = env.TEST_OVERSEER.getByName("producer-removal-non-producer"); + await runInDurableObject(stub, async (instance: OverseerDurableObject) => { + let impl = getImpl(instance); + seedGatekeeper(impl, 1); + seedGatekeeper(impl, 2); + seedRestrictedObservation(impl, 1, 100); + impl.storage.containsRestrictedData.put(true); + seedCollaborator(impl); + + let sharing = await impl.getSharingManager(); + expect(impl.removalBlockedByRestrictedData(2, sharing)).toBe(false); + expect(impl.removalBlockedByRestrictedData(1, sharing)).toBe(true); + }); + }); + + it("blocks a legacy (unverifiable) producer while a collaborator exists", async () => { + let stub = env.TEST_OVERSEER.getByName("producer-removal-legacy"); + await runInDurableObject(stub, async (instance: OverseerDurableObject) => { + let impl = getImpl(instance); + seedGatekeeper(impl, 1, /* creationSpec */ false); + seedRestrictedObservation(impl, 1, 100); + impl.storage.containsRestrictedData.put(true); + seedCollaborator(impl); + + // The legacy record is what denies every non-owner open (#inScopeGatekeepers throws on + // it), so removing it while shared would readmit the collaborator unverified. + expect(impl.removalBlockedByRestrictedData(1, await impl.getSharingManager())).toBe(true); + }); + }); + + it("blocks on an outstanding share link alone", async () => { + let stub = env.TEST_OVERSEER.getByName("producer-removal-link-only"); + await runInDurableObject(stub, async (instance: OverseerDurableObject) => { + let impl = getImpl(instance); + seedGatekeeper(impl, 1); + seedRestrictedObservation(impl, 1, 100); + impl.storage.containsRestrictedData.put(true); + seedShareLink(impl); + + // No collaborator yet, but the link's keys are multi-redeemable and redemption is gated + // only while the record exists. + expect(impl.removalBlockedByRestrictedData(1, await impl.getSharingManager())).toBe(true); + }); + }); + + it("does not block a latched producer while the workspace is unshared", async () => { + let stub = env.TEST_OVERSEER.getByName("producer-removal-unshared"); + await runInDurableObject(stub, async (instance: OverseerDurableObject) => { + let impl = getImpl(instance); + seedGatekeeper(impl, 1); + seedRestrictedObservation(impl, 1, 100); + impl.storage.containsRestrictedData.put(true); + + expect(impl.removalBlockedByRestrictedData(1, await impl.getSharingManager())).toBe(false); + }); + }); + + it("falls back to guarding every connection when the latch is set with no producer", async () => { + let stub = env.TEST_OVERSEER.getByName("producer-removal-empty-producers"); + await runInDurableObject(stub, async (instance: OverseerDurableObject) => { + let impl = getImpl(instance); + seedGatekeeper(impl, 1); + // Should be impossible (the latch and its action record are written together), so fail + // closed: with the latch set and no derivable producer set, everything is guarded. + impl.storage.containsRestrictedData.put(true); + seedCollaborator(impl); + + expect(impl.removalBlockedByRestrictedData(1, await impl.getSharingManager())).toBe(true); + }); + }); +}); + +// GatekeeperClientImpl.remove() must make its decision against sharing state read *after* its +// only real yield (the cold sharing manager's whoami RPC) and in the same synchronous block as +// the delete: a grant landing during the yield is seen by the check, and nothing can land +// between the check and the delete. Pinned by parking whoami on a deferred so the yield is a +// real in-test suspension point. +describe("GatekeeperClientImpl.remove ordering", () => { + it("sees a collaborator granted while the sharing manager is being fetched", async () => { + let stub = env.TEST_OVERSEER.getByName("producer-removal-mid-yield-grant"); + await runInDurableObject(stub, async (instance: OverseerDurableObject) => { + // Not getImpl(): impl.ownerProfileId stays unset so getSharingManager() must fetch the + // owner profile through the (stubbed) owner User DO -- the parked deferred below. + let impl = (instance as unknown as { impl: any }).impl; + let releaseWhoami!: (profile: { id: string; name: string }) => void; + let whoami = new Promise<{ id: string; name: string }>(resolve => { + releaseWhoami = resolve; + }); + impl.ownerId = "owner-do-id"; + impl.users = { + idFromString: (id: string) => id, + get: () => ({ whoami: () => whoami }), + }; + impl.getGatekeeperFacet = () => ({ + describe: async () => ({ title: "Producer", url: "test://producer" }), + }); + + // A real client for a real record, so the test drives the actual remove() path. + let client = await impl.addGatekeeper({} as any, { + type: "gatekeeper", + vendorId: "testvendor", + resourceUrl: "https://example.com/producer", + typeUrlPattern: "https://*", + }); + let id = await client.getId(); + seedRestrictedObservation(impl, id, 100); + impl.storage.containsRestrictedData.put(true); + + // Start the removal: it runs synchronously up to the parked whoami. Grant a collaborator + // mid-park, then release -- the decision must see the grant and refuse. + let removal = client.remove(); + seedCollaborator(impl); + releaseWhoami({ id: OWNER, name: "Alice" }); + + await expect(removal).rejects.toThrow(/cannot be removed/); + expect(impl.storage.gatekeepers.get(id)).toBeDefined(); + }); + }); +}); + +// The ambient reconciliation removes a capsule record whose account is gone or was replaced -- +// an internal removal that must consult the same guard: a stale record that is a restricted +// producer still anchors collaborator verification. +describe("ensureAmbientCapsules reconciliation", () => { + const AMBIENT_ID = 1; + + // Seeds a stale ambient producer (record bound to accountId 10, owner now holding accountId + // 20) plus the latch, and fakes the owner's User DO and the gatekeeper facet so + // ensureAmbientCapsules can run without any real cross-DO call. + function seedStaleAmbientProducer(impl: any): void { + impl.storage.gatekeepers.put({ + id: AMBIENT_ID, + resourceTitle: "Test Ambient", + class: {} as any, + creationSpec: { type: "ambient", vendorId: "testvendor", accountId: 10 }, + }); + // Keep freshly-provisioned records clear of the seeded id. + impl.storage.nextGatekeeperId.put(10); + seedRestrictedObservation(impl, AMBIENT_ID, 100); + impl.storage.containsRestrictedData.put(true); + + impl.ownerId = "owner-do-id"; + impl.users = { + idFromString: (id: string) => id, + get: () => ({ + listProvidedAccounts: async () => [{ + vendorId: "testvendor", + accountId: 20, + description: { singleton: { tsType: "TestThing" } }, + }], + getSingletonGatekeeperClass: async () => ({} as any), + }), + }; + impl.getGatekeeperFacet = () => ({ + describe: async () => ({ title: "Test Ambient", url: "test://ambient" }), + }); + } + + function ambientRecords(impl: any): { id: number; accountId: number }[] { + return [...impl.storage.gatekeepers.list()] + .filter((gk: any) => gk.creationSpec?.type === "ambient") + .map((gk: any) => ({ id: gk.id, accountId: gk.creationSpec.accountId })); + } + + it("keeps a guarded stale producer and still provisions the replacement", async () => { + let stub = env.TEST_OVERSEER.getByName("ambient-reconcile-guarded"); + await runInDurableObject(stub, async (instance: OverseerDurableObject) => { + let impl = getImpl(instance); + seedStaleAmbientProducer(impl); + seedCollaborator(impl); + + await impl.ensureAmbientCapsules(); + + // The stale record anchors the collaborator's verification, so it survives; the + // replacement account still gets its own fresh capsule record. + let records = ambientRecords(impl); + expect(records).toContainEqual({ id: AMBIENT_ID, accountId: 10 }); + expect(records.filter(r => r.accountId === 20)).toHaveLength(1); + }); + }); + + it("still reconciles a stale producer away while the workspace is unshared", async () => { + let stub = env.TEST_OVERSEER.getByName("ambient-reconcile-unshared"); + await runInDurableObject(stub, async (instance: OverseerDurableObject) => { + let impl = getImpl(instance); + seedStaleAmbientProducer(impl); + + await impl.ensureAmbientCapsules(); + + let records = ambientRecords(impl); + expect(records.find(r => r.id === AMBIENT_ID)).toBeUndefined(); + expect(records.filter(r => r.accountId === 20)).toHaveLength(1); + }); + }); +}); + +// assertNewSharingAllowed() must refuse every new grant once a restricted producer cannot verify +// collaborators -- whether its record is gone, is legacy (no creationSpec; observerVendorId +// throws, so recipients hard-deny at open while the grant blocks producer removal), or is an +// aiModel/agentSpawner producer with no vendor account (filtered out of every verification scope, +// so recipients would open completely unverified and read the restricted history in chat). +describe("assertNewSharingAllowed", () => { + it("allows sharing while a verifiable producer's record survives", async () => { + let stub = env.TEST_OVERSEER.getByName("sharing-allowed-verifiable"); + await runInDurableObject(stub, async (instance: OverseerDurableObject) => { + let impl = getImpl(instance); + seedGatekeeper(impl, 1); + seedRestrictedObservation(impl, 1, 100); + impl.storage.containsRestrictedData.put(true); + + expect(() => impl.assertNewSharingAllowed()).not.toThrow(); + }); + }); + + it("refuses when a producer's record has been removed", async () => { + let stub = env.TEST_OVERSEER.getByName("sharing-allowed-removed"); + await runInDurableObject(stub, async (instance: OverseerDurableObject) => { + let impl = getImpl(instance); + seedRestrictedObservation(impl, 1, 100); + impl.storage.containsRestrictedData.put(true); + + expect(() => impl.assertNewSharingAllowed()).toThrow(/has since been removed/); + }); + }); + + it("refuses a legacy producer that cannot verify collaborators", async () => { + let stub = env.TEST_OVERSEER.getByName("sharing-allowed-legacy"); + await runInDurableObject(stub, async (instance: OverseerDurableObject) => { + let impl = getImpl(instance); + seedGatekeeper(impl, 1, /* creationSpec */ false); + seedRestrictedObservation(impl, 1, 100); + impl.storage.containsRestrictedData.put(true); + + expect(() => impl.assertNewSharingAllowed()).toThrow(/cannot verify collaborators/); + }); + }); + + it("refuses an aiModel producer that cannot verify collaborators", async () => { + let stub = env.TEST_OVERSEER.getByName("sharing-allowed-ai-model"); + await runInDurableObject(stub, async (instance: OverseerDurableObject) => { + let impl = getImpl(instance); + impl.storage.gatekeepers.put({ + id: 1, + resourceTitle: "AI model", + class: {} as any, + creationSpec: { + type: "aiModel", modelId: "m-1", provider: "anthropic", modelName: "claude-sonnet-5", + }, + }); + seedRestrictedObservation(impl, 1, 100); + impl.storage.containsRestrictedData.put(true); + + // No vendor account stands behind the producer (observerVendorId returns null), so no + // recipient could ever be verified against it. + expect(() => impl.assertNewSharingAllowed()).toThrow(/cannot verify collaborators/); + }); + }); + + it("never refuses while the workspace is unlatched", async () => { + let stub = env.TEST_OVERSEER.getByName("sharing-allowed-unlatched"); + await runInDurableObject(stub, async (instance: OverseerDurableObject) => { + let impl = getImpl(instance); + // Even a restricted-looking observation through a missing record does not refuse without + // the latch: the latch and the record are written together, so unlatched means none. + seedRestrictedObservation(impl, 1, 100); + + expect(() => impl.assertNewSharingAllowed()).not.toThrow(); + }); + }); +}); diff --git a/packages/workshop-backend/__tests__/sharing.test.ts b/packages/workshop-backend/__tests__/sharing.test.ts index 720908f02..a55b9f782 100644 --- a/packages/workshop-backend/__tests__/sharing.test.ts +++ b/packages/workshop-backend/__tests__/sharing.test.ts @@ -96,27 +96,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", () => { @@ -190,6 +169,52 @@ describe("redeemShareKey", () => { }); expect(storage.collaborators.get("a")).toBeUndefined(); }); + + 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-existing edge", async () => { + let { mgr } = makeManager(); + let { key } = await mgr.createShareLink({ caller: owner, role: "build" }); + await mgr.redeemShareKey({ + rawKey: key, profileId: "a", fetchProfile: async () => profile("a"), + }); + + // An existing 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.toBeUndefined(); + expect(mgr.getEffectiveRole("a")).toBe("build"); + }); + + it("invokes a passing assertGrantAllowed once and writes the edge", async () => { + let { storage, mgr } = makeManager(); + let { key, linkId } = await mgr.createShareLink({ caller: owner, role: "build" }); + + let calls = 0; + await mgr.redeemShareKey({ + rawKey: key, profileId: "a", fetchProfile: async () => profile("a"), + assertGrantAllowed: () => { calls++; }, + }); + + expect(calls).toBe(1); + expect(storage.collaborators.get("a")!.addedBy).toEqual([ + expect.objectContaining({ type: "shareKey", keyId: linkId }), + ]); + expect(mgr.getEffectiveRole("a")).toBe("build"); + }); }); describe("addCollaborator", () => { @@ -501,6 +526,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", () => { @@ -562,6 +607,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" }); diff --git a/packages/workshop-backend/src/overseer.ts b/packages/workshop-backend/src/overseer.ts index 889573a16..0a1b9df59 100644 --- a/packages/workshop-backend/src/overseer.ts +++ b/packages/workshop-backend/src/overseer.ts @@ -9,7 +9,7 @@ import { DurableObject, WorkerEntrypoint, RpcStub as NativeRpcStub, RpcTarget as NativeRpcTarget, restore, } from "cloudflare:workers"; -import { createTypedStorage, collection, keyString } from "@gadgets/typed-storage"; +import { createTypedStorage, collection, singleton, keyString } from "@gadgets/typed-storage"; import type { ListOptions } from "@gadgets/typed-storage"; import { GitStore, commitIdentityForAuthor, filesEqual, gitObjectsCollection, threeWayMerge } from "./git-store"; @@ -42,7 +42,8 @@ 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"; @@ -396,8 +397,13 @@ function fallbackBindingName(base: string, isTaken: (name: string) => boolean): function observerVendorId(record: GatekeeperRecord): string | null { if (!record.creationSpec) { + // There is no reconnect affordance for a legacy record (it never persisted its vendor + // identity), so the message points at the two real remedies: the owner removing the + // connection (allowed only while unshared), or moving the work to a new workspace. throw new Error( - "This workspace has a legacy connection that must be reconnected by its owner before it can be shared."); + "This workspace has a legacy connection that cannot verify collaborators' access. Its " + + "owner must remove the connection before the workspace can be shared, or start a new " + + "workspace."); } return "vendorId" in record.creationSpec ? record.creationSpec.vendorId : null; } @@ -1025,9 +1031,10 @@ 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`. - prohibitAllSharing: false, + // 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. The key on disk predates the flag's rename. + containsRestrictedData: singleton(false, {storageKey: "prohibitAllSharing"}), }, collections: { @@ -1394,6 +1401,21 @@ export function sanitizeMessageFormatRefs( return accepted.toSorted((a, b) => a.position - b.position); } +// Action records that predate the flag's rename carry `containsRestrictedData` under its old +// name, `prohibitAllSharing`. Records are data at rest and are never rewritten, so 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; @@ -2210,7 +2232,8 @@ class OverseerImpl implements AgentHooks { } throw new Error(`There is already a binding named "${name}".`); } - if (!this.storage.gatekeepers.get(target)) { + let targetRecord = this.storage.gatekeepers.get(target); + if (!targetRecord) { if (this.storage.gadgets.get(target)) { throw new Error(`Gadget-to-gadget bindings are not supported yet.`); } @@ -2221,6 +2244,14 @@ class OverseerImpl implements AgentHooks { // The gadget's env changed, so its code must reload. this.bumpVersion([gadgetId]); + + // A permanent edge puts an account-requiring connection into every "use" collaborator's + // verification scope (#gadgetBoundGatekeeperIds), since the gadget UI they drive can now + // invoke it. A pending edge is invisible to them until it's promoted, which restarts then. + if (chatId === undefined && targetRecord.creationSpec && + "vendorId" in targetRecord.creationSpec) { + this.#restartIfShared("Gadget restarted because a connection was bound to a gadget."); + } } // Remove the named binding edge from the gadget. The target gatekeeper itself survives, @@ -3644,6 +3675,12 @@ class OverseerImpl implements AgentHooks { throw new Error("The chat's code is being actively edited; please retry."); } + // Promotion below can widen every "use" collaborator's verification scope, so snapshot the + // scope first and compare after. Comparing the effective scope rather than restarting on any + // promotion matters because most merges promote neither: a gadget with no bindings, or an edge + // to a vendorless connection, is in nobody's verification scope. + let useScopeBefore = this.#accountRequiringUseScope(); + // Promote provisional gadgets whose creation is covered by this merge: accepting the chat's // changes through `mergeThrough` makes them permanent workspace members. Each covered // creation sits on an unmerged, unreverted "changes" message at `pending.sequence` (a @@ -3676,6 +3713,10 @@ class OverseerImpl implements AgentHooks { } } + // Did the promotions above actually bring an account-requiring connection into "use" scope? + let widenedUseScope = + [...this.#accountRequiringUseScope()].some(id => !useScopeBefore.has(id)); + // Fast-forward each committed gadget's head. for (let {gadgetId, commitId} of commits) { let record = this.storage.gadgets.get(gadgetId)!; @@ -3764,6 +3805,13 @@ class OverseerImpl implements AgentHooks { interaction_type: "code_merged", }); + // Sever live sessions whose verification scope the promotions widened, now that the writes + // above have landed: a "use" collaborator's session was admitted against the narrower scope, + // and the gadget UI they drive can now invoke a connection nobody verified them against. + if (widenedUseScope) { + this.#restartIfShared("Gadget restarted because accepted changes added gadget bindings."); + } + return {outcome: "merged"}; } @@ -4258,13 +4306,15 @@ class OverseerImpl implements AgentHooks { } } - getGatekeeperFacet(id: number): Fetcher> { + // `cls` is for the one caller that has the class in hand but has deliberately not published the + // record yet (`addGatekeeper`); everyone else resolves it from the record. + getGatekeeperFacet(id: number, cls?: GatekeeperClass): Fetcher> { return this.ctx.facets.get(`gatekeeper${id}`, async () => { - let cls = this.storage.gatekeepers.get(id)?.class; - if (!cls) { + let resolved = cls ?? this.storage.gatekeepers.get(id)?.class; + if (!resolved) { throw new Error("no such gatekeeper?"); } - return {class: cls}; + return {class: resolved}; }); } @@ -4339,9 +4389,14 @@ class OverseerImpl implements AgentHooks { class: cls, creationSpec, }; - this.storage.gatekeepers.put(gatekeeperRecord); - let facet = this.getGatekeeperFacet(id); + // The record is published only once, below, after describe() resolves -- the facet takes the + // class directly so it needs no record to exist yet. Publishing it before the await instead + // would expose the connection for as long as describe() takes, which is entirely before + // #restartIfShared severs the sessions that were never verified against it: the DO's input gate + // is open across the await, ids are allocated sequentially, so a live build session can guess + // this one, and getGatekeeperById (OverseerClientInterface) gates on nothing but existence. + let facet = this.getGatekeeperFacet(id, cls); try { let description = await facet.describe(); gatekeeperRecord.resourceTitle = description.title; @@ -4349,10 +4404,20 @@ class OverseerImpl implements AgentHooks { gatekeeperRecord.hasSlashCommands = description.hasSlashCommands; this.storage.gatekeepers.put(gatekeeperRecord); } catch (error) { + // Still the right teardown with nothing published: it deletes the facet we just created, and + // deleting an unwritten record is a no-op. this.removeGatekeeper(id); throw error; } + // A new account-requiring connection is in every "build" collaborator's verification scope + // immediately -- a live build session can getGatekeeperById() and openSession() on it with no + // observer check -- so sever those sessions. A vendorless spec (aiModel/agentSpawner) is in + // nobody's scope (#inScopeGatekeepers skips it), so it widens nothing. + if (creationSpec && "vendorId" in creationSpec) { + this.#restartIfShared("Gadget restarted because a new connection was added."); + } + return new GatekeeperClientImpl(this, id, facet); } @@ -4444,17 +4509,6 @@ 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); - } - // Forward exclusion: the gatekeeper may name observers who must not see this observation. Since // v1 has no per-thread hiding, the only way to let such an observation proceed is if the named // observer has already lost access in the sharing graph. If any named observer is still @@ -4464,6 +4518,31 @@ class OverseerImpl implements AgentHooks { await this.#enforceExcludeObservers(description.excludeObservers); } + if (description.containsRestrictedData) { + // Resolved here rather than up front: on a cold DO this is an RPC to the owner's User DO, + // and an ordinary unrestricted observation must not pay for it. The producer record is read + // *after* that await, so the check below and the latch are one synchronous block -- a record + // read before the await could be stale by the time it is checked, and latching against a + // stale one permanently bricks sharing. + let sharing = await this.getSharingManager(); + let producer = this.storage.gatekeepers.get(gatekeeperId); + + // An in-flight facet RPC can outlive removeGatekeeper, so a restricted observation can + // arrive naming a connection this workspace no longer has. Latching a missing producer id + // permanently bricks sharing (assertNewSharingAllowed's missing-record branch), so refuse + // the read instead -- including on an unshared workspace, where nothing else would stop it. + // This same read is what refuses a connection removed during the exclusion awaits above, + // where the latch is not yet set and so removalBlockedByRestrictedData does not yet protect + // the producer. + if (!producer) { + throw new Error( + "This observation was blocked because it contains sensitive data, but the " + + "connection it was read through has been removed from this workspace."); + } + this.#assertUnverifiableProducerUnshared(producer, sharing); + this.storage.containsRestrictedData.put(true); + } + let actionId = this.storage.nextActionId.get(); this.storage.nextActionId.put(actionId + 1); @@ -4578,6 +4657,104 @@ class OverseerImpl implements AgentHooks { }); } + // Refuse a restricted observation from a producer nobody can ever be verified against: a + // gatekeeper with no vendor account behind it (aiModel/agentSpawner) or a legacy record with no + // creationSpec. Every *other* producer is enforced at admission -- a collaborator cannot open + // the workspace without passing addObserver() for it, and anything that widens what they must + // pass restarts every live session (see #restartIfShared) -- but #inScopeGatekeepers skips + // these, so no collaborator is ever asked about them and admission cannot see them at all. + // Consistent with assertNewSharingAllowed(), which treats the same case as unshareable. + // + // Deliberately synchronous (the sharing manager is a parameter, not an internal await) so the + // caller can check and latch in one synchronous block -- see authorizeObservation. + #assertUnverifiableProducerUnshared(gatekeeper: GatekeeperRecord, sharing: SharingManager): void { + if (sharing.listCollaborators().length === 0) return; + + let vendorId: string | null = null; + try { + vendorId = observerVendorId(gatekeeper); + } catch { + // Legacy connection with no creationSpec: treat as unverifiable. + } + if (vendorId !== null) return; + + // The message reaches sandboxed gadget code and agent output -- an audience that can't + // otherwise list collaborators -- so it reports only that the workspace is shared, naming + // neither the collaborators nor their profile ids (the full email on OAuth/CF Access + // deployments). + throw new Error( + "This observation was blocked because it contains sensitive data, but it was read " + + "through a connection that cannot verify anyone's access to that data, and this " + + "workspace is shared. Its collaborators must be removed before this data can be read."); + } + + // The connection ids this workspace has read restricted data through: the producers the latch + // guards. Derived by scanning the action log for observations whose description carries + // `containsRestrictedData`, since nothing else records which connection a latched read came + // through. + 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; + } + + // True if removing gatekeeper `id` is blocked because it anchors restricted-data verification: + // the workspace is latched, `id` is a restricted producer (or the producer set is unexpectedly + // empty -- see below), and the sharing graph still has collaborators or outstanding share + // links. Shared by GatekeeperClientImpl.remove() and the ambient reconciliation in + // ensureAmbientCapsules(): while the workspace is shared, deleting a producer's record would + // let a never-verified party see the data -- the record is what observer verification runs + // against at every open, and for an unverifiable record it is what refuses the producer's reads + // outright -- even though the restricted data outlives it in chat history and storage. + // + // Deliberately synchronous (the sharing manager is a parameter, not an internal await) so each + // caller can check and delete in one synchronous block -- see GatekeeperClientImpl.remove(). + removalBlockedByRestrictedData(id: WorkpieceId, sharing: SharingManager): boolean { + if (!this.storage.containsRestrictedData.get()) return false; + // 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.restrictedProducerIds(); + if (producers.size > 0 && !producers.has(id)) return false; + return sharing.listCollaborators().length > 0 || sharing.listShareLinkRecords().length > 0; + } + + // Refuse a new sharing grant once the workspace has read restricted data through a connection + // that can no longer verify a recipient's access to it -- one that has since been removed, or + // that never had a vendor account behind it. Every other producer verifies its collaborators at + // each open, so sharing stays available. + assertNewSharingAllowed(): void { + if (!this.storage.containsRestrictedData.get()) return; + for (let id of this.restrictedProducerIds()) { + let producer = this.storage.gatekeepers.get(id); + if (!producer) { + 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."); + } + let vendorId: string | null = null; + try { + vendorId = observerVendorId(producer); + } catch { + // Legacy connection with no creationSpec: treat as unverifiable. + } + if (vendorId === null) { + throw new Error( + "This workspace can no longer be shared: it read sensitive data through a connection " + + "that cannot verify collaborators' 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. @@ -4608,6 +4785,9 @@ class OverseerImpl implements AgentHooks { for (let observerId of observerIds) { let observer = this.storage.observers.byObserverId.get(observerId); if (!observer) continue; + // TODO: This deletes by profileId from a snapshot that can go stale across the awaited + // removeObserver fan-out, so a re-granted profile's *replacement* observer record can be + // deleted here, after which exclusions naming the new id silently no-op fail-open. this.storage.observers.delete(observer.profileId); await this.#removeObserverFromGatekeepers(observerId, gatekeeperIds); } @@ -4616,7 +4796,7 @@ class OverseerImpl implements AgentHooks { // Provides web-fetch with the Workers AI binding and AI Gateway config it needs to call // `env.WORKERS_AI.toMarkdown()`. The initiator is needed for AI Gateway metadata. getWebFetchEnv(): WebFetchEnv { - if (this.storage.prohibitAllSharing.get()) { + if (this.storage.containsRestrictedData.get()) { // TODO: Disallwing fetches is a bit draconian. Ideally, we would have some way to detect // if a URL is well-known, and therefore not a leak problem. E.g. if the URL is already in // a search index, then it's not leaking anything. If we had a search provider we could @@ -4665,7 +4845,7 @@ class OverseerImpl implements AgentHooks { async submitAction(gatekeeperId: number, action: number, description: ActionDescription, caller: GatekeeperCaller) : Promise { - if (this.storage.prohibitAllSharing.get()) { + if (this.storage.containsRestrictedData.get()) { throw new Error( "This workspace has observed sensitive data. To prevent leaks, the workspace is prohibited " + "from performing actions."); @@ -4848,7 +5028,7 @@ class OverseerImpl implements AgentHooks { // User DO ids whose outputs index this workspace is keeping live, one token per open session. // // In memory, not persisted, which is what makes fanning out to collaborators safe: revoking - // access aborts the DO (see scheduleRevocationRestart()), so this is destroyed with the sessions + // access aborts the DO (see scheduleAccessRestart()), so this is destroyed with the sessions // it describes and can only be rebuilt by an open() that re-checks the permission graph. #connectedIndexes = new Map>(); @@ -4975,30 +5155,61 @@ class OverseerImpl implements AgentHooks { return codeVersion; } - // Force every client to disconnect and re-authenticate after a collaborator has been removed or - // downgraded, so that someone who just lost access can't keep using a session that's already - // open. Authorization is only checked at open() (see the sharing docs), so without this a stale - // session would survive until something else happened to disconnect it. + // Force every client to disconnect and re-authenticate, so that no session outlives a change to + // what its holder is entitled to. Both checks that gate a session run only at open() (see the + // sharing docs), so without this a stale session would survive until something else happened to + // disconnect it. Two kinds of change need it: + // - Access removed or downgraded (removeCollaborator, revokeShareLink, workspace deletion): + // someone who just lost access could keep using the session they already have. + // - Verification scope widened (see #restartIfShared): a collaborator's live session was + // verified against a smaller set of gatekeepers than the workspace now holds. // // We restart by aborting the whole DO. Aborting propagates to clients: the `notifyClosed` stub // handed to each session is disposed without being called, which AuthenticatedApiImpl detects // and reacts to by killing the browser WebSocket, forcing a reconnect that re-runs open() and - // re-checks the (now-changed) permission graph. Removing/downgrading collaborators is rare, so - // the disruption is acceptable -- and DOs restart unpredictably anyway, so reconnects need to - // be made as painless as possible regardless. + // re-checks the (now-changed) permission graph. These events are rare, so the disruption is + // acceptable -- and DOs restart unpredictably anyway, so reconnects need to be made as painless + // as possible regardless. // // Two precautions before the abort: - // - `ctx.abort()` does not respect the output gate, so we explicitly flush the severed edge to - // disk with `ctx.storage.sync()`. Otherwise a restart could come back with the change lost, - // leaving the removed user still authorized. + // - `ctx.abort()` does not respect the output gate, so we explicitly flush the triggering change + // to disk with `ctx.storage.sync()`. Otherwise a restart could come back with the change lost, + // leaving the removed user still authorized (or the widened scope unrecorded). By the same + // token, callers must schedule the restart *after* the write that triggered it, never before + // further writes in the same turn -- those would be racing the abort. // - We delay the abort briefly so the triggering RPC's response can reach the caller (typically // the owner, who is also connected and will be disconnected) before their connection drops. // Without the delay their own removeCollaborator()/revokeShareLink() call might reject with a // connection error even though it succeeded. - async scheduleRevocationRestart(): Promise { + async scheduleAccessRestart(reason: string): Promise { await this.ctx.storage.sync(); await scheduler.wait(100); - this.ctx.abort("Gadget restarted to revoke access for a removed collaborator."); + this.ctx.abort(reason); + } + + // Sessions are authorized and verified only at open(), so widening what a live session's holder + // must be verified against leaves that session holding unverified access. Restart everyone -- + // the same mechanism used when access is revoked -- so each client's next open() re-runs + // authorizeCollaborator/ensureObserver against the new scope. No-op when the workspace has no + // collaborators: the owner is never an observer, so there is nobody to re-verify and no reason + // to disturb the one session that exists. + // + // Fire-and-forget: the callers are synchronous (bindWorkpiece) or already past their last write, + // and getSharingManager() is async, so failures are logged rather than left as an unhandled + // rejection. Failing to restart is fail-open for the widened scope, hence the `error` level. + // + // Note that ensureAmbientCapsules() calls addGatekeeper() from inside open(), so on a shared + // workspace the first open after an ambient capsule appears bounces itself once; the capsule + // exists by then, so the client's retry is clean. + #restartIfShared(reason: string): void { + this.getSharingManager().then(sharing => { + if (sharing.listCollaborators().length === 0) return; + return this.scheduleAccessRestart(reason); + }).catch(error => { + this.logger.error("failed to restart sessions after verification scope widened", { + event: "workspace.scope.restart.failed", error, + }); + }); } // Last timestamp generated by getChatTimestamp(), if it has been called during this session. @@ -6187,11 +6398,14 @@ class OverseerImpl implements AgentHooks { // single round trip both provisions them and reads them back before we wire up capsules. let accounts = (await ownerDo.listProvidedAccounts()) .filter(account => account.description.singleton?.tsType); + let sharing = await this.getSharingManager(); // Reconcile existing ambient capsule records against the owner's current singleton accounts. Each // record is keyed to a specific accountId; if that account is gone (disconnected) or was replaced // (an optional account removed and re-added with a new accountId), the record is stale and would - // point the capsule at a deleted account — so remove it. Snapshot the list since we mutate it. + // point the capsule at a deleted account — so remove it. With the sharing manager fetched above, + // the loop is fully synchronous: each removal-blocked check runs in the same synchronous block as + // the delete it gates, and the snapshot below cannot go stale mid-iteration. let currentAccountId = new Map(accounts.map(account => [account.vendorId, account.accountId])); let bound = new Set(); // Snapshot before iterating, since removeGatekeeper() mutates the collection. @@ -6200,6 +6414,18 @@ class OverseerImpl implements AgentHooks { if (gk.creationSpec?.type !== "ambient") continue; if (currentAccountId.get(gk.creationSpec.vendorId) === gk.creationSpec.accountId) { bound.add(gk.creationSpec.vendorId); + } else if (this.removalBlockedByRestrictedData(gk.id, sharing)) { + // A stale ambient record that anchors restricted-data verification must survive until + // the owner unshares -- deleting it here would be the same unchecked readmission + // GatekeeperClientImpl.remove() guards against, minus the user intent. Not added to + // `bound`, so a replacement account still gets a fresh capsule record; + // prepareChatBindings tolerates the duplicate vendor (names dedupe via the fallback + // binding name, and the dead record's session just fails). + this.logger.warn("skipping removal of stale ambient restricted producer", { + event: "singleton.capsules.reconcile.blocked", + gatekeeperId: gk.id, + vendorId: gk.creationSpec.vendorId, + }); } else { this.removeGatekeeper(gk.id); } @@ -7757,23 +7983,49 @@ 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 account-requiring subset of #gadgetBoundGatekeeperIds(): exactly what a "use" collaborator + // is verified against, as an id set two states can be compared by (see mergeChatChanges). + // + // Uses the non-throwing gatekeeperVendorId() rather than #inScopeGatekeepers("use"), whose + // observerVendorId() throws on a legacy record with no creationSpec: an unrelated legacy + // connection must not turn a caller's ordinary bookkeeping into an error. + #accountRequiringUseScope(): Set { + let ids = new Set(); + for (let id of this.#gadgetBoundGatekeeperIds()) { + if (gatekeeperVendorId(this.storage.gatekeepers.get(id))) ids.add(id); + } + return ids; + } + // 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. + // + // TODO(known-risk): scoping by role means a "use" collaborator is never verified against a + // producer that no gadget binds -- yet restricted data read from such a producer can reach + // gadget state and the UI they drive, because provenance is not tracked past the observation. + // Deliberately accepted for v1; the required fix (verify every restricted producer, or isolate + // restricted data by provenance) is recorded under "Known security risk -- never-bound + // producers" in plans/restricted-data-sharing.md, and worked through in docs/observers.md + // edge case 4. #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()) { @@ -7790,8 +8042,8 @@ class OverseerImpl implements AgentHooks { // Best-effort `removeObserver(observerId)` across the given gatekeeper ids. Never throws; logs // and continues on error. An orphaned observer entry only ever causes superfluous future checks, - // never a data leak (the leak-relevant gate is authorizeObservation, which keys off the live - // sharing graph). + // never a data leak: a registration is what admits an open, and every open re-runs addObserver, + // so a stale one grants nothing on its own. async #removeObserverFromGatekeepers(observerId: string, gatekeeperIds: number[]): Promise { await Promise.all(gatekeeperIds.map(async id => { try { @@ -7808,8 +8060,8 @@ class OverseerImpl implements AgentHooks { // For each affected collaborator who is now fully unauthorized (newRole === null) and has an // observer record: best-effort removeObserver on all gatekeeper facets, then delete the record. // All calls are best-effort -- an orphaned observer entry only causes superfluous future checks, - // never a data leak (the leak-relevant gate is authorizeObservation, keyed off the live sharing - // graph). See observers-implementation-plan.md §5 Step 6. + // never a data leak: a registration is what admits an open, and every open re-runs addObserver, + // so a stale one grants nothing on its own. See observers-implementation-plan.md §5 Step 6. async tearDownLostObservers(affected: AffectedCollaborator[]): Promise { let gatekeeperIds = [...this.storage.gatekeepers.list()].map(gk => gk.id); for (let entry of affected) { @@ -7849,6 +8101,30 @@ 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. + async authorizeCollaborator( + profileId: string, + clientUser: DurableObjectStub, + opts: { + configureCb?: RpcStub; + requireRole?: CollaboratorRole; + } = {}): Promise { + let sharing = await this.getSharingManager(); + let role = sharing.getEffectiveRole(profileId); + if (!role || (opts.requireRole && roleRank(role) < roleRank(opts.requireRole))) return null; + + await this.ensureObserver(profileId, clientUser, role, opts.configureCb); + return 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 @@ -7856,19 +8132,40 @@ class OverseerImpl implements AgentHooks { // resource access promptly. Returns when fully verified; throws to deny access. // // See observers-implementation-plan.md §5 Step 3. + // + // TODO: Concurrent opens by the same profile race this method -- two calls mint two observerIds + // and the last-written record forgets the other's gatekeeper registrations -- so verification + // needs to be serialized per profile. async ensureObserver( profileId: string, clientUser: DurableObjectStub, role: CollaboratorRole, configureCb?: RpcStub): Promise { - // 1. Select in-scope gatekeepers. If none require an account, there is nothing to verify and - // no observer record is needed (built-in gatekeepers never name observers in - // excludeObservers). + // 1. Select in-scope gatekeepers. If none require an account, there is nothing to verify + // (built-in gatekeepers never name observers in excludeObservers). let inScope = this.#inScopeGatekeepers(role); - if (inScope.length === 0) return; - // 2. Load any existing observer record, and build a working copy of its account choices. + // 2. Load any existing observer record, and prune every account choice for a gatekeeper now + // outside this collaborator's verification scope, keeping the record an accurate statement + // of what their most recent open verified: entry present => verified at that open. + // Rebinding a connection keeps its gatekeeper id, so a stale entry from before an unbind + // would otherwise re-register them off a choice made for a scope the workspace no longer + // has, instead of asking them again. let record = this.storage.observers.get(profileId); + if (record) { + let inScopeIds = new Set(inScope.map(gk => gk.id)); + let pruned = false; + for (let key of Object.keys(record.accountChoices)) { + if (!inScopeIds.has(Number(key))) { + delete record.accountChoices[Number(key)]; + pruned = true; + } + } + if (pruned) this.storage.observers.put(record); + } + if (inScope.length === 0) return; + + // Build a working copy of the (pruned) account choices. let accountChoices: {[gatekeeperId: number]: number} = {...record?.accountChoices}; // Gatekeeper ids registered before this call (their account choice came from the persisted @@ -7878,8 +8175,18 @@ class OverseerImpl implements AgentHooks { inScope.filter(gk => gk.id in accountChoices).map(gk => gk.id)); let observerId = record?.observerId ?? crypto.randomUUID(); + // Whether this collaborator was already an admitted observer when the call began. A returning + // observer's `observerId` is already persisted, so it stays resolvable no matter how this call + // ends -- which is what makes keeping their registrations on a failure safe (see the catch). + let returningObserver = record !== undefined; // 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 and have not verified + // since + let invalidated = new Set(); + // Whether a failure scrubbed a previously-persisted account choice, i.e. this collaborator's + // verified coverage shrank. See the catch below. + let scrubbedCoverage = false; // Failures from the previous pass, keyed by gatekeeper id: an already-configured binding whose // chosen account was disconnected, or which the gatekeeper refused. @@ -7970,25 +8277,45 @@ class OverseerImpl implements AgentHooks { let fail = (reason: string, err?: unknown) => { failures.set(gk.id, {accountId, reason}); + // The persisted record is what asserts this collaborator was verified for this + // producer, so scrub the failed gatekeeper from it: this open is not going to renew + // that assertion. Scrub synchronously with the failure determination, re-reading the + // record since the awaits since load may have let a concurrent open update it. + // 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); + // The scrub only rewrites what the record claims; the collaborator's other + // sessions are still open and still hold the access it used to justify. Note the + // shrink so the catch below can sever them. + scrubbedCoverage = true; + } this.logger.warn("observer verification failed", { event: "gatekeeper.observer.verify.failed", gatekeeperId: gk.id, vendorId, accountId, observerId, error: err, }); }; - let verifier = await clientUser.getVerifier(accountId, vendorId); - if (!verifier) { - // Account gone -> the overseer authors the reason. (Wrong vendor throws above.) - fail("This account is no longer connected."); - return; - } - try { + let verifier = await clientUser.getVerifier(accountId, vendorId); + if (!verifier) { + // Account gone -> the overseer authors the reason. (Wrong vendor throws above.) + fail("This account is no longer connected."); + return; + } await this.getGatekeeperFacet(gk.id).addObserver(observerId, verifier); if (!registeredBeforeCall.has(gk.id)) newlyAdded.add(gk.id); + // Keep `invalidated` meaning "failed and has not verified since": this binding just + // verified, so it no longer rests on a scrubbed choice. Its persisted coverage stays + // scrubbed until step 6, so an open that never gets there leaves the record claiming + // less than it did before -- never more. + invalidated.delete(gk.id); } catch (err) { // Either a settled denial or an operational failure (expired credentials, upstream - // outage). Treat every failure as repairable and let the user try again. + // outage) fail(stringifyError(err), err); } })); @@ -8023,9 +8350,40 @@ 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]); + // This open is being denied, but the collaborator may hold other sessions that opened while + // the scrubbed choice still verified them. Sever every session so they must re-verify -- + // whoever can't will simply be denied their next open. This cannot loop: a second identical + // failure finds the entry already scrubbed, so no flag and no restart. A re-prompt that + // repairs the failure never reaches here, and step 6 re-persists full coverage. + // + // Scheduled first, ahead of the equally best-effort rollback below: it is fire-and-forget, so + // ordering it first takes a gatekeeper RPC fan-out off the path between determining the denial + // and the abort, and the rollback then proceeds under the scheduled restart. + // + // TODO(known-limitation): a re-prompt the client never answers still defers this + // indefinitely, since `configureCb.configure()` is awaited above and the failure is only + // terminal once the re-prompt budget is spent. Stalling the modal is equivalent to never + // re-opening, which docs/observers.md edge case 3 already accepts, so it grants no access + // the collaborator doesn't already hold. See plans/restricted-data-sharing.md, Known + // limitations. + if (scrubbedCoverage) { + this.#restartIfShared( + "Gadget restarted because a collaborator failed to re-verify their access."); + } + + // Best-effort remove the observers we registered during *this* call, since we didn't persist + // the user's observer record. + // + // A first-ever verification rolls back the invalidated ones too: nothing referenced those + // registrations before this call, and the freshly-minted observerId is discarded with the + // unpersisted record, so anything left behind lingers unresolvable. + // + // A *returning* observer's registrations are kept instead. De-registering one is fail-open: + // the gatekeeper stops naming that observer in `excludeObservers`, so an observation it + // would have excluded them from is admitted with nothing left to block it. + let rollback = returningObserver ? newlyAdded : new Set([...newlyAdded, ...invalidated]); + await this.#removeObserverFromGatekeepers(observerId, [...rollback]); + throw err; } @@ -8318,13 +8676,6 @@ export class OverseerDurableObject extends DurableObject { let role: CollaboratorRole = "build"; if (!isOwner) { - if (this.impl.storage.prohibitAllSharing.get()) { - // `prohibitAllSharing` can only have been set when the gadget had no shares (see - // `authorizeObservation`), and no new shares can be created while it's set, so any - // non-owner reaching here is necessarily unauthorized. - throw createOpenGadgetError(OPEN_GADGET_ERROR_CODES.workspaceAccessDenied); - } - let sharing = await this.impl.getSharingManager(); // If a share key was provided, redeem it. The owner already has full access and should not @@ -8334,31 +8685,34 @@ export class OverseerDurableObject extends DurableObject { 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. + // 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; + + // 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. // // 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); + let effectiveRole = await this.impl.authorizeCollaborator( + profileId, clientUser, {configureCb: configureObservers}); 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); - // Fire-and-forget a call to the collaborator's user DO so the gadget appears on // (or is refreshed on) their home page. let title = this.impl.storage.title.get(); @@ -8430,15 +8784,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, @@ -9116,7 +9481,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.containsRestrictedData.get(), role: "build", defaultGadgetId: this.impl.defaultGadgetId, }; @@ -9135,7 +9500,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.containsRestrictedData.get(), role: "build", defaultGadgetId: this.impl.defaultGadgetId, }; @@ -9157,9 +9522,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); } }; @@ -9167,13 +9532,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.containsRestrictedData.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.containsRestrictedData.subscribe(restrictedDataSubscriber); callback(metadata).catch(unsubscribe); @@ -9307,7 +9672,7 @@ class OverseerClientInterface extends RpcTarget implements Overseer { await this.impl.ctx.blockConcurrencyWhile(async () => { await this.#owner.deleteGadget(this.impl.ctx.id.toString()); await this.impl.ctx.storage.deleteAll(); - this.impl.scheduleRevocationRestart(); + this.impl.scheduleAccessRestart("Gadget restarted because the workspace was deleted."); this.impl.ownerId = undefined; }); @@ -10383,8 +10748,10 @@ 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. Sharing stays + // available even after the workspace observes sensitive data (`containsRestrictedData`): + // access to that data is enforced per-gatekeeper by observer verification, not by blocking + // sharing wholesale. async listObserverRequirements( role: CollaboratorRole): Promise { @@ -10405,13 +10772,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, @@ -10436,7 +10802,8 @@ class OverseerClientInterface extends RpcTarget implements Overseer { // excluded). A no-op removal -- e.g. severing a share-link edge nobody relied on -- shouldn't // disconnect everyone. if (affected.length > 0) { - this.impl.scheduleRevocationRestart(); + this.impl.scheduleAccessRestart( + "Gadget restarted to revoke access for a removed collaborator."); } return affected; } @@ -10455,7 +10822,8 @@ class OverseerClientInterface extends RpcTarget implements Overseer { await this.impl.refreshAffectedCollaboratorListings(affected); // Only restart if someone actually lost access or was downgraded (see removeCollaborator). if (affected.length > 0) { - this.impl.scheduleRevocationRestart(); + this.impl.scheduleAccessRestart( + "Gadget restarted to revoke access for a revoked share link."); } return affected; } @@ -10464,25 +10832,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 { @@ -11143,7 +11506,24 @@ class GatekeeperClientImpl> } async remove(): Promise { + // 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. Only the producers themselves are guarded -- a non-producer connection + // anchors no restricted-data verification, so it stays removable while shared. + let sharing = await this.impl.getSharingManager(); + // Checked in the same synchronous block as the delete, after the only await (cf. + // addCollaborator): a check ahead of the yield could pass, a concurrent grant land during + // it, and the delete still run past it. let record = this.impl.storage.gatekeepers.get(this.id); + if (record && this.impl.removalBlockedByRestrictedData(this.id, sharing)) { + 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..88039e236 100644 --- a/packages/workshop-backend/src/sharing.ts +++ b/packages/workshop-backend/src/sharing.ts @@ -15,18 +15,22 @@ // re-adding a removed collaborator restores them and, transitively, everyone they had shared with. // (Records and revoked keys accumulate in storage; a future GC could reclaim long-dead entries.) // -// NOTE: The `prohibitAllSharing` policy flag intentionally does NOT live here. It is a broader -// "is this gadget allowed to communicate with anyone other than the owner?" policy (it also -// gates gatekeeper writes and web fetches) and is expected to grow into a separate policy engine. -// The Overseer enforces that flag; this module only exposes `hasAnyShares()` so the policy can -// ask about the current sharing state. +// NOTE: The sensitive-data (`containsRestrictedData`) policy intentionally does NOT live here. +// It is a broader "what may this gadget do after reading restricted data?" policy (it gates +// gatekeeper writes and web fetches, and requires per-gatekeeper observer verification of +// collaborators) and is expected to grow into a separate policy engine. The Overseer enforces +// it; this module only answers questions about the sharing graph. import { AiChatAuthorInfo, CollaboratorInfo, PermissionEdge, CollaboratorRole, AffectedCollaborator } from "@gadgets/workshop-shared/api"; 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 +160,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()) { @@ -215,11 +199,21 @@ export class SharingManager { * collaborators are redeemed without any RPC. * * A key whose link is revoked behaves like an unknown key (it cannot be redeemed). + * + * TODO: Redemption is one-step: the edge written here is real before the redeeming open()'s + * observer verification runs. Two accepted consequences, both fail-closed (availability, not + * confidentiality): an unverified redeemer is a current collaborator, so restricted reads + * block from redemption until they verify (or are removed, or the link is revoked); and a + * recipient whose verification is refused keeps the edge -- visible in listCollaborators, + * blocking restricted reads until removed. Two-phase redemption (a pending edge that grants + * nothing until verification confirms it) is the planned fix for both. */ async redeemShareKey(opts: { rawKey: string; profileId: string; fetchProfile: () => Promise; + /** See createShareLink: run synchronously with the put, a throw persists nothing. */ + assertGrantAllowed?: () => void; }): Promise { let hash = await hashShareKey(opts.rawKey); let keyRecord = this.storage.shareKeys.get(hash); @@ -236,10 +230,12 @@ export class SharingManager { 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). + // link (redeeming a second key of the same link is a no-op, so no new grant and no policy + // check). let alreadyHasEdge = existing.addedBy.some( e => e.type === "shareKey" && e.keyId === linkId); if (!alreadyHasEdge) { + opts.assertGrantAllowed?.(); existing.addedBy.push({ type: "shareKey", keyId: linkId, @@ -251,6 +247,7 @@ export class SharingManager { } else { // New collaborator -- need full profile from their user DO. let profile = await opts.fetchProfile(); + opts.assertGrantAllowed?.(); this.storage.collaborators.put({ profile, addedBy: [{ @@ -288,8 +285,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 +429,16 @@ 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. + */ + assertGrantAllowed?: () => void; + }) : Promise<{ key: string; linkId: string }> { let callerRole = this.#requireCallerRole(opts.caller); if (roleRank(opts.role) > roleRank(callerRole)) { @@ -441,6 +447,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 +459,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 +479,7 @@ export class SharingManager { } let { key, hash } = await this.#mintKey(); + opts.assertGrantAllowed?.(); this.storage.shareKeys.put({ id: hash, alias: link.id }); return { key }; } diff --git a/packages/workshop-frontend/src/ShareModal.tsx b/packages/workshop-frontend/src/ShareModal.tsx index b6212442d..e395e289e 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 || containsRestrictedData || 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 (containsRestrictedData || 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 (containsRestrictedData || copyingLinkRef.current) return copyingLinkRef.current = true setCopyingLinkId(linkId) try { @@ -775,7 +775,7 @@ 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 ? (
@@ -821,20 +821,20 @@ export default function ShareModal({ open, onClose, overseer, metadata, currentU data-bwignore="true" data-form-type="other" className="h-9 min-w-0 flex-1 appearance-none border-0 bg-transparent p-0 text-[14px] leading-5 tracking-[-0.25px] text-kumo-default outline-none placeholder:text-kumo-inactive disabled:cursor-not-allowed [&::-webkit-search-cancel-button]:hidden" - disabled={sharingProhibited} + disabled={containsRestrictedData} /> {adding ? 'Inviting…' : 'Invite'} @@ -911,16 +911,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 || containsRestrictedData} /> - + {creatingLink ? 'Creating…' : 'Create link'} setShowLinkComposer(false)}> @@ -932,7 +932,7 @@ export default function ShareModal({ open, onClose, overseer, metadata, currentU