From 15d239e9241654d97ed30c8bb334c1693a797e60 Mon Sep 17 00:00:00 2001 From: Grady Zhuo Date: Mon, 6 Jul 2026 18:50:53 +0800 Subject: [PATCH] [FIX] test isolation: tests no longer touch the real ~/.claude MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Tool.defaultConfigDir` built paths from `homeDirectoryForCurrentUser` (getpwuid — ignores env), so it always resolved to the developer's real ~/.claude/.codex/ .gemini even when `withIsolatedHome` had redirected ORRERY_HOME. Origin-takeover code symlinks at `tool.defaultConfigDir`, so running the suite repointed the real ~/.claude projects/sessions/session-env into per-test temp dirs (data loss risk when /var/folders is purged). It also made the "active account is marked" ListCommand test flaky (it reads the real ~/.claude/metadata.json). - Add `userHomeURL()` in OrreryHome — homeDirectoryForCurrentUser, overridable via `ORRERY_USER_HOME`. Route `Tool.defaultConfigDir` (and orreryHomeURL's fallback) through it. Unset in production, so behavior is unchanged there. - `withIsolatedHome` sets `ORRERY_USER_HOME` to the temp dir. A dedicated var, NOT $HOME: setting $HOME breaks macOS Keychain resolution (login keychain is located via $HOME), which would fail the credential round-trip tests. - Add TestIsolationTests asserting defaultConfigDir stays inside the temp home. Verified: full suite green (366/366), and the real ~/.claude symlinks are byte-identical before and after a full `swift test` run (no hijack). --- Sources/OrreryCore/Models/Tool.swift | 4 ++- Sources/OrreryCore/Storage/OrreryHome.swift | 19 +++++++++++-- Tests/OrreryTests/TestHelpers.swift | 12 +++++++++ Tests/OrreryTests/TestIsolationTests.swift | 30 +++++++++++++++++++++ 4 files changed, 62 insertions(+), 3 deletions(-) create mode 100644 Tests/OrreryTests/TestIsolationTests.swift diff --git a/Sources/OrreryCore/Models/Tool.swift b/Sources/OrreryCore/Models/Tool.swift index f4815ad..a413aaf 100644 --- a/Sources/OrreryCore/Models/Tool.swift +++ b/Sources/OrreryCore/Models/Tool.swift @@ -17,7 +17,9 @@ public enum Tool: String, Codable, CaseIterable, Sendable { /// The system default config directory (when not using Orrery). public var defaultConfigDir: URL { - let home = FileManager.default.homeDirectoryForCurrentUser + // userHomeURL() is overridable (ORRERY_USER_HOME) so tests can isolate + // ~/.claude etc. from the developer's real home; unchanged in production. + let home = userHomeURL() switch self { case .claude: return home.appendingPathComponent(".claude") case .codex: return home.appendingPathComponent(".codex") diff --git a/Sources/OrreryCore/Storage/OrreryHome.swift b/Sources/OrreryCore/Storage/OrreryHome.swift index 3e372e0..fffa6f6 100644 --- a/Sources/OrreryCore/Storage/OrreryHome.swift +++ b/Sources/OrreryCore/Storage/OrreryHome.swift @@ -1,11 +1,26 @@ import Foundation +/// The current user's home directory. Single source of truth for "the home dir" +/// used to build `~/.claude` etc., so tests can isolate those paths from the +/// developer's real home by setting `ORRERY_USER_HOME` (see `withIsolatedHome`). +/// +/// A dedicated override — not `$HOME` — is used deliberately: setting the OS +/// `$HOME` would break macOS Keychain resolution (it locates the login keychain +/// via `$HOME`). In production `ORRERY_USER_HOME` is unset, so this is exactly +/// `homeDirectoryForCurrentUser` and behavior is unchanged. +public func userHomeURL() -> URL { + if let override = ProcessInfo.processInfo.environment["ORRERY_USER_HOME"], + !override.isEmpty { + return URL(fileURLWithPath: override) + } + return FileManager.default.homeDirectoryForCurrentUser +} + /// Resolves the orrery home directory (`$ORRERY_HOME`, else `~/.orrery`). /// Single source of truth shared by EnvironmentStore and AccountStore. public func orreryHomeURL() -> URL { if let custom = ProcessInfo.processInfo.environment["ORRERY_HOME"] { return URL(fileURLWithPath: custom) } - return FileManager.default.homeDirectoryForCurrentUser - .appendingPathComponent(".orrery") + return userHomeURL().appendingPathComponent(".orrery") } diff --git a/Tests/OrreryTests/TestHelpers.swift b/Tests/OrreryTests/TestHelpers.swift index 11009ba..c4c3e98 100644 --- a/Tests/OrreryTests/TestHelpers.swift +++ b/Tests/OrreryTests/TestHelpers.swift @@ -23,7 +23,14 @@ func withIsolatedHome(_ body: () throws -> Void) rethrows { let savedHome = ProcessInfo.processInfo.environment["ORRERY_HOME"] let savedActiveEnv = ProcessInfo.processInfo.environment["ORRERY_ACTIVE_ENV"] + // ORRERY_USER_HOME is redirected too: `Tool.defaultConfigDir` (and other + // home-relative paths) resolve via `userHomeURL()`, which honors it. Without + // this a test that triggers origin-takeover code would symlink/write into the + // developer's real ~/.claude even though ORRERY_HOME was isolated. We use a + // dedicated var, NOT $HOME — setting $HOME breaks macOS Keychain resolution. + let savedUserHome = ProcessInfo.processInfo.environment["ORRERY_USER_HOME"] setenv("ORRERY_HOME", tmpDir.path, 1) + setenv("ORRERY_USER_HOME", tmpDir.path, 1) unsetenv("ORRERY_ACTIVE_ENV") defer { if let savedHome { @@ -31,6 +38,11 @@ func withIsolatedHome(_ body: () throws -> Void) rethrows { } else { unsetenv("ORRERY_HOME") } + if let savedUserHome { + setenv("ORRERY_USER_HOME", savedUserHome, 1) + } else { + unsetenv("ORRERY_USER_HOME") + } if let savedActiveEnv { setenv("ORRERY_ACTIVE_ENV", savedActiveEnv, 1) } else { diff --git a/Tests/OrreryTests/TestIsolationTests.swift b/Tests/OrreryTests/TestIsolationTests.swift new file mode 100644 index 0000000..5082b2a --- /dev/null +++ b/Tests/OrreryTests/TestIsolationTests.swift @@ -0,0 +1,30 @@ +import Foundation +import Testing +@testable import OrreryCore + +/// Guards against the test-isolation defect where the suite touched the real +/// ~/.claude: `Tool.defaultConfigDir` used `homeDirectoryForCurrentUser` (which +/// ignores $HOME), so origin-takeover code — which symlinks at +/// `tool.defaultConfigDir` — hijacked the developer's real ~/.claude even though +/// `withIsolatedHome` had redirected ORRERY_HOME. +@Suite("test isolation") +struct TestIsolationTests { + + @Test("withIsolatedHome isolates tool.defaultConfigDir from the real home") + func isolatesDefaultConfigDir() { + // Captured OUTSIDE isolation: homeDirectoryForCurrentUser ignores $HOME, + // so this is always the developer's real home. + let realClaude = FileManager.default.homeDirectoryForCurrentUser + .appendingPathComponent(".claude").path + + withIsolatedHome { + let isolated = Tool.claude.defaultConfigDir.path + #expect(isolated != realClaude, + "defaultConfigDir must NOT resolve to the real ~/.claude inside withIsolatedHome") + + let tmpHome = ProcessInfo.processInfo.environment["ORRERY_HOME"] ?? "" + #expect(!tmpHome.isEmpty && isolated.hasPrefix(tmpHome), + "defaultConfigDir should resolve under the isolated temp home") + } + } +}