Restore audit log API key access#1577
Conversation
📝 WalkthroughWalkthroughAdds API-key aware authorization to the organization audit endpoint with a branched auth flow for API keys vs JWTs; updates route middleware; restores an RLS SELECT policy to allow audit_logs reads for API-key contexts; and adds SQL tests validating audit logging under API-key authentication. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Middleware as Middleware
participant OrgService as OrgService
participant DB as Database (RLS)
rect rgba(100,150,200,0.5)
Note over Client,DB: API-key request flow
Client->>Middleware: GET /audit (API-Key header)
Middleware->>OrgService: validate api key present
OrgService->>OrgService: apikeyHasOrgRightWithPolicy(apikey, org_id)
alt apikey has org right
OrgService->>OrgService: hasOrgRightApikey(capgkey, org_id)
alt org access allowed
OrgService->>DB: SELECT audit_logs (RLS applies)
DB-->>Client: audit log rows
else org access denied
OrgService-->>Client: 401 / 400 (org_requires_expiring_key / invalid_org_id)
end
else apikey invalid
OrgService-->>Client: 400 invalid_org_id
end
end
rect rgba(200,150,100,0.5)
Note over Client,DB: JWT request flow
Client->>Middleware: GET /audit (JWT)
Middleware->>OrgService: call check_min_rights RPC with min_right 'super_admin'
alt check_min_rights OK
OrgService->>DB: SELECT audit_logs (RLS applies)
DB-->>Client: audit log rows
else insufficient rights
OrgService-->>Client: 400 invalid_org_id
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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)
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 (4.0.0)supabase/migrations/20260204100000_restore_audit_logs_apikey.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 |
![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: 4f5554ea93
ℹ️ 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".
| if (!apikeyHasOrgRight(auth.apikey, body.orgId)) { | ||
| throw simpleError('invalid_org_id', 'You can\'t access this organization', { org_id: body.orgId }) | ||
| } | ||
|
|
||
| const hasRight = await hasOrgRightApikey(c, body.orgId, auth.userId, 'super_admin', c.get('capgkey') as string) |
There was a problem hiding this comment.
This new API key path only checks limited_to_orgs and RBAC via hasOrgRightApikey, but it never enforces the org’s API key policy (e.g., require_apikey_expiration). As a result, a non‑expiring API key can now read audit logs for an org that requires expiring keys, whereas other org endpoints gate API keys through apikeyHasOrgRightWithPolicy/checkApikeyMeetsOrgPolicy. This is a permission regression for orgs that mandate expiring keys.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This pull request restores API key authentication support for the audit logs endpoint that was inadvertently removed in a previous optimization. The changes include updating the database RLS policy, backend endpoint middleware, and adding comprehensive SQL tests.
Changes:
- Restored RLS policy on audit_logs table to allow both JWT (authenticated) and API key (anon) access with proper authorization checks
- Updated the audit logs endpoint to use
middlewareV2instead ofmiddlewareAuthto support both authentication methods - Enhanced authorization logic in audit.ts to handle both JWT and API key authentication flows
- Added comprehensive SQL tests to verify audit log creation with API key authentication
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| supabase/migrations/20260204100000_restore_audit_logs_apikey.sql | Grants SELECT on audit_logs to anon role and recreates RLS policy to support both authenticated and anon (API key) access |
| supabase/tests/40_test_audit_log_apikey.sql | Adds comprehensive SQL tests verifying that audit log triggers correctly capture user_id from API keys and that old_record/new_record data is properly stored |
| supabase/functions/_backend/public/organization/index.ts | Updates audit endpoint to use middlewareV2 instead of middlewareAuth to support both JWT and API key authentication |
| supabase/functions/_backend/public/organization/audit.ts | Adds branching logic to handle both API key and JWT authentication, with appropriate authorization checks for each auth type |
| if (!apikeyHasOrgRight(auth.apikey, body.orgId)) { | ||
| throw simpleError('invalid_org_id', 'You can\'t access this organization', { org_id: body.orgId }) | ||
| } | ||
|
|
||
| const hasRight = await hasOrgRightApikey(c, body.orgId, auth.userId, 'super_admin', c.get('capgkey') as string) | ||
| if (!hasRight) { | ||
| throw simpleError('invalid_org_id', 'You can\'t access this organization', { org_id: body.orgId }) | ||
| } |
There was a problem hiding this comment.
Consider adding a comment explaining why both apikeyHasOrgRight (line 46) and hasOrgRightApikey (line 50) checks are necessary. The first checks if the API key is scoped to the org, while the second checks if the user has super_admin rights via RLS. This redundancy might not be obvious to future maintainers.
| throw simpleError('invalid_org_id', 'You can\'t access this organization', { org_id: body.orgId }) | ||
| } | ||
|
|
||
| const hasRight = await hasOrgRightApikey(c, body.orgId, auth.userId, 'super_admin', c.get('capgkey') as string) |
There was a problem hiding this comment.
The type assertion as string assumes that capgkey is always set when API key authentication succeeds. While this is currently guaranteed by the middleware, consider adding a runtime check or using optional chaining to make the code more defensive against future refactoring. For example: const capgkey = c.get('capgkey'); if (!capgkey) { throw simpleError('not_authorized', 'Not authorized') }
| const hasRight = await hasOrgRightApikey(c, body.orgId, auth.userId, 'super_admin', c.get('capgkey') as string) | |
| const capgkey = c.get('capgkey') | |
| if (!capgkey || typeof capgkey !== 'string') { | |
| throw simpleError('not_authorized', 'Not authorized') | |
| } | |
| const hasRight = await hasOrgRightApikey(c, body.orgId, auth.userId, 'super_admin', capgkey) |
riderx
force-pushed
the
riderx/audit-log-apikey
branch
from
4f5554e to
1945870
Compare
![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
🤖 Fix all issues with AI agents
In `@supabase/functions/_backend/public/organization/audit.ts`:
- Around line 41-53: The apikey path currently skips verifying the API key's
owner org so a key tied to one org can be used to access others; update the
auth.apikey flow (where auth.authType === 'apikey') to first resolve the key's
owner_org from the apikeys table (or via existing apikey lookup) and verify
body.orgId matches owner_org before calling apikeyHasOrgRight/hasOrgRightApikey;
also ensure internal endpoints use middlewareAPISecret and external keys use
middlewareKey when validating the key, and fail with
simpleError('invalid_org_id', ...) if owner_org does not match.
In `@supabase/tests/40_test_audit_log_apikey.sql`:
- Around line 6-17: The test embeds a hard-coded API key literal
("ae6e7458-c46d-4c00-aa3b-153b0b8520ea") in the set_config('request.headers',
'{"capgkey": "..."}') call which trips Gitleaks; either add this key to the
repo's Gitleaks allowlist or replace the literal with a test fixture/variable
(e.g. read from an env var or a test_data table) and update the DO block to use
that variable instead so the API key is not stored directly in the SQL; ensure
the symbol capgkey and the set_config('request.headers', ...) usage are updated
accordingly.
- Around line 66-90: The audit-log assertion is too broad; capture the specific
app_versions record id when updating (e.g., SELECT id INTO v_updated_id FROM
public.app_versions WHERE name='99.0.0-test-audit' AND app_id='com.demo.app' or
use UPDATE ... RETURNING id INTO v_updated_id) and then include AND record_id =
v_updated_id in the WHERE of the audit_logs COUNT check (still keep
table_name='app_versions', operation='UPDATE', user_id matching and 'comment' =
ANY(changed_fields)) so the test only counts audit rows for the updated record.
| -- Use existing seed identities: | ||
| -- API key: ae6e7458-c46d-4c00-aa3b-153b0b8520ea (mode: all, user: 6aa76066-55ef-4238-ade6-0b32334a4097) | ||
| -- Org: 046a36ac-e03c-4590-9257-bd6c9dba9ee8 | ||
| -- App: com.demo.app | ||
|
|
||
| SELECT plan(6); | ||
|
|
||
| -- Test 1: Verify get_identity returns user_id when API key header is set | ||
| DO $$ | ||
| BEGIN | ||
| PERFORM set_config('request.headers', '{"capgkey": "ae6e7458-c46d-4c00-aa3b-153b0b8520ea"}', true); | ||
| END $$; |
There was a problem hiding this comment.
Mask/allowlist the hard‑coded API key to avoid Gitleaks failures.
Static analysis already flags this literal. Please allowlist the test key in the repo’s gitleaks config or move it behind a test fixture/variable so CI doesn’t fail and secrets aren’t embedded in code.
🧰 Tools
🪛 Gitleaks (8.30.0)
[high] 7-7: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 16-16: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🤖 Prompt for AI Agents
In `@supabase/tests/40_test_audit_log_apikey.sql` around lines 6 - 17, The test
embeds a hard-coded API key literal ("ae6e7458-c46d-4c00-aa3b-153b0b8520ea") in
the set_config('request.headers', '{"capgkey": "..."}') call which trips
Gitleaks; either add this key to the repo's Gitleaks allowlist or replace the
literal with a test fixture/variable (e.g. read from an env var or a test_data
table) and update the DO block to use that variable instead so the API key is
not stored directly in the SQL; ensure the symbol capgkey and the
set_config('request.headers', ...) usage are updated accordingly.
| -- Test 4: Update app_version with API key context and verify audit log is created | ||
| DO $$ | ||
| DECLARE | ||
| v_audit_count int; | ||
| BEGIN | ||
| -- Set API key context | ||
| PERFORM set_config('request.headers', '{"capgkey": "ae6e7458-c46d-4c00-aa3b-153b0b8520ea"}', true); | ||
|
|
||
| -- Update the app_version | ||
| UPDATE public.app_versions | ||
| SET comment = 'Updated via API key test' | ||
| WHERE name = '99.0.0-test-audit' AND app_id = 'com.demo.app'; | ||
|
|
||
| -- Check that an audit log was created for the UPDATE | ||
| SELECT COUNT(*) INTO v_audit_count | ||
| FROM public.audit_logs | ||
| WHERE table_name = 'app_versions' | ||
| AND operation = 'UPDATE' | ||
| AND user_id = '6aa76066-55ef-4238-ade6-0b32334a4097' | ||
| AND 'comment' = ANY(changed_fields); | ||
|
|
||
| IF v_audit_count = 0 THEN | ||
| RAISE EXCEPTION 'No audit log created for app_version UPDATE with API key'; | ||
| END IF; | ||
|
|
There was a problem hiding this comment.
Make UPDATE audit_log assertion specific to the updated record.
The current COUNT can be satisfied by unrelated audit rows for the same user/table. Filter by the updated record_id to avoid false positives.
♻️ Suggested fix
DO $$
DECLARE
+ v_version_id bigint;
v_audit_count int;
BEGIN
-- Set API key context
PERFORM set_config('request.headers', '{"capgkey": "ae6e7458-c46d-4c00-aa3b-153b0b8520ea"}', true);
+ SELECT id INTO v_version_id
+ FROM public.app_versions
+ WHERE name = '99.0.0-test-audit' AND app_id = 'com.demo.app';
+
-- Update the app_version
UPDATE public.app_versions
SET comment = 'Updated via API key test'
WHERE name = '99.0.0-test-audit' AND app_id = 'com.demo.app';
-- Check that an audit log was created for the UPDATE
SELECT COUNT(*) INTO v_audit_count
FROM public.audit_logs
WHERE table_name = 'app_versions'
+ AND record_id = v_version_id::text
AND operation = 'UPDATE'
AND user_id = '6aa76066-55ef-4238-ade6-0b32334a4097'
AND 'comment' = ANY(changed_fields);📝 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.
| -- Test 4: Update app_version with API key context and verify audit log is created | |
| DO $$ | |
| DECLARE | |
| v_audit_count int; | |
| BEGIN | |
| -- Set API key context | |
| PERFORM set_config('request.headers', '{"capgkey": "ae6e7458-c46d-4c00-aa3b-153b0b8520ea"}', true); | |
| -- Update the app_version | |
| UPDATE public.app_versions | |
| SET comment = 'Updated via API key test' | |
| WHERE name = '99.0.0-test-audit' AND app_id = 'com.demo.app'; | |
| -- Check that an audit log was created for the UPDATE | |
| SELECT COUNT(*) INTO v_audit_count | |
| FROM public.audit_logs | |
| WHERE table_name = 'app_versions' | |
| AND operation = 'UPDATE' | |
| AND user_id = '6aa76066-55ef-4238-ade6-0b32334a4097' | |
| AND 'comment' = ANY(changed_fields); | |
| IF v_audit_count = 0 THEN | |
| RAISE EXCEPTION 'No audit log created for app_version UPDATE with API key'; | |
| END IF; | |
| -- Test 4: Update app_version with API key context and verify audit log is created | |
| DO $$ | |
| DECLARE | |
| v_version_id bigint; | |
| v_audit_count int; | |
| BEGIN | |
| -- Set API key context | |
| PERFORM set_config('request.headers', '{"capgkey": "ae6e7458-c46d-4c00-aa3b-153b0b8520ea"}', true); | |
| SELECT id INTO v_version_id | |
| FROM public.app_versions | |
| WHERE name = '99.0.0-test-audit' AND app_id = 'com.demo.app'; | |
| -- Update the app_version | |
| UPDATE public.app_versions | |
| SET comment = 'Updated via API key test' | |
| WHERE name = '99.0.0-test-audit' AND app_id = 'com.demo.app'; | |
| -- Check that an audit log was created for the UPDATE | |
| SELECT COUNT(*) INTO v_audit_count | |
| FROM public.audit_logs | |
| WHERE table_name = 'app_versions' | |
| AND record_id = v_version_id::text | |
| AND operation = 'UPDATE' | |
| AND user_id = '6aa76066-55ef-4238-ade6-0b32334a4097' | |
| AND 'comment' = ANY(changed_fields); | |
| IF v_audit_count = 0 THEN | |
| RAISE EXCEPTION 'No audit log created for app_version UPDATE with API key'; | |
| END IF; | |
🧰 Tools
🪛 Gitleaks (8.30.0)
[high] 72-72: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🤖 Prompt for AI Agents
In `@supabase/tests/40_test_audit_log_apikey.sql` around lines 66 - 90, The
audit-log assertion is too broad; capture the specific app_versions record id
when updating (e.g., SELECT id INTO v_updated_id FROM public.app_versions WHERE
name='99.0.0-test-audit' AND app_id='com.demo.app' or use UPDATE ... RETURNING
id INTO v_updated_id) and then include AND record_id = v_updated_id in the WHERE
of the audit_logs COUNT check (still keep table_name='app_versions',
operation='UPDATE', user_id matching and 'comment' = ANY(changed_fields)) so the
test only counts audit rows for the updated record.
riderx
force-pushed
the
riderx/audit-log-apikey
branch
from
bf1047b to
d007417
Compare
|
Summary (AI generated)
Test plan (AI generated)
Screenshots (AI generated)
Checklist (AI generated)
bun run lint:backend && bun run lint.Generated with AI
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores