fix: harden security across HTTP capabilities, SSRF, crypto, and SQL#245
Open
atani wants to merge 1 commit intoavihaymenahem:mainfrom
Open
fix: harden security across HTTP capabilities, SSRF, crypto, and SQL#245atani wants to merge 1 commit intoavihaymenahem:mainfrom
atani wants to merge 1 commit intoavihaymenahem:mainfrom
Conversation
- Restrict HTTP capability: block plain HTTP except localhost AI servers (Ollama:11434, LM Studio:1234), allow all HTTPS - Prevent SSRF via List-Unsubscribe: add isSafeUrl() guard blocking loopback, private IPv4, IPv6 private ranges (fc/fd, fe80, ::ffff mapped), and disable redirect following - Fix SQL injection in compactQueue: parameterize accountId - Harden decryption: throw on failure instead of silent plaintext fallback, use Promise.allSettled so one corrupted account doesn't block all - Add OAuth token_url whitelist in Rust backend with prefix matching for Microsoft tenant GUIDs - Improve phishing detection: support ccTLDs (.co.uk, .co.jp, etc.) in domain comparison - Add 19 tests for isSafeUrl and 4 tests for ccTLD phishing detection
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hi there!
First off — Velo is an incredibly well-built email client. The architecture is clean, the Tauri integration is solid, and the codebase is remarkably consistent across 130+ test files. Really impressive work.
I ran a security audit while evaluating Velo for personal use and found a few areas where the already-strong security posture could be tightened further. This PR addresses the findings with minimal, focused changes.
Summary
7 targeted security hardening fixes across the frontend and Rust backend:
isSafeUrl()guard blocks requests to loopback, private IPv4/IPv6 ranges (including::ffff:mapped addresses and cloud metadata169.254.x.x), with redirect following disabledaccountIdincompactQueue()(the only place in the codebase that used string interpolation — everything else was already properly parameterized)decryptField()now throws on failure instead of silently falling back to raw values, preventing potential credential tampering.getAllAccounts()usesPromise.allSettledso one corrupted account doesn't block the restgetRegistrableDomain()now handles 38 multi-part ccTLDs (.co.uk,.co.jp,.com.au, etc.) to reduce false positives/negatives in brand impersonation detectionTest plan
cargo checkpassesisSafeUrl()— IPv4/IPv6 private ranges, loopback, schemes, mapped addresses.co.uk,.co.jp,.com.au)