fix(rbac): scope forced-device policies to channels

[sellimenes]

Summary

• update channel_devices RLS to use request-aware channel-scoped forced-device permissions
• require channel.read_forced_devices for direct reads and channel.manage_forced_devices for direct writes
• sync supabase/schemas/prod.sql and add a pgTAP policy regression test

Test plan

• git diff --check
• GitHub Actions Run tests passed, including the Supabase pgTAP suite with supabase/tests/55_test_channel_devices_forced_device_rbac.sql
• GitHub Actions Run Playwright tests passed
• SonarCloud, dead-code, CodSpeed, benchmark, Socket, and CodeRabbit checks passed

I could not run the Supabase pgTAP suite locally because this environment does not have bun, supabase, or psql installed. The first CI run exposed a pgTAP compatibility issue in the new policy test; I fixed it and the latest CI run is green.

Summary by CodeRabbit

• Bug Fixes
  • Enhanced access control and security for channel device management operations with improved permission validation across the system.

[coderabbitai]

Warning

Rate limit exceeded

@sellimenes has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 6 minutes and 30 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c6be5f47-5e26-4868-9830-e69dbf347b65

📥 Commits

Reviewing files that changed from the base of the PR and between bad06de and 586f69f.

📒 Files selected for processing (4)

• supabase/migrations/20260511090000_channel_devices_forced_device_rbac.sql
• supabase/schemas/prod.sql
• supabase/tests/26_test_rls_policies.sql
• supabase/tests/55_test_channel_devices_forced_device_rbac.sql

📝 Walkthrough

Walkthrough

This PR migrates public.channel_devices RLS policies from legacy check_min_rights authorization to channel-scoped RBAC checks. A new migration defines policies using rbac_check_permission_request with permission functions for forced-device management and reads. The same policies are applied to the schema snapshot. A test validates policy correctness.

Changes

Channel Devices RLS Policy RBAC Authorization

Layer / File(s)	Summary
RLS Policy Migration supabase/migrations/20260511090000_channel_devices_forced_device_rbac.sql	New migration drops existing RLS policies for public.channel_devices and creates DELETE, INSERT, SELECT, and UPDATE policies using rbac_check_permission_request with rbac_perm_channel_manage_forced_devices (writes) and rbac_perm_channel_read_forced_devices (reads) scoped to (owner_org, app_id, channel_id).
Policy Documentation supabase/migrations/20260511090000_channel_devices_forced_device_rbac.sql	COMMENT ON POLICY statements document channel-scoped RBAC permission requirements for direct operations, including that UPDATE requires manage permission for both old and new target channels.
Schema File Synchronization supabase/schemas/prod.sql	Updates DELETE, INSERT, SELECT, and UPDATE policy expressions and comments in the schema snapshot to match the migration, replacing legacy check_min_rights calls with RBAC authorization checks scoped by (owner_org, app_id, channel_id).
RLS Policy Test Verification supabase/tests/55_test_channel_devices_forced_device_rbac.sql	TAP-style test validates five assertions: read policy uses channel.read_forced_devices with channel_id scoping; delete, insert, and both update clauses use channel.manage_forced_devices with channel_id scoping in their respective qual and with_check expressions.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

A rabbit hops through database rows,
With RLS policies in rightful flows,
From legacy checks to RBAC so bright,
Channel-scoped permissions: now read and write,
Forced devices managed with test-proven might! 🐰✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely summarizes the main change: implementing channel-scoped RBAC policies for forced-device access control, which aligns with all three modified files.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Description check	✅ Passed	The pull request description provides a clear summary, detailed test plan with CI validation, and explains local testing limitations.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

• Generate code and open pull requests
• Plan features and break down work
• Investigate incidents and troubleshoot customer tickets together
• Automate recurring tasks and respond to alerts with triggers
• Summarize progress and report instantly

Built for teams:

• Shared memory across your entire org—no repeating context
• Per-thread sandboxes to safely plan and execute work
• Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codspeed-hq]

Merging this PR will not alter performance

✅ 43 untouched benchmarks
⏩ 2 skipped benchmarks¹

---

_{Comparing sellimenes:harden-channel-devices-rbac (586f69f) with main (e85eca6)}

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

[tngus6007]

There is a policy-name mismatch that looks security-relevant here. The migration drops "Allow read for auth, api keys (read+)", but the schema snapshot shows the existing channel_devices SELECT policy is named "Allow read for auth (read+)".

If production has the schema-snapshot name, the old broad SELECT policy will not be dropped. PostgreSQL combines permissive RLS policies with OR semantics, so the new channel.read_forced_devices policy can be added while the old check_min_rights('read', get_identity_org_appid(...), ..., NULL::bigint) policy still grants app-scoped reads. That would leave the direct-read path broader than intended.

I would drop both legacy names in the migration and add a regression assertion that no channel_devices policy still references check_min_rights after the migration, e.g. checking pg_policies for tablename = 'channel_devices' and qual/with_check NOT LIKE '%check_min_rights%'.

[sellimenes]

Addressed in b424d9a.

• The migration now drops both legacy SELECT policy names: Allow read for auth, api keys (read+) and Allow read for auth (read+).
• The recreated SELECT policy now uses the schema-snapshot name Allow read for auth (read+), so migrated databases and fresh schema state align.
• Added a pgTAP regression assertion that no channel_devices RLS policy retains check_min_rights in qual or with_check.

git diff --check passes locally; waiting for the refreshed CI run.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[digzrow-coder]

I think this still needs a regression that the row's app_id / owner_org must match the submitted channel_id before the policy grants access.

The new channel_devices policies pass the row values into rbac_check_permission_request(..., owner_org, app_id, channel_id), but rbac_check_permission_direct currently resolves p_channel_id back to channels.owner_org/app_id and uses that as the effective permission scope. Since channel_devices only has independent foreign keys to apps(app_id) and channels(id), I don't see a constraint proving those row values describe the same channel.

That means a mismatched row can satisfy channel.manage_forced_devices for channel A while writing a channel_devices record whose app_id points at app B. auto_owner_org_by_app_id only normalizes owner_org from app_id; it doesn't prove the channel belongs to that app. I'd add either a composite FK/trigger enforcing channel_devices.(app_id, owner_org) matches channels.(app_id, owner_org), or an explicit helper/policy check plus a pgTAP regression for mismatched channel/app rows.

[KCDaemon]

Rechecked the latest head (586f69f) and I think this still needs one more authorization/data-integrity guard before merge. The new channel_devices policies pass the row's owner_org, app_id, and channel_id into rbac_check_permission_request(), but rbac_check_permission_direct() resolves p_channel_id back through public.channels and then uses the channel's own owner_org/app_id as the effective permission scope.

That means the permission check can succeed for channel A while the channel_devices row being inserted/updated still carries an independent app_id for app B. The table currently has separate FKs to apps(app_id) and channels(id), and the auto_owner_org_by_app_id trigger only normalizes owner_org from the row's app_id; it does not prove that the submitted channel belongs to that same app/org.

The added pgTAP test verifies the policy text uses channel_id and that old check_min_rights grants are gone, but it does not exercise a mismatched channel/app row. I would keep this blocked until either a composite FK/trigger enforces channel_devices.(app_id, owner_org) matches the referenced channel, or the RLS policy explicitly checks that relationship, with a regression covering the mismatch case. git diff --check origin/main...origin/pr-2193 passes locally.