diff --git a/src/permissions/mode.ts b/src/permissions/mode.ts index 9166281..2bc1a98 100644 --- a/src/permissions/mode.ts +++ b/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*(?"[^"]+"|'[^']+'|\S+)\s+<<-?\s*(?"[^"]+"|'[^']+'|\S+)|<<-?\s*(?"[^"]+"|'[^']+'|\S+)\s+>\s*(?"[^"]+"|'[^']+'|\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 diff --git a/src/permissions/readOnlyShell.ts b/src/permissions/readOnlyShell.ts index ae7a32e..0112578 100644 --- a/src/permissions/readOnlyShell.ts +++ b/src/permissions/readOnlyShell.ts @@ -111,9 +111,107 @@ const SAFE_GH_COMMANDS: Record | 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; } diff --git a/src/tests/permissions-mode.test.ts b/src/tests/permissions-mode.test.ts index 353f00d..b3c6bf3 100644 --- a/src/tests/permissions-mode.test.ts +++ b/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", diff --git a/src/tests/permissions/readOnlyShell.test.ts b/src/tests/permissions/readOnlyShell.test.ts index 57b3075..12ae4e5 100644 --- a/src/tests/permissions/readOnlyShell.test.ts +++ b/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); }); });