[ericfitz]

Summary

Eight bug fixes, combined into one PR. Each issue's commits are kept in order; the squash title decides the version bump (fix:, so a PATCH bump; the spec changes in #1104 and #1109 also bump the schema version).

Verification (combined branch)

• make verify: verify: OK.
• make test-integration (PostgreSQL): 97 passed, 0 failed, 9 skipped; 8 packages passed.
• Full CATS run on the k3s dev cluster from this branch (20261018T170611Z): valid. 110,703 requests; 0.07% connection errors; 0.13% unauthenticated; no 5xx other than the expected 501s from DummyTransferEncoding; no undocumented status codes; all 19 fixtures survived. The only true positives were the 14 request_access 409s that the new rule 104 classifies; after reclassification there are 0.
• Reviews: a review per issue plus a final review of the combined tree; security review found no high-confidence issues; Oracle review of fix(dbtool): system setting writes bypass Timmy validation #1108 and fix: diagram PUT hangs under CATS fuzzing, followed by ~20s of transport failures #1115: APPROVED WITH NOTES (follow-up fix(dbtool): make setting seed writes atomic and reject non-string setting values #1117).

#1103: Count authenticated REST requests in the per-user rate limiter

Summary

The per-user REST rate limiter read the Gin context key user_id, which only the WebSocket handler sets, so it never counted a REST request. It now keys on userInternalUUID (the key the quota store and seeded quotas use), falling back to userID.

Client-visible change

Per-user limiting is enforced for the first time: authenticated responses carry X-RateLimit-* headers, and a user over the limit gets 429 with Retry-After (documented on every operation).

• Default quota: 1000 requests/minute and 60000/hour per user (api/user_api_quota_store.go). Admins can raise a user's quota with /admin/quotas/users/{user_id}.
• Service-account tokens share the owning user's bucket. WebSocket ticket requests count too.
• tmi-ux's REST pollers are sparse (collaboration dialog refresh, token heartbeat, session activity timer), and diagram edits go over WebSocket, so the defaults leave wide headroom.
• Dev config and the integration harness disable rate limiting. The CATS seed gives the fuzzing user 100000/minute.

Tests

• Unit: TestRateLimitMiddleware_JWTContextKeys uses the real JWT context keys. All 3 subtests failed before the fix and pass after it.
• End to end: TestUserAPIRateLimiting_Integration (opt-in, TMI_TEST_ENABLE_RATE_LIMITING=true make test-integration). It logs in a real user through OAuth and sets that user's quota to 5/minute, then calls GET /threat_models.
  • On main: it fails at request 1, which has no X-RateLimit-Limit header.
  • On this branch: requests 1-5 succeed with X-RateLimit-Remaining decreasing, and request 6 gets 429 with Retry-After and rate_limit_exceeded. Suite: 100 passed, 0 failed.
• Note: the k3s dev cluster runs with server.disable_rate_limiting=true, so CATS on k3s does not exercise this limiter.

#1104: Restrict survey template status to active, inactive or archived

Summary

SurveyBase.status allowed any string up to 128 characters, but the column is size:20 and only active, inactive and archived are valid. POST and PUT /admin/surveys copied status through unvalidated, so a long value failed in the database.

• The spec now has enum: [active, inactive, archived] on the field (still nullable) and on the GET /admin/surveys?status= filter.
• One shared validateSurveyStatus is used by POST, PUT and PATCH. It returns 400 invalid_input naming status.
• IsConfidentialQueryParam's stale "secret_project" description is corrected.
• No column change, so no Oracle review is needed. x-go-type: string keeps the generated fields *string.

Client-visible change

POST, PUT and PATCH with any other status, and GET /admin/surveys?status=<other>, now return 400 (the GET previously returned 200 with an empty list).

Tests

• The handler tests failed with the validator disabled and pass with it.
• Validator-middleware tests show null and omitted values are accepted and unknown values get 400.

#1105: Return documented 400 for client credential create constraint errors

Summary

POST /me/client_credentials mapped a DB constraint or duplicate error to 409, which the operation does not document. It now returns 400 invalid_input. The only unique column, client_id, is server-generated, so no real conflict case exists.

The sweep checked all 17 other StatusConflict handler sites, and every one belongs to an operation that documents 409. The shared store-error mapper's duplicate branch is tracked separately in #1118.

Tests

TestRespondClientCredentialCreateError: the constraint and duplicate cases got 409 before the fix and get 400 after it.

#1106: Apply the is_confidential filter to the triage survey response list

Summary

GET /triage/survey_responses documented is_confidential but ignored it. The parameter is now intersected with the caller's reviewer scope from #1100 and can never widen it:

• A Security Reviewers-only member asking for is_confidential=true gets 200 with an empty list.
• A member of both reviewer groups gets the parameter applied as given.
• A non-boolean value gets 400 from the generated binder.

Handler-only change; not DB-touching.

Tests

• Matrix {Security Reviewers, Confidential Project Reviewers, both} x {absent, true, false}, plus a non-reviewer. Four cases failed against the pre-fix handler.
• The 400 case runs through the real router.

#1107: Honor group-based access in collaboration sessions

Summary

Collaboration authorization passed nil groups at four sites:

