Repository navigation
fix(security): enforce URL validation on navigation and API request tools - #239
Open
beardthelion wants to merge 1 commit into
Open
beardthelion wants to merge 1 commit into
beardthelion wants to merge 1 commit into
Conversation
…ools
playwright_navigate passed args.url to page.goto unchecked, and the
playwright_get/post/put/patch/delete tools passed it to apiContext
unchecked. This allowed SSRF to loopback/internal destinations and
file:// reads, with response content returned to the caller.
Adds src/tools/common/urlValidator.ts:
- scheme allowlist (http/https only), rejects file:, data:, javascript:,
and URLs with embedded credentials
- literal-IP checks on the unbracketed hostname, so IPv6 literals like
[::1] and [::ffff:7f00:1] cannot slip past net.isIP
- dns.lookup({all: true}) resolution checking every A and AAAA answer,
failing closed on lookup errors
- full IPv4 + IPv6 private/reserved blocklists, including IPv4-mapped
IPv6, NAT64, Teredo, and 6to4 forms
- installNavigationGuard(): a browser-context route hook that revalidates
every request URL, covering redirects, subresources, and new tabs
- fetchWithValidation(): manual redirect following for the API tools so
each hop is revalidated before fetch
Wires validation into NavigationTool and all five API request tools.
Builds on the approach sketched in executeautomation#208; that version missed bracketed
IPv6 literals, AAAA-only hostnames, and redirect hops.
Known limitation: validation-time DNS and connect-time DNS are separate
resolutions, so a fast-rebinding hostname can still race the check.
Playwright does not expose connection pinning; this bounds the residual
risk rather than eliminating it.
This branch has not been deployed
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
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
src/tools/common/urlValidator.ts: http/https scheme allowlist, literal-IP checks on the unbracketed hostname,dns.lookup({all: true})covering A and AAAA with every answer checked, fail-closed on resolution errors, and blocklists for loopback, private, link-local, CGNAT, multicast, reserved, mapped-IPv4, NAT64, Teredo, and 6to4 ranges.playwright_navigatevalidates the URL beforepage.goto, and the browser context installs arouteguard so redirects, subresources, and new tabs are revalidated per request.playwright_get/post/put/patch/deletevalidate before fetching and follow redirects manually (maxRedirects: 0) so eachLocationis checked.file:,data:,javascript:, and credentialed URLs.This overlaps with #208 but closes cases that PR leaves open: bracketed IPv6 literals (
[::1],[::ffff:7f00:1]), AAAA-only hostnames, DNS error paths, and redirect hops.Test plan
npm test: 150/150 pass (mocks updated for thefetch-based API path and therouteguard)127.0.0.1,localhost,[::1],[::ffff:7f00:1],[fd00::1],file://, and redirects to internal addresses are blocked;https://example.comstill worksKnown limitation: Playwright does not expose connection pinning, so a hostname that rebinds between validation and connect retains a narrow TOCTOU window.