Compare commits

...

1 Commits

Author SHA1 Message Date
backryun
266ffc9756 fix(security): correct XML double-unescape and non-CSPRNG nonce from CodeQL sweep
Two real defects surfaced by the 2026-08-12 code-scanning triage.

decodeXmlText decoded `&` before `"`/`'`, so `"` — the encoding of
the literal text `"` — collapsed to `"` in a second pass. The decoded values feed the
workspace-root trust comparison in parseTrustedCodexEnvironment, so an encoded path could
decode into a different path than the client declared. Decoding `&` last fixes it.

The tinycms nonce fell back to `Date.now()` plus a non-cryptographic PRNG when
`crypto.randomUUID` was absent. That nonce is signed into the provider's anti-replay
payload, so the fallback produced a predictable value silently. It is now always
`randomUUID()` from node:crypto, which is present on every supported runtime.

Both guards are mutation-validated: reverting either fix makes the new test fail.

Refs #9985
2026-08-12 03:48:39 -03:00
3 changed files with 72 additions and 14 deletions

View File

@@ -1,3 +1,5 @@
import { randomUUID } from "node:crypto";
import { BaseExecutor, type ExecuteInput } from "./base.ts";
import { makeExecutorErrorResult as makeErrorResult } from "../utils/error.ts";
import { initTinyCmsWasm, generateSecurePayload } from "./tinycmsSigner.ts";
@@ -28,9 +30,9 @@ async function fetchChallenge(uuid: string): Promise<any> {
const res = await fetch(CHALLENGE_URL, {
method: "GET",
headers: {
"uuid": uuid,
uuid: uuid,
"x-origin": "https://gov.freegpt.win",
"Accept": "application/json",
Accept: "application/json",
"User-Agent": "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36",
},
});
@@ -67,10 +69,10 @@ export class TinyCmsExecutor extends BaseExecutor {
const challengeObj = await fetchChallenge(uuid);
const timestamp = Date.now().toString();
const nonceJs =
typeof crypto !== "undefined" && crypto.randomUUID
? crypto.randomUUID()
: `${Date.now()}-${Math.random().toString(16).slice(2)}`;
// The nonce is signed into the anti-replay payload below, so it must come from a CSPRNG.
// `randomUUID` from node:crypto is always available on the supported runtimes — the old
// non-cryptographic fallback produced a predictable nonce with no way to notice.
const nonceJs = randomUUID();
const securePayload = generateSecurePayload(
uuid,
@@ -121,12 +123,7 @@ export class TinyCmsExecutor extends BaseExecutor {
body: response.body,
};
} catch (err: any) {
return makeErrorResult(
500,
`TinyCMS Error: ${err.message}`,
body,
CHAT_URL
);
return makeErrorResult(500, `TinyCMS Error: ${err.message}`, body, CHAT_URL);
}
}
}

View File

@@ -237,12 +237,16 @@ function trustedEnvironmentText(parsed: CodexParsedRequest): string {
}
function decodeXmlText(value: string): string {
// `&amp;` MUST be decoded last: decoding it first turns `&amp;quot;` (the encoding of the
// literal text `&quot;`) into `&quot;`, which the following pass then decodes again into `"`.
// These values feed the workspace-root trust comparison below, so a double-unescape lets an
// encoded path decode into a different path than the one the client actually declared.
return value
.replaceAll("&lt;", "<")
.replaceAll("&gt;", ">")
.replaceAll("&amp;", "&")
.replaceAll("&quot;", '"')
.replaceAll("&#39;", "'");
.replaceAll("&#39;", "'")
.replaceAll("&amp;", "&");
}
function uniqueAbsolutePaths(values: string[], field: string): string[] {

View File

@@ -0,0 +1,57 @@
import assert from "node:assert/strict";
import { readFileSync } from "node:fs";
import { join } from "node:path";
import test from "node:test";
const REPO_ROOT = join(import.meta.dirname, "..", "..");
/**
* Regression guards for the CodeQL alerts triaged on 2026-08-12.
*
* Both are source-level invariants rather than behavioral round-trips: the functions they
* protect are module-private (`decodeXmlText`) or only reachable through a live upstream
* handshake (`tinycms` nonce), so the guard asserts the property on the source itself.
*/
test("decodeXmlText decodes &amp; last so encoded entities do not double-unescape", () => {
const source = readFileSync(
join(REPO_ROOT, "open-sse/vendor/codex-chatgpt-web/adapters/chatgpt-web/environment.ts"),
"utf8"
);
const body = /function decodeXmlText\(value: string\): string \{([\s\S]*?)\n\}/.exec(source)?.[1];
assert.ok(body, "decodeXmlText not found — update this guard if the helper was renamed");
const order = [...body.matchAll(/replaceAll\("(&[^"]+;)"/g)].map((match) => match[1]);
assert.ok(order.length >= 2, `expected several entity replacements, got ${order.length}`);
assert.equal(
order.at(-1),
"&amp;",
`"&amp;" must be the LAST entity decoded, otherwise "&amp;quot;" decodes to '"' instead ` +
`of the literal "&quot;". Current order: ${order.join(" -> ")}`
);
// Mirror the implementation to document the property the ordering buys us.
const decode = (value: string) =>
order.reduce((acc, entity) => {
const plain = { "&lt;": "<", "&gt;": ">", "&quot;": '"', "&#39;": "'", "&amp;": "&" }[entity];
return plain === undefined ? acc : acc.replaceAll(entity, plain);
}, value);
assert.equal(decode("&amp;quot;"), "&quot;");
assert.equal(decode("&amp;#39;"), "&#39;");
assert.equal(decode("&amp;lt;"), "&lt;");
});
test("tinycms signs its anti-replay nonce with a CSPRNG, never Math.random", () => {
const source = readFileSync(join(REPO_ROOT, "open-sse/executors/tinycms.ts"), "utf8");
assert.match(
source,
/const nonceJs = randomUUID\(\)/,
"the tinycms nonce must come from node:crypto randomUUID"
);
assert.doesNotMatch(
source,
/Math\.random\(\)/,
"Math.random() is not a CSPRNG — the nonce is signed into the anti-replay payload"
);
});