fix(ssh): defensive ControlPath dir + sweep stale sockets

Layered hardening on top of the /tmp ControlPath move from #20:

- ensureControlDir uses POSIX mkdir(0700) + lstat instead of
  createDirectory + setAttributes. Closes the /tmp pre-creation
  TOCTOU: any local user can pre-create /tmp/scarf-ssh-<uid>, and
  the old code would silently fail to chmod a hostile dir back to
  0700 (since we wouldn't own it). Now we refuse to use a dir that
  isn't a real directory we own with mode 0700, and log via
  os.Logger.

- sweepStaleControlSockets removes ControlMaster socket files
  older than 30 minutes from controlDirPath() at app launch.
  Symmetric to sweepOrphanSnapshots — keeps /tmp/scarf-ssh-<uid>/
  from accumulating crashed-master / unclean-exit orphans
  indefinitely until reboot. The 30-min threshold (vs ControlPersist's
  10 min) ensures any concurrent Scarf instance's live sockets
  are untouched.
This commit is contained in:
Alan Wizemann
2026-04-20 15:45:20 -07:00
parent d2a447fcc4
commit 1823160546
2 changed files with 66 additions and 6 deletions
@@ -129,6 +129,7 @@ final class ServerRegistry {
var keep: Set<ServerID> = [ServerContext.local.id]
for entry in entries { keep.insert(entry.id) }
SSHTransport.sweepOrphanSnapshots(keeping: keep)
SSHTransport.sweepStaleControlSockets()
}
// MARK: - Persistence
+65 -6
View File
@@ -97,6 +97,31 @@ struct SSHTransport: ServerTransport {
}
}
/// Remove ControlMaster socket files older than `staleAfter` seconds.
///
/// Socket basenames are %C hashes (not ServerIDs), so we can't keep "still
/// registered" sockets the way `sweepOrphanSnapshots` does. But
/// `ControlPersist` is 600s anything older than 30 minutes is guaranteed
/// to be a dead orphan from a crashed master, an unclean app exit, or a
/// server removed while another Scarf instance was holding the dir.
/// Wiping these on launch keeps `/tmp/scarf-ssh-<uid>/` from accumulating
/// indefinitely until reboot, while leaving any concurrent Scarf
/// instance's live sockets (always <600s old) untouched.
static func sweepStaleControlSockets(staleAfter: TimeInterval = 1800) {
let root = controlDirPath()
guard let entries = try? FileManager.default.contentsOfDirectory(atPath: root) else { return }
let cutoff = Date().addingTimeInterval(-staleAfter)
for name in entries {
let path = root + "/" + name
guard let attrs = try? FileManager.default.attributesOfItem(atPath: path),
let mtime = attrs[.modificationDate] as? Date
else { continue }
if mtime < cutoff {
try? FileManager.default.removeItem(atPath: path)
}
}
}
/// Ask OpenSSH to shut down this host's ControlMaster socket, so the TCP
/// session isn't held open after the user removes this server. If no
/// master is currently running, `ssh -O exit` exits non-zero we ignore
@@ -140,13 +165,47 @@ struct SSHTransport: ServerTransport {
return args
}
/// Ensure the ControlMaster socket directory exists. Called before every
/// ssh invocation. Cheap `createDirectory(withIntermediateDirectories: true)`
/// is a no-op when present.
/// Ensure the ControlMaster socket directory exists, is a real directory
/// (not a symlink), is owned by us, and has mode 0700. Called before every
/// ssh invocation.
///
/// Defensive against `/tmp` pre-creation: any local user can create
/// `/tmp/scarf-ssh-<uid>` before Scarf launches. Plain `mkdir -p` plus
/// `setAttributes` would silently accept a hostile dir (since the chmod
/// fails when we don't own it, and the Foundation API swallows that). So
/// we use POSIX `mkdir` (atomic, sets perms at create time, doesn't
/// follow symlinks) and `lstat` to verify ownership when the entry
/// already exists.
nonisolated private func ensureControlDir() {
try? FileManager.default.createDirectory(atPath: controlDir, withIntermediateDirectories: true)
// 0700 so socket files aren't visible to other users on the Mac.
try? FileManager.default.setAttributes([.posixPermissions: 0o700], ofItemAtPath: controlDir)
let path = controlDir
let mkResult = path.withCString { mkdir($0, 0o700) }
if mkResult == 0 { return }
let mkErr = errno
if mkErr != EEXIST {
Self.logger.error("Failed to create ControlDir \(path, privacy: .public): errno=\(mkErr)")
return
}
var st = Darwin.stat()
let lstatResult = path.withCString { lstat($0, &st) }
guard lstatResult == 0 else {
Self.logger.error("Could not lstat existing ControlDir \(path, privacy: .public): errno=\(errno)")
return
}
guard (st.st_mode & S_IFMT) == S_IFDIR else {
Self.logger.error("ControlDir \(path, privacy: .public) exists but is not a directory (possibly a symlink) — refusing to use")
return
}
guard st.st_uid == getuid() else {
Self.logger.error("ControlDir \(path, privacy: .public) owned by uid \(st.st_uid), expected \(getuid()) — refusing to use")
return
}
if (st.st_mode & 0o777) != 0o700 {
Self.logger.warning("ControlDir \(path, privacy: .public) had mode \(String(st.st_mode & 0o777, radix: 8), privacy: .public), repairing to 700")
_ = path.withCString { chmod($0, 0o700) }
}
}
/// Shell-quote a single argument for remote execution. The remote shell