Skip to content

fix(network): validate and pin outbound destinations - #84

Open
iO7i wants to merge 1 commit into
KernelCode:mainfrom
iO7i:fix/resolved-network-policy
Open

iO7i wants to merge 1 commit into
KernelCode:mainfrom
iO7i:fix/resolved-network-policy

Conversation

@iO7i

@iO7i iO7i commented Sep 8, 2026

Copy link
Copy Markdown

Why

Addresses the outbound-network SSRF finding in #66 (not the other findings).

curl_url currently checks the hostname string, while curl resolves the actual destination later. A public-looking name can therefore pass validation and resolve to an internal address. The tool also accepts non-HTTP URL schemes despite its HTTP contract.

The invariant here is: the destination used by execution must be the destination that passed validation.

What changed

  • Added a shared, dependency-free network policy using Node's DNS and IP primitives. Resolve all returned addresses; reject empty, failed, timed-out, malformed, or mixed public/non-public results.
  • Restrict curl_url to HTTP(S). Execute the canonical parsed URL and pin hostname requests with --resolve, retaining the hostname for HTTP and TLS. Literal IP URLs are validated and already numeric.
  • Disable curlrc loading, URL globbing, and environment proxies for this tool so they cannot redirect execution around the pin. Redirect following remains disabled; certificate verification is unchanged.
  • Make check_port probe the validated numeric address with netcat's numeric-only mode. The explicit/default localhost diagnostic remains permitted and uses 127.0.0.1 without DNS; other loopback/private targets are rejected.
  • Add npm test using Node's built-in runner, with injected DNS and command execution. No new package dependency.

The address policy is deliberately conservative: IPv4 special-use ranges are denied, and IPv6 is restricted to ordinary global unicast excluding special-purpose/tunneling ranges. This also rejects documentation ranges and IPv4-mapped IPv6, even when the embedded IPv4 address is public. Pinning chooses one validated address and does not introduce connection fallback. These tools now require direct outbound access rather than an environment proxy.

Scope is limited to these outbound tools; no changes to trust levels, tool assignment, nginx reload behavior, or proxy policy elsewhere. This is not a sandbox for other tools or a replacement for deployment-level egress controls.

Behavioral verification

48 deterministic tests pass on Node 22.12.0. Coverage includes private/special IPv4 and IPv6 literals, alternate numeric URL spellings, non-HTTP schemes, private and mixed DNS answers in both orders, DNS failure/timeout, and the explicit localhost exception.

Handler-level tests verify that a resolver which would return loopback on a second call is called only once, the resulting command pins the first validated address, custom ports/IPv6 pins are formatted correctly, the original HTTP(S) hostname is retained, and blocked requests never reach command execution. The tests also check curlrc/proxy/globbing safeguards and absence of redirect/TLS-bypass flags.

DNS and command execution are mocked throughout; no live network, curl/netcat process, or end-to-end application run is claimed. The existing frozen pnpm install fails because the lockfile still lists better-sqlite3, which is absent from package.json; the lockfile is unchanged here.

@iO7i
iO7i marked this pull request as ready for review September 8, 2026 15:51
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.

1 participant