Skip to content

feat(meshcore): auto-acknowledge automation with channels, DM, and macros#3245

Merged
Yeraze merged 6 commits into
mainfrom
feat/meshcore-auto-acknowledge
May 28, 2026
Merged

feat(meshcore): auto-acknowledge automation with channels, DM, and macros#3245
Yeraze merged 6 commits into
mainfrom
feat/meshcore-auto-acknowledge

Conversation

@Yeraze
Copy link
Copy Markdown
Owner

@Yeraze Yeraze commented May 28, 2026

Summary

  • Recreates the Meshtastic Auto-Acknowledge feature in the MeshCore Automations tab with an identical UI: per-channel allowlist + separate DM checkbox, Always respond via DM override, regex trigger with live test panel, per-sender cooldown, and a templated reply with token macros + sample preview.
  • Backend wires a new checkAutoAcknowledge handler into meshcoreManager.handleBridgeEvent for both contact_message (DM) and channel_message paths, with regex validation, ReDoS protection, and contact-prefix resolution for DM addressing.
  • Per-source settings persist under meshcoreAutoAck* keys; CRUD lives at GET/POST /api/sources/:id/meshcore/automation/autoack (mirrors the auto-pathfinding route shape).

Supported macros

{NODE_ID}, {NODE_NAME}, {LONG_NAME}, {SHORT_NAME}, {DATE}, {TIME}, {SNR}, {VERSION}

Test plan

  • Open the MeshCore source → Automations tab and confirm the new Auto-Acknowledge card renders below Auto-Pathfinding.
  • Toggle the master switch, set regex to ^ping, enable a single channel, save — confirm the SaveBar fires and the values reload on refresh.
  • Send ping on the enabled channel from another node → reply is published on the same channel.
  • Enable Always respond via DM → confirm the reply switches to a DM to the sender.
  • Send ping on a non-allowlisted channel → confirm no reply.
  • Disable the Direct Messages checkbox → confirm DM triggers are ignored.
  • Set cooldown to 30s and send two pings within the window → second is suppressed.
  • Test all tokens render correctly in the live preview and in the actual reply.
  • Invalid regex (e.g. () blocks save with an error toast.

🤖 Generated with Claude Code

…cros

Recreates the Meshtastic Auto-Acknowledge feature in the MeshCore
Automations tab: per-channel allowlist plus a separate DM checkbox,
"Always respond via DM" override, regex trigger with live test panel,
per-sender cooldown, and a templated reply with token macros
({NODE_ID}, {NODE_NAME}, {LONG_NAME}, {SHORT_NAME}, {DATE}, {TIME},
{SNR}, {VERSION}) plus a sample preview.

Backend wires checkAutoAcknowledge into handleBridgeEvent for both
contact_message and channel_message paths; routes live under
/api/sources/:id/meshcore/automation/autoack and persist via the
existing per-source settings store.

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

github-actions Bot commented May 28, 2026

Claude finished @Yeraze's task —— View job


PR Review: Auto-Acknowledge Automation

Comprehensive review of this 824-line auto-acknowledge implementation focusing on code quality, security, performance, and best practices.

Todo List:

  • Read and understand the new MeshCoreAutoAckSection component
  • Review the automation view integration
  • Analyze backend settings constants
  • Review meshcoreManager integration and handler logic
  • Review API routes implementation
  • Check for security concerns and input validation
  • Assess performance implications
  • Evaluate test coverage and patterns
  • Provide comprehensive feedback summary

🟢 Strengths

Excellent Architecture & Integration:

  • Perfect adherence to multi-source patterns from CLAUDE.md
  • Clean separation: UI component → API routes → manager handler
  • Mirrors existing auto-pathfinding structure for consistency
  • All settings properly registered in VALID_SETTINGS_KEYS and PER_SOURCE_SETTINGS_KEYS

Outstanding UX Implementation:

  • Live regex testing with visual feedback (green/red indicators)
  • Real-time message template preview with token substitution
  • Intuitive per-channel allowlist with DM checkbox
  • Proper loading states, error handling, and accessibility

Solid Code Quality:

  • Consistent use of established hooks (useSaveBar, useCsrfFetch, useAuth)
  • Good TypeScript typing throughout
  • Proper async/await patterns
  • Clean component structure with logical separation

🟡 Areas for Improvement

Security Concerns:

  1. ReDoS Protection Could Be Strengthened (MeshCoreAutoAckSection.tsx:46):

    if (/(\.\*){2,}|(\+.*\+)|(\*.*\*)|(\{[0-9]{3,}\})|(\{[0-9]+,\})/.test(pattern)) {
      return { valid: false, error: 'Pattern too complex (possible ReDoS)' };
    }

    Recommendation: Add regex compilation timeout and more comprehensive ReDoS patterns

  2. Missing Input Length Validation:

    • Message templates have no length limits (could cause UI/DB issues)
    • Test messages field unlimited (potential memory issues)
    • Recommendation: Add reasonable limits (e.g., 500 chars for templates, 5KB for test messages)
  3. Template Token Injection (meshcoreManager.ts:3284-3290):

    • Token replacement appears safe but no explicit sanitization
    • Recommendation: Consider XSS protection if tokens could contain user input

Code Quality Issues:

  1. Silent Error Handling (MeshCoreAutoAckSection.tsx:107-108):

    } catch {
      // ignore — keep defaults
    }

    Recommendation: Add debug logging for troubleshooting

  2. Race Condition in Cooldown (meshcoreManager.ts:3264-3269):

    const last = this.autoAckCooldowns.get(cooldownKey) || 0;
    if (Date.now() - last < cooldownSeconds * 1000) return;
    this.autoAckCooldowns.set(cooldownKey, Date.now());

    Issue: Two rapid messages could both pass cooldown check
    Recommendation: Set timestamp before the check to make it atomic

  3. Inefficient Array Comparison (MeshCoreAutoAckSection.tsx:142):

    const channelsChanged = JSON.stringify([...settings.channels].sort()) !== JSON.stringify([...initial.channels].sort());

    Recommendation: Use Set-based comparison for better performance

