Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions framework/configstore/clientconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ type ClientConfig struct {
RoutingChainMaxDepth int `json:"routing_chain_max_depth"` // Maximum depth for routing rule chain evaluation (default: 10)
MCPExternalClientURL *schemas.SecretVar `json:"mcp_external_client_url,omitempty"` // Public base URL used as redirect_uri when Bifrost acts as an OAuth client to upstream MCP servers. Supports env var syntax ("env.MY_VAR")
ConfigHash string `json:"-"` // Config hash for reconciliation (not serialized)
DumpErrorsInConsoleLogs bool `json:"dump_errors_in_console_logs"` // Dump error details in console logs
}

// UnmarshalJSON defaults all bool fields to true when absent from JSON.
Expand Down Expand Up @@ -236,6 +237,11 @@ func (c *ClientConfig) GenerateClientConfigHash() (string, error) {
hash.Write([]byte("asyncJobResultTTL:0"))
}

// Only hash non-default value to avoid legacy config hash churn on upgrade.
if c.DumpErrorsInConsoleLogs {
hash.Write([]byte("dumpErrorsInConsoleLogs:true"))
}

// Hash integer fields
data, err := sonic.Marshal(c.InitialPoolSize)
if err != nil {
Expand Down
30 changes: 30 additions & 0 deletions framework/configstore/migrations.go
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,7 @@ var configstoreMigrationSteps = []migrationStep{
{IDs: []string{"add_customer_name_unique_constraint_dedup", "add_customer_name_unique_constraint_index"}, run: migrationAddCustomerNameUniqueConstraint},
{IDs: []string{"null_legacy_customer_budget_id_refs"}, run: migrationNullLegacyCustomerBudgetID},
{IDs: []string{"add_skills_repo_tables"}, run: migrationAddSkillsRepoTables},
{IDs: []string{"add_dump_errors_in_console_logs_column"}, run: migrationAddDumpErrorsInConsoleLogsColumn},
}

// quoteSQLiteIdentifier quotes a SQLite identifier, escaping any double quotes.
Expand Down Expand Up @@ -4277,6 +4278,35 @@ func migrationAddDisableDBPingsInHealthColumn(ctx context.Context, db *gorm.DB,
return nil
}

// migrationAddDumpErrorsInConsoleLogsColumn adds the dump_errors_in_console_logs column to the client config table
func migrationAddDumpErrorsInConsoleLogsColumn(ctx context.Context, db *gorm.DB, logger schemas.Logger) error {
migrationName := "add_dump_errors_in_console_logs_column"
logger.Info("[configstore] starting migration %s", migrationName)
defer logger.Info("[configstore] finished migration %s", migrationName)
m := migrator.New(db, migrator.DefaultOptions, []*migrator.Migration{{
ID: migrationName,
Migrate: func(tx *gorm.DB) error {
tx = tx.WithContext(ctx)
if err := addColumnIfNotExists(tx, logger, &tables.TableClientConfig{}, "dump_errors_in_console_logs"); err != nil {
return err
}
return nil
},
Rollback: func(tx *gorm.DB) error {
tx = tx.WithContext(ctx)
if err := dropColumnIfExists(tx, logger, &tables.TableClientConfig{}, "dump_errors_in_console_logs"); err != nil {
return err
}
return nil
},
}})
err := m.Migrate()
if err != nil {
return fmt.Errorf("error while running db migration: %s", err.Error())
}
return nil
}

// migrationAddIsPingAvailableColumnToMCPClientTable adds the is_ping_available column to the config_mcp_clients table
func migrationAddIsPingAvailableColumnToMCPClientTable(ctx context.Context, db *gorm.DB, logger schemas.Logger) error {
migrationName := "add_is_ping_available_column"
Expand Down
2 changes: 2 additions & 0 deletions framework/configstore/rdb.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,7 @@ func (s *RDBConfigStore) UpdateClientConfig(ctx context.Context, config *ClientC
EnableLogging: config.EnableLogging,
DisableContentLogging: config.DisableContentLogging,
DisableDBPingsInHealth: config.DisableDBPingsInHealth,
DumpErrorsInConsoleLogs: config.DumpErrorsInConsoleLogs,
LogRetentionDays: config.LogRetentionDays,
EnforceAuthOnInference: config.EnforceAuthOnInference,
EnforceGovernanceHeader: config.EnforceGovernanceHeader,
Expand Down Expand Up @@ -500,6 +501,7 @@ func (s *RDBConfigStore) GetClientConfig(ctx context.Context) (*ClientConfig, er
EnableLogging: dbConfig.EnableLogging,
DisableContentLogging: dbConfig.DisableContentLogging,
DisableDBPingsInHealth: dbConfig.DisableDBPingsInHealth,
DumpErrorsInConsoleLogs: dbConfig.DumpErrorsInConsoleLogs,
LogRetentionDays: dbConfig.LogRetentionDays,
EnforceAuthOnInference: dbConfig.EnforceAuthOnInference,
EnforceGovernanceHeader: dbConfig.EnforceGovernanceHeader,
Expand Down
1 change: 1 addition & 0 deletions framework/configstore/tables/clientconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ type TableClientConfig struct {
EnableLogging *bool `gorm:"default:true" json:"enable_logging"`
DisableContentLogging bool `gorm:"default:false" json:"disable_content_logging"` // DisableContentLogging controls whether sensitive content (inputs, outputs, embeddings, etc.) is logged
DisableDBPingsInHealth bool `gorm:"default:false" json:"disable_db_pings_in_health"`
DumpErrorsInConsoleLogs bool `gorm:"default:false" json:"dump_errors_in_console_logs"` // Dump full error details to the server console logs
LogRetentionDays int `gorm:"default:365" json:"log_retention_days" validate:"min=1"` // Number of days to retain logs (minimum 1 day)
EnforceAuthOnInference bool `gorm:"default:false" json:"enforce_auth_on_inference"`
EnforceGovernanceHeader bool `gorm:"" json:"enforce_governance_header"`
Expand Down
1 change: 1 addition & 0 deletions helm-charts/bifrost/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -687,6 +687,7 @@ bifrost:
| Parameter | Description | Default |
| --------------------------------------------- | ------------------------------------------- | ------- |
| `bifrost.client.disableDbPingsInHealth` | Disable DB pings in health check | `false` |
| `bifrost.client.dumpErrorsInConsoleLogs` | Dump full error details to server console | `false` |
| `bifrost.client.headerFilterConfig.allowlist` | Headers allowed to forward to LLM providers | `[]` |
| `bifrost.client.headerFilterConfig.denylist` | Headers blocked from forwarding | `[]` |

Expand Down
3 changes: 3 additions & 0 deletions helm-charts/bifrost/templates/_helpers.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,9 @@ false
{{- if hasKey .Values.bifrost.client "disableDbPingsInHealth" }}
{{- $_ := set $client "disable_db_pings_in_health" .Values.bifrost.client.disableDbPingsInHealth }}
{{- end }}
{{- if hasKey .Values.bifrost.client "dumpErrorsInConsoleLogs" }}
{{- $_ := set $client "dump_errors_in_console_logs" .Values.bifrost.client.dumpErrorsInConsoleLogs }}
{{- end }}
{{- if .Values.bifrost.client.headerFilterConfig }}
{{- $headerFilter := dict }}
{{- if .Values.bifrost.client.headerFilterConfig.allowlist }}
Expand Down
5 changes: 5 additions & 0 deletions helm-charts/bifrost/values.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,11 @@
"description": "Disable DB pings in health check",
"default": false
},
"dumpErrorsInConsoleLogs": {
"type": "boolean",
"description": "Dump full error details to the server console logs. Useful for debugging; may be noisy in production.",
"default": false
},
"logRetentionDays": {
"type": "integer",
"minimum": 1,
Expand Down
1 change: 1 addition & 0 deletions helm-charts/bifrost/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,7 @@ bifrost:
enableLogging: true
disableContentLogging: false
disableDbPingsInHealth: false
dumpErrorsInConsoleLogs: false
logRetentionDays: 365
# Deprecated: use enforceAuthOnInference instead.
enforceGovernanceHeader: false
Expand Down
2 changes: 1 addition & 1 deletion tests/cmd/e2eseed/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ require (
github.com/mattn/go-colorable v0.1.14 // indirect
github.com/mattn/go-isatty v0.0.20 // indirect
github.com/mattn/go-sqlite3 v1.14.32 // indirect
github.com/maximhq/bifrost/core v1.5.21 // indirect
github.com/maximhq/bifrost/core v1.5.22 // indirect
github.com/maximhq/bifrost/framework v1.3.16 // indirect
github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c // indirect
github.com/planetscale/vtprotobuf v0.6.1-0.20240319094008-0393e58bdf10 // indirect
Expand Down
2 changes: 1 addition & 1 deletion tests/cmd/seed/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ replace (
)

require (
github.com/maximhq/bifrost/core v1.5.21
github.com/maximhq/bifrost/core v1.5.22
github.com/maximhq/bifrost/framework v1.3.16
gorm.io/driver/postgres v1.6.0
gorm.io/driver/sqlite v1.6.0
Expand Down
2 changes: 1 addition & 1 deletion tests/cmd/seedvks/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ replace (

require (
github.com/google/uuid v1.6.0
github.com/maximhq/bifrost/core v1.5.21
github.com/maximhq/bifrost/core v1.5.22
github.com/maximhq/bifrost/framework v1.3.16
gorm.io/driver/postgres v1.6.0
gorm.io/gorm v1.31.1
Expand Down
3 changes: 3 additions & 0 deletions transports/bifrost-http/handlers/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -443,6 +443,9 @@ func (h *ConfigHandler) updateConfig(ctx *fasthttp.RequestCtx) {
// and ReloadClientConfigFromConfigStore mutates the struct in place so the next request picks up the new value.
updatedConfig.DisableContentLogging = payload.ClientConfig.DisableContentLogging
updatedConfig.DisableDBPingsInHealth = payload.ClientConfig.DisableDBPingsInHealth
// No restart needed - ReloadClientConfigFromConfigStore calls CorsMiddleware.UpdateConfig,
// which atomically swaps in a fresh immutable snapshot carrying the new value.
updatedConfig.DumpErrorsInConsoleLogs = payload.ClientConfig.DumpErrorsInConsoleLogs

updatedConfig.EnforceAuthOnInference = payload.ClientConfig.EnforceAuthOnInference
// Sync deprecated columns to match new field so they stay consistent in the DB
Expand Down
73 changes: 66 additions & 7 deletions transports/bifrost-http/handlers/middlewares.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,62 @@ func clientForwardedIP(ctx *fasthttp.RequestCtx) string {
return ""
}

// corsMiddlewareConfig is an immutable snapshot of the CORS-relevant client config.
// The slices are cloned at construction so a hot reload mutating the source
// ClientConfig in place cannot race with in-flight requests reading these fields.
type corsMiddlewareConfig struct {
dumpErrorsInConsoleLogs bool
allowedOrigins []string
allowedHeaders []string
}

// newCorsMiddlewareConfig builds an immutable snapshot from the live config,
// cloning the slices so the snapshot never aliases the shared ClientConfig.
func newCorsMiddlewareConfig(config *lib.Config) *corsMiddlewareConfig {
if config == nil || config.ClientConfig == nil {
return nil
}
return &corsMiddlewareConfig{
dumpErrorsInConsoleLogs: config.ClientConfig.DumpErrorsInConsoleLogs,
allowedOrigins: slices.Clone(config.ClientConfig.AllowedOrigins),
allowedHeaders: slices.Clone(config.ClientConfig.AllowedHeaders),
}
}

// CorsMiddleware handles CORS headers for localhost and configured allowed origins.
// The snapshot is held in an atomic.Pointer so UpdateConfig can swap it at runtime
// without racing in-flight requests, which read the pointer concurrently. Because the
// snapshot is immutable (slices cloned), readers never observe a torn or half-updated
// config even while a reload swaps in a new one.
type CorsMiddleware struct {
config atomic.Pointer[corsMiddlewareConfig]
}

func NewCorsMiddleware(config *lib.Config) *CorsMiddleware {
c := &CorsMiddleware{}
c.config.Store(newCorsMiddlewareConfig(config))
return c
}

// UpdateConfig atomically swaps in a fresh immutable snapshot of the configuration.
// In-flight requests reading the pointer observe either the old or the new snapshot,
// never a torn value. ReloadClientConfigFromConfigStore must call this whenever the
// client config is refreshed, mirroring how AuthMiddleware is updated.
func (c *CorsMiddleware) UpdateConfig(config *lib.Config) {
c.config.Store(newCorsMiddlewareConfig(config))
}

// CorsMiddleware handles CORS headers for localhost and configured allowed origins
func CorsMiddleware(config *lib.Config) schemas.BifrostHTTPMiddleware {
func (c *CorsMiddleware) Middleware() schemas.BifrostHTTPMiddleware {
return func(next fasthttp.RequestHandler) fasthttp.RequestHandler {
return func(ctx *fasthttp.RequestCtx) {
// Snapshot the config once per request so a concurrent UpdateConfig swap
// cannot apply two different configs within a single response.
cfg := c.config.Load()
Comment thread
coderabbitai[bot] marked this conversation as resolved.
if cfg == nil {
SendError(ctx, fasthttp.StatusInternalServerError, "CORS middleware configuration not loaded")
return
}
shouldLog := slices.IndexFunc(loggingSkipPaths, func(path string) bool {
return strings.HasPrefix(string(ctx.RequestURI()), path)
}) == -1
Expand All @@ -98,19 +150,26 @@ func CorsMiddleware(config *lib.Config) schemas.BifrostHTTPMiddleware {
if traceID, ok := ctx.UserValue(schemas.BifrostContextKeyTraceID).(string); ok && traceID != "" {
logBuilder = logBuilder.Str("trace_id", traceID)
}
if cfg.dumpErrorsInConsoleLogs {
if statusCode >= 400 && !ctx.Response.IsBodyStream() {
if body := ctx.Response.Body(); len(body) > 0 {
logBuilder = logBuilder.Str("http.error", string(body))
}
}
}
logBuilder.Send()
}()
}
origin := string(ctx.Request.Header.Peek("Origin"))
allowed := IsOriginAllowed(origin, config.ClientConfig.AllowedOrigins)
allowed := IsOriginAllowed(origin, cfg.allowedOrigins)
// Credentialed responses are sent when the origin is not matched solely by a
// wildcard AllowedOrigins — i.e. the origin is localhost or explicitly listed.
credentialed := !slices.Contains(config.ClientConfig.AllowedOrigins, "*") ||
credentialed := !slices.Contains(cfg.allowedOrigins, "*") ||
isLocalhostOrigin(origin) ||
slices.Contains(config.ClientConfig.AllowedOrigins, origin)
slices.Contains(cfg.allowedOrigins, origin)

allowedHeaders := []string{"Content-Type", "Authorization", "X-Requested-With", "X-Stainless-Timeout", "X-Api-Key", "X-OpenAI-Agents-SDK", "X-Operation-ID"}
if slices.Contains(config.ClientConfig.AllowedHeaders, "*") {
if slices.Contains(cfg.allowedHeaders, "*") {
if credentialed {
// Per the Fetch spec, Access-Control-Allow-Headers: * is NOT treated as a
// wildcard when Access-Control-Allow-Credentials: true is set — browsers
Expand All @@ -123,9 +182,9 @@ func CorsMiddleware(config *lib.Config) schemas.BifrostHTTPMiddleware {
} else {
allowedHeaders = []string{"*"}
}
} else if len(config.ClientConfig.AllowedHeaders) > 0 {
} else if len(cfg.allowedHeaders) > 0 {
// append allowed headers from config to the default headers
for _, header := range config.ClientConfig.AllowedHeaders {
for _, header := range cfg.allowedHeaders {
if !slices.Contains(allowedHeaders, header) {
allowedHeaders = append(allowedHeaders, header)
}
Expand Down
Loading
Loading