Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .changeset/bold-stars-6d152.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
'astro': patch
---

Fixes double URL-encoded paths returning 400 Bad Request on on-demand routes

Previously, any URL containing a double-encoded character (like `%255B`, which is `[` encoded twice) was unconditionally rejected with a `400 Bad Request` before middleware or route handlers could run. This broke embedded tools like Sanity Studio whose client-side router legitimately produces double-encoded URLs.

The fix replaces the rejection approach with iterative decoding — multi-level percent-encoding is now fully resolved to its canonical form before being passed to middleware and route matching. This preserves the security fix for CVE-2025-66202 (middleware authorization bypass via double encoding) because middleware now always sees the fully decoded path, making bypass impossible. For example, `/api/%2561dmin` is decoded to `/api/admin`, which middleware can correctly block.
5 changes: 5 additions & 0 deletions .changeset/fix-compress-html-head-join.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"astro": patch
---

fix(render): honour compressHTML when joining head elements
9 changes: 6 additions & 3 deletions packages/astro/src/assets/build/generate.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import fs, { readFileSync } from 'node:fs';
import { basename } from 'node:path/posix';
import colors from 'piccolore';
import { getOutDirWithinCwd } from '../../core/build/common.js';
import type { StaticBuildOptions } from '../../core/build/types.js';
import { getTimeStat } from '../../core/build/util.js';
import { AstroError } from '../../core/errors/errors.js';
import { AstroErrorData } from '../../core/errors/index.js';
import type { AstroLogger } from '../../core/logger/core.js';
import { isRemotePath, removeLeadingForwardSlash } from '../../core/path.js';
import { getClientOutputDirectory } from '../../prerender/utils.js';
import type { MapValue } from '../../type-utils.js';
import type { AstroConfig } from '../../types/public/config.js';
import { getConfiguredImageService } from '../internal.js';
Expand Down Expand Up @@ -76,8 +76,11 @@ export async function prepareAssetsGenerationEnv(
serverRoot = new URL('.prerender/', settings.config.build.server);
clientRoot = settings.config.build.client;
} else {
serverRoot = getOutDirWithinCwd(settings.config.outDir);
clientRoot = settings.config.outDir;
// For static builds, images have already been moved to the client output directory
// by ssrMoveAssets. Use getClientOutputDirectory to respect preserveBuildClientDir.
const clientOutputDir = getClientOutputDirectory(settings);
serverRoot = clientOutputDir;
clientRoot = clientOutputDir;
}

