Scope bounty submissions for API key auth - #393
Conversation
Greptile SummaryThis PR fixes a data-leakage bug in
Confidence Score: 4/5The scoping fix is logically correct and closes the leakage path for both api_key and agentpass callers; the only rough edge is a missing error check on the bounty fetch that can mask DB failures as 404s. The core access-control logic is sound: creators see all submissions, everyone else is filtered to their own. The bounty fetch drops the error field, so a transient DB failure looks identical to a missing bounty — a diagnostic issue but not a security or correctness regression on the happy path. Test coverage is limited to the api_key path, leaving session and agentpass behavior unverified by tests. route.ts — the bounty fetch error handling; route.test.ts — missing session-auth coverage Important Files Changed
Sequence DiagramsequenceDiagram
participant Client
participant Route as GET /bounties/[id]/submissions
participant Auth as getAuthContext
participant DB as Supabase (service role or RLS)
Client->>Route: GET request (API key / session)
Route->>Auth: getAuthContext(request)
Auth-->>Route: "{ user, supabase }"
Route->>DB: "SELECT id, creator_id FROM bounties WHERE id = :id"
DB-->>Route: "bounty | null"
alt bounty not found
Route-->>Client: 404 Bounty not found
else bounty found
alt "user.id === bounty.creator_id"
Route->>DB: "SELECT * FROM bounty_submissions WHERE bounty_id = :id ORDER BY created_at DESC"
else user is not creator
Route->>DB: "SELECT * FROM bounty_submissions WHERE bounty_id = :id AND submitter_id = :userId ORDER BY created_at DESC"
end
DB-->>Route: "{ data, error }"
Route-->>Client: "200 { data }"
end
Reviews (1): Last reviewed commit: "Scope bounty submissions for API keys" | Re-trigger Greptile |
| const { data: bounty } = await (supabase as any) | ||
| .from("bounties") | ||
| .select("id, creator_id") | ||
| .eq("id", id) | ||
| .single(); | ||
|
|
||
| if (!bounty) { | ||
| return NextResponse.json({ error: "Bounty not found" }, { status: 404 }); | ||
| } |
There was a problem hiding this comment.
Bounty fetch error silently collapsed to 404
The error field from the .single() call is never destructured or inspected. Supabase returns { data: null, error: <PgError> } for both a legitimate "row not found" (PGRST116) and genuine database errors. Only the !bounty (null data) branch is checked, so any real DB error at this step returns a misleading 404 Bounty not found response to the caller instead of a 500.
| vi.clearAllMocks(); | ||
| mockGetAuthContext.mockResolvedValue({ | ||
| user: { id: "user-1", authMethod: "api_key" }, | ||
| supabase: supabaseClient, | ||
| } as never); | ||
| }); | ||
|
|
There was a problem hiding this comment.
Test coverage limited to
api_key auth
Both test cases fix authMethod to "api_key", which exercises only the service-role (RLS-bypassed) code path. The "session" path (session-scoped client, RLS active) is untested, so regressions in that path — e.g. a creator losing access to all submissions because RLS policies diverge from the new app-level logic — would not be caught. Adding a describe block for session auth with both creator and non-creator sub-cases would complete coverage.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Fixes bounty submission visibility under API-key auth by explicitly loading the bounty creator and filtering non-creator requests by submitter_id, avoiding service-role/RLS bypass leakage.
Validation: