From 9aa7f0f1aa1f251ba2ab6ab96bfe73eef1b449b9 Mon Sep 17 00:00:00 2001 From: Callin Mullaney <57088-callinmullaney@users.noreply.drupalcode.org> Date: Fri, 12 Jun 2026 16:07:51 -0500 Subject: [PATCH 1/2] fix: execute hook scripts without shell strings --- src/util/fs/executeScript.ts | 32 +++++++++++++++++++++++++------- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/src/util/fs/executeScript.ts b/src/util/fs/executeScript.ts index 26569e8..3e86836 100644 --- a/src/util/fs/executeScript.ts +++ b/src/util/fs/executeScript.ts @@ -1,4 +1,6 @@ -import { exec } from 'child_process'; +import { execFile } from 'child_process'; +import { dirname } from 'path'; + /** * Takes a path to a script, and executes it. * @@ -8,12 +10,28 @@ export default async function executeScript( scriptPath: string, ): Promise { return new Promise((resolve, reject) => { - exec(scriptPath, (error, stdout, stderr) => { - if (error) { - return reject(error); - } + execFile( + process.execPath, + [scriptPath], + { + // Run from the hook directory so hook-relative file operations do not + // depend on the shell location that invoked the CLI. + cwd: dirname(scriptPath), + encoding: 'utf8', + }, + (error, stdout, stderr) => { + if (error) { + const output = stderr.trim() || error.message; + + return reject( + new Error( + `Unable to execute hook script "${scriptPath}": ${output}`, + ), + ); + } - resolve(stdout ? stdout : stderr); - }); + resolve(stdout || stderr || ''); + }, + ); }); } From 7021b0cd6d22efd3cc0f4c891ebccdcfb9f87333 Mon Sep 17 00:00:00 2001 From: Callin Mullaney <57088-callinmullaney@users.noreply.drupalcode.org> Date: Fri, 12 Jun 2026 16:08:45 -0500 Subject: [PATCH 2/2] test: cover hook script execution --- src/util/fs/executeScript.test.ts | 108 ++++++++++++++++++++++++------ 1 file changed, 87 insertions(+), 21 deletions(-) diff --git a/src/util/fs/executeScript.test.ts b/src/util/fs/executeScript.test.ts index 02f372b..81b3ea2 100644 --- a/src/util/fs/executeScript.test.ts +++ b/src/util/fs/executeScript.test.ts @@ -1,40 +1,106 @@ -// @ts-nocheck -// This file is escaping ts checks for now, because the child_process.exec -// mock fn is selecting a specific exec overload that is incorrect. -// @TODO: dig into this and figure out how to get typescript to use -// the correct overload. -import childproc from 'child_process'; +import { execFile } from 'child_process'; import executeScript from './executeScript.js'; -const execMock = jest.spyOn(childproc, 'exec'); +jest.mock('child_process', () => ({ + execFile: jest.fn(), +})); + +const execFileMock = execFile as unknown as jest.Mock; + +type ExecFileCallback = ( + error: Error | null, + stdout: string, + stderr: string, +) => void; + +function mockExecFileResult( + error: Error | null, + stdout = '', + stderr = '', +): void { + execFileMock.mockImplementationOnce( + ( + _command: string, + _args: string[], + _options: { cwd: string; encoding: string }, + callback: ExecFileCallback, + ) => { + callback(error, stdout, stderr); + }, + ); +} describe('executeScript', () => { - it('can execute a script, and resolve the stdout', async () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('executes a hook script with node and resolves stdout', async () => { expect.assertions(2); - execMock.mockImplementationOnce((_, callback: () => void) => - callback(null, 'done'), + const scriptPath = '/project/.cli/init.js'; + mockExecFileResult(null, 'done'); + + await expect(executeScript(scriptPath)).resolves.toBe('done'); + expect(execFileMock).toHaveBeenCalledWith( + process.execPath, + [scriptPath], + { + cwd: '/project/.cli', + encoding: 'utf8', + }, + expect.any(Function), ); - await expect(executeScript('path.js')).resolves.toBe('done'); - expect(execMock).toHaveBeenCalledWith('path.js', expect.any(Function)); }); - it('can execute a script, and resolve the stderr', async () => { + it('passes a hook path with spaces as an argument instead of a shell string', async () => { expect.assertions(1); - execMock.mockImplementationOnce((_, callback: () => void) => - callback(null, null, 'well, that went poorly'), + const scriptPath = '/project with spaces/.cli/init.js'; + mockExecFileResult(null, 'done'); + + await executeScript(scriptPath); + + expect(execFileMock).toHaveBeenCalledWith( + process.execPath, + [scriptPath], + { + cwd: '/project with spaces/.cli', + encoding: 'utf8', + }, + expect.any(Function), ); - await expect(executeScript('path.js')).resolves.toBe( + }); + + it('resolves stderr when stdout is empty', async () => { + expect.assertions(1); + mockExecFileResult(null, '', 'well, that went poorly'); + + await expect(executeScript('/project/.cli/init.js')).resolves.toBe( 'well, that went poorly', ); }); - it('can execute a script, and reject with an error', async () => { + it('resolves an empty string when stdout and stderr are empty', async () => { + expect.assertions(1); + mockExecFileResult(null); + + await expect(executeScript('/project/.cli/init.js')).resolves.toBe(''); + }); + + it('rejects failed hooks with stderr context', async () => { expect.assertions(1); - execMock.mockImplementationOnce((_, callback: () => void) => - callback(new Error('well, that went SUPER poorly')), + mockExecFileResult(new Error('Command failed'), '', 'hook failed\n'); + + await expect(executeScript('/project/.cli/init.js')).rejects.toThrow( + 'Unable to execute hook script "/project/.cli/init.js": hook failed', ); - await expect(executeScript('path.js')).rejects.toEqual( - Error('well, that went SUPER poorly'), + }); + + it('rejects execution failures with the process error message', async () => { + expect.assertions(1); + mockExecFileResult(new Error('spawn failed')); + + await expect(executeScript('/project/.cli/init.js')).rejects.toThrow( + 'Unable to execute hook script "/project/.cli/init.js": spawn failed', ); }); });