danyang-match

132 Commits 2 Branches 8 Tags

3 Commits

Include renames

Author	SHA1	Message	Date
WorkBuddy	2766e53fe2	docs: 幂等键检修报告；勘误 API_INVENTORY 与 HTTP 报告；同步 STATUS/JIRA 新增 docs/幂等键检修报告-2026-09-18.md：记录双向断裂（后端零处读取该头、前端 Date.now 每次生成新键）、五处审计发现、机制设计与五个取舍（先占位后执行 / 重放 / 失败释放占位 / 孤儿占位接管 / 作用域含 actor_id）、列类型不变量扫描、E2E 与守门证据、三处同形状问题的连带修复。同轮勘误： - docs/HTTP语义检修报告-2026-09-18.md：其中「Idempotency-Key 已支持」一句在写下时不成立（后端当时零处读取），已显式标注勘误并保留原文，以免「结论先于核实」被当作既成事实继续引用。 - docs/api/API_INVENTORY.md：原记载 POST /activities/{id}/interests 「提交活动互选」，后端从未实现（Phase 16 已并入 /feedback 的 interestTargetUserIds），按真实端点重列并附勘误说明。 docs/STATUS.md：迁移范围更新为 V001~V046；回归行更新为 669 / 0 / 0 / 3；新增「幂等键检修」章节；补登 HTTP 语义检修章节在两份文档中的缺失条目。 docs/JIRA_BACKLOG.md：补登 HTTP 语义检修与幂等键检修两行（此前均未登记）。	2026-09-18 16:37:47 +08:00
WorkBuddy	cbb4f64fdc	fix(security): stop operator identity from reaching user endpoints; guard path rules Continues the HTTP-semantics work. Fixing 401/403 exposed a worse defect on the same axis: who is allowed to reach which endpoint. Root cause chain: - PlatformPrincipal has two implementations (OperatorPrincipal / UserPrincipal), chosen by JwtAuthFilter from the token type. - `@AuthenticationPrincipal UserPrincipal` against an OperatorPrincipal is a type mismatch, and Spring Security returns null (errorOnInvalidType defaults to false) instead of failing. - 244 uses of that annotation repo-wide; only 48 null-check. The rest NPE -> 500 when reached with an operator identity. - The fallback rule was `.authenticated()`, which an operator token satisfies, so such requests did reach user-facing controllers. Adding ~196 null checks would be mechanical, easy to miss, and does nothing to constrain future code. The mismatch is between identity type and endpoint, so it is fixed once, at the authentication layer: the fallback becomes `.hasRole("USER")` (operator -> ROLE_OPERATOR, user -> ROLE_USER, per JwtAuthFilter). Operator requests to user endpoints now get 403 before any controller runs. Trap worth recording: operator endpoints are not all under /operator/ — /counseling/services/operator/queue, /content/operator/articles, /activities/operator/checkin/scan, /service-appointments/operator/list and others sit deeper inside business paths. The first attempt covered them with `requestMatchers("//operator/")`, which fails at runtime: PatternParseException: No more pattern data allowed after {...} or * pattern element Spring's PathPattern permits `` only as the final element. Worse, the symptom is a 500 from the whole filter chain (Tomcat HTML error page, not our JSON), and E2E went from 37 passed to 33 failed — an error far removed from its cause. Now the nine prefixes are listed explicitly, each ending in /. Because explicit enumeration can be forgotten as endpoints are added, and because the failure mode is "operators locked out of their own admin module" (invisible to user-side tests), SecurityPathCoverageTest parses the OPERATOR rule prefixes out of SecurityConfig source, walks every Controller.java, joins class-level and method-level mapping paths, and asserts every path containing /operator/ is covered (/auth/operator/* excluded by design). Two things that would silently neuter it, both handled: it must parse only the .hasRole("OPERATOR") chain — pulling in the permitAll or fallback rules would mark everything covered — and the test working directory is the repo root, not the Maven module root. Verification: negatively validated by removing /content/operator/** -> exactly one test turns red listing the six affected ContentController endpoints; restored. E2E 40 passed (37 + 3 identity-isolation cases). Backend regression 664 / 0 failures / 0 errors / 3 skipped.	2026-09-18 15:05:41 +08:00
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

Author

SHA1

Message

Date

