Harden observer verification on the restricted-data model - #345
Closed
Maximo-Guk wants to merge 15 commits into
Closed
Harden observer verification on the restricted-data model#345Maximo-Guk wants to merge 15 commits into
Maximo-Guk wants to merge 15 commits into
Conversation
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 keeps its historical storage key, since typed-storage keys are property names and renaming one would silently unlatch every workspace that already carries it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Maximo-Guk
marked this pull request as draft
August 26, 2026 15:07
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
PermissionEdge gains a `pending` flag plus per-attempt `pendingAttempts` claims, so a redeemed share key can be recorded without granting anything until its recipient is verified -- and concurrent redemptions of one link can each withdraw only their own claim on failure. Also restates what containsRestrictedData means now: sharing is governed by observer verification rather than banned outright. The server still implements the old behavior; that changes in the next commit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sharing a workspace that has read restricted data is no longer refused outright. Instead authorizeObservation admits the observation only when every current collaborator is already verified as an observer of the producing gatekeeper, held to each collaborator's role scope. Share-key redemption becomes two-phase: redeemShareKey writes a pending edge that grants nothing, the redeeming open() verifies the recipient at the role that edge would grant, and only then is it confirmed -- capped at the verified role, rolled back on refusal. This also stops a refused recipient persisting in the sharing graph, which the previous flow left behind. receiveExternalMessage now routes through authorizeCollaborator too -- the single gate both non-owner entry points share -- verifying external callers (non-interactively) where the old code checked only the role. Known races in the surrounding observer machinery -- most of them preexisting -- are deliberately not fixed here: each is marked with a TODO(observer-verification-fixes), and the fixes land in that stacked PR, so this PR stays reviewable as the model change alone. Tests follow in a later commit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Pulled forward from the stacked observer-verification-fixes PR: the coverage guard (#assertSensitiveObservationCoverage) reads the *persisted* observer record from other turns, so a collaborator whose live re-verification just failed must not keep a persisted entry saying they are covered -- until now a revoked collaborator stayed "verified" for restricted reads until their next successful open. fail() now drops the failed gatekeeper from the persisted accountChoices synchronously with the failure determination, getVerifier moves inside the per-gatekeeper try so a verifier-acquisition rejection scrubs like any other refusal (and surfaces the descriptive denial rather than the raw RPC error, with no mid-flight Promise.all rejection to stale the rollback snapshot), and the terminal catch de-registers invalidated registrations alongside newly-added ones. Carried into this PR because it is a stated precondition of the coverage guard; the other soundness precondition (the out-of-scope prune) follows in the next commit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Pulled forward from the stacked observer-verification-fixes PR: the coverage guard's key-presence check rests on the invariant "entry present => verified at this collaborator's most recent open". Without the prune, a "use" collaborator opening while a connection is unbound from every gadget verifies nothing against it, yet their stale entry survives to be trusted by the guard the moment the connection is rebound -- an entry their last open never verified. ensureObserver's step 2 now drops every account choice for a gatekeeper outside the collaborator's live verification scope, even when the remaining scope is empty (that is exactly the everything-unbound open). The gatekeeper-side registration is deliberately kept: it preserves forward exclusion via byObserverId, and the next successful open's addObserver overwrites the verifier. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
restricted-data.test.ts covers the legacy-name reader; sharing.test.ts gains the pending-redemption model cases (adoption, claim-counted reverts, assertGrantAllowed on every grant vector) and loses the hasAnyShares cases with the method; restricted-producer-removal.test.ts covers the removal guard and ambient reconciliation; restricted-observation-latch.test.ts pins the synchronous coverage check and the removed-connection refusal; verification-scope.test.ts pins the #scopeGeneration topology detection (commit-gate rollback cases stay with the stacked observer-verification-fixes PR, whose fix they pin). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
sensitive-observations.test.ts drives the whole model against real DOs and the extended test gatekeeper (per-resource restricted flag, controllable verify outcomes): the latch, the coverage guard, pending redemptions and their concurrent/mid-topology/mid-revocation windows, the producer-removal guard, and ambient reconciliation. The mid-revocation case pins only the denial and the inert edge -- the denied open's observer-record residue is the deferred commit-gate fix in the stacked observer-verification-fixes PR. external-message-verification.test.ts proves receiveExternalMessage holds collaborators to live observer verification (with the gatekeeper's own refusal reason surfaced) and denies a "use" collaborator by role before verification runs. observer-role-scope.test.ts covers role-scoped verification needs. The fixture gains an external control surface, real per-account sessions, and empty required-secrets hardening so local .dev.vars never leak into suites. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
docs/observers.md gains the coverage rules, the residuals, and the per-collaborator verification flow; docs/sharing.md documents pending redemption, the policy hooks, and the revocation interplay. Passages that describe the deferred observer-machinery hardening (the verification commit gate, per-profile serialization, the revocation-restart fail-closed window, the exclusion-gate races, the external path's commit-time re-assertion) are collected in each doc's Known-limitations section, matching the code's TODO ledger. The plan doc's commit sequence is updated to reflect this split. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ensureObserver loads the observer record, awaits verifier RPCs (and possibly the configuration modal, which parks on user input indefinitely), then persists the record. Input gates don't cover those awaits, so two concurrent opens for one profile raced: two first opens each minted their own observerId, registering both with the gatekeepers while the last-written record forgot the other id existed, and a later open's final put could overwrite state a concurrent open had just written -- including resurrecting coverage a concurrent verification's fail() had just scrubbed. A per-profile promise chain now serializes the whole body, following the existing #preparingChatMessages pattern. blockConcurrencyWhile is not usable here: it would freeze the entire DO for an unbounded modal wait. Distinct profiles stay concurrent. Rebuilt on the restricted-data model (ledger rows 2 and 5); subsumes the original b866502, cb5260b, and 5375e86's scrub-composition case. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…led re-verification. ensureObserver's terminal catch deregistered newlyAdded and invalidated registrations gatekeeper-side unconditionally. For a re-verification failure the removed registration is what preserves forward exclusion: the gatekeeper loses the id, prepareObservation stops naming the observer in excludeObservers, and their still-live session (a failed re-verification never restarts sessions) sees later excluded observations. Restricted reads were never at risk -- coverage is scrubbed synchronously in fail() -- so keeping the registration is fail-closed (it can only add exclusion names) and self-heals (the next successful open's addObserver overwrites the verifier). The rollback now runs only for a first-ever verification (!record): that collaborator was never admitted, has no live session, and the minted id would otherwise linger unresolvable inside the gatekeepers. One exception for a returning collaborator: if a teardown racing this call's parked awaits deleted the record it anchored on, the just-re-asserted registrations reference an id no record resolves -- a retained verifier for a removed user -- so the full in-scope set is removed. Rebuilt on the restricted-data model (ledger rows 7 and 8); subsumes the original 546a964, dbdc363, and 4db373a's overseer change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Verification parks across real await windows (verifier RPCs, the configuration modal), and everything authorizeCollaborator decided at entry can go stale in them. ensureObserver now takes a commitGate callback, run synchronously with the step-6 record persist (and at the empty-scope success exit) inside the per-profile lock; a throw takes the same rollback as any other verification failure. The gate re-checks the live role for every caller: a removal landing mid-park used to be caught only by the revocation restart, so a verification resolving inside that window slipped through -- step 6's blind put *resurrected* the observer record tearDownLostObservers had just deleted (record and account choices were loaded pre-park), and open() handed out a full stale-role capability besides. A mid-park downgrade is capped at the live role by the post-verification re-derivation, now meaningful on the keyless path too. For a pending redemption the gate also runs the topology re-check and the confirming grant, so success writes the grant and the record atomically (one synchronous block, crash-atomic), while a denial -- topology change, revoked link, removed creator -- persists nothing: no confirming grant, no observer record, no lingering registrations, and the !record rollback discriminator stays truthful for the retry. Rebuilt on the restricted-data model (ledger rows 14, 15, 17, and 18); subsumes the original 334e2eb and 428d09b, and the stacked 7fd354f, 0f39a79, and a0e3948. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…stifies. Three related holes in authorizeObservation's excludeObservers gate, fixed by splitting it into a synchronous decision (#decideExcludeObservers) and a deferred teardown (#tearDownExcludedObservers): - A mid-registration observer id was unresolvable via byObserverId (a first-time verification registers its minted id with gatekeepers before the record is persisted), so an excluded observation naming it was admitted -- and the collaborator admitted moments later with the data already in chat history. Such ids are now held in #pendingObserverIds for the duration and fail closed with a distinct message. - The exclusion was decided and the losers' cross-worker teardown awaited *before* the action record was written, so a re-grant landing in that window admitted an observation naming a collaborator who was authorized again by the time it was recorded. The decision, the restricted latch, and the record now form one synchronous block; only the teardown is deferred (still awaited before the observation returns, so no data flows first). The latch also moves below the exclusion decision: a blocked observation delivers no data, so it must not leave the workspace restricted. - The teardown deleted by profileId from a snapshot its own awaited fan-out could stale (and the ids, which cross the RPC boundary, weren't deduped): a re-granted profile's *replacement* record could be deleted, after which exclusions naming the new id silently no-op fail-open. Ids are now deduped and only the snapshotted record is deleted. Rebuilt on the restricted-data model (ledger rows 9, 10, 11, and 19); subsumes the original 7ba9382, 44fcf5b, and a1ab665, and the stacked ed1e9ee's ordering rework. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The DO abort that actually ends a removed collaborator's live sessions runs only after two awaited RPC phases -- tearDownLostObservers (serial removeObserver fan-out per lost collaborator) and refreshAffectedCollaboratorListings (chunked cross-DO round trips) -- a window that scales with collaborator and gatekeeper count. Inside it the removed user still watches the session fan-out, yet no per-observation check can see them: their observer record was deleted at the sever (an exclusion naming their id reads as "not an active observer"), the coverage guard's zero-collaborators early return admits restricted data when the *last* collaborator was removed, and once the teardown's removeObserver fan-out completes, gatekeepers stop naming them in excludeObservers at all. authorizeObservation now consults an in-memory #revocationRestartPending flag -- set by tearDownLostObservers synchronously with the sever, for downgrades too -- and fails every observation closed while it is set, checked before the restricted-data latch so a blocked observation does not latch. The flag is deliberately never cleared: the abort destroys it with the DO, and if the restart is somehow lost, staying blocked is the safe direction. Rebuilt on the restricted-data model (ledger rows 12, 13, and 20); subsumes the original a0938b7 and aa6d698, and the stacked e3d4fd6. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…commit. receiveExternalMessage's entry gate (authorizeCollaborator) is separated from the writes it justifies by real await windows -- the owner-registration RPC, the model-context fetch, and newChat/sendChatMessage's own message preparation. A concurrent verification failure (fail()'s coverage scrub) or sharing change landing in those windows stripped the caller's access after the check had already passed, and the submission still committed: the prompt persisted, the agent ran, and the reply left the Workshop. The registration now carries assertStillAuthorized, built on assertCollaboratorStillVerified: a synchronous re-check of the live role and (when the live scope is nonempty) the persisted observer record's full coverage, mirroring ensureObserver's success invariant. newChat runs it as the first statement of the transaction that commits the prompt and registers the response target; sendChatMessage runs it just before materializeChatChanges -- its first write, which cannot move inside the transaction (non-transactional side effects) -- with no awaits between the check and the transaction, so one check covers the whole synchronous write sequence. A stale caller gets the same "open the workspace to verify" denial as the entry gate. Absent for the owner, whose access cannot go stale. Also corrects tearDownLostObservers' doc comment, which stated the delete/removeObserver order backwards (preexisting on main). Rebuilt on the restricted-data model (ledger row 16); subsumes the original 83c9f18 and the stacked 1f261a6. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
docs/observers.md and docs/sharing.md drop the "Known limitation, see PR favor of describing the now-implemented behavior: the verification commit gate, per-profile serialization, the revocation-restart fail-closed window, the exclusion gate's pending map and by-id teardown, the first-ever vs. re-verification rollback split, and the external path's commit-time re-assertion. The restricted-data plan doc's commit sequence now points at this stacked PR instead of a deferred worklist, and the latch suite regains its original pre-fix narration (the ordering it describes exists again). Subsumes the original 383a4f8. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Maximo-Guk
force-pushed
the
restricted-data
branch
from
August 26, 2026 16:57
a179cba to
7f920aa
Compare
Maximo-Guk
force-pushed
the
observer-verification-fixes
branch
from
August 26, 2026 16:57
e5f64a7 to
7e40b7e
Compare
Preview:
|
Maximo-Guk
force-pushed
the
restricted-data
branch
4 times, most recently
from
August 26, 2026 21:48
6c66662 to
8e8a803
Compare
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
1 participant
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.
WIP