fix(audit-logs): capture user_id for API key authenticated operations by riderx · Pull Request #1305 · Cap-go/capgo

riderx · 2025-12-28T06:41:20Z

Summary

Fixed issue where app versions created via CLI/API were not appearing in audit logs. The audit_log_trigger was calling get_identity() without parameters, which only checks JWT authentication. API key authenticated users had NULL user_id in audit logs. Fixed by passing key_mode parameter to get_identity().

Test plan

SQL tests verify INSERT/UPDATE/DELETE on app_versions with API key context creates audit logs
E2E tests verify audit logs contain correct user_id from API key for bundle operations
All existing audit log tests continue to pass

Checklist

My code follows the code style of this project and passes bun run lint:backend && bun run lint.
My change requires a change to the documentation.
My change has adequate E2E test coverage.

Summary by CodeRabbit

Bug Fixes
- Fixed audit log user identification for API key authentication. Audit logs now correctly attribute database operations to users authenticating via API keys, matching JWT-based authentication behavior.
- Enhanced audit log accuracy to prevent missing or incorrectly attributed operation records across all authentication methods.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

The audit_log_trigger was calling get_identity() without parameters, which only checks auth.uid() (JWT authentication). This caused API key authenticated users (CLI/API) to have NULL user_id in audit logs. Fixed by passing key_mode parameter to get_identity() to also check for API key authentication. Added SQL test (40_test_audit_log_apikey.sql) and end-to-end tests verifying that INSERT/UPDATE/DELETE operations on app_versions via API key are properly logged with the correct user_id. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>

coderabbitai · 2025-12-28T06:41:32Z

Warning

Rate limit exceeded

@riderx has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 19 minutes and 26 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between d640db4 and f927ead.

📒 Files selected for processing (1)

tests/audit-logs.test.ts

📝 Walkthrough

Walkthrough

This change fixes audit log user identification for API key authentication by replacing the audit trigger function to correctly call get_identity with a key_mode parameter. It includes comprehensive SQL and TypeScript tests validating INSERT, UPDATE, and DELETE operations on audit logs when using API key authentication.

Changes

Cohort / File(s)	Summary
Audit Log Trigger Function `supabase/migrations/20251228063320_fix_audit_log_apikey.sql`	Introduces new PostgreSQL trigger function `audit_log_trigger()` that correctly identifies users authenticated via API keys by calling `get_identity` with key_mode parameter; skips org DELETE logging; handles DELETE, INSERT, UPDATE operations; converts OLD/NEW rows to JSONB; computes changed_fields for updates; derives org_id and record_id based on modified table; validates org existence before logging
Test Suites `supabase/tests/40_test_audit_log_apikey.sql`, `tests/audit-logs.test.ts`	SQL test script validates `get_identity` behavior with/without key_mode parameter and INSERT/UPDATE/DELETE operations on app_versions; TypeScript test suite extended to verify audit logs for appVersions created/modified/soft-deleted via API key, including user_id extraction from key context and changed_fields validation

Sequence Diagram

sequenceDiagram
    participant Client as Client (API Key)
    participant Trigger as audit_log_trigger()
    participant GetIdentity as get_identity(key_mode)
    participant AuditLogs as audit_logs table
    participant OrgTable as orgs table

    Client->>Trigger: INSERT/UPDATE/DELETE operation
    Note over Trigger: Trigger fires<br/>(except org deletes)
    Trigger->>GetIdentity: Call with key_mode<br/>(for API key auth)
    GetIdentity-->>Trigger: Return user_id
    alt User identified
        Trigger->>OrgTable: Check org existence
        alt Org exists
            Trigger->>AuditLogs: Insert audit log entry<br/>(table, operation, user_id,<br/>org_id, old/new records,<br/>changed_fields)
            AuditLogs-->>Trigger: Logged
        end
    end
    Trigger-->>Client: Return NEW/OLD

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A hop and a key, now audit logs sing,
API keys reveal who did the thing!
Changed fields tracked, old records kept near,
No more mystery—the truth's crystal clear! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: fixing audit logs to capture user_id for API key authenticated operations.
Description check	✅ Passed	The description provides a clear summary of the fix, test plan with specific test coverage details, and a mostly complete checklist; only documentation-related items are unchecked as not applicable.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

supabase/migrations/20251228063320_fix_audit_log_apikey.sql (1)
51-57: Consider tracking removed fields for completeness.

The current implementation only iterates over jsonb_object_keys(v_new_record), which means if a field exists in OLD but is removed in NEW, it won't appear in changed_fields. For most use cases this is fine, but if tracking column removal is important, you may want to also iterate over v_old_record keys.
🔎 Optional enhancement to detect removed keys
     -- Calculate changed fields by comparing old and new values
     FOR v_key IN SELECT jsonb_object_keys(v_new_record)
     LOOP
       IF v_old_record->v_key IS DISTINCT FROM v_new_record->v_key THEN
         v_changed_fields := array_append(v_changed_fields, v_key);
       END IF;
     END LOOP;
+    
+    -- Also check for keys that exist in old but not in new (removed fields)
+    FOR v_key IN SELECT jsonb_object_keys(v_old_record)
+    LOOP
+      IF NOT v_new_record ? v_key THEN
+        v_changed_fields := array_append(v_changed_fields, v_key);
+      END IF;
+    END LOOP;

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6a1b355 and d640db4.

📒 Files selected for processing (3)

supabase/migrations/20251228063320_fix_audit_log_apikey.sql
supabase/tests/40_test_audit_log_apikey.sql
tests/audit-logs.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/20251228063320_fix_audit_log_apikey.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/20251228063320_fix_audit_log_apikey.sql
supabase/tests/40_test_audit_log_apikey.sql
tests/audit-logs.test.ts

supabase/migrations/*.sql

📄 CodeRabbit inference engine (AGENTS.md)

supabase/migrations/*.sql: When creating schema changes, use supabase 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 the process_all_cron_tasks function in a new migration file to add your job if needed

Files:

supabase/migrations/20251228063320_fix_audit_log_apikey.sql

**/*.{ts,tsx,js,jsx,vue}

📄 CodeRabbit inference engine (CLAUDE.md)

Use single quotes and no semicolons per @antfu/eslint-config

Files:

tests/audit-logs.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/audit-logs.test.ts

**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Use TypeScript strict mode with path aliases mapping ~/ to src/

Files:

tests/audit-logs.test.ts

tests/**/*.{ts,js}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

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

Files:

tests/audit-logs.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

📚 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/20251228063320_fix_audit_log_apikey.sql

📚 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/20251228063320_fix_audit_log_apikey.sql
tests/audit-logs.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/20251228063320_fix_audit_log_apikey.sql
supabase/tests/40_test_audit_log_apikey.sql
tests/audit-logs.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/20251228063320_fix_audit_log_apikey.sql

📚 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/20251228063320_fix_audit_log_apikey.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/audit-logs.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/audit-logs.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/audit-logs.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/audit-logs.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/audit-logs.test.ts

🧬 Code graph analysis (1)

tests/audit-logs.test.ts (1)

tests/test-utils.ts (3)

getSupabaseClient (324-343)

BASE_URL (19-19)

USER_ID (45-45)

🪛 GitHub Actions: Run tests

tests/audit-logs.test.ts

[error] 5-5: vue-tsc --noEmit failed with TS6133: 'APIKEY_TEST_ALL' is declared but its value is never read.

🪛 Gitleaks (8.30.0)

supabase/tests/40_test_audit_log_apikey.sql

[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)

[high] 42-42: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

[high] 72-72: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

[high] 103-103: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

[high] 138-138: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

🔇 Additional comments (14)

supabase/tests/40_test_audit_log_apikey.sql (6)

1-11: LGTM! Well-structured test setup.

The test file properly documents the bug being fixed, references seed data identities, and plans for 6 tests. The use of BEGIN/ROLLBACK ensures test isolation. The Gitleaks warnings are false positives—these are test fixture UUIDs from seed data, not actual secrets.

26-33: Good documentation of the original bug behavior.

Test 2 explicitly demonstrates the pre-fix bug where parameterless get_identity() returns NULL for API key auth. This serves as regression documentation and helps future developers understand why the key_mode parameter is required.

35-64: LGTM!

The INSERT test correctly validates that audit logs are created with the proper user_id from API key context. The pattern of using RAISE EXCEPTION in the DO block followed by SELECT ok(true, ...) is appropriate for pgTAP testing.

66-94: LGTM!

The UPDATE test properly validates that changed_fields tracking works with API key authentication. The query filtering is sufficient given the transaction isolation.

96-129: LGTM!

