Skip to content

fix(keys): 非 admin 自助建 key 403 + errors.auth.forbidden 原始键名显示 (#1259)#1267

Merged
ding113 merged 3 commits into
devfrom
fix/self-service-key-creation
Jun 11, 2026
Merged

fix(keys): 非 admin 自助建 key 403 + errors.auth.forbidden 原始键名显示 (#1259)#1267
ding113 merged 3 commits into
devfrom
fix/self-service-key-creation

Conversation

@ding113

@ding113 ding113 commented Jun 11, 2026

Copy link
Copy Markdown
Owner

Closes #1259

问题(两个缺陷)

  1. 功能缺陷:非 admin 用户(持 canLoginWebUi key)在「用户管理」页给自己建 key 时,请求走 POST /api/v1/users/{userId}/keys(requireAuth("admin")),在中间件层被 403 拦截,根本到不了已自带 self 校验的 addKey action(src/actions/keys.ts)。读路径有 users:self 兜底,写路径没有——refactor: add v1 REST management OpenAPI #1140 引入的回归。
  2. 显示缺陷:_compat.toActionResult 原样回传 REST errorCode(auth.forbidden),表单 getErrorMessage() 查不到该翻译键,toast 显示字面量 errors.auth.forbidden

修复(issue 推荐的方案 A)

自助建 key 写路径

  • 新增 read-tier POST /api/v1/users:self/keys(keys/router.ts),目标 userId 始终取自会话,复用现有 addKey action(其内部仍做 self 校验)
  • 安全围栏:read tier 会放行 canLoginWebUi=false 的只读会话,而该端点是写操作且可铸造出能登录 Web UI 的新 key——handler 显式拒绝只读会话(403 auth.forbidden)
  • KeyCreateSchema.strict():body 夹带 userId 直接 400,无法指定他人
  • api-client addKey() 仿照 getUsers():admin 路由 403 auth.forbidden 时回退 self 端点;其他错误不重试
  • isAdminForbidden 下沉到 api-client/v1/errors.ts 统一复用(users.ts 原本地实现移除)

errorCode 显示

  • _compat.toActionResult / toVoidActionResult 统一过 getApiErrorMessageKey() 映射(auth.forbiddenPERMISSION_DENIED),所有表单一次性修复
  • 映射表补充 key.not_found→KEY_NOT_FOUND / key.action_failed→OPERATION_FAILED / user.not_found→USER_NOT_FOUND / user.action_failed→OPERATION_FAILED
  • 更新既有契约断言(provider group counts 失败用例:期望值从原始 auth.forbidden 改为映射后的 PERMISSION_DENIED)

OpenAPI

  • openapi:generate 重新生成 openapi-types.gen.ts;openapi:check / openapi:lint 通过

测试(TDD,先红后绿)

  • tests/api/v1/keys/keys.self.test.ts(7 用例):非 admin 201 且 userId 取自会话 / admin 同样可用 / body 夹带 userId 400 / 只读会话 403 / 未认证 401 / action 失败透传 / admin 路由保持 admin-only 回归守卫
  • tests/unit/api/v1/api-client-actions.test.ts:403 回退 self 端点、非授权错误不重试、key.action_failed 映射
  • tests/unit/frontend/api-error-i18n.test.ts:key/user REST 码映射断言

验证

🤖 Generated with Claude Code

Greptile Summary

This PR fixes two bugs from #1259: non-admin users with canLoginWebUi keys could not create keys for themselves (the POST /api/v1/users/{userId}/keys route is admin-only at the middleware tier, so requests were 403'd before reaching the self-check in addKey), and raw REST error codes like auth.forbidden were leaking into the UI as untranslated strings.

  • Adds a new POST /api/v1/users:self/keys endpoint at the read auth tier that always derives the target user from the session, with an explicit fence blocking canLoginWebUi=false sessions from escalating. The frontend addKey() client function falls back to this endpoint when the admin route returns 403.
  • Routes toActionResult / toVoidActionResult through getApiErrorMessageKey() so all REST error codes are pre-mapped to i18n-namespace keys before reaching form error handlers.
  • Extracts isAdminForbidden into errors.ts for shared use, and extends the error-code mapping table with key.* / user.* entries.

Confidence Score: 5/5

Safe to merge. The new self-service key endpoint is correctly fenced at the handler level (canLoginWebUi check), the KeyCreateSchema is already .strict() so request bodies cannot inject a userId, and the client-side retry is scoped strictly to 403 auth.forbidden responses from the admin route.

The changes are well-scoped: a new read-tier endpoint with explicit privilege checks, a client-side fallback that cannot be triggered by any error other than a genuine 403 from the admin route, and a pre-mapping fix for error codes that had no translation. The existing addKey action's own self-check remains in place as a second layer. Tests cover the privilege fence, the strict-schema 400, the dual-403 PERMISSION_DENIED path, and a regression guard confirming the admin-only route stays admin-only. No existing behaviour is changed for admin callers.

No files require special attention.

Important Files Changed

Filename Overview
src/app/api/v1/resources/keys/handlers.ts Adds createSelfKey handler; derives userId from session and explicitly rejects canLoginWebUi=false sessions to prevent privilege escalation.
src/app/api/v1/resources/keys/router.ts Registers the new POST /users:self/keys route at the read auth tier using KeyCreateSchema (already .strict()) to prevent body-injected userId.
src/lib/api-client/v1/actions/keys.ts addKey now retries via /users:self/keys when the admin route returns 403 auth.forbidden; non-auth errors are re-thrown without retry.
src/lib/api-client/v1/actions/_compat.ts toActionResult and toVoidActionResult now map ApiError.errorCode through getApiErrorMessageKey() before returning, fixing untranslated error display.
src/lib/api-client/v1/errors.ts Adds isAdminForbidden helper and extends API_ERROR_MESSAGE_KEYS with key.* and user.* entries.
src/lib/api-client/v1/actions/users.ts Removed local isAdminForbidden in favour of the shared version from errors.ts; no behavioural change.
src/lib/api-client/v1/openapi-types.gen.ts Generated type additions for the new /users:self/keys POST operation; no hand-authored changes.
tests/api/v1/keys/keys.self.test.ts New integration tests covering 7 scenarios including the read-only 403 fence, strict-schema 400, and admin-route regression guard.
tests/unit/api/v1/api-client-actions.test.ts Extended with tests for the 403 fallback, non-auth no-retry, errorCode mapping, and the dual-403 PERMISSION_DENIED path.
tests/unit/frontend/api-error-i18n.test.ts Adds assertions for the newly added key.* and user.* REST-code-to-i18n-key mappings.

Sequence Diagram

sequenceDiagram
    participant UI as "Dashboard UI"
    participant AC as "addKey() client"
    participant AR as "POST /users/{id}/keys (admin)"
    participant SR as "POST /users:self/keys (read)"
    participant ACT as "addKey action"

    UI->>AC: "addKey({ userId, name })"
    AC->>AR: "POST /api/v1/users/{userId}/keys"
    alt "user is admin"
        AR->>ACT: "callAction"
        ACT-->>AR: "ok: true, key"
        AR-->>AC: "201 Created"
        AC-->>UI: "ok: true, key"
    else "user is non-admin"
        AR-->>AC: "403 auth.forbidden"
        AC->>SR: "retry POST /api/v1/users:self/keys"
        SR->>ACT: "callAction with session userId"
        ACT-->>SR: "ok: true, key"
        SR-->>AC: "201 Created"
        AC-->>UI: "ok: true, key"
    end
Loading

Reviews (3): Last reviewed commit: "test(api-client): reset postMock in doub..." | Re-trigger Greptile

Non-admin dashboard users previously could not create API keys because the
per-user key creation route was admin-only. This adds a new read-tier
POST /api/v1/users:self/keys endpoint that derives the target user from
the authenticated session, rejecting read-only sessions to prevent
privilege escalation.

The API client now falls back to this self-service endpoint when the
admin route returns 403 Forbidden. Additionally, raw REST error codes
are now mapped to translation keys via getApiErrorMessageKey so frontend
forms display localized messages instead of literal error code strings.
@coderabbitai

coderabbitai Bot commented Jun 11, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

新增 POST /api/v1/users:self/keys 自助建 key 端点;客户端在管理员 403 时回退到 self 端点;统一前端错误码映射并补充单元与端到端测试。

Changes

自助建key功能与错误处理

Layer / File(s) Summary
错误判定和映射基础
src/lib/api-client/v1/errors.ts
新增isAdminForbidden()识别403+auth.forbidden场景;扩展API_ERROR_MESSAGE_KEYS以包含 key.*user.* 映射。
自助建key的REST端点
src/app/api/v1/resources/keys/handlers.ts, src/app/api/v1/resources/keys/router.ts, src/lib/api-client/v1/openapi-types.gen.ts
新增createSelfKey处理器与 POST /api/v1/users:self/keys 路由,处理会话鉴权与 WebUI 登录能力校验,强制将 userId 设为会话用户并生成 OpenAPI 类型。
错误码映射中间件
src/lib/api-client/v1/actions/_compat.ts
toActionResulttoVoidActionResult 的错误码来源改为 getApiErrorMessageKey(error),以返回翻译命名空间键。
Client端的admin禁止错误处理与回退
src/lib/api-client/v1/actions/keys.ts, src/lib/api-client/v1/actions/users.ts
addKey 在管理员端点返回被判定为 admin-forbidden 时回退到 users:self/keysusers.ts 改为使用共享的 isAdminForbidden 判定。
端到端测试
tests/api/v1/keys/keys.self.test.ts
新增自建 key 的端到端测试,覆盖会话类型、未经授权、权限拒绝、创建成功与错误透传等场景,验证 Location 与 Cache-Control。
单元测试和兼容性验证
tests/unit/api/v1/api-client-actions.test.ts, tests/unit/frontend/api-error-i18n.test.ts
扩展兼容性测试覆盖 addKey 回退与不回退分支、remove/getKeys 错误映射、以及 errorCode -> i18n key 的映射断言。

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 27.27% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed 标题准确概括了主要变更:为非 admin 用户自助建 key 添加回退路径,并修复错误码显示问题。
Linked Issues check ✅ Passed 代码变更全面实现了 #1259 的所有目标:新增 self 写端点、client 端回退逻辑、错误码映射,以及完整的安全检查与测试覆盖。
Out of Scope Changes check ✅ Passed 所有变更均在 #1259 的范围内,包括新端点、client 回退、错误码映射与测试,无超出范围的改动。
Description check ✅ Passed PR 描述与变更集高度相关,详细说明了两处缺陷、修复方案、实现细节和测试覆盖。

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/self-service-key-creation

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added bug Something isn't working area:core area:i18n labels Jun 11, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a self-service endpoint (POST /api/v1/users:self/keys) allowing non-admin users to create API keys for themselves. It also updates the API client to catch forbidden errors on the admin-only per-user route and automatically retry using the self-service endpoint. Additionally, it improves error handling by pre-mapping REST error codes to translatable error keys and adds comprehensive tests for these changes. I have no feedback to provide as there are no review comments.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
src/app/api/v1/resources/keys/handlers.ts (1)

96-100: ⚡ Quick win

建议把 userId 放到展开对象之后,固定会话用户覆盖权。

当前 [{ userId: sessionUserId, ...body.data }] 的安全性依赖 KeyCreateSchema 永远不含 userId。建议改为后置赋值,直接在 handler 层锁死目标用户,降低后续 schema 演进带来的越权回归风险。

建议修改
   const result = await callAction(
     c,
     actions.addKey,
-    [{ userId: sessionUserId, ...body.data }] as never[],
+    [{ ...body.data, userId: sessionUserId }] as never[],
     c.get("auth")
   );
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/app/api/v1/resources/keys/handlers.ts` around lines 96 - 100, The handler
currently constructs the payload as [{ userId: sessionUserId, ...body.data }]
which can be overridden if body.data ever includes userId; change the payload
construction so userId: sessionUserId is added after the spread (e.g. [{
...body.data, userId: sessionUserId }]) before calling
callAction(actions.addKey, ...) to ensure the sessionUserId always wins; locate
this in the callAction invocation in the handler that references sessionUserId,
body.data, and actions.addKey and make the replacement.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/lib/api-client/v1/openapi-types.gen.ts`:
- Around line 32894-32903: The generated 201 response type is too loose because
POST /api/v1/users:self/keys currently uses GenericKeyResponseSchema
(z.record(z.string(), z.unknown())), so add a structured schema (e.g.,
KeyCreateResponseSchema) in the keys schema module that defines the exact fields
(id, generatedKey, name, etc. with appropriate zod types), then update the
router's 201 response to use KeyCreateResponseSchema instead of
GenericKeyResponseSchema (in the keys router for the create key endpoint) and
regenerate the OpenAPI types so the response type becomes the structured
KeyCreateResponseSchema.

---

Nitpick comments:
In `@src/app/api/v1/resources/keys/handlers.ts`:
- Around line 96-100: The handler currently constructs the payload as [{ userId:
sessionUserId, ...body.data }] which can be overridden if body.data ever
includes userId; change the payload construction so userId: sessionUserId is
added after the spread (e.g. [{ ...body.data, userId: sessionUserId }]) before
calling callAction(actions.addKey, ...) to ensure the sessionUserId always wins;
locate this in the callAction invocation in the handler that references
sessionUserId, body.data, and actions.addKey and make the replacement.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5e56245b-2608-4510-a37b-b9c1a78ec692

📥 Commits

Reviewing files that changed from the base of the PR and between 499d925 and 8ec16aa.

📒 Files selected for processing (10)
  • src/app/api/v1/resources/keys/handlers.ts
  • src/app/api/v1/resources/keys/router.ts
  • src/lib/api-client/v1/actions/_compat.ts
  • src/lib/api-client/v1/actions/keys.ts
  • src/lib/api-client/v1/actions/users.ts
  • src/lib/api-client/v1/errors.ts
  • src/lib/api-client/v1/openapi-types.gen.ts
  • tests/api/v1/keys/keys.self.test.ts
  • tests/unit/api/v1/api-client-actions.test.ts
  • tests/unit/frontend/api-error-i18n.test.ts

Comment on lines +32894 to +32903
201: {
headers: {
[name: string]: unknown;
};
content: {
"application/json": {
[key: string]: unknown;
};
};
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# 描述: 查找 users:self/keys 端点的 OpenAPI 定义

# 搜索 router 或 handler 文件中的 OpenAPI 响应 schema 定义
rg -n -A 10 -B 2 "users:self/keys" --type=ts -g '!*.gen.ts' -g '!*.test.ts'

Repository: ding113/claude-code-hub

Length of output: 3187


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# 1) 定位 /users:self/keys createRoute 的 responses 配置(排除 gen 文件/测试)
rg -n --context=5 'path:\s*"/users:self/keys"' src/app/api/v1/resources/keys/router.ts

# 2) 直接把路由定义附近的内容打印出来(确保包含 responses/201/404)
#    取一个较小区间避免输出过多;如不够我再按需加大
sed -n '100,220p' src/app/api/v1/resources/keys/router.ts

# 3) 查找该文件/端点相关的“Key not found”描述字符串
rg -n --hidden 'Key not found' src/app/api/v1/resources/keys/router.ts src/app/api/v1/resources/keys/handlers.ts src/app/api/v1/resources/keys -S

# 4) 查找 201 响应在该路由上的 schema 引用(例如 KeyCreateSchema/KeySchema 等)
rg -n --hidden '201|404|responses|response' src/app/api/v1/resources/keys/router.ts src/app/api/v1/resources/keys/handlers.ts -S

Repository: ding113/claude-code-hub

Length of output: 13549


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# 1) 看 /users:self/keys 相关的 404 定义具体在哪里(problemResponses)
sed -n '1,120p' src/app/api/v1/resources/keys/router.ts

# 2) 读取 keys/handlers.ts 中 createUserKey/createSelfKey + actionError 的 404 映射逻辑
sed -n '1,140p' src/app/api/v1/resources/keys/handlers.ts
sed -n '240,330p' src/app/api/v1/resources/keys/handlers.ts

# 3) 查找 GenericKeyResponseSchema 的定义(以及它为什么在生成类型里变成索引签名/unknown)
rg -n "export const GenericKeyResponseSchema|GenericKeyResponseSchema" src/app/api/v1/resources/keys -S
rg -n "GenericKeyResponseSchema" src/app/api/v1 -S

# 打印命中的 schema 定义附近内容(尽量小范围)
python3 - <<'PY'
import subprocess, re
paths = subprocess.check_output(["bash","-lc", "rg -n \"GenericKeyResponseSchema\" src/app/api/v1 -S"], text=True)
hits = [line for line in paths.splitlines() if ":" in line]
for line in hits[:20]:
    # line format: file:line:...
    file = line.split(":",2)[0]
    ln = line.split(":",2)[1]
    print(file, ln)
PY

Repository: ding113/claude-code-hub

Length of output: 14230


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# 找到 keys schemas 文件里 GenericKeyResponseSchema / KeyCreateSchema 的定义
fd -a "schemas/keys.*" src/lib/api/v1/schemas || true
fd -a "keys.ts" src/lib/api/v1/schemas || true

# 直接在常见文件中定位(以免 fd 输出不准)
rg -n "export const GenericKeyResponseSchema|export const KeyCreateSchema" src/lib/api/v1/schemas -S

# 读取包含定义的片段(各不超过 ~120 行)
# GenericKeyResponseSchema 所在文件与行号先从上一步 rg 获取
FILE1=$(rg -n "export const GenericKeyResponseSchema" src/lib/api/v1/schemas -S | head -n1 | cut -d: -f1)
LINE1=$(rg -n "export const GenericKeyResponseSchema" src/lib/api/v1/schemas -S | head -n1 | cut -d: -f2)
FILE2=$(rg -n "export const KeyCreateSchema" src/lib/api/v1/schemas -S | head -n1 | cut -d: -f1)
LINE2=$(rg -n "export const KeyCreateSchema" src/lib/api/v1/schemas -S | head -n1 | cut -d: -f2)

echo "GenericKeyResponseSchema at: $FILE1:$LINE1"
sed -n "$((LINE1-10)),$((LINE1+140))p" "$FILE1"

echo "KeyCreateSchema at: $FILE2:$LINE2"
sed -n "$((LINE2-10)),$((LINE2+140))p" "$FILE2"

# 额外:检查 quota/limit*Usd 是否是 number | null 且 optional
rg -n "limit.*Usd|quota|Usd" "$FILE2" -S
rg -n "limit.*Usd|quota|Usd" "$FILE1" -S

Repository: ding113/claude-code-hub

Length of output: 6487


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# 1) 定位 addKey 定义
rg -n "addKey" src/app src/actions src/lib -S

# 2) 直接读取 keys action 文件中 addKey 相关实现(控制输出范围)
# 先找文件,再用 sed 取局部
FILE=$(rg -l "function addKey|addKey\s*:" src -S | head -n 5 | head -n 1 || true)
echo "Candidate addKey file: $FILE"

if [[ -n "${FILE:-}" && -f "$FILE" ]]; then
  sed -n '1,220p' "$FILE"
  rg -n "addKey" "$FILE" -n
  # 输出 addKey 附近
  START=$(rg -n "addKey" "$FILE" | head -n1 | cut -d: -f1)
  sed -n "$((START-40)),$((START+120))p" "$FILE"
fi

# 3) 若上面没找到,改用文件定位:keys action 典型路径
# 尝试在 src/actions 里找 keys.ts 或 keys/index.ts
fd -e ts -a "keys.ts|keys\.ts" src/actions || true
fd -e ts -a "keys" src/actions || true

Repository: ding113/claude-code-hub

Length of output: 7743


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# 1) 读取服务端的 addKey 实现,重点看 NOT_FOUND/错误码来源
rg -n "export async function addKey" src/actions/keys.ts
START=$(rg -n "export async function addKey" src/actions/keys.ts | head -n1 | cut -d: -f1)
# 取一个稳妥区间,避免出现负数行号
A=$((START-80)); if [ "$A" -lt 1 ]; then A=1; fi
B=$((START+220))
sed -n "${A},${B}p" src/actions/keys.ts

# 2) 读取 keys schema 里 KeyMutationFields/limit*Usd 的完整描述(确认 null 的语义)
rg -n "KeyMutationFields|limit5hUsd|limitDailyUsd|limitWeeklyUsd|limitMonthlyUsd|limitTotalUsd" src/lib/api/v1/schemas/keys.ts
sed -n '1,120p' src/lib/api/v1/schemas/keys.ts

Repository: ding113/claude-code-hub

Length of output: 14911


201 响应体类型过于宽松:建议源 schema 改为结构化 key 创建返回对象

当前 POST /api/v1/users:self/keys 的 201 application/json 直接引用 GenericKeyResponseSchema,而该 schema 在 src/lib/api/v1/schemas/keys.ts 中被定义为 z.record(z.string(), z.unknown()),因此生成类型退化为 { [key: string]: unknown },无法为消费端提供 id / generatedKey / name 等字段的类型安全。

建议在源 keys schema 中新增(或替换掉通用)KeyCreateResponseSchema,并在 src/app/api/v1/resources/keys/router.ts 的 201 响应里把 schema: GenericKeyResponseSchema 替换为该结构化 schema 后再重新生成 openapi-types.gen.ts

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/lib/api-client/v1/openapi-types.gen.ts` around lines 32894 - 32903, The
generated 201 response type is too loose because POST /api/v1/users:self/keys
currently uses GenericKeyResponseSchema (z.record(z.string(), z.unknown())), so
add a structured schema (e.g., KeyCreateResponseSchema) in the keys schema
module that defines the exact fields (id, generatedKey, name, etc. with
appropriate zod types), then update the router's 201 response to use
KeyCreateResponseSchema instead of GenericKeyResponseSchema (in the keys router
for the create key endpoint) and regenerate the OpenAPI types so the response
type becomes the structured KeyCreateResponseSchema.

@github-actions

Copy link
Copy Markdown
Contributor

🧪 测试结果

测试类型 状态
代码质量
单元测试
集成测试
API 测试

总体结果: ✅ 所有测试通过

@github-actions github-actions Bot added the size/L Large PR (< 1000 lines) label Jun 11, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

No significant issues identified. The PR is well-structured with a layered security model (session-derived userId, strict schema rejecting body injection, explicit read-only session rejection, action-level self-check), comprehensive test coverage (7 integration tests, 3 unit tests, i18n mapping assertions), and a correct error-code mapping change that properly falls back for unmapped codes.

PR Size: L

  • Lines changed: 597 (587 additions + 10 deletions)
  • Files changed: 10 (~213 lines are auto-generated OpenAPI types)
  • Net meaningful changes: ~10 files, ~380 lines of hand-written code

Issues Found

Category Critical High Medium Low
Logic/Bugs 0 0 0 0
Security 0 0 0 0
Error Handling 0 0 0 0
Types 0 0 0 0
Comments/Docs 0 0 0 0
Tests 0 0 0 0
Simplification 0 0 0 0

Review Coverage

  • Logic and correctness - Clean. Retry logic in addKey correctly retries only on 403 auth.forbidden; userId is stripped from body before self-endpoint retry; session-derived userId prevents impersonation.
  • Security (OWASP Top 10) - Clean. Four-layer defense: requireAuth("read") middleware, explicit canLoginWebUi === false rejection, KeyCreateSchema.strict() rejecting body userId, action-level session.user.id \!== data.userId self-check. No privilege escalation path identified.
  • Error handling - Clean. All catch blocks either re-throw or return proper error responses. The _compat.ts mapping change uses ?? error.errorCode fallback preserving unmapped Server Action codes.
  • Type safety - Clean. No new any types. All type casts follow existing patterns (as never[] for callAction, as { id?: number } for response data).
  • Documentation accuracy - Clean. All new comments accurately describe the code behavior and reference issue #1259.
  • Test coverage - Adequate. 7 integration tests (non-admin 201, admin 201, strict schema 400, read-only 403, unauthenticated 401, action failure passthrough, admin-route regression guard) + 3 unit tests (retry fallback, non-auth no-retry, code mapping) + i18n assertions.
  • Code clarity - Good. isAdminForbidden extraction reduces duplication; comments explain non-obvious security decisions.

Automated review by Claude AI

Deny-by-default canLoginWebUi check (only explicit true proceeds) to
prevent read-tier sessions from minting Web-UI-capable keys. Spread
session userId last in addKey payload to prevent the request body from
overriding the target user.

Add tests covering toVoidActionResult delete-code passthrough and
double-403 self-fallback permission denied mapping.
@github-actions

Copy link
Copy Markdown
Contributor

🧪 测试结果

测试类型 状态
代码质量
单元测试
集成测试
API 测试

总体结果: ✅ 所有测试通过

A preceding test left a persistent implementation on postMock, which
interfered with the mockRejectedValueOnce calls in the double-403
self-fallback case. Adding mockReset ensures only the intended
rejections are in play.
@github-actions

Copy link
Copy Markdown
Contributor

🧪 测试结果

测试类型 状态
代码质量
单元测试
集成测试
API 测试

总体结果: ✅ 所有测试通过

@ding113

ding113 commented Jun 11, 2026

Copy link
Copy Markdown
Owner Author

评审驱动的修复(deep-review + /code-review max effort)

经多代理评审 + 机器人评论交叉核对,已 push 以下修复(CI 全绿,CodeRabbit 的 CHANGES_REQUESTED 已随之解除):

  1. 自助建 key 鉴权围栏改为默认拒绝 — 由 canLoginWebUi === false 改为 !== true(deny-by-default)。can_login_web_ui 列在 schema 中实际可空(default(false)notNull),围栏原先依赖 toKey?? true 兜底;改为正向断言后,即便出现 undefined/null 也会被正确拦截,不再依赖远处的转换器不变量。

  2. 会话 userId 末位展开(CodeRabbit/gemini 命中)addKey payload 由 [{ userId, ...body.data }] 改为 [{ ...body.data, userId }],即便将来 schema 放宽也无法让请求体覆写目标用户(纵深防御;当前 KeyCreateSchema.strict() 已无 userId 字段)。

  3. 补齐两条关键契约测试toVoidActionResult 对删除业务码(CANNOT_DELETE_LAST_KEY)的透传、以及只读会话双 403(admin 路由 + self 兜底均 403)映射为 PERMISSION_DENIED;并修复一处持久化 mock 串味(postMock.mockReset())。

@ding113
ding113 merged commit a497529 into dev Jun 11, 2026
10 checks passed
@github-project-automation github-project-automation Bot moved this from Backlog to Done in Claude Code Hub Roadmap Jun 11, 2026
@github-actions github-actions Bot mentioned this pull request Jun 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:core area:i18n bug Something isn't working size/L Large PR (< 1000 lines)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant