docs(dashboard): correct the stale session-storage description of sign-in - #451
Conversation
GitHub Actions
Inactive
|
Warning Review limit reached
Next review available in: 48 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (18)
WalkthroughThe dashboard guide now documents master-key exchange into cookie-backed sessions. Generated dashboard assets were rebuilt with updated hashed chunk references, a new bundled ChangesDashboard documentation and bundle refresh
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
✨ Simplify code
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. Comment |
There was a problem hiding this comment.
Pull request overview
Updates the bundled admin-dashboard guide (docs/dashboard.md) so it accurately reflects the current dashboard sign-in behavior (master key exchanged once for an HttpOnly session cookie with a configurable TTL, plus a non-secret localStorage marker), and rebuilds the committed dashboard bundle so the in-app docs match.
Changes:
- Corrects the dashboard guide’s “where the master key lives” and sign-in persistence descriptions to match the cookie-based session implementation.
- Updates the security note to describe the actual session cookie attributes and revocation behavior (sign-out and master-key rotation).
- Rebuilds and commits the dashboard static bundle so the shipped UI includes the updated guide.
Reviewed changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/gateway/static/dashboard/index.html | Updates the built dashboard entrypoint to reference the newly built hashed JS bundle. |
| src/gateway/static/dashboard/assets/UsersPage-7LqE-NgO.js | Rebuilt bundle artifact reflecting the new main bundle hash / imports. |
| src/gateway/static/dashboard/assets/UsagePage-ERarDcgx.js | Rebuilt bundle artifact reflecting the new main bundle hash / imports. |
| src/gateway/static/dashboard/assets/ToolsGuardrailsPage-CS19b3Ji.js | Rebuilt bundle artifact reflecting the new main bundle hash / imports. |
| src/gateway/static/dashboard/assets/TablePagination-Gcdv1tY0.js | Rebuilt bundle artifact reflecting the new main bundle hash / imports. |
| src/gateway/static/dashboard/assets/SettingsPage-9ZoEiKHk.js | Rebuilt bundle artifact reflecting the new main bundle hash / imports. |
| src/gateway/static/dashboard/assets/ProvidersPage-CuxkpteE.js | Rebuilt bundle artifact reflecting the new main bundle hash / imports. |
| src/gateway/static/dashboard/assets/OverviewPage-DQPI4_pY.js | Rebuilt bundle artifact reflecting the new main bundle hash / imports. |
| src/gateway/static/dashboard/assets/ModelScopeControl-DZ334hCb.js | Rebuilt bundle artifact reflecting the new main bundle hash / imports. |
| src/gateway/static/dashboard/assets/KeysPage-9JGzsx7l.js | Rebuilt bundle artifact reflecting the new main bundle hash / imports. |
| src/gateway/static/dashboard/assets/ConfirmDialog-C27eWF-P.js | Rebuilt bundle artifact reflecting the new main bundle hash / imports. |
| src/gateway/static/dashboard/assets/BudgetsPage-DPDuyKZo.js | Rebuilt bundle artifact reflecting the new main bundle hash / imports. |
| src/gateway/static/dashboard/assets/AliasesPage-B8ZSDZeI.js | Rebuilt bundle artifact reflecting the new main bundle hash / imports. |
| src/gateway/static/dashboard/assets/ActivityPage-Bh1uAn4D.js | Rebuilt bundle artifact reflecting the new main bundle hash / imports. |
| docs/dashboard.md | Corrects the sign-in/session documentation to reflect cookie-based sessions and their TTL/revocation behavior. |
khaledosman
left a comment
khaledosman
left a comment
There was a problem hiding this comment.
Checked every claim against the implementation and they hold: dashboard_session_service.py:157-163 sets httponly=True, samesite="strict", and secure from the request scheme; core/config.py:207 defaults dashboard_session_ttl_hours to 168. The old text was wrong in both directions and the correction is accurate.
Two optional wording notes inline; neither blocks.
Heads up on landing order: this, #450, and #449 all edit docs/dashboard.md and therefore rewrite the same hashed bundle assets. Whoever lands second needs a rebase plus npm --prefix web run build or the staleness check fails. This one is docs-only and cheapest to rebase, so it is the natural one to land first.
🤖 Generated with Claude Code
GitHub Actions
Inactive
…n-in The guide still described the pre-cookie sign-in: master key held in the tab's session storage, gone when the tab closes. Sign-in has since become an exchange for an HttpOnly session cookie (dashboard_session_ttl_hours, a week by default), so the key is never kept in the browser at all and the session deliberately does outlive the tab. Readers were being told the opposite of both halves. Correct the three places that said it: the two-key table, the first-run sign-in step, and the security note, which now also records the cookie's attributes and that rotating the master key logs other browsers out. Surviving a gateway restart is qualified rather than claimed outright, since sessions are rows in the gateway's database and a container on the default sqlite:///./otari.db with no mounted volume starts every run signed out. A second security note covers what the correction otherwise removed: a session runs its full TTL with no idle timeout (is_valid_dashboard_session compares expires_at and never extends it), so an unattended browser stays signed in, and the old wrong text at least implied a tab close logged you out. The guide is bundled into the dashboard (DocsPage imports it with ?raw), so the committed bundle is rebuilt alongside it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
f87cd2c to
84a7c8e
Compare
GitHub Actions
Inactive
Description
Split out of #450, where reviewing a nearby change turned this up.
The dashboard guide still described the pre-cookie sign-in, and both halves of what it said are now backwards:
/v1/auth/sessionand exchanged for an HttpOnly cookie. The browser never stores the key; only a non-secretotari.dashboard.hasSessionmarker lands in localStorage, so the app can render signed-in without probing the server first.dashboard_session_ttl_hourslifetime (168 hours by default), and sessions are rows indashboard_sessions, so a sign-in deliberately survives closing the tab and restarting the gateway.Three places said it, and all three are corrected: the two-key table's "Where it lives" row, first-run step 4, and the security note. The security note is retitled "A session cookie, not a stored key" and now records the cookie's actual attributes (
HttpOnly,SameSite=Strict,Securewhen the request arrives over HTTPS) along with what sign-out revokes and the fact that rotating the master key logs other browsers out.Checked against the implementation rather than the old prose:
dashboard_session_service.py:142for the cookie attributes,core/config.py:207for the 168-hour default,web/src/auth/AuthContext.tsx:12for what the browser actually retains, andapi/routes/settings.py:412for the rotation behavior. Swept the repo for the same claim elsewhere; the only other hit isweb/src/components/Login.test.tsx:49, a test asserting the key is not in session storage, which agrees with the corrected text.The guide is bundled into the dashboard (
DocsPageimports it with?raw), so the committed bundle is rebuilt alongside it. That means this PR and #450 both touch the built bundle: whichever lands second will need a rebase and a rebuild.PR Type
Relevant issues
None filed. Noticed while reviewing #450.
Checklist
tests/unit,tests/integration). Documentation only, so no behavior to test; the existing suites and the bundle staleness check pass.make lint,make typecheck,make test).uv run python scripts/generate_openapi.py). No API change.AI Usage
AI Model/Tool used:
Claude Code (Opus 5)
Any additional AI details you'd like to share:
Claude spotted the stale text while reviewing #450, verified the current behavior against the source files cited above, and wrote the correction and this description. The reasoning and decisions are @njbrake's.
Summary
Benefits