Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"changes": [
{
"packageName": "@microsoft/rush",
"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\").",
"type": "minor"
}
]
}
1 change: 1 addition & 0 deletions common/reviews/api/rush-lib.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,7 @@ export interface IExperimentsJson {
printEventHooksOutputToConsole?: boolean;
rushAlerts?: boolean;
strictChangefileValidation?: boolean;
trimRushEnvironmentVariablesForOperations?: boolean;
useDirectFileTransfersForBuildCache?: boolean;
useIPCScriptsInWatchMode?: boolean;
usePnpmFrozenLockfileForRushInstall?: boolean;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,5 +141,14 @@
* must implement the optional file-based methods for this to take effect; otherwise it falls back to the
* buffer-based approach.
*/
/*[LINE "HYPOTHETICAL"]*/ "useDirectFileTransfersForBuildCache": true
/*[LINE "HYPOTHETICAL"]*/ "useDirectFileTransfersForBuildCache": true,

/**
* 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.
*/
/*[LINE "HYPOTHETICAL"]*/ "trimRushEnvironmentVariablesForOperations": true
}
8 changes: 7 additions & 1 deletion libraries/rush-lib/src/api/EnvironmentConfiguration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,12 @@ export const EnvironmentVariableNames = {
RUSH_QUIET_MODE: 'RUSH_QUIET_MODE'
} as const;

/**
* Matches the names of environment variables that are reserved for use by Rush itself.
* @internal
*/
export const RUSH_ENVIRONMENT_VARIABLE_NAME_REGEXP: RegExp = /^RUSH_/i;

let _hasBeenValidated: boolean = false;

let _rushTempFolderOverride: string | undefined;
Expand Down Expand Up @@ -496,7 +502,7 @@ export class EnvironmentConfiguration {

const unknownEnvVariables: string[] = [];
for (const envVarName in process.env) {
if (process.env.hasOwnProperty(envVarName) && envVarName.match(/^RUSH_/i)) {
if (process.env.hasOwnProperty(envVarName) && envVarName.match(RUSH_ENVIRONMENT_VARIABLE_NAME_REGEXP)) {
const value: string | undefined = process.env[envVarName];
// Environment variables are only case-insensitive on Windows
const normalizedEnvVarName: string = IS_WINDOWS ? envVarName.toUpperCase() : envVarName;
Expand Down
9 changes: 9 additions & 0 deletions libraries/rush-lib/src/api/ExperimentsConfiguration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,15 @@ export interface IExperimentsJson {
* effect; otherwise it falls back to the buffer-based approach.
*/
useDirectFileTransfersForBuildCache?: boolean;

/**
* 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.
*/
trimRushEnvironmentVariablesForOperations?: boolean;
}

const _EXPERIMENTS_JSON_SCHEMA: JsonSchema = JsonSchema.fromLoadedObject(schemaJson);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ import { getVariantAsync, VARIANT_PARAMETER } from '../../api/Variants';
import { Selection } from '../../logic/Selection';
import { NodeDiagnosticDirPlugin } from '../../logic/operations/NodeDiagnosticDirPlugin';
import { IgnoredParametersPlugin } from '../../logic/operations/IgnoredParametersPlugin';
import { TrimRushEnvironmentVariablesPlugin } from '../../logic/operations/TrimRushEnvironmentVariablesPlugin';
import { DebugHashesPlugin } from '../../logic/operations/DebugHashesPlugin';
import { measureAsyncFn, measureFn } from '../../utilities/performance';

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

if (
this.rushConfiguration.experimentsConfiguration.configuration
.trimRushEnvironmentVariablesForOperations
) {
// Trim RUSH_-prefixed environment variables before forwarding to operation processes
new TrimRushEnvironmentVariablesPlugin().apply(hooks);
}

// Forward ignored parameters to child processes as an environment variable
new IgnoredParametersPlugin().apply(hooks);

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
// See LICENSE in the project root for license information.

import { RUSH_ENVIRONMENT_VARIABLE_NAME_REGEXP } from '../../api/EnvironmentConfiguration';
import type { IPhasedCommandPlugin, PhasedCommandHooks } from '../../pluginFramework/PhasedCommandHooks';
import type { IEnvironment } from '../../utilities/Utilities';

const PLUGIN_NAME: 'TrimRushEnvironmentVariablesPlugin' = 'TrimRushEnvironmentVariablesPlugin';

/**
* Phased command plugin that removes environment variables whose names begin with `RUSH_` before
* they are forwarded to operation processes. Enabled via the `trimRushEnvironmentVariablesForOperations`
* experiment in experiments.json.
*/
export class TrimRushEnvironmentVariablesPlugin implements IPhasedCommandPlugin {
public apply(hooks: PhasedCommandHooks): void {
hooks.onGraphCreatedAsync.tap(PLUGIN_NAME, (graph) => {
graph.hooks.createEnvironmentForOperation.tap(PLUGIN_NAME, (env: IEnvironment) => {
const trimmedEnv: IEnvironment = {};
for (const key of Object.getOwnPropertyNames(env)) {
if (!RUSH_ENVIRONMENT_VARIABLE_NAME_REGEXP.test(key)) {
trimmedEnv[key] = env[key];
}
}

return trimmedEnv;
});
});
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
// See LICENSE in the project root for license information.

import path from 'node:path';
import { JsonFile } from '@rushstack/node-core-library';

import { RushConfiguration } from '../../../api/RushConfiguration';
import { CommandLineConfiguration, type IPhasedCommandConfig } from '../../../api/CommandLineConfiguration';
import type { Operation } from '../Operation';
import type { ICommandLineJson } from '../../../api/CommandLineJson';
import { PhasedOperationPlugin } from '../PhasedOperationPlugin';
import { ShellOperationRunnerPlugin } from '../ShellOperationRunnerPlugin';
import { TrimRushEnvironmentVariablesPlugin } from '../TrimRushEnvironmentVariablesPlugin';
import {
type ICreateOperationsContext,
type IOperationGraphContext,
PhasedCommandHooks
} from '../../../pluginFramework/PhasedCommandHooks';
import type { IOperationGraph } from '../IOperationGraph';
import { OperationGraphHooks } from '../../../pluginFramework/OperationGraphHooks';
import type { IEnvironment } from '../../../utilities/Utilities';
import type { IOperationRunnerContext } from '../IOperationRunner';
import type { IOperationExecutionResult } from '../IOperationExecutionResult';

/**
* Helper function to create a minimal mock record for testing the createEnvironmentForOperation hook
*/
function createMockRecord(operation: Operation): IOperationRunnerContext & IOperationExecutionResult {
return {
operation,
environment: undefined
} as IOperationRunnerContext & IOperationExecutionResult;
}

describe(TrimRushEnvironmentVariablesPlugin.name, () => {
it('should remove RUSH_-prefixed environment variables while preserving others', async () => {
const rushJsonFile: string = path.resolve(__dirname, `../../test/parameterIgnoringRepo/rush.json`);
const commandLineJsonFile: string = path.resolve(
__dirname,
`../../test/parameterIgnoringRepo/common/config/rush/command-line.json`
);

const rushConfiguration = RushConfiguration.loadFromConfigurationFile(rushJsonFile);
const commandLineJson: ICommandLineJson = await JsonFile.loadAsync(commandLineJsonFile);

const commandLineConfiguration = new CommandLineConfiguration(commandLineJson);
const buildCommand: IPhasedCommandConfig = commandLineConfiguration.commands.get(
'build'
)! as IPhasedCommandConfig;

const fakeCreateOperationsContext: Pick<
ICreateOperationsContext,
'phaseSelection' | 'projectSelection' | 'projectConfigurations' | 'rushConfiguration'
> = {
phaseSelection: buildCommand.phases,
projectSelection: new Set(rushConfiguration.projects),
projectConfigurations: new Map(),
rushConfiguration
};

const hooks: PhasedCommandHooks = new PhasedCommandHooks();

// Apply plugins
new PhasedOperationPlugin().apply(hooks);
new ShellOperationRunnerPlugin().apply(hooks);
new TrimRushEnvironmentVariablesPlugin().apply(hooks);

const operations: Set<Operation> = await hooks.createOperationsAsync.promise(
new Set(),
fakeCreateOperationsContext as ICreateOperationsContext
);

// Set up a mock graph and invoke onGraphCreatedAsync so the plugin registers its graph hooks
const graphHooks: OperationGraphHooks = new OperationGraphHooks();
const fakeGraph: IOperationGraph = { hooks: graphHooks } as IOperationGraph;
await hooks.onGraphCreatedAsync.promise(fakeGraph, fakeCreateOperationsContext as IOperationGraphContext);

const operation = Array.from(operations)[0];
expect(operation).toBeDefined();

const mockRecord = createMockRecord(operation);

const initialEnvironment: IEnvironment = {
...process.env,
RUSH_TEMP_FOLDER: 'some-temp-folder',
rush_someMixedCaseVar: 'should also be trimmed',
RUSHSTACK_FILE_ERROR_BASE_FOLDER: 'should be preserved',
PATH: process.env.PATH,
SOME_OTHER_VAR: 'should be preserved'
};

const env: IEnvironment = graphHooks.createEnvironmentForOperation.call(initialEnvironment, mockRecord);

expect(env.RUSH_TEMP_FOLDER).toBeUndefined();
expect(env.rush_someMixedCaseVar).toBeUndefined();
expect(env.RUSHSTACK_FILE_ERROR_BASE_FOLDER).toBe('should be preserved');
expect(env.SOME_OTHER_VAR).toBe('should be preserved');
});
});
4 changes: 4 additions & 0 deletions libraries/rush-lib/src/schemas/experiments.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,10 @@
"useDirectFileTransfersForBuildCache": {
"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.",
"type": "boolean"
},
"trimRushEnvironmentVariablesForOperations": {
Comment thread
iclanton marked this conversation as resolved.
"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.",
"type": "boolean"
}
},
"additionalProperties": false
Expand Down