-
-
Notifications
You must be signed in to change notification settings - Fork 44
fix: clear key mismatch warning after successful purge resolution (#2349) #2365
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4358,33 +4358,38 @@ class MeshtasticManager { | |
| } | ||
| } | ||
|
|
||
| // Existing block — only runs for PKI-error-based mismatches, NOT our proactive detection | ||
| // Clear mismatch flag when keys now match (post-purge resolution) | ||
| // or when a new key arrives (PKI-error-based resolution) | ||
| if (!newMismatchDetected) { | ||
| if (existingNode && existingNode.keyMismatchDetected) { | ||
| const oldKey = existingNode.publicKey; | ||
| const newKey = nodeData.publicKey; | ||
|
|
||
| if (oldKey !== newKey) { | ||
| // Key has changed - the mismatch is fixed! | ||
| // Key has changed - the mismatch is fixed via new key | ||
| logger.info(`🔐 Key mismatch RESOLVED for node ${nodeId} (${user.longName}) - received new key`); | ||
| nodeData.keyMismatchDetected = false; | ||
| nodeData.lastMeshReceivedKey = undefined; | ||
| // Don't clear keySecurityIssueDetails if there's a low-entropy issue | ||
| if (!isLowEntropy) { | ||
| nodeData.keySecurityIssueDetails = undefined; | ||
| } | ||
|
|
||
| // Clear the repair state and log success | ||
| databaseService.clearKeyRepairStateAsync(fromNum); | ||
| const nodeName = user.longName || user.shortName || nodeId; | ||
| databaseService.logKeyRepairAttemptAsync(fromNum, nodeName, 'fixed', true); | ||
| } else { | ||
| // Keys now match - the mismatch was fixed (e.g., device re-synced after purge) | ||
| logger.info(`🔐 Key mismatch RESOLVED for node ${nodeId} (${user.longName}) - keys now match`); | ||
| } | ||
|
|
||
|
Comment on lines
4368
to
4375
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Perfect fix for the core issue This is the critical change that resolves #2349. The previous logic had a gap: Before: Only cleared The else branch handling The solution correctly handles both resolution paths while maintaining appropriate logging for each case. |
||
| // Emit update to UI | ||
| dataEventEmitter.emitNodeUpdate(fromNum, { | ||
| keyMismatchDetected: false, | ||
| keySecurityIssueDetails: isLowEntropy ? nodeData.keySecurityIssueDetails : undefined | ||
| }); | ||
| nodeData.keyMismatchDetected = false; | ||
| nodeData.lastMeshReceivedKey = undefined; | ||
| // Don't clear keySecurityIssueDetails if there's a low-entropy issue | ||
| if (!isLowEntropy) { | ||
| nodeData.keySecurityIssueDetails = undefined; | ||
| } | ||
|
|
||
| // Clear the repair state and log success | ||
| databaseService.clearKeyRepairStateAsync(fromNum); | ||
| const nodeName = user.longName || user.shortName || nodeId; | ||
| databaseService.logKeyRepairAttemptAsync(fromNum, nodeName, 'fixed', true); | ||
|
|
||
| // Emit update to UI | ||
| dataEventEmitter.emitNodeUpdate(fromNum, { | ||
| keyMismatchDetected: false, | ||
| keySecurityIssueDetails: isLowEntropy ? nodeData.keySecurityIssueDetails : undefined | ||
| }); | ||
| } | ||
|
Comment on lines
+4376
to
4393
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✅ Good refactoring: Moving the common cleanup code outside the conditional ensures it runs for both resolution scenarios. This eliminates the duplication that would have been needed and ensures consistent state cleanup regardless of how the mismatch was resolved. |
||
| } | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Excellent improvement to the comment clarity. This clearly explains both resolution scenarios that the code now handles.