Add validation of perms array, and skip bridging accessories with malformed data

[NorthernMan54]

After digging through a supplied /accessories dump, we found that a Homebridge plugin can place null values in a characteristic’s perms array.

If this malformed data reaches HomeKit, the entire bridge may show No Response. When adding a new bridge, HomeKit instead reports Accessory Out of Compliance.

The fix validates every perms array before publication. If an accessory fails validation, HAP-NodeJS logs the plugin, accessory, service, and characteristic responsible, then blocks that accessory so the valid accessories can continue working.

Sample logs:

[8/17/2026, 9:58:43 AM] [homebridge-hap-replay-test]
This plugin threw an error from the characteristic 'Air Pressure':

HAP-NodeJS cannot publish accessory 'Lasne':
service 'Lasne' (0000008A-0000-1000-8000-0026BB765291),
characteristic 'Air Pressure'
(E863F10F-079E-48FF-8F27-9C2605A29F52)
contains invalid permissions: [null,"ev"].

[8/17/2026, 9:58:43 AM] [HAP Replay Test]
Blocked captured AID 76:
HAP-NodeJS cannot publish accessory 'Lasne' ...

[coveralls]

Coverage Report for CI Build 32147210020

Warning

No base build found for commit c9d4a41 on beta-2.2.3.
Coverage changes can't be calculated without a base build.
If a base build is processing, this comment will update automatically when it completes.

Coverage: 66.735%

Details

• Patch coverage: 45 of 45 lines across 2 files are fully covered (100%).

Uncovered Changes

No uncovered changes found.

Coverage Regressions

Requires a base build to compare against. How to fix this →

---

Coverage Stats

Relevant Lines:	9670
Covered Lines:	6771
Line Coverage:	70.02%
Relevant Branches:	3452
Covered Branches:	1986
Branch Coverage:	57.53%
Branches in Coverage %:	Yes
Coverage Strength:	364.82 hits per line

---

💛 - Coveralls

[bwp91]

The throw-on-add / skip-on-publish split is the right shape now. A few things to change before this goes in:

• VALID_PERMISSIONS hardcodes the wire strings, so it is no longer tied to the Perms enum. Add a permission to the enum later and this will reject it as invalid, with nothing to catch it at compile time. Moving the enum itself into util/characteristic-permissions.ts and re-exporting it from Characteristic.ts would fix that and the circular import you are working around.
• validatePerms([]) returns true — every on an empty array is vacuously true. Both callers happen to check the length separately so nothing is broken today, but the check belongs inside the function.
• Nothing tests that publish() calls removeBridgedAccessoriesWithInvalidPermissions(). The test calls the private method directly, so deleting that line from publish() leaves the suite green. Please add a case that goes through publish() itself.
• validateAccessory has lost its doc comment — the existing block has been repurposed for validateCharacteristicPermissions.
• Changelog: the heading should be ## v2.2.3 (Pending Release) to match the rest of the file, and the bullet should be the commit subject, lowercase and without the full stop.
• The commit title Fix for #1128 will close that issue on merge. Please unlink it — malformed perms have not been shown to be the cause there, and I want to keep that one open.

[hjdhjd]

I've taken this the last mile and pushed two commits onto the branch. The diagnosis here was on-point, and the setProps validation was the right instinct...what I've changed is where the enforcement lives.

What moved and why. The add-time throw and the publish-time removal sweep are gone. In their place, the serving boundary now does the enforcement: both toHAP and internalHAPRepresentation draw their bridged accessories through a single filter that excludes any accessory whose permissions HomeKit would reject. Three reasons for the move:

• Throwing from addBridgedAccessory lands in Homebridge call paths that have no try/catch around them (bridgeService.ts platform registration, childBridgeFork.ts). One bad accessory would take down bridge startup instead of degrading gracefully - the exact opposite of what we want here.
• Validating at add time misses the case where a plugin assigns props.perms directly after the accessory is already published. That mutation reaches neither setProps nor any event, so the only place it can always be caught is the moment we build what we serve. There's a test covering exactly this now.
• Removal is destructive and exclusion is not. A quarantined accessory keeps its place on the bridge, its aid, its cached identifiers, and its context. Fix the plugin and it comes back exactly where it was, under the same identity HomeKit knew it by.

Because both representation builders share the one filter, the served database and the hashed configuration can't disagree - a quarantine transition bumps c#, controllers re-read, and everyone converges. The transition also emits a characteristic warning naming the accessory, service, characteristic, and the offending array, once, so the responsible plugin is identifiable from the log. One deliberate attribute: a quarantined accessory's aid remains reachable for reads and events until the controller re-reads after the c# bump.

The setProps throw stays, consolidated into a single predicate (isValidPerms) that now lives beside the Perms enum and references its members rather than restating the wire strings - which also picks up @bwp91's points about the enum-tied set and the vacuous empty check. The validateAccessory doc comment is restored, and the new tests drive the real /accessories route rather than the private helpers. I've verified each new test fails when the behavior it pins is broken. 901 tests passing, lint and docs clean.

Second commit. While in there I fixed a latent bug: validateAccessory's recursion guard read this.bridged, which is false on a bridge, so a bridge never actually validated its bridged accessories despite the doc saying it does. Bridges validate their children at publish now.

On #1128 - there's also another interesting bit...the captured dump carries a second compliance defect besides the perms: an accessory name with a trailing space. @NorthernMan54, since you have the replay harness up, a bisect would settle causation - the dump minus the three weather accessories, then minus only the trailing-space name. If the name turns out to matter to tvOS 26 too, promoting name validation into this same quarantine mechanism is a natural follow-up. Nice work!

[Copilot]

Pull request overview

Adds characteristic permission validation and quarantines malformed bridged accessories so valid accessories remain available.

Changes:

• Adds runtime permission validation.
• Excludes and reports invalid bridged accessories.
• Adds tests and release notes.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/lib/Characteristic.ts	Adds permission validation.
src/lib/Characteristic.spec.ts	Tests validation behavior.
src/lib/Accessory.ts	Implements publication checks and quarantine.
src/lib/Accessory.spec.ts	Tests blocking and recovery behavior.
CHANGELOG.md	Documents the fixes.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/lib/Characteristic.ts:1879

