Skip to content

[security] fix(api-server): scope run approvals by run id - #56129

Merged
teknium1 merged 1 commit into
mainfrom
hermes/hermes-f8d4d371
Jul 1, 2026
Merged

[security] fix(api-server): scope run approvals by run id#56129
teknium1 merged 1 commit into
mainfrom
hermes/hermes-f8d4d371

Conversation

@teknium1

@teknium1 teknium1 commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Summary

API-server /v1/runs dangerous-command approval queues are now scoped by the generated run_id, so one API run can no longer resolve another run's pending approval.

Root cause: the approval queue key was gateway_session_key or session_id or run_id. session_id is caller-controllable continuity metadata that concurrent runs can intentionally share, so two runs with the same session_id shared one approval queue — and resolve_all=true on one run drained the other run's pending dangerous-command approval.

Changes

  • gateway/platforms/api_server.py: approval_session_key = run_id (was gateway_session_key or session_id or run_id). The two read sites (/v1/runs/{run_id}/approval, stream sweeper) already pull the stored key from _run_approval_sessions[run_id], so they inherit the fix.
  • tests/gateway/test_api_server_runs.py: regression — two runs with the same session_id, separate pending approvals, resolve_all=true on the attacker run resolves exactly one and leaves the victim entry untouched.

session_id / X-Hermes-Session-Key remain the conversation/memory scope. once / session / always / deny all preserved, now scoped to the target run.

Validation

Before After
resolve_all=true on attacker run drains shared queue → resolved: 2, victim unblocked drains only target run → resolved: 1, victim untouched
  • scripts/run_tests.sh tests/gateway/test_api_server_runs.py -q → 23/23 pass
  • E2E: real resolve_gateway_approval(run_id, resolve_all=True) against two seeded queues — attacker queue drained, victim's pending approval event stays unset.

Salvaged from #24212 by @Hinotoi-agent; commit re-anchored to current main (original diff was rooted at an older revision), authorship preserved.

Infographic

infographic

@alt-glitch alt-glitch added type/security Security vulnerability or hardening comp/gateway Gateway runner, session dispatch, delivery area/auth Authentication, OAuth, credential pools sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data P2 Medium — degraded but workaround exists labels Jul 1, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Salvage of #24212 by @Hinotoi-agent (commit re-anchored to current main, authorship preserved) -> related, not duplicate. Same api-server run-scoping family as #42459 (session-key scoping) and #45888 (Responses-path approval wiring).

@teknium1
teknium1 merged commit 66325a7 into main Jul 1, 2026
31 checks passed
@teknium1
teknium1 deleted the hermes/hermes-f8d4d371 branch July 1, 2026 07:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/auth Authentication, OAuth, credential pools comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/security Security vulnerability or hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants