A cross-vault secret mv that cannot remove the source fails (closes #159)

When `secret mv` copied a secret into another vault but could not remove
the source, it printed a warning and the usual "Moved" line and exited 0.
It now fails. If the source secret's directory is still in place, the
error says the copy holds all versions and the source remains. If the
source left its vault and only the temporary directory it was moved into
could not be deleted, the error says the secret is moved and names that
directory, which the error of RemoveDirAtomic now does.

The tests are in package cli and call moveSecret, because the state
directory lock refuses the wrapped filesystems that make the removal fail.

Model: opus-5-5

README.md

@@ -118,11 +118,7 @@ are allowed, and a name must not be empty, `.` or `..`.
 
 #### `secret vault select <name>`
 
-Switches to the specified vault for subsequent operations. When `currentvault`
-is empty or names a vault that does not exist, every command that uses the
-current vault fails with an error that names the file and says to run
-`secret vault select <name>`. `secret vault list` still lists the vaults, and
-`secret info` still runs; both show no vault as current.
+Switches to the specified vault for subsequent operations.
 
 #### `secret vault remove <name> [--force]` / `secret vault rm` ⚠️ 🛑
 

internal/cli/current_vault_test.go

@@ -1,60 +0,0 @@
-package cli_test
-
-import (
-	"bytes"
-	"path/filepath"
-	"strconv"
-	"testing"
-
-	"github.com/spf13/afero"
-	"github.com/spf13/cobra"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-	"sneak.berlin/go/secret/internal/cli"
-	"sneak.berlin/go/secret/internal/secret"
-	"sneak.berlin/go/secret/internal/vault"
-)
-
-// TestListFailsWhenCurrentVaultDoesNotExist is a regression test for
-// https://git.eeqj.de/sneak/secret/issues/156, where a currentvault file
-// that was empty, held an invalid vault name, or named a vault that does
-// not exist, made `secret list` report an empty vault. Listing must fail
-// with an error that names the file and says to run `secret vault select`,
-// `secret vault list` must still list the vaults, and selecting a vault
-// must make listing work again. The name "." would make vaults.d itself the
-// vault, and "../vaults.d/work" would reach the vault work by a path.
-func TestListFailsWhenCurrentVaultDoesNotExist(t *testing.T) {
-	t.Parallel()
-
-	for _, name := range []string{"", ".", "../vaults.d/work", "gone"} {
-		t.Run(strconv.Quote(name), func(t *testing.T) {
-			t.Parallel()
-
-			fs := newTwoVaultFs(t)
-			currentVaultPath := filepath.Join(testStateDir, "currentvault")
-			require.NoError(t, afero.WriteFile(fs, currentVaultPath,
-				[]byte(name), secret.FilePerms))
-
-			c := cli.NewCLIInstanceWithStateDir(fs, testStateDir)
-
-			var out bytes.Buffer
-
-			cmd := &cobra.Command{}
-			cmd.SetOut(&out)
-
-			err := c.ListSecrets(cmd, false, true, "")
-			require.ErrorIs(t, err, vault.ErrVaultNotFound)
-			require.ErrorContains(t, err, currentVaultPath)
-			require.ErrorContains(t, err, "secret vault select")
-			assert.Empty(t, out.String())
-
-			require.NoError(t, c.ListVaults(cmd, false))
-			assert.Equal(t, "Available vaults:\n  default\n  work\n", out.String())
-
-			require.NoError(t, c.SelectVault(cmd, "default"))
-			out.Reset()
-			require.NoError(t, c.ListSecrets(cmd, false, true, ""))
-			assert.Equal(t, "x\n", out.String())
-		})
-	}
-}

internal/cli/secrets_size_test.go

@@ -36,11 +36,17 @@ func newSizeTestVault(t *testing.T) (afero.Fs, *vault.Vault) {
 
 	fs := afero.NewMemMapFs()
 
-	// Create vault, which selects it as the current vault
+	// Create vault
 	_, err := vault.CreateVault(fs, testStateDir, testVaultName,
 		testMnemonicBuffer(t), nil)
 	require.NoError(t, err)
 
+	// Set current vault
+	currentVaultPath := filepath.Join(testStateDir, "currentvault")
+	vaultPath := filepath.Join(testStateDir, "vaults.d", testVaultName)
+	err = afero.WriteFile(fs, currentVaultPath, []byte(vaultPath), 0o600)
+	require.NoError(t, err)
+
 	// Get vault and set up long-term key
 	vlt, err := vault.GetCurrentVault(fs, testStateDir)
 	require.NoError(t, err)

internal/vault/management.go

@@ -55,10 +55,7 @@ func ValidateVaultName(name string) error {
 }
 
 // ResolveVaultSymlink reads the currentvault file to get the path to the current vault
-// The file contains just the vault name (e.g., "default"). An empty or
-// invalid name, or the name of a vault that does not exist, is an error
-// wrapping ErrVaultNotFound that names the file and says to run
-// 'secret vault select <name>'.
+// The file contains just the vault name (e.g., "default")
 func ResolveVaultSymlink(fs afero.Fs, currentVaultPath string) (string, error) {
 	secret.Debug("resolveVaultSymlink starting", "path", currentVaultPath)
 
@@ -77,21 +74,6 @@ func ResolveVaultSymlink(fs afero.Fs, currentVaultPath string) (string, error) {
 	stateDir := filepath.Dir(currentVaultPath)
 	absolutePath := filepath.Join(stateDir, "vaults.d", vaultName)
 
-	// An empty name would make vaults.d itself the vault's directory
-	exists := false
-	if isValidVaultName(vaultName) {
-		exists, err = afero.DirExists(fs, absolutePath)
-		if err != nil {
-			return "", fmt.Errorf("failed to check if vault exists: %w", err)
-		}
-	}
-
-	if !exists {
-		return "", fmt.Errorf("vault '%s' named in %s %w; run "+
-			"'secret vault select <name>' to select a vault",
-			vaultName, currentVaultPath, ErrVaultNotFound)
-	}
-
 	secret.Debug("Resolved to absolute path", "absolute_path", absolutePath)
 
 	return absolutePath, nil