From 79bc02141297ef429a14da49d06d778255c0422c Mon Sep 17 00:00:00 2001 From: sneak Date: Sun, 4 Oct 2026 19:58:51 +0000 Subject: [PATCH] Check errors by identity, not by message text, in tests (closes #49) Tests that asserted a failure by a fragment of its message now use errors.Is: a refactor returning the wrong error, or wrapping with %v instead of %w, now fails them. New tests return each exported error of internal/vault and pkg/bip85 that no test returned, and check wrapped causes (os.ErrNotExist, ErrMnemonicMismatch through GetSecret, ErrInvalidPathComponent through DeriveBIP85Entropy). The 999-versions test moves into package secret to name its unexported error. Checks of errors no test can name keep their text; they are listed on the issue. Model: opus-5-5 --- TODO.md | 7 ++ internal/cli/create_vault_test.go | 20 ++- internal/cli/integration_test.go | 71 +++++------ internal/cli/move_test.go | 37 +++--- internal/cli/path_traversal_test.go | 72 ++++++----- internal/cli/secrets_size_test.go | 120 ++++++++---------- internal/cli/version_test.go | 6 +- internal/secret/keychainunlocker_test.go | 10 +- internal/secret/seunlocker_test.go | 10 +- internal/secret/version_internal_test.go | 31 +++++ internal/secret/version_test.go | 22 ---- internal/vault/errors_test.go | 138 +++++++++++++++++++++ internal/vault/integration_test.go | 7 +- internal/vault/integration_version_test.go | 12 +- internal/vault/path_traversal_test.go | 11 +- internal/vault/secrets_version_test.go | 6 +- internal/vault/vault_error_test.go | 10 +- pkg/agehd/agehd_test.go | 23 +--- pkg/bip85/bip85_test.go | 44 +++++-- 19 files changed, 401 insertions(+), 256 deletions(-) create mode 100644 internal/secret/version_internal_test.go create mode 100644 internal/vault/errors_test.go diff --git a/TODO.md b/TODO.md index 2f205dd..cdbff2a 100644 --- a/TODO.md +++ b/TODO.md @@ -18,6 +18,13 @@ https://git.eeqj.de/sneak/secret/milestone/12 # Completed Steps +- 2026-10-04: Tests check which error a failure returns with `errors.Is`, + not by matching words of its message + (https://git.eeqj.de/sneak/secret/issues/49). Every exported error that + can be returned has a test that the function returns it, and errors + wrapping a cause are checked through the wrapping. Checks that still match + text, because the error has no exported value the test can name, are + listed on the issue. - 2026-10-04: When a vault cannot be opened through its current unlocker, because a file the unlocker needs is missing or damaged, its keychain item or Secure Enclave key is gone, or the passphrase is wrong, the error now diff --git a/internal/cli/create_vault_test.go b/internal/cli/create_vault_test.go index d1c1e2a..d590a3c 100644 --- a/internal/cli/create_vault_test.go +++ b/internal/cli/create_vault_test.go @@ -66,22 +66,18 @@ func TestCreateExistingVaultChangesNothing(t *testing.T) { tests := []struct { command string - want string run func(c *cli.Instance) error }{ { "init", - "failed to create default vault: vault default already exists", func(c *cli.Instance) error { return c.Init(cmd) }, }, { "vault create default", - "vault default already exists", func(c *cli.Instance) error { return c.CreateVault(cmd, "default") }, }, { "vault create work", - "vault work already exists", func(c *cli.Instance) error { return c.CreateVault(cmd, "work") }, }, } @@ -92,7 +88,7 @@ func TestCreateExistingVaultChangesNothing(t *testing.T) { err := tt.run(newCLI(fs)) - require.EqualError(t, err, tt.want) + require.ErrorIs(t, err, vault.ErrVaultExists) require.Equal(t, before, snapshotStateDir(t, fs)) }) } @@ -227,9 +223,7 @@ func TestStopDuringCreateLeavesWholeVaultOrNone(t *testing.T) { fs := afero.NewMemMapFs() require.NoError(t, fs.MkdirAll(testStateDir, secret.DirPerms)) - requireStopsLeaveWholeVaultOrNone(t, fs, "default", - "failed to create default vault: vault default already exists", - mnemonic, passphrase, + requireStopsLeaveWholeVaultOrNone(t, fs, "default", mnemonic, passphrase, func(c *cli.Instance) error { return c.Init(cmd) }) }) @@ -239,8 +233,7 @@ func TestStopDuringCreateLeavesWholeVaultOrNone(t *testing.T) { _, err := vault.CreateVault(fs, testStateDir, "default", mnemonic, nil) require.NoError(t, err) - requireStopsLeaveWholeVaultOrNone(t, fs, "work", "vault work already exists", - mnemonic, passphrase, + requireStopsLeaveWholeVaultOrNone(t, fs, "work", mnemonic, passphrase, func(c *cli.Instance) error { return c.CreateVault(cmd, "work") }) }) } @@ -248,9 +241,10 @@ func TestStopDuringCreateLeavesWholeVaultOrNone(t *testing.T) { // requireStopsLeaveWholeVaultOrNone checks, as // TestStopDuringCreateLeavesWholeVaultOrNone describes, the stops of the // command run, creating the vault name on fs with mnemonic and passphrase. -// Run again where the vault is there, the command must fail with exists. +// Run again where the vault is there, the command must fail with +// vault.ErrVaultExists. func requireStopsLeaveWholeVaultOrNone( - t *testing.T, fs afero.Fs, name, exists string, + t *testing.T, fs afero.Fs, name string, mnemonic, passphrase *memguard.LockedBuffer, run func(c *cli.Instance) error, ) { @@ -312,7 +306,7 @@ func requireStopsLeaveWholeVaultOrNone( c.UnlockPassphrase = passphrase if _, there := state[vaultDir+"/"]; there { - require.EqualError(t, run(c), exists) + require.ErrorIs(t, run(c), vault.ErrVaultExists) } else { require.NoError(t, run(c)) } diff --git a/internal/cli/integration_test.go b/internal/cli/integration_test.go index d6a54e9..17d580c 100644 --- a/internal/cli/integration_test.go +++ b/internal/cli/integration_test.go @@ -680,10 +680,10 @@ func test06GetSecret(t *testing.T, testMnemonic string, runSecret func(...string require.NoError(t, err, "get secret should succeed") assert.Equal(t, "password123", strings.TrimSpace(output), "should return correct secret value") - // Test that without mnemonic, we get an error - output, err = runSecret("get", "database/password") - require.Error(t, err, "get should fail without unlock method") - assert.Contains(t, output, "failed to unlock vault", "should indicate unlock failure") + // Test that without mnemonic, we get an error: the passphrase unlocker + // cannot ask for its passphrase, as the tests have no terminal + _, err = runSecret("get", "database/password") + require.ErrorIs(t, err, secret.ErrPassphraseNotRead, "get should fail without unlock method") } func test07AddSecretVersion(t *testing.T, tempDir, testMnemonic string, runSecret func(...string) (string, error), runSecretWithEnv func(map[string]string, ...string) (string, error), runSecretWithStdin func(string, map[string]string, ...string) (string, error)) { @@ -839,12 +839,11 @@ func test09GetSpecificVersion(t *testing.T, tempDir, testMnemonic string, runSec assert.Equal(t, "newpassword456", strings.TrimSpace(output), "should return new secret value without --version") // An empty --version is not a version; it does not mean the current one - output, err = runSecretWithEnv(map[string]string{ + _, err = runSecretWithEnv(map[string]string{ secret.EnvMnemonic: testMnemonic, }, "get", "--version", "", "database/password") - require.Error(t, err, "get with an empty version should fail") - assert.Contains(t, output, "version '' not found", "should reject the empty version") + require.ErrorIs(t, err, vault.ErrVersionNotFound, "should reject the empty version") } func test10PromoteVersion(t *testing.T, tempDir, testMnemonic string, runSecret func(...string) (string, error), runSecretWithEnv func(map[string]string, ...string) (string, error)) { @@ -1158,11 +1157,7 @@ func testInvalidSecretNames(t *testing.T, testMnemonic string, runSecretWithStdi shouldFail := slices.Contains(definitelyInvalid, invalidName) if shouldFail { - require.Error(t, err, "add '%s' should fail", invalidName) - - if err != nil { - assert.Contains(t, output, "invalid secret name", "should indicate invalid name for '%s'", invalidName) - } + require.ErrorIs(t, err, vault.ErrInvalidSecretName, "add '%s' should fail", invalidName) } else { // For the slash cases and .hidden, they might succeed // Just log what happened @@ -1310,9 +1305,8 @@ func test12cCrossVaultMove(t *testing.T, testMnemonic string, runSecretWithEnv f require.NoError(t, err, "add force/test in work should succeed") // Move without force should fail - output, err = runSecretWithEnv(env, "move", "work:force/test", "default") - require.Error(t, err, "move without force should fail when dest exists") - assert.Contains(t, output, "already exists", "should indicate destination exists") + _, err = runSecretWithEnv(env, "move", "work:force/test", "default") + require.ErrorIs(t, err, vault.ErrSecretExists, "move without force should fail when dest exists") // Move with force should succeed output, err = runSecretWithEnv(env, "move", "--force", "work:force/test", "default") @@ -1427,9 +1421,8 @@ func test14SwitchVault(t *testing.T, tempDir string, runSecret func(...string) ( require.NoError(t, err, "vault select default should succeed") // Test selecting non-existent vault - output, err := runSecret("vault", "select", "nonexistent") - require.Error(t, err, "selecting non-existent vault should fail") - assert.Contains(t, output, "does not exist", "should indicate vault doesn't exist") + _, err = runSecret("vault", "select", "nonexistent") + require.ErrorIs(t, err, vault.ErrVaultNotFound, "selecting non-existent vault should fail") } func test15VaultIsolation(t *testing.T, testMnemonic string, runSecret func(...string) (string, error), runSecretWithEnv func(map[string]string, ...string) (string, error), runSecretWithStdin func(string, map[string]string, ...string) (string, error)) { @@ -1450,11 +1443,10 @@ func test15VaultIsolation(t *testing.T, testMnemonic string, runSecret func(...s require.NoError(t, err, "vault select work should succeed") // Try to get the default-only secret (should fail) - output, err := runSecretWithEnv(map[string]string{ + _, err = runSecretWithEnv(map[string]string{ secret.EnvMnemonic: testMnemonic, }, "get", "default-only/secret") - require.Error(t, err, "should not be able to get default vault secret from work vault") - assert.Contains(t, output, "not found", "should indicate secret not found") + require.ErrorIs(t, err, vault.ErrSecretNotFound, "should not be able to get default vault secret from work vault") // Add a unique secret to work vault _, err = runSecretWithStdin("work-vault-secret", map[string]string{ @@ -1467,14 +1459,13 @@ func test15VaultIsolation(t *testing.T, testMnemonic string, runSecret func(...s require.NoError(t, err, "vault select default should succeed") // Try to get the work-only secret (should fail) - output, err = runSecretWithEnv(map[string]string{ + _, err = runSecretWithEnv(map[string]string{ secret.EnvMnemonic: testMnemonic, }, "get", "work-only/secret") - require.Error(t, err, "should not be able to get work vault secret from default vault") - assert.Contains(t, output, "not found", "should indicate secret not found") + require.ErrorIs(t, err, vault.ErrSecretNotFound, "should not be able to get work vault secret from default vault") // Verify we can still get the default-only secret - output, err = runSecretWithEnv(map[string]string{ + output, err := runSecretWithEnv(map[string]string{ secret.EnvMnemonic: testMnemonic, }, "get", "default-only/secret") require.NoError(t, err, "get default-only secret should succeed") @@ -1586,11 +1577,10 @@ func test17ImportFromFile(t *testing.T, tempDir, testMnemonic string, runSecretW // Just verify the import succeeded // Test importing non-existent file - output, err = runSecretWithEnv(map[string]string{ + _, err = runSecretWithEnv(map[string]string{ secret.EnvMnemonic: testMnemonic, }, "import", "imported/nonexistent", "--source", "/nonexistent/file") - require.Error(t, err, "importing non-existent file should fail") - assert.Contains(t, output, "failed", "should indicate failure") + require.ErrorIs(t, err, os.ErrNotExist, "importing non-existent file should fail") // Verify filesystem structure defaultVaultDir := filepath.Join(tempDir, "vaults.d", "default") @@ -1905,11 +1895,10 @@ func test23ErrorHandling(t *testing.T, tempDir, secretPath, testMnemonic string, t.Helper() // Get non-existent secret - output, err := runSecretWithEnv(map[string]string{ + _, err := runSecretWithEnv(map[string]string{ secret.EnvMnemonic: testMnemonic, }, "get", "nonexistent/secret") - require.Error(t, err, "get non-existent secret should fail") - assert.Contains(t, output, "not found", "should indicate secret not found") + require.ErrorIs(t, err, vault.ErrSecretNotFound, "get non-existent secret should fail") // Add secret without mnemonic or unlocker unsetMnemonic := os.Getenv(secret.EnvMnemonic) @@ -1939,13 +1928,12 @@ func test23ErrorHandling(t *testing.T, tempDir, secretPath, testMnemonic string, // Invalid secret names (already tested in test 12) // Non-existent vault operations - output, err = runSecret("vault", "select", "nonexistent") - require.Error(t, err, "select non-existent vault should fail") - assert.Contains(t, output, "does not exist", "should indicate vault doesn't exist") + _, err = runSecret("vault", "select", "nonexistent") + require.ErrorIs(t, err, vault.ErrVaultNotFound, "select non-existent vault should fail") // Import to non-existent vault with test passphrase testPassphrase := "test-passphrase-123" // Define testPassphrase locally - output, err = runSecretWithEnv(map[string]string{ + output, err := runSecretWithEnv(map[string]string{ secret.EnvMnemonic: testMnemonic, secret.EnvUnlockPassphrase: testPassphrase, }, "vault", "import", "nonexistent") @@ -1953,11 +1941,10 @@ func test23ErrorHandling(t *testing.T, tempDir, secretPath, testMnemonic string, assert.Contains(t, output, "does not exist", "should indicate vault doesn't exist") // Get specific version that doesn't exist - output, err = runSecretWithEnv(map[string]string{ + _, err = runSecretWithEnv(map[string]string{ secret.EnvMnemonic: testMnemonic, }, "get", "--version", "99999999.999", "database/password") - require.Error(t, err, "get non-existent version should fail") - assert.Contains(t, output, "not found", "should indicate version not found") + require.ErrorIs(t, err, vault.ErrVersionNotFound, "get non-existent version should fail") // Promote non-existent version output, err = runSecretWithEnv(map[string]string{ @@ -2367,11 +2354,10 @@ func test30BackupRestore(t *testing.T, tempDir, secretPath, testMnemonic string, assert.NotEmpty(t, output, "restored secret should have value") // Verify post-backup secret is gone - output, err = runSecretWithEnv(map[string]string{ + _, err = runSecretWithEnv(map[string]string{ secret.EnvMnemonic: testMnemonic, }, "get", "post-backup/secret") - require.Error(t, err, "post-backup secret should not exist after restore") - assert.Contains(t, output, "not found", "should indicate secret not found") + require.ErrorIs(t, err, vault.ErrSecretNotFound, "post-backup secret should not exist after restore") t.Log("Backup and restore completed successfully") } @@ -2436,8 +2422,7 @@ func test31EnvMnemonicUsesVaultDerivationIndex(t *testing.T, tempDir, secretPath t.Logf("Output: %s", getOutput) // This is the expected behavior with the current bug - require.Error(t, err, "get should fail due to wrong derivation index") - assert.Contains(t, getOutput, "derived public key does not match vault", "should indicate key derivation failure") + require.ErrorIs(t, err, vault.ErrMnemonicMismatch, "get should fail due to wrong derivation index") // Document what should happen when the bug is fixed t.Log("When the bug is fixed, GetValue should read vault metadata and use derivation index 1") diff --git a/internal/cli/move_test.go b/internal/cli/move_test.go index 1ddb70e..1075946 100644 --- a/internal/cli/move_test.go +++ b/internal/cli/move_test.go @@ -30,6 +30,8 @@ func TestRejectedMoveWithinVaultLeavesStateUnchanged(t *testing.T) { workX = "work:x" ) + // internal/cli declares these errors itself and does not export them, so + // only their text can be compared. tests := []struct { command string source, dest string @@ -52,21 +54,6 @@ func TestRejectedMoveWithinVaultLeavesStateUnchanged(t *testing.T) { "mv --force nosuch:x nosuch:y", "nosuch:x", "nosuch:y", true, "vault 'nosuch' does not exist", }, - // Each of these spells "work" a second way. The spelling is not a - // valid vault name, so the move is not taken for a move between two - // vaults, which would delete the destination, here the source. - { - "mv --force work:x work/:x", workX, "work/:x", true, - vault.ValidateVaultName("work/").Error(), - }, - { - "mv --force work/:x work:", "work/:x", "work:", true, - vault.ValidateVaultName("work/").Error(), - }, - { - "mv --force work:x ./work:x", workX, "./work:x", true, - vault.ValidateVaultName("./work").Error(), - }, } for _, tt := range tests { @@ -82,6 +69,26 @@ func TestRejectedMoveWithinVaultLeavesStateUnchanged(t *testing.T) { require.EqualError(t, err, tt.wantErr) }) } + + // Each of these spells "work" a second way. The spelling is not a valid + // vault name, so the move is not taken for a move between two vaults, + // which would delete the destination, here the source. + invalidNames := []struct{ source, dest string }{ + {workX, "work/:x"}, + {"work/:x", "work:"}, + {workX, "./work:x"}, + } + + for _, tt := range invalidNames { + t.Run("mv --force "+tt.source+" "+tt.dest, func(t *testing.T) { + t.Parallel() + + requireRejectedAndUnchanged(t, before, vault.ErrInvalidVaultName, + func(c *cli.Instance) error { + return c.MoveSecret(&cobra.Command{}, tt.source, tt.dest, true) + }) + }) + } } // TestMoveWithinOtherVaultKeepsCurrentVault checks that `secret mv work:x diff --git a/internal/cli/path_traversal_test.go b/internal/cli/path_traversal_test.go index a7d472a..d69bac5 100644 --- a/internal/cli/path_traversal_test.go +++ b/internal/cli/path_traversal_test.go @@ -154,11 +154,10 @@ func newFsFromSnapshot(t *testing.T, tree map[string]string) afero.Fs { } // requireRejectedAndUnchanged runs a command on a copy of the state -// directory recorded in before. It requires an error with exactly the -// message of want, so that a later check rejecting the argument does not -// count, and everything under the state directory as it was: the error -// alone proves nothing, since it could come after the vault had already -// been deleted. +// directory recorded in before. It requires the error want, so that a later +// check rejecting the argument does not count, and everything under the +// state directory as it was: the error alone proves nothing, since it could +// come after the vault had already been deleted. func requireRejectedAndUnchanged( t *testing.T, before map[string]string, want error, run func(c *cli.Instance) error, @@ -170,7 +169,7 @@ func requireRejectedAndUnchanged( err := run(cli.NewCLIInstanceWithStateDir(fs, testStateDir)) require.Equal(t, before, snapshotStateDir(t, fs)) - require.EqualError(t, err, want.Error()) + require.ErrorIs(t, err, want) } // TestInvalidSecretNameLeavesVaultsUnchanged is a regression test for @@ -193,77 +192,76 @@ func TestInvalidSecretNameLeavesVaultsUnchanged(t *testing.T) { cmd := &cobra.Command{} tests := []struct { - command string - rejected string // the secret name the command must reject - run func(c *cli.Instance) error + command string + run func(c *cli.Instance) error }{ - {"rm --force ..", "..", func(c *cli.Instance) error { + {"rm --force ..", func(c *cli.Instance) error { return c.RemoveSecret(cmd, "..", true) }}, - {"rm --force .", ".", func(c *cli.Instance) error { + {"rm --force .", func(c *cli.Instance) error { return c.RemoveSecret(cmd, ".", true) }}, - {`rm --force ""`, "", func(c *cli.Instance) error { + {`rm --force ""`, func(c *cli.Instance) error { return c.RemoveSecret(cmd, "", true) }}, - {"rm --force ../../etc", "../../etc", func(c *cli.Instance) error { + {"rm --force ../../etc", func(c *cli.Instance) error { return c.RemoveSecret(cmd, "../../etc", true) }}, - {"mv --force .. x", "..", func(c *cli.Instance) error { + {"mv --force .. x", func(c *cli.Instance) error { return c.MoveSecret(cmd, "..", "x", true) }}, - {"mv --force x ..", "..", func(c *cli.Instance) error { + {"mv --force x ..", func(c *cli.Instance) error { return c.MoveSecret(cmd, "x", "..", true) }}, - {`mv --force x ""`, "", func(c *cli.Instance) error { + {`mv --force x ""`, func(c *cli.Instance) error { return c.MoveSecret(cmd, "x", "", true) }}, // "work" is not the current vault: a move within it must not // select it when a name is rejected. - {"mv --force work:.. work:x", "..", func(c *cli.Instance) error { + {"mv --force work:.. work:x", func(c *cli.Instance) error { return c.MoveSecret(cmd, "work:..", "work:x", true) }}, - {"mv --force work:x work:..", "..", func(c *cli.Instance) error { + {"mv --force work:x work:..", func(c *cli.Instance) error { return c.MoveSecret(cmd, "work:x", "work:..", true) }}, - {"mv --force default:.. work", "..", func(c *cli.Instance) error { + {"mv --force default:.. work", func(c *cli.Instance) error { return c.MoveSecret(cmd, "default:..", "work", true) }}, - {"mv --force default:.. work:y", "..", func(c *cli.Instance) error { + {"mv --force default:.. work:y", func(c *cli.Instance) error { return c.MoveSecret(cmd, "default:..", "work:y", true) }}, - {"mv --force default:x work:..", "..", func(c *cli.Instance) error { + {"mv --force default:x work:..", func(c *cli.Instance) error { return c.MoveSecret(cmd, "default:x", "work:..", true) }}, - {"import --force ..", "..", func(c *cli.Instance) error { + {"import --force ..", func(c *cli.Instance) error { return c.ImportSecret(cmd, "..", missingFile, true) }}, - {"import --force .", ".", func(c *cli.Instance) error { + {"import --force .", func(c *cli.Instance) error { return c.ImportSecret(cmd, ".", missingFile, true) }}, - {"import --force ../../etc", "../../etc", func(c *cli.Instance) error { + {"import --force ../../etc", func(c *cli.Instance) error { return c.ImportSecret(cmd, "../../etc", missingFile, true) }}, - {"version list ..", "..", func(c *cli.Instance) error { + {"version list ..", func(c *cli.Instance) error { return c.ListVersions(cmd, "..") }}, - {"version promote ..", "..", func(c *cli.Instance) error { + {"version promote ..", func(c *cli.Instance) error { return c.PromoteVersion(cmd, "..", testVersion) }}, - {"version rm --force ..", "..", func(c *cli.Instance) error { + {"version rm --force ..", func(c *cli.Instance) error { return c.RemoveVersion(cmd, "..", testVersion, true) }}, - {"encrypt ..", "..", func(c *cli.Instance) error { + {"encrypt ..", func(c *cli.Instance) error { return c.Encrypt("..", "", "") }}, - {"decrypt ..", "..", func(c *cli.Instance) error { + {"decrypt ..", func(c *cli.Instance) error { return c.Decrypt("..", "", "") }}, } for _, tt := range tests { t.Run(tt.command, func(t *testing.T) { - requireRejectedAndUnchanged(t, before, vault.ValidateSecretName(tt.rejected), tt.run) + requireRejectedAndUnchanged(t, before, vault.ErrInvalidSecretName, tt.run) }) } } @@ -299,10 +297,18 @@ func TestInvalidVersionLeavesVaultsUnchanged(t *testing.T) { for _, tt := range commands { for _, version := range []string{"", ".", "..", "../../..", "a/b"} { t.Run(fmt.Sprintf("%s %q", tt.command, version), func(t *testing.T) { + fs := newFsFromSnapshot(t, before) + + err := tt.run(cli.NewCLIInstanceWithStateDir(fs, testStateDir), version) + + require.Equal(t, before, snapshotStateDir(t, fs)) + + // Compared as text: `version rm` and `version promote` return + // internal/cli's own error of this text, which errors.Is does + // not match to vault.ErrVersionNotFound. want := fmt.Errorf("version '%s' %w '%s'", version, vault.ErrVersionNotFound, "x") - requireRejectedAndUnchanged(t, before, want, - func(c *cli.Instance) error { return tt.run(c, version) }) + require.EqualError(t, err, want.Error()) }) } } @@ -355,7 +361,7 @@ func TestInvalidVaultNameLeavesStateUnchanged(t *testing.T) { for _, tt := range commands { for _, name := range []string{"", ".", "..", "a/b"} { t.Run(fmt.Sprintf(tt.command, name), func(t *testing.T) { - requireRejectedAndUnchanged(t, before, vault.ValidateVaultName(name), + requireRejectedAndUnchanged(t, before, vault.ErrInvalidVaultName, func(c *cli.Instance) error { c.Mnemonic = mnemonic c.UnlockPassphrase = passphrase diff --git a/internal/cli/secrets_size_test.go b/internal/cli/secrets_size_test.go index 3df84b8..5c041f4 100644 --- a/internal/cli/secrets_size_test.go +++ b/internal/cli/secrets_size_test.go @@ -93,8 +93,8 @@ func newSizeTestVault(t *testing.T) (afero.Fs, *vault.Vault) { } // runAddSecretSizeCase adds a secret of the given size through stdin and -// verifies the outcome. -func runAddSecretSizeCase(t *testing.T, size int, wantErr bool, errMsg string) { +// verifies the outcome: wantErr, or the secret stored when wantErr is nil. +func runAddSecretSizeCase(t *testing.T, size int, wantErr error) { t.Helper() skipIfLockedMemoryTooLow(t, size) @@ -128,9 +128,8 @@ func runAddSecretSizeCase(t *testing.T, size int, wantErr bool, errMsg string) { secretName := fmt.Sprintf("test-secret-%d", size) err = cli.AddSecret(secretName, false) - if wantErr { - require.Error(t, err) - assert.Contains(t, err.Error(), errMsg) + if wantErr != nil { + require.ErrorIs(t, err, wantErr) return } @@ -148,8 +147,8 @@ func runAddSecretSizeCase(t *testing.T, size int, wantErr bool, errMsg string) { } // runImportSecretSizeCase imports a secret file of the given size and -// verifies the outcome. -func runImportSecretSizeCase(t *testing.T, size int, wantErr bool, errMsg string) { +// verifies the outcome: wantErr, or the secret stored when wantErr is nil. +func runImportSecretSizeCase(t *testing.T, size int, wantErr error) { t.Helper() skipIfLockedMemoryTooLow(t, size) @@ -181,9 +180,8 @@ func runImportSecretSizeCase(t *testing.T, size int, wantErr bool, errMsg string secretName := fmt.Sprintf("imported-secret-%d", size) err = cli.ImportSecret(cmd, secretName, testFile, false) - if wantErr { - require.Error(t, err) - assert.Contains(t, err.Error(), errMsg) + if wantErr != nil { + require.ErrorIs(t, err, wantErr) return } @@ -205,57 +203,48 @@ func runImportSecretSizeCase(t *testing.T, size int, wantErr bool, errMsg string //nolint:paralleltest // together the subtests lock more than the memlock limit func TestAddSecretVariousSizes(t *testing.T) { tests := []struct { - name string - size int - shouldError bool - errorMsg string + name string + size int + wantErr error }{ { - name: "1KB secret", - size: 1024, - shouldError: false, + name: "1KB secret", + size: 1024, }, { - name: "10KB secret", - size: 10 * 1024, - shouldError: false, + name: "10KB secret", + size: 10 * 1024, }, { - name: "100KB secret", - size: 100 * 1024, - shouldError: false, + name: "100KB secret", + size: 100 * 1024, }, { - name: "1MB secret", - size: 1024 * 1024, - shouldError: false, + name: "1MB secret", + size: 1024 * 1024, }, { - name: "10MB secret", - size: 10 * 1024 * 1024, - shouldError: false, + name: "10MB secret", + size: 10 * 1024 * 1024, }, { - name: "99MB secret", - size: 99 * 1024 * 1024, - shouldError: false, + name: "99MB secret", + size: 99 * 1024 * 1024, }, { - name: "100MB secret minus 1 byte", - size: 100*1024*1024 - 1, - shouldError: false, + name: "100MB secret minus 1 byte", + size: 100*1024*1024 - 1, }, { - name: "101MB secret - should fail", - size: 101 * 1024 * 1024, - shouldError: true, - errorMsg: "secret too large: exceeds 100MB limit", + name: "101MB secret - should fail", + size: 101 * 1024 * 1024, + wantErr: errSecretTooLarge, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - runAddSecretSizeCase(t, tt.size, tt.shouldError, tt.errorMsg) + runAddSecretSizeCase(t, tt.size, tt.wantErr) }) } } @@ -265,57 +254,48 @@ func TestAddSecretVariousSizes(t *testing.T) { //nolint:paralleltest // together the subtests lock more than the memlock limit func TestImportSecretVariousSizes(t *testing.T) { tests := []struct { - name string - size int - shouldError bool - errorMsg string + name string + size int + wantErr error }{ { - name: "1KB file", - size: 1024, - shouldError: false, + name: "1KB file", + size: 1024, }, { - name: "10KB file", - size: 10 * 1024, - shouldError: false, + name: "10KB file", + size: 10 * 1024, }, { - name: "100KB file", - size: 100 * 1024, - shouldError: false, + name: "100KB file", + size: 100 * 1024, }, { - name: "1MB file", - size: 1024 * 1024, - shouldError: false, + name: "1MB file", + size: 1024 * 1024, }, { - name: "10MB file", - size: 10 * 1024 * 1024, - shouldError: false, + name: "10MB file", + size: 10 * 1024 * 1024, }, { - name: "99MB file", - size: 99 * 1024 * 1024, - shouldError: false, + name: "99MB file", + size: 99 * 1024 * 1024, }, { - name: "100MB file", - size: 100 * 1024 * 1024, - shouldError: false, + name: "100MB file", + size: 100 * 1024 * 1024, }, { - name: "101MB file - should fail", - size: 101 * 1024 * 1024, - shouldError: true, - errorMsg: "secret file too large: exceeds 100MB limit", + name: "101MB file - should fail", + size: 101 * 1024 * 1024, + wantErr: errSecretFileTooLarge, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - runImportSecretSizeCase(t, tt.size, tt.shouldError, tt.errorMsg) + runImportSecretSizeCase(t, tt.size, tt.wantErr) }) } } diff --git a/internal/cli/version_test.go b/internal/cli/version_test.go index 2e81e60..0afa5c2 100644 --- a/internal/cli/version_test.go +++ b/internal/cli/version_test.go @@ -171,8 +171,7 @@ func TestListVersionsNonExistentSecret(t *testing.T) { // Try to list versions of non-existent secret err := cli.ListVersions(cmd, "nonexistent/secret") - require.Error(t, err) - assert.Contains(t, err.Error(), "not found") + require.ErrorIs(t, err, errSecretNotFound) } func TestPromoteVersionCommand(t *testing.T) { @@ -266,8 +265,7 @@ func TestPromoteNonExistentVersion(t *testing.T) { // Try to promote non-existent version err = cli.PromoteVersion(cmd, "test/secret", "20991231.999") - require.Error(t, err) - assert.Contains(t, err.Error(), "not found") + require.ErrorIs(t, err, errVersionNotFound) } func TestGetSecretWithVersion(t *testing.T) { diff --git a/internal/secret/keychainunlocker_test.go b/internal/secret/keychainunlocker_test.go index 68f934b..918e244 100644 --- a/internal/secret/keychainunlocker_test.go +++ b/internal/secret/keychainunlocker_test.go @@ -72,9 +72,12 @@ func TestKeychainInvalidItemName(t *testing.T) { testData := memguard.NewBufferFromBytes([]byte("test")) defer testData.Destroy() + // Test an empty item name + err := storeInKeychain("", testData) + require.ErrorIs(t, err, errKeychainItemNameEmpty) + // Test invalid item names invalidNames := []string{ - "", // Empty name "test space", // Contains space "test/slash", // Contains slash "test\\backslash", // Contains backslash @@ -96,9 +99,8 @@ func TestKeychainInvalidItemName(t *testing.T) { for _, name := range invalidNames { err := storeInKeychain(name, testData) - assert.Error(t, err, "Expected error for invalid name: %s", name) - assert.Contains(t, err.Error(), "invalid keychain item name", - "Error should mention invalid name for: %s", name) + require.ErrorIs(t, err, errInvalidKeychainItemName, + "Expected error for invalid name: %s", name) } // Test valid names (should not error on validation) diff --git a/internal/secret/seunlocker_test.go b/internal/secret/seunlocker_test.go index a737999..4cdecc2 100644 --- a/internal/secret/seunlocker_test.go +++ b/internal/secret/seunlocker_test.go @@ -4,6 +4,8 @@ package secret import ( + "os" + "path/filepath" "testing" "time" @@ -106,6 +108,10 @@ func TestSecureEnclaveUnlockerGetIdentityMissingFile(t *testing.T) { // GetIdentity should fail because the encrypted longterm key file is missing identity, err := unlocker.GetIdentity() assert.Nil(t, identity) - require.Error(t, err) - assert.Contains(t, err.Error(), "failed to read SE-encrypted long-term key") + + var cause *os.PathError + + require.ErrorAs(t, err, &cause) + require.ErrorIs(t, err, os.ErrNotExist) + assert.Equal(t, filepath.Join(dir, seLongtermFilename), cause.Path) } diff --git a/internal/secret/version_internal_test.go b/internal/secret/version_internal_test.go new file mode 100644 index 0000000..d2b4f91 --- /dev/null +++ b/internal/secret/version_internal_test.go @@ -0,0 +1,31 @@ +package secret + +import ( + "fmt" + "path/filepath" + "testing" + "time" + + "github.com/spf13/afero" + "github.com/stretchr/testify/require" +) + +func TestGenerateVersionNameMaxSerial(t *testing.T) { + t.Parallel() + + fs := afero.NewMemMapFs() + secretDir := "/test/secret" + versionsDir := filepath.Join(secretDir, "versions") + + // Create 999 versions + today := time.Now().Format("20060102") + for i := 1; i <= 999; i++ { + versionName := fmt.Sprintf("%s.%03d", today, i) + err := fs.MkdirAll(filepath.Join(versionsDir, versionName), 0o755) + require.NoError(t, err) + } + + // Try to create one more - should fail + _, err := GenerateVersionName(fs, secretDir) + require.ErrorIs(t, err, errMaxVersionsPerDay) +} diff --git a/internal/secret/version_test.go b/internal/secret/version_test.go index a0ab039..9741678 100644 --- a/internal/secret/version_test.go +++ b/internal/secret/version_test.go @@ -36,7 +36,6 @@ package secret_test import ( "errors" - "fmt" "path/filepath" "testing" "time" @@ -127,27 +126,6 @@ func TestGenerateVersionName(t *testing.T) { assert.NotEqual(t, version1, version2) } -func TestGenerateVersionNameMaxSerial(t *testing.T) { - t.Parallel() - - fs := afero.NewMemMapFs() - secretDir := testSecretDir - versionsDir := filepath.Join(secretDir, "versions") - - // Create 999 versions - today := time.Now().Format("20060102") - for i := 1; i <= 999; i++ { - versionName := fmt.Sprintf("%s.%03d", today, i) - err := fs.MkdirAll(filepath.Join(versionsDir, versionName), 0o755) - require.NoError(t, err) - } - - // Try to create one more - should fail - _, err := secret.GenerateVersionName(fs, secretDir) - require.Error(t, err) - assert.Contains(t, err.Error(), "exceeded maximum versions per day") -} - func TestNewVersion(t *testing.T) { t.Parallel() diff --git a/internal/vault/errors_test.go b/internal/vault/errors_test.go new file mode 100644 index 0000000..b8c2b44 --- /dev/null +++ b/internal/vault/errors_test.go @@ -0,0 +1,138 @@ +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/require" +) + +const ( + // otherMnemonic is a valid BIP39 mnemonic other than testMnemonic. + otherMnemonic = "legal winner thank year wave sausage worth useful " + + "legal winner thank yellow" + + // missingName names no vault, secret or unlocker. + missingName = "missing" +) + +// newErrorTestVault creates the vault testVaultName, with the secret +// testSecretName in it, on a new in-memory filesystem. +func newErrorTestVault(t *testing.T) *vault.Vault { + t.Helper() + + vlt, err := vault.CreateVault(afero.NewMemMapFs(), testStateDir, + testVaultName, testMnemonicBuffer(t), nil) + require.NoError(t, err) + + value := memguard.NewBufferFromBytes([]byte("value")) + t.Cleanup(value.Destroy) + + require.NoError(t, vlt.AddSecret(testSecretName, value, false)) + + return vlt +} + +// TestVaultErrors checks that each failure returns its exported error, +// wrapped or not, so that errors.Is tells it apart from the others. +func TestVaultErrors(t *testing.T) { + t.Parallel() + + vaultDir := filepath.Join(testStateDir, "vaults.d", testVaultName) + + tests := []struct { + name string + run func(vlt *vault.Vault) error + want error + }{ + {"create an existing vault", func(vlt *vault.Vault) error { + _, err := vault.CreateVault(vlt.GetFilesystem(), testStateDir, + testVaultName, nil, nil) + + return err + }, vault.ErrVaultExists}, + {"select a missing vault", func(vlt *vault.Vault) error { + return vault.SelectVault(vlt.GetFilesystem(), testStateDir, missingName) + }, vault.ErrVaultNotFound}, + {"add a nil value", func(vlt *vault.Vault) error { + return vlt.AddSecret(missingName, nil, false) + }, vault.ErrNilValueBuffer}, + {"get a missing secret", func(vlt *vault.Vault) error { + _, err := vlt.GetSecret(missingName) + + return err + }, vault.ErrSecretNotFound}, + {"copy onto an existing secret", func(vlt *vault.Vault) error { + return vlt.CopySecretAllVersions(vlt, testSecretName, testSecretName, false) + }, vault.ErrSecretExists}, + {"copy a secret without versions", func(vlt *vault.Vault) error { + const versionless = "versionless" + + err := vlt.GetFilesystem().MkdirAll( + filepath.Join(vaultDir, "secrets.d", versionless), secret.DirPerms) + if err != nil { + return err + } + + return vlt.CopySecretAllVersions(vlt, versionless, "copy", false) + }, vault.ErrNoVersions}, + {"remove a missing unlocker", func(vlt *vault.Vault) error { + return vlt.RemoveUnlocker(missingName) + }, vault.ErrUnlockerNotFound}, + {"select a missing unlocker", func(vlt *vault.Vault) error { + return vlt.SelectUnlocker(missingName) + }, vault.ErrUnlockerNotFound}, + {"unlocker of an unknown type", func(vlt *vault.Vault) error { + fs := vlt.GetFilesystem() + + err := afero.WriteFile(fs, + filepath.Join(vaultDir, "unlockers.d", "odd", "unlocker-metadata.json"), + []byte(`{"type":"odd"}`), secret.FilePerms) + if err != nil { + return err + } + + err = afero.WriteFile(fs, filepath.Join(vaultDir, "current-unlocker"), + []byte("odd"), secret.FilePerms) + if err != nil { + return err + } + + _, err = vlt.GetCurrentUnlocker() + + return err + }, vault.ErrUnsupportedUnlockerType}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + require.ErrorIs(t, tt.run(newErrorTestVault(t)), tt.want) + }) + } +} + +// TestGetSecretWithWrongMnemonic checks that getting a secret that exists, +// from a vault the given mnemonic does not open, fails with +// ErrMnemonicMismatch through GetSecret's wrapping, and not with +// ErrSecretNotFound. +func TestGetSecretWithWrongMnemonic(t *testing.T) { + t.Parallel() + + created := newErrorTestVault(t) + + mnemonic := memguard.NewBufferFromBytes([]byte(otherMnemonic)) + t.Cleanup(mnemonic.Destroy) + + vlt := vault.NewVault(created.GetFilesystem(), testStateDir, testVaultName) + vlt.SetMnemonic(mnemonic) + + _, err := vlt.GetSecret(testSecretName) + require.ErrorIs(t, err, vault.ErrMnemonicMismatch) + require.NotErrorIs(t, err, vault.ErrSecretNotFound) +} diff --git a/internal/vault/integration_test.go b/internal/vault/integration_test.go index 4c28f0c..bae3301 100644 --- a/internal/vault/integration_test.go +++ b/internal/vault/integration_test.go @@ -2,6 +2,7 @@ package vault_test import ( "bytes" + "errors" "os" "path/filepath" "slices" @@ -341,9 +342,9 @@ func testVaultNameValidation(t *testing.T, fs afero.Fs, tempDir string) { for _, name := range invalidNames { _, err := vault.CreateVault(fs, stateDir, name, testMnemonicBuffer(t), nil) - if err == nil { - t.Errorf("Expected error creating vault with invalid name %q, "+ - "but got none", name) + if !errors.Is(err, vault.ErrInvalidVaultName) { + t.Errorf("Expected ErrInvalidVaultName creating vault with "+ + "invalid name %q, got %v", name, err) } } } diff --git a/internal/vault/integration_version_test.go b/internal/vault/integration_version_test.go index 00e2b8b..a1a528c 100644 --- a/internal/vault/integration_version_test.go +++ b/internal/vault/integration_version_test.go @@ -320,10 +320,10 @@ func testVersionSerialLimits( err = fs.MkdirAll(filepath.Join(secretDir, versionName), 0o755) require.NoError(t, err) - // Should fail to create 1000th version + // Should fail to create 1000th version. The error is unexported in + // package secret, whose own test checks that it is the one returned. _, err = secret.GenerateVersionName(fs, filepath.Dir(secretDir)) require.Error(t, err) - assert.Contains(t, err.Error(), "exceeded maximum versions per day") } func testVersionErrorCases(t *testing.T, vault *Vault, secretName string) { @@ -331,20 +331,18 @@ func testVersionErrorCases(t *testing.T, vault *Vault, secretName string) { // Try to get non-existent version _, err := vault.GetSecretVersion(secretName, "99991231.999") - require.Error(t, err) - assert.Contains(t, err.Error(), "not found") + require.ErrorIs(t, err, ErrVersionNotFound) // Try to get version of non-existent secret _, err = vault.GetSecretVersion("nonexistent/secret", "") - require.Error(t, err) + require.ErrorIs(t, err, ErrSecretNotFound) // Try to add secret without force when it exists failBuffer := memguard.NewBufferFromBytes([]byte("should-fail")) defer failBuffer.Destroy() err = vault.AddSecret(secretName, failBuffer, false) - require.Error(t, err) - assert.Contains(t, err.Error(), "already exists") + require.ErrorIs(t, err, ErrSecretExists) } // TestVersionConcurrency tests concurrent version operations diff --git a/internal/vault/path_traversal_test.go b/internal/vault/path_traversal_test.go index e88a147..25e58ff 100644 --- a/internal/vault/path_traversal_test.go +++ b/internal/vault/path_traversal_test.go @@ -41,10 +41,8 @@ func TestGetSecretVersionRejectsPathTraversal(t *testing.T) { t.Parallel() _, err := vlt.GetSecretVersion(name, "") - require.Error(t, err, + require.ErrorIs(t, err, vault.ErrInvalidSecretName, "GetSecretVersion should reject malicious name: %s", name) - require.Contains(t, err.Error(), "invalid secret name", - "error should indicate invalid name for: %s", name) }) } } @@ -61,8 +59,7 @@ func TestGetSecretRejectsPathTraversal(t *testing.T) { require.NoError(t, err) _, err = vlt.GetSecret("../../../etc/passwd") - require.Error(t, err) - require.Contains(t, err.Error(), "invalid secret name") + require.ErrorIs(t, err, vault.ErrInvalidSecretName) } // TestGetSecretObjectRejectsPathTraversal verifies GetSecretObject @@ -87,8 +84,8 @@ func TestGetSecretObjectRejectsPathTraversal(t *testing.T) { t.Parallel() _, err := vlt.GetSecretObject(name) - require.Error(t, err, "GetSecretObject should reject: %s", name) - require.Contains(t, err.Error(), "invalid secret name") + require.ErrorIs(t, err, vault.ErrInvalidSecretName, + "GetSecretObject should reject: %s", name) }) } } diff --git a/internal/vault/secrets_version_test.go b/internal/vault/secrets_version_test.go index 9a85aad..d128a0a 100644 --- a/internal/vault/secrets_version_test.go +++ b/internal/vault/secrets_version_test.go @@ -143,8 +143,7 @@ func TestVaultAddSecretMultipleVersions(t *testing.T) { defer failBuffer.Destroy() err := vault.AddSecret(testSecretPath, failBuffer, false) - require.Error(t, err) - assert.Contains(t, err.Error(), "already exists") + require.ErrorIs(t, err, ErrSecretExists) // Add with force - should create new version addTestSecretToVault(t, vault, testSecretPath, []byte("version-2"), true) @@ -309,8 +308,7 @@ func TestVaultGetNonExistentVersion(t *testing.T) { // Try to get non-existent version _, err := vault.GetSecretVersion(testSecretPath, "20991231.999") - require.Error(t, err) - assert.Contains(t, err.Error(), "not found") + require.ErrorIs(t, err, ErrVersionNotFound) } func TestUpdateVersionMetadata(t *testing.T) { diff --git a/internal/vault/vault_error_test.go b/internal/vault/vault_error_test.go index bec8833..664a518 100644 --- a/internal/vault/vault_error_test.go +++ b/internal/vault/vault_error_test.go @@ -1,6 +1,7 @@ package vault_test import ( + "os" "path/filepath" "testing" @@ -36,8 +37,13 @@ func TestAddSecretFailsWithMissingPublicKey(t *testing.T) { defer value.Destroy() err := vlt.AddSecret(testSecretName, 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") + + var cause *os.PathError + + require.ErrorAs(t, err, &cause) + require.ErrorIs(t, err, os.ErrNotExist, + "AddSecret should fail when public key is missing") + assert.Equal(t, filepath.Join(vaultDir, "pub.age"), cause.Path) // Verify that the secret directory was NOT created secretDir := filepath.Join(vaultDir, "secrets.d", testSecretName) diff --git a/pkg/agehd/agehd_test.go b/pkg/agehd/agehd_test.go index 42db934..4aba8d4 100644 --- a/pkg/agehd/agehd_test.go +++ b/pkg/agehd/agehd_test.go @@ -38,7 +38,6 @@ const ( testMessageLargePattern = "A" // Error messages for validation - errorMsgNeed32Bytes = "need 32-byte scalar, got" errorMsgInvalidXPRV = "invalid-xprv" // Test constants for various scenarios @@ -330,24 +329,17 @@ func TestClampFunction(t *testing.T) { } } -// requireIdentityError asserts that identity derivation failed with an -// error containing errorMsg and returned no identity. +// requireIdentityError asserts that identity derivation failed with +// errInvalidScalarSize and returned no identity. func requireIdentityError( t *testing.T, identity *age.X25519Identity, err error, - errorMsg string, ) { t.Helper() - if err == nil { - t.Errorf("expected error but got none") - } else if !strings.Contains(err.Error(), errorMsg) { - t.Errorf( - "expected error containing %q, got %q", - errorMsg, - err.Error(), - ) + if !errors.Is(err, errInvalidScalarSize) { + t.Errorf("expected errInvalidScalarSize, got %v", err) } if identity != nil { @@ -363,31 +355,26 @@ func TestIdentityFromEntropyEdgeCases(t *testing.T) { name string entropy []byte expectError bool - errorMsg string }{ { name: "nil entropy", entropy: nil, expectError: true, - errorMsg: errorMsgNeed32Bytes + " 0", }, { name: "empty entropy", entropy: []byte{}, expectError: true, - errorMsg: errorMsgNeed32Bytes + " 0", }, { name: "too short entropy", entropy: make([]byte, 31), expectError: true, - errorMsg: errorMsgNeed32Bytes + " 31", }, { name: "too long entropy", entropy: make([]byte, 33), expectError: true, - errorMsg: errorMsgNeed32Bytes + " 33", }, { name: "valid 32-byte entropy", @@ -419,7 +406,7 @@ func TestIdentityFromEntropyEdgeCases(t *testing.T) { identity, err := IdentityFromEntropy(tt.entropy) if tt.expectError { - requireIdentityError(t, identity, err, tt.errorMsg) + requireIdentityError(t, identity, err) return } diff --git a/pkg/bip85/bip85_test.go b/pkg/bip85/bip85_test.go index f4107cf..c0ac4d7 100644 --- a/pkg/bip85/bip85_test.go +++ b/pkg/bip85/bip85_test.go @@ -4,6 +4,7 @@ package bip85_test import ( "bytes" "encoding/hex" + "errors" "fmt" "strings" "testing" @@ -1013,14 +1014,13 @@ func TestHexDerivation(t *testing.T) { func TestInvalidParameters(t *testing.T) { t.Parallel() - logTestVector(t, "Invalid Parameters") - masterKey := mustParseTestMasterKey(t) // Test cases for parameter validation testCases := []struct { name string testFunc func() error + want error }{ { name: "BIP39 invalid word count", @@ -1030,6 +1030,7 @@ func TestInvalidParameters(t *testing.T) { return err }, + want: bip85.ErrInvalidWordCount, }, { name: "Base64 password too short", @@ -1039,6 +1040,7 @@ func TestInvalidParameters(t *testing.T) { return err }, + want: bip85.ErrInvalidBase64PwdLen, }, { name: "Base64 password too long", @@ -1048,6 +1050,7 @@ func TestInvalidParameters(t *testing.T) { return err }, + want: bip85.ErrInvalidBase64PwdLen, }, { name: "Base85 password too short", @@ -1057,6 +1060,7 @@ func TestInvalidParameters(t *testing.T) { return err }, + want: bip85.ErrInvalidBase85PwdLen, }, { name: "Base85 password too long", @@ -1066,6 +1070,7 @@ func TestInvalidParameters(t *testing.T) { return err }, + want: bip85.ErrInvalidBase85PwdLen, }, { name: "Hex data too small", @@ -1075,6 +1080,7 @@ func TestInvalidParameters(t *testing.T) { return err }, + want: bip85.ErrInvalidNumBytes, }, { name: "Hex data too large", @@ -1084,23 +1090,43 @@ func TestInvalidParameters(t *testing.T) { return err }, + want: bip85.ErrInvalidNumBytes, }, } // Run all validation test cases for _, tc := range testCases { - t.Logf("Testing: %s", tc.name) - err := tc.testFunc() - if err == nil { - t.Errorf("Expected error for %s, but got nil", tc.name) - } else { - t.Logf("Got expected error: %v", err) - t.Logf("RESULT: PASS") + if !errors.Is(err, tc.want) { + t.Errorf("Expected %v for %s, got %v", tc.want, tc.name, err) } } } +// TestDeriveBIP85EntropyErrors checks that DeriveBIP85Entropy returns +// ErrNotPrivateKey for a public master key, and ErrInvalidPathComponent, +// wrapped, for a path component that is not a number. +func TestDeriveBIP85EntropyErrors(t *testing.T) { + t.Parallel() + + masterKey := mustParseTestMasterKey(t) + + publicKey, err := masterKey.Neuter() + if err != nil { + t.Fatalf("Failed to get the public key of the master key: %v", err) + } + + _, err = bip85.DeriveBIP85Entropy(publicKey, testCase1Path) + if !errors.Is(err, bip85.ErrNotPrivateKey) { + t.Errorf("Expected ErrNotPrivateKey, got %v", err) + } + + _, err = bip85.DeriveBIP85Entropy(masterKey, bip85.BIP85_MASTER_PATH+"/x'") + if !errors.Is(err, bip85.ErrInvalidPathComponent) { + t.Errorf("Expected ErrInvalidPathComponent, got %v", err) + } +} + // TestAdditionalDeriveHex tests additional hex derivation scenarios func TestAdditionalDeriveHex(t *testing.T) { t.Parallel()