• the connect-time reader check
• presenter reassignment
• checkMutationPermission
• checkReadPermission

A user whose writer or reader role came only from a group grant was therefore refused in a collaboration session, although REST allowed them. The WebSocket ticket path also never carried groups.

• The ticket now records the minting request's JWT groups claim. The ticket validator sets userGroups, and WebSocketClient.Groups holds them for the socket's lifetime, the same freeze semantics as JWT claims on REST.
• Old tickets decode to no groups and fail closed.
• Session participant listing (GetActiveSessionsForUser and GetActiveSessions) now resolves each participant's permissions with that participant's own groups, not the requester's.

Tests

• TestWebSocketGroupBasedAccess: a group-only writer can mutate, a group-only reader can read but not mutate, and no-group users are denied.
• Ticket round-trip tests cover legacy tickets.
• TestBuildSessionParticipants_UsesParticipantGroups.
• Each failed before its fix.

#1108: Validate system settings in dbtool before writing them

Summary

tmi-dbtool wrote system_settings directly, bypassing the semantic validators SettingsService.Set uses (#1102).

• api.ValidateSettingSemantics is now exported, so the rules stay in one table. dbtool validates every setting before its first write in --import-config (including --dry-run), in the data seed pre-pass, and in seedSetting.
• An invalid value exits non-zero with a message naming the key, and nothing is written.
• timmy.llm_max_tokens gains a validator matching the startup check, so all three keys the startup check validates are covered at runtime.

Oracle review (oracle-db-admin): APPROVED WITH NOTES. No SQL, model or write-order change. The non-transactional write loop and non-string seed values are tracked in #1117.

Tests

TestRunConfigSeed_RejectsInvalidTimmySetting, TestRunConfigSeed_RejectsNegativeMaxTokens and TestRunConfigSeed_DryRunRejectsInvalid each failed before their fix.

#1109: Reject root-path and prohibited-field JSON Patch operations

Summary

JSON Patch path and prohibited-field checks now live in shared helpers that every JSON Patch handler uses.

• Shape: ValidatePatchOperationPaths rejects a root path (""), any path not starting with /, and move or copy with such a from. All 20 JSON Patch operations go through it, 15 via ParsePatchRequest and two directly (diagram PATCH and bulk threat PATCH).
• Prohibited fields: FindProhibitedPatchPath matches by segment, checks path and the from of move and copy, and covers sub-paths. It replaces the per-handler exact-match loops for project, team, notes and surveys.
• Owner gate: CheckOwnershipChanges now counts move and copy targets and sources, and /owner/... sub-paths.
• Survey status: survey response PATCH rejects removing /status, or moving it away, with 400 instead of returning 200 and dropping the change.
• Bulk threat PATCH: it validates every entry before applying any, and applies the same prohibited list as single-threat PATCH. The store-time partial-commit risk that remains is tracked in fix(api): bulk PATCH/PUT can partially commit when a store-time error hits a later entry #1120.
• Spec: the JsonPatchDocument path pattern is now ^(/[^/]*)+$. A from property is declared (additionalProperties: false previously rejected every move and copy).

Client-visible change

• Root-path, non-slash and prohibited-field patches return 400.
• Move and copy now pass request validation.

Tests

• Handler-level tests failed before each fix (200) and pass after it (400): survey response root path, diagram, bulk threats (store not called), project, team, notes, and survey status removal.

#1115: Seed CATS decoys so DELETE does not consume nested-path fixtures

Summary

The hang: it didn't reproduce in isolation. A rerun of only /threat_models/{threat_model_id}/diagrams/{diagram_id} on the k3s NodePort, with server logs captured (run 20261018T072650Z), made 16,854 requests with 0 transport errors, a maximum response time of 1,007 ms, and no 5xx. The original timed-out request was one that request validation rejects in 152 ms or less.

The real defect: the rerun exposed it. CATS walks paths in lexical order, so DELETE on the diagram path ran at test 202 and consumed the only seeded diagram before every nested diagram path ran.

• The seed adds throwaway diagram, threat, document, asset, note, repository and survey fixtures. The refData decoy list points each exact anchor path at its throwaway, and the global ids resolve by name to the real fixtures.
• The real repository gets a name, and a stale metadata target that a live seed caught is fixed. TestCatsSeedRefsAllResolve now checks every seed ref offline.
• scripts/cats-check-fixtures.py reads the fixture manifest instead of a hard-coded list of 6 anchors (pytest: 10 passed).

Verification

• A live path-only run from this branch (20261018T075612Z) passed the run-validity gates: 0 transport errors, 0 true positives.
• The decoy diagram was consumed and the real diagram survived (checked in the DB).
• The survival report probed all 19 fixtures and every one returned 200.
• New false-positive rule DOCUMENT_REQUEST_ACCESS_NOT_PENDING_409 (rule 104): with the seeded document now reachable, every fuzzed POST …/documents/{document_id}/request_access gets the documented 409 because that document is not in pending_access. The rule matches only that operation, status and server message.

Fixes #1103
Fixes #1104
Fixes #1105
Fixes #1106
Fixes #1107
Fixes #1108
Fixes #1109
Fixes #1115

🤖 Generated with Claude Code

https://claude.ai/code/session_015szS68rpMLaq712AFsbMfr