diff --git a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/PrivateDirectoryCheck.swift b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/PrivateDirectoryCheck.swift new file mode 100644 index 000000000000..319dd17f57b6 --- /dev/null +++ b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/PrivateDirectoryCheck.swift @@ -0,0 +1,48 @@ +public import Darwin + +/// Makes a directory private to one user before cmux writes into it, such as +/// the per-surface agent command shim directories under a temporary directory. +/// +/// A directory in a shared temporary directory can already exist under another +/// user's control, so the path is opened without following a symlink and kept +/// only when it is a real directory this user owns. It is then set to 0700. +/// +/// ```swift +/// guard PrivateDirectoryCheck().makePrivate(atPath: directory.path) else { return nil } +/// ``` +public struct PrivateDirectoryCheck: Sendable { + /// The user that must own the directory. + public let owner: uid_t + + /// Creates a check for directories owned by `owner`, the effective user by default. + public init(owner: uid_t = geteuid()) { + self.owner = owner + } + + /// Sets the directory at `path` to mode 0700 when it is a real directory + /// owned by ``owner``. + /// + /// - Returns: `true` when `path` is still that directory, not a symlink, + /// owned by ``owner`` and writable by no one else; otherwise `false`, + /// without changing anything the path does not own. + public func makePrivate(atPath path: String) -> Bool { + let fd = open(path, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC) + guard fd >= 0 else { return false } + defer { close(fd) } + var opened = stat() + guard fstat(fd, &opened) == 0, + (opened.st_mode & S_IFMT) == S_IFDIR, + opened.st_uid == owner, + fchmod(fd, 0o700) == 0 else { + return false + } + // The path must still name the directory that was opened and changed. + var current = stat() + guard lstat(path, ¤t) == 0 else { return false } + return current.st_dev == opened.st_dev + && current.st_ino == opened.st_ino + && (current.st_mode & S_IFMT) == S_IFDIR + && current.st_uid == owner + && current.st_mode & (S_IWGRP | S_IWOTH) == 0 + } +} diff --git a/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/PrivateDirectoryCheckTests.swift b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/PrivateDirectoryCheckTests.swift new file mode 100644 index 000000000000..fccfbfae89ef --- /dev/null +++ b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/PrivateDirectoryCheckTests.swift @@ -0,0 +1,69 @@ +import Darwin +import Foundation +import Testing +@testable import CmuxFoundation + +@Suite struct PrivateDirectoryCheckTests { + private let directory: URL + + init() throws { + directory = FileManager.default.temporaryDirectory + .appendingPathComponent("cmux-private-directory-\(UUID().uuidString)", isDirectory: true) + try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true) + } + + private func path(_ name: String) -> String { + directory.appendingPathComponent(name).path + } + + private func makeDirectory(_ name: String, mode: mode_t) throws -> String { + let path = path(name) + try FileManager.default.createDirectory(atPath: path, withIntermediateDirectories: false) + #expect(chmod(path, mode) == 0) + return path + } + + private func mode(atPath path: String) -> mode_t? { + var info = stat() + guard lstat(path, &info) == 0 else { return nil } + return info.st_mode & 0o7777 + } + + @Test func tightensAnOwnedDirectory() throws { + defer { try? FileManager.default.removeItem(at: directory) } + let shared = try makeDirectory("shared", mode: 0o775) + #expect(PrivateDirectoryCheck().makePrivate(atPath: shared)) + #expect(mode(atPath: shared) == 0o700) + } + + @Test func rejectsASymlinkAndLeavesItsTarget() throws { + defer { try? FileManager.default.removeItem(at: directory) } + let target = try makeDirectory("target", mode: 0o755) + let link = path("link") + try FileManager.default.createSymbolicLink(atPath: link, withDestinationPath: target) + #expect(!PrivateDirectoryCheck().makePrivate(atPath: link)) + #expect(mode(atPath: target) == 0o755) + } + + @Test func rejectsADirectoryAnotherUserOwns() throws { + defer { try? FileManager.default.removeItem(at: directory) } + let foreign = try makeDirectory("foreign", mode: 0o755) + let check = PrivateDirectoryCheck(owner: geteuid() &+ 1) + #expect(!check.makePrivate(atPath: foreign)) + #expect(mode(atPath: foreign) == 0o755) + } + + @Test func rejectsARegularFile() throws { + defer { try? FileManager.default.removeItem(at: directory) } + let file = path("file") + try "kept\n".write(toFile: file, atomically: false, encoding: .utf8) + #expect(chmod(file, 0o644) == 0) + #expect(!PrivateDirectoryCheck().makePrivate(atPath: file)) + #expect(mode(atPath: file) == 0o644) + } + + @Test func rejectsAMissingPath() { + defer { try? FileManager.default.removeItem(at: directory) } + #expect(!PrivateDirectoryCheck().makePrivate(atPath: path("missing"))) + } +} diff --git a/Packages/macOS/CmuxTerminal/Sources/CmuxTerminal/Spawn/TerminalSurface+AgentCommandShims.swift b/Packages/macOS/CmuxTerminal/Sources/CmuxTerminal/Spawn/TerminalSurface+AgentCommandShims.swift index 3725d2ee7c5b..9b6e0fd7fb02 100644 --- a/Packages/macOS/CmuxTerminal/Sources/CmuxTerminal/Spawn/TerminalSurface+AgentCommandShims.swift +++ b/Packages/macOS/CmuxTerminal/Sources/CmuxTerminal/Spawn/TerminalSurface+AgentCommandShims.swift @@ -1,4 +1,5 @@ public import Foundation +import CmuxFoundation public import CmuxTerminalCore extension TerminalSurface { @@ -119,12 +120,15 @@ extension TerminalSurface { defer { try? fileManager.removeItem(at: stagingDirectory) } + // A shared temporary directory lets another user create these + // directories first or plant a symlink, so each one must be a real + // directory this user owns before anything is written into it. + let privateDirectoryCheck = PrivateDirectoryCheck() do { try fileManager.createDirectory(at: shimParentDirectory, withIntermediateDirectories: true) + guard privateDirectoryCheck.makePrivate(atPath: shimParentDirectory.path) else { return nil } try fileManager.createDirectory(at: stagingDirectory, withIntermediateDirectories: false) - for directory in [shimParentDirectory, stagingDirectory] { - try fileManager.setAttributes([.posixPermissions: 0o700], ofItemAtPath: directory.path) - } + guard privateDirectoryCheck.makePrivate(atPath: stagingDirectory.path) else { return nil } } catch { return nil } @@ -167,6 +171,7 @@ extension TerminalSurface { guard !shims.isEmpty else { return nil } do { if fileManager.fileExists(atPath: shimDirectory.path) { + guard privateDirectoryCheck.makePrivate(atPath: shimDirectory.path) else { return nil } _ = try fileManager.replaceItemAt( shimDirectory, withItemAt: stagingDirectory, @@ -176,10 +181,10 @@ extension TerminalSurface { } else { try fileManager.moveItem(at: stagingDirectory, to: shimDirectory) } - try fileManager.setAttributes([.posixPermissions: 0o700], ofItemAtPath: shimDirectory.path) } catch { return nil } + guard privateDirectoryCheck.makePrivate(atPath: shimDirectory.path) else { return nil } return TerminalSurfaceAgentCommandShimSet( directoryPath: shimDirectory.path, shims: shims diff --git a/Packages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/TerminalSurfaceCommandShimPermissionsTests.swift b/Packages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/TerminalSurfaceCommandShimPermissionsTests.swift index 25d1f3aa8dc1..1bc2fa84fbc4 100644 --- a/Packages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/TerminalSurfaceCommandShimPermissionsTests.swift +++ b/Packages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/TerminalSurfaceCommandShimPermissionsTests.swift @@ -94,6 +94,75 @@ struct TerminalSurfaceCommandShimPermissionsTests { } } + @Test("Install skips a symlinked shim parent") + func installSkipsSymlinkedShimParent() throws { + let fileManager = FileManager.default + let root = URL.temporaryDirectory.appending( + path: "TerminalSurfaceCommandShimSymlinkParentTests-\(UUID().uuidString)", + directoryHint: .isDirectory + ) + let temporaryDirectory = root.appending(path: "tmp", directoryHint: .isDirectory) + let parentDirectory = temporaryDirectory.appending( + path: "cmux-cli-shims", + directoryHint: .isDirectory + ) + let linkTarget = root.appending(path: "elsewhere", directoryHint: .isDirectory) + let wrapperDirectory = try makeClaudeWrapperDirectory(in: root) + defer { try? fileManager.removeItem(at: root) } + + for directory in [temporaryDirectory, linkTarget] { + try fileManager.createDirectory(at: directory, withIntermediateDirectories: true) + } + try fileManager.setAttributes([.posixPermissions: 0o755], ofItemAtPath: linkTarget.path) + try fileManager.createSymbolicLink(at: parentDirectory, withDestinationURL: linkTarget) + + let shims = TerminalSurface.installAgentCommandShimsIfPossible( + wrapperDirectoryURL: wrapperDirectory, + surfaceId: UUID(), + temporaryDirectory: temporaryDirectory, + fileManager: fileManager + ) + #expect(shims == nil) + #expect(try fileManager.contentsOfDirectory(atPath: linkTarget.path).isEmpty) + #expect(try posixPermissions(atPath: linkTarget.path) == 0o755) + } + + @Test("Install skips a symlinked surface directory") + func installSkipsSymlinkedSurfaceDirectory() throws { + let fileManager = FileManager.default + let root = URL.temporaryDirectory.appending( + path: "TerminalSurfaceCommandShimSymlinkSurfaceTests-\(UUID().uuidString)", + directoryHint: .isDirectory + ) + let temporaryDirectory = root.appending(path: "tmp", directoryHint: .isDirectory) + let parentDirectory = temporaryDirectory.appending( + path: "cmux-cli-shims", + directoryHint: .isDirectory + ) + let surfaceId = UUID() + let shimDirectory = parentDirectory.appending(path: surfaceId.uuidString, directoryHint: .isDirectory) + let linkTarget = root.appending(path: "elsewhere", directoryHint: .isDirectory) + let wrapperDirectory = try makeClaudeWrapperDirectory(in: root) + defer { try? fileManager.removeItem(at: root) } + + for directory in [parentDirectory, linkTarget] { + try fileManager.createDirectory(at: directory, withIntermediateDirectories: true) + } + try fileManager.setAttributes([.posixPermissions: 0o700], ofItemAtPath: parentDirectory.path) + try fileManager.setAttributes([.posixPermissions: 0o755], ofItemAtPath: linkTarget.path) + try fileManager.createSymbolicLink(at: shimDirectory, withDestinationURL: linkTarget) + + let shims = TerminalSurface.installAgentCommandShimsIfPossible( + wrapperDirectoryURL: wrapperDirectory, + surfaceId: surfaceId, + temporaryDirectory: temporaryDirectory, + fileManager: fileManager + ) + #expect(shims == nil) + #expect(try fileManager.contentsOfDirectory(atPath: linkTarget.path).isEmpty) + #expect(try posixPermissions(atPath: linkTarget.path) == 0o755) + } + @Test("Claude integration toggle controls the per-surface shim") func claudeIntegrationToggleControlsPerSurfaceShim() throws { let fileManager = FileManager.default @@ -401,6 +470,21 @@ struct TerminalSurfaceCommandShimPermissionsTests { #expect(scanCounter.value == 2) } + private func makeClaudeWrapperDirectory(in root: URL) throws -> URL { + let fileManager = FileManager.default + let wrapperDirectory = root.appending(path: "bin", directoryHint: .isDirectory) + let wrapper = wrapperDirectory.appending(path: "cmux-claude-wrapper", directoryHint: .notDirectory) + try fileManager.createDirectory(at: wrapperDirectory, withIntermediateDirectories: true) + try "#!/bin/sh\nexit 0\n".write(to: wrapper, atomically: true, encoding: .utf8) + try fileManager.setAttributes([.posixPermissions: 0o700], ofItemAtPath: wrapper.path) + return wrapperDirectory + } + + private func posixPermissions(atPath path: String) throws -> UInt16 { + let attributes = try FileManager.default.attributesOfItem(atPath: path) + return try #require(attributes[.posixPermissions] as? NSNumber).uint16Value + } + private func capturedArguments( from shim: TerminalSurfaceAgentCommandShim, logURL: URL, diff --git a/Resources/shell-integration/nushell/cmux-nushell-bootstrap.nu b/Resources/shell-integration/nushell/cmux-nushell-bootstrap.nu index 037d43f00454..094ae8b76a97 100644 --- a/Resources/shell-integration/nushell/cmux-nushell-bootstrap.nu +++ b/Resources/shell-integration/nushell/cmux-nushell-bootstrap.nu @@ -7,10 +7,18 @@ # # User config commonly rebuilds PATH with its own prepends, which shadows the # per-surface cmux-cli-shims directory cmux front-loaded at spawn (the claude -# wrapper that injects session tracking + notification hooks). Re-front every -# shim entry, preserving the relative order of everything else — nushell's +# wrapper that injects session tracking + notification hooks). Re-front that +# directory, preserving the relative order of everything else — nushell's # equivalent of the zsh integration's "keep the bundled wrapper ahead of later -# PATH mutations". Also normalizes PATH back to a list when user config left -# it a colon-joined string. -def --env _cmux_refront_cli_shims [] { if ($env.CMUX_SURFACE_ID? | default "") == "" { return }; let raw = ($env.PATH? | default []); let entries = if ($raw | describe | str starts-with "list") { $raw } else { $raw | split row (char esep) }; let shims = ($entries | where {|p| $p | str contains "cmux-cli-shims" }); $env.PATH = ($shims ++ ($entries | where {|p| not ($p | str contains "cmux-cli-shims") })) } +# PATH mutations". The app sets $CMUX_AGENT_COMMAND_SHIM_ROOT whenever any +# agent shim exists and $CMUX_CLAUDE_WRAPPER_SHIM_ROOT only for the Claude +# shim, so both are candidates. Also normalizes PATH back to a list when user +# config left it a colon-joined string. +# +# The shim root can sit in a shared temporary directory, so it moves only when +# it is a real directory (not a symlink) owned by this user. nushell has no +# owner check of its own; stat, which does not follow a symlink here, reports +# the type and owner. When that can't be confirmed, PATH keeps its order. +def _cmux_owned_shim_root [root: string] { if not ($root | str starts-with "/") { return false }; try { let found = (^/usr/bin/stat -f "%HT:%u" -- $root | complete); let uid = (^/usr/bin/id -u | complete); $found.exit_code == 0 and $uid.exit_code == 0 and ($found.stdout | str trim) == $"Directory:($uid.stdout | str trim)" } catch { false } } +def --env _cmux_refront_cli_shims [] { if ($env.CMUX_SURFACE_ID? | default "") == "" { return }; let raw = ($env.PATH? | default []); let entries = if ($raw | describe | str starts-with "list") { $raw } else { $raw | split row (char esep) }; let roots = ([($env.CMUX_AGENT_COMMAND_SHIM_ROOT? | default ""), ($env.CMUX_CLAUDE_WRAPPER_SHIM_ROOT? | default "")] | uniq | where {|r| ($r in $entries) and (_cmux_owned_shim_root $r) }); $env.PATH = ($roots ++ ($entries | where {|p| $p not-in $roots })) } _cmux_refront_cli_shims diff --git a/Sources/WorkspaceInitialCommandLoginShell.swift b/Sources/WorkspaceInitialCommandLoginShell.swift index bf32baa24ed3..60856906027b 100644 --- a/Sources/WorkspaceInitialCommandLoginShell.swift +++ b/Sources/WorkspaceInitialCommandLoginShell.swift @@ -28,9 +28,11 @@ enum WorkspaceInitialCommandLoginShell { /// Login profiles can prepend other tool directories (Homebrew's `shellenv` puts /// `/opt/homebrew/bin` first) ahead of the per-surface shim directory that cmux /// seeds into the spawned PATH, which would route `claude`/`codex` around cmux's - /// wrapper hooks. The payload therefore re-prepends the shim directory - /// unconditionally after profiles run; a duplicate PATH entry is harmless and - /// matches what interactive shell integration already produces. + /// wrapper hooks. The payload therefore re-prepends the shim directory after + /// profiles run; a duplicate PATH entry is harmless and matches what interactive + /// shell integration already produces. The directory can sit in a shared + /// temporary directory, so it is prepended only when it is a real directory + /// (not a symlink) owned by this user. static func wrap(_ command: String, userShell: String?) -> String { var shellPath: String if let userShell, userShell.hasPrefix("/") { @@ -43,18 +45,18 @@ enum WorkspaceInitialCommandLoginShell { switch (shellPath as NSString).lastPathComponent { case "fish": payload = """ - if test -n "$CMUX_CLAUDE_WRAPPER_SHIM_ROOT"; and test -d "$CMUX_CLAUDE_WRAPPER_SHIM_ROOT"; set -gx PATH "$CMUX_CLAUDE_WRAPPER_SHIM_ROOT" $PATH; end + if test -n "$CMUX_CLAUDE_WRAPPER_SHIM_ROOT"; and test -d "$CMUX_CLAUDE_WRAPPER_SHIM_ROOT"; and not test -L "$CMUX_CLAUDE_WRAPPER_SHIM_ROOT"; and test -O "$CMUX_CLAUDE_WRAPPER_SHIM_ROOT"; set -gx PATH "$CMUX_CLAUDE_WRAPPER_SHIM_ROOT" $PATH; end \(command) """ case "zsh", "bash", "sh", "ksh", "dash": payload = """ - if [ -n "${CMUX_CLAUDE_WRAPPER_SHIM_ROOT:-}" ] && [ -d "${CMUX_CLAUDE_WRAPPER_SHIM_ROOT}" ]; then PATH="${CMUX_CLAUDE_WRAPPER_SHIM_ROOT}${PATH:+:$PATH}"; export PATH; fi + \(posixShimRootPrepend) \(command) """ default: shellPath = "/bin/zsh" payload = """ - if [ -n "${CMUX_CLAUDE_WRAPPER_SHIM_ROOT:-}" ] && [ -d "${CMUX_CLAUDE_WRAPPER_SHIM_ROOT}" ]; then PATH="${CMUX_CLAUDE_WRAPPER_SHIM_ROOT}${PATH:+:$PATH}"; export PATH; fi + \(posixShimRootPrepend) \(command) """ } @@ -62,6 +64,8 @@ enum WorkspaceInitialCommandLoginShell { return "\(shellSingleQuoted(shellPath)) -lc \(shellSingleQuoted(payload))" } + private static let posixShimRootPrepend = #"if [ -n "${CMUX_CLAUDE_WRAPPER_SHIM_ROOT:-}" ] && [ -d "${CMUX_CLAUDE_WRAPPER_SHIM_ROOT}" ] && [ ! -L "${CMUX_CLAUDE_WRAPPER_SHIM_ROOT}" ] && [ -O "${CMUX_CLAUDE_WRAPPER_SHIM_ROOT}" ]; then PATH="${CMUX_CLAUDE_WRAPPER_SHIM_ROOT}${PATH:+:$PATH}"; export PATH; fi"# + private static func shellSingleQuoted(_ value: String) -> String { "'" + value.replacingOccurrences(of: "'", with: "'\"'\"'") + "'" } diff --git a/cmuxTests/WorkspaceCreateWorkingDirectoryTests.swift b/cmuxTests/WorkspaceCreateWorkingDirectoryTests.swift index 1f9ad1808413..927e5e572f0f 100644 --- a/cmuxTests/WorkspaceCreateWorkingDirectoryTests.swift +++ b/cmuxTests/WorkspaceCreateWorkingDirectoryTests.swift @@ -117,7 +117,7 @@ import Testing @Test func workspaceInitialCommandWrapsZshExactly() { let actual = WorkspaceInitialCommandLoginShell.wrap("echo zsh", userShell: "/bin/zsh") let expected = """ - '/bin/zsh' -lc 'if [ -n "${CMUX_CLAUDE_WRAPPER_SHIM_ROOT:-}" ] && [ -d "${CMUX_CLAUDE_WRAPPER_SHIM_ROOT}" ]; then PATH="${CMUX_CLAUDE_WRAPPER_SHIM_ROOT}${PATH:+:$PATH}"; export PATH; fi + '/bin/zsh' -lc 'if [ -n "${CMUX_CLAUDE_WRAPPER_SHIM_ROOT:-}" ] && [ -d "${CMUX_CLAUDE_WRAPPER_SHIM_ROOT}" ] && [ ! -L "${CMUX_CLAUDE_WRAPPER_SHIM_ROOT}" ] && [ -O "${CMUX_CLAUDE_WRAPPER_SHIM_ROOT}" ]; then PATH="${CMUX_CLAUDE_WRAPPER_SHIM_ROOT}${PATH:+:$PATH}"; export PATH; fi echo zsh' """ @@ -127,7 +127,7 @@ import Testing @Test func workspaceInitialCommandWrapsBashExactly() { let actual = WorkspaceInitialCommandLoginShell.wrap("echo bash", userShell: "/bin/bash") let expected = """ - '/bin/bash' -lc 'if [ -n "${CMUX_CLAUDE_WRAPPER_SHIM_ROOT:-}" ] && [ -d "${CMUX_CLAUDE_WRAPPER_SHIM_ROOT}" ]; then PATH="${CMUX_CLAUDE_WRAPPER_SHIM_ROOT}${PATH:+:$PATH}"; export PATH; fi + '/bin/bash' -lc 'if [ -n "${CMUX_CLAUDE_WRAPPER_SHIM_ROOT:-}" ] && [ -d "${CMUX_CLAUDE_WRAPPER_SHIM_ROOT}" ] && [ ! -L "${CMUX_CLAUDE_WRAPPER_SHIM_ROOT}" ] && [ -O "${CMUX_CLAUDE_WRAPPER_SHIM_ROOT}" ]; then PATH="${CMUX_CLAUDE_WRAPPER_SHIM_ROOT}${PATH:+:$PATH}"; export PATH; fi echo bash' """ @@ -137,7 +137,7 @@ import Testing @Test func workspaceInitialCommandWrapsFishExactly() { let actual = WorkspaceInitialCommandLoginShell.wrap("echo fish", userShell: "/usr/local/bin/fish") let expected = """ - '/usr/local/bin/fish' -lc 'if test -n "$CMUX_CLAUDE_WRAPPER_SHIM_ROOT"; and test -d "$CMUX_CLAUDE_WRAPPER_SHIM_ROOT"; set -gx PATH "$CMUX_CLAUDE_WRAPPER_SHIM_ROOT" $PATH; end + '/usr/local/bin/fish' -lc 'if test -n "$CMUX_CLAUDE_WRAPPER_SHIM_ROOT"; and test -d "$CMUX_CLAUDE_WRAPPER_SHIM_ROOT"; and not test -L "$CMUX_CLAUDE_WRAPPER_SHIM_ROOT"; and test -O "$CMUX_CLAUDE_WRAPPER_SHIM_ROOT"; set -gx PATH "$CMUX_CLAUDE_WRAPPER_SHIM_ROOT" $PATH; end echo fish' """ @@ -147,7 +147,7 @@ import Testing @Test func workspaceInitialCommandFallsBackToZshForNilShell() { let actual = WorkspaceInitialCommandLoginShell.wrap("echo nil", userShell: nil) let expected = """ - '/bin/zsh' -lc 'if [ -n "${CMUX_CLAUDE_WRAPPER_SHIM_ROOT:-}" ] && [ -d "${CMUX_CLAUDE_WRAPPER_SHIM_ROOT}" ]; then PATH="${CMUX_CLAUDE_WRAPPER_SHIM_ROOT}${PATH:+:$PATH}"; export PATH; fi + '/bin/zsh' -lc 'if [ -n "${CMUX_CLAUDE_WRAPPER_SHIM_ROOT:-}" ] && [ -d "${CMUX_CLAUDE_WRAPPER_SHIM_ROOT}" ] && [ ! -L "${CMUX_CLAUDE_WRAPPER_SHIM_ROOT}" ] && [ -O "${CMUX_CLAUDE_WRAPPER_SHIM_ROOT}" ]; then PATH="${CMUX_CLAUDE_WRAPPER_SHIM_ROOT}${PATH:+:$PATH}"; export PATH; fi echo nil' """ @@ -157,7 +157,7 @@ import Testing @Test func workspaceInitialCommandFallsBackToZshForUnknownShell() { let actual = WorkspaceInitialCommandLoginShell.wrap("echo unknown", userShell: "/opt/weird/nu") let expected = """ - '/bin/zsh' -lc 'if [ -n "${CMUX_CLAUDE_WRAPPER_SHIM_ROOT:-}" ] && [ -d "${CMUX_CLAUDE_WRAPPER_SHIM_ROOT}" ]; then PATH="${CMUX_CLAUDE_WRAPPER_SHIM_ROOT}${PATH:+:$PATH}"; export PATH; fi + '/bin/zsh' -lc 'if [ -n "${CMUX_CLAUDE_WRAPPER_SHIM_ROOT:-}" ] && [ -d "${CMUX_CLAUDE_WRAPPER_SHIM_ROOT}" ] && [ ! -L "${CMUX_CLAUDE_WRAPPER_SHIM_ROOT}" ] && [ -O "${CMUX_CLAUDE_WRAPPER_SHIM_ROOT}" ]; then PATH="${CMUX_CLAUDE_WRAPPER_SHIM_ROOT}${PATH:+:$PATH}"; export PATH; fi echo unknown' """ @@ -168,7 +168,7 @@ import Testing let command = "printf 'hello'\necho done" let actual = WorkspaceInitialCommandLoginShell.wrap(command, userShell: "/bin/zsh") let expected = """ - '/bin/zsh' -lc 'if [ -n "${CMUX_CLAUDE_WRAPPER_SHIM_ROOT:-}" ] && [ -d "${CMUX_CLAUDE_WRAPPER_SHIM_ROOT}" ]; then PATH="${CMUX_CLAUDE_WRAPPER_SHIM_ROOT}${PATH:+:$PATH}"; export PATH; fi + '/bin/zsh' -lc 'if [ -n "${CMUX_CLAUDE_WRAPPER_SHIM_ROOT:-}" ] && [ -d "${CMUX_CLAUDE_WRAPPER_SHIM_ROOT}" ] && [ ! -L "${CMUX_CLAUDE_WRAPPER_SHIM_ROOT}" ] && [ -O "${CMUX_CLAUDE_WRAPPER_SHIM_ROOT}" ]; then PATH="${CMUX_CLAUDE_WRAPPER_SHIM_ROOT}${PATH:+:$PATH}"; export PATH; fi printf '"'"'hello'"'"' echo done' """ diff --git a/tests/fixtures/WorkspaceInitialCommandLoginShellFixture.swift b/tests/fixtures/WorkspaceInitialCommandLoginShellFixture.swift new file mode 100644 index 000000000000..87754242595c --- /dev/null +++ b/tests/fixtures/WorkspaceInitialCommandLoginShellFixture.swift @@ -0,0 +1,11 @@ +import Foundation + +/// Prints the login-shell wrapper for a workspace command: `SHELL COMMAND`. +@main +struct WorkspaceInitialCommandLoginShellFixture { + static func main() { + let arguments = CommandLine.arguments + guard arguments.count == 3 else { exit(2) } + print(WorkspaceInitialCommandLoginShell.wrap(arguments[2], userShell: arguments[1])) + } +} diff --git a/tests/test-execution.toml b/tests/test-execution.toml index 31ec1b661819..aae43da65b28 100644 --- a/tests/test-execution.toml +++ b/tests/test-execution.toml @@ -286,6 +286,10 @@ lane = "macos-shell" path = "tests/test_shell_no_git_watch.py" lane = "macos-shell" +[[test]] +path = "tests/test_workspace_initial_command_shim_root_owner.py" +lane = "macos-shell" + [[test]] path = "tests/test_claude_hook_stop_last_assistant.py" lane = "macos-cli-no-socket-post-fish" diff --git a/tests/test_nushell_shim_path_refront.py b/tests/test_nushell_shim_path_refront.py index cd83fc5f12f1..516e6dbc8651 100644 --- a/tests/test_nushell_shim_path_refront.py +++ b/tests/test_nushell_shim_path_refront.py @@ -24,6 +24,11 @@ 2. With the bootstrap applied, ``which claude`` resolves to the shim again. 3. Actually running ``claude`` executes the shim, arguments intact. 4. The bootstrap normalizes a user config that left ``PATH`` a string. +5. Only the surface's shim root moves, and only when it is a real directory + this user owns: a symlinked root, a root another user owns and any other + ``cmux-cli-shims`` entry stay where they are. +6. The shim root still moves when Claude integration is off, since the + Codex, Pi, Amp and Hermes shims live there too. It is deterministic: no PTY, no sleeps, no network. Skips loudly when ``nu`` is not installed locally, but fails when ``CI`` is set so the suite can never @@ -98,8 +103,11 @@ def _write_executable(path: Path, contents: str) -> None: path.chmod(0o755) -def _make_sandbox(tmp: Path, env_nu_body: str) -> dict: - """Build an isolated HOME/XDG tree with a decoy claude, a shim claude, and the env cmux would spawn with.""" +def _make_sandbox(tmp: Path, env_nu_body: str, symlinked_root: bool = False) -> dict: + """Build an isolated HOME/XDG tree with a decoy claude, a shim claude, and the env cmux would spawn with. + + With `symlinked_root`, the shim root is a symlink to a directory holding the shim claude. + """ home = tmp / "home" xdg = tmp / "xdg" nushell_config = xdg / "nushell" @@ -117,7 +125,13 @@ def _make_sandbox(tmp: Path, env_nu_body: str) -> dict: ) shim_dir = tmp / "cmux-cli-shims" / SURFACE_ID - shim_dir.mkdir(parents=True) + if symlinked_root: + target = tmp / "elsewhere" + target.mkdir() + shim_dir.parent.mkdir() + shim_dir.symlink_to(target, target_is_directory=True) + else: + shim_dir.mkdir(parents=True) _write_executable( shim_dir / "claude", '#!/bin/sh\nprintf \'SHIM-CLAUDE %s\\n\' "$*"\n', @@ -136,6 +150,9 @@ def _make_sandbox(tmp: Path, env_nu_body: str) -> dict: [str(shim_dir), "/usr/bin", "/bin", "/usr/sbin", "/sbin"] ), "CMUX_SURFACE_ID": SURFACE_ID, + # The app sets both when the Claude shim is installed. + "CMUX_AGENT_COMMAND_SHIM_ROOT": str(shim_dir), + "CMUX_CLAUDE_WRAPPER_SHIM_ROOT": str(shim_dir), } ) return {"env": env, "decoy_dir": decoy_dir, "shim_dir": shim_dir} @@ -162,6 +179,12 @@ def _run_nu(nu: str, env: dict, script: str) -> subprocess.CompletedProcess: """ +def _path_entries(proc: subprocess.CompletedProcess) -> list: + """Split the colon-joined PATH a probe printed on its last line.""" + lines = [line for line in proc.stdout.splitlines() if line.strip()] + return lines[-1].split(os.pathsep) if lines else [] + + def _debug(proc: subprocess.CompletedProcess) -> str: """Render process output for assertion failure messages.""" return ( @@ -281,10 +304,129 @@ def test_nushell_bootstrap_noops_outside_cmux() -> None: ) +def test_nushell_bootstrap_skips_a_symlinked_shim_root() -> None: + nu = _require_nu() + if nu is None: + return + one_liner = _bootstrap_one_liner() + + with tempfile.TemporaryDirectory(prefix="cmux-nu-symlink-") as td: + tmp = Path(td) + sandbox = _make_sandbox( + tmp, + _DECOY_PREPEND_ENV_NU.replace("__DECOY_DIR__", str(tmp / "decoy-bin")), + symlinked_root=True, + ) + env = sandbox["env"] + decoy_claude = str(sandbox["decoy_dir"] / "claude") + + ran = _run_nu(nu, env, one_liner + "; which claude | get 0.path") + assert ran.returncode == 0, "bootstrap run failed" + _debug(ran) + assert ran.stdout.strip() == decoy_claude, ( + "bootstrap must not move a symlinked shim root ahead of the user's " + f"PATH (got {ran.stdout.strip()!r})" + _debug(ran) + ) + + +def test_nushell_bootstrap_skips_a_shim_root_another_user_owns() -> None: + nu = _require_nu() + if nu is None: + return + if os.geteuid() == 0: + print("SKIP: root owns the system directory used as another user's shim root") + return + one_liner = _bootstrap_one_liner() + other_owner = "/usr/share" + assert os.stat(other_owner).st_uid != os.geteuid() + + with tempfile.TemporaryDirectory(prefix="cmux-nu-owner-") as td: + tmp = Path(td) + sandbox = _make_sandbox( + tmp, + _DECOY_PREPEND_ENV_NU.replace("__DECOY_DIR__", str(tmp / "decoy-bin")), + ) + env = sandbox["env"] + env["CMUX_AGENT_COMMAND_SHIM_ROOT"] = other_owner + env["CMUX_CLAUDE_WRAPPER_SHIM_ROOT"] = other_owner + env["PATH"] = os.pathsep.join([other_owner, "/usr/bin", "/bin", "/usr/sbin", "/sbin"]) + decoy_dir = str(sandbox["decoy_dir"]) + + ran = _run_nu(nu, env, one_liner + "; $env.PATH | str join (char esep)") + assert ran.returncode == 0, "bootstrap run failed" + _debug(ran) + entries = _path_entries(ran) + assert decoy_dir in entries and other_owner in entries, ( + f"PATH lost an entry (got {entries!r})" + _debug(ran) + ) + assert entries.index(decoy_dir) < entries.index(other_owner), ( + "bootstrap must not move a shim root another user owns ahead of " + f"the user's PATH (got {entries!r})" + _debug(ran) + ) + + +def test_nushell_bootstrap_moves_only_the_shim_root() -> None: + nu = _require_nu() + if nu is None: + return + one_liner = _bootstrap_one_liner() + + with tempfile.TemporaryDirectory(prefix="cmux-nu-only-root-") as td: + tmp = Path(td) + sandbox = _make_sandbox( + tmp, + _DECOY_PREPEND_ENV_NU.replace("__DECOY_DIR__", str(tmp / "decoy-bin")), + ) + env = sandbox["env"] + shim_dir = str(sandbox["shim_dir"]) + decoy_dir = str(sandbox["decoy_dir"]) + other_shims = tmp / "cmux-cli-shims" / "other-surface" + other_shims.mkdir() + env["PATH"] = os.pathsep.join( + [shim_dir, str(other_shims), "/usr/bin", "/bin", "/usr/sbin", "/sbin"] + ) + + ran = _run_nu(nu, env, one_liner + "; $env.PATH | str join (char esep)") + assert ran.returncode == 0, "bootstrap run failed" + _debug(ran) + watched = {shim_dir, decoy_dir, str(other_shims)} + order = [entry for entry in _path_entries(ran) if entry in watched] + assert order == [shim_dir, decoy_dir, str(other_shims)], ( + "bootstrap must move only the shim root and keep other entries in " + f"their original order (got {order!r})" + _debug(ran) + ) + + +def test_nushell_bootstrap_refronts_shim_root_without_claude_integration() -> None: + nu = _require_nu() + if nu is None: + return + one_liner = _bootstrap_one_liner() + + with tempfile.TemporaryDirectory(prefix="cmux-nu-no-claude-") as td: + tmp = Path(td) + sandbox = _make_sandbox( + tmp, + _DECOY_PREPEND_ENV_NU.replace("__DECOY_DIR__", str(tmp / "decoy-bin")), + ) + env = sandbox["env"] + # Without the Claude shim, the app sets only the shared agent root. + env.pop("CMUX_CLAUDE_WRAPPER_SHIM_ROOT", None) + shim_claude = str(sandbox["shim_dir"] / "claude") + + ran = _run_nu(nu, env, one_liner + "; which claude | get 0.path") + assert ran.returncode == 0, "bootstrap run failed" + _debug(ran) + assert ran.stdout.strip() == shim_claude, ( + "bootstrap must re-front the agent shim root when Claude " + f"integration is off (got {ran.stdout.strip()!r})" + _debug(ran) + ) + + if __name__ == "__main__": test_nushell_bootstrap_refronts_shim_over_user_path_prepends() test_nushell_bootstrap_normalizes_string_path() test_nushell_bootstrap_noops_outside_cmux() + test_nushell_bootstrap_skips_a_symlinked_shim_root() + test_nushell_bootstrap_skips_a_shim_root_another_user_owns() + test_nushell_bootstrap_moves_only_the_shim_root() + test_nushell_bootstrap_refronts_shim_root_without_claude_integration() if _find_nu() is None: print("SKIP: nushell (nu) not found; nothing was verified") else: diff --git a/tests/test_workspace_initial_command_shim_root_owner.py b/tests/test_workspace_initial_command_shim_root_owner.py new file mode 100755 index 000000000000..9171b08782b2 --- /dev/null +++ b/tests/test_workspace_initial_command_shim_root_owner.py @@ -0,0 +1,111 @@ +#!/usr/bin/env python3 +"""A workspace's initial command puts the Claude shim root first on PATH only +when that root is a real directory this user owns. + +Compiles the app's login-shell wrapper with a small driver and runs the +wrapped command in each available login shell. No app, socket or CLI build. +""" + +from __future__ import annotations + +import os +from pathlib import Path +import shutil +import subprocess +import tempfile +import unittest + +ROOT = Path(__file__).resolve().parents[1] +POSIX_SHELLS = ["zsh", "bash", "sh", "ksh", "dash"] +PRINT_PATH = {"posix": "printf '%s\\n' \"$PATH\"", "fish": "string join : $PATH"} + + +def available_shells() -> dict[str, str]: + """Maps each shell path to the kind of command it runs.""" + shells = {f"/bin/{name}": "posix" for name in POSIX_SHELLS if os.access(f"/bin/{name}", os.X_OK)} + fish = shutil.which("fish") + if fish is not None: + shells[fish] = "fish" + return shells + + +class WorkspaceInitialCommandShimRootOwner(unittest.TestCase): + @classmethod + def setUpClass(cls) -> None: + cls.temp = tempfile.TemporaryDirectory(prefix="cmux-login-shell-test-") + cls.addClassCleanup(cls.temp.cleanup) + cls.binary = Path(cls.temp.name) / "login-shell-fixture" + build = subprocess.run([ + "xcrun", "swiftc", "-swift-version", "5", + "-module-cache-path", str(Path(cls.temp.name) / "cache"), + str(ROOT / "Sources/WorkspaceInitialCommandLoginShell.swift"), + str(ROOT / "tests/fixtures/WorkspaceInitialCommandLoginShellFixture.swift"), + "-o", str(cls.binary), + ], capture_output=True, text=True, timeout=300) + if build.returncode: + raise RuntimeError(build.stderr) + cls.shells = available_shells() + + def setUp(self) -> None: + self.sandbox = Path(tempfile.mkdtemp(prefix="case-", dir=self.temp.name)) + self.home = self.sandbox / "home" + self.home.mkdir() + + def path_entries(self, shell: str, kind: str, shim_root: str) -> list[str]: + wrapped = subprocess.run([str(self.binary), shell, PRINT_PATH[kind]], + capture_output=True, text=True, timeout=30, check=True).stdout[:-1] + env = { + "HOME": str(self.home), + "PATH": "/usr/bin:/bin", + "TMPDIR": str(self.sandbox), + "XDG_CONFIG_HOME": str(self.home / ".config"), + "XDG_DATA_HOME": str(self.home / ".local/share"), + "CMUX_CLAUDE_WRAPPER_SHIM_ROOT": shim_root, + } + result = subprocess.run(["/bin/sh", "-c", wrapped], env=env, cwd=self.home, + capture_output=True, text=True, timeout=60) + self.assertEqual(result.returncode, 0, f"{shell}: {result.stderr}") + lines = result.stdout.strip().splitlines() + self.assertTrue(lines, f"{shell} printed no PATH: {result.stderr}") + return lines[-1].split(":") + + def assert_prepended(self, shim_root: str) -> None: + for shell, kind in self.shells.items(): + with self.subTest(shell=shell): + self.assertEqual(self.path_entries(shell, kind, shim_root)[0], shim_root) + + def assert_not_on_path(self, shim_root: str) -> None: + for shell, kind in self.shells.items(): + with self.subTest(shell=shell): + self.assertNotIn(shim_root, self.path_entries(shell, kind, shim_root)) + + def test_owned_directory_is_prepended(self) -> None: + shim_root = self.sandbox / "shims" + shim_root.mkdir(mode=0o700) + self.assert_prepended(str(shim_root)) + + def test_symlink_is_not_prepended(self) -> None: + target = self.sandbox / "target" + target.mkdir(mode=0o700) + shim_root = self.sandbox / "shims" + os.symlink(target, shim_root) + self.assert_not_on_path(str(shim_root)) + + def test_directory_another_user_owns_is_not_prepended(self) -> None: + if os.geteuid() == 0: + self.skipTest("root owns the system directory used here") + shim_root = "/usr/share" + self.assertNotEqual(os.stat(shim_root).st_uid, os.geteuid()) + self.assert_not_on_path(shim_root) + + def test_missing_path_is_not_prepended(self) -> None: + self.assert_not_on_path(str(self.sandbox / "missing")) + + def test_regular_file_is_not_prepended(self) -> None: + shim_root = self.sandbox / "shims" + shim_root.write_text("") + self.assert_not_on_path(str(shim_root)) + + +if __name__ == "__main__": + unittest.main()