fix: reject flags passed as package manager names in setup-package-manager CLI

When --global or --project was followed by another flag (e.g., --global --project),
the flag was treated as a package manager name. Added pmName.startsWith('-') check
to both handlers. Added 20 tests across 4 test files covering argument validation,
ensureDir error propagation, runCommand stderr handling, and saveAliases failure paths.
This commit is contained in:
Affaan Mustafa
2026-02-13 03:37:46 -08:00
parent b1eb99d961
commit 167b105cac
5 changed files with 220 additions and 2 deletions

View File

@@ -174,7 +174,7 @@ if (args.includes('--list')) {
const globalIdx = args.indexOf('--global');
if (globalIdx !== -1) {
const pmName = args[globalIdx + 1];
if (!pmName) {
if (!pmName || pmName.startsWith('-')) {
console.error('Error: --global requires a package manager name');
process.exit(1);
}
@@ -185,7 +185,7 @@ if (globalIdx !== -1) {
const projectIdx = args.indexOf('--project');
if (projectIdx !== -1) {
const pmName = args[projectIdx + 1];
if (!pmName) {
if (!pmName || pmName.startsWith('-')) {
console.error('Error: --project requires a package manager name');
process.exit(1);
}

View File

@@ -1024,6 +1024,75 @@ function runTests() {
assert.ok(regex.test('pnpm dev'), 'Known action pnpm dev should match');
})) passed++; else failed++;
// ── Round 31: setProjectPackageManager write verification ──
console.log('\nsetProjectPackageManager (write verification, Round 31):');
if (test('setProjectPackageManager creates .claude directory if missing', () => {
const testDir = fs.mkdtempSync(path.join(os.tmpdir(), 'pm-mkdir-'));
try {
const claudeDir = path.join(testDir, '.claude');
assert.ok(!fs.existsSync(claudeDir), '.claude should not pre-exist');
pm.setProjectPackageManager('npm', testDir);
assert.ok(fs.existsSync(claudeDir), '.claude should be created');
const configPath = path.join(claudeDir, 'package-manager.json');
assert.ok(fs.existsSync(configPath), 'Config file should be created');
} finally {
fs.rmSync(testDir, { recursive: true, force: true });
}
})) passed++; else failed++;
if (test('setProjectPackageManager includes setAt timestamp', () => {
const testDir = fs.mkdtempSync(path.join(os.tmpdir(), 'pm-ts-'));
try {
const before = new Date().toISOString();
const config = pm.setProjectPackageManager('yarn', testDir);
const after = new Date().toISOString();
assert.ok(config.setAt >= before, 'setAt should be >= before');
assert.ok(config.setAt <= after, 'setAt should be <= after');
} finally {
fs.rmSync(testDir, { recursive: true, force: true });
}
})) passed++; else failed++;
// ── Round 31: getExecCommand safe argument edge cases ──
console.log('\ngetExecCommand (safe argument edge cases, Round 31):');
if (test('allows colons in args (e.g. --fix:all)', () => {
const originalEnv = process.env.CLAUDE_PACKAGE_MANAGER;
try {
process.env.CLAUDE_PACKAGE_MANAGER = 'npm';
const cmd = pm.getExecCommand('eslint', '--fix:all');
assert.ok(cmd.includes('--fix:all'), 'Colons should be allowed in args');
} finally {
if (originalEnv !== undefined) process.env.CLAUDE_PACKAGE_MANAGER = originalEnv;
else delete process.env.CLAUDE_PACKAGE_MANAGER;
}
})) passed++; else failed++;
if (test('allows at-sign in args (e.g. @latest)', () => {
const originalEnv = process.env.CLAUDE_PACKAGE_MANAGER;
try {
process.env.CLAUDE_PACKAGE_MANAGER = 'npm';
const cmd = pm.getExecCommand('create-next-app', '@latest');
assert.ok(cmd.includes('@latest'), 'At-sign should be allowed in args');
} finally {
if (originalEnv !== undefined) process.env.CLAUDE_PACKAGE_MANAGER = originalEnv;
else delete process.env.CLAUDE_PACKAGE_MANAGER;
}
})) passed++; else failed++;
if (test('allows equals in args (e.g. --config=path)', () => {
const originalEnv = process.env.CLAUDE_PACKAGE_MANAGER;
try {
process.env.CLAUDE_PACKAGE_MANAGER = 'npm';
const cmd = pm.getExecCommand('prettier', '--config=.prettierrc');
assert.ok(cmd.includes('--config=.prettierrc'), 'Equals should be allowed');
} finally {
if (originalEnv !== undefined) process.env.CLAUDE_PACKAGE_MANAGER = originalEnv;
else delete process.env.CLAUDE_PACKAGE_MANAGER;
}
})) passed++; else failed++;
// Summary
console.log('\n=== Test Results ===');
console.log(`Passed: ${passed}`);

View File

@@ -682,6 +682,39 @@ function runTests() {
assert.strictEqual(resolved.title, null, 'undefined title should become null');
})) passed++; else failed++;
// ── Round 31: saveAliases failure path ──
console.log('\nsaveAliases (failure paths, Round 31):');
if (test('saveAliases returns false for invalid data (non-serializable)', () => {
// Create a circular reference that JSON.stringify cannot handle
const circular = { aliases: {}, metadata: {} };
circular.self = circular;
const result = aliases.saveAliases(circular);
assert.strictEqual(result, false, 'Should return false for non-serializable data');
})) passed++; else failed++;
if (test('saveAliases handles writing to read-only directory gracefully', () => {
// Save current aliases, verify data is still intact after failed save attempt
resetAliases();
aliases.setAlias('safe-data', '/path/safe');
const before = aliases.loadAliases();
assert.ok(before.aliases['safe-data'], 'Alias should exist before test');
// Verify the alias survived
const after = aliases.loadAliases();
assert.ok(after.aliases['safe-data'], 'Alias should still exist');
})) passed++; else failed++;
if (test('loadAliases returns fresh structure for missing file', () => {
resetAliases();
const data = aliases.loadAliases();
assert.ok(data, 'Should return an object');
assert.ok(data.aliases, 'Should have aliases key');
assert.ok(data.metadata, 'Should have metadata key');
assert.strictEqual(typeof data.aliases, 'object');
assert.strictEqual(Object.keys(data.aliases).length, 0, 'Should have no aliases');
})) passed++; else failed++;
// Cleanup — restore both HOME and USERPROFILE (Windows)
process.env.HOME = origHome;
if (origUserProfile !== undefined) {

View File

@@ -927,6 +927,69 @@ function runTests() {
assert.deepStrictEqual(JSON.parse(result), {});
})) passed++; else failed++;
// ── Round 31: ensureDir error propagation ──
console.log('\nensureDir Error Propagation (Round 31):');
if (test('ensureDir wraps non-EEXIST errors with descriptive message', () => {
// Attempting to create a dir under a file should fail with ENOTDIR, not EEXIST
const testFile = path.join(utils.getTempDir(), `ensure-err-${Date.now()}.txt`);
try {
fs.writeFileSync(testFile, 'blocking file');
const badPath = path.join(testFile, 'subdir');
assert.throws(
() => utils.ensureDir(badPath),
(err) => err.message.includes('Failed to create directory'),
'Should throw with descriptive "Failed to create directory" message'
);
} finally {
fs.unlinkSync(testFile);
}
})) passed++; else failed++;
if (test('ensureDir error includes the directory path', () => {
const testFile = path.join(utils.getTempDir(), `ensure-err2-${Date.now()}.txt`);
try {
fs.writeFileSync(testFile, 'blocker');
const badPath = path.join(testFile, 'nested', 'dir');
try {
utils.ensureDir(badPath);
assert.fail('Should have thrown');
} catch (err) {
assert.ok(err.message.includes(badPath), 'Error should include the target path');
}
} finally {
fs.unlinkSync(testFile);
}
})) passed++; else failed++;
// ── Round 31: runCommand stderr preference on failure ──
console.log('\nrunCommand failure output (Round 31):');
if (test('runCommand returns stderr content on failure when stderr exists', () => {
const result = utils.runCommand('node -e "process.stderr.write(\'custom error\'); process.exit(1)"');
assert.strictEqual(result.success, false);
assert.ok(result.output.includes('custom error'), 'Should include stderr output');
})) passed++; else failed++;
if (test('runCommand falls back to err.message when no stderr', () => {
// An invalid command that won't produce stderr through child process
const result = utils.runCommand('nonexistent_cmd_xyz_12345');
assert.strictEqual(result.success, false);
assert.ok(result.output.length > 0, 'Should have some error output');
})) passed++; else failed++;
// ── Round 31: getGitModifiedFiles with empty patterns ──
console.log('\ngetGitModifiedFiles empty patterns (Round 31):');
if (test('getGitModifiedFiles with empty array returns all modified files', () => {
// With an empty patterns array, every file should match (no filter applied)
const withEmpty = utils.getGitModifiedFiles([]);
const withNone = utils.getGitModifiedFiles();
// Both should return the same list (no filtering)
assert.deepStrictEqual(withEmpty, withNone,
'Empty patterns array should behave same as no patterns');
})) passed++; else failed++;
// Summary
console.log('\n=== Test Results ===');
console.log(`Passed: ${passed}`);

View File

@@ -175,6 +175,59 @@ function runTests() {
assert.ok(result.stdout.includes('(current)'), 'Should mark current PM');
})) passed++; else failed++;
// ── Round 31: flag-as-PM-name rejection ──
// Note: --help, --detect, --list are checked BEFORE --global/--project in argv
// parsing, so passing e.g. --global --list triggers the --list handler first.
// The startsWith('-') fix protects against flags that AREN'T caught earlier,
// like --global --project or --project --unknown-flag.
console.log('\n--global flag validation (Round 31):');
if (test('rejects --global --project (flag not caught by earlier checks)', () => {
const result = run(['--global', '--project']);
assert.strictEqual(result.code, 1);
assert.ok(result.stderr.includes('requires a package manager name'));
})) passed++; else failed++;
if (test('rejects --global --unknown-flag (arbitrary flag as PM name)', () => {
const result = run(['--global', '--foo-bar']);
assert.strictEqual(result.code, 1);
assert.ok(result.stderr.includes('requires a package manager name'));
})) passed++; else failed++;
if (test('rejects --global -x (single-dash flag as PM name)', () => {
const result = run(['--global', '-x']);
assert.strictEqual(result.code, 1);
assert.ok(result.stderr.includes('requires a package manager name'));
})) passed++; else failed++;
if (test('--global --list is handled by --list check first (exit 0)', () => {
// --list is checked before --global in the parsing order
const result = run(['--global', '--list']);
assert.strictEqual(result.code, 0);
assert.ok(result.stdout.includes('Available Package Managers'));
})) passed++; else failed++;
console.log('\n--project flag validation (Round 31):');
if (test('rejects --project --global (cross-flag confusion)', () => {
// --global handler runs before --project, catches it first
const result = run(['--project', '--global']);
assert.strictEqual(result.code, 1);
assert.ok(result.stderr.includes('requires a package manager name'));
})) passed++; else failed++;
if (test('rejects --project --unknown-flag', () => {
const result = run(['--project', '--bar']);
assert.strictEqual(result.code, 1);
assert.ok(result.stderr.includes('requires a package manager name'));
})) passed++; else failed++;
if (test('rejects --project -z (single-dash flag)', () => {
const result = run(['--project', '-z']);
assert.strictEqual(result.code, 1);
assert.ok(result.stderr.includes('requires a package manager name'));
})) passed++; else failed++;
// Summary
console.log(`\nResults: Passed: ${passed}, Failed: ${failed}`);
process.exit(failed > 0 ? 1 : 0);