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
17 changes: 14 additions & 3 deletions core/mcp/credstore/per_user_headers.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,14 @@ func (r *perUserHeadersResolver) buildAuthRequiredError(ctx *schemas.BifrostCont
}

// missingRequiredHeaderKeys returns the names of any required header key
// that's absent or whose stored value is empty in storedHeaders. Comparison
// is case-insensitive at the wire level but the schema is the source of
// truth — we look up by the exact key the admin declared.
// that's absent or whose stored value is empty in storedHeaders.
//
// Both inputs are assumed to be in canonical form (lowercase + trimmed) —
// see the invariant doc on mcputils.CanonicalizeHeaderKey. All write
// boundaries (HTTP create/update, flow submit, config.json load) run
// the inputs through that helper, so exact map lookup here is correct.
// Do NOT add defensive case-folding inside this function: it would mask
// a missed write-side canonicalization rather than catching it.
func missingRequiredHeaderKeys(required []string, storedHeaders map[string]string) []string {
if len(storedHeaders) == 0 {
return append([]string(nil), required...)
Expand All @@ -128,6 +133,12 @@ func missingRequiredHeaderKeys(required []string, storedHeaders map[string]strin
// user-submitted credential values for the required keys. Keys not declared
// by the current schema are dropped on purpose so a stale row that still
// stores a deprecated key cannot leak it onto the wire.
//
// Required keys and storedHeaders keys are both canonical (lowercase +
// trimmed) by the write-side invariant — see missingRequiredHeaderKeys
// above and mcputils.CanonicalizeHeaderKey. http.Header.Set runs its own
// MIME canonicalization on the way out (so "authorization" becomes
// "Authorization" on the wire), which is what upstream servers expect.
func buildPerUserHeaderValues(required []string, storedHeaders map[string]string) http.Header {
out := http.Header{}
for _, key := range required {
Expand Down
72 changes: 71 additions & 1 deletion core/mcp/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,16 @@ func FlattenHeaders(h http.Header) map[string]string {
// BuildMCPCallbackBaseURL extracts the base URL set on the BifrostContext by
// the HTTP middleware (e.g. "https://host"). Per-user OAuth and per-user
// headers resolvers append their respective paths on top.
//
// Trailing slashes are stripped defensively. The sole writer today
// (lib/ctx.go BuildBaseURL) already normalizes, but OAuth providers match
// redirect URIs exactly — a `https://host//api/oauth/callback` produced by
// a future writer that forgets to trim would silently break every per-user
// OAuth flow. Guarding once on the read side keeps that invariant local
// to this function rather than spread across every potential writer.
func BuildMCPCallbackBaseURL(ctx *schemas.BifrostContext) string {
if base, ok := ctx.Value(schemas.BifrostContextKeyMCPCallbackBaseURL).(string); ok && base != "" {
return base
return strings.TrimRight(base, "/")
}
return ""
}
Expand Down Expand Up @@ -91,6 +98,69 @@ func matchesPerUserHeaderKey(name string, perUserKeys []string) bool {
return false
}

// Canonical-form invariant for per-user-headers data
// =====================================================
// HTTP header names are case-insensitive on the wire (RFC 7230 §3.2),
// so anywhere the per-user-headers feature compares a schema key against
// a stored or submitted header name we'd need EqualFold lookups. Doing
// that defensively at every read site is fragile — a single missed call
// site re-introduces the bug (stored `authorization` looking missing
// against schema `Authorization`, etc.).
//
// Instead we enforce a write-time invariant: every external boundary
// that accepts a header key (or a credential header map) lowercases and
// trims via the helpers below before persisting. Downstream code can
// then assume canonical form and use plain map lookups.
//
// Write boundaries that MUST call these:
// - createMCPClient / updateMCPClient / resolvePerUserHeaderKeys
// (handlers/mcp.go) for MCPClientConfig.PerUserHeaderKeys
// - flowSubmit (handlers/mcp_per_user_headers.go) for the
// user-submitted credential.Headers map
// - loadMCPClientConfigFromFile (lib/config.go) for the config.json
// load path
//
// New write paths added in the future must canonicalize too — there is
// no defensive case-folding on the read side anymore.

// CanonicalizeHeaderKey returns the canonical lowercase + trimmed form
// of a single header key. Empty input returns empty.
func CanonicalizeHeaderKey(key string) string {
return strings.ToLower(strings.TrimSpace(key))
}

// CanonicalizeHeaderKeys returns a new slice with every entry passed
// through CanonicalizeHeaderKey. Nil in → nil out so a caller that
// uses "nil means preserve existing" semantics (e.g.
// resolvePerUserHeaderKeys, UpdateMCPClientConfig) keeps that signal.
// The input slice is not mutated.
func CanonicalizeHeaderKeys(keys []string) []string {
if keys == nil {
return nil
}
out := make([]string, len(keys))
for i, k := range keys {
out[i] = CanonicalizeHeaderKey(k)
}
return out
}

// CanonicalizeHeaderMap returns a new map whose keys are passed through
// CanonicalizeHeaderKey. On collision (e.g. "Authorization" and
// "authorization" both present in the input), the last value wins —
// callers that need duplicate detection should run it on the raw input
// before calling this. Nil in → nil out.
func CanonicalizeHeaderMap(m map[string]string) map[string]string {
if m == nil {
return nil
}
out := make(map[string]string, len(m))
for k, v := range m {
out[CanonicalizeHeaderKey(k)] = v
}
return out
}

// ExtractFilteredExtras returns just the per-request "extra" headers carried
// in the BifrostContext (BifrostContextKeyMCPExtraHeaders), scoped by the
// client's AllowedExtraHeaders. Static config headers are NOT included here —
Expand Down
Loading
Loading