mirror of
https://github.com/affaan-m/ECC.git
synced 2026-09-10 11:47:54 +02:00
fix(plan-canvas): harden startup and PDF admission
This commit is contained in:
@@ -411,6 +411,14 @@ function createPlanCanvasServer({
|
||||
if (pdfMatch && (req.method === 'GET' || req.method === 'POST')) {
|
||||
const session = store.get(pdfMatch[1]);
|
||||
if (!session) return sendJson(res, 404, { error: 'unknown session' });
|
||||
let requestedSnapshot = null;
|
||||
if (req.method === 'POST') {
|
||||
const body = await readJsonBody(req, MAX_PDF_SNAPSHOT_BYTES);
|
||||
if (typeof body.html !== 'string' || !body.html.trim()) {
|
||||
return sendJson(res, 400, { error: 'html snapshot is required' });
|
||||
}
|
||||
requestedSnapshot = { key: session.key, html: body.html };
|
||||
}
|
||||
if (pdfExportActive) {
|
||||
res.setHeader('retry-after', '1');
|
||||
return sendJson(res, 429, {
|
||||
@@ -420,13 +428,7 @@ function createPlanCanvasServer({
|
||||
}
|
||||
pdfExportActive = true;
|
||||
try {
|
||||
if (req.method === 'POST') {
|
||||
const body = await readJsonBody(req, MAX_PDF_SNAPSHOT_BYTES);
|
||||
if (typeof body.html !== 'string' || !body.html.trim()) {
|
||||
return sendJson(res, 400, { error: 'html snapshot is required' });
|
||||
}
|
||||
pdfSnapshot = { key: session.key, html: body.html };
|
||||
}
|
||||
pdfSnapshot = requestedSnapshot;
|
||||
const pdf = await pdfExporter({
|
||||
url: `http://${req.headers.host}/artifact/${session.key}/?pdf=1`,
|
||||
artifactFile: session.file
|
||||
|
||||
+21
-6
@@ -39,6 +39,7 @@ const {
|
||||
} = require('./lib/plan-canvas/server');
|
||||
|
||||
const VERSION = require('../package.json').version;
|
||||
const DEFAULT_START_LOCK_LEASE_MS = 5 * 1000;
|
||||
|
||||
const SAFE_REQUEST_PATHS = new Set([
|
||||
'/',
|
||||
@@ -200,7 +201,7 @@ function readServerStartTicket(file) {
|
||||
}
|
||||
}
|
||||
|
||||
function listServerStartTickets(lockDir, port, ownToken, staleAfterMs = 60 * 1000) {
|
||||
function listServerStartTickets(lockDir, port, ownToken, staleAfterMs = DEFAULT_START_LOCK_LEASE_MS) {
|
||||
const prefix = `ecc-plan-canvas-${validatePort(port)}-`;
|
||||
const entries = [];
|
||||
for (const name of fs.readdirSync(lockDir)) {
|
||||
@@ -219,10 +220,13 @@ function listServerStartTickets(lockDir, port, ownToken, staleAfterMs = 60 * 100
|
||||
}
|
||||
continue;
|
||||
}
|
||||
const stale = value.token !== ownToken && !processIsAlive(value.pid);
|
||||
const stale = value.token !== ownToken && (
|
||||
!processIsAlive(value.pid) || Date.now() - value.mtimeMs > staleAfterMs
|
||||
);
|
||||
if (stale) {
|
||||
// Ticket names contain a never-reused random owner token, so removing a
|
||||
// dead owner's exact path cannot delete a later caller's live ticket.
|
||||
// A bounded, renewable lease prevents PID reuse from making an exited
|
||||
// owner's ticket look live forever. Ticket names contain a never-reused
|
||||
// owner token, so this cannot delete a later caller's ticket path.
|
||||
try { fs.rmSync(file, { force: true }); } catch { /* already removed */ }
|
||||
continue;
|
||||
}
|
||||
@@ -233,6 +237,7 @@ function listServerStartTickets(lockDir, port, ownToken, staleAfterMs = 60 * 100
|
||||
|
||||
async function withServerStartLock(port, task, {
|
||||
timeoutMs = 15 * 1000,
|
||||
leaseMs = DEFAULT_START_LOCK_LEASE_MS,
|
||||
lockDir = path.join(os.homedir(), '.claude', 'plan-canvas', 'locks')
|
||||
} = {}) {
|
||||
const lockPort = validatePort(port);
|
||||
@@ -241,18 +246,27 @@ async function withServerStartLock(port, task, {
|
||||
const choosingFile = path.join(lockDir, `${prefix}.choosing`);
|
||||
const ticketFile = path.join(lockDir, `${prefix}.ticket`);
|
||||
const startedAt = Date.now();
|
||||
const lockLeaseMs = Number.isFinite(leaseMs) && leaseMs > 0
|
||||
? leaseMs
|
||||
: DEFAULT_START_LOCK_LEASE_MS;
|
||||
let leaseTimer = null;
|
||||
fs.mkdirSync(lockDir, { recursive: true, mode: 0o700 });
|
||||
fs.writeFileSync(choosingFile, JSON.stringify({ pid: process.pid, token }), { flag: 'wx', mode: 0o600 });
|
||||
|
||||
try {
|
||||
const existing = listServerStartTickets(lockDir, lockPort, token)
|
||||
const existing = listServerStartTickets(lockDir, lockPort, token, lockLeaseMs)
|
||||
.filter(entry => !entry.choosing && Number.isInteger(entry.number));
|
||||
const number = existing.reduce((maximum, entry) => Math.max(maximum, entry.number), 0) + 1;
|
||||
fs.writeFileSync(ticketFile, JSON.stringify({ pid: process.pid, token, number }), { flag: 'wx', mode: 0o600 });
|
||||
fs.rmSync(choosingFile, { force: true });
|
||||
leaseTimer = setInterval(() => {
|
||||
const now = new Date();
|
||||
try { fs.utimesSync(ticketFile, now, now); } catch { /* cleanup or lease loss */ }
|
||||
}, Math.max(25, Math.floor(lockLeaseMs / 3)));
|
||||
if (leaseTimer.unref) leaseTimer.unref();
|
||||
|
||||
while (true) {
|
||||
const entries = listServerStartTickets(lockDir, lockPort, token);
|
||||
const entries = listServerStartTickets(lockDir, lockPort, token, lockLeaseMs);
|
||||
const anotherOwnerIsChoosing = entries.some(entry => entry.choosing && entry.token !== token);
|
||||
const tickets = entries
|
||||
.filter(entry => !entry.choosing && Number.isInteger(entry.number))
|
||||
@@ -265,6 +279,7 @@ async function withServerStartLock(port, task, {
|
||||
}
|
||||
return await task();
|
||||
} finally {
|
||||
clearInterval(leaseTimer);
|
||||
fs.rmSync(choosingFile, { force: true });
|
||||
fs.rmSync(ticketFile, { force: true });
|
||||
}
|
||||
|
||||
@@ -159,6 +159,28 @@ async function main() {
|
||||
assert.strictEqual(maximumActive, 1);
|
||||
});
|
||||
|
||||
await test('port-scoped startup lock renews its lease while the owner is active', async () => {
|
||||
const lockPort = port + 6;
|
||||
const lockDir = path.join(tmp, 'startup-locks');
|
||||
let releaseFirst;
|
||||
let markFirstEntered;
|
||||
let secondEntered = false;
|
||||
const firstEntered = new Promise(resolve => { markFirstEntered = resolve; });
|
||||
const first = withServerStartLock(lockPort, async () => {
|
||||
markFirstEntered();
|
||||
await new Promise(resolve => { releaseFirst = resolve; });
|
||||
}, { lockDir, timeoutMs: 1000, leaseMs: 100 });
|
||||
await firstEntered;
|
||||
const second = withServerStartLock(lockPort, async () => {
|
||||
secondEntered = true;
|
||||
}, { lockDir, timeoutMs: 1000, leaseMs: 100 });
|
||||
await new Promise(resolve => setTimeout(resolve, 250));
|
||||
assert.strictEqual(secondEntered, false);
|
||||
releaseFirst();
|
||||
await Promise.all([first, second]);
|
||||
assert.strictEqual(secondEntered, true);
|
||||
});
|
||||
|
||||
await test('port-scoped startup lock recovers dead tickets and failed owners', async () => {
|
||||
const lockPort = port + 3;
|
||||
const lockDir = path.join(tmp, 'startup-locks');
|
||||
@@ -177,6 +199,26 @@ async function main() {
|
||||
assert.ok(!fs.readdirSync(lockDir).some(name => name.startsWith(`ecc-plan-canvas-${lockPort}-`)));
|
||||
});
|
||||
|
||||
await test('port-scoped startup lock expires a stale ticket after PID reuse', async () => {
|
||||
const lockPort = port + 5;
|
||||
const lockDir = path.join(tmp, 'startup-locks');
|
||||
fs.mkdirSync(lockDir, { recursive: true });
|
||||
const reusedPidTicket = path.join(lockDir, `ecc-plan-canvas-${lockPort}-reused-pid.ticket`);
|
||||
fs.writeFileSync(
|
||||
reusedPidTicket,
|
||||
JSON.stringify({ pid: process.pid, token: 'reused-pid', number: 1 })
|
||||
);
|
||||
assert.strictEqual(
|
||||
await withServerStartLock(lockPort, async () => 'recovered', {
|
||||
lockDir,
|
||||
timeoutMs: 1000,
|
||||
leaseMs: 100
|
||||
}),
|
||||
'recovered'
|
||||
);
|
||||
assert.ok(!fs.existsSync(reusedPidTicket));
|
||||
});
|
||||
|
||||
await test('unrelated UDP traffic on the Canvas port does not block startup', async () => {
|
||||
const servicePort = port + 4;
|
||||
const unrelatedSocket = dgram.createSocket('udp4');
|
||||
|
||||
@@ -286,6 +286,27 @@ async function main() {
|
||||
assert.strictEqual((await first).statusCode, 200);
|
||||
})) passed++; else failed++;
|
||||
|
||||
if (await test('an incomplete PDF snapshot body does not consume renderer admission', async () => {
|
||||
const stalled = http.request({
|
||||
host: '127.0.0.1',
|
||||
port,
|
||||
method: 'POST',
|
||||
path: `/api/session/${key}/pdf`,
|
||||
agent: false,
|
||||
headers: { 'content-type': 'application/json', 'content-length': 1024 }
|
||||
});
|
||||
stalled.on('error', () => {});
|
||||
stalled.write('{"html":"partial');
|
||||
try {
|
||||
await new Promise(resolve => setTimeout(resolve, 50));
|
||||
const competing = await request(port, 'GET', `/api/session/${key}/pdf`);
|
||||
assert.strictEqual(competing.statusCode, 200);
|
||||
assert.strictEqual(competing.headers['content-type'], 'application/pdf');
|
||||
} finally {
|
||||
stalled.destroy();
|
||||
}
|
||||
})) passed++; else failed++;
|
||||
|
||||
if (await test('PDF failures log diagnostics without disclosing local paths', async () => {
|
||||
try {
|
||||
const rendererError = new Error('Chromium failed at /Users/private/browser-profile');
|
||||
|
||||
Reference in New Issue
Block a user