database: Complete stabilization migration - 7 audit fixes - #22
Merged
Conversation
This migration applies all critical schema fixes in a single transaction: 1. Auth linkage (users.id references auth.users) 2. Cascading deletes (ON DELETE CASCADE/SET NULL) 3. Missing columns (model_attempted, validation_report, validation_passed) 4. Timezone safety (timestamptz for all timestamps) 5. Concurrent safety (UNIQUE constraint on user_video) 6. Quota underflow prevention (CHECK constraint) 7. Performance indexes (cache lookup, vector, foreign keys) Additionally creates two RPCs: - increment_user_quota: Atomic quota enforcement - reset_user_quota: Monthly quota reset helper Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Reviewer's GuideSingle stabilization migration that hardens core public tables (users, analyses, usage_logs, stripe_events), normalizes timestamps, enforces auth linkages and constraints, adds performance indexes, and introduces two quota-related RPC functions. Sequence diagram for quota enforcement via increment_user_quota RPCsequenceDiagram
actor User
participant ApiServer
participant Supabase
User->>ApiServer: POST /api/analyses
ApiServer->>Supabase: RPC increment_user_quota(p_user_id, p_increment)
Supabase-->>ApiServer: new_quota, tier
ApiServer-->>User: 200 OK with analysis result
Entity relationship diagram for hardened core tables and constraintserDiagram
users {
uuid id PK
integer analyses_used
timestamptz last_reset_date
text tier
}
analyses {
uuid id PK
uuid user_id FK
text video_id
text model_attempted
text model_used
boolean validation_passed
vector embedding
varchar shared_token
timestamptz created_at
}
usage_logs {
uuid id PK
uuid user_id FK
text action
integer tokens_used
numeric cost_usd
timestamptz created_at
}
stripe_events {
uuid id PK
uuid user_id FK
text event_type
integer amount_cents
text status
timestamptz created_at
}
users ||--o{ analyses : user_id_cascade
users ||--o{ usage_logs : user_id_cascade
users ||--o{ stripe_events : user_id_set_null
analyses }o--o{ analyses : unique_user_video
users ||--|| users : check_analyses_used_non_negative
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
WalkthroughThis PR hardens the database schema and enforces RLS, refactors the analyses API to run background work with next.after(), tightens QStash/webhook and SSRF checks, adds Edge-safe middleware and multi-runtime Sentry init, improves Supabase client auth handling, adds embedding/Redis resiliency, tunes OpenRouter token budgeting, and updates CI/CD pnpm/Vercel tooling and docs. ChangesDatabase Schema Stabilization and RLS Security
Web Application Hardening and Instrumentation
Resiliency, Services, and Tests
CI/CD Workflows and Tooling
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
🚥 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)
✨ Simplify code
Warning Review ran into problems🔥 ProblemsStopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a 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. Comment |
|
Updates to Preview Branch (database/stabilization) ↗︎
Tasks are run on every commit but only new migration files are pushed.
❌ Branch Error • Wed, 20 May 2026 21:18:14 UTC View logs for this Workflow Run ↗︎. |
|
❌ Linting failed. Please fix linting errors before merging. |
|
❌ Pipeline Status: All checks passed. Ready to merge. |
![sourcery-ai[bot]](https://avatars.github.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 4 issues, and left some high level feedback:
- The
ALTER ... TYPE timestamp with time zone USING ... AT TIME ZONE 'UTC'conversions (e.g. forcreated_at,updated_at,published_at) can silently change existing data semantics and, in the case ofpublished_at, fill nulls withNOW(); consider guarding these with explicit checks or a two-step migration that preserves existing nulls and respects whether the columns are alreadytimestamptz. - Both
increment_user_quotaandreset_user_quotaassume that ausersrow exists forp_user_idand report success even if theUPDATEaffects zero rows; it would be safer to checkFOUND(or useRETURNINGand handle no-row cases) and either raise an error or return a clear failure indicator when the user does not exist. - The
CREATE EXTENSION IF NOT EXISTS vectorinside theDOblock couples extension management with table migration and may fail in restricted environments; consider moving extension creation to a separate top-level migration statement so that superuser/owner privileges and failure modes are easier to control.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `ALTER ... TYPE timestamp with time zone USING ... AT TIME ZONE 'UTC'` conversions (e.g. for `created_at`, `updated_at`, `published_at`) can silently change existing data semantics and, in the case of `published_at`, fill nulls with `NOW()`; consider guarding these with explicit checks or a two-step migration that preserves existing nulls and respects whether the columns are already `timestamptz`.
- Both `increment_user_quota` and `reset_user_quota` assume that a `users` row exists for `p_user_id` and report success even if the `UPDATE` affects zero rows; it would be safer to check `FOUND` (or use `RETURNING` and handle no-row cases) and either raise an error or return a clear failure indicator when the user does not exist.
- The `CREATE EXTENSION IF NOT EXISTS vector` inside the `DO` block couples extension management with table migration and may fail in restricted environments; consider moving extension creation to a separate top-level migration statement so that superuser/owner privileges and failure modes are easier to control.
## Individual Comments
### Comment 1
<location path="supabase/migrations/20260519_complete_stabilization.sql" line_range="48-57" />
<code_context>
+
+ -- Add vector column for semantic search (future embedding support)
+ CREATE EXTENSION IF NOT EXISTS vector;
+ ALTER TABLE public.analyses
+ ADD COLUMN IF NOT EXISTS embedding vector(1536);
+
+ -- Add shared token support for read-only sharing
+ ALTER TABLE public.analyses
+ ADD COLUMN IF NOT EXISTS shared_token character varying UNIQUE,
+ ADD COLUMN IF NOT EXISTS shared_expires_at timestamp with time zone;
+
+ -- Convert all timestamps to timestamptz
+ ALTER TABLE public.analyses
+ ALTER COLUMN created_at TYPE timestamp with time zone USING created_at AT TIME ZONE 'UTC',
+ ALTER COLUMN updated_at TYPE timestamp with time zone USING updated_at AT TIME ZONE 'UTC',
+ ALTER COLUMN published_at TYPE timestamp with time zone USING COALESCE(published_at, NOW()) AT TIME ZONE 'UTC';
+
+ -- Drop old foreign key if it exists without cascading delete
</code_context>
<issue_to_address>
**issue (bug_risk):** Converting `published_at` while coercing NULLs to `NOW()` changes semantics for existing rows.
`COALESCE(published_at, NOW())` will retroactively assign a timestamp to all rows where `published_at` is currently NULL, erasing the distinction between NULL and “published”. If NULL is meaningful, keep it as NULL in the migration, e.g. `USING CASE WHEN published_at IS NULL THEN NULL ELSE published_at AT TIME ZONE 'UTC' END` (adjusted for the current column type as needed).
</issue_to_address>
### Comment 2
<location path="supabase/migrations/20260519_complete_stabilization.sql" line_range="23-32" />
<code_context>
+ IF EXISTS (SELECT 1 FROM information_schema.tables WHERE table_name = 'users' AND table_schema = 'public') THEN
+
+ -- Add missing columns to users table
+ ALTER TABLE public.users
+ ADD COLUMN IF NOT EXISTS name text,
+ ADD COLUMN IF NOT EXISTS avatar_url text,
+ ADD COLUMN IF NOT EXISTS stripe_customer_id text UNIQUE,
+ ADD COLUMN IF NOT EXISTS stripe_subscription_id text;
+
+ -- Convert timestamps to timestamptz for timezone safety
+ ALTER TABLE public.users
+ ALTER COLUMN created_at TYPE timestamp with time zone USING created_at AT TIME ZONE 'UTC',
+ ALTER COLUMN updated_at TYPE timestamp with time zone USING updated_at AT TIME ZONE 'UTC',
+ ALTER COLUMN last_reset_date TYPE timestamp with time zone USING last_reset_date AT TIME ZONE 'UTC';
+
+ -- Add quota underflow prevention check constraint
</code_context>
<issue_to_address>
**issue (bug_risk):** Blindly applying `AT TIME ZONE 'UTC'` assumes current columns are `timestamp without time zone`.
If any of these columns (`created_at`, `updated_at`, `last_reset_date`) are already `timestamptz` in some environments, `... USING col AT TIME ZONE 'UTC'` will incorrectly shift stored times. Consider either conditioning the migration on the existing data type (e.g. only when `data_type = 'timestamp without time zone'`) or using a `USING` expression that behaves correctly for both `timestamp` and `timestamptz`.
</issue_to_address>
### Comment 3
<location path="supabase/migrations/20260519_complete_stabilization.sql" line_range="169-170" />
<code_context>
+ CREATE INDEX IF NOT EXISTS idx_analyses_user_id
+ ON public.analyses(user_id);
+
+ CREATE INDEX IF NOT EXISTS idx_usage_logs_user_id
+ ON public.usage_logs(user_id);
+
+ CREATE INDEX IF NOT EXISTS idx_stripe_events_user_id
</code_context>
<issue_to_address>
**issue:** Index creation for `usage_logs` and `stripe_events` is conditioned only on `analyses` existing.
The outer `IF EXISTS` only checks for `public.analyses`, but this block also creates indexes on `usage_logs` and `stripe_events`. In an environment where `analyses` exists but one of those tables does not, the migration will fail. Please add per-table existence checks or move each index creation into its respective table-specific section above.
</issue_to_address>
### Comment 4
<location path="supabase/migrations/20260519_complete_stabilization.sql" line_range="219-228" />
<code_context>
+ -- FIX #7: CREATE reset_user_quota RPC (Monthly reset helper)
+ -- ============================================================================
+
+ CREATE OR REPLACE FUNCTION public.reset_user_quota(p_user_id uuid)
+ RETURNS TABLE (
+ reset_success boolean,
+ new_quota integer,
+ reset_date timestamp with time zone
+ )
+ LANGUAGE plpgsql
+ SECURITY DEFINER
+ SET search_path = public
+ AS $$
+ DECLARE
+ v_now timestamp with time zone;
+ BEGIN
+ v_now := CURRENT_TIMESTAMP AT TIME ZONE 'UTC';
+
+ UPDATE public.users
</code_context>
<issue_to_address>
**nitpick (bug_risk):** `CURRENT_TIMESTAMP AT TIME ZONE 'UTC'` introduces a redundant cast and can subtly shift values.
`CURRENT_TIMESTAMP` is already `timestamptz`, so `AT TIME ZONE 'UTC'` converts it to `timestamp` and then it’s implicitly cast back to `timestamptz` for `v_now`, which can be misleading. Use plain `CURRENT_TIMESTAMP`/`now()` for a UTC `timestamptz`, or only use `AT TIME ZONE 'UTC'` when storing a `timestamp without time zone`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+23
to
+32
| ALTER TABLE public.users | ||
| ADD COLUMN IF NOT EXISTS name text, | ||
| ADD COLUMN IF NOT EXISTS avatar_url text, | ||
| ADD COLUMN IF NOT EXISTS stripe_customer_id text UNIQUE, | ||
| ADD COLUMN IF NOT EXISTS stripe_subscription_id text; | ||
|
|
||
| -- Convert timestamps to timestamptz for timezone safety | ||
| ALTER TABLE public.users | ||
| ALTER COLUMN created_at TYPE timestamp with time zone USING created_at AT TIME ZONE 'UTC', | ||
| ALTER COLUMN updated_at TYPE timestamp with time zone USING updated_at AT TIME ZONE 'UTC', |
There was a problem hiding this comment.
issue (bug_risk): Blindly applying AT TIME ZONE 'UTC' assumes current columns are timestamp without time zone.
If any of these columns (created_at, updated_at, last_reset_date) are already timestamptz in some environments, ... USING col AT TIME ZONE 'UTC' will incorrectly shift stored times. Consider either conditioning the migration on the existing data type (e.g. only when data_type = 'timestamp without time zone') or using a USING expression that behaves correctly for both timestamp and timestamptz.
Comment on lines
+169
to
+170
| CREATE INDEX IF NOT EXISTS idx_usage_logs_user_id | ||
| ON public.usage_logs(user_id); |
There was a problem hiding this comment.
issue: Index creation for usage_logs and stripe_events is conditioned only on analyses existing.
The outer IF EXISTS only checks for public.analyses, but this block also creates indexes on usage_logs and stripe_events. In an environment where analyses exists but one of those tables does not, the migration will fail. Please add per-table existence checks or move each index creation into its respective table-specific section above.
Comment on lines
+219
to
+228
| CREATE OR REPLACE FUNCTION public.reset_user_quota(p_user_id uuid) | ||
| RETURNS TABLE ( | ||
| reset_success boolean, | ||
| new_quota integer, | ||
| reset_date timestamp with time zone | ||
| ) | ||
| LANGUAGE plpgsql | ||
| SECURITY DEFINER | ||
| SET search_path = public | ||
| AS $$ |
There was a problem hiding this comment.
nitpick (bug_risk): CURRENT_TIMESTAMP AT TIME ZONE 'UTC' introduces a redundant cast and can subtly shift values.
CURRENT_TIMESTAMP is already timestamptz, so AT TIME ZONE 'UTC' converts it to timestamp and then it’s implicitly cast back to timestamptz for v_now, which can be misleading. Use plain CURRENT_TIMESTAMP/now() for a UTC timestamptz, or only use AT TIME ZONE 'UTC' when storing a timestamp without time zone.
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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.
Inline comments:
In `@supabase/migrations/20260519_complete_stabilization.sql`:
- Around line 54-55: The migration adds model_used to table public.analyses with
a DEFAULT of 'anthropic/claude-3.5-haiku' but then immediately changes the
default to 'anthropic/claude-haiku-4.5'; update the migration so the initial ADD
COLUMN for model_used does not set a conflicting DEFAULT (remove the DEFAULT
clause from the ALTER TABLE ... ADD COLUMN IF NOT EXISTS model_used VARCHAR(255)
statement) or else set the ADD COLUMN default to the intended final value
('anthropic/claude-haiku-4.5') so only one default is applied; ensure the later
ALTER COLUMN/default change for model_used is consistent with this choice.
- Around line 82-84: The migration adds a UNIQUE constraint with ALTER TABLE ...
ADD CONSTRAINT unique_user_video which will fail if that constraint already
exists; wrap the ADD CONSTRAINT in a guard that checks pg_constraint (e.g.
SELECT 1 FROM pg_constraint WHERE conname = 'unique_user_video') and only runs
the ALTER TABLE when it does not exist, so the migration becomes idempotent
while still creating the unique_user_video constraint on public.analyses when
absent.
- Around line 35-37: The migration adds a non-idempotent constraint
(check_analyses_used_non_negative on table public.users) which will fail if
re-run; update the migration to guard the ALTER TABLE ... ADD CONSTRAINT by
first checking existence (e.g., query pg_constraint/pg_class/pg_namespace for
conname = 'check_analyses_used_non_negative') and only run ADD CONSTRAINT when
it does not exist, or alternatively run ALTER TABLE public.users DROP CONSTRAINT
IF EXISTS check_analyses_used_non_negative; then run ADD CONSTRAINT
check_analyses_used_non_negative CHECK (analyses_used >= 0) so the migration
becomes idempotent.
- Line 71: The ALTER statement currently uses COALESCE(published_at, NOW())
which turns intentional NULLs into the current timestamp; update the USING
expression for ALTER COLUMN published_at to preserve NULLs instead of
substituting NOW(). Replace COALESCE(published_at, NOW()) AT TIME ZONE 'UTC'
with an expression that returns NULL when published_at IS NULL (e.g., USING
(CASE WHEN published_at IS NULL THEN NULL ELSE published_at AT TIME ZONE 'UTC'
END) or simply USING (published_at AT TIME ZONE 'UTC') ) so unpublished rows
remain NULL.
- Around line 159-173: The index creation for idx_usage_logs_user_id and
idx_stripe_events_user_id is incorrectly placed inside the IF block that only
checks for the analyses table; remove those two CREATE INDEX statements from the
analyses block and add them into their respective table-existence blocks: add
CREATE INDEX IF NOT EXISTS idx_usage_logs_user_id ON public.usage_logs(user_id)
inside the usage_logs IF block, and CREATE INDEX IF NOT EXISTS
idx_stripe_events_user_id ON public.stripe_events(user_id) inside the
stripe_events IF block, ensuring each index is only created when its table
exists.
- Around line 234-244: The UPDATE against public.users using p_user_id can
affect zero rows but the function always returns true; modify the logic after
the UPDATE to check whether any rows were updated (use FOUND or GET DIAGNOSTICS
ROW_COUNT) and return a failure result when none were affected. Concretely,
after the UPDATE on public.users (and before the RETURN QUERY that currently
emits reset_success = true), test IF NOT FOUND (or if row_count = 0) then RETURN
QUERY SELECT false AS reset_success, NULL::integer AS new_quota, v_now AS
reset_date (or appropriate values), else return the existing success tuple;
reference p_user_id, the UPDATE on public.users, and the current RETURN QUERY
when implementing the conditional.
- Around line 203-210: The UPDATE currently does nothing if p_user_id doesn't
match any row and the function silently returns no rows; modify the function
containing the UPDATE (the block using p_user_id, p_increment, v_new_quota,
v_tier) to detect when no row was updated by checking SQL%ROWCOUNT (or
equivalent) immediately after the UPDATE and, if it is 0, raise an exception
(RAISE EXCEPTION 'user not found: %', p_user_id) or otherwise return a clearly
distinguishable result instead of returning an empty result set; ensure the
subsequent RETURN QUERY SELECT v_new_quota, v_tier only runs when a row was
updated.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f736af3a-e021-4903-b49f-3a40d97a8897
📒 Files selected for processing (1)
supabase/migrations/20260519_complete_stabilization.sql
Comment on lines
+35
to
+37
| -- Add quota underflow prevention check constraint | ||
| ALTER TABLE public.users | ||
| ADD CONSTRAINT check_analyses_used_non_negative CHECK (analyses_used >= 0); |
Contributor
There was a problem hiding this comment.
Non-idempotent constraint addition will fail on re-run.
ADD CONSTRAINT does not support IF NOT EXISTS in PostgreSQL. If this migration is re-applied or the constraint already exists, it will error. Wrap in an existence check or drop first.
Proposed fix: Guard with existence check
-- Add quota underflow prevention check constraint
- ALTER TABLE public.users
- ADD CONSTRAINT check_analyses_used_non_negative CHECK (analyses_used >= 0);
+ IF NOT EXISTS (
+ SELECT 1 FROM information_schema.table_constraints
+ WHERE table_schema = 'public' AND table_name = 'users'
+ AND constraint_name = 'check_analyses_used_non_negative'
+ ) THEN
+ ALTER TABLE public.users
+ ADD CONSTRAINT check_analyses_used_non_negative CHECK (analyses_used >= 0);
+ END IF;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| -- Add quota underflow prevention check constraint | |
| ALTER TABLE public.users | |
| ADD CONSTRAINT check_analyses_used_non_negative CHECK (analyses_used >= 0); | |
| -- Add quota underflow prevention check constraint | |
| IF NOT EXISTS ( | |
| SELECT 1 FROM information_schema.table_constraints | |
| WHERE table_schema = 'public' AND table_name = 'users' | |
| AND constraint_name = 'check_analyses_used_non_negative' | |
| ) THEN | |
| ALTER TABLE public.users | |
| ADD CONSTRAINT check_analyses_used_non_negative CHECK (analyses_used >= 0); | |
| END IF; |
🤖 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 `@supabase/migrations/20260519_complete_stabilization.sql` around lines 35 -
37, The migration adds a non-idempotent constraint
(check_analyses_used_non_negative on table public.users) which will fail if
re-run; update the migration to guard the ALTER TABLE ... ADD CONSTRAINT by
first checking existence (e.g., query pg_constraint/pg_class/pg_namespace for
conname = 'check_analyses_used_non_negative') and only run ADD CONSTRAINT when
it does not exist, or alternatively run ALTER TABLE public.users DROP CONSTRAINT
IF EXISTS check_analyses_used_non_negative; then run ADD CONSTRAINT
check_analyses_used_non_negative CHECK (analyses_used >= 0) so the migration
becomes idempotent.
![cubic-dev-ai[bot]](https://avatars.github.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
7 issues found across 1 file
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/migrations/20260519_complete_stabilization.sql">
<violation number="1" location="supabase/migrations/20260519_complete_stabilization.sql:16">
P0: The DO block and nested function bodies reuse `$$` delimiters, which breaks SQL parsing and can make the migration fail before applying changes.</violation>
<violation number="2" location="supabase/migrations/20260519_complete_stabilization.sql:37">
P2: Guard this constraint creation with an existence check; this migration will fail if `check_analyses_used_non_negative` already exists.</violation>
<violation number="3" location="supabase/migrations/20260519_complete_stabilization.sql:71">
P2: Converting `published_at` with `COALESCE(..., NOW())` mutates NULL values and can incorrectly stamp unpublished records with a publish time.</violation>
<violation number="4" location="supabase/migrations/20260519_complete_stabilization.sql:84">
P2: Make this UNIQUE constraint addition idempotent; adding `unique_user_video` unconditionally can fail and stop the migration when it already exists.</violation>
<violation number="5" location="supabase/migrations/20260519_complete_stabilization.sql:169">
P1: Index creation for `usage_logs`/`stripe_events` is not existence-guarded, so this migration can fail on partial schemas.</violation>
<violation number="6" location="supabase/migrations/20260519_complete_stabilization.sql:197">
P1: `SECURITY DEFINER` quota RPCs update rows by arbitrary `p_user_id` without ownership checks, creating a privilege-escalation path.</violation>
<violation number="7" location="supabase/migrations/20260519_complete_stabilization.sql:242">
P2: Do not unconditionally return `reset_success = true`; when no user row matches `p_user_id`, this reports a successful reset that never happened.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
- Creates root-level package.json with workspace scripts (type-check, lint, test, build, dev) - Routes all verification gates through pnpm -r to run across all workspaces - Resolves 'ERR_PNPM_NO_IMPORTER_MANIFEST_FOUND' error in CI/CD pipeline - Enables GitHub Actions to discover and run tests correctly Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
…ility) - Regenerate lockfile to match Next.js 16.2.6 and all dependencies - Remove unstable_after import (not available in Next.js 16) - Replace after() callback with void promise pattern for background tasks - Replace crypto.timingSafeEqual with custom Edge Runtime-compatible version - Move database insert to synchronous path before streaming response All pre-commit gates pass: ✅ pnpm build ✅ pnpm type-check ✅ pnpm lint Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
|
❌ Pipeline Status: All checks passed. Ready to merge. |
There was a problem hiding this comment.
1 issue found across 6 files (changes from recent commits).
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="web/app/api/analyses/route.ts">
<violation number="1" location="web/app/api/analyses/route.ts:466">
P2: The new synchronous insert treats unique-key conflicts as a 500. With `UNIQUE(user_id, video_id)`, concurrent requests for the same video can now fail as internal errors instead of a deterministic "already in progress" response.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Comment on lines
+466
to
+469
| return NextResponse.json( | ||
| { error: 'Database error: Could not create analysis record' }, | ||
| { status: 500 } | ||
| ); |
There was a problem hiding this comment.
P2: The new synchronous insert treats unique-key conflicts as a 500. With UNIQUE(user_id, video_id), concurrent requests for the same video can now fail as internal errors instead of a deterministic "already in progress" response.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At web/app/api/analyses/route.ts, line 466:
<comment>The new synchronous insert treats unique-key conflicts as a 500. With `UNIQUE(user_id, video_id)`, concurrent requests for the same video can now fail as internal errors instead of a deterministic "already in progress" response.</comment>
<file context>
@@ -435,6 +435,40 @@ export async function POST(request: NextRequest) {
+ );
+ } catch (dbErr) {
+ console.error('[analyses] Failed to create initial record', { analysisId, error: String(dbErr) });
+ return NextResponse.json(
+ { error: 'Database error: Could not create analysis record' },
+ { status: 500 }
</file context>
Suggested change
| return NextResponse.json( | |
| { error: 'Database error: Could not create analysis record' }, | |
| { status: 500 } | |
| ); | |
| const isUniqueConflict = typeof dbErr === 'object' && dbErr !== null && 'code' in dbErr && (dbErr as { code?: string }).code === '23505'; | |
| return NextResponse.json( | |
| { error: isUniqueConflict ? 'Analysis already in progress for this video' : 'Database error: Could not create analysis record' }, | |
| { status: isUniqueConflict ? 409 : 500 } | |
| ); |
|
❌ Pipeline Status: All checks passed. Ready to merge. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/app/api/analyses/route.ts (1)
495-555:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd quota increment after successful analysis creation.
The
analyses_usedcounter is never incremented after an analysis completes. While theincrement_user_quota()RPC exists in migrations, it's never called in the flow. Users with free tier limits (3 analyses/month) can run unlimited analyses because the quota counter never increases.After the markdown update succeeds in the background task (around line 530), call the
increment_user_quota()RPC to increment the user'sanalyses_usedcounter. This prevents quota bypass and ensures fair billing.🤖 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 `@web/app/api/analyses/route.ts` around lines 495 - 555, The background task that collects markdown and updates the analyses row (inside the async void IIFE that reads from processorStream and uses parseSSELine and trackDatabaseQuery) must call the Postgres RPC increment_user_quota after the markdown DB update succeeds and before publishing the validation task; add a supabase.rpc('increment_user_quota', { user_id: userId || 'anonymous' }) invocation (or similar RPC call) right after the successful trackDatabaseQuery update, await it, and handle/log any RPC errors (Sentry.captureException/addBreadcrumb) so quota increments won't silently fail; ensure this runs only on successful update and does not block the overall background flow if it errors.
🤖 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.
Inline comments:
In `@supabase/migrations/20260519_rls_lockdown.sql`:
- Around line 105-108: The users UPDATE RLS policy incorrectly references NEW.id
in the WITH CHECK expression (NEW is invalid in RLS expressions); change that
reference to the column name directly (use id) so the condition reads
auth.role() = 'authenticated' AND auth.uid() = id (i.e., remove the NEW. prefix
in the WITH CHECK of the users UPDATE policy).
- Around line 47-50: The RLS policies use the invalid pseudo-record NEW; update
the WITH CHECK expressions to reference columns directly (remove the NEW.
prefix). Specifically, in the analyses_update_own policy replace auth.uid() =
NEW.user_id with auth.uid() = user_id, and in the users_update policy replace
auth.uid() = NEW.id with auth.uid() = id so the policy expressions reference the
proposed new column values correctly.
In `@supabase/migrations/20260519220000_complete_stabilization.sql`:
- Around line 226-253: The function reset_user_quota currently returns
reset_success = true unconditionally; modify it to check the UPDATE row count
(use GET DIAGNOSTICS v_affected = ROW_COUNT; or check FOUND) after the UPDATE
and set reset_success accordingly. Declare an integer variable (e.g.,
v_affected) alongside v_now, and if v_affected = 0 RETURN QUERY a row with
reset_success = false (and new_quota/reset_date as NULL or sensible defaults);
otherwise RETURN QUERY the existing success row (true, 0, v_now). Ensure you
update the RETURN QUERY logic in reset_user_quota to use v_affected to decide
the returned values.
- Around line 35-37: The CHECK constraint addition is not idempotent: update the
ALTER TABLE block that adds check_analyses_used_non_negative on public.users to
guard against duplicates by first dropping the constraint if it exists (use DROP
CONSTRAINT IF EXISTS check_analyses_used_non_negative on table public.users) and
then re-create it with ADD CONSTRAINT check_analyses_used_non_negative CHECK
(analyses_used >= 0); this mirrors the pattern used for other constraints in the
migration and ensures rerunning the migration won’t fail.
In `@web/middleware.ts`:
- Around line 6-13: The function timingSafeStringEqual leaks length via an early
return; remove the early-length check in timingSafeStringEqual and make the
comparison run a fixed number of operations by iterating up to the maximum of
a.length and b.length, using charCodeAt with a default 0 for out-of-range
indices, accumulate mismatches with bitwise ops (and also incorporate the length
difference into the accumulator, e.g., by OR-ing a.length ^ b.length) and
finally return whether the accumulator is zero so the function executes the same
steps regardless of input lengths.
---
Outside diff comments:
In `@web/app/api/analyses/route.ts`:
- Around line 495-555: The background task that collects markdown and updates
the analyses row (inside the async void IIFE that reads from processorStream and
uses parseSSELine and trackDatabaseQuery) must call the Postgres RPC
increment_user_quota after the markdown DB update succeeds and before publishing
the validation task; add a supabase.rpc('increment_user_quota', { user_id:
userId || 'anonymous' }) invocation (or similar RPC call) right after the
successful trackDatabaseQuery update, await it, and handle/log any RPC errors
(Sentry.captureException/addBreadcrumb) so quota increments won't silently fail;
ensure this runs only on successful update and does not block the overall
background flow if it errors.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 28eab1f9-4218-463a-92e0-9f90454e491c
⛔ Files ignored due to path filters (2)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlweb/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
package.jsonsupabase/migrations/20260519220000_complete_stabilization.sqlsupabase/migrations/20260519_rls_lockdown.sqlweb/app/api/analyses/route.tsweb/lib/qstash-client.tsweb/middleware.ts
Comment on lines
+35
to
+37
| -- Add quota underflow prevention check constraint | ||
| ALTER TABLE public.users | ||
| ADD CONSTRAINT check_analyses_used_non_negative CHECK (analyses_used >= 0); |
Contributor
There was a problem hiding this comment.
CHECK constraint addition is not idempotent.
ADD CONSTRAINT fails if the constraint already exists. Unlike foreign keys elsewhere in this migration (which use DROP CONSTRAINT IF EXISTS first), this CHECK constraint has no guard. Re-running the migration will error.
Proposed fix: add conditional guard
-- Add quota underflow prevention check constraint
- ALTER TABLE public.users
- ADD CONSTRAINT check_analyses_used_non_negative CHECK (analyses_used >= 0);
+ IF NOT EXISTS (
+ SELECT 1 FROM information_schema.table_constraints
+ WHERE constraint_name = 'check_analyses_used_non_negative'
+ AND table_schema = 'public' AND table_name = 'users'
+ ) THEN
+ ALTER TABLE public.users
+ ADD CONSTRAINT check_analyses_used_non_negative CHECK (analyses_used >= 0);
+ END IF;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| -- Add quota underflow prevention check constraint | |
| ALTER TABLE public.users | |
| ADD CONSTRAINT check_analyses_used_non_negative CHECK (analyses_used >= 0); | |
| -- Add quota underflow prevention check constraint | |
| IF NOT EXISTS ( | |
| SELECT 1 FROM information_schema.table_constraints | |
| WHERE constraint_name = 'check_analyses_used_non_negative' | |
| AND table_schema = 'public' AND table_name = 'users' | |
| ) THEN | |
| ALTER TABLE public.users | |
| ADD CONSTRAINT check_analyses_used_non_negative CHECK (analyses_used >= 0); | |
| END IF; |
🤖 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 `@supabase/migrations/20260519220000_complete_stabilization.sql` around lines
35 - 37, The CHECK constraint addition is not idempotent: update the ALTER TABLE
block that adds check_analyses_used_non_negative on public.users to guard
against duplicates by first dropping the constraint if it exists (use DROP
CONSTRAINT IF EXISTS check_analyses_used_non_negative on table public.users) and
then re-create it with ADD CONSTRAINT check_analyses_used_non_negative CHECK
(analyses_used >= 0); this mirrors the pattern used for other constraints in the
migration and ensures rerunning the migration won’t fail.
Comment on lines
+226
to
+253
| CREATE OR REPLACE FUNCTION public.reset_user_quota(p_user_id uuid) | ||
| RETURNS TABLE ( | ||
| reset_success boolean, | ||
| new_quota integer, | ||
| reset_date timestamp with time zone | ||
| ) | ||
| LANGUAGE plpgsql | ||
| SECURITY DEFINER | ||
| SET search_path = public | ||
| AS $$ | ||
| DECLARE | ||
| v_now timestamp with time zone; | ||
| BEGIN | ||
| v_now := CURRENT_TIMESTAMP AT TIME ZONE 'UTC'; | ||
|
|
||
| UPDATE public.users | ||
| SET | ||
| analyses_used = 0, | ||
| last_reset_date = v_now | ||
| WHERE id = p_user_id; | ||
|
|
||
| RETURN QUERY | ||
| SELECT | ||
| true as reset_success, | ||
| 0 as new_quota, | ||
| v_now as reset_date; | ||
| END; | ||
| $$; |
Contributor
There was a problem hiding this comment.
reset_user_quota returns success even when no user is updated.
If p_user_id doesn't exist, the UPDATE affects zero rows, but the function still returns reset_success = true. This masks errors and could lead to silent failures in quota reset workflows.
Proposed fix: check affected row count
CREATE OR REPLACE FUNCTION public.reset_user_quota(p_user_id uuid)
RETURNS TABLE (
reset_success boolean,
new_quota integer,
reset_date timestamp with time zone
)
LANGUAGE plpgsql
SECURITY DEFINER
SET search_path = public
AS $$
DECLARE
v_now timestamp with time zone;
+ v_rows_affected integer;
BEGIN
v_now := CURRENT_TIMESTAMP AT TIME ZONE 'UTC';
UPDATE public.users
SET
analyses_used = 0,
last_reset_date = v_now
WHERE id = p_user_id;
+ GET DIAGNOSTICS v_rows_affected = ROW_COUNT;
+
RETURN QUERY
SELECT
- true as reset_success,
+ (v_rows_affected > 0) as reset_success,
0 as new_quota,
v_now as reset_date;
END;
$$;🤖 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 `@supabase/migrations/20260519220000_complete_stabilization.sql` around lines
226 - 253, The function reset_user_quota currently returns reset_success = true
unconditionally; modify it to check the UPDATE row count (use GET DIAGNOSTICS
v_affected = ROW_COUNT; or check FOUND) after the UPDATE and set reset_success
accordingly. Declare an integer variable (e.g., v_affected) alongside v_now, and
if v_affected = 0 RETURN QUERY a row with reset_success = false (and
new_quota/reset_date as NULL or sensible defaults); otherwise RETURN QUERY the
existing success row (true, 0, v_now). Ensure you update the RETURN QUERY logic
in reset_user_quota to use v_affected to decide the returned values.
Comment on lines
+6
to
+13
| function timingSafeStringEqual(a: string, b: string): boolean { | ||
| if (a.length !== b.length) return false; | ||
| let mismatch = 0; | ||
| for (let i = 0; i < a.length; i++) { | ||
| mismatch |= a.charCodeAt(i) ^ b.charCodeAt(i); | ||
| } | ||
| return mismatch === 0; | ||
| } |
Contributor
There was a problem hiding this comment.
Timing leak in length comparison defeats constant-time security.
The early return on line 7 when lengths differ leaks timing information about the secret's length. An attacker can probe different input lengths and measure response times to determine the expected token length, which significantly reduces the search space for brute-force attacks.
A proper constant-time comparison must always perform the same number of operations regardless of input. Even though this is dev-only, it degrades the security guarantee that timing-safe comparison should provide.
🔒 Proposed fix for constant-time comparison
-function timingSafeStringEqual(a: string, b: string): boolean {
- if (a.length !== b.length) return false;
- let mismatch = 0;
- for (let i = 0; i < a.length; i++) {
- mismatch |= a.charCodeAt(i) ^ b.charCodeAt(i);
- }
- return mismatch === 0;
-}
+function timingSafeStringEqual(a: string, b: string): boolean {
+ // Include length difference in mismatch to avoid timing leak
+ const maxLen = Math.max(a.length, b.length);
+ let mismatch = a.length ^ b.length;
+
+ for (let i = 0; i < maxLen; i++) {
+ const aCode = i < a.length ? a.charCodeAt(i) : 0;
+ const bCode = i < b.length ? b.charCodeAt(i) : 0;
+ mismatch |= aCode ^ bCode;
+ }
+ return mismatch === 0;
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function timingSafeStringEqual(a: string, b: string): boolean { | |
| if (a.length !== b.length) return false; | |
| let mismatch = 0; | |
| for (let i = 0; i < a.length; i++) { | |
| mismatch |= a.charCodeAt(i) ^ b.charCodeAt(i); | |
| } | |
| return mismatch === 0; | |
| } | |
| function timingSafeStringEqual(a: string, b: string): boolean { | |
| // Include length difference in mismatch to avoid timing leak | |
| const maxLen = Math.max(a.length, b.length); | |
| let mismatch = a.length ^ b.length; | |
| for (let i = 0; i < maxLen; i++) { | |
| const aCode = i < a.length ? a.charCodeAt(i) : 0; | |
| const bCode = i < b.length ? b.charCodeAt(i) : 0; | |
| mismatch |= aCode ^ bCode; | |
| } | |
| return mismatch === 0; | |
| } |
🤖 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 `@web/middleware.ts` around lines 6 - 13, The function timingSafeStringEqual
leaks length via an early return; remove the early-length check in
timingSafeStringEqual and make the comparison run a fixed number of operations
by iterating up to the maximum of a.length and b.length, using charCodeAt with a
default 0 for out-of-range indices, accumulate mismatches with bitwise ops (and
also incorporate the length difference into the accumulator, e.g., by OR-ing
a.length ^ b.length) and finally return whether the accumulator is zero so the
function executes the same steps regardless of input lengths.
|
❌ Pipeline Status: All checks passed. Ready to merge. |
There was a problem hiding this comment.
4 issues found across 7 files (changes from recent commits).
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/migrations/20260519_rls_lockdown.sql">
<violation number="1" location="supabase/migrations/20260519_rls_lockdown.sql:38">
P2: Policy creation was moved outside table-existence guards, so this migration can fail hard when any target table is absent.</violation>
</file>
<file name="supabase/migrations/20260519220000_complete_stabilization.sql">
<violation number="1" location="supabase/migrations/20260519220000_complete_stabilization.sql:197">
P1: These SECURITY DEFINER RPCs lack caller authorization and execute-privilege hardening, so they can be used to update arbitrary users’ quota rows if callable by non-admin roles.</violation>
<violation number="2" location="supabase/migrations/20260519220000_complete_stabilization.sql:214">
P2: Validate `p_increment` as positive; currently negative values can decrease `analyses_used` and undermine quota enforcement.</violation>
<violation number="3" location="supabase/migrations/20260519220000_complete_stabilization.sql:239">
P2: This UTC conversion is incorrect for `timestamptz`; it can introduce timezone shifts when storing `last_reset_date`.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| -- This RPC atomically increments the user quota counter | ||
| -- Used by rate-limit enforcement to prevent race conditions | ||
|
|
||
| CREATE OR REPLACE FUNCTION public.increment_user_quota( |
There was a problem hiding this comment.
P1: These SECURITY DEFINER RPCs lack caller authorization and execute-privilege hardening, so they can be used to update arbitrary users’ quota rows if callable by non-admin roles.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At supabase/migrations/20260519220000_complete_stabilization.sql, line 197:
<comment>These SECURITY DEFINER RPCs lack caller authorization and execute-privilege hardening, so they can be used to update arbitrary users’ quota rows if callable by non-admin roles.</comment>
<file context>
@@ -173,81 +176,81 @@ DO $$ BEGIN
+-- This RPC atomically increments the user quota counter
+-- Used by rate-limit enforcement to prevent race conditions
+
+CREATE OR REPLACE FUNCTION public.increment_user_quota(
+ p_user_id uuid,
+ p_increment integer DEFAULT 1
</file context>
| -- ============================================================================ | ||
| -- PUBLIC.ANALYSES TABLE: Restrict to authenticated users, enforce user_id match | ||
| -- ============================================================================ | ||
| CREATE POLICY "analyses_insert_own" |
There was a problem hiding this comment.
P2: Policy creation was moved outside table-existence guards, so this migration can fail hard when any target table is absent.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At supabase/migrations/20260519_rls_lockdown.sql, line 38:
<comment>Policy creation was moved outside table-existence guards, so this migration can fail hard when any target table is absent.</comment>
<file context>
@@ -2,154 +2,129 @@
+-- ============================================================================
+-- PUBLIC.ANALYSES TABLE: Restrict to authenticated users, enforce user_id match
+-- ============================================================================
+CREATE POLICY "analyses_insert_own"
+ ON public.analyses
+ FOR INSERT
</file context>
| DECLARE | ||
| v_now timestamp with time zone; | ||
| BEGIN | ||
| v_now := CURRENT_TIMESTAMP AT TIME ZONE 'UTC'; |
There was a problem hiding this comment.
P2: This UTC conversion is incorrect for timestamptz; it can introduce timezone shifts when storing last_reset_date.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At supabase/migrations/20260519220000_complete_stabilization.sql, line 239:
<comment>This UTC conversion is incorrect for `timestamptz`; it can introduce timezone shifts when storing `last_reset_date`.</comment>
<file context>
@@ -173,81 +176,81 @@ DO $$ BEGIN
+DECLARE
+ v_now timestamp with time zone;
+BEGIN
+ v_now := CURRENT_TIMESTAMP AT TIME ZONE 'UTC';
+
+ UPDATE public.users
</file context>
Suggested change
| v_now := CURRENT_TIMESTAMP AT TIME ZONE 'UTC'; | |
| v_now := CURRENT_TIMESTAMP; |
| v_tier text; | ||
| BEGIN | ||
| UPDATE public.users | ||
| SET analyses_used = analyses_used + p_increment |
There was a problem hiding this comment.
P2: Validate p_increment as positive; currently negative values can decrease analyses_used and undermine quota enforcement.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At supabase/migrations/20260519220000_complete_stabilization.sql, line 214:
<comment>Validate `p_increment` as positive; currently negative values can decrease `analyses_used` and undermine quota enforcement.</comment>
<file context>
@@ -173,81 +176,81 @@ DO $$ BEGIN
+ v_tier text;
+BEGIN
+ UPDATE public.users
+ SET analyses_used = analyses_used + p_increment
+ WHERE id = p_user_id
+ RETURNING analyses_used, "tier" INTO v_new_quota, v_tier;
</file context>
Suggested change
| SET analyses_used = analyses_used + p_increment | |
| SET analyses_used = analyses_used + GREATEST(p_increment, 0) |
|
❌ Pipeline Status: All checks passed. Ready to merge. |
There was a problem hiding this comment.
3 issues found across 4 files (changes from recent commits).
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=".github/workflows/staging-deploy.yml">
<violation number="1" location=".github/workflows/staging-deploy.yml:70">
P1: Avoid using a floating GitHub Action ref (`@master`) for deployments; pin to an immutable version (or commit SHA) to keep CI reproducible and reduce supply-chain risk.</violation>
</file>
<file name=".github/workflows/ci-cd.yml">
<violation number="1" location=".github/workflows/ci-cd.yml:324">
P1: Avoid using a floating `@master` ref for GitHub Actions in deployment. Pin to a fixed version (or commit SHA) to prevent supply-chain drift and secret exposure risk.</violation>
</file>
<file name="docs/ops/GITHUB_SECRETS_SETUP.md">
<violation number="1" location="docs/ops/GITHUB_SECRETS_SETUP.md:44">
P2: `vercel whoami` returns the username, not the Organization ID (`orgId`). Using it for `VERCEL_ORG_ID` will cause Vercel CLI authentication to fail in GitHub Actions.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
|
|
||
| - name: Deploy to Vercel (Staging) | ||
| uses: vercel/action@v5.1.0 | ||
| uses: amondnet/vercel-action@master |
There was a problem hiding this comment.
P1: Avoid using a floating GitHub Action ref (@master) for deployments; pin to an immutable version (or commit SHA) to keep CI reproducible and reduce supply-chain risk.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/staging-deploy.yml, line 70:
<comment>Avoid using a floating GitHub Action ref (`@master`) for deployments; pin to an immutable version (or commit SHA) to keep CI reproducible and reduce supply-chain risk.</comment>
<file context>
@@ -67,12 +67,11 @@ jobs:
- name: Deploy to Vercel (Staging)
- uses: vercel/action@v5.1.0
+ uses: amondnet/vercel-action@master
with:
vercel-token: ${{ secrets.VERCEL_TOKEN }}
</file context>
|
|
||
| - name: Deploy to Vercel | ||
| uses: vercel/action@v5.1.0 | ||
| uses: amondnet/vercel-action@master |
There was a problem hiding this comment.
P1: Avoid using a floating @master ref for GitHub Actions in deployment. Pin to a fixed version (or commit SHA) to prevent supply-chain drift and secret exposure risk.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/ci-cd.yml, line 324:
<comment>Avoid using a floating `@master` ref for GitHub Actions in deployment. Pin to a fixed version (or commit SHA) to prevent supply-chain drift and secret exposure risk.</comment>
<file context>
@@ -321,12 +321,12 @@ jobs:
- name: Deploy to Vercel
- uses: vercel/action@v5.1.0
+ uses: amondnet/vercel-action@master
with:
vercel-token: ${{ secrets.VERCEL_TOKEN }}
</file context>
|
|
||
| **Get VERCEL_ORG_ID:** | ||
| ```bash | ||
| vercel whoami |
There was a problem hiding this comment.
P2: vercel whoami returns the username, not the Organization ID (orgId). Using it for VERCEL_ORG_ID will cause Vercel CLI authentication to fail in GitHub Actions.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/ops/GITHUB_SECRETS_SETUP.md, line 44:
<comment>`vercel whoami` returns the username, not the Organization ID (`orgId`). Using it for `VERCEL_ORG_ID` will cause Vercel CLI authentication to fail in GitHub Actions.</comment>
<file context>
@@ -0,0 +1,116 @@
+
+**Get VERCEL_ORG_ID:**
+```bash
+vercel whoami
+# Returns org ID
+```
</file context>
|
❌ Linting failed. Please fix linting errors before merging. |
|
❌ Pipeline Status: All checks passed. Ready to merge. |
|
❌ Pipeline Status: All checks passed. Ready to merge. |
|
❌ Pipeline Status: All checks passed. Ready to merge. |
|
❌ Pipeline Status: All checks passed. Ready to merge. |
|
❌ Pipeline Status: All checks passed. Ready to merge. |
|
❌ Pipeline Status: All checks passed. Ready to merge. |
|
❌ Pipeline Status: All checks passed. Ready to merge. |
|
✅ Pipeline Status: All checks passed. Ready to merge. |
6 tasks
5 tasks
TechHypeXP
added a commit
that referenced
this pull request
Jul 31, 2026
* database: Add complete stabilization migration with all 7 audit fixes This migration applies all critical schema fixes in a single transaction: 1. Auth linkage (users.id references auth.users) 2. Cascading deletes (ON DELETE CASCADE/SET NULL) 3. Missing columns (model_attempted, validation_report, validation_passed) 4. Timezone safety (timestamptz for all timestamps) 5. Concurrent safety (UNIQUE constraint on user_video) 6. Quota underflow prevention (CHECK constraint) 7. Performance indexes (cache lookup, vector, foreign keys) Additionally creates two RPCs: - increment_user_quota: Atomic quota enforcement - reset_user_quota: Monthly quota reset helper Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * build: Add root package.json for pnpm monorepo configuration - Creates root-level package.json with workspace scripts (type-check, lint, test, build, dev) - Routes all verification gates through pnpm -r to run across all workspaces - Resolves 'ERR_PNPM_NO_IMPORTER_MANIFEST_FOUND' error in CI/CD pipeline - Enables GitHub Actions to discover and run tests correctly Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * build: sync pnpm-lock.yaml with package.json (Next.js 16.2.6 compatibility) - Regenerate lockfile to match Next.js 16.2.6 and all dependencies - Remove unstable_after import (not available in Next.js 16) - Replace after() callback with void promise pattern for background tasks - Replace crypto.timingSafeEqual with custom Edge Runtime-compatible version - Move database insert to synchronous path before streaming response All pre-commit gates pass: ✅ pnpm build ✅ pnpm type-check ✅ pnpm lint Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * fix(db): resolve migration version collision by timestamping to 20260519220000 * fix(rls): resolve RLS policy syntax - use direct column references instead of NEW * chore(db): remove duplicate migration files from version control * ci: fix pipeline issues - update Vercel action, add TypeScript deprecation ignore * docs(ops): add GitHub Actions secrets configuration guide * fix(build): reconcile lockfile with Next.js 16.2.6 manifest * fix(security): implement sentry fail-silent guard and enhance middleware public route exclusions * fix(sentry+cors): finalize robust instrumentation and inject preflight headers * fix(env): resolve composite action context bug and strict TS env null checks * fix(ci): unify monorepo root install and scoped build filter * fix(ci): standardize monorepo workflows, bump pnpm to 11.1.3, and remove legacy pathing * docs(history): add session handover report for 2026-05-20 * fix(web): migrate to proxy convention and optimize turbopack root * crit(security): restore middleware auth gate and fix CI secret false positive * feat(stabilization): Complete Tier 1-3 security and resilience hardening TIER 1: FOUNDATION - RLS & Request-Scoped Auth - Database: Apply RLS lockdown migration (20260520) • Enforce auth.uid() = user_id on analyses table • Enforce auth.uid() = id on users table • Immutable audit trail on usage_logs - Supabase Client: Implement request-scoped factory • Add getSupabaseClientWithAuth() for JWT-bound clients • Extract user tokens from cookies for RLS enforcement • Maintain backward compatibility with getSupabaseClient() • Ensure multi-tenant data isolation per request TIER 2: EXTERNAL RESILIENCE - Timeout & SSRF Hardening - SSRF Validation (analyses/route.ts): • Replace endsWith() with exact hostname matching • Use Set-based allowlist against worker domain spoofing - Embedding Timeout (embeddings.ts): • Add 5s AbortController timeout to OpenRouter calls • Clear timeout in finally block to prevent memory leaks • Retry with exponential backoff on timeout - Webhook Verification Timeout (qstash-client.ts): • Wrap receiver.verify() in Promise.race with 5s timeout • Prevent signature verification from hanging webhook handlers TIER 3: FUNCTIONAL LOGIC - Token Budget & Redis Resilience - Token Math (services/openrouter.ts): • Implement dynamic max_tokens scaling per Law #2 • Base: 4000 tokens + (transcript_length / 10) • Cap: 10000 tokens maximum • Prevents truncation on long transcripts, limits budget overruns - Redis Circuit Breaker (redis.ts): • Add exponential backoff retry to executeRedisScript() • 3 retries at 100ms, 200ms, 400ms delays • Distinguish transient (timeout, connection) vs permanent (auth) errors • Mark Redis unavailable after retries exhausted, fallback to in-memory cache All changes verified via: ✓ pnpm build (13.9s) ✓ pnpm type-check (0 errors) ✓ pnpm lint (0 violations) Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * fix(ci): update Node.js version to 22 for pnpm 11.1.3 compatibility * fix(ci): resolve tsconfig deprecation and fix monorepo pathing * fix(ci): remove invalid tsconfig option and move to CLI flag * fix(ci): enable recursive pnpm install for monorepo workspace linking * fix(ci): add .npmrc to unify peer dependency resolution * fix(ci): hard-code auto-install-peers to resolve lockfile mismatch * fix(ci): use env var for pnpm peer settings and force cache refresh * ci(config): remove branch restrictions to enable stabilization validation * fix(ci): perform clean lockfile regeneration to clear config metadata * fix(ci): remove invalid ignoreDeprecations flag to resolve TS5103 * fix(build): replace hardcoded turbopack root path and anchor distDir * fix(ci): exclude node_modules from security scan * fix(ci): inject supabase environment variables into build step * fix(ci): inject dummy env fallbacks to satisfy static build validation * fix(build): add missing openrouter key fallback for static build validation --------- Co-authored-by: Kelly Bakri <noreply@github.com> Co-authored-by: Claude Haiku 4.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR applies all critical schema fixes identified in the database audit report as a single initial stabilization migration.
Fixes Applied (7/7)
timestamptzfor UTC consistencyAdditional Updates
increment_user_quota()RPC for atomic quota enforcementreset_user_quota()RPC for monthly quota resetsVerification
✅ All columns in insert statement match migration schema (route.ts:469-483)
Test Plan
supabase db pushto apply migration to remotenpx supabase gen types typescript🤖 Generated with Claude Code
Summary by Sourcery
Apply a stabilization database migration that hardens core tables, constraints, and performance characteristics based on the audit findings.
Enhancements:
Summary by cubic
Completes the stabilization migration and security hardening: applies all 7 audit fixes with strict RLS and request‑scoped auth, safer timeouts, and exact hostname allowlists. Standardizes CI on Node 24 +
pnpm@11.1.3, finalizes Sentry/QStash, upgrades tonext@16.2.6, anchors Turbopack, and injects Supabase/OpenRouter env vars at build with safe CI fallbacks.New Features
increment_user_quota()andreset_user_quota()for atomic quota checks and monthly resets.vectorenabled;analyses.embedding,analyses.shared_token,analyses.shared_expires_at.Refactors
timestamptz,UNIQUE(user_id, video_id), non‑negative checks, perf indexes) plus strict RLS onusers/analyses/usage_logs.@supabase/ssr; background tasks withafter(); exact‑hostname SSRF allowlist; dynamicmax_tokens; 5s timeouts for embeddings and QStash verify; Redis backoff/circuit breaker; requireNEXT_PUBLIC_APP_URLfor QStash.client/server/edge) with DSN guard.pnpm@11.1.3; rootpackage.json+ monorepopnpm-lock.yaml+.npmrc; secret scan excludesnode_modules; Vercel deploy viaamondnet/vercel-action@master; build injects Supabase + OpenRouter env with dummy fallbacks..nextdistDir; web onnext@16.2.6.Written for commit 03a88ba. Summary will update on new commits. Review in cubic
Summary by CodeRabbit
New Features
Improvements
Chores