diff --git a/internal/cli/move_cross_vault_test.go b/internal/cli/move_cross_vault_test.go new file mode 100644 index 0000000..798d667 --- /dev/null +++ b/internal/cli/move_cross_vault_test.go @@ -0,0 +1,85 @@ +//nolint:testpackage // white-box test of unexported internals +package cli + +import ( + "errors" + "testing" + + "github.com/awnumar/memguard" + "github.com/spf13/afero" + "github.com/spf13/cobra" + "github.com/stretchr/testify/require" + "sneak.berlin/go/secret/internal/vault" +) + +// errRenameFailed is returned by renameFailFs in place of a successful +// rename. +var errRenameFailed = errors.New("permission denied") + +// renameFailFs fails every Rename of one path, as a read-only parent +// directory does. +type renameFailFs struct { + afero.Fs + + path string +} + +func (f *renameFailFs) Rename(oldname, newname string) error { + if oldname == f.path { + return errRenameFailed + } + + return f.Fs.Rename(oldname, newname) +} + +// TestCrossVaultMoveFailsWhenSourceRemains is a regression test for +// https://git.eeqj.de/sneak/secret/issues/159, where a move between two +// vaults whose source could not be removed reported the secret moved and +// succeeded. It must fail, and the copy and the source must both be there. +// The state directory lock refuses the wrapped filesystem, so the test +// calls moveSecret, which MoveSecret runs once it holds the lock. +func TestCrossVaultMoveFailsWhenSourceRemains(t *testing.T) { + t.Parallel() + + base := afero.NewMemMapFs() + + _, err := vault.CreateVault(base, testStateDir, "work", + testMnemonicBuffer(t), nil) + require.NoError(t, err) + + source, err := vault.CreateVault(base, testStateDir, "default", + testMnemonicBuffer(t), nil) + require.NoError(t, err) + + err = source.AddSecret("x", + memguard.NewBufferFromBytes([]byte("value")), false) + require.NoError(t, err) + + instance := NewCLIInstanceWithStateDir(&renameFailFs{ + Fs: base, + path: testStateDir + "/vaults.d/default/secrets.d/x", + }, testStateDir) + instance.Mnemonic = testMnemonicBuffer(t) + + err = instance.moveSecret(&cobra.Command{}, "default:x", "work:y", false) + require.ErrorIs(t, err, errRenameFailed) + require.ErrorContains(t, err, "secret 'default:x' is copied with all "+ + "1 version(s) to 'work:y', but the source remains") + + // requireValue requires that the vault vaultName holds secretName with + // the value stored above. + requireValue := func(vaultName, secretName string) { + vlt := vault.NewVault(base, testStateDir, vaultName) + vlt.Mnemonic = testMnemonicBuffer(t) + + value, err := vlt.GetSecret(secretName) + require.NoError(t, err, vaultName+":"+secretName) + + defer value.Destroy() + + require.Equal(t, []byte("value"), value.Bytes()) + } + + requireValue("default", "x") + requireValue("work", "y") +} diff --git a/internal/cli/secrets.go b/internal/cli/secrets.go index 59c64e9..827c232 100644 --- a/internal/cli/secrets.go +++ b/internal/cli/secrets.go @@ -1097,12 +1097,10 @@ func (cli *Instance) moveSecretCrossVault( // Delete source secret err = secret.RemoveDirAtomic(cli.fs, srcSecretDir) if err != nil { - // Copy succeeded but delete failed - warn but don't fail - cmd.Printf("Warning: copied secret but failed to remove source: %v\n", err) - cmd.Printf("Moved secret '%s:%s' to '%s:%s' (%d version(s))\n", - srcVault.Name, srcSecretName, destVault.Name, destSecretName, versionCount) - - return nil + return fmt.Errorf("secret '%s:%s' is copied with all %d version(s) "+ + "to '%s:%s', but the source remains: %w", + srcVault.Name, srcSecretName, versionCount, + destVault.Name, destSecretName, err) } cmd.Printf("Moved secret '%s:%s' to '%s:%s' (%d version(s))\n",