diff --git a/core/schemas/envvar.go b/core/schemas/envvar.go
index 28b95861a3..c8fe249699 100644
--- a/core/schemas/envvar.go
+++ b/core/schemas/envvar.go
@@ -147,6 +147,34 @@ func (e *EnvVar) Redacted() *EnvVar {
}
}
+// MarshalJSON serializes the EnvVar to JSON.
+// SECURITY: When the value was sourced from an environment variable, the resolved
+// value is automatically redacted before being serialized. This ensures that secrets
+// injected via env vars are never leaked through any JSON API response, regardless
+// of whether the surrounding code remembered to call Redacted() explicitly.
+//
+// Plain (non-env) values are still emitted as-is — callers that want to mask those
+// must continue using Redacted() at the field level (this matches the existing
+// per-provider redaction logic).
+//
+// This does NOT affect:
+// - GORM persistence (uses the Value() driver method, not JSON)
+// - Encryption (operates on the Val field directly)
+// - Internal LLM request paths (use GetValue() directly)
+func (e EnvVar) MarshalJSON() ([]byte, error) {
+ type envVarAlias EnvVar
+ out := envVarAlias(e)
+ if e.FromEnv {
+ // Redact the resolved value but keep the env var reference and from_env flag
+ // so the UI still knows which env var backs this field.
+ redacted := e.Redacted()
+ if redacted != nil {
+ out = envVarAlias(*redacted)
+ }
+ }
+ return sonic.Marshal(out)
+}
+
// UnmarshalJSON unmarshals the value from JSON.
func (e *EnvVar) UnmarshalJSON(data []byte) error {
// This is always going to be value
@@ -262,6 +290,17 @@ func (e *EnvVar) IsFromEnv() bool {
return e.FromEnv
}
+// IsSet returns true if the EnvVar has a resolved value or an environment variable reference.
+// This should be used instead of GetValue() != "" when checking whether a field was configured,
+// because env var references may have an empty Val before resolution (e.g., when the env var
+// is not available in the current environment).
+func (e *EnvVar) IsSet() bool {
+ if e == nil {
+ return false
+ }
+ return e.Val != "" || e.EnvVar != ""
+}
+
// GetValue returns the value.
func (e *EnvVar) GetValue() string {
if e == nil {
diff --git a/core/schemas/envvar_test.go b/core/schemas/envvar_test.go
index 5a451b5058..9b22673ae8 100644
--- a/core/schemas/envvar_test.go
+++ b/core/schemas/envvar_test.go
@@ -419,3 +419,191 @@ func TestEnvVar_IsRedacted(t *testing.T) {
})
}
}
+
+// TestEnvVar_IsSet verifies the semantic difference between GetValue() != "" and IsSet().
+// IsSet() must return true when the EnvVar references an env var (regardless of whether
+// that env var has been resolved to a non-empty Val). This is the property that the
+// BeforeSave hooks rely on so env var references survive persistence.
+func TestEnvVar_IsSet(t *testing.T) {
+ tests := []struct {
+ name string
+ input *EnvVar
+ expected bool
+ }{
+ {
+ name: "nil envvar",
+ input: nil,
+ expected: false,
+ },
+ {
+ name: "completely empty",
+ input: &EnvVar{},
+ expected: false,
+ },
+ {
+ name: "only Val set (plain value)",
+ input: &EnvVar{Val: "abc"},
+ expected: true,
+ },
+ {
+ name: "only EnvVar reference set (env not resolved on this server)",
+ input: &EnvVar{EnvVar: "env.MISSING", FromEnv: true},
+ expected: true,
+ },
+ {
+ name: "Val and EnvVar both set (env was resolved)",
+ input: &EnvVar{Val: "resolved-secret", EnvVar: "env.X", FromEnv: true},
+ expected: true,
+ },
+ {
+ name: "FromEnv true but no reference and no value",
+ input: &EnvVar{FromEnv: true},
+ expected: false,
+ },
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ if got := tt.input.IsSet(); got != tt.expected {
+ t.Errorf("IsSet() = %v, want %v", got, tt.expected)
+ }
+ })
+ }
+}
+
+// TestEnvVar_MarshalJSON_AutoRedactsEnvBackedValues verifies that any EnvVar marshaled
+// to JSON with FromEnv=true is automatically masked, regardless of whether the
+// surrounding code remembered to call Redacted() explicitly. This is the defense-in-depth
+// guarantee that prevents env-resolved secrets from leaking through unredacted fields.
+func TestEnvVar_MarshalJSON_AutoRedactsEnvBackedValues(t *testing.T) {
+ tests := []struct {
+ name string
+ input EnvVar
+ wantValue string
+ wantEnvVar string
+ wantFromEnv bool
+ }{
+ {
+ name: "env-backed long secret is redacted",
+ input: EnvVar{Val: "sk-1234567890abcdefghijklmnop", EnvVar: "env.OPENAI_API_KEY", FromEnv: true},
+ wantValue: "sk-1************************mnop",
+ wantEnvVar: "env.OPENAI_API_KEY",
+ wantFromEnv: true,
+ },
+ {
+ name: "env-backed short secret is fully masked",
+ input: EnvVar{Val: "12345678", EnvVar: "env.SHORT", FromEnv: true},
+ wantValue: "********",
+ wantEnvVar: "env.SHORT",
+ wantFromEnv: true,
+ },
+ {
+ name: "env-backed unresolved on this server keeps empty value",
+ input: EnvVar{Val: "", EnvVar: "env.MISSING", FromEnv: true},
+ wantValue: "",
+ wantEnvVar: "env.MISSING",
+ wantFromEnv: true,
+ },
+ {
+ name: "plain value (not from env) is NOT redacted",
+ input: EnvVar{Val: "2024-10-21", EnvVar: "", FromEnv: false},
+ wantValue: "2024-10-21",
+ wantEnvVar: "",
+ wantFromEnv: false,
+ },
+ {
+ name: "empty plain value passes through",
+ input: EnvVar{Val: "", EnvVar: "", FromEnv: false},
+ wantValue: "",
+ wantEnvVar: "",
+ wantFromEnv: false,
+ },
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ data, err := json.Marshal(tt.input)
+ if err != nil {
+ t.Fatalf("Marshal failed: %v", err)
+ }
+ var got struct {
+ Value string `json:"value"`
+ EnvVar string `json:"env_var"`
+ FromEnv bool `json:"from_env"`
+ }
+ if err := json.Unmarshal(data, &got); err != nil {
+ t.Fatalf("Unmarshal of marshaled output failed: %v", err)
+ }
+ if got.Value != tt.wantValue {
+ t.Errorf("value: got %q, want %q", got.Value, tt.wantValue)
+ }
+ if got.EnvVar != tt.wantEnvVar {
+ t.Errorf("env_var: got %q, want %q", got.EnvVar, tt.wantEnvVar)
+ }
+ if got.FromEnv != tt.wantFromEnv {
+ t.Errorf("from_env: got %v, want %v", got.FromEnv, tt.wantFromEnv)
+ }
+ })
+ }
+}
+
+// TestEnvVar_MarshalJSON_DoesNotMutateOriginal ensures the auto-redaction in MarshalJSON
+// does not mutate the receiver. The inference path calls GetValue() to build the actual
+// HTTP request to the LLM provider, so the original Val must remain intact.
+func TestEnvVar_MarshalJSON_DoesNotMutateOriginal(t *testing.T) {
+ original := EnvVar{Val: "real-secret-value", EnvVar: "env.SECRET", FromEnv: true}
+ if _, err := json.Marshal(original); err != nil {
+ t.Fatalf("Marshal failed: %v", err)
+ }
+ if original.Val != "real-secret-value" {
+ t.Errorf("MarshalJSON mutated Val: got %q, want %q", original.Val, "real-secret-value")
+ }
+ if original.GetValue() != "real-secret-value" {
+ t.Errorf("GetValue() returns mutated value: got %q", original.GetValue())
+ }
+}
+
+// TestEnvVar_MarshalJSON_RoundTripIsRedacted verifies that a marshaled-then-unmarshaled
+// env-backed EnvVar is recognized as redacted. The merge logic in provider_keys.go relies
+// on this so it can detect "the UI sent back the same redacted value, don't overwrite".
+func TestEnvVar_MarshalJSON_RoundTripIsRedacted(t *testing.T) {
+ original := EnvVar{Val: "sk-1234567890abcdefghijklmnop", EnvVar: "env.KEY", FromEnv: true}
+ data, err := json.Marshal(original)
+ if err != nil {
+ t.Fatalf("Marshal failed: %v", err)
+ }
+ var roundTripped EnvVar
+ if err := json.Unmarshal(data, &roundTripped); err != nil {
+ t.Fatalf("Unmarshal failed: %v", err)
+ }
+ if !roundTripped.IsRedacted() {
+ t.Errorf("Round-tripped env-backed value should be IsRedacted, got Val=%q", roundTripped.Val)
+ }
+ if roundTripped.EnvVar != "env.KEY" {
+ t.Errorf("env_var reference lost in round-trip: got %q, want %q", roundTripped.EnvVar, "env.KEY")
+ }
+}
+
+// TestEnvVar_MarshalJSON_DoesNotAffectGetValue is a critical safety net: marshaling an
+// EnvVar to JSON must NOT change what GetValue() returns. The inference path uses
+// GetValue() to build outgoing LLM requests; if marshaling were to mutate the value,
+// every request after a UI fetch would silently start using the redacted mask as the
+// API key.
+func TestEnvVar_MarshalJSON_DoesNotAffectGetValue(t *testing.T) {
+ os.Setenv("MY_REAL_API_KEY", "sk-real-secret-1234567890abcdef")
+ defer os.Unsetenv("MY_REAL_API_KEY")
+
+ ev := NewEnvVar("env.MY_REAL_API_KEY")
+ if ev.GetValue() != "sk-real-secret-1234567890abcdef" {
+ t.Fatalf("setup: GetValue() = %q, want resolved env value", ev.GetValue())
+ }
+
+ // Marshaling would redact in the JSON output, but must not touch the in-memory Val.
+ if _, err := json.Marshal(ev); err != nil {
+ t.Fatalf("Marshal failed: %v", err)
+ }
+
+ if ev.GetValue() != "sk-real-secret-1234567890abcdef" {
+ t.Errorf("GetValue() returns mutated value after MarshalJSON: got %q", ev.GetValue())
+ }
+}
diff --git a/framework/configstore/clientconfig_redaction_test.go b/framework/configstore/clientconfig_redaction_test.go
new file mode 100644
index 0000000000..8bff430fb0
--- /dev/null
+++ b/framework/configstore/clientconfig_redaction_test.go
@@ -0,0 +1,242 @@
+package configstore
+
+import (
+ "encoding/json"
+ "strings"
+ "testing"
+
+ "github.com/maximhq/bifrost/core/schemas"
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+)
+
+// TestProviderConfig_Redacted_AutoMasksEnvBackedFields verifies that env-backed
+// values in any provider config field are automatically redacted in the JSON output
+// of a Redacted() ProviderConfig — even fields that don't have explicit Redacted()
+// calls (like Azure APIVersion). This is the defense-in-depth guarantee provided
+// by EnvVar.MarshalJSON.
+func TestProviderConfig_Redacted_AutoMasksEnvBackedFields(t *testing.T) {
+ t.Setenv("MY_AZURE_API_VERSION_SECRET", "2024-10-21-preview-secret")
+
+ apiVersion := schemas.NewEnvVar("env.MY_AZURE_API_VERSION_SECRET")
+ require.True(t, apiVersion.IsFromEnv(), "setup: APIVersion should be FromEnv")
+ require.Equal(t, "2024-10-21-preview-secret", apiVersion.GetValue(),
+ "setup: APIVersion should be resolved")
+
+ config := ProviderConfig{
+ Keys: []schemas.Key{{
+ ID: "k1",
+ Name: "test",
+ Value: schemas.EnvVar{Val: ""},
+ AzureKeyConfig: &schemas.AzureKeyConfig{
+ Endpoint: *schemas.NewEnvVar("https://foo.openai.azure.com"),
+ APIVersion: apiVersion,
+ },
+ }},
+ }
+
+ redacted := config.Redacted()
+ require.NotNil(t, redacted)
+ require.Len(t, redacted.Keys, 1)
+ require.NotNil(t, redacted.Keys[0].AzureKeyConfig)
+ require.NotNil(t, redacted.Keys[0].AzureKeyConfig.APIVersion)
+
+ // Marshal the APIVersion field as it would be sent to the UI.
+ data, err := json.Marshal(redacted.Keys[0].AzureKeyConfig.APIVersion)
+ require.NoError(t, err)
+
+ var out struct {
+ Value string `json:"value"`
+ EnvVar string `json:"env_var"`
+ FromEnv bool `json:"from_env"`
+ }
+ require.NoError(t, json.Unmarshal(data, &out))
+
+ assert.NotContains(t, out.Value, "preview-secret",
+ "resolved env value leaked through APIVersion JSON output: %q", out.Value)
+ assert.Equal(t, "env.MY_AZURE_API_VERSION_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")
+}
+
+// TestProviderConfig_Redacted_DoesNotMaskPlainNonSecretFields verifies that the
+// auto-redaction does NOT touch plain (non-env-backed) values. A user-typed
+// api_version like "2024-10-21" must show as-is in the UI.
+func TestProviderConfig_Redacted_DoesNotMaskPlainNonSecretFields(t *testing.T) {
+ config := ProviderConfig{
+ Keys: []schemas.Key{{
+ ID: "k1",
+ Name: "test",
+ Value: schemas.EnvVar{Val: ""},
+ AzureKeyConfig: &schemas.AzureKeyConfig{
+ Endpoint: *schemas.NewEnvVar("https://foo.openai.azure.com"),
+ APIVersion: schemas.NewEnvVar("2024-10-21"),
+ },
+ }},
+ }
+
+ redacted := config.Redacted()
+ require.NotNil(t, redacted)
+ require.Len(t, redacted.Keys, 1)
+ require.NotNil(t, redacted.Keys[0].AzureKeyConfig)
+ require.NotNil(t, redacted.Keys[0].AzureKeyConfig.APIVersion)
+
+ data, err := json.Marshal(redacted.Keys[0].AzureKeyConfig.APIVersion)
+ require.NoError(t, err)
+
+ var out struct {
+ Value string `json:"value"`
+ FromEnv bool `json:"from_env"`
+ }
+ require.NoError(t, json.Unmarshal(data, &out))
+
+ assert.Equal(t, "2024-10-21", out.Value,
+ "plain APIVersion was incorrectly redacted")
+ assert.False(t, out.FromEnv)
+}
+
+// TestProviderConfig_Redacted_PreservesEnvVarReferenceForVertex verifies that
+// env-backed Vertex fields appear in the redacted output with the env reference
+// intact and the resolved value masked. This is the user-facing fix for the
+// "I see resolved env values in the UI" bug.
+func TestProviderConfig_Redacted_PreservesEnvVarReferenceForVertex(t *testing.T) {
+ t.Setenv("MY_VERTEX_PROJECT_ID_SECRET", "super-secret-project-12345")
+
+ projectID := schemas.NewEnvVar("env.MY_VERTEX_PROJECT_ID_SECRET")
+ require.Equal(t, "super-secret-project-12345", projectID.GetValue())
+
+ config := ProviderConfig{
+ Keys: []schemas.Key{{
+ ID: "k1",
+ Name: "test",
+ Value: schemas.EnvVar{Val: ""},
+ VertexKeyConfig: &schemas.VertexKeyConfig{
+ ProjectID: *projectID,
+ Region: *schemas.NewEnvVar("us-central1"),
+ },
+ }},
+ }
+
+ redacted := config.Redacted()
+ data, err := json.Marshal(redacted.Keys[0].VertexKeyConfig.ProjectID)
+ require.NoError(t, err)
+
+ var out struct {
+ Value string `json:"value"`
+ EnvVar string `json:"env_var"`
+ FromEnv bool `json:"from_env"`
+ }
+ 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)
+}
+
+// TestProviderConfig_Redacted_DoesNotMutateOriginal ensures Redacted() and the
+// subsequent JSON marshaling do not mutate the original config in memory. The
+// inference path reads from the in-memory config and calls GetValue() to build
+// outgoing LLM requests; if Redacted() or MarshalJSON were to mutate state, every
+// inference request after a UI fetch would silently start using masked values.
+func TestProviderConfig_Redacted_DoesNotMutateOriginal(t *testing.T) {
+ t.Setenv("MY_REAL_KEY", "sk-real-secret-1234567890abcdef")
+
+ keyValue := schemas.NewEnvVar("env.MY_REAL_KEY")
+ require.Equal(t, "sk-real-secret-1234567890abcdef", keyValue.GetValue())
+
+ config := ProviderConfig{
+ Keys: []schemas.Key{{
+ ID: "k1",
+ Name: "test",
+ Value: *keyValue,
+ }},
+ }
+
+ redacted := config.Redacted()
+ _, err := json.Marshal(redacted)
+ require.NoError(t, err)
+
+ // Original must still hold the resolved value.
+ assert.Equal(t, "sk-real-secret-1234567890abcdef", config.Keys[0].Value.GetValue(),
+ "Redacted() or MarshalJSON mutated the original key Value")
+}
+
+// TestProviderConfig_Redacted_FullJSONHasNoLeakedEnvSecrets is a high-level smoke
+// test: build a config containing env-backed values across multiple provider types
+// and assert that no resolved secret string appears anywhere in the marshaled
+// redacted JSON.
+func TestProviderConfig_Redacted_FullJSONHasNoLeakedEnvSecrets(t *testing.T) {
+ t.Setenv("LEAK_TEST_AZURE_ENDPOINT", "https://leaked-azure.example.com")
+ t.Setenv("LEAK_TEST_AZURE_APIVER", "leaked-api-version-string")
+ t.Setenv("LEAK_TEST_VERTEX_PROJECT", "leaked-vertex-project-id")
+ t.Setenv("LEAK_TEST_BEDROCK_ACCESS", "AKIAIOSFODNN7LEAKED1")
+ t.Setenv("LEAK_TEST_OPENAI_KEY", "sk-leaked-openai-key-1234567890")
+
+ config := ProviderConfig{
+ Keys: []schemas.Key{
+ {
+ ID: "openai-k",
+ Name: "openai",
+ Value: *schemas.NewEnvVar("env.LEAK_TEST_OPENAI_KEY"),
+ },
+ {
+ ID: "azure-k",
+ Name: "azure",
+ Value: schemas.EnvVar{Val: ""},
+ AzureKeyConfig: &schemas.AzureKeyConfig{
+ Endpoint: *schemas.NewEnvVar("env.LEAK_TEST_AZURE_ENDPOINT"),
+ APIVersion: schemas.NewEnvVar("env.LEAK_TEST_AZURE_APIVER"),
+ },
+ },
+ {
+ ID: "vertex-k",
+ Name: "vertex",
+ Value: schemas.EnvVar{Val: ""},
+ VertexKeyConfig: &schemas.VertexKeyConfig{
+ ProjectID: *schemas.NewEnvVar("env.LEAK_TEST_VERTEX_PROJECT"),
+ Region: *schemas.NewEnvVar("us-central1"),
+ },
+ },
+ {
+ ID: "bedrock-k",
+ Name: "bedrock",
+ Value: schemas.EnvVar{Val: ""},
+ BedrockKeyConfig: &schemas.BedrockKeyConfig{
+ AccessKey: *schemas.NewEnvVar("env.LEAK_TEST_BEDROCK_ACCESS"),
+ SecretKey: schemas.EnvVar{Val: ""},
+ },
+ },
+ },
+ }
+
+ redacted := config.Redacted()
+ data, err := json.Marshal(redacted)
+ require.NoError(t, err)
+ jsonStr := string(data)
+
+ leakedSecrets := []string{
+ "https://leaked-azure.example.com",
+ "leaked-api-version-string",
+ "leaked-vertex-project-id",
+ "AKIAIOSFODNN7LEAKED1",
+ "sk-leaked-openai-key-1234567890",
+ }
+ for _, secret := range leakedSecrets {
+ assert.False(t, strings.Contains(jsonStr, secret),
+ "resolved env secret %q leaked into redacted JSON output", secret)
+ }
+
+ // And the env var references must be present so the UI can render them.
+ expectedRefs := []string{
+ "env.LEAK_TEST_OPENAI_KEY",
+ "env.LEAK_TEST_AZURE_ENDPOINT",
+ "env.LEAK_TEST_AZURE_APIVER",
+ "env.LEAK_TEST_VERTEX_PROJECT",
+ "env.LEAK_TEST_BEDROCK_ACCESS",
+ }
+ for _, ref := range expectedRefs {
+ assert.True(t, strings.Contains(jsonStr, ref),
+ "env var reference %q missing from redacted JSON output", ref)
+ }
+}
diff --git a/framework/configstore/tables/encryption_test.go b/framework/configstore/tables/encryption_test.go
index 58297384fd..314b28be59 100644
--- a/framework/configstore/tables/encryption_test.go
+++ b/framework/configstore/tables/encryption_test.go
@@ -1,6 +1,7 @@
package tables
import (
+ "os"
"testing"
"time"
@@ -1724,3 +1725,258 @@ func TestPostgres_EncryptedColumns_AreText(t *testing.T) {
})
}
}
+
+// ============================================================================
+// Env-var-reference persistence regression tests
+//
+// These tests guard against a class of bugs where BeforeSave used GetValue() != ""
+// to decide whether to persist a config field. When a field was set via env var
+// reference (e.g. "env.AZURE_ENDPOINT") and the env var was not set on the server,
+// GetValue() would return "" and the field — including the env reference — would be
+// dropped from the DB. On the next reload the entire provider-specific config block
+// could vanish.
+//
+// IsSet() (which checks both Val and EnvVar) is the correct check, and AfterFind
+// reconstruction must consider all fields in the config, not just one.
+// ============================================================================
+
+// TestTableKey_VertexUnresolvedEnvVar_RoundTrip verifies that a Vertex key configured
+// with an env var reference for ProjectID survives the BeforeSave/AfterFind round-trip
+// even when the env var is NOT set on the server (so the resolved Val is empty).
+func TestTableKey_VertexUnresolvedEnvVar_RoundTrip(t *testing.T) {
+ // Make sure the env var is NOT set so the resolved Val is empty.
+ require.NoError(t, os.Unsetenv("FAKE_VERTEX_PROJECT_ID_FOR_TEST"))
+
+ db := setupTestDB(t)
+
+ key := &TableKey{
+ Name: "vertex-unresolved-env",
+ ProviderID: 1,
+ Provider: "vertex",
+ KeyID: "vertex-env-uuid-1",
+ Value: *schemas.NewEnvVar(""),
+ VertexKeyConfig: &schemas.VertexKeyConfig{
+ ProjectID: schemas.EnvVar{
+ Val: "",
+ EnvVar: "env.FAKE_VERTEX_PROJECT_ID_FOR_TEST",
+ FromEnv: true,
+ },
+ Region: *schemas.NewEnvVar("us-central1"),
+ },
+ }
+
+ require.NoError(t, db.Create(key).Error)
+
+ // Read back through GORM (triggers AfterFind reconstruction).
+ var found TableKey
+ require.NoError(t, db.First(&found, key.ID).Error)
+
+ // 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,
+ "env var reference for ProjectID lost on round-trip")
+ assert.True(t, found.VertexKeyConfig.ProjectID.FromEnv,
+ "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")
+}
+
+// TestTableKey_AzureUnresolvedEnvVar_RoundTrip verifies the same property for Azure.
+// This also exercises the broadened AfterFind reconstruction condition: when only the
+// endpoint is set (and unresolved), the entire AzureKeyConfig must still be reconstructed.
+func TestTableKey_AzureUnresolvedEnvVar_RoundTrip(t *testing.T) {
+ require.NoError(t, os.Unsetenv("FAKE_AZURE_ENDPOINT_FOR_TEST"))
+
+ db := setupTestDB(t)
+
+ key := &TableKey{
+ Name: "azure-unresolved-env",
+ ProviderID: 1,
+ Provider: "azure",
+ KeyID: "azure-env-uuid-1",
+ Value: *schemas.NewEnvVar(""),
+ AzureKeyConfig: &schemas.AzureKeyConfig{
+ Endpoint: schemas.EnvVar{
+ Val: "",
+ EnvVar: "env.FAKE_AZURE_ENDPOINT_FOR_TEST",
+ FromEnv: true,
+ },
+ },
+ }
+
+ require.NoError(t, db.Create(key).Error)
+
+ var found TableKey
+ 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,
+ "env var reference for Endpoint lost on round-trip")
+ assert.True(t, found.AzureKeyConfig.Endpoint.FromEnv,
+ "FromEnv flag for Endpoint lost on round-trip")
+}
+
+// TestTableKey_AzureOnlyApiVersion_AfterFindReconstructs verifies that AzureKeyConfig
+// is reconstructed from the DB even when ONLY a non-endpoint Azure field is set.
+// Before the fix, AfterFind only checked AzureEndpoint != nil and would silently drop
+// the entire Azure config when only api_version (or any other Azure field) was present.
+func TestTableKey_AzureOnlyApiVersion_AfterFindReconstructs(t *testing.T) {
+ db := setupTestDB(t)
+
+ apiVersion := schemas.NewEnvVar("2024-10-21")
+ key := &TableKey{
+ Name: "azure-only-apiversion",
+ ProviderID: 1,
+ Provider: "azure",
+ KeyID: "azure-apiver-uuid-1",
+ Value: *schemas.NewEnvVar(""),
+ AzureKeyConfig: &schemas.AzureKeyConfig{
+ // No endpoint, no client id — only api_version.
+ APIVersion: apiVersion,
+ },
+ }
+
+ require.NoError(t, db.Create(key).Error)
+
+ var found TableKey
+ require.NoError(t, db.First(&found, key.ID).Error)
+
+ require.NotNil(t, found.AzureKeyConfig,
+ "AzureKeyConfig should be reconstructed when only api_version is present")
+ require.NotNil(t, found.AzureKeyConfig.APIVersion)
+ assert.Equal(t, "2024-10-21", found.AzureKeyConfig.APIVersion.GetValue())
+}
+
+// TestTableKey_BedrockUnresolvedEnvVar_RoundTrip verifies the same property for
+// Bedrock explicit credentials.
+func TestTableKey_BedrockUnresolvedEnvVar_RoundTrip(t *testing.T) {
+ require.NoError(t, os.Unsetenv("FAKE_AWS_ACCESS_KEY_FOR_TEST"))
+ require.NoError(t, os.Unsetenv("FAKE_AWS_SECRET_KEY_FOR_TEST"))
+
+ db := setupTestDB(t)
+
+ key := &TableKey{
+ Name: "bedrock-unresolved-env",
+ ProviderID: 1,
+ Provider: "bedrock",
+ KeyID: "bedrock-env-uuid-1",
+ Value: *schemas.NewEnvVar(""),
+ BedrockKeyConfig: &schemas.BedrockKeyConfig{
+ AccessKey: schemas.EnvVar{
+ Val: "",
+ EnvVar: "env.FAKE_AWS_ACCESS_KEY_FOR_TEST",
+ FromEnv: true,
+ },
+ SecretKey: schemas.EnvVar{
+ Val: "",
+ EnvVar: "env.FAKE_AWS_SECRET_KEY_FOR_TEST",
+ FromEnv: true,
+ },
+ Region: schemas.NewEnvVar("us-west-2"),
+ },
+ }
+
+ require.NoError(t, db.Create(key).Error)
+
+ var found TableKey
+ 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,
+ "env var reference for AccessKey lost on round-trip")
+ assert.Equal(t, "env.FAKE_AWS_SECRET_KEY_FOR_TEST", found.BedrockKeyConfig.SecretKey.EnvVar,
+ "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())
+}
+
+// TestTableKey_OllamaUnresolvedEnvVar_RoundTrip and TestTableKey_SGLUnresolvedEnvVar_RoundTrip
+// verify the same property for the recently-added providers, which also use env-aware persistence.
+func TestTableKey_OllamaUnresolvedEnvVar_RoundTrip(t *testing.T) {
+ require.NoError(t, os.Unsetenv("FAKE_OLLAMA_URL_FOR_TEST"))
+
+ db := setupTestDB(t)
+
+ key := &TableKey{
+ Name: "ollama-unresolved-env",
+ ProviderID: 1,
+ Provider: "ollama",
+ KeyID: "ollama-env-uuid-1",
+ Value: *schemas.NewEnvVar(""),
+ OllamaKeyConfig: &schemas.OllamaKeyConfig{
+ URL: schemas.EnvVar{
+ Val: "",
+ EnvVar: "env.FAKE_OLLAMA_URL_FOR_TEST",
+ FromEnv: true,
+ },
+ },
+ }
+
+ require.NoError(t, db.Create(key).Error)
+
+ var found TableKey
+ 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)
+}
+
+func TestTableKey_SGLUnresolvedEnvVar_RoundTrip(t *testing.T) {
+ require.NoError(t, os.Unsetenv("FAKE_SGL_URL_FOR_TEST"))
+
+ db := setupTestDB(t)
+
+ key := &TableKey{
+ Name: "sgl-unresolved-env",
+ ProviderID: 1,
+ Provider: "sgl",
+ KeyID: "sgl-env-uuid-1",
+ Value: *schemas.NewEnvVar(""),
+ SGLKeyConfig: &schemas.SGLKeyConfig{
+ URL: schemas.EnvVar{
+ Val: "",
+ EnvVar: "env.FAKE_SGL_URL_FOR_TEST",
+ FromEnv: true,
+ },
+ },
+ }
+
+ require.NoError(t, db.Create(key).Error)
+
+ var found TableKey
+ 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)
+}
+
+// TestTableKey_VertexPlainValue_RoundTrip is a sanity check ensuring that plain
+// (non-env-backed) values still round-trip cleanly through the persistence layer
+// after the IsSet() change. Both branches of the BeforeSave check matter.
+func TestTableKey_VertexPlainValue_RoundTrip(t *testing.T) {
+ db := setupTestDB(t)
+
+ key := &TableKey{
+ Name: "vertex-plain",
+ ProviderID: 1,
+ Provider: "vertex",
+ KeyID: "vertex-plain-uuid-1",
+ Value: *schemas.NewEnvVar(""),
+ VertexKeyConfig: &schemas.VertexKeyConfig{
+ ProjectID: *schemas.NewEnvVar("my-gcp-project"),
+ Region: *schemas.NewEnvVar("us-central1"),
+ },
+ }
+
+ require.NoError(t, db.Create(key).Error)
+
+ var found TableKey
+ require.NoError(t, db.First(&found, key.ID).Error)
+
+ require.NotNil(t, found.VertexKeyConfig)
+ assert.Equal(t, "my-gcp-project", found.VertexKeyConfig.ProjectID.GetValue())
+ assert.False(t, found.VertexKeyConfig.ProjectID.FromEnv)
+ assert.Equal(t, "us-central1", found.VertexKeyConfig.Region.GetValue())
+}
diff --git a/framework/configstore/tables/key.go b/framework/configstore/tables/key.go
index 73ba61f2e1..e790f73a00 100644
--- a/framework/configstore/tables/key.go
+++ b/framework/configstore/tables/key.go
@@ -130,7 +130,7 @@ func (k *TableKey) BeforeSave(tx *gorm.DB) error {
// shared pointer, the caller's in-memory config is silently corrupted.
// See: TestBeforeSave_DoesNotMutateSharedProviderConfigs
if k.AzureKeyConfig != nil {
- if k.AzureKeyConfig.Endpoint.GetValue() != "" {
+ if k.AzureKeyConfig.Endpoint.IsSet() {
ep := k.AzureKeyConfig.Endpoint
k.AzureEndpoint = &ep
} else {
@@ -179,25 +179,25 @@ func (k *TableKey) BeforeSave(tx *gorm.DB) error {
k.AzureScopesJSON = nil
}
if k.VertexKeyConfig != nil {
- if k.VertexKeyConfig.ProjectID.GetValue() != "" {
+ if k.VertexKeyConfig.ProjectID.IsSet() {
pid := k.VertexKeyConfig.ProjectID
k.VertexProjectID = &pid
} else {
k.VertexProjectID = nil
}
- if k.VertexKeyConfig.ProjectNumber.GetValue() != "" {
+ if k.VertexKeyConfig.ProjectNumber.IsSet() {
pn := k.VertexKeyConfig.ProjectNumber
k.VertexProjectNumber = &pn
} else {
k.VertexProjectNumber = nil
}
- if k.VertexKeyConfig.Region.GetValue() != "" {
+ if k.VertexKeyConfig.Region.IsSet() {
vr := k.VertexKeyConfig.Region
k.VertexRegion = &vr
} else {
k.VertexRegion = nil
}
- if k.VertexKeyConfig.AuthCredentials.GetValue() != "" {
+ if k.VertexKeyConfig.AuthCredentials.IsSet() {
ac := k.VertexKeyConfig.AuthCredentials
k.VertexAuthCredentials = &ac
} else {
@@ -210,14 +210,14 @@ func (k *TableKey) BeforeSave(tx *gorm.DB) error {
k.VertexAuthCredentials = nil
}
if k.BedrockKeyConfig != nil {
- if k.BedrockKeyConfig.AccessKey.GetValue() != "" {
+ if k.BedrockKeyConfig.AccessKey.IsSet() {
// Copy to avoid encrypting the shared BedrockKeyConfig through the pointer
ak := k.BedrockKeyConfig.AccessKey
k.BedrockAccessKey = &ak
} else {
k.BedrockAccessKey = nil
}
- if k.BedrockKeyConfig.SecretKey.GetValue() != "" {
+ if k.BedrockKeyConfig.SecretKey.IsSet() {
// Copy to avoid encrypting the shared BedrockKeyConfig through the pointer
sk := k.BedrockKeyConfig.SecretKey
k.BedrockSecretKey = &sk
@@ -298,7 +298,7 @@ func (k *TableKey) BeforeSave(tx *gorm.DB) error {
}
if k.VLLMKeyConfig != nil {
- if k.VLLMKeyConfig.URL.GetValue() != "" {
+ if k.VLLMKeyConfig.URL.IsSet() {
u := k.VLLMKeyConfig.URL // Value-copy to prevent shared pointer mutation
k.VLLMUrl = &u
} else {
@@ -322,14 +322,14 @@ func (k *TableKey) BeforeSave(tx *gorm.DB) error {
k.ReplicateUseDeploymentsEndpoint = nil
}
- if k.OllamaKeyConfig != nil && k.OllamaKeyConfig.URL.GetValue() != "" {
+ if k.OllamaKeyConfig != nil && k.OllamaKeyConfig.URL.IsSet() {
u := k.OllamaKeyConfig.URL
k.OllamaUrl = &u
} else {
k.OllamaUrl = nil
}
- if k.SGLKeyConfig != nil && k.SGLKeyConfig.URL.GetValue() != "" {
+ if k.SGLKeyConfig != nil && k.SGLKeyConfig.URL.IsSet() {
u := k.SGLKeyConfig.URL
k.SGLUrl = &u
} else {
@@ -522,7 +522,7 @@ func (k *TableKey) AfterFind(tx *gorm.DB) error {
k.UseForBatchAPI = &useForBatchAPI
}
// Reconstruct Azure config if fields are present
- if k.AzureEndpoint != nil {
+ if k.AzureEndpoint != nil || k.AzureAPIVersion != nil || k.AzureClientID != nil || k.AzureClientSecret != nil || k.AzureTenantID != nil || (k.AzureScopesJSON != nil && *k.AzureScopesJSON != "") {
var scopes []string
if k.AzureScopesJSON != nil && *k.AzureScopesJSON != "" {
if err := json.Unmarshal([]byte(*k.AzureScopesJSON), &scopes); err != nil {
diff --git a/ui/app/workspace/providers/fragments/apiKeysFormFragment.tsx b/ui/app/workspace/providers/fragments/apiKeysFormFragment.tsx
index 33413edcd1..3a7888b0a5 100644
--- a/ui/app/workspace/providers/fragments/apiKeysFormFragment.tsx
+++ b/ui/app/workspace/providers/fragments/apiKeysFormFragment.tsx
@@ -11,6 +11,7 @@ import { Separator } from "@/components/ui/separator";
import { Switch } from "@/components/ui/switch";
import { Tabs, TabsList, TabsTrigger } from "@/components/ui/tabs";
import { TagInput } from "@/components/ui/tagInput";
+import { Textarea } from "@/components/ui/textarea";
import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from "@/components/ui/tooltip";
import { isRedacted } from "@/lib/utils/validation";
import { Info, Plus, Trash2 } from "lucide-react";
@@ -94,48 +95,69 @@ export function ApiKeyFormFragment({ control, providerName, form }: Props) {
// Auth type state for Bedrock: 'iam_role', 'explicit', or 'api_key'
const [bedrockAuthType, setBedrockAuthType] = useState<"iam_role" | "explicit" | "api_key">("iam_role");
+ // Auth type state for Vertex: 'service_account', 'service_account_json', or 'api_key'
+ const [vertexAuthType, setVertexAuthType] = useState<"service_account" | "service_account_json" | "api_key">("service_account");
+
// Detect auth type from existing form values when editing
useEffect(() => {
if (form.formState.isDirty) return;
if (isAzure) {
- const clientId = form.getValues("key.azure_key_config.client_id")?.value;
- const clientSecret = form.getValues("key.azure_key_config.client_secret")?.value;
- const tenantId = form.getValues("key.azure_key_config.tenant_id")?.value;
- const apiKey = form.getValues("key.value")?.value;
- if (clientId || clientSecret || tenantId) {
- setAzureAuthType("entra_id");
- } else if (!apiKey) {
- setAzureAuthType("default_credential");
+ const clientId = form.getValues("key.azure_key_config.client_id");
+ const clientSecret = form.getValues("key.azure_key_config.client_secret");
+ const tenantId = form.getValues("key.azure_key_config.tenant_id");
+ const apiKey = form.getValues("key.value");
+ const hasEntraField = clientId?.value || clientId?.env_var || clientSecret?.value || clientSecret?.env_var || tenantId?.value || tenantId?.env_var;
+ const hasApiKey = apiKey?.value || apiKey?.env_var;
+ let detected: "api_key" | "entra_id" | "default_credential" = "api_key";
+ if (hasEntraField) {
+ detected = "entra_id";
+ } else if (!hasApiKey) {
+ detected = "default_credential";
}
+ setAzureAuthType(detected);
+ form.setValue("key.azure_key_config._auth_type", detected);
}
}, [isAzure, form]);
useEffect(() => {
if (form.formState.isDirty) return;
- if (isBedrock) {
- const accessKey = form.getValues("key.bedrock_key_config.access_key")?.value;
- const secretKey = form.getValues("key.bedrock_key_config.secret_key")?.value;
+ if (isVertex) {
+ const authCredentials = form.getValues("key.vertex_key_config.auth_credentials")?.value;
+ const authCredentialsEnv = form.getValues("key.vertex_key_config.auth_credentials")?.env_var;
const apiKey = form.getValues("key.value")?.value;
- if (accessKey || secretKey) {
- setBedrockAuthType("explicit");
- } else if (apiKey) {
- setBedrockAuthType("api_key");
+ const apiKeyEnv = form.getValues("key.value")?.env_var;
+ let detected: "service_account" | "service_account_json" | "api_key" = "service_account";
+ if (authCredentials || authCredentialsEnv) {
+ detected = "service_account_json";
+ } else if (apiKey || apiKeyEnv) {
+ detected = "api_key";
+ }
+ setVertexAuthType(detected);
+ form.setValue("key.vertex_key_config._auth_type", detected);
+ }
+ }, [isVertex, form]);
+
+ useEffect(() => {
+ if (form.formState.isDirty) return;
+ if (isBedrock) {
+ const accessKey = form.getValues("key.bedrock_key_config.access_key");
+ const secretKey = form.getValues("key.bedrock_key_config.secret_key");
+ const apiKey = form.getValues("key.value");
+ const hasExplicitCreds = accessKey?.value || accessKey?.env_var || secretKey?.value || secretKey?.env_var;
+ const hasApiKey = apiKey?.value || apiKey?.env_var;
+ let detected: "iam_role" | "explicit" | "api_key" = "iam_role";
+ if (hasExplicitCreds) {
+ detected = "explicit";
+ } else if (hasApiKey) {
+ detected = "api_key";
}
+ setBedrockAuthType(detected);
+ form.setValue("key.bedrock_key_config._auth_type", detected);
}
}, [isBedrock, form]);
return (
- {isVertex && (
-
-
- Authentication Methods
-
- You can either use service account authentication or API key authentication. Please leave API Key empty when using service
- account authentication.
-
-
- )}
- {/* Hide API Key field for Azure when using Entra ID/Default Credential, and for Bedrock when not using API Key auth */}
- {!(isAzure && (azureAuthType === "entra_id" || azureAuthType === "default_credential")) && !(isBedrock) && (
+ {/* Hide API Key field for providers with dedicated auth tabs */}
+ {!isAzure && !isBedrock && !isVertex && (
(
- API Key {isVertex ? "(Supported only for gemini and fine-tuned models)" : isVLLM ? "(Optional)" : ""}
+ API Key {isVLLM ? "(Optional)" : ""}
@@ -377,6 +399,7 @@ export function ApiKeyFormFragment({ control, providerName, form }: Props) {
value={azureAuthType}
onValueChange={(v) => {
setAzureAuthType(v as "api_key" | "entra_id" | "default_credential");
+ form.setValue("key.azure_key_config._auth_type", v, { shouldDirty: true, shouldValidate: true });
if (v === "entra_id" || v === "default_credential") {
// Clear API key when switching away from API Key
form.setValue("key.value", undefined, { shouldDirty: true });
@@ -391,15 +414,15 @@ export function ApiKeyFormFragment({ control, providerName, form }: Props) {
}}
>
+
+ Default Credential
+
API Key
Entra ID (Service Principal)
-
- Default Credential
-
@@ -538,6 +561,42 @@ export function ApiKeyFormFragment({ control, providerName, form }: Props) {
{isVertex && (
+
+ Authentication Method
+ {
+ setVertexAuthType(v as "service_account" | "service_account_json" | "api_key");
+ form.setValue("key.vertex_key_config._auth_type", v, { shouldDirty: true, shouldValidate: true });
+ if (v === "service_account" || v === "api_key") {
+ // Clear auth credentials when switching away from service account JSON
+ form.setValue("key.vertex_key_config.auth_credentials", undefined, { shouldDirty: true });
+ }
+ if (v === "service_account" || v === "service_account_json") {
+ // Clear API key when switching away from API Key
+ form.setValue("key.value", undefined, { shouldDirty: true });
+ }
+ }}
+ >
+
+
+ Service Account (Attached)
+
+
+ Service Account (JSON)
+
+
+ API Key
+
+
+
+ {vertexAuthType === "service_account" && (
+
+ Uses the service account attached to your environment (GCE, GKE, Cloud Run). No credentials required.
+
+ )}
+
+
)}
/>
-
-
- Service Account Authentication
-
- Leave both API Key and Auth Credentials empty to use service account attached to your environment.
-
-
- (
-
- Auth Credentials
- Service account JSON object or env.VAR_NAME
-
-
-
-
- {isRedacted(field.value?.value ?? "") && (
-
-
- Credentials are stored securely. Edit to update.
-