From 4a1edcdc516cc7e21984919755be77463e1ace62 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 21 Sep 2026 00:42:54 +0000 Subject: [PATCH] [Security] Contain theme file operations within the theme directory Theme file keys originate from remote API responses, but were joined onto the theme root without verifying the result stayed inside it. Resolve each key and reject any that escapes the theme directory before reading, writing, or removing the file. Co-Authored-By: Claude Opus 4.8 --- .../theme/src/cli/utilities/theme-fs.test.ts | 19 +++++++++++++ packages/theme/src/cli/utilities/theme-fs.ts | 27 ++++++++++++++++--- 2 files changed, 42 insertions(+), 4 deletions(-) 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) {