feat: dead nodes report with bulk delete on Security tab

[Yeraze]

Summary

New "Dead Nodes" section on the Security tab showing nodes not heard from in 7+ days, with bulk delete capability.

Features

• Table showing name, ID, hardware model, last heard (relative time), and local/device DB indicator
• Checkbox selection per node with Select All / Deselect All
• "Delete X node(s)" button removes selected nodes from both local DB and device NodeDB
• Confirmation dialog before deletion
• Excludes local node, broadcast address, and ignored nodes
• Audit logged for accountability

New API Endpoints

• GET /api/security/dead-nodes — returns nodes not heard in 7+ days with device DB status
• POST /api/security/dead-nodes/bulk-delete — batch delete with device NodeDB removal

Changes

File	Change
src/server/routes/securityRoutes.ts	Two new endpoints for dead nodes
src/components/SecurityTab.tsx	Dead nodes UI section with checkbox table and bulk delete

Test plan

• npx vitest run — 3070 tests pass
• npm run build — no TypeScript errors
• Deploy on SQLite, verify dead nodes list appears
• Verify broadcast node excluded
• Test bulk delete with confirmation

🤖 Generated with Claude Code

[github-actions]

Claude finished @Yeraze's task —— View job

---

PR Review: Dead Nodes Report Feature

Reviewing the dead nodes report implementation for code quality, security, performance, and potential issues.

Todo List

• Read and analyze SecurityTab.tsx changes
• Read and analyze securityRoutes.ts changes
• Review API security and validation
• Check UI implementation and UX
• Assess performance implications
• Provide comprehensive feedback

---

Overall Assessment

This is a well-implemented feature that adds valuable dead node cleanup functionality to the Security tab. The code follows established patterns and includes proper security measures.

✅ Strengths

Code Quality

• Consistent patterns: Follows existing codebase conventions for API routes, UI components, and error handling
• Type safety: Proper TypeScript interfaces and typing throughout
• Good separation: Clean separation between UI logic and API endpoints
• Proper error handling: Comprehensive error handling with user feedback

Security

• Permission checks: Properly implements requirePermission('security', 'read') for API access
• Input validation: Validates nodeNums array in bulk delete endpoint
• Audit logging: Includes audit trail for delete operations with user context
• Safe filtering: Excludes local node, broadcast address, and ignored nodes from deletion

UX/UI Design

• Clear interface: Well-designed table with checkbox selection and bulk actions
• User feedback: Loading states, confirmation dialogs, and clear status indicators
• Accessibility: Proper button states, tooltips, and visual hierarchy
• Responsive design: Uses established CSS patterns and flex layouts

⚠️ Areas for Improvement

API Routes (securityRoutes.ts:384-439)

Missing Write Permission Check

