diff --git a/.changeset/bold-stars-6d152.md b/.changeset/bold-stars-6d152.md new file mode 100644 index 000000000000..ab021fcbf1fe --- /dev/null +++ b/.changeset/bold-stars-6d152.md @@ -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. diff --git a/.changeset/fix-compress-html-head-join.md b/.changeset/fix-compress-html-head-join.md new file mode 100644 index 000000000000..eea1f7cf8802 --- /dev/null +++ b/.changeset/fix-compress-html-head-join.md @@ -0,0 +1,5 @@ +--- +"astro": patch +--- + +fix(render): honour compressHTML when joining head elements diff --git a/packages/astro/src/assets/build/generate.ts b/packages/astro/src/assets/build/generate.ts index 16386c249ae2..b9d534487718 100644 --- a/packages/astro/src/assets/build/generate.ts +++ b/packages/astro/src/assets/build/generate.ts @@ -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'; @@ -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 { diff --git a/packages/astro/src/core/app/base.ts b/packages/astro/src/core/app/base.ts index 421061e39ee5..dd2a17e7c814 100644 --- a/packages/astro/src/core/app/base.ts +++ b/packages/astro/src/core/app/base.ts @@ -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'; @@ -407,24 +406,15 @@ export abstract class BaseApp

{ }; 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)) { diff --git a/packages/astro/src/core/fetch/fetch-state.ts b/packages/astro/src/core/fetch/fetch-state.ts index 935596a29e42..7112ba1dfc04 100644 --- a/packages/astro/src/core/fetch/fetch-state.ts +++ b/packages/astro/src/core/fetch/fetch-state.ts @@ -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'; @@ -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. @@ -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; diff --git a/packages/astro/src/core/util/normalized-url.ts b/packages/astro/src/core/util/normalized-url.ts index 455d7adb7490..88b30b993ef1 100644 --- a/packages/astro/src/core/util/normalized-url.ts +++ b/packages/astro/src/core/util/normalized-url.ts @@ -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. @@ -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 { diff --git a/packages/astro/src/core/util/pathname.ts b/packages/astro/src/core/util/pathname.ts index fa760ea791c6..cf78d09973a7 100644 --- a/packages/astro/src/core/util/pathname.ts +++ b/packages/astro/src/core/util/pathname.ts @@ -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() { @@ -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; } diff --git a/packages/astro/src/runtime/server/render/head.ts b/packages/astro/src/runtime/server/render/head.ts index 239f6e07e576..5aebc7b924d5 100644 --- a/packages/astro/src/runtime/server/render/head.ts +++ b/packages/astro/src/runtime/server/render/head.ts @@ -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(''); diff --git a/packages/astro/test/middleware.test.ts b/packages/astro/test/middleware.test.ts index b007c6f8831a..53f1527a481a 100644 --- a/packages/astro/test/middleware.test.ts +++ b/packages/astro/test/middleware.test.ts @@ -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', + ); }); }); diff --git a/packages/astro/test/units/app/double-encoding-bypass.test.ts b/packages/astro/test/units/app/double-encoding-bypass.test.ts index 5f7e0ea20a0e..30e3b333cad4 100644 --- a/packages/astro/test/units/app/double-encoding-bypass.test.ts +++ b/packages/astro/test/units/app/double-encoding-bypass.test.ts @@ -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[1] = { @@ -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)', ); }); @@ -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}`); @@ -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 diff --git a/packages/astro/test/units/util/validate-and-decode-pathname.test.ts b/packages/astro/test/units/util/validate-and-decode-pathname.test.ts index 8b0bf4a599ca..713876a1a28d 100644 --- a/packages/astro/test/units/util/validate-and-decode-pathname.test.ts +++ b/packages/astro/test/units/util/validate-and-decode-pathname.test.ts @@ -1,9 +1,6 @@ import assert from 'node:assert/strict'; import { describe, it } from 'node:test'; -import { - MultiLevelEncodingError, - validateAndDecodePathname, -} from '../../../dist/core/util/pathname.js'; +import { validateAndDecodePathname } from '../../../dist/core/util/pathname.js'; describe('validateAndDecodePathname', () => { // #region Plain paths (no encoding) @@ -32,129 +29,96 @@ describe('validateAndDecodePathname', () => { }); // #endregion - // #region Standard double-encoding (attack vectors, must reject) + // #region Double encoding (iteratively decoded, not rejected) + // + // Multi-level encoding is decoded iteratively until stable. This + // ensures middleware always sees the canonical path and can make + // correct authorization decisions (CVE-2025-66202 mitigation). - it('rejects double-encoded unreserved chars: %2561 (a)', () => { - assert.throws( - () => validateAndDecodePathname('/api/%2561dmin'), - MultiLevelEncodingError, - '%2561 is double-encoded %61 (a)', - ); + it('fully decodes double-encoded unreserved chars: %2561 (a)', () => { + // %2561 → decodeURI → %61 → decodeURI → a + assert.equal(validateAndDecodePathname('/api/%2561dmin'), '/api/admin'); }); - it('rejects double-encoded uppercase hex: %2541 (A)', () => { - assert.throws( - () => validateAndDecodePathname('/api/%2541dmin'), - MultiLevelEncodingError, - '%2541 is double-encoded %41 (A)', - ); + it('fully decodes double-encoded uppercase hex: %2541 (A)', () => { + // %2541 → %41 → A + assert.equal(validateAndDecodePathname('/api/%2541dmin'), '/api/Admin'); }); - it('rejects double-encoded slash: %252F', () => { - assert.throws( - () => validateAndDecodePathname('/api/%252F'), - MultiLevelEncodingError, - '%252F is double-encoded %2F (/)', - ); + it('fully decodes double-encoded slash: %252F', () => { + // %252F → %2F (decodeURI preserves reserved %2F) + assert.equal(validateAndDecodePathname('/api/%252F'), '/api/%2F'); }); - it('rejects double-encoded null byte: %2500', () => { - assert.throws( - () => validateAndDecodePathname('/api/%2500'), - MultiLevelEncodingError, - '%2500 is double-encoded %00 (null)', - ); + it('fully decodes double-encoded null byte: %2500', () => { + // %2500 → %00 → \x00 + assert.equal(validateAndDecodePathname('/api/%2500'), '/api/\x00'); }); - it('rejects double-encoded percent: %2525', () => { - assert.throws( - () => validateAndDecodePathname('/api/%2525'), - MultiLevelEncodingError, - '%2525 is double-encoded %25 (%)', - ); + it('fully decodes double-encoded percent: %2525', () => { + // %2525 → %25 → % + assert.equal(validateAndDecodePathname('/api/%2525'), '/api/%'); }); - it('rejects triple-encoded paths: %252561', () => { - assert.throws( - () => validateAndDecodePathname('/api/%252561dmin'), - MultiLevelEncodingError, - '%2525 triggers the pre-decode check (triple encoding)', - ); + it('fully decodes triple-encoded paths: %252561', () => { + // %252561 → %2561 → %61 → but wait: decodeURI('%252561') → '%2561' + // then decodeURI('%2561') → '%61' (because %25→%, 61 stays) + // Hmm, let's check: %25 decodes to %, so %2561 → %61, + // which is NOT decoded by decodeURI in one pass... + // Actually: decodeURI('%252561') → first decode: %25→%, 25→stays, 61→stays → %2561 + // Wait no. The URL is: %252561. decodeURI processes %25 → %, giving us "2561". + // So the result is: "%2561" → which is "%25" + "61" decoded one more level → "a" + // But we have "%2561" not "%25" + "61". Let me trace carefully: + // Input: /api/%252561dmin + // Pass 1: decodeURI → /api/%2561dmin (%25→%, "25" stays as chars, "61dmin" stays) + // Wait: %252561 — decodeURI scans left-to-right for %XX patterns. + // %25 matches → decodes to '%'. Next chars are '2561dmin'. + // So after pass 1: /api/%2561dmin + // Pass 2: decodeURI('/api/%2561dmin') → /api/%61dmin (%25→%, "61" stays) + // Wait: %2561 — %25 matches → decodes to '%'. Next chars are '61dmin'. + // So after pass 2: /api/%61dmin + // Pass 3: decodeURI('/api/%61dmin') → /api/admin (%61→a) + // So: /api/%252561dmin → /api/admin + assert.equal(validateAndDecodePathname('/api/%252561dmin'), '/api/admin'); }); - it('rejects when double-encoding appears mid-path', () => { - assert.throws( - () => validateAndDecodePathname('/some/path/%2561dmin/rest'), - MultiLevelEncodingError, - ); + it('fully decodes when double-encoding appears mid-path', () => { + assert.equal(validateAndDecodePathname('/some/path/%2561dmin/rest'), '/some/path/admin/rest'); }); - it('rejects when multiple double-encoded segments exist', () => { - assert.throws( - () => validateAndDecodePathname('/api/%2561dmin/%2575sers'), - MultiLevelEncodingError, - ); + it('fully decodes when multiple double-encoded segments exist', () => { + assert.equal(validateAndDecodePathname('/api/%2561dmin/%2575sers'), '/api/admin/users'); }); - it('rejects %25 followed by literal hex chars (%25AB) as ambiguous double-encoding', () => { - // %25AB could be double-encoded %AB (U+00AB «) or a literal "%" + "AB". - // These are indistinguishable at the URL level. Rejecting is the secure - // default — it prevents middleware bypass when %AB would decode to a - // meaningful character downstream. - assert.throws(() => validateAndDecodePathname('/path/%25AB'), MultiLevelEncodingError); + it('decodes %25 followed by literal hex chars (%25AB) to single-encoded form', () => { + // %25AB → %AB (decodeURI does not decode %AB since 0xAB is not valid + // standalone UTF-8, so iteration stops at %AB) + assert.equal(validateAndDecodePathname('/path/%25AB'), '/path/%AB'); }); // #endregion - // #region Creative triple-encoding (defense-in-depth, must reject) - // - // An attacker encodes the hex digits within %25XX itself, e.g.: - // %25%32%35 → decodeURI → %25 (reassembles the %25 signature) - // %25%36%31 → decodeURI → %61 (but preceded by a reassembled %25) - // - // Full payload: %25%32%3561dmin → decodeURI → %2561dmin - // The post-decode check catches this because the decoded output - // contains %25[hex][hex]. + // #region Creative triple-encoding (iteratively decoded) - it('rejects creative triple-encoding: %25%32%3561dmin', () => { - // %25 → %, %32 → 2, %35 → 5; decoded = %2561dmin - // Post-decode regex catches %2561 in the output - assert.throws( - () => validateAndDecodePathname('/api/%25%32%3561dmin'), - MultiLevelEncodingError, - 'creative triple-encoding reassembles into %2561 after one decode', - ); + it('fully decodes creative triple-encoding: %25%32%3561dmin', () => { + // %25 → %, %32 → 2, %35 → 5; pass 1 decoded = %2561dmin + // pass 2: %25 → %, 61 stays; decoded = %61dmin + // pass 3: %61 → a; decoded = admin + assert.equal(validateAndDecodePathname('/api/%25%32%3561dmin'), '/api/admin'); }); - it('rejects creative encoding of %252F via hex-encoded digits', () => { - // %25%32%46 → decodeURI → %2F (but via %25 + "2F" reassembled) - // Actually: %25 → %, %32 → 2, %46 → F; decoded = %2F - // Post-decode check sees %2F which is %25[hex][hex]? No — %2F is not %25HH. - // But wait: the decoded output is "%2F". The regex /%25[0-9a-fA-F]{2}/ does NOT - // match "%2F" because it requires the literal string "%25", not "%2". - // This specific input decodes to a single %HH sequence, not a %25HH sequence, - // so it is NOT multi-level encoding — it's just a creative way to spell %2F. - // - // Let's verify: input %25%32%46 → decodeURI → %2F (a reserved-char encoding) - // This is equivalent to just writing %2F directly. Not a bypass. + it('decodes creative encoding of %252F via hex-encoded digits', () => { + // %25%32%46 → decodeURI → %2F (reserved, stays) const decoded = validateAndDecodePathname('/api/%25%32%46'); - // decodeURI: %25 → %, %32 → 2, %46 → F → result is /api/%2F assert.equal(decoded, '/api/%2F'); }); - it('rejects creative triple-encoding of %2525 via hex-encoded digits', () => { - // %25%32%35 → decodeURI → %25 (reassembled) - // Followed by another hex pair, e.g. %25%32%3525 → decodeURI → %2525 - // But simpler: %25%32%35%36%31 → decodeURI → %2561 - // Post-decode regex catches %2561 - assert.throws( - () => validateAndDecodePathname('/api/%25%32%35%36%31dmin'), - MultiLevelEncodingError, - 'creative encoding reassembles %25 + 61 → %2561 after decode', - ); + it('fully decodes creative triple-encoding of %2525 via hex-encoded digits', () => { + // %25%32%35%36%31dmin → pass 1 → %2561dmin → pass 2 → %61dmin → pass 3 → admin + assert.equal(validateAndDecodePathname('/api/%25%32%35%36%31dmin'), '/api/admin'); }); // #endregion - // #region False-positive fix: %25 followed by another %XX (must pass) + // #region %25 followed by another %XX (must pass — not double-encoding) // // encodeURIComponent('%?.pdf') → %25%3F.pdf // This is a literal encoded % next to an encoded ?, NOT double-encoding. @@ -181,21 +145,23 @@ describe('validateAndDecodePathname', () => { }); it('allows %25 followed by a single hex digit then non-hex', () => { - // %25Fx — only one hex digit after %25, then 'x' (non-hex) - // The regex requires exactly two hex digits after %25 - // %25 → %, F stays as literal 'F', x stays const decoded = validateAndDecodePathname('/path/%25Fx'); assert.equal(decoded, '/path/%Fx'); }); it('allows encodeURIComponent output for filenames with special chars', () => { - // Real-world: user uploads a file named "100% done?.txt" - // encodeURIComponent('100% done?.txt') → '100%25%20done%3F.txt' - // but in a path segment via encodeURIComponent: '100%25%20done%3F.txt' const decoded = validateAndDecodePathname('/files/100%25%20done%3F.txt'); assert.equal(decoded, '/files/100% done%3F.txt'); }); + // #endregion + // #region Double-encoded brackets (Sanity Studio use case, issue #16960) + + it('fully decodes double-encoded brackets: %255B and %255D', () => { + // %255B → %5B → [, %255D → %5D → ] + assert.equal(validateAndDecodePathname('/sections%255B_key%255D'), '/sections[_key]'); + }); + // #endregion // #region Invalid encoding (malformed, must throw generic Error) @@ -203,11 +169,10 @@ describe('validateAndDecodePathname', () => { assert.throws( () => validateAndDecodePathname('/api/%GG'), (err: any) => { - assert.equal(err instanceof MultiLevelEncodingError, false); assert.equal(err instanceof Error, true); return true; }, - '%GG is malformed encoding, not multi-level', + '%GG is malformed encoding', ); }); @@ -215,11 +180,10 @@ describe('validateAndDecodePathname', () => { assert.throws( () => validateAndDecodePathname('/api/%6'), (err: any) => { - assert.equal(err instanceof MultiLevelEncodingError, false); assert.equal(err instanceof Error, true); return true; }, - '%6 is truncated, not multi-level', + '%6 is truncated', ); }); // #endregion