From 5a7748d2585d8e57894cc46f61eacd27bc1577eb Mon Sep 17 00:00:00 2001 From: Callin Mullaney <57088-callinmullaney@users.noreply.drupalcode.org> Date: Fri, 12 Jun 2026 16:00:00 -0500 Subject: [PATCH 1/3] feat: add safe project path resolver --- src/util/fs/safeResolveWithin.test.ts | 53 +++++++++++++++++++++++++++ src/util/fs/safeResolveWithin.ts | 50 +++++++++++++++++++++++++ 2 files changed, 103 insertions(+) create mode 100644 src/util/fs/safeResolveWithin.test.ts create mode 100644 src/util/fs/safeResolveWithin.ts diff --git a/src/util/fs/safeResolveWithin.test.ts b/src/util/fs/safeResolveWithin.test.ts new file mode 100644 index 0000000..ecf77b1 --- /dev/null +++ b/src/util/fs/safeResolveWithin.test.ts @@ -0,0 +1,53 @@ +import safeResolveWithin from './safeResolveWithin.js'; + +describe('safeResolveWithin', () => { + const root = '/workspace/project'; + + it('accepts normalized paths that remain inside the root', () => { + expect( + safeResolveWithin( + root, + './components/../components/00-base/button', + 'Component destination', + ), + ).toBe('/workspace/project/components/00-base/button'); + }); + + it('accepts target path segments that remain inside the root', () => { + expect( + safeResolveWithin( + root, + ['components', '00-base', 'button'], + 'Component destination', + ), + ).toBe('/workspace/project/components/00-base/button'); + }); + + it('rejects path traversal outside the root', () => { + expect(() => + safeResolveWithin(root, '../../outside', 'Component destination'), + ).toThrow( + 'Component destination "../../outside" resolves to "/outside", which is outside the expected root "/workspace/project".', + ); + }); + + it('rejects absolute paths outside the root', () => { + expect(() => + safeResolveWithin(root, '/tmp/outside', 'Asset destination'), + ).toThrow( + 'Asset destination "/tmp/outside" resolves to "/tmp/outside", which is outside the expected root "/workspace/project".', + ); + }); + + it('rejects the root itself by default', () => { + expect(() => safeResolveWithin(root, '.', 'Asset destination')).toThrow( + 'Asset destination "." resolves to the expected root "/workspace/project", but a path inside the root is required.', + ); + }); + + it('allows the root itself only when explicitly requested', () => { + expect( + safeResolveWithin(root, '.', 'Project root', { allowRoot: true }), + ).toBe('/workspace/project'); + }); +}); diff --git a/src/util/fs/safeResolveWithin.ts b/src/util/fs/safeResolveWithin.ts new file mode 100644 index 0000000..253e16d --- /dev/null +++ b/src/util/fs/safeResolveWithin.ts @@ -0,0 +1,50 @@ +import { isAbsolute, relative, resolve } from 'path'; + +type SafeResolveWithinOptions = { + allowRoot?: boolean; +}; + +function formatTarget(target: string | string[]): string { + return Array.isArray(target) ? target.join('/') : target; +} + +/** + * Resolve a target path and ensure it stays within an expected root. + * + * @param rootPath absolute or relative root directory path. + * @param target path string or path segments to resolve from the root. + * @param label user-facing label for error messages. + * @param options.allowRoot whether the target may resolve exactly to the root. + * @returns absolute resolved target path. + */ +export default function safeResolveWithin( + rootPath: string, + target: string | string[], + label: string, + { allowRoot = false }: SafeResolveWithinOptions = {}, +): string { + const root = resolve(rootPath); + const resolvedTarget = Array.isArray(target) + ? resolve(root, ...target) + : resolve(root, target); + const targetLabel = formatTarget(target); + const relativePath = relative(root, resolvedTarget); + + if (relativePath === '') { + if (allowRoot) { + return resolvedTarget; + } + + throw new Error( + `${label} "${targetLabel}" resolves to the expected root "${root}", but a path inside the root is required.`, + ); + } + + if (relativePath.startsWith('..') || isAbsolute(relativePath)) { + throw new Error( + `${label} "${targetLabel}" resolves to "${resolvedTarget}", which is outside the expected root "${root}".`, + ); + } + + return resolvedTarget; +} From ce5caf712e3d524a9d14bbf566afe1e98e762df1 Mon Sep 17 00:00:00 2001 From: Callin Mullaney <57088-callinmullaney@users.noreply.drupalcode.org> Date: Fri, 12 Jun 2026 16:01:51 -0500 Subject: [PATCH 2/3] fix: guard component file destinations --- src/util/project/generateComponent.test.ts | 31 ++++++++++++++++ src/util/project/generateComponent.ts | 28 ++++++++++---- .../project/installComponentFromCache.test.ts | 37 ++++++++++++++++++- src/util/project/installComponentFromCache.ts | 9 ++++- 4 files changed, 95 insertions(+), 10 deletions(-) diff --git a/src/util/project/generateComponent.test.ts b/src/util/project/generateComponent.test.ts index f43397c..0fe59aa 100644 --- a/src/util/project/generateComponent.test.ts +++ b/src/util/project/generateComponent.test.ts @@ -229,6 +229,37 @@ describe('generateComponent', () => { ); }); + it('rejects unsafe structure directories before writing or removing files', async () => { + expect.assertions(3); + setStdinIsTTY(false); + pathExistsMock.mockResolvedValue(true); + + await expect( + generateComponent( + { + ...variant, + structureImplementations: [ + { + name: 'base', + directory: '../../outside', + }, + ], + } as EmulsifyVariant, + 'link', + { + directory: 'base', + format: 'default', + yes: true, + }, + ), + ).rejects.toThrow( + 'Component structure directory "../../outside" resolves to "/home/uname/Projects/cornflake/web/themes/outside", which is outside the expected root "/home/uname/Projects/cornflake/web/themes/custom/themename".', + ); + + expect(removeMock).not.toHaveBeenCalled(); + expect(writeFileMock).not.toHaveBeenCalled(); + }); + it('should cancel component creation if user declines overwrite', async () => { expect.assertions(2); (select as jest.Mock).mockResolvedValueOnce('default'); // format diff --git a/src/util/project/generateComponent.ts b/src/util/project/generateComponent.ts index c201f25..d49e830 100644 --- a/src/util/project/generateComponent.ts +++ b/src/util/project/generateComponent.ts @@ -3,12 +3,13 @@ import type { CreateComponentHandlerOptions } from '@emulsify-cli/handlers'; import { select, confirm } from '@inquirer/prompts'; import { promises as fs } from 'fs'; -import { join, dirname } from 'path'; +import { dirname } from 'path'; import { pathExists, remove } from 'fs-extra'; import { cyan, green, bold, yellow } from 'colorette'; import log from '../../lib/log.js'; import findFileInCurrentPath from '../fs/findFileInCurrentPath.js'; +import safeResolveWithin from '../fs/safeResolveWithin.js'; import { EMULSIFY_PROJECT_CONFIG_FILE } from '../../lib/constants.js'; import deriveComponentNames from '../deriveComponentNames.js'; import resolveComponentTemplate from './resolveComponentTemplate.js'; @@ -96,6 +97,7 @@ export default async function generateComponent( 'Unable to find an Emulsify project to create the component into.', ); } + const projectRoot = dirname(path); // Prompts are only used for interactive TTY sessions; CI must provide flags so // the command never waits for input it cannot receive. @@ -141,14 +143,23 @@ export default async function generateComponent( } // Calculate the parent path based on the path to the Emulsify project and the component's structure. - const parentPath = join(dirname(path), structure.directory); + const parentPath = safeResolveWithin( + projectRoot, + structure.directory, + 'Component structure directory', + { allowRoot: true }, + ); if (!(await pathExists(parentPath))) { // Create the component's parent directory. await fs.mkdir(parentPath, { recursive: true }); } // Calculate the destination path (always kebab-case folder name). - const destination = join(dirname(path), structure.directory, filename); + const destination = safeResolveWithin( + projectRoot, + [structure.directory, filename], + 'Component destination', + ); // If the component already exists within the project, // ask the user if they want to replace it. @@ -244,16 +255,19 @@ export default async function generateComponent( // the byte-for-byte built-in builders for each known generated artifact. const templateFile = (await resolveComponentTemplate( - dirname(path), + projectRoot, format, artifact.logicalName, templateVars, )) ?? artifact.build(); - await fs.writeFile( - join(destination, artifact.destinationName), - templateFile, + const artifactDestination = safeResolveWithin( + projectRoot, + [structure.directory, filename, artifact.destinationName], + 'Component file destination', ); + + await fs.writeFile(artifactDestination, templateFile); } return log( diff --git a/src/util/project/installComponentFromCache.test.ts b/src/util/project/installComponentFromCache.test.ts index 759f0b1..011aa38 100644 --- a/src/util/project/installComponentFromCache.test.ts +++ b/src/util/project/installComponentFromCache.test.ts @@ -11,8 +11,17 @@ const findFileMock = (findFileInCurrentPath as jest.Mock).mockReturnValue( '/home/username/Projects/drupal-project/web/themes/custom/themename/project.emulsify.json', ); const pathExistsMock = (pathExists as jest.Mock).mockResolvedValue(false); +const copyItemMock = copyItemFromCache as jest.Mock; describe('installComponentFromCache', () => { + beforeEach(() => { + jest.clearAllMocks(); + findFileMock.mockReturnValue( + '/home/username/Projects/drupal-project/web/themes/custom/themename/project.emulsify.json', + ); + pathExistsMock.mockResolvedValue(false); + }); + const system = { name: 'compound', } as EmulsifySystem; @@ -71,6 +80,32 @@ describe('installComponentFromCache', () => { ); }); + it('rejects unsafe component destinations before checking or copying files', async () => { + expect.assertions(3); + + await expect( + installComponentFromCache( + system, + { + ...variant, + structureImplementations: [ + { + name: 'base', + directory: '../../outside', + }, + ], + } as EmulsifyVariant, + 'link', + true, + ), + ).rejects.toThrow( + 'Component destination "../../outside/link" resolves to "/home/username/Projects/drupal-project/web/themes/outside/link", which is outside the expected root "/home/username/Projects/drupal-project/web/themes/custom/themename".', + ); + + expect(pathExistsMock).not.toHaveBeenCalled(); + expect(copyItemMock).not.toHaveBeenCalled(); + }); + it('throws an error if the component is already installed, and force is false', async () => { expect.assertions(1); pathExistsMock.mockResolvedValueOnce(true); @@ -84,7 +119,7 @@ describe('installComponentFromCache', () => { it('copies the component from the cached item into the correct destination', async () => { expect.assertions(1); await installComponentFromCache(system, variant, 'link'); - expect(copyItemFromCache as jest.Mock).toHaveBeenCalledWith( + expect(copyItemMock).toHaveBeenCalledWith( 'systems', ['compound', './components/00-base', 'link'], '/home/username/Projects/drupal-project/web/themes/custom/themename/components/00-base/link', diff --git a/src/util/project/installComponentFromCache.ts b/src/util/project/installComponentFromCache.ts index dbfb3eb..ccf836c 100644 --- a/src/util/project/installComponentFromCache.ts +++ b/src/util/project/installComponentFromCache.ts @@ -1,8 +1,9 @@ import { pathExists } from 'fs-extra'; import type { EmulsifySystem, EmulsifyVariant } from '@emulsify-cli/config'; -import { join, dirname } from 'path'; +import { dirname } from 'path'; import { EMULSIFY_PROJECT_CONFIG_FILE } from '../../lib/constants.js'; import findFileInCurrentPath from '../fs/findFileInCurrentPath.js'; +import safeResolveWithin from '../fs/safeResolveWithin.js'; import copyItemFromCache from '../cache/copyItemFromCache.js'; /** @@ -37,7 +38,11 @@ export function getComponentDestination( ); } - return join(dirname(projectConfigPath), structure.directory, component.name); + return safeResolveWithin( + dirname(projectConfigPath), + [structure.directory, component.name], + 'Component destination', + ); } /** From 7c77ee901e3bd6ca7ed7c566cbb44a7eba61e2d0 Mon Sep 17 00:00:00 2001 From: Callin Mullaney <57088-callinmullaney@users.noreply.drupalcode.org> Date: Fri, 12 Jun 2026 16:02:36 -0500 Subject: [PATCH 3/3] fix: guard general asset destinations --- .../installGeneralAssetsFromCache.test.ts | 27 ++++++++++++++++++- .../project/installGeneralAssetsFromCache.ts | 10 +++++-- 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/src/util/project/installGeneralAssetsFromCache.test.ts b/src/util/project/installGeneralAssetsFromCache.test.ts index abd5bd2..1f57c41 100644 --- a/src/util/project/installGeneralAssetsFromCache.test.ts +++ b/src/util/project/installGeneralAssetsFromCache.test.ts @@ -13,8 +13,13 @@ const copyItemMock = (copyItemFromCache as jest.Mock).mockResolvedValue(true); describe('installGeneralAssetsFromCache', () => { beforeEach(() => { - copyItemMock.mockClear(); + jest.clearAllMocks(); + findFileMock.mockReturnValue( + '/home/username/Projects/drupal-project/web/themes/custom/themename/project.emulsify.json', + ); + copyItemMock.mockResolvedValue(true); }); + const system = { name: 'compound', } as EmulsifySystem; @@ -45,6 +50,26 @@ describe('installGeneralAssetsFromCache', () => { ); }); + it('rejects unsafe general asset destination paths before copying files', async () => { + expect.assertions(2); + + await expect( + installGeneralAssetsFromCache(system, { + directories: [ + { + name: 'unsafe', + path: './components/unsafe', + destinationPath: '../../outside', + }, + ], + } as EmulsifyVariant), + ).rejects.toThrow( + 'General asset destination "../../outside" resolves to "/home/username/Projects/drupal-project/web/themes/outside", which is outside the expected root "/home/username/Projects/drupal-project/web/themes/custom/themename".', + ); + + expect(copyItemMock).not.toHaveBeenCalled(); + }); + it('copies all general files and directories into the Emulsify project', async () => { expect.assertions(2); await installGeneralAssetsFromCache(system, variant); diff --git a/src/util/project/installGeneralAssetsFromCache.ts b/src/util/project/installGeneralAssetsFromCache.ts index ed80606..4aaacd9 100644 --- a/src/util/project/installGeneralAssetsFromCache.ts +++ b/src/util/project/installGeneralAssetsFromCache.ts @@ -1,7 +1,8 @@ import type { EmulsifySystem, EmulsifyVariant } from '@emulsify-cli/config'; -import { join, dirname } from 'path'; +import { dirname } from 'path'; import { EMULSIFY_PROJECT_CONFIG_FILE } from '../../lib/constants.js'; import findFileInCurrentPath from '../fs/findFileInCurrentPath.js'; +import safeResolveWithin from '../fs/safeResolveWithin.js'; import copyItemFromCache from '../cache/copyItemFromCache.js'; import catchLater from '../catchLater.js'; @@ -24,11 +25,16 @@ export default async function installGeneralAssetsFromCache( 'Unable to find an Emulsify project to install assets into.', ); } + const projectRoot = dirname(path); const assets = [...(variant.directories || []), ...(variant.files || [])]; const promises = []; for (const asset of assets) { - const destination = join(dirname(path), asset.destinationPath); + const destination = safeResolveWithin( + projectRoot, + asset.destinationPath, + 'General asset destination', + ); promises.push( catchLater( copyItemFromCache(