Repository navigation
Browser toolbox E: consequence-aware acting gate - #28
Draft
josephschorr wants to merge 10 commits into
Draft
josephschorr wants to merge 10 commits into
josephschorr wants to merge 10 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
josephschorr
added this pull request to stack #29
October 11, 2026 03:34
Adds the web_action resource and its pagestate_matches caveat (write and check), action-fact extraction and the stable web_action key, the benign-churn-stable page-state fingerprint (with the target-text-change TOCTOU case pinned), and the ConsequenceJudge interface with the deterministic tiers and the Haiku backend plus its labeled corpus.
Gates browser mutates at dispatch on a per-action web_action grant, persists an approved grant with its fingerprint caveat and an append-only audit record, re-prompts when the page changed since approval, and makes a JIT-approved declared slot grant standing under planGate:disabled.
Classifies only form COMMITS as mutates (not field edits), tunes the mutate threshold against a live Haiku eval, and fixes the approval screenshot never becoming viewable in the chat UI (capture timeout, SessionRef wire-shape, dist rebuild) — plus a human-readable action descriptor and the stale-ref re-read path.
… generation-scoped refs)
Fail-closed is the package invariant; this hardens the acting gate on every
path the fable security review (and the adversarial re-review that followed)
found it could slip.
BLOCKER — press committed forms un-gated. A press carries no element ref: its
Execute sends the keystroke to document.activeElement, ignoring any ref the
model supplies. Yet stampWebAction classified it from a ref, so a bogus
`{"key":"Enter","ref":"e99"}` made actionFacts return ErrStaleRef, took the
stale-ref "don't gate" exemption, and let Enter submit a form on the coarse
web_host grant alone; a benign-but-resolving ref mis-classified press off an
element Execute never touches. press is now classified from its KEY, up front,
before any ref is read: per the "only submission commits" rule, a committing/
activating key (Enter/Return/Space) is a mutate and gated, while every other
key (Tab, Escape, the arrows, editing keys, a single typed character) edits or
navigates local page state and is not — which also avoids the over-gating a
blanket "gate every press" fix would have reintroduced. A gated press captures
its fingerprint against the FOCUSED element (capturePageStateAtFocus) so the
approval binds to the page the user saw and stays approvable (an empty
fingerprint would make the grant permanently unapprovable — pagestate_matches
denies ""); the key is folded into the web_action ActionKey (Enter and Space
don't share a grant) and named on the card ("Press Enter on example.com").
MAJOR — credential value leaked via ActionFacts.Text. text() fell through to
el.value, so a later acting call on a filled field extracted the typed secret
into the approval card, the judge prompt, and the web_action object id. Dropped
the el.value fallback (a control's value is not an accessible name) and blank
facts.Text outright when the resolved node is in the session's blanked set.
MAJOR — concurrent dispatch could rebind a classified ref to a different
element. Refs are now generation-scoped ("s<gen>e<N>", minted per snapshot via
Session.nextRefGen); a parallel re-mint advances the generation, so an older
ref is absent from the current map and resolves stale instead of silently
rebinding.
MAJOR — model-supplied web_action keys flowed through on benign/stale/nil-judge
paths. NamedArgs now deletes the four reserved web_action keys before stamping,
on every path.
Minors: a nil judge now fail-closed gates (mutate, empty fingerprint) instead
of skipping the gate; classifyMutate gates on tier==mutate OR P(mutate)>=
threshold; the deterministic fast-path only clears on a non-error, non-mutate
verdict; haiku parseVerdict rejects probabilities outside [0,1]; and
interestingRole mints refs for radio/checkbox/option/tab carrying their real AX
role, so the judge's selection-toggle and tab-switch recognizers can fire.
Tests: new coverage for each fix — the press key table, the press ref-escape
regression, a real-Chrome focus-anchored-fingerprint test, generation-scoped
ref invalidation, the credential-blanking path. go test -race
./pkg/agent/tool/browser/... green (headless Chrome present).
…an-gate wedge fix Security fixes from a fable review, runner + authz gate only. MAJOR (3-way convergent): the web_action control stamps (required/key/fingerprint/description) were lifted from ANY tool's arg map. MCP tools carry raw model JSON (no NamedCallArgs), so a prompt-injected model could spoof the approval card's trusted "What" line and mint a fabricated, page-fingerprint-caveated web_action grant for a key/fingerprint of its choosing. Added a trusted discriminator — authz.Permission.OwnsWebActionArgs() (the resolved Check targets web_host, the same gate the screenshot path uses) — and apply it at every lift: BuildInputs, the ask-descriptor lift, and the hook's stamp (defense-in-depth). A non-browser tool's keys are now ignored and left as ordinary, visible args. Fail-closed: a nil Check is not the acting tool. MAJOR: a plan phase naming a web_action slot emitted an UNCAVEATED web_action binding into BindApproved's single atomic GrantSlots batch, which SpiceDB refuses — wedging the whole batch so the human approves the phase and NOTHING binds (web_host included). BindApproved now filters web_action (mirroring CopySlotGrants); a per-action grant is mintable only via the fingerprinted persistWebActionApproval path. Reworded WebActionResource's PlanningNote to steer planners to name the HOST as a web_host slot, never web_action. Minors: - awaitScreenshotReady: track + wrap lastErr so a persistent non-NotFound Get failure surfaces instead of a bare "timed out". - checkExternalSlotGrant: UnresolvedResource is written true (was a tautology). - checkOneSlotGrant honors in.RequireFreshest (FullyConsistent override) so a channelsd-written grant is visible to the next external check. - WebHostValueTransforms prefixes casefold_identity so a free-form "AA.com" slot value grants where the lower-cased CurrentHost() check looks. - Disclose the standing upgrade on the JIT card: a declared-slot JIT approval is rebound to session length, so the card now says so. Tests: trust-gate (BuildInputs + OwnsWebActionArgs + card spoof), plan-gate web_action filter, persistWebActionApproval orchestration, casefold, RequireFreshest, standing-upgrade disclosure.
Two MAJORs from a fable review, verified against the real envtest apiserver: - The unattended-site XValidation rule errored on every sites-less BrowserToolbox (CEL's list access on an absent optional `sites` throws, and the error only absorbs when the rule's right `||` operand is true, which a sites-less blocklist can never satisfy). Guard with `!has(self.access.sites) || ...`. - An explicit `identity: ""` satisfied CEL's has() (which only asks "was it set") so it bypassed persistent-requires-identity with no runtime backstop, unlike the unattended bypass. Add MinLength=1 to BrowserAccess.Identity so "" is unrepresentable. Plus minors: stop leaving stale ResolvedBrowserToolboxes status entries when a class's browser list drops to zero; classify mintBrowserToken/ensureBrowserStateSecret/reconcileBrowserPod's apiserver Get/Create blips as transient (requeue) instead of permanently failing the session, mirroring resolveBrowserCatalog's existing terminal/transient split; add a CatalogWakeMarker so a wake spanning several reconciles (polling a booting browser pod) re-lists the identity backend at most once instead of every ~2s poll; and correct two doc overstatements (TokenHash "triggers pod replacement", and the CRD doc claiming the resolved spec snapshot is frozen for the session's lifetime). Regenerated via mage gen:api + mage manifests; config/crds and install.yaml included.
…eb_action grant clobber #59 ROOT CAUSE (chat/registry.go): a slot a FAILED Adopt left reserved but not live was indistinguishable from one actively being wired, so rehydrate refused both with a permanent ErrSessionNotFound (404) until reservationTTL (10min) reclaimed it. Add an explicit inFlight marker to liveSlot, set for the duration of a build/wire call and cleared on failure; claimForWiring folds "reserve/ reuse" and "claim the exclusive right to wire" into one locked decision so two concurrent wirings can never race (closing a real leak: publish's unconditional slot.entry overwrite). rehydrate now retries an abandoned reservation instead of 404ing, and maps a genuinely-busy slot to ErrSessionUnavailable (503, retryable) instead of 404. Also makes acceptsSession log (rate-limited) a drop for a known-but-not-live session instead of silently vanishing it, which is what made #59 a mystery. Login-code leak (channelkinds/browser/interaction.go): ResponseText (the login_human_step secret) rode the OUT envelope's full payload straight onto every tab's websocket frame via the browser interactionSender's KindInteractionApplied arm. Zero it before it reaches the sink; the runner, which subscribes the same subject directly, is unaffected. web_action grant clobber (channelsd + runner): a Layer-2 (webActionKey != "") approval's det.ResourceType/ResourceID/Permission is the acting tool's own coarse web_host triple, not the resource the ask was about. Binding it TOUCHed (replaced the expiration of) an existing session-length web_host grant down to the ask's 30s external-effect TTL, so every later acting call on that host re-prompted. Carry WebActionKey through channelevents.ToolApprovalDetails, set it on the runner side in buildToolCallPending (det.WebActionKey = ask.WebActionKey), and skip the primary-grant write for a Layer-2 ask in the channelsd handler — the runner already mints the real, narrowly-scoped web_action grant itself. Covered on both halves: a runner table test that the marker travels onto the published Details, and the channelsd-side TestToolApprovalHandler_WebActionAsk_DoesNotTouchWebHostGrant. Minors: rate-limit channelsd outbound relay's "no sender for this kind on this channel" log to once per session instead of once per envelope (fired at full rate for every browser-kind session, with no Accept to skip them pre-Get); widen the frontend SessionRef type with an optional `ns` field + export sessionNamespace/sessionRefKey from types.ts, routing InteractionCard's screenshotDownloadURL and ChatView's artifactDownloadURL through the one shared, tolerant helper instead of two divergent copies. mage web:build run; pkg/web/webui/webassets/dist included in this commit.
A multi-lens review before the PR surfaced four real defects on the acting path, all fixed fail-closed: - SSRF via a 4-in-6 mapped unspecified address. browserpolicy's IsPrivateOrMetadataHost relied on netip's IsUnspecified, which (unlike IsPrivate/IsLoopback) does NOT unmap, so ::ffff:0.0.0.0 slipped past both the policy pre-check and the dial-time DNS-rebinding guard and reached loopback (0.0.0.0 dials localhost on Linux) — including Chrome's own unauthenticated DevTools port. Unmap the address once up front so every predicate sees the canonical form. - Credential value leaked into the page-state fingerprint. pageStateScript's text() fell through to el.value — the twin actionFactsScript was hardened against this but pageStateScript was missed — so a typed password/OTP was sha256'd into WebActionFingerprintArg, a persisted authz input (offline brute-forceable for a low-entropy OTP/PIN), and also destabilized the fingerprint on every keystroke. Drop the fallback, mirroring actionFactsScript. - Approval storm on a Layer-1 mutate deny. The web_action marker was stamped on WebActionRequired alone, so a MUTATE denied at Layer 1 (coarse web_host grant missing — e.g. the first acting call on a host is itself a mutate) was mis-stamped, making channelsd SKIP the web_host grant write and re-deny every later mutate at Layer 1. Gate the marker on a new Result.CoarseGrantSatisfied, set only once Layer 1 passes, so a Layer-1 deny raises a coarse web_host ask. - Goroutine leak on concurrent rehydrate. chat registry's rehydrate cleared the inFlight marker BEFORE publish (Adopt clears it after), reopening the race the marker closes: a second wiring in that window built a duplicate sessionEntry whose health-watcher goroutine publish's overwrite then orphaned. Reorder endWiring after publish, mirroring Adopt. Tests: mapped-unspecified policy cases (unit + Decide), a real-Chrome filled-field fingerprint-stability test, and a Layer-1-deny hook test asserting the coarse ask carries no web_action marker.
…e precedence) Three lower-severity findings from the pre-PR review: - Screenshot blanked-set TOCTOU. A type/code op marked its field blanked AFTER its do() closure released the session semaphore, while CaptureRedacted read the blanked set BEFORE acquiring it — so a concurrent capture could snapshot a just-typed credential before its field was marked for redaction, shipping it unredacted in a human-visible, persisted approval image. Mark the field inside the type's own do() closure and read the blanked set inside CaptureRedacted's closure, so both sit under the same semaphore and serialize. - wait_for invalidated the caller's refs. Its poll loop called setRefs with a fresh generation every iteration, replacing the ref map and breaking the read -> wait_for -> type flow with a spurious "ref is stale". wait_for only checks presence, so it no longer publishes a ref snapshot. - Identity catalog could widen an explicit site. DeriveBrowserEffectiveSites appended identity-derived hosts unconditionally, and browserpolicy.Decide ORs across entries with no precedence, so a looser identity record (a 1Password "AnywhereOnWebsite", Exact:false) for a host the operator listed Exact:true silently widened it to subdomains. An explicit site is now authoritative for its host; identity entries only add hosts the operator did not name. Tests: an explicit-beats-colliding-identity case for DeriveBrowserEffectiveSites; the screenshot and wait_for fixes are covered by the existing browser suites.
request_human_step kind:code (OTP) was non-functional on the browser chat
surface: the ask was identical to a confirm/CAPTCHA, so the card showed only
Approve/Deny, and clicking Approve always resolved with an empty ResponseText —
the runner failed closed ("approved but provided no code") every time.
Signal code-vs-confirm on the ask. A new publisher-authored, trusted
channelevents.InteractionRequestPayload.ResponseInput (placeholder + required)
tells a surface to render a free-text input. humanStepRequester.RequestCode
stamps "wants_code" on its ask Payload, and buildLoginHumanStepPending sets
ResponseInput only for a code ask; confirm/CAPTCHA leave it nil.
Thread the typed code inbound (surface -> runner). The chat decision handler's
responseText flows handlers.go -> Registry.SubmitDecision -> the listener chain
-> clienthosted.Listener -> InteractionDecisionPayload.ResponseText, which the
runner already consumes (host_approval.go setHumanStepCode) and types into the
page. The local/CLI wrapper passes "".
Preserve the login-code leak fix: ResponseText stays zeroed on the OUTBOUND
applied broadcast (browser interactionSender); the code rides only the inbound
decision and the runner's own OUT subscription, never a tab frame.
Frontend: InteractionCard renders a styled, accessible text input when the ask
carries responseInput and threads its value as responseText on the decision;
ordinary button-only cards are unchanged. dist rebuilt and committed.
Tests: RequestCode stamps wants_code; a code ask sets ResponseInput (confirm
none); the listener chain threads responseText; InteractionCard shows the input
and carries the typed value (and an ordinary card neither). go build/test/vet
(incl. integration+e2e tags) green; frontend typecheck + vitest green.
josephschorr
force-pushed
the
pr/browser-e
branch
from
October 11, 2026 04:02
e1981d1 to
3879c13
Compare
This branch was successfully deployed
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.
Adds the consequence judge (deterministic tiers + Haiku backend) and the per-mutate
web_actiongrant: a caveated, page-state-fingerprinted gate that re-prompts when the page changed since approval. Also carries the post-development hardening — the pre-PR multi-lens review fixes (SSRF guard, fingerprint-leak, Layer-1 approval-storm, rehydrate goroutine leak, screenshot TOCTOU, and more) and the browser-surfacelogin_human_stepcode-entry feature.This is part of the stacked browser toolbox series (A→E), to be reviewed and merged bottom-up. Top of the stack, on D.