Skip to content

fix: guarded fetch falls back to IPv6 when IPv4 is unreachable#110124

Draft
an2in wants to merge 1 commit into
openclaw:mainfrom
an2in:agent/ssrf-dual-stack-fallback
Draft

fix: guarded fetch falls back to IPv6 when IPv4 is unreachable#110124
an2in wants to merge 1 commit into
openclaw:mainfrom
an2in:agent/ssrf-dual-stack-fallback

Conversation

@an2in

@an2in an2in commented Jul 17, 2026

Copy link
Copy Markdown

Closes #87763

What Problem This Solves

Fixes an issue where guarded provider requests on dual-stack hosts can fail when IPv4 is unreachable even though IPv6 is available.

Why This Change Was Made

Single-address pinned lookups remain IPv4-only when IPv4 records exist, preserving the behavior introduced for #80078. All-record lookups used by Node/Undici Happy Eyeballs now receive the complete SSRF-validated, IPv4-first pinned record set, so IPv6 remains available as a fallback. Explicit-family and hostname-fallback behavior are unchanged.

User Impact

Users on networks where a provider's IPv4 route is unavailable but IPv6 works can complete guarded provider requests without disabling DNS pinning or weakening resolved-address checks.

Evidence

  • Updated src/infra/net/ssrf.pinning.test.ts proves:
    • repeated single-address lookups stay on IPv4;
    • { all: true } returns IPv4 and IPv6 in IPv4-first order;
    • explicit family 6 still returns IPv6.
  • git diff --check passes.
  • A source-aware Codex review found no actionable regression.
  • Secretless fork CI completed successfully at exact head fdea545196ae73695d1fc6fe54c1252af8c4374b: 125 jobs passed and 6 scope-inapplicable jobs skipped. The network/security test shard, build artifacts, lint, production types, and openclaw/ci-gate all passed.

Exact-head dual-stack runtime proof

The following redacted NODE_DEBUG=net excerpts came from disposable Node 24 containers on an Ubuntu 24.04.4 VM. Both containers checked out exact head fdea545196ae73695d1fc6fe54c1252af8c4374b and invoked fetchWithSsrFGuard from that checkout. No API key or authorization header was supplied; the containers received only PROOF_SHA and NODE_DEBUG as environment variables.

