Repair the two plugin-auth admin-audit durability swallows — batch 6 of the #12981 worklist

.changeset/auth-admin-audit-swallow-batch-6.md

@@ -0,0 +1,47 @@
+---
+'@objectstack/plugin-auth': minor
+---
+
+Report the refused admin-audit writes two `catch { }` sites swallowed (#12981 batch 6)
+
+The two tier-1 DARK durability swallows on `plugin-auth`'s admin surface: an
+administrative action landed, its audit row was refused, and the endpoint
+answered `200` with nothing written anywhere. Control flow is unchanged at both
+sites — an admin operation must never fail over its own audit — but the refusal
+is no longer silent.
+
+Both `catch` blocks were doing two jobs and were only right about one of them:
+
+- **plugin-audit UNINSTALLED** — there is no `sys_audit_log` object at all, so
+  nothing ever claimed the action would be audited. Silence is correct, and
+  reporting here would put a line on every admin action in every deployment that
+  does not run plugin-audit.
+- **plugin-audit INSTALLED, the write REFUSED** — the action happened, the audit
+  record did not, and nothing retries or reconstructs it.
+
+Both spelled `catch { }`. Each site now asks `getSchema('sys_audit_log')` — the
+registry that owns the answer — instead of reading the driver's error text,
+which would decide the same question by guessing. `getSchema` is declared
+**optional** on `AdminUserDataEngine` and `IdentityImportEngine`, so it is
+additive and no host that type-checks today stops doing so; where it is absent
+the site cannot measure the difference and therefore reports, because an
+unmeasurable write must not be a silent one.
+
+What was hiding in the silence:
+
+- `admin-user-endpoints.ts :: writeAdminAudit` — `sys_account` is in
+  plugin-audit's `SKIP_OBJECTS`, so for `/admin/set-user-password` its generic
+  writer emits **zero** rows and the row refused here was the only record that a
+  password was ever administratively reset.
+- `admin-import-users.ts` run-level row — `action: 'import'` with a null
+  `record_id` is a shape plugin-audit's `actionFor` structurally cannot emit. The
+  per-row `create` rows still land, which is what made this dangerous: the trail
+  looked complete while who ran the import, under which password policy, and what
+  it did in aggregate was gone.
+
+Both sinks (`AdminUserEndpointDeps.logger`, `IdentityImportDeps.logger`) are
+`{ warn(msg: string): void }` and both are re-exported from the package
+`index.ts`. Neither declares `error`, so the LEVEL stays `warn` and remains
+#13398's question; only the SILENCE is repaired here. Each seam is pinned by a
+test that fails if it goes quiet again, plus absence-asserting cases so a seam
+that warns unconditionally cannot pass.

packages/plugins/plugin-auth/src/admin-import-users.ts

@@ -89,6 +89,19 @@ export interface IdentityImportEngine {
   find(objectName: string, query?: any): Promise<any[]>;
   update(objectName: string, data: any, options?: any): Promise<any>;
   insert(objectName: string, data: any, options?: any): Promise<any>;
+  /**
+   * [#12981] Optional registry probe — `ObjectQL.getSchema`, which answers
+   * `undefined` for an object no package registered.
+   *
+   * Separates the two outcomes the run-level audit `catch` used to spell
+   * identically: plugin-audit UNINSTALLED (no `sys_audit_log` object — nothing
+   * was ever claimed, so silence is correct) from plugin-audit INSTALLED AND
+   * THE WRITE REFUSED (the import ran, its only run-level record did not land,
+   * and the endpoint still answers 200). Optional, therefore additive: a host
+   * or mock without it keeps type-checking, and a site that cannot measure the
+   * difference REPORTS rather than going quiet.
+   */
+  getSchema?(objectName: string): unknown;
 }
 
 export interface IdentityImportDeps {
@@ -510,23 +523,56 @@ export async function runAdminImportUsers(
     // `sys_audit_log` table, and an import must not fail over its own audit.
     // Both facts are pinned in
     // `packages/qa/dogfood/test/admin-identity-audit-trail.dogfood.test.ts`.
-    try {
-      await engine.insert('sys_audit_log', {
-        action: 'import',
-        user_id: actor.id,
-        actor: actor.id,
-        object_name: 'sys_user',
-        metadata: JSON.stringify({
-          event: 'user.import_run',
-          mode, matchBy, passwordPolicy: policy,
-          total: prepared.rows.length,
-          created: summary.created, updated: summary.updated,
-          skipped: summary.skipped, errors: summary.errors + preErrors,
-          // How `auto` (and the fixed policies) split the batch across channels.
-          delivery,
-        }),
-      }, { context: SYSTEM_CTX } as any);
-    } catch { /* audit table may not exist — never fail the import */ }
+    //
+    // [#12981] "Best-effort" was doing two jobs and only one of them was
+    // right. plugin-audit UNINSTALLED means no `sys_audit_log` object, so
+    // nothing ever claimed the run would be audited — a declared skip, taken
+    // silently by the probe below. A REFUSED write is the other thing
+    // entirely, and it was wearing the same `catch`.
+    const auditRegistered = !engine.getSchema || Boolean(engine.getSchema('sys_audit_log'));
+    if (auditRegistered) {
+      try {
+        await engine.insert('sys_audit_log', {
+          action: 'import',
+          user_id: actor.id,
+          actor: actor.id,
+          object_name: 'sys_user',
+          metadata: JSON.stringify({
+            event: 'user.import_run',
+            mode, matchBy, passwordPolicy: policy,
+            total: prepared.rows.length,
+            created: summary.created, updated: summary.updated,
+            skipped: summary.skipped, errors: summary.errors + preErrors,
+            // How `auto` (and the fixed policies) split the batch across channels.
+            delivery,
+          }),
+        }, { context: SYSTEM_CTX } as any);
+      } catch (e) {
+        // [#12981] An import must not fail over its own audit — control flow is
+        // unchanged and the run still answers 200 with its summary. It must not
+        // be SILENT either. `sys_audit_log` is registered (checked above), so
+        // this is a refused write, and the run-level row is the ONLY record of
+        // it: plugin-audit's `actionFor` maps afterInsert/Update/Delete to
+        // create/update/delete and nothing else, so `action: 'import'` with a
+        // null `record_id` is a shape its writer structurally cannot emit. The
+        // per-row `create` rows survive, which is what makes this dangerous —
+        // the trail looks populated while WHO ran the import, under WHICH
+        // policy, and WHAT the run did overall is simply gone.
+        deps.logger?.warn(
+          `[AuthPlugin] the run-level sys_audit_log row for this user import was NOT written — `
+            + `the import itself SUCCEEDED (created ${summary.created}, updated ${summary.updated}, `
+            + `skipped ${summary.skipped}) and the endpoint answers 200, so nothing looks wrong. `
+            + 'plugin-audit is installed (sys_audit_log is registered), so this is a REFUSED '
+            + "write, not an absent plugin. plugin-audit's per-row create rows still landed, so "
+            + 'the audit trail LOOKS complete while the only record of who ran this import, under '
+            + 'which password policy, and what it did in aggregate is absent. Nothing retries it '
+            + 'and no later boot reconstructs it. Remedy: restore write access to sys_audit_log '
+            + `(permissions, driver connectivity) before the next import. Cause: ${
+              (e as Error)?.message ?? e
+            }`,
+        );
+      }
+    }
   }
 
   const errors = summary.errors + preErrors;

packages/plugins/plugin-auth/src/admin-user-endpoints.ts

@@ -116,6 +116,24 @@ export interface AdminUserDataEngine {
    * isn't wired, in which case the org bind simply no-ops.
    */
   find?(object: string, query?: unknown, opts?: unknown): Promise<unknown>;
+  /**
+   * [#12981] Optional registry probe — `ObjectQL.getSchema`, which answers
+   * `undefined` for an object no package registered.
+   *
+   * It is here to separate two outcomes the audit `catch` below used to spell
+   * identically: plugin-audit UNINSTALLED (no `sys_audit_log` object, so
+   * nothing was ever claimed and silence is correct) from plugin-audit
+   * INSTALLED AND THE WRITE REFUSED (the admin action landed, its audit row did
+   * not, and the endpoint still answers 200). Reading the driver's error text
+   * would decide the same question by guessing; this asks the registry that
+   * owns the answer.
+   *
+   * Optional because lean mocks and hosts that wire no ObjectQL engine do not
+   * carry it — additive, so nothing that type-checks today stops doing so. When
+   * it is absent the site cannot tell the two apart and therefore REPORTS: an
+   * unmeasurable write must never be a silent one.
+   */
+  getSchema?(objectName: string): unknown;
 }
 
 /** The gated caller, passed by the route after its ADR-0068 check. */
@@ -376,6 +394,12 @@ async function writeAdminAudit(
 ): Promise<void> {
   const engine = deps.getDataEngine();
   if (!engine) return;
+  // plugin-audit is OPTIONAL, and with it uninstalled there is no
+  // `sys_audit_log` object at all. That case is a DECLARED skip, not a
+  // swallow: nothing ever claimed this action would be audited, so there is
+  // nothing to report and the channel stays quiet — which is what keeps the
+  // `warn` below meaningful instead of one more line nobody reads.
+  if (engine.getSchema && !engine.getSchema('sys_audit_log')) return;
   try {
     await engine.insert(
       'sys_audit_log',
@@ -389,9 +413,31 @@ async function writeAdminAudit(
       },
       { context: SYSTEM_CTX },
     );
-  } catch {
-    // plugin-audit may not be installed (no sys_audit_log table) — audit is
-    // best-effort by design here; the operation itself must not fail.
+  } catch (error) {
+    // [#12981] The operation itself must NOT fail over its own audit — control
+    // flow is unchanged and the endpoint still answers 200. But it must not be
+    // SILENT either, and this site is the sharpest case in the family: the
+    // header above records that `sys_account` is in plugin-audit's
+    // `SKIP_OBJECTS`, so for `/admin/set-user-password` the generic writer
+    // emits ZERO rows and the row refused here is the ONLY record that a
+    // password was administratively reset. Nothing retries it and no later
+    // boot reconstructs it — the reset simply has no trail, while the admin
+    // who performed it reads `success: true`. `sys_audit_log` is registered
+    // (checked above), so this is a refused write, not an absent plugin.
+    deps.logger?.warn(
+      `[AuthPlugin] the sys_audit_log row for this administrative '${entry.action}' on sys_user `
+        + `${entry.recordId} was NOT written — the operation itself SUCCEEDED and the endpoint `
+        + 'answers 200, so nothing looks wrong. plugin-audit is installed (sys_audit_log is '
+        + 'registered), so this is a REFUSED write, not an absent plugin. This row carries the '
+        + "admin's decisions (event, passwordGenerated, mustChangePassword, placeholderEmail, "
+        + 'membershipCreated), none of which is derivable from the stored row, and for '
+        + '/admin/set-user-password it is the only audit record that exists at all because '
+        + "sys_account is in plugin-audit's SKIP_OBJECTS. Nothing retries this write, so the "
+        + 'action stays permanently untrailed. Remedy: restore write access to sys_audit_log '
+        + `(permissions, driver connectivity), then treat this line as the audit record. Cause: ${
+          (error as Error)?.message ?? error
+        }`,
+    );
   }
 }