From 3ba8857a0318d68ba9c10b7faa81ead59d62e3df Mon Sep 17 00:00:00 2001 From: sneak Date: Wed, 14 Oct 2026 20:57:24 +0000 Subject: [PATCH] Stop vault creation when a vault's metadata cannot be read (closes #140) GetNextDerivationIndex passed over a vault whose vault-metadata.json could not be read or parsed, so the next vault made from the same mnemonic could get that vault's derivation index and so the same long-term key. It now returns an error naming the file, and vault create, vault import and init stop before writing anything. A vault directory with no metadata file is still passed over. The regression test corrupts one vault's metadata and requires CreateVault with the same mnemonic to fail, naming the file, and leave no new vault. Unverified by test: an unreadable metadata file; checked by hand only. Model: opus-5-5 --- README.md | 4 +++- TODO.md | 7 +++++++ internal/vault/metadata.go | 19 +++++++++++++------ internal/vault/metadata_test.go | 30 ++++++++++++++++++++++++++++++ 4 files changed, 53 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 1e2b834..a6b6c5d 100644 --- a/README.md +++ b/README.md @@ -522,7 +522,9 @@ secret decrypt encryption/mykey --input document.txt.age --output document.txt ### Vault Management - **Derivation Index**: Each vault uses a unique derivation index from the - mnemonic, and thus a unique key pair + mnemonic, and thus a unique key pair; creating a vault or importing a mnemonic + fails, with an error naming the file, when any vault's `vault-metadata.json` + cannot be read or parsed, as that vault's index is then unknown - **Public Key Hash**: Double SHA-256 hash of the vault's public key; the same hash of the index-0 public key identifies vaults from the same mnemonic - **Automatic Key Derivation**: When creating vaults with a mnemonic, keys are diff --git a/TODO.md b/TODO.md index d659129..faf3954 100644 --- a/TODO.md +++ b/TODO.md @@ -18,6 +18,13 @@ https://git.eeqj.de/sneak/secret/milestone/12 # Completed Steps +- 2026-10-14: Creating a vault, or importing a mnemonic into one, fails with an + error naming the file when any vault's `vault-metadata.json` exists but cannot + be read or parsed, and writes nothing + (https://git.eeqj.de/sneak/secret/issues/140). `vault.GetNextDerivationIndex` + passed over such a vault, so the next vault made from the same mnemonic could + be given its derivation index, and so its long-term key. A vault directory + with no `vault-metadata.json` is still passed over. - 2026-10-14: The passphrase and mnemonic prompts ask on the controlling terminal (`/dev/tty`) instead of stdin, so `echo v | secret add s1 --force`, `cat file | secret encrypt key` and `cat file.age | secret decrypt key` ask diff --git a/internal/vault/metadata.go b/internal/vault/metadata.go index bfeb538..218ef54 100644 --- a/internal/vault/metadata.go +++ b/internal/vault/metadata.go @@ -4,7 +4,9 @@ import ( "crypto/sha256" "encoding/hex" "encoding/json" + "errors" "fmt" + "os" "path/filepath" "github.com/spf13/afero" @@ -33,7 +35,10 @@ func ComputeDoubleSHA256(data []byte) string { } // GetNextDerivationIndex finds the next available derivation index for a given mnemonic -// by deriving the public key for index 0 and using its hash to identify related vaults +// by deriving the public key for index 0 and using its hash to identify related vaults. +// It fails when a vault's metadata file exists but cannot be read or parsed: the +// index that vault uses is then unknown, and handing it out again would give a +// second vault the same long-term key. func GetNextDerivationIndex( fs afero.Fs, stateDir string, mnemonic string, ) (uint32, error) { @@ -72,21 +77,23 @@ func GetNextDerivationIndex( continue } - // Try to read vault metadata metadataPath := filepath.Join(vaultsDir, entry.Name(), "vault-metadata.json") metadataBytes, err := afero.ReadFile(fs, metadataPath) - if err != nil { - // Skip vaults without metadata + if errors.Is(err, os.ErrNotExist) { + // Skip directories without metadata continue } + if err != nil { + return 0, fmt.Errorf("failed to read vault metadata %s: %w", metadataPath, err) + } + var metadata Metadata err = json.Unmarshal(metadataBytes, &metadata) if err != nil { - // Skip vaults with invalid metadata - continue + return 0, fmt.Errorf("failed to parse vault metadata %s: %w", metadataPath, err) } // Check if this vault uses the same mnemonic by comparing family hashes diff --git a/internal/vault/metadata_test.go b/internal/vault/metadata_test.go index f3d6206..3291dd7 100644 --- a/internal/vault/metadata_test.go +++ b/internal/vault/metadata_test.go @@ -6,6 +6,9 @@ import ( "testing" "github.com/spf13/afero" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "sneak.berlin/go/secret/internal/secret" "sneak.berlin/go/secret/internal/vault" "sneak.berlin/go/secret/pkg/agehd" ) @@ -163,6 +166,33 @@ func testGetNextDerivationIndex(t *testing.T, fs afero.Fs) { } } +// TestCreateVaultWithCorruptMetadataFails checks that creating a vault from +// a mnemonic fails, naming the file and writing nothing, when another +// vault's metadata cannot be parsed. That vault's derivation index is then +// unknown, so the new vault could get the same one and the same key. +func TestCreateVaultWithCorruptMetadataFails(t *testing.T) { + t.Parallel() + + fs := afero.NewMemMapFs() + + _, err := vault.CreateVault(fs, testStateDir, "work", testMnemonicBuffer(t), nil) + require.NoError(t, err) + + vaultsDir := filepath.Join(testStateDir, "vaults.d") + metadataPath := filepath.Join(vaultsDir, "work", "vault-metadata.json") + + err = afero.WriteFile(fs, metadataPath, []byte("{"), secret.FilePerms) + require.NoError(t, err) + + _, err = vault.CreateVault(fs, testStateDir, "other", testMnemonicBuffer(t), nil) + require.ErrorContains(t, err, metadataPath) + + entries, err := afero.ReadDir(fs, vaultsDir) + require.NoError(t, err) + require.Len(t, entries, 1) + assert.Equal(t, "work", entries[0].Name()) +} + func testMetadataPersistence(t *testing.T, fs afero.Fs) { t.Helper()