fix(providers): allow plain-HTTP custom endpoints blocked by ATS (#499)#505
Open
devzahirul wants to merge 2 commits into
Open
fix(providers): allow plain-HTTP custom endpoints blocked by ATS (#499)#505devzahirul wants to merge 2 commits into
devzahirul wants to merge 2 commits into
Conversation
…ic-dev#499) Custom providers pointed at plain-HTTP endpoints (local LLM proxies such as cliproxyapi/new-api on LAN hosts) failed every request with NSURLError -1022 because the app had no App Transport Security configuration, so macOS blocked all cleartext connections beyond loopback. Since custom provider hosts are user-configured at runtime, they cannot be enumerated as NSExceptionDomains, and NSAllowsLocalNetworking does not cover multi-label LAN hostnames or remote proxies. Info.plist now sets NSAllowsArbitraryLoads; all bundled endpoints remain HTTPS. To keep the choice informed, both custom provider forms show a plaintext-traffic warning for HTTP endpoints beyond loopback, classified by the new ModelRepository.isInsecureRemoteEndpoint helper.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a4a3c4b23a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
… form too The editProviderSection shown after a custom provider is verified has its own base URL field, which was missing the insecure-endpoint warning added to the details and add forms. All three base URL entry points now share the same warning row.
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
1 participant
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
Fixes #499. Custom providers pointed at a plain-HTTP endpoint — e.g. the reporter's
http://mars-main.box:8317/v1, or local LLM proxies like cliproxyapi/new-api that have no TLS cert — fail every request (model refresh, verification, enhancement) with:Root cause
The app bundle had no
NSAppTransportSecurityconfiguration, so macOS applies the default ATS policy: cleartext HTTP is blocked at the CFNetwork layer for every host except loopback. The provider form itself acceptshttp://URLs (saveNewProvideronly checks for non-empty), so users could configure an endpoint that could never work, and the failure only surfaced later as error -1022.Why
NSAllowsArbitraryLoadsNSExceptionDomainsrequires the hosts to be known at build time; custom provider hosts are user-configured at runtime.NSAllowsLocalNetworkingonly exempts local-network names (single-label,.local); it does not cover multi-label private DNS names like the reporter'smars-main.box, nor self-hosted proxies reached across networks.So a global allowance is the only mechanism that actually covers the reported setups. Every endpoint the app itself ships with (model downloads, updater, analytics) remains HTTPS — this change only permits endpoints the user explicitly configures.
Keeping the choice informed
Both custom provider forms (add and edit) now show a warning under the Base URL field when it is plain HTTP to a non-loopback host:
Classification lives in
ModelRepository.isInsecureRemoteEndpoint: loopback (localhost/127.x/::1) is exempt since that traffic never leaves the machine; other LAN hosts are not, since that traffic is observable on the network.Out of scope
HTTPS endpoints with self-signed/invalid certificates still fail TLS validation — that is certificate trust, not ATS, and would need a separate opt-in trust override if wanted.
Changes
Info.plist—NSAppTransportSecurity→NSAllowsArbitraryLoads, with a comment documenting the rationale.ModelRepository—isInsecureRemoteEndpoint(_:)helper next to the existingisLocalEndpoint.AISettingsView+AIConfiguration— shared warning row under the Base URL field in the add and edit provider forms.InsecureEndpointTestscovering remote-HTTP flagging (including the reporter's URL shape), loopback exemption, HTTPS, and invalid input.Testing
swiftlint --strict— 0 violations.xcodebuild testsuite — 57/57 passing.Info.plistcontains the ATS key (plutil -p).Closes #499