feat(#462): add federation get verb #683
Assignees
jason.woltje
Clear assignees
jason.woltje
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: mosaicstack/stack#683
Reference in New Issue
Block a user
Delete Branch "feat/federation-m3-verb-get"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
Implements FED-M3-06 get verb for inbound federation reads:
Verification
Notes
86e106fcc9after M3-05 merged.Refs #462
REVIEW-OF-RECORD — REQUEST CHANGES (#683, head
87a5f479ec). The scoped happy-path tests pass, but the trust-boundary requirements for FED-M3-06 are not met yet.CRITICAL #1 — getNote uses OR-widening instead of the M3-05 AND-intersect. In get-query.service.ts, the decision is
hasPersonalAccess || hasTeamAccess(if (!hasPersonalAccess && !hasTeamAccess) deny). That leaks both directions: (a) any user's note on an authorized/team-visible mission is returned becausemissionIds.includes(row.missionId)is enough, and (b) the subject user's note on a mission outside the authorized mission/project set is returned becauseincludePersonal && row.userId === subjectUserIdis enough. This must mirror the approved list verb:missionTasks.userId = subjectUserIdANDmissionTasks.missionId in authorizedMissionIds, with no OR personal/team branch.CRITICAL #2 — missing federation context still becomes a plain Error/500. GetController currently throws
new Error('Federation context missing after auth guard'); the required behavior is the structured federation unauthorized envelope (401), matching ListController.IMPORTANT #3 — unsupported resources do not fail closed. getByResource has no default branch; if an unexpected resource reaches it,
query.get()can resolve undefined and the controller dereferencesresult.status, producing a 500 instead of an explicit deny/invalid-request path.IMPORTANT #4 — coverage does not prove the trust boundary. The current get-query tests are mocked/unit guardrails only; there is no PGlite/scoped regression proving cross-user note exclusion or subject-note-on-unauthorized-mission exclusion. Please add the same kind of DB-backed regressions that closed #682 for listNotes, plus controller coverage for missing-fed-context -> 401 and unsupported resource fail-closed.
Reviewer verification on head
87a5f479: scoped get tests 10/10 pass; gateway typecheck pass; gateway lint pass; prettier check pass for changed get/module/scratchpad files. These gates are green locally, but they do not cover the blockers above. REQUEST CHANGES.87a5f479ecto80a259b206REVIEW-OF-RECORD — APPROVE (#683, head
80a259b206). Re-review confirms all prior blockers from comment 15971 are resolved.Trust boundary: getNote now mirrors the approved list verb boundary. A note is returned only when
row.userId === subjectUserIdANDrow.missionIdis in the authorized mission IDs derived from grant/native-RBAC-visible projects. The old personal/team OR-widening branch is gone, so team scope no longer widens into another user's notes and personal subject ownership no longer bypasses the authorized mission/project set.Error handling/fail-closed: missing federation context now returns a structured
401 unauthorizedfederation envelope, not a plain Error/500.getByResourcenow has an explicit default denial for unsupported resources, which the controller maps to a structured 403 scope violation instead of dereferencing undefined.Coverage: the new PGlite regressions prove cross-user note exclusion, subject-note-on-unauthorized-mission exclusion, valid subject+authorized-mission intersection, and includePersonal=false exclusion. Controller tests cover missing-fed-context 401 and unsupported-resource fail-closed behavior.
Reviewer verification on head
80a259b2: scoped gateway get tests 17/17 pass; gateway typecheck pass; gateway lint pass; prettier check pass for changed get/module/scratchpad files. PR CI reported green on pipeline 1620. APPROVE.