Skip to content

Filter out messages not coming from the frontend external bus#6680

Merged
jpelgrom merged 5 commits intomainfrom
feature/only_allow_main_frame
Apr 11, 2026
Merged

Filter out messages not coming from the frontend external bus#6680
jpelgrom merged 5 commits intomainfrom
feature/only_allow_main_frame

Conversation

@TimoPtr
Copy link
Copy Markdown
Member

@TimoPtr TimoPtr commented Apr 7, 2026

Summary

We only want to react on messages sent from the frontend. This PR only concern the WebViewActivity and not the FrontendScreen (this is going to be done in a second PR). We've introduced a new externalAppV2 to be able to use the WebViewFeature.WEB_MESSAGE_LISTENER that allow us to properly check the origin of the message, if not available we simply fallback to the old JS injection.

I decided also to move the JS function that we inject into the externalBus, the reason behind this change is that in the frontend we can add this message in the codebase as well so that it never conflict with the one we are injecting like handleBlob.

The JS interface is now added/removed each time we load the URL instead of directly from the onCreate it allows us to properly check the server version.

This requires home-assistant/frontend#51446 to be merged. If it not merged by Friday we will need to update the min version for the support of the V2.

Checklist

  • New or updated tests have been added to cover the changes following the testing guidelines.
  • The code follows the project's code style and best_practices.
  • The changes have been thoroughly tested, and edge cases have been considered.
  • Changes are backward compatible whenever feasible. Any breaking changes are documented in the changelog for users and/or in the code for developers depending on the relevance.

Any other notes

@TimoPtr TimoPtr force-pushed the feature/only_allow_main_frame branch from 97203fd to 2b59c35 Compare April 7, 2026 11:00
@TimoPtr TimoPtr requested a review from jpelgrom April 7, 2026 11:11
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 7, 2026

Test Results

  213 files    221 suites   9m 32s ⏱️
1 688 tests 1 688 ✅ 0 💤 0 ❌
1 751 runs  1 751 ✅ 0 💤 0 ❌

Results for commit 2b59c35.

♻️ This comment has been updated with latest results.

@TimoPtr TimoPtr marked this pull request as ready for review April 7, 2026 11:50
Copilot AI review requested due to automatic review settings April 7, 2026 11:50
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates WebViewActivity’s native↔frontend bridge so the app only reacts to messages originating from the currently loaded Home Assistant frontend, using WebViewFeature.WEB_MESSAGE_LISTENER (externalAppV2) when available and falling back to the legacy JS interface (externalApp) otherwise.

Changes:

  • Added a new externalAppV2 bridge using WebViewCompat.addWebMessageListener with main-frame + same-origin filtering.
  • Refactored legacy externalApp handling into shared handlers (handleGetExternalAuth, handleRevokeExternalAuth, handleExternalBusMessage).
  • Moved blob-download and theme-change callbacks to flow through the external bus and re-register the bridge on URL load.

Comment thread app/src/main/kotlin/io/homeassistant/companion/android/webview/WebViewActivity.kt Outdated
Comment on lines +905 to +908
Timber.d("External bus $message")
webView.post {
val json = message.toJsonObjectOrNull() ?: return@post
handleExternalBusMessage(json)
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Timber.d("External bus $message") / Timber.d("External bus $payload") logs the entire bus message. These payloads can contain sensitive user/server data; consider logging only the message type/id (and use sensitive(...) if any values are needed) to avoid leaking data and reduce log noise.

Suggested change
Timber.d("External bus $message")
webView.post {
val json = message.toJsonObjectOrNull() ?: return@post
handleExternalBusMessage(json)
val json = message.toJsonObjectOrNull()
val messageType = json?.getStringOrNull("type") ?: "unknown"
val messageId = json?.getStringOrNull("id")
if (messageId != null) {
Timber.d("External bus type=$messageType id=${sensitive(messageId)}")
} else {
Timber.d("External bus type=$messageType")
}
webView.post {
val parsedJson = json ?: return@post
handleExternalBusMessage(parsedJson)

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jpelgrom It was already logging the payload before, lemme know what you think. It might be enough to display the ID and type.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be nice to limit logging here to be safe, but I'd still prefer the full message in debug builds

Comment thread app/src/main/kotlin/io/homeassistant/companion/android/webview/WebViewActivity.kt Outdated
Copy link
Copy Markdown
Member

@jpelgrom jpelgrom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice improvement to use the web message listener.

This PR only concern the WebViewActivity and not the FrontendScreen (this is going to be done in a second PR).

Do you intend to build on this and extract some code/constants from the activity, or duplicate code?

Comment thread app/src/main/kotlin/io/homeassistant/companion/android/webview/WebViewActivity.kt Outdated
Comment thread app/src/main/kotlin/io/homeassistant/companion/android/webview/WebViewActivity.kt Outdated
Comment thread app/src/main/kotlin/io/homeassistant/companion/android/webview/WebViewActivity.kt Outdated
Comment thread app/src/main/kotlin/io/homeassistant/companion/android/webview/WebViewActivity.kt Outdated
@TimoPtr TimoPtr requested a review from jpelgrom April 8, 2026 07:48
@TimoPtr
Copy link
Copy Markdown
Member Author

TimoPtr commented Apr 8, 2026

Nice improvement to use the web message listener.

This PR only concern the WebViewActivity and not the FrontendScreen (this is going to be done in a second PR).

Do you intend to build on this and extract some code/constants from the activity, or duplicate code?

I'm going to change things within the WebViewActivity to reuse some const, but the logic is a bit different in the FrontendScreen.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

Comment thread app/src/main/kotlin/io/homeassistant/companion/android/webview/WebViewActivity.kt Outdated
Comment thread app/src/main/kotlin/io/homeassistant/companion/android/webview/WebViewActivity.kt Outdated
Comment thread app/src/main/kotlin/io/homeassistant/companion/android/webview/WebViewActivity.kt Outdated
Comment thread app/src/main/kotlin/io/homeassistant/companion/android/webview/WebViewActivity.kt Outdated
Copy link
Copy Markdown
Member

@jpelgrom jpelgrom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice modernisation!

Had to wait for a bit to make sure 2026.4.2 shipped to avoid breaking in case of last minute changes, but it's available now.

@jpelgrom jpelgrom merged commit 1943374 into main Apr 11, 2026
24 checks passed
@jpelgrom jpelgrom deleted the feature/only_allow_main_frame branch April 11, 2026 20:31
@TimoPtr TimoPtr added the WebViewActivity replacement Ongoing work to replace the WebViewActivity in favor of a well tested compose screen using nav. label Apr 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed WebViewActivity replacement Ongoing work to replace the WebViewActivity in favor of a well tested compose screen using nav.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants