Skip to content

fix: prefer IPv4 for pinned SSRF lookups#80162

Merged
scoootscooob merged 1 commit into
openclaw:mainfrom
scoootscooob:codex/80078-ssrf-ipv6-pinning
May 10, 2026
Merged

fix: prefer IPv4 for pinned SSRF lookups#80162
scoootscooob merged 1 commit into
openclaw:mainfrom
scoootscooob:codex/80078-ssrf-ipv6-pinning

Conversation

@scoootscooob

@scoootscooob scoootscooob commented May 10, 2026

Copy link
Copy Markdown
Member

Summary

Real behavior proof

  • Behavior or issue addressed: Automatic pinned lookups for dual-stack hostnames should continue using IPv4 in an IPv4-working setup, while explicit IPv6 lookups still work.
  • Real environment tested: macOS local OpenClaw worktree, Node v25.2.1, pnpm 10.33.2, real local IPv4-only HTTP server bound to 127.0.0.1 and OpenClaw's production pinned lookup helper loaded through tsx.
  • Exact steps or command run after this patch: Ran pnpm exec tsx -e '<script>' that starts a real IPv4-only HTTP server, creates a pinned dual-stack lookup for dualstack.openclaw.test with 127.0.0.1 plus 2607:6bc0::10, performs two real HTTP requests through the default lookup path, then asks for explicit family 6.
  • Evidence after fix: Terminal output from the after-fix live socket smoke:
{
  "first": "200:ok",
  "second": "200:ok",
  "explicitV6": {
    "address": "2607:6bc0::10",
    "family": 6
  }
}
  • Observed result after fix: Both default HTTP requests reached the IPv4-only server successfully, proving the default pinned lookup did not rotate onto the unreachable IPv6 record; the explicit IPv6 lookup still returned the IPv6 address with family 6.
  • What was not tested: No additional gaps.

Verification

  • OPENCLAW_VITEST_FS_MODULE_CACHE_PATH=/tmp/openclaw-vitest-cache-80078 pnpm test src/infra/net/ssrf.pinning.test.ts src/infra/net/ssrf.dispatcher.test.ts src/infra/unhandled-rejections.test.ts src/agents/provider-transport-fetch.test.ts
  • pnpm check:changed
  • pnpm exec oxfmt --check --threads=1 src/infra/net/ssrf.ts src/infra/net/ssrf.pinning.test.ts src/infra/unhandled-rejections.ts src/infra/unhandled-rejections.test.ts CHANGELOG.md
  • git diff --check

@openclaw-barnacle openclaw-barnacle Bot added size: S maintainer Maintainer-authored PR labels May 10, 2026
@clawsweeper

clawsweeper Bot commented May 10, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge.

Summary
The branch changes SSRF pinned lookup selection to prefer IPv4 for automatic dual-stack lookups, treats EADDRNOTAVAIL as transient/benign, and adds regression tests plus changelog text.

Reproducibility: yes. source-level reproduction is high confidence: the linked report gives concrete WSL2/no-IPv6-route steps, current main can rotate automatic pinned lookups from IPv4 to IPv6, and EADDRNOTAVAIL is absent from the relevant classifiers. I did not run the full WSL2 Gateway crash scenario in this read-only pass.

Real behavior proof
Sufficient (terminal): The PR body includes copied after-fix terminal output from a real local IPv4-only socket smoke using OpenClaw's production pinned lookup helper, showing default requests succeed while explicit IPv6 lookup still returns IPv6.

Next step before merge
The PR has a protected maintainer label and changes network/SSRF lookup policy, so the remaining action is maintainer review and landing judgment rather than automated repair.

Security
Cleared: The diff stays within existing SSRF lookup selection, network-error classification, tests, and changelog, and it does not weaken DNS-answer validation or add supply-chain risk.

Review details

Best possible solution:

Merge the focused network fix after maintainer review if the IPv4-preferred automatic lookup policy is accepted, keeping DNS-answer validation before candidate selection and the new regression coverage in place.

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

Yes, source-level reproduction is high confidence: the linked report gives concrete WSL2/no-IPv6-route steps, current main can rotate automatic pinned lookups from IPv4 to IPv6, and EADDRNOTAVAIL is absent from the relevant classifiers. I did not run the full WSL2 Gateway crash scenario in this read-only pass.

Is this the best way to solve the issue?

Yes, from source review this is the narrowest maintainable fix for the reported dual-stack trigger: keep automatic pinned lookups on IPv4 when IPv4 exists, preserve explicit IPv6 lookup behavior, and classify the observed network error like adjacent unreachable-network errors. The remaining question is maintainer policy acceptance, not a repairable code defect.

Acceptance criteria:

  • Contributor reported: OPENCLAW_VITEST_FS_MODULE_CACHE_PATH=/tmp/openclaw-vitest-cache-80078 pnpm test src/infra/net/ssrf.pinning.test.ts src/infra/net/ssrf.dispatcher.test.ts src/infra/unhandled-rejections.test.ts src/agents/provider-transport-fetch.test.ts
  • Contributor reported: pnpm check:changed
  • Contributor reported: pnpm exec oxfmt --check --threads=1 src/infra/net/ssrf.ts src/infra/net/ssrf.pinning.test.ts src/infra/unhandled-rejections.ts src/infra/unhandled-rejections.test.ts CHANGELOG.md
  • Contributor reported: git diff --check

What I checked:

  • Protected open PR state: Live PR metadata shows this PR is open, unmerged, based on main 09132effa82a463beb75839040f3a083f535471a, at head 89b3fac6cecc5f7d4dd650f9d915957580c68b75, and has the protected maintainer label plus proof: sufficient. (89b3fac6cecc)
  • Current main can rotate automatic pinned lookups onto IPv6: Current main builds pinned records from all resolved addresses, uses all records when no explicit family is requested, and advances a shared round-robin index, so a dual-stack host can return IPv4 first and IPv6 on a later automatic lookup. (src/infra/net/ssrf.ts:344, 09132effa82a)
  • Current main omits EADDRNOTAVAIL from transient and benign handling: ENETUNREACH is classified in the transient and benign network sets/regexes on current main, while EADDRNOTAVAIL is absent before this PR. (src/infra/unhandled-rejections.ts:51, 09132effa82a)
  • The affected runtime path uses pinned dispatchers after DNS policy validation: Guarded fetch resolves and policy-checks the target hostname before constructing createPinnedDispatcher, so the PR changes the production guarded transport path while keeping preselection DNS-answer validation in place. (src/infra/net/fetch-guard.ts:441, 09132effa82a)
  • Node lookup contract reaches the automatic all-record path: A local Node v24.14.1 probe showed net.connect({ autoSelectFamily: true }) calls a custom lookup with { "hints": 32, "all": true }, matching the pinned lookup path changed by the PR.
  • PR keeps automatic candidates on IPv4 while preserving explicit IPv6: At the PR head, createPinnedLookup builds automaticRecords from IPv4 records when any exist, uses them for automatic lookups, and still filters the original records for explicit family 4 or 6 requests. (src/infra/net/ssrf.ts:348, 89b3fac6cecc)

Likely related people:

  • steipete: Commit history shows repeated recent work on pinned SSRF dispatchers, guarded fetch family fallback, SSRF security hardening, and transient unhandled network error classification. (role: recent network/SSRF area contributor; confidence: high; commits: e9ed688c2c06, ecec68d06d19, 824d1e095b5e; files: src/infra/net/ssrf.ts, src/infra/net/ssrf.dispatcher.test.ts, src/infra/unhandled-rejections.ts)
  • obviyus: File history includes recent guarded fetch cleanup and prior SSRF/Telegram transport work, including global family policy handling for pinned dispatchers. (role: recent guarded-fetch and pinned-dispatcher contributor; confidence: medium; commits: 2465217b238a, e4825a0f9385, 30fd2bbe195a; files: src/infra/net/ssrf.ts, src/infra/net/ssrf.dispatcher.test.ts)
  • Glucksberg: History for the pinning test surface includes the earlier fix that sorted IPv4 before IPv6 in SSRF pinned DNS for IPv6-broken hosts, which is directly adjacent to this PR's policy change. (role: IPv4-before-IPv6 SSRF ordering contributor; confidence: medium; commits: dd9ba974d035; files: src/infra/net/ssrf.pinning.test.ts, src/infra/net/ssrf.ts)
  • HemantSudarshan: History for src/infra/unhandled-rejections.ts includes recent work keeping unreachable polling sockets non-fatal, adjacent to this PR's EADDRNOTAVAIL classifier change. (role: recent transient network error contributor; confidence: medium; commits: db6951088a19; files: src/infra/unhandled-rejections.ts)

Remaining risk / open question:

  • The WSL2 gateway crash path from the linked report was not live-reproduced in this read-only review; the reproduction confidence is source-level plus reporter logs and PR terminal smoke proof.
  • The intentional policy tradeoff is that automatic pinned lookups no longer attempt IPv6 when an A record exists, so a maintainer should accept that behavior before landing.
  • The linked report's broader TLSSocket uncaught-error concern may still matter for explicit IPv6 or IPv6-only unreachable paths; this PR targets the dual-stack IPv4-working crash trigger.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 09132effa82a.

Re-review progress:

@scoootscooob
scoootscooob marked this pull request as ready for review May 10, 2026 07:42
@clawsweeper clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 10, 2026
@scoootscooob
scoootscooob force-pushed the codex/80078-ssrf-ipv6-pinning branch 2 times, most recently from 14d6e93 to 8080627 Compare May 10, 2026 08:04
@scoootscooob
scoootscooob force-pushed the codex/80078-ssrf-ipv6-pinning branch from 8080627 to 89b3fac Compare May 10, 2026 08:09
@scoootscooob
scoootscooob merged commit 1382a5e into openclaw:main May 10, 2026
110 checks passed
longstoryscott pushed a commit to longstoryscott/openclaw that referenced this pull request May 13, 2026
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request May 24, 2026
jameslcowan pushed a commit to jameslcowan/openclaw that referenced this pull request Jun 2, 2026
sablehead pushed a commit to sablehead/openclaw that referenced this pull request Jun 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintainer Maintainer-authored PR proof: sufficient ClawSweeper judged the real behavior proof convincing. size: S

Projects

None yet

1 participant