router.post('/dead-nodes/bulk-delete', async (req: Request, res: Response) => {

The bulk delete endpoint lacks explicit write permission validation. Should add:

router.post('/dead-nodes/bulk-delete', requirePermission('security', 'write'), async (req: Request, res: Response) => {

Input Validation
The nodeNums validation at line 389 could be more robust:

// Current
if (!Array.isArray(nodeNums) || nodeNums.length === 0) {
  return res.status(400).json({ error: 'nodeNums must be a non-empty array' });
}

// Suggested improvement
if (!Array.isArray(nodeNums) || nodeNums.length === 0 || nodeNums.length > 100) {
  return res.status(400).json({ error: 'nodeNums must be a non-empty array (max 100)' });
}

// Validate each nodeNum is a valid number
for (const nodeNum of nodeNums) {
  if (!Number.isInteger(Number(nodeNum)) || Number(nodeNum) <= 0) {
    return res.status(400).json({ error: 'All nodeNums must be positive integers' });
  }
}

Error Recovery
The individual node deletion loop (lines 395-419) continues on failure, which is good, but consider adding a summary of partial failures to the response.

UI Component (SecurityTab.tsx:779-873)

Performance Considerations
The deadNodes.map(n => n.nodeNum) call in toggleAllDeadNodes (line 291) recreates the array on every toggle. Consider memoizing:

const allDeadNodeNums = useMemo(() => deadNodes.map(n => n.nodeNum), [deadNodes]);

Error Handling Enhancement
The error handling in handleBulkDeleteDeadNodes (line 308) only logs to console. Consider showing user feedback:

} catch (err) {
  logger.error('Error bulk deleting dead nodes:', err);
  setError(err instanceof Error ? err.message : t('security.failed_delete_nodes'));
} finally {

Accessibility
The table at line 826 could benefit from ARIA labels and better keyboard navigation support.

📊 Performance Assessment

Database Queries

• Dead nodes query uses getAllNodes() which is appropriate for the filtering logic
• Efficient filtering with multiple exclusion criteria
• Results sorted appropriately (oldest first)

UI Rendering

• Uses React best practices with proper state management
• Checkbox operations are efficient with Set operations
• Table rendering scales reasonably for expected dead node counts

🔒 Security Assessment

Access Control: ✅ Proper permission checks
Input Validation: ⚠️ Could be more robust (see suggestions above)
Audit Trail: ✅ Comprehensive logging
Data Exposure: ✅ No sensitive data leaked
CSRF Protection: ✅ Uses existing middleware patterns

🧪 Test Coverage

The PR description mentions 3070 tests passing, but specific test coverage for the new endpoints would be valuable. Consider adding:

• Unit tests for the dead nodes filtering logic
• API endpoint tests for both success and error cases
• UI component tests for selection and bulk operations

📝 Minor Suggestions

1. Constants: Consider extracting the 7-day threshold to a configurable setting
2. Translations: All UI strings use translation keys - excellent i18n support
3. Loading States: The isDeletingNodes state provides good UX feedback
4. Responsive Design: Table layout may need mobile responsive considerations

Summary

This is a solid feature implementation that follows the codebase patterns well. The main improvement needed is adding the missing write permission check to the bulk delete endpoint. The code quality is high and the security approach is sound.

Recommendation: Approve after addressing the permission check issue.

[github-actions]

Claude finished @Yeraze's task —— View job

---

PR Review: Dead Nodes Report Feature

Reviewing the dead nodes report implementation for code quality, security, performance, and potential issues.

Todo List

• Read and analyze SecurityTab.tsx changes
• Read and analyze securityRoutes.ts changes
• Review API security and validation
• Check UI implementation and UX
• Assess performance implications
• Provide comprehensive feedback

---

Overall Assessment

This is a well-implemented feature that adds valuable dead node cleanup functionality to the Security tab. The code follows established patterns and includes proper security measures.

✅ Strengths

Code Quality

• Consistent patterns: Follows existing codebase conventions for API routes, UI components, and error handling
• Type safety: Proper TypeScript interfaces and typing throughout
• Good separation: Clean separation between UI logic and API endpoints
• Proper error handling: Comprehensive error handling with user feedback

Security

• Permission checks: Properly implements requirePermission('security', 'read') for GET API access
• Write permissions: Bulk delete endpoint correctly uses requirePermission('security', 'write') at securityRoutes.ts:384
• Input validation: Validates nodeNums array in bulk delete endpoint
• Audit logging: Includes audit trail for delete operations with user context
• Safe filtering: Excludes local node, broadcast address, and ignored nodes from deletion

UX/UI Design

• Clear interface: Well-designed table with checkbox selection and bulk actions
• User feedback: Loading states, confirmation dialogs, and clear status indicators
• Accessibility: Proper button states, tooltips, and visual hierarchy
• Responsive design: Uses established CSS patterns and flex layouts

⚠️ Areas for Improvement

API Routes (securityRoutes.ts:384-439)

Input Validation Enhancement
The nodeNums validation at line 389 could be more robust:

// Current
if (!Array.isArray(nodeNums) || nodeNums.length === 0) {
  return res.status(400).json({ error: 'nodeNums must be a non-empty array' });
}

// Suggested improvement
if (!Array.isArray(nodeNums) || nodeNums.length === 0 || nodeNums.length > 100) {
  return res.status(400).json({ error: 'nodeNums must be a non-empty array (max 100)' });
}

// Validate each nodeNum is a valid number
for (const nodeNum of nodeNums) {
  if (!Number.isInteger(Number(nodeNum)) || Number(nodeNum) <= 0) {
    return res.status(400).json({ error: 'All nodeNums must be positive integers' });
  }
}

Error Recovery Summary
The individual node deletion loop (lines 395-419) continues on failure, which is good, but consider adding a summary of partial failures to the response for better user feedback.

UI Component (SecurityTab.tsx:779-873)

Performance Considerations
The deadNodes.map(n => n.nodeNum) call in toggleAllDeadNodes (line 291) recreates the array on every toggle. Consider memoizing:

const allDeadNodeNums = useMemo(() => deadNodes.map(n => n.nodeNum), [deadNodes]);

Error Handling Enhancement
The error handling in handleBulkDeleteDeadNodes (line 308) only logs to console. Consider showing user feedback:

} catch (err) {
  logger.error('Error bulk deleting dead nodes:', err);
  setError(err instanceof Error ? err.message : t('security.failed_delete_nodes'));
} finally {

Accessibility
The table at line 826 could benefit from ARIA labels and better keyboard navigation support for improved accessibility.

📊 Performance Assessment

Database Queries

• Dead nodes query uses getAllNodes() which is appropriate for the filtering logic
• Efficient filtering with multiple exclusion criteria
• Results sorted appropriately (oldest first)

UI Rendering

• Uses React best practices with proper state management
• Checkbox operations are efficient with Set operations
• Table rendering scales reasonably for expected dead node counts

🔒 Security Assessment

Access Control: ✅ Proper permission checks
Input Validation: ⚠️ Could be more robust (see suggestions above)
Audit Trail: ✅ Comprehensive logging
Data Exposure: ✅ No sensitive data leaked
CSRF Protection: ✅ Uses existing middleware patterns

🧪 Test Coverage

The PR description mentions 3070 tests passing, but specific test coverage for the new endpoints would be valuable. Consider adding:

• Unit tests for the dead nodes filtering logic
• API endpoint tests for both success and error cases
• UI component tests for selection and bulk operations

📝 Minor Suggestions

1. Constants: Consider extracting the 7-day threshold to a configurable setting
2. Translations: All UI strings use translation keys - excellent i18n support
3. Loading States: The isDeletingNodes state provides good UX feedback
4. Responsive Design: Table layout may need mobile responsive considerations

Summary

This is a solid feature implementation that follows the codebase patterns well. The security implementation is sound with proper permission checks and audit logging. The main improvements would be enhancing input validation and adding better error feedback to users.

Recommendation: Approve with minor suggestions for input validation improvements.