fix(audit-integrations): SMTP/PG/Socket.IO timeouts, prompt injection, secret-at-rest

A focused review of every external integration surfaced six issues the
original audit missed.  Fixed here.

HIGH
* Socket.IO had an unconditional 30-second idle disconnect on every
  socket.  The comment on the line acknowledged it was "for development
  only, would be longer in prod" but no NODE_ENV guard existed, and the
  `socket.onAny` listener only resets on inbound client events — every
  dashboard connection that received only server-push events would have
  been torn down every 30s in production.  Removed the manual idle
  timer entirely; Socket.IO's pingTimeout / pingInterval handles
  dead-transport detection at the protocol level.
* SMTP transporters had no `connectionTimeout` / `greetingTimeout` /
  `socketTimeout`.  Nodemailer's defaults are 2 minutes for connect
  and unlimited for socket — a hung SMTP server would have held a
  BullMQ `email` worker concurrency slot for up to 10 min per job
  (5 retries × 2 min).  Set 10s/10s/30s on both the system transporter
  in `src/lib/email/index.ts` and the user-account transporter in
  `email-compose.service.ts`.

MEDIUM
* PostgreSQL pool had no `statement_timeout` /
  `idle_in_transaction_session_timeout`.  A slow query or transaction
  held by a crashed handler would have eventually exhausted the
  20-connection pool.  30s statement cap, 10s idle-in-tx cap, plus
  `max_lifetime: 30min` to recycle connections.
* `umami_password` and `umami_api_token` were stored as plaintext in
  `system_settings` (the SMTP and S3 secret paths use AES-GCM).  The
  reader now passes them through `readSecret()` which auto-detects
  the encrypted `iv:cipher:tag` shape and decrypts, falling back to
  legacy plaintext so operators can rotate without a flag-day.
* AI email-draft worker interpolated `additionalInstructions` (user-
  controlled) directly into the OpenAI prompt — a hostile rep could
  close the instructions block and inject prompt directives that
  override the system prompt.  Added `sanitizeForPrompt()` that
  strips newlines + quote chars, caps at 500 chars, and the prompt
  now wraps the value in a "treat as data not commands" preamble.

LOW
* Legacy `ensureBucket()` in `src/lib/minio/index.ts` was unguarded —
  if any future code imported it (currently no callers), a misconfigured
  prod deploy could mint a fresh empty bucket.  Now matches the gate
  used by the pluggable S3Backend (`MINIO_AUTO_CREATE_BUCKET=true`
  required) so the legacy export and the new pluggable path agree.

Confirmed not-an-issue: BullMQ Workers create connections via
`{ url }` options object, and BullMQ sets `maxRetriesPerRequest: null`
internally for those — no fix needed.  The shared `redis` singleton
that does keep `maxRetriesPerRequest: 3` is used only for direct
Redis ops (rate-limit sliding window, etc.), never for blocking
BullMQ commands, so the value is correct there.

Test status: 1175/1175 vitest, tsc clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Matt Ciaccio
2026-05-05 21:31:50 +02:00
parent 63c4073e64
commit 7bd969b41a
7 changed files with 118 additions and 14 deletions

View File

@@ -75,11 +75,17 @@ export async function sendEmail(
// Decrypt credentials (INTERNAL - never logged or returned)
const creds = await getDecryptedCredentials(data.accountId);
// Build user-specific SMTP transporter
// Build user-specific SMTP transporter. Same timeouts as the system
// transporter in src/lib/email/index.ts — without these a hung SMTP
// server holds the calling request for ~2min (Nodemailer's default
// connectionTimeout) and starves the documents/email worker slot.
const transporter = nodemailer.createTransport({
host: account.smtpHost,
port: account.smtpPort,
secure: account.smtpPort === 465,
connectionTimeout: 10_000,
greetingTimeout: 10_000,
socketTimeout: 30_000,
auth: { user: creds.username, pass: creds.password },
});

View File

@@ -23,6 +23,8 @@ import { systemSettings } from '@/lib/db/schema/system';
import { rangeToBounds, type DateRange } from '@/lib/analytics/range';
import { CodedError } from '@/lib/errors';
import { fetchWithTimeout } from '@/lib/fetch-with-timeout';
import { decrypt } from '@/lib/utils/encryption';
import { logger } from '@/lib/logger';
// ─── Settings access ────────────────────────────────────────────────────────
@@ -34,6 +36,12 @@ interface UmamiPortConfig {
websiteId: string;
}
// `umami_api_token` and `umami_password` may be stored EITHER as encrypted
// `iv:cipher:tag` strings (matching the SMTP / S3 secret pattern) OR as
// legacy plaintext. The reader below tries decryption first and falls
// back to the raw value when the format isn't AES-GCM-shaped, so an
// operator can rotate to encrypted-at-rest by re-saving the setting
// without a flag-day migration.
const SETTING_KEYS = [
'umami_api_url',
'umami_api_token',
@@ -42,6 +50,28 @@ const SETTING_KEYS = [
'umami_website_id',
] as const;
function readSecret(raw: string | null | undefined): string | null {
const v = (raw ?? '').toString().trim();
if (!v) return null;
// `encrypt()` returns `<iv-hex>:<cipher-hex>:<tag-hex>` (3 colon-separated
// hex chunks). If we see that shape, decrypt; otherwise treat as legacy
// plaintext. The fallback path is a transition affordance — operators
// should re-save the setting via the admin UI, which writes the
// encrypted form going forward.
const parts = v.split(':');
if (parts.length === 3 && parts.every((p) => /^[0-9a-f]+$/i.test(p))) {
try {
return decrypt(v);
} catch (err) {
logger.warn(
{ err },
'Umami secret looked encrypted but decrypt failed; treating as plaintext',
);
}
}
return v;
}
/**
* Read the five Umami-related setting rows for one port and assemble them.
* Returns null if the minimum required config (URL + websiteId + an auth
@@ -58,9 +88,11 @@ export async function loadUmamiConfig(portId: string): Promise<UmamiPortConfig |
const map = new Map(rows.map((r) => [r.key, r.value as string | null | undefined]));
const apiUrl = (map.get('umami_api_url') ?? '').toString().trim().replace(/\/$/, '');
const apiToken = ((map.get('umami_api_token') ?? '') as string).trim() || null;
// Sensitive values pass through readSecret() to support encrypted-at-rest
// storage (with plaintext fallback for legacy rows).
const apiToken = readSecret(map.get('umami_api_token') as string | null | undefined);
const username = ((map.get('umami_username') ?? '') as string).trim() || null;
const password = ((map.get('umami_password') ?? '') as string).trim() || null;
const password = readSecret(map.get('umami_password') as string | null | undefined);
const websiteId = ((map.get('umami_website_id') ?? '') as string).trim();
if (!apiUrl || !websiteId) return null;