IPv4 unreachable, IPv6 succeeds (https://openrouter.ai/api/v1/models, host networking):

SCENARIO=ipv4-unreachable_ipv6-success
PROOF_SHA=fdea545196ae73695d1fc6fe54c1252af8c4374b
NODE=v24.18.0
DNS_IPV4_COUNT=2
DNS_IPV6_COUNT=2
PINNED_FAMILY_ORDER=4,4,6,6
FORCED_IPV4=error:ETIMEDOUT
FORCED_IPV6=status:200
NET: connect/multiple: attempting to connect to <IPv4>:443 (addressType: 4)
NET: connect/multiple: connection to <IPv4>:443 timed out
NET: connect/multiple: attempting to connect to <IPv6>:443 (addressType: 6)
NET: connect/multiple: connection attempt to <IPv6>:443 completed with status 0
GUARDED_FETCH_STATUS=200
GUARDED_FETCH_OK=true
ELAPSED_MS=584

IPv6 unreachable, IPv4 succeeds (https://www.google.com/generate_204, IPv4-only Docker bridge):

SCENARIO=ipv6-unreachable_ipv4-success
PROOF_SHA=fdea545196ae73695d1fc6fe54c1252af8c4374b
NODE=v24.18.0
DNS_IPV4_COUNT=1
DNS_IPV6_COUNT=1
PINNED_FAMILY_ORDER=4,6
FORCED_IPV4=status:204
FORCED_IPV6=error:ENETUNREACH
NET: connect/multiple: attempting to connect to <IPv4>:443 (addressType: 4)
NET: connect/multiple: connection attempt to <IPv4>:443 completed with status 0
GUARDED_FETCH_STATUS=204
GUARDED_FETCH_OK=true
ELAPSED_MS=334

The original shipped-version observation is consistent with the exact-head proof: on Ubuntu 24.04.4 LTS with Node v24.15.0 and OpenClaw 2026.5.26, direct IPv6 worked while guarded IPv4 timed out; retaining the validated IPv6 candidate restored the provider request.

AI-assisted: Codex helped diagnose, implement, and review this change. I reviewed the code and understand the behavior. The original diagnostic transcript is not attached because it contained credentials; the exact-head proof above was rerun without credentials and is sanitized.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. labels Jul 17, 2026
@clawsweeper

clawsweeper Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 17, 2026, 3:26 PM ET / 19:26 UTC.

Summary
This PR changes pinned DNS lookups so Node’s all-address Happy Eyeballs lookup receives both validated IPv4 and IPv6 records while ordinary single-address lookups remain IPv4-first.

PR surface: Source +4, Tests +3. Total +7 across 2 files.

Reproducibility: yes. at source level: Node requests all addresses for autoSelectFamily, and the PR’s exact-head terminal evidence shows an unreachable IPv4 connection falling through to validated IPv6. The original provider-specific environment was not independently rerun in this read-only review.

Review metrics: none identified.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #87763
Summary: This PR is an evidence-backed candidate fix for the linked open guarded-fetch Happy-Eyeballs failure report.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

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

Rank-up moves:

  • Rebase onto current main and obtain explicit networking/security-owner acceptance of the SSRF-validated dual-stack policy.

Risk before merge

  • [P1] Merging restores IPv6 connection attempts in an SSRF-guarded outbound path; although the patch uses the existing validated record set, an owner should explicitly accept that dual-stack policy and refresh the review against the current main base before merge.

Maintainer options:

  1. Confirm the guarded dual-stack policy (recommended)
    Rebase onto current main, then obtain explicit networking/security-owner acceptance that all SSRF-validated records may be offered to Node’s Happy Eyeballs path.
  2. Retain IPv4-only automatic lookup
    Pause this patch if maintainers do not want guarded fetch to attempt validated IPv6 addresses automatically.

Next step before merge

  • [P2] No mechanical repair remains; a networking/security owner must decide whether exposing all already validated address families to Node Happy Eyeballs is the intended SSRF-guarded connection policy.

Maintainer decision needed

  • Question: Should guarded fetch permit Node Happy Eyeballs to attempt all already SSRF-validated IPv4 and IPv6 records when it requests lookup(..., { all: true })?
  • Rationale: The implementation is narrow and evidence-backed, but it deliberately changes the outbound connection family policy of an SSRF-guarded path; that policy boundary needs explicit owner acceptance.
  • Likely owner: georgenaz — Their canonical report contains the concrete provider-routing failure mode that this policy decision resolves, though a networking/security code owner should make the final decision.
  • Options:
    • Accept validated dual-stack fallback (recommended): Approve the patch after rebasing and confirming that validated IPv6 candidates are intended to be usable only through Node’s all-address Happy Eyeballs path.
    • Keep IPv4-only automatic resolution: Retain the current behavior and pursue a different owner-approved mitigation for affected dual-stack routes.

Security
Cleared: The two-file diff adds no dependency, workflow, permission, or new destination source; it exposes only records already validated by the existing SSRF resolution path.

Review details

Best possible solution:

Accept the smallest dual-stack policy that returns only the already SSRF-validated address set for all: true, retain IPv4-first single-address behavior, and land only after the networking/security owner confirms that this is the intended guarded-fetch contract.

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

Yes at source level: Node requests all addresses for autoSelectFamily, and the PR’s exact-head terminal evidence shows an unreachable IPv4 connection falling through to validated IPv6. The original provider-specific environment was not independently rerun in this read-only review.

Is this the best way to solve the issue?

Yes, conditionally: returning the existing validated record set only for the dependency’s all: true contract is narrower than a second fallback dispatcher or a global DNS/config workaround, but the guarded-path IPv6 policy still needs owner approval.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P1: The linked report describes provider requests timing out and leaving affected gateway workflows unable to obtain model responses.
  • merge-risk: 🚨 availability: The patch changes which network families guarded fetch can attempt during connection fallback.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body provides sanitized exact-head terminal output from real Node 24 dual-stack requests in both asymmetric route directions, with successful guarded-fetch results after the relevant fallback.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides sanitized exact-head terminal output from real Node 24 dual-stack requests in both asymmetric route directions, with successful guarded-fetch results after the relevant fallback.
Evidence reviewed

PR surface:

Source +4, Tests +3. Total +7 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 5 1 +4
Tests 1 5 2 +3
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 10 3 +7

What I checked:

  • Changed lookup policy: The patch changes opts.all lookups to return the complete validated records list, while retaining automaticRecords for ordinary lookups and family-specific filtering. (src/infra/net/ssrf.ts:524, fdea545196ae)
  • Regression coverage: The updated unit expectation distinguishes ordinary IPv4-first lookup behavior, all-address results containing both families, and explicit IPv6 lookup behavior. (src/infra/net/ssrf.pinning.test.ts:54, fdea545196ae)
  • Node contract: Node’s autoSelectFamily calls custom lookup with all: true and attempts returned IPv4/IPv6 addresses until one connects; returning address objects for the all-address case is the applicable dependency contract.
  • After-fix runtime proof: The PR body records exact-head Node 24 runs showing a guarded request succeeds after an unreachable IPv4 attempt falls through to IPv6, while the reverse network condition continues to succeed over IPv4. (fdea545196ae)
  • Canonical report: The linked open report identifies this same custom-lookup/Happy-Eyeballs interaction as a P1 provider-connectivity problem and is explicitly named as the PR’s target.

Likely related people:

  • georgenaz: Authored the linked open report that documents the affected guarded-fetch and provider-routing failure mode. (role: canonical-report author; confidence: medium)
  • takamasa-aiso: Reported the earlier dual-stack pinned-lookup regression whose IPv4-first behavior this PR deliberately preserves for single-address callers. (role: adjacent behavior reporter; confidence: low)
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.
Review history (2 earlier review cycles)
  • reviewed 2026-07-17T18:10:08.539Z sha fdea545 :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-17T18:44:13.280Z sha fdea545 :: needs maintainer review before merge. :: none

@an2in

an2in commented Jul 17, 2026

Copy link
Copy Markdown
Author

@clawsweeper re-review

Exact-head dual-stack runtime proof for both asymmetric route directions and the successful secretless full-CI run have been added to the PR body. The PR remains draft for maintainer/security policy review.

@clawsweeper

clawsweeper Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jul 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. P1 High-priority user-facing bug, regression, or broken workflow. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: XS status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SSRF guard pinned DNS dispatcher causes model fetch timeouts when autoSelectFamily is enabled

1 participant