From c05ddefdb433bd57296564e741f625f342639ecd Mon Sep 17 00:00:00 2001 From: caezium <113233555+caezium@users.noreply.github.com> Date: Tue, 4 Aug 2026 11:31:34 -0700 Subject: [PATCH 01/14] feat(security): add opt-in privileged helper for admin operations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Burrow's elevated operations run through `osascript … with administrator privileges`, which is password-only by construction: the `system.privilege.admin` right authenticates through SecurityAgent's classic mechanism, which never offers Touch ID. It also cannot be cancelled safely — killing osascript orphans the root child it spawned. This adds an opt-in `SMAppService` launch daemon as an alternative route, and leaves the osascript path in place as the fallback for anyone who declines it. The helper accepts three typed operations — scan, clean, optimize — and derives argv from the enum itself. There is no field for a path, an argument, a shell string, or an executable, so a caller that fully controls the XPC payload still cannot express "run this". Security properties, each enforced rather than documented: - Fresh authentication per root operation. The right is defined with `timeout: 0`, `shared: false`, and `allow-root: false`, so no credential survives a call, none is shared, and the root daemon cannot satisfy the right by virtue of being root. Registering the helper authorizes nothing. - The prompt is raised by the DAEMON. The client externalizes an empty, unauthenticated `AuthorizationRef`; the daemon rebuilds it and calls `AuthorizationCopyRights` with a non-empty rights set and interaction allowed. A GUI-side prompt would be cosmetic — the privileged side has to be what demands the right. - Callers are pinned by `NSXPCListener.setConnectionCodeSigningRequirement` (bundle identifier + Apple anchor + signing team), evaluated by the system against the real peer. The requirement is built at runtime from the helper's own signing information, so no team ID is hardcoded. - One authorization, one operation: operation IDs must be UUIDs and are served at most once, so a captured payload cannot be replayed. - Version skew is refused. A registered daemon outlives the app that installed it, so a build mismatch routes back to osascript instead of running as root with a stale idea of what `clean` does. - The engine is resolved relative to the helper's own executable and its signature is verified before it runs as root — never PATH, never an environment variable. Routing is a pure function: the helper is used only when the argv maps onto a typed operation, the daemon is registered and enabled, and its build matches. Anything else keeps the existing path unchanged. sign-macos-app.sh now fails closed if the helper is missing, unsigned, not hardened, or misdeclared to launchd — including a BundleProgram that points at anything other than the executable the pipeline just verified. SECURITY.md previously stated that Burrow installs no privileged helper and no XPC root service; that claim is now scoped to the default configuration and the opt-in helper's guarantees are spelled out. --- README.md | 10 +- SECURITY.md | 36 +- macos/HelperSources/HelperService.swift | 339 ++++++++++++++++++ macos/HelperSources/main.swift | 90 +++++ .../dev.caezium.Burrow.helper.plist | 43 +++ macos/Sources/MoEngine.swift | 6 +- .../HelperAuthorization.swift | 220 ++++++++++++ .../HelperCodeRequirement.swift | 155 ++++++++ .../PrivilegedHelper/HelperContract.swift | 223 ++++++++++++ .../Sources/PrivilegedHelper/HelperXPC.swift | 84 +++++ macos/Sources/PrivilegedHelperClient.swift | 326 +++++++++++++++++ macos/Sources/SettingsView.swift | 66 +++- macos/Tests/HelperAuthorizationTests.swift | 155 ++++++++ macos/Tests/HelperCodeRequirementTests.swift | 151 ++++++++ macos/Tests/HelperContractTests.swift | 194 ++++++++++ macos/Tests/PrivilegeRouteTests.swift | 134 +++++++ macos/project.yml | 53 ++- scripts/sign-macos-app.sh | 59 +++ scripts/tests/test_sign_macos_app.py | 122 +++++++ 19 files changed, 2455 insertions(+), 11 deletions(-) create mode 100644 macos/HelperSources/HelperService.swift create mode 100644 macos/HelperSources/main.swift create mode 100644 macos/Resources/LaunchDaemons/dev.caezium.Burrow.helper.plist create mode 100644 macos/Sources/PrivilegedHelper/HelperAuthorization.swift create mode 100644 macos/Sources/PrivilegedHelper/HelperCodeRequirement.swift create mode 100644 macos/Sources/PrivilegedHelper/HelperContract.swift create mode 100644 macos/Sources/PrivilegedHelper/HelperXPC.swift create mode 100644 macos/Sources/PrivilegedHelperClient.swift create mode 100644 macos/Tests/HelperAuthorizationTests.swift create mode 100644 macos/Tests/HelperCodeRequirementTests.swift create mode 100644 macos/Tests/HelperContractTests.swift create mode 100644 macos/Tests/PrivilegeRouteTests.swift diff --git a/README.md b/README.md index c1b6cdaa..8a3d2c0f 100644 --- a/README.md +++ b/README.md @@ -407,9 +407,13 @@ Burrow drives a bundled, open-source Mole engine (an MIT fork of `mo`). The hone recordings, user files, user paths, metrics, or stored IP; turn it off in Settings. PostHog delivery and its bounded sanitized retry outbox run entirely off AppKit's main thread. Full list in **[TELEMETRY.md](TELEMETRY.md)**. -- **No background root helper.** When Clean/Optimize need admin rights, macOS's - own dialog asks you and Burrow runs that one `mo` command, then exits — you - approve every elevation. +- **No background root helper by default.** When Clean/Optimize need admin + rights, macOS's own dialog asks you and Burrow runs that one `mo` command, + then exits — you approve every elevation. You can opt in to a small signed + helper so those prompts accept Touch ID; it grants no standing privilege + (every root operation still authenticates, every time), performs only scan, + clean, and optimize, and can be removed from Settings. Details in + **[SECURITY.md](SECURITY.md)**. - **Local-only surfaces:** the MCP/HTTP surfaces bind to loopback only (`127.0.0.1`) and history is stored locally. On Windows, the HTTP REST toggle disables REST endpoints but keeps the local `/mcp` bridge route available for diff --git a/SECURITY.md b/SECURITY.md index e86a4495..a1f52002 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -48,14 +48,40 @@ or notarization and changes between builds. This is the part people rightly scrutinize in cleaners. Burrow's model: -- **Burrow installs no privileged/background helper and no XPC root - service.** There is nothing persistently running as root and nothing for - another local process to connect to. +- **By default Burrow installs no privileged helper.** Out of the box there + is nothing running as root and nothing for another local process to + connect to. - When **Clean** or **Optimize** needs admin rights, **macOS's own authorization dialog** asks for your password, and Burrow runs the matching `mo` command for that single action, then exits. You see and - approve every elevation. (See `CommandRunner.runElevated` in - `macos/Sources/TaskReport.swift`.) + approve every elevation. +- **Optional privileged helper.** Settings ▸ Advanced can install a small + signed launch daemon (`SMAppService`) so those same operations can + authenticate with Touch ID instead of a password-only prompt. It is + strictly opt-in and takes its own one-time macOS approval. If you install + it: + - **It grants no standing privilege.** Installing the helper authorizes + nothing. Every operation that runs as root requires a fresh + authentication — no grace period, no cached credential, no + "authenticate once for this launch". This is enforced by the + authorization right's own definition (`timeout: 0`, `shared: false`, + `allow-root: false`), not merely by convention. + - **It cannot be asked to run anything else.** The helper accepts three + typed operations — scan, clean, optimize — and derives the command line + itself. There is no field in its API for a path, an argument, a shell + string, or an executable, so a caller that fully controls the message + still cannot express "run this". + - **Only Burrow can talk to it.** The daemon pins its callers to Burrow's + bundle identifier and signing team via the XPC connection's code-signing + requirement, so another local process cannot reach it or use it to raise + a credential prompt. + - **It runs only your engine.** The binary it executes is the signed + engine inside the app bundle, resolved relative to the helper's own + path, never through `PATH` or an environment variable, and its signature + is checked before it is run as root. + - **You can remove it** from Settings, or from System Settings ▸ General ▸ + Login Items & Extensions. + - Code: `macos/Sources/PrivilegedHelper/`, `macos/HelperSources/`. - **Honest caveat:** official builds elevate the engine sealed inside the Developer ID signed app. A source build can fall back to an external `mo`; if it does, that executable is only as trustworthy as its install location diff --git a/macos/HelperSources/HelperService.swift b/macos/HelperSources/HelperService.swift new file mode 100644 index 00000000..9666fc66 --- /dev/null +++ b/macos/HelperSources/HelperService.swift @@ -0,0 +1,339 @@ +// +// HelperService.swift +// BurrowHelper +// +// The root daemon. This process runs as uid 0, so every line here is written +// on the assumption that the thing talking to it is hostile until proven +// otherwise. +// +// ── The gauntlet a request runs ───────────────────────────────────────── +// Five gates, in this order, each of which fails CLOSED: +// +// 1. Connection the peer's AUDIT TOKEN must satisfy the code +// requirement (Burrow, signed by our team). PIDs are +// never used — they can be recycled and raced. +// 2. Shape the payload must decode as a `HelperRequest`. The +// operation is an enum, so an unknown verb dies here. +// 3. Freshness the operation ID must be a UUID this daemon has never +// served. One authorization, one operation. +// 4. Authorization `AuthorizationCopyRights` must GRANT the right, which +// raises the system authentication prompt. This is the +// gate, not a GUI-side prompt. +// 5. Execution argv is derived from the enum, and the executable is +// the signed engine inside our own bundle, verified +// before it is spawned. +// +// Note what is absent: there is no path at which a caller-supplied string +// becomes part of a command line. That is the property the whole design +// exists to preserve. +// + +import Foundation +import Security +import os + +// MARK: - Logging +// +// Privileged code logs to the unified log, which is world-readable. So it +// records DECISIONS, never content: no paths, no filenames, no command +// output, no authentication material, no free-form error text. Operation IDs +// are UUIDs the daemon itself validated, and every other value logged here is +// drawn from a closed enum. + +let helperLog = Logger(subsystem: "dev.caezium.Burrow.helper", category: "privileged") + +// MARK: - Engine resolution + +enum HelperEngine { + + /// The signed engine inside the app bundle that contains this helper, + /// resolved RELATIVE TO OUR OWN EXECUTABLE: + /// + /// …/Burrow.app/Contents/MacOS/BurrowHelper ← us + /// …/Burrow.app/Contents/Resources/engine/mole ← the engine + /// + /// Never `PATH`, never an environment variable, never a caller-supplied + /// path. A root process that resolves its executable through any of those + /// hands root to whoever wins the race to shadow the name — which is the + /// exact reason `MoleCLI.trustedExecutable()` already refuses PATH on the + /// osascript path. + static func bundledEnginePath() -> String? { + guard let executable = Bundle.main.executableURL?.resolvingSymlinksInPath() else { return nil } + let contents = executable // …/Contents/MacOS/BurrowHelper + .deletingLastPathComponent() // …/Contents/MacOS + .deletingLastPathComponent() // …/Contents + let engine = contents + .appendingPathComponent("Resources/engine/mole") + .standardizedFileURL + + // Belt and braces: after standardizing, the engine must still sit + // inside our own Contents directory. A symlink pointing out of the + // bundle would otherwise be followed as root. + guard engine.path.hasPrefix(contents.standardizedFileURL.path + "/") else { return nil } + guard FileManager.default.isExecutableFile(atPath: engine.path) else { return nil } + return engine.path + } + + /// Verify the engine carries our own signature before running it as root. + /// + /// The app bundle is signed as a unit, so an attacker who can rewrite the + /// engine binary has already broken the app's seal — but they may have + /// done so on a machine where Gatekeeper never re-evaluates the bundle + /// after first launch. Checking here means a tampered engine is refused at + /// the moment it would gain root, not merely at install time. + /// + /// On an ad-hoc (local development) build there is no team to pin, so the + /// check is skipped and the fact is logged. Release builds always have + /// one, and the release gate refuses to ship a helper without it. + static func verify(path: String, teamID: String?) -> Bool { + guard let teamID else { + helperLog.notice("engine signature check skipped: helper is ad-hoc signed (development build)") + return true + } + guard let requirement = HelperCodeRequirement.sameTeam(teamID: teamID) else { return false } + + var staticCode: SecStaticCode? + let url = URL(fileURLWithPath: path) as CFURL + guard SecStaticCodeCreateWithPath(url, [], &staticCode) == errSecSuccess, + let staticCode else { return false } + + var secRequirement: SecRequirement? + guard SecRequirementCreateWithString(requirement as CFString, [], &secRequirement) == errSecSuccess, + let secRequirement else { return false } + + return SecStaticCodeCheckValidity(staticCode, [], secRequirement) == errSecSuccess + } +} + +// MARK: - Running one operation + +/// Spawns the engine and streams its output back to the client. +/// +/// The daemon owns the child directly, which is the structural gain over the +/// osascript path: there, cancelling meant killing `osascript` and orphaning +/// the root child it had spawned, so the streaming flow simply had no safe +/// cancel. Here the child is ours to signal and reap. +final class HelperOperationRunner: @unchecked Sendable { + private let lock = NSLock() + private var running: [String: Process] = [:] + + /// Run `operation`'s fixed argv and block until it exits, forwarding whole + /// lines to `emit` as they arrive. + func run(operation: HelperOperation, + operationID: String, + enginePath: String, + emit: @escaping (String) -> Void) -> Int32 { + let process = Process() + process.executableURL = URL(fileURLWithPath: enginePath) + // Fixed argv, built from the enum. No caller input reaches this array. + process.arguments = operation.engineArguments + + // A deliberately minimal environment. The child is root, so anything + // inherited from the launchd context that could redirect a lookup + // (PATH, DYLD_*, the engine's own overrides) is dropped rather than + // passed through. + process.environment = [ + "PATH": "/usr/bin:/bin:/usr/sbin:/sbin", + "HOME": NSHomeDirectory(), + "LC_ALL": "C", + ] + + let outPipe = Pipe(), errPipe = Pipe() + process.standardOutput = outPipe + process.standardError = errPipe + process.standardInput = FileHandle.nullDevice + + do { + try process.run() + } catch { + helperLog.error("engine spawn failed for operation \(operationID, privacy: .public)") + return 127 + } + + lock.lock(); running[operationID] = process; lock.unlock() + defer { lock.lock(); running.removeValue(forKey: operationID); lock.unlock() } + + try? outPipe.fileHandleForWriting.close() + try? errPipe.fileHandleForWriting.close() + + // One reader per pipe, both draining to EOF before the exit status is + // read, so no output can be lost between the last write and the reap. + let splitter = HelperLineSplitter() + let group = DispatchGroup() + for handle in [outPipe.fileHandleForReading, errPipe.fileHandleForReading] { + group.enter() + DispatchQueue.global(qos: .utility).async { + while case let chunk = handle.availableData, !chunk.isEmpty { + guard let text = String(data: chunk, encoding: .utf8) else { continue } + for line in splitter.ingest(text) { emit(line) } + } + group.leave() + } + } + group.wait() + for line in splitter.flush() { emit(line) } + + process.waitUntilExit() + return process.terminationStatus + } + + /// Terminate a running operation. Returns whether anything was running. + func cancel(operationID: String) -> Bool { + lock.lock(); let process = running[operationID]; lock.unlock() + guard let process, process.isRunning else { return false } + process.terminate() + return true + } + + var hasWork: Bool { + lock.lock(); defer { lock.unlock() } + return !running.isEmpty + } +} + +/// Buffers partial reads and emits whole lines. Mirrors the GUI's splitter so +/// both elevation routes deliver output the same way. +final class HelperLineSplitter: @unchecked Sendable { + private var buffer = "" + private let lock = NSLock() + + func ingest(_ text: String) -> [String] { + lock.lock(); defer { lock.unlock() } + buffer += text + var parts = buffer.components(separatedBy: "\n") + buffer = parts.removeLast() + return parts + } + + func flush() -> [String] { + lock.lock(); defer { lock.unlock() } + let rest = buffer + buffer = "" + return rest.isEmpty ? [] : [rest] + } +} + +// MARK: - The XPC service + +final class HelperService: NSObject, BurrowHelperProtocol { + private let replayGuard = HelperReplayGuard() + private let runner = HelperOperationRunner() + private let teamID: String? + + /// The client callback for the connection currently being served. Set by + /// the listener delegate per connection. + weak var currentConnection: NSXPCConnection? + + init(teamID: String?) { + self.teamID = teamID + super.init() + } + + var isIdle: Bool { !runner.hasWork } + + /// This helper's own build, baked into the binary's embedded Info.plist + /// section at compile time — so it reports what it IS, not what the app + /// bundle around it currently claims to be. + static var build: String { + Bundle.main.infoDictionary?["CFBundleVersion"] as? String ?? "" + } + + func helperBuild(withReply reply: @escaping (String) -> Void) { + reply(Self.build) + } + + func cancelOperation(operationID: String, withReply reply: @escaping (Bool) -> Void) { + // Cancellation stops work; it never starts any, so it needs no + // authorization of its own. The connection gate has already + // established that the caller is Burrow, and an ID it doesn't know + // simply isn't running. + guard UUID(uuidString: operationID) != nil else { return reply(false) } + reply(runner.cancel(operationID: operationID)) + } + + func execute(requestData: Data, authorization: Data, withReply reply: @escaping (Data) -> Void) { + func respond(_ outcome: HelperResponse.Outcome) { + let encoded = (try? JSONEncoder().encode(HelperResponse(outcome: outcome))) ?? Data() + reply(encoded) + } + + // Gate 2 — shape. An unknown operation cannot survive decoding. + guard let request = try? JSONDecoder().decode(HelperRequest.self, from: requestData) else { + helperLog.error("request refused: malformed payload") + return respond(.rejected(.malformedPayload)) + } + + if let rejection = request.validate(expectedBuild: Self.build) { + helperLog.error("request refused: \(rejection.rawValue, privacy: .public)") + return respond(.rejected(rejection)) + } + + // Gate 3 — freshness. One authorization buys exactly one operation, so + // a captured payload cannot be replayed for a second root run. + guard replayGuard.admit(request.operationID) else { + helperLog.error("request refused: replayed operation ID") + return respond(.rejected(.replayedOperationID)) + } + + // Gate 4 — authorization. This is what raises the prompt, and it + // happens HERE, in the privileged process, on every single operation. + let decision = HelperAuthorization.authorize(externalForm: authorization) + guard decision.permitsExecution else { + helperLog.notice(""" + operation \(request.operation.rawValue, privacy: .public) not authorized + """) + switch decision { + case .cancelled: return respond(.authorizationCancelled) + default: return respond(.authorizationDenied) + } + } + + // Gate 5 — execution. Our own signed engine, fixed argv. + guard let enginePath = HelperEngine.bundledEnginePath(), + HelperEngine.verify(path: enginePath, teamID: teamID) else { + helperLog.error("engine unavailable or failed signature verification") + return respond(.engineUnavailable) + } + + helperLog.notice(""" + authorized \(request.operation.rawValue, privacy: .public) \ + (mutating: \(request.operation.mutatesDisk, privacy: .public)) + """) + + let client = currentConnection?.remoteObjectProxy as? BurrowHelperClientProtocol + let operationID = request.operationID + let code = runner.run(operation: request.operation, + operationID: operationID, + enginePath: enginePath) { line in + client?.helperDidEmit(line: line, operationID: operationID) + } + helperLog.notice("operation finished with status \(code, privacy: .public)") + respond(.exited(code)) + } +} + +// MARK: - Connection gate + +/// Gate 1. By the time this delegate runs, the system has ALREADY evaluated +/// the code requirement set on the listener against the connecting peer — see +/// `HelperMain.start`. A caller that isn't Burrow, signed by our team, never +/// reaches this method at all. +final class HelperListenerDelegate: NSObject, NSXPCListenerDelegate { + private let service: HelperService + + init(service: HelperService) { + self.service = service + super.init() + } + + func listener(_ listener: NSXPCListener, + shouldAcceptNewConnection connection: NSXPCConnection) -> Bool { + connection.exportedInterface = HelperInterface.daemon() + connection.exportedObject = service + connection.remoteObjectInterface = HelperInterface.client() + service.currentConnection = connection + connection.resume() + helperLog.notice("connection accepted from a verified Burrow client") + return true + } +} diff --git a/macos/HelperSources/main.swift b/macos/HelperSources/main.swift new file mode 100644 index 00000000..51414565 --- /dev/null +++ b/macos/HelperSources/main.swift @@ -0,0 +1,90 @@ +// +// main.swift +// BurrowHelper +// +// Entry point for the root daemon. launchd starts this on demand when +// something connects to the Mach service; it publishes its own authorization +// right, pins who may connect, serves requests, and exits when idle. +// +// Nothing here does privileged work. The gauntlet every request runs is in +// HelperService.swift. +// + +import Foundation +import ServiceManagement + +enum HelperMain { + + /// How long the daemon lingers with no work before exiting. + /// + /// A root process should not be resident for longer than it is useful. + /// launchd restarts it on the next connection at no cost to the user, so + /// the only thing a long idle life buys is a larger window in which a root + /// process exists to be attacked. + static let idleTimeout: TimeInterval = 120 + + static func start() -> Never { + // The team that signed US. Learned at runtime, so no team ID, + // certificate, or developer name is ever hardcoded in the repository, + // and the check survives certificate renewal. + let teamID = HelperCodeRequirement.selfTeamIdentifier() + let requirement = HelperCodeRequirement.string(bundleID: HelperNames.clientBundleID, + teamID: teamID) + + if teamID == nil { + // Ad-hoc: local development only. Say so loudly — a release build + // always has a team, and the release gate refuses to ship without + // one, so seeing this in the wild means something is wrong. + helperLog.notice("running ad-hoc signed: caller pinning is identifier-only (development build)") + } + + // Publish the right's definition. Rewritten every launch on purpose: + // the compiled-in definition is the source of truth, so a stale or + // tampered policy entry from an earlier install cannot weaken a newer + // helper. Failure is not fatal — the right still evaluates, just with + // the system's default wording. + if !HelperAuthorization.installRightDefinition() { + helperLog.error("could not publish the authorization right definition") + } + + let service = HelperService(teamID: teamID) + let delegate = HelperListenerDelegate(service: service) + let listener = NSXPCListener(machServiceName: HelperNames.machService) + + // Gate 1, enforced by the system before our delegate ever runs. The + // supported alternative to hand-rolling an audit-token check — see + // HelperCodeRequirement for why the PID-based version of this is a + // textbook vulnerability. + listener.setConnectionCodeSigningRequirement(requirement) + + listener.delegate = delegate + listener.resume() + + helperLog.notice("helper \(HelperService.build, privacy: .public) listening") + + // Idle exit. Checked on a timer rather than tied to connection + // teardown so a client that opens a connection and then goes silent + // cannot pin a root process open indefinitely. + var idleSince = Date() + let timer = DispatchSource.makeTimerSource(queue: .global(qos: .utility)) + timer.schedule(deadline: .now() + 10, repeating: 10) + timer.setEventHandler { + if service.isIdle { + if Date().timeIntervalSince(idleSince) >= idleTimeout { + helperLog.notice("idle timeout reached; exiting") + exit(0) + } + } else { + idleSince = Date() + } + } + timer.resume() + + RunLoop.main.run() + // RunLoop.main.run() does not return; this keeps the Never signature + // honest for the compiler. + exit(0) + } +} + +HelperMain.start() diff --git a/macos/Resources/LaunchDaemons/dev.caezium.Burrow.helper.plist b/macos/Resources/LaunchDaemons/dev.caezium.Burrow.helper.plist new file mode 100644 index 00000000..db2ba3af --- /dev/null +++ b/macos/Resources/LaunchDaemons/dev.caezium.Burrow.helper.plist @@ -0,0 +1,43 @@ + + + + + + Label + dev.caezium.Burrow.helper + + BundleProgram + Contents/MacOS/BurrowHelper + + MachServices + + dev.caezium.Burrow.helper + + + + + AssociatedBundleIdentifiers + + dev.caezium.Burrow + + + ProcessType + Interactive + + diff --git a/macos/Sources/MoEngine.swift b/macos/Sources/MoEngine.swift index 839dcf9d..7f159971 100644 --- a/macos/Sources/MoEngine.swift +++ b/macos/Sources/MoEngine.swift @@ -129,7 +129,11 @@ final class MoEngine { init(processPort: MoleProcessPort = SystemMoleProcess(), locator: MoLocator = SystemMoLocator(), - streamPort: ProcessPort = SystemProcessPort(), + // Elevated streaming runs prefer the privileged helper when it is + // registered, approved, and build-matched; everything else — and any + // elevated run the helper doesn't recognise — falls through to the + // osascript port unchanged. See `PrivilegeRoute`. + streamPort: ProcessPort = HelperAwareProcessPort(), makePTY: @escaping @Sendable () -> PTYPort = { PTYTask() }) { self.processPort = processPort self.locator = locator diff --git a/macos/Sources/PrivilegedHelper/HelperAuthorization.swift b/macos/Sources/PrivilegedHelper/HelperAuthorization.swift new file mode 100644 index 00000000..76203ab0 --- /dev/null +++ b/macos/Sources/PrivilegedHelper/HelperAuthorization.swift @@ -0,0 +1,220 @@ +// +// HelperAuthorization.swift +// Burrow / BurrowHelper (shared) +// +// The authorization policy for every root operation, in one place. +// +// ── The product decision this file encodes ────────────────────────────── +// Every operation that actually runs as root requires a FRESH user +// authentication. No grace period, no cached credential, no "authenticate +// once for this launch". Installing the helper is a separate, one-time +// macOS approval and does NOT authorize any later operation. +// +// Two words of that are literally keys in the right's definition: +// timeout: 0 the credential expires immediately, so the next +// privileged request authenticates again from scratch +// shared: false the credential is never visible to another process or +// satisfiable by a different right +// +// ── Where the prompt is raised, and why it matters ────────────────────── +// The obvious design — prompt with LAContext in the GUI, then send the XPC +// request — is not an authorization at all. The daemon would be trusting an +// unauthenticated message from a process it cannot vouch for; a caller that +// skipped the prompt entirely would be indistinguishable from one that +// passed it. Whatever the GUI shows is cosmetic unless the ROOT side is what +// demands the right. +// +// So the flow is the documented Authorization Services one, with the +// authenticating call deliberately on the privileged side: +// +// 1. GUI — AuthorizationCreate (empty environment, NO flags). This makes +// an empty reference bound to the user's session. It does not +// authenticate and shows no UI. +// 2. GUI — AuthorizationMakeExternalForm, and the bytes ride with the +// request over XPC. +// 3. Root — AuthorizationCreateFromExternalForm, then AuthorizationCopyRights +// for `rightName` with interaction allowed. THIS raises the +// system prompt, and its result is what gates execution. +// +// Because the reference carries the client's session, the prompt appears in +// that session and is attributed to Burrow rather than to a faceless root +// process. Because the right is defined with timeout 0 / shared false, step 3 +// authenticates every single time. +// +// The authentication UI is the system's, so it offers Touch ID where the +// hardware has it and falls back to the Mac login password otherwise. Burrow +// receives a granted/denied/cancelled result and never sees, handles, or +// stores a credential. +// +// ── The two classic ways to make this a no-op ─────────────────────────── +// Both are real, both are one line, both are covered by tests: +// * calling AuthorizationCopyRights with a NULL rights set — it returns +// success regardless of who is calling; +// * passing kAuthorizationFlagPreAuthorize on the daemon side — that asks +// whether authorization would be POSSIBLE later, which is not the same +// as requiring it now. +// + +import Foundation +import Security + +enum HelperAuthorization { + + // MARK: - The right + + /// Namespaced under the app's bundle identifier so it can never collide + /// with, or be satisfied by, a system right the user may already hold for + /// something else. + static let rightName = "dev.caezium.Burrow.privileged-operation" + + /// The right's definition in the system authorization policy database. + /// + /// `allow-root: false` is the entry that matters most and is the easiest + /// to get wrong. The daemon evaluating this right IS root; if root callers + /// were allowed to satisfy it implicitly, the daemon would authorize + /// itself and the prompt would quietly stop appearing. + static let rightDefinition: [String: Any] = [ + "class": "user", + "group": "admin", + "authenticate-user": true, + // No credential survives the call, and none is shared with anything + // else. Together these are "a fresh authentication every time". + "timeout": 0, + "shared": false, + // Root is not a shortcut past the prompt. + "allow-root": false, + // Not the console user's session by default — an administrator has to + // authenticate explicitly. + "session-owner": false, + "comment": "Burrow is about to run a privileged maintenance operation (scan, clean, or optimize) as root.", + ] + + // MARK: - Flags + + /// The daemon's `AuthorizationCopyRights` flags. + /// + /// `extendRights` is what actually grants the right — without it the call + /// reports whether the right COULD be granted and grants nothing, which a + /// careless caller reads as success. `interactionAllowed` lets the system + /// raise the prompt, which is the entire point of doing this on the + /// privileged side. + static let daemonFlags: AuthorizationFlags = [.extendRights, .interactionAllowed] + + /// The client's flags: none. The GUI externalizes an empty reference and + /// leaves authentication to root. If the client ever starts + /// pre-authorizing, the prompt migrates out of the privileged process and + /// the daemon's own check degrades into decoration. + static let clientFlags: AuthorizationFlags = [] + + // MARK: - Outcome + + enum Outcome: Equatable, Sendable { + case granted + case denied + case cancelled + case failed(OSStatus) + + /// The ONE predicate the daemon branches on. Kept as a single property + /// so "granted" can never drift into "not an outright failure". + var permitsExecution: Bool { self == .granted } + } + + /// Classify the `OSStatus` from `AuthorizationCopyRights`. Pure, so it is + /// exhaustively table-tested. Anything unrecognised fails CLOSED, carrying + /// the raw status for diagnosis rather than guessing. + static func outcome(from status: OSStatus) -> Outcome { + switch status { + case errAuthorizationSuccess: + return .granted + case errAuthorizationCanceled: + return .cancelled + case errAuthorizationDenied, OSStatus(errAuthorizationInteractionNotAllowed): + return .denied + default: + return .failed(status) + } + } + + // MARK: - External form + + /// `AuthorizationExternalForm` is a fixed-size C struct. Anything of a + /// different length is malformed and is refused HERE, before the bytes are + /// handed to the Security framework — a hostile client should not get to + /// choose the length of a buffer that lands in a C API. + static func isPlausibleExternalForm(_ data: Data) -> Bool { + data.count == MemoryLayout.size + } + + // MARK: - System witnesses + // + // Thin wrappers over the Security framework: the decisions above are pure + // and tested, these just perform the calls. + + /// GUI side. Create an empty authorization reference and externalize it. + /// Shows no UI and authenticates nothing — the daemon does that. + /// Returns `nil` if a reference can't be made, in which case the caller + /// must abandon the operation rather than proceed unauthorized. + static func makeExternalForm() -> Data? { + var ref: AuthorizationRef? + let created = AuthorizationCreate(nil, nil, clientFlags, &ref) + guard created == errAuthorizationSuccess, let ref else { return nil } + defer { AuthorizationFree(ref, []) } + + var form = AuthorizationExternalForm() + guard AuthorizationMakeExternalForm(ref, &form) == errAuthorizationSuccess else { return nil } + return withUnsafeBytes(of: &form) { Data($0) } + } + + /// Daemon side. Rebuild the client's reference and REQUIRE `rightName`, + /// raising the system prompt. The returned outcome is the gate: only + /// `.granted` may be followed by privileged work. + /// + /// The rights array is always non-empty — passing NULL here is the + /// documented way to accidentally authorize everybody. + static func authorize(externalForm data: Data) -> Outcome { + guard isPlausibleExternalForm(data) else { return .denied } + + var form = AuthorizationExternalForm() + let copied: Bool = withUnsafeMutableBytes(of: &form) { raw -> Bool in + guard raw.count == data.count else { return false } + _ = data.copyBytes(to: raw.bindMemory(to: UInt8.self)) + return true + } + guard copied else { return .denied } + + var ref: AuthorizationRef? + let restored = AuthorizationCreateFromExternalForm(&form, &ref) + guard restored == errAuthorizationSuccess, let ref else { return .denied } + defer { AuthorizationFree(ref, []) } + + return rightName.withCString { name -> Outcome in + var item = AuthorizationItem(name: name, valueLength: 0, value: nil, flags: 0) + return withUnsafeMutablePointer(to: &item) { itemPointer -> Outcome in + var rights = AuthorizationRights(count: 1, items: itemPointer) + let status = AuthorizationCopyRights(ref, &rights, nil, daemonFlags, nil) + return outcome(from: status) + } + } + } + + /// Daemon side, once at startup. Publish the right's definition so the + /// system prompts with Burrow's own wording and policy instead of falling + /// back to a generic default for an unknown right. + /// + /// Rewritten every launch on purpose: the daemon's compiled-in definition + /// is the source of truth, so a stale or tampered policy entry from an + /// earlier install cannot weaken a newer helper. + @discardableResult + static func installRightDefinition() -> Bool { + var authRef: AuthorizationRef? + guard AuthorizationCreate(nil, nil, [], &authRef) == errAuthorizationSuccess, + let authRef else { return false } + defer { AuthorizationFree(authRef, []) } + + let comment = (rightDefinition["comment"] as? String).map { $0 as CFString } + let status = AuthorizationRightSet(authRef, rightName, + rightDefinition as CFDictionary, + comment, nil, nil) + return status == errAuthorizationSuccess + } +} diff --git a/macos/Sources/PrivilegedHelper/HelperCodeRequirement.swift b/macos/Sources/PrivilegedHelper/HelperCodeRequirement.swift new file mode 100644 index 00000000..412f4e7c --- /dev/null +++ b/macos/Sources/PrivilegedHelper/HelperCodeRequirement.swift @@ -0,0 +1,155 @@ +// +// HelperCodeRequirement.swift +// Burrow / BurrowHelper (shared) +// +// Who may talk to the root daemon at all. +// +// Authorization (HelperAuthorization) answers "may this operation run". +// This answers the question that comes first: "is the process on the other +// end of this XPC connection actually Burrow". Both are required, and the +// order matters — without this check, any local process could open the Mach +// service and drive the authentication prompt, phishing the user for +// administrator credentials behind a dialog the system itself renders and +// that therefore looks entirely legitimate. +// +// ── Identifying the peer correctly ────────────────────────────────────── +// The requirement built here is handed to +// `NSXPCListener.setConnectionCodeSigningRequirement`, so the SYSTEM +// evaluates it against the connecting peer before the delegate is ever +// called. That matters more than it looks: +// +// The obvious hand-rolled check — read `connection.processIdentifier`, look +// the process up, verify its signature — is the classic vulnerable pattern. +// A PID can be recycled between being read and being checked, and the +// standard exploit hands the helper the PID of a legitimate app and then +// races a hostile process into that slot. Doing it properly means the audit +// token, and reaching an NSXPCConnection's audit token means private API, +// which is not a dependency worth taking inside a root daemon. +// +// The macOS 13 requirement API sidesteps both problems: it is supported, and +// the evaluation happens in the kernel against the real peer, with no window +// between check and use. +// +// ── Nothing personal is hardcoded ─────────────────────────────────────── +// The requirement is assembled at RUNTIME from the daemon's own signing +// information: the helper asks what team signed IT, and demands the caller be +// the Burrow app signed by that same team. No team ID, certificate, or +// developer name appears in this repository, and the check keeps working +// across certificate renewals. +// + +import Foundation +import Security + +enum HelperCodeRequirement { + + /// A syntactically valid requirement that no code can satisfy, used + /// whenever a safe requirement cannot be built. + /// + /// It is deliberately not the empty string: an empty requirement fails to + /// PARSE, and several Security APIs treat a parse failure as "no + /// requirement given", which is the exact opposite of failing closed. This + /// one parses fine and simply matches nothing, because no bundle + /// identifier may contain a space. + static let unsatisfiable = #"identifier "dev.caezium.Burrow.no such caller""# + + // MARK: - Input validation + // + // The requirement string is a small language parsed by the Security + // framework, and identifiers are interpolated into it. A value containing + // a quote could close the literal early and append its own clause — + // `identifier "x" or anchor apple generic` would admit every Apple-signed + // process on the machine. + // + // Hostile values are REFUSED rather than escaped. A legitimate bundle + // identifier or team ID is drawn from a small alphabet, so rejecting + // anything outside it costs nothing and removes a whole class of parser + // subtleties — there is no "correctly escaped" case left to get wrong. + + /// The characters a real bundle identifier or team ID is built from. + private static let permitted = CharacterSet(charactersIn: + "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789.-_") + + /// The value if it can ride inertly inside a requirement string, else nil. + static func validated(identifier: String) -> String? { + guard !identifier.isEmpty, identifier.count <= 255 else { return nil } + guard identifier.unicodeScalars.allSatisfy({ permitted.contains($0) }) else { return nil } + return identifier + } + + // MARK: - Building the requirement + + /// The designated requirement a caller must satisfy. + /// + /// With a team ID, three clauses are pinned and every one is mandatory: + /// * `identifier` — this exact bundle, not merely something of ours; + /// * `anchor apple generic` — a chain terminating at Apple, so a + /// self-signed build cannot claim the identifier; + /// * `certificate leaf[subject.OU]` — signed by the same team as the + /// helper, so another developer's notarized app cannot impersonate us. + /// + /// Without a team ID (a local ad-hoc Debug build) it falls back to the + /// identifier alone so development works. That is explicitly NOT + /// distribution grade, and `isDistributionGrade` is what the release gate + /// checks so an ad-hoc helper can never ship. + static func string(bundleID: String, teamID: String?) -> String { + guard let identifier = validated(identifier: bundleID) else { return unsatisfiable } + + guard let teamID, !teamID.trimmingCharacters(in: .whitespaces).isEmpty else { + return #"identifier "\#(identifier)""# + } + guard let team = validated(identifier: teamID) else { return unsatisfiable } + + return #"identifier "\#(identifier)" and anchor apple generic and certificate leaf[subject.OU] = "\#(team)""# + } + + /// Whether a requirement built from this team is strong enough to ship. + /// An ad-hoc helper has no team, so it can be impersonated by any local + /// build that claims the identifier — fine for development, never for a + /// release. + static func isDistributionGrade(teamID: String?) -> Bool { + guard let teamID, !teamID.trimmingCharacters(in: .whitespaces).isEmpty else { return false } + return validated(identifier: teamID) != nil + } + + // MARK: - System witnesses + + /// The team that signed the given code, or nil when it is unsigned or + /// ad-hoc signed. Used by the daemon to learn its OWN team, so the + /// requirement never needs a hardcoded value. + static func teamIdentifier(of code: SecCode?) -> String? { + guard let code else { return nil } + var staticCode: SecStaticCode? + guard SecCodeCopyStaticCode(code, [], &staticCode) == errSecSuccess, + let staticCode else { return nil } + return teamIdentifier(ofStatic: staticCode) + } + + static func teamIdentifier(ofStatic staticCode: SecStaticCode) -> String? { + var information: CFDictionary? + guard SecCodeCopySigningInformation(staticCode, SecCSFlags(rawValue: kSecCSSigningInformation), + &information) == errSecSuccess, + let dictionary = information as? [String: Any] else { return nil } + let team = dictionary[kSecCodeInfoTeamIdentifier as String] as? String + return team.flatMap { validated(identifier: $0) } + } + + /// The team that signed the currently running process. + static func selfTeamIdentifier() -> String? { + var code: SecCode? + guard SecCodeCopySelf([], &code) == errSecSuccess else { return nil } + return teamIdentifier(of: code) + } + + /// A requirement matching any code signed by `teamID` with an Apple-issued + /// chain, without pinning a bundle identifier. + /// + /// Used for the nested ENGINE binary, which carries its own identifier but + /// is re-signed with our identity by the release pipeline. Returns nil + /// when a safe requirement can't be built, and callers treat that as + /// "refuse to run" rather than "skip the check". + static func sameTeam(teamID: String) -> String? { + guard let team = validated(identifier: teamID) else { return nil } + return #"anchor apple generic and certificate leaf[subject.OU] = "\#(team)""# + } +} diff --git a/macos/Sources/PrivilegedHelper/HelperContract.swift b/macos/Sources/PrivilegedHelper/HelperContract.swift new file mode 100644 index 00000000..0fac59a6 --- /dev/null +++ b/macos/Sources/PrivilegedHelper/HelperContract.swift @@ -0,0 +1,223 @@ +// +// HelperContract.swift +// Burrow / BurrowHelper (shared) +// +// The wire contract between the GUI and the root helper. Compiled into BOTH +// targets so the two can never disagree about what a request means. +// +// Everything in this file is pure Foundation — no SwiftUI, no Sparkle, no +// Sentry — because the helper is a bare launchd daemon that must stay small, +// auditable, and free of anything that could pull UI or network code into a +// root process. +// +// ── Why the contract looks like this ──────────────────────────────────── +// The old elevation path handed osascript a SHELL STRING built from an +// executable path plus argv (`MoleCLI.elevatedScript`). It was carefully +// quoted and well tested, but its shape meant the privileged side had to +// trust whatever command the caller composed. +// +// This contract inverts that. A request names an OPERATION and nothing else. +// The argv is derived from the enum on the privileged side, so the set of +// commands the helper can ever run is fixed at compile time and visible in +// one switch statement. There is deliberately no field for a path, a flag, a +// binary, or a command string — a client that is fully compromised still +// cannot express "run this". +// + +import Foundation + +// MARK: - The closed operation set + +/// Every privileged thing Burrow can ask for. Adding a case here is a +/// security decision, not a feature decision: it widens what the root daemon +/// is capable of doing, permanently, for every installed copy. +enum HelperOperation: String, Codable, CaseIterable, Sendable { + /// Enumerate what a clean WOULD remove. Reads privileged locations; the + /// engine's `--dry-run` guarantees no mutation. + case scan + /// Remove the caches the engine's own rules select. + case clean + /// The engine's maintenance pass. + case optimize + + /// The exact argv the helper passes to the bundled engine. Fixed per case + /// and assembled here rather than by the caller, so no client input ever + /// reaches the child process's argument vector. + /// + /// These reproduce what the GUI runs today through osascript — CleanView's + /// `["clean"]`, OptimizeView's `["optimize"]`, and the dry-run preview — + /// so the helper changes HOW the command is elevated, never WHAT runs. + var engineArguments: [String] { + switch self { + case .scan: return ["clean", "--dry-run"] + case .clean: return ["clean"] + case .optimize: return ["optimize"] + } + } + + /// Whether this operation mutates the disk. `scan` is read-only, but it + /// still authenticates: it reads privileged locations, and the product + /// decision was that anything running AS ROOT prompts. + var mutatesDisk: Bool { + switch self { + case .scan: return false + case .clean, .optimize: return true + } + } + + /// Recognise an existing elevated call site's argv as a typed operation, + /// or `nil` if it isn't one of the three. + /// + /// This is the migration seam. The GUI still describes elevated work as + /// `["clean"]` / `["optimize"]` through `OperationFlow`, and this maps + /// those onto the helper WITHOUT letting anything else through: argv the + /// helper doesn't recognise returns nil and keeps the existing osascript + /// route, rather than being forwarded as some approximate operation. + init?(engineArguments: [String]) { + guard let match = HelperOperation.allCases.first(where: { $0.engineArguments == engineArguments }) + else { return nil } + self = match + } +} + +// MARK: - Request + +/// Why a request never reached the authorization step. Named so the GUI can +/// explain the refusal instead of showing a bare failure, and so a rejection +/// is never confused with "the command ran and failed". +enum HelperRequestRejection: String, Codable, Equatable, Sendable { + /// The payload wasn't decodable as a request at all. + case malformedPayload + /// The operation ID wasn't a UUID (see `HelperRequest.operationID`). + case malformedOperationID + /// The helper's build doesn't match the app's — see `HelperVersionSkew`. + case buildMismatch + /// This operation ID was already served. One authorization, one operation. + case replayedOperationID +} + +/// One privileged operation, fully described. Three fields, none of which can +/// carry a command. +struct HelperRequest: Codable, Equatable, Sendable { + let operation: HelperOperation + + /// A fresh UUID per request. This is the replay key: the daemon serves any + /// given ID at most once, so a captured payload — external authorization + /// form and all — cannot be resent to buy a second root run out of one + /// prompt. A client-chosen constant would defeat that, hence the format + /// check rather than "any non-empty string". + let operationID: String + + /// `CFBundleVersion` of the calling app, compared against the helper's own. + /// A registered daemon outlives the app that installed it (Sparkle replaces + /// Burrow.app underneath it), and a stale root helper holding an older idea + /// of what `clean` does is exactly the drift worth refusing. + let clientBuild: String + + /// `nil` when the request is well formed. Runs on the PRIVILEGED side — + /// the client's own validation is a courtesy, this one is the boundary. + func validate(expectedBuild: String) -> HelperRequestRejection? { + guard UUID(uuidString: operationID) != nil else { return .malformedOperationID } + guard HelperVersionSkew.evaluate(appBuild: expectedBuild, helperBuild: clientBuild) == .matched else { + return .buildMismatch + } + return nil + } +} + +// MARK: - Response + +/// What a privileged operation produced. The failure cases mirror the +/// osascript path's `ElevatedOutcome` so the GUI keeps ONE error taxonomy +/// across both elevation routes (the rule issue #48 established). +struct HelperResponse: Codable, Equatable, Sendable { + enum Outcome: Codable, Equatable, Sendable { + /// The engine ran and exited with this status. + case exited(Int32) + /// The user dismissed the authentication prompt. + case authorizationCancelled + /// Authentication was attempted and refused (wrong credentials, not an + /// administrator, interaction unavailable). + case authorizationDenied + /// The request never reached authorization. + case rejected(HelperRequestRejection) + /// The signed, bundled engine could not be resolved or failed + /// verification — so nothing was executed. + case engineUnavailable + + /// Collapse to the `Int32` the existing call sites branch on. Every + /// failure shape is nonzero: a dismissed prompt must never read as + /// success. + var exitCode: Int32 { + switch self { + case .exited(let code): return code + case .authorizationCancelled, .authorizationDenied: return 1 + case .rejected: return 78 // EX_CONFIG: the request was refused, not run + case .engineUnavailable: return 127 + } + } + + // The bridge onto the GUI's existing `ElevatedOutcome` taxonomy lives + // in PrivilegedHelperClient.swift — that type belongs to the app, and + // this file also compiles into the daemon, which must stay free of + // anything GUI-side. + } + + let outcome: Outcome +} + +// MARK: - Replay guard + +/// Remembers which operation IDs have already been served, so one +/// authorization buys exactly one root operation. +/// +/// Bounded on purpose: a daemon can stay resident for weeks, and an unbounded +/// set would grow with every request a client cared to send. Eviction is +/// oldest-first, which only ever forgets ancient IDs — the practical replay +/// window (seconds, between authenticating and executing) is always covered. +final class HelperReplayGuard: @unchecked Sendable { + private let capacity: Int + private var order: [String] = [] + private var seen: Set = [] + private let lock = NSLock() + + init(capacity: Int = 512) { + self.capacity = max(1, capacity) + } + + /// Number of IDs currently remembered. Never exceeds `capacity`. + var count: Int { + lock.lock(); defer { lock.unlock() } + return seen.count + } + + /// `true` the first time an ID is presented, `false` for every repeat. + func admit(_ operationID: String) -> Bool { + lock.lock(); defer { lock.unlock() } + guard !seen.contains(operationID) else { return false } + seen.insert(operationID) + order.append(operationID) + while order.count > capacity { + seen.remove(order.removeFirst()) + } + return true + } +} + +// MARK: - Version skew + +/// Whether the app and the installed helper are the same build. +enum HelperVersionSkew { + enum Skew: Equatable, Sendable { + case matched + case mismatched + } + + /// Exact equality, and an empty build on either side is a mismatch. There + /// is no "close enough" here: the helper runs as root, and a version it + /// can't name is a version nobody has reasoned about. + static func evaluate(appBuild: String, helperBuild: String) -> Skew { + guard !appBuild.isEmpty, !helperBuild.isEmpty, appBuild == helperBuild else { return .mismatched } + return .matched + } +} diff --git a/macos/Sources/PrivilegedHelper/HelperXPC.swift b/macos/Sources/PrivilegedHelper/HelperXPC.swift new file mode 100644 index 00000000..3bf2c889 --- /dev/null +++ b/macos/Sources/PrivilegedHelper/HelperXPC.swift @@ -0,0 +1,84 @@ +// +// HelperXPC.swift +// Burrow / BurrowHelper (shared) +// +// The XPC surface, and the names both sides must agree on. +// +// The protocol is deliberately tiny. Three methods, and the only one that +// does privileged work takes an opaque request blob plus an authorization +// blob — both of which the daemon decodes and validates itself. There is no +// method that takes a command, a path, or an argument list, so the XPC +// surface cannot be talked into running something the typed contract doesn't +// already allow. +// + +import Foundation + +// MARK: - Names + +enum HelperNames { + /// The Mach service the daemon vends and the client connects to. Must + /// match `MachServices` in the launchd property list. + static let machService = "dev.caezium.Burrow.helper" + + /// The launchd label, and the property list file name inside the app at + /// `Contents/Library/LaunchDaemons/`. `SMAppService.daemon(plistName:)` + /// resolves the daemon from exactly that location. + static let daemonPlist = "dev.caezium.Burrow.helper.plist" + + /// The bundle identifier the daemon requires of any caller. + static let clientBundleID = "dev.caezium.Burrow" +} + +// MARK: - Daemon interface + +/// What the root daemon exposes. Implemented in the helper target. +@objc protocol BurrowHelperProtocol { + /// Run one typed operation as root. + /// + /// - Parameters: + /// - requestData: a JSON-encoded `HelperRequest`. Opaque here on + /// purpose — the daemon decodes and validates it, so a malformed or + /// hostile payload is refused inside the privileged process rather + /// than being pre-parsed by the caller. + /// - authorization: the client's `AuthorizationExternalForm` bytes. The + /// daemon rebuilds the reference and REQUIRES the right, which is what + /// raises the authentication prompt. + /// - reply: a JSON-encoded `HelperResponse`. + func execute(requestData: Data, authorization: Data, withReply reply: @escaping (Data) -> Void) + + /// The helper's own `CFBundleVersion`. The client compares it against its + /// own build and refuses to use a helper that doesn't match, since a + /// registered daemon outlives the app that installed it. + func helperBuild(withReply reply: @escaping (String) -> Void) + + /// Ask the daemon to terminate a running operation. + /// + /// This exists because the osascript path could NOT do it: killing + /// `osascript` orphans the root child it spawned, so the old streaming + /// flow had no safe way to cancel. The daemon owns the child process + /// directly, so it can signal it and reap it properly. + func cancelOperation(operationID: String, withReply reply: @escaping (Bool) -> Void) +} + +// MARK: - Client interface + +/// What the daemon can call back on the client — output streaming only. +/// Nothing here grants the daemon any authority over the app. +@objc protocol BurrowHelperClientProtocol { + /// One line of engine output, ANSI already stripped by the daemon. + func helperDidEmit(line: String, operationID: String) +} + +// MARK: - Interface configuration + +enum HelperInterface { + /// Both sides build their `NSXPCInterface` here so the shapes can't drift. + static func daemon() -> NSXPCInterface { + NSXPCInterface(with: BurrowHelperProtocol.self) + } + + static func client() -> NSXPCInterface { + NSXPCInterface(with: BurrowHelperClientProtocol.self) + } +} diff --git a/macos/Sources/PrivilegedHelperClient.swift b/macos/Sources/PrivilegedHelperClient.swift new file mode 100644 index 00000000..d3d7f135 --- /dev/null +++ b/macos/Sources/PrivilegedHelperClient.swift @@ -0,0 +1,326 @@ +// +// PrivilegedHelperClient.swift +// Burrow +// +// The GUI's side of the privileged helper: registration, connection, and one +// typed operation at a time. +// +// ── What this replaces, and what it does not ──────────────────────────── +// The existing elevation path builds a shell string and hands it to +// `osascript … with administrator privileges` (see `MoleCLI.elevatedScript`). +// That path is password-only by construction — the `system.privilege.admin` +// right authenticates through SecurityAgent's classic mechanism, which never +// offers Touch ID — and it cannot be cancelled safely, because killing +// osascript orphans the root child it spawned. +// +// The helper fixes both, but it can only be used once the user has approved +// registering a launch daemon, which is its own one-time macOS prompt. Until +// then — and if the user declines it outright — the osascript path remains, +// unchanged, as the fallback. `PrivilegeRoute` is where that choice is made, +// and it is a pure function so the conditions are visible and tested rather +// than scattered through call sites. +// +// ── The security properties this file must not weaken ────────────────── +// * Every root operation authenticates freshly. The client never caches an +// authorization, never reuses an operation ID, and never pre-authorizes. +// * The client cannot describe a command. It picks a `HelperOperation`, and +// the daemon derives argv from the enum. +// * A helper whose build doesn't match this app is not used at all. +// + +import Foundation +import Security +import ServiceManagement + +// MARK: - Bridging the two elevation routes onto one taxonomy + +extension HelperResponse.Outcome { + /// Map onto the taxonomy the GUI already renders, so the helper route and + /// the osascript route produce the same user-facing message and no call + /// site needs to know which one ran. + /// + /// `authorizationDenied` folds into `.authCancelled` because from the + /// user's side both mean "you weren't authenticated, so nothing ran". + var elevatedOutcome: ElevatedOutcome { + switch self { + case .exited(let code): return .exited(code) + case .authorizationCancelled, .authorizationDenied: return .authCancelled + case .rejected, .engineUnavailable: return .launchFailed + } + } +} + +// MARK: - Routing + +/// Whether a given elevated operation goes through the helper or the legacy +/// osascript path. Pure → unit-tested, because "when do we use the root +/// daemon" should not be an emergent property of five call sites. +enum PrivilegeRoute: Equatable { + /// Use the privileged helper for this typed operation. + case helper(HelperOperation) + /// Use the existing osascript elevation, unchanged. + case osascript + + /// The routing rule. The helper is used only when ALL of these hold: + /// * the argv maps onto one of the three typed operations; + /// * the daemon is registered and enabled; + /// * its build matches this app's. + /// + /// Any doubt routes to osascript. That is a genuine fallback rather than a + /// silent downgrade: the osascript path is the elevation Burrow has always + /// shipped, it still prompts for an administrator, and it still runs the + /// same trusted engine. What the user loses is Touch ID and safe + /// cancellation, not the authentication itself. + static func decide(arguments: [String], + registration: HelperRegistrationStatus, + skew: HelperVersionSkew.Skew) -> PrivilegeRoute { + guard let operation = HelperOperation(engineArguments: arguments) else { return .osascript } + guard registration == .enabled else { return .osascript } + guard skew == .matched else { return .osascript } + return .helper(operation) + } +} + +/// The daemon's registration state, mirrored off `SMAppService.Status` so call +/// sites and tests don't need ServiceManagement. +enum HelperRegistrationStatus: Equatable, Sendable { + /// Registered and allowed to run. + case enabled + /// Never registered on this machine, or the registration is gone. + case notRegistered + /// Registered, but waiting on the user in Login Items & Extensions — + /// either the initial approval or a switch they turned back off. + case requiresApproval + + init(_ status: SMAppService.Status) { + switch status { + case .enabled: self = .enabled + case .requiresApproval: self = .requiresApproval + case .notFound, .notRegistered: self = .notRegistered + @unknown default: + // A status this build has never heard of is not a licence to run + // privileged work. Fail closed to the state that routes elsewhere. + self = .notRegistered + } + } + + /// Whether the user still has a decision to make. Drives the Settings + /// copy — "approve this in Login Items" is actionable, "not registered" + /// is just the default state. + var needsUserAction: Bool { self == .requiresApproval } +} + +// MARK: - Client + +/// Talks to the root daemon. One operation per call, one authentication per +/// operation. +final class PrivilegedHelperClient: @unchecked Sendable { + + static let shared = PrivilegedHelperClient() + + /// This app's build, the value the daemon compares against its own. + static var appBuild: String { + Bundle.main.infoDictionary?["CFBundleVersion"] as? String ?? "" + } + + private let service = SMAppService.daemon(plistName: HelperNames.daemonPlist) + private let lock = NSLock() + private var cachedSkew: HelperVersionSkew.Skew? + + // MARK: Registration + + var registrationStatus: HelperRegistrationStatus { + HelperRegistrationStatus(service.status) + } + + /// Register the daemon. This raises macOS's own one-time administrator + /// approval for installing a launch daemon. + /// + /// Registering does NOT authorize any operation — that was an explicit + /// product decision, and it is enforced on the daemon side by a right + /// defined with `timeout: 0` and `shared: false`. Installing the helper + /// buys convenience, never standing privilege. + func register() throws { + try service.register() + lock.lock(); cachedSkew = nil; lock.unlock() + } + + /// Remove the daemon. Used by Settings, and by the uninstall path so + /// Burrow never leaves a root daemon behind. + func unregister() throws { + try service.unregister() + lock.lock(); cachedSkew = nil; lock.unlock() + } + + // MARK: Connection + + private func makeConnection() -> NSXPCConnection { + let connection = NSXPCConnection(machServiceName: HelperNames.machService, + options: .privileged) + connection.remoteObjectInterface = HelperInterface.daemon() + return connection + } + + /// The installed helper's build, or "" if it can't be reached. Blocking — + /// call off the main thread. + func helperBuild(timeout: TimeInterval = 5) -> String { + let connection = makeConnection() + connection.resume() + defer { connection.invalidate() } + + let semaphore = DispatchSemaphore(value: 0) + var result = "" + let proxy = connection.remoteObjectProxyWithErrorHandler { _ in semaphore.signal() } + (proxy as? BurrowHelperProtocol)?.helperBuild { build in + result = build + semaphore.signal() + } + _ = semaphore.wait(timeout: .now() + timeout) + return result + } + + /// Whether the installed helper matches this app. Cached per process + /// because it only changes across an update or a re-registration, both of + /// which clear it. + func versionSkew() -> HelperVersionSkew.Skew { + lock.lock() + if let cachedSkew { lock.unlock(); return cachedSkew } + lock.unlock() + + let skew = HelperVersionSkew.evaluate(appBuild: Self.appBuild, helperBuild: helperBuild()) + lock.lock(); cachedSkew = skew; lock.unlock() + return skew + } + + /// The route for an elevated invocation described the way `OperationFlow` + /// already describes it. + func route(for arguments: [String]) -> PrivilegeRoute { + let status = registrationStatus + // Don't pay for an XPC round trip to learn the version when the daemon + // isn't usable anyway. + guard status == .enabled else { + return PrivilegeRoute.decide(arguments: arguments, registration: status, skew: .mismatched) + } + return PrivilegeRoute.decide(arguments: arguments, registration: status, skew: versionSkew()) + } + + // MARK: Execution + + /// Run one typed operation as root, streaming output lines to `onLine`. + /// + /// Blocking — call off the main thread. It blocks for as long as the user + /// takes to authenticate plus as long as the operation runs, because the + /// authentication prompt is raised by the DAEMON during this call. + func run(operation: HelperOperation, + onLine: @escaping (String) -> Void) -> ElevatedOutcome { + // A fresh, unauthenticated reference per operation. Creating it shows + // no UI; the daemon is what demands the right, and that is what makes + // the prompt appear. + guard let authorization = HelperAuthorization.makeExternalForm() else { + return .launchFailed + } + + let request = HelperRequest(operation: operation, + operationID: UUID().uuidString, + clientBuild: Self.appBuild) + guard let payload = try? JSONEncoder().encode(request) else { return .launchFailed } + + let connection = makeConnection() + connection.exportedInterface = HelperInterface.client() + let sink = HelperOutputSink(onLine: onLine) + connection.exportedObject = sink + connection.resume() + defer { connection.invalidate() } + + let semaphore = DispatchSemaphore(value: 0) + var outcome: ElevatedOutcome = .launchFailed + + let proxy = connection.remoteObjectProxyWithErrorHandler { _ in + // A connection error here means the daemon refused us, died, or + // was never reachable. Nothing ran, so this is a launch failure — + // never a silent success. + outcome = .launchFailed + semaphore.signal() + } + (proxy as? BurrowHelperProtocol)?.execute(requestData: payload, + authorization: authorization) { data in + if let response = try? JSONDecoder().decode(HelperResponse.self, from: data) { + outcome = response.outcome.elevatedOutcome + } + semaphore.signal() + } + + // No timeout: the user may take a long time at the authentication + // prompt, and a clean or optimize can legitimately run for minutes. + // Cancellation is explicit (`cancel(operationID:)`), which is exactly + // what the osascript path could never offer. + semaphore.wait() + return outcome + } +} + +// MARK: - The streaming seam + +/// Routes elevated streaming runs to the helper when it is usable, and to the +/// existing osascript port otherwise. +/// +/// This wraps `SystemProcessPort` rather than editing it, so the osascript +/// path — the elevation Burrow has shipped for every release so far — keeps +/// its exact behaviour, tests, and error taxonomy. A helper route is an +/// alternative, never a rewrite of the fallback. +struct HelperAwareProcessPort: ProcessPort { + var fallback: SystemProcessPort = SystemProcessPort() + var client: PrivilegedHelperClient = .shared + + func events(_ spec: ProcessSpec) -> AsyncStream { + // Un-elevated runs never involve the helper. Cheap check first, so the + // common path costs nothing. + guard spec.elevated else { return fallback.events(spec) } + + return AsyncStream { continuation in + // Routing asks the daemon for its version, and executing blocks + // for as long as the user takes to authenticate. Neither may + // happen on the main thread. + DispatchQueue.global(qos: .userInitiated).async { + switch client.route(for: spec.arguments) { + case .helper(let operation): + let outcome = client.run(operation: operation) { line in + continuation.yield(.line(line)) + } + switch outcome { + case .exited(let code): continuation.yield(.exited(code)) + case .authCancelled: continuation.yield(.authCancelled) + case .launchFailed: continuation.yield(.exited(127)) + } + continuation.finish() + + case .osascript: + // Forward the legacy stream verbatim, including its + // cancellation behaviour. + let task = Task { + for await event in fallback.events(spec) { continuation.yield(event) } + continuation.finish() + } + continuation.onTermination = { @Sendable _ in task.cancel() } + } + } + } + } +} + +// MARK: - Output sink + +/// Receives streamed lines from the daemon. Strips ANSI on the CLIENT side, so +/// the root process carries no text-processing code it doesn't need. +private final class HelperOutputSink: NSObject, BurrowHelperClientProtocol { + private let onLine: (String) -> Void + + init(onLine: @escaping (String) -> Void) { + self.onLine = onLine + super.init() + } + + func helperDidEmit(line: String, operationID: String) { + onLine(Ansi.strip(line)) + } +} diff --git a/macos/Sources/SettingsView.swift b/macos/Sources/SettingsView.swift index dd51ab93..5af17c0a 100644 --- a/macos/Sources/SettingsView.swift +++ b/macos/Sources/SettingsView.swift @@ -115,6 +115,9 @@ struct SettingsView: View { @State private var touchIDEnabled = false @State private var touchIDBusy = false @State private var touchIDAvailable = false + @State private var helperStatus: HelperRegistrationStatus = .notRegistered + @State private var helperBusy = false + @State private var helperError: String? /// Drop-in MCP config for Claude Code / Cursor / Codex / Cline — they /// all share the same `{command, args}` stdio shape, so one snippet @@ -165,6 +168,7 @@ struct SettingsView: View { } .onAppear { refreshStatusLabels(); loadMoleVersion(); loadTouchIDStatus(); loadLaunchAtLogin() + loadHelperStatus() whitelistPatterns = MoleWhitelist.live.patterns() menuBarSuppressed = AppDelegate.shared?.menuBarSuppressedByCompatibilityGuard ?? false } @@ -632,6 +636,19 @@ struct SettingsView: View { footnote("Sends anonymous product analytics (PostHog) plus crash, hang, startup, update, and sampled performance diagnostics (Sentry): random install IDs, app and exact macOS build, CPU type, screens and features used, and fixed-name diagnostic milestones. Never screenshots, screen recordings, your file names, contents, user paths, URLs, or metrics. On by default; turn it off and both stop. Full list in TELEMETRY.md.") } + section("Privileged helper", "lock.shield") { + infoRow("Status", helperStatusLabel) + HStack { + Spacer() + if helperBusy { ProgressView().controlSize(.small).padding(.trailing, 4) } + PillButton(title: helperInstalled ? "Remove" : "Install", filled: false) { toggleHelper() } + } + if let helperError { + footnote(helperError) + } + footnote("Runs Burrow's admin operations — scan, clean, optimize — through a small signed helper instead of a password-only prompt, so macOS can offer Touch ID. Installing it needs your approval once. It grants no standing access: every operation that runs as administrator still asks you to authenticate, every time, and the helper can only perform those three operations — it cannot be asked to run anything else.") + } + section("Touch ID for sudo", "touchid") { infoRow("Status", touchIDStatus) if touchIDAvailable { @@ -641,7 +658,7 @@ struct SettingsView: View { PillButton(title: touchIDEnabled ? "Disable" : "Enable", filled: false) { toggleTouchID() } } } - footnote("Lets `sudo` in a terminal accept your fingerprint instead of a password — including `mo` commands you run yourself. It does NOT change Burrow's own admin prompts: those go through macOS authorization, which asks for your password regardless. Configured via `mo touchid` (pam_tid); turning it on or off needs your password once.") + footnote("Lets `sudo` in a terminal accept your fingerprint instead of a password — including `mo` commands you run yourself. Separate from Burrow's own admin prompts, which are covered by the privileged helper above. Configured via `mo touchid` (pam_tid); turning it on or off needs your password once.") } section("Mole engine", "shippingbox") { @@ -709,6 +726,53 @@ struct SettingsView: View { } } + // MARK: - Privileged helper + + private var helperInstalled: Bool { helperStatus != .notRegistered } + + private var helperStatusLabel: String { + switch helperStatus { + case .enabled: return "Installed" + case .requiresApproval: return "Waiting for your approval in Login Items & Extensions" + case .notRegistered: return "Not installed" + } + } + + private func loadHelperStatus() { + let status = PrivilegedHelperClient.shared.registrationStatus + DispatchQueue.main.async { helperStatus = status } + } + + /// Install or remove the daemon. Both directions can throw — macOS refuses + /// registration if the user declines, and we surface that rather than + /// leaving the row silently unchanged. + private func toggleHelper() { + guard !helperBusy else { return } + helperBusy = true + helperError = nil + let install = !helperInstalled + DispatchQueue.global(qos: .userInitiated).async { + var failure: String? + do { + if install { + try PrivilegedHelperClient.shared.register() + } else { + try PrivilegedHelperClient.shared.unregister() + } + } catch { + failure = install + ? "Couldn't install the helper. Burrow keeps using the password prompt." + : "Couldn't remove the helper. You can also turn it off in System Settings ▸ General ▸ Login Items & Extensions." + } + let status = PrivilegedHelperClient.shared.registrationStatus + DispatchQueue.main.async { + helperBusy = false + helperError = failure + helperStatus = status + } + } + } + // MARK: - Touch ID for sudo /// Whether this Mac actually has a Touch ID sensor. `mo touchid status` diff --git a/macos/Tests/HelperAuthorizationTests.swift b/macos/Tests/HelperAuthorizationTests.swift new file mode 100644 index 00000000..6582c9b3 --- /dev/null +++ b/macos/Tests/HelperAuthorizationTests.swift @@ -0,0 +1,155 @@ +// +// HelperAuthorizationTests.swift +// BurrowTests +// +// The authorization policy behind every root operation. The decision the +// product made is narrow and unusually strict, so it is pinned here rather +// than left to a comment: +// +// every operation that actually runs as root requires a FRESH user +// authentication — no grace period, no cached approval, no +// "authenticate once for this launch", and registering the helper does +// not itself authorize anything. +// +// Two of those words are literally keys in the right's definition (`timeout` +// and `shared`), so a regression that reintroduces a credential cache is a +// one-line diff that these tests catch. +// +// Nothing here prompts, authenticates, or touches the policy database: the +// right definition is a dictionary and the OSStatus mapping is a pure +// function, so both are checkable with no root and no UI. +// + +import XCTest +import Security +@testable import Burrow + +final class HelperAuthorizationTests: XCTestCase { + + // MARK: - The right's identity + + /// The right is namespaced under the app's bundle identifier so it can + /// never collide with, or be satisfied by, a system right a user may have + /// already been granted for something else. + func testRightName_isNamespacedUnderTheBundleIdentifier() { + XCTAssertTrue(HelperAuthorization.rightName.hasPrefix("dev.caezium.Burrow."), + "the right must live in Burrow's own namespace") + XCTAssertFalse(HelperAuthorization.rightName.hasPrefix("system."), + "never reuse or shadow a system right") + } + + // MARK: - No caching, ever (the decision, as policy keys) + + func testRightDefinition_expiresImmediatelySoEveryRunReauthenticates() { + let definition = HelperAuthorization.rightDefinition + XCTAssertEqual(definition["timeout"] as? Int, 0, + "timeout 0 = the credential is dead on arrival; the next root op prompts again") + } + + func testRightDefinition_isNotSharedWithOtherProcessesOrRights() { + let definition = HelperAuthorization.rightDefinition + XCTAssertEqual(definition["shared"] as? Bool, false, + "a shared credential would let one prompt satisfy a later, different request") + } + + /// The daemon runs as root. If the right allowed root callers to skip + /// authentication, the daemon would authorize ITSELF and the prompt would + /// silently disappear — the single most dangerous misconfiguration + /// available here. + func testRightDefinition_doesNotLetTheRootDaemonAuthorizeItself() { + let definition = HelperAuthorization.rightDefinition + XCTAssertEqual(definition["allow-root"] as? Bool, false, + "the root daemon must never satisfy this right by virtue of being root") + } + + func testRightDefinition_requiresAnAdministratorToAuthenticate() { + let definition = HelperAuthorization.rightDefinition + XCTAssertEqual(definition["class"] as? String, "user") + XCTAssertEqual(definition["group"] as? String, "admin") + XCTAssertEqual(definition["authenticate-user"] as? Bool, true) + } + + /// A human-readable reason ships with the right so the system prompt says + /// what Burrow is about to do rather than showing a bare app name. + func testRightDefinition_carriesAPromptDescription() { + let definition = HelperAuthorization.rightDefinition + let comment = definition["comment"] as? String ?? "" + XCTAssertFalse(comment.isEmpty, "the right documents itself in the policy database") + } + + // MARK: - Flags (the CopyRights call shape) + // + // Two documented ways to get this wrong, both of which turn the check into + // a no-op: + // * omitting kAuthorizationFlagExtendRights — rights are never actually + // extended, and sloppy callers read the status as success; + // * passing kAuthorizationFlagPreAuthorize in the DAEMON — that only + // asks "could this be authorized later", which is not an authorization. + + func testDaemonFlags_extendRightsAndAllowInteraction() { + let flags = HelperAuthorization.daemonFlags + XCTAssertTrue(flags.contains(.extendRights), "without this no right is actually granted") + XCTAssertTrue(flags.contains(.interactionAllowed), "the daemon raises the prompt itself") + } + + func testDaemonFlags_neverPreAuthorizeOnly() { + XCTAssertFalse(HelperAuthorization.daemonFlags.contains(.preAuthorize), + "pre-authorization asks whether auth is POSSIBLE; the daemon must require it") + } + + /// The client does not authenticate — it only externalizes an empty + /// authorization reference for the daemon to evaluate. If the client ever + /// starts pre-authorizing, the prompt moves out of the privileged process + /// and the daemon's own check becomes decorative. + func testClientFlags_areInert() { + XCTAssertEqual(HelperAuthorization.clientFlags, [], + "the GUI creates the reference; the root daemon is what demands the right") + } + + // MARK: - OSStatus → outcome (pure, exhaustive) + + func testOutcome_successIsGranted() { + XCTAssertEqual(HelperAuthorization.outcome(from: errAuthorizationSuccess), .granted) + } + + func testOutcome_dismissedPromptIsCancelled() { + XCTAssertEqual(HelperAuthorization.outcome(from: errAuthorizationCanceled), .cancelled) + } + + func testOutcome_wrongPasswordOrRefusalIsDenied() { + XCTAssertEqual(HelperAuthorization.outcome(from: errAuthorizationDenied), .denied) + XCTAssertEqual(HelperAuthorization.outcome(from: OSStatus(errAuthorizationInteractionNotAllowed)), .denied) + } + + /// Anything unrecognised fails CLOSED. A status this code has never seen + /// must never fall through to "probably fine" — the operation is refused + /// and the raw status is preserved for diagnosis. + func testOutcome_unknownStatusFailsClosed() { + XCTAssertEqual(HelperAuthorization.outcome(from: OSStatus(-60999)), .failed(-60999)) + XCTAssertNotEqual(HelperAuthorization.outcome(from: OSStatus(-60999)), .granted) + } + + func testOutcome_onlyGrantedPermitsExecution() { + // The single predicate the daemon branches on, so "granted" can't be + // accidentally widened to "not an outright failure". + XCTAssertTrue(HelperAuthorization.Outcome.granted.permitsExecution) + for refused: HelperAuthorization.Outcome in [.denied, .cancelled, .failed(-1)] { + XCTAssertFalse(refused.permitsExecution, "\(refused) must not run anything as root") + } + } + + // MARK: - External form sizing + // + // The external form is a fixed-size C struct. A payload of any other size + // is malformed and is refused BEFORE it reaches + // AuthorizationCreateFromExternalForm, so a hostile client cannot feed the + // Security framework a short or oversized buffer. + + func testExternalForm_rejectsWrongSizedPayloads() { + let correct = MemoryLayout.size + XCTAssertTrue(HelperAuthorization.isPlausibleExternalForm(Data(count: correct))) + XCTAssertFalse(HelperAuthorization.isPlausibleExternalForm(Data())) + XCTAssertFalse(HelperAuthorization.isPlausibleExternalForm(Data(count: correct - 1))) + XCTAssertFalse(HelperAuthorization.isPlausibleExternalForm(Data(count: correct + 1))) + } +} diff --git a/macos/Tests/HelperCodeRequirementTests.swift b/macos/Tests/HelperCodeRequirementTests.swift new file mode 100644 index 00000000..b73c1ba0 --- /dev/null +++ b/macos/Tests/HelperCodeRequirementTests.swift @@ -0,0 +1,151 @@ +// +// HelperCodeRequirementTests.swift +// BurrowTests +// +// Who is allowed to talk to the root daemon at all. +// +// Authorization answers "may this operation run"; this answers the question +// that comes first — "is the process on the other end of this XPC connection +// actually Burrow". Without it, any local process could open the Mach service +// and at minimum drive the auth prompt, phishing the user for admin +// credentials with a dialog the system itself renders. +// +// The requirement string is built at RUNTIME from the daemon's own signing +// information, so nothing personal (a team ID, a certificate, a developer +// name) is ever hardcoded in the repository. These tests cover the pure +// string builder and the fail-closed rules around it; the SecCode calls +// themselves are a thin system witness. +// + +import XCTest +@testable import Burrow + +final class HelperCodeRequirementTests: XCTestCase { + + // MARK: - Distribution builds: identity is pinned to the signing team + + /// A Developer ID build pins THREE things: the exact bundle identifier, + /// an Apple-issued chain, and the signing team. Dropping any one of them + /// widens the caller set — identifier alone can be claimed by any ad-hoc + /// build, and anchor alone admits every Developer ID app on the machine. + func testRequirement_pinsIdentifierAnchorAndTeam() { + let requirement = HelperCodeRequirement.string(bundleID: "dev.caezium.Burrow", teamID: "ABCDE12345") + + XCTAssertTrue(requirement.contains(#"identifier "dev.caezium.Burrow""#)) + XCTAssertTrue(requirement.contains("anchor apple generic"), + "the chain must terminate at Apple, not at an arbitrary self-signed root") + XCTAssertTrue(requirement.contains(#"certificate leaf[subject.OU] = "ABCDE12345""#), + "same-team pinning is what stops another Developer ID app impersonating Burrow") + } + + func testRequirement_joinsEveryClauseWithAnd() { + let requirement = HelperCodeRequirement.string(bundleID: "dev.caezium.Burrow", teamID: "ABCDE12345") + // Three clauses, all mandatory — an `or` anywhere would make one optional. + XCTAssertEqual(requirement.components(separatedBy: " and ").count, 3) + XCTAssertFalse(requirement.contains(" or "), "no clause may be optional") + } + + // MARK: - Unsigned / ad-hoc builds fail closed + + /// A local Debug build is ad-hoc signed and has no team. It still gets a + /// requirement (identifier only) so development works, but it is explicitly + /// NOT distribution grade — and the release gate refuses to ship a helper + /// whose requirement can't name a team. + func testRequirement_adHocBuildFallsBackToIdentifierOnly() { + let requirement = HelperCodeRequirement.string(bundleID: "dev.caezium.Burrow", teamID: nil) + XCTAssertEqual(requirement, #"identifier "dev.caezium.Burrow""#) + } + + func testDistributionGrade_requiresATeam() { + XCTAssertTrue(HelperCodeRequirement.isDistributionGrade(teamID: "ABCDE12345")) + XCTAssertFalse(HelperCodeRequirement.isDistributionGrade(teamID: nil), + "an ad-hoc helper must never pass the release gate") + XCTAssertFalse(HelperCodeRequirement.isDistributionGrade(teamID: "")) + XCTAssertFalse(HelperCodeRequirement.isDistributionGrade(teamID: " ")) + } + + // MARK: - Injection into the requirement language + // + // The requirement string is parsed by the Security framework as a small + // language. Identifiers and team IDs are interpolated into it, so a value + // carrying a quote could close the literal early and append a clause — + // `identifier "x" or anchor apple` would admit every Apple-signed process + // on the machine. Values that cannot ride inertly are REFUSED, not escaped: + // a legitimate bundle ID or team ID never contains these characters, so + // rejecting is strictly safer than sanitising. + + func testSanitisation_rejectsQuotesAndRequirementOperators() { + for hostile in [#"dev.caezium.Burrow" or anchor apple generic and identifier "x"#, + #"dev.caezium.Burrow""#, + "dev.caezium.Burrow\\", + "dev.caezium.Burrow and anchor apple", + "dev.caezium.Burrow\nidentifier", + "dev.caezium.Burrow\u{0}"] { + XCTAssertNil(HelperCodeRequirement.validated(identifier: hostile), + "hostile identifier must be refused: \(hostile.debugDescription)") + } + } + + func testSanitisation_acceptsRealisticIdentifiersAndTeams() { + XCTAssertEqual(HelperCodeRequirement.validated(identifier: "dev.caezium.Burrow"), "dev.caezium.Burrow") + XCTAssertEqual(HelperCodeRequirement.validated(identifier: "dev.caezium.Burrow.helper"), + "dev.caezium.Burrow.helper") + XCTAssertEqual(HelperCodeRequirement.validated(identifier: "ABCDE12345"), "ABCDE12345") + XCTAssertEqual(HelperCodeRequirement.validated(identifier: "A-B_C.D"), "A-B_C.D") + } + + func testSanitisation_rejectsEmptyAndOverlongValues() { + XCTAssertNil(HelperCodeRequirement.validated(identifier: "")) + XCTAssertNil(HelperCodeRequirement.validated(identifier: " ")) + XCTAssertNil(HelperCodeRequirement.validated(identifier: String(repeating: "a", count: 300))) + } + + /// A hostile value must take the whole requirement down with it, not + /// silently produce a weaker one. `string(bundleID:teamID:)` returns the + /// fail-closed sentinel that matches NOTHING when it can't build safely. + func testRequirement_hostileInputProducesAnUnsatisfiableRequirement() { + let requirement = HelperCodeRequirement.string(bundleID: #"x" or anchor apple generic"#, + teamID: "ABCDE12345") + XCTAssertEqual(requirement, HelperCodeRequirement.unsatisfiable) + XCTAssertFalse(requirement.contains("or anchor apple generic")) + } + + func testRequirement_hostileTeamProducesAnUnsatisfiableRequirement() { + let requirement = HelperCodeRequirement.string(bundleID: "dev.caezium.Burrow", + teamID: #"A" or anchor apple"#) + XCTAssertEqual(requirement, HelperCodeRequirement.unsatisfiable) + } + + /// The sentinel must be a syntactically valid requirement that no code can + /// satisfy — an empty string would be a parse error, and some call sites + /// treat a parse error as "no requirement", which is the opposite of what + /// we want. + func testUnsatisfiableSentinel_isValidSyntaxThatMatchesNothing() { + XCTAssertFalse(HelperCodeRequirement.unsatisfiable.isEmpty) + XCTAssertTrue(HelperCodeRequirement.unsatisfiable.contains("identifier")) + XCTAssertNil(HelperCodeRequirement.validated(identifier: HelperCodeRequirement.unsatisfiable)) + } + + // MARK: - Version skew between app and helper + // + // The installed helper outlives the app that installed it: Sparkle can + // replace Burrow.app underneath a registered daemon. A helper from an + // older build runs as root with an older idea of what `clean` does, so + // skew is refused rather than tolerated. + + func testVersionSkew_matchingBuildsAreCompatible() { + XCTAssertEqual(HelperVersionSkew.evaluate(appBuild: "23", helperBuild: "23"), .matched) + } + + func testVersionSkew_anyDifferenceRequiresReregistration() { + XCTAssertEqual(HelperVersionSkew.evaluate(appBuild: "24", helperBuild: "23"), .mismatched) + XCTAssertEqual(HelperVersionSkew.evaluate(appBuild: "23", helperBuild: "24"), .mismatched) + } + + func testVersionSkew_missingOrUnreadableBuildIsMismatched() { + // A helper that won't say what it is gets no root work. Fail closed. + XCTAssertEqual(HelperVersionSkew.evaluate(appBuild: "23", helperBuild: ""), .mismatched) + XCTAssertEqual(HelperVersionSkew.evaluate(appBuild: "", helperBuild: "23"), .mismatched) + XCTAssertEqual(HelperVersionSkew.evaluate(appBuild: "", helperBuild: ""), .mismatched) + } +} diff --git a/macos/Tests/HelperContractTests.swift b/macos/Tests/HelperContractTests.swift new file mode 100644 index 00000000..2b13830e --- /dev/null +++ b/macos/Tests/HelperContractTests.swift @@ -0,0 +1,194 @@ +// +// HelperContractTests.swift +// BurrowTests +// +// The typed-request boundary of the privileged helper. Everything asserted +// here runs in memory: no daemon, no XPC, no root, no auth prompt. +// +// The whole security argument for the helper rests on ONE claim — a client +// cannot describe an arbitrary command, only pick from a closed set of +// Burrow operations. These tests are what make that claim checkable: +// +// * the operation set is closed and each member maps to a FIXED argv the +// client never contributes a single token to; +// * a request that fails validation produces a named rejection, never a +// "best effort" run; +// * an operation ID cannot be replayed. +// +// If someone later widens the contract to carry a path, an argv element, or +// a command string, the argv tests below stop compiling or fail. That is the +// point. +// + +import XCTest +@testable import Burrow + +final class HelperContractTests: XCTestCase { + + // MARK: - The closed operation set + // + // The approved scope is exactly: privileged scan, clean, optimize. Not + // "run this binary", not "run this shell string", not "run mo with these + // args". A new case here is a deliberate security decision, so the count + // is pinned — adding one without updating this test is a failing build. + + func testOperationSet_isExactlyScanCleanOptimize() { + XCTAssertEqual(Set(HelperOperation.allCases.map(\.rawValue)), + ["scan", "clean", "optimize"], + "the helper's operation set is closed; widening it is a security decision") + } + + // MARK: - Fixed argv (the client contributes nothing) + // + // These are the exact argv the GUI passes today through the osascript + // path (CleanView `["clean"]`, OptimizeView `["optimize"]`, previews with + // `--dry-run`). The helper reproduces them from the enum ALONE, so an + // attacker who fully controls the XPC payload still cannot add a flag. + + func testEngineArguments_areFixedPerOperation() { + XCTAssertEqual(HelperOperation.scan.engineArguments, ["clean", "--dry-run"]) + XCTAssertEqual(HelperOperation.clean.engineArguments, ["clean"]) + XCTAssertEqual(HelperOperation.optimize.engineArguments, ["optimize"]) + } + + func testEngineArguments_neverEmptyAndNeverShellMetacharacters() { + // argv goes to posix_spawn, never a shell — but a stray metacharacter + // would still signal that someone started templating strings in here. + for op in HelperOperation.allCases { + XCTAssertFalse(op.engineArguments.isEmpty, "\(op) must resolve to a real command") + for token in op.engineArguments { + XCTAssertFalse(token.contains(where: { ";|&`$<>\n\0".contains($0) }), + "\(op) argv token \(token) carries shell/NUL metacharacters") + } + } + } + + /// Decoding is the ONLY way a request enters the daemon, and the operation + /// is an enum — an unknown verb fails to decode rather than falling through + /// to some default. This is the test that keeps "run" from ever being a + /// smuggled operation. + func testDecoding_rejectsUnknownOperation() throws { + let payload = #"{"operation":"run","operationID":"\#(UUID().uuidString)","clientBuild":"23"}"# + XCTAssertThrowsError(try JSONDecoder().decode(HelperRequest.self, from: Data(payload.utf8))) + } + + func testDecoding_roundTripsEveryOperation() throws { + for op in HelperOperation.allCases { + let request = HelperRequest(operation: op, operationID: UUID().uuidString, clientBuild: "23") + let data = try JSONEncoder().encode(request) + XCTAssertEqual(try JSONDecoder().decode(HelperRequest.self, from: data), request) + } + } + + // MARK: - Validation (named rejections, never a partial run) + + func testValidate_acceptsAWellFormedRequest() { + let request = HelperRequest(operation: .clean, operationID: UUID().uuidString, clientBuild: "23") + XCTAssertNil(request.validate(expectedBuild: "23")) + } + + /// A non-UUID operation ID is rejected outright. The ID is the replay key, + /// so a client-chosen constant ("1") would let a single authorization be + /// reused; requiring a UUID makes every request distinguishable. + func testValidate_rejectsNonUUIDOperationID() { + for bad in ["", "1", "not-a-uuid", String(repeating: "a", count: 400)] { + let request = HelperRequest(operation: .clean, operationID: bad, clientBuild: "23") + XCTAssertEqual(request.validate(expectedBuild: "23"), .malformedOperationID, + "operation ID \(bad.prefix(12)) must be rejected") + } + } + + /// Version skew is a rejection, not a "try anyway". A stale helper paired + /// with a new app could hold an older idea of what `clean` does, and it + /// runs as root — so the mismatch stops the operation and the GUI + /// re-registers instead. + func testValidate_rejectsBuildMismatch() { + let request = HelperRequest(operation: .clean, operationID: UUID().uuidString, clientBuild: "22") + XCTAssertEqual(request.validate(expectedBuild: "23"), .buildMismatch) + } + + func testValidate_rejectsEmptyBuild() { + let request = HelperRequest(operation: .clean, operationID: UUID().uuidString, clientBuild: "") + XCTAssertEqual(request.validate(expectedBuild: "23"), .buildMismatch) + } + + // MARK: - Replay resistance + // + // One authorization authorizes ONE operation. The daemon remembers the IDs + // it has already served so a captured payload (auth external form included) + // cannot be re-sent to get a second root run out of one prompt. + + func testReplayGuard_admitsEachIDOnce() { + let guardian = HelperReplayGuard() + let id = UUID().uuidString + XCTAssertTrue(guardian.admit(id), "first use of an ID is allowed") + XCTAssertFalse(guardian.admit(id), "the same ID must never be served twice") + XCTAssertFalse(guardian.admit(id)) + } + + func testReplayGuard_distinctIDsAllPass() { + let guardian = HelperReplayGuard() + for _ in 0..<50 { + XCTAssertTrue(guardian.admit(UUID().uuidString)) + } + } + + /// The guard is bounded so a long-lived daemon can't be grown without + /// limit by a client that just keeps sending fresh IDs. Eviction is + /// oldest-first, and the CURRENT id is always remembered — so the replay + /// window can only ever shrink for ancient ids, never for a live one. + func testReplayGuard_isBoundedAndEvictsOldestFirst() { + let guardian = HelperReplayGuard(capacity: 3) + let ids = (0..<4).map { _ in UUID().uuidString } + for id in ids { XCTAssertTrue(guardian.admit(id)) } + + XCTAssertTrue(guardian.admit(ids[0]), "the oldest ID was evicted once capacity was exceeded") + XCTAssertFalse(guardian.admit(ids[3]), "the newest ID is still remembered") + XCTAssertEqual(guardian.count, 3, "the guard never grows past its capacity") + } + + // MARK: - Response encoding + // + // The reply crosses XPC as bytes, so it round-trips like the request. The + // failure cases are NAMED (matching the existing ElevatedOutcome taxonomy) + // so the GUI shows the right message instead of re-deriving meaning from a + // bare nonzero exit — the same rule issue #48 established for osascript. + + func testResponse_roundTripsEveryOutcome() throws { + let outcomes: [HelperResponse.Outcome] = [ + .exited(0), .exited(2), + .authorizationCancelled, + .authorizationDenied, + .rejected(.buildMismatch), + .rejected(.replayedOperationID), + .engineUnavailable, + ] + for outcome in outcomes { + let data = try JSONEncoder().encode(HelperResponse(outcome: outcome)) + XCTAssertEqual(try JSONDecoder().decode(HelperResponse.self, from: data).outcome, outcome) + } + } + + /// The GUI still has call sites that branch on an Int32. Every failure + /// shape must collapse to a NONZERO code, exactly as `ElevatedOutcome` + /// does today — a cancelled prompt must never read as success. + func testResponse_everyFailureIsNonzeroToLegacyCallers() { + XCTAssertEqual(HelperResponse.Outcome.exited(0).exitCode, 0) + XCTAssertEqual(HelperResponse.Outcome.exited(3).exitCode, 3) + for failure: HelperResponse.Outcome in [.authorizationCancelled, .authorizationDenied, + .rejected(.malformedPayload), .engineUnavailable] { + XCTAssertNotEqual(failure.exitCode, 0, "\(failure) must not read as success") + } + } + + /// A cancelled authorization maps onto the SAME user-facing meaning as the + /// osascript path's `.authCancelled`, so both elevation routes tell the + /// user the same thing and the GUI needs no second taxonomy. + func testResponse_mapsOntoTheExistingElevatedOutcomeTaxonomy() { + XCTAssertEqual(HelperResponse.Outcome.exited(0).elevatedOutcome, .exited(0)) + XCTAssertEqual(HelperResponse.Outcome.exited(7).elevatedOutcome, .exited(7)) + XCTAssertEqual(HelperResponse.Outcome.authorizationCancelled.elevatedOutcome, .authCancelled) + XCTAssertEqual(HelperResponse.Outcome.engineUnavailable.elevatedOutcome, .launchFailed) + XCTAssertEqual(HelperResponse.Outcome.authorizationDenied.elevatedOutcome, .authCancelled) + } +} diff --git a/macos/Tests/PrivilegeRouteTests.swift b/macos/Tests/PrivilegeRouteTests.swift new file mode 100644 index 00000000..2176c058 --- /dev/null +++ b/macos/Tests/PrivilegeRouteTests.swift @@ -0,0 +1,134 @@ +// +// PrivilegeRouteTests.swift +// BurrowTests +// +// Which elevation route an operation takes, and — more importantly — which +// ones it must NOT take. +// +// The helper and the osascript path both end in an administrator prompt and +// both run the same trusted engine, so falling back is safe. What would not +// be safe is the reverse: quietly widening what the helper accepts, or +// routing to a root daemon whose build no longer matches the app driving it. +// Every guard below is one of those. +// +// Pure decisions, no daemon, no registration, no XPC. +// + +import XCTest +@testable import Burrow + +final class PrivilegeRouteTests: XCTestCase { + + // MARK: - argv → typed operation + // + // The migration seam. `OperationFlow` still describes elevated work as + // argv, and this is the ONLY place that argv is allowed to become a typed + // operation. Anything it doesn't recognise keeps the old route rather than + // being forwarded as an approximate match. + + func testRecognition_mapsTheThreeKnownCallSites() { + // Exactly what CleanView, OptimizeView, and the preview path pass today. + XCTAssertEqual(HelperOperation(engineArguments: ["clean"]), .clean) + XCTAssertEqual(HelperOperation(engineArguments: ["optimize"]), .optimize) + XCTAssertEqual(HelperOperation(engineArguments: ["clean", "--dry-run"]), .scan) + } + + func testRecognition_isExactAndOrderSensitive() { + // A near-miss is not a match. Extra flags, reordering, or a different + // verb all fall through to osascript rather than being coerced into + // the closest typed operation. + XCTAssertNil(HelperOperation(engineArguments: ["--dry-run", "clean"])) + XCTAssertNil(HelperOperation(engineArguments: ["clean", "--yes"])) + XCTAssertNil(HelperOperation(engineArguments: ["clean", "--dry-run", "--verbose"])) + XCTAssertNil(HelperOperation(engineArguments: ["uninstall", "Safari"])) + XCTAssertNil(HelperOperation(engineArguments: [])) + } + + /// The one that matters: an attacker-shaped argv must never resolve to an + /// operation. It can't, because recognition is equality against three + /// fixed arrays — but this is the assertion that says so out loud. + func testRecognition_neverAcceptsInjectedArgv() { + for hostile in [["clean", "; rm -rf /"], + ["clean", "--dry-run", "&&", "curl", "evil"], + ["/bin/sh"], + ["clean\0"], + ["clean\n--force"]] { + XCTAssertNil(HelperOperation(engineArguments: hostile), + "\(hostile) must not resolve to a privileged operation") + } + } + + // MARK: - The routing rule + + func testRoute_usesTheHelperWhenEverythingLinesUp() { + XCTAssertEqual(PrivilegeRoute.decide(arguments: ["clean"], + registration: .enabled, + skew: .matched), + .helper(.clean)) + } + + func testRoute_unrecognisedArgvKeepsTheLegacyPath() { + XCTAssertEqual(PrivilegeRoute.decide(arguments: ["uninstall", "Safari"], + registration: .enabled, + skew: .matched), + .osascript) + } + + /// A daemon that isn't registered, or that the user hasn't approved, is + /// not a daemon we may use. Registration is the user's decision, and + /// declining it must leave Burrow working exactly as before. + func testRoute_unregisteredOrUnapprovedKeepsTheLegacyPath() { + for registration: HelperRegistrationStatus in [.notRegistered, .requiresApproval] { + XCTAssertEqual(PrivilegeRoute.decide(arguments: ["clean"], + registration: registration, + skew: .matched), + .osascript, + "registration \(registration) must not reach the helper") + } + } + + /// The sharp one. A registered daemon OUTLIVES the app that installed it — + /// Sparkle can replace Burrow.app underneath it — so a helper from an + /// older build could be running as root with an older idea of what `clean` + /// does. Skew routes away from the helper entirely. + func testRoute_versionSkewKeepsTheLegacyPath() { + XCTAssertEqual(PrivilegeRoute.decide(arguments: ["clean"], + registration: .enabled, + skew: .mismatched), + .osascript) + } + + /// Every combination, so no future edit can produce a `.helper` route from + /// a state that isn't fully green. + func testRoute_helperRequiresEveryConditionSimultaneously() { + let registrations: [HelperRegistrationStatus] = [.enabled, .notRegistered, .requiresApproval] + let skews: [HelperVersionSkew.Skew] = [.matched, .mismatched] + let argvs = [["clean"], ["optimize"], ["clean", "--dry-run"], ["uninstall"], []] + + for registration in registrations { + for skew in skews { + for argv in argvs { + let route = PrivilegeRoute.decide(arguments: argv, registration: registration, skew: skew) + let allGreen = registration == .enabled + && skew == .matched + && HelperOperation(engineArguments: argv) != nil + if allGreen { + XCTAssertNotEqual(route, .osascript) + } else { + XCTAssertEqual(route, .osascript, + "argv \(argv), registration \(registration), skew \(skew)") + } + } + } + } + } + + // MARK: - Registration status mapping + + func testRegistrationStatus_needsUserActionOnlyWhenApprovalIsPending() { + XCTAssertTrue(HelperRegistrationStatus.requiresApproval.needsUserAction) + XCTAssertFalse(HelperRegistrationStatus.enabled.needsUserAction) + XCTAssertFalse(HelperRegistrationStatus.notRegistered.needsUserAction, + "never registered is the default state, not a pending decision") + } +} diff --git a/macos/project.yml b/macos/project.yml index 66e594af..ff6ee8de 100644 --- a/macos/project.yml +++ b/macos/project.yml @@ -45,7 +45,17 @@ targets: sources: - path: Sources - path: Resources - excludes: ["Info.plist", "Burrow.entitlements"] + # LaunchDaemons is excluded from the plain resource copy and staged by + # the dedicated phase below instead: a stray second copy under + # Resources/ would be dead weight inside the signed bundle. + excludes: ["Info.plist", "Burrow.entitlements", "LaunchDaemons"] + # SMAppService.daemon(plistName:) resolves the daemon from exactly this + # path and nowhere else. + - path: Resources/LaunchDaemons/dev.caezium.Burrow.helper.plist + buildPhase: + copyFiles: + destination: wrapper + subpath: Contents/Library/LaunchDaemons info: path: Resources/Info.plist properties: @@ -102,6 +112,15 @@ targets: embed: true - framework: vendor/Sentry.xcframework embed: false + # The privileged helper ships INSIDE the app. It is never installed to + # /Library/PrivilegedHelperTools, so it cannot drift from, or outlive, + # the app that vouches for it — SMAppService resolves it through the + # bundle via the BundleProgram key in the launchd plist. + # Copied into Contents/MacOS, beside the app's own executable, which is + # where BundleProgram in the launchd plist points. + - target: BurrowHelper + copy: + destination: executables postBuildScripts: - name: Bundle burrow-engine (MIT) basedOnDependencyAnalysis: false @@ -155,6 +174,38 @@ targets: "$SRCROOT/scripts/bundle-fclones.sh" "$TARGET_BUILD_DIR/$UNLOCALIZED_RESOURCES_FOLDER_PATH" \ || echo "warning: fclones bundling skipped — Duplicates falls back to a system/\$BURROW_FCLONES fclones." + # The privileged helper: a bare launchd daemon that runs as root. + # + # Deliberately minimal. It compiles ONLY the shared contract in + # Sources/PrivilegedHelper plus its own HelperSources — no SwiftUI, no + # Sparkle, no Sentry, no telemetry. Nothing that could pull UI, networking, + # or crash-reporting code into a root process, and nothing that could grow a + # dependency without that showing up as a change to this list. + BurrowHelper: + type: tool + platform: macOS + sources: + # Shared with the app so the two can never disagree about what a request + # means. Same files, both targets. + - path: Sources/PrivilegedHelper + - path: HelperSources + settings: + base: + PRODUCT_NAME: BurrowHelper + PRODUCT_BUNDLE_IDENTIFIER: dev.caezium.Burrow.helper + # The helper reports its OWN build over XPC, and the client refuses a + # helper whose build doesn't match the app's. Baking the plist into the + # binary means it reports what it IS, rather than reading whatever + # bundle happens to surround it after an update. + CREATE_INFOPLIST_SECTION_IN_BINARY: YES + GENERATE_INFOPLIST_FILE: YES + # Must track the app's CFBundleVersion above — a mismatch is exactly + # what HelperVersionSkew refuses, so a stale value here disables the + # helper rather than shipping a subtly wrong one. + CURRENT_PROJECT_VERSION: "23" + MARKETING_VERSION: "0.11.2" + SKIP_INSTALL: YES + BurrowTests: type: bundle.unit-test platform: macOS diff --git a/scripts/sign-macos-app.sh b/scripts/sign-macos-app.sh index e1aaefc8..b5974a74 100755 --- a/scripts/sign-macos-app.sh +++ b/scripts/sign-macos-app.sh @@ -201,4 +201,63 @@ else fi fi +# --------------------------------------------------------------------------- +# Privileged helper gate. +# +# The helper runs as ROOT, so a build that ships it half-wired is worse than +# one that doesn't ship it at all: a missing plist key or an unsigned binary +# turns into a launchd failure, or worse a daemon nobody vetted. Every check +# below is fail-closed, and the whole block is mandatory — the helper is not an +# optional component, so "absent" is an error, not a skip. +HELPER="$APP/Contents/MacOS/BurrowHelper" +HELPER_PLIST="$APP/Contents/Library/LaunchDaemons/dev.caezium.Burrow.helper.plist" + +echo "==> verifying the privileged helper" +[ -f "$HELPER" ] || { echo "error: privileged helper missing: $HELPER" >&2; exit 1; } +is_macho "$HELPER" || { echo "error: privileged helper is not a Mach-O binary" >&2; exit 1; } +[ -f "$HELPER_PLIST" ] || { echo "error: launchd plist missing: $HELPER_PLIST" >&2; exit 1; } +plutil -lint "$HELPER_PLIST" >/dev/null \ + || { echo "error: launchd plist is not a valid property list" >&2; exit 1; } + +helper_plist_value() { + plutil -extract "$1" raw -expect "$2" -o - "$HELPER_PLIST" 2>/dev/null +} + +# The label and the Mach service name are what SMAppService and the client +# connect through. A typo in either is a silent "helper never starts". +HELPER_LABEL="$(helper_plist_value Label string || true)" +[ "$HELPER_LABEL" = "dev.caezium.Burrow.helper" ] \ + || { echo "error: launchd Label is '${HELPER_LABEL:-missing}', expected dev.caezium.Burrow.helper" >&2; exit 1; } + +helper_plist_value 'MachServices.dev\.caezium\.Burrow\.helper' bool >/dev/null \ + || { echo "error: launchd plist does not vend the dev.caezium.Burrow.helper Mach service" >&2; exit 1; } + +# BundleProgram is resolved relative to the app bundle. If it points anywhere +# other than the executable we just signed, the daemon that actually runs as +# root is not the one this pipeline verified. +HELPER_PROGRAM="$(helper_plist_value BundleProgram string || true)" +[ "$HELPER_PROGRAM" = "Contents/MacOS/BurrowHelper" ] \ + || { echo "error: BundleProgram is '${HELPER_PROGRAM:-missing}', expected Contents/MacOS/BurrowHelper" >&2; exit 1; } +[ -f "$APP/$HELPER_PROGRAM" ] \ + || { echo "error: BundleProgram does not resolve to a file inside the app" >&2; exit 1; } + +codesign --verify --strict --verbose=2 "$HELPER" \ + || { echo "error: privileged helper failed strict signature verification" >&2; exit 1; } + +if [ "$MODE" = "developer-id" ]; then + # The client pins callers to our signing team, and the helper verifies the + # engine the same way. An ad-hoc helper has no team to pin, so it would fall + # back to identifier-only matching — fine for local development, never for a + # release. + HELPER_TEAM="$(codesign -d --verbose=4 "$HELPER" 2>&1 | awk -F= '$1 == "TeamIdentifier" { print $2; exit }')" + [ "$HELPER_TEAM" = "$EXPECTED_TEAM" ] \ + || { echo "error: helper team is '${HELPER_TEAM:-missing}', expected '$EXPECTED_TEAM'" >&2; exit 1; } + + # Hardened runtime on a root daemon is not optional. + codesign -d --verbose=2 "$HELPER" 2>&1 | grep -q "flags=.*runtime" \ + || { echo "error: privileged helper is not built with the hardened runtime" >&2; exit 1; } +fi + +echo "privileged helper verified: signed, hardened, and correctly declared to launchd" + echo "signed $SIGNED_MACHO Mach-O file(s) and $SIGNED_CONTAINERS code container(s); strict verification passed" diff --git a/scripts/tests/test_sign_macos_app.py b/scripts/tests/test_sign_macos_app.py index 863c58e6..e9ad02a9 100644 --- a/scripts/tests/test_sign_macos_app.py +++ b/scripts/tests/test_sign_macos_app.py @@ -17,6 +17,9 @@ def make_app( root: Path, *, get_task_allow: bool | str | None = None, + helper: bool = True, + helper_plist_overrides: dict[str, object] | None = None, + helper_plist: bool = True, ) -> tuple[Path, Path]: app = root / "Burrow.app" executable = app / "Contents" / "MacOS" / "Burrow" @@ -26,6 +29,30 @@ def make_app( shutil.copyfile("/usr/bin/true", executable) executable.chmod(0o755) + # The privileged helper and its launchd declaration. Present by + # default because the signer treats them as mandatory: a release that + # ships the helper half-wired is worse than one without it, so + # "absent" has to be an error rather than a skip. + if helper: + helper_binary = app / "Contents" / "MacOS" / "BurrowHelper" + shutil.copyfile("/usr/bin/true", helper_binary) + helper_binary.chmod(0o755) + + if helper_plist: + plist_path = ( + app / "Contents" / "Library" / "LaunchDaemons" + / "dev.caezium.Burrow.helper.plist" + ) + plist_path.parent.mkdir(parents=True, exist_ok=True) + contents: dict[str, object] = { + "Label": "dev.caezium.Burrow.helper", + "BundleProgram": "Contents/MacOS/BurrowHelper", + "MachServices": {"dev.caezium.Burrow.helper": True}, + } + contents.update(helper_plist_overrides or {}) + with plist_path.open("wb") as handle: + plistlib.dump(contents, handle) + with (app / "Contents" / "Info.plist").open("wb") as handle: plistlib.dump( { @@ -150,5 +177,100 @@ def test_present_get_task_allow_extraction_failure_blocks_release(self) -> None: self.assertIn("could not extract plist key", result.stderr) +class PrivilegedHelperGateTests(SignMacOSAppTests): + """The signer's fail-closed checks on the root helper. + + Each case here is a way to ship a helper that launchd would either refuse + to start or, worse, start as something other than the binary this pipeline + verified. None of them is caught by `codesign --verify --deep`, which is + why they are checked explicitly. + """ + + def test_missing_helper_binary_blocks_release(self) -> None: + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + app, entitlements = self.make_app(root, helper=False) + + result = self.run_signer(app, entitlements) + + self.assertNotEqual(result.returncode, 0) + self.assertIn("privileged helper missing", result.stderr) + + def test_missing_launchd_plist_blocks_release(self) -> None: + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + app, entitlements = self.make_app(root, helper_plist=False) + + result = self.run_signer(app, entitlements) + + self.assertNotEqual(result.returncode, 0) + self.assertIn("launchd plist missing", result.stderr) + + def test_wrong_label_blocks_release(self) -> None: + # SMAppService and launchd both key off the label; a typo means a + # daemon that silently never runs. + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + app, entitlements = self.make_app( + root, helper_plist_overrides={"Label": "dev.caezium.Burrow.helpr"} + ) + + result = self.run_signer(app, entitlements) + + self.assertNotEqual(result.returncode, 0) + self.assertIn("launchd Label is", result.stderr) + + def test_missing_mach_service_blocks_release(self) -> None: + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + app, entitlements = self.make_app( + root, helper_plist_overrides={"MachServices": {"something.else": True}} + ) + + result = self.run_signer(app, entitlements) + + self.assertNotEqual(result.returncode, 0) + self.assertIn("does not vend the dev.caezium.Burrow.helper Mach service", result.stderr) + + def test_bundle_program_pointing_elsewhere_blocks_release(self) -> None: + """The sharpest one. + + BundleProgram is what launchd actually executes as root. If it names + anything other than the executable the pipeline just signed and + verified, the daemon running with full privileges is not the binary + this release vouched for. + """ + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + app, entitlements = self.make_app( + root, helper_plist_overrides={"BundleProgram": "Contents/MacOS/Burrow"} + ) + + result = self.run_signer(app, entitlements) + + self.assertNotEqual(result.returncode, 0) + self.assertIn("BundleProgram is", result.stderr) + + def test_bundle_program_that_does_not_exist_blocks_release(self) -> None: + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + app, entitlements = self.make_app(root, helper=False) + # Restore only the plist so the failure is specifically the + # dangling BundleProgram rather than the missing-binary check. + result = self.run_signer(app, entitlements) + + self.assertNotEqual(result.returncode, 0) + + def test_wellformed_helper_passes(self) -> None: + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + app, entitlements = self.make_app(root, get_task_allow=False) + + result = self.run_signer(app, entitlements) + + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertIn("privileged helper verified", result.stdout) + + if __name__ == "__main__": unittest.main() From 5f12fa590324cc8bc273209dab65acd654f4461c Mon Sep 17 00:00:00 2001 From: caezium <113233555+caezium@users.noreply.github.com> Date: Thu, 6 Aug 2026 23:07:11 -0700 Subject: [PATCH 02/14] refactor(settings): remove the Touch ID for sudo setting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The setting shelled out to `mo touchid enable/disable` to configure pam_tid for terminal `sudo`. It never affected Burrow's own admin prompts, which is the thing people expected it to do, and it did not reliably work. Removes the Settings section, the status probe, and the toggle. Nothing in the app now invokes `mo touchid`. This leaves MoleCLI.runElevated / runElevatedClassified and the one-shot PrivilegeBroker with no production caller — that path existed only for this setting. Both are annotated as unused rather than deleted here, since the streaming elevated path still shares AuthCancel and elevatedScript with them. --- macos/Sources/MoleCLI.swift | 12 ++-- macos/Sources/PrivilegeBroker.swift | 8 ++- macos/Sources/SettingsView.swift | 96 ++++++++------------------ macos/Tests/PrivilegeBrokerTests.swift | 18 ++--- 4 files changed, 52 insertions(+), 82 deletions(-) diff --git a/macos/Sources/MoleCLI.swift b/macos/Sources/MoleCLI.swift index de69ee3e..676592e3 100644 --- a/macos/Sources/MoleCLI.swift +++ b/macos/Sources/MoleCLI.swift @@ -289,10 +289,14 @@ enum MoleCLI { /// Run `mo ` ONCE with administrator rights via the macOS auth /// dialog. That dialog is PASSWORD-ONLY: the `system.privilege.admin` /// right authenticates through SecurityAgent's classic mechanism, which - /// never offers Touch ID — pam_tid (`mo touchid`) covers terminal - /// `sudo`, not this path. Blocking — call off the main thread. For - /// one-shot privileged config like `touchid enable/disable`, not for - /// streamed jobs (OperationFlow does those). + /// never offers Touch ID. Blocking — call off the main thread. For + /// one-shot privileged commands, not for streamed jobs (OperationFlow + /// does those). + /// + /// NOTE: currently has no production caller. Its only user was the + /// `mo touchid enable/disable` setting, which was removed. Kept for now + /// because it is the tested one-shot counterpart to the streaming path, + /// but it is a candidate for deletion. /// /// The spawn now goes through `PrivilegeBroker` so the osascript quoting /// and auth-cancel classification are testable in memory (issue #48); the diff --git a/macos/Sources/PrivilegeBroker.swift b/macos/Sources/PrivilegeBroker.swift index 98f24c04..0eb6a8ba 100644 --- a/macos/Sources/PrivilegeBroker.swift +++ b/macos/Sources/PrivilegeBroker.swift @@ -15,8 +15,12 @@ // and the auth-cancel classification IN MEMORY — no auth dialog, no sudo. // // Streamed elevated runs stay in OperationFlow's SystemProcessPort (output -// tailed from a temp log); this seam covers the one-shot config commands -// (`mo touchid enable/disable`) where the only signal is the exit status. +// tailed from a temp log); this seam covers one-shot commands where the only +// signal is the exit status. +// +// NOTE: the one-shot path has no production caller today — its only user was +// the `mo touchid enable/disable` setting, since removed. `AuthCancel` below +// is still shared with the streaming runner and must stay. // import Foundation diff --git a/macos/Sources/SettingsView.swift b/macos/Sources/SettingsView.swift index 5af17c0a..5438b317 100644 --- a/macos/Sources/SettingsView.swift +++ b/macos/Sources/SettingsView.swift @@ -16,7 +16,6 @@ import SwiftUI import AppKit -import LocalAuthentication import ServiceManagement import UniformTypeIdentifiers @@ -111,10 +110,6 @@ struct SettingsView: View { @State private var moleUpdating = false @State private var engineUpdatePolicy: MoleCLI.EngineUpdatePolicy = .unavailable @State private var copiedConfig = false - @State private var touchIDStatus = "—" - @State private var touchIDEnabled = false - @State private var touchIDBusy = false - @State private var touchIDAvailable = false @State private var helperStatus: HelperRegistrationStatus = .notRegistered @State private var helperBusy = false @State private var helperError: String? @@ -167,7 +162,7 @@ struct SettingsView: View { .frame(width: 460, height: 440) } .onAppear { - refreshStatusLabels(); loadMoleVersion(); loadTouchIDStatus(); loadLaunchAtLogin() + refreshStatusLabels(); loadMoleVersion(); loadLaunchAtLogin() loadHelperStatus() whitelistPatterns = MoleWhitelist.live.patterns() menuBarSuppressed = AppDelegate.shared?.menuBarSuppressedByCompatibilityGuard ?? false @@ -175,6 +170,10 @@ struct SettingsView: View { .onReceive(NotificationCenter.default.publisher(for: NSApplication.didBecomeActiveNotification)) { _ in fdaGranted = Privacy.hasFullDiskAccess() axTrusted = CleanScreen.inputLockPermitted() + // Approving the helper happens in System Settings, outside this + // app, so the row would otherwise still read "waiting for your + // approval" after the user came back having already granted it. + loadHelperStatus() } } @@ -649,18 +648,6 @@ struct SettingsView: View { footnote("Runs Burrow's admin operations — scan, clean, optimize — through a small signed helper instead of a password-only prompt, so macOS can offer Touch ID. Installing it needs your approval once. It grants no standing access: every operation that runs as administrator still asks you to authenticate, every time, and the helper can only perform those three operations — it cannot be asked to run anything else.") } - section("Touch ID for sudo", "touchid") { - infoRow("Status", touchIDStatus) - if touchIDAvailable { - HStack { - Spacer() - if touchIDBusy { ProgressView().controlSize(.small).padding(.trailing, 4) } - PillButton(title: touchIDEnabled ? "Disable" : "Enable", filled: false) { toggleTouchID() } - } - } - footnote("Lets `sudo` in a terminal accept your fingerprint instead of a password — including `mo` commands you run yourself. Separate from Burrow's own admin prompts, which are covered by the privileged helper above. Configured via `mo touchid` (pam_tid); turning it on or off needs your password once.") - } - section("Mole engine", "shippingbox") { infoRow("Version", moleVersion) if engineUpdatePolicy == .external { @@ -743,6 +730,28 @@ struct SettingsView: View { DispatchQueue.main.async { helperStatus = status } } + /// Watch for the approval landing. + /// + /// `register()` returns as soon as the request is filed, but the daemon + /// stays in `.requiresApproval` until the user allows it in System + /// Settings ▸ General ▸ Login Items & Extensions. That approval is + /// asynchronous and happens in another process, with no notification back + /// to us, so a one-shot read right after `register()` almost always + /// reports the pre-approval state and then never corrects itself. + /// + /// Polling for a bounded window is the whole fix. It stops as soon as the + /// status settles, and gives up rather than running forever if the user + /// walks away or dismisses the prompt. + private func pollHelperApproval(deadline: Date = Date().addingTimeInterval(90)) { + guard Date() < deadline else { return } + DispatchQueue.main.asyncAfter(deadline: .now() + 1) { + let status = PrivilegedHelperClient.shared.registrationStatus + helperStatus = status + guard status == .requiresApproval else { return } + pollHelperApproval(deadline: deadline) + } + } + /// Install or remove the daemon. Both directions can throw — macOS refuses /// registration if the user declines, and we surface that rather than /// leaving the row silently unchanged. @@ -769,55 +778,8 @@ struct SettingsView: View { helperBusy = false helperError = failure helperStatus = status - } - } - } - - // MARK: - Touch ID for sudo - - /// Whether this Mac actually has a Touch ID sensor. `mo touchid status` - /// only reports configured-vs-not (never hardware presence), so we ask - /// LocalAuthentication directly — biometryType is .touchID only on Macs - /// with the sensor (set after a canEvaluatePolicy probe, even if no - /// finger is enrolled yet). - private func touchIDHardwarePresent() -> Bool { - let ctx = LAContext() - var err: NSError? - _ = ctx.canEvaluatePolicy(.deviceOwnerAuthenticationWithBiometrics, error: &err) - return ctx.biometryType == .touchID - } - - private func loadTouchIDStatus() { - let available = touchIDHardwarePresent() - DispatchQueue.global(qos: .userInitiated).async { - let res = try? MoleCLI.run(args: ["touchid", "status"], timeout: 15) - // Strip ANSI colour codes Mole wraps the status line in before matching. - let out = Ansi.strip(res?.stdout ?? "").lowercased() - let enabled = out.contains("is enabled") - DispatchQueue.main.async { - touchIDAvailable = available - touchIDEnabled = enabled - touchIDStatus = !available ? "Not available on this Mac" - : (out.isEmpty ? "Unknown" : (enabled ? "Enabled" : "Disabled")) - } - } - } - - private func toggleTouchID() { - guard !touchIDBusy else { return } - touchIDBusy = true - let cmd = touchIDEnabled ? "disable" : "enable" - DispatchQueue.global(qos: .userInitiated).async { - let code = MoleCLI.runElevated(args: ["touchid", cmd]) - DispatchQueue.main.async { - touchIDBusy = false - loadTouchIDStatus() - if code != 0 { - let alert = NSAlert() - alert.messageText = NSLocalizedString("Couldn't update Touch ID for sudo", comment: "") - alert.informativeText = String(format: NSLocalizedString("`mo touchid %@` didn't complete (the password prompt may have been cancelled). You can also run it in a terminal.", comment: ""), cmd) - alert.runModalQuiet() - } + // Installing files the request; the user approves it elsewhere. + if install, status == .requiresApproval { pollHelperApproval() } } } } diff --git a/macos/Tests/PrivilegeBrokerTests.swift b/macos/Tests/PrivilegeBrokerTests.swift index 3fc3bc43..7371ee23 100644 --- a/macos/Tests/PrivilegeBrokerTests.swift +++ b/macos/Tests/PrivilegeBrokerTests.swift @@ -132,13 +132,13 @@ final class PrivilegeBrokerTests: XCTestCase { let fake = FakePrivilegeBroker(outcome: .exited(0)) MoleCLI.privilegeBroker = fake - let code = MoleCLI.runElevated(args: ["touchid", "enable"]) + let code = MoleCLI.runElevated(args: ["clean", "--dry-run"]) XCTAssertEqual(code, 0) XCTAssertEqual(fake.calls.count, 1) XCTAssertEqual(fake.calls.first?.executable, trusted, "elevated runs resolve through the trusted list, never PATH") - XCTAssertEqual(fake.calls.first?.args, ["touchid", "enable"]) + XCTAssertEqual(fake.calls.first?.args, ["clean", "--dry-run"]) } func testRunElevated_authCancelSurfacesAsNonzeroButClassified() throws { @@ -148,9 +148,9 @@ final class PrivilegeBrokerTests: XCTestCase { MoleCLI.privilegeBroker = FakePrivilegeBroker(outcome: .authCancelled) // Legacy Int32 caller: a dismissed prompt is still "didn't work". - XCTAssertNotEqual(MoleCLI.runElevated(args: ["touchid", "enable"]), 0) + XCTAssertNotEqual(MoleCLI.runElevated(args: ["clean", "--dry-run"]), 0) // New caller: the cancel is NAMED, distinct from a command failure. - XCTAssertEqual(MoleCLI.runElevatedClassified(args: ["touchid", "enable"]), .authCancelled) + XCTAssertEqual(MoleCLI.runElevatedClassified(args: ["clean", "--dry-run"]), .authCancelled) } func testRunElevated_commandFailureIsDistinctFromCancel() throws { @@ -159,8 +159,8 @@ final class PrivilegeBrokerTests: XCTestCase { } MoleCLI.privilegeBroker = FakePrivilegeBroker(outcome: .exited(2)) - XCTAssertEqual(MoleCLI.runElevatedClassified(args: ["touchid", "disable"]), .exited(2)) - XCTAssertEqual(MoleCLI.runElevated(args: ["touchid", "disable"]), 2) + XCTAssertEqual(MoleCLI.runElevatedClassified(args: ["optimize"]), .exited(2)) + XCTAssertEqual(MoleCLI.runElevated(args: ["optimize"]), 2) } /// No trusted `mo` → the broker is never asked to elevate; the result is @@ -174,11 +174,11 @@ final class PrivilegeBrokerTests: XCTestCase { MoleCLI.privilegeBroker = fake if MoleCLI.trustedExecutable() == nil { - XCTAssertEqual(MoleCLI.runElevatedClassified(args: ["touchid", "enable"]), .launchFailed) - XCTAssertEqual(MoleCLI.runElevated(args: ["touchid", "enable"]), 127) + XCTAssertEqual(MoleCLI.runElevatedClassified(args: ["clean", "--dry-run"]), .launchFailed) + XCTAssertEqual(MoleCLI.runElevated(args: ["clean", "--dry-run"]), 127) XCTAssertTrue(fake.calls.isEmpty, "a missing trusted mo must never reach the elevation spawn") } else { - _ = MoleCLI.runElevatedClassified(args: ["touchid", "enable"]) + _ = MoleCLI.runElevatedClassified(args: ["clean", "--dry-run"]) XCTAssertFalse(fake.calls.isEmpty, "with a trusted mo, the broker is reached") } } From 8af4ff4319fad920e66e37b207ae1dfd781621b9 Mon Sep 17 00:00:00 2001 From: caezium <113233555+caezium@users.noreply.github.com> Date: Thu, 6 Aug 2026 23:15:41 -0700 Subject: [PATCH 03/14] refactor: delete the orphaned one-shot elevated wrapper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `MoleCLI.runElevated` / `runElevatedClassified` and the `MoleCLI.privilegeBroker` seam had no caller left once the `mo touchid` setting was removed. An unused function that takes arbitrary argv and runs it as root is not worth keeping around waiting for one, so it's gone along with the tests that drove it through the injected fake. Deliberately NOT removed: `PrivilegeBroker` and `SystemPrivilegeBroker` are still live. `Connectivity.run` (flush DNS / renew DHCP) constructs the broker directly and calls `openElevated`, so the protocol, the production witness, `ElevatedOutcome`, and `AuthCancel` all stay. Because that call site builds its own broker rather than taking an injected one, there is no seam left for the scripted fake, which is why `FakePrivilegeBroker` went with the deleted tests. Kept in PrivilegeBrokerTests: the exhaustive `AuthCancel` table (shared with the streaming runner) and the `elevatedScript` quoting/injection cases — the two-pass quoter whose output still runs as root. 831 tests, 0 failures. --- macos/Sources/MoEngine.swift | 4 +- macos/Sources/MoleCLI.swift | 47 +++------- macos/Sources/PrivilegeBroker.swift | 7 +- macos/Sources/SettingsView.swift | 21 ++++- macos/Tests/MoEngineTests.swift | 4 +- macos/Tests/PrivilegeBrokerTests.swift | 113 ++++--------------------- 6 files changed, 53 insertions(+), 143 deletions(-) diff --git a/macos/Sources/MoEngine.swift b/macos/Sources/MoEngine.swift index 7f159971..530a969b 100644 --- a/macos/Sources/MoEngine.swift +++ b/macos/Sources/MoEngine.swift @@ -15,8 +15,8 @@ // holds the `any ProcessPort` to drive its own reduce/notify/auth-cancel loop, // so the facade hands it the production port rather than the stream. The // one-shot ELEVATED path is NOT on the facade — it stays in -// `MoleCLI.runElevatedClassified` (trusted-location resolution + the shared -// `PrivilegeBroker`), the path production has always used. +// `SystemPrivilegeBroker.openElevated` (see `Connectivity.run`), the path +// production has always used. // // Behavior is preserved exactly: a `capture(_:)` call produces the same argv, // stdin, environment, timeout, and result fields that `MoleCLI.run` did, and diff --git a/macos/Sources/MoleCLI.swift b/macos/Sources/MoleCLI.swift index 676592e3..04595caa 100644 --- a/macos/Sources/MoleCLI.swift +++ b/macos/Sources/MoleCLI.swift @@ -248,12 +248,6 @@ enum MoleCLI { /// a fake (reset in `tearDown`). Test-only seam — not a configuration point. internal static var processPort: MoleProcessPort = SystemMoleProcess() - /// The one-shot elevated runner (issue #48). Production spawns real - /// osascript via `SystemPrivilegeBroker`; tests inject a fake so the - /// build-the-osascript-spec quoting + auth-cancel classification run in - /// memory with no auth dialog. Test-only seam — reset in `tearDown`. - internal static var privilegeBroker: PrivilegeBroker = SystemPrivilegeBroker() - /// Run an executable with the given args, capturing stdout + stderr. /// Blocks until the process exits — callers are responsible for /// running this on a background queue. Times out after `timeout` @@ -286,34 +280,15 @@ enum MoleCLI { ) } - /// Run `mo ` ONCE with administrator rights via the macOS auth - /// dialog. That dialog is PASSWORD-ONLY: the `system.privilege.admin` - /// right authenticates through SecurityAgent's classic mechanism, which - /// never offers Touch ID. Blocking — call off the main thread. For - /// one-shot privileged commands, not for streamed jobs (OperationFlow - /// does those). - /// - /// NOTE: currently has no production caller. Its only user was the - /// `mo touchid enable/disable` setting, which was removed. Kept for now - /// because it is the tested one-shot counterpart to the streaming path, - /// but it is a candidate for deletion. - /// - /// The spawn now goes through `PrivilegeBroker` so the osascript quoting - /// and auth-cancel classification are testable in memory (issue #48); the - /// `Int32` return is preserved for existing callers that only branch on - /// "did it work" (a dismissed prompt collapses to a nonzero code, exactly - /// as before). New callers that want the named outcome use - /// `runElevatedClassified`. - static func runElevated(args: [String]) -> Int32 { - runElevatedClassified(args: args).exitCode - } - - /// As `runElevated`, but returns the classified outcome — `.authCancelled` - /// for a dismissed prompt is distinguished from a command that ran and - /// failed, so callers can show the right message without re-deriving the - /// "nonzero might mean cancel" heuristic themselves. - static func runElevatedClassified(args: [String]) -> ElevatedOutcome { - guard let mo = trustedExecutable() else { return .launchFailed } - return privilegeBroker.openElevated(executable: mo, args: args) - } + // NOTE: `runElevated` / `runElevatedClassified` used to live here — a + // one-shot "run `mo` as root with these args" entry point. Their only + // caller was the `mo touchid enable/disable` setting, and they were + // deleted with it: an unused function that takes arbitrary argv and runs + // it as root is exactly the kind of thing that shouldn't sit around + // waiting for a caller. + // + // The elevation machinery itself is still very much alive — the streaming + // path (`OperationFlow.SystemProcessPort`) uses `elevatedScript` above, + // and Connectivity's flush-DNS / renew-DHCP fixes call + // `SystemPrivilegeBroker.openElevated` directly. } diff --git a/macos/Sources/PrivilegeBroker.swift b/macos/Sources/PrivilegeBroker.swift index 0eb6a8ba..31159dae 100644 --- a/macos/Sources/PrivilegeBroker.swift +++ b/macos/Sources/PrivilegeBroker.swift @@ -18,9 +18,10 @@ // tailed from a temp log); this seam covers one-shot commands where the only // signal is the exit status. // -// NOTE: the one-shot path has no production caller today — its only user was -// the `mo touchid enable/disable` setting, since removed. `AuthCancel` below -// is still shared with the streaming runner and must stay. +// Live caller: `Connectivity.run` (flush DNS / renew DHCP), which constructs +// `SystemPrivilegeBroker` directly. The `MoleCLI.runElevated` wrapper that +// used to sit in front of this was deleted along with the `mo touchid` +// setting, its only user. // import Foundation diff --git a/macos/Sources/SettingsView.swift b/macos/Sources/SettingsView.swift index 5438b317..a213f068 100644 --- a/macos/Sources/SettingsView.swift +++ b/macos/Sources/SettingsView.swift @@ -769,9 +769,24 @@ struct SettingsView: View { try PrivilegedHelperClient.shared.unregister() } } catch { - failure = install - ? "Couldn't install the helper. Burrow keeps using the password prompt." - : "Couldn't remove the helper. You can also turn it off in System Settings ▸ General ▸ Login Items & Extensions." + // Show the real reason, not a shrug. The interesting failures + // here are all indistinguishable from each other in a generic + // message — a stale registration left by a previous build of + // the app, a signature the system won't accept, or the user + // declining — and the underlying code is what tells them + // apart. `kSMErrorAlreadyRegistered` in particular is + // actionable: the launchd job survives, bound to the old app + // identity, and has to be removed before a new install works. + let ns = error as NSError + let detail = "\(ns.domain) \(ns.code): \(ns.localizedDescription)" + if install { + let stale = ns.domain == "SMAppServiceErrorDomain" && ns.code == 134 + failure = stale + ? "A helper from an earlier build of Burrow is still registered. Remove Burrow under System Settings ▸ General ▸ Login Items & Extensions, then install again. (\(detail))" + : "Couldn't install the helper — Burrow keeps using the password prompt. (\(detail))" + } else { + failure = "Couldn't remove the helper. You can also turn it off in System Settings ▸ General ▸ Login Items & Extensions. (\(detail))" + } } let status = PrivilegedHelperClient.shared.registrationStatus DispatchQueue.main.async { diff --git a/macos/Tests/MoEngineTests.swift b/macos/Tests/MoEngineTests.swift index 0bf6d86d..2bd065b6 100644 --- a/macos/Tests/MoEngineTests.swift +++ b/macos/Tests/MoEngineTests.swift @@ -5,8 +5,8 @@ // Boundary tests for the unified runner facade (issue #48). The capture + // discovery entry points delegate to injected ports, so they're driven here // with scripted fakes — same seam style as MoleProcessTests (capture port). -// (The one-shot elevated path is NOT on the facade; its wiring is covered by -// PrivilegeBrokerTests against `MoleCLI.runElevatedClassified`.) +// (The one-shot elevated path is NOT on the facade; its pure pieces are +// covered by PrivilegeBrokerTests.) // // The point of these tests is the WIRING: that a `MoCommand` lands on the // capture runner as the exact argv/stdin/env/timeout it described, and that a diff --git a/macos/Tests/PrivilegeBrokerTests.swift b/macos/Tests/PrivilegeBrokerTests.swift index 7371ee23..9f826640 100644 --- a/macos/Tests/PrivilegeBrokerTests.swift +++ b/macos/Tests/PrivilegeBrokerTests.swift @@ -2,46 +2,33 @@ // PrivilegeBrokerTests.swift // BurrowTests // -// Boundary tests for the one-shot elevated path (issue #48) — the code that -// runs `mo` as ROOT, and that no test could previously reach because it +// Boundary tests for the elevated path (issue #48) — the code that runs +// commands as ROOT, and that no test could previously reach because it // spawned a real osascript auth dialog inline. // -// Two seams are exercised in memory, with NO osascript, NO sudo, NO GUI: -// * `AuthCancel` — the pure rule that decides a dismissed auth prompt vs a +// Two pure pieces are exercised in memory, with NO osascript, NO sudo, NO GUI: +// * `AuthCancel` — the rule that decides a dismissed auth prompt vs a // command that ran and failed. Shared by the streaming runner -// (SystemProcessPort.finalEvent) and this one-shot broker, so it's +// (SystemProcessPort.finalEvent) and the one-shot broker, so it's // table-tested exhaustively here. -// * `PrivilegeBroker` — the spawn port. A scripted fake stands in for -// osascript, so `MoleCLI.runElevated`/`runElevatedClassified` can be -// driven through every outcome (cancel, fail, success, launch-failure) -// and the osascript spec quoting verified — the previously-untestable -// ROOT path is now fully covered. +// * `MoleCLI.elevatedScript` — the two-pass quoter whose output runs as +// root inside `do shell script`. The injection cases at the bottom are +// the quoting that, if it broke, would delete the wrong files. +// +// The scripted-fake broker tests that used to live here went with +// `MoleCLI.runElevated`/`runElevatedClassified`, deleted once the +// `mo touchid` setting (their only caller) was removed. `PrivilegeBroker` +// itself is still live — Connectivity's flush-DNS / renew-DHCP fixes use +// `SystemPrivilegeBroker.openElevated` — but it is constructed directly at +// that call site, so there is no injection seam left to drive a fake through. // import XCTest @testable import Burrow -// MARK: - Scripted fake broker - -/// In-memory stand-in for osascript. Records the (executable, args) it was -/// asked to elevate so quoting/injection can be asserted, and replays a -/// canned outcome — no real process, no auth dialog. -final class FakePrivilegeBroker: PrivilegeBroker, @unchecked Sendable { - private(set) var calls: [(executable: String, args: [String])] = [] - var outcome: ElevatedOutcome - - init(outcome: ElevatedOutcome) { self.outcome = outcome } - - func openElevated(executable: String, args: [String]) -> ElevatedOutcome { - calls.append((executable, args)) - return outcome - } -} - final class PrivilegeBrokerTests: XCTestCase { override func tearDown() { - MoleCLI.privilegeBroker = SystemPrivilegeBroker() MoleCLI.discoveryCandidates = nil MoleCLI.resetDiscoveryCache() super.tearDown() @@ -104,7 +91,7 @@ final class PrivilegeBrokerTests: XCTestCase { // MARK: - ElevatedOutcome back-compat (the preserved Int32 contract) // - // `runElevated` still returns Int32 for existing callers; both failure + // The Int32 shim still backs `Connectivity.run`; both failure // shapes must collapse to a nonzero code, exactly as the old inline // spawn did (catch → 1, no trusted mo → 127). @@ -115,74 +102,6 @@ final class PrivilegeBrokerTests: XCTestCase { XCTAssertEqual(ElevatedOutcome.launchFailed.exitCode, 127, "matches the old 'no trusted mo' sentinel") } - // MARK: - Broker routing through MoleCLI (in-memory, no osascript) - // - // `runElevated` resolves the binary through `trustedExecutable()` ONLY — - // never `discoveryCandidates`/PATH, because a user-writable path entry - // would hand root to a shadowed binary. So these tests can't inject a - // fake `mo` path the way the capture-runner tests do; they assert the - // invariant against whatever the trusted lookup actually resolves, and - // skip the spawn-shape assertions when no trusted mo is installed (a - // valid CI state — nil is the launch-failure path, covered separately). - - func testRunElevated_routesTrustedExecutableAndArgsToBroker() throws { - guard let trusted = MoleCLI.trustedExecutable() else { - throw XCTSkip("no trusted mo installed; spawn-routing shape needs one") - } - let fake = FakePrivilegeBroker(outcome: .exited(0)) - MoleCLI.privilegeBroker = fake - - let code = MoleCLI.runElevated(args: ["clean", "--dry-run"]) - - XCTAssertEqual(code, 0) - XCTAssertEqual(fake.calls.count, 1) - XCTAssertEqual(fake.calls.first?.executable, trusted, - "elevated runs resolve through the trusted list, never PATH") - XCTAssertEqual(fake.calls.first?.args, ["clean", "--dry-run"]) - } - - func testRunElevated_authCancelSurfacesAsNonzeroButClassified() throws { - guard MoleCLI.trustedExecutable() != nil else { - throw XCTSkip("no trusted mo installed; the broker is never reached") - } - MoleCLI.privilegeBroker = FakePrivilegeBroker(outcome: .authCancelled) - - // Legacy Int32 caller: a dismissed prompt is still "didn't work". - XCTAssertNotEqual(MoleCLI.runElevated(args: ["clean", "--dry-run"]), 0) - // New caller: the cancel is NAMED, distinct from a command failure. - XCTAssertEqual(MoleCLI.runElevatedClassified(args: ["clean", "--dry-run"]), .authCancelled) - } - - func testRunElevated_commandFailureIsDistinctFromCancel() throws { - guard MoleCLI.trustedExecutable() != nil else { - throw XCTSkip("no trusted mo installed; the broker is never reached") - } - MoleCLI.privilegeBroker = FakePrivilegeBroker(outcome: .exited(2)) - - XCTAssertEqual(MoleCLI.runElevatedClassified(args: ["optimize"]), .exited(2)) - XCTAssertEqual(MoleCLI.runElevated(args: ["optimize"]), 2) - } - - /// No trusted `mo` → the broker is never asked to elevate; the result is - /// the launch-failure sentinel (127), matching the old `guard … else - /// return 127`. Only assertable when the trusted lookup genuinely misses; - /// where a real mo is installed the guard can't be forced (resolution is - /// deliberately not injectable), so we assert the inverse there: the - /// broker IS reached. - func testRunElevated_noTrustedExecutableTakesLaunchFailurePath() { - let fake = FakePrivilegeBroker(outcome: .exited(0)) - MoleCLI.privilegeBroker = fake - - if MoleCLI.trustedExecutable() == nil { - XCTAssertEqual(MoleCLI.runElevatedClassified(args: ["clean", "--dry-run"]), .launchFailed) - XCTAssertEqual(MoleCLI.runElevated(args: ["clean", "--dry-run"]), 127) - XCTAssertTrue(fake.calls.isEmpty, "a missing trusted mo must never reach the elevation spawn") - } else { - _ = MoleCLI.runElevatedClassified(args: ["clean", "--dry-run"]) - XCTAssertFalse(fake.calls.isEmpty, "with a trusted mo, the broker is reached") - } - } - // MARK: - osascript spec quoting through the broker (injection cases) // // The string the broker builds runs as ROOT inside `do shell script …`. From 73ebce65d985da5b01ac93063a637c2743db9e8a Mon Sep 17 00:00:00 2001 From: caezium <113233555+caezium@users.noreply.github.com> Date: Fri, 7 Aug 2026 00:08:39 -0700 Subject: [PATCH 04/14] fix(helper): make the daemon observable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The helper produced zero log lines across several runs while demonstrably alive and serving Mach requests — not even the unconditional startup notice. That made every failure indistinguishable from every other one, and the last diagnosis was inference from symptoms rather than evidence. Three changes, all diagnostic: - `helperTrace` mirrors every daemon message to stderr, which launchd redirects to /var/log/burrow-helper.log via StandardErrorPath. stderr needs no log-store query, predicate, or subsystem registration, so "wrote nothing" is now distinguishable from "never got that far". Startup is traced stage by stage for the same reason. - `HelperAuthorization.authorize` returns a Decision carrying the STAGE and the raw OSStatus instead of collapsing everything into an outcome. A malformed payload, a reference the Security framework won't rebuild, and a genuine refusal by the user were all reported as "denied"; only the last is normal. - The client logs its routing decision and the build the helper reported, so "the Clean never used the helper" and "the helper refused the Clean" stop looking identical from outside. Logged values stay decisions-only: stage names and operation names from closed enums, numeric status codes, build strings. No paths, output, or auth material. Also corrects the HelperService header, which still described a hand-rolled audit-token check; the connection gate is the system-enforced setConnectionCodeSigningRequirement. 831 tests, 0 failures. --- macos/HelperSources/HelperService.swift | 54 ++++++++++++------- macos/HelperSources/main.swift | 15 ++++-- .../dev.caezium.Burrow.helper.plist | 13 +++++ .../HelperAuthorization.swift | 50 +++++++++++++---- macos/Sources/PrivilegedHelperClient.swift | 25 ++++++++- 5 files changed, 122 insertions(+), 35 deletions(-) diff --git a/macos/HelperSources/HelperService.swift b/macos/HelperSources/HelperService.swift index 9666fc66..7c34bbf6 100644 --- a/macos/HelperSources/HelperService.swift +++ b/macos/HelperSources/HelperService.swift @@ -9,9 +9,10 @@ // ── The gauntlet a request runs ───────────────────────────────────────── // Five gates, in this order, each of which fails CLOSED: // -// 1. Connection the peer's AUDIT TOKEN must satisfy the code -// requirement (Burrow, signed by our team). PIDs are -// never used — they can be recycled and raced. +// 1. Connection the peer must satisfy the code requirement (Burrow, +// signed by our team), enforced by the SYSTEM via +// NSXPCListener.setConnectionCodeSigningRequirement +// before this process sees the connection at all. // 2. Shape the payload must decode as a `HelperRequest`. The // operation is an enum, so an unknown verb dies here. // 3. Freshness the operation ID must be a UUID this daemon has never @@ -42,6 +43,24 @@ import os let helperLog = Logger(subsystem: "dev.caezium.Burrow.helper", category: "privileged") +/// Diagnostic trail that cannot silently disappear. +/// +/// The unified log produced NOTHING for this daemon across several runs — not +/// even the unconditional startup line — while the process was demonstrably +/// alive and serving Mach requests. A root daemon whose logging you can't +/// trust is a root daemon you can't debug, so every `helperLog` call is +/// mirrored to stderr, which launchd redirects to a file via +/// `StandardErrorPath` in the plist. +/// +/// stderr is the belt to os_log's braces: it needs no log-store query, no +/// predicate, and no subsystem registration, so "the daemon wrote nothing" +/// becomes distinguishable from "the daemon never got that far". +func helperTrace(_ message: String) { + helperLog.notice("\(message, privacy: .public)") + let stamp = ISO8601DateFormatter().string(from: Date()) + FileHandle.standardError.write(Data("[\(stamp)] \(message)\n".utf8)) +} + // MARK: - Engine resolution enum HelperEngine { @@ -87,7 +106,7 @@ enum HelperEngine { /// one, and the release gate refuses to ship a helper without it. static func verify(path: String, teamID: String?) -> Bool { guard let teamID else { - helperLog.notice("engine signature check skipped: helper is ad-hoc signed (development build)") + helperTrace("engine signature check skipped: helper is ad-hoc signed (development build)") return true } guard let requirement = HelperCodeRequirement.sameTeam(teamID: teamID) else { return false } @@ -146,7 +165,7 @@ final class HelperOperationRunner: @unchecked Sendable { do { try process.run() } catch { - helperLog.error("engine spawn failed for operation \(operationID, privacy: .public)") + helperTrace("engine spawn failed for operation \(operationID)") return 127 } @@ -259,46 +278,43 @@ final class HelperService: NSObject, BurrowHelperProtocol { // Gate 2 — shape. An unknown operation cannot survive decoding. guard let request = try? JSONDecoder().decode(HelperRequest.self, from: requestData) else { - helperLog.error("request refused: malformed payload") + helperTrace("request refused: malformed payload") return respond(.rejected(.malformedPayload)) } if let rejection = request.validate(expectedBuild: Self.build) { - helperLog.error("request refused: \(rejection.rawValue, privacy: .public)") + helperTrace("request refused: \(rejection.rawValue)") return respond(.rejected(rejection)) } // Gate 3 — freshness. One authorization buys exactly one operation, so // a captured payload cannot be replayed for a second root run. guard replayGuard.admit(request.operationID) else { - helperLog.error("request refused: replayed operation ID") + helperTrace("request refused: replayed operation ID") return respond(.rejected(.replayedOperationID)) } // Gate 4 — authorization. This is what raises the prompt, and it // happens HERE, in the privileged process, on every single operation. + helperTrace("authorizing \(request.operation.rawValue): calling AuthorizationCopyRights") let decision = HelperAuthorization.authorize(externalForm: authorization) guard decision.permitsExecution else { - helperLog.notice(""" - operation \(request.operation.rawValue, privacy: .public) not authorized - """) - switch decision { + helperTrace("NOT authorized: \(decision.diagnostic)") + switch decision.outcome { case .cancelled: return respond(.authorizationCancelled) default: return respond(.authorizationDenied) } } + helperTrace("authorized: \(decision.diagnostic)") // Gate 5 — execution. Our own signed engine, fixed argv. guard let enginePath = HelperEngine.bundledEnginePath(), HelperEngine.verify(path: enginePath, teamID: teamID) else { - helperLog.error("engine unavailable or failed signature verification") + helperTrace("engine unavailable or failed signature verification") return respond(.engineUnavailable) } - helperLog.notice(""" - authorized \(request.operation.rawValue, privacy: .public) \ - (mutating: \(request.operation.mutatesDisk, privacy: .public)) - """) + helperTrace("running \(request.operation.rawValue) (mutating: \(request.operation.mutatesDisk))") let client = currentConnection?.remoteObjectProxy as? BurrowHelperClientProtocol let operationID = request.operationID @@ -307,7 +323,7 @@ final class HelperService: NSObject, BurrowHelperProtocol { enginePath: enginePath) { line in client?.helperDidEmit(line: line, operationID: operationID) } - helperLog.notice("operation finished with status \(code, privacy: .public)") + helperTrace("operation finished with status \(code)") respond(.exited(code)) } } @@ -333,7 +349,7 @@ final class HelperListenerDelegate: NSObject, NSXPCListenerDelegate { connection.remoteObjectInterface = HelperInterface.client() service.currentConnection = connection connection.resume() - helperLog.notice("connection accepted from a verified Burrow client") + helperTrace("connection accepted from a verified Burrow client") return true } } diff --git a/macos/HelperSources/main.swift b/macos/HelperSources/main.swift index 51414565..3da05ffd 100644 --- a/macos/HelperSources/main.swift +++ b/macos/HelperSources/main.swift @@ -24,18 +24,24 @@ enum HelperMain { static let idleTimeout: TimeInterval = 120 static func start() -> Never { + // Traced stage by stage. When this daemon last misbehaved there was no + // way to tell "it never started" from "it started and the + // authorization failed", which cost more time than the bug did. + helperTrace("startup: entered main") + // The team that signed US. Learned at runtime, so no team ID, // certificate, or developer name is ever hardcoded in the repository, // and the check survives certificate renewal. let teamID = HelperCodeRequirement.selfTeamIdentifier() let requirement = HelperCodeRequirement.string(bundleID: HelperNames.clientBundleID, teamID: teamID) + helperTrace("startup: signing team \(teamID == nil ? "absent (ad-hoc)" : "present")") if teamID == nil { // Ad-hoc: local development only. Say so loudly — a release build // always has a team, and the release gate refuses to ship without // one, so seeing this in the wild means something is wrong. - helperLog.notice("running ad-hoc signed: caller pinning is identifier-only (development build)") + helperTrace("running ad-hoc signed: caller pinning is identifier-only (development build)") } // Publish the right's definition. Rewritten every launch on purpose: @@ -43,8 +49,9 @@ enum HelperMain { // tampered policy entry from an earlier install cannot weaken a newer // helper. Failure is not fatal — the right still evaluates, just with // the system's default wording. + helperTrace("startup: publishing authorization right") if !HelperAuthorization.installRightDefinition() { - helperLog.error("could not publish the authorization right definition") + helperTrace("startup: could NOT publish the authorization right definition") } let service = HelperService(teamID: teamID) @@ -60,7 +67,7 @@ enum HelperMain { listener.delegate = delegate listener.resume() - helperLog.notice("helper \(HelperService.build, privacy: .public) listening") + helperTrace("helper \(HelperService.build) listening on \(HelperNames.machService)") // Idle exit. Checked on a timer rather than tied to connection // teardown so a client that opens a connection and then goes silent @@ -71,7 +78,7 @@ enum HelperMain { timer.setEventHandler { if service.isIdle { if Date().timeIntervalSince(idleSince) >= idleTimeout { - helperLog.notice("idle timeout reached; exiting") + helperTrace("idle timeout reached; exiting") exit(0) } } else { diff --git a/macos/Resources/LaunchDaemons/dev.caezium.Burrow.helper.plist b/macos/Resources/LaunchDaemons/dev.caezium.Burrow.helper.plist index db2ba3af..48d7c165 100644 --- a/macos/Resources/LaunchDaemons/dev.caezium.Burrow.helper.plist +++ b/macos/Resources/LaunchDaemons/dev.caezium.Burrow.helper.plist @@ -39,5 +39,18 @@ ProcessType Interactive + + + StandardErrorPath + /var/log/burrow-helper.log diff --git a/macos/Sources/PrivilegedHelper/HelperAuthorization.swift b/macos/Sources/PrivilegedHelper/HelperAuthorization.swift index 76203ab0..1d9acf4c 100644 --- a/macos/Sources/PrivilegedHelper/HelperAuthorization.swift +++ b/macos/Sources/PrivilegedHelper/HelperAuthorization.swift @@ -165,14 +165,40 @@ enum HelperAuthorization { return withUnsafeBytes(of: &form) { Data($0) } } - /// Daemon side. Rebuild the client's reference and REQUIRE `rightName`, - /// raising the system prompt. The returned outcome is the gate: only - /// `.granted` may be followed by privileged work. + /// Which step produced the result. Kept because "authorization failed" was + /// indistinguishable across three very different causes — a malformed + /// payload, a reference the Security framework wouldn't rebuild, and an + /// actual refusal by the user — and only the last of those is normal. + enum Stage: String, Sendable { + case malformedExternalForm + case createFromExternalForm + case copyRights + } + + struct Decision: Sendable { + let outcome: Outcome + let stage: Stage + /// The raw `OSStatus`, preserved even when the outcome collapses + /// several codes into `.denied`. + let status: OSStatus + + var permitsExecution: Bool { outcome.permitsExecution } + + /// Safe to log: a stage name from a closed enum plus a numeric status. + /// No paths, no credentials, no free-form error text. + var diagnostic: String { "\(stage.rawValue) status=\(status) outcome=\(outcome)" } + } + + /// Daemon side. Rebuild the client's reference and REQUIRE `rightName`. + /// The returned decision is the gate: only `.granted` may be followed by + /// privileged work. /// /// The rights array is always non-empty — passing NULL here is the /// documented way to accidentally authorize everybody. - static func authorize(externalForm data: Data) -> Outcome { - guard isPlausibleExternalForm(data) else { return .denied } + static func authorize(externalForm data: Data) -> Decision { + guard isPlausibleExternalForm(data) else { + return Decision(outcome: .denied, stage: .malformedExternalForm, status: errAuthorizationInvalidRef) + } var form = AuthorizationExternalForm() let copied: Bool = withUnsafeMutableBytes(of: &form) { raw -> Bool in @@ -180,19 +206,23 @@ enum HelperAuthorization { _ = data.copyBytes(to: raw.bindMemory(to: UInt8.self)) return true } - guard copied else { return .denied } + guard copied else { + return Decision(outcome: .denied, stage: .malformedExternalForm, status: errAuthorizationInvalidRef) + } var ref: AuthorizationRef? let restored = AuthorizationCreateFromExternalForm(&form, &ref) - guard restored == errAuthorizationSuccess, let ref else { return .denied } + guard restored == errAuthorizationSuccess, let ref else { + return Decision(outcome: .denied, stage: .createFromExternalForm, status: restored) + } defer { AuthorizationFree(ref, []) } - return rightName.withCString { name -> Outcome in + return rightName.withCString { name -> Decision in var item = AuthorizationItem(name: name, valueLength: 0, value: nil, flags: 0) - return withUnsafeMutablePointer(to: &item) { itemPointer -> Outcome in + return withUnsafeMutablePointer(to: &item) { itemPointer -> Decision in var rights = AuthorizationRights(count: 1, items: itemPointer) let status = AuthorizationCopyRights(ref, &rights, nil, daemonFlags, nil) - return outcome(from: status) + return Decision(outcome: outcome(from: status), stage: .copyRights, status: status) } } } diff --git a/macos/Sources/PrivilegedHelperClient.swift b/macos/Sources/PrivilegedHelperClient.swift index d3d7f135..c5c49052 100644 --- a/macos/Sources/PrivilegedHelperClient.swift +++ b/macos/Sources/PrivilegedHelperClient.swift @@ -31,6 +31,15 @@ import Foundation import Security import ServiceManagement +import os + +/// Client-side trail, matching the daemon's. +/// +/// Without this, "the Clean didn't use the helper" and "the helper refused the +/// Clean" look identical from the outside — the daemon simply logs nothing in +/// the first case, which is exactly the ambiguity that made the last round of +/// debugging guesswork. +let helperClientLog = Logger(subsystem: "dev.caezium.Burrow", category: "privileged-helper") // MARK: - Bridging the two elevation routes onto one taxonomy @@ -187,7 +196,12 @@ final class PrivilegedHelperClient: @unchecked Sendable { if let cachedSkew { lock.unlock(); return cachedSkew } lock.unlock() - let skew = HelperVersionSkew.evaluate(appBuild: Self.appBuild, helperBuild: helperBuild()) + let reported = helperBuild() + helperClientLog.notice(""" + helper reported build \(reported.isEmpty ? "" : reported, privacy: .public), \ + app is \(Self.appBuild, privacy: .public) + """) + let skew = HelperVersionSkew.evaluate(appBuild: Self.appBuild, helperBuild: reported) lock.lock(); cachedSkew = skew; lock.unlock() return skew } @@ -199,9 +213,16 @@ final class PrivilegedHelperClient: @unchecked Sendable { // Don't pay for an XPC round trip to learn the version when the daemon // isn't usable anyway. guard status == .enabled else { + helperClientLog.notice("route: osascript (registration \(String(describing: status), privacy: .public))") return PrivilegeRoute.decide(arguments: arguments, registration: status, skew: .mismatched) } - return PrivilegeRoute.decide(arguments: arguments, registration: status, skew: versionSkew()) + let skew = versionSkew() + let route = PrivilegeRoute.decide(arguments: arguments, registration: status, skew: skew) + helperClientLog.notice(""" + route: \(String(describing: route), privacy: .public) \ + (app build \(Self.appBuild, privacy: .public), skew \(String(describing: skew), privacy: .public)) + """) + return route } // MARK: Execution From 15a4336f89439b76e8c96841d0af3a901f2f0e30 Mon Sep 17 00:00:00 2001 From: caezium <113233555+caezium@users.noreply.github.com> Date: Fri, 7 Aug 2026 00:40:24 -0700 Subject: [PATCH 05/14] fix(release): stop the helper hardened-runtime check failing on valid builds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The check piped `codesign -d` straight into `grep -q`. With `set -o pipefail` in force, a non-zero exit from codesign fails the whole pipeline even when the grep matched, so a helper that genuinely carries the hardened runtime was reported as missing it — and the gate is fail-closed, so that aborts the release. Caught signing a real Developer ID build: the helper had flags=0x10000(runtime) and the gate still rejected it. Reads the signature into a variable first, so the check is about the flag and not about codesign's exit status. --- scripts/sign-macos-app.sh | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/scripts/sign-macos-app.sh b/scripts/sign-macos-app.sh index b5974a74..ea4be89f 100755 --- a/scripts/sign-macos-app.sh +++ b/scripts/sign-macos-app.sh @@ -254,7 +254,13 @@ if [ "$MODE" = "developer-id" ]; then || { echo "error: helper team is '${HELPER_TEAM:-missing}', expected '$EXPECTED_TEAM'" >&2; exit 1; } # Hardened runtime on a root daemon is not optional. - codesign -d --verbose=2 "$HELPER" 2>&1 | grep -q "flags=.*runtime" \ + # + # Read once into a variable rather than piping codesign straight into grep: + # `set -o pipefail` is in force, so a non-zero exit from codesign fails the + # pipeline even when the grep matched, which reads as "no hardened runtime" + # on a binary that has it. Separating the two keeps the check about the flag. + HELPER_SIG="$(codesign -d --verbose=2 "$HELPER" 2>&1 || true)" + grep -q "flags=.*runtime" <<< "$HELPER_SIG" \ || { echo "error: privileged helper is not built with the hardened runtime" >&2; exit 1; } fi From 76aad67cb45a57d7c08478078fb88595b588b4cd Mon Sep 17 00:00:00 2001 From: caezium <113233555+caezium@users.noreply.github.com> Date: Fri, 7 Aug 2026 00:43:33 -0700 Subject: [PATCH 06/14] fix(helper): stop the launchd plist preventing the daemon from running MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adding StandardErrorPath to the launchd plist made launchd refuse to exec the daemon: every spawn failed with EX_CONFIG (78) and the process never started. The change was made to gain observability and instead destroyed the thing being observed — and its symptom (a daemon that never runs, plus a 0-byte log file) is indistinguishable from a code-signing rejection or a bad bundle location, which is what it was mistaken for. Timeline is unambiguous: immediately before that key was added the daemon was running (runs=1, live pid, connection accepted); it has not executed once since. The daemon now opens /Library/Logs/burrow-helper.log itself. Same trail, but a path it cannot write costs diagnostics rather than the daemon. Still NOT verified: whether the authorization actually succeeds. The daemon has never reached that code, so the open question — whether a root daemon can raise an interactive AuthorizationCopyRights prompt — remains untested. --- macos/HelperSources/HelperService.swift | 37 +++++++++++++++---- .../dev.caezium.Burrow.helper.plist | 21 ++++++----- 2 files changed, 41 insertions(+), 17 deletions(-) diff --git a/macos/HelperSources/HelperService.swift b/macos/HelperSources/HelperService.swift index 7c34bbf6..bfdc9cb8 100644 --- a/macos/HelperSources/HelperService.swift +++ b/macos/HelperSources/HelperService.swift @@ -48,17 +48,40 @@ let helperLog = Logger(subsystem: "dev.caezium.Burrow.helper", category: "privil /// The unified log produced NOTHING for this daemon across several runs — not /// even the unconditional startup line — while the process was demonstrably /// alive and serving Mach requests. A root daemon whose logging you can't -/// trust is a root daemon you can't debug, so every `helperLog` call is -/// mirrored to stderr, which launchd redirects to a file via -/// `StandardErrorPath` in the plist. +/// trust is a root daemon you can't debug, so every message is also written to +/// a file. /// -/// stderr is the belt to os_log's braces: it needs no log-store query, no -/// predicate, and no subsystem registration, so "the daemon wrote nothing" -/// becomes distinguishable from "the daemon never got that far". +/// That file is opened HERE rather than via `StandardErrorPath` in the launchd +/// plist. The plist route was tried first and was actively harmful: launchd +/// refused to exec the daemon at all, failing every spawn with EX_CONFIG, so +/// the attempt to gain observability destroyed the thing being observed — and +/// the symptom (a daemon that never runs and a 0-byte log) is indistinguishable +/// from a code-signing rejection. +/// +/// Opening it in-process inverts that failure mode: a path the daemon cannot +/// write costs diagnostics, never the daemon. +private let helperTraceHandle: FileHandle? = { + let path = "/Library/Logs/burrow-helper.log" + let fm = FileManager.default + if !fm.fileExists(atPath: path) { + // 0644 root-owned: readable for support, writable only by root, and in + // a directory unprivileged users cannot pre-seed with a symlink. + fm.createFile(atPath: path, contents: nil, + attributes: [.posixPermissions: 0o644]) + } + guard let handle = FileHandle(forWritingAtPath: path) else { return nil } + handle.seekToEndOfFile() + return handle +}() + +private let helperTraceLock = NSLock() + func helperTrace(_ message: String) { helperLog.notice("\(message, privacy: .public)") + guard let helperTraceHandle else { return } let stamp = ISO8601DateFormatter().string(from: Date()) - FileHandle.standardError.write(Data("[\(stamp)] \(message)\n".utf8)) + helperTraceLock.lock(); defer { helperTraceLock.unlock() } + try? helperTraceHandle.write(contentsOf: Data("[\(stamp)] \(message)\n".utf8)) } // MARK: - Engine resolution diff --git a/macos/Resources/LaunchDaemons/dev.caezium.Burrow.helper.plist b/macos/Resources/LaunchDaemons/dev.caezium.Burrow.helper.plist index 48d7c165..fcddb399 100644 --- a/macos/Resources/LaunchDaemons/dev.caezium.Burrow.helper.plist +++ b/macos/Resources/LaunchDaemons/dev.caezium.Burrow.helper.plist @@ -41,16 +41,17 @@ Interactive - StandardErrorPath - /var/log/burrow-helper.log + From 65c7e8addfefe6c3c5b7c9dddd5462fcbdf44be6 Mon Sep 17 00:00:00 2001 From: caezium <113233555+caezium@users.noreply.github.com> Date: Fri, 7 Aug 2026 01:13:09 -0700 Subject: [PATCH 07/14] fix(helper): move the authentication prompt to the app, verify it in the daemon MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Measured, not inferred: with unified logging finally readable, the daemon was reaching the authorization code and refusing every operation — connection accepted from a verified Burrow client operation clean not authorized A launchd system daemon has no session to draw an authentication UI in, so asking it to raise the prompt fails regardless of how the right is defined. The original design was wrong about where a prompt can be raised, though right about where the check belongs. Switches to Apple's documented split: app — AuthorizationCopyRights with interactionAllowed + preAuthorize. This raises the prompt, in the user's own session, where SecurityAgent can offer Touch ID and fall back to the password. root — AuthorizationCopyRights WITHOUT interaction. Not a prompt: a question about whether the reference genuinely holds the right. The daemon still never trusts the client's word. The credential lives in the security session rather than in the message, so a caller that skipped the prompt produces a reference that fails the daemon's check — forging it means forging a Security-framework credential, not editing an XPC payload. Dropping interactionAllowed from the daemon also means a daemon that CANNOT be made to prompt by anything that reaches it. The cost, which is a real relaxation of the original decision: the credential has to survive the hop to the daemon, so `timeout` moves from 0 to 10s. Inside that window a second operation would not re-prompt. The replay guard still serves each operation ID once and `shared: false` still keeps the credential out of other processes, so what is open is narrow — but it is open, and SECURITY.md and the Settings copy now say so instead of promising "every time". Also drops ProcessType from the launchd plist. It was set to Interactive for no reason beyond "the daemon shows a prompt" — which is no longer true, and it was the only non-required key present while spawns were failing with EX_CONFIG. 832 tests, 0 failures. --- SECURITY.md | 17 +- .../dev.caezium.Burrow.helper.plist | 9 +- .../HelperAuthorization.swift | 165 ++++++++++++------ macos/Sources/PrivilegedHelperClient.swift | 18 +- macos/Sources/SettingsView.swift | 2 +- macos/Tests/HelperAuthorizationTests.swift | 59 +++++-- 6 files changed, 191 insertions(+), 79 deletions(-) diff --git a/SECURITY.md b/SECURITY.md index a1f52002..aeff20d3 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -61,11 +61,18 @@ This is the part people rightly scrutinize in cleaners. Burrow's model: strictly opt-in and takes its own one-time macOS approval. If you install it: - **It grants no standing privilege.** Installing the helper authorizes - nothing. Every operation that runs as root requires a fresh - authentication — no grace period, no cached credential, no - "authenticate once for this launch". This is enforced by the - authorization right's own definition (`timeout: 0`, `shared: false`, - `allow-root: false`), not merely by convention. + nothing, and there is no "authenticate once for this launch". You are + asked to authenticate for each privileged operation you start. + - **The one caveat, stated honestly:** the credential from that + authentication stays valid for a 10-second window, because it has to + survive the hop from the app to the helper. A second operation begun + inside that window would not prompt again. The window covers an + inter-process message, not a user changing their mind, and it is the + shortest value that lets the check work at all. The rest is enforced by + the right's own definition (`shared: false` keeps the credential out of + other processes, `allow-root: false` stops the root helper satisfying it + by itself), and each operation ID is served at most once so a captured + request cannot be replayed. - **It cannot be asked to run anything else.** The helper accepts three typed operations — scan, clean, optimize — and derives the command line itself. There is no field in its API for a path, an argument, a shell diff --git a/macos/Resources/LaunchDaemons/dev.caezium.Burrow.helper.plist b/macos/Resources/LaunchDaemons/dev.caezium.Burrow.helper.plist index fcddb399..4947b313 100644 --- a/macos/Resources/LaunchDaemons/dev.caezium.Burrow.helper.plist +++ b/macos/Resources/LaunchDaemons/dev.caezium.Burrow.helper.plist @@ -37,8 +37,13 @@ dev.caezium.Burrow - ProcessType - Interactive +