Skip to content

Commit 946be5c

Browse files
committed
Keep Malibu repair recovery fail closed under filesystem races
Align app-side repair admission with installer evidence and make rollback preserve mixed-content installs without blocking on special files. Constraint: Legacy standalone installs may contain owner-private evidence, FIFOs or special files, and unrelated support content; repairs must fail closed without discarding user data. Rejected: Path-based recursive copies and pre-verifier reads | they can block, race, or admit semantically invalid repairs. Confidence: high Scope-risk: broad Directive: Keep app-side repair predicates and installer recovery snapshots descriptor-validated and semantically aligned. Tested: xcodebuild test (331 tests); install upgrade evidence rollback matrix; provider upgrade transaction; transaction lock; launchd migration; watchdog rollback and health; referral handoff; watchdog inline drift; bash -n; git diff --check. Not-tested: Fresh GitHub CI on this final head.
1 parent 33823d5 commit 946be5c

14 files changed

Lines changed: 2257 additions & 176 deletions

ops/macprovider-watchdog/watchdog.sh

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -409,7 +409,7 @@ def verify_root():
409409
raise RuntimeError(f"not_directory:{path}")
410410
411411
def read_marker():
412-
fd = os.open(pending, os.O_RDONLY | getattr(os, "O_NOFOLLOW", 0))
412+
fd = os.open(pending, os.O_RDONLY | getattr(os, "O_NONBLOCK", 0) | getattr(os, "O_NOFOLLOW", 0))
413413
try:
414414
raw = os.read(fd, 65536)
415415
finally:
@@ -508,7 +508,7 @@ def current_binary_version(path):
508508
509509
def read_success_sentinel(path):
510510
reject_path(path)
511-
fd = os.open(path, os.O_RDONLY | getattr(os, "O_NOFOLLOW", 0))
511+
fd = os.open(path, os.O_RDONLY | getattr(os, "O_NONBLOCK", 0) | getattr(os, "O_NOFOLLOW", 0))
512512
try:
513513
payload = json.loads(os.read(fd, 65536).decode("utf-8"))
514514
finally:
@@ -569,7 +569,7 @@ def process_success_sentinel(marker):
569569
570570
def sha256(path):
571571
h = hashlib.sha256()
572-
fd = os.open(path, os.O_RDONLY | getattr(os, "O_NOFOLLOW", 0))
572+
fd = os.open(path, os.O_RDONLY | getattr(os, "O_NONBLOCK", 0) | getattr(os, "O_NOFOLLOW", 0))
573573
try:
574574
while True:
575575
chunk = os.read(fd, 1024 * 1024)
@@ -1038,7 +1038,7 @@ def validate_malibu_app_backup(release_backup):
10381038
state_st = reject_path(state_path)
10391039
if not stat.S_ISREG(archive_st.st_mode) or not stat.S_ISREG(state_st.st_mode):
10401040
raise RuntimeError("malibu_backup_not_regular")
1041-
fd = os.open(state_path, os.O_RDONLY | getattr(os, "O_NOFOLLOW", 0))
1041+
fd = os.open(state_path, os.O_RDONLY | getattr(os, "O_NONBLOCK", 0) | getattr(os, "O_NOFOLLOW", 0))
10421042
try:
10431043
raw = os.read(fd, 65537)
10441044
finally:
@@ -1150,7 +1150,7 @@ def fsync_release_tree(root_path):
11501150
directories.append(current)
11511151
for name in file_names:
11521152
path = os.path.join(current, name)
1153-
fd = os.open(path, os.O_RDONLY | getattr(os, "O_NOFOLLOW", 0))
1153+
fd = os.open(path, os.O_RDONLY | getattr(os, "O_NONBLOCK", 0) | getattr(os, "O_NOFOLLOW", 0))
11541154
try:
11551155
os.fsync(fd)
11561156
finally:
@@ -1165,7 +1165,7 @@ def fsync_release_tree(root_path):
11651165
def atomic_copy_binary(source, target, mode):
11661166
temporary = os.path.join(os.path.dirname(target), f".macprovider-cli.rollback-restore-{uuid.uuid4()}")
11671167
try:
1168-
source_fd = os.open(source, os.O_RDONLY | getattr(os, "O_NOFOLLOW", 0))
1168+
source_fd = os.open(source, os.O_RDONLY | getattr(os, "O_NONBLOCK", 0) | getattr(os, "O_NOFOLLOW", 0))
11691169
try:
11701170
destination_fd = os.open(temporary, os.O_CREAT | os.O_EXCL | os.O_WRONLY | getattr(os, "O_NOFOLLOW", 0), mode)
11711171
try:

