fix(dashboard/audit): use canonical getStoredApiKey for export download (audit: audit-export-401)#5324
Conversation
…ad (audit: audit-export-401)
downloadExport in AuditPage.tsx:110 read the bearer token via
safeStorageGet('librefang-api-key'), which only checks localStorage.
But the credential layer was migrated in #3620 to prefer
sessionStorage and explicitly WIPE localStorage on save — so the
inline accessor always returned '', sent Authorization: Bearer
(empty), and every Export click 401'd for every signed-in user
since that release.
Drop the inline accessor and route through getStoredApiKey() from
api.ts (the same one authHeader() and every other token consumer
goes through — it checks sessionStorage first and falls back to
localStorage for legacy tokens).
Other safeStorageGet sites in pages/ were audited:
- TerminalPage.tsx (terminal.fontSize + hint keys) — different key
- ChatPage.tsx (librefang.chat.show_hand_agents) — different key
None target librefang-api-key, so no further changes needed.
Verified: pnpm typecheck + pnpm build clean.
Delete the audit doc + INDEX update per the #5265 convention: narrative 119→118, Critical 7→6, Total 120→119, drop the bullet under Critical, and drop the audit-export-401 leg from Pass 1 priority #4 (the entry was a combined 'list-sessions + audit-export' line; list-sessions is addressed by a separate PR so the audit-export-401 half is removed in isolation here).
Delete the audit doc + INDEX update per the #5265 convention: narrative 119→118, Critical 7→6, Total 120→119, drop the bullet under Critical, and drop the audit-export-401 leg from Pass 1 priority #4 (the entry was a combined 'list-sessions + audit-export' line; list-sessions is addressed by a separate PR so the audit-export-401 half is removed in isolation here).
18fdd5f to
ffa2297
Compare
# Conflicts: # openapi.json # xtask/baselines/openapi.sha256
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d8ec5b4fca
ℹ️ 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".
|
|
||
| ### Fixed | ||
|
|
||
| - **Dashboard `AuditPage` Export downloads no longer 401 for every user** (audit `docs/issues/audit-export-401.md`, Severity: Critical). `AuditPage.tsx:110` 's `downloadExport` helper read the bearer token via the bare `safeStorageGet("librefang-api-key")` accessor, which **only checks `localStorage`**. The current credential layer (`api.ts:1038-1047 getStoredApiKey()`) prefers `sessionStorage` and #3620 explicitly wipes `localStorage` on save (`api.ts:3595-3596`) so the token has been in `sessionStorage` for every signed-in user since that release. Result: every "Export audit log" click read an empty token, sent `Authorization: Bearer ` (one trailing space, no value), the server replied 401, and the `ApiError` surfaced to the user with no actionable hint. Broken for every user since #3620. Fix: drop the inline `safeStorageGet` and use the canonical `getStoredApiKey` accessor from `api.ts` (the same one `authHeader()` and every other token consumer goes through). The other `safeStorageGet` call sites in `pages/` were audited and are correctly scoped (`TerminalPage.tsx`'s `terminal.fontSize` + hint keys, `ChatPage.tsx`'s `librefang.chat.show_hand_agents`) — none of them target `librefang-api-key`. (@houko) |
There was a problem hiding this comment.
Keep changelog audit reference resolvable
This changelog bullet cites docs/issues/audit-export-401.md as the supporting audit, but the same commit deletes that file, so the release note now points to a non-existent document and loses the root-cause traceability it claims to provide. Please either keep the audit file (or move it and update the path) so the reference remains valid.
Useful? React with 👍 / 👎.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Closes #5330
Summary
Audit:
docs/issues/audit-export-401.md(Severity: Critical).AuditPage.tsx:110'sdownloadExportreads the bearer token via thebare
safeStorageGet("librefang-api-key")accessor, which onlychecks
localStorage. The credential layer was migrated in #3620to prefer
sessionStorageand explicitly wipelocalStorageonsave (
api.ts:3341-3342). So:sessionStorage,localStoragewiped.AuditPagereadslocalStorage→ empty → sendsAuthorization: Bearer(one trailing space, no value) → 401.ApiErrorsurfaces to the user with no actionable hint.Broken for every user since the storage migration in #3620.
Fix
Drop the inline
safeStorageGetand route through the canonicalgetStoredApiKeyaccessor fromapi.ts(the same oneauthHeader()and every other token consumer goes through — it checks
sessionStoragefirst and falls back tolocalStoragefor legacytokens).
Audit of related call sites
Other
safeStorageGetsites inpages/were audited and are notthe same bug:
TerminalPage.tsx(terminal.fontSize+ hint keys) — differentkey, never written via
sessionStorageChatPage.tsx(librefang.chat.show_hand_agents) — different keyNone of them target
librefang-api-key, so no further changes.A lint rule / grep gate against
safeStorageGet("librefang-api-key")in
pages/(suggested in the audit doc) is intentionally deferred —that's a separate dashboard tooling change.
Verification
Test plan
sessionStorage), navigate to Auditpage, click "Export CSV" / "Export JSON" — file downloads with
non-empty body
localStorageset (legacypre-Dashboard stores bearer token in localStorage; WebSocket auth via ?token= URL query string #3620 storage state) — also succeeds, since
getStoredApiKeyfalls back