-
Notifications
You must be signed in to change notification settings - Fork 697
[rush] Upgrade tar to 7.5.6 to fix security vulnerability #5573
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
babf688
Initial plan
Copilot 32cf333
Upgrade tar from 6.2.1 to 7.5.6 to fix security vulnerability
Copilot 58eb627
Add integration tests for npm and yarn modes with tar 7.x
Copilot fde5bc4
Translate bash test scripts to TypeScript using @rushstack/node-core-…
Copilot 79f01d2
Address code review feedback - use rig.json, Terminal, async operatio…
Copilot 8abfd26
Address final code review feedback and add tests to CI/CD pipelines
Copilot 42c22f8
Fix CI pipeline steps and move @microsoft/rush to nonbrowser-approved…
Copilot 53c74ef
Add changelog entry for tar upgrade
Copilot c7e297a
Fix changelog entry - use @microsoft/rush with type 'none' for lockst…
Copilot 35391fd
Update repo README with new integration test project
Copilot 9497ef0
Fix Windows path handling in testBuiltCodeAsync
Copilot c3f406c
Address code review feedback - use ensureEmptyFolderAsync, verify sym…
Copilot 78a8208
Rush update.
iclanton 55d7166
Fix Windows path issue - use os.tmpdir() instead of hardcoded /tmp
Copilot File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
97 changes: 97 additions & 0 deletions
97
build-tests/rush-package-manager-integration-test/README.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,97 @@ | ||
| # Rush Package Manager Integration Tests | ||
|
|
||
| This directory contains integration tests for verifying Rush works correctly with different package managers after the tar 7.x upgrade. | ||
|
|
||
| ## Background | ||
|
|
||
| Rush's npm and yarn modes use temp project tarballs (stored in `common/temp/projects/`) to simulate package installations. The tar library is used to: | ||
| 1. **Create** tarballs from temp project folders (`TempProjectHelper.createTempProjectTarball`) | ||
| 2. **Extract** tarballs during the linking process (`NpmLinkManager._linkProjectAsync`) | ||
|
|
||
| These tests ensure the tar 7.x upgrade works correctly with these workflows. | ||
|
|
||
| ## Tests | ||
|
|
||
| The test suite is written in TypeScript using `@rushstack/node-core-library` for cross-platform compatibility. | ||
|
|
||
| ### testNpmMode.ts | ||
| Tests Rush npm mode by: | ||
| - Initializing a Rush repo with `npmVersion` configured | ||
| - Creating two projects with dependencies | ||
| - Running `rush update` | ||
| - Running `rush install` | ||
| - Running `rush build` (verifies everything works end-to-end) | ||
|
|
||
| ### testYarnMode.ts | ||
| Tests Rush yarn mode by: | ||
| - Initializing a Rush repo with `yarnVersion` configured | ||
| - Creating two projects with dependencies | ||
| - Running `rush update` | ||
| - Running `rush install` | ||
| - Running `rush build` (verifies everything works end-to-end) | ||
|
|
||
| ## Prerequisites | ||
|
|
||
| Before running these tests: | ||
| 1. Build Rush locally: `rush build --to rush` | ||
| 2. Build this test project: `rush build --to rush-package-manager-integration-test` | ||
| 3. Ensure you have Node.js 18+ installed | ||
|
|
||
| ## Running the Tests | ||
|
|
||
| ```bash | ||
| # Build the test project first | ||
| cd build-tests/rush-package-manager-integration-test | ||
| rush build | ||
|
|
||
| # Run all tests | ||
| npm run test | ||
| ``` | ||
|
|
||
| Or from the root of the repo: | ||
| ```bash | ||
| rush build --to rush-package-manager-integration-test | ||
| cd build-tests/rush-package-manager-integration-test | ||
| npm run test | ||
| ``` | ||
|
|
||
| ## What Gets Tested | ||
|
|
||
| These integration tests verify: | ||
| - ✓ Temp project tarballs are created correctly using tar 7.x | ||
| - ✓ Tarballs are extracted correctly during `rush install` | ||
| - ✓ File permissions are preserved (tar filter function works) | ||
| - ✓ Dependencies are linked properly between projects | ||
| - ✓ The complete workflow (update → install → build) succeeds | ||
| - ✓ Built code executes correctly | ||
|
|
||
| ## Test Output | ||
|
|
||
| Each test creates a temporary Rush repository in `/tmp/rush-package-manager-test/`: | ||
| - `/tmp/rush-package-manager-test/npm-test-repo/` - npm mode test repository | ||
| - `/tmp/rush-package-manager-test/yarn-test-repo/` - yarn mode test repository | ||
|
|
||
| These directories are cleaned up at the start of each test run. | ||
|
|
||
| ## Implementation | ||
|
|
||
| The tests use: | ||
| - **TypeScript** for type safety and better IDE support | ||
| - **@rushstack/node-core-library** for cross-platform file operations and process execution | ||
| - **TestHelper class** to encapsulate common test operations | ||
| - Modular test functions that can be run independently or together | ||
|
|
||
| ## Related Code | ||
|
|
||
| The tar library is used in: | ||
| - `libraries/rush-lib/src/logic/TempProjectHelper.ts` - Creates tarballs | ||
| - `libraries/rush-lib/src/logic/npm/NpmLinkManager.ts` - Extracts tarballs | ||
|
|
||
| ## Troubleshooting | ||
|
|
||
| If tests fail: | ||
| 1. Check that Rush built successfully: `rush build --to rush` | ||
| 2. Check that the test project built: `rush build --to rush-package-manager-integration-test` | ||
| 3. Verify Node.js version: `node --version` (should be 18+) | ||
| 4. Look for error messages in the test output | ||
| 5. Inspect the temp test repo: `ls -la temp/npm-test-repo/common/temp/projects/` |
7 changes: 7 additions & 0 deletions
7
build-tests/rush-package-manager-integration-test/config/rig.json
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| { | ||
| // The "rig.json" file directs tools to look for their config files in an external package. | ||
| // Documentation for this system: https://www.npmjs.com/package/@rushstack/rig-package | ||
| "$schema": "https://developer.microsoft.com/json-schemas/rig-package/rig.schema.json", | ||
|
|
||
| "rigPackageName": "local-node-rig" | ||
| } |
16 changes: 16 additions & 0 deletions
16
build-tests/rush-package-manager-integration-test/eslint.config.js
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. | ||
| // See LICENSE in the project root for license information. | ||
|
|
||
| const nodeProfile = require('local-node-rig/profiles/default/includes/eslint/flat/profile/node'); | ||
|
|
||
| module.exports = [ | ||
| ...nodeProfile, | ||
| { | ||
| files: ['**/*.ts', '**/*.tsx'], | ||
| languageOptions: { | ||
| parserOptions: { | ||
| tsconfigRootDir: __dirname | ||
| } | ||
| } | ||
| } | ||
| ]; |
20 changes: 20 additions & 0 deletions
20
build-tests/rush-package-manager-integration-test/package.json
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| { | ||
| "name": "rush-package-manager-integration-test", | ||
| "version": "1.0.0", | ||
| "private": true, | ||
| "description": "Integration tests for non-pnpm package managers in Rush.", | ||
| "license": "MIT", | ||
| "scripts": { | ||
| "_phase:build": "heft build --clean", | ||
| "build": "heft build --clean", | ||
| "test": "node lib/runTests.js" | ||
| }, | ||
| "devDependencies": { | ||
| "@microsoft/rush": "workspace:*", | ||
| "@microsoft/rush-lib": "workspace:*", | ||
| "@rushstack/heft": "workspace:*", | ||
| "@rushstack/node-core-library": "workspace:*", | ||
| "@rushstack/terminal": "workspace:*", | ||
| "local-node-rig": "workspace:*" | ||
| } | ||
| } |
205 changes: 205 additions & 0 deletions
205
build-tests/rush-package-manager-integration-test/src/TestHelper.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,205 @@ | ||
| // 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 * as child_process from 'node:child_process'; | ||
|
|
||
| import { FileSystem, Executable, JsonFile, type JsonObject } from '@rushstack/node-core-library'; | ||
| import type { ITerminal } from '@rushstack/terminal'; | ||
|
|
||
| /** | ||
| * Helper class for running integration tests with Rush package managers | ||
| */ | ||
| export class TestHelper { | ||
| private readonly _rushBinPath: string; | ||
| private readonly _terminal: ITerminal; | ||
|
|
||
| public constructor(terminal: ITerminal) { | ||
| this._terminal = terminal; | ||
| // Resolve rush bin path from @microsoft/rush dependency | ||
| this._rushBinPath = require.resolve('@microsoft/rush/lib/start-dev'); | ||
| } | ||
|
|
||
| /** | ||
| * Execute a Rush command using the locally-built Rush | ||
| */ | ||
| public async executeRushAsync(args: string[], workingDirectory: string): Promise<void> { | ||
| this._terminal.writeLine(`Executing: ${process.argv0} ${this._rushBinPath} ${args.join(' ')}`); | ||
|
|
||
| const childProcess: child_process.ChildProcess = Executable.spawn( | ||
| process.argv0, | ||
| [this._rushBinPath, ...args], | ||
| { | ||
| currentWorkingDirectory: workingDirectory, | ||
| stdio: 'inherit' | ||
| } | ||
| ); | ||
|
|
||
| await Executable.waitForExitAsync(childProcess, { | ||
| throwOnNonZeroExitCode: true | ||
| }); | ||
| } | ||
|
|
||
| /** | ||
| * Create a test Rush repository with the specified package manager | ||
| */ | ||
| public async createTestRepoAsync( | ||
| testRepoPath: string, | ||
| packageManagerType: 'npm' | 'yarn', | ||
| packageManagerVersion: string | ||
| ): Promise<void> { | ||
| // Clean up previous test run and create empty test repo directory | ||
| this._terminal.writeLine(`Creating test repository at ${testRepoPath}...`); | ||
| await FileSystem.ensureEmptyFolderAsync(testRepoPath); | ||
|
|
||
| // Initialize Rush repo | ||
| this._terminal.writeLine('Initializing Rush repo...'); | ||
| await this.executeRushAsync(['init'], testRepoPath); | ||
|
|
||
| // Configure rush.json for the specified package manager | ||
| this._terminal.writeLine(`Configuring rush.json for ${packageManagerType} mode...`); | ||
| const rushJsonPath: string = path.join(testRepoPath, 'rush.json'); | ||
| const rushJson: JsonObject = await JsonFile.loadAsync(rushJsonPath); | ||
|
|
||
| // Update package manager configuration | ||
| if (packageManagerType === 'npm') { | ||
| delete rushJson.pnpmVersion; | ||
| delete rushJson.yarnVersion; | ||
| rushJson.npmVersion = packageManagerVersion; | ||
| } else if (packageManagerType === 'yarn') { | ||
| delete rushJson.pnpmVersion; | ||
| delete rushJson.npmVersion; | ||
| rushJson.yarnVersion = packageManagerVersion; | ||
| } | ||
|
|
||
| // Add test projects | ||
| rushJson.projects = [ | ||
| { | ||
| packageName: 'test-project-a', | ||
| projectFolder: 'projects/test-project-a' | ||
| }, | ||
| { | ||
| packageName: 'test-project-b', | ||
| projectFolder: 'projects/test-project-b' | ||
| } | ||
| ]; | ||
|
|
||
| // Update nodeSupportedVersionRange to match current environment | ||
| rushJson.nodeSupportedVersionRange = '>=18.0.0'; | ||
|
|
||
| await JsonFile.saveAsync(rushJson, rushJsonPath, { updateExistingFile: true }); | ||
| } | ||
|
|
||
| /** | ||
| * Create a test project with the specified configuration | ||
| */ | ||
| public async createTestProjectAsync( | ||
| testRepoPath: string, | ||
| projectName: string, | ||
| version: string, | ||
| dependencies: Record<string, string>, | ||
| buildScript: string | ||
| ): Promise<void> { | ||
| const projectPath: string = path.join(testRepoPath, 'projects', projectName); | ||
| await FileSystem.ensureFolderAsync(projectPath); | ||
|
|
||
| const packageJson: JsonObject = { | ||
| name: projectName, | ||
| version: version, | ||
| main: 'lib/index.js', | ||
| scripts: { | ||
| build: buildScript | ||
| }, | ||
| dependencies: dependencies | ||
| }; | ||
|
|
||
| await JsonFile.saveAsync(packageJson, path.join(projectPath, 'package.json')); | ||
| } | ||
|
|
||
| /** | ||
| * Verify that temp project tarballs were created | ||
| */ | ||
| public async verifyTempTarballsAsync(testRepoPath: string, projectNames: string[]): Promise<void> { | ||
| this._terminal.writeLine('\nVerifying temp project tarballs were created...'); | ||
| for (const projectName of projectNames) { | ||
| const tarballPath: string = path.join(testRepoPath, 'common/temp/projects', `${projectName}.tgz`); | ||
| if (!(await FileSystem.existsAsync(tarballPath))) { | ||
| throw new Error(`ERROR: ${projectName}.tgz was not created!`); | ||
| } | ||
| } | ||
| this._terminal.writeLine('✓ Temp project tarballs created successfully'); | ||
| } | ||
|
|
||
| /** | ||
| * Verify that dependencies are installed correctly | ||
| */ | ||
| public async verifyDependenciesAsync( | ||
| testRepoPath: string, | ||
| projectName: string, | ||
| expectedDependencies: string[] | ||
| ): Promise<void> { | ||
| this._terminal.writeLine('\nVerifying node_modules structure...'); | ||
| const projectPath: string = path.join(testRepoPath, 'projects', projectName); | ||
| const projectNodeModules: string = path.join(projectPath, 'node_modules'); | ||
|
|
||
| for (const dep of expectedDependencies) { | ||
| const depPath: string = path.join(projectNodeModules, dep); | ||
| if (!(await FileSystem.existsAsync(depPath))) { | ||
| throw new Error(`ERROR: ${dep} not found in ${projectName}!`); | ||
| } | ||
|
|
||
| // Verify symlinks resolve correctly for local dependencies | ||
| if (dep.startsWith('test-project-')) { | ||
| const depRealPath: string = await FileSystem.getRealPathAsync(depPath); | ||
| const expectedRealPath: string = path.join(testRepoPath, 'projects', dep); | ||
| if (depRealPath !== expectedRealPath) { | ||
| throw new Error( | ||
| `ERROR: Symlink for ${dep} does not resolve correctly!\n` + | ||
| `Expected: ${expectedRealPath}\n` + | ||
| `Actual: ${depRealPath}` | ||
| ); | ||
| } | ||
| } | ||
| } | ||
| this._terminal.writeLine('✓ Dependencies installed correctly'); | ||
| } | ||
|
|
||
| /** | ||
| * Verify that build outputs were created | ||
| */ | ||
| public async verifyBuildOutputsAsync(testRepoPath: string, projectNames: string[]): Promise<void> { | ||
| this._terminal.writeLine('\nVerifying build outputs...'); | ||
| for (const projectName of projectNames) { | ||
| const outputPath: string = path.join(testRepoPath, 'projects', projectName, 'lib/index.js'); | ||
| if (!(await FileSystem.existsAsync(outputPath))) { | ||
| throw new Error(`ERROR: ${projectName} build output not found!`); | ||
| } | ||
| } | ||
| this._terminal.writeLine('✓ Build completed successfully'); | ||
| } | ||
|
|
||
| /** | ||
| * Test that the built code executes correctly | ||
| */ | ||
| public async testBuiltCodeAsync(testRepoPath: string, projectName: string): Promise<void> { | ||
| this._terminal.writeLine('\nTesting built code...'); | ||
| const projectLib: string = path.join(testRepoPath, 'projects', projectName, 'lib/index.js'); | ||
|
|
||
| // Use forward slashes for require() path on all platforms | ||
| const projectLibPosix: string = projectLib.split(path.sep).join(path.posix.sep); | ||
|
|
||
| // Use Executable.spawnSync to capture output | ||
| const result: string = Executable.spawnSync( | ||
| process.argv0, | ||
| ['-e', `const b = require('${projectLibPosix}'); console.log(b.test());`], | ||
| { | ||
| currentWorkingDirectory: testRepoPath | ||
| } | ||
| ).stdout.toString(); | ||
|
|
||
| if (!result.includes('Using: Hello from A')) { | ||
| throw new Error('ERROR: Built code did not execute as expected!'); | ||
| } | ||
| this._terminal.writeLine('✓ Built code executes correctly'); | ||
| } | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.