From 508114d4843a6d05095f76ad162d1167329a4dc3 Mon Sep 17 00:00:00 2001 From: Zach <3724288+zedkipp@users.noreply.github.com> Date: Fri, 10 Apr 2026 09:34:11 -0600 Subject: [PATCH] feat: user secret database encryption (#24218) Add dbcrypt support for user secret values. When database encryption is enabled, secret values are transparently encrypted on write and decrypted on read through the existing dbcrypt store wrapper. - Wrap `CreateUserSecret`, `GetUserSecretByUserIDAndName`, `ListUserSecretsWithValues`, and `UpdateUserSecretByUserIDAndName` in enterprise/dbcrypt/dbcrypt.go. - Add rotate and decrypt support for user secrets in enterprise/dbcrypt/cliutil.go (`server dbcrypt rotate` and `server dbcrypt decrypt`). - Add internal tests covering encrypt-on-create, decrypt-on-read, re-encrypt-on-update, and plaintext passthrough when no cipher is configured. --- docs/admin/security/database-encryption.md | 1 + enterprise/cli/server_dbcrypt_test.go | 19 ++ enterprise/dbcrypt/cliutil.go | 58 ++++++ enterprise/dbcrypt/dbcrypt.go | 54 ++++++ enterprise/dbcrypt/dbcrypt_internal_test.go | 195 ++++++++++++++++++++ 5 files changed, 327 insertions(+) diff --git a/docs/admin/security/database-encryption.md b/docs/admin/security/database-encryption.md index ecdea90dba..7d6f0f4cbf 100644 --- a/docs/admin/security/database-encryption.md +++ b/docs/admin/security/database-encryption.md @@ -23,6 +23,7 @@ The following database fields are currently encrypted: - `external_auth_links.oauth_access_token` - `external_auth_links.oauth_refresh_token` - `crypto_keys.secret` +- `user_secrets.value` Additional database fields may be encrypted in the future. diff --git a/enterprise/cli/server_dbcrypt_test.go b/enterprise/cli/server_dbcrypt_test.go index b50b8c0c50..79a64ea1e3 100644 --- a/enterprise/cli/server_dbcrypt_test.go +++ b/enterprise/cli/server_dbcrypt_test.go @@ -197,6 +197,10 @@ func TestServerDBCrypt(t *testing.T) { gitAuthLinks, err := db.GetExternalAuthLinksByUserID(ctx, usr.ID) require.NoError(t, err, "failed to get git auth links for user %s", usr.ID) require.Empty(t, gitAuthLinks) + + userSecrets, err := db.ListUserSecretsWithValues(ctx, usr.ID) + require.NoError(t, err, "failed to get user secrets for user %s", usr.ID) + require.Empty(t, userSecrets) } // Validate that the key has been revoked in the database. @@ -242,6 +246,14 @@ func genData(t *testing.T, db database.Store) []database.User { OAuthRefreshToken: "refresh-" + usr.ID.String(), }) } + + _ = dbgen.UserSecret(t, db, database.UserSecret{ + UserID: usr.ID, + Name: "secret-" + usr.ID.String(), + Value: "value-" + usr.ID.String(), + EnvName: "", + FilePath: "", + }) users = append(users, usr) } } @@ -283,6 +295,13 @@ func requireEncryptedWithCipher(ctx context.Context, t *testing.T, db database.S require.Equal(t, c.HexDigest(), gal.OAuthAccessTokenKeyID.String) require.Equal(t, c.HexDigest(), gal.OAuthRefreshTokenKeyID.String) } + + userSecrets, err := db.ListUserSecretsWithValues(ctx, userID) + require.NoError(t, err, "failed to get user secrets for user %s", userID) + for _, s := range userSecrets { + requireEncryptedEquals(t, c, "value-"+userID.String(), s.Value) + require.Equal(t, c.HexDigest(), s.ValueKeyID.String) + } } // nullCipher is a dbcrypt.Cipher that does not encrypt or decrypt. diff --git a/enterprise/dbcrypt/cliutil.go b/enterprise/dbcrypt/cliutil.go index 803c3d643d..84a2a2344a 100644 --- a/enterprise/dbcrypt/cliutil.go +++ b/enterprise/dbcrypt/cliutil.go @@ -96,6 +96,34 @@ func Rotate(ctx context.Context, log slog.Logger, sqlDB *sql.DB, ciphers []Ciphe } log.Debug(ctx, "encrypted user chat provider key", slog.F("user_id", uid), slog.F("chat_provider_id", userProviderKey.ChatProviderID), slog.F("current", idx+1), slog.F("cipher", ciphers[0].HexDigest())) } + + userSecrets, err := cryptTx.ListUserSecretsWithValues(ctx, uid) + if err != nil { + return xerrors.Errorf("get user secrets for user %s: %w", uid, err) + } + for _, secret := range userSecrets { + if secret.ValueKeyID.Valid && secret.ValueKeyID.String == ciphers[0].HexDigest() { + log.Debug(ctx, "skipping user secret", slog.F("user_id", uid), slog.F("secret_name", secret.Name), slog.F("current", idx+1), slog.F("cipher", ciphers[0].HexDigest())) + continue + } + if _, err := cryptTx.UpdateUserSecretByUserIDAndName(ctx, database.UpdateUserSecretByUserIDAndNameParams{ + UserID: uid, + Name: secret.Name, + UpdateValue: true, + Value: secret.Value, + ValueKeyID: sql.NullString{}, // dbcrypt will re-encrypt + UpdateDescription: false, + Description: "", + UpdateEnvName: false, + EnvName: "", + UpdateFilePath: false, + FilePath: "", + }); err != nil { + return xerrors.Errorf("rotate user secret user_id=%s name=%s: %w", uid, secret.Name, err) + } + log.Debug(ctx, "rotated user secret", slog.F("user_id", uid), slog.F("secret_name", secret.Name), slog.F("current", idx+1), slog.F("cipher", ciphers[0].HexDigest())) + } + return nil }, &database.TxOptions{ Isolation: sql.LevelRepeatableRead, @@ -235,6 +263,34 @@ func Decrypt(ctx context.Context, log slog.Logger, sqlDB *sql.DB, ciphers []Ciph } log.Debug(ctx, "decrypted user chat provider key", slog.F("user_id", uid), slog.F("chat_provider_id", userProviderKey.ChatProviderID), slog.F("current", idx+1)) } + + userSecrets, err := tx.ListUserSecretsWithValues(ctx, uid) + if err != nil { + return xerrors.Errorf("get user secrets for user %s: %w", uid, err) + } + for _, secret := range userSecrets { + if !secret.ValueKeyID.Valid { + log.Debug(ctx, "skipping user secret", slog.F("user_id", uid), slog.F("secret_name", secret.Name), slog.F("current", idx+1)) + continue + } + if _, err := tx.UpdateUserSecretByUserIDAndName(ctx, database.UpdateUserSecretByUserIDAndNameParams{ + UserID: uid, + Name: secret.Name, + UpdateValue: true, + Value: secret.Value, + ValueKeyID: sql.NullString{}, // clear the key ID + UpdateDescription: false, + Description: "", + UpdateEnvName: false, + EnvName: "", + UpdateFilePath: false, + FilePath: "", + }); err != nil { + return xerrors.Errorf("decrypt user secret user_id=%s name=%s: %w", uid, secret.Name, err) + } + log.Debug(ctx, "decrypted user secret", slog.F("user_id", uid), slog.F("secret_name", secret.Name), slog.F("current", idx+1)) + } + return nil }, &database.TxOptions{ Isolation: sql.LevelRepeatableRead, @@ -292,6 +348,8 @@ DELETE FROM external_auth_links OR oauth_refresh_token_key_id IS NOT NULL; DELETE FROM user_chat_provider_keys WHERE api_key_key_id IS NOT NULL; +DELETE FROM user_secrets + WHERE value_key_id IS NOT NULL; UPDATE chat_providers SET api_key = '', api_key_key_id = NULL diff --git a/enterprise/dbcrypt/dbcrypt.go b/enterprise/dbcrypt/dbcrypt.go index 6779b02b8f..f2d2133d07 100644 --- a/enterprise/dbcrypt/dbcrypt.go +++ b/enterprise/dbcrypt/dbcrypt.go @@ -717,6 +717,60 @@ func (db *dbCrypt) UpsertMCPServerUserToken(ctx context.Context, params database return tok, nil } +func (db *dbCrypt) CreateUserSecret(ctx context.Context, params database.CreateUserSecretParams) (database.UserSecret, error) { + if err := db.encryptField(¶ms.Value, ¶ms.ValueKeyID); err != nil { + return database.UserSecret{}, err + } + secret, err := db.Store.CreateUserSecret(ctx, params) + if err != nil { + return database.UserSecret{}, err + } + if err := db.decryptField(&secret.Value, secret.ValueKeyID); err != nil { + return database.UserSecret{}, err + } + return secret, nil +} + +func (db *dbCrypt) GetUserSecretByUserIDAndName(ctx context.Context, arg database.GetUserSecretByUserIDAndNameParams) (database.UserSecret, error) { + secret, err := db.Store.GetUserSecretByUserIDAndName(ctx, arg) + if err != nil { + return database.UserSecret{}, err + } + if err := db.decryptField(&secret.Value, secret.ValueKeyID); err != nil { + return database.UserSecret{}, err + } + return secret, nil +} + +func (db *dbCrypt) ListUserSecretsWithValues(ctx context.Context, userID uuid.UUID) ([]database.UserSecret, error) { + secrets, err := db.Store.ListUserSecretsWithValues(ctx, userID) + if err != nil { + return nil, err + } + for i := range secrets { + if err := db.decryptField(&secrets[i].Value, secrets[i].ValueKeyID); err != nil { + return nil, err + } + } + return secrets, nil +} + +func (db *dbCrypt) UpdateUserSecretByUserIDAndName(ctx context.Context, arg database.UpdateUserSecretByUserIDAndNameParams) (database.UserSecret, error) { + if arg.UpdateValue { + if err := db.encryptField(&arg.Value, &arg.ValueKeyID); err != nil { + return database.UserSecret{}, err + } + } + secret, err := db.Store.UpdateUserSecretByUserIDAndName(ctx, arg) + if err != nil { + return database.UserSecret{}, err + } + if err := db.decryptField(&secret.Value, secret.ValueKeyID); err != nil { + return database.UserSecret{}, err + } + return secret, nil +} + func (db *dbCrypt) encryptField(field *string, digest *sql.NullString) error { // If no cipher is loaded, then we can't encrypt anything! if db.ciphers == nil || db.primaryCipherDigest == "" { diff --git a/enterprise/dbcrypt/dbcrypt_internal_test.go b/enterprise/dbcrypt/dbcrypt_internal_test.go index eb82fad168..abece5e7e7 100644 --- a/enterprise/dbcrypt/dbcrypt_internal_test.go +++ b/enterprise/dbcrypt/dbcrypt_internal_test.go @@ -1287,3 +1287,198 @@ func TestUserChatProviderKeys(t *testing.T) { requireEncryptedEquals(t, ciphers[0], rawKey.APIKey, updatedAPIKey) }) } + +func TestUserSecrets(t *testing.T) { + t.Parallel() + ctx := context.Background() + + const ( + //nolint:gosec // test credentials + initialValue = "super-secret-value-initial" + //nolint:gosec // test credentials + updatedValue = "super-secret-value-updated" + ) + + insertUserSecret := func( + t *testing.T, + crypt *dbCrypt, + ciphers []Cipher, + ) database.UserSecret { + t.Helper() + user := dbgen.User(t, crypt, database.User{}) + secret, err := crypt.CreateUserSecret(ctx, database.CreateUserSecretParams{ + ID: uuid.New(), + UserID: user.ID, + Name: "test-secret-" + uuid.NewString()[:8], + Value: initialValue, + }) + require.NoError(t, err) + require.Equal(t, initialValue, secret.Value) + if len(ciphers) > 0 { + require.Equal(t, ciphers[0].HexDigest(), secret.ValueKeyID.String) + } + return secret + } + + t.Run("CreateUserSecretEncryptsValue", func(t *testing.T) { + t.Parallel() + db, crypt, ciphers := setup(t) + secret := insertUserSecret(t, crypt, ciphers) + + // Reading through crypt should return plaintext. + got, err := crypt.GetUserSecretByUserIDAndName(ctx, database.GetUserSecretByUserIDAndNameParams{ + UserID: secret.UserID, + Name: secret.Name, + }) + require.NoError(t, err) + require.Equal(t, initialValue, got.Value) + + // Reading through raw DB should return encrypted value. + raw, err := db.GetUserSecretByUserIDAndName(ctx, database.GetUserSecretByUserIDAndNameParams{ + UserID: secret.UserID, + Name: secret.Name, + }) + require.NoError(t, err) + require.NotEqual(t, initialValue, raw.Value) + requireEncryptedEquals(t, ciphers[0], raw.Value, initialValue) + }) + + t.Run("ListUserSecretsWithValuesDecrypts", func(t *testing.T) { + t.Parallel() + _, crypt, ciphers := setup(t) + secret := insertUserSecret(t, crypt, ciphers) + + secrets, err := crypt.ListUserSecretsWithValues(ctx, secret.UserID) + require.NoError(t, err) + require.Len(t, secrets, 1) + require.Equal(t, initialValue, secrets[0].Value) + }) + + t.Run("UpdateUserSecretReEncryptsValue", func(t *testing.T) { + t.Parallel() + db, crypt, ciphers := setup(t) + secret := insertUserSecret(t, crypt, ciphers) + + updated, err := crypt.UpdateUserSecretByUserIDAndName(ctx, database.UpdateUserSecretByUserIDAndNameParams{ + UserID: secret.UserID, + Name: secret.Name, + UpdateValue: true, + Value: updatedValue, + ValueKeyID: sql.NullString{}, + }) + require.NoError(t, err) + require.Equal(t, updatedValue, updated.Value) + require.Equal(t, ciphers[0].HexDigest(), updated.ValueKeyID.String) + + // Raw DB should have new encrypted value. + raw, err := db.GetUserSecretByUserIDAndName(ctx, database.GetUserSecretByUserIDAndNameParams{ + UserID: secret.UserID, + Name: secret.Name, + }) + require.NoError(t, err) + require.NotEqual(t, updatedValue, raw.Value) + requireEncryptedEquals(t, ciphers[0], raw.Value, updatedValue) + }) + + t.Run("NoCipherStoresPlaintext", func(t *testing.T) { + t.Parallel() + db, crypt := setupNoCiphers(t) + user := dbgen.User(t, crypt, database.User{}) + + secret, err := crypt.CreateUserSecret(ctx, database.CreateUserSecretParams{ + ID: uuid.New(), + UserID: user.ID, + Name: "plaintext-secret", + Value: initialValue, + }) + require.NoError(t, err) + require.Equal(t, initialValue, secret.Value) + require.False(t, secret.ValueKeyID.Valid) + + // Raw DB should also have plaintext. + raw, err := db.GetUserSecretByUserIDAndName(ctx, database.GetUserSecretByUserIDAndNameParams{ + UserID: user.ID, + Name: "plaintext-secret", + }) + require.NoError(t, err) + require.Equal(t, initialValue, raw.Value) + require.False(t, raw.ValueKeyID.Valid) + }) + + t.Run("UpdateMetadataOnlySkipsEncryption", func(t *testing.T) { + t.Parallel() + db, crypt, ciphers := setup(t) + secret := insertUserSecret(t, crypt, ciphers) + + // Read the raw encrypted value from the database. + rawBefore, err := db.GetUserSecretByUserIDAndName(ctx, database.GetUserSecretByUserIDAndNameParams{ + UserID: secret.UserID, + Name: secret.Name, + }) + require.NoError(t, err) + + // Perform a metadata-only update (no value change). + updated, err := crypt.UpdateUserSecretByUserIDAndName(ctx, database.UpdateUserSecretByUserIDAndNameParams{ + UserID: secret.UserID, + Name: secret.Name, + UpdateValue: false, + Value: "", + ValueKeyID: sql.NullString{}, + UpdateDescription: true, + Description: "updated description", + UpdateEnvName: false, + EnvName: "", + UpdateFilePath: false, + FilePath: "", + }) + require.NoError(t, err) + require.Equal(t, "updated description", updated.Description) + require.Equal(t, initialValue, updated.Value) + + // Read the raw encrypted value again. + rawAfter, err := db.GetUserSecretByUserIDAndName(ctx, database.GetUserSecretByUserIDAndNameParams{ + UserID: secret.UserID, + Name: secret.Name, + }) + require.NoError(t, err) + require.Equal(t, rawBefore.Value, rawAfter.Value) + require.Equal(t, rawBefore.ValueKeyID, rawAfter.ValueKeyID) + }) + + t.Run("GetUserSecretDecryptErr", func(t *testing.T) { + t.Parallel() + db, crypt, ciphers := setup(t) + user := dbgen.User(t, db, database.User{}) + dbgen.UserSecret(t, db, database.UserSecret{ + UserID: user.ID, + Name: "corrupt-secret", + Value: fakeBase64RandomData(t, 32), + ValueKeyID: sql.NullString{String: ciphers[0].HexDigest(), Valid: true}, + }) + + _, err := crypt.GetUserSecretByUserIDAndName(ctx, database.GetUserSecretByUserIDAndNameParams{ + UserID: user.ID, + Name: "corrupt-secret", + }) + require.Error(t, err) + var derr *DecryptFailedError + require.ErrorAs(t, err, &derr) + }) + + t.Run("ListUserSecretsWithValuesDecryptErr", func(t *testing.T) { + t.Parallel() + db, crypt, ciphers := setup(t) + user := dbgen.User(t, db, database.User{}) + dbgen.UserSecret(t, db, database.UserSecret{ + UserID: user.ID, + Name: "corrupt-list-secret", + Value: fakeBase64RandomData(t, 32), + ValueKeyID: sql.NullString{String: ciphers[0].HexDigest(), Valid: true}, + }) + + _, err := crypt.ListUserSecretsWithValues(ctx, user.ID) + require.Error(t, err) + var derr *DecryptFailedError + require.ErrorAs(t, err, &derr) + }) +}