fix: set pgrst.db_schemas in 0028/0029 tests#162
Merged
Conversation
#160 added a pgrst.db_schemas filter to the function lints to match PostgREST's API exposure scope, but the test SQL didn't set the guc, so the positive cases stopped firing. Mirrors the pattern from test/sql/0023_sensitive_columns_exposed.sql.
soedirgo
approved these changes
Apr 27, 2026
joshenlim
pushed a commit
to supabase/supabase
that referenced
this pull request
Apr 27, 2026
…R functions (#45260) ## I have read the [CONTRIBUTING.md](https://github.com/supabase/supabase/blob/master/CONTRIBUTING.md) file. YES ## What kind of change does this PR introduce? Feature — wires up three new advisor lints landed in splinter, and updates the self-hosted SQL bundle for the existing `pg_graphql_anon_table_exposed` lint to track splinter's correctness fixes. Companion to `supabase/splinter` #160 (already merged) and #162 (test fix in flight). ## What is the current behavior? Splinter's `main` now exposes four lints in the pg_graphql / SECURITY DEFINER family: - `pg_graphql_anon_table_exposed` (0026, existing) — wired into Studio in #45253; SQL in `packages/pg-meta` is the original version that uses `has_table_privilege` and the relkind set `('r','p','v','m')`. - `pg_graphql_authenticated_table_exposed` (0027, new) — paired check against the `authenticated` role. Studio renders any new finding without a `lintInfoMap` entry as a row with no icon, no title mapping, and no "Fix" CTA. Self-hosted users do not see the lint at all because `packages/pg-meta` does not include it. - `anon_security_definer_function_executable` (0028, new) — `SECURITY DEFINER` function executable by `anon`. Same Studio + self-hosted gaps as 0027. - `authenticated_security_definer_function_executable` (0029, new) — same against `authenticated`. Splinter has also updated 0026 itself (PR #160) in two ways that need to flow into the self-hosted SQL bundle: 1. **`relkind` filter:** `('r','p','v','m')` → `('r','v','m','f')`. Drops partitioned table roots (pg_graphql does not expose them; their leaf partitions are still covered as `'r'`) and adds foreign tables, which pg_graphql does expose. 2. **Privilege predicate:** `has_table_privilege(role, oid, 'SELECT')` → `EXISTS` over `pg_attribute` calling `has_column_privilege`. Catches column-level grants such as `GRANT SELECT (col) ON t TO anon`, which pg_graphql's introspection exposes but `has_table_privilege` missed. Cloud projects auto-fetch `splinter.sql` via the platform mgmt-api's `getLintSql` (1-hour cache TTL), so they pick up #160's lint and SQL changes independently of this PR. This PR is about the Studio display mapping and the self-hosted SQL bundle. ## What is the new behavior? Two minimal additions, mirroring the integration shape of #45253. ### `apps/studio/components/interfaces/Linter/Linter.utils.tsx` Three new entries appended to `lintInfoMap`: - `pg_graphql_authenticated_table_exposed` — `Eye` icon (paired with the existing `pg_graphql_anon_table_exposed` entry); link points to the Table Editor scoped to `metadata.schema` + `metadata.name`; `linkText: 'View object'`; `category: 'security'`. - `anon_security_definer_function_executable` — `Unlock` icon (signals "this thing is callable when it shouldn't be"); link points to the Database Functions browser scoped to `metadata.schema` + `metadata.name`; `linkText: 'View function'`; `category: 'security'`. - `authenticated_security_definer_function_executable` — same as 0028 against `authenticated`. Each entry's `docsLink` points at the splinter-hosted lint doc. ### `packages/pg-meta/src/sql/studio/advisor/lints.ts` The existing `pg_graphql_anon_table_exposed` SQL block is updated in place to match the new splinter version: new `relkind` set, `case` statement for `'f'`, and the `EXISTS` over `pg_attribute` privilege check. Three new `union all` blocks are appended for 0027/0028/0029. The function lints (0028/0029) include the `pgrst.db_schemas` filter (mirroring lint `0023_sensitive_columns_exposed`) so findings are scoped to schemas PostgREST actually exposes; the self-hosted query wrapper already sets the GUC when `exposedSchemas` is passed (`enrichLintsQuery`). ## Coverage of the four exposure paths | Role | Tables/views/MVs/foreign tables | SECURITY DEFINER functions | |------|---------|----------| | `anon` | 0026 (existing, updated) | 0028 (new) | | `authenticated` | 0027 (new) | 0029 (new) | The 0026/0027 pair covers `pg_graphql` introspection visibility; the 0028/0029 pair covers RLS bypass via privileged function execution through `/rest/v1/rpc` (and `/graphql/v1` for compatible return types). Each lint's doc cross-references its sibling so an operator hitting one is steered toward the others. ## Verification - `cd packages/pg-meta && npx tsc --noEmit` — clean. - `cd apps/studio && npx tsc --noEmit` — clean for the changed file. (Other unrelated TS errors exist in the working tree but are pre-existing and not introduced by this PR.) - `cd apps/studio && npx eslint components/interfaces/Linter/Linter.utils.tsx` — clean. ## Files - `apps/studio/components/interfaces/Linter/Linter.utils.tsx` — adds three `lintInfoMap` entries (0027, 0028, 0029). - `packages/pg-meta/src/sql/studio/advisor/lints.ts` — updates the 0026 SQL block to match splinter's correctness fixes, appends 0027/0028/0029 SQL blocks. ## Related - supabase/splinter#160 — adds 0027/0028/0029 and rewrites 0026 (merged). - supabase/splinter#162 — fixes test setup for 0028/0029 (in flight; does not affect the SQL shipped here). - #45253 — original 0026 Studio integration. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added security linting to detect authenticated-table exposure and executable SECURITY DEFINER functions. * Added signed-in visibility checks alongside anonymous checks. * **Bug Fixes / Improvements** * Improved relation type handling for accurate table/foreign/partition classification. * Switched to column-level privilege analysis for visibility. * Improved entity naming shown in lints (includes function argument display). <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Danny White <3104761+dnywh@users.noreply.github.com>
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
PR #160 added a
pgrst.db_schemasfilter to lints0028and0029to scope function findings to schemas PostgREST actually exposes via/rest/v1/rpc. The test SQL files for those lints were not updated to set the GUC, so every positive case in the test stopped firing — the lint correctly returns 0 rows whenpgrst.db_schemasis unset, which makes "DEFINER function with EXECUTE to anon" look like a non-finding to the test harness.This PR sets
set local pgrst.db_schemas = 'public';at the top of both test files, mirroring the established pattern fromtest/sql/0023_sensitive_columns_exposed.sql(which uses the same filter for the same reason).Why this slipped past CI
#160's CI run actually reported
2 of 28 tests failedin its log, but the workflow turned green. The cause is indockerfiles/docker-compose.yml:bash -c "A; B || true"returns the exit code of the last command —cp ... || true, which is always 0 — so the container exits 0 even wheninstallcheckexits non-zero. The actions run conclusion is therefore success regardless of pg_regress results.Two tactical options for fixing that, neither in scope for this PR:
;-and-|| truechain with./bin/installcheck; rc=$?; cp ... 2>/dev/null || true; exit $rcso the cp still runs (for diagnostics) but the installcheck exit code is preserved.post_commandhook so the test container can fail cleanly.Worth a follow-up so future test breakage cannot hide behind a green check.
Verification
Confirmed locally on
supabase/postgres:15.1.1.13. Each positive case in0028and0029now produces the expected row counts:case_definer_default_grant: 1 row (SECURITY DEFINER function created with defaultPUBLICEXECUTE).case_overloads: 2 rows with distinctcache_keyvalues driven bypg_get_function_identity_arguments.Files
test/sql/0028_anon_security_definer_function_executable.sql— addsset local pgrst.db_schemas = 'public';test/sql/0029_authenticated_security_definer_function_executable.sql— same.test/expected/0028_anon_security_definer_function_executable.out— re-promoted to include the new echo line.test/expected/0029_authenticated_security_definer_function_executable.out— same.