fix(codex): guard loopback WS classification with isIP to prevent DNS hostname bypass by lsr911 · Pull Request #110692 · openclaw/openclaw

lsr911 · 2026-07-18T12:31:05Z

Problem

The remote-auth boundary in assertCodexAppServerConnectionSecurity uses isLoopbackWebSocketUrl to classify WS transports as local-loopback (auth-free) vs remote (requires authToken/Authorization). Before this fix, host.startsWith("127.") matched DNS hostnames like ws://127.evil.com/app, letting a remote WebSocket endpoint skip authentication entirely.

Fix

Add isIP(host) === 4 guard so only literal IPv4 loopback addresses qualify. Same canonical pattern as extensions/slack/src/monitor/relay-source.ts:271.

Host	Before (broken)	After (fixed)
`ws://127.0.0.1:3333/app`	loopback (auth-free) ✅	loopback (auth-free) ✅
`ws://127.255.255.254/codex`	loopback ✅	loopback ✅
`wss://127.evil.com/app`	loopback 🔴	remote (needs auth) ✅
`wss://127.example.com/codex`	loopback 🔴	remote (needs auth) ✅

Evidence

Focused regression tests (6/6):

$ node scripts/run-vitest.mjs extensions/codex/src/app-server/loopback-ws-auth.test.ts
  Tests  6 passed — rejects DNS hostnames, preserves literal 127/8, localhost, ::1, non-WS transports, and valid remote auth

oxfmt, oxlint, tsgo extensions prod+test types all pass.

Real behavior proof — production module, no mocks:

== codex WS remote-auth boundary (production module) ==
literal 127.0.0.1 (IPv4 loopback)                -> ALLOWED (loopback)  ✅
literal 127.255.255.254 (127/8)                  -> ALLOWED (loopback)  ✅
::1 (IPv6 loopback)                              -> ALLOWED (loopback)  ✅
localhost                                        -> ALLOWED (loopback)  ✅
DNS 127.evil.com (BYPASS CANDIDATE)              -> DENIED  (remote)   ✅
DNS 127.example.com (BYPASS CANDIDATE)           -> DENIED  (remote)   ✅
DNS ws.127.com (BYPASS CANDIDATE)                -> DENIED  (remote)   ✅

… hostname bypass The remote-auth boundary in assertCodexAppServerConnectionSecurity uses isLoopbackWebSocketUrl to classify WS transports as local-loopback (auth-free) vs remote (requires authToken/Authorization). Before this fix, host.startsWith("127.") matched DNS hostnames like 127.evil.com, letting a remote WebSocket endpoint skip authentication entirely. Add isIP(host)===4 guard so only literal IPv4 loopback addresses qualify. Same canonical pattern as extensions/slack/src/monitor/relay-source.ts:271. Co-Authored-By: Claude <[email protected]>

clawsweeper · 2026-07-18T12:38:42Z

Codex review: needs maintainer review before merge. Reviewed July 18, 2026, 8:35 AM ET / 12:35 UTC.

Summary
The branch changes Codex app-server WebSocket loopback detection so only literal IPv4 127/8 hosts bypass remote-auth requirements, and adds regression tests for hostname lookalikes.

PR surface: Source +4, Tests +110. Total +114 across 2 files.

Reproducibility: yes. from source, with medium confidence: current-main host.startsWith("127.") classifies a hostname such as ws://127.evil.com/app as loopback before the proposed isIP guard. The inline production-module proof reports that the changed branch denies those hosts.

Review metrics: 1 noteworthy metric.

Authentication classification: 1 changed remote-auth exemption. The change converts DNS names beginning with 127. from auth-free to remote-auth-required, which can affect an existing endpoint configuration.

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🐚 platinum hermit
Patch quality: 🦐 gold shrimp
Result: needs maintainer review before merge.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

Inspect the sibling Codex app-server source for its endpoint and authentication expectations before approving the auth-boundary change.

Risk before merge

[P1] Existing configurations that point at a DNS hostname beginning with 127. and omit remote credentials will begin failing closed after merge; that is the intended security correction, but it should be explicitly accepted with the Codex app-server contract check.

Maintainer options:

Verify the upstream endpoint contract before merge (recommended)
Confirm directly in the sibling Codex source that non-literal WebSocket hosts are not a supported auth-free local transport, then merge this fail-closed correction.
Retain the current permissive classification
Accept the hostname lookalike behavior only if maintainers intentionally support unauthenticated remote endpoints matching 127.*, with that exception documented as a security policy.

Next step before merge

[P2] The only remaining merge blocker is the repository-required direct upstream Codex contract verification; it is not a safe automated repair task.

