4f2430d354
- Fixed URL whitelist to use proper domain boundary matching - Added WhitelistTest.kt with 6 test cases - Added IMPROVEMENTS.md with code review notes Security fix identified by Spirit 👻 Co-authored-by: Spirit <spirit@hobbs.farm> Co-authored-by: Margaret <margaret@hobbs.farm>
1.2 KiB
1.2 KiB
Improvements & Code Review Notes
Security Fixes Applied
P0: Whitelist Bypass Vulnerability (FIXED)
Issue: Original whitelist check used url.contains(domain) which would allow bypass:
https://hobbs.farm.evil.comwould matchhobbs.farm❌
Fix: Use proper domain boundary matching:
val host = Uri.parse(url).host?.lowercase() ?: return false
return whitelist.any { domain ->
host == domain || host.endsWith(".$domain")
}
Recommended Future Improvements
P1: PIN Storage Security
- Current: Plain text in DataStore
- Recommended: SHA-256 hash with salt
- Implementation: Use
MessageDigestto hash before storing
P2: UX Enhancements
- Add loading indicator (ProgressBar) during page loads
- Add network error screen with retry button
- Consider swipe-to-refresh in admin settings
P2: Code Quality
- Add unit tests for whitelist logic (added: WhitelistTest.kt)
- Add instrumented tests for WebView behavior
- Consider Kotlin coroutines for async operations
Test Coverage
| Component | Tests | Status |
|---|---|---|
| Whitelist matching | 6 | ✅ |
| Idle timer | TBD | ⏳ |
| Admin PIN | TBD | ⏳ |
| Boot receiver | TBD | ⏳ |
Code review by Spirit 👻 Coordinated by Pluto 🪐