[pull] master from supabase:master - #1176
Merged
Merged
Conversation
## 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? Database migration — adds a table for collecting free-form product feedback submitted from Supabase interfaces (starting with the CLI and the MCP server), including support for deleting a submission via a server-issued token. ## What is the current behavior? There is no destination for feedback submitted from the CLI or MCP server. The existing `feedback` and `feedback_comments` tables are scoped to the docs feedback widget, so interface feedback would otherwise end up as ad-hoc GitHub issues — with no way to revoke something submitted by accident (e.g. a secret key pasted into the message). ## What is the new behavior? Adds `public.interfaces_feedback`: | Column | Type | Notes | | --- | --- | --- | | `id` | `bigint` identity | primary key (not exposed through the API) | | `created_at` | `timestamptz` | `not null default now()` | | `feedback` | `text` | `not null`, ≤ 1000 chars — the free-form feedback | | `delete_token` | `uuid` | server-generated, `unique not null`; authorizes deleting the row | | `user_agent` | `text` | ≤ 255 chars; interface + version, also identifies the source interface | | `user_id` | `text` | optional, ≤ 255 chars; unverified, interface-defined identifier | | `project_ref` | `text` | optional, ≤ 255 chars | | `metadata` | `jsonb` | ≤ 8 KB catch-all | **Submission** happens exclusively through a `SECURITY DEFINER` function, `submit_interfaces_feedback(...)`, which inserts the row and returns the server-generated `delete_token` exactly once. There is no insert grant or policy on the table itself, so clients cannot insert directly or supply their own token — the function is the only door. Execute is revoked from `PUBLIC` and granted to `anon` only (both statements matter: local and hosted databases have different default function ACLs). **Deletion** is a hard `DELETE` authorized by presenting the token in an `x-feedback-token` request header. RLS policies compare the row's `delete_token` against that header (`current_setting('request.headers', ...)`) — the URL filter is never the security boundary; a request without the matching header affects zero rows, even with no filter or someone else's token in the filter. Tokens never expire (the delete right shouldn't lapse). The header is cast to `uuid` and compared against the untransformed column, so lookups use the unique index on `delete_token` even for header-only reads; a malformed token header is rejected with a `400` (`22P02`), consistent with what a malformed URL filter value already returns. **Context gate (defense-in-depth)**: rows submitted with a `project_ref` and/or `user_id` additionally require the matching `x-feedback-project-ref` / `x-feedback-user-id` headers — on both reads and deletes — so a leaked bare token can neither read the submission text back nor remove the row. A `NULL` column imposes no requirement: context-free rows keep token-only behavior, and extra headers sent against them are ignored (this keeps clients that always send their current context from being locked out of rows submitted without it). These are client-supplied, unverified values, so the gate is a knowledge factor rather than an identity check; clients should persist `{delete_token, project_ref, user_id}` together at submit time and re-present them byte-exact (`project_ref`/`user_id` are compared as plain text). **Reads** are limited to `grant select (feedback, delete_token)` behind the same token-scoped policy: a token-holder can preview their own submission text before deleting and confirm the delete matched (`Prefer: count=exact` → `Content-Range: */1` vs `*/0`). No other columns are readable by any API role; `delete_token` needs select because PostgREST requires a WHERE clause on deletes and filter columns require select privilege. Verified locally via `supabase db reset` + the local REST API: token issuance, token-scoped preview and delete, zero-row results for missing/wrong/malformed tokens (including a victim's token in the filter without the header), the full context-gate matrix (project+user, project-only, and context-free rows, incl. lenient extra-header behavior), denied direct inserts and column reads, length caps enforced through the function, and no execute for `authenticated`. ## Additional context Linear tickets: [CLI-1946](https://linear.app/supabase/issue/CLI-1946), [CLI-1999](https://linear.app/supabase/issue/CLI-1999) The client-side flows (`supabase feedback add` / `feedback delete` in the CLI, and the MCP tool) land separately in their respective repos and will call the RPC / DELETE endpoint described above. Supersedes #48378 — recreated on a fresh git branch so that the Supabase preview branch used for testing this table isn't shared with unrelated work. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added support for collecting and storing feedback submitted through interfaces. * Feedback can include submission source, timestamps, user details, project references, and additional metadata. * Added secure feedback submission with controlled access to protect submitted information. * Added support for authorized feedback removal using a secure deletion token. * Added safeguards to validate feedback content and restrict access to permitted information. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Stacked on #49015, which is stacked on #49011. Review those first. ## Problem The Row Level Security guide spent 225 lines and 5 benchmark tables on performance, 29% of the page. The `RLS Performance and Best Practices` troubleshooting entry already covers the same six tips with the same numbers, from the same source. Neither page tells you how to check whether RLS is your bottleneck in the first place. Four of the six tips are not tuning advice. Indexes, `select`-wrapping, role scoping, and `security definer` safety change whether a policy is correct and safe, not just fast. ## Solution - Add `guides/database/postgres/row-level-security-performance`. It carries the client-filter rule, the join-rewrite rule, all 5 benchmark tables merged into one, and a new `Diagnose whether RLS is the bottleneck` section: toggle RLS off to confirm it's the cost, then read the plan under an impersonated role. That diagnostic exists in the troubleshooting entry and has never been in the guide. - Keep every rule that affects correctness on the RLS guide, grouped under `Write policies that scale`. These are also the four the `build-docs-002-rls-guide` eval grades, and an agent reads the guide top-down. - Repoint the Grafana IO troubleshooting entry at the new page. - Rewrite `More resources` as `Related content`. Every link now says what it is and when to use it. Adds `Advanced pgTAP testing`, the deepest RLS testing content in the docs, which nothing here linked. Drops discussion 14576: locked, mislabeled here as "RLS Guide and Best Practices" when it is "RLS **Performance** and Best Practices", and superseded by the troubleshooting entry and this new page. **Ownership rule** so the two pages don't drift: the RLS guide owns the rule and the correct form. The performance page owns the measurement and the optimizer explanation. If a sentence on the performance page tells you what to write, it belongs on the guide. Scoped out of this PR: `More resources` was assigned to the restructure PR in the plan, but the 14576 link is what this PR supersedes, so leaving it would ship a stale pointer. ## Manual testing 1. Open the [RLS performance guide](https://docs-git-docs-rls-performance-split-supabase.vercel.app/docs/guides/database/postgres/row-level-security-performance) on the preview. It appears in the left nav under Database, Access and security, directly below Row Level Security. 2. Select the three rule links in its intro. Each lands on the matching section of the RLS guide. 3. Open the [Row Level Security guide](https://docs-git-docs-rls-performance-split-supabase.vercel.app/docs/guides/database/postgres/row-level-security) and go to `Write policies that scale`. It holds indexes, `select`-wrapping, and role scoping, with one link out to the performance page. 4. Open the [Grafana IO troubleshooting entry](https://docs-git-docs-rls-performance-split-supabase.vercel.app/docs/guides/troubleshooting/interpreting-supabase-grafana-io-charts-MUynDR) and select the RLS performance guide link. It lands on the new page. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added a dedicated guide for diagnosing and improving PostgreSQL Row Level Security performance. * Expanded guidance on indexing, query filters, role targeting, function usage, and avoiding costly policy joins. * Updated the Row Level Security guide with streamlined, scalable policy recommendations and links to related resources. * Added the new performance guide to the Database documentation navigation. * Updated troubleshooting guidance to reference the dedicated performance guide. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…ize and apps pages (#49267) <!-- ccr-slack-attribution --> _Requested by **Ali Waseem** · [Slack thread](https://supabase.slack.com/archives/C063LNYJJKS/p1787146439389169)_ ## 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? Bug fix. ## What is the current behavior? Opening `/authorize` for an OAuth app whose `name` the platform API omitted crashed the entire page with `TypeError: Cannot read properties of undefined (reading 'toLowerCase')` ([SUPABASE-APP-K7E](https://supabase.sentry.io/issues/7679644991/)). The user got a full-page error instead of a consent screen, and could neither authorize nor decline. The same class of crash hit the project-level OAuth apps list ([SUPABASE-APP-JB1](https://supabase.sentry.io/issues/7502074939/)). Typing in the search box called `.toLowerCase()` on `client_name` for every app, so one app registered without a name broke search for the whole list. The project-claim page crashed the same way, reading the first character of the name for the fallback avatar. ## What is the new behavior? The trusted-partner helpers treat a missing name as "no trusted partner matched" and return `null`. The apps filter treats a missing name or client ID as "does not match the search string". The claim page falls back to a placeholder initial instead of indexing into `undefined`. The authorize page now renders normally, minus the optional partner-impersonation caution, which cannot be evaluated without a name. Three changes: - `apps/studio/components/interfaces/Organization/OAuthApps/OAuthApps.utils.ts` — `findTrustedPartnerByName` accepts `string | null | undefined` and returns `null` early on a falsy name; `getOAuthImpersonationWarning`'s `name` param widened to match (its existing `if (!namedPartner) return null` already handles the rest). - `apps/studio/components/interfaces/Auth/OAuthApps/oauthApps.utils.ts` — `filterOAuthApps` optional-chains `client_name` and `client_id` before `.toLowerCase()`, defaulting each match to `false`. - `apps/studio/components/interfaces/Organization/ProjectClaim/confirm.tsx` — `{requester.name?.[0] ?? '?'}` for the fallback avatar initial. Each is a separate commit so any one can be dropped independently. ## Additional context ### Root cause, not fixed here `apps/studio/data/api-authorization/api-authorization-query.ts:37` returns `data as ApiAuthorizationResponse`, an unchecked cast with no runtime validation, even though the openapi-fetch client already types the endpoint from the generated schema. Both the generated `GetOAuthAuthorizationResponse` and the hand-written local type declare `name: string` as required, so this was invisible to TypeScript. The durable fix is to derive the type from the schema and drop the cast, which is the house pattern elsewhere in `apps/studio/data`, and to correct the OpenAPI spec at source if the API can legitimately omit `name`. Left out deliberately to keep this cherry-pickable. ### Not in scope `requester.scopes` is optional in the schema but required in the local type, and is read unguarded in several places. Defaulting it to `[]` would tell a user an app requested no permissions on a live consent screen, so it needs a product decision rather than a drive-by guard. ### Testing No local checks were run. This clone has no `node_modules` and `pnpm install` is blocked in the environment, so `npm run build`, typecheck, lint, Prettier and tests were all left to CI. Please treat CI as the verification for this PR. There is also a coverage gap worth noting: `apps/studio/tests/components/ApiAuthorization.test.tsx:48-62` hardcodes `name: 'Test App'` in `createMockAuthResponse`, and no test omits the field, which is why none of these crashes were caught. --- _Generated by [Claude Code](https://claude.ai/code/session_01P489vrPdHcJfMfzCGM9rZ5)_ --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Ali Waseem <waseema393@gmail.com>
## Problem The guide alternated between context, procedure, and reference on almost every heading. A reader who wanted to write a policy passed through four context or reference sections to reach one. A reader who wanted the model had to skip three procedures. ## Solution - Group into three sections by information type: `Understand Row Level Security`, `Secure a table with RLS`, and `RLS reference`, with a navigation intro. - Merge the four policy sections. They repeated the same setup block, burying the clause that differed. One setup block now precedes four short policy examples. - Move the auto-enable recipe into `event-triggers.mdx`, whose stub section's entire body was a link back here. - Relocate the stranded `auth.uid()` caution into the `auth.uid()` reference. - Lift the revoke-and-grant procedure out of the danger admonition and merge it with the two other places that taught `enable row level security`. - Point the Grafana IO chart entry at the performance guide. Its `#rls-performance-recommendations` anchor went away when tuning split out in #49016. 765 lines to 582. 30 headings to 25. Headings are demoted rather than renamed wherever anything links to them. Every inbound anchor in the repo still resolves; the only one removed, `#auto-enable-rls-for-new-tables`, was referenced solely by the `event-triggers.mdx` stub this PR replaces. ## Note on the history Rebuilt from `master` after #49011, #49015, and #49016 merged. The branch previously carried those 10 commits plus rebase churn against them. Rebasing naively would have reverted review feedback from #49016 (`70fa812`), which removed the benchmarks table and the "This guide" opener from the performance guide. Those are deliberately not restored here. The only changes to that file are two missing `await`s and a join predicate that was a tautology while unqualified. The three PRs stacked on this one (#49268, #49269, #49270) have been rebased onto the new base. ## Manual testing 1. Open the [Row Level Security guide](https://docs-git-docs-rls-restructure-supabase.vercel.app/docs/guides/database/postgres/row-level-security) on the preview. Three top-level sections appear in the table of contents. 2. Select each link in the intro. All three jump to their section. 3. Open [Event triggers](https://docs-git-docs-rls-restructure-supabase.vercel.app/docs/guides/database/postgres/event-triggers). The auto-enable section holds the full recipe instead of a link. 4. Open the [performance guide](https://docs-git-docs-rls-restructure-supabase.vercel.app/docs/guides/database/postgres/row-level-security-performance). No benchmarks table, and the three bullets at the top link into the RLS guide. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Reworked the Row Level Security guide with clearer guidance on grants, policies, permissions, performance, testing, views, and secure functions. * Added a complete example for automatically enabling RLS on newly created public tables. * Improved SQL examples and clarified table references in RLS performance guidance. * Corrected grammar in the Grafana chart troubleshooting documentation. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
github-actions
Bot
added
the
documentation
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )