Fix error handling in AddSecret to clean up on failure

- Clean up secret directory if Save() fails for new secrets
- Add tests to verify cleanup behavior
- Ensures failed secret additions don't leave orphaned directories
This commit is contained in:
Jeffrey Paul 2025-07-26 22:03:31 +02:00
parent 75c3d22b62
commit 92c41bdb0c
2 changed files with 93 additions and 0 deletions

View File

@ -204,6 +204,12 @@ func (v *Vault) AddSecret(name string, value *memguard.LockedBuffer, force bool)
if err := newVersion.Save(value); err != nil {
secret.Debug("Failed to save new version", "error", err, "version", versionName)
// Clean up the secret directory if this was a new secret
if !exists {
secret.Debug("Cleaning up secret directory due to save failure", "secret_dir", secretDir)
_ = v.fs.RemoveAll(secretDir)
}
return fmt.Errorf("failed to save version: %w", err)
}

View File

@ -0,0 +1,87 @@
package vault_test
import (
"path/filepath"
"testing"
"git.eeqj.de/sneak/secret/internal/secret"
"git.eeqj.de/sneak/secret/internal/vault"
"github.com/awnumar/memguard"
"github.com/spf13/afero"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestAddSecretFailsWithMissingPublicKey(t *testing.T) {
// Create in-memory filesystem
fs := afero.NewMemMapFs()
stateDir := "/test/state"
// Create a vault directory without a public key (simulating the error condition)
vaultDir := filepath.Join(stateDir, "vaults.d", "broken")
require.NoError(t, fs.MkdirAll(vaultDir, secret.DirPerms))
// Create currentvault symlink
currentVaultPath := filepath.Join(stateDir, "currentvault")
require.NoError(t, afero.WriteFile(fs, currentVaultPath, []byte(vaultDir), secret.FilePerms))
// Create vault instance
vlt := vault.NewVault(fs, stateDir, "broken")
// Try to add a secret - this should fail
secretName := "test-secret"
value := memguard.NewBufferFromBytes([]byte("test-value"))
defer value.Destroy()
err := vlt.AddSecret(secretName, value, false)
require.Error(t, err, "AddSecret should fail when public key is missing")
assert.Contains(t, err.Error(), "failed to read long-term public key")
// Verify that the secret directory was NOT created
secretDir := filepath.Join(vaultDir, "secrets.d", secretName)
exists, _ := afero.DirExists(fs, secretDir)
assert.False(t, exists, "Secret directory should not exist after failed AddSecret")
// Verify the secrets.d directory is empty or doesn't exist
secretsDir := filepath.Join(vaultDir, "secrets.d")
if exists, _ := afero.DirExists(fs, secretsDir); exists {
entries, err := afero.ReadDir(fs, secretsDir)
require.NoError(t, err)
assert.Empty(t, entries, "secrets.d directory should be empty after failed AddSecret")
}
}
func TestAddSecretCleansUpOnFailure(t *testing.T) {
// Create in-memory filesystem
fs := afero.NewMemMapFs()
stateDir := "/test/state"
// Create a vault directory with public key
vaultDir := filepath.Join(stateDir, "vaults.d", "test")
require.NoError(t, fs.MkdirAll(vaultDir, secret.DirPerms))
// Create a mock public key that will cause encryption to fail
// by using an invalid age public key format
pubKeyPath := filepath.Join(vaultDir, "pub.age")
require.NoError(t, afero.WriteFile(fs, pubKeyPath, []byte("invalid-public-key"), secret.FilePerms))
// Create currentvault symlink
currentVaultPath := filepath.Join(stateDir, "currentvault")
require.NoError(t, afero.WriteFile(fs, currentVaultPath, []byte(vaultDir), secret.FilePerms))
// Create vault instance
vlt := vault.NewVault(fs, stateDir, "test")
// Try to add a secret - this should fail during encryption
secretName := "test-secret"
value := memguard.NewBufferFromBytes([]byte("test-value"))
defer value.Destroy()
err := vlt.AddSecret(secretName, value, false)
require.Error(t, err, "AddSecret should fail with invalid public key")
// Verify that the secret directory was NOT created
secretDir := filepath.Join(vaultDir, "secrets.d", secretName)
exists, _ := afero.DirExists(fs, secretDir)
assert.False(t, exists, "Secret directory should not exist after failed AddSecret")
}