From 8071dd2f3519e203a4ac8115e677dde97fa026a7 Mon Sep 17 00:00:00 2001 From: Matthew Phillips Date: Fri, 14 Aug 2026 13:56:50 -0400 Subject: [PATCH 1/2] Respect path-segment boundaries when stripping the configured base --- .changeset/nine-colts-add.md | 6 + packages/astro/src/core/app/base.ts | 11 +- packages/astro/src/core/fetch/fetch-state.ts | 17 +-- packages/astro/src/core/i18n/domain.ts | 19 +-- .../units/app/base-prefix-boundary.test.ts | 126 ++++++++++++++++++ packages/internal-helpers/src/path.ts | 22 +++ 6 files changed, 164 insertions(+), 37 deletions(-) create mode 100644 .changeset/nine-colts-add.md create mode 100644 packages/astro/test/units/app/base-prefix-boundary.test.ts diff --git a/.changeset/nine-colts-add.md b/.changeset/nine-colts-add.md new file mode 100644 index 000000000000..c93fb193a4c2 --- /dev/null +++ b/.changeset/nine-colts-add.md @@ -0,0 +1,6 @@ +--- +'astro': patch +'@astrojs/internal-helpers': patch +--- + +Fixes base path stripping to respect path-segment boundaries. With a configured `base` such as `/docs`, a request like `/docs-archive/page` is no longer treated as being under the base, so routing and `context.url.pathname` now agree on the same pathname. diff --git a/packages/astro/src/core/app/base.ts b/packages/astro/src/core/app/base.ts index 2bf4695bd954..a98dee624dd7 100644 --- a/packages/astro/src/core/app/base.ts +++ b/packages/astro/src/core/app/base.ts @@ -1,7 +1,7 @@ import { - collapseDuplicateLeadingSlashes, prependForwardSlash, removeTrailingForwardSlash, + stripRequestBase, } from '@astrojs/internal-helpers/path'; import { matchPattern } from '@astrojs/internal-helpers/remote'; import { computePathnameFromDomain } from '../i18n/domain.js'; @@ -249,14 +249,7 @@ export abstract class BaseApp