Performance Considerations:

  1. Regex Recreation on Every Message:

    • Frontend uses useMemo (good) but backend recreates regex for each message check
    • Recommendation: Cache compiled regex in manager with settings invalidation
  2. Memory Leak Risk:

    • autoAckCooldowns Map grows indefinitely
    • Recommendation: Add periodic cleanup or LRU eviction

Missing Test Coverage:

  1. No Tests for New Functionality:
    • No unit tests for checkAutoAcknowledge handler
    • No integration tests for API routes
    • No component tests for UI validation
    • Critical: Per CLAUDE.md requirements, all tests must pass before PR

🔴 Critical Issues

  1. No Test Coverage: This violates CLAUDE.md requirements. The codebase expects comprehensive testing, especially for new automation features.

  2. Settings Validation Gap: API routes accept settings without validating regex patterns (validation only happens in UI)


🎯 Recommended Actions

Before Merge:

  1. Add comprehensive test coverage (unit tests for manager, API routes, component)
  2. Fix cooldown race condition
  3. Add input length limits
  4. Implement regex caching in manager
  5. Add periodic cleanup for cooldown Map

Nice to Have:

  • More robust ReDoS protection
  • Performance optimization for channel comparison
  • Better error logging

📊 Overall Assessment

This is a well-architected feature that follows project conventions excellently. The code quality is good and the UX is thoughtful. However, the lack of test coverage is a critical blocker per repository standards. With proper testing and the security/performance fixes above, this would be a solid addition to MeshMonitor.

Recommendation: Request test coverage before merge, address critical security/performance issues.


Job Run

Comment thread src/server/routes/meshcoreRoutes.ts Fixed
The pathfinding view filled the viewport and clipped the new
Auto-Acknowledge section off the bottom with no way to scroll. Give
the container its own overflowY so all sections are reachable.

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

github-actions Bot commented May 28, 2026

Claude finished @Yeraze's task —— View job


PR Review: Auto-Acknowledge Automation

Comprehensive review of this 824-line auto-acknowledge implementation focusing on code quality, security, performance, and best practices.

Todo List:

  • Read and understand the new MeshCoreAutoAckSection component
  • Review the automation view integration
  • Analyze backend settings constants
  • Review meshcoreManager integration and handler logic
  • Review API routes implementation
  • Check for security concerns and input validation
  • Assess performance implications
  • Evaluate test coverage and patterns
  • Provide comprehensive feedback summary

🟢 Strengths

Excellent Architecture & Integration:

  • Perfect adherence to multi-source patterns from CLAUDE.md
  • Clean separation: UI component → API routes → manager handler
  • Mirrors existing auto-pathfinding structure for consistency
  • All settings properly registered in VALID_SETTINGS_KEYS and PER_SOURCE_SETTINGS_KEYS

Outstanding UX Implementation:

  • Live regex testing with visual feedback (green/red indicators)
  • Real-time message template preview with token substitution
  • Intuitive per-channel allowlist with DM checkbox
  • Proper loading states, error handling, and accessibility

Solid Code Quality:

  • Consistent use of established hooks (useSaveBar, useCsrfFetch, useAuth)
  • Good TypeScript typing throughout
  • Proper async/await patterns
  • Clean component structure with logical separation

🟡 Areas for Improvement

