From e0e43548b70c11cfc04eb6aaacbf7a11b7e2558b Mon Sep 17 00:00:00 2001 From: sneak Date: Tue, 22 Sep 2026 10:41:59 +0000 Subject: [PATCH] Parse age_recipients at config load and never echo the entry (closes #153) Config.Validate now parses every age_recipients entry with age.ParseX25519Recipient, so a bad recipient fails at config load instead of deep in a backup after the snapshot row and tree walk. On failure the error names the position (age_recipients[N]) and never the value: a recipient string can itself be a secret key an operator pasted by mistake, and age's own error quotes its input. An entry starting with AGE-SECRET-KEY- (compared case-insensitively) gets a specific message. The remaining parse sites (blobgen.NewWriter, crypto NewEncryptor and UpdateRecipients), reachable by callers that skip config.Load, likewise drop the value and age's wrapped error, naming only the position. test/config.yaml's placeholder second recipient is replaced with a valid X25519 key so it still loads. Model: opus-4-8 --- internal/blobgen/writer.go | 11 ++++- internal/blobgen/writer_test.go | 17 +++++++ internal/config/config.go | 39 ++++++++++++++- internal/config/config_test.go | 78 ++++++++++++++++++++++++++++++ internal/crypto/encryption.go | 17 +++++-- internal/crypto/encryption_test.go | 20 ++++++++ test/config.yaml | 2 +- 7 files changed, 176 insertions(+), 8 deletions(-) diff --git a/internal/blobgen/writer.go b/internal/blobgen/writer.go index 8305046..8525df9 100644 --- a/internal/blobgen/writer.go +++ b/internal/blobgen/writer.go @@ -27,6 +27,11 @@ const reservedCompressionCPUs = 2 var ErrInvalidCompressionLevel = errors.New( "invalid compression level: must be between 1 and 19") +// errInvalidRecipient is returned when a recipient string does not parse as +// an X25519 age1... public key. It omits the value, which can be sensitive. +var errInvalidRecipient = errors.New( + "not a valid X25519 age1... recipient") + // Writer wraps compression and encryption with SHA256 hashing. // Data flows: input -> tee(hasher, compressor -> encryptor -> destination) // The hash is computed on the uncompressed input for deterministic content-addressing. @@ -57,10 +62,12 @@ func NewWriter( // Parse recipients var ageRecipients []age.Recipient - for _, recipient := range recipients { + for i, recipient := range recipients { + // The recipient string can be sensitive (e.g. a secret key pasted by + // mistake), so the error names its position, never its value. r, err := age.ParseX25519Recipient(recipient) if err != nil { - return nil, fmt.Errorf("parsing recipient %s: %w", recipient, err) + return nil, fmt.Errorf("%w: recipient %d", errInvalidRecipient, i) } ageRecipients = append(ageRecipients, r) diff --git a/internal/blobgen/writer_test.go b/internal/blobgen/writer_test.go index e218f52..8ea56fa 100644 --- a/internal/blobgen/writer_test.go +++ b/internal/blobgen/writer_test.go @@ -108,3 +108,20 @@ func TestWriterDeterministicHash(t *testing.T) { t.Logf("Encrypted size 1: %d bytes", buf1.Len()) t.Logf("Encrypted size 2: %d bytes", buf2.Len()) } + +// TestNewWriterSecretKeyNotEchoed verifies that a secret key mistakenly passed +// as a recipient does not appear in the returned error. A recipient string can +// be sensitive, so the error must name only the position, not the value. +func TestNewWriterSecretKeyNotEchoed(t *testing.T) { + t.Parallel() + + secretKey := "AGE-SECRET-KEY-19CR5YSFW59HM4TLD6GX" + + "VEDMZFTVVF7PPHKUT68TXSFPK7APHXA2QS2NJA5" + + var buf bytes.Buffer + + _, err := blobgen.NewWriter(&buf, 3, []string{secretKey}) + require.Error(t, err) + assert.NotContains(t, err.Error(), secretKey, + "error must not echo the recipient value") +} diff --git a/internal/config/config.go b/internal/config/config.go index a103192..a5c5621 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -22,6 +22,13 @@ import ( const appName = "vaultik" +// secretKeyPrefix marks an age secret (private) key. It is compared +// case-insensitively so a recipient entry that is actually a private key is +// caught and never passed to age or echoed back. +// +//nolint:gosec // G101: marker for detecting a pasted secret key, not a credential +const secretKeyPrefix = "AGE-SECRET-KEY-" + // Defaults and validation bounds for tunable settings. const ( defaultBlobSizeLimit = Size(10 * 1024 * 1024 * 1024) // 10GB @@ -38,6 +45,10 @@ var ( errNoConfigPath = errors.New("config path not provided") errNoAgeRecipients = errors.New( "at least one age_recipient is required (generate with: age-keygen)") + errRecipientIsSecretKey = errors.New( + "an age secret key was given where a public key (age1...) belongs") + errRecipientNotX25519 = errors.New( + "not a valid recipient; only X25519 age1... public keys are supported") errNoSnapshots = errors.New( "at least one snapshot must be configured (see config.example.yml)") errSnapshotNoPaths = errors.New("snapshot must have at least one path") @@ -290,7 +301,9 @@ func Load(path string) (*Config, error) { // Validate checks if the configuration is valid and complete. // It ensures all required fields are present and have valid values: -// - At least one age recipient must be specified +// - At least one age recipient must be specified, and every recipient must +// parse as an X25519 age1... public key (so a bad entry fails at load, not +// mid-backup); errors name the position, never the value // - At least one snapshot must be configured with at least one path // - Storage must be configured (either storage_url or s3.* fields) // - Chunk size must be at least 1MB @@ -305,6 +318,13 @@ func (c *Config) Validate() error { return errNoAgeRecipients } + for i, recipient := range c.AgeRecipients { + err := validateAgeRecipient(recipient) + if err != nil { + return fmt.Errorf("age_recipients[%d]: %w", i, err) + } + } + if len(c.Snapshots) == 0 { return errNoSnapshots } @@ -342,6 +362,23 @@ func (c *Config) Validate() error { return nil } +// validateAgeRecipient parses one age_recipients entry with the age library +// and returns a value-free error on failure. A recipient string can be +// sensitive (an operator may paste a secret key by mistake), so neither the +// entry nor age's own error (which quotes its input) is ever included. +func validateAgeRecipient(recipient string) error { + if strings.HasPrefix(strings.ToUpper(recipient), secretKeyPrefix) { + return errRecipientIsSecretKey + } + + _, err := age.ParseX25519Recipient(recipient) + if err != nil { + return errRecipientNotX25519 + } + + return nil +} + // validateStorage validates storage configuration. // If StorageURL is set, it takes precedence. S3 URLs require credentials. // File URLs don't require any S3 configuration. diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 3231175..823a5bc 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -4,6 +4,7 @@ import ( "errors" "os" "path/filepath" + "strings" "testing" "sneak.berlin/go/vaultik/internal/chunker" @@ -178,6 +179,83 @@ func TestValidateBlobSizeLimit(t *testing.T) { } } +// TestValidateAgeRecipients checks that recipients are parsed at config load +// (a bad entry fails immediately, not mid-backup) and that no invalid entry — +// least of all a pasted secret key — is echoed in the error. +func TestValidateAgeRecipients(t *testing.T) { + t.Parallel() + + baseConfig := func(recipients []string) *Config { + return &Config{ + AgeRecipients: recipients, + Snapshots: map[string]SnapshotConfig{"test": {Paths: []string{"/tmp/src"}}}, + StorageURL: "file:///tmp/vaultik-test-store", + ChunkSize: Size(10 * 1024 * 1024), + BlobSizeLimit: Size(10 * 1024 * 1024 * 1024), + CompressionLevel: 3, + } + } + + tests := []struct { + name string + recipients []string + wantErr bool + }{ + { + name: "config init placeholder is rejected", + recipients: []string{"age1REPLACE_WITH_YOUR_PUBLIC_KEY"}, + wantErr: true, + }, + { + name: "ssh-ed25519 recipient is rejected", + recipients: []string{"ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIexamplekeydata"}, + wantErr: true, + }, + { + name: "truncated age1 string is rejected", + recipients: []string{"age1short"}, + wantErr: true, + }, + { + name: "secret key passed as recipient is rejected", + recipients: []string{testIntegrationAgePrivateKey}, + wantErr: true, + }, + { + name: "two valid recipients are accepted", + recipients: []string{testSneakAgePublicKey, testIntegrationAgePublicKey}, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + err := baseConfig(tt.recipients).Validate() + if !tt.wantErr { + if err != nil { + t.Fatalf("Validate() unexpected error: %v", err) + } + + return + } + + if err == nil { + t.Fatal("Validate() returned nil, want error") + } + + // The entry itself must never appear in the error, since a + // recipient string can be a secret key. + for _, recipient := range tt.recipients { + if strings.Contains(err.Error(), recipient) { + t.Fatalf("Validate() error echoed the recipient value: %v", err) + } + } + }) + } +} + // TestExtractAgeSecretKey tests extraction of AGE-SECRET-KEY from various inputs func TestExtractAgeSecretKey(t *testing.T) { t.Parallel() diff --git a/internal/crypto/encryption.go b/internal/crypto/encryption.go index 04f36dd..dd5d71d 100644 --- a/internal/crypto/encryption.go +++ b/internal/crypto/encryption.go @@ -17,6 +17,11 @@ import ( // without any recipient public keys. var ErrNoRecipients = errors.New("at least one recipient is required") +// errInvalidRecipient is returned when a recipient string does not parse as +// an X25519 age1... public key. It omits the value, which can be sensitive. +var errInvalidRecipient = errors.New( + "not a valid X25519 age1... recipient") + // Encryptor provides thread-safe encryption using the age encryption library. // It supports encrypting data for multiple recipients simultaneously, allowing // any of the corresponding private keys to decrypt the data. This is useful @@ -36,10 +41,12 @@ func NewEncryptor(publicKeys []string) (*Encryptor, error) { } recipients := make([]age.Recipient, 0, len(publicKeys)) - for _, key := range publicKeys { + for i, key := range publicKeys { + // The key string can be sensitive (e.g. a secret key pasted by + // mistake), so the error names its position, never its value. recipient, err := age.ParseX25519Recipient(key) if err != nil { - return nil, fmt.Errorf("parsing age recipient %s: %w", key, err) + return nil, fmt.Errorf("%w: recipient %d", errInvalidRecipient, i) } recipients = append(recipients, recipient) @@ -142,10 +149,12 @@ func (e *Encryptor) UpdateRecipients(publicKeys []string) error { } recipients := make([]age.Recipient, 0, len(publicKeys)) - for _, key := range publicKeys { + for i, key := range publicKeys { + // The key string can be sensitive (e.g. a secret key pasted by + // mistake), so the error names its position, never its value. recipient, err := age.ParseX25519Recipient(key) if err != nil { - return fmt.Errorf("parsing age recipient %s: %w", key, err) + return fmt.Errorf("%w: recipient %d", errInvalidRecipient, i) } recipients = append(recipients, recipient) diff --git a/internal/crypto/encryption_test.go b/internal/crypto/encryption_test.go index 3f24d47..90a9784 100644 --- a/internal/crypto/encryption_test.go +++ b/internal/crypto/encryption_test.go @@ -2,6 +2,7 @@ package crypto_test import ( "bytes" + "strings" "testing" "filippo.io/age" @@ -176,3 +177,22 @@ func TestEncryptorUpdateRecipients(t *testing.T) { t.Error("should not decrypt with identity1") } } + +// TestNewEncryptorSecretKeyNotEchoed verifies that a secret key mistakenly +// passed as a recipient does not appear in the returned error. A recipient +// string can be sensitive, so the error must name only the position. +func TestNewEncryptorSecretKeyNotEchoed(t *testing.T) { + t.Parallel() + + secretKey := "AGE-SECRET-KEY-19CR5YSFW59HM4TLD6GX" + + "VEDMZFTVVF7PPHKUT68TXSFPK7APHXA2QS2NJA5" + + _, err := crypto.NewEncryptor([]string{secretKey}) + if err == nil { + t.Fatal("NewEncryptor returned nil, want error") + } + + if strings.Contains(err.Error(), secretKey) { + t.Fatalf("error echoed the recipient value: %v", err) + } +} diff --git a/test/config.yaml b/test/config.yaml index 07804ae..f67577f 100644 --- a/test/config.yaml +++ b/test/config.yaml @@ -1,6 +1,6 @@ age_recipients: - age1278m9q7dp3chsh2dcy82qk27v047zywyvtxwnj4cvt0z65jw6a7q5dqhfj # sneak's long term age key - - age1otherpubkey... # add additional recipients as needed + - age1ezrjmfpwsc95svdg0y54mums3zevgzu0x0ecq2f7tp8a05gl0sjq9q9wjg # add additional recipients as needed snapshots: test: paths: