fix(auth): require independent session authority for stored-key reads

[luvs01]

Summary

• Require a current session issued through explicit pairing or trusted Tailscale identity before revealing an existing data-plane key. An automatically bootstrapped loopback GUI session is not sufficient; the existing raw-admin-token refusal remains.
• Implement the predicate through the server-owned session control, using the current session record and existing token, origin, CSRF, expiry and configuration checks. Client-supplied issuance or identity headers cannot stand in for that record, and checking disclosure authority does not renew its expiry.
• Recheck authority both before and after awaiting the request body, so expiration or revocation during reception cannot disclose a stored value. Missing session-control implementations fail closed. Successful reads retain Cache-Control: no-store.
• Preserve ordinary local dashboard access, masked key listing, key creation, rotation and deletion. Operators use the existing pairing flow or trusted identity session to reveal stored values; no new password, Windows Hello flow or user configuration is added.
• Add six cases to the existing API-key suite, extend the real headless CLI/HTTP pairing regression, and update route metadata, owning contracts and English/Korean guidance. This change does not modify stored key values.

Fork-only draft requested by the fork owner. No upstream submission, merge, installation, credential rotation or Security Cloud finding closure is performed. Pairing retains its existing configuration-write authority boundary; this is not human-presence authentication or protection from an unrestricted same-user writer. Other management operations are not redesigned here.

Verification

Base: 5d69e5cdc58441ac5c1735ea2f81f1bc46eff0ac.
Published head: 9c944c4af397257078a4651a8c6911d5043dcfa7.
Exact tested and read-back tree: d5bcc961d2f7395fd624e140d68e096cea847ef0.
Nine files change, with 202 insertions and 16 deletions. Temporary publication workflows and transfer files are absent from this tree.

Actual Linux / repository-pinned Bun 1.4.0 execution

• Final focused run: 249 passed, 1 Windows-native skip, 0 failed; 2,269 assertions across eight files. Both the test process and isolated-environment cleanup completed successfully. Repeated verification runs are not added to that total.
• Original-source red control: the new automatic-loopback HTTP regression fails on the unchanged production implementation, receiving 200 instead of the required 403. After the correction, anonymous sessions receive the same refusal for an existing ID, an absent ID and a malformed body; masked listing remains available and persisted keys are unchanged.
• Separate post-body red control: removing only the second authorization check makes both revocation-during-reception and expiry-during-reception regressions fail, receiving 200 instead of 403. The check was restored before the final run and tree verification.
• Positive and negative coverage includes actual HTTP grant redemption, valid paired reads with no-store, wrong CSRF, revoked sessions, unavailable controls, a dispatcher lacking the new predicate, and trusted Tailscale issuance through the production issuer/control. Untrusted ingress cannot manufacture that issuance; wrong origin, admin token, expiry and deletion are rejected without renewing the session.
• The existing real subprocess test starts a source server without a terminal, runs the actual CLI configuration-write pairing flow and redeems its grant. It now also proves automatic-session key disclosure is denied and the resulting paired session can read the synthetic fixture key. Existing replay, origin, credential, CSRF and pre-SSH enrollment guards remain intact.
• Root TypeScript, privacy scan, structure SSOT and whitespace checks passed. An initial aggregate failed the existing file-size guard after a new case pushed the management-auth test file over its cap. The case was moved into the related API-key suite; no baseline, limit or assertion was weakened. The final aggregate above includes the unchanged ratchet and layout guards.

Executed focused files:

tests/server/api-keys-routes.test.ts
tests/server/server-management-auth.test.ts
tests/server/management-route-registry.test.ts
tests/gui/gui-pair-http.test.ts
tests/gui/gui-pair-delivery.test.ts
tests/test-layout.test.ts
tests/test-layout-tooling.test.ts
tests/ci-workflows/file-size-ratchet.test.ts

Tests used the repository's exported createIsolatedTestEnvironment() and bun test --isolate within its returned environment, with inherited HTTP proxy variables cleared. This retains private-home and credential isolation while avoiding the standard wrapper's unrelated automatic GUI dependency installation, unavailable in this offline executor.

Other executed commands:

bun node_modules/typescript/bin/tsc --noEmit
bun scripts/privacy-scan.ts
bun scripts/structure-ssot.ts
git diff --cached --check

Publication run 37423634727 checked the readable patch's SHA-256, original preimages, exact nine-file scope and complete resulting tree before creating the new ref. Connector readback confirms the published parent and tree. No patched application code or dependency installation executed with its publication write token. Incomplete preliminary transfer data was discarded before this checksum-gated publication. This is transport verification, not an additional application test run.

Limits and outstanding review

The skipped case is the existing native Windows owner/effective-DACL test. No native Windows/macOS execution, real Tailscale Serve deployment, live provider request, interactive browser acceptance, installed-package/service validation, documentation build or full repository/import-connected test suite was performed. Tailscale coverage exercises the real issuer and authorization code with a controlled trusted-ingress input, not a deployed identity proxy. All keys are synthetic fixtures; no user key store or installed service was read or changed by the tests.

