fix(api_server): scope Runs API control endpoints by X-Hermes-Session-Key

[necoweb3]

Summary

Fixes cross-session authorization gaps in gateway/platforms/api_server.py where /v1/runs follow-up endpoints (GET /v1/runs/{run_id}, GET /v1/runs/{run_id}/events, POST /v1/runs/{run_id}/approval, POST /v1/runs/{run_id}/stop) only validated the bearer token and run_id, ignoring X-Hermes-Session-Key.

Why

In multi-tenant or multi-client API server deployments sharing a single API_SERVER_KEY, frontends and client platforms rely on X-Hermes-Session-Key for per-user/channel session isolation. On main, any client knowing a run_id could query run status, stream lifecycle events, resolve approval prompts, or abort another session's active run.

With this change, follow-up endpoints enforce caller ownership matching the creator's X-Hermes-Session-Key. Cross-key requests return HTTP 404 (run_not_found) to prevent leaking run existence across session boundaries.

Key Changes

• APIServerPlatform: Record creator X-Hermes-Session-Key in _run_session_keys[run_id].
• Authorization Guard: Added _authorize_run_session_key(request, run_id) returning 404 if caller session key doesn't match the run owner.
• Wired Handlers: Applied guard to _handle_get_run, _handle_run_events, _handle_run_approval, and _handle_stop_run.
• Cleanup: Clean up stored session keys upon run completion or eviction.
• Tests: Added regression test cases in tests/gateway/test_api_server_runs.py verifying status, events, approval, and stop access isolation.

Test

python -m pytest tests/gateway/test_api_server_runs.py -q --timeout-method=thread

[alt-glitch]

This was generated by AI during triage.

Related to #24212's run-approval isolation work. This patch instead introduces per-session-key ownership checks across the Runs control surface; SECURITY.md currently treats session identifiers as routing handles, so maintainers should decide whether that trust-model change is intended.

[teknium1]

Thanks for tracing all four Runs control paths and adding focused active-run regressions.

Problems

• X-Hermes-Session-Key is currently a caller-supplied long-term-memory scope (website/docs/user-guide/features/api-server.md:489-500), while SECURITY.md:210-217 defines session identifiers as routing handles rather than authorization boundaries and treats all callers with an adapter's authorization as equally trusted. Turning this header into a per-caller capability needs an explicit maintainer trust-model decision and corresponding documentation.
• The new cleanup at gateway/platforms/api_server.py:4690 removes the owner mapping when the task ends, but terminal statuses remain for _RUN_STATUS_TTL = 3600 seconds (gateway/platforms/api_server.py:4284-4285). _authorize_run_session_key() allows requests with no mapping (gateway/platforms/api_server.py:4311-4312), so a different key can read retained terminal status after completion. The same gap can apply while a completed stream transport remains retained. The new status test keeps the run active through its checks (tests/gateway/test_api_server_runs.py:298-331).

Suggested changes

• Resolve the authorization-model decision before landing; if accepted, document the changed API-server contract.
• Keep ownership metadata through the status/stream retention lifetimes, and add terminal-status and retained-stream cross-key regressions.

Automated hermes-sweeper review.

[teknium1]

X-Hermes-Session-Key is presently a caller-supplied memory/routing scope, not an authenticated caller identity (SECURITY.md:210-217; website/docs/user-guide/features/api-server.md:489-500). Using it as the owner capability changes the documented authorization model and needs an explicit maintainer decision plus documentation.

[teknium1]

Do not remove ownership here while _run_statuses and possibly _run_streams remain retained. _authorize_run_session_key() permits access when the mapping is absent, so a different key can read a terminal status retained for _RUN_STATUS_TTL. Retain this mapping until the corresponding retained status and transport state are evicted.

[GottZ]

This was generated by AI during triage.

Summary

Two PRs address the same cross-session Runs API concern: both record the creator's X-Hermes-Session-Key, enforce it across status, events, approval, and stop endpoints, and add wrong-key regression tests. Their shared implementation does not resolve whether a documented routing handle should become an authorization boundary, and it removes ownership before retained terminal state is evicted.

Related pull requests

• #42459 [closed] duplicate — (+152/-0) — duplicate predecessor: The closed PR introduced the same owner map, four endpoint guards, lifecycle cleanup, 404 behavior, and active-run tests later carried by #70385; it remains relevant as the near-identical earlier implementation and review history, but its cleanup has the same retained-state authorization gap.
• #70385 related — (+152/-1) — keep open with a salvage path: The diff usefully traces all four Runs control paths and adds focused wrong-key tests, consistent with the visible keep_open review on #70385. Before acceptance, it needs an explicit maintainer decision and documentation for changing X-Hermes-Session-Key from a routing handle into an ownership capability, plus owner-map retention through status and stream eviction and regression coverage for completed retained runs.

Duplicates

#42459 and #70385 are substantively duplicate implementations of the same session-key owner map, endpoint guards, cleanup, and active-run regression coverage; #42459 is the closed predecessor of #70385.

Suggested consolidation

Keep #70385 open with a salvage path: retain its four endpoint checks and focused regression tests, but first resolve the documented trust-model conflict and fix the lifecycle gap identified in the keep_open review by retaining ownership until the corresponding status and transport state are evicted and testing post-completion access. Treat closed #42459 as the duplicate predecessor and consolidate any useful review history into #70385 rather than reviving it.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    subgraph Dup42459 ["PRs duplicating each other"]
        P42459["PR #42459 (closed)"]
        P70385["PR #70385 (open)"]
    end
    class P42459 closed
    class P70385 open
    class P70385 target
    click P42459 "https://github.com/NousResearch/hermes-agent/pull/42459"
    click P70385 "https://github.com/NousResearch/hermes-agent/pull/70385"

Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 2 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 23 kB of PR diffs, 3 kB of issue/PR text, 3 kB of discussion (6 comments), 0 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.