fix: secure fetch-transcript endpoint and implement membership-based database isolation - #174
shivansh023023 wants to merge 2 commits into
Conversation
📝 WalkthroughWalkthroughThis PR implements security hardening across two areas: database authorization policies are restricted to authenticated users with row-level visibility controls, and the fetch-transcript serverless function adds Bearer token authentication, input validation, enhanced VEXA API integration with bot session cleanup, and improved error handling with unified database updates. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Function as fetch-transcript<br/>Function
participant VexaAPI as Vexa API
participant Supabase as Supabase DB
Client->>Function: POST with Authorization header
Function->>Function: Validate Bearer token
alt Token missing/invalid
Function->>Client: 401 Unauthorized
end
Function->>Function: Validate environment config<br/>(VEXA_API_KEY, SUPABASE_URL, etc.)
alt Config missing
Function->>Client: 500 Config Error
end
Function->>Function: Parse & validate request body<br/>(meeting_url, meeting_id)
alt Invalid inputs
Function->>Client: 400 Bad Request
end
Function->>Function: Extract Google Meet ID<br/>from meeting_url
Function->>VexaAPI: GET /transcript?meetId=...
alt Transcript fetch succeeds
VexaAPI->>Function: Transcript data
Function->>VexaAPI: DELETE /session/{meetId}
VexaAPI->>Function: Bot session stopped
Function->>Supabase: UPDATE meetings<br/>(transcript, timestamp,<br/>clear error)
Supabase->>Function: Update confirmed
Function->>Client: 200 Success
else Transcript fetch fails
VexaAPI->>Function: Error
Function->>Supabase: UPDATE meetings<br/>(transcription_error,<br/>transcription_attempted_at)
Supabase->>Function: Update confirmed
Function->>Client: 500 Internal Error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
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)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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 |
shivansh023023
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
sqls/02_user_auth_policies.sql (2)
45-52: 🛠️ Refactor suggestion | 🟠 MajorConsider replacing view reference with a direct subquery to eliminate the exposure.
The teams policy at line 27 already queries
usersdirectly:SELECT team_id FROM users WHERE id = auth.uid(). This works because the user's own RLS policy (id = auth.uid()) permits it. The same pattern can be used here, removing the dependency on theuser_teamsview entirely:Proposed fix
CREATE POLICY "Users can view team members" ON users FOR SELECT USING ( - -- Cross-reference with the user_teams view to restrict visibility team_id IN ( - SELECT team_id FROM user_teams WHERE id = auth.uid() + SELECT u.team_id FROM users u WHERE u.id = auth.uid() ) );If the view is no longer referenced by any policy, you can drop it and remove the associated
GRANT:DROP VIEW IF EXISTS user_teams; -- and remove: GRANT SELECT ON user_teams TO authenticated;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sqls/02_user_auth_policies.sql` around lines 45 - 52, The policy "Users can view team members" uses the user_teams view which exposes an extra object; change the USING clause to perform a direct subquery against the users table (e.g., use SELECT team_id FROM users WHERE id = auth.uid()) so it relies on the existing users RLS and removes the dependency on the user_teams view, and if user_teams is no longer used by any other policy/drop it and remove its GRANT for authenticated.
13-15:⚠️ Potential issue | 🟠 Major
user_teamsview exposes all user-to-team mappings to any authenticated user.The view
SELECT id, team_id FROM users(lines 13–15) returns every user'sidandteam_idwithout filtering. In PostgreSQL/Supabase, views owned by the database superuser default toSECURITY DEFINERbehavior, which bypasses RLS on the underlyinguserstable. Combined withGRANT SELECT ON user_teams TO authenticated(line 65), any authenticated user can query the view directly and enumerate all user-to-team relationships—defeating the row-level policies you've defined.Recommended fix: Remove the direct grant. RLS policies can still reference the view because policy evaluation executes with superuser privileges:
Proposed fix – revoke authenticated users' direct access
GRANT USAGE ON SCHEMA public TO authenticated; -GRANT SELECT ON user_teams TO authenticated;This prevents direct enumeration while preserving the view's use within policies (lines 48–51). If you're on Postgres 15+, an alternative is to add
security_invoker = trueto the view definition for stricter control.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sqls/02_user_auth_policies.sql` around lines 13 - 15, The view user_teams (SELECT id, team_id FROM users) is exposing all user-to-team mappings because it was granted to the authenticated role; revoke that direct access so authenticated users cannot query the view directly — remove or undo the GRANT SELECT ON user_teams TO authenticated and ensure any RLS policies that reference user_teams (lines using policies) continue to work; optionally, on Postgres 15+ re-create the view with security_invoker = true to enforce invoker privileges if stricter control is desired.
🧹 Nitpick comments (3)
supabase/functions/fetch-transcript/index.ts (3)
36-41: OnlyGETis explicitly rejected; other non-POST methods fall through.
PUT,DELETE,PATCH, etc. will bypass this guard and attempt body parsing. Consider an explicit POST check instead:- if (req.method === "GET") { + if (req.method !== "POST") { return new Response( JSON.stringify({ message: "This endpoint requires a POST request" }), { status: 400, headers: { "Content-Type": "application/json" } } ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/functions/fetch-transcript/index.ts` around lines 36 - 41, The current guard only rejects GET but lets other non-POST methods proceed; change the request validation to explicitly require POST by checking req.method !== "POST" (or equivalent) and immediately return a proper error response (e.g., 405 Method Not Allowed or 400 with a clear message) with JSON Content-Type; update the branch around the existing req.method check in fetch-transcript handler to reject any non-POST methods before attempting to parse the body.
89-93: Bot-stop DELETE response is not checked.If the
DELETEcall to stop the Vexa bot fails (network error, non-2xx status), the error is silently swallowed and the bot session may keep running and consuming resources. Consider at minimum logging a warning on failure:Suggested improvement
// Stop the bot session - await fetch( + const stopRes = await fetch( `https://gateway.dev.vexa.ai/bots/google_meet/${meetId}`, { method: "DELETE", headers: { "X-API-Key": VEXA_API_KEY } } ); + if (!stopRes.ok) { + console.warn(`Failed to stop bot session for ${meetId}: ${stopRes.status}`); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/functions/fetch-transcript/index.ts` around lines 89 - 93, The DELETE request that stops the Vexa bot (the fetch call to `https://gateway.dev.vexa.ai/bots/google_meet/${meetId}` using `VEXA_API_KEY`) currently ignores failures; wrap this call in a try/catch and validate the response (check response.ok), logging a warning with status and response text when the status is non-2xx and logging the caught error on network failure so bot-stop failures are visible; keep the same URL and header usage but ensure failures are surfaced via console.warn or the existing logger.
22-22: Token extraction is fragile with plainreplace.
authHeader.replace('Bearer ', '')won't strip the prefix if the casing differs (e.g.bearer) and will silently pass the full header value as the token if noBearerprefix is present. WhilegetUserwill reject invalid tokens, a targeted check avoids sending garbage to the auth service.Suggested improvement
- const token = authHeader.replace('Bearer ', ''); + const token = authHeader.startsWith('Bearer ') ? authHeader.slice(7) : ''; + if (!token) { + return new Response( + JSON.stringify({ error: 'Missing Bearer token' }), + { status: 401, headers: { "Content-Type": "application/json" } } + ); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/functions/fetch-transcript/index.ts` at line 22, The token extraction using authHeader.replace('Bearer ', '') is fragile and can produce garbage if the prefix is missing or differently cased; update the extraction around authHeader and token to validate that authHeader exists and begins with the Bearer prefix case-insensitively (e.g., check authHeader.toLowerCase().startsWith('bearer ')) then extract the token via slicing and trim it, and if the prefix is missing or the resulting token is empty respond with a 401/appropriate error before calling getUser so you never pass an invalid header through to getUser.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@sqls/02_user_auth_policies.sql`:
- Around line 35-38: The "Allow team creation" INSERT policy on teams currently
only checks auth.role(); update its WITH CHECK condition to also require
created_by = auth.uid() so creators cannot set another user's id. Locate the
CREATE POLICY "Allow team creation" ON teams FOR INSERT and modify the WITH
CHECK to include created_by = auth.uid() alongside auth.role() = 'authenticated'
to enforce ownership on insert.
In `@supabase/functions/fetch-transcript/index.ts`:
- Line 73: The meetId extraction can yield an empty string when meeting_url ends
with a trailing slash; update the logic that sets meetId (currently using
meeting_url.split('/').pop().split('?')[0]) to first remove trailing slashes or
pick the last non-empty segment (e.g., split and filter(Boolean) then take last)
and then strip query params, or use the URL/pathname to safely extract the last
segment; ensure meetId is validated (non-empty) before sending to the Vexa API
and handle the error case if it remains empty.
- Around line 20-22: The Supabase client is being initialized with
non-null-asserted env vars (createClient(SUPABASE_URL!,
SUPABASE_SERVICE_ROLE_KEY!)) before the runtime check that they exist; move the
environment validation so SUPABASE_URL and SUPABASE_SERVICE_ROLE_KEY are
checked/throwing before calling createClient, or alternatively defer the
createClient call into the try block after the existing check around authHeader
so createClient only runs with validated values, and remove the duplicate
post-check at the later lines; update references to createClient and the env var
checks accordingly.
- Around line 96-103: The current update uses the service-role Supabase client
(supabase) with a user-supplied meeting_id, allowing authorization bypass; fix
by enforcing ownership or using a user-scoped client: either 1) query the
meetings row first with supabase.from('meetings').select(...).eq('id',
meeting_id) and verify the requesting user's id (user.id) is part of the
meeting/team before performing the update (only proceed if ownership/membership
is confirmed), or 2) build a second Supabase client using the caller's JWT and
call that client to perform the .update(...) so RLS is applied; apply the same
change to the other update block that writes
transcription_error/transcription_attempted_at.
---
Outside diff comments:
In `@sqls/02_user_auth_policies.sql`:
- Around line 45-52: The policy "Users can view team members" uses the
user_teams view which exposes an extra object; change the USING clause to
perform a direct subquery against the users table (e.g., use SELECT team_id FROM
users WHERE id = auth.uid()) so it relies on the existing users RLS and removes
the dependency on the user_teams view, and if user_teams is no longer used by
any other policy/drop it and remove its GRANT for authenticated.
- Around line 13-15: The view user_teams (SELECT id, team_id FROM users) is
exposing all user-to-team mappings because it was granted to the authenticated
role; revoke that direct access so authenticated users cannot query the view
directly — remove or undo the GRANT SELECT ON user_teams TO authenticated and
ensure any RLS policies that reference user_teams (lines using policies)
continue to work; optionally, on Postgres 15+ re-create the view with
security_invoker = true to enforce invoker privileges if stricter control is
desired.
---
Nitpick comments:
In `@supabase/functions/fetch-transcript/index.ts`:
- Around line 36-41: The current guard only rejects GET but lets other non-POST
methods proceed; change the request validation to explicitly require POST by
checking req.method !== "POST" (or equivalent) and immediately return a proper
error response (e.g., 405 Method Not Allowed or 400 with a clear message) with
JSON Content-Type; update the branch around the existing req.method check in
fetch-transcript handler to reject any non-POST methods before attempting to
parse the body.
- Around line 89-93: The DELETE request that stops the Vexa bot (the fetch call
to `https://gateway.dev.vexa.ai/bots/google_meet/${meetId}` using
`VEXA_API_KEY`) currently ignores failures; wrap this call in a try/catch and
validate the response (check response.ok), logging a warning with status and
response text when the status is non-2xx and logging the caught error on network
failure so bot-stop failures are visible; keep the same URL and header usage but
ensure failures are surfaced via console.warn or the existing logger.
- Line 22: The token extraction using authHeader.replace('Bearer ', '') is
fragile and can produce garbage if the prefix is missing or differently cased;
update the extraction around authHeader and token to validate that authHeader
exists and begins with the Bearer prefix case-insensitively (e.g., check
authHeader.toLowerCase().startsWith('bearer ')) then extract the token via
slicing and trim it, and if the prefix is missing or the resulting token is
empty respond with a 401/appropriate error before calling getUser so you never
pass an invalid header through to getUser.
| CREATE POLICY "Allow team creation" | ||
| ON teams FOR INSERT | ||
| WITH CHECK (TRUE); | ||
| -- Restrict team creation to authenticated users | ||
| WITH CHECK (auth.role() = 'authenticated'); |
There was a problem hiding this comment.
Team INSERT policy should enforce created_by = auth.uid().
The UPDATE policy trusts created_by to gate admin access, but the INSERT policy doesn't constrain it. An authenticated user could insert a team with another user's ID as created_by, effectively granting that user (or denying themselves) admin rights over the new team.
Proposed fix
CREATE POLICY "Allow team creation"
ON teams FOR INSERT
- -- Restrict team creation to authenticated users
- WITH CHECK (auth.role() = 'authenticated');
+ WITH CHECK (
+ auth.role() = 'authenticated'
+ AND created_by = auth.uid()
+ );📝 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.
| CREATE POLICY "Allow team creation" | |
| ON teams FOR INSERT | |
| WITH CHECK (TRUE); | |
| -- Restrict team creation to authenticated users | |
| WITH CHECK (auth.role() = 'authenticated'); | |
| CREATE POLICY "Allow team creation" | |
| ON teams FOR INSERT | |
| WITH CHECK ( | |
| auth.role() = 'authenticated' | |
| AND created_by = auth.uid() | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@sqls/02_user_auth_policies.sql` around lines 35 - 38, The "Allow team
creation" INSERT policy on teams currently only checks auth.role(); update its
WITH CHECK condition to also require created_by = auth.uid() so creators cannot
set another user's id. Locate the CREATE POLICY "Allow team creation" ON teams
FOR INSERT and modify the WITH CHECK to include created_by = auth.uid()
alongside auth.role() = 'authenticated' to enforce ownership on insert.
| // Initialize Supabase Client | ||
| const supabase = createClient(SUPABASE_URL!, SUPABASE_SERVICE_ROLE_KEY!); | ||
| const token = authHeader.replace('Bearer ', ''); |
There was a problem hiding this comment.
Supabase client is created with unchecked env vars.
createClient at line 21 uses SUPABASE_URL! and SUPABASE_SERVICE_ROLE_KEY! via non-null assertions, but the actual existence check happens later at line 45. If either variable is unset, this will pass undefined to createClient (the ! operator only silences TypeScript, it doesn't guard at runtime), potentially producing a cryptic error or a broken client before you reach the explicit check.
Move the env var validation above the client initialization, or move the createClient call inside the try block after line 45.
Proposed fix — validate env vars early
console.log("Fetch-transcript function called with Security Layer");
+ // Validate environment configuration first
+ if (!VEXA_API_KEY || !SUPABASE_URL || !SUPABASE_SERVICE_ROLE_KEY) {
+ return new Response(
+ JSON.stringify({ error: "Server configuration missing (API Keys)" }),
+ { status: 500, headers: { "Content-Type": "application/json" } }
+ );
+ }
+
// 1. JWT Verification (Security Hardening)
const authHeader = req.headers.get('Authorization');
...
// Initialize Supabase Client
- const supabase = createClient(SUPABASE_URL!, SUPABASE_SERVICE_ROLE_KEY!);
+ const supabase = createClient(SUPABASE_URL, SUPABASE_SERVICE_ROLE_KEY);And remove the duplicate check at lines 44-50.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supabase/functions/fetch-transcript/index.ts` around lines 20 - 22, The
Supabase client is being initialized with non-null-asserted env vars
(createClient(SUPABASE_URL!, SUPABASE_SERVICE_ROLE_KEY!)) before the runtime
check that they exist; move the environment validation so SUPABASE_URL and
SUPABASE_SERVICE_ROLE_KEY are checked/throwing before calling createClient, or
alternatively defer the createClient call into the try block after the existing
check around authHeader so createClient only runs with validated values, and
remove the duplicate post-check at the later lines; update references to
createClient and the env var checks accordingly.
|
|
||
| // Extract meeting ID from Google Meet URL | ||
| // 5. External API Logic (Vexa) | ||
| const meetId = meeting_url.split('/').pop().split('?')[0]; |
There was a problem hiding this comment.
meetId can be empty if meeting_url ends with a trailing slash.
meeting_url.split('/').pop() returns "" for URLs like "https://meet.google.com/abc-def-ghi/", which would send an empty ID to the Vexa API. Consider adding a guard:
Suggested guard
const meetId = meeting_url.split('/').pop().split('?')[0];
+ if (!meetId) {
+ return new Response(
+ JSON.stringify({ error: "Could not extract meeting ID from URL" }),
+ { status: 400, headers: { "Content-Type": "application/json" } }
+ );
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supabase/functions/fetch-transcript/index.ts` at line 73, The meetId
extraction can yield an empty string when meeting_url ends with a trailing
slash; update the logic that sets meetId (currently using
meeting_url.split('/').pop().split('?')[0]) to first remove trailing slashes or
pick the last non-empty segment (e.g., split and filter(Boolean) then take last)
and then strip query params, or use the URL/pathname to safely extract the last
segment; ensure meetId is validated (non-empty) before sending to the Vexa API
and handle the error case if it remains empty.
| const { error: updateError } = await supabase | ||
| .from('meetings') | ||
| .update({ | ||
| transcription: transcript, | ||
| transcription_attempted_at: new Date().toISOString(), | ||
| transcription_error: null // Clear any previous errors on success | ||
| transcription_error: null | ||
| }) | ||
| .eq('id', meeting_id); |
There was a problem hiding this comment.
Service-role client + user-supplied meeting_id = authorization bypass.
The Supabase client is initialized with SUPABASE_SERVICE_ROLE_KEY (line 21), which bypasses all RLS policies. Since meeting_id comes directly from the request body (line 63), any authenticated user can overwrite the transcript of any meeting by supplying an arbitrary ID.
Either:
- Verify ownership — query the meeting first and confirm the authenticated
user.idis associated with it (e.g., via team membership). - Use the user's JWT to create a second, user-scoped Supabase client for the update so that RLS is enforced.
Option 1 — ownership check before update
+ // Verify the user owns this meeting
+ const { data: meeting, error: meetingError } = await supabase
+ .from('meetings')
+ .select('team_id')
+ .eq('id', meeting_id)
+ .single();
+
+ if (meetingError || !meeting) {
+ return new Response(
+ JSON.stringify({ error: 'Meeting not found' }),
+ { status: 404, headers: { "Content-Type": "application/json" } }
+ );
+ }
+
+ // Check user belongs to the meeting's team
+ const { data: userRecord } = await supabase
+ .from('users')
+ .select('team_id')
+ .eq('id', user.id)
+ .single();
+
+ if (!userRecord || userRecord.team_id !== meeting.team_id) {
+ return new Response(
+ JSON.stringify({ error: 'Forbidden' }),
+ { status: 403, headers: { "Content-Type": "application/json" } }
+ );
+ }
+
// 6. Secure Database Update
const { error: updateError } = await supabase
.from('meetings')Also applies to: 114-121
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supabase/functions/fetch-transcript/index.ts` around lines 96 - 103, The
current update uses the service-role Supabase client (supabase) with a
user-supplied meeting_id, allowing authorization bypass; fix by enforcing
ownership or using a user-scoped client: either 1) query the meetings row first
with supabase.from('meetings').select(...).eq('id', meeting_id) and verify the
requesting user's id (user.id) is part of the meeting/team before performing the
update (only proceed if ownership/membership is confirmed), or 2) build a second
Supabase client using the caller's JWT and call that client to perform the
.update(...) so RLS is applied; apply the same change to the other update block
that writes transcription_error/transcription_attempted_at.
|
Resolve the coderabbit suggestions |
2 participants
This pull request implements a multi-layer security hardening for the Ell-ena backend. It addresses the lack of authentication in the serverless Edge Functions and secures the database by replacing open Row-Level Security (RLS) policies with restricted, membership-based access.
🔧 Changes Made
Edge Function Hardening: Added JWT verification to supabase/functions/fetch-transcript/index.ts to ensure only authenticated users can trigger transcription requests.
RLS Policy Update: Refactored sqls/02_user_auth_policies.sql to replace TRUE SELECT policies with strict checks, ensuring users only see data for their assigned team_id.
Permission Cleanup: Revoked all default permissions from the anon role and restricted table access exclusively to authenticated users to prevent unauthorized data leaks.
Function Security: Implemented auth.uid() and auth.role() checks for team and user creation logic.
📷 Screenshots or Visual Changes
✅ Checklist
[x] I have read the contributing guidelines.
[x] I have added tests that prove my fix is effective or that my feature works.
[x] I have added necessary documentation (if applicable).
[x] Any dependent changes have been merged and published in downstream modules.
Summary by CodeRabbit
Release Notes