fix: channel database no longer shadows device channels with same PSK

[Yeraze]

Summary

Fixes the Channel Database "shadowing" bug where a database channel with the same PSK as a device channel would intercept all incoming messages, causing them to appear in the database channel view instead of the device channel.

Root cause: When server-side decryption matched a database channel, the message was always assigned to CHANNEL_DB_OFFSET + dbChannelId, even when a device channel had the identical PSK. This caused MQTT-proxied messages to show in the database channel chat window instead of the device channel.

Fix: After server-side decryption, check if any active device channel (0-7) has the same PSK as the matched database channel. If so, assign the message to the device channel slot instead. The database channel still gets decryption credit (counter incremented) and the encrypted packet is still forwarded to the physical node unchanged.

Fixes #2413, fixes #2375

Changes

File	Change
src/server/meshtasticManager.ts	PSK comparison after server decryption to prefer device channels

Test plan

• npx vitest run — 3070 tests pass
• npm run build — no TypeScript errors
• User @LN4CY to verify: MQTT messages now appear in device channel instead of database channel when PSKs match

🤖 Generated with Claude Code

[github-actions]

Claude finished @Yeraze's task —— View job

---

PR Review: Channel Database Shadowing Fix

Reviewing this pull request for code quality, potential bugs, performance considerations, security concerns, and test coverage.

Todo List

• Read and analyze the modified file
• Review code quality and best practices
• Check for potential bugs or edge cases
• Evaluate performance considerations
• Assess security concerns
• Evaluate test coverage
• Provide comprehensive feedback

Code Review Summary

This PR addresses a legitimate shadowing bug where database channels with the same PSK as device channels would intercept messages. The fix is well-implemented and follows existing patterns in the codebase.

✅ Code Quality & Best Practices

• Clean implementation: The logic is clearly structured with appropriate comments explaining the behavior
• Consistent style: Follows the existing codebase patterns for async/await and database access
• Good variable naming: matchingDeviceChannel, dbChannel are descriptive
• Proper logging: Debug log explains the channel selection decision at src/server/meshtasticManager.ts:3920

✅ Bug Analysis

• Root cause correctly identified: Database channels were always taking precedence regardless of device channel PSK matches
• Logic is sound: The fix properly checks for PSK matches and filters for enabled channels (role !== 0)
• Fallback preserved: When no device channel matches, it correctly falls back to the original database channel behavior
• No edge cases missed: Handles null PSKs gracefully with optional chaining

✅ Performance Considerations

• Minimal overhead: Only adds database queries when server-side decryption occurs (already an edge case)
• Efficient queries: Uses existing getByIdAsync and getAllChannels methods
• No hot path impact: This logic only runs when context?.decryptedBy === 'server'

✅ Security Assessment

• No security concerns: The fix doesn't introduce any new attack vectors
• Maintains encryption flow: Database channel still gets decryption credit and packet forwarding is unchanged
• Proper access control: Still respects the existing channel permissions system

⚠️ Minor Observations

1. Database query optimization: The getAllChannels() call could potentially be cached or optimized since it's fetching all channels just to find PSK matches. However, this is likely negligible given the infrequency of server-side decryption.

2. Test coverage: While the existing tests pass, there don't appear to be specific tests for this exact shadowing scenario. Consider adding tests that:

   • Set up a database channel and device channel with identical PSKs
   • Verify messages go to the device channel, not the database channel
   • Verify the database channel still gets decryption credit

✅ Implementation Correctness

The fix correctly:

• Only applies to server-decrypted messages (context?.decryptedBy === 'server')
• Filters device channels by enabled state (role !== 0)
• Compares PSKs as strings (consistent with the database schema)
• Maintains backward compatibility for non-matching cases
• Preserves all existing functionality (decryption credit, packet forwarding)

📋 Recommendation

APPROVE - This is a well-crafted fix that addresses the reported issue without introducing regressions. The implementation is clean, follows existing patterns, and maintains security. The only suggestion would be to add specific test cases for this scenario in future iterations.

The fix resolves issues #2413 and #2375 as intended, and the manual testing plan looks appropriate.

---

[github-actions]

Claude finished @Yeraze's task —— View job

---

PR Review: Channel Database Shadowing Fix

Reviewing this pull request for code quality, potential bugs, performance considerations, security concerns, and test coverage.

Todo List

• Read and analyze the modified file
• Review code quality and best practices
• Check for potential bugs or edge cases
• Evaluate performance considerations
• Assess security concerns
• Evaluate test coverage
• Provide comprehensive feedback

Code Review Summary

This PR addresses a legitimate shadowing bug where database channels with the same PSK as device channels would intercept messages. The fix is well-implemented and follows existing patterns in the codebase.

✅ Code Quality & Best Practices

• Clean implementation: The logic is clearly structured with appropriate comments explaining the behavior
• Consistent style: Follows the existing codebase patterns for async/await and database access
• Good variable naming: matchingDeviceChannel, dbChannel are descriptive
• Proper logging: Debug log explains the channel selection decision at src/server/meshtasticManager.ts:3920

✅ Bug Analysis

• Root cause correctly identified: Database channels were always taking precedence regardless of device channel PSK matches
• Logic is sound: The fix properly checks for PSK matches and filters for enabled channels (role !== 0)
• Fallback preserved: When no device channel matches, it correctly falls back to the original database channel behavior
• No edge cases missed: Handles null PSKs gracefully with optional chaining

✅ Performance Considerations

• Minimal overhead: Only adds database queries when server-side decryption occurs (already an edge case)
• Efficient queries: Uses existing getByIdAsync and getAllChannels methods
• No hot path impact: This logic only runs when context?.decryptedBy === 'server'

✅ Security Assessment

• No security concerns: The fix doesn't introduce any new attack vectors
• Maintains encryption flow: Database channel still gets decryption credit and packet forwarding is unchanged
• Proper access control: Still respects the existing channel permissions system

⚠️ Minor Observations

1. Database query optimization: The getAllChannels() call could potentially be cached or optimized since it's fetching all channels just to find PSK matches. However, this is likely negligible given the infrequency of server-side decryption.

2. Test coverage: While the existing tests pass, there don't appear to be specific tests for this exact shadowing scenario. Consider adding tests that:

   • Set up a database channel and device channel with identical PSKs
   • Verify messages go to the device channel, not the database channel
   • Verify the database channel still gets decryption credit

✅ Implementation Correctness

The fix correctly:

• Only applies to server-decrypted messages (context?.decryptedBy === 'server')
• Filters device channels by enabled state (role !== 0)
• Compares PSKs as strings (consistent with the database schema)
• Maintains backward compatibility for non-matching cases
• Preserves all existing functionality (decryption credit, packet forwarding)

📋 Recommendation

APPROVE - This is a well-crafted fix that addresses the reported issue without introducing regressions. The implementation is clean, follows existing patterns, and maintains security. The only suggestion would be to add specific test cases for this scenario in future iterations.

The fix resolves issues #2413 and #2375 as intended, and the manual testing plan looks appropriate.

[LN4CY]

verified on 3.10.0 release, working perfectly