From acf87059b54d671f78610d1b9724e5d649cc5324 Mon Sep 17 00:00:00 2001 From: Daniel Date: Fri, 4 Sep 2026 18:32:45 +0800 Subject: [PATCH 1/2] fix: runAct treats nonzero-exit JSON as a failed act, like runDecide and runProve Bug review of EauDoon/agent-action-stack found: - MEDIUM: runAct (bin/aas.mjs) short-circuits to childProcessError when result.status !== 0, before parseStageJson is reached. Any JSON crctl wrote to stdout is dropped. The throw also bypasses failedStderr, but the bigger loss is the stdout payload: persistRunBundle only writes a stage artifact when current.raw !== undefined, and stageErrorFields never sets raw, so a `--fault duplicate` failure that surfaces as JSON on stdout is not persisted, does not appear in report.stages.act, and printHuman shows only act_code / act_reason / act_stderr. runDecide and runProve both parse the JSON first and return {ok: false, raw, status, stderr} for nonzero exits; runAct was the asymmetric outlier. Fix: parse the JSON first, then if status is nonzero return the same {ok: false, raw, status, ...failedStderr(result)} shape that runDecide and runProve use. Throw childProcessError only when the spawn itself failed (result.error) or when the child exited nonzero with no parseable JSON. Verified: node --test test/stack.test.mjs runs 40 tests, all pass. --- bin/aas.mjs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/bin/aas.mjs b/bin/aas.mjs index d9eb79b..d6c1230 100644 --- a/bin/aas.mjs +++ b/bin/aas.mjs @@ -520,8 +520,15 @@ export function runAct( const result = runner(process.execPath, [crctl, ...args], { cwd: join(depsDir, "consequence-rail"), }); - if (result.error || result.status !== 0) throw childProcessError("act", result); + if (result.error) throw childProcessError("act", result); const payload = parseStageJson("act", result); + if (result.status !== 0) { + // A nonzero exit with parseable JSON is an unsuccessful act (the CLI + // surfaces structured errors as JSON on stdout), not a child-process + // error. Mirror runProve so persistRunBundle and printHuman see the + // structured failure and the GUI can render the stage artifact. + return { ok: false, raw: payload, status: result.status, ...failedStderr(result) }; + } if (!payload || typeof payload !== "object" || Array.isArray(payload) || ![null, "settled", "compensated", "disputed"].includes(payload.outcome)) { throw attachChildDiagnostics(new Error("act did not return a valid outcome"), { From c4db84b5b629bf1d8e089a6d4708b05fa0c8c691 Mon Sep 17 00:00:00 2001 From: Daniel Date: Fri, 4 Sep 2026 18:43:28 +0800 Subject: [PATCH 2/2] fix: GUI returns 400 (not 403) when a POST omits the Origin header Bug review of EauDoon/agent-action-stack found: - LOW: bin/aas-gui.mjs requestBoundaryFailure returns the same "origin" code whether the caller is missing the Origin header entirely or sent a wrong-origin header. The handler at line 100 returns 403 Forbidden for both, with the body { error: "Forbidden" }. A programmatic local client that forgets to send Origin (curl, a CI step, a non-browser agent) sees a generic 403 and has no way to tell that the only fix is to add the header. A wrong-origin POST is genuinely forbidden; a missing-Origin POST is a misconfiguration that should be 400. Fix: add a "missing-origin" branch in requestBoundaryFailure and a 400 response with a clear "Origin header required for POST" message in the handler. Wrong-origin POSTs still get 403. Update test/gui.test.mjs to assert the 400 and the new error message. Verified: node --test test/gui.test.mjs runs 2 tests, all pass. node --test test/stack.test.mjs still runs 40 tests, all pass. --- bin/aas-gui.mjs | 13 ++++++++++++- test/gui.test.mjs | 3 ++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/bin/aas-gui.mjs b/bin/aas-gui.mjs index ffc3f78..b7cd4a6 100644 --- a/bin/aas-gui.mjs +++ b/bin/aas-gui.mjs @@ -53,6 +53,12 @@ function requestBoundaryFailure(request) { const hostHeaders = request.rawHeaders.filter((value, index) => index % 2 === 0 && value.toLowerCase() === "host"); if (hostHeaders.length !== 1 || request.headers.host !== expectedHost) return "host"; const origin = request.headers.origin; + // Distinguish a missing-Origin POST (caller forgot to identify itself) from + // a wrong-Origin POST (caller is some other origin). The first is a 400 + // ("you forgot to send Origin"), the second is a 403 ("Origin does not + // match this server"). Both still return 403 today; the missing-Origin + // case is the surprising one for programmatic local clients. + if (request.method === "POST" && origin === undefined) return "missing-origin"; if (request.method === "POST" && origin !== `http://${expectedHost}`) return "origin"; if (origin !== undefined && origin !== `http://${expectedHost}`) return "origin"; return null; @@ -90,7 +96,12 @@ export function createGuiServer({ return createServer(async (request, response) => { const url = new URL(request.url ?? "/", "http://127.0.0.1"); try { - if (requestBoundaryFailure(request) !== null) { + const boundary = requestBoundaryFailure(request); + if (boundary === "missing-origin") { + sendJson(response, 400, { error: "Origin header required for POST" }); + return; + } + if (boundary !== null) { sendJson(response, 403, { error: "Forbidden" }); return; } diff --git a/test/gui.test.mjs b/test/gui.test.mjs index cb23110..800376f 100644 --- a/test/gui.test.mjs +++ b/test/gui.test.mjs @@ -76,7 +76,8 @@ test("GUI rejects rebinding requests, cross-origin runs, unsafe options, and uns }); assert.equal(foreignOrigin.status, 403); const missingOrigin = await requestServer(server, "/api/run", { method: "POST" }); - assert.equal(missingOrigin.status, 403); + assert.equal(missingOrigin.status, 400); + assert.equal(JSON.parse(missingOrigin.body).error, "Origin header required for POST"); const getRun = await requestServer(server, "/api/run?response=pass&fault=none"); assert.equal(getRun.status, 404); const unsafeOption = await requestServer(server, "/api/run?response=..%2Fsecret&fault=none", {