Skip to content
Merged
Original file line number Diff line number Diff line change
@@ -0,0 +1,203 @@
-- Slim audit_logs: do not store internal app_versions upload/migrate bookkeeping.
-- Capgo-EU evidence: r2_path/storage_provider/manifest pipeline updates were hundreds
-- of MB of TOAST with no user-facing audit value. Soft-delete and real edits stay.
-- Historical cleanup stays out of this migration to avoid a WAL/lock storm.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Existing bookkeeping audit rows are left untouched, so this migration only prevents future writes and does not remove the historical TOAST footprint described in the PR. The migration should include the promised historical cleanup, or ship an explicit, bounded rollout step that is guaranteed to run alongside it.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At supabase/migrations/20260725163259_slim_audit_logs_skip_bookkeeping.sql, line 4:

<comment>Existing bookkeeping audit rows are left untouched, so this migration only prevents future writes and does not remove the historical TOAST footprint described in the PR. The migration should include the promised historical cleanup, or ship an explicit, bounded rollout step that is guaranteed to run alongside it.</comment>

<file context>
@@ -0,0 +1,203 @@
+-- Slim audit_logs: do not store internal app_versions upload/migrate bookkeeping.
+-- Capgo-EU evidence: r2_path/storage_provider/manifest pipeline updates were hundreds
+-- of MB of TOAST with no user-facing audit value. Soft-delete and real edits stay.
+-- Historical cleanup stays out of this migration to avoid a WAL/lock storm.
+-- Run bounded ops deletes separately if needed.
+
</file context>

-- Run bounded ops deletes separately if needed.

CREATE OR REPLACE FUNCTION "public"."audit_log_trigger"() RETURNS "trigger"
LANGUAGE "plpgsql" SECURITY DEFINER
SET "search_path" TO ''
AS $$
DECLARE
v_old_record jsonb;
v_new_record jsonb;
v_changed_fields text[];
v_org_id uuid;
v_record_id text;
v_user_id uuid;
v_key text;
v_api_key_text text;
v_api_key public.apikeys%ROWTYPE;
v_actor_type text := 'system';
v_actor_user_id uuid;
v_actor_user_email text;
v_actor_apikey_id bigint;
v_actor_apikey_name text;
v_stats_refresh_fields constant text[] := ARRAY['stats_refresh_requested_at', 'stats_updated_at', 'updated_at'];
v_background_counter_fields constant text[] := ARRAY['channel_device_count', 'manifest_bundle_count', 'updated_at'];
v_fat_app_version_fields constant text[] := ARRAY['manifest', 'native_packages'];
BEGIN
SELECT auth.uid() INTO v_actor_user_id;

IF v_actor_user_id IS NOT NULL THEN
v_actor_type := 'user';
ELSE
SELECT public.get_apikey_header() INTO v_api_key_text;

IF v_api_key_text IS NOT NULL THEN
SELECT *
INTO v_api_key
FROM public.find_apikey_by_value(v_api_key_text)
LIMIT 1;

-- Attribute only valid, write-capable API keys; a read-only key present on
-- a request must not be recorded as the actor of a mutation.
IF v_api_key.id IS NOT NULL
AND NOT public.is_apikey_expired(v_api_key.expires_at)
AND (
public.is_allowed_capgkey(v_api_key_text, '{upload}'::text[])
OR public.is_allowed_capgkey(v_api_key_text, '{write}'::text[])
OR public.is_allowed_capgkey(v_api_key_text, '{all}'::text[])
) THEN
v_actor_type := 'apikey';
v_actor_user_id := v_api_key.user_id;
v_actor_apikey_id := v_api_key.id;
v_actor_apikey_name := v_api_key.name;
END IF;
END IF;
END IF;

IF v_actor_user_id IS NOT NULL THEN
SELECT users.email
INTO v_actor_user_email
FROM public.users AS users
WHERE users.id = v_actor_user_id;
END IF;

v_user_id := v_actor_user_id;

-- Skip internal app_versions upload/migrate bookkeeping before the generic
-- changed_fields walk. Compare via to_jsonb only (this trigger is shared across
-- tables; never touch NEW.column names that only exist on app_versions).
IF TG_OP = 'UPDATE' AND TG_TABLE_NAME = 'app_versions' THEN
v_old_record := pg_catalog.to_jsonb(OLD);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Bookkeeping-only updates still serialize the full manifest and native_packages rows before returning, so the hot path retains the TOAST CPU/memory work this skip is intended to avoid. Keep the pre-skip comparison column-based (or otherwise exclude fat values before conversion) and only build audit JSON after determining that an audit is needed.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At supabase/migrations/20260725163259_slim_audit_logs_skip_bookkeeping.sql, line 73:

<comment>Bookkeeping-only updates still serialize the full `manifest` and `native_packages` rows before returning, so the hot path retains the TOAST CPU/memory work this skip is intended to avoid. Keep the pre-skip comparison column-based (or otherwise exclude fat values before conversion) and only build audit JSON after determining that an audit is needed.</comment>

<file context>
@@ -66,34 +66,29 @@ BEGIN
+  -- changed_fields walk. Compare via to_jsonb only (this trigger is shared across
+  -- tables; never touch NEW.column names that only exist on app_versions).
+  IF TG_OP = 'UPDATE' AND TG_TABLE_NAME = 'app_versions' THEN
+    v_old_record := pg_catalog.to_jsonb(OLD);
+    v_new_record := pg_catalog.to_jsonb(NEW);
+    IF (
</file context>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The early-return check for bookkeeping-only updates unconditionally serializes OLD and NEW to JSONB via to_jsonb(). When a user-facing field change (comment, checksum, etc.) causes the check to not match, the function falls through to the generic ELSE branch which serializes both rows to JSONB a second time. Since manifest is a custom array type (manifest_entry[]) that can hold thousands of entries per AGENTS.md, this double-serialization adds unnecessary CPU/memory overhead to every user-facing UPDATE on app_versions.

Consider restructuring the early-return check inside the existing ELSE branch so that the to_jsonb call happens exactly once — the serialized value is then reused for both the bookkeeping comparison and the changed_fields walk. This avoids redundant work and recovers the performance characteristic the previous column-comparison approach had for the non-skipped path.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At supabase/migrations/20260725163259_slim_audit_logs_skip_bookkeeping.sql, line 73:

<comment>The early-return check for bookkeeping-only updates unconditionally serializes OLD and NEW to JSONB via `to_jsonb()`. When a user-facing field change (comment, checksum, etc.) causes the check to not match, the function falls through to the generic ELSE branch which serializes both rows to JSONB a second time. Since `manifest` is a custom array type (`manifest_entry[]`) that can hold thousands of entries per AGENTS.md, this double-serialization adds unnecessary CPU/memory overhead to every user-facing UPDATE on `app_versions`.

Consider restructuring the early-return check inside the existing ELSE branch so that the `to_jsonb` call happens exactly once — the serialized value is then reused for both the bookkeeping comparison and the changed_fields walk. This avoids redundant work and recovers the performance characteristic the previous column-comparison approach had for the non-skipped path.</comment>

<file context>
@@ -66,34 +66,29 @@ BEGIN
+  -- changed_fields walk. Compare via to_jsonb only (this trigger is shared across
+  -- tables; never touch NEW.column names that only exist on app_versions).
+  IF TG_OP = 'UPDATE' AND TG_TABLE_NAME = 'app_versions' THEN
+    v_old_record := pg_catalog.to_jsonb(OLD);
+    v_new_record := pg_catalog.to_jsonb(NEW);
+    IF (
</file context>

v_new_record := pg_catalog.to_jsonb(NEW);
IF (
v_old_record
- 'manifest'

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Manifest creation/correction and native_packages-only edits now disappear from audit history, not just the reclaim write. Restrict the skip to the non-NULL-to-NULL manifest transition and leave native_packages outside the ignored set.

(Based on your team's feedback about preserving native_packages audit field history.)

View Feedback

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At supabase/migrations/20260725163259_slim_audit_logs_skip_bookkeeping.sql, line 77:

<comment>Manifest creation/correction and native_packages-only edits now disappear from audit history, not just the reclaim write. Restrict the skip to the non-NULL-to-NULL manifest transition and leave native_packages outside the ignored set.

(Based on your team's feedback about preserving native_packages audit field history.) </comment>

<file context>
@@ -0,0 +1,205 @@
+    v_new_record := pg_catalog.to_jsonb(NEW);
+    IF (
+      v_old_record
+        - 'manifest'
+        - 'native_packages'
+        - 'updated_at'
</file context>

- 'updated_at'
- 'manifest_count'
- 'storage_provider'
- 'r2_path'
) IS NOT DISTINCT FROM (
v_new_record
- 'manifest'
- 'updated_at'
- 'manifest_count'
- 'storage_provider'
- 'r2_path'
Comment thread
riderx marked this conversation as resolved.
) THEN
RETURN NEW;
END IF;
END IF;

IF TG_OP = 'DELETE' THEN
v_old_record := pg_catalog.to_jsonb(OLD);
v_new_record := NULL;
ELSIF TG_OP = 'INSERT' THEN
v_old_record := NULL;
v_new_record := pg_catalog.to_jsonb(NEW);
ELSE
v_old_record := pg_catalog.to_jsonb(OLD);
v_new_record := pg_catalog.to_jsonb(NEW);

FOR v_key IN SELECT pg_catalog.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 := pg_catalog.array_append(v_changed_fields, v_key);
END IF;
END LOOP;

IF TG_TABLE_NAME = ANY(ARRAY['apps', 'orgs'])
AND v_changed_fields && ARRAY['stats_refresh_requested_at', 'stats_updated_at']
AND NOT EXISTS (
SELECT 1
FROM pg_catalog.unnest(v_changed_fields) AS changed_field(field_name)
WHERE changed_field.field_name <> ALL(v_stats_refresh_fields)
) THEN
RETURN NEW;
END IF;

IF v_actor_type = 'system'
AND TG_TABLE_NAME = 'apps'
AND v_changed_fields && ARRAY['channel_device_count', 'manifest_bundle_count']
AND NOT EXISTS (
SELECT 1
FROM pg_catalog.unnest(v_changed_fields) AS changed_field(field_name)
WHERE changed_field.field_name <> ALL(v_background_counter_fields)
) THEN
RETURN NEW;
END IF;
END IF;

-- Never persist multi-MB array/json columns in audit TOAST.
-- Keep fat field names in changed_fields when co-occurring with real user edits.
IF TG_TABLE_NAME = 'app_versions' THEN
IF v_old_record IS NOT NULL THEN
v_old_record := v_old_record - v_fat_app_version_fields;
END IF;
IF v_new_record IS NOT NULL THEN
v_new_record := v_new_record - v_fat_app_version_fields;
END IF;

END IF;
Comment thread
coderabbitai[bot] marked this conversation as resolved.

CASE TG_TABLE_NAME
WHEN 'orgs' THEN
v_org_id := COALESCE(NEW.id, OLD.id);
v_record_id := COALESCE(NEW.id, OLD.id)::text;
WHEN 'apps' THEN
v_org_id := COALESCE(NEW.owner_org, OLD.owner_org);
v_record_id := COALESCE(NEW.app_id, OLD.app_id)::text;
WHEN 'channels' THEN
v_org_id := COALESCE(NEW.owner_org, OLD.owner_org);
v_record_id := COALESCE(NEW.id, OLD.id)::text;
WHEN 'app_versions' THEN
v_org_id := COALESCE(NEW.owner_org, OLD.owner_org);
v_record_id := COALESCE(NEW.id, OLD.id)::text;
WHEN 'org_users' THEN
v_org_id := COALESCE(NEW.org_id, OLD.org_id);
v_record_id := COALESCE(NEW.id, OLD.id)::text;
ELSE
v_org_id := NULL;
v_record_id := NULL;
END CASE;

IF v_org_id IS NOT NULL THEN
INSERT INTO public.audit_logs (
table_name,
record_id,
operation,
user_id,
org_id,
old_record,
new_record,
changed_fields,
actor_type,
actor_user_id,
actor_user_email,
actor_apikey_id,
actor_apikey_name
) VALUES (
TG_TABLE_NAME,
v_record_id,
TG_OP,
v_user_id,
v_org_id,
v_old_record,
v_new_record,
v_changed_fields,
v_actor_type,
v_actor_user_id,
v_actor_user_email,
v_actor_apikey_id,
v_actor_apikey_name
);
END IF;

RETURN COALESCE(NEW, OLD);
END;
$$;


ALTER FUNCTION "public"."audit_log_trigger"() OWNER TO "postgres";

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Function privileges remain whatever the target database previously had because this migration only changes ownership. Revoke PUBLIC and grant EXECUTE only to service_role so this SECURITY DEFINER entry point has an enforced, self-contained ACL.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At supabase/migrations/20260725163259_slim_audit_logs_skip_bookkeeping.sql, line 205:

<comment>Function privileges remain whatever the target database previously had because this migration only changes ownership. Revoke PUBLIC and grant EXECUTE only to service_role so this SECURITY DEFINER entry point has an enforced, self-contained ACL.</comment>

<file context>
@@ -0,0 +1,205 @@
+$$;
+
+
+ALTER FUNCTION "public"."audit_log_trigger"() OWNER TO "postgres";
</file context>

117 changes: 117 additions & 0 deletions tests/cleanup_swap_memory.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,123 @@ describe('swap memory cleanup functions', () => {
await executeSQL(`DELETE FROM public.apps WHERE app_id = $1`, [appId])
})

async function seedAuditAppVersion(appId: string, orgId: string) {
await executeSQL(
`INSERT INTO public.apps (app_id, name, icon_url, owner_org)
VALUES ($1, 'swap-audit-skip', '', $2::uuid)`,
[appId, orgId],
)
const versionRows = await executeSQL(
`INSERT INTO public.app_versions (app_id, name, owner_org, storage_provider, comment)
VALUES ($1, $2, $3::uuid, 'r2-direct', 'seed')
RETURNING id`,
[appId, `1.0.0-${randomUUID().slice(0, 8)}`, orgId],
)
return versionRows[0]?.id as number
}

async function clearVersionAudits(versionId: number) {
await executeSQL(
`DELETE FROM public.audit_logs
WHERE table_name = 'app_versions' AND record_id = $1`,
[String(versionId)],
)
}

it('audit_log_trigger skips dual-storage migrate finalize', async () => {
const appId = `com.swap.auditskip.${randomUUID().slice(0, 8)}`
const orgId = (await executeSQL(`SELECT id FROM public.orgs ORDER BY created_at LIMIT 1`))[0]?.id as string
const versionId = await seedAuditAppVersion(appId, orgId)

await executeSQL(
`UPDATE public.app_versions
SET manifest = ARRAY[ROW('a.js', 'apps/a.js', 'hash')::public.manifest_entry]
WHERE id = $1`,
[versionId],
)
await clearVersionAudits(versionId)

await executeSQL(
`INSERT INTO public.manifest (app_version_id, file_name, s3_path, file_hash)
VALUES ($1, 'a.js', 'apps/a.js', 'hash')`,
[versionId],
)
await executeSQL(
`UPDATE public.app_versions
SET manifest = NULL, manifest_count = 1, updated_at = now()
WHERE id = $1`,
[versionId],
)

const logs = await executeSQL(
`SELECT id FROM public.audit_logs
WHERE table_name = 'app_versions' AND record_id = $1 AND operation = 'UPDATE'`,
[String(versionId)],
)
expect(logs).toHaveLength(0)

await executeSQL(`DELETE FROM public.manifest WHERE app_version_id = $1`, [versionId])
await executeSQL(`DELETE FROM public.app_versions WHERE id = $1`, [versionId])
await executeSQL(`DELETE FROM public.apps WHERE app_id = $1`, [appId])
})

it('audit_log_trigger skips r2_path and storage_provider bookkeeping', async () => {
const appId = `com.swap.auditpath.${randomUUID().slice(0, 8)}`
const orgId = (await executeSQL(`SELECT id FROM public.orgs ORDER BY created_at LIMIT 1`))[0]?.id as string
const versionId = await seedAuditAppVersion(appId, orgId)
await clearVersionAudits(versionId)

await executeSQL(
`UPDATE public.app_versions
SET r2_path = 'apps/test/bundle.zip', updated_at = now()
WHERE id = $1`,
[versionId],
)
await executeSQL(
`UPDATE public.app_versions
SET storage_provider = 'r2', updated_at = now()
WHERE id = $1`,
[versionId],
)

const logs = await executeSQL(
`SELECT id, changed_fields FROM public.audit_logs
WHERE table_name = 'app_versions' AND record_id = $1 AND operation = 'UPDATE'`,
[String(versionId)],
)
expect(logs).toHaveLength(0)

await executeSQL(`DELETE FROM public.app_versions WHERE id = $1`, [versionId])
await executeSQL(`DELETE FROM public.apps WHERE app_id = $1`, [appId])
})

it('audit_log_trigger still records soft-delete', async () => {
const appId = `com.swap.auditdel.${randomUUID().slice(0, 8)}`
const orgId = (await executeSQL(`SELECT id FROM public.orgs ORDER BY created_at LIMIT 1`))[0]?.id as string
const versionId = await seedAuditAppVersion(appId, orgId)
await clearVersionAudits(versionId)

await executeSQL(
`UPDATE public.app_versions
SET deleted = true, deleted_at = now(), updated_at = now()
WHERE id = $1`,
[versionId],
)

const logs = await executeSQL(
`SELECT changed_fields FROM public.audit_logs
WHERE table_name = 'app_versions' AND record_id = $1 AND operation = 'UPDATE'
ORDER BY id DESC LIMIT 1`,
[String(versionId)],
)
expect(logs).toHaveLength(1)
expect(logs[0]?.changed_fields).toContain('deleted')

await executeSQL(`DELETE FROM public.audit_logs WHERE record_id = $1 AND table_name = 'app_versions'`, [String(versionId)])
await executeSQL(`DELETE FROM public.app_versions WHERE id = $1`, [versionId])
await executeSQL(`DELETE FROM public.apps WHERE app_id = $1`, [appId])
})

it('null_migrated_app_version_manifests skips partially migrated arrays', async () => {
const appId = `com.swap.partialmanifest.${randomUUID().slice(0, 8)}`
const orgRows = await executeSQL(
Expand Down
Loading