fix(api): restrict /api/fetch to the hosts we actually fetch from - #513
Merged
Conversation
`/api/fetch` passed its request body straight to `fetch`, so the dev server would request whatever it was pointed at. It is dev-only — the deployed site is a static export with no API routes, and the one caller guards on `location.hostname === 'localhost'` — so this is hardening a local tool rather than closing a hole on the live site. Three layers, in `src/lib/server/safe-fetch.ts`: - A host allowlist. `image-util.ts` already knows the only three hosts we fetch HTML from; `src/lib/fetchable-hosts.ts` makes that list something both sides import instead of each keeping its own copy. The suffix match anchors on a dot, so `imgbox.com.evil.com` does not pass. - A resolved-address check, covering the ranges that are easy to miss by hand: IPv4-mapped IPv6, CGNAT, multicast and reserved space. - Manual redirect handling, revalidating every hop. Without it an allowed host can redirect us to link-local metadata and the allowlist means nothing. This does not stop DNS rebinding, and says so in a comment rather than implying otherwise: `fetch` resolves independently of our lookup. The allowlist is what makes that acceptable — exploiting it needs authoritative DNS for one of three specific domains. Rejected URLs now answer 400 instead of 500, so "unsupported link" and "upstream is down" are distinguishable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
/api/fetchpassed its request body straight tofetch, so the dev server would request whatever it was pointed at. It is dev-only — the deployed site is a static export with no API routes, and the one caller guards onlocation.hostname === 'localhost'— so this is hardening a local tool rather than closing a hole on the live site.Three layers, in
src/lib/server/safe-fetch.ts:image-util.tsalready knows the only three hosts we fetch HTML from;src/lib/fetchable-hosts.tsmakes that list something both sides import instead of each keeping its own copy. The suffix match anchors on a dot, soimgbox.com.evil.comdoes not pass.This does not stop DNS rebinding, and says so in a comment rather than implying otherwise:
fetchresolves independently of our lookup. The allowlist is what makes that acceptable — exploiting it needs authoritative DNS for one of three specific domains.Rejected URLs now answer 400 instead of 500, so "unsupported link" and "upstream is down" are distinguishable.