fix: audit fixes for websearch, notifications, and channel pricing

P0: fix wildcard matching test assertion (config order, not longest prefix)
P0: add TotalRecharged to auth cache snapshot (v5) for percentage threshold
P1: move pricing rules into per-platform sections in ChannelsView
P1: populate account name cache when editing existing channel rules
P1: sanitize email subject headers to prevent SMTP injection
P1: make Redis INCR+EXPIRE idempotent for rate limiting
P1: deep copy FeaturesConfig in Channel.Clone()
P2: clean up stale email="" placeholder comments
P2: replace log.Printf with slog in email_service.go
This commit is contained in:
erio
2026-04-13 13:59:35 +08:00
parent 47a13d3105
commit ad351d4252
13 changed files with 172 additions and 86 deletions
+1 -1
View File
@@ -1 +1 @@
0.1.110.19
0.1.110.20
@@ -3,7 +3,7 @@ package dto
import "github.com/Wei-Shaw/sub2api/internal/service"
// NotifyEmailEntry represents a notification email with enable/disable and verification state.
// Email="" is a placeholder for the "primary email" (user's registration email or first admin email).
// All emails are user-managed; maximum 3 entries per user.
type NotifyEmailEntry struct {
Email string `json:"email"`
Disabled bool `json:"disabled"`
+1 -1
View File
@@ -217,7 +217,7 @@ func (h *UserHandler) RemoveNotifyEmail(c *gin.Context) {
// ToggleNotifyEmailRequest represents the request to toggle a notify email's disabled state
type ToggleNotifyEmailRequest struct {
Email string `json:"email"` // empty string for primary email placeholder
Email string `json:"email" binding:"required,email"`
Disabled bool `json:"disabled"`
}
+3 -3
View File
@@ -156,9 +156,9 @@ func (c *emailCache) IncrNotifyCodeUserRate(ctx context.Context, userID int64, w
if err != nil {
return 0, err
}
// Set TTL only on first increment (when count becomes 1)
if count == 1 {
c.rdb.Expire(ctx, key, window)
// Always set TTL (idempotent) to avoid orphan keys if process crashes between INCR and EXPIRE.
if err := c.rdb.Expire(ctx, key, window).Err(); err != nil {
return count, fmt.Errorf("expire notify code rate key: %w", err)
}
return count, nil
}
@@ -145,14 +145,14 @@ func TestFindPricingForModel(t *testing.T) {
wantNil: true,
},
{
name: "longer wildcard prefix wins over shorter",
name: "wildcard matches by config order (first match wins)",
list: []ChannelModelPricing{
{ID: 10, Models: []string{"claude-*"}},
{ID: 11, Models: []string{"claude-opus-*"}},
},
platform: "",
model: "claude-opus-4",
wantID: 11, // "claude-opus-" (12 chars) > "claude-" (7 chars)
wantID: 10, // config order: "claude-*" is first and matches, so it wins
},
{
name: "shorter wildcard used when longer does not match",
@@ -42,6 +42,7 @@ type APIKeyAuthUserSnapshot struct {
BalanceNotifyThresholdType string `json:"balance_notify_threshold_type"`
BalanceNotifyThreshold *float64 `json:"balance_notify_threshold,omitempty"`
BalanceNotifyExtraEmails []NotifyEmailEntry `json:"balance_notify_extra_emails,omitempty"`
TotalRecharged float64 `json:"total_recharged"`
}
// APIKeyAuthGroupSnapshot 分组快照
@@ -13,7 +13,7 @@ import (
"github.com/dgraph-io/ristretto"
)
const apiKeyAuthSnapshotVersion = 4 // v4: added balance notification fields to UserSnapshot
const apiKeyAuthSnapshotVersion = 5 // v5: added TotalRecharged for percentage threshold
type apiKeyAuthCacheConfig struct {
l1Size int
@@ -230,6 +230,7 @@ func (s *APIKeyService) snapshotFromAPIKey(apiKey *APIKey) *APIKeyAuthSnapshot {
BalanceNotifyThresholdType: apiKey.User.BalanceNotifyThresholdType,
BalanceNotifyThreshold: apiKey.User.BalanceNotifyThreshold,
BalanceNotifyExtraEmails: apiKey.User.BalanceNotifyExtraEmails,
TotalRecharged: apiKey.User.TotalRecharged,
},
}
if apiKey.Group != nil {
@@ -291,6 +292,7 @@ func (s *APIKeyService) snapshotToAPIKey(key string, snapshot *APIKeyAuthSnapsho
BalanceNotifyThresholdType: snapshot.User.BalanceNotifyThresholdType,
BalanceNotifyThreshold: snapshot.User.BalanceNotifyThreshold,
BalanceNotifyExtraEmails: snapshot.User.BalanceNotifyExtraEmails,
TotalRecharged: snapshot.User.TotalRecharged,
},
}
if snapshot.Group != nil {
@@ -309,7 +309,7 @@ func (s *BalanceNotifyService) sendBalanceLowEmails(recipients []string, userNam
if displayName == "" {
displayName = userEmail
}
subject := fmt.Sprintf("[%s] 余额不足提醒 / Balance Low Alert", siteName)
subject := fmt.Sprintf("[%s] 余额不足提醒 / Balance Low Alert", sanitizeEmailHeader(siteName))
body := s.buildBalanceLowEmailBody(html.EscapeString(displayName), balance, threshold, html.EscapeString(siteName))
s.sendEmails(recipients, subject, body, "user_email", userEmail, "balance", balance)
}
@@ -321,11 +321,16 @@ func (s *BalanceNotifyService) sendQuotaAlertEmails(adminEmails []string, accoun
dimLabel = dimension
}
subject := fmt.Sprintf("[%s] 账号限额告警 / Account Quota Alert - %s", siteName, accountName)
subject := fmt.Sprintf("[%s] 账号限额告警 / Account Quota Alert - %s", sanitizeEmailHeader(siteName), sanitizeEmailHeader(accountName))
body := s.buildQuotaAlertEmailBody(html.EscapeString(accountName), html.EscapeString(dimLabel), used, limit, threshold, html.EscapeString(siteName))
s.sendEmails(adminEmails, subject, body, "account", accountName, "dimension", dimension)
}
// sanitizeEmailHeader removes CR/LF characters to prevent SMTP header injection.
func sanitizeEmailHeader(s string) string {
return strings.NewReplacer("\r", "", "\n", "").Replace(s)
}
// balanceLowEmailTemplate is the HTML template for balance low notifications.
// Format args: siteName, userName, userName, balance, threshold, threshold.
const balanceLowEmailTemplate = `<!DOCTYPE html>
+16
View File
@@ -211,6 +211,9 @@ func (c *Channel) Clone() *Channel {
cp.ModelMapping[platform] = inner
}
}
if c.FeaturesConfig != nil {
cp.FeaturesConfig = deepCopyFeaturesConfig(c.FeaturesConfig)
}
if c.AccountStatsPricingRules != nil {
cp.AccountStatsPricingRules = make([]AccountStatsPricingRule, len(c.AccountStatsPricingRules))
for i, rule := range c.AccountStatsPricingRules {
@@ -234,6 +237,19 @@ func (c *Channel) Clone() *Channel {
return &cp
}
// deepCopyFeaturesConfig creates a deep copy of FeaturesConfig to prevent cache pollution.
func deepCopyFeaturesConfig(src map[string]any) map[string]any {
dst := make(map[string]any, len(src))
for k, v := range src {
if inner, ok := v.(map[string]any); ok {
dst[k] = deepCopyFeaturesConfig(inner)
} else {
dst[k] = v
}
}
return dst
}
// ValidateIntervals 校验区间列表的合法性。
// 规则:MinTokens >= 0;MaxTokens 若非 nil 则 > 0 且 > MinTokens;
// 所有价格字段 >= 0;区间按 MinTokens 排序后无重叠((min, max] 语义);
+6 -6
View File
@@ -7,7 +7,7 @@ import (
"crypto/tls"
"encoding/hex"
"fmt"
"log"
"log/slog"
"math/big"
"net/smtp"
"net/url"
@@ -296,7 +296,7 @@ func (s *EmailService) VerifyCode(ctx context.Context, email, code string) error
if subtle.ConstantTimeCompare([]byte(data.Code), []byte(code)) != 1 {
data.Attempts++
if err := s.cache.SetVerificationCode(ctx, email, data, verifyCodeTTL); err != nil {
log.Printf("[Email] Failed to update verification attempt count: %v", err)
slog.Error("failed to update verification attempt count", "email", email, "error", err)
}
if data.Attempts >= maxVerifyCodeAttempts {
return ErrVerifyCodeMaxAttempts
@@ -306,7 +306,7 @@ func (s *EmailService) VerifyCode(ctx context.Context, email, code string) error
// 验证成功,删除验证码
if err := s.cache.DeleteVerificationCode(ctx, email); err != nil {
log.Printf("[Email] Failed to delete verification code after success: %v", err)
slog.Error("failed to delete verification code after success", "email", email, "error", err)
}
return nil
}
@@ -456,7 +456,7 @@ func (s *EmailService) SendPasswordResetEmail(ctx context.Context, email, siteNa
func (s *EmailService) SendPasswordResetEmailWithCooldown(ctx context.Context, email, siteName, resetURL string) error {
// Check email cooldown to prevent email bombing
if s.cache.IsPasswordResetEmailInCooldown(ctx, email) {
log.Printf("[Email] Password reset email skipped (cooldown): %s", email)
slog.Info("password reset email skipped due to cooldown", "email", email)
return nil // Silent success to prevent revealing cooldown to attackers
}
@@ -467,7 +467,7 @@ func (s *EmailService) SendPasswordResetEmailWithCooldown(ctx context.Context, e
// Set cooldown marker (Redis TTL handles expiration)
if err := s.cache.SetPasswordResetEmailCooldown(ctx, email, passwordResetEmailCooldown); err != nil {
log.Printf("[Email] Failed to set password reset cooldown for %s: %v", email, err)
slog.Error("failed to set password reset cooldown", "email", email, "error", err)
}
return nil
@@ -497,7 +497,7 @@ func (s *EmailService) ConsumePasswordResetToken(ctx context.Context, email, tok
// Delete after verification (one-time use)
if err := s.cache.DeletePasswordResetToken(ctx, email); err != nil {
log.Printf("[Email] Failed to delete password reset token after consumption: %v", err)
slog.Error("failed to delete password reset token after consumption", "email", email, "error", err)
}
return nil
}
@@ -6,7 +6,7 @@ import (
)
// NotifyEmailEntry represents a notification email with enable/disable and verification state.
// Email="" is a placeholder for the "primary email" (user's registration email or first admin email).
// All emails are user-managed; maximum 3 entries per user.
type NotifyEmailEntry struct {
Email string `json:"email"`
Disabled bool `json:"disabled"`
+1 -1
View File
@@ -20,7 +20,7 @@ var (
)
const (
maxNotifyEmails = 3 // Total limit: primary (email="") + up to 2 extra
maxNotifyEmails = 3 // Maximum number of notification emails per user
// User-level rate limiting for notify email verification codes
notifyCodeUserRateLimit = 5
+129 -67
View File
@@ -421,7 +421,7 @@
</h4>
<button
type="button"
@click="addAccountStatsRule()"
@click="addAccountStatsRule(sIdx)"
class="rounded-lg border border-primary-300 px-3 py-1 text-xs font-medium text-primary-600 hover:bg-primary-50 dark:border-primary-600 dark:text-primary-400 dark:hover:bg-primary-900/20"
>
+ {{ t('admin.channels.form.addRule') }}
@@ -430,14 +430,14 @@
<!-- Filter rules for this platform's groups -->
<p
v-if="form.account_stats_pricing_rules.length === 0"
v-if="section.account_stats_pricing_rules.length === 0"
class="text-xs italic text-gray-400 dark:text-gray-500"
>
{{ t('admin.channels.form.noRulesConfigured') }}
</p>
<div
v-for="(rule, ruleIndex) in form.account_stats_pricing_rules"
v-for="(rule, ruleIndex) in section.account_stats_pricing_rules"
:key="ruleIndex"
class="space-y-3 rounded-lg border border-gray-200 p-4 dark:border-dark-600"
>
@@ -447,7 +447,7 @@
:placeholder="t('admin.channels.form.ruleName')"
class="bg-transparent text-sm font-medium text-gray-700 placeholder-gray-400 outline-none dark:text-gray-300"
/>
<button type="button" @click="removeAccountStatsRule(ruleIndex)" class="text-xs text-red-500 hover:text-red-700">
<button type="button" @click="removeAccountStatsRule(sIdx, ruleIndex)" class="text-xs text-red-500 hover:text-red-700">
{{ t('common.delete') }}
</button>
</div>
@@ -524,7 +524,7 @@
<div>
<div class="mb-1 flex items-center justify-between">
<label class="text-xs text-gray-500 dark:text-gray-400">{{ t('admin.channels.form.ruleModelPricing') }}</label>
<button type="button" @click="addRulePricingEntry(ruleIndex)" class="text-xs text-primary-600 hover:text-primary-700">
<button type="button" @click="addRulePricingEntry(sIdx, ruleIndex)" class="text-xs text-primary-600 hover:text-primary-700">
+ {{ t('common.add') }}
</button>
</div>
@@ -538,7 +538,7 @@
:entry="entry"
:platform="section.platform"
@update="rule.pricing.splice(pIdx, 1, $event)"
@remove="removeRulePricingEntry(ruleIndex, pIdx)"
@remove="removeRulePricingEntry(sIdx, ruleIndex, pIdx)"
/>
</div>
</div>
@@ -625,6 +625,14 @@ async function loadWebSearchGlobalState() {
}
}
// ── Form-level pricing rule type (per-platform) ──
interface FormPricingRule {
name: string
group_ids: number[]
account_ids: number[]
pricing: PricingFormEntry[]
}
// ── Platform Section type ──
interface PlatformSection {
platform: GroupPlatform
@@ -634,6 +642,7 @@ interface PlatformSection {
model_mapping: Record<string, string>
model_pricing: PricingFormEntry[]
web_search_emulation: boolean
account_stats_pricing_rules: FormPricingRule[]
}
// ── Table columns ──
@@ -703,12 +712,6 @@ const form = reactive({
billing_model_source: 'channel_mapped' as string,
platforms: [] as PlatformSection[],
apply_pricing_to_account_stats: false,
account_stats_pricing_rules: [] as Array<{
name: string
group_ids: number[]
account_ids: number[]
pricing: PricingFormEntry[]
}>
})
let abortController: AbortController | null = null
@@ -754,6 +757,7 @@ function addPlatformSection(platform: GroupPlatform) {
model_mapping: {},
model_pricing: [],
web_search_emulation: false,
account_stats_pricing_rules: [],
})
}
@@ -867,8 +871,8 @@ function renameMappingKey(sectionIdx: number, oldKey: string, newKey: string) {
}
// ── Account Stats Pricing helpers ──
function addAccountStatsRule() {
form.account_stats_pricing_rules.push({
function addAccountStatsRule(sectionIdx: number) {
form.platforms[sectionIdx].account_stats_pricing_rules.push({
name: '',
group_ids: [],
account_ids: [],
@@ -876,8 +880,8 @@ function addAccountStatsRule() {
})
}
function addRulePricingEntry(ruleIndex: number) {
form.account_stats_pricing_rules[ruleIndex].pricing.push({
function addRulePricingEntry(sectionIdx: number, ruleIndex: number) {
form.platforms[sectionIdx].account_stats_pricing_rules[ruleIndex].pricing.push({
models: [],
billing_mode: 'token',
input_price: null,
@@ -890,15 +894,15 @@ function addRulePricingEntry(ruleIndex: number) {
})
}
function removeAccountStatsRule(ruleIndex: number) {
form.account_stats_pricing_rules.splice(ruleIndex, 1)
function removeAccountStatsRule(sectionIdx: number, ruleIndex: number) {
form.platforms[sectionIdx].account_stats_pricing_rules.splice(ruleIndex, 1)
// Clear all search state since indices shift after removal
ruleAccountSearchRunner.clearAll()
clearAllRuleAccountSearchState()
}
function removeRulePricingEntry(ruleIndex: number, pricingIndex: number) {
form.account_stats_pricing_rules[ruleIndex].pricing.splice(pricingIndex, 1)
function removeRulePricingEntry(sectionIdx: number, ruleIndex: number, pricingIndex: number) {
form.platforms[sectionIdx].account_stats_pricing_rules[ruleIndex].pricing.splice(pricingIndex, 1)
}
function getGroupNameById(groupId: number): string {
@@ -980,38 +984,33 @@ function clearAllRuleAccountSearchState() {
showRuleAccountDropdown.value = {}
}
function inferRulePlatform(groupIds: number[]): string {
const platforms = new Set<string>()
for (const gid of groupIds) {
const group = allGroups.value.find(g => g.id === gid)
if (group) platforms.add(group.platform)
}
return platforms.size === 1 ? [...platforms][0] : ''
}
function accountStatsRulesToAPI(): AccountStatsPricingRule[] {
return form.account_stats_pricing_rules.map(rule => {
const platform = inferRulePlatform(rule.group_ids)
return {
name: rule.name,
group_ids: rule.group_ids,
account_ids: rule.account_ids,
pricing: rule.pricing
.filter(p => p.models.length > 0)
.map(p => ({
platform,
models: p.models,
billing_mode: p.billing_mode,
input_price: mTokToPerToken(p.input_price),
output_price: mTokToPerToken(p.output_price),
cache_write_price: mTokToPerToken(p.cache_write_price),
cache_read_price: mTokToPerToken(p.cache_read_price),
image_output_price: mTokToPerToken(p.image_output_price),
per_request_price: p.per_request_price != null && p.per_request_price !== '' ? Number(p.per_request_price) : null,
intervals: formIntervalsToAPI(p.intervals || [])
}))
const rules: AccountStatsPricingRule[] = []
for (const section of form.platforms) {
if (!section.enabled) continue
for (const rule of section.account_stats_pricing_rules) {
rules.push({
name: rule.name,
group_ids: rule.group_ids,
account_ids: rule.account_ids,
pricing: rule.pricing
.filter(p => p.models.length > 0)
.map(p => ({
platform: section.platform,
models: p.models,
billing_mode: p.billing_mode,
input_price: mTokToPerToken(p.input_price),
output_price: mTokToPerToken(p.output_price),
cache_write_price: mTokToPerToken(p.cache_write_price),
cache_read_price: mTokToPerToken(p.cache_read_price),
image_output_price: mTokToPerToken(p.image_output_price),
per_request_price: p.per_request_price != null && p.per_request_price !== '' ? Number(p.per_request_price) : null,
intervals: formIntervalsToAPI(p.intervals || [])
}))
})
}
})
}
return rules
}
// ── Form ↔ API conversion ──
@@ -1120,6 +1119,7 @@ function apiToForm(channel: Channel): PlatformSection[] {
model_mapping: { ...mapping },
model_pricing: pricing,
web_search_emulation: webSearchEnabled,
account_stats_pricing_rules: [],
})
}
@@ -1213,7 +1213,6 @@ function resetForm() {
form.billing_model_source = 'channel_mapped'
form.platforms = []
form.apply_pricing_to_account_stats = false
form.account_stats_pricing_rules = []
activeTab.value = 'basic'
ruleAccountSearchRunner.clearAll()
clearAllRuleAccountSearchState()
@@ -1235,28 +1234,91 @@ async function openEditDialog(channel: Channel) {
form.restrict_models = channel.restrict_models || false
form.billing_model_source = channel.billing_model_source || 'channel_mapped'
form.apply_pricing_to_account_stats = channel.apply_pricing_to_account_stats || false
form.account_stats_pricing_rules = (channel.account_stats_pricing_rules || []).map(rule => ({
name: rule.name || '',
group_ids: [...(rule.group_ids || [])],
account_ids: [...(rule.account_ids || [])],
pricing: (rule.pricing || []).map(p => ({
models: [...(p.models || [])],
billing_mode: p.billing_mode,
input_price: perTokenToMTok(p.input_price),
output_price: perTokenToMTok(p.output_price),
cache_write_price: perTokenToMTok(p.cache_write_price),
cache_read_price: perTokenToMTok(p.cache_read_price),
image_output_price: perTokenToMTok(p.image_output_price),
per_request_price: p.per_request_price,
intervals: apiIntervalsToForm(p.intervals || [])
} as PricingFormEntry))
}))
// Must load groups first so apiToForm can map groupID → platform
await Promise.all([loadGroups(), loadAllChannelsForConflict()])
form.platforms = apiToForm(channel)
// Distribute channel-level rules into per-platform sections
distributeRulesToPlatforms(channel.account_stats_pricing_rules || [])
// Populate ruleAccountNameCache for existing rule accounts
await populateRuleAccountNameCache()
showDialog.value = true
}
/** Distribute flat channel-level rules into the matching platform section based on group_ids */
function distributeRulesToPlatforms(apiRules: AccountStatsPricingRule[]) {
// Build groupID → platform lookup
const groupPlatformMap = new Map<number, GroupPlatform>()
for (const g of allGroups.value) {
groupPlatformMap.set(g.id, g.platform)
}
for (const apiRule of apiRules) {
// Infer platform from group_ids
const platforms = new Set<GroupPlatform>()
for (const gid of apiRule.group_ids || []) {
const p = groupPlatformMap.get(gid)
if (p) platforms.add(p)
}
// If pricing has a platform field, use that as fallback
if (platforms.size === 0 && apiRule.pricing?.length > 0) {
const p = apiRule.pricing[0].platform as GroupPlatform | undefined
if (p) platforms.add(p)
}
const targetPlatform = platforms.size >= 1 ? [...platforms][0] : null
if (!targetPlatform) continue
const section = form.platforms.find(s => s.platform === targetPlatform)
if (!section) continue
const formRule: FormPricingRule = {
name: apiRule.name || '',
group_ids: [...(apiRule.group_ids || [])],
account_ids: [...(apiRule.account_ids || [])],
pricing: (apiRule.pricing || []).map(p => ({
models: [...(p.models || [])],
billing_mode: p.billing_mode,
input_price: perTokenToMTok(p.input_price),
output_price: perTokenToMTok(p.output_price),
cache_write_price: perTokenToMTok(p.cache_write_price),
cache_read_price: perTokenToMTok(p.cache_read_price),
image_output_price: perTokenToMTok(p.image_output_price),
per_request_price: p.per_request_price,
intervals: apiIntervalsToForm(p.intervals || [])
} as PricingFormEntry))
}
section.account_stats_pricing_rules.push(formRule)
}
}
/** Populate ruleAccountNameCache by fetching account details for all account_ids in rules */
async function populateRuleAccountNameCache() {
const allAccountIds = new Set<number>()
for (const section of form.platforms) {
for (const rule of section.account_stats_pricing_rules) {
for (const id of rule.account_ids) {
allAccountIds.add(id)
}
}
}
if (allAccountIds.size === 0) return
// Fetch account details in parallel (batch of individual getById calls)
const ids = [...allAccountIds]
const results = await Promise.allSettled(
ids.map(id => adminAPI.accounts.getById(id))
)
for (let i = 0; i < ids.length; i++) {
const result = results[i]
if (result.status === 'fulfilled') {
ruleAccountNameCache.value[ids[i]] = result.value.name
}
// If rejected, the cache won't have the name, so it'll show "#ID" which is acceptable
}
}
function closeDialog() {
showDialog.value = false
editingChannel.value = null