fix: support hashed API keys in RLS identity functions#1351
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughRefactors identity-resolution SQL functions to use Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant RLS_Fn as public.get_identity<br/>(and variants)
participant Finder as find_apikey_by_value
participant Expiry as is_apikey_expired
participant DB as apikeys/users tables
Client->>RLS_Fn: call RPC / invoke query (header capgkey)
RLS_Fn->>RLS_Fn: extract API key from header
RLS_Fn->>Finder: lookup key (plain or hashed)
Finder->>DB: query apikeys table
DB-->>Finder: apikey record or NULL
Finder-->>RLS_Fn: apikey_id (or NULL)
alt apikey found
RLS_Fn->>Expiry: check expiration(apikey_id)
Expiry-->>RLS_Fn: expired? boolean
alt not expired & mode/org/app ok
RLS_Fn->>DB: resolve user_id
DB-->>RLS_Fn: user_id
RLS_Fn-->>Client: return user_id
else expired or restrictions fail
RLS_Fn->>RLS_Fn: log denial
RLS_Fn-->>Client: return NULL
end
else no apikey match
RLS_Fn->>RLS_Fn: log denial
RLS_Fn-->>Client: return NULL
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 SQLFluff (3.5.0)supabase/migrations/20260102140000_fix_get_identity_hashed_apikeys.sqlUser Error: No dialect was specified. You must configure a dialect or specify one on the command line using --dialect after the command. Available dialects: 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ 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".
| GRANT EXECUTE ON FUNCTION "public"."find_apikey_by_value"(text) TO "anon"; | ||
| GRANT EXECUTE ON FUNCTION "public"."find_apikey_by_value"(text) TO "authenticated"; |
There was a problem hiding this comment.
Avoid granting find_apikey_by_value to anon/auth
Granting EXECUTE on find_apikey_by_value to anon/authenticated makes this SECURITY DEFINER function callable over RPC by any client with the anon key, and it returns the full apikeys row (including key for non‑hashed keys and metadata) while bypassing RLS. That means a client can probe API key values directly and read fields that are otherwise protected by the apikeys RLS policies. If the intent is only to enable RLS identity lookups, it’s safer to keep this function non‑public and expose a narrower wrapper (e.g., returning only user_id) for policies.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
supabase/migrations/20260102140000_fix_get_identity_hashed_apikeys.sql (1)
221-222: Security concern: grantingfind_apikey_by_valueto anon/authenticated exposes API key metadata.This has been flagged in a previous review. Since
find_apikey_by_valueisSECURITY DEFINERand returns the fullapikeysrow (including the plainkeyfor non-hashed keys), granting EXECUTE to anon/authenticated makes it callable via RPC, allowing clients to probe API key values.Consider one of:
- Not granting EXECUTE to anon/authenticated (RLS functions already run as SECURITY DEFINER)
- Creating a narrower internal function that only returns what's needed
- Prefixing with underscore to hide from PostgREST API while keeping grants
🧹 Nitpick comments (2)
tests/hashed-apikey-rls.test.ts (1)
17-28: Consider parameterized JSON for request.headers injection.The current string interpolation is acceptable for test inputs but could be made more robust:
async function execWithCapgkey(sql: string, capgkey: string): Promise<any> { const client = await pool.connect() try { - await client.query(`SET request.headers = '{"capgkey": "${capgkey}"}'`) + const headersJson = JSON.stringify({ capgkey }) + await client.query(`SET request.headers = '${headersJson.replace(/'/g, "''")}'`) const result = await client.query(sql) return result.rows }This would handle edge cases if a key value ever contained quotes. Since inputs are controlled in tests, this is optional.
supabase/migrations/20260102140000_fix_get_identity_hashed_apikeys.sql (1)
199-205: Inconsistent null/empty array handling between org and app restrictions.The org check at line 192 uses
COALESCE(array_length(...), 0) > 0while the app check at line 200 usesIS DISTINCT FROM '{}'. While both work correctly, the different patterns make the code harder to reason about:
COALESCE(array_length(arr, 1), 0) > 0explicitly handles NULL (returns FALSE)arr IS DISTINCT FROM '{}'returns TRUE for NULL, then relies onANY(NULL)returning NULLFor consistency with the org check pattern:
Suggested change
- IF api_key.limited_to_apps IS DISTINCT FROM '{}' THEN + IF COALESCE(array_length(api_key.limited_to_apps, 1), 0) > 0 THEN IF NOT (app_id = ANY(api_key.limited_to_apps)) THEN
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
supabase/migrations/20260102140000_fix_get_identity_hashed_apikeys.sqltests/apikeys.test.tstests/hashed-apikey-rls.test.ts
🧰 Additional context used
📓 Path-based instructions (7)
supabase/migrations/**/*.sql
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Database migrations must be created with
supabase migration new <feature_slug>and never modify previously committed migrations
Files:
supabase/migrations/20260102140000_fix_get_identity_hashed_apikeys.sql
**/{migrations,tests,__tests__}/**/*.{sql,ts,js}
📄 CodeRabbit inference engine (AGENTS.md)
Always cover database changes with Postgres-level tests and complement them with end-to-end tests for affected user flows
Files:
supabase/migrations/20260102140000_fix_get_identity_hashed_apikeys.sqltests/hashed-apikey-rls.test.tstests/apikeys.test.ts
supabase/migrations/*.sql
📄 CodeRabbit inference engine (AGENTS.md)
supabase/migrations/*.sql: When creating schema changes, usesupabase migration new <feature_slug>to create a single migration file and keep editing that file until the feature ships; never edit previously committed migrations
A migration that introduces a new table may include seed inserts for that table, treating seeding as part of the current feature and not modifying previously committed migrations
Do not create new cron jobs; instead update theprocess_all_cron_tasksfunction in a new migration file to add your job if needed
Files:
supabase/migrations/20260102140000_fix_get_identity_hashed_apikeys.sql
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use single quotes and no semicolons per @antfu/eslint-config
Files:
tests/hashed-apikey-rls.test.tstests/apikeys.test.ts
tests/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Backend tests should be located in the
tests/directory and use Vitest test runner
Files:
tests/hashed-apikey-rls.test.tstests/apikeys.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use TypeScript strict mode with path aliases mapping
~/tosrc/
Files:
tests/hashed-apikey-rls.test.tstests/apikeys.test.ts
tests/**/*.{ts,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Backend tests must use helpers from
tests/test-utils.tsincludinggetEndpointUrl(path)for correct worker routing andUSE_CLOUDFLARE_WORKERS=truefor CF Workers testing
Files:
tests/hashed-apikey-rls.test.tstests/apikeys.test.ts
🧠 Learnings (11)
📓 Common learnings
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-23T02:53:12.055Z
Learning: Applies to supabase/functions/_backend/**/*.{ts,js} : Check `c.get('auth')?.authType` to determine authentication type ('apikey' vs 'jwt') in backend endpoints
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-23T02:53:12.055Z
Learning: Applies to supabase/functions/_backend/**/*.{ts,js} : Use `middlewareAPISecret` for internal API endpoints and `middlewareKey` for external API keys; validate against `owner_org` in the `apikeys` table
📚 Learning: 2025-12-24T14:11:10.256Z
Learnt from: WcaleNieWolny
Repo: Cap-go/capgo PR: 1300
File: supabase/migrations/20251224103713_2fa_enforcement.sql:409-539
Timestamp: 2025-12-24T14:11:10.256Z
Learning: In supabase/migrations for get_orgs_v6 and get_orgs_v7: The inner functions with user_id parameter (get_orgs_v6(uuid) and get_orgs_v7(uuid)) should NOT be granted to anon/authenticated roles as this allows any user to query other users' organizations; only the no-argument wrapper functions should be public as they perform authentication checks.
Applied to files:
supabase/migrations/20260102140000_fix_get_identity_hashed_apikeys.sqltests/hashed-apikey-rls.test.ts
📚 Learning: 2025-12-23T02:53:12.055Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-23T02:53:12.055Z
Learning: Applies to supabase/functions/_backend/**/*.{ts,js} : Use `middlewareAPISecret` for internal API endpoints and `middlewareKey` for external API keys; validate against `owner_org` in the `apikeys` table
Applied to files:
supabase/migrations/20260102140000_fix_get_identity_hashed_apikeys.sqltests/hashed-apikey-rls.test.tstests/apikeys.test.ts
📚 Learning: 2025-12-27T03:51:23.575Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-27T03:51:23.575Z
Learning: Applies to supabase/seed.sql : Update `supabase/seed.sql` to back new or evolved tests; keep fixtures focused on current behavior while leaving committed migrations unchanged
Applied to files:
supabase/migrations/20260102140000_fix_get_identity_hashed_apikeys.sqltests/hashed-apikey-rls.test.tstests/apikeys.test.ts
📚 Learning: 2025-12-23T02:53:12.055Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-23T02:53:12.055Z
Learning: Applies to supabase/functions/_backend/**/*.{ts,js} : Check `c.get('auth')?.authType` to determine authentication type ('apikey' vs 'jwt') in backend endpoints
Applied to files:
supabase/migrations/20260102140000_fix_get_identity_hashed_apikeys.sqltests/hashed-apikey-rls.test.tstests/apikeys.test.ts
📚 Learning: 2025-12-25T11:22:13.039Z
Learnt from: WcaleNieWolny
Repo: Cap-go/capgo PR: 1300
File: supabase/migrations/20251224103713_2fa_enforcement.sql:85-96
Timestamp: 2025-12-25T11:22:13.039Z
Learning: In SQL migrations under the repository (e.g., supabase/migrations), enforce that when an org has enforcing_2fa=true, all users (including super_admins) must have 2FA enabled before accessing any org functions, including check_org_members_2fa_enabled. Do not grant admin exceptions to 2FA requirements. This ensures consistent security enforcement across all org-related operations; implement this rule within relevant migrations and associated stored procedures/tests.
Applied to files:
supabase/migrations/20260102140000_fix_get_identity_hashed_apikeys.sql
📚 Learning: 2025-12-27T03:51:23.575Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-27T03:51:23.575Z
Learning: Applies to **/{migrations,tests,__tests__}/**/*.{sql,ts,js} : Always cover database changes with Postgres-level tests and complement them with end-to-end tests for affected user flows
Applied to files:
tests/hashed-apikey-rls.test.ts
📚 Learning: 2025-12-23T02:53:12.055Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-23T02:53:12.055Z
Learning: Backend tests modify local database; always reset database with `supabase db reset` before running tests to ensure clean state
Applied to files:
tests/apikeys.test.ts
📚 Learning: 2025-12-05T17:34:25.556Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-05T17:34:25.556Z
Learning: Backend tests that modify local database state require a running Supabase instance
Applied to files:
tests/apikeys.test.ts
📚 Learning: 2025-12-23T02:53:12.055Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-23T02:53:12.055Z
Learning: Applies to tests/**/*.{ts,js} : Backend tests must use helpers from `tests/test-utils.ts` including `getEndpointUrl(path)` for correct worker routing and `USE_CLOUDFLARE_WORKERS=true` for CF Workers testing
Applied to files:
tests/apikeys.test.ts
📚 Learning: 2025-12-05T17:34:25.556Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-05T17:34:25.556Z
Learning: Applies to tests/**/*.{test,spec}.{ts,tsx} : Backend tests should be located in the `tests/` directory and use Vitest test runner
Applied to files:
tests/apikeys.test.ts
🧬 Code graph analysis (2)
tests/hashed-apikey-rls.test.ts (1)
tests/test-utils.ts (5)
BASE_URL(19-19)POSTGRES_URL(7-7)USER_ID(45-45)ORG_ID(42-42)APP_NAME(57-57)
tests/apikeys.test.ts (2)
tests/test-utils.ts (1)
BASE_URL(19-19)supabase/functions/_backend/utils/postgres_schema.ts (2)
apps(19-32)orgs(94-103)
🪛 Gitleaks (8.30.0)
tests/hashed-apikey-rls.test.ts
[high] 445-445: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run tests
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (8)
tests/apikeys.test.ts (2)
1-4: LGTM! Import reorganization is appropriate.Moving
createClientimport to the top level alongside other imports improves code organization and aligns with module-level import conventions.
548-640: Well-structured RLS integration tests for hashed API key support.These tests effectively validate the core feature by simulating CLI usage patterns—creating API keys and using them directly with the Supabase SDK via the
capgkeyheader. The coverage of both hashed and plain key scenarios, along with proper cleanup, ensures the identity resolution path works correctly for real-world usage.tests/hashed-apikey-rls.test.ts (5)
1-11: Excellent documentation and imports for the test file.The header comment clearly explains the purpose of these tests and their importance for CLI usage. The imports are well-organized and include all necessary dependencies.
88-175: Comprehensive test coverage forget_identity()with hashed API keys.The test suite thoroughly covers key scenarios: plain keys, hashed keys, invalid keys, mode mismatches, expired keys, and non-expired keys. This validates the core identity resolution logic for both key types.
307-368: Good direct testing offind_apikey_by_value()function.Testing the underlying lookup function directly ensures it correctly handles plain key matching, hashed key matching (by hashing the input and comparing), and non-existent keys. This complements the higher-level identity function tests.
439-458: Static analysis false positive: intentional test value for invalid key.The
'invalid-key-12345'string flagged by Gitleaks is an intentionally invalid test value used to verify that RLS correctly blocks access when an invalid API key is provided. This is expected behavior for a negative test case.
370-458: SDK-based RLS tests validate end-to-end behavior.These tests exercise the full path from Supabase client through PostgREST to RLS policies, ensuring hashed API keys work correctly in real-world CLI usage scenarios. Good coverage of table queries and RPC calls.
supabase/migrations/20260102140000_fix_get_identity_hashed_apikeys.sql (1)
16-54: Well-structured refactor ofget_identity()for hashed key support.The function correctly:
- Prioritizes JWT authentication via
auth.uid()- Falls back to API key lookup using
find_apikey_by_value- Validates mode membership and expiration
- Logs denials for observability
The use of
SECURITY DEFINERwith emptysearch_pathfollows security best practices.
Fixed a critical bug where hashed API keys were not working with RLS policies. The get_identity functions only checked the plain 'key' column, but for hashed keys, the key is NULL and only key_hash is populated. Updated get_identity(), get_identity_apikey_only(), get_identity_org_allowed(), and get_identity_org_appid() to use find_apikey_by_value() which checks both plain and hashed keys. This enables CLI usage with hashed API keys. Added 21 comprehensive tests in hashed-apikey-rls.test.ts verifying: - Plain and hashed key lookup in all identity functions - Expiration checking with hashed keys - Organization and app restriction enforcement - RLS policy enforcement via Supabase SDK 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Move createClient import from dynamic imports inside test bodies to a static import at the top of the file for cleaner code. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The get_org_members RPC function returns is_tmp for pending invitations, but the API schema was filtering it out. This fix allows the field to be returned to clients. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
29a8662 to
96a066e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/hashed-apikey-rls.test.ts (1)
58-64: Consider validating deletion response.The function silently ignores failures, which could leave orphaned test keys in the database if the API endpoint has issues. For test reliability, consider logging or throwing on unexpected status codes.
🔎 Suggested improvement
async function deleteApiKey(id: number): Promise<void> { - await fetch(`${BASE_URL}/apikey/${id}`, { + const response = await fetch(`${BASE_URL}/apikey/${id}`, { method: 'DELETE', headers, }) + if (!response.ok) { + console.warn(`Failed to delete API key ${id}: ${response.status}`) + } }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
supabase/functions/_backend/public/organization/members/get.tssupabase/migrations/20260102140000_fix_get_identity_hashed_apikeys.sqltests/apikeys.test.tstests/hashed-apikey-rls.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- supabase/migrations/20260102140000_fix_get_identity_hashed_apikeys.sql
- tests/apikeys.test.ts
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use single quotes and no semicolons per @antfu/eslint-config
Files:
supabase/functions/_backend/public/organization/members/get.tstests/hashed-apikey-rls.test.ts
supabase/functions/_backend/**
📄 CodeRabbit inference engine (CLAUDE.md)
Backend logic should be organized in
supabase/functions/_backend/with subdirectories for plugins, private endpoints, public endpoints, triggers, and utilities
Files:
supabase/functions/_backend/public/organization/members/get.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use TypeScript strict mode with path aliases mapping
~/tosrc/
Files:
supabase/functions/_backend/public/organization/members/get.tstests/hashed-apikey-rls.test.ts
supabase/functions/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Supabase Edge Functions use Deno runtime
Files:
supabase/functions/_backend/public/organization/members/get.ts
supabase/functions/_backend/**/*.{ts,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
supabase/functions/_backend/**/*.{ts,js}: Backend code must be placed insupabase/functions/_backend/as shared code deployed to Cloudflare Workers (API/Plugin/Files workers), Supabase Edge Functions, and other platforms
UsecreateHonofromutils/hono.tsfor all Hono framework application initialization and routing
All database operations must usegetPgClient()orgetDrizzleClient()fromutils/pg.tsfor PostgreSQL access during active migration to Cloudflare D1
All Hono endpoint handlers must acceptContext<MiddlewareKeyVariables>and usec.get('requestId'),c.get('apikey'), andc.get('auth')for request context
Use structured logging withcloudlog({ requestId: c.get('requestId'), message: '...' })for all backend logging
UsemiddlewareAPISecretfor internal API endpoints andmiddlewareKeyfor external API keys; validate againstowner_orgin theapikeystable
Checkc.get('auth')?.authTypeto determine authentication type ('apikey' vs 'jwt') in backend endpoints
Use Drizzle ORM query patterns withschemafrompostgress_schema.tsfor all database operations; usealiasV2()for self-joins or multiple table references
Files:
supabase/functions/_backend/public/organization/members/get.ts
supabase/functions/**/*.{ts,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Backend ESLint must pass before commit; run
bun lint:backendfor backend files
Files:
supabase/functions/_backend/public/organization/members/get.ts
tests/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Backend tests should be located in the
tests/directory and use Vitest test runner
Files:
tests/hashed-apikey-rls.test.ts
tests/**/*.{ts,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Backend tests must use helpers from
tests/test-utils.tsincludinggetEndpointUrl(path)for correct worker routing andUSE_CLOUDFLARE_WORKERS=truefor CF Workers testing
Files:
tests/hashed-apikey-rls.test.ts
**/{migrations,tests,__tests__}/**/*.{sql,ts,js}
📄 CodeRabbit inference engine (AGENTS.md)
Always cover database changes with Postgres-level tests and complement them with end-to-end tests for affected user flows
Files:
tests/hashed-apikey-rls.test.ts
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-23T02:53:12.055Z
Learning: Applies to supabase/functions/_backend/**/*.{ts,js} : Check `c.get('auth')?.authType` to determine authentication type ('apikey' vs 'jwt') in backend endpoints
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-23T02:53:12.055Z
Learning: Applies to supabase/functions/_backend/**/*.{ts,js} : Use `middlewareAPISecret` for internal API endpoints and `middlewareKey` for external API keys; validate against `owner_org` in the `apikeys` table
📚 Learning: 2025-12-23T02:53:12.055Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-23T02:53:12.055Z
Learning: Applies to supabase/functions/_backend/**/*.{ts,js} : Use `middlewareAPISecret` for internal API endpoints and `middlewareKey` for external API keys; validate against `owner_org` in the `apikeys` table
Applied to files:
tests/hashed-apikey-rls.test.ts
📚 Learning: 2025-12-23T02:53:12.055Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-23T02:53:12.055Z
Learning: Applies to supabase/functions/_backend/**/*.{ts,js} : Check `c.get('auth')?.authType` to determine authentication type ('apikey' vs 'jwt') in backend endpoints
Applied to files:
tests/hashed-apikey-rls.test.ts
📚 Learning: 2025-12-27T03:51:23.575Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-27T03:51:23.575Z
Learning: Applies to **/{migrations,tests,__tests__}/**/*.{sql,ts,js} : Always cover database changes with Postgres-level tests and complement them with end-to-end tests for affected user flows
Applied to files:
tests/hashed-apikey-rls.test.ts
📚 Learning: 2025-12-27T03:51:23.575Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-27T03:51:23.575Z
Learning: Applies to supabase/seed.sql : Update `supabase/seed.sql` to back new or evolved tests; keep fixtures focused on current behavior while leaving committed migrations unchanged
Applied to files:
tests/hashed-apikey-rls.test.ts
📚 Learning: 2025-12-24T14:11:10.256Z
Learnt from: WcaleNieWolny
Repo: Cap-go/capgo PR: 1300
File: supabase/migrations/20251224103713_2fa_enforcement.sql:409-539
Timestamp: 2025-12-24T14:11:10.256Z
Learning: In supabase/migrations for get_orgs_v6 and get_orgs_v7: The inner functions with user_id parameter (get_orgs_v6(uuid) and get_orgs_v7(uuid)) should NOT be granted to anon/authenticated roles as this allows any user to query other users' organizations; only the no-argument wrapper functions should be public as they perform authentication checks.
Applied to files:
tests/hashed-apikey-rls.test.ts
🧬 Code graph analysis (1)
tests/hashed-apikey-rls.test.ts (1)
tests/test-utils.ts (5)
BASE_URL(19-19)POSTGRES_URL(7-7)USER_ID(45-45)ORG_ID(42-42)APP_NAME(57-57)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run tests
🔇 Additional comments (7)
tests/hashed-apikey-rls.test.ts (6)
1-14: LGTM!Clean imports and proper organization of dependencies for the test suite.
30-56: LGTM!Helper functions properly validate response status and provide meaningful error messages on failure.
66-86: LGTM!Proper resource lifecycle management with pool creation in
beforeAlland cleanup inafterAll.
88-305: Well-structured test coverage for identity functions.The tests comprehensively cover plain keys, hashed keys, invalid keys, mode restrictions, expiration handling, and org/app limitations. Each describe block properly manages its own test keys with appropriate setup and cleanup.
307-368: LGTM!The
find_apikey_by_value()tests properly validate lookup behavior for both plain keys (matching bykeycolumn) and hashed keys (hashing input and matchingkey_hash). Good verification of return values including theNULLkey for hashed entries.
370-459: SDK integration tests provide good end-to-end coverage.These tests validate real-world CLI usage patterns where the Supabase SDK is used directly with the
capgkeyheader. The test at lines 439-458 correctly verifies that RLS blocks access gracefully (empty array) rather than throwing an error.supabase/functions/_backend/public/organization/members/get.ts (1)
28-28: No issues found - the schema change is correct.The
get_org_membersRPC function has been returning theis_tmpfield since migration 20250613034031_tmp_users_table.sql. The schema change at line 28 simply synchronizes the TypeScript validation with the current RPC implementation. Since the field is guaranteed to be present (returned asfalsefor existing members andtruefor pending invitations), marking it as required in the Zod schema is correct and will not cause safeParse failures.
| // Helper to execute SQL with capgkey header set | ||
| async function execWithCapgkey(sql: string, capgkey: string): Promise<any> { | ||
| const client = await pool.connect() | ||
| try { | ||
| // Set the capgkey header in request.headers (how Supabase passes it to RLS) | ||
| await client.query(`SET request.headers = '{"capgkey": "${capgkey}"}'`) | ||
| const result = await client.query(sql) | ||
| return result.rows | ||
| } | ||
| finally { | ||
| client.release() | ||
| } | ||
| } |
There was a problem hiding this comment.
Potential SQL injection via unescaped capgkey value.
The capgkey is directly interpolated into the JSON string. If the key contains characters like " or \, it could break the JSON structure or allow injection. Use JSON.stringify to properly escape the value.
🔎 Proposed fix
async function execWithCapgkey(sql: string, capgkey: string): Promise<any> {
const client = await pool.connect()
try {
// Set the capgkey header in request.headers (how Supabase passes it to RLS)
- await client.query(`SET request.headers = '{"capgkey": "${capgkey}"}'`)
+ await client.query(`SET request.headers = ${client.escapeLiteral(JSON.stringify({ capgkey }))}`)
const result = await client.query(sql)
return result.rows
}
finally {
client.release()
}
}🤖 Prompt for AI Agents
In tests/hashed-apikey-rls.test.ts around lines 16-28, the capgkey is
interpolated directly into the JSON string used in the SET request.headers call,
creating an injection risk; change the code to build a proper JSON string with
JSON.stringify(capgkey) (or build the full headers object and JSON.stringify it)
and pass that JSON via a parameterized query (e.g. SET request.headers = $1)
instead of string interpolation so special characters are escaped safely.
|
* fix: support hashed API keys in RLS identity functions Fixed a critical bug where hashed API keys were not working with RLS policies. The get_identity functions only checked the plain 'key' column, but for hashed keys, the key is NULL and only key_hash is populated. Updated get_identity(), get_identity_apikey_only(), get_identity_org_allowed(), and get_identity_org_appid() to use find_apikey_by_value() which checks both plain and hashed keys. This enables CLI usage with hashed API keys. Added 21 comprehensive tests in hashed-apikey-rls.test.ts verifying: - Plain and hashed key lookup in all identity functions - Expiration checking with hashed keys - Organization and app restriction enforcement - RLS policy enforcement via Supabase SDK 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * refactor: move supabase-js import to top level in apikeys tests Move createClient import from dynamic imports inside test bodies to a static import at the top of the file for cleaner code. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * chore: trigger CI rebuild * fix: add is_tmp field to organization members response schema The get_org_members RPC function returns is_tmp for pending invitations, but the API schema was filtering it out. This fix allows the field to be returned to clients. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Haiku 4.5 <noreply@anthropic.com> Co-authored-by: WcaleNieWolny <isupermichael007@gmail.com>
Summary
Fixed a critical bug where hashed API keys weren't working with RLS policies. The get_identity functions only checked the plain 'key' column, failing when key is NULL and key_hash contains the SHA-256. Updated all identity functions to use find_apikey_by_value() for both plain and hashed key lookups.
Test plan
Checklist
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes / Behavior
Testing
Other
✏️ Tip: You can customize this high-level summary in your review settings.