mirror of
https://github.com/Memo-2023/mana-monorepo.git
synced 2026-05-14 20:21:09 +02:00
fix(mana-auth): security_events INSERT crashed on undefined optional fields
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.
This commit is contained in:
parent
4fce6a3ede
commit
ed746297b5
1 changed files with 17 additions and 2 deletions
|
|
@ -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
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue