Skip to content

fix: resolve persistent device DB warning and security config not saving#2351

Merged
Yeraze merged 1 commit intomainfrom
fix/device-nodedb-warning-and-security-config
Mar 20, 2026
Merged

fix: resolve persistent device DB warning and security config not saving#2351
Yeraze merged 1 commit intomainfrom
fix/device-nodedb-warning-and-security-config

Conversation

@Yeraze
Copy link
Owner

@Yeraze Yeraze commented Mar 20, 2026

Summary

  • "Not in your radio's database" warning never clears: deviceNodeNums was only populated during initial config sync (processNodeInfoProtobuf). When a node later sent NodeInfo over the mesh (exchanging keys), the set was never updated, so the warning persisted indefinitely. Now adds nodes to deviceNodeNums in processNodeInfoMessageProtobuf when NodeInfo arrives over the mesh.

  • Security config settings don't persist after save: Two sub-issues:

    1. getCurrentConfig() was missing Proto3 default handling for security config booleans (debugLogApiEnabled, serialEnabled, isManaged, adminChannelEnabled). Proto3 omits false values, so these came back as undefined instead of false.
    2. After saving security config via admin command, the cached actualDeviceConfig was not updated, so the frontend read stale values until the next config resync. Added updateCachedDeviceConfig() method and call it after successful security config saves.

Test plan

  • Connect to a node, navigate to Messages tab, select a node that shows the "not in radio's database" warning — verify warning clears after receiving NodeInfo from that node
  • Go to Configuration > Security, toggle "Debug Log API Enabled" on, save — verify it remains enabled after refreshing the page
  • Verify other security settings (Serial Enabled, Is Managed, Admin Channel Enabled) also persist correctly
  • Run full test suite (2987 tests passing)

🤖 Generated with Claude Code

Two fixes:

1. "Not in your radio's database" warning never clears - deviceNodeNums was
   only populated during initial config sync. Now also adds nodes when
   NodeInfo packets arrive over the mesh, which is when keys are exchanged.

2. Security config settings (Debug Log API, Serial Enabled, etc.) don't
   persist after save - missing Proto3 default handling for security config
   booleans in getCurrentConfig(), and cached config not updated after save.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions
Copy link

github-actions bot commented Mar 20, 2026

Claude encountered an error —— View job


I'll analyze this and get back to you.

@github-actions
Copy link

github-actions bot commented Mar 20, 2026

Claude finished @Yeraze's task —— View job


Comprehensive PR Review

I've thoroughly analyzed this pull request that fixes two distinct issues: persistent device DB warnings and security config persistence problems.

Code Quality & Best Practices

Device DB Warning Fix (meshtasticManager.ts:4265)

  • Consistent Pattern: Follows established pattern used at lines 5713 (initial sync) and 9678 (node removal)
  • Logical Placement: Correctly placed in processNodeInfoMessageProtobuf() where mesh NodeInfo packets are processed
  • Clean Implementation: Simple, targeted fix with good commenting

Security Config Persistence

  • Proto3 Pattern Consistency: Follows the exact same pattern used for LoRA, Position, MQTT configs throughout the codebase (2591-2790, 3161-3285)
  • Clean Architecture: New updateCachedDeviceConfig() method is well-designed with proper null checks and logging
  • Targeted Application: Cache update only applies to setSecurityConfig on local node, avoiding unnecessary overhead

⚠️ Potential Issues & Edge Cases

  1. Input Validation: No validation that fromNum is valid before adding to deviceNodeNums Set. While Sets handle duplicates gracefully, invalid node numbers could theoretically be added.

  2. Race Conditions: The cache update in server.ts:7090 happens immediately after sending the admin command, but there's no guarantee the device has processed it. However, this is acceptable since the real config will sync on next connection.

  3. Test Coverage: No tests found covering these specific code paths (deviceNodeNums, updateCachedDeviceConfig, security config defaults).

Performance Considerations

  • Minimal Overhead: Set.add() operation is O(1)
  • Efficient Cache Update: Only updates specific config section, not entire cache
  • No Memory Leaks: deviceNodeNums Set is properly managed (cleared on disconnect, entries removed on node deletion)

Security Considerations

  • No Security Risks: Changes don't introduce any security vulnerabilities
  • Proper Scope: Security config cache updates only apply to local node administration
  • Input Safety: Proto3 default handling uses safe undefined checks with explicit fallbacks

⚠️ Test Coverage Gaps

Missing Test Cases:

  • Device DB warning clearance when NodeInfo packets arrive
  • Security config boolean defaults handling in getCurrentConfig()
  • Cache update behavior after setSecurityConfig admin commands
  • Proto3 default value behavior verification

📝 Suggestions for Enhancement

  1. Add Validation (meshtasticManager.ts:4265):
// Track that this node is in the radio's database
// means the radio has the node's identity (and typically its public key for DMs)
if (fromNum && fromNum > 0) {
  this.deviceNodeNums.add(fromNum);
}
  1. Consider Future Config Types: The updateCachedDeviceConfig() method could be extended to handle other config sections that might have similar persistence issues.

Overall Assessment

This is a well-implemented fix that:

  • Addresses real user-facing issues with targeted solutions
  • Follows established codebase patterns consistently
  • Maintains code quality and architectural principles
  • Has minimal performance impact
  • Introduces no security concerns

The changes are production-ready. The missing test coverage and minor validation enhancement are suggestions for future improvement, not blocking issues.

Recommendation: ✅ Approve for merge


@Yeraze Yeraze merged commit 66e98d7 into main Mar 20, 2026
14 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant