Skip to content

fix(rbac): block preview public channels - #2713

Open
riderx wants to merge 2 commits into
mainfrom
codex/guard-app-preview-public-channel
Open

fix(rbac): block preview public channels#2713
riderx wants to merge 2 commits into
mainfrom
codex/guard-app-preview-public-channel

Conversation

@riderx

@riderx riderx commented Jul 17, 2026

Copy link
Copy Markdown
Member

Summary

  • Require app.update_settings to create a public/default channel.
  • Keep App Preview keys able to create private PR channels and atomically upload/promote their bundles.
  • Enforce the same boundary for direct CLI table writes and the raw-SQL create-and-promote path.

RLS execution model

  • Surface: public.channels INSERT WITH CHECK; it runs once per inserted channel row from the CLI/PostgREST path.
  • Existing app.create_channel remains required for every new channel.
  • A row with public = true additionally requires the existing, caller-scoped app.update_settings RBAC check on the row owner app. App Preview does not receive that permission.
  • The policy adds no tables, joins, or new helper functions. It uses the existing RBAC request helper with the insert row indexed owner_org and app_id values.
  • The endpoint atomic create-and-promote transaction uses a direct PostgreSQL client and therefore bypasses RLS; it performs the matching app.update_settings guard before inserting.

EXPLAIN and integration follow-up

  • Local 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).
  • Before this leaves draft, collect policy-plan evidence for App Preview non-public insert, App Preview public insert denial, and an app-admin public insert. CI must also pass the isolated lifecycle test.

Validation

  • bun lint
  • bun lint:backend
  • Focused ESLint for changed tests
  • bun run typecheck:backend
  • bunx vitest run tests/channel-post.unit.test.ts - 16 passed
  • bun test:unit - 160 files / 1,065 tests passed

Note

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_channel as before, and public: true on insert additionally requires app.update_settings (preview keys lack this). A migration tightens channels INSERT RLS the same way so CLI paths like channel add --default cannot 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

    • Public channel creation now requires both channel-creation and app-settings permissions.
    • Unauthorized public channel requests return clearer access errors with relevant app and channel details.
    • Preview channel creation now enforces restrictions against unauthorized or unsupported channel and bundle operations.
  • Tests

    • Added coverage confirming permission checks, error responses, and prevention of improperly created preview channels.

@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Public 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.

Changes

Public channel authorization

Layer / File(s) Summary
Handler permission checks
supabase/functions/_backend/public/channel/post.ts, tests/channel-post.unit.test.ts
Public channel creation checks app.update_settings after app.create_channel; the unit test verifies denial behavior and call order.
Database and preview lifecycle guards
supabase/migrations/.../20260717133500_app_preview_public_channel_guard.sql, tests/cli-preview-lifecycle.test.ts
The channels insert policy adds conditional RBAC checks, and lifecycle tests verify blocked default and public preview channel creation.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

Suggested labels: codex

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly matches the main change: blocking public preview channels with RBAC.
Description check ✅ Passed The description is detailed and aligned with the PR, though it omits the template's checklist and explicit test-plan section.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

Comment @coderabbitai help to get the list of available commands.

@codspeed-hq

codspeed-hq Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 43 untouched benchmarks
⏩ 2 skipped benchmarks1


Comparing codex/guard-app-preview-public-channel (ca093f8) with main (87cbc99)

Open in CodSpeed

Footnotes

  1. 2 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@riderx
riderx marked this pull request as ready for review July 27, 2026 11:27
@riderx
riderx deployed to deepsec-pr July 27, 2026 11:28 — with GitHub Actions Active
@cursor

cursor Bot commented Jul 27, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot 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)

@cursor

cursor Bot commented Jul 27, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot 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)

@coderabbitai coderabbitai Bot added the codex label Jul 27, 2026
@sonarqubecloud

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 with channel.update_settings but without app.update_settings can still flip existing private channels to public, creating a privacy/authorization regression — enforce the same app.update_settings guard 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 }))) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant