fix(core): treat a word-initial # as a comment when splitting shell commands

packages/core/src/permissions/permission-manager.test.ts

@@ -671,6 +671,94 @@ describe('splitCompoundCommand', () => {
       expect(splitCompoundCommand(command)).toEqual(['echo hi']);
     },
   );
+
+  // A word-initial `#` outside quotes opens a comment that runs to the end of
+  // the physical line, so every operator inside it is inert. Confirmed against
+  // bash: each of these traces a single `+ echo a`, and
+  // `bash -xc "echo 'a' # comment ; echo B"` traces a single `+ echo a`.
+  //
+  // The `'a\'` rows passed before the comment state existed too, but only by
+  // accident — the backslash inside the quotes was read as an escape that
+  // swallowed the closing `'`, so the scanner sat in an unterminated string
+  // that masked the operator. They keep passing once #11765 makes that
+  // backslash literal, because the comment then masks the operator for the
+  // right reason.
+  it.each([
+    [`echo 'a' # comment ; echo B`],
+    [`echo 'a\\' # note: use ; carefully`],
+    [`echo 'a\\' # trailing && touch /tmp/x`],
+    [`echo 'a\\' # trailing | touch /tmp/x`],
+    ['echo hi # nothing here'],
+    [`git status # don't ; echo B`],
+    ['echo a # c && rm -rf /tmp/x'],
+    ['echo a # c || rm -rf /tmp/x'],
+    ['echo a # c | rm -rf /tmp/x'],
+    ['echo a # c & rm -rf /tmp/x'],
+  ])(
+    'does not split %s, where the operator is inside a comment',
+    async (command) => {
+      expect(splitCompoundCommand(command)).toEqual([command]);
+    },
+  );
+
+  // Over-correction guard: `#` starts a comment only at the start of a word, so
+  // bash really does run two commands here.
+  it('still splits echo a#b ; echo B, where # is not at a word start', async () => {
+    expect(splitCompoundCommand('echo a#b ; echo B')).toEqual([
+      'echo a#b',
+      'echo B',
+    ]);
+  });
+
+  // A word starts after an operator as well as after whitespace:
+  // `bash -xc "echo a;#c ; rm -rf /tmp/x"` traces a single `+ echo a`. The
+  // leftover `#c …` segment is pre-existing behaviour — bash runs one command
+  // here, not two — and is harmless in the fail-closed direction.
+  it('reads a # straight after an operator as a comment', async () => {
+    expect(splitCompoundCommand('echo a;#c ; rm -rf /tmp/x')).toEqual([
+      'echo a',
+      '#c ; rm -rf /tmp/x',
+    ]);
+  });
+
+  // Inside quotes a `#` is literal, so the operator after the closing quote is
+  // a real boundary.
+  it('does not treat a quoted # as a comment', async () => {
+    expect(splitCompoundCommand(`echo '# c' ; rm -rf /tmp/x`)).toEqual([
+      `echo '# c'`,
+      'rm -rf /tmp/x',
+    ]);
+  });
+
+  // `#` at index 0 is at a word start, so a shebang line is a comment and the
+  // newline after it still bounds the real command.
+  it('treats a leading # as a comment', async () => {
+    expect(splitCompoundCommand('#!/bin/sh\necho hi')).toEqual([
+      '#!/bin/sh',
+      'echo hi',
+    ]);
+  });
+
+  // The comment ends at the *physical* newline, and that newline stays a
+  // boundary: `bash -xc $'echo hi # c\nrm -rf /tmp/x'` traces both commands, so
+  // the `rm` must keep its own segment and its own rule check.
+  it('keeps the newline that ends a comment as a boundary', async () => {
+    expect(splitCompoundCommand('echo hi # c\nrm -rf /tmp/x')).toEqual([
+      'echo hi # c',
+      'rm -rf /tmp/x',
+    ]);
+  });
+
+  // …and a backslash does not reach past that newline. bash does not continue a
+  // line inside a comment, so it runs the `rm` here; letting the existing
+  // `escaped` flag find the newline folded both commands into one segment and
+  // the `rm` lost its rule check.
+  it('does not let a trailing backslash extend a comment', async () => {
+    expect(splitCompoundCommand('echo hi # foo \\\nrm -rf /tmp/x')).toEqual([
+      'echo hi # foo \\',
+      'rm -rf /tmp/x',
+    ]);
+  });
 });
 
 // ─── splitCompoundCommandSegments ────────────────────────────────────────────
@@ -2363,6 +2451,63 @@ describe('PermissionManager', () => {
         'deny',
       );
     });
