fix(housekeeping): harden people read boundaries
This commit is contained in:
Simo committed
1 parent
e3f8b51d31
commit
d1382c839e
12 files changed
+1635
-446
No files matched your search
@@ -1,6 +1,6 @@
|
||||
# Task 11 — People workflow read models
|
||||
|
||||
Status: DONE
|
||||
Status: DONE — fix round 1
|
||||
|
||||
## Delivered scope
|
||||
|
||||
@@ -15,8 +15,25 @@ Status: DONE
|
||||
- User mail and current IP remain independently nullable fields. Each is projected only when the capability context contains the existing `PERMS.USERS_VIEW`; `PERMS.MOD_USERS_VIEW` alone receives the safe base projection with both values set to `null`, and a context with neither permission is forbidden.
|
||||
- No new ACL slug or rank threshold was introduced. Staff filtering reuses the existing `getMinStaffRank()` source.
|
||||
- The production user selection is explicit and excludes passwords, authentication tickets, secrets, and two-factor material. VPN settings intentionally exclude `vpn_api_key`.
|
||||
- Adapters fail closed. Missing detail entities map to `NOT_FOUND`; invalid identifiers to `VALIDATION`; adapter and count failures to `DEPENDENCY_UNAVAILABLE`. No partial-result shape is returned because no People DTO explicitly names failed sources.
|
||||
- Pagination clamps page size to 100 and offset to 1,000,000. Production list adapters fetch the full prefix required for in-memory stable sorting/pagination, avoiding double-offset truncation.
|
||||
- Adapters fail closed. Malformed driver envelopes, invalid or non-positive identifiers, corrupt links, non-serializable DTO values, and count failures map to `DEPENDENCY_UNAVAILABLE`. Missing valid detail entities map to `NOT_FOUND`; invalid request identifiers map to `VALIDATION`. No partial-result shape is returned because no People DTO explicitly names failed sources.
|
||||
- Pagination clamps page size to 100 and offset to 10,000. Deterministic primary sorting, numeric-ID tie breaking, and `LIMIT`/`OFFSET` now execute in the database; no list query fetches a prefix for locale re-sorting or second slicing.
|
||||
- Raw production adapters and `buildPeopleUserSelection` are module-private. Runtime exports expose only context-authorized query factories and singleton query surfaces.
|
||||
- Multi-account clusters use one bounded CTE/window query, cap accounts per cluster at 100, and never issue one query per IP cluster.
|
||||
- User detail/edit now includes the operator's watched state and canonical permission-rank data. Support ticket reads use the existing unified inbox through a strict, fail-closed, database-paged mode that includes CMS and help-center rows while leaving the legacy tolerant mode unchanged.
|
||||
- Support desk/detail DTOs explicitly include queue counts, bounded staff, and the relevant active ban. Ticket messages/replies and staff rows are bounded.
|
||||
- Active bans are filtered before sorting. Expiry `0` remains the permanent-active sentinel; expired rows cannot hide permanent or future-active bans in lists or details.
|
||||
|
||||
## Official fix round 1 findings
|
||||
|
||||
1. **Authorization boundary:** fixed by making all raw production adapters and the user selection builder module-private and testing only guarded public query surfaces plus source/runtime export contracts.
|
||||
2. **Pagination and sorting:** fixed by moving declared sort fields, deterministic tie ordering, and bounded `LIMIT`/`OFFSET` to production adapters. The exact `user10`/`user2` and support `status`/`updatedAt` page regressions are covered.
|
||||
3. **Resource bounds:** fixed by replacing multi-account prefix loading plus per-row `Promise.all` with one bounded batched CTE/window query. No million-row prefix and no N+1 cluster query remain.
|
||||
4. **Fail-closed database validation:** fixed across People models and community/support/moderation query boundaries. Invalid driver shapes and invalid identifiers cannot become empty lists, `NOT_FOUND`, ID `0`, or corrupt canonical links.
|
||||
5. **Canonical dependencies:** fixed watched and permission-rank data for user detail/edit; `/support/tickets` now calls strict `fetchUnifiedTicketInbox`; support queue/staff/active-ban data is explicit in canonical query DTOs.
|
||||
6. **Moderation capability equality:** fixed after direct user authorization. `moderationQuery.capability` now exactly equals the existing eleven-slug overview union already used by the route and `run`; no route, mutation, rank threshold, or new slug changed.
|
||||
7. **Active bans:** fixed list/detail selection so permanent `ban_expire = 0` and future-active bans are deterministic and expired rows cannot hide them.
|
||||
|
||||
The two official Minor findings remain parked and unchanged as instructed.
|
||||
|
||||
## Strict TDD evidence
|
||||
|
||||
@@ -80,7 +97,7 @@ Production-source contract RED:
|
||||
pnpm exec vitest run --coverage.enabled=false src/features/housekeeping/domains/people/queries/people-adapters-production.test.ts
|
||||
Test Files 1 failed (1)
|
||||
Tests 2 failed (2)
|
||||
Reason: production adapters and buildPeopleUserSelection were not yet exported.
|
||||
Reason: raw production adapters and buildPeopleUserSelection were exported as bypassable runtime internals.
|
||||
```
|
||||
|
||||
Pagination regression RED after adding the production contract fixture:
|
||||
@@ -92,7 +109,7 @@ Tests 1 failed | 2 passed (3)
|
||||
Expected ["203.0.113.1", "203.0.113.2"], received ["203.0.113.2"].
|
||||
```
|
||||
|
||||
GREEN after the minimal prefix-fetch correction:
|
||||
GREEN for the original baseline implementation:
|
||||
|
||||
```text
|
||||
pnpm exec vitest run --coverage.enabled=false src/features/housekeeping/domains/people/queries/people-adapters-production.test.ts
|
||||
@@ -102,27 +119,161 @@ Tests 3 passed (3)
|
||||
|
||||
The query tests cover adversarial page size/offset/search, stable tie sorting, empty/missing entities, adapter and count failures, PII capability combinations, serializable DTOs, explicit source projection, and production pagination.
|
||||
|
||||
## Fix round 1 strict behavioral TDD evidence
|
||||
|
||||
### Authorization boundary
|
||||
|
||||
RED:
|
||||
|
||||
```text
|
||||
pnpm exec vitest run --coverage.enabled=false src/features/housekeeping/domains/people/queries/people-adapters-production.test.ts
|
||||
Test Files 1 failed (1)
|
||||
Tests 2 failed (2)
|
||||
Observed runtime exports: buildPeopleUserSelection and peopleUsersAdapters.
|
||||
```
|
||||
|
||||
GREEN:
|
||||
|
||||
```text
|
||||
pnpm exec vitest run --coverage.enabled=false src/features/housekeeping/domains/people/queries/people-adapters-production.test.ts
|
||||
Test Files 1 passed (1)
|
||||
Tests 3 passed (3)
|
||||
```
|
||||
|
||||
### Database pagination and declared sorting
|
||||
|
||||
RED:
|
||||
|
||||
```text
|
||||
pnpm exec vitest run --coverage.enabled=false src/features/housekeeping/domains/people/models.test.ts src/features/housekeeping/domains/people/queries/people-queries.test.ts
|
||||
Test Files 2 failed (2)
|
||||
Tests 4 failed | 14 passed (18)
|
||||
Failures: offset 999999 was not capped; DB pages were sliced a second time for user10/user2 and support status/updatedAt.
|
||||
```
|
||||
|
||||
GREEN:
|
||||
|
||||
```text
|
||||
pnpm exec vitest run --coverage.enabled=false src/features/housekeeping/domains/people/models.test.ts src/features/housekeeping/domains/people/queries/people-queries.test.ts
|
||||
Test Files 2 passed (2)
|
||||
Tests 18 passed (18)
|
||||
```
|
||||
|
||||
### Multi-account resource bounds
|
||||
|
||||
RED:
|
||||
|
||||
```text
|
||||
pnpm exec vitest run --coverage.enabled=false src/features/housekeeping/domains/people/queries/people-adapters-production.test.ts
|
||||
Test Files 1 failed (1)
|
||||
Tests 1 failed | 2 passed (3)
|
||||
Observed prefix result plus one query per IP cluster.
|
||||
```
|
||||
|
||||
GREEN:
|
||||
|
||||
```text
|
||||
pnpm exec vitest run --coverage.enabled=false src/features/housekeeping/domains/people/queries/people-adapters-production.test.ts
|
||||
Test Files 1 passed (1)
|
||||
Tests 3 passed (3)
|
||||
```
|
||||
|
||||
### Fail-closed validation
|
||||
|
||||
RED:
|
||||
|
||||
```text
|
||||
pnpm exec vitest run --coverage.enabled=false src/features/housekeeping/domains/people/models.test.ts src/features/housekeeping/domains/people/queries/people-queries.test.ts src/features/housekeeping/domains/people/queries/people-adapters-production.test.ts
|
||||
Test Files 3 failed (3)
|
||||
Tests 5 failed | 21 passed (26)
|
||||
Failures covered ID 0, corrupt links/dates, corrupt detail shapes, and malformed driver envelopes.
|
||||
```
|
||||
|
||||
GREEN:
|
||||
|
||||
```text
|
||||
same command
|
||||
Test Files 3 passed (3)
|
||||
Tests 26 passed (26)
|
||||
```
|
||||
|
||||
### Canonical dependencies and unified support inbox
|
||||
|
||||
RED:
|
||||
|
||||
```text
|
||||
pnpm exec vitest run --coverage.enabled=false src/features/housekeeping/domains/people/queries/people-queries.test.ts src/features/housekeeping/domains/people/queries/people-adapters-production.test.ts -t "watched state|hydrates desk|strict unified"
|
||||
Test Files 2 failed (2)
|
||||
Tests 3 failed | 22 skipped (25)
|
||||
```
|
||||
|
||||
GREEN:
|
||||
|
||||
```text
|
||||
pnpm exec vitest run --coverage.enabled=false src/features/housekeeping/domains/people/models.test.ts src/features/housekeeping/domains/people/queries/people-queries.test.ts src/features/housekeeping/domains/people/queries/people-adapters-production.test.ts -t "watched state|hydrates desk|strict unified|normalizes BigInt"
|
||||
Test Files 3 passed (3)
|
||||
Tests 4 passed | 27 skipped (31)
|
||||
```
|
||||
|
||||
### Moderation capability equality
|
||||
|
||||
RED captured before direct authorization:
|
||||
|
||||
```text
|
||||
pnpm exec vitest run --coverage.enabled=false src/features/housekeeping/domains/people/queries/people-queries.test.ts -t "eleven-permission"
|
||||
Test Files 1 failed (1)
|
||||
Tests 1 failed | 18 skipped (19)
|
||||
Expected the route/run eleven-slug union; query metadata still contains six slugs.
|
||||
```
|
||||
|
||||
GREEN after the user directly authorized only this isolated metadata hunk:
|
||||
|
||||
```text
|
||||
pnpm exec vitest run --coverage.enabled=false src/features/housekeeping/domains/people/queries/people-queries.test.ts -t "eleven-permission"
|
||||
Test Files 1 passed (1)
|
||||
Tests 1 passed | 18 skipped (19)
|
||||
```
|
||||
|
||||
### Active-ban semantics
|
||||
|
||||
RED:
|
||||
|
||||
```text
|
||||
pnpm exec vitest run --coverage.enabled=false src/features/housekeeping/domains/people/queries/people-adapters-production.test.ts -t "permanent"
|
||||
Test Files 1 failed (1)
|
||||
Tests 1 failed | 4 skipped (5)
|
||||
Expected permanent expiresAt 0; received null.
|
||||
```
|
||||
|
||||
GREEN:
|
||||
|
||||
```text
|
||||
same command
|
||||
Test Files 1 passed (1)
|
||||
Tests 1 passed | 4 skipped (5)
|
||||
```
|
||||
|
||||
## Verification
|
||||
|
||||
```text
|
||||
pnpm exec vitest run --coverage.enabled=false src/features/housekeeping/domains/people/routes.test.ts src/features/housekeeping/domains/people/models.test.ts src/features/housekeeping/domains/people/queries/people-queries.test.ts src/features/housekeeping/domains/people/queries/people-adapters-production.test.ts
|
||||
Test Files 4 passed (4)
|
||||
Tests 21 passed (21)
|
||||
Tests 35 passed (35)
|
||||
|
||||
pnpm exec vitest run --coverage.enabled=false src/features/housekeeping/domains/people src/features/housekeeping/foundation/foundation-source-contract.test.ts src/features/housekeeping/foundation/authorization.test.ts src/features/housekeeping/foundation/capability-context.test.ts src/features/housekeeping/foundation/contracts/contracts.test.ts
|
||||
Test Files 8 passed (8)
|
||||
Tests 65 passed (65)
|
||||
pnpm exec vitest run --coverage.enabled=false src/features/housekeeping/domains/people src/features/housekeeping/foundation/foundation-source-contract.test.ts src/features/housekeeping/foundation/authorization.test.ts src/features/housekeeping/foundation/capability-context.test.ts src/features/housekeeping/foundation/server-capability-context.test.ts src/features/housekeeping/foundation/contracts/contracts.test.ts
|
||||
Test Files 9 passed (9)
|
||||
Tests 81 passed (81)
|
||||
|
||||
pnpm test:housekeeping
|
||||
Test Files 49 passed (49)
|
||||
Tests 416 passed (416)
|
||||
Tests 430 passed (430)
|
||||
|
||||
pnpm typecheck
|
||||
tsc --noEmit
|
||||
Exit 0
|
||||
|
||||
pnpm exec biome check --formatter-enabled=false <12 exact Task 11 TypeScript files>
|
||||
Checked 12 files. No fixes applied.
|
||||
pnpm exec biome check --formatter-enabled=false <11 exact changed Task 11 TypeScript files>
|
||||
Checked 11 files. No fixes applied.
|
||||
|
||||
git diff --check
|
||||
Exit 0
|
||||
|
||||
Reference in new issue
Block a user