Files
danyang-match/docs/HTTP语义检修报告-2026-09-18.md
T
WorkBuddy 7682abd85c fix(api): give failures real HTTP semantics; fix 401/403 confusion and CORS default
Phase 23B ended with an E2E assertion changed to expect "HTTP 200 + body.code
== BAD_REQUEST", documented at the time as "this project's convention". That
description was accurate about the code and wrong about the intent: it described
a defect, and I made the test accommodate it instead of fixing it.

Auditing it found the convention was already broken, not merely risky:

- ~100 sites (105, across 30 controllers) do `return ApiResponse.error(...)` /
  `forbidden()` / `notFound()` with no exception and no `@ResponseStatus`, so
  the HTTP status is always 200. Gateways and APM cannot separate 4xx from 5xx;
  callers cannot tell "fix your request" (400) from "don't retry" (403) from
  "check state first" (409) from "retry later" (429).
- miniapp/utils/api.js already had 401/403/429 branches that could never be
  reached, since the backend never returned those codes. The concrete damage:
  an expired token returned 403, so `onSessionExpired` never fired and users
  were stuck with no way back to the login flow.
- SecurityConfig had no `authenticationEntryPoint`, so Spring's default
  `Http403ForbiddenEntryPoint` returned 403 for unauthenticated calls, with a
  framework body that has no `code` field at all.
- CorsConfig defaulted `allowed-origins` to `*` together with
  `allowCredentials(true)`, while its comment claimed "localhost only by
  default". Comment contradicting code is worse than no comment: it reads as
  safe.
- Only 4 files / 15 assertions in the whole backend suite touched HTTP status,
  which is how this survived.

Approach: one definition, two applications, zero controller changes.
- ApiResponseCode gains an `httpStatus` field — the single source of truth. The
  constructor requires it, so a new code cannot be added without one.
- GlobalExceptionHandler reads status from the enum instead of hardcoding
  `@ResponseStatus` per handler.
- New ApiResponseStatusAdvice (ResponseBodyAdvice) fills in the status just
  before the body is written, covering all ~100 direct-return sites without
  touching them. Two constraints, both noted in code: only downgrade when the
  current status is still 2xx (the advice also sees exception-handler results,
  and must not override an already-correct 401/403/409), and never alter the
  body.
- SecurityConfig: entry point returns 401 with a uniform ApiResponse body;
  access denied handler returns 403 likewise.
- CorsConfig: default changed to empty (deny). Both clients are same-origin
  anyway — nginx proxies /api/ and Vite proxies /api — so nothing depends on it.
- miniapp api.js: 401/403/429/409 branches now prefer the server's
  `body.message`, otherwise "账号已被封禁" would be flattened to "无权限访问".

Implementation trap worth remembering: `ServerHttpResponse` has `setStatusCode`
but no getter, so reading the current status requires casting to
`ServletServerHttpResponse`.

Verification: 37 E2E passed (33 + 4 new contract cases), including a live-stack
check that a controller's direct `notFound()` now yields 404 — that path throws
no exception and never reaches the exception handler, so if the advice were
inactive it would silently return 200 while the enum unit tests stayed green.
Backend regression 662 / 0 failures / 0 errors / 3 skipped (+6 new guards).

The first regression run had 3 failures, all tests asserting the old wrong
behavior (anonymous access expected 403; outsiders reading a conversation
expected 200). Fixed the assertions, not the implementation. Where 404 is used
instead of 403 for "invisible", that is deliberate — it prevents probing whether
another user's conversation exists — and the substantive assertions (e.g. that an
unauthorized response must not leak message bodies) were kept.

Not done, needs a product decision: third-party credential issuance, whether
rate limiting is actually active, publishing OpenAPI externally, per-caller
audit and quotas, pagination/idempotency coverage, desensitization boundaries.
2026-09-18 14:34:58 +08:00

9.0 KiB
Raw Blame History

HTTP 语义检修报告(面向第三方开放)

  • 日期:2026-09-18
  • 起因:Phase 23B 收尾时留下的问题——「HTTP 200 + body.code 这套约定,将来要开放第三方还守得住吗?」
  • 结论:守不住,且当时就已经是断的。已检修;过程中暴露 3 处被测试固化的错误期望与 1 处 CORS 安全缺陷。

一、起因:一条"迁就实现"的断言

