Conversation
| const response = await fetch( | ||
| `https://api.supabase.com/v1/projects/${session.projectRef}/database/query`, | ||
| { | ||
| method: 'POST', | ||
| headers: { | ||
| Authorization: `Bearer ${session.accessToken}`, | ||
| 'Content-Type': 'application/json', | ||
| }, | ||
| body: JSON.stringify({ query: SYNC_PROGRESS_QUERY }), | ||
| } | ||
| ) |
Check failure
Code scanning / CodeQL
Server-side request forgery Critical
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 17 hours ago
In general, the problem is that supabaseProjectRef (from the deploy request body) is stored as session.projectRef and later interpolated into a URL path without any validation. To fix this without changing the visible behavior, we should validate and normalize the project reference at the point of intake in packages/dashboard/src/app/api/deploy/route.ts, rejecting values that are clearly malformed, and store only this sanitized value in the session. That way, by the time sync-progress/route.ts reads session.projectRef, it is guaranteed to be a safe path segment for the Supabase API URL.
The single best fix here is:
- In
deploy/route.ts, add validation logic right after readingsupabaseProjectReffrom the request body:- Trim whitespace.
- Ensure it matches a conservative pattern of allowed characters (e.g., lowercase letters, digits, and dashes), which aligns with typical Supabase project ref formats like
abcd1234. - Optionally enforce a reasonable length bound.
- If the value fails validation, return a
400error.
- Use the sanitized value (e.g.,
normalizedSupabaseProjectRef) in both:- The call to
install({ supabaseProjectRef: ... }). - The call to
createSession(...).
- The call to
- No change is required in
sync-progress/route.tsbecause it will now only ever see a validatedprojectRef.
This keeps existing functionality (deploying and storing sessions) but prevents arbitrary, potentially dangerous path components from being persisted and later used to build request URLs. No new external dependencies are strictly necessary; we can implement validation with a simple regular expression.
Concretely:
- Edit
packages/dashboard/src/app/api/deploy/route.ts:- After destructuring
supabaseAccessToken, supabaseProjectRef, stripeKey, add validation logic. - Introduce a new variable, e.g.,
const normalizedSupabaseProjectRef = supabaseProjectRef.trim();then verify it with a regex like/^[a-z0-9-]{1,64}$/. - If invalid, return
400with an error message. - Use
normalizedSupabaseProjectRefinstead ofsupabaseProjectRefwhen callinginstallandcreateSession.
- After destructuring
- No changes are needed in
sessions.tsorsync-progress/route.tsbecause they operate on the sanitized value stored in the session.
| @@ -17,15 +17,22 @@ | ||
| return NextResponse.json({ error: 'Missing required fields' }, { status: 400 }) | ||
| } | ||
|
|
||
| const normalizedSupabaseProjectRef = supabaseProjectRef.trim() | ||
| // Allow only typical Supabase project ref characters to avoid unsafe URL path segments | ||
| const projectRefPattern = /^[a-z0-9-]{1,64}$/ | ||
| if (!projectRefPattern.test(normalizedSupabaseProjectRef)) { | ||
| return NextResponse.json({ error: 'Invalid Supabase project reference' }, { status: 400 }) | ||
| } | ||
|
|
||
| await install({ | ||
| supabaseAccessToken, | ||
| supabaseProjectRef, | ||
| supabaseProjectRef: normalizedSupabaseProjectRef, | ||
| stripeKey, | ||
| workerIntervalSeconds: 30, | ||
| }) | ||
|
|
||
| // Create session to store credentials server-side (for Management API queries) | ||
| const sessionId = createSession(supabaseProjectRef, supabaseAccessToken) | ||
| const sessionId = createSession(normalizedSupabaseProjectRef, supabaseAccessToken) | ||
|
|
||
| return NextResponse.json({ | ||
| success: true, |
There was a problem hiding this comment.
False positive, user provides a projectRef as intended
What kind of change does this PR introduce?
Adds a progress status in the dashboard
What is the current behavior?
After deployment there is just a text fields that says something like "syncing since..."
What is the new behavior?
Additional context
Add any other context or screenshots.