Harden get_identity_apikey_only RPC permissions#1765
Conversation
|
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:
📝 WalkthroughWalkthroughAdds tests for RPC permissions of Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a8740dd3bd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| import { describe, expect, it } from 'vitest' | ||
| import { APIKEY_TEST_ALL } from './test-utils' | ||
|
|
||
| const SUPABASE_URL = env.SUPABASE_URL as string |
There was a problem hiding this comment.
This test uses env.SUPABASE_URL directly, but the suite already documents and works around a real CI issue where localhost is resolved to ::1 while Supabase is bound to 127.0.0.1 (see the normalization helpers in tests/test-utils.ts). In environments where SUPABASE_URL is http://localhost:..., these RPC calls can fail with connection errors unrelated to permissions, making this new regression test flaky or consistently failing.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/get-identity-apikey-only-rpc.test.ts`:
- Around line 2-4: Reorder the top imports so built-in modules come first: move
"import { env } from 'node:process'" before the external import "import {
createClient } from '@supabase/supabase-js'"; leave the test imports ("import {
describe, expect, it } from 'vitest'") after the external import so the
file-level import order satisfies the ESLint builtin→external→test pattern.
- Around line 13-47: Add a new concurrent test that asserts the authenticated
role is denied RPC access: create a Supabase client that represents a logged-in
user (use createClient and set an authenticated JWT/session so the client acts
as the authenticated role), include the same global header { capgkey:
APIKEY_TEST_ALL }, call rpc('get_identity_apikey_only', { keymode: keyModes })
and assert data is null and error is not null with error.message containing
'permission denied' (mirroring the anon test structure but using an
authenticated session instead of SUPABASE_ANON_KEY or SUPABASE_SERVICE_KEY).
- Line 11: The test fails because keyModes is declared as a readonly tuple with
"as const" and cannot be passed into the RPC parameter typed as
Database["public"]["Enums"]["key_mode"][]; remove the "as const" so keyModes is
a mutable array whose element type matches
Database["public"]["Enums"]["key_mode"], or explicitly type it as
Database["public"]["Enums"]["key_mode"][] (update the const declaration of
keyModes used by the RPC calls in get-identity-apikey-only-rpc.test.ts).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c36e1c99-4967-4a6c-98b3-b162b0b924a3
📒 Files selected for processing (2)
supabase/migrations/20260308120000_hardening_get_identity_apikey_only_rpcs.sqltests/get-identity-apikey-only-rpc.test.ts
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/get-identity-apikey-only-rpc.test.ts (1)
7-19: Simplify redundant return logic.When
!rawevaluates to true,raw || ''will always return''sincerawis falsy. The conditional can be simplified.♻️ Proposed simplification
function normalizeLocalhostUrl(raw: string | undefined): string { if (!raw) - return raw || '' + return '' try {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/get-identity-apikey-only-rpc.test.ts` around lines 7 - 19, The guard in normalizeLocalhostUrl uses the redundant expression "if (!raw) return raw || ''"; change it to a simple explicit return for the falsy case (e.g., "if (!raw) return ''") so the logic is clearer and no unnecessary fallback is evaluated; keep the rest of the function (URL parsing, hostname substitution, and catch behavior) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/get-identity-apikey-only-rpc.test.ts`:
- Around line 7-19: The guard in normalizeLocalhostUrl uses the redundant
expression "if (!raw) return raw || ''"; change it to a simple explicit return
for the falsy case (e.g., "if (!raw) return ''") so the logic is clearer and no
unnecessary fallback is evaluated; keep the rest of the function (URL parsing,
hostname substitution, and catch behavior) unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1c4cce2c-94d5-457a-828b-aa7d649f7293
📒 Files selected for processing (1)
tests/get-identity-apikey-only-rpc.test.ts
riderx
force-pushed
the
riderx/fix-attached-issue-2
branch
from
5cabc70 to
024d29f
Compare
|
1 participant
Summary
Test plan
Screenshots
Checklist
bun run lint:backend && bun run lint.
Summary by CodeRabbit
Bug Fixes
Tests