Rename prohibitAllSharing to containsRestrictedData - #234
Draft
Maximo-Guk wants to merge 25 commits into
Draft
Conversation
github-actions
Bot
added
workshop/frontend
Maximo-Guk
force-pushed
the
restricted-data-rename
branch
from
August 17, 2026 03:45
c04d5e3 to
aa71300
Compare
Maximo-Guk
force-pushed
the
restricted-data-rename
branch
from
August 17, 2026 13:17
cf3449d to
f633a1a
Compare
Maximo-Guk
force-pushed
the
restricted-data-rename
branch
from
August 18, 2026 14:34
f633a1a to
bc6f07a
Compare
Preview:
|
Maximo-Guk
force-pushed
the
restricted-data-rename
branch
2 times, most recently
from
August 18, 2026 16:38
413e955 to
b2c3e58
Compare
Maximo-Guk
marked this pull request as ready for review
August 18, 2026 16:55
Maximo-Guk
marked this pull request as draft
August 18, 2026 16:55
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Maximo-Guk
force-pushed
the
restricted-data-rename
branch
2 times, most recently
from
August 19, 2026 16:04
cd22d16 to
b7c9372
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Maximo-Guk
force-pushed
the
restricted-data-rename
branch
from
August 19, 2026 19:52
41ae6a0 to
740374a
Compare
Comment on lines
+2996
to
+3011
| vendorId !== null && this.#inScopeGatekeepers("use").some(gk => gk.id === gatekeeperId); | ||
|
|
||
| for (let collaborator of collaborators) { | ||
| // A verifiable gatekeeper outside a "use" collaborator's scope is one the UI can't invoke | ||
| // and ensureObserver can't cover; only the unverifiable case above blocks regardless of | ||
| // role. An absent role means "build" (see CollaboratorInfo), which fails safe here. | ||
| if (vendorId && (collaborator.role ?? "build") === "use" && !inUseScope) continue; | ||
| let observer = vendorId ? this.storage.observers.get(collaborator.profile.id) : undefined; | ||
| if (!observer || !(gatekeeperId in observer.accountChoices)) { | ||
| throw new Error( | ||
| "This observation was blocked because it contains sensitive data, but this " + | ||
| `workspace is shared with ${collaborator.profile.name} (${collaborator.profile.id}), ` + | ||
| "who has not been verified to have access to that data. They must re-open the " + | ||
| "workspace (which verifies their access) or be removed from it before this data " + | ||
| "can be read."); | ||
| } |
There was a problem hiding this comment.
🟨 A "use" collaborator can now open a workspace holding restricted data without being verified against the connection that read it
Removing the wholesale block in open() (packages/workshop-backend/src/overseer.ts:6751-6756) means a non-owner may now open a workspace that has observed containsRestrictedData, and verification is only run for gatekeepers in their role's scope. For a use collaborator, #inScopeGatekeepers("use") (packages/workshop-backend/src/overseer.ts:6091-6112) only includes connections bound by a non-pending gadget, so a connection that read restricted data through the agent's chat bindings only (never bound to a gadget) is never verified against them, and the observation-time coverage guard explicitly skips them for that connection (packages/workshop-backend/src/overseer.ts:3002). If the agent persists that restricted data into gadget code/storage, the use collaborator renders it through the gadget UI with no access check at all — previously impossible, since a latched workspace refused every non-owner open.
Was this helpful? React with 👍 or 👎 to provide feedback.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Maximo-Guk
force-pushed
the
restricted-data-rename
branch
from
August 23, 2026 20:24
fc6f271 to
b7d1b72
Compare
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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 Opus 5 <noreply@anthropic.com>
PermissionEdge gains a `pending` flag, so a redeemed share key can be recorded without granting anything until its recipient is verified. 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 Opus 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. The external-message path's inline ensureObserver call folds into authorizeCollaborator, the single gate both entry points now share. Tests follow in the next commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Covers the coverage guard, the two-phase redemption and its rollback, and the removal guard on a restricted-data producer. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
End-to-end coverage over real Durable Objects: a restricted observation against verified and unverified collaborators, share-key redemption held pending until verification, and the role scope a "use" collaborator is verified against. The test gatekeeper fixture grows a per-resource restricted flag and a controllable verification outcome so the harness can drive both paths. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
docs/observers.md gains the coverage rules, the role-scope tradeoff and the accepted residuals; docs/sharing.md describes pending redemption. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A collaborator whose provider-side access had been revoked kept admitting the producer's restricted observations to their still-live older session, because the persisted observer record still listed the gatekeeper. The failed gatekeeper is now dropped from that record synchronously with the failure determination, and the terminal catch de-registers invalidated gatekeepers alongside newly-added ones. Fail-closed by design: an outage or expired credential scrubs the same way, blocking that producer's restricted reads until the collaborator re-opens successfully. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Redemption is two-phase, so a restricted-data producer removed between the pending write and the confirm must still refuse the grant. That removal is invisible to the topology fingerprint, since an unverifiable legacy producer's remove() skips the share-link guard. Confirm now runs the same assertGrantAllowed callback the other grant-writing mutators use, in the granting write's synchronous block. An already-confirmed edge skips it, matching redeemShareKey. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A racing removal necessarily aimed at a previously confirmed edge, since pending-only recipients are invisible to listCollaborators. The re-add carries no incremental authority -- the recipient holds the live, manually re-redeemable link -- and revoking the link is the durable exclusion. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The scrub-on-failure and the per-profile serialization compose: a concurrent open's success must not resurrect coverage a failed live check just scrubbed. Asserts the interaction now that both exist. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…oducer gap. Records the per-profile verification lock, the three frontend retention hardenings, and -- as an explicitly unimplemented open gap -- the unverifiable-producer removal exemption, with the decided fail-closed remedy. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ared. remove()'s producer guard exempted unverifiable records -- legacy no-creationSpec connections and aiModel/agentSpawner -- on the theory that a record anchoring no verification is safe to delete, removal being "its own remedy". Backwards once the data has been read: a legacy record is the *blocker* (#inScopeGatekeepers throws on it, denying every non-owner open), so removing it while shared readmitted every existing collaborator unverified while the restricted data persists in chat history, gadget storage and code. The guard core moves to OverseerImpl.removalBlockedByRestrictedData() -- one predicate for "would deleting this record readmit an unverified party" -- and remove() consults it for every record, exemption deleted. Fail-closed by decision: no migration (legacy records never persisted the vendor identity) and no reconnect flow; after an unshared removal the workspace is permanently owner-only, and the documented recovery for an owner who needs to share such a workspace again is a new one. The legacy-connection error message and both docs lose their reconnect/exemption framing; the redemption-time gate that made the exemption survivable is re-framed as defense-in-depth and kept. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…oducer. ensureAmbientCapsules() reconciles stale ambient capsule records (account disconnected or replaced) via raw removeGatekeeper(), bypassing the producer-removal guard entirely: disconnecting an ambient account could delete a restricted-data producer while the workspace is shared, silently un-anchoring collaborator verification with no user intent behind the removal. Latent for the first-party singletons (neither reads restricted data today), live for any future ambient producer. The reconcile now consults the same removalBlockedByRestrictedData() predicate remove() uses: a guarded stale record survives -- and is logged -- until the owner unshares, while a replacement account still gets its own fresh capsule record (prepareChatBindings dedupes the duplicate vendor's binding name, and the dead record's session just fails). The creation-failure rollback stays unguarded: a just-created record cannot be a producer. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
In ensureObserver's per-gatekeeper verify, the getVerifier await sat outside the try whose catch scrubs the persisted coverage, so a rejection there -- deterministic on the User DO's vendor-mismatch throw, or any cross-worker transport failure -- denied the open but left the persisted accountChoices entry intact: #assertSensitiveObservationCoverage kept admitting that producer's restricted reads to the collaborator's older live sessions, the exact stale-coverage hole the scrub exists to close. The await moves inside the try, so every failure of the verify goes through fail(): coverage is scrubbed for the failed gatekeeper only, the failure gets the re-prompt/#describeObserverFailures treatment instead of leaking the raw RPC error, and the callbacks no longer reject -- so Promise.all can't reject mid-flight and the terminal catch's newlyAdded/invalidated rollback snapshot can no longer miss registrations that complete after a sibling's rejection. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…al test. The "removal unblocks the observation" test raced scheduleRevocationRestart()'s 100ms pre-abort window: the reopen loop's first attempt (immediate, a few ms locally) could fully succeed against the doomed pre-abort DO instance -- a probe on a fresh connection can only detect a DO that is *already* dead -- and the abort then killed the returned session under the assertions. Flake, not deterministic failure. The test now first waits for the abort itself, using the one signal a fresh connection cannot provide: the pre-removal session stub dies with the old instance. The reopen loop stays (the DO still has to come back up), and with no second scheduled abort the session it returns is stable. Verified with 10 consecutive clean runs of the suite. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ensureObserver verified only the collaborator's in-scope gatekeepers but kept (and re-persisted) account choices for everything else, so a "use" collaborator opening while a connection was unbound from every gadget re-verified nothing against it yet kept their stale entry. Rebinding the connection keeps the same gatekeeper id (only gadget binding edges change), so the coverage guard's pure key-presence check then trusted coverage that the collaborator's most recent open never verified. Prune out-of-scope entries from the persisted record at every open -- including an empty-scope open, which is exactly the everything-unbound case -- restoring the invariant "entry present => verified at this collaborator's most recent open". The gatekeeper-side registration is kept (forward exclusion via byObserverId), as is the record itself even when its accountChoices empties. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ollaborators. assertNewSharingAllowed threw only when a restricted producer's record was *missing*, so two unverifiable-but-present flavors passed. A legacy record (no creationSpec) let the grant succeed only for recipients to hard-deny at open (#inScopeGatekeepers throws on it) while the grant itself blocked producer removal via removalBlockedByRestrictedData -- wedging the workspace until sharing was undone. Worse, an aiModel/agentSpawner producer (no vendor account, filtered out of every verification scope) let recipients open completely unverified and read the restricted history in chat. Resolve each producer's vendor id exactly as the coverage guard does and refuse both flavors with one message. All five call sites (the sharing mutators and both redemption phases) stay synchronous and benefit unchanged. No removal remedy is offered: after removing the producer, the missing-record branch throws anyway. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…led re-verification. ensureObserver's terminal catch deregistered newlyAdded ∪ invalidated gatekeeper-side unconditionally. For a re-verification failure the removed registration is what preserves forward exclusion: ObserverTracker 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 non-restricted 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, the same discriminator as #pendingObserverIds): that collaborator was never admitted, has no live session, and the minted id would otherwise linger unresolvable inside the gatekeepers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e latch. authorizeObservation awaited the coverage check (one memoized await inside) and the excluded observers' cross-worker teardown between its checks and the restricted latch. During that window the latch read as unset, so a concurrent turn could add a collaborator the coverage check never saw (assertNewSharingAllowed short-circuits pre-latch), remove the restricted producer (removalBlockedByRestrictedData likewise), or scrub coverage the check had already trusted. The method now takes the sharing manager once up top and runs every check, the latch, and the action record in one synchronous block (the house rule -- cf. addCollaborator): the coverage guard loses its internal await and takes the manager as a parameter, and enforcement splits into a synchronous #decideExcludeObservers plus a #tearDownExcludedObservers awaited after the writes (still awaited, not waitUntil: ApprovalQueueImpl returns this promise to gatekeeper workers, so an admitted observation implies the teardown ran before data flows). Also refuse a restricted observation naming a connection this workspace no longer has -- an in-flight facet RPC can outlive removeGatekeeper with no interleaving at all, and with zero collaborators it sailed past the coverage check's early return and latched a missing producer id, permanently bricking sharing via assertNewSharingAllowed's missing-record branch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…cation. authorizeCollaborator guarded a pending redemption's verification window with a content fingerprint of the connection and gadget-bound id sets, captured before ensureObserver (which parks unboundedly on the configuration modal) and compared after. A change reverted within the window -- add a connection then remove it, or bind an existing connection into a gadget then unbind it -- leaves the sets byte-identical, so a redeemer whose verification never ran against the interim topology (and who, still pending, is invisible to the coverage guard) was confirmed anyway. Replace the fingerprint with an in-memory generation (#scopeGeneration) bumped by typed-storage subscribers installed once in the constructor: every gatekeepers add/remove, and every gadgets transition whose per-record projection (nothing while provisional, else the visible binding targets -- exactly the record's contribution to #gadgetBoundGatekeeperIds) changes. Subscribers dispatch synchronously inside transactionSync before the write, so each decision derives from the callback's own old/new records rather than a recompute-by-listing, which would read pre-event state. Title/commitId writes never bump, so a collaborator parked in the modal isn't spuriously denied by unrelated activity; the rare over-bump (a projection change that leaves the union unchanged) is accepted as fail-closed and retryable. In-memory suffices: a DO restart severs the in-flight open this guards. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Concurrent redemptions of one link share a single pending edge, but
revertShareKeyRedemption severed it by (linkId, pending) alone -- so open A's
failure landing inside open B's verification window (spanning ensureCapsules
and the role read) yanked the edge B was settling: B saw a transient
workspaceAccessDenied, or -- holding another narrower edge -- verified at the
narrow role while its confirm's missing-edge re-add then wrote a confirmed
edge at the link's *full* role (fail-closed downstream via the coverage
guard, but the graph recorded more than was verified).
Add PermissionEdge.pendingAttempts: redeemShareKey returns {linkId,
attemptId} and records the claim (pushing onto an adopted edge's list, or
writing a fresh edge with one claim); revertShareKeyRedemption takes the
attemptId, withdraws only that claim, and severs the edge only when none
remain (an absent list is treated as empty and severs -- defensive);
confirmShareKeyRedemption clears the claims with the pending flag, since a
real grant supersedes them all. The confirm's missing-edge re-add is
unchanged: its remaining trigger is the owner-removal race, which severs
edges without consulting claims. A crashed open's stale claim can leave a
pending edge lingering -- harmless: pending edges grant nothing, adoption
keeps working, and the next successful confirm clears everything. The field
is bookkeeping only and confers no authority; it is visible in
CollaboratorInfo.addedBy, which clients ignore.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Maximo-Guk
force-pushed
the
restricted-data-rename
branch
from
August 23, 2026 22:29
840f0fd to
cf1c12f
Compare
Maximo-Guk
changed the base branch from
bigquery-public-data
to
observer-verification-fixes
August 23, 2026 22:32
iskandarbuyk94-dotcom
approved these changes
Aug 23, 2026
…justifies. Two remaining entry points still separated an authorization check from the write (or admission) it justified by real await windows. Both are fixed the same way as the restricted-observation latch: parameterize the check as a synchronous closure and run it in the same synchronous block as the write. 1. A topology-rejected share-key redemption left its observer record behind. authorizeCollaborator awaited ensureObserver -- which persisted the record and dropped the pending-id entry -- and only then ran the deny paths (the scope-generation check and the confirm-time policy re-check). open()'s catch reverts only the sharing edge, so the record and the gatekeeper registrations survived a denial. The leftover record made #decideExcludeObservers read the never-admitted redeemer as "lost access" (admitting an excluded observation), and broke the !record discriminator so a retry skipped the fail-closed #pendingObserverIds guard for its whole parked window. Now the topology check and the confirming grant run as ensureObserver's commit gate, invoked at its success exits inside the per-profile verification lock, immediately before the record persist: a denial throws into the existing first-ever-failure rollback (no record, no registrations, no pending id), while success writes the grant and the record back-to-back. Cleaning up outside the lock instead would race a queued sibling verification, and a gate invoked after ensureObserver returned would run after the lock's release, letting that sibling mint a second observerId. A denied *upgrade* redemption deliberately keeps the existing record and registrations: that party passed a full verification before, and the record is what the coverage guard and forward exclusion rest on for their still-live sessions. 2. receiveExternalMessage checked authorizeCollaborator, then crossed real awaits (owner registration, the caller's context RPC, message preparation) before sendChatMessage/newChat committed the prompt and startAgent ran over the unfiltered chat tail. A concurrent verification's fail() scrubs the caller's coverage synchronously in that window; nothing re-checked, and the reply left the Workshop. The response-target registration now carries an assertStillAuthorized closure -- wrapping the new synchronous assertCollaboratorStillVerified, which mirrors ensureObserver's success invariant (effective role, plus full coverage of the live scope, so a connection added mid-flight fails closed) -- run by newChat/sendChatMessage as the first statement of the transaction that writes the prompt: a stale caller aborts the transaction, so no message, no chat, no response target, and no agent turn. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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>
…it gate. A share link revoked (or its creator's access lost) while the redeeming open's verification waited -- possibly parked on the configuration modal -- was previously caught only by the post-confirm role re-derivation. By then the commit gate had already run: the observer record and gatekeeper verifier registrations were persisted, so the *denied* redeemer retained full observer coverage. That residue escaped the "re-granted access blocks restricted reads until re-verified" property that revocation teardown enforces for everyone else (pending edges are invisible to revokeShareLink's affected set, so no restart aborts such a redeemer), and it also pushed the user's retry onto the re-verification rollback path instead of first-ever. Re-derive the live role (counting the pending edge) inside the commit gate, before the confirming grant: a throw there rides the first-ever rollback -- no observer record, no gatekeeper registrations -- and open()'s catch severs the pending edge. The post-confirm re-derivation remains as the residual guard for changes landing in the await gaps after the gate. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
2 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.
Old model: prohibitAllSharing blocked all sharing the moment sensitive data was observed. No one other than the owner could access a latched workspace.
New model: containsRestrictedData allows sharing but requires per-collaborator observer verification against each gatekeeper that read sensitive data. The workspace still goes into a restricted mode (no actions against gatekeepers, no web fetches), but collaborators can open it if they pass verification.
WIP