Skip to content

fix(dashboard/audit): use canonical getStoredApiKey for export download (audit: audit-export-401)#5324

Merged
houko merged 8 commits into
mainfrom
fix/audit-export-401
May 22, 2026
Merged

fix(dashboard/audit): use canonical getStoredApiKey for export download (audit: audit-export-401)#5324
houko merged 8 commits into
mainfrom
fix/audit-export-401

Conversation

@houko

@houko houko commented May 20, 2026

Copy link
Copy Markdown
Contributor

Closes #5330


Summary

Audit: docs/issues/audit-export-401.md (Severity: Critical).

AuditPage.tsx:110 's downloadExport reads the bearer token via the
bare safeStorageGet("librefang-api-key") accessor, which only
checks localStorage
. The credential layer was migrated in #3620
to prefer sessionStorage and explicitly wipe localStorage on
save
(api.ts:3341-3342). So:

  1. User logs in → token stored in sessionStorage, localStorage wiped.
  2. User clicks "Export audit log".
  3. AuditPage reads localStorage → empty → sends
    Authorization: Bearer (one trailing space, no value) → 401.
  4. ApiError surfaces to the user with no actionable hint.

Broken for every user since the storage migration in #3620.

Fix

Drop the inline safeStorageGet and route through the canonical
getStoredApiKey accessor 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).

-import { safeStorageGet } from "../lib/safeStorage";
+import { getStoredApiKey } from "../api";
@@
-  const token = safeStorageGet("librefang-api-key") || "";
+  const token = getStoredApiKey();

Audit of related call sites

Other safeStorageGet sites in pages/ were audited and are not
the same bug:

  • TerminalPage.tsx (terminal.fontSize + hint keys) — different
    key, never written via sessionStorage
  • ChatPage.tsx (librefang.chat.show_hand_agents) — different key

None 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

pnpm typecheck    # clean
pnpm build        # succeeded

Test plan

…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.
@github-actions github-actions Bot added area/docs Documentation and guides size/S 10-49 lines changed has-conflicts PR has merge conflicts that need resolution no-rust-required This task does not require Rust knowledge labels May 20, 2026
houko added a commit that referenced this pull request May 20, 2026
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).
@houko
houko force-pushed the fix/audit-export-401 branch from 18fdd5f to ffa2297 Compare May 20, 2026 13:45
@github-actions github-actions Bot added size/M 50-249 lines changed and removed size/S 10-49 lines changed labels May 20, 2026
@github-actions github-actions Bot added ready-for-review PR is ready for maintainer review and removed has-conflicts PR has merge conflicts that need resolution labels May 21, 2026
@github-actions github-actions Bot added has-conflicts PR has merge conflicts that need resolution and removed ready-for-review PR is ready for maintainer review labels May 21, 2026
# Conflicts:
#	openapi.json
#	xtask/baselines/openapi.sha256

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread CHANGELOG.md

### 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)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

@github-actions github-actions Bot added area/sdk JavaScript and Python SDKs ready-for-review PR is ready for maintainer review size/XL 1000+ lines changed and removed has-conflicts PR has merge conflicts that need resolution no-rust-required This task does not require Rust knowledge size/M 50-249 lines changed labels May 21, 2026
@houko
houko merged commit 282bef2 into main May 22, 2026
7 checks passed
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@houko
houko deleted the fix/audit-export-401 branch May 22, 2026 01:32
@github-actions github-actions Bot added size/M 50-249 lines changed no-rust-required This task does not require Rust knowledge and removed area/sdk JavaScript and Python SDKs size/XL 1000+ lines changed labels May 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/docs Documentation and guides no-rust-required This task does not require Rust knowledge ready-for-review PR is ready for maintainer review size/M 50-249 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(dashboard/audit): Audit CSV/JSON export downloads silently 401 since #3620

1 participant