Skip to content

fix(network): scope fake-IP SSRF policy to provider hosts#76887

Merged
steipete merged 1 commit intomainfrom
codex/fake-ip-scoped-ssrf
May 3, 2026
Merged

fix(network): scope fake-IP SSRF policy to provider hosts#76887
steipete merged 1 commit intomainfrom
codex/fake-ip-scoped-ssrf

Conversation

@steipete
Copy link
Copy Markdown
Contributor

@steipete steipete commented May 3, 2026

No description provided.

@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation agents Agent runtime and tooling size: M maintainer Maintainer-authored PR labels May 3, 2026
@clawsweeper
Copy link
Copy Markdown
Contributor

clawsweeper Bot commented May 3, 2026

Codex review: needs maintainer review before merge.

Summary
The PR adds hostname-scoped fake-IP SSRF allowances for trusted web-search and configured model-provider HTTP hosts, updates explicit proxy policy handling, and adds docs, tests, and a changelog entry.

Reproducibility: yes. source inspection gives a high-confidence reproduction path: make provider or trusted web endpoint DNS resolve to 198.18.0.0/15 or fc00::/7 on current main and the guarded fetch path blocks it because those policy flags are absent. I did not run a live fake-IP proxy repro in this read-only review.

Next step before merge
Protected maintainer label plus draft/mergeability state make this normal maintainer review, and no narrow repair finding was found.

Security
Cleared: The diff changes SSRF policy but keeps the fake-IP allowance hostname-scoped and does not add dependencies, scripts, credentials, workflow execution, or broader private-network access.

Review details

Best possible solution:

Finish maintainer review on this draft and, if the security boundary is accepted, land this narrower host-scoped policy as the superseding fix for #76530 and #76549.

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

Yes, source inspection gives a high-confidence reproduction path: make provider or trusted web endpoint DNS resolve to 198.18.0.0/15 or fc00::/7 on current main and the guarded fetch path blocks it because those policy flags are absent. I did not run a live fake-IP proxy repro in this read-only review.

Is this the best way to solve the issue?

Yes, the proposed direction appears narrower than the related broad allow-host proposals because it uses target hostname allowlisting plus specific fake-IP range flags instead of general private-network bypass. Final merge should wait for the draft/protected review and completed validation.

What I checked:

Likely related people:

Remaining risk / open question:

  • The cited Testbox pnpm check:changed run was still in progress at review time, so completed broad validation remains pending.
  • This changes SSRF policy semantics around fake-IP DNS ranges, so the protected maintainer review should explicitly confirm the host-scoped allowance is the intended security boundary.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 03e35b1d8392.

@steipete steipete force-pushed the codex/fake-ip-scoped-ssrf branch from 63b72d9 to a32607a Compare May 3, 2026 19:22
@steipete steipete marked this pull request as ready for review May 3, 2026 19:22
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a32607a72d

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/infra/net/ssrf.ts
Comment on lines +114 to +117
return {
allowRfc2544BenchmarkRange: true,
allowIpv6UniqueLocalRange: true,
hostnameAllowlist: [parsed.hostname],
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject fake-IP allowlist policy for literal IP base URLs

ssrfPolicyFromHttpBaseUrlFakeIpHostnameAllowlist currently returns a policy even when baseUrl is an IP literal, which turns on allowRfc2544BenchmarkRange/allowIpv6UniqueLocalRange and allows direct requests to 198.18.0.0/15 or fc00::/7 without allowPrivateNetwork. That broadens access beyond the intended “fake-IP DNS answer for trusted hostnames” scope and weakens SSRF protections for misconfigured or attacker-influenced base URLs; this helper should bail out when parsed.hostname is an IP literal.

Useful? React with 👍 / 👎.

@steipete steipete merged commit edb7e00 into main May 3, 2026
159 of 160 checks passed
@steipete steipete deleted the codex/fake-ip-scoped-ssrf branch May 3, 2026 19:27
@steipete
Copy link
Copy Markdown
Contributor Author

steipete commented May 3, 2026

Landed via rebase.

Landed commit: edb7e00
Source SHA: a32607a
Proof: local targeted format/Vitest/docs checks plus Blacksmith Testbox pnpm check:changed at https://github.com/openclaw/openclaw/actions/runs/25288382484. PR CI was clean before merge.

Thanks @zqchris for the fake-IP DNS reports in #76530 and #76549.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling docs Improvements or additions to documentation maintainer Maintainer-authored PR size: M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant