From f8fa6f645b3a1c32a6233c1ee291df4dac45fb7e Mon Sep 17 00:00:00 2001 From: Micah Alpern Date: Wed, 4 Nov 2026 20:10:15 -0800 Subject: [PATCH 01/30] Review the session engine's TCP control port (ADR-046) The engine's unauthenticated port on 127.0.0.1:37001 now fronts the session worker, which holds the user's input permissions. ADR-046 lists what the port accepts and broadcasts, who can reach it, and proposes an owner-checked Unix socket with a peer check. ADR-013 points to it, and the KanataTCPClient security note no longer says the surface is limited to config reloads. Refs #1354 Co-Authored-By: Claude Opus 5.5 --- .../Services/Networking/KanataTCPClient.swift | 29 +- docs/adr/README.md | 3 +- docs/adr/adr-013-tcp-without-auth.md | 10 +- .../adr-046-session-engine-control-channel.md | 266 ++++++++++++++++++ docs/adr/index.md | 3 +- 5 files changed, 286 insertions(+), 25 deletions(-) create mode 100644 docs/adr/adr-046-session-engine-control-channel.md diff --git a/Sources/KeyPathAppKit/Services/Networking/KanataTCPClient.swift b/Sources/KeyPathAppKit/Services/Networking/KanataTCPClient.swift index 2217f11ed..a0f838023 100644 --- a/Sources/KeyPathAppKit/Services/Networking/KanataTCPClient.swift +++ b/Sources/KeyPathAppKit/Services/Networking/KanataTCPClient.swift @@ -56,25 +56,16 @@ final class CompletionFlag: @unchecked Sendable { /// - Simple timeout mechanism /// - Connection pooling for efficiency /// -/// **SECURITY NOTE (ADR-013):** -/// Kanata v1.9.0 TCP server does NOT support authentication. -/// The tcp_server.rs code explicitly ignores Authenticate messages: -/// ```rust -/// ClientMessage::Authenticate { .. } => { -/// log::debug!("TCP server ignoring authentication message (not needed for TCP)"); -/// continue; -/// } -/// ``` -/// -/// This means: -/// - No client identity verification -/// - Any local process can send commands to localhost:37001 -/// - Limited attack surface: only config reloads, not arbitrary code execution -/// -/// **Future Work:** -/// Consider contributing authentication support to upstream Kanata. -/// Design would mirror the UDP authentication (token-based, session expiry). -/// Until then, rely on localhost-only binding and OS-level process isolation. +/// **SECURITY NOTE (ADR-013, under review in ADR-046 / #1354):** +/// The engine's TCP server has no authentication, and loopback TCP is not +/// scoped to a user or to the App Sandbox. Any local process, including other +/// users' processes and sandboxed apps, can connect to localhost:37001 and: +/// - receive every `KeyInput` event (a keystroke stream) and the config text +/// - press virtual keys (`ActOnFakeKey`), change layers, move the mouse +/// - load any readable file as the config (`ReloadFile`) +/// Virtual keys bound to `push-msg` reach `ActionDispatcher` through +/// `KanataEventListener`. ADR-046 proposes moving this channel to an +/// owner-checked Unix socket. actor KanataTCPClient { let host: String let port: Int diff --git a/docs/adr/README.md b/docs/adr/README.md index cb807ae93..55e8303d4 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -10,7 +10,7 @@ This directory contains Architecture Decision Records (ADRs) documenting signifi | [ADR-006](adr-006-apple-api-priority.md) | Apple API Priority in Permission Checks | Partly superseded by ADR-044 | | [ADR-008](adr-008-validation-refactor.md) | Stateless Validation via SystemValidator | Accepted | | [ADR-009](adr-009-service-extraction-mvvm.md) | Service Extraction & MVVM Pattern | Accepted | -| [ADR-013](adr-013-tcp-without-auth.md) | TCP Communication Without Authentication | Accepted (under review) | +| [ADR-013](adr-013-tcp-without-auth.md) | TCP Communication Without Authentication | Under review, see ADR-046 | | [ADR-014](adr-014-xpc-signature-mismatch.md) | XPC Signature Mismatch Prevention | Accepted (see ADR-045 note) | | [ADR-015](adr-015-installer-engine.md) | InstallerEngine Façade | Accepted (see ADR-045 note) | | [ADR-016](adr-016-tcc-database-reading.md) | TCC Database Reading for Sequential Permission Flow | Superseded by ADR-044 | @@ -43,6 +43,7 @@ This directory contains Architecture Decision Records (ADRs) documenting signifi | [ADR-043](adr-043-opportunistic-manager-consolidation.md) | Opportunistic Manager Consolidation | Accepted | | [ADR-044](adr-044-remove-full-disk-access.md) | Remove Full Disk Access and browser-history suggestions | Accepted | | [ADR-045](adr-045-retire-privileged-runtime-stack.md) | Retire the Privileged Runtime Stack | Accepted | +| [ADR-046](adr-046-session-engine-control-channel.md) | Session Engine Control Channel | Proposed | ## Key Decisions Summary diff --git a/docs/adr/adr-013-tcp-without-auth.md b/docs/adr/adr-013-tcp-without-auth.md index 5512058e4..344fd68e8 100644 --- a/docs/adr/adr-013-tcp-without-auth.md +++ b/docs/adr/adr-013-tcp-without-auth.md @@ -1,11 +1,13 @@ # ADR-013: TCP Communication Without Authentication -> **Note (2026-10-31):** This was written for the root daemon. The runtime now runs -> in a user session that holds the user's input permissions, so this risk needs -> review. See ADR-045; tracked in +> **Note (2026-11-04):** This was written for the root daemon. The session +> runtime holds the user's input permissions, and the port now exposes a +> keystroke stream and input injection to every local process. The review and +> the proposed replacement are in +> [ADR-046](adr-046-session-engine-control-channel.md); tracked in > [#1354](https://github.com/malpern/KeyPath/issues/1354). -**Status:** Accepted (under review, see ADR-045) +**Status:** Under review; to be superseded by [ADR-046](adr-046-session-engine-control-channel.md) (Proposed) **Date:** 2024 ## Context diff --git a/docs/adr/adr-046-session-engine-control-channel.md b/docs/adr/adr-046-session-engine-control-channel.md new file mode 100644 index 000000000..0baaf3bd7 --- /dev/null +++ b/docs/adr/adr-046-session-engine-control-channel.md @@ -0,0 +1,266 @@ +# ADR-046: Session Engine Control Channel + +**Status:** Proposed +**Date:** 2026-11-04 +**Supersedes (when accepted):** [ADR-013](adr-013-tcp-without-auth.md) +**Tracking:** [#1354](https://github.com/malpern/KeyPath/issues/1354) (release-blocker) + +## Context + +ADR-013 accepted an unauthenticated JSON control port on `127.0.0.1:37001`. The +engine behind it was a root daemon, and the risk was written up as "any local +process can send commands … low security impact". + +ADR-045 retired that daemon. The engine now runs inside the session worker, an +unprivileged process in the user's session that holds the user's Accessibility +and Input Monitoring grants. It still opens the same port: +`Rust/KeyPathKanataHostBridge` passes the port to +`kanata_state_machine::SocketAddrWrapper::from_str`, which binds +`127.0.0.1:`, and starts the fork's `TcpServer` and notification loop. + +Loopback TCP is not scoped to a user, to a code signature, or to the App Sandbox. +Any process on the Mac that can open a socket can connect. That includes +processes of other logged-in users and sandboxed apps that hold +`com.apple.security.network.client`. The server does no authentication of any +kind. Every accepted connection joins the broadcast set before it has sent a +byte. + +Facts below were read from the fork at `External/kanata` (`keypath/bundled`, +`d9085a9`) and from KeyPath `master` at `e42ffea0`. They were not exercised on a +live install for this ADR. + +### What a connected client can do + +Commands the server accepts (`tcp_protocol/src/lib.rs`, `ClientMessage`): + +| Command | Effect | Used by KeyPath | +|---|---|---| +| `Hello` | Version and capability list | Yes | +| `RequestLayerNames`, `RequestFakeKeyNames`, `RequestCurrentLayerName` | Read layer and virtual-key names | Yes | +| `RequestCurrentLayerInfo` | Returns the current layer's **config text** | Yes (overlay/inspector) | +| `RequestInputGrab` | Read whether input is seized, and which devices | Yes | +| `ChangeLayer { new }` | Switch the active layer | Yes | +| `ActOnFakeKey { name, action }` | Press, release, tap or toggle any virtual key defined in the loaded config. A virtual key runs whatever the config binds to it: key output, macros, layer changes, `push-msg` | Yes (`keypath://fakekey`, inspector) | +| `SetMouse { x, y }` | Move the pointer to absolute coordinates | No | +| `Reload`, `ReloadNext`, `ReloadPrev`, `ReloadNum` | Reload or switch between known configs | `Reload` only | +| `ReloadFile { path }` | Load **any existing file the worker can read** as the active config (`request_live_reload_file`) | No | + +Anything else, including KeyPath's own `SubscribeHrmTrace`, fails to +deserialize and closes the connection. That request is sent only when `Hello` +advertises `hrm-trace`, which this fork does not. + +Events broadcast to **every** connected client (`Kanata::start_notification_loop`): + +- `KeyInput { key, action, t }` for every physical key press, release and repeat + (`emit_key_input`, called for every event in the processing loop). Together + with timestamps this is a keystroke log of everything typed outside + Secure Input contexts. +- `LayerChange`, `HoldActivated`, `TapActivated`, `ChordResolved`, + `TapDanceResolved`, `InputGrab`, and `MessagePush`, which carries the + `keypath://…` action URIs the config emits. + +`cmd` is compiled out (`Scripts/build-kanata.sh`, #879), so a config cannot run +shell commands inside the engine. + +### What KeyPath does with the port + +| Client | What it sends or reads | +|---|---| +| `KanataEventListener` (app) | Persistent connection. Reads `KeyInput`, layer and tap-hold events, `InputGrab`. Dispatches every `MessagePush` that parses as a `keypath://` URI to `RuleCollectionsManager.handleActionURI`, which calls `ActionDispatcher.shared.dispatch` | +| `KanataTCPClient` via `ConfigReloadCoordinator`, `ConfigHotReloadService`, `ReloadSafetyMonitor` | `Hello`, `Reload { wait }` | +| `ActionDispatcher` (`keypath://fakekey/…`, `keypath://layer/…`) | `ActOnFakeKey`, `ChangeLayer` | +| `VirtualKeysInspectorView`, `RuleCollectionsManager`, `AppContextService`, `HrmObservabilityService` | Virtual-key names, `ActOnFakeKey`, layer state | +| `ServiceHealthMonitor`, `DiagnosticsService`, `SystemValidator`, `WizardCommunicationPage` | `Hello`/health probes | +| `keypath-cli` (`ConfigFacade.tcpReload`, `tcpGetCurrentLayer`) | Opens its **own** TCP connection, unlike `status/start/stop/doctor`, which use the owner-checked Unix socket in `docs/architecture/cli-runtime-control.md` | +| `Scripts/verify-installed-app.sh` | Checks with `lsof` that the listener on the port belongs to the expected worker | + +The app never sends `ReloadFile`, `ReloadNext/Prev/Num` or `SetMouse`. + +## Threat model + +**Asset:** the user's keystrokes, and the authority of a process that holds +Accessibility and Input Monitoring: it can emit input, and KeyPath runs actions +on its behalf. + +**Attackers considered:** + +1. **A process of another macOS user** on the same Mac (fast user switching, + a shared Mac, a service account). Today it can connect. +2. **A sandboxed app with the network-client entitlement.** The sandbox keeps + it out of `~/Library`, but not off loopback TCP. Today it can connect. +3. **A same-user, unsandboxed process with no TCC grants**, such as a script, a + package postinstall, or a compromised developer tool. Today it can connect. + It can already do a lot as the user, but not read global keystrokes or post + input without its own grants. This port gives it both. +4. **A process that binds `127.0.0.1:37001` before the worker does** (any user, + for example at login or during a restart). The worker's + `TcpListener::bind(...).expect` then panics, so the runtime fails to start, + and KeyPath's own clients connect to the squatter instead. KeyPath does not + check who owns the listener; only `verify-installed-app.sh` does. + +Web pages are out of scope: browsers speak HTTP or WebSocket to that port, the +first line is not valid JSON, and the server disconnects. + +**Out of scope:** root, and a same-user process that can already inject code +into KeyPath or the worker. Those defeat any channel design. + +### Concrete attacks today + +| From | Attack | Impact | +|---|---|---| +| 1, 2, 3 | Connect and read `KeyInput` | **Keystroke logging** of the active user, including another user's keystrokes (1) and from inside a sandbox (2) | +| 1, 2, 3 | `RequestCurrentLayerInfo`, `RequestFakeKeyNames` | Read the user's config and the names of every virtual key | +| 1, 2, 3 | `ActOnFakeKey` | Trigger any virtual-key action: output keys and macros into the focused app, change layers, emit `push-msg` | +| 1, 2, 3 | `ActOnFakeKey` on a key bound to `push-msg` | KeyPath's event listener receives the resulting `keypath://` URI and runs it: launch apps, open any URL, `system` actions, `runtime/stop`, and `script` when the user has enabled scripts. If the user also turned off the confirmation dialog, the script runs without a prompt | +| 1, 2, 3 | `ReloadFile` with an attacker-written `.kbd` | Replace the user's mappings with arbitrary ones: key injection, swallowing keys, `push-msg` actions of the attacker's choosing, which reach `ActionDispatcher` as above. 2 cannot plant a readable file outside its container easily; 1 and 3 can | +| 1, 2, 3 | `SetMouse` | Move the pointer | +| 1, 2, 3 | Open many connections, or connect and never read | One OS thread per connection with no limit. The notification loop writes to every client with blocking `write_all` while holding the client lock, so one stalled reader can delay events, including `push-msg` actions, for KeyPath. Not tested | +| 4 | Squat the port | Runtime fails to start. KeyPath's listener connects to the squatter and dispatches any `MessagePush` it sends as `keypath://` actions | + +So ADR-013's note "Limited attack surface: only config reloads, not arbitrary +code execution" (repeated in `KanataTCPClient.swift`) is wrong for the session +runtime. The port exposes a keystroke stream, input injection, config +replacement, and an indirect path to KeyPath's action dispatcher. + +## Options + +### A. Keep TCP, narrow the commands + +Remove `ReloadFile`, `ReloadNext/Prev/Num` and `SetMouse` from the fork (KeyPath +uses none of them). Make `KeyInput` opt-in per connection. Cap connections and +give notification writes a timeout. + +- Removes config replacement and pointer control. +- **Does not** remove keystroke logging, virtual-key injection or the + `push-msg` path, because KeyPath itself needs `KeyInput`, `ActOnFakeKey` and + `ChangeLayer`. Opt-in `KeyInput` only means the attacker has to ask. +- Does nothing for attackers 1, 2 or 4. +- Cost: small fork change (one to two days). + +Not enough on its own. Worth doing anyway as defence in depth. + +### B. Keep TCP, add a per-launch token + +The app generates a random token per worker launch, passes it to the worker, +and writes it to a 0600 file for the CLI. The server requires a valid +`Authenticate { token }` as the first message before anything else. + +- Stops 1, 2 and 3 from issuing commands or receiving events, if the server is + restructured so that a connection joins the broadcast set and gets its + initial `LayerChange` only after authenticating. Today it joins on accept. +- Does not stop squatting (4) unless the client also authenticates the server + (the server proves it knows the token). That is a challenge-response, not a + bearer token. +- Unauthenticated connections still cost a thread each until they time out. +- Token handling is new code with the usual hazards: constant-time compare, + never logged, rotated per launch, delivered to the CLI by file permissions, + the same protection a Unix socket gets for free. +- Cost: fork change of similar size to C, plus a token lifecycle in the app, + worker and CLI. + +### C. Move engine control to a Unix-domain socket with a peer check + +Replace the TCP listener with a Unix stream socket in a per-user directory, +using the pattern `RuntimeControlServer` already ships +(`docs/architecture/cli-runtime-control.md`): a 0700 directory checked to be a +real directory owned by this user, the socket created under umask 0177, and +every connection refused unless `getpeereid` returns the worker's uid. Release +builds start the worker with no TCP port at all. + +Then narrow who may connect: the only legitimate peer is the KeyPath app that +launched this worker. The worker already receives that app's PID (`ownerPID` in +`SessionRuntimeWorker`, recorded as `parentPID` in its report). The server checks `LOCAL_PEERPID` against +it, and can additionally check the peer's code signature through +`LOCAL_PEERTOKEN` and `SecCodeCreateWithAuditToken` against KeyPath's +designated requirement (team `X2RKZ5TG99`). The CLI stops talking to the engine +directly and asks the app over the existing runtime-control socket, as +`status/start/stop` already do. + +- Stops 1 and 2 outright: another uid fails `getpeereid` and directory + permissions, and a sandboxed app cannot reach the path. +- Stops 4: the path is inside a directory only this user owns, so nobody else + can pre-create it, and a stale socket is replaced the same way + `RuntimeControlServer` replaces one. +- Stops 3 from talking to the engine when the peer-PID or signature check is in + place. A uid check alone does not stop 3. +- Removes the cross-process trust problem for `MessagePush`: KeyPath only + dispatches actions it received from its own worker. +- Cost: see Migration below. The largest of the three, and the only one that + closes every case in the threat model. + +## Decision (proposed) + +Adopt **C**, and do **A** in the same fork change. + +The first public session-runtime release ships only after phases 1 to 3 below. +Phase 4 can follow in a point release. + +## Migration + +1. **Fork (`malpern/kanata`, a new branch merged into `keypath/bundled` by + fast-forward; never force-push it).** + - Make the server generic over a stream type and add a Unix listener + (`std::os::unix::net::UnixListener`), selected by the bridge. Keep the + TCP path for kanata's own binary and upstream users of the fork's code. + - Check the peer before the connection joins the broadcast set: uid via + `getpeereid`, and PID via `LOCAL_PEERPID` when the bridge supplies an + expected PID. + - Return an error instead of panicking when the listener cannot be created. + - Option A: drop `ReloadFile`, `ReloadNext/Prev/Num` and `SetMouse` from the + session build (feature flag), cap connections, and put a write timeout on + notification sockets so a stalled client is dropped rather than blocking + the loop. + - Rust tests for the peer check and the removed commands. + Estimate: roughly 300 to 500 lines with tests. +2. **Bridge and worker.** `keypath_kanata_bridge_create_passthru_runtime` takes a + socket path (and expected peer PID) instead of `tcp_port`. The worker creates + and validates the directory (reuse `RuntimeControlServer`'s checks) and + reports the socket path in its owned report instead of `tcpPort`. +3. **Swift clients.** One transport change: `KanataTCPClient+Connection` and + `KanataEventListener.connectAndStream` build `NWConnection`s from host and + port. They move to `NWEndpoint.unix(path:)` (confirm availability on the + minimum macOS during implementation; fall back to a POSIX socket if not). + The other callers listed above go through those two types and need no + protocol change. `ConfigFacade.tcpReload`/`tcpGetCurrentLayer` move to a + runtime-control request handled by the app. `PreferencesService.tcpServerPort` + and the legacy port migration retire. `verify-installed-app.sh` checks the + socket's owner and the connecting worker instead of a TCP listener. + Estimate: two to four days including tests. +4. **Signature check.** Add the `LOCAL_PEERTOKEN` code-signing check against + KeyPath's designated requirement. Debug builds signed with another identity + need an explicit, logged development override. +5. **Docs.** Mark ADR-013 superseded, rewrite the TCP section of + `guides/privacy.md` (it says the risk is limited to remapping keys; it also + omits keystroke events) and publish it, update + `docs/architecture/overview.md` and `cli-runtime-control.md`, and fix the + `KanataTCPClient` security note. + +**Compatibility:** anyone scripting `localhost:37001` directly (for example +from Hammerspoon) loses that. The supported routes are `keypath://` URLs and +`keypath-cli`. Call it out in release notes. + +**Fork divergence:** this widens the difference from upstream kanata's TCP +server. A Unix-socket listener is generally useful on Linux and macOS, so offer +it upstream; the peer-PID check stays KeyPath-specific. + +## Consequences + +- Until this lands, the session runtime exposes a keystroke stream and input + injection to every local process, including other users and sandboxed apps. + #1354 stays a release-blocker. The release process needs a check that fails + while open `release-blocker` issues exist; nothing enforces that today + (`release-doctor.sh` does not look at issues). +- The engine socket and the runtime-control socket share one directory model, + so a fix to either applies to both. +- KeyPath's `MessagePush` handling can then assume its sender. Until then, any + hardening of `ActionDispatcher` (for example requiring confirmation for + `script`, `open` with non-http schemes, and `runtime/stop` when they arrive + from the engine) reduces the impact of the `push-msg` path. + +## Related, not decided here + +The `keypath://` URL scheme is also an unauthenticated local entry point: any +process can `open keypath://script/…`, `keypath://fakekey/…` or +`keypath://runtime/stop`. Scripts are still gated by the user's script setting. +It deserves its own review, because moving the engine to a Unix socket does +not change it. diff --git a/docs/adr/index.md b/docs/adr/index.md index 50cc76fd8..76a3ca0c5 100644 --- a/docs/adr/index.md +++ b/docs/adr/index.md @@ -16,7 +16,7 @@ This section documents significant architectural decisions in KeyPath. Each ADR | [ADR-006](/adr/adr-006-apple-api-priority) | Apple API Priority in Permission Checks | Partly superseded by ADR-044 | | [ADR-008](/adr/adr-008-validation-refactor) | Stateless Validation via SystemValidator | Accepted | | [ADR-009](/adr/adr-009-service-extraction-mvvm) | Service Extraction & MVVM Pattern | Accepted | -| [ADR-013](/adr/adr-013-tcp-without-auth) | TCP Communication Without Authentication | Accepted (under review) | +| [ADR-013](/adr/adr-013-tcp-without-auth) | TCP Communication Without Authentication | Under review, see ADR-046 | | [ADR-014](/adr/adr-014-xpc-signature-mismatch) | XPC Signature Mismatch Prevention | Accepted (see ADR-045 note) | | [ADR-015](/adr/adr-015-installer-engine) | InstallerEngine Façade | Accepted (see ADR-045 note) | | [ADR-016](/adr/adr-016-tcc-database-reading) | TCC Database Reading for Sequential Permission Flow | Superseded by ADR-044 | @@ -49,6 +49,7 @@ This section documents significant architectural decisions in KeyPath. Each ADR | [ADR-043](/adr/adr-043-opportunistic-manager-consolidation) | Opportunistic Manager Consolidation | Accepted | | [ADR-044](/adr/adr-044-remove-full-disk-access) | Remove Full Disk Access and browser-history suggestions | Accepted | | [ADR-045](/adr/adr-045-retire-privileged-runtime-stack) | Retire the Privileged Runtime Stack | Accepted | +| [ADR-046](/adr/adr-046-session-engine-control-channel) | Session Engine Control Channel | Proposed | ## Key Decisions Summary From 96cf1b2f2ef42247287451f71123f613ff041841 Mon Sep 17 00:00:00 2001 From: Micah Alpern Date: Wed, 4 Nov 2026 20:18:12 -0800 Subject: [PATCH 02/30] Address review notes on ADR-046 Co-Authored-By: Claude Opus 5.5 --- .../Services/Networking/KanataTCPClient.swift | 3 +- docs/adr/adr-013-tcp-without-auth.md | 4 +-- .../adr-046-session-engine-control-channel.md | 34 +++++++++++++------ 3 files changed, 28 insertions(+), 13 deletions(-) diff --git a/Sources/KeyPathAppKit/Services/Networking/KanataTCPClient.swift b/Sources/KeyPathAppKit/Services/Networking/KanataTCPClient.swift index a0f838023..f53cdfc94 100644 --- a/Sources/KeyPathAppKit/Services/Networking/KanataTCPClient.swift +++ b/Sources/KeyPathAppKit/Services/Networking/KanataTCPClient.swift @@ -59,7 +59,8 @@ final class CompletionFlag: @unchecked Sendable { /// **SECURITY NOTE (ADR-013, under review in ADR-046 / #1354):** /// The engine's TCP server has no authentication, and loopback TCP is not /// scoped to a user or to the App Sandbox. Any local process, including other -/// users' processes and sandboxed apps, can connect to localhost:37001 and: +/// users' processes and sandboxed apps with the network-client entitlement, +/// can connect to the engine port (default 37001) and: /// - receive every `KeyInput` event (a keystroke stream) and the config text /// - press virtual keys (`ActOnFakeKey`), change layers, move the mouse /// - load any readable file as the config (`ReloadFile`) diff --git a/docs/adr/adr-013-tcp-without-auth.md b/docs/adr/adr-013-tcp-without-auth.md index 344fd68e8..9e212257b 100644 --- a/docs/adr/adr-013-tcp-without-auth.md +++ b/docs/adr/adr-013-tcp-without-auth.md @@ -22,12 +22,12 @@ Use TCP without authentication for localhost IPC. - Kanata 1.9.0 TCP server does not support authentication - Connection is localhost-only (127.0.0.1) -- Same security model as other local IPC (Unix sockets, etc.) +- Same security model as other local IPC (Unix sockets, etc.) *(Wrong: loopback TCP has no peer-uid check and is reachable from other users and sandboxed apps. See ADR-046.)* ## Security Considerations - Any local process can send commands to Kanata -- This is acceptable for keyboard remapping (low security impact) +- This is acceptable for keyboard remapping (low security impact) *(No longer true for the session runtime: the port exposes a keystroke stream and input injection. See ADR-046.)* - If Kanata adds auth in future versions, we should adopt it ## Implementation diff --git a/docs/adr/adr-046-session-engine-control-channel.md b/docs/adr/adr-046-session-engine-control-channel.md index 0baaf3bd7..34b41d50e 100644 --- a/docs/adr/adr-046-session-engine-control-channel.md +++ b/docs/adr/adr-046-session-engine-control-channel.md @@ -93,10 +93,16 @@ on its behalf. It can already do a lot as the user, but not read global keystrokes or post input without its own grants. This port gives it both. 4. **A process that binds `127.0.0.1:37001` before the worker does** (any user, - for example at login or during a restart). The worker's - `TcpListener::bind(...).expect` then panics, so the runtime fails to start, - and KeyPath's own clients connect to the squatter instead. KeyPath does not - check who owns the listener; only `verify-installed-app.sh` does. + for example at login or during a restart). The worker uses the bridge's + passthru entry point, which has no bind probe, so the fork's + `TcpListener::bind(...).expect` panics inside an `extern "C"` function and + aborts the worker. (`keypath_kanata_bridge_run_runtime` does probe and + return an error, but the probe drops its listener before the server binds, + so it is a check-then-use race.) KeyPath's own clients then connect to the + squatter. KeyPath does not check who owns the listener; only + `verify-installed-app.sh` does. The retired stack's + `detectKanataTCPPortConflict` matched on a process name containing + "kanata", which a squatter can avoid. Web pages are out of scope: browsers speak HTTP or WebSocket to that port, the first line is not valid JSON, and the server disconnects. @@ -205,13 +211,15 @@ Phase 4 can follow in a point release. - Check the peer before the connection joins the broadcast set: uid via `getpeereid`, and PID via `LOCAL_PEERPID` when the bridge supplies an expected PID. - - Return an error instead of panicking when the listener cannot be created. + - Return an error instead of panicking when the listener cannot be created, + by binding once and keeping that listener, not by probing first. - Option A: drop `ReloadFile`, `ReloadNext/Prev/Num` and `SetMouse` from the session build (feature flag), cap connections, and put a write timeout on notification sockets so a stalled client is dropped rather than blocking the loop. - Rust tests for the peer check and the removed commands. - Estimate: roughly 300 to 500 lines with tests. + Estimate: roughly 300 to 500 lines of new code and tests, plus rewriting + the bridge's existing passthru TCP test (`Rust/KeyPathKanataHostBridge/src/lib.rs`). 2. **Bridge and worker.** `keypath_kanata_bridge_create_passthru_runtime` takes a socket path (and expected peer PID) instead of `tcp_port`. The worker creates and validates the directory (reuse `RuntimeControlServer`'s checks) and @@ -230,13 +238,19 @@ Phase 4 can follow in a point release. KeyPath's designated requirement. Debug builds signed with another identity need an explicit, logged development override. 5. **Docs.** Mark ADR-013 superseded, rewrite the TCP section of - `guides/privacy.md` (it says the risk is limited to remapping keys; it also - omits keystroke events) and publish it, update + `guides/privacy.md` (it says key events cross the connection, but not that + any other local process receives them, and calls the risk low) and publish it, update `docs/architecture/overview.md` and `cli-runtime-control.md`, and fix the - `KanataTCPClient` security note. + `KanataTCPClient` security note. Withdraw or redesign + `docs/architecture/context-integration-architecture.md`, whose unimplemented + design has tmux, vim and shell hooks send `ContextUpdate` JSON to + `nc 127.0.0.1 37001`. **Compatibility:** anyone scripting `localhost:37001` directly (for example -from Hammerspoon) loses that. The supported routes are `keypath://` URLs and +from Hammerspoon) loses that. So does the context-integration design above: +third-party processes feeding context to the engine is exactly what the peer +check forbids, so if it is revived it goes through the app (a `keypath://` URL +or runtime-control request), which can validate what it forwards. The supported routes are `keypath://` URLs and `keypath-cli`. Call it out in release notes. **Fork divergence:** this widens the difference from upstream kanata's TCP From 18d842644c074c69fa2e5e86965ce90037cc878b Mon Sep 17 00:00:00 2001 From: Micah Alpern Date: Wed, 4 Nov 2026 20:19:43 -0800 Subject: [PATCH 03/30] ADR-046: state the PID check's limits and require live confirmation Co-Authored-By: Claude Opus 5.5 --- docs/adr/adr-046-session-engine-control-channel.md | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/docs/adr/adr-046-session-engine-control-channel.md b/docs/adr/adr-046-session-engine-control-channel.md index 34b41d50e..f0fc8267e 100644 --- a/docs/adr/adr-046-session-engine-control-channel.md +++ b/docs/adr/adr-046-session-engine-control-channel.md @@ -188,7 +188,12 @@ directly and asks the app over the existing runtime-control socket, as can pre-create it, and a stale socket is replaced the same way `RuntimeControlServer` replaces one. - Stops 3 from talking to the engine when the peer-PID or signature check is in - place. A uid check alone does not stop 3. + place. A uid check alone does not stop 3. The PID check is weaker than the + signature check: it holds only while the launching app is alive (its PID + cannot be reused until then, so the worker must exit when the app does), and + it trusts whatever code runs in that PID. Until phase 4 ships, attacker 3 is + stopped from connecting directly but not from injecting into or impersonating + the app, which this ADR treats as out of scope. - Removes the cross-process trust problem for `MessagePush`: KeyPath only dispatches actions it received from its own worker. - Cost: see Migration below. The largest of the three, and the only one that @@ -201,6 +206,11 @@ Adopt **C**, and do **A** in the same fork change. The first public session-runtime release ships only after phases 1 to 3 below. Phase 4 can follow in a point release. +Before this ADR is accepted, confirm the main claims on a live install in a +lab VM: a second macOS user, and a sandboxed app with the network-client +entitlement, each connect to the port and receive `KeyInput`. This ADR was +written from source alone. + ## Migration 1. **Fork (`malpern/kanata`, a new branch merged into `keypath/bundled` by From 339131cfda34e4dc1712ca5324a971a605b5c5f1 Mon Sep 17 00:00:00 2001 From: Micah Alpern Date: Wed, 4 Nov 2026 20:21:07 -0800 Subject: [PATCH 04/30] ADR-046: spell out owner-exit detection and release-time coverage Co-Authored-By: Claude Opus 5.5 --- docs/adr/README.md | 2 +- docs/adr/adr-046-session-engine-control-channel.md | 13 ++++++++++--- docs/adr/index.md | 2 +- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/docs/adr/README.md b/docs/adr/README.md index 55e8303d4..49de2548c 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -10,7 +10,7 @@ This directory contains Architecture Decision Records (ADRs) documenting signifi | [ADR-006](adr-006-apple-api-priority.md) | Apple API Priority in Permission Checks | Partly superseded by ADR-044 | | [ADR-008](adr-008-validation-refactor.md) | Stateless Validation via SystemValidator | Accepted | | [ADR-009](adr-009-service-extraction-mvvm.md) | Service Extraction & MVVM Pattern | Accepted | -| [ADR-013](adr-013-tcp-without-auth.md) | TCP Communication Without Authentication | Under review, see ADR-046 | +| [ADR-013](adr-013-tcp-without-auth.md) | TCP Communication Without Authentication | Under review; proposed to be superseded by ADR-046 | | [ADR-014](adr-014-xpc-signature-mismatch.md) | XPC Signature Mismatch Prevention | Accepted (see ADR-045 note) | | [ADR-015](adr-015-installer-engine.md) | InstallerEngine Façade | Accepted (see ADR-045 note) | | [ADR-016](adr-016-tcc-database-reading.md) | TCC Database Reading for Sequential Permission Flow | Superseded by ADR-044 | diff --git a/docs/adr/adr-046-session-engine-control-channel.md b/docs/adr/adr-046-session-engine-control-channel.md index f0fc8267e..183b01b22 100644 --- a/docs/adr/adr-046-session-engine-control-channel.md +++ b/docs/adr/adr-046-session-engine-control-channel.md @@ -191,13 +191,20 @@ directly and asks the app over the existing runtime-control socket, as place. A uid check alone does not stop 3. The PID check is weaker than the signature check: it holds only while the launching app is alive (its PID cannot be reused until then, so the worker must exit when the app does), and - it trusts whatever code runs in that PID. Until phase 4 ships, attacker 3 is + it trusts whatever code runs in that PID. Today the worker notices the app + has gone by polling `isProcessAlive(ownerPID)` in its 5 ms tick + (`SessionRuntimeWorker.tick`), and a dead PID can be reused inside that + window. Phase 1 therefore has the server re-check the owner's liveness on + every accept, and the worker watch the owner with a kqueue `NOTE_EXIT` + source so it stops listening the moment the app exits. Until phase 4 ships, attacker 3 is stopped from connecting directly but not from injecting into or impersonating the app, which this ADR treats as out of scope. - Removes the cross-process trust problem for `MessagePush`: KeyPath only dispatches actions it received from its own worker. -- Cost: see Migration below. The largest of the three, and the only one that - closes every case in the threat model. +- Cost: see Migration below. The largest of the three. With phase 4 it is the + only option that closes every case in the threat model. At release (phases 1 + to 3) it closes 1, 2 and 4 and stops direct connections from 3, leaving + injection into or impersonation of the app. ## Decision (proposed) diff --git a/docs/adr/index.md b/docs/adr/index.md index 76a3ca0c5..74f2f4570 100644 --- a/docs/adr/index.md +++ b/docs/adr/index.md @@ -16,7 +16,7 @@ This section documents significant architectural decisions in KeyPath. Each ADR | [ADR-006](/adr/adr-006-apple-api-priority) | Apple API Priority in Permission Checks | Partly superseded by ADR-044 | | [ADR-008](/adr/adr-008-validation-refactor) | Stateless Validation via SystemValidator | Accepted | | [ADR-009](/adr/adr-009-service-extraction-mvvm) | Service Extraction & MVVM Pattern | Accepted | -| [ADR-013](/adr/adr-013-tcp-without-auth) | TCP Communication Without Authentication | Under review, see ADR-046 | +| [ADR-013](/adr/adr-013-tcp-without-auth) | TCP Communication Without Authentication | Under review; proposed to be superseded by ADR-046 | | [ADR-014](/adr/adr-014-xpc-signature-mismatch) | XPC Signature Mismatch Prevention | Accepted (see ADR-045 note) | | [ADR-015](/adr/adr-015-installer-engine) | InstallerEngine Façade | Accepted (see ADR-045 note) | | [ADR-016](/adr/adr-016-tcc-database-reading) | TCC Database Reading for Sequential Permission Flow | Superseded by ADR-044 | From 56a172a2f44ea16af11552f4938c12887e175e41 Mon Sep 17 00:00:00 2001 From: Micah Alpern Date: Wed, 4 Nov 2026 20:23:59 -0800 Subject: [PATCH 05/30] ADR-046: add an attacker coverage table and phase 1 acceptance criterion Co-Authored-By: Claude Opus 5.5 --- docs/adr/adr-046-session-engine-control-channel.md | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/docs/adr/adr-046-session-engine-control-channel.md b/docs/adr/adr-046-session-engine-control-channel.md index 183b01b22..cf797441d 100644 --- a/docs/adr/adr-046-session-engine-control-channel.md +++ b/docs/adr/adr-046-session-engine-control-channel.md @@ -213,6 +213,13 @@ Adopt **C**, and do **A** in the same fork change. The first public session-runtime release ships only after phases 1 to 3 below. Phase 4 can follow in a point release. +| Attacker | Today | After phases 1–3 (release) | After phase 4 | +|---|---|---|---| +| 1. Another macOS user | Full access | Blocked (uid check, 0700 directory) | Blocked | +| 2. Sandboxed app | Full access | Blocked (cannot reach the path) | Blocked | +| 3. Same user, unsandboxed | Full access | Direct connection blocked (PID check); injecting into or impersonating the app still possible | Blocked unless it injects code into KeyPath (out of scope) | +| 4. Port squatter | Takes over the channel | Blocked (no TCP port; socket in a directory only this user owns) | Blocked | + Before this ADR is accepted, confirm the main claims on a live install in a lab VM: a second macOS user, and a sandboxed app with the network-client entitlement, each connect to the port and receive `KeyInput`. This ADR was @@ -228,6 +235,10 @@ written from source alone. - Check the peer before the connection joins the broadcast set: uid via `getpeereid`, and PID via `LOCAL_PEERPID` when the bridge supplies an expected PID. + - **Acceptance criterion:** the worker watches its owner with a kqueue + `NOTE_EXIT` source and stops listening as soon as the app exits, and the + server re-checks the owner on every accept. Without both, the PID check is + not meaningful. - Return an error instead of panicking when the listener cannot be created, by binding once and keeping that listener, not by probing first. - Option A: drop `ReloadFile`, `ReloadNext/Prev/Num` and `SetMouse` from the @@ -268,7 +279,8 @@ from Hammerspoon) loses that. So does the context-integration design above: third-party processes feeding context to the engine is exactly what the peer check forbids, so if it is revived it goes through the app (a `keypath://` URL or runtime-control request), which can validate what it forwards. The supported routes are `keypath://` URLs and -`keypath-cli`. Call it out in release notes. +`keypath-cli`. Phase 5 adds a release-notes entry for the release that ships +this, and an item in `docs/process/release-process.md` to check it is there. **Fork divergence:** this widens the difference from upstream kanata's TCP server. A Unix-socket listener is generally useful on Linux and macOS, so offer From 0f1930b30a9b0ae199104b91e27893ed78c7446b Mon Sep 17 00:00:00 2001 From: Micah Alpern Date: Wed, 4 Nov 2026 20:25:37 -0800 Subject: [PATCH 06/30] ADR-046: clarify the peer check, and qualify the source-only claims Co-Authored-By: Claude Opus 5.5 --- .../KeyPathAppKit/Services/Networking/KanataTCPClient.swift | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Sources/KeyPathAppKit/Services/Networking/KanataTCPClient.swift b/Sources/KeyPathAppKit/Services/Networking/KanataTCPClient.swift index f53cdfc94..4e9b8d0a6 100644 --- a/Sources/KeyPathAppKit/Services/Networking/KanataTCPClient.swift +++ b/Sources/KeyPathAppKit/Services/Networking/KanataTCPClient.swift @@ -58,7 +58,8 @@ final class CompletionFlag: @unchecked Sendable { /// /// **SECURITY NOTE (ADR-013, under review in ADR-046 / #1354):** /// The engine's TCP server has no authentication, and loopback TCP is not -/// scoped to a user or to the App Sandbox. Any local process, including other +/// scoped to a user or to the App Sandbox. From reading the source (not yet +/// confirmed on a live install), any local process, including other /// users' processes and sandboxed apps with the network-client entitlement, /// can connect to the engine port (default 37001) and: /// - receive every `KeyInput` event (a keystroke stream) and the config text From 51d59422227656f55cc9c03415bcd0296f0d3bc8 Mon Sep 17 00:00:00 2001 From: Micah Alpern Date: Wed, 4 Nov 2026 20:25:56 -0800 Subject: [PATCH 07/30] ADR-046: require the peer check on the accepted socket; align ADR-013 wording Co-Authored-By: Claude Opus 5.5 --- docs/adr/adr-013-tcp-without-auth.md | 10 +++++----- docs/adr/adr-046-session-engine-control-channel.md | 3 +++ 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/docs/adr/adr-013-tcp-without-auth.md b/docs/adr/adr-013-tcp-without-auth.md index 9e212257b..53da0d56a 100644 --- a/docs/adr/adr-013-tcp-without-auth.md +++ b/docs/adr/adr-013-tcp-without-auth.md @@ -1,13 +1,13 @@ # ADR-013: TCP Communication Without Authentication -> **Note (2026-11-04):** This was written for the root daemon. The session -> runtime holds the user's input permissions, and the port now exposes a -> keystroke stream and input injection to every local process. The review and -> the proposed replacement are in +> **Note (2026-11-04):** This was written for the root daemon, which ADR-045 +> retired. The session runtime holds the user's input permissions, and from a +> source review the port exposes a keystroke stream and input injection to +> every local process. The review and the proposed replacement are in > [ADR-046](adr-046-session-engine-control-channel.md); tracked in > [#1354](https://github.com/malpern/KeyPath/issues/1354). -**Status:** Under review; to be superseded by [ADR-046](adr-046-session-engine-control-channel.md) (Proposed) +**Status:** Under review; proposed to be superseded by [ADR-046](adr-046-session-engine-control-channel.md) (Proposed) **Date:** 2024 ## Context diff --git a/docs/adr/adr-046-session-engine-control-channel.md b/docs/adr/adr-046-session-engine-control-channel.md index cf797441d..b85477eb5 100644 --- a/docs/adr/adr-046-session-engine-control-channel.md +++ b/docs/adr/adr-046-session-engine-control-channel.md @@ -235,6 +235,9 @@ written from source alone. - Check the peer before the connection joins the broadcast set: uid via `getpeereid`, and PID via `LOCAL_PEERPID` when the bridge supplies an expected PID. + Both are read from the accepted socket itself, never from a later + lookup, and the PID must equal the launching app's PID, not merely belong + to a process of this user. - **Acceptance criterion:** the worker watches its owner with a kqueue `NOTE_EXIT` source and stops listening as soon as the app exits, and the server re-checks the owner on every accept. Without both, the PID check is From f02e9520368619fd6fcdef4bec52845756d32b6c Mon Sep 17 00:00:00 2001 From: Micah Alpern Date: Wed, 4 Nov 2026 20:26:44 -0800 Subject: [PATCH 08/30] ADR-046: label migration steps as phases; require an owner-exit test Co-Authored-By: Claude Opus 5.5 --- docs/adr/adr-046-session-engine-control-channel.md | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/docs/adr/adr-046-session-engine-control-channel.md b/docs/adr/adr-046-session-engine-control-channel.md index b85477eb5..793ab3282 100644 --- a/docs/adr/adr-046-session-engine-control-channel.md +++ b/docs/adr/adr-046-session-engine-control-channel.md @@ -227,7 +227,7 @@ written from source alone. ## Migration -1. **Fork (`malpern/kanata`, a new branch merged into `keypath/bundled` by +1. **Phase 1, fork (`malpern/kanata`, a new branch merged into `keypath/bundled` by fast-forward; never force-push it).** - Make the server generic over a stream type and add a Unix listener (`std::os::unix::net::UnixListener`), selected by the bridge. Keep the @@ -241,7 +241,8 @@ written from source alone. - **Acceptance criterion:** the worker watches its owner with a kqueue `NOTE_EXIT` source and stops listening as soon as the app exits, and the server re-checks the owner on every accept. Without both, the PID check is - not meaningful. + not meaningful. Required test: start the worker, kill the owner app, + and assert the socket refuses new connections and the worker exits. - Return an error instead of panicking when the listener cannot be created, by binding once and keeping that listener, not by probing first. - Option A: drop `ReloadFile`, `ReloadNext/Prev/Num` and `SetMouse` from the @@ -251,11 +252,11 @@ written from source alone. - Rust tests for the peer check and the removed commands. Estimate: roughly 300 to 500 lines of new code and tests, plus rewriting the bridge's existing passthru TCP test (`Rust/KeyPathKanataHostBridge/src/lib.rs`). -2. **Bridge and worker.** `keypath_kanata_bridge_create_passthru_runtime` takes a +2. **Phase 2, bridge and worker.** `keypath_kanata_bridge_create_passthru_runtime` takes a socket path (and expected peer PID) instead of `tcp_port`. The worker creates and validates the directory (reuse `RuntimeControlServer`'s checks) and reports the socket path in its owned report instead of `tcpPort`. -3. **Swift clients.** One transport change: `KanataTCPClient+Connection` and +3. **Phase 3, Swift clients.** One transport change: `KanataTCPClient+Connection` and `KanataEventListener.connectAndStream` build `NWConnection`s from host and port. They move to `NWEndpoint.unix(path:)` (confirm availability on the minimum macOS during implementation; fall back to a POSIX socket if not). @@ -265,10 +266,10 @@ written from source alone. and the legacy port migration retire. `verify-installed-app.sh` checks the socket's owner and the connecting worker instead of a TCP listener. Estimate: two to four days including tests. -4. **Signature check.** Add the `LOCAL_PEERTOKEN` code-signing check against +4. **Phase 4, signature check.** Add the `LOCAL_PEERTOKEN` code-signing check against KeyPath's designated requirement. Debug builds signed with another identity need an explicit, logged development override. -5. **Docs.** Mark ADR-013 superseded, rewrite the TCP section of +5. **Phase 5, docs.** Mark ADR-013 superseded, rewrite the TCP section of `guides/privacy.md` (it says key events cross the connection, but not that any other local process receives them, and calls the risk low) and publish it, update `docs/architecture/overview.md` and `cli-runtime-control.md`, and fix the From dd78843fef6b1344163ba5af67c73c2a11efd701 Mon Sep 17 00:00:00 2001 From: Micah Alpern Date: Wed, 4 Nov 2026 20:27:45 -0800 Subject: [PATCH 09/30] ADR-013: drop redundant status suffix Co-Authored-By: Claude Opus 5.5 --- docs/adr/adr-013-tcp-without-auth.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/adr/adr-013-tcp-without-auth.md b/docs/adr/adr-013-tcp-without-auth.md index 53da0d56a..45e7d3a2b 100644 --- a/docs/adr/adr-013-tcp-without-auth.md +++ b/docs/adr/adr-013-tcp-without-auth.md @@ -7,7 +7,7 @@ > [ADR-046](adr-046-session-engine-control-channel.md); tracked in > [#1354](https://github.com/malpern/KeyPath/issues/1354). -**Status:** Under review; proposed to be superseded by [ADR-046](adr-046-session-engine-control-channel.md) (Proposed) +**Status:** Under review; proposed to be superseded by [ADR-046](adr-046-session-engine-control-channel.md) **Date:** 2024 ## Context From c4f177dbdfd7f1526d7b7eaed2658870b7db925c Mon Sep 17 00:00:00 2001 From: Micah Alpern Date: Wed, 4 Nov 2026 20:28:51 -0800 Subject: [PATCH 10/30] ADR-046: pass the owner PID per launch; note the sun_path limit Co-Authored-By: Claude Opus 5.5 --- docs/adr/adr-046-session-engine-control-channel.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/docs/adr/adr-046-session-engine-control-channel.md b/docs/adr/adr-046-session-engine-control-channel.md index 793ab3282..995026a9b 100644 --- a/docs/adr/adr-046-session-engine-control-channel.md +++ b/docs/adr/adr-046-session-engine-control-channel.md @@ -253,7 +253,12 @@ written from source alone. Estimate: roughly 300 to 500 lines of new code and tests, plus rewriting the bridge's existing passthru TCP test (`Rust/KeyPathKanataHostBridge/src/lib.rs`). 2. **Phase 2, bridge and worker.** `keypath_kanata_bridge_create_passthru_runtime` takes a - socket path (and expected peer PID) instead of `tcp_port`. The worker creates + socket path (and expected peer PID) instead of `tcp_port`. The expected PID + is the app that launched this worker, passed on every launch, so a relaunch + or a new app instance never inherits an old PID. Keep the path short: + macOS limits `sun_path` to 104 bytes, so reuse the runtime-control directory + (`~/Library/Application Support/KeyPath/control/`) with a short file name and + fail with a clear error, never truncate, if a long home path exceeds it. The worker creates and validates the directory (reuse `RuntimeControlServer`'s checks) and reports the socket path in its owned report instead of `tcpPort`. 3. **Phase 3, Swift clients.** One transport change: `KanataTCPClient+Connection` and From 9c13d2579f3a7e0887f43bbe786d71723585b49e Mon Sep 17 00:00:00 2001 From: Micah Alpern Date: Wed, 4 Nov 2026 20:29:27 -0800 Subject: [PATCH 11/30] ADR-046: worker exits with its owner; mark attacker 3 as partly mitigated Co-Authored-By: Claude Opus 5.5 --- docs/adr/adr-046-session-engine-control-channel.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/adr/adr-046-session-engine-control-channel.md b/docs/adr/adr-046-session-engine-control-channel.md index 995026a9b..02231e3ce 100644 --- a/docs/adr/adr-046-session-engine-control-channel.md +++ b/docs/adr/adr-046-session-engine-control-channel.md @@ -217,7 +217,7 @@ Phase 4 can follow in a point release. |---|---|---|---| | 1. Another macOS user | Full access | Blocked (uid check, 0700 directory) | Blocked | | 2. Sandboxed app | Full access | Blocked (cannot reach the path) | Blocked | -| 3. Same user, unsandboxed | Full access | Direct connection blocked (PID check); injecting into or impersonating the app still possible | Blocked unless it injects code into KeyPath (out of scope) | +| 3. Same user, unsandboxed | Full access | Partly mitigated: direct connection blocked (PID check), but injecting into or impersonating the app is still possible | Blocked unless it injects code into KeyPath (out of scope) | | 4. Port squatter | Takes over the channel | Blocked (no TCP port; socket in a directory only this user owns) | Blocked | Before this ADR is accepted, confirm the main claims on a live install in a @@ -239,7 +239,7 @@ written from source alone. lookup, and the PID must equal the launching app's PID, not merely belong to a process of this user. - **Acceptance criterion:** the worker watches its owner with a kqueue - `NOTE_EXIT` source and stops listening as soon as the app exits, and the + `NOTE_EXIT` source and, when the app exits, stops listening and exits, and the server re-checks the owner on every accept. Without both, the PID check is not meaningful. Required test: start the worker, kill the owner app, and assert the socket refuses new connections and the worker exits. From 606fc5fb8ca0cb3b994dac815200f8bd8f94dda7 Mon Sep 17 00:00:00 2001 From: Micah Alpern Date: Wed, 4 Nov 2026 20:30:41 -0800 Subject: [PATCH 12/30] ADR-046: give long home paths a socket-directory fallback Co-Authored-By: Claude Opus 5.5 --- docs/adr/adr-046-session-engine-control-channel.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/docs/adr/adr-046-session-engine-control-channel.md b/docs/adr/adr-046-session-engine-control-channel.md index 02231e3ce..332a76748 100644 --- a/docs/adr/adr-046-session-engine-control-channel.md +++ b/docs/adr/adr-046-session-engine-control-channel.md @@ -258,7 +258,11 @@ written from source alone. or a new app instance never inherits an old PID. Keep the path short: macOS limits `sun_path` to 104 bytes, so reuse the runtime-control directory (`~/Library/Application Support/KeyPath/control/`) with a short file name and - fail with a clear error, never truncate, if a long home path exceeds it. The worker creates + never truncate. If a long home path exceeds it, fall back to a short + directory under the per-user temporary directory + (`confstr(_CS_DARWIN_USER_TEMP_DIR)`, which macOS creates 0700 for this + user), with the same ownership and mode checks. Fail with a clear error only + if both are too long. The worker creates and validates the directory (reuse `RuntimeControlServer`'s checks) and reports the socket path in its owned report instead of `tcpPort`. 3. **Phase 3, Swift clients.** One transport change: `KanataTCPClient+Connection` and From 04cbc6032fdb1728e648cee0ed24f210a562e861 Mon Sep 17 00:00:00 2001 From: Micah Alpern Date: Wed, 4 Nov 2026 20:31:48 -0800 Subject: [PATCH 13/30] ADR-046: note the PID check's fork/exec limit Co-Authored-By: Claude Opus 5.5 --- docs/adr/adr-046-session-engine-control-channel.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/adr/adr-046-session-engine-control-channel.md b/docs/adr/adr-046-session-engine-control-channel.md index 332a76748..ec9d539fd 100644 --- a/docs/adr/adr-046-session-engine-control-channel.md +++ b/docs/adr/adr-046-session-engine-control-channel.md @@ -191,7 +191,10 @@ directly and asks the app over the existing runtime-control socket, as place. A uid check alone does not stop 3. The PID check is weaker than the signature check: it holds only while the launching app is alive (its PID cannot be reused until then, so the worker must exit when the app does), and - it trusts whatever code runs in that PID. Today the worker notices the app + it trusts whatever code runs in that PID. `LOCAL_PEERPID` also reports the + process that connected, so a connection the app opens and then hands to a + forked or exec'd child still passes. The phase 4 signature check is the real + fix for both. Today the worker notices the app has gone by polling `isProcessAlive(ownerPID)` in its 5 ms tick (`SessionRuntimeWorker.tick`), and a dead PID can be reused inside that window. Phase 1 therefore has the server re-check the owner's liveness on From 99826ea451046b04d01d8da530ca8de8db2ace52 Mon Sep 17 00:00:00 2001 From: Micah Alpern Date: Wed, 4 Nov 2026 20:33:03 -0800 Subject: [PATCH 14/30] ADR-046: state that worker adoption stays within one app process Co-Authored-By: Claude Opus 5.5 --- docs/adr/adr-046-session-engine-control-channel.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/docs/adr/adr-046-session-engine-control-channel.md b/docs/adr/adr-046-session-engine-control-channel.md index ec9d539fd..c211c5028 100644 --- a/docs/adr/adr-046-session-engine-control-channel.md +++ b/docs/adr/adr-046-session-engine-control-channel.md @@ -258,7 +258,12 @@ written from source alone. 2. **Phase 2, bridge and worker.** `keypath_kanata_bridge_create_passthru_runtime` takes a socket path (and expected peer PID) instead of `tcp_port`. The expected PID is the app that launched this worker, passed on every launch, so a relaunch - or a new app instance never inherits an old PID. Keep the path short: + or a new app instance never inherits an old PID. This keeps today's + adoption rule: `adoptRunningSession` only re-supervises a worker launched by + the same app process (`currentSessionReport` needs that process's + `sessionApplication` and nonce), and a worker whose owner has exited already + stops itself (`owner-exited`). Nothing adopts a worker across app launches, + and the owner watch makes that impossible by design. Keep the path short: macOS limits `sun_path` to 104 bytes, so reuse the runtime-control directory (`~/Library/Application Support/KeyPath/control/`) with a short file name and never truncate. If a long home path exceeds it, fall back to a short From 6da68c01944726278976f5c9d932700e8aaab8d1 Mon Sep 17 00:00:00 2001 From: Micah Alpern Date: Wed, 4 Nov 2026 20:34:42 -0800 Subject: [PATCH 15/30] ADR-046: land phases 1 and 2 together; test the owner-exit window Co-Authored-By: Claude Opus 5.5 --- docs/adr/adr-046-session-engine-control-channel.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/docs/adr/adr-046-session-engine-control-channel.md b/docs/adr/adr-046-session-engine-control-channel.md index c211c5028..0303123d5 100644 --- a/docs/adr/adr-046-session-engine-control-channel.md +++ b/docs/adr/adr-046-session-engine-control-channel.md @@ -245,7 +245,9 @@ written from source alone. `NOTE_EXIT` source and, when the app exits, stops listening and exits, and the server re-checks the owner on every accept. Without both, the PID check is not meaningful. Required test: start the worker, kill the owner app, - and assert the socket refuses new connections and the worker exits. + and assert the socket refuses new connections and the worker exits. A + second test connects after the owner has exited but before the exit + watch fires, and asserts the per-accept re-check refuses it. - Return an error instead of panicking when the listener cannot be created, by binding once and keeping that listener, not by probing first. - Option A: drop `ReloadFile`, `ReloadNext/Prev/Num` and `SetMouse` from the @@ -255,6 +257,9 @@ written from source alone. - Rust tests for the peer check and the removed commands. Estimate: roughly 300 to 500 lines of new code and tests, plus rewriting the bridge's existing passthru TCP test (`Rust/KeyPathKanataHostBridge/src/lib.rs`). + Phases 1 and 2 land together: the new listener, the removal of the bind + probe and the bridge API change ship in one fork update, so no build has + the Unix socket while still panicking on, or probing, a TCP bind. 2. **Phase 2, bridge and worker.** `keypath_kanata_bridge_create_passthru_runtime` takes a socket path (and expected peer PID) instead of `tcp_port`. The expected PID is the app that launched this worker, passed on every launch, so a relaunch From 0d45d0952651ee94d7d6423f4fdea35749c5eee5 Mon Sep 17 00:00:00 2001 From: Micah Alpern Date: Wed, 4 Nov 2026 20:35:37 -0800 Subject: [PATCH 16/30] ADR-046: record the inherited-connection gap in phase 1 criteria Co-Authored-By: Claude Opus 5.5 --- docs/adr/adr-046-session-engine-control-channel.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/adr/adr-046-session-engine-control-channel.md b/docs/adr/adr-046-session-engine-control-channel.md index 0303123d5..5233575e4 100644 --- a/docs/adr/adr-046-session-engine-control-channel.md +++ b/docs/adr/adr-046-session-engine-control-channel.md @@ -247,7 +247,9 @@ written from source alone. not meaningful. Required test: start the worker, kill the owner app, and assert the socket refuses new connections and the worker exits. A second test connects after the owner has exited but before the exit - watch fires, and asserts the per-accept re-check refuses it. + watch fires, and asserts the per-accept re-check refuses it. A + connection the app hands to a child process is a known gap at this phase + (see Option C); phase 4 closes it and adds its test. - Return an error instead of panicking when the listener cannot be created, by binding once and keeping that listener, not by probing first. - Option A: drop `ReloadFile`, `ReloadNext/Prev/Num` and `SetMouse` from the From 05af8e59f35f593c0f8249ee65f03f5e825083c1 Mon Sep 17 00:00:00 2001 From: Micah Alpern Date: Thu, 5 Nov 2026 21:32:15 -0800 Subject: [PATCH 17/30] ADR-046: name the fork branch that holds the pinned commit Co-Authored-By: Claude Opus 5.5 --- docs/adr/adr-046-session-engine-control-channel.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/adr/adr-046-session-engine-control-channel.md b/docs/adr/adr-046-session-engine-control-channel.md index 5233575e4..445de3cdc 100644 --- a/docs/adr/adr-046-session-engine-control-channel.md +++ b/docs/adr/adr-046-session-engine-control-channel.md @@ -25,8 +25,10 @@ processes of other logged-in users and sandboxed apps that hold kind. Every accepted connection joins the broadcast set before it has sent a byte. -Facts below were read from the fork at `External/kanata` (`keypath/bundled`, -`d9085a9`) and from KeyPath `master` at `e42ffea0`. They were not exercised on a +Facts below were read from the fork at `External/kanata`, at `d9085a9`, the +commit KeyPath `master` (`e42ffea0`) pins, and from KeyPath at that commit. +`d9085a9` is on the fork's `keypath/explicit-source-keys-20261017` branch, not +yet on `keypath/bundled`. They were not exercised on a live install for this ADR. ### What a connected client can do From 0e29a54b7d21a8fc466fb54e38c6b2288dca8632 Mon Sep 17 00:00:00 2001 From: Micah Alpern Date: Thu, 5 Nov 2026 21:34:57 -0800 Subject: [PATCH 18/30] ADR-046: match the owner's PID version, not just its liveness A liveness re-check cannot tell a reused PID from the owner. Compare the peer's audit-token PID and PID version against the owner's instead. Co-Authored-By: Claude Opus 5.5 --- .../adr-046-session-engine-control-channel.md | 25 ++++++++++++------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/docs/adr/adr-046-session-engine-control-channel.md b/docs/adr/adr-046-session-engine-control-channel.md index 445de3cdc..2003420e1 100644 --- a/docs/adr/adr-046-session-engine-control-channel.md +++ b/docs/adr/adr-046-session-engine-control-channel.md @@ -177,7 +177,7 @@ builds start the worker with no TCP port at all. Then narrow who may connect: the only legitimate peer is the KeyPath app that launched this worker. The worker already receives that app's PID (`ownerPID` in -`SessionRuntimeWorker`, recorded as `parentPID` in its report). The server checks `LOCAL_PEERPID` against +`SessionRuntimeWorker`, recorded as `parentPID` in its report). The server checks the peer's PID and PID version (from `LOCAL_PEERTOKEN`) against it, and can additionally check the peer's code signature through `LOCAL_PEERTOKEN` and `SecCodeCreateWithAuditToken` against KeyPath's designated requirement (team `X2RKZ5TG99`). The CLI stops talking to the engine @@ -193,15 +193,20 @@ directly and asks the app over the existing runtime-control socket, as place. A uid check alone does not stop 3. The PID check is weaker than the signature check: it holds only while the launching app is alive (its PID cannot be reused until then, so the worker must exit when the app does), and - it trusts whatever code runs in that PID. `LOCAL_PEERPID` also reports the + it trusts whatever code runs in that PID. The peer token also identifies the process that connected, so a connection the app opens and then hands to a forked or exec'd child still passes. The phase 4 signature check is the real fix for both. Today the worker notices the app has gone by polling `isProcessAlive(ownerPID)` in its 5 ms tick (`SessionRuntimeWorker.tick`), and a dead PID can be reused inside that - window. Phase 1 therefore has the server re-check the owner's liveness on - every accept, and the worker watch the owner with a kqueue `NOTE_EXIT` - source so it stops listening the moment the app exits. Until phase 4 ships, attacker 3 is + window. A liveness re-check on accept cannot tell a reused PID from the + owner, so phase 1 compares identity, not liveness: the app passes its PID + and its PID version (`audit_token_to_pidversion` on its own + `TASK_AUDIT_TOKEN`) at launch, and the server reads the peer's audit token + from the accepted socket (`LOCAL_PEERTOKEN`) and requires both to match. The + PID version changes when a PID is reused, which closes the race. The worker + also watches the owner with a kqueue `NOTE_EXIT` source so it stops + listening the moment the app exits. Until phase 4 ships, attacker 3 is stopped from connecting directly but not from injecting into or impersonating the app, which this ADR treats as out of scope. - Removes the cross-process trust problem for `MessagePush`: KeyPath only @@ -238,18 +243,20 @@ written from source alone. (`std::os::unix::net::UnixListener`), selected by the bridge. Keep the TCP path for kanata's own binary and upstream users of the fork's code. - Check the peer before the connection joins the broadcast set: uid via - `getpeereid`, and PID via `LOCAL_PEERPID` when the bridge supplies an + `getpeereid`, and PID plus PID version via `LOCAL_PEERTOKEN` when the bridge supplies an expected PID. Both are read from the accepted socket itself, never from a later lookup, and the PID must equal the launching app's PID, not merely belong to a process of this user. - **Acceptance criterion:** the worker watches its owner with a kqueue `NOTE_EXIT` source and, when the app exits, stops listening and exits, and the - server re-checks the owner on every accept. Without both, the PID check is - not meaningful. Required test: start the worker, kill the owner app, + server matches the peer's PID and PID version against the owner's on + every accept. Without both, the PID check is not meaningful. Required test: start the worker, kill the owner app, and assert the socket refuses new connections and the worker exits. A second test connects after the owner has exited but before the exit - watch fires, and asserts the per-accept re-check refuses it. A + watch fires, from a process given the owner's PID number but a + different PID version (simulated in the check), and asserts it is + refused. A connection the app hands to a child process is a known gap at this phase (see Option C); phase 4 closes it and adds its test. - Return an error instead of panicking when the listener cannot be created, From a8c2e4b53bae8cb9497a35f2932e594cbf824602 Mon Sep 17 00:00:00 2001 From: Micah Alpern Date: Thu, 5 Nov 2026 21:36:57 -0800 Subject: [PATCH 19/30] ADR-046: specify listener-failure, early-owner-exit and socket-path discovery Co-Authored-By: Claude Opus 5.5 --- .../Services/Networking/KanataTCPClient.swift | 1 + docs/adr/adr-046-session-engine-control-channel.md | 14 +++++++++++--- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/Sources/KeyPathAppKit/Services/Networking/KanataTCPClient.swift b/Sources/KeyPathAppKit/Services/Networking/KanataTCPClient.swift index 4e9b8d0a6..1579b2d13 100644 --- a/Sources/KeyPathAppKit/Services/Networking/KanataTCPClient.swift +++ b/Sources/KeyPathAppKit/Services/Networking/KanataTCPClient.swift @@ -68,6 +68,7 @@ final class CompletionFlag: @unchecked Sendable { /// Virtual keys bound to `push-msg` reach `ActionDispatcher` through /// `KanataEventListener`. ADR-046 proposes moving this channel to an /// owner-checked Unix socket. +/// TODO(#1354): revise this note after the lab-VM check and again when ADR-046 lands. actor KanataTCPClient { let host: String let port: Int diff --git a/docs/adr/adr-046-session-engine-control-channel.md b/docs/adr/adr-046-session-engine-control-channel.md index 2003420e1..64715c296 100644 --- a/docs/adr/adr-046-session-engine-control-channel.md +++ b/docs/adr/adr-046-session-engine-control-channel.md @@ -256,11 +256,17 @@ written from source alone. second test connects after the owner has exited but before the exit watch fires, from a process given the owner's PID number but a different PID version (simulated in the check), and asserts it is - refused. A + refused. If the owner has already exited when the watch is registered, + registration fails with `ESRCH`; the worker treats that as owner exit and + stops at once. A third test covers that case. A connection the app hands to a child process is a known gap at this phase (see Option C); phase 4 closes it and adds its test. - Return an error instead of panicking when the listener cannot be created, - by binding once and keeping that listener, not by probing first. + by binding once and keeping that listener, not by probing first. The + bridge returns the error through its existing error buffer, and + `SessionRuntimeWorker` ends with a failed report naming the cause (for + example, socket directory not owned by this user), which the app shows. + It does not retry in a loop. - Option A: drop `ReloadFile`, `ReloadNext/Prev/Num` and `SetMouse` from the session build (feature flag), cap connections, and put a write timeout on notification sockets so a stalled client is dropped rather than blocking @@ -285,7 +291,9 @@ written from source alone. never truncate. If a long home path exceeds it, fall back to a short directory under the per-user temporary directory (`confstr(_CS_DARWIN_USER_TEMP_DIR)`, which macOS creates 0700 for this - user), with the same ownership and mode checks. Fail with a clear error only + user), with the same ownership and mode checks. Because the location can + vary, the app, the CLI and `verify-installed-app.sh` take the path from the + worker's report, never from an assumed directory. Fail with a clear error only if both are too long. The worker creates and validates the directory (reuse `RuntimeControlServer`'s checks) and reports the socket path in its owned report instead of `tcpPort`. From 963be1b4f63e0dfca9c5abdb71c3e5b8403d4bef Mon Sep 17 00:00:00 2001 From: Micah Alpern Date: Thu, 5 Nov 2026 21:38:41 -0800 Subject: [PATCH 20/30] ADR-046: state the accepted release risk and the submodule-bump order Also keep ADR-013's original 2026-10-31 note for history. Co-Authored-By: Claude Opus 5.5 --- docs/adr/adr-013-tcp-without-auth.md | 4 ++++ .../adr-046-session-engine-control-channel.md | 18 +++++++++++++----- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/docs/adr/adr-013-tcp-without-auth.md b/docs/adr/adr-013-tcp-without-auth.md index 45e7d3a2b..aabddd54c 100644 --- a/docs/adr/adr-013-tcp-without-auth.md +++ b/docs/adr/adr-013-tcp-without-auth.md @@ -1,5 +1,9 @@ # ADR-013: TCP Communication Without Authentication +> **Note (2026-10-31):** This was written for the root daemon. The runtime now runs +> in a user session that holds the user's input permissions, so this risk needs +> review. See ADR-045. +> > **Note (2026-11-04):** This was written for the root daemon, which ADR-045 > retired. The session runtime holds the user's input permissions, and from a > source review the port exposes a keystroke stream and input injection to diff --git a/docs/adr/adr-046-session-engine-control-channel.md b/docs/adr/adr-046-session-engine-control-channel.md index 64715c296..88d1fbcef 100644 --- a/docs/adr/adr-046-session-engine-control-channel.md +++ b/docs/adr/adr-046-session-engine-control-channel.md @@ -25,7 +25,7 @@ processes of other logged-in users and sandboxed apps that hold kind. Every accepted connection joins the broadcast set before it has sent a byte. -Facts below were read from the fork at `External/kanata`, at `d9085a9`, the +As of 2026-11-04, facts below were read from the fork at `External/kanata`, at `d9085a9`, the commit KeyPath `master` (`e42ffea0`) pins, and from KeyPath at that commit. `d9085a9` is on the fork's `keypath/explicit-source-keys-20261017` branch, not yet on `keypath/bundled`. They were not exercised on a @@ -221,7 +221,12 @@ directly and asks the app over the existing runtime-control socket, as Adopt **C**, and do **A** in the same fork change. The first public session-runtime release ships only after phases 1 to 3 below. -Phase 4 can follow in a point release. +Phase 4 can follow in a point release. Shipping before phase 4 accepts one +residual risk on purpose: a same-user, unsandboxed process that injects into +or impersonates the app (attacker 3, partly mitigated in the table below). +That process already runs as the user, and closing the gap needs the +signature check. Accepting this ADR means accepting that risk for the first +release; if it is not acceptable, phase 4 joins the release gate. | Attacker | Today | After phases 1–3 (release) | After phase 4 | |---|---|---|---| @@ -274,9 +279,12 @@ written from source alone. - Rust tests for the peer check and the removed commands. Estimate: roughly 300 to 500 lines of new code and tests, plus rewriting the bridge's existing passthru TCP test (`Rust/KeyPathKanataHostBridge/src/lib.rs`). - Phases 1 and 2 land together: the new listener, the removal of the bind - probe and the bridge API change ship in one fork update, so no build has - the Unix socket while still panicking on, or probing, a TCP bind. + Phases 1 and 2 land together. The fork change merges to `keypath/bundled` + first, but KeyPath only picks it up in one KeyPath PR that bumps the + submodule and changes the bridge (`Rust/KeyPathKanataHostBridge`, which + lives in this repository) and the worker. No KeyPath commit pins the new + fork without the matching bridge change, so no build has the Unix socket + while still panicking on, or probing, a TCP bind. 2. **Phase 2, bridge and worker.** `keypath_kanata_bridge_create_passthru_runtime` takes a socket path (and expected peer PID) instead of `tcp_port`. The expected PID is the app that launched this worker, passed on every launch, so a relaunch From 8d1d0df4823feb98e1d6d7af0f149b6c0673d41a Mon Sep 17 00:00:00 2001 From: Micah Alpern Date: Thu, 5 Nov 2026 21:38:53 -0800 Subject: [PATCH 21/30] ADR-013: drop the repeated sentence from the new note Co-Authored-By: Claude Opus 5.5 --- docs/adr/adr-013-tcp-without-auth.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/docs/adr/adr-013-tcp-without-auth.md b/docs/adr/adr-013-tcp-without-auth.md index aabddd54c..ded685a14 100644 --- a/docs/adr/adr-013-tcp-without-auth.md +++ b/docs/adr/adr-013-tcp-without-auth.md @@ -4,9 +4,7 @@ > in a user session that holds the user's input permissions, so this risk needs > review. See ADR-045. > -> **Note (2026-11-04):** This was written for the root daemon, which ADR-045 -> retired. The session runtime holds the user's input permissions, and from a -> source review the port exposes a keystroke stream and input injection to +> **Note (2026-11-04):** From a source review, the port exposes a keystroke stream and input injection to > every local process. The review and the proposed replacement are in > [ADR-046](adr-046-session-engine-control-channel.md); tracked in > [#1354](https://github.com/malpern/KeyPath/issues/1354). From e48904487d4c34ee0eb085ee6a074ff8a0c798ec Mon Sep 17 00:00:00 2001 From: Micah Alpern Date: Thu, 5 Nov 2026 21:41:59 -0800 Subject: [PATCH 22/30] ADR-046: test the socket-path fallback Co-Authored-By: Claude Opus 5.5 --- docs/adr/adr-046-session-engine-control-channel.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/adr/adr-046-session-engine-control-channel.md b/docs/adr/adr-046-session-engine-control-channel.md index 88d1fbcef..67e3fa329 100644 --- a/docs/adr/adr-046-session-engine-control-channel.md +++ b/docs/adr/adr-046-session-engine-control-channel.md @@ -301,7 +301,9 @@ written from source alone. (`confstr(_CS_DARWIN_USER_TEMP_DIR)`, which macOS creates 0700 for this user), with the same ownership and mode checks. Because the location can vary, the app, the CLI and `verify-installed-app.sh` take the path from the - worker's report, never from an assumed directory. Fail with a clear error only + worker's report, never from an assumed directory. Tests: a home path that + pushes the primary path past 104 bytes uses the fallback, and one that + overflows both fails with the clear error. Fail with a clear error only if both are too long. The worker creates and validates the directory (reuse `RuntimeControlServer`'s checks) and reports the socket path in its owned report instead of `tcpPort`. From d98b519e4995d81b452a977bbcf970809156c87e Mon Sep 17 00:00:00 2001 From: Micah Alpern Date: Thu, 5 Nov 2026 21:44:32 -0800 Subject: [PATCH 23/30] ADR-046: add a fast-forward check and a re-verify step for the shipping fork commit Co-Authored-By: Claude Opus 5.5 --- docs/adr/adr-046-session-engine-control-channel.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/docs/adr/adr-046-session-engine-control-channel.md b/docs/adr/adr-046-session-engine-control-channel.md index 67e3fa329..a1fb04b25 100644 --- a/docs/adr/adr-046-session-engine-control-channel.md +++ b/docs/adr/adr-046-session-engine-control-channel.md @@ -284,7 +284,12 @@ written from source alone. submodule and changes the bridge (`Rust/KeyPathKanataHostBridge`, which lives in this repository) and the worker. No KeyPath commit pins the new fork without the matching bridge change, so no build has the Unix socket - while still panicking on, or probing, a TCP bind. + while still panicking on, or probing, a TCP bind. Before pushing to + `keypath/bundled`, confirm the push is a fast-forward + (`git log --oneline origin/keypath/bundled..` lists only the new + commits, and `git merge-base --is-ancestor origin/keypath/bundled ` + succeeds). Before accepting this ADR, re-check the claims above against the + fork commit that will ship, since `d9085a9` is not yet on `keypath/bundled`. 2. **Phase 2, bridge and worker.** `keypath_kanata_bridge_create_passthru_runtime` takes a socket path (and expected peer PID) instead of `tcp_port`. The expected PID is the app that launched this worker, passed on every launch, so a relaunch From 9f8b949b5b6fe10f557539e3763d3a1a79008afc Mon Sep 17 00:00:00 2001 From: Micah Alpern Date: Thu, 5 Nov 2026 21:45:03 -0800 Subject: [PATCH 24/30] ADR-046: d9085a9 is now on keypath/bundled Co-Authored-By: Claude Opus 5.5 --- docs/adr/adr-046-session-engine-control-channel.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/adr/adr-046-session-engine-control-channel.md b/docs/adr/adr-046-session-engine-control-channel.md index a1fb04b25..1cccddca6 100644 --- a/docs/adr/adr-046-session-engine-control-channel.md +++ b/docs/adr/adr-046-session-engine-control-channel.md @@ -27,8 +27,8 @@ byte. As of 2026-11-04, facts below were read from the fork at `External/kanata`, at `d9085a9`, the commit KeyPath `master` (`e42ffea0`) pins, and from KeyPath at that commit. -`d9085a9` is on the fork's `keypath/explicit-source-keys-20261017` branch, not -yet on `keypath/bundled`. They were not exercised on a +`d9085a9` is on `keypath/bundled`: the branch tip `54cdba9` merges it, with +an identical tree. They were not exercised on a live install for this ADR. ### What a connected client can do @@ -289,7 +289,7 @@ written from source alone. (`git log --oneline origin/keypath/bundled..` lists only the new commits, and `git merge-base --is-ancestor origin/keypath/bundled ` succeeds). Before accepting this ADR, re-check the claims above against the - fork commit that will ship, since `d9085a9` is not yet on `keypath/bundled`. + fork commit that will ship, if it is no longer `d9085a9`. 2. **Phase 2, bridge and worker.** `keypath_kanata_bridge_create_passthru_runtime` takes a socket path (and expected peer PID) instead of `tcp_port`. The expected PID is the app that launched this worker, passed on every launch, so a relaunch From 940e3cbfecd37c35bd6f17a3aa682058156c2021 Mon Sep 17 00:00:00 2001 From: Micah Alpern Date: Thu, 5 Nov 2026 21:45:49 -0800 Subject: [PATCH 25/30] ADR-046: confirm LOCAL_PEERTOKEN availability during implementation Co-Authored-By: Claude Opus 5.5 --- docs/adr/adr-046-session-engine-control-channel.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/adr/adr-046-session-engine-control-channel.md b/docs/adr/adr-046-session-engine-control-channel.md index 1cccddca6..9edd65c04 100644 --- a/docs/adr/adr-046-session-engine-control-channel.md +++ b/docs/adr/adr-046-session-engine-control-channel.md @@ -204,7 +204,10 @@ directly and asks the app over the existing runtime-control socket, as and its PID version (`audit_token_to_pidversion` on its own `TASK_AUDIT_TOKEN`) at launch, and the server reads the peer's audit token from the accepted socket (`LOCAL_PEERTOKEN`) and requires both to match. The - PID version changes when a PID is reused, which closes the race. The worker + PID version changes when a PID is reused, which closes the race. Confirm + during implementation that `LOCAL_PEERTOKEN` and `audit_token_to_pidversion` + are usable on KeyPath's minimum macOS (15), as for `NWEndpoint.unix`; if + not, phase 1 needs another way to bind a connection to the owner. The worker also watches the owner with a kqueue `NOTE_EXIT` source so it stops listening the moment the app exits. Until phase 4 ships, attacker 3 is stopped from connecting directly but not from injecting into or impersonating From 414bbc66438066afda9286acfdefacdbfcd58294 Mon Sep 17 00:00:00 2001 From: Micah Alpern Date: Thu, 5 Nov 2026 21:46:46 -0800 Subject: [PATCH 26/30] ADR-046: run the Rust acceptance tests in CI; check LOCAL_PEERTOKEN in the lab Co-Authored-By: Claude Opus 5.5 --- docs/adr/adr-046-session-engine-control-channel.md | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/docs/adr/adr-046-session-engine-control-channel.md b/docs/adr/adr-046-session-engine-control-channel.md index 9edd65c04..cb3e5213b 100644 --- a/docs/adr/adr-046-session-engine-control-channel.md +++ b/docs/adr/adr-046-session-engine-control-channel.md @@ -240,8 +240,10 @@ release; if it is not acceptable, phase 4 joins the release gate. Before this ADR is accepted, confirm the main claims on a live install in a lab VM: a second macOS user, and a sandboxed app with the network-client -entitlement, each connect to the port and receive `KeyInput`. This ADR was -written from source alone. +entitlement, each connect to the port and receive `KeyInput`. In the same VM, +check that `LOCAL_PEERTOKEN` returns the peer's audit token on an `AF_UNIX` +stream socket on macOS 15, since the attacker-3 mitigation depends on it. This +ADR was written from source alone. ## Migration @@ -292,7 +294,11 @@ written from source alone. (`git log --oneline origin/keypath/bundled..` lists only the new commits, and `git merge-base --is-ancestor origin/keypath/bundled ` succeeds). Before accepting this ADR, re-check the claims above against the - fork commit that will ship, if it is no longer `d9085a9`. + fork commit that will ship, if it is no longer `d9085a9`. CI today only + builds the fork and the bridge (`cargo build` in `.github/workflows/ci.yml`); + it runs no Rust tests. The KeyPath PR that lands phases 1 and 2 adds a + `cargo test` step for the bridge and the fork's peer-check tests to a + required CI job, so the acceptance tests above cannot be skipped. 2. **Phase 2, bridge and worker.** `keypath_kanata_bridge_create_passthru_runtime` takes a socket path (and expected peer PID) instead of `tcp_port`. The expected PID is the app that launched this worker, passed on every launch, so a relaunch From 6f7e62612dac4726b06bc32363f846f18d4a9020 Mon Sep 17 00:00:00 2001 From: Micah Alpern Date: Thu, 5 Nov 2026 21:47:56 -0800 Subject: [PATCH 27/30] ADR-046: state the panic assumption; make the peer-token reader injectable Co-Authored-By: Claude Opus 5.5 --- docs/adr/adr-046-session-engine-control-channel.md | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/docs/adr/adr-046-session-engine-control-channel.md b/docs/adr/adr-046-session-engine-control-channel.md index cb3e5213b..63cff263d 100644 --- a/docs/adr/adr-046-session-engine-control-channel.md +++ b/docs/adr/adr-046-session-engine-control-channel.md @@ -98,7 +98,10 @@ on its behalf. for example at login or during a restart). The worker uses the bridge's passthru entry point, which has no bind probe, so the fork's `TcpListener::bind(...).expect` panics inside an `extern "C"` function and - aborts the worker. (`keypath_kanata_bridge_run_runtime` does probe and + aborts the worker. (The bridge crate sets no panic profile, so it unwinds by + default, has no `catch_unwind`, and Rust 1.81 and later abort when a panic + unwinds out of an `extern "C"` function. The fork's `panic = "abort"` + applies only when kanata is the top-level crate.) (`keypath_kanata_bridge_run_runtime` does probe and return an error, but the probe drops its listener before the server binds, so it is a check-then-use race.) KeyPath's own clients then connect to the squatter. KeyPath does not check who owns the listener; only @@ -265,8 +268,8 @@ ADR was written from source alone. and assert the socket refuses new connections and the worker exits. A second test connects after the owner has exited but before the exit watch fires, from a process given the owner's PID number but a - different PID version (simulated in the check), and asserts it is - refused. If the owner has already exited when the watch is registered, + different PID version, and asserts it is refused. The peer-token reader + is injectable so this runs deterministically without a second process. If the owner has already exited when the watch is registered, registration fails with `ESRCH`; the worker treats that as owner exit and stops at once. A third test covers that case. A connection the app hands to a child process is a known gap at this phase From 3ea834a68950b44c94fc209f00892ffb7db2a05d Mon Sep 17 00:00:00 2001 From: Micah Alpern Date: Thu, 5 Nov 2026 21:48:41 -0800 Subject: [PATCH 28/30] ADR-046: repeat the LOCAL_PEERTOKEN caveat next to the release gate Co-Authored-By: Claude Opus 5.5 --- docs/adr/adr-046-session-engine-control-channel.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/adr/adr-046-session-engine-control-channel.md b/docs/adr/adr-046-session-engine-control-channel.md index 63cff263d..f5b960476 100644 --- a/docs/adr/adr-046-session-engine-control-channel.md +++ b/docs/adr/adr-046-session-engine-control-channel.md @@ -232,7 +232,9 @@ residual risk on purpose: a same-user, unsandboxed process that injects into or impersonates the app (attacker 3, partly mitigated in the table below). That process already runs as the user, and closing the gap needs the signature check. Accepting this ADR means accepting that risk for the first -release; if it is not acceptable, phase 4 joins the release gate. +release; if it is not acceptable, phase 4 joins the release gate. The +phase 1–3 mitigation for attacker 3 also assumes `LOCAL_PEERTOKEN` works on +`AF_UNIX` sockets on macOS 15, which the lab check below must confirm. | Attacker | Today | After phases 1–3 (release) | After phase 4 | |---|---|---|---| From 6f43c72a8c94fc3d77630f4048ade49106984a24 Mon Sep 17 00:00:00 2001 From: Micah Alpern Date: Thu, 5 Nov 2026 21:49:48 -0800 Subject: [PATCH 29/30] ADR-046: name the LOCAL_PEERTOKEN fallback; hedge ADR-013's inline notes Co-Authored-By: Claude Opus 5.5 --- docs/adr/adr-013-tcp-without-auth.md | 4 ++-- docs/adr/adr-046-session-engine-control-channel.md | 8 ++++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/docs/adr/adr-013-tcp-without-auth.md b/docs/adr/adr-013-tcp-without-auth.md index ded685a14..3d39c907f 100644 --- a/docs/adr/adr-013-tcp-without-auth.md +++ b/docs/adr/adr-013-tcp-without-auth.md @@ -24,12 +24,12 @@ Use TCP without authentication for localhost IPC. - Kanata 1.9.0 TCP server does not support authentication - Connection is localhost-only (127.0.0.1) -- Same security model as other local IPC (Unix sockets, etc.) *(Wrong: loopback TCP has no peer-uid check and is reachable from other users and sandboxed apps. See ADR-046.)* +- Same security model as other local IPC (Unix sockets, etc.) *(Wrong: loopback TCP has no peer-uid check and is reachable from other users and sandboxed apps, per a source review not yet confirmed live. See ADR-046.)* ## Security Considerations - Any local process can send commands to Kanata -- This is acceptable for keyboard remapping (low security impact) *(No longer true for the session runtime: the port exposes a keystroke stream and input injection. See ADR-046.)* +- This is acceptable for keyboard remapping (low security impact) *(No longer true for the session runtime: per a source review, the port exposes a keystroke stream and input injection. See ADR-046.)* - If Kanata adds auth in future versions, we should adopt it ## Implementation diff --git a/docs/adr/adr-046-session-engine-control-channel.md b/docs/adr/adr-046-session-engine-control-channel.md index f5b960476..13a0873cb 100644 --- a/docs/adr/adr-046-session-engine-control-channel.md +++ b/docs/adr/adr-046-session-engine-control-channel.md @@ -210,7 +210,10 @@ directly and asks the app over the existing runtime-control socket, as PID version changes when a PID is reused, which closes the race. Confirm during implementation that `LOCAL_PEERTOKEN` and `audit_token_to_pidversion` are usable on KeyPath's minimum macOS (15), as for `NWEndpoint.unix`; if - not, phase 1 needs another way to bind a connection to the owner. The worker + not, the fallback is `LOCAL_PEERPID` plus the `NOTE_EXIT` watch, with the + PID-reuse window between owner exit and the watch firing recorded as an + accepted risk, and phase 4's signature check moves into the release gate. + The worker also watches the owner with a kqueue `NOTE_EXIT` source so it stops listening the moment the app exits. Until phase 4 ships, attacker 3 is stopped from connecting directly but not from injecting into or impersonating @@ -339,7 +342,8 @@ ADR was written from source alone. 4. **Phase 4, signature check.** Add the `LOCAL_PEERTOKEN` code-signing check against KeyPath's designated requirement. Debug builds signed with another identity need an explicit, logged development override. -5. **Phase 5, docs.** Mark ADR-013 superseded, rewrite the TCP section of +5. **Phase 5, docs.** Mark ADR-013 superseded (its status line, the README and + `index.md` rows, and this ADR's own header change together), rewrite the TCP section of `guides/privacy.md` (it says key events cross the connection, but not that any other local process receives them, and calls the risk low) and publish it, update `docs/architecture/overview.md` and `cli-runtime-control.md`, and fix the From 9f3b7831da8647630da455d11e9cb3c17e035f8c Mon Sep 17 00:00:00 2001 From: Micah Alpern Date: Thu, 5 Nov 2026 21:52:45 -0800 Subject: [PATCH 30/30] ADR-046: make the lab check an explicit blocker for acceptance Co-Authored-By: Claude Opus 5.5 --- docs/adr/adr-046-session-engine-control-channel.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/adr/adr-046-session-engine-control-channel.md b/docs/adr/adr-046-session-engine-control-channel.md index 13a0873cb..79505268a 100644 --- a/docs/adr/adr-046-session-engine-control-channel.md +++ b/docs/adr/adr-046-session-engine-control-channel.md @@ -246,8 +246,8 @@ phase 1–3 mitigation for attacker 3 also assumes `LOCAL_PEERTOKEN` works on | 3. Same user, unsandboxed | Full access | Partly mitigated: direct connection blocked (PID check), but injecting into or impersonating the app is still possible | Blocked unless it injects code into KeyPath (out of scope) | | 4. Port squatter | Takes over the channel | Blocked (no TCP port; socket in a directory only this user owns) | Blocked | -Before this ADR is accepted, confirm the main claims on a live install in a -lab VM: a second macOS user, and a sandboxed app with the network-client +**Blocks acceptance:** this ADR stays Proposed until a live install in a lab VM +confirms the main claims: a second macOS user, and a sandboxed app with the network-client entitlement, each connect to the port and receive `KeyInput`. In the same VM, check that `LOCAL_PEERTOKEN` returns the peer's audit token on an `AF_UNIX` stream socket on macOS 15, since the attacker-3 mitigation depends on it. This