[Fix] Production-grade SSRF guard with numeric IP parsing and DNS pre-resolution#189
Merged
[Fix] Production-grade SSRF guard with numeric IP parsing and DNS pre-resolution#189
Conversation
… pre-resolution Extract SSRF protection from auto-extract.ts into a shared net/ module: - ssrf-guard.ts: numeric IPv4/IPv6 range checks covering all RFC 1918, link-local, CGNAT, benchmark, and reserved ranges; IPv4-mapped IPv6 detection; DNS pre-resolution to block rebinding attacks - safe-fetch.ts: unified HTTPS-only fetch with SSRF guard, size limit, and timeout — replaces ad-hoc fetch logic in both auto-extract.ts and artifact-handlers.ts The previous string-prefix approach had false positives (blocking 10.cdn.example.com) and false negatives (IPv6-mapped IPv4 bypass, DNS rebinding). This replaces it with proper numeric IP parsing via node:net isIP() and pre-fetch DNS resolution via node:dns/promises. Adds 56 tests (49 ssrf-guard + 7 safe-fetch).
Contributor
|
Hi @samzong, DetailsInstructions for interacting with me using comments are available here. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replace the string-prefix SSRF check in
auto-extract.tswith a proper numeric IP parsing module and DNS pre-resolution guard, extracted into a sharednet/layer used by all outbound fetch paths.Type of change
[Fix]bug fixWhy is this needed?
The previous
isPrivateHostonly matched three literal strings (localhost,127.0.0.1,::1). This missed all RFC 1918 ranges, link-local, CGNAT, reserved, and IPv6 private addresses. The todo.md suggested a string-prefix approach which introduced false positives (blocking10.cdn.example.com) and still missed IPv6-mapped IPv4 and DNS rebinding vectors. Additionally,artifact-handlers.tshad a barenet.fetchwith zero SSRF protection.What changed?
src/main/net/ssrf-guard.ts: Numeric IPv4/IPv6 range checks vianode:netisIP()covering 10/8, 172.16/12, 192.168/16, 127/8, 169.254/16, 0/8, 100.64/10 (CGNAT), 198.18/15 (benchmark), 240/4 (reserved), IPv6 loopback/ULA/link-local, and IPv4-mapped IPv6 (both dotted and hex forms). DNS pre-resolution vianode:dns/promisesblocks rebinding attacks.src/main/net/safe-fetch.ts: Unified HTTPS-only fetch wrapper with SSRF guard, configurable size limit (default 10MB), and timeout (default 10s).auto-extract.ts: Removed inlineisPrivateHost,fetchToBuffer, and related constants. Now usessafeFetch.artifact-handlers.ts: Replaced unprotectednet.fetchinartifact:save-image-urlwithsafeFetch.Architecture impact
docs/architecture-invariants.md: noneLinked issues
Closes the SSRF private host check gap identified in the worktree todo.
Validation
pnpm lintpnpm testpnpm check:ui-contractpnpm check(full gate: lint + architecture + ui-contract + renderer-copy + i18n + dead-code + format + typecheck + test)pnpm buildCommands, screenshots, or notes:
Screenshots or recordings
No UI changes.
Release note
NONE.Checklist
[Feat],[Fix],[UI],[Docs],[Refactor],[Build], or[Chore]