Compare commits

...

1 Commits

Author SHA1 Message Date
backryun
bc7a9c369c fix(security): resolve open CodeQL alerts
Alert 806 (js/insecure-randomness, open-sse/executors/tinycms.ts): the
TinyCMS nonce is signed into x-secure-signature and reused as
x-secure-nonce / x-session-id, so the Math.random() fallback made a
signed request predictable and replayable. Use randomUUID() from
node:crypto unconditionally.

Alert 811 (js/double-escaping, chatgpt-web adapters/environment.ts):
decodeXmlText() decoded & before " / ', so the bare & it
produced was re-consumed and the text was unescaped twice
(" collapsed to "). These values become the trusted Codex
sandbox cwd / workspace_roots, so the double-unescape silently rewrote
the workspace boundary. Decode & last.

Alerts 813/814 (js/incomplete-url-substring-sanitization, test files):
replace the includes() URL checks with exact comparisons
(new URL(url).hostname === ... and an explicit === over the recorded
URL array). Both assertions get strictly tighter.

Regression guards: tests/unit/tinycms-secure-nonce-randomness.test.ts
and tests/unit/chatgpt-web-environment-double-unescape.test.ts, both
failing before the fix and passing after.
2026-08-12 10:59:01 -03:00
6 changed files with 264 additions and 55 deletions

View File

@@ -1,3 +1,5 @@
import { randomUUID } from "node:crypto";
import { BaseExecutor, type ExecuteInput, type ExecutorExecuteResult } 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,11 @@ 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)}`;
// 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).
const nonceJs = randomUUID();
const securePayload = generateSecurePayload(
uuid,
@@ -122,12 +125,7 @@ export class TinyCmsExecutor extends BaseExecutor {
transformedBody: bodyObj,
};
} 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,15 @@ function trustedEnvironmentText(parsed: CodexParsedRequest): string {
}
function decodeXmlText(value: string): string {
// `&amp;` MUST be decoded last: decoding it first produces a bare `&` that the
// later passes re-consume, so `&amp;quot;` would collapse to `"` instead of the
// literal `&quot;` (double-unescape — CodeQL js/double-escaping).
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,83 @@
/**
* CodeQL alert 811 — js/double-escaping (HIGH) on
* `open-sse/vendor/codex-chatgpt-web/adapters/chatgpt-web/environment.ts`.
*
* `decodeXmlText()` unescapes the XML entities of the trusted Codex
* `<environment_context>` block. It decoded `&amp;` BEFORE `&quot;` / `&#39;`,
* so the `&` it produced was re-consumed by a later `replaceAll` and the text
* was unescaped twice: `&amp;quot;` collapsed to `"` instead of `&quot;`.
*
* These values become sandbox `cwd` / `workspace_roots` paths, so a
* double-unescape silently rewrites the trusted workspace boundary.
* `&amp;` must be decoded LAST.
*/
import test from "node:test";
import assert from "node:assert/strict";
import { extractChatGptTurnEnvironment } from "../../open-sse/vendor/codex-chatgpt-web/adapters/chatgpt-web/environment.ts";
function parsedRequestWithCwd(cwdLiteral: string) {
const environmentText = [
"<environment_context>",
` <cwd>${cwdLiteral}</cwd>`,
" <sandbox_mode>read-only</sandbox_mode>",
"</environment_context>",
].join("\n");
const turnMetadata = { internal_chat_message_metadata_passthrough: { turn_id: "turn-1" } };
return {
context: { tools: [] },
_rawBody: {
client_metadata: {
"x-codex-turn-metadata": JSON.stringify({ thread_id: "thread-1", turn_id: "turn-1" }),
},
input: [
{ type: "message", role: "system", content: [{ type: "input_text", text: "sys" }] },
{
type: "message",
role: "user",
content: [{ type: "input_text", text: environmentText }],
...turnMetadata,
},
{
type: "message",
role: "user",
content: [{ type: "input_text", text: "hello" }],
...turnMetadata,
},
],
},
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} as any;
}
test("decoding the trusted Codex environment does not double-unescape &amp;quot;", () => {
const env = extractChatGptTurnEnvironment(parsedRequestWithCwd("/tmp/ws&amp;quot;dir"));
assert.equal(
env.cwd,
"/tmp/ws&quot;dir",
'`&amp;quot;` must decode to the literal text `&quot;`, not to a double-unescaped `"`'
);
});
test("decoding the trusted Codex environment does not double-unescape &amp;lt; / &amp;#39;", () => {
assert.equal(
extractChatGptTurnEnvironment(parsedRequestWithCwd("/tmp/ws&amp;lt;dir")).cwd,
"/tmp/ws&lt;dir"
);
assert.equal(
extractChatGptTurnEnvironment(parsedRequestWithCwd("/tmp/ws&amp;#39;dir")).cwd,
"/tmp/ws&#39;dir"
);
});
test("single-level XML entities still decode normally", () => {
assert.equal(extractChatGptTurnEnvironment(parsedRequestWithCwd("/tmp/a&amp;b")).cwd, "/tmp/a&b");
assert.equal(
extractChatGptTurnEnvironment(parsedRequestWithCwd("/tmp/a&quot;b")).cwd,
'/tmp/a"b'
);
assert.equal(extractChatGptTurnEnvironment(parsedRequestWithCwd("/tmp/a&#39;b")).cwd, "/tmp/a'b");
assert.equal(extractChatGptTurnEnvironment(parsedRequestWithCwd("/tmp/a&gt;b")).cwd, "/tmp/a>b");
});

