fix(audit-cleanup): unblock Helm + move self-audit out of tx (#3268)

- .kontinuous/templates/audit-cleanup-cron.yaml:
  `.Values.app.imagePackage` is scoped to the app subchart and renders
  as nil from the project template context (build failure:
  "nil pointer evaluating interface {}.imagePackage"). Inline the
  literal `app` since values.yaml:9 pins it statically.
- .kontinuous/templates/audit-cleanup-cron.yaml:
  switch `restartPolicy: OnFailure` to `Never` — the combination with
  `backoffLimit: 0` was contradictory (container would restart on
  failure but the Job would not re-queue). Both now agree on fail-fast.
- scripts/audit-cleanup.mjs:
  move the success self-audit INSERT outside the transaction. A
  failure writing the audit row must not roll back cleanup work that
  already committed. The failure-path log was already out-of-tx.
- scripts/audit-cleanup.mjs:
  use `fileURLToPath` + `realpathSync` for the `isMain` guard so the
  CLI fires correctly under pnpm's content-addressable store and
  Docker bind-mounts that hand symlinked paths to Node.
- integration test: null-guard `afterAll` so teardown cannot crash on
  an un-initialized `sql` if `beforeAll` threw.

Author: Viczei

.kontinuous/templates/audit-cleanup-cron.yaml

@@ -19,13 +19,19 @@ spec:
           labels:
             app: audit-cleanup-daily
         spec:
-          restartPolicy: OnFailure
+          # `Never` is consistent with `backoffLimit: 0`: the kubelet does not
+          # restart the failed container and the Job does not re-queue a new
+          # pod. A failed cleanup waits for the next daily cron run.
+          restartPolicy: Never
           securityContext:
             runAsUser: 1000
             runAsNonRoot: true
           containers:
             - name: audit-cleanup
-              image: "{{ .Values.global.registry }}/{{ .Values.global.imageProject }}/{{ .Values.global.imageRepository }}/{{ .Values.app.imagePackage }}:{{ .Values.global.imageTag }}"
+              # `app` matches `.Values.app.imagePackage` in `values.yaml` — but
+              # that lookup lands in the `app` subchart's scope, not the
+              # project-level templates, so the literal is inlined here.
+              image: "{{ .Values.global.registry }}/{{ .Values.global.imageProject }}/{{ .Values.global.imageRepository }}/app:{{ .Values.global.imageTag }}"
               securityContext:
                 allowPrivilegeEscalation: false
               env:

packages/app/scripts/audit-cleanup.mjs

@@ -1,4 +1,5 @@
-import path from "node:path";
+import { realpathSync } from "node:fs";
+import { fileURLToPath } from "node:url";
 import postgres from "postgres";
 
 /**
@@ -11,9 +12,11 @@ import postgres from "postgres";
  *    (high-volume access logs containing IP addresses)
  *  - long retention (365 d by default): every other category (security logs)
  *
- * Both DELETEs and the self-audit INSERT run inside a single transaction — if
- * anything fails, the whole operation rolls back and the next daily run
- * re-attempts the purge.
+ * The two DELETEs are atomic (single transaction). The success self-audit is
+ * appended after the transaction commits — a failure writing the audit row
+ * must not roll back work we already accomplished. Cleanup failures go
+ * through `logFailure`, which inserts outside any transaction so the record
+ * survives the rollback.
  *
  * Env vars:
  *  - DATABASE_URL (or POSTGRES_* fallback, same convention as migrate.mjs)
@@ -112,7 +115,7 @@ export async function runAuditCleanup({
 	const shortThreshold = subtractDays(now, shortRetentionDays);
 	const longThreshold = subtractDays(now, longRetentionDays);
 
-	return await sql.begin(async (txRaw) => {
+	const summary = await sql.begin(async (txRaw) => {
 		// postgres-js `TransactionSql` is declared as `Omit<Sql, ...>`, which
 		// — quirk of TS's `Omit` — strips the call signatures on Sql. Cast
 		// back to Sql so we can use the template-tag + dynamic-array helper.
@@ -130,31 +133,46 @@ export async function runAuditCleanup({
 
 		const deletedShort = Number(shortResult.count ?? 0);
 		const deletedLong = Number(longResult.count ?? 0);
-		const deletedTotal = deletedShort + deletedLong;
+		return {
+			deletedShort,
+			deletedLong,
+			deletedTotal: deletedShort + deletedLong,
+		};
+	});
 
-		// `created_at` is NOT NULL with no SQL-level default — the drizzle
-		// schema supplies `$defaultFn(() => new Date())` at the ORM layer,
-		// which the raw SQL path has to replicate explicitly.
-		await tx`
+	// Self-audit runs OUTSIDE the transaction so a failure to record the
+	// audit row (constraint violation, disk full, etc.) cannot roll back
+	// cleanup work that has already been committed. We still log success
+	// via an INSERT so the cleanup cron is visible in `audit.action_log`.
+	// `created_at` is NOT NULL with no SQL-level default — the drizzle
+	// schema supplies `$defaultFn(() => new Date())` at the ORM layer,
+	// which the raw SQL path has to replicate explicitly.
+	try {
+		await sql`
 			INSERT INTO audit.action_log (id, created_at, action, category, status, metadata)
 			VALUES (
 				${crypto.randomUUID()},
 				${new Date()},
 				${AUDIT_CLEANUP_ACTION},
 				${AUDIT_CLEANUP_CATEGORY},
 				'success',
-				${tx.json({
-					deletedShort,
-					deletedLong,
-					deletedTotal,
+				${sql.json({
+					deletedShort: summary.deletedShort,
+					deletedLong: summary.deletedLong,
+					deletedTotal: summary.deletedTotal,
 					shortRetentionDays,
 					longRetentionDays,
 				})}
 			)
 		`;
+	} catch (auditError) {
+		console.error(
+			"[audit-cleanup] Cleanup succeeded but self-audit insert failed:",
+			auditError,
+		);
+	}
 
-		return { deletedShort, deletedLong, deletedTotal };
-	});
+	return summary;
 }
 
 /**
@@ -186,7 +204,14 @@ async function logFailure(sql, error) {
 const isMain = (() => {
 	const entry = process.argv[1];
 	if (!entry) return false;
-	return import.meta.url === `file://${path.resolve(entry)}`;
+	// `realpathSync` resolves symlinks — needed because pnpm's content-
+	// addressable store or Docker bind-mounts may hand Node a symlink path
+	// that doesn't match `import.meta.url`'s canonicalized form.
+	try {
+		return fileURLToPath(import.meta.url) === realpathSync(entry);
+	} catch {
+		return false;
+	}
 })();
 
 if (isMain) {

packages/app/src/server/audit/__tests__/auditCleanupScript.integration.test.ts

@@ -19,7 +19,10 @@ import { runAuditCleanup } from "#scripts/audit-cleanup.mjs";
 import { env } from "~/env.js";
 
 describe("audit-cleanup.mjs (integration)", () => {
-	let sql: ReturnType<typeof postgres>;
+	// Definite-assignment assertion — initialized synchronously in `beforeAll`.
+	// If `beforeAll` throws, vitest skips the tests and reports the original
+	// error; any afterAll failure from an undefined `sql` is suppressed below.
+	let sql!: ReturnType<typeof postgres>;
 
 	beforeAll(() => {
 		// DATABASE_URL is populated by `integration-setup.ts` before this file
@@ -28,6 +31,7 @@ describe("audit-cleanup.mjs (integration)", () => {
 	});
 
 	afterAll(async () => {
+		if (!sql) return;
 		await sql`DELETE FROM audit.action_log`;
 		await sql.end();
 	});