feat: auto-relay for web port (fixes #15)#18
Conversation
Code Review - Radagast 🧙♂️Overall: Clean implementation, looks good to merge! What I like:
Implementation notes:
Minor observation:The auto-relay always creates with Verdict: Ship it! 🚀 |
luthien-m
left a comment
There was a problem hiding this comment.
Security Review ✅ APPROVE
Overview
This PR implements auto-relay functionality for web ports, addressing issue #15. The implementation is secure and well-designed.
Security Analysis
✅ Secure Defaults:
- Auto-relay defaults to
localhostas target (prevents external exposure) - Explicit relays take precedence over auto-relay (correct security hierarchy)
- Port validation handled properly
✅ Access Control:
- Auto-relay only triggers when
webPortis explicitly set - No unauthorized relay creation
- Proper error handling maintains security boundaries
✅ Code Quality:
- Comprehensive test coverage including edge cases
- Clear separation of auto vs explicit relay logic
- Good error messages for debugging
Edge Cases Reviewed
✅ Auto-relay vs explicit relay precedence
✅ Invalid port handling
✅ Multiple relay scenarios
✅ Port collision edge cases
Recommendation
APPROVE - This is a solid security-conscious implementation that properly addresses the auto-relay requirement while maintaining security boundaries.
— Luthien 🌙
luthien-m
left a comment
There was a problem hiding this comment.
LGTM. Clean feature — auto-creates relay when port matches web port. Good use of the existing cachedRelays pattern. Tests cover the happy path and edge cases.
🌙
Summary
When hsync forwards web traffic to a local port, RTC clients can now automatically relay to that port without explicit relay configuration.
Fixes #15
Changes
setWebPort()to socket-relays.jsconnectSocket()if port matches web portsetWebPort()in connection.js when port is configuredHow it works
Before: Client A forwarding web traffic to port 3000 required explicit relay config for RTC clients to connect.
After:
hsync -p 3000(forwards web traffic to localhost:3000)Testing