Return client errors for invalid image paths

.changeset/breezy-bobcats-run.md

@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Returns appropriate 400 and 404 responses from the image endpoint for invalid and missing local image paths

packages/astro/src/assets/endpoint/dev.ts

@@ -4,13 +4,13 @@ import { readFile } from 'node:fs/promises';
 import os from 'node:os';
 import { type AnymatchFn, isFileLoadingAllowed, type ResolvedConfig } from 'vite';
 import type { APIRoute } from '../../types/public/common.js';
-import { handleImageRequest, loadRemoteImage } from './shared.js';
+import { handleImageRequest, type LocalImageLoadResult, loadRemoteImage } from './shared.js';
 
 function replaceFileSystemReferences(src: string) {
 	return os.platform().includes('win32') ? src.replace(/^\/@fs\//, '') : src.replace(/^\/@fs/, '');
 }
 
-async function loadLocalImage(src: string, url: URL) {
+async function loadLocalImage(src: string, url: URL): Promise<LocalImageLoadResult> {
 	let returnValue: Buffer | undefined;
 	let fsPath: string | undefined;
 
@@ -55,12 +55,13 @@ async function loadLocalImage(src: string, url: URL) {
 		const sourceUrl = new URL(src, url.origin);
 		// This is only allowed if this is the same origin
 		if (sourceUrl.origin !== url.origin) {
-			return undefined;
+			return { kind: 'invalid-path' };
 		}
-		return loadRemoteImage(sourceUrl);
+		const buffer = await loadRemoteImage(sourceUrl);
+		return buffer ? { kind: 'loaded', buffer } : { kind: 'failed' };
 	}
 
-	return returnValue;
+	return returnValue ? { kind: 'loaded', buffer: returnValue } : { kind: 'failed' };
 }
 
 /**

packages/astro/src/assets/endpoint/node.ts

@@ -5,31 +5,44 @@ import path from 'node:path';
 import { fileURLToPath } from 'node:url';
 import { isParentDirectory } from '@astrojs/internal-helpers/path';
 import type { APIRoute } from '../../types/public/common.js';
-import { handleImageRequest } from './shared.js';
+import { handleImageRequest, type LocalImageLoadResult } from './shared.js';
 
-async function loadLocalImage(src: string, url: URL) {
+async function loadLocalImage(src: string, url: URL): Promise<LocalImageLoadResult> {
 	const outDirURL = resolveOutDir();
 	// If the _image segment isn't at the start of the path, we have a base
 	const idx = url.pathname.indexOf('/_image');
 	if (idx > 0) {
 		// Remove the base path
 		src = src.slice(idx);
 	}
-	if (!URL.canParse('.' + src, outDirURL)) {
-		return undefined;
-	}
-	const fileUrl = new URL('.' + src, outDirURL);
-	if (fileUrl.protocol !== 'file:') {
-		return undefined;
-	}
-	if (!isParentDirectory(fileURLToPath(outDirURL), fileURLToPath(fileUrl))) {
-		return undefined;
-	}
 
+	let fileUrl: URL;
 	try {
-		return await readFile(fileUrl);
+		if (!URL.canParse('.' + src, outDirURL)) {
+			return { kind: 'invalid-path' };
+		}
+		fileUrl = new URL('.' + src, outDirURL);
+		if (
+			fileUrl.protocol !== 'file:' ||
+			!isParentDirectory(fileURLToPath(outDirURL), fileURLToPath(fileUrl))
+		) {
+			return { kind: 'invalid-path' };
+		}
 	} catch {
-		return undefined;
+		return { kind: 'invalid-path' };
+	}
+
+	try {
+		return { kind: 'loaded', buffer: await readFile(fileUrl) };
+	} catch (error) {
+		if (
+			error instanceof Error &&
+			'code' in error &&
+			(error.code === 'ENOENT' || error.code === 'ENOTDIR' || error.code === 'EISDIR')
+		) {
+			return { kind: 'not-found' };
+		}
+		return { kind: 'failed' };
 	}
 }
 

packages/astro/src/assets/endpoint/shared.ts

@@ -33,13 +33,19 @@ export async function loadRemoteImage(src: URL): Promise<Buffer | undefined> {
 	}
 }
 
+export type LocalImageLoadResult =
+	| { kind: 'loaded'; buffer: Buffer }
+	| { kind: 'invalid-path' }
+	| { kind: 'not-found' }
+	| { kind: 'failed' };
+
 export const handleImageRequest = async ({
 	request,
 	loadLocalImage,
 	logger,
 }: {
 	request: Request;
-	loadLocalImage: (src: string, baseUrl: URL) => Promise<Buffer | undefined>;
+	loadLocalImage: (src: string, baseUrl: URL) => Promise<LocalImageLoadResult>;
 	logger: AstroRuntimeLogger;
 }) => {
 	const imageService = await getConfiguredImageService();
@@ -73,7 +79,17 @@ export const handleImageRequest = async ({
 
 		inputBuffer = await loadRemoteImage(new URL(transform.src));
 	} else {
-		inputBuffer = await loadLocalImage(removeQueryString(transform.src), url);
+		const result = await loadLocalImage(removeQueryString(transform.src), url);
+		switch (result.kind) {
+			case 'invalid-path':
+				return new Response('Invalid request', { status: 400 });
+			case 'not-found':
+				return new Response('Not Found', { status: 404 });
+			case 'failed':
+				return new Response('Internal Server Error', { status: 500 });
+			case 'loaded':
+				inputBuffer = result.buffer;
+		}
 	}
 
 	if (!inputBuffer) {

packages/astro/test/core-image.test.ts

@@ -1542,22 +1542,18 @@ describe('prod ssr', () => {
 			let response = await app.render(request);
 			const body = await response.text();
 
-			// Most paths are malformed local paths (500), but some backslash patterns
-			// are now correctly detected as remote and get 403
 			const { isRemotePath } = await import('@astrojs/internal-helpers/path');
-			const isDetectedAsRemote = isRemotePath(path);
-			const expectedStatus = isDetectedAsRemote ? 403 : 500;
-			const expectedBodyText = isDetectedAsRemote ? 'Forbidden' : 'Internal Server Error';
+			const expectedStatuses = isRemotePath(path) ? [403] : [400, 404, 500];
 
 			assert.equal(
-				response.status,
-				expectedStatus,
-				`Path "${path}" should return ${expectedStatus}`,
+				expectedStatuses.includes(response.status),
+				true,
+				`Path "${path}" should return ${expectedStatuses.join(' or ')}`,
 			);
 			assert.equal(
-				body.includes(expectedBodyText),
+				['Invalid request', 'Forbidden', 'Not Found', 'Internal Server Error'].includes(body),
 				true,
-				`Path "${path}" body should include "${expectedBodyText}"`,
+				`Path "${path}" should return an opaque error`,
 			);
 		}
 
@@ -1567,6 +1563,28 @@ describe('prod ssr', () => {
 		assert.equal(response.status, 200);
 	});
 
+	it('returns 400 for local paths with parent directory segments', async () => {
+		const app = await fixture.loadTestAdapterApp();
+		const paths = ['/../../../.env', '/foo/../../../.env', '/foo\\..\\..\\.env', '/%2e%2e/.env'];
+
+		for (const path of paths) {
+			const request = new Request('http://example.com/_image?href=' + encodeURIComponent(path));
+			const response = await app.render(request);
+
+			assert.equal(response.status, 400, `Path "${path}" should return 400`);
+			assert.equal(await response.text(), 'Invalid request');
+		}
+	});
+
+	it('returns 404 when a valid local image is not found', async () => {
+		const app = await fixture.loadTestAdapterApp();
+		const request = new Request('http://example.com/_image?href=/does-not-exist.png');
+		const response = await app.render(request);
+
+		assert.equal(response.status, 404);
+		assert.equal(await response.text(), 'Not Found');
+	});
+
 	it('prerendered routes images are built', async () => {
 		const html = await fixture.readFile('/client/prerender/index.html');
 		const $ = cheerio.load(html);