Skip to content

feat(cmd): add CommandGate for per-command confirmation (#400)#401

Merged
kke merged 1 commit into
mainfrom
feat/command-confirmation
Jul 6, 2026
Merged

feat(cmd): add CommandGate for per-command confirmation (#400)#401
kke merged 1 commit into
mainfrom
feat/command-confirmation

Conversation

@kke

@kke kke commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Fixes #400

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.

@kke kke added the enhancement New feature or request label Jul 5, 2026
@kke
kke requested a review from Copilot July 5, 2026 22:32

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds an injectable per-command confirmation mechanism (“CommandGate”) that is consulted at the cmd.Executor.Start chokepoint, enabling a single prompt per fully-decorated command (including sudo wrapping), with support for redaction and an Ungated() escape hatch for internal probes.

Changes:

  • Introduces cmd.CommandGate (+ helpers) and wires gate checks into cmd.Executor.Start, returning cmd.ErrCommandRejected on denial.
  • Adds cmd.Ungated() and applies it to privilege-detection probes (sudo/doas/uid0/Windows admin checks) to avoid changing detection behavior under a rejecting gate.
  • Adds client options rig.WithCommandGate / rig.WithConfirmFunc and tests/examples covering propagation to derived runners (sudo/FS/services) and rejection behavior.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
sudo/windows.go Marks Windows admin probe as Ungated() so confirmation doesn’t alter privilege detection.
sudo/sudo.go Marks sudo availability probe as Ungated().
sudo/noop.go Marks UID0/root probe as Ungated().
sudo/doas.go Marks doas availability probe as Ungated().
example_test.go Adds an example demonstrating WithConfirmFunc behavior and output.
cmd/runner.go Adds ErrCommandRejected error sentinel.
cmd/executor.go Adds gate plumbing to Executor and consults the gate in Start; refactors closer cleanup via closeAll.
cmd/execoptions.go Adds skipGate option + Ungated() exec option to bypass the gate per-command.
cmd/confirm.go Introduces CommandGate, CommandGateFunc, and CommandGateSetter API.
cmd/confirm_test.go Unit tests for gate allow/deny, decoration visibility, and redaction.
clientoptions.go Adds WithCommandGate / WithConfirmFunc options and stores the configured gate in client options.
client.go Injects the configured gate into runners that support CommandGateSetter.
client_confirm_test.go Integration tests verifying gating occurs once per sudo-wrapped command and propagates to derived FS operations.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread cmd/executor.go Outdated
Comment thread clientoptions.go Outdated
Comment thread cmd/executor.go Outdated
@kke
kke force-pushed the feat/command-confirmation branch from 6210ebc to da79c16 Compare July 6, 2026 05:52
@kke
kke requested a review from Copilot July 6, 2026 05:53

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Comment thread cmd/executor.go Outdated
@kke
kke force-pushed the feat/command-confirmation branch from da79c16 to 12ec7cb Compare July 6, 2026 06:00
@kke
kke requested a review from Copilot July 6, 2026 06:00

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.

Comment thread cmd/executor.go Outdated
Comment thread cmd/executor.go Outdated
@kke
kke force-pushed the feat/command-confirmation branch from 12ec7cb to 07fc95a Compare July 6, 2026 06:20
@kke
kke requested a review from Copilot July 6, 2026 06:21

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Comment thread cmd/executor.go Outdated
@kke
kke force-pushed the feat/command-confirmation branch from 07fc95a to bf28063 Compare July 6, 2026 06:29
@kke
kke requested a review from Copilot July 6, 2026 06:29

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.

Comment thread clientoptions.go Outdated
Comment thread cmd/executor.go Outdated
Comment thread cmd/confirm.go Outdated
@kke
kke force-pushed the feat/command-confirmation branch from bf28063 to aa4232e Compare July 6, 2026 07:20
@kke
kke requested a review from Copilot July 6, 2026 07:20

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.

Comment thread cmd/confirm.go Outdated
Comment thread client.go
@kke
kke force-pushed the feat/command-confirmation branch from aa4232e to e02480f Compare July 6, 2026 08:11
@kke
kke requested a review from Copilot July 6, 2026 08:12

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.

Comment thread cmd/confirm.go Outdated
Comment thread clientoptions.go Outdated
@kke
kke force-pushed the feat/command-confirmation branch from e02480f to c44f591 Compare July 6, 2026 08:27
@kke
kke requested a review from Copilot July 6, 2026 08:27

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.

Comment thread clientoptions.go
Comment thread cmd/executor.go
@kke
kke force-pushed the feat/command-confirmation branch from 380ab0a to 4872550 Compare July 6, 2026 09:00
@kke
kke requested a review from Copilot July 6, 2026 09:00

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.

Comment thread cmd/executor.go Outdated
Comment thread client.go
Comment thread clientoptions.go
@kke
kke force-pushed the feat/command-confirmation branch from 4872550 to 3373b6c Compare July 6, 2026 10:30
@kke
kke requested a review from Copilot July 6, 2026 10:30

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.

Comment thread cmd/confirm.go
Comment thread cmd/executor.go Outdated
Comment thread client.go Outdated
@kke
kke force-pushed the feat/command-confirmation branch from 3373b6c to dc61a79 Compare July 6, 2026 10:36
@kke
kke requested a review from Copilot July 6, 2026 10:36

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.

Comment thread cmd/executor.go Outdated
Comment thread cmd/executor.go
@kke
kke force-pushed the feat/command-confirmation branch from dc61a79 to 34a72be Compare July 6, 2026 10:52
@kke
kke requested a review from Copilot July 6, 2026 10:52

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.

Comment thread cmd/executor.go
Comment thread clientoptions.go Outdated
Comment thread client.go
@kke
kke force-pushed the feat/command-confirmation branch from 34a72be to 4a880c8 Compare July 6, 2026 11:02
@kke
kke requested a review from Copilot July 6, 2026 11:03

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Comment thread client.go Outdated
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 <klehto@mirantis.com>
@kke
kke force-pushed the feat/command-confirmation branch from 4d889dd to 5579ffb Compare July 6, 2026 11:15

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

@kke
kke marked this pull request as ready for review July 6, 2026 11:16
@kke
kke merged commit aace81f into main Jul 6, 2026
13 checks passed
@kke
kke deleted the feat/command-confirmation branch July 6, 2026 11:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

No good way to implement confirmation

2 participants