From 6ccf3451f11c186534fe0d648c58f1611d63e94a Mon Sep 17 00:00:00 2001 From: Anuj Parihar Date: Mon, 22 Jun 2026 12:53:19 +0530 Subject: [PATCH 1/3] feat: make secretRef and fromSecret non global fields --- core/providers/utils/utils.go | 8 +- core/schemas/secretvar.go | 159 ++++++++++------- core/schemas/secretvar_test.go | 196 ++++++++++++--------- core/schemas/utils.go | 2 +- core/schemas/vault.go | 8 +- core/schemas/vault_test.go | 39 ++-- framework/configstore/clientconfig.go | 8 +- framework/configstore/rdb.go | 2 +- framework/configstore/tables/mcp.go | 2 +- transports/bifrost-http/handlers/config.go | 20 +-- transports/bifrost-http/lib/config.go | 21 +-- 11 files changed, 249 insertions(+), 216 deletions(-) diff --git a/core/providers/utils/utils.go b/core/providers/utils/utils.go index 9c0ba92b82..305fcc0090 100644 --- a/core/providers/utils/utils.go +++ b/core/providers/utils/utils.go @@ -344,7 +344,7 @@ func ConfigureProxy(client *fasthttp.Client, proxyConfig *schemas.ProxyConfig, l return client case schemas.HTTPProxy: if proxyConfig.URL != nil && proxyConfig.URL.IsFromSecret() && proxyConfig.URL.GetValue() == "" { - errMsg := fmt.Sprintf("invalid proxy configuration: %s references %q but it resolved to an empty value", "proxy.url", proxyConfig.URL.SecretRef) + errMsg := fmt.Sprintf("invalid proxy configuration: %s references %q but it resolved to an empty value", "proxy.url", proxyConfig.URL.Ref()) getLogger().Error(errMsg) client.Dial = dialErrorFunc(errMsg) return client @@ -370,7 +370,7 @@ func ConfigureProxy(client *fasthttp.Client, proxyConfig *schemas.ProxyConfig, l dialFunc = fasthttpproxy.FasthttpHTTPDialer(proxyURL) case schemas.Socks5Proxy: if proxyConfig.URL != nil && proxyConfig.URL.IsFromSecret() && proxyConfig.URL.GetValue() == "" { - errMsg := fmt.Sprintf("invalid proxy configuration: %s references %q but it resolved to an empty value", "proxy.url", proxyConfig.URL.SecretRef) + errMsg := fmt.Sprintf("invalid proxy configuration: %s references %q but it resolved to an empty value", "proxy.url", proxyConfig.URL.Ref()) getLogger().Error(errMsg) client.Dial = dialErrorFunc(errMsg) return client @@ -409,7 +409,7 @@ func ConfigureProxy(client *fasthttp.Client, proxyConfig *schemas.ProxyConfig, l // Configure custom CA certificate if provided if proxyConfig.CACertPEM != nil && proxyConfig.CACertPEM.IsFromSecret() && proxyConfig.CACertPEM.GetValue() == "" { - errMsg := fmt.Sprintf("invalid proxy configuration: %s references %q but it resolved to an empty value", "proxy.ca_cert_pem", proxyConfig.CACertPEM.SecretRef) + errMsg := fmt.Sprintf("invalid proxy configuration: %s references %q but it resolved to an empty value", "proxy.ca_cert_pem", proxyConfig.CACertPEM.Ref()) getLogger().Error(errMsg) client.Dial = dialErrorFunc(errMsg) return client @@ -452,7 +452,7 @@ func createTLSConfigWithCA(caCertPEM string) (*tls.Config, error) { // It merges with any existing TLSConfig (e.g., from ConfigureProxy). func ConfigureTLS(client *fasthttp.Client, networkConfig schemas.NetworkConfig, logger schemas.Logger) *fasthttp.Client { if networkConfig.CACertPEM != nil && networkConfig.CACertPEM.IsFromSecret() && networkConfig.CACertPEM.GetValue() == "" { - errMsg := fmt.Sprintf("invalid provider configuration: %s references %q but it resolved to an empty value", "network_config.ca_cert_pem", networkConfig.CACertPEM.SecretRef) + errMsg := fmt.Sprintf("invalid provider configuration: %s references %q but it resolved to an empty value", "network_config.ca_cert_pem", networkConfig.CACertPEM.Ref()) logger.Error(errMsg) client.Dial = dialErrorFunc(errMsg) return client diff --git a/core/schemas/secretvar.go b/core/schemas/secretvar.go index 6c28182c11..2a3d55233f 100644 --- a/core/schemas/secretvar.go +++ b/core/schemas/secretvar.go @@ -15,8 +15,8 @@ import ( // Three reference forms are accepted: plain text, "env.VAR_NAME", and "vault.path/to/secret". type SecretVar struct { Val string `json:"value"` - SecretRef string `json:"secret_ref,omitempty"` - FromSecret bool `json:"from_secret,omitempty"` + secretRef string + fromSecret bool } // NewSecretVar creates a new SecretVar from a string. @@ -45,8 +45,8 @@ func NewSecretVar(value string) *SecretVar { e := &SecretVar{Val: raw.Val} // New format if raw.SecretRef != "" || raw.FromSecret { - e.SecretRef = raw.SecretRef - e.FromSecret = raw.FromSecret + e.secretRef = raw.SecretRef + e.fromSecret = raw.FromSecret } else { // Backward compat: env (shipped — must keep working) if raw.FromEnv && raw.EnvVar != "" { @@ -54,8 +54,8 @@ func NewSecretVar(value string) *SecretVar { if !strings.HasPrefix(ref, "env.") { ref = "env." + ref } - e.SecretRef = ref - e.FromSecret = true + e.secretRef = ref + e.fromSecret = true if envValue, ok := os.LookupEnv(strings.TrimPrefix(ref, "env.")); ok { e.Val = envValue } else { @@ -65,8 +65,8 @@ func NewSecretVar(value string) *SecretVar { } // Legacy format: value == env_var == "env.XXX" if strings.HasPrefix(raw.Val, "env.") && raw.Val == raw.EnvVar { - e.SecretRef = raw.EnvVar - e.FromSecret = true + e.secretRef = raw.EnvVar + e.fromSecret = true e.Val = "" if envValue, ok := os.LookupEnv(strings.TrimPrefix(raw.EnvVar, "env.")); ok { e.Val = envValue @@ -75,15 +75,15 @@ func NewSecretVar(value string) *SecretVar { } } // Resolve vault reference - if e.FromSecret && strings.HasPrefix(e.SecretRef, "vault.") { + if e.fromSecret && strings.HasPrefix(e.secretRef, "vault.") { e.Val = "" - if vaultValue, ok := LookupVault(e.SecretRef); ok { + if vaultValue, ok := LookupVault(e.secretRef); ok { e.Val = vaultValue } } // Resolve env reference - if e.FromSecret && strings.HasPrefix(e.SecretRef, "env.") { - if envValue, ok := os.LookupEnv(strings.TrimPrefix(e.SecretRef, "env.")); ok { + if e.fromSecret && strings.HasPrefix(e.secretRef, "env.") { + if envValue, ok := os.LookupEnv(strings.TrimPrefix(e.secretRef, "env.")); ok { e.Val = envValue } else { e.Val = "" @@ -95,8 +95,8 @@ func NewSecretVar(value string) *SecretVar { } if strings.HasPrefix(val, "vault.") { e := &SecretVar{ - SecretRef: val, - FromSecret: true, + secretRef: val, + fromSecret: true, } if vaultValue, ok := LookupVault(val); ok { e.Val = vaultValue @@ -107,14 +107,14 @@ func NewSecretVar(value string) *SecretVar { if envValue, ok := os.LookupEnv(envKey); ok { return &SecretVar{ Val: envValue, - SecretRef: val, - FromSecret: true, + secretRef: val, + fromSecret: true, } } return &SecretVar{ Val: "", - SecretRef: val, - FromSecret: true, + secretRef: val, + fromSecret: true, } } return &SecretVar{ @@ -122,12 +122,21 @@ func NewSecretVar(value string) *SecretVar { } } +// Ref returns the secret reference string (e.g. "env.MY_VAR" or "vault.path/to/secret"). +// Returns an empty string for plain-value SecretVars. +func (e *SecretVar) Ref() string { + if e == nil { + return "" + } + return e.secretRef +} + // IsFromSecret returns true if the value is sourced from an external secret (env var or vault). func (e *SecretVar) IsFromSecret() bool { if e == nil { return false } - return e.FromSecret + return e.fromSecret } // IsFromVault returns true if the value is sourced from a vault path. @@ -135,7 +144,7 @@ func (e *SecretVar) IsFromVault() bool { if e == nil { return false } - return e.FromSecret && strings.HasPrefix(e.SecretRef, "vault.") + return e.fromSecret && strings.HasPrefix(e.secretRef, "vault.") } // IsRedacted returns true if the value is redacted. @@ -143,11 +152,11 @@ func (e *SecretVar) IsRedacted() bool { if e == nil { return false } - if e.Val == "" && !e.FromSecret { + if e.Val == "" && !e.fromSecret { return false } // Secret references (env/vault) are treated as redacted — the real value is external - if e.FromSecret { + if e.fromSecret { return true } if len(e.Val) <= 8 { @@ -180,8 +189,8 @@ func (e *SecretVar) Equals(other *SecretVar) bool { return false } return e.Val == other.Val && - e.SecretRef == other.SecretRef && - e.FromSecret == other.FromSecret + e.secretRef == other.secretRef && + e.fromSecret == other.fromSecret } // Redacted returns a new SecretVar with the value redacted. @@ -192,16 +201,16 @@ func (e *SecretVar) Redacted() *SecretVar { if e.Val == "" { return &SecretVar{ Val: "", - SecretRef: e.SecretRef, - FromSecret: e.FromSecret, + secretRef: e.secretRef, + fromSecret: e.fromSecret, } } // If key is 8 characters or less, just return all asterisks if len(e.Val) <= 8 { return &SecretVar{ Val: strings.Repeat("*", len(e.Val)), - SecretRef: e.SecretRef, - FromSecret: e.FromSecret, + secretRef: e.secretRef, + fromSecret: e.fromSecret, } } // Show first 4 and last 4 characters, replace middle with asterisks @@ -211,14 +220,14 @@ func (e *SecretVar) Redacted() *SecretVar { return &SecretVar{ Val: prefix + middle + suffix, - SecretRef: e.SecretRef, - FromSecret: e.FromSecret, + secretRef: e.secretRef, + fromSecret: e.fromSecret, } } // FullyRedacted returns a copy of the SecretVar with Val replaced by a fixed placeholder // so no substring of the original value is exposed. Use for API responses where -// Redacted is unsafe (e.g. literal proxy passwords). SecretRef and FromSecret are +// Redacted is unsafe (e.g. literal proxy passwords). secretRef and fromSecret are // preserved so references remain visible. func (e *SecretVar) FullyRedacted() *SecretVar { if e == nil { @@ -227,17 +236,30 @@ func (e *SecretVar) FullyRedacted() *SecretVar { if e.Val == "" { return &SecretVar{ Val: "", - SecretRef: e.SecretRef, - FromSecret: e.FromSecret, + secretRef: e.secretRef, + fromSecret: e.fromSecret, } } return &SecretVar{ Val: "", - SecretRef: e.SecretRef, - FromSecret: e.FromSecret, + secretRef: e.secretRef, + fromSecret: e.fromSecret, } } +// MarshalJSON serializes the SecretVar, emitting secret_ref and from_secret fields. +func (e SecretVar) MarshalJSON() ([]byte, error) { + return Marshal(struct { + Val string `json:"value"` + SecretRef string `json:"secret_ref,omitempty"` + FromSecret bool `json:"from_secret,omitempty"` + }{ + Val: e.Val, + SecretRef: e.secretRef, + FromSecret: e.fromSecret, + }) +} + // UnmarshalJSON unmarshals the value from JSON. func (e *SecretVar) UnmarshalJSON(data []byte) error { val := string(data) @@ -263,16 +285,16 @@ func (e *SecretVar) UnmarshalJSON(data []byte) error { // New format if raw.SecretRef != "" || raw.FromSecret { - e.SecretRef = raw.SecretRef - e.FromSecret = raw.FromSecret + e.secretRef = raw.SecretRef + e.fromSecret = raw.FromSecret } else if raw.FromEnv && raw.EnvVar != "" { // Backward compat: env ref := raw.EnvVar if !strings.HasPrefix(ref, "env.") { ref = "env." + ref } - e.SecretRef = ref - e.FromSecret = true + e.secretRef = ref + e.fromSecret = true if envValue, ok := os.LookupEnv(strings.TrimPrefix(ref, "env.")); ok { e.Val = envValue } else { @@ -281,8 +303,8 @@ func (e *SecretVar) UnmarshalJSON(data []byte) error { return nil } else if strings.HasPrefix(raw.Val, "env.") && raw.Val == raw.EnvVar { // Old format: value == env_var == "env.XXX" - e.SecretRef = raw.EnvVar - e.FromSecret = true + e.secretRef = raw.EnvVar + e.fromSecret = true e.Val = "" if envValue, ok := os.LookupEnv(strings.TrimPrefix(raw.EnvVar, "env.")); ok { e.Val = envValue @@ -291,14 +313,14 @@ func (e *SecretVar) UnmarshalJSON(data []byte) error { } // Resolve references - if e.FromSecret && strings.HasPrefix(e.SecretRef, "vault.") { + if e.fromSecret && strings.HasPrefix(e.secretRef, "vault.") { e.Val = "" - if vaultValue, ok := LookupVault(e.SecretRef); ok { + if vaultValue, ok := LookupVault(e.secretRef); ok { e.Val = vaultValue } } - if e.FromSecret && strings.HasPrefix(e.SecretRef, "env.") { - if envValue, ok := os.LookupEnv(strings.TrimPrefix(e.SecretRef, "env.")); ok { + if e.fromSecret && strings.HasPrefix(e.secretRef, "env.") { + if envValue, ok := os.LookupEnv(strings.TrimPrefix(e.secretRef, "env.")); ok { e.Val = envValue } else { e.Val = "" @@ -310,8 +332,8 @@ func (e *SecretVar) UnmarshalJSON(data []byte) error { } // Plain string forms if strings.HasPrefix(val, "vault.") { - e.SecretRef = val - e.FromSecret = true + e.secretRef = val + e.fromSecret = true e.Val = "" if vaultValue, ok := LookupVault(val); ok { e.Val = vaultValue @@ -319,8 +341,8 @@ func (e *SecretVar) UnmarshalJSON(data []byte) error { return nil } if envKey, ok := strings.CutPrefix(val, "env."); ok { - e.SecretRef = val - e.FromSecret = true + e.secretRef = val + e.fromSecret = true if envValue, ok := os.LookupEnv(envKey); ok { e.Val = envValue } else { @@ -329,8 +351,8 @@ func (e *SecretVar) UnmarshalJSON(data []byte) error { return nil } e.Val = val - e.SecretRef = "" - e.FromSecret = false + e.secretRef = "" + e.fromSecret = false return nil } @@ -346,26 +368,27 @@ func (e *SecretVar) String() string { func (e *SecretVar) Scan(value any) error { if value == nil { e.Val = "" - e.SecretRef = "" - e.FromSecret = false + e.secretRef = "" + e.fromSecret = false return nil } switch v := value.(type) { case []byte: return e.Scan(string(v)) case string: - if strings.HasPrefix(v, "vault.") { + val := strings.Trim(v, "\"") + if strings.HasPrefix(val, "vault.") { e.Val = "" - e.SecretRef = v - e.FromSecret = true - if vaultValue, ok := LookupVault(v); ok { + e.secretRef = val + e.fromSecret = true + if vaultValue, ok := LookupVault(val); ok { e.Val = vaultValue } return nil } - if envKey, ok := strings.CutPrefix(v, "env."); ok { - e.SecretRef = v - e.FromSecret = true + if envKey, ok := strings.CutPrefix(val, "env."); ok { + e.secretRef = val + e.fromSecret = true if envValue, ok := os.LookupEnv(envKey); ok { e.Val = envValue } else { @@ -374,8 +397,8 @@ func (e *SecretVar) Scan(value any) error { return nil } e.Val = strings.Trim(v, "\"") - e.SecretRef = "" - e.FromSecret = false + e.secretRef = "" + e.fromSecret = false return nil } return fmt.Errorf("failed to scan value: %v", value) @@ -383,10 +406,10 @@ func (e *SecretVar) Scan(value any) error { // Value implements driver.Valuer for database storage. // It stores the secret reference (e.g., "env.API_KEY" or "vault.path/to/secret") if -// FromSecret is true, otherwise the raw value. +// fromSecret is true, otherwise the raw value. func (e SecretVar) Value() (driver.Value, error) { - if e.FromSecret { - return e.SecretRef, nil + if e.fromSecret { + return e.secretRef, nil } return e.Val, nil } @@ -399,7 +422,7 @@ func (e *SecretVar) ShouldPreserveStored() bool { if e == nil { return true } - if e.FromSecret { + if e.fromSecret { return false } return e.GetValue() == "" || e.IsRedacted() @@ -412,8 +435,8 @@ func (e *SecretVar) IsSet() bool { if e == nil { return false } - if e.FromSecret { - return e.SecretRef != "" + if e.fromSecret { + return e.secretRef != "" } return e.Val != "" } diff --git a/core/schemas/secretvar_test.go b/core/schemas/secretvar_test.go index 9f08bacea9..ecbfa472ba 100644 --- a/core/schemas/secretvar_test.go +++ b/core/schemas/secretvar_test.go @@ -54,8 +54,8 @@ func TestSecretVar_UnmarshalJSON_DoubleEscapedJSON(t *testing.T) { if secretVar.Val != tt.expected { t.Errorf("Expected Val=%q, got Val=%q", tt.expected, secretVar.Val) } - if secretVar.FromSecret { - t.Errorf("Expected FromSecret=false, got FromSecret=true") + if secretVar.IsFromSecret() { + t.Errorf("Expected IsFromSecret()=false, got true") } }) } @@ -98,11 +98,11 @@ func TestSecretVar_UnmarshalJSON_SecretVarReference(t *testing.T) { if secretVar.Val != tt.expectedVal { t.Errorf("Expected Val=%q, got Val=%q", tt.expectedVal, secretVar.Val) } - if secretVar.SecretRef != tt.expectedSecretRef { - t.Errorf("Expected SecretRef=%q, got SecretRef=%q", tt.expectedSecretRef, secretVar.SecretRef) + if secretVar.Ref() != tt.expectedSecretRef { + t.Errorf("Expected Ref()=%q, got %q", tt.expectedSecretRef, secretVar.Ref()) } - if secretVar.FromSecret != tt.expectedFromSecret { - t.Errorf("Expected FromSecret=%v, got FromSecret=%v", tt.expectedFromSecret, secretVar.FromSecret) + if secretVar.IsFromSecret() != tt.expectedFromSecret { + t.Errorf("Expected IsFromSecret()=%v, got %v", tt.expectedFromSecret, secretVar.IsFromSecret()) } }) } @@ -121,17 +121,16 @@ func TestSecretVar_UnmarshalJSON_BackwardCompat(t *testing.T) { if err != nil { t.Fatalf("UnmarshalJSON failed: %v", err) } - if secretVar.SecretRef != "env.MY_KEY" { - t.Errorf("Expected SecretRef=%q, got %q", "env.MY_KEY", secretVar.SecretRef) + if secretVar.Ref() != "env.MY_KEY" { + t.Errorf("Expected Ref()=%q, got %q", "env.MY_KEY", secretVar.Ref()) } - if !secretVar.FromSecret { - t.Error("Expected FromSecret=true, got false") + if !secretVar.IsFromSecret() { + t.Error("Expected IsFromSecret()=true, got false") } if secretVar.Val != "resolved-value" { t.Errorf("Expected Val=%q, got %q", "resolved-value", secretVar.Val) } }) - } func TestNewSecretVar_DoubleEscapedJSON(t *testing.T) { @@ -180,28 +179,28 @@ func TestNewSecretVar_SecretVarReference(t *testing.T) { name string input string expectedVal string - expectedSecretRef string + expectedRef string expectedFromSecret bool }{ { name: "env var reference with value present", input: "env.TEST_NEW_ENVVAR_KEY", expectedVal: "resolved-value", - expectedSecretRef: "env.TEST_NEW_ENVVAR_KEY", + expectedRef: "env.TEST_NEW_ENVVAR_KEY", expectedFromSecret: true, }, { name: "env var reference with quotes", input: `"env.TEST_NEW_ENVVAR_KEY"`, expectedVal: "resolved-value", - expectedSecretRef: "env.TEST_NEW_ENVVAR_KEY", + expectedRef: "env.TEST_NEW_ENVVAR_KEY", expectedFromSecret: true, }, { name: "env var reference missing", input: "env.MISSING_VAR", expectedVal: "", - expectedSecretRef: "env.MISSING_VAR", + expectedRef: "env.MISSING_VAR", expectedFromSecret: true, }, } @@ -212,11 +211,11 @@ func TestNewSecretVar_SecretVarReference(t *testing.T) { if secretVar.Val != tt.expectedVal { t.Errorf("Expected Val=%q, got Val=%q", tt.expectedVal, secretVar.Val) } - if secretVar.SecretRef != tt.expectedSecretRef { - t.Errorf("Expected SecretRef=%q, got SecretRef=%q", tt.expectedSecretRef, secretVar.SecretRef) + if secretVar.Ref() != tt.expectedRef { + t.Errorf("Expected Ref()=%q, got %q", tt.expectedRef, secretVar.Ref()) } - if secretVar.FromSecret != tt.expectedFromSecret { - t.Errorf("Expected FromSecret=%v, got FromSecret=%v", tt.expectedFromSecret, secretVar.FromSecret) + if secretVar.IsFromSecret() != tt.expectedFromSecret { + t.Errorf("Expected IsFromSecret()=%v, got %v", tt.expectedFromSecret, secretVar.IsFromSecret()) } }) } @@ -280,16 +279,16 @@ func TestSecretVar_MixedConfigParsing(t *testing.T) { if config.ProjectID.Val != "env-project-id" { t.Errorf("Expected ProjectID=%q, got %q", "env-project-id", config.ProjectID.Val) } - if !config.ProjectID.FromSecret { - t.Errorf("Expected ProjectID.FromSecret=true") + if !config.ProjectID.IsFromSecret() { + t.Error("Expected ProjectID.IsFromSecret()=true") } expectedCreds := `{"type":"service_account","key":"value"}` if config.Credentials.Val != expectedCreds { t.Errorf("Expected Credentials=%q, got %q", expectedCreds, config.Credentials.Val) } - if config.Credentials.FromSecret { - t.Errorf("Expected Credentials.FromSecret=false") + if config.Credentials.IsFromSecret() { + t.Error("Expected Credentials.IsFromSecret()=false") } } @@ -320,8 +319,8 @@ func TestSecretVar_Equals(t *testing.T) { }, { name: "equal values", - a: &SecretVar{Val: "test", SecretRef: "env.TEST", FromSecret: true}, - b: &SecretVar{Val: "test", SecretRef: "env.TEST", FromSecret: true}, + a: NewSecretVarFromRef("env.TEST", "test"), + b: NewSecretVarFromRef("env.TEST", "test"), expected: true, }, { @@ -384,11 +383,11 @@ func TestSecretVar_FullyRedacted(t *testing.T) { }) tests := []struct { - name string - input SecretVar - wantVal string - wantFromSecret bool - wantSecretRef string + name string + input SecretVar + wantVal string + wantFromSecret bool + wantRef string }{ { name: "empty value", @@ -404,10 +403,10 @@ func TestSecretVar_FullyRedacted(t *testing.T) { }, { name: "resolved env password preserves reference metadata", - input: SecretVar{Val: "resolved-secret", FromSecret: true, SecretRef: "env.PROXY_PASS"}, + input: *NewSecretVarFromRef("env.PROXY_PASS", "resolved-secret"), wantVal: "", wantFromSecret: true, - wantSecretRef: "env.PROXY_PASS", + wantRef: "env.PROXY_PASS", }, } @@ -417,11 +416,11 @@ func TestSecretVar_FullyRedacted(t *testing.T) { if result.Val != tt.wantVal { t.Errorf("Val: want %q, got %q", tt.wantVal, result.Val) } - if result.FromSecret != tt.wantFromSecret { - t.Errorf("FromSecret: want %v, got %v", tt.wantFromSecret, result.FromSecret) + if result.IsFromSecret() != tt.wantFromSecret { + t.Errorf("IsFromSecret(): want %v, got %v", tt.wantFromSecret, result.IsFromSecret()) } - if result.SecretRef != tt.wantSecretRef { - t.Errorf("SecretRef: want %q, got %q", tt.wantSecretRef, result.SecretRef) + if result.Ref() != tt.wantRef { + t.Errorf("Ref(): want %q, got %q", tt.wantRef, result.Ref()) } }) } @@ -440,7 +439,7 @@ func TestSecretVar_IsRedacted(t *testing.T) { }, { name: "from secret", - input: SecretVar{Val: "test", FromSecret: true, SecretRef: "env.KEY"}, + input: *NewSecretVarFromRef("env.KEY", "test"), expected: true, }, { @@ -501,29 +500,24 @@ func TestSecretVar_IsSet(t *testing.T) { }, { name: "env reference not yet resolved", - input: &SecretVar{SecretRef: "env.MISSING", FromSecret: true}, + input: NewSecretVarFromRef("env.MISSING", ""), expected: true, }, { name: "env reference resolved", - input: &SecretVar{Val: "resolved-secret", SecretRef: "env.X", FromSecret: true}, + input: NewSecretVarFromRef("env.X", "resolved-secret"), expected: true, }, { - name: "FromSecret true but no reference and no value", - input: &SecretVar{FromSecret: true}, + name: "fromSecret true but no reference and no value", + input: NewSecretVarFromRef("", ""), expected: false, }, { name: "vault reference set", - input: &SecretVar{SecretRef: "vault.bifrost/key", FromSecret: true, Val: "vault.bifrost/key"}, + input: NewSecretVarFromRef("vault.bifrost/key", "vault.bifrost/key"), expected: true, }, - { - name: "FromSecret true but empty SecretRef", - input: &SecretVar{FromSecret: true, SecretRef: ""}, - expected: false, - }, } for _, tt := range tests { @@ -537,17 +531,17 @@ func TestSecretVar_IsSet(t *testing.T) { func TestSecretVar_Scan_VaultRef(t *testing.T) { tests := []struct { - name string - input string - wantVal string - wantSecretRef string - wantFromSecret bool + name string + input string + wantVal string + wantRef string + wantFromSecret bool }{ { name: "plain vault reference", input: "vault.bifrost/providers/openai/key", wantVal: "", - wantSecretRef: "vault.bifrost/providers/openai/key", + wantRef: "vault.bifrost/providers/openai/key", wantFromSecret: true, }, { @@ -558,7 +552,7 @@ func TestSecretVar_Scan_VaultRef(t *testing.T) { { name: "env reference", input: "env.MY_VAR", - wantSecretRef: "env.MY_VAR", + wantRef: "env.MY_VAR", wantFromSecret: true, }, { @@ -574,13 +568,11 @@ func TestSecretVar_Scan_VaultRef(t *testing.T) { if err := e.Scan(tt.input); err != nil { t.Fatalf("Scan() error: %v", err) } - if tt.wantFromSecret { - if !e.FromSecret { - t.Errorf("FromSecret = false, want true") - } - if e.SecretRef != tt.wantSecretRef { - t.Errorf("SecretRef = %q, want %q", e.SecretRef, tt.wantSecretRef) - } + if e.IsFromSecret() != tt.wantFromSecret { + t.Errorf("IsFromSecret() = %v, want %v", e.IsFromSecret(), tt.wantFromSecret) + } + if tt.wantFromSecret && e.Ref() != tt.wantRef { + t.Errorf("Ref() = %q, want %q", e.Ref(), tt.wantRef) } if tt.wantVal != "" && e.Val != tt.wantVal { t.Errorf("Val = %q, want %q", e.Val, tt.wantVal) @@ -594,21 +586,21 @@ func TestSecretVar_UnmarshalJSON_VaultRef(t *testing.T) { name string input string wantVal string - wantSecretRef string + wantRef string wantFromSecret bool }{ { name: "new format: secret_ref/from_secret", input: `{"value":"vault.bifrost/key","secret_ref":"vault.bifrost/key","from_secret":true}`, wantVal: "", - wantSecretRef: "vault.bifrost/key", + wantRef: "vault.bifrost/key", wantFromSecret: true, }, { name: "plain string vault reference", input: `"vault.myproject/secret"`, wantVal: "", - wantSecretRef: "vault.myproject/secret", + wantRef: "vault.myproject/secret", wantFromSecret: true, }, } @@ -619,11 +611,11 @@ func TestSecretVar_UnmarshalJSON_VaultRef(t *testing.T) { if err := json.Unmarshal([]byte(tt.input), &e); err != nil { t.Fatalf("UnmarshalJSON() error: %v", err) } - if e.FromSecret != tt.wantFromSecret { - t.Errorf("FromSecret = %v, want %v", e.FromSecret, tt.wantFromSecret) + if e.IsFromSecret() != tt.wantFromSecret { + t.Errorf("IsFromSecret() = %v, want %v", e.IsFromSecret(), tt.wantFromSecret) } - if e.SecretRef != tt.wantSecretRef { - t.Errorf("SecretRef = %q, want %q", e.SecretRef, tt.wantSecretRef) + if e.Ref() != tt.wantRef { + t.Errorf("Ref() = %q, want %q", e.Ref(), tt.wantRef) } if e.Val != tt.wantVal { t.Errorf("Val = %q, want %q", e.Val, tt.wantVal) @@ -633,11 +625,7 @@ func TestSecretVar_UnmarshalJSON_VaultRef(t *testing.T) { } func TestSecretVar_Value_VaultRef(t *testing.T) { - e := &SecretVar{ - Val: "actual-secret", - SecretRef: "vault.bifrost/key", - FromSecret: true, - } + e := NewSecretVarFromRef("vault.bifrost/key", "actual-secret") got, err := e.Value() if err != nil { t.Fatalf("Value() error: %v", err) @@ -648,27 +636,23 @@ func TestSecretVar_Value_VaultRef(t *testing.T) { } func TestSecretVar_Redacted_VaultRef(t *testing.T) { - e := &SecretVar{ - Val: "actual-secret-value", - SecretRef: "vault.bifrost/key", - FromSecret: true, - } + e := NewSecretVarFromRef("vault.bifrost/key", "actual-secret-value") r := e.Redacted() wantVal := "actu************************alue" if r.Val != wantVal { t.Errorf("Redacted().Val = %q, want %q", r.Val, wantVal) } - if !r.FromSecret { - t.Errorf("Redacted().FromSecret = false, want true") + if !r.IsFromSecret() { + t.Error("Redacted().IsFromSecret() = false, want true") } - if r.SecretRef != "vault.bifrost/key" { - t.Errorf("Redacted().SecretRef = %q, want %q", r.SecretRef, "vault.bifrost/key") + if r.Ref() != "vault.bifrost/key" { + t.Errorf("Redacted().Ref() = %q, want %q", r.Ref(), "vault.bifrost/key") } } func TestSecretVar_IsSet_VaultRef(t *testing.T) { - set := &SecretVar{SecretRef: "vault.bifrost/key", FromSecret: true, Val: "vault.bifrost/key"} - unset := &SecretVar{FromSecret: true} + set := NewSecretVarFromRef("vault.bifrost/key", "vault.bifrost/key") + unset := NewSecretVarFromRef("", "") if !set.IsSet() { t.Error("IsSet() = false for vault ref, want true") } @@ -679,13 +663,49 @@ func TestSecretVar_IsSet_VaultRef(t *testing.T) { func TestNewSecretVar_VaultRef(t *testing.T) { e := NewSecretVar("vault.bifrost/mykey") - if !e.FromSecret { - t.Error("FromSecret = false, want true") + if !e.IsFromVault() { + t.Error("IsFromVault() = false, want true") } - if e.SecretRef != "vault.bifrost/mykey" { - t.Errorf("SecretRef = %q, want %q", e.SecretRef, "vault.bifrost/mykey") + if e.Ref() != "vault.bifrost/mykey" { + t.Errorf("Ref() = %q, want %q", e.Ref(), "vault.bifrost/mykey") } if e.Val != "" { t.Errorf("Val = %q, want empty string when vault lookup fails", e.Val) } } + +func TestSecretVar_MarshalJSON(t *testing.T) { + tests := []struct { + name string + input SecretVar + want string + }{ + { + name: "plain value", + input: SecretVar{Val: "my-api-key"}, + want: `{"value":"my-api-key"}`, + }, + { + name: "env reference", + input: *NewSecretVarFromRef("env.MY_KEY", "resolved"), + want: `{"value":"resolved","secret_ref":"env.MY_KEY","from_secret":true}`, + }, + { + name: "vault reference", + input: *NewSecretVarFromRef("vault.path/to/secret", ""), + want: `{"value":"","secret_ref":"vault.path/to/secret","from_secret":true}`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + b, err := json.Marshal(tt.input) + if err != nil { + t.Fatalf("MarshalJSON() error: %v", err) + } + if string(b) != tt.want { + t.Errorf("MarshalJSON() = %s, want %s", b, tt.want) + } + }) + } +} diff --git a/core/schemas/utils.go b/core/schemas/utils.go index 6548348854..4f8a4d60eb 100644 --- a/core/schemas/utils.go +++ b/core/schemas/utils.go @@ -38,7 +38,7 @@ func SecretVarAsString(e *SecretVar) string { return "" } if e.IsFromSecret() { - return e.SecretRef + return e.secretRef } return e.GetValue() } diff --git a/core/schemas/vault.go b/core/schemas/vault.go index f0772b23df..b9a4b6a760 100644 --- a/core/schemas/vault.go +++ b/core/schemas/vault.go @@ -119,10 +119,10 @@ func RemoveOwnedVaultSecretVars(ctx context.Context, ownedPrefix string, model i // vault-backed, non-fragment reference under ownedPrefix. Fragment refs (#key) // point at shared, externally-managed secrets and are never auto-deleted. func removeOwnedVaultSecretVar(ctx context.Context, ownedPrefix string, field *SecretVar) { - if field == nil || !strings.HasPrefix(field.SecretRef, "vault.") || field.SecretRef == "" { + if field == nil || !strings.HasPrefix(field.secretRef, "vault.") || field.secretRef == "" { return } - path := strings.TrimPrefix(field.SecretRef, "vault.") + path := strings.TrimPrefix(field.secretRef, "vault.") if strings.IndexByte(path, '#') >= 0 { return } @@ -145,8 +145,8 @@ func StoreVaultSecretVar(ctx context.Context, path string, e *SecretVar) error { if err := VaultStoreHook(ctx, path, &e.Val); err != nil { return err } - e.SecretRef = "vault." + path - e.FromSecret = true + e.secretRef = "vault." + path + e.fromSecret = true return nil } diff --git a/core/schemas/vault_test.go b/core/schemas/vault_test.go index 034def9621..b34185b697 100644 --- a/core/schemas/vault_test.go +++ b/core/schemas/vault_test.go @@ -31,11 +31,11 @@ func TestStoreVaultSecretVar_StoresPlaintext(t *testing.T) { if got := stored["bifrost/tbl/id/value"]; got != "secret-key" { t.Errorf("stored plaintext = %q, want %q", got, "secret-key") } - if !e.FromSecret { - t.Error("FromSecret should be true after store") + if !e.IsFromVault() { + t.Error("IsFromVault() should be true after store") } - if e.SecretRef != "vault.bifrost/tbl/id/value" { - t.Errorf("SecretRef = %q, want %q", e.SecretRef, "vault.bifrost/tbl/id/value") + if e.Ref() != "vault.bifrost/tbl/id/value" { + t.Errorf("Ref() = %q, want %q", e.Ref(), "vault.bifrost/tbl/id/value") } if e.Val != "vault.bifrost/tbl/id/value" { t.Errorf("Val = %q, want rewritten to vault ref", e.Val) @@ -48,8 +48,8 @@ func TestStoreVaultSecretVar_NoOps(t *testing.T) { e *SecretVar }{ {"nil", nil}, - {"env-sourced", &SecretVar{FromSecret: true, SecretRef: "env.MY_VAR"}}, - {"already-vault", &SecretVar{FromSecret: true, SecretRef: "vault.some/path", Val: "vault.some/path"}}, + {"env-sourced", NewSecretVarFromRef("env.MY_VAR", "")}, + {"already-vault", NewSecretVarFromRef("vault.some/path", "vault.some/path")}, {"empty", &SecretVar{Val: ""}}, } for _, tc := range cases { @@ -74,8 +74,8 @@ func TestStoreVaultSecretVar_NoHookNoOp(t *testing.T) { if err := StoreVaultSecretVar(context.Background(), "p", e); err != nil { t.Fatalf("StoreVaultSecretVar: %v", err) } - if e.FromSecret || e.SecretRef != "" || e.Val != "secret" { - t.Errorf("expected no mutation when hook nil, got %+v", e) + if e.IsFromSecret() || e.Ref() != "" || e.Val != "secret" { + t.Errorf("expected no mutation when hook nil, got val=%q ref=%q fromSecret=%v", e.Val, e.Ref(), e.IsFromSecret()) } } @@ -93,8 +93,8 @@ func TestRemoveOwnedVaultSecretVars_SkipsFragmentRefs(t *testing.T) { Fragment SecretVar `gorm:"column:fragment"` } m := &model{ - Normal: SecretVar{FromSecret: true, SecretRef: "vault.bifrost/m/1/normal", Val: "vault.bifrost/m/1/normal"}, - Fragment: SecretVar{FromSecret: true, SecretRef: "vault.external/db#apiKey", Val: "vault.external/db#apiKey"}, + Normal: *NewSecretVarFromRef("vault.bifrost/m/1/normal", "vault.bifrost/m/1/normal"), + Fragment: *NewSecretVarFromRef("vault.external/db#apiKey", "vault.external/db#apiKey"), } RemoveOwnedVaultSecretVars(context.Background(), "bifrost/m/1", m) @@ -119,7 +119,7 @@ func TestStoreOwnedVaultSecretVars_WalksFields(t *testing.T) { Plain: SecretVar{Val: "p1"}, Ptr: &SecretVar{Val: "p2"}, Snake: SecretVar{Val: "p3"}, - EnvBased: SecretVar{FromSecret: true, SecretRef: "env.X"}, + EnvBased: *NewSecretVarFromRef("env.X", ""), } if err := StoreOwnedVaultSecretVars(context.Background(), "bifrost/m/1", m); err != nil { @@ -139,8 +139,8 @@ func TestStoreOwnedVaultSecretVars_WalksFields(t *testing.T) { t.Errorf("stored[%q] = %q, want %q", path, stored[path], val) } } - if !m.Plain.FromSecret || !m.Ptr.FromSecret || !m.Snake.FromSecret { - t.Error("SecretVar fields should be marked FromSecret after store") + if !m.Plain.IsFromVault() || !m.Ptr.IsFromVault() || !m.Snake.IsFromVault() { + t.Error("SecretVar fields should be vault-backed after store") } } @@ -153,7 +153,7 @@ func TestStoreOwnedVaultSecretVars_WalksMap(t *testing.T) { m := &model{ Headers: map[string]SecretVar{ "Authorization": {Val: "secret-token"}, - "X-Env": {FromSecret: true, SecretRef: "env.X"}, + "X-Env": *NewSecretVarFromRef("env.X", ""), }, } @@ -168,11 +168,10 @@ func TestStoreOwnedVaultSecretVars_WalksMap(t *testing.T) { t.Errorf("stored Authorization = %q, want %q", got, "secret-token") } auth := m.Headers["Authorization"] - if !auth.FromSecret || auth.SecretRef != "vault.bifrost/m/1/headers/Authorization" { - t.Errorf("map entry not converted to ref: %+v", auth) + if !auth.IsFromVault() || auth.Ref() != "vault.bifrost/m/1/headers/Authorization" { + t.Errorf("map entry not converted to vault ref: val=%q ref=%q fromVault=%v", auth.Val, auth.Ref(), auth.IsFromVault()) } - if env := m.Headers["X-Env"]; env.FromSecret && env.SecretRef == "env.X" { - // still FromSecret=true (it was env-sourced), but it should NOT have been vault-stored + if env := m.Headers["X-Env"]; env.IsFromSecret() && env.Ref() == "env.X" { if stored["bifrost/m/1/headers/X-Env"] != "" { t.Error("env-sourced header should not be vault-stored") } @@ -193,8 +192,8 @@ func TestRemoveOwnedVaultSecretVars_WalksMap(t *testing.T) { } m := &model{ Headers: map[string]SecretVar{ - "Owned": {FromSecret: true, SecretRef: "vault.bifrost/m/1/headers/Owned", Val: "vault.bifrost/m/1/headers/Owned"}, - "External": {FromSecret: true, SecretRef: "vault.external/db#key", Val: "vault.external/db#key"}, + "Owned": *NewSecretVarFromRef("vault.bifrost/m/1/headers/Owned", "vault.bifrost/m/1/headers/Owned"), + "External": *NewSecretVarFromRef("vault.external/db#key", "vault.external/db#key"), }, } diff --git a/framework/configstore/clientconfig.go b/framework/configstore/clientconfig.go index 2c42a0869c..4f7b3084b6 100644 --- a/framework/configstore/clientconfig.go +++ b/framework/configstore/clientconfig.go @@ -360,7 +360,7 @@ func (c *ClientConfig) GenerateClientConfigHash() (string, error) { if c.MCPExternalClientURL.IsSet() { if c.MCPExternalClientURL.IsFromSecret() { - hash.Write([]byte("externalClientURL:ref:" + c.MCPExternalClientURL.SecretRef)) + hash.Write([]byte("externalClientURL:ref:" + c.MCPExternalClientURL.Ref())) } else { hash.Write([]byte("externalClientURL:val:" + c.MCPExternalClientURL.GetValue())) } @@ -652,7 +652,7 @@ func GenerateKeyHash(key schemas.Key) (string, error) { hash.Write([]byte(key.Name)) // Hash Value (prefix with source type to prevent collisions between ref and literal) if key.Value.IsFromSecret() { - hash.Write([]byte("ref:" + key.Value.SecretRef)) + hash.Write([]byte("ref:" + key.Value.Ref())) } else { hash.Write([]byte("val:" + key.Value.Val)) } @@ -1326,7 +1326,7 @@ func GenerateMCPClientHash(m tables.TableMCPClient) (string, error) { // Hash ConnectionString if m.ConnectionString != nil { if m.ConnectionString.IsFromSecret() { - hash.Write([]byte(m.ConnectionString.SecretRef)) + hash.Write([]byte(m.ConnectionString.Ref())) } else { hash.Write([]byte(m.ConnectionString.Val)) } @@ -1372,7 +1372,7 @@ func GenerateMCPClientHash(m tables.TableMCPClient) (string, error) { for _, k := range keys { val := m.Headers[k] if val.IsFromSecret() { - hash.Write([]byte(k + ":ref:" + val.SecretRef)) + hash.Write([]byte(k + ":ref:" + val.Ref())) } else { hash.Write([]byte(k + ":val:" + val.Val)) } diff --git a/framework/configstore/rdb.go b/framework/configstore/rdb.go index 73347c3efc..97bbcf811d 100644 --- a/framework/configstore/rdb.go +++ b/framework/configstore/rdb.go @@ -2044,7 +2044,7 @@ func (s *RDBConfigStore) UpdateMCPClientConfig(ctx context.Context, id string, c if clientConfigCopy.Headers != nil { for key, value := range clientConfigCopy.Headers { if value.IsFromSecret() { - headersToSerialize[key] = value.SecretRef + headersToSerialize[key] = value.Ref() } else { headersToSerialize[key] = value.GetValue() } diff --git a/framework/configstore/tables/mcp.go b/framework/configstore/tables/mcp.go index a9c4fdb2f5..bec9eefeaa 100644 --- a/framework/configstore/tables/mcp.go +++ b/framework/configstore/tables/mcp.go @@ -128,7 +128,7 @@ func (c *TableMCPClient) BeforeSave(tx *gorm.DB) error { headersToSerialize := make(map[string]string, len(c.Headers)) for key, value := range c.Headers { if value.IsFromSecret() { - headersToSerialize[key] = value.SecretRef + headersToSerialize[key] = value.Ref() } else { headersToSerialize[key] = value.GetValue() } diff --git a/transports/bifrost-http/handlers/config.go b/transports/bifrost-http/handlers/config.go index 09981d4fc8..84ed7c6fec 100644 --- a/transports/bifrost-http/handlers/config.go +++ b/transports/bifrost-http/handlers/config.go @@ -137,11 +137,7 @@ func (h *ConfigHandler) getConfig(ctx *fasthttp.RequestCtx) { // If not from env, show as the value var passwordSecretVar *schemas.SecretVar if authConfig.AdminPassword != nil && authConfig.AdminPassword.IsFromSecret() { - passwordSecretVar = &schemas.SecretVar{ - Val: "", - SecretRef: authConfig.AdminPassword.SecretRef, - FromSecret: true, - } + passwordSecretVar = authConfig.AdminPassword.FullyRedacted() } else { passwordSecretVar = &schemas.SecretVar{ Val: "", @@ -695,11 +691,11 @@ func (h *ConfigHandler) updateConfig(ctx *fasthttp.RequestCtx) { // Validate env variables are set if referenced if payload.AuthConfig.AdminUserName.IsFromSecret() && payload.AuthConfig.AdminUserName.GetValue() == "" { - SendError(ctx, fasthttp.StatusBadRequest, fmt.Sprintf("external reference %s for admin_username resolved to an empty value", payload.AuthConfig.AdminUserName.SecretRef)) + SendError(ctx, fasthttp.StatusBadRequest, fmt.Sprintf("external reference %s for admin_username resolved to an empty value", payload.AuthConfig.AdminUserName.Ref())) return } if payload.AuthConfig.AdminPassword.IsFromSecret() && payload.AuthConfig.AdminPassword.GetValue() == "" { - SendError(ctx, fasthttp.StatusBadRequest, fmt.Sprintf("external reference %s for admin_password resolved to an empty value", payload.AuthConfig.AdminPassword.SecretRef)) + SendError(ctx, fasthttp.StatusBadRequest, fmt.Sprintf("external reference %s for admin_password resolved to an empty value", payload.AuthConfig.AdminPassword.Ref())) return } @@ -726,10 +722,12 @@ func (h *ConfigHandler) updateConfig(ctx *fasthttp.RequestCtx) { return } // Preserve env/vault reference metadata when storing hashed password - payload.AuthConfig.AdminPassword = &schemas.SecretVar{ - Val: hashedPassword, - SecretRef: payload.AuthConfig.AdminPassword.SecretRef, - FromSecret: payload.AuthConfig.AdminPassword.IsFromSecret(), + if payload.AuthConfig.AdminPassword.IsFromSecret() { + sv := *payload.AuthConfig.AdminPassword + sv.Val = hashedPassword + payload.AuthConfig.AdminPassword = &sv + } else { + payload.AuthConfig.AdminPassword = &schemas.SecretVar{Val: hashedPassword} } } } diff --git a/transports/bifrost-http/lib/config.go b/transports/bifrost-http/lib/config.go index 21edade6cd..1ceabefa6e 100644 --- a/transports/bifrost-http/lib/config.go +++ b/transports/bifrost-http/lib/config.go @@ -3643,11 +3643,12 @@ func preserveSecretVar(source *schemas.SecretVar, value string) *schemas.SecretV if source == nil { return schemas.NewSecretVar(value) } - return &schemas.SecretVar{ - Val: value, - SecretRef: source.SecretRef, - FromSecret: source.IsFromSecret(), + if source.IsFromSecret() { + sv := *source + sv.Val = value + return &sv } + return &schemas.SecretVar{Val: value} } // loadAuthConfig loads auth config from file. @@ -3692,18 +3693,10 @@ func loadAuthConfig(ctx context.Context, config *Config, configData *ConfigData) } // Fail-closed: if env/vault reference is unresolved, don't persist empty credentials. if authConfig.AdminUserName != nil && authConfig.AdminUserName.GetValue() == "" && authConfig.AdminUserName.IsFromSecret() { - logger.Warn("username set with external reference but value is empty: %s — skipping auth config", authConfig.AdminUserName.SecretRef) - if dbAuthConfig != nil { - config.GovernanceConfig.AuthConfig = dbAuthConfig - } - return + logger.Warn("username set with external reference but value is empty: %s", authConfig.AdminUserName.Ref()) } if authConfig.AdminPassword != nil && authConfig.AdminPassword.GetValue() == "" && authConfig.AdminPassword.IsFromSecret() { - logger.Warn("password set with external reference but value is empty: %s — skipping auth config", authConfig.AdminPassword.SecretRef) - if dbAuthConfig != nil { - config.GovernanceConfig.AuthConfig = dbAuthConfig - } - return + logger.Warn("password set with external reference but value is empty: %s", authConfig.AdminPassword.Ref()) } if authConfig.AdminPassword == nil || authConfig.AdminUserName == nil { logger.Warn("auth config is missing admin_username or admin_password, skipping auth config processing") From f6e7cf691ac87dca387a5199667a6014c933e14c Mon Sep 17 00:00:00 2001 From: Anuj Parihar Date: Mon, 22 Jun 2026 13:02:16 +0530 Subject: [PATCH 2/3] refactor: change ref -> secretRef --- core/providers/utils/utils.go | 8 +-- core/schemas/secretvar.go | 10 +-- core/schemas/secretvar_test.go | 60 ++++++++-------- core/schemas/vault_test.go | 30 ++++---- framework/configstore/clientconfig.go | 8 +-- .../clientconfig_redaction_test.go | 30 ++++---- framework/configstore/encryption_test.go | 58 ++++++++-------- framework/configstore/rdb.go | 2 +- .../configstore/tables/encryption_test.go | 68 ++++++------------- framework/configstore/tables/mcp.go | 2 +- .../tables/virtualkey_secretvar_test.go | 18 ++--- transports/bifrost-http/handlers/config.go | 4 +- transports/bifrost-http/lib/config.go | 4 +- transports/bifrost-http/lib/config_test.go | 22 +++--- transports/bifrost-http/lib/ctx_test.go | 4 +- transports/config.schema.json | 2 + 16 files changed, 152 insertions(+), 178 deletions(-) diff --git a/core/providers/utils/utils.go b/core/providers/utils/utils.go index 305fcc0090..52443f57f8 100644 --- a/core/providers/utils/utils.go +++ b/core/providers/utils/utils.go @@ -344,7 +344,7 @@ func ConfigureProxy(client *fasthttp.Client, proxyConfig *schemas.ProxyConfig, l return client case schemas.HTTPProxy: if proxyConfig.URL != nil && proxyConfig.URL.IsFromSecret() && proxyConfig.URL.GetValue() == "" { - errMsg := fmt.Sprintf("invalid proxy configuration: %s references %q but it resolved to an empty value", "proxy.url", proxyConfig.URL.Ref()) + errMsg := fmt.Sprintf("invalid proxy configuration: %s references %q but it resolved to an empty value", "proxy.url", proxyConfig.URL.GetSecretRef()) getLogger().Error(errMsg) client.Dial = dialErrorFunc(errMsg) return client @@ -370,7 +370,7 @@ func ConfigureProxy(client *fasthttp.Client, proxyConfig *schemas.ProxyConfig, l dialFunc = fasthttpproxy.FasthttpHTTPDialer(proxyURL) case schemas.Socks5Proxy: if proxyConfig.URL != nil && proxyConfig.URL.IsFromSecret() && proxyConfig.URL.GetValue() == "" { - errMsg := fmt.Sprintf("invalid proxy configuration: %s references %q but it resolved to an empty value", "proxy.url", proxyConfig.URL.Ref()) + errMsg := fmt.Sprintf("invalid proxy configuration: %s references %q but it resolved to an empty value", "proxy.url", proxyConfig.URL.GetSecretRef()) getLogger().Error(errMsg) client.Dial = dialErrorFunc(errMsg) return client @@ -409,7 +409,7 @@ func ConfigureProxy(client *fasthttp.Client, proxyConfig *schemas.ProxyConfig, l // Configure custom CA certificate if provided if proxyConfig.CACertPEM != nil && proxyConfig.CACertPEM.IsFromSecret() && proxyConfig.CACertPEM.GetValue() == "" { - errMsg := fmt.Sprintf("invalid proxy configuration: %s references %q but it resolved to an empty value", "proxy.ca_cert_pem", proxyConfig.CACertPEM.Ref()) + errMsg := fmt.Sprintf("invalid proxy configuration: %s references %q but it resolved to an empty value", "proxy.ca_cert_pem", proxyConfig.CACertPEM.GetSecretRef()) getLogger().Error(errMsg) client.Dial = dialErrorFunc(errMsg) return client @@ -452,7 +452,7 @@ func createTLSConfigWithCA(caCertPEM string) (*tls.Config, error) { // It merges with any existing TLSConfig (e.g., from ConfigureProxy). func ConfigureTLS(client *fasthttp.Client, networkConfig schemas.NetworkConfig, logger schemas.Logger) *fasthttp.Client { if networkConfig.CACertPEM != nil && networkConfig.CACertPEM.IsFromSecret() && networkConfig.CACertPEM.GetValue() == "" { - errMsg := fmt.Sprintf("invalid provider configuration: %s references %q but it resolved to an empty value", "network_config.ca_cert_pem", networkConfig.CACertPEM.Ref()) + errMsg := fmt.Sprintf("invalid provider configuration: %s references %q but it resolved to an empty value", "network_config.ca_cert_pem", networkConfig.CACertPEM.GetSecretRef()) logger.Error(errMsg) client.Dial = dialErrorFunc(errMsg) return client diff --git a/core/schemas/secretvar.go b/core/schemas/secretvar.go index 2a3d55233f..b1b9e7911a 100644 --- a/core/schemas/secretvar.go +++ b/core/schemas/secretvar.go @@ -29,9 +29,7 @@ func NewSecretVar(value string) *SecretVar { // If it's a valid JSON object following the SecretVar schema, unmarshal it if sonic.Valid([]byte(value)) { valueNode, _ := sonic.Get([]byte(val), "value") - envNode, _ := sonic.Get([]byte(val), "env_var") - secretRefNode, _ := sonic.Get([]byte(val), "secret_ref") - if valueNode.Exists() && (envNode.Exists() || secretRefNode.Exists()) { + if valueNode.Exists() { type secretVarCompat struct { Val string `json:"value"` SecretRef string `json:"secret_ref"` @@ -124,7 +122,7 @@ func NewSecretVar(value string) *SecretVar { // Ref returns the secret reference string (e.g. "env.MY_VAR" or "vault.path/to/secret"). // Returns an empty string for plain-value SecretVars. -func (e *SecretVar) Ref() string { +func (e *SecretVar) GetSecretRef() string { if e == nil { return "" } @@ -268,9 +266,7 @@ func (e *SecretVar) UnmarshalJSON(data []byte) error { } if sonic.Valid(data) { valueNode, _ := sonic.Get(data, "value") - envNode, _ := sonic.Get(data, "env_var") - secretRefNode, _ := sonic.Get(data, "secret_ref") - if valueNode.Exists() && (envNode.Exists() || secretRefNode.Exists()) { + if valueNode.Exists() { type secretVarCompat struct { Val string `json:"value"` SecretRef string `json:"secret_ref"` diff --git a/core/schemas/secretvar_test.go b/core/schemas/secretvar_test.go index ecbfa472ba..a6a24763c7 100644 --- a/core/schemas/secretvar_test.go +++ b/core/schemas/secretvar_test.go @@ -98,8 +98,8 @@ func TestSecretVar_UnmarshalJSON_SecretVarReference(t *testing.T) { if secretVar.Val != tt.expectedVal { t.Errorf("Expected Val=%q, got Val=%q", tt.expectedVal, secretVar.Val) } - if secretVar.Ref() != tt.expectedSecretRef { - t.Errorf("Expected Ref()=%q, got %q", tt.expectedSecretRef, secretVar.Ref()) + if secretVar.GetSecretRef() != tt.expectedSecretRef { + t.Errorf("Expected Ref()=%q, got %q", tt.expectedSecretRef, secretVar.GetSecretRef()) } if secretVar.IsFromSecret() != tt.expectedFromSecret { t.Errorf("Expected IsFromSecret()=%v, got %v", tt.expectedFromSecret, secretVar.IsFromSecret()) @@ -121,8 +121,8 @@ func TestSecretVar_UnmarshalJSON_BackwardCompat(t *testing.T) { if err != nil { t.Fatalf("UnmarshalJSON failed: %v", err) } - if secretVar.Ref() != "env.MY_KEY" { - t.Errorf("Expected Ref()=%q, got %q", "env.MY_KEY", secretVar.Ref()) + if secretVar.GetSecretRef() != "env.MY_KEY" { + t.Errorf("Expected Ref()=%q, got %q", "env.MY_KEY", secretVar.GetSecretRef()) } if !secretVar.IsFromSecret() { t.Error("Expected IsFromSecret()=true, got false") @@ -211,8 +211,8 @@ func TestNewSecretVar_SecretVarReference(t *testing.T) { if secretVar.Val != tt.expectedVal { t.Errorf("Expected Val=%q, got Val=%q", tt.expectedVal, secretVar.Val) } - if secretVar.Ref() != tt.expectedRef { - t.Errorf("Expected Ref()=%q, got %q", tt.expectedRef, secretVar.Ref()) + if secretVar.GetSecretRef() != tt.expectedRef { + t.Errorf("Expected Ref()=%q, got %q", tt.expectedRef, secretVar.GetSecretRef()) } if secretVar.IsFromSecret() != tt.expectedFromSecret { t.Errorf("Expected IsFromSecret()=%v, got %v", tt.expectedFromSecret, secretVar.IsFromSecret()) @@ -319,8 +319,8 @@ func TestSecretVar_Equals(t *testing.T) { }, { name: "equal values", - a: NewSecretVarFromRef("env.TEST", "test"), - b: NewSecretVarFromRef("env.TEST", "test"), + a: &SecretVar{Val: "test", secretRef: "env.TEST", fromSecret: true}, + b: &SecretVar{Val: "test", secretRef: "env.TEST", fromSecret: true}, expected: true, }, { @@ -403,7 +403,7 @@ func TestSecretVar_FullyRedacted(t *testing.T) { }, { name: "resolved env password preserves reference metadata", - input: *NewSecretVarFromRef("env.PROXY_PASS", "resolved-secret"), + input: SecretVar{Val: "resolved-secret", secretRef: "env.PROXY_PASS", fromSecret: true}, wantVal: "", wantFromSecret: true, wantRef: "env.PROXY_PASS", @@ -419,8 +419,8 @@ func TestSecretVar_FullyRedacted(t *testing.T) { if result.IsFromSecret() != tt.wantFromSecret { t.Errorf("IsFromSecret(): want %v, got %v", tt.wantFromSecret, result.IsFromSecret()) } - if result.Ref() != tt.wantRef { - t.Errorf("Ref(): want %q, got %q", tt.wantRef, result.Ref()) + if result.GetSecretRef() != tt.wantRef { + t.Errorf("Ref(): want %q, got %q", tt.wantRef, result.GetSecretRef()) } }) } @@ -439,7 +439,7 @@ func TestSecretVar_IsRedacted(t *testing.T) { }, { name: "from secret", - input: *NewSecretVarFromRef("env.KEY", "test"), + input: SecretVar{Val: "test", secretRef: "env.KEY", fromSecret: true}, expected: true, }, { @@ -500,22 +500,22 @@ func TestSecretVar_IsSet(t *testing.T) { }, { name: "env reference not yet resolved", - input: NewSecretVarFromRef("env.MISSING", ""), + input: &SecretVar{secretRef: "env.MISSING", fromSecret: true}, expected: true, }, { name: "env reference resolved", - input: NewSecretVarFromRef("env.X", "resolved-secret"), + input: &SecretVar{Val: "resolved-secret", secretRef: "env.X", fromSecret: true}, expected: true, }, { name: "fromSecret true but no reference and no value", - input: NewSecretVarFromRef("", ""), + input: &SecretVar{fromSecret: true}, expected: false, }, { name: "vault reference set", - input: NewSecretVarFromRef("vault.bifrost/key", "vault.bifrost/key"), + input: &SecretVar{Val: "vault.bifrost/key", secretRef: "vault.bifrost/key", fromSecret: true}, expected: true, }, } @@ -571,8 +571,8 @@ func TestSecretVar_Scan_VaultRef(t *testing.T) { if e.IsFromSecret() != tt.wantFromSecret { t.Errorf("IsFromSecret() = %v, want %v", e.IsFromSecret(), tt.wantFromSecret) } - if tt.wantFromSecret && e.Ref() != tt.wantRef { - t.Errorf("Ref() = %q, want %q", e.Ref(), tt.wantRef) + if tt.wantFromSecret && e.GetSecretRef() != tt.wantRef { + t.Errorf("Ref() = %q, want %q", e.GetSecretRef(), tt.wantRef) } if tt.wantVal != "" && e.Val != tt.wantVal { t.Errorf("Val = %q, want %q", e.Val, tt.wantVal) @@ -614,8 +614,8 @@ func TestSecretVar_UnmarshalJSON_VaultRef(t *testing.T) { if e.IsFromSecret() != tt.wantFromSecret { t.Errorf("IsFromSecret() = %v, want %v", e.IsFromSecret(), tt.wantFromSecret) } - if e.Ref() != tt.wantRef { - t.Errorf("Ref() = %q, want %q", e.Ref(), tt.wantRef) + if e.GetSecretRef() != tt.wantRef { + t.Errorf("Ref() = %q, want %q", e.GetSecretRef(), tt.wantRef) } if e.Val != tt.wantVal { t.Errorf("Val = %q, want %q", e.Val, tt.wantVal) @@ -625,7 +625,7 @@ func TestSecretVar_UnmarshalJSON_VaultRef(t *testing.T) { } func TestSecretVar_Value_VaultRef(t *testing.T) { - e := NewSecretVarFromRef("vault.bifrost/key", "actual-secret") + e := &SecretVar{Val: "actual-secret", secretRef: "vault.bifrost/key", fromSecret: true} got, err := e.Value() if err != nil { t.Fatalf("Value() error: %v", err) @@ -636,7 +636,7 @@ func TestSecretVar_Value_VaultRef(t *testing.T) { } func TestSecretVar_Redacted_VaultRef(t *testing.T) { - e := NewSecretVarFromRef("vault.bifrost/key", "actual-secret-value") + e := &SecretVar{Val: "actual-secret-value", secretRef: "vault.bifrost/key", fromSecret: true} r := e.Redacted() wantVal := "actu************************alue" if r.Val != wantVal { @@ -645,14 +645,14 @@ func TestSecretVar_Redacted_VaultRef(t *testing.T) { if !r.IsFromSecret() { t.Error("Redacted().IsFromSecret() = false, want true") } - if r.Ref() != "vault.bifrost/key" { - t.Errorf("Redacted().Ref() = %q, want %q", r.Ref(), "vault.bifrost/key") + if r.GetSecretRef() != "vault.bifrost/key" { + t.Errorf("Redacted().GetSecretRef() = %q, want %q", r.GetSecretRef(), "vault.bifrost/key") } } func TestSecretVar_IsSet_VaultRef(t *testing.T) { - set := NewSecretVarFromRef("vault.bifrost/key", "vault.bifrost/key") - unset := NewSecretVarFromRef("", "") + set := &SecretVar{Val: "vault.bifrost/key", secretRef: "vault.bifrost/key", fromSecret: true} + unset := &SecretVar{fromSecret: true} if !set.IsSet() { t.Error("IsSet() = false for vault ref, want true") } @@ -666,8 +666,8 @@ func TestNewSecretVar_VaultRef(t *testing.T) { if !e.IsFromVault() { t.Error("IsFromVault() = false, want true") } - if e.Ref() != "vault.bifrost/mykey" { - t.Errorf("Ref() = %q, want %q", e.Ref(), "vault.bifrost/mykey") + if e.GetSecretRef() != "vault.bifrost/mykey" { + t.Errorf("Ref() = %q, want %q", e.GetSecretRef(), "vault.bifrost/mykey") } if e.Val != "" { t.Errorf("Val = %q, want empty string when vault lookup fails", e.Val) @@ -687,12 +687,12 @@ func TestSecretVar_MarshalJSON(t *testing.T) { }, { name: "env reference", - input: *NewSecretVarFromRef("env.MY_KEY", "resolved"), + input: SecretVar{Val: "resolved", secretRef: "env.MY_KEY", fromSecret: true}, want: `{"value":"resolved","secret_ref":"env.MY_KEY","from_secret":true}`, }, { name: "vault reference", - input: *NewSecretVarFromRef("vault.path/to/secret", ""), + input: SecretVar{secretRef: "vault.path/to/secret", fromSecret: true}, want: `{"value":"","secret_ref":"vault.path/to/secret","from_secret":true}`, }, } diff --git a/core/schemas/vault_test.go b/core/schemas/vault_test.go index b34185b697..99b4a8022a 100644 --- a/core/schemas/vault_test.go +++ b/core/schemas/vault_test.go @@ -34,8 +34,8 @@ func TestStoreVaultSecretVar_StoresPlaintext(t *testing.T) { if !e.IsFromVault() { t.Error("IsFromVault() should be true after store") } - if e.Ref() != "vault.bifrost/tbl/id/value" { - t.Errorf("Ref() = %q, want %q", e.Ref(), "vault.bifrost/tbl/id/value") + if e.GetSecretRef() != "vault.bifrost/tbl/id/value" { + t.Errorf("Ref() = %q, want %q", e.GetSecretRef(), "vault.bifrost/tbl/id/value") } if e.Val != "vault.bifrost/tbl/id/value" { t.Errorf("Val = %q, want rewritten to vault ref", e.Val) @@ -48,8 +48,8 @@ func TestStoreVaultSecretVar_NoOps(t *testing.T) { e *SecretVar }{ {"nil", nil}, - {"env-sourced", NewSecretVarFromRef("env.MY_VAR", "")}, - {"already-vault", NewSecretVarFromRef("vault.some/path", "vault.some/path")}, + {"env-sourced", &SecretVar{secretRef: "env.MY_VAR", fromSecret: true}}, + {"already-vault", &SecretVar{Val: "vault.some/path", secretRef: "vault.some/path", fromSecret: true}}, {"empty", &SecretVar{Val: ""}}, } for _, tc := range cases { @@ -74,8 +74,8 @@ func TestStoreVaultSecretVar_NoHookNoOp(t *testing.T) { if err := StoreVaultSecretVar(context.Background(), "p", e); err != nil { t.Fatalf("StoreVaultSecretVar: %v", err) } - if e.IsFromSecret() || e.Ref() != "" || e.Val != "secret" { - t.Errorf("expected no mutation when hook nil, got val=%q ref=%q fromSecret=%v", e.Val, e.Ref(), e.IsFromSecret()) + if e.IsFromSecret() || e.GetSecretRef() != "" || e.Val != "secret" { + t.Errorf("expected no mutation when hook nil, got val=%q ref=%q fromSecret=%v", e.Val, e.GetSecretRef(), e.IsFromSecret()) } } @@ -93,8 +93,8 @@ func TestRemoveOwnedVaultSecretVars_SkipsFragmentRefs(t *testing.T) { Fragment SecretVar `gorm:"column:fragment"` } m := &model{ - Normal: *NewSecretVarFromRef("vault.bifrost/m/1/normal", "vault.bifrost/m/1/normal"), - Fragment: *NewSecretVarFromRef("vault.external/db#apiKey", "vault.external/db#apiKey"), + Normal: SecretVar{Val: "vault.bifrost/m/1/normal", secretRef: "vault.bifrost/m/1/normal", fromSecret: true}, + Fragment: SecretVar{Val: "vault.external/db#apiKey", secretRef: "vault.external/db#apiKey", fromSecret: true}, } RemoveOwnedVaultSecretVars(context.Background(), "bifrost/m/1", m) @@ -119,7 +119,7 @@ func TestStoreOwnedVaultSecretVars_WalksFields(t *testing.T) { Plain: SecretVar{Val: "p1"}, Ptr: &SecretVar{Val: "p2"}, Snake: SecretVar{Val: "p3"}, - EnvBased: *NewSecretVarFromRef("env.X", ""), + EnvBased: SecretVar{secretRef: "env.X", fromSecret: true}, } if err := StoreOwnedVaultSecretVars(context.Background(), "bifrost/m/1", m); err != nil { @@ -153,7 +153,7 @@ func TestStoreOwnedVaultSecretVars_WalksMap(t *testing.T) { m := &model{ Headers: map[string]SecretVar{ "Authorization": {Val: "secret-token"}, - "X-Env": *NewSecretVarFromRef("env.X", ""), + "X-Env": SecretVar{secretRef: "env.X", fromSecret: true}, }, } @@ -168,10 +168,10 @@ func TestStoreOwnedVaultSecretVars_WalksMap(t *testing.T) { t.Errorf("stored Authorization = %q, want %q", got, "secret-token") } auth := m.Headers["Authorization"] - if !auth.IsFromVault() || auth.Ref() != "vault.bifrost/m/1/headers/Authorization" { - t.Errorf("map entry not converted to vault ref: val=%q ref=%q fromVault=%v", auth.Val, auth.Ref(), auth.IsFromVault()) + if !auth.IsFromVault() || auth.GetSecretRef() != "vault.bifrost/m/1/headers/Authorization" { + t.Errorf("map entry not converted to vault ref: val=%q ref=%q fromVault=%v", auth.Val, auth.GetSecretRef(), auth.IsFromVault()) } - if env := m.Headers["X-Env"]; env.IsFromSecret() && env.Ref() == "env.X" { + if env := m.Headers["X-Env"]; env.IsFromSecret() && env.GetSecretRef() == "env.X" { if stored["bifrost/m/1/headers/X-Env"] != "" { t.Error("env-sourced header should not be vault-stored") } @@ -192,8 +192,8 @@ func TestRemoveOwnedVaultSecretVars_WalksMap(t *testing.T) { } m := &model{ Headers: map[string]SecretVar{ - "Owned": *NewSecretVarFromRef("vault.bifrost/m/1/headers/Owned", "vault.bifrost/m/1/headers/Owned"), - "External": *NewSecretVarFromRef("vault.external/db#key", "vault.external/db#key"), + "Owned": SecretVar{Val: "vault.bifrost/m/1/headers/Owned", secretRef: "vault.bifrost/m/1/headers/Owned", fromSecret: true}, + "External": SecretVar{Val: "vault.external/db#key", secretRef: "vault.external/db#key", fromSecret: true}, }, } diff --git a/framework/configstore/clientconfig.go b/framework/configstore/clientconfig.go index 4f7b3084b6..31ed38beea 100644 --- a/framework/configstore/clientconfig.go +++ b/framework/configstore/clientconfig.go @@ -360,7 +360,7 @@ func (c *ClientConfig) GenerateClientConfigHash() (string, error) { if c.MCPExternalClientURL.IsSet() { if c.MCPExternalClientURL.IsFromSecret() { - hash.Write([]byte("externalClientURL:ref:" + c.MCPExternalClientURL.Ref())) + hash.Write([]byte("externalClientURL:ref:" + c.MCPExternalClientURL.GetSecretRef())) } else { hash.Write([]byte("externalClientURL:val:" + c.MCPExternalClientURL.GetValue())) } @@ -652,7 +652,7 @@ func GenerateKeyHash(key schemas.Key) (string, error) { hash.Write([]byte(key.Name)) // Hash Value (prefix with source type to prevent collisions between ref and literal) if key.Value.IsFromSecret() { - hash.Write([]byte("ref:" + key.Value.Ref())) + hash.Write([]byte("ref:" + key.Value.GetSecretRef())) } else { hash.Write([]byte("val:" + key.Value.Val)) } @@ -1326,7 +1326,7 @@ func GenerateMCPClientHash(m tables.TableMCPClient) (string, error) { // Hash ConnectionString if m.ConnectionString != nil { if m.ConnectionString.IsFromSecret() { - hash.Write([]byte(m.ConnectionString.Ref())) + hash.Write([]byte(m.ConnectionString.GetSecretRef())) } else { hash.Write([]byte(m.ConnectionString.Val)) } @@ -1372,7 +1372,7 @@ func GenerateMCPClientHash(m tables.TableMCPClient) (string, error) { for _, k := range keys { val := m.Headers[k] if val.IsFromSecret() { - hash.Write([]byte(k + ":ref:" + val.Ref())) + hash.Write([]byte(k + ":ref:" + val.GetSecretRef())) } else { hash.Write([]byte(k + ":val:" + val.Val)) } diff --git a/framework/configstore/clientconfig_redaction_test.go b/framework/configstore/clientconfig_redaction_test.go index 76ab210144..5eee812ba9 100644 --- a/framework/configstore/clientconfig_redaction_test.go +++ b/framework/configstore/clientconfig_redaction_test.go @@ -17,7 +17,7 @@ func TestProviderConfig_Redacted_AutoMasksEnvBackedFields(t *testing.T) { t.Setenv("MY_AZURE_ENDPOINT_SECRET", "https://secret-resource.openai.azure.com") endpoint := schemas.NewSecretVar("env.MY_AZURE_ENDPOINT_SECRET") - require.True(t, endpoint.IsFromEnv(), "setup: Endpoint should be FromEnv") + require.True(t, endpoint.IsFromSecret(), "setup: Endpoint should be FromSecret") require.Equal(t, "https://secret-resource.openai.azure.com", endpoint.GetValue(), "setup: Endpoint should be resolved") @@ -42,17 +42,17 @@ func TestProviderConfig_Redacted_AutoMasksEnvBackedFields(t *testing.T) { require.NoError(t, err) var out struct { - Value string `json:"value"` - EnvVar string `json:"env_var"` - FromEnv bool `json:"from_env"` + Value string `json:"value"` + SecretRef string `json:"secret_ref"` + FromSecret bool `json:"from_secret"` } require.NoError(t, json.Unmarshal(data, &out)) assert.NotContains(t, out.Value, "secret-resource", "resolved env value leaked through Endpoint JSON output: %q", out.Value) - assert.Equal(t, "env.MY_AZURE_ENDPOINT_SECRET", out.EnvVar, - "env var reference must be preserved so the UI can show it") - assert.True(t, out.FromEnv, "from_env flag must be preserved") + assert.Equal(t, "env.MY_AZURE_ENDPOINT_SECRET", out.SecretRef, + "secret ref must be preserved so the UI can show it") + assert.True(t, out.FromSecret, "from_secret flag must be preserved") } // TestProviderConfig_Redacted_DoesNotMaskPlainNonSecretFields verifies that the @@ -79,14 +79,14 @@ func TestProviderConfig_Redacted_DoesNotMaskPlainNonSecretFields(t *testing.T) { require.NoError(t, err) var out struct { - Value string `json:"value"` - FromEnv bool `json:"from_env"` + Value string `json:"value"` + FromSecret bool `json:"from_secret"` } require.NoError(t, json.Unmarshal(data, &out)) assert.Equal(t, "https://foo.openai.azure.com", out.Value, "plain Endpoint was incorrectly redacted") - assert.False(t, out.FromEnv) + assert.False(t, out.FromSecret) } // TestProviderConfig_Redacted_PreservesSecretVarReferenceForVertex verifies that @@ -116,16 +116,16 @@ func TestProviderConfig_Redacted_PreservesSecretVarReferenceForVertex(t *testing require.NoError(t, err) var out struct { - Value string `json:"value"` - EnvVar string `json:"env_var"` - FromEnv bool `json:"from_env"` + Value string `json:"value"` + SecretRef string `json:"secret_ref"` + FromSecret bool `json:"from_secret"` } require.NoError(t, json.Unmarshal(data, &out)) assert.NotContains(t, out.Value, "super-secret-project", "resolved Vertex ProjectID env value leaked: %q", out.Value) - assert.Equal(t, "env.MY_VERTEX_PROJECT_ID_SECRET", out.EnvVar) - assert.True(t, out.FromEnv) + assert.Equal(t, "env.MY_VERTEX_PROJECT_ID_SECRET", out.SecretRef) + assert.True(t, out.FromSecret) } // TestProviderConfig_Redacted_DoesNotMutateOriginal ensures Redacted() does not diff --git a/framework/configstore/encryption_test.go b/framework/configstore/encryption_test.go index 40e5cbf9b8..60233f88b5 100644 --- a/framework/configstore/encryption_test.go +++ b/framework/configstore/encryption_test.go @@ -428,7 +428,7 @@ func TestEncryptPlaintextVirtualKeys_EncryptsAndDecryptsCorrectly(t *testing.T) // GORM hooks should decrypt on read var found tables.TableVirtualKey require.NoError(t, db.Where("id = ?", "vk-batch-1").First(&found).Error) - assert.Equal(t, "vk-batch-secret", found.Value) + assert.Equal(t, "vk-batch-secret", found.Value.GetValue()) } func TestEncryptPlaintextOAuthConfigs_EncryptsAndDecryptsCorrectly(t *testing.T) { @@ -986,11 +986,11 @@ func TestBeforeSave_SecretVarBackedFields_NotEncrypted(t *testing.T) { } // Verify the SecretVars resolved correctly and are marked as FromEnv - require.True(t, azureCfg.Endpoint.IsFromEnv()) + require.True(t, azureCfg.Endpoint.IsFromSecret()) require.Equal(t, "https://env-resource.openai.azure.com", azureCfg.Endpoint.GetValue()) - require.True(t, azureCfg.ClientSecret.IsFromEnv()) - require.True(t, vertexCfg.AuthCredentials.IsFromEnv()) - require.True(t, bedrockCfg.AccessKey.IsFromEnv()) + require.True(t, azureCfg.ClientSecret.IsFromSecret()) + require.True(t, vertexCfg.AuthCredentials.IsFromSecret()) + require.True(t, bedrockCfg.AccessKey.IsFromSecret()) key := &tables.TableKey{ Name: "env-backed-key", @@ -1016,53 +1016,53 @@ func TestBeforeSave_SecretVarBackedFields_NotEncrypted(t *testing.T) { // The shared config structs must NOT be mutated assert.Equal(t, "https://env-resource.openai.azure.com", azureCfg.Endpoint.GetValue()) - assert.True(t, azureCfg.Endpoint.IsFromEnv()) + assert.True(t, azureCfg.Endpoint.IsFromSecret()) assert.Equal(t, "env-azure-client-secret", azureCfg.ClientSecret.GetValue()) - assert.True(t, azureCfg.ClientSecret.IsFromEnv()) + assert.True(t, azureCfg.ClientSecret.IsFromSecret()) assert.Equal(t, "env-vertex-creds-json", vertexCfg.AuthCredentials.GetValue()) - assert.True(t, vertexCfg.AuthCredentials.IsFromEnv()) + assert.True(t, vertexCfg.AuthCredentials.IsFromSecret()) assert.Equal(t, "env-AKIA-ACCESS", bedrockCfg.AccessKey.GetValue()) - assert.True(t, bedrockCfg.AccessKey.IsFromEnv()) + assert.True(t, bedrockCfg.AccessKey.IsFromSecret()) assert.Equal(t, "env-bedrock-secret", bedrockCfg.SecretKey.GetValue()) - assert.True(t, bedrockCfg.SecretKey.IsFromEnv()) + assert.True(t, bedrockCfg.SecretKey.IsFromSecret()) assert.Equal(t, "env-bedrock-session", bedrockCfg.SessionToken.GetValue()) - assert.True(t, bedrockCfg.SessionToken.IsFromEnv()) + assert.True(t, bedrockCfg.SessionToken.IsFromSecret()) // GORM round-trip: AfterFind should reconstruct env-backed SecretVars correctly var found tables.TableKey require.NoError(t, db.Where("name = ?", "env-backed-key").First(&found).Error) assert.Equal(t, "sk-azure-from-env", found.Value.GetValue()) - assert.True(t, found.Value.IsFromEnv()) + assert.True(t, found.Value.IsFromSecret()) require.NotNil(t, found.AzureKeyConfig) assert.Equal(t, "https://env-resource.openai.azure.com", found.AzureKeyConfig.Endpoint.GetValue()) - assert.True(t, found.AzureKeyConfig.Endpoint.IsFromEnv()) + assert.True(t, found.AzureKeyConfig.Endpoint.IsFromSecret()) assert.Equal(t, "env-azure-client-secret", found.AzureKeyConfig.ClientSecret.GetValue()) - assert.True(t, found.AzureKeyConfig.ClientSecret.IsFromEnv()) + assert.True(t, found.AzureKeyConfig.ClientSecret.IsFromSecret()) assert.Equal(t, "env-azure-client-id", found.AzureKeyConfig.ClientID.GetValue()) - assert.True(t, found.AzureKeyConfig.ClientID.IsFromEnv()) + assert.True(t, found.AzureKeyConfig.ClientID.IsFromSecret()) assert.Equal(t, "env-azure-tenant-id", found.AzureKeyConfig.TenantID.GetValue()) - assert.True(t, found.AzureKeyConfig.TenantID.IsFromEnv()) + assert.True(t, found.AzureKeyConfig.TenantID.IsFromSecret()) require.NotNil(t, found.VertexKeyConfig) assert.Equal(t, "env-vertex-project", found.VertexKeyConfig.ProjectID.GetValue()) - assert.True(t, found.VertexKeyConfig.ProjectID.IsFromEnv()) + assert.True(t, found.VertexKeyConfig.ProjectID.IsFromSecret()) assert.Equal(t, "env-us-central1", found.VertexKeyConfig.Region.GetValue()) - assert.True(t, found.VertexKeyConfig.Region.IsFromEnv()) + assert.True(t, found.VertexKeyConfig.Region.IsFromSecret()) assert.Equal(t, "env-vertex-creds-json", found.VertexKeyConfig.AuthCredentials.GetValue()) - assert.True(t, found.VertexKeyConfig.AuthCredentials.IsFromEnv()) + assert.True(t, found.VertexKeyConfig.AuthCredentials.IsFromSecret()) require.NotNil(t, found.BedrockKeyConfig) assert.Equal(t, "env-AKIA-ACCESS", found.BedrockKeyConfig.AccessKey.GetValue()) - assert.True(t, found.BedrockKeyConfig.AccessKey.IsFromEnv()) + assert.True(t, found.BedrockKeyConfig.AccessKey.IsFromSecret()) assert.Equal(t, "env-bedrock-secret", found.BedrockKeyConfig.SecretKey.GetValue()) - assert.True(t, found.BedrockKeyConfig.SecretKey.IsFromEnv()) + assert.True(t, found.BedrockKeyConfig.SecretKey.IsFromSecret()) assert.Equal(t, "env-bedrock-session", found.BedrockKeyConfig.SessionToken.GetValue()) - assert.True(t, found.BedrockKeyConfig.SessionToken.IsFromEnv()) + assert.True(t, found.BedrockKeyConfig.SessionToken.IsFromSecret()) assert.Equal(t, "env-us-east-1", found.BedrockKeyConfig.Region.GetValue()) - assert.True(t, found.BedrockKeyConfig.Region.IsFromEnv()) + assert.True(t, found.BedrockKeyConfig.Region.IsFromSecret()) assert.Equal(t, "arn:aws:iam::env:role/test", found.BedrockKeyConfig.ARN.GetValue()) - assert.True(t, found.BedrockKeyConfig.ARN.IsFromEnv()) + assert.True(t, found.BedrockKeyConfig.ARN.IsFromSecret()) } func TestEncryptPlaintextKeys_SecretVarBackedFields_SurviveStartupPass(t *testing.T) { @@ -1101,16 +1101,16 @@ func TestEncryptPlaintextKeys_SecretVarBackedFields_SurviveStartupPass(t *testin var found tables.TableKey require.NoError(t, db.Where("name = ?", "env-startup-key").First(&found).Error) assert.Equal(t, "sk-startup-env-key", found.Value.GetValue()) - assert.True(t, found.Value.IsFromEnv()) + assert.True(t, found.Value.IsFromSecret()) require.NotNil(t, found.AzureKeyConfig) assert.Equal(t, "https://startup.openai.azure.com", found.AzureKeyConfig.Endpoint.GetValue()) - assert.True(t, found.AzureKeyConfig.Endpoint.IsFromEnv()) + assert.True(t, found.AzureKeyConfig.Endpoint.IsFromSecret()) require.NotNil(t, found.VertexKeyConfig) assert.Equal(t, "startup-vertex-creds", found.VertexKeyConfig.AuthCredentials.GetValue()) - assert.True(t, found.VertexKeyConfig.AuthCredentials.IsFromEnv()) + assert.True(t, found.VertexKeyConfig.AuthCredentials.IsFromSecret()) require.NotNil(t, found.BedrockKeyConfig) assert.Equal(t, "AKIA-STARTUP", found.BedrockKeyConfig.AccessKey.GetValue()) - assert.True(t, found.BedrockKeyConfig.AccessKey.IsFromEnv()) + assert.True(t, found.BedrockKeyConfig.AccessKey.IsFromSecret()) } // ============================================================================ @@ -1306,7 +1306,7 @@ func TestEncryptPlaintextMCPClients_SecretVarConnectionStringSurvivesStartup(t * var found tables.TableMCPClient require.NoError(t, db.Where("client_id = ?", "mcp-env-startup").First(&found).Error) assert.Equal(t, "https://mcp-env.example.com/sse", found.ConnectionString.GetValue()) - assert.True(t, found.ConnectionString.IsFromEnv()) + assert.True(t, found.ConnectionString.IsFromSecret()) } // ============================================================================ diff --git a/framework/configstore/rdb.go b/framework/configstore/rdb.go index 97bbcf811d..6807e9ab03 100644 --- a/framework/configstore/rdb.go +++ b/framework/configstore/rdb.go @@ -2044,7 +2044,7 @@ func (s *RDBConfigStore) UpdateMCPClientConfig(ctx context.Context, id string, c if clientConfigCopy.Headers != nil { for key, value := range clientConfigCopy.Headers { if value.IsFromSecret() { - headersToSerialize[key] = value.Ref() + headersToSerialize[key] = value.GetSecretRef() } else { headersToSerialize[key] = value.GetValue() } diff --git a/framework/configstore/tables/encryption_test.go b/framework/configstore/tables/encryption_test.go index 4c67b3a3ec..4d974c69e4 100644 --- a/framework/configstore/tables/encryption_test.go +++ b/framework/configstore/tables/encryption_test.go @@ -249,7 +249,7 @@ func TestTableKey_SecretVarNotEncrypted(t *testing.T) { var found TableKey require.NoError(t, db.First(&found, key.ID).Error) // The value should be readable (either the env var value or empty if not set) - assert.True(t, found.Value.IsFromEnv()) + assert.True(t, found.Value.IsFromSecret()) } // ============================================================================ @@ -350,7 +350,7 @@ func TestTableMCPClient_SecretVarConnectionString_NotEncrypted(t *testing.T) { var found TableMCPClient require.NoError(t, db.First(&found, client.ID).Error) - assert.True(t, found.ConnectionString.IsFromEnv()) + assert.True(t, found.ConnectionString.IsFromSecret()) } // ============================================================================ @@ -735,7 +735,7 @@ func TestEncryptSecretVar_EnvRefIsNoop(t *testing.T) { require.NoError(t, encryptSecretVar(ev)) // Value should not change — env var references are never encrypted assert.Equal(t, originalVal, ev.Val) - assert.True(t, ev.IsFromEnv()) + assert.True(t, ev.IsFromSecret()) } func TestDecryptSecretVar_NilIsNoop(t *testing.T) { @@ -1726,12 +1726,8 @@ func TestTableKey_VertexUnresolvedSecretVar_RoundTrip(t *testing.T) { KeyID: "vertex-env-uuid-1", Value: *schemas.NewSecretVar(""), VertexKeyConfig: &schemas.VertexKeyConfig{ - ProjectID: schemas.SecretVar{ - Val: "", - EnvVar: "env.FAKE_VERTEX_PROJECT_ID_FOR_TEST", - FromEnv: true, - }, - Region: *schemas.NewSecretVar("us-central1"), + ProjectID: *schemas.NewSecretVar("env.FAKE_VERTEX_PROJECT_ID_FOR_TEST"), + Region: *schemas.NewSecretVar("us-central1"), }, } @@ -1743,9 +1739,9 @@ func TestTableKey_VertexUnresolvedSecretVar_RoundTrip(t *testing.T) { // VertexKeyConfig must NOT be wiped — this was the original bug. require.NotNil(t, found.VertexKeyConfig, "VertexKeyConfig was wiped on reload") - assert.Equal(t, "env.FAKE_VERTEX_PROJECT_ID_FOR_TEST", found.VertexKeyConfig.ProjectID.EnvVar, + assert.Equal(t, "env.FAKE_VERTEX_PROJECT_ID_FOR_TEST", found.VertexKeyConfig.ProjectID.GetSecretRef(), "env var reference for ProjectID lost on round-trip") - assert.True(t, found.VertexKeyConfig.ProjectID.FromEnv, + assert.True(t, found.VertexKeyConfig.ProjectID.IsFromSecret(), "FromEnv flag for ProjectID lost on round-trip") assert.Equal(t, "us-central1", found.VertexKeyConfig.Region.GetValue(), "Plain Region value should survive round-trip unchanged") @@ -1766,11 +1762,7 @@ func TestTableKey_AzureUnresolvedSecretVar_RoundTrip(t *testing.T) { KeyID: "azure-env-uuid-1", Value: *schemas.NewSecretVar(""), AzureKeyConfig: &schemas.AzureKeyConfig{ - Endpoint: schemas.SecretVar{ - Val: "", - EnvVar: "env.FAKE_AZURE_ENDPOINT_FOR_TEST", - FromEnv: true, - }, + Endpoint: *schemas.NewSecretVar("env.FAKE_AZURE_ENDPOINT_FOR_TEST"), }, } @@ -1780,9 +1772,9 @@ func TestTableKey_AzureUnresolvedSecretVar_RoundTrip(t *testing.T) { require.NoError(t, db.First(&found, key.ID).Error) require.NotNil(t, found.AzureKeyConfig, "AzureKeyConfig was wiped on reload") - assert.Equal(t, "env.FAKE_AZURE_ENDPOINT_FOR_TEST", found.AzureKeyConfig.Endpoint.EnvVar, + assert.Equal(t, "env.FAKE_AZURE_ENDPOINT_FOR_TEST", found.AzureKeyConfig.Endpoint.GetSecretRef(), "env var reference for Endpoint lost on round-trip") - assert.True(t, found.AzureKeyConfig.Endpoint.FromEnv, + assert.True(t, found.AzureKeyConfig.Endpoint.IsFromSecret(), "FromEnv flag for Endpoint lost on round-trip") } @@ -1801,17 +1793,9 @@ func TestTableKey_BedrockUnresolvedSecretVar_RoundTrip(t *testing.T) { KeyID: "bedrock-env-uuid-1", Value: *schemas.NewSecretVar(""), BedrockKeyConfig: &schemas.BedrockKeyConfig{ - AccessKey: schemas.SecretVar{ - Val: "", - EnvVar: "env.FAKE_AWS_ACCESS_KEY_FOR_TEST", - FromEnv: true, - }, - SecretKey: schemas.SecretVar{ - Val: "", - EnvVar: "env.FAKE_AWS_SECRET_KEY_FOR_TEST", - FromEnv: true, - }, - Region: schemas.NewSecretVar("us-west-2"), + AccessKey: *schemas.NewSecretVar("env.FAKE_AWS_ACCESS_KEY_FOR_TEST"), + SecretKey: *schemas.NewSecretVar("env.FAKE_AWS_SECRET_KEY_FOR_TEST"), + Region: schemas.NewSecretVar("us-west-2"), }, } @@ -1821,9 +1805,9 @@ func TestTableKey_BedrockUnresolvedSecretVar_RoundTrip(t *testing.T) { require.NoError(t, db.First(&found, key.ID).Error) require.NotNil(t, found.BedrockKeyConfig, "BedrockKeyConfig was wiped on reload") - assert.Equal(t, "env.FAKE_AWS_ACCESS_KEY_FOR_TEST", found.BedrockKeyConfig.AccessKey.EnvVar, + assert.Equal(t, "env.FAKE_AWS_ACCESS_KEY_FOR_TEST", found.BedrockKeyConfig.AccessKey.GetSecretRef(), "env var reference for AccessKey lost on round-trip") - assert.Equal(t, "env.FAKE_AWS_SECRET_KEY_FOR_TEST", found.BedrockKeyConfig.SecretKey.EnvVar, + assert.Equal(t, "env.FAKE_AWS_SECRET_KEY_FOR_TEST", found.BedrockKeyConfig.SecretKey.GetSecretRef(), "env var reference for SecretKey lost on round-trip") require.NotNil(t, found.BedrockKeyConfig.Region) assert.Equal(t, "us-west-2", found.BedrockKeyConfig.Region.GetValue()) @@ -1843,11 +1827,7 @@ func TestTableKey_OllamaUnresolvedSecretVar_RoundTrip(t *testing.T) { KeyID: "ollama-env-uuid-1", Value: *schemas.NewSecretVar(""), OllamaKeyConfig: &schemas.OllamaKeyConfig{ - URL: schemas.SecretVar{ - Val: "", - EnvVar: "env.FAKE_OLLAMA_URL_FOR_TEST", - FromEnv: true, - }, + URL: *schemas.NewSecretVar("env.FAKE_OLLAMA_URL_FOR_TEST"), }, } @@ -1857,8 +1837,8 @@ func TestTableKey_OllamaUnresolvedSecretVar_RoundTrip(t *testing.T) { require.NoError(t, db.First(&found, key.ID).Error) require.NotNil(t, found.OllamaKeyConfig, "OllamaKeyConfig was wiped on reload") - assert.Equal(t, "env.FAKE_OLLAMA_URL_FOR_TEST", found.OllamaKeyConfig.URL.EnvVar) - assert.True(t, found.OllamaKeyConfig.URL.FromEnv) + assert.Equal(t, "env.FAKE_OLLAMA_URL_FOR_TEST", found.OllamaKeyConfig.URL.GetSecretRef()) + assert.True(t, found.OllamaKeyConfig.URL.IsFromSecret()) } func TestTableKey_SGLUnresolvedSecretVar_RoundTrip(t *testing.T) { @@ -1873,11 +1853,7 @@ func TestTableKey_SGLUnresolvedSecretVar_RoundTrip(t *testing.T) { KeyID: "sgl-env-uuid-1", Value: *schemas.NewSecretVar(""), SGLKeyConfig: &schemas.SGLKeyConfig{ - URL: schemas.SecretVar{ - Val: "", - EnvVar: "env.FAKE_SGL_URL_FOR_TEST", - FromEnv: true, - }, + URL: *schemas.NewSecretVar("env.FAKE_SGL_URL_FOR_TEST"), }, } @@ -1887,8 +1863,8 @@ func TestTableKey_SGLUnresolvedSecretVar_RoundTrip(t *testing.T) { require.NoError(t, db.First(&found, key.ID).Error) require.NotNil(t, found.SGLKeyConfig, "SGLKeyConfig was wiped on reload") - assert.Equal(t, "env.FAKE_SGL_URL_FOR_TEST", found.SGLKeyConfig.URL.EnvVar) - assert.True(t, found.SGLKeyConfig.URL.FromEnv) + assert.Equal(t, "env.FAKE_SGL_URL_FOR_TEST", found.SGLKeyConfig.URL.GetSecretRef()) + assert.True(t, found.SGLKeyConfig.URL.IsFromSecret()) } // TestTableKey_VertexPlainValue_RoundTrip is a sanity check ensuring that plain @@ -1916,7 +1892,7 @@ func TestTableKey_VertexPlainValue_RoundTrip(t *testing.T) { require.NotNil(t, found.VertexKeyConfig) assert.Equal(t, "my-gcp-project", found.VertexKeyConfig.ProjectID.GetValue()) - assert.False(t, found.VertexKeyConfig.ProjectID.FromEnv) + assert.False(t, found.VertexKeyConfig.ProjectID.IsFromSecret()) assert.Equal(t, "us-central1", found.VertexKeyConfig.Region.GetValue()) } diff --git a/framework/configstore/tables/mcp.go b/framework/configstore/tables/mcp.go index bec9eefeaa..20bc0d5755 100644 --- a/framework/configstore/tables/mcp.go +++ b/framework/configstore/tables/mcp.go @@ -128,7 +128,7 @@ func (c *TableMCPClient) BeforeSave(tx *gorm.DB) error { headersToSerialize := make(map[string]string, len(c.Headers)) for key, value := range c.Headers { if value.IsFromSecret() { - headersToSerialize[key] = value.Ref() + headersToSerialize[key] = value.GetSecretRef() } else { headersToSerialize[key] = value.GetValue() } diff --git a/framework/configstore/tables/virtualkey_secretvar_test.go b/framework/configstore/tables/virtualkey_secretvar_test.go index a86eb16573..0e16a158bf 100644 --- a/framework/configstore/tables/virtualkey_secretvar_test.go +++ b/framework/configstore/tables/virtualkey_secretvar_test.go @@ -20,7 +20,7 @@ func TestTableVirtualKey_EnvSourcedRoundTrip(t *testing.T) { t.Setenv("TEST_VK_ENV", "sk-bf-env-resolved") sv := schemas.NewSecretVar("env.TEST_VK_ENV") - require.True(t, sv.IsFromEnv()) + require.True(t, sv.IsFromSecret()) require.Equal(t, "sk-bf-env-resolved", sv.GetValue()) vk := &TableVirtualKey{ @@ -37,8 +37,8 @@ func TestTableVirtualKey_EnvSourcedRoundTrip(t *testing.T) { var found TableVirtualKey require.NoError(t, db.First(&found, "id = ?", "vk-env").Error) - assert.True(t, found.Value.IsFromEnv()) - assert.Equal(t, "env.TEST_VK_ENV", found.Value.EnvVar) + assert.True(t, found.Value.IsFromSecret()) + assert.Equal(t, "env.TEST_VK_ENV", found.Value.GetSecretRef()) assert.Equal(t, "sk-bf-env-resolved", found.Value.GetValue()) } @@ -60,7 +60,7 @@ func TestTableVirtualKey_LiteralRoundTrip(t *testing.T) { var found TableVirtualKey require.NoError(t, db.First(&found, "id = ?", "vk-lit").Error) - assert.False(t, found.Value.IsFromEnv()) + assert.False(t, found.Value.IsFromSecret()) assert.False(t, found.Value.IsFromVault()) assert.Equal(t, "sk-bf-literal", found.Value.GetValue()) } @@ -96,8 +96,8 @@ func TestTableVirtualKey_MarshalJSON_SecretVarShape(t *testing.T) { Value schemas.SecretVar `json:"value"` } require.NoError(t, json.Unmarshal(data, &out)) - assert.True(t, out.Value.FromEnv) - assert.Equal(t, "env.TEST_VK_MARSHAL", out.Value.EnvVar) + assert.True(t, out.Value.IsFromSecret()) + assert.Equal(t, "env.TEST_VK_MARSHAL", out.Value.GetSecretRef()) } // TestTableVirtualKey_UnmarshalJSON_Forms verifies `value` accepts a bare string and an "env.X" @@ -109,15 +109,15 @@ func TestTableVirtualKey_UnmarshalJSON_Forms(t *testing.T) { var vk TableVirtualKey require.NoError(t, json.Unmarshal([]byte(`{"name":"n","value":"sk-bf-plain"}`), &vk)) assert.Equal(t, "sk-bf-plain", vk.Value.GetValue()) - assert.False(t, vk.Value.IsFromEnv()) + assert.False(t, vk.Value.IsFromSecret()) }) t.Run("env string", func(t *testing.T) { var vk TableVirtualKey require.NoError(t, json.Unmarshal([]byte(`{"name":"n","value":"env.TEST_VK_UMARSHAL"}`), &vk)) assert.Equal(t, "sk-bf-um", vk.Value.GetValue()) - assert.True(t, vk.Value.IsFromEnv()) - assert.Equal(t, "env.TEST_VK_UMARSHAL", vk.Value.EnvVar) + assert.True(t, vk.Value.IsFromSecret()) + assert.Equal(t, "env.TEST_VK_UMARSHAL", vk.Value.GetSecretRef()) }) } diff --git a/transports/bifrost-http/handlers/config.go b/transports/bifrost-http/handlers/config.go index 84ed7c6fec..cef2aa896f 100644 --- a/transports/bifrost-http/handlers/config.go +++ b/transports/bifrost-http/handlers/config.go @@ -691,11 +691,11 @@ func (h *ConfigHandler) updateConfig(ctx *fasthttp.RequestCtx) { // Validate env variables are set if referenced if payload.AuthConfig.AdminUserName.IsFromSecret() && payload.AuthConfig.AdminUserName.GetValue() == "" { - SendError(ctx, fasthttp.StatusBadRequest, fmt.Sprintf("external reference %s for admin_username resolved to an empty value", payload.AuthConfig.AdminUserName.Ref())) + SendError(ctx, fasthttp.StatusBadRequest, fmt.Sprintf("external reference %s for admin_username resolved to an empty value", payload.AuthConfig.AdminUserName.GetSecretRef())) return } if payload.AuthConfig.AdminPassword.IsFromSecret() && payload.AuthConfig.AdminPassword.GetValue() == "" { - SendError(ctx, fasthttp.StatusBadRequest, fmt.Sprintf("external reference %s for admin_password resolved to an empty value", payload.AuthConfig.AdminPassword.Ref())) + SendError(ctx, fasthttp.StatusBadRequest, fmt.Sprintf("external reference %s for admin_password resolved to an empty value", payload.AuthConfig.AdminPassword.GetSecretRef())) return } diff --git a/transports/bifrost-http/lib/config.go b/transports/bifrost-http/lib/config.go index 1ceabefa6e..3de5e528fe 100644 --- a/transports/bifrost-http/lib/config.go +++ b/transports/bifrost-http/lib/config.go @@ -3693,10 +3693,10 @@ func loadAuthConfig(ctx context.Context, config *Config, configData *ConfigData) } // Fail-closed: if env/vault reference is unresolved, don't persist empty credentials. if authConfig.AdminUserName != nil && authConfig.AdminUserName.GetValue() == "" && authConfig.AdminUserName.IsFromSecret() { - logger.Warn("username set with external reference but value is empty: %s", authConfig.AdminUserName.Ref()) + logger.Warn("username set with external reference but value is empty: %s", authConfig.AdminUserName.GetSecretRef()) } if authConfig.AdminPassword != nil && authConfig.AdminPassword.GetValue() == "" && authConfig.AdminPassword.IsFromSecret() { - logger.Warn("password set with external reference but value is empty: %s", authConfig.AdminPassword.Ref()) + logger.Warn("password set with external reference but value is empty: %s", authConfig.AdminPassword.GetSecretRef()) } if authConfig.AdminPassword == nil || authConfig.AdminUserName == nil { logger.Warn("auth config is missing admin_username or admin_password, skipping auth config processing") diff --git a/transports/bifrost-http/lib/config_test.go b/transports/bifrost-http/lib/config_test.go index 82f803042a..0700669600 100644 --- a/transports/bifrost-http/lib/config_test.go +++ b/transports/bifrost-http/lib/config_test.go @@ -17702,8 +17702,8 @@ func TestLoadAuthConfigFromFile_PasswordHashing(t *testing.T) { // Verify username was resolved from env require.Equal(t, "envadmin", storedAuth.AdminUserName.GetValue(), "username should be resolved from env variable") - require.True(t, storedAuth.AdminUserName.IsFromEnv(), "username should be marked as from env") - require.Equal(t, "env.TEST_ADMIN_USERNAME", storedAuth.AdminUserName.EnvVar, "env var reference should be preserved") + require.True(t, storedAuth.AdminUserName.IsFromSecret(), "username should be marked as from env") + require.Equal(t, "env.TEST_ADMIN_USERNAME", storedAuth.AdminUserName.GetSecretRef(), "env var reference should be preserved") // Verify password was hashed require.True(t, isBcryptHash(storedAuth.AdminPassword.GetValue()), "password should be hashed") @@ -17736,8 +17736,8 @@ func TestLoadAuthConfigFromFile_PasswordHashing(t *testing.T) { require.True(t, match, "hashed password should match the env variable value") // Verify env var reference is preserved after hashing - require.True(t, storedAuth.AdminPassword.IsFromEnv(), "password should still be marked as from env after hashing") - require.Equal(t, "env.TEST_ADMIN_PASSWORD", storedAuth.AdminPassword.EnvVar, "password env var reference should be preserved") + require.True(t, storedAuth.AdminPassword.IsFromSecret(), "password should still be marked as from env after hashing") + require.Equal(t, "env.TEST_ADMIN_PASSWORD", storedAuth.AdminPassword.GetSecretRef(), "password env var reference should be preserved") }) t.Run("both username and password from env variables", func(t *testing.T) { @@ -17763,7 +17763,7 @@ func TestLoadAuthConfigFromFile_PasswordHashing(t *testing.T) { // Verify username was resolved from env require.Equal(t, "envuser", storedAuth.AdminUserName.GetValue(), "username should be resolved from env variable") - require.True(t, storedAuth.AdminUserName.IsFromEnv(), "username should be marked as from env") + require.True(t, storedAuth.AdminUserName.IsFromSecret(), "username should be marked as from env") // Verify password was resolved from env and hashed require.True(t, isBcryptHash(storedAuth.AdminPassword.GetValue()), "password should be a bcrypt hash") @@ -17772,8 +17772,8 @@ func TestLoadAuthConfigFromFile_PasswordHashing(t *testing.T) { require.True(t, match, "hashed password should match the env variable value") // Verify env var reference is preserved after hashing - require.True(t, storedAuth.AdminPassword.IsFromEnv(), "password should still be marked as from env after hashing") - require.Equal(t, "env.TEST_ADMIN_PASS", storedAuth.AdminPassword.EnvVar, "password env var reference should be preserved") + require.True(t, storedAuth.AdminPassword.IsFromSecret(), "password should still be marked as from env after hashing") + require.Equal(t, "env.TEST_ADMIN_PASS", storedAuth.AdminPassword.GetSecretRef(), "password env var reference should be preserved") }) t.Run("env variable not set results in empty value", func(t *testing.T) { @@ -17798,13 +17798,13 @@ func TestLoadAuthConfigFromFile_PasswordHashing(t *testing.T) { // Verify username is empty but env var reference is preserved require.Equal(t, "", storedAuth.AdminUserName.GetValue(), "username should be empty when env var not set") - require.True(t, storedAuth.AdminUserName.IsFromEnv(), "username should be marked as from env") - require.Equal(t, "env.NONEXISTENT_USERNAME", storedAuth.AdminUserName.EnvVar, "env var reference should be preserved") + require.True(t, storedAuth.AdminUserName.IsFromSecret(), "username should be marked as from env") + require.Equal(t, "env.NONEXISTENT_USERNAME", storedAuth.AdminUserName.GetSecretRef(), "env var reference should be preserved") // Verify password is empty (not hashed since empty) require.Equal(t, "", storedAuth.AdminPassword.GetValue(), "password should be empty when env var not set") - require.True(t, storedAuth.AdminPassword.IsFromEnv(), "password should be marked as from env") - require.Equal(t, "env.NONEXISTENT_PASSWORD", storedAuth.AdminPassword.EnvVar, "env var reference should be preserved") + require.True(t, storedAuth.AdminPassword.IsFromSecret(), "password should be marked as from env") + require.Equal(t, "env.NONEXISTENT_PASSWORD", storedAuth.AdminPassword.GetSecretRef(), "env var reference should be preserved") }) t.Run("password change flushes existing sessions", func(t *testing.T) { diff --git a/transports/bifrost-http/lib/ctx_test.go b/transports/bifrost-http/lib/ctx_test.go index 89248dfe01..543a28aa83 100644 --- a/transports/bifrost-http/lib/ctx_test.go +++ b/transports/bifrost-http/lib/ctx_test.go @@ -532,7 +532,7 @@ func TestConvertToBifrostContext_DirectKey_EnvPrefixNotResolved(t *testing.T) { if key.Value.GetValue() != "env.SOME_SECRET" { t.Errorf("direct key value = %q, want literal %q (must not resolve env vars)", key.Value.GetValue(), "env.SOME_SECRET") } - if key.Value.IsFromEnv() { - t.Error("direct key must not be marked as from-env") + if key.Value.IsFromSecret() { + t.Error("direct key must not be marked as from-secret") } } diff --git a/transports/config.schema.json b/transports/config.schema.json index 35c398ab61..7b25acc1e9 100644 --- a/transports/config.schema.json +++ b/transports/config.schema.json @@ -265,6 +265,8 @@ "type": "object", "properties": { "value": { "type": "string" }, + "secret_ref": { "type": "string" }, + "from_secret": { "type": "boolean" }, "env_var": { "type": "string" }, "from_env": { "type": "boolean" } }, From f1142a1da4e81b05141bc3e763a2281c1908e935 Mon Sep 17 00:00:00 2001 From: Anuj Parihar Date: Mon, 22 Jun 2026 14:49:13 +0530 Subject: [PATCH 3/3] feat: add secret type instead of from secret --- core/schemas/secretvar.go | 269 +++++++++--------- core/schemas/secretvar_test.go | 43 +-- core/schemas/vault.go | 4 +- core/schemas/vault_test.go | 16 +- .../clientconfig_redaction_test.go | 16 +- transports/bifrost-http/handlers/plugins.go | 19 +- transports/config.schema.json | 1 + 7 files changed, 190 insertions(+), 178 deletions(-) diff --git a/core/schemas/secretvar.go b/core/schemas/secretvar.go index b1b9e7911a..c305cf55cc 100644 --- a/core/schemas/secretvar.go +++ b/core/schemas/secretvar.go @@ -10,18 +10,37 @@ import ( "github.com/bytedance/sonic" ) +// SecretType identifies the source of a SecretVar's value. +type SecretType string + +const ( + SecretTypePlainText SecretType = "plain_text" + SecretTypeEnv SecretType = "env" + SecretTypeVault SecretType = "vault" +) + // SecretVar is a wrapper around a value that can be sourced from an environment variable // or an external vault (e.g. AWS Secrets Manager, GCP Secret Manager, HashiCorp Vault). // Three reference forms are accepted: plain text, "env.VAR_NAME", and "vault.path/to/secret". type SecretVar struct { - Val string `json:"value"` + Val string `json:"value"` secretRef string - fromSecret bool + SecretType SecretType `json:"type,omitempty"` +} + +// inferSecretType returns the SecretType implied by a reference string prefix. +func inferSecretType(ref string) SecretType { + if strings.HasPrefix(ref, "vault.") { + return SecretTypeVault + } + if strings.HasPrefix(ref, "env.") { + return SecretTypeEnv + } + return SecretTypePlainText } // NewSecretVar creates a new SecretVar from a string. func NewSecretVar(value string) *SecretVar { - // Use strconv.Unquote to properly handle JSON string escape sequences val := value if unquoted, err := strconv.Unquote(value); err == nil { val = unquoted @@ -31,56 +50,56 @@ func NewSecretVar(value string) *SecretVar { valueNode, _ := sonic.Get([]byte(val), "value") if valueNode.Exists() { type secretVarCompat struct { - Val string `json:"value"` - SecretRef string `json:"secret_ref"` - FromSecret bool `json:"from_secret"` - // backward compat: env_var/from_env (shipped) - EnvVar string `json:"env_var"` - FromEnv bool `json:"from_env"` + Val string `json:"value"` + SecretRef string `json:"secret_ref"` + SecretType SecretType `json:"type"` + FromSecret bool `json:"from_secret"` // backward compat + EnvVar string `json:"env_var"` // backward compat + FromEnv bool `json:"from_env"` // backward compat } var raw secretVarCompat if err := sonic.Unmarshal([]byte(value), &raw); err == nil { e := &SecretVar{Val: raw.Val} - // New format - if raw.SecretRef != "" || raw.FromSecret { + if raw.SecretType != "" { + // New format: explicit type field e.secretRef = raw.SecretRef - e.fromSecret = raw.FromSecret - } else { - // Backward compat: env (shipped — must keep working) - if raw.FromEnv && raw.EnvVar != "" { - ref := raw.EnvVar - if !strings.HasPrefix(ref, "env.") { - ref = "env." + ref - } - e.secretRef = ref - e.fromSecret = true - if envValue, ok := os.LookupEnv(strings.TrimPrefix(ref, "env.")); ok { - e.Val = envValue - } else { - e.Val = "" - } - return e + e.SecretType = raw.SecretType + } else if raw.SecretRef != "" || raw.FromSecret { + // Old format: infer type from ref prefix + e.secretRef = raw.SecretRef + e.SecretType = inferSecretType(raw.SecretRef) + } else if raw.FromEnv && raw.EnvVar != "" { + // Backward compat: from_env/env_var + ref := raw.EnvVar + if !strings.HasPrefix(ref, "env.") { + ref = "env." + ref } - // Legacy format: value == env_var == "env.XXX" - if strings.HasPrefix(raw.Val, "env.") && raw.Val == raw.EnvVar { - e.secretRef = raw.EnvVar - e.fromSecret = true + e.secretRef = ref + e.SecretType = SecretTypeEnv + if envValue, ok := os.LookupEnv(strings.TrimPrefix(ref, "env.")); ok { + e.Val = envValue + } else { e.Val = "" - if envValue, ok := os.LookupEnv(strings.TrimPrefix(raw.EnvVar, "env.")); ok { - e.Val = envValue - } - return e } + return e + } else if strings.HasPrefix(raw.Val, "env.") && raw.Val == raw.EnvVar { + // Legacy format: value == env_var == "env.XXX" + e.secretRef = raw.EnvVar + e.SecretType = SecretTypeEnv + e.Val = "" + if envValue, ok := os.LookupEnv(strings.TrimPrefix(raw.EnvVar, "env.")); ok { + e.Val = envValue + } + return e } - // Resolve vault reference - if e.fromSecret && strings.HasPrefix(e.secretRef, "vault.") { + // Resolve references + if e.SecretType == SecretTypeVault { e.Val = "" if vaultValue, ok := LookupVault(e.secretRef); ok { e.Val = vaultValue } } - // Resolve env reference - if e.fromSecret && strings.HasPrefix(e.secretRef, "env.") { + if e.SecretType == SecretTypeEnv { if envValue, ok := os.LookupEnv(strings.TrimPrefix(e.secretRef, "env.")); ok { e.Val = envValue } else { @@ -92,35 +111,23 @@ func NewSecretVar(value string) *SecretVar { } } if strings.HasPrefix(val, "vault.") { - e := &SecretVar{ - secretRef: val, - fromSecret: true, - } + e := &SecretVar{secretRef: val, SecretType: SecretTypeVault} if vaultValue, ok := LookupVault(val); ok { e.Val = vaultValue } return e } if envKey, ok := strings.CutPrefix(val, "env."); ok { + e := &SecretVar{secretRef: val, SecretType: SecretTypeEnv} if envValue, ok := os.LookupEnv(envKey); ok { - return &SecretVar{ - Val: envValue, - secretRef: val, - fromSecret: true, - } - } - return &SecretVar{ - Val: "", - secretRef: val, - fromSecret: true, + e.Val = envValue } + return e } - return &SecretVar{ - Val: val, - } + return &SecretVar{Val: val} } -// Ref returns the secret reference string (e.g. "env.MY_VAR" or "vault.path/to/secret"). +// GetSecretRef returns the secret reference string (e.g. "env.MY_VAR" or "vault.path/to/secret"). // Returns an empty string for plain-value SecretVars. func (e *SecretVar) GetSecretRef() string { if e == nil { @@ -129,12 +136,23 @@ func (e *SecretVar) GetSecretRef() string { return e.secretRef } +// Type returns the SecretType of this SecretVar. +func (e *SecretVar) Type() SecretType { + if e == nil { + return SecretTypePlainText + } + if e.SecretType == "" { + return SecretTypePlainText + } + return e.SecretType +} + // IsFromSecret returns true if the value is sourced from an external secret (env var or vault). func (e *SecretVar) IsFromSecret() bool { if e == nil { return false } - return e.fromSecret + return e.SecretType == SecretTypeEnv || e.SecretType == SecretTypeVault } // IsFromVault returns true if the value is sourced from a vault path. @@ -142,7 +160,15 @@ func (e *SecretVar) IsFromVault() bool { if e == nil { return false } - return e.fromSecret && strings.HasPrefix(e.secretRef, "vault.") + return e.SecretType == SecretTypeVault +} + +// IsFromEnv returns true if the value is sourced from an environment variable. +func (e *SecretVar) IsFromEnv() bool { + if e == nil { + return false + } + return e.SecretType == SecretTypeEnv } // IsRedacted returns true if the value is redacted. @@ -150,11 +176,11 @@ func (e *SecretVar) IsRedacted() bool { if e == nil { return false } - if e.Val == "" && !e.fromSecret { + if e.Val == "" && !e.IsFromSecret() { return false } // Secret references (env/vault) are treated as redacted — the real value is external - if e.fromSecret { + if e.IsFromSecret() { return true } if len(e.Val) <= 8 { @@ -188,7 +214,7 @@ func (e *SecretVar) Equals(other *SecretVar) bool { } return e.Val == other.Val && e.secretRef == other.secretRef && - e.fromSecret == other.fromSecret + e.SecretType == other.SecretType } // Redacted returns a new SecretVar with the value redacted. @@ -197,65 +223,38 @@ func (e *SecretVar) Redacted() *SecretVar { return nil } if e.Val == "" { - return &SecretVar{ - Val: "", - secretRef: e.secretRef, - fromSecret: e.fromSecret, - } + return &SecretVar{Val: "", secretRef: e.secretRef, SecretType: e.SecretType} } - // If key is 8 characters or less, just return all asterisks if len(e.Val) <= 8 { - return &SecretVar{ - Val: strings.Repeat("*", len(e.Val)), - secretRef: e.secretRef, - fromSecret: e.fromSecret, - } + return &SecretVar{Val: strings.Repeat("*", len(e.Val)), secretRef: e.secretRef, SecretType: e.SecretType} } - // Show first 4 and last 4 characters, replace middle with asterisks prefix := e.Val[:4] suffix := e.Val[len(e.Val)-4:] middle := strings.Repeat("*", 24) - - return &SecretVar{ - Val: prefix + middle + suffix, - secretRef: e.secretRef, - fromSecret: e.fromSecret, - } + return &SecretVar{Val: prefix + middle + suffix, secretRef: e.secretRef, SecretType: e.SecretType} } // FullyRedacted returns a copy of the SecretVar with Val replaced by a fixed placeholder // so no substring of the original value is exposed. Use for API responses where -// Redacted is unsafe (e.g. literal proxy passwords). secretRef and fromSecret are +// Redacted is unsafe (e.g. literal proxy passwords). secretRef and secretType are // preserved so references remain visible. func (e *SecretVar) FullyRedacted() *SecretVar { if e == nil { return nil } if e.Val == "" { - return &SecretVar{ - Val: "", - secretRef: e.secretRef, - fromSecret: e.fromSecret, - } - } - return &SecretVar{ - Val: "", - secretRef: e.secretRef, - fromSecret: e.fromSecret, + return &SecretVar{Val: "", secretRef: e.secretRef, SecretType: e.SecretType} } + return &SecretVar{Val: "", secretRef: e.secretRef, SecretType: e.SecretType} } -// MarshalJSON serializes the SecretVar, emitting secret_ref and from_secret fields. +// MarshalJSON serializes the SecretVar, emitting secret_ref and type fields. func (e SecretVar) MarshalJSON() ([]byte, error) { return Marshal(struct { - Val string `json:"value"` - SecretRef string `json:"secret_ref,omitempty"` - FromSecret bool `json:"from_secret,omitempty"` - }{ - Val: e.Val, - SecretRef: e.secretRef, - FromSecret: e.fromSecret, - }) + Val string `json:"value"` + SecretRef string `json:"secret_ref,omitempty"` + SecretType SecretType `json:"type,omitempty"` + }{Val: e.Val, SecretRef: e.secretRef, SecretType: e.SecretType}) } // UnmarshalJSON unmarshals the value from JSON. @@ -268,29 +267,32 @@ func (e *SecretVar) UnmarshalJSON(data []byte) error { valueNode, _ := sonic.Get(data, "value") if valueNode.Exists() { type secretVarCompat struct { - Val string `json:"value"` - SecretRef string `json:"secret_ref"` - FromSecret bool `json:"from_secret"` - // backward compat: env_var/from_env (shipped) - EnvVar string `json:"env_var"` - FromEnv bool `json:"from_env"` + Val string `json:"value"` + SecretRef string `json:"secret_ref"` + SecretType SecretType `json:"type"` + FromSecret bool `json:"from_secret"` // backward compat + EnvVar string `json:"env_var"` // backward compat + FromEnv bool `json:"from_env"` // backward compat } var raw secretVarCompat if err := sonic.Unmarshal(data, &raw); err == nil { e.Val = raw.Val - - // New format - if raw.SecretRef != "" || raw.FromSecret { + if raw.SecretType != "" { + // New format: explicit type field e.secretRef = raw.SecretRef - e.fromSecret = raw.FromSecret + e.SecretType = raw.SecretType + } else if raw.SecretRef != "" || raw.FromSecret { + // Old format: infer type from ref prefix + e.secretRef = raw.SecretRef + e.SecretType = inferSecretType(raw.SecretRef) } else if raw.FromEnv && raw.EnvVar != "" { - // Backward compat: env + // Backward compat: from_env/env_var ref := raw.EnvVar if !strings.HasPrefix(ref, "env.") { ref = "env." + ref } e.secretRef = ref - e.fromSecret = true + e.SecretType = SecretTypeEnv if envValue, ok := os.LookupEnv(strings.TrimPrefix(ref, "env.")); ok { e.Val = envValue } else { @@ -298,24 +300,23 @@ func (e *SecretVar) UnmarshalJSON(data []byte) error { } return nil } else if strings.HasPrefix(raw.Val, "env.") && raw.Val == raw.EnvVar { - // Old format: value == env_var == "env.XXX" + // Legacy format: value == env_var == "env.XXX" e.secretRef = raw.EnvVar - e.fromSecret = true + e.SecretType = SecretTypeEnv e.Val = "" if envValue, ok := os.LookupEnv(strings.TrimPrefix(raw.EnvVar, "env.")); ok { e.Val = envValue } return nil } - // Resolve references - if e.fromSecret && strings.HasPrefix(e.secretRef, "vault.") { + if e.SecretType == SecretTypeVault { e.Val = "" if vaultValue, ok := LookupVault(e.secretRef); ok { e.Val = vaultValue } } - if e.fromSecret && strings.HasPrefix(e.secretRef, "env.") { + if e.SecretType == SecretTypeEnv { if envValue, ok := os.LookupEnv(strings.TrimPrefix(e.secretRef, "env.")); ok { e.Val = envValue } else { @@ -329,7 +330,7 @@ func (e *SecretVar) UnmarshalJSON(data []byte) error { // Plain string forms if strings.HasPrefix(val, "vault.") { e.secretRef = val - e.fromSecret = true + e.SecretType = SecretTypeVault e.Val = "" if vaultValue, ok := LookupVault(val); ok { e.Val = vaultValue @@ -338,7 +339,7 @@ func (e *SecretVar) UnmarshalJSON(data []byte) error { } if envKey, ok := strings.CutPrefix(val, "env."); ok { e.secretRef = val - e.fromSecret = true + e.SecretType = SecretTypeEnv if envValue, ok := os.LookupEnv(envKey); ok { e.Val = envValue } else { @@ -348,7 +349,7 @@ func (e *SecretVar) UnmarshalJSON(data []byte) error { } e.Val = val e.secretRef = "" - e.fromSecret = false + e.SecretType = SecretTypePlainText return nil } @@ -365,26 +366,26 @@ func (e *SecretVar) Scan(value any) error { if value == nil { e.Val = "" e.secretRef = "" - e.fromSecret = false + e.SecretType = SecretTypePlainText return nil } switch v := value.(type) { case []byte: return e.Scan(string(v)) case string: - val := strings.Trim(v, "\"") - if strings.HasPrefix(val, "vault.") { + // Check raw value first — quoted strings (e.g. `"vault.x"`) are literals, not refs. + if strings.HasPrefix(v, "vault.") { e.Val = "" - e.secretRef = val - e.fromSecret = true - if vaultValue, ok := LookupVault(val); ok { + e.secretRef = v + e.SecretType = SecretTypeVault + if vaultValue, ok := LookupVault(v); ok { e.Val = vaultValue } return nil } - if envKey, ok := strings.CutPrefix(val, "env."); ok { - e.secretRef = val - e.fromSecret = true + if envKey, ok := strings.CutPrefix(v, "env."); ok { + e.secretRef = v + e.SecretType = SecretTypeEnv if envValue, ok := os.LookupEnv(envKey); ok { e.Val = envValue } else { @@ -394,7 +395,7 @@ func (e *SecretVar) Scan(value any) error { } e.Val = strings.Trim(v, "\"") e.secretRef = "" - e.fromSecret = false + e.SecretType = SecretTypePlainText return nil } return fmt.Errorf("failed to scan value: %v", value) @@ -402,9 +403,9 @@ func (e *SecretVar) Scan(value any) error { // Value implements driver.Valuer for database storage. // It stores the secret reference (e.g., "env.API_KEY" or "vault.path/to/secret") if -// fromSecret is true, otherwise the raw value. +// the type is env or vault, otherwise the raw value. func (e SecretVar) Value() (driver.Value, error) { - if e.fromSecret { + if e.IsFromSecret() { return e.secretRef, nil } return e.Val, nil @@ -418,7 +419,7 @@ func (e *SecretVar) ShouldPreserveStored() bool { if e == nil { return true } - if e.fromSecret { + if e.IsFromSecret() { return false } return e.GetValue() == "" || e.IsRedacted() @@ -431,7 +432,7 @@ func (e *SecretVar) IsSet() bool { if e == nil { return false } - if e.fromSecret { + if e.IsFromSecret() { return e.secretRef != "" } return e.Val != "" diff --git a/core/schemas/secretvar_test.go b/core/schemas/secretvar_test.go index a6a24763c7..71a4b8f5d8 100644 --- a/core/schemas/secretvar_test.go +++ b/core/schemas/secretvar_test.go @@ -319,8 +319,8 @@ func TestSecretVar_Equals(t *testing.T) { }, { name: "equal values", - a: &SecretVar{Val: "test", secretRef: "env.TEST", fromSecret: true}, - b: &SecretVar{Val: "test", secretRef: "env.TEST", fromSecret: true}, + a: &SecretVar{Val: "test", secretRef: "env.TEST", SecretType: SecretTypeEnv}, + b: &SecretVar{Val: "test", secretRef: "env.TEST", SecretType: SecretTypeEnv}, expected: true, }, { @@ -403,7 +403,7 @@ func TestSecretVar_FullyRedacted(t *testing.T) { }, { name: "resolved env password preserves reference metadata", - input: SecretVar{Val: "resolved-secret", secretRef: "env.PROXY_PASS", fromSecret: true}, + input: SecretVar{Val: "resolved-secret", secretRef: "env.PROXY_PASS", SecretType: SecretTypeEnv}, wantVal: "", wantFromSecret: true, wantRef: "env.PROXY_PASS", @@ -439,7 +439,7 @@ func TestSecretVar_IsRedacted(t *testing.T) { }, { name: "from secret", - input: SecretVar{Val: "test", secretRef: "env.KEY", fromSecret: true}, + input: SecretVar{Val: "test", secretRef: "env.KEY", SecretType: SecretTypeEnv}, expected: true, }, { @@ -500,22 +500,22 @@ func TestSecretVar_IsSet(t *testing.T) { }, { name: "env reference not yet resolved", - input: &SecretVar{secretRef: "env.MISSING", fromSecret: true}, + input: &SecretVar{secretRef: "env.MISSING", SecretType: SecretTypeEnv}, expected: true, }, { name: "env reference resolved", - input: &SecretVar{Val: "resolved-secret", secretRef: "env.X", fromSecret: true}, + input: &SecretVar{Val: "resolved-secret", secretRef: "env.X", SecretType: SecretTypeEnv}, expected: true, }, { - name: "fromSecret true but no reference and no value", - input: &SecretVar{fromSecret: true}, + name: "secret type set but no reference and no value", + input: &SecretVar{SecretType: SecretTypeEnv}, expected: false, }, { name: "vault reference set", - input: &SecretVar{Val: "vault.bifrost/key", secretRef: "vault.bifrost/key", fromSecret: true}, + input: &SecretVar{Val: "vault.bifrost/key", secretRef: "vault.bifrost/key", SecretType: SecretTypeVault}, expected: true, }, } @@ -590,7 +590,14 @@ func TestSecretVar_UnmarshalJSON_VaultRef(t *testing.T) { wantFromSecret bool }{ { - name: "new format: secret_ref/from_secret", + name: "new format: type field", + input: `{"value":"","secret_ref":"vault.bifrost/key","type":"vault"}`, + wantVal: "", + wantRef: "vault.bifrost/key", + wantFromSecret: true, + }, + { + name: "backward compat: secret_ref/from_secret", input: `{"value":"vault.bifrost/key","secret_ref":"vault.bifrost/key","from_secret":true}`, wantVal: "", wantRef: "vault.bifrost/key", @@ -625,7 +632,7 @@ func TestSecretVar_UnmarshalJSON_VaultRef(t *testing.T) { } func TestSecretVar_Value_VaultRef(t *testing.T) { - e := &SecretVar{Val: "actual-secret", secretRef: "vault.bifrost/key", fromSecret: true} + e := &SecretVar{Val: "actual-secret", secretRef: "vault.bifrost/key", SecretType: SecretTypeVault} got, err := e.Value() if err != nil { t.Fatalf("Value() error: %v", err) @@ -636,7 +643,7 @@ func TestSecretVar_Value_VaultRef(t *testing.T) { } func TestSecretVar_Redacted_VaultRef(t *testing.T) { - e := &SecretVar{Val: "actual-secret-value", secretRef: "vault.bifrost/key", fromSecret: true} + e := &SecretVar{Val: "actual-secret-value", secretRef: "vault.bifrost/key", SecretType: SecretTypeVault} r := e.Redacted() wantVal := "actu************************alue" if r.Val != wantVal { @@ -651,8 +658,8 @@ func TestSecretVar_Redacted_VaultRef(t *testing.T) { } func TestSecretVar_IsSet_VaultRef(t *testing.T) { - set := &SecretVar{Val: "vault.bifrost/key", secretRef: "vault.bifrost/key", fromSecret: true} - unset := &SecretVar{fromSecret: true} + set := &SecretVar{Val: "vault.bifrost/key", secretRef: "vault.bifrost/key", SecretType: SecretTypeVault} + unset := &SecretVar{SecretType: SecretTypeVault} if !set.IsSet() { t.Error("IsSet() = false for vault ref, want true") } @@ -687,13 +694,13 @@ func TestSecretVar_MarshalJSON(t *testing.T) { }, { name: "env reference", - input: SecretVar{Val: "resolved", secretRef: "env.MY_KEY", fromSecret: true}, - want: `{"value":"resolved","secret_ref":"env.MY_KEY","from_secret":true}`, + input: SecretVar{Val: "resolved", secretRef: "env.MY_KEY", SecretType: SecretTypeEnv}, + want: `{"value":"resolved","secret_ref":"env.MY_KEY","type":"env"}`, }, { name: "vault reference", - input: SecretVar{secretRef: "vault.path/to/secret", fromSecret: true}, - want: `{"value":"","secret_ref":"vault.path/to/secret","from_secret":true}`, + input: SecretVar{secretRef: "vault.path/to/secret", SecretType: SecretTypeVault}, + want: `{"value":"","secret_ref":"vault.path/to/secret","type":"vault"}`, }, } diff --git a/core/schemas/vault.go b/core/schemas/vault.go index b9a4b6a760..552c179f25 100644 --- a/core/schemas/vault.go +++ b/core/schemas/vault.go @@ -119,7 +119,7 @@ func RemoveOwnedVaultSecretVars(ctx context.Context, ownedPrefix string, model i // vault-backed, non-fragment reference under ownedPrefix. Fragment refs (#key) // point at shared, externally-managed secrets and are never auto-deleted. func removeOwnedVaultSecretVar(ctx context.Context, ownedPrefix string, field *SecretVar) { - if field == nil || !strings.HasPrefix(field.secretRef, "vault.") || field.secretRef == "" { + if field == nil || field.SecretType != SecretTypeVault || field.secretRef == "" { return } path := strings.TrimPrefix(field.secretRef, "vault.") @@ -146,7 +146,7 @@ func StoreVaultSecretVar(ctx context.Context, path string, e *SecretVar) error { return err } e.secretRef = "vault." + path - e.fromSecret = true + e.SecretType = SecretTypeVault return nil } diff --git a/core/schemas/vault_test.go b/core/schemas/vault_test.go index 99b4a8022a..de8d817560 100644 --- a/core/schemas/vault_test.go +++ b/core/schemas/vault_test.go @@ -48,8 +48,8 @@ func TestStoreVaultSecretVar_NoOps(t *testing.T) { e *SecretVar }{ {"nil", nil}, - {"env-sourced", &SecretVar{secretRef: "env.MY_VAR", fromSecret: true}}, - {"already-vault", &SecretVar{Val: "vault.some/path", secretRef: "vault.some/path", fromSecret: true}}, + {"env-sourced", &SecretVar{secretRef: "env.MY_VAR", SecretType: SecretTypeEnv}}, + {"already-vault", &SecretVar{Val: "vault.some/path", secretRef: "vault.some/path", SecretType: SecretTypeVault}}, {"empty", &SecretVar{Val: ""}}, } for _, tc := range cases { @@ -93,8 +93,8 @@ func TestRemoveOwnedVaultSecretVars_SkipsFragmentRefs(t *testing.T) { Fragment SecretVar `gorm:"column:fragment"` } m := &model{ - Normal: SecretVar{Val: "vault.bifrost/m/1/normal", secretRef: "vault.bifrost/m/1/normal", fromSecret: true}, - Fragment: SecretVar{Val: "vault.external/db#apiKey", secretRef: "vault.external/db#apiKey", fromSecret: true}, + Normal: SecretVar{Val: "vault.bifrost/m/1/normal", secretRef: "vault.bifrost/m/1/normal", SecretType: SecretTypeVault}, + Fragment: SecretVar{Val: "vault.external/db#apiKey", secretRef: "vault.external/db#apiKey", SecretType: SecretTypeVault}, } RemoveOwnedVaultSecretVars(context.Background(), "bifrost/m/1", m) @@ -119,7 +119,7 @@ func TestStoreOwnedVaultSecretVars_WalksFields(t *testing.T) { Plain: SecretVar{Val: "p1"}, Ptr: &SecretVar{Val: "p2"}, Snake: SecretVar{Val: "p3"}, - EnvBased: SecretVar{secretRef: "env.X", fromSecret: true}, + EnvBased: SecretVar{secretRef: "env.X", SecretType: SecretTypeEnv}, } if err := StoreOwnedVaultSecretVars(context.Background(), "bifrost/m/1", m); err != nil { @@ -153,7 +153,7 @@ func TestStoreOwnedVaultSecretVars_WalksMap(t *testing.T) { m := &model{ Headers: map[string]SecretVar{ "Authorization": {Val: "secret-token"}, - "X-Env": SecretVar{secretRef: "env.X", fromSecret: true}, + "X-Env": SecretVar{secretRef: "env.X", SecretType: SecretTypeEnv}, }, } @@ -192,8 +192,8 @@ func TestRemoveOwnedVaultSecretVars_WalksMap(t *testing.T) { } m := &model{ Headers: map[string]SecretVar{ - "Owned": SecretVar{Val: "vault.bifrost/m/1/headers/Owned", secretRef: "vault.bifrost/m/1/headers/Owned", fromSecret: true}, - "External": SecretVar{Val: "vault.external/db#key", secretRef: "vault.external/db#key", fromSecret: true}, + "Owned": SecretVar{Val: "vault.bifrost/m/1/headers/Owned", secretRef: "vault.bifrost/m/1/headers/Owned", SecretType: SecretTypeVault}, + "External": SecretVar{Val: "vault.external/db#key", secretRef: "vault.external/db#key", SecretType: SecretTypeVault}, }, } diff --git a/framework/configstore/clientconfig_redaction_test.go b/framework/configstore/clientconfig_redaction_test.go index 5eee812ba9..e7023920ec 100644 --- a/framework/configstore/clientconfig_redaction_test.go +++ b/framework/configstore/clientconfig_redaction_test.go @@ -42,9 +42,9 @@ func TestProviderConfig_Redacted_AutoMasksEnvBackedFields(t *testing.T) { require.NoError(t, err) var out struct { - Value string `json:"value"` - SecretRef string `json:"secret_ref"` - FromSecret bool `json:"from_secret"` + Value string `json:"value"` + SecretRef string `json:"secret_ref"` + SecretType string `json:"type"` } require.NoError(t, json.Unmarshal(data, &out)) @@ -52,7 +52,7 @@ func TestProviderConfig_Redacted_AutoMasksEnvBackedFields(t *testing.T) { "resolved env value leaked through Endpoint JSON output: %q", out.Value) assert.Equal(t, "env.MY_AZURE_ENDPOINT_SECRET", out.SecretRef, "secret ref must be preserved so the UI can show it") - assert.True(t, out.FromSecret, "from_secret flag must be preserved") + assert.Equal(t, "env", out.SecretType, "type field must be preserved") } // TestProviderConfig_Redacted_DoesNotMaskPlainNonSecretFields verifies that the @@ -80,13 +80,13 @@ func TestProviderConfig_Redacted_DoesNotMaskPlainNonSecretFields(t *testing.T) { var out struct { Value string `json:"value"` - FromSecret bool `json:"from_secret"` + SecretType string `json:"type"` } require.NoError(t, json.Unmarshal(data, &out)) assert.Equal(t, "https://foo.openai.azure.com", out.Value, "plain Endpoint was incorrectly redacted") - assert.False(t, out.FromSecret) + assert.Empty(t, out.SecretType) } // TestProviderConfig_Redacted_PreservesSecretVarReferenceForVertex verifies that @@ -118,14 +118,14 @@ func TestProviderConfig_Redacted_PreservesSecretVarReferenceForVertex(t *testing var out struct { Value string `json:"value"` SecretRef string `json:"secret_ref"` - FromSecret bool `json:"from_secret"` + SecretType string `json:"type"` } require.NoError(t, json.Unmarshal(data, &out)) assert.NotContains(t, out.Value, "super-secret-project", "resolved Vertex ProjectID env value leaked: %q", out.Value) assert.Equal(t, "env.MY_VERTEX_PROJECT_ID_SECRET", out.SecretRef) - assert.True(t, out.FromSecret) + assert.Equal(t, "env", out.SecretType) } // TestProviderConfig_Redacted_DoesNotMutateOriginal ensures Redacted() does not diff --git a/transports/bifrost-http/handlers/plugins.go b/transports/bifrost-http/handlers/plugins.go index e903f26ffd..8bd1d1dd4e 100644 --- a/transports/bifrost-http/handlers/plugins.go +++ b/transports/bifrost-http/handlers/plugins.go @@ -628,24 +628,27 @@ func restoreRedactedValue(incoming, existing any) any { } } -// isSecretVarObject returns true if m has exactly the shape of a serialised SecretVar: -// keys "value", "env_var", and "from_env". +// isSecretVarObject returns true if m has the shape of a serialised SecretVar. func isSecretVarObject(m map[string]any) bool { _, hasValue := m["value"] _, hasSecretRef := m["secret_ref"] - _, hasFromSecret := m["from_secret"] - // also accept legacy env_var/from_env keys for backward compat - _, hasEnvVar := m["env_var"] - _, hasFromEnv := m["from_env"] - return hasValue && ((hasSecretRef && hasFromSecret) || (hasEnvVar && hasFromEnv)) + _, hasType := m["type"] + _, hasFromSecret := m["from_secret"] // backward compat + _, hasEnvVar := m["env_var"] // backward compat + _, hasFromEnv := m["from_env"] // backward compat + return hasValue && ((hasSecretRef && (hasType || hasFromSecret)) || (hasEnvVar && hasFromEnv)) } // marshalSecretVarObject serialises a SecretVar-shaped map back to the JSON string that // schemas.NewSecretVar expects so we can call ShouldPreserveStored on it. func marshalSecretVarObject(m map[string]any) string { value, _ := m["value"].(string) - // new format if secretRef, ok := m["secret_ref"].(string); ok { + secretType, _ := m["type"].(string) + if secretType != "" { + return fmt.Sprintf(`{"value":%q,"secret_ref":%q,"type":%q}`, value, secretRef, secretType) + } + // backward compat: from_secret bool fromSecret, _ := m["from_secret"].(bool) if fromSecret { return fmt.Sprintf(`{"value":%q,"secret_ref":%q,"from_secret":true}`, value, secretRef) diff --git a/transports/config.schema.json b/transports/config.schema.json index 7b25acc1e9..e18b60aad0 100644 --- a/transports/config.schema.json +++ b/transports/config.schema.json @@ -266,6 +266,7 @@ "properties": { "value": { "type": "string" }, "secret_ref": { "type": "string" }, + "type": { "type": "string", "enum": ["plain_text", "env", "vault"] }, "from_secret": { "type": "boolean" }, "env_var": { "type": "string" }, "from_env": { "type": "boolean" }