feat(apikeys): migrate legacy keys to v2#2279
Open
riderx wants to merge 27 commits into
Open
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughMigrates API keys to RBAC-backed role_bindings across DB, backend, middleware, frontend, and tests; removes legacy mode/limited fields, adds binding-aware helpers, and redirects old org API-keys pages to /apikeys. ChangesAPI Keys V2 RBAC migration and UI redirects
Sequence Diagram(s): Estimated code review effort: Possibly related PRs:
Suggested reviewers:
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
tests/hashed-apikey-rls.test.ts (1)
201-240:⚠️ Potential issue | 🟠 Major | ⚡ Quick winWrap key creation + binding in one transaction.
If binding fails after key insertion, this helper can leave orphaned API keys and make later assertions flaky.
Suggested fix
async function createHashedApiKey( name: string, options: ApiKeyAccessOptions = {}, ): Promise<{ id: number, key: string, key_hash: string }> { const client = await pool.connect() const plainKey = randomUUID() try { + await client.query('BEGIN') const { rows } = await client.query( `INSERT INTO public.apikeys (user_id, key, key_hash, name) VALUES ($1, NULL, encode(extensions.digest($2, 'sha256'), 'hex'), $3) RETURNING id, key_hash, rbac_id`, [RLS_TEST_USER_ID, plainKey, name], ) await bindApiKeyAccess(client, rows[0].rbac_id, RLS_TEST_USER_ID, options) + await client.query('COMMIT') return { id: Number(rows[0].id), key: plainKey, key_hash: rows[0].key_hash } } + catch (error) { + await client.query('ROLLBACK') + throw error + } finally { client.release() } }🤖 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 `@tests/hashed-apikey-rls.test.ts` around lines 201 - 240, The helper functions createHashedApiKey and createPlainApiKey currently insert the apikey row and then call bindApiKeyAccess separately, which can leave orphaned rows if binding fails; update both functions to run the insert and bindApiKeyAccess inside a single DB transaction on the same client by issuing BEGIN before the insert, COMMIT after successful bindApiKeyAccess, and ROLLBACK on any error (then rethrow), ensuring the client.release() still runs in the finally block and the original returned values (id, key, key_hash) are preserved.supabase/functions/_backend/public/apikey/post.ts (1)
84-96:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFail fast on malformed app-scoped bindings.
scope_type === 'app'is not required to carry anapp_idhere, so malformed payloads only fail later inside the transaction path and come back asbinding_failedinstead of a direct 400.🧩 Suggested validation
if (typeof binding.org_id !== 'string' || !binding.org_id) { throw simpleError('invalid_bindings', 'Each binding must have an org_id') } + if (binding.scope_type === 'app' && (typeof binding.app_id !== 'string' || !binding.app_id)) { + throw simpleError('invalid_bindings', 'Each app binding must have an app_id') + }🤖 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/functions/_backend/public/apikey/post.ts` around lines 84 - 96, The loop over bindings currently validates scope_type but doesn't require an app_id for app-scoped entries, which lets malformed app bindings fail later; inside the existing bindings loop (the for (const binding of bindings) block) add a guard after checking binding.scope_type that if binding.scope_type === 'app' then verify typeof binding.app_id === 'string' && binding.app_id is non-empty and if not throw simpleError('invalid_bindings','Each app-scoped binding must have an app_id'); this ensures malformed app-scoped bindings fail fast.supabase/functions/_backend/utils/rbac.ts (1)
411-447:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't make
apikeyStringeffectively mandatory without a fallback.
checkPermissionPg()still exposesapikeyStringas optional, but this new branch returnsfalsewhenever it is omitted for RBAC-backed API-key auth. That breaks the helper’s own contract and will turn valid API-key requests into false denies for callers that rely on the existing signature.Suggested fix
const { orgId = null, appId = null, channelId = null } = scope const auth = c.get('auth') + const effectiveApikeyString = apikeyString ?? auth?.apikey?.key ?? c.get('capgkey') ?? null cloudlog({ requestId: c.get('requestId'), message: 'checkPermissionPg: checking', permission, scope, userId, - hasApikey: !!apikeyString, + hasApikey: !!effectiveApikeyString, }) try { if (auth?.authType === 'apikey' && auth.apikey?.rbac_id) { - if (!apikeyString) + if (!effectiveApikeyString) return false const rbacOnlyResult = await drizzleClient.execute( sql`SELECT public.rbac_check_permission_direct( ${permission}, ${userId}::uuid, ${orgId}::uuid, ${appId}, ${channelId}::bigint, - ${apikeyString} + ${effectiveApikeyString} ) AS allowed`, )🤖 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/functions/_backend/utils/rbac.ts` around lines 411 - 447, The new branch in checkPermissionPg that handles auth.authType === 'apikey' && auth.apikey?.rbac_id should not unconditionally return false when apikeyString is missing; instead, if apikeyString is absent, do not short-circuit — either derive it from the existing auth object (e.g., auth.apikey.token / auth.apikey.value) before calling drizzleClient.execute(sql`public.rbac_check_permission_direct(...)`) or skip the rbac-only branch so the function falls back to the normal permission checks. Update the block around auth.authType, auth.apikey?.rbac_id and the apikeyString check so missing apikeyString is handled by attempting to obtain it from auth.apikey or by continuing execution (not returning false), then call drizzleClient.execute and use the rbacOnlyAllowed logic as before.
🧹 Nitpick comments (4)
tests/apikey-atomic-bindings.test.ts (1)
250-250: 💤 Low valueTest name should begin with lowercase.
ESLint requires test descriptions to start with a lowercase letter. Change "V2 key can authenticate" to "v2 key can authenticate".
🔧 Proposed fix
- it('V2 key can authenticate', async () => { + it('v2 key can authenticate', async () => {🤖 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 `@tests/apikey-atomic-bindings.test.ts` at line 250, The test description string in the it(...) call currently starts with an uppercase "V2 key can authenticate"; change that string to start with a lowercase 'v' (i.e., "v2 key can authenticate") so ESLint passes; update the it(...) invocation that contains "V2 key can authenticate" accordingly and ensure any snapshots or test references that rely on the exact description are updated to match the new lowercase description.tests/organization-api.test.ts (1)
414-488: ⚡ Quick winAlways clean up
setupKeyin afinallyblock.
setupKeyis deleted only on the happy path. If an assertion fails earlier, the key remains and can contaminate later tests.Suggested fix
it.concurrent('rejects DELETE /organization/members outside bound organization scope without deleting role bindings', async () => { const setupKey = await createDirectApiKeyWithBindings({ userId: USER_ID, key: randomUUID(), name: `Scoped target setup ${randomUUID()}`, orgId: targetOrgId, roleName: 'org_super_admin', }) if (!setupKey?.key || typeof setupKey.id !== 'number') throw new Error('Failed to seed scoped setup API key') - const setupHeaders = { - 'Content-Type': 'application/json', - 'capgkey': setupKey.key, - } + try { + const setupHeaders = { + 'Content-Type': 'application/json', + 'capgkey': setupKey.key, + } - const addMemberResponse = await fetch(`${BASE_URL}/organization/members`, { - headers: setupHeaders, - method: 'POST', - body: JSON.stringify({ - orgId: targetOrgId, - email: USER_ADMIN_EMAIL, - invite_type: 'read', - }), - }) - expect(addMemberResponse.status).toBe(200) + const addMemberResponse = await fetch(`${BASE_URL}/organization/members`, { + headers: setupHeaders, + method: 'POST', + body: JSON.stringify({ + orgId: targetOrgId, + email: USER_ADMIN_EMAIL, + invite_type: 'read', + }), + }) + expect(addMemberResponse.status).toBe(200) - // ... existing assertions ... - - await getSupabaseClient().from('org_users').delete().eq('org_id', targetOrgId).eq('user_id', userData!.id) - await getSupabaseClient().from('apikeys').delete().eq('id', setupKey.id) + // ... existing assertions ... + await getSupabaseClient().from('org_users').delete().eq('org_id', targetOrgId).eq('user_id', userData!.id) + } + finally { + await getSupabaseClient().from('apikeys').delete().eq('id', setupKey.id) + } })🤖 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 `@tests/organization-api.test.ts` around lines 414 - 488, The test "rejects DELETE /organization/members outside bound organization scope without deleting role bindings" currently only deletes setupKey and seeded org_users on the happy path; wrap the teardown in a finally block so cleanup always runs: after creating setupKey via createDirectApiKeyWithBindings and using setupKey.id later, ensure a finally block deletes the API key (from 'apikeys') if setupKey?.id exists and removes the seeded org_users row (and any other seeded rows) to avoid leaking state between tests; locate references to setupKey, createDirectApiKeyWithBindings, and the explicit getSupabaseClient().from('org_users').delete()/from('apikeys').delete() calls to move them into the finally.tests/hashed-apikey-rls.test.ts (1)
1234-1260: ⚡ Quick winRemove
.then()chaining afterawaitin RLS assertions.Mix of async/await and promise chaining reduces readability. Separate the await from the property access to maintain consistent async/await style.
Suggested fix
- const webhookRows = await execWithRoleClaims( + const webhookResult = await execWithRoleClaims( 'SELECT id FROM public.webhooks WHERE id = $1', { role: 'authenticated', claims: { sub: USER_ID_RLS, role: 'authenticated', aud: 'authenticated', }, headers: { capgkey: unauthorizedKey.key }, params: [webhookId], }, - ).then(result => result.rows) + ) + const webhookRows = webhookResult.rows - const deliveryRows = await execWithRoleClaims( + const deliveryResult = await execWithRoleClaims( 'SELECT id FROM public.webhook_deliveries WHERE id = $1', { role: 'authenticated', claims: { sub: USER_ID_RLS, role: 'authenticated', aud: 'authenticated', }, headers: { capgkey: unauthorizedKey.key }, params: [deliveryId], }, - ).then(result => result.rows) + ) + const deliveryRows = deliveryResult.rows🤖 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 `@tests/hashed-apikey-rls.test.ts` around lines 1234 - 1260, The two RLS query calls use await combined with .then() chaining; update the calls to use await directly and then read the rows property on the resolved result to improve readability: call execWithRoleClaims(...) with await (in the statements assigning webhookRows and deliveryRows), then set webhookRows = result.rows and deliveryRows = result.rows respectively. Keep the same arguments (role/claims/headers/params and identifiers like USER_ID_RLS, unauthorizedKey.key, webhookId, deliveryId) and the function name execWithRoleClaims to locate the lines to change.tests/statistics.test.ts (1)
338-368: ⚡ Quick winRun the newly added nested-key tests as concurrent.
These two tests are independent and can use
it.concurrent(...)to match the test guideline for intra-file parallelism.Suggested fix
- it('rejects nested API key creation from API key auth for org bindings', async () => { + it.concurrent('rejects nested API key creation from API key auth for org bindings', async () => { @@ - it('rejects nested API key creation from API key auth for app bindings', async () => { + it.concurrent('rejects nested API key creation from API key auth for app bindings', async () => {Per coding guidelines: "Design all tests for parallel execution across files; use it.concurrent() instead of it() to run tests in parallel within the same file for faster CI/CD".
🤖 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 `@tests/statistics.test.ts` around lines 338 - 368, The two tests "'rejects nested API key creation from API key auth for org bindings'" and "'rejects nested API key creation from API key auth for app bindings'" should be converted to run concurrently: replace the two it(...) declarations with it.concurrent(...) while keeping the same async test bodies (the fetch calls using headersStats and appApiKeyBindings(APPNAME, 'app_reader') and the same assertions), ensuring test names and logic remain unchanged so they run in parallel within the file.
🤖 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 `@src/pages/admin/dashboard/replication.vue`:
- Around line 111-112: The thrown Error message in the session check inside
replication.vue is out-of-date; update the message in the conditional that
checks "if (!session?.access_token)" (the block that currently throws "No
session available and replication secret is not configured") to accurately
reflect the current behavior—e.g., throw a clear error like "No session access
token available" or "No session available" so it no longer references a
replication secret.
In `@src/services/apikeys.ts`:
- Around line 101-112: The current lookup in the apikeys helper only filters by
org_id and treats app-scoped keys with an org_member binding as valid for the
whole org; change the logic that queries and validates role_bindings so we only
accept a key if it has either (a) a role_binding for the specific app (match on
app_id or whatever field identifies the app) or (b) an org-level binding that
conveys admin-capable permissions (e.g., role in ['org_admin','owner'] or
equivalent). Concretely: after fetching bindings for principal_ids in rbacIds,
filter those bindings to require either a binding with the requested app
identifier or a binding whose role grants org-wide admin rights before building
scopedKeyIds and picking from liveKeys (variables to update: bindings,
scopedKeyIds, and the final liveKeys.find(...)); return null if no such
app-scoped or org-admin-capable binding exists.
- Around line 18-30: The code currently looks up an app by options.appId and
then quietly proceeds even if options.orgId conflicts with the app's owner_org;
change the logic in the block that queries supabase (the code that sets appUuid
= app?.id and orgId ||= app?.owner_org) to validate ownership: after the
.single() call, if options.orgId is provided and does not equal app.owner_org,
throw a descriptive error (e.g., "appId does not belong to orgId"); otherwise
set appUuid = app.id and set orgId to app.owner_org when orgId is not already
set. Ensure the thrown error halts processing so we fail fast on appId/orgId
mismatches.
In `@supabase/functions/_backend/public/apikey/post.ts`:
- Around line 73-75: The handler currently only rejects callers with authType
'apikey' but later assumes auth.userId (used when inserting apikeys.user_id and
calling createRoleBindingForPrincipal), so add an explicit guard right after
middlewareV2(['all']) that checks c.get('auth')?.authType === 'user' (or
otherwise confirms the caller is a JWT session) and throw a simple auth error
for any non-user types (API secret, external API key, etc.) to prevent undefined
auth.userId from being used; update the existing apikey branch to coexist with
this broader guard so createRoleBindingForPrincipal and apikeys.user_id always
receive a valid auth.userId.
In `@supabase/functions/_backend/utils/hono_middleware.ts`:
- Around line 401-424: The is_limited predicate currently evaluates true when
active_bindings is empty, allowing binding-less subkeys to pass; modify the
expression that defines is_limited so it first requires that active_bindings
contains at least one row (e.g., prepend EXISTS (SELECT 1 FROM active_bindings)
AND ( ...existing OR/NOT EXISTS logic... )) so that when there are zero
active_bindings is_limited is false and validateSubkeyLimits() will reject the
subkey; update the SELECT that produces is_limited accordingly (refer to the
is_limited expression in this diff).
In `@supabase/functions/_backend/utils/supabase.ts`:
- Around line 845-850: The current_orgs CTE and the pre-check in createApiKey()
incorrectly source membership only from public.org_users; change both places to
use the V2 membership source (the same canonical membership query used elsewhere
in this module for RBAC/group/direct bindings) so current_orgs includes orgs
from direct RBAC bindings and group membership as well as org_users; update the
CTE definition named current_orgs and the earlier existence check to reuse that
V2 membership query/logic (apply the same replacement at the second occurrence
referenced in the diff) so keys are bound to every org the user actually belongs
to.
- Around line 831-843: The idempotency check in createApiKey is racy because it
reads the count with getPgClient outside a transactional lock and may allow
concurrent calls to both see zero and create duplicates; fix by making the
“create defaults once” operation atomic—either acquire a row lock on the user
(SELECT ... FOR UPDATE within a transaction in createApiKey) and then
check/insert inside that same transaction, or add a uniqueness constraint on
(user_id, name) and perform the insert(s) with ON CONFLICT DO NOTHING inside a
transaction so duplicate inserts are safely ignored (apply the same change to
the later block referenced around lines 857-943). Ensure you update references
in createApiKey and any helper that performs the insert so all existence checks
and inserts happen inside the same transaction/lock or use the INSERT ... ON
CONFLICT guard.
In `@supabase/migrations/20260517085536_migrate_apikeys_to_v2.sql`:
- Around line 1088-1090: The trigger currently returns immediately on INSERT (IF
TG_OP = 'INSERT' THEN RETURN NEW;), which bypasses org expiration enforcement;
modify the trigger function so that INSERT paths execute the same expiration
validation/enforcement logic as UPDATE/other paths instead of returning
early—either remove the early RETURN NEW or call the expiration enforcement
routine before returning, ensuring TG_OP = 'INSERT' flows through the existing
expiration checks and uses the same variables/handlers as the other branches.
- Around line 360-370: The function is_allowed_capgkey currently ignores the
keymode parameter (PERFORM keymode) and therefore only checks existence/expiry;
update is_allowed_capgkey(apikey, keymode[]) to enforce mode gating by verifying
the fetched api_key's mode is contained in the provided keymode array (e.g.,
check api_key.mode ∈ keymode or use ANY/array match against keymode) and return
false if not matched, keeping the existing existence and expiry checks
(references: function is_allowed_capgkey, parameter keymode, variable api_key,
helper public.find_apikey_by_value and public.is_apikey_expired).
- Around line 534-572: The API-key branch returns v_allowed before enforcing org
2FA; after v_effective_user_id := v_api_key.user_id and before calling
public.rbac_has_permission (and before any RETURN v_allowed), add a check that
if the org identified by v_effective_org_id has enforcing_2fa = true then verify
the user (v_effective_user_id) has 2FA enabled using your existing 2FA check
function, and if not, RETURN false; apply the same insertion to the other
direct-permission helper where the API-key branch returns early (the block
around v_api_key handling referenced in the comment).
In `@tests/apikeys-expiration.test.ts`:
- Line 6: The named imports in the test file are out of alphabetical order and
trigger the perfectionist/sort-named-imports lint rule; reorder the specifiers
in the import statement so they are alphabetized (for example place
appApiKeyBindings, BASE_URL, createDirectApiKeyWithBindings, executeSQL,
fetchWithRetry, getAuthHeadersForCredentials, getSupabaseClient,
normalizeLocalhostUrl, orgApiKeyBindings, resetAndSeedAppData, resetAppData,
TEST_EMAIL, USER_EMAIL_APIKEY_EXPIRATION, USER_ID_APIKEY_EXPIRATION,
USER_PASSWORD in proper sorted order) to satisfy the rule while keeping the same
named imports and no other changes.
In `@tests/app.test.ts`:
- Around line 203-218: The test "should not delete app with subkey if rights are
read-only" currently only asserts deleteApp.status is 400; update it to parse
the response body (e.g., await deleteApp.json() or .text()) and assert the body
contains the authorization error for insufficient permissions—e.g., check a
specific error code or message such as error.code === 'insufficient_permissions'
or error.message includes 'permission'—so that createDirectApiKeyWithBindings +
the DELETE fetch is validated as an auth failure rather than any generic bad
request.
In `@tests/build-job-scope.test.ts`:
- Line 3: The import specifiers (BASE_URL, USER_ID_2,
createDirectApiKeyWithBindings, getSupabaseClient, resetAndSeedAppData,
resetAppData) are not alphabetically sorted and trigger
perfectionist/sort-named-imports; reorder the named imports so they are sorted
(e.g., BASE_URL, USER_ID_2, createDirectApiKeyWithBindings, getSupabaseClient,
resetAppData, resetAndSeedAppData or according to your project's specifier sort
rules) in the import statement that contains those symbols.
In `@tests/hashed-apikey-rls.test.ts`:
- Line 13: The inline type import "type PoolClient" should be split to satisfy
the import/consistent-type-specifier-style rule: change the import of pg so that
the runtime symbol Pool is imported separately from the type-only symbol
PoolClient (e.g., keep import { Pool } from 'pg' and add a separate import type
{ PoolClient } from 'pg'), updating the import statement that currently
references both Pool and type PoolClient.
In `@tests/rbac-permissions.test.ts`:
- Around line 52-67: The INSERT ... SELECT may insert zero rows silently; after
the query that inserts into public.role_bindings (the call using variables
apiKeyRbacId, orgId, USER_ID, roleName), check the result's rowCount (or
equivalent returned by your query helper) and throw an error or assert if
rowCount === 0 so the test fails fast when no role binding was created; include
context in the error mentioning the roleName and apiKeyRbacId to aid debugging.
In `@tests/statistics.test.ts`:
- Line 3: Reorder the named imports in the import statement so that
all-uppercase identifiers appear before camelCase ones to satisfy the
perfectionist/sort-named-imports rule; for example, place APP_NAME_STATS,
BASE_URL, ORG_ID_STATS, USER_ID_STATS first, followed by appApiKeyBindings,
createDirectApiKeyWithBindings, getAuthHeadersForCredentials, getSupabaseClient,
and headersStats, updating the import line that currently lists
appApiKeyBindings, APP_NAME_STATS, BASE_URL, createDirectApiKeyWithBindings,
getAuthHeadersForCredentials, getSupabaseClient, headersStats, ORG_ID_STATS,
USER_ID_STATS accordingly.
---
Outside diff comments:
In `@supabase/functions/_backend/public/apikey/post.ts`:
- Around line 84-96: The loop over bindings currently validates scope_type but
doesn't require an app_id for app-scoped entries, which lets malformed app
bindings fail later; inside the existing bindings loop (the for (const binding
of bindings) block) add a guard after checking binding.scope_type that if
binding.scope_type === 'app' then verify typeof binding.app_id === 'string' &&
binding.app_id is non-empty and if not throw
simpleError('invalid_bindings','Each app-scoped binding must have an app_id');
this ensures malformed app-scoped bindings fail fast.
In `@supabase/functions/_backend/utils/rbac.ts`:
- Around line 411-447: The new branch in checkPermissionPg that handles
auth.authType === 'apikey' && auth.apikey?.rbac_id should not unconditionally
return false when apikeyString is missing; instead, if apikeyString is absent,
do not short-circuit — either derive it from the existing auth object (e.g.,
auth.apikey.token / auth.apikey.value) before calling
drizzleClient.execute(sql`public.rbac_check_permission_direct(...)`) or skip the
rbac-only branch so the function falls back to the normal permission checks.
Update the block around auth.authType, auth.apikey?.rbac_id and the apikeyString
check so missing apikeyString is handled by attempting to obtain it from
auth.apikey or by continuing execution (not returning false), then call
drizzleClient.execute and use the rbacOnlyAllowed logic as before.
In `@tests/hashed-apikey-rls.test.ts`:
- Around line 201-240: The helper functions createHashedApiKey and
createPlainApiKey currently insert the apikey row and then call bindApiKeyAccess
separately, which can leave orphaned rows if binding fails; update both
functions to run the insert and bindApiKeyAccess inside a single DB transaction
on the same client by issuing BEGIN before the insert, COMMIT after successful
bindApiKeyAccess, and ROLLBACK on any error (then rethrow), ensuring the
client.release() still runs in the finally block and the original returned
values (id, key, key_hash) are preserved.
---
Nitpick comments:
In `@tests/apikey-atomic-bindings.test.ts`:
- Line 250: The test description string in the it(...) call currently starts
with an uppercase "V2 key can authenticate"; change that string to start with a
lowercase 'v' (i.e., "v2 key can authenticate") so ESLint passes; update the
it(...) invocation that contains "V2 key can authenticate" accordingly and
ensure any snapshots or test references that rely on the exact description are
updated to match the new lowercase description.
In `@tests/hashed-apikey-rls.test.ts`:
- Around line 1234-1260: The two RLS query calls use await combined with .then()
chaining; update the calls to use await directly and then read the rows property
on the resolved result to improve readability: call execWithRoleClaims(...) with
await (in the statements assigning webhookRows and deliveryRows), then set
webhookRows = result.rows and deliveryRows = result.rows respectively. Keep the
same arguments (role/claims/headers/params and identifiers like USER_ID_RLS,
unauthorizedKey.key, webhookId, deliveryId) and the function name
execWithRoleClaims to locate the lines to change.
In `@tests/organization-api.test.ts`:
- Around line 414-488: The test "rejects DELETE /organization/members outside
bound organization scope without deleting role bindings" currently only deletes
setupKey and seeded org_users on the happy path; wrap the teardown in a finally
block so cleanup always runs: after creating setupKey via
createDirectApiKeyWithBindings and using setupKey.id later, ensure a finally
block deletes the API key (from 'apikeys') if setupKey?.id exists and removes
the seeded org_users row (and any other seeded rows) to avoid leaking state
between tests; locate references to setupKey, createDirectApiKeyWithBindings,
and the explicit
getSupabaseClient().from('org_users').delete()/from('apikeys').delete() calls to
move them into the finally.
In `@tests/statistics.test.ts`:
- Around line 338-368: The two tests "'rejects nested API key creation from API
key auth for org bindings'" and "'rejects nested API key creation from API key
auth for app bindings'" should be converted to run concurrently: replace the two
it(...) declarations with it.concurrent(...) while keeping the same async test
bodies (the fetch calls using headersStats and appApiKeyBindings(APPNAME,
'app_reader') and the same assertions), ensuring test names and logic remain
unchanged so they run in parallel within the file.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7868bd3c-f6cd-448a-88d5-e3c724ca5a3e
📒 Files selected for processing (54)
src/components.d.tssrc/components/Sidebar.vuesrc/components/dashboard/AppOnboardingFlow.vuesrc/components/dashboard/StepsApp.vuesrc/components/dashboard/StepsBuild.vuesrc/components/dashboard/StepsBundle.vuesrc/components/organization/ApiKeyRbacManager.vuesrc/constants/organizationTabs.tssrc/pages/ApiKeys.vuesrc/pages/admin/dashboard/replication.vuesrc/pages/settings/organization/ApiKeys.[id].vuesrc/pages/settings/organization/ApiKeys.vuesrc/services/apikeys.tssrc/types/supabase.types.tssupabase/functions/_backend/private/delete_failed_version.tssupabase/functions/_backend/private/role_bindings.tssupabase/functions/_backend/private/upload_link.tssupabase/functions/_backend/public/apikey/delete.tssupabase/functions/_backend/public/apikey/get.tssupabase/functions/_backend/public/apikey/post.tssupabase/functions/_backend/public/apikey/put.tssupabase/functions/_backend/public/apikey/scope.tssupabase/functions/_backend/public/app/get.tssupabase/functions/_backend/public/device/index.tssupabase/functions/_backend/public/organization/post.tssupabase/functions/_backend/public/statistics/index.tssupabase/functions/_backend/public/webhooks/index.tssupabase/functions/_backend/utils/hono_middleware.tssupabase/functions/_backend/utils/postgres_schema.tssupabase/functions/_backend/utils/rbac.tssupabase/functions/_backend/utils/supabase.tssupabase/functions/_backend/utils/supabase.types.tssupabase/migrations/20260517085536_migrate_apikeys_to_v2.sqlsupabase/seed.sqlsupabase/tests/53_test_apikey_creation_security.sqltests/apikey-atomic-bindings.test.tstests/apikeys-expiration.test.tstests/apikeys.test.tstests/app-error-cases.test.tstests/app.test.tstests/audit-logs.test.tstests/build-job-scope.test.tstests/bundle.test.tstests/cli-sdk-utils.tstests/cli.test.tstests/enforce-encrypted-bundles.test.tstests/files-security.test.tstests/hashed-apikey-rls.test.tstests/organization-api.test.tstests/rbac-permissions.test.tstests/statistics.test.tstests/test-utils.tstests/webhooks-apikey-policy.test.tstests/webhooks.test.ts
💤 Files with no reviewable changes (7)
- src/components/organization/ApiKeyRbacManager.vue
- supabase/functions/_backend/private/delete_failed_version.ts
- supabase/functions/_backend/public/device/index.ts
- src/constants/organizationTabs.ts
- src/components.d.ts
- supabase/functions/_backend/private/upload_link.ts
- supabase/functions/_backend/utils/postgres_schema.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
supabase/tests/20_test_org_management_functions.sql (1)
125-181:⚠️ Potential issue | 🟠 Major | ⚡ Quick winThese V2 API-key checks are effectively no-ops.
Line 131, Line 144, Line 162, and Line 179 all assert
count(*) >= 0, which is always true. That means Tests 3-5 will still pass ifget_orgs_v6()returns no rows at all. Test 5 also no longer exercises the “without legacy scope columns” case; it just repeats the same happy-path call.Suggested assertion shape
SELECT ok( - ( - SELECT count(*) - FROM - get_orgs_v6() - ) >= 0, + EXISTS ( + SELECT 1 + FROM get_orgs_v6() + ), 'get_orgs_v6 API key test - works with org-bound V2 API key' ); SELECT ok( - ( - SELECT count(*) - FROM - get_orgs_v6() - WHERE - gid = '22dbad8a-b885-4309-9b3b-a09f8460fb6d' - ) >= 0, + EXISTS ( + SELECT 1 + FROM get_orgs_v6() + WHERE gid = '22dbad8a-b885-4309-9b3b-a09f8460fb6d' + ), 'get_orgs_v6 API key test - V2 API key returns bound organizations' );🤖 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/tests/20_test_org_management_functions.sql` around lines 125 - 181, Replace the no-op assertions that use "count(*) >= 0" with real checks: in the general API-key tests that call get_orgs_v6() change to assert count(*) > 0 (or check specific returned gid) so the call must return rows; for the bound-organization test that filters WHERE gid = '22dbad8a-b885-4309-9b3b-a09f8460fb6d' assert count(*) = 1 (or > 0) instead of >= 0; for the "without legacy scope columns" case, ensure you set_config the API key that represents the no-legacy-scope key (the same capgkey value used in the set_config calls) and assert get_orgs_v6() returns rows (count(*) > 0) and/or assert expected columns are present/absent to exercise the legacy-scope difference rather than repeating the same happy-path check.
🤖 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/tests/00-supabase_test_helpers.sql`:
- Around line 479-487: Upsert currently overwrites key and key_hash with NULL
when a later call supplies only one credential form; change the ON CONFLICT DO
UPDATE to preserve existing key material by only replacing key and key_hash when
the incoming value is non-NULL (e.g., use COALESCE(EXCLUDED.key, apikeys.key)
and COALESCE(EXCLUDED.key_hash, apikeys.key_hash) or equivalent), while still
updating other fields (user_id, name, expires_at) as before so repeated calls
with the same p_id don’t null out existing credentials.
In `@supabase/tests/46_test_rbac_legacy_apikey_effective_user.sql`:
- Around line 42-51: The fixture currently gives the API key its own org/app
bindings via the tests.create_v2_apikey call (see 'legacy-effective-user-key',
public.rbac_role_org_super_admin(), public.rbac_role_app_admin(), and the
'com.test.legacyeffective.read' scope), so tests can pass even if
rbac_check_permission_direct(..., NULL::uuid, ..., 'legacy-effective-user-key')
stops resolving the effective user; change the seed so only the api_keys row is
created without attaching org/app roles or scopes — i.e., call
tests.create_v2_apikey for 'legacy-effective-user-key' but pass NULL (or omit)
the org role, app role and permission scope arguments so the permission must
come from tests.get_supabase_uid('legacy_apikey_effective_member')'s legacy
rights instead of the key itself.
---
Outside diff comments:
In `@supabase/tests/20_test_org_management_functions.sql`:
- Around line 125-181: Replace the no-op assertions that use "count(*) >= 0"
with real checks: in the general API-key tests that call get_orgs_v6() change to
assert count(*) > 0 (or check specific returned gid) so the call must return
rows; for the bound-organization test that filters WHERE gid =
'22dbad8a-b885-4309-9b3b-a09f8460fb6d' assert count(*) = 1 (or > 0) instead of
>= 0; for the "without legacy scope columns" case, ensure you set_config the API
key that represents the no-legacy-scope key (the same capgkey value used in the
set_config calls) and assert get_orgs_v6() returns rows (count(*) > 0) and/or
assert expected columns are present/absent to exercise the legacy-scope
difference rather than repeating the same happy-path check.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: b42a8392-c1b6-4445-a1db-934c79183ce3
📒 Files selected for processing (19)
playwright/e2e/apikeys.spec.tssupabase/migrations/20260517085536_migrate_apikeys_to_v2.sqlsupabase/tests/00-supabase_test_helpers.sqlsupabase/tests/07_auth_functions.sqlsupabase/tests/14_test_apikey.sqlsupabase/tests/19_test_identity_functions.sqlsupabase/tests/20_test_org_management_functions.sqlsupabase/tests/26_test_rls_policies.sqlsupabase/tests/33_test_rbac_phase1.sqlsupabase/tests/41_test_reject_access_due_to_2fa_for_app.sqlsupabase/tests/42_test_apikey_expiration.sqlsupabase/tests/42_test_reject_access_due_to_2fa_for_org.sqlsupabase/tests/43_test_rbac_permission_2fa.sqlsupabase/tests/45_test_org_create_app_permission.sqlsupabase/tests/46_test_rbac_legacy_apikey_effective_user.sqlsupabase/tests/47_test_get_org_apikeys_permissions.sqlsupabase/tests/48_test_rbac_apikey_user_mismatch.sqlsupabase/tests/49_test_apikey_oracle_rpc_permissions.sqlsupabase/tests/54_test_usage_credit_rls_performance.sql
🚧 Files skipped from review as they are similar to previous changes (1)
- supabase/migrations/20260517085536_migrate_apikeys_to_v2.sql
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/20260517085536_migrate_apikeys_to_v2.sql`:
- Around line 204-205: After setting OWNER for each new SECURITY DEFINER helper,
explicitly revoke default PUBLIC permissions to prevent public EXECUTE access
drift: add REVOKE ALL ON FUNCTION "public"."get_identity"("keymode"
"public"."key_mode"[]) FROM PUBLIC immediately after the ALTER FUNCTION ...
OWNER TO "postgres" statement, and do the same for the other definer helpers
referenced (public.rbac_check_permission_direct, public.get_user_org_ids,
public.get_orgs_v6, public.get_orgs_v7 and the other function ALTER OWNER
statements flagged in the review) so each function has an explicit REVOKE ALL
... FROM PUBLIC following its ALTER FUNCTION OWNER call.
- Around line 1271-1286: The RLS policies currently grant access TO
"authenticated", "anon" which reopens the table to anon; change each policy
(e.g. the policy titled "Allow admin to select webhooks" that calls
public.check_min_rights) to grant only TO "authenticated" and move any API-key
identity resolution into the SECURITY DEFINER helper (use or update helpers like
public.get_apikey_header() or get_identity_org_appid() to return the resolved
uuid/org/app id); apply the same change to the other policy blocks listed (lines
covering 1288-1303, 1305-1332, 1334-1349, 1355-1370, 1372-1387, 1389-1416) so no
policy includes "anon" and all API-key logic is implemented inside the helper
functions.
- Around line 1234-1264: The function public.find_apikey_by_value is an
unauthenticated oracle because it runs as SECURITY DEFINER, returns full
public.apikeys rows, and is granted EXECUTE to anon/authenticated; fix by either
converting it to SECURITY INVOKER or adding an explicit authorization check that
ties the lookup to the request context before returning any fields (e.g., verify
caller identity/permissions via request headers/JWT claims) and avoid returning
the full row (only return permitted fields or a boolean). Also revoke the
EXECUTE grants to anon/authenticated (or restrict them) and keep service_role if
needed; update or reuse public.check_apikey_hashed_key_enforcement to enforce
authorization when the function is invoked. Ensure the function signature/name
(find_apikey_by_value) and the helper check_apikey_hashed_key_enforcement are
the loci of these changes.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 41f71709-e729-4628-abab-b4a3bce49d8a
📒 Files selected for processing (1)
supabase/migrations/20260517085536_migrate_apikeys_to_v2.sql
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
tests/webhook-signature.test.ts (1)
134-135: ⚡ Quick winPropagate API-key cleanup failures in teardown.
This cleanup currently ignores DB errors; making it strict prevents hidden residue between test files.
Proposed change
if (webhookApiKeyId !== null) - await getSupabaseClient().from('apikeys').delete().eq('id', webhookApiKeyId) + await getSupabaseClient().from('apikeys').delete().eq('id', webhookApiKeyId).throwOnError()🤖 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 `@tests/webhook-signature.test.ts` around lines 134 - 135, The teardown currently swallows DB errors when deleting the test API key; change the deletion to await the Supabase response from getSupabaseClient().from('apikeys').delete().eq('id', webhookApiKeyId'), check the returned error (or status) and throw or fail the test if the deletion failed so cleanup failures are propagated (i.e., capture the result, if result.error is set then rethrow or assert to surface the failure).tests/bundle-semver-validation.test.ts (1)
37-42: ⚡ Quick winMake API-key teardown strict to avoid silent test-data leaks.
If this delete fails, the suite can leave residue and become flaky across parallel files. Please fail fast on cleanup errors.
Proposed change
if (semverApiKeyId !== null) { await getSupabaseClient() .from('apikeys') .delete() .eq('id', semverApiKeyId) + .throwOnError() }🤖 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 `@tests/bundle-semver-validation.test.ts` around lines 37 - 42, The teardown delete for the test API key is currently fire-and-forget; change it to capture and check the Supabase response (e.g., const { error } = await getSupabaseClient().from('apikeys').delete().eq('id', semverApiKeyId)) and if error is non-null throw or fail the test (throw new Error with the error message) so cleanup failures cause the test suite to fail fast; reference semverApiKeyId and the getSupabaseClient().from('apikeys').delete().eq(...) call when making the change.tests/private-error-cases.test.ts (1)
74-75: ⚡ Quick winMake teardown API-key deletion fail-fast.
Silent cleanup failures can leave org-scoped key rows behind and introduce cross-test flakiness.
Proposed change
if (testOrgApiKeyId !== null) - await getSupabaseClient().from('apikeys').delete().eq('id', testOrgApiKeyId) + await getSupabaseClient().from('apikeys').delete().eq('id', testOrgApiKeyId).throwOnError()🤖 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 `@tests/private-error-cases.test.ts` around lines 74 - 75, The teardown currently deletes the test API key row with getSupabaseClient().from('apikeys').delete().eq('id', testOrgApiKeyId) but ignores failures; update the teardown to treat deletion as fail-fast by awaiting the delete response and checking its error and/or affected row count: after calling getSupabaseClient().from('apikeys').delete().eq('id', testOrgApiKeyId) assert that the response has no error and that one row was deleted (or throw with the response.error) so any cleanup failure surfaces immediately (refer to testOrgApiKeyId and getSupabaseClient()).
🤖 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.
Nitpick comments:
In `@tests/bundle-semver-validation.test.ts`:
- Around line 37-42: The teardown delete for the test API key is currently
fire-and-forget; change it to capture and check the Supabase response (e.g.,
const { error } = await getSupabaseClient().from('apikeys').delete().eq('id',
semverApiKeyId)) and if error is non-null throw or fail the test (throw new
Error with the error message) so cleanup failures cause the test suite to fail
fast; reference semverApiKeyId and the
getSupabaseClient().from('apikeys').delete().eq(...) call when making the
change.
In `@tests/private-error-cases.test.ts`:
- Around line 74-75: The teardown currently deletes the test API key row with
getSupabaseClient().from('apikeys').delete().eq('id', testOrgApiKeyId) but
ignores failures; update the teardown to treat deletion as fail-fast by awaiting
the delete response and checking its error and/or affected row count: after
calling getSupabaseClient().from('apikeys').delete().eq('id', testOrgApiKeyId)
assert that the response has no error and that one row was deleted (or throw
with the response.error) so any cleanup failure surfaces immediately (refer to
testOrgApiKeyId and getSupabaseClient()).
In `@tests/webhook-signature.test.ts`:
- Around line 134-135: The teardown currently swallows DB errors when deleting
the test API key; change the deletion to await the Supabase response from
getSupabaseClient().from('apikeys').delete().eq('id', webhookApiKeyId'), check
the returned error (or status) and throw or fail the test if the deletion failed
so cleanup failures are propagated (i.e., capture the result, if result.error is
set then rethrow or assert to surface the failure).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a6869976-c4a4-45b8-8f17-ebeaa661fb7d
📒 Files selected for processing (4)
tests/bundle-semver-validation.test.tstests/manifest-rls.test.tstests/private-error-cases.test.tstests/webhook-signature.test.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
tests/test-utils.ts (1)
240-247: ⚡ Quick winUse an
interfaceforApiKeyBinding.This declaration is already tripping
ts/consistent-type-definitions, so keeping it as a type alias leaves the helper out of line with the repo's TS linting.Proposed fix
-type ApiKeyBinding = { +interface ApiKeyBinding { role_name: string scope_type: 'org' | 'app' | 'channel' org_id: string app_id?: string | null channel_id?: string | number | null reason?: string -} +}🤖 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 `@tests/test-utils.ts` around lines 240 - 247, Replace the type alias ApiKeyBinding with an interface declaration named ApiKeyBinding keeping the same properties and types (role_name: string, scope_type: 'org' | 'app' | 'channel', org_id: string, optional app_id?: string | null, optional channel_id?: string | number | null, optional reason?: string); update the declaration in the tests/test-utils module so lint rule ts/consistent-type-definitions is satisfied and ensure all usages of ApiKeyBinding continue to reference the same identifier.
🤖 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 `@tests/test-utils.ts`:
- Around line 326-349: The code creates bindings using options.orgId and
app.owner_org without validating they match, allowing cross-org keys; update the
block handling options.appId so that after fetching app (app.owner_org) you
validate: if options.orgId is provided and options.orgId !== app.owner_org then
throw an Error (reject the mismatch); otherwise use app.owner_org as the org_id
when pushing into bindingRows (i.e., set org_id = app.owner_org if options.orgId
is absent). Ensure you apply this check near the existing code that queries apps
and before the bindingRows.push (references: options.appId, options.orgId,
app.owner_org, bindingRows, apiKey.rbac_id, appRole.id).
- Around line 291-360: The helper creates an apikey row (apiKey from
supabase.insert) and then performs role lookups and inserts bindingRows without
cleaning up if any later step fails; update the function (around the apiKey
variable and subsequent role/binding code) to wrap the role resolution and
binding insertion in try/catch, and in the catch ensure you delete the created
apikey (use supabase.from('apikeys').delete().eq('id', apiKey.id) and await it)
before rethrowing the original error so orphaned apikeys are not left behind;
apply this cleanup for failures in org/app role resolution and
role_bindings.insert, and preserve throwing the original error.
---
Nitpick comments:
In `@tests/test-utils.ts`:
- Around line 240-247: Replace the type alias ApiKeyBinding with an interface
declaration named ApiKeyBinding keeping the same properties and types
(role_name: string, scope_type: 'org' | 'app' | 'channel', org_id: string,
optional app_id?: string | null, optional channel_id?: string | number | null,
optional reason?: string); update the declaration in the tests/test-utils module
so lint rule ts/consistent-type-definitions is satisfied and ensure all usages
of ApiKeyBinding continue to reference the same identifier.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 541994d0-ddc8-4ec1-9307-e2d73e25abb8
📒 Files selected for processing (2)
tests/app-id-validation.test.tstests/test-utils.ts
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
# Conflicts: # src/components/dashboard/AppOnboardingFlow.vue # src/pages/admin/dashboard/replication.vue # src/types/supabase.types.ts # supabase/functions/_backend/utils/supabase.types.ts
Contributor
Merging this PR will not alter performance
Comparing 
Footnotes
|
|
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 (AI generated)
org_users.user_rightis compatibility data, not an authority source.use_new_rbacUI split.Motivation (AI generated)
Legacy API keys and legacy org rights created two parallel permission systems. This consolidates runtime authorization on RBAC/V2 while keeping old clients working through compatibility wrappers, so users do not need to update the CLI for the migration to work.
Business Impact (AI generated)
Test Plan (AI generated)
bun lintbun lint:backendbun typecheckbun test tests/invites.unit.test.tsRun tests/ CodeQL / CodeRabbit on this PR updatebun run supabase:db:resetcould not complete in this worktree because the Docker/Supabase reset process timed out after 45s before producing SQL output; CI is being used for migration-backed validation.