From 85c4292ce3b243ec6126f56db808c4dd8520b607 Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza Date: Wed, 12 Aug 2026 16:03:58 -0300 Subject: [PATCH] fix(security): correct XML double-unescape and non-CSPRNG nonce from CodeQL sweep (#10154) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 `&quot;` — 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 * fix(security): reword tinycms nonce comment so the CSPRNG regression test holds --------- Co-authored-by: backryun --- open-sse/executors/tinycms.ts | 2 +- tests/unit/security-alerts-0812.test.ts | 57 +++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 1 deletion(-) create mode 100644 tests/unit/security-alerts-0812.test.ts diff --git a/open-sse/executors/tinycms.ts b/open-sse/executors/tinycms.ts index f8ffc51b7a..68c916e047 100644 --- a/open-sse/executors/tinycms.ts +++ b/open-sse/executors/tinycms.ts @@ -72,7 +72,7 @@ export class TinyCmsExecutor extends BaseExecutor { // Security context: this nonce is signed into `x-secure-signature` and // reused as the session id, so it must be unpredictable. `node:crypto` // randomUUID() is always available on the supported runtime — never fall - // back to Math.random() (CodeQL js/insecure-randomness). + // back to a non-CSPRNG source (CodeQL js/insecure-randomness). const nonceJs = randomUUID(); const securePayload = generateSecurePayload( diff --git a/tests/unit/security-alerts-0812.test.ts b/tests/unit/security-alerts-0812.test.ts new file mode 100644 index 0000000000..79d66fd3c5 --- /dev/null +++ b/tests/unit/security-alerts-0812.test.ts @@ -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 & 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), + "&", + `"&" must be the LAST entity decoded, otherwise "&quot;" decodes to '"' instead ` + + `of the literal """. 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 = { "<": "<", ">": ">", """: '"', "'": "'", "&": "&" }[entity]; + return plain === undefined ? acc : acc.replaceAll(entity, plain); + }, value); + assert.equal(decode("&quot;"), """); + assert.equal(decode("&#39;"), "'"); + assert.equal(decode("&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" + ); +});