phase3-binary/app/Sources/Malibu/Onboarding/LaunchProviderController.swift

Lines changed: 92 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -536,6 +536,11 @@ struct StartupState: Equatable {
536536
homeDirectory: homeDirectory,
537537
fileManager: fm
538538
)
539+
let launchdRepairEvidenceExists = Self.launchdRepairEvidenceExists(
540+
paths: paths,
541+
homeDirectory: homeDirectory,
542+
fileManager: fm
543+
)
539544

540545
var backgroundProviderHealthy = false
541546
if ProviderConfig.readProviderID(paths: paths) != nil,
@@ -548,7 +553,7 @@ struct StartupState: Equatable {
548553
homeDirectory: homeDirectory,
549554
fileManager: fm
550555
)
551-
let launchdJobNeedsRepair = launchdInstallEvidenceExists && providerRepairState.needsRepair
556+
let launchdJobNeedsRepair = launchdRepairEvidenceExists && providerRepairState.needsRepair
552557
// A loaded provider label with an unexpected executable/plist is a
553558
// conflict even when the on-disk evidence is missing or unsafe. The
554559
// installer intentionally refuses label-only reclamation without a
@@ -568,7 +573,7 @@ struct StartupState: Equatable {
568573
// Do not gate watchdog inspection on a readable plist. A loaded job
569574
// whose plist disappeared or became unsafe is itself a manual
570575
// conflict; suppressing that state creates an endless repair loop.
571-
let watchdogJobNeedsRepair = watchdogRepairState.needsRepair
576+
let watchdogJobNeedsRepair = launchdRepairEvidenceExists && watchdogRepairState.needsRepair
572577
let watchdogNeedsManualIntervention = watchdogRepairState.requiresManualIntervention
573578

574579
return StartupState(
@@ -637,6 +642,91 @@ struct StartupState: Equatable {
637642
return trusted(manifest) || trusted(launchd)
638643
}
639644

645+
/// Repair is admitted only when Malibu can present the same durable
646+
/// incumbent identity that install.sh requires before bypassing referral
647+
/// admission. A stale plist or manifest by itself is onboarding evidence,
648+
/// not proof that the existing provider can be safely repaired in place.
649+
static func launchdRepairEvidenceExists(
650+
paths: ProviderPaths = .current,
651+
homeDirectory: URL = FileManager.default.homeDirectoryForCurrentUser,
652+
fileManager: FileManager = .default
653+
) -> Bool {
654+
let fm = fileManager
655+
let configDirectory = paths.configFile.deletingLastPathComponent()
656+
let providerIDURL = configDirectory.appendingPathComponent("provider_id")
657+
let manifest = homeDirectory.appendingPathComponent(
658+
"Library/Application Support/macprovider/install_manifest.json"
659+
)
660+
let launchd = homeDirectory.appendingPathComponent(
661+
"Library/LaunchAgents/live.streamvc.macprovider.plist"
662+
)
663+
func trusted(_ url: URL, maxBytes: Int = 1024 * 1024) -> Bool {
664+
InstalledProviderMonitor.isSafePrivateDirectoryChain(
665+
url.deletingLastPathComponent(),
666+
under: homeDirectory
667+
) && InstalledProviderMonitor.hasTrustedPrivateFile(
668+
at: url,
669+
maxBytes: maxBytes,
670+
fileManager: fm
671+
)
672+
}
673+
guard let configuredProviderID = ProviderConfig.readProviderID(paths: paths),
674+
trusted(paths.configFile),
675+
let savedProviderID = InstalledProviderMonitor.readOwnerPrivateRegularFile(
676+
providerIDURL,
677+
maxBytes: 64 * 1024,
678+
fileManager: fm
679+
),
680+
String(decoding: savedProviderID, as: UTF8.self)
681+
.trimmingCharacters(in: .whitespacesAndNewlines) == configuredProviderID,
682+
trusted(providerIDURL, maxBytes: 64 * 1024),
683+
trusted(manifest),
684+
trusted(launchd),
685+
let manifestData = InstalledProviderMonitor.readOwnerPrivateRegularFile(
686+
manifest,
687+
maxBytes: 64 * 1024,
688+
fileManager: fm
689+
),
690+
let manifestObject = try? JSONSerialization.jsonObject(with: manifestData),
691+
let manifestValues = manifestObject as? [String: Any],
692+
let installPrefix = manifestValues["install_prefix"] as? String,
693+
let binaryPath = manifestValues["binary_path"] as? String,
694+
let launchdLabels = manifestValues["launchd_labels"] as? [String],
695+
let launchdPlists = manifestValues["launchd_plists"] as? [String] else {
696+
return false
697+
}
698+
let installDirectory = URL(fileURLWithPath: installPrefix).standardizedFileURL
699+
let expectedBinaryPath = installDirectory.appendingPathComponent("macprovider-cli").path
700+
guard installPrefix == installDirectory.path,
701+
InstalledProviderMonitor.isSupportedProviderInstallDirectory(
702+
installDirectory,
703+
under: homeDirectory
704+
),
705+
binaryPath == expectedBinaryPath,
706+
launchdLabels.contains(InstalledProviderMonitor.providerLaunchdLabel),
707+
launchdPlists.contains(launchd.path),
708+
let plistData = InstalledProviderMonitor.readOwnerPrivateRegularFile(
709+
launchd,
710+
maxBytes: 64 * 1024,
711+
fileManager: fm
712+
),
713+
let plistObject = try? PropertyListSerialization.propertyList(
714+
from: plistData,
715+
options: [],
716+
format: nil
717+
),
718+
let plistValues = plistObject as? [String: Any],
719+
plistValues["Label"] as? String == InstalledProviderMonitor.providerLaunchdLabel else {
720+
return false
721+
}
722+
let plistProgram = (plistValues["Program"] as? String)
723+
?? (plistValues["ProgramArguments"] as? [String])?.first
724+
let legacyBinaryPath = homeDirectory
725+
.appendingPathComponent(".local/bin/macprovider-cli")
726+
.path
727+
return plistProgram == binaryPath || plistProgram == legacyBinaryPath
728+
}
729+
640730
static func applyMigrationDecision(
641731
_ decision: MigrationDecision,
642732
paths: ProviderPaths = .current,

phase3-binary/app/Sources/Malibu/System/InstalledProviderMonitor.swift

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -164,13 +164,14 @@ enum InstalledProviderMonitor {
164164
case unavailable
165165
case notLoaded
166166
case validExecutable
167+
case legacyExecutable(path: String)
167168
case missingExecutable(path: String)
168169
case unexpectedExecutable(path: String)
169170
case unexpectedPlist(path: String)
170171

171172
var needsRepair: Bool {
172173
switch self {
173-
case .missingExecutable:
174+
case .legacyExecutable, .missingExecutable:
174175
return true
175176
case .unavailable, .notLoaded, .validExecutable, .unexpectedExecutable, .unexpectedPlist:
176177
return false
@@ -181,14 +182,15 @@ enum InstalledProviderMonitor {
181182
switch self {
182183
case .unexpectedExecutable, .unexpectedPlist:
183184
return true
184-
case .unavailable, .notLoaded, .validExecutable, .missingExecutable:
185+
case .unavailable, .notLoaded, .validExecutable, .legacyExecutable, .missingExecutable:
185186
return false
186187
}
187188
}
188189
}
189190

190-
/// A readable plist is not enough to attach Malibu to a loaded job. Repair
191-
/// is reserved for a managed job whose executable is missing; an unloaded
191+
/// A readable plist is not enough to attach Malibu to a loaded job. Legacy
192+
/// standalone executables are repairable stale ownership, while unknown
193+
/// executable/plist identities remain manual conflicts. An unloaded
192194
/// managed plist is still inspected so a stale binary cannot dead-end startup.
193195
static func launchdServiceRepairState(
194196
uid: uid_t = getuid(),
@@ -233,6 +235,9 @@ enum InstalledProviderMonitor {
233235
guard isOwnerPrivateExecutable(atPath: plistIdentity.program) else {
234236
return .missingExecutable(path: plistIdentity.program)
235237
}
238+
if plistIdentity.program == legacyProgram {
239+
return .legacyExecutable(path: plistIdentity.program)
240+
}
236241
return .notLoaded
237242
}
238243
guard let identity = parseLaunchdServiceIdentity(inspection.output) else {
@@ -263,6 +268,9 @@ enum InstalledProviderMonitor {
263268
guard isOwnerPrivateExecutable(atPath: identity.program) else {
264269
return .missingExecutable(path: identity.program)
265270
}
271+
if identity.program == legacyProgram {
272+
return .legacyExecutable(path: identity.program)
273+
}
266274
return .validExecutable
267275
}
268276

@@ -292,7 +300,7 @@ enum InstalledProviderMonitor {
292300
fileManager: FileManager
293301
) -> Data? {
294302
let descriptor = url.path.withCString {
295-
Darwin.open($0, O_RDONLY | O_CLOEXEC | O_NOFOLLOW)
303+
Darwin.open($0, O_RDONLY | O_NONBLOCK | O_CLOEXEC | O_NOFOLLOW)
296304
}
297305
guard descriptor >= 0 else { return nil }
298306
defer { _ = Darwin.close(descriptor) }
@@ -354,7 +362,7 @@ enum InstalledProviderMonitor {
354362

355363
static func isOwnerPrivateExecutable(atPath path: String) -> Bool {
356364
let descriptor = path.withCString {
357-
Darwin.open($0, O_RDONLY | O_CLOEXEC | O_NOFOLLOW)
365+
Darwin.open($0, O_RDONLY | O_NONBLOCK | O_CLOEXEC | O_NOFOLLOW)
358366
}
359367
guard descriptor >= 0 else { return false }
360368
defer { _ = Darwin.close(descriptor) }

phase3-binary/app/Sources/Malibu/System/LogTailReader.swift

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,8 @@ final class LogTailReader: ObservableObject {
152152
(fileInfo.st_mode & S_IFMT) == S_IFREG,
153153
fileInfo.st_uid == getuid(),
154154
Int(fileInfo.st_nlink) == 1,
155+
fileInfo.st_mode & (S_IWGRP | S_IWOTH) == 0,
156+
hasNoExtendedACL(descriptor),
155157
fileInfo.st_size >= 0 else {
156158
return nil
157159
}
@@ -199,6 +201,15 @@ final class LogTailReader: ObservableObject {
199201
return ReadResult(offset: nextOffset, pendingFragment: parsed.pendingFragment, lines: parsed.lines)
200202
}
201203

204+
private nonisolated static func hasNoExtendedACL(_ descriptor: Int32) -> Bool {
205+
errno = 0
206+
guard let acl = acl_get_fd_np(descriptor, ACL_TYPE_EXTENDED) else {
207+
return errno == 0 || errno == ENOENT
208+
}
209+
_ = acl_free(UnsafeMutableRawPointer(acl))
210+
return false
211+
}
212+
202213
nonisolated private static func parseChunk(
203214
_ chunk: String,
204215
pendingFragment: String,

phase3-binary/app/Sources/Malibu/System/ProviderConfig.swift

Lines changed: 77 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,22 +23,92 @@ enum ProviderConfig {
2323
}
2424

2525
static func readProviderID(paths: ProviderPaths = .current) -> String? {
26-
guard let contents = try? String(contentsOf: paths.configFile) else { return nil }
26+
guard let contents = readTrustedConfigContents(paths: paths) else { return nil }
2727
return parseTopLevelValue(named: "provider_id", from: contents)
2828
}
2929

30+
/// Read the shared config through a nonblocking, no-follow descriptor so a
31+
/// malformed or replaced config path cannot wedge startup before the
32+
/// caller's trust checks run. The installer applies the same owner/private
33+
/// regular-file contract before using this identity for repair.
34+
private static func readTrustedConfigContents(paths: ProviderPaths) -> String? {
35+
let maxBytes = 1024 * 1024
36+
let configDirectory = paths.configFile.deletingLastPathComponent()
37+
let homeDirectory = configDirectory
38+
.deletingLastPathComponent()
39+
.deletingLastPathComponent()
40+
guard InstalledProviderMonitor.isSafePrivateDirectoryChain(
41+
configDirectory,
42+
under: homeDirectory
43+
) else {
44+
return nil
45+
}
46+
let descriptor = paths.configFile.path.withCString {
47+
Darwin.open($0, O_RDONLY | O_NONBLOCK | O_CLOEXEC | O_NOFOLLOW)
48+
}
49+
guard descriptor >= 0 else { return nil }
50+
defer { _ = Darwin.close(descriptor) }
51+
52+
var before = stat()
53+
guard Darwin.fstat(descriptor, &before) == 0,
54+
(before.st_mode & S_IFMT) == S_IFREG,
55+
before.st_uid == getuid(),
56+
before.st_nlink == 1,
57+
before.st_mode & (S_IWGRP | S_IWOTH) == 0,
58+
before.st_size >= 0,
59+
before.st_size <= off_t(maxBytes),
60+
hasNoExtendedACL(descriptor) else {
61+
return nil
62+
}
63+
var data = Data()
64+
data.reserveCapacity(Int(before.st_size))
65+
var buffer = [UInt8](repeating: 0, count: min(64 * 1024, maxBytes))
66+
while data.count < Int(before.st_size) {
67+
let count = buffer.withUnsafeMutableBytes { storage in
68+
Darwin.read(descriptor, storage.baseAddress, storage.count)
69+
}
70+
guard count > 0 else { return nil }
71+
data.append(buffer, count: count)
72+
}
73+
var after = stat()
74+
var pathInfo = stat()
75+
guard Darwin.fstat(descriptor, &after) == 0,
76+
Darwin.lstat(paths.configFile.path, &pathInfo) == 0,
77+
after.st_dev == before.st_dev,
78+
after.st_ino == before.st_ino,
79+
after.st_size == before.st_size,
80+
after.st_mtimespec.tv_sec == before.st_mtimespec.tv_sec,
81+
after.st_mtimespec.tv_nsec == before.st_mtimespec.tv_nsec,
82+
after.st_ctimespec.tv_sec == before.st_ctimespec.tv_sec,
83+
after.st_ctimespec.tv_nsec == before.st_ctimespec.tv_nsec,
84+
(pathInfo.st_dev, pathInfo.st_ino) == (before.st_dev, before.st_ino),
85+
hasNoExtendedACL(descriptor),
86+
data.count == Int(before.st_size) else {
87+
return nil
88+
}
89+
return String(data: data, encoding: .utf8)
90+
}
91+
92+
private static func hasNoExtendedACL(_ descriptor: Int32) -> Bool {
93+
guard let acl = acl_get_fd_np(descriptor, ACL_TYPE_EXTENDED) else {
94+
return errno == 0 || errno == ENOENT
95+
}
96+
acl_free(UnsafeMutableRawPointer(acl))
97+
return false
98+
}
99+
30100
static func hasProviderToken(paths: ProviderPaths = .current) -> Bool {
31-
guard let contents = try? String(contentsOf: paths.configFile) else { return false }
101+
guard let contents = readTrustedConfigContents(paths: paths) else { return false }
32102
return parseTopLevelValue(named: "provider_token", from: contents) != nil
33103
}
34104

35105
static func readModel(paths: ProviderPaths = .current) -> String? {
36-
guard let contents = try? String(contentsOf: paths.configFile) else { return nil }
106+
guard let contents = readTrustedConfigContents(paths: paths) else { return nil }
37107
return parseTopLevelValue(named: "model", from: contents)
38108
}
39109

40110
static func readHTTPPort(paths: ProviderPaths = .current) -> Int? {
41-
guard let contents = try? String(contentsOf: paths.configFile),
111+
guard let contents = readTrustedConfigContents(paths: paths),
42112
let value = parseTopLevelValue(named: "port", from: contents),
43113
let port = Int(value),
44114
(1024...65535).contains(port) else {
@@ -48,7 +118,7 @@ enum ProviderConfig {
48118
}
49119

50120
static func readLinkState(paths: ProviderPaths = .current) -> LinkState? {
51-
guard let contents = try? String(contentsOf: paths.configFile),
121+
guard let contents = readTrustedConfigContents(paths: paths),
52122
let value = parseTopLevelValue(named: "link_state", from: contents)
53123
else {
54124
return nil
@@ -63,7 +133,7 @@ enum ProviderConfig {
63133
paths: ProviderPaths = .current,
64134
environment: [String: String] = ProcessInfo.processInfo.environment
65135
) -> Bool {
66-
let contents = try? String(contentsOf: paths.configFile)
136+
let contents = readTrustedConfigContents(paths: paths)
67137
return automaticUpdatesEnabled(configContents: contents, environment: environment)
68138
}
69139

@@ -187,7 +257,7 @@ enum ProviderConfig {
187257
}
188258

189259
static func validateServeConfigShape(paths: ProviderPaths = .current) throws {
190-
guard let contents = try? String(contentsOf: paths.configFile) else {
260+
guard let contents = readTrustedConfigContents(paths: paths) else {
191261
throw ServeConfigError.missingField("config.yaml")
192262
}
193263
let config = try AutotuneServeConfig(

0 commit comments

Comments
 (0)