Phase 23B 里我把一条 E2E 断言改成了:

# 本项目的约定:业务错误是 HTTP 200 + body.code
assert body.get("code") == "BAD_REQUEST"

那段注释准确描述了代码行为,但它描述的对象恰恰是一个缺陷: handleIllegalArgument 是全局异常处理器里唯一没有 @ResponseStatus 的 handler, 于是它返回 HTTP 200。我让断言迁就了实现,而不是修实现。

顺着这条线查下去,发现问题远不止一处。


二、审计发现

1. 主体问题:约 100 处失败响应顶着 HTTP 200

ApiResponse.error(...) / forbidden() / notFound() 在 Controller 里被直接 return,不抛异常、不带 @ResponseStatus —— HTTP 状态恒为 200。 统计:105 处,分布在 30 个 Controller。

后果分两层:

  • 内部:网关、监控、APM 无法按状态码统计错误率,「4xx 客户端错」与「5xx 服务端故障」全混在 200 里。
  • 外部:第三方集成商不会去比对字符串业务码。他们看的是状态码—— 尤其是「能不能重试」这件事:400 改参数、403 别重试、409 先查状态、429 稍后重试。 全部返回 200 等于把这四种处置方式全部抹平。

2. 前后端契约当时就是断的(不只是"将来的风险")

小程序 utils/api.js 早就写好了 401 / 403 / 429 三个分支:

} else if (res.statusCode === 401) { this.clearToken(); ... }   // 触发重新登录
else if (res.statusCode === 403) { ... }
else if (res.statusCode === 429) { ... }

后端从未返回过这些状态码,这三个分支永远走不到,全部退化成 BUSINESS_ERROR。

最实际的一处伤害:token 过期时后端回 403,小程序因此不会触发 onSessionExpired, 用户卡在无法重新登录的状态。

3. 未认证返回 403,且 body 不是统一结构

SecurityConfig 从未配置 authenticationEntryPoint,Spring Security 默认走 Http403ForbiddenEntryPoint:没带 token 和 token 失效都返回 403, body 是框架默认响应,连 code 字段都没有(实测 body.code 为 None)。

401(需要认证)与 403(认证了但权限不足)混同,是第三方集成最基础的分流依据缺失。

4. CORS 默认放行任意来源,且注释在撒谎

@Value("${cors.allowed-origins:*}")     // 默认通配
config.setAllowCredentials(true);        // 允许携带凭证
// 注释却写着:"默认只允许localhost(开发环境)"

allowCredentials(true) + 通配来源 = 任意站点可发起带凭证的跨域请求(CSRF 直通)。 而注释声称的限制并不存在。注释与代码不符比没有注释更危险,它会让人误以为已经安全。

5. 状态码这层契约几乎无人看守

后端全量测试中,只有 4 个文件、15 处断言触及 HTTP 状态码。 这也是这套错配能长期存在的原因。


三、检修内容

设计取向:一处定义、两处应用、业务代码零改动

改造 105 处 Controller 代价高、回归面大。改为:

改动 作用
ApiResponseCode 新增 httpStatus 字段 业务码 → HTTP 状态的单一真相源,构造函数强制携带,新增码不填就编译不过
GlobalExceptionHandler 改为从枚举读状态 移除各处硬编码的 @ResponseStatus,避免与枚举产生分歧
新增 ApiResponseStatusAdvice(ResponseBodyAdvice) 在响应写出前统一补状态,覆盖 105 处 Controller 直返,不动任何 Controller
SecurityConfig 补 401/403 入口点 未认证回 401,且 body 统一为 ApiResponse
CorsConfig 默认值改为空 默认拒绝跨域;生产由部署环境显式列出来源
小程序 api.js 401/403/429/409 分支 优先使用服务端 body.message,避免业务原因被硬编码文案冲掉

Advice 的两个关键约束(都写进了代码注释):

  1. 只补 2xx。 异常处理器已经设好 401/403/409/500,无差别覆盖会把已正确的状态改错。 由于 ServerHttpResponse 只有 setter 没有 getter,需向下转型到 ServletServerHttpResponse 才能读到当前状态。
  2. 只改状态行,body 一字不动。 依赖 success/code 判断的调用方行为不变; 依赖状态码的调用方从此走到正确分支。

业务码映射规则

