mirror of
https://github.com/Stirling-Tools/Stirling-PDF.git
synced 2026-09-03 05:10:16 +03:00
EthanHealy01
08b08aa8a1
Review Flow PR 3. Stacked on #7296. A recorded failure becomes readable by the person who caused it. ## What changes Before this, reading or triaging a failure required leader permissions: `FileRunEventController.requireFailureReviewAllowed()` returned 403 to anyone who could not edit policies. #7296 lets any user report a failure, so they could file into a queue they could never read. That gate is removed from the endpoints and the decision moves into `FileRunEventService`: | Caller | Reads and closes | |---|---| | Team leader or admin | the whole team's failures (unchanged) | | Anyone else | only failures where `actor` is them | | Team unresolvable | nothing | | Name unresolvable | nothing | `GET /kinds` is also opened. It returns static enum metadata, and a member needs it to render failures they can already see. ## Additions - An `actor` predicate on both list queries in `FileRunEventRepository`, threaded through `FileRunEventStore.list`. - `ReadScope` (permitted, teamId, actor) replacing `TeamScope`, with `wholeTeam` / `mine` / `denied` factories. - An actor filter on `dispatch`, so acting on another person's row answers **404, not 403** — the same response as an id that does not exist. ## Fixes - **`report()` filed rows under the wrong team.** It took the team from the read scope, which returns null for a caller who cannot be named, so such a report landed unteamed in the bucket every team shares. It now uses a dedicated `currentTeamId()`. - **`forgetFiles` narrows to the caller even for a leader.** File ids are minted by each client, so scoping on team alone would let one caller close a colleague's incidents by naming ids. - The controller no longer injects `PolicyManagementAuthority` or `ApplicationProperties`; with the gate gone it decides nothing. ## Team isolation Unchanged and covered by database-backed tests rather than mocks. `FileRunEventStoreDbTest` asserts that a caller with a team sees only their own team's rows and never the unteamed ones, and that the actor predicate narrows within a team without ever widening across one. Delete either clause from the JPQL and one of those tests fails. No endpoint accepts a team parameter; the team always comes from the authenticated principal. **Attribution is fixed here too, because this PR depends on it.** A failure's actor was read from the MDC audit principal, which carries the BILLING identity — for a stored policy, always its owner. Since reads are now narrowed to the rows you are the actor on, a wrong actor means the member who caused a failure and holds the document reads nothing, while the policy owner is handed incidents from runs they never triggered. The triggering user is now carried on the run, separate from the billing principal and the output owner, and is null for a trigger-fired sweep so an unattended failure stays ownerless. `PolicyFailureAttributionTest` runs the real engine, recorder, store and service together. The two sides used to assert independently — the engine's test matched the actor with `any()`, which is how this went unnoticed. ## How to test Needs a proprietary or SaaS build with login enabled and two accounts in the same team, one a leader and one not. `task dev:all` gives you the stack. 1. **As the member**, fail a tool: open a PDF and run **Remove Password** with a wrong password. 2. **Still as the member**, go to `/processor/documents` → **Failures**. Before this PR you got nothing here. Now you see your own row, and only yours. 3. **As the leader**, open the same view. You see the whole team's rows, including the member's. 4. **Member cannot reach a colleague's row.** As the leader, copy a row's id from **Show raw JSON**. As the member, `POST /api/v1/file-run-events/{thatId}/actions/DISMISS`. It answers **404**, and the row is untouched — it must not answer 403, which would confirm the row exists. 5. **Member can close their own.** Dismiss your own row as the member. It leaves the default view. 6. **Deleting a file only closes your own rows.** As the leader, delete a file in your editor. The member's incidents are untouched even if the leader's client happened to name the same ids. ## Migration None. `actor` is an existing column; this only adds predicates to existing queries.