Jinn deploy

[farmer-data]

⚠️ 提交说明 / PR Notice

Important

• 请提供人工撰写的简洁摘要，避免直接粘贴未经整理的 AI 输出。

📝 变更描述 / Description

(简述：做了什么？为什么这样改能生效？请基于你对代码逻辑的理解来写，避免粘贴未经整理的内容)

🚀 变更类型 / Type of change

• 🐛 Bug 修复 (Bug fix) - 请关联对应 Issue，避免将设计取舍、理解偏差或预期不一致直接归类为 bug
• ✨ 新功能 (New feature) - 重大特性建议先通过 Issue 沟通
• ⚡ 性能优化 / 重构 (Refactor)
• 📝 文档更新 (Documentation)

🔗 关联任务 / Related Issue

• Closes # (如有)

✅ 提交前检查项 / Checklist

• 人工确认: 我已亲自整理并撰写此描述，没有直接粘贴未经处理的 AI 输出。
• 非重复提交: 我已搜索现有的 Issues 与 PRs，确认不是重复提交。
• Bug fix 说明: 若此 PR 标记为 Bug fix，我已提交或关联对应 Issue，且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。
• 变更理解: 我已理解这些更改的工作原理及可能影响。
• 范围聚焦: 本 PR 未包含任何与当前任务无关的代码改动。
• 本地验证: 已在本地运行并通过测试或手动验证，维护者可以据此复核结果。
• 安全合规: 代码中无敏感凭据，且符合项目代码规范。

📸 运行证明 / Proof of Work

(请在此粘贴截图、关键日志或测试报告，以证明变更生效)

Summary by CodeRabbit

• New Features
  • Added email-only sign-in with one-time codes, rate limiting, lockout protection, and localized messages.
  • Added persistent access tokens for authenticated non-browser clients.
  • Added Airwallex subscription payments, cancellations, deferred plan changes, renewals, and webhook processing.
  • Added usage dashboards, monthly cost/image allowances, image deduplication, and localized quota messaging.
  • Added recurring subscription charging and cancellation reconciliation.
• Bug Fixes
  • Prevented billing identifiers and profile fields from being cleared during edits.
  • Improved payment, webhook, email, and access-control reliability.
• Documentation
  • Added deployment, rollback, and troubleshooting guidance.

[coderabbitai]

Walkthrough

This PR adds email authentication, Airwallex payment and subscription billing, usage accounting and limits, recurring reconciliation, CORS updates, deployment procedures, and related persistence and test coverage.

Changes

Email authentication

