🔒 Enforce HTTPS by disabling cleartext traffic - #227
Conversation
🎯 What: Disabling global cleartext traffic support in the application.⚠️ Risk: Allowing cleartext traffic exposes users to man-in-the-middle (MITM) attacks, potentially leading to data interception or injection. 🛡️ Solution: Set android:usesCleartextTraffic="false" in the manifest and added a network_security_config.xml to strictly limit cleartext traffic to 127.0.0.1 for the local dashboard WebView. Co-authored-by: Asutorufa <16442314+Asutorufa@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
There was a problem hiding this comment.
Code Review
This pull request enhances network security by disabling global cleartext traffic in the Android manifest and introducing a dedicated network security configuration. Feedback was provided regarding the network_security_config.xml file, specifically noting that the element does not support IP addresses and that a is required to ensure cleartext traffic is restricted on older Android versions (API 24-27).
| <network-security-config> | ||
| <domain-config cleartextTrafficPermitted="true"> | ||
| <domain includeSubdomains="false">127.0.0.1</domain> | ||
| </domain-config> | ||
| </network-security-config> |
There was a problem hiding this comment.
The Network Security Configuration has two issues that prevent it from fully achieving the pull request's security goals and may break local functionality:
- IP Address in Domain: The
<domain>element does not support IP addresses (e.g.,127.0.0.1). It only supports hostnames. If the application accesses the local dashboard viahttp://127.0.0.1, it will be blocked because this rule will not match. You should uselocalhostinstead and ensure the application refers to the local service viahttp://localhost. - Missing Base Config: On Android 7.0 to 8.1 (API 24-27), the presence of a network security configuration file overrides the
android:usesCleartextTraffic="false"flag in the manifest. Since no<base-config>is defined here, cleartext traffic will be permitted by default on those versions. Adding an explicit<base-config cleartextTrafficPermitted="false" />is necessary to enforce the policy globally across all supported Android versions.
Reference: Android Network Security Configuration
| <network-security-config> | |
| <domain-config cleartextTrafficPermitted="true"> | |
| <domain includeSubdomains="false">127.0.0.1</domain> | |
| </domain-config> | |
| </network-security-config> | |
| <network-security-config> | |
| <base-config cleartextTrafficPermitted="false" /> | |
| <domain-config cleartextTrafficPermitted="true"> | |
| <domain includeSubdomains="false">localhost</domain> | |
| </domain-config> | |
| </network-security-config> |
🎯 What: Disabling global cleartext traffic support in the application.
⚠️ Risk: Allowing cleartext traffic exposes users to man-in-the-middle (MITM) attacks, potentially leading to data interception or injection.
🛡️ Solution: Set
android:usesCleartextTraffic="false"in the manifest and added anetwork_security_config.xmlto strictly limit cleartext traffic to127.0.0.1for the local dashboard WebView.PR created automatically by Jules for task 13859112245081247526 started by @Asutorufa