fix(models): route the AI Gateway catalog fetchers through the credential-redirect guard

[briandevans]

What does this PR do?

hermes_cli/models.py has one opener for catalog HTTP: _urlopen_model_catalog_request, a thin wrapper over open_credentialed_url from hermes_cli/urllib_security.py. That module's docstring states the policy — "Security policy for credential-bearing stdlib urllib requests" — and the wrapper's own docstring states the guarantee: "Open catalog requests without forwarding headers across origins."

On main the module has eleven call sites going through that wrapper and three going through raw urllib.request.urlopen. All three raw ones are the AI Gateway catalog calls, and one of them carries a credential:

def _fetch_ai_gateway_models(timeout: float = 5.0) -> Optional[list[str]]:
    api_key = os.getenv("AI_GATEWAY_API_KEY", "").strip()
    ...
    base_url = os.getenv("AI_GATEWAY_BASE_URL", "").strip()   # user-settable
    ...
    headers = {"Authorization": f"Bearer {api_key}", ...}
    with urllib.request.urlopen(req, timeout=timeout) as resp:   # default redirect handling

Stdlib's default HTTPRedirectHandler re-sends every header it was given when it follows a redirect, including across origins. So a 302 from the configured gateway endpoint hands AI_GATEWAY_API_KEY to whatever host the Location header names. The endpoint is user-settable — the function reads AI_GATEWAY_BASE_URL from the environment and only falls back to the hermes_constants default when it is empty — so pointing the provider at a self-hosted or proxied gateway is enough to make the redirect reachable. The function swallows every exception and the picker path (_fetch_ai_gateway_models is called while building the AI Gateway model list) shows the built-in curated list on failure, so nothing surfaces to the user either way.

