Respect path-segment boundaries when stripping the configured base

.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.

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';
@@ -281,14 +281,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);
 	}
 
 	/**

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';
@@ -1016,18 +1015,12 @@ export class FetchState implements AstroFetchState {
 	 * Strips the manifest'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.manifest.base;
-		if (pathname.startsWith(base)) {
-			const baseWithoutTrailingSlash = removeTrailingForwardSlash(base);
-			pathname = pathname.slice(baseWithoutTrailingSlash.length + 1);
-		}
-		return prependForwardSlash(pathname);
+		return prependForwardSlash(stripRequestBase(normalizedPathname, this.manifest.base));
 	}
 
 	/**

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;
-}

packages/astro/src/core/routing/match-request.ts

@@ -1,30 +1,11 @@
-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';
 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;

packages/astro/test/units/app/base-prefix-boundary.test.ts

@@ -0,0 +1,146 @@
+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<typeof parseRoute>[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`<h1>Admin Panel</h1>`;
+});
+
+const publicPage = createComponent((_result: any, _props: any, _slots: any) => {
+	return render`<h1>Public</h1>`;
+});
+
+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<typeof createAuthMiddleware>) {
+	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',
+		);
+	});
+
+	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',
+		);
+	});
+});

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) {