grabber: scope (0,0) built-in seize match to internal-bus transports#47
Merged
jackielii merged 1 commit intoJun 7, 2026
Merged
Conversation
The (0,0) FIFO built-in alias matched on keyboard usage alone, which captured every connected keyboard (external USB/Bluetooth + the Karabiner VHIDD), not just the internal one. A one-shot un-seize pass in start() released the extras, but it closed any non-zero-VendorID device (so it also broke an explicitly-aliased external) and never re-ran for devices that enumerated later (replug, wake), leaving an external seized and remapped. Built-In=1 looked like the obvious scope, but IOHIDManager device-matching silently ignores the Built-In key (verified on Apple Silicon / Tahoe with both CFNumber 1 and CFBoolean true; the external still matched). Transport is honored, so match the built-in on its internal-bus transport (FIFO, plus SPI defensively). That makes the match exact, so the un-seize pass is gone: external keyboards report USB/Bluetooth, and the Karabiner VHIDD exposes no Transport property at all (hidutil reports it as null), so neither can match the transport-scoped dict. The only devices left that match are the internal keyboard and any explicit (vendor,product) alias. Adds a unit test for the match predicate and a SKHD_HID_LIVE-gated live test that asserts the production match captures no non-built-in device on real hardware (skipped in CI).
Owner
|
thanks |
jackielii
added a commit
that referenced
this pull request
Jun 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
The
(0,0)FIFO built-in alias I added in #45 matches on keyboard usage alone, so the seize grabs every connected keyboard (external USB/Bluetooth ones, plus the Karabiner VHIDD), not just the internal one. Thestart()un-seize pass tried to release the extras, but it closed any non-zero-VendorID device (so it also broke an explicitly-aliased external keyboard), and a keyboard that re-enumerated afterwards (replug, wake) was never re-filtered, so it stayed seized and its keys picked up the forwarded[device builtin]remaps.What surfaced this: a
backslash/backspaceswap scoped to[device builtin]leaking onto an external keyboard, intermittently.The fix
Scope the
(0,0)match byTransportinstead of usage alone:Transport ∈ {FIFO, SPI}+ keyboard usage, so only the internal-bus keyboard matches. External USB/Bluetooth keyboards never enter the seize, at initial open or on later re-enumeration.I tried
Built-In=1first, since it's the obvious scope and whathidutil --matchinguses. But IOHIDManager device-matching silently ignores theBuilt-Inkey. Verified on Apple Silicon / Tahoe with bothCFNumber 1andCFBoolean true: the external still matched.Transportis honored, so the match uses that.With the match scoped this way it's exact, so the old per-
start()un-seize pass is gone: external keyboards reportUSB/Bluetooth, and the Karabiner VHIDD I inject into exposes noTransportproperty at all (checked:hidutil property --matching '{"VendorID":0x16c0}' --get Transportreturns(null)), so none of them can match the transport-scoped dict. The only matches are the internal keyboard and any explicit(vendor,product)alias. Dropping the pass also fixes the old bug where it closed an explicitly-aliased external whenever a(0,0)alias coexisted.Verification
Two tests: a unit test for the match predicate, and a
SKHD_HID_LIVE=1-gated live test that builds the production match, opens the manager in observe mode (no seize, so no root), and asserts no non-built-in device is in the matched set. The live test is skipped in CI, since it needs real hardware and an external keyboard to mean anything.On my machine (Apple Silicon, NEO ERGO USB external attached):
Before this change the external was in the matched set.
What I haven't verified
Only
FIFOis confirmed here.SPIis included defensively, but I couldn't test a machine that reports it. If a built-in reports some other transport its keyboard won't be matched, so its remaps silently stop. That's the same class of risk the(0,0)alias already carried, just narrower now.This is verified at the match layer (observe mode): the change is to which devices the matching dict selects, plus removing the post-match pass. The seize mechanism itself (
IOHIDManagerOpenwithkIOHIDOptionsTypeSeizeDevice) is untouched. I didn't run the seize end-to-end on this branch because of an unrelated TCC/signing issue on my install.DeviceCheck.isPresent(used to decide whether to forward rules to the grabber) still detects the built-in by a VID-less heuristic rather than transport — same effective result as the old seize match, so the two no longer use identical logic. I left it alone here since reconciling them properly means sharing one matcher across the agent and grabber modules; happy to do that as a follow-up.If you'd rather go further, the per-device seize model Karabiner uses (open the manager without seize, then
IOHIDDeviceOpen(SeizeDevice)per chosen device) would be the more thorough route, and would also make built-in detection a per-device property read instead of a transport guess. I can take that on if you want it. This PR is the smaller, contained fix to the matching I introduced in #45.