• The truthy guard above means falsy malformed values such as "", false, or 0 never reach this validator; setProps silently keeps the old permissions instead of rejecting the invalid input. Preserve the existing null/undefined no-op behavior for this required property, but validate every other supplied value (and add a falsy case to the table test).

      if (!isValidPerms(props.perms)) {
        throw new Error(`characteristic '${this.displayName}' (${this.UUID}) contains invalid permissions: ${describePerms(props.perms)}`);

[NorthernMan54]

On #1128 - there's also another interesting bit...the captured dump carries a second compliance defect besides the perms: an accessory name with a trailing space. @NorthernMan54, since you have the replay harness up, a bisect would settle causation - the dump minus the three weather accessories, then minus only the trailing-space name. If the name turns out to matter to tvOS 26 too, promoting name validation into this same quarantine mechanism is a natural follow-up.

@hjdhjd During testing we tested with just minus the three weather accessories and it past compliance. The names also include an Emoji and it past compliance as well as the trailing space issue.

[bwp91]

from claude:

---

This is good work from both of you, and the diagnosis in particular — going from a /accessories dump to a specific null in a perms array, with a replay harness to prove it, is the hard part.

I agree with the move to the serving boundary. Catching a direct props.perms assignment is something setProps validation structurally cannot do, and keeping the aid, cached identifiers and context so a fixed plugin comes back where it was is the right instinct. Routing both representation builders through one filter, so the served database and the hashed configuration cannot disagree, is the detail I'd have most wanted and would most likely have forgotten.

The validateAccessory catch is real. if (this.bridged) is false on a bridge, so bridged accessories have never been validated despite the doc saying they are.

One consequence of fixing that, worth deciding on deliberately: every bridged accessory now gets the AccessoryInformation checks for the first time. Those are console.log warnings about missing Manufacturer, Model, SerialNumber and names. On a large bridge that could be a lot of new output on first publish, and #1116 was already someone reacting to that kind of noise. It is correct behaviour, but it will read as a regression to anyone who updates and finds their log full of warnings about accessories that were fine yesterday. Worth deciding whether it ships in the same release as the fix, or whether those drop to debug level.

The one thing I'd want resolved before this goes in

The reasoning for dropping the addBridgedAccessory throw was that it lands in call paths with no try/catch around them. That is right, but the setProps throw we've kept reaches the same place:

• Characteristic's constructor calls this.setProps(props) (Characteristic.ts:1734), so a custom characteristic that declares bad perms throws the moment it is constructed.
• server.ts:269 calls restoreCachedPlatformAccessories() with nothing around it, and that synchronously calls platformPlugins.configureAccessory(accessory) at bridgeService.ts:532.

So the plugin that started all this — the Eve Air Pressure characteristic carrying [null, "ev"] — would, on this branch, throw during startup rather than be quarantined. Its failure mode changes from one unresponsive accessory to Homebridge not starting at all. The old assert(props.perms.length > 0) let that value through, so this is new for input that used to work.

That is the opposite of what the quarantine mechanism is for. My preference is that setProps emits a characteristic warning and leaves the value in place, and the serving-boundary filter stays the single enforcement point — it already catches this case, and it degrades instead of failing hard. If we do want a throw there, the callers need wrapping first, and that is a Homebridge-side change rather than one here.

Two decisions I'd like your view on

1. Should isValidPerms and describePerms be public API?

src/index.ts does export * from "./lib/Characteristic", so exporting them from that file publishes them automatically, TypeDoc included. Whatever we ship becomes something we have to keep.

• Both public, as now. Plugin authors can check an array before handing it over, which is a genuine kindness. The cost is that both are permanent — removing or reshaping either later is a breaking change, and describePerms is really our diagnostic formatter rather than anything an author needs.
• isValidPerms public, describePerms internal. Authors get the half with a use case, and we stay free to change how we format the message. My preference.
• Both internal. No new surface at all, everything stays changeable. If someone asks for the check later we can export it then — adding is easy, removing is not.

2. Should findPermsViolation() keep running on every build?

It walks every characteristic of every bridged accessory on each /accessories response and each configuration hash.

• Leave it. Always correct, because the model is re-read every time — which is the entire point, since a direct props.perms assignment reaches no event. The cost is one extra pass over the same data the serialisation is about to walk anyway, so it is bounded by work we already do. This is what I'd choose.
• Fold the check into the existing serialisation walk. Saves the second pass, but the filter has to decide inclusion before building, so it means restructuring both builders and giving up the single readable predicate. A lot of disruption for a saving I don't think anyone can measure.
• Cache the result per accessory, invalidated on setProps. Cheapest in the steady state, and I'd reject it — invalidating on setProps reintroduces the exact blind spot this design was built to close. A plugin assigning props.perms directly would keep a stale "valid" verdict forever, which is worse than the problem.

On the trailing space and the emoji

That looks settled by the testing above: removing only the three weather accessories passed compliance, and the names with an emoji and a trailing space passed too. So the perms array is the cause, and name validation is not indicated. Worth saying so explicitly on #1128 so nobody reopens that line later.

[matbe81]

As the original reporter whose logs and /accessories dump led to this investigation, I just wanted to clarify one detail regarding the trailing-space warning.

The accessory named:

Lave-vaisselle 

is not related to homebridge-weather-plus. It comes from a dishwasher exposed through Home Connect (https://github.com/thoukydides/homebridge-homeconnect).

I corrected the device name by removing the trailing space:

"Lave-vaisselle " → "Lave-vaisselle"

However, the Homebridge accessory continued to expose the previous name with the trailing space, so the change made on the device/Home Connect side did not propagate to the existing accessory.

Just mentioning this to avoid associating that particular warning with Weather Plus; the malformed perms issue identified in the dump is a separate issue.

[hjdhjd]

As the original reporter whose logs and /accessories dump led to this investigation, I just wanted to clarify one detail regarding the trailing-space warning.

The accessory named:

Lave-vaisselle 

is not related to homebridge-weather-plus. It comes from a dishwasher exposed through Home Connect (https://github.com/thoukydides/homebridge-homeconnect).

I corrected the device name by removing the trailing space:

"Lave-vaisselle " → "Lave-vaisselle"

However, the Homebridge accessory continued to expose the previous name with the trailing space, so the change made on the device/Home Connect side did not propagate to the existing accessory.

Just mentioning this to avoid associating that particular warning with Weather Plus; the malformed perms issue identified in the dump is a separate issue.

Thanks - I didn't think it would ultimately be a cause...but it is an issue that should be addressed on the user front to ensure you don't end up with any odd behavior downstream (Siri, etc). Thanks again!

[matbe81]

Thank you. I opened a dedicated issue for the relevant plugin: thoukydides/homebridge-homeconnect#400

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/lib/Accessory.ts:995

• The boolean suppresses diagnostics for a new violation while the accessory remains quarantined. For example, if two characteristics are invalid, fixing the first leaves the accessory excluded because the second is now returned, but no warning identifies that second characteristic. Track the current offending service/characteristic (rather than only quarantine state) and emit again when that identity changes; configuration updates should still occur only when entering or leaving quarantine.

      if (violation) {
        if (!accessory.permsQuarantined) {
          accessory.permsQuarantined = true;
          accessory.sendCharacteristicWarning(

[hjdhjd]

from claude:

This is good work from both of you, and the diagnosis in particular — going from a /accessories dump to a specific null in a perms array, with a replay harness to prove it, is the hard part.

I agree with the move to the serving boundary. Catching a direct props.perms assignment is something setProps validation structurally cannot do, and keeping the aid, cached identifiers and context so a fixed plugin comes back where it was is the right instinct. Routing both representation builders through one filter, so the served database and the hashed configuration cannot disagree, is the detail I'd have most wanted and would most likely have forgotten.

The validateAccessory catch is real. if (this.bridged) is false on a bridge, so bridged accessories have never been validated despite the doc saying they are.

One consequence of fixing that, worth deciding on deliberately: every bridged accessory now gets the AccessoryInformation checks for the first time. Those are console.log warnings about missing Manufacturer, Model, SerialNumber and names. On a large bridge that could be a lot of new output on first publish, and #1116 was already someone reacting to that kind of noise. It is correct behaviour, but it will read as a regression to anyone who updates and finds their log full of warnings about accessories that were fine yesterday. Worth deciding whether it ships in the same release as the fix, or whether those drop to debug level.

The one thing I'd want resolved before this goes in

The reasoning for dropping the addBridgedAccessory throw was that it lands in call paths with no try/catch around them. That is right, but the setProps throw we've kept reaches the same place:

• Characteristic's constructor calls this.setProps(props) (Characteristic.ts:1734), so a custom characteristic that declares bad perms throws the moment it is constructed.
• server.ts:269 calls restoreCachedPlatformAccessories() with nothing around it, and that synchronously calls platformPlugins.configureAccessory(accessory) at bridgeService.ts:532.

So the plugin that started all this — the Eve Air Pressure characteristic carrying [null, "ev"] — would, on this branch, throw during startup rather than be quarantined. Its failure mode changes from one unresponsive accessory to Homebridge not starting at all. The old assert(props.perms.length > 0) let that value through, so this is new for input that used to work.

That is the opposite of what the quarantine mechanism is for. My preference is that setProps emits a characteristic warning and leaves the value in place, and the serving-boundary filter stays the single enforcement point — it already catches this case, and it degrades instead of failing hard. If we do want a throw there, the callers need wrapping first, and that is a Homebridge-side change rather than one here.

Two decisions I'd like your view on

1. Should isValidPerms and describePerms be public API?

src/index.ts does export * from "./lib/Characteristic", so exporting them from that file publishes them automatically, TypeDoc included. Whatever we ship becomes something we have to keep.

• Both public, as now. Plugin authors can check an array before handing it over, which is a genuine kindness. The cost is that both are permanent — removing or reshaping either later is a breaking change, and describePerms is really our diagnostic formatter rather than anything an author needs.
• isValidPerms public, describePerms internal. Authors get the half with a use case, and we stay free to change how we format the message. My preference.
• Both internal. No new surface at all, everything stays changeable. If someone asks for the check later we can export it then — adding is easy, removing is not.

2. Should findPermsViolation() keep running on every build?

It walks every characteristic of every bridged accessory on each /accessories response and each configuration hash.

• Leave it. Always correct, because the model is re-read every time — which is the entire point, since a direct props.perms assignment reaches no event. The cost is one extra pass over the same data the serialisation is about to walk anyway, so it is bounded by work we already do. This is what I'd choose.
• Fold the check into the existing serialisation walk. Saves the second pass, but the filter has to decide inclusion before building, so it means restructuring both builders and giving up the single readable predicate. A lot of disruption for a saving I don't think anyone can measure.
• Cache the result per accessory, invalidated on setProps. Cheapest in the steady state, and I'd reject it — invalidating on setProps reintroduces the exact blind spot this design was built to close. A plugin assigning props.perms directly would keep a stale "valid" verdict forever, which is worse than the problem.

On the trailing space and the emoji

That looks settled by the testing above: removing only the three weather accessories passed compliance, and the names with an emoji and a trailing space passed too. So the perms array is the cause, and name validation is not indicated. Worth saying so explicitly on #1128 so nobody reopens that line later.

I flipped the setProps throw to a warning, the serving boundary stays the single enforcement point, and nothing else moves. The reason is the cache...Characteristic.serialize persists props into cachedAccessories, deserialize feeds them back through setProps, and PlatformAccessory.deserialize runs outside the try/catch in loadCachedPlatformAccessoriesFromDisk - that catch covers only the storage read. The throw escapes server.start() into cli.ts's errorHandler, which terminates the process. Net effect: any install that ever cached malformed perms boot-loops, and fixing the plugin doesn't cure it. The constructor frames (new constructor(...), getServices(), configureAccessory) are equally unwrapped and fail the same way. setProps now warns through the characteristic warning channel and keeps the declared value for the quarantine to catch. A characteristic under construction has no warning subscriber yet, so that case falls back to the console...the stack is captured either way, so attribution survives.

One correction to the analysis above: the cached-restore path does wrap addBridgedAccessory. It's registration that doesn't - handleRegisterPlatformAccessories propagates straight into the plugin's own registerPlatformAccessories() call - which is why the add-time throw came out in the first place.

The publish-time throw stays. Both Homebridge call sites wrap publish() with .catch(), and a standalone accessory is the root of its own database with nowhere to quarantine to. Correct and contained.

Neither helper becomes public API. This is a bugfix - we aren't growing the API surface here. Both are @private now.

findPermsViolation keeps running per build. Direct props.perms mutation reaches no event, so re-reading the model at serve time is the only always-correct answer.

The validateAccessory warnings ship as-is. The bug was that they never ran. Homebridge presets a FirmwareRevision placeholder before configureAccessory, so the most common gap won't fire on restore. If it ends up being too noisy, we can always tweak it to be debug-level...let's see.