diff --git a/.env.example b/.env.example index e73f3203f1..1aa19cd898 100644 --- a/.env.example +++ b/.env.example @@ -3043,6 +3043,13 @@ QUOTA_STORE_DRIVER=sqlite # CHATGPT_WEB_CODEX_CHROME_PATH=/usr/bin/chromium # CHROME_PATH=/usr/bin/chromium # CHATGPT_WEB_CODEX_CDP_URL=http://chatgpt-web-codex-browser:9223 +# CDP_PROXY_TOKEN required by docker/chatgpt-web-codex-browser/cdp-proxy.mjs (#13679): +# when set, every request to the CDP proxy sidecar must present it as an +# `X-Omni-Cdp-Token` header. Left unset, the proxy keeps forwarding requests +# unauthenticated (network isolation via docker-compose.yml's dedicated +# `chatgpt-web-codex-net` is the default mitigation). Generate with: +# `openssl rand -hex 32` +# CDP_PROXY_TOKEN= # CHATGPT_WEB_CODEX_TUNNEL_ID=tunnel_0123456789abcdef0123456789abcdef # CHATGPT_WEB_CODEX_RUNTIME_KEY= # CHATGPT_WEB_CODEX_CONNECTOR_NAME=OmniRoute Codex v2 diff --git a/changelog.d/fixes/13679-cdp-proxy-auth-network-isolation.md b/changelog.d/fixes/13679-cdp-proxy-auth-network-isolation.md new file mode 100644 index 0000000000..1612b0e558 --- /dev/null +++ b/changelog.d/fixes/13679-cdp-proxy-auth-network-isolation.md @@ -0,0 +1 @@ +- **fix(docker):** isolate the ChatGPT Web (Codex) CDP proxy sidecar onto its own Compose network, add an opt-in `CDP_PROXY_TOKEN` auth gate to `cdp-proxy.mjs`, and stop the VNC browser-login CDP bridge from starting when no token is configured (#13679) diff --git a/docker-compose.yml b/docker-compose.yml index 831f4ea175..0bf20c4785 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -142,11 +142,24 @@ services: - "${APP_BIND_HOST:-127.0.0.1}:${DASHBOARD_PORT:-20128}:${DASHBOARD_PORT:-20128}" - "${APP_BIND_HOST:-127.0.0.1}:${API_PORT:-20129}:${API_PORT:-20129}" - "${APP_BIND_HOST:-127.0.0.1}:${LIVE_WS_PORT:-20132}:${LIVE_WS_PORT:-20132}" + # SECURITY (#13679): joins BOTH `default` (to keep reaching redis and the + # other sidecars) AND the dedicated `chatgpt-web-codex-net` (the one + # legitimate consumer of the CDP proxy below). + networks: + - default + - chatgpt-web-codex-net profiles: - web # Internal-only Chromium runtime for ChatGPT Web (Codex). No CDP or browser # UI port is published to the host. + # + # SECURITY (#13679): isolated onto its own `chatgpt-web-codex-net` network + # instead of the shared implicit default bridge — its cdp-proxy.mjs + # sidecar republishes Chromium's CDP on 0.0.0.0:9223, and CDP grants full + # control over a live browser session. Without this isolation, any + # compromised sibling container (redis, qdrant, bifrost, cliproxyapi, + # codex-app-server, ...) on the default network could reach it. chatgpt-web-codex-browser: build: context: . @@ -154,8 +167,12 @@ services: image: omniroute:chatgpt-web-codex-browser restart: unless-stopped shm_size: "2gb" + environment: + - CDP_PROXY_TOKEN=${CDP_PROXY_TOKEN:-} volumes: - chatgpt-web-codex-browser-data:/browser-profile + networks: + - chatgpt-web-codex-net profiles: - web @@ -370,7 +387,12 @@ services: # compose network by the omniroute app. healthcheck: test: - ["CMD", "node", "-e", "require('http').get('http://127.0.0.1:1456/readyz',r=>process.exit(r.statusCode===200?0:1)).on('error',()=>process.exit(1))"] + [ + "CMD", + "node", + "-e", + "require('http').get('http://127.0.0.1:1456/readyz',r=>process.exit(r.statusCode===200?0:1)).on('error',()=>process.exit(1))", + ] interval: 30s timeout: 5s retries: 3 @@ -378,6 +400,13 @@ services: profiles: - codex-app-server +networks: + # SECURITY (#13679): dedicated network for the unauthenticated-by-default + # CDP proxy sidecar (docker/chatgpt-web-codex-browser/cdp-proxy.mjs) — + # shared only with omniroute-web, not with redis/qdrant/bifrost/cliproxyapi/ + # codex-app-server or any other sibling on the implicit default network. + chatgpt-web-codex-net: {} + volumes: chatgpt-web-codex-browser-data: name: omniroute-chatgpt-web-codex-browser-data diff --git a/docker/chatgpt-web-codex-browser/cdp-proxy.mjs b/docker/chatgpt-web-codex-browser/cdp-proxy.mjs index a340348803..8615a22678 100644 --- a/docker/chatgpt-web-codex-browser/cdp-proxy.mjs +++ b/docker/chatgpt-web-codex-browser/cdp-proxy.mjs @@ -5,6 +5,31 @@ const listenPort = 9223; const upstreamHost = "127.0.0.1"; const upstreamPort = 9222; +// SECURITY (#13679): this proxy republishes Chromium's loopback CDP onto +// 0.0.0.0:9223 with no auth of its own — CDP grants full control over a +// live browser session (Runtime.evaluate, cookie theft, etc). When the +// operator sets CDP_PROXY_TOKEN, every request/WS-upgrade MUST present it as +// an `X-Omni-Cdp-Token: ` header before a single byte is forwarded +// upstream, mirroring the gate docker/vnc-browser/chromium/cdp-bridge.py +// already has (#12571). Left unset, the proxy keeps its historical +// zero-config behavior — the primary mitigation for the shared-bridge risk +// is docker-compose.yml isolating this service onto its own network so no +// unrelated sibling container can reach it at all. +const TOKEN = process.env.CDP_PROXY_TOKEN || ""; +const TOKEN_HEADER = "x-omni-cdp-token"; + +if (!TOKEN) { + console.error( + "[cdp-proxy] WARNING: running without CDP_PROXY_TOKEN — every request is forwarded " + + "unauthenticated. Set CDP_PROXY_TOKEN to require an X-Omni-Cdp-Token header (#13679)." + ); +} + +function hasValidToken(headers) { + if (!TOKEN) return true; + return headers[TOKEN_HEADER] === TOKEN; +} + function proxyHeaders(headers) { const next = { ...headers, host: `${upstreamHost}:${upstreamPort}` }; delete next.connection; @@ -13,6 +38,11 @@ function proxyHeaders(headers) { } const server = http.createServer((request, response) => { + if (!hasValidToken(request.headers)) { + response.writeHead(403, { "content-type": "application/json" }); + response.end(JSON.stringify({ error: "missing or invalid X-Omni-Cdp-Token" })); + return; + } const upstream = http.request( { host: upstreamHost, @@ -48,6 +78,10 @@ const server = http.createServer((request, response) => { }); server.on("upgrade", (request, socket, head) => { + if (!hasValidToken(request.headers)) { + socket.destroy(); + return; + } const upstream = net.connect(upstreamPort, upstreamHost, () => { const upgradeHeaders = { ...request.headers, @@ -69,4 +103,6 @@ server.on("upgrade", (request, socket, head) => { upstream.on("error", () => socket.destroy()); }); -server.listen(listenPort, "0.0.0.0"); +server.listen(listenPort, "0.0.0.0", () => { + console.error(`[cdp-proxy] listening on 0.0.0.0:${listenPort}`); +}); diff --git a/docker/vnc-browser/chromium/svc-de-run b/docker/vnc-browser/chromium/svc-de-run index 3d2dee7f49..7cc6e40a7e 100644 --- a/docker/vnc-browser/chromium/svc-de-run +++ b/docker/vnc-browser/chromium/svc-de-run @@ -10,7 +10,13 @@ if [[ "${PIXELFLUX_WAYLAND,,}" == "true" ]]; then echo "[svc-de] ${SOCKET_PATH} found launching de" cd $HOME # OmniRoute: bridge Chromium DevTools (127.0.0.1:9222) to 0.0.0.0:9223. - ( sleep 8; python3 /usr/local/bin/cdp-bridge.py >/proc/1/fd/2 2>&1 ) & + # SECURITY (#13679): only start the bridge when CDP_BRIDGE_TOKEN is + # configured — cdp-bridge.py already fails closed for every caller when + # it is unset (#12571), so an unconfigured container gains nothing by + # running an always-listening 0.0.0.0:9223 process anyway. + if [ -n "${CDP_BRIDGE_TOKEN:-}" ]; then + ( sleep 8; python3 /usr/local/bin/cdp-bridge.py >/proc/1/fd/2 2>&1 ) & + fi exec s6-setuidgid abc \ /bin/bash /defaults/startwm_wayland.sh & PID=$! @@ -57,7 +63,13 @@ chmod 777 /tmp/selkies* # run cd $HOME # OmniRoute: bridge Chromium DevTools (127.0.0.1:9222) to 0.0.0.0:9223. -( sleep 8; python3 /usr/local/bin/cdp-bridge.py >/proc/1/fd/2 2>&1 ) & +# SECURITY (#13679): only start the bridge when CDP_BRIDGE_TOKEN is +# configured — cdp-bridge.py already fails closed for every caller when it +# is unset (#12571), so an unconfigured container gains nothing by running +# an always-listening 0.0.0.0:9223 process anyway. +if [ -n "${CDP_BRIDGE_TOKEN:-}" ]; then + ( sleep 8; python3 /usr/local/bin/cdp-bridge.py >/proc/1/fd/2 2>&1 ) & +fi exec s6-setuidgid abc \ /bin/bash /defaults/startwm.sh & PID=$! diff --git a/tests/unit/cdp-proxy-auth-gate-13679.test.ts b/tests/unit/cdp-proxy-auth-gate-13679.test.ts new file mode 100644 index 0000000000..13dfa946c6 --- /dev/null +++ b/tests/unit/cdp-proxy-auth-gate-13679.test.ts @@ -0,0 +1,140 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import http from "node:http"; +import { spawn, type ChildProcess } from "node:child_process"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const PROXY_SCRIPT = path.resolve( + __dirname, + "../../docker/chatgpt-web-codex-browser/cdp-proxy.mjs" +); +const UPSTREAM_PORT = 9222; // upstreamPort in cdp-proxy.mjs +const PROXY_PORT = 9223; // listenPort in cdp-proxy.mjs +const TOKEN = "test-secret-token-13679"; +const TOKEN_HEADER = "X-Omni-Cdp-Token"; + +// #13679 item #9: docker/chatgpt-web-codex-browser/cdp-proxy.mjs republishes +// Chromium's loopback CDP (127.0.0.1:9222) onto 0.0.0.0:9223 with NO auth +// check at all — unlike the sibling docker/vnc-browser/chromium/cdp-bridge.py, +// which requires an `X-Omni-Cdp-Token` header once CDP_BRIDGE_TOKEN is set +// (#12571). This proves cdp-proxy.mjs must gate requests the same way once an +// operator opts in via CDP_PROXY_TOKEN. + +function waitForListening(server: http.Server): Promise { + return new Promise((resolve, reject) => { + server.once("listening", () => resolve()); + server.once("error", reject); + }); +} + +function waitForProxyReady(child: ChildProcess): Promise { + return new Promise((resolve, reject) => { + const timer = setTimeout( + () => reject(new Error("cdp-proxy.mjs did not report ready in time")), + 5000 + ); + child.stderr?.on("data", (chunk: Buffer) => { + if (chunk.toString("utf8").includes("listening on")) { + clearTimeout(timer); + resolve(); + } + }); + child.once("error", (err) => { + clearTimeout(timer); + reject(err); + }); + child.once("exit", (code) => { + clearTimeout(timer); + reject(new Error(`cdp-proxy.mjs exited early with code ${code}`)); + }); + }); +} + +function startUpstream(): Promise<{ server: http.Server; receivedAnyRequest: () => boolean }> { + let received = false; + const server = http.createServer((_req, res) => { + received = true; + // Real Chromium CDP responses always carry Content-Length (never + // chunked) — match that here, since cdp-proxy.mjs blindly re-adds a + // computed content-length on top of whatever the upstream sent. + const body = "{}"; + res.writeHead(200, { + "content-type": "application/json", + "content-length": String(body.length), + }); + res.end(body); + }); + return waitForListening(server.listen(UPSTREAM_PORT, "127.0.0.1")).then(() => ({ + server, + receivedAnyRequest: () => received, + })); +} + +function startProxy(): ChildProcess { + return spawn(process.execPath, [PROXY_SCRIPT], { + stdio: ["ignore", "ignore", "pipe"], + env: { ...process.env, CDP_PROXY_TOKEN: TOKEN }, + }); +} + +function requestProxy(headers: Record): Promise { + return new Promise((resolve) => { + const req = http.request( + { host: "127.0.0.1", port: PROXY_PORT, path: "/json/version", method: "GET", headers }, + (res) => { + res.resume(); + resolve(res.statusCode ?? null); + } + ); + req.on("error", () => resolve(null)); + req.end(); + }); +} + +test("cdp-proxy.mjs must reject a request with no CDP token once CDP_PROXY_TOKEN is set (#13679)", async () => { + const upstream = await startUpstream(); + const proxy = startProxy(); + + try { + await waitForProxyReady(proxy); + const status = await requestProxy({}); + + assert.notEqual( + status, + 200, + "cdp-proxy.mjs forwarded an unauthenticated request straight through to Chromium's CDP " + + "port even though CDP_PROXY_TOKEN was set — the proxy has no auth gate at all" + ); + assert.equal( + upstream.receivedAnyRequest(), + false, + "cdp-proxy.mjs must not forward the request upstream before checking the CDP token" + ); + } finally { + proxy.kill("SIGKILL"); + await new Promise((resolve) => upstream.server.close(() => resolve())); + } +}); + +test("cdp-proxy.mjs forwards the request once the caller presents the configured token (#13679)", async () => { + const upstream = await startUpstream(); + const proxy = startProxy(); + + try { + await waitForProxyReady(proxy); + const status = await requestProxy({ [TOKEN_HEADER]: TOKEN }); + + assert.equal( + status, + 200, + "cdp-proxy.mjs should forward the request once the caller presents the correct " + + "CDP_PROXY_TOKEN" + ); + assert.equal(upstream.receivedAnyRequest(), true); + } finally { + proxy.kill("SIGKILL"); + await new Promise((resolve) => upstream.server.close(() => resolve())); + } +}); diff --git a/tests/unit/compose-cdp-proxy-network-isolation-13679.test.ts b/tests/unit/compose-cdp-proxy-network-isolation-13679.test.ts new file mode 100644 index 0000000000..a16a773b6f --- /dev/null +++ b/tests/unit/compose-cdp-proxy-network-isolation-13679.test.ts @@ -0,0 +1,99 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import path from "node:path"; + +const REPO_ROOT = path.resolve(import.meta.dirname, "../.."); + +// #13679 item #9: docker-compose.yml has no top-level `networks:` key, so +// Compose puts every service (redis, qdrant, bifrost, cliproxyapi, +// codex-app-server, chatgpt-web-codex-browser, ...) on the same implicit +// default bridge network. The chatgpt-web-codex-browser sidecar exposes an +// UNAUTHENTICATED CDP proxy on 9223 (docker/chatgpt-web-codex-browser/cdp-proxy.mjs) +// — any compromised sibling container on that shared bridge can reach it and +// take full control of the live browser session. It must be isolated onto a +// dedicated network shared only with the one legitimate consumer +// (omniroute-web). + +function readCompose(): string { + return fs.readFileSync(path.join(REPO_ROOT, "docker-compose.yml"), "utf8"); +} + +function serviceBlock(compose: string, serviceName: string): string { + const lines = compose.split("\n"); + const startIndex = lines.findIndex((line) => new RegExp(`^ ${serviceName}:\\s*$`).test(line)); + assert.notEqual(startIndex, -1, `service '${serviceName}' not found in docker-compose.yml`); + const rest = lines.slice(startIndex + 1); + const endOffset = rest.findIndex((line) => /^ \S/.test(line) || /^\S/.test(line)); + const block = endOffset === -1 ? rest : rest.slice(0, endOffset); + return block.join("\n"); +} + +/** + * The service's `networks:` entries, read line by line. A regex over the whole + * block would need nested quantifiers (`(\s*-\s*.*\n)*`), which CodeQL flags as + * a ReDoS risk (js/redos) — and the line walk is easier to read anyway. + */ +function listedNetworks(serviceYaml: string): string[] { + const lines = serviceYaml.split("\n"); + const start = lines.findIndex((line) => /^\s*networks:\s*$/.test(line)); + if (start === -1) return []; + const names: string[] = []; + for (const line of lines.slice(start + 1)) { + const item = line.match(/^\s*-\s*(\S+)\s*$/); + if (!item) break; + names.push(item[1]); + } + return names; +} + +test("docker-compose.yml declares a dedicated network for the CDP proxy sidecar", () => { + const compose = readCompose(); + assert.match( + compose, + /^networks:\s*$/m, + "docker-compose.yml must declare a top-level `networks:` key — without it every " + + "service shares the implicit default bridge, so any sibling container can reach " + + "the unauthenticated chatgpt-web-codex-browser CDP proxy on 9223" + ); +}); + +test("chatgpt-web-codex-browser is isolated off the shared default network", () => { + const compose = readCompose(); + const block = serviceBlock(compose, "chatgpt-web-codex-browser"); + assert.match( + block, + /networks:/, + "chatgpt-web-codex-browser must declare an explicit `networks:` list — otherwise it " + + "attaches to the implicit default network shared with redis/qdrant/bifrost/etc." + ); + assert.ok( + !listedNetworks(block).includes("default"), + "chatgpt-web-codex-browser must not also list `default` — that would put it right back " + + "on the shared bridge with every unrelated sibling container" + ); +}); + +test("omniroute-web (the one legitimate CDP consumer) stays reachable via the dedicated network", () => { + const compose = readCompose(); + const block = serviceBlock(compose, "omniroute-web"); + assert.match( + block, + /networks:/, + "omniroute-web must explicitly join the dedicated CDP-proxy network to keep reaching " + + "chatgpt-web-codex-browser:9223 after the sidecar is isolated off the default network" + ); +}); + +test("unrelated sidecars (redis, qdrant) are not put on the CDP-proxy network", () => { + const compose = readCompose(); + for (const serviceName of ["redis", "qdrant", "bifrost"]) { + const block = serviceBlock(compose, serviceName); + assert.doesNotMatch( + block, + /chatgpt-web-codex/, + `${serviceName} must not reference the chatgpt-web-codex-browser network — it has no ` + + "legitimate reason to reach the CDP proxy sidecar" + ); + } +}); diff --git a/tests/unit/vnc-svc-de-run-cdp-bridge-gate-13679.test.ts b/tests/unit/vnc-svc-de-run-cdp-bridge-gate-13679.test.ts new file mode 100644 index 0000000000..d3268d1e60 --- /dev/null +++ b/tests/unit/vnc-svc-de-run-cdp-bridge-gate-13679.test.ts @@ -0,0 +1,44 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import path from "node:path"; + +const REPO_ROOT = path.resolve(import.meta.dirname, "../.."); +const SVC_DE_RUN = path.join(REPO_ROOT, "docker/vnc-browser/chromium/svc-de-run"); + +// #13679 item #10 (residual gap): svc-de-run unconditionally spawns +// cdp-bridge.py on every container start, even though CDP_BRIDGE_TOKEN may be +// unset (in which case cdp-bridge.py's has_valid_token() fails closed for +// every caller anyway, per #12571) — the process still binds 0.0.0.0:9223 +// and accepts+drops connections for no reason. Gate the spawn behind the +// token actually being configured so an unconfigured container does not run +// a debug listener at all. + +function readScript(): string { + return fs.readFileSync(SVC_DE_RUN, "utf8"); +} + +test("svc-de-run only starts the CDP bridge when CDP_BRIDGE_TOKEN is configured", () => { + const script = readScript(); + const lines = script.split("\n"); + const launchIndexes = lines + .map((line, index) => ({ line, index })) + .filter(({ line }) => !/^\s*#/.test(line) && line.includes("cdp-bridge.py")) + .map(({ index }) => index); + + assert.ok( + launchIndexes.length >= 2, + "expected the wayland and X11 branches to both still launch cdp-bridge.py" + ); + + const GUARD_WINDOW = 5; + for (const index of launchIndexes) { + const precedingLines = lines.slice(Math.max(0, index - GUARD_WINDOW), index).join("\n"); + assert.match( + precedingLines, + /if\s*\[\s*-n\s*"\$\{CDP_BRIDGE_TOKEN/, + `svc-de-run must only launch cdp-bridge.py inside an "if [ -n \\"\${CDP_BRIDGE_TOKEN...` + + `\\" ]" guard, but found an unconditional launch at line ${index + 1}: ${lines[index].trim()}` + ); + } +});