feat(security): route more self-service reads through begin_with_user (BUNYIP-344) #364

Merged

longjacksonle merged 1 commit from feat/BUNYIP-344-route-remaining-selfservice-reads into main

2026-07-13 18:51:37 +02:00

longjacksonle commented

2026-07-13 18:48:58 +02:00

Owner

What

Follow-up to the BUNYIP-344 phase-1 slice (#363, merged): route two more genuinely self-service reads through begin_with_user, so per-user RLS applies to them once the NOBYPASSRLS role is provisioned.

Download entitlement gate (handlers/download.rs, under MemberUser): the caller's own application_entitlements read now runs inside a begin_with_user transaction.
Trusted-device revoke (handlers/user.rs, under AuthenticatedUser): the ownership lookup and the revoke UPDATE now share one begin_with_user transaction, so on the isolating role a device owned by another user is invisible (lookup returns None -> 404) and the UPDATE can only touch the caller's own row. The in-Rust ownership check is kept as defence in depth.

The repo methods (EntitlementRepository::active_application_ids, TrustedDeviceRepository::find_by_id / revoke) were widened from &PgPool to impl PgExecutor, which is backward compatible (&PgPool already satisfies the bound), so no other caller changes.

Deliberately not routed

user_totp and recovery_codes reads flow through TotpService, and email_change_requests reads are by-hash - all shared with the pre-auth login / 2FA / confirm flows where there is no current-user GUC. Routing them through begin_with_user would fail-close authentication on the isolating role, so they stay on the primary system pool with their application-level WHERE user_id filters. Decoupling TotpService from the login path is a separate, larger refactor.

Verification

just check-container green (fmt + clippy -D warnings + 102 tests).
Verified against a real Postgres: the isolation regression test now also seeds and asserts application_entitlements, proving user A cannot see user B's entitlement with a crafted no-WHERE query, alongside the existing trusted_devices read/write guards. The DB-backed test skips when RLS_TEST_DATABASE_URL is unset, so the no-Postgres CI stays green.

Depends on / relates to

Activation still needs BUNYIP-360 (provision the bunyip_app NOBYPASSRLS role + set APP_DATABASE_URL). Until then these paths use the primary-pool fallback and the change is a runtime no-op.

🤖 Generated with Claude Code

## What Follow-up to the BUNYIP-344 phase-1 slice (#363, merged): route two more genuinely self-service reads through `begin_with_user`, so per-user RLS applies to them once the NOBYPASSRLS role is provisioned. - **Download entitlement gate** (`handlers/download.rs`, under `MemberUser`): the caller's own `application_entitlements` read now runs inside a `begin_with_user` transaction. - **Trusted-device revoke** (`handlers/user.rs`, under `AuthenticatedUser`): the ownership lookup and the revoke `UPDATE` now share one `begin_with_user` transaction, so on the isolating role a device owned by another user is invisible (lookup returns `None` -> 404) and the `UPDATE` can only touch the caller's own row. The in-Rust ownership check is kept as defence in depth. The repo methods (`EntitlementRepository::active_application_ids`, `TrustedDeviceRepository::find_by_id` / `revoke`) were widened from `&PgPool` to `impl PgExecutor`, which is backward compatible (`&PgPool` already satisfies the bound), so no other caller changes. ## Deliberately not routed `user_totp` and `recovery_codes` reads flow through `TotpService`, and `email_change_requests` reads are by-hash - all shared with the pre-auth login / 2FA / confirm flows where there is no current-user GUC. Routing them through `begin_with_user` would fail-close authentication on the isolating role, so they stay on the primary system pool with their application-level `WHERE user_id` filters. Decoupling `TotpService` from the login path is a separate, larger refactor. ## Verification - `just check-container` green (fmt + clippy -D warnings + 102 tests). - Verified against a real Postgres: the isolation regression test now also seeds and asserts `application_entitlements`, proving user A cannot see user B's entitlement with a crafted no-WHERE query, alongside the existing `trusted_devices` read/write guards. The DB-backed test skips when `RLS_TEST_DATABASE_URL` is unset, so the no-Postgres CI stays green. ## Depends on / relates to Activation still needs BUNYIP-360 (provision the `bunyip_app` NOBYPASSRLS role + set `APP_DATABASE_URL`). Until then these paths use the primary-pool fallback and the change is a runtime no-op. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

longjacksonle added 1 commit

2026-07-13 18:48:58 +02:00

feat(security): route more self-service reads through begin_with_user (BUNYIP-344)

E2E / Playwright against deployment (pull_request) Failing after 27s

Details

Create release / Create release from merged PR (pull_request) Has been skipped

Details

Check / fmt + clippy + build + tests (pull_request) Successful in 51m58s

Details

17d67b2f85

Extend the per-user RLS coverage from BUNYIP-344 phase 1 to two more genuinely self-service handler paths, so they run on the NOBYPASSRLS pool with the app.current_user_id GUC set:

- The download entitlement gate (handlers/download.rs, under MemberUser) now reads the caller's own application_entitlements inside a begin_with_user transaction.
- The trusted-device revoke handler (handlers/user.rs, under AuthenticatedUser) now does its ownership lookup and the revoke UPDATE inside one begin_with_user transaction, so on the isolating role a device owned by another user is invisible (lookup returns None -> 404) and the UPDATE can only touch the caller's own row. The in-Rust ownership check is kept as defence in depth.

The repo reads/writes involved (EntitlementRepository::active_application_ids, TrustedDeviceRepository::find_by_id and revoke) were widened from &PgPool to impl PgExecutor, which is backward compatible because &PgPool already satisfies the bound, so no other caller changes.

Deliberately NOT routed: the user_totp and recovery_codes reads flow through TotpService, and the email_change_requests reads are by-hash, both shared with the pre-auth login / 2FA / confirm flows where no current-user GUC exists. Routing them through begin_with_user would fail-close authentication on the isolating role, so they stay on the primary system pool and keep their application-level WHERE user_id filters. Decoupling TotpService from the login path is a separate, larger refactor.

Verified against a real Postgres: the isolation regression test now also seeds and asserts application_entitlements, proving user A cannot see user B's entitlement with a crafted no-WHERE query, alongside the existing trusted_devices read/write guards. Full just check-container green (fmt + clippy -D warnings + 102 tests); the DB-backed test skips when RLS_TEST_DATABASE_URL is unset.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L5dcYueNHByRnWJDYoDX1W