fix(rbac): block preview public channels - #2713
Conversation
📝 WalkthroughWalkthroughPublic channel creation now requires settings-update permission in addition to channel-creation permission. The database insert policy and unit and lifecycle tests enforce and verify these authorization guards. ChangesPublic channel authorization
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
Merging this PR will not alter performance
Comparing Footnotes
|
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_577ec601-13f5-4795-991d-aa88a224e618) |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_fa0907f3-988c-4aab-8f1d-9a7834fd8632) |
|
There was a problem hiding this comment.
1 issue found and verified against the latest diff
Confidence score: 2/5
- In
supabase/functions/_backend/public/channel/post.ts, permission checks are inconsistent between create and update paths, so users withchannel.update_settingsbut withoutapp.update_settingscan still flip existing private channels to public, creating a privacy/authorization regression — enforce the sameapp.update_settingsguard on update/publicization flows as on creation.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="supabase/functions/_backend/public/channel/post.ts">
<violation number="1" location="supabase/functions/_backend/public/channel/post.ts:351">
P1: Existing private channels can still be made public by identities with `channel.update_settings` but not `app.update_settings`, since this guard runs only for creation. Apply the same `app.update_settings` check whenever an incoming update sets `body.public === true` (or specifically when it transitions private to public).</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| } | ||
| // A public/default channel changes the app's delivery configuration. Preview | ||
| // keys may bootstrap private channels only, so they cannot make one public. | ||
| if (body.public === true && !(await checkPermission(c, 'app.update_settings', { appId: body.app_id }))) { |
There was a problem hiding this comment.
P1: Existing private channels can still be made public by identities with channel.update_settings but not app.update_settings, since this guard runs only for creation. Apply the same app.update_settings check whenever an incoming update sets body.public === true (or specifically when it transitions private to public).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At supabase/functions/_backend/public/channel/post.ts, line 351:
<comment>Existing private channels can still be made public by identities with `channel.update_settings` but not `app.update_settings`, since this guard runs only for creation. Apply the same `app.update_settings` check whenever an incoming update sets `body.public === true` (or specifically when it transitions private to public).</comment>
<file context>
@@ -342,8 +342,15 @@ export async function post(c: Context<MiddlewareKeyVariables>, body: ChannelSet,
+ }
+ // A public/default channel changes the app's delivery configuration. Preview
+ // keys may bootstrap private channels only, so they cannot make one public.
+ if (body.public === true && !(await checkPermission(c, 'app.update_settings', { appId: body.app_id }))) {
+ throw simpleError('cannot_access_app', 'You can\'t access this app', { app_id: body.app_id, channel: body.channel })
+ }
</file context>



Summary
app.update_settingsto create a public/default channel.RLS execution model
public.channelsINSERTWITH CHECK; it runs once per inserted channel row from the CLI/PostgREST path.app.create_channelremains required for every new channel.public = trueadditionally requires the existing, caller-scopedapp.update_settingsRBAC check on the row owner app. App Preview does not receive that permission.owner_organdapp_idvalues.app.update_settingsguard before inserting.EXPLAIN and integration follow-up
EXPLAIN (ANALYZE, BUFFERS)and the affected lifecycle integration test are pending: the local Supabase runtime cannot start because Docker/Dory is unavailable (Cannot connect to .../.dory/dory.sock).Validation
bun lintbun lint:backendbun run typecheck:backendbunx vitest run tests/channel-post.unit.test.ts- 16 passedbun test:unit- 160 files / 1,065 tests passedNote
Medium Risk
Touches RBAC and channel INSERT policy on a security-sensitive boundary; scope is narrow and aligned with existing permission helpers.
Overview
App Preview keys can still create private PR channels, but they can no longer mark a new channel as public/default, which would change app delivery settings.
On the channel POST handler, creating a channel now requires
app.create_channelas before, andpublic: trueon insert additionally requiresapp.update_settings(preview keys lack this). A migration tightenschannelsINSERT RLS the same way so CLI paths likechannel add --defaultcannot bypass the API.Tests cover the POST permission check and preview lifecycle: default-channel CLI add and public channel POST are rejected with no row created; private preview upload/promote behavior stays intact.
Reviewed by Cursor Bugbot for commit ca093f8. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
Bug Fixes
Tests