{ } public removeBase(pathname: string) { - // Collapse multiple leading slashes to prevent middleware authorization bypass. - // Without this, `//admin` would be treated as starting with base `/` and sliced - // to `/admin` for routing, while middleware still sees `//admin` in the URL. - pathname = collapseDuplicateLeadingSlashes(pathname); - if (pathname.startsWith(this.manifest.base)) { - return pathname.slice(this.baseWithoutTrailingSlash.length + 1); - } - return pathname; + return stripRequestBase(pathname, this.manifest.base); } /** diff --git a/packages/astro/src/core/fetch/fetch-state.ts b/packages/astro/src/core/fetch/fetch-state.ts index a04950ff4bb4..f5283e8f6a92 100644 --- a/packages/astro/src/core/fetch/fetch-state.ts +++ b/packages/astro/src/core/fetch/fetch-state.ts @@ -1,9 +1,8 @@ import colors from 'piccolore'; import { - collapseDuplicateLeadingSlashes, collapseDuplicateSlashes, prependForwardSlash, - removeTrailingForwardSlash, + stripRequestBase, } from '@astrojs/internal-helpers/path'; import { deserializeActionResult } from '../../actions/runtime/client.js'; import { createCallAction, createGetActionResult, hasActionPayload } from '../../actions/utils.js'; @@ -965,18 +964,12 @@ export class FetchState implements AstroFetchState { * Strips the pipeline's base from a normalized request pathname and prepends * a forward slash. * - * Mirrors `BaseApp.removeBase`, including the - * `collapseDuplicateLeadingSlashes` fix that prevents middleware - * authorization bypass when the URL starts with `//`. + * Mirrors `BaseApp.removeBase`: the router matches against this stripped path + * while middleware reads the un-stripped `context.url.pathname`, so both must + * strip the base identically. */ #computePathname(normalizedPathname: string): string { - let pathname = collapseDuplicateLeadingSlashes(normalizedPathname); - const base = this.pipeline.manifest.base; - if (pathname.startsWith(base)) { - const baseWithoutTrailingSlash = removeTrailingForwardSlash(base); - pathname = pathname.slice(baseWithoutTrailingSlash.length + 1); - } - return prependForwardSlash(pathname); + return prependForwardSlash(stripRequestBase(normalizedPathname, this.pipeline.manifest.base)); } /** diff --git a/packages/astro/src/core/i18n/domain.ts b/packages/astro/src/core/i18n/domain.ts index b04a27d1e8c1..110d28012e7e 100644 --- a/packages/astro/src/core/i18n/domain.ts +++ b/packages/astro/src/core/i18n/domain.ts @@ -1,9 +1,9 @@ import { appendForwardSlash, - collapseDuplicateLeadingSlashes, joinPaths, prependForwardSlash, removeTrailingForwardSlash, + stripRequestBase, } from '@astrojs/internal-helpers/path'; import { normalizeTheLocale } from '../../i18n/path.js'; import type { SSRManifest } from '../app/types.js'; @@ -72,13 +72,13 @@ export function computePathnameFromDomain( } if (locale) { - const requestPathname = pathnameFromRequest ?? removeBase(url.pathname, base); + const requestPathname = pathnameFromRequest ?? stripRequestBase(url.pathname, base); pathname = prependForwardSlash(joinPaths(normalizeTheLocale(locale), requestPathname)); if (trailingSlash === 'always') { pathname = appendForwardSlash(pathname); } else if (trailingSlash === 'never') { pathname = removeTrailingForwardSlash(pathname); - } else if (requestPathname.endsWith('/')) { + } else if (url.pathname.endsWith('/')) { // trailingSlash === 'ignore': preserve the original trailing slash pathname = appendForwardSlash(pathname); } @@ -94,16 +94,3 @@ export function computePathnameFromDomain( } return pathname; } - -/** - * Mirror of `BaseApp.removeBase`, including the - * `collapseDuplicateLeadingSlashes` fix that prevents middleware - * authorization bypass when the URL starts with `//`. - */ -function removeBase(pathname: string, base: string): string { - pathname = collapseDuplicateLeadingSlashes(pathname); - if (pathname.startsWith(base)) { - return pathname.slice(removeTrailingForwardSlash(base).length + 1); - } - return pathname; -} diff --git a/packages/astro/test/units/app/base-prefix-boundary.test.ts b/packages/astro/test/units/app/base-prefix-boundary.test.ts new file mode 100644 index 000000000000..ef82ab78ba7a --- /dev/null +++ b/packages/astro/test/units/app/base-prefix-boundary.test.ts @@ -0,0 +1,126 @@ +import assert from 'node:assert/strict'; +import { describe, it } from 'node:test'; +import type { MiddlewareHandler } from '../../../dist/types/public/common.js'; +import { App } from '../../../dist/core/app/app.js'; +import { parseRoute } from '../../../dist/core/routing/parse-route.js'; +import { createComponent, render } from '../../../dist/runtime/server/index.js'; +import { createManifest, createRouteInfo } from './test-helpers.ts'; + +/** + * Tests that base stripping respects path-segment boundaries. + * + * With `base: '/app'`, only `/app` and paths under `/app/` belong to the base. + * Stripping the base with a plain prefix check treats `/appX/admin` as under the + * base and slices it to `/admin`, so the router matches `/admin` while + * `context.url.pathname` still reads `/appX/admin`. Middleware keyed on the + * base-prefixed pathname then sees a path it does not recognize. + */ + +const routeOptions: Parameters[1] = { + config: { base: '/app', trailingSlash: 'ignore' }, + pageExtensions: [], +} as any; + +const adminRouteData = parseRoute('admin', routeOptions, { + component: 'src/pages/admin.astro', +}); + +const publicRouteData = parseRoute('index.astro', routeOptions, { + component: 'src/pages/index.astro', +}); + +const adminPage = createComponent((_result: any, _props: any, _slots: any) => { + return render`

Admin Panel

`; +}); + +const publicPage = createComponent((_result: any, _props: any, _slots: any) => { + return render`

Public

`; +}); + +const pageMap = new Map([ + [ + adminRouteData.component, + async () => ({ + page: async () => ({ + default: adminPage, + }), + }), + ], + [ + publicRouteData.component, + async () => ({ + page: async () => ({ + default: publicPage, + }), + }), + ], +]); + +/** + * Middleware that blocks access to the base-prefixed `/app/admin` route, + * as recommended in the official Astro authentication docs. + */ +function createAuthMiddleware() { + return (async () => ({ + onRequest: (async (context, next) => { + const pathname = context.url.pathname; + if (pathname === '/app/admin' || pathname.startsWith('/app/admin/')) { + return new Response('Forbidden', { status: 403 }); + } + return next(); + }) satisfies MiddlewareHandler, + })) as () => Promise<{ onRequest: MiddlewareHandler }>; +} + +function createApp(middleware: ReturnType) { + return new App( + createManifest({ + base: '/app', + routes: [createRouteInfo(adminRouteData), createRouteInfo(publicRouteData)], + pageMap: pageMap as any, + middleware: middleware as any, + }) as any, + ); +} + +describe('base stripping respects path-segment boundaries', () => { + it('middleware blocks /app/admin with normal request', async () => { + const app = createApp(createAuthMiddleware()); + const request = new Request('http://example.com/app/admin'); + const response = await app.render(request); + assert.equal(response.status, 403, '/app/admin should be blocked by middleware'); + }); + + it('middleware blocks /appX/admin (single-character prefix extension)', async () => { + const app = createApp(createAuthMiddleware()); + const request = new Request('http://example.com/appX/admin'); + const response = await app.render(request); + assert.notEqual( + response.status, + 200, + '/appX/admin should not resolve to the protected /admin route', + ); + }); + + it('middleware blocks /app2/admin (digit prefix extension)', async () => { + const app = createApp(createAuthMiddleware()); + const request = new Request('http://example.com/app2/admin'); + const response = await app.render(request); + assert.notEqual( + response.status, + 200, + '/app2/admin should not resolve to the protected /admin route', + ); + }); + + it('middleware blocks /app-/admin (punctuation prefix extension)', async () => { + const app = createApp(createAuthMiddleware()); + const request = new Request('http://example.com/app-/admin'); + const response = await app.render(request); + assert.notEqual( + response.status, + 200, + '/app-/admin should not resolve to the protected /admin route', + ); + }); +}); diff --git a/packages/internal-helpers/src/path.ts b/packages/internal-helpers/src/path.ts index 9b7a82923181..368c999f470b 100644 --- a/packages/internal-helpers/src/path.ts +++ b/packages/internal-helpers/src/path.ts @@ -262,6 +262,28 @@ export function removeBase(path: string, base: string) { return path; } +/** + * Strips a configured request `base` from a pathname, only when the pathname is + * the base itself or lies under it at a path-segment boundary. `stripRequestBase('/appX/admin', '/app')` + * leaves the pathname untouched, whereas `stripRequestBase('/app/admin', '/app')` returns `/admin`. + * + * Leading slashes are collapsed first so `//admin` cannot pose as being under a + * `/` base. Route matching and `context.url.pathname` derive from separate + * copies of the request path, so a mismatch here lets a request reach a route + * whose base-prefixed pathname middleware never observes. + */ +export function stripRequestBase(pathname: string, base: string): string { + pathname = collapseDuplicateLeadingSlashes(pathname); + const baseWithoutTrailingSlash = removeTrailingForwardSlash(base); + if (pathname === baseWithoutTrailingSlash) { + return '/'; + } + if (pathname.startsWith(baseWithoutTrailingSlash + '/')) { + return pathname.slice(baseWithoutTrailingSlash.length); + } + return pathname; +} + const WITH_FILE_EXT = /\/[^/]+\.\w+$/; export function hasFileExtension(path: string) { From 4c3e06890b5ce97834c57697911769c451219837 Mon Sep 17 00:00:00 2001 From: Matthew Phillips Date: Tue, 18 Aug 2026 10:04:43 -0400 Subject: [PATCH 2/2] Use stripRequestBase in matchRequest and cover the app.match path --- .../astro/src/core/routing/match-request.ts | 23 ++----------------- .../units/app/base-prefix-boundary.test.ts | 20 ++++++++++++++++ 2 files changed, 22 insertions(+), 21 deletions(-) diff --git a/packages/astro/src/core/routing/match-request.ts b/packages/astro/src/core/routing/match-request.ts index de63c8d5b67b..2ad59fc97bfa 100644 --- a/packages/astro/src/core/routing/match-request.ts +++ b/packages/astro/src/core/routing/match-request.ts @@ -1,8 +1,4 @@ -import { - collapseDuplicateLeadingSlashes, - prependForwardSlash, - removeTrailingForwardSlash, -} from '@astrojs/internal-helpers/path'; +import { prependForwardSlash, stripRequestBase } from '@astrojs/internal-helpers/path'; import type { RouteData } from '../../types/public/internal.js'; import type { SSRManifest } from '../app/types.js'; import { computePathnameFromDomain } from '../i18n/domain.js'; @@ -10,21 +6,6 @@ import { AstroIntegrationLogger } from '../logger/core.js'; import { getLogger } from '../logger/manifest-logger.js'; import { matchAllRoutes, matchRoute } from './route-table.js'; -/** - * Strips the manifest base from a pathname (the pure counterpart of - * `BaseApp.removeBase`). Collapses multiple leading slashes first to prevent - * middleware authorization bypass: without this, `//admin` would be treated as - * starting with base `/` and sliced to `/admin` for routing, while middleware - * still sees `//admin` in the URL. - */ -function removeBase(manifest: SSRManifest, pathname: string): string { - pathname = collapseDuplicateLeadingSlashes(pathname); - if (pathname.startsWith(manifest.base)) { - return pathname.slice(removeTrailingForwardSlash(manifest.base).length + 1); - } - return pathname; -} - /** * Decodes a pathname with `decodeURI`, falling back to the raw pathname when it * contains an invalid percent-sequence (e.g. `%C0%AF`, an overlong-UTF-8 encoding of @@ -73,7 +54,7 @@ export function matchRequest( getLogger(manifest), ); if (!pathname) { - pathname = prependForwardSlash(removeBase(manifest, url.pathname)); + pathname = prependForwardSlash(stripRequestBase(url.pathname, manifest.base)); } const routeData = matchRoute(manifest, safeDecodeURI(manifest, pathname)); if (!routeData) return undefined; diff --git a/packages/astro/test/units/app/base-prefix-boundary.test.ts b/packages/astro/test/units/app/base-prefix-boundary.test.ts index ef82ab78ba7a..df01b9720f51 100644 --- a/packages/astro/test/units/app/base-prefix-boundary.test.ts +++ b/packages/astro/test/units/app/base-prefix-boundary.test.ts @@ -123,4 +123,24 @@ describe('base stripping respects path-segment boundaries', () => { '/app-/admin should not resolve to the protected /admin route', ); }); + + it('app.match does not resolve /appX/admin to the protected route', () => { + const app = createApp(createAuthMiddleware()); + const request = new Request('http://example.com/appX/admin'); + const routeData = app.match(request); + assert.equal(routeData, undefined, '/appX/admin should not match a route under the base'); + }); + + it('middleware blocks /appX/admin when the adapter supplies routeData', async () => { + const app = createApp(createAuthMiddleware()); + const request = new Request('http://example.com/appX/admin'); + // Adapters may call app.match() and thread the result back into render(). + const routeData = app.match(request); + const response = await app.render(request, { routeData }); + assert.notEqual( + response.status, + 200, + '/appX/admin should not render the protected /admin route via the routeData render path', + ); + }); });