security: use request.url instead of req.headers.host in revalidation patch - #1331
security: use request.url instead of req.headers.host in revalidation patch#1331Ashutosh0x wants to merge 1 commit into
Conversation
🦋 Changeset detectedLatest commit: 034f85e The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| kind: identifier | ||
|
|
||
| fix: await (await import("@opennextjs/cloudflare")).getCloudflareContext().env.WORKER_SELF_REFERENCE.fetch(\`\${$REQ.headers.host.includes("localhost") ? "http":"https" }://\${$REQ.headers.host}$URL_PATH\`,{method:'HEAD', headers:$HEADERS}) | ||
| fix: await (await import("@opennextjs/cloudflare")).getCloudflareContext().env.WORKER_SELF_REFERENCE.fetch(\`\${new URL($REQ.url).protocol}//\${new URL($REQ.url).host}$URL_PATH\`,{method:'HEAD', headers:$HEADERS}) |
There was a problem hiding this comment.
🔴 On-demand page revalidation always fails with an invalid URL error
The revalidation request is built from the incoming request's path-only address (new URL($REQ.url) at packages/cloudflare/src/cli/build/patches/plugins/res-revalidate.ts:56) instead of a full address, so every on-demand revalidation call throws instead of refreshing the page.
Impact: res.revalidate() in Pages Router API routes breaks at runtime for all deployed apps, so pages can no longer be refreshed on demand.
Why `req.url` is not an absolute URL in this code path
The patch rewrites Next's revalidate() helper in api-utils / pages-api.runtime.prod.js, where req is a Node-style IncomingMessage. In Node/Next semantics req.url is a path with query (e.g. /api/revalidate?x=1), never an absolute URL — which is exactly why Next's original code composes https://${req.headers.host}${urlPath}, and why the sibling patch in packages/cloudflare/src/cli/build/patches/plugins/next-server.spec.ts:158 builds `${protocol}://${this.fetchHostname}:${this.port}${req.url}` from req.url as a suffix.
new URL("/api/foo") throws TypeError: Invalid URL. Because the generated code is inside the try block of revalidate(), the throw is converted to Failed to revalidate <path>: Invalid URL, so every res.revalidate() call fails.
A safe alternative that keeps the host non-user-controllable is to use a fixed placeholder origin for the service-binding call (the binding routes to the worker regardless of the URL host), e.g. `https://assets.local${urlPath}`, or to build the URL from req.url only as the path component.
Prompt for agents
The fix in res-revalidate.ts replaces `req.headers.host` with `new URL(req.url)`, but in the patched Next.js code (`api-utils` revalidate helper inside pages-api.runtime.prod.js / node/api-resolver.js) `req` is a Node IncomingMessage whose `url` is a path-only string such as `/api/revalidate?x=1`. `new URL()` on a relative path throws `TypeError: Invalid URL`, and since the generated call sits inside the revalidate try/catch, every `res.revalidate()` invocation will fail with `Failed to revalidate ...: Invalid URL`. Rework the ast-grep `fix` template so it produces a valid absolute URL without relying on a user-controllable host — e.g. use a constant internal origin for the WORKER_SELF_REFERENCE service-binding fetch (the binding routes to the worker regardless of host), or parse `req.url` with an explicit base. Update the inline snapshots in res-revalidate.spec.ts accordingly (the minified-code snapshot was not regenerated and currently still shows the old output).
Was this helpful? React with 👍 or 👎 to provide feedback.
| "@opennextjs/cloudflare": patch | ||
| --- | ||
|
|
||
| security: use request.url instead of req.headers.host in revalidation patch |
There was a problem hiding this comment.
🟡 Changeset uses a change type that is not allowed by the project's contribution rules
The new changeset entry starts with the type word security: (.changeset/fix-host-header-revalidation.md:5), which is not among the change types the repository allows, so the generated changelog entry will be inconsistent.
Impact: The released changelog gets an entry that does not follow the project's documented format.
Rule reference
AGENTS.md and CONTRIBUTING.md both require the changeset title to be <type>: <imperative title> where type is one of feature | fix | refactor | docs | chore. security is not in that list; fix is the appropriate type here.
| security: use request.url instead of req.headers.host in revalidation patch | |
| fix: use request.url instead of req.headers.host in revalidation patch |
Was this helpful? React with 👍 or 👎 to provide feedback.
48e4b1a to
c000f40
Compare
c000f40 to
034f85e
Compare
Summary
The patchResRevalidate code in
es-revalidate.ts interpolates
eq.headers.host directly into the WORKER_SELF_REFERENCE.fetch() URL without validation. The HTTP Host header is user-controllable, allowing host header injection in the internal service binding request.
The Fix
Replace
eq.headers.host with
ew URL(req.url).host, which is set by the Cloudflare Workers runtime and is not user-controllable. Also use
ew URL(req.url).protocol instead of the localhost string check for protocol detection.
The WORKER_SELF_REFERENCE service binding routes requests to the correct worker regardless of the URL, so this change has zero functional impact.
Before
js WORKER_SELF_REFERENCE.fetch( ${req.headers.host.includes('localhost') ? 'http' : 'https'}://${req.headers.host}${urlPath}, {method: 'HEAD', headers: revalidateHeaders} )After
js WORKER_SELF_REFERENCE.fetch( ${new URL(req.url).protocol}//${new URL(req.url).host}${urlPath}, {method: 'HEAD', headers: revalidateHeaders} )Security Impact