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

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

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()