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
22 changes: 8 additions & 14 deletions transports/bifrost-http/handlers/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -794,9 +794,11 @@ func headerFilterConfigEqual(a, b *configstoreTables.GlobalHeaderFilterConfig) b
return slices.Equal(a.Allowlist, b.Allowlist) && slices.Equal(a.Denylist, b.Denylist)
}

// validateHeaderFilterConfig validates that no security headers are in the allowlist or denylist
// validateHeaderFilterConfig validates that no exact security header names are in the allowlist or denylist
// and that wildcard patterns use valid syntax (only trailing * is supported).
// Returns an error if any security headers are found or patterns are invalid.
// Wildcard patterns that would match security headers are allowed because security headers
// are unconditionally stripped at runtime regardless of configuration.
// Returns an error if any exact security headers are found or patterns are invalid.
func validateHeaderFilterConfig(config *configstoreTables.GlobalHeaderFilterConfig) error {
if config == nil {
return nil
Expand Down Expand Up @@ -830,31 +832,23 @@ func validateHeaderFilterConfig(config *configstoreTables.GlobalHeaderFilterConf

var foundSecurityHeaders []string

// Check allowlist for security headers (including wildcard patterns)
// Check allowlist for exact security header names.
// Wildcard patterns are allowed — security headers are always stripped at runtime
// unconditionally in ctx.go, regardless of allowlist/denylist configuration.
for _, header := range config.Allowlist {
headerLower := strings.ToLower(strings.TrimSpace(header))
if strings.Contains(headerLower, "*") {
for _, secHeader := range securityHeaders {
if lib.HeaderMatchesPattern(headerLower, secHeader) && !slices.Contains(foundSecurityHeaders, secHeader) {
foundSecurityHeaders = append(foundSecurityHeaders, secHeader)
}
}
continue
}
if slices.Contains(securityHeaders, headerLower) {
foundSecurityHeaders = append(foundSecurityHeaders, headerLower)
}
}
Comment on lines +835 to 846

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 authorization is NOT in the runtime security denylist

The PR comment says "security headers are always stripped at runtime unconditionally in ctx.go", but that's only true for most of the security headers — authorization is not in the securityDenylist map in ctx.go:

// ctx.go ~line 146
securityDenylist := map[string]bool{
    "proxy-authorization": true,
    "cookie":              true,
    "host":                true,
    "content-length":      true,
    "connection":          true,
    "transfer-encoding":   true,
    "x-api-key":           true,
    "x-goog-api-key":      true,
    "x-bf-api-key":        true,
    "x-bf-api-key-id":     true,
    "x-bf-vk":             true,
    // ← "authorization" is absent
}

The direct-header-forwarding path in ctx.go (lines ~358–378) only guards against securityDenylist headers. Because authorization is absent from that map, a request-level client that sends Authorization: Bearer sk-ant-… will have that header forwarded directly to the upstream AI provider whenever:

  1. The allowlist contains a wildcard such as authorization* or * (now accepted by this PR), and
  2. The authorization header doesn't happen to carry a governance virtual-key prefix (which causes return true early at line 231–237, before the forwarding section is reached).

In practice this means: if an admin configures authorization* in the header-filter allowlist (newly valid under this change), any API caller can inject an arbitrary Authorization header and Bifrost will forward it to the upstream provider — overriding Bifrost's own credential management.

The original validation that rejected wildcard patterns matching security headers was the correct guard. If the intent is to allow authorization* for non-security custom headers, authorization should first be added to the securityDenylist in ctx.go so the runtime actually enforces the strip unconditionally.


// Check denylist for security headers (including wildcard patterns)
// Check denylist for exact security header names.
for _, header := range config.Denylist {
headerLower := strings.ToLower(strings.TrimSpace(header))
if strings.Contains(headerLower, "*") {
for _, secHeader := range securityHeaders {
if lib.HeaderMatchesPattern(headerLower, secHeader) && !slices.Contains(foundSecurityHeaders, secHeader) {
foundSecurityHeaders = append(foundSecurityHeaders, secHeader)
}
}
continue
}
if slices.Contains(securityHeaders, headerLower) && !slices.Contains(foundSecurityHeaders, headerLower) {
Expand Down
12 changes: 3 additions & 9 deletions transports/bifrost-http/handlers/config_headerfilter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,28 +85,22 @@ func TestValidateHeaderFilterConfig(t *testing.T) {
errSubstr: "not allowed to be configured",
},
{
name: "wildcard matching security header rejected",
name: "wildcard matching security header allowed (runtime strips security headers)",
config: &configstoreTables.GlobalHeaderFilterConfig{
Allowlist: []string{"authorization*"},
},
wantErr: true,
errSubstr: "not allowed to be configured",
},
{
name: "wildcard prefix matching security headers rejected",
name: "wildcard prefix matching security headers allowed (runtime strips security headers)",
config: &configstoreTables.GlobalHeaderFilterConfig{
Allowlist: []string{"x-api-*"},
},
wantErr: true,
errSubstr: "not allowed to be configured",
},
{
name: "bare wildcard in allowlist rejected (matches all security headers)",
name: "bare wildcard in allowlist allowed (runtime strips security headers)",
config: &configstoreTables.GlobalHeaderFilterConfig{
Allowlist: []string{"*"},
},
wantErr: true,
errSubstr: "not allowed to be configured",
},
// Invalid wildcard syntax
{
Expand Down
2 changes: 1 addition & 1 deletion ui/app/workspace/config/views/clientSettingsView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ export default function ClientSettingsView() {
await updateCoreConfig({ ...bifrostConfig!, client_config: cleanedConfig }).unwrap();
coreConfigSaved = true;
} catch (error) {
toast.error(`Failed to save core config: ${getErrorMessage(error)}`);
toast.error(`Failed to save client config: ${getErrorMessage(error)}`);
}
}

Expand Down
Loading