diff --git a/codex-cli/src/approvals.ts b/codex-cli/src/approvals.ts index 032acec0..e626da7f 100644 --- a/codex-cli/src/approvals.ts +++ b/codex-cli/src/approvals.ts @@ -365,6 +365,11 @@ export function isSafeCommand( reason: "View file contents", group: "Reading files", }; + case "nl": + return { + reason: "View file with line numbers", + group: "Reading files", + }; case "rg": return { reason: "Ripgrep search", @@ -448,11 +453,15 @@ export function isSafeCommand( } break; case "sed": + // We allow two types of sed invocations: + // 1. `sed -n 1,200p FILE` + // 2. `sed -n 1,200p` because the file is passed via stdin, e.g., + // `nl -ba README.md | sed -n '1,200p'` if ( cmd1 === "-n" && isValidSedNArg(cmd2) && - typeof cmd3 === "string" && - command.length === 4 + (command.length === 3 || + (typeof cmd3 === "string" && command.length === 4)) ) { return { reason: "Sed print subset", diff --git a/codex-cli/tests/approvals.test.ts b/codex-cli/tests/approvals.test.ts index 94daacce..c592c395 100644 --- a/codex-cli/tests/approvals.test.ts +++ b/codex-cli/tests/approvals.test.ts @@ -32,6 +32,12 @@ describe("canAutoApprove()", () => { group: "Reading files", runInSandbox: false, }); + expect(check(["nl", "-ba", "README.md"])).toEqual({ + type: "auto-approve", + reason: "View file with line numbers", + group: "Reading files", + runInSandbox: false, + }); expect(check(["pwd"])).toEqual({ type: "auto-approve", reason: "Print working directory", @@ -147,4 +153,41 @@ describe("canAutoApprove()", () => { type: "ask-user", }); }); + + test("sed", () => { + // `sed` used to read lines from a file. + expect(check(["sed", "-n", "1,200p", "filename.txt"])).toEqual({ + type: "auto-approve", + reason: "Sed print subset", + group: "Reading files", + runInSandbox: false, + }); + // Bad quoting! The model is doing the wrong thing here, so this should not + // be auto-approved. + expect(check(["sed", "-n", "'1,200p'", "filename.txt"])).toEqual({ + type: "ask-user", + }); + // Extra arg: here we are extra conservative, we do not auto-approve. + expect(check(["sed", "-n", "1,200p", "file1.txt", "file2.txt"])).toEqual({ + type: "ask-user", + }); + + // `sed` used to read lines from a file with a shell command. + expect(check(["bash", "-lc", "sed -n '1,200p' filename.txt"])).toEqual({ + type: "auto-approve", + reason: "Sed print subset", + group: "Reading files", + runInSandbox: false, + }); + + // Pipe the output of `nl` to `sed`. + expect( + check(["bash", "-lc", "nl -ba README.md | sed -n '1,200p'"]), + ).toEqual({ + type: "auto-approve", + reason: "View file with line numbers", + group: "Reading files", + runInSandbox: false, + }); + }); });