feat: add account_id param to GET /api/sandboxes for Org API keys#207
Conversation
- Add buildGetSandboxesParams.ts for consistent auth/access handling - Update validateGetSandboxesRequest to support account_id query param - Add 403 response for unauthorized account_id usage - Add comprehensive tests for all scenarios Co-Authored-By: Claude Opus 4.5 <[email protected]>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughA new utility module extracts sandbox query parameter building logic from validation code into a reusable function that handles authentication context, account access validation, and org membership checks to construct appropriate SelectAccountSandboxes parameters. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ❌ 1❌ Failed checks (1 warning)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
- Move access control check from buildGetSandboxesParams to validateGetSandboxesRequest - buildGetSandboxesParams now assumes access has been validated by caller - Update tests to reflect the new responsibility separation Co-Authored-By: Claude Opus 4.5 <[email protected]>
The refactor in 32523bf moved the access check out of the builder and into validateGetSandboxesRequest, breaking the established pattern used by buildGetChatsParams and buildGetPulsesParams. This left the builder as a security-unaware function that blindly accepted any target_account_id. Align with the correct pattern: canAccessAccount lives inside the builder so any caller gets defense-in-depth access checks. Co-Authored-By: Claude Opus 4.5 <[email protected]>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@lib/sandbox/buildGetSandboxesParams.ts`:
- Around line 58-63: When org_id is provided in buildGetSandboxesParams, the
code currently calls getAccountOrganizations and returns accountIds even if that
call failed (empty array), which lets selectAccountSandboxes drop all filters;
fix by detecting when getAccountOrganizations returns an empty member list:
after const memberAccountIds = orgMembers.map(...), if memberAccountIds.length
=== 0 return { params: null, error: new Error('no organization members found') }
(or appropriate error shape), and also include orgId: org_id in the returned
params object (return { params: { accountIds: memberAccountIds, sandboxId:
sandbox_id, orgId: org_id }, error: null }) so selectAccountSandboxes can still
apply an org-level filter as a defensive safeguard.
🧹 Nitpick comments (2)
lib/sandbox/validateGetSandboxesRequest.ts (2)
14-24: Consider renaming to validateGetSandboxesQuery.ts for consistency.
This matches the validate*.ts naming convention and improves discoverability.
9-12: Export the inferred query type and align filename with query parameter validation.The schema validates query parameters (sandbox_id, account_id from searchParams), so it should follow the naming and type export patterns used throughout the codebase:
♻️ Suggested changes
const getSandboxesQuerySchema = z.object({ sandbox_id: z.string().optional(), account_id: z.string().uuid("account_id must be a valid UUID").optional(), }); + +export type GetSandboxesQuery = z.infer<typeof getSandboxesQuerySchema>;Also rename the file from
validateGetSandboxesRequest.tstovalidateGetSandboxesQuery.tsto match the guideline pattern for query validation functions.
| if (org_id) { | ||
| // Org key: fetch all member account IDs for this organization | ||
| const orgMembers = await getAccountOrganizations({ organizationId: org_id }); | ||
| const memberAccountIds = orgMembers.map(member => member.account_id); | ||
| return { params: { accountIds: memberAccountIds, sandboxId: sandbox_id }, error: null }; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd 'buildGetSandboxesParams\.ts$' lib -t fRepository: Recoupable-com/Recoup-API
Length of output: 107
🏁 Script executed:
fd 'getAccountOrganizations' lib -t f | head -20Repository: Recoupable-com/Recoup-API
Length of output: 133
🏁 Script executed:
fd 'selectAccountSandboxes' lib -t f | head -20Repository: Recoupable-com/Recoup-API
Length of output: 197
🏁 Script executed:
wc -l lib/sandbox/buildGetSandboxesParams.tsRepository: Recoupable-com/Recoup-API
Length of output: 110
🏁 Script executed:
cat -n lib/sandbox/buildGetSandboxesParams.tsRepository: Recoupable-com/Recoup-API
Length of output: 3280
🏁 Script executed:
wc -l lib/supabase/account_organization_ids/getAccountOrganizations.tsRepository: Recoupable-com/Recoup-API
Length of output: 136
🏁 Script executed:
cat -n lib/supabase/account_organization_ids/getAccountOrganizations.tsRepository: Recoupable-com/Recoup-API
Length of output: 2175
🏁 Script executed:
wc -l lib/supabase/account_sandboxes/selectAccountSandboxes.tsRepository: Recoupable-com/Recoup-API
Length of output: 128
🏁 Script executed:
cat -n lib/supabase/account_sandboxes/selectAccountSandboxes.tsRepository: Recoupable-com/Recoup-API
Length of output: 2089
🏁 Script executed:
cat -n lib/supabase/account_sandboxes/__tests__/selectAccountSandboxes.test.tsRepository: Recoupable-com/Recoup-API
Length of output: 5393
🏁 Script executed:
# Verify that selectAccountSandboxes does NOT apply any filter when both accountIds and orgId are absent/empty
cat -n lib/supabase/account_sandboxes/selectAccountSandboxes.ts | sed -n '24,48p'Repository: Recoupable-com/Recoup-API
Length of output: 995
Fail-open vulnerability: empty org member lookup exposes all sandboxes.
getAccountOrganizations returns [] on error (line 58 of getAccountOrganizations.ts). When this happens, buildGetSandboxesParams passes accountIds: [] to selectAccountSandboxes without orgId. In selectAccountSandboxes, the condition accountIds && accountIds.length > 0 fails, and since orgId is not passed, the fallback else if (orgId) also fails. This leaves the query completely unfiltered—all sandboxes are returned to the org key.
Guard against this by checking memberAccountIds.length === 0 and returning an error. Additionally, pass orgId to params so that selectAccountSandboxes can apply an organization filter as a defensive safeguard:
Suggested fail-closed guard
if (org_id) {
// Org key: fetch all member account IDs for this organization
const orgMembers = await getAccountOrganizations({ organizationId: org_id });
const memberAccountIds = orgMembers.map(member => member.account_id);
+ if (memberAccountIds.length === 0) {
+ return { params: null, error: "No organization members found for this org" };
+ }
- return { params: { accountIds: memberAccountIds, sandboxId: sandbox_id }, error: null };
+ return {
+ params: { accountIds: memberAccountIds, orgId: org_id, sandboxId: sandbox_id },
+ error: null,
+ };
}📝 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.
| if (org_id) { | |
| // Org key: fetch all member account IDs for this organization | |
| const orgMembers = await getAccountOrganizations({ organizationId: org_id }); | |
| const memberAccountIds = orgMembers.map(member => member.account_id); | |
| return { params: { accountIds: memberAccountIds, sandboxId: sandbox_id }, error: null }; | |
| } | |
| if (org_id) { | |
| // Org key: fetch all member account IDs for this organization | |
| const orgMembers = await getAccountOrganizations({ organizationId: org_id }); | |
| const memberAccountIds = orgMembers.map(member => member.account_id); | |
| if (memberAccountIds.length === 0) { | |
| return { params: null, error: "No organization members found for this org" }; | |
| } | |
| return { | |
| params: { accountIds: memberAccountIds, orgId: org_id, sandboxId: sandbox_id }, | |
| error: null, | |
| }; | |
| } |
🤖 Prompt for AI Agents
In `@lib/sandbox/buildGetSandboxesParams.ts` around lines 58 - 63, When org_id is
provided in buildGetSandboxesParams, the code currently calls
getAccountOrganizations and returns accountIds even if that call failed (empty
array), which lets selectAccountSandboxes drop all filters; fix by detecting
when getAccountOrganizations returns an empty member list: after const
memberAccountIds = orgMembers.map(...), if memberAccountIds.length === 0 return
{ params: null, error: new Error('no organization members found') } (or
appropriate error shape), and also include orgId: org_id in the returned params
object (return { params: { accountIds: memberAccountIds, sandboxId: sandbox_id,
orgId: org_id }, error: null }) so selectAccountSandboxes can still apply an
org-level filter as a defensive safeguard.
1 participant
Summary
account_idquery parameter toGET /api/sandboxesendpoint (org API keys only)buildGetSandboxesParams.tsfor consistent auth/access handlingaccount_idusagevalidateGetSandboxesRequest.tsto use the new builder patternChanges
lib/sandbox/buildGetSandboxesParams.ts- Handles authorization logic for account_id filteringlib/sandbox/validateGetSandboxesRequest.ts- Added account_id query param supportlib/sandbox/__tests__/buildGetSandboxesParams.test.ts- 11 test caseslib/sandbox/__tests__/validateGetSandboxesRequest.test.ts- 9 test casesTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements