diff --git a/packages/theme/src/cli/utilities/theme-fs.test.ts b/packages/theme/src/cli/utilities/theme-fs.test.ts index f2ded99a372..1c730203022 100644 --- a/packages/theme/src/cli/utilities/theme-fs.test.ts +++ b/packages/theme/src/cli/utilities/theme-fs.test.ts @@ -27,6 +27,7 @@ import {Operation, type Checksum, type ThemeAsset} from '@shopify/cli-kit/node/t import {dirname, joinPath} from '@shopify/cli-kit/node/path' import {recordError} from '@shopify/cli-kit/node/analytics' import {AdminSession} from '@shopify/cli-kit/node/session' +import {AbortError} from '@shopify/cli-kit/node/error' import EventEmitter from 'events' import {fileURLToPath} from 'node:url' @@ -337,6 +338,24 @@ describe('theme-fs', () => { writeFileSpy.mockRestore() }) }) + + test('"write" rejects keys that resolve outside of the theme directory', async () => { + await inTemporaryDirectory(async (tmpDir) => { + // Given + const root = joinPath(tmpDir, 'theme') + await mkdir(root) + const themeFileSystem = mountThemeFileSystem(root) + await themeFileSystem.ready() + const escapingKey = '../escaped.liquid' + + // When + const writePromise = themeFileSystem.write({key: escapingKey, checksum: '1010', value: 'content'}) + + // Then + await expect(writePromise).rejects.toThrow(AbortError) + await expect(fileExists(joinPath(tmpDir, 'escaped.liquid'))).resolves.toBe(false) + }) + }) }) describe('themeFileSystem.read', async () => { diff --git a/packages/theme/src/cli/utilities/theme-fs.ts b/packages/theme/src/cli/utilities/theme-fs.ts index 4cc4e3c3f7e..94a5307dbc8 100644 --- a/packages/theme/src/cli/utilities/theme-fs.ts +++ b/packages/theme/src/cli/utilities/theme-fs.ts @@ -6,11 +6,12 @@ import {triggerBrowserFullReload} from './theme-environment/hot-reload/server.js import {getListingFilePath, updateSettingsDataForListing} from './theme-listing.js' import {DEFAULT_IGNORE_PATTERNS, timestampDateFormat} from '../constants.js' import {glob, readFile, ReadOptions, fileExists, mkdir, writeFile, removeFile} from '@shopify/cli-kit/node/fs' -import {joinPath, basename, relativePath} from '@shopify/cli-kit/node/path' +import {joinPath, basename, relativePath, resolvePath, isSubpath} from '@shopify/cli-kit/node/path' import {lookupMimeType, setMimeTypes} from '@shopify/cli-kit/node/mimes' import {outputContent, outputDebug, outputInfo, outputToken, outputWarn} from '@shopify/cli-kit/node/output' import {buildThemeAsset} from '@shopify/cli-kit/node/themes/factories' import {recordError} from '@shopify/cli-kit/node/analytics' +import {AbortError} from '@shopify/cli-kit/node/error' import {AdminSession} from '@shopify/cli-kit/node/session' import {bulkUploadThemeAssets, deleteThemeAssets} from '@shopify/cli-kit/node/themes/api' @@ -395,8 +396,26 @@ export function handleSyncUpdate( } } +/** + * Resolves a theme file key against the theme root, ensuring the result stays inside it. + * + * File keys come from the remote theme, so a key containing `..` segments would otherwise + * make `joinPath` resolve to a path outside the theme directory and let a remote response + * read or overwrite arbitrary files on the developer's machine. + */ +function resolveThemeFilePath(root: string, key: Key): string { + const absoluteRoot = resolvePath(root) + const absolutePath = resolvePath(absoluteRoot, key) + + if (!isSubpath(absoluteRoot, absolutePath)) { + throw new AbortError(`Theme file "${key}" is outside of the theme directory.`) + } + + return absolutePath +} + async function writeThemeFile(root: string, {key, attachment, value}: ThemeAsset) { - const absolutePath = joinPath(root, key) + const absolutePath = resolveThemeFilePath(root, key) await ensureDirExists(absolutePath) @@ -411,7 +430,7 @@ async function writeThemeFile(root: string, {key, attachment, value}: ThemeAsset export async function readThemeFile(root: string, path: Key): Promise { const options: ReadOptions = isTextFile(path) ? {encoding: 'utf8'} : {} - const absolutePath = joinPath(root, path) + const absolutePath = resolveThemeFilePath(root, path) const themeFileExists = await fileExists(absolutePath) if (!themeFileExists) { @@ -444,7 +463,7 @@ async function readThemeFileWithListing( } async function removeThemeFile(root: string, path: Key) { - const absolutePath = joinPath(root, path) + const absolutePath = resolveThemeFilePath(root, path) const themeFileExists = await fileExists(absolutePath) if (!themeFileExists) {