This is not a new class of bug in this repo — it is a hole a revert punched in an already-completed sweep. "fix(models): strip credentials on catalog redirects" and "fix(security): enforce one redirect credential policy" landed on 2026-07-11 and converted this file's catalog calls to the wrapper. These three functions were not in the file at the time: they had been deleted by "remove Vercel AI Gateway and Vercel Sandbox (#33067)" on 2026-05-27, so the sweep never saw them. On 2026-07-29 "Revert 'remove Vercel AI Gateway and Vercel Sandbox (#33067)'" restored them verbatim — including their pre-sweep raw urlopen calls and the eight pre-sweep patch("urllib.request.urlopen") mocks in their test file. git merge-base --is-ancestor confirms the sweep is an ancestor of the revert, so this is purely restored-stale code, not a design decision.

Same remedy and same shape as the merged "fix(providers): route Actual's fetch_models through the credential-redirect guard". The mock repoint is the same one merged as "fix(tests): patch catalog urlopen wrapper in gemini probe tests".

Related Issue

No filed issue — found by auditing this module's raw urllib.request.urlopen call sites against the redirect-credential policy the module already implements.

Type of Change

• 🐛 Bug fix (non-breaking change that fixes an issue)
• ✨ New feature (non-breaking change that adds functionality)
• 🔒 Security fix
• 📝 Documentation update
• ✅ Tests (adding or improving test coverage)
• ♻️ Refactor (no behavior change)
• 🎯 New skill (bundled or hub)

Changes Made

Three atomic commits.

1. fix(models): route the AI Gateway model probe through the catalog opener — hermes_cli/models.py, _fetch_ai_gateway_models. The credential-bearing site. Call-site change only: no new import (the file already imports open_credentialed_url and defines _urlopen_model_catalog_request a few lines below it), and no change to URL resolution, headers, or parsing.

2. fix(models): route the remaining AI Gateway catalog fetchers through the catalog opener — hermes_cli/models.py, fetch_ai_gateway_models and fetch_ai_gateway_pricing, plus the mock repoint in tests/hermes_cli/test_ai_gateway_models.py. These are the other two sites the same revert restored. They carry no credential header today, so this is not a second leak — but they are the only catalog fetchers in the module that bypass the wrapper, and the wrapper is where the module's whole HTTP policy lives, not only its redirect rule. open_credentialed_url resolves an explicit TLS context through _resolved_https_context, which honors HERMES_CA_BUNDLE, REQUESTS_CA_BUNDLE and CURL_CA_BUNDLE and falls back to certifi on macOS; plain urlopen honors none of those except SSL_CERT_FILE, which the stdlib reads on its own. So behind a TLS-inspecting proxy configured through Hermes, or on a macOS Python without a usable system trust store, these two calls fail verification while the module's other eleven succeed — and both swallow the exception, so the picker silently shows the built-in VERCEL_AI_GATEWAY_MODELS list with no live pricing and no error.

3. test(models): pin cross-origin credential stripping on the AI Gateway probe — tests/hermes_cli/test_ai_gateway_models.py. Wire-level regression test for _fetch_ai_gateway_models, which had no coverage at all.

Coverage of the root cause

Every raw urllib.request.urlopen call site in hermes_cli/models.py is converted; the file now has zero. Deliberately not widened beyond this file:

Elsewhere	Why not here
gateway/relay/media.py, gateway/relay/__init__.py	already covered by an open PR
dashboard_register.py, gateway_enroll.py, nous_account.py, nous_billing.py	already covered by an open PR
copilot_auth.py, web_server.py (ElevenLabs)	hardcoded hosts — no user-settable redirect origin
banner.py, browser_connect.py, debug.py, diagnostics_upload.py, model_catalog.py, security_audit.py, webhook.py, tui_gateway/methods_images.py	no credential header — outside the invariant

How to Test

uv run --with pytest --with pytest-asyncio python3 -m pytest \
  tests/hermes_cli/test_ai_gateway_models.py \
  tests/hermes_cli/test_urllib_security.py \
  tests/hermes_cli/test_models.py \
  tests/hermes_cli/test_model_validation.py -q

⇒ 85 passed on this branch (84 on main; the extra one is the new test).

Mutation proof for commit 1. Revert only the _fetch_ai_gateway_models call site to urllib.request.urlopen and re-run:

FAILED tests/hermes_cli/test_ai_gateway_models.py::test_ai_gateway_probe_drops_bearer_on_cross_origin_redirect
E   AssertionError: assert 'authorization' not in {'accept-encoding': 'identity',
E     'host': '127.0.0.1:65211', 'authorization': 'Bearer ai-gateway-secret',
E     'user-agent': 'hermes-cli/0.20.1', ...}
1 failed, 9 passed

The other nine tests stay green, so the failure isolates that one hunk. The sink is a second local server on a different port; the assertion is the header it actually received off the wire, with the real SafeCredentialRedirectHandler in the path — hermes_cli.urllib_security is not mocked. The test also asserts the redirect was followed and its payload parsed, so it cannot pass vacuously on a connection error, and that user-agent still arrives: the guard drops the credential, not every header.

Mutation proof for commit 2. Restore hermes_cli/models.py from origin/main (git checkout origin/main -- hermes_cli/models.py) with this branch's test file in place ⇒ 8 failed, 2 passed: the seven repointed tests plus the new redirect test. Restoring the branch's version ⇒ 10 passed.

Pre-existing baseline, unrelated to this PR. Running tests/hermes_cli/test_api_key_providers.py in the same session as the files above fails TestDeepInfraProviderProfile::test_profile_registered_with_alias_and_aux with KeyError: 'DEEPINFRA_API_KEY'. It reproduces identically on a clean origin/main worktree with the same command (1 failed, 188 passed there vs 1 failed, 189 passed here — the extra pass is the new test) and passes in isolation on both, so it is a cross-file ordering baseline, not a regression from this change.

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
• I searched for existing PRs to make sure this isn't a duplicate
• My PR contains only changes related to this fix/feature (no unrelated commits)
• I've run pytest tests/ -q and all tests pass — ran the four focused files above (85 passed), not the full suite
• I've added tests for my changes (required for bug fixes, strongly encouraged for features)
• I've tested on my platform: macOS (Darwin 25.4), Python 3.11.15 via uv run

Documentation & Housekeeping

• I've updated relevant documentation (README, docs/, docstrings) — the test module docstring and the _mock_urlopen helper docstring, which named urlopen() after the repoint
• I've updated cli-config.yaml.example if I added/changed config keys — N/A
• I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A
• I've considered cross-platform impact (Windows, macOS) per the compatibility guide — no platform-specific code; the new test binds 127.0.0.1 on an ephemeral port, the same pattern tests/hermes_cli/test_urllib_security.py already uses in CI
• I've updated tool descriptions/schemas if I changed tool behavior — N/A

[Copilot]

Pull request overview

This PR closes a security gap in hermes_cli/models.py by routing the AI Gateway catalog fetchers through the existing guarded catalog opener, ensuring credential-bearing requests do not forward headers across cross-origin redirects and that catalog fetches follow the module’s established urllib/TLS policy.

Changes:

• Switched the remaining AI Gateway catalog fetchers in hermes_cli/models.py from raw urllib.request.urlopen(...) to _urlopen_model_catalog_request(...).
• Updated existing AI Gateway model/pricing tests to patch _urlopen_model_catalog_request instead of urllib.request.urlopen.
• Added a wire-level regression test that verifies Authorization is stripped when a request redirects to a different origin.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
hermes_cli/models.py	Routes AI Gateway catalog requests through the guarded catalog opener to enforce redirect/header and TLS policy consistently.
tests/hermes_cli/test_ai_gateway_models.py	Repoints mocks to the guarded opener and adds a redirect regression test verifying credential stripping on cross-origin redirects.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[egilewski]

looks mergeable

All three AI Gateway model-catalog requests in the patch now use the existing credential-safe opener. Current main's raw urlopen path could forward Authorization or installed-opener credentials across a redirect. The replayed patch changes only those sinks and adds regression coverage; source review, focused catalog tests, and direct redirect-handler probes found no residual credential-forwarding bypass or unrelated security regression.

Security evidence:

• trust boundary: Untrusted inputs are AI_GATEWAY_BASE_URL, catalog redirect locations, response payloads, and headers injected by an installed urllib opener. The secret source is AI_GATEWAY_API_KEY in _fetch_ai_gateway_models plus any installed-opener credentials; the sink is each /models request and redirect hop in hermes_cli/models.py. The shared credential-safe opener is the enforcement boundary.
• source/sink/invariant: Each changed call constructs a Request and invokes _urlopen_model_catalog_request, which delegates to open_credentialed_url. SafeCredentialRedirectHandler compares normalized origins and removes every non-allowlisted header on cross-origin redirects; _CrossOriginRequestSanitizer runs after installed processors. Same-origin headers remain available, while Authorization, Cookie, and arbitrary credential headers cannot be re-added on a later cross-origin hop.
• current-main reproduction: Current-main source used raw urllib.request.urlopen in fetch_ai_gateway_models, fetch_ai_gateway_pricing, and _fetch_ai_gateway_models. Those calls entered urllib's default redirect path without origin-aware header sanitization. The replayed patch replaces each call with _urlopen_model_catalog_request while preserving existing parsing and fallback behavior.
• PR-head or patch-replay validation: A run-owned patch replay against current GitHub main was reviewed. It changes exactly the three AI Gateway catalog opener calls and updates their corresponding tests; the receiving helper and shared security tests were inspected.
• positive/negative cases: Focused catalog tests cover pricing translation, malformed pricing filtering, model filtering, free-model tagging and promotion, and fallback/error handling. Direct handler probes cover same-origin credential preservation, cross-origin removal of Authorization and Cookie, and multi-hop no-resurrection. The added regression test covers a redirect that still returns a valid model payload while asserting the credential is absent and the User-Agent survives.
• residual bypass search: All AI Gateway catalog and model-discovery call sites in hermes_cli/models.py were searched. The three relevant requests now route through _urlopen_model_catalog_request, and no AI_GATEWAY_BASE_URL request remains on raw urllib.request.urlopen. Other provider paths are either already on the shared helper or do not carry this API key.
• reviewer validation: Reviewed the changed source and diff for data flow from API key and base URL to network sinks, inspected the opener-cloning and sanitizer implementation, ran focused catalog tests and direct sanitizer probes. No source-backed security finding remains.

Not checked:

• loopback wire redirect execution
• ruff lint execution
• live network integration
• CodeRabbit review

Signed: GPT-5.6-luna-max in Codex

[Enough1122]

AI code review — automated review for reference, author can ignore or act on any point.

fix(models): route the AI Gateway catalog fetchers through the credential-redirect guard — No blocking issues found. A few minor observations:

1. The redirect regression test (test_ai_gateway_probe_drops_bearer_on_cross_origin_redirect) is well built: it drives a real redirect to a loopback sink and asserts both that the credential never crosses and that benign headers survive.

2. Minor: the _serve helper never calls server_close() — shutdown() stops serve_forever but the listening socket is released only at GC. Harmless at this scale, but calling server_close() in the test's finally makes teardown deterministic.

3. Minor: the tests now patch hermes_cli.models._urlopen_model_catalog_request rather than urllib.request.urlopen. This pins the three known call sites to the guarded opener, but a future catalog call added via raw urlopen would silently bypass the guard and stay green. The redirect test covers the probe path only; consider extending the same wire-level pattern to fetch_ai_gateway_models and fetch_ai_gateway_pricing so all three sinks are regression-guarded identically.

[andrexibiza]

No blocking findings.

I traced all three changed sinks into _urlopen_model_catalog_request and then through open_credentialed_url. The enforcement boundary is sound: redirects are compared against the original normalized origin, cross-origin requests retain only Accept and User-Agent, and the final request sanitizer prevents installed urllib processors or opener-level headers from resurrecting credentials after the redirect handler runs.

I also replayed the wire behavior independently with the same request shape. Raw urllib.request.urlopen forwarded Authorization: Bearer ai-gateway-secret to the redirected origin. The guarded opener kept the bearer on the initial request, removed it from the cross-origin hop, still followed and parsed the response, and preserved Accept and User-Agent. That confirms the new regression is exercising the real leak rather than passing on a failed redirect.

The test split is sufficient. The credential-bearing _fetch_ai_gateway_models path gets the end-to-end loopback regression. The other two call sites are pinned to _urlopen_model_catalog_request by their mocks: reverting either to raw urlopen bypasses the mock and breaks the existing behavior tests. Their redirect/TLS semantics are already covered centrally by test_urllib_security.py; duplicating the wire harness for credential-free requests would add little signal. Likewise, adding server_close() would be deterministic cleanup, but it is non-blocking and the existing shared wire tests use the same shutdown() pattern.

Current main still contains the same three raw AI Gateway calls, so the fix has not been superseded. CI and Docker workflows are green at 3f61a27. The branch is 587 commits behind current main, so the head needs refreshing before merge, but I found no source-backed defect in the patch itself.