fix: allow quoted pipes in read-only bash parsing (#1194)

src/permissions/mode.ts

@@ -5,6 +5,7 @@ import { homedir } from "node:os";
 import { isAbsolute, join, relative, resolve } from "node:path";
 
 import { isReadOnlyShellCommand } from "./readOnlyShell";
+import { unwrapShellLauncherCommand } from "./shell-command-normalization";
 
 export type PermissionMode =
   | "default"
@@ -96,6 +97,87 @@ function extractApplyPatchPaths(input: string): string[] {
   return paths;
 }
 
+function stripMatchingQuotes(value: string): string {
+  const trimmed = value.trim();
+  if (trimmed.length < 2) {
+    return trimmed;
+  }
+
+  const first = trimmed[0];
+  const last = trimmed[trimmed.length - 1];
+  if ((first === '"' || first === "'") && last === first) {
+    return trimmed.slice(1, -1);
+  }
+  return trimmed;
+}
+
+/**
+ * Detect commands that are exclusively a heredoc write to a file:
+ *   cat > /path/to/file <<'EOF'\n...\nEOF
+ *   cat <<'EOF' > /path/to/file\n...\nEOF
+ *
+ * Returns the target file path when recognized, otherwise null.
+ */
+function extractPlanFileWritePathFromShellCommand(
+  command: string | string[] | undefined,
+): string | null {
+  if (!command) {
+    return null;
+  }
+
+  const commandString =
+    typeof command === "string" ? command : (command.join(" ") ?? "");
+  const normalizedCommand = unwrapShellLauncherCommand(commandString).trim();
+  if (!normalizedCommand) {
+    return null;
+  }
+
+  const lines = normalizedCommand.split(/\r?\n/);
+  const firstLine = lines[0]?.trim() ?? "";
+  if (!firstLine) {
+    return null;
+  }
+
+  const firstLineMatch = firstLine.match(
+    /^cat\s+(?:>\s*(?<path1>"[^"]+"|'[^']+'|\S+)\s+<<-?\s*(?<delim1>"[^"]+"|'[^']+'|\S+)|<<-?\s*(?<delim2>"[^"]+"|'[^']+'|\S+)\s+>\s*(?<path2>"[^"]+"|'[^']+'|\S+))\s*$/,
+  );
+  if (!firstLineMatch?.groups) {
+    return null;
+  }
+
+  const rawPath = firstLineMatch.groups.path1 || firstLineMatch.groups.path2;
+  const rawDelim = firstLineMatch.groups.delim1 || firstLineMatch.groups.delim2;
+
+  if (!rawPath || !rawDelim) {
+    return null;
+  }
+
+  const delimiter = stripMatchingQuotes(rawDelim);
+  if (!delimiter) {
+    return null;
+  }
+
+  // Find heredoc terminator line and ensure nothing non-whitespace follows it.
+  let terminatorLine = -1;
+  for (let i = 1; i < lines.length; i += 1) {
+    if ((lines[i] ?? "") === delimiter) {
+      terminatorLine = i;
+      break;
+    }
+  }
+  if (terminatorLine === -1) {
+    return null;
+  }
+
+  for (let i = terminatorLine + 1; i < lines.length; i += 1) {
+    if ((lines[i] ?? "").trim().length > 0) {
+      return null;
+    }
+  }
+
+  return stripMatchingQuotes(rawPath);
+}
+
 /**
  * Permission mode state for the current session.
  * Set via CLI --permission-mode flag or settings.json defaultMode.
@@ -334,6 +416,22 @@ class PermissionModeManager {
           ) {
             return "allow";
           }
+
+          // Special case: allow shell heredoc writes when they ONLY target
+          // a markdown file in ~/.letta/plans/.
+          const planWritePath =
+            extractPlanFileWritePathFromShellCommand(command);
+          if (planWritePath) {
+            const plansDir = join(homedir(), ".letta", "plans");
+            const resolvedPath = resolvePlanTargetPath(
+              planWritePath,
+              workingDirectory,
+            );
+
+            if (resolvedPath && isPathInPlansDir(resolvedPath, plansDir)) {
+              return "allow";
+            }
+          }
         }
 
         // Everything else denied in plan mode

src/permissions/readOnlyShell.ts

@@ -111,9 +111,107 @@ const SAFE_GH_COMMANDS: Record<string, Set<string> | null> = {
   status: null, // top-level command, no action needed
 };
 
-// Operators that are always dangerous (file redirects, command substitution)
-// Note: &&, ||, ; are handled by splitting and checking each segment
-const DANGEROUS_OPERATOR_PATTERN = /(>>|>|\$\(|`)/;
+/**
+ * Split a shell command into segments on unquoted separators: |, &&, ||, ;
+ * Returns null if dangerous operators are found:
+ * - redirects (>, >>) outside quotes
+ * - command substitution ($(), backticks) outside single quotes
+ */
+function splitShellSegments(input: string): string[] | null {
+  const segments: string[] = [];
+  let current = "";
+  let i = 0;
+  let quote: "single" | "double" | null = null;
+
+  while (i < input.length) {
+    const ch = input[i];
+
+    if (!ch) {
+      i += 1;
+      continue;
+    }
+
+    if (quote === "single") {
+      current += ch;
+      if (ch === "'") {
+        quote = null;
+      }
+      i += 1;
+      continue;
+    }
+
+    if (quote === "double") {
+      if (ch === "\\" && i + 1 < input.length) {
+        current += input.slice(i, i + 2);
+        i += 2;
+        continue;
+      }
+
+      // Command substitution still evaluates inside double quotes.
+      if (ch === "`" || input.startsWith("$(", i)) {
+        return null;
+      }
+
+      current += ch;
+      if (ch === '"') {
+        quote = null;
+      }
+      i += 1;
+      continue;
+    }
+
+    if (ch === "'") {
+      quote = "single";
+      current += ch;
+      i += 1;
+      continue;
+    }
+    if (ch === '"') {
+      quote = "double";
+      current += ch;
+      i += 1;
+      continue;
+    }
+
+    if (input.startsWith(">>", i) || ch === ">") {
+      return null;
+    }
+    if (ch === "`" || input.startsWith("$(", i)) {
+      return null;
+    }
+
+    if (input.startsWith("&&", i)) {
+      segments.push(current);
+      current = "";
+      i += 2;
+      continue;
+    }
+    if (input.startsWith("||", i)) {
+      segments.push(current);
+      current = "";
+      i += 2;
+      continue;
+    }
+    if (ch === ";") {
+      segments.push(current);
+      current = "";
+      i += 1;
+      continue;
+    }
+    if (ch === "|") {
+      segments.push(current);
+      current = "";
+      i += 1;
+      continue;
+    }
+
+    current += ch;
+    i += 1;
+  }
+
+  segments.push(current);
+  return segments.map((segment) => segment.trim()).filter(Boolean);
+}
 
 export interface ReadOnlyShellOptions {
   /**
@@ -152,18 +250,8 @@ export function isReadOnlyShellCommand(
     return false;
   }
 
-  if (DANGEROUS_OPERATOR_PATTERN.test(trimmed)) {
-    return false;
-  }
-
-  // Split on command separators: |, &&, ||, ;
-  // Each segment must be safe for the whole command to be safe
-  const segments = trimmed
-    .split(/\||&&|\|\||;/)
-    .map((segment) => segment.trim())
-    .filter(Boolean);
-
-  if (segments.length === 0) {
+  const segments = splitShellSegments(trimmed);
+  if (!segments || segments.length === 0) {
     return false;
   }
 

src/tests/permissions-mode.test.ts

@@ -430,6 +430,74 @@ test("plan mode - denies non-read-only Bash", () => {
   expect(result.matchedRule).toBe("plan mode");
 });
 
+test("plan mode - allows Bash heredoc write to plan file", () => {
+  permissionMode.setMode("plan");
+
+  const permissions: PermissionRules = {
+    allow: [],
+    deny: [],
+    ask: [],
+  };
+
+  const planPath = join(homedir(), ".letta", "plans", "unit-test-plan.md");
+  const command = `cat > ${planPath} <<'EOF'\n# Plan\n- step 1\nEOF`;
+
+  const result = checkPermission(
+    "Bash",
+    { command },
+    permissions,
+    "/Users/test/project",
+  );
+
+  expect(result.decision).toBe("allow");
+  expect(result.matchedRule).toBe("plan mode");
+});
+
+test("plan mode - denies Bash heredoc write outside plans dir", () => {
+  permissionMode.setMode("plan");
+
+  const permissions: PermissionRules = {
+    allow: [],
+    deny: [],
+    ask: [],
+  };
+
+  const command = "cat > /tmp/not-a-plan.md <<'EOF'\n# Plan\nEOF";
+
+  const result = checkPermission(
+    "Bash",
+    { command },
+    permissions,
+    "/Users/test/project",
+  );
+
+  expect(result.decision).toBe("deny");
+  expect(result.matchedRule).toBe("plan mode");
+});
+
+test("plan mode - denies Bash heredoc write when extra commands follow", () => {
+  permissionMode.setMode("plan");
+
+  const permissions: PermissionRules = {
+    allow: [],
+    deny: [],
+    ask: [],
+  };
+
+  const planPath = join(homedir(), ".letta", "plans", "unit-test-plan.md");
+  const command = `cat > ${planPath} <<'EOF'\n# Plan\nEOF\necho 'extra command'`;
+
+  const result = checkPermission(
+    "Bash",
+    { command },
+    permissions,
+    "/Users/test/project",
+  );
+
+  expect(result.decision).toBe("deny");
+  expect(result.matchedRule).toBe("plan mode");
+});
+
 test("plan mode - allows read-only Bash commands", () => {
   permissionMode.setMode("plan");
 
@@ -503,6 +571,18 @@ test("plan mode - allows read-only Bash commands", () => {
   );
   expect(chainedResult.decision).toBe("allow");
 
+  // quoted pipes in regex patterns should be treated as literals and allowed
+  const quotedPipeResult = checkPermission(
+    "Bash",
+    {
+      command:
+        'rg -n "memfs|memory filesystem|memory_filesystem|skills/|SKILL.md|git-backed|sync" letta tests -S',
+    },
+    permissions,
+    "/Users/test/project",
+  );
+  expect(quotedPipeResult.decision).toBe("allow");
+
   // cd && dangerous command should still be denied
   const cdDangerousResult = checkPermission(
     "Bash",

src/tests/permissions/readOnlyShell.test.ts

@@ -182,6 +182,15 @@ describe("isReadOnlyShellCommand", () => {
       expect(isReadOnlyShellCommand("ls -la | grep txt | wc -l")).toBe(true);
     });
 
+    test("allows pipe characters inside quoted args", () => {
+      expect(
+        isReadOnlyShellCommand(
+          'rg -n "memfs|memory filesystem|memory_filesystem|skills/|SKILL.md|git-backed|sync" letta tests -S',
+        ),
+      ).toBe(true);
+      expect(isReadOnlyShellCommand("grep 'foo|bar|baz' file.txt")).toBe(true);
+    });
+
     test("blocks pipes with unsafe commands", () => {
       expect(isReadOnlyShellCommand("cat file | rm")).toBe(false);
       expect(isReadOnlyShellCommand("echo test | bash")).toBe(false);
@@ -203,6 +212,13 @@ describe("isReadOnlyShellCommand", () => {
     test("blocks command substitution", () => {
       expect(isReadOnlyShellCommand("echo $(rm file)")).toBe(false);
       expect(isReadOnlyShellCommand("echo `rm file`")).toBe(false);
+      expect(isReadOnlyShellCommand('echo "$(rm file)"')).toBe(false);
+      expect(isReadOnlyShellCommand('echo "`rm file`"')).toBe(false);
+    });
+
+    test("allows literal redirects inside quotes", () => {
+      expect(isReadOnlyShellCommand('echo "a > b"')).toBe(true);
+      expect(isReadOnlyShellCommand("echo 'a >> b'")).toBe(true);
     });
   });