return {
Expand Down
28 changes: 9 additions & 19 deletions packages/astro/src/core/app/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import { appSymbol } from '../constants.js';
import { DefaultErrorHandler } from '../errors/default-handler.js';
import type { ErrorHandler } from '../errors/handler.js';
import { setRenderOptions } from './render-options.js';
import { MultiLevelEncodingError } from '../util/pathname.js';
import type { WaitUntilHook } from '../wait-until.js';
import type { AppPipeline } from './pipeline.js';
import type { SSRManifest } from './types.js';
Expand Down Expand Up @@ -407,24 +406,15 @@ export abstract class BaseApp<P extends Pipeline = AppPipeline> {
};

let response: Response;
try {
if (this.#fetchHandler instanceof DefaultFetchHandler) {
// Fast path: pass options directly, skip Reflect.set/get round-trip
Reflect.set(request, appSymbol, this);
response = await this.#fetchHandler.renderWithOptions(request, resolvedOptions);
} else {
// User-provided fetch handler: stamp options + app on the request
setRenderOptions(request, resolvedOptions);
Reflect.set(request, appSymbol, this);
response = await this.#fetchHandler.fetch(request);
}
} catch (err: any) {
// Multi-level encoding (e.g., %2561 → %61) is rejected during URL
// normalization in FetchState. Return 400 without rendering an error page.
if (err instanceof MultiLevelEncodingError) {
return new Response('Bad Request', { status: 400 });
}
throw err;
if (this.#fetchHandler instanceof DefaultFetchHandler) {
// Fast path: pass options directly, skip Reflect.set/get round-trip
Reflect.set(request, appSymbol, this);
response = await this.#fetchHandler.renderWithOptions(request, resolvedOptions);
} else {
// User-provided fetch handler: stamp options + app on the request
setRenderOptions(request, resolvedOptions);
Reflect.set(request, appSymbol, this);
response = await this.#fetchHandler.fetch(request);
}
this.#warnMissingFeatures();
if (response.headers.get(ASTRO_ERROR_HEADER)) {
Expand Down
9 changes: 5 additions & 4 deletions packages/astro/src/core/fetch/fetch-state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import { getParams, getProps } from '../render/index.js';
import { Rewrites } from '../rewrites/handler.js';
import { isRoute404or500, isRouteServerIsland } from '../routing/match.js';
import { normalizeUrl } from '../util/normalized-url.js';
import { validateAndDecodePathname } from '../util/pathname.js';
import { getOriginPathname, setOriginPathname } from '../routing/rewrite.js';
import { computePathnameFromDomain } from '../i18n/domain.js';
import { getCustom404Route, routeHasHtmlExtension } from '../routing/helpers.js';
Expand Down Expand Up @@ -839,9 +840,9 @@ export class FetchState implements AstroFetchState {
return;
}

// this.pathname is already decoded by #computePathname, so no
// additional decodeURI here — that would double-decode and allow
// double-encoded paths like /%2561dmin to bypass route checks.
// this.pathname is already fully decoded by #computePathname
// (which iteratively decodes all encoding levels), so no
// additional decoding is needed here.
const matched = pipeline.matchRoute(this.pathname);
// In production SSR, prerendered routes are served as static files
// by the hosting layer and should not be rendered by the app.
Expand Down Expand Up @@ -899,7 +900,7 @@ export class FetchState implements AstroFetchState {
}
pathname = prependForwardSlash(pathname);
try {
return decodeURI(pathname);
return validateAndDecodePathname(pathname);
} catch (e: any) {
this.pipeline.logger.error(null, e.toString());
return pathname;
Expand Down
11 changes: 3 additions & 8 deletions packages/astro/src/core/util/normalized-url.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { collapseDuplicateSlashes } from '@astrojs/internal-helpers/path';
import { MultiLevelEncodingError, validateAndDecodePathname } from './pathname.js';
import { validateAndDecodePathname } from './pathname.js';

/**
* Creates a normalized URL from a request URL string.
Expand All @@ -16,13 +16,8 @@ export function createNormalizedUrl(requestUrl: string): URL {
export function normalizeUrl(url: URL): URL {
try {
url.pathname = validateAndDecodePathname(url.pathname);
} catch (e) {
// Multi-level encoding (e.g., %2561 → %61) must be rejected, not silently decoded.
// Let this error propagate so the caller can return a 400 response.
if (e instanceof MultiLevelEncodingError) {
throw e;
}
// For other decoding failures (truly malformed URLs), fall back gracefully.
} catch {
// For decoding failures (truly malformed URLs), fall back gracefully.
try {
url.pathname = decodeURI(url.pathname);
} catch {
Expand Down
52 changes: 31 additions & 21 deletions packages/astro/src/core/util/pathname.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@
* Error thrown when multi-level URL encoding is detected in a pathname.
* This is a distinct error type so callers can handle it specifically
* (e.g., returning a 400 response) rather than falling back to partial decoding.
*
* @deprecated No longer thrown internally — multi-level encoding is now
* decoded iteratively instead of rejected. Kept for backwards compatibility
* in case third-party code references the class.
*/
export class MultiLevelEncodingError extends Error {
constructor() {
Expand All @@ -10,37 +14,43 @@ export class MultiLevelEncodingError extends Error {
}
}

const ENCODING_REGEX = /%25[0-9a-fA-F]{2}/;

/**
* Validates that a pathname is not multi-level encoded.
* Detects if a pathname contains encoding that was encoded again (e.g., %2561dmin where %25 decodes to %).
* This prevents double/triple encoding bypasses of security checks.
* Decodes a pathname iteratively until stable, collapsing all levels of
* percent-encoding into a single canonical form. This prevents
* double/triple encoding from bypassing middleware authorization checks
* (CVE-2025-66202) — instead of rejecting multi-level encoding, we
* fully resolve it so middleware always sees the true decoded path.
*
* @param pathname - The pathname to validate
* @returns The decoded pathname if valid
* @throws MultiLevelEncodingError if multi-level encoding is detected
* @throws Error if the pathname contains invalid URL encoding
* @param pathname - The pathname to decode
* @returns The fully decoded pathname
* @throws Error if the pathname contains invalid URL encoding that
* cannot be decoded at all (e.g., a bare `%` not followed by hex digits)
*/
export function validateAndDecodePathname(pathname: string): string {
// %25 (encoded %) followed by two hex digits is the signature of double-encoding.
// Example: %2561 is %25 + 61, which decodes to %61, then to 'a'.
// This is ambiguous with a literal "%" followed by hex characters (e.g. a file
// named "%AB"), but rejecting it is the secure default — the alternative allows
// middleware auth bypasses.
if (ENCODING_REGEX.test(pathname)) {
throw new MultiLevelEncodingError();
}
let decoded: string;
try {
decoded = decodeURI(pathname);
} catch (_e) {
throw new Error('Invalid URL encoding');
}
// Defense-in-depth: catch creative encodings that reassemble
// into %25HH after the first decode pass
if (ENCODING_REGEX.test(decoded)) {
throw new MultiLevelEncodingError();
// Iteratively decode until stable. Multi-level encoding (e.g.,
// %2561 → %61 → a) is resolved completely so that downstream code
// — especially middleware auth checks — always sees the canonical
// pathname regardless of how many encoding layers the client used.
// We cap iterations to prevent infinite loops on pathological input.
let iterations = 0;
while (decoded !== pathname && iterations < 10) {
pathname = decoded;
try {
decoded = decodeURI(pathname);
} catch {
// decodeURI can fail when a decoded literal '%' forms an
// invalid sequence with adjacent characters (e.g., '%?.pdf'
// after decoding %25%3F). This is fine — we've decoded as
// far as possible.
break;
}
iterations++;
}
return decoded;
}
3 changes: 2 additions & 1 deletion packages/astro/src/runtime/server/render/head.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,8 @@ export function renderAllHeadContent(result: SSRResult) {
// consist of CSS modules which should naturally take precedence over CSS styles, so the
// order will still work. In prod, all CSS are stylesheet links.
// In the future, it may be better to have only an array of head elements to avoid these assumptions.
content += styles.join('\n') + links.join('\n') + scripts.join('\n');
const sep = result.compressHTML === true || result.compressHTML === 'jsx' ? '' : '\n';
content += styles.join(sep) + links.join(sep) + scripts.join(sep);

content += result._metadata.extraHead.join('');

Expand Down
19 changes: 15 additions & 4 deletions packages/astro/test/middleware.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,16 +120,27 @@ describe('Middleware API in PROD mode, SSR', () => {
});

describe('Path encoding in middleware', () => {
it('should reject double-encoded paths with 400', async () => {
it('middleware protects double-encoded /admin path', async () => {
// %2561dmin is decoded iteratively: %2561 → %61 → a → admin
// Middleware sees /admin and redirects (no auth token).
const request = new Request('http://example.com/%2561dmin');
const response = await app.render(request);
assert.equal(response.status, 400);
assert.equal(
response.status,
302,
'double-encoded /admin should trigger middleware redirect',
);
});

it('should reject triple-encoded paths with 400', async () => {
it('middleware protects triple-encoded /admin path', async () => {
// %252561dmin → %2561dmin → %61dmin → admin
const request = new Request('http://example.com/%252561dmin');
const response = await app.render(request);
assert.equal(response.status, 400);
assert.equal(
response.status,
302,
'triple-encoded /admin should trigger middleware redirect',
);
});
});

Expand Down
62 changes: 37 additions & 25 deletions packages/astro/test/units/app/double-encoding-bypass.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,9 @@ import { createManifest, createRouteInfo } from './test-helpers.ts';
/**
* Tests that double-URL-encoded paths do not bypass middleware authorization.
*
* When a path like /api/%2561dmin/users is received, validateAndDecodePathname
* detects the multi-level encoding and must reject the request rather than
* silently falling back to a single decodeURI() that leaves middleware
* seeing a half-decoded pathname (/api/%61dmin/users) that doesn't match
* its authorization checks.
* Multi-level encoding is decoded iteratively so middleware always sees the
* canonical path. For example, /api/%2561dmin/users is decoded to
* /api/admin/users, which the auth middleware correctly blocks with 401.
*/

const routeOptions: Parameters<typeof parseRoute>[1] = {
Expand Down Expand Up @@ -109,25 +107,27 @@ describe('URL normalization: double-encoding middleware bypass', () => {
);
});

it('rejects double-encoded /api/%2561dmin/users with 400', async () => {
it('middleware blocks double-encoded /api/%2561dmin/users (iteratively decoded)', async () => {
// Double-encoded: %2561 → %61 → a. Middleware sees /api/admin/users
// and correctly returns 401 Unauthorized.
const app = createApp(createAuthMiddleware());
const request = new Request('http://example.com/api/%2561dmin/users');
const response = await app.render(request);
assert.equal(
response.status,
400,
'/api/%2561dmin/users should be rejected as a bad request, not served',
401,
'/api/%2561dmin/users should be blocked by middleware (decoded to /api/admin/users)',
);
});

it('rejects double-encoded paths with multiple encoded segments', async () => {
it('middleware blocks double-encoded paths with multiple encoded segments', async () => {
const app = createApp(createAuthMiddleware());
const request = new Request('http://example.com/api/%2561dmin/%75sers');
const response = await app.render(request);
assert.equal(
response.status,
400,
'/api/%2561dmin/%75sers should be rejected as a bad request',
401,
'/api/%2561dmin/%75sers should be blocked by middleware (decoded to /api/admin/users)',
);
});

Expand Down Expand Up @@ -161,10 +161,6 @@ describe('URL normalization: double-encoding middleware bypass', () => {
// #region False-positive regression tests (issue #16781)

it('accepts encodeURIComponent output with a literal % next to a reserved char', async () => {
// encodeURIComponent('%?.pdf') → '%25%3F.pdf'. After decodeURI, %25 → %
// and %3F stays (reserved), yielding '%%3F.pdf'. The pre-decode pattern
// %25%3F is not multi-level encoding (the byte after %25 is `%`, not hex),
// so it must reach the handler instead of being rejected as a bad request.
const app = createApp(createAuthMiddleware());
const filename = encodeURIComponent('%?.pdf');
const request = new Request(`http://example.com/api/uploads/${filename}`);
Expand Down Expand Up @@ -193,30 +189,46 @@ describe('URL normalization: double-encoding middleware bypass', () => {
});

// #endregion
// #region Defense-in-depth: creative triple-encoding
// #region Double-encoded non-admin paths (issue #16960 — Sanity Studio)
// These should be decoded and served, not rejected.

it('serves double-encoded non-admin API routes', async () => {
const app = createApp(createAuthMiddleware());
// %255B → %5B → [, %255D → %5D → ]
const request = new Request('http://example.com/api/sections%255B_key%255D');
const response = await app.render(request);
assert.equal(response.status, 200, '/api/sections%255B_key%255D should be accessible');
const body = await response.json();
assert.equal(body.path, 'sections[_key]');
});

// #endregion
// #region Creative triple-encoding (decoded, middleware still catches attacks)

it('rejects creative triple-encoding where hex digits in %25XX are themselves encoded', async () => {
// %25%32%3561dmin: %25 → %, %32 → 2, %35 → 5 → decoded = %2561dmin
// Post-decode regex catches the reassembled %2561 signature
it('middleware blocks creative triple-encoding that decodes to /api/admin', async () => {
// %25%32%3561dmin: %25 → %, %32 → 2, %35 → 5 → pass 1 = %2561dmin
// → pass 2 = %61dmin → pass 3 = admin
// Middleware sees /api/admin and blocks it.
const app = createApp(createAuthMiddleware());
const request = new Request('http://example.com/api/%25%32%3561dmin/users');
const response = await app.render(request);
assert.equal(
response.status,
400,
'creative triple-encoding %25%32%3561dmin should be rejected',
401,
'creative triple-encoding %25%32%3561dmin should be blocked by middleware',
);
});

it('rejects creative encoding that reassembles %2525 after one decode', async () => {
// %25%32%35%32%35 → decodeURI → %2525 → post-decode regex catches %2525
it('serves creative encoding that decodes to a non-admin path', async () => {
// %25%32%35%32%35 → decodeURI → %2525 → %25 → %
// This decodes to just "%", which is a non-admin path.
const app = createApp(createAuthMiddleware());
const request = new Request('http://example.com/api/%25%32%35%32%35');
const response = await app.render(request);
assert.equal(
response.status,
400,
'creative encoding that reassembles %2525 should be rejected',
200,
'creative encoding that decodes to a non-admin path should be served',
);
});
// #endregion
Expand Down
Loading
Loading