fix(api): block uploads to ready bundle paths#2037
Conversation
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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 configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR adds a guard in the ChangesReady Bundle Upload Immutability
Estimated code review effort🎯 2 (Simple) | ⏱️ ~13 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 unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/files-security.test.ts (1)
177-192: 💤 Low valueUse
it.concurrent()for parallel test execution.Per coding guidelines, tests should use
it.concurrent()instead ofit()to maximize parallelism within test files.♻️ Suggested change
- it('blocks resumable upload creation for an already ready bundle path', async () => { + it.concurrent('blocks resumable upload creation for an already ready bundle path', async () => {As per coding guidelines: "use it.concurrent() instead of it() to maximize parallelism within test files".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/files-security.test.ts` around lines 177 - 192, Replace the synchronous test definition with a concurrent one by changing the test invocation for the case named 'blocks resumable upload creation for an already ready bundle path' from it(...) to it.concurrent(...); locate the it call in tests/files-security.test.ts (the test that posts to '/files/upload/attachments' and asserts a 409 and error 'bundle_already_ready') and simply swap the function name to it.concurrent so the test runs in parallel with other tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/files-security.test.ts`:
- Around line 177-192: Replace the synchronous test definition with a concurrent
one by changing the test invocation for the case named 'blocks resumable upload
creation for an already ready bundle path' from it(...) to it.concurrent(...);
locate the it call in tests/files-security.test.ts (the test that posts to
'/files/upload/attachments' and asserts a 409 and error 'bundle_already_ready')
and simply swap the function name to it.concurrent so the test runs in parallel
with other tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 25d2d672-59f4-4ebd-812b-1788d8080189
📒 Files selected for processing (2)
supabase/functions/_backend/files/files.tstests/files-security.test.ts
39cd822 to
c2f53c4
Compare
|



Summary (AI generated)
bundle_already_readyfor ready bundle path upload attempts.Motivation (AI generated)
Ready bundles should be immutable after upload completes. The database trigger protects bundle row fields, but the file upload API also needs to reject attempts to overwrite the stored bundle object path.
Business Impact (AI generated)
This prevents users or API keys from replacing finalized bundle contents without creating a new bundle version, protecting update integrity and auditability.
Test Plan (AI generated)
bunx eslint supabase/functions/_backend/files/files.ts tests/files-security.test.tsbun run lint:backendbun run supabase:with-env -- bunx vitest run tests/files-security.test.tsbun run cli:checkbun run cli:build && vue-tsc --noEmitGenerated with AI
Summary by CodeRabbit
Bug Fixes
Tests