Fix 11 bugs: restore crash, draft leak, email failures, memory leaks
Critical fixes: - sanitizeLoadedHitStats/VisitorStats: restore full state shape so a snapshot restore no longer crashes hit-counting middleware (missing byPathReal, byPathBot, byDayReal, byDayBot, botReasons, ipHashIndex) - /questions/share/🆔 read state.questions only, not draft questions - inbound-email: validate date with Number.isFinite before toISOString - study-reminders: wrap each send in try/catch so one failure doesn't block remaining users; persist sent-markers after each success Security: - getClientIp: use req.ip (trust-proxy-resolved) instead of raw x-forwarded-for header to prevent IP spoofing - env-snapshot.env: delete immediately after backup tar stream ends so secrets don't linger on disk between exports Correctness / UX: - contact form: email failures no longer 500 the user after the submission is already saved; log and fall through instead - study-account profile: cap data URI avatar at 6 MB - admin enrollment PATCH: validate slug against study catalog - signup: return 503 at MAX_STUDY_USERS instead of silently dropping oldest accounts Memory leaks: - contactHits, downloadHits Maps: prune stale entries at 5000 entries - resendEmailSubmissionIndex: trim to 2000 entries (oldest first) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
+20
-1
@@ -1058,21 +1058,40 @@ function sanitizeLoadedContactSubmissions(value) {
|
|||||||
function sanitizeLoadedHitStats(value) {
|
function sanitizeLoadedHitStats(value) {
|
||||||
return {
|
return {
|
||||||
totalHits: Number(value?.totalHits) || 0,
|
totalHits: Number(value?.totalHits) || 0,
|
||||||
|
realHits: Number(value?.realHits) || 0,
|
||||||
|
botHits: Number(value?.botHits) || 0,
|
||||||
firstHitAt: typeof value?.firstHitAt === 'string' ? value.firstHitAt : null,
|
firstHitAt: typeof value?.firstHitAt === 'string' ? value.firstHitAt : null,
|
||||||
lastHitAt: typeof value?.lastHitAt === 'string' ? value.lastHitAt : null,
|
lastHitAt: typeof value?.lastHitAt === 'string' ? value.lastHitAt : null,
|
||||||
byPath: value?.byPath && typeof value.byPath === 'object' ? value.byPath : {},
|
byPath: value?.byPath && typeof value.byPath === 'object' ? value.byPath : {},
|
||||||
|
byPathReal: value?.byPathReal && typeof value.byPathReal === 'object' ? value.byPathReal : {},
|
||||||
|
byPathBot: value?.byPathBot && typeof value.byPathBot === 'object' ? value.byPathBot : {},
|
||||||
byDay: value?.byDay && typeof value.byDay === 'object' ? value.byDay : {},
|
byDay: value?.byDay && typeof value.byDay === 'object' ? value.byDay : {},
|
||||||
|
byDayReal: value?.byDayReal && typeof value.byDayReal === 'object' ? value.byDayReal : {},
|
||||||
|
byDayBot: value?.byDayBot && typeof value.byDayBot === 'object' ? value.byDayBot : {},
|
||||||
|
botReasons: value?.botReasons && typeof value.botReasons === 'object' ? value.botReasons : {},
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
function sanitizeLoadedVisitorStats(value) {
|
function sanitizeLoadedVisitorStats(value) {
|
||||||
|
const loadedVisitors = value?.visitors && typeof value.visitors === 'object' ? value.visitors : {}
|
||||||
|
|
||||||
|
let ipHashIndex = value?.ipHashIndex && typeof value.ipHashIndex === 'object' ? value.ipHashIndex : {}
|
||||||
|
if (Object.keys(ipHashIndex).length === 0 && Object.keys(loadedVisitors).length > 0) {
|
||||||
|
for (const [vid, visitor] of Object.entries(loadedVisitors)) {
|
||||||
|
if (visitor?.ipHash && typeof visitor.ipHash === 'string') {
|
||||||
|
ipHashIndex[visitor.ipHash] = vid
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
return {
|
return {
|
||||||
totalVisits: Number(value?.totalVisits) || 0,
|
totalVisits: Number(value?.totalVisits) || 0,
|
||||||
uniqueVisitors: Number(value?.uniqueVisitors) || 0,
|
uniqueVisitors: Number(value?.uniqueVisitors) || 0,
|
||||||
returningVisits: Number(value?.returningVisits) || 0,
|
returningVisits: Number(value?.returningVisits) || 0,
|
||||||
firstVisitAt: typeof value?.firstVisitAt === 'string' ? value.firstVisitAt : null,
|
firstVisitAt: typeof value?.firstVisitAt === 'string' ? value.firstVisitAt : null,
|
||||||
lastVisitAt: typeof value?.lastVisitAt === 'string' ? value.lastVisitAt : null,
|
lastVisitAt: typeof value?.lastVisitAt === 'string' ? value.lastVisitAt : null,
|
||||||
visitors: value?.visitors && typeof value.visitors === 'object' ? value.visitors : {},
|
visitors: loadedVisitors,
|
||||||
|
ipHashIndex,
|
||||||
recentVisits: Array.isArray(value?.recentVisits) ? value.recentVisits.slice(0, MAX_RECENT_VISITS) : [],
|
recentVisits: Array.isArray(value?.recentVisits) ? value.recentVisits.slice(0, MAX_RECENT_VISITS) : [],
|
||||||
geoCacheByIp: value?.geoCacheByIp && typeof value.geoCacheByIp === 'object' ? value.geoCacheByIp : {},
|
geoCacheByIp: value?.geoCacheByIp && typeof value.geoCacheByIp === 'object' ? value.geoCacheByIp : {},
|
||||||
}
|
}
|
||||||
|
|||||||
+2
-4
@@ -472,10 +472,8 @@ export function normalizeIp(rawIp) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
export function getClientIp(req) {
|
export function getClientIp(req) {
|
||||||
const forwarded = req.headers['x-forwarded-for']
|
// Use req.ip: Express derives this from x-forwarded-for according to the
|
||||||
if (forwarded) {
|
// configured trust proxy hop count, preventing header spoofing by clients.
|
||||||
return normalizeIp(forwarded)
|
|
||||||
}
|
|
||||||
return normalizeIp(req.ip)
|
return normalizeIp(req.ip)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -14,6 +14,8 @@ import {
|
|||||||
import {
|
import {
|
||||||
hashStudyPassword,
|
hashStudyPassword,
|
||||||
getStudyCatalog,
|
getStudyCatalog,
|
||||||
|
normalizeStudySlug,
|
||||||
|
isEnrollableStudySlug,
|
||||||
} from '../study-helpers.js'
|
} from '../study-helpers.js'
|
||||||
|
|
||||||
export function register(app) {
|
export function register(app) {
|
||||||
@@ -157,7 +159,10 @@ export function register(app) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
if (typeof addEnrollment === 'string' && addEnrollment.trim()) {
|
if (typeof addEnrollment === 'string' && addEnrollment.trim()) {
|
||||||
const slug = addEnrollment.trim()
|
const slug = normalizeStudySlug(addEnrollment)
|
||||||
|
if (!slug || !isEnrollableStudySlug(slug)) {
|
||||||
|
res.status(400).json({ message: `Unknown study slug: ${addEnrollment.trim()}` }); return
|
||||||
|
}
|
||||||
if (!Array.isArray(user.enrolledStudySlugs)) user.enrolledStudySlugs = []
|
if (!Array.isArray(user.enrolledStudySlugs)) user.enrolledStudySlugs = []
|
||||||
if (!user.enrolledStudySlugs.includes(slug)) user.enrolledStudySlugs.push(slug)
|
if (!user.enrolledStudySlugs.includes(slug)) user.enrolledStudySlugs.push(slug)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,4 +1,4 @@
|
|||||||
import { mkdir, mkdtemp, readdir, rm, cp, writeFile } from 'node:fs/promises'
|
import { mkdir, mkdtemp, readdir, rm, cp, writeFile, unlink } from 'node:fs/promises'
|
||||||
import os from 'node:os'
|
import os from 'node:os'
|
||||||
import path from 'node:path'
|
import path from 'node:path'
|
||||||
import express from 'express'
|
import express from 'express'
|
||||||
@@ -62,6 +62,8 @@ const ENV_SNAPSHOT_KEYS = [
|
|||||||
'TITUS_STUDY_DOWNLOAD_NAME',
|
'TITUS_STUDY_DOWNLOAD_NAME',
|
||||||
]
|
]
|
||||||
|
|
||||||
|
// Write env snapshot into DATA_DIR for inclusion in the archive, then delete
|
||||||
|
// it immediately after the stream finishes so secrets don't linger on disk.
|
||||||
async function writeEnvSnapshot() {
|
async function writeEnvSnapshot() {
|
||||||
const lines = [
|
const lines = [
|
||||||
'# Siteforge environment snapshot — regenerated on every full backup export.',
|
'# Siteforge environment snapshot — regenerated on every full backup export.',
|
||||||
@@ -78,6 +80,10 @@ async function writeEnvSnapshot() {
|
|||||||
await writeFile(path.join(DATA_DIR, 'env-snapshot.env'), `${lines.join('\n')}\n`, 'utf8')
|
await writeFile(path.join(DATA_DIR, 'env-snapshot.env'), `${lines.join('\n')}\n`, 'utf8')
|
||||||
}
|
}
|
||||||
|
|
||||||
|
async function deleteEnvSnapshot() {
|
||||||
|
await unlink(path.join(DATA_DIR, 'env-snapshot.env')).catch(() => {})
|
||||||
|
}
|
||||||
|
|
||||||
// Files that identify an archive as a Siteforge data backup. At least one
|
// Files that identify an archive as a Siteforge data backup. At least one
|
||||||
// must be present at the top level of an uploaded archive before we restore.
|
// must be present at the top level of an uploaded archive before we restore.
|
||||||
const KNOWN_DATA_FILES = [
|
const KNOWN_DATA_FILES = [
|
||||||
@@ -170,9 +176,12 @@ export function register(app) {
|
|||||||
console.error('[admin-backup] export stream failed:', err)
|
console.error('[admin-backup] export stream failed:', err)
|
||||||
res.destroy(err)
|
res.destroy(err)
|
||||||
})
|
})
|
||||||
|
archive.on('end', () => { deleteEnvSnapshot() })
|
||||||
|
res.on('close', () => { deleteEnvSnapshot() })
|
||||||
archive.pipe(res)
|
archive.pipe(res)
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
console.error('[admin-backup] export failed:', err)
|
console.error('[admin-backup] export failed:', err)
|
||||||
|
deleteEnvSnapshot()
|
||||||
if (!res.headersSent) res.status(500).json({ message: 'Full backup export failed.' })
|
if (!res.headersSent) res.status(500).json({ message: 'Full backup export failed.' })
|
||||||
}
|
}
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -65,6 +65,11 @@ function registerResendMessageForSubmission(submissionId, stream, sendResult) {
|
|||||||
const resendMessageId = extractResendMessageId(sendResult)
|
const resendMessageId = extractResendMessageId(sendResult)
|
||||||
if (!resendMessageId || !submissionId || !stream) return
|
if (!resendMessageId || !submissionId || !stream) return
|
||||||
state.resendEmailSubmissionIndex.set(resendMessageId, { submissionId, stream })
|
state.resendEmailSubmissionIndex.set(resendMessageId, { submissionId, stream })
|
||||||
|
// Trim the index when it grows large; oldest entries are least likely to receive webhooks
|
||||||
|
if (state.resendEmailSubmissionIndex.size > 2000) {
|
||||||
|
const firstKey = state.resendEmailSubmissionIndex.keys().next().value
|
||||||
|
state.resendEmailSubmissionIndex.delete(firstKey)
|
||||||
|
}
|
||||||
upsertContactEmailStatus(submissionId, stream, { resendEmailId: resendMessageId })
|
upsertContactEmailStatus(submissionId, stream, { resendEmailId: resendMessageId })
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -82,6 +87,11 @@ function contactRateLimit(req, res, next) {
|
|||||||
if (now - entry.start > windowMs) { entry.count = 0; entry.start = now }
|
if (now - entry.start > windowMs) { entry.count = 0; entry.start = now }
|
||||||
entry.count += 1
|
entry.count += 1
|
||||||
contactHits.set(ip, entry)
|
contactHits.set(ip, entry)
|
||||||
|
// Prune stale entries to prevent unbounded growth
|
||||||
|
if (contactHits.size > 5000) {
|
||||||
|
const cutoff = now - windowMs
|
||||||
|
for (const [k, v] of contactHits) { if (v.start < cutoff) contactHits.delete(k) }
|
||||||
|
}
|
||||||
if (entry.count > 5) {
|
if (entry.count > 5) {
|
||||||
res.status(429).json({ message: 'Too many messages. Please wait a few minutes.' })
|
res.status(429).json({ message: 'Too many messages. Please wait a few minutes.' })
|
||||||
return
|
return
|
||||||
@@ -251,7 +261,8 @@ export function register(app) {
|
|||||||
lastEventType: 'email.failed',
|
lastEventType: 'email.failed',
|
||||||
error: String(welcomeErr?.message ?? welcomeErr ?? 'unknown error').slice(0, 600),
|
error: String(welcomeErr?.message ?? welcomeErr ?? 'unknown error').slice(0, 600),
|
||||||
})
|
})
|
||||||
throw welcomeErr
|
console.error('[contact] welcome email failed:', welcomeErr)
|
||||||
|
// Submission is already saved — don't 500 the user; fall through to admin notification.
|
||||||
}
|
}
|
||||||
} else if (shouldSendWelcome && USE_RESEND_AUTOMATION_WELCOME) {
|
} else if (shouldSendWelcome && USE_RESEND_AUTOMATION_WELCOME) {
|
||||||
upsertContactEmailStatus(submission.id, 'welcome', { status: 'automation-enabled', lastEventType: 'email.automation.enabled', error: null })
|
upsertContactEmailStatus(submission.id, 'welcome', { status: 'automation-enabled', lastEventType: 'email.automation.enabled', error: null })
|
||||||
@@ -284,7 +295,8 @@ export function register(app) {
|
|||||||
lastEventType: 'email.failed',
|
lastEventType: 'email.failed',
|
||||||
error: String(adminSendErr?.message ?? adminSendErr ?? 'unknown error').slice(0, 600),
|
error: String(adminSendErr?.message ?? adminSendErr ?? 'unknown error').slice(0, 600),
|
||||||
})
|
})
|
||||||
throw adminSendErr
|
console.error('[contact] admin notification email failed:', adminSendErr)
|
||||||
|
// Submission is already saved — don't 500 the user.
|
||||||
}
|
}
|
||||||
|
|
||||||
res.json({ ok: true, welcomeSent, welcomeHandledByAutomation: shouldSendWelcome && USE_RESEND_AUTOMATION_WELCOME })
|
res.json({ ok: true, welcomeSent, welcomeHandledByAutomation: shouldSendWelcome && USE_RESEND_AUTOMATION_WELCOME })
|
||||||
|
|||||||
@@ -28,6 +28,11 @@ function studyDownloadRateLimit(req, res, next) {
|
|||||||
if (now - entry.start > windowMs) { entry.count = 0; entry.start = now }
|
if (now - entry.start > windowMs) { entry.count = 0; entry.start = now }
|
||||||
entry.count += 1
|
entry.count += 1
|
||||||
downloadHits.set(ip, entry)
|
downloadHits.set(ip, entry)
|
||||||
|
// Prune stale entries to prevent unbounded growth
|
||||||
|
if (downloadHits.size > 5000) {
|
||||||
|
const cutoff = now - windowMs
|
||||||
|
for (const [k, v] of downloadHits) { if (v.start < cutoff) downloadHits.delete(k) }
|
||||||
|
}
|
||||||
if (entry.count > 10) {
|
if (entry.count > 10) {
|
||||||
res.status(429).json({ message: 'Too many download requests. Please wait a few minutes.' })
|
res.status(429).json({ message: 'Too many download requests. Please wait a few minutes.' })
|
||||||
return
|
return
|
||||||
|
|||||||
@@ -43,7 +43,7 @@ export function register(app) {
|
|||||||
|
|
||||||
const submission = {
|
const submission = {
|
||||||
id: randomUUID(),
|
id: randomUUID(),
|
||||||
submittedAt: date ? new Date(date).toISOString() : new Date().toISOString(),
|
submittedAt: (typeof date === 'string' || typeof date === 'number') && Number.isFinite(Date.parse(date)) ? new Date(date).toISOString() : new Date().toISOString(),
|
||||||
name: fromName || fromEmail,
|
name: fromName || fromEmail,
|
||||||
email: fromEmail,
|
email: fromEmail,
|
||||||
message: [subject ? `Subject: ${subject}` : '', body ?? ''].filter(Boolean).join('\n\n'),
|
message: [subject ? `Subject: ${subject}` : '', body ?? ''].filter(Boolean).join('\n\n'),
|
||||||
|
|||||||
@@ -93,7 +93,7 @@ export function register(app) {
|
|||||||
if (!id || !/^[\w-]{1,120}$/.test(id)) {
|
if (!id || !/^[\w-]{1,120}$/.test(id)) {
|
||||||
res.redirect(302, '/questions'); return
|
res.redirect(302, '/questions'); return
|
||||||
}
|
}
|
||||||
const sourceQuestions = state.draftQuestions ?? state.questions
|
const sourceQuestions = state.questions
|
||||||
const question = sourceQuestions.find(q => q.id === id && q.isApproved === true && q.answer)
|
const question = sourceQuestions.find(q => q.id === id && q.isApproved === true && q.answer)
|
||||||
if (!question) {
|
if (!question) {
|
||||||
res.redirect(302, '/questions'); return
|
res.redirect(302, '/questions'); return
|
||||||
|
|||||||
@@ -237,6 +237,10 @@ export function register(app) {
|
|||||||
res.status(400).json({ message: 'Avatar must be a valid uploaded image, data URI, or https URL.' })
|
res.status(400).json({ message: 'Avatar must be a valid uploaded image, data URI, or https URL.' })
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
if (avatarUrl.startsWith('data:image/') && avatarUrl.length > 6 * 1024 * 1024) {
|
||||||
|
res.status(400).json({ message: 'Avatar data URI is too large. Please use the upload endpoint instead.' })
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
user.displayName = displayName
|
user.displayName = displayName
|
||||||
user.avatarUrl = avatarUrl
|
user.avatarUrl = avatarUrl
|
||||||
|
|||||||
@@ -81,6 +81,11 @@ export function register(app) {
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (state.studyUsers.length >= MAX_STUDY_USERS) {
|
||||||
|
res.status(503).json({ message: 'Account registration is temporarily unavailable. Please try again later.' })
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
const now = new Date().toISOString()
|
const now = new Date().toISOString()
|
||||||
const user = {
|
const user = {
|
||||||
id: randomUUID(),
|
id: randomUUID(),
|
||||||
@@ -97,9 +102,6 @@ export function register(app) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
state.studyUsers.push(user)
|
state.studyUsers.push(user)
|
||||||
if (state.studyUsers.length > MAX_STUDY_USERS) {
|
|
||||||
state.studyUsers = state.studyUsers.slice(state.studyUsers.length - MAX_STUDY_USERS)
|
|
||||||
}
|
|
||||||
queueStudyUsersWrite()
|
queueStudyUsersWrite()
|
||||||
|
|
||||||
if (subscribe) {
|
if (subscribe) {
|
||||||
|
|||||||
@@ -650,9 +650,16 @@ export async function scheduleStudyReminders(sendStudyReminderEmail) {
|
|||||||
const canonical = state.cachedSiteContent?.seo?.canonicalUrl || 'https://versebyversewithnate.us/'
|
const canonical = state.cachedSiteContent?.seo?.canonicalUrl || 'https://versebyversewithnate.us/'
|
||||||
const base = canonical.endsWith('/') ? canonical.slice(0, -1) : canonical
|
const base = canonical.endsWith('/') ? canonical.slice(0, -1) : canonical
|
||||||
const sectionUrl = `${base}/study/${study.slug}/${section.id}`
|
const sectionUrl = `${base}/study/${study.slug}/${section.id}`
|
||||||
|
try {
|
||||||
await sendStudyReminderEmail(email, displayName, study.title, section.title, section.reference, sectionUrl)
|
await sendStudyReminderEmail(email, displayName, study.title, section.title, section.reference, sectionUrl)
|
||||||
|
} catch (err) {
|
||||||
|
console.error(`[study-reminders] failed to send reminder to ${email} for ${studySlug}/${sectionId}:`, err)
|
||||||
|
continue
|
||||||
|
}
|
||||||
userSent[studySlug] = [...sentForStudy, sectionId]
|
userSent[studySlug] = [...sentForStudy, sectionId]
|
||||||
state.studyReminders.users[user.id] = userSent
|
state.studyReminders.users[user.id] = userSent
|
||||||
|
// Persist after each successful send so a later crash doesn't re-send
|
||||||
|
queueStudyRemindersWrite()
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user