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 --- 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: