feat(middleware): ship withPostgresClient and withPostgresAdminClient - #115
Open
tomaspozo wants to merge 7 commits into
Open
feat(middleware): ship withPostgresClient and withPostgresAdminClient#115tomaspozo wants to merge 7 commits into
tomaspozo wants to merge 7 commits into
Conversation
…arden it
Renames the export to sit alongside withSupabaseClient /
withSupabaseAdminClient, and extracts the pool into a shared core module so
the service-role companion can reuse it. Safe to rename now: the old name
exists only on 1.5.0-rc.* / beta, never on a stable release.
Three correctness fixes alongside it:
- The pool cache was keyed on nothing, so a second connectionString in the
same process silently queried the first database. Now keyed per string.
- The missing-connection-string 500 returned { error }, not the package's
standard { message, code }.
- An unguarded rollback in the catch could replace the caller's real error
with a connection error.
Adds unit coverage for each, plus a type-level check that composing without
an upstream jwtClaims stays a compile-time error.
Contributes ctx.postgresAdmin — a pg client that bypasses RLS, exported from ./middleware/postgres-admin. Queries run as-is under the connection-string role: no claim injection, no role switch, no wrapping transaction. Declares no upstream prerequisite, so unlike withPostgresClient it composes under auth: 'secret' and auth: 'none'. Shares the pool cache with the scoped half — same connection string, one pool. That is safe because everything the scoped half sets is transaction-local, so a connection always returns clean. Kept as a second middleware rather than a property on ctx.postgres: defineMiddleware contributes exactly one ctx key, and the split keeps the RLS bypass visible at the composition site.
Adds /my-notes-pg and /all-notes-pg to the core Node app and the Deno edge function, both running the identical unfiltered SELECT — one through ctx.postgres, one through ctx.postgresAdmin. user2 sees none of user1's rows through the scoped client and sees them through the admin one, which proves claim injection, the role drop, and the bypass in a single contrast. The edge function passes connectionString explicitly from E2E_DB_URL: the CLI injects a SUPABASE_DB_URL addressing the database by container name, and Deno's DNS resolver rejects the underscores in it. The Node app still covers the SUPABASE_DB_URL default path.
Adds docs/postgres.md covering both halves, the SQL each query runs, the two composition paths, table grants, the RLS bypass and why it is a separate middleware, and guidance to write policies with the auth.* helpers rather than reading request.jwt.claim.* directly. Wires both subpaths into typedoc entryPoints — without which neither export reached api-docs/ — and adds README sections, Exports and env-var rows, and api-reference entries.
commit: |
pg-pool only removes a client when release() is given a truthy argument, so the previous release() returned a connection whose transaction could not be unwound straight back to the pool — potentially still inside the caller's transaction with their role set. That was survivable while the pool served one middleware. It is not now that withPostgresAdminClient shares it: that middleware begins no transaction and sets up no session state, so it would silently inherit the leftover role on the next checkout.
withPostgresClient silently mapped every role that was not 'authenticated' to 'anon'. For a forged service_role that was the intent, but Supabase also supports custom roles via the role claim, and RLS applies to those normally — so a legitimate `role: manager` token was being answered with zero rows and no indication that the role was the reason. Now only 'authenticated' and 'anon' are assumed, and anything else short-circuits with a 500 and code UNSUPPORTED_ROLE before the handler runs or a connection is checked out. service_role gets a message pointing at withPostgresAdminClient; other roles are named in the error. Custom roles remain unsupported — the reason is that PostgREST connects as the unprivileged authenticator, where `grant <role> to authenticator` is itself the authorization, while we connect as postgres and have no such boundary to lean on. Documented, and tracked separately. Also hoists the per-request claims serialization out of the per-query path.
The table covered 8 of 13 entry points. Adding the postgres pair made the omission look deliberate rather than incidental — a reader could reasonably conclude withClaims has no subpath, which matters because it is the documented prerequisite for composing withPostgresClient standalone.
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
1 participant
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.
Ships the RLS-scoped Postgres middleware and its service-role companion as a pair, closing SDK-1207 and SDK-1208.
withPostgresalready existed onmain— it arrived with #88 — so most of this is finishing the job: correctness fixes, the rename that puts it alongside its siblings, the admin half, e2e proof, and docs.What lands
withPostgresClient./middleware/postgresctx.postgreswithPostgresAdminClient./middleware/postgres-adminctx.postgresAdminThe pair mirrors
withSupabaseClient/withSupabaseAdminClient. They stay two middleware becausedefineMiddlewarecontributes exactly one ctx key — and because that keeps the RLS bypass visible at the composition site, greppable per handler. They share one connection pool, which is safe because everything the scoped half sets is transaction-local.Release ordering — worth knowing before merging #114
This renames
withPostgres→withPostgresClient. That is free right now: the old name exists only on1.5.0-rc.*and1.5.0-beta.0, while npmlatestis still1.4.1, so no stable consumer has ever imported it.If #114 (
chore(main): release server 1.5.0) merges before this PR, 1.5.0 ships the old name and the rename becomes a genuine breaking change. So this wants to land first. Timing of the release itself is the team's call.Deliberately not marked
feat!/BREAKING CHANGE:— that would make release-please cut a major, which would be wrong for a name no stable release has carried.Role handling: refuse, don't downgrade
withPostgresClientpreviously mapped every role that wasn'tauthenticatedtoanon. For a forgedservice_rolethat was the intent — but Supabase also supports custom Postgres roles via theroleclaim, with policies writtento manager, and RLS still applies to them. So a legitimaterole: managertoken was answered with zero rows and no clue why.Only
authenticatedandanonare assumed now. Anything else short-circuits with a 500 andcode: 'UNSUPPORTED_ROLE'before the handler runs or a connection is checked out —service_rolegets a message pointing atwithPostgresAdminClient, other roles are named in the error. A missing or absentroleclaim is stillanon.Custom roles remain unsupported, and the reason is worth stating: PostgREST connects as the unprivileged
authenticator, sogrant manager to authenticatoris the authorization and Postgres decides what's reachable. We connect aspostgres(rolbypassrls = t), with no equivalent boundary — so v1 assumes a fixed pair rather than trusting the claim. Real support is SDK-1504, usingpg_has_role('authenticator', …) and not rolsuper and not rolbypassrls, which rejectsservice_roleautomatically without a denylist.No impact on key-based auth:
jwtClaimsis only populated inusermode, soauth: 'secret'and dual-auth compositions are unaffected.Correctness fixes to the existing middleware
connectionStringin the same process silently queried the first database. Now keyed per connection string.{ error }instead of the package-wide{ message, code }thatwithClaimsand friends use.rollbackin thecatchreplaced the caller's error with a connection error. Worse,pg-poolonly discards a client whenrelease()gets a truthy argument, so a connection whose transaction couldn't be unwound went straight back to the pool — possibly still in that transaction with the caller's role set. Harmless when the pool served one middleware; not oncewithPostgresAdminClientshares it and sets up no session state of its own.Verification
jwtClaimsstays a compile-time error — verified by removing the@ts-expect-errorand confirming the message ismiddleware-prereq: key 'jwtClaims' is not yet on the context (check ordering)./my-notes-pgand/all-notes-pgrun the identical unfilteredselect … from notes. user2 gets none of user1's rows throughctx.postgresand gets them throughctx.postgresAdmin. Without claim injection user1 would see nothing; without the role drop the connection would still be superuser and user2 would see everything. The contrast can't pass by accident.lint,typecheck,typecheck:e2e,build,attw,smoke(26 entrypoints),typedoc(no new warnings), andjsr publish --dry-run(no slow types) all clean.The edge function passes
connectionStringexplicitly fromE2E_DB_URL: the CLI injects aSUPABASE_DB_URLaddressing the database by container name (supabase_db_<project>), and Deno's DNS resolver rejects the underscores. Local-stack artifact only — a deployed function gets a real pooler hostname. The Node app still covers theSUPABASE_DB_URLdefault path.Scope notes
withPostgres()wrapper. The name is left unused and reserved so adding one later is non-breaking — tracked in SDK-1503 with the arguments both ways.docs/postgres.mdnow documents why onlyrequest.jwt.claimsis set and not the legacyrequest.jwt.claim.*GUCs.