← back to Visual Factory
apply codex p2/p2/p3 abort-handling fixes
287ac34bb500a13b6eda3a6e9967d15017644fa8 · 2026-04-30 23:24:27 -0700 · temp
Files touched
M server.jsM src/llm.jsM src/pipeline.jsM src/stages/compose.jsM src/stages/intake.jsM src/stages/render.js
Diff
commit 287ac34bb500a13b6eda3a6e9967d15017644fa8
Author: temp <temp@local>
Date: Thu Apr 30 23:24:27 2026 -0700
apply codex p2/p2/p3 abort-handling fixes
---
server.js | 100 ++++++++++++++++++++++++++++++++++++++------------
src/llm.js | 5 +++
src/pipeline.js | 7 ++++
src/stages/compose.js | 5 ++-
src/stages/intake.js | 5 ++-
src/stages/render.js | 7 ++++
6 files changed, 101 insertions(+), 28 deletions(-)
diff --git a/server.js b/server.js
index d5066fd..029c3ab 100644
--- a/server.js
+++ b/server.js
@@ -123,28 +123,77 @@ async function appendManifest(entry) {
});
}
-// Codex P1 #7 fix: validate any URL we hand off to Norma to fetch. Reject
-// non-HTTP(S), private/loopback IPs, and metadata-service ranges so a poisoned
-// `image_url` can't pivot Norma into reading internal resources.
+// Codex P1 #7 fix (round 2): validate any URL we hand off to Norma to fetch.
+// Reject non-HTTP(S), userinfo, private/loopback IPs (incl. bracketed IPv6,
+// 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;
+
+function _normalizeHost(rawHost) {
+ let h = (rawHost || '').toLowerCase();
+ if (h.startsWith('[') && h.endsWith(']')) h = h.slice(1, -1); // strip IPv6 brackets
+ if (h.endsWith('.')) h = h.slice(0, -1); // strip trailing dot
+ return h;
+}
+function _isBlockedIPv4(addr) {
+ const v4 = addr.match(/^(\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})$/);
+ if (!v4) return false;
+ const [, a, b] = v4.map(Number);
+ if (a === 127 || a === 10 || a === 0) return true;
+ if (a === 169 && b === 254) return true; // link-local + AWS metadata
+ if (a === 172 && b >= 16 && b <= 31) return true;
+ if (a === 192 && b === 168) return true;
+ if (a === 100 && b >= 64 && b <= 127) return true; // CGNAT
+ return false;
+}
+function _isBlockedIPv6(addr) {
+ const a = addr.toLowerCase();
+ if (a === '::1' || a === '::') return true;
+ if (a.startsWith('fe80:') || a.startsWith('fe80::')) return true;
+ // ULA: fc00::/7
+ if (/^f[cd][0-9a-f]{2}:/.test(a)) return true;
+ // IPv4-mapped (::ffff:a.b.c.d or ::ffff:7f00:1)
+ const mappedDot = a.match(/^::ffff:(\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3})$/);
+ if (mappedDot && _isBlockedIPv4(mappedDot[1])) return true;
+ const mappedHex = a.match(/^::ffff:([0-9a-f]{1,4}):([0-9a-f]{1,4})$/);
+ if (mappedHex) {
+ const hi = parseInt(mappedHex[1], 16), lo = parseInt(mappedHex[2], 16);
+ const v4 = `${(hi >> 8) & 0xff}.${hi & 0xff}.${(lo >> 8) & 0xff}.${lo & 0xff}`;
+ if (_isBlockedIPv4(v4)) return true;
+ }
+ return false;
+}
function isAllowedExternalUrl(input) {
let u;
try { u = new URL(input); } catch { return false; }
if (!/^https?:$/.test(u.protocol)) return false;
- const host = u.hostname.toLowerCase();
- if (host === 'localhost' || host === '0.0.0.0') return false;
- // IPv4 literal — block loopback + RFC1918 + link-local + AWS/GCP metadata.
- const v4 = host.match(/^(\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})$/);
- if (v4) {
- const [, a, b] = v4.map(Number);
- if (a === 127 || a === 10 || a === 0) return false;
- if (a === 169 && b === 254) return false; // 169.254.169.254 metadata
- if (a === 172 && b >= 16 && b <= 31) return false;
- if (a === 192 && b === 168) return false;
- }
- // IPv6 literal — block loopback / link-local. Browsers wrap in [].
- if (host === '::1' || host.startsWith('fe80:') || host.startsWith('fc') || host.startsWith('fd')) return false;
+ // userinfo is a confusion vector — `http://user@127.0.0.1/` etc. — disallow.
+ if (u.username || u.password) return false;
+ const host = _normalizeHost(u.hostname);
+ if (!host) return false;
+ if (host === 'localhost') return false;
+ if (_isBlockedIPv4(host)) return false;
+ if (_isBlockedIPv6(host)) return false;
return true;
}
+// Async second-line check: resolve hostname and reject if any A/AAAA points
+// into private space. Defends against DNS rebinding + private-DNS names that
+// the literal-only check above can't see.
+async function isAllowedAfterDns(input) {
+ if (!isAllowedExternalUrl(input)) return false;
+ const u = new URL(input);
+ const host = _normalizeHost(u.hostname);
+ if (/^\d{1,3}(\.\d{1,3}){3}$/.test(host) || host.includes(':')) return true; // literal — done
+ try {
+ const [v4s, v6s] = await Promise.all([
+ dns.resolve4(host).catch(() => []),
+ dns.resolve6(host).catch(() => []),
+ ]);
+ const all = [...v4s, ...v6s];
+ if (!all.length) return false;
+ return all.every(a => !_isBlockedIPv4(a) && !_isBlockedIPv6(a));
+ } catch { return false; }
+}
// Reveal the published file in Finder. macOS-only; non-fatal if `open` is missing.
function revealInFinder(filePath) {
@@ -600,10 +649,7 @@ app.post('/runs/:id/activate', async (req, res) => {
html_committed: htmlCommitted,
});
}
- await pg.query(
- "INSERT INTO events (run_id, stage, stage_name, level, message, payload) VALUES ($1,$2,$3,$4,$5,$6)",
- [runId, 8, 'activate', 'info', existed ? 'overwrote target' : 'wrote target', JSON.stringify({ srcPng, destPng })]
- );
+ // Codex round-2 P2: was firing this INSERT twice (dup audit row).
await pg.query(
"INSERT INTO events (run_id, stage, stage_name, level, message, payload) VALUES ($1,$2,$3,$4,$5,$6)",
[runId, 8, 'activate', 'info', existed ? 'overwrote target' : 'wrote target', JSON.stringify({ srcPng, destPng })]
@@ -690,7 +736,13 @@ async function reapOrphans() {
} catch (err) { console.error('[visual-factory] reapOrphans failed:', err.message); }
}
-app.listen(PORT, '127.0.0.1', async () => {
- console.log(`[visual-factory] orchestrator listening on http://127.0.0.1:${PORT} (db=${PG_DATABASE})`);
- await reapOrphans();
-});
+// Codex round-2 P1: reap before opening the listener, so a request that lands
+// in the brief boot window can't get insta-orphaned by a reaper that hasn't
+// run yet. Top-level await isn't available in a CommonJS module, so we wrap
+// in an async IIFE.
+(async () => {
+ try { await reapOrphans(); } catch (e) { console.error('[visual-factory] boot reaper threw:', e.message); }
+ app.listen(PORT, '127.0.0.1', () => {
+ console.log(`[visual-factory] orchestrator listening on http://127.0.0.1:${PORT} (db=${PG_DATABASE})`);
+ });
+})();
diff --git a/src/llm.js b/src/llm.js
index 548053e..81338d4 100644
--- a/src/llm.js
+++ b/src/llm.js
@@ -71,7 +71,12 @@ async function ollama({ model, system, prompt, format = null, temperature = 0.2,
if (attempt === delays.length) break;
}
// Honor abort during back-off too — otherwise a cancelled stage waits the full 19s.
+ // If the signal aborted just before this Promise body runs, addEventListener
+ // attaches to an already-aborted signal and never fires. Check first.
await new Promise((resolve, reject) => {
+ if (signal?.aborted) {
+ return reject(new Error('ollama call aborted (during retry back-off)'));
+ }
const t = setTimeout(resolve, delays[attempt]);
if (signal) {
const onAbort = () => { clearTimeout(t); reject(new Error('ollama call aborted (during retry back-off)')); };
diff --git a/src/pipeline.js b/src/pipeline.js
index 6daa27b..6070704 100644
--- a/src/pipeline.js
+++ b/src/pipeline.js
@@ -129,6 +129,13 @@ async function runPipeline(pg, run) {
await logEvent(pg, runId, 5, 'critic', review.approved ? 'info' : 'warn',
`score=${review.score} approved=${review.approved}`, { review });
} catch (err) {
+ // Abort/timeout MUST propagate — the outer crash handler marks the run
+ // failed instead of cataloging it as `completed_unapproved` (which looks
+ // like a quality reject, not a cancelled stage).
+ if (err?.name === 'AbortError' || /aborted|timed out/i.test(err?.message || '')) {
+ await logEvent(pg, runId, 5, 'critic', 'error', 'critic stage aborted/timed out', { error: err.message });
+ throw err;
+ }
review = { approved: false, score: 0, issues: [`critic call failed: ${err.message}`], fix_instructions: [] };
await logEvent(pg, runId, 5, 'critic', 'error', 'critic call failed', { error: err.message });
}
diff --git a/src/stages/compose.js b/src/stages/compose.js
index 9197acb..5367b20 100644
--- a/src/stages/compose.js
+++ b/src/stages/compose.js
@@ -22,7 +22,7 @@ REQUIREMENTS:
- No lorem ipsum — use the block.text values verbatim.
- Polished, intentional layout. NOT a generic landing-page hero.`;
-async function runCompose({ runId, spec }) {
+async function runCompose({ runId, spec, signal = null }) {
const userPrompt = `Spec:
${JSON.stringify(spec, null, 2)}
@@ -35,7 +35,8 @@ Write the HTML now.`;
model: process.env.COMPOSE_MODEL || 'qwen3:14b',
system: SYSTEM,
prompt: userPrompt,
- temperature: 0.4
+ temperature: 0.4,
+ signal
})).trim();
const runDir = path.join(SANDBOX_ROOT, `run-${runId}`);
diff --git a/src/stages/intake.js b/src/stages/intake.js
index 81059fd..79da011 100644
--- a/src/stages/intake.js
+++ b/src/stages/intake.js
@@ -18,13 +18,14 @@ No prose, no markdown, no code fences.`;
const KEBAB = /^[a-z0-9-]{2,60}$/;
-async function runIntake({ brief }) {
+async function runIntake({ brief, signal = null }) {
const spec = await ollama({
model: process.env.COMPOSE_MODEL || 'qwen3:14b',
system: SYSTEM,
prompt: brief,
format: 'json',
- temperature: 0.2
+ temperature: 0.2,
+ signal
});
const errors = [];
diff --git a/src/stages/render.js b/src/stages/render.js
index 688dcc1..b13a70b 100644
--- a/src/stages/render.js
+++ b/src/stages/render.js
@@ -8,6 +8,13 @@ async function runRender({ htmlPath, width, height, signal = null }) {
if (signal?.aborted) throw new Error('render aborted before launch');
const pngPath = htmlPath.replace(/\.html$/, '.png');
const browser = await chromium.launch({ headless: true });
+ // If abort fired during launch, the addEventListener below registers on an
+ // already-aborted signal and never fires — render would proceed in the
+ // background after withTimeout has rejected. Check now and bail clean.
+ if (signal?.aborted) {
+ await browser.close().catch(() => {});
+ throw new Error('render aborted during chromium launch');
+ }
// Wire external abort to browser.close() so a stage timeout actually kills
// the headless Chromium instead of leaving a zombie process eating GPU.
let onAbort = null;
← 7c502d8 snapshot for codex review
·
back to Visual Factory
·
use DNS-aware SSRF gate on /runs/:id/post-ig 5de52a6 →