[Security] Prevent path traversal in dev console assets middleware

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',
+      })
+    })
+  })
+})

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,
   })
 })
 

packages/cli-kit/src/public/node/http.ts

@@ -254,9 +254,22 @@ export function downloadFile(url: string, to: string): Promise<string> {
 
       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)))
         })