From 920c86b4f87530d80787c905063270c4005604b9 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 22 Apr 2026 22:40:30 +0000 Subject: [PATCH] M0 verification: fix two real regressions before starting M1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two bugs caught by a post-M0d audit, both of which would have bitten users before any test exercised them on Mac: 1. GatewayViewModel.swift lost its `import ScarfCore` during the M0d revert (when I moved it back to the Mac target after finding it wasn't portable). The file references ServerContext everywhere and wouldn't compile in Xcode without the import. Added back. 2. SSHTransport.sshSubprocessEnvironment() regressed in M0b. The original Mac code ran HermesFileService.enrichedEnvironment(), which tries `zsh -l -i` (login + interactive, with prompt-framework defangs) FIRST, then falls back to `zsh -l`. Most users with 1Password / Secretive / manual ssh-add export SSH_AUTH_SOCK from their `.zshrc` (interactive shell init), NOT `.zprofile`. My M0b replacement used `zsh -l` only — so it would have silently failed to find their ssh-agent socket, and SSH auth would break with "Permission denied" (exit 255) for everyone who set up their agent the normal way. Fix is a dependency-inversion injection point instead of a local shell probe: SSHTransport.environmentEnricher is a `(@Sendable () -> [String: String])?` static that the Mac target wires at launch to HermesFileService.enrichedEnvironment(). Same exact code path executed as before M0b; no duplication; iOS leaves it `nil` and falls back to ProcessInfo.processInfo.environment (Citadel will own the SSH agent on iOS in M4+, not the login shell). Tests can set a stub closure. scarfApp.init() now sets `SSHTransport.environmentEnricher = { HermesFileService.enrichedEnvironment() }` right before the existing warm-up Task. Test coverage: M0b suite gains `sshTransportEnvironmentEnricherInjection`, which pins the injection-point shape so a future refactor can't silently drop it. Audit results (for confidence before M1): - Exhaustive grep of every moved type across main target → 0 files reference ScarfCore types without `import ScarfCore` (after the GatewayVM fix). - `scarf.xcodeproj/project.pbxproj` has no stale path references (PBXFileSystemSynchronizedRootGroup auto-discovers). - `xcshareddata/xcschemes/*.xcscheme` has no stale path references. - `.build/` correctly gitignored. - Zero leftover temp scripts / `.orig` / `.bak` files. `swift test`: 52 / 52 passing on Linux. https://claude.ai/code/session_019yMRP6mwZWfzVrPTqevx2y --- .../ScarfCore/Transport/SSHTransport.swift | 91 ++++++------------- .../ScarfCoreTests/M0bTransportTests.swift | 28 ++++++ .../Gateway/ViewModels/GatewayViewModel.swift | 1 + scarf/scarf/scarfApp.swift | 8 ++ 4 files changed, 64 insertions(+), 64 deletions(-) diff --git a/scarf/Packages/ScarfCore/Sources/ScarfCore/Transport/SSHTransport.swift b/scarf/Packages/ScarfCore/Sources/ScarfCore/Transport/SSHTransport.swift index c2642cd..0a0ef3e 100644 --- a/scarf/Packages/ScarfCore/Sources/ScarfCore/Transport/SSHTransport.swift +++ b/scarf/Packages/ScarfCore/Sources/ScarfCore/Transport/SSHTransport.swift @@ -440,77 +440,40 @@ public struct SSHTransport: ServerTransport { return proc } - /// Environment for an ssh/scp subprocess: process env merged with - /// SSH_AUTH_SOCK / SSH_AGENT_PID harvested from the user's login shell. - /// Without this, GUI-launched Scarf can't reach 1Password / Secretive / - /// `ssh-add`'d keys that the user's terminal sees fine. + /// Injection point for ssh/scp subprocess environment enrichment. /// - /// **macOS-only enrichment.** On iOS there's no user login shell — SSH - /// agent is provided by the app itself (Citadel) in M4, not by a - /// `ssh-add`'d key loaded via `.zshrc`. On Linux CI there's no SSH - /// invocation actually happening, just compilation checks. Both cases - /// fall back to `ProcessInfo.processInfo.environment` verbatim. + /// On the Mac app, this is wired at startup to + /// `HermesFileService.enrichedEnvironment()` — the full two-attempt + /// login-shell probe (`zsh -l -i` with prompt defangs, fallback to + /// `zsh -l`) that harvests SSH_AUTH_SOCK + SSH_AGENT_PID from + /// 1Password / Secretive / `.zshrc`-exported agents. Without this + /// harvesting, a GUI-launched Scarf can't reach ssh-agent sockets + /// that the user's Terminal sees fine — auth fails with "Permission + /// denied" / exit 255. + /// + /// On iOS the agent comes from Citadel (M4+), not from a login shell + /// probe — leave this `nil` and iOS falls back to + /// `ProcessInfo.processInfo.environment` alone. + /// + /// Set once at app launch (startup is single-threaded). Tests may + /// inject a stub. + nonisolated(unsafe) public static var environmentEnricher: (@Sendable () -> [String: String])? + + /// Environment for an ssh/scp subprocess: process env merged with + /// anything the configured `environmentEnricher` produces. The enricher + /// only wins for keys the process env doesn't already have, so an + /// explicit `SSH_AUTH_SOCK=…` in the Xcode scheme / launchd plist + /// survives. nonisolated private static func sshSubprocessEnvironment() -> [String: String] { var env = ProcessInfo.processInfo.environment - #if os(macOS) - let shellEnv = Self.macLoginShellSSHAgent() - for key in ["SSH_AUTH_SOCK", "SSH_AGENT_PID"] { - if env[key] == nil, let value = shellEnv[key], !value.isEmpty { - env[key] = value - } + guard let enricher = Self.environmentEnricher else { return env } + let extra = enricher() + for (key, value) in extra where env[key] == nil && !value.isEmpty { + env[key] = value } - #endif return env } - /// macOS-only: probe `/bin/zsh -l -c` for `SSH_AUTH_SOCK` and - /// `SSH_AGENT_PID`. GUI-launched apps don't inherit the user's shell - /// env, so without this, `ssh` spawned from Scarf can't reach the - /// ssh-agent and authentication fails with "Permission denied" - /// (exit 255) even though terminal ssh works fine. - /// - /// Scoped down from the broader `HermesFileService.enrichedEnvironment()` - /// — we only need two vars here, no PATH/credentials harvesting — so - /// SSHTransport can live in `ScarfCore` without a main-target - /// dependency. Cached after first probe for the process lifetime. - #if os(macOS) - nonisolated private static let macLoginShellSSHAgentCache: [String: String] = { - let pipe = Pipe() - let proc = Process() - proc.executableURL = URL(fileURLWithPath: "/bin/zsh") - proc.arguments = ["-l", "-c", #"printf '%s\0%s\0%s\0%s\0' "SSH_AUTH_SOCK" "$SSH_AUTH_SOCK" "SSH_AGENT_PID" "$SSH_AGENT_PID""#] - proc.standardOutput = pipe - proc.standardError = Pipe() - do { - try proc.run() - } catch { - return [:] - } - // Bounded wait so a broken login shell doesn't hang app launch. - let deadline = Date().addingTimeInterval(3.0) - while proc.isRunning && Date() < deadline { - Thread.sleep(forTimeInterval: 0.05) - } - if proc.isRunning { - proc.terminate() - return [:] - } - let data = (try? pipe.fileHandleForReading.readToEnd()) ?? Data() - let parts = data.split(separator: 0).map { String(data: Data($0), encoding: .utf8) ?? "" } - var out: [String: String] = [:] - var i = 0 - while i + 1 < parts.count { - out[parts[i]] = parts[i + 1] - i += 2 - } - return out - }() - - nonisolated private static func macLoginShellSSHAgent() -> [String: String] { - macLoginShellSSHAgentCache - } - #endif - // MARK: - SQLite snapshot public func snapshotSQLite(remotePath: String) throws -> URL { diff --git a/scarf/Packages/ScarfCore/Tests/ScarfCoreTests/M0bTransportTests.swift b/scarf/Packages/ScarfCore/Tests/ScarfCoreTests/M0bTransportTests.swift index c677115..07d7282 100644 --- a/scarf/Packages/ScarfCore/Tests/ScarfCoreTests/M0bTransportTests.swift +++ b/scarf/Packages/ScarfCore/Tests/ScarfCoreTests/M0bTransportTests.swift @@ -229,4 +229,32 @@ import Foundation let url = try transport.snapshotSQLite(remotePath: "/tmp/some/state.db") #expect(url.path == "/tmp/some/state.db") } + + /// The Mac target wires `SSHTransport.environmentEnricher` at launch to + /// `HermesFileService.enrichedEnvironment()` so SSH subprocesses + /// inherit SSH_AUTH_SOCK from the user's login shell (1Password / + /// Secretive / `.zshrc`-exported agents). iOS leaves it `nil` (Citadel + /// owns the agent). Pin the injection-point shape — a regression here + /// would silently break ssh-agent access for GUI-launched Scarf on + /// machines where `ssh-add` lives in `.zshrc` rather than `.zprofile`. + @Test func sshTransportEnvironmentEnricherInjection() { + let previous = SSHTransport.environmentEnricher + defer { SSHTransport.environmentEnricher = previous } + + // Default (no enricher) → nothing injected. + SSHTransport.environmentEnricher = nil + + // With enricher → its keys merged into the returned env. + SSHTransport.environmentEnricher = { + ["SSH_AUTH_SOCK": "/tmp/fake.sock", "SSH_AGENT_PID": "4242"] + } + // We can't call `sshSubprocessEnvironment()` directly (it's + // private). Instead assert the injection point exists + can be + // overridden — exercising the full dispatch path is the + // integration test's job, not this unit's. + #expect(SSHTransport.environmentEnricher != nil) + let sample = SSHTransport.environmentEnricher?() + #expect(sample?["SSH_AUTH_SOCK"] == "/tmp/fake.sock") + #expect(sample?["SSH_AGENT_PID"] == "4242") + } } diff --git a/scarf/scarf/Features/Gateway/ViewModels/GatewayViewModel.swift b/scarf/scarf/Features/Gateway/ViewModels/GatewayViewModel.swift index eab0a99..bf8754f 100644 --- a/scarf/scarf/Features/Gateway/ViewModels/GatewayViewModel.swift +++ b/scarf/scarf/Features/Gateway/ViewModels/GatewayViewModel.swift @@ -1,4 +1,5 @@ import Foundation +import ScarfCore struct GatewayInfo { let pid: Int? diff --git a/scarf/scarf/scarfApp.swift b/scarf/scarf/scarfApp.swift index 1f23557..7210e20 100644 --- a/scarf/scarf/scarfApp.swift +++ b/scarf/scarf/scarfApp.swift @@ -27,6 +27,14 @@ struct ScarfApp: App { // wasn't running. Cheap: just an `ls` of the snapshots root. registry.sweepOrphanCaches() + // Wire ScarfCore's SSHTransport to the Mac-target login-shell env + // probe. Without this, `ssh`/`scp` subprocesses spawned from Scarf + // can't reach 1Password / Secretive / `.zshrc`-exported ssh-agent + // sockets and auth fails with "Permission denied" (exit 255) even + // though terminal ssh works fine. iOS leaves this unset — Citadel + // owns the agent there. + SSHTransport.environmentEnricher = { HermesFileService.enrichedEnvironment() } + // Warm up the login-shell env probe off-main at launch. Without // this, the first MainActor caller (chat preflight, OAuth flow, // signal-cli detect, etc.) blocks for 5-8 seconds while