Conversation
… detection - BUILTIN_CLIENT_KEYWORDS set (7 keywords) - 3-of-3 signal detection: x-app, UA prefix, betas array - Sub-client extraction via ENTRYPOINT_MAP - matchClientPattern: builtin keyword path + custom UA substring path - isClientAllowed: blacklist-before-whitelist logic - 48 unit tests, all passing
…oundation layer - types/user.ts: blockedClients in 5 interfaces - types/provider.ts: allowedClients/blockedClients in Provider, ProviderDisplay, batch types - types/message.ts: client_restriction_filtered in ProviderChainItem.reason, client_restriction in filteredProviders.reason - drizzle/schema.ts: blocked_clients on users, allowed_clients/blocked_clients on providers - validation/schemas.ts: blockedClients in user schemas, allowed_clients/blocked_clients in provider schemas - actions/providers.ts: thread new fields through - lib/provider-patch-contract.ts: add allowed_clients/blocked_clients to patch contract
…ep 1.5, repository fields - client-guard.ts: use isClientAllowed from client-detector, add blockedClients support - provider-selector.ts: Step 1.5 client restriction pre-filter (neutral, no circuit breaker) - repository/user.ts: blockedClients read/write - repository/provider.ts: allowedClients/blockedClients read/write - repository/_shared/transformers.ts: blockedClients in transformer - types/provider.ts: additional batch type entries - tests: 78 tests passing (guard + detector)
…nts through actions - messages/*/provider-chain.json: client_restriction in filterReasons/filterDetails, client_restriction_filtered in reasons - messages/*/dashboard.json: 7 Claude Code sub-client labels in presetClients, blockedClients in userEditSection.fields - actions/users.ts: blockedClients in all create/update calls - actions/providers.ts: allowedClients/blockedClients in all create/update calls
…e Code panel and blockedClients - Rewrite access-restrictions-section.tsx with Allow/Block toggles for all 7 Claude Code sub-clients and 3 other presets (gemini-cli, factory-cli, codex-cli) - Add blockedClients prop to AccessRestrictionsSectionProps and UserEditSectionProps - Add custom blockedClients tag input alongside existing allowedClients tag input - Update use-user-translations with blockedClients i18n fields and all claude-code preset translations - Wire blockedClients through create-user-dialog and edit-user-dialog
- Add allowedClients/blockedClients to RoutingState and action types - Initialize and handle new fields in provider form context/reducer - Render two TagInput fields in routing section for client restrictions - Map new fields to API payload in form submission - Add i18n keys for all 5 languages (en, zh-CN, zh-TW, ja, ru)
…Code sub-client labels
|
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:
📝 WalkthroughWalkthrough在数据库、后端、代理与前端中新增并传播客户端级别的允许/阻止列表（users.blockedClients、providers.allowedClients、providers.blockedClients）；实现客户端检测/isClientAllowed 逻辑并在 provider 选择与守卫中应用；扩展类型、验证、表单、翻译与单元测试，新增若干索引迁移。 Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
Contributor
Summary of ChangesHello @ding113, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances API access control by introducing a robust client restriction system. It allows for fine-grained management of client access at both the user and provider levels, incorporating a sophisticated detection mechanism for various Claude Code sub-clients. The changes ensure that API requests can be precisely filtered based on client identity, improving security and operational flexibility. The user interface has been updated to provide intuitive controls for configuring these new restrictions. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
github-actions
bot
added
enhancement
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
160
to
173
| Allow | ||
| </Label> | ||
| </div> | ||
| <div className="flex items-center gap-1.5"> | ||
| <Checkbox | ||
| id={`block-${value}`} | ||
| checked={isBlocked} | ||
| onCheckedChange={(checked) => handleBlockToggle(value, checked === true)} | ||
| /> | ||
| <Label | ||
| htmlFor={`block-${value}`} | ||
| className="text-xs font-normal cursor-pointer text-muted-foreground" | ||
| > | ||
| Block |
There was a problem hiding this comment.
Hardcoded "Allow"/"Block" labels bypass i18n
The labels "Allow" (line 160) and "Block" (line 173) are hardcoded English strings, but this app supports 5 languages (en, zh-CN, zh-TW, ja, ru). These should be passed through the translations prop or pulled from a translation key, similar to how other labels in this component use translations.fields.allowedClients.label etc.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/[locale]/dashboard/_components/user/forms/access-restrictions-section.tsx
Line: 160-173
Comment:
**Hardcoded "Allow"/"Block" labels bypass i18n**
The labels "Allow" (line 160) and "Block" (line 173) are hardcoded English strings, but this app supports 5 languages (en, zh-CN, zh-TW, ja, ru). These should be passed through the `translations` prop or pulled from a translation key, similar to how other labels in this component use `translations.fields.allowedClients.label` etc.
How can I resolve this? If you propose a fix, please make it concise.![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a new client restriction feature, allowing both 'allowed clients' and 'blocked clients' to be configured for users and providers. Database schema migrations add allowed_clients and blocked_clients JSONB columns to the providers table and blocked_clients to the users table. The UI has been updated across multiple languages to include input fields for these new client restriction lists, along with expanded preset client options, particularly for various Claude Code clients. Backend actions and validation schemas have been modified to support these new fields for user and provider management. A new client-detector.ts utility was added to centralize client detection logic, including specific handling for Claude Code clients based on multiple request signals (User-Agent, x-app header, betas). The ProxyClientGuard and ProxyProviderResolver were updated to enforce these client restrictions, rejecting requests or filtering providers based on the configured allow/block lists. Logging for the provider chain now includes a client_restriction_filtered reason. Review comments highlighted the need for null checks when accessing request message properties, suggested batching state updates in the UI to prevent race conditions and multiple re-renders, and recommended optimizing the isClientAllowed function in client-detector.ts to avoid redundant computations for each pattern.
| signals.push("ua-prefix"); | ||
| } | ||
|
|
||
| const betas = (session.request.message as any).betas; |
Contributor
There was a problem hiding this comment.
Accessing betas on session.request.message without a null check can lead to a runtime crash if the request body is not present or not a valid object (e.g., in non-JSON requests or malformed payloads).
Suggested change
| const betas = (session.request.message as any).betas; | |
| const betas = (session.request.message as any)?.betas; |
Comment on lines
91
to
104
| const handleAllowToggle = (clientValue: string, checked: boolean) => { | ||
| if (checked) { | ||
| onChange("allowedClients", [...currentClients, clientValue]); | ||
| onChange("allowedClients", [...allowed.filter((c) => c !== clientValue), clientValue]); | ||
| onChange( | ||
| "blockedClients", | ||
| blocked.filter((c) => c !== clientValue) | ||
| ); | ||
| } else { | ||
| onChange( | ||
| "allowedClients", | ||
| currentClients.filter((c) => c !== clientValue) | ||
| allowed.filter((c) => c !== clientValue) | ||
| ); | ||
| } | ||
| }; |
Contributor
There was a problem hiding this comment.
Calling onChange twice in succession for related fields (allowedClients and blockedClients) can lead to race conditions and inconsistent state if the parent's state updates are not synchronous or functional. It also triggers unnecessary multiple re-renders. Use a single batch update instead.
const handleAllowToggle = (clientValue: string, checked: boolean) => {
if (checked) {
onChange({
allowedClients: [...allowed.filter((c) => c !== clientValue), clientValue],
blockedClients: blocked.filter((c) => c !== clientValue),
});
} else {
onChange("allowedClients", allowed.filter((c) => c !== clientValue));
}
};
|
|
||
| export function matchClientPattern(session: ProxySession, pattern: string): boolean { | ||
| if (!isBuiltinKeyword(pattern)) { | ||
| const normalize = (s: string) => s.toLowerCase().replace(/[-_]/g, ""); |
Contributor
There was a problem hiding this comment.
The normalize function is duplicated in matchClientPattern and detectClientFull. It should be moved to the top level of the module to improve maintainability and allow for reuse in optimized loops.
const normalize = (s: string) => s.toLowerCase().replace(/[-_]/g, "");
export function matchClientPattern(session: ProxySession, pattern: string): boolean {
Comment on lines
140
to
157
| export function isClientAllowed( | ||
| session: ProxySession, | ||
| allowedClients: string[], | ||
| blockedClients?: string[] | ||
| ): boolean { | ||
| if (blockedClients && blockedClients.length > 0) { | ||
| const isBlocked = blockedClients.some((pattern) => matchClientPattern(session, pattern)); | ||
| if (isBlocked) { | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| if (allowedClients.length === 0) { | ||
| return true; | ||
| } | ||
|
|
||
| return allowedClients.some((pattern) => matchClientPattern(session, pattern)); | ||
| } |
Contributor
There was a problem hiding this comment.
The current implementation of isClientAllowed is inefficient when multiple patterns are provided. It re-runs Claude Code signal detection and User-Agent normalization for every pattern in the list. Since this function is called for every provider during selection, it should be refactored to pre-calculate these values once per call.
export function isClientAllowed(
session: ProxySession,
allowedClients: string[],
blockedClients?: string[]
): boolean {
if (allowedClients.length === 0 && (!blockedClients || blockedClients.length === 0)) {
return true;
}
const claudeCode = confirmClaudeCodeSignals(session);
const ua = session.userAgent?.trim();
const normalizedUA = ua ? normalize(ua) : "";
const subClient = claudeCode.confirmed ? extractSubClient(ua ?? "") : null;
const matches = (pattern: string): boolean => {
if (isBuiltinKeyword(pattern)) {
return claudeCode.confirmed && (pattern === CLAUDE_CODE_KEYWORD_PREFIX || subClient === pattern);
}
if (!normalizedUA) return false;
const normalizedPattern = normalize(pattern);
return normalizedPattern !== "" && normalizedUA.includes(normalizedPattern);
};
if (blockedClients?.some(matches)) return false;
return allowedClients.length === 0 || allowedClients.some(matches);
}| allowedModels: string[]; | ||
| modelSuggestions: string[]; | ||
| onChange: (field: "allowedClients" | "allowedModels", value: string[]) => void; | ||
| onChange: (field: "allowedClients" | "blockedClients" | "allowedModels", value: string[]) => void; |
Contributor
There was a problem hiding this comment.
The onChange prop type should be updated to support batch updates (passing an object). This is necessary to ensure atomic updates when a toggle affects both allowed and blocked lists simultaneously.
Suggested change
| onChange: (field: "allowedClients" | "blockedClients" | "allowedModels", value: string[]) => void; | |
| onChange: (field: any, value?: any) => void; |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
messages/zh-CN/dashboard.json (1)
1813-1825:⚠️ Potential issue | 🟡 Minor
claude-cli与claude-code-cli的显示名称重复，用户无法区分
userEditSection.presetClients中现有条目"claude-cli": "Claude Code CLI"与新增的"claude-code-cli": "Claude Code CLI"呈现相同的标签名称。虽然这两个键在架构中有不同的用途（claude-cli是实际 CLI 应用的 User-Agent 标识符，claude-code-cli是新框架中的 claude-code 子客户端模式），但在 UI 中显示相同的标签会造成用户混淆。建议将
claude-cli的显示名称加以区分，标注其为旧版：-"claude-cli": "Claude Code CLI", +"claude-cli": "Claude Code CLI（旧版）", "gemini-cli": "Gemini CLI",或者，如果
claude-cli已在新框架中弃用，考虑移除该项并在使用处同步更新。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@messages/zh-CN/dashboard.json` around lines 1813 - 1825, The two entries in userEditSection.presetClients use identical display text which confuses users: update the label for "claude-cli" (currently "Claude Code CLI") to a distinct name such as "Claude CLI (legacy)" or similar so it is clearly differentiated from "claude-code-cli" (which should remain "Claude Code CLI"); alternatively, if "claude-cli" is deprecated, remove the "claude-cli" key and ensure any references to that User-Agent identifier are updated accordingly.src/app/[locale]/dashboard/_components/user/edit-user-dialog.tsx (1)
49-55:⚠️ Potential issue | 🔴 Critical添加缺失的
blockedClients字段到编辑用户表单的三个关键环节用户编辑对话框中的黑名单客户端字段存在完全相同的三处遗漏，导致已保存的黑名单在打开对话框时丢失、编辑时无法跟踪、提交时被静默丢弃：
环节 allowedClients（正确）blockedClients（缺失）EditUserSchema（第 49-55 行）✅ 已定义 ❌ 缺失 buildDefaultValues（第 59-78 行）✅ user.allowedClients || []❌ 缺失 user.blockedClients || []editUser调用（第 100-117 行）✅ data.allowedClients❌ 缺失 data.blockedClients第 273 行
UserEditSection接收的currentUserDraft.blockedClients因 Schema 中不存在该字段，始终为undefined。修复方案
const EditUserSchema = UpdateUserSchema.extend({ name: z.string().min(1).max(64), providerGroup: z.string().max(200).nullable().optional(), allowedClients: z.array(z.string().max(64)).max(50).optional().default([]), + blockedClients: z.array(z.string().max(64)).max(50).optional().default([]), allowedModels: z.array(z.string().max(64)).max(50).optional().default([]), dailyQuota: z.number().nullable().optional(), });function buildDefaultValues(user: UserDisplay): EditUserValues { return { ... allowedClients: user.allowedClients || [], + blockedClients: user.blockedClients || [], allowedModels: user.allowedModels || [], }; }const userRes = await editUser(user.id, { ... allowedClients: data.allowedClients, + blockedClients: data.blockedClients, allowedModels: data.allowedModels, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/dashboard/_components/user/edit-user-dialog.tsx around lines 49 - 55, The EditUser form is missing the blockedClients field in three places causing saved blacklists to be lost; add blockedClients handling to the Zod schema, default building, and submit payload: extend EditUserSchema (alongside allowedClients) with blockedClients: z.array(z.string().max(64)).max(50).optional().default([]) so currentUserDraft.blockedClients is populated, update buildDefaultValues to set blockedClients: user.blockedClients || [], and include data.blockedClients when calling editUser so the server receives the updated blacklist (affecting UserEditSection's currentUserDraft.blockedClients).src/app/[locale]/dashboard/_components/user/create-user-dialog.tsx (1)
41-47:⚠️ Potential issue | 🔴 Critical
blockedClients在数据链路中缺失，导致用户设置的黑名单客户端无法持久化虽然
blockedClients字段已在数据库、类型定义和UpdateUserSchema中定义，但在创建用户的表单流程中存在两处遗漏，使得 UI 中编辑的黑名单数据被静默丢弃：
buildDefaultValues()缺失初始化 （第 74-112 行）
- 初始化
allowedClients: []✓- 初始化
allowedModels: []✓- 缺少
blockedClients: []❌ → 导致currentUserDraft.blockedClients始终为undefined
createUserOnly()调用缺失参数 （第 143-159 行）
- 传入
allowedClients: data.user.allowedClients✓- 传入
allowedModels: data.user.allowedModels✓- 缺少
blockedClients: data.user.blockedClients❌ → 即使表单有数据也被丢弃第 366 行虽然已将
blockedClients传给UserEditSection做展示，但由于上述数据流断裂，用户编辑的内容无法被保存。🐛 建议修复
function buildDefaultValues(): CreateFormValues { return { user: { ... allowedClients: [], + blockedClients: [], allowedModels: [], },const userRes = await createUserOnly({ ... allowedClients: data.user.allowedClients, + blockedClients: data.user.blockedClients, allowedModels: data.user.allowedModels, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/dashboard/_components/user/create-user-dialog.tsx around lines 41 - 47, The create-user flow is dropping the blockedClients field: update buildDefaultValues() to initialize blockedClients: [] alongside allowedClients/allowedModels so currentUserDraft.blockedClients is never undefined, and update createUserOnly(...) call to include blockedClients: data.user.blockedClients when constructing the payload so the form value is persisted; confirm CreateUserSchema already supports blockedClients and that UserEditSection continues to receive the field for display.src/lib/provider-patch-contract.ts (1)
26-36:⚠️ Potential issue | 🟠 Major新增字段缺少 set 校验，导致合法补丁被拒
allowed_clients/blocked_clients已加入 PATCH_FIELDS，但isValidSetValue未新增对应分支，set模式会被判为非法，进而使normalizeProviderBatchPatchDraft失败。需要补充校验逻辑。建议修复
@@ case "allowed_models": return Array.isArray(value) && value.every((model) => typeof model === "string"); + case "allowed_clients": + case "blocked_clients": + return Array.isArray(value) && value.every((client) => typeof client === "string");Also applies to: 206-275
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/provider-patch-contract.ts` around lines 26 - 36, PATCH_FIELDS was updated to include "allowed_clients" and "blocked_clients" but isValidSetValue lacks branches to accept set-mode values for these fields causing normalizeProviderBatchPatchDraft to reject valid patches; update isValidSetValue to handle "allowed_clients" and "blocked_clients" by allowing Set-like inputs (e.g., arrays/sets or the same input shape accepted for "allowed_models") and add corresponding validation rules, and ensure normalizeProviderBatchPatchDraft continues to accept and normalize these fields in set mode.
🧹 Nitpick comments (6)
src/app/v1/_lib/proxy/client-detector.ts (2)
48-54: 建议将as any替换为更安全的Record<string, unknown>类型断言
as any完全绕过了类型检查。考虑到betas字段后续已经通过Array.isArray和typeof做了运行时校验，可以使用更保守的断言：♻️ 建议修改
-const betas = (session.request.message as any).betas; +const betas = (session.request.message as Record<string, unknown>).betas;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/v1/_lib/proxy/client-detector.ts` around lines 48 - 54, Replace the unsafe cast "as any" when extracting betas from session.request.message with a narrower type like Record<string, unknown> and treat the extracted value as unknown before runtime checks; specifically, change the extraction in client-detector.ts so you cast session.request.message to Record<string, unknown> (or a dedicated Message type) and read the "betas" property from that object, then keep the existing Array.isArray and typeof checks (the betas.some check and the /^claude-code-/i test) to narrow the type and push "betas-claude-code" into signals only after those runtime validations succeed.
140-156:confirmClaudeCodeSignals对同一 session 会被多次调用当
allowedClients/blockedClients包含多个 builtin 关键字时，isClientAllowed通过.some()循环调用matchClientPattern，每次遇到 builtin 模式都会调用confirmClaudeCodeSignals（header/request 读取）。由于 session 在请求生命周期内不变，结果每次都相同。对于小规模模式列表（典型使用场景）性能影响可忽略，但如果未来模式列表较长，可考虑在
isClientAllowed入口处调用一次并复用结果：♻️ 建议优化（可选）
export function isClientAllowed( session: ProxySession, allowedClients: string[], blockedClients?: string[] ): boolean { + // 对 builtin 模式预计算一次 Claude Code 信号确认结果 + // 避免在 .some() 循环中重复读取 headers/request if (blockedClients && blockedClients.length > 0) { const isBlocked = blockedClients.some((pattern) => matchClientPattern(session, pattern));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/v1/_lib/proxy/client-detector.ts` around lines 140 - 156, isClientAllowed currently may call matchClientPattern multiple times per session which in turn can invoke confirmClaudeCodeSignals repeatedly; compute and cache the session-specific client signal once and reuse it: inside isClientAllowed (with parameters session, allowedClients, blockedClients) call confirmClaudeCodeSignals(session) once (or compute a memoized flag on the provided ProxySession) and have matchClientPattern consult that cached result instead of re-reading headers each iteration so both the blockedClients.some(...) and allowedClients.some(...) loops reuse the single computed value.tests/unit/proxy/client-guard.test.ts (1)
234-247: 缺少"仅 blockedClients + 无 UA"的测试用例
when only blockedClients is configured分组目前只测试了有 UA 的情况。若无 UA 时（undefined），当前实现会返回 400 "UA required"，而非在黑名单未命中后放行。建议补充该边界用例，使预期行为明确可见：test("should reject with UA-required when UA is missing, even with only blockedClients", async () => { const session = createMockSession(undefined, [], ["codex-cli"]); const result = await ProxyClientGuard.ensure(session); // 当前行为：400（UA required） // 若修复为仅黑名单时不强制 UA，则应改为 null（允许） expect(result).not.toBeNull(); expect(result?.status).toBe(400); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/proxy/client-guard.test.ts` around lines 234 - 247, Add a unit test under the "when only blockedClients is configured (no allowedClients)" describe block that creates a session with undefined UA using createMockSession(undefined, [], ["codex-cli"]) and calls ProxyClientGuard.ensure(session); assert the current behavior by expecting a non-null result with status 400 (UA required) so the edge case is explicitly covered and the expected behavior is pinned down for future changes to ProxyClientGuard.ensure.src/app/[locale]/settings/providers/_components/forms/provider-form/sections/routing-section.tsx (1)
158-238: 客户端限制 FieldGroup 被放置在"Model Configuration" SectionCard 内，语义不当客户端限制（允许/阻止哪些客户端访问此 provider）是独立的访问控制概念，与模型白名单（允许哪些 AI 模型）属于不同维度。当前将其置于以
modelWhitelist.title/desc为标题的 SectionCard 中，会让用户误以为两者相关。建议将客户端限制提取为独立的
SectionCard（参考ScheduleParams或AdvancedSettings等其他 card 的写法），使功能分区更清晰。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/settings/providers/_components/forms/provider-form/sections/routing-section.tsx around lines 158 - 238, Move the two client restriction FieldGroup blocks (the TagInput groups using state.routing.allowedClients and state.routing.blockedClients) out of the existing SectionCard titled by t("sections.routing.modelWhitelist.title") and into their own SectionCard component; remove those FieldGroup nodes from the Model Configuration SectionCard and create a new SectionCard (e.g., title t("sections.routing.clientRestrictions.title") and description t("sections.routing.clientRestrictions.desc")) that contains the two FieldGroup entries using the same TagInput props and dispatch actions ("SET_ALLOWED_CLIENTS" and "SET_BLOCKED_CLIENTS"); preserve all state references (state.routing.allowedClients, state.routing.blockedClients), placeholders (t("sections.routing.clientRestrictions.allowedPlaceholder"/"blockedPlaceholder")) and any existing classes so UI/behavior remains unchanged.src/app/[locale]/dashboard/logs/_components/provider-chain-popover.tsx (1)
107-113:getItemStatus中client_restriction_filtered分支为不可达死代码
isActualRequest()在第 37 行对"client_restriction_filtered"直接返回false，意味着这类 item 永远不会进入actualRequests（第 374 行chain.filter(isActualRequest)）。而getItemStatus只在actualRequests.map(...)内部被调用（第 456 行），因此第 107–113 行永远不会执行，MinusCircle的导入也随之成为无效引用。如果设计意图是不在时间线中展示这类过滤事件，可直接删除该分支以消除混淆；若将来需要在 UI 中展示，则应同时修改
isActualRequest的逻辑或单独渲染过滤项。♻️ 建议清理
-import { - AlertTriangle, - CheckCircle, - ChevronRight, - InfoIcon, - Link2, - MinusCircle, - RefreshCw, - XCircle, - Zap, -} from "lucide-react"; +import { + AlertTriangle, + CheckCircle, + ChevronRight, + InfoIcon, + Link2, + RefreshCw, + XCircle, + Zap, +} from "lucide-react";- if (item.reason === "client_restriction_filtered") { - return { - icon: MinusCircle, - color: "text-muted-foreground", - bgColor: "bg-muted/30", - }; - }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/dashboard/logs/_components/provider-chain-popover.tsx around lines 107 - 113, The branch in getItemStatus handling "client_restriction_filtered" is unreachable because isActualRequest returns false for that type and actualRequests is created via chain.filter(isActualRequest), so that case (and the MinusCircle import) should be removed to avoid dead code; alternatively, if you intend to show filtered events, update isActualRequest to include "client_restriction_filtered" (and ensure actualRequests/where it’s mapped will include and render those items) and keep the getItemStatus branch—pick one approach and apply it consistently (adjust getItemStatus, isActualRequest, actualRequests mapping and imports accordingly).src/drizzle/schema.ts (1)
75-78:blockedClients列定义正确，但与allowedClients存在.notNull()不一致。新增的
blockedClients正确使用了.notNull().default([])，但同表中已有的allowedClients（第 69 行）缺少.notNull()。这意味着allowedClients在数据库中可为 NULL，而blockedClients不可为 NULL。虽然运行时代码通过
user.allowedClients || []做了兜底，但建议后续统一两者的约束。此处不阻塞本次 PR。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/drizzle/schema.ts` around lines 75 - 78, The schema defines blockedClients with .notNull().default([]) but allowedClients lacks .notNull(), causing inconsistent nullability; update the allowedClients column definition in src/drizzle/schema.ts (the allowedClients entry alongside blockedClients) to include .notNull().default([]) so both columns use the same non-null default behavior, then regenerate/apply the DB migration to keep schema and runtime assumptions consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@messages/en/dashboard.json`:
- Around line 1855-1865: The two entries "claude-cli" and "claude-code-cli" in
messages/en/dashboard.json both map to the same display name, causing admin
confusion; change the display value for "claude-code-cli" to a distinct,
descriptive label (e.g., "Claude Code CLI (builtin / strict detection)") or
append a clarifying suffix like "(builtin: header+UA+betas)" so admins can tell
it uses the new three-signal builtin check, and/or update the allowedClients
description text to explicitly document the difference between "claude-cli" (UA
substring match) and "claude-code-cli" (x-app header + UA prefix + betas).
In
`@src/app/`[locale]/dashboard/_components/user/forms/access-restrictions-section.tsx:
- Around line 141-174: The "Allow" and "Block" label text in renderPresetRow are
hardcoded; update the component to use translation keys from the existing
translations object (e.g., translations.allow and translations.block or whatever
convention the file uses) instead of the literal strings, wire those translated
values into the Label components rendered next to the Checkbox inputs (keep
Checkbox ids and handlers handleAllowToggle and handleBlockToggle unchanged),
and add the corresponding keys/values for all five locales (zh-CN, zh-TW, en,
ja, ru) in the i18n resource files so the labels are localized.
In
`@src/app/`[locale]/settings/providers/_components/forms/provider-form/sections/routing-section.tsx:
- Around line 223-237: The two TagInput controls for
state.routing.allowedClients and state.routing.blockedClients are missing the
disabled prop so they remain editable during form submission; update both
TagInput instances (the ones dispatching SET_ALLOWED_CLIENTS and
SET_BLOCKED_CLIENTS) to include disabled={state.ui.isPending} so they mirror
other controls and become non-interactive while the form is pending.
In `@src/app/v1/_lib/proxy/client-guard.ts`:
- Around line 14-28: The code currently forces a User-Agent when either
allowedClients or blockedClients is non-empty, which incorrectly rejects UA-less
requests for blacklist-only configs; change the UA requirement so
session.userAgent is only required when a whitelist is configured
(allowedClients.length > 0) — i.e., keep the early return when both lists are
empty, but replace the unconditional UA check with a conditional that only
enforces UA if allowedClients.length > 0 (leave blockedClients handling and
isClientAllowed usage unchanged so blacklist-only flows allow empty UA to be
checked by the block matcher).
In `@src/app/v1/_lib/proxy/provider-selector.ts`:
- Around line 767-789: The findReusable() path currently reuses a provider
without re-checking provider-level client restrictions; update findReusable (the
reuse decision logic) to call isClientAllowed(session, provider.allowedClients
?? [], provider.blockedClients ?? []) after the model check and before returning
a reusable provider, and if it returns false treat the provider as non-reusable
(push an entry into context.filteredProviders with id, name, reason:
"client_restriction", details: "provider_client_restriction") and continue
searching; mirror the existing model-mismatch cleanup behavior so clients that
become disallowed won't be reused for that provider.
---
Outside diff comments:
In `@messages/zh-CN/dashboard.json`:
- Around line 1813-1825: The two entries in userEditSection.presetClients use
identical display text which confuses users: update the label for "claude-cli"
(currently "Claude Code CLI") to a distinct name such as "Claude CLI (legacy)"
or similar so it is clearly differentiated from "claude-code-cli" (which should
remain "Claude Code CLI"); alternatively, if "claude-cli" is deprecated, remove
the "claude-cli" key and ensure any references to that User-Agent identifier are
updated accordingly.
In `@src/app/`[locale]/dashboard/_components/user/create-user-dialog.tsx:
- Around line 41-47: The create-user flow is dropping the blockedClients field:
update buildDefaultValues() to initialize blockedClients: [] alongside
allowedClients/allowedModels so currentUserDraft.blockedClients is never
undefined, and update createUserOnly(...) call to include blockedClients:
data.user.blockedClients when constructing the payload so the form value is
persisted; confirm CreateUserSchema already supports blockedClients and that
UserEditSection continues to receive the field for display.
In `@src/app/`[locale]/dashboard/_components/user/edit-user-dialog.tsx:
- Around line 49-55: The EditUser form is missing the blockedClients field in
three places causing saved blacklists to be lost; add blockedClients handling to
the Zod schema, default building, and submit payload: extend EditUserSchema
(alongside allowedClients) with blockedClients:
z.array(z.string().max(64)).max(50).optional().default([]) so
currentUserDraft.blockedClients is populated, update buildDefaultValues to set
blockedClients: user.blockedClients || [], and include data.blockedClients when
calling editUser so the server receives the updated blacklist (affecting
UserEditSection's currentUserDraft.blockedClients).
In `@src/lib/provider-patch-contract.ts`:
- Around line 26-36: PATCH_FIELDS was updated to include "allowed_clients" and
"blocked_clients" but isValidSetValue lacks branches to accept set-mode values
for these fields causing normalizeProviderBatchPatchDraft to reject valid
patches; update isValidSetValue to handle "allowed_clients" and
"blocked_clients" by allowing Set-like inputs (e.g., arrays/sets or the same
input shape accepted for "allowed_models") and add corresponding validation
rules, and ensure normalizeProviderBatchPatchDraft continues to accept and
normalize these fields in set mode.
---
Nitpick comments:
In `@src/app/`[locale]/dashboard/logs/_components/provider-chain-popover.tsx:
- Around line 107-113: The branch in getItemStatus handling
"client_restriction_filtered" is unreachable because isActualRequest returns
false for that type and actualRequests is created via
chain.filter(isActualRequest), so that case (and the MinusCircle import) should
be removed to avoid dead code; alternatively, if you intend to show filtered
events, update isActualRequest to include "client_restriction_filtered" (and
ensure actualRequests/where it’s mapped will include and render those items) and
keep the getItemStatus branch—pick one approach and apply it consistently
(adjust getItemStatus, isActualRequest, actualRequests mapping and imports
accordingly).
In
`@src/app/`[locale]/settings/providers/_components/forms/provider-form/sections/routing-section.tsx:
- Around line 158-238: Move the two client restriction FieldGroup blocks (the
TagInput groups using state.routing.allowedClients and
state.routing.blockedClients) out of the existing SectionCard titled by
t("sections.routing.modelWhitelist.title") and into their own SectionCard
component; remove those FieldGroup nodes from the Model Configuration
SectionCard and create a new SectionCard (e.g., title
t("sections.routing.clientRestrictions.title") and description
t("sections.routing.clientRestrictions.desc")) that contains the two FieldGroup
entries using the same TagInput props and dispatch actions
("SET_ALLOWED_CLIENTS" and "SET_BLOCKED_CLIENTS"); preserve all state references
(state.routing.allowedClients, state.routing.blockedClients), placeholders
(t("sections.routing.clientRestrictions.allowedPlaceholder"/"blockedPlaceholder"))
and any existing classes so UI/behavior remains unchanged.
In `@src/app/v1/_lib/proxy/client-detector.ts`:
- Around line 48-54: Replace the unsafe cast "as any" when extracting betas from
session.request.message with a narrower type like Record<string, unknown> and
treat the extracted value as unknown before runtime checks; specifically, change
the extraction in client-detector.ts so you cast session.request.message to
Record<string, unknown> (or a dedicated Message type) and read the "betas"
property from that object, then keep the existing Array.isArray and typeof
checks (the betas.some check and the /^claude-code-/i test) to narrow the type
and push "betas-claude-code" into signals only after those runtime validations
succeed.
- Around line 140-156: isClientAllowed currently may call matchClientPattern
multiple times per session which in turn can invoke confirmClaudeCodeSignals
repeatedly; compute and cache the session-specific client signal once and reuse
it: inside isClientAllowed (with parameters session, allowedClients,
blockedClients) call confirmClaudeCodeSignals(session) once (or compute a
memoized flag on the provided ProxySession) and have matchClientPattern consult
that cached result instead of re-reading headers each iteration so both the
blockedClients.some(...) and allowedClients.some(...) loops reuse the single
computed value.
In `@src/drizzle/schema.ts`:
- Around line 75-78: The schema defines blockedClients with
.notNull().default([]) but allowedClients lacks .notNull(), causing inconsistent
nullability; update the allowedClients column definition in
src/drizzle/schema.ts (the allowedClients entry alongside blockedClients) to
include .notNull().default([]) so both columns use the same non-null default
behavior, then regenerate/apply the DB migration to keep schema and runtime
assumptions consistent.
In `@tests/unit/proxy/client-guard.test.ts`:
- Around line 234-247: Add a unit test under the "when only blockedClients is
configured (no allowedClients)" describe block that creates a session with
undefined UA using createMockSession(undefined, [], ["codex-cli"]) and calls
ProxyClientGuard.ensure(session); assert the current behavior by expecting a
non-null result with status 400 (UA required) so the edge case is explicitly
covered and the expected behavior is pinned down for future changes to
ProxyClientGuard.ensure.
src/app/[locale]/dashboard/_components/user/forms/access-restrictions-section.tsx
Outdated
Show resolved
Hide resolved
...app/[locale]/settings/providers/_components/forms/provider-form/sections/routing-section.tsx
Show resolved
Hide resolved
github-actions
bot
added
size/L
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review Summary
This is a well-structured feature addition implementing per-user and per-provider client restrictions with a multi-signal Claude Code detection engine. The blacklist-before-whitelist logic is sound and comprehensive test coverage (48 new unit tests) is included.
PR Size: L
- Lines changed: 4,955 (4,832 additions, 123 deletions)
- Files changed: 44
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 0 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 0 | 1 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Standards | 0 | 1 | 0 | 0 |
High Priority Issues
1. [STANDARD-VIOLATION] Hardcoded English labels bypass i18n system
File: src/app/[locale]/dashboard/_components/user/forms/access-restrictions-section.tsx:180,192
Why this is a problem: Per CLAUDE.md: All user-facing strings must use i18n (5 languages supported). Never hardcode display text. The hardcoded Allow and Block labels bypass the translation system, displaying English text regardless of users locale preference.
Suggested fix: Add translation keys to the translations type and i18n files for all 5 languages.
Medium Priority Issues
2. [ERROR-NO-USER-FEEDBACK] Blocked client error lacks specific pattern feedback
File: src/app/v1/_lib/proxy/client-guard.ts:33-37
Why this is a problem: When a client is blocked via blockedClients, the error message is generic: Client not allowed. Your client is not in the allowed list. This is misleading because the client might not be in an explicit allowed list - it could be explicitly blocked.
Suggested fix: Distinguish between block and allow failures in isClientAllowed return value to provide accurate error messages.
Positive Observations
- Comprehensive test coverage with 48 new unit tests for client-detector
- Sound blacklist-before-whitelist logic with correct evaluation order
- Clean type threading through all layers (repository, actions, UI)
- Proper database constraints with NOT NULL DEFAULT for new columns
- Good documentation with detailed comments explaining the 3-of-3 signal detection
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - Minor issue noted
- Type safety - Clean
- Documentation accuracy - Clean
- Test coverage - Adequate (48 new tests)
- Code clarity - Good
Automated review by Claude AI
| expiresAt: validatedData.expiresAt, | ||
| allowedClients: validatedData.allowedClients, | ||
| blockedClients: validatedData.blockedClients, | ||
| allowedModels: validatedData.allowedModels, |
Contributor
There was a problem hiding this comment.
[Critical] [SECURITY-VULNERABILITY] blockedClients can be modified by non-admins
Evidence:
src/actions/users.ts:1225persists the field:blockedClients: validatedData.blockedClients,src/lib/permissions/user-field-permissions.ts:29-33marksallowedClientsas admin-only, but does not includeblockedClients.src/lib/permissions/user-field-permissions.ts:46-47defaults to allow when a field is missing from the permission map:if (!permission) return true;
Why this is a problem: updateUserAction allows non-admin users to update their own user record. Because blockedClients is not protected by USER_FIELD_PERMISSIONS, a non-admin can clear or change the admin-enforced client blocklist and bypass client restrictions.
Suggested fix (protect blockedClients the same way as allowedClients):
// src/lib/permissions/user-field-permissions.ts
export const USER_FIELD_PERMISSIONS = {
// ...
allowedClients: { requiredRole: "admin" },
blockedClients: { requiredRole: "admin" },
// ...
} as const;
| } | ||
|
|
||
| const betas = (session.request.message as any).betas; | ||
| if ( |
Contributor
There was a problem hiding this comment.
[Medium] [TYPE-ANY-USAGE] Avoid as any when reading request betas
Evidence:
src/app/v1/_lib/proxy/client-detector.ts:48usesany:const betas = (session.request.message as any).betas;
Why this is a problem: any bypasses type checking in a core detection path and can mask accidental shape changes in session.request.message. ProxySession.request.message is already Record<string, unknown>, so you can access fields as unknown and narrow safely.
Suggested fix (use unknown + narrowing, no any):
// src/app/v1/_lib/proxy/client-detector.ts
const betas = session.request.message["betas"];
if (
Array.isArray(betas) &&
betas.some((beta) => typeof beta === "string" && /^claude-code-/i.test(beta))
) {
signals.push("betas-claude-code");
}
Contributor
There was a problem hiding this comment.
Code Review Summary
This PR is XL (4955 lines / 44 files) and mixes database migrations, proxy routing/guard logic, UI form changes, and i18n updates. The breadth makes it difficult to validate behaviorally and increases regression risk; an access-control regression was introduced around user client restriction fields.
PR Size: XL
- Lines changed: 4955
- Files changed: 44
- Split suggestions:
- PR 1: DB schema/migration + repository transformers/types
- PR 2: Proxy pipeline (client-detector, client-guard, provider-selector) + unit tests
- PR 3: Dashboard UI/forms + i18n message updates
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 0 | 0 | 0 |
| Security | 1 | 0 | 0 | 0 |
| Error Handling | 0 | 0 | 0 | 0 |
| Types | 0 | 0 | 1 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Critical Issues (Must Fix)
src/actions/users.ts:1225— non-admins can modifyblockedClientsdue to missing entry insrc/lib/permissions/user-field-permissions.ts.
High Priority Issues (Should Fix)
src/app/v1/_lib/proxy/client-detector.ts:48— usesas anyto readbetasfrom request message; preferunknown+ narrowing.
Review Coverage
- Logic and correctness
- Security (OWASP Top 10)
- Error handling
- Type safety
- Documentation accuracy
- Test coverage
- Code clarity
Automated review by Codex AI
Contributor
There was a problem hiding this comment.
- Identified PR
#812(feat(proxy): client restriction refactor - blockedClients + Claude Code sub-client detection) and applied the labelsize/XL(4832 additions + 123 deletions across 44 files). - Posted 2 inline review comments:
src/actions/users.ts:1225— [Critical] [SECURITY-VULNERABILITY]:blockedClientsis writable by non-admins due to missing entry insrc/lib/permissions/user-field-permissions.ts(suggested addingblockedClients: { requiredRole: "admin" }).src/app/v1/_lib/proxy/client-detector.ts:48— [Medium] [TYPE-ANY-USAGE]: replace(session.request.message as any).betaswithunknownaccess + narrowing.
- Submitted the required PR review summary (includes XL split suggestions and the Issues Found table).
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
src/app/[locale]/settings/providers/_components/forms/provider-form/sections/routing-section.tsx (1)
82-85: 与access-restrictions-section.tsx不同，此处未使用useMemo包裹splitPresetAndCustomClients
access-restrictions-section.tsx（Line 70-78）对splitPresetAndCustomClients的结果使用了useMemo，而此处在每次渲染时直接调用。虽然当前性能影响可忽略，但为保持一致性可以考虑对齐。可选：使用 useMemo 保持一致
+import { useMemo } from "react"; + - const { customValues: customAllowedClients } = splitPresetAndCustomClients(allowedClients); - const { customValues: customBlockedClients } = splitPresetAndCustomClients(blockedClients); + const { customValues: customAllowedClients } = useMemo( + () => splitPresetAndCustomClients(allowedClients), + [allowedClients] + ); + const { customValues: customBlockedClients } = useMemo( + () => splitPresetAndCustomClients(blockedClients), + [blockedClients] + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/settings/providers/_components/forms/provider-form/sections/routing-section.tsx around lines 82 - 85, The calls to splitPresetAndCustomClients for state.routing.allowedClients and state.routing.blockedClients are done on every render—make them consistent with access-restrictions-section.tsx by memoizing the results: wrap the splitPresetAndCustomClients calls in useMemo so customAllowedClients and customBlockedClients are only recomputed when their respective inputs (state.routing.allowedClients / state.routing.blockedClients) change; ensure useMemo is imported and reference the same identifiers (splitPresetAndCustomClients, customAllowedClients, customBlockedClients) when updating the code.src/lib/validation/schemas.test.ts (1)
108-123: 内层base变量遮蔽了外层同名声明（Line 55）Line 109 的
const base与 Line 55 的外层base内容完全相同，属于冗余的变量遮蔽。可以直接复用外层base。建议：移除冗余的内层 base 声明
test("allowed_clients/blocked_clients 支持 null 并归一化为空数组", () => { - const base = { - name: "测试供应商", - url: "https://api.example.com", - key: "sk-test", - }; - const parsed = CreateProviderSchema.parse({ ...base, allowed_clients: null,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/validation/schemas.test.ts` around lines 108 - 123, The inner test declares a redundant const base that shadows the outer base; remove the inner "const base" and reuse the outer base when calling CreateProviderSchema.parse (i.e., pass { ...base, allowed_clients: null, blocked_clients: null }), so the test no longer shadows the outer variable and still asserts parsed.allowed_clients and parsed.blocked_clients normalize to [].src/app/[locale]/dashboard/_components/user/forms/access-restrictions-section.tsx (1)
114-152: 仅 Checkbox 需要disabled属性，ArrayTagInputField 已内置支持
renderPresetRow中的 Checkbox 缺少disabled属性（第 124、137 行）。与routing-section.tsx的做法一致，建议在 AccessRestrictionsSection 的 Props 中添加可选disabled属性并传递至 Checkbox 组件。注意：ArrayTagInputField 已通过 TagInput 组件隐式支持 disabled 属性，无需接口变更。
建议：为 Checkbox 添加 disabled 属性
export interface AccessRestrictionsSectionProps { allowedClients: string[]; blockedClients: string[]; allowedModels: string[]; modelSuggestions: string[]; onChange: (field: "allowedClients" | "blockedClients" | "allowedModels", value: string[]) => void; + disabled?: boolean; translations: { /* ... */ }; } // 在 renderPresetRow 中的 Checkbox 上传递 <Checkbox id={`allow-${value}`} checked={isAllowed} + disabled={disabled} onCheckedChange={(checked) => handleAllowToggle(value, checked === true)} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/dashboard/_components/user/forms/access-restrictions-section.tsx around lines 114 - 152, Add an optional disabled prop to the AccessRestrictionsSection component props and pass it into the two Checkbox usages inside renderPresetRow so the allow/block checkboxes become disabled when the parent wants them disabled; specifically, update the component signature to accept (disabled?: boolean) and set disabled={disabled} on the Checkbox with id `allow-${value}` (used with handleAllowToggle) and the Checkbox with id `block-${value}` (used with handleBlockToggle), leaving ArrayTagInputField unchanged.src/app/[locale]/dashboard/_components/user/forms/user-form.tsx (1)
21-21: 相对路径导入应改用@/路径别名文件中其余所有内部导入（第 7–20 行）均使用
@/别名，此处使用相对路径"./access-restrictions-section"不一致。♻️ 建议修改
-import { AccessRestrictionsSection } from "./access-restrictions-section"; +import { AccessRestrictionsSection } from "@/app/[locale]/dashboard/_components/user/forms/access-restrictions-section";As per coding guidelines, "Use path alias
@/to reference files in./src/directory".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/dashboard/_components/user/forms/user-form.tsx at line 21, The import for AccessRestrictionsSection uses a relative path; change the import of AccessRestrictionsSection to use the project path alias (replace "./access-restrictions-section" with the corresponding "@/..." alias used by the other imports in this file) so it matches the existing alias style for internal modules and keeps imports consistent with the rest of the file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/actions/providers.ts`:
- Around line 1408-1413: The patch mapping can write null into NOT NULL fields
allowedClients/blockedClients; when applying patches ensure null is normalized
to an empty array. Update the mapping for applyUpdates.allowed_clients and
applyUpdates.blocked_clients (the branches that set result.allowedClients and
result.blockedClients) to coalesce null to [] so the DB semantic "no
restriction" is always represented by an empty array rather than null.
In `@src/lib/validation/schemas.ts`:
- Around line 77-86: CLIENT_PATTERN_SCHEMA currently allows empty strings and
uses hard-coded messages; update CLIENT_PATTERN_SCHEMA to enforce non-empty
trimmed strings (use .trim().min(1)) to prevent empty-pattern matching, and
remove literal Chinese messages—use the zod-i18n error-code mapping
(src/lib/utils/zod-i18n.ts) instead (follow pattern from
ANTHROPIC_MAX_TOKENS_PREFERENCE). Ensure CLIENT_PATTERN_ARRAY_SCHEMA,
OPTIONAL_CLIENT_PATTERN_ARRAY_SCHEMA and
OPTIONAL_CLIENT_PATTERN_ARRAY_WITH_DEFAULT_SCHEMA continue to compose from the
updated CLIENT_PATTERN_SCHEMA so defaults/preprocessing still work and error
messages surface via the i18n mapping rather than inline text.
---
Nitpick comments:
In
`@src/app/`[locale]/dashboard/_components/user/forms/access-restrictions-section.tsx:
- Around line 114-152: Add an optional disabled prop to the
AccessRestrictionsSection component props and pass it into the two Checkbox
usages inside renderPresetRow so the allow/block checkboxes become disabled when
the parent wants them disabled; specifically, update the component signature to
accept (disabled?: boolean) and set disabled={disabled} on the Checkbox with id
`allow-${value}` (used with handleAllowToggle) and the Checkbox with id
`block-${value}` (used with handleBlockToggle), leaving ArrayTagInputField
unchanged.
In `@src/app/`[locale]/dashboard/_components/user/forms/user-form.tsx:
- Line 21: The import for AccessRestrictionsSection uses a relative path; change
the import of AccessRestrictionsSection to use the project path alias (replace
"./access-restrictions-section" with the corresponding "@/..." alias used by the
other imports in this file) so it matches the existing alias style for internal
modules and keeps imports consistent with the rest of the file.
In
`@src/app/`[locale]/settings/providers/_components/forms/provider-form/sections/routing-section.tsx:
- Around line 82-85: The calls to splitPresetAndCustomClients for
state.routing.allowedClients and state.routing.blockedClients are done on every
render—make them consistent with access-restrictions-section.tsx by memoizing
the results: wrap the splitPresetAndCustomClients calls in useMemo so
customAllowedClients and customBlockedClients are only recomputed when their
respective inputs (state.routing.allowedClients / state.routing.blockedClients)
change; ensure useMemo is imported and reference the same identifiers
(splitPresetAndCustomClients, customAllowedClients, customBlockedClients) when
updating the code.
In `@src/lib/validation/schemas.test.ts`:
- Around line 108-123: The inner test declares a redundant const base that
shadows the outer base; remove the inner "const base" and reuse the outer base
when calling CreateProviderSchema.parse (i.e., pass { ...base, allowed_clients:
null, blocked_clients: null }), so the test no longer shadows the outer variable
and still asserts parsed.allowed_clients and parsed.blocked_clients normalize to
[].
src/lib/validation/schemas.ts
Outdated
Comment on lines
77
to
86
| const CLIENT_PATTERN_SCHEMA = z.string().max(64, "客户端模式长度不能超过64个字符"); | ||
| const CLIENT_PATTERN_ARRAY_SCHEMA = z | ||
| .array(CLIENT_PATTERN_SCHEMA) | ||
| .max(50, "客户端模式数量不能超过50个"); | ||
| const OPTIONAL_CLIENT_PATTERN_ARRAY_SCHEMA = z.preprocess( | ||
| (value) => (value === null ? [] : value), | ||
| CLIENT_PATTERN_ARRAY_SCHEMA.optional() | ||
| ); | ||
| const OPTIONAL_CLIENT_PATTERN_ARRAY_WITH_DEFAULT_SCHEMA = | ||
| OPTIONAL_CLIENT_PATTERN_ARRAY_SCHEMA.default([]); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check the actual code in the file
cat -n src/lib/validation/schemas.ts | head -100Repository: ding113/claude-code-hub
Length of output: 3992
🏁 Script executed:
# Check package.json for Zod version
cat package.json | grep -A 2 -B 2 "zod"Repository: ding113/claude-code-hub
Length of output: 313
🏁 Script executed:
# Search for i18n patterns in the codebase to understand how user-facing strings are typically handled
rg -t ts -t tsx "useTranslation|i18n|translation" --max-count=10Repository: ding113/claude-code-hub
Length of output: 94
🏁 Script executed:
# Search for other validation error messages to see if they use i18n
rg -t ts -t tsx "z\.string\(\).*max|z\.string\(\).*min" -A 1 --max-count=15Repository: ding113/claude-code-hub
Length of output: 94
🏁 Script executed:
# Search for i18n/translation patterns more broadly
rg "useTranslation|i18n\.t|translation" --max-count=20Repository: ding113/claude-code-hub
Length of output: 50380
🏁 Script executed:
# Look for where translations might be defined
fd "i18n|locale|translation" -type fRepository: ding113/claude-code-hub
Length of output: 237
🏁 Script executed:
# Check if there are any translation files or configs
find . -type f \( -name "*i18n*" -o -name "*locale*" -o -name "*translation*" \) | head -20Repository: ding113/claude-code-hub
Length of output: 600
🏁 Script executed:
# Look at the coding guidelines more carefully - search how existing error messages are structured
rg "\.min\(|\.max\(.*," src/lib/validation/schemas.ts | head -20Repository: ding113/claude-code-hub
Length of output: 1012
🏁 Script executed:
# Check the zod-i18n utility implementation
cat src/lib/utils/zod-i18n.tsRepository: ding113/claude-code-hub
Length of output: 6486
🏁 Script executed:
# Search for examples of chained Zod methods in the codebase
rg "\.trim\(\)" -A 2Repository: ding113/claude-code-hub
Length of output: 50380
🏁 Script executed:
# Check if there are any examples of how validation errors are currently handled
rg "CLIENT_PATTERN|pattern.*match" --max-count=15Repository: ding113/claude-code-hub
Length of output: 3486
User-Agent 空模式匹配存在安全风险，错误消息需要符合 i18n 规范
CLIENT_PATTERN_SCHEMA 仅限制最大长度，未禁止空字符串。由于模式匹配采用 User-Agent 子串匹配，空字符串会匹配所有请求，造成意外的全量放行或封锁。建议使用 .trim().min(1) 确保非空，并将错误消息迁移至 i18n 系统。
建议修正
-const CLIENT_PATTERN_SCHEMA = z.string().max(64, "客户端模式长度不能超过64个字符");
+const CLIENT_PATTERN_SCHEMA = z
+ .string()
+ .trim()
+ .min(1, "validate:string.min")
+ .max(64, "validate:string.max");Zod 错误消息应通过 src/lib/utils/zod-i18n.ts 的 error code 映射系统处理，而非硬编码。参考现有 ANTHROPIC_MAX_TOKENS_PREFERENCE 的 refine 模式或使用 zod-i18n 的自动映射机制。
根据编码规范，所有用户面向字符串必须使用 i18n（支持 5 种语言），不能硬编码文本。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/validation/schemas.ts` around lines 77 - 86, CLIENT_PATTERN_SCHEMA
currently allows empty strings and uses hard-coded messages; update
CLIENT_PATTERN_SCHEMA to enforce non-empty trimmed strings (use .trim().min(1))
to prevent empty-pattern matching, and remove literal Chinese messages—use the
zod-i18n error-code mapping (src/lib/utils/zod-i18n.ts) instead (follow pattern
from ANTHROPIC_MAX_TOKENS_PREFERENCE). Ensure CLIENT_PATTERN_ARRAY_SCHEMA,
OPTIONAL_CLIENT_PATTERN_ARRAY_SCHEMA and
OPTIONAL_CLIENT_PATTERN_ARRAY_WITH_DEFAULT_SCHEMA continue to compose from the
updated CLIENT_PATTERN_SCHEMA so defaults/preprocessing still work and error
messages surface via the i18n mapping rather than inline text.
- Remove `as any` cast in confirmClaudeCodeSignals; use dot access on Record<string, unknown> - Extract `normalize` to module-level constant; eliminates duplicate inline definitions - Pre-compute Claude Code signals once in isClientAllowed to avoid per-pattern redundancy; guard with early return when both lists are empty to avoid unnecessary work - Only require User-Agent when allowedClients is configured; blocklist-only requests with no UA now pass through correctly instead of returning 400 - Add provider-level client restriction check in findReusable() to prevent stale sessions from bypassing restrictions added after binding - Disambiguate claude-code-cli builtin label from claude-cli UA-match in all 5 locales
Comment on lines
145
to
146
| const hasBlockList = !!blockedClients && blockedClients.length > 0; | ||
| if (!hasBlockList && allowedClients.length === 0) return true; |
There was a problem hiding this comment.
blocklist check happens even when allowedClients is empty and blockedClients has items, but early return on line 146 prevents the check from running
If only a blocklist is configured (no allowlist), the function returns true before checking if the client matches any blocked patterns. The blocklist check on lines 165-167 never executes in this case.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/v1/_lib/proxy/client-detector.ts
Line: 145-146
Comment:
blocklist check happens even when `allowedClients` is empty and `blockedClients` has items, but early return on line 146 prevents the check from running
If only a blocklist is configured (no allowlist), the function returns `true` before checking if the client matches any blocked patterns. The blocklist check on lines 165-167 never executes in this case.
How can I resolve this? If you propose a fix, please make it concise.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/app/v1/_lib/proxy/provider-selector.ts (2)
779-801: Step 1.5 客户端限制预过滤逻辑清晰，但context.totalProviders未同步更新Line 764 初始化
context.totalProviders = visibleProviders.length，但 line 800 将visibleProviders重新赋值为经客户端过滤后的列表。后续 Step 2 使用的visibleProviders数量已减少，但context.totalProviders仍然反映过滤前的值。这意味着在决策上下文和详细错误响应中，
totalProviders包含了被客户端限制过滤掉的供应商数量，可能对调试和 UI 展示造成困惑。建议在 line 801 之后同步更新context.totalProviders，或在文档/注释中说明totalProviders含义为"分组过滤后、客户端过滤前"的数量。对比：分组过滤（Step 1, line 729）在
context初始化之前完成，因此totalProviders已经反映分组过滤后的值。但客户端过滤在context初始化之后，导致不一致。可选修复：在客户端过滤后同步 totalProviders
clientFilteredProviders.push(p); } visibleProviders = clientFilteredProviders; + context.totalProviders = visibleProviders.length; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/v1/_lib/proxy/provider-selector.ts` around lines 779 - 801, The client-filtering loop updates visibleProviders but doesn't update context.totalProviders, leaving totalProviders reflecting the pre-client-filter count; after the client filter completes (i.e., after the block that reassigns visibleProviders via clientFilteredProviders) update context.totalProviders to the new visibleProviders.length so context.totalProviders reflects the count after client restrictions (update the variable referenced as context.totalProviders and the visibleProviders array used in this file, e.g., immediately after the code that sets visibleProviders = clientFilteredProviders).
482-495:clientRestrictedProviders与filteredProviders存在数据冗余
details.filteredProviders（line 485-488）已包含所有被过滤的供应商（含reason: "client_restriction"的条目），而details.clientRestrictedProviders（lines 490-495）又单独提取了一份子集。两者包含相同信息。如果这是为了方便前端解析，建议添加注释说明意图；否则可以考虑移除冗余字段以简化 API 响应。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/v1/_lib/proxy/provider-selector.ts` around lines 482 - 495, The code creates redundant data: details.filteredProviders already includes entries with reason === "client_restriction" but you also build details.clientRestrictedProviders from clientRestricted; remove the redundancy by deleting the clientRestricted variable and the block that sets details.clientRestrictedProviders (or, if frontend consumers require the separate list, keep it but add a clear comment above clientRestrictedProviders explaining it's a convenience subset for the UI), and then run a quick search for references to clientRestrictedProviders to update any callers; refer to identifiers filteredProviders, clientRestricted, details.filteredProviders, and details.clientRestrictedProviders when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/v1/_lib/proxy/client-detector.ts`:
- Around line 1-21: The import in client-detector.ts uses a relative path;
update it to use the repository path alias instead: replace the relative import
of ProxySession (the symbol ProxySession referenced in
src/app/v1/_lib/proxy/client-detector.ts) with the alias-based import using the
"@/..." path that points to the same module (e.g.,
"@/app/v1/_lib/proxy/session") so it follows the project convention for internal
module imports.
---
Duplicate comments:
In `@src/app/v1/_lib/proxy/provider-selector.ts`:
- Around line 591-601: This provider-level client restriction in findReusable is
correct: ensure the isClientAllowed check uses provider.allowedClients and
provider.blockedClients (providerAllowed/providerBlocked) and when it returns
false you call SessionManager.clearSessionProvider(session.sessionId) and return
null so the flow falls back to pickRandomProvider; leave this logic as-is and
ensure the same cleanup pattern used for model-mismatch (the code around
findReusable and SessionManager.clearSessionProvider) is preserved.
---
Nitpick comments:
In `@src/app/v1/_lib/proxy/provider-selector.ts`:
- Around line 779-801: The client-filtering loop updates visibleProviders but
doesn't update context.totalProviders, leaving totalProviders reflecting the
pre-client-filter count; after the client filter completes (i.e., after the
block that reassigns visibleProviders via clientFilteredProviders) update
context.totalProviders to the new visibleProviders.length so
context.totalProviders reflects the count after client restrictions (update the
variable referenced as context.totalProviders and the visibleProviders array
used in this file, e.g., immediately after the code that sets visibleProviders =
clientFilteredProviders).
- Around line 482-495: The code creates redundant data:
details.filteredProviders already includes entries with reason ===
"client_restriction" but you also build details.clientRestrictedProviders from
clientRestricted; remove the redundancy by deleting the clientRestricted
variable and the block that sets details.clientRestrictedProviders (or, if
frontend consumers require the separate list, keep it but add a clear comment
above clientRestrictedProviders explaining it's a convenience subset for the
UI), and then run a quick search for references to clientRestrictedProviders to
update any callers; refer to identifiers filteredProviders, clientRestricted,
details.filteredProviders, and details.clientRestrictedProviders when making the
change.
Comment on lines
+1
to
+21
| import type { ProxySession } from "./session"; | ||
|
|
||
| export const CLAUDE_CODE_KEYWORD_PREFIX = "claude-code"; | ||
|
|
||
| export const BUILTIN_CLIENT_KEYWORDS = new Set([ | ||
| "claude-code", | ||
| "claude-code-cli", | ||
| "claude-code-cli-sdk", | ||
| "claude-code-vscode", | ||
| "claude-code-sdk-ts", | ||
| "claude-code-sdk-py", | ||
| "claude-code-gh-action", | ||
| ]); | ||
|
|
||
| export interface ClientDetectionResult { | ||
| matched: boolean; | ||
| hubConfirmed: boolean; | ||
| subClient: string | null; | ||
| signals: string[]; | ||
| supplementary: string[]; | ||
| } |
There was a problem hiding this comment.
改用 @/ 路径别名导入内部模块。
当前相对路径导入不符合仓库规范。
Proposed fix
-import type { ProxySession } from "./session";
+import type { ProxySession } from "@/app/v1/_lib/proxy/session";As per coding guidelines: Use path alias @/ to reference files in ./src/ directory.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| import type { ProxySession } from "./session"; | |
| export const CLAUDE_CODE_KEYWORD_PREFIX = "claude-code"; | |
| export const BUILTIN_CLIENT_KEYWORDS = new Set([ | |
| "claude-code", | |
| "claude-code-cli", | |
| "claude-code-cli-sdk", | |
| "claude-code-vscode", | |
| "claude-code-sdk-ts", | |
| "claude-code-sdk-py", | |
| "claude-code-gh-action", | |
| ]); | |
| export interface ClientDetectionResult { | |
| matched: boolean; | |
| hubConfirmed: boolean; | |
| subClient: string | null; | |
| signals: string[]; | |
| supplementary: string[]; | |
| } | |
| import type { ProxySession } from "@/app/v1/_lib/proxy/session"; | |
| export const CLAUDE_CODE_KEYWORD_PREFIX = "claude-code"; | |
| export const BUILTIN_CLIENT_KEYWORDS = new Set([ | |
| "claude-code", | |
| "claude-code-cli", | |
| "claude-code-cli-sdk", | |
| "claude-code-vscode", | |
| "claude-code-sdk-ts", | |
| "claude-code-sdk-py", | |
| "claude-code-gh-action", | |
| ]); | |
| export interface ClientDetectionResult { | |
| matched: boolean; | |
| hubConfirmed: boolean; | |
| subClient: string | null; | |
| signals: string[]; | |
| supplementary: string[]; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/v1/_lib/proxy/client-detector.ts` around lines 1 - 21, The import in
client-detector.ts uses a relative path; update it to use the repository path
alias instead: replace the relative import of ProxySession (the symbol
ProxySession referenced in src/app/v1/_lib/proxy/client-detector.ts) with the
alias-based import using the "@/..." path that points to the same module (e.g.,
"@/app/v1/_lib/proxy/session") so it follows the project convention for internal
module imports.
…hain Replace generic "client_restriction" filter reason with specific match context (blocklist_hit/allowlist_miss, matched pattern, detected client). Session reuse path now records a chain item when rejecting a provider. Client guard returns actionable error messages showing which pattern blocked or which allowlist was missed. Dashboard and text timeline render the new context details for auditing.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
src/app/[locale]/dashboard/logs/_components/error-details-dialog/components/LogicTraceTab.tsx (1)
600-636:clientRestrictionContext渲染逻辑在两处完全重复，建议提取为共享组件。Lines 418-463（origin chain 路径）与 Lines 600-636（主过滤路径）的 JSX 结构完全一致，包括 matchType 判断、detectedClient 显示和 allowlist/blocklist 渲染。可提取为内部组件或 helper 以消除重复。
提取示例
function ClientRestrictionDetails({ context, tChain, }: { context: NonNullable<(typeof filteredProviders)[number]["clientRestrictionContext"]>; tChain: ReturnType<typeof useTranslations>; }) { return ( <div className="ml-4 mt-1 space-y-0.5 text-[10px] text-muted-foreground"> <div> {context.matchedPattern ? tChain(`filterDetails.${context.matchType}`, { pattern: context.matchedPattern }) : tChain(`filterDetails.${context.matchType}`)} </div> {context.detectedClient && ( <div>{tChain("filterDetails.detectedClient", { client: context.detectedClient })}</div> )} {context.providerAllowlist.length > 0 && ( <div> {tChain("filterDetails.providerAllowlist", { list: context.providerAllowlist.join(", "), })} </div> )} {context.providerBlocklist.length > 0 && ( <div> {tChain("filterDetails.providerBlocklist", { list: context.providerBlocklist.join(", "), })} </div> )} </div> ); }然后在两处分别替换为：
{p.clientRestrictionContext && ( <ClientRestrictionDetails context={p.clientRestrictionContext} tChain={tChain} /> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/dashboard/logs/_components/error-details-dialog/components/LogicTraceTab.tsx around lines 600 - 636, Duplicate JSX rendering for clientRestrictionContext appears in the origin-chain and main-filter sections; extract it into a small shared component (e.g., ClientRestrictionDetails) and replace both blocks with <ClientRestrictionDetails context={p.clientRestrictionContext} tChain={tChain} />. The new component should accept props (context, tChain), render the matchedPattern conditional using context.matchType and context.matchedPattern, conditionally render context.detectedClient, and only render providerAllowlist/providerBlocklist when their .length > 0 (joining with ", "), matching the existing logic in LogicTraceTab (references: clientRestrictionContext, tChain). Ensure proper typing (NonNullable of the original clientRestrictionContext type) and keep the same className and layout.src/app/v1/_lib/proxy/provider-selector.ts (2)
816-845: 客户端限制预过滤逻辑正确，可考虑优化信号检测的重复计算Step 1.5 的预过滤逻辑正确：跳过无限制的供应商、记录被过滤原因、保持
filteredProviders上下文完整。但
isClientAllowedDetailed内部每次调用都会重新执行confirmClaudeCodeSignals(session)（解析 headers、UA、betas）。当多个供应商配置了客户端限制时，相同的信号检测会重复执行。供应商数量通常较少，性能影响可忽略，但如果后续供应商规模扩大，可考虑将信号预计算提取到循环外部。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/v1/_lib/proxy/provider-selector.ts` around lines 816 - 845, The loop calls isClientAllowedDetailed(session, ...) for each provider which internally re-runs confirmClaudeCodeSignals(session); to avoid repeated signal parsing, precompute the signals once before iterating visibleProviders (e.g., const clientSignals = confirmClaudeCodeSignals(session)) and change calls to use an overload or new helper that accepts these precomputed signals (or pass clientSignals into isClientAllowedDetailed) so the check uses the cached signals; update references to isClientAllowedDetailed and any parameter lists to accept the precomputed signal object, leaving the rest of the filtering logic and context.filteredProviders population intact.
489-495: 详细错误响应中的clientRestrictedProviders仅暴露 id 和 reason，脱敏处理得当与上方
filteredProviders的脱敏逻辑（Line 485-488）一致，未泄露供应商名称或限制模式。不过此处的
clientRestrictedProviders信息与filteredProviders（Line 485-488）存在数据重复——filteredProviders已包含reason: "client_restriction"的条目。如果是为了方便客户端快速识别，建议添加注释说明意图。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/v1/_lib/proxy/provider-selector.ts` around lines 489 - 495, The response currently sets details.clientRestrictedProviders to only id and reason (good for redaction) but duplicates entries already present in filteredProviders (which contains reason: "client_restriction"); update the provider-selector logic so the redaction remains consistent (only id and reason) and add a concise inline comment near the assignment of details.clientRestrictedProviders explaining why this separate field exists (e.g., "duplicate minimal list included for client-side quick identification of client_restriction entries") or, if duplication is unintended, remove the assignment and rely on filteredProviders instead; reference the variables details.clientRestrictedProviders and filteredProviders when making the change.tests/unit/proxy/client-detector.test.ts (1)
261-296:isClientAllowed测试覆盖了关键的黑白名单优先级场景涵盖了：黑名单优先于白名单、双列表为空、仅白名单匹配/不匹配、仅黑名单匹配/不匹配、以及黑名单不匹配+白名单匹配的组合场景。
建议补充一个两个列表都有模式但均不匹配的场景（预期结果为
allowlist_miss），以确保回退逻辑的覆盖：建议补充的测试用例
+ test("should reject when blocked does not match and allowed also does not match", () => { + const session = createMockSession({ userAgent: "UnknownClient/1.0" }); + expect(isClientAllowed(session, ["gemini-cli"], ["codex-cli"])).toBe(false); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/proxy/client-detector.test.ts` around lines 261 - 296, Add a test to assert the behaviour when both allowedClients and blockedClients are provided but neither matches the session userAgent: create a session (e.g., via createMockSession with a userAgent that doesn't match either pattern) and call isClientAllowed(session, ["some-allowed"], ["some-blocked"]) expecting the function to return the status representing an allowlist miss (i.e., false or the specific enum/value your implementation uses for allowlist_miss); place this new test alongside the other isClientAllowed tests referencing isClientAllowed and createMockSession so the fallback logic is covered.src/lib/utils/provider-chain-formatter.ts (1)
449-450: 编码规范：变更行中使用了 emoji 字符根据编码规范
**/*.{js,ts,tsx,jsx}: "Never use emoji characters in any code, comments, or string literals"，新增的"🚫"违反了该规则。不过该文件中已有大量 emoji 作为图标使用（如"⚡"、"💰"等），新增代码与现有模式一致。建议后续统一将这些 icon 提取为常量或使用 i18n，当前可暂不处理。As per coding guidelines:
**/*.{js,ts,tsx,jsx}: Never use emoji characters in any code, comments, or string literals.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/utils/provider-chain-formatter.ts` around lines 449 - 450, The new line assigns emoji string literals to the local variable icon based on f.reason (values "circuit_open" and "client_restriction"), which violates the rule against emoji in source; replace the emoji literals with descriptive constants (e.g., ICON_CIRCUIT_OPEN, ICON_CLIENT_RESTRICTION, ICON_MONEY) defined/ exported from a shared constants or icons module (or use icon keys like "circuit_open" / "client_restriction" if your UI resolves them), update the ternary in provider-chain-formatter to use those constants instead of "⚡"/"🚫"/"💰", and ensure any consumers render the actual glyphs/icons in the presentation layer rather than embedding emoji in code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@messages/ja/provider-chain.json`:
- Around line 83-88: The JSON localization entries for filterDetails contain
untranslated English keys—update the values for the keys
"session_reuse_client_restriction", "blocklist_hit", "allowlist_miss",
"detectedClient", "providerAllowlist", and "providerBlocklist" with Japanese
translations; for example replace "session_reuse_client_restriction" value with
"セッション再利用拒否：クライアント制限", "blocklist_hit" with "パターンによりブロック: {pattern}",
"allowlist_miss" with "許可リストに含まれていません", and similarly provide natural Japanese
equivalents for "detectedClient", "providerAllowlist", and "providerBlocklist"
ensuring placeholders like {pattern}, {client}, and {list} are preserved.
In `@messages/ru/provider-chain.json`:
- Around line 83-88: 在 messages/ru/provider-chain.json 中为 filterDetails 新增的六个
key（session_reuse_client_restriction、blocklist_hit、allowlist_miss、detectedClient、providerAllowlist、providerBlocklist）补充俄文翻译以替换当前的英文文本；确保每个
key 的值为正确的俄语短语并与已翻译的 line 82 风格一致（包括占位符 {pattern}/{client}/{list} 的保留位置和格式）。
In `@messages/zh-CN/provider-chain.json`:
- Around line 83-88: The JSON keys session_reuse_client_restriction,
blocklist_hit, allowlist_miss, detectedClient, providerAllowlist, and
providerBlocklist are still in English; update their values to Chinese
translations consistent with the surrounding entries (e.g., mirror the style of
provider_client_restriction), preserving placeholders exactly ({pattern},
{client}, {list}) and punctuation; ensure the new Chinese strings read naturally
(e.g., "会话重用被拒绝：因客户端限制", "被模式拦截：{pattern}", "不在允许列表中", "检测到：{client}",
"供应商允许列表：{list}", "供应商黑名单：{list}") and save the file so all user-facing strings
are localized.
In `@src/app/v1/_lib/proxy/client-guard.ts`:
- Around line 33-41: The error response currently exposes internal config by
including result.matchedPattern and the full allowedClients array in the body
returned via ProxyResponses.buildError; change this to return a generic denial
message (e.g., "Client not allowed") for both blocklist_hit and other cases,
keep detectedClient out of the response, and log the detailed info
(result.matchedPattern, allowedClients, result.detectedClient, result.matchType)
on the server side instead; update the code paths around
ProxyResponses.buildError and the handling of
result.matchType/result.matchedPattern/allowedClients to use the generic message
while emitting a server-side log entry with the sensitive details.
---
Duplicate comments:
In `@src/app/v1/_lib/proxy/provider-selector.ts`:
- Around line 591-638: Ensure session reuse enforces provider-level client
restrictions: in the findReusable/session reuse path, read
provider.allowedClients and provider.blockedClients, call
isClientAllowedDetailed(session, providerAllowed, providerBlocked), and if
clientResult.allowed is false then log the debug message (including sessionId,
provider.id, clientResult.matchType/matchedPattern/detectedClient), call
session.addProviderToChain with reason "client_restriction_filtered" and the
decisionContext containing filteredProviders with id/name/reason/details and
clientRestrictionContext (matchType, matchedPattern, detectedClient,
providerAllowlist, providerBlocklist), then call
SessionManager.clearSessionProvider(session.sessionId) and return null to
trigger re-selection.
---
Nitpick comments:
In
`@src/app/`[locale]/dashboard/logs/_components/error-details-dialog/components/LogicTraceTab.tsx:
- Around line 600-636: Duplicate JSX rendering for clientRestrictionContext
appears in the origin-chain and main-filter sections; extract it into a small
shared component (e.g., ClientRestrictionDetails) and replace both blocks with
<ClientRestrictionDetails context={p.clientRestrictionContext} tChain={tChain}
/>. The new component should accept props (context, tChain), render the
matchedPattern conditional using context.matchType and context.matchedPattern,
conditionally render context.detectedClient, and only render
providerAllowlist/providerBlocklist when their .length > 0 (joining with ", "),
matching the existing logic in LogicTraceTab (references:
clientRestrictionContext, tChain). Ensure proper typing (NonNullable of the
original clientRestrictionContext type) and keep the same className and layout.
In `@src/app/v1/_lib/proxy/provider-selector.ts`:
- Around line 816-845: The loop calls isClientAllowedDetailed(session, ...) for
each provider which internally re-runs confirmClaudeCodeSignals(session); to
avoid repeated signal parsing, precompute the signals once before iterating
visibleProviders (e.g., const clientSignals = confirmClaudeCodeSignals(session))
and change calls to use an overload or new helper that accepts these precomputed
signals (or pass clientSignals into isClientAllowedDetailed) so the check uses
the cached signals; update references to isClientAllowedDetailed and any
parameter lists to accept the precomputed signal object, leaving the rest of the
filtering logic and context.filteredProviders population intact.
- Around line 489-495: The response currently sets
details.clientRestrictedProviders to only id and reason (good for redaction) but
duplicates entries already present in filteredProviders (which contains reason:
"client_restriction"); update the provider-selector logic so the redaction
remains consistent (only id and reason) and add a concise inline comment near
the assignment of details.clientRestrictedProviders explaining why this separate
field exists (e.g., "duplicate minimal list included for client-side quick
identification of client_restriction entries") or, if duplication is unintended,
remove the assignment and rely on filteredProviders instead; reference the
variables details.clientRestrictedProviders and filteredProviders when making
the change.
In `@src/lib/utils/provider-chain-formatter.ts`:
- Around line 449-450: The new line assigns emoji string literals to the local
variable icon based on f.reason (values "circuit_open" and
"client_restriction"), which violates the rule against emoji in source; replace
the emoji literals with descriptive constants (e.g., ICON_CIRCUIT_OPEN,
ICON_CLIENT_RESTRICTION, ICON_MONEY) defined/ exported from a shared constants
or icons module (or use icon keys like "circuit_open" / "client_restriction" if
your UI resolves them), update the ternary in provider-chain-formatter to use
those constants instead of "⚡"/"🚫"/"💰", and ensure any consumers render the
actual glyphs/icons in the presentation layer rather than embedding emoji in
code.
In `@tests/unit/proxy/client-detector.test.ts`:
- Around line 261-296: Add a test to assert the behaviour when both
allowedClients and blockedClients are provided but neither matches the session
userAgent: create a session (e.g., via createMockSession with a userAgent that
doesn't match either pattern) and call isClientAllowed(session,
["some-allowed"], ["some-blocked"]) expecting the function to return the status
representing an allowlist miss (i.e., false or the specific enum/value your
implementation uses for allowlist_miss); place this new test alongside the other
isClientAllowed tests referencing isClientAllowed and createMockSession so the
fallback logic is covered.
Comment on lines
+83
to
+88
| "session_reuse_client_restriction": "Session reuse rejected: client restriction", | ||
| "blocklist_hit": "Blocked by pattern: {pattern}", | ||
| "allowlist_miss": "Not in allowed list", | ||
| "detectedClient": "Detected: {client}", | ||
| "providerAllowlist": "Allowlist: {list}", | ||
| "providerBlocklist": "Blocklist: {list}" |
There was a problem hiding this comment.
filterDetails 中多个新增 key 未翻译为日文。
与 zh-CN 同样的问题：Line 82 provider_client_restriction 已正确翻译为日文，但 Lines 83-88 的 6 个 key 值仍为英文原文。日文用户会在 UI 中看到未翻译的英文字符串。
请补充日文翻译，例如：
session_reuse_client_restriction→"セッション再利用拒否：クライアント制限"blocklist_hit→"パターンによりブロック: {pattern}"allowlist_miss→"許可リストに含まれていません"- 以此类推
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@messages/ja/provider-chain.json` around lines 83 - 88, The JSON localization
entries for filterDetails contain untranslated English keys—update the values
for the keys "session_reuse_client_restriction", "blocklist_hit",
"allowlist_miss", "detectedClient", "providerAllowlist", and "providerBlocklist"
with Japanese translations; for example replace
"session_reuse_client_restriction" value with "セッション再利用拒否：クライアント制限",
"blocklist_hit" with "パターンによりブロック: {pattern}", "allowlist_miss" with
"許可リストに含まれていません", and similarly provide natural Japanese equivalents for
"detectedClient", "providerAllowlist", and "providerBlocklist" ensuring
placeholders like {pattern}, {client}, and {list} are preserved.
Comment on lines
+83
to
+88
| "session_reuse_client_restriction": "Session reuse rejected: client restriction", | ||
| "blocklist_hit": "Blocked by pattern: {pattern}", | ||
| "allowlist_miss": "Not in allowed list", | ||
| "detectedClient": "Detected: {client}", | ||
| "providerAllowlist": "Allowlist: {list}", | ||
| "providerBlocklist": "Blocklist: {list}" |
There was a problem hiding this comment.
filterDetails 中多个新增 key 未翻译为俄文。
与其他非英文 locale 相同的问题。Line 82 已正确翻译为俄文，但 Lines 83-88 的 6 个 key 值仍为英文原文。
请补充俄文翻译以保持一致性。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@messages/ru/provider-chain.json` around lines 83 - 88, 在
messages/ru/provider-chain.json 中为 filterDetails 新增的六个
key（session_reuse_client_restriction、blocklist_hit、allowlist_miss、detectedClient、providerAllowlist、providerBlocklist）补充俄文翻译以替换当前的英文文本；确保每个
key 的值为正确的俄语短语并与已翻译的 line 82 风格一致（包括占位符 {pattern}/{client}/{list} 的保留位置和格式）。
Comment on lines
+83
to
+88
| "session_reuse_client_restriction": "Session reuse rejected: client restriction", | ||
| "blocklist_hit": "Blocked by pattern: {pattern}", | ||
| "allowlist_miss": "Not in allowed list", | ||
| "detectedClient": "Detected: {client}", | ||
| "providerAllowlist": "Allowlist: {list}", | ||
| "providerBlocklist": "Blocklist: {list}" |
There was a problem hiding this comment.
filterDetails 中多个新增 key 未翻译为中文。
Lines 83-88 的值仍为英文原文，与该文件中其它 key 的中文翻译风格不一致。对比 Line 82 provider_client_restriction 已正确翻译为 "供应商因客户端限制被跳过"，以下 6 个 key 应补充中文翻译：
session_reuse_client_restrictionblocklist_hitallowlist_missdetectedClientproviderAllowlistproviderBlocklist
建议翻译参考
- "session_reuse_client_restriction": "Session reuse rejected: client restriction",
- "blocklist_hit": "Blocked by pattern: {pattern}",
- "allowlist_miss": "Not in allowed list",
- "detectedClient": "Detected: {client}",
- "providerAllowlist": "Allowlist: {list}",
- "providerBlocklist": "Blocklist: {list}"
+ "session_reuse_client_restriction": "会话复用被拒绝：客户端受限",
+ "blocklist_hit": "被模式阻止: {pattern}",
+ "allowlist_miss": "不在白名单中",
+ "detectedClient": "检测到: {client}",
+ "providerAllowlist": "白名单: {list}",
+ "providerBlocklist": "黑名单: {list}"As per coding guidelines, **/*.{ts,tsx,js,jsx}: All user-facing strings must use i18n (5 languages supported: zh-CN, zh-TW, en, ja, ru). Never hardcode display text — 此处虽为 JSON 翻译文件本身，但遗留英文会导致中文用户看到未翻译的字符串，违背 i18n 全量覆盖的要求。
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "session_reuse_client_restriction": "Session reuse rejected: client restriction", | |
| "blocklist_hit": "Blocked by pattern: {pattern}", | |
| "allowlist_miss": "Not in allowed list", | |
| "detectedClient": "Detected: {client}", | |
| "providerAllowlist": "Allowlist: {list}", | |
| "providerBlocklist": "Blocklist: {list}" | |
| "session_reuse_client_restriction": "会话复用被拒绝：客户端受限", | |
| "blocklist_hit": "被模式阻止: {pattern}", | |
| "allowlist_miss": "不在白名单中", | |
| "detectedClient": "检测到: {client}", | |
| "providerAllowlist": "白名单: {list}", | |
| "providerBlocklist": "黑名单: {list}" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@messages/zh-CN/provider-chain.json` around lines 83 - 88, The JSON keys
session_reuse_client_restriction, blocklist_hit, allowlist_miss, detectedClient,
providerAllowlist, and providerBlocklist are still in English; update their
values to Chinese translations consistent with the surrounding entries (e.g.,
mirror the style of provider_client_restriction), preserving placeholders
exactly ({pattern}, {client}, {list}) and punctuation; ensure the new Chinese
strings read naturally (e.g., "会话重用被拒绝：因客户端限制", "被模式拦截：{pattern}", "不在允许列表中",
"检测到：{client}", "供应商允许列表：{list}", "供应商黑名单：{list}") and save the file so all
user-facing strings are localized.
Comment on lines
+33
to
+41
| if (!result.allowed) { | ||
| const detected = result.detectedClient ? ` (detected: ${result.detectedClient})` : ""; | ||
| let message: string; | ||
| if (result.matchType === "blocklist_hit") { | ||
| message = `Client blocked by pattern: ${result.matchedPattern}${detected}`; | ||
| } else { | ||
| message = `Client not in allowed list: [${allowedClients.join(", ")}]${detected}`; | ||
| } | ||
| return ProxyResponses.buildError(400, message, "invalid_request_error"); |
There was a problem hiding this comment.
错误消息泄露了内部客户端限制配置
Line 37 暴露了匹配到的阻止模式（matchedPattern），Line 39 将完整的 allowedClients 列表拼接进响应体返回给客户端。在多租户场景下，这会将管理员配置的限制规则泄露给终端用户，攻击者可据此调整请求绕过限制。
建议仅返回通用拒绝消息，将详细匹配信息保留在服务端日志中。
建议修改
if (!result.allowed) {
const detected = result.detectedClient ? ` (detected: ${result.detectedClient})` : "";
- let message: string;
- if (result.matchType === "blocklist_hit") {
- message = `Client blocked by pattern: ${result.matchedPattern}${detected}`;
- } else {
- message = `Client not in allowed list: [${allowedClients.join(", ")}]${detected}`;
- }
+ logger.warn("ProxyClientGuard: Client rejected", {
+ matchType: result.matchType,
+ matchedPattern: result.matchedPattern,
+ detectedClient: result.detectedClient,
+ allowedClients,
+ blockedClients,
+ });
+ const message = result.matchType === "blocklist_hit"
+ ? `Client not allowed${detected}`
+ : `Client not in allowed list${detected}`;
return ProxyResponses.buildError(400, message, "invalid_request_error");
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/v1/_lib/proxy/client-guard.ts` around lines 33 - 41, The error
response currently exposes internal config by including result.matchedPattern
and the full allowedClients array in the body returned via
ProxyResponses.buildError; change this to return a generic denial message (e.g.,
"Client not allowed") for both blocklist_hit and other cases, keep
detectedClient out of the response, and log the detailed info
(result.matchedPattern, allowedClients, result.detectedClient, result.matchType)
on the server side instead; update the code paths around
ProxyResponses.buildError and the handling of
result.matchType/result.matchedPattern/allowedClients to use the generic message
while emitting a server-side log entry with the sensitive details.
…pty pattern validation - Add allowed_clients/blocked_clients to PATCH_FIELD_CLEAR_VALUE with [] default to prevent null being written to NOT NULL columns during batch clear - Add null coalescing in mapApplyUpdatesToRepositoryFormat for client fields - Enforce non-empty client patterns with .trim().min(1) in CLIENT_PATTERN_SCHEMA
There was a problem hiding this comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/lib/validation/schemas.ts`:
- Around line 77-84: Replace the three hardcoded Chinese messages in
CLIENT_PATTERN_SCHEMA and CLIENT_PATTERN_ARRAY_SCHEMA with i18n error codes from
the project's zod-i18n mapping: remove the inline strings for .min(1), .max(64)
and .max(50) and supply the corresponding error code keys (via the message field
or the mechanism used by src/lib/utils/zod-i18n.ts) so that zod validation uses
the centralized i18n resolver; update CLIENT_PATTERN_SCHEMA and
CLIENT_PATTERN_ARRAY_SCHEMA to reference those i18n keys (using the same
pattern/keys used elsewhere with zod-i18n.ts) rather than literal text.
…queries - Add 3 covering indexes on usage_ledger (user/key/provider + created_at + cost_usd) to enable index-only scans for SUM(cost_usd) aggregation queries - Add composite index on message_request (session_id, created_at, user_id, key) for LATERAL JOIN session user-info lookup - Replace idx_usage_ledger_key_cost to include created_at for time range filtering - Add maxAgeDays=365 time boundary to sumUserTotalCostBatch, sumKeyTotalCostBatchByIds, and sumLedgerTotalCostBatch to avoid full table scans - Refactor sumKeyTotalCostBatchByIds to two-step PK lookup + direct aggregate, eliminating varchar LEFT JOIN - Rewrite aggregateMultipleSessionStats step 4 with LATERAL JOIN (1 index probe per session instead of returning all rows and deduplicating in JS) Addresses 12-15s query regression on usage_ledger after migration from message_request, and 1.5s session lookup on message_request.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
src/repository/message.ts (1)
879-897: 对db.execute()原始结果的类型断言缺乏运行时保护
as Array<{user_id: number, ...}>是纯编译期断言，不验证实际返回值。若后续调整 SQL 列别名或切换 DB 驱动（如从 postgres.js 换为 node-postgres，后者对整型返回string），userId: row.user_id（类型实为unknown）会静默传入错误类型，且 TypeScript 不会报错。建议至少添加轻量运行时验证，或在注释中明确说明此断言依赖 postgres.js 驱动的整型数值行为。
♻️ 建议的防御性方案（示例）
- for (const row of Array.from(userInfoRows) as Array<{ - session_id: string; - user_name: string; - user_id: number; - key_name: string; - key_id: number; - user_agent: string | null; - api_type: string | null; - }>) { + // NOTE: Relies on postgres.js returning integer columns as JS numbers. + // If the driver is changed, validate user_id / key_id are numbers before use. + for (const _row of userInfoRows) { + const row = _row as { + session_id: string; + user_name: string; + user_id: number; + key_name: string; + key_id: number; + user_agent: string | null; + api_type: string | null; + };或更进一步，使用 Zod 做轻量运行时校验（仅在 dev/test 模式下开启）：
// 可复用的行 schema（不在热路径中校验可用 .passthrough() + 仅 dev 断言） const UserInfoRowSchema = z.object({ session_id: z.string(), user_name: z.string(), user_id: z.number(), key_name: z.string(), key_id: z.number(), user_agent: z.string().nullable(), api_type: z.string().nullable(), });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/repository/message.ts` around lines 879 - 897, The code blindly asserts db.execute() results as Array<...> and maps row.user_id into userId without runtime verification; update the handling around userInfoRows and the mapping into userInfoMap to perform lightweight runtime validation/conversion (e.g., check that row.session_id and row.user_name are strings and that row.user_id and row.key_id are numbers or numeric strings and convert them as needed), or plug a small schema check (Zod or hand-rolled) for each row before inserting into userInfoMap so userId/keyId are guaranteed numbers and nullable fields (user_agent, api_type) are validated; ensure checks target the symbols userInfoRows, each row (row.session_id, row.user_id, row.key_id, row.user_agent, row.api_type) and the code that sets userInfoMap.set(...).src/drizzle/schema.ts (1)
901-913: 三个旧索引已成为新覆盖索引的前缀，可考虑清理新增/修改后的索引是现有三个索引的严格超集：
可能冗余的旧索引 新的覆盖索引（已包含旧索引的全部列） usageLedgerKeyCreatedAtIdx(key, createdAt)usageLedgerKeyCostIdx(key, createdAt, costUsd)usageLedgerUserCreatedAtIdx(userId, createdAt)usageLedgerUserCostCoverIdx(userId, createdAt, costUsd)usageLedgerProviderCreatedAtIdx(finalProviderId, createdAt)usageLedgerProviderCostCoverIdx(finalProviderId, createdAt, costUsd)三个旧索引都与新索引使用相同的
WHERE blockedBy IS NULL部分谓词，PostgreSQL 查询规划器对绝大多数查询应会优先选择更宽的覆盖索引。保留前缀索引只会增加写放大和存储开销。建议在确认新索引的执行计划稳定后，通过后续迁移删除上述三个旧索引以减少维护成本。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/drizzle/schema.ts` around lines 901 - 913, The three new covering indexes usageLedgerKeyCostIdx, usageLedgerUserCostCoverIdx and usageLedgerProviderCostCoverIdx are strict supersets of the older indexes usageLedgerKeyCreatedAtIdx, usageLedgerUserCreatedAtIdx and usageLedgerProviderCreatedAtIdx (same WHERE predicate blockedBy IS NULL), so prepare a follow-up migration to drop those three old indexes after verifying plans: run representative EXPLAIN/EXPLAIN (ANALYZE, BUFFERS) queries to confirm the planner chooses the new wider indexes and no query regressions occur, then add a migration that removes usageLedgerKeyCreatedAtIdx, usageLedgerUserCreatedAtIdx and usageLedgerProviderCreatedAtIdx (preserving the predicate if needed) and deploy with monitoring for any performance/replication impact.src/repository/statistics.ts (2)
555-562: 批量 PK 查询结果可顺带预热keyStringByIdCache
sumKeyTotalCostBatchByIds已通过批量查询拿到id → key映射，但没有写入keyStringByIdCache。后续调用sumKeyCostInTimeRange、sumKeyQuotaCostsById、findKeyCostEntriesInTimeRange等单键函数时，仍会各自触发单条 PK 查询。批量解析后顺带填充缓存可降低这些后续请求的 DB 往返。♻️ 建议补充缓存填充
const keyStringToId = new Map(keyMappings.map((k) => [k.key, k.id])); + for (const { id, key } of keyMappings) { + keyStringByIdCache.set(id, key); + } const keyStrings = keyMappings.map((k) => k.key);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/repository/statistics.ts` around lines 555 - 562, sumKeyTotalCostBatchByIds builds a batch mapping (keyMappings → keyStringToId/keyStrings) but never writes those results into keyStringByIdCache, causing subsequent single-key helpers (sumKeyCostInTimeRange, sumKeyQuotaCostsById, findKeyCostEntriesInTimeRange) to re-query the DB; after producing keyMappings (or when keyStrings.length > 0) populate keyStringByIdCache for each id→key pair (using the same keys from keyMappings/keyStringToId) so later single-key lookups hit the cache instead of issuing additional PK queries.
507-508:maxAgeDays的 JSDoc 未覆盖<= 0的边界语义代码中
Number.isFinite(maxAgeDays) && maxAgeDays > 0的判断使maxAgeDays = 0或负数的行为等同于Infinity（返回全时段数据），但公开的@param注释仅提到"use Infinity to include all"，未声明<= 0的语义。两个 batch 函数的 JSDoc 均存在此问题。📝 建议更新 JSDoc
- * `@param` maxAgeDays - Only include records newer than this many days (default 365, use Infinity to include all) + * `@param` maxAgeDays - Only include records newer than this many days (default 365). Use Infinity, 0, or a negative value to include all records.Also applies to: 542-543
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/repository/statistics.ts` around lines 507 - 508, Update the JSDoc for the parameter maxAgeDays in both batch functions to explicitly document that values <= 0 are treated as "include all" (equivalent to passing Infinity) in addition to the current "use Infinity to include all" note; mention the default is 365 and the runtime check that implements this behavior is Number.isFinite(maxAgeDays) && maxAgeDays > 0 so non-finite or non-positive values cause the function to include all records.src/repository/usage-ledger.ts (1)
86-90:timeConditions类型注解与 drizzle 内部实现耦合过紧
ReturnType<typeof gte>[]直接依赖 drizzle-ormgte的返回类型。若 drizzle-orm 升级后gte的返回类型发生变化，此处类型注解可能失效。可改用更通用的SQL[]或直接省略类型注解让 TypeScript 推断：♻️ 建议的类型简化
- const timeConditions: ReturnType<typeof gte>[] = []; + const timeConditions: SQL[] = [];或直接省略显式类型让 TS 推断：
- const timeConditions: ReturnType<typeof gte>[] = []; + const timeConditions = [] as Parameters<typeof and>;注意同时需要在 import 中补充
SQL类型（来自drizzle-orm）。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/repository/usage-ledger.ts` around lines 86 - 90, The current timeConditions variable is tightly coupled to drizzle's gte return type (ReturnType<typeof gte>[]); change it to a more stable type by using SQL[] from drizzle-orm or by removing the explicit annotation and letting TypeScript infer the type; update imports to include SQL if you choose SQL[] and keep the usage with gte(usageLedger.createdAt, cutoffDate) unchanged.drizzle/0075_faithful_speed_demon.sql (1)
1-5: 生产部署时非并发CREATE INDEX可能造成表锁
CREATE INDEX（不带CONCURRENTLY）在执行期间会对目标表持有ShareLock，阻塞并发写操作。若usage_ledger或message_request数据量较大，本次迁移可能造成较长时间的写入阻塞。Drizzle 迁移默认在事务中执行，无法使用
CREATE INDEX CONCURRENTLY（该语句不允许在事务中运行）。建议在业务低峰期执行本次迁移，或考虑拆分为独立脚本在事务外运行。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@drizzle/0075_faithful_speed_demon.sql` around lines 1 - 5, This migration issues blocking CREATE INDEX statements on message_request and usage_ledger (indices: idx_message_request_session_user_info, idx_usage_ledger_user_cost_cover, idx_usage_ledger_provider_cost_cover, idx_usage_ledger_key_cost) which will take a ShareLock and can block writes; to fix, remove these index creations from the transactional Drizzle migration and instead run them outside a transaction using CREATE INDEX CONCURRENTLY (or run this script as a standalone non-transactional migration during a low-traffic window), or split into a separate deployment script that executes CREATE INDEX CONCURRENTLY for each of the listed index names on their respective tables (message_request, usage_ledger).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/drizzle/schema.ts`:
- Around line 75-77: users.allowedClients is nullable while users.blockedClients
is defined as .notNull(), causing inconsistent TypeScript types and runtime NPE
risk; update the schema so allowedClients matches blockedClients by adding
.notNull().default([]) to the allowedClients column definition in
src/drizzle/schema.ts (same pattern used for providers), and add a migration
that updates existing NULL allowed_clients rows to '[]' and alters the column to
SET NOT NULL and DEFAULT '[]' in the DB; alternatively, if you prefer the
opposite change, make blockedClients nullable to match allowedClients and adjust
types/usages accordingly, but ensure schema and DB are kept consistent.
In `@src/repository/message.ts`:
- Around line 855-861: The lateral subquery selecting from message_request
(alias mr) uses ORDER BY created_at LIMIT 1 which is non-deterministic when
multiple rows share the same created_at; update the ORDER BY clause in the CROSS
JOIN LATERAL subquery to use a deterministic tiebreaker by appending the primary
key (e.g., "id ASC") after created_at (so ORDER BY created_at, id ASC LIMIT 1)
to ensure consistent user_agent/api_type selection.
---
Duplicate comments:
In `@src/lib/validation/schemas.ts`:
- Around line 77-90: Replace the hardcoded Chinese validation messages in
CLIENT_PATTERN_SCHEMA and CLIENT_PATTERN_ARRAY_SCHEMA with calls to the i18n
translation system (use the existing i18n utility used project-wide) so messages
are localized for zh-CN, zh-TW, en, ja, ru; update the .min(...) and .max(...)
error message arguments to use translation keys/strings from i18n (not literal
Chinese), leaving the schema chaining (.trim(), .min(), .max(), .array(),
.default(), OPTIONAL_CLIENT_PATTERN_ARRAY_SCHEMA) otherwise unchanged and
ensuring OPTIONAL_CLIENT_PATTERN_ARRAY_SCHEMA and
OPTIONAL_CLIENT_PATTERN_ARRAY_WITH_DEFAULT_SCHEMA continue to reference the
updated schemas. Ensure the chosen i18n keys are consistent with project
conventions and supply translated strings for all supported locales.
---
Nitpick comments:
In `@drizzle/0075_faithful_speed_demon.sql`:
- Around line 1-5: This migration issues blocking CREATE INDEX statements on
message_request and usage_ledger (indices:
idx_message_request_session_user_info, idx_usage_ledger_user_cost_cover,
idx_usage_ledger_provider_cost_cover, idx_usage_ledger_key_cost) which will take
a ShareLock and can block writes; to fix, remove these index creations from the
transactional Drizzle migration and instead run them outside a transaction using
CREATE INDEX CONCURRENTLY (or run this script as a standalone non-transactional
migration during a low-traffic window), or split into a separate deployment
script that executes CREATE INDEX CONCURRENTLY for each of the listed index
names on their respective tables (message_request, usage_ledger).
In `@src/drizzle/schema.ts`:
- Around line 901-913: The three new covering indexes usageLedgerKeyCostIdx,
usageLedgerUserCostCoverIdx and usageLedgerProviderCostCoverIdx are strict
supersets of the older indexes usageLedgerKeyCreatedAtIdx,
usageLedgerUserCreatedAtIdx and usageLedgerProviderCreatedAtIdx (same WHERE
predicate blockedBy IS NULL), so prepare a follow-up migration to drop those
three old indexes after verifying plans: run representative EXPLAIN/EXPLAIN
(ANALYZE, BUFFERS) queries to confirm the planner chooses the new wider indexes
and no query regressions occur, then add a migration that removes
usageLedgerKeyCreatedAtIdx, usageLedgerUserCreatedAtIdx and
usageLedgerProviderCreatedAtIdx (preserving the predicate if needed) and deploy
with monitoring for any performance/replication impact.
In `@src/repository/message.ts`:
- Around line 879-897: The code blindly asserts db.execute() results as
Array<...> and maps row.user_id into userId without runtime verification; update
the handling around userInfoRows and the mapping into userInfoMap to perform
lightweight runtime validation/conversion (e.g., check that row.session_id and
row.user_name are strings and that row.user_id and row.key_id are numbers or
numeric strings and convert them as needed), or plug a small schema check (Zod
or hand-rolled) for each row before inserting into userInfoMap so userId/keyId
are guaranteed numbers and nullable fields (user_agent, api_type) are validated;
ensure checks target the symbols userInfoRows, each row (row.session_id,
row.user_id, row.key_id, row.user_agent, row.api_type) and the code that sets
userInfoMap.set(...).
In `@src/repository/statistics.ts`:
- Around line 555-562: sumKeyTotalCostBatchByIds builds a batch mapping
(keyMappings → keyStringToId/keyStrings) but never writes those results into
keyStringByIdCache, causing subsequent single-key helpers
(sumKeyCostInTimeRange, sumKeyQuotaCostsById, findKeyCostEntriesInTimeRange) to
re-query the DB; after producing keyMappings (or when keyStrings.length > 0)
populate keyStringByIdCache for each id→key pair (using the same keys from
keyMappings/keyStringToId) so later single-key lookups hit the cache instead of
issuing additional PK queries.
- Around line 507-508: Update the JSDoc for the parameter maxAgeDays in both
batch functions to explicitly document that values <= 0 are treated as "include
all" (equivalent to passing Infinity) in addition to the current "use Infinity
to include all" note; mention the default is 365 and the runtime check that
implements this behavior is Number.isFinite(maxAgeDays) && maxAgeDays > 0 so
non-finite or non-positive values cause the function to include all records.
In `@src/repository/usage-ledger.ts`:
- Around line 86-90: The current timeConditions variable is tightly coupled to
drizzle's gte return type (ReturnType<typeof gte>[]); change it to a more stable
type by using SQL[] from drizzle-orm or by removing the explicit annotation and
letting TypeScript infer the type; update imports to include SQL if you choose
SQL[] and keep the usage with gte(usageLedger.createdAt, cutoffDate) unchanged.
Comment on lines
+75
to
+77
| // Blocked clients (CLI/IDE blocklist) | ||
| // Non-empty = listed patterns are denied even if allowedClients permits them | ||
| blockedClients: jsonb('blocked_clients').$type<string[]>().notNull().default([]), |
There was a problem hiding this comment.
users.blockedClients 的 .notNull() 与同表 allowedClients 不一致
新增的 blockedClients 加了 .notNull()，但同一 users 表中已有的 allowedClients（第 69 行）没有 .notNull()。Drizzle 推断类型时：
| 字段 | DB 约束 | TypeScript 推断类型 |
|---|---|---|
allowedClients (已有) |
nullable | string[] | null |
blockedClients (新增) |
NOT NULL | string[] |
这与 providers 表的做法（两者都是 notNull().default([])）形成不一致，且可能导致下游代码在访问 user.allowedClients 时未做 null 判断而报错（例如 .allowedClients.includes(...) 在值为 NULL 时抛出）。
建议通过新迁移给 allowedClients 补加 NOT NULL DEFAULT '[]' 约束，或在所有使用 user.allowedClients 的地方显式加上空值兜底（?? []）。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/drizzle/schema.ts` around lines 75 - 77, users.allowedClients is nullable
while users.blockedClients is defined as .notNull(), causing inconsistent
TypeScript types and runtime NPE risk; update the schema so allowedClients
matches blockedClients by adding .notNull().default([]) to the allowedClients
column definition in src/drizzle/schema.ts (same pattern used for providers),
and add a migration that updates existing NULL allowed_clients rows to '[]' and
alters the column to SET NOT NULL and DEFAULT '[]' in the DB; alternatively, if
you prefer the opposite change, make blockedClients nullable to match
allowedClients and adjust types/usages accordingly, but ensure schema and DB are
kept consistent.
Comment on lines
+855
to
+861
| CROSS JOIN LATERAL ( | ||
| SELECT user_id, key, user_agent, api_type | ||
| FROM message_request | ||
| WHERE session_id = sid AND deleted_at IS NULL | ||
| ORDER BY created_at | ||
| LIMIT 1 | ||
| ) mr |
There was a problem hiding this comment.
LATERAL 子查询缺少排序决胜字段，导致结果不确定
ORDER BY created_at LIMIT 1 在同一 session 内多条请求具有相同 created_at 时，返回的行是非确定性的，可能导致 user_agent / api_type 在两次查询间不一致。建议追加 id ASC 作为决胜字段（与主键自增语义一致）。
🛠️ 建议修复
- ORDER BY created_at
+ ORDER BY created_at, id
LIMIT 1📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| CROSS JOIN LATERAL ( | |
| SELECT user_id, key, user_agent, api_type | |
| FROM message_request | |
| WHERE session_id = sid AND deleted_at IS NULL | |
| ORDER BY created_at | |
| LIMIT 1 | |
| ) mr | |
| CROSS JOIN LATERAL ( | |
| SELECT user_id, key, user_agent, api_type | |
| FROM message_request | |
| WHERE session_id = sid AND deleted_at IS NULL | |
| ORDER BY created_at, id | |
| LIMIT 1 | |
| ) mr |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/repository/message.ts` around lines 855 - 861, The lateral subquery
selecting from message_request (alias mr) uses ORDER BY created_at LIMIT 1 which
is non-deterministic when multiple rows share the same created_at; update the
ORDER BY clause in the CROSS JOIN LATERAL subquery to use a deterministic
tiebreaker by appending the primary key (e.g., "id ASC") after created_at (so
ORDER BY created_at, id ASC LIMIT 1) to ensure consistent user_agent/api_type
selection.
github-project-automation
bot
moved this from Backlog
to Done
in Claude Code Hub Roadmap
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds per-user and per-provider client restriction support with a new multi-signal Claude Code detection engine.
Changes
Core Detection Engine
client-detector.tsmodule with 7 builtin Claude Code keywordsx-appheader, UA prefix, betas arrayisClientAllowed(): blacklist-before-whitelist logicDatabase
blocked_clientsonusers,allowed_clients/blocked_clientsonprovidersProxy Pipeline
client-guard.ts: refactored to useisClientAllowed, now supports blocklistprovider-selector.ts: Step 1.5 neutral client restriction pre-filter (no circuit breaker impact)Repository / Actions / Types
allowedClients/blockedClientsthrough all layersUI
client_restriction_filteredi18n
Test Results
Greptile Summary
This PR implements a comprehensive client restriction system with per-user and per-provider blocklist/allowlist capabilities. The centerpiece is a new Claude Code detection engine (
client-detector.ts) that uses 3-signal verification (x-app header, UA prefix, betas array) to confirm Claude Code clients and extract sub-clients (cli, vscode, sdk-ts, etc.) via entrypoint mapping.Key changes:
isClientAllowedDetailedchecks blocklist first, then allowlist, enabling both blacklist-only and whitelist modesblocked_clientsadded to users, bothallowed_clientsandblocked_clientsadded to providers via migration 0074provider-selector.ts(lines 816-845) for neutral pre-filtering before health checks; also applied during session reuse (lines 592-638)Architecture: The system provides defense-in-depth with user-level checks in
client-guard.tsrejecting requests early, followed by provider-level filtering during selection and session reuse validation. Provider chain logging includes detailedclient_restriction_filteredcontext for observability.Migration 0075 is unrelated - it adds covering indexes for usage_ledger query optimization.
Confidence Score: 5/5
Important Files Changed
isClientAllowedDetailedisClientAllowedDetailedwith proper blocklist support and detailed error messagesallowed_clientsandblocked_clientsjsonb columns to providers table,blocked_clientsto users table, all with empty array defaultsallowedClientsandblockedClientsfields on users (line 69, 77) and providers (line 203-204) tablesFlowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD Start[API Request] --> UserGuard[Client Guard: User-level Check] UserGuard -->|User has restrictions| CheckUserClient{Match user<br/>allowedClients/<br/>blockedClients?} CheckUserClient -->|Blocked| Reject1[400 Error: Client blocked] CheckUserClient -->|Not allowed| Reject2[400 Error: Not in allowlist] CheckUserClient -->|Allowed| SessionReuse{Session<br/>reuse?} UserGuard -->|No restrictions| SessionReuse SessionReuse -->|Yes| CheckSessionProvider[Check session provider<br/>client restrictions] CheckSessionProvider -->|Blocked| ClearSession[Clear session binding] CheckSessionProvider -->|Allowed| UseProvider[Use cached provider] SessionReuse -->|No| ProviderSelection[Provider Selection Pipeline] ClearSession --> ProviderSelection ProviderSelection --> Step1[Step 1: Load all providers] Step1 --> Step15[Step 1.5: Filter by<br/>provider-level<br/>client restrictions] Step15 --> Step2[Step 2: Filter by<br/>enabled/model/format] Step2 --> Step3[Step 3: Health checks] Step3 --> Step4[Step 4: Priority/weight<br/>selection] Step4 --> Selected[Provider selected] Selected --> DetectClient[Client Detection Engine] DetectClient --> Signal1{x-app=cli?} DetectClient --> Signal2{UA prefix<br/>claude-cli/?} DetectClient --> Signal3{betas contains<br/>claude-code-*?} Signal1 & Signal2 & Signal3 -->|All 3 match| Confirmed[Claude Code confirmed] Confirmed --> ExtractSub[Extract sub-client<br/>from UA entrypoint] ExtractSub --> SubClient[claude-code-cli<br/>claude-code-vscode<br/>etc.] Signal1 & Signal2 & Signal3 -->|< 3 match| NotConfirmed[Not Claude Code] NotConfirmed --> CustomMatch[Custom pattern<br/>substring match] SubClient --> BlocklistCheck{In provider<br/>blocklist?} CustomMatch --> BlocklistCheck BlocklistCheck -->|Yes| ProviderBlocked[Skip provider] BlocklistCheck -->|No| AllowlistCheck{Allowlist<br/>configured?} AllowlistCheck -->|No| ProviderAllowed[Provider allowed] AllowlistCheck -->|Yes| InAllowlist{In provider<br/>allowlist?} InAllowlist -->|Yes| ProviderAllowed InAllowlist -->|No| ProviderBlocked ProviderBlocked --> Step15 ProviderAllowed --> ProxyRequest[Forward to provider] UseProvider --> ProxyRequest ProxyRequest --> Response[Return response]Last reviewed commit: 0529517