Security Concerns:

  1. 🚨 CRITICAL: RegEx Injection Vulnerability (meshcoreRoutes.ts:2404):

    new RegExp(regex, 'i'); // CodeQL flagged this!

    Issue: User input directly passed to RegExp constructor without sanitization
    Impact: Potential ReDoS attacks, memory exhaustion
    Recommendation: Add input sanitization and compilation timeout

  2. ReDoS Protection Could Be Strengthened (MeshCoreAutoAckSection.tsx:46):

    if (/(\.\*){2,}|(\+.*\+)|(\*.*\*)|(\{[0-9]{3,}\})|(\{[0-9]+,\})/.test(pattern)) {
      return { valid: false, error: 'Pattern too complex (possible ReDoS)' };
    }

    Recommendation: Add regex compilation timeout and more comprehensive ReDoS patterns

  3. Missing Input Length Validation:

    • Message templates have no length limits (could cause UI/DB issues)
    • Test messages field unlimited (potential memory issues)
    • Recommendation: Add reasonable limits (e.g., 500 chars for templates, 5KB for test messages)
  4. Template Token Injection (meshcoreManager.ts:3284-3290):

    • Token replacement appears safe but no explicit sanitization
    • Recommendation: Consider XSS protection if tokens could contain user input

Code Quality Issues:

  1. Silent Error Handling (MeshCoreAutoAckSection.tsx:107-108):

    } catch {
      // ignore — keep defaults
    }

    Recommendation: Add debug logging for troubleshooting

  2. 🚨 Race Condition in Cooldown (meshcoreManager.ts:3264-3269):

    const last = this.autoAckCooldowns.get(cooldownKey) || 0;
    if (Date.now() - last < cooldownSeconds * 1000) return;
    this.autoAckCooldowns.set(cooldownKey, Date.now());

    Issue: Two rapid messages could both pass cooldown check
    Recommendation: Set timestamp before the check to make it atomic

  3. Inefficient Array Comparison (MeshCoreAutoAckSection.tsx:142):

    const channelsChanged = JSON.stringify([...settings.channels].sort()) !== JSON.stringify([...initial.channels].sort());

    Recommendation: Use Set-based comparison for better performance

Performance Considerations:

  1. Regex Recreation on Every Message:

    • Frontend uses useMemo (good) but backend recreates regex for each message check
    • Recommendation: Cache compiled regex in manager with settings invalidation
  2. Memory Leak Risk:

    • autoAckCooldowns Map grows indefinitely
    • Recommendation: Add periodic cleanup or LRU eviction

Missing Test Coverage:

  1. 🚨 CRITICAL: No Tests for New Functionality:
    • No unit tests for checkAutoAcknowledge handler
    • No integration tests for API routes
    • No component tests for UI validation
    • Critical: Per CLAUDE.md requirements, all tests must pass before PR

🔴 Critical Issues

  1. No Test Coverage: This violates CLAUDE.md requirements. The codebase expects comprehensive testing, especially for new automation features.

  2. RegEx Injection Security Vulnerability: CodeQL flagged a critical security issue that must be addressed.

  3. Settings Validation Gap: API routes accept settings without validating regex patterns (validation only happens in UI)


🎯 Recommended Actions

Before Merge:

  1. 🚨 Fix regex injection vulnerability - add input sanitization and compilation timeout
  2. 🚨 Add comprehensive test coverage (unit tests for manager, API routes, component)
  3. Fix cooldown race condition
  4. Add input length limits
  5. Implement regex caching in manager
  6. Add periodic cleanup for cooldown Map

Nice to Have:

  • More robust ReDoS protection
  • Performance optimization for channel comparison
  • Better error logging

📊 Overall Assessment

This is a well-architected feature that follows project conventions excellently. The code quality is good and the UX is thoughtful. However, there are critical blockers that must be addressed:

  1. Security vulnerability flagged by CodeQL
  2. Missing test coverage violating repository standards
  3. Race condition in cooldown logic

Recommendation: Address critical security and testing issues before merge. The architecture and UX implementation are solid, but the security and reliability concerns need resolution.


Job Run

Forward the packed `pathLen` byte from ContactMsgRecv/ChannelMsgRecv
in the bridge events, decode the bottom 6 bits to a hop count
(0xFF → 0 = direct), and pull the sender contact's cached `outPath`
hash chain for DMs as the {ROUTE} expansion. Channel messages have
no sender pubkey on the wire, so {ROUTE} falls back to "—".

Supports {HOPS}, {NUMBER_HOPS} (alias for Meshtastic parity), and
{ROUTE} (arrow-separated hash chain, e.g. "a3 → 7f → 02").

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

github-actions Bot commented May 28, 2026

Claude finished @Yeraze's task —— View job


PR Review: Auto-Acknowledge Automation

Comprehensive review of this 880-line auto-acknowledge implementation focusing on code quality, security, performance, and best practices.

