From d88ed64489990bfb3a867608e9bbf10612a1ae28 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 22 Sep 2026 15:45:27 +0200 Subject: [PATCH] Parse the age identity key once and accept every identity in it (closes #165) Restore and verify --deep now parse the configured age secret key a single time through a new helper that uses age.ParseIdentities and hands every identity to age.Decrypt. A key file with several identities (a whole age-keygen file) is fully accepted, so a blob encrypted to any of its recipients decrypts, not just the first. The helper is the first step of both commands, so a missing or unparseable key fails before anything is downloaded. Its error names the config source and never echoes the key value. config.extractAgeSecretKey and its silent fallback are removed; the key is stored raw and parsed only where decryption happens. README, the restore help, and the missing-key error now read the key from a file with \$(cat ...) rather than typed literally, keeping it out of shell history. Model: opus-4-8 --- README.md | 21 ++-- internal/blobgen/reader.go | 8 +- internal/cli/snapshot_restore.go | 8 +- internal/cli/snapshot_restore_test.go | 37 ++++++ internal/config/config.go | 65 +++++++---- internal/config/config_test.go | 63 ++++------ internal/vaultik/blob_fetch.go | 5 +- internal/vaultik/restore.go | 50 +++++--- internal/vaultik/restore_identity_test.go | 108 ++++++++++++++++++ .../vaultik/restore_malformed_key_test.go | 44 +++++++ internal/vaultik/verify.go | 36 +++--- 11 files changed, 329 insertions(+), 116 deletions(-) create mode 100644 internal/cli/snapshot_restore_test.go create mode 100644 internal/vaultik/restore_identity_test.go create mode 100644 internal/vaultik/restore_malformed_key_test.go diff --git a/README.md b/README.md index a7d26c2..86e89e6 100644 --- a/README.md +++ b/README.md @@ -74,11 +74,16 @@ Requirements that no existing tool meets: # verify a snapshot (shallow: checks all blobs are present with the listed size) vaultik snapshot verify +# put the private key file in the environment (reading it from the file +# keeps the key out of your shell history); the whole age-keygen file, +# with one or more identities, is accepted +export VAULTIK_AGE_SECRET_KEY="$(cat vaultik_backup_private_key.txt)" + # deep verify (downloads and cryptographically verifies every blob) -VAULTIK_AGE_SECRET_KEY='AGE-SECRET-KEY-...' vaultik snapshot verify --deep +vaultik snapshot verify --deep # restore (requires the private key) -VAULTIK_AGE_SECRET_KEY='AGE-SECRET-KEY-...' vaultik snapshot restore /tmp/restored +vaultik snapshot restore /tmp/restored # daily cron job: back up, keep a 4-week rolling window of snapshots # 0 3 * * * vaultik snapshot create --cron --prune --keep-newer-than 4w @@ -119,15 +124,17 @@ Use that remote key — the hex printed inside ``, or the full `remote_key` from `snapshot list --json` — to restore and verify: ```sh +# put the private key file in the environment (reading it from the file +# keeps the key out of your shell history) +export VAULTIK_AGE_SECRET_KEY="$(cat vaultik_backup_private_key.txt)" + # restore everything to /tmp/restored, then check every restored file's # chunk hashes -VAULTIK_AGE_SECRET_KEY='AGE-SECRET-KEY-...' \ - vaultik snapshot restore --verify /tmp/restored +vaultik snapshot restore --verify /tmp/restored # optionally, deep-verify the snapshot against the store (downloads and # cryptographically checks every blob) -VAULTIK_AGE_SECRET_KEY='AGE-SECRET-KEY-...' \ - vaultik snapshot verify --deep +vaultik snapshot verify --deep ``` `age_recipients` (the public key) is not needed to restore — only the @@ -217,7 +224,7 @@ and `vaultik prune --json | jq .` both work as written. ### environment variables -* `VAULTIK_AGE_SECRET_KEY`: Age private key for decryption (required for `snapshot restore` and `snapshot verify --deep`) +* `VAULTIK_AGE_SECRET_KEY`: Age private key for decryption (required for `snapshot restore` and `snapshot verify --deep`). May hold the whole `age-keygen` file — comments and every identity in it are accepted. Set it from the file, e.g. `export VAULTIK_AGE_SECRET_KEY="$(cat vaultik_backup_private_key.txt)"`, so the key is not typed into your shell history. * `VAULTIK_CONFIG`: Path to config file (overridden by `--config`) * `VAULTIK_INDEX_PATH`: Override local SQLite index path * `VAULTIK_CPUPROFILE`: Write a CPU profile to this path for the duration of the run (development/debugging) diff --git a/internal/blobgen/reader.go b/internal/blobgen/reader.go index 94f3b90..7c6ebeb 100644 --- a/internal/blobgen/reader.go +++ b/internal/blobgen/reader.go @@ -20,10 +20,12 @@ type Reader struct { bytesRead int64 } -// NewReader creates a new Reader that decrypts, decompresses, and verifies data -func NewReader(r io.Reader, identity age.Identity) (*Reader, error) { +// NewReader creates a new Reader that decrypts, decompresses, and verifies +// data. Every supplied identity is offered to age.Decrypt, so a blob +// encrypted to any one of them can be read. +func NewReader(r io.Reader, identities ...age.Identity) (*Reader, error) { // Create decryption reader - decReader, err := age.Decrypt(r, identity) + decReader, err := age.Decrypt(r, identities...) if err != nil { return nil, fmt.Errorf("creating decryption reader: %w", err) } diff --git a/internal/cli/snapshot_restore.go b/internal/cli/snapshot_restore.go index 45707be..ce28542 100644 --- a/internal/cli/snapshot_restore.go +++ b/internal/cli/snapshot_restore.go @@ -35,8 +35,12 @@ The snapshot may be named by its ID or, when restoring on a host with no local index, by the remote key that 'snapshot list' prints for a remote-only snapshot (an unambiguous leading part is enough). -Requires the VAULTIK_AGE_SECRET_KEY environment variable to be set with -the age private key. +Requires the age private key in the VAULTIK_AGE_SECRET_KEY environment +variable. The variable may hold the whole age-keygen file (comments and +all of its identities are accepted); read it from the file rather than +typing the key, so it does not land in your shell history: + + export VAULTIK_AGE_SECRET_KEY="$(cat vaultik_backup_private_key.txt)" Examples: # Restore entire snapshot diff --git a/internal/cli/snapshot_restore_test.go b/internal/cli/snapshot_restore_test.go new file mode 100644 index 0000000..d78a837 --- /dev/null +++ b/internal/cli/snapshot_restore_test.go @@ -0,0 +1,37 @@ +package cli //nolint:testpackage // exercises the unexported command constructor + +import ( + "strings" + "testing" + + "github.com/spf13/pflag" +) + +// TestRestoreCommandDoesNotTakeKeyAsArgument guards the fix for the age +// key being echoed on the command line: restore must take the key only +// from the environment, never as a flag value, and its help must show the +// file-based form rather than a literal key that would land in shell +// history. +func TestRestoreCommandDoesNotTakeKeyAsArgument(t *testing.T) { + t.Parallel() + + cmd := newSnapshotRestoreCommand() + + cmd.Flags().VisitAll(func(f *pflag.Flag) { + lower := strings.ToLower(f.Name) + for _, banned := range []string{"key", "secret", "age", "identity"} { + if strings.Contains(lower, banned) { + t.Errorf("restore must not accept the key as a flag; found --%s", f.Name) + } + } + }) + + help := cmd.Long + if strings.Contains(help, "AGE-SECRET-KEY-") { + t.Error("restore help must not show a literal age private key to type") + } + + if !strings.Contains(help, "$(cat ") { + t.Error("restore help should read the key from a file, e.g. $(cat ...)") + } +} diff --git a/internal/config/config.go b/internal/config/config.go index a5c5621..2608d9d 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -135,6 +135,26 @@ func (c *Config) SnapshotNames() []string { return names } +// Names of the two places the age secret key can be configured, used by +// AgeSecretKeySourceName for error messages that must not echo the value. +// +//nolint:gosec // G101: these are the names of the config sources, not a key +const ( + ageSecretKeySourceEnv = "VAULTIK_AGE_SECRET_KEY" + ageSecretKeySourceConfig = "age_secret_key" +) + +// AgeSecretKeySourceName returns the human name of where AgeSecretKey was +// configured. A Config built directly (as in tests) has no recorded +// source, so it reports the config-file field name. +func (c *Config) AgeSecretKeySourceName() string { + if c.AgeSecretKeySource != "" { + return c.AgeSecretKeySource + } + + return ageSecretKeySourceConfig +} + // Config represents the application configuration for Vaultik. // It defines all settings for backup operations, including source directories, // encryption recipients, storage configuration, and performance tuning parameters. @@ -144,8 +164,13 @@ func (c *Config) SnapshotNames() []string { type Config struct { AgeRecipients []string `yaml:"age_recipients"` AgeSecretKey string `yaml:"age_secret_key"` - BlobSizeLimit Size `yaml:"blob_size_limit"` - ChunkSize Size `yaml:"chunk_size"` + // AgeSecretKeySource names where AgeSecretKey was configured + // ("VAULTIK_AGE_SECRET_KEY" or "age_secret_key") so a later parse + // failure can name the source without echoing the secret value. It is + // set by Load and never read from or written to the config file. + AgeSecretKeySource string `yaml:"-"` + BlobSizeLimit Size `yaml:"blob_size_limit"` + ChunkSize Size `yaml:"chunk_size"` // Exclude holds global excludes applied to all snapshots. Exclude []string `yaml:"exclude"` Hostname string `yaml:"hostname"` @@ -254,10 +279,7 @@ func Load(path string) (*Config, error) { cfg.IndexPath = expandTilde(envIndexPath) } - // Check for environment variable override for AgeSecretKey - if envAgeSecretKey := os.Getenv("VAULTIK_AGE_SECRET_KEY"); envAgeSecretKey != "" { - cfg.AgeSecretKey = extractAgeSecretKey(envAgeSecretKey) - } + cfg.setAgeSecretKey() // Get hostname if not set if cfg.Hostname == "" { @@ -379,6 +401,21 @@ func validateAgeRecipient(recipient string) error { return nil } +// setAgeSecretKey records the age secret key and where it came from. The +// value is stored raw and parsed only where decryption happens +// (internal/vaultik), so backup, list and prune keep working whatever the +// field holds. The environment variable overrides the config-file field. +func (c *Config) setAgeSecretKey() { + if c.AgeSecretKey != "" { + c.AgeSecretKeySource = ageSecretKeySourceConfig + } + + if env := os.Getenv("VAULTIK_AGE_SECRET_KEY"); env != "" { + c.AgeSecretKey = env + c.AgeSecretKeySource = ageSecretKeySourceEnv + } +} + // validateStorage validates storage configuration. // If StorageURL is set, it takes precedence. S3 URLs require credentials. // File URLs don't require any S3 configuration. @@ -435,22 +472,6 @@ func (c *Config) validateStorageURL() error { } } -// extractAgeSecretKey extracts the AGE-SECRET-KEY from the input using -// the age library's parser, which handles comments and whitespace. -func extractAgeSecretKey(input string) string { - identities, err := age.ParseIdentities(strings.NewReader(input)) - if err != nil || len(identities) == 0 { - // Fall back to trimmed input if parsing fails - return strings.TrimSpace(input) - } - // Return the string representation of the first identity - if id, ok := identities[0].(*age.X25519Identity); ok { - return id.String() - } - - return strings.TrimSpace(input) -} - // Module exports the config module for fx dependency injection. // It provides the Config type to other modules in the application. // diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 591ac86..44e9fe6 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -1,4 +1,4 @@ -package config //nolint:testpackage // exercises unexported extractAgeSecretKey +package config //nolint:testpackage // exercises unexported source constants import ( "errors" @@ -298,53 +298,31 @@ func TestValidateAgeRecipients(t *testing.T) { } } -// TestExtractAgeSecretKey tests extraction of AGE-SECRET-KEY from various inputs -func TestExtractAgeSecretKey(t *testing.T) { +// TestAgeSecretKeySourceName checks the name reported for the configured +// age secret key: the recorded source when Load set one, and the +// config-file field name for a Config built directly (as in tests). +func TestAgeSecretKeySourceName(t *testing.T) { t.Parallel() tests := []struct { - name string - input string - expected string + name string + source string + want string }{ { - name: "plain key", - input: testIntegrationAgePrivateKey, - expected: testIntegrationAgePrivateKey, + name: "unset defaults to config field", + source: "", + want: ageSecretKeySourceConfig, }, { - name: "key with trailing newline", - input: testIntegrationAgePrivateKey + "\n", - expected: testIntegrationAgePrivateKey, + name: "environment source", + source: ageSecretKeySourceEnv, + want: ageSecretKeySourceEnv, }, { - name: "full age-keygen output", - input: "# created: 2025-01-14T12:00:00Z\n" + - "# public key: " + testIntegrationAgePublicKey + "\n" + - testIntegrationAgePrivateKey + "\n", - expected: testIntegrationAgePrivateKey, - }, - { - name: "age-keygen output with extra blank lines", - input: "# created: 2025-01-14T12:00:00Z\n" + - "# public key: " + testIntegrationAgePublicKey + "\n\n" + - testIntegrationAgePrivateKey + "\n\n", - expected: testIntegrationAgePrivateKey, - }, - { - name: "key with leading whitespace", - input: " " + testIntegrationAgePrivateKey + " ", - expected: testIntegrationAgePrivateKey, - }, - { - name: "empty input", - input: "", - expected: "", - }, - { - name: "only comments", - input: "# this is a comment\n# another comment", - expected: "# this is a comment\n# another comment", + name: "config-file source", + source: ageSecretKeySourceConfig, + want: ageSecretKeySourceConfig, }, } @@ -352,10 +330,9 @@ func TestExtractAgeSecretKey(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - result := extractAgeSecretKey(tt.input) - if result != tt.expected { - t.Errorf("extractAgeSecretKey(%q) = %q, want %q", - tt.input, result, tt.expected) + cfg := &Config{AgeSecretKeySource: tt.source} + if got := cfg.AgeSecretKeySourceName(); got != tt.want { + t.Errorf("AgeSecretKeySourceName() = %q, want %q", got, tt.want) } }) } diff --git a/internal/vaultik/blob_fetch.go b/internal/vaultik/blob_fetch.go index 8f7c001..0ff34e8 100644 --- a/internal/vaultik/blob_fetch.go +++ b/internal/vaultik/blob_fetch.go @@ -74,14 +74,15 @@ func (h *hashVerifyReader) Close() error { // The hash is verified when the returned reader is closed (after fully reading). // This avoids buffering the entire blob in memory. func (v *Vaultik) FetchAndDecryptBlob( - ctx context.Context, blobHash string, expectedSize int64, identity age.Identity, + ctx context.Context, blobHash string, expectedSize int64, + identities ...age.Identity, ) (io.ReadCloser, error) { rc, _, err := v.FetchBlob(ctx, blobHash, expectedSize) if err != nil { return nil, err } - reader, err := blobgen.NewReader(rc, identity) + reader, err := blobgen.NewReader(rc, identities...) if err != nil { _ = rc.Close() diff --git a/internal/vaultik/restore.go b/internal/vaultik/restore.go index 9f7f198..ea603d4 100644 --- a/internal/vaultik/restore.go +++ b/internal/vaultik/restore.go @@ -30,8 +30,13 @@ var ( errDecryptionKeyRequired = errors.New( "decryption key required for restore\n\n" + "Set the VAULTIK_AGE_SECRET_KEY environment variable to your " + - "age private key:\n" + - " export VAULTIK_AGE_SECRET_KEY='AGE-SECRET-KEY-...'") + "age private key file:\n" + + " export VAULTIK_AGE_SECRET_KEY=\"$(cat vaultik_backup_private_key.txt)\"") + // errInvalidAgeSecretKey is returned when the configured key does not + // parse as any age identity. It names the source but never the value, + // which is secret, so the message is safe to print and log. + errInvalidAgeSecretKey = errors.New( + "configured age secret key holds no usable age identity") errBlobMissingFromIndex = errors.New("blob hash missing from blob index") errChunkNotInAnyBlob = errors.New("chunk not found in any blob") errBlobIDNotInHashIndex = errors.New("blob id missing from hash index") @@ -97,7 +102,7 @@ type RestoreResult struct { func (v *Vaultik) Restore(opts *RestoreOptions) error { startTime := time.Now() - identity, err := v.prepareRestoreIdentity() + identities, err := v.restoreIdentities() if err != nil { return err } @@ -111,7 +116,7 @@ func (v *Vaultik) Restore(opts *RestoreOptions) error { // Step 1: Download and decrypt the snapshot metadata database log.Info("Downloading snapshot metadata...") - tempDB, tempDir, err := v.downloadSnapshotDB(opts.SnapshotID, identity) + tempDB, tempDir, err := v.downloadSnapshotDB(opts.SnapshotID, identities) if err != nil { return fmt.Errorf("downloading snapshot database: %w", err) } @@ -160,7 +165,7 @@ func (v *Vaultik) Restore(opts *RestoreOptions) error { } // Step 5: Restore files - result, err := v.restoreAllFiles(files, repos, opts, identity, chunkToBlobMap) + result, err := v.restoreAllFiles(files, repos, opts, identities, chunkToBlobMap) if err != nil { return err } @@ -221,21 +226,28 @@ func (v *Vaultik) finishRestore( return nil } -// prepareRestoreIdentity validates that an age secret key is configured -// and parses it. -// -//nolint:ireturn // age.Identity is the decryption abstraction by design -func (v *Vaultik) prepareRestoreIdentity() (age.Identity, error) { +// restoreIdentities parses the configured age secret key once into every +// identity it contains. The value may be a single key line or a whole +// age-keygen file with several identities; all of them are returned so +// blobgen (via age.Decrypt) can read a blob encrypted to any of their +// recipients. This is the first step of both restore and deep verify, so +// a missing or unparseable key fails before anything is downloaded. The +// error names the configuration source but never the key value. +func (v *Vaultik) restoreIdentities() ([]age.Identity, error) { if v.Config.AgeSecretKey == "" { return nil, errDecryptionKeyRequired } - identity, err := age.ParseX25519Identity(v.Config.AgeSecretKey) + // age.ParseIdentities skips comment and blank lines and rejects a + // malformed key. Its error can quote the offending line, so it is not + // wrapped here — that would leak the secret into the message. + identities, err := age.ParseIdentities(strings.NewReader(v.Config.AgeSecretKey)) if err != nil { - return nil, fmt.Errorf("parsing age secret key: %w", err) + return nil, fmt.Errorf("%w (source: %s)", + errInvalidAgeSecretKey, v.Config.AgeSecretKeySourceName()) } - return identity, nil + return identities, nil } // restoreAllFiles processes files in blob-locality order: drain every @@ -248,7 +260,7 @@ func (v *Vaultik) restoreAllFiles( files []*database.File, repos *database.Repositories, opts *RestoreOptions, - identity age.Identity, + identities []age.Identity, chunkToBlobMap map[string]*database.BlobChunk, ) (*RestoreResult, error) { result := &RestoreResult{} @@ -302,7 +314,7 @@ func (v *Vaultik) restoreAllFiles( ctx: v.ctx, repos: repos, opts: opts, - identity: identity, + identities: identities, chunkToBlobMap: chunkToBlobMap, blobByHash: blobByHash, blobIDToHash: blobIDToHash, @@ -616,7 +628,7 @@ func (v *Vaultik) handleRestoreVerification( // for a remote-only snapshot) is used as-is, so a host with no local // index can restore the snapshots it can only see on the store. func (v *Vaultik) downloadSnapshotDB( - snapshotID string, identity age.Identity, + snapshotID string, identities []age.Identity, ) (*database.DB, string, error) { remoteKey, err := v.resolveSnapshotRemoteKey(snapshotID) if err != nil { @@ -643,7 +655,7 @@ func (v *Vaultik) downloadSnapshotDB( "size", ubytes(int64(len(encryptedData)))) // Decrypt and decompress using blobgen.Reader - blobReader, err := blobgen.NewReader(bytes.NewReader(encryptedData), identity) + blobReader, err := blobgen.NewReader(bytes.NewReader(encryptedData), identities...) if err != nil { return nil, "", fmt.Errorf("creating decryption reader: %w", err) } @@ -833,7 +845,7 @@ type restoreSession struct { ctx context.Context //nolint:containedctx // per-restore state by design repos *database.Repositories opts *RestoreOptions - identity age.Identity + identities []age.Identity chunkToBlobMap map[string]*database.BlobChunk blobByHash map[string]*database.Blob blobIDToHash map[string]string @@ -1195,7 +1207,7 @@ func (s *restoreSession) downloadBlobToCache( start := time.Now() t0 := time.Now() - rc, err := s.v.FetchAndDecryptBlob(s.ctx, blobHash, expectedSize, s.identity) + rc, err := s.v.FetchAndDecryptBlob(s.ctx, blobHash, expectedSize, s.identities...) fetchSetupDur := time.Since(t0) if err != nil { diff --git a/internal/vaultik/restore_identity_test.go b/internal/vaultik/restore_identity_test.go new file mode 100644 index 0000000..50fd70d --- /dev/null +++ b/internal/vaultik/restore_identity_test.go @@ -0,0 +1,108 @@ +package vaultik //nolint:testpackage // exercises unexported restoreIdentities + +import ( + "bytes" + "io" + "testing" + + "filippo.io/age" + "github.com/stretchr/testify/require" + "sneak.berlin/go/vaultik/internal/blobgen" + "sneak.berlin/go/vaultik/internal/config" +) + +// encryptBlobTo returns a blobgen blob of plaintext encrypted to exactly +// one recipient, so a decryptor succeeds only if it holds that recipient's +// identity. +func encryptBlobTo(t *testing.T, recipient string, plaintext []byte) []byte { + t.Helper() + + var buf bytes.Buffer + + writer, err := blobgen.NewWriter(&buf, 1, []string{recipient}) + require.NoError(t, err) + + _, err = writer.Write(plaintext) + require.NoError(t, err) + require.NoError(t, writer.Close()) + + return buf.Bytes() +} + +// decryptBlobWith reads a blob back through the identities and returns its +// plaintext. +func decryptBlobWith(t *testing.T, blob []byte, identities []age.Identity) []byte { + t.Helper() + + reader, err := blobgen.NewReader(bytes.NewReader(blob), identities...) + require.NoError(t, err) + + plaintext, err := io.ReadAll(reader) + require.NoError(t, err) + require.NoError(t, reader.Close()) + + return plaintext +} + +// TestRestoreIdentitiesAcceptsEveryIdentity proves a key file holding two +// identities yields both, so a blob encrypted only to the second +// recipient — the one the previous single-identity parse dropped — still +// decrypts. +func TestRestoreIdentitiesAcceptsEveryIdentity(t *testing.T) { + t.Parallel() + + first, err := age.GenerateX25519Identity() + require.NoError(t, err) + + second, err := age.GenerateX25519Identity() + require.NoError(t, err) + + // A whole age-keygen-style file: comment lines plus two identity lines. + keyFile := "# public key: " + first.Recipient().String() + "\n" + + first.String() + "\n" + + "# public key: " + second.Recipient().String() + "\n" + + second.String() + "\n" + + v := &Vaultik{Config: &config.Config{AgeSecretKey: keyFile}} + + identities, err := v.restoreIdentities() + require.NoError(t, err) + require.Len(t, identities, 2) + + plaintext := []byte("payload encrypted only to the second identity") + blob := encryptBlobTo(t, second.Recipient().String(), plaintext) + + require.Equal(t, plaintext, decryptBlobWith(t, blob, identities)) +} + +// TestRestoreIdentitiesAcceptsTrailingNewline mirrors a YAML +// age_secret_key value that carries a trailing newline: it must still +// parse to its one identity and decrypt a blob encrypted to it. +func TestRestoreIdentitiesAcceptsTrailingNewline(t *testing.T) { + t.Parallel() + + id, err := age.GenerateX25519Identity() + require.NoError(t, err) + + v := &Vaultik{Config: &config.Config{AgeSecretKey: id.String() + "\n"}} + + identities, err := v.restoreIdentities() + require.NoError(t, err) + require.Len(t, identities, 1) + + plaintext := []byte("value with a trailing newline") + blob := encryptBlobTo(t, id.Recipient().String(), plaintext) + + require.Equal(t, plaintext, decryptBlobWith(t, blob, identities)) +} + +// TestRestoreIdentitiesMissingKey reports the dedicated missing-key error +// rather than a parse failure, so the user is told to set the key. +func TestRestoreIdentitiesMissingKey(t *testing.T) { + t.Parallel() + + v := &Vaultik{Config: &config.Config{}} + + _, err := v.restoreIdentities() + require.ErrorIs(t, err, errDecryptionKeyRequired) +} diff --git a/internal/vaultik/restore_malformed_key_test.go b/internal/vaultik/restore_malformed_key_test.go new file mode 100644 index 0000000..d8618e4 --- /dev/null +++ b/internal/vaultik/restore_malformed_key_test.go @@ -0,0 +1,44 @@ +package vaultik_test + +import ( + "context" + "io" + "testing" + + "github.com/stretchr/testify/require" + "sneak.berlin/go/vaultik/internal/config" + "sneak.berlin/go/vaultik/internal/ui" + "sneak.berlin/go/vaultik/internal/vaultik" +) + +// TestRestoreRejectsMalformedKeyBeforeDownload verifies that a malformed +// age secret key stops restore at the parse step: nothing is fetched from +// the store, and the error does not echo the key value (which is secret). +func TestRestoreRejectsMalformedKeyBeforeDownload(t *testing.T) { + t.Parallel() + + const malformed = "this-is-not-a-valid-age-key" + + mock := NewMockStorer() + + v := &vaultik.Vaultik{ + Config: &config.Config{AgeSecretKey: malformed}, + Storage: mock, + Stdout: io.Discard, + Stderr: io.Discard, + UI: ui.NewWithColor(io.Discard, false), + } + v.SetContext(context.Background()) + + err := v.Restore(&vaultik.RestoreOptions{ + SnapshotID: "any-snapshot", + TargetDir: t.TempDir(), + }) + require.Error(t, err) + require.NotContains(t, err.Error(), malformed, + "error must not echo the key value") + require.Contains(t, err.Error(), "age_secret_key", + "error should name the configuration source") + require.Empty(t, mock.GetCalls(), + "a malformed key must fail before anything is fetched") +} diff --git a/internal/vaultik/verify.go b/internal/vaultik/verify.go index 5df1105..6f95fd1 100644 --- a/internal/vaultik/verify.go +++ b/internal/vaultik/verify.go @@ -94,11 +94,10 @@ func (v *Vaultik) RunDeepVerify(snapshotID string, opts *VerifyOptions) error { } // Parse the age secret key once, the same way restore does, and reuse - // the identity for the database and every blob. - identity, err := v.prepareRestoreIdentity() + // the identities for the database and every blob. + identities, err := v.restoreIdentities() if err != nil { - return v.deepVerifyFailure(result, opts, - fmt.Sprintf("parsing age secret key: %v", err), err) + return v.deepVerifyFailure(result, opts, err.Error(), err) } log.Info("Starting snapshot verification", "snapshot_id", snapshotID, "mode", "deep") @@ -108,7 +107,7 @@ func (v *Vaultik) RunDeepVerify(snapshotID string, opts *VerifyOptions) error { } manifest, tempDB, dbBlobs, err := v.loadVerificationData( - snapshotID, opts, result, identity) + snapshotID, opts, result, identities) if err != nil { return err } @@ -129,7 +128,7 @@ func (v *Vaultik) RunDeepVerify(snapshotID string, opts *VerifyOptions) error { result.TotalSize = totalSize err = v.runVerificationSteps( - manifest, dbBlobs, tempDB, opts, result, totalSize, identity) + manifest, dbBlobs, tempDB, opts, result, totalSize, identities) if err != nil { return err } @@ -154,7 +153,7 @@ func (v *Vaultik) RunDeepVerify(snapshotID string, opts *VerifyOptions) error { // loadVerificationData downloads manifest, database, and blob list for verification func (v *Vaultik) loadVerificationData( snapshotID string, opts *VerifyOptions, result *VerifyResult, - identity age.Identity, + identities []age.Identity, ) (*snapshot.Manifest, *tempDB, []snapshot.BlobInfo, error) { // Resolve the identifier to the snapshot's remote key. A human ID is // hashed; a remote key (or its abbreviation, as printed for a @@ -191,7 +190,8 @@ func (v *Vaultik) loadVerificationData( v.stdoutf("Downloading and decrypting database...\n") } - tdb, err := v.downloadVerifiedSnapshotDB(snapshotID, remoteKey, opts, result, identity) + tdb, err := v.downloadVerifiedSnapshotDB( + snapshotID, remoteKey, opts, result, identities) if err != nil { return nil, nil, nil, err } @@ -230,7 +230,7 @@ func (v *Vaultik) loadVerificationData( // identity so nothing is left on disk. func (v *Vaultik) downloadVerifiedSnapshotDB( snapshotID, remoteKey string, opts *VerifyOptions, result *VerifyResult, - identity age.Identity, + identities []age.Identity, ) (*tempDB, error) { dbPath := fmt.Sprintf("metadata/%s/db.zst.age", remoteKey) log.Info("Downloading encrypted database", "path", dbPath) @@ -244,7 +244,7 @@ func (v *Vaultik) downloadVerifiedSnapshotDB( defer func() { _ = dbReader.Close() }() - tdb, err := v.decryptAndLoadDatabase(dbReader, identity) + tdb, err := v.decryptAndLoadDatabase(dbReader, identities) if err != nil { return nil, v.deepVerifyFailure(result, opts, fmt.Sprintf("failed to decrypt database: %v", err), @@ -270,7 +270,7 @@ func (v *Vaultik) runVerificationSteps( opts *VerifyOptions, result *VerifyResult, totalSize int64, - identity age.Identity, + identities []age.Identity, ) error { if !opts.JSON { v.stdoutf("Verifying manifest against database...\n") @@ -297,7 +297,7 @@ func (v *Vaultik) runVerificationSteps( len(dbBlobs), ubytes(totalSize)) } - err = v.performDeepVerificationFromDB(dbBlobs, tdb.db.Conn(), opts, identity) + err = v.performDeepVerificationFromDB(dbBlobs, tdb.db.Conn(), opts, identities) if err != nil { return v.deepVerifyFailure(result, opts, err.Error(), err) } @@ -325,10 +325,10 @@ func (t *tempDB) Close() error { // from the encrypted stream. It reads through the same blobgen reader restore // uses, streaming the decrypted, decompressed database to a temp file. func (v *Vaultik) decryptAndLoadDatabase( - reader io.ReadCloser, identity age.Identity, + reader io.ReadCloser, identities []age.Identity, ) (*tempDB, error) { // Decrypt and decompress through the shared blobgen reader. - blobReader, err := blobgen.NewReader(reader, identity) + blobReader, err := blobgen.NewReader(reader, identities...) if err != nil { return nil, fmt.Errorf("failed to create decryption reader: %w", err) } @@ -389,7 +389,7 @@ func (v *Vaultik) decryptAndLoadDatabase( // verifyBlob downloads and verifies a single blob func (v *Vaultik) verifyBlob( - blobInfo snapshot.BlobInfo, db *sql.DB, identity age.Identity, + blobInfo snapshot.BlobInfo, db *sql.DB, identities []age.Identity, ) error { // Download blob using shared fetch method reader, _, err := v.FetchBlob(v.ctx, blobInfo.Hash, blobInfo.CompressedSize) @@ -403,7 +403,7 @@ func (v *Vaultik) verifyBlob( // the plaintext as it is read. A blob's hash — its remote name — is the // double SHA-256 of that plaintext (see blobgen.DoubleSHA256), not of the // encrypted bytes. - blobReader, err := blobgen.NewReader(reader, identity) + blobReader, err := blobgen.NewReader(reader, identities...) if err != nil { return fmt.Errorf("failed to create blob reader: %w", err) } @@ -687,7 +687,7 @@ func (v *Vaultik) verifyBlobExistenceFromDB(blobs []snapshot.BlobInfo) error { // each blob using the database as source. func (v *Vaultik) performDeepVerificationFromDB( blobs []snapshot.BlobInfo, db *sql.DB, opts *VerifyOptions, - identity age.Identity, + identities []age.Identity, ) error { // Calculate total bytes for ETA var totalBytesExpected int64 @@ -705,7 +705,7 @@ func (v *Vaultik) performDeepVerificationFromDB( for i, blobInfo := range blobs { // Verify individual blob - err := v.verifyBlob(blobInfo, db, identity) + err := v.verifyBlob(blobInfo, db, identities) if err != nil { return fmt.Errorf("blob %s verification failed: %w", blobInfo.Hash, err) }