fix(models): restore authenticated Actual discovery on macOS - #86492
Merged
Conversation
Contributor
૮ >ﻌ< ა ci reviewran on 2021341 — chore: map contributor email
|
…direct guard ActualProfile.fetch_models() overrides ProviderProfile's default implementation with its own Actual-specific base_url resolution (ACTUAL_BASE_URL env var, hosted-vs-local normalization), but called raw urllib.request.urlopen(req, timeout=timeout) directly instead of the base class's open_credentialed_url(). Every other provider either uses the base class default or forwards to it via super() and gets SafeCredentialRedirectHandler for free — Actual is the only provider that attaches a Bearer token to its own Request object and opens it with the stdlib's default redirect handling, which forwards every header, including Authorization, across a cross-origin redirect. Actual's own feature surface makes the trigger realistic: ACTUAL_BASE_URL is a first-class, documented way to point this provider at a self-hosted or local-offline endpoint (see the local-loopback no-auth path already handled elsewhere in this provider), so a misconfigured or compromised endpoint 302-ing to another host leaks ACTUAL_API_KEY to it. Fix: import and call the same open_credentialed_url() the base class uses, keeping Actual's own URL-resolution logic unchanged. Adds an end-to-end regression test using two real local HTTP servers (no mocking of the security module itself) — one redirects, the other records the Authorization header it receives — mirroring test_urllib_security.py's own redirect tests. Also repoints the existing fetch_models test's mock from urllib.request.urlopen to hermes_cli.urllib_security.open_credentialed_url, since fetch_models no longer calls the former. Mutation-verified: the new redirect test fails on pre-fix code with the Authorization header observed at the redirect target.
alt-glitch
added
type/bug
This was referenced Aug 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Actual model catalog discovery works again on macOS: the plugin's
/v1/modelsfetch now routes through Hermes' credential-safe urllib opener, and Hermes-owned urllib openers get an explicit CA context (env bundles first, certifi fallback on macOS only) so Homebrew-Python cert stores no longer kill the request before it's sent.Root cause:
ActualProfile.fetch_modelscalled rawurllib.request.urlopen— bypassing the cross-origin credential-stripping guard entirely — and urllib's default trust store on affected macOS environments failed verification pre-send, making an authenticated catalog look like a missing-key failure (Desktop showed zero models).Salvaged from #83554 by @somewheresy — commits preserved for both authors (@pierrenode's redirect-guard routing + Justin Bennington's certifi CA context). Incorporates the #80247 author commit.
Changes
plugins/model-providers/actual/__init__.py:fetch_modelsusesopen_credentialed_urlinstead of raw urlopenhermes_cli/urllib_security.py:_resolved_https_context()— HERMES_CA_BUNDLE/SSL_CERT_FILE/REQUESTS_CA_BUNDLE/CURL_CA_BUNDLE precedence, macOS-only certifi fallback, never disables verification; only applies when Hermes builds the opener (installed app openers untouched)tests/: real two-server redirect e2e proving Bearer never crosses origins, CA selection, no-credential no-probe behaviorValidation
E2E (real imports): env-CA precedence honored, bad bundle path warns and falls through, darwin gets certifi context, linux default unchanged. Author verified live on macOS: 3 models returned + full AIAgent round-trip through provider
actual.Note: overlaps
_secure_opener_from_installed_policywith #86491 (per-request ssl_context seam) — semantically compatible; whichever merges second gets a trivial rebase.Infographic