fix(hostnames): read tenant hostnames through the BYPASSRLS role - #46
Merged
Conversation
Every hostname lookup has been returning nothing in production since it
shipped. `organisations` and `teams` carry FORCE ROW LEVEL SECURITY, and this
service reached for the tenant-scoped client — the one `uoa_app` connects as.
On a `/domain/*` route there is no tenant context to satisfy the policies with:
no `app.org_id`, no `app.user_id`, and the domain branch of
`organisations_select` additionally requires an `org_members` row for that
absent user. So no branch can ever match.
It does not error. It returns zero rows, and every caller read that as an
answer:
* /domain/organisations/resolve and /domain/teams/resolve answered 404 for
tenants that plainly exist — `nessie-works` and `kilomayo` are both present
on this domain and neither could be resolved.
* /domain/slug-available answered `available: true` for every slug, including
taken ones. A create dialog would have told two tenants the same address was
free; only the unique constraint stood behind it.
The fix is the pattern this codebase already uses for exactly this case:
`team-avatar.service.ts` is a `/domain/*` route reading `teams` and it goes
through `getAdminPrisma()` — the BYPASSRLS role that `DATABASE_ADMIN_URL` names
in production, which `test-db.ts` documents as the one domain-hash auth runs on.
That makes the domain predicate the only remaining boundary, so the file now
says so where someone editing it will read it: every query filters on the
caller's own domain, teams through their organisation's, and anything added
here must keep that.
The unit tests could not have caught this — they inject `deps.prisma`, so they
exercised the query shapes and never the client that runs them. The new
integration test uses the real roles: `uoa_admin` finds the tenant, `uoa_app`
finds nothing and reports a taken slug as available, which is the failure
exactly. It also pins the cross-domain refusals, since those are now carried by
the predicate rather than by the policy.
DB-less 1883 passed; integration 396 passed; lint clean.
Co-Authored-By: Claude Opus 5 <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Every tenant-hostname lookup has been returning nothing in production since it shipped.
organisationsandteamscarryFORCE ROW LEVEL SECURITY, andteam-hostname.service.tsreached for the tenant-scoped client — the oneuoa_appconnects as. On a/domain/*route there is no tenant context to satisfy the policies with: noapp.org_id, noapp.user_id, and the domain branch oforganisations_selectadditionally wants anorg_membersrow for that absent user. No branch can match.It does not error. It returns zero rows, and every caller read that as an answer:
/domain/organisations/resolveand/domain/teams/resolveanswered 404 for tenants that plainly exist —nessie-worksandkilomayoare both on this domain and neither resolved./domain/slug-availableansweredavailable: truefor every slug, taken ones included. A create dialog would have offered two tenants the same address; only the unique constraint stood behind it.The fix is the pattern already in this codebase for exactly this case:
team-avatar.service.tsis a/domain/*route readingteams, and it goes throughgetAdminPrisma()— the BYPASSRLS roleDATABASE_ADMIN_URLnames in production, whichtest-db.tsdocuments as the role domain-hash auth runs on.That leaves the domain predicate as the only boundary, so the file now says so where someone editing it will read it: every query filters on the caller's own domain, teams through their organisation's, and anything added here has to keep doing that.
Testing
deps.prisma, so they exercised query shapes and never the client that runs them.API/tests/integration/team-hostname-rls.test.tsuses the real roles:uoa_adminfinds the tenant;uoa_appfinds nothing and reports a taken slug as available — the shipped failure, reproduced. It also pins the cross-domain refusals, which are now carried by the predicate rather than by the policy.🤖 Generated with Claude Code