feat: use *SecretVar for OAuth2Config credentials to preserve env./vault.

core/schemas/oauth.go

@@ -78,17 +78,17 @@ type OAuth2Provider interface {
 
 // OauthConfig represents OAuth client configuration
 type OAuth2Config struct {
-	ID              string   `json:"id"`
-	ClientID        string   `json:"client_id,omitempty"`        // Optional: Will be obtained via dynamic registration (RFC 7591) if not provided
-	ClientSecret    string   `json:"client_secret,omitempty"`    // Optional: For public clients using PKCE, or obtained via dynamic registration
-	AuthorizeURL    string   `json:"authorize_url,omitempty"`    // Optional: Will be discovered from ServerURL if not provided
-	TokenURL        string   `json:"token_url,omitempty"`        // Optional: Will be discovered from ServerURL if not provided
-	RegistrationURL *string  `json:"registration_url,omitempty"` // Optional: For dynamic client registration (RFC 7591), can be discovered
-	RedirectURI     string   `json:"redirect_uri"`               // Required
-	Scopes          []string `json:"scopes,omitempty"`           // Optional: Can be discovered
-	ServerURL       string   `json:"server_url"`                 // MCP server URL for OAuth discovery (required if URLs not provided)
-	Resource        string   `json:"resource,omitempty"`         // Optional OAuth resource indicator (RFC 8707); omitted when empty
-	UseDiscovery    bool     `json:"use_discovery,omitempty"`    // Deprecated: Discovery now happens automatically when URLs are missing
+	ID              string     `json:"id"`
+	ClientID        *SecretVar `json:"client_id,omitempty"`        // Optional: Will be obtained via dynamic registration (RFC 7591) if not provided. Supports env./vault. references.
+	ClientSecret    *SecretVar `json:"client_secret,omitempty"`    // Optional: For public clients using PKCE, or obtained via dynamic registration. Supports env./vault. references.
+	AuthorizeURL    string     `json:"authorize_url,omitempty"`    // Optional: Will be discovered from ServerURL if not provided
+	TokenURL        string     `json:"token_url,omitempty"`        // Optional: Will be discovered from ServerURL if not provided
+	RegistrationURL *string    `json:"registration_url,omitempty"` // Optional: For dynamic client registration (RFC 7591), can be discovered
+	RedirectURI     string     `json:"redirect_uri"`               // Required
+	Scopes          []string   `json:"scopes,omitempty"`           // Optional: Can be discovered
+	ServerURL       string     `json:"server_url"`                 // MCP server URL for OAuth discovery (required if URLs not provided)
+	Resource        string     `json:"resource,omitempty"`         // Optional OAuth resource indicator (RFC 8707); omitted when empty
+	UseDiscovery    bool       `json:"use_discovery,omitempty"`    // Deprecated: Discovery now happens automatically when URLs are missing
 }
 
 // OauthToken represents OAuth access and refresh tokens

framework/oauth2/main.go

@@ -653,10 +653,10 @@ func (p *OAuth2Provider) InitiateOAuthFlow(ctx context.Context, config *schemas.
 
 	// Dynamic Client Registration (RFC 7591)
 	// If client_id is NOT provided, attempt dynamic registration
-	clientID := config.ClientID // storage value — may be "env.MY_VAR" reference or plain ID
+	clientID := config.ClientID // carries env./vault. reference metadata; resolved at use time
 	clientSecret := config.ClientSecret
 
-	if clientID == "" {
+	if !clientID.IsSet() {
 		// Check if registration URL is available
 		if registrationURL == nil || *registrationURL == "" {
 			return nil, fmt.Errorf("client_id is required when the OAuth provider does not support dynamic client registration (RFC 7591). Please provide client_id manually or use an OAuth provider that supports dynamic registration")
@@ -684,11 +684,17 @@ func (p *OAuth2Provider) InitiateOAuthFlow(ctx context.Context, config *schemas.
 			return nil, fmt.Errorf("dynamic client registration failed: %w. Please provide client_id manually", err)
 		}
 
-		// Use dynamically registered credentials
-		clientID = regResp.ClientID
-		clientSecret = regResp.ClientSecret // May be empty for public clients
+		// Use dynamically registered credentials. Literal values, not
+		// NewSecretVar: these are opaque strings from an external
+		// authorization server's registration response, and NewSecretVar
+		// would parse an "env."/"vault." prefix as a reference — a
+		// registered client_id/client_secret happening to start with one of
+		// those prefixes would then resolve a local deployment secret
+		// instead of being stored as-is.
+		clientID = &schemas.SecretVar{Val: regResp.ClientID}
+		clientSecret = &schemas.SecretVar{Val: regResp.ClientSecret} // May be empty for public clients
 
-		logger.Debug("Dynamic client registration successful: client_id: %s, has_secret: %t", clientID, clientSecret != "")
+		logger.Debug("Dynamic client registration successful: client_id: %s, has_secret: %t", regResp.ClientID, clientSecret.IsSet())
 	}
 
 	// Generate PKCE challenge
@@ -714,8 +720,8 @@ func (p *OAuth2Provider) InitiateOAuthFlow(ctx context.Context, config *schemas.
 	expiresAt := time.Now().Add(15 * time.Minute)
 	oauthConfigRecord := &tables.TableOauthConfig{
 		ID:              oauthConfigID,
-		ClientID:        schemas.NewSecretVar(clientID), // May be from dynamic registration
-		ClientSecret:    schemas.NewSecretVar(clientSecret),
+		ClientID:        clientID, // May be from dynamic registration
+		ClientSecret:    clientSecret,
 		AuthorizeURL:    authorizeURL,
 		TokenURL:        tokenURL,
 		RegistrationURL: registrationURL,
@@ -773,7 +779,7 @@ func (p *OAuth2Provider) InitiateOAuthFlow(ctx context.Context, config *schemas.
 
 	// Resolve env var reference to actual value for use in the authorize URL.
 	// The reference ("env.MY_VAR") is stored in DB; the resolved value is sent to the provider.
-	resolvedClientID := schemas.NewSecretVar(clientID).GetValue()
+	resolvedClientID := clientID.GetValue()
 
 	// Build authorize URL with PKCE (using dynamically registered or user-provided client_id)
 	authURL := p.buildAuthorizeURLWithPKCE(

transports/bifrost-http/handlers/mcp.go

@@ -540,22 +540,21 @@ func (h *MCPHandler) verifyMCPClientHeaders(ctx *fasthttp.RequestCtx) {
 }
 
 // pendingOAuthConfigToRequest converts the persisted shared-OAuth bootstrap
-// shape (plain strings on OAuth2Config) into the request shape consumed by
-// runOAuthBootstrap / InitiateOAuthFlow (*EnvVar on credentials). The
-// non-empty Val / empty EnvVar combination is the canonical "plaintext
-// value, not an env reference" form.
+// shape into the request shape consumed by runOAuthBootstrap /
+// InitiateOAuthFlow. Credentials are *SecretVar on both sides, so env./vault.
+// reference metadata passes through intact.
 func pendingOAuthConfigToRequest(cfg *schemas.OAuth2Config) *OAuthConfigRequest {
 	req := &OAuthConfigRequest{
 		AuthorizeURL: cfg.AuthorizeURL,
 		TokenURL:     cfg.TokenURL,
 		Scopes:       cfg.Scopes,
 		Resource:     cfg.Resource,
 	}
-	if cfg.ClientID != "" {
-		req.ClientID = &schemas.SecretVar{Val: cfg.ClientID}
+	if cfg.ClientID.IsSet() {
+		req.ClientID = cfg.ClientID
 	}
-	if cfg.ClientSecret != "" {
-		req.ClientSecret = &schemas.SecretVar{Val: cfg.ClientSecret}
+	if cfg.ClientSecret.IsSet() {
+		req.ClientSecret = cfg.ClientSecret
 	}
 	if cfg.RegistrationURL != nil {
 		req.RegistrationURL = *cfg.RegistrationURL

transports/bifrost-http/handlers/mcpoauth2.go

@@ -249,22 +249,9 @@ func (h *OAuthHandler) InitiateOAuthFlow(ctx context.Context, req OAuthInitiatio
 		registrationURL = &req.RegistrationURL
 	}
 
-	clientID := ""
-	if req.ClientID != nil {
-		if v, _ := req.ClientID.Value(); v != nil {
-			clientID, _ = v.(string)
-		}
-	}
-	clientSecret := ""
-	if req.ClientSecret != nil {
-		if v, _ := req.ClientSecret.Value(); v != nil {
-			clientSecret, _ = v.(string)
-		}
-	}
-
 	config := &schemas.OAuth2Config{
-		ClientID:        clientID,
-		ClientSecret:    clientSecret,
+		ClientID:        req.ClientID,
+		ClientSecret:    req.ClientSecret,
 		AuthorizeURL:    req.AuthorizeURL,
 		TokenURL:        req.TokenURL,
 		RegistrationURL: registrationURL,

transports/bifrost-http/lib/config.go

@@ -1921,24 +1921,24 @@ func rotateMCPOauthConfigFromFile(ctx context.Context, store configstore.ConfigS
 			return true
 		}
 	}
-	// NewSecretVar resolves env./vault. references immediately and sets Val
-	// to "" when the reference doesn't resolve (e.g. the env var isn't set
-	// on this boot). Guard against persisting that empty value: it would
+	// IsSet() is true for a declared env./vault. reference even when it
+	// doesn't resolve (ref present, Val ""; see its own doc comment). Guard
+	// against persisting that empty value on top of IsSet(): it would
 	// silently blank a real stored credential and RotateMCPOAuthConfig would
 	// see it as drift, cascading every bound token to needs_reauth over a
 	// transient env var absence. Skip the field (keep the stored value) and
 	// warn instead.
-	if fileBlock.ClientID != "" {
-		if declared := schemas.NewSecretVar(fileBlock.ClientID); declared.GetValue() != "" {
-			fields.ClientID = declared
+	if fileBlock.ClientID.IsSet() {
+		if fileBlock.ClientID.GetValue() != "" {
+			fields.ClientID = fileBlock.ClientID
 		} else {
 			logger.Warn("oauth client_id declared for MCP client %q resolves to an empty value; keeping the stored value", clientName)
 			incomplete = true
 		}
 	}
-	if fileBlock.ClientSecret != "" {
-		if declared := schemas.NewSecretVar(fileBlock.ClientSecret); declared.GetValue() != "" {
-			fields.ClientSecret = declared
+	if fileBlock.ClientSecret.IsSet() {
+		if fileBlock.ClientSecret.GetValue() != "" {
+			fields.ClientSecret = fileBlock.ClientSecret
 		} else {
 			logger.Warn("oauth client_secret declared for MCP client %q resolves to an empty value; keeping the stored value", clientName)
 			incomplete = true
@@ -6790,18 +6790,13 @@ func (c *Config) RedactMCPClientConfig(config *schemas.MCPClientConfig) *schemas
 	}
 
 	// Redact credentials inside the inline `oauth_config` bootstrap block.
-	// Its fields are plain strings, so route them through the SecretVar
-	// redaction to keep the wire format identical to the sibling
-	// oauth_client_id / oauth_client_secret fields. Copy the struct first —
-	// configCopy shares the pointer with the live config.
+	// Copy the struct first — configCopy shares the pointer with the live
+	// config, and Redacted() returns a fresh SecretVar so the live stash is
+	// never mutated.
 	if config.PendingOAuthConfig != nil {
 		pendingCopy := *config.PendingOAuthConfig
-		if pendingCopy.ClientID != "" {
-			pendingCopy.ClientID = (&schemas.SecretVar{Val: pendingCopy.ClientID}).Redacted().Val
-		}
-		if pendingCopy.ClientSecret != "" {
-			pendingCopy.ClientSecret = (&schemas.SecretVar{Val: pendingCopy.ClientSecret}).Redacted().Val
-		}
+		pendingCopy.ClientID = pendingCopy.ClientID.Redacted()
+		pendingCopy.ClientSecret = pendingCopy.ClientSecret.Redacted()
 		configCopy.PendingOAuthConfig = &pendingCopy
 	}
 

transports/bifrost-http/lib/config_test.go

@@ -3166,12 +3166,12 @@ func TestPinMCPClientImmutableFields(t *testing.T) {
 		// perturb mid-flight.
 		existing := baseExisting()
 		existing.OauthConfigID = nil
-		existing.PendingOAuthConfig = &schemas.OAuth2Config{ClientID: "abc", Scopes: []string{"read"}}
+		existing.PendingOAuthConfig = &schemas.OAuth2Config{ClientID: schemas.NewSecretVar("abc"), Scopes: []string{"read"}}
 		fileClient := fileClientFor(existing)
-		fileClient.PendingOAuthConfig = &schemas.OAuth2Config{ClientID: "xyz", Scopes: []string{"read"}}
+		fileClient.PendingOAuthConfig = &schemas.OAuth2Config{ClientID: schemas.NewSecretVar("xyz"), Scopes: []string{"read"}}
 		changed := pinMCPClientImmutableFields(fileClient, existing)
 		require.Empty(t, changed, "oauth_config is no longer part of the immutable-fields report")
-		require.Equal(t, "abc", fileClient.PendingOAuthConfig.ClientID)
+		require.Equal(t, "abc", fileClient.PendingOAuthConfig.ClientID.GetValue())
 	})
 
 	t.Run("per_user_headers key schema cannot be emptied", func(t *testing.T) {
@@ -3255,7 +3255,7 @@ func TestMergeMCPConfig_OauthCredentialDriftTriggersRotation(t *testing.T) {
 				ConnectionString: schemas.NewSecretVar("https://mcp.notion.so/sse"),
 				AuthType:         schemas.MCPAuthTypeOauth,
 				PendingOAuthConfig: &schemas.OAuth2Config{
-					ClientID: "new-client-id",
+					ClientID: schemas.NewSecretVar("new-client-id"),
 				},
 			},
 		},
@@ -3354,7 +3354,7 @@ func TestMergeMCPConfig_OauthNoDriftDoesNotTriggerRotation(t *testing.T) {
 				ConnectionString: schemas.NewSecretVar("https://mcp.notion.so/sse"),
 				AuthType:         schemas.MCPAuthTypeOauth,
 				PendingOAuthConfig: &schemas.OAuth2Config{
-					ClientID:     "stored-client-id",
+					ClientID:     schemas.NewSecretVar("stored-client-id"),
 					AuthorizeURL: "https://auth.example.com/authorize",
 					TokenURL:     "https://auth.example.com/token",
 					Scopes:       []string{"read"},
@@ -3394,7 +3394,7 @@ func TestMergeMCPConfig_UnresolvedSecretRefDoesNotCheckpointConfigHash(t *testin
 				ConnectionString: schemas.NewSecretVar("https://mcp.notion.so/sse"),
 				AuthType:         schemas.MCPAuthTypeOauth,
 				PendingOAuthConfig: &schemas.OAuth2Config{
-					ClientID: "env.BIFROST_TEST_UNSET_CLIENT_ID_XYZ",
+					ClientID: schemas.NewSecretVar("env.BIFROST_TEST_UNSET_CLIENT_ID_XYZ"),
 				},
 			},
 		},
@@ -3512,7 +3512,7 @@ func TestSyncMCPConfigFromFile_OauthCredentialDriftTriggersRotation(t *testing.T
 					ConnectionString: schemas.NewSecretVar("https://mcp.notion.so/sse"),
 					AuthType:         schemas.MCPAuthTypeOauth,
 					PendingOAuthConfig: &schemas.OAuth2Config{
-						ClientID: "new-client-id-sync",
+						ClientID: schemas.NewSecretVar("new-client-id-sync"),
 					},
 				},
 			},