fix: harden dashboard security and runtime status - #61305
AndreasG78 wants to merge 4 commits into
Conversation
|
Local verification completed after rebasing this PR onto current Evidence:
The upstream Actions run for this fork PR is currently |
|
Fork-local CI is green for the current head f55a3f1a36d43423e47b9e6f50296bd3ece8301a. Evidence: https://github.com/AndreasG78/hermes-agent/actions/runs/28998070436 |
- add CSRF/Origin checks for cookie-auth mutating dashboard requests - block sensitive file API access for credential stores - fix stale dashboard detection and profile-specific gateway liveness - improve modal inertness/focus behavior and secret input UX Verification: targeted Python tests, web lint/typecheck/tests/build, runtime smoke.
7c7bdf1 to
396c8aa
Compare
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the concrete dashboard hardening and profile-status coverage. The underlying main-branch gaps are real: hermes_cli/web_server.py:2017-2130 lacks raw-filesystem sensitive-path checks, and hermes_cli/web_server.py:2455-2507 uses process-scoped gateway identity defaults while serving a selected profile.
Problems
- The new CSRF check at
hermes_cli/dashboard_auth/middleware.py:481is bypassed forPOST /auth/logout, because_GATE_PUBLIC_PREFIXESincludes that route at current-mainmiddleware.py:47. The handler revokes its refresh token and clears session cookies (dashboard_auth/routes.py:556-583). Please protect this cookie-mutating route too and add same-origin/cross-origin logout coverage. gateway/run.py:8993-9027adds apending_self_reset.jsonprotocol that is outside this PR's stated scope. A current-main search found no producer for that state file, and the changed restart tests only assert prompt text. Please split it into a focused, end-to-end-tested gateway change.
Suggested changes
- Preserve unauthenticated logout behavior if needed, but validate Origin/Referer when cookie-authenticated logout performs revocation.
- Keep the dashboard security/status salvage focused on the tested dashboard changes.
Automated hermes-sweeper review.
| return response | ||
|
|
||
| request.state.session = session | ||
| if request.method.upper() in {"POST", "PUT", "PATCH", "DELETE"} and not _csrf_origin_ok(request): |
There was a problem hiding this comment.
This check does not cover POST /auth/logout: _path_is_public() returns earlier for that route, while its handler revokes the refresh token and clears session cookies. Please apply the same origin validation to cookie-bearing logout and add same-origin/cross-origin coverage.
| # consumed as update answers instead of being dispatched normally. | ||
| _quick_key = self._session_key_for_source(source) | ||
|
|
||
| # Profile-local context-hygiene watchdogs may request a silent session |
There was a problem hiding this comment.
This introduces a new pending_self_reset.json state-file protocol outside the PR summary. Current main has no producer for this file, and the changed tests exercise only restart-resume prompt wording. Please split this into a focused gateway change with an end-to-end producer/consumer test.
|
too large to review safely This PR changes 553 production lines before tests and docs. Please split it or add a focused justification if it should stay together. Signed: GPT-5.6-luna-high in Codex |
fix: harden dashboard security and runtime status
|
|
+1 for the Origin/Referer check on cookie-authenticated mutating routes — we have been running an equivalent local patch on a self-hosted 0.21.3 dashboard since September 2026 without side effects. One argument that may help prioritise this: the current protection against CSRF is effectively Two details from our version, in case useful:
Minor, related: |
Summary
/api/status?profile=...Verification
.venv/bin/python -m py_compile hermes_cli/main.py hermes_cli/dashboard_auth/middleware.py hermes_cli/web_server.pyscripts/run_tests.sh tests/hermes_cli/test_update_stale_dashboard.py tests/hermes_cli/test_dashboard_auth_middleware.py tests/hermes_cli/test_web_server_fs.py tests/hermes_cli/test_web_server_gateway_topology.py -q→ 103 passednpm run --workspace web lint -- --max-warnings=0npm run --workspace web typechecknpm run --workspace web test -- --run→ 6 files / 33 tests passednpm run --workspace web build→ passed; existing Vite chunk-size warning remainsgit diff --check127.0.0.1:9119:/api/status?profile=assistantreportsgateway_running=True, dashboard status reports exactly one server process, stale detector returns the dashboard child PIDNotes