Todo List:

  • Read and understand the new MeshCoreAutoAckSection component
  • Review the automation view integration
  • Analyze backend settings constants
  • Review meshcoreManager integration and handler logic
  • Review API routes implementation
  • Check for security concerns and input validation (especially CodeQL-flagged regex issue)
  • Assess performance implications
  • Evaluate test coverage and patterns
  • Provide comprehensive feedback summary

🟢 Strengths

Excellent Architecture & Integration:

  • Perfect adherence to multi-source patterns from CLAUDE.md
  • Clean separation: UI component → API routes → manager handler
  • Mirrors existing auto-pathfinding structure for consistency
  • All settings properly registered in VALID_SETTINGS_KEYS and PER_SOURCE_SETTINGS_KEYS

Outstanding UX Implementation:

  • Live regex testing with visual feedback (green/red indicators)
  • Real-time message template preview with token substitution
  • Intuitive per-channel allowlist with DM checkbox
  • Proper loading states, error handling, and accessibility

Solid Code Quality:

  • Consistent use of established hooks (useSaveBar, useCsrfFetch, useAuth)
  • Good TypeScript typing throughout
  • Proper async/await patterns
  • Clean component structure with logical separation

🟡 Areas for Improvement

Security Concerns:

  1. 🚨 CRITICAL: RegEx Injection Vulnerability (meshcoreRoutes.ts:2404):

    new RegExp(regex, 'i'); // CodeQL flagged this!

    Issue: User input directly passed to RegExp constructor without sanitization
    Impact: Potential ReDoS attacks, memory exhaustion
    Recommendation: Add input sanitization and compilation timeout

  2. ReDoS Protection Could Be Strengthened (MeshCoreAutoAckSection.tsx:46):

    if (/(\.\*){2,}|(\+.*\+)|(\*.*\*)|(\{[0-9]{3,}\})|(\{[0-9]+,\})/.test(pattern)) {
      return { valid: false, error: 'Pattern too complex (possible ReDoS)' };
    }

    Recommendation: Add regex compilation timeout and more comprehensive ReDoS patterns

  3. Missing Input Length Validation:

    • Message templates have no length limits (could cause UI/DB issues)
    • Test messages field unlimited (potential memory issues)
    • Recommendation: Add reasonable limits (e.g., 500 chars for templates, 5KB for test messages)
  4. Template Token Injection (meshcoreManager.ts:3284-3290):

    • Token replacement appears safe but no explicit sanitization
    • Recommendation: Consider XSS protection if tokens could contain user input

Code Quality Issues:

  1. Silent Error Handling (MeshCoreAutoAckSection.tsx:107-108):

    } catch {
      // ignore — keep defaults
    }

    Recommendation: Add debug logging for troubleshooting

  2. 🚨 Race Condition in Cooldown (meshcoreManager.ts:3264-3269):

    const last = this.autoAckCooldowns.get(cooldownKey) || 0;
    if (Date.now() - last < cooldownSeconds * 1000) return;
    this.autoAckCooldowns.set(cooldownKey, Date.now());

    Issue: Two rapid messages could both pass cooldown check
    Recommendation: Set timestamp before the check to make it atomic

  3. Inefficient Array Comparison (MeshCoreAutoAckSection.tsx:142):

    const channelsChanged = JSON.stringify([...settings.channels].sort()) !== JSON.stringify([...initial.channels].sort());

    Recommendation: Use Set-based comparison for better performance

Performance Considerations:

  1. Regex Recreation on Every Message:

    • Frontend uses useMemo (good) but backend recreates regex for each message check
    • Recommendation: Cache compiled regex in manager with settings invalidation
  2. Memory Leak Risk:

    • autoAckCooldowns Map grows indefinitely
    • Recommendation: Add periodic cleanup or LRU eviction

Missing Test Coverage:

  1. 🚨 CRITICAL: No Tests for New Functionality:
    • No unit tests for checkAutoAcknowledge handler
    • No integration tests for API routes
    • No component tests for UI validation
    • Critical: Per CLAUDE.md requirements, all tests must pass before PR

🔴 Critical Issues

  1. No Test Coverage: This violates CLAUDE.md requirements. The codebase expects comprehensive testing, especially for new automation features.

  2. RegEx Injection Security Vulnerability: CodeQL flagged a critical security issue that must be addressed.

  3. Settings Validation Gap: API routes accept settings without validating regex patterns (validation only happens in UI)


🎯 Recommended Actions

Before Merge:

  1. 🚨 Fix regex injection vulnerability - add input sanitization and compilation timeout
  2. 🚨 Add comprehensive test coverage (unit tests for manager, API routes, component)
  3. Fix cooldown race condition
  4. Add input length limits
  5. Implement regex caching in manager
  6. Add periodic cleanup for cooldown Map

Nice to Have:

  • More robust ReDoS protection
  • Performance optimization for channel comparison
  • Better error logging

📊 Overall Assessment

