feat: add dual_credential_conflict_behavior to resolve IDP token + VK conflicts on inference requests - #5201
Merged
akshaydeo merged 1 commit intoJul 16, 2026
Conversation
Contributor
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe change adds configurable dual-credential conflict behavior across client configuration, database persistence and migration, transport schema validation, hashing, and MCP request handling. ChangesDual credential conflict behavior
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Comment |
Contributor
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
BearTS
changed the title
dual_credential_conflict_behavior and release Helm chart v2.1.28
BearTS
marked this pull request as ready for review
July 14, 2026 17:40
Contributor
Confidence Score: 4/5This is close, but the MCP auth path should be fixed before merging.
transports/bifrost-http/handlers/mcpserver.go
|
| Filename | Overview |
|---|---|
| framework/configstore/clientconfig.go | Adds the public client config field and includes non-default conflict policies in the client config hash. |
| framework/configstore/migrations.go | Adds an upgrade migration for the new conflict behavior column. |
| framework/configstore/rdb.go | Persists and reloads the new client conflict behavior field. |
| framework/configstore/tables/clientconfig.go | Defines the stored conflict behavior type and column default. |
| transports/bifrost-http/handlers/mcpserver.go | Removes the local MCP conflict rejection and relies on upstream policy handling. |
| transports/config.schema.json | Adds the new client config field to the JSON schema. |
Reviews (8): Last reviewed commit: "feat: add dual credential conflict confi..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
transports/bifrost-http/handlers/mcpserver.go (1)
643-681: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winHonor
dual_credential_conflict_behaviorfor bearer-token + VK requests
Authorization: Bearer <jwt>still hard-fails when a VK header is present. Apply the newerror/prefer_vk/prefer_idppolicy here too, or document that MCP JWT auth is intentionally excluded from it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@transports/bifrost-http/handlers/mcpserver.go` around lines 643 - 681, Update the JWT path in the MCP authentication flow, near extractBearerJWT and getVKFromRequest, to apply the configured dual_credential_conflict_behavior when both a bearer JWT and VK header are present. Preserve error rejection for the error policy, route authentication through the VK for prefer_vk, and retain the JWT/IDP identity for prefer_idp; do not unconditionally return the current conflict error.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@framework/configstore/tables/clientconfig.go`:
- Around line 14-21: The persisted default for DualCredentialConflictBehavior is
currently empty instead of the valid prefer_idp value. Update the associated
GORM/schema default configuration for DualCredentialConflictBehavior to persist
DualCredentialConflictBehaviorPreferIDP ("prefer_idp") when unset, keeping the
declared enum constants unchanged.
In `@helm-charts/bifrost/README.md`:
- Line 13: Correct the dual-credential configuration paths in both
helm-charts/bifrost/README.md:13-13 and docs/changelogs/helm-v2.1.28.mdx:10-10,
replacing bifrost.scim.dualCredentialConflictBehavior with
bifrost.client.dualCredentialConflictBehavior and
scim_config.dual_credential_conflict_behavior with
client.dual_credential_conflict_behavior.
---
Outside diff comments:
In `@transports/bifrost-http/handlers/mcpserver.go`:
- Around line 643-681: Update the JWT path in the MCP authentication flow, near
extractBearerJWT and getVKFromRequest, to apply the configured
dual_credential_conflict_behavior when both a bearer JWT and VK header are
present. Preserve error rejection for the error policy, route authentication
through the VK for prefer_vk, and retain the JWT/IDP identity for prefer_idp; do
not unconditionally return the current conflict error.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 3d6b2b2a-3391-4331-aa92-93357d3170ca
📒 Files selected for processing (11)
docs/changelogs/helm-v2.1.28.mdxdocs/docs.jsonframework/configstore/clientconfig.goframework/configstore/tables/clientconfig.gohelm-charts/bifrost/Chart.yamlhelm-charts/bifrost/README.mdhelm-charts/bifrost/templates/_helpers.tplhelm-charts/bifrost/values.schema.jsonhelm-charts/bifrost/values.yamltransports/bifrost-http/handlers/mcpserver.gotransports/config.schema.json
BearTS
force-pushed
the
07-14-feat_add_dual_credential_conflict_configuration
branch
from
July 15, 2026 05:05
8cb90c1 to
a9cb10b
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
helm-charts/bifrost/README.md (1)
16-16:⚠️ Potential issue | 🟠 MajorUse the
clientconfiguration path.The chart schema and template use
bifrost.client.dualCredentialConflictBehaviorand renderclient.dual_credential_conflict_behavior; the documentedscim/scim_configpath will not configure anything.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helm-charts/bifrost/README.md` at line 16, Update the README configuration entry to use bifrost.client.dualCredentialConflictBehavior and state that it renders to client.dual_credential_conflict_behavior. Preserve the documented behavior values and their meanings while removing the incorrect scim/scim_config configuration path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@framework/configstore/tables/clientconfig.go`:
- Around line 38-41: Update the DualCredentialConflictBehavior GORM tag to use
prefer_idp as the persisted default instead of an empty string. Add migration or
initialization logic for this column to convert existing empty values to
prefer_idp, while preserving the supported enum modes.
---
Duplicate comments:
In `@helm-charts/bifrost/README.md`:
- Line 16: Update the README configuration entry to use
bifrost.client.dualCredentialConflictBehavior and state that it renders to
client.dual_credential_conflict_behavior. Preserve the documented behavior
values and their meanings while removing the incorrect scim/scim_config
configuration path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1c66e5a3-3abb-42e5-b758-7261863d39f8
📒 Files selected for processing (9)
docs/changelogs/helm-v2.1.28.mdxframework/configstore/clientconfig.goframework/configstore/tables/clientconfig.gohelm-charts/bifrost/README.mdhelm-charts/bifrost/templates/_helpers.tplhelm-charts/bifrost/values.schema.jsonhelm-charts/bifrost/values.yamltransports/bifrost-http/handlers/mcpserver.gotransports/config.schema.json
🚧 Files skipped from review as they are similar to previous changes (5)
- helm-charts/bifrost/templates/_helpers.tpl
- helm-charts/bifrost/values.yaml
- helm-charts/bifrost/values.schema.json
- transports/config.schema.json
- transports/bifrost-http/handlers/mcpserver.go
BearTS
force-pushed
the
07-14-feat_add_dual_credential_conflict_configuration
branch
from
July 15, 2026 05:21
a9cb10b to
b6dfc4f
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@framework/configstore/clientconfig.go`:
- Around line 171-175: Align the documented default for
DualCredentialConflictBehavior with runtime behavior: update
helm-charts/bifrost/values.schema.json lines 466-470 and
helm-charts/bifrost/README.md line 16 from error to prefer_idp;
framework/configstore/clientconfig.go lines 171-175 requires no direct change
because it already preserves the omitted-value default.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1236dafa-ec51-4413-b9f4-09109785043a
📒 Files selected for processing (10)
docs/changelogs/helm-v2.1.28.mdxframework/configstore/clientconfig.goframework/configstore/rdb.goframework/configstore/tables/clientconfig.gohelm-charts/bifrost/README.mdhelm-charts/bifrost/templates/_helpers.tplhelm-charts/bifrost/values.schema.jsonhelm-charts/bifrost/values.yamltransports/bifrost-http/handlers/mcpserver.gotransports/config.schema.json
🚧 Files skipped from review as they are similar to previous changes (5)
- transports/config.schema.json
- transports/bifrost-http/handlers/mcpserver.go
- docs/changelogs/helm-v2.1.28.mdx
- helm-charts/bifrost/values.yaml
- framework/configstore/tables/clientconfig.go
BearTS
changed the title
dual_credential_conflict_behavior and release Helm chart v2.1.28dual_credential_conflict_behavior to resolve IDP token + VK conflicts on inference requests
BearTS
force-pushed
the
07-14-feat_add_dual_credential_conflict_configuration
branch
2 times, most recently
from
July 15, 2026 05:45
72c22e6 to
d6b95ca
Compare
BearTS
force-pushed
the
07-14-feat_add_dual_credential_conflict_configuration
branch
from
July 15, 2026 05:58
d6b95ca to
7d7a212
Compare
BearTS
force-pushed
the
07-14-feat_add_dual_credential_conflict_configuration
branch
from
July 15, 2026 06:06
7d7a212 to
e621e47
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@framework/configstore/clientconfig.go`:
- Around line 171-176: Update the config hashing logic around
DualCredentialConflictBehavior so the explicit prefer_idp value is excluded from
the hash, matching omitted values and avoiding false reconciliation changes.
Alternatively, ensure the migration that backfills prefer_idp recomputes
config_hash for affected rows; keep hashing non-default conflict behaviors
unchanged.
In `@framework/configstore/migrations.go`:
- Around line 5399-5428: Make migrationAddDualCredentialConflictBehaviorColumn
non-rollbackable by replacing its Rollback implementation with an explicit error
return instead of calling dropColumnIfExists. Add a migration test in
framework/configstore/migrations_test.go covering this rollback behavior and
asserting the expected error, while preserving the existing migration behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 2fd2983e-43a8-4035-bed5-9b3c2dc210d7
📒 Files selected for processing (7)
framework/configstore/clientconfig.goframework/configstore/migrations.goframework/configstore/migrations_test.goframework/configstore/rdb.goframework/configstore/tables/clientconfig.gotransports/bifrost-http/handlers/mcpserver.gotransports/config.schema.json
🚧 Files skipped from review as they are similar to previous changes (3)
- transports/bifrost-http/handlers/mcpserver.go
- framework/configstore/rdb.go
- framework/configstore/tables/clientconfig.go
18 tasks
BearTS
force-pushed
the
07-14-feat_add_dual_credential_conflict_configuration
branch
from
July 15, 2026 06:20
e621e47 to
7902710
Compare
BearTS
force-pushed
the
graphite-base/5201
branch
from
July 15, 2026 06:20
c0909f9 to
7b67f20
Compare
BearTS
force-pushed
the
07-14-feat_add_dual_credential_conflict_configuration
branch
from
July 15, 2026 06:43
7902710 to
7b7dcf6
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@transports/config.schema.json`:
- Around line 127-131: Update the dual_credential_conflict_behavior schema
property to declare "prefer_idp" as its default value, while preserving the
existing string type, enum values, and description.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 03652962-f4b5-4ee7-8b8a-92457890ec5b
📒 Files selected for processing (7)
framework/configstore/clientconfig.goframework/configstore/migrations.goframework/configstore/migrations_test.goframework/configstore/rdb.goframework/configstore/tables/clientconfig.gotransports/bifrost-http/handlers/mcpserver.gotransports/config.schema.json
🚧 Files skipped from review as they are similar to previous changes (6)
- framework/configstore/rdb.go
- framework/configstore/migrations_test.go
- framework/configstore/clientconfig.go
- framework/configstore/migrations.go
- transports/bifrost-http/handlers/mcpserver.go
- framework/configstore/tables/clientconfig.go
17 tasks
Contributor
Merge activity
|
akshaydeo
deleted the
07-14-feat_add_dual_credential_conflict_configuration
branch
July 16, 2026 04:21
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Introduces a
dual_credential_conflict_behaviorconfiguration option that controls what Bifrost does when an inference request presents both an IDP access token (Authorization: Bearer <jwt>) and a virtual key (x-bf-vkheader) simultaneously. Previously, the MCP server handler hard-rejected such requests with an error. This change moves that decision upstream into a configurable policy, giving operators explicit control over the resolution strategy.Changes
DualCredentialConflictBehaviortyped string (error|prefer_vk|prefer_idp) totables.TableClientConfigwith a DB column default ofprefer_idp.DualCredentialConflictBehaviorfield toClientConfigand wired it throughUpdateClientConfigandGetClientConfigin the RDB config store.GenerateClientConfigHashto include the new field only when non-empty, avoiding hash churn on upgrade for existing configs that carry an empty value (treated as theprefer_idpdefault).MCPServerHandler.getMCPServerForRequest; the conflict is now resolved upstream in the inference middleware before identity is stamped.bifrost.client.dualCredentialConflictBehaviorin the Helm chart (values.yaml,values.schema.json,_helpers.tpl) and documented it in the Helm README and changelog.dual_credential_conflict_behaviortotransports/config.schema.json.Type of change
Affected areas
How to test
go test ./framework/configstore/... ./transports/bifrost-http/handlers/...Scenario 1 —
prefer_idp(default): Send a request with bothAuthorization: Bearer <jwt>andx-bf-vk: <key>. Expect the IDP token to be used for identity and the VK to be silently ignored.Scenario 2 —
prefer_vk: Setdual_credential_conflict_behavior: "prefer_vk"in config. Send the same dual-credential request. Expect the IDP token to be dropped and the virtual key to be used for authentication.Scenario 3 —
error: Setdual_credential_conflict_behavior: "error". Send the same dual-credential request. Expect a400response rejecting the request.Helm: Set
bifrost.client.dualCredentialConflictBehavior: "prefer_vk"invalues.yamland confirm the rendered config containsdual_credential_conflict_behavior: prefer_vk.Breaking changes
The hard-coded rejection of dual-credential requests in the MCP server handler has been removed. Deployments that relied on that error to block such requests should explicitly set
dual_credential_conflict_behavior: "error"to preserve the previous behavior. The new default (prefer_idp) is behaviorally equivalent to the prior IDP-wins path for non-MCP inference flows, but MCP flows that previously errored will now succeed underprefer_idp.Security considerations
This change directly affects authentication resolution. Operators should explicitly configure
dual_credential_conflict_behavior: "error"if their security policy requires that requests cannot present mixed credential types. The defaultprefer_idpmeans a virtual key present alongside a valid IDP token is silently ignored rather than rejected, which may be unexpected in strict zero-trust environments.Checklist
docs/contributing/README.mdand followed the guidelines