fix(audit): derive attribution for legacy inserts instead of defaulting it

The DEFAULT 'unknown' added in the last pass stopped a previous-release
process from raising NotNullViolation mid-rollout, but it threw the
attribution away to do it. The highest-volume auth_events writers are OIDC
login and silent renewal, where the actor is simply the user, so a rollout
window would have flattened exactly the rows that were trivially recoverable.

Lifts both derivation rules into SQL functions -- auth_events_derive_actor
and auth_events_derive_target -- so the backfill and a BEFORE INSERT trigger
share one definition rather than two copies of the same CASE drifting apart.

The trigger guards on NEW.actor_id IS NULL, which identifies a legacy insert
exactly because the repository always supplies one. That distinction matters
for target_id: a current writer sets it NULL deliberately for events with no
user target, and deriving there would name the actor as their own target.

Kept rather than scheduled for removal: it is a no-op on the path the
repository takes, and it keeps the column's contract true for any writer that
bypasses it.

docs/content/Deploying/OIDC-SSO.mdx

@@ -190,7 +190,9 @@ Authentication activity is recorded in `auth_events`, an append-only Postgres ta
 - `actor_id` — who performed the action. Never empty.
 - `target_id` — the user the action was performed on, or `NULL` when the event is not about a user (a team change, an instance-wide policy).
 
-They differ whenever someone acts on someone else: an admin deactivating an account is `actor_id = <the admin>`, `target_id = <the account>`. For self-service events such as a login they are the same user. Automated actors are named `system:oidc` (a group-driven role change at login) and `system:scim` (provisioning). `actor_id` defaults to `unknown`, which only a process from a release predating the column can produce.
+They differ whenever someone acts on someone else: an admin deactivating an account is `actor_id = <the admin>`, `target_id = <the account>`. For self-service events such as a login they are the same user. Automated actors are named `system:oidc` (a group-driven role change at login) and `system:scim` (provisioning).
+
+A `BEFORE INSERT` trigger fills both columns for any row that arrives without an `actor_id`, applying the same rules as the migration's backfill. That keeps attribution intact for a writer from a release that predates the columns — during a rolling deployment, for instance — instead of rejecting the insert or flattening the actor to a sentinel.
 
 | Event | Recorded when |
 | --- | --- |

docsgpt/alembic/versions/0034_auth_events_actor_target.py

@@ -24,6 +24,11 @@ Backfill rules, applied only to rows that predate the columns:
   the actor and have no single user target: ``team.*``, and instance- or
   team-scoped ``quota_policy_*`` changes.
 
+The same two rules are installed as SQL functions and applied by a BEFORE
+INSERT trigger to any row that arrives without an ``actor_id``, so a writer
+from a release that predates these columns keeps its attribution instead of
+being rejected or flattened to a sentinel.
+
 Also adds the indexes the global admin feed needs. Before this the only index
 was ``(user_id, created_at DESC)``, so the cross-user feed — which orders by
 ``created_at DESC`` with no user predicate — degraded to a sequential scan plus
@@ -50,18 +55,32 @@ def upgrade() -> None:
     op.execute("ALTER TABLE auth_events ADD COLUMN IF NOT EXISTS actor_id TEXT;")
     op.execute("ALTER TABLE auth_events ADD COLUMN IF NOT EXISTS target_id TEXT;")
 
-    # Backfill. ``actor_id IS NULL`` scopes this to pre-migration rows, so a
-    # re-run (or a downgrade/upgrade cycle) never clobbers written values.
+    # The derivation rules, as functions, so the backfill below and the
+    # compatibility trigger share one definition instead of two copies of the
+    # same CASE drifting apart.
     op.execute(
         """
-        UPDATE auth_events
-        SET actor_id = COALESCE(
+        CREATE OR REPLACE FUNCTION auth_events_derive_actor(
+            metadata jsonb, user_id text
+        ) RETURNS text
+        LANGUAGE sql IMMUTABLE AS $$
+            SELECT COALESCE(
                 metadata->>'by',
                 metadata->>'granted_by',
                 metadata->>'revoked_by',
                 user_id
-            ),
-            target_id = CASE
+            );
+        $$;
+        """
+    )
+    op.execute(
+        """
+        CREATE OR REPLACE FUNCTION auth_events_derive_target(
+            event text, metadata jsonb, user_id text
+        ) RETURNS text
+        LANGUAGE sql IMMUTABLE AS $$
+            SELECT CASE
+                -- Team events were always filed under the actor.
                 WHEN event LIKE 'team.%' THEN NULL
                 -- An instance or team quota policy is filed under the acting
                 -- admin, not a target user (see api/admin/quotas.py::_audit);
@@ -69,22 +88,71 @@ def upgrade() -> None:
                 WHEN event IN ('quota_policy_set', 'quota_policy_deleted')
                      AND COALESCE(metadata->>'scope', '') <> 'user' THEN NULL
                 ELSE user_id
-            END
+            END;
+        $$;
+        """
+    )
+
+    # Backfill. ``actor_id IS NULL`` scopes this to pre-migration rows, so a
+    # re-run (or a downgrade/upgrade cycle) never clobbers written values.
+    op.execute(
+        """
+        UPDATE auth_events
+        SET actor_id = auth_events_derive_actor(metadata, user_id),
+            target_id = auth_events_derive_target(event, metadata, user_id)
         WHERE actor_id IS NULL;
         """
     )
 
-    # Backfilled every row above, so the NOT NULL is safe to assert now. New
-    # rows always carry an actor (the repository derives one from ``user_id``).
+    # A writer from a release that predates these columns names only the old
+    # ones, so ``actor_id`` arrives NULL. Two things follow, and the trigger
+    # handles both:
     #
