Skip to content

Commit 13e266d

Browse files
sbldevnetclaude
sbldevnet
and
claude
committed
feat(confirm): require explicit --yes in non-interactive environments
Replace --skip with --yes across all write commands. ui.Confirm now returns an error when stdin is not a TTY instead of auto-approving, forcing callers to pass --yes in CI/scripts. Confirmation is always shown regardless of output format. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent bd0d740 commit 13e266d

8 files changed

Lines changed: 42 additions & 39 deletions

File tree

‎cmd/delete.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ func Delete(newRM func(context.Context) (rootmanager.RootManager, error)) *cobra
2828
cmd.AddCommand(deleteSubcommand(newRM, "certificates", "Delete root user Signin Certificates", "Delete existing root user Signing Certificates for specific AWS Organization member accounts."))
2929
cmd.AddCommand(DeleteS3BucketPolicy(newRM))
3030
cmd.AddCommand(DeleteSQSQueuePolicy(newRM))
31-
cmd.PersistentFlags().BoolVar(&skipFlag, "skip", false, "Skip the confirmation prompt")
31+
cmd.PersistentFlags().BoolVar(&skipFlag, "yes", false, "Skip the confirmation prompt")
3232
return cmd
3333
}
3434

@@ -72,7 +72,7 @@ func runDelete(newRM func(context.Context) (rootmanager.RootManager, error), w i
7272
}
7373
slog.Debug("selected accounts", "accounts", strings.Join(auditAccounts, ", "))
7474

