Dev#12
Conversation
- Introduced `SIMPLEAUDIT_VISUALIZER_EMAIL` environment variable to customize the contact email shown in the authentication overlay. - Updated the README to reflect the new email configuration. - Modified the server response to include the contact email. - Adjusted the frontend to dynamically display the contact email based on server response. - Added tests to verify default and custom contact email functionality.
There was a problem hiding this comment.
Pull request overview
This PR adds optional secret-based authentication to the SimpleAudit visualization server/UI, and extends the visualizers to display an expected_behavior field in scenario results.
Changes:
- Add optional
X-Secretheader auth to the FastAPI visualization endpoints plus a frontend auth overlay flow. - Normalize/render
expected_behavioras a list in bothvisualizer.htmland the standalonescenario_viewer.html. - Update visualization docs and example JSON to include
expected_behavior, plus a small root README link tweak.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
simpleaudit/visualization/visualizer.html |
Adds auth overlay + authenticated fetch wrapper; renders expected_behavior in scenario details. |
simpleaudit/visualization/server.py |
Adds env-configured secret gating for API routes and an /api/auth endpoint. |
simpleaudit/visualization/scenario_viewer.html |
Adds expected_behavior normalization/rendering; changes header title to link to /. |
simpleaudit/visualization/README.md |
Documents new auth environment variables and behavior. |
examples/test_visualisation_results.json |
Updates examples to provide expected_behavior arrays instead of null. |
README.md |
Updates LICENSE link to a GitHub URL. |
Comments suppressed due to low confidence (1)
simpleaudit/visualization/server.py:198
- The path traversal protection in
get_json_file()relies onfull_path.startswith(os.path.normpath(RESULTS_DIR)), which can be bypassed by sibling prefixes (e.g.,/tmp/results2starts with/tmp/results). Use a robust containment check likeos.path.commonpath([full_path, RESULTS_DIR]) == os.path.normpath(RESULTS_DIR)orPath(full_path).resolve().is_relative_to(Path(RESULTS_DIR).resolve()).
@app.get("/api/json/{file_path:path}", dependencies=[Depends(check_secret)])
async def get_json_file(file_path: str):
"""Get the contents of a specific JSON file."""
if not RESULTS_DIR:
raise HTTPException(status_code=500, detail="Results directory not set")
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| is enabled. When the server has no secret configured it still | ||
| returns 200 but sets ``enabled`` to False. | ||
|
|
||
| The response also includes ``contact_email`` which is read from the | ||
| ``SIMPLEAUDIT_VISUALIZER_EMAIL`` environment variable and defaults | ||
| to the original maintainer address. The frontend uses this to | ||
| populate the authentication overlay message. | ||
| """ | ||
| try: | ||
| check_secret(request) | ||
| except HTTPException as exc: | ||
| # propagate unauthorized status | ||
| raise | ||
|
|
||
| return JSONResponse(content={"ok": True, "enabled": bool(SECRET), "contact_email": CONTACT_EMAIL}) |
There was a problem hiding this comment.
/api/auth returns HTTP 401 when the secret is missing/invalid, which prevents the frontend from reading contact_email (and even enabled) from the response. This makes the SIMPLEAUDIT_VISUALIZER_EMAIL override ineffective when the overlay is shown. Consider returning a 200 with a JSON payload like {enabled: true, ok: false, contact_email: ...} for missing/invalid secrets (or include contact_email in the 401 body and have the client parse it).
| is enabled. When the server has no secret configured it still | |
| returns 200 but sets ``enabled`` to False. | |
| The response also includes ``contact_email`` which is read from the | |
| ``SIMPLEAUDIT_VISUALIZER_EMAIL`` environment variable and defaults | |
| to the original maintainer address. The frontend uses this to | |
| populate the authentication overlay message. | |
| """ | |
| try: | |
| check_secret(request) | |
| except HTTPException as exc: | |
| # propagate unauthorized status | |
| raise | |
| return JSONResponse(content={"ok": True, "enabled": bool(SECRET), "contact_email": CONTACT_EMAIL}) | |
| is enabled. It always returns 200 so the frontend can read both | |
| ``enabled`` and ``contact_email`` even when the secret is missing | |
| or invalid. | |
| The response also includes ``contact_email`` which is read from the | |
| ``SIMPLEAUDIT_VISUALIZER_EMAIL`` environment variable and defaults | |
| to the original maintainer address. The frontend uses this to | |
| populate the authentication overlay message. | |
| """ | |
| enabled = bool(SECRET) | |
| token = request.headers.get("X-Secret") | |
| ok = (not enabled) or (token == SECRET) | |
| return JSONResponse( | |
| content={ | |
| "ok": ok, | |
| "enabled": enabled, | |
| "contact_email": CONTACT_EMAIL, | |
| } | |
| ) |
| try: | ||
| check_secret(request) | ||
| except HTTPException as exc: | ||
| # propagate unauthorized status | ||
| raise |
There was a problem hiding this comment.
auth_check() wraps check_secret() in a try/except that immediately re-raises, and also binds exc without using it. This is dead code and can be removed (or handle the exception meaningfully) to avoid lint issues and keep the endpoint logic clear.
| try: | |
| check_secret(request) | |
| except HTTPException as exc: | |
| # propagate unauthorized status | |
| raise | |
| check_secret(request) |
|
|
||
| try { | ||
| const response = await fetch('/api/files'); | ||
| const response = await apiFetch('/api/files'); |
There was a problem hiding this comment.
There’s a large indentation anomaly before const response = await apiFetch('/api/files');, which makes this block harder to read and suggests an accidental tab/spaces paste. Re-indent the try block consistently with surrounding code.
| const response = await apiFetch('/api/files'); | |
| const response = await apiFetch('/api/files'); |
| d="M9 12l2 2 4-4m6 2a9 9 0 11-18 0 9 9 0 0118 0z" /> | ||
| </svg> | ||
| <span>SimpleAudit Scenario Viewer</span> | ||
| <a href="/" class="hover:underline text-indigo-700">SimpleAudit Scenario Viewer</a> |
There was a problem hiding this comment.
The standalone viewer is documented as working completely offline; linking the title to href="/" will navigate away (and likely break) when opened via file:// or in offline contexts. Consider keeping this as plain text, or only rendering the link when served over HTTP(S) (or use a relative link that preserves offline usage).
| <a href="/" class="hover:underline text-indigo-700">SimpleAudit Scenario Viewer</a> | |
| <span class="text-indigo-700">SimpleAudit Scenario Viewer</span> |
| By default the authentication overlay includes a contact email address. You can override it by setting `SIMPLEAUDIT_VISUALIZER_EMAIL` before starting the server | ||
|
|
||
|
|
There was a problem hiding this comment.
This README states that setting SIMPLEAUDIT_VISUALIZER_EMAIL will change the contact email shown in the auth overlay, but with the current /api/auth behavior returning 401 for missing/invalid secrets, the frontend can’t read contact_email when the overlay is shown. Either adjust the API/frontend so the email is available pre-auth, or update the docs to match the actual behavior.
| By default the authentication overlay includes a contact email address. You can override it by setting `SIMPLEAUDIT_VISUALIZER_EMAIL` before starting the server | |
| `SIMPLEAUDIT_VISUALIZER_EMAIL` configures the server contact email, but with the current authentication flow that value is not exposed to unauthenticated clients before the secret key is entered. | |
| In other words, do not expect the authentication overlay itself to reflect `SIMPLEAUDIT_VISUALIZER_EMAIL` unless the auth API/frontend is updated to make that information available pre-auth. |
| if not SECRET: | ||
| return | ||
| token = request.headers.get("X-Secret") | ||
| if token != SECRET: | ||
| raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Unauthorized") |
There was a problem hiding this comment.
Secret comparison in check_secret() uses a plain != string compare. If this endpoint can be reached by untrusted clients, prefer secrets.compare_digest(token or "", SECRET) to avoid timing leaks and to handle None cleanly.
| const resp = await fetch('/api/auth', {headers}); | ||
| if (resp.status === 401) { | ||
| // key required | ||
| document.getElementById('auth-overlay').classList.remove('hidden'); | ||
| document.getElementById('logout-button').classList.add('hidden'); | ||
| return false; | ||
| } |
There was a problem hiding this comment.
When /api/auth responds 401, ensureLoggedIn() returns before updating the contact email link, so the overlay can never reflect SIMPLEAUDIT_VISUALIZER_EMAIL overrides. If the server is updated to return contact_email even when unauthenticated, update this branch to parse the response (or call a non-authenticated endpoint) so the overlay shows the correct contact info.
| const token = localStorage.getItem('visualizer_secret'); | ||
| if (token) { | ||
| opts.headers['X-Secret'] = token; | ||
| } |
There was a problem hiding this comment.
The secret is persisted in localStorage, which makes it accessible to any XSS on this origin and keeps it across browser restarts. If persistence isn’t required, prefer sessionStorage or keeping it in-memory; if it is required, document the risk and consider scoping/expiry (e.g., timestamp + re-prompt).
No description provided.