From e3e67748c95036a2da150c9bc78d966c16de5926 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Thu, 30 Jul 2026 05:30:41 +0000 Subject: [PATCH] bind the email folder filter instead of interpolating it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit /email/messages and /email/stats built `labels LIKE '%${folder}%'` by string interpolation, and `folder` comes straight off the query string. Five statements across the two handlers were exposed. The fragment is bound now, and it carries its parameters with it because each handler builds several statements from the same fragment and has to spread them in order. Checked against an in-memory table: inbox/INBOX/SENT/all return exactly what they returned before, and `x' OR 1=1 --` now matches nothing instead of being SQL. Also: page and limit reached the bindings as NaN for any non-numeric value, so a mistyped query param was a 500. They fall back to their defaults now. And deleted src/servers/sidecar/email-cron.ts — 92 lines imported by nothing. The live cron is sidecar/email/email-cron.ts; this was an older copy that still reached into queue-runner and google-auth directly, so leaving it there invites someone to fix the wrong file. This is the first commit on the email branch; the placement problems (the whole mail store, both syncs, and the resync coalescing that cannot work across processes) are untouched and much larger — see SIDECAR_WORK_LOG.md. Co-Authored-By: Claude Opus 4.8 --- src/servers/api/email/email.ts | 57 ++++++++++++------- src/servers/sidecar/email-cron.ts | 92 ------------------------------- 2 files changed, 38 insertions(+), 111 deletions(-) delete mode 100644 src/servers/sidecar/email-cron.ts diff --git a/src/servers/api/email/email.ts b/src/servers/api/email/email.ts index dddd0ebf..3066fa4e 100644 --- a/src/servers/api/email/email.ts +++ b/src/servers/api/email/email.ts @@ -14,7 +14,9 @@ export const emailRouter = createRouter(); emailRouter.route('/accounts', accountsRouter); // ── Sending (SMTP) — sends as the connected account using its app password ── -async function getSmtpTransport(userId: number): Promise<{ transport: ReturnType; from: string }> { +async function getSmtpTransport( + userId: number, +): Promise<{ transport: ReturnType; from: string }> { const accounts = await getEmailAccounts(userId); const acct = accounts.find((a) => a.enabled) ?? accounts[0]; if (!acct) throw errors.BAD_REQUEST('No email account configured'); @@ -35,9 +37,14 @@ emailRouter.post('/send', async (ctx) => { const to = str(form.to).trim(); if (!to) throw errors.BAD_REQUEST('At least one recipient is required'); - const toFiles = (raw: unknown) => (Array.isArray(raw) ? raw : raw ? [raw] : []).filter((f): f is File => f instanceof File); + const toFiles = (raw: unknown) => + (Array.isArray(raw) ? raw : raw ? [raw] : []).filter((f): f is File => f instanceof File); const attachments = await Promise.all( - toFiles(form.files).map(async (f) => ({ filename: f.name, content: Buffer.from(await f.arrayBuffer()), contentType: f.type || undefined })), + toFiles(form.files).map(async (f) => ({ + filename: f.name, + content: Buffer.from(await f.arrayBuffer()), + contentType: f.type || undefined, + })), ); // Inline images: cid `inline-` matches the `` the composer put in the html. const inline = await Promise.all( @@ -164,14 +171,24 @@ emailRouter.get('/search', async (ctx) => { } }); +// `folder` is request input and was being interpolated straight into the SQL. It is bound now — the +// fragment and its parameters travel together because each call site builds several statements from the +// same fragment and has to spread the params in the right order. +type FolderFilter = { where: string; params: string[] }; +const folderFilter = (folder: string): FolderFilter => + folder === 'all' + ? { where: 'deleted = 0', params: [] } + : { where: 'deleted = 0 AND labels LIKE ?', params: [`%${folder}%`] }; + emailRouter.get('/messages', async (ctx) => { const user = ctx.get('user'); - const page = Number(ctx.req.query('page') ?? '1'); - const limit = Number(ctx.req.query('limit') ?? '50'); + // `|| n` also catches NaN from a non-numeric query param, which used to reach the bindings as NaN. + const page = Math.max(Number(ctx.req.query('page') ?? '1') || 1, 1); + const limit = Math.max(Number(ctx.req.query('limit') ?? '50') || 50, 1); const folder = ctx.req.query('folder') ?? 'inbox'; const offset = (page - 1) * limit; - const folderWhere = folder === 'all' ? 'deleted = 0' : `deleted = 0 AND labels LIKE '%${folder}%'`; + const { where: folderWhere, params: folderParams } = folderFilter(folder); const db = await openUserEmailDb(user.email, user.id); if (!db) return ctx.json({ messages: [], total: 0 }); @@ -190,10 +207,10 @@ emailRouter.get('/messages', async (ctx) => { ) WHERE rn = 1 ORDER BY date DESC LIMIT ? OFFSET ?`, ) - .all(limit, offset) as Record[]; + .all(...folderParams, limit, offset) as Record[]; const countRow = db .query(`SELECT COUNT(DISTINCT COALESCE(thread_id, id)) as total FROM emails WHERE ${folderWhere}`) - .get() as { total: number }; + .get(...folderParams) as { total: number }; const messages = rows.map(rowToSummary); return ctx.json({ messages, total: countRow.total }); } finally { @@ -202,9 +219,9 @@ emailRouter.get('/messages', async (ctx) => { }); function buildMessage(db: ReturnType, row: Record): EmailMessage { - const attachmentRows = db.query('SELECT * FROM attachments WHERE email_id = ? ORDER BY idx').all(row.id as string) as Array< - Record - >; + const attachmentRows = db + .query('SELECT * FROM attachments WHERE email_id = ? ORDER BY idx') + .all(row.id as string) as Array>; const from = row.from_name ? `${row.from_name} <${row.from_address}>` : (row.from_address as string); return { id: row.id as string, @@ -250,9 +267,10 @@ emailRouter.get('/thread/:id', async (ctx) => { const db = await openUserEmailDb(user.email, user.id); if (!db) return ctx.text('Not found', 404); try { - const head = db.query('SELECT thread_id, subject FROM emails WHERE id = ? AND deleted = 0').get(id) as - | { thread_id: string | null; subject: string } - | null; + const head = db.query('SELECT thread_id, subject FROM emails WHERE id = ? AND deleted = 0').get(id) as { + thread_id: string | null; + subject: string; + } | null; if (!head) return ctx.text('Not found', 404); const threadKey = head.thread_id ?? id; @@ -363,23 +381,24 @@ emailRouter.get('/stats', async (ctx) => { const user = ctx.get('user'); const folder = ctx.req.query('folder') ?? 'inbox'; - const folderWhere = folder === 'all' ? 'deleted = 0' : `deleted = 0 AND labels LIKE '%${folder}%'`; + const { where: folderWhere, params: folderParams } = folderFilter(folder); const db = await openUserEmailDb(user.email, user.id); if (!db) return ctx.json({ total: 0, byDomain: [], bySender: [] }); try { - const total = (db.query(`SELECT COUNT(*) as count FROM emails WHERE ${folderWhere}`).get() as { count: number }) - .count; + const total = ( + db.query(`SELECT COUNT(*) as count FROM emails WHERE ${folderWhere}`).get(...folderParams) as { count: number } + ).count; const byDomain = db .query( `SELECT from_domain, COUNT(*) as count FROM emails WHERE ${folderWhere} GROUP BY from_domain ORDER BY count DESC LIMIT 20`, ) - .all() as Array<{ from_domain: string; count: number }>; + .all(...folderParams) as Array<{ from_domain: string; count: number }>; const bySender = db .query( `SELECT from_address, from_name, COUNT(*) as count FROM emails WHERE ${folderWhere} GROUP BY from_address ORDER BY count DESC LIMIT 20`, ) - .all() as Array<{ from_address: string; from_name: string; count: number }>; + .all(...folderParams) as Array<{ from_address: string; from_name: string; count: number }>; return ctx.json({ total, byDomain, bySender }); } finally { diff --git a/src/servers/sidecar/email-cron.ts b/src/servers/sidecar/email-cron.ts deleted file mode 100644 index 63eac8ba..00000000 --- a/src/servers/sidecar/email-cron.ts +++ /dev/null @@ -1,92 +0,0 @@ -import { getAllSyncedAccounts, getUserById } from 'officerdb'; -import { getValidGoogleAccessToken } from '../api/integrations/google-auth'; -import * as queueRunner from './queue-runner'; - -const INTERVAL_MS = 10 * 60 * 1000; // 10 minutes - -let timer: ReturnType | null = null; - -async function tick() { - try { - const accounts = await getAllSyncedAccounts(); - if (accounts.length === 0) return; - - const allJobs = await queueRunner.listAllJobs(); - const activeEmailSyncIds = new Set( - allJobs - .filter((j) => j.type === 'email-sync' && (j.status === 'queued' || j.status === 'running')) - .map((j) => (j.meta as Record | undefined)?.emailAccountId), - ); - - for (const account of accounts) { - if (activeEmailSyncIds.has(account.id)) continue; - - const user = await getUserById(account.userId); - if (!user) continue; - - // Resolve IMAP auth - const imapAuth: Record = { user: account.email }; - if (account.authType === 'oauth') { - let accessToken: string | null = null; - try { - accessToken = await getValidGoogleAccessToken(account.userId); - } catch (err) { - console.log(`[email-cron] Skipping ${account.email}: token refresh failed —`, err instanceof Error ? err.message : err); - continue; - } - if (!accessToken) { - console.log(`[email-cron] Skipping ${account.email}: no OAuth access token`); - continue; - } - imapAuth.accessToken = accessToken; - } else { - const creds = account.credentials as Record; - imapAuth.pass = creds.password; - } - - try { - await queueRunner.enqueue({ - lane: 'email', - type: 'email-sync', - userId: user.email, - meta: { - emailAccountId: account.id, - userEmail: user.email, - account: { - id: account.id, - userId: account.userId, - email: account.email, - imapHost: account.imapHost, - imapPort: account.imapPort, - imapSecure: account.imapSecure, - provider: account.provider, - authType: account.authType, - credentials: account.credentials, - }, - imapAuth, - }, - }); - console.log(`[email-cron] Enqueued incremental sync for ${account.email}`); - } catch (err) { - console.error(`[email-cron] Failed to enqueue sync for ${account.email}:`, err instanceof Error ? err.message : err); - } - } - } catch (err) { - console.error('[email-cron] Error:', err instanceof Error ? err.message : err); - } -} - -export function initEmailCron() { - if (timer) return; - console.log(`[email-cron] Starting email sync cron (every ${INTERVAL_MS / 60_000} min)`); - timer = setInterval(tick, INTERVAL_MS); - // Run first tick after a short delay to let the queue initialize - setTimeout(tick, 30_000); -} - -export function stopEmailCron() { - if (timer) { - clearInterval(timer); - timer = null; - } -}