Sync foundation/main (11 commits) + fork config fallout fixes - #29
Merged
Merged
Conversation
The gatekeeper asked Google for all tab content and then rejected any document with multiple or nested tabs, blocking both directly bound Docs and read-only Docs opened through Drive. Google's model is a recursive tab tree whose bodies have independent index spaces, so reads must traverse Document.tabs and every write Location/Range must carry the immutable tabId. The capability stays whole-document; tabId is only an operation target. No tab create, rename, move, duplicate or delete API is added. - docs-api.ts flattens the provider tree into a preorder adjacency list, deriving ancestry, sibling index and nesting level from the tree actually returned rather than trusting tabProperties, and requiring a globally unique non-empty tabId and a non-empty body. - markdown-converter.ts converts one tab at a time (DocTabSnapshot), since character indices restart per tab, and stamps the selected tabId on every insert location and delete/style/bullet/link range. - google.ts caches a per-tab snapshot, resolves a selector fail-closed (omission is legal only when the flattened list holds one tab, and an unknown ID never falls back to the first), replays the pending queue in global order but per tab, and scopes committed-marker suppression to the tab that owns the marker. - getContent/replaceText/appendText take an optional tabId; listTabs() is new on the read session shared by Drive. Agent guidance requires listing tabs before reading one. Both sessions reuse one revision for ten seconds and then recheck it, so concurrent reads share a fetch without pinning a long-lived Drive session to the revision it first saw. Google populates revisionId only for callers with edit access, so it is typed as optional and a cache is never confirmed by an absent one: a view-only Doc is refetched rather than spending a request on an answer that could not confirm it. A modification time needs the opposite default, since a document offering no change token must not look edited by every read, so the bound session reports Drive's modifiedTime for one. Only a refused grant falls back to the first observation, for an account whose grant predates the picker's metadata scope; a quota 403 (Drive rate-limits with that status too), an outage or a malformed body are raised instead, because dating a document from one would report it unchanged for as long as Drive stayed unhealthy and the stored observation would outlive the incident. Approval and observation text names a tab by ID as well as title: titles are user-authored, need not be unique, and may be empty, while the write targets the ID. An approved edit that cannot be applied now reports why, rather than being removed while the overseer records it as applied. Throwing is the documented applyAction contract: the record stays pending and the user is offered a retry or a discard. The edit is invalidated rather than removed, so later edits stop queuing behind it and a repeated approval repeats the reason instead of decaying into an unknown-action error; rejecting clears it. A pending edit stored before this change names no tab. The old code refused to read a document with more than one tab, so such an edit was approved against a document that had exactly one: it is retargeted while the document still holds a single tab, and invalidated only once tabs added since leave its target unknowable. Both the replay filter and the apply-time lookup search every tab for such an edit's marker, so a write that committed before the upgrade and lost its response is reconciled instead of being dropped with its named range orphaned in the document. A failed read or edit authorizes a generic observation before its error is thrown: the error distinguishes a live tab from a missing one, and for replaceText, present text from absent.
…conformance consumer (cloudflare#460) * feat(gatekeeper-kit): replayable runs with account-adjudicated expiry CredentialSource.run gains a replayable option: a credential rejection is retried once with credentials minted through a refreshCredentials channel, and only a rejection of those reports expiry — so derived-bearer ports stop reporting routine stale bearers as grant death. The flag without a channel throws at the call. The refresh is observed, never adopted: plain reads stay the snapshot's only writer, which removes the second concurrent writer earlier revisions fenced around. noteCredentialsExpired now returns the account's verdict on the reported identity — an explicit false resolves as the fixed retry message with the cache authority dropped; anything else (lost answers included) fails closed as accepted. The verdict is asked first, then clear and fence land as one synchronous transition, and a reconnect crossing the refresh fences any authority not adopted past it. Replays coalesce per rejected read via SingleFlight's new object keys. Plan §4.6/§4.13/§5.6/§5.8/§6 reconciled with the reduced model. * fix(gatekeeper-kit): enforce API invariants and document usage * fix(gatekeeper-kit)!: close the adoption gaps a consumer audit found A second audit read the kit as a new gatekeeper author would. Five places advertised a safety property the callbacks could not deliver, or narrowed a canonical contract until the high-level path had to be abandoned. - Actions distinguish a terminal failure known to have left no provider effect from one whose outcome is unknown, and the dependency cascade consults the resolution oracle apply already uses instead of retiring dependents whose reference the provider had bound. - `describe` can declare `pushedCommits` again, with the compiler forcing every future `ActionDescription` field into a kit-owned or provider-owned list. - Provider-backed cursors authorize the exact page they return, including one served from the buffer and the terminal answer of a walk that disclosed nothing -- a zero-result search is an existence oracle. - Observer strategies declare whether they can enforce collection ACLs, and the gate refuses a scope a strategy would silently ignore. - `ObservationGate` reaches the git cache, so a gatekeeper returning commit ids no longer needs a raw queue stub. - Journals and caches require a named keyspace, with an explicit legacy opt-in. Carries two changes made alongside it: `KvTtlCache.partitionedBy` now reads a live `cacheAuthority()` that only vouches for credentials the source still stands behind, and `KvScannable.list` gained storage-level page bounds. * test(gatekeeper-kit): add a conformance consumer that assembles the kit Every existing suite tests one leaf. This one builds a gatekeeper from all of them at once against a fake provider -- account Durable Object on CredentialCoordinator and the connect handshake, resource facet on CredentialSource, ObservationGate, defineActions, KvTtlCache and TokenCursor -- because the contracts a new consumer trips over only appear in assembly. It runs in workerd with real Durable Objects, so persisted stubs, RPC boundaries and storage are genuine. Sixteen tests cover OAuth completion and stale-attempt refusal, repeated credential rotation, tracked child ACLs, a zero-result cursor, a non-idempotent action whose provider outcome is unknown, provisional dependencies, git-cache access, cache repartition on reconnect, action fencing and journal namespace isolation. It found a defect in the API it exercises: `ObservationGate.getGitCache()` was annotated `Promise<GitCache>`, so `using` on the returned caller-owned stub did not compile although the doc told consumers to use it. The return type is now the queue stub own, which keeps it `Disposable`. Also records two things a consumer cannot do, both learned the hard way here: a plain object passed as the approval queue crosses RPC as call-scoped stubs that are disposed when the call returns, and the kit stateful objects are Durable-Object-local by construction -- `perStorage` keys coordination on storage-object identity, so a journal or gate that crossed a boundary would have lost that even if it serialized. * feat(gatekeeper-kit)!: declare action fences, fence connect completion, rename sets to collections Three API reshapes that are free now and impossible after the first adopter. An action fence was an optional argument at every call site, so omitting one was invisible: the gatekeeper works, its tests pass, and an action approved under one provider account later applies under the next. `defineActions` now requires a set-level `fence` policy with per-kind `fenceOverrides`, and `submit` refuses a fenced kind staged without one. The policy is `"authority" | "none"` rather than naming a connection: the journal never interprets the value, so a provider whose actions should survive re-authorization of the same account fences on a stable account id instead. The kit still cannot capture the fence -- it must ride the staging operation own `CredentialRead`, since a second read taken inside `submit` could land after a reconnect. `claimOAuth` consumes its nonce before the provider token exchange, so a revoke or newer reconnect can land while that exchange is in flight and be overwritten by the older completion. `connect(credentials, { ifGeneration })` compares the connection the attempt started under -- captured through `advanceToOAuth` metadata, which already carries it -- and throws `ConnectionSupersededError` rather than storing a mint the account has moved past. Opt-in, because a flow with no round trip has no window to fence. A "set" never said set of what. The concept is a provider-side access-controlled grouping -- a Confluence space, a Jira project, a repo -- whose ACL governs the items a read returned. `trackedCollectionObservers`, `hasCollectionAccess` and the rest follow. Storage is untouched: `collectionPrefix` still defaults to `"observed:"`, and the kit observers module has no importer outside the kit. * fix(gatekeeper-kit): lease the observation gate and close the review findings A cursor is returned to the gadget and walked later, so it outlives the call that made it. Built on the session own gate, the first `next()` after the session released its stub failed -- and what failed was the authorization, not the data. `ObservationGate.lease()` opens a second gate over its own duplicate of the queue; both share the binding strategy, so exclusions stay one decision. Making the conformance consumer use it surfaced a constraint worth recording: `dup` is reserved over RPC, so only a real `RpcStub` can lease, not a service binding. Review follow-ups: - An unknown observation outcome latches `observer-withheld` and deletes its marker at once. Compaction only ever ran from `addObserver`, so a binding that admits nobody accumulated one durable key per ambiguous failure -- and since the overseer marks no refusal, that is the default path. - A cache entry is dated from when `load()` resolved, not from when the post-load fence read returned; that read is a live account call that can refresh credentials, and dating from its completion extended the caller TTL. - `ActionOutcomeUnknownError` non-replay rests on `claimBeforeApply`, and the doc promised it unconditionally. It now states the precondition, and the kit logs when the guarantee was unavailable. - Conformance handlers compare the action fence against the read their provider call runs under, closing the window apply entry check cannot; the connect race reproduces a revoke landing inside the exchange rather than before it. `plans/gatekeeper-kit.md` is reconciled: its Status claimed §4 matched the shipped signatures while still naming `trackedSetObservers`, `authority()` and an optional cache `options`, and arguing for the no-extra-round-trip cache partitioning the live fence replaced. * fix(gatekeeper-kit)!: remember grant death, narrow the observation gate Two boundary corrections a consumer audit demonstrated. The account forgot a provider-confirmed grant death as soon as the call that found it returned. `#expired` only notified, so `snapshot()` kept serving the same dead grant to every other facet -- and a facet whose cache authority was still vouched for served warm hits from it. The coordinator now records the dead grant's identity fence under `credentials:expired` and refuses it from `#connected()`, so `fresh()` and `rotate()` both fail before any provider work, including while the access token is still inside its own expiry window. The fence is identity-checked on both sides: a stale failure never buries a successor, a mint that lands after the burial goes to `discardMint` rather than being committed, and `#overtaken` will not hand back a successor that is itself marked. The grant stays stored, so account-owned revoke keeps its material and a failed expiry notification is still retried by a later read -- death is durable, delivery is not. `ObservationGate` demanded `RpcStub<ApprovalQueue>` while the canonical read-only capability is `RpcStub<ObservationAuthorizer>`, which catalog and slash-command handlers actually receive; passing one was a TS2345, and calling `authorizeObservation` raw to get around it would skip the strategy's derived exclusions. Everything the gate needs -- authorize, git cache, dup, dispose -- is on the authorizer, so it takes that instead. Only the pass-through `actions` getter needed the wider surface, and it is gone: a session already holds the whole approval queue, so it owns that stub and gives the gate `queue.dup()`. BREAKING CHANGE: `ObservationGate` now takes `RpcStub<ObservationAuthorizer>`. The exported `ActionQueue` type and the `ObservationGate.actions` getter are removed; stage actions through the session's own approval-queue stub.
…are#464) * Bind gatekeeper connect completion to the initiating browser A gatekeeper connect URL is a bearer capability: whoever finishes OAuth at it has their provider tokens delivered into the account that started the flow, and nothing tied the completing browser to that user. An attacker could start a connect and phish a victim into opening the URL. The gatekeeper's final page now posts a single-use ticket to the window that opened it, targeted at the Workshop's origin from PUBLIC_BASE_URL, and the Workshop activates the grant only when that ticket is redeemed over the initiating user's own session. Kernel changes: - GatekeeperConnectCallback.complete() stages the account in the user's DO under the SHA-256 of a fresh 256-bit ticket and returns the ConnectHandoff instead of persisting the account. - New GatekeeperConnectCallback.reconnectComplete() for reconnect and ensureResources flows, whose credentials the gatekeeper now stages until the Workshop calls the new GatekeeperUser.commitReconnect(). credentialsRestored() stays for out-of-band refresh recovery. - AuthenticatedApi.completeConnectHandoff(ticket) redeems a ticket in the caller's own DO (single use, two-minute lifetime); an alarm revokes staged connects whose ticket never came back. - LoginConnectCallbackImpl mints the ticket but does not yet enforce it; the sign-in takeover is a documented follow-up. - PUBLIC_BASE_URL is now required for connects; the dev server defaults it to the frontend's origin.
…ession (cloudflare#478) Deleting a workspace schedules its DO's abort about 100ms out (Overseer.scheduleAccessRestart), and the abort severs every session that still has the workspace open: the session's notifyClosed stub is dropped uncalled, which AuthenticatedApiImpl reads as a lost DO and answers by closing the WebSocket. The lifecycle test disposed its workspace stub right after deleteSelf() and kept polling listGadgets() on the same session, so it raced that close and failed intermittently with "Peer closed WebSocket: 3000 RPC session was shut down by disposing the main stub". It now logs in over a new session for the post-deletion check, as a reconnecting client would. Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
…astChannel transport (cloudflare#473) * workshop-shared, workshop-backend: redeem connect handoffs from the popup with a per-flow nonce Every flow start (connectAccount, reconnectAccount, ensureAccountResources, startGatekeeperLogin) mints a server-side nonce and returns it with the url. The popup's own /connect/handoff page presents ticket and nonce together: completeConnectHandoff(ticket, nonce) over the popup's session for connects, the public confirmLogin(ticket, nonce) for sign-in, after which the login tab receives the token through LoginAttempt.receive(). Connect flows are kept as hashes in the user DO (pendingConnectFlows, swept by the existing alarm) and must name the ticket's account; the PendingLogin DO is addressed by the nonce's hash. Both records are deleted before the checks, so a ticket or nonce is spent however the redemption goes. The BroadcastChannel envelope constants and LoginAttempt.claim() are gone.
* typed-storage: Let a schema declare the storage key it lives under.
A schema property name is also the KV key it maps to, so renaming a property
in code is a storage migration. Give a singleton slot somewhere to say
otherwise: `singleton(defaultValue, {storageKey})` declares the key on disk
explicitly, and a bare default value stays the shorthand for the common case
and behaves exactly as before. Collections get the same option as
`storageName`, which prefixes the records and every index alike.
This is the schema-level version of what would otherwise be a special case at
each call site, and it keeps the old name on disk with no migration.
* Refactor: Rename prohibitAllSharing -> containsRestrictedData.
The flag's real meaning is "this observation contains restricted data". What
the platform does about that is policy, which shouldn't be baked into the name
-- the next commits replace the all-or-nothing lockdown with per-collaborator
observer verification.
ObservationDescription.prohibitAllSharing and GadgetMetadata.sharingProhibited
both become containsRestrictedData. No alias: this is a hard rename, so the
gatekeeper call sites move in the same commit.
The overseer's durable singleton is renamed too, and declares its old name as
its `storageKey` so nothing on disk moves. Without that, every workspace that
has already observed restricted data would silently unlatch.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…oudflare#382) * Restricted data part 1: Extend the API. Restate what `ObservationDescription.containsRestrictedData` means now that the enforcement is per-collaborator observer verification rather than an all-or-nothing sharing lockdown, and state the two limits of the model plainly: verification is held to the collaborator's role scope, and enforcement is at admission rather than at each read. No functional change; the implementation follows. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Restricted data part 2: Govern restricted reads by observer verification. Reading restricted data no longer locks the workspace down. The old model blocked the observation outright if the workspace was shared and then refused all future sharing, which made every sensitive data source unusable the moment a workspace had a single collaborator. The observer verification machinery already answers the real question -- does this collaborator have access to the same data? -- at every open, and a widening of that scope now restarts every live session, so admission is a sound enforcement point. So: drop the `hasAnyShares()` block in `authorizeObservation` and the three guards on the sharing mutators. Keep the two guards that are about leaking data back out rather than about who may see it -- no actions and no public web fetches once the latch is set. What replaces them is narrower. A producer nobody can ever be verified against (a vendorless connection, or a legacy record with no `creationSpec`) is still refused while the workspace is shared, because `#inScopeGatekeepers` skips it and so admission cannot see it at all. Removing a producer's record is blocked while the workspace is shared, since that record is what verification runs against. And a new grant -- a collaborator, a share link, another key for one, or a redemption -- is refused if some producer can no longer verify anyone. Each of those checks runs in the same synchronous block as the write it gates, after every await, so a concurrent change cannot slip between check and write. `sharing.ts` loses `hasAnyShares()` and gains an optional `assertGrantAllowed` on each grant-writing method, invoked at that write. Two smaller things fall out. `getSharingManager()` moves inside the `containsRestrictedData` branch, so an ordinary observation on a cold DO no longer pays for an owner User DO round trip; the producer record is then read after that await, since latching against a stale record would permanently brick sharing. And the restart on a terminal re-verification failure is hoisted ahead of the best-effort rollback, taking a gatekeeper RPC fan-out off the path between determining the denial and the abort. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Restricted data part 3: Backend tests. Covers the latch (what sets it, and the cases that must refuse the read rather than latch), the producer-removal guard and its exemptions, the grant checks on each sharing mutator, and the tolerance for action records written before the flag's rename. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Restricted data part 4: Integration tests. Drives the model end to end through the test gatekeeper: a restricted read on a shared workspace, the unverifiable-producer refusal, the removal guard, the action and web-fetch blocks, and the restart that forces re-verification when scope widens. `TestSession.readThing()` takes an optional `restricted` flag so a test can trip the latch through the same `ApprovalQueue` funnel a shipping gatekeeper uses. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Restricted data part 5: Document the model. Rewrites the observer document's model section around admission-time enforcement, states the two limits (role-scoped verification, and enforcement at admission rather than at each read) as edge cases with their reasoning, and records the design under plans/restricted-data-sharing.md -- including the known risk of a producer no gadget binds. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Restricted data: gate addCollaborator inside the sharing manager, skipping no-op re-grants. The overseer called assertNewSharingAllowed() unconditionally before SharingManager.addCollaborator() could learn whether the caller already had an edge to this profile, so a same-or-lower re-grant (a note update or a pure no-op) was refused once the workspace became permanently owner-only. Every other grant mutator takes an assertGrantAllowed hook and runs it at the write; redeemShareKey skips it for an existing edge. addCollaborator now takes the same hook and invokes it only when a grant is created: a new record, a new edge from this sharer, or a role rise on the existing edge. The check still runs in the same synchronous block as the storage write, after every await. maxRole is gone with the rewrite. Unreachable in practice (the removal guard refuses to remove a producer while any reachable collaborator exists), fixed for consistency with the documented design in plans/restricted-data-sharing.md §4. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * Restricted data part 6: Drop the producer guards. Per review: the second layer part 2 stacked on top of observer verification is gone. Deleted `#assertUnverifiableProducerUnshared`, `restrictedProducerIds` (which scanned the whole action log), `removalBlockedByRestrictedData`, `assertNewSharingAllowed`, the removal guard in `GatekeeperClientImpl.remove()` and ambient reconciliation, the missing-producer refusal in `authorizeObservation`, the legacy `prohibitAllSharing` read shim, and the `assertGrantAllowed` hook plumbing in `SharingManager` that only existed to carry the assertion. `authorizeObservation` now just latches. What remains is the whole model: a collaborator is verified against every in-scope gatekeeper at admission, and the latch blocks actions and public-web fetches. Removing a connection is not guarded; when a removal UI is built, it will ask the owner to certify that no sensitive data from that connection has been retained in the workspace, for any connection. Legacy records with no creationSpec are not worth the complexity: the owner starts a new workspace. Tests and docs for the deleted machinery are removed, and every comment the branch added is cut or trimmed to what the code still does. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…es (cloudflare#308) * Restricted data part 5: UI changes. The Share modal no longer replaces itself with a "can't be shared" view when the workspace has read restricted data. Sharing controls stay live and a notice explains that collaborators must be able to see the data themselves. The server allows sharing after the restricted latch (assertNewSharingAllowed refuses only unverifiable producers) and GadgetMetadata.containsRestrictedData documents that such a workspace can still be shared, so the modal's job is to warn and to surface a server refusal verbatim -- which the existing toast catches already do. Regression tests pin both: with the flag set, the banner renders in place of the wall and every management affordance (invite, link creation/copying, collaborator removal, link revocation) stays reachable; and a server-side "can no longer be shared" rejection reaches the user as an error toast. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Retain a consumed share key so a failed open can retry. Opening with a #share= fragment strips the key from the URL, so an open that failed while the recipient's access was still being verified had nothing left to retry with. The key is now held and replayed on the next attempt. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Bugfix: Identity-key retained share keys and clear them on logout. The retained key moves to sessionStorage so a reload can still retry, which means it can outlive the session that captured it. Each entry is therefore stamped with the capturing user's id and ignored -- and swept -- when the current session's id doesn't match, so one user's pending share key can never be redeemed under the next user's account in the same tab. logout() sweeps the whole prefix as well, including malformed and older unstamped entries. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Bugfix: Abandon a superseded workspace open before it creates a capability. The retained-storage path awaits identity resolution before openGadget. An attempt superseded while parked there had already run its cleanup -- with nothing yet to dispose -- so on resuming it minted a stub its cleanup can never reach and published it over the replacement attempt's state: a stale capability, or the wrong workspace's when the id changed. Bail after the await, before any capability is created, like the checks the later awaits already have. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Bugfix: Bind the retained share key to the session that captured it. The in-memory retention tier carried no identity and was replayed on whatever authenticated stub the effect ran with. Its safety rested on a rendering invariant two files away -- that an identity change always unmounts the editor -- which nothing local enforced; an account switcher or soft logout would have silently turned it into a cross-user key replay. The ref now records the stub that captured it and is replayed only on that stub. Any other stub falls through to the sessionStorage tier, whose entries are identity-stamped and checked. Stub identity rather than an async userId keeps the common same-session retry pipelined. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Bugfix: Invalidate a pending share-key stamp on discard and logout. The capture path's identity stamp is asynchronous, gated by a flag local to one load attempt -- but the storage it writes is global. A stamp resolving after a *different* attempt succeeded (or after logout swept the tier) wrote the entry back, resurrecting a key that could silently re-redeem the still-active link after an owner removes the collaborator. Invalidation now lives in retainedShareKeys.ts as generation counters: a capture takes a write token, and clearing a workspace's entry (or the logout sweep) voids every token taken before it. The per-attempt flag is deleted -- its scope was the defect. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Bugfix: Clear a retained share key as soon as the keyed open succeeds. The server confirms a share-key redemption inside open(), before the client holds the capability -- but retention was discarded only after subscribeToMetadata resolved. That call has real post-open failure modes for exactly the keyed audience (the non-owner whoami round trip, a WS drop), and every error path keeps the key by design, so a confirmed-then-failed subscribe left all three retry paths (retry button, reconnect stub swap, remount) armed with a live key -- and a re-redemption after an owner removal silently re-grants access, since links are multi-redeemable and owner removal wipes edges but not the link. Keyed opens now await the open promise (one extra round trip, keyed opens only -- the pipelined RpcPromise stays usable as the stub) and discard both retention tiers the moment success is knowable; an open failure rejects there and keeps retention, matching the server's reverted redemption. The tail clear stays for the keyless corner where a retained entry existed but was not attached (the identity-unknown path). The denial tests now model the denial where it really lands -- openGadget's promise rejecting -- rather than as a subscribeToMetadata throw. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Bugfix: Don't let a superseded keyed open clear a newer attempt's retained key. The finding-7 fix awaited the keyed open and then unconditionally discarded both retention tiers -- with no cancelled check, unlike every other side-effect site in the hook. The await can park across the attempt's cancellation, and a superseded attempt no longer owns the retention state: a newer attempt may have captured its own key -- possibly another user's, on a swapped stub -- into the very ref and sessionStorage entry the late clear wipes, and clearRetainedShareKey's write-token bump also permanently voids that attempt's still-in-flight identity stamp, so its failed open dead-ends unretryable. Bail before the clears when cancelled, matching the hook's invariant everywhere else. Skipping the clear loses nothing: replaying the superseded attempt's confirmed key later is a server-side no-op (a confirmed edge skips redemption), and whichever attempt next succeeds clears retention itself. The stub was assigned before the await, so the cleanup already disposed it and a plain return is correct. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Bugfix: Let a cancelled keyed open clear its own retained share key. The round-2 fix's blanket cancelled-bail before the post-open clears discarded positive knowledge: reaching that line means the open *resolved*, i.e. the server durably confirmed the redemption (nothing in disposal reverts it). A confirmed-then-cancelled attempt (unmount, stub swap, retry) left the identity-stamped sessionStorage entry behind -- the stamp is deliberately not cancelled-gated -- and every replay path later re-redeemed the still-live link, silently restoring access after an owner removal. Clearing is now attempt-owned: clearRetainedShareKey takes an `onlyKey` and no-ops (no removal, no generation bump) when the stored entry carries a different key, so a newer capture's retention and in-flight stamp survive -- which is what keeps the round-2 superseded-attempt test passing unchanged -- while a matching or absent entry is cleared and its pending stamp voided even after cancellation. The absent-entry bump is deliberate fail-toward-security; its residual (voiding a concurrent newer attempt's in-flight stamp) is documented with the recovery being a re-click of the invite link. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Bugfix: Void a cleared share key's own pending stamp without touching other keys'. The round-3 attempt-owned clear (clearRetainedShareKey with onlyKey) returned without any invalidation when a different key occupied the entry -- so a superseded attempt A whose open the server confirmed, but whose identity stamp was still parked in whoami(), never voided that stamp: it landed late, overwrote the newer attempt B's entry with A's *confirmed* key, and a later mount replayed it -- re-redeeming the still-live link after an owner removal. The generations were per-workspace, so A's stamp could not be voided without also voiding B's. Add a per-(workspace, key) generation tier: beginRetainedShareKeyWrite now records the key it will stamp, commitRetainedShareKeyWrite checks all three tiers, and an onlyKey clear always bumps exactly its own key's generation -- voiding the calling attempt's stamp even when a different key occupies the entry -- while removing the entry only when it is absent or matching, and leaving the workspace generation alone. That last part also retires round 3's documented absent-entry residual: an attempt-owned clear can no longer void a concurrent newer attempt's in-flight stamp. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Bugfix: Bail a cancelled attempt before it mutates newer retention. In the retained-storage path, a cancelled attempt parked in whoami() resumed and mutated retention before the pre-open cancelled check: the identity-match branch re-armed the in-memory ref over a newer attempt's capture, and the mismatch branch called the unscoped clearRetainedShareKey(id) -- sweeping a newer attempt's entry and, via the workspace-generation bump, permanently voiding its in-flight identity stamp. Bail immediately after the identity resolves: a cancelled attempt no longer owns retention, so it must neither re-arm the ref nor judge an entry that may have been replaced while it was parked. And scope the identity-mismatch sweep to the key this branch actually read and judged (clearRetainedShareKey(id, retained.key), from the previous commit's key-scoped clears), so a newer capture's different-key entry and stamp survive even if the branch is ever reached with stale data. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Bugfix: Scope attempt-owned share-key clears by capture, not raw key. Attempt-owned clears (the post-open success clear and the identity- mismatch sweep) identified their retention by (workspaceId, raw key), so two captures of the *same* invite key collided: after a same-tab user switch, user A's disposed open of key K resolving late would remove user B's freshly captured entry for the same K and permanently void B's in-flight identity stamp (the per-(workspace, key) write generation was shared), dead-ending B's retry on the access-denied page. An availability bug only -- clearing is the fail-safe direction. Each fragment capture now gets a unique captureId, stored in the entry, the in-memory ref, and the write token. Attempt-owned clears bump that capture's own generation and remove the entry only when it carries the same captureId, so a same-key successor capture survives both the removal and the stamp-voiding. Workspace-scoped and global clears are unchanged. The stored entry shape gains a required captureId with no migration: the v2 format exists only on this branch, so entries without one simply read as absent. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Mitigate duplicated-tab replay of a retained share key. Browsers copy sessionStorage into a duplicated tab, so a retained share-key entry cleared in the original tab lives on in the copy: after a successful redemption and a later collaborator removal, the duplicate's revocation-restart reconnect would silently re-redeem the still-live link, undoing the removal. Two frontend mitigations bound this without touching the kernel API surface. Entries now expire 15 minutes after their identity stamp is written (the legitimate failed-open retry/reload fits well inside that; a copy replaying after a later removal does not), and clears propagate across same-origin tabs over a BroadcastChannel -- capture-scoped clears, which a duplicate's copied entry answers to because it shares the original's captureId, and the logout sweep, since tabs share the login session. Workspace-scoped clears name no capture and deliberately stay local, so an independent sibling capture still legitimately retrying is never blanket-cleared. Documented residual: a duplicate discarded or unloaded at broadcast time that reactivates within the TTL can still replay once. The link itself stays multi-use server-side (docs/sharing.md already carries the matching manual re-redeem residual); a single-use server-side retry capability would close both and remains a possible kernel-side follow-up. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Restate the retained-share-key rationale for one-step redemption. Comments and the plan doc still described two-phase redemption: a failed open reverting the redemption server-side, so a retry had to re-send the key, and success "confirming" it. Under one-step redemption (cloudflare#340) the edge is real the moment the server redeems, so a failure after that point retries keylessly and nothing is ever reverted or confirmed. The mechanism is unchanged and still earns its keep: an open can fail *before* the redemption lands (a transport failure, a server throw ahead of the redemption, an attempt superseded before issuing), and the client cannot distinguish that from a post-redemption failure, so it retains the key on every failure -- replaying a key whose edge already exists is a server-side no-op. Prose only; no behavior change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Bugfix: Supersede an older capture's pending share-key stamp on a newer capture. Write tokens tracked clears but not newer captures, so two captures for one workspace with identity stamps in flight raced on the entry: when the older capture's whoami() resolved last, its stamp overwrote the newer capture's entry. Nothing had cleared, so no generation moved -- the older key (spent or not) became what a reload replayed under the newer capture's session. Starting a capture now bumps the workspace generation before taking its token, so every older pending stamp for that workspace fails its workspace check at commit time. The newest capture owns the slot outright, whether or not the older attempt ever succeeded or cleared. Other workspaces' pending stamps are untouched, and the per-capture tier keeps its job: it is still what lets the newest capture's own success clear void its stamp without voiding a later capture's. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * Bugfix: Let a broadcast share-key clear reach a live duplicate's in-memory key. The cross-tab clear broadcast swept a duplicated tab's sessionStorage copy but not the two other places the same tab could still hold the key: the hook's retainedShareKeyRef, re-armed from the copied entry (same captureId), and a local the reload path had read the entry into before parking in its identity await. "Try again" on the same stub replayed the ref, and the resumed await attached the stale local -- so a live duplicate re-redeemed the cleared key exactly as if nothing had been broadcast. Clears now notify subscribers (subscribeToRetainedShareKeyClears): capture-scoped clears and the logout sweep, local and received alike, with a capture-scoped clear reported whether or not a stored entry matched, since the ref is a separate tier. Workspace-scoped clears stay unreported: they name no capture, are never broadcast, and the hook drops its own ref before issuing them. A listener's throw cannot break the clear. The hook subscribes and drops its ref on a clear naming its capture (or on the sweep), leaving a newer local capture with a different id untouched; and the reload path re-reads storage after the identity await, proceeding keylessly when the entry it judged has since been swept (sibling broadcast, logout, or TTL). The residual shrinks to what the module header already documents: only a duplicate unloaded at broadcast time that reactivates within the TTL can still replay once. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * Bugfix: Remove an older capture's stored share-key entry when a newer capture begins. Starting a capture superseded the older capture's *pending* stamp (workspace generation bump) but left its already-committed entry in storage until the new capture's own stamp landed. For one identity round trip the slot held the older key, so a reload inside that window replayed the older capture instead of the one just taken -- "the newest capture owns the slot" was true of the stamp but not of the entry. beginRetainedShareKeyWrite now also removes the workspace's stored entry, after bumping the generation and before returning the token, so the slot is empty rather than stale until the new stamp lands. The removal is local only: a sibling tab's entry under the same workspace is its own capture, or a duplicate's copy of an older one that the older capture's own success clear reaches. Other workspaces' entries and pending stamps, and the per-capture tier, are untouched. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * Bugfix: Broadcast the clear of a share-key entry skipped by an unresolvable identity. The reload path leaves a retained entry in place when whoami() rejects (identity unknown, so the key is neither attached nor discarded). If the keyless open then succeeds, the follow-up clear at the success site was workspace-scoped -- and that scope names no capture, so it is never broadcast. A duplicated tab's copy of the same entry (same captureId), and a live duplicate's re-armed in-memory ref, survived the original's success and could replay the still-live link after an owner removal. Narrow -- it needs whoami() to fail while openGadget on the same stub succeeds -- but it left one success path outside the "broadcast reaches every tier" claim. The keyless success now reads whatever entry is still stored for the workspace and clears it by capture id first, which is the scope that reaches siblings, before issuing the workspace-scoped clear that voids the workspace's in-flight stamps. The leftover can only be the entry this attempt read but could not judge, or nothing: a newer local capture would have cancelled this attempt before it got here. Only duplicates of this tab share the capture id, so the broadcast cannot touch an independent sibling's capture, and a wrongly cleared key costs a re-click of the link. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * Reword the restricted-data sharing warning to match role-scoped verification. The share modal's banner told an owner that invitees "must be verified to have access to the same data". The server's check is narrower: verification is scoped to the recipient's role (build recipients are checked against every account-requiring connection present, use recipients only against connections a gadget binds or an enabled hook feeds), and producers that were never bound, are unbound, or were removed after the read fall outside it -- their persisted output is visible to anyone who can open the workspace (docs/observers.md edge case 4). The banner is the copy an owner reads while deciding to share, so it now states the guarantee actually made, and the disclosure that goes with it: invitees verify their own access to the connections the workspace uses at their access level, and anything already saved is visible to everyone who can open it. The "Recipient verification" panel beneath it already lists those connections per level. Prose only; no server change. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * Bugfix: Broadcast the clear of a share-key entry displaced by a newer capture. beginRetainedShareKeyWrite removed the displaced entry locally only, on the reasoning that a duplicated tab's copy of it would be reached by the displaced capture's own success clear. That only holds if the displaced open ever succeeds somewhere. Once a newer capture displaces it in the original tab, the original's attempt is gone, so the only remaining clear path for the displaced capture is the duplicate redeeming it itself. A duplicate whose own attempt failed transiently keeps the copy; the newer capture's success clear names the newer capture and misses it; and a reconnect in the duplicate inside the TTL replays the displaced key after an owner may have removed the collaborator. Clear the displaced entry by its capture id instead, which broadcasts. Capture ids are per-capture UUIDs, so the clear can only reach copies of this tab's entry, never an independent sibling capture, and this tab's own hook ref already holds the new capture before the begin runs, so the local notification drops nothing. The bare removal stays behind it for an entry the reader rejects (malformed or v1), which no capture-scoped clear can name. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * Bugfix: Clear an unjudged retained share key when its keyless open confirms after cancellation. The identity-unknown reload path leaves a stored entry it could neither attach nor judge, and the keyless success at the end of the attempt is what discards it. A cancellation landing while the metadata subscribe was in flight skipped that clear, even though the subscribe resolving proves the keyless open succeeded (the pipelined call would have rejected otherwise), so the spent entry survived for a later reload to replay against the still-live link after an owner removal. Mirror the keyed path's confirm-after-cancel clear, scoped to the capture this attempt actually read so a newer attempt's entry and in-flight stamp survive. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * Bugfix: Leave another workspace's in-memory share key alone on a keyless success. By the time a keyless open succeeds, any in-memory ref for that workspace is already gone (dropped on a foreign stub, cleared by the keyed success, or superseded by a newer capture cancelling the attempt), so the unconditional ref clear could only ever drop a different workspace's retention: capture a key for A, have A's open fail transiently, open B keylessly, and A's same-stub retry lost its key. Nothing about B's success says anything about A's key, and every other ref clear in the hook is already scoped by id. Guard this one too. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * Trim the retained-share-key comments. Comments only. The rationale blocks in retainedShareKeys.ts and useWorkspaceOpen.ts had grown by accretion over the bugfix series and repeated themselves across sites; each is cut to what a reader needs at that site, with the model and residuals stated once in the module and ref headers. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * Remove client-side share-key retention. Reverts the retained-share-key series to the pre-series state: the `#share=` fragment is stripped on open and sent once, and a first open that fails before the server redeems the key is retried keylessly, so the user re-clicks the invite link. Retaining the key across retries, reconnects and reloads reopened replay-after-removal and cross-user paths that took identity stamps, generation tokens, a TTL and cross-tab broadcasts to close, and still left residuals (a mangled fragment stuck until the TTL, a lost role upgrade), all to save one link re-click. Restores useWorkspaceOpen.ts, its test, useAuth.ts, its test and docs/sharing.md to their base versions, deletes retainedShareKeys.ts and its test, and drops the retention items from the sharing plan. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Bumps [vitest](https://github.com/vitest-dev/vitest/tree/HEAD/packages/vitest) from 4.1.10 to 4.1.11. - [Release notes](https://github.com/vitest-dev/vitest/releases) - [Changelog](https://github.com/vitest-dev/vitest/blob/main/docs/releases.md) - [Commits](https://github.com/vitest-dev/vitest/commits/v4.1.11/packages/vitest) --- updated-dependencies: - dependency-name: vitest dependency-version: 4.1.11 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
- Move the Tier-1 boundary (owned prefixes, removed paths, format exceptions) from audit consts into scripts/fork/fork-boundary.json, read by both the audit and the sync script. - Add a Tier-1 collision check to the audit: a fork-owned prefix that also exists upstream fails, with Tier-2 remediation. This reclassifies the four wrangler.jsonc files (briefly and wrongly Tier 1) back to fork-managed upstream files; the divergence inventory records them. - Add pnpm fork:sync: impact report (--dry-run), policy-driven merge (only deliberate removals auto-resolve), and --verify (removed-name survivors, uncached typecheck, audit). - Document the Tier-1/Tier-2 policy and the new flow in docs/fork-maintenance.md and README.md.
The token-based connect handoff (upstream cloudflare#464/cloudflare#473) fails closed when PUBLIC_BASE_URL is unset. Default it to the browser-facing workshop origin so local connects work without exporting it by hand.
Upstream: cloudflare#454 cloudflare#450 cloudflare#433 cloudflare#460 cloudflare#464 cloudflare#478 cloudflare#473 cloudflare#381 cloudflare#382 cloudflare#308 cloudflare#470. 20 files conflicted (65 hunks); all resolved Tier-2 by hand, no always-ours. `pnpm fork:sync --verify` is clean. Connect handoff (cloudflare#464/cloudflare#473) adopted wholesale; fork initiator guards kept in front of it (github/linear/cloudflare/email routes still 403 a foreign browser before any code exchange or staging; server injects initiator, client never sends it). GatekeeperModal uses openConnectWindow; the pre-sync PUBLIC_BASE_URL dev default was superseded by upstream's identical one. Restricted data (cloudflare#381/cloudflare#382/cloudflare#308) adopted with the fork's owner-only tier retained: containsRestrictedData follows upstream (verified sharing, no lockdown), while prohibitWorkspaceSharing/owner-only connections keep the absolute no-sharing tier (isWorkspaceSharingProhibited narrowed to them; hasAnyShares re-added to SharingManager; sharingProhibited re-added to GadgetMetadata). The per-read collaborator-coverage quarantine is gone -- a widening restarts instead -- and authorizeObservation takes only the revocation pause (legacy records must not trip the observation path). Fork observer-privacy tests rewritten to the restart model. Also kept: credential-mutation hook wrapped around cloudflare#460's fencing and publish/commit split; MCP initiator plumbing against the staged-reconnect flow (SELF_CLOSING_HTML deleted with the handoff adoption). Sync tooling gap fixed en route: verify had no way to ack an intentionally retained upstream-removed name, so reviewedSurvivors (path-scoped) was added to fork-boundary.json with 14 entries, and the boundary config is excluded from its own survivor grep. Verified: uncached typecheck + merge audit clean; lint 0 errors; backend 881, integration 82, frontend 474, mcp-shared 361, kit 550, ai-executor 63, github 168, linear 5, cloudflare 116, mcp 18, portal 43, scheduler 120, context 42 (all uncached). @gadgets/scripts has 8 failures identical on the pre-merge HEAD (pre-existing release-manifest/preview drift).
…rness The fork's deployment tuning (ae28b29) added wrangler keys the repo's own tooling fails closed on: - manifest-lib: read and drop limits/placement (no v1-contract field; customer instances get platform defaults), with a regression test - staging-config: strip dev-only browser.remote from generated preview bindings - integration harness: drop the backend assets binding for test boots (inline configs resolve assets.directory against the harness root; tests drive /api directly) Regenerate golden-manifest.json (backend gains assetsConfig + ASSETS binding).
…ormal dev browser.remote only selects wrangler dev's remote proxy (wrangler strips it from deployed configs), so it enabled nothing in production. Remove it from the backend config and pin the real prod requirement - the BROWSER binding shipping in the release manifest - in the deploy-contract test. The fork's production backend assets stanza flowed into generated dev configs; normal dev serves the frontend from Vite and must not require a frontend build. run-dev-server now drops backend assets outside --serve-frontend-assets mode (run-local keeps them).
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
github-actions
Bot
added
kernel
workshop/shared
workshop/frontend
gatekeeper
delivery
mcp/shared
labels
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Adam Poulemanos (bashandbone)
deleted the
sync/foundation-2026-09-14
branch
September 14, 2026 20:57
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Upstream sync plus two fork fix commits.
pnpm fork:sync --verifyis clean on the merge.Upstream (foundation/main → 5af356e, 11 commits)
Fork fixes (config fallout from ae28b29)
b23db94d: manifest-lib reads and dropslimits/placement(no v1-contract field; golden regenerated); staging strips dev-onlybrowser.remote; integration harness drops backend assets for test boots (inline configs resolveassets.directoryagainst the harness root).a8815836: removesbrowser.remote(verified dev-only in wrangler's own source; prod Browser Rendering comes from the binding, now pinned in the deploy-contract test); run-dev-server drops backend assets in normal dev mode (run-local unchanged).Verification
types:scripts, packagetsc, lint (0 errors),fork:sync --verifyclean.Open (not in this PR)
artifactsprod namespace flows verbatim into preview configs — needs a product call (cut vs per-preview namespace).