Shell serial stack: here-strings, heredoc terminators, arithmetic/comment openers, secret ordering (#945)

* Treat here-strings as inline arguments, never heredoc openers

* Match heredoc terminators exactly, with tab-stripping only for <<-

Trim-based closing accepted a space-indented marker for plain << and kept a stray carriage return in CRLF markers, so the splitter and the approval display disagreed with the shell about where a heredoc ends. Compare exact lines instead.

* Ignore heredoc openers inside arithmetic and comments

A << inside ((/$(( is the left-shift operator and a << after a top-level # is documentation, so neither the splitter nor the approval display may open a heredoc there and swallow the following chain. Track arithmetic depth and skip #-to-EOL comments in both.

* Lock secret-before-listing ordering for shell secret paths

Pure ls of a secret name still asks, and chains flag the content-reading half; bounded listings stay exempt and unbounded listings still ask. The secret-first ordering already holds at all three sites (classify auto-allow, gate segment guard, auto-shell policy) — these tests pin it.

* Document list-free listings and dump-locked secret reads in auto mode

Author: TheGreatAxios

docs/IMPLEMENTATION.md

@@ -185,6 +185,17 @@ When auto is on, the gate auto-allows workspace file tools in `AUTO_ALLOWED_TOOL
 
 Unmatched shell auto-allows, including contained non-force `git worktree add`/`remove`/`prune` and read-only `list`. Path-arg tools that escape the workspace are denied at authorize time (the same sandbox path-escape enforces at execution). Writes under the in-workspace session state root (legacy `.agent-state`), mutating MCP, and unknown built-ins still prompt. Authorization hard-denies (catastrophic commands, open-ended shell search) remain independent of auto mode.
 
+Listings are list-free, dumps are dump-locked: a bounded `ls`/`tree` prints names only, so it auto-allows even in a directory containing a secret file — but the secret check runs before the listing exemption, so naming the secret itself still asks. Anything that dumps file contents never auto-allows in auto mode; shell references stay ask (not deny) so legitimate uses proceed after an explicit yes, while path-keyed tools hard-deny.
+
+| Command                                   | Verdict                                                 |
+| ----------------------------------------- | ------------------------------------------------------- |
+| `ls` in a directory containing `.env`     | auto-allow (names only, no secret named)                |
+| `ls .env`                                 | ask (secret check beats the listing exemption)          |
+| `cat .env`, `head .corbits/settings.json` | ask, never auto-allow                                   |
+| `bun --env-file=.env run …`               | ask; runs after an explicit yes                         |
+| `read_file` on `.env`                     | hard deny via secret-guard, even under skip-permissions |
+| `cat README.md` in the workspace          | auto-allow under the existing contained-read rules      |
+
 ### Reasoning Effort
 
 **Shift+Tab** in the TUI cycles reasoning effort for the live model (`cycleReasoningEffort` in `src/provider/reasoning-effort.ts`); the runner rebuilds inference sources and the prompt-border `profile · model · effort` label so the next turn picks it up. Plain Tab still toggles focus.

src/permission/classify-security.test.ts

@@ -1080,3 +1080,52 @@ describe("CL-6988 — nested / escaped interpreter peels do not auto-allow", ()
     expect(rule?.effect === "ask" || rule?.effect === "deny").toBe(true);
   });
 });
+
+describe("CL-5420 — secret checks run before pure-listing exemptions", () => {
+  test("a pure listing of a secret name still asks", () => {
+    const rule = autoShellRuleForCall(shellCall("ls .env"));
+    expect(rule?.name).toBe("sensitive-path");
+    expect(rule?.effect).toBe("ask");
+  });
+
+  test("a chain with a safe listing half flags the content-reading half", () => {
+    const rule = autoShellRuleForCall(shellCall("ls /tmp && cat .env"));
+    expect(rule?.name).toBe("sensitive-path");
+    expect(rule?.effect).toBe("ask");
+  });
+
+  test("a bounded listing with no secret reference stays exempt", () => {
+    expect(autoShellRuleForCall(shellCall("ls /tmp"))).toBeUndefined();
+  });
+
+  test("a flag-glued secret path asks", () => {
+    const rule = autoShellRuleForCall(
+      shellCall("bun --env-file=.env run publish.ts"),
+    );
+    expect(rule?.name).toBe("sensitive-path");
+    expect(rule?.effect).toBe("ask");
+  });
+
+  test("unbounded listing still asks", () => {
+    expect(autoShellRuleForCall(shellCall("ls -R"))?.name).toBe(
+      "unbounded-listing",
+    );
+  });
+
+  test("the gate asks on a pure listing of a secret name", async () => {
+    let asked = 0;
+    const gate = createPermissionGate({
+      approvals: [],
+      requestApproval: async () => {
+        asked++;
+        return { allow: false };
+      },
+      interactive: true,
+      skipPermissions: false,
+      reactorGated: false,
+    });
+    const verdict = await gate.evaluate(shellCall("ls .env"));
+    expect(verdict.allowed).toBe(false);
+    expect(asked).toBe(1);
+  });
+});

src/permission/command.test.ts

@@ -151,3 +151,111 @@ describe("splitChainedCommand redirect and background fragments", () => {
     expect(splitChainedCommand(prose)).toEqual([prose]);
   });
 });
+
+describe("splitChainedCommand heredoc boundaries", () => {
+  // A marker glued to `<<` is still an opener, and separators trailing the
+  // opener line do not split while the heredoc body is pending.
+  test("keeps separators on the opener line inside a glued-marker heredoc", () => {
+    const command = "cat <<B && echo done\nbody\nB";
+    expect(splitChainedCommand(command)).toEqual([command]);
+    const semicolon = "cat <<EOF; echo done\nbody\nEOF";
+    expect(splitChainedCommand(semicolon)).toEqual([semicolon]);
+  });
+
+  test("opens and closes a heredoc across CRLF line endings", () => {
+    const command = "cat <<EOF\r\nbody\r\nEOF";
+    expect(splitChainedCommand(command)).toEqual([command]);
+    expect(
+      splitChainedCommand("cat <<EOF\r\nbody\r\nEOF\r\n&& echo done"),
+    ).toEqual(["cat <<EOF\r\nbody\r\nEOF", "echo done"]);
+  });
+
+  // Only `<<-` strips leading tabs from the closing line; a space-indented
+  // close never terminates a plain `<<` heredoc.
+  test("closes <<- on a tab-indented marker but not << on spaces", () => {
+    expect(
+      splitChainedCommand("cat <<-EOF\nbody\n\tEOF\n&& echo evil"),
+    ).toEqual(["cat <<-EOF\nbody\n\tEOF", "echo evil"]);
+    const spaces = "cat <<EOF\nbody\n  EOF\n&& echo evil";
+    expect(splitChainedCommand(spaces)).toEqual([spaces]);
+  });
+
+  test("an unterminated heredoc swallows a later chain separator", () => {
+    const command = "cat <<EOF\nbody\n&& echo evil";
+    expect(splitChainedCommand(command)).toEqual([command]);
+  });
+
+  // Single-slot heredoc state: a second `<<` inside the body is payload, so
+  // the outer marker still closes and the following chain still splits.
+  test("treats a second << inside the body as payload, not a nested opener", () => {
+    expect(
+      splitChainedCommand("cat <<OUTER\nfoo <<INNER\nOUTER\n&& echo done"),
+    ).toEqual(["cat <<OUTER\nfoo <<INNER\nOUTER", "echo done"]);
+  });
+});
+
+describe("splitChainedCommand lexical context (arithmetic and comments)", () => {
+  // Inside `((` / `$((` the `<<` token is the left-shift operator, never a
+  // heredoc opener — the chain after it must still split.
+  test("never opens a heredoc inside arithmetic expansion", () => {
+    expect(splitChainedCommand("echo $((a<<1))")).toEqual(["echo $((a<<1))"]);
+    expect(splitChainedCommand("echo $((a << 1)) && echo done")).toEqual([
+      "echo $((a << 1))",
+      "echo done",
+    ]);
+  });
+
+  test("never opens a heredoc inside a (( )) arithmetic command", () => {
+    expect(splitChainedCommand("((x = a << 1)) && echo done")).toEqual([
+      "x = a << 1",
+      "echo done",
+    ]);
+  });
+
+  // A bare `( ... )` subshell is not arithmetic: a heredoc inside it is real.
+  test("still opens a heredoc inside a bare-paren subshell", () => {
+    const command = "(cat <<EOF\nbody\nEOF) && echo done";
+    expect(splitChainedCommand(command)).toEqual([command]);
+  });
+
+  // A top-level `#` starts a comment through end of line: a `<<` down there
+  // documents rather than opens, so the next line still splits.
+  test("never opens a heredoc from a #-to-EOL comment", () => {
+    expect(splitChainedCommand("# example: cat <<EOF\necho hi")).toEqual([
+      "# example: cat <<EOF",
+      "echo hi",
+    ]);
+    expect(splitChainedCommand("echo hi # tail <<EOF\n&& echo done")).toEqual([
+      "echo hi # tail <<EOF",
+      "echo done",
+    ]);
+  });
+
+  // Comment text never touches arithmetic depth: an unbalanced `((` inside
+  // a `#` comment must not poison later lines, so a genuine heredoc after
+  // the comment still opens and the following chain still splits.
+  test("never counts comment parens toward arithmetic depth", () => {
+    const command = "# (( \ncat <<EOF\nbody\nEOF\n&& echo done";
+    expect(splitChainedCommand(command)).toEqual([
+      "# ((",
+      "cat <<EOF\nbody\nEOF",
+      "echo done",
+    ]);
+  });
+
+  // Chain operators after `#` still split, so a dangerous command hiding
+  // behind a comment still surfaces as its own approval subject.
+  test("still splits chain operators after a # comment", () => {
+    expect(splitChainedCommand("# note && rm -rf /")).toEqual([
+      "# note",
+      "rm -rf /",
+    ]);
+  });
+
+  // A `#` line inside a genuine heredoc body stays payload: the marker still
+  // closes and the following chain still splits.
+  test("keeps a # line inside a heredoc body as payload", () => {
+    const command = "cat <<EOF\n# payload\nEOF\necho done";
+    expect(splitChainedCommand(command)).toEqual([command]);
+  });
+});

src/permission/command.ts

@@ -1,6 +1,9 @@
 import type { ApprovalScope } from "./types.js";
 import { escapeGlobLiteral } from "./matcher.js";
-import { parseHeredocOpener } from "../shell/command-segments.js";
+import {
+  isHeredocTerminator,
+  parseHeredocOpener,
+} from "../shell/command-segments.js";
 
 export { splitChainedCommand } from "../shell/command-segments.js";
 
@@ -29,6 +32,7 @@ export function stripCommentLines(command: string): string {
   let commentState: "unknown" | "yes" | "no" = "unknown";
   let quote: '"' | "'" | "`" | null = null;
   let heredocMarker: string | null = null;
+  let heredocStripTabs = false;
 
   const flushLine = (): void => {
     if (commentState !== "yes") out += line;
@@ -44,7 +48,10 @@ export function stripCommentLines(command: string): string {
       if (ch === "\n") {
         const lines = line.split("\n");
         const lastLine = lines[lines.length - 2] ?? "";
-        if (lastLine.trim() === heredocMarker) heredocMarker = null;
+        if (isHeredocTerminator(lastLine, heredocMarker, heredocStripTabs)) {
+          heredocMarker = null;
+          heredocStripTabs = false;
+        }
         out += line;
         line = "";
       }
@@ -91,6 +98,7 @@ export function stripCommentLines(command: string): string {
         line += command.slice(i, opener.lineEnd);
         i = opener.lineEnd - 1;
         heredocMarker = opener.marker;
+        heredocStripTabs = opener.stripTabs;
         continue;
       }
     }

src/permission/permission.test.ts

@@ -176,6 +176,23 @@ describe("splitChainedCommand", () => {
     expect(splitChainedCommand(cmd)).toHaveLength(2);
   });
 
+  test("does not treat a here-string (<<<) as a heredoc opener", () => {
+    expect(splitChainedCommand('cat <<< "word" && echo hi')).toEqual([
+      'cat <<< "word"',
+      "echo hi",
+    ]);
+    expect(splitChainedCommand("cmd <<<EOF")).toEqual(["cmd <<<EOF"]);
+    expect(splitChainedCommand("<<< EOF && echo done")).toEqual([
+      "<<< EOF",
+      "echo done",
+    ]);
+  });
+
+  test("still treats <<- as a heredoc opener", () => {
+    const cmd = "cat <<-EOF\nbody\nEOF";
+    expect(splitChainedCommand(cmd)).toHaveLength(1);
+  });
+
   test("treats shell line continuation (backslash + newline) as glue, not a chain split", () => {
     // Common pattern from agents emitting readable multi-line shell calls.
     expect(splitChainedCommand("cd foo && \\\nbun test")).toEqual([
@@ -3962,6 +3979,11 @@ describe("stripCommentLines", () => {
     expect(stripCommentLines(command)).toBe(command);
   });
 
+  test("a here-string never swallows a later line into a heredoc body", () => {
+    const command = 'cat <<< "word"\n# a real comment';
+    expect(stripCommentLines(command)).toBe('cat <<< "word"\n');
+  });
+
   test("leaves a real command with a trailing inline comment untouched", () => {
     expect(stripCommentLines("ls -la # list files")).toBe(
       "ls -la # list files",