This is a well-architected feature that follows project conventions excellently. The code quality is good and the UX is thoughtful. However, there are critical blockers that must be addressed:

  1. Security vulnerability flagged by CodeQL
  2. Missing test coverage violating repository standards
  3. Race condition in cooldown logic

Recommendation: Address critical security and testing issues before merge. The architecture and UX implementation are solid, but the security and reliability concerns need resolution.


Job Run

ContactMsgRecv / ChannelMsgRecv only carry the packed pathLen byte
(hop count), not the relay-hash chain. LogRxData (0x88) is emitted
for every received OTA packet and exposes the raw bytes, which we
parse with meshcore.js's Packet.fromBytes to recover the actual path
hashes.

Wire the LogRxData listener in the native backend, buffer the path
for TXT_MSG packets only (single-slot, consumed by the next
contact/channel recv event since the firmware emits them in
order), and forward `path_hops` through the bridge event. The
manager prefers per-packet hops over the contact's cached outPath
fallback so {ROUTE} reflects the actual route this packet took.

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

github-actions Bot commented May 28, 2026

Claude finished @Yeraze's task —— View job


PR Review: Auto-Acknowledge Automation

Comprehensive review of this 880-line auto-acknowledge implementation focusing on code quality, security, performance, and best practices.

Todo List:

  • Read and understand the new MeshCoreAutoAckSection component
  • Review the automation view integration
  • Analyze backend settings constants
  • Review meshcoreManager integration and handler logic
  • Review API routes implementation
  • Check for security concerns and input validation (especially CodeQL-flagged regex issue)
  • Assess performance implications
  • Evaluate test coverage and patterns
  • Provide comprehensive feedback summary

🟢 Strengths

Excellent Architecture & Integration:

  • Perfect adherence to multi-source patterns from CLAUDE.md
  • Clean separation: UI component → API routes → manager handler
  • Mirrors existing auto-pathfinding structure for consistency
  • All settings properly registered in VALID_SETTINGS_KEYS and PER_SOURCE_SETTINGS_KEYS

Outstanding UX Implementation:

  • Live regex testing with visual feedback (green/red indicators)
  • Real-time message template preview with token substitution
  • Intuitive per-channel allowlist with DM checkbox
  • Proper loading states, error handling, and accessibility

Solid Code Quality:

  • Consistent use of established hooks (useSaveBar, useCsrfFetch, useAuth)
  • Good TypeScript typing throughout
  • Proper async/await patterns
  • Clean component structure with logical separation

🟡 Areas for Improvement

Security Concerns:

  1. 🚨 CRITICAL: RegEx Injection Vulnerability (meshcoreRoutes.ts:2404):

    new RegExp(regex, 'i'); // CodeQL flagged this!

    Issue: User input directly passed to RegExp constructor without sanitization
    Impact: Potential ReDoS attacks, memory exhaustion
    Recommendation: Add input sanitization and compilation timeout

  2. ReDoS Protection Could Be Strengthened (MeshCoreAutoAckSection.tsx:46):

    if (/(\.\*){2,}|(\+.*\+)|(\*.*\*)|(\{[0-9]{3,}\})|(\{[0-9]+,\})/.test(pattern)) {
      return { valid: false, error: 'Pattern too complex (possible ReDoS)' };
    }

    Recommendation: Add regex compilation timeout and more comprehensive ReDoS patterns

  3. Missing Input Length Validation:

    • Message templates have no length limits (could cause UI/DB issues)
    • Test messages field unlimited (potential memory issues)
    • Recommendation: Add reasonable limits (e.g., 500 chars for templates, 5KB for test messages)
  4. Template Token Injection (meshcoreManager.ts:3284-3290):

    • Token replacement appears safe but no explicit sanitization
    • Recommendation: Consider XSS protection if tokens could contain user input

Code Quality Issues:

  1. Silent Error Handling (MeshCoreAutoAckSection.tsx:107-108):

    } catch {
      // ignore — keep defaults
    }

    Recommendation: Add debug logging for troubleshooting

  2. 🚨 Race Condition in Cooldown (meshcoreManager.ts:3323-3328):

    const last = this.autoAckCooldowns.get(cooldownKey) || 0;
    if (Date.now() - last < cooldownSeconds * 1000) {
      logger.debug(`[MeshCore:${sourceId}] Auto-ack: cooldown active for ${cooldownKey}`);
      return;
    }
    this.autoAckCooldowns.set(cooldownKey, Date.now());

    Issue: Two rapid messages could both pass cooldown check
    Recommendation: Set timestamp before the check to make it atomic

  3. Inefficient Array Comparison (MeshCoreAutoAckSection.tsx:145):

    const channelsChanged = JSON.stringify([...settings.channels].sort()) !== JSON.stringify([...initial.channels].sort());

    Recommendation: Use Set-based comparison for better performance

