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- 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.

Model: opus-4-8

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)

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")
+}

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.

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

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)

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)
+	}
+}