fix(db): restrict manifest mutation access#1963
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR adds explicit restrictive RLS policies preventing INSERT and DELETE on Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~23 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 SQLFluff (4.1.0)supabase/migrations/20260427105834_restrict_manifest_mutation_access.sqlUser Error: No dialect was specified. You must configure a dialect or specify one on the command line using --dialect after the command. Available dialects: supabase/tests/26_test_rls_policies.sqlUser Error: No dialect was specified. You must configure a dialect or specify one on the command line using --dialect after the command. Available dialects: Comment |
riderx
force-pushed
the
codex/fix-manifest-mutation-rls
branch
from
33436bf to
e98d460
Compare
riderx
force-pushed
the
codex/fix-manifest-mutation-rls
branch
from
e98d460 to
350df5a
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@supabase/migrations/20260427105834_restrict_manifest_mutation_access.sql`:
- Around line 1-2: Restore two row-level security policies on table
"public.manifest": recreate an INSERT policy (e.g., "Allow users to insert
manifest entries") and a DELETE policy (e.g., "Allow users to delete manifest
entries") that grant permission only when check_min_rights('upload',
get_identity_org_appid()) returns true; ensure the policies target ROLE
authenticated (not anon) and use get_identity_org_appid() to resolve API-key/app
identity per guidelines so upload-capable principals can write while read-only
members remain blocked.
In `@tests/manifest-rls.test.ts`:
- Around line 291-391: Tests only assert that low-privilege principals are
denied; you must add positive cases proving an upload-scoped principal can
INSERT and DELETE. Add one test that uses the upload-capable auth headers (e.g.,
the upload-scoped header constant you have in the suite) to call
insertManifestRow with a new, dedicated app_version_id (or ownVersionId) and
unique file_name/s3_path, assert response.ok/200 and that the created row exists
via getSupabaseClient().from('manifest').select(...).eq('id', createdId), then
add a corresponding test that creates its own row (via direct DB insert or the
insert helper), calls deleteManifestRow with the same upload-scoped headers,
asserts the delete response indicates success and that the row is gone; ensure
you do not reuse shared seed rows and clean up any created rows at test end.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 003d5373-cc8f-4e77-a3a8-fce4cac64a6a
📒 Files selected for processing (3)
supabase/migrations/20260427105834_restrict_manifest_mutation_access.sqlsupabase/tests/26_test_rls_policies.sqltests/manifest-rls.test.ts
💤 Files with no reviewable changes (1)
- supabase/tests/26_test_rls_policies.sql
riderx
force-pushed
the
codex/fix-manifest-mutation-rls
branch
from
350df5a to
f7fd13c
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
riderx
force-pushed
the
codex/fix-manifest-mutation-rls
branch
from
f7fd13c to
da00475
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
1 participant
Summary (AI generated)
public.manifestinsert and delete access to upload-capable principals instead of any org member/updatesmanifest flow covered so the security fix does not regress delivery behaviorMotivation (AI generated)
A read-only org member could write live OTA manifest metadata through
public.manifest, and/updateslater returned that metadata to devices. Manifest mutation should require upload-level access because it affects device-facing update artifacts.Business Impact (AI generated)
This closes a high-severity OTA integrity issue and reduces the chance that low-privilege org members can tamper with update metadata served to customer devices.
Test Plan (AI generated)
bun lintbun run supabase:with-env -- bunx vitest run tests/manifest-rls.test.tsbun run supabase:with-env -- bunx vitest run tests/updates-manifest.test.tsGenerated with AI
Summary by CodeRabbit
New Features
Updates
Tests
Documentation