Skip to content

Commit 99e7cf2

Browse files
Copiloticlanton
andauthored
Add trimRushEnvironmentVariablesForOperations experiment
Co-authored-by: iclanton <5010588+iclanton@users.noreply.github.com>
1 parent a88b242 commit 99e7cf2

7 files changed

Lines changed: 167 additions & 0 deletions

File tree

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
{
2+
"changes": [
3+
{
4+
"packageName": "@microsoft/rush-lib",
5+
"comment": "Add a new `trimRushEnvironmentVariablesForOperations` experiment that, when enabled, omits environment variables whose names begin with `RUSH_` from the environment forwarded to operation processes (e.g. \"build\", \"test\").",
6+
"type": "minor"
7+
}
8+
]
9+
}

common/reviews/api/rush-lib.api.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -491,6 +491,7 @@ export interface IExperimentsJson {
491491
printEventHooksOutputToConsole?: boolean;
492492
rushAlerts?: boolean;
493493
strictChangefileValidation?: boolean;
494+
trimRushEnvironmentVariablesForOperations?: boolean;
494495
useDirectFileTransfersForBuildCache?: boolean;
495496
useIPCScriptsInWatchMode?: boolean;
496497
usePnpmFrozenLockfileForRushInstall?: boolean;

libraries/rush-lib/src/api/ExperimentsConfiguration.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,15 @@ export interface IExperimentsJson {
153153
* effect; otherwise it falls back to the buffer-based approach.
154154
*/
155155
useDirectFileTransfersForBuildCache?: boolean;
156+
157+
/**
158+
* By default, Rush forwards its entire process environment (minus a small denylist) to the shell
159+
* commands it invokes for operations (e.g. "build", "test"). If true, environment variables whose
160+
* names begin with `RUSH_` will additionally be omitted from that forwarded environment. This can
161+
* help prevent operation scripts from accidentally depending on Rush's own internal environment
162+
* variables.
163+
*/
164+
trimRushEnvironmentVariablesForOperations?: boolean;
156165
}
157166

158167
const _EXPERIMENTS_JSON_SCHEMA: JsonSchema = JsonSchema.fromLoadedObject(schemaJson);

libraries/rush-lib/src/cli/scriptActions/PhasedScriptAction.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ import { getVariantAsync, VARIANT_PARAMETER } from '../../api/Variants';
5959
import { Selection } from '../../logic/Selection';
6060
import { NodeDiagnosticDirPlugin } from '../../logic/operations/NodeDiagnosticDirPlugin';
6161
import { IgnoredParametersPlugin } from '../../logic/operations/IgnoredParametersPlugin';
62+
import { TrimRushEnvironmentVariablesPlugin } from '../../logic/operations/TrimRushEnvironmentVariablesPlugin';
6263
import { DebugHashesPlugin } from '../../logic/operations/DebugHashesPlugin';
6364
import { measureAsyncFn, measureFn } from '../../utilities/performance';
6465

@@ -404,6 +405,14 @@ export class PhasedScriptAction extends BaseScriptAction<IPhasedCommandConfig> i
404405
// Verifies correctness of rush-project.json entries for the graph
405406
new ValidateOperationsPlugin(terminal).apply(hooks);
406407

408+
if (
409+
this.rushConfiguration.experimentsConfiguration.configuration
410+
.trimRushEnvironmentVariablesForOperations
411+
) {
412+
// Trim RUSH_-prefixed environment variables before forwarding to operation processes
413+
new TrimRushEnvironmentVariablesPlugin().apply(hooks);
414+
}
415+
407416
// Forward ignored parameters to child processes as an environment variable
408417
new IgnoredParametersPlugin().apply(hooks);
409418

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
2+
// See LICENSE in the project root for license information.
3+
4+
import type { IPhasedCommandPlugin, PhasedCommandHooks } from '../../pluginFramework/PhasedCommandHooks';
5+
import type { IEnvironment } from '../../utilities/Utilities';
6+
7+
const PLUGIN_NAME: 'TrimRushEnvironmentVariablesPlugin' = 'TrimRushEnvironmentVariablesPlugin';
8+
9+
/**
10+
* Prefix used by environment variables that Rush itself defines and consumes.
11+
*/
12+
const RUSH_ENVIRONMENT_VARIABLE_NAME_PREFIX: 'RUSH_' = 'RUSH_';
13+
14+
/**
15+
* Phased command plugin that removes environment variables whose names begin with `RUSH_` before
16+
* they are forwarded to operation processes. Enabled via the `trimRushEnvironmentVariablesForOperations`
17+
* experiment in experiments.json.
18+
*/
19+
export class TrimRushEnvironmentVariablesPlugin implements IPhasedCommandPlugin {
20+
public apply(hooks: PhasedCommandHooks): void {
21+
hooks.onGraphCreatedAsync.tap(PLUGIN_NAME, (graph) => {
22+
graph.hooks.createEnvironmentForOperation.tap(PLUGIN_NAME, (env: IEnvironment) => {
23+
for (const key of Object.getOwnPropertyNames(env)) {
24+
if (key.toUpperCase().startsWith(RUSH_ENVIRONMENT_VARIABLE_NAME_PREFIX)) {
25+
delete env[key];
26+
}
27+
}
28+
29+
return env;
30+
});
31+
});
32+
}
33+
}
Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
2+
// See LICENSE in the project root for license information.
3+
4+
import path from 'node:path';
5+
import { JsonFile } from '@rushstack/node-core-library';
6+
7+
import { RushConfiguration } from '../../../api/RushConfiguration';
8+
import { CommandLineConfiguration, type IPhasedCommandConfig } from '../../../api/CommandLineConfiguration';
9+
import type { Operation } from '../Operation';
10+
import type { ICommandLineJson } from '../../../api/CommandLineJson';
11+
import { PhasedOperationPlugin } from '../PhasedOperationPlugin';
12+
import { ShellOperationRunnerPlugin } from '../ShellOperationRunnerPlugin';
13+
import { TrimRushEnvironmentVariablesPlugin } from '../TrimRushEnvironmentVariablesPlugin';
14+
import {
15+
type ICreateOperationsContext,
16+
type IOperationGraphContext,
17+
PhasedCommandHooks
18+
} from '../../../pluginFramework/PhasedCommandHooks';
19+
import type { IOperationGraph } from '../IOperationGraph';
20+
import { OperationGraphHooks } from '../../../pluginFramework/OperationGraphHooks';
21+
import type { IEnvironment } from '../../../utilities/Utilities';
22+
import type { IOperationRunnerContext } from '../IOperationRunner';
23+
import type { IOperationExecutionResult } from '../IOperationExecutionResult';
24+
25+
/**
26+
* Helper function to create a minimal mock record for testing the createEnvironmentForOperation hook
27+
*/
28+
function createMockRecord(operation: Operation): IOperationRunnerContext & IOperationExecutionResult {
29+
return {
30+
operation,
31+
environment: undefined
32+
} as IOperationRunnerContext & IOperationExecutionResult;
33+
}
34+
35+
describe(TrimRushEnvironmentVariablesPlugin.name, () => {
36+
it('should remove RUSH_-prefixed environment variables while preserving others', async () => {
37+
const rushJsonFile: string = path.resolve(__dirname, `../../test/parameterIgnoringRepo/rush.json`);
38+
const commandLineJsonFile: string = path.resolve(
39+
__dirname,
40+
`../../test/parameterIgnoringRepo/common/config/rush/command-line.json`
41+
);
42+
43+
const rushConfiguration = RushConfiguration.loadFromConfigurationFile(rushJsonFile);
44+
const commandLineJson: ICommandLineJson = JsonFile.load(commandLineJsonFile);
45+
46+
const commandLineConfiguration = new CommandLineConfiguration(commandLineJson);
47+
const buildCommand: IPhasedCommandConfig = commandLineConfiguration.commands.get(
48+
'build'
49+
)! as IPhasedCommandConfig;
50+
51+
const fakeCreateOperationsContext: Pick<
52+
ICreateOperationsContext,
53+
'phaseSelection' | 'projectSelection' | 'projectConfigurations' | 'rushConfiguration'
54+
> = {
55+
phaseSelection: buildCommand.phases,
56+
projectSelection: new Set(rushConfiguration.projects),
57+
projectConfigurations: new Map(),
58+
rushConfiguration
59+
};
60+
61+
const hooks: PhasedCommandHooks = new PhasedCommandHooks();
62+
63+
// Apply plugins
64+
new PhasedOperationPlugin().apply(hooks);
65+
new ShellOperationRunnerPlugin().apply(hooks);
66+
new TrimRushEnvironmentVariablesPlugin().apply(hooks);
67+
68+
const operations: Set<Operation> = await hooks.createOperationsAsync.promise(
69+
new Set(),
70+
fakeCreateOperationsContext as unknown as ICreateOperationsContext
71+
);
72+
73+
// Set up a mock graph and invoke onGraphCreatedAsync so the plugin registers its graph hooks
74+
const graphHooks: OperationGraphHooks = new OperationGraphHooks();
75+
const fakeGraph: IOperationGraph = { hooks: graphHooks } as unknown as IOperationGraph;
76+
await hooks.onGraphCreatedAsync.promise(
77+
fakeGraph,
78+
fakeCreateOperationsContext as unknown as IOperationGraphContext
79+
);
80+
81+
const operation = Array.from(operations)[0];
82+
expect(operation).toBeDefined();
83+
84+
const mockRecord = createMockRecord(operation);
85+
86+
const initialEnvironment: IEnvironment = {
87+
...process.env,
88+
RUSH_TEMP_FOLDER: 'some-temp-folder',
89+
Rush_Some_Mixed_Case_Var: 'should also be trimmed',
90+
RUSHSTACK_FILE_ERROR_BASE_FOLDER: 'should be preserved',
91+
PATH: process.env.PATH,
92+
SOME_OTHER_VAR: 'should be preserved'
93+
};
94+
95+
const env: IEnvironment = graphHooks.createEnvironmentForOperation.call(initialEnvironment, mockRecord);
96+
97+
expect(env.RUSH_TEMP_FOLDER).toBeUndefined();
98+
expect(env.Rush_Some_Mixed_Case_Var).toBeUndefined();
99+
expect(env.RUSHSTACK_FILE_ERROR_BASE_FOLDER).toBe('should be preserved');
100+
expect(env.SOME_OTHER_VAR).toBe('should be preserved');
101+
});
102+
});

libraries/rush-lib/src/schemas/experiments.schema.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,10 @@
9393
"useDirectFileTransfersForBuildCache": {
9494
"description": "If true, the build cache will use file-based APIs to transfer cache entries to and from cloud storage. This avoids loading the entire cache entry into memory, which can prevent out-of-memory errors for large build outputs and allow cache entries to exceed the limit of a single Buffer. The cloud cache provider plugin must implement the optional file-based methods for this to take effect; otherwise it falls back to the buffer-based approach.",
9595
"type": "boolean"
96+
},
97+
"trimRushEnvironmentVariablesForOperations": {
98+
"description": "By default, Rush forwards its entire process environment (minus a small denylist) to the shell commands it invokes for operations (e.g. 'build', 'test'). If true, environment variables whose names begin with `RUSH_` will additionally be omitted from that forwarded environment. This can help prevent operation scripts from accidentally depending on Rush's own internal environment variables.",
99+
"type": "boolean"
96100
}
97101
},
98102
"additionalProperties": false

0 commit comments

Comments
 (0)