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 email codes (TTL, resend cooldown, rate limiting, and max-attempt lockout) plus email-login i18n messages.
  • Added “system access tokens” for non-browser authenticated requests.
  • Added Airwallex subscription pay/cancel/change-plan and webhook-driven renewals, plus deferred annual plan switching with deferred/effective_at.
  • Added agreement-based recurring charging support.
• Bug Fixes
  • Prevented plan edits from clearing existing Airwallex billing identifiers.
  • Prevented empty profile fields from overwriting existing values.
  • Improved webhook and email formatting hardening (e.g., signature validation and MIME header).
• Documentation
  • Added a new end-to-end deployment/rollback procedure.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: cafb4b0f-533f-4e1f-9710-5b588a203485

📥 Commits

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

📒 Files selected for processing (4)

• controller/subscription_payment_airwallex.go
• controller/subscription_payment_airwallex_changeplan_test.go
• service/airwallex/billing.go
• service/airwallex/billing_test.go

---

Walkthrough

This PR adds email-based authentication with verification codes and access tokens, introduces Airwallex payment and subscription billing with signed webhook reconciliation, adds recurring-charge scheduling, updates CORS and persistence behavior, and documents deployment procedures.

Changes

Email authentication

Layer / File(s)	Summary
Email login configuration and verification common/*, model/option.go, i18n/*	Adds email-login settings, numeric OTP generation, attempt limits, option parsing, email formatting, and translations.
Email login handlers and 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 API client and billing operations service/airwallex/*, setting/payment_airwallex.go	Adds authenticated customer, checkout, payment-intent, subscription, and deferred price-switching API operations with caching and tests.
Subscription persistence model/subscription.go, model/main.go, model/topup.go	Adds Airwallex price/customer mappings, subscription helpers, payment constants, and migration coverage.
Payment and webhook flows controller/subscription_payment_airwallex.go, router/api-router.go, controller/testdata/*	Adds payment, cancellation, deferred plan-switch, signed webhook, renewal, and reconciliation handlers.
Validation and recurring charging controller/*airwallex*_test.go, service/subscription_recurring*, service/subscription_reset_task.go	Adds coverage for webhook idempotency, plan-switch retries, API contracts, recurring charger registration, and scheduled execution.

Platform and deployment

Layer / File(s)	Summary
Deployment documentation and build settings DEPLOY.md, Dockerfile, CLAUDE.md	Documents deployment, rollback, troubleshooting, server layout, rule numbering, and build environment procedures.
CORS and data-update behavior middleware/*, model/user.go, controller/subscription.go	Adds trusted-origin CORS handling and prevents empty plan or user fields from overwriting stored values.

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

Possibly related PRs

• QuantumNous/new-api#3289: Adds overlapping email-login i18n keys and locale-file changes.
• QuantumNous/new-api#6329: Overlaps with the authentication and access-token plumbing changed in controller/user.go.

Poem

A bunny checks out with a hop and a cheer,
While signed billing events arrive crystal-clear.
Six-digit codes guard the login door,
And renewals keep hopping evermore.
Trusted origins dance in the queue—
Deployment rolls back when it must do.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title is too vague and doesn’t describe the main changes, which span Airwallex billing, email login, and deployment docs.	Use a specific title that names the primary change, such as deferred Airwallex plan switching or the new email login flow.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
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