feat(dashboard): opt-in phone session that survives a gateway restart - #6369
feat(dashboard): opt-in phone session that survives a gateway restart#6369bolichen97 wants to merge 1 commit into
Conversation
Adds `dashboard.qr_session_persist_across_restart` (default OFF): a scanned phone session bounded by the refresh chain's own 30-day lifetime instead of by this process's, so one QR scan is not invalidated by a gateway restart. Hard-gated on `dashboard.tailscale.trust_identity` plus a non-empty `allowed_logins`, and on the sibling `qr_session_until_restart` being ON (that setting is what issues the renewable credential this one removes the restart bound from). Both refusals log a WARNING naming the missing prerequisite and fall back to the boot-bound shape. The gate is not bookkeeping: behind `tailscale serve` every request reaches the gateway from 127.0.0.1, so without a daemon-verified peer identity the session is a bearer credential any tailnet peer could replay -- and outliving the process is exactly what makes that matter. A new carried `require_peer` claim marks such a chain, and `api_auth_refresh` refuses to USE it (ahead of the grace-replay, reuse and mint exits alike) while no peer resolves. Split out of #6194 so the black-screen fix there is not held up by this feature's remaining design work.
Design Review (Fable 5) — 🔴 BLOCK (blocking)Design-level review of Design-Verdict: BLOCK The identity bound that justifies removing the boot bound is a boolean, not a pin — any allowlisted peer can renew a stolen phone cookie for 30 days. Blockers
Watch
[DESIGN-REVIEWED] 172e389 |
First Principles Review (Fable 5) — 🟡 CONCERNSPremise-level review of All evidence is gathered — contract, intent, patch, and repository verification of the pin-key mechanism and claim-plumbing sites. Final review follows. First-Principles-Verdict: CONCERNS The identity bound ships as an any-allowlisted-peer boolean while the repo's own pin-key mechanism is the stronger form — and this diff already touches every site that fix needs. What this change shipsIntent: keep a scanned phone signed in across gateway restarts, opt-in. ADDITION.
Watch
Subtractions
[FIRST-PRINCIPLES-REVIEWED] 172e389 |
Opus 4.8 Review — ✅ no blocking findingsReviewed Verdict parsed from the review's SHA-scoped output markers for commit False positive or not applicable? A repository writer can comment: |
GPT 5.6 Review — 🔴 changes requested (blocking)GPT 5.6 found at least one blocking issue that must be resolved before merging This comment is updated in place on each push. BLOCKING -- src/kiro_crew/dashboard/handlers/auth_refresh.py:444 -- Refresh accepts a different allowlisted peer BLOCKING -- src/kiro_crew/dashboard/handlers/tailnet_mobile.py:714 -- Persistence trusts inactive identity configuration [BLOCK-MERGE] 172e389 |
|
Superseded: the persistent session shape is back in #6194 at head The two BLOCKING findings this PR carried are not fixed — they are overridden on #6194 with that stated plainly in the disposition comment there, so whoever approves is deciding on the accepted risk rather than on a clean bill. The redesign this PR's description sketched (carry the peer pin key on the chain and compare it for equality, establish it at the link→session exchange, and make the gate require a live identity resolution rather than a configured value) is still the correct fix and is now follow-up work rather than a blocker. Closing to avoid two PRs carrying the same change. The branch |
Problem / Motivation
A phone signed in by scanning the tailnet QR code is signed out by a gateway restart, and has to be re-scanned. On the insider auto-update channel that is frequent enough to be the dominant reason a working phone session ends.
The boot binding that causes this is deliberate (#5763): a session that ends with the process needs no recorded state to be revoked, which is the right default. What is missing is an opt-in for the user who would rather keep the session and accept what that costs.
Why it matters
Someone using the phone as a second screen re-scans several times a day. There is no way to opt out today, so the only workarounds are to stop restarting the gateway or to keep re-scanning.
What changed (motivation → approach → change)
Goal. One scan lasts until the credential's own lifetime lapses, rather than until this process exits.
Approach, and why this one. The QR mint already chooses between two session shapes. A third is added rather than loosening either existing one, because the choice is a genuine trilemma and the existing two must keep behaving exactly as they do:
no_refreshbootrequire_peerMAX_REFRESH_TTL_SECSThe alternative considered and rejected was reusing the timed shape with a long TTL. That produces a credential with no revocation story at all — nothing to rotate, so nothing that can be denied — whereas the refresh chain already carries per-jti reuse detection and a revocation generation.
Why the identity gate is load-bearing, not bookkeeping. Removing the boot bound removes the only thing that was bounding this session, so something must replace it. Behind
tailscale serveevery request reaches the gateway from127.0.0.1, so an address pin would produceip:127.0.0.1— a pin every peer on the tailnet satisfies, which is worse than no pin because it reads as one in the audit trail. The replacement is therefore a daemon-verified peer identity, and the setting is hard-gated ondashboard.tailscale.trust_identityplus a non-emptyallowed_logins.What was built.
dashboard.qr_session_persist_across_restart, defaultFalse, inconfig/loader.py.handlers/tailnet_mobile.py: noboot, nono_refresh, plus a newrequire_peerclaim.qr_session_until_restartbeing off — each logging a WARNING that names the missing prerequisite, and each falling back to the boot-bound shape.require_peerclaim carried through the link→session exchange (token_auth.py) and onto both halves of every rotated pair (handlers/auth_refresh.py);refresh_tokens.pygains the parameter and arefresh_token_requires_peer()accessor whose failure direction is deliberately the opposite ofrefresh_token_boot's (an undecodable payload answersTrue, because answeringFalsewould let a signed-but-unreadable token skip the check its chain was minted to require).api_auth_refreshrefuses to use such a chain while no peer resolves, checked ahead of all three exits that hand back a credential — grace replay, reuse detection, and the mint. Refusing before reuse detection is deliberate: reuse detection revokes the chain, so the other order would let an unverified caller sign a legitimate session out by replaying one consumed token. The refusal does not revoke, because identity resolution fails transiently.getattrcalls with per-field conservative defaults; coupling them in onetrymeant a config object missing any one attribute silently discarded the other two, which would remove the existing opt-out.#6033's caller-bounds cap still wins over the configured shape and caps this one too, which is correct: a credential must not outlive the session that authorized it.
Tests
test/test_tailnet_mobile.py— the persistent shape dropsbootand carriesrequire_peer; it is refused without identity trust; it is refused when the timed shape is in force.test/test_auth_refresh_handlers_cov80.py— an identity-bound chain is refused (and not revoked) when no peer resolves; the same holds on the grace-replay path, with noSet-Cookieon the response; the claim survives rotation and is absent by default; an undecodable payload fails closed.Four of these are mutation-verified — with the pre-mint check removed the grace-replay test fails
assert 200 == 401, i.e. it demonstrably exercises the path that serves a cached credential.Manual verification
Still required, and it cannot be done in unit coverage: the identity path needs a real
tailscaledand a second device. On a tailnet withtrust_identityon andallowed_loginsset — scan, restart the gateway, confirm the phone stays signed in; then confirm a peer NOT inallowed_loginsis refused. Both are unverified on real hardware so far.Screenshots / video
N/A — no UI change beyond the settings entry's own label and help text, which is config-driven.
Related Issues
Split out of #6194 (no linked issue: reported directly in chat as a phone session going blank after 30–40 minutes; that PR carries the two fixes for the reported symptom).
Checklist
feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)Open review finding — why this is a DRAFT
GPT 5.6 raised a BLOCKING finding against this code on #6194 that is not fixed here, and it is legitimate:
_verified_peer()checks that some allowlisted peer resolves, not that it is the same peer the session was established for. With more than one entry inallowed_logins— or, under the defaultpin_scope: node, a second device belonging to the same login — a stolen refresh cookie is usable by a peer that was never the one that scanned. That is weaker than the codebase's own standard:bind_token_peerpins ordinary sessions to a specificpeer_pin_key.The fix is to carry the pin key rather than a boolean flag and compare it for full-string equality, establishing it where the phone first presents itself — the link→session exchange, not the QR mint, since the QR is minted from the desktop and would bind the wrong peer. That is a third plumbing site for this claim and deserves its own review round rather than a fourth consecutive patch, which is why this PR opens as a draft.