语义 状态码 含义(告诉调用方下一步做什么)
参数/校验问题 400 改请求就能通过
身份或状态不允许 403 改请求也没用,别重试
与当前状态冲突 409 先查状态,别盲目重试
速率或冷却限制 429 等一会儿重试

典型:USER_BANNED / USER_UNDER_18 / REQUIRE_MUTUAL_MATCH / SENSITIVE_ACCESS_DENIED / MATCHMAKER_CANNOT_LIKE → 403; ALREADY_LIKED / ACTIVITY_FULL / DUPLICATE_SUBMISSION → 409; RATE_LIMITED / PASS_COOLDOWN → 429。


四、验证证据

1. 活栈实测(最关键:证明 Advice 真的生效)

场景 路径 结果
Controller 直返 notFound() POST /matches/{不存在UUID}/success/request 404
非法活动类型(异常处理器) GET /activities/types/not-a-real-type 400
非 UUID 路径参数 GET /counseling/services/not-a-uuid 400
未认证 GET /notifications(无 token) 401

第一条是判定性的:那条路径不抛异常、不进异常处理器, 若 Advice 失效(包扫描漏了、supports 写错),它会悄悄退回 200,而枚举层的单元测试依然全绿。

2. E2E

37 passed(原 33 + 新增 4 条契约用例)。 新增 tests/e2e/http_semantics_test.py 把「失败必须携带真实 HTTP 语义」钉在活栈上。

3. 后端全量回归

Tests run: 662, Failures: 0, Errors: 0, Skipped: 3   BUILD SUCCESS

基线 656(Phase 23B 后),净 +6 为新增的 ApiResponseCodeHttpStatusTest(6 条映射语义守卫)。 3 个 skip 均有显式原因,无一为空壳。

首个回归轮次为 662 / 3 failures,三处失败均已判定为「测试断言了旧的错误行为」 (详见第五节),修正断言(而非改回实现)后归零。


五、被测试固化的错误期望(3 处)

回归从 656 → 662 时暴露 3 处失败,全部是测试在断言旧的错误行为:

测试 旧期望 新实际 判定
AuthControllerTest.testProtectedEndpointRejectsAnonymousRequest 403 401 新行为对
ChatPhase09IntegrationTest.outsiderCannotReadConversationDetailNorUnread 200 404 新行为对
ChatPhase09IntegrationTest.outsiderCannotReadMessages 200 404 新行为对

后两处的原注释直接写着「HTTP 层仍是 200」,正是本轮检修掉的约定。 改断言时保留了各自的实质性安全断言(如「越权响应不得泄露聊天正文」)。

「用 404 而非 403 表达不可见」是刻意的安全设计:若回 403, 攻击者可枚举 ID 区分「存在但无权」与「根本不存在」,从而探测他人关系是否存在。 该设计在 MatchController#authorizedMatch 注释中亦有说明,本轮予以保留。


六、遗留:真正开放第三方还需要什么(本轮未做)

以下均属范围扩张,需产品决策,本轮不擅自实施:

  1. 第三方凭据机制:目前只有微信登录与运营账号登录,第三方没有标准途径获取 token。 需定义 API Key / OAuth Client Credentials,以及与之配套的权限范围与配额。
  2. 限流是否真实生效:RATE_LIMITED 业务码与 429 映射已就位, 但限流本身是否启用、阈值多少,本轮未验证。
  3. OpenAPI 文档对外发布:已有 OpenApiConfig,是否开放、开放哪些端点需决策。
  4. 审计与可观测:第三方调用需要独立的调用审计与配额计量。
  5. 分页稳定性与幂等:Idempotency-Key 已支持,但需确认第三方可见端点的覆盖范围。
  6. 敏感字段脱敏边界:现有 INV-011 等约束面向内部角色,第三方场景需重新界定。

七、结论

本轮把「失败必须携带真实 HTTP 语义」从约定变成了机制:

  • 状态码的唯一来源是 ApiResponseCode.httpStatus,新增业务码不声明就编译不过;
  • 105 处 Controller 直返零改动即获得正确状态;
  • 未认证与无权限在 HTTP 层分离,小程序终于能触发重新登录;
  • CORS 默认从「放行任意来源」改为「默认拒绝」。

顺带修掉了一个当时就会伤害用户的缺陷:token 过期不再让用户卡死。