From 5579ffba516c937914375c29c04b73127780518a Mon Sep 17 00:00:00 2001 From: Kimmo Lehto Date: Mon, 6 Jul 2026 01:16:44 +0300 Subject: [PATCH] feat(cmd): add CommandGate for per-command confirmation (#400) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Restores rig v0.x's built-in per-command confirmation as an injectable gate rather than a global. A CommandGate is consulted in Executor.Start, the single chokepoint every Exec/Start/remotefs/service call funnels through, with the fully decorated (sudo-wrapped), PowerShell-decoded, redacted command — so it fires exactly once per command at the outermost runner and can show a human-readable prompt without leaking secrets. The gate is set once on the Client via WithCommandGate / WithConfirmFunc and propagated to every derived runner (sudo clone, filesystem, services) the same way the logger is injected, avoiding the leaky Runner-wrapper approach that misses the StartProcess chaining path. Internal privilege-detection probes (sudo/doas availability, UID0 and Windows admin checks) run Ungated so a rejecting gate cannot silently disable sudo instead of surfacing an error. Adds cmd.CommandGate, cmd.CommandGateFunc, cmd.CommandGateSetter, cmd.ErrCommandRejected, the cmd.Ungated() exec option, and the rig.WithCommandGate / rig.WithConfirmFunc client options. Signed-off-by: Kimmo Lehto --- client.go | 55 ++++++++++-- client_test.go | 198 ++++++++++++++++++++++++++++++++++++++++++++ clientoptions.go | 55 +++++++++++- cmd/confirm.go | 55 ++++++++++++ cmd/confirm_test.go | 88 ++++++++++++++++++++ cmd/execoptions.go | 16 ++++ cmd/executor.go | 93 +++++++++++++++++---- cmd/runner.go | 3 + example_test.go | 37 +++++++++ sudo/doas.go | 4 +- sudo/noop.go | 4 +- sudo/sudo.go | 4 +- sudo/windows.go | 4 +- 13 files changed, 587 insertions(+), 29 deletions(-) create mode 100644 cmd/confirm.go create mode 100644 cmd/confirm_test.go diff --git a/client.go b/client.go index dfb488e4..362552d5 100644 --- a/client.go +++ b/client.go @@ -228,6 +228,7 @@ func (c *Client) setup(opts ...ClientOption) error { c.Runner = c.options.GetRunner(c.connection) log.InjectLogger(logger, c.Runner) + c.injectCommandGate(c.Runner) c.SudoProvider = c.options.GetSudoProvider(c.Runner) c.InitSystemProvider = c.options.GetInitSystemProvider(c.Runner) @@ -238,6 +239,24 @@ func (c *Client) setup(opts ...ClientOption) error { return c.initErr } +// injectCommandGate installs the configured [cmd.CommandGate] onto the given +// runner when the runner supports it. This is how a gate set once on the client +// reaches every derived runner: the base runner here, and sudo runners via the +// re-run of setup during [Client.Clone]. A nil configured gate is applied too, +// clearing any gate a reused runner may already carry so that +// WithCommandGate(nil) reliably disables gating. +func (c *Client) injectCommandGate(runner cmd.Runner) { + setter, ok := runner.(cmd.CommandGateSetter) + if !ok { + if c.options.commandGate != nil { + c.Log().Warn("command gate configured but the runner does not support it; commands will run ungated", + "runner", fmt.Sprintf("%T", runner)) + } + return + } + setter.SetCommandGate(c.options.commandGate) +} + // Service returns a manager for a named service on the remote host using // the host's init system if one can be detected. This can be used to // start, stop, restart, and check the status of services. @@ -371,14 +390,38 @@ var errInteractiveNotSupported = errors.New("the connection does not provide int // ExecInteractive executes a command on the host and passes stdin/stdout/stderr as-is to the session. // The session is terminated when ctx is cancelled or its deadline is exceeded. -func (c *Client) ExecInteractive(ctx context.Context, cmd string, stdin io.Reader, stdout, stderr io.Writer) error { - if conn, ok := c.connection.(protocol.InteractiveExecer); ok { - if err := conn.ExecInteractive(ctx, cmd, stdin, stdout, stderr); err != nil { - return fmt.Errorf("exec interactive: %w", err) +// +// A configured [cmd.CommandGate] is consulted for the command before the +// session starts. Because interactive exec runs directly on the connection +// rather than through the runner, the gate sees the raw command as given here, +// without sudo/shell decoration or secret redaction, and commands typed inside +// the interactive session are not gated. +func (c *Client) ExecInteractive(ctx context.Context, command string, stdin io.Reader, stdout, stderr io.Writer) error { + conn, ok := c.connection.(protocol.InteractiveExecer) + if !ok { + return errInteractiveNotSupported + } + // Short-circuit a cancelled/expired context before consulting the gate, so + // a gate implementation is never asked to prompt for a doomed session. This + // mirrors Executor.Start. + if err := ctx.Err(); err != nil { + return fmt.Errorf("exec interactive: %w", err) + } + if gate := c.options.commandGate; gate != nil { + if err := gate.AllowCommand(ctx, c.String(), command); err != nil { + // A context cancellation/deadline is not a rejection: wrap it + // without cmd.ErrCommandRejected so errors.Is reports the context + // error but not a rejection, letting callers distinguish the two. + if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) { + return fmt.Errorf("exec interactive: %w", err) + } + return fmt.Errorf("exec interactive: %w: %w", cmd.ErrCommandRejected, err) } - return nil } - return errInteractiveNotSupported + if err := conn.ExecInteractive(ctx, command, stdin, stdout, stderr); err != nil { + return fmt.Errorf("exec interactive: %w", err) + } + return nil } // The provider Getters would be available and working via the embedding already, but the diff --git a/client_test.go b/client_test.go index 2fdf24cf..61008356 100644 --- a/client_test.go +++ b/client_test.go @@ -4,6 +4,8 @@ import ( "context" "errors" "io" + "strings" + "sync" "testing" "github.com/k0sproject/rig/v2" @@ -13,6 +15,7 @@ import ( "github.com/k0sproject/rig/v2/packagemanager" "github.com/k0sproject/rig/v2/remotefs" "github.com/k0sproject/rig/v2/rigtest" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "gopkg.in/yaml.v2" ) @@ -403,3 +406,198 @@ func TestClientExecInteractiveNotSupported(t *testing.T) { require.Error(t, err) require.ErrorContains(t, err, "interactive") } + +var errNotRoot = errors.New("not root") + +// forceSudo makes the sudo registry deterministically select the "sudo" +// decorator against a mock connection. The mock otherwise succeeds on every +// command, which the root check would read as "already root" and both the sudo +// and doas probes would pass (leaving the choice to registry iteration order). +// It fails the UID0/root probe and the doas probe so only sudo qualifies. +func forceSudo(conn *rigtest.MockConnection) { + conn.AddCommand(rigtest.Contains("id -u"), func(_ *rigtest.A) error { return errNotRoot }) + conn.AddCommand(rigtest.Contains("doas -n"), func(_ *rigtest.A) error { return errNotRoot }) +} + +// gateRecorder records the commands presented to a CommandGate and can be +// configured to allow or deny them. +type gateRecorder struct { + mu sync.Mutex + seen []string + allow bool +} + +func (g *gateRecorder) confirm(_, command string) bool { + g.mu.Lock() + defer g.mu.Unlock() + g.seen = append(g.seen, command) + return g.allow +} + +func (g *gateRecorder) commands() []string { + g.mu.Lock() + defer g.mu.Unlock() + return append([]string(nil), g.seen...) +} + +func TestConfirmFuncGatesSudoCommandOnce(t *testing.T) { + conn := rigtest.NewMockConnection() + forceSudo(conn) + conn.AddCommand(rigtest.Contains("systemctl restart nginx"), func(_ *rigtest.A) error { return nil }) + + rec := &gateRecorder{allow: true} + client, err := rig.NewClient(rig.WithConnection(conn), rig.WithConfirmFunc(rec.confirm)) + require.NoError(t, err) + require.NoError(t, client.Connect(context.Background())) + + require.NoError(t, client.Sudo().ExecContext(context.Background(), "systemctl restart nginx")) + + seen := rec.commands() + // Exactly one prompt: the sudo-availability probe is Ungated and must not + // be presented, and the sudo wrapping must not cause a double prompt. + require.Len(t, seen, 1, "expected exactly one gated command, got %v", seen) + assert.Contains(t, seen[0], "systemctl restart nginx") + assert.Contains(t, seen[0], "sudo -n", "gate must see the fully sudo-wrapped command") +} + +func TestConfirmFuncSudoProbeIsUngated(t *testing.T) { + conn := rigtest.NewMockConnection() + forceSudo(conn) + + rec := &gateRecorder{allow: false} // deny everything + client, err := rig.NewClient(rig.WithConnection(conn), rig.WithConfirmFunc(rec.confirm)) + require.NoError(t, err) + require.NoError(t, client.Connect(context.Background())) + + // Denying the gate must not disable sudo: the probe runs Ungated, so the + // sudo runner is a real sudo executor and rejection surfaces on the actual + // command rather than silently downgrading to a non-sudo runner. + err = client.Sudo().ExecContext(context.Background(), "id") + require.Error(t, err) + assert.ErrorIs(t, err, cmd.ErrCommandRejected) + + // All privilege probes (sudo/doas availability, UID-0) are Ungated, so the + // only command the gate should have seen is the real, sudo-wrapped "id". + // A leaked probe would show up as an extra entry here. + seen := rec.commands() + require.Len(t, seen, 1, "gate must see only the real command, not the ungated probes; got %v", seen) + assert.Contains(t, seen[0], "id", "gate must see the real command") + assert.NotContains(t, seen[0], "true", "the sudo probe (true) must not be presented to the gate") +} + +func TestConfirmFuncRejectionBlocksExecution(t *testing.T) { + conn := rigtest.NewMockConnection() + conn.AddCommand(rigtest.Match("."), func(_ *rigtest.A) error { return nil }) + + rec := &gateRecorder{allow: false} + client, err := rig.NewClient(rig.WithConnection(conn), rig.WithConfirmFunc(rec.confirm)) + require.NoError(t, err) + require.NoError(t, client.Connect(context.Background())) + + err = client.ExecContext(context.Background(), "rm -rf /data") + require.Error(t, err) + assert.ErrorIs(t, err, cmd.ErrCommandRejected) + assert.NoError(t, conn.NotReceived(rigtest.Contains("rm -rf /data")), "rejected command must not reach the host") +} + +func TestCommandGateContextErrorNotRejection(t *testing.T) { + conn := rigtest.NewMockConnection() + conn.AddCommand(rigtest.Match("."), func(_ *rigtest.A) error { return nil }) + + // A gate that returns a context error (e.g. it honored cancellation while + // prompting) must surface as cancellation, not as an explicit rejection. + gate := cmd.CommandGateFunc(func(_ context.Context, _, _ string) error { + return context.Canceled + }) + client, err := rig.NewClient(rig.WithConnection(conn), rig.WithCommandGate(gate)) + require.NoError(t, err) + require.NoError(t, client.Connect(context.Background())) + + err = client.ExecContext(context.Background(), "uptime") + require.Error(t, err) + assert.ErrorIs(t, err, context.Canceled, "context error must pass through") + assert.NotErrorIs(t, err, cmd.ErrCommandRejected, "cancellation must not look like a rejection") + assert.NoError(t, conn.NotReceived(rigtest.Contains("uptime")), "command must not reach the host") +} + +func TestConfirmFuncNilDisablesGating(t *testing.T) { + conn := rigtest.NewMockConnection() + conn.AddCommand(rigtest.Match("."), func(_ *rigtest.A) error { return nil }) + + client, err := rig.NewClient(rig.WithConnection(conn), rig.WithConfirmFunc(nil)) + require.NoError(t, err) + require.NoError(t, client.Connect(context.Background())) + + // A nil confirm func must disable gating rather than panic on invocation. + require.NoError(t, client.ExecContext(context.Background(), "echo hello")) + require.NoError(t, conn.Received(rigtest.Contains("echo hello"))) +} + +func TestConfirmFuncGatesExecInteractive(t *testing.T) { + conn := &interactiveConn{MockConnection: rigtest.NewMockConnection()} + + rec := &gateRecorder{allow: true} + client, err := rig.NewClient(rig.WithConnection(conn), rig.WithConfirmFunc(rec.confirm)) + require.NoError(t, err) + require.NoError(t, client.Connect(context.Background())) + + require.NoError(t, client.ExecInteractive(context.Background(), "top", nil, nil, nil)) + assert.Equal(t, "top", conn.receivedCmd, "allowed interactive command must start the session") + assert.Contains(t, rec.commands(), "top", "the interactive command must be presented to the gate") +} + +func TestConfirmFuncRejectsExecInteractive(t *testing.T) { + conn := &interactiveConn{MockConnection: rigtest.NewMockConnection()} + + rec := &gateRecorder{allow: false} + client, err := rig.NewClient(rig.WithConnection(conn), rig.WithConfirmFunc(rec.confirm)) + require.NoError(t, err) + require.NoError(t, client.Connect(context.Background())) + + err = client.ExecInteractive(context.Background(), "top", nil, nil, nil) + require.Error(t, err) + assert.ErrorIs(t, err, cmd.ErrCommandRejected) + assert.Empty(t, conn.receivedCmd, "rejected interactive command must not start the session") +} + +func TestExecInteractiveCancelledContextSkipsGate(t *testing.T) { + conn := &interactiveConn{MockConnection: rigtest.NewMockConnection()} + + rec := &gateRecorder{allow: true} + client, err := rig.NewClient(rig.WithConnection(conn), rig.WithConfirmFunc(rec.confirm)) + require.NoError(t, err) + require.NoError(t, client.Connect(context.Background())) + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + err = client.ExecInteractive(ctx, "top", nil, nil, nil) + require.Error(t, err) + assert.ErrorIs(t, err, context.Canceled, "cancelled context must propagate") + assert.NotErrorIs(t, err, cmd.ErrCommandRejected, "cancellation must not look like a rejection") + assert.Empty(t, rec.commands(), "gate must not be consulted for a cancelled session") + assert.Empty(t, conn.receivedCmd, "cancelled session must not start") +} + +func TestConfirmFuncGatesFilesystemOps(t *testing.T) { + conn := rigtest.NewMockConnection() + forceSudo(conn) + + rec := &gateRecorder{allow: true} + client, err := rig.NewClient(rig.WithConnection(conn), rig.WithConfirmFunc(rec.confirm)) + require.NoError(t, err) + require.NoError(t, client.Connect(context.Background())) + + // Touch the filesystem service on the sudo client; its lazy detection runs + // commands through the sudo runner, proving the gate propagates to FS. + _, _ = client.Sudo().FS().Stat("/etc/hostname") + + var sawSudoWrapped bool + for _, c := range rec.commands() { + if strings.Contains(c, "sudo -n") { + sawSudoWrapped = true + break + } + } + assert.True(t, sawSudoWrapped, "filesystem operations on a sudo client must be gated, got %v", rec.commands()) +} diff --git a/clientoptions.go b/clientoptions.go index d455d89a..cbdf2698 100644 --- a/clientoptions.go +++ b/clientoptions.go @@ -1,6 +1,7 @@ package rig import ( + "context" "errors" "fmt" @@ -14,7 +15,10 @@ import ( "github.com/k0sproject/rig/v2/sudo" ) -var errNilOSRelease = errors.New("os release provider returned nil release without error") +var ( + errNilOSRelease = errors.New("os release provider returned nil release without error") + errConfirmDeclined = errors.New("declined by confirmation callback") +) // ConnectionFactory can create connections. When a connection is not given, the factory is used // to build a connection. @@ -33,6 +37,7 @@ type ClientOptions struct { connection protocol.Connection connectionFactory ConnectionFactory runner cmd.Runner + commandGate cmd.CommandGate retryConnection bool providersContainer } @@ -135,6 +140,7 @@ func (o *ClientOptions) Clone() *ClientOptions { connection: o.connection, connectionFactory: o.connectionFactory, runner: o.runner, + commandGate: o.commandGate, retryConnection: o.retryConnection, providersContainer: o.providersContainer, } @@ -201,6 +207,53 @@ func WithRunner(runner cmd.Runner) ClientOption { } } +// WithCommandGate installs a [cmd.CommandGate] that is consulted before +// commands run on the client and all of its derived runners (sudo, filesystem, +// services). Returning a non-nil error from the gate aborts the command, +// wrapped in [cmd.ErrCommandRejected]; a context cancellation/deadline error is +// the exception and propagates as a context error without [cmd.ErrCommandRejected]. +// Use [WithConfirmFunc] for the common case of a yes/no confirmation prompt. +// See [cmd.CommandGate] for exactly which commands are gated and which internal +// paths are not. +// +// The gate reaches a runner only if that runner implements +// [cmd.CommandGateSetter]. The default runner does; a custom runner supplied +// via [WithRunner] that does not implement it runs ungated, and the client +// logs a warning during setup. +func WithCommandGate(gate cmd.CommandGate) ClientOption { + return func(o *ClientOptions) { + o.commandGate = gate + } +} + +// WithConfirmFunc installs a confirmation callback consulted before commands +// run on the client and all of its derived runners (sudo, filesystem, +// services). The callback receives the host string and the human-readable, +// redacted command; returning false aborts the command with +// [cmd.ErrCommandRejected]. This is a convenience wrapper around +// [WithCommandGate]; see [cmd.CommandGate] for which commands are gated. A nil +// fn disables gating, matching WithCommandGate(nil). +// +// One exception: [Client.ExecInteractive] runs directly on the connection, so +// its command reaches the callback raw — undecorated and unredacted. Avoid +// logging the callback argument as-is if interactive commands may carry +// secrets. +func WithConfirmFunc(fn func(host, command string) bool) ClientOption { + if fn == nil { + return WithCommandGate(nil) + } + gate := cmd.CommandGateFunc(func(ctx context.Context, host, command string) error { + if err := ctx.Err(); err != nil { + return err //nolint:wrapcheck // pass the context error through unchanged so it isn't misread as a rejection + } + if fn(host, command) { + return nil + } + return errConfirmDeclined + }) + return WithCommandGate(gate) +} + // WithConnectionFactory is a functional option that sets the connection factory to use for connecting. func WithConnectionFactory(factory ConnectionFactory) ClientOption { return func(o *ClientOptions) { diff --git a/cmd/confirm.go b/cmd/confirm.go new file mode 100644 index 00000000..f4456c73 --- /dev/null +++ b/cmd/confirm.go @@ -0,0 +1,55 @@ +package cmd + +import "context" + +// CommandGate is consulted before a command runs, on the [Executor] execution +// path that every Exec/Start call (and thus every remotefs and service call) +// funnels through. Returning a non-nil error aborts the command before it +// starts; the error is propagated to the caller wrapped in [ErrCommandRejected]. +// +// A context cancellation or deadline error is the one exception: it is +// propagated without [ErrCommandRejected], so errors.Is reports the context +// error but not a rejection. Callers that treat rejections specially can thus +// distinguish an explicit refusal from a cancelled/expired context. +// +// A gate fires once per command at the outermost runner, so a command that is +// wrapped by sudo (or any other decorator chain) is presented in its final, +// fully wrapped form exactly once. On that path the command argument is the +// human-readable form — decorated, sudo-wrapped, PowerShell-decoded, and with +// any registered secrets redacted — suitable for a confirmation prompt or +// audit log. +// +// The following do not go through this path and are therefore not gated: +// - [Executor.StartProcess], the low-level primitive used for runner chaining. +// - The built-in Windows-detection probe (ver.exe), which runs via +// StartProcess at runner construction to decide command formatting. +// - Privilege-escalation probes (sudo/doas availability, UID-0 and +// Windows-admin checks), which run [Ungated] so a rejecting gate surfaces +// an error rather than silently disabling sudo. +// +// [github.com/k0sproject/rig/v2.Client.ExecInteractive] consults the gate +// separately, with the raw (undecorated, unredacted) command. +// +// AllowCommand may block (for example to prompt on stdin); it should honor the +// supplied context and return its error when the caller cancels. +type CommandGate interface { + // AllowCommand reports whether the command may run on the named host. + // A nil return allows the command; a non-nil error aborts it. + AllowCommand(ctx context.Context, host, command string) error +} + +// CommandGateFunc adapts an ordinary function to the [CommandGate] interface. +type CommandGateFunc func(ctx context.Context, host, command string) error + +// AllowCommand implements [CommandGate]. +func (f CommandGateFunc) AllowCommand(ctx context.Context, host, command string) error { + return f(ctx, host, command) +} + +// CommandGateSetter is implemented by runners that support installing a +// [CommandGate]. [Executor] implements it, allowing a gate configured on a +// [github.com/k0sproject/rig/v2.Client] to be propagated to every runner the +// client derives (sudo, filesystem, services). +type CommandGateSetter interface { + SetCommandGate(gate CommandGate) +} diff --git a/cmd/confirm_test.go b/cmd/confirm_test.go new file mode 100644 index 00000000..19371864 --- /dev/null +++ b/cmd/confirm_test.go @@ -0,0 +1,88 @@ +package cmd_test + +import ( + "context" + "errors" + "testing" + + "github.com/k0sproject/rig/v2/cmd" + "github.com/k0sproject/rig/v2/rigtest" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestCommandGateAllows(t *testing.T) { + conn := rigtest.NewMockConnection() + conn.AddCommand(rigtest.Equal("echo hi"), func(_ *rigtest.A) error { return nil }) + runner := cmd.NewExecutor(conn) + + var seen string + runner.SetCommandGate(cmd.CommandGateFunc(func(_ context.Context, _, command string) error { + seen = command + return nil + })) + + require.NoError(t, runner.Exec("echo hi")) + assert.Equal(t, "echo hi", seen, "gate should receive the formatted command") + rigtest.ReceivedEqual(t, conn, "echo hi") +} + +func TestCommandGateRejects(t *testing.T) { + conn := rigtest.NewMockConnection() + conn.AddCommand(rigtest.Match("."), func(_ *rigtest.A) error { return nil }) + runner := cmd.NewExecutor(conn) + + denied := errors.New("nope") + runner.SetCommandGate(cmd.CommandGateFunc(func(_ context.Context, _, _ string) error { + return denied + })) + + err := runner.Exec("rm -rf /") + require.Error(t, err) + assert.ErrorIs(t, err, cmd.ErrCommandRejected, "error must wrap ErrCommandRejected") + assert.ErrorIs(t, err, denied, "error must wrap the gate's own error") + assert.Zero(t, conn.Len(), "a rejected command must never reach the connection") +} + +func TestCommandGateReceivesDecoratedCommand(t *testing.T) { + conn := rigtest.NewMockConnection() + conn.AddCommand(rigtest.Match("."), func(_ *rigtest.A) error { return nil }) + runner := cmd.NewExecutor(conn, func(c string) string { return "sudo " + c }) + + var seen string + runner.SetCommandGate(cmd.CommandGateFunc(func(_ context.Context, _, command string) error { + seen = command + return nil + })) + + require.NoError(t, runner.Exec("whoami")) + assert.Equal(t, "sudo whoami", seen, "gate must see the fully decorated command") +} + +func TestCommandGateRedactsSecrets(t *testing.T) { + conn := rigtest.NewMockConnection() + conn.AddCommand(rigtest.Match("."), func(_ *rigtest.A) error { return nil }) + runner := cmd.NewExecutor(conn) + + secret := "s3cr3t" + var seen string + runner.SetCommandGate(cmd.CommandGateFunc(func(_ context.Context, _, command string) error { + seen = command + return nil + })) + + require.NoError(t, runner.Exec("login --token "+secret, cmd.Redact(secret))) + assert.NotContains(t, seen, secret, "gate must receive the redacted command") + assert.Contains(t, seen, cmd.DefaultRedactMask) +} + +func TestCommandGateNilHasNoEffect(t *testing.T) { + conn := rigtest.NewMockConnection() + conn.AddCommand(rigtest.Equal("echo hi"), func(_ *rigtest.A) error { return nil }) + runner := cmd.NewExecutor(conn) + + runner.SetCommandGate(nil) + + require.NoError(t, runner.Exec("echo hi")) + rigtest.ReceivedEqual(t, conn, "echo hi") +} diff --git a/cmd/execoptions.go b/cmd/execoptions.go index 2c3db576..7076a69f 100644 --- a/cmd/execoptions.go +++ b/cmd/execoptions.go @@ -51,6 +51,8 @@ type ExecOptions struct { traceOut io.Writer traceErr io.Writer outputClosers []io.Closer + + skipGate bool } // Format returns the command string with all per-call decorators applied. @@ -71,6 +73,11 @@ func (o *ExecOptions) LogCommand() bool { return o.logCommand } +// SkipGate returns true if the [CommandGate] should not be consulted for this command. +func (o *ExecOptions) SkipGate() bool { + return o.skipGate +} + var ( errCharDevice = errors.New("reader is a character device") errUnknownReader = errors.New("unknown type of reader") @@ -337,6 +344,15 @@ func Logger(l log.Logger) ExecOption { } } +// Ungated exec option for exempting a single command from the runner's +// [CommandGate]. Use it for internal probes and detection commands that must +// run unconditionally, so a rejecting gate cannot silently change behavior. +func Ungated() ExecOption { + return func(o *ExecOptions) { + o.skipGate = true + } +} + // Trace exec option for attaching a [Tracer] to a single command execution. // If the Tracer also implements [OutputTracer], per-line stdout and stderr // hooks are enabled automatically. diff --git a/cmd/executor.go b/cmd/executor.go index 3f92e054..f5ed13a9 100644 --- a/cmd/executor.go +++ b/cmd/executor.go @@ -50,6 +50,7 @@ type Executor struct { isWin func() bool osKnown atomic.Bool tracer Tracer + gate CommandGate } func isWinFunc(conn protocol.ProcessStarter) func() bool { @@ -82,6 +83,16 @@ func (r *Executor) SetTracer(t Tracer) { r.tracer = t } +// SetCommandGate installs a [CommandGate] that is consulted by the Start and +// Exec family of methods before a command runs. A nil gate disables gating. +// The gate is not consulted by [Executor.StartProcess], which exists only to +// satisfy the connection interface for runner chaining. SetCommandGate must +// not be called concurrently with Start, Exec, or any other command-execution +// method. +func (r *Executor) SetCommandGate(gate CommandGate) { + r.gate = gate +} + func (r *Executor) formatCommandForOS(command string, execOpts *ExecOptions, isWindows bool) string { cmd := r.Format(execOpts.Format(command)) if isWindows && !isExe(cmd) { @@ -285,9 +296,38 @@ func decodeEncoded(cmd string) string { return strings.Join(parts, " ") } +// closeAll closes every closer, ignoring individual close errors. +func closeAll(closers []io.Closer) { + for _, c := range closers { + _ = c.Close() + } +} + +// gateCommand consults the runner's [CommandGate] for the given command, which +// must already be in redacted, human-readable form. It returns nil when there +// is no gate, when the command opts out via [Ungated], or when the gate allows +// the command. A non-nil return means the command must not be started: a +// rejection wraps [ErrCommandRejected], while a context cancellation/deadline +// is wrapped without [ErrCommandRejected] so that errors.Is reports the context +// error but not a rejection, letting callers tell the two apart. +func (r *Executor) gateCommand(ctx context.Context, execOpts *ExecOptions, redacted string) error { + if r.gate == nil || execOpts.SkipGate() { + return nil + } + err := r.gate.AllowCommand(ctx, r.String(), redacted) + if err == nil { + return nil + } + if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) { + log.Trace(ctx, "command gate cancelled", log.HostAttr(r), log.KeyCommand, redacted, log.KeyError, err) + return fmt.Errorf("command gate: %w", err) + } + log.Trace(ctx, "command rejected by gate", log.HostAttr(r), log.KeyCommand, redacted, log.KeyError, err) + return fmt.Errorf("%w: %w", ErrCommandRejected, err) +} + // Start starts the command and returns a Waiter. func (r *Executor) Start(ctx context.Context, command string, opts ...ExecOption) (protocol.Waiter, error) { - log.Trace(ctx, "starting command", log.HostAttr(r), log.KeyCommand, command) if ctx.Err() != nil { return nil, fmt.Errorf("runner context error: %w", ctx.Err()) } @@ -320,8 +360,24 @@ func (r *Executor) Start(ctx context.Context, command string, opts ...ExecOption // non-prefixed commands through cmd.exe. cmd := r.formatCommand(command, execOpts) + // redactedCmd is the human-readable command (secrets masked, PowerShell + // -EncodedCommand decoded), computed once and reused for gating and every + // log line below. Tracer callbacks deliberately receive the raw formatted + // cmd, as their contract documents. + redactedCmd := execOpts.Redact(decodeEncoded(cmd)) + + // Consult the gate before emitting any command-logging or tracer + // lifecycle events so a rejected command produces no dangling + // "executing"/CommandFormatted/ProcessStarted events. + if err := r.gateCommand(ctx, execOpts, redactedCmd); err != nil { + closeAll(traceClosers) + return nil, err + } + + log.Trace(ctx, "starting command", log.HostAttr(r), log.KeyCommand, redactedCmd) + if execOpts.LogCommand() { - r.Log().Debug("executing command", log.KeyCommand, execOpts.Redact(decodeEncoded(cmd))) + r.Log().Debug("executing command", log.KeyCommand, redactedCmd) } if tracer != nil { @@ -334,18 +390,14 @@ func (r *Executor) Start(ctx context.Context, command string, opts ...ExecOption waiter, err := r.connection.StartProcess(ctx, cmd, execOpts.Stdin(), stdout, stderr) //nolint:contextcheck // Stdin() uses trace logger which takes context if err != nil { - for _, c := range traceClosers { - _ = c.Close() - } - log.Trace(ctx, "start process failed", log.HostAttr(r), log.KeyCommand, cmd, log.KeyError, err) + closeAll(traceClosers) + log.Trace(ctx, "start process failed", log.HostAttr(r), log.KeyCommand, redactedCmd, log.KeyError, err) return nil, fmt.Errorf("runner start command: %w", err) } if waiter == nil { - for _, c := range traceClosers { - _ = c.Close() - } - log.Trace(ctx, "start process returned nil waiter", log.HostAttr(r), log.KeyCommand, cmd, log.KeyError, err) + closeAll(traceClosers) + log.Trace(ctx, "start process returned nil waiter", log.HostAttr(r), log.KeyCommand, redactedCmd, log.KeyError, errInternal) return nil, fmt.Errorf("%w: connection returned no error but a nil waiter", errInternal) } @@ -405,20 +457,22 @@ func (r *Executor) ExecOutputContext(ctx context.Context, command string, opts . opts = append(opts, Stdout(out)) - log.Trace(ctx, "starting command for execoutput", log.HostAttr(r), log.KeyCommand, command) + // Start gates the command and logs it once in its canonical redacted, + // decoded, fully decorated form. These traces only mark the execoutput + // lifecycle, so they deliberately omit the command to avoid re-deriving + // (and drifting from) that canonical form. proc, err := r.Start(ctx, command, opts...) if err != nil { return "", fmt.Errorf("start command: %w", err) } - log.Trace(ctx, "waiting on command", log.HostAttr(r), log.KeyCommand, command) + log.Trace(ctx, "waiting on command", log.HostAttr(r)) if err := proc.Wait(); err != nil { - log.Trace(ctx, "waiting returned an error", log.HostAttr(r), log.KeyCommand, command, log.KeyError, err) + log.Trace(ctx, "waiting returned an error", log.HostAttr(r), log.KeyError, err) return "", fmt.Errorf("command result: %w", err) } - log.Trace(ctx, "command finished", log.HostAttr(r), log.KeyCommand, command) - execOpts := Build(opts...) - return execOpts.FormatOutput(out.String()), nil + log.Trace(ctx, "command finished", log.HostAttr(r)) + return Build(opts...).FormatOutput(out.String()), nil } // ExecOutput executes the command and returns the stdout output or an error. @@ -435,14 +489,17 @@ func (r *Executor) ExecReaderContext(ctx context.Context, command string, opts . return pipeR } opts = append(opts, Stdout(pipeW)) + // ExecContext -> Start gates and logs the command once in its canonical + // redacted, decoded form. These traces only mark the execreader lifecycle, + // so they deliberately omit the command to avoid re-deriving it here. go func() { if err := r.ExecContext(ctx, command, opts...); err != nil { - log.Trace(ctx, "execreader: execcontext returned an error", log.HostAttr(r), log.KeyCommand, command, log.KeyError, err) + log.Trace(ctx, "execreader: execcontext returned an error", log.HostAttr(r), log.KeyError, err) pipeW.CloseWithError(fmt.Errorf("exec reader: %w", err)) } else { pipeW.Close() } - log.Trace(ctx, "execreader: execcontext finished", log.HostAttr(r), log.KeyCommand, command) + log.Trace(ctx, "execreader: execcontext finished", log.HostAttr(r)) }() return pipeR } diff --git a/cmd/runner.go b/cmd/runner.go index 39a4d348..bef7ee61 100644 --- a/cmd/runner.go +++ b/cmd/runner.go @@ -17,6 +17,9 @@ var ( // ErrWroteStderr is returned when a windows command writes to stderr, unless AllowWinStderr is set. ErrWroteStderr = errors.New("command wrote output to stderr") + + // ErrCommandRejected is returned when a [CommandGate] refuses to allow a command to run. + ErrCommandRejected = errors.New("command rejected") ) // DecorateFunc is a function that takes a string and returns a decorated string. diff --git a/example_test.go b/example_test.go index a7899af1..be099ac7 100644 --- a/example_test.go +++ b/example_test.go @@ -133,6 +133,43 @@ func ExampleClient_Sudo() { // uid=0(root) } +// ExampleWithConfirmFunc demonstrates gating every command behind a +// confirmation callback. The callback receives the host and the redacted +// command; returning false aborts the command with [cmd.ErrCommandRejected] +// before it reaches the host. This applies to the client and every runner it +// derives (sudo, filesystem, services). +func ExampleWithConfirmFunc() { + conn := rigtest.NewMockConnection() + conn.AddCommand(rigtest.Match("."), func(_ *rigtest.A) error { return nil }) + + client, err := rig.NewClient( + rig.WithConnection(conn), + rig.WithConfirmFunc(func(_, command string) bool { + allowed := !strings.Contains(command, "rm -rf") + fmt.Printf("confirm %q -> %v\n", command, allowed) + return allowed + }), + ) + if err != nil { + fmt.Println(err) + return + } + + // An approved command runs normally. + if err := client.Exec("systemctl restart nginx"); err != nil { + fmt.Println("restart:", err) + } + + // A rejected command never reaches the host. + if err := client.Exec("rm -rf /important"); errors.Is(err, cmd.ErrCommandRejected) { + fmt.Println("blocked") + } + // Output: + // confirm "systemctl restart nginx" -> true + // confirm "rm -rf /important" -> false + // blocked +} + // ExampleClient_FS demonstrates using Client.FS to read a file from the remote // host via the fs.FS interface. func ExampleClient_FS() { diff --git a/sudo/doas.go b/sudo/doas.go index 2d07d6fb..313f057c 100644 --- a/sudo/doas.go +++ b/sudo/doas.go @@ -16,7 +16,9 @@ func RegisterDoas(repository *Registry) { if c.IsWindows() { return nil, false } - if c.Exec(Doas("true")) != nil { + // Ungated: a CommandGate that rejects this probe would silently + // disable doas rather than surface an error, so it must always run. + if c.Exec(Doas("true"), cmd.Ungated()) != nil { return nil, false } return cmd.NewExecutor(c, Doas), true diff --git a/sudo/noop.go b/sudo/noop.go index bdae3298..5f123657 100644 --- a/sudo/noop.go +++ b/sudo/noop.go @@ -15,7 +15,9 @@ func RegisterUID0Noop(repository *Registry) { if c.IsWindows() { return nil, false } - if c.Exec(`[ "$(id -u)" = 0 ]`) != nil { + // Ungated: a CommandGate that rejects this probe would silently change + // sudo detection, so it must always run. + if c.Exec(`[ "$(id -u)" = 0 ]`, cmd.Ungated()) != nil { return nil, false } return cmd.NewExecutor(c, Noop), true diff --git a/sudo/sudo.go b/sudo/sudo.go index de788d9c..ab6eede3 100644 --- a/sudo/sudo.go +++ b/sudo/sudo.go @@ -16,7 +16,9 @@ func RegisterSudo(repository *Registry) { if c.IsWindows() { return nil, false } - if c.Exec(Sudo("true")) != nil { + // Ungated: a CommandGate that rejects this probe would silently + // disable sudo rather than surface an error, so it must always run. + if c.Exec(Sudo("true"), cmd.Ungated()) != nil { return nil, false } return cmd.NewExecutor(c, Sudo), true diff --git a/sudo/windows.go b/sudo/windows.go index 22339c4f..a155f1f0 100644 --- a/sudo/windows.go +++ b/sudo/windows.go @@ -16,7 +16,9 @@ func RegisterWindowsNoop(repository *Registry) { return nil, false } const isAdminCmd = `if (-not (New-Object Security.Principal.WindowsPrincipal([Security.Principal.WindowsIdentity]::GetCurrent())).IsInRole([Security.Principal.WindowsBuiltInRole]::Administrator)) { exit 1 }` - if runner.Exec(isAdminCmd, cmd.PS()) != nil { + // Ungated: a CommandGate that rejects this probe would silently change + // privilege detection, so it must always run. + if runner.Exec(isAdminCmd, cmd.PS(), cmd.Ungated()) != nil { return nil, false } return cmd.NewExecutor(runner, Noop), true