Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions internal/agent/command_prefix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,43 @@ func TestProposedCommandPrefixRejectsUnsafeRequestedPrefix(t *testing.T) {
}
}

// The launcher denylist is only worth anything if it holds on the path a user
// actually sees. A model naming its own prefix_rule is that path: before
// launcher names were normalized, "python3.11" was offered as a one-token
// grant, and hasStringPrefix then matched every later python3.11 invocation.
func TestProposedCommandPrefixRejectsLauncherSpellingsInRequestedPrefix(t *testing.T) {
for _, testCase := range []struct {
command string
rule []any
}{
{"python3.11 script.py", []any{"python3.11"}},
{"python.exe script.py", []any{"python.exe"}},
{"node.exe app.js", []any{"node.exe"}},
{"cmd /c whoami", []any{"cmd"}},
{"busybox sh -c id", []any{"busybox"}},
{"uv run main.py", []any{"uv", "run"}},
{"python3.11 -c import_os", []any{"python3.11", "-c"}},
} {
got := proposedCommandPrefix("bash", map[string]any{
"command": testCase.command,
"prefix_rule": testCase.rule,
})
if got != nil {
t.Errorf("prefix_rule %v for %q was offered as %#v, want rejected", testCase.rule, testCase.command, got)
}
}
}

func TestProposedCommandPrefixStillOffersOrdinaryCommands(t *testing.T) {
got := proposedCommandPrefix("bash", map[string]any{
"command": "cargo build --release",
"prefix_rule": []any{"cargo", "build"},
})
if len(got) != 2 || got[0] != "cargo" || got[1] != "build" {
t.Fatalf("ordinary prefix rule = %#v, want [cargo build]", got)
}
}

