node-zwave-js icon indicating copy to clipboard operation
node-zwave-js copied to clipboard

feat(zwave-js): skip interview if S2 failed

Open renaiku opened this issue 5 months ago • 3 comments

Fixes #7011

This answer to the issue guided me to the solution.

Problem:

A node should not be interviewed when security S2 fails

Solution:

Adding InclusionStrategy in InclusionResult allows to check for low security of S2 strategy just before the interview.

renaiku avatar Jul 30 '25 15:07 renaiku

a) The InclusionResult is used by applications, and those applications pass in the inclusion strategy when starting the inclusion. So this would essentially pass unnecessary info back

Oh I see. Would moving the skip condition before the "node added" event would be better then ?

// Skip the interview if S2 failed
if (result.lowSecurity
  && opts.strategy == InclusionStrategy.Security_S2) return;

this.emit("node added", newNode, result);

b) It does not consider the interviews that happen when Z-Wave JS restarts, loads its network information from cache and some devices have not fully been interviewed.

Can you point me where this happen please ? I would like to look into it.

renaiku avatar Aug 03 '25 13:08 renaiku

Would moving the skip condition before the "node added" event would be better then ?

Not really. The event is needed for applications. I think it would make more sense to store this as an attribute of the node that also gets persisted to the cache.

Can you point me where this happen please ? I would like to look into it.

See the initializeControllerAndNodes method in Driver.ts - pretty far at the end of that method.

AlCalzone avatar Aug 03 '25 20:08 AlCalzone

Yes sorry @AlCalzone, I 100% forgot 🥲

If it's ok to put the skip there in the Driver.ts file, I can quickly go to a computer with git on it and update the PR.

    /** This is called when a new node has been added to the network */
	private onNodeAdded(node: ZWaveNode): void {
		this.addNodeEventHandlers(node);

		if (this._options.interview?.disableOnNodeAdded) return;
		if (this._options.testingHooks?.skipNodeInterview) return;

		if (node.failedS2) return; // <===== SKIP ADDED HERE

		// Interview the node
		// don't await the interview, because it may take a very long time
		// if a node is asleep
		void this.interviewNodeInternal(node);
	}

EDIT: pushed it

renaiku avatar Aug 07 '25 14:08 renaiku