[security] fix(providers): validate sibling endpoint overrides#1269
[security] fix(providers): validate sibling endpoint overrides#1269Hinotoi-agent wants to merge 9 commits into
Conversation
|
Codex review: needs changes before merge. Reviewed June 5, 2026, 12:07 PM ET / 16:07 UTC. Summary Reproducibility: yes. Source inspection of PR head shows validatedHost returns URL.host, which omits an explicit port, while current main returns the cleaned host override directly before URL reconstruction. Review metrics: 2 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:
Copy recommended automerge instructionNext step before merge
Security Review findings
Review detailsBest possible solution: Keep the shared validator, but carry a validated host authority that preserves explicit ports for host-base overrides and make the focused tests reflect default custom-HTTPS compatibility plus optional strict provider-host mode. Do we have a high-confidence way to reproduce the issue? Yes. Source inspection of PR head shows validatedHost returns URL.host, which omits an explicit port, while current main returns the cleaned host override directly before URL reconstruction. Is this the best way to solve the issue? No, not yet. The shared validator is the right repair direction, but it needs a validated host-authority path that preserves ports and a corrected MiniMax test expectation before this is the narrowest safe solution. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model gpt-5.5, reasoning high; reviewed against 65e39f4dcb3a. Label changesLabel changes:
Label justifications:
Evidence reviewedSecurity concerns:
Acceptance criteria:
What 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
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9e898bc703
ℹ️ 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".
7ad669a to
8c40a44
Compare
|
Updated for the review blockers:
Maintainers: please explicitly sign off on the provider-owned-only endpoint policy. This intentionally breaks arbitrary proxy/test override domains for these credentialed provider paths; HTTPS provider-owned hosts and bare provider hosts remain supported. @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
8c40a44 to
af676e4
Compare
|
Updated again for the Alibaba diagnostic categorization blocker and pushed What changed:
Validation: Standalone diagnostic proof from a temporary local package depending on this checkout's The direct focused test command is still blocked locally by the existing Maintainers: still requesting explicit sign-off on the provider-owned-only endpoint policy for these credentialed provider overrides. @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
af676e4 to
59b2f45
Compare
|
Addressed the latest ClawSweeper blocker and pushed an amended commit ( What changed:
Validation: Local focused The temporary smoke executable imports the real @clawsweeper could you re-review this update when you get a chance? |
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
Summary
MINIMAX_REQUIRE_PROVIDER_ENDPOINT_OVERRIDES=trueALIBABA_CODING_PLAN_REQUIRE_PROVIDER_ENDPOINT_OVERRIDES=truehttp://overrides, rejecteduser:pass@URLs, encoded-delimiter bypasses, accepted provider-owned HTTPS/bare host overrides, and fail-closed fetcher behavior.Compatibility note
This revision keeps the compatibility path requested in review: custom HTTPS proxy/test domains remain accepted by default for MiniMax and Alibaba Coding Plan endpoint overrides.
The hard security boundary is now scheme/authority hygiene:
https://...custom proxy/test domains are accepted by default.http://...overrides are rejected so cookies/API credentials are not sent in cleartext.Operators that prefer provider-owned hosts only can opt into strict mode with
MINIMAX_REQUIRE_PROVIDER_ENDPOINT_OVERRIDES=trueorALIBABA_CODING_PLAN_REQUIRE_PROVIDER_ENDPOINT_OVERRIDES=true.Validation
git diff --check— passed.swift build --target CodexBarCore— passed:CodexBarCore— passed. The proof exercised the actual MiniMax/Alibaba settings readers and fetcher validators with redacted credentials/cookies:swift test --filter 'MiniMaxProviderTests|AlibabaCodingPlanProviderTests'— still blocked before the focused tests by the existing localKeyboardShortcuts/SwiftUI#Previewmacro toolchain issue (PreviewsMacrosplugin not found) with the active developer directory using CommandLineTools rather than full Xcode.