feat(gatekeeper): support safe fd redirects in chain parser (#95)

* feat(gatekeeper): support safe fd redirects in chain parser

Allow fd-to-fd redirects (2>&1, >&2, <&3, >&-) and /dev/null
redirects (>/dev/null, 2>/dev/null, >>/dev/null) in the chain
parser instead of immediately classifying them as unparseable.
Arbitrary file redirects (> file.txt, >> file.txt, < file.txt)
remain blocked.

* chore: update bun.lock

* chore: apply AI code review suggestions

- Add bounds check to whitespace-skipping while loop in /dev/null redirect parser (gemini-code-assist)
- Add word-boundary check after /dev/null match to prevent approving paths like /dev/nullicious (cubic-dev-ai P1)
- Add regression test for /dev/nullicious boundary case

* chore: apply AI code review suggestions

Replace ambiguous splitChainedCommands assertion for /dev/nullicious with
evaluate(bash(...)) to explicitly verify the unsafe redirect is passed through

* chore: update bun.lock after rebase

Author: amondnet

plugins/gatekeeper/dist/pre-tool-use.js

@@ -2,6 +2,9 @@
 import process from "node:process";
 
 // src/chain-parser.ts
+function isDigit(ch) {
+  return ch !== undefined && ch >= "0" && ch <= "9";
+}
 function parseChainedCommand(cmd) {
   if (/\$\(|`|\n|<\(|>\(/.test(cmd)) {
     return { kind: "unparseable" };
@@ -37,7 +40,33 @@ function parseChainedCommand(cmd) {
         current += ch;
         continue;
       }
-      if (ch === "<" || ch === ">") {
+      if (ch === ">" || ch === "<") {
+        if (ch === "<") {
+          if (cmd[i + 1] === "&" && (isDigit(cmd[i + 2]) || cmd[i + 2] === "-")) {
+            current += cmd.slice(i, i + 3);
+            i += 2;
+            continue;
+          }
+          return { kind: "unparseable" };
+        }
+        let j = i + 1;
+        if (cmd[j] === ">") {
+          j++;
+        }
+        if (cmd[j] === "&" && (isDigit(cmd[j + 1]) || cmd[j + 1] === "-")) {
+          current += cmd.slice(i, j + 2);
+          i = j + 1;
+          continue;
+        }
+        let k = j;
+        while (k < cmd.length && (cmd[k] === " " || cmd[k] === "\t"))
+          k++;
+        const afterNull = cmd[k + 9];
+        if (cmd.slice(k, k + 9) === "/dev/null" && (afterNull === undefined || afterNull === " " || afterNull === "\t" || afterNull === ";" || afterNull === "&" || afterNull === "|" || afterNull === ">" || afterNull === "<")) {
+          current += cmd.slice(i, k + 9);
+          i = k + 8;
+          continue;
+        }
         return { kind: "unparseable" };
       }
       const next = cmd[i + 1];

plugins/gatekeeper/src/chain-parser.ts

@@ -6,6 +6,10 @@
  * caused the two functions to diverge and produce inconsistent security decisions.
  */
 
+function isDigit(ch: string | undefined): boolean {
+  return ch !== undefined && ch >= '0' && ch <= '9'
+}
+
 export type ParseResult
   /**
    * Reject and send to AI review. Covers: subshell, backtick, newline,
@@ -23,7 +27,9 @@ export type ParseResult
  *
  * Security invariants guaranteed by this function:
  * - $(), backtick, newline, <(), >() → unparseable  (subshell / process substitution)
- * - Unquoted < or >               → unparseable  (file redirect: arbitrary write/read)
+ * - Unquoted < or > to files      → unparseable  (file redirect: arbitrary write/read)
+ * - Safe fd redirects (2>&1, >&2, <&3, >&-) → allowed (fd-to-fd, no file I/O)
+ * - >/dev/null, >>/dev/null, N>/dev/null    → allowed (discard output, safe)
  * - ||                            → unparseable  (right side runs on LEFT failure; semantics differ)
  * - single |                      → unparseable  (pipe: stdout→stdin, context-dependent)
  * - Lone &                        → unparseable  (background execution; different semantics)
@@ -78,8 +84,48 @@ export function parseChainedCommand(cmd: string): ParseResult {
         continue
       }
 
-      // Redirect operators outside quotes → potential arbitrary file write/read
-      if (ch === '<' || ch === '>') {
+      // Redirect operators outside quotes — allow safe fd redirects and /dev/null
+      if (ch === '>' || ch === '<') {
+        // Check if preceded by a digit (e.g., the '2' in '2>&1' or '2>/dev/null')
+        // We already consumed that digit into `current`, so just note it for context.
+
+        if (ch === '<') {
+          // <&N or <&- → fd input redirect (safe)
+          if (cmd[i + 1] === '&' && (isDigit(cmd[i + 2]) || cmd[i + 2] === '-')) {
+            current += cmd.slice(i, i + 3)
+            i += 2
+            continue
+          }
+          // Any other < → unsafe
+          return { kind: 'unparseable' }
+        }
+
+        // ch === '>'
+        let j = i + 1
+
+        // >> (append) — advance past second >
+        if (cmd[j] === '>') {
+          j++
+        }
+
+        // >&N or >&- → fd-to-fd redirect (safe)
+        if (cmd[j] === '&' && (isDigit(cmd[j + 1]) || cmd[j + 1] === '-')) {
+          current += cmd.slice(i, j + 2)
+          i = j + 1
+          continue
+        }
+
+        // > /dev/null or >> /dev/null (with optional whitespace)
+        let k = j
+        while (k < cmd.length && (cmd[k] === ' ' || cmd[k] === '\t')) k++
+        const afterNull = cmd[k + 9]
+        if (cmd.slice(k, k + 9) === '/dev/null' && (afterNull === undefined || afterNull === ' ' || afterNull === '\t' || afterNull === ';' || afterNull === '&' || afterNull === '|' || afterNull === '>' || afterNull === '<')) {
+          current += cmd.slice(i, k + 9)
+          i = k + 8
+          continue
+        }
+
+        // Any other > or >> → unsafe (arbitrary file write)
         return { kind: 'unparseable' }
       }
 

plugins/gatekeeper/src/pre-tool-use.test.ts

@@ -75,11 +75,40 @@ describe('splitChainedCommands', () => {
     expect(splitChainedCommands('echo hello > >(tee output.txt)')).toBeNull()
   })
 
-  test('should return null for redirect operators outside quotes', () => {
+  test('should return null for unsafe redirect operators outside quotes', () => {
     expect(splitChainedCommands('echo test > file.txt')).toBeNull()
     expect(splitChainedCommands('echo test >> file.txt')).toBeNull()
     expect(splitChainedCommands('cat < file.txt')).toBeNull()
+  })
+
+  test('should treat safe fd redirects as part of the command (single)', () => {
+    // fd-to-fd redirects
+    expect(splitChainedCommands('echo test 2>&1')).toBeNull() // null = single (no chain ops)
+    expect(splitChainedCommands('echo test >&2')).toBeNull()
+    expect(splitChainedCommands('cmd 2>&-')).toBeNull()
+    expect(splitChainedCommands('cmd <&3')).toBeNull()
+    expect(splitChainedCommands('cmd <&-')).toBeNull()
+    // /dev/null redirects
     expect(splitChainedCommands('echo test 2>/dev/null')).toBeNull()
+    expect(splitChainedCommands('echo test >/dev/null')).toBeNull()
+    expect(splitChainedCommands('echo test >>/dev/null')).toBeNull()
+    expect(splitChainedCommands('echo test 2>>/dev/null')).toBeNull()
+    // /dev/null with whitespace
+    expect(splitChainedCommands('echo test > /dev/null')).toBeNull()
+    expect(splitChainedCommands('echo test 2> /dev/null')).toBeNull()
+    // /dev/null — word-boundary check: must not approve /dev/nullicious etc.
+    expect(evaluate(bash('echo test > /dev/nullicious'))).toBeNull() // unsafe redirect must remain passthrough
+  })
+
+  test('should split chains containing safe fd redirects', () => {
+    expect(splitChainedCommands('cd /path && bun test 2>&1')).toEqual([
+      'cd /path',
+      'bun test 2>&1',
+    ])
+    expect(splitChainedCommands('cmd1 2>/dev/null && cmd2')).toEqual([
+      'cmd1 2>/dev/null',
+      'cmd2',
+    ])
   })
 
   test('should return null when no chain operators present', () => {
@@ -711,12 +740,26 @@ describe('chain parsing: safe chains are allowed in Layer 1', () => {
     expectPassthrough(bash('npm test || npm install'))
   })
 
-  test('should passthrough redirect operators', () => {
+  test('should passthrough unsafe redirect operators', () => {
     expectPassthrough(bash('echo test > file.txt'))
     expectPassthrough(bash('echo test >> file.txt'))
     expectPassthrough(bash('cat < file.txt'))
   })
 
+  test('should allow commands with safe fd redirects', () => {
+    expectAllow(bash('echo test 2>&1'))
+    expectAllow(bash('echo test 2>/dev/null'))
+    expectAllow(bash('echo test >/dev/null'))
+  })
+
+  test('should allow chain with safe fd redirects', () => {
+    expectAllow(bash('npm test && bun test --bail --timeout 120000 src/test.ts 2>&1'))
+  })
+
+  test('should passthrough pipe even with fd redirect before it', () => {
+    expectPassthrough(bash('ls 2>&1 | grep foo'))
+  })
+
   test('should passthrough process substitution', () => {
     expectPassthrough(bash('diff <(cat /etc/passwd) /etc/hosts'))
     expectPassthrough(bash('cat <(whoami)'))