🔒 [security fix] Fix WebView JavascriptInterface vulnerability - #194
🔒 [security fix] Fix WebView JavascriptInterface vulnerability#194alvin000009238 wants to merge 1 commit into
Conversation
Co-authored-by: alvin000009238 <107313913+alvin000009238@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. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request replaces the insecure addJavascriptInterface with WebViewCompat.addWebMessageListener to restrict WebView communication to trusted origins, enhancing security. The review feedback points out a potential bug where the onLoginSuccess lambda is captured inside the AndroidView's factory block, which could lead to stale callback execution during recompositions, and suggests using rememberUpdatedState to resolve it.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| loginHandled = true | ||
| val cookieString = CookieManager.getInstance() | ||
| .getCookie("https://$SCHOOL_DOMAIN") ?: "" | ||
| post { onLoginSuccess(studentNo, cookieString) } |
There was a problem hiding this comment.
The onLoginSuccess lambda is captured inside the AndroidView's factory block, which only runs once when the WebView is created. If onLoginSuccess changes during recomposition (for example, if the parent composable recomposes with a new callback instance), the WebView will still invoke the stale version of the lambda captured during the initial creation.\n\nTo prevent this stale lambda capture bug, you should use rememberUpdatedState to keep a reference to the latest lambda.\n\nSince the top of WebViewContent is outside the modified diff hunks, you can apply this fix by adding the following at the beginning of WebViewContent (around line 193):\n\nkotlin\nval currentOnLoginSuccess by rememberUpdatedState(onLoginSuccess)\n\n\nAnd then update line 237 to use currentOnLoginSuccess:\n\nkotlin\npost { currentOnLoginSuccess(studentNo, cookieString) }\n
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a8382f6b54
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| val cookieString = CookieManager.getInstance() | ||
| .getCookie("https://$SCHOOL_DOMAIN") ?: "" | ||
| post { onLoginSuccess(studentNo, cookieString) } | ||
| if (WebViewFeature.isFeatureSupported(WebViewFeature.WEB_MESSAGE_LISTENER)) { |
There was a problem hiding this comment.
On devices whose installed WebView provider does not support WEB_MESSAGE_LISTENER, this branch simply skips registering any native bridge while the injected login hook now only calls window.AndroidLogin.postMessage(...). In that environment the school login can succeed in the WebView, but onLoginSuccess is never invoked, leaving users stuck on the login screen with no error or recovery. Please surface an update/unsupported-WebView flow or provide a safe fallback for this case.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Replace the insecure WebView JavaScript bridge with an origin-restricted messaging API to mitigate addJavascriptInterface abuse.
Changes:
- Replaced
addJavascriptInterface+@JavascriptInterfacebridge withWebViewCompat.addWebMessageListenergated byWebViewFeature.WEB_MESSAGE_LISTENER. - Updated injected JS payload to use
window.AndroidLogin.postMessage(loginId)for communication. - Added
androidx.webkit:webkitdependency to support WebViewCompat APIs.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pr_details.txt | Adds a textual summary of the PR’s security rationale and approach. |
| android/app/src/main/java/com/clhs/score/ui/WebViewLoginScreen.kt | Switches WebView bridge from addJavascriptInterface to origin-restricted messaging and updates JS hook accordingly. |
| android/app/build.gradle.kts | Adds the AndroidX WebKit dependency required for WebViewCompat / WebViewFeature. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (WebViewFeature.isFeatureSupported(WebViewFeature.WEB_MESSAGE_LISTENER)) { | ||
| WebViewCompat.addWebMessageListener( | ||
| this, | ||
| "AndroidLogin", | ||
| setOf("https://$SCHOOL_DOMAIN"), | ||
| ) { _, message, _, _, _ -> | ||
| val studentNo = message.data | ||
| if (studentNo != null) { | ||
| if (loginHandled || !isTrustedLoginPage) return@addWebMessageListener | ||
| loginHandled = true | ||
| val cookieString = CookieManager.getInstance() | ||
| .getCookie("https://$SCHOOL_DOMAIN") ?: "" | ||
| post { onLoginSuccess(studentNo, cookieString) } | ||
| } | ||
| } | ||
| } | ||
| addJavascriptInterface(jsInterface, "AndroidLogin") | ||
|
|
🎯 What: Replaced insecure
addJavascriptInterfacewith secureWebViewCompat.addWebMessageListener.addJavascriptInterfacedoes not allow specifying an origin, making it inherently vulnerable.🛡️ Solution: Used
WebViewCompat.addWebMessageListenerfromandroidx.webkit:webkit. This modern API restricts access to the injected JavaScript object only to the specific, trusted origins ("https://shcloud2.k12ea.gov.tw"). The JavaScript payload was updated to usewindow.AndroidLogin.postMessageto communicate securely.PR created automatically by Jules for task 4970981808506226386 started by @alvin000009238