feat: auto-approve nl and support piping to sed (#920)

Auto-approved:

```
["nl", "-ba", "README.md"]
["sed", "-n", "1,200p", "filename.txt"]
["bash", "-lc", "sed -n '1,200p' filename.txt"]
["bash", "-lc", "nl -ba README.md | sed -n '1,200p'"]
```

Not auto approved:

```
["sed", "-n", "'1,200p'", "filename.txt"]
["sed", "-n", "1,200p", "file1.txt", "file2.txt"]
```
This commit is contained in:
Michael Bolin
2025-05-13 13:06:35 -07:00
committed by GitHub
parent 0ac7e8d55b
commit a786c1d188
2 changed files with 54 additions and 2 deletions

View File

@@ -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",

View File

@@ -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,
});
});
});