Second hardening pass: atomic data writes, bible-passage limits, webhook SSRF guard, note sanitizing, CI gates; v2.10.1
Publish Container / build (push) Successful in 15m46s
Publish Container / build (push) Successful in 15m46s
- Atomic temp-file + rename for all JSON data writes (no truncated files on crash) - /api/bible-passage: rate limit, range caps (60 verses), bounded cache - Admin webhooks: https-only, block private/internal/Tailscale addresses (checked at registration and delivery), no redirects - Resend webhook token compare is timing-safe - DOMPurify on rendered study notes and broadcast preview - CI runs lint + tests before build - Permissions-Policy and HSTS (production) headers; JSON error handler for /api - Tests for URL safety and atomic writes Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
8a796964b9
commit
2a64f29a00
18 files changed
+209
-17
No files matched your search
@@ -0,0 +1,17 @@
|
||||
import { writeFile as fsWriteFile, rename, unlink } from 'node:fs/promises'
|
||||
import { randomBytes } from 'node:crypto'
|
||||
|
||||
// Drop-in replacement for fs.promises.writeFile that never leaves a truncated
|
||||
// file behind: the data goes to a sibling temp file which is then renamed over
|
||||
// the target (atomic on the same filesystem). A crash mid-write leaves the
|
||||
// previous version intact instead of a half-written JSON file.
|
||||
export async function writeFile(file, data, options) {
|
||||
const tmp = `${file}.${process.pid}.${randomBytes(4).toString('hex')}.tmp`
|
||||
try {
|
||||
await fsWriteFile(tmp, data, options)
|
||||
await rename(tmp, file)
|
||||
} catch (err) {
|
||||
await unlink(tmp).catch(() => {})
|
||||
throw err
|
||||
}
|
||||
}
|
||||
+2
-1
@@ -1,5 +1,6 @@
|
||||
import { createHash, randomUUID, timingSafeEqual, createHmac, randomFillSync } from 'node:crypto'
|
||||
import { readFile, writeFile } from 'node:fs/promises'
|
||||
import { readFile } from 'node:fs/promises'
|
||||
import { writeFile } from './atomic-write.js'
|
||||
import { readFileSync } from 'node:fs'
|
||||
import path from 'node:path'
|
||||
import { parseCookies, cookieSecureFlag } from './helpers.js'
|
||||
|
||||
+7
-3
@@ -1,5 +1,7 @@
|
||||
import { migrateScriptKeys } from './episode-script-keys.js'
|
||||
import { mkdir, readFile, readdir, stat, unlink, writeFile } from 'node:fs/promises'
|
||||
import { mkdir, readFile, readdir, stat, unlink } from 'node:fs/promises'
|
||||
import { writeFile } from './atomic-write.js'
|
||||
import { assertPublicHttpsUrl } from './url-safety.js'
|
||||
import path from 'node:path'
|
||||
import { randomUUID } from 'node:crypto'
|
||||
import { sanitizeSiteContent } from './helpers.js'
|
||||
@@ -533,12 +535,14 @@ export async function fireWebhooks(eventType, payload) {
|
||||
if (targets.length === 0) return
|
||||
const body = JSON.stringify({ event: eventType, data: payload, firedAt: new Date().toISOString() })
|
||||
for (const wh of targets) {
|
||||
fetch(wh.url, {
|
||||
// Re-check at delivery time: DNS may have changed since the webhook was registered.
|
||||
assertPublicHttpsUrl(wh.url).then(() => fetch(wh.url, {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json', 'User-Agent': 'Siteforge-Webhook/1.0' },
|
||||
body,
|
||||
redirect: 'manual',
|
||||
signal: AbortSignal.timeout(8000),
|
||||
})
|
||||
}))
|
||||
.then(r => {
|
||||
appendAuditEntry('webhook-fired', `${eventType} → ${wh.url} (${r.status})`)
|
||||
})
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { readFile, writeFile } from 'node:fs/promises'
|
||||
import { readFile } from 'node:fs/promises'
|
||||
import { writeFile } from './atomic-write.js'
|
||||
import { sendNuggetsUploadNotification } from './email.js'
|
||||
import {
|
||||
SEAFILE_URL,
|
||||
|
||||
@@ -1,4 +1,6 @@
|
||||
import { mkdir, readFile, writeFile } from 'node:fs/promises'
|
||||
import { mkdir, readFile } from 'node:fs/promises'
|
||||
import { writeFile } from '../atomic-write.js'
|
||||
import { assertPublicHttpsUrl } from '../url-safety.js'
|
||||
import { sanitizeSiteContent } from '../helpers.js'
|
||||
import { requireAdminAuth, isValidAdminSession } from '../auth.js'
|
||||
import {
|
||||
@@ -524,12 +526,18 @@ export function register(app) {
|
||||
res.json({ webhooks: state.webhooks })
|
||||
})
|
||||
|
||||
app.post('/api/admin-webhooks', requireAdminAuth, (req, res) => {
|
||||
app.post('/api/admin-webhooks', requireAdminAuth, async (req, res) => {
|
||||
const { url, events } = req.body ?? {}
|
||||
if (!url || typeof url !== 'string' || !/^https:\/\//i.test(url.trim())) {
|
||||
res.status(400).json({ message: 'A valid HTTPS URL is required.' })
|
||||
return
|
||||
}
|
||||
try {
|
||||
await assertPublicHttpsUrl(url.trim())
|
||||
} catch (err) {
|
||||
res.status(400).json({ message: `Webhook URL rejected: ${err.message}` })
|
||||
return
|
||||
}
|
||||
if (state.webhooks.length >= 50) {
|
||||
res.status(400).json({ message: 'Maximum of 50 webhooks reached.' })
|
||||
return
|
||||
|
||||
@@ -1,6 +1,19 @@
|
||||
import rateLimit from 'express-rate-limit'
|
||||
import { BIBLE_BOOKS } from '../../shared/bible-books.js'
|
||||
|
||||
const passageCache = new Map()
|
||||
const PASSAGE_CACHE_MAX = 2000
|
||||
const MAX_CHAPTER = 150
|
||||
const MAX_VERSE = 176
|
||||
const MAX_VERSES_PER_REQUEST = 60
|
||||
|
||||
const passageRateLimiter = rateLimit({
|
||||
windowMs: 60 * 1000,
|
||||
max: 60,
|
||||
standardHeaders: true,
|
||||
legacyHeaders: false,
|
||||
message: { message: 'Too many passage requests. Please slow down.' },
|
||||
})
|
||||
|
||||
// Keyed by lowercase, whitespace-stripped book name/alias (matching the format
|
||||
// the admin's "Bible Book" field and the /api/bible-passage `book` query param
|
||||
@@ -27,7 +40,7 @@ function parseReference(reference) {
|
||||
}
|
||||
|
||||
export function register(app) {
|
||||
app.get('/api/bible-passage', async (req, res) => {
|
||||
app.get('/api/bible-passage', passageRateLimiter, async (req, res) => {
|
||||
const book = String(req.query.book ?? '').trim().toLowerCase().replace(/[\s-]/g, '')
|
||||
const reference = String(req.query.reference ?? '').trim()
|
||||
|
||||
@@ -43,6 +56,16 @@ export function register(app) {
|
||||
return
|
||||
}
|
||||
|
||||
if (
|
||||
parsed.chapter < 1 || parsed.chapter > MAX_CHAPTER
|
||||
|| parsed.verseStart < 1 || parsed.verseEnd > MAX_VERSE
|
||||
|| parsed.verseEnd < parsed.verseStart
|
||||
|| parsed.verseEnd - parsed.verseStart + 1 > MAX_VERSES_PER_REQUEST
|
||||
) {
|
||||
res.status(400).json({ message: `Passage out of range (max ${MAX_VERSES_PER_REQUEST} verses).` })
|
||||
return
|
||||
}
|
||||
|
||||
const cacheKey = `BSB:${bookNum}:${parsed.chapter}:${parsed.verseStart}:${parsed.verseEnd}`
|
||||
if (passageCache.has(cacheKey)) {
|
||||
res.json({ text: passageCache.get(cacheKey) })
|
||||
@@ -66,6 +89,7 @@ export function register(app) {
|
||||
}
|
||||
|
||||
const text = results.map(v => `v. ${v.verse} ${String(v.text).trim()}`).join('\n\n')
|
||||
if (passageCache.size >= PASSAGE_CACHE_MAX) passageCache.delete(passageCache.keys().next().value)
|
||||
passageCache.set(cacheKey, text)
|
||||
res.json({ text })
|
||||
} catch {
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { randomUUID } from 'node:crypto'
|
||||
import { randomUUID, timingSafeEqual } from 'node:crypto'
|
||||
import { Resend } from 'resend'
|
||||
import { requireAdminAuth } from '../auth.js'
|
||||
import { escapeHtml, splitName, buildAbsoluteUrl } from '../helpers.js'
|
||||
@@ -311,7 +311,9 @@ export function register(app) {
|
||||
|| String(req.query?.token || '').trim()
|
||||
|| (req.get('authorization') || '').replace(/^Bearer\s+/i, '').trim()
|
||||
|
||||
if (!providedToken || providedToken !== expectedToken) {
|
||||
const tokenA = Buffer.from(providedToken, 'utf8')
|
||||
const tokenB = Buffer.from(expectedToken, 'utf8')
|
||||
if (!providedToken || tokenA.length !== tokenB.length || !timingSafeEqual(tokenA, tokenB)) {
|
||||
res.status(401).json({ message: 'Unauthorized webhook.' }); return
|
||||
}
|
||||
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { randomUUID } from 'node:crypto'
|
||||
import { readFile, writeFile, unlink, mkdir } from 'node:fs/promises'
|
||||
import { readFile, unlink, mkdir } from 'node:fs/promises'
|
||||
import { writeFile } from '../atomic-write.js'
|
||||
import path from 'node:path'
|
||||
import multer from 'multer'
|
||||
import rateLimit from 'express-rate-limit'
|
||||
|
||||
@@ -0,0 +1,50 @@
|
||||
import { lookup } from 'node:dns/promises'
|
||||
import net from 'node:net'
|
||||
|
||||
function isPrivateIPv4(ip) {
|
||||
const [a, b] = ip.split('.').map(Number)
|
||||
return (
|
||||
a === 0 || a === 10 || a === 127
|
||||
|| (a === 100 && b >= 64 && b <= 127) // CGNAT / Tailscale
|
||||
|| (a === 169 && b === 254) // link-local, cloud metadata
|
||||
|| (a === 172 && b >= 16 && b <= 31)
|
||||
|| (a === 192 && b === 168)
|
||||
|| a >= 224 // multicast / reserved
|
||||
)
|
||||
}
|
||||
|
||||
export function isPrivateAddress(ip) {
|
||||
if (net.isIPv4(ip)) return isPrivateIPv4(ip)
|
||||
if (net.isIPv6(ip)) {
|
||||
const lower = ip.toLowerCase()
|
||||
if (lower === '::' || lower === '::1') return true
|
||||
if (lower.startsWith('fc') || lower.startsWith('fd')) return true // unique local
|
||||
if (/^fe[89ab]/.test(lower)) return true // link-local
|
||||
const mapped = lower.match(/^::ffff:(\d+\.\d+\.\d+\.\d+)$/)
|
||||
if (mapped) return isPrivateIPv4(mapped[1])
|
||||
return false
|
||||
}
|
||||
return true // not an IP at all: treat as unsafe
|
||||
}
|
||||
|
||||
// Throws unless `rawUrl` is an https URL whose host resolves only to public
|
||||
// addresses. Called when a webhook is registered and again before each delivery
|
||||
// (DNS can change between the two).
|
||||
export async function assertPublicHttpsUrl(rawUrl) {
|
||||
let parsed
|
||||
try {
|
||||
parsed = new URL(rawUrl)
|
||||
} catch {
|
||||
throw new Error('Malformed URL.')
|
||||
}
|
||||
if (parsed.protocol !== 'https:') throw new Error('Only https:// URLs are allowed.')
|
||||
if (parsed.username || parsed.password) throw new Error('URLs with embedded credentials are not allowed.')
|
||||
const host = parsed.hostname.replace(/^\[|\]$/g, '')
|
||||
if (host === 'localhost' || host.endsWith('.localhost') || host.endsWith('.local') || host.endsWith('.internal')) {
|
||||
throw new Error('Internal hostnames are not allowed.')
|
||||
}
|
||||
const addresses = net.isIP(host) ? [{ address: host }] : await lookup(host, { all: true })
|
||||
if (addresses.length === 0 || addresses.some(a => isPrivateAddress(a.address))) {
|
||||
throw new Error('URL resolves to a private or internal address.')
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user