From 0026ca068ce9d8082af320558358520ab034337c Mon Sep 17 00:00:00 2001 From: Luca Di Maio Date: Sat, 20 Jun 2026 13:46:07 +0200 Subject: [PATCH] fix(rm): honor --rm-home and gate the custom-home prompt on it Thread RemoveHome into removeContainer; only prompt to delete the custom home when --rm-home is set, interactive, and it differs from the host home matching the shell. Without the flag the home is never touched. Signed-off-by: Luca Di Maio --- pkg/commands/rm.go | 8 ++-- pkg/commands/rm_test.go | 87 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 4 deletions(-) diff --git a/pkg/commands/rm.go b/pkg/commands/rm.go index c96aabca..f695f857 100644 --- a/pkg/commands/rm.go +++ b/pkg/commands/rm.go @@ -69,7 +69,7 @@ func (c *RmCommand) Execute(ctx context.Context, options RmOptions) (*RmResult, var removedDistroboxes []containermanager.Container for _, currentDistrobox := range distroboxesToRemove { - err := c.removeContainer(ctx, currentDistrobox, options.Force, options.NoTTY, options.Verbose, userHome) + err := c.removeContainer(ctx, currentDistrobox, options.Force, options.NoTTY, options.RemoveHome, options.Verbose, userHome) if err != nil { //nolint:forbidigo // waiting for the logger implementation fmt.Printf("error deleting %s: %s", currentDistrobox.Name, err) @@ -85,6 +85,7 @@ func (c *RmCommand) removeContainer( container containermanager.Container, force bool, noTTY bool, + removeHomeRequested bool, verbose bool, userHome string, ) error { @@ -103,14 +104,13 @@ func (c *RmCommand) removeContainer( } removeHome := false - if !noTTY && inspectOutput.ContainerHome != userHome { + if removeHomeRequested && !noTTY && inspectOutput.ContainerHome != userHome { question := fmt.Sprintf( "Do you really want to remove custom home of container %s (%s)?", container.Name, inspectOutput.ContainerHome, ) - answer := c.prompter.Prompt(question, false) - removeHome = answer + removeHome = c.prompter.Prompt(question, false) } cmOptions := containermanager.RmOptions{ diff --git a/pkg/commands/rm_test.go b/pkg/commands/rm_test.go index 771c995b..468345a3 100644 --- a/pkg/commands/rm_test.go +++ b/pkg/commands/rm_test.go @@ -24,6 +24,93 @@ func newTestRmCommand(mock *testutil.MockContainerManager) *commands.RmCommand { return commands.NewRmCommand(&config.Values{}, mock, prompter) } +func newTestRmCommandWithInput(mock *testutil.MockContainerManager, input string) *commands.RmCommand { + prompter := ui.NewPrompter(*bufio.NewReader(strings.NewReader(input)), io.Discard) + return commands.NewRmCommand(&config.Values{}, mock, prompter) +} + +// lastRemoveOptions returns the RmOptions passed to the most recent +// containerManager.Remove call recorded by the mock. +func lastRemoveOptions(t *testing.T, mock *testutil.MockContainerManager) containermanager.RmOptions { + t.Helper() + require.NotEmpty(t, mock.Spy.Remove, "expected containerManager.Remove to be called") + last := mock.Spy.Remove[len(mock.Spy.Remove)-1] + opts, ok := last[1].(containermanager.RmOptions) + require.True(t, ok, "second Remove arg should be containermanager.RmOptions") + return opts +} + +// `rm` without --rm-home must never remove the custom home, even +// interactively with a custom home and a "yes" waiting on stdin (the shell only +// ever prompts/removes when --rm-home is set, distrobox-rm:364-371). +func TestRmCommand_RmHome_NotRequested_NeverPrompts(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + + containerName := "test-rm-no-flag" + mock := &testutil.MockContainerManager{ + ListContainersResult: []containermanager.Container{ + {Name: containerName, Status: "Exited", Labels: map[string]string{"manager": "distrobox"}}, + }, + InspectContainerResult: &containermanager.InspectResult{ContainerHome: "/custom/home/dir"}, + } + // Seed a "y" — if the prompt were (wrongly) shown, the home would be removed. + cmd := newTestRmCommandWithInput(mock, "y\n") + + _, err := cmd.Execute(context.Background(), commands.RmOptions{ + ContainerNames: []string{containerName}, + RemoveHome: false, + }) + require.NoError(t, err) + assert.False(t, lastRemoveOptions(t, mock).RemoveHome, + "home must not be removed without --rm-home") +} + +// `rm --rm-home` interactively prompts; an explicit "yes" removes the home. +func TestRmCommand_RmHome_Requested_ConfirmedRemoves(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + + containerName := "test-rm-flag-yes" + mock := &testutil.MockContainerManager{ + ListContainersResult: []containermanager.Container{ + {Name: containerName, Status: "Exited", Labels: map[string]string{"manager": "distrobox"}}, + }, + InspectContainerResult: &containermanager.InspectResult{ContainerHome: "/custom/home/dir"}, + } + cmd := newTestRmCommandWithInput(mock, "y\n") + + _, err := cmd.Execute(context.Background(), commands.RmOptions{ + ContainerNames: []string{containerName}, + RemoveHome: true, + }) + require.NoError(t, err) + assert.True(t, lastRemoveOptions(t, mock).RemoveHome, + "home should be removed with --rm-home and a confirmed prompt") +} + +// `rm --rm-home --yes` (non-interactive) never removes the home, matching +// the shell which skips the prompt and defaults to "no" (distrobox-rm:365-366). +func TestRmCommand_RmHome_Requested_NonInteractiveKeepsHome(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + + containerName := "test-rm-flag-yes-noninteractive" + mock := &testutil.MockContainerManager{ + ListContainersResult: []containermanager.Container{ + {Name: containerName, Status: "Exited", Labels: map[string]string{"manager": "distrobox"}}, + }, + InspectContainerResult: &containermanager.InspectResult{ContainerHome: "/custom/home/dir"}, + } + cmd := newTestRmCommand(mock) + + _, err := cmd.Execute(context.Background(), commands.RmOptions{ + ContainerNames: []string{containerName}, + RemoveHome: true, + NoTTY: true, + }) + require.NoError(t, err) + assert.False(t, lastRemoveOptions(t, mock).RemoveHome, + "non-interactive rm must not remove the home even with --rm-home") +} + // writeExportedDesktopApp writes a minimal desktop file that // findExportedDesktopApps will match for the given containerName (i.e. // the per-app desktop entries created by distrobox-export, not the