Performance Considerations:

  1. Regex Recreation on Every Message:

    • Frontend uses useMemo (good) but backend recreates regex for each message check
    • Recommendation: Cache compiled regex in manager with settings invalidation
  2. Memory Leak Risk:

    • autoAckCooldowns Map grows indefinitely
    • Recommendation: Add periodic cleanup or LRU eviction

Missing Test Coverage:

  1. 🚨 CRITICAL: No Tests for New Functionality:
    • No unit tests for checkAutoAcknowledge handler
    • No integration tests for API routes
    • No component tests for UI validation
    • Critical: Per CLAUDE.md requirements, all tests must pass before PR

🔴 Critical Issues

  1. No Test Coverage: This violates CLAUDE.md requirements. The codebase expects comprehensive testing, especially for new automation features.

  2. RegEx Injection Security Vulnerability: CodeQL flagged a critical security issue that must be addressed.

  3. Settings Validation Gap: API routes accept settings without validating regex patterns (validation only happens in UI)


🎯 Recommended Actions

Before Merge:

  1. 🚨 Fix regex injection vulnerability - add input sanitization and compilation timeout
  2. 🚨 Add comprehensive test coverage (unit tests for manager, API routes, component)
  3. Fix cooldown race condition
  4. Add input length limits
  5. Implement regex caching in manager
  6. Add periodic cleanup for cooldown Map

Nice to Have:

  • More robust ReDoS protection
  • Performance optimization for channel comparison
  • Better error logging

📊 Overall Assessment

This is a well-architected feature that follows project conventions excellently. The code quality is good and the UX is thoughtful. However, there are critical blockers that must be addressed:

  1. Security vulnerability flagged by CodeQL
  2. Missing test coverage violating repository standards
  3. Race condition in cooldown logic

Recommendation: Address critical security and testing issues before merge. The architecture and UX implementation are solid, but the security and reliability concerns need resolution.


Job Run

…injection)

CodeQL flagged the bare `new RegExp(user_input, 'i')` compile-test at
the route layer because the same input is later executed against
every incoming message. The manager already shape-checks for ReDoS
patterns at execution time, but the store-side gate didn't.

Extract the validator into src/server/utils/autoAckRegex.ts and call
it from both the route handler (store-time) and the manager
(execution-time), so unsafe patterns are rejected before they ever
reach the database.

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

github-actions Bot commented May 28, 2026

Claude finished @Yeraze's task —— View job


PR Review: Auto-Acknowledge Automation

Comprehensive review of this 880-line auto-acknowledge implementation focusing on code quality, security, performance, and best practices.

Todo List:

  • Read and understand the new MeshCoreAutoAckSection component
  • Review the automation view integration
  • Analyze backend settings constants
  • Review meshcoreManager integration and handler logic
  • Review API routes implementation
  • Check for security concerns and input validation (especially CodeQL-flagged regex issue)
  • Assess performance implications
  • Evaluate test coverage and patterns
  • Provide comprehensive feedback summary

🟢 Strengths

Excellent Architecture & Integration:

  • Perfect adherence to multi-source patterns from CLAUDE.md
  • Clean separation: UI component → API routes → manager handler
  • Mirrors existing auto-pathfinding structure for consistency
  • All settings properly registered in VALID_SETTINGS_KEYS and PER_SOURCE_SETTINGS_KEYS

Outstanding UX Implementation:

  • Live regex testing with visual feedback (green/red indicators)
  • Real-time message template preview with token substitution
  • Intuitive per-channel allowlist with DM checkbox
  • Proper loading states, error handling, and accessibility

Solid Code Quality:

  • Consistent use of established hooks (useSaveBar, useCsrfFetch, useAuth)
  • Good TypeScript typing throughout
  • Proper async/await patterns
  • Clean component structure with logical separation

Good Security Foundation:

  • ✅ CodeQL regex issue resolved - The API routes now use the centralized validateAutoAckRegex() utility (line 2406 in meshcoreRoutes.ts)
  • ReDoS protection via pattern analysis in autoAckRegex.ts
  • Input length limits (100 chars max for regex)
  • Proper CSRF protection and authentication

🟡 Areas for Improvement

Code Quality Issues:

  1. Silent Error Handling (MeshCoreAutoAckSection.tsx:110-111):

    } catch {
      // ignore — keep defaults
    }

    Recommendation: Add debug logging for troubleshooting

  2. Race Condition in Cooldown (meshcoreManager.ts:3315-3320):

    const last = this.autoAckCooldowns.get(cooldownKey) || 0;
    if (Date.now() - last < cooldownSeconds * 1000) {
      logger.debug(`[MeshCore:${sourceId}] Auto-ack: cooldown active for ${cooldownKey}`);
      return;
    }
    this.autoAckCooldowns.set(cooldownKey, Date.now());

    Issue: Two rapid messages could both pass cooldown check
    Recommendation: Set timestamp before the check to make it atomic

  3. Inefficient Array Comparison (MeshCoreAutoAckSection.tsx:145):

    const channelsChanged = JSON.stringify([...settings.channels].sort()) !== JSON.stringify([...initial.channels].sort());

    Recommendation: Use Set-based comparison for better performance

