psa-systems/mokosh-server
Watch
9
0
Fork
You've already forked mokosh-server
 1
Code Issues Pull requests Projects Releases 10 Packages Wiki Activity Actions

fix(workers): scope notification template lookup; audit from_trusted #195

Merged
David merged 1 commit from feat/pms-261-worker-from-trusted-audit into main 2026-06-13 15:24:34 +02:00
Conversation 0 Commits 1 Files changed 9 +271 -4
David commented 2026-06-13 15:17:33 +02:00
Owner
Copy link

Step 6 of PMS-255 per-user isolation. Audits the cross-tenant background workers (calendar reminder, SLA sweep, billing recurring, RMM ingest, notification dispatcher) and every from_trusted escape hatch against the fail-closed RLS policy (migration 038).

Behavioral fix: NotificationsService::dispatch fetched the rule's notification_templates row on the bare pool with no app.current_tenant GUC. notification_templates carries a tenant_id and is covered by the fail-closed policy, so under an unprivileged (NOBYPASSRLS) connection that read matches zero rows and silently drops the template, degrading to the default subject/body. The lookup now runs through begin_with_tenant(tenant_id); the template_id comes off this tenant's own rule, so the row resolves under the same tenant.

Audit findings (no behavior change, SAFETY notes only): each worker already wraps its per-tenant unit of work (claim_reminder, claim_sla_notification, dispatch, generate_due_recurring_invoices) in begin_with_tenant, so the GUC is set to exactly the tenant being processed. The cross-tenant enumeration scans in calendar::due_reminders and the SLA sweep are deliberately unscoped (they project tenant_id off DB rows to drive the loop) and run as the BYPASSRLS migration role; both now carry a note explaining the posture and the NOBYPASSRLS migration path. Every from_trusted site (calendar/sla workers, portal ticket bridges, tenants super-admin handlers, the tenants create path, and the legacy auth dispatch/audit sites) now carries a current // SAFETY (PMS-261): note tying the trusted tenant to an authenticated or DB-derived source.

Adds tests/worker_tenant_isolation.rs: drives the dispatcher (the shared worker fan-out) for tenant A while tenant B holds a matching rule plus template, and asserts the worker neither reads tenant B's rule nor writes a notification row under tenant B, and that tenant A's row resolves against tenant A's own template.

#PMS-261

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com

Step 6 of PMS-255 per-user isolation. Audits the cross-tenant background workers (calendar reminder, SLA sweep, billing recurring, RMM ingest, notification dispatcher) and every `from_trusted` escape hatch against the fail-closed RLS policy (migration 038). Behavioral fix: `NotificationsService::dispatch` fetched the rule's `notification_templates` row on the bare pool with no `app.current_tenant` GUC. `notification_templates` carries a `tenant_id` and is covered by the fail-closed policy, so under an unprivileged (NOBYPASSRLS) connection that read matches zero rows and silently drops the template, degrading to the default subject/body. The lookup now runs through `begin_with_tenant(tenant_id)`; the `template_id` comes off this tenant's own rule, so the row resolves under the same tenant. Audit findings (no behavior change, SAFETY notes only): each worker already wraps its per-tenant unit of work (`claim_reminder`, `claim_sla_notification`, `dispatch`, `generate_due_recurring_invoices`) in `begin_with_tenant`, so the GUC is set to exactly the tenant being processed. The cross-tenant enumeration scans in `calendar::due_reminders` and the SLA sweep are deliberately unscoped (they project `tenant_id` off DB rows to drive the loop) and run as the BYPASSRLS migration role; both now carry a note explaining the posture and the NOBYPASSRLS migration path. Every `from_trusted` site (calendar/sla workers, portal ticket bridges, tenants super-admin handlers, the tenants create path, and the legacy auth dispatch/audit sites) now carries a current `// SAFETY (PMS-261):` note tying the trusted tenant to an authenticated or DB-derived source. Adds tests/worker_tenant_isolation.rs: drives the dispatcher (the shared worker fan-out) for tenant A while tenant B holds a matching rule plus template, and asserts the worker neither reads tenant B's rule nor writes a notification row under tenant B, and that tenant A's row resolves against tenant A's own template. #PMS-261 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
fix(workers): scope notification template lookup; audit from_trusted
Some checks failed
E2E (staging) / Playwright against staging (pull_request) Failing after 20s
Details
Check / fmt + clippy + compile + tests (pull_request) Successful in 1m40s
Details
Build OCI container / Build and push mokosh-api image (push) Successful in 3m45s
Details
Create release / Create release from merged PR (pull_request) Has been skipped
Details
2a9f970f59 
Step 6 of PMS-255 per-user isolation. Audits the cross-tenant background workers (calendar reminder, SLA sweep, billing recurring, RMM ingest, notification dispatcher) and every `from_trusted` escape hatch against the fail-closed RLS policy (migration 038).

Behavioral fix: `NotificationsService::dispatch` fetched the rule's `notification_templates` row on the bare pool with no `app.current_tenant` GUC. `notification_templates` carries a `tenant_id` and is covered by the fail-closed policy, so under an unprivileged (NOBYPASSRLS) connection that read matches zero rows and silently drops the template, degrading to the default subject/body. The lookup now runs through `begin_with_tenant(tenant_id)`; the `template_id` comes off this tenant's own rule, so the row resolves under the same tenant.

Audit findings (no behavior change, SAFETY notes only): each worker already wraps its per-tenant unit of work (`claim_reminder`, `claim_sla_notification`, `dispatch`, `generate_due_recurring_invoices`) in `begin_with_tenant`, so the GUC is set to exactly the tenant being processed. The cross-tenant enumeration scans in `calendar::due_reminders` and the SLA sweep are deliberately unscoped (they project `tenant_id` off DB rows to drive the loop) and run as the BYPASSRLS migration role; both now carry a note explaining the posture and the NOBYPASSRLS migration path. Every `from_trusted` site (calendar/sla workers, portal ticket bridges, tenants super-admin handlers, the tenants create path, and the legacy auth dispatch/audit sites) now carries a current `// SAFETY (PMS-261):` note tying the trusted tenant to an authenticated or DB-derived source.

Adds tests/worker_tenant_isolation.rs: drives the dispatcher (the shared worker fan-out) for tenant A while tenant B holds a matching rule plus template, and asserts the worker neither reads tenant B's rule nor writes a notification row under tenant B, and that tenant A's row resolves against tenant A's own template.

#PMS-261

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
David merged commit a927e4e9da into main 2026-06-13 15:24:34 +02:00
David deleted branch feat/pms-261-worker-from-trusted-audit 2026-06-13 15:24:34 +02:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
psa-systems/mokosh-server!195
No description provided.