The DELETE test correctly captures the version_id before deletion and uses it to verify the audit log entry. This is the proper pattern for testing DELETE audit logs.

131-202: LGTM! Comprehensive record content verification.

This test thoroughly validates the old_record and new_record JSONB fields contain the expected data. The explicit DELETE on line 197 is technically redundant given the ROLLBACK at the end, but it doesn't cause issues.

tests/audit-logs.test.ts (4)

340-351: LGTM! Good test setup and cleanup pattern.

The test suite correctly uses a unique version name per run and cleans up both the test version and related audit logs in afterAll. This prevents test pollution across runs.

353-404: LGTM! This test validates the core fix.

The assertion on line 396 (expect(versionAuditLog.user_id).toBe(USER_ID)) is the key validation that API key authentication now correctly populates user_id in audit logs. The test properly creates a bundle via the API and verifies the resulting audit log entry.

406-458: LGTM!

Good defensive check to skip if the prior INSERT test failed. The test correctly validates that UPDATE operations via API key are properly logged with the user's identity.

460-518: LGTM! Excellent soft-delete test.

The test correctly recognizes that soft-deletes (setting deleted=true) trigger UPDATE operations rather than DELETE. The validation of changed_fields containing 'deleted' and verifying new_record.deleted === true provides thorough coverage of the soft-delete audit trail.

supabase/migrations/20251228063320_fix_audit_log_apikey.sql (4)

6-11: LGTM! Good security configuration.

The function correctly uses SECURITY DEFINER with SET search_path = '' to prevent search path injection attacks. This is the recommended pattern for privileged trigger functions.

29-38: This is the core fix - correctly implemented.

Passing '{read,upload,write,all}'::public.key_mode[] to get_identity() enables it to check API key authentication in addition to JWT auth. The early return with COALESCE(NEW, OLD) when no user is identified ensures the trigger doesn't block operations while skipping audit logging for unauthenticated contexts (e.g., system operations).

60-81: LGTM! Comprehensive table handling.

The CASE statement correctly extracts org_id and record_id for each audited table. The COALESCE(NEW.*, OLD.*) pattern properly handles all operation types (INSERT has no OLD, DELETE has no NEW).

83-98: LGTM! Defensive org existence check.

The EXISTS check prevents foreign key violations when audit logging child table changes after the parent org has been deleted. This is a robust approach to handle edge cases in deletion cascades.

tests/audit-logs.test.ts

🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>

sonarqubecloud · 2025-12-28T06:57:45Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

…#1305) * fix(audit-logs): capture user_id for API key authenticated operations The audit_log_trigger was calling get_identity() without parameters, which only checks auth.uid() (JWT authentication). This caused API key authenticated users (CLI/API) to have NULL user_id in audit logs. Fixed by passing key_mode parameter to get_identity() to also check for API key authentication. Added SQL test (40_test_audit_log_apikey.sql) and end-to-end tests verifying that INSERT/UPDATE/DELETE operations on app_versions via API key are properly logged with the correct user_id. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * fix: remove unused APIKEY_TEST_ALL import 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Haiku 4.5 <noreply@anthropic.com>

coderabbitai bot reviewed Dec 28, 2025

View reviewed changes

tests/audit-logs.test.ts Outdated Show resolved Hide resolved

fix: remove unused APIKEY_TEST_ALL import

f927ead

🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>

riderx merged commit 9135d8a into main Dec 28, 2025
11 checks passed

riderx deleted the riderx/audit-log-missing branch December 28, 2025 07:02

coderabbitai bot mentioned this pull request Jan 2, 2026

fix: support hashed API keys in RLS identity functions #1351

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(audit-logs): capture user_id for API key authenticated operations#1305

fix(audit-logs): capture user_id for API key authenticated operations#1305
riderx merged 2 commits intomainfrom
riderx/audit-log-missing

riderx commented Dec 28, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Dec 28, 2025 •

edited

Loading

Rate limit exceeded

Walkthrough

Changes

Sequence Diagram

Estimated code review effort

Poem

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

sonarqubecloud bot commented Dec 28, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

riderx commented Dec 28, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Checklist

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Dec 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rate limit exceeded

Walkthrough

Changes

Sequence Diagram

Estimated code review effort

Poem

Pre-merge checks and finishing touches

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sonarqubecloud bot commented Dec 28, 2025

Quality Gate passed

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

riderx commented Dec 28, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Dec 28, 2025 •

edited

Loading