View File

@@ -17,10 +17,7 @@ import assert from "node:assert/strict";
import { WEB_COOKIE_PROVIDERS } from "../../src/shared/constants/providers/web-cookie.ts";
import { REGISTRY } from "../../open-sse/config/providers/index.ts";
import { getExecutor, TinyCmsExecutor } from "../../open-sse/executors/index.ts";
import {
setupDomMocks,
type DomMockRestore,
} from "../../open-sse/executors/tinycmsSigner.ts";
import { setupDomMocks, type DomMockRestore } from "../../open-sse/executors/tinycmsSigner.ts";
// tinycmsSigner.ts intentionally does NOT install its window/document/canvas
// shims as a module-load side effect (see setupDomMocks() there) — doing so
@@ -41,9 +38,10 @@ after(() => {
// ── Catalog / WEB_COOKIE_PROVIDERS ────────────────────────────────────────────
test("tinycms-web is present in WEB_COOKIE_PROVIDERS", () => {
const p = (WEB_COOKIE_PROVIDERS as Record<string, unknown>)[
"tinycms-web"
] as Record<string, unknown>;
const p = (WEB_COOKIE_PROVIDERS as Record<string, unknown>)["tinycms-web"] as Record<
string,
unknown
>;
assert.ok(p, "WEB_COOKIE_PROVIDERS['tinycms-web'] must exist");
assert.equal(p.id, "tinycms-web");
assert.equal(p.alias, "tcw");
@@ -51,9 +49,10 @@ test("tinycms-web is present in WEB_COOKIE_PROVIDERS", () => {
});
test("tinycms-web WEB_COOKIE_PROVIDERS entry is marked as free-tier", () => {
const p = (WEB_COOKIE_PROVIDERS as Record<string, unknown>)[
"tinycms-web"
] as Record<string, unknown>;
const p = (WEB_COOKIE_PROVIDERS as Record<string, unknown>)["tinycms-web"] as Record<
string,
unknown
>;
assert.equal(p.hasFree, true);
assert.ok(typeof p.freeNote === "string" && (p.freeNote as string).length > 0);
assert.ok(typeof p.authHint === "string" && (p.authHint as string).length > 0);
@@ -79,10 +78,7 @@ test("tinycms-web registry has all expected models", () => {
assert.ok(ids.includes("gpt-5-free"), "gpt-5-free must be registered");
assert.ok(ids.includes("gpt-5.3-free"), "gpt-5.3-free must be registered");
assert.ok(
ids.includes("gpt-5.3-thinking-free"),
"gpt-5.3-thinking-free must be registered"
);
assert.ok(ids.includes("gpt-5.3-thinking-free"), "gpt-5.3-thinking-free must be registered");
assert.ok(ids.includes("deepseek-v4-flash"), "deepseek-v4-flash must be registered");
assert.ok(ids.includes("claude-sonnet-5"), "claude-sonnet-5 must be registered");
assert.ok(ids.includes("gemini-3.5-flash"), "gemini-3.5-flash must be registered");
@@ -140,10 +136,7 @@ test("TinyCmsExecutor returns 401 when UUID is missing", async () => {
assert.equal(result.response.status, 401);
const body = await result.response.json();
const errMsg = body?.error?.message || "";
assert.ok(
errMsg.includes("Invalid or missing device UUID"),
"error must mention missing UUID"
);
assert.ok(errMsg.includes("Invalid or missing device UUID"), "error must mention missing UUID");
// Hard Rule #12: must NOT leak stack traces
assert.ok(!errMsg.includes("at /"), "error must not contain a stack trace path");
});
@@ -161,10 +154,7 @@ test("TinyCmsExecutor returns 401 when UUID does not start with 'R'", async () =
assert.equal(result.response.status, 401);
const body = await result.response.json();
const errMsg = body?.error?.message || "";
assert.ok(
errMsg.includes("Invalid or missing device UUID"),
"error must mention missing UUID"
);
assert.ok(errMsg.includes("Invalid or missing device UUID"), "error must mention missing UUID");
assert.ok(!errMsg.includes("at /"), "error must not contain a stack trace path");
});
@@ -172,7 +162,7 @@ test("TinyCmsExecutor returns the standard executor response envelope on success
const originalFetch = globalThis.fetch;
globalThis.fetch = async (input) => {
const url = String(input);
if (url.includes("api64.ipify.org")) {
if (new URL(url).hostname === "api64.ipify.org") {
return new Response(JSON.stringify({ ip: "127.0.0.1" }), {
headers: { "Content-Type": "application/json" },
});
@@ -217,10 +207,7 @@ test("TinyCmsExecutor returns the standard executor response envelope on success
test("initTinyCmsWasm module exports expected functions", async () => {
const signer = await import("../../open-sse/executors/tinycmsSigner.ts");
assert.ok(
typeof signer.initTinyCmsWasm === "function",
"must export initTinyCmsWasm function"
);
assert.ok(typeof signer.initTinyCmsWasm === "function", "must export initTinyCmsWasm function");
assert.ok(
typeof signer.generateSecurePayload === "function",
"must export generateSecurePayload function"
@@ -254,10 +241,7 @@ test("TinyCmsExecutor sanitizes errors (no stack traces in error response)", asy
assert.ok(result.response, "response must be present");
const body = await result.response.json();
const errMsg = body?.error?.message || "";
assert.ok(
errMsg.includes("Invalid or missing device UUID"),
"error must mention missing UUID"
);
assert.ok(errMsg.includes("Invalid or missing device UUID"), "error must mention missing UUID");
assert.ok(!errMsg.includes("at /"), "error must not contain a stack trace path (Hard Rule #12)");
});
@@ -274,12 +258,6 @@ test("tinycms-web credential requirement is kind: token with app-config-uuid", a
assert.equal(req.credentialName, "app-config-uuid");
assert.equal(req.acceptsFullCookieHeader, false);
assert.ok(Array.isArray(req.storageKeys), "must have storageKeys array");
assert.ok(
(req.storageKeys as string[]).includes("apiKey"),
"apiKey must be in storageKeys"
);
assert.ok(
(req.storageKeys as string[]).includes("uuid"),
"uuid must be in storageKeys"
);
assert.ok((req.storageKeys as string[]).includes("apiKey"), "apiKey must be in storageKeys");
assert.ok((req.storageKeys as string[]).includes("uuid"), "uuid must be in storageKeys");
});

View File

@@ -0,0 +1,142 @@
/**
* CodeQL alert 806 — js/insecure-randomness (HIGH) on
* `open-sse/executors/tinycms.ts`.
*
* The TinyCMS executor derives `x-secure-nonce` / `x-session-id` from a nonce
* that is fed into the upstream request signature (`generateSecurePayload`).
* That is a security context, so the nonce must never fall back to
* `Math.random()` — a predictable nonce lets an observer replay or forge a
* signed request.
*
* The regression guard runs the executor with a `globalThis.crypto` that has no
* `randomUUID` (the exact condition that used to select the `Math.random()`
* fallback) and asserts the emitted nonce is still a cryptographically strong
* UUID.
*/
import test, { before, after } from "node:test";
import assert from "node:assert/strict";
import { TinyCmsExecutor } from "../../open-sse/executors/index.ts";
import { setupDomMocks, type DomMockRestore } from "../../open-sse/executors/tinycmsSigner.ts";
let restoreDomMocks: DomMockRestore;
before(() => {
restoreDomMocks = setupDomMocks();
});
after(() => {
restoreDomMocks();
});
const UUID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i;
test("TinyCMS nonce stays cryptographically strong when globalThis.crypto has no randomUUID", async () => {
const originalFetch = globalThis.fetch;
const originalCryptoDescriptor = Object.getOwnPropertyDescriptor(globalThis, "crypto")!;
const realCrypto = globalThis.crypto;
// Keep every other WebCrypto capability, drop only `randomUUID`. This is the
// branch that previously fell back to `Math.random()`.
Object.defineProperty(globalThis, "crypto", {
configurable: true,
value: {
getRandomValues: (array: ArrayBufferView) => realCrypto.getRandomValues(array as never),
subtle: realCrypto.subtle,
},
});
const seenHeaders: Record<string, string>[] = [];
globalThis.fetch = (async (input: unknown, init?: RequestInit) => {
const url = String(input);
if (new URL(url).hostname === "api64.ipify.org") {
return new Response(JSON.stringify({ ip: "127.0.0.1" }), {
headers: { "Content-Type": "application/json" },
});
}
if (new URL(url).pathname === "/api/challenge") {
return new Response(
JSON.stringify({
challenge: "test",
challengeId: "challenge-id",
expiresAt: Date.now() + 60_000,
version: "1",
difficulty: 0,
}),
{ headers: { "Content-Type": "application/json" } }
);
}
seenHeaders.push((init?.headers ?? {}) as Record<string, string>);
return new Response("upstream body", { status: 200 });
}) as typeof fetch;
try {
await new TinyCmsExecutor().execute({
model: "gpt-5-free",
body: { messages: [{ role: "user", content: "hi" }] },
stream: false,
credentials: { apiKey: "Rtest-device" },
});
assert.equal(seenHeaders.length, 1, "the executor must reach the chat endpoint exactly once");
const headers = seenHeaders[0]!;
assert.match(
headers["x-secure-nonce"] ?? "",
UUID_RE,
"x-secure-nonce must be a crypto-strong UUID, never a Math.random() fallback"
);
assert.match(
headers["x-session-id"] ?? "",
UUID_RE,
"x-session-id must be a crypto-strong UUID, never a Math.random() fallback"
);
} finally {
globalThis.fetch = originalFetch;
Object.defineProperty(globalThis, "crypto", originalCryptoDescriptor);
}
});
test("consecutive TinyCMS nonces are unique", async () => {
const originalFetch = globalThis.fetch;
const nonces: string[] = [];
globalThis.fetch = (async (input: unknown, init?: RequestInit) => {
const url = String(input);
if (new URL(url).hostname === "api64.ipify.org") {
return new Response(JSON.stringify({ ip: "127.0.0.1" }), {
headers: { "Content-Type": "application/json" },
});
}
if (new URL(url).pathname === "/api/challenge") {
return new Response(
JSON.stringify({
challenge: "test",
challengeId: "challenge-id",
expiresAt: Date.now() + 60_000,
version: "1",
difficulty: 0,
}),
{ headers: { "Content-Type": "application/json" } }
);
}
nonces.push(((init?.headers ?? {}) as Record<string, string>)["x-secure-nonce"] ?? "");
return new Response("upstream body", { status: 200 });
}) as typeof fetch;
try {
const executor = new TinyCmsExecutor();
for (let i = 0; i < 3; i += 1) {
await executor.execute({
model: "gpt-5-free",
body: { messages: [{ role: "user", content: "hi" }] },
stream: false,
credentials: { apiKey: "Rtest-device" },
});
}
assert.equal(nonces.length, 3);
assert.equal(new Set(nonces).size, 3, "each request must carry a distinct nonce");
} finally {
globalThis.fetch = originalFetch;
}
});

View File

@@ -55,14 +55,19 @@ test("#8014: ZaiWebExecutor must POST to the current chat.z.ai v2 chat-completio
assert.ok(requested.length > 0, "the direct path must actually reach fetch");
assert.ok(
!requested.includes(STALE_URL),
// Exact-URL match (not a substring test): `requested` holds whole URLs.
!requested.some((url) => url === STALE_URL),
`zai-web executor POSTed to the stale endpoint — matches #8014's model-independent 404 "Not Found"`
);
// The executor also probes the homepage for the frontend version and calls
// /api/v1/chats/new first, so pick the completions request by its path.
const completions = requested.filter((u) => new URL(u).pathname.endsWith("/chat/completions"));
assert.equal(completions.length, 1, `expected exactly one completions request, got ${requested}`);
assert.equal(
completions.length,
1,
`expected exactly one completions request, got ${requested}`
);
assert.equal(
new URL(completions[0]).pathname,
"/api/v2/chat/completions",