Fix two observer-verification gaps - #306
Draft
Maximo-Guk wants to merge 4 commits into
Draft
Conversation
Maximo-Guk
marked this pull request as draft
August 23, 2026 04:14
Preview:
|
Observer verification only ever ran in open(). receiveExternalMessage authorized on the effective role alone, so a build collaborator could drive the agent -- and read its replies -- without having been verified against anything the workspace has read. A collaborator added directly, who never opened the workspace in a browser, was never verified at all; one whose verification had failed in the browser kept working through this path. The fix lands as authorizeCollaborator, a single role + observer-verification gate on the Overseer: resolve the effective role, deny below the caller's `requireRole` floor before verification runs (so a "use" caller gets the plain denial rather than being verified for access this path can never grant), then run the same ensureObserver check open() applies. receiveExternalMessage routes through it non-interactively and tells an unverified caller to open the workspace instead. open() still runs the same steps inline; migrating it onto the gate is left to the share-key redemption rework that has to restructure that path anyway. Independent of the restricted-data work that follows: it applies to any workspace with observer-verified gatekeepers, and is reachable only on deployments that bind an external message gateway. Co-Authored-By: Claude Opus 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. 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. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Exercises the authorizeCollaborator gate end-to-end through the real ExternalMessageGateway entrypoint: an unverified build collaborator is refused until they open the workspace (which verifies them), and a "use" collaborator is denied by role before verification ever runs. The fixture worker grows a control surface for submitting external messages (a service binding to the Workshop's gateway entrypoint, plus an Overseer namespace binding used only to derive the workspace id behind a gadgetKey), and its sessions become real: readThing()/doThing() drive observations and actions through the same ApprovalQueue funnel a shipping gatekeeper uses. Also hardens the harness against local-dev leakage: worker configs declare an empty required-secrets list so a developer's .dev.vars (say CF_AI_GATEWAY_*) can't change suite behavior -- these tests depend on no test user having an AI model. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Maximo-Guk
force-pushed
the
observer-verification-fixes
branch
from
August 23, 2026 16:33
0f52f50 to
6779242
Compare
A first-time ensureObserver registers its freshly minted observerId with gatekeepers (addObserver) before the observer record -- and with it the byObserverId reverse index -- is persisted, and the window in between spans awaits (sibling verifier RPCs, even the unbounded configuration modal). A gatekeeper that already accepted the registration may name that id in an observation's excludeObservers; #enforceExcludeObservers resolved it via byObserverId, found nothing, and read it as "not an active observer -> ignore" -- the observation proceeded, and the collaborator was admitted moments later with the data already in chat history. Track such ids in an in-memory #pendingObserverIds map for the duration of the registration (set on mint, deleted in a finally that also covers the step-6 put, so there is no gap where neither the map nor the index resolves the id), and have #enforceExcludeObservers fail closed on them with a distinct "collaborator currently being verified" message. In-memory is the right scope: a DO restart kills the in-flight open, and its gatekeeper-side registration then references an id no record will ever carry, so ignoring it is correct. Re-verification is unaffected -- it reads the observerId from the persisted record, which the index already resolves. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit 0c21a62)
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