fix(security): harden codex app-server transport (#11205 post-merge review) (#11281)

TDD: 4 red-to-green tests reproducing the two review findings + 6 new cases; 30/30 in tests/unit/codex-app-server.test.ts; typecheck/eslint/env-doc-sync/file-size clean. Merged --admin over the inherited 2026-08-23 base-red cluster (#9985): the remaining reds (CLI catalog/registry tests, @testing-library allowlist) are proven tip failures unrelated to this diff — shard logs show only CLI-cluster failures, and this PR itself drains the mutation-test-coverage red (stryker registration for #11267's test). Behavior change for app-server deployments is documented in the PR body (sandbox default + binding refusal).
This commit is contained in:
Diego Rodrigues de Sa e Souza
2026-08-23 16:49:19 -03:00
committed by GitHub
parent 527da6565d
commit bdf63d2171
10 changed files with 418 additions and 61 deletions

View File

@@ -1324,8 +1324,14 @@ CURSOR_USER_AGENT="Cursor/3.4"
# Approval policy passed to the app-server turn (e.g. never, on-request).
# OMNIROUTE_CODEX_APPSERVER_APPROVAL=never
# Sandbox policy passed to the app-server turn (e.g. read-only,
# workspace-write, danger-full-access).
# workspace-write, danger-full-access). When unset, the executor defaults to
# "workspace-write" (hardened; used to be "danger-full-access").
# OMNIROUTE_CODEX_APPSERVER_SANDBOX=read-only
# Auto-approve the app-server's own approval prompts (command/file/permission
# execution on the host). Defaults to OFF — prompts are auto-denied. Set to
# true/1/yes only when you trust the deployment to run codex-decided host
# commands. Per-connection override: providerSpecificData.codexAppServerAutoApprove.
# OMNIROUTE_CODEX_APPSERVER_AUTO_APPROVE=false
# ═══════════════════════════════════════════════════════════════════════════════
# 13. CLI FINGERPRINT COMPATIBILITY (Anti-Detection)

View File

@@ -0,0 +1 @@
- Hardened the Codex app-server transport after the post-merge security review of #11205: approval prompts from the app-server (its own command/file/permission execution — not the harness tool passthrough) are now auto-denied by default, with opt-in auto-approval via `providerSpecificData.codexAppServerAutoApprove` / `OMNIROUTE_CODEX_APPSERVER_AUTO_APPROVE`; the default codex sandbox changed from `danger-full-access` to `workspace-write` (override per connection or env); env-sourced capability tokens are now only sent to env-sourced URLs or operator-local hosts (loopback/RFC1918/link-local/ULA/localhost/single-label LAN names/*.local/*.ts.net/*.internal), so a connection's providerSpecificData URL can no longer exfiltrate the operator's env token; and the `/readyz` health probe no longer follows redirects while carrying the bearer token.

View File

@@ -433,7 +433,7 @@
"src/shared/components/analytics/charts.tsx": 1346,
"src/shared/services/cliRuntime.ts": 1459,
"src/sse/handlers/chat.ts": 2493,
"src/sse/services/auth.ts": 3337,
"src/sse/services/auth.ts": 3344,
"_rebaseline_2026_08_23_11186_synced_inventory_routing": "PR #11186 (pacocartones) own growth: src/sse/services/auth.ts 3260->3337 (+77, loadAdvertisedModelsForSelfHostedConnections + the modelNotAdvertised candidate-filter predicate — pins chat routing to the connection whose synced inventory actually advertises the model, fixing spurious model-not-found on multi-host self-hosted setups; at the existing credential-selection chokepoint, not extractable without splitting the selection flow). Covered by tests/unit/chat-routing-synced-inventory-11089.test.ts. Owner pre-authorized baseline bumps 2026-08-22.",
"tests/unit/account-fallback-service.test.ts": 2044,
"tests/unit/provider-validation-specialty.test.ts": 3880,
@@ -464,14 +464,15 @@
"src/app/(dashboard)/dashboard/providers/[id]/providerPageHelpers.ts": 1014,
"open-sse/config/imageRegistry.ts": 1034,
"src/sse/handlers/chatHelpers.ts": 1019,
"src/shared/middleware/chatBodyAdmission.ts": 1009,
"src/shared/middleware/chatBodyAdmission.ts": 1118,
"_rebaseline_2026_08_22_11020_sigterm_drain": "PR #11020 (RaviTharuma) own growth: chatBodyAdmission.ts 1005->1009 (+4, heavyweight admission leases now increment the SIGTERM drain counter and releaseChatAdmissionWhenDone holds it for the SSE lifetime — closes #11015; +4 are the lease/drain wiring lines at the existing admission chokepoint). Covered by tests/unit/chat-body-admission.test.ts heavyweight-lease cases. Owner pre-authorized baseline bumps 2026-08-22.",
"_rebaseline_2026_08_20_10668_tabitoken_gateway": "#10668 (yawar-aquil) own catalog growth: src/shared/constants/providers/apikey/gateways.ts 1268->1283 (+15, entirely this PR diff -- one new tabitoken gateway entry, data lines only; base moved from 1255 to 1268 via other merges since the PR forked). Not combination drift: reproducible on the PR branch alone, so the WS5.5 release-captain rule does not apply. Extraction is not available -- the file is pure data (own header: \"Pure data; merged by apikey/index.ts via spread\") and already split into 6 family files under apikey/. Same precedent as _rebaseline_2026_08_14_imagetotext_servicekinds (#10275/#10291, gateways.ts 1250->1255, data lines only) and _rebaseline_2026_08_11_v3850_merge_storm_provider_registry (owner-authorized for this same file).",
"open-sse/executors/commandCode.ts": 1059,
"_rebaseline_2026_08_21_10859_vision_bridge_catalog": "#10859 own growth (Vision Bridge fixes #10808/#10809): src/lib/modelCapabilities.ts 1006->1016 (+10, cmd/gpt-5.3-codex* text-only capability resolution) and open-sse/executors/commandCode.ts 988->1023 (+35, Command Code wire-model normalization for bare ids + reasoning field fallback for opencode-routed gateways). Cohesive bug fixes at the existing capability-resolution / executor chokepoints; not extractable mid-fix. Covered by tests/unit/model-capabilities-command-code-codex-textonly-10703.test.ts, tests/unit/command-code-vision.test.ts, tests/unit/opencode-mimo-reasoning-details-nonstream.test.ts. Pushed directly to release (own-session miss: the original rebaseline was made in a throwaway validation worktree and never landed on the PR branch or the release before merge).",
"_rebaseline_2026_08_21_10907_sticky_pin_clear": "#10907 own growth: open-sse/executors/commandCode.ts 1023->1038 (+15, effort-suffix sanitization threading for the sticky-pin-clear fix). Cohesive change at the existing executor chokepoint. Covered by tests/unit/command-code-executor.test.ts.",
"_rebaseline_2026_08_21_10986_reasoning_only_content": "#10986 own growth: open-sse/executors/commandCode.ts 1038->1059 (+21, reasoning-only content fallback — when upstream emits only reasoning-delta events and never a text-delta, surface the reasoning text as message.content in createJsonResponse and emit a synthetic content delta in createStreamResponse). Cohesive bug fix at the existing executor chokepoint (mirrors precedent style of #10907/#10859). Covered by tests/unit/command-code-executor.test.ts (2 new cases: non-stream + streaming).",
"_rebaseline_2026_08_21_11069_m365_har_import": "#11069 own growth: AddApiKeyModal.tsx 1073->1080 (+7 = Import .har file button for the copilot-m365-web credential modal — M365 is the only provider whose credential (access_token+chathubPath) must be extracted from a DevTools HAR WebSocket URL, added as a new modal affordance). Cohesive UI at the existing modal chokepoint; not extractable. Covered by tests/unit/m365-har-import*.test.ts."
"_rebaseline_2026_08_21_11069_m365_har_import": "#11069 own growth: AddApiKeyModal.tsx 1073->1080 (+7 = Import .har file button for the copilot-m365-web credential modal — M365 is the only provider whose credential (access_token+chathubPath) must be extracted from a DevTools HAR WebSocket URL, added as a new modal affordance). Cohesive UI at the existing modal chokepoint; not extractable. Covered by tests/unit/m365-har-import*.test.ts.",
"_rebaseline_2026_08_23_tip_drift_post_batch0823": "Tip drift after the 2026-08-23 merge wave: chatBodyAdmission.ts 1009->1118 (+109, gate count incl. +1) and auth.ts 3337->3344 (+7), both grown by merges already on origin/release/v3.8.50 (verified identical on the pristine tip) — not by the codex-appserver-hardening PR that carries this bump. Owner pre-authorized baseline bumps 2026-08-22."
},
"_rebaseline_base_2026_08_10_proxyfetch": "Base-red fix (green-prs sweep, issue #9985): open-sse/utils/proxyFetch.ts 1207 > cap 1000 — new proxied-TLS fetch helper introduced by the Fal reference-image work. Owner-authorized quick rebaseline to green; structural slim tracked for v3.9.0.",
"_rebaseline_2026_07_27_v3849_train2": "Merge-train 2 (7 PRs) — owner-approved 2026-07-27. Single entry: chatCore.ts 4955->5006 (#8595, Responses multi-turn image compaction before the context hard-reject). Genuine irreducible growth at the existing compaction chokepoint in handleChatCore — the PR adds a last-resort retry against the concrete budget plus the estimateFinalInputTokens helper, both wired at the pre-existing call site rather than a new branch. Covered by tests/unit/8560-responses-image-compaction.test.ts (4 tests).",

View File

@@ -742,7 +742,8 @@ REQUEST_TIMEOUT_MS (global override)
| `OMNIROUTE_CODEX_APPSERVER_WS_TOKEN_FILE` | _(unset)_ | Path to a file holding the app-server capability token (from `codex app-server --ws-token-file`). Used when `OMNIROUTE_CODEX_APPSERVER_WS_TOKEN` is unset. Per-connection override: `providerSpecificData.codexAppServerTokenFile`. |
| `OMNIROUTE_CODEX_APPSERVER_CWD` | `/tmp` | Working directory the app-server turn runs in. Per-connection override: `providerSpecificData.codexAppServerCwd`. |
| `OMNIROUTE_CODEX_APPSERVER_APPROVAL` | _(unset)_ | Approval policy passed to the app-server turn (e.g. `never`, `on-request`). Per-connection override: `providerSpecificData.codexAppServerApprovalPolicy`. |
| `OMNIROUTE_CODEX_APPSERVER_SANDBOX` | _(unset)_ | Sandbox policy passed to the app-server turn (e.g. `read-only`, `workspace-write`, `danger-full-access`). Per-connection override: `providerSpecificData.codexAppServerSandbox`. |
| `OMNIROUTE_CODEX_APPSERVER_SANDBOX` | _(unset)_ | Sandbox policy passed to the app-server turn (e.g. `read-only`, `workspace-write`, `danger-full-access`). When unset the executor defaults to `workspace-write` (hardened; previously `danger-full-access`). Per-connection override: `providerSpecificData.codexAppServerSandbox`. |
| `OMNIROUTE_CODEX_APPSERVER_AUTO_APPROVE` | `false` | Auto-approve the app-server's own approval prompts (command/file/permission execution on the host). Off by default — prompts are auto-denied; harness tool calls are unaffected (they travel the separate `item/tool/call` passthrough). Accepts `true`/`1`/`yes`. Per-connection override: `providerSpecificData.codexAppServerAutoApprove`. |
| `FETCH_HEADERS_TIMEOUT_MS` | = `FETCH_TIMEOUT_MS` | Time to receive response headers. |
| `OMNIROUTE_DIRECT_HEADERS_TIMEOUT_MS` | `30000` (30s) | Maximum response-start wait (ms) for each direct no-proxy attempt. A timeout retries once on a fresh socket; set `0` to disable the bound and retain the previous behavior. |
| `FETCH_BODY_TIMEOUT_MS` | = `FETCH_TIMEOUT_MS` | Time to receive the full response body. |

View File

@@ -11,7 +11,7 @@ import {
CodexAppServerClient,
type CodexAppServerClientOptions,
} from "./codex/appServerClient.ts";
import { resolveAppServerConfig, type CodexAppServerConfig } from "./codex/appServerConfig.ts";
import { resolveAppServerConfig, resolveThreadStartPolicy, type CodexAppServerConfig } from "./codex/appServerConfig.ts";
import {
translateNotification,
translateToolCall,
@@ -232,13 +232,20 @@ export class CodexAppServerExecutor extends BaseExecutor {
"codex_app_server_unconfigured"
);
}
// Turn policy (hardened after the #11205 security review): approvalPolicy
// "never", sandbox "workspace-write", autoApprove off unless the operator
// opted in — see resolveThreadStartPolicy.
const policy = resolveThreadStartPolicy(config, psd);
const promptText = extractPromptText(input.body);
const effort = extractEffort(input.body);
const toolMaps = buildAppServerToolMaps(input.body);
const hasTools = toolMaps.specs.length > 0;
const events = new AsyncEventQueue<AdapterEvent>();
const client = new CodexAppServerClient(this.clientOptions);
const client = new CodexAppServerClient({
...this.clientOptions,
autoApproveApprovals: policy.autoApprove,
});
const run = async () => {
let terminated = false;
@@ -284,17 +291,16 @@ export class CodexAppServerExecutor extends BaseExecutor {
cwd: config.cwd,
// OmniRoute is a router: the HARNESS that consumes OmniRoute owns tool
// execution and policy. codex must therefore NEVER block a turn waiting
// on its own interactive approval, and its own sandbox must not gate the
// model — the harness decides what actually runs. So we pair
// approvalPolicy:"never" (non-interactive; codex never prompts) with
// sandbox:"danger-full-access" (codex's own sandbox imposes no
// restriction), mirroring codexInstructions.ts:50 ("never +
// danger-full-access = take advantage of it"). Any server→client
// approval request that still arrives is auto-APPROVED by the client
// (see CodexAppServerClient), never denied — denial would sabotage the
// harness's tool calls. Callers can override both via providerSpecificData.
approvalPolicy: config.approvalPolicy ?? "never",
sandbox: config.sandbox ?? "danger-full-access",
// on its own interactive approval (approvalPolicy "never"). Its own
// sandbox defaults to "workspace-write" (hardened after the #11205
// security review; WAS "danger-full-access") so codex-decided host
// commands are confined to the turn's cwd tree — widen only via an
// explicit operator override. Server→client approval prompts (codex's
// own command/file/permission requests, NOT the harness tool
// passthrough) are auto-DENIED by the client unless the operator opted
// into auto-approval (see CodexAppServerClient).
approvalPolicy: policy.approvalPolicy,
sandbox: policy.sandbox,
// INBOUND harness tools → codex. The client tells the app-server which
// function tools are available for the thread via the `dynamicTools`
// field on thread/start (a DynamicToolSpec[] under the experimental API,

View File

@@ -12,9 +12,10 @@
* command / patch / permission. OmniRoute is a ROUTER — the harness that consumes
* it owns tool execution and policy — so codex must never stall a turn on its own
* interactive approval. Every inbound ServerRequest is always answered: approval
* prompts are auto-APPROVED (so the model's agentic tool calls proceed; the harness
* decides what really runs), and anything else we can't service gets a JSON-RPC
* error so the id is always settled and the turn never hangs.
* prompts are auto-DENIED by default (they gate codex's OWN host execution, not
* the harness's tools; auto-approval is an explicit operator opt-in — hardening
* after the #11205 security review), and anything else we can't service gets a
* JSON-RPC error so the id is always settled and the turn never hangs.
*/
// wreq-js WebSocket surface (mirrors the private type in codex.ts:71-77).
@@ -37,7 +38,12 @@ interface PendingReq {
}
// The set of ServerRequest methods that are approval prompts (see PROTOCOL-DIGEST
// "Server -> client REQUESTS"). All of these get an auto-denial decision.
// "Server -> client REQUESTS"). All of these get an auto-DENIAL decision unless
// the operator explicitly opted into auto-approval (hardening after the #11205
// security review): these prompts gate codex's OWN command/file/permission
// execution on the host, NOT the harness's dynamic tools (those travel the
// separate item/tool/call passthrough), so denying by default never sabotages
// harness tool calls — it closes a prompt-injection → host-execution path.
const APPROVAL_REQUEST_METHODS = new Set<string>([
"item/commandExecution/requestApproval",
"item/fileChange/requestApproval",
@@ -47,12 +53,20 @@ const APPROVAL_REQUEST_METHODS = new Set<string>([
]);
const ROUTER_APPROVAL_NOTE = "router: harness-controlled execution";
const ROUTER_DENIAL_NOTE =
"router: denied by default (set codexAppServerAutoApprove to opt in)";
export interface CodexAppServerClientOptions {
/** Transport factory. Defaults to the shared wreq-js websocket() when omitted. */
websocketFn?: CodexAppServerWebsocketFn | null;
/** Default per-request timeout (ms). */
defaultTimeoutMs?: number;
/**
* Auto-APPROVE codex's own approval prompts (command/file/permission).
* Defaults to FALSE — prompts are auto-denied. Enable only when the operator
* trusts the app-server deployment to run codex-decided host commands.
*/
autoApproveApprovals?: boolean;
}
/**
@@ -89,11 +103,13 @@ export class CodexAppServerClient {
private toolCallHandler: CodexAppServerToolCallHandler | null = null;
private readonly websocketFn: CodexAppServerWebsocketFn | null;
private readonly defaultTimeoutMs: number;
private readonly autoApproveApprovals: boolean;
private closed = false;
constructor(options: CodexAppServerClientOptions = {}) {
this.websocketFn = options.websocketFn ?? null;
this.defaultTimeoutMs = options.defaultTimeoutMs ?? 120_000;
this.autoApproveApprovals = options.autoApproveApprovals === true;
}
/**
@@ -232,22 +248,27 @@ export class CodexAppServerClient {
}
/**
* Always answer an inbound ServerRequest so its id is settled. Approval prompts
* are auto-APPROVED (OmniRoute is a router; the harness that consumes it owns
* execution policy, so codex's own approval must not block the turn). Anything
* we cannot service gets a JSON-RPC error so the id is still settled.
* Always answer an inbound ServerRequest so its id is settled. Approval
* prompts are auto-DENIED unless the operator opted into auto-approval
* (hardening after the #11205 security review): they gate codex's OWN host
* command/file execution, not the harness's tools. Anything we cannot
* service gets a JSON-RPC error so the id is still settled.
*/
private answerServerRequest(id: number, method: string): void {
if (!this.ws || this.closed) return;
if (APPROVAL_REQUEST_METHODS.has(method)) {
// ReviewDecision "approved" — let the model's agentic action proceed. The
// harness downstream of OmniRoute is the real gate. Note the note field is
// advisory; the decision string is what codex acts on.
// ReviewDecision — "denied" by default; "approved" only with the explicit
// operator opt-in. The note field is advisory; the decision string is
// what codex acts on.
const approved = this.autoApproveApprovals;
this.ws.send(
JSON.stringify({
jsonrpc: "2.0",
id,
result: { decision: "approved", note: ROUTER_APPROVAL_NOTE },
result: {
decision: approved ? "approved" : "denied",
note: approved ? ROUTER_APPROVAL_NOTE : ROUTER_DENIAL_NOTE,
},
})
);
return;

View File

@@ -22,15 +22,19 @@ export interface CodexAppServerConfig {
*/
approvalPolicy?: string;
/**
* Optional codex sandbox override (SandboxMode). Defaults to "danger-full-access"
* in the executor so codex's own sandbox does not gate the model; the harness is
* the real gate. Callers may tighten this per request via providerSpecificData.
* Optional codex sandbox override (SandboxMode). Defaults to "workspace-write"
* in the executor (hardened after the #11205 security review; WAS
* "danger-full-access") so codex's own command/file execution is confined to
* the turn's cwd tree. Widen per connection via providerSpecificData or env.
*/
sandbox?: string;
}
type ProviderSpecificData = Record<string, unknown> | null | undefined;
/** Where a resolved value came from — the SSRF binding below keys off this. */
type ConfigSource = "psd" | "env";
function firstString(...values: unknown[]): string | null {
for (const value of values) {
if (typeof value === "string" && value.trim().length > 0) return value.trim();
@@ -38,23 +42,47 @@ function firstString(...values: unknown[]): string | null {
return null;
}
function firstStringWithSource(
psdValue: unknown,
envValue: unknown
): { value: string; source: ConfigSource } | null {
if (typeof psdValue === "string" && psdValue.trim().length > 0) {
return { value: psdValue.trim(), source: "psd" };
}
if (typeof envValue === "string" && envValue.trim().length > 0) {
return { value: envValue.trim(), source: "env" };
}
return null;
}
/**
* Read the capability token, preferring an inline token, then a token FILE path.
* The token file (produced by `codex app-server --ws-token-file <path>`) holds the
* same hex string that is presented as the bearer token.
* same hex string that is presented as the bearer token. The source of the value
* (psd vs env) is tracked for the credential/URL binding rule.
*/
function resolveToken(psd: ProviderSpecificData): string | null {
const inline = firstString(
psd?.codexAppServerToken,
process.env.OMNIROUTE_CODEX_APPSERVER_WS_TOKEN
);
if (inline) return inline;
function resolveTokenWithSource(
psd: ProviderSpecificData
): { value: string; source: ConfigSource } | null {
const inlinePsd = firstString(psd?.codexAppServerToken);
if (inlinePsd) return { value: inlinePsd, source: "psd" };
const inlineEnv = firstString(process.env.OMNIROUTE_CODEX_APPSERVER_WS_TOKEN);
if (inlineEnv) return { value: inlineEnv, source: "env" };
const tokenFile = firstString(
psd?.codexAppServerTokenFile,
process.env.OMNIROUTE_CODEX_APPSERVER_WS_TOKEN_FILE
);
if (!tokenFile) return null;
const filePsd = firstString(psd?.codexAppServerTokenFile);
if (filePsd) {
const contents = readTokenFile(filePsd);
if (contents) return { value: contents, source: "psd" };
}
const fileEnv = firstString(process.env.OMNIROUTE_CODEX_APPSERVER_WS_TOKEN_FILE);
if (fileEnv) {
const contents = readTokenFile(fileEnv);
if (contents) return { value: contents, source: "env" };
}
return null;
}
function readTokenFile(tokenFile: string): string | null {
try {
const contents = readFileSync(tokenFile, "utf8").trim();
return contents.length > 0 ? contents : null;
@@ -67,18 +95,80 @@ function isWebSocketUrl(url: string): boolean {
return url.startsWith("ws://") || url.startsWith("wss://");
}
function urlHostname(url: string): string | null {
try {
return new URL(url).hostname || null;
} catch {
return null;
}
}
/**
* Is this hostname inside the operator's own network? Used by the
* credential/URL binding rule: an ENV-sourced capability token (the operator's
* shared secret, not visible to whoever wrote a connection's
* providerSpecificData) may only be sent to env-configured URLs or to
* operator-local hosts. Literal addresses only — no DNS resolution, so a
* public hostname can never smuggle an env token out via DNS. Single-label
* names (`ts-egress`) resolve via the operator's own hosts/mDNS and count as
* local; dotted names must carry a known-local suffix.
*/
export function isLocalAppServerHost(hostname: string): boolean {
const h = hostname
.trim()
.toLowerCase()
.replace(/^\[|\]$/g, "");
if (!h) return false;
if (h === "localhost" || h.endsWith(".localhost")) return true;
if (h.endsWith(".local") || h.endsWith(".ts.net") || h.endsWith(".internal")) return true;
if (h.includes(":")) {
// IPv6: loopback, ULA (fc00::/7), link-local (fe80::/10)
if (h === "::1") return true;
return /^f[cd]/.test(h) || /^fe[89ab]/.test(h);
}
const m = /^(\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})$/.exec(h);
if (m) {
const a = Number(m[1]);
const b = Number(m[2]);
if (a === 10 || a === 127) return true;
if (a === 172 && b >= 16 && b <= 31) return true;
if (a === 192 && b === 168) return true;
if (a === 169 && b === 254) return true;
return false;
}
// single-label hostname (no dots): LAN/hosts-file/mDNS name
if (!h.includes(".")) return true;
return false;
}
/**
* Resolve the app-server connection config from providerSpecificData with env
* fallbacks. Returns `null` when not fully configured (URL + token both required)
* so the gating predicate `isCodexAppServerRequired` stays false and Codex falls
* back to its other transports.
*
* CREDENTIAL/URL BINDING (hardening after the #11205 security review): an
* env-sourced token is the operator's shared secret. It is only ever paired
* with (a) an env-sourced URL, or (b) an operator-local host
* (isLocalAppServerHost). A providerSpecificData URL pointing at an outside
* host combined with an env token is refused (returns null) — otherwise anyone
* able to write a connection could exfiltrate the env credential. A
* psd-sourced token may go anywhere: whoever wrote the psd already knows it.
*/
export function resolveAppServerConfig(psd: ProviderSpecificData): CodexAppServerConfig | null {
const url = firstString(psd?.codexAppServerUrl, process.env.OMNIROUTE_CODEX_APPSERVER_WS);
if (!url || !isWebSocketUrl(url)) return null;
const urlRes = firstStringWithSource(psd?.codexAppServerUrl, process.env.OMNIROUTE_CODEX_APPSERVER_WS);
if (!urlRes || !isWebSocketUrl(urlRes.value)) return null;
const token = resolveToken(psd);
if (!token) return null;
const tokenRes = resolveTokenWithSource(psd);
if (!tokenRes) return null;
if (tokenRes.source === "env" && urlRes.source === "psd") {
const host = urlHostname(urlRes.value);
if (!host || !isLocalAppServerHost(host)) return null;
}
const url = urlRes.value;
const token = tokenRes.value;
const cwd =
firstString(psd?.codexAppServerCwd, process.env.OMNIROUTE_CODEX_APPSERVER_CWD) ?? "/tmp";
@@ -92,3 +182,35 @@ export function resolveAppServerConfig(psd: ProviderSpecificData): CodexAppServe
return { url, token, cwd, ...(approvalPolicy ? { approvalPolicy } : {}), ...(sandbox ? { sandbox } : {}) };
}
/**
* The turn/start policy triple for a resolved config (hardening after the
* #11205 security review):
* - approvalPolicy defaults to "never": codex must not block a router turn on
* its own interactive approval (unchanged).
* - sandbox defaults to "workspace-write" (WAS "danger-full-access"): codex's
* own command/file execution is confined to the turn's cwd tree unless the
* operator explicitly widens it (providerSpecificData.codexAppServerSandbox /
* OMNIROUTE_CODEX_APPSERVER_SANDBOX). With "never" + a permissive sandbox,
* codex would run model-decided commands on the host with no gate at all.
* - autoApprove defaults to false: server→client approval prompts are answered
* "denied" unless the operator opts in via
* providerSpecificData.codexAppServerAutoApprove ("true"/"1"/"yes") or
* OMNIROUTE_CODEX_APPSERVER_AUTO_APPROVE. Harness tool calls are unaffected —
* they travel the separate item/tool/call passthrough.
*/
export function resolveThreadStartPolicy(
config: CodexAppServerConfig,
psd: ProviderSpecificData
): { approvalPolicy: string; sandbox: string; autoApprove: boolean } {
const raw = firstString(
psd?.codexAppServerAutoApprove,
process.env.OMNIROUTE_CODEX_APPSERVER_AUTO_APPROVE
);
const autoApprove = raw === "true" || raw === "1" || raw === "yes";
return {
approvalPolicy: config.approvalPolicy ?? "never",
sandbox: config.sandbox ?? "workspace-write",
autoApprove,
};
}

View File

@@ -61,7 +61,11 @@ export async function testCodexAppServerConnection(
);
const config = resolveAppServerConfig(psd);
if (!config) {
const error = "Codex app-server transport is not configured (missing url or token)";
// Also reached when the credential/URL binding refused (env token + remote
// psd URL) — the resolve deliberately returns null there so the token can
// never leave the operator's network (see appServerConfig.ts).
const error =
"Codex app-server transport is not configured (missing url/token, or the env-token/remote-URL binding was refused)";
return {
valid: false,
error,
@@ -81,6 +85,10 @@ export async function testCodexAppServerConnection(
method: "GET",
headers: { Authorization: `Bearer ${config.token}` },
signal: controller.signal,
// Never follow redirects carrying the bearer token (SSRF hardening after
// the #11205 security review): a 30x to an outside host would exfiltrate
// the capability token. A redirect response is simply "not ready".
redirect: "manual",
});
if (res.status !== 200) {
const error = `Codex app-server not ready (${readyzUrl} → HTTP ${res.status})`;

View File

@@ -89,6 +89,7 @@
"tests/unit/auth-terminal-status.test.ts",
"tests/unit/authz/discovery-routes-local-only.test.ts",
"tests/unit/authz/oauth-autoimport-local-only.test.ts",
"tests/unit/quota-exhaustion-cutoff-opencode.test.ts",
"tests/unit/authz/route-guard-local-prefix.test.ts",
"tests/unit/authz/route-guard-skills-collect.test.ts",
"tests/unit/authz/route-guard-version-get-exemption.test.ts",

View File

@@ -213,9 +213,9 @@ test("mapUsage: converts snake_case token counts", () => {
assert.equal(mapUsage(undefined), undefined);
});
// ── Client: stall-guard auto-approval ───────────────────────────────────────
// ── Client: approval stall-guard (deny-by-default, opt-in approve) ─────────
test("CodexAppServerClient: server approval request is auto-approved", async () => {
test("CodexAppServerClient: server approval request is auto-DENIED by default (#11205 hardening)", async () => {
const ctrl = makeFakeSocket();
const { fn } = fakeTransport(ctrl);
const client = new CodexAppServerClient({ websocketFn: fn });
@@ -234,12 +234,30 @@ test("CodexAppServerClient: server approval request is auto-approved", async ()
const reply = ctrl.sent.find((f) => f.id === 99);
assert.ok(reply, "client must reply to the server approval request");
// OmniRoute is a router: approvals are auto-APPROVED so the model's agentic
// tool calls proceed; the harness downstream is the real execution gate.
assert.equal(
(reply!.result as Record<string, unknown>).decision,
"approved"
);
// Security contract (post-#11205 review): codex's OWN command/file/permission
// executions are denied by default — approval prompts are NOT the harness
// tool-call passthrough (that path is item/tool/call, handled separately), so
// denying never sabotages harness tools. Blanket auto-approve + a permissive
// sandbox is a confused-deputy for prompt-injected turns.
assert.equal((reply!.result as Record<string, unknown>).decision, "denied");
});
test("CodexAppServerClient: approval request is auto-approved only with explicit opt-in", async () => {
const ctrl = makeFakeSocket();
const { fn } = fakeTransport(ctrl);
const client = new CodexAppServerClient({ websocketFn: fn, autoApproveApprovals: true });
await client.connect("ws://x", "tok");
ctrl.emit({
jsonrpc: "2.0",
id: 100,
method: "item/fileChange/requestApproval",
params: { changes: [] },
});
const reply = ctrl.sent.find((f) => f.id === 100);
assert.ok(reply, "client must reply to the server approval request");
assert.equal((reply!.result as Record<string, unknown>).decision, "approved");
});
test("CodexAppServerClient: non-approval server request gets a JSON-RPC error", async () => {
@@ -332,11 +350,13 @@ test("CodexAppServerExecutor: streaming turn emits initialize → thread/start
assert.deepEqual(lifecycle, ["initialize", "thread/start", "turn/start"]);
// thread/start carried the router defaults: approvalPolicy:"never" (codex
// never blocks on its own approval) + sandbox:"danger-full-access" (codex's
// own sandbox does not gate the model; the harness is the real execution gate).
// never blocks on its own approval) + sandbox:"workspace-write" — hardened
// default post-#11205 security review (was "danger-full-access"): codex's own
// sandbox now confines writes to the turn's cwd tree unless the operator
// explicitly opts back into a wider sandbox via providerSpecificData/env.
const threadStart = sent.find((f) => f.method === "thread/start");
assert.equal((threadStart!.params as Record<string, unknown>).approvalPolicy, "never");
assert.equal((threadStart!.params as Record<string, unknown>).sandbox, "danger-full-access");
assert.equal((threadStart!.params as Record<string, unknown>).sandbox, "workspace-write");
// turn/start carried the text input with text_elements:[]
const turnStart = sent.find((f) => f.method === "turn/start");
@@ -700,3 +720,173 @@ test("probeCodexAppServerAuth: no transport → unknown (does not throw)", async
assert.equal(status.state, "unknown");
});
// ── Security hardening (#11205 post-merge review) ───────────────────────────
// Two findings from the automated push review on the original #11205 merge:
// (1) the readyz health probe sent the bearer token to any URL a connection
// config pointed at, following redirects (SSRF / credential exfil);
// (2) env-sourced credentials were happily paired with a
// providerSpecificData-sourced URL, so anyone able to write a connection
// could harvest the operator's env token.
// The binding rule: env-sourced tokens are only sent to env-sourced URLs or to
// operator-local hosts (loopback / RFC1918 / link-local / ULA / localhost /
// single-label LAN names / *.local / *.ts.net / *.internal). A psd-sourced
// token may go anywhere — whoever wrote the psd already knows it.
function withEnv<T>(vars: Record<string, string | undefined>, fn: () => T): T {
const prev: Record<string, string | undefined> = {};
for (const k of Object.keys(vars)) {
prev[k] = process.env[k];
if (vars[k] === undefined) delete process.env[k];
else process.env[k] = vars[k];
}
try {
return fn();
} finally {
for (const k of Object.keys(vars)) {
if (prev[k] === undefined) delete process.env[k];
else process.env[k] = prev[k];
}
}
}
const BINDING_ENV_KEYS = {
OMNIROUTE_CODEX_APPSERVER_WS: undefined,
OMNIROUTE_CODEX_APPSERVER_WS_TOKEN: "env-token-hex",
OMNIROUTE_CODEX_APPSERVER_WS_TOKEN_FILE: undefined,
} as const;
test("resolveAppServerConfig: refuses env token → remote psd URL (SSRF binding)", () => {
withEnv({ ...BINDING_ENV_KEYS }, () => {
// attacker/lower-priv connection config points the URL at an outside host;
// the env token must NOT be attached → unconfigured (null), feature off.
assert.equal(
resolveAppServerConfig({
codexTransport: "app-server",
codexAppServerUrl: "wss://evil.example.com:8443",
}),
null
);
// dotted hostnames are not local even when they look benign
assert.equal(
resolveAppServerConfig({
codexTransport: "app-server",
codexAppServerUrl: "ws://appserver.evil-corp.io:1456",
}),
null
);
});
});
test("resolveAppServerConfig: env token allowed to operator-local psd URLs", () => {
withEnv({ ...BINDING_ENV_KEYS }, () => {
const localUrls = [
"ws://127.0.0.1:1456",
"ws://localhost:1456",
"ws://[::1]:1456",
"ws://10.0.0.5:1456",
"ws://172.16.3.4:1456",
"ws://192.168.0.15:1456",
"ws://169.254.1.1:1456",
"ws://ts-egress:1456", // single-label LAN/hosts-file name
"ws://codex.local:1456",
"ws://node1.ts.net:1456",
"ws://sidecar.internal:1456",
];
for (const url of localUrls) {
const cfg = resolveAppServerConfig({ codexTransport: "app-server", codexAppServerUrl: url });
assert.ok(cfg, `expected env token to bind to local URL ${url}`);
assert.equal(cfg!.token, "env-token-hex");
}
});
});
test("resolveAppServerConfig: psd-sourced token may pair with any psd URL", () => {
withEnv(
{
OMNIROUTE_CODEX_APPSERVER_WS: undefined,
OMNIROUTE_CODEX_APPSERVER_WS_TOKEN: undefined,
OMNIROUTE_CODEX_APPSERVER_WS_TOKEN_FILE: undefined,
},
() => {
const cfg = resolveAppServerConfig({
codexTransport: "app-server",
codexAppServerUrl: "wss://codex.remote.example.com:443",
codexAppServerToken: "psd-token",
});
assert.ok(cfg, "psd token + psd URL is self-consistent, allowed");
assert.equal(cfg!.token, "psd-token");
}
);
});
test("resolveAppServerConfig: env URL + env token pairs regardless of host", () => {
withEnv(
{
OMNIROUTE_CODEX_APPSERVER_WS: "wss://codex-remote.example.com:8443",
OMNIROUTE_CODEX_APPSERVER_WS_TOKEN: "env-token-hex",
OMNIROUTE_CODEX_APPSERVER_WS_TOKEN_FILE: undefined,
},
() => {
const cfg = resolveAppServerConfig({ codexTransport: "app-server" });
assert.ok(cfg, "operator's own env pair is self-consistent, allowed");
assert.equal(cfg!.url, "wss://codex-remote.example.com:8443");
}
);
});
// ── Health probe: redirect pinning + binding inheritance ────────────────────
test("testCodexAppServerConnection: readyz probe pins redirects (no token leak via 30x)", async () => {
const { testCodexAppServerConnection } = await import(
"../../src/app/api/providers/[id]/test/codexAppServerHealth.ts"
);
const originalFetch = globalThis.fetch;
const seen: Array<{ url: string; init?: RequestInit }> = [];
globalThis.fetch = (async (url: unknown, init?: RequestInit) => {
seen.push({ url: String(url), init });
return new Response("not ready", { status: 503 });
}) as typeof fetch;
try {
const result = await testCodexAppServerConnection({
provider: "codex-app-server",
providerSpecificData: {
codexAppServerUrl: "ws://127.0.0.1:1456",
codexAppServerToken: "deadbeef",
},
});
assert.ok(result, "app-server provider must take the readyz path");
assert.equal(result!.valid, false);
assert.equal(seen.length, 1);
assert.equal(seen[0].url, "http://127.0.0.1:1456/readyz");
assert.equal(seen[0].init?.redirect, "manual", "bearer token must never follow a redirect");
} finally {
globalThis.fetch = originalFetch;
}
});
test("testCodexAppServerConnection: env token + remote psd URL reports unconfigured, no network", async () => {
const { testCodexAppServerConnection } = await import(
"../../src/app/api/providers/[id]/test/codexAppServerHealth.ts"
);
const originalFetch = globalThis.fetch;
let fetched = false;
globalThis.fetch = (async () => {
fetched = true;
return new Response("ok", { status: 200 });
}) as typeof fetch;
try {
await withEnv({ ...BINDING_ENV_KEYS }, async () => {
const result = await testCodexAppServerConnection({
provider: "codex-app-server",
providerSpecificData: { codexAppServerUrl: "wss://evil.example.com:8443" },
});
assert.ok(result);
assert.equal(result!.valid, false);
assert.match(String((result!.diagnosis as { code?: string })?.code), /app_server_unconfigured/);
});
assert.equal(fetched, false, "binding refusal must happen before any network call");
} finally {
globalThis.fetch = originalFetch;
}
});