Add token-protected dashboard snapshots to the CLI server#1026
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 08a598e122
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| staleAfterSeconds: max(180, Int(refreshInterval.rounded(.up)) * 3), | ||
| host: DashboardHostPayload( | ||
| codexBarVersion: codexBarVersion, | ||
| refreshIntervalSeconds: max(0, Int(refreshInterval.rounded()))), |
There was a problem hiding this comment.
Reject huge refresh intervals before Int conversion
When a user passes a very large but finite --refresh-interval (for example 1e300), decodeServeRefreshInterval accepts it, but the first dashboard snapshot converts it with Int(refreshInterval.rounded(...)); Swift traps on out-of-range Double to Int conversion, killing the server instead of reporting an argument error. Clamp or reject values above the representable range before these conversions.
Useful? React with 👍 / 👎.
e3165ac to
babb10f
Compare
|
Codex review: found issues before merge. Reviewed June 9, 2026, 11:10 PM ET / 03:10 UTC. Summary Reproducibility: yes. for the review finding: source inspection shows the claim route reads Review metrics: 2 noteworthy metrics.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Merge only after the pairing threat model is truthful in docs and PR text, or after the implementation changes to satisfy the stronger guarantee, with maintainer acceptance of the new HTTP security boundary. Do we have a high-confidence way to reproduce the issue? Yes for the review finding: source inspection shows the claim route reads Is this the best way to solve the issue? No. The dashboard endpoint direction is plausible, but the current pairing docs and threat model are not the narrowest safe landing path until they match the actual HTTP claim flow and maintainer security sign-off is explicit. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model gpt-5.5, reasoning high; reviewed against a4f278d91fe4. Label changesLabel changes:
Label justifications:
Evidence reviewedSecurity concerns:
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
950b4e7 to
bdaf906
Compare
082038b to
c1824a1
Compare
Add --dashboard-pairing for keyboard-light dashboard clients. The server prints a one-time 6-digit code that never travels over the network; discovery advertises only the code length. A client claims a generated bearer token by submitting the code. Five failed claims lock pairing until restart, a successful claim closes pairing, and pairing tokens never authorize data routes before they are claimed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
c1824a1 to
6b3863a
Compare
Summary
codexbar servewith a token-protected dashboard snapshot endpoint.none: omit account identity fieldsredacted: redact sensitive identity values while preserving useful account context such as domains/plansfull: include full identity values for trusted local/private deployments--dashboard-pairingfor keyboard-light clients: the server prints a one-time 6-digit code that is entered on the client to claim a generated bearer token.Rationale
Pairing security model (revised per review)
auth.type: "code"andcodeLength; the 6-digit code is printed exclusively on the server console.--dashboard-pairing; token-only LAN serving via--dashboard-tokenis unchanged and remains the default requirement for non-loopback hosts.Implementation notes
Live behavior proof (redacted)
CodexBarCLIfrom this branch on Ubuntu 24.04 (Swift 6.2.1, same toolchain as CI) and exercised the full pairing lifecycle against the running server.Server startup:
Discovery does not expose the code:
Wrong code is rejected; unauthorized snapshot is denied:
Correct code claims a token, pairing closes, token authorizes:
Server log across the session:
Commands run
swift build --product CodexBarCLI(Linux, Swift 6.2.1): builds clean.swift test --parallel(Linux, via CIbuild-linux-clijobs on both x64 and arm64): passed, including the new pairing lifecycle tests.lint-build-testCI job: SwiftFormat/SwiftLint clean, build and tests on the latest head.git diff --check upstream/main...HEAD: passed.Screenshots/GIFs
Reference
Contribution notes
CONTRIBUTING.mdor pull request template was found.AGENTS.md.mainata4f278d9.docs: add dashboard snapshot API referencefeat(cli): add dashboard snapshot payload builderfeat(cli): serve token-protected dashboard snapshotsfeat(dashboard): add code-based pairing