From f69af16e02ca8229acc6673af9b9de88718360a6 Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Tue, 7 Apr 2026 10:31:16 -0400 Subject: [PATCH 1/2] fix(interp): remove special case for help builtin command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The help command is no longer implicitly allowed — it must be explicitly included in the AllowedCommands list like any other command. Removes the name != "help" checks from the command allowlist enforcement and the CommandAllowed callback. Closes #175 Co-Authored-By: Claude Opus 4.6 (1M context) --- interp/runner_exec.go | 10 ++++------ tests/scenarios/cmd/help/always_available.yaml | 10 ---------- tests/scenarios/cmd/help/restricted_commands.yaml | 4 ++-- 3 files changed, 6 insertions(+), 18 deletions(-) delete mode 100644 tests/scenarios/cmd/help/always_available.yaml diff --git a/interp/runner_exec.go b/interp/runner_exec.go index 62734ee4..d0b5d4c5 100644 --- a/interp/runner_exec.go +++ b/interp/runner_exec.go @@ -256,9 +256,7 @@ func (r *Runner) call(ctx context.Context, pos syntax.Pos, args []string) { } name := args[0] - // Check whether the command is allowed. "help" is always permitted so - // users can discover available commands regardless of the active policy. - if !r.allowAllCommands && name != "help" && !r.allowedCommands[name] { + if !r.allowAllCommands && !r.allowedCommands[name] { r.errf("%s: command not allowed\n", name) r.exit.code = 127 return @@ -267,7 +265,7 @@ func (r *Runner) call(ctx context.Context, pos syntax.Pos, args []string) { if fn, ok := builtins.Lookup(name); ok { var runCmd func(context.Context, string, string, []string) (uint8, error) runCmd = func(ctx context.Context, dir string, cmdName string, cmdArgs []string) (uint8, error) { - if !r.allowAllCommands && cmdName != "help" && !r.allowedCommands[cmdName] { + if !r.allowAllCommands && !r.allowedCommands[cmdName] { return 127, fmt.Errorf("%s: command not allowed", cmdName) } cmdFn, ok := builtins.Lookup(cmdName) @@ -324,7 +322,7 @@ func (r *Runner) call(ctx context.Context, pos syntax.Pos, args []string) { return builtins.FileID{Dev: dev, Ino: ino}, true }, CommandAllowed: func(n string) bool { - return r.allowAllCommands || n == "help" || r.allowedCommands[n] + return r.allowAllCommands || r.allowedCommands[n] }, } if r.stdin != nil { @@ -386,7 +384,7 @@ func (r *Runner) call(ctx context.Context, pos syntax.Pos, args []string) { return builtins.FileID{Dev: dev, Ino: ino}, true }, CommandAllowed: func(cmdName string) bool { - return r.allowAllCommands || cmdName == "help" || r.allowedCommands[cmdName] + return r.allowAllCommands || r.allowedCommands[cmdName] }, RunCommand: runCmd, Proc: r.proc, diff --git a/tests/scenarios/cmd/help/always_available.yaml b/tests/scenarios/cmd/help/always_available.yaml deleted file mode 100644 index 546307d7..00000000 --- a/tests/scenarios/cmd/help/always_available.yaml +++ /dev/null @@ -1,10 +0,0 @@ -description: Help is available even when not in the allowed commands list. -skip_assert_against_bash: true -input: - allowed_commands: ["rshell:echo"] - script: |+ - help -expect: - stdout_contains: ["echo", "help"] - stderr: "" - exit_code: 0 diff --git a/tests/scenarios/cmd/help/restricted_commands.yaml b/tests/scenarios/cmd/help/restricted_commands.yaml index 16883e20..9f7ed8e4 100644 --- a/tests/scenarios/cmd/help/restricted_commands.yaml +++ b/tests/scenarios/cmd/help/restricted_commands.yaml @@ -1,7 +1,7 @@ -description: Help lists only commands allowed by the active policy, plus itself. +description: Help lists only commands allowed by the active policy. skip_assert_against_bash: true input: - allowed_commands: ["rshell:echo"] + allowed_commands: ["rshell:echo", "rshell:help"] script: |+ help expect: From 9355a95b211e2671488c5ffd48bf70279c786d88 Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Tue, 7 Apr 2026 10:38:34 -0400 Subject: [PATCH 2/2] fix(tests): update help tests to require rshell:help in allowed commands Tests that restricted commands but expected help to be implicitly available now explicitly include rshell:help. TestHelpAlwaysAvailable replaced with TestHelpNotAllowedWhenNotInList that verifies help is blocked when not in the allowlist. Co-Authored-By: Claude Opus 4.6 (1M context) --- builtins/tests/help/help_test.go | 34 ++++++++++++-------------------- 1 file changed, 13 insertions(+), 21 deletions(-) diff --git a/builtins/tests/help/help_test.go b/builtins/tests/help/help_test.go index ff7ec50f..ccb9d026 100644 --- a/builtins/tests/help/help_test.go +++ b/builtins/tests/help/help_test.go @@ -167,20 +167,19 @@ func TestHelpColumnsAligned(t *testing.T) { func TestHelpRestrictedShowsOnlyAllowed(t *testing.T) { stdout, stderr, code := runScript(t, "help", "", - interp.AllowedCommands([]string{"rshell:echo"})) + interp.AllowedCommands([]string{"rshell:echo", "rshell:help"})) assert.Equal(t, 0, code) assert.Empty(t, stderr) assert.Contains(t, stdout, "echo") - assert.Contains(t, stdout, "help") // help is always listed + assert.Contains(t, stdout, "help") assert.NotContains(t, stdout, "cat") assert.NotContains(t, stdout, "grep") assert.NotContains(t, stdout, "ls") } func TestHelpRestrictedSingleCommand(t *testing.T) { - // Only "ls" is explicitly allowed; help should still appear. stdout, _, code := runScript(t, "help", "", - interp.AllowedCommands([]string{"rshell:ls"})) + interp.AllowedCommands([]string{"rshell:ls", "rshell:help"})) assert.Equal(t, 0, code) assert.Contains(t, stdout, "help") assert.Contains(t, stdout, "ls") @@ -188,10 +187,10 @@ func TestHelpRestrictedSingleCommand(t *testing.T) { } func TestHelpRestrictedAlignmentAdjusts(t *testing.T) { - // With "wc" (2-char) and "strings" (7-char) plus implicit "help" (4-char), + // With "wc" (2-char), "strings" (7-char), and "help" (4-char), // the column width should match the longest allowed name. stdout, _, code := runScript(t, "help", "", - interp.AllowedCommands([]string{"rshell:wc", "rshell:strings"})) + interp.AllowedCommands([]string{"rshell:wc", "rshell:strings", "rshell:help"})) assert.Equal(t, 0, code) lines := strings.Split(strings.TrimSpace(stdout), "\n") @@ -210,27 +209,20 @@ func TestHelpRestrictedAlignmentAdjusts(t *testing.T) { } } -func TestHelpAlwaysAvailable(t *testing.T) { - // help is not in the allowed list, but should still run. - stdout, stderr, code := runScript(t, "help", "", +func TestHelpNotAllowedWhenNotInList(t *testing.T) { + // help is not in the allowed list and should be blocked. + _, stderr, code := runScript(t, "help", "", interp.AllowedCommands([]string{"rshell:echo", "rshell:ls"})) - assert.Equal(t, 0, code) - assert.Empty(t, stderr) - assert.Contains(t, stdout, "help") - assert.Contains(t, stdout, "echo") - assert.Contains(t, stdout, "ls") - assert.NotContains(t, stdout, "cat") + assert.Equal(t, 127, code) + assert.Contains(t, stderr, "command not allowed") } func TestHelpAlwaysAvailableNoCommands(t *testing.T) { // Even with an empty allowed list, help should work. - stdout, stderr, code := runScript(t, "help", "", + _, stderr, code := runScript(t, "help", "", interp.AllowedCommands([]string{})) - assert.Equal(t, 0, code) - assert.Empty(t, stderr) - // Only help itself should be listed. - assert.Contains(t, stdout, "help") - assert.NotContains(t, stdout, "echo") + assert.Equal(t, 127, code) + assert.Contains(t, stderr, "command not allowed") } // --- Error handling ---