Conversation
f749ae1 to
79b83d7
Compare
|
Heads-up on the red |
ra-co88
left a comment
There was a problem hiding this comment.
Verdict: approve — one-time-code bootstrap, correctly designed and tested. (Comment review: GitHub blocks formal self-approval on your own PR.)
Supersession check (upstream/main @ 2dc399e): apps/local/src/otc.ts does not exist upstream and local-auth.tsx has no otc/exchange markers — the token-in-URL bootstrap (with its browser-history/referrer/process-listing leakage) is still the upstream shape. Not superseded.
The security core is right on every axis:
- Single-use, destructive consumption:
consumereturns the id or null; a consumed code is dead forever. The replay test asserts 400 on second exchange. - Instance-bound: the store is an in-memory Map that dies with the process — a code minted by daemon generation N can never be replayed against generation N+1. This kills the reboot-replay class entirely.
- TTL-bounded (60s) and high-entropy (128 bits), expired codes pruned on issue.
- Mint is bearer-gated; redeem is unauthenticated by design (the code IS the authorization) — and the bearer arrives in the response body plus an HttpOnly SameSite=strict cookie, with the bearer (not the cookie) as the request gate per
makeIsAuthorized. Transport hardening without semantic confusion. - Nothing touches localStorage; the client applies the bearer to the in-memory connection and strips the query param. The e2e (auth.test.ts) covers the full browser flow including the await-before-router-mount ordering that otherwise caches 401s behind the auth gate.
- Negative controls are real: replayed code ⇒ 400, unknown code ⇒ rejected, mint without bearer ⇒ rejected, expired code ⇒ rejected with the TTL honored by the store (fake-clock test). All four failure classes have dedicated tests — this is the negative-control discipline the framework mandates.
Legacy ?_token= is replaced, not bolted alongside.
CI caveat, not blocking: the single failing shard (E2E cloud 13of16, run 2026-08-30) is the same cap-eviction openSession flake that #1895's restart-envelope retry fixes; this branch predates it and touches no session code.
Good to merge.
79b83d7 to
218fc7e
Compare
|
Rebased onto current upstream/main (was 30+ commits behind, with a stale 2026-08-30 CI run showing the pre-#1895 cap-eviction flake as the only red). The rebase is a pure base refresh — no content changes. Fresh CI will run on the new head, and the #1895 restart-envelope backstop is now in the base, so the old openSession flake can no longer fail these runs. |
What
The local bootstrap flow replaces the bearer-token-in-URL with a one-time-code exchange: the CLI mints a single-use code (bearer-gated), the browser redeems it at
/api/auth/exchange, and the daemon hands back the session credential. The code dies with the process and is consumed on first use.Why
The previous bootstrap put the long-lived bearer token directly in the URL — leaking it through browser history, referrer headers, and process listings. A one-time code that is exchanged server-side and never persists means a leaked URL is worthless after first use (and worthless at all if unused before the daemon exits — the store is instance-bound).
What changed
otc.ts: in-memory single-use code store, instance-bound.POST /api/auth/otc(bearer-gated mint) andPOST /api/auth/exchange(one-time redeem) in the local server.?_token=URL is still accepted for compatibility, but the token is exchanged out of the URL immediately rather than persisted in it.Rebased over #1865 (the long-poll OAuth await endpoint landed in the same file); the routes are independent.
Test plan
Exchange suite: mint→redeem succeeds once, replay fails, cross-instance code fails, malformed body rejected. 5 tests green against current main. Local boot verified end-to-end: CLI prints the code URL, exchange returns the bearer, mint returns a fresh code, and the fresh code redeems to the same bearer.