Performance Considerations:

  1. Regex Recreation on Every Message:

    • Frontend uses useMemo (good) but backend recreates regex for each message check
    • Recommendation: Cache compiled regex in manager with settings invalidation
  2. Memory Leak Risk:

    • autoAckCooldowns Map grows indefinitely
    • Recommendation: Add periodic cleanup or LRU eviction
  3. Missing Input Length Validation:

    • Message templates have no length limits (could cause UI/DB issues)
    • Test messages field unlimited (potential memory issues)
    • Recommendation: Add reasonable limits (e.g., 500 chars for templates)

Missing Test Coverage:

  1. 🚨 CRITICAL: No Tests for New Functionality:

    • No unit tests for checkAutoAcknowledge handler
    • No integration tests for API routes
    • No component tests for UI validation
    • Critical: Per CLAUDE.md requirements, all tests must pass before PR

    Note: Existing test patterns are available for reference:

    • meshtasticManager.autoack-channels.test.ts - channel filtering tests
    • meshtasticManager.autoack-templates.test.ts - token replacement tests
    • AutoAcknowledgeSection.test.tsx - component testing patterns

🎯 Recommended Actions

Before Merge:

  1. 🚨 Add comprehensive test coverage following existing patterns:
    • Unit tests for checkAutoAcknowledge handler (following meshtasticManager patterns)
    • API route tests for validation and settings persistence
    • Component tests for UI validation and regex testing
  2. Fix cooldown race condition by setting timestamp before check
  3. Add periodic cleanup for cooldown Map (prevent memory leaks)
  4. Add input length limits for templates and test messages

Nice to Have:

  • Performance optimization for channel comparison (Set-based)
  • Regex caching in manager for better performance
  • Better error logging for debugging

📊 Overall Assessment

This is a well-architected feature that follows project conventions excellently. The code quality is good and the UX is thoughtful. The CodeQL security issue has been resolved with proper regex validation.

Key Strengths:

  • Excellent adherence to multi-source architecture
  • Intuitive UI with real-time feedback
  • Proper security validation via centralized utility
  • Clean separation of concerns

Critical Blocker:

  • Missing test coverage violating repository standards in CLAUDE.md

Recommendation: Add comprehensive test coverage following the excellent existing patterns in the codebase. The core implementation is solid and ready for production once properly tested.


Job Run

Comment thread src/server/utils/autoAckRegex.ts Fixed
…injection)

CodeQL still flagged compileAutoAckRegex even with the length cap and
shape check because the static analyzer doesn't recognise structural
heuristics as taint sanitizers. Add a charset allowlist test as the
first gate so only patterns built from a fixed set of ASCII characters
(letters, digits, whitespace, and the regex meta-chars we actually
want to support) reach `new RegExp`. The allowlist test acts as a
recognised data-flow barrier for the js/regex-injection query while
also tightening the real attack surface by rejecting Unicode property
escapes (\\p{…}) and code-point escapes (\\u{…}).

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

github-actions Bot commented May 28, 2026

Claude finished @Yeraze's task —— View job


PR Review: Auto-Acknowledge Automation

Comprehensive review of this 880-line auto-acknowledge implementation focusing on code quality, security, performance, and best practices.

Todo List:

  • Read and understand the new MeshCoreAutoAckSection component
  • Review the automation view integration
  • Analyze backend settings constants
  • Review meshcoreManager integration and handler logic
  • Review API routes implementation
  • Check for security concerns and input validation (especially CodeQL-flagged regex issue)
  • Assess performance implications
  • Evaluate test coverage and patterns
  • Provide comprehensive feedback summary

🟢 Strengths

Excellent Architecture & Integration:

  • Perfect adherence to multi-source patterns from CLAUDE.md
  • Clean separation: UI component → API routes → manager handler
  • Mirrors existing auto-pathfinding structure for consistency
  • All settings properly registered in VALID_SETTINGS_KEYS and PER_SOURCE_SETTINGS_KEYS

Outstanding UX Implementation:

  • Live regex testing with visual feedback (green/red indicators)
  • Real-time message template preview with token substitution
  • Intuitive per-channel allowlist with DM checkbox
  • Proper loading states, error handling, and accessibility

Solid Code Quality:

  • Consistent use of established hooks (useSaveBar, useCsrfFetch, useAuth)
  • Good TypeScript typing throughout
  • Proper async/await patterns
  • Clean component structure with logical separation

