dashboard: take the key out of the URL in a browser - #561
Merged
Conversation
Three places put the admin key in a URL, because the requests that need it are started by the browser rather than by script and a URL carries no headers: the event stream, the asset builders behind `<img>`/`<audio>`/the VRM model, and the avatar popup's `?key=`. A URL is also a history entry, a `Referer`, and a line in every proxy log between the browser and the origin. The kernel has taken a session cookie since #559. This makes the browser use it. The browser attaches it by itself to same-origin requests, including the ones markup starts, so nothing has to put a credential in a URL and nothing does. Tauri keeps `?token=` and `?key=`, and that is not an oversight: the desktop shell serves the SPA from a custom protocol, so its origin is not the API's, a SameSite cookie is never sent, and removing the query parameter would break the event stream and every avatar there. Each switch is `isTauri` and the reason is always this one. The switches sit in `withToken`, `urlToken` and `openVrmWindowInner` rather than at the dozen call sites, because one call site would forget. `restoreBrowserSession` runs before React mounts, and the timing is the point: a child's effect runs before its provider's, so a mint started from a provider effect would happen after the consumers have already asked whether a credential exists — and their effects do not re-run, because the key they depend on never changed. Ending the session is folded into forgetting the key for the same reason: a browser that has dropped the key while still holding a live admin cookie has not signed out, it has only lost the ability to say so.
The CORS allowlist has no `x-agent-token`, and a reader hitting a CORS error from a future browser-side tool call would reasonably read that as an omission and add it. It is not: an agent token names one agent and carries that agent's capability gate, while the browser signs in as the operator. Adding the header would not unblock a flow, it would declare that a page may hold an agent's credential. Nothing is blocked by its absence today — the only caller presenting one is a server-side process, and CORS constrains browsers.
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
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.
Completes the "no key in the URL" condition of the browser sign-in line. #559
gave the kernel a session cookie and #560 let the edge mint one; this makes the
dashboard actually use it.
The three places the key reached a URL
Not one leak but a class, because these requests are started by the browser
rather than by script and a URL carries no headers:
useEventStreambuilt?token=;withToken()behind<img src>,<audio>and the VRM model;?key=, read back byVrmViewerPage.A URL is also a history entry, a
Referer, and a line in every proxy logbetween the browser and the origin. The cookie is attached by the browser
itself to same-origin requests, including the ones markup starts, so nothing
has to put a credential in a URL and nothing does.
Tauri keeps the query parameter, deliberately
The desktop shell serves the SPA from a custom protocol, so its origin is not
the API's: a
SameSite=Strictcookie the kernel sets is never sent from it, andremoving
?token=would break the event stream and every avatar there. Everyswitch in this PR is
isTauriand the reason is always this one.The switches live in
withToken,urlTokenandopenVrmWindowInner— one perchannel — rather than at the dozen call sites, because one call site would
forget.
Ordering, which is the subtle part
restoreBrowserSession()runs inbootstrap()before React mounts. That timingis load-bearing rather than an optimisation: a child's effect runs before its
provider's, so a mint started from a provider effect happens after the consumers
have already asked whether a credential exists — and their effects do not re-run,
because the key they depend on never changed. The stream would then never open at
all. Restoring before React removes the question instead of racing it; the
provider effect still covers a key entered later.
Ending the session is folded into
forgetApiKeyfor a related reason: a browserthat has dropped the key while still holding a live admin cookie has not signed
out, it has only lost the ability to say so.
The CORS question this closes
x-agent-tokenstays out ofallow_headers, and the second commit writes downwhy so the next reader does not "fix" it. An agent token names one agent and
carries that agent's capability gate; the browser signs in as the operator and
its tool calls are resolved inside the kernel. Adding the header would not
unblock a flow — it would declare that a page may hold an agent's credential.
Nothing is blocked by its absence today: the only caller presenting one is a
server-side process, and CORS constrains browsers.
Verification
Local gates:
biome check src/,tsc --noEmit,vitest run(107 tests, up from84),
npm run build, pluscargo fmt,scripts/lint-rust.shandcheck-docs-facts.pyfor the kernel comment. The dashboard test-count claim indocs/DEVELOPMENT.mdis updated.Mutation testing — 12/12 mutants behaved as expected. Every switch and every
piece of wiring was deleted or inverted in turn against the committed baseline:
never put the key in a URL, whichever builder or binding is usedappend the cache-buster with "&" after the tokenopens with a bare URL once the session cookie is in placedoes not open the stream before there is a credential for it?key=againopens a popup with no credential in its URLmints a session when a key is enteredends the session when the key is forgottenmints from the stored key before anything else asksretries after a refusal instead of caching the "no" forevertells the kernel and forgets the recordasks the kernel once and reports that it was grantedThe first run of that harness was invalid and the self-test is what said so.
It passed
--reporter=basic, which vitest 4 no longer has — it tried to load itas a custom reporter module and every run exited non-zero regardless of the
mutation, so all twelve reported CAUGHT including the no-op. The numbers above
are from the fixed harness, whose checker was first validated against a known-good
tree (exit 0) and a known-bad one (exit 1).
Not in this PR
Referer, history. The assertions here are that no builder or hook producesa URL carrying the key, which is a property of the code; confirming what a
browser then does with it needs the kernel reachable, and that waits on the
headless Linux gate.
(Edited after merge: an earlier version of this note said the browser-mode
"Browser access is deprecated" banner now points the opposite way and should be
replaced. That was wrong and is withdrawn — this work makes the browser route
usable, it does not narrow what the desktop app is for, and the two do not
conflict. The banner is left alone.)