Repository navigation
fix(api): stop /api/grade from fetching arbitrary internal URLs - #33
Merged
Merged
Conversation
`/api/grade?url=` handed a caller-supplied string straight to `fetch()`: no scheme check, no address check, no timeout and no size cap, on a route with `maxDuration: 300`. Anything the function could reach, a visitor could reach through it — `169.254.169.254` included — and read back as a 400 or a grade. The multipart branch had the matching problem: with `bodyParser: false` nothing bounded the stream it buffered. The feature is "paste a link to your CV", so a host allowlist isn't on the table. `fetchRemoteResume` guards where the connection actually lands: - https only, no embedded credentials, port 443 only — an arbitrary port turns this into an internal port scanner. - Every resolved address is checked against loopback, RFC1918, CGNAT, link-local, benchmarking, documentation, multicast and reserved space, v4 and v6, including the transition formats that hide a v4 address inside a v6 one. If any record for a host is private the host is refused, so a round-robin that mixes public and private answers doesn't get a second roll. - The socket is pinned to the address we validated, with `servername` and `Host` still set to the name, so a DNS rebind can't swap the metadata endpoint in between the check and the connect. Validate-then-`fetch()` leaves that window open. - Redirects are followed by hand, three at most, each hop re-parsed and re-resolved from scratch — a `Location` is a fresh caller-supplied URL. - 20s deadline across all hops, and a 10MB cap on both `content-length` and the running total, since a chunked response declares nothing. Failures carry stable codes, so the route answers 4xx instead of falling through to the 500, and `getErrorMessage` maps them to copy. It now reads the code before looking at the status: our own 413 knows the route's limit, while Vercel's plain-text 413 is still the one that mentions its 4.5MB payload cap. Tests are network-free: address classification both ways, and the URL shapes refused before a socket opens — decimal and hex loopback encodings among them, which `new URL()` normalizes and the resolver would otherwise reach happily. The timeout and both size-cap branches were checked against real origins locally; those cases stay out of the suite rather than make CI depend on third-party hosts. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xa76D7n4CLkdKt99Su1HRy
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This branch was successfully 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.
/api/grade?url=handed a caller-supplied string straight tofetch(): no scheme check, no address check, no timeout and no size cap, on a route withmaxDuration: 300. Anything the function could reach, a visitor could reach through it —169.254.169.254included — and read back as a 400 or a grade. The multipart branch had the matching problem: withbodyParser: falsenothing bounded the stream it buffered.The feature is "paste a link to your CV", so a host allowlist isn't on the table.
fetchRemoteResumeguards where the connection actually lands:::ffff:, NAT64, 6to4). If any record for a host is private the host is refused, so a round-robin mixing public and private answers doesn't get a second roll.servernameandHoststill set to the name, so a DNS rebind can't swap the metadata endpoint in between the check and the connect. Validate-then-fetch()leaves that window open.Locationis a fresh caller-supplied URL.content-lengthand the running total, since a chunked response declares nothing.Failures carry stable codes, so the route answers 4xx instead of falling through to the 500, and
getErrorMessagemaps them to copy. It now reads the code before looking at the status: our own 413 knows the route's limit, while Vercel's plain-text 413 is still the one that mentions its 4.5MB payload cap.Behavior change worth flagging
A genuinely slow origin now fails with
ResumeFetchTimeoutat 20s instead of grinding against the 300s route budget.maxDuration: 300is untouched — the model call still lives under it.Testing
22 new cases, all network-free: address classification both ways, and the URL shapes refused before a socket opens — decimal (
https://2130706433/) and hex loopback encodings among them, whichnew URL()normalizes and the resolver would otherwise reach happily. Plus the route's own contract: 400 for loopback/metadata/non-https, 413 for an upload over the cap.Verified separately against real origins, kept out of the suite so CI doesn't depend on third-party hosts: a real public PDF still downloads, a redirect to a public host still follows, a public hostname resolving to 127.0.0.1 is blocked, and both size-cap branches and both deadline branches fire.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Xa76D7n4CLkdKt99Su1HRy