container system start ignores launchctl exit status and daemon app-root, silently adopting a daemon bound to a different --app-root
- Dominant language
- Swift
- Stars
- 49.9k
- Forks
- 1.8k
- Avg merge
- 1d 20h
- Merged PRs (30d)
- 22
Description
## Summary
`container system start` can report success while not actually honoring the requested `--app-root`. Two compounding gaps cause this:
1. **The `launchctl bootstrap` exit status is discarded.** A failed or no-op load (e.g. when the label is already bootstrapped) is never surfaced.
2. **There is no `--app-root` identity check.** The launchd label and mach service are a fixed constant, so when a daemon is already running for app-root A, `container system start --app-root B` reaches the existing daemon over the shared mach service, the health-check ping succeeds, and `start` reports success — even though the daemon is still bound to app-root A and B's plist/environment were never applied.
The health-check ping already returns the live daemon's actual `appRoot`, but `system start` throws that value away, so the mismatch goes undetected.
## Affected files
**`Sources/ContainerPlugin/ServiceManager.swift`**
- `runLaunchctlCommand(args:)` — lines 21–34: returns `terminationStatus` but only `throw`s if `/bin/launchctl` fails to spawn (line 30); the status itself is never inspected.
- `register(plistPath:)` — lines 37–40: discards the status with `_ = try runLaunchctlCommand(args: ["bootstrap", domain, plistPath])` (line 39).
- For contrast, the same file *does* check the status in `enumerate()` (line 78), `isRegistered(fullServiceLabel:)` (lines 96–98), and `getLaunchdSessionType()` (lines 114–116) — so the omission in `register()` is inconsistent with the rest of the file.
**`Sources/ContainerCommands/System/SystemStart.swift`**
- Fixed, app-root-independent label and mach service: lines 116 and 121 (`com.apple.container.apiserver`).
- `try ServiceManager.register(plistPath:)` — line 130 (treated as success regardless of the bootstrap result).
- `_ = try await ClientHealthCheck.ping(timeout: timeout)` — line 135: the `SystemHealth` result (including the daemon's `appRoot`) is discarded; it is never compared to the `appRoot` option declared at line 39.
- No `isRegistered` / `bootout` / `kickstart` guard exists anywhere in this start path.
**`Sources/Services/ContainerAPIService/Client/ClientHealthCheck.swift`**
- `ping(timeout:)` — lines 31–63: targets the fixed service `com.apple.container.apiserver` (line 23) and returns `SystemHealth` carrying the daemon's actual `appRoot` (lines 35, 54–55).
Supporting evidence that the returned `appRoot` is usable: **`Sources/ContainerCommands/System/SystemStatus.swift`** lines 84–94 already consume `systemHealth.appRoot`, demonstrating the value is available and that `start` simply does not use it.
## Steps to reproduce
1. Start the service bound to app-root A: `container system start --app-root /path/A`. The apiserver launches as a launchd singleton under the fixed label `com.apple.container.apiserver`, reading `CONTAINER_APP_ROOT` from its plist-injected environment, and binds the fixed mach service.
2. Without stopping it, start again with a different app-root: `container system start --app-root /path/B`.
3. `register()` runs `launchctl bootstrap` for the already-bootstrapped label. `launchctl` returns a non-zero status (the already-loaded case), but `register()` discards it (`ServiceManager.swift:39`), so `register()` returns normally and `SystemStart.run()` treats it as success.
4. `ClientHealthCheck.ping` (`SystemStart.swift:135`) connects over the fixed mach service to the **existing** daemon (still bound to app-root A) and succeeds.
5. The ping's `SystemHealth.appRoot` would reveal that the live daemon is bound to A, not B, but it is discarded with `_ = try await ...`, so no comparison against the requested `--app-root` is made.
6. `system start --app-root B` exits successfully, having neither booted out the existing daemon nor applied B's plist/environment.
## Expected behavior
- A non-zero / no-op result from `launchctl bootstrap` should be surfaced (or otherwise handled), not silently swallowed by `register()`.
- When a daemon is already running, `system start` should either detect it (e.g. via `isRegistered` / `bootout` before re-bootstrapping) or compare the running daemon's `appRoot` (already returned by `ClientHealthCheck.ping`) against the requested `--app-root` and fail loudly on mismatch, rather than reporting success for an app-root it did not actually start.
## Actual behavior
`system start --app-root B` reports success while the running daemon remains bound to app-root A. The bootstrap's failure/no-op status is discarded, and the ping result that would expose the mismatch is also discarded. The user believes B is running; it is not.
## Suggested fix
Either fix is independently useful; together they close the gap:
1. **Surface the bootstrap status.** In `ServiceManager.register(plistPath:)`, inspect the return value of `runLaunchctlCommand` and throw on a non-zero status (consistent with `enumerate()` / `isRegistered()` / `getLaunchdSessionType()` in the same file).
2. **Add an app-root identity check in the start path.** Before bootstrapping, check `ServiceManager.isRegistered(...)`; if already running, either `bootout` first or capture `ClientHealthCheck.ping`'s result (as `SystemStatus.swift` does) and compare `systemHealth.appRoot` to the requested `appRoot`, failing with a clear message on mismatch. `SystemStart.swift:135` already calls `ping` but discards the result, so the comparison is nearly free.
## Severity & scope
`container` is a single-user, local-development tool, and running multiple daemons against distinct app-roots simultaneously is an uncommon workflow, so real-world impact is limited. The harm is a misleading success message: the user is told app-root B started when daemon A is what is actually running, which can cause confusing downstream behavior (commands operate against the wrong data root) that is hard to diagnose because no error is shown. There is no data corruption or security impact.
(Note: the exact non-zero exit of `launchctl bootstrap` on an already-loaded label is documented macOS behavior; the two code-discard defects above are unconditionally present in the source regardless.)
Contributor guide
Research direction
Read Sources/ContainerPlugin/ServiceManager.swift, especially runLaunchctlCommand(args:) and register(plistPath:), then follow SystemStart.run in Sources/ContainerCommands/System/SystemStart.swift. Compare its discarded bootstrap status and ClientHealthCheck.ping result with the checked statuses elsewhere and SystemStatus.swift's appRoot use. Reproduce with two different --app-root values; done means bootstrap failures or app-root mismatches are reported instead of success.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- macos, swift
- Domain
- cli, operating-systems
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 55/100