WorkBuddy

2766e53fe2

docs: 幂等键检修报告；勘误 API_INVENTORY 与 HTTP 报告；同步 STATUS/JIRA

新增 docs/幂等键检修报告-2026-09-18.md：记录双向断裂（后端零处读取该头、
前端 Date.now 每次生成新键）、五处审计发现、机制设计与五个取舍
（先占位后执行 / 重放 / 失败释放占位 / 孤儿占位接管 / 作用域含 actor_id）、
列类型不变量扫描、E2E 与守门证据、三处同形状问题的连带修复。

同轮勘误：

- docs/HTTP语义检修报告-2026-09-18.md：其中「Idempotency-Key 已支持」一句
  在写下时不成立（后端当时零处读取），已显式标注勘误并保留原文，
  以免「结论先于核实」被当作既成事实继续引用。
- docs/api/API_INVENTORY.md：原记载 POST /activities/{id}/interests
  「提交活动互选」，后端从未实现（Phase 16 已并入 /feedback 的
  interestTargetUserIds），按真实端点重列并附勘误说明。

docs/STATUS.md：迁移范围更新为 V001~V046；回归行更新为 669 / 0 / 0 / 3；
新增「幂等键检修」章节；补登 HTTP 语义检修章节在两份文档中的缺失条目。
docs/JIRA_BACKLOG.md：补登 HTTP 语义检修与幂等键检修两行
（此前均未登记）。

2026-09-18 16:37:47 +08:00

WorkBuddy

cbb4f64fdc

fix(security): stop operator identity from reaching user endpoints; guard path rules

Continues the HTTP-semantics work. Fixing 401/403 exposed a worse defect on the
same axis: who is allowed to reach which endpoint.

Root cause chain:
- PlatformPrincipal has two implementations (OperatorPrincipal / UserPrincipal),
  chosen by JwtAuthFilter from the token type.
- `@AuthenticationPrincipal UserPrincipal` against an OperatorPrincipal is a type
  mismatch, and Spring Security returns null (errorOnInvalidType defaults to
  false) instead of failing.
- 244 uses of that annotation repo-wide; only 48 null-check. The rest NPE -> 500
  when reached with an operator identity.
- The fallback rule was `.authenticated()`, which an operator token satisfies, so
  such requests did reach user-facing controllers.

Adding ~196 null checks would be mechanical, easy to miss, and does nothing to
constrain future code. The mismatch is between identity type and endpoint, so it
is fixed once, at the authentication layer: the fallback becomes
`.hasRole("USER")` (operator -> ROLE_OPERATOR, user -> ROLE_USER, per JwtAuthFilter).
Operator requests to user endpoints now get 403 before any controller runs.

Trap worth recording: operator endpoints are not all under /operator/ —
/counseling/services/operator/queue, /content/operator/articles,
/activities/operator/checkin/scan, /service-appointments/operator/list and others
sit deeper inside business paths. The first attempt covered them with
`requestMatchers("/**/operator/**")`, which fails at runtime:

  PatternParseException: No more pattern data allowed after {*...} or ** pattern element

Spring's PathPattern permits `**` only as the final element. Worse, the symptom is
a 500 from the whole filter chain (Tomcat HTML error page, not our JSON), and E2E
went from 37 passed to 33 failed — an error far removed from its cause. Now the
nine prefixes are listed explicitly, each ending in /**.

Because explicit enumeration can be forgotten as endpoints are added, and because
the failure mode is "operators locked out of their own admin module" (invisible to
user-side tests), SecurityPathCoverageTest parses the OPERATOR rule prefixes out of
SecurityConfig source, walks every *Controller.java, joins class-level and
method-level mapping paths, and asserts every path containing /operator/ is
covered (/auth/operator/** excluded by design). Two things that would silently
neuter it, both handled: it must parse only the .hasRole("OPERATOR") chain — pulling
in the permitAll or fallback rules would mark everything covered — and the test
working directory is the repo root, not the Maven module root.

Verification: negatively validated by removing /content/operator/** -> exactly one
test turns red listing the six affected ContentController endpoints; restored.
E2E 40 passed (37 + 3 identity-isolation cases). Backend regression 664 / 0
failures / 0 errors / 3 skipped.

2026-09-18 15:05:41 +08:00

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