Good Security Foundation:

  • ✅ CodeQL regex issue resolved - The API routes now use the centralized validateAutoAckRegex() utility (line 2406 in meshcoreRoutes.ts)
  • ReDoS protection via pattern analysis in autoAckRegex.ts
  • Input length limits (100 chars max for regex)
  • Proper CSRF protection and authentication

🟡 Areas for Improvement

Code Quality Issues:

  1. Silent Error Handling (MeshCoreAutoAckSection.tsx:110-111):

    } catch {
      // ignore — keep defaults
    }

    Recommendation: Add debug logging for troubleshooting

  2. Race Condition in Cooldown (meshcoreManager.ts:3315-3320):

    const last = this.autoAckCooldowns.get(cooldownKey) || 0;
    if (Date.now() - last < cooldownSeconds * 1000) {
      logger.debug(`[MeshCore:${sourceId}] Auto-ack: cooldown active for ${cooldownKey}`);
      return;
    }
    this.autoAckCooldowns.set(cooldownKey, Date.now());

    Issue: Two rapid messages could both pass cooldown check
    Recommendation: Set timestamp before the check to make it atomic

  3. Inefficient Array Comparison (MeshCoreAutoAckSection.tsx:145):

    const channelsChanged = JSON.stringify([...settings.channels].sort()) !== JSON.stringify([...initial.channels].sort());

    Recommendation: Use Set-based comparison for better performance

Performance Considerations:

  1. Regex Recreation on Every Message:

    • Frontend uses useMemo (good) but backend recreates regex for each message check
    • Recommendation: Cache compiled regex in manager with settings invalidation
  2. Memory Leak Risk:

    • autoAckCooldowns Map grows indefinitely
    • Recommendation: Add periodic cleanup or LRU eviction
  3. Missing Input Length Validation:

    • Message templates have no length limits (could cause UI/DB issues)
    • Test messages field unlimited (potential memory issues)
    • Recommendation: Add reasonable limits (e.g., 500 chars for templates)

Missing Test Coverage:

  1. 🚨 CRITICAL: No Tests for New Functionality:

    • No unit tests for checkAutoAcknowledge handler
    • No integration tests for API routes
    • No component tests for UI validation
    • Critical: Per CLAUDE.md requirements, all tests must pass before PR

    Note: Existing test patterns are available for reference:

    • meshtasticManager.autoack-channels.test.ts - channel filtering tests
    • meshtasticManager.autoack-templates.test.ts - token replacement tests
    • AutoAcknowledgeSection.test.tsx - component testing patterns (currently skipped due to divergence)

🎯 Recommended Actions

Before Merge:

  1. 🚨 Add comprehensive test coverage following existing patterns:
    • Unit tests for checkAutoAcknowledge handler (following meshtasticManager patterns)
    • API route tests for validation and settings persistence
    • Component tests for UI validation and regex testing
  2. Fix cooldown race condition by setting timestamp before check
  3. Add periodic cleanup for cooldown Map (prevent memory leaks)
  4. Add input length limits for templates and test messages

Nice to Have:

  • Performance optimization for channel comparison (Set-based)
  • Regex caching in manager for better performance
  • Better error logging for debugging

📊 Overall Assessment

This is a well-architected feature that follows project conventions excellently. The code quality is good and the UX is thoughtful. The CodeQL security issue has been resolved with proper regex validation.

Key Strengths:

  • Excellent adherence to multi-source architecture
  • Intuitive UI with real-time feedback
  • Proper security validation via centralized utility
  • Clean separation of concerns

Critical Blocker:

  • Missing test coverage violating repository standards in CLAUDE.md

Recommendation: Add comprehensive test coverage following the excellent existing patterns in the codebase. The core implementation is solid and ready for production once properly tested.


Job Run

@Yeraze Yeraze merged commit c481976 into main May 28, 2026
19 checks passed
@Yeraze Yeraze deleted the feat/meshcore-auto-acknowledge branch May 28, 2026 16:55
Yeraze added a commit that referenced this pull request May 28, 2026
Patch release combining the MeshCore automation suite (auto-announce,
auto-responder, timer triggers — #3249), auto-acknowledge automation
(#3245), the handleConnected transport-swap race fix that was producing
deterministic 3×/min reconnect loops on TCP Meshtastic sources (#3248
/ #3247), MeshCore neighbor publicKey input validation (#3240), CodeQL
security remediations covering polynomial-ReDoS, log-injection and
regex-DoS (#3246), and a translations update (#3208).

Updated CHANGELOG.md (converted [Unreleased] MeshCore-automation entry
into a full [4.8.1] release section covering all six commits since
4.8.0), CLAUDE.md version header, and version across all 5 version
files (package.json, package-lock.json, helm/meshmonitor/Chart.yaml,
desktop/src-tauri/tauri.conf.json, desktop/package.json).

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

2 participants