diff --git a/builtins/help/help.go b/builtins/help/help.go index 927c63813..92cd07042 100644 --- a/builtins/help/help.go +++ b/builtins/help/help.go @@ -81,18 +81,12 @@ func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { return builtins.Result{Code: 1} } - // Use static Help text if available (for commands that don't - // handle --help, like echo, true, false). - if meta.Help != "" { - callCtx.Outf("%s\n", meta.Help) - return builtins.Result{} - } - // RemediationOnly builtins must never have their handler invoked - // outside remediation mode, even via `help --help`-style - // lookup below. This mirrors the dispatch-level gate in interp so - // a builtin that forgets its own remediation check cannot still - // run its handler (and leak whatever it writes) through help. + // outside remediation mode, even via static Help text or the + // `help --help`-style lookup below. This mirrors the + // dispatch-level gate in interp so a builtin that forgets its own + // remediation check cannot still run its handler (and leak + // whatever it writes) through help. if meta.RemediationOnly && !callCtx.RemediationMode { msg := meta.RemediationDeniedMessage if msg == "" { @@ -102,6 +96,13 @@ func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { return builtins.Result{Code: 1} } + // Use static Help text if available (for commands that don't + // handle --help, like echo, true, false). + if meta.Help != "" { + callCtx.Outf("%s\n", meta.Help) + return builtins.Result{} + } + // Otherwise, invoke the command with --help and capture the output. if handler, ok := builtins.Lookup(name); ok && handler != nil { var buf bytes.Buffer diff --git a/interp/check_commands.go b/interp/check_commands.go index 221e77963..b8038cb53 100644 --- a/interp/check_commands.go +++ b/interp/check_commands.go @@ -58,7 +58,18 @@ func (c *scriptChecker) arguments(entry *CommandCheck, args []string, elevated b operation := allowedpaths.PathRead stdinDash := true switch entry.Command { - case "echo", "printf", "true", "false", "exit", "break", "continue", "read", "tr", "uname", "help": + case "help": + // `help ` refuses a RemediationOnly command outside + // remediation mode (builtins/help), the same gate dispatch applies. + if len(operands) == 1 { + if _, isFeature := builtins.Feature(operands[0]); !isFeature { + if message, denied := remediationOnlyRefusal(operands[0], c.policy.remediationMode); denied { + entry.issue(CheckRemediationRequired, message) + } + } + } + return + case "echo", "printf", "true", "false", "exit", "break", "continue", "read", "tr", "uname": return case "df", "free", "ip", "lsof", "pmap", "ps", "ss", "uptime", "vmstat", "ping", "ntfs-du": // These commands use fixed/trusted host capabilities, not user-selected diff --git a/interp/check_test.go b/interp/check_test.go index 8d5e97aca..2adb1e947 100644 --- a/interp/check_test.go +++ b/interp/check_test.go @@ -17,6 +17,7 @@ import ( "testing" "time" + "github.com/DataDog/rshell/builtins" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "mvdan.cc/sh/v3/expand" @@ -452,3 +453,37 @@ func TestCheckElevatedXargsChildRequiresGrant(t *testing.T) { }) } } + +// TestCheckHelpOfRemediationOnlyBuiltinMatchesRun asserts Check never approves +// `help ` for a RemediationOnly builtin outside remediation mode when Run +// refuses it. The builtins are enumerated from the registry so new ones are +// covered. +func TestCheckHelpOfRemediationOnlyBuiltinMatchesRun(t *testing.T) { + checkRunner(t) // registers the builtins + var names []string + for _, name := range builtins.Names() { + if meta, ok := builtins.Meta(name); ok && meta.RemediationOnly { + names = append(names, name) + } + } + require.NotEmpty(t, names) + for _, name := range names { + for _, mode := range []Mode{ModeReadOnly, ModeRemediation} { + t.Run(name+"/"+string(mode), func(t *testing.T) { + script := "help " + name + runner := checkRunner(t, allowAllCommandsOpt(), WithMode(mode)) + result := checkScript(t, runner, script) + program, err := ParseScript(script, "") + require.NoError(t, err) + err = runner.Run(context.Background(), program) + assert.Equal(t, err == nil, result.Allowed, "%+v", result) + if mode == ModeReadOnly { + assert.False(t, result.Allowed, "%+v", result) + assert.True(t, checkHasIssue(result, CheckRemediationRequired), "%+v", result) + } else { + assert.True(t, result.Allowed, "%+v", result) + } + }) + } + } +}