Merge pull request #34 from kenryu42/fix/positional-args-parsing

fix: positional args parsing

Author: kenryu42

dist/bin/cc-safety-net.js

@@ -35,7 +35,7 @@ describe('getSystemInfo', () => {
     const sysInfo = await getSystemInfo();
     expect(sysInfo.bunVersion).toMatch(/^\d+\.\d+/);
     expect(sysInfo.platform).toContain(process.platform);
-  });
+  }, 15000);
 
   test('handles non-existent commands gracefully', async () => {
     const sysInfo = await getSystemInfo();
@@ -48,7 +48,7 @@ describe('getSystemInfo', () => {
     expect(sysInfo.geminiCliVersion === null || typeof sysInfo.geminiCliVersion === 'string').toBe(
       true,
     );
-  });
+  }, 15000);
 
   test('handles commands that exit with non-zero code', async () => {
     const failingFetcher = async (_args: string[]) => null;

dist/index.js

@@ -158,6 +158,106 @@ describe('explainCommand edge cases', () => {
     const result = explainCommand('git status', { cwd: '/tmp' });
     expect(result.result).toBe('allowed');
   });
+
+  test('rm redirect to dev null is allowed when target is safe', () => {
+    const result = explainCommand('rm -rf /tmp/foo 2>/dev/null');
+    expect(result.result).toBe('allowed');
+    expect(result.trace.steps).toContainEqual({
+      type: 'parse',
+      input: 'rm -rf /tmp/foo 2>/dev/null',
+      segments: [['rm', '-rf', '/tmp/foo']],
+    });
+  });
+
+  test('redirect target command substitution remains blocked', () => {
+    const result = explainCommand('echo x >$(git reset --hard)');
+    expect(result.result).toBe('blocked');
+    expect(result.reason).toContain('git reset --hard');
+  });
+
+  test('nested rm redirect to dev null is allowed in command substitution', () => {
+    const result = explainCommand('echo $(rm -rf /tmp/foo 2>/dev/null)');
+    expect(result.result).toBe('allowed');
+    expect(result.trace.steps).toContainEqual({
+      type: 'parse',
+      input: 'echo $(rm -rf /tmp/foo 2>/dev/null)',
+      segments: [['echo'], ['rm', '-rf', '/tmp/foo']],
+    });
+  });
+
+  test('numeric rm target before redirect is preserved in explain trace', () => {
+    const result = explainCommand('rm -rf 7 > /dev/null');
+    expect(result.result).toBe('allowed');
+    expect(result.trace.steps).toContainEqual({
+      type: 'parse',
+      input: 'rm -rf 7 > /dev/null',
+      segments: [['rm', '-rf', '7']],
+    });
+  });
+
+  test('attached io-number redirect is stripped from explain trace', () => {
+    const result = explainCommand('rm -rf 123>/dev/null');
+    expect(result.result).toBe('allowed');
+    expect(result.trace.steps).toContainEqual({
+      type: 'parse',
+      input: 'rm -rf 123>/dev/null',
+      segments: [['rm', '-rf']],
+    });
+  });
+
+  test('spaced numeric rm arg before redirect stays visible in explain trace', () => {
+    const result = explainCommand('rm -rf 123 >/dev/null');
+    expect(result.result).toBe('allowed');
+    expect(result.trace.steps).toContainEqual({
+      type: 'parse',
+      input: 'rm -rf 123 >/dev/null',
+      segments: [['rm', '-rf', '123']],
+    });
+  });
+
+  test('backticks inside arithmetic expansion remain blocked in explain trace', () => {
+    const result = explainCommand('echo $((`git reset --hard` + 1))');
+    expect(result.result).toBe('blocked');
+    expect(result.reason).toContain('git reset --hard');
+  });
+
+  test('process substitution remains blocked in explain trace', () => {
+    const result = explainCommand('echo <(git reset --hard)');
+    expect(result.result).toBe('blocked');
+    expect(result.reason).toContain('git reset --hard');
+    expect(result.trace.steps).toContainEqual({
+      type: 'parse',
+      input: 'echo <(git reset --hard)',
+      segments: [['echo'], ['git', 'reset', '--hard']],
+    });
+  });
+
+  test('quoted literal backticks in redirect target do not hide blocked args in explain trace', () => {
+    const result = explainCommand("git checkout >'file`name' -- foo");
+    expect(result.result).toBe('blocked');
+    expect(result.reason).toContain('git checkout --');
+    expect(result.trace.steps).toContainEqual({
+      type: 'parse',
+      input: "git checkout >'file`name' -- foo",
+      segments: [['git', 'checkout', '--', 'foo']],
+    });
+  });
+
+  test('single-quoted backticks in redirect targets stay literal in explain trace', () => {
+    const result = explainCommand("echo >'a`git reset --hard`b'");
+    expect(result.result).toBe('allowed');
+    expect(result.trace.steps).toContainEqual({
+      type: 'parse',
+      input: "echo >'a`git reset --hard`b'",
+      segments: [['echo']],
+    });
+  });
+
+  test('attached backtick substitutions outside redirect targets stay blocked in explain trace', () => {
+    const result = explainCommand('echo foo`git reset --hard`bar');
+    expect(result.result).toBe('blocked');
+    expect(result.reason).toContain('git reset --hard');
+  });
 });
 
 describe('explainCommand rm with home directory', () => {

src/core/shell.ts

@@ -202,6 +202,92 @@ describe('edge cases', () => {
     test('command substitution find without delete allowed', () => {
       assertAllowed('echo $(find . -name foo )');
     });
+
+    test('grouped subshell command substitution git reset hard blocked', () => {
+      assertBlocked('echo $( (git reset --hard) )', 'git reset --hard');
+    });
+
+    test('grouped subshell command substitution rm root blocked', () => {
+      assertBlocked('echo $( (rm -rf /) )', 'extremely dangerous');
+    });
+
+    test('command substitution in redirect target git reset hard blocked', () => {
+      assertBlocked('echo x >$(git reset --hard)', 'git reset --hard');
+    });
+
+    test('command substitution in redirect target rm root blocked', () => {
+      assertBlocked('echo x >$(rm -rf /)', 'extremely dangerous');
+    });
+
+    test('attached command substitution in redirect target git reset hard blocked', () => {
+      assertBlocked('echo x >file$(git reset --hard)', 'git reset --hard');
+    });
+
+    test('attached command substitution in redirect target rm root blocked', () => {
+      assertBlocked('echo x >$TMPDIR/$(rm -rf /)', 'extremely dangerous');
+    });
+
+    test('command substitution keeps arguments after redirects blocked', () => {
+      assertBlocked('echo $(rm -rf 2>/dev/null /)', 'extremely dangerous');
+    });
+
+    test('arithmetic expansion keeps nested git reset blocked', () => {
+      assertBlocked('echo $(( $(git reset --hard) + 1 ))', 'git reset --hard');
+    });
+
+    test('arithmetic expansion keeps nested rm root blocked', () => {
+      assertBlocked('echo $(( $(rm -rf /) + 1 ))', 'extremely dangerous');
+    });
+
+    test('arithmetic expansion with adjacent nested git reset blocked', () => {
+      assertBlocked('echo $((foo+$(git reset --hard)))', 'git reset --hard');
+    });
+
+    test('arithmetic expansion with adjacent nested rm root blocked', () => {
+      assertBlocked('echo $((1+$(rm -rf /)))', 'extremely dangerous');
+    });
+
+    test('arithmetic expansion with backticks keeps nested git reset blocked', () => {
+      assertBlocked('echo $((`git reset --hard` + 1))', 'git reset --hard');
+      assertBlocked('echo $((foo`git reset --hard`bar))', 'git reset --hard');
+    });
+
+    test('quoted arithmetic expressions that resemble guarded commands stay allowed', () => {
+      assertAllowed('echo "$(( rm -rf /x ))"');
+      assertAllowed('echo "$(( foo + bar ))"');
+    });
+
+    test('quoted backtick substitution in redirect target git reset hard blocked', () => {
+      assertBlocked('echo x >"`git reset --hard`"', 'git reset --hard');
+    });
+
+    test('bare backtick redirect target git reset hard blocked', () => {
+      assertBlocked('echo x >`git reset --hard`', 'git reset --hard');
+    });
+
+    test('bare backtick redirect target inside command substitution blocked', () => {
+      assertBlocked('echo $(echo x >`git reset --hard`)', 'git reset --hard');
+    });
+
+    test('process substitution git reset hard blocked', () => {
+      assertBlocked('echo <(git reset --hard)', 'git reset --hard');
+      assertBlocked('cat >(git reset --hard)', 'git reset --hard');
+      assertBlocked('echo x > >(git reset --hard)', 'git reset --hard');
+      assertBlocked('echo foo < <(git reset --hard)', 'git reset --hard');
+    });
+
+    test('quoted literal backticks in redirect targets do not hide blocked args', () => {
+      assertBlocked("git checkout >'file`name' -- foo", 'git checkout --');
+      assertBlocked("rm -rf >'file`name' /", 'extremely dangerous');
+    });
+
+    test('single-quoted backticks in redirect targets stay literal', () => {
+      assertAllowed("echo >'a`git reset --hard`b'");
+    });
+
+    test('attached backtick substitutions outside redirect targets stay blocked', () => {
+      assertBlocked('echo foo`git reset --hard`bar', 'git reset --hard');
+    });
   });
 
   describe('xargs', () => {
@@ -491,6 +577,10 @@ describe('edge cases', () => {
       assertBlocked('echo ok &>out && git reset --hard', 'git reset --hard');
     });
 
+    test('redirect before checkout path still blocks', () => {
+      assertBlocked('git checkout 2>/dev/null -- foo', 'git checkout --');
+    });
+
     test('pipe stderr and stdout split blocked', () => {
       assertBlocked('echo ok |& git reset --hard', 'git reset --hard');
     });