Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Sources/KeyPathApp/Info.plist
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
<key>CFBundleDisplayName</key>
<string>KeyPath</string>
<key>CFBundleVersion</key>
<string>10</string>
<string>11</string>
<key>CFBundleShortVersionString</key>
<string>1.0.1</string>
<key>CFBundlePackageType</key>
Expand Down
2 changes: 1 addition & 1 deletion Sources/KeyPathCore/KeyPathHelperContract.swift
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/// Shared identity and compatibility contract for the privileged helper.
public enum KeyPathHelperContract {
/// Version returned by the helper XPC service and packaged in its Info.plist.
public static let version = "1.3.1"
public static let version = "1.3.2"
}
46 changes: 35 additions & 11 deletions Sources/KeyPathHelper/HelperService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,13 @@
executePrivilegedOperation(
name: "stopKanataService",
operation: {
// Capture the live process before disabling the service. Once a
// launchd job is disabled, `launchctl kill system/<label>` can no
// longer resolve the target even though its existing process is
// still alive. Signalling the captured PID closes that gap while
// keeping the disable-before-signal ordering that prevents a
// KeepAlive respawn.
let processID = Self.serviceProcessID(Self.kanataServiceID)
// com.keypath.kanata is KeepAlive. Disable it before signaling the
// process so launchd does not immediately respawn it while the CLI
// is waiting for the stopped postcondition.
Expand All @@ -184,17 +191,27 @@
"Failed to disable KeyPath Kanata service: \(disableResult.out)"
)
}
let result = Self.run(
"/bin/launchctl",
["kill", "SIGTERM", Self.kanataServiceTarget],
timeout: 15
)
if result.status != 0,
result.out.localizedCaseInsensitiveContains("No process to signal"),
!Self.isServiceHealthy(Self.kanataServiceID)
{
// The registered KeepAlive job is already stopped or waiting
// for launchd's throttle window. Stop is idempotently complete.
guard let processID else {
if !Self.isServiceHealthy(Self.kanataServiceID) {
// The registered KeepAlive job is already stopped or
// waiting for launchd's throttle window. Stop is
// idempotently complete.
return
}
_ = Self.run(
"/bin/launchctl",
["enable", Self.kanataServiceTarget],
timeout: 15
)
throw HelperError.operationFailed(
"Failed to identify the KeyPath Kanata service process"
)
}
let result = Self.run("/bin/kill", ["-TERM", processID], timeout: 15)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Revalidate the PID after disabling the job

If Kanata exits or restarts between the initial launchctl print and the disable operation, processID identifies the previous process rather than the current service instance. The raw root-level kill can then either return ESRCH while a replacement remains alive—causing stop to re-enable the job and fail—or, after PID reuse, signal an unrelated process. Re-read and validate the job's current PID after disabling it, or use another service-owned mechanism that preserves process identity, before signaling.

Useful? React with 👍 / 👎.

if result.status != 0, !Self.isServiceHealthy(Self.kanataServiceID) {
// The process exited between inspection and signaling. The
// service remains disabled, so the stopped postcondition is
// already satisfied.
return
}
guard result.status == 0 else {
Expand Down Expand Up @@ -862,7 +879,7 @@
return (
result.status,
result.out
+ "\nVirtualHID activation may be waiting for macOS approval. Open System Settings > Privacy & Security and approve the Karabiner VirtualHIDDevice system extension, then retry repair."

Check warning on line 882 in Sources/KeyPathHelper/HelperService.swift

View workflow job for this annotation

GitHub Actions / code-quality

Line should be 200 characters or less; currently it has 204 characters (line_length)
)
}
return result
Expand Down Expand Up @@ -904,6 +921,13 @@
return r.status == 0
}

static func serviceProcessID(_ serviceID: String) -> String? {
let result = run("/bin/launchctl", ["print", "system/\(serviceID)"])
guard result.status == 0 else { return nil }
return firstMatch(#"\bpid\s*=\s*([0-9]+)"#, in: result.out)
?? firstMatch(#""PID"\s*=\s*([0-9]+)"#, in: result.out)
}

static func isServiceHealthy(_ serviceID: String) -> Bool {
let r = run("/bin/launchctl", ["print", "system/\(serviceID)"])
guard r.status == 0 else { return false }
Expand Down
4 changes: 2 additions & 2 deletions Sources/KeyPathHelper/Info.plist
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@

<!-- Version (should match main app) -->
<key>CFBundleShortVersionString</key>
<string>1.3.1</string>
<string>1.3.2</string>

<key>CFBundleVersion</key>
<string>4</string>
<string>5</string>

<!-- Info dictionary version -->
<key>CFBundleInfoDictionaryVersion</key>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,19 @@ final class HelperServiceKeepAliveLifecycleLintTests: XCTestCase {
let source = try helperServiceSource()
let stopBody = try functionBody(named: "stopKanataService", in: source)

let inspect = try XCTUnwrap(stopBody.range(of: "Self.serviceProcessID(Self.kanataServiceID)"))
let disable = try XCTUnwrap(stopBody.range(of: "[\"disable\", Self.kanataServiceTarget]"))
let signal = try XCTUnwrap(stopBody.range(of: "[\"kill\", \"SIGTERM\", Self.kanataServiceTarget]"))
let signal = try XCTUnwrap(stopBody.range(of: "Self.run(\"/bin/kill\", [\"-TERM\", processID]"))
let restore = try XCTUnwrap(
stopBody.range(
of: "[\"enable\", Self.kanataServiceTarget]",
range: signal.upperBound ..< stopBody.endIndex
)
)
XCTAssertLessThan(inspect.lowerBound, disable.lowerBound)
XCTAssertLessThan(disable.lowerBound, signal.lowerBound)
XCTAssertLessThan(signal.lowerBound, restore.lowerBound)
XCTAssertFalse(stopBody.contains("[\"kill\", \"SIGTERM\", Self.kanataServiceTarget]"))
}

func testHelperReenablesServiceBeforeStartingOrRestartingIt() throws {
Expand Down
14 changes: 10 additions & 4 deletions docs/bugs/cli-service-control-helper-bypass.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,13 @@ reported `Could not stop Kanata service`.

The helper now disables `system/com.keypath.kanata` before signaling it. Start and restart
explicitly re-enable the job before kickstart, and an unexpected signal failure restores the
enabled state. The helper contract advanced to 1.3.1 so installations cannot retain the earlier
behavior while reporting the helper as fresh. A lifecycle lint test preserves the required
disable-before-kill and enable-before-kickstart ordering; installed-app acceptance verifies the
real launchd transition.
enabled state.

A second installed-app acceptance run exposed a launchd ordering detail: after `launchctl
disable`, `launchctl kill system/com.keypath.kanata` could no longer resolve the target even
though its existing process was still alive. The helper now captures the registered service PID
before disabling the job, then signals that exact PID after the disable succeeds. The helper
contract advanced to 1.3.2 so installations cannot retain either earlier behavior while
reporting the helper as fresh. A lifecycle lint test preserves the required
inspect-before-disable-before-signal and enable-before-kickstart ordering; installed-app
acceptance verifies the real launchd transition.
Loading