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
74 changes: 62 additions & 12 deletions CLI/cmux.swift
Original file line number Diff line number Diff line change
Expand Up @@ -23286,19 +23286,51 @@ struct CMUXCLI {
return root
}

private func createClaudeNodeOptionsRestoreModule() throws -> URL {
let rawTemporaryDirectory = ProcessInfo.processInfo.environment["TMPDIR"]?
/// Tokens occupied by a `--require` for a cmux-written restore module at `index`, or `0`.
///
/// Such a path may already be reaped, so it must never be carried forward into `NODE_OPTIONS`
/// or into the saved original. Both the `=` and space-separated spellings count: a persisted
/// launch environment can carry either.
private func claudeNodeOptionsRestoreModuleRequireWidth(_ tokens: [String], index: Int) -> Int {
ClaudeNodeOptionsRestoreModule.ownedRequireTokenWidth(
tokens,
index: index,
moduleDirectory: claudeNodeOptionsDirectory()
)
}

private func claudeNodeOptionsDirectory() -> URL {
let override = ProcessInfo.processInfo.environment["CMUX_NODE_OPTIONS_DIR"]?
.trimmingCharacters(in: .whitespacesAndNewlines)
let temporaryDirectory: String
if let rawTemporaryDirectory, !rawTemporaryDirectory.isEmpty {
temporaryDirectory = rawTemporaryDirectory
} else {
temporaryDirectory = NSTemporaryDirectory()
if let override, !override.isEmpty {
return URL(fileURLWithPath: override, isDirectory: true)
}
let root = URL(fileURLWithPath: temporaryDirectory, isDirectory: true)
.appendingPathComponent("cmux-claude-node-options", isDirectory: true)
try FileManager.default.createDirectory(at: root, withIntermediateDirectories: true, attributes: nil)
let restoreModuleURL = root.appendingPathComponent("restore-node-options.cjs", isDirectory: false)
// Not $TMPDIR: macOS reaps files there after a few days of no access, while a session
// that outlives the reaper keeps --require pointing at the deleted module, so every
// child node exits with MODULE_NOT_FOUND before running any user code. The CLI is a
// composition root, so it names the concrete FileManager.default here.
return CmuxStateDirectory.url(homeDirectory: FileManager.default.homeDirectoryForCurrentUser)
.appendingPathComponent(ClaudeNodeOptionsRestoreModule.directoryName, isDirectory: true)
}

private func createClaudeNodeOptionsRestoreModule() throws -> URL {
let root = claudeNodeOptionsDirectory()
let restoreModuleURL = root.appendingPathComponent(ClaudeNodeOptionsRestoreModule.fileName, isDirectory: false)
// node splits NODE_OPTIONS on whitespace, so a path holding a space or a double quote
// cannot be expressed there. Fail so the caller skips the injection entirely.
let unexpressible = CharacterSet.whitespacesAndNewlines.union(CharacterSet(charactersIn: "\""))
guard restoreModuleURL.path.rangeOfCharacter(from: unexpressible) == nil else {
throw CLIError(message: "Claude NODE_OPTIONS restore module path is not expressible in NODE_OPTIONS: \(restoreModuleURL.path)")
}
// 0700, matching SocketControlPasswordStore: whichever component creates
// ~/.local/state/cmux sets its mode, and createDirectory does not re-apply attributes to
// a directory that already exists. Creating it 0755 here would leave the socket control
// password sitting in a world-readable directory.
try FileManager.default.createDirectory(
at: root,
withIntermediateDirectories: true,
attributes: [.posixPermissions: 0o700]
)
try writeShimIfChanged(Self.claudeNodeOptionsRestoreModule, to: restoreModuleURL)
return restoreModuleURL
}
Expand Down Expand Up @@ -30810,7 +30842,7 @@ struct CMUXCLI {

private func mergedNodeOptions(existing: String?, restoreModulePath: String) -> String {
let requireOption = "--require=\(restoreModulePath)"
let memoryOption = "--max-old-space-size=4096"
let memoryOption = "--max-old-space-size=\(ClaudeNodeOptionsRestoreModule.injectedHeapCapMB)"
let cleanedExisting = cleanedNodeOptions(existing)
guard !cleanedExisting.isEmpty else {
return "\(requireOption) \(memoryOption)"
Expand All @@ -30836,6 +30868,11 @@ struct CMUXCLI {
index += 1
continue
}
let ownedRequireWidth = claudeNodeOptionsRestoreModuleRequireWidth(tokens, index: index)
if ownedRequireWidth > 0 {
index += ownedRequireWidth
continue
}
filtered.append(token)
index += 1
}
Expand All @@ -30850,13 +30887,26 @@ struct CMUXCLI {

var normalized: [String] = []
var index = 0
var shouldDropInjectedHeapCap = false
while index < tokens.count {
let token = tokens[index]
if shouldDropInjectedHeapCap, ClaudeNodeOptionsRestoreModule.isInjectedHeapCap(tokens, index: index) {
index += ClaudeNodeOptionsRestoreModule.heapCapTokenWidth(tokens, index: index)
shouldDropInjectedHeapCap = false
continue
}
shouldDropInjectedHeapCap = false
if token == "--max-old-space-size", index + 1 < tokens.count {
normalized.append("--max-old-space-size=\(tokens[index + 1])")
index += 2
continue
}
let ownedRequireWidth = claudeNodeOptionsRestoreModuleRequireWidth(tokens, index: index)
if ownedRequireWidth > 0 {
index += ownedRequireWidth
shouldDropInjectedHeapCap = true
continue
}
normalized.append(token)
index += 1
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,106 @@ public struct ClaudeConfigDirectoryPath: Sendable {
}
}

/// The preload module cmux injects via `NODE_OPTIONS=--require=…` so that child node processes
/// get the caller's original `NODE_OPTIONS` back instead of cmux's own heap cap.
///
/// The claude wrapper, the CLI, and this policy each handle the module from a different side —
/// two write it, one strips it back out — so the rule for "is this path ours" lives here once.
public struct ClaudeNodeOptionsRestoreModule: Sendable {
private init() {}

/// The module's file name, identical in every directory cmux has written it to.
public static let fileName = "restore-node-options.cjs"

/// The directory holding the module under the cmux state directory.
public static let directoryName = "node-options"

/// The `$TMPDIR` directory older builds used. The remote daemon still appends a random suffix
/// to it, so both the exact name and the `<name>-<random>` form count as cmux's.
private static let legacyDirectoryName = "cmux-claude-node-options"

/// The V8 heap cap, in MB, that cmux injects alongside the module. Named because unwinding an
/// injected `NODE_OPTIONS` has to tell cmux's own value apart from one the caller chose.
public static let injectedHeapCapMB = "4096"

/// Name of the cmux state directory. Duplicated from `CmuxStateDirectory` because this target
/// has no dependencies; it is the leaf of `~/.local/state/cmux`.
private static let stateDirectoryName = "cmux"

/// The full directory tail cmux writes the module to, below the user's home. Matching the
/// whole tail rather than just `cmux/node-options` keeps an unrelated preload at, say,
/// `/opt/vendor/cmux/node-options/` from being claimed as ours.
private static let stateDirectoryTail = [".local", "state", stateDirectoryName, directoryName]

/// Whether a `--require` path points at a cmux-written copy of the module.
///
/// The file name alone is not enough: a caller may legitimately preload their own module of
/// the same name, and dropping that would silently change their runtime. Path shape alone is
/// not enough either — it is both too generous (an unrelated `…/cmux/node-options/`) and too
/// mean (a configured directory that looks like neither known shape). So the authoritative
/// test is `moduleDirectory`, the directory the caller actually writes to; the name and tail
/// checks only recognise copies left by another process or an older build.
///
/// - Parameter moduleDirectory: Where this process writes the module, when it writes one.
/// Callers that only strip inherited values (they never write) pass `nil`.
public static func isCmuxOwnedPath(_ path: String, moduleDirectory: URL? = nil) -> Bool {
let unquoted = path.trimmingCharacters(in: CharacterSet(charactersIn: "'\""))
let moduleURL = URL(fileURLWithPath: unquoted).standardizedFileURL
guard moduleURL.lastPathComponent == fileName else { return false }

let parent = moduleURL.deletingLastPathComponent()
if let moduleDirectory, parent == moduleDirectory.standardizedFileURL {
return true
}

let parentName = parent.lastPathComponent
if parentName == legacyDirectoryName || parentName.hasPrefix("\(legacyDirectoryName)-") {
return true
}
return parent.pathComponents.suffix(stateDirectoryTail.count).elementsEqual(stateDirectoryTail)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
}

/// How many tokens a cmux-owned `--require` occupies at `index`, or `0` when the token there
/// is not one.
///
/// Both `--require=<path>` and `--require <path>` (and the `-r` spellings) have to be
/// recognised: a persisted launch environment can carry either shape, and missing the
/// space-separated one leaves a dead preload in `NODE_OPTIONS` — the failure this whole
/// mechanism exists to avoid.
public static func ownedRequireTokenWidth(
_ tokens: [String],
index: Int,
moduleDirectory: URL? = nil
) -> Int {
guard index < tokens.count else { return 0 }
let token = tokens[index]

for prefix in ["--require=", "-r="] where token.hasPrefix(prefix) {
let path = String(token.dropFirst(prefix.count))
return isCmuxOwnedPath(path, moduleDirectory: moduleDirectory) ? 1 : 0
}
guard token == "--require" || token == "-r", index + 1 < tokens.count else { return 0 }
return isCmuxOwnedPath(tokens[index + 1], moduleDirectory: moduleDirectory) ? 2 : 0
}

/// Whether the token at `index` is the heap cap cmux injects, rather than one the caller chose.
public static func isInjectedHeapCap(_ tokens: [String], index: Int) -> Bool {
guard index < tokens.count else { return false }
let token = tokens[index]
if token == "--max-old-space-size" {
return index + 1 < tokens.count && tokens[index + 1] == injectedHeapCapMB
}
return token == "--max-old-space-size=\(injectedHeapCapMB)"
}

/// How many tokens the heap cap at `index` occupies: two for the space-separated form, one for
/// the `=` form.
public static func heapCapTokenWidth(_ tokens: [String], index: Int) -> Int {
guard index < tokens.count else { return 1 }
return tokens[index] == "--max-old-space-size" ? min(2, tokens.count - index) : 1
}
}

/// Selects the non-secret launch environment values that are safe to replay when restoring agents.
public struct AgentLaunchEnvironmentPolicy: Sendable {
/// Creates a launch environment policy.
Expand Down Expand Up @@ -210,22 +310,16 @@ public struct AgentLaunchEnvironmentPolicy: Sendable {
while index < tokens.count {
let token = tokens[index]

if shouldDropInjectedHeapCap, isInjectedNodeHeapCap(tokens, index: index) {
index += nodeHeapCapWidth(tokens, index: index)
if shouldDropInjectedHeapCap, ClaudeNodeOptionsRestoreModule.isInjectedHeapCap(tokens, index: index) {
index += ClaudeNodeOptionsRestoreModule.heapCapTokenWidth(tokens, index: index)
shouldDropInjectedHeapCap = false
continue
}
shouldDropInjectedHeapCap = false

if isRequireOption(token), index + 1 < tokens.count,
isCmuxNodeOptionsRestoreModulePath(tokens[index + 1]) {
index += 2
shouldDropInjectedHeapCap = true
continue
}
if let path = inlineRequireOptionPath(token),
isCmuxNodeOptionsRestoreModulePath(path) {
index += 1
let ownedRequireWidth = ClaudeNodeOptionsRestoreModule.ownedRequireTokenWidth(tokens, index: index)
if ownedRequireWidth > 0 {
index += ownedRequireWidth
shouldDropInjectedHeapCap = true
continue
}
Expand All @@ -247,36 +341,4 @@ public struct AgentLaunchEnvironmentPolicy: Sendable {
return trimmed
}

private func isRequireOption(_ token: String) -> Bool {
token == "--require" || token == "-r"
}

private func inlineRequireOptionPath(_ token: String) -> String? {
for prefix in ["--require=", "-r="] where token.hasPrefix(prefix) {
return String(token.dropFirst(prefix.count))
}
return nil
}

private func isCmuxNodeOptionsRestoreModulePath(_ value: String) -> Bool {
let trimmed = value.trimmingCharacters(in: CharacterSet(charactersIn: "'\""))
guard URL(fileURLWithPath: trimmed).lastPathComponent == "restore-node-options.cjs" else {
return false
}
return trimmed.contains("/cmux-")
}

private func isInjectedNodeHeapCap(_ tokens: [String], index: Int) -> Bool {
guard index < tokens.count else { return false }
let token = tokens[index]
if token == "--max-old-space-size" {
return index + 1 < tokens.count && tokens[index + 1] == "4096"
}
return token == "--max-old-space-size=4096"
}

private func nodeHeapCapWidth(_ tokens: [String], index: Int) -> Int {
guard index < tokens.count else { return 1 }
return tokens[index] == "--max-old-space-size" ? min(2, tokens.count - index) : 1
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,113 @@ import Testing

@Suite("AgentLaunchEnvironmentPolicy")
struct AgentLaunchEnvironmentPolicyTests {
@Test(
"Drops the cmux NODE_OPTIONS restore preload from every directory it has shipped in",
arguments: [
"/var/folders/ab/T/cmux-claude-node-options/restore-node-options.cjs",
"/tmp/cmux-claude-node-options-9f3a/restore-node-options.cjs",
"/Users/someone/.local/state/cmux/node-options/restore-node-options.cjs",
]
)
func dropsCmuxNodeOptionsRestorePreload(modulePath: String) {
let selected = AgentLaunchEnvironmentPolicy().selectedEnvironment(
from: ["NODE_OPTIONS": "--require=\(modulePath) --max-old-space-size=4096 --trace-warnings"],
kind: "claude"
)

#expect(selected == ["NODE_OPTIONS": "--trace-warnings"])
}

@Test("Keeps a user's own preload that merely shares the module name")
func keepsUnrelatedPreloadWithTheSameModuleName() {
let selected = AgentLaunchEnvironmentPolicy().selectedEnvironment(
from: ["NODE_OPTIONS": "--require=/opt/vendor/restore-node-options.cjs --max-old-space-size=4096"],
kind: "claude"
)

#expect(selected == ["NODE_OPTIONS": "--require=/opt/vendor/restore-node-options.cjs --max-old-space-size=4096"])
}

@Test(
"Keeps a caller preload that only resembles the cmux location, and its following options",
arguments: [
"/opt/vendor/cmux/node-options/restore-node-options.cjs",
"/Users/me/Code/cmux-fork/restore-node-options.cjs",
"/srv/cmux/restore-node-options.cjs",
]
)
func keepsPreloadThatOnlyResemblesTheCmuxLocation(callerPreload: String) {
// The trailing 4096 is the caller's here: nothing cmux-owned precedes it, so the
// injected-heap-cap unwind must not consume it either.
let nodeOptions = "--require=\(callerPreload) --max-old-space-size=4096 --trace-warnings"
let selected = AgentLaunchEnvironmentPolicy().selectedEnvironment(
from: ["NODE_OPTIONS": nodeOptions],
kind: "claude"
)

#expect(selected == ["NODE_OPTIONS": nodeOptions])
}

@Test(
"Drops a cmux preload written in any --require spelling, with its paired heap cap",
arguments: [
"--require=/tmp/cmux-claude-node-options/restore-node-options.cjs --max-old-space-size=4096",
"--require /tmp/cmux-claude-node-options/restore-node-options.cjs --max-old-space-size 4096",
"-r /tmp/cmux-claude-node-options/restore-node-options.cjs --max-old-space-size=4096",
"-r=/tmp/cmux-claude-node-options/restore-node-options.cjs --max-old-space-size 4096",
]
)
func dropsPreloadInEverySpelling(injected: String) {
// A persisted environment can carry either spelling, and a surviving dead preload is the
// MODULE_NOT_FOUND crash this mechanism exists to prevent.
let selected = AgentLaunchEnvironmentPolicy().selectedEnvironment(
from: ["NODE_OPTIONS": "\(injected) --trace-warnings"],
kind: "claude"
)

#expect(selected == ["NODE_OPTIONS": "--trace-warnings"])
}

@Test(
"Drops a cmux preload whose path arrives wrapped in quotes",
arguments: [
"--require=\"/tmp/cmux-claude-node-options/restore-node-options.cjs\"",
"--require \"/tmp/cmux-claude-node-options/restore-node-options.cjs\"",
]
)
func dropsQuotedPreload(injected: String) {
// node consumes surrounding double quotes in NODE_OPTIONS, so an inherited value can
// carry them around the path.
let selected = AgentLaunchEnvironmentPolicy().selectedEnvironment(
from: ["NODE_OPTIONS": "\(injected) --trace-warnings"],
kind: "claude"
)

#expect(selected == ["NODE_OPTIONS": "--trace-warnings"])
}

@Test("Keeps a caller preload written in the space-separated form")
func keepsCallerPreloadInSpaceForm() {
let nodeOptions = "--require /opt/vendor/instrument.cjs --trace-warnings"
let selected = AgentLaunchEnvironmentPolicy().selectedEnvironment(
from: ["NODE_OPTIONS": nodeOptions],
kind: "claude"
)

#expect(selected == ["NODE_OPTIONS": nodeOptions])
}

@Test("Keeps a heap cap the caller chose while dropping the one cmux injected")
func keepsCallerHeapCapAndDropsInjectedOne() {
let modulePath = "/Users/someone/.local/state/cmux/node-options/restore-node-options.cjs"
let selected = AgentLaunchEnvironmentPolicy().selectedEnvironment(
from: ["NODE_OPTIONS": "--require=\(modulePath) --max-old-space-size=4096 --max-old-space-size=2048"],
kind: "claude"
)

#expect(selected == ["NODE_OPTIONS": "--max-old-space-size=2048"])
}

@Test("Preserves OMP config roots without persisting secrets")
func preservesOmpConfigRootsWithoutPersistingSecrets() {
let selected = AgentLaunchEnvironmentPolicy().selectedEnvironment(
Expand Down
Loading
Loading