diff --git a/common/changes/@microsoft/rush/fix-publish-command-injection_2026-07-13-15-40.json b/common/changes/@microsoft/rush/fix-publish-command-injection_2026-07-13-15-40.json new file mode 100644 index 0000000000..394179ed14 --- /dev/null +++ b/common/changes/@microsoft/rush/fix-publish-command-injection_2026-07-13-15-40.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@microsoft/rush", + "comment": "Pass change file paths as discrete Git arguments when adding commit details during publishing.", + "type": "patch" + } + ], + "packageName": "@microsoft/rush" +} diff --git a/libraries/rush-lib/src/logic/PublishUtilities.ts b/libraries/rush-lib/src/logic/PublishUtilities.ts index a76f946f92..bf62ebe05a 100644 --- a/libraries/rush-lib/src/logic/PublishUtilities.ts +++ b/libraries/rush-lib/src/logic/PublishUtilities.ts @@ -7,7 +7,7 @@ */ import * as path from 'node:path'; -import { execSync } from 'node:child_process'; +import type child_process from 'node:child_process'; import * as semver from 'semver'; @@ -17,7 +17,8 @@ import { FileConstants, Text, Enum, - InternalError + InternalError, + Executable } from '@rushstack/node-core-library'; import { type IChangeInfo, ChangeType, type IVersionPolicyChangeInfo } from '../api/ChangeManagement'; @@ -83,13 +84,14 @@ export class PublishUtilities { // Add the minimum changes defined by the change descriptions. for (const changeFilePath of files) { const changeRequest: IChangeInfo = JsonFile.load(changeFilePath); + const changes: IChangeInfo[] = changeRequest.changes!; if (includeCommitDetails) { const git: Git = new Git(rushConfiguration); - PublishUtilities._updateCommitDetails(git, changeFilePath, changeRequest.changes); + await PublishUtilities._updateCommitDetailsAsync(git, changeFilePath, changes); } - for (const change of changeRequest.changes!) { + for (const change of changes) { PublishUtilities._addChange({ change, changeFilePath, @@ -374,16 +376,31 @@ export class PublishUtilities { ); } - private static _updateCommitDetails(git: Git, filename: string, changes: IChangeInfo[] | undefined): void { + private static async _updateCommitDetailsAsync( + git: Git, + filename: string, + changes: IChangeInfo[] + ): Promise { try { const gitPath: string = git.getGitPathOrThrow(); - const fileLog: string = execSync(`${gitPath} log -n 1 ${filename}`, { - cwd: path.dirname(filename) - }).toString(); + const gitProcess: child_process.ChildProcess = Executable.spawn( + gitPath, + ['log', '-n', '1', '--', filename], + { + currentWorkingDirectory: path.dirname(filename) + } + ); + const { stdout: fileLog, exitCode, signal } = await Executable.waitForExitAsync(gitProcess, { + encoding: 'utf8' + }); + if (exitCode !== 0 || signal) { + return; + } + const author: string = fileLog.match(/Author: (.*)/)![1]; const commit: string = fileLog.match(/commit (.*)/)![1]; - changes!.forEach((change) => { + changes.forEach((change) => { change.author = author; change.commit = commit; }); diff --git a/libraries/rush-lib/src/logic/test/PublishUtilities.test.ts b/libraries/rush-lib/src/logic/test/PublishUtilities.test.ts index 3ac792fe0f..a9b73038c9 100644 --- a/libraries/rush-lib/src/logic/test/PublishUtilities.test.ts +++ b/libraries/rush-lib/src/logic/test/PublishUtilities.test.ts @@ -1,16 +1,34 @@ // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. // See LICENSE in the project root for license information. +import * as path from 'node:path'; +import type { ChildProcess } from 'node:child_process'; + +import { Executable, type IWaitForExitResult } from '@rushstack/node-core-library'; import { type IChangeInfo, ChangeType } from '../../api/ChangeManagement'; import { RushConfiguration } from '../../api/RushConfiguration'; import type { RushConfigurationProject } from '../../api/RushConfigurationProject'; import { PublishUtilities, type IChangeRequests } from '../PublishUtilities'; import { ChangeFiles } from '../ChangeFiles'; +import { Git } from '../Git'; function createChangeFiles(changesFolder: string): ChangeFiles { return new ChangeFiles({ changesFolder } as unknown as RushConfiguration); } +function createGitResult( + stdout: string, + exitCode: IWaitForExitResult['exitCode'] = 0, + signal: IWaitForExitResult['signal'] = null +): IWaitForExitResult { + return { + stdout, + stderr: '', + exitCode, + signal + }; +} + function generateChangeSnapshot( allPackages: ReadonlyMap, allChanges: IChangeRequests @@ -83,6 +101,10 @@ describe(PublishUtilities.findChangeRequestsAsync.name, () => { repoRushConfiguration = RushConfiguration.loadFromConfigurationFile(`${__dirname}/repo/rush.json`); }); + afterEach(() => { + jest.restoreAllMocks(); + }); + it('returns no changes in an empty change folder', async () => { const allPackages: ReadonlyMap = packagesRushConfiguration.projectsByName; @@ -96,6 +118,94 @@ describe(PublishUtilities.findChangeRequestsAsync.name, () => { expect(allChanges.versionPolicyChanges.size).toEqual(0); }); + it('passes change file paths as discrete Git arguments', async () => { + const gitPath: string = path.resolve('git with spaces', 'git.exe'); + const changeFilePath: string = path.resolve( + 'repo with spaces', + 'common', + 'changes', + 'change & echo injected.json' + ); + const changes: IChangeInfo[] = [{ packageName: 'd' }]; + const git: Git = new Git(packagesRushConfiguration); + const gitProcess: ChildProcess = {} as ChildProcess; + + jest.spyOn(git, 'getGitPathOrThrow').mockReturnValue(gitPath); + const spawnSpy: jest.SpyInstance = jest.spyOn(Executable, 'spawn').mockReturnValue(gitProcess); + const waitForExitSpy: jest.SpyInstance = jest + .spyOn(Executable, 'waitForExitAsync') + .mockResolvedValue( + createGitResult('commit 0123456789abcdef\nAuthor: Test Author \n') + ); + + await PublishUtilities['_updateCommitDetailsAsync'](git, changeFilePath, changes); + + expect(spawnSpy).toHaveBeenCalledWith( + gitPath, + ['log', '-n', '1', '--', changeFilePath], + { currentWorkingDirectory: path.dirname(changeFilePath) } + ); + expect(waitForExitSpy).toHaveBeenCalledWith(gitProcess, { encoding: 'utf8' }); + expect(changes).toEqual([ + { + packageName: 'd', + author: 'Test Author ', + commit: '0123456789abcdef' + } + ]); + }); + + it('delegates Git wrapper paths to Executable', async () => { + const gitPath: string = path.resolve('git-wrapper', 'git.cmd'); + const changeFilePath: string = path.resolve('repo', 'common', 'changes', 'change.json'); + const changes: IChangeInfo[] = [{ packageName: 'd' }]; + const git: Git = new Git(packagesRushConfiguration); + const gitProcess: ChildProcess = {} as ChildProcess; + + jest.spyOn(git, 'getGitPathOrThrow').mockReturnValue(gitPath); + const spawnSpy: jest.SpyInstance = jest.spyOn(Executable, 'spawn').mockReturnValue(gitProcess); + jest + .spyOn(Executable, 'waitForExitAsync') + .mockResolvedValue( + createGitResult('commit 0123456789abcdef\nAuthor: Test Author \n') + ); + + await PublishUtilities['_updateCommitDetailsAsync'](git, changeFilePath, changes); + + expect(spawnSpy).toHaveBeenCalledWith( + gitPath, + ['log', '-n', '1', '--', changeFilePath], + { currentWorkingDirectory: path.dirname(changeFilePath) } + ); + expect(changes[0].commit).toEqual('0123456789abcdef'); + }); + + it.each([ + { exitCode: 1, signal: null }, + { exitCode: null, signal: 'SIGTERM' } + ])( + 'does not use Git output from an unsuccessful process ($exitCode, $signal)', + async ({ exitCode, signal }) => { + const changes: IChangeInfo[] = [{ packageName: 'd' }]; + const git: Git = new Git(packagesRushConfiguration); + const gitProcess: ChildProcess = {} as ChildProcess; + + jest.spyOn(git, 'getGitPathOrThrow').mockReturnValue(path.resolve('git.exe')); + jest.spyOn(Executable, 'spawn').mockReturnValue(gitProcess); + jest.spyOn(Executable, 'waitForExitAsync').mockResolvedValue( + createGitResult( + 'commit 0123456789abcdef\nAuthor: Test Author \n', + exitCode, + signal + ) + ); + + await PublishUtilities['_updateCommitDetailsAsync'](git, path.resolve('change.json'), changes); + + expect(changes).toEqual([{ packageName: 'd' }]); + } + ); + it('returns 1 change when changing a leaf package', async () => { const allPackages: ReadonlyMap = packagesRushConfiguration.projectsByName;