proxy: add opt-in inbound bearer middleware (HERMES_API_KEY)

[Slimydog21]

Summary

Adds an opt-in inbound bearer middleware to hermes proxy. When HERMES_API_KEY resolves to a non-empty value (via hermes_cli.config.get_env_value so ~/.hermes/.env is honored), the proxy enforces a constant-time bearer match on every inbound /v1/* request. When unset, the legacy localhost-only behavior is preserved unchanged — existing nous-on-127.0.0.1 deployments are not regressed.

Why

hermes proxy today assumes the bind-to-127.0.0.1 boundary is the security perimeter. That's fine for the nous upstream when callers are the operator's own local apps. It's not sufficient the moment the proxy is exposed beyond localhost — e.g. a Cloudflare Tunnel terminating at the proxy's 127.0.0.1:8645 to make Hermes-managed OAuth-backed inference reachable from another machine (the case the new xai-oauth upstream is being used for).

Without this gate, anyone who learns the tunnel URL can spend the operator's OAuth-attributed quota anonymously.

Design choices

• Opt-in, not mandatory: deployments that don't set the env var keep working unchanged.
• /health exempt: operators need to probe status without sending the credential.
• Constant-time compare via secrets.compare_digest — defense-in-depth.
• Inbound bearer replaced before forwarding: preserves the existing contract that the client's Authorization header never leaks to upstream.
• Env resolution via hermes_cli.config.get_env_value so ~/.hermes/.env (the standard Hermes location) is honored, matching the pattern already used by tools/xai_http.py:resolve_xai_http_credentials.

Tests

7 new tests, all green. The full proxy test file now has 35 passing:

• env unset → no enforcement (legacy contract preserved)
• env set, missing Authorization → 401 with code inbound_auth_missing
• env set, wrong token → 401 with code inbound_auth_mismatch
• env set, malformed scheme (e.g. Basic) → 401
• env set, matching token → request forwarded; inbound bearer NOT leaked to upstream (replaced by adapter's bearer per the existing contract)
• /health always open regardless of env
• _resolve_inbound_bearer strips whitespace + treats empty as unset

Test plan

• pytest tests/hermes_cli/test_proxy.py — 35 passed
• Manual verification: started hermes proxy start --provider xai-oauth with HERMES_API_KEY set, confirmed unauthenticated requests get 401 and matching-bearer requests forward to xAI cleanly

[magnus919]

Code Review Summary

Verdict: Comment — clean, well-structured implementation. One minor style nit, nothing blocking.

What it does

Adds an opt-in inbound bearer auth middleware to hermes proxy. When HERMES_API_KEY is set (via env or ~/.hermes/.env), the proxy requires Authorization: Bearer <token> on every inbound request except /health. When unset, legacy localhost-only behavior is preserved.

✅ Looks Good

• Opt-in by design: unset env var = no behavior change. Existing localhost deployments are not regressed.
• Constant-time compare: uses secrets.compare_digest — correct for bearer token comparison.
• /health exempted: operators can probe without credentials.
• Inbound bearer stripped before forwarding: preserves the existing contract that client auth doesn't leak upstream.
• Env resolution via get_env_value: honors ~/.hermes/.env, matching the pattern used elsewhere in the codebase (tools/xai_http.py).
• Error codes are distinct: inbound_auth_missing vs inbound_auth_mismatch — useful for debugging.
• Test coverage: 7 new tests covering unset, missing, wrong, malformed, matching, health exemption, and whitespace stripping. 35 total passing.

💡 Minor suggestion (non-blocking)

The import secrets as _secrets is inside the middleware handler function, meaning it runs on every request. For a string comparison on a hot path, this is trivially fast, but conventionally these imports go at module level. Consider moving it to the top of the function or module.

Summary

Solid, well-tested auth middleware for the proxy. The opt-in design correctly avoids regressing localhost-only deployments while securing exposed endpoints. The implementation is straightforward and the test coverage is thorough.

[teknium1]

Thanks for the security-focused proxy hardening. I verified the premise against current main: create_app() still builds a plain web.Application() in hermes_cli/proxy/server.py:92, while docs still warn that exposed proxy instances accept any bearer at website/docs/user-guide/features/subscription-proxy.md:171.

Problems

• The behavior change needs user-facing docs/help updates. Current docs say the proxy ignores client auth at website/docs/user-guide/features/subscription-proxy.md:65 and has no auth of its own at website/docs/user-guide/features/subscription-proxy.md:171; CLI/help copy says “any bearer” at hermes_cli/proxy/cli.py:62 and hermes_cli/subcommands/gateway.py:251.
• HERMES_API_KEY is a broad name that current main already uses as a generic API key fallback in tui_gateway/server.py:9851. A proxy-specific secret name would reduce surprising opt-in behavior.

Suggested changes

• Add a short “optional inbound bearer” section to the subscription proxy docs and env reference.
• Consider renaming the secret to a proxy-specific key such as HERMES_PROXY_API_KEY / HERMES_PROXY_INBOUND_BEARER.

This is an automated hermes-sweeper review.

[teknium1]

Consider a proxy-specific secret name here. Current main already references HERMES_API_KEY as a generic API key in tui_gateway/server.py:9851, so this could silently enable proxy inbound auth for users who set that generic key for another purpose.

[teknium1]

Thanks for the security-focused proxy hardening. The premise still holds on current main: create_app() has no inbound auth middleware at hermes_cli/proxy/server.py:96, and the user guide warns that LAN exposure accepts any bearer at website/docs/user-guide/features/subscription-proxy.md:171.

Problems

• HERMES_API_KEY is already read as a generic API-key fallback by tui_gateway/server.py:13920; using it here can unexpectedly turn on proxy authentication for an inherited generic key.
• The changed contract is not reflected in current docs/help: subscription-proxy.md:65,171, hermes_cli/proxy/cli.py:62, and hermes_cli/subcommands/gateway.py:312 still say the client may use any bearer.
• The new resolver test covers os.environ, but not the documented .env fallback implemented by hermes_cli/config.py:7754-7756.

Suggested changes

• Use and document a proxy-specific secret name.
• Update proxy documentation, CLI/help copy, and the environment-variable reference.
• Add an isolated temporary-HERMES_HOME test for .env resolution.
• Preserve client_max_size=MAX_REQUEST_BYTES when resolving the current constructor conflict from 8986981df.

This is an automated hermes-sweeper review.

[GottZ]

This was generated by AI during triage.

Summary

Three PRs in this complex modify proxy behavior, but they address distinct causes: #28077 adds optional inbound authentication, #29279 closes upstream resources when response preparation fails, and #63579 corrects timeout classification while adding broader failure-path tests.

Related pull requests

• #28077 related — (+274/-1) — keep open for revision: the diff adds constant-time opt-in bearer enforcement and preserves upstream credential replacement, directly addressing unauthenticated exposure. Consistent with the keep_open reviews on #28077, it should use a proxy-specific environment variable, update documentation/help that currently promises “any bearer,” and test the documented .env resolution path before merge.
• #29279 related — (+102/-14) — merge independently: the diff moves StreamResponse preparation inside the existing cleanup try/finally, so prepare failures release the upstream response and close its ClientSession; the regression test exercises that exact leak path. This matches the keep_open review on #29279 and does not overlap #28077’s authentication change.
• #63579 [closed] related — (+129/-9) — separate closed fix, still relevant: the diff orders asyncio.TimeoutError before aiohttp.ClientError so ServerTimeoutError produces the intended 504 rather than 502, with route-level regression coverage; its additional BlueBubbles tests are unrelated to the proxy cause. The positive keep_open review on #63579 confirms the timeout bug remained on main, but this is not an implementation of #28077 or #29279.

Suggested consolidation

Merge #29279 after CI as the focused resource-cleanup fix; keep #28077 open until its contributor-requested naming, documentation/help, and .env-test gaps are resolved. Do not close any of these PRs as duplicates: #63579’s closed timeout-classification fix should be evaluated separately rather than consolidated into either open PR.

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

[magnus919]

@GottZ the botslop is getting kind of excessive. Especially when the primary contributor has already weighed in and your bot is just adding noise to all of our inboxes.

[GottZ]

@GottZ the botslop is getting kind of excessive. Especially when the primary contributor has already weighed in and your bot is just adding noise to all of our inboxes.

@magnus919 That is fair, and the specific version of it is fair too: teknium1 had already posted a keep_open review here on July 13, and my comment largely restated it. A thread where the maintainer has already ruled is exactly where an automated cross-PR note adds the least and costs the most. I am adding a suppression rule for that case — if a maintainer review already exists on the target with appropriate recentcy, the triage comment gets dropped rather than posted.

On the linking, it may help to see what the filter removed rather than what it kept. The visible links are the residue, not the output.

For this PR specifically
click here to see this pr's complex

• 90 PRs touch at least one file that this one touches; 26 of those are still open.
• 25 of those 26 were deliberately not mentioned. Exactly one made it into the comment.
• 15 PRs surfaced as embedding-level duplicate candidates (similarity 0.66–0.76). All 15 were rejected — none entered the complex.
• What survived was a three-node complex and a single comment, whose main finding is negative: those three PRs are not duplicates of each other and should not be consolidated. One is an independent resource-cleanup fix that can merge on its own; the other is a separate, already-closed timeout fix that should not be folded in.

That negative result is the point. The expensive failure mode in a backlog this size is not a missing link — it is closing a PR as a duplicate when it actually fixes something else, or merging one of six identical PRs while the other five sit open for months.

Why this exists at all

The repository currently has 17,773 open PRs and 8,387 open issues, and roughly 30% of the open PRs are likely duplicates of another open PR. At that scale nobody can answer "has someone already fixed this, and can I close five of these six?" by reading threads. The graph answers that from the diffs rather than the titles, so review capacity goes to distinct work instead of the same fix six times over.

One current example, deliberately without issue numbers so this comment does not ping seventeen more threads: ten open PRs are all fixing the same CPython 3.14 ThreadPoolExecutor change, and seven more have already been closed as duplicates of that same set. Nine of the ten are the same small guard in different wrappers.
click here to see that complex

On inbox noise, honestly

Across all targets, 90.5% received exactly one comment (3,229 of 3,569), averaging 1.17 per target. One cross-PR note per thread is the intent, not a conversation. But the tail is real: a handful of threads received up to nine, and the maintainer-already-reviewed case above should never have been posted at all. Both are worth fixing, and I would rather hear it than not.

If you would prefer no automated triage comments on threads you are involved in, say so and I will exclude you — no argument needed.