[pull] main from withastro:main

.changeset/harden-attribute-escaping.md

@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Removes an escape hatch that skipped attribute escaping for URL values containing `&`, ensuring all dynamic attribute values are consistently escaped

packages/astro/src/runtime/server/render/util.ts

@@ -120,11 +120,6 @@ Make sure to use the static attribute syntax (\`${key}={value}\`) instead of the
 		return markHTMLString(` class="${toAttributeString(value, shouldEscape)}"`);
 	}
 
-	// Prevents URLs in attributes from being escaped in static builds
-	if (typeof value === 'string' && value.includes('&') && isHttpUrl(value)) {
-		return markHTMLString(` ${key}="${toAttributeString(value, false)}"`);
-	}
-
 	// Boolean values only need the key
 	if (htmlBooleanAttributes.test(key)) {
 		return handleBooleanAttribute(key, value, shouldEscape, tagName);
@@ -308,13 +303,3 @@ export function promiseWithResolvers<T = any>(): PromiseWithResolvers<T> {
 		reject,
 	};
 }
-
-const VALID_PROTOCOLS = ['http:', 'https:'];
-function isHttpUrl(url: string) {
-	try {
-		const parsedUrl = new URL(url);
-		return VALID_PROTOCOLS.includes(parsedUrl.protocol);
-	} catch {
-		return false;
-	}
-}

packages/astro/test/astro-attrs.test.js

@@ -70,9 +70,9 @@ describe('Attributes', async () => {
 			"normal attributes should not have values if it's an empty string",
 		);
 
-		// cheerio will unescape the values, so checking that the url rendered unescaped to begin with has to be done manually
+		// cheerio will unescape the values, so checking that the url rendered escaped has to be done manually
 		assert.equal(
-			html.includes('https://example.com/api/og?title=hello&description=somedescription'),
+			html.includes('https://example.com/api/og?title=hello&description=somedescription'),
 			true,
 		);
 

packages/astro/test/units/app/url-attribute-xss.test.js

@@ -0,0 +1,36 @@
+// @ts-check
+import assert from 'node:assert/strict';
+import { describe, it } from 'node:test';
+import { addAttribute } from '../../../dist/runtime/server/render/util.js';
+
+describe('addAttribute URL XSS', () => {
+	it('escapes double quotes in HTTP URLs containing &', () => {
+		const maliciousUrl = 'https://evil.com/?a=1&b=2"+onmouseover="alert(1)';
+		const result = String(addAttribute(maliciousUrl, 'href'));
+
+		// The " must be escaped so the value stays inside a single attribute
+		assert.ok(result.includes('"'), `double quotes should be escaped, got: ${result}`);
+		assert.match(
+			result,
+			/^\s+href="[^"]*"$/,
+			`should be a single well-formed attribute, got: ${result}`,
+		);
+	});
+
+	it('escapes & in HTTP URLs', () => {
+		const url = 'https://example.com/?a=1&b=2&c=3';
+		const result = String(addAttribute(url, 'href'));
+
+		assert.ok(result.includes('&'), `ampersands should be escaped, got: ${result}`);
+		assert.match(
+			result,
+			/^\s+href="[^"]*"$/,
+			`should be a single well-formed attribute, got: ${result}`,
+		);
+	});
+
+	it('handles null and undefined values', () => {
+		assert.equal(addAttribute(null, 'href'), '');
+		assert.equal(addAttribute(undefined, 'href'), '');
+	});
+});