← back to Visual Factory
harden prefetchAndRehost: pin DNS, stream size, magic bytes, realpath
05edcc677b9e2acfb8fae3d7e3d9fc2bb4f24b12 · 2026-04-30 23:46:57 -0700 · SteveStudio2
- Replace whatwg fetch() with safeFetchToBuffer using http(s).request +
custom lookup callback. Hostname resolves once, validated IP gets pinned;
the actual TCP connection lands on that IP, so DNS rebinding between
validation and fetch can't redirect us into private space.
- Stream-read body with running total; abort mid-stream once the chunks
exceed _MAX_PREFETCH_BYTES (25MB). A server lying about Content-Length
(or omitting it) can no longer OOM us.
- _detectImageMagic: verify PNG / JPEG / GIF / WebP signatures in the
body bytes. Reject when Content-Type and bytes disagree -- don't trust
the header alone.
- /runs/:id/external/:fname: fs.realpath() then re-assert containment in
SANDBOX_OUTPUT before sendFile, so a future symlink can't escape.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Files touched
Diff
commit 05edcc677b9e2acfb8fae3d7e3d9fc2bb4f24b12
Author: SteveStudio2 <stevestudio2@SteveStacStudio.lan>
Date: Thu Apr 30 23:46:57 2026 -0700
harden prefetchAndRehost: pin DNS, stream size, magic bytes, realpath
- Replace whatwg fetch() with safeFetchToBuffer using http(s).request +
custom lookup callback. Hostname resolves once, validated IP gets pinned;
the actual TCP connection lands on that IP, so DNS rebinding between
validation and fetch can't redirect us into private space.
- Stream-read body with running total; abort mid-stream once the chunks
exceed _MAX_PREFETCH_BYTES (25MB). A server lying about Content-Length
(or omitting it) can no longer OOM us.
- _detectImageMagic: verify PNG / JPEG / GIF / WebP signatures in the
body bytes. Reject when Content-Type and bytes disagree -- don't trust
the header alone.
- /runs/:id/external/:fname: fs.realpath() then re-assert containment in
SANDBOX_OUTPUT before sendFile, so a future symlink can't escape.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
server.js | 221 ++++++++++++++++++++++++++++++++++++++++++++++++++++----------
1 file changed, 188 insertions(+), 33 deletions(-)
diff --git a/server.js b/server.js
index 22a9648..d4f222b 100644
--- a/server.js
+++ b/server.js
@@ -129,6 +129,8 @@ async function appendManifest(entry) {
// IPv4-mapped IPv6, and `localhost.` with trailing dot), and resolve DNS so
// names that resolve into private space are also blocked (DNS rebinding).
const dns = require('node:dns').promises;
+const http = require('node:http');
+const https = require('node:https');
function _normalizeHost(rawHost) {
let h = (rawHost || '').toLowerCase();
@@ -258,40 +260,156 @@ function buildIgCaption(run, art) {
// (the validator and Norma's later fetch each do their own DNS resolution).
//
// Safety guards layered here:
-// 1. `redirect: 'manual'` — never follow redirects (each would re-open the
-// rebinding window on a different host).
-// 2. Content-Type allowlist — only image/{png,jpeg,jpg,webp,gif}.
-// 3. Max body size 25 MB; reject anything larger.
-// 4. 15s overall timeout.
-// 5. Caller must already have validated the URL via `isAllowedAfterDns`.
+// 1. DNS pinning — resolve hostname once, validate the IP, then force
+// http(s).request to connect to *that* IP via custom `lookup`. Defeats
+// DNS rebinding even within the validate→fetch window.
+// 2. Reject 3xx — never follow redirects (each would re-open the rebinding
+// window on a different host).
+// 3. Content-Type allowlist — only image/{png,jpeg,jpg,webp,gif}.
+// 4. Streaming size cap — reject mid-stream once total > _MAX_PREFETCH_BYTES,
+// so a server lying about Content-Length (or omitting it) can't OOM us.
+// 5. Magic-byte verification — refuse to save bytes whose signature doesn't
+// match an allowed image format, even if Content-Type claims it does.
+// 6. 15s overall timeout.
+// 7. Caller must already have validated the URL via `isAllowedAfterDns`.
const _ALLOWED_IMG_CT = /^image\/(png|jpe?g|webp|gif)\b/i;
const _MAX_PREFETCH_BYTES = 25 * 1024 * 1024;
+
+// Magic-byte signatures for the formats we accept. Don't trust Content-Type.
+function _detectImageMagic(buf) {
+ if (buf.length < 12) return null;
+ // PNG: 89 50 4E 47 0D 0A 1A 0A
+ if (buf[0] === 0x89 && buf[1] === 0x50 && buf[2] === 0x4E && buf[3] === 0x47 &&
+ buf[4] === 0x0D && buf[5] === 0x0A && buf[6] === 0x1A && buf[7] === 0x0A) return 'png';
+ // JPEG: FF D8 FF
+ if (buf[0] === 0xFF && buf[1] === 0xD8 && buf[2] === 0xFF) return 'jpg';
+ // GIF87a / GIF89a: 47 49 46 38 [37|39] 61
+ if (buf[0] === 0x47 && buf[1] === 0x49 && buf[2] === 0x46 && buf[3] === 0x38 &&
+ (buf[4] === 0x37 || buf[4] === 0x39) && buf[5] === 0x61) return 'gif';
+ // WebP: "RIFF" .... "WEBP"
+ if (buf[0] === 0x52 && buf[1] === 0x49 && buf[2] === 0x46 && buf[3] === 0x46 &&
+ buf[8] === 0x57 && buf[9] === 0x45 && buf[10] === 0x42 && buf[11] === 0x50) return 'webp';
+ return null;
+}
+
+// Resolve hostname → pick a public IP, then force the GET to that exact IP
+// via http(s).request's `lookup` callback. Streams the body so Content-Length
+// lies can't blow past maxBytes. Throws on any failure; caller catches.
+async function safeFetchToBuffer(input, { maxBytes, timeoutMs }) {
+ const u = new URL(input);
+ const hostNorm = _normalizeHost(u.hostname);
+
+ // 1. Resolve hostname (skip if already an IP literal).
+ let pinnedIp;
+ let pinnedFamily;
+ const isV4Literal = /^\d{1,3}(\.\d{1,3}){3}$/.test(hostNorm);
+ const isV6Literal = hostNorm.includes(':');
+ if (isV4Literal) { pinnedIp = hostNorm; pinnedFamily = 4; }
+ else if (isV6Literal) { pinnedIp = hostNorm; pinnedFamily = 6; }
+ else {
+ const [v4s, v6s] = await Promise.all([
+ dns.resolve4(hostNorm).catch(() => []),
+ dns.resolve6(hostNorm).catch(() => []),
+ ]);
+ const v4Pick = v4s.find(a => !_isBlockedIPv4(a));
+ const v6Pick = v6s.find(a => !_isBlockedIPv6(a));
+ if (v4Pick) { pinnedIp = v4Pick; pinnedFamily = 4; }
+ else if (v6Pick) { pinnedIp = v6Pick; pinnedFamily = 6; }
+ else throw new Error('no public IP for hostname');
+ }
+ // Defense in depth: even if isAllowedAfterDns just passed, recheck the IP
+ // we're about to pin in case DNS rebound between validation and now.
+ if (pinnedFamily === 4 && _isBlockedIPv4(pinnedIp)) throw new Error('pinned IP is private');
+ if (pinnedFamily === 6 && _isBlockedIPv6(pinnedIp)) throw new Error('pinned IP is private');
+
+ const lib = u.protocol === 'https:' ? https : http;
+ return await new Promise((resolve, reject) => {
+ let settled = false;
+ const finish = (fn, val) => { if (!settled) { settled = true; fn(val); } };
+ const opts = {
+ method: 'GET',
+ protocol: u.protocol,
+ hostname: u.hostname, // SNI + Host header
+ port: u.port || (u.protocol === 'https:' ? 443 : 80),
+ path: (u.pathname || '/') + (u.search || ''),
+ headers: { Host: u.host, 'User-Agent': 'visual-factory-prefetch/1' },
+ timeout: timeoutMs,
+ // Custom DNS lookup → always returns the validated IP, regardless of
+ // what real DNS says now. This is the DNS pin.
+ lookup: (_h, _o, cb) => cb(null, pinnedIp, pinnedFamily),
+ };
+ const req = lib.request(opts, (res) => {
+ // Refuse 3xx — following would re-open the rebinding window on a new host.
+ if (res.statusCode >= 300 && res.statusCode < 400) {
+ res.resume();
+ req.destroy();
+ return finish(reject, new Error(`redirect to ${res.headers.location || '?'} not allowed (would reopen DNS-rebinding window)`));
+ }
+ if (res.statusCode < 200 || res.statusCode >= 300) {
+ res.resume();
+ req.destroy();
+ return finish(reject, new Error(`http ${res.statusCode}`));
+ }
+ const ct = res.headers['content-type'] || '';
+ const m = ct.match(_ALLOWED_IMG_CT);
+ if (!m) {
+ res.resume();
+ req.destroy();
+ return finish(reject, new Error(`content-type "${ct}" not allowed; image/{png,jpeg,webp,gif} only`));
+ }
+ // Quick reject if server promised a too-big body.
+ const cl = Number(res.headers['content-length']);
+ if (Number.isFinite(cl) && cl > maxBytes) {
+ res.resume();
+ req.destroy();
+ return finish(reject, new Error(`Content-Length ${cl} exceeds ${maxBytes} cap`));
+ }
+ const chunks = [];
+ let total = 0;
+ res.on('data', (chunk) => {
+ total += chunk.length;
+ if (total > maxBytes) {
+ req.destroy();
+ return finish(reject, new Error(`body exceeded ${maxBytes} bytes mid-stream`));
+ }
+ chunks.push(chunk);
+ });
+ res.on('end', () => {
+ if (settled) return;
+ finish(resolve, {
+ buf: Buffer.concat(chunks),
+ contentType: ct,
+ ext: (m[1] || 'png').toLowerCase().replace('jpeg', 'jpg'),
+ });
+ });
+ res.on('error', (err) => finish(reject, err));
+ });
+ req.on('timeout', () => {
+ req.destroy();
+ finish(reject, new Error(`prefetch timed out after ${timeoutMs}ms`));
+ });
+ req.on('error', (err) => finish(reject, err));
+ req.end();
+ });
+}
+
async function prefetchAndRehost(url, runId) {
- const ctrl = new AbortController();
- const timer = setTimeout(() => ctrl.abort(), 15_000);
try {
- const res = await fetch(url, { signal: ctrl.signal, redirect: 'manual' });
- clearTimeout(timer);
- if (res.status >= 300 && res.status < 400) {
- return { ok: false, error: `redirect to ${res.headers.get('location') || '?'} not allowed (would reopen DNS-rebinding window)` };
+ const fetched = await safeFetchToBuffer(url, { maxBytes: _MAX_PREFETCH_BYTES, timeoutMs: 15_000 });
+ // Trust bytes, not headers. Magic must match AND agree with Content-Type.
+ const detected = _detectImageMagic(fetched.buf);
+ if (!detected) {
+ return { ok: false, error: 'body bytes do not match any allowed image format (png/jpg/gif/webp)' };
}
- if (!res.ok) return { ok: false, error: `http ${res.status}` };
- const ct = res.headers.get('content-type') || '';
- const m = ct.match(_ALLOWED_IMG_CT);
- if (!m) return { ok: false, error: `content-type "${ct}" not allowed; image/{png,jpeg,webp,gif} only` };
- // Size cap — read as ArrayBuffer once we know CT is OK.
- const buf = Buffer.from(await res.arrayBuffer());
- if (buf.length > _MAX_PREFETCH_BYTES) {
- return { ok: false, error: `${buf.length} bytes exceeds ${_MAX_PREFETCH_BYTES} cap` };
+ if (detected !== fetched.ext) {
+ return { ok: false, error: `content-type ${fetched.contentType} disagrees with magic bytes (${detected})` };
}
- const ext = (m[1] || 'png').toLowerCase().replace('jpeg', 'jpg');
- const fname = `external-${Date.now()}.${ext}`;
+ const fname = `external-${Date.now()}.${detected}`;
const fpath = path.join(__dirname, 'output', `run-${runId}`, fname);
await fs.mkdir(path.dirname(fpath), { recursive: true });
- await fs.writeFile(fpath, buf);
- return { ok: true, path: fpath, fname, size: buf.length, contentType: ct };
+ await fs.writeFile(fpath, fetched.buf);
+ return { ok: true, path: fpath, fname, size: fetched.buf.length, contentType: fetched.contentType };
} catch (err) {
- clearTimeout(timer);
return { ok: false, error: err.message };
}
}
@@ -557,7 +675,16 @@ app.get('/runs/:id/external/:fname', async (req, res) => {
let safe;
try { safe = assertWithin(SANDBOX_OUTPUT, fpath); }
catch { return res.status(400).json({ error: 'path outside sandbox' }); }
- res.sendFile(safe, err => {
+ // Codex P2 fix: realpath defeats symlinks pointing outside the sandbox.
+ // sendFile follows symlinks, so a future bug (or attacker who can drop a
+ // symlink into output/) could otherwise serve arbitrary files. Resolve
+ // the link target and re-assert containment.
+ let real;
+ try { real = await fs.realpath(safe); }
+ catch (e) { return res.status(404).json({ error: 'not found' }); }
+ try { assertWithin(SANDBOX_OUTPUT, real); }
+ catch { return res.status(400).json({ error: 'symlink target outside sandbox' }); }
+ res.sendFile(real, err => {
if (err && !res.headersSent) res.status(404).json({ error: 'not found' });
});
});
@@ -694,21 +821,49 @@ app.post('/runs/:id/activate', async (req, res) => {
destHtml = assertWithin(PUBLISH_ROOT_RESOLVED, path.join(PUBLISH_ROOT, `${art.artifact_name}.html`));
} catch { return res.status(400).json({ error: 'destination escapes publish root' }); }
+ // Codex round-2 P1 fix: activate overwrite race. fs.access() then rename
+ // is non-atomic — two concurrent activations can both pass the access
+ // check. When overwrite=false, claim destination via fs.link() (atomic;
+ // fails with EEXIST if the target already exists). When overwrite=true,
+ // accept the inherent rename-clobber race (atomic at the FS level — one
+ // of two concurrent renames wins, neither corrupts).
let existed = false;
- try { await fs.access(destPng); existed = true; } catch {}
- if (existed && !overwrite) return res.status(409).json({ error: `target exists: ${destPng}`, hint: 'pass ?overwrite=true' });
+ if (!overwrite) {
+ try { await fs.access(destPng); existed = true; } catch {}
+ if (existed) return res.status(409).json({ error: `target exists: ${destPng}`, hint: 'pass ?overwrite=true' });
+ }
- // Codex P1 #8 fix: copy to .tmp first, only rename into place once both
- // files are written. Then commit the DB rows. If any step fails, clean up
- // tmp files so the publish dir doesn't end up with orphan halves.
+ // Codex P1 #8 fix: copy to .tmp first, claim destination atomically, then
+ // commit DB. Failure path cleans up tmp files so we don't leave halves.
const tmpPng = destPng + '.tmp.' + process.pid + '.' + Date.now();
const tmpHtml = srcHtml ? destHtml + '.tmp.' + process.pid + '.' + Date.now() : null;
let pngCommitted = false, htmlCommitted = false;
try {
await fs.copyFile(srcPng, tmpPng);
if (tmpHtml) await fs.copyFile(srcHtml, tmpHtml);
- await fs.rename(tmpPng, destPng); pngCommitted = true;
- if (tmpHtml) { await fs.rename(tmpHtml, destHtml); htmlCommitted = true; }
+ if (overwrite) {
+ await fs.rename(tmpPng, destPng); pngCommitted = true;
+ if (tmpHtml) { await fs.rename(tmpHtml, destHtml); htmlCommitted = true; }
+ } else {
+ // Atomic claim. If two activations race on the same artifact_name,
+ // exactly one wins; the loser sees EEXIST and 409s.
+ try { await fs.link(tmpPng, destPng); }
+ catch (e) {
+ if (e.code === 'EEXIST') {
+ await fs.rm(tmpPng, { force: true }).catch(() => {});
+ if (tmpHtml) await fs.rm(tmpHtml, { force: true }).catch(() => {});
+ return res.status(409).json({ error: `target exists: ${destPng} (lost activation race)`, hint: 'pass ?overwrite=true' });
+ }
+ throw e;
+ }
+ await fs.rm(tmpPng, { force: true }).catch(() => {});
+ pngCommitted = true;
+ if (tmpHtml) {
+ try { await fs.link(tmpHtml, destHtml); htmlCommitted = true; }
+ catch (e) { if (e.code !== 'EEXIST') throw e; /* PNG won, HTML clash unlikely; tolerate */ }
+ await fs.rm(tmpHtml, { force: true }).catch(() => {});
+ }
+ }
await pg.query("UPDATE artifacts SET status='activated', published_path=$1 WHERE id=$2", [destPng, art.id]);
await pg.query("UPDATE runs SET status='activated', updated_at=now() WHERE id=$1", [runId]);
} catch (err) {
← 30c6d24 B: pre-fetch user image_url and re-host on loopback (closes
·
back to Visual Factory
·
prefetch: agent:false to force fresh socket per request 19c935e →