diff --git a/codex-cli/src/utils/agent/exec.ts b/codex-cli/src/utils/agent/exec.ts index 876e144a..25c6f86a 100644 --- a/codex-cli/src/utils/agent/exec.ts +++ b/codex-cli/src/utils/agent/exec.ts @@ -14,10 +14,16 @@ import { parse } from "shell-quote"; const DEFAULT_TIMEOUT_MS = 10_000; // 10 seconds function requiresShell(cmd: Array): boolean { - return cmd.some((arg) => { - const tokens = parse(arg) as Array; + // If the command is a single string that contains shell operators, + // it needs to be run with shell: true + if (cmd.length === 1 && cmd[0] !== undefined) { + const tokens = parse(cmd[0]) as Array; return tokens.some((token) => typeof token === "object" && "op" in token); - }); + } + + // If the command is split into multiple arguments, we don't need shell: true + // even if one of the arguments is a shell operator like '|' + return false; } /** diff --git a/codex-cli/tests/fixed-requires-shell.test.ts b/codex-cli/tests/fixed-requires-shell.test.ts new file mode 100644 index 00000000..dd1058db --- /dev/null +++ b/codex-cli/tests/fixed-requires-shell.test.ts @@ -0,0 +1,38 @@ +import { describe, it, expect } from "vitest"; +import { parse } from "shell-quote"; + +// The fixed requiresShell function +function requiresShell(cmd: Array): boolean { + // If the command is a single string that contains shell operators, + // it needs to be run with shell: true + if (cmd.length === 1 && cmd[0] !== undefined) { + const tokens = parse(cmd[0]) as Array; + return tokens.some((token) => typeof token === "object" && "op" in token); + } + + // If the command is split into multiple arguments, we don't need shell: true + // even if one of the arguments is a shell operator like '|' + return false; +} + +describe("fixed requiresShell function", () => { + it("should detect pipe in a single argument", () => { + const cmd = ['grep -n "finally:" some-file | head']; + expect(requiresShell(cmd)).toBe(true); + }); + + it("should not detect pipe in separate arguments", () => { + const cmd = ["grep", "-n", "finally:", "some-file", "|", "head"]; + expect(requiresShell(cmd)).toBe(false); + }); + + it("should handle other shell operators in a single argument", () => { + const cmd = ["echo hello && echo world"]; + expect(requiresShell(cmd)).toBe(true); + }); + + it("should not enable shell for normal commands", () => { + const cmd = ["ls", "-la"]; + expect(requiresShell(cmd)).toBe(false); + }); +}); diff --git a/codex-cli/tests/pipe-command.test.ts b/codex-cli/tests/pipe-command.test.ts new file mode 100644 index 00000000..652fee99 --- /dev/null +++ b/codex-cli/tests/pipe-command.test.ts @@ -0,0 +1,19 @@ +import { describe, it, expect } from "vitest"; +import { parse } from "shell-quote"; + +/* eslint-disable no-console */ + +describe("shell-quote parse with pipes", () => { + it("should correctly parse a command with a pipe", () => { + const cmd = 'grep -n "finally:" some-file | head'; + const tokens = parse(cmd); + console.log("Parsed tokens:", JSON.stringify(tokens, null, 2)); + + // Check if any token has an 'op' property + const hasOpToken = tokens.some( + (token) => typeof token === "object" && "op" in token, + ); + + expect(hasOpToken).toBe(true); + }); +}); diff --git a/codex-cli/tests/requires-shell.test.ts b/codex-cli/tests/requires-shell.test.ts new file mode 100644 index 00000000..5264490e --- /dev/null +++ b/codex-cli/tests/requires-shell.test.ts @@ -0,0 +1,49 @@ +import { describe, it, expect } from "vitest"; +import { parse } from "shell-quote"; + +/* eslint-disable no-console */ + +// Recreate the requiresShell function for testing +function requiresShell(cmd: Array): boolean { + // If the command is a single string that contains shell operators, + // it needs to be run with shell: true + if (cmd.length === 1 && cmd[0] !== undefined) { + const tokens = parse(cmd[0]) as Array; + console.log( + `Parsing argument: "${cmd[0]}", tokens:`, + JSON.stringify(tokens, null, 2), + ); + return tokens.some((token) => typeof token === "object" && "op" in token); + } + + // If the command is split into multiple arguments, we don't need shell: true + // even if one of the arguments is a shell operator like '|' + cmd.forEach((arg) => { + const tokens = parse(arg) as Array; + console.log( + `Parsing argument: "${arg}", tokens:`, + JSON.stringify(tokens, null, 2), + ); + }); + console.log("Result for separate arguments: false"); + return false; +} + +describe("requiresShell function", () => { + it("should detect pipe in a single argument", () => { + const cmd = ['grep -n "finally:" some-file | head']; + expect(requiresShell(cmd)).toBe(true); + }); + + it("should not detect pipe in separate arguments", () => { + const cmd = ["grep", "-n", "finally:", "some-file", "|", "head"]; + const result = requiresShell(cmd); + console.log("Result for separate arguments:", result); + expect(result).toBe(false); + }); + + it("should handle other shell operators", () => { + const cmd = ["echo hello && echo world"]; + expect(requiresShell(cmd)).toBe(true); + }); +});