Skip to content

fix(network): check where a public-only host name resolves, not only how it is spelled - #15064

Merged
diegosouzapw merged 3 commits into
diegosouzapw:release/v3.8.51from
HouMinXi:fix/safe-outbound-fetch-resolve-and-pin
Sep 29, 2026
Merged

diegosouzapw merged 3 commits into
diegosouzapw:release/v3.8.51from
HouMinXi:fix/safe-outbound-fetch-resolve-and-pin

Conversation

@HouMinXi

@HouMinXi HouMinXi commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

⚠️ base-red inherited: #15032

Summary

safeOutboundFetch with guard "public-only" rejected private hosts by looking at
the URL's host name. A name is only a label: an attacker-owned name whose DNS
answer is 127.0.0.1, a LAN address or the cloud metadata address passed that
check and the request went there. The image and media downloads already resolve
every answer and pin the connection; the other "public-only" callers (the
built-in HTTP skill, the plugin marketplace, the favicon and model-discovery
fetches) did not.

Resolve the host when the guard is "public-only" and refuse it when any answer
is not public. A lookup that fails is left to the request, which fails the same
way. The built-in HTTP skill, whose URL comes from the caller's input, also asks
for the new pinDns option, so the connection goes to the address that was
checked rather than to a second answer from a name that changes it between
lookups. The pin is skipped for a request that goes through a proxy, which
resolves the name itself, and for one that follows redirects, which leave the
checked host. The "block-metadata" mode used for operator provider URLs is
unchanged.

The tests make names resolve to a real loopback listener, both for the guard's
lookup and for the connection, and count what reaches it.

Related Issues

  • None.

Validation

  • Change type: other (authz / security hardening)
  • Focused tests and category gates from the golden path: the test listed below, the 36 existing test files that use the guarded fetch, npm run typecheck:core, npm run check:cycles
  • npm run lint on the changed files (Prettier and ESLint clean)
  • Reconciled with the current active release base (release/v3.8.51 at 5b230f1e5d); focused checks rerun afterward
  • Production-code changes include a new or updated automated test in this PR

Each new test was run against the change reverted (it fails) and restored (it passes).

Tests Added Or Updated

  • tests/unit/safe-outbound-fetch-dns-guard.test.ts

Coverage Notes

The listed test exercises every changed production path: private, loopback, link-local and mixed answers, a public answer, a failed lookup, literals and the other guard modes (no lookup), and the pinned connection.

Reviewer Notes

Only guard: "public-only" resolves the host; block-metadata (operator provider URLs) and none are unchanged, so a name that resolves to a metadata address is still not caught in that mode. A name that does not resolve is not blocked by the guard; the request fails on its own. Without pinDns the check and the connection are two separate lookups, so a resolver that changes its answer between them can still get through; only the HTTP skill asks for the pin so far, and other callers can opt in.

Maintainer rework (merge-batch 2026-09-28 (release drain))

  • The public-only lookup had no deadline and ran before the request timeout, so a resolver that never answers held the request forever. It is now bounded by the request timeout (at most 5 s) and follows the caller's abort signal.
  • With pinDns, a failed lookup fell back to an unpinned connection that resolves the name again — the very answer the check exists to judge. When the connection was going to be pinned, a failed lookup now refuses the request (NETWORK_ERROR); unpinned requests keep the previous "let the request fail on its own" behaviour.
  • skills-builtins-sandbox.test.ts mocked globalThis.fetch, which the now-pinned HTTP skill no longer uses (it failed on the previous head and reached the real network). It now answers the lookup and stands in for the pinned connection through a test-only override (setSafeOutboundPinnedFetchTestOverride, same pattern as security: pinDns off at the three new public-only image fetch sites (DNS-rebinding TOCTOU) #13883).
  • The new test file no longer uses explicit any (lint error under tests/).
  • Reconciled with the tip: the createPinnedFetch large-body fix already landed there (fix(sse): download Adobe Firefly and UC persona reference images with the guarded fetch #15045 + dns-pinned-fetch-large-body.test.ts), so the tip's version is kept.
  • Red/green for the two added tests: both fail on the previous head (lookup failure connects unpinned to the loopback listener; a hanging lookup never returns) and pass now. Related suites: 40 files around safeOutboundFetch / remote image fetch / plugins / skills, all green.

…how it is spelled

safeOutboundFetch with guard "public-only" rejected private hosts by
looking at the URL's host name. A name is only a label: an attacker-owned
name whose DNS answer is 127.0.0.1, a LAN address or the cloud metadata
address passed that check and the request went there. The image and media
downloads already resolve every answer and pin the connection; the other
"public-only" callers (the built-in HTTP skill, the plugin marketplace,
the favicon and model-discovery fetches) did not.

Resolve the host when the guard is "public-only" and refuse it when any
answer is not public. A lookup that fails is left to the request, which
fails the same way. The built-in HTTP skill, whose URL comes from the
caller's input, also asks for pinDns, so the connection goes to the
address that was checked rather than to a second answer from a name that
changes it between lookups. The pin is skipped for a request that goes
through a proxy, which resolves the name itself, and for one that follows
redirects, which leave the checked host. The "block-metadata" mode used
for operator provider URLs is unchanged.

The tests make names resolve to a real loopback listener, both for the
guard's lookup and for the connection, and count what reaches it.

Signed-off-by: Minxi Hou <houminxi@gmail.com>
…a pinned request cannot resolve

Maintainer rework on top of the DNS check:
- the lookup had no deadline of its own and ran before the request timeout, so a
  resolver that never answers held the request forever; it is now bounded by the
  request timeout (at most 5 s) and follows the caller's abort signal;
- with pinDns, a failed lookup fell back to an unpinned connection that resolves the
  name again, which is exactly the answer the check exists to judge; the request is
  now refused (NETWORK_ERROR) when the connection was going to be pinned;
- createPinnedFetch awaited dispatcher.close() before returning, and close() waits for
  the body the caller has not read yet, so any response over ~100 KB deadlocked until
  the timeout; the close is no longer awaited (the built-in HTTP skill now goes through
  this path);
- the built-in HTTP skill test answers the lookup and stands in for the pinned
  connection through a test-only override (same pattern as diegosouzapw#13883);
- the new test file no longer uses explicit any (lint error under tests/).
# Conflicts:
#	src/shared/network/dnsPinnedFetch.ts
@diegosouzapw
diegosouzapw merged commit 0739f92 into diegosouzapw:release/v3.8.51 Sep 29, 2026
11 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants