[codex] Add actor attribution to audit logs#2462
Conversation
📝 WalkthroughWalkthroughThis PR introduces audit log actor attribution, tracking whether changes originated from a user, API key, or system, plus org-id tombstoning to prevent UUID reuse while retaining audit history. The backend migration auto-generates audit logs via triggers; the frontend displays actor information in a new column and modal section. ChangesAudit Log Actor Attribution
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
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.2.1)supabase/tests/40_test_audit_log_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: supabase/migrations/20260608160711_audit_log_actor_attribution.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: supabase/tests/29_test_delete_accounts_marked_for_deletion.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: Comment |
Merging this PR will not alter performance
Comparing 
Footnotes
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
![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: c26343e090
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 478246fcda
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?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: 8f2effed3d
ℹ️ 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".
| UPDATE "public"."audit_logs" | ||
| SET | ||
| "actor_type" = CASE WHEN "user_id" IS NULL THEN 'unknown' ELSE 'user' END, | ||
| "actor_user_id" = "user_id" |
There was a problem hiding this comment.
For historical rows created before actor_type existed, a non-null user_id does not prove a human user made the change: the old trigger populated user_id from get_identity(), which also returned the API-key owner for API-key requests. With this backfill, existing API-key mutations are permanently shown as actor_type = 'user' in the audit UI/API instead of unknown or otherwise distinguishable, so customers reviewing retained audit logs will see misleading actor attribution for all pre-migration API-key activity.
Useful? React with 👍 / 👎.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/components/tables/AuditLogTable.vue`:
- Around line 253-255: The getActorUserEmail helper currently returns
actor_user_email or user?.email unconditionally; change
getActorUserEmail(ExtendedAuditLog) so it only returns those values when the
audit item represents a user actor (e.g., item.actor_type === 'user'), otherwise
return null; likewise update places that fallback to actor_user_id / user_id
(calls/usages of getActorUserEmail and any inline fallbacks) to only use those
user-specific fields when actor_type === 'user' to prevent showing user
emails/IDs for non-user actors (API key/system).
In `@supabase/migrations/20260608160711_audit_log_actor_attribution.sql`:
- Around line 358-500: The SECURITY DEFINER function
public.delete_accounts_marked_for_deletion() is currently executable by any
role; revoke public execute and explicitly grant EXECUTE only to trusted
roles/groups (e.g., your admin role(s)) to prevent privilege escalation. Add
statements after the function definition that REVOKE EXECUTE ON FUNCTION
public.delete_accounts_marked_for_deletion() FROM PUBLIC; and then GRANT EXECUTE
ON FUNCTION public.delete_accounts_marked_for_deletion() TO <trusted_role1>[,
<trusted_role2>] (replace with your actual admin/trusted role names), ensuring
the function remains owned by postgres.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 68f48908-b20b-4600-ad4e-1b872de3c325
📒 Files selected for processing (11)
messages/en.jsonsrc/components/tables/AuditLogTable.vuesrc/types/supabase.types.tssupabase/functions/_backend/public/organization/audit.tssupabase/functions/_backend/utils/supabase.types.tssupabase/functions/_backend/utils/webhook.tssupabase/migrations/20260608160711_audit_log_actor_attribution.sqlsupabase/seed.sqlsupabase/tests/29_test_delete_accounts_marked_for_deletion.sqlsupabase/tests/40_test_audit_log_apikey.sqltests/audit-logs.test.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Cap-go/capacitor-updater(manual)
| function getActorUserEmail(item: ExtendedAuditLog): string | null { | ||
| return item.actor_user_email || item.user?.email || null | ||
| } |
There was a problem hiding this comment.
Restrict member/email fallback to user actors only.
Current fallback (actor_user_id || user_id + unconditional getActorUserEmail(...) render) can display user email for non-user actors (API key/system), which conflicts with the actor attribution model and can mislead incident/support analysis.
Proposed fix
function getActorUserEmail(item: ExtendedAuditLog): string | null {
- return item.actor_user_email || item.user?.email || null
+ if (item.actor_type !== 'user')
+ return null
+ return item.actor_user_email || item.user?.email || null
}
@@
const rows = (data ?? []) as ExtendedAuditLog[]
for (const item of rows) {
- const memberId = item.actor_user_id || item.user_id
+ const memberId = item.actor_type === 'user'
+ ? (item.actor_user_id || item.user_id)
+ : null
if (memberId) {
const member = membersMap.value.get(memberId)
if (member) {
item.user = member
}
}
}Also applies to: 356-357, 637-640
🤖 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 `@src/components/tables/AuditLogTable.vue` around lines 253 - 255, The
getActorUserEmail helper currently returns actor_user_email or user?.email
unconditionally; change getActorUserEmail(ExtendedAuditLog) so it only returns
those values when the audit item represents a user actor (e.g., item.actor_type
=== 'user'), otherwise return null; likewise update places that fallback to
actor_user_id / user_id (calls/usages of getActorUserEmail and any inline
fallbacks) to only use those user-specific fields when actor_type === 'user' to
prevent showing user emails/IDs for non-user actors (API key/system).
| CREATE OR REPLACE FUNCTION "public"."delete_accounts_marked_for_deletion"() RETURNS TABLE("deleted_count" integer, "deleted_user_ids" "uuid"[]) | ||
| LANGUAGE "plpgsql" SECURITY DEFINER | ||
| SET search_path = '' | ||
| AS $$ | ||
| DECLARE | ||
| account_record RECORD; | ||
| org_record RECORD; | ||
| deleted_users UUID[] := ARRAY[]::UUID[]; | ||
| total_deleted INTEGER := 0; | ||
| other_super_admins_count INTEGER; | ||
| replacement_owner_id UUID; | ||
| BEGIN | ||
| -- Loop through all accounts marked for deletion where removal_date has passed | ||
| FOR account_record IN | ||
| SELECT "account_id", "removal_date", "removed_data" | ||
| FROM "public"."to_delete_accounts" | ||
| WHERE "removal_date" < NOW() | ||
| LOOP | ||
| BEGIN | ||
| -- Process each org the user belongs to | ||
| FOR org_record IN | ||
| SELECT DISTINCT "org_id", "user_right" | ||
| FROM "public"."org_users" | ||
| WHERE "user_id" = account_record.account_id | ||
| LOOP | ||
| -- Reset replacement_owner_id for each org | ||
| replacement_owner_id := NULL; | ||
|
|
||
| -- Check if user is a super_admin in this org | ||
| IF org_record.user_right = 'super_admin'::"public"."user_min_right" THEN | ||
| -- Count other super_admins in this org (excluding the user being deleted) | ||
| SELECT COUNT(*) INTO other_super_admins_count | ||
| FROM "public"."org_users" | ||
| WHERE "org_id" = org_record.org_id | ||
| AND "user_id" != account_record.account_id | ||
| AND "user_right" = 'super_admin'::"public"."user_min_right"; | ||
|
|
||
| IF other_super_admins_count = 0 THEN | ||
| -- User is the last super_admin: DELETE all org resources | ||
| RAISE NOTICE 'User % is last super_admin of org %. Deleting all org resources.', | ||
| account_record.account_id, org_record.org_id; | ||
|
|
||
| -- Delete deploy_history for this org | ||
| DELETE FROM "public"."deploy_history" WHERE "owner_org" = org_record.org_id; | ||
|
|
||
| -- Delete channel_devices for this org | ||
| DELETE FROM "public"."channel_devices" WHERE "owner_org" = org_record.org_id; | ||
|
|
||
| -- Delete channels for this org | ||
| DELETE FROM "public"."channels" WHERE "owner_org" = org_record.org_id; | ||
|
|
||
| -- Delete app_versions for this org | ||
| DELETE FROM "public"."app_versions" WHERE "owner_org" = org_record.org_id; | ||
|
|
||
| -- Delete apps for this org | ||
| DELETE FROM "public"."apps" WHERE "owner_org" = org_record.org_id; | ||
|
|
||
| -- Delete the org itself since user is last super_admin. Audit logs | ||
| -- intentionally keep their org_id snapshot without a foreign key. | ||
| DELETE FROM "public"."orgs" WHERE "id" = org_record.org_id; | ||
|
|
||
| -- Skip ownership transfer since all resources are deleted | ||
| CONTINUE; | ||
| END IF; | ||
| END IF; | ||
|
|
||
| -- If we reach here, we need to transfer ownership (either non-super_admin or non-last super_admin) | ||
| -- Find a super_admin to transfer ownership to | ||
| SELECT "user_id" INTO replacement_owner_id | ||
| FROM "public"."org_users" | ||
| WHERE "org_id" = org_record.org_id | ||
| AND "user_id" != account_record.account_id | ||
| AND "user_right" = 'super_admin'::"public"."user_min_right" | ||
| LIMIT 1; | ||
|
|
||
| IF replacement_owner_id IS NOT NULL THEN | ||
| RAISE NOTICE 'Transferring ownership from user % to user % in org %', | ||
| account_record.account_id, replacement_owner_id, org_record.org_id; | ||
|
|
||
| -- Transfer app ownership | ||
| UPDATE "public"."apps" | ||
| SET "user_id" = replacement_owner_id, "updated_at" = NOW() | ||
| WHERE "user_id" = account_record.account_id AND "owner_org" = org_record.org_id; | ||
|
|
||
| -- Transfer app_versions ownership | ||
| UPDATE "public"."app_versions" | ||
| SET "user_id" = replacement_owner_id, "updated_at" = NOW() | ||
| WHERE "user_id" = account_record.account_id AND "owner_org" = org_record.org_id; | ||
|
|
||
| -- Transfer channels ownership | ||
| UPDATE "public"."channels" | ||
| SET "created_by" = replacement_owner_id, "updated_at" = NOW() | ||
| WHERE "created_by" = account_record.account_id AND "owner_org" = org_record.org_id; | ||
|
|
||
| -- Transfer deploy_history ownership | ||
| UPDATE "public"."deploy_history" | ||
| SET "created_by" = replacement_owner_id, "updated_at" = NOW() | ||
| WHERE "created_by" = account_record.account_id AND "owner_org" = org_record.org_id; | ||
|
|
||
| -- Transfer org ownership if user created it | ||
| UPDATE "public"."orgs" | ||
| SET "created_by" = replacement_owner_id, "updated_at" = NOW() | ||
| WHERE "id" = org_record.org_id AND "created_by" = account_record.account_id; | ||
| ELSE | ||
| RAISE WARNING 'No super_admin found to transfer ownership in org % for user %', | ||
| org_record.org_id, account_record.account_id; | ||
| END IF; | ||
| END LOOP; | ||
|
|
||
| -- Delete from public.users table | ||
| DELETE FROM "public"."users" WHERE "id" = account_record.account_id; | ||
|
|
||
| -- Delete from auth.users table | ||
| DELETE FROM "auth"."users" WHERE "id" = account_record.account_id; | ||
|
|
||
| -- Remove from to_delete_accounts table | ||
| DELETE FROM "public"."to_delete_accounts" WHERE "account_id" = account_record.account_id; | ||
|
|
||
| -- Track the deleted user | ||
| deleted_users := "array_append"(deleted_users, account_record.account_id); | ||
| total_deleted := total_deleted + 1; | ||
|
|
||
| -- Log the deletion | ||
| RAISE NOTICE 'Successfully deleted account: % (removal date: %)', | ||
| account_record.account_id, account_record.removal_date; | ||
|
|
||
| EXCEPTION | ||
| WHEN OTHERS THEN | ||
| -- Log the error but continue with other accounts | ||
| RAISE WARNING 'Failed to delete account %: %', account_record.account_id, SQLERRM; | ||
| END; | ||
| END LOOP; | ||
|
|
||
| -- Return results | ||
| deleted_count := total_deleted; | ||
| deleted_user_ids := deleted_users; | ||
| RETURN NEXT; | ||
|
|
||
| RAISE NOTICE 'Deletion process completed. Total accounts deleted: %', total_deleted; | ||
| END; | ||
| $$; | ||
|
|
||
| ALTER FUNCTION "public"."delete_accounts_marked_for_deletion"() OWNER TO "postgres"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
psql "$DATABASE_URL" -X -A -F $'\t' -c "
SELECT grantee, privilege_type
FROM information_schema.routine_privileges
WHERE routine_schema = 'public'
AND routine_name = 'delete_accounts_marked_for_deletion'
ORDER BY grantee, privilege_type;"Repository: Cap-go/capgo
Length of output: 101
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="supabase/migrations/20260608160711_audit_log_actor_attribution.sql"
# Ensure file exists
ls -l "$FILE"
# Show nearby parts where the function is defined and where grants/revokes might appear
rg -n "delete_accounts_marked_for_deletion" "$FILE"
# Extract a window around the function end/ALTER FUNCTION line(s)
# (Use line numbers from rg output to focus; fallback to approximate range.)
python3 - <<'PY'
import re
path="supabase/migrations/20260608160711_audit_log_actor_attribution.sql"
with open(path,'r',encoding='utf-8') as f:
lines=f.readlines()
# find indices containing function name
idx=[i for i,l in enumerate(lines) if "delete_accounts_marked_for_deletion" in l]
print("matches:", idx)
if idx:
i=idx[-1]
start=max(0,i-120)
end=min(len(lines), i+120)
for j in range(start,end):
print(f"{j+1}\t{lines[j].rstrip()}")
PY
# Also search for grants/revokes for that specific function name in the repo migration file
rg -n "GRANT EXECUTE|REVOKE ALL ON FUNCTION|REVOKE EXECUTE|GRANT ON FUNCTION" "$FILE" || true
# Also check whether there are any generic post-creation GRANT/REVOKE patterns for SECURITY DEFINER functions in this migration
rg -n "SECURITY DEFINER|REVOKE|GRANT" "$FILE" | head -n 200Repository: Cap-go/capgo
Length of output: 7231
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="supabase/migrations/20260608160711_audit_log_actor_attribution.sql"
# Show every line that mentions the function name
rg -n "delete_accounts_marked_for_deletion" "$FILE" -n
# Search for any grant/revoke statements that might not match the earlier regexes
rg -n "GRANT .*EXECUTE|REVOKE .*EXECUTE|GRANT .*ON FUNCTION|REVOKE .*ON FUNCTION" "$FILE" || true
# Search for the function name within a small window after any GRANT/REVOKE keyword occurrences
python3 - <<'PY'
import re
path="supabase/migrations/20260608160711_audit_log_actor_attribution.sql"
with open(path,'r',encoding='utf-8') as f:
lines=f.readlines()
hits=[]
keywords=re.compile(r'\b(GRANT|REVOKE)\b', re.I)
for i,l in enumerate(lines):
if keywords.search(l):
window="".join(lines[max(0,i-3):min(len(lines),i+10)])
if "delete_accounts_marked_for_deletion" in window:
hits.append(i+1)
print("grant/revoke windows containing function name (line numbers):", hits)
PYRepository: Cap-go/capgo
Length of output: 833
Restrict execution of public.delete_accounts_marked_for_deletion() to trusted roles.
public.delete_accounts_marked_for_deletion() is a SECURITY DEFINER destructive function, but this migration adds no REVOKE ... FROM PUBLIC / scoped GRANT EXECUTE for it (unlike other SECURITY DEFINER functions in the same file), so unintended callers may be able to execute it with elevated privileges.
🔒 Proposed fix
ALTER FUNCTION "public"."delete_accounts_marked_for_deletion"() OWNER TO "postgres";
+REVOKE ALL ON FUNCTION "public"."delete_accounts_marked_for_deletion"() FROM PUBLIC;
+REVOKE ALL ON FUNCTION "public"."delete_accounts_marked_for_deletion"() FROM "anon";
+REVOKE ALL ON FUNCTION "public"."delete_accounts_marked_for_deletion"() FROM "authenticated";
+GRANT EXECUTE ON FUNCTION "public"."delete_accounts_marked_for_deletion"() TO "service_role";🤖 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/migrations/20260608160711_audit_log_actor_attribution.sql` around
lines 358 - 500, The SECURITY DEFINER function
public.delete_accounts_marked_for_deletion() is currently executable by any
role; revoke public execute and explicitly grant EXECUTE only to trusted
roles/groups (e.g., your admin role(s)) to prevent privilege escalation. Add
statements after the function definition that REVOKE EXECUTE ON FUNCTION
public.delete_accounts_marked_for_deletion() FROM PUBLIC; and then GRANT EXECUTE
ON FUNCTION public.delete_accounts_marked_for_deletion() TO <trusted_role1>[,
<trusted_role2>] (replace with your actual admin/trusted role names), ensuring
the function remains owned by postgres.
1 participant
Summary (AI generated)
Motivation (AI generated)
Audit logs previously only stored a user id, which made it hard to distinguish direct user actions from API key actions or automated backend actions. The old foreign keys could also erase or mutate attribution when dependent resources were deleted.
Business Impact (AI generated)
This improves incident investigation and customer support by making audit trails clearer and more durable. Teams can identify whether an action came from a user, an API key id/name, or automation without exposing API key secrets.
Test Plan (AI generated)
bun lintbun typecheckbun run supabase:with-env -- bunx vitest run tests/audit-logs.test.tsbun run supabase:with-env -- bunx vitest run tests/audit-logs.test.ts tests/webhook-delivery-security.unit.test.ts tests/webhook-delivery-redirect.unit.test.ts tests/webhook-queue-processing.test.tsbun run supabase:with-env -- bunx vitest run tests/organization-api.test.tsbun run supabase:with-env -- bunx vitest run tests/chart-refresh-rpc.test.tsbun run supabase:with-env -- bunx vitest run tests/webhook-signature.test.tsbun run supabase:with-env -- bunx vitest run tests/bundle.test.tsbun test:allwas run twice locally; each run had one unrelated parallel-suite failure, and each failed file passed when rerun alone.Generated with AI
Summary by CodeRabbit