fix(security): enforce encrypted bundle updates#1817
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a SQL migration that replaces the existing Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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.0.4)supabase/migrations/20260317100429_fix_encrypted_bundle_update_enforcement.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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/enforce-encrypted-bundles.test.ts (1)
48-86: Consider restructuring for concurrent test execution.The tests use
it()instead ofit.concurrent()because they sequentially toggleenforce_encrypted_bundleson the same org. While this is a valid reason, consider whether each test could use a separate test org or reset atomically within each test to enable concurrency. The current approach is functional but may slow down the test suite.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/enforce-encrypted-bundles.test.ts` around lines 48 - 86, Tests toggle the same org's enforce_encrypted_bundles setting sequentially (using it()) which prevents safe concurrency; update tests that call getSupabaseClient and use ORG_ID_ENCRYPTED so each test either (a) creates and uses an isolated org id via a helper (e.g., createTestOrg/deleteTestOrg) and then switch to it.concurrent() for the two specs, or (b) perform an atomic setup/teardown around each test with beforeEach/afterEach that sets enforce_encrypted_bundles to a known value so tests can remain independent; change the two specs ("should have enforce_encrypted_bundles = false by default" and "should allow updating enforce_encrypted_bundles setting") to use the chosen isolation helper or setup/teardown and then enable concurrency where safe.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/enforce-encrypted-bundles.test.ts`:
- Around line 48-86: Tests toggle the same org's enforce_encrypted_bundles
setting sequentially (using it()) which prevents safe concurrency; update tests
that call getSupabaseClient and use ORG_ID_ENCRYPTED so each test either (a)
creates and uses an isolated org id via a helper (e.g.,
createTestOrg/deleteTestOrg) and then switch to it.concurrent() for the two
specs, or (b) perform an atomic setup/teardown around each test with
beforeEach/afterEach that sets enforce_encrypted_bundles to a known value so
tests can remain independent; change the two specs ("should have
enforce_encrypted_bundles = false by default" and "should allow updating
enforce_encrypted_bundles setting") to use the chosen isolation helper or
setup/teardown and then enable concurrency where safe.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4b648ad6-ea1f-4f9a-94ef-a82b2989d08a
📒 Files selected for processing (2)
supabase/migrations/20260317100429_fix_encrypted_bundle_update_enforcement.sqltests/enforce-encrypted-bundles.test.ts
|



Summary (AI generated)
app_versionsupdates tosession_key,key_id,app_id, andowner_org.session_keyafter org encrypted-bundle enforcement is enabled.Motivation (AI generated)
An organization-level encrypted-bundle control was only enforced at insert time. Direct PostgREST updates could later downgrade an existing encrypted bundle by clearing
session_key, which bypassed the intended backend invariant.Business Impact (AI generated)
This closes a security-control bypass on OTA bundle metadata, preserving the integrity of encrypted-bundle enforcement for customers who enable it.
Test Plan (AI generated)
bun run supabase:startbun run supabase:functions:servebun run supabase:with-env -- bunx vitest run tests/enforce-encrypted-bundles.test.tsbun run lint:backend -- --fix=falsebun run test:backendGenerated with AI
Summary by CodeRabbit
Bug Fixes
Tests