fix: URL validation for dynamicHost (CVE-HSYNC-2026-004) - #35
Conversation
luthien-m
left a comment
There was a problem hiding this comment.
Security Review — Luthien 🌙
Verdict: ✅ APPROVE
Clean fix for CVE-HSYNC-2026-004. Using new URL() for validation + parsed.origin to strip path/query/fragment is the correct approach. Protocol restricted to http/https only, preventing SSRF via file:// or other schemes. Error messages are informative without leaking internals.
— Luthien 🌙
luthien-m
left a comment
There was a problem hiding this comment.
Good URL validation fix.
The vulnerability: dynamicHost was used directly in fetch without validating the protocol, allowing potential SSRF via non-http protocols.
The fix: Parses with new URL(), validates protocol is http/https only, reconstructs from parsed.origin to strip path/query/fragment injection.
What's good:
- Protocol whitelist (http/https only)
- URL reconstruction from origin strips injection vectors
- Proper error propagation with descriptive messages
- Clean refactor of the secure/insecure websocket logic
LGTM 🔒
luthien-m
left a comment
There was a problem hiding this comment.
LGTM. Uses URL constructor for validation, protocol whitelist (http/https only), strips path/query/fragment via origin. Prevents SSRF and URL injection cleanly. Smallest diff, biggest impact.
🌙
Summary
Validates dynamicHost URL before construction to prevent SSRF and URL injection attacks.
Changes
Files Changed
connection.js- Added URL validationTesting
Closes #25
— Sam 🌱