[wrangler] Validate header rules reject multiple wildcards (#12276)

Co-authored-by: emily-shen <eshen@cloudflare.com>
Co-authored-by: emily-shen <69125074+emily-shen@users.noreply.github.com>
Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com>

Author: 3 people

.changeset/reject-multiple-wildcards-headers.md

@@ -0,0 +1,7 @@
+---
+"@cloudflare/workers-shared": patch
+---
+
+Warn when `_headers` rules contain multiple wildcards or wildcard combined with `:splat`
+
+Rules containing multiple wildcards (e.g. `https://*.workers.dev/*`) or combining a wildcard with a `:splat` placeholder (e.g. `https://*.pages.dev/:splat`) are now rejected during parsing. Previously this would fail silently during dev.

fixtures/workers-with-assets/public/_headers

@@ -1,2 +1,2 @@
 /
-	X-Header: Custom-Value
+	X-Header: Custom-Value

packages/workers-shared/utils/configuration/parseHeaders.ts

@@ -2,6 +2,7 @@ import {
 	HEADER_SEPARATOR,
 	MAX_HEADER_RULES,
 	MAX_LINE_LENGTH,
+	SPLAT_REGEX,
 	UNSET_OPERATOR,
 } from "./constants";
 import { validateUrl } from "./validateURL";
@@ -24,6 +25,10 @@ export function parseHeaders(
 	const invalid: InvalidHeadersRule[] = [];
 
 	let rule: (HeadersRule & { line: string }) | undefined = undefined;
+	// When a path line is rejected (invalid URL, multiple wildcards, etc.),
+	// we silently skip subsequent header lines until the next path line
+	// rather than emitting confusing "Path should come before header" errors.
+	let skipUntilNextPath = false;
 
 	for (let i = 0; i < lines.length; i++) {
 		const line = (lines[i] || "").trim();
@@ -41,6 +46,8 @@ export function parseHeaders(
 		}
 
 		if (LINE_IS_PROBABLY_A_PATH.test(line)) {
+			skipUntilNextPath = false;
+
 			if (rules.length >= maxRules) {
 				invalid.push({
 					message: `Maximum number of rules supported is ${maxRules}. Skipping remaining ${
@@ -74,6 +81,19 @@ export function parseHeaders(
 					message: pathError,
 				});
 				rule = undefined;
+				skipUntilNextPath = true;
+				continue;
+			}
+
+			const wildcardError = validateNoMultipleWildcards(path as string);
+			if (wildcardError) {
+				invalid.push({
+					line,
+					lineNumber: i + 1,
+					message: wildcardError,
+				});
+				rule = undefined;
+				skipUntilNextPath = true;
 				continue;
 			}
 
@@ -86,6 +106,10 @@ export function parseHeaders(
 			continue;
 		}
 
+		if (skipUntilNextPath) {
+			continue;
+		}
+
 		if (!line.includes(HEADER_SEPARATOR)) {
 			if (!rule) {
 				invalid.push({
@@ -174,3 +198,24 @@ export function parseHeaders(
 function isValidRule(rule: HeadersRule) {
 	return Object.keys(rule.headers).length > 0 || rule.unsetHeaders.length > 0;
 }
+
+/**
+ * At runtime, `*` wildcards are converted to `:splat` placeholders. This means
+ * a path with multiple wildcards, or a wildcard combined with an explicit
+ * `:splat` placeholder, would result in duplicate `:splat` parameters which is
+ * unsupported.
+ */
+function validateNoMultipleWildcards(path: string): string | undefined {
+	const wildcardCount = (path.match(SPLAT_REGEX) ?? []).length;
+	const hasSplatPlaceholder = /:splat(?!\w)/.test(path);
+
+	if (wildcardCount > 1) {
+		return `Only one wildcard is allowed per rule. Use a named placeholder (e.g. :project) instead. Skipping ${path}.`;
+	}
+
+	if (wildcardCount > 0 && hasSplatPlaceholder) {
+		return `Cannot combine a wildcard * with a :splat placeholder because wildcards are converted to :splat at runtime. Skipping ${path}.`;
+	}
+
+	return undefined;
+}

packages/workers-shared/utils/tests/parseHeaders.invalid.test.ts

@@ -124,6 +124,93 @@ test("parseHeaders should reject any rules after the first 100", ({
 	});
 });
 
+test("parseHeaders should reject paths with multiple wildcards", ({
+	expect,
+}) => {
+	const input = `
+  # Multiple wildcards in an absolute URL
+  https://*.pages.dev/*
+    x-custom: value
+  # Multiple wildcards in a relative path
+  /blog/*/posts/*
+    x-custom: value
+  # Single wildcard is fine
+  https://*.pages.dev/
+    x-custom: value
+  /blog/*
+    x-custom: value
+`;
+	const result = parseHeaders(input);
+	expect(result).toEqual({
+		rules: [
+			{
+				path: "https://*.pages.dev/",
+				headers: { "x-custom": "value" },
+				unsetHeaders: [],
+			},
+			{
+				path: "/blog/*",
+				headers: { "x-custom": "value" },
+				unsetHeaders: [],
+			},
+		],
+		invalid: [
+			{
+				line: "https://*.pages.dev/*",
+				lineNumber: 3,
+				message:
+					"Only one wildcard is allowed per rule. Use a named placeholder (e.g. :project) instead. Skipping https://*.pages.dev/*.",
+			},
+			{
+				line: "/blog/*/posts/*",
+				lineNumber: 6,
+				message:
+					"Only one wildcard is allowed per rule. Use a named placeholder (e.g. :project) instead. Skipping /blog/*/posts/*.",
+			},
+		],
+	});
+});
+
+test("parseHeaders should reject paths combining wildcard with :splat placeholder", ({
+	expect,
+}) => {
+	const input = `
+  # Wildcard + :splat in an absolute URL
+  https://*.pages.dev/:splat
+    x-custom: value
+  # Wildcard + :splat in a relative path
+  /blog/*/:splat
+    x-custom: value
+  # Just :splat without wildcard is fine
+  /blog/:splat
+    x-custom: value
+`;
+	const result = parseHeaders(input);
+	expect(result).toEqual({
+		rules: [
+			{
+				path: "/blog/:splat",
+				headers: { "x-custom": "value" },
+				unsetHeaders: [],
+			},
+		],
+		invalid: [
+			{
+				line: "https://*.pages.dev/:splat",
+				lineNumber: 3,
+				message:
+					"Cannot combine a wildcard * with a :splat placeholder because wildcards are converted to :splat at runtime. Skipping https://*.pages.dev/:splat.",
+			},
+			{
+				line: "/blog/*/:splat",
+				lineNumber: 6,
+				message:
+					"Cannot combine a wildcard * with a :splat placeholder because wildcards are converted to :splat at runtime. Skipping /blog/*/:splat.",
+			},
+		],
+	});
+});
+
 test("parseHeaders should reject malformed URLs", ({ expect }) => {
 	const input = `
   # Spaces should be URI encoded
@@ -167,33 +254,18 @@ test("parseHeaders should reject malformed URLs", ({ expect }) => {
 				message:
 					'URLs should either be relative (e.g. begin with a forward-slash), or use HTTPS (e.g. begin with "https://").',
 			},
-			{
-				line: "invalid: things",
-				lineNumber: 17,
-				message: "Path should come before header (invalid: things)",
-			},
 			{
 				line: "https://nah.com:8080",
 				lineNumber: 19,
 				message:
 					"Specifying ports is not supported. Skipping absolute URL https://nah.com:8080.",
 			},
-			{
-				line: "invalid: also",
-				lineNumber: 20,
-				message: "Path should come before header (invalid: also)",
-			},
 			{
 				line: "https://nah.com:8080/blog",
 				lineNumber: 21,
 				message:
 					"Specifying ports is not supported. Skipping absolute URL https://nah.com:8080/blog.",
 			},
-			{
-				line: "invalid: 2",
-				lineNumber: 22,
-				message: "Path should come before header (invalid: 2)",
-			},
 			{
 				line: "nah.com",
 				lineNumber: 30,