fix(docker): restore add-on device access after USB re-enumeration#6877
fix(docker): restore add-on device access after USB re-enumeration#6877carlossg wants to merge 2 commits into
Conversation
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
This is not true, the options validation adds it: supervisor/supervisor/apps/options.py Lines 171 to 195 in b54a01e So it adds it to the resulting
That is handled by the above code, by-id paths are resolved and the underlying device is added. Did updating the permission not work for you? You should see If its not working, what type of installation do you have? Can you share the logs? |
|
@agners thanks for the review, I've checked a bit more and part of the PR is not needed I'll post Claude analysis
|
b1189ce to
ee6118b
Compare
|
I upgraded core and it happened again. Attaching logs issue-zwave_js.log |
ee6118b to
942af50
Compare
agners
left a comment
There was a problem hiding this comment.
Can you update the PR description to accurately reflect what the current change is addressing?
…ased devices Two bugs caused a crash loop when a USB device re-enumerates to a different minor number (e.g. ttyACM0→ttyACM1) after a HAOS reboot: 1. _hw_listener was only registered when addon.static_devices was non-empty. Addons that expose a device via the options schema (e.g. Z-Wave JS `device:` option) never had the listener registered, so add_devices_allowed was never called when the device reappeared at a new minor. 2. _hardware_events matched only device.path and device.sysfs against static_devices. When static_devices (or the new options path) contains a by-id symlink, the match always failed because by-id paths live in device.links. Fix: extend the listener registration condition to also cover addon.devices (options-based), and expand the path-matching set to include device.links so by-id paths resolve correctly. For options-based devices, compare the incoming Device against addon.devices (which re-evaluates options.json against the live hardware list, picking up the new minor number automatically). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
942af50 to
844c770
Compare
There was a problem hiding this comment.
Pull request overview
This PR fixes add-ons losing device access after USB re-enumeration by ensuring the Docker app registers for hardware hotplug events even when devices are configured via options, and by matching hotplug events against the options-resolved device set.
Changes:
- Register the hardware event listener when either manifest
static_devicesor options-deriveddevicesare present. - Update the hotplug handler to treat an incoming
Deviceas relevant if it matches eitherstatic_devicesorapp.devices. - Add a regression test covering options-based device hotplug handling.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
supervisor/docker/app.py |
Extends hardware listener registration and event matching to include options-based devices. |
tests/docker/test_app.py |
Adds coverage to ensure options-based devices register the listener and trigger cgroup permission updates on hotplug. |
| if ( | ||
| not any( | ||
| device_path in (device.path, device.sysfs) | ||
| for device_path in self.app.static_devices | ||
| ) |
agners
left a comment
There was a problem hiding this comment.
Ok, the PR description is much better. But I think it misses one core point: The user needs to choose a by-id device, which can symlink to a different real device on reenumeration (it is stated in the reproduction steps, but I think this should be noted in the main description as well).
| was non-empty. Add-ons like Z-Wave JS that configure the device via the | ||
| options schema (addon.devices) never had a listener, so cgroup permissions | ||
| were never updated after a USB re-enumeration. | ||
| """ |
There was a problem hiding this comment.
A full end-to-end test that sets a real device: option resolving to a by-id link (and ideally simulates a minor-number change on re-enumeration) would be nice.
| device_path in (device.path, device.sysfs) | ||
| for device_path in self.app.static_devices | ||
| ) | ||
| and device not in self.app.devices |
There was a problem hiding this comment.
Hm, this means on every hardware event we do a full validation, including pwnd hashing and whatnot.
The device extraction is just a side effect, but it's really not ideal doing this as part of the validation.
Now this is a bit bigger refactor, but I think its worth doing since this would introduce a lot of validation otherwise:
The cleaner separation is two distinct concerns:
- What did the user configure? A set of device paths, depends only on schema + options.json. Changes only when options change.
- Does this event's device match one of them? Should be a cheap comparison.
And the key realization: in the event handler you don't need to resolve anything against live hardware. The incoming device already carries .path, .sysfs, and .links (which includes the by-id link). So you just intersect the configured paths against the device's own identifiers:
async def _hardware_events(self, device: Device) -> None:
allowed_paths = set(self.app.static_devices) | self.app.option_device_paths
if not allowed_paths & {device.path, device.sysfs, *device.links}:
return
...
permission = self.sys_hardware.policy.get_cgroups_rule(device) # uses event's live major:minor This is strictly better because:
- drops the per-event full validation entirely,
- also fixes by-id matching for both static_devices and options-based devices (which I think your earlier iteration tried to do too).
- uses the incoming device's current major:minor for the cgroup rule, which is the whole point of re-enumeration handling.
Refactor _hardware_events to avoid per-event full options validation
(including pwnd hashing). Introduce AppOptions.extract_device_paths and
AppModel.option_device_paths to extract raw device paths from options
without resolving against live hardware. Use set-intersection against
{device.path, device.sysfs, *device.links} so by-id symlinks match
correctly after re-enumeration for both static and options-based devices.
Update test to use real schema/options setup and simulate a minor-number
change (ttyACM0→ttyACM1) with a stable by-id symlink.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Proposed change
Add-ons that get their serial device from the options schema (e.g. Z-Wave JS
device:) lose access whenever the device re-enumerates to a different minornumber (e.g.
ttyACM0→ttyACM1after a HAOS reboot). The add-on thencrash-loops on
EACCESuntil the user restarts it.Key prerequisite: the user must have selected the device via a
/dev/serial/by-id/…symlink (the stable path shown in the add-on UI). Onre-enumeration the kernel assigns a new device node and a new major:minor pair,
but the by-id symlink stays the same. Supervisor must therefore match the
incoming hardware event via that stable by-id link and re-grant cgroup access
with the device's current major:minor.
Two independent bugs in
DockerAppcaused this:Listener never registered for options-based devices.
_hw_listenerwasonly wired up when
addon.static_devices(the manifest'sdevices:list)was non-empty. Add-ons that expose a device only via the options schema
never registered the listener, so
add_devices_allowedwas never calledwhen the device reappeared at a new minor.
Options-based devices never matched in
_hardware_events. The handlercompared the incoming
Deviceonly againststatic_devices(and onlyagainst
.path/.sysfs, not.links). Devices configured via the optionsschema were never matched, so access was never re-granted even if the
listener had been registered.
Fix
Introduce
AppOptions.extract_device_pathsandAppModel.option_device_paths:a cheap property that walks the raw schema and options to return the configured
device path strings as
set[Path], without running full options validation(no hardware lookup, no pwnd hashing).
In
_hardware_events, replace the old two-condition guard with a singleset-intersection:
This is strictly better:
device.links) for both static andoptions-based devices.
the whole point of re-enumeration handling.
Reproduction
device:option at a/dev/serial/by-id/…symlink.
minor.
EACCESreading the device.After fix: Supervisor re-grants cgroup access and the add-on recovers.
Type of change
Additional information
Checklist
ruff format supervisor tests)If API endpoints or add-on configuration are added/changed: