feat: auto-detect ChatGPT subscription tokens and route to chatgpt.com#4548
Open
Purvi09 wants to merge 12 commits into
Open
feat: auto-detect ChatGPT subscription tokens and route to chatgpt.com#4548Purvi09 wants to merge 12 commits into
Purvi09 wants to merge 12 commits into
Conversation
## Summary Fixes a bug where OTEL plugin headers were being overwritten with redacted placeholder values when saving a plugin configuration. After the multi-profile change, header values stored as plain strings inside the `profiles` array were not being restored from the database before saving, causing real credentials to be replaced with masked values like `****`. ## Changes - Extracted `restoreRedactedValue` as a standalone recursive helper, replacing the inline logic in `restoreRedactedFromExisting`. This allows the restoration logic to descend into both nested maps and slices. - Added slice traversal support (index-aligned) so that elements within arrays like the OTEL `profiles` array are individually checked and restored. - Added plain-string redaction detection so that header values stored as raw strings (rather than `EnvVar` objects) are also restored from the existing DB config when they carry a redaction artifact. Empty strings are intentionally left as-is to allow clearing a value. - Added `TestRestoreRedacted_OTELProfilesHeaders` to cover both failure modes: slice traversal and plain-string secret restoration. Also asserts that genuinely new (non-redacted) values pass through unchanged. ## Type of change - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [x] Transports (HTTP) - [ ] Providers/Integrations - [x] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go test ./transports/bifrost-http/handlers/... ``` Verify that saving an OTEL plugin configuration with multiple profiles, after a GET that returns redacted header values, does not overwrite the stored credentials in the database. Confirm that providing a genuinely new header value still persists correctly. ## Screenshots/Recordings N/A ## Breaking changes - [ ] Yes - [x] No ## Related issues N/A ## Security considerations This fix ensures that redacted credential placeholders returned to the client are never written back over real secrets stored in the database. The restoration logic only replaces values that are confirmed redaction artifacts; empty strings and non-redacted values are always passed through as-is, preserving the ability to clear a credential intentionally. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [ ] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
Contributor
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds ChatGPT subscription JWT auto-detection to Bifrost's OpenAI ChangesChatGPT Passthrough Routing
Sequence Diagram(s)sequenceDiagram
participant Client as Codex CLI
participant PreCallback as HTTP PreCallback
participant ParseChatGPTJWT
participant OpenAIProvider
participant ChatGPTBackend as chatgpt.com/backend-api/codex/responses
Client->>PreCallback: POST /openai/responses\nAuthorization: Bearer <JWT>
PreCallback->>ParseChatGPTJWT: ParseChatGPTJWT(token)
ParseChatGPTJWT-->>PreCallback: accountID, ok=true
PreCallback->>PreCallback: set ChatGPTPassthrough=true\nSkipKeySelection=true\nExtraHeaders[Authorization]=Bearer token
PreCallback->>OpenAIProvider: Responses(ctx, request)
OpenAIProvider->>OpenAIProvider: IsChatGPTPassthrough(ctx)==true\nskip fallback, Store=false\nURL=ChatGPTCodexURL
OpenAIProvider->>ChatGPTBackend: POST chatgpt.com/backend-api/codex/responses\nAuthorization: Bearer <original JWT>
ChatGPTBackend-->>OpenAIProvider: response
OpenAIProvider-->>Client: response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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" Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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
🤖 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/bifrost-http/integrations/openai.go`:
- Around line 765-767: The SetValue call on BifrostContextKeyExtraHeaders is
replacing the entire map with only the Authorization header, which drops any
previously collected forwarded headers for the request. Instead of replacing the
map, retrieve the existing BifrostContextKeyExtraHeaders map from bifrostCtx
first (handling the case where it may not exist yet), add the Authorization
header to that map, and then set the updated map back. This ensures the ChatGPT
Authorization passthrough is added as a special case without breaking other
forwarded headers that were collected according to client.header_filter_config.
- Around line 759-760: The Bearer scheme matching in the authHeader validation
uses case-sensitive string comparison with strings.HasPrefix checking for
"Bearer " (capital B), but the HTTP Authorization header scheme is
case-insensitive per RFC 7235. Modify the code to perform case-insensitive
comparison by converting authHeader to lowercase before checking with
strings.HasPrefix, and similarly convert the scheme when extracting the token
using strings.TrimPrefix to handle variations like "bearer", "BEARER", or
"Bearer ".
🪄 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: 4925a162-1fdc-4493-99ab-f3433109b899
📒 Files selected for processing (5)
core/providers/openai/chatgpt_passthrough.gocore/providers/openai/chatgpt_passthrough_test.gocore/providers/openai/openai.gocore/schemas/bifrost.gotransports/bifrost-http/integrations/openai.go
Contributor
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+19
to
+21
| // ParseChatGPTJWT parses a raw bearer token, checks for the ChatGPT subscription | ||
| // JWT claim, and returns the chatgpt_account_id. No signature verification is | ||
| // Returns ("", false) for any non-ChatGPT or malformed token. |
Contributor
There was a problem hiding this comment.
ParseChatGPTJWT has a truncated sentence on the second line. "No signature verification is" is missing its predicate, making the doc incomplete and potentially confusing.
Suggested change
| // ParseChatGPTJWT parses a raw bearer token, checks for the ChatGPT subscription | |
| // JWT claim, and returns the chatgpt_account_id. No signature verification is | |
| // Returns ("", false) for any non-ChatGPT or malformed token. | |
| // ParseChatGPTJWT parses a raw bearer token, checks for the ChatGPT subscription | |
| // JWT claim, and returns the chatgpt_account_id. No signature verification is | |
| // performed — the token is used only as a routing hint. | |
| // Returns ("", false) for any non-ChatGPT or malformed token. |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
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/bifrost-http/integrations/openai.go`:
- Around line 764-770: The code is setting a new Authorization header but does
not remove existing Authorization entries that may use different casing (e.g.,
"authorization", "AUTHORIZATION"). Since HTTP header names are case-insensitive,
this can cause both the old and new credentials to be sent. When iterating
through the existing headers in the for loop before copying them to the headers
map, add a case-insensitive check to skip any Authorization header variants so
that only the new Bearer token authorization is present. Use a case-insensitive
comparison approach when filtering out the existing authorization headers during
the copy operation.
🪄 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: b2943818-ffe4-416a-aef3-50f68052c5ac
📒 Files selected for processing (2)
core/providers/openai/openai.gotransports/bifrost-http/integrations/openai.go
Comment on lines
+764
to
+770
| existing, _ := bifrostCtx.Value(schemas.BifrostContextKeyExtraHeaders).(map[string][]string) | ||
| headers := make(map[string][]string, len(existing)+1) | ||
| for k, v := range existing { | ||
| headers[k] = v | ||
| } | ||
| headers["Authorization"] = []string{"Bearer " + token} | ||
| bifrostCtx.SetValue(schemas.BifrostContextKeyExtraHeaders, headers) |
Contributor
There was a problem hiding this comment.
Drop existing Authorization entries case-insensitively before injecting the passthrough token.
headers["Authorization"] does not replace an existing authorization/AUTHORIZATION entry. Since auth header names are case-insensitive, downstream header merging can send or choose the wrong credential; remove all existing Authorization variants while copying.
🛡️ Proposed fix
existing, _ := bifrostCtx.Value(schemas.BifrostContextKeyExtraHeaders).(map[string][]string)
headers := make(map[string][]string, len(existing)+1)
for k, v := range existing {
- headers[k] = v
+ if strings.EqualFold(k, "Authorization") {
+ continue
+ }
+ headers[k] = append([]string(nil), v...)
}
headers["Authorization"] = []string{"Bearer " + token}
bifrostCtx.SetValue(schemas.BifrostContextKeyExtraHeaders, headers)As per coding guidelines, client.header_filter_config controls forwarded headers and should not accidentally allow sensitive auth headers beyond the passthrough pre-hook.
🤖 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/integrations/openai.go` around lines 764 - 770, The
code is setting a new Authorization header but does not remove existing
Authorization entries that may use different casing (e.g., "authorization",
"AUTHORIZATION"). Since HTTP header names are case-insensitive, this can cause
both the old and new credentials to be sent. When iterating through the existing
headers in the for loop before copying them to the headers map, add a
case-insensitive check to skip any Authorization header variants so that only
the new Bearer token authorization is present. Use a case-insensitive comparison
approach when filtering out the existing authorization headers during the copy
operation.
Source: Coding guidelines
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
6 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
When a user logs into Codex CLI using their ChatGPT subscription (
codex loginwith ChatGPT OAuth), the resulting bearer token is a JWT containing achatgpt_account_idclaim. Sending this token toapi.openai.com/v1/responsesreturns 401 because it is a subscription token, not an API key.This PR auto-detects that token in Bifrost's
/v1/responsespre-hook and transparently reroutes the request tochatgpt.com/backend-api/codex/responses— the endpoint that actually accepts it. No config change or URL change is required from the user.Changes
core/providers/openai/chatgpt_passthrough.go— new file withParseChatGPTJWT(decodes the JWT payload and checks for thechatgpt_account_idclaim underhttps://api.openai.com/auth) andIsChatGPTPassthrough(reads the detection flag from context)core/schemas/bifrost.go— addsBifrostContextKeyChatGPTPassthroughcontext key (bool) set by the pre-hook and read by the providertransports/bifrost-http/integrations/openai.go— pre-hook forPOST /v1/responsesthat callsParseChatGPTJWTon the incoming bearer token; if detected, sets the URL override tochatgpt.com/backend-api/codex/responses, skips Bifrost key selection, and forwards the original bearer tokencore/providers/openai/openai.go—Responses()andResponsesStream()forcestore: falsewhenIsChatGPTPassthroughis true (required by the chatgpt.com backend)core/providers/openai/chatgpt_passthrough_test.go— unit tests forParseChatGPTJWTcovering valid JWT, missing claim, malformed tokens, non-JWT API keys, and edge casesType of change
Affected areas
How to test
Unit tests:
Breaking changes
Related issues
Closes #4459
Security considerations
BifrostContextKeySkipKeySelectionis set so Bifrost does not attempt to inject a stored API key over the user's token.Checklist
docs/contributing/README.mdand followed the guidelines