feat(crypto): AES-GCM at-rest for MCP and DataSource credentials with graceful key-rotation handling

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.
This commit is contained in:
wizardchen
2026-05-17 15:10:04 +08:00
committed by lyingbug
parent a9d37970ae
commit dc634e863a
8 changed files with 396 additions and 34 deletions
+60 -3
View File
@@ -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
}
+118
View File
@@ -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")
}
+46 -4
View File
@@ -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
+107
View File
@@ -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")
}
+14 -9
View File
@@ -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
}
+6 -4
View File
@@ -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
}
+6 -5
View File
@@ -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
}
+39 -9
View File
@@ -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
}