75-
if outputFlag == "table" && !skipFlag {
75+
if !skipFlag {
7676
confirmed, err := ui.Confirm(fmt.Sprintf("Delete %s root credentials for %d account(s)?", credentialType, len(auditAccounts)))
7777
if err != nil {
7878
return err

‎cmd/delete_s3_bucket_policy.go‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -71,16 +71,16 @@ func runDeleteS3BucketPolicy(newRM func(context.Context) (rootmanager.RootManage
7171
if outputFlag == "table" {
7272
fmt.Fprintf(w, "Current bucket policy for %s:\n\n", bucketName)
7373
output.RenderPolicy(w, policy)
74+
}
7475

75-
if !skipFlag {
76-
confirmed, err := ui.Confirm("Delete this policy?")
77-
if err != nil {
78-
return err
79-
}
80-
if !confirmed {
81-
fmt.Fprintln(w, "Aborted.")
82-
return nil
83-
}
76+
if !skipFlag {
77+
confirmed, err := ui.Confirm("Delete this policy?")
78+
if err != nil {
79+
return err
80+
}
81+
if !confirmed {
82+
fmt.Fprintln(w, "Aborted.")
83+
return nil
8484
}
8585
}
8686

‎cmd/delete_s3_bucket_policy_test.go‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,6 @@ import (
1010
"github.com/unicrons/aws-root-manager/rootmanager"
1111
)
1212

13-
// When getBucketPolicyResult is empty, the command prints "No bucket policy found." and exits
14-
// without invoking the TUI confirmation — safe to use in tests.
15-
1613
func TestDeleteS3BucketPolicyCommand_NoPolicyFound(t *testing.T) {
1714
mock := &mockRootManager{
1815
getBucketPolicyResult: "",
@@ -53,9 +50,6 @@ func TestDeleteS3BucketPolicyCommand_FactoryError(t *testing.T) {
5350

5451
func TestDeleteS3BucketPolicyCommand_DeletionFailure(t *testing.T) {
5552
mock := &mockRootManager{
56-
// Return non-empty policy so get succeeds, but deletion fails.
57-
// Confirmation TUI is skipped because tests aren't interactive —
58-
// PromptSingle returns -1 (no selection), which maps to "No".
5953
getBucketPolicyResult: `{"Version":"2012-10-17"}`,
6054
deleteBucketResult: rootmanager.PolicyDeletionResult{
6155
AccountId: "123456789012", Success: false, Error: "access denied",
@@ -64,10 +58,9 @@ func TestDeleteS3BucketPolicyCommand_DeletionFailure(t *testing.T) {
6458

6559
cmd := Delete(newMockFactory(mock))
6660
cmd.SilenceErrors = true
67-
cmd.SetArgs([]string{"s3-bucket-policy", "--account", "123456789012", "--bucket", "my-bucket"})
61+
cmd.SetArgs([]string{"s3-bucket-policy", "--account", "123456789012", "--bucket", "my-bucket", "--yes"})
6862

69-
// Non-interactive: confirm TUI will fail/return no-selection → "Aborted."
70-
_ = cmd.Execute()
63+
require.Error(t, cmd.Execute())
7164
}
7265

7366
func TestDeleteS3BucketPolicyCommand_NoBucketsFoundInTUI(t *testing.T) {

‎cmd/delete_sqs_queue_policy.go‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -70,16 +70,16 @@ func runDeleteSQSQueuePolicy(newRM func(context.Context) (rootmanager.RootManage
7070
if outputFlag == "table" {
7171
fmt.Fprintf(w, "Current queue policy for %s:\n\n", queueUrl)
7272
output.RenderPolicy(w, policy)
73+
}
7374

74-
if !skipFlag {
75-
confirmed, err := ui.Confirm("Delete this policy?")
76-
if err != nil {
77-
return err
78-
}
79-
if !confirmed {
80-
fmt.Fprintln(w, "Aborted.")
81-
return nil
82-
}
75+
if !skipFlag {
76+
confirmed, err := ui.Confirm("Delete this policy?")
77+
if err != nil {
78+
return err
79+
}
80+
if !confirmed {
81+
fmt.Fprintln(w, "Aborted.")
82+
return nil
8383
}
8484
}
8585

‎cmd/delete_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ func TestDeleteCommand_Success(t *testing.T) {
2323
var buf bytes.Buffer
2424
cmd := Delete(newMockFactory(mock))
2525
cmd.SetOut(&buf)
26-
cmd.SetArgs([]string{"all", "--accounts", "123456789012", "--skip"})
26+
cmd.SetArgs([]string{"all", "--accounts", "123456789012", "--yes"})
2727

2828
require.NoError(t, cmd.Execute())
2929
assert.NotEmpty(t, buf.String())
@@ -42,7 +42,7 @@ func TestDeleteCommand_CertificatesSubcommand(t *testing.T) {
4242
var buf bytes.Buffer
4343
cmd := Delete(newMockFactory(mock))
4444
cmd.SetOut(&buf)
45-
cmd.SetArgs([]string{"certificates", "--accounts", "123456789012", "--skip"})
45+
cmd.SetArgs([]string{"certificates", "--accounts", "123456789012", "--yes"})
4646

4747
require.NoError(t, cmd.Execute())
4848
assert.NotEmpty(t, buf.String())
@@ -72,7 +72,7 @@ func TestDeleteCommand_DeleteFailure(t *testing.T) {
7272

7373
cmd := Delete(newMockFactory(mock))
7474
cmd.SilenceErrors = true
75-
cmd.SetArgs([]string{"all", "--accounts", "123456789012", "--skip"})
75+
cmd.SetArgs([]string{"all", "--accounts", "123456789012", "--yes"})
7676

7777
require.Error(t, cmd.Execute())
7878
}

‎cmd/recovery.go‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import (
1515
)
1616

1717
func Recovery(newRM func(context.Context) (rootmanager.RootManager, error)) *cobra.Command {
18-
var skip bool
1918
cmd := &cobra.Command{
2019
Use: "recovery",
2120
Short: "Allow root password recovery",
@@ -45,7 +44,7 @@ func Recovery(newRM func(context.Context) (rootmanager.RootManager, error)) *cob
4544
}
4645
slog.Debug("selected accounts", "accounts", strings.Join(targetAccounts, ", "))
4746

48-
if outputFlag == "table" && !skip {
47+
if !skipFlag {
4948
confirmed, err := ui.Confirm(fmt.Sprintf("Restore root password for %d account(s)?", len(targetAccounts)))
5049
if err != nil {
5150
return err
@@ -90,6 +89,6 @@ func Recovery(newRM func(context.Context) (rootmanager.RootManager, error)) *cob
9089
},
9190
}
9291
cmd.PersistentFlags().StringSliceVarP(&accountsFlags, "accounts", "a", []string{}, "List of tarjet AWS account IDs (comma-separated). Use \"all\" to select all accounts.")
93-
cmd.Flags().BoolVar(&skip, "skip", false, "Skip the confirmation prompt")
92+
cmd.Flags().BoolVar(&skipFlag, "yes", false, "Skip the confirmation prompt")
9493
return cmd
9594
}

‎cmd/recovery_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ func TestRecoveryCommand_Success(t *testing.T) {
2020
var buf bytes.Buffer
2121
cmd := Recovery(newMockFactory(mock))
2222
cmd.SetOut(&buf)
23-
cmd.SetArgs([]string{"--accounts", "123456789012", "--skip"})
23+
cmd.SetArgs([]string{"--accounts", "123456789012", "--yes"})
2424

2525
require.NoError(t, cmd.Execute())
2626
assert.NotEmpty(t, buf.String())
@@ -36,7 +36,7 @@ func TestRecoveryCommand_AlreadyExists(t *testing.T) {
3636
var buf bytes.Buffer
3737
cmd := Recovery(newMockFactory(mock))
3838
cmd.SetOut(&buf)
39-
cmd.SetArgs([]string{"--accounts", "123456789012", "--skip"})
39+
cmd.SetArgs([]string{"--accounts", "123456789012", "--yes"})
4040

4141
require.NoError(t, cmd.Execute())
4242
assert.Contains(t, buf.String(), "already exists")
@@ -48,7 +48,7 @@ func TestRecoveryCommand_FactoryError(t *testing.T) {
4848
cmd := Recovery(newFailingFactory(factoryErr))
4949
cmd.SilenceErrors = true
5050
cmd.SilenceUsage = true
51-
cmd.SetArgs([]string{"--accounts", "123456789012"})
51+
cmd.SetArgs([]string{"--accounts", "123456789012", "--yes"})
5252

5353
err := cmd.Execute()
5454
require.Error(t, err)
@@ -65,7 +65,7 @@ func TestRecoveryCommand_RecoveryFailure(t *testing.T) {
6565
cmd := Recovery(newMockFactory(mock))
6666
cmd.SilenceErrors = true
6767
cmd.SilenceUsage = true
68-
cmd.SetArgs([]string{"--accounts", "123456789012", "--skip"})
68+
cmd.SetArgs([]string{"--accounts", "123456789012", "--yes"})
6969

7070
require.Error(t, cmd.Execute())
7171
}

‎internal/cli/ui/confirm.go‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,18 @@
11
package ui
22

3+
import (
4+
"fmt"
5+
"os"
6+
7+
"github.com/charmbracelet/x/term"
8+
)
9+
310
// Confirm shows a yes/no single-select TUI. Returns true if the user chose "Yes".
11+
// Returns an error if stdin is not a TTY — callers must pass --yes to skip confirmation in non-interactive environments.
412
func Confirm(question string) (bool, error) {
13+
if !term.IsTerminal(os.Stdin.Fd()) {
14+
return false, fmt.Errorf("confirmation required: run in an interactive terminal or use --yes to confirm")
15+
}
516
idx, err := PromptSingle(question, []string{"Yes", "No"})
617
if err != nil {
718
return false, err

0 commit comments

Comments
 (0)