Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughAWS SDK for Go v2 dependencies across multiple modules are updated to newer patch versions (v1.41.0 → v1.41.3, v1.32.6 → v1.32.11, etc.), alongside smithy-go updates. One code change modifies Bearer token extraction in the Bifrost HTTP handler to preserve the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
|
🧪 Test Suite AvailableThis PR can be tested by a repository admin. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
transports/bifrost-http/handlers/wsresponses.go (1)
488-488: Normalizesk-bf-virtual key handling across all auth headers.Line 488 now preserves the prefix for
Authorization, but Line 502 and Line 515 still strip it forx-api-key/x-goog-api-key. This can produce differentBifrostContextKeyVirtualKeyvalues for the same credential depending on header source.Suggested consistency patch
if auth.apiKey != "" { if strings.HasPrefix(auth.apiKey, "sk-bf-") { - ctx.SetValue(schemas.BifrostContextKeyVirtualKey, strings.TrimPrefix(auth.apiKey, "sk-bf-")) + ctx.SetValue(schemas.BifrostContextKeyVirtualKey, auth.apiKey) } else if h.handlerStore.ShouldAllowDirectKeys() { key := schemas.Key{ ID: "header-provided", Value: *schemas.NewEnvVar(auth.apiKey), Models: []string{}, Weight: 1.0, } ctx.SetValue(schemas.BifrostContextKeyDirectKey, key) } } if auth.googAPIKey != "" { if strings.HasPrefix(auth.googAPIKey, "sk-bf-") { - ctx.SetValue(schemas.BifrostContextKeyVirtualKey, strings.TrimPrefix(auth.googAPIKey, "sk-bf-")) + ctx.SetValue(schemas.BifrostContextKeyVirtualKey, auth.googAPIKey) } else if h.handlerStore.ShouldAllowDirectKeys() { key := schemas.Key{ ID: "header-provided", Value: *schemas.NewEnvVar(auth.googAPIKey), Models: []string{}, Weight: 1.0, } ctx.SetValue(schemas.BifrostContextKeyDirectKey, key) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@transports/bifrost-http/handlers/wsresponses.go` at line 488, Auth header handling is inconsistent: preserve the 'sk-bf-' prefix for virtual keys across all header types (Authorization, x-api-key, x-goog-api-key) instead of stripping it in some branches; find the code paths that parse these headers (the spots that ultimately call ctx.SetValue(schemas.BifrostContextKeyVirtualKey, token)) and change the x-api-key and x-goog-api-key parsing so they do not remove the 'sk-bf-' prefix (i.e., set ctx.SetValue(schemas.BifrostContextKeyVirtualKey, token) with the original token including the prefix, matching the Authorization branch).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@transports/bifrost-http/handlers/wsresponses.go`:
- Line 488: Auth header handling is inconsistent: preserve the 'sk-bf-' prefix
for virtual keys across all header types (Authorization, x-api-key,
x-goog-api-key) instead of stripping it in some branches; find the code paths
that parse these headers (the spots that ultimately call
ctx.SetValue(schemas.BifrostContextKeyVirtualKey, token)) and change the
x-api-key and x-goog-api-key parsing so they do not remove the 'sk-bf-' prefix
(i.e., set ctx.SetValue(schemas.BifrostContextKeyVirtualKey, token) with the
original token including the prefix, matching the Authorization branch).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b1bd15bf-f337-4cac-bbf7-ad36be321a55
⛔ Files ignored due to path filters (11)
core/go.sumis excluded by!**/*.sumframework/go.sumis excluded by!**/*.sumplugins/governance/go.sumis excluded by!**/*.sumplugins/jsonparser/go.sumis excluded by!**/*.sumplugins/litellmcompat/go.sumis excluded by!**/*.sumplugins/logging/go.sumis excluded by!**/*.sumplugins/mocker/go.sumis excluded by!**/*.sumplugins/otel/go.sumis excluded by!**/*.sumplugins/semanticcache/go.sumis excluded by!**/*.sumplugins/telemetry/go.sumis excluded by!**/*.sumtransports/go.sumis excluded by!**/*.sum
📒 Files selected for processing (12)
core/go.modframework/go.modplugins/governance/go.modplugins/jsonparser/go.modplugins/litellmcompat/go.modplugins/logging/go.modplugins/mocker/go.modplugins/otel/go.modplugins/semanticcache/go.modplugins/telemetry/go.modtransports/bifrost-http/handlers/wsresponses.gotransports/go.mod
|
@akshaydeo I am using latest version of codex, requires_openai_auth = true
openai_base_url = "BIFROST_URL/v1"However, I got following websocket error in the bifrost. Doesn't this PR fixes it? Is the commit not released yet? Thanks {"level":"warn","time":"2026-03-31T13:58:49Z","message":"websocket read error: websocket: close 1006 (abnormal closure): unexpected EOF"}Note that i have also tried |
|
this is actually released already - and should ideally work. Ill test once and respond here |
Thanks. I can help to test the feature if you need it. |
3 participants
Summary
Updates AWS SDK for Go v2 dependencies to their latest versions and fixes virtual key handling in WebSocket responses by removing incorrect prefix trimming.
Changes
Type of change
Affected areas
How to test
Validate the dependency updates and virtual key handling:
The virtual key fix ensures that Bearer tokens starting with "sk-bf-" are stored with their full value rather than having the prefix incorrectly removed.
Screenshots/Recordings
N/A - Dependency updates and backend logic fix
Breaking changes
Related issues
N/A
Security considerations
The virtual key handling fix ensures proper token validation and storage, maintaining security boundaries for API key authentication.
Checklist
docs/contributing/README.mdand followed the guidelines