[security] fix(providers): reject sibling HTTP endpoint overrides#1256
[security] fix(providers): reject sibling HTTP endpoint overrides#1256Hinotoi-agent wants to merge 4 commits into
Conversation
|
Codex review: needs maintainer review before merge. Reviewed May 31, 2026, 11:47 PM ET / 03:47 UTC. Summary Reproducibility: yes. source inspection gives a high-confidence reproduction path: current main accepts explicit Review metrics: 3 noteworthy metrics.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land the HTTPS-only override hardening after maintainer acceptance of the compatibility tradeoff and required CI or focused provider tests pass. Do we have a high-confidence way to reproduce the issue? Yes, source inspection gives a high-confidence reproduction path: current main accepts explicit Is this the best way to solve the issue? Yes, with maintainer acceptance: validating endpoint overrides before request construction is the narrow security fix and preserves HTTPS plus bare-host normalization. The remaining question is whether the fail-closed behavior for explicit plaintext overrides is an acceptable compatibility tradeoff. AGENTS.md: found and applied where relevant. Codex review notes: model gpt-5.5, reasoning high; reviewed against a6a538efc489. Label changesLabel justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
|
@clawsweeper re-review please — updated the branch at I also updated the PR body with copied redacted after-fix output, including:
The local |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
f13d1e9 to
0b0ea3d
Compare
|
Rebased onto landed #1269 and removed the duplicate stacked validator commits. Exact head Additional fixes:
Verification:
|
Summary
This PR hardens sibling provider API endpoint overrides for credentialed usage calls.
Security issues covered
Before this PR
OPENROUTER_API_URL,CODEBUFF_API_URL,GROQ_API_URL, andELEVENLABS_API_URLcould be set to explicithttp://...URLs.After this PR
Why this matters
These API URL settings are provider-owned credentialed endpoints. Once a token/API key is present, allowing an explicit HTTP override creates an unnecessary downgrade path for authenticated traffic. Rejecting non-HTTPS overrides keeps the secure default while preserving a safe HTTPS override mechanism for trusted local testing or alternate provider-compatible endpoints.
How this differs from related issue/PR
This is a sibling-provider follow-up to #1236. That PR covers Alibaba and MiniMax endpoint overrides. This PR applies the same boundary to OpenRouter, Codebuff, Groq, and ElevenLabs, whose settings readers and usage fetchers are separate code paths.
Attack flow
Root cause
Changes in this PR
validateEndpointOverrides(environment:)guards to OpenRouter, Codebuff, Groq, and ElevenLabs credentialed usage fetchers.ProviderEndpointOverrideSecurityTestscovering accepted HTTPS, bare-host normalization, rejected HTTP fallback, and thrown validation errors.Maintainer impact
Type of change
Test plan
swift build --target CodexBarCoreswiftformatlint mode viamake checkreported0/998 files require formattingswift test --filter ProviderEndpointOverrideSecurityTests— blocked locally by existingKeyboardShortcuts#Previewmacro/plugin build failure before these tests run:external macro implementation type 'PreviewsMacros.SwiftUIView' could not be found for macro 'Preview(_:body:)'make check— blocked locally after SwiftFormat passes because the wrapper SwiftLint invocation traps while loading SourceKit:Fatal error: Loading sourcekitdInProc.framework/Versions/A/sourcekitdInProc failedExecuted with redacted/fake endpoint values only; no provider services or real credentials were used.
The smoke script asserts:
localhost:8080andlocalhost:8080/v1are treated as bare host/path overrides and normalize to HTTPS.http://attacker.test...remains rejected by the validation path for OpenRouter, Codebuff, Groq, and ElevenLabs.Disclosure notes