From f515613fa999dcba77aa39dbf2c0cce966d0fed0 Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Wed, 22 Apr 2026 11:29:52 -0400 Subject: [PATCH 1/6] fix(interp): block $(/dev/null`}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + stdout, stderr, code := runBypass(t, tc.script, dir, []string{"rshell:echo"}) + assert.Equal(t, 2, code, "script must be rejected at validation") + assert.NotContains(t, stdout, "leak", "file contents must not leak") + assert.Contains(t, stderr, "< input redirection requires a command", + "rejection message should mention the input redirect rule") + }) + } +} + +// TestPentestInputRedirBypass_NegativeCases verifies we did NOT break +// the legitimate `cmd < file` form. These must continue to work when +// the command is in the allowlist. +func TestPentestInputRedirBypass_NegativeCases(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "data.txt"), []byte("hello\n"), 0644)) + + cases := []struct { + name string + script string + want string + }{ + {"cat with <", `cat < data.txt`, "hello\n"}, + {"head with <", `head < data.txt`, "hello\n"}, + {"tail with <", `tail < data.txt`, "hello\n"}, + {"explicit fd 0", `cat 0< data.txt`, "hello\n"}, + {"in cmdsubst via cat", `x=$(cat < data.txt); echo "$x"`, "hello\n"}, + {"pipe then <", `cat < data.txt | cat`, "hello\n"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + stdout, stderr, code := runBypass(t, tc.script, dir, + []string{"rshell:cat", "rshell:head", "rshell:tail", "rshell:echo"}) + assert.Equal(t, 0, code, "legitimate cmd Date: Wed, 22 Apr 2026 12:53:36 -0400 Subject: [PATCH 2/6] fix(interp): restrict < to file-reading builtins The previous commit rejected bare < redirects but accepted any command paired with <. That is broader than needed: `echo < file` would open a file purely to hand its stdin to a command that ignores it. Narrow the rule to the 11 file-reading builtins (cat, cut, grep, head, sed, sort, strings, tail, tr, uniq, wc) so < only applies where reading is the command's actual purpose. Also reject dynamic command names since they cannot be validated statically. --- SHELL_FEATURES.md | 2 +- interp/tests/cmdsubst_pentest_test.go | 10 +- interp/tests/cmdsubst_test.go | 6 +- .../tests/redir_input_bypass_pentest_test.go | 123 +++++++++++++++++- interp/validate.go | 113 ++++++++++++++-- .../cat_shortcut_bypass.yaml | 2 +- .../input_non_file_reader.yaml | 16 +++ .../command_substitution/cat_shortcut.yaml | 2 +- .../cat_shortcut_directory.yaml | 2 +- .../cat_shortcut_exit_status.yaml | 2 +- 10 files changed, 248 insertions(+), 30 deletions(-) create mode 100644 tests/scenarios/shell/blocked_redirects/input_non_file_reader.yaml diff --git a/SHELL_FEATURES.md b/SHELL_FEATURES.md index 0693bda0..b7c35bf5 100644 --- a/SHELL_FEATURES.md +++ b/SHELL_FEATURES.md @@ -70,7 +70,7 @@ Blocked features are rejected before execution with exit code 2. ## Pipes and Redirections - ✅ `|` — pipe stdout -- ✅ `<` — input redirection (read-only, within AllowedPaths) +- ✅ `<` — input redirection (read-only, within AllowedPaths); only permitted directly on file-reading builtins: `cat`, `cut`, `grep`, `head`, `sed`, `sort`, `strings`, `tail`, `tr`, `uniq`, `wc` - ✅ `</dev/null`, `2>/dev/null` — redirect stdout or stderr to /dev/null (output is discarded; only `/dev/null` is allowed as target) diff --git a/interp/tests/cmdsubst_pentest_test.go b/interp/tests/cmdsubst_pentest_test.go index 7f538803..58b8161e 100644 --- a/interp/tests/cmdsubst_pentest_test.go +++ b/interp/tests/cmdsubst_pentest_test.go @@ -55,7 +55,7 @@ func TestCmdSubstPentestPathTraversal(t *testing.T) { // the command allowlist). _, stderr, code := cmdSubstRun(t, `x=$(<../../../../../../etc/passwd)`, dir) assert.Equal(t, 2, code) - assert.Contains(t, stderr, "< input redirection requires a command") + assert.Contains(t, stderr, "file-reading command") } func TestCmdSubstPentestAbsolutePath(t *testing.T) { @@ -63,7 +63,7 @@ func TestCmdSubstPentestAbsolutePath(t *testing.T) { // Absolute path outside sandbox via $(<…) — rejected at validation. _, stderr, code := cmdSubstRun(t, `x=$(<"/etc/hosts")`, dir) assert.Equal(t, 2, code) - assert.Contains(t, stderr, "< input redirection requires a command") + assert.Contains(t, stderr, "file-reading command") } // --- No allowed paths at all --- @@ -74,7 +74,7 @@ func TestCmdSubstPentestNoAllowedPaths(t *testing.T) { // $( Date: Wed, 22 Apr 2026 13:06:41 -0400 Subject: [PATCH 3/6] fix(interp): gate $(/dev/null`, `2>/dev/null` — redirect stdout or stderr to /dev/null (output is discarded; only `/dev/null` is allowed as target) diff --git a/analysis/symbols_interp.go b/analysis/symbols_interp.go index 01663f1c..3c52e7d5 100644 --- a/analysis/symbols_interp.go +++ b/analysis/symbols_interp.go @@ -30,7 +30,9 @@ var interpAllowedSymbols = []string{ "fmt.Fprintln", // 🟠 writes to an io.Writer with newline; delegates to Write, no filesystem access. "fmt.Sprintf", // 🟢 string formatting; pure function, no I/O. "io.Closer", // 🟢 interface type for closing; no side effects. + "io.Copy", // 🟠 copies from Reader to Writer; no filesystem access, delegates to Read/Write. "io.Discard", // 🟢 write sink that discards all data; no side effects. + "io.LimitReader", // 🟢 wraps a Reader with a byte cap; pure function, no I/O. "io.Reader", // 🟢 interface type for reading; no side effects. "io.ReadWriteCloser", // 🟢 combined interface type; no side effects. "io.Writer", // 🟢 interface type for writing; no side effects. diff --git a/interp/runner_expand.go b/interp/runner_expand.go index c11f7080..ef46d64f 100644 --- a/interp/runner_expand.go +++ b/interp/runner_expand.go @@ -12,6 +12,7 @@ import ( "fmt" "io" "io/fs" + "os" "strings" "sync" @@ -64,17 +65,54 @@ const MaxGlobReadDirCalls = 10_000 // cmdSubst handles command substitution ($(...) and `...`). // It runs the commands in a subshell and writes their stdout to w. // -// The $(/dev/null`}, - } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - stdout, stderr, code := runBypass(t, tc.script, dir, []string{"rshell:echo"}) - assert.Equal(t, 2, code, "script must be rejected at validation") - assert.NotContains(t, stdout, "leak", "file contents must not leak") - assert.Contains(t, stderr, "file-reading command", - "rejection message should mention the input redirect rule") - }) - } -} - -// TestPentestInputRedirBypass_NegativeCases verifies we did NOT break -// the legitimate `cmd < file` form. These must continue to work when -// the command is in the allowlist. -func TestPentestInputRedirBypass_NegativeCases(t *testing.T) { - dir := t.TempDir() - require.NoError(t, os.WriteFile(filepath.Join(dir, "data.txt"), []byte("hello\n"), 0644)) - - cases := []struct { - name string - script string - want string - }{ - {"cat with <", `cat < data.txt`, "hello\n"}, - {"head with <", `head < data.txt`, "hello\n"}, - {"tail with <", `tail < data.txt`, "hello\n"}, - {"explicit fd 0", `cat 0< data.txt`, "hello\n"}, - {"in cmdsubst via cat", `x=$(cat < data.txt); echo "$x"`, "hello\n"}, - {"pipe then <", `cat < data.txt | cat`, "hello\n"}, - } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - stdout, stderr, code := runBypass(t, tc.script, dir, - []string{"rshell:cat", "rshell:head", "rshell:tail", "rshell:echo"}) - assert.Equal(t, 0, code, "legitimate cmd Date: Wed, 22 Apr 2026 13:24:36 -0400 Subject: [PATCH 4/6] test(interp): extend $( Date: Wed, 22 Apr 2026 13:34:15 -0400 Subject: [PATCH 5/6] test(scenarios): assert exact stderr in cat_shortcut_bypass Per AGENTS.md, scenarios should prefer expect.stderr over stderr_contains when the output is deterministic. The blocked $( Date: Wed, 22 Apr 2026 13:54:27 -0400 Subject: [PATCH 6/6] =?UTF-8?q?test(scenarios):=20inverse=20of=20cat=5Fsho?= =?UTF-8?q?rtcut=5Fbypass=20=E2=80=94=20succeed=20when=20rshell:cat=20is?= =?UTF-8?q?=20allowed?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pairs with blocked_redirects/cat_shortcut_bypass.yaml to prove the gate decision hinges purely on cat's membership in allowed_commands and has no other side effects on stdout/stderr. --- .../cat_shortcut_allowed_explicit.yaml | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 tests/scenarios/shell/command_substitution/cat_shortcut_allowed_explicit.yaml diff --git a/tests/scenarios/shell/command_substitution/cat_shortcut_allowed_explicit.yaml b/tests/scenarios/shell/command_substitution/cat_shortcut_allowed_explicit.yaml new file mode 100644 index 00000000..59fabf87 --- /dev/null +++ b/tests/scenarios/shell/command_substitution/cat_shortcut_allowed_explicit.yaml @@ -0,0 +1,17 @@ +description: $(