fix: improve Stripe checkout handling - #4477
Open
yanowo wants to merge 5 commits into
Open
Conversation
Contributor
WalkthroughThis PR introduces a comprehensive Stripe payment system overhaul with live/test mode separation, refactors Vertex AI URL building into centralized utilities, streamlines Claude media content conversion, adds a new frontend theme switcher, and improves transaction-scoped timestamp handling. Core changes span settings, models, controllers, and frontend components. ChangesStripe Payment System Overhaul
Vertex AI URL Building Refactor
Claude Media Content Refactor
Stream Timeout Fallback
Test Infrastructure Enhancement
Frontend Enhancement
Sequence Diagram(s)sequenceDiagram
participant Client
participant TopUpHandler as TopUp Handler
participant Stripe as Stripe Helpers
participant Model
participant DB as Database
Client->>TopUpHandler: POST /stripe/topup (checkout session request)
TopUpHandler->>Stripe: Check setting.StripeEnabled & isStripeAPISecretConfigured()
alt Not Enabled/Configured
TopUpHandler-->>Client: Error response
else Enabled & Configured
TopUpHandler->>Stripe: isStripeLiveMode() → derive liveMode
TopUpHandler->>Model: GetStripeCustomerID(user, liveMode)
Model-->>TopUpHandler: customer ID
TopUpHandler->>Stripe: genStripeLink() with customer ID & liveMode
Stripe-->>TopUpHandler: checkout session
TopUpHandler-->>Client: session URL
end
sequenceDiagram
participant Webhook as Stripe Webhook
participant Handler as TopUp/Subscription Handler
participant Model as Model (Transaction)
participant DB as Database
Webhook->>Handler: charge.succeeded event + livemode flag
Handler->>Model: Recharge(referenceId, customerId, livemode, ...)
Model->>Model: stripeCustomerUpdateFields(customerId, livemode)
Model->>DB: Update User with correct customer field (Test/Live)
DB-->>Model: Done
Handler->>Model: CompleteStripeSubscriptionOrder(..., customerId, livemode)
Model->>DB: updateStripeCustomerForUserTx(tx, userId, customerId, livemode)
DB-->>Model: Done
Model->>DB: Mark subscription order success
DB-->>Model: Done
Model-->>Handler: Success
Handler-->>Webhook: 200 OK
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
controller/subscription_payment_stripe.go (1)
47-53:⚠️ Potential issue | 🟡 MinorUse the shared webhook-config helper here.
This path accepts a whitespace-only
StripeWebhookSecretbecause it only checks== "", while webhook availability uses trimmed validation. That can let users start checkout even though the callback endpoint is effectively disabled, leaving paid subscription orders stuck inpending.Suggested fix
- if setting.StripeWebhookSecret == "" { + if !isStripeWebhookConfigured() { common.ApiErrorMsg(c, "Stripe Webhook 未配置") return }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/subscription_payment_stripe.go` around lines 47 - 53, The current check uses setting.StripeWebhookSecret == "" which allows whitespace-only secrets; replace this with the shared webhook-config helper to validate webhook availability (the same trimmed/non-empty logic used elsewhere) rather than a raw equality check. Specifically, in the handler that calls isStripeAPISecretConfigured() and then checks setting.StripeWebhookSecret, call the shared webhook helper (the project’s webhook-config validation function) to determine if the Stripe webhook is configured and if not return the common.ApiErrorMsg(c, "Stripe Webhook 未配置"); ensure the helper trims the secret so whitespace-only values are treated as missing.
🧹 Nitpick comments (2)
web/src/components/topup/paymentWindow.js (2)
30-41: Hardcoded English placeholder text bypasses i18n.The placeholder copy ("Redirecting to payment...", "Please keep this window open.") is shown to every user — including non-English locales — while the API round-trip is in flight. Per the repo's i18n guidelines, user-facing strings should flow through
i18next. Since the helper is invoked outside React, accept the translated strings as parameters and pass them in from the caller (which already hast(...)available).♻️ Suggested signature change
-export const openPaymentWindow = (title = 'Payment') => { +export const openPaymentWindow = (title = 'Payment', options = {}) => { + const { + headline = 'Redirecting to payment...', + description = 'Please keep this window open.', + } = options; if (typeof window === 'undefined') { return null; } @@ - paymentWindow.document.body.innerHTML = ` + paymentWindow.document.body.innerHTML = ` <main style="min-height: 100vh; display: grid; place-items: center; color: `#0f172a`; background: `#f8fafc`;"> <section style="text-align: center; padding: 24px;"> - <div style="font-size: 16px; font-weight: 600;">Redirecting to payment...</div> - <div style="margin-top: 8px; font-size: 13px; color: `#64748b`;">Please keep this window open.</div> + <div style="font-size: 16px; font-weight: 600;">${headline}</div> + <div style="margin-top: 8px; font-size: 13px; color: `#64748b`;">${description}</div> </section> </main> `;Then in
index.jsx:- const stripePaymentWindow = isStripePayment - ? openPaymentWindow('Stripe') - : null; + const stripePaymentWindow = isStripePayment + ? openPaymentWindow('Stripe', { + headline: t('正在跳转支付...'), + description: t('请保持此窗口打开'), + }) + : null;Note: if
headline/descriptionwill ever come from untrusted sources, switch frominnerHTMLtotextContenton created nodes to avoid HTML injection. Today the values are static, so this is forward-looking guidance.As per coding guidelines: "Frontend i18n translations: use
i18nextwithreact-i18next... callt('中文key')in components."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/topup/paymentWindow.js` around lines 30 - 41, The placeholder strings are hardcoded in the payment popup (the paymentWindow.document.body.innerHTML block) and must be plumbed through i18n; change the helper's signature to accept translated strings (e.g., headline and description) and replace the hardcoded "Redirecting to payment..." and "Please keep this window open." with those parameters, then update the caller (index.jsx) to pass t('...') results into the helper; if these values could ever be untrusted, render them with textContent on created nodes instead of using innerHTML to avoid injection.
47-59: Verify the silent main-window fallback is desired, and consider validating the URL scheme.Two minor concerns worth confirming:
- When the popup is missing/closed (popup-blocker, user closed it, or
openPaymentWindowreturnednull), this redirects the current tab viawindow.location.href = url, navigating the user away from the top-up page. The caller has no way to distinguish "popup redirected" vs "main window redirected" — both returntrue. If the intent is strictly "redirect the popup or do nothing," a different return value for the fallback case would let the caller decide (e.g., show a message with a manual link).urlis only checked for truthiness. Whilepay_linkoriginates from Stripe via your backend, a defense-in-depth scheme check (https:only) would prevent navigation tojavascript:/data:URLs if the pay link is ever tampered with upstream.♻️ Optional hardening
export const redirectPaymentWindow = (paymentWindow, url) => { if (!url || typeof window === 'undefined') { return false; } + try { + const parsed = new URL(url, window.location.origin); + if (parsed.protocol !== 'https:' && parsed.protocol !== 'http:') { + return false; + } + } catch { + return false; + } if (paymentWindow && !paymentWindow.closed) { paymentWindow.location.href = url; return true; } window.location.href = url; return true; };Could you confirm with the team whether falling back to redirecting the main tab (vs. surfacing an error and keeping the user on the top-up page) is the intended UX when the popup is unavailable?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/topup/paymentWindow.js` around lines 47 - 59, The redirectPaymentWindow function currently always returns true and silently falls back to redirecting the main window; change it so it distinguishes the two outcomes (e.g., return "popup" when paymentWindow redirected, "main" when main window redirected, or boolean/enum as agreed) so callers can handle the fallback, and add a scheme whitelist check on url (allow only https: and optionally http: per policy) before navigating—if the URL fails validation, do not navigate and return an explicit error result (e.g., "invalid_url"). Update references to redirectPaymentWindow accordingly to handle the new return values; function name: redirectPaymentWindow, variables: paymentWindow, url.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@controller/subscription_payment_stripe.go`:
- Around line 43-46: The current guard in SubscriptionRequestStripePay uses
setting.StripeEnabled to block all Stripe flows; change it so the check only
blocks one-time top-up/recharge flows. Update the conditional in
SubscriptionRequestStripePay to detect whether the incoming request is a top-up
(e.g., inspect the request payload/field that distinguishes top-ups vs
subscription checkouts) and only return an error when setting.StripeEnabled is
false AND the request is a top-up; allow normal subscription checkout paths to
proceed regardless of the top-up flag. Ensure you reference
setting.StripeEnabled and the request/property used to identify top-ups (the
field used in SubscriptionRequestStripePay) so reviewers can locate the change.
---
Outside diff comments:
In `@controller/subscription_payment_stripe.go`:
- Around line 47-53: The current check uses setting.StripeWebhookSecret == ""
which allows whitespace-only secrets; replace this with the shared
webhook-config helper to validate webhook availability (the same
trimmed/non-empty logic used elsewhere) rather than a raw equality check.
Specifically, in the handler that calls isStripeAPISecretConfigured() and then
checks setting.StripeWebhookSecret, call the shared webhook helper (the
project’s webhook-config validation function) to determine if the Stripe webhook
is configured and if not return the common.ApiErrorMsg(c, "Stripe Webhook 未配置");
ensure the helper trims the secret so whitespace-only values are treated as
missing.
---
Nitpick comments:
In `@web/src/components/topup/paymentWindow.js`:
- Around line 30-41: The placeholder strings are hardcoded in the payment popup
(the paymentWindow.document.body.innerHTML block) and must be plumbed through
i18n; change the helper's signature to accept translated strings (e.g., headline
and description) and replace the hardcoded "Redirecting to payment..." and
"Please keep this window open." with those parameters, then update the caller
(index.jsx) to pass t('...') results into the helper; if these values could ever
be untrusted, render them with textContent on created nodes instead of using
innerHTML to avoid injection.
- Around line 47-59: The redirectPaymentWindow function currently always returns
true and silently falls back to redirecting the main window; change it so it
distinguishes the two outcomes (e.g., return "popup" when paymentWindow
redirected, "main" when main window redirected, or boolean/enum as agreed) so
callers can handle the fallback, and add a scheme whitelist check on url (allow
only https: and optionally http: per policy) before navigating—if the URL fails
validation, do not navigate and return an explicit error result (e.g.,
"invalid_url"). Update references to redirectPaymentWindow accordingly to handle
the new return values; function name: redirectPaymentWindow, variables:
paymentWindow, url.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f95fb85c-2a93-48f7-a294-56da6b97c6ba
📒 Files selected for processing (23)
controller/payment_webhook_availability.gocontroller/payment_webhook_availability_test.gocontroller/subscription_payment_stripe.gocontroller/topup_stripe.gomodel/db_time.gomodel/option.gomodel/stripe_customer_test.gomodel/subscription.gomodel/topup.gomodel/user.gosetting/payment_stripe.goweb/src/components/settings/PaymentSetting.jsxweb/src/components/topup/SubscriptionPlansCard.jsxweb/src/components/topup/index.jsxweb/src/components/topup/paymentWindow.jsweb/src/i18n/locales/en.jsonweb/src/i18n/locales/fr.jsonweb/src/i18n/locales/ja.jsonweb/src/i18n/locales/ru.jsonweb/src/i18n/locales/vi.jsonweb/src/i18n/locales/zh-CN.jsonweb/src/i18n/locales/zh-TW.jsonweb/src/pages/Setting/Payment/SettingsPaymentGatewayStripe.jsx
Comment on lines
+43
to
+46
| if !setting.StripeEnabled { | ||
| common.ApiErrorMsg(c, "Stripe 未启用") | ||
| return | ||
| } |
Contributor
There was a problem hiding this comment.
This turns the new top-up switch into a subscription kill switch.
SubscriptionRequestStripePay now refuses all Stripe subscription checkouts when setting.StripeEnabled is false. Based on the PR scope, that flag is the new Stripe recharge/top-up switch, so disabling one-time top-ups would also block selling subscriptions here.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@controller/subscription_payment_stripe.go` around lines 43 - 46, The current
guard in SubscriptionRequestStripePay uses setting.StripeEnabled to block all
Stripe flows; change it so the check only blocks one-time top-up/recharge flows.
Update the conditional in SubscriptionRequestStripePay to detect whether the
incoming request is a top-up (e.g., inspect the request payload/field that
distinguishes top-ups vs subscription checkouts) and only return an error when
setting.StripeEnabled is false AND the request is a top-up; allow normal
subscription checkout paths to proceed regardless of the top-up flag. Ensure you
reference setting.StripeEnabled and the request/property used to identify
top-ups (the field used in SubscriptionRequestStripePay) so reviewers can locate
the change.
Author
There was a problem hiding this comment.
Thanks for flagging this. This behavior is intentional.
In this project, StripeEnabled is intended to control whether users can initiate Stripe payments, not only one-time top-ups. Subscriptions and top-ups both create Stripe Checkout sessions, so if Stripe is disabled from the payment settings, subscription checkout should also be blocked. Otherwise, disabling Stripe recharge would still leave another Stripe payment entry point available.
Webhook processing remains independent from this switch, so existing or delayed Stripe events can still be handled even after Stripe payments are disabled.
The UI helper text also states that users cannot pay with Stripe when this option is disabled, so blocking subscription checkout is consistent with the intended behavior.
Contributor
There was a problem hiding this comment.
Only users with a collaborator, contributor, member, or owner role can interact with CodeRabbit.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@model/user.go`:
- Around line 58-69: GetStripeCustomerID changed behavior to ignore legacy
StripeCustomer in live mode but still let test-mode fall back to it, which will
cause live customer IDs to be passed to Stripe test keys; add an explicit
migration or documentation: implement a migration/backfill that inspects the
legacy StripeCustomer column and, based on the deployment's historical mode or
current config, moves values into StripeCustomerLive or StripeCustomerTest (or
clears them) so GetStripeCustomerID's assumptions hold, and ensure AutoMigrate
(model/main.go) triggers or documents this migration path and that columns
StripeCustomerLive, StripeCustomerTest and StripeCustomer are handled safely
during rollout.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Comment on lines
+58
to
+69
| func GetStripeCustomerID(user *User, liveMode bool) string { | ||
| if user == nil { | ||
| return "" | ||
| } | ||
| if liveMode { | ||
| return strings.TrimSpace(user.StripeCustomerLive) | ||
| } | ||
| if customerId := strings.TrimSpace(user.StripeCustomerTest); customerId != "" { | ||
| return customerId | ||
| } | ||
| return strings.TrimSpace(user.StripeCustomer) | ||
| } |
Contributor
There was a problem hiding this comment.
❓ Verification inconclusive
Script executed:
#!/bin/bash
# Find any migration / autoMigrate logic that backfills or splits the legacy stripe_customer column.
rg -nP --type=go -C3 '\bstripe_customer(_live|_test)?\b'
rg -nP --type=go -C2 '\bAutoMigrate\b'Repository: QuantumNous/new-api
Repository: QuantumNous/new-api
Exit code: 0
stdout:
model/user.go-48- LinuxDOId string `json:"linux_do_id" gorm:"column:linux_do_id;index"`
model/user.go-49- Setting string `json:"setting" gorm:"type:text;column:setting"`
model/user.go-50- Remark string `json:"remark,omitempty" gorm:"type:varchar(255)" validate:"max=255"`
model/user.go:51: StripeCustomer string `json:"stripe_customer" gorm:"type:varchar(64);column:stripe_customer;index"`
model/user.go:52: StripeCustomerTest string `json:"stripe_customer_test" gorm:"type:varchar(64);column:stripe_customer_test;index"`
model/user.go:53: StripeCustomerLive string `json:"stripe_customer_live" gorm:"type:varchar(64);column:stripe_customer_live;index"`
model/user.go-54- CreatedAt int64 `json:"created_at" gorm:"autoCreateTime;column:created_at"`
model/user.go-55- LastLoginAt int64 `json:"last_login_at" gorm:"default:0;column:last_login_at"`
model/user.go-56-}
--
model/user.go-75- }
model/user.go-76- if liveMode {
model/user.go-77- return map[string]interface{}{
model/user.go:78: "stripe_customer_live": customerId,
model/user.go-79- }
model/user.go-80- }
model/user.go-81- return map[string]interface{}{
model/user.go:82: "stripe_customer": customerId,
model/user.go:83: "stripe_customer_test": customerId,
model/user.go-84- }
model/user.go-85-}
model/user.go-86-
--
controller/user.go-412- "inviter_id": user.InviterId,
controller/user.go-413- "linux_do_id": user.LinuxDOId,
controller/user.go-414- "setting": user.Setting,
controller/user.go:415: "stripe_customer": user.StripeCustomer,
controller/user.go-416- "sidebar_modules": userSetting.SidebarModules, // 正确提取sidebar_modules字段
controller/user.go-417- "permissions": permissions, // 新增权限字段
controller/user.go-418- }
service/waffo_pancake_test.go-30- model.LOG_DB = db
service/waffo_pancake_test.go-31-
service/waffo_pancake_test.go:32: require.NoError(t, db.AutoMigrate(&model.User{}, &model.TopUp{}))
service/waffo_pancake_test.go-33-
service/waffo_pancake_test.go-34- t.Cleanup(func() {
--
service/task_billing_test.go-37- common.LogConsumeEnabled = true
service/task_billing_test.go-38-
service/task_billing_test.go:39: if err := db.AutoMigrate(
service/task_billing_test.go-40- &model.Task{},
service/task_billing_test.go-41- &model.User{},
--
model/task_cas_test.go-34- sqlDB.SetMaxOpenConns(1)
model/task_cas_test.go-35-
model/task_cas_test.go:36: if err := db.AutoMigrate(
model/task_cas_test.go-37- &Task{},
model/task_cas_test.go-38- &User{},
--
model/main.go-256- }
model/main.go-257-
model/main.go:258: err := DB.AutoMigrate(
model/main.go-259- &Channel{},
model/main.go-260- &Token{},
--
model/main.go-290- }
model/main.go-291- } else {
model/main.go:292: if err := DB.AutoMigrate(&SubscriptionPlan{}); err != nil {
model/main.go-293- return err
model/main.go-294- }
--
model/main.go-337- go func(model interface{}, name string) {
model/main.go-338- defer wg.Done()
model/main.go:339: if err := DB.AutoMigrate(model); err != nil {
model/main.go-340- errChan <- fmt.Errorf("failed to migrate %s: %v", name, err)
model/main.go-341- }
--
model/main.go-358- }
model/main.go-359- } else {
model/main.go:360: if err := DB.AutoMigrate(&SubscriptionPlan{}); err != nil {
model/main.go-361- return err
model/main.go-362- }
--
model/main.go-368-func migrateLOGDB() error {
model/main.go-369- var err error
model/main.go:370: if err = LOG_DB.AutoMigrate(&Log{}); err != nil {
model/main.go-371- return err
model/main.go-372- }
--
controller/token_test.go-102- t.Helper()
controller/token_test.go-103-
controller/token_test.go:104: if err := db.AutoMigrate(&model.Token{}); err != nil {
controller/token_test.go-105- t.Fatalf("failed to migrate token table: %v", err)
controller/token_test.go-106- }
--
controller/token_test.go-281- longKey := strings.Repeat("b", 64)
controller/token_test.go-282-
controller/token_test.go:283: if err := db.AutoMigrate(&legacyToken{}); err != nil {
controller/token_test.go-284- t.Fatalf("failed to create legacy token schema: %v", err)
controller/token_test.go-285- }
--
controller/model_list_test.go-44- model.LOG_DB = db
controller/model_list_test.go-45-
controller/model_list_test.go:46: require.NoError(t, db.AutoMigrate(&model.User{}, &model.Channel{}, &model.Ability{}, &model.Model{}, &model.Vendor{}))
controller/model_list_test.go-47-
controller/model_list_test.go-48- t.Cleanup(func() {
Legacy StripeCustomer fallback asymmetry requires explicit migration guidance.
The design is intentional per tests: live mode ignores legacy stripe_customer, test mode falls back to it. This assumes pre-existing stripe_customer data is test-mode IDs.
However, the codebase contains no explicit migration or backfill logic to handle existing installations. AutoMigrate (model/main.go) will add the new columns but will not split or reclassify legacy data. For installations that previously ran Stripe in live mode, the legacy stripe_customer column holds live customer IDs (cus_live_*).
After this change:
- Live-mode lookups correctly return
""and create new customers (legacy ID orphaned — minor inefficiency). - Test-mode lookups will return the legacy live customer ID and pass it to Stripe with a test secret key, causing rejection (
No such customer).
Recommendation: Document this migration caveat in the PR/changelog. For deployments that previously used live mode, either:
- Clear the legacy
stripe_customercolumn before switching to test mode, or - Implement a migration routine that inspects
stripe_customerand conditionally backfills the mode-specific columns based on current configuration.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@model/user.go` around lines 58 - 69, GetStripeCustomerID changed behavior to
ignore legacy StripeCustomer in live mode but still let test-mode fall back to
it, which will cause live customer IDs to be passed to Stripe test keys; add an
explicit migration or documentation: implement a migration/backfill that
inspects the legacy StripeCustomer column and, based on the deployment's
historical mode or current config, moves values into StripeCustomerLive or
StripeCustomerTest (or clears them) so GetStripeCustomerID's assumptions hold,
and ensure AutoMigrate (model/main.go) triggers or documents this migration path
and that columns StripeCustomerLive, StripeCustomerTest and StripeCustomer are
handled safely during rollout.
Author
There was a problem hiding this comment.
Thanks, this is a valid migration caveat. I have added a migration note to the PR description explaining how existing stripe_customer data should be handled for deployments that previously used live mode.
Contributor
There was a problem hiding this comment.
Only users with a collaborator, contributor, member, or owner role can interact with CodeRabbit.
4 tasks
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@relay/channel/claude/relay-claude.go`:
- Around line 446-458: The current early return in the file handling logic uses
mimeType := messageFileMimeType(file) and bails out if mimeType == "" ||
mimeType == "application/octet-stream" before calling service.GetBase64Data,
which allows supported attachments to be dropped prior to MIME normalization;
change the flow so you always call service.GetBase64Data(c, source, ...) first,
then if loadedMimeType != "" assign mimeType = loadedMimeType and only then
decide to return nil when mimeType is still empty or "application/octet-stream"
(or otherwise unsupported) after normalization; apply the same change to the
analogous blocks around the other ranges noted (lines ~478-487 and ~491-503) to
ensure attachments with data URIs or filenames (e.g., .png/.pdf) are preserved
until MIME is resolved by GetBase64Data.
- Around line 429-437: The media handling path calls
mediaMessage.ToFileSource(), service.GetBase64Data(...) and then unconditionally
returns buildClaudeBinaryMediaMessage(...), but buildClaudeBinaryMediaMessage
treats non-PDF types as "image" while Claude only supports image/jpeg,
image/png, image/gif, image/webp and application/pdf; add a MIME-type guard
after GetBase64Data that checks mimeType and returns an error for any mime type
not in the allowed set (image/jpeg, image/png, image/gif, image/webp,
application/pdf) before calling buildClaudeBinaryMediaMessage; apply the same
guard in both places referenced (the block using
mediaMessage.ToFileSource()/GetBase64Data and the secondary block around lines
506-518) to reject audio/video/other types early.
In `@relay/channel/task/vertex/adaptor.go`:
- Around line 229-241: The current code in the function that uses
extractRegionFromOperationName(upstreamName) silently defaults to "us-central1"
when parsing fails, causing a mismatch with the submit path which uses "global";
update the function (the region handling before calling
vertexcore.BuildGoogleModelURL) to either return an error when
extractRegionFromOperationName returns empty (fail fast) or change the fallback
to the same default used by the submit path ("global") so polling targets the
same endpoint as submission; modify the logic around
extractRegionFromOperationName, and ensure the final call to
vertexcore.BuildGoogleModelURL uses the corrected region value.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c980ea93-6fc8-424c-a503-30dc54fadf06
📒 Files selected for processing (9)
model/option.gorelay/channel/claude/relay-claude.gorelay/channel/task/vertex/adaptor.gorelay/channel/vertex/adaptor.gorelay/channel/vertex/url_builder.gorelay/helper/stream_scanner.goservice/channel_affinity_usage_cache_test.goweb/classic/src/components/settings/OtherSetting.jsxweb/classic/src/components/settings/PaymentSetting.jsx
✅ Files skipped from review due to trivial changes (1)
- model/option.go
Comment on lines
+429
to
+437
| source := mediaMessage.ToFileSource() | ||
| if source == nil { | ||
| return nil, nil | ||
| } | ||
| base64Data, mimeType, err := service.GetBase64Data(c, source, "formatting media for Claude") | ||
| if err != nil { | ||
| return nil, fmt.Errorf("get file data failed: %s", err.Error()) | ||
| } | ||
| return buildClaudeBinaryMediaMessage(base64Data, mimeType), nil |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Anthropic Messages API supported input content block types and whether a content block with type: "image"may use non-image MIME types such asaudio/orvideo/``
💡 Result:
Supported Messages API input content block types (the messages[].content array) include (among others) text, image, and various tool-related blocks like tool_use and tool_result. Whether a content block with type: "image" may use non-image MIME types (e.g., audio/* or video/): No. In the Messages API, the image block’s source.media_type is for images and the documented supported media types are only image/jpeg, image/png, image/gif, and image/webp. Using an audio/ or video/* MIME type for an image block is not supported by the API contract. Practical guidance: If you need audio/video, you cannot send them via an image content block; instead you’d need to convert them to supported image formats (e.g., extract frames) or use whatever separate API/tooling Anthropic provides for those media types (not via type: "image").
Citations:
- 1: https://docs.anthropic.com/claude/reference/messages
- 2: https://docs.anthropic.com/en/docs/build-with-claude/vision
- 3: https://docs.anthropic.com/en/docs/upgrading-to-the-messages-api
Reject unsupported media types before building Claude blocks.
The new helper path sends any non-file media source through buildClaudeBinaryMediaMessage, but that helper defaults every non-PDF MIME type to Type: "image". The Anthropic Messages API's image content block only supports image/jpeg, image/png, image/gif, and image/webp. Sending audio/*, video/*, or other unsupported MIME types will produce invalid API calls.
Add a guard to reject non-image, non-PDF MIME types:
Suggested fix
func convertMediaContentToClaudeMessage(c *gin.Context, mediaMessage dto.MediaContent) (*dto.ClaudeMediaMessage, error) {
if mediaMessage.Type == dto.ContentTypeFile {
return convertOpenAIFileContentToClaudeMessage(c, mediaMessage)
}
source := mediaMessage.ToFileSource()
if source == nil {
return nil, nil
}
base64Data, mimeType, err := service.GetBase64Data(c, source, "formatting media for Claude")
if err != nil {
return nil, fmt.Errorf("get file data failed: %s", err.Error())
}
+ if !strings.HasPrefix(mimeType, "image/") && mimeType != "application/pdf" {
+ return nil, nil
+ }
return buildClaudeBinaryMediaMessage(base64Data, mimeType), nil
}Applies to lines 429-437 and 506-518.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| source := mediaMessage.ToFileSource() | |
| if source == nil { | |
| return nil, nil | |
| } | |
| base64Data, mimeType, err := service.GetBase64Data(c, source, "formatting media for Claude") | |
| if err != nil { | |
| return nil, fmt.Errorf("get file data failed: %s", err.Error()) | |
| } | |
| return buildClaudeBinaryMediaMessage(base64Data, mimeType), nil | |
| source := mediaMessage.ToFileSource() | |
| if source == nil { | |
| return nil, nil | |
| } | |
| base64Data, mimeType, err := service.GetBase64Data(c, source, "formatting media for Claude") | |
| if err != nil { | |
| return nil, fmt.Errorf("get file data failed: %s", err.Error()) | |
| } | |
| if !strings.HasPrefix(mimeType, "image/") && mimeType != "application/pdf" { | |
| return nil, nil | |
| } | |
| return buildClaudeBinaryMediaMessage(base64Data, mimeType), nil |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@relay/channel/claude/relay-claude.go` around lines 429 - 437, The media
handling path calls mediaMessage.ToFileSource(), service.GetBase64Data(...) and
then unconditionally returns buildClaudeBinaryMediaMessage(...), but
buildClaudeBinaryMediaMessage treats non-PDF types as "image" while Claude only
supports image/jpeg, image/png, image/gif, image/webp and application/pdf; add a
MIME-type guard after GetBase64Data that checks mimeType and returns an error
for any mime type not in the allowed set (image/jpeg, image/png, image/gif,
image/webp, application/pdf) before calling buildClaudeBinaryMediaMessage; apply
the same guard in both places referenced (the block using
mediaMessage.ToFileSource()/GetBase64Data and the secondary block around lines
506-518) to reject audio/video/other types early.
Comment on lines
+446
to
+458
| mimeType := messageFileMimeType(file) | ||
| if mimeType == "" || mimeType == "application/octet-stream" { | ||
| return nil, nil | ||
| } | ||
|
|
||
| source := types.NewFileSourceFromData(file.FileData, mimeType) | ||
| base64Data, loadedMimeType, err := service.GetBase64Data(c, source, "formatting file for Claude") | ||
| if err != nil { | ||
| return nil, fmt.Errorf("get file data failed: %s", err.Error()) | ||
| } | ||
| if loadedMimeType != "" { | ||
| mimeType = loadedMimeType | ||
| } |
Contributor
There was a problem hiding this comment.
Don't drop attachments before MIME normalization.
This bails out on "" / application/octet-stream before service.GetBase64Data can refine loadedMimeType, and mimeTypeFromDataURI only recognizes headers that contain ;.... Valid inputs like data:image/png,... or generic data:application/octet-stream;... plus a .png / .pdf filename can therefore disappear even though they are actually supported.
Suggested fix
func convertOpenAIFileContentToClaudeMessage(c *gin.Context, mediaMessage dto.MediaContent) (*dto.ClaudeMediaMessage, error) {
file := mediaMessage.GetFile()
if file == nil || file.FileData == "" {
return nil, nil
}
mimeType := messageFileMimeType(file)
- if mimeType == "" || mimeType == "application/octet-stream" {
- return nil, nil
- }
source := types.NewFileSourceFromData(file.FileData, mimeType)
base64Data, loadedMimeType, err := service.GetBase64Data(c, source, "formatting file for Claude")
if err != nil {
return nil, fmt.Errorf("get file data failed: %s", err.Error())
}
- if loadedMimeType != "" {
+ if loadedMimeType != "" && loadedMimeType != "application/octet-stream" {
mimeType = loadedMimeType
}
+ if mimeType == "" || mimeType == "application/octet-stream" {
+ return nil, nil
+ }
if strings.HasPrefix(mimeType, "text/") {
textBytes, err := base64.StdEncoding.DecodeString(base64Data)
if err != nil {
return nil, fmt.Errorf("decode text file failed: %s", err.Error())
@@
func messageFileMimeType(file *dto.MessageFile) string {
if file == nil {
return ""
}
- if mimeType := mimeTypeFromDataURI(file.FileData); mimeType != "" {
+ if mimeType := mimeTypeFromDataURI(file.FileData); mimeType != "" && mimeType != "application/octet-stream" {
return mimeType
}
if dot := strings.LastIndex(file.FileName, "."); dot != -1 && dot+1 < len(file.FileName) {
return service.GetMimeTypeByExtension(file.FileName[dot+1:])
}
return ""
}
func mimeTypeFromDataURI(data string) string {
if !strings.HasPrefix(data, "data:") {
return ""
}
idx := strings.Index(data, ",")
if idx == -1 {
return ""
}
- header := data[:idx]
- if semi := strings.Index(header, ";"); semi != -1 && len("data:") < semi {
- return header[len("data:"):semi]
- }
- return ""
+ header := data[len("data:"):idx]
+ if semi := strings.Index(header, ";"); semi != -1 {
+ header = header[:semi]
+ }
+ return header
}Also applies to: 478-487, 491-503
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@relay/channel/claude/relay-claude.go` around lines 446 - 458, The current
early return in the file handling logic uses mimeType :=
messageFileMimeType(file) and bails out if mimeType == "" || mimeType ==
"application/octet-stream" before calling service.GetBase64Data, which allows
supported attachments to be dropped prior to MIME normalization; change the flow
so you always call service.GetBase64Data(c, source, ...) first, then if
loadedMimeType != "" assign mimeType = loadedMimeType and only then decide to
return nil when mimeType is still empty or "application/octet-stream" (or
otherwise unsupported) after normalization; apply the same change to the
analogous blocks around the other ranges noted (lines ~478-487 and ~491-503) to
ensure attachments with data URIs or filenames (e.g., .png/.pdf) are preserved
until MIME is resolved by GetBase64Data.
Comment on lines
+229
to
+241
| region := extractRegionFromOperationName(upstreamName) | ||
| if region == "" { | ||
| region = "us-central1" | ||
| } | ||
| project := extractProjectFromOperationName(upstreamName) | ||
| modelName := extractModelFromOperationName(upstreamName) | ||
| if strings.TrimSpace(modelName) == "" { | ||
| return "", fmt.Errorf("cannot extract model from operation name") | ||
| } | ||
| if strings.TrimSpace(project) == "" { | ||
| return "", fmt.Errorf("cannot extract project from operation name") | ||
| } | ||
| return vertexcore.BuildGoogleModelURL(baseURL, vertexcore.DefaultAPIVersion, project, region, modelName, "fetchPredictOperation"), nil |
Contributor
There was a problem hiding this comment.
Don't silently switch fetches to us-central1 on parse misses.
Line 230 falls back to us-central1 when the operation name parser can't recover a region, but BuildRequestURL uses global when the submit path can't derive one. If the upstream operation format ever changes, polling will hit a different endpoint than submit used and fail with opaque 404s. Please either fail fast here or reuse the same default as submit.
Suggested fix
func buildFetchOperationURL(baseURL, upstreamName string) (string, error) {
region := extractRegionFromOperationName(upstreamName)
- if region == "" {
- region = "us-central1"
+ if strings.TrimSpace(region) == "" {
+ return "", fmt.Errorf("cannot extract region from operation name")
}
project := extractProjectFromOperationName(upstreamName)
modelName := extractModelFromOperationName(upstreamName)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| region := extractRegionFromOperationName(upstreamName) | |
| if region == "" { | |
| region = "us-central1" | |
| } | |
| project := extractProjectFromOperationName(upstreamName) | |
| modelName := extractModelFromOperationName(upstreamName) | |
| if strings.TrimSpace(modelName) == "" { | |
| return "", fmt.Errorf("cannot extract model from operation name") | |
| } | |
| if strings.TrimSpace(project) == "" { | |
| return "", fmt.Errorf("cannot extract project from operation name") | |
| } | |
| return vertexcore.BuildGoogleModelURL(baseURL, vertexcore.DefaultAPIVersion, project, region, modelName, "fetchPredictOperation"), nil | |
| region := extractRegionFromOperationName(upstreamName) | |
| if strings.TrimSpace(region) == "" { | |
| return "", fmt.Errorf("cannot extract region from operation name") | |
| } | |
| project := extractProjectFromOperationName(upstreamName) | |
| modelName := extractModelFromOperationName(upstreamName) | |
| if strings.TrimSpace(modelName) == "" { | |
| return "", fmt.Errorf("cannot extract model from operation name") | |
| } | |
| if strings.TrimSpace(project) == "" { | |
| return "", fmt.Errorf("cannot extract project from operation name") | |
| } | |
| return vertexcore.BuildGoogleModelURL(baseURL, vertexcore.DefaultAPIVersion, project, region, modelName, "fetchPredictOperation"), nil |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@relay/channel/task/vertex/adaptor.go` around lines 229 - 241, The current
code in the function that uses extractRegionFromOperationName(upstreamName)
silently defaults to "us-central1" when parsing fails, causing a mismatch with
the submit path which uses "global"; update the function (the region handling
before calling vertexcore.BuildGoogleModelURL) to either return an error when
extractRegionFromOperationName returns empty (fail fast) or change the fallback
to the same default used by the submit path ("global") so polling targets the
same endpoint as submission; modify the logic around
extractRegionFromOperationName, and ensure the final call to
vertexcore.BuildGoogleModelURL uses the corrected region value.
11 tasks
Calcium-Ion
force-pushed
the
main
branch
2 times, most recently
from
August 30, 2026 15:03
51fdfc5 to
2b6f1df
Compare
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
1 participant
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.
📝 變更描述 / Description
此 PR 修正兩個 Stripe Checkout 相關問題。
第一，前端現在會在使用者確認 Stripe 付款時，先同步開啟一個臨時付款視窗，等後端回傳 Checkout URL 後再將該視窗導向 Stripe。這可以避免手機版 Safari 因為
window.open發生在非同步 API 回應之後，而封鎖 Stripe 支付頁面。第二，Stripe customer id 現在會依 test/live 模式分開保存。測試模式 customer id 會寫入
stripe_customer_test，正式模式 customer id 會寫入stripe_customer_live，建立 Checkout Session 時也會根據目前設定的 Stripe API key 模式選擇正確的 customer id。這可以避免先輸入測試 key 後產生的 customer id，在切換成正式 key 後仍被正式環境重用，導致 Stripe 回傳 customer id 錯誤。此 PR 也新增 Stripe 支付可用性開關。關閉後會保留 Stripe 金鑰與價格設定，但使用者不能再建立 Stripe Checkout，包括儲值與訂閱付款。Webhook 處理仍保持獨立，不依賴此開關與 Price ID，避免關閉 Stripe 支付或調整設定時，尚未完成的 Stripe 回調無法被正確處理。
舊版只有
stripe_customer欄位，該欄位沒有記錄 customer id 是由 Stripe test key 還是 live key 建立。Stripe customer id 本身也無法可靠判斷 test/live 模式，因此程式無法在升級時自動百分之百正確分類既有資料。此 PR 的行為如下：
stripe_customer_live，不會 fallback 到舊的stripe_customer，避免測試環境 customer id 被正式 Stripe key 重用。stripe_customer_test，並保留對舊stripe_customer的 fallback，以維持既有測試環境資料的相容性。如果既有部署過去曾使用 live mode，舊的
stripe_customer可能存放的是 live customer id。這類部署日後若切換到 test mode，建議先依自身歷史設定處理舊資料：stripe_customer回填到stripe_customer_live。stripe_customer，讓系統重新建立 test customer。🚀 變更類型 / Type of change
🔗 關聯任務 / Related Issue
✅ 提交前檢查項 / Checklist
Bug fix，我已提交或關聯對應 Issue，且不會將設計取捨、預期不一致或理解偏差直接歸類為 bug。📸 執行證明 / Proof of Work