From 7165992b123901394e1c065b510f94bfa76e74a9 Mon Sep 17 00:00:00 2001 From: anonpenguin23 Date: Mon, 15 Jun 2026 13:54:35 +0300 Subject: [PATCH] =?UTF-8?q?fix(serverless):=20get=5Fsecret=20returns=20emp?= =?UTF-8?q?ty=20=E2=80=94=20base64=20round-trip=20for=20stored=20secrets?= =?UTF-8?q?=20(#837)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second #837 root cause (the key-derivation fix was necessary but not sufficient): DBSecretsManager stored the AES-GCM ciphertext as a raw []byte parameter, but the rqlite client serializes []byte as base64 and reads it back as that base64 TEXT — never the original bytes. So decrypt() always received base64 ASCII instead of ciphertext and failed, making get_secret return empty for every stored secret (exactly the reported symptom). Encode the ciphertext to an explicit base64 string in Set and decode it in Get (with a raw fallback), making the round-trip symmetric and driver-independent. The test mock now emulates rqlite's blob->base64-text behavior so it's a real regression guard. --- core/pkg/serverless/hostfunctions/secrets.go | 20 +++++++++++++++++-- .../serverless/hostfunctions/secrets_test.go | 16 ++++++++++++++- 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/core/pkg/serverless/hostfunctions/secrets.go b/core/pkg/serverless/hostfunctions/secrets.go index 5dce599..8ed506a 100644 --- a/core/pkg/serverless/hostfunctions/secrets.go +++ b/core/pkg/serverless/hostfunctions/secrets.go @@ -5,6 +5,7 @@ import ( "crypto/aes" "crypto/cipher" "crypto/rand" + "encoding/base64" "encoding/hex" "fmt" "time" @@ -72,6 +73,13 @@ func (s *DBSecretsManager) Set(ctx context.Context, namespace, name, value strin return fmt.Errorf("failed to encrypt secret: %w", err) } + // Store the ciphertext as an EXPLICIT base64 string (bugboard #837): the + // rqlite client serializes a raw []byte parameter as base64 and reads it + // back as that base64 TEXT — not the original bytes — so a raw-blob write + // round-tripped into base64 that decrypt() could never open. Encoding here + // (and decoding in Get) makes the round-trip deterministic and symmetric. + encoded := base64.StdEncoding.EncodeToString(encrypted) + // Upsert the secret query := ` INSERT INTO function_secrets (id, namespace, name, encrypted_value, created_at, updated_at) @@ -83,7 +91,7 @@ func (s *DBSecretsManager) Set(ctx context.Context, namespace, name, value strin id := fmt.Sprintf("%s:%s", namespace, name) now := time.Now() - if _, err := s.db.Exec(ctx, query, id, namespace, name, encrypted, now, now); err != nil { + if _, err := s.db.Exec(ctx, query, id, namespace, name, encoded, now, now); err != nil { return fmt.Errorf("failed to save secret: %w", err) } @@ -105,7 +113,15 @@ func (s *DBSecretsManager) Get(ctx context.Context, namespace, name string) (str return "", serverless.ErrSecretNotFound } - decrypted, err := s.decrypt(rows[0].EncryptedValue) + // Decode the base64 wrapper written by Set. Fall back to the raw bytes for + // any value that isn't valid base64 (defensive — should not occur once all + // writes go through the encode path above). bugboard #837. + ciphertext, decErr := base64.StdEncoding.DecodeString(string(rows[0].EncryptedValue)) + if decErr != nil { + ciphertext = rows[0].EncryptedValue + } + + decrypted, err := s.decrypt(ciphertext) if err != nil { return "", fmt.Errorf("failed to decrypt secret: %w", err) } diff --git a/core/pkg/serverless/hostfunctions/secrets_test.go b/core/pkg/serverless/hostfunctions/secrets_test.go index 4ad1f70..2b3b3af 100644 --- a/core/pkg/serverless/hostfunctions/secrets_test.go +++ b/core/pkg/serverless/hostfunctions/secrets_test.go @@ -3,6 +3,7 @@ package hostfunctions import ( "context" "database/sql" + "encoding/base64" "errors" "strings" "testing" @@ -35,7 +36,20 @@ func (f *fakeSecretsDB) Exec(ctx context.Context, query string, args ...any) (sq if strings.Contains(query, "INSERT INTO function_secrets") { namespace, _ := args[1].(string) name, _ := args[2].(string) - enc, _ := args[3].([]byte) + // Emulate how real rqlite persists the param into the encrypted_value + // column (bugboard #837): a string is stored as text as-is, while a raw + // []byte param is base64-encoded and stored as text — and READ BACK as + // that base64 text, never the original bytes. The fix passes an explicit + // base64 string from Set and decodes it in Get, so the round-trip is + // symmetric. (A regression to a raw []byte Set + no-decode Get would + // fail this test: decrypt would receive base64 ASCII, not ciphertext.) + var enc []byte + switch v := args[3].(type) { + case string: + enc = []byte(v) + case []byte: + enc = []byte(base64.StdEncoding.EncodeToString(v)) + } cp := make([]byte, len(enc)) copy(cp, enc) f.store[storeKey(namespace, name)] = cp