fix(security): per-channel write gate on channel mutators (MM-SEC-4)

[Yeraze]

Summary

• Five legacy channel-mutator endpoints gated on a static channel_0:read|write permission while operating on the URL's :id. A user with channel_0:write could mutate any channel — privilege escalation between authenticated users that bypassed the per-channel permission model.
• Switches each endpoint to requireAuth() + per-row hasPermission(req.user, 'channel_${id}', ...) using the URL's actual id. Admins always pass.

Background

Reported as MM-SEC-4 in docs/security/2026-05-05-mm-sec-1-2-3-review.md (the follow-up flagged in the MM-SEC-2 advisory). Not anonymous-exploitable — anonymous defaults don't grant write — but it broke the per-channel permission model the rest of the codebase honors.

Endpoints

Endpoint	Old gate	New gate
GET /api/channels/:id/export	channel_0:read	channel_${id}:read
PUT /api/channels/:id	channel_0:write	channel_${id}:write
DELETE /api/channels/:id	channel_0:write	channel_${id}:write
POST /api/channels/:slotId/import	channel_0:write	channel_${slotId}:write
POST /api/channels/reorder	channel_0:write	channel_${i}:write for every moved slot

For reorder: the affected-slot set is the union of source and destination indices for any pair where newOrder[i] !== i. The caller must hold write permission for every slot in that set. Permutations are cycle-closed, so the symmetric check is equivalent — the explicit union is clearer.

Files

• src/server/server.ts — five endpoint patches.

Test plan

• npx tsc --noEmit clean
• CI suite green
• Manual: as a user with only channel_2:write, PUT /api/channels/2 succeeds; PUT /api/channels/0 returns 403
• Manual: same user attempts /api/channels/reorder swapping slots 0 and 1 — 403 (lacks channel_0:write); swapping slots 2 and 3 still succeeds (assuming both are granted)
• Manual: as admin, all five endpoints behave as before

Behavior change

Users who previously relied on having channel_0:write to manage every channel will now need explicit per-channel grants. This matches the documented per-channel permission model and the existing v1 API behavior. Operators may need to add per-channel writes for accounts that were previously over-privileged via channel_0.

Out of scope

• Tests are not added; the five sites live in the server.ts monolith. The fix is a straight pattern application of the per-row check that the v1 channels routes already use and have coverage for.

🤖 Generated with Claude Code

[github-actions]

Claude finished @Yeraze's task —— View job

---

Pull Request Review

Review Tasks

• Analyze the code changes in src/server/server.ts
• Evaluate the security fix implementation
• Check for potential bugs or edge cases
• Review code quality and best practices
• Assess performance considerations
• Evaluate test coverage concerns
• Provide comprehensive feedback

Overall Assessment

This is an excellent security fix that properly addresses MM-SEC-4 privilege escalation. The implementation is sound, well-documented, and follows established patterns in the codebase.

Security Analysis ✅

Strong Points:

• Correctly fixes privilege escalation: Replaces static channel_0:write checks with per-channel channel_${id}:write validation
• Comprehensive coverage: All 5 vulnerable endpoints properly secured
• Admin bypass preserved: req.user?.isAdmin correctly bypasses permission checks
• Granular permissions: Each endpoint checks exactly the channels it affects

Reorder endpoint logic is particularly well-designed:

• Builds affectedSlots set containing both source and destination indices for any changed position
• Requires write permission for every affected channel slot
• Early returns on identity operations (performance optimization)

Code Quality ✅

Excellent implementation:

• Consistent pattern: All endpoints follow identical permission check structure
• Clear error messages: Detailed 403 responses with required permissions and helpful context
• Proper TypeScript: Correct type assertions for ResourceType
• Good documentation: Inline comments explaining the MM-SEC-4 context

Error handling example:

return res.status(403).json({
  error: 'Insufficient permissions',
  code: 'FORBIDDEN', 
  required: { resource: channelResource, action: 'write' },
  message: `Write access required for channel ${channelId}`
});

Performance Considerations ✅

Efficient implementation:

• Minimal overhead: Single hasPermission() call per affected channel
• Early validation: Input validation before expensive permission checks
• Smart reorder logic: Only checks permissions for channels that actually move

The affectedSlots algorithm is optimal - O(n) where n=8, avoiding redundant permission checks.

Edge Cases & Potential Issues ⚠️

Minor considerations:

1. Error message consistency: Most endpoints use Write access required for channel ${id}, but reorder uses a longer message. Consider standardizing.

2. Input validation order: All endpoints properly validate channel IDs before permission checks, which is correct.

3. Type safety: The as import('../types/permission.js').ResourceType assertion is safe since channel IDs are validated as 0-7.

Test Coverage Assessment ⚠️

Understanding the test gap:

• Author correctly notes that these endpoints live in the monolithic server.ts
• The permission check pattern is well-tested in v1 API endpoints
• Recommendation: While integration tests aren't required for this fix, consider adding basic permission test cases when refactoring these endpoints out of the monolith

Suggested manual test cases (from PR description are comprehensive):

# Test per-channel enforcement
curl -X PUT /api/channels/2 -d '{"name":"test"}' # Should succeed with channel_2:write
curl -X PUT /api/channels/0 -d '{"name":"test"}' # Should fail with 403

# Test reorder permissions  
# User with only channel_2:write tries to swap 0<->1: should fail
# User with only channel_2:write tries to swap 2<->3: should succeed

Recommendations

✅ Approve with confidence

• Security vulnerability properly addressed
• Implementation follows established patterns
• Code quality is high
• Performance impact is minimal

📝 Future considerations

1. Extract permission constants: Consider centralizing permission check patterns
2. Integration tests: Add test coverage during future refactoring
3. Audit logging: Consider logging permission failures for security monitoring

Behavior Change Impact ⚠️

The breaking change is intentional and necessary:

• Users with channel_0:write can no longer modify all channels
• This matches the documented permission model and existing v1 API behavior
• Operators may need to grant explicit per-channel permissions
• Trade-off is worth it for proper security

Summary

LGTM 🚀 This is a well-implemented security fix that:

• Eliminates the privilege escalation vulnerability
• Maintains backward compatibility where appropriate
• Follows existing code patterns and conventions
• Includes clear documentation and error handling

The code quality is excellent and the security fix is comprehensive. Ready to merge pending CI validation.

---

[github-actions]

System Test Results

MeshMonitor System Test Results

Test Run: 2026-05-05 20:25:58 EDT

Test Summary

Test Suite	Result
Configuration Import	✅ PASSED
Quick Start Test	✅ PASSED
Security Test	✅ PASSED
V1 API Test	✅ PASSED
Reverse Proxy Test	✅ PASSED
Reverse Proxy + OIDC	✅ PASSED
Virtual Node CLI Test	✅ PASSED
Backup & Restore Test	✅ PASSED
Database Migration Test	✅ PASSED
DB Backing Consistency	✅ PASSED
API Exercise (3 DBs)	✅ PASSED

✅ Overall Result: PASSED

All deployment configurations are working correctly!

Test Details

Configuration Import:

• Tests configuration import and device reboot cycle
• Verifies channel roles, PSKs, and LoRa configuration
• Note: Channel name verification skipped due to architectural limitation

Quick Start Test:

• Zero-config deployment (no SESSION_SECRET or COOKIE_SECURE required)
• HTTP access without HSTS
• Auto-generated admin user with default credentials
• Session cookies work over HTTP
• Meshtastic node connection and message exchange verified

Security Test:

• Verifies Node IP address hidden from anonymous users in API responses
• Verifies MQTT configuration hidden from anonymous users
• Verifies Node IP address visible to authenticated users
• Verifies MQTT configuration visible to authenticated users
• Verifies protected endpoints require authentication

V1 API Test:

• Tests v1 REST API endpoints with Bearer token authentication
• Verifies Bearer token requests bypass CSRF protection
• Verifies POST/PUT/DELETE work without CSRF token when using Bearer auth
• Verifies session-based requests still require CSRF token

Reverse Proxy Test:

• Production deployment with COOKIE_SECURE=true
• HTTPS-ready configuration
• Trust proxy enabled for reverse proxy compatibility
• CORS configured for HTTPS domain
• Meshtastic node connection and message exchange verified

Reverse Proxy + OIDC Test:

• OIDC authentication integration
• Mock OIDC provider health checks
• Authorization flow and session creation
• Hybrid mode (OIDC + local auth)
• Meshtastic node connection verified

Virtual Node CLI Test:

• Virtual Node Server enabled on TCP port 4404
• Meshtastic Python client successfully connects
• Node data download and synchronization verified
• Test message sent on gauntlet channel (index 3)
• Message delivery confirmed via Web UI API
• Virtual Node Server connection logging verified

Backup & Restore Test:

• System backup created from running dev container
• New container spun up with RESTORE_FROM_BACKUP env var
• Data integrity verified (node count, message count, settings)
• Restore event logged in audit log
• Dev container unaffected by restore test

Database Migration Test:

• SQLite to PostgreSQL migration verified
• SQLite to MySQL migration verified
• Data integrity confirmed for both target databases
• Row counts match between source and target

DB Backing Consistency Test:

• SQLite, PostgreSQL, and MySQL backends tested with same device
• Node counts within ±10 across all three backends
• Favorite counts identical across all backends
• Key station verified as favorite on all backends