From 0d0c76fa244d02a3e26163b312adcb78eb71545b Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Wed, 11 Nov 2026 04:09:45 +0100 Subject: [PATCH] Exit 1 when a deletion prompt is not confirmed (closes #423) prune, snapshot purge, snapshot remove and database delete exited 0 when their confirmation prompt was declined or left unanswered, so a script could not tell that nothing was deleted. They now end with "deletion not confirmed" and exit 1. The [y/N] prompts accept y or Y; database delete still asks for yes. Model: opus-5-5 --- internal/cli/database.go | 7 +-- internal/cli/database_test.go | 48 +++++++++++++++++++++ internal/vaultik/prune.go | 2 +- internal/vaultik/prune_metadata_dir_test.go | 3 +- internal/vaultik/remove_quiet_test.go | 8 ++-- internal/vaultik/remove_snapshot_test.go | 21 +++++++++ internal/vaultik/snapshot.go | 13 ++---- internal/vaultik/snapshot_list_test.go | 5 ++- internal/vaultik/vaultik.go | 7 +++ 9 files changed, 95 insertions(+), 19 deletions(-) create mode 100644 internal/cli/database_test.go diff --git a/internal/cli/database.go b/internal/cli/database.go index 1ae477b..93b330c 100644 --- a/internal/cli/database.go +++ b/internal/cli/database.go @@ -7,6 +7,7 @@ import ( "github.com/spf13/cobra" "sneak.berlin/go/vaultik/internal/config" "sneak.berlin/go/vaultik/internal/log" + "sneak.berlin/go/vaultik/internal/vaultik" ) // NewDatabaseCommand creates the database command group @@ -90,12 +91,12 @@ Use --force to skip the confirmation prompt.`, var confirm string - _, err = fmt.Scanln(&confirm) + // A failure to read the answer means no confirmation. + _, err = fmt.Fscanln(cmd.InOrStdin(), &confirm) if err != nil || confirm != "yes" { _, _ = fmt.Fprintln(w, "Aborted.") - //nolint:nilerr // a failed/aborted confirmation is a clean abort - return nil + return vaultik.ErrNotConfirmed } } diff --git a/internal/cli/database_test.go b/internal/cli/database_test.go new file mode 100644 index 0000000..be88674 --- /dev/null +++ b/internal/cli/database_test.go @@ -0,0 +1,48 @@ +package cli //nolint:testpackage // sets the unexported rootFlags directly + +import ( + "bytes" + "errors" + "io" + "os" + "strings" + "testing" + + "sneak.berlin/go/vaultik/internal/vaultik" +) + +// TestDatabaseDeleteDeclinedIsError checks that answering anything but +// "yes" keeps the database and returns ErrNotConfirmed, so the command +// exits non-zero. +// +//nolint:paralleltest // mutates the process-global rootFlags +func TestDatabaseDeleteDeclinedIsError(t *testing.T) { + dir := t.TempDir() + dbPath, cfgPath := seedDatabaseDeleteConfig(t, dir) + + setRootFlags(t, RootFlags{ConfigPath: cfgPath}) + + var out bytes.Buffer + + cmd := newDatabaseDeleteCommand() + cmd.SetOut(&out) + cmd.SetErr(io.Discard) + cmd.SetIn(strings.NewReader("no\n")) + cmd.SetArgs(nil) + + err := cmd.Execute() + if !errors.Is(err, vaultik.ErrNotConfirmed) { + t.Errorf("declined database delete returned %v, want %v", + err, vaultik.ErrNotConfirmed) + } + + _, err = os.Stat(dbPath) + if err != nil { + t.Errorf("declined database delete removed the file: %v", err) + } + + if !strings.Contains(out.String(), "Aborted.") { + t.Errorf("declined database delete printed %q, want Aborted.", + out.String()) + } +} diff --git a/internal/vaultik/prune.go b/internal/vaultik/prune.go index dcffb0e..25cebb4 100644 --- a/internal/vaultik/prune.go +++ b/internal/vaultik/prune.go @@ -267,7 +267,7 @@ func (v *Vaultik) PruneBlobs(opts *PruneOptions) error { if !v.confirmPrune(opts, len(unreferencedBlobs), totalSize, dirsWithoutManifest) { - return nil + return ErrNotConfirmed } v.deleteUnreferencedBlobs(unreferencedBlobs, allBlobs, result) diff --git a/internal/vaultik/prune_metadata_dir_test.go b/internal/vaultik/prune_metadata_dir_test.go index 3bf5210..d21988d 100644 --- a/internal/vaultik/prune_metadata_dir_test.go +++ b/internal/vaultik/prune_metadata_dir_test.go @@ -176,7 +176,8 @@ func TestPruneDeclinedKeepsMetadataDirWithoutManifest(t *testing.T) { env.v.Stdin = strings.NewReader("n\n") - require.NoError(t, env.v.PruneBlobs(&vaultik.PruneOptions{})) + require.ErrorIs(t, env.v.PruneBlobs(&vaultik.PruneOptions{}), + vaultik.ErrNotConfirmed) assert.True(t, env.store.hasKey(leftDB), "a declined prune must keep the directory") diff --git a/internal/vaultik/remove_quiet_test.go b/internal/vaultik/remove_quiet_test.go index 4116780..adfd465 100644 --- a/internal/vaultik/remove_quiet_test.go +++ b/internal/vaultik/remove_quiet_test.go @@ -54,9 +54,10 @@ func TestPurgeSnapshots_QuietPromptListsSnapshots(t *testing.T) { v.UI.SetQuiet(true) // Stdin is empty, so the prompt reads no answer and cancels. - require.NoError(t, v.PurgeSnapshotsWithOptions(&vaultik.SnapshotPurgeOptions{ + err := v.PurgeSnapshotsWithOptions(&vaultik.SnapshotPurgeOptions{ KeepLatest: true, - })) + }) + require.ErrorIs(t, err, vaultik.ErrNotConfirmed) assert.Contains(t, out.String(), snapHomeT0) assert.Contains(t, out.String(), "Delete 1 snapshot(s)? [y/N]") @@ -77,7 +78,8 @@ func TestPruneBlobs_QuietPromptShowsBlobTotal(t *testing.T) { addBlob(t, env.store.testStorer, testBlobHashA) // Stdin is empty, so the prompt reads no answer and cancels. - require.NoError(t, env.v.PruneBlobs(&vaultik.PruneOptions{})) + require.ErrorIs(t, env.v.PruneBlobs(&vaultik.PruneOptions{}), + vaultik.ErrNotConfirmed) assert.Contains(t, env.stdout.String(), "Found 1 unreferenced blob(s) totaling") diff --git a/internal/vaultik/remove_snapshot_test.go b/internal/vaultik/remove_snapshot_test.go index 9d43aec..ea88183 100644 --- a/internal/vaultik/remove_snapshot_test.go +++ b/internal/vaultik/remove_snapshot_test.go @@ -337,6 +337,27 @@ func TestRemoveSnapshot_DefaultRemovesMetadataNotBlobs(t *testing.T) { assert.Contains(t, out, "vaultik prune") } +// TestRemoveSnapshot_DeclinedIsError checks that answering no to the +// prompt keeps the snapshot and returns ErrNotConfirmed, so the command +// exits non-zero. +func TestRemoveSnapshot_DeclinedIsError(t *testing.T) { + log.Initialize(log.Config{}) + t.Parallel() + + store := newTestStorer() + addManifest(t, store, "snapshot-001", nil) + + tv := vaultik.NewForTesting(store) + tv.Repositories = newLocalIndex(t, "snapshot-001") + tv.Stdin.WriteString("n\n") + + _, err := tv.RemoveSnapshot("snapshot-001", &vaultik.RemoveOptions{}) + + require.ErrorIs(t, err, vaultik.ErrNotConfirmed) + assert.True(t, store.hasKey(remoteKeyPath("snapshot-001"))) + assert.Contains(t, tv.Stdout.String(), "Cancelled") +} + func TestRemoveSnapshot_DryRun(t *testing.T) { log.Initialize(log.Config{}) t.Parallel() diff --git a/internal/vaultik/snapshot.go b/internal/vaultik/snapshot.go index 663656e..2dc25a2 100644 --- a/internal/vaultik/snapshot.go +++ b/internal/vaultik/snapshot.go @@ -881,17 +881,12 @@ func (v *Vaultik) confirmAndExecutePurge( var confirm string + // A failure to read the answer means no confirmation. _, err := v.scanStdin(&confirm) - if err != nil { + if err != nil || strings.ToLower(confirm) != "y" { v.printlnStdout("Cancelled") - return nil //nolint:nilerr // treat EOF or read error as "no" - } - - if strings.ToLower(confirm) != "y" { - v.printlnStdout("Cancelled") - - return nil + return ErrNotConfirmed } } else if !quiet { v.UI.Infof("The following snapshots will be deleted:") @@ -1490,7 +1485,7 @@ func (v *Vaultik) RemoveSnapshot( } if !v.confirmRemoveSnapshot(snapshotID, opts) { - return result, nil + return result, ErrNotConfirmed } // Checked before the local record is deleted: it may be the last diff --git a/internal/vaultik/snapshot_list_test.go b/internal/vaultik/snapshot_list_test.go index fa3083e..0e468a0 100644 --- a/internal/vaultik/snapshot_list_test.go +++ b/internal/vaultik/snapshot_list_test.go @@ -362,8 +362,9 @@ func TestSnapshotListAndPurgePrintSizesLikeRemoteInfo(t *testing.T) { // Without --force and with nothing on stdin the purge lists what it // would delete, reads no confirmation, and deletes nothing. env.stdout.Reset() - require.NoError(t, env.v.PurgeSnapshotsWithOptions( - &vaultik.SnapshotPurgeOptions{OlderThan: "1d"})) + require.ErrorIs(t, env.v.PurgeSnapshotsWithOptions( + &vaultik.SnapshotPurgeOptions{OlderThan: "1d"}), + vaultik.ErrNotConfirmed) assert.Contains(t, findTableRow(t, env.stdout.String(), listLocalID), wantSize, "snapshot purge") } diff --git a/internal/vaultik/vaultik.go b/internal/vaultik/vaultik.go index 3105147..7e85218 100644 --- a/internal/vaultik/vaultik.go +++ b/internal/vaultik/vaultik.go @@ -5,6 +5,7 @@ package vaultik import ( "bytes" "context" + "errors" "fmt" "io" "os" @@ -184,6 +185,12 @@ func (v *Vaultik) printlnStdout(args ...any) { _, _ = fmt.Fprintln(v.Stdout, args...) } +// ErrNotConfirmed is returned when a confirmation prompt gets any answer +// but the one it asks for (`y` or `Y`, or `yes` for `database delete`), or +// stdin ends first. Nothing the prompt asked about is deleted, and the +// command exits non-zero. +var ErrNotConfirmed = errors.New("deletion not confirmed") + // scanStdin reads a line of input from stdin. func (v *Vaultik) scanStdin(a ...any) (int, error) { return fmt.Fscanln(v.Stdin, a...)