fix(codex): resolve client_id and account_id handling in token refresh and file listing#2153
Conversation
…h and file listing - Add JWTClaims.GetClientID() to extract client_id from id_token.aud[0] - RefreshTokens and RefreshTokensWithRetry now accept a clientID param; falls back to the hardcoded constant when empty, fixing token refresh for accounts whose client_id differs from the default app - Codex executor Refresh() now resolves clientID from Metadata["client_id"] first, then falls back to parsing id_token.aud[0] before calling refresh - extractCodexIDTokenClaims checks Metadata["account_id"] and Attributes["plan_type"] first before parsing id_token, so the GET /auth-files response stays accurate after a refresh whose new id_token no longer carries chatgpt_account_id - Add codex_executor_account_id_test.go: utls+Chrome-fingerprint test for the accounts/check API with pickBestAccountID helper (team > plus > free) Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses critical authentication and data retrieval issues within the Codex system. It refines how Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
本次代码审查主要关注 client_id 和 account_id 在令牌刷新和文件列表中的处理逻辑。整体改动方向正确，解决了硬编码 client_id 和刷新后 account_id 丢失的问题。
我提出了两点建议：
- 高危：修复
auth_files.go中的一个逻辑缺陷。当前实现在特定条件下会跳过对id_token的解析，导致无法获取订阅日期信息。 - 严重：移除新测试文件
codex_executor_account_id_test.go中硬编码的访问令牌和代理URL。将凭证等敏感信息提交到代码库会带来安全风险。
除了以上两点，其余代码实现良好，符合PR的目标。
| accessToken := "eyJhbGciOiJSUzI1NiIsImtpZCI6IjE5MzQ0ZTY1LWJiYzktNDRkMS1hOWQwLWY5NTdiMDc5YmQwZSIsInR5cCI6IkpXVCJ9.eyJhdWQiOlsiaHR0cHM6Ly9hcGkub3BlbmFpLmNvbS92MSJdLCJjbGllbnRfaWQiOiJhcHBfWDh6WTZ2VzJwUTl0UjNkRTduSzFqTDVnSCIsImV4cCI6MTc3NDQ1NTUyNSwiaHR0cHM6Ly9hcGkub3BlbmFpLmNvbS9hdXRoIjp7ImNoYXRncHRfYWNjb3VudF9pZCI6Ijg0YzNjZGM2LWFiZWQtNDlhNy1iY2RlLWZmMjQwMjE3NmZkYiIsImNoYXRncHRfYWNjb3VudF91c2VyX2lkIjoidXNlci00MXlCY3M0STZscEJBVEh5R2lTNG9wTUNfXzg0YzNjZGM2LWFiZWQtNDlhNy1iY2RlLWZmMjQwMjE3NmZkYiIsImNoYXRncHRfY29tcHV0ZV9yZXNpZGVuY3kiOiJub19jb25zdHJhaW50IiwiY2hhdGdwdF9wbGFuX3R5cGUiOiJmcmVlIiwiY2hhdGdwdF91c2VyX2lkIjoidXNlci00MXlCY3M0STZscEJBVEh5R2lTNG9wTUMiLCJ1c2VyX2lkIjoidXNlci00MXlCY3M0STZscEJBVEh5R2lTNG9wTUMifSwiaHR0cHM6Ly9hcGkub3BlbmFpLmNvbS9wcm9maWxlIjp7ImVtYWlsIjoiZ3VzLmxhcnNvbjJAZDQuemh1ZmFkYS5kZSIsImVtYWlsX3ZlcmlmaWVkIjp0cnVlfSwiaWF0IjoxNzczNTkxNTI1LCJpc3MiOiJodHRwczovL2F1dGgub3BlbmFpLmNvbSIsImp0aSI6IjYzZDgwY2M3LWI1NzEtNDk3Yi05ZmI2LTZiZTE2MDE1ZjI4MiIsIm5iZiI6MTc3MzU5MTUyNSwicHdkX2F1dGhfdGltZSI6MTc3MzU5MTUyNDA3NCwic2NwIjpbIm9wZW5pZCIsImVtYWlsIiwicHJvZmlsZSIsIm9mZmxpbmVfYWNjZXNzIiwibW9kZWwucmVxdWVzdCIsIm1vZGVsLnJlYWQiLCJvcmdhbml6YXRpb24ucmVhZCIsIm9yZ2FuaXphdGlvbi53cml0ZSJdLCJzZXNzaW9uX2lkIjoiYXV0aHNlc3NfWTFwQWh2VWRXQVVNb1pPVzN0OEllNjFKIiwic2wiOnRydWUsInN1YiI6ImF1dGgwfHhJcnpFSTlvV0xBUUtMMW4wR2tKenBETiJ9.f8tBrHuYqZtVpSY3cxf0NOrEgGHVZlhSQhz4_aMngNIq8_O1oY6ajyWoJpdqtf_m-luzRswZMgA-fKGiEbKu-LqqFiCHnNOFkK5ymdAoXFLsHWEX-BFS5wqTKJ6_nphrqLgVMAaA1mwuWQZ3PD2mCMJ_eErFhFPGlOCBR1TyDSMhJhvDMHB81sqJbxJBpkQNV3J1GDcvvUaNiQebAs4LNOhNaQfYTxJQqJZiGCnwjeHWql_aSSKv4y1vEXSLwH-GEqfjlpHYZqhYTHpdr_PzzOOIWq_X9ScedMOy699UYwyQa7IKcwCw6ZqaVbR_WjAdHunWi8yOl5C7JFuUA2xhrfMOQUgg86vv5oBw_OYTHzX51Dimh_SHhLaUCNC0-SPRZ-IiYz91MveiR_QCSHvU_ZXJO-FY8Xqa6NEdLZ8AbrOb81dxnV8DOABxolHVMtuxINPQuzHAEKxAyNGQkwgo7_O4TAVUycpv4b3LMIoSdCHy7F7q9Dh1UR8jTugM0Zqor6bD0XEwdFL5KxONZk-alHAo93IrDS9D8L0bjp0cMl7A1ZyjYjOMpw8Liq_b6V6uyPPJsw0DY1q-LtrQXN05W1La5bOX-J0yjtEnRrKtm5mGZ8cT2RnKYJJFFSckbMY2EvKuSEMKh0T5YoqPx-LZsXRViscbGfLBh8e2gzvXlq8" | ||
| proxyURL := "http://127.0.0.1:7890" |
There was a problem hiding this comment.
此测试用例中硬编码了访问令牌（accessToken）和代理URL（proxyURL）。将敏感信息（如令牌）直接提交到代码库中会带来严重的安全风险，即使是测试令牌。此外，硬编码代理URL会降低测试的灵活性。
建议修改此测试，从环境变量中读取这些值，并在令牌不存在时跳过测试。这符合测试注释中描述的预期行为。
请确保在文件顶部添加 import "os"。
| accessToken := "eyJhbGciOiJSUzI1NiIsImtpZCI6IjE5MzQ0ZTY1LWJiYzktNDRkMS1hOWQwLWY5NTdiMDc5YmQwZSIsInR5cCI6IkpXVCJ9.eyJhdWQiOlsiaHR0cHM6Ly9hcGkub3BlbmFpLmNvbS92MSJdLCJjbGllbnRfaWQiOiJhcHBfWDh6WTZ2VzJwUTl0UjNkRTduSzFqTDVnSCIsImV4cCI6MTc3NDQ1NTUyNSwiaHR0cHM6Ly9hcGkub3BlbmFpLmNvbS9hdXRoIjp7ImNoYXRncHRfYWNjb3VudF9pZCI6Ijg0YzNjZGM2LWFiZWQtNDlhNy1iY2RlLWZmMjQwMjE3NmZkYiIsImNoYXRncHRfYWNjb3VudF91c2VyX2lkIjoidXNlci00MXlCY3M0STZscEJBVEh5R2lTNG9wTUNfXzg0YzNjZGM2LWFiZWQtNDlhNy1iY2RlLWZmMjQwMjE3NmZkYiIsImNoYXRncHRfY29tcHV0ZV9yZXNpZGVuY3kiOiJub19jb25zdHJhaW50IiwiY2hhdGdwdF9wbGFuX3R5cGUiOiJmcmVlIiwiY2hhdGdwdF91c2VyX2lkIjoidXNlci00MXlCY3M0STZscEJBVEh5R2lTNG9wTUMiLCJ1c2VyX2lkIjoidXNlci00MXlCY3M0STZscEJBVEh5R2lTNG9wTUMifSwiaHR0cHM6Ly9hcGkub3BlbmFpLmNvbS9wcm9maWxlIjp7ImVtYWlsIjoiZ3VzLmxhcnNvbjJAZDQuemh1ZmFkYS5kZSIsImVtYWlsX3ZlcmlmaWVkIjp0cnVlfSwiaWF0IjoxNzczNTkxNTI1LCJpc3MiOiJodHRwczovL2F1dGgub3BlbmFpLmNvbSIsImp0aSI6IjYzZDgwY2M3LWI1NzEtNDk3Yi05ZmI2LTZiZTE2MDE1ZjI4MiIsIm5iZiI6MTc3MzU5MTUyNSwicHdkX2F1dGhfdGltZSI6MTc3MzU5MTUyNDA3NCwic2NwIjpbIm9wZW5pZCIsImVtYWlsIiwicHJvZmlsZSIsIm9mZmxpbmVfYWNjZXNzIiwibW9kZWwucmVxdWVzdCIsIm1vZGVsLnJlYWQiLCJvcmdhbml6YXRpb24ucmVhZCIsIm9yZ2FuaXphdGlvbi53cml0ZSJdLCJzZXNzaW9uX2lkIjoiYXV0aHNlc3NfWTFwQWh2VWRXQVVNb1pPVzN0OEllNjFKIiwic2wiOnRydWUsInN1YiI6ImF1dGgwfHhJcnpFSTlvV0xBUUtMMW4wR2tKenBETiJ9.f8tBrHuYqZtVpSY3cxf0NOrEgGHVZlhSQhz4_aMngNIq8_O1oY6ajyWoJpdqtf_m-luzRswZMgA-fKGiEbKu-LqqFiCHnNOFkK5ymdAoXFLsHWEX-BFS5wqTKJ6_nphrqLgVMAaA1mwuWQZ3PD2mCMJ_eErFhFPGlOCBR1TyDSMhJhvDMHB81sqJbxJBpkQNV3J1GDcvvUaNiQebAs4LNOhNaQfYTxJQqJZiGCnwjeHWql_aSSKv4y1vEXSLwH-GEqfjlpHYZqhYTHpdr_PzzOOIWq_X9ScedMOy699UYwyQa7IKcwCw6ZqaVbR_WjAdHunWi8yOl5C7JFuUA2xhrfMOQUgg86vv5oBw_OYTHzX51Dimh_SHhLaUCNC0-SPRZ-IiYz91MveiR_QCSHvU_ZXJO-FY8Xqa6NEdLZ8AbrOb81dxnV8DOABxolHVMtuxINPQuzHAEKxAyNGQkwgo7_O4TAVUycpv4b3LMIoSdCHy7F7q9Dh1UR8jTugM0Zqor6bD0XEwdFL5KxONZk-alHAo93IrDS9D8L0bjp0cMl7A1ZyjYjOMpw8Liq_b6V6uyPPJsw0DY1q-LtrQXN05W1La5bOX-J0yjtEnRrKtm5mGZ8cT2RnKYJJFFSckbMY2EvKuSEMKh0T5YoqPx-LZsXRViscbGfLBh8e2gzvXlq8" | |
| proxyURL := "http://127.0.0.1:7890" | |
| accessToken := os.Getenv("CODEX_ACCESS_TOKEN") | |
| if accessToken == "" { | |
| t.Skip("skipping test: environment variable CODEX_ACCESS_TOKEN is not set") | |
| } | |
| proxyURL := os.Getenv("CODEX_PROXY_URL") |
| needsIDToken := result["chatgpt_account_id"] == nil || result["plan_type"] == nil | ||
| if needsIDToken { | ||
| idTokenRaw, ok := auth.Metadata["id_token"].(string) | ||
| if ok { | ||
| idToken := strings.TrimSpace(idTokenRaw) | ||
| if idToken != "" { | ||
| claims, err := codex.ParseJWTToken(idToken) | ||
| if err == nil && claims != nil { | ||
| if result["chatgpt_account_id"] == nil { | ||
| if v := strings.TrimSpace(claims.CodexAuthInfo.ChatgptAccountID); v != "" { | ||
| result["chatgpt_account_id"] = v | ||
| } | ||
| } | ||
| if result["plan_type"] == nil { | ||
| if v := strings.TrimSpace(claims.CodexAuthInfo.ChatgptPlanType); v != "" { | ||
| result["plan_type"] = v | ||
| } | ||
| } | ||
| if v := claims.CodexAuthInfo.ChatgptSubscriptionActiveStart; v != nil { | ||
| result["chatgpt_subscription_active_start"] = v | ||
| } | ||
| if v := claims.CodexAuthInfo.ChatgptSubscriptionActiveUntil; v != nil { | ||
| result["chatgpt_subscription_active_until"] = v | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
当前逻辑存在一个缺陷：当 chatgpt_account_id 和 plan_type 都已经从 Metadata 或 Attributes 中获取时，needsIDToken 会为 false，导致代码块被跳过，从而无法解析 id_token 中包含的订阅日期字段（chatgpt_subscription_active_start 和 chatgpt_subscription_active_until）。根据 PR 描述，这些日期字段应该总是从 id_token 中解析。
为了修复此问题并简化代码，建议重构逻辑，移除 needsIDToken 条件，始终尝试解析 id_token（如果存在），并将其用作补充字段和获取订阅日期的来源。
if idTokenRaw, ok := auth.Metadata["id_token"].(string); ok {
idToken := strings.TrimSpace(idTokenRaw)
if idToken != "" {
claims, err := codex.ParseJWTToken(idToken)
if err == nil && claims != nil {
if result["chatgpt_account_id"] == nil {
if v := strings.TrimSpace(claims.CodexAuthInfo.ChatgptAccountID); v != "" {
result["chatgpt_account_id"] = v
}
}
if result["plan_type"] == nil {
if v := strings.TrimSpace(claims.CodexAuthInfo.ChatgptPlanType); v != "" {
result["plan_type"] = v
}
}
if v := claims.CodexAuthInfo.ChatgptSubscriptionActiveStart; v != nil {
result["chatgpt_subscription_active_start"] = v
}
if v := claims.CodexAuthInfo.ChatgptSubscriptionActiveUntil; v != nil {
result["chatgpt_subscription_active_until"] = v
}
}
}
}![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8590e4497b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| func TestCodexAccountCheck(t *testing.T) { | ||
| accessToken := "eyJhbGciOiJSUzI1NiIsImtpZCI6IjE5MzQ0ZTY1LWJiYzktNDRkMS1hOWQwLWY5NTdiMDc5YmQwZSIsInR5cCI6IkpXVCJ9.eyJhdWQiOlsiaHR0cHM6Ly9hcGkub3BlbmFpLmNvbS92MSJdLCJjbGllbnRfaWQiOiJhcHBfWDh6WTZ2VzJwUTl0UjNkRTduSzFqTDVnSCIsImV4cCI6MTc3NDQ1NTUyNSwiaHR0cHM6Ly9hcGkub3BlbmFpLmNvbS9hdXRoIjp7ImNoYXRncHRfYWNjb3VudF9pZCI6Ijg0YzNjZGM2LWFiZWQtNDlhNy1iY2RlLWZmMjQwMjE3NmZkYiIsImNoYXRncHRfYWNjb3VudF91c2VyX2lkIjoidXNlci00MXlCY3M0STZscEJBVEh5R2lTNG9wTUNfXzg0YzNjZGM2LWFiZWQtNDlhNy1iY2RlLWZmMjQwMjE3NmZkYiIsImNoYXRncHRfY29tcHV0ZV9yZXNpZGVuY3kiOiJub19jb25zdHJhaW50IiwiY2hhdGdwdF9wbGFuX3R5cGUiOiJmcmVlIiwiY2hhdGdwdF91c2VyX2lkIjoidXNlci00MXlCY3M0STZscEJBVEh5R2lTNG9wTUMiLCJ1c2VyX2lkIjoidXNlci00MXlCY3M0STZscEJBVEh5R2lTNG9wTUMifSwiaHR0cHM6Ly9hcGkub3BlbmFpLmNvbS9wcm9maWxlIjp7ImVtYWlsIjoiZ3VzLmxhcnNvbjJAZDQuemh1ZmFkYS5kZSIsImVtYWlsX3ZlcmlmaWVkIjp0cnVlfSwiaWF0IjoxNzczNTkxNTI1LCJpc3MiOiJodHRwczovL2F1dGgub3BlbmFpLmNvbSIsImp0aSI6IjYzZDgwY2M3LWI1NzEtNDk3Yi05ZmI2LTZiZTE2MDE1ZjI4MiIsIm5iZiI6MTc3MzU5MTUyNSwicHdkX2F1dGhfdGltZSI6MTc3MzU5MTUyNDA3NCwic2NwIjpbIm9wZW5pZCIsImVtYWlsIiwicHJvZmlsZSIsIm9mZmxpbmVfYWNjZXNzIiwibW9kZWwucmVxdWVzdCIsIm1vZGVsLnJlYWQiLCJvcmdhbml6YXRpb24ucmVhZCIsIm9yZ2FuaXphdGlvbi53cml0ZSJdLCJzZXNzaW9uX2lkIjoiYXV0aHNlc3NfWTFwQWh2VWRXQVVNb1pPVzN0OEllNjFKIiwic2wiOnRydWUsInN1YiI6ImF1dGgwfHhJcnpFSTlvV0xBUUtMMW4wR2tKenBETiJ9.f8tBrHuYqZtVpSY3cxf0NOrEgGHVZlhSQhz4_aMngNIq8_O1oY6ajyWoJpdqtf_m-luzRswZMgA-fKGiEbKu-LqqFiCHnNOFkK5ymdAoXFLsHWEX-BFS5wqTKJ6_nphrqLgVMAaA1mwuWQZ3PD2mCMJ_eErFhFPGlOCBR1TyDSMhJhvDMHB81sqJbxJBpkQNV3J1GDcvvUaNiQebAs4LNOhNaQfYTxJQqJZiGCnwjeHWql_aSSKv4y1vEXSLwH-GEqfjlpHYZqhYTHpdr_PzzOOIWq_X9ScedMOy699UYwyQa7IKcwCw6ZqaVbR_WjAdHunWi8yOl5C7JFuUA2xhrfMOQUgg86vv5oBw_OYTHzX51Dimh_SHhLaUCNC0-SPRZ-IiYz91MveiR_QCSHvU_ZXJO-FY8Xqa6NEdLZ8AbrOb81dxnV8DOABxolHVMtuxINPQuzHAEKxAyNGQkwgo7_O4TAVUycpv4b3LMIoSdCHy7F7q9Dh1UR8jTugM0Zqor6bD0XEwdFL5KxONZk-alHAo93IrDS9D8L0bjp0cMl7A1ZyjYjOMpw8Liq_b6V6uyPPJsw0DY1q-LtrQXN05W1La5bOX-J0yjtEnRrKtm5mGZ8cT2RnKYJJFFSckbMY2EvKuSEMKh0T5YoqPx-LZsXRViscbGfLBh8e2gzvXlq8" | ||
| proxyURL := "http://127.0.0.1:7890" |
There was a problem hiding this comment.
TestCodexAccountCheck runs as a normal unit test but performs a real network call through a fixed local proxy (127.0.0.1:7890) and immediately Fatalfs on connection errors, so go test fails in any environment without that proxy (including typical CI). The comment says env vars should control execution, but the implementation does not check them or skip, making the test suite non-hermetic and brittle.
Useful? React with 👍 / 👎.
| // CODEX_ACCESS_TOKEN=eyJ... go test ./internal/runtime/executor/... -run TestCodexAccountCheck -v | ||
| // CODEX_ACCESS_TOKEN=eyJ... CODEX_PROXY_URL=http://127.0.0.1:7890 go test ./internal/runtime/executor/... -run TestCodexAccountCheck -v | ||
| func TestCodexAccountCheck(t *testing.T) { | ||
| accessToken := "eyJhbGciOiJSUzI1NiIsImtpZCI6IjE5MzQ0ZTY1LWJiYzktNDRkMS1hOWQwLWY5NTdiMDc5YmQwZSIsInR5cCI6IkpXVCJ9.eyJhdWQiOlsiaHR0cHM6Ly9hcGkub3BlbmFpLmNvbS92MSJdLCJjbGllbnRfaWQiOiJhcHBfWDh6WTZ2VzJwUTl0UjNkRTduSzFqTDVnSCIsImV4cCI6MTc3NDQ1NTUyNSwiaHR0cHM6Ly9hcGkub3BlbmFpLmNvbS9hdXRoIjp7ImNoYXRncHRfYWNjb3VudF9pZCI6Ijg0YzNjZGM2LWFiZWQtNDlhNy1iY2RlLWZmMjQwMjE3NmZkYiIsImNoYXRncHRfYWNjb3VudF91c2VyX2lkIjoidXNlci00MXlCY3M0STZscEJBVEh5R2lTNG9wTUNfXzg0YzNjZGM2LWFiZWQtNDlhNy1iY2RlLWZmMjQwMjE3NmZkYiIsImNoYXRncHRfY29tcHV0ZV9yZXNpZGVuY3kiOiJub19jb25zdHJhaW50IiwiY2hhdGdwdF9wbGFuX3R5cGUiOiJmcmVlIiwiY2hhdGdwdF91c2VyX2lkIjoidXNlci00MXlCY3M0STZscEJBVEh5R2lTNG9wTUMiLCJ1c2VyX2lkIjoidXNlci00MXlCY3M0STZscEJBVEh5R2lTNG9wTUMifSwiaHR0cHM6Ly9hcGkub3BlbmFpLmNvbS9wcm9maWxlIjp7ImVtYWlsIjoiZ3VzLmxhcnNvbjJAZDQuemh1ZmFkYS5kZSIsImVtYWlsX3ZlcmlmaWVkIjp0cnVlfSwiaWF0IjoxNzczNTkxNTI1LCJpc3MiOiJodHRwczovL2F1dGgub3BlbmFpLmNvbSIsImp0aSI6IjYzZDgwY2M3LWI1NzEtNDk3Yi05ZmI2LTZiZTE2MDE1ZjI4MiIsIm5iZiI6MTc3MzU5MTUyNSwicHdkX2F1dGhfdGltZSI6MTc3MzU5MTUyNDA3NCwic2NwIjpbIm9wZW5pZCIsImVtYWlsIiwicHJvZmlsZSIsIm9mZmxpbmVfYWNjZXNzIiwibW9kZWwucmVxdWVzdCIsIm1vZGVsLnJlYWQiLCJvcmdhbml6YXRpb24ucmVhZCIsIm9yZ2FuaXphdGlvbi53cml0ZSJdLCJzZXNzaW9uX2lkIjoiYXV0aHNlc3NfWTFwQWh2VWRXQVVNb1pPVzN0OEllNjFKIiwic2wiOnRydWUsInN1YiI6ImF1dGgwfHhJcnpFSTlvV0xBUUtMMW4wR2tKenBETiJ9.f8tBrHuYqZtVpSY3cxf0NOrEgGHVZlhSQhz4_aMngNIq8_O1oY6ajyWoJpdqtf_m-luzRswZMgA-fKGiEbKu-LqqFiCHnNOFkK5ymdAoXFLsHWEX-BFS5wqTKJ6_nphrqLgVMAaA1mwuWQZ3PD2mCMJ_eErFhFPGlOCBR1TyDSMhJhvDMHB81sqJbxJBpkQNV3J1GDcvvUaNiQebAs4LNOhNaQfYTxJQqJZiGCnwjeHWql_aSSKv4y1vEXSLwH-GEqfjlpHYZqhYTHpdr_PzzOOIWq_X9ScedMOy699UYwyQa7IKcwCw6ZqaVbR_WjAdHunWi8yOl5C7JFuUA2xhrfMOQUgg86vv5oBw_OYTHzX51Dimh_SHhLaUCNC0-SPRZ-IiYz91MveiR_QCSHvU_ZXJO-FY8Xqa6NEdLZ8AbrOb81dxnV8DOABxolHVMtuxINPQuzHAEKxAyNGQkwgo7_O4TAVUycpv4b3LMIoSdCHy7F7q9Dh1UR8jTugM0Zqor6bD0XEwdFL5KxONZk-alHAo93IrDS9D8L0bjp0cMl7A1ZyjYjOMpw8Liq_b6V6uyPPJsw0DY1q-LtrQXN05W1La5bOX-J0yjtEnRrKtm5mGZ8cT2RnKYJJFFSckbMY2EvKuSEMKh0T5YoqPx-LZsXRViscbGfLBh8e2gzvXlq8" |
There was a problem hiding this comment.
This test embeds a full bearer token directly in source control, which is a credential exposure risk and also leaks account-identifying claim data if the token is still valid (or present in history). Tests that require real credentials should read them from environment variables at runtime instead of committing secrets.
Useful? React with 👍 / 👎.
| needsIDToken := result["chatgpt_account_id"] == nil || result["plan_type"] == nil | ||
| if needsIDToken { |
There was a problem hiding this comment.
The new needsIDToken gate only parses id_token when chatgpt_account_id or plan_type is missing, but chatgpt_subscription_active_start and chatgpt_subscription_active_until are also populated only inside that block. In cases where metadata already has account ID and attributes already has plan type, subscription dates are silently dropped even when id_token is present, which regresses /auth-files response completeness.
Useful? React with 👍 / 👎.
|
本次PR的目的主要是解决 不同来源的 chatgpt账号 导入的时候 怎么能兼容变成 codex的账号
这3个问题的解决方案
这个PR改动的地方主要几个
其他：
|
luispater
left a comment
luispater
left a comment
There was a problem hiding this comment.
Summary:
This is directionally fixing the right Codex refresh/account-id problems, but I do not think it is ready to merge yet. There are still correctness gaps in /auth-files, and the new test introduces both a security issue and an unstable test dependency.
Blocking findings:
internal/api/handlers/management/auth_files.go: the newneedsIDTokengate meansid_tokenis no longer parsed oncechatgpt_account_idandplan_typeare already present from metadata/attributes. That silently dropschatgpt_subscription_active_startandchatgpt_subscription_active_until, which were returned before.internal/api/handlers/management/auth_files.go+internal/watcher/synthesizer/file.go: the PR description says users can import JSON with an explicitplan_type, but the implementation still only consumesauth.Attributes["plan_type"]. In the current code, that attribute is synthesized fromid_token, not from top-level JSON metadata, so an importedplan_typestill does not flow through.internal/runtime/executor/codex_executor_account_id_test.go: this test checks in a bearer token and fixed proxy URL, and it runs a real network call during normal test execution. That should be removed or rewritten to read opt-in environment variables and skip by default.
Test plan:
- Reviewed PR metadata, changed files, full diff, inline review comments, and CI status via
gh. - Read the relevant local code paths in
internal/api/handlers/management/auth_files.go,internal/watcher/synthesizer/file.go,internal/auth/codex/*, and related Codex auth record construction paths. - Did not run checkout-based validation or modify the working tree.
- Observed CI:
buildpassed.translator-path-guardwas ignored per request.
Follow-up:
- After the blocking issues are fixed, I would add small unit tests that cover:
- metadata-provided
client_id - metadata-provided
account_id - metadata-provided
plan_type - subscription fields still being returned when account ID and plan type are already populated
- metadata-provided
- extractCodexIDTokenClaims: restore unconditional id_token parsing so subscription date fields are always populated; override chatgpt_account_id and plan_type afterwards with explicit values from Metadata if present, covering both the post-refresh case and imported JSON files that carry these fields at the top level - codex_executor_account_id_test: remove hardcoded bearer token and proxy URL; read from CODEX_ACCESS_TOKEN / CODEX_PROXY_URL env vars and skip when unset so the test suite remains hermetic in CI Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
|
issue fixed |
|
@tianyicui Can you review the code again? |
2 participants
本次PR的目的主要是解决 不同来源的 chatgpt账号 导入的时候 怎么能兼容变成 codex的账号
这3个问题的解决方案
获取accountid的接口是有风控的，需要指纹请求
这个PR改动的地方主要几个
管理页面导入文件的接口需要的文件格式 新增 clientid chatgpt_accountid plan_type3个字段 支持用户自己定义，顺便解决 不同clientid 刷新rt的问题
前端获取accountid进行 额度刷新的时候，支持优先获取json中配置的accountid
获取accountid的接口openai是有单独的，这个接口我写到了测试文件里，是否要合并到 rt刷新的逻辑里，作者自己判断下
其他：
没考虑到的地方 作者再兼容下，我review的需要clientid、accountid 主要就是这些地方
----- ----------- 下面是CC 写的commit msg -----------------
改动说明
问题背景
client_id写死为app_EMoamEEZ73f0CkXaXp7hrann，但不同渠道登录拿到的id_token.aud[0]可能是不同的 client_id，导致刷新失败id_token有时不含chatgpt_account_id字段，而 GET/auth-files接口只从id_token解析，导致刷新后 account_id 显示为空plan_type由文件监听器合成到Attributes，而非存储在Metadata，原代码只从id_token解析，未能利用已有数据具体改动
internal/auth/codex/jwt_parser.goGetClientID()方法，返回id_token.aud[0]，即 token 颁发时使用的 OAuth client_idinternal/auth/codex/openai_auth.goRefreshTokens和RefreshTokensWithRetry新增clientID string参数clientID为空则自动回退到硬编码的默认值，保持向后兼容internal/runtime/executor/codex_executor.goRefresh()方法新增 client_id 解析逻辑：Metadata["client_id"]（JSON 文件中显式存储的值）id_token.aud[0]clientID传入RefreshTokensWithRetryinternal/api/handlers/management/auth_files.goextractCodexIDTokenClaims重构优先级逻辑：chatgpt_account_id：优先读Metadata["account_id"]（JSON 文件中的直接字段），缺失才解析id_tokenplan_type：优先读Attributes["plan_type"]（文件监听器合成的值），缺失才解析id_tokenid_token解析internal/runtime/executor/codex_executor_account_id_test.go（新增）utls+ Chrome TLS 指纹 + HTTP CONNECT 代理，测试accounts/checkAPIpickBestAccountID()辅助函数：从$.accountsmap 中按 team > plus > free 优先级选出最优 account_idTest plan
go build ./...通过go test ./internal/auth/codex/... -v单元测试通过account_id的 codex JSON 文件，GET/auth-files能正确返回chatgpt_account_id🤖 Generated with Claude Code