Audit: admin-gated web endpoint for the audit log - #32
Merged
Conversation
Expose the global audit log over the admin panel listener as GET /api/audit — owner-only, because entries carry player IPs (personal data). The route mirrors the desktop audit:query: from/to, sources, actions, serverId, actor, ip, text, ok, limit, offset all ride the query string, and it returns the same AuditPage (entries newest-first + total + bySource counts). Panel gets an owner-only "Audit" tab (hidden client-side for non-owners, enforced server-side regardless) with a searchable, source/outcome- filtered table. Panel strings stay English, consistent with the rest of the (non-i18n) admin panel. WEB smoke: owner GET 200 newest-first + source/text/ok filters, a non-owner token 403, no token 401. typecheck + build + WEB smoke green. The panel JS consumer is inspection-only (no headless DOM harness for the served panel); the endpoint it calls is fully smoke-covered. Closes #7. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Surfaced by the PR #32 self-review. The old parse was `okRaw == null ? undefined : okRaw === 'true'`, so a present-but-empty or garbage `?ok=` fell through to `false` and silently filtered to failures only. Now an empty/unrecognised value is ignored (undefined); only the literal true/false apply. WEB smoke (which uses ok=false explicitly) still green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Owner
Author
Code reviewOverviewExposes the global audit log over the admin-panel listener as owner-only Correctness / security
Findings
Conventions
Verificationtypecheck (node + web) ✓ · build ✓ · Verdict: ready to merge. One real nit found + fixed; no blockers. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Item #7 (part of #1): expose the audit log over the web panel. Because entries carry player IPs — personal data — the endpoint is owner-only, a stricter gate than any per-server scope.
How
GET /api/audit(admin-panel listener, owner-only): mirrors the desktopaudit:query. All offrom,to,sources,actions,serverId,actor,ip,text,ok,limit,offsetride the query string; returns the sameAuditPage(entries newest-first,total,bySource). Non-owner → 403, no token → 401.role !== 'owner', but the server gate is the real boundary), with a text search + source + outcome filter and a compact table. Panel strings stay English, matching the rest of the non-i18n admin panel.Privacy
Owner-only by design; the client-side tab hiding is cosmetic, the
user.role === 'owner'server check is the enforcement. IPs never reach a lesser role.Verification
MSMS_SMOKE_WEB: owner GET 200 newest-first +sources/text/okfilters, non-owner token 403, no token 401 — all green by exit code.Verification gap (disclosed)
The panel
loadAuditJS consumer is inspection-only — the admin panel is served as a static HTML string with no headless DOM harness, so panel JS isn't exercised by a smoke (same as the rest of the panel). The endpoint it calls is fully covered.Closes #7.