+
+    // #11815 — bash runs only the `echo` here, and `Bash(echo *)` covers it, so
+    // the commented-out tail must not drag the whole line into a prompt.
+    it('operator inside a comment: treated as single command', async () => {
+      pm = new PermissionManager(
+        makeConfig({
+          permissionsAllow: ['Bash(echo *)'],
+        }),
+      );
+      pm.initialize();
+      expect(
+        await pm.evaluate({
+          toolName: 'run_shell_command',
+          command: `echo 'a' # comment ; rm -rf /tmp/x`,
+        }),
+      ).toBe('allow');
+    });
+
+    // The comment stops at the physical newline and bash runs the `rm` on the
+    // next line, so the backslash before that newline must not extend the
+    // comment over it and cost the `rm` its deny check.
+    it('command after a comment line: deny still fires', async () => {
+      pm = new PermissionManager(
+        makeConfig({
+          permissionsDeny: ['Bash(rm *)'],
+        }),
+      );
+      pm.initialize();
+      expect(
+        await pm.evaluate({
+          toolName: 'run_shell_command',
+          command: 'echo hi # foo \\\nrm -rf /tmp/x',
+        }),
+      ).toBe('deny');
+    });
+
+    // Only `walkCompoundCommand` strips heredoc bodies; the Bash-rule paths
+    // split the raw command, so this `#` line reaches the comment state. bash
+    // hands that line to `cat` as data and never runs the `rm` —
+    // `bash -xc $'cat <<EOF\n# hi ; rm -rf /\nEOF'` traces only `+ cat` — so the
+    // deny that fired before this change was a false positive on text that is
+    // never executed. Making these paths heredoc-aware is #9417's job, not
+    // this change's.
+    it('heredoc body comment line: no deny on text bash never runs', async () => {
+      pm = new PermissionManager(
+        makeConfig({
+          permissionsDeny: ['Bash(rm *)'],
+        }),
+      );
+      pm.initialize();
+      expect(
+        await pm.evaluate({
+          toolName: 'run_shell_command',
+          command: 'cat <<EOF\n# hi ; rm -rf /\nEOF',
+        }),
+      ).not.toBe('deny');
+    });
   });
 
   describe('file path evaluation', () => {

packages/core/src/permissions/rule-parser.ts

@@ -837,6 +837,33 @@ function isAsyncOperator(command: string, index: number): boolean {
   return true;
 }
 
+/**
+ * Operators after which a `#` still starts a word, and so a comment.
+ *
+ * Whitespace and the start of the input are handled by {@link isCommentStart}
+ * itself. Confirmed against bash: `echo a;#c` and `echo a&#c` each run one
+ * command, and `echo a|#c` is a syntax error because the `#` comments out the
+ * right-hand side of the pipe. Redirection operators and `(` are deliberately
+ * absent: bash starts a word there too, but leaving them literal only
+ * over-splits, which stays fail-closed, and no reported shape needs them.
+ */
+const COMMENT_WORD_BOUNDARIES = [';', '&', '|'];
+
+/**
+ * Whether the `#` at `index` opens a comment rather than being a literal.
+ *
+ * bash only starts a comment at the start of a word, so `echo a#b` prints
+ * `a#b` while `echo a #b` prints `a`. The same word rule keeps the arithmetic
+ * base prefix in `echo $(( 8#17 ))` literal.
+ */
+function isCommentStart(command: string, index: number): boolean {
+  if (index === 0) {
+    return true;
+  }
+  const previous = command[index - 1]!;
+  return /\s/.test(previous) || COMMENT_WORD_BOUNDARIES.includes(previous);
+}
+
 /**
  * One segment of a compound command, together with the operator that ended it.
  */
@@ -893,6 +920,24 @@ export function splitCompoundCommandSegments(
       continue;
     }
 
+    // A word-initial `#` opens a comment that runs to the end of the physical
+    // line, so an operator inside it is not a boundary: bash runs
+    // `echo 'a' # comment ; echo B` as a single `echo` (#11815).
+    //
+    // This scan deliberately ignores `escaped`. A backslash does not continue a
+    // line inside a comment — bash really does run the `rm` in `echo hi # foo \`
+    // followed by a newline and `rm -rf /` — so honouring the escape would fold
+    // that second command into the comment's segment and cost it its own rule
+    // check. For the same reason the newline is left for the operator scan
+    // below, and stays a boundary.
+    if (ch === '#' && isCommentStart(command, i)) {
+      while (i < command.length && command[i] !== '\n') {
+        i++;
+      }
+      i--; // -1 because the loop will i++, landing back on the newline
+      continue;
+    }
+
     if (ch === '(' && command[i + 1] === '(') {
       arithmeticDepth++;
       i++;
@@ -945,6 +990,7 @@ export function splitCompoundCommandSegments(
  *   "git status && rm -rf /"  → ["git status", "rm -rf /"]
  *   "ls -la | grep foo"      → ["ls -la", "grep foo"]
  *   "echo 'a && b'"          → ["echo 'a && b'"]  (inside quotes)
+ *   "echo 'a' # c ; echo B"  → ["echo 'a' # c ; echo B"]  (inside a comment)
  *   "a && b || c"            → ["a", "b", "c"]
  *   "git status & rm -rf /"  → ["git status", "rm -rf /"]  (async operator)
  *   "build &> log.txt"       → ["build &> log.txt"]  (redirection, not async)