Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 63 additions & 5 deletions desktop/src-tauri/src/cli_command_posix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,24 @@ pub fn render_shim(cli: &Path, owner: &str) -> Result<String> {
pub fn render_helper(bin: &Path) -> Result<String> {
Ok(HELPER.replace("@BIN@", &quote(bin)?))
}
fn is_temporary_bundle_path(path: &Path, temp_dir: &Path) -> bool {
[
Path::new("/tmp"),
Path::new("/private/tmp"),
Path::new("/var/tmp"),
Path::new("/private/var/tmp"),
Path::new("/var/folders"),
Path::new("/private/var/folders"),
]
.into_iter()
.chain(std::iter::once(temp_dir))
.any(|root| path.starts_with(root))
|| path.starts_with("/Volumes")
Comment on lines +107 to +109

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Custom temp root blocks installed apps

When TMPDIR names a user's home, stable_bundle rejects the installed app under that home's Applications directory. The environment-derived root matches the stable bundle path, so Desktop cannot register its bundled command.

Learn more

The bundle check runs before validating a normal macOS app under Applications. std::env::temp_dir() follows the process environment, so its value is not necessarily a dedicated temporary directory. A temporary root that contains a stable install path makes the stable install look temporary even after canonicalization.

Example: Alice has TMPDIR=/Users/alice and runs /Users/alice/Applications/OpenCodex.app/Contents/MacOS/OpenCodex. The executable starts with the reported temporary root, so command registration refuses the installed app.

Recommended fix: Restrict the dynamic temporary root to recognized temporary locations, or explicitly preserve installed Applications paths before applying that root. Add a test with TMPDIR set to an ancestor of a stable user Applications bundle.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

}
fn has_app_translocation_marker(path: &Path) -> bool {
path.components()
.any(|component| component.as_os_str().to_str() == Some("AppTranslocation"))
}
pub fn block(shell: &str, root: &Path) -> Result<String> {
let body = if shell == "fish" {
format!(
Expand Down Expand Up @@ -148,18 +166,18 @@ pub fn stable_bundle(exe: &Path, debug: bool, version: &str) -> Result<Bundle> {
if debug {
return Err("development-launch".into());
}
let raw = text(exe)?;
if raw.starts_with("/Volumes/")
|| raw.starts_with("/private/var/folders/") && raw.contains("/AppTranslocation/")
|| raw.contains("/tmp/.mount_")
text(exe)?;
if has_app_translocation_marker(exe)
|| is_temporary_bundle_path(exe, &std::env::temp_dir())
|| std::env::var_os("APPIMAGE").is_some()
|| std::env::var_os("APPDIR").is_some()
{
return Err("temporary-bundle".into());
}
let real = fs::canonicalize(exe).map_err(|_| "bundle-unavailable")?;
let s = text(&real)?;
if s.starts_with("/Volumes/") || s.contains("/AppTranslocation/") || s.contains("/tmp/.mount_")
if has_app_translocation_marker(&real)
|| is_temporary_bundle_path(&real, &std::env::temp_dir())
{
return Err("temporary-bundle".into());
}
Expand Down Expand Up @@ -206,6 +224,46 @@ pub fn stable_bundle(exe: &Path, debug: bool, version: &str) -> Result<Bundle> {
version: version.into(),
})
}

#[cfg(test)]
mod temporary_bundle_path_tests {
use super::{has_app_translocation_marker, is_temporary_bundle_path};
use std::path::Path;

#[test]
fn temporary_bundle_roots_are_component_aware() {
let temp_dir = Path::new("/var/folders/test/app");
for path in [
"/tmp/App.app/Contents/MacOS/app",
"/private/tmp/App.app/Contents/MacOS/app",
"/var/tmp/App.app/Contents/MacOS/app",
"/private/var/tmp/App.app/Contents/MacOS/app",
"/var/folders/xx/App.app/Contents/MacOS/app",
"/private/var/folders/xx/App.app/Contents/MacOS/app",
"/Volumes/Installer/App.app/Contents/MacOS/app",
] {
assert!(is_temporary_bundle_path(Path::new(path), temp_dir), "{path}");
}
for path in [
"/Applications/OpenCodex.app/Contents/MacOS/app",
"/Users/alice/Applications/OpenCodex.app/Contents/MacOS/app",
"/tmp-old/OpenCodex.app/Contents/MacOS/app",
"/private/var/folder-old/OpenCodex.app/Contents/MacOS/app",
] {
assert!(!is_temporary_bundle_path(Path::new(path), temp_dir), "{path}");
}
assert!(is_temporary_bundle_path(
Path::new("/var/folders/test/app/OpenCodex.app/Contents/MacOS/app"),
temp_dir,
));
assert!(has_app_translocation_marker(Path::new(
"/Volumes/Installer/AppTranslocation/OpenCodex.app/Contents/MacOS/app"
)));
assert!(!has_app_translocation_marker(Path::new(
"/Volumes/Installer/AppTranslocation-old/OpenCodex.app/Contents/MacOS/app"
)));
}
}
// Return exact block range, including its line terminator. Reject malformed or changed markers.
fn range(bytes: &[u8], expected: &str) -> Result<Option<std::ops::Range<usize>>> {
let s = std::str::from_utf8(bytes).map_err(|_| "rc-not-utf8")?;
Expand Down
1 change: 1 addition & 0 deletions scripts/diagnostics/windows-version-control/src/control.rs
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,7 @@ pub fn run() -> Result<(), Box<dyn std::error::Error>> {
.env("OPENCODEX_ADMIN_AUTH_TOKEN", "version-fixture-admin")
.env("OPENCODEX_API_AUTH_TOKEN", "version-fixture-data")
.env("CODEX_CI", "1")
.env("OCX_TEST_HOME_GUARD", "1")
.args(["--no-env-file", "--no-orphans", "--cpu-prof"])
.arg(format!("--cpu-prof-dir={}", root.display()))
.arg("--cpu-prof-name=version.cpuprofile")
Expand Down
2 changes: 2 additions & 0 deletions scripts/test-layout/layout.json
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@
"pnpm-command-isolation.test.ts": "update", "provider-antigravity-quota-retry.test.ts": "providers", "project-config-warning-snapshot.test.ts": "codex-integration", "codex-quota-auto-refresh-generation.test.ts": "codex-integration", "codex-account-clear-paused.test.ts": "codex-integration",
"low-quota-protection.test.ts": "codex-integration",
"bun-runtime-preflight.test.ts": "lib",
"bounded-file-read.test.ts": "lib",
"responses-compaction-recovery.test.ts": "responses", "responses-canonical-nonstream.test.ts": "responses", "compaction-recovery-settings.test.ts": "config", "responses-compaction-recovery-policy.test.ts": "responses", "plugin-loader.test.ts": "lib", "plugin-upstream-hooks.test.ts": "lib",
"cli-kiro-auto-selection.test.ts": "cli", "codebuddy-live-models.test.ts": "providers", "kiro-auto-selection.test.ts": "providers/kiro", "kiro-quota-metrics.test.ts": "providers/kiro", "management-provider-request-pacing.test.ts": "server",
"desktop-supervised-restart.test.ts": "clients", "cli-restart-handoff.test.ts": "cli",
Expand Down Expand Up @@ -131,6 +132,7 @@
"core-link-boundary.test.ts": "lab",
"acl-error-classification.test.ts": "lib",
"active-registry-admission.test.ts": "codex-integration", "active-turn-disconnect.test.ts": "codex-integration",
"active-turn-cancellation-retention.test.ts": "server",
"adapter-buffered-tool-conformance.test.ts": "adapters",
"adapter-error-inline.test.ts": "adapters",
"adapter-event-oauth-failover.test.ts": "oauth",
Expand Down
7 changes: 4 additions & 3 deletions scripts/test-layout/seeds.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,16 @@
"codex-integration": {
"match": [
"^context-compat\\.test\\.ts$",
"^(?:active|app|bearer|catalog|combos\\.test\\.ts|doctor\\.test\\.ts|effort|gather|history|injection|issue|multi|native|parallel|project|selected|slug|ultrafast|warmup\\.test\\.ts)-"
"^(?!active-turn-cancellation-retention\\.test\\.ts$)(?:active|app|bearer|catalog|combos\\.test\\.ts|doctor\\.test\\.ts|effort|gather|history|injection|issue|multi|native|parallel|project|selected|slug|ultrafast|warmup\\.test\\.ts)-"
]
},
"server": {
"match": [
"^context-history\\.test\\.ts$",
"^aside-profiles-routes",
"^update-async-routes\\.test\\.ts$",
"^(?:account|alias|bounded|cancel|config\\.test\\.ts|consume|data|debug|error|errors|fetch|health|input|loopback|management|memory|outbound|owned|passive|port|ports\\.test\\.ts|proxy|relay|response|retry|server|session|sidebar|stream|v2)-",
"^active-turn-cancellation-retention\\.test\\.ts$",
"^(?!bounded-file-read\\.test\\.ts$)(?:account|alias|bounded|cancel|config\\.test\\.ts|consume|data|debug|error|errors|fetch|health|input|loopback|management|memory|outbound|owned|passive|port|ports\\.test\\.ts|proxy|relay|response|retry|server|session|sidebar|stream|v2)-",
"^subagent-model-force-api\\.test\\.ts$"
]
},
Expand Down Expand Up @@ -106,7 +107,7 @@
},
"lib": {
"match": [
"^(?:acl|clearable|credential|debug\\.test\\.ts|optional|redact\\.test\\.ts|remove|self|stall|strict|transient)-"
"^(?:acl|bounded|clearable|credential|debug\\.test\\.ts|optional|redact\\.test\\.ts|remove|self|stall|strict|transient)-"
]
},
"clients": {
Expand Down
23 changes: 18 additions & 5 deletions src/claude/desktop-3p-library.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { existsSync, readFileSync } from "node:fs";
import { homedir } from "node:os";
import { join } from "node:path";
import { readBoundedRegularFileSync } from "../lib/bounded-file-read";
import { claudeDesktopConfigLibraryDir, resolveConfigLibraryDir } from "./desktop-3p-paths";

export interface Desktop3pConfigLibraryOptions {
Expand Down Expand Up @@ -43,10 +43,17 @@ export interface Desktop3pMetadata {
}

export const DESKTOP_PICKER_ENTRY_NAME = "opencodex-picker";
export const MAX_DESKTOP_PROFILE_FILE_BYTES = 4 * 1024 * 1024;

export function parseMetadata(path: string): Desktop3pMetadata {
if (!existsSync(path)) return { entries: [] };
const parsed = JSON.parse(readFileSync(path, "utf8")) as Partial<Desktop3pMetadata>;
let bytes: Buffer;
try {
bytes = readBoundedRegularFileSync(path, MAX_DESKTOP_PROFILE_FILE_BYTES);
} catch (error) {
if ((error as NodeJS.ErrnoException).code === "ENOENT") return { entries: [] };
throw error;
}
const parsed = JSON.parse(bytes.toString("utf8")) as Partial<Desktop3pMetadata>;
if (!Array.isArray(parsed.entries)) throw new Error("Claude Desktop 3P _meta.json has no entries array");
return { ...parsed, entries: parsed.entries };
}
Expand Down Expand Up @@ -81,8 +88,14 @@ export const OPENCODEX_DESKTOP_PROFILE_KEYS = new Set([
]);

export function readDesktopProfileForeignKeys(path: string): Record<string, unknown> {
if (!existsSync(path)) return {};
const parsed = JSON.parse(readFileSync(path, "utf8")) as unknown;
let bytes: Buffer;
try {
bytes = readBoundedRegularFileSync(path, MAX_DESKTOP_PROFILE_FILE_BYTES);
} catch (error) {
if ((error as NodeJS.ErrnoException).code === "ENOENT") return {};
throw error;
}
const parsed = JSON.parse(bytes.toString("utf8")) as unknown;
if (!isRecord(parsed)) throw new Error("Claude Desktop 3P profile is not a JSON object");
return Object.fromEntries(
Object.entries(parsed).filter(([key]) => !OPENCODEX_DESKTOP_PROFILE_KEYS.has(key)),
Expand Down
16 changes: 12 additions & 4 deletions src/claude/desktop-3p.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,14 @@ import { canonicalDirectory } from "./desktop-remote-store-io";
import {
resolveDesktop3pConfigLibraryPath, parseMetadata, SAFE_DESKTOP_PROFILE_ID, isRecord,
isOwnedDesktopEntry, isOwnedDesktopGatewayEntry, profilePath, readDesktopProfileForeignKeys,
MAX_DESKTOP_PROFILE_FILE_BYTES,
type Desktop3pConfigLibraryOptions, type Desktop3pMetadata, type Desktop3pMetadataEntry,
} from "./desktop-3p-library";
export { resolveDesktop3pConfigLibraryPath, type Desktop3pConfigLibraryOptions } from "./desktop-3p-library";
import { createHash, randomUUID } from "node:crypto";
import { copyFileSync, existsSync, mkdirSync, readFileSync, lstatSync, unlinkSync } from "node:fs";
import { copyFileSync, existsSync, mkdirSync, lstatSync, unlinkSync } from "node:fs";
import { join } from "node:path";
import { readBoundedRegularFileSync } from "../lib/bounded-file-read";
import { atomicWriteFile, readConfigDiagnostics, withConfigMutationLockSync } from "../config";
import { atomicWriteFileNoFollow } from "../config/atomic-write";
import { claudeDesktopIntegrationEnabled } from "../codex/desired-state";
Expand Down Expand Up @@ -476,7 +478,7 @@ export function inspectDesktop3pConfigLibrary(
let profile: Record<string, unknown>;
let fingerprint: string;
try {
const source = readFileSync(selectedProfilePath, "utf8");
const source = readBoundedRegularFileSync(selectedProfilePath, MAX_DESKTOP_PROFILE_FILE_BYTES).toString("utf8");
const parsed = JSON.parse(source) as unknown;
if (!isRecord(parsed)) return { kind: "broken", libraryPath, selectedProfilePath, appliedId, residualPaths, ownedProfileActive };
profile = parsed;
Expand Down Expand Up @@ -709,7 +711,10 @@ export function writeDesktop3pConfig(
const metadata = parseMetadata(join(inspected.libraryPath, "_meta.json"));
if (!isOwnedDesktopGatewayEntry(metadata.entries.find(entry => entry.id === inspected.appliedId))) return refused;
try {
const selected = JSON.parse(readFileSync(inspected.selectedProfilePath, "utf8")) as Record<string, unknown>;
const selected = JSON.parse(readBoundedRegularFileSync(
inspected.selectedProfilePath,
MAX_DESKTOP_PROFILE_FILE_BYTES,
).toString("utf8")) as Record<string, unknown>;
mode = selected.modelDiscoveryEnabled !== true ? "static"
: Array.isArray(selected.inferenceModels) ? "hybrid" : "discovery";
} catch { return refused; }
Expand Down Expand Up @@ -825,7 +830,10 @@ function writeDesktop3pConfigWithGenerator(
const freshMetadata = parseMetadata(metadataPath);
if (freshMetadata.appliedId !== refreshOnly.requiredId
|| !isOwnedDesktopGatewayEntry(freshMetadata.entries.find(entry => entry.id === refreshOnly.requiredId))
|| createHash("sha256").update(readFileSync(configPath)).digest("hex").slice(0, 16) !== refreshOnly.expectedFingerprint) {
|| createHash("sha256").update(readBoundedRegularFileSync(
configPath,
MAX_DESKTOP_PROFILE_FILE_BYTES,
)).digest("hex").slice(0, 16) !== refreshOnly.expectedFingerprint) {
throw new Error();
}
} catch { throw new Error("desktop_refresh_only_skipped"); }
Expand Down
86 changes: 83 additions & 3 deletions src/claude/intercept/picker-listener.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ export interface PickerListenerOptions {
maxEncodedBytes?: number;
/** Test seam: concurrent upstream requests; production uses PICKER_MAX_ACTIVE_UPSTREAMS. */
maxActiveUpstreams?: number;
/** Test seam: maximum time for a client upload to finish. */
uploadDeadlineMs?: number;
/** Test seam: maximum time without upload progress. */
uploadIdleTimeoutMs?: number;
log?: (line: string) => void;
}
export interface PickerListenerHandle { port: number; close(): Promise<void> }
Expand All @@ -51,6 +55,9 @@ export const PICKER_MAX_ACTIVE_UPSTREAMS = 256;
export const PICKER_MAX_CONCURRENT_STREAMS = 100;
/** A connection that has not sent a complete ClientHello by then is dropped. */
const CLIENT_HELLO_TIMEOUT_MS = 10_000;
const PICKER_UPLOAD_DEADLINE_MS = 60_000;
const PICKER_UPLOAD_IDLE_TIMEOUT_MS = 15_000;
const MAX_TIMER_DELAY_MS = 2_147_483_647;

const HOP_HEADERS = new Set([
"connection", "keep-alive", "proxy-connection", "proxy-authenticate", "proxy-authorization",
Expand Down Expand Up @@ -152,6 +159,13 @@ export async function startPickerListener(options: PickerListenerOptions): Promi
const upstream = options.upstream ?? { host: "claude.ai", port: 443, servername: "claude.ai" };
const cap = options.maxEncodedBytes ?? BOOTSTRAP_MAX_ENCODED_BYTES;
const maxActiveUpstreams = options.maxActiveUpstreams ?? PICKER_MAX_ACTIVE_UPSTREAMS;
const uploadDeadlineMs = options.uploadDeadlineMs ?? PICKER_UPLOAD_DEADLINE_MS;
const uploadIdleTimeoutMs = options.uploadIdleTimeoutMs ?? PICKER_UPLOAD_IDLE_TIMEOUT_MS;
for (const [name, value] of [["uploadDeadlineMs", uploadDeadlineMs], ["uploadIdleTimeoutMs", uploadIdleTimeoutMs]] as const) {
if (!Number.isFinite(value) || value <= 0 || value > MAX_TIMER_DELAY_MS) {
throw new RangeError(`${name} must be a positive finite timer duration`);
}
}
let activeUpstreams = 0;
const upgrades = new Set<Duplex>();
const spliced = new Set<Socket>();
Expand Down Expand Up @@ -209,10 +223,50 @@ export async function startPickerListener(options: PickerListenerOptions): Promi
const refuse = (status: 400 | 503) => {
// Do not leave a refused, still-uploading stream behind after delivering its empty reply.
if (uploading && !req.complete) {
req.once("error", () => res.destroy());
responseWritable.once("finish", () => closeInput(true));
if (h2) {
responseWritable.once("finish", () => closeInput(true));
} else {
let settled = false;
let deadlineTimer: NodeJS.Timeout | undefined;
let idleTimer: NodeJS.Timeout | undefined;
const cleanup = () => {
if (settled) return;
settled = true;
if (deadlineTimer) clearTimeout(deadlineTimer);
if (idleTimer) clearTimeout(idleTimer);
req.off("data", onProgress);
req.off("end", onBodyEnd);
req.off("close", onBodyEnd);
req.off("error", onBodyEnd);
responseWritable.off("finish", onReplyFinished);
};
const closeOnTimeout = () => {
cleanup();
closeInput(true);
};
const onProgress = () => {
if (settled || !deadlineTimer) return;
if (idleTimer) clearTimeout(idleTimer);
idleTimer = setTimeout(closeOnTimeout, uploadIdleTimeoutMs);
idleTimer.unref?.();
};
const onBodyEnd = () => cleanup();
const onReplyFinished = () => {
if (settled) return;
if (req.complete) { cleanup(); return; }
deadlineTimer = setTimeout(closeOnTimeout, uploadDeadlineMs);
deadlineTimer.unref?.();
onProgress();
};
req.on("data", onProgress);
req.once("end", onBodyEnd);
req.once("close", onBodyEnd);
req.once("error", onBodyEnd);
responseWritable.once("finish", onReplyFinished);
req.resume();
}
}
res.writeHead(status, { "Content-Length": "0", ...(!h2 && uploading && !req.complete ? { Connection: "close" } : {}) });
res.writeHead(status, { "Content-Length": "0" });
res.end();
options.log?.(`picker request refused ${status}`);
};
Expand Down Expand Up @@ -329,9 +383,25 @@ export async function startPickerListener(options: PickerListenerOptions): Promi
if (h2) (req as Http2ServerRequest).stream.once("close", onClientClose);
if (uploading) {
let uploadFinished = false;
let deadlineTimer: NodeJS.Timeout | undefined;
let idleTimer: NodeJS.Timeout | undefined;
let expireUpload: () => void = () => {};
const clearUploadTimers = () => {
if (deadlineTimer) clearTimeout(deadlineTimer);
if (idleTimer) clearTimeout(idleTimer);
deadlineTimer = undefined;
idleTimer = undefined;
};
const onUploadProgress = () => {
if (idleTimer) clearTimeout(idleTimer);
idleTimer = setTimeout(expireUpload, uploadIdleTimeoutMs);
idleTimer.unref?.();
};
const finishUpload = () => {
if (uploadFinished) return;
uploadFinished = true;
clearUploadTimers();
req.off("data", onUploadProgress);
req.off("end", finishUpload);
req.off("close", finishUpload);
upReq.off("close", onUpstreamClose);
Expand All @@ -349,16 +419,26 @@ export async function startPickerListener(options: PickerListenerOptions): Promi
upReq.destroy();
}
};
expireUpload = () => {
if (uploadFinished) return;
clientGone = true;
stopUnfinishedUpload();
release();
};
const onResponseFinish = () => stopUnfinishedUpload(true);
const onUpstreamClose = () => {
if (!upstreamResponded && !res.writableEnded) stopUnfinishedUpload();
};
req.once("end", finishUpload);
req.once("close", finishUpload);
req.on("data", onUploadProgress);
upReq.once("close", onUpstreamClose);
responseWritable.once("finish", onResponseFinish);
res.once("close", stopUnfinishedUpload);
if (h2) (req as Http2ServerRequest).stream.once("close", finishUpload);
deadlineTimer = setTimeout(expireUpload, uploadDeadlineMs);
deadlineTimer.unref?.();
onUploadProgress();
}
req.pipe(upReq);
};
Expand Down
Loading