From 0b30b35f864310bee8485c952d1877e82e2b9b1a Mon Sep 17 00:00:00 2001 From: Matthew Phillips Date: Thu, 2 Jul 2026 08:18:27 -0400 Subject: [PATCH 1/6] Apply the origin check to Astro Actions regardless of pipeline order (#17250) * Apply origin check to Astro Actions independent of pipeline order * Apply origin check at endpoint dispatch in composable pipeline --- .changeset/jolly-clocks-thank.md | 5 + packages/astro/src/actions/handler.ts | 15 +++ packages/astro/src/core/app/middlewares.ts | 66 ----------- packages/astro/src/core/app/origin-check.ts | 94 +++++++++++++++ packages/astro/src/core/base-pipeline.ts | 2 +- packages/astro/src/core/pages/handler.ts | 17 ++- .../units/actions/action-origin-check.test.ts | 110 ++++++++++++++++++ packages/astro/test/units/app/csrf.test.ts | 2 +- .../units/hono/pages-origin-check.test.ts | 83 +++++++++++++ 9 files changed, 325 insertions(+), 69 deletions(-) create mode 100644 .changeset/jolly-clocks-thank.md delete mode 100644 packages/astro/src/core/app/middlewares.ts create mode 100644 packages/astro/src/core/app/origin-check.ts create mode 100644 packages/astro/test/units/actions/action-origin-check.test.ts create mode 100644 packages/astro/test/units/hono/pages-origin-check.test.ts diff --git a/.changeset/jolly-clocks-thank.md b/.changeset/jolly-clocks-thank.md new file mode 100644 index 000000000000..0e44ed713f6b --- /dev/null +++ b/.changeset/jolly-clocks-thank.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Fixes the `security.checkOrigin` check so it is applied consistently to Astro Actions and on-demand endpoints, regardless of how the request pipeline is composed. Previously, the origin check could be skipped in the composable `astro/hono` pipeline depending on the order of the `middleware()` primitive (or when it was omitted). diff --git a/packages/astro/src/actions/handler.ts b/packages/astro/src/actions/handler.ts index 50d0255ab196..cd3a39d540a7 100644 --- a/packages/astro/src/actions/handler.ts +++ b/packages/astro/src/actions/handler.ts @@ -1,4 +1,8 @@ import type { APIContext } from '../types/public/context.js'; +import { + createCrossOriginForbiddenResponse, + isForbiddenCrossOriginRequest, +} from '../core/app/origin-check.js'; import { PipelineFeatures } from '../core/base-pipeline.js'; import type { FetchState } from '../core/fetch/fetch-state.js'; import { getActionContext, serializeActionResult } from './runtime/server.js'; @@ -41,6 +45,17 @@ export class ActionHandler { return undefined; } + // The origin check normally runs in the origin-check middleware, but the + // action dispatch can run before that middleware depending on how the + // pipeline is composed. Apply the same check here so it holds regardless + // of ordering. + if ( + state.pipeline.manifest.checkOrigin && + isForbiddenCrossOriginRequest(apiContext.request, apiContext.url, apiContext.isPrerendered) + ) { + return Promise.resolve(createCrossOriginForbiddenResponse(apiContext.request)); + } + return this.#executeAction(action, setActionResult); } diff --git a/packages/astro/src/core/app/middlewares.ts b/packages/astro/src/core/app/middlewares.ts deleted file mode 100644 index cea45343ce54..000000000000 --- a/packages/astro/src/core/app/middlewares.ts +++ /dev/null @@ -1,66 +0,0 @@ -import type { MiddlewareHandler } from '../../types/public/common.js'; -import { defineMiddleware } from '../middleware/defineMiddleware.js'; - -/** - * Content types that can be passed when sending a request via a form - * - * https://developer.mozilla.org/en-US/docs/Web/API/HTMLFormElement/enctype - * @private - */ -const FORM_CONTENT_TYPES = [ - 'application/x-www-form-urlencoded', - 'multipart/form-data', - 'text/plain', -]; - -// Note: TRACE is unsupported by undici/Node.js -const SAFE_METHODS = ['GET', 'HEAD', 'OPTIONS']; - -/** - * Returns a middleware function in charge to check the `origin` header. - * - * @private - */ -export function createOriginCheckMiddleware(): MiddlewareHandler { - return defineMiddleware((context, next) => { - const { request, url, isPrerendered } = context; - // Prerendered pages should be excluded - if (isPrerendered) { - return next(); - } - // Safe methods don't require origin check - if (SAFE_METHODS.includes(request.method)) { - return next(); - } - const isSameOrigin = request.headers.get('origin') === url.origin; - - const hasContentType = request.headers.has('content-type'); - if (hasContentType) { - const formLikeHeader = hasFormLikeHeader(request.headers.get('content-type')); - if (formLikeHeader && !isSameOrigin) { - return new Response(`Cross-site ${request.method} form submissions are forbidden`, { - status: 403, - }); - } - } else { - if (!isSameOrigin) { - return new Response(`Cross-site ${request.method} form submissions are forbidden`, { - status: 403, - }); - } - } - - return next(); - }); -} - -export function hasFormLikeHeader(contentType: string | null): boolean { - if (contentType) { - for (const FORM_CONTENT_TYPE of FORM_CONTENT_TYPES) { - if (contentType.toLowerCase().includes(FORM_CONTENT_TYPE)) { - return true; - } - } - } - return false; -} diff --git a/packages/astro/src/core/app/origin-check.ts b/packages/astro/src/core/app/origin-check.ts new file mode 100644 index 000000000000..e0950e331db1 --- /dev/null +++ b/packages/astro/src/core/app/origin-check.ts @@ -0,0 +1,94 @@ +/** + * Home of the `security.checkOrigin` logic. Exposes the shared predicate and + * response used to reject cross-site submissions, plus the middleware factory + * that installs the check in the request pipeline. Consumers (the pipeline + * middleware and the Astro Actions dispatch) import from here so the check + * stays consistent regardless of where it runs. + */ +import type { MiddlewareHandler } from '../../types/public/common.js'; +import { defineMiddleware } from '../middleware/defineMiddleware.js'; + +/** + * Content types that can be passed when sending a request via a form + * + * https://developer.mozilla.org/en-US/docs/Web/API/HTMLFormElement/enctype + * @private + */ +const FORM_CONTENT_TYPES = [ + 'application/x-www-form-urlencoded', + 'multipart/form-data', + 'text/plain', +]; + +// Note: TRACE is unsupported by undici/Node.js +const SAFE_METHODS = ['GET', 'HEAD', 'OPTIONS']; + +/** + * Determines whether a request should be rejected because it is a cross-site + * submission for a route rendered on demand. + * + * This encapsulates the shared logic used by both the origin-check middleware + * and the Astro Actions dispatch, so the check is applied consistently + * regardless of where the request is handled. + * + * @private + */ +export function isForbiddenCrossOriginRequest( + request: Request, + url: URL, + isPrerendered: boolean, +): boolean { + // Prerendered pages should be excluded + if (isPrerendered) { + return false; + } + // Safe methods don't require origin check + if (SAFE_METHODS.includes(request.method)) { + return false; + } + const isSameOrigin = request.headers.get('origin') === url.origin; + + const hasContentType = request.headers.has('content-type'); + if (hasContentType) { + const formLikeHeader = hasFormLikeHeader(request.headers.get('content-type')); + return formLikeHeader && !isSameOrigin; + } + return !isSameOrigin; +} + +/** + * Builds the 403 response returned when a cross-site submission is rejected. + * + * @private + */ +export function createCrossOriginForbiddenResponse(request: Request): Response { + return new Response(`Cross-site ${request.method} form submissions are forbidden`, { + status: 403, + }); +} + +/** + * Returns a middleware function in charge to check the `origin` header. + * + * @private + */ +export function createOriginCheckMiddleware(): MiddlewareHandler { + return defineMiddleware((context, next) => { + const { request, url, isPrerendered } = context; + if (isForbiddenCrossOriginRequest(request, url, isPrerendered)) { + return createCrossOriginForbiddenResponse(request); + } + return next(); + }); +} + +export function hasFormLikeHeader(contentType: string | null): boolean { + if (contentType) { + for (const FORM_CONTENT_TYPE of FORM_CONTENT_TYPES) { + if (contentType.toLowerCase().includes(FORM_CONTENT_TYPE)) { + return true; + } + } + } + return false; +} diff --git a/packages/astro/src/core/base-pipeline.ts b/packages/astro/src/core/base-pipeline.ts index 973508a36986..b0234dc76d19 100644 --- a/packages/astro/src/core/base-pipeline.ts +++ b/packages/astro/src/core/base-pipeline.ts @@ -11,7 +11,7 @@ import type { SSRManifest, SSRResult, } from '../types/public/internal.js'; -import { createOriginCheckMiddleware } from './app/middlewares.js'; +import { createOriginCheckMiddleware } from './app/origin-check.js'; import type { ServerIslandMappings } from './app/types.js'; import type { SinglePageBuiltModule } from './build/types.js'; import { ActionNotFoundError } from './errors/errors-data.js'; diff --git a/packages/astro/src/core/pages/handler.ts b/packages/astro/src/core/pages/handler.ts index 855822b7b67e..62239f8133b5 100644 --- a/packages/astro/src/core/pages/handler.ts +++ b/packages/astro/src/core/pages/handler.ts @@ -5,6 +5,10 @@ import type { BaseApp } from '../app/base.js'; import type { FetchState } from '../fetch/fetch-state.js'; import type { Pipeline } from '../base-pipeline.js'; import { ASTRO_ERROR_HEADER } from '../constants.js'; +import { + createCrossOriginForbiddenResponse, + isForbiddenCrossOriginRequest, +} from '../app/origin-check.js'; import { getCookiesFromResponse } from '../cookies/response.js'; // Shared empty-slots object so we don't allocate `{}` on every render for @@ -116,8 +120,19 @@ export class PagesHandler { if (!state.routeData) { return new Response(null, { status: 404, headers: { [ASTRO_ERROR_HEADER]: 'true' } }); } + const ctx = state.getAPIContext(); + // The origin check normally runs in the origin-check middleware, but a + // composable pipeline can dispatch here without running `middleware()` + // first (or at all). Apply the same check so it holds regardless of how + // the pipeline is composed. + if ( + this.#pipeline.manifest.checkOrigin && + isForbiddenCrossOriginRequest(ctx.request, ctx.url, ctx.isPrerendered) + ) { + return createCrossOriginForbiddenResponse(ctx.request); + } try { - return await this.handle(state, state.getAPIContext()); + return await this.handle(state, ctx); } catch (err: any) { // The header marker can't carry the error object, so render the // 500 page directly to preserve `error` and the logged stack. diff --git a/packages/astro/test/units/actions/action-origin-check.test.ts b/packages/astro/test/units/actions/action-origin-check.test.ts new file mode 100644 index 000000000000..b602809bed0f --- /dev/null +++ b/packages/astro/test/units/actions/action-origin-check.test.ts @@ -0,0 +1,110 @@ +import assert from 'node:assert/strict'; +import { describe, it } from 'node:test'; +import { Hono } from 'hono'; +import { defineAction } from '../../../dist/actions/runtime/server.js'; +import { appSymbol } from '../../../dist/core/constants.js'; +import { actions, middleware, pages } from '../../../dist/core/hono/index.js'; +import { createPage, createRouteData, createTestApp } from '../mocks.ts'; +import { spreadPart, staticPart } from '../routing/test-helpers.ts'; +import { createComponent, render } from '../../../dist/runtime/server/index.js'; +import type { RouteData } from '../../../dist/types/public/internal.js'; + +const noopPage = createComponent(() => render``); + +const actionRouteData: RouteData = createRouteData({ + route: '/_actions/[...path]', + type: 'endpoint', + component: 'astro/actions/runtime/entrypoints/route.js', + segments: [[staticPart('_actions')], [spreadPart('...path')]], + pathname: undefined, +}); + +/** + * Builds an App exposing a single `deleteAccount` form action, with the + * origin check enabled (the default). `deleteAccount.ran` flips to `true` + * only when the action handler actually executes, so tests can assert + * whether a request reached the action. + */ +function createActionsApp() { + const state = { ran: false }; + const app = createTestApp( + [ + createPage(noopPage, { route: '/' }), + { + routeData: actionRouteData, + module: (async () => ({ + page: () => import('../../../dist/actions/runtime/entrypoints/route.js'), + })) as any, + }, + ], + { + checkOrigin: true, + actionBodySizeLimit: 1024 * 1024, + actions: () => ({ + server: { + deleteAccount: defineAction({ + accept: 'form', + handler: async () => { + state.ran = true; + return { deleted: 1 }; + }, + }), + }, + }), + }, + ); + return { app, actionState: state }; +} + +/** + * Composes a Hono app around the given Astro app the way the `astro/hono` + * primitives expect (the app is attached to the request; the primitives + * derive their per-request `FetchState` from it). `actions()` is mounted + * *before* `middleware()` — the order shipped in the `advanced-routing` + * example — so the action dispatch runs before the origin-check middleware + * would. + */ +function createHonoApp(astroApp: ReturnType['app']) { + const hono = new Hono(); + hono.use(async (context, next) => { + Reflect.set(context.req.raw, appSymbol, astroApp); + await next(); + }); + hono.use(actions()); + hono.use(middleware()); + hono.use(pages()); + return hono; +} + +function actionRequest(origin: string) { + return new Request('http://localhost/_actions/deleteAccount', { + method: 'POST', + headers: { + origin, + 'content-type': 'application/x-www-form-urlencoded', + }, + body: 'confirm=1', + }); +} + +describe('Actions origin check with actions() mounted before middleware()', () => { + it('blocks a cross-origin action request before it runs', async () => { + const { app, actionState } = createActionsApp(); + const hono = createHonoApp(app); + + const res = await hono.fetch(actionRequest('http://evil.example')); + + assert.equal(res.status, 403); + assert.equal(actionState.ran, false, 'the action must not run for a cross-origin request'); + }); + + it('allows a same-origin action request', async () => { + const { app, actionState } = createActionsApp(); + const hono = createHonoApp(app); + + const res = await hono.fetch(actionRequest('http://localhost')); + + assert.equal(res.ok, true); + assert.equal(actionState.ran, true, 'the action should run for a same-origin request'); + }); +}); diff --git a/packages/astro/test/units/app/csrf.test.ts b/packages/astro/test/units/app/csrf.test.ts index ba00e57b78a7..bf66da3e22f6 100644 --- a/packages/astro/test/units/app/csrf.test.ts +++ b/packages/astro/test/units/app/csrf.test.ts @@ -3,7 +3,7 @@ import { describe, it } from 'node:test'; import { hasFormLikeHeader, createOriginCheckMiddleware, -} from '../../../dist/core/app/middlewares.js'; +} from '../../../dist/core/app/origin-check.js'; import { callMiddleware } from '../../../dist/core/middleware/callMiddleware.js'; import { createMockAPIContext, createResponseFunction } from '../mocks.ts'; diff --git a/packages/astro/test/units/hono/pages-origin-check.test.ts b/packages/astro/test/units/hono/pages-origin-check.test.ts new file mode 100644 index 000000000000..22b898d736f0 --- /dev/null +++ b/packages/astro/test/units/hono/pages-origin-check.test.ts @@ -0,0 +1,83 @@ +import assert from 'node:assert/strict'; +import { describe, it } from 'node:test'; +import { Hono } from 'hono'; +import { appSymbol } from '../../../dist/core/constants.js'; +import { pages } from '../../../dist/core/hono/index.js'; +import { createComponent, render } from '../../../dist/runtime/server/index.js'; +import { createEndpoint, createPage, createTestApp } from '../mocks.ts'; + +const noopPage = createComponent(() => render``); + +/** + * Builds an App with a single on-demand `POST /api/delete` endpoint and the + * origin check enabled (the default). `endpointState.ran` flips to `true` + * only when the endpoint handler actually executes. + */ +function createEndpointApp() { + const endpointState = { ran: false }; + const app = createTestApp( + [ + createPage(noopPage, { route: '/' }), + createEndpoint( + { + POST: () => { + endpointState.ran = true; + return new Response('deleted'); + }, + }, + { route: '/api/delete' }, + ), + ], + { checkOrigin: true }, + ); + return { app, endpointState }; +} + +/** + * Composes a Hono app that dispatches through `pages()` *without* mounting + * `middleware()` — a valid composition for a site with no custom middleware. + * The origin check historically lived only in `middleware()`, so this + * exercises the endpoint dispatch sink directly. + */ +function createHonoApp(astroApp: ReturnType['app']) { + const hono = new Hono(); + hono.use(async (context, next) => { + Reflect.set(context.req.raw, appSymbol, astroApp); + await next(); + }); + hono.use(pages()); + return hono; +} + +function endpointRequest(origin: string) { + return new Request('http://localhost/api/delete', { + method: 'POST', + headers: { + origin, + 'content-type': 'application/x-www-form-urlencoded', + }, + body: 'x=1', + }); +} + +describe('Endpoint origin check with pages() and no middleware()', () => { + it('blocks a cross-origin endpoint request before it runs', async () => { + const { app, endpointState } = createEndpointApp(); + const hono = createHonoApp(app); + + const res = await hono.fetch(endpointRequest('http://evil.example')); + + assert.equal(res.status, 403); + assert.equal(endpointState.ran, false, 'the endpoint must not run for a cross-origin request'); + }); + + it('allows a same-origin endpoint request', async () => { + const { app, endpointState } = createEndpointApp(); + const hono = createHonoApp(app); + + const res = await hono.fetch(endpointRequest('http://localhost')); + + assert.equal(res.ok, true); + assert.equal(endpointState.ran, true, 'the endpoint should run for a same-origin request'); + }); +}); From 014296439e084384432e13f2e5b192b0f595045d Mon Sep 17 00:00:00 2001 From: Franco Kaddour <147673141+FrancoKaddour@users.noreply.github.com> Date: Thu, 2 Jul 2026 10:57:46 -0300 Subject: [PATCH 2/6] fix(@astrojs/solid-js): detect Svelte 5 components using the `$$renderer` prop (#17270) Svelte renamed the internal renderer prop from `$$payload` to `$$renderer` in Svelte 5.x (#14433 fixed the Svelte renderer check). The Solid renderer copies that check to exclude Svelte components from being claimed as Solid, but was never updated to cover the new prop name. Projects that mix `@astrojs/solid-js` with `@astrojs/svelte` and use a newer Svelte 5 release now compile components with `$$renderer`, which the Solid renderer doesn't recognise, causing it to render those Svelte components as empty strings instead of passing them to the Svelte renderer. Fixes by mirroring the same two-prop check that the Svelte renderer already uses. --- .changeset/fix-solid-svelte5-renderer-prop-detection.md | 5 +++++ packages/integrations/solid/src/server.ts | 4 +++- 2 files changed, 8 insertions(+), 1 deletion(-) create mode 100644 .changeset/fix-solid-svelte5-renderer-prop-detection.md diff --git a/.changeset/fix-solid-svelte5-renderer-prop-detection.md b/.changeset/fix-solid-svelte5-renderer-prop-detection.md new file mode 100644 index 000000000000..7aacbad21331 --- /dev/null +++ b/.changeset/fix-solid-svelte5-renderer-prop-detection.md @@ -0,0 +1,5 @@ +--- +'@astrojs/solid-js': patch +--- + +Fix `@astrojs/solid-js` incorrectly claiming Svelte 5 components compiled with the newer `$$renderer` prop (instead of the legacy `$$payload`). Projects mixing Solid and Svelte could see Svelte components silently rendered as empty strings by the Solid renderer. diff --git a/packages/integrations/solid/src/server.ts b/packages/integrations/solid/src/server.ts index e6390fffe924..4bb81fc5a91f 100644 --- a/packages/integrations/solid/src/server.ts +++ b/packages/integrations/solid/src/server.ts @@ -26,7 +26,9 @@ async function check( // Svelte component renders fine by Solid as an empty string. The only way to detect // if this isn't a Solid but Svelte component is to unfortunately copy the check // implementation of the Svelte renderer. - if (Component.toString().includes('$$payload')) return false; + // `$$payload` is the legacy prop name; `$$renderer` is the name used since Svelte 5.x. + const componentStr = Component.toString(); + if (componentStr.includes('$$payload') || componentStr.includes('$$renderer')) return false; // There is nothing particularly special about Solid components. Basically they are just functions. // In general, components from other frameworks (eg, MDX, React, etc.) tend to render as "undefined", From eb6f97e391ee587747e37609c255c7cd4b9cce3c Mon Sep 17 00:00:00 2001 From: Matthew Phillips Date: Thu, 2 Jul 2026 10:34:10 -0400 Subject: [PATCH 3/6] Treat backslash-prefixed paths as internal in trailing-slash handling (#17252) * Treat backslash-prefixed paths as internal in trailing-slash handling * Fix TypeScript error in backslash-prefixed path test --- .changeset/node-backslash-trailing-slash.md | 10 +++++ .../node/test/trailing-slash.test.ts | 42 +++++++++++++++++++ packages/internal-helpers/src/path.ts | 8 +++- packages/internal-helpers/test/path.test.ts | 32 +++++++++++++- 4 files changed, 90 insertions(+), 2 deletions(-) create mode 100644 .changeset/node-backslash-trailing-slash.md diff --git a/.changeset/node-backslash-trailing-slash.md b/.changeset/node-backslash-trailing-slash.md new file mode 100644 index 000000000000..3626474ce1b4 --- /dev/null +++ b/.changeset/node-backslash-trailing-slash.md @@ -0,0 +1,10 @@ +--- +'@astrojs/internal-helpers': patch +'@astrojs/node': patch +--- + +Fixes trailing-slash handling for request paths that begin with a backslash + +With `trailingSlash: 'always'`, the standalone Node server could append a trailing slash to a request path that begins with a backslash (for example `/\example.com/foo`) and echo that path back in the `Location` header of a `301` response. Because browsers resolve a leading `\` the same way as `/`, the resulting `Location` could point off-site. + +Such paths are now recognized as internal paths, matching the existing handling for paths that begin with `//`, so they are no longer rewritten with a trailing slash. diff --git a/packages/integrations/node/test/trailing-slash.test.ts b/packages/integrations/node/test/trailing-slash.test.ts index a135a3b10495..430717803a53 100644 --- a/packages/integrations/node/test/trailing-slash.test.ts +++ b/packages/integrations/node/test/trailing-slash.test.ts @@ -1,9 +1,40 @@ import * as assert from 'node:assert/strict'; +import net from 'node:net'; import { after, before, describe, it } from 'node:test'; import * as cheerio from 'cheerio'; import nodejs from '../dist/index.js'; import { type Fixture, loadFixture, waitServerListen, type AdapterServer } from './test-utils.ts'; +/** + * Sends a raw HTTP/1.1 request so the exact request-target (including + * characters like `\` that URL parsers would otherwise normalize) reaches + * the server unchanged, mirroring `curl --path-as-is`. + */ +function rawRequest( + host: string, + port: number, + requestTarget: string, +): Promise<{ statusLine: string; head: string }> { + return new Promise((resolve, reject) => { + const socket = net.connect(port, host, () => { + socket.write( + `GET ${requestTarget} HTTP/1.1\r\nHost: ${host}:${port}\r\nConnection: close\r\n\r\n`, + ); + }); + let data = ''; + socket.setEncoding('utf8'); + socket.on('data', (chunk) => { + data += chunk; + }); + socket.on('error', reject); + socket.on('end', () => { + const head = data.split('\r\n\r\n')[0] ?? ''; + const statusLine = head.split('\r\n')[0] ?? ''; + resolve({ statusLine, head }); + }); + }); +} + describe('Trailing slash', () => { let fixture: Fixture; let server: AdapterServer; @@ -197,6 +228,17 @@ describe('Trailing slash', () => { ); assert.equal(res.status, 404); }); + + it('Does not append a trailing slash to a backslash-prefixed path', async () => { + // A path like `/\example.com/press` is treated by browsers as + // `//example.com/press`. It has to be sent as a raw request line + // (like `curl --path-as-is`) because URL parsers fold the + // backslash to a forward slash. It must not produce a 301 that + // echoes the backslash path back in the `Location` header. + const raw = await rawRequest(server.host ?? 'localhost', server.port, '/\\example.com/press'); + assert.match(raw.statusLine, /404/); + assert.equal(/^location:/im.test(raw.head), false); + }); }); }); describe('Never', async () => { diff --git a/packages/internal-helpers/src/path.ts b/packages/internal-helpers/src/path.ts index 58b0a67cf69e..9b7a82923181 100644 --- a/packages/internal-helpers/src/path.ts +++ b/packages/internal-helpers/src/path.ts @@ -83,7 +83,13 @@ const INTERNAL_PREFIXES = new Set(['/_', '/@', '/.', '//']); const JUST_SLASHES = /^\/{2,}$/; export function isInternalPath(path: string) { - return INTERNAL_PREFIXES.has(path.slice(0, 2)) && !JUST_SLASHES.test(path); + // Browsers follow the WHATWG URL spec and treat backslashes as forward + // slashes when resolving a path, so `/\host` behaves like `//host`. Fold + // backslashes to forward slashes before comparing the prefix so those + // paths are recognized as internal too, instead of being appended with a + // trailing slash and echoed back into a `Location` header. + const prefix = path.slice(0, 2).replace(/\\/g, '/'); + return INTERNAL_PREFIXES.has(prefix) && !JUST_SLASHES.test(path); } export function joinPaths(...paths: (string | undefined)[]) { diff --git a/packages/internal-helpers/test/path.test.ts b/packages/internal-helpers/test/path.test.ts index 3b1f89768261..c2be19d06478 100644 --- a/packages/internal-helpers/test/path.test.ts +++ b/packages/internal-helpers/test/path.test.ts @@ -1,6 +1,11 @@ import assert from 'node:assert/strict'; import { describe, it } from 'node:test'; -import { isParentDirectory, isRemotePath, normalizePathname } from '../dist/path.js'; +import { + isInternalPath, + isParentDirectory, + isRemotePath, + normalizePathname, +} from '../dist/path.js'; describe('isRemotePath', () => { const remotePaths = [ @@ -848,3 +853,28 @@ describe('normalizePathname', () => { }); }); }); + +describe('isInternalPath', () => { + it('recognizes known internal prefixes', () => { + assert.equal(isInternalPath('/_astro/index.js'), true); + assert.equal(isInternalPath('/@vite/client'), true); + assert.equal(isInternalPath('/.well-known/foo'), true); + assert.equal(isInternalPath('//example.com/press'), true); + }); + + it('treats backslash-prefixed paths like their forward-slash equivalent', () => { + // Browsers fold `\` to `/`, so `/\host` resolves like `//host`. + assert.equal(isInternalPath('/\\example.com/press'), true); + assert.equal(isInternalPath('/\\'), true); + }); + + it('does not flag regular paths', () => { + assert.equal(isInternalPath('/about'), false); + assert.equal(isInternalPath('/one/two'), false); + }); + + it('does not flag paths that are only slashes', () => { + assert.equal(isInternalPath('//'), false); + assert.equal(isInternalPath('///'), false); + }); +}); From fee9069cc1547fb6b4725b728207e3aea90364e4 Mon Sep 17 00:00:00 2001 From: Matthew Phillips Date: Thu, 2 Jul 2026 14:35:42 +0000 Subject: [PATCH 4/6] [ci] format --- packages/integrations/node/test/trailing-slash.test.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/integrations/node/test/trailing-slash.test.ts b/packages/integrations/node/test/trailing-slash.test.ts index 430717803a53..0a39cc7ebaf4 100644 --- a/packages/integrations/node/test/trailing-slash.test.ts +++ b/packages/integrations/node/test/trailing-slash.test.ts @@ -235,7 +235,11 @@ describe('Trailing slash', () => { // (like `curl --path-as-is`) because URL parsers fold the // backslash to a forward slash. It must not produce a 301 that // echoes the backslash path back in the `Location` header. - const raw = await rawRequest(server.host ?? 'localhost', server.port, '/\\example.com/press'); + const raw = await rawRequest( + server.host ?? 'localhost', + server.port, + '/\\example.com/press', + ); assert.match(raw.statusLine, /404/); assert.equal(/^location:/im.test(raw.head), false); }); From 5240e26c9dd91f9bc7140dcfacdb48d5a132830d Mon Sep 17 00:00:00 2001 From: Matthew Phillips Date: Thu, 2 Jul 2026 10:50:04 -0400 Subject: [PATCH 5/6] Validate attribute names on custom HTML elements during SSR (#17251) * Harden renderHTMLElement to drop invalid attribute names * update changeset * Apply suggestion from @matthewp --- .changeset/quick-otters-attend.md | 5 ++ .../astro/src/runtime/server/render/dom.ts | 8 ++- .../astro/src/runtime/server/render/util.ts | 2 +- .../test/units/render/html-primitives.test.ts | 51 ++++++++++++++++++- 4 files changed, 63 insertions(+), 3 deletions(-) create mode 100644 .changeset/quick-otters-attend.md diff --git a/.changeset/quick-otters-attend.md b/.changeset/quick-otters-attend.md new file mode 100644 index 000000000000..337594a650b0 --- /dev/null +++ b/.changeset/quick-otters-attend.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Hardens the handling of attribute rendering when using with custom elements. diff --git a/packages/astro/src/runtime/server/render/dom.ts b/packages/astro/src/runtime/server/render/dom.ts index f92b4889a38f..fc08efd02c47 100644 --- a/packages/astro/src/runtime/server/render/dom.ts +++ b/packages/astro/src/runtime/server/render/dom.ts @@ -1,7 +1,7 @@ import type { SSRResult } from '../../../types/public/internal.js'; import { markHTMLString } from '../escape.js'; import { renderSlotToString } from './slot.js'; -import { toAttributeString } from './util.js'; +import { INVALID_ATTR_NAME_CHAR, toAttributeString } from './util.js'; export function componentIsHTMLElement(Component: unknown) { return typeof HTMLElement !== 'undefined' && HTMLElement.isPrototypeOf(Component as object); @@ -18,6 +18,12 @@ export async function renderHTMLElement( let attrHTML = ''; for (const attr in props) { + // Reject attribute names with characters that could break out of the attribute context. + // Without this guard, untrusted prop keys spread onto the element could inject arbitrary + // markup or event-handler attributes (XSS). Mirrors the guard in `addAttribute`. + if (INVALID_ATTR_NAME_CHAR.test(attr)) { + continue; + } attrHTML += ` ${attr}="${toAttributeString(await props[attr])}"`; } diff --git a/packages/astro/src/runtime/server/render/util.ts b/packages/astro/src/runtime/server/render/util.ts index 88db687ad355..083abd5940c8 100644 --- a/packages/astro/src/runtime/server/render/util.ts +++ b/packages/astro/src/runtime/server/render/util.ts @@ -15,7 +15,7 @@ const DOUBLE_QUOTE_REGEX = /"/g; const STATIC_DIRECTIVES = new Set(['set:html', 'set:text']); // Per the HTML spec, attribute names must not contain ASCII whitespace, ", ', >, /, or =. -const INVALID_ATTR_NAME_CHAR = /[\s"'>/=]/; +export const INVALID_ATTR_NAME_CHAR = /[\s"'>/=]/; // converts (most) arbitrary strings to valid JS identifiers const toIdent = (k: string) => diff --git a/packages/astro/test/units/render/html-primitives.test.ts b/packages/astro/test/units/render/html-primitives.test.ts index ff399518cc18..96b1960e484d 100644 --- a/packages/astro/test/units/render/html-primitives.test.ts +++ b/packages/astro/test/units/render/html-primitives.test.ts @@ -1,5 +1,5 @@ import assert from 'node:assert/strict'; -import { describe, it } from 'node:test'; +import { after, before, describe, it } from 'node:test'; import * as cheerio from 'cheerio'; import { addAttribute, @@ -15,6 +15,7 @@ import { Fragment, render as renderTemplate, renderComponent, + renderHTMLElement, renderSlot, unescapeHTML, } from '../../../dist/runtime/server/index.js'; @@ -263,6 +264,54 @@ describe('spreadAttributes rejects invalid attribute keys', () => { }); }); +describe('renderHTMLElement rejects invalid attribute keys', () => { + // renderHTMLElement resolves the tag name through customElements.getName(). + // In a Node test environment this global doesn't exist, so we stub it. + const originalCustomElements = globalThis.customElements; + const result = {} as any; + + before(() => { + globalThis.customElements = { + getName: () => 'my-el', + } as any; + }); + + after(() => { + globalThis.customElements = originalCustomElements; + }); + + it('drops malicious keys while keeping valid ones', async () => { + const html = await renderHTMLElement( + result, + class {} as any, + { + 'onmouseover=alert(document.domain) x': 'y', + 'x>': 'z', + 'data-safe': 'ok', + }, + {}, + ); + const output = String(html); + assert.ok(output.includes('data-safe="ok"')); + assert.ok(!output.includes('onmouseover')); + assert.ok(!output.includes('