From dc634e863a9819bfa8752535c6331c424fc7db4e Mon Sep 17 00:00:00 2001 From: wizardchen Date: Sun, 17 May 2026 15:10:04 +0800 Subject: [PATCH] feat(crypto): AES-GCM at-rest for MCP and DataSource credentials with graceful key-rotation handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two encryption gaps and one fail-loud regression, fixed together because they are inseparable in practice. At-rest encryption gaps ----------------------- Before this change, only Tenant.APIKey, Tenant.Credentials.WeKnoraCloud.AppSecret, Model.Parameters.APIKey/AppSecret, and WebSearchProviderParameters.APIKey were encrypted with SYSTEM_AES_KEY. MCP and DataSource were missed: - MCPAuthConfig.Value/Scan went through bare json.Marshal/Unmarshal, so api_key and token were written to mcp_services.auth_config (json) in plaintext regardless of whether SYSTEM_AES_KEY was set. - DataSource.Config (types.JSON) is a byte passthrough, so connector secrets (Notion token, GitHub PAT, Feishu app_secret, ...) inside the Credentials map were stored in plaintext too. Now both follow the ModelParameters / WebSearchProviderParameters pattern. Encryption operates on a local copy to avoid mutating the caller's in-memory struct (otherwise the next read of the same *MCPAuthConfig would surface ciphertext as if it were the API key). For DataSource the ToJSON ↔ ParseConfig pair is the only application boundary that materializes credentials, so encrypting there is sufficient and avoids having to fork the generic types.JSON. Backward compatibility ---------------------- DecryptStoredSecret returns rows without the enc:v1: prefix verbatim, so historical plaintext records keep working. Next write re-saves them encrypted. No migration step. Graceful Scan-time degradation ------------------------------ The strict "fail loud on key rotation" behaviour of DecryptStoredSecret turned into a footgun once it ran from GORM Scan: a single broken row caused ListModels / ListMCPServices / ListDataSources to return zero results, hiding all other rows and giving the operator no way to find which credential needed re-entering. New utils.DecryptStoredSecretLenient: same logic, but on failure returns ("", false). The strict variant is kept for hot paths that are about to actually USE the credential (so a rotated key surfaces loudly there rather than silently degrading to "missing api key"). All Scan paths switch to lenient + per-failure log line: ModelParameters.Scan, WebSearchProviderParameters.Scan, MCPAuthConfig.Scan, DataSource.ParseConfig, CredentialsConfig.Scan. On lenient failure the field blanks out (never ciphertext), the row still loads, the DTO reports configured=false so the UI shows "not configured", and any upstream call attempted with the empty value fails with a clear "missing api key" rather than sending ciphertext as the key. Tenant.AfterFind (Tenant.APIKey) intentionally stays strict — it is the primary HTTP auth credential and a silent empty value would change auth semantics ambiguously. --- internal/types/datasource.go | 63 +++++++++++++- internal/types/datasource_test.go | 118 ++++++++++++++++++++++++++ internal/types/mcp.go | 50 ++++++++++- internal/types/mcp_test.go | 107 +++++++++++++++++++++++ internal/types/model.go | 23 +++-- internal/types/tenant.go | 10 ++- internal/types/web_search_provider.go | 11 +-- internal/utils/crypto.go | 48 +++++++++-- 8 files changed, 396 insertions(+), 34 deletions(-) create mode 100644 internal/types/datasource_test.go create mode 100644 internal/types/mcp_test.go diff --git a/internal/types/datasource.go b/internal/types/datasource.go index 9d78bed1..92e2c67f 100644 --- a/internal/types/datasource.go +++ b/internal/types/datasource.go @@ -2,8 +2,10 @@ package types import ( "encoding/json" + "log" "time" + "github.com/Tencent/WeKnora/internal/utils" "github.com/google/uuid" "gorm.io/gorm" ) @@ -344,12 +346,38 @@ type DataSourceSyncPayload struct { MaxItems int `json:"max_items,omitempty"` } -// ToJSON converts a value to JSON type +// ToJSON converts a DataSourceConfig to the JSON blob stored in +// DataSource.Config. +// +// When SYSTEM_AES_KEY is configured, every string value inside +// Credentials is AES-256-GCM encrypted before serialization. Non-string +// values (numbers, bools, nested objects) pass through untouched. This is +// the only write path through which credentials reach the DB (the GORM +// JSON type itself is a byte passthrough), so encrypting here is +// sufficient to keep DataSource.Config at rest fully encrypted. +// +// Encryption operates on a shallow copy of Credentials to avoid mutating +// the caller's in-memory map (subsequent reads would otherwise see +// ciphertext). func (d *DataSourceConfig) ToJSON() (JSON, error) { if d == nil { return nil, nil } - bytes, err := json.Marshal(d) + out := *d + if key := utils.GetAESKey(); key != nil && len(out.Credentials) > 0 { + encCreds := make(map[string]interface{}, len(out.Credentials)) + for k, v := range out.Credentials { + if s, ok := v.(string); ok && s != "" { + if enc, err := utils.EncryptAESGCM(s, key); err == nil { + encCreds[k] = enc + continue + } + } + encCreds[k] = v + } + out.Credentials = encCreds + } + bytes, err := json.Marshal(&out) if err != nil { return nil, err } @@ -380,7 +408,16 @@ func (r *SyncResult) ToJSON() (JSON, error) { return JSON(bytes), nil } -// ParseConfig parses the encrypted config JSON back to DataSourceConfig +// ParseConfig deserializes DataSource.Config and decrypts any encrypted +// Credentials entries. +// +// DecryptStoredSecret transparently handles three cases per credential: +// - empty string: untouched +// - legacy plaintext (no enc:v1: prefix): returned as-is, so historical +// rows continue to work without a migration step +// - enc:v1: encrypted: decrypted with SYSTEM_AES_KEY; missing/rotated +// key surfaces as an error so we fail loudly rather than handing +// ciphertext to the upstream connector as the credential func (d *DataSource) ParseConfig() (*DataSourceConfig, error) { if len(d.Config) == 0 { return nil, nil @@ -389,6 +426,26 @@ func (d *DataSource) ParseConfig() (*DataSourceConfig, error) { if err := json.Unmarshal(d.Config, &config); err != nil { return nil, err } + for k, v := range config.Credentials { + s, ok := v.(string) + if !ok || s == "" { + continue + } + if plain, ok := utils.DecryptStoredSecretLenient(s); ok { + config.Credentials[k] = plain + continue + } + // Same rationale as the other Scan paths: don't fail the load — + // blank the field so the row stays visible. ParseConfig callers + // then see an empty credential string; HasCredentials() returns + // false; the UI surfaces "credential not configured" and the + // user can re-enter without losing the rest of the data source. + log.Printf( + "[crypto] datasource credential %q: decrypt failed (SYSTEM_AES_KEY missing/rotated?), treating as unconfigured", + k, + ) + config.Credentials[k] = "" + } return &config, nil } diff --git a/internal/types/datasource_test.go b/internal/types/datasource_test.go new file mode 100644 index 00000000..17a7c2f0 --- /dev/null +++ b/internal/types/datasource_test.go @@ -0,0 +1,118 @@ +package types + +import ( + "encoding/json" + "strings" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestDataSourceConfig_ToJSON_EncryptsStringCredentials(t *testing.T) { + withAESKey(t, testAESKey32) + cfg := &DataSourceConfig{ + Type: "github", + Credentials: map[string]interface{}{ + "token": "ghp-real-secret", + "client_secret": "cs-real-secret", + "refresh_count": 42, // non-string, must pass through untouched + "refresh_enabled": true, // non-string, must pass through untouched + }, + Settings: map[string]interface{}{"branch": "main"}, + } + blob, err := cfg.ToJSON() + assert.NoError(t, err) + s := string(blob) + + // Plaintext secrets must not appear in the persisted JSON. + assert.NotContains(t, s, "ghp-real-secret") + assert.NotContains(t, s, "cs-real-secret") + + // String credentials are encrypted with the enc:v1: prefix. + var raw map[string]json.RawMessage + assert.NoError(t, json.Unmarshal(blob, &raw)) + var creds map[string]interface{} + assert.NoError(t, json.Unmarshal(raw["credentials"], &creds)) + assert.True(t, strings.HasPrefix(creds["token"].(string), "enc:v1:")) + assert.True(t, strings.HasPrefix(creds["client_secret"].(string), "enc:v1:")) + + // Non-string credentials survive verbatim (some connectors persist + // integers / bools alongside the secrets — refresh_count and friends). + assert.EqualValues(t, 42, creds["refresh_count"]) + assert.EqualValues(t, true, creds["refresh_enabled"]) + + // In-memory caller's map must not be mutated. + assert.Equal(t, "ghp-real-secret", cfg.Credentials["token"]) + assert.Equal(t, "cs-real-secret", cfg.Credentials["client_secret"]) +} + +func TestDataSourceConfig_ToJSON_PassthroughWhenNoKey(t *testing.T) { + withAESKey(t, "") + cfg := &DataSourceConfig{ + Credentials: map[string]interface{}{"token": "plain-token"}, + } + blob, err := cfg.ToJSON() + assert.NoError(t, err) + assert.Contains(t, string(blob), "plain-token") +} + +func TestDataSource_ParseConfig_Roundtrip(t *testing.T) { + withAESKey(t, testAESKey32) + original := &DataSourceConfig{ + Type: "notion", + Credentials: map[string]interface{}{ + "token": "ntn-roundtrip", + "team_id": "team-public-id", + }, + ResourceIDs: []string{"page-1"}, + } + blob, err := original.ToJSON() + assert.NoError(t, err) + + ds := &DataSource{Config: blob} + parsed, err := ds.ParseConfig() + assert.NoError(t, err) + assert.Equal(t, "ntn-roundtrip", parsed.Credentials["token"]) + assert.Equal(t, "team-public-id", parsed.Credentials["team_id"]) + assert.Equal(t, []string{"page-1"}, parsed.ResourceIDs) +} + +func TestDataSource_ParseConfig_LegacyPlaintext(t *testing.T) { + // Rows persisted before encryption was wired in have no enc:v1: prefix + // — DecryptStoredSecret returns them verbatim. Historical data must + // keep working without an offline migration. + withAESKey(t, testAESKey32) + legacy := JSON([]byte(`{"type":"github","credentials":{"token":"legacy-plain"}}`)) + ds := &DataSource{Config: legacy} + parsed, err := ds.ParseConfig() + assert.NoError(t, err) + assert.Equal(t, "legacy-plain", parsed.Credentials["token"]) +} + +func TestDataSource_ParseConfig_MissingKeyDegradesGracefully(t *testing.T) { + // Encrypt with key, drop the key, then load. ParseConfig must succeed + // so the data source row is still visible — the credential blanks + // out so HasCredentials() returns false and the UI shows + // "not configured". A loud Scan failure would break the whole list. + withAESKey(t, testAESKey32) + cfg := &DataSourceConfig{ + Type: "github", + Credentials: map[string]interface{}{ + "token": "secret", + "refresh_count": 42, // non-string survives regardless + }, + ResourceIDs: []string{"repo-1"}, + } + blob, err := cfg.ToJSON() + assert.NoError(t, err) + + withAESKey(t, "") + ds := &DataSource{Config: blob} + parsed, err := ds.ParseConfig() + assert.NoError(t, err) + assert.Equal(t, "", parsed.Credentials["token"], "encrypted credential must blank out, not leak ciphertext") + assert.EqualValues(t, 42, parsed.Credentials["refresh_count"], "non-string credentials unaffected") + assert.Equal(t, []string{"repo-1"}, parsed.ResourceIDs, "non-credential config survives") + assert.False(t, parsed.HasCredentials() && parsed.Credentials["token"] != "", + "HasCredentials should not report a usable credential when decrypt failed") +} diff --git a/internal/types/mcp.go b/internal/types/mcp.go index 20d9f6bc..e44e84dc 100644 --- a/internal/types/mcp.go +++ b/internal/types/mcp.go @@ -3,8 +3,10 @@ package types import ( "database/sql/driver" "encoding/json" + "log" "time" + "github.com/Tencent/WeKnora/internal/utils" "github.com/google/uuid" "gorm.io/gorm" ) @@ -144,15 +146,40 @@ func (h *MCPHeaders) Scan(value interface{}) error { return json.Unmarshal(b, h) } -// Value implements driver.Valuer interface for MCPAuthConfig +// Value implements driver.Valuer for MCPAuthConfig. +// +// When SYSTEM_AES_KEY is configured, APIKey and Token are AES-256-GCM +// encrypted before serialization — mirroring the ModelParameters / +// WebSearchProviderParameters pattern so MCP secrets are not the odd +// resource stored in plaintext. Encryption operates on a local copy to +// avoid mutating the caller's in-memory struct (subsequent reads of the +// same *MCPAuthConfig would otherwise see ciphertext). func (c *MCPAuthConfig) Value() (driver.Value, error) { if c == nil { return nil, nil } - return json.Marshal(c) + out := *c + if key := utils.GetAESKey(); key != nil { + if out.APIKey != "" { + if encrypted, err := utils.EncryptAESGCM(out.APIKey, key); err == nil { + out.APIKey = encrypted + } + } + if out.Token != "" { + if encrypted, err := utils.EncryptAESGCM(out.Token, key); err == nil { + out.Token = encrypted + } + } + } + return json.Marshal(&out) } -// Scan implements sql.Scanner interface for MCPAuthConfig +// Scan implements sql.Scanner for MCPAuthConfig. +// +// Legacy plaintext rows (no enc:v1: prefix) are returned as-is; encrypted +// rows are decrypted. DecryptStoredSecret fails loudly if the prefix is +// present but SYSTEM_AES_KEY is missing/rotated, so we surface that as a +// Scan error rather than letting ciphertext leak upstream as the API key. func (c *MCPAuthConfig) Scan(value interface{}) error { if value == nil { return nil @@ -161,7 +188,22 @@ func (c *MCPAuthConfig) Scan(value interface{}) error { if !ok { return nil } - return json.Unmarshal(b, c) + if err := json.Unmarshal(b, c); err != nil { + return err + } + if plain, ok := utils.DecryptStoredSecretLenient(c.APIKey); ok { + c.APIKey = plain + } else { + log.Printf("[crypto] mcp auth_config api_key: decrypt failed (SYSTEM_AES_KEY missing/rotated?), treating as unconfigured") + c.APIKey = "" + } + if plain, ok := utils.DecryptStoredSecretLenient(c.Token); ok { + c.Token = plain + } else { + log.Printf("[crypto] mcp auth_config token: decrypt failed (SYSTEM_AES_KEY missing/rotated?), treating as unconfigured") + c.Token = "" + } + return nil } // Value implements driver.Valuer interface for MCPAdvancedConfig diff --git a/internal/types/mcp_test.go b/internal/types/mcp_test.go new file mode 100644 index 00000000..49b86368 --- /dev/null +++ b/internal/types/mcp_test.go @@ -0,0 +1,107 @@ +package types + +import ( + "encoding/json" + "os" + "strings" + "testing" + + "github.com/stretchr/testify/assert" +) + +// 32-byte AES-256 key used across encryption round-trip tests. +const testAESKey32 = "0123456789abcdef0123456789abcdef" + +func withAESKey(t *testing.T, key string) { + t.Helper() + prev := os.Getenv("SYSTEM_AES_KEY") + t.Setenv("SYSTEM_AES_KEY", key) + t.Cleanup(func() { _ = os.Setenv("SYSTEM_AES_KEY", prev) }) +} + +func TestMCPAuthConfig_Value_EncryptsWhenKeySet(t *testing.T) { + withAESKey(t, testAESKey32) + cfg := &MCPAuthConfig{ + APIKey: "sk-real-secret", + Token: "bearer-real-secret", + } + v, err := cfg.Value() + assert.NoError(t, err) + raw, ok := v.([]byte) + assert.True(t, ok) + + // The persisted bytes must be ciphertext, never the plaintext value. + assert.NotContains(t, string(raw), "sk-real-secret") + assert.NotContains(t, string(raw), "bearer-real-secret") + + var stored MCPAuthConfig + assert.NoError(t, json.Unmarshal(raw, &stored)) + assert.True(t, strings.HasPrefix(stored.APIKey, "enc:v1:")) + assert.True(t, strings.HasPrefix(stored.Token, "enc:v1:")) + + // In-memory caller's struct must not be mutated — encryption operates + // on a local copy. Otherwise the next read of cfg.APIKey would surface + // ciphertext as if it were the API key. + assert.Equal(t, "sk-real-secret", cfg.APIKey) + assert.Equal(t, "bearer-real-secret", cfg.Token) +} + +func TestMCPAuthConfig_Value_PassthroughWhenNoKey(t *testing.T) { + withAESKey(t, "") + cfg := &MCPAuthConfig{APIKey: "plain-key", Token: "plain-tok"} + v, err := cfg.Value() + assert.NoError(t, err) + raw := v.([]byte) + // Without SYSTEM_AES_KEY we fall back to plaintext storage (matches + // the existing Model/WebSearch convention so deployments without a + // key don't break). + assert.Contains(t, string(raw), "plain-key") + assert.Contains(t, string(raw), "plain-tok") +} + +func TestMCPAuthConfig_ScanRoundtrip(t *testing.T) { + withAESKey(t, testAESKey32) + original := &MCPAuthConfig{ + APIKey: "sk-roundtrip", + Token: "tok-roundtrip", + CustomHeaders: map[string]string{"X-Trace": "abc"}, + } + v, err := original.Value() + assert.NoError(t, err) + + var scanned MCPAuthConfig + assert.NoError(t, scanned.Scan(v.([]byte))) + assert.Equal(t, "sk-roundtrip", scanned.APIKey) + assert.Equal(t, "tok-roundtrip", scanned.Token) + assert.Equal(t, "abc", scanned.CustomHeaders["X-Trace"]) +} + +func TestMCPAuthConfig_Scan_LegacyPlaintextPassesThrough(t *testing.T) { + // Rows written before encryption was enabled have no enc:v1: prefix — + // DecryptStoredSecret must return them unchanged so historical data + // continues to work without a migration. + withAESKey(t, testAESKey32) + legacy := []byte(`{"api_key":"legacy-plain","token":"legacy-plain-tok"}`) + var scanned MCPAuthConfig + assert.NoError(t, scanned.Scan(legacy)) + assert.Equal(t, "legacy-plain", scanned.APIKey) + assert.Equal(t, "legacy-plain-tok", scanned.Token) +} + +func TestMCPAuthConfig_Scan_MissingKeyDegradesGracefully(t *testing.T) { + // Encrypt with key set, drop the key, then try to load. The row must + // still Scan successfully (otherwise a single broken row would crash + // list endpoints and hide every MCP service from the operator). The + // secret blanks out so the UI shows "credential not configured" and + // no ciphertext is ever sent upstream as the API key. + withAESKey(t, testAESKey32) + v, err := (&MCPAuthConfig{APIKey: "x", Token: "y", CustomHeaders: map[string]string{"a": "b"}}).Value() + assert.NoError(t, err) + + withAESKey(t, "") + var scanned MCPAuthConfig + assert.NoError(t, scanned.Scan(v.([]byte))) + assert.Empty(t, scanned.APIKey, "encrypted api_key must blank out when key is missing, not leak ciphertext") + assert.Empty(t, scanned.Token, "encrypted token must blank out when key is missing") + assert.Equal(t, "b", scanned.CustomHeaders["a"], "non-secret fields must still load") +} diff --git a/internal/types/model.go b/internal/types/model.go index 0bbcc662..28752918 100644 --- a/internal/types/model.go +++ b/internal/types/model.go @@ -3,7 +3,7 @@ package types import ( "database/sql/driver" "encoding/json" - "fmt" + "log" "time" "github.com/Tencent/WeKnora/internal/utils" @@ -146,16 +146,21 @@ func (c *ModelParameters) Scan(value interface{}) error { if err := json.Unmarshal(b, c); err != nil { return err } - apiKey, err := utils.DecryptStoredSecret(c.APIKey) - if err != nil { - return fmt.Errorf("decrypt model parameters api_key: %w", err) + // Lenient decrypt: a row with broken ciphertext (key rotated/removed) + // must still load — otherwise a single failure breaks ListModels and + // the user can't even see which model needs re-credentialing. + if plain, ok := utils.DecryptStoredSecretLenient(c.APIKey); ok { + c.APIKey = plain + } else { + log.Printf("[crypto] model parameters api_key: decrypt failed (SYSTEM_AES_KEY missing/rotated?), treating as unconfigured") + c.APIKey = "" } - c.APIKey = apiKey - appSecret, err := utils.DecryptStoredSecret(c.AppSecret) - if err != nil { - return fmt.Errorf("decrypt model parameters app_secret: %w", err) + if plain, ok := utils.DecryptStoredSecretLenient(c.AppSecret); ok { + c.AppSecret = plain + } else { + log.Printf("[crypto] model parameters app_secret: decrypt failed (SYSTEM_AES_KEY missing/rotated?), treating as unconfigured") + c.AppSecret = "" } - c.AppSecret = appSecret return nil } diff --git a/internal/types/tenant.go b/internal/types/tenant.go index d6cd9cc7..b55e30ed 100644 --- a/internal/types/tenant.go +++ b/internal/types/tenant.go @@ -4,6 +4,7 @@ import ( "database/sql/driver" "encoding/json" "fmt" + "log" "os" "strings" "time" @@ -311,11 +312,12 @@ func (c *CredentialsConfig) Scan(value interface{}) error { return err } if c.WeKnoraCloud != nil { - decrypted, err := utils.DecryptStoredSecret(c.WeKnoraCloud.AppSecret) - if err != nil { - return fmt.Errorf("decrypt tenant credentials we_knora_cloud.app_secret: %w", err) + if plain, ok := utils.DecryptStoredSecretLenient(c.WeKnoraCloud.AppSecret); ok { + c.WeKnoraCloud.AppSecret = plain + } else { + log.Printf("[crypto] tenant credentials we_knora_cloud.app_secret: decrypt failed (SYSTEM_AES_KEY missing/rotated?), treating as unconfigured") + c.WeKnoraCloud.AppSecret = "" } - c.WeKnoraCloud.AppSecret = decrypted } return nil } diff --git a/internal/types/web_search_provider.go b/internal/types/web_search_provider.go index 0f1b3ac7..f12202a0 100644 --- a/internal/types/web_search_provider.go +++ b/internal/types/web_search_provider.go @@ -3,7 +3,7 @@ package types import ( "database/sql/driver" "encoding/json" - "fmt" + "log" "time" "github.com/Tencent/WeKnora/internal/utils" @@ -109,11 +109,12 @@ func (p *WebSearchProviderParameters) Scan(value interface{}) error { if err := json.Unmarshal(b, p); err != nil { return err } - apiKey, err := utils.DecryptStoredSecret(p.APIKey) - if err != nil { - return fmt.Errorf("decrypt web search provider api_key: %w", err) + if plain, ok := utils.DecryptStoredSecretLenient(p.APIKey); ok { + p.APIKey = plain + } else { + log.Printf("[crypto] web search provider api_key: decrypt failed (SYSTEM_AES_KEY missing/rotated?), treating as unconfigured") + p.APIKey = "" } - p.APIKey = apiKey return nil } diff --git a/internal/utils/crypto.go b/internal/utils/crypto.go index b1d911f7..f301db78 100644 --- a/internal/utils/crypto.go +++ b/internal/utils/crypto.go @@ -96,24 +96,28 @@ func DecryptAESGCM(encrypted string, key []byte) (string, error) { } // DecryptStoredSecret decrypts a value loaded from the database with strict -// error propagation. Use this from GORM Scan/AfterFind hooks for any field that -// stores AES-encrypted secrets (API keys, passwords, app secrets). +// error propagation. Use this in hot-path code that is about to actually +// USE the credential (e.g. building an upstream HTTP request) — a rotated +// or missing key must surface as a loud error rather than silently +// degrading to "no credential", because the user expects the configured +// upstream call to either work or fail with a clear "auth misconfigured" +// message. // // Behaviour: // - empty input -> empty output, no error. // - no enc:v1: prefix -> returned as-is (legacy plaintext column), no error. // - has enc:v1: prefix and SYSTEM_AES_KEY is missing or wrong length -> -// returns ErrEncryptedDataMissingKey. Callers MUST propagate this so the -// load fails loudly instead of silently surfacing ciphertext as a credential. +// returns ErrEncryptedDataMissingKey. // - has enc:v1: prefix and key is set -> decrypts, returns any decryption // error verbatim (e.g. base64 decode failure, GCM auth tag mismatch from a // rotated key). // -// The previous lenient pattern (`if decrypted, err := ...; err == nil { ... }`) -// hid the rotated-key case and caused the encrypted ciphertext to be sent -// upstream as the actual API key, surfacing as 401/403 from third-party -// vendors. Always prefer this helper over calling DecryptAESGCM directly when -// loading from the database. +// For GORM Scan paths that load a whole row (where erroring out hides the +// rest of the resource from the user) use DecryptStoredSecretLenient +// instead — it lets the row load with the secret blanked out so the UI +// can render "credential not configured" and the user can re-enter it, +// without crashing list endpoints when the operator rotates or removes +// SYSTEM_AES_KEY. func DecryptStoredSecret(encrypted string) (string, error) { if encrypted == "" { return "", nil @@ -127,3 +131,29 @@ func DecryptStoredSecret(encrypted string) (string, error) { } return DecryptAESGCM(encrypted, key) } + +// DecryptStoredSecretLenient is the load-path counterpart for use inside +// GORM Scan / AfterFind hooks. It prefers graceful degradation over +// failing the whole row load. +// +// Returns: +// - (plaintext, true) for empty input, legacy plaintext, or successful decrypt +// - ("", false) when the value has the enc:v1: prefix but cannot be +// decrypted (SYSTEM_AES_KEY missing, rotated, or ciphertext corrupted). +// The bool=false case is the operator's signal: log a warning and treat +// the field as unconfigured. The row still loads so the rest of the +// resource displays normally; the UI shows "credential not configured", +// and any upstream call attempted with this empty value fails with a +// clear "missing api key" rather than sending ciphertext as the key. +// +// Rationale: a single line of broken ciphertext used to crash entire list +// endpoints (e.g. ListModels returning "" because one row failed Scan), +// hiding all other models from the user. Lenient load + loud per-row log +// is strictly more recoverable than fail-fast Scan. +func DecryptStoredSecretLenient(encrypted string) (plaintext string, ok bool) { + plain, err := DecryptStoredSecret(encrypted) + if err != nil { + return "", false + } + return plain, true +}