Skip to content

Commit 705af95

Browse files
ATKasemclaude
andcommitted
[rush] Fix "rush-pnpm patch-commit" writing absolute paths into pnpm-config.json
When running "pnpm patch-commit"/"patch-remove" with pnpm 9+, pnpm rewrites pre-existing patchedDependencies entries using absolute paths into common/temp. Rush then copied those into pnpm-config.json, leaking the local checkout location. Normalize absolute paths under the common/temp folder back to relative paths at the point they are recorded, inside PnpmOptionsConfiguration.updateGlobalPatchedDependencies. This is the single sink for both the pnpm 11+ (pnpm-workspace.yaml) and older (package.json) sources, so both are covered. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 6bb6a2b commit 705af95

3 files changed

Lines changed: 85 additions & 1 deletion

File tree

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
{
2+
"changes": [
3+
{
4+
"packageName": "@microsoft/rush",
5+
"comment": "Fix an issue where \"rush-pnpm patch-commit\" rewrote the pre-existing \"globalPatchedDependencies\" entries in pnpm-config.json using absolute paths when running with pnpm >= 9.",
6+
"type": "patch"
7+
}
8+
],
9+
"packageName": "@microsoft/rush"
10+
}

libraries/rush-lib/src/logic/pnpm/PnpmOptionsConfiguration.ts

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
22
// See LICENSE in the project root for license information.
33

4-
import { JsonFile, type JsonObject } from '@rushstack/node-core-library';
4+
import * as path from 'node:path';
5+
6+
import { JsonFile, type JsonObject, Path } from '@rushstack/node-core-library';
57
import { NonProjectConfigurationFile } from '@rushstack/heft-config-file';
68
import { ConsoleTerminalProvider, Terminal } from '@rushstack/terminal';
79

@@ -211,6 +213,7 @@ export interface IPnpmOptionsJson extends IPackageManagerOptionsJsonBase {
211213
*/
212214
export class PnpmOptionsConfiguration extends PackageManagerOptionsConfigurationBase {
213215
private readonly _json: JsonObject;
216+
private readonly _commonTempFolder: string;
214217
private _globalPatchedDependencies: Record<string, string> | undefined;
215218

216219
/**
@@ -556,6 +559,7 @@ export class PnpmOptionsConfiguration extends PackageManagerOptionsConfiguration
556559
private constructor(json: IPnpmOptionsJson, commonTempFolder: string, jsonFilename?: string) {
557560
super(json);
558561
this._json = json;
562+
this._commonTempFolder = commonTempFolder;
559563
this.jsonFilename = jsonFilename;
560564
this.pnpmStore = json.pnpmStore || 'local';
561565
if (EnvironmentConfiguration.pnpmStorePathOverride) {
@@ -636,8 +640,25 @@ export class PnpmOptionsConfiguration extends PackageManagerOptionsConfiguration
636640

637641
/**
638642
* Updates patchedDependencies field of the PNPM options in the common/config/rush/pnpm-config.json file.
643+
*
644+
* @remarks
645+
* When running "pnpm patch-commit"/"pnpm patch-remove" (pnpm 9 and newer), pnpm rewrites the
646+
* pre-existing entries using absolute paths pointing into the common/temp folder. Normalize any such
647+
* value back to a path relative to the common/temp folder (e.g. "patches/example\@1.0.0.patch") so the location
648+
* of the local checkout does not leak into pnpm-config.json.
639649
*/
640650
public updateGlobalPatchedDependencies(patchedDependencies: Record<string, string> | undefined): void {
651+
if (patchedDependencies) {
652+
const normalized: Record<string, string> = {};
653+
for (const [dependency, patchPath] of Object.entries(patchedDependencies)) {
654+
normalized[dependency] =
655+
path.isAbsolute(patchPath) && Path.isUnder(patchPath, this._commonTempFolder)
656+
? Path.convertToSlashes(path.relative(this._commonTempFolder, patchPath))
657+
: patchPath;
658+
}
659+
patchedDependencies = normalized;
660+
}
661+
641662
this._globalPatchedDependencies = patchedDependencies;
642663
this._json.globalPatchedDependencies = patchedDependencies;
643664
if (this.jsonFilename) {

libraries/rush-lib/src/logic/pnpm/test/PnpmOptionsConfiguration.test.ts

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,4 +169,57 @@ describe(PnpmOptionsConfiguration.name, () => {
169169
}
170170
});
171171
});
172+
173+
describe('updateGlobalPatchedDependencies', () => {
174+
function update(
175+
patchedDependencies: Record<string, string> | undefined
176+
): Record<string, string> | undefined {
177+
// No jsonFilename, so updateGlobalPatchedDependencies won't try to write to disk
178+
const pnpmConfiguration: PnpmOptionsConfiguration = PnpmOptionsConfiguration.loadFromJsonObject(
179+
{},
180+
fakeCommonTempFolder
181+
);
182+
pnpmConfiguration.updateGlobalPatchedDependencies(patchedDependencies);
183+
return pnpmConfiguration.globalPatchedDependencies;
184+
}
185+
186+
it('converts absolute patch paths under the common temp folder back to relative paths', () => {
187+
// pnpm >= 9 "patch-commit"/"patch-remove" rewrite pre-existing "patchedDependencies" entries
188+
// using absolute paths pointing into the common/temp folder
189+
expect(
190+
update({
191+
'example@1.0.0': path.join(fakeCommonTempFolder, 'patches', 'example@1.0.0.patch'),
192+
'@scope/example2@2.0.0': path.join(fakeCommonTempFolder, 'patches', '@scope__example2@2.0.0.patch')
193+
})
194+
).toEqual({
195+
'example@1.0.0': 'patches/example@1.0.0.patch',
196+
'@scope/example2@2.0.0': 'patches/@scope__example2@2.0.0.patch'
197+
});
198+
});
199+
200+
it('leaves relative patch paths unchanged', () => {
201+
expect(
202+
update({
203+
'example@1.0.0': 'patches/example@1.0.0.patch'
204+
})
205+
).toEqual({
206+
'example@1.0.0': 'patches/example@1.0.0.patch'
207+
});
208+
});
209+
210+
it('leaves absolute patch paths outside the common temp folder unchanged', () => {
211+
const outsidePath: string = path.join(path.sep, 'somewhere', 'else', 'example@1.0.0.patch');
212+
expect(
213+
update({
214+
'example@1.0.0': outsidePath
215+
})
216+
).toEqual({
217+
'example@1.0.0': outsidePath
218+
});
219+
});
220+
221+
it('passes through undefined', () => {
222+
expect(update(undefined)).toBeUndefined();
223+
});
224+
});
172225
});

0 commit comments

Comments
 (0)