func TestProposedCommandPrefixRejectsUnsafeShellForms(t *testing.T) {
cases := []string{
"cat < in > out",
Expand Down
11 changes: 10 additions & 1 deletion internal/execution/contracts.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,16 @@ import (
"strings"
)

const PolicyVersion = 1
// PolicyVersion is the version of the approval policy that recorded grants were
// evaluated under. Bump it whenever a change narrows what may be approved: the
// grant store then backs the file up, keeps deny grants, invalidates prior
// approvals, and tells the user once, instead of silently honoring approvals
// that the current rules would have refused.
//
// v2 tightened command-prefix validation so a launcher spelled with a version
// or an executable extension ("python3.11", "python.exe") can no longer be
// approved as an ordinary command.
const PolicyVersion = 2

type Origin string

Expand Down
115 changes: 105 additions & 10 deletions internal/sandbox/command_prefix.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,10 @@ func unsafeCommandPrefix(prefix []string) bool {
return true
}
}
normalized := append([]string(nil), prefix...)
normalized[0] = normalizeLauncherName(normalized[0])
for _, banned := range bannedCommandPrefixSuggestions {
if sameStringSlice(prefix, banned) {
if sameStringSlice(prefix, banned) || sameStringSlice(normalized, banned) {
return true
}
}
Expand All @@ -165,25 +167,118 @@ func unsafeCommandPrefixPart(part string) bool {
}

func unsafeCommandPrefixLauncher(program string) bool {
program = strings.ToLower(strings.TrimSpace(program))
if strings.ContainsAny(program, `/\`) {
// A path is already refused. ":" and "~" are refused for the same reason:
// on Windows they introduce names that resolve to a different file than the
// one matched here, and no list can keep up with them. "PYTHON~1.EXE" is
// python.exe under an 8.3 short name, and 8.3 truncates the stem, so
// "POWERS~1.EXE" cannot be recognized by name at all; "python.exe::$DATA"
// names the same executable through its default stream. Refusing the shape
// costs a permission prompt on names no real command uses.
if strings.ContainsAny(program, `/\:~`) {
return true
}
switch program {
case "bash", "sh", "zsh", "/bin/bash", "/bin/zsh",
"pwsh", "powershell", "powershell.exe",
"env", "sudo", "doas", "su", "run0", "osascript",
name := normalizeLauncherName(program)
return bannedLauncherName(name) || bannedLauncherName(strings.TrimRight(name, "0123456789"))
}

func bannedLauncherName(name string) bool {
switch name {
case "bash", "sh", "zsh", "dash", "ash", "ksh", "csh", "tcsh", "fish", "busybox",
"pwsh", "powershell", "cmd", "wsl",
"env", "sudo", "sudoedit", "doas", "su", "run0", "osascript",
"command", "eval", "exec", "time",
"find", "xargs", "timeout", "nice", "nohup", "watch", "setsid", "stdbuf", "ionice",
"ssh", "make", "npm", "npx",
"python", "python3", "py", "pythonw", "pyw", "pypy", "pypy3",
"node", "perl", "ruby", "php", "lua", "deno", "bun":
"ssh", "make", "npm", "npx", "pnpm", "yarn", "bunx",
"python", "py", "pythonw", "pyw", "pypy", "uv", "uvx",
"node", "nodejs", "perl", "ruby", "php", "lua", "deno", "bun":
return true
default:
return false
}
}

// normalizeLauncherName reduces a program name to the launcher it actually runs
// so the list above cannot be stepped around by spelling the same interpreter
// differently: "python3.11", "python.exe" and "PYTHON3" all reach it as a name
// the list matches. internal/agent's commandName already normalizes case and
// Windows executable extensions on the allow side; this side matched raw, so
// every versioned or .exe-suffixed launcher validated as an ordinary command
// and persisted as a prefix grant that auto-allows every later invocation.
//
// A name is only ever narrowed toward an existing entry, so the failure
// direction is an extra permission prompt, never a silent grant.
func normalizeLauncherName(program string) string {
name := strings.ToLower(strings.TrimSpace(program))
// Windows discards trailing dots and spaces when resolving a filename, so
// "python." starts python.exe. Drop them before anything matches on the
// result, on every platform: the deny side must not depend on which OS is
// reading the grant, since the grants file travels with a synced home
// directory.
name = strings.TrimRight(name, ". ")
for _, suffix := range []string{".exe", ".cmd", ".bat", ".com", ".ps1"} {
if strings.HasSuffix(name, suffix) {
name = strings.TrimSuffix(name, suffix)
name = strings.TrimRight(name, ". ")
break
}
}
return trimLauncherVersion(trimLauncherABISuffix(trimLauncherBuildSuffix(name)))
}

// trimLauncherBuildSuffix drops the build-channel suffixes distributions and
// upstreams append to an otherwise unchanged interpreter, so "python3.11-dbg",
// "bash-static" and "pwsh-preview" are the launchers they say they are. The set
// is closed on purpose: a general "-word" strip would also swallow
// "node-gyp", "python3-config" and "ruby-lsp", which are ordinary tools a user
// should still be able to approve.
func trimLauncherBuildSuffix(name string) string {
for _, suffix := range []string{"-dbg", "-debug", "-static", "-nightly", "-preview", "-beta"} {
if strings.HasSuffix(name, suffix) {
return strings.TrimSuffix(name, suffix)
}
}
return name
}

// trimLauncherABISuffix drops CPython's ABI flags from a versioned interpreter
// name, so "python3.7m" and "python3.6dm" reduce to their version, and the
// free-threaded builds "python3.13t" and "python3.13td" reduce to theirs. The
// flags are only removed when a digit sits underneath them, which leaves
// ordinary names that happen to end in those letters alone: "sha256sum" keeps
// its "sum", "zstd" its "td", "cat" its "t".
func trimLauncherABISuffix(name string) string {
trimmed := strings.TrimRight(name, "dmut")
if trimmed == name || trimmed == "" {
return name
}
if last := trimmed[len(trimmed)-1]; last < '0' || last > '9' {
return name
}
return trimmed
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

// trimLauncherVersion drops a trailing "<separator><digits>" version from a
// program name, so "python3.11" becomes "python3" and "python2.7" becomes
// "python2". Digits without a separator are part of the name, which keeps
// "base64", "7z" and "sha256sum" intact; the caller handles the "python3" shape
// by also testing the digit-stripped form.
func trimLauncherVersion(name string) string {
for {
trimmed := strings.TrimRight(name, "0123456789")
if trimmed == name || trimmed == "" {
return name
}
if !strings.HasSuffix(trimmed, ".") && !strings.HasSuffix(trimmed, "-") {
return name
}
next := trimmed[:len(trimmed)-1]
if next == "" {
return name
}
name = next
}
}

func sameStringSlice(left []string, right []string) bool {
if len(left) != len(right) {
return false
Expand Down
115 changes: 115 additions & 0 deletions internal/sandbox/command_prefix_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
package sandbox

import "testing"

func TestUnsafeCommandPrefixLauncherIgnoresVersionAndExtension(t *testing.T) {
for _, program := range []string{
"python3", "python", "node", "git", "sh", "bash", "sudo",
"python3.11", "python3.12", "python2.7", "python3.13.1",
"python.exe", "python3.exe", "node.exe", "git.exe", "bash.exe",
"PYTHON3.11", "Node.EXE",
"cmd", "cmd.exe", "wsl", "wsl.exe",
// Windows resolves these to the same executable.
"python.", "python3.", "python3.11.", "python.exe.", "PYTHON.",
"python.cmd", "python.bat", "python.com", "python.ps1",
"PYTHON~1.EXE", "POWERS~1.EXE", "python.exe::$DATA",
// Versioned and ABI-suffixed interpreters.
"python3.7m", "python3.6dm", "pythonw3.11", "python-3.11",
// Free-threaded CPython (PEP 703) ships beside the GIL build.
"python3.13t", "python3.13t.exe", "python3.14t", "python3.13td",
// Distribution and build-channel spellings of the same interpreter.
"nodejs", "nodejs.exe", "node-nightly", "python3-dbg", "python3.11-dbg",
"python3.13t-dbg", "python3-debug", "python3.11-debug", "node-beta",
"pwsh-preview", "bash-static", "sudoedit",
// Twins of npm and npx, which the list already refuses.
"pnpm", "yarn", "bunx", "pnpm.cmd", "yarn.exe",
Comment thread
coderabbitai[bot] marked this conversation as resolved.
"perl5.36", "ruby3.2", "php8", "lua5.4", "node22.exe", "bash5",
} {
if !unsafeCommandPrefix([]string{program}) {
t.Errorf("unsafeCommandPrefix([%q]) = false, want true", program)
}
}
}

func TestUnsafeCommandPrefixKeepsOrdinaryCommandsGrantable(t *testing.T) {
for _, prefix := range [][]string{
{"cargo", "build"},
{"go", "test"},
{"ls", "-la"},
{"rg", "--json"},
{"7z", "l"},
{"base64", "-d"},
{"kubectl", "get", "pods"},
{"docker", "ps"},
{"sha256sum", "file"},
{"gcc-13", "-c"},
{"node-gyp", "build"},
{"python3-config", "--includes"},
{"s3cmd", "ls"},
{"cat", "file"},
{"zstd", "-d"},
{"sqlite3", "db"},
{"yt-dlp", "url"},
{"perl-doc", "-h"},
{"php-fpm", "-t"},
{"ruby-lsp", "--version"},
{"lua-language-server", "--check"},
{"nodemon", "app.js"},
} {
if unsafeCommandPrefix(prefix) {
t.Errorf("unsafeCommandPrefix(%q) = true, want false", prefix)
}
}
}
func TestNormalizeLauncherName(t *testing.T) {
for _, testCase := range []struct{ in, want string }{
{"python3.11", "python3"},
{"python2.7", "python2"},
{"pypy3", "pypy3"},
{"node.exe", "node"},
{"POWERSHELL.EXE", "powershell"},
{"7z", "7z"},
{"base64", "base64"},
{"sha256sum", "sha256sum"},
{"python.", "python"},
{"python3.11.exe.", "python3"},
{"python3.7m", "python3"},
{"python3.6dm", "python3"},
{"python3.13t", "python3"},
{"python3.13td", "python3"},
{"python3.11-dbg", "python3"},
{"node-nightly", "node"},
{"node-gyp", "node-gyp"},
{"python3-config", "python3-config"},
{"zstd", "zstd"},
{"cat", "cat"},
{"cargo", "cargo"},
{"", ""},
} {
if got := normalizeLauncherName(testCase.in); got != testCase.want {
t.Errorf("normalizeLauncherName(%q) = %q, want %q", testCase.in, got, testCase.want)
}
}
}

// Session-scoped grants are the agent's other entry point (loop.go grants them
// for a single turn without touching the store), so they must refuse the same
// launcher spellings the persisted path does.
func TestGrantCommandPrefixForSessionRefusesLauncherVariants(t *testing.T) {
engine := &Engine{commandPrefixes: newCommandPrefixGrantSet()}
engine.GrantCommandPrefixForSession("bash", []string{"cargo", "build"})
if _, ok := engine.LookupCommandPrefixForSession("bash", []string{"cargo", "build", "--release"}); !ok {
t.Fatal("CONTROL BROKEN: an ordinary session prefix grant did not match")
}
for _, prefix := range [][]string{
{"python3.11", "-c"},
{"python.exe", "-c"},
{"cmd", "/c"},
} {
engine.GrantCommandPrefixForSession("bash", prefix)
command := append(append([]string(nil), prefix...), "whatever")
if _, ok := engine.LookupCommandPrefixForSession("bash", command); ok {
t.Errorf("session grant %q matched %q, want refused", prefix, command)
}
}
}
Loading
Loading