Focused coverage is the explicit resource exception. Current-head hosted CI and independent security review of the session/disclosure boundary remain required. Pending, skipped or older-head checks are not passes. Keep the finding open until this correction is actually integrated and verified.

Checklist

• Scope stays focused and avoids unrelated cleanup.
• Documentation and route metadata describe the independently issued session requirement.
• Focused repository tests and root checks passed on the published tree.
• Independent security review of stored-secret disclosure and session revalidation is complete.
• Required current-head CI and remaining operational coverage are accepted.

Review readiness checklist

• Required final validation and its scope are accepted.
• Current dev ancestry is rechecked before readiness.
• All correct review findings are resolved.
• Ready for review. Keep draft pending the outstanding gates.

UI screenshot

Stored-key reveal refused on an automatic loopback session (8b39d3008): the server returns the standing 403, the key table shows the operator-authorized-session notice, and the local pairing form offers ocx gui pair on the same standalone origin.

Link to Devin session: https://app.devin.ai/sessions/4618b44763d145869890b93014329ccd
Open in Devin Desktop: https://app.devin.ai/desktop/session/4618b44763d145869890b93014329ccd?variant=devin

[coderabbitai]

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

• Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

✅ Deterministic PR hygiene checks passed.

[devin-ai-integration]

Devin Review found 1 potential issue.

[devin-ai-integration]

Review triage complete — pushed 8b39d3008.

Fixed (1):

• 🟡 Devin Review — "Stored-key reveal has no pairing path in the local key screen". Genuine gap the 403 made real on standalone loopback dashboards: the key table now discriminates the server's standing refusal from a transient failure, shows a notice explaining the requirement, offers ConnectPairingForm local under the same gate RemoteLink uses, and auto-retries the refused reveal once pairing lands. Committed as fix(gui): offer local pairing when stored-key reveal is refused.

Dismissed (0): no other unresolved threads from Devin Review, CodeRabbit, or the Codex connector — CodeRabbit skipped this draft and the connector left no comments on the head.

Checks: bun run typecheck ✓, gui tsc -b ✓, bun run privacy:scan ✓, bun run structure:check ✓, react-doctor ✓, focused tests ✓ (tests/gui/gui-pair-http.test.ts — the reveal-authority regression — plus gui-pair-capability, gui-api-error, html-lang-boot-script). bun run test:changed hit its 900s suite cap running 357 files (everything that ran passed). Two environment limits on this box, both unrelated to the diff: gui-pair-delivery fails on Windows ACL hardening (EACLIDENTITY inside src/lib/windows-secret-acl.ts), and lint:gui's oxlint .ts plugin can't load under Node 20 — CI's Ubuntu lane covers it.

[devin-ai-integration]

Done. The PR had exactly one unresolved finding across all three bots — Devin Review's "stored-key reveal has no pairing path in the local key screen" — and it was genuine: this PR's new 403 hits standalone loopback dashboards, where pairing is possible but was undiscoverable from the keys screen.

Fixed in 8b39d3008, pushed to codex/fix-key-reveal-session-authority-20261006:

• handleReveal now returns a discriminated RevealKeyResult so 403-denied is told apart from transient failure
• ApiKeysListPanel shows a warning notice on refusal, and — gated on the same canPairLocally conditions RemoteLink uses — renders ConnectPairingForm local; pairing success auto-retries the refused reveal
• New api.key.revealDenied key added to all 11 locale catalogs; structure/dashboard-and-usage.md updated

Checks: typecheck ✓, gui tsc -b ✓, privacy:scan ✓, structure:check ✓, react-doctor ✓, focused tests incl. gui-pair-http (the reveal-authority regression) ✓.

Caveats: test:changed hit its 900s cap mid-run (all executed tests passed); lint:gui can't load its .ts oxlint plugin under this VM's Node 20 (CI's Ubuntu lane covers it); 11 gui-pair-delivery tests fail on Windows ACL hardening (EACLIDENTITY in windows-secret-acl.ts) — environmental, untouched by the diff.

Thread PRRT_kwDOTmCbas6pX7Jc replied to and resolved; summary posted on PR #709: #709

[github-actions]

✅ READY

• all PR quality gates passed.

Hygiene

✅ Deterministic PR hygiene checks passed.

[devin-ai-integration]

Re-triage pass on head 8b39d3008: the triggering review was the prior run's own thread reply (no new review pass). All findings remain addressed — the single Devin Review thread is resolved, CodeRabbit skipped this draft, and the Codex connector left no comments. Nothing outstanding.

[devin-ai-integration]

No new findings — the trigger was a self-fired event. Review id 5425922106 (08:45:19) is the previous run's own thread reply submitted as a pull_request_review, not a fresh bot pass.

Full inventory of PR #709 at head 8b39d3008:

• Devin Review: 1 thread ("stored-key reveal has no pairing path") — fixed by the prior run in 8b39d3008, thread already resolved. No review exists on the new head yet.
• CodeRabbit: skipped (draft PR), posted only its summary card. No findings.
• Codex connector: no comments on the PR.

Nothing to fix or dismiss; posted a one-line re-triage note on the PR so the record is clear.