Layer / File(s)	Summary
Email authentication contracts and verification primitives common/*, model/option.go, i18n/*	Adds email-login settings, numeric codes, attempt tracking, option handling, email formatting, and translations.
Email login handlers and session tokens controller/user.go, router/api-router.go	Adds email login and code verification endpoints, auto-provisioning, resend cooldowns, and access_token responses.

Airwallex billing

Layer / File(s)	Summary
Airwallex client and billing API operations service/airwallex/*, setting/payment_airwallex.go	Adds authenticated customer, checkout, payment-intent, subscription, and deferred price-switching operations with caching and tests.
Subscription persistence model/subscription.go, model/main.go, model/topup.go	Adds Airwallex price and customer mappings, cancellation and renewal helpers, payment constants, migrations, and tests.
Payment and webhook controllers controller/subscription_payment_airwallex.go, router/api-router.go, controller/testdata/*	Adds payment, cancellation, plan-switch, signed webhook, checkout completion, invoice renewal, and one-off payment handling.
Recurring charging and reconciliation service/subscription_recurring.go, service/subscription_reconcile.go, service/subscription_reset_task.go	Adds recurring charger registration, Airwallex cancellation reconciliation, scheduled execution, and tests.

Usage accounting and limits

Layer / File(s)	Summary
Usage storage and cycles model/usage_counter.go, service/usage*.go, service/text_quota.go	Adds cycle-based counters, image deduplication, atomic reservations, shadow cost accrual, subscription aggregation, and billing-anchored cycles.
Usage gates and messages setting/usage_limit.go, service/usage_gate.go, service/subscription_pause.go, i18n/*	Adds monthly cost and image limits, localized allowance checks, image charging, and subscription-pause messages.
Usage API and rate-limit integration middleware/model-rate-limit.go, controller/usage.go, router/api-router.go	Adds Redis request-window reads, usage-meter construction, authenticated usage retrieval, and tests.

Platform and deployment

Layer / File(s)	Summary
Deployment procedure and build settings DEPLOY.md, Dockerfile, CLAUDE.md	Documents deployment, rollback, troubleshooting, server layout, rule numbering, and Go build settings.
CORS and configuration safeguards middleware/cors.go, model/option.go, model/user.go, controller/subscription.go, controller/option.go	Adds trusted-origin CORS handling, option validation, Airwallex setting updates, and protection against empty-field overwrites.

Estimated code review effort: 5 (Critical) | ~120 minutes

Possibly related PRs

• QuantumNous/new-api#6329: Overlaps with the email authentication and access-token changes in controller/user.go.
• QuantumNous/new-api#6529: Also changes Go builder dependency configuration through GOPROXY.

Poem

A rabbit sends codes through the night,
While billing webhooks hop into sight.
Usage counts gather, clean and bright,
Plans switch softly at the right date.
CORS guards the gate with care,
Deployment rolls back when needed there.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 39.47% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title refers to the deployment documentation and configuration changes, but it does not describe the main usage tracking and subscription control changes.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches 💡 1

⚔️ Resolve merge conflicts 💡

• Resolve merge conflict in branch jinn-deploy

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Note

Due to the large number of review comments, Critical severity comments were prioritized as inline comments.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

CLAUDE.md (1)

109-122: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Restore or explicitly replace the protected-project guardrail.

The change removes the Protected Project Information — DO NOT Modify or Delete rule. Unless an equivalent policy exists elsewhere, future AI-assisted edits can modify or delete protected files and configuration without this repository-level safeguard.

🤖 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 `@CLAUDE.md` around lines 109 - 122, Restore the “Protected Project Information
— DO NOT Modify or Delete” rule in CLAUDE.md, or replace it with an equivalent
repository-level safeguard that clearly identifies protected files and
configuration and prohibits modifying or deleting them. Ensure the guardrail
remains prominent and enforceable for AI-assisted edits.

controller/user.go (1)

330-348: 🔒 Security & Privacy | 🔴 Critical | 🏗️ Heavy lift

Every login response now leaks the system management access token, including browser password logins.

setupLogin is shared by Login, Verify2FALogin, passkey and OAuth flows, so the long-lived, non-expiring access_token — the credential that authorizes admin/user management endpoints — is now returned in the JSON body of every successful browser login and becomes reachable to any script on the page. The session cookie is HttpOnly precisely to avoid this; a single XSS now yields a persistent credential that survives logout. It also silently mints tokens for users who never requested one.

Scope this to the clients that actually need it (e.g. only when the request carries the desktop/Office client marker, or a dedicated /api/user/login/email/verify-code response field) rather than the shared helper.

🔒 Sketch

-	accessToken := ensureAccessToken(user)
+	// Only non-browser surfaces that cannot carry the session cookie need this.
+	var accessToken string
+	if wantsAccessToken(c) {
+		accessToken = ensureAccessToken(user)
+	}

with wantsAccessToken gating on an explicit client header/flag set by the desktop and Office webview clients.

🤖 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 `@controller/user.go` around lines 330 - 348, Update the shared login response
flow around setupLogin so accessToken is minted and included in
data.access_token only when an explicit desktop/Office client marker or
equivalent wantsAccessToken flag is present. Keep browser password, 2FA,
passkey, and OAuth responses cookie-only, and ensure clients that do not request
the token do not trigger ensureAccessToken(user).

🟠 Major comments (22)

service/subscription_recurring_test.go-17-22 (1)

17-22: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Use testify/require for fatal assertions.

Replace the t.Fatalf/t.Fatal assertions in this file with require checks, and wrap the empty-registry no-panic expectation in the new test with require.NotPanics.

🤖 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 `@service/subscription_recurring_test.go` around lines 17 - 22, Replace all
t.Fatal/t.Fatalf assertions in service/subscription_recurring_test.go with
equivalent testify/require checks, adding the required import. In
TestChargeDueAgreementSubscriptionsNoopWhenEmpty, wrap
ChargeDueAgreementSubscriptions with require.NotPanics to explicitly enforce the
no-panic expectation.

Source: Coding guidelines

controller/subscription_payment_airwallex.go-159-182 (1)

159-182: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Insert the pending order before creating the Airwallex checkout.

If order.Insert() fails after genAirwallexSubscriptionLink succeeded, the hosted checkout already exists and the user can pay — but no local order row exists, so billing_checkout.completed will hit the "订单未找到，忽略" path and silently drop a paid charge (the same failure class the fixtures in this PR were written to regress). Inserting the pending order first (and cleaning up / leaving it pending on link failure) keeps the webhook resolvable.

🤖 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 `@controller/subscription_payment_airwallex.go` around lines 159 - 182, Move
the pending SubscriptionOrder creation and order.Insert call before
genAirwallexSubscriptionLink in the subscription payment flow. If link
generation fails, retain or clean up the inserted pending order according to
existing conventions, while ensuring successful checkouts always have a
resolvable local order for billing_checkout.completed.

middleware/cors.go-16-20 (1)

16-20: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Credentialed CORS allowlist is compiled in and includes https://localhost:3000/3001.

These entries ship to production, so any page an attacker can get running on the victim's localhost:3000/3001 (dev server, malicious local app, a compromised local tool) can issue cookie-authenticated requests to /api/user/self, /api/token/*, etc. The same list also gates payment return URLs via TrustedBrowserOrigin. Recommend sourcing this from configuration/env and gating the localhost entries behind non-production mode.

🤖 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 `@middleware/cors.go` around lines 16 - 20, Update trustedCredentialedOrigins
so credentialed CORS origins are sourced from configuration or environment
rather than compiled into the binary, and ensure localhost:3000 and
localhost:3001 are added only in non-production mode. Preserve the production
account portal origin and ensure the same configured allowlist remains available
to TrustedBrowserOrigin for payment return-URL validation.

middleware/cors.go-36-43 (1)

36-43: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Vary: Origin must be set on the wildcard branch too.

The response body/headers differ per origin (echoed origin + credentials vs *), but Vary: Origin is only emitted for trusted origins. A shared cache can therefore serve a trusted origin's credentialed Access-Control-Allow-Origin to another origin, or cache * for a trusted origin and break its credentialed flow.

🛡️ Proposed fix

 		origin := c.Request.Header.Get("Origin")
 		if origin != "" {
+			c.Header("Vary", "Origin")
 			if trustedCredentialedOrigins[origin] {
 				c.Header("Access-Control-Allow-Origin", origin)
 				c.Header("Access-Control-Allow-Credentials", "true")
-				c.Header("Vary", "Origin")
 			} else {
 				c.Header("Access-Control-Allow-Origin", "*")
 			}

🤖 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 `@middleware/cors.go` around lines 36 - 43, Update the origin-handling branch
in the CORS middleware so the wildcard response also sets the `Vary` header to
`Origin`. Ensure every non-empty-origin response path, including the trusted
credentialed and untrusted wildcard branches, emits `Vary: Origin`.

controller/subscription_payment_airwallex.go-442-451 (1)

442-451: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Enforce Airwallex x-timestamp freshness before accepting signatures.

verifyAirwallexSignature hashes the timestamp, but it never checks whether the timestamp is recent; because x-timestamp is in Unix milliseconds, a captured request can be replayed indefinitely. Reject signed headers whose timestamp is outside a short tolerance window, such as ±5 minutes.

🤖 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 `@controller/subscription_payment_airwallex.go` around lines 442 - 451, Update
verifyAirwallexSignature to parse x-timestamp as a Unix-millisecond value and
reject it when it falls outside a short freshness window, such as ±5 minutes
from the current time. Return false for empty, malformed, or stale/future
timestamps before validating the HMAC, while preserving the existing signature
computation for fresh requests.

DEPLOY.md-28-28 (1)

28-28: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Pin the runtime base image.

calciumion/new-api:latest makes redeployments and rollbacks non-reproducible: rebuilding an old binary can silently pull a different upstream image. Use an immutable version or digest and update it deliberately.

🤖 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 `@DEPLOY.md` at line 28, Update the Dockerfile.newapi runtime base image
reference from calciumion/new-api:latest to an immutable version tag or digest,
and ensure future upgrades change this reference deliberately while preserving
the existing bin/new-api copy behavior.

DEPLOY.md-72-72 (1)

72-72: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Do not use docker compose restart for secret rotation.

Restarting an existing container does not reapply changed environment variables. Require docker compose up -d --force-recreate (or equivalent) after editing .env.

🤖 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 `@DEPLOY.md` at line 72, Update the “Secrets rotation” guidance in DEPLOY.md to
remove docker compose restart as an acceptable action and require docker compose
up -d --force-recreate for new-api after editing /opt/newapi/.env, or an
equivalent container recreation command.

Dockerfile-22-22 (1)

22-22: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Do not disable Go module checksum verification in the build settings.

Both the Docker builder and the documented deploy-time env globally set GOSUMDB=off, which bypasses Go’s checksum-database integrity checks. Remove the global flag from Dockerfile#L22 and document a trusted checksum-verification path in DEPLOY.md#L49-L50 instead; use scoped GONOSUMDB only if some modules cannot reach the checksum database.

🤖 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 `@Dockerfile` at line 22, Remove the global GOSUMDB=off setting from Dockerfile
line 22 while preserving the other Go build environment variables. Update
DEPLOY.md lines 49-50 to document a trusted checksum-verification configuration,
using scoped GONOSUMDB only for modules that cannot access the checksum
database; no direct Dockerfile change is needed beyond removing GOSUMDB.

model/subscription_billing_customer_test.go-5-25 (1)

5-25: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Use require/assert in the new backend tests.

• model/subscription_billing_customer_test.go#L5-L25: replace setup and fatal t.Fatal/t.Fatalf assertions with require.
• model/subscription_plan_by_price_test.go#L5-L44: replace setup and fatal assertions with require.
• model/subscription_supersede_test.go#L9-L93: replace setup and fatal assertions with require; use assert for independent postconditions where appropriate.

As per coding guidelines, new Go backend tests must use testify/require for setup and fatal assertions and testify/assert for non-fatal checks.

🤖 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 `@model/subscription_billing_customer_test.go` around lines 5 - 25, Replace
fatal test assertions and setup checks in
model/subscription_billing_customer_test.go lines 5-25 and
model/subscription_plan_by_price_test.go lines 5-44 with testify/require calls.
In model/subscription_supersede_test.go lines 9-93, use require for setup and
assertions that must stop execution, and assert for independent postconditions;
add or reuse the appropriate testify imports.

Source: Coding guidelines

model/subscription.go-394-410 (1)

394-410: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Fail closed on duplicate Airwallex price mappings.

This lookup silently selects the lowest-ID plan when two rows share a price ID. A duplicated configuration can therefore grant the entitlement for a different plan than the charged Airwallex price. Enforce uniqueness for non-empty IDs in plan writes and return an error if this lookup finds multiple rows.

🤖 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 `@model/subscription.go` around lines 394 - 410, Update
GetSubscriptionPlanByAirwallexPriceId to detect multiple matching
SubscriptionPlan rows and return an error instead of selecting the lowest-ID
result; preserve nil results for no matches or blank IDs. Enforce uniqueness of
non-empty airwallex_price_id values in the SubscriptionPlan write/create/update
paths, while allowing empty IDs to remain non-unique.

controller/subscription.go-280-286 (1)

280-286: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Preserve “omitted” versus “explicitly clear” field semantics.

Both sites treat "" as absent, so successful API calls cannot clear existing values.

• controller/subscription.go#L280-L286: use a presence-aware request field so an explicit empty airwallex_price_id can disable that billing mapping while omission preserves it.
• model/user.go#L522-L538: use presence-aware update fields so admins can explicitly clear username, display_name, or remark without omitted fields erasing stored values.

🤖 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 `@controller/subscription.go` around lines 280 - 286, Preserve
omitted-versus-explicit-empty semantics at both update sites: in
controller/subscription.go lines 280-286, make the AirwallexPriceId request
field presence-aware so omission leaves the existing mapping unchanged while an
explicit empty value clears it; in model/user.go lines 522-538, make username,
display_name, and remark update fields presence-aware so omitted fields are
preserved and explicitly empty values clear the stored values.

model/subscription.go-166-166 (1)

166-166: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Add airwallex_price_id to the SQLite subscription_plans migration.

ensureSubscriptionPlanTableSQLite creates/updates subscription_plans separately, and the current required column list omits this new field. Add the column to both the initial CREATE TABLE DDL and the required upgrade list, using ALTER TABLE ... ADD COLUMN.

🤖 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 `@model/subscription.go` at line 166, Add the Airwallex price ID column to the
SQLite subscription plan schema: update ensureSubscriptionPlanTableSQLite’s
initial CREATE TABLE DDL and its required upgrade-column list, and add missing
columns through ALTER TABLE ... ADD COLUMN using the existing migration pattern.

Source: Coding guidelines

model/subscription.go-1361-1376 (1)

1361-1376: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Make the billing-customer save atomic.

AirwallexBillingCustomer.user_id has a uniqueIndex, but the read/create sequence at model/subscription.go:1366-1371 lets concurrent checkouts create customers and then hit a duplicate-key error on Create after Airwallex has already allocated the billing customer. Replace it with a GORM-backed upsert on user_id, e.g. using clause.OnConflict { Columns: []clause.Column{{Name: "user_id"}}, DoUpdates: clause.Assignments(map[string]any{"customer_id": true}) }.Create(...).

🤖 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 `@model/subscription.go` around lines 1361 - 1376, Replace the
read-then-create/update sequence in SaveAirwallexBillingCustomerId with a single
GORM upsert keyed by user_id, using the existing unique index and OnConflict
configuration. Ensure conflicts update customer_id and preserve CreatedAt for
inserts, while retaining the current input validation and error propagation.

Source: Coding guidelines

service/airwallex/client_test.go-1-131 (1)

1-131: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

New Airwallex test files don't use testify as required.

Both files are entirely new and use raw t.Fatalf/manual if err != nil checks instead of testify/require and testify/assert.

• service/airwallex/client_test.go#L1-L131: replace fatal setup/assertion checks (e.g. lines 55-61, 69-72, 82-85) with require.NoError/require.Equal, and non-fatal checks with assert.*.
• service/airwallex/billing_test.go#L1-L168: same replacement across all assertions (e.g. lines 17-25, 64-72, 90-100).

As per coding guidelines: "New or substantially rewritten Go backend tests must use testify/require for setup and fatal assertions and testify/assert for non-fatal checks."

🤖 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 `@service/airwallex/client_test.go` around lines 1 - 131, Replace manual checks
throughout service/airwallex/client_test.go lines 1-131 and
service/airwallex/billing_test.go lines 1-168 with testify assertions: use
require.NoError and other require methods for setup or fatal conditions, and
assert methods for non-fatal validations. Add the testify/require and
testify/assert imports while preserving each test’s existing expectations and
coverage.

Source: Coding guidelines

service/airwallex/client.go-42-78 (1)

42-78: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Lock held across the entire Airwallex login HTTP call serializes all Airwallex traffic.

tokenMu is acquired at function entry and released only via defer, so it stays held for the full network round-trip to /authentication/login. Since every caller of do() goes through getToken() first, a slow or hung Airwallex login endpoint blocks all concurrent Airwallex operations (webhooks, checkouts, cancellations, price switches) one at a time for up to the 30s client timeout — a cascading stall under load or during an Airwallex outage.

🔒 Proposed fix: release the lock before the network call (double-checked locking)

 func getToken() (string, error) {
 	tokenMu.Lock()
-	defer tokenMu.Unlock()
 	if cachedToken != "" && time.Now().Before(tokenExpiry) {
-		return cachedToken, nil
+		token := cachedToken
+		tokenMu.Unlock()
+		return token, nil
 	}
+	tokenMu.Unlock()
+
 	req, err := http.NewRequest(http.MethodPost, setting.AirwallexApiBase+"/api/v1/authentication/login", nil)
 	...
 	if loginResp.Token == "" {
 		return "", errors.New("airwallex login returned empty token")
 	}
+	tokenMu.Lock()
 	cachedToken = loginResp.Token
 	tokenExpiry = time.Now().Add(tokenLifetime)
+	tokenMu.Unlock()
 	return cachedToken, nil
 }

This allows brief duplicate concurrent logins near expiry (harmless — Airwallex tokens aren't single-use) while preventing a slow login from stalling unrelated in-flight requests.

🤖 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 `@service/airwallex/client.go` around lines 42 - 78, Update getToken so tokenMu
is held only while checking cachedToken/tokenExpiry and while storing a newly
fetched token, not during HTTP request creation, execution, body reading, or
response parsing. Use double-checked locking before publishing the refreshed
token, preserving cached-token reuse and allowing concurrent login attempts
without serializing Airwallex operations.

controller/user.go-207-215 (1)

207-215: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

EmailLoginCodeTTL is never enforced — it only formats the email text.

The code's real lifetime comes from common.VerifyCodeWithKey, which compares against VerificationValidMinutes*60 (10 min), not EmailLoginCodeTTL. An operator lowering EmailLoginCodeTTL to 300 gets an email saying "expires in 5 minutes" while the code stays valid for 10. Either drop the setting or thread it into verification.

🐛 Suggested direction

Add a TTL-aware verify in common/verification.go and call it here:

// common/verification.go
func VerifyCodeWithKeyTTL(key, code, purpose string, ttlSeconds int) bool {
	verificationMutex.Lock()
	defer verificationMutex.Unlock()
	value, ok := verificationMap[purpose+key]
	if !ok || int(time.Since(value.time).Seconds()) >= ttlSeconds {
		return false
	}
	return subtle.ConstantTimeCompare([]byte(code), []byte(value.code)) == 1
}

then in VerifyEmailLoginCode: common.VerifyCodeWithKeyTTL(req.Email, req.Code, common.EmailLoginPurpose, common.EmailLoginCodeTTL).

🤖 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 `@controller/user.go` around lines 207 - 215, Enforce common.EmailLoginCodeTTL
during email-code verification instead of only using it in the expiration
message. Add a TTL-aware verification helper in common/verification.go alongside
VerifyCodeWithKey, then update VerifyEmailLoginCode to call it with req.Email,
req.Code, common.EmailLoginPurpose, and common.EmailLoginCodeTTL while
preserving the existing constant-time comparison and invalid-code behavior.

router/api-router.go-16-21 (1)

16-21: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Applying CORS() to the whole /api group puts Access-Control-Allow-Origin: * on the entire authenticated surface.

middleware.CORS() falls back to Access-Control-Allow-Origin: * for any origin outside trustedCredentialedOrigins, so admin, /api/option (RootAuth), /api/channel, and /api/token responses now all become cross-origin readable. Cookie auth is still protected (browsers reject credentials with *), but the access_token this PR starts returning is an Authorization-header credential, which * does not restrain — any page can script authenticated calls once it obtains one.

The stated need is only /api/user/login/*, /api/token/*, /api/user/self. Either scope the middleware to those groups, or drop the * fallback so only allowlisted origins get a CORS header. Note the OPTIONS short-circuit also now precedes GlobalAPIRateLimit() for every /api path.

🤖 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 `@router/api-router.go` around lines 16 - 21, Restrict CORS application in the
apiRouter setup to the required authenticated endpoints—/api/user/login/*,
/api/token/*, and /api/user/self—instead of applying middleware.CORS() to the
entire /api group. Preserve preflight handling for those routes while ensuring
admin, /api/option, and /api/channel remain without the wildcard CORS fallback
and continue through GlobalAPIRateLimit().

common/verification.go-87-108 (1)

87-108: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Unbounded in-memory email-login state. Both maps are keyed by an attacker-supplied email and have no expiry sweep, so abandoned sign-in attempts accumulate for the process lifetime. verificationMap already prunes via removeExpiredPairs; these two do not.

• common/verification.go#L87-L108: store a timestamp with each attempt count and prune entries older than the code lifetime when the map exceeds a size threshold, mirroring removeExpiredPairs.
• controller/user.go#L171-L174: emailLoginLastSent is only deleted on successful verification — prune entries older than EmailLoginCodeResendCooldown on write, or replace it with a TTL cache.

🤖 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 `@common/verification.go` around lines 87 - 108, The verification attempt and
resend-tracking maps retain attacker-supplied email entries indefinitely. In
common/verification.go lines 87-108, update IncrementAttemptsAndMaybeInvalidate
and ResetAttempts to store attempt timestamps and, once the map exceeds a size
threshold, prune entries older than the verification-code lifetime, mirroring
removeExpiredPairs. In controller/user.go lines 171-174, prune
emailLoginLastSent entries older than EmailLoginCodeResendCooldown during
writes, or replace the map with an equivalent TTL cache.

controller/user.go-278-302 (1)

278-302: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Concurrent verifications with the same code can create duplicate users.

VerifyCodeWithKey (Line 264) and DeleteKey (Line 274) are separate critical sections, so two in-flight requests carrying the same valid code both pass, both miss the lookup, and both Insert. User.Email is declared gorm:"index" — not unique — so nothing stops two rows sharing the address, after which every First(&user) by email becomes nondeterministic. Use a single find-or-create guarded by a transaction (or add a unique constraint on email and treat the duplicate-key error as "already created, re-select").

🤖 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 `@controller/user.go` around lines 278 - 302, Make the verified-email user
creation path around VerifyCodeWithKey, DeleteKey, and the post-insert lookup
concurrency-safe: use one transaction for the existence check and creation, or
enforce a unique email constraint and handle duplicate-key errors by
re-selecting the existing user. Ensure concurrent requests with the same code
cannot leave duplicate User rows and preserve the existing response/error
behavior.

controller/user.go-118-125 (1)

118-125: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Email case handling is inconsistent between V0 and V1, and case-sensitivity differs per database.

LoginByEmail only trims, while RequestEmailLoginCode/VerifyEmailLoginCode lowercase (Lines 190, 257). On PostgreSQL and SQLite (binary collation) WHERE email = ? is case-sensitive, so Foo@x.com via V0 and foo@x.com via V1 resolve to two different accounts; on MySQL's default _ci collation they resolve to one. Normalize identically in both handlers.

🐛 Proposed fix

-	req.Email = strings.TrimSpace(req.Email)
+	req.Email = strings.TrimSpace(strings.ToLower(req.Email))

As per coding guidelines, "All database code must remain compatible with SQLite, MySQL >= 5.7.8, and PostgreSQL >= 9.6."

🤖 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 `@controller/user.go` around lines 118 - 125, Normalize the email consistently
in LoginByEmail by trimming whitespace and converting it to lowercase before
validation and the model.DB lookup, matching the normalization already used by
RequestEmailLoginCode and VerifyEmailLoginCode. Preserve the existing
invalid-parameter handling and query behavior after normalization.

Source: Coding guidelines

controller/user.go-234-238 (1)

234-238: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

Blocking SMTP call on the request thread, and the email address is written to logs.

Two problems here:

1. common.SendEmail is synchronous and net/smtp/tls.Dial are used without any deadline, so a hung SMTP endpoint pins this handler (and the client) until the OS TCP timeout. The comment says "fire-and-forget" but the call is not. The PR summary also describes this as asynchronous delivery.
2. The failure log embeds the full address; the same pattern appears on Lines 140 and 299. Logging user email identifiers is a privacy/compliance exposure — mask or log the user id instead.

🔒 Proposed fix

-	if err := common.SendEmail(subject, req.Email, content); err != nil {
-		common.SysLog(fmt.Sprintf("RequestEmailLoginCode SendEmail failed for %s: %v", req.Email, err))
-	}
+	go func(to, subject, content string) {
+		if err := common.SendEmail(subject, to, content); err != nil {
+			common.SysLog(fmt.Sprintf("RequestEmailLoginCode SendEmail failed for %s: %v", maskEmail(to), err))
+		}
+	}(req.Email, subject, content)

where maskEmail keeps only the first character and the domain. A dial/read deadline in common.SendEmail is still worth adding regardless.

🤖 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 `@controller/user.go` around lines 234 - 238, Make the email delivery path in
the request handler genuinely fire-and-forget by moving common.SendEmail out of
the request thread while preserving failure logging. Replace full email
addresses in the failure logs at the shown location and the corresponding
logging sites near lines 140 and 299 with maskEmail output or a user identifier.
Add appropriate dial/read deadlines inside common.SendEmail so SMTP operations
cannot hang indefinitely.

common/constants.go-94-97 (1)

94-97: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Defaulting the email-login master switch to true opens passwordless account creation on every existing deployment.

EmailOnlyLoginEnabled = true means any instance that upgrades immediately exposes /api/user/login/email/request-code and /verify-code, which auto-create accounts (with QuotaForNewUser credits) for any address that can receive mail. Every other auth provider in this codebase defaults off (GitHubOAuthEnabled, TelegramOAuthEnabled, …). Recommend defaulting to false and enabling it explicitly for the JINN deployment.

🔒 Proposed change

-var EmailOnlyLoginEnabled = true
+var EmailOnlyLoginEnabled = false

🤖 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 `@common/constants.go` around lines 94 - 97, Change the default value of
EmailOnlyLoginEnabled to false so existing deployments do not enable email-based
account creation on upgrade. Preserve explicit configuration support for
enabling it in the JINN deployment, consistent with the other authentication
provider switches.

🟡 Minor comments (6)

service/subscription_recurring_test.go-17-20 (1)

17-20: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Restore the global registry with t.Cleanup.

These tests permanently replace recurringChargers; a fatal assertion or later test can observe an empty registry and lose pre-registered chargers. Snapshot it before setup and restore it via t.Cleanup.

Proposed fix

+func isolateRecurringChargers(t *testing.T) {
+	t.Helper()
+	recurringChargersMu.Lock()
+	previous := recurringChargers
+	recurringChargers = map[string]RecurringCharger{}
+	recurringChargersMu.Unlock()
+
+	t.Cleanup(func() {
+		recurringChargersMu.Lock()
+		recurringChargers = previous
+		recurringChargersMu.Unlock()
+	})
+}
+
 func TestChargeDueAgreementSubscriptionsNoopWhenEmpty(t *testing.T) {
-	recurringChargersMu.Lock()
-	recurringChargers = map[string]RecurringCharger{}
-	recurringChargersMu.Unlock()
+	isolateRecurringChargers(t)
 	ChargeDueAgreementSubscriptions() // must not panic — the HK/Airwallex steady state
 }

Also applies to: 24-27, 46-48

🤖 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 `@service/subscription_recurring_test.go` around lines 17 - 20, Update
TestChargeDueAgreementSubscriptionsNoopWhenEmpty and the other affected test
setup blocks to snapshot the existing recurringChargers registry before
replacing it, then register a t.Cleanup callback that restores the snapshot
under recurringChargersMu; keep the test’s empty-registry setup unchanged during
execution.

DEPLOY.md-23-23 (1)

23-23: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Specify a language for the directory-tree fence.

Use text here to satisfy Markdown linting and improve renderer behavior.

Proposed fix

-```
+```text

🤖 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 `@DEPLOY.md` at line 23, Specify the text language on the directory-tree fenced
code block in DEPLOY.md by changing its opening fence to use text, while
preserving the existing block contents.

Source: Linters/SAST tools

model/subscription_billing_customer_test.go-6-10 (1)

6-10: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Reset the test’s persisted state.

AutoMigrate does not clear an existing mapping for user 4242, so test ordering or repeated execution can fail the initial-empty assertion. Initialize the table state before querying it.

As per coding guidelines, backend tests must initialize database state explicitly.

🤖 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 `@model/subscription_billing_customer_test.go` around lines 6 - 10, Reset the
persisted AirwallexBillingCustomer state before the initial lookup in this test:
after AutoMigrate and before GetAirwallexBillingCustomerId(4242), explicitly
clear or initialize the table using the test database setup mechanism. Preserve
the assertion that user 4242 returns an empty ID.

Source: Coding guidelines

service/airwallex/client_test.go-64-74 (1)

64-74: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Manual state restore is skipped on assertion failure, leaking global config into later tests.

t.Fatalf at line 71 exits before line 73 restores setting.AirwallexApiKey, so a failure here corrupts AirwallexApiKey for every test run afterward, obscuring the real failure.

🧹 Proposed fix: restore via t.Cleanup

 func TestLoginFailureSurfaced(t *testing.T) {
 	var logins int32
 	mockServer(t, &logins, func(w http.ResponseWriter, r *http.Request) {})
 	setting.AirwallexApiKey = "wrong"
+	t.Cleanup(func() { setting.AirwallexApiKey = "key" })
 	ResetTokenCache()
 	_, err := CreateCustomer("req-1", "7", "")
 	if err == nil || !strings.Contains(err.Error(), "login failed") {
 		t.Fatalf("expected login failure, got %v", err)
 	}
-	setting.AirwallexApiKey = "key"
 }

🤖 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 `@service/airwallex/client_test.go` around lines 64 - 74, Update
TestLoginFailureSurfaced to register restoration of setting.AirwallexApiKey
through t.Cleanup immediately after changing it to "wrong", and remove the
manual reset at the end so cleanup runs even when the assertion calls t.Fatalf.

common/constants.go-99-103 (1)

99-103: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Doc comment contradicts the value.

The comment says "Default true preserves V0 behaviour during the V1 rollout", but the variable is false. Either fix the comment or the default.

📝 Proposed fix

-// Default true preserves V0 behaviour during the V1 rollout. Flip to false
-// once all desktop clients ship with V1 code-entry support.
+// Default false: V0 is opt-in only. Operators enable it temporarily for
+// clients that have not yet shipped V1 code-entry support.
 var EmailOnlyLoginAutoCreateNoVerify = false

🤖 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 `@common/constants.go` around lines 99 - 103, Resolve the contradiction between
the doc comment and EmailOnlyLoginAutoCreateNoVerify by making the documented
default match the intended rollout behavior: either set the variable to true to
preserve the stated V0 default, or revise the comment to accurately describe a
false default and its implications.

common/verification.go-43-56 (1)

43-56: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

Modulo bias, and the fallback emits non-numeric codes.

digits[int(b[i])%10] is biased (256 % 10 ≠ 0), so digits 0–5 are ~4% more likely than 6–9 — a small but avoidable entropy loss on a 6-digit sign-in OTP. Also, the rand.Read fallback returns a UUID-derived hex string, which violates the "digit-only" contract the client's OTP input relies on; since crypto/rand.Read never returns a partial read without error, failing closed is cleaner.

🔒 Proposed fix using rejection-free `rand.Int`

 func GenerateNumericVerificationCode(length int) string {
-	const digits = "0123456789"
-	b := make([]byte, length)
-	if _, err := rand.Read(b); err != nil {
-		return GenerateVerificationCode(length)
-	}
-	for i := range b {
-		b[i] = digits[int(b[i])%10]
-	}
-	return string(b)
+	const digits = "0123456789"
+	b := make([]byte, length)
+	for i := range b {
+		n, err := rand.Int(rand.Reader, big.NewInt(10))
+		if err != nil {
+			return ""
+		}
+		b[i] = digits[n.Int64()]
+	}
+	return string(b)
 }

(requires math/big; callers must treat "" as a generation failure)

🤖 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 `@common/verification.go` around lines 43 - 56, Update
GenerateNumericVerificationCode to eliminate modulo bias by generating each
digit with crypto/rand.Int over a bound of 10, adding the required math/big
dependency. On any randomness failure, return an empty string instead of calling
GenerateVerificationCode, while preserving the digit-only output contract for
successful generation.

🧹 Nitpick comments (6)

controller/subscription_payment_airwallex_billing_test.go (1)

371-377: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Redundant conversion: n is already int64.

♻️

-	return int64(n)
+	return n

🤖 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 `@controller/subscription_payment_airwallex_billing_test.go` around lines 371 -
377, Remove the redundant int64 conversion in countRenewalOrders and return the
already-typed n value directly.

controller/subscription_payment_airwallex.go (2)

760-765: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Unused raw parameter.

handleAirwallexIntentSucceeded never reads raw; the payload is rebuilt from event.Data.Object. Drop the parameter and the argument at Line 487.

🤖 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 `@controller/subscription_payment_airwallex.go` around lines 760 - 765, Remove
the unused raw parameter from handleAirwallexIntentSucceeded and update its call
site at the referenced handler to stop passing the raw payload argument,
preserving the existing event-based processing.

---

281-295: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value

Partial cancellation is reported as a total failure.

If subscription N fails after N-1 were cancelled, the user sees "取消订阅失败，请稍后重试" and a retry will re-cancel already-cancelled subscriptions. Consider continuing the loop, tracking failures, and reporting the counts.

🤖 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 `@controller/subscription_payment_airwallex.go` around lines 281 - 295, Update
the subscription cancellation loop to continue processing remaining
subscriptions when Airwallex.CancelBillingSubscription fails instead of
returning immediately. Track cancelled and failed counts, preserve error logging
for each failure, and return a response that reports both counts so retries do
not appear to be an all-or-nothing operation.

controller/subscription_payment_airwallex_planswitch_test.go (1)

41-227: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

New Go tests should use testify/require for fatal assertions instead of if err != nil { t.Fatal(...) }. Both new test files mix the two styles; the guideline requires require for setup/fatal assertions and assert for non-fatal checks.

• controller/subscription_payment_airwallex_planswitch_test.go#L41-L227: replace the if err := ...; err != nil { t.Fatal(err) } blocks and the if got != want { t.Fatalf(...) } assertions with require.NoError / require.Equal.
• controller/subscription_payment_airwallex_billing_test.go#L141-L152: convert loadEvent's two t.Fatal(err) blocks to require.NoError(t, err).

As per coding guidelines, "New or substantially rewritten Go backend tests must use testify/require for setup and fatal assertions and testify/assert for non-fatal checks".

🤖 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 `@controller/subscription_payment_airwallex_planswitch_test.go` around lines 41
- 227, Update controller/subscription_payment_airwallex_planswitch_test.go lines
41-227 to use testify/require: replace fatal setup error checks with
require.NoError and fatal equality assertions with require.Equal, preserving
existing messages where needed; use assert only for non-fatal checks. Also
update loadEvent in controller/subscription_payment_airwallex_billing_test.go
lines 141-152 by replacing both t.Fatal(err) error checks with
require.NoError(t, err).

Source: Coding guidelines

service/airwallex/client.go (1)

42-131: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

No context.Context propagation for outbound Airwallex calls.

getToken() and do() use http.NewRequest rather than http.NewRequestWithContext, so callers (e.g. the webhook handler, which has its own request context) can't bound or cancel in-flight Airwallex calls beyond the shared package-level HTTPClient 30s timeout.

🤖 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 `@service/airwallex/client.go` around lines 42 - 131, The Airwallex request
helpers do not propagate caller cancellation or deadlines. Update getToken and
do to accept a context.Context and create their outbound requests with
http.NewRequestWithContext; pass the caller’s context through do to getToken and
update all call sites to provide the appropriate request context, preserving
existing authentication and response handling.

i18n/keys.go (1)

82-82: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Use or remove the new email login expiry key.

MsgEmailLoginCodeExpired is not referenced anywhere, while VerifyEmailLoginCode still maps expired codes back to the same “invalid” response as wrong codes. Either return this key for expired codes or remove the key and its translations.

🤖 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 `@i18n/keys.go` at line 82, Update VerifyEmailLoginCode to return
MsgEmailLoginCodeExpired when the login code is expired, preserving the existing
invalid response for incorrect codes; alternatively, remove the unused
MsgEmailLoginCodeExpired constant and all corresponding translations if expiry
should remain mapped to invalid.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 4640fb22-dfe0-427f-9b09-38707d502069

📥 Commits

Reviewing files that changed from the base of the PR and between 66ee6b8 and a1eb367.

⛔ Files ignored due to path filters (1)

• web/default/bun.lock is excluded by !**/*.lock

📒 Files selected for processing (38)

• CLAUDE.md
• DEPLOY.md
• Dockerfile
• common/constants.go
• common/email.go
• common/verification.go
• controller/subscription.go
• controller/subscription_payment_airwallex.go
• controller/subscription_payment_airwallex_billing_test.go
• controller/subscription_payment_airwallex_changeplan_test.go
• controller/subscription_payment_airwallex_planswitch_test.go
• controller/subscription_payment_airwallex_test.go
• controller/testdata/awx_billing_checkout_completed.json
• controller/testdata/awx_billing_checkout_completed_slim.json
• controller/testdata/awx_invoice_paid.json
• controller/user.go
• i18n/keys.go
• i18n/locales/en.yaml
• i18n/locales/zh-CN.yaml
• i18n/locales/zh-TW.yaml
• middleware/cors.go
• model/main.go
• model/option.go
• model/subscription.go
• model/subscription_billing_customer_test.go
• model/subscription_plan_by_price_test.go
• model/subscription_supersede_test.go
• model/topup.go
• model/user.go
• router/api-router.go
• service/airwallex/billing.go
• service/airwallex/billing_test.go
• service/airwallex/client.go
• service/airwallex/client_test.go
• service/subscription_recurring.go
• service/subscription_recurring_test.go
• service/subscription_reset_task.go
• setting/payment_airwallex.go

[coderabbitai]

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 (2)

middleware/cors.go (2)

34-59: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Set Vary: Origin for every origin-bearing response.

The response differs by Origin: trusted origins receive a specific origin and credentials, while others receive *. Setting Vary only in the trusted branch can let a shared cache reuse a wildcard response for a credentialed request, breaking browser API calls.

Proposed fix

 if origin != "" {
+	c.Header("Vary", "Origin")
 	if trustedCredentialedOrigins[origin] {
 		c.Header("Access-Control-Allow-Origin", origin)
 		c.Header("Access-Control-Allow-Credentials", "true")
-		c.Header("Vary", "Origin")
 	} else {

🤖 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 `@middleware/cors.go` around lines 34 - 59, Update CORS so every response with
a non-empty Origin request header sets Vary: Origin, not only responses matching
trustedCredentialedOrigins. Keep the existing trusted-origin and wildcard
Access-Control-Allow-Origin behavior unchanged.

---

55-57: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Abort only actual CORS preflight requests.

This currently intercepts every OPTIONS request, even without Origin or Access-Control-Request-Method, preventing any legitimate API OPTIONS handler from running.

Proposed fix

-if c.Request.Method == http.MethodOptions {
+if c.Request.Method == http.MethodOptions &&
+	origin != "" &&
+	c.Request.Header.Get("Access-Control-Request-Method") != "" {
 	c.AbortWithStatus(http.StatusNoContent)
 	return
 }

🤖 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 `@middleware/cors.go` around lines 55 - 57, Update the OPTIONS handling in the
CORS middleware to abort only when the request includes both CORS preflight
indicators: an Origin header and an Access-Control-Request-Method header. Allow
other OPTIONS requests to continue to the downstream handler.

🧹 Nitpick comments (1)

middleware/cors_test.go (1)

11-34: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add tests for the actual CORS() middleware behavior.

These tests only exercise the allowlist helper. Add httptest/Gin cases covering trusted credentialed responses, wildcard responses, Vary: Origin, allowed headers, and OPTIONS preflight handling; otherwise the main changed behavior in middleware/cors.go remains unprotected.

🤖 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 `@middleware/cors_test.go` around lines 11 - 34, Extend the CORS tests beyond
TrustedBrowserOrigin by adding Gin/httptest coverage for the CORS() middleware.
Verify trusted origins receive credentialed responses, untrusted origins receive
wildcard responses, responses include Vary: Origin and the configured allowed
headers, and OPTIONS requests are handled correctly as preflight requests.

Source: Coding guidelines

🤖 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 `@middleware/cors_test.go`:
- Line 3: Update the assertions in the tests in middleware/cors_test.go to use
testify/assert instead of t.Errorf, including the assertions at the referenced
import and test lines; add the required testify/assert import and preserve the
existing non-fatal assertion behavior.

---

Outside diff comments:
In `@middleware/cors.go`:
- Around line 34-59: Update CORS so every response with a non-empty Origin
request header sets Vary: Origin, not only responses matching
trustedCredentialedOrigins. Keep the existing trusted-origin and wildcard
Access-Control-Allow-Origin behavior unchanged.
- Around line 55-57: Update the OPTIONS handling in the CORS middleware to abort
only when the request includes both CORS preflight indicators: an Origin header
and an Access-Control-Request-Method header. Allow other OPTIONS requests to
continue to the downstream handler.

---

Nitpick comments:
In `@middleware/cors_test.go`:
- Around line 11-34: Extend the CORS tests beyond TrustedBrowserOrigin by adding
Gin/httptest coverage for the CORS() middleware. Verify trusted origins receive
credentialed responses, untrusted origins receive wildcard responses, responses
include Vary: Origin and the configured allowed headers, and OPTIONS requests
are handled correctly as preflight requests.

🪄 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 Plus

Run ID: 18036ee6-0ff7-44d4-820e-e3a6a8e33656

📥 Commits

Reviewing files that changed from the base of the PR and between a1eb367 and 0715d0e.

📒 Files selected for processing (2)

• middleware/cors.go
• middleware/cors_test.go

[coderabbitai]

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Use testify/assert for these non-fatal assertions.

The repository guideline requires new Go backend tests to use testify/assert or testify/require, rather than t.Errorf.

Proposed fix

 import "testing"
+import "github.com/stretchr/testify/assert"

-		if !TrustedBrowserOrigin(origin) {
-			t.Errorf("origin %s must be trusted; payment return URLs from it are silently discarded otherwise", origin)
-		}
+		assert.True(t, TrustedBrowserOrigin(origin), "origin %s must be trusted", origin)

-		if TrustedBrowserOrigin(origin) {
-			t.Errorf("origin %q must NOT be trusted", origin)
-		}
+		assert.False(t, TrustedBrowserOrigin(origin), "origin %q must NOT be trusted", origin)

Also applies to: 16-17, 30-31

🤖 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 `@middleware/cors_test.go` at line 3, Update the assertions in the tests in
middleware/cors_test.go to use testify/assert instead of t.Errorf, including the
assertions at the referenced import and test lines; add the required
testify/assert import and preserve the existing non-fatal assertion behavior.

Source: Coding guidelines

[coderabbitai]

Actionable comments posted: 17

🧹 Nitpick comments (3)

service/subscription_pause.go (1)

41-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Replace the substring match with a sentinel error.

causeIsMissingSubscription depends on the exact text "no active subscription" produced by the model layer. If that text is reworded, or if the error is wrapped with a different message, the check returns false and the user sees the "your plan is paused, it refills on X" copy instead of "you have no subscription". Nothing fails loudly when that happens.

Export a sentinel from the model package and compare with errors.Is. The comment already identifies this as the intended direction.

♻️ Proposed refactor

In the model package, where the error is produced:

// ErrNoActiveSubscription reports that the user has no active subscription row.
var ErrNoActiveSubscription = errors.New("no active subscription")

Then:

 func causeIsMissingSubscription(cause error) bool {
-	return cause != nil && strings.Contains(cause.Error(), "no active subscription")
+	return errors.Is(cause, model.ErrNoActiveSubscription)
 }

🤖 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 `@service/subscription_pause.go` around lines 41 - 46, Replace the string-based
check in causeIsMissingSubscription with errors.Is against an exported
model-package sentinel named ErrNoActiveSubscription. Define and return or wrap
this sentinel where the model reports the missing subscription, preserving
detection through wrapped errors and removing the direct error-text dependency.

model/usage_counter.go (1)

34-40: 🗄️ Data Integrity & Integration | 🔵 Trivial

Plan pruning for user_image_uploads.

The table stores one row per distinct image per user per cycle and nothing deletes past cycles. On a vision-heavy account the row count grows without an upper bound, and the unique index grows with it. Add a retention job that deletes rows with cycle_start older than the current cycle, or document the retention owner.

The stored image_hash is a SHA-256 of the image URL. For data: URLs the hash is derived from user content, so retention also has a privacy dimension.

🤖 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 `@model/usage_counter.go` around lines 34 - 40, Add a retention job for
UserImageUpload that deletes records whose cycle_start is older than the current
cycle, and integrate it with the existing scheduled cleanup mechanism. Ensure
the cleanup covers both table rows and the associated unique index while
preserving current-cycle records.

service/text_quota.go (1)

312-330: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Move the accrual off the synchronous settlement path.

accrueShadowUsage performs one SELECT through CycleSubscriptionFor and one write transaction through model.AddUsage. Both run inline in PostTextConsumeQuota, so every settled request now pays two extra database round trips before the handler returns. The accrual is advisory and already fails open on error, so it does not need to block the response. PostTextConsumeQuota already dispatches perfmetrics.RecordRelaySample through gopool.Go; use the same pattern here.

Copy the needed scalars before the goroutine starts, so the goroutine does not read relayInfo after the request completes.

♻️ Proposed refactor

 func accrueShadowUsage(relayInfo *relaycommon.RelayInfo, usage *dto.Usage) {
 	shadow := ShadowCost(
 		usage.PromptTokens,
 		usage.CompletionTokens,
 		usage.PromptTokensDetails.CachedTokens,
 		relayInfo.PriceData.ModelRatio,
 		relayInfo.PriceData.CompletionRatio,
 		relayInfo.PriceData.CacheRatio,
 	)
-	cycleStart, _ := UsageCycle(CycleMonth, CycleSubscriptionFor(relayInfo.UserId), time.Now())
-	if err := model.AddUsage(relayInfo.UserId, CycleMonth, cycleStart, shadow, 1); err != nil {
-		common.SysLog("usage accrual failed: " + err.Error())
-	}
+	userId := relayInfo.UserId
+	gopool.Go(func() {
+		cycleStart, _ := UsageCycle(CycleMonth, CycleSubscriptionFor(userId), time.Now())
+		if err := model.AddUsage(userId, CycleMonth, cycleStart, shadow, 1); err != nil {
+			common.SysLog("usage accrual failed: " + err.Error())
+		}
+	})
 }

The existing tests in service/usage_shadow_accrual_test.go assert the counter immediately after PostTextConsumeQuota returns. If you adopt this change, those tests need a synchronization seam.

🤖 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 `@service/text_quota.go` around lines 312 - 330, Move shadow usage accrual in
PostTextConsumeQuota to an asynchronous gopool.Go callback, matching the
existing perfmetrics.RecordRelaySample pattern, so settlement does not wait for
CycleSubscriptionFor or model.AddUsage. Before launching the goroutine, copy all
required scalar values from relayInfo and usage; have the callback perform the
existing accrueShadowUsage logic without accessing request-owned relayInfo
afterward. Add or reuse a synchronization seam so usage_shadow_accrual_test.go
can wait for completion before asserting the counter.

🤖 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 `@controller/subscription_payment_airwallex.go`:
- Around line 147-159: Add an atomic pending Airwallex order reservation
immediately before genAirwallexSubscriptionLink, so concurrent checkout requests
cannot both proceed after HasRenewingUserSubscription. Reject or reuse an
unexpired reservation for the user, and release or expire the reservation if
link creation fails; preserve the existing completed-subscription checks and
error responses.

In `@controller/usage.go`:
- Around line 111-119: Update percentUsed to avoid multiplying used by 100
before division, using overflow-safe arithmetic that preserves nearest-integer
rounding and caps valid results at 100. Add a regression test covering counters
near math.MaxInt64 and verify the result remains correct.

In `@middleware/model-rate-limit.go`:
- Around line 202-219: Update recordRedisRequest to store unambiguous UTC
timestamps, and update both rateLimitWindowStart and checkRedisRateLimit to read
the new format while retaining parsing support for existing local-clock entries.
Ensure DST fall-back entries are ordered correctly and do not rely on the
negative-age clamp to determine the window start.

In `@model/subscription_cancel_test.go`:
- Around line 9-188: Replace direct t.Fatal/t.Fatalf setup and fatal assertions
with testify/require, and replace non-fatal t.Errorf checks with testify/assert
throughout model/subscription_cancel_test.go lines 9-188 and
service/subscription_reconcile_test.go lines 52-149. Add or reuse the testify
assertion imports while preserving each test’s existing expectations and control
flow.

In `@model/subscription.go`:
- Around line 913-916: Update runSubscriptionQuotaResetOnce and its subscription
query so reconciliation progresses beyond the first fixed batch, using a durable
cursor or another database-compatible paging strategy that eventually visits
every eligible subscription. Preserve the existing eligibility filters and
ordering, and add a regression test with more than 200 candidates verifying
later subscriptions are reconciled.
- Around line 875-877: Track cancellations by Airwallex subscription ID instead
of user ID: in model/subscription.go:875-877, persist the provider ID and update
only its mapped local row; in
controller/subscription_payment_airwallex.go:296-324 and :568-582, pass each
cancelled sub.Id, including the webhook ID after fallback fetching; in
service/subscription_reconcile.go:61-80, compare mapped local IDs against active
Airwallex IDs before cancelling; in model/subscription_cancel_test.go:9-188, add
coverage for an old cancellation arriving after a replacement subscription
starts.
- Around line 887-895: Update model/subscription.go lines 887-895 in
GetUserIdByAirwallexBillingCustomerId to return (int, error), preserving (0,
nil) only for empty or unknown customer IDs and propagating database lookup
errors. Update controller/subscription_payment_airwallex.go lines 577-580 to
handle the returned error and return it from the webhook handler so failed
lookups produce a retryable failure; adjust callers for the new signature.
- Around line 940-943: Update HasRenewingUserSubscription to restrict the
active-subscription query to order-backed subscriptions, excluding subscriptions
with Source "admin" while preserving the existing renewal conditions. Extend
TestHasRenewingUserSubscription with an admin-granted subscription case that
verifies it does not block checkout.

In `@model/task_cas_test.go`:
- Around line 49-51: The test cleanup list used by truncateTables must include
the migrated user_usage_counters and user_image_uploads tables. Update
truncateTables to delete both UserUsageCounter and UserImageUpload records,
preserving explicit fixture initialization so test state cannot leak between
tests.

In `@model/usage_counter.go`:
- Around line 100-121: Ensure the MySQL SQL_DSN does not include
clientFoundRows=true, preserving RowsAffected semantics so the fresh count from
unique-hash insertion reflects only newly inserted rows. Keep the existing
Create call and conditional images_used update unchanged.

In `@service/billing_session.go`:
- Around line 193-201: Add rollback for images reserved by ReserveImages when
either PreConsumeTokenQuota or s.funding.PreConsume fails after the reservation
commits. Update the relevant failure paths in the billing session flow to
release the reservation before returning the error, while preserving existing
error responses and successful consumption behavior.

In `@service/task_billing_test.go`:
- Around line 71-72: Update the test database cleanup near the existing
user_usage_counters and user_image_uploads deletions to also delete rows from
the AirwallexBillingCustomer table, using its actual table name
airwallex_billing_customers. Ensure billing-customer mappings are cleared
between tests.

In `@service/usage_cost_test.go`:
- Around line 31-42: Add a regression assertion to
TestShadowCostClampsNegativeAndOversizedCachedCounts for a negative prompt
count, then update ShadowCost to clamp promptTokens to zero before calculating
fresh, preserving the existing cachedTokens and completionTokens clamping
behavior.

In `@service/usage_cost.go`:
- Around line 11-29: Update ShadowCost to clamp promptTokens to zero before
reconciling cachedTokens, preventing negative fresh or cached token amounts and
ensuring the returned charge cannot be negative. Validate modelRatio,
completionRatio, and cacheRatio with math.IsNaN/math.IsInf, returning zero when
any ratio is non-finite before calling decimal.NewFromFloat; add the math
import.

In `@service/usage_images_test.go`:
- Around line 21-32: Update imageMessageWithPointerType and its test usage to
construct the distinct alternate image-content representation produced by
dto.Message.ParseContent, rather than another pointer-based dto.MediaContent
shape. Preserve imageMessage for the existing representation and ensure the test
at lines 83-90 exercises cross-representation deduplication; remove the helper
only if that distinct representation cannot be constructed.

In `@setting/usage_limit_test.go`:
- Around line 9-57: Update the monthly-limit tests around
UpdateMonthlyCostLimitGroupByJSONString and
UpdateMonthlyImageLimitGroupByJSONString to capture each setting’s existing JSON
configuration before mutation, then restore that exact value in t.Cleanup
instead of resetting to "{}". Initialize the relevant settings explicitly in
each test fixture while preserving the existing assertions.

In `@setting/usage_limit.go`:
- Around line 30-35: Update setting/usage_limit.go:30-35 in
UpdateMonthlyCostLimitGroupByJSONString and setting/usage_limit.go:66-71 in the
corresponding image-limit update function to decode into temporary maps, run the
appropriate non-negative validation helper, and replace MonthlyCostLimitGroup or
MonthlyImageLimitGroup only after decoding and validation succeed; preserve the
existing live map when either step fails.

---

Nitpick comments:
In `@model/usage_counter.go`:
- Around line 34-40: Add a retention job for UserImageUpload that deletes
records whose cycle_start is older than the current cycle, and integrate it with
the existing scheduled cleanup mechanism. Ensure the cleanup covers both table
rows and the associated unique index while preserving current-cycle records.

In `@service/subscription_pause.go`:
- Around line 41-46: Replace the string-based check in
causeIsMissingSubscription with errors.Is against an exported model-package
sentinel named ErrNoActiveSubscription. Define and return or wrap this sentinel
where the model reports the missing subscription, preserving detection through
wrapped errors and removing the direct error-text dependency.

In `@service/text_quota.go`:
- Around line 312-330: Move shadow usage accrual in PostTextConsumeQuota to an
asynchronous gopool.Go callback, matching the existing
perfmetrics.RecordRelaySample pattern, so settlement does not wait for
CycleSubscriptionFor or model.AddUsage. Before launching the goroutine, copy all
required scalar values from relayInfo and usage; have the callback perform the
existing accrueShadowUsage logic without accessing request-owned relayInfo
afterward. Add or reuse a synchronization seam so usage_shadow_accrual_test.go
can wait for completion before asserting the counter.

🪄 Autofix

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 Plus

Run ID: 1f26e7cd-90c9-42a2-9faf-90c12d230744

📥 Commits

Reviewing files that changed from the base of the PR and between a8dc14b and 1aab1dd.

📒 Files selected for processing (39)

• controller/option.go
• controller/subscription_payment_airwallex.go
• controller/usage.go
• controller/usage_test.go
• i18n/keys.go
• i18n/locales/en.yaml
• i18n/locales/zh-CN.yaml
• i18n/locales/zh-TW.yaml
• middleware/model-rate-limit.go
• middleware/model-rate-limit_test.go
• model/main.go
• model/option.go
• model/subscription.go
• model/subscription_cancel_test.go
• model/task_cas_test.go
• model/usage_counter.go
• model/usage_counter_test.go
• router/api-router.go
• service/billing_session.go
• service/subscription_pause.go
• service/subscription_pause_test.go
• service/subscription_reconcile.go
• service/subscription_reconcile_test.go
• service/subscription_reset_task.go
• service/task_billing_test.go
• service/text_quota.go
• service/usage.go
• service/usage_cost.go
• service/usage_cost_test.go
• service/usage_cycle.go
• service/usage_cycle_test.go
• service/usage_gate.go
• service/usage_gate_test.go
• service/usage_images.go
• service/usage_images_test.go
• service/usage_shadow_accrual_test.go
• service/usage_test.go
• setting/usage_limit.go
• setting/usage_limit_test.go

🚧 Files skipped from review as they are similar to previous changes (3)

• model/main.go
• model/option.go
• router/api-router.go

[coderabbitai]

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Reserve checkout creation before calling Airwallex.

HasRenewingUserSubscription only sees completed local subscriptions. Two requests can both pass this check before either hosted checkout is paid. Each request can then create a separate Airwallex subscription checkout.

Create and atomically claim one pending Airwallex order before genAirwallexSubscriptionLink. Reject or reuse an unexpired pending order. Release or expire the reservation when link creation fails.

🤖 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 `@controller/subscription_payment_airwallex.go` around lines 147 - 159, Add an
atomic pending Airwallex order reservation immediately before
genAirwallexSubscriptionLink, so concurrent checkout requests cannot both
proceed after HasRenewingUserSubscription. Reject or reuse an unexpired
reservation for the user, and release or expire the reservation if link creation
fails; preserve the existing completed-subscription checks and error responses.

[coderabbitai]

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Use overflow-safe percentage arithmetic.

used*100 can overflow before division. For large valid counters, this can return a negative or incorrect percent instead of the required capped value.

Calculate the rounded fraction with overflow-safe arithmetic. Add a regression case near math.MaxInt64.

🤖 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 `@controller/usage.go` around lines 111 - 119, Update percentUsed to avoid
multiplying used by 100 before division, using overflow-safe arithmetic that
preserves nearest-integer rounding and caps valid results at 100. Add a
regression test covering counters near math.MaxInt64 and verify the result
remains correct.

[coderabbitai]

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n -C 8 'timeFormat|recordRedisRequest|rateLimitWindowStart|time\.Now\(\)\.Format' \
  middleware/model-rate-limit.go middleware/model-rate-limit_test.go

Repository: QuantumNous/new-api

Length of output: 11190

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- time format and imports ---'
sed -n '1,85p' middleware/model-rate-limit.go
printf '%s\n' '--- callers and time-zone configuration ---'
rg -n -C 4 'countRateLimitWindow|rateLimitWindowStart|time\.Local|TZ=|time/tzdata|LoadLocation|ModelRequestRateLimit' --glob '!vendor/**' .
printf '%s\n' '--- timezone-sensitive behavior probe ---'
cat >/tmp/rate_limit_dst_probe.go <<'EOF'
package main

import (
	"fmt"
	"os"
	"time"
)

const timeFormat = "2006-01-02T15:04:05Z"

func rateLimitWindowStart(entry string, now time.Time) (int64, bool) {
	oldest, err := time.Parse(timeFormat, entry)
	if err != nil {
		return 0, false
	}
	nowInSameFrame, err := time.Parse(timeFormat, now.Format(timeFormat))
	if err != nil {
		return 0, false
	}
	age := nowInSameFrame.Sub(oldest)
	if age < 0 {
		age = 0
	}
	return now.Add(-age).Unix(), true
}

func main() {
	loc, err := time.LoadLocation("America/New_York")
	if err != nil {
		panic(err)
	}
	// The repeated 01:30 local hour during the 2025 fall-back transition.
	first := time.Date(2025, 11, 2, 1, 30, 0, 0, loc)
	second := first.Add(time.Hour)
	entry := first.Format(timeFormat)
	now := second
	start, ok := rateLimitWindowStart(entry, now)
	fmt.Printf("TZ=%q local=%s entry=%q now=%s actual_age=%s start=%s ok=%v\n",
		os.Getenv("TZ"), first, entry, now, now.Sub(first), time.Unix(start, 0).In(loc), ok)
}
EOF
TZ=America/New_York go run /tmp/rate_limit_dst_probe.go

Repository: QuantumNous/new-api

Length of output: 50377

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- timeFormat definition ---'
rg -n -C 2 'timeFormat\s*=' middleware --glob '*.go'

printf '%s\n' '--- image and runtime timezone defaults ---'
rg -n -C 3 'ENV[[:space:]]+TZ|TZ=|time\.Local|zoneinfo' Dockerfile* docker-compose*.yml .github docs README* 2>/dev/null | head -200 || true

printf '%s\n' '--- standalone DST probe ---'
cat >/tmp/rate_limit_dst_probe.go <<'EOF'
package main

import (
	"fmt"
	"time"
)

const timeFormat = "2006-01-02T15:04:05Z"

func rateLimitWindowStart(entry string, now time.Time) (int64, bool) {
	oldest, err := time.Parse(timeFormat, entry)
	if err != nil {
		return 0, false
	}
	nowInSameFrame, err := time.Parse(timeFormat, now.Format(timeFormat))
	if err != nil {
		return 0, false
	}
	age := nowInSameFrame.Sub(oldest)
	if age < 0 {
		age = 0
	}
	return now.Add(-age).Unix(), true
}

func main() {
	loc, err := time.LoadLocation("America/New_York")
	if err != nil {
		panic(err)
	}
	first := time.Date(2025, 11, 2, 1, 30, 0, 0, loc)
	second := first.Add(time.Hour)
	entry := first.Format(timeFormat)
	start, ok := rateLimitWindowStart(entry, second)
	fmt.Printf("first=%s\nsecond=%s\nentry=%q\nactual_age=%s\nreported_start=%s\nok=%v\n",
		first, second, entry, second.Sub(first), time.Unix(start, 0).In(loc), ok)
}
EOF
go run /tmp/rate_limit_dst_probe.go

Repository: QuantumNous/new-api

Length of output: 8818

---

Store rate-limit entries with unambiguous timestamps.

When time.Local uses daylight-saving time, the repeated fall-back hour can make an entry appear newer than now. The negative-age clamp then reports now as the window start. Update both readers and retain compatibility with existing entries.

🤖 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 `@middleware/model-rate-limit.go` around lines 202 - 219, Update
recordRedisRequest to store unambiguous UTC timestamps, and update both
rateLimitWindowStart and checkRedisRateLimit to read the new format while
retaining parsing support for existing local-clock entries. Ensure DST fall-back
entries are ordered correctly and do not rely on the negative-age clamp to
determine the window start.

[coderabbitai]

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Use testify assertions in the new backend tests.

These new tests use direct t.Fatal, t.Fatalf, and t.Errorf assertions. Convert setup and fatal assertions to require, and non-fatal assertions to assert.

• model/subscription_cancel_test.go#L9-L188: replace direct test assertions with require and assert.
• service/subscription_reconcile_test.go#L52-L149: replace direct test assertions with require and assert.

As per coding guidelines, “New or substantially rewritten tests must use testify/require for setup and fatal assertions and testify/assert for non-fatal checks.”

📍 Affects 2 files

• model/subscription_cancel_test.go#L9-L188 (this comment)
• service/subscription_reconcile_test.go#L52-L149

🤖 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 `@model/subscription_cancel_test.go` around lines 9 - 188, Replace direct
t.Fatal/t.Fatalf setup and fatal assertions with testify/require, and replace
non-fatal t.Errorf checks with testify/assert throughout
model/subscription_cancel_test.go lines 9-188 and
service/subscription_reconcile_test.go lines 52-149. Add or reuse the testify
assertion imports while preserving each test’s existing expectations and control
flow.

Source: Coding guidelines

[coderabbitai]

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Track cancellation by Airwallex subscription ID, not by user ID.

A cancellation for an old subscription currently marks every active local subscription for the user as non-renewing. A delayed webhook can therefore cancel a newer replacement subscription. The reconciliation pass has the same problem.

• model/subscription.go#L875-L877: persist an Airwallex subscription ID on the local subscription and update only the row mapped to that ID.
• controller/subscription_payment_airwallex.go#L296-L324: pass each cancelled Airwallex sub.Id to the local update.
• controller/subscription_payment_airwallex.go#L568-L582: use the webhook subscription ID after the fallback fetch.
• service/subscription_reconcile.go#L61-L80: compare mapped local subscription IDs with the active Airwallex IDs before marking rows cancelled.
• model/subscription_cancel_test.go#L9-L188: add a regression test where an old cancellation arrives after a replacement subscription starts.

📍 Affects 4 files

• model/subscription.go#L875-L877 (this comment)
• controller/subscription_payment_airwallex.go#L296-L324
• controller/subscription_payment_airwallex.go#L568-L582
• service/subscription_reconcile.go#L61-L80
• model/subscription_cancel_test.go#L9-L188

🤖 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 `@model/subscription.go` around lines 875 - 877, Track cancellations by
Airwallex subscription ID instead of user ID: in model/subscription.go:875-877,
persist the provider ID and update only its mapped local row; in
controller/subscription_payment_airwallex.go:296-324 and :568-582, pass each
cancelled sub.Id, including the webhook ID after fallback fetching; in
service/subscription_reconcile.go:61-80, compare mapped local IDs against active
Airwallex IDs before cancelling; in model/subscription_cancel_test.go:9-188, add
coverage for an old cancellation arriving after a replacement subscription
starts.

[coderabbitai]

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Clamp negative prompt token counts.

ShadowCost clamps cachedTokens and completionTokens, but it does not clamp promptTokens. A negative prompt count can create a negative shadow cost and decrement cumulative usage.

Add a regression case for a negative prompt count. Clamp promptTokens to zero before computing fresh.

🤖 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 `@service/usage_cost_test.go` around lines 31 - 42, Add a regression assertion
to TestShadowCostClampsNegativeAndOversizedCachedCounts for a negative prompt
count, then update ShadowCost to clamp promptTokens to zero before calculating
fresh, preserving the existing cachedTokens and completionTokens clamping
behavior.

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Clamp promptTokens and reject non-finite ratios.

Two unvalidated inputs reach the arithmetic.

A negative promptTokens is not clamped. The cachedTokens > promptTokens branch then sets cachedTokens to that negative value, fresh becomes 0, and cached becomes negative. ShadowCost returns a negative value, and model.AddUsage decrements cost_used, which lowers the recorded usage below the truth.

decimal.NewFromFloat panics on NaN and Inf. modelRatio <= 0 is false for NaN, so a NaN ratio reaches decimal.NewFromFloat and panics inside the settlement path.

🛡️ Proposed fix

 func ShadowCost(promptTokens, completionTokens, cachedTokens int, modelRatio, completionRatio, cacheRatio float64) int64 {
-	if modelRatio <= 0 {
+	if !(modelRatio > 0) || math.IsInf(modelRatio, 0) {
 		return 0
 	}
+	if math.IsNaN(completionRatio) || math.IsInf(completionRatio, 0) {
+		completionRatio = 0
+	}
+	if math.IsNaN(cacheRatio) || math.IsInf(cacheRatio, 0) {
+		cacheRatio = 0
+	}
+	if promptTokens < 0 {
+		promptTokens = 0
+	}
 	if cachedTokens < 0 {
 		cachedTokens = 0
 	}

Add "math" to the imports.

As per coding guidelines: "Billing and quota code must never produce a negative charge or credit through overflow or unvalidated input."

📝 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

	func ShadowCost(promptTokens, completionTokens, cachedTokens int, modelRatio, completionRatio, cacheRatio float64) int64 {
	if modelRatio <= 0 {
	return 0
	}
	if cachedTokens < 0 {
	cachedTokens = 0
	}
	if completionTokens < 0 {
	completionTokens = 0
	}
	if cachedTokens > promptTokens {
	cachedTokens = promptTokens
	}
	fresh := decimal.NewFromInt(int64(promptTokens - cachedTokens))
	cached := decimal.NewFromInt(int64(cachedTokens)).Mul(decimal.NewFromFloat(cacheRatio))
	output := decimal.NewFromInt(int64(completionTokens)).Mul(decimal.NewFromFloat(completionRatio))
	weighted := fresh.Add(cached).Add(output)
	return weighted.Mul(decimal.NewFromFloat(modelRatio)).Round(0).IntPart()
	func ShadowCost(promptTokens, completionTokens, cachedTokens int, modelRatio, completionRatio, cacheRatio float64) int64 {
	if !(modelRatio > 0) || math.IsInf(modelRatio, 0) {
	return 0
	}
	if math.IsNaN(completionRatio) || math.IsInf(completionRatio, 0) {
	completionRatio = 0
	}
	if math.IsNaN(cacheRatio) || math.IsInf(cacheRatio, 0) {
	cacheRatio = 0
	}
	if promptTokens < 0 {
	promptTokens = 0
	}
	if cachedTokens < 0 {
	cachedTokens = 0
	}
	if completionTokens < 0 {
	completionTokens = 0
	}
	if cachedTokens > promptTokens {
	cachedTokens = promptTokens
	}
	fresh := decimal.NewFromInt(int64(promptTokens - cachedTokens))
	cached := decimal.NewFromInt(int64(cachedTokens)).Mul(decimal.NewFromFloat(cacheRatio))
	output := decimal.NewFromInt(int64(completionTokens)).Mul(decimal.NewFromFloat(completionRatio))
	weighted := fresh.Add(cached).Add(output)
	return weighted.Mul(decimal.NewFromFloat(modelRatio)).Round(0).IntPart()
	}

🤖 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 `@service/usage_cost.go` around lines 11 - 29, Update ShadowCost to clamp
promptTokens to zero before reconciling cachedTokens, preventing negative fresh
or cached token amounts and ensuring the returned charge cannot be negative.
Validate modelRatio, completionRatio, and cacheRatio with math.IsNaN/math.IsInf,
returning zero when any ratio is non-finite before calling decimal.NewFromFloat;
add the math import.

Source: Coding guidelines

[coderabbitai]

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Test a distinct image-content representation.

imageMessageWithPointerType constructs the same dto.MediaContent shape as imageMessage. The test at lines 83-90 therefore repeats pointer coverage and does not verify cross-representation deduplication.

Construct the alternate representation supported by dto.Message.ParseContent, or remove the duplicate helper and claim.

As per coding guidelines, backend tests must protect API contracts and regression paths.

Also applies to: 83-90

🤖 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 `@service/usage_images_test.go` around lines 21 - 32, Update
imageMessageWithPointerType and its test usage to construct the distinct
alternate image-content representation produced by dto.Message.ParseContent,
rather than another pointer-based dto.MediaContent shape. Preserve imageMessage
for the existing representation and ensure the test at lines 83-90 exercises
cross-representation deduplication; remove the helper only if that distinct
representation cannot be constructed.

Source: Coding guidelines

[coderabbitai]

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Restore the previous monthly-limit configuration.

Each cleanup replaces shared settings with {}. This can change the configuration observed by later tests.

Capture each prior JSON value before mutation. Restore that value in t.Cleanup.

As per coding guidelines, initialize settings explicitly in test fixtures.

🤖 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 `@setting/usage_limit_test.go` around lines 9 - 57, Update the monthly-limit
tests around UpdateMonthlyCostLimitGroupByJSONString and
UpdateMonthlyImageLimitGroupByJSONString to capture each setting’s existing JSON
configuration before mutation, then restore that exact value in t.Cleanup
instead of resetting to "{}". Initialize the relevant settings explicitly in
each test fixture while preserving the existing assertions.

Source: Coding guidelines

[coderabbitai]

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Commit monthly-limit maps only after validation succeeds.

Both update functions clear the live map before common.Unmarshal completes. Invalid or partially decoded JSON can erase valid running configuration. Both functions also bypass their corresponding non-negative validation helper.

• setting/usage_limit.go#L30-L35: Decode and validate into a temporary cost-limit map. Replace MonthlyCostLimitGroup only after success.
• setting/usage_limit.go#L66-L71: Decode and validate into a temporary image-limit map. Replace MonthlyImageLimitGroup only after success.

Proposed update pattern

 func UpdateMonthlyCostLimitGroupByJSONString(jsonStr string) error {
+	next := make(map[string]int64)
+	if err := common.Unmarshal([]byte(jsonStr), &next); err != nil {
+		return err
+	}
+	for group, limit := range next {
+		if limit < 0 {
+			return errors.New("monthly cost limit must be >= 0 for group " + group)
+		}
+	}
 	monthlyLimitMu.Lock()
 	defer monthlyLimitMu.Unlock()
-	MonthlyCostLimitGroup = make(map[string]int64)
-	return common.Unmarshal([]byte(jsonStr), &MonthlyCostLimitGroup)
+	MonthlyCostLimitGroup = next
+	return nil
 }

📍 Affects 1 file

• setting/usage_limit.go#L30-L35 (this comment)
• setting/usage_limit.go#L66-L71

🤖 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 `@setting/usage_limit.go` around lines 30 - 35, Update
setting/usage_limit.go:30-35 in UpdateMonthlyCostLimitGroupByJSONString and
setting/usage_limit.go:66-71 in the corresponding image-limit update function to
decode into temporary maps, run the appropriate non-negative validation helper,
and replace MonthlyCostLimitGroup or MonthlyImageLimitGroup only after decoding
and validation succeed; preserve the existing live map when either step fails.