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.
WorkBuddy
admin