diff --git a/packages/app/src/cli/services/dev/extension/server/middlewares.test.ts b/packages/app/src/cli/services/dev/extension/server/middlewares.test.ts index 206d63ed971..01a7bcce6a9 100644 --- a/packages/app/src/cli/services/dev/extension/server/middlewares.test.ts +++ b/packages/app/src/cli/services/dev/extension/server/middlewares.test.ts @@ -6,6 +6,7 @@ import { noCacheMiddleware, redirectToDevConsoleMiddleware, getExtensionPointMiddleware, + devConsoleAssetsMiddleware, } from './middlewares.js' import * as utilities from './utilities.js' import {GetExtensionsMiddlewareOptions} from './models.js' @@ -16,6 +17,7 @@ import {testUIExtension} from '../../../../models/app/app.test-data.js' import {AppEventWatcher} from '../../app-events/app-event-watcher.js' import {copyConfigKeyEntry} from '../../../build/steps/include-assets/copy-config-key-entry.js' import {describe, expect, vi, test} from 'vitest' +import * as file from '@shopify/cli-kit/node/fs' import {inTemporaryDirectory, mkdir, touchFile, writeFile} from '@shopify/cli-kit/node/fs' import * as h3 from 'h3' import {joinPath} from '@shopify/cli-kit/node/path' @@ -846,3 +848,29 @@ describe('getExtensionPointMiddleware()', () => { expect(h3.sendRedirect).toHaveBeenCalledWith(event, 'http://www.mock.com/redirect/url', 307) }) }) + +describe('devConsoleAssetsMiddleware()', () => { + test('returns 404 for path traversal attempts', async () => { + await inTemporaryDirectory(async (tmpDir: string) => { + vi.spyOn(utilities, 'sendError').mockImplementation(() => {}) + const rootDirectory = joinPath(tmpDir, 'assets') + await mkdir(rootDirectory) + + // Mock findPathUp to return our temp rootDirectory + vi.spyOn(file, 'findPathUp').mockResolvedValue(rootDirectory) + + const event = getMockEvent({ + params: { + assetPath: '../secret.txt', + }, + }) + + await devConsoleAssetsMiddleware(event) + + expect(utilities.sendError).toHaveBeenCalledWith(event, { + statusCode: 404, + statusMessage: 'Not Found', + }) + }) + }) +}) diff --git a/packages/app/src/cli/services/dev/extension/server/middlewares.ts b/packages/app/src/cli/services/dev/extension/server/middlewares.ts index 1f1789662a3..99f72396b79 100644 --- a/packages/app/src/cli/services/dev/extension/server/middlewares.ts +++ b/packages/app/src/cli/services/dev/extension/server/middlewares.ts @@ -146,8 +146,15 @@ export const devConsoleAssetsMiddleware = defineEventHandler(async (event) => { }) } + const normalizedRootDirectory = resolvePath(rootDirectory) + const candidate = resolvePath(joinPath(normalizedRootDirectory, assetPath)) + + if (!isSubpath(normalizedRootDirectory, candidate)) { + return sendError(event, {statusCode: 404, statusMessage: 'Not Found'}) + } + return fileServerMiddleware(event, { - filePath: joinPath(rootDirectory, assetPath), + filePath: candidate, }) }) diff --git a/packages/cli-kit/src/public/node/http.ts b/packages/cli-kit/src/public/node/http.ts index 8cbc8556a57..eb7a4ba6fa8 100644 --- a/packages/cli-kit/src/public/node/http.ts +++ b/packages/cli-kit/src/public/node/http.ts @@ -254,9 +254,22 @@ export function downloadFile(url: string, to: string): Promise { nodeFetch(url, {redirect: 'follow'}) .then((res) => { - res.body?.pipe(file) + if (!res.ok) { + file.destroy() + tryToRemoveFile() + reject(new Error(`Failed to download file from ${url}. Status: ${res.status} ${res.statusText}`)) + return + } + if (!res.body) { + file.destroy() + tryToRemoveFile() + reject(new Error(`Failed to download file from ${url}. No response body.`)) + return + } + res.body.pipe(file) }) .catch((err) => { + file.destroy() tryToRemoveFile() reject(err instanceof Error ? err : new Error(String(err))) })