Security
Cleared: The diff uses a Node built-in parser and narrows, rather than expands, the unauthenticated WebSocket exemption; no diff-introduced supply-chain or security regression is visible.

Review details

Best possible solution:

Verify the exact sibling Codex app-server endpoint and authentication contract, then land the literal-IP-only classification with the focused regression tests if remote non-literal WebSocket URLs are required to authenticate.

Do we have a high-confidence way to reproduce the issue?

Yes from source, with medium confidence: current-main host.startsWith("127.") classifies a hostname such as ws://127.evil.com/app as loopback before the proposed isIP guard. The inline production-module proof reports that the changed branch denies those hosts.

Is this the best way to solve the issue?

Likely yes: using Node's built-in IP parser at the existing classifier is the narrowest way to distinguish literal IPv4 loopback from DNS names. A final best-fix judgment requires the repository-mandated direct sibling Codex source check.

AGENTS.md: unclear because the file could not be read completely.

Codex review notes: model internal, reasoning high; reviewed against af2662f25a85.

Label changes

Label changes:

add P1: This PR addresses a plausible remote authentication-boundary bypass in the Codex WebSocket path.
add merge-risk: 🚨 compatibility: The intentional fail-closed change can require credentials for an existing endpoint configured with a 127.* DNS hostname.
add proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix production-module terminal output that directly shows hostname lookalikes denied and legitimate loopback variants allowed; redact any private endpoint details if refreshed.
add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🐚 platinum hermit and patch quality is 🦐 gold shrimp.
add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes after-fix production-module terminal output that directly shows hostname lookalikes denied and legitimate loopback variants allowed; redact any private endpoint details if refreshed.

Label justifications:

P1: This PR addresses a plausible remote authentication-boundary bypass in the Codex WebSocket path.
merge-risk: 🚨 compatibility: The intentional fail-closed change can require credentials for an existing endpoint configured with a 127.* DNS hostname.
rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🐚 platinum hermit and patch quality is 🦐 gold shrimp.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes after-fix production-module terminal output that directly shows hostname lookalikes denied and legitimate loopback variants allowed; redact any private endpoint details if refreshed.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix production-module terminal output that directly shows hostname lookalikes denied and legitimate loopback variants allowed; redact any private endpoint details if refreshed.

Evidence reviewed

PR surface:

Source +4, Tests +110. Total +114 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	5	1	+4
Tests	1	110	0	+110
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	115	1	+114

What I checked:

Changed authentication classifier: The branch imports Node's built-in isIP and changes the 127. fallback so it applies only when the parsed host is an IPv4 literal; this rejects DNS names such as 127.evil.com while retaining literal 127/8 behavior. (extensions/codex/src/app-server/config.ts:1345, cab3336eda41)
Focused regression coverage: The proposed test exercises the canonical consumer, assertCodexAppServerConnectionSecurity, for DNS hostname lookalikes, literal IPv4 loopback, localhost, IPv6 loopback, and authenticated remote WebSocket behavior. (extensions/codex/src/app-server/loopback-ws-auth.test.ts:1, cab3336eda41)
Contributor-supplied live proof: The PR body includes production-module terminal output showing 127.evil.com, 127.example.com, and ws.127.com denied without remote credentials while literal loopback hosts remain allowed. (cab3336eda41)
Upstream transport contract: Codex documents WebSocket app-server transport as ws://IP:PORT and experimental, supporting a literal-address interpretation of its listener endpoint; a direct sibling ../codex source inspection remains required by repository policy before a Codex merge verdict. (codex-rs/app-server/README.md:264)

Likely related people:

unknown: A feature-history and blame pass was required, but the read-only shell could not execute repository inspection commands; the PR author is intentionally not listed solely for proposing this branch. (role: current-main area owner unavailable; confidence: low; files: extensions/codex/src/app-server/config.ts)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

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 keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

lsr911 mentioned this pull request Jul 18, 2026

fix(litellm): guard loopback hostname auto-allow with isIP to prevent DNS SSRF bypass #110693

Merged

openclaw-barnacle Bot added extensions: codex size: S triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. labels Jul 18, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(codex): guard loopback WS classification with isIP to prevent DNS hostname bypass#110692

fix(codex): guard loopback WS classification with isIP to prevent DNS hostname bypass#110692
lsr911 wants to merge 1 commit into
openclaw:mainfrom
lsr911:fix/codex-loopback-ws-ip-guard

lsr911 commented Jul 18, 2026

Uh oh!

clawsweeper Bot commented Jul 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

lsr911 commented Jul 18, 2026

Problem

Fix

Evidence

Uh oh!

clawsweeper Bot commented Jul 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant