Feature/add sms notification - #4306
Open
jiong-chen wants to merge 8 commits into
Open
Conversation
架构设计：
- 管理员在「系统设置」中配置短信服务商和凭证
- 用户在「个人设置 → 通知配置」中选择"短信通知"，只需填写手机号
支持 4 种短信服务商：
1. 阿里云短信 — POP v1 签名 (HMAC-SHA1)，纯 HTTP 调用 dysmsapi.aliyuncs.com
2. SendCloud — MD5 签名，POST form 到 sendcloud.net/smsapi/send
3. 腾讯云短信 — TC3-HMAC-SHA256 签名，POST JSON 到 sms.tencentcloudapi.com
4. 通用 HTTP 接口 — 自定义 URL/方法/模板，支持 {{phone}}/{{title}}/{{content}} 占位符
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds SMS notification support: new SMS configuration constants, option persistence, phone-number capture and validation, provider-specific sending (Aliyun, SendCloud, Tencent, custom HTTP), integration into notification/quota flows, and corresponding frontend system/personal settings and i18n entries. Changes
Sequence Diagram(s)sequenceDiagram
participant User as Client
participant API as API Server
participant Notify as Notification Service
participant Provider as SMS Provider (Aliyun/SendCloud/Tencent/Custom)
User->>API: PUT /api/user/setting (sms_phone_number)
activate API
API->>API: Validate phone format & SMSProvider configured
API->>API: Persist settings
deactivate API
Note over User: Trigger event (e.g., quota warning)
User->>API: Action triggers notification
activate API
API->>Notify: NotifyUser(NotifyTypeSms, payload)
activate Notify
Notify->>Notify: Retrieve user's sms_phone_number
Notify->>Notify: Build template values and content
Notify->>Provider: Send via selected provider
alt Aliyun
Provider->>Provider: GET with HMAC-SHA1 signature
else SendCloud
Provider->>Provider: POST form with MD5 sign
else Tencent
Provider->>Provider: POST JSON with TC3-HMAC-SHA256
else Custom
Provider->>Provider: Interpolated GET/POST (SSRF/worker checks)
end
Provider-->>Notify: Response (success/error)
Notify-->>API: Result
deactivate Notify
deactivate API
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (2)
service/sms_notify.go (2)
398-432: Minor: hardcoded Content-Type and User-Agent reduce custom-provider flexibility.
Content-Type: application/json; charset=utf-8is forced regardless of what the admin's template body looks like, which will break integrations that expectapplication/x-www-form-urlencodedor other formats. Consider either auto-detecting from the template, or exposing the headers as an admin-configurable field. Also, theOneAPI-SMS-Notify/1.0User-Agent reads like a leftover from an upstream fork —NewAPI-SMS-Notify/1.0would match this project's branding.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@service/sms_notify.go` around lines 398 - 432, The code currently forces "Content-Type: application/json; charset=utf-8" and "User-Agent: OneAPI-SMS-Notify/1.0" for both workerReq.Headers and req.Header, breaking non-JSON providers and using the wrong branding; update the logic in the send/custom provider path (references: workerReq.Headers, DoWorkerRequest, finalBody, finalURL, req.Header.Set, method) to use headers supplied by the admin/provider configuration (if present) or fall back to auto-detecting Content-Type from the template/body (e.g., form-urlencoded vs JSON) before setting headers, and change the default User-Agent to "NewAPI-SMS-Notify/1.0"; ensure both the worker-request branch (workerReq.Headers) and direct http branch (req.Header) use this configurable/default behavior.
260-265: Consider making provider region/endpoint configurable.Tencent region is hardcoded to
ap-guangzhouand Aliyun uses only the globaldysmsapi.aliyuncs.comendpoint. Users with templates provisioned in other regions (e.g., Tencentap-singapore, Aliyun internationaldysmsapi.ap-southeast-1.aliyuncs.com) will either get region-mismatch errors or be unable to use the integration. A new optional admin field (e.g.,SMSTencentRegion,SMSAliyunEndpoint) would unblock those deployments.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@service/sms_notify.go` around lines 260 - 265, The region/endpoint for SMS providers is hardcoded causing region-mismatch errors; update sendTencentSms to accept a region parameter (or read from a config field like SMSTencentRegion) and use that instead of the fixed "ap-guangzhou", and likewise make the Aliyun SMS calls use a configurable endpoint (e.g., SMSAliyunEndpoint) with the current values as defaults; add the new optional admin/config fields, pass them into sendTencentSms (and the Aliyun send function), and ensure fallback to the original hardcoded values when the config fields are empty.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@common/constants.go`:
- Around line 88-89: The comment for the SMSProvider constant is missing the
"sendcloud" provider; update the comment near the SMSProvider variable (symbol
SMSProvider in common/constants.go) to enumerate all supported providers
including "sendcloud" (matching the switch in service/sms_notify.go and options
in SystemSetting.jsx) so the comment lists aliyun, tencent, sendcloud, custom.
In `@controller/option.go`:
- Around line 74-79: The current blocklist in controller/option.go hides
SMSCustomUrl and SMSCustomTemplate (and explicitly lists some IDs) which breaks
admin UX by masking non-sensitive configuration; update the filtering so
SMSCustomUrl and SMSCustomTemplate are removed from the sensitive-keys check and
only true secrets/keys remain (rely on existing suffix-based filters like
*Secret/*Key), and if you intentionally want to treat IDs as sensitive, add a
brief comment next to the SMSAliyunAccessKeyId / SMSTencentSecretId entries
explaining that policy decision.
In `@controller/user.go`:
- Around line 1191-1210: Replace the three raw c.JSON responses for SMS
validation (empty phone, invalid format using phoneRegex, and missing
common.SMSProvider) with the i18n-aware helper common.ApiErrorI18n so messages
go through localization; locate the checks that reference req.SmsPhoneNumber,
phoneRegex (regexp.MustCompile(`^\+?[0-9]{7,15}$`)) and the conditional on
common.SMSProvider and call common.ApiErrorI18n(c,
"<appropriate_error_key_or_message>") or the existing error key constants used
elsewhere in this handler instead of c.JSON so the responses are localized.
In `@model/option.go`:
- Around line 380-381: The assignment in the switch case for "SMSCustomMethod"
unconditionally sets common.SMSCustomMethod to value, which can overwrite the
default "POST" with an empty or invalid string; change the handler in the case
"SMSCustomMethod" to validate/sanitize value before assignment: if value is
empty or not a valid HTTP method (e.g., not one of GET, POST, PUT, DELETE,
PATCH, OPTIONS, HEAD), do not overwrite common.SMSCustomMethod and leave the
default intact, otherwise assign the sanitized/uppercased method to
common.SMSCustomMethod.
- Around line 352-383: The GET /api/option filtering in controller/option.go
currently omits SMSTencentSmsSdkAppId from the sensitive-keys filter; update the
filter used in the handler (the function that builds the options response for
GET /api/option) to include "SMSTencentSmsSdkAppId" so it is excluded from
responses, and also review/add any of the other SMS fields from model/option.go
(SMSAliyunSignName, SMSAliyunTemplateCode, SMSSendCloudTemplateId,
SMSTencentSignName, SMSTencentTemplateId, SMSCustomMethod) to the same filter if
your threat model requires them to be hidden. Ensure you modify the same
string-list or matching logic used for keys ending with Secret/Key/Token so the
change follows the existing filtering approach.
In `@service/quota.go`:
- Around line 444-448: The SMS template values use values[0]=remainQuota and
values[1]=threshold but when BillingSource == BillingSourceSubscription the
computed remainQuota (relayInfo.UserQuota - consumeQuota) isn't representative
and checkAndSendSubscriptionQuotaNotify currently lacks an SMS path; update
checkAndSendSubscriptionQuotaNotify to mirror the SMS branch logic used where
sendAliyunSms/sendSendCloudSms expect templateValues[0]→user_money and
templateValues[1]→balance_warn, compute remainQuota consistently (use the same
relayInfo.UserQuota - consumeQuota or appropriate subscription remaining
calculation), set values = []interface{}{FormatQuota(remainQuota),
FormatQuota(threshold)}, and call the same SMS send helpers so subscription
quota notifications match the non-subscription SMS behavior.
In `@service/sms_notify.go`:
- Around line 3-23: The file imports and directly uses encoding/json; replace
all direct json.Marshal and json.Unmarshal calls in service/sms_notify.go with
the repository wrappers common.Marshal(...) and common.Unmarshal(..., &v)
(occurrences noted around the JSON call sites referenced in the review: lines
~109, 161, 190, 235, 287, 349, 366), and remove the direct "encoding/json"
import from the import block; ensure error handling and return types remain the
same after swapping to common.Marshal/common.Unmarshal so functions that call
these (e.g., any methods in this file that reference those lines) continue to
behave identically.
In `@web/src/i18n/locales/en.json`:
- Line 3301: There is a duplicate i18n key "敏感信息不会发送到前端显示" in en.json; locate
both occurrences (one earlier and the second at the shown diff) and remove the
redundant entry or consolidate them so only a single definition remains; ensure
the remaining value is the correct English translation ("Sensitive information
will not be sent to the frontend" or the intended text) and run a quick JSON
lint to keep the file valid after removal.
In `@web/src/i18n/locales/fr.json`:
- Line 3267: Remove the duplicate i18n key "敏感信息不会发送到前端显示" by deleting the later
occurrence and keeping the original entry (ensure the single remaining
translation is correct); search the locale JSON for any other duplicates of that
key and remove them, then validate the JSON (lint/parse) to confirm no
duplicate-key overwrite issues remain.
In `@web/src/i18n/locales/ja.json`:
- Line 3248: Remove the duplicate JSON entry for the Chinese key "敏感信息不会发送到前端显示"
(the later translation "機密情報はフロントエンドに表示されません") so the original translation
("機密情報はフロントエンドに送信されず、表示されることはありません") remains authoritative; if you actually need
a different SMS-specific wording, introduce a new Chinese source key and update
the calling component instead, then run the i18n pipeline (bun run i18n:extract,
bun run i18n:sync, bun run i18n:lint) to keep locale files consistent.
- Around line 3216-3249: The ja.json SMS block introduced a duplicate key
"敏感信息不会发送到前端显示" (already present elsewhere in ja.json) which will overwrite the
original—remove the duplicate entry from the SMS section (the new one around the
SMS keys) and keep the original translation; then ensure the 34 SMS keys you
added (e.g., "短信通知", "短信服务商", "阿里云短信", "腾讯云短信", "通用HTTP接口", "手机号码", "短信签名",
"模板Code", "模板ID", "接口地址", "请求方法", "请求模板", etc.) are propagated to other locales
by running the localization sync and lint commands: run "bun run i18n:sync" to
add these keys to zh-TW.json and other locale files, then run "bun run
i18n:lint" to validate and fix any structural issues.
In `@web/src/i18n/locales/ru.json`:
- Line 3281: There is a duplicated JSON translation key "敏感信息不会发送到前端显示" — remove
the later occurrence (or consolidate its value with the original) so the key
appears only once in the ru locale, then re-run the i18n pipeline (bun run
i18n:extract, bun run i18n:sync, bun run i18n:lint) to ensure no other
duplicates or lint errors remain.
In `@web/src/i18n/locales/vi.json`:
- Around line 3748-3749: The JSON contains duplicate translation keys "未配置" and
"敏感信息不会发送到前端显示" that override earlier entries; locate the earlier definitions of
those keys in the same vi.json and remove or reconcile these later duplicate
entries so each Chinese source string appears only once (keeping the intended
Vietnamese value), or consolidate by updating the original key's value instead
of adding duplicates; ensure the file remains a flat JSON object and validate no
other duplicate Chinese keys exist.
In `@web/src/i18n/locales/zh-CN.json`:
- Line 2535: Remove the duplicate JSON key "敏感信息不会发送到前端显示" from this locale file
by deleting this occurrence so only the original mapping remains (the earlier
entry already has the identical value); ensure the surrounding commas remain
valid and the JSON stays well-formed after removal.
---
Nitpick comments:
In `@service/sms_notify.go`:
- Around line 398-432: The code currently forces "Content-Type:
application/json; charset=utf-8" and "User-Agent: OneAPI-SMS-Notify/1.0" for
both workerReq.Headers and req.Header, breaking non-JSON providers and using the
wrong branding; update the logic in the send/custom provider path (references:
workerReq.Headers, DoWorkerRequest, finalBody, finalURL, req.Header.Set, method)
to use headers supplied by the admin/provider configuration (if present) or fall
back to auto-detecting Content-Type from the template/body (e.g.,
form-urlencoded vs JSON) before setting headers, and change the default
User-Agent to "NewAPI-SMS-Notify/1.0"; ensure both the worker-request branch
(workerReq.Headers) and direct http branch (req.Header) use this
configurable/default behavior.
- Around line 260-265: The region/endpoint for SMS providers is hardcoded
causing region-mismatch errors; update sendTencentSms to accept a region
parameter (or read from a config field like SMSTencentRegion) and use that
instead of the fixed "ap-guangzhou", and likewise make the Aliyun SMS calls use
a configurable endpoint (e.g., SMSAliyunEndpoint) with the current values as
defaults; add the new optional admin/config fields, pass them into
sendTencentSms (and the Aliyun send function), and ensure fallback to the
original hardcoded values when the config fields are empty.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ad5713e6-6b1a-47e0-9865-34630f9809b8
📒 Files selected for processing (17)
common/constants.gocontroller/option.gocontroller/user.godto/user_settings.gomodel/option.goservice/quota.goservice/sms_notify.goservice/user_notify.goweb/src/components/settings/PersonalSetting.jsxweb/src/components/settings/SystemSetting.jsxweb/src/components/settings/personal/cards/NotificationSettings.jsxweb/src/i18n/locales/en.jsonweb/src/i18n/locales/fr.jsonweb/src/i18n/locales/ja.jsonweb/src/i18n/locales/ru.jsonweb/src/i18n/locales/vi.jsonweb/src/i18n/locales/zh-CN.json
Comment on lines
+352
to
+383
| case "SMSProvider": | ||
| common.SMSProvider = value | ||
| case "SMSAliyunAccessKeyId": | ||
| common.SMSAliyunAccessKeyId = value | ||
| case "SMSAliyunAccessKeySecret": | ||
| common.SMSAliyunAccessKeySecret = value | ||
| case "SMSAliyunSignName": | ||
| common.SMSAliyunSignName = value | ||
| case "SMSAliyunTemplateCode": | ||
| common.SMSAliyunTemplateCode = value | ||
| case "SMSSendCloudSmsUser": | ||
| common.SMSSendCloudSmsUser = value | ||
| case "SMSSendCloudSmsKey": | ||
| common.SMSSendCloudSmsKey = value | ||
| case "SMSSendCloudTemplateId": | ||
| common.SMSSendCloudTemplateId = value | ||
| case "SMSTencentSecretId": | ||
| common.SMSTencentSecretId = value | ||
| case "SMSTencentSecretKey": | ||
| common.SMSTencentSecretKey = value | ||
| case "SMSTencentSmsSdkAppId": | ||
| common.SMSTencentSmsSdkAppId = value | ||
| case "SMSTencentSignName": | ||
| common.SMSTencentSignName = value | ||
| case "SMSTencentTemplateId": | ||
| common.SMSTencentTemplateId = value | ||
| case "SMSCustomUrl": | ||
| common.SMSCustomUrl = value | ||
| case "SMSCustomMethod": | ||
| common.SMSCustomMethod = value | ||
| case "SMSCustomTemplate": | ||
| common.SMSCustomTemplate = value |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Enumerate which SMS keys are actually filtered by controller/option.go and which are returned.
rg -nP -C3 '(SMSAliyunAccessKeyId|SMSSendCloudSmsUser|SMSTencentSecretId|SMSCustomUrl|SMSCustomTemplate|HasSuffix\(k,\s*"(Secret|Key|Token|secret|api_key)"\))' controller/option.go
echo "---"
# Find any other place SMS* options are read/written (to confirm no additional exposure paths)
rg -nP -C2 '\bSMS[A-Z][A-Za-z]+' --type=goRepository: QuantumNous/new-api
Length of output: 7675
Fix credential exposure: SMSTencentSmsSdkAppId is returned unfiltered in GET /api/option/.
The filtering logic in controller/option.go (lines 69–79) uses suffix matching (*Secret, *Key, Token) and explicitly lists 5 SMS keys to exclude. However, SMSTencentSmsSdkAppId matches none of these rules and is returned to the frontend. While nominally a tenant ID rather than a secret, it identifies the Tencent application and should be filtered. Also verify that other unfiltered SMS keys (SMSAliyunSignName, SMSAliyunTemplateCode, SMSSendCloudTemplateId, SMSTencentSignName, SMSTencentTemplateId, SMSCustomMethod) do not require filtering based on your threat model.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@model/option.go` around lines 352 - 383, The GET /api/option filtering in
controller/option.go currently omits SMSTencentSmsSdkAppId from the
sensitive-keys filter; update the filter used in the handler (the function that
builds the options response for GET /api/option) to include
"SMSTencentSmsSdkAppId" so it is excluded from responses, and also review/add
any of the other SMS fields from model/option.go (SMSAliyunSignName,
SMSAliyunTemplateCode, SMSSendCloudTemplateId, SMSTencentSignName,
SMSTencentTemplateId, SMSCustomMethod) to the same filter if your threat model
requires them to be hidden. Ensure you modify the same string-list or matching
logic used for keys ending with Secret/Key/Token so the change follows the
existing filtering approach.
- 补全SMSProvider注释中遗漏的sendcloud服务商 - 调整敏感字段过滤：移除非敏感的SMSCustomUrl/SMSCustomTemplate，新增SMSTencentSmsSdkAppId - SMS验证错误响应改用i18n国际化(ApiErrorI18n)，新增3组翻译(en/zh-CN/zh-TW) - SMSCustomMethod赋值前增加HTTP方法校验，空值保留默认POST - sms_notify.go中encoding/json替换为common.Marshal/Unmarshal(遵循Rule 1) - User-Agent从OneAPI改为NewAPI匹配项目品牌 - 移除6个locale JSON文件中重复的i18n key - 订阅额度通知(checkAndSendSubscriptionQuotaNotify)补充SMS分支
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
model/option.go (1)
380-389:⚠️ Potential issue | 🟡 MinorEmpty
SMSCustomMethodcan keep a stale non-default method in memory.At Line 385-387, empty input does not reset
common.SMSCustomMethod. If it was previouslyGET, saving an empty value can continue usingGETuntil restart instead of defaulting toPOST.🔧 Suggested fix
case "SMSCustomMethod": - upperMethod := strings.ToUpper(value) + upperMethod := strings.ToUpper(strings.TrimSpace(value)) switch upperMethod { case "GET", "POST", "PUT", "DELETE", "PATCH": common.SMSCustomMethod = upperMethod case "": - // 空值保留默认的 POST + common.SMSCustomMethod = "POST" default: common.SMSCustomMethod = "POST" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@model/option.go` around lines 380 - 389, The empty-input branch in the "SMSCustomMethod" case currently leaves common.SMSCustomMethod unchanged, allowing a stale method to persist; change the "" branch in the switch over upperMethod inside the "SMSCustomMethod" case to explicitly set common.SMSCustomMethod = "POST" (the default) so that empty input resets the value; ensure the allowed-method switch (case "GET","POST","PUT","DELETE","PATCH") still assigns upperMethod and the default branch also falls back to "POST".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@service/quota.go`:
- Around line 444-448: The SMS branch currently clears the content variable
(content = "") which strips the rendered message before calling the custom SMS
provider; remove that assignment and ensure content remains the human-readable
message used elsewhere, while still populating values (e.g., keep values =
[]interface{}{logger.FormatQuota(remainQuota), logger.FormatQuota(threshold)}).
Update the branch that checks notifyType == dto.NotifyTypeSms (and the other
similar SMS branches around lines ~500) to stop clearing content so {{content}}
is available to service/sms_notify.go and custom providers can receive the
remaining-quota/threshold message.
In `@service/sms_notify.go`:
- Around line 336-358: The code currently treats a nil Response.Error as success
even when result.Response.SendStatusSet is missing or empty; update the
post-unmarshal checks in sms_notify.go (around the result struct handling) to
explicitly reject an empty or missing SendStatusSet: after checking
result.Response.Error, verify that len(result.Response.SendStatusSet) > 0 and
return an error (e.g. "tencent sms failed: empty SendStatusSet") if it is zero
or nil before inspecting SendStatusSet[0].Code; this ensures you don't report
success when per-recipient results are absent.
- Around line 392-442: The worker branch is rejecting valid http:// endpoints
because it calls DoWorkerRequest without the same URL validation/allowances as
the direct branch; before constructing/dispatching WorkerRequest in the
system_setting.EnableWorker() path, run the same
common.ValidateURLWithFetchSetting(...) check used in the else branch to allow
http per fetch settings, and if WorkerRequest/DoWorkerRequest supports an
explicit flag (e.g., AllowInsecureHTTP or similar tied to
WorkerAllowHttpImageRequestEnabled) set that flag on WorkerRequest so the worker
will not force HTTPS; update usages around WorkerRequest and DoWorkerRequest to
preserve POST body handling (workerReq.Body) and to mirror the non-worker
header/validation behavior.
In `@web/src/i18n/locales/ja.json`:
- Around line 3216-3248: The ja.json locale is missing backend SMS validation
keys used by the server; add Japanese translations for the keys
setting.sms_phone_empty, setting.sms_phone_invalid, and
setting.sms_not_configured so validation errors from controller/user.go (SMS
validation) and i18n key lookups return human-readable messages; mirror the
existing English/Chinese semantics from the UI strings (e.g., use
"電話番号を入力してください" for sms_phone_empty, "電話番号の形式が正しくありません" for sms_phone_invalid,
and "SMSサービスが設定されていません" for sms_not_configured).
---
Duplicate comments:
In `@model/option.go`:
- Around line 380-389: The empty-input branch in the "SMSCustomMethod" case
currently leaves common.SMSCustomMethod unchanged, allowing a stale method to
persist; change the "" branch in the switch over upperMethod inside the
"SMSCustomMethod" case to explicitly set common.SMSCustomMethod = "POST" (the
default) so that empty input resets the value; ensure the allowed-method switch
(case "GET","POST","PUT","DELETE","PATCH") still assigns upperMethod and the
default branch also falls back to "POST".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6f75dee1-f3d9-4f9f-8480-337d50a33898
📒 Files selected for processing (16)
common/constants.gocontroller/option.gocontroller/user.goi18n/keys.goi18n/locales/en.yamli18n/locales/zh-CN.yamli18n/locales/zh-TW.yamlmodel/option.goservice/quota.goservice/sms_notify.goweb/src/i18n/locales/en.jsonweb/src/i18n/locales/fr.jsonweb/src/i18n/locales/ja.jsonweb/src/i18n/locales/ru.jsonweb/src/i18n/locales/vi.jsonweb/src/i18n/locales/zh-CN.json
✅ Files skipped from review due to trivial changes (7)
- i18n/locales/zh-CN.yaml
- i18n/locales/zh-TW.yaml
- i18n/locales/en.yaml
- i18n/keys.go
- common/constants.go
- web/src/i18n/locales/zh-CN.json
- web/src/i18n/locales/ru.json
🚧 Files skipped from review as they are similar to previous changes (1)
- web/src/i18n/locales/fr.json
Comment on lines
+3216
to
+3248
| "短信通知": "SMS通知", | ||
| "短信服务商": "SMSプロバイダー", | ||
| "阿里云短信": "Alibaba Cloud SMS", | ||
| "腾讯云短信": "Tencent Cloud SMS", | ||
| "通用HTTP接口": "カスタムHTTP API", | ||
| "手机号码": "電話番号", | ||
| "请输入接收短信的手机号码": "SMSを受信する電話番号を入力してください", | ||
| "请输入手机号码": "電話番号を入力してください", | ||
| "手机号码格式不正确": "電話番号の形式が正しくありません", | ||
| "短信服务由管理员统一配置，您只需填写接收通知的手机号码": "SMSサービスは管理者が設定します。電話番号のみ入力してください", | ||
| "短信签名": "SMS署名", | ||
| "模板Code": "テンプレートコード", | ||
| "请输入模板Code": "テンプレートコードを入力してください", | ||
| "模板ID": "テンプレートID", | ||
| "请输入模板ID": "テンプレートIDを入力してください", | ||
| "接口地址": "API URL", | ||
| "请输入短信接口地址": "SMS API URLを入力してください", | ||
| "请求方法": "リクエストメソッド", | ||
| "请求模板": "リクエストテンプレート", | ||
| "请输入请求模板": "リクエストテンプレートを入力してください", | ||
| "短信接口地址必须以http://或https://开头": "SMS API URLはhttp://またはhttps://で始まる必要があります", | ||
| "阿里云短信配置说明": "Alibaba Cloud SMS設定", | ||
| "腾讯云短信配置说明": "Tencent Cloud SMS設定", | ||
| "通用HTTP接口说明": "カスタムHTTP API説明", | ||
| "请输入短信签名": "SMS署名を入力してください", | ||
| "在阿里云短信服务控制台创建签名和模板后获取相关参数": "Alibaba Cloud SMSコンソールで署名とテンプレートを作成した後、パラメータを取得してください", | ||
| "在腾讯云短信控制台创建应用、签名和模板后获取相关参数": "Tencent Cloud SMSコンソールでアプリ、署名、テンプレートを作成した後、パラメータを取得してください", | ||
| "支持模板变量: {{phone}} (手机号), {{title}} (通知标题), {{content}} (通知内容)": "テンプレート変数: {{phone}} (電話番号), {{title}} (通知タイトル), {{content}} (通知内容)", | ||
| "短信模板中需包含一个变量用于接收通知内容": "SMSテンプレートには通知内容用の変数が1つ必要です", | ||
| "配置短信服务": "SMSサービスの設定", | ||
| "用以支持短信通知推送，用户可在个人设置中选择短信通知方式": "SMS通知をサポートします。ユーザーは個人設定でSMS通知を選択できます", | ||
| "未配置": "未設定", | ||
| "保存短信设置": "SMS設定を保存", |
Contributor
There was a problem hiding this comment.
Add missing backend SMS error i18n keys for Japanese.
This block adds UI SMS strings, but the backend also returns setting.sms_phone_empty, setting.sms_phone_invalid, and setting.sms_not_configured (see controller/user.go Line 1188 and i18n/keys.go Line 249). I don’t see these keys in ja.json, so Japanese users will see raw key names on validation errors.
🛠️ Proposed fix
"保存短信设置": "SMS設定を保存",
+ "setting.sms_phone_empty": "電話番号を入力してください",
+ "setting.sms_phone_invalid": "電話番号の形式が正しくありません",
+ "setting.sms_not_configured": "SMSサービスが未設定です。管理者にお問い合わせください",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/i18n/locales/ja.json` around lines 3216 - 3248, The ja.json locale is
missing backend SMS validation keys used by the server; add Japanese
translations for the keys setting.sms_phone_empty, setting.sms_phone_invalid,
and setting.sms_not_configured so validation errors from controller/user.go (SMS
validation) and i18n key lookups return human-readable messages; mirror the
existing English/Chinese semantics from the UI strings (e.g., use
"電話番号を入力してください" for sms_phone_empty, "電話番号の形式が正しくありません" for sms_phone_invalid,
and "SMSサービスが設定されていません" for sms_not_configured).
…ication # Conflicts: # controller/option.go # web/src/i18n/locales/zh-CN.json
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
service/quota.go (1)
481-485:⚠️ Potential issue | 🟠 MajorPreserve SMS
contentfor custom-provider templates.On Line 483 and Line 539, setting
content = ""drops the human-readable message for custom HTTP SMS integrations that render{{content}}, so users receive incomplete notifications.🛠️ Proposed fix
- } else if notifyType == dto.NotifyTypeSms { - // SMS短信使用模板变量：values[0]=当前余额, values[1]=告警阈值 - content = "" - remainQuota := relayInfo.UserQuota - consumeQuota - values = []interface{}{logger.FormatQuota(remainQuota), logger.FormatQuota(threshold)} + } else if notifyType == dto.NotifyTypeSms { + // 保留 content 以兼容 custom HTTP 模板中的 {{content}} + content = "当前剩余额度为 {{value}}，已触发告警阈值 {{value}}，请及时充值。" + remainQuota := relayInfo.UserQuota - consumeQuota + values = []interface{}{logger.FormatQuota(remainQuota), logger.FormatQuota(threshold)} } @@ - } else if notifyType == dto.NotifyTypeSms { - // SMS短信使用模板变量：values[0]=当前余额, values[1]=告警阈值 - content = "" - values = []interface{}{logger.FormatQuota(int(remaining)), logger.FormatQuota(threshold)} + } else if notifyType == dto.NotifyTypeSms { + content = "当前剩余额度为 {{value}}，已触发告警阈值 {{value}}，请及时充值。" + values = []interface{}{logger.FormatQuota(int(remaining)), logger.FormatQuota(threshold)} }Also applies to: 537-540
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@service/quota.go` around lines 481 - 485, In the NotifyTypeSms branches (where notifyType == dto.NotifyTypeSms) don't overwrite the existing content with an empty string; instead remove the assignments content = "" and simply set values = []interface{}{logger.FormatQuota(remainQuota), logger.FormatQuota(threshold)} so the original human-readable content remains for custom HTTP SMS templates that use {{content}}; make the same change in the other symmetric SMS branch (the second block that also sets content = "") so both SMS paths preserve the message body while still populating values using relayInfo.UserQuota, consumeQuota, threshold and logger.FormatQuota.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@service/quota.go`:
- Around line 481-485: In the NotifyTypeSms branches (where notifyType ==
dto.NotifyTypeSms) don't overwrite the existing content with an empty string;
instead remove the assignments content = "" and simply set values =
[]interface{}{logger.FormatQuota(remainQuota), logger.FormatQuota(threshold)} so
the original human-readable content remains for custom HTTP SMS templates that
use {{content}}; make the same change in the other symmetric SMS branch (the
second block that also sets content = "") so both SMS paths preserve the message
body while still populating values using relayInfo.UserQuota, consumeQuota,
threshold and logger.FormatQuota.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bdd86e8f-08bf-4005-8011-c51f91394014
📒 Files selected for processing (16)
common/constants.gocontroller/option.gocontroller/user.goi18n/keys.goi18n/locales/en.yamli18n/locales/zh-CN.yamli18n/locales/zh-TW.yamlmodel/option.goservice/quota.goweb/src/components/settings/PersonalSetting.jsxweb/src/i18n/locales/en.jsonweb/src/i18n/locales/fr.jsonweb/src/i18n/locales/ja.jsonweb/src/i18n/locales/ru.jsonweb/src/i18n/locales/vi.jsonweb/src/i18n/locales/zh-CN.json
✅ Files skipped from review due to trivial changes (9)
- i18n/locales/zh-CN.yaml
- i18n/locales/zh-TW.yaml
- i18n/locales/en.yaml
- i18n/keys.go
- web/src/i18n/locales/fr.json
- web/src/i18n/locales/en.json
- web/src/i18n/locales/ja.json
- web/src/i18n/locales/vi.json
- web/src/i18n/locales/ru.json
🚧 Files skipped from review as they are similar to previous changes (3)
- controller/option.go
- web/src/i18n/locales/zh-CN.json
- web/src/components/settings/PersonalSetting.jsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/classic/src/components/settings/SystemSetting.jsx`:
- Around line 388-405: The loop over smsKeys currently prevents clearing saved
non-secret SMS fields because of the inputs[key] !== '' guard; adjust the logic
in the block that iterates smsKeys (the code using smsKeys, originInputs, and
inputs) to only skip empty values for masked/credential fields (the same ones
omitted by GetOptions) rather than for all keys—i.e., remove the global
inputs[key] !== '' check and instead check if the key is a credential/masked
field before skipping empty values; ensure you still handle SMSProvider deduping
(the existing SMSProvider existence check) and call updateOptions(options)
unchanged when options.length > 0.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ab4f24ca-2c04-41ab-a6db-09786bdc3199
📒 Files selected for processing (5)
common/constants.gocontroller/option.goweb/classic/src/components/settings/PersonalSetting.jsxweb/classic/src/components/settings/SystemSetting.jsxweb/classic/src/components/settings/personal/cards/NotificationSettings.jsx
✅ Files skipped from review due to trivial changes (1)
- common/constants.go
- 腾讯云SMS：空SendStatusSet时返回错误而非静默成功
- 自定义SMS：HTTP URL不走Worker（Worker会拒绝非HTTPS），回退到直连
- 额度通知SMS：content保留可读文本供自定义HTTP接口的{{content}}占位符使用
- Classic前端submitSMS：区分凭证字段和配置字段，允许清空非凭证项
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
service/sms_notify.go (1)
396-423:⚠️ Potential issue | 🟠 MajorKeep fetch-policy validation in the worker path.
useWorkernow skipscommon.ValidateURLWithFetchSetting(...)entirely for HTTPS URLs, so enabling worker mode lets custom SMS requests bypass the same SSRF/domain/IP restrictions that the direct path enforces. Please validatefinalURLbeforeDoWorkerRequest, then dispatch through the worker only after it passes.🛠️ Suggested fix
+ fetchSetting := system_setting.GetFetchSetting() + if err := common.ValidateURLWithFetchSetting( + finalURL, + fetchSetting.EnableSSRFProtection, + fetchSetting.AllowPrivateIp, + fetchSetting.DomainFilterMode, + fetchSetting.IpFilterMode, + fetchSetting.DomainList, + fetchSetting.IpList, + fetchSetting.AllowedPorts, + fetchSetting.ApplyIPFilterForDomain, + ); err != nil { + return fmt.Errorf("request reject: %v", err) + } + useWorker := system_setting.EnableWorker() && strings.HasPrefix(strings.ToLower(finalURL), "https://")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@service/sms_notify.go` around lines 396 - 423, The worker path currently skips SSRF/domain/IP validation, so before calling DoWorkerRequest when useWorker is true, run common.ValidateURLWithFetchSetting(finalURL, ...) using the same fetchSetting used in the non-worker branch (obtain via system_setting.GetFetchSetting()), return the validation error if any, and only then build the WorkerRequest and call DoWorkerRequest; update symbols referenced: useWorker, finalURL, system_setting.GetFetchSetting(), common.ValidateURLWithFetchSetting(...), DoWorkerRequest(), and WorkerRequest to ensure parity with the non-worker path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@model/option.go`:
- Around line 392-401: The OptionMap is being written with the raw input before
validation so unsupported or blank methods can be exposed; update the
"SMSCustomMethod" branch to validate and normalize the input first (upper-case
and check against the exact allowed set used by service/sms_notify.go) and then
assign that normalized value into both common.SMSCustomMethod and
common.OptionMap[key] (use "POST" as the fallback for empty/unsupported values);
ensure the allowed-method list you check here exactly matches the sender
implementation in service/sms_notify.go so config cannot drift from runtime
behavior.
---
Duplicate comments:
In `@service/sms_notify.go`:
- Around line 396-423: The worker path currently skips SSRF/domain/IP
validation, so before calling DoWorkerRequest when useWorker is true, run
common.ValidateURLWithFetchSetting(finalURL, ...) using the same fetchSetting
used in the non-worker branch (obtain via system_setting.GetFetchSetting()),
return the validation error if any, and only then build the WorkerRequest and
call DoWorkerRequest; update symbols referenced: useWorker, finalURL,
system_setting.GetFetchSetting(), common.ValidateURLWithFetchSetting(...),
DoWorkerRequest(), and WorkerRequest to ensure parity with the non-worker path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3b958e1d-9824-402f-a927-5ced96a9a8aa
📒 Files selected for processing (4)
model/option.goservice/quota.goservice/sms_notify.goweb/classic/src/components/settings/SystemSetting.jsx
Comment on lines
+392
to
+401
| case "SMSCustomMethod": | ||
| upperMethod := strings.ToUpper(value) | ||
| switch upperMethod { | ||
| case "GET", "POST", "PUT", "DELETE", "PATCH": | ||
| common.SMSCustomMethod = upperMethod | ||
| case "": | ||
| // 空值保留默认的 POST | ||
| default: | ||
| common.SMSCustomMethod = "POST" | ||
| } |
Contributor
There was a problem hiding this comment.
Normalize SMSCustomMethod before storing it.
common.OptionMap[key] is written with the raw input before this branch runs, so a blank or unsupported method can still be exposed via /api/option/ even though common.SMSCustomMethod falls back to POST. This also accepts methods that service/sms_notify.go does not actually send, so the config can drift from runtime behavior.
🛠️ Suggested fix
case "SMSCustomMethod":
upperMethod := strings.ToUpper(value)
switch upperMethod {
- case "GET", "POST", "PUT", "DELETE", "PATCH":
+ case "GET", "POST":
common.SMSCustomMethod = upperMethod
+ common.OptionMap[key] = upperMethod
case "":
- // 空值保留默认的 POST
+ common.SMSCustomMethod = "POST"
+ common.OptionMap[key] = "POST"
default:
common.SMSCustomMethod = "POST"
+ common.OptionMap[key] = "POST"
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@model/option.go` around lines 392 - 401, The OptionMap is being written with
the raw input before validation so unsupported or blank methods can be exposed;
update the "SMSCustomMethod" branch to validate and normalize the input first
(upper-case and check against the exact allowed set used by
service/sms_notify.go) and then assign that normalized value into both
common.SMSCustomMethod and common.OptionMap[key] (use "POST" as the fallback for
empty/unsupported values); ensure the allowed-method list you check here exactly
matches the sender implementation in service/sms_notify.go so config cannot
drift from runtime behavior.
Calcium-Ion
force-pushed
the
main
branch
2 times, most recently
from
August 30, 2026 15:03
51fdfc5 to
2b6f1df
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Important
为系统增加了多渠道短信通知能力。后端实现了统一的短信发送抽象层，管理员可通过面板配置密钥，用户在个人中心绑定手机号即可接收告警/通知。
📝 变更描述 / Description
(简述：做了什么？为什么这样改能生效？请基于你对代码逻辑的理解来写，避免粘贴未经整理的内容)
工作原理：
配置层：在系统设置中增加了 SMS 配置项，用于存储服务商凭证。
发送层：实现了独立的 SMS 服务模块，根据配置动态加载对应的 SDK 或 HTTP 请求逻辑。
调用层：在原有的通知触发逻辑中增加了 type=sms 的判断分支，仅当用户已绑定手机且余额/配置正常时执行。
签名算法：针对阿里云（HMAC-SHA1）、腾讯云（TC3-HMAC-SHA256）和 SendCloud（MD5）的不同鉴权方式进行了适配。
🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Bug fix，我已提交或关联对应 Issue，且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。📸 运行证明 / Proof of Work
(请在此粘贴截图、关键日志或测试报告，以证明变更生效)
Summary by CodeRabbit