-    # The DEFAULT is what makes this safe during a rolling deploy: a
-    # still-running previous-release process inserts (user_id, event, ip,
-    # user_agent, metadata) with no actor, and those inserts share the
-    # request's transaction in admin/routes.py -- a NotNullViolation there
-    # would 500 the deactivate or role grant and roll its write back with it.
-    # 'unknown' is the same sentinel ``record_event`` writes for an
-    # unauthenticated actor, so the column stays readable either way.
-    op.execute("ALTER TABLE auth_events ALTER COLUMN actor_id SET DEFAULT 'unknown';")
+    #   1. The NOT NULL below would reject the insert. In admin/routes.py that
+    #      insert shares the request's transaction, so a NotNullViolation would
+    #      500 the deactivate or role grant and roll its write back with it.
+    #   2. Defaulting it to a sentinel would keep the write but lose the
+    #      attribution -- and the highest-volume events are the recoverable
+    #      ones: for a login or a silent renewal the actor is simply the user.
+    #
+    # ``actor_id IS NULL`` identifies a legacy insert exactly, because the
+    # repository always supplies one. That matters for ``target_id``: a current
+    # writer sets it to NULL deliberately for events with no user target
+    # (``conversation.deleted``), and deriving there would wrongly name the
+    # actor as their own target.
+    #
+    # Kept rather than dropped in a follow-up: it costs nothing on the path the
+    # repository takes, and it keeps the column's contract true for any writer
+    # that bypasses it.
+    op.execute(
+        """
+        CREATE OR REPLACE FUNCTION auth_events_fill_attribution()
+        RETURNS trigger
+        LANGUAGE plpgsql AS $$
+        BEGIN
+            IF NEW.actor_id IS NULL THEN
+                NEW.actor_id := auth_events_derive_actor(
+                    NEW.metadata, NEW.user_id
+                );
+                NEW.target_id := auth_events_derive_target(
+                    NEW.event, NEW.metadata, NEW.user_id
+                );
+            END IF;
+            RETURN NEW;
+        END;
+        $$;
+        """
+    )
+    op.execute("DROP TRIGGER IF EXISTS auth_events_fill_attribution_trg ON auth_events;")
+    op.execute(
+        """
+        CREATE TRIGGER auth_events_fill_attribution_trg
+        BEFORE INSERT ON auth_events
+        FOR EACH ROW EXECUTE FUNCTION auth_events_fill_attribution();
+        """
+    )
+
+    # Backfilled above and guaranteed by the trigger, so this is safe to assert.
     op.execute("ALTER TABLE auth_events ALTER COLUMN actor_id SET NOT NULL;")
 
     op.execute(
@@ -105,5 +173,9 @@ def downgrade() -> None:
     op.execute("DROP INDEX IF EXISTS auth_events_actor_idx;")
     op.execute("DROP INDEX IF EXISTS auth_events_event_created_idx;")
     op.execute("DROP INDEX IF EXISTS auth_events_created_idx;")
+    op.execute("DROP TRIGGER IF EXISTS auth_events_fill_attribution_trg ON auth_events;")
+    op.execute("DROP FUNCTION IF EXISTS auth_events_fill_attribution();")
     op.execute("ALTER TABLE auth_events DROP COLUMN IF EXISTS target_id;")
     op.execute("ALTER TABLE auth_events DROP COLUMN IF EXISTS actor_id;")
+    op.execute("DROP FUNCTION IF EXISTS auth_events_derive_target(text, jsonb, text);")
+    op.execute("DROP FUNCTION IF EXISTS auth_events_derive_actor(jsonb, text);")

tests/storage/db/test_migration_0034.py

@@ -126,24 +126,61 @@ class TestMigration0034RoundTrip:
         # Team events were always filed under the actor; they have no user target.
         assert rows["team.create"] == "actor-3|-"
 
-    def test_actor_id_has_a_default_for_rolling_deploys(self, pg_engine):
-        """A previous-release process must not 500 mid-rollout.
+    def test_legacy_insert_keeps_its_attribution(self, pg_engine):
+        """A previous-release writer must neither 500 nor lose attribution.
 
-        Its inserts name only the old columns; without a DEFAULT the NOT NULL
-        raises, and in admin/routes.py that insert shares the request's
-        transaction, so the role grant beside it would roll back too.
+        Its inserts name only the old columns, so ``actor_id`` arrives NULL.
+        The NOT NULL alone would reject them -- and in admin/routes.py that
+        insert shares the request's transaction, so the role grant beside it
+        would roll back too. A bare sentinel would keep the row but throw the
+        actor away, and the highest-volume events are the recoverable ones.
         """
         with pg_engine.begin() as conn:
             conn.execute(
                 text(
-                    "INSERT INTO auth_events (user_id, event, metadata) "
-                    "VALUES ('legacy', 'oidc_login', '{}'::jsonb)"
+                    "INSERT INTO auth_events (user_id, event, metadata) VALUES "
+                    # A login: the user is their own actor and target.
+                    "('legacy-login', 'oidc_login', '{}'::jsonb), "
+                    # An admin action: the actor is recoverable from metadata.
+                    "('legacy-victim', 'admin_user_deactivated', "
+                    " '{\"by\": \"admin-legacy\"}'::jsonb), "
+                    # A team event: filed under the actor, no user target.
+                    "('legacy-actor', 'team.create', '{}'::jsonb)"
                 )
             )
-            actor = conn.execute(
-                text("SELECT actor_id FROM auth_events WHERE user_id = 'legacy'")
-            ).scalar()
-        assert actor == "unknown"
+            rows = dict(
+                conn.execute(
+                    text(
+                        "SELECT user_id, actor_id || '|' || COALESCE(target_id, '-') "
+                        "FROM auth_events WHERE user_id LIKE 'legacy-%'"
+                    )
+                ).fetchall()
+            )
+        assert rows["legacy-login"] == "legacy-login|legacy-login"
+        assert rows["legacy-victim"] == "admin-legacy|legacy-victim"
+        assert rows["legacy-actor"] == "legacy-actor|-"
+
+    def test_trigger_leaves_a_current_writer_alone(self, pg_engine):
+        """A deliberate NULL target must survive.
+
+        ``conversation.deleted`` acts on a resource, not an account. Deriving
+        a target there would name the actor as their own target.
+        """
+        with pg_engine.begin() as conn:
+            conn.execute(
+                text(
+                    "INSERT INTO auth_events "
+                    "(user_id, actor_id, target_id, event, metadata) "
+                    "VALUES ('u9', 'u9', NULL, 'conversation.deleted', '{}'::jsonb)"
+                )
+            )
+            row = conn.execute(
+                text(
+                    "SELECT actor_id, target_id FROM auth_events "
+                    "WHERE event = 'conversation.deleted'"
+                )
+            ).fetchone()
+        assert tuple(row) == ("u9", None)
 
     def test_backfill_scopes_quota_policy_targets(self, pg_engine):
         """Only a user-scoped quota policy has a user target.