From ed746297b5b439004463c3954b12c77592a26567 Mon Sep 17 00:00:00 2001
From: Till JS <tills95@gmail.com>
Date: Wed, 8 Apr 2026 17:59:23 +0200
Subject: [PATCH] fix(mana-auth): security_events INSERT crashed on undefined
 optional fields
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

logEvent() builds its INSERT via a raw `sql` tagged template:

    sql\`INSERT INTO auth.security_events
        (..., user_id, ip_address, user_agent, metadata, ...)
        VALUES (..., \${params.userId}, \${params.ipAddress},
                     \${params.userAgent}, \${...metadata}, ...)\`

Most call sites only pass userId+eventType (or only eventType for the
LOGIN_FAILURE / PASSWORD_RESET_REQUESTED / PROFILE_UPDATED /
PASSWORD_CHANGED / ACCOUNT_DELETED events). The other params land in
the template as `undefined`, and postgres-js's tagged-template renderer
collapses `${undefined}` into literal nothing — producing this:

    VALUES (gen_random_uuid(), $1, $2, , , $3::jsonb, NOW())
                                       ^^^^

Postgres rejects with "syntax error at or near \",\"". The catch block
swallowed it as a `console.warn('Failed to log security event
(non-critical):', params.eventType)` with no error detail, which is why
this has been silently broken for who knows how long — every register,
every login, every password change has been losing its audit row.

Fix:
- Coerce optional params to `null` (`params.userId ?? null`) before
  interpolation. NULL is what postgres-js renders for an explicit null.
- Surface the actual error in the catch warn so the next time something
  similar happens it shows up in logs instead of just "non-critical".

Verified the diagnosis by toggling `log_statement = all` on the test
postgres, triggering a register, and reading the literal failed
statement out of postgres logs.
---
 services/mana-auth/src/services/security.ts | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/services/mana-auth/src/services/security.ts b/services/mana-auth/src/services/security.ts
index bb91555b6..37ebe43c3 100644
--- a/services/mana-auth/src/services/security.ts
+++ b/services/mana-auth/src/services/security.ts
@@ -38,14 +38,29 @@ export class SecurityEventsService {
 		userAgent?: string;
 		metadata?: Record<string, unknown>;
 	}) {
+		// postgres-js renders `undefined` as literal nothing in tagged-template
+		// SQL — `${undefined}` collapses the parameter slot, producing
+		// `VALUES (..., , , ...)` and a syntax error. Explicitly fall back to
+		// `null` so optional fields go in as NULL.
+		const userId = params.userId ?? null;
+		const ipAddress = params.ipAddress ?? null;
+		const userAgent = params.userAgent ?? null;
+		const metadata = JSON.stringify(params.metadata ?? {});
 		try {
 			// Use raw SQL since securityEvents table may be in auth schema
 			await this.db.execute(
 				sql`INSERT INTO auth.security_events (id, user_id, event_type, ip_address, user_agent, metadata, created_at)
-				VALUES (gen_random_uuid(), ${params.userId}, ${params.eventType}, ${params.ipAddress}, ${params.userAgent}, ${JSON.stringify(params.metadata || {})}::jsonb, NOW())`
+				VALUES (gen_random_uuid(), ${userId}, ${params.eventType}, ${ipAddress}, ${userAgent}, ${metadata}::jsonb, NOW())`
 			);
 		} catch (error) {
-			console.warn('Failed to log security event (non-critical):', params.eventType);
+			// Audit logging is non-critical, so we never throw — but actually
+			// surface the error message so the failure mode is debuggable
+			// instead of a silent warn that hides the real cause.
+			console.warn(
+				'Failed to log security event (non-critical):',
+				params.eventType,
+				error instanceof Error ? error.message : error
+			);
 		}
 	}