Skip to content

fix(models): use certifi CA bundle for live model-list probes - #51563

Closed
isaachuangGMICLOUD wants to merge 1 commit into
NousResearch:mainfrom
isaachuangGMICLOUD:fix/certifi-live-model-probes
Closed

fix(models): use certifi CA bundle for live model-list probes#51563
isaachuangGMICLOUD wants to merge 1 commit into
NousResearch:mainfrom
isaachuangGMICLOUD:fix/certifi-live-model-probes

Conversation

@isaachuangGMICLOUD

Copy link
Copy Markdown
Contributor

What does this PR do?

The live /models probe (probe_api_models) is a hand-rolled urllib call that trusts Python's default OpenSSL trust store. On python.org macOS installs that never ran "Install Certificates.command" that store is empty, so the probe fails with CERTIFICATE_VERIFY_FAILED even though the server cert is valid - and the picker silently degrades to the static fallback list. That is exactly how a GMI partner concluded GLM-5.2 was "not supported" when GMI does serve it live (zai-org/GLM-5.2-FP8).

curl and Hermes's own chat path both work on the same machine, because the OpenAI SDK / httpx / requests already bundle and use certifi. Only these hand-rolled urllib probes were left relying on the (often empty) system store. This PR brings the straggler in line.

Related Issue

Related to #51533 (the visibility/warning + GLM-5.2 catalog change). This PR is the root-cause fix; #51533 makes any residual fetch failure visible. Both touch probe_api_models, so whichever merges second needs a one-line conflict resolution (the line just before the probe loop and the urlopen(...) call).

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

  • hermes_cli/models.py
    • Add _models_ssl_context(): builds ssl.create_default_context() (system trust store) and unions certifi.where() on top of it. Returns None on any error so the probe falls back to urllib's exact previous behaviour.
    • probe_api_models(): pass that context to urllib.request.urlopen(...).
  • tests/hermes_cli/test_models.py: add TestModelsSSLContext (context has a non-empty trust store; the probe passes an SSLContext to urlopen).

How to Test

  1. pytest tests/hermes_cli/test_models.py -q -> passes.
  2. End-to-end: with no SSL_CERT_FILE set on a machine whose Python has an empty default trust store, provider_model_ids("gmi") now returns the full live list (incl. zai-org/GLM-5.2-FP8) instead of the static fallback. Before this change the same call returned only the static fallback.

Why union (not replace)?

Using create_default_context(cafile=certifi.where()) would replace the trust store and drop any corporate/internal root that only lives in the system store. Starting from create_default_context() and adding certifi via load_verify_locations makes the trust set a superset of the old default - it can only add trust, never remove it - so no probe that verified before can start failing.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix
  • I've run the relevant suite: tests/hermes_cli/test_models.py (78 passed). Full pytest tests/ -q has pre-existing collection errors in unrelated tests/acp/* modules, not caused by this PR.
  • I've added tests for my changes
  • I've tested on my platform: macOS 15.5 (Darwin 24.5.0)

Documentation & Housekeeping

  • Documentation: N/A (no user-facing config/behaviour to document beyond the docstring)
  • cli-config.yaml.example: N/A (no config keys changed)
  • CONTRIBUTING.md / AGENTS.md: N/A (no architecture/workflow change)
  • I've considered cross-platform impact: union trust store is a superset of the default on every platform; certifi is already a pinned dependency; helper returns None on error
  • I've updated tool descriptions/schemas: N/A (no tool behavior changed)

Screenshots / Logs

Before (empty default trust store): provider_model_ids("gmi") -> 6 static models, no GLM-5.2.
After: provider_model_ids("gmi") -> 65 live models incl. zai-org/GLM-5.2-FP8, with no SSL_CERT_FILE set.

The live /models probe is a hand-rolled urllib call that trusts Python's
default OpenSSL trust store. On python.org macOS installs that never ran
"Install Certificates.command" that store is empty, so the probe fails with
CERTIFICATE_VERIFY_FAILED even though the cert is valid — and the picker
silently degrades to the static fallback list (e.g. GLM-5.2 on GMI looked
"unsupported"). curl and the chat path both work because the OpenAI SDK /
httpx / requests already bundle certifi; only these hand-rolled probes were
left relying on the system store.

Union certifi (a pinned dependency) on top of the system trust store in
probe_api_models so these probes match what the rest of Hermes already does.
Union (not replace) keeps any corporate/internal CA that only lives in the
system store; the helper returns None on any error, so a probe that worked
before can never start failing.

Verified: with no SSL_CERT_FILE set, provider_model_ids("gmi") returns the
full live list (incl. zai-org/GLM-5.2-FP8) instead of the static fallback.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@isaachuangGMICLOUD
isaachuangGMICLOUD force-pushed the fix/certifi-live-model-probes branch from 6bade10 to 83598a7 Compare June 23, 2026 22:38
@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists labels Jun 23, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for tracing the macOS certificate-store failure. The underlying gap is still present on current main, but this branch predates the catalog-request security refactor.

Problems

  • probe_api_models() now calls _urlopen_model_catalog_request() at hermes_cli/models.py:3608, not urllib.request.urlopen() directly. The shared wrapper was introduced by 6e75ba7fa066c1667cdc09db02c0c1dca8cd4671 and its wire-level regression test ensures probe_api_models() does not leak arbitrary credential headers across redirects (tests/hermes_cli/test_urllib_security.py:409). A direct urlopen(..., context=...) salvage would bypass that protection.
  • The same wrapper serves other live catalogs, including OpenRouter (hermes_cli/models.py:1409) and GitHub Copilot (hermes_cli/models.py:2881), so the CA-context handling should be added at the shared wrapper layer.
  • The added urllib.request.urlopen mock no longer covers the current probe path.

Suggested changes

  • Thread a certifi-unioned SSL context through open_credentialed_url() while preserving its installed-opener and redirect-sanitization behavior, then test through that current path.

Automated hermes-sweeper review.


def fake_urlopen(req, timeout=None, context=None):
captured["context"] = context
raise OSError("stop after capturing")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current main no longer invokes urllib.request.urlopen from probe_api_models; it calls _urlopen_model_catalog_request() (hermes_cli/models.py:3608) so the credential-safe redirect wrapper stays in force. Please retarget this test to the current wrapper path when salvaging the SSL-context change.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 15, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Closing as resolved on main — you diagnosed this correctly and first, and the fix that landed covers your scenario through a different seam. Two things happened since this PR was opened:

  1. probe_api_models no longer calls raw urllib.request.urlopen — it routes through open_credentialed_url / the secured opener (so this branch's patch target is gone), and
  2. fix(models): restore authenticated Actual discovery on macOS #86492 (merged today) gave Hermes-owned urllib openers an explicit CA context: HERMES_CA_BUNDLE/SSL_CERT_FILE/REQUESTS_CA_BUNDLE/CURL_CA_BUNDLE first, then certifi on macOS — which fixes the empty python.org trust-store case this PR targeted. Per-provider ssl_ca_cert/ssl_verify additionally reach the probe via fix(providers): honor per-provider TLS on custom /models and pricing probes #86491.

One deliberate difference from your approach: the landed fix uses certifi as a macOS fallback rather than unioning certifi on top of the system store everywhere — corporate/internal CAs in the system store stay authoritative on Linux. If you hit a concrete case the fallback misses (e.g. empty store on a non-macOS platform), please open an issue.

Thanks for the clear root-cause writeup — sorry this one sat long enough to be overtaken.

@teknium1 teknium1 closed this Aug 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants