fix: [SDK-3475] restrict notification component exports#2659
fix: [SDK-3475] restrict notification component exports#2659fadi-george wants to merge 3 commits into
Conversation
📊 Diff Coverage Report✓ Coverage check passed (no source files changed) |
|
@claude[agent] review |
|
@claude review |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
HMS regression from the prior revision is fixed; remaining nit on QUICKBOOT_POWERON is left as an inline comment for the author to consider.
Extended reasoning...
Overview
This is a manifest-only PR that flips six notification components in the notifications module from android:exported="true" to android:exported="false" to satisfy an Android security scanner finding. The single touched file is OneSignalSDK/onesignal/notifications/src/main/AndroidManifest.xml. The PR scope is narrow and well-described.
Security risks
The PR itself is a defensive hardening change (tightening component visibility). The risk profile is the inverse of typical PRs: the danger is not that the change introduces an exploit, but that it severs a legitimate cross-UID launch path. The prior revision did exactly that for NotificationOpenedActivityHMS, and was corrected in commit 22e4d2d to keep that activity exported. The remaining cross-UID concern is the QUICKBOOT_POWERON action on BootUpReceiver, which the bug hunter has flagged as a nit-level inline comment.
Level of scrutiny
Low–medium. Manifest-only, six lines changed, fully reversible, and the externally-launched HMS entry point has already been re-exported after my prior review. The pattern (security-scanner-driven export tightening) is well-understood and the other five components are safe (PendingIntent-driven or protected broadcasts).
Other factors
The author has demonstrated responsiveness — my previous critical finding on HMS was addressed in a follow-up commit. The remaining QUICKBOOT_POWERON finding is genuinely minor: it affects a small population of legacy HTC Sense / older Lenovo / older Samsung quick-power-on devices on API 21–25, only the quickboot fast-resume path (full-reboot restore via BOOT_COMPLETED is unaffected since it is a protected broadcast). Given the negligible 2026 impact and the trivial nature of the rest of the diff, this is approvable; the author can decide whether to drop the now-unreachable QUICKBOOT_POWERON filter action as documented cleanup.
a92d753 to
2784275
Compare
|
@claude review |
There was a problem hiding this comment.
LGTM — both previously flagged issues have been resolved: NotificationOpenedActivityHMS remains exported="true" (preserving the HMS Core cross-UID launch path), and the unreachable QUICKBOOT_POWERON action was removed from BootUpReceiver now that it's non-exported.
What was reviewed: the full manifest diff restricting six notification components to non-exported, verifying each is only invoked via internal PendingIntent or protected system broadcasts (BOOT_COMPLETED, MY_PACKAGE_REPLACED); confirmed FCMBroadcastReceiver and NotificationOpenedActivityHMS correctly remain exported for their external entry-point roles.
Extended reasoning...
Overview
This PR modifies a single file, OneSignalSDK/onesignal/notifications/src/main/AndroidManifest.xml, restricting android:exported to false on six notification components (NotificationDismissReceiver, BootUpReceiver, UpgradeReceiver, NotificationOpenedActivity, NotificationOpenedActivityAndroid22AndOlder) that don't require external app invocation, in response to security scanner findings. FCMBroadcastReceiver and NotificationOpenedActivityHMS are intentionally left exported since they are legitimate external entry points (FCM and HMS Core respectively).
Security risks
This is a security-hardening change with no negative security risk itself — it reduces attack surface by preventing arbitrary third-party apps from directly invoking OneSignal's internal notification-handling components. The main risk was functional regression (breaking legitimate external invocation paths), not a new vulnerability.
Level of scrutiny
I previously flagged two functional regressions in this PR across earlier runs: (1) NotificationOpenedActivityHMS being set non-exported would have broken Huawei HMS notification opens (cross-UID launch from HMS Core), and (2) BootUpReceiver retaining the QUICKBOOT_POWERON action while becoming non-exported would leave that action silently unreachable on legacy OEM devices. Both have since been fixed in dedicated follow-up commits (a64e1e0 and 2784275), confirmed by re-reading the current manifest on disk, which matches the PR diff exactly. No new bugs were found by the bug hunting system this run.
Other factors
The change is manifest-only, mechanical in nature (flipping boolean attributes and removing one dead intent-filter action), and the author has done reasonable manual verification (inspecting the built AAR for exported flags and re-running the relevant Gradle tasks). Given both substantive concerns from earlier reviews are now resolved and the remaining change is narrow and well-understood, I'm comfortable approving.
abdulraqeeb33
left a comment
abdulraqeeb33
left a comment
There was a problem hiding this comment.
Please make sure to test this in browser stack on a real device.
Co-authored-by: Cursor <cursoragent@cursor.com>
2784275 to
f5e6bae
Compare
There was a problem hiding this comment.
LGTM — the previously flagged QUICKBOOT_POWERON issue has been resolved by removing that now-unreachable action from BootUpReceiver's intent-filter, and NotificationOpenedActivityHMS remains exported="true" as required for HMS Core's cross-UID launch. No new bugs found this run.
Extended reasoning...
Overview
This is a follow-up review after the author addressed prior feedback in commit a92d753. The PR restricts six OneSignal-owned notification components (dismiss receiver, boot/upgrade receivers, two notification-open activities) to android:exported="false", since they are only invoked via app-created PendingIntents or protected system broadcasts (BOOT_COMPLETED, MY_PACKAGE_REPLACED). FCMBroadcastReceiver and NotificationOpenedActivityHMS are correctly left exported since they are legitimately launched cross-UID (FCM via a permission guard, HMS Core via explicit component name from the backend payload).
Resolution of prior findings
- The earlier concern about NotificationOpenedActivityHMS being flipped to non-exported does not apply to the current diff/state — it remains exported="true", matching the PR description and verified against the current manifest in the repo.
- The QUICKBOOT_POWERON concern (a legacy OEM broadcast not in the protected-broadcast list, which would have become unreachable on a non-exported receiver) was fixed by removing that action from BootUpReceiver's intent-filter, leaving only BOOT_COMPLETED (a protected broadcast delivered from system_server, safe on a non-exported receiver).
Security risks
None beyond the intended effect of the PR: reducing the externally-launchable attack surface of these components, which is a security improvement. No auth/crypto logic is touched.
Level of scrutiny
Low-to-moderate. This is a single manifest file change with narrow, well-understood Android component-export semantics. The remaining components (NotificationDismissReceiver, NotificationOpenedActivity, NotificationOpenedActivityAndroid22AndOlder, UpgradeReceiver) are only invoked in-app via PendingIntent or via protected system broadcasts, both of which are unaffected by exported=false.
Other factors
Author re-ran the manifest processing and AAR inspection to confirm the intended exported values and confirmed unit tests pass. Both issues raised on the previous round have been directly addressed with a clear explanation in the follow-up commit.
2 participants
Description
One Line Summary
Restrict OneSignal notification components that do not need external app access to non-exported Android components.
Details
Motivation
Android security scanners flag several OneSignal notification components as exported without a guarding permission. The affected SDK-owned notification open, dismiss, boot restore, and upgrade restore components are invoked by app-created
PendingIntents or protected system broadcasts, so they should not be externally launchable by arbitrary apps.Scope
This updates the notifications module manifest only.
FCMBroadcastReceiverremains exported because it is the FCM entry point and remains protected bycom.google.android.c2dm.permission.SEND.NotificationOpenedActivityHMSalso remains exported because HMS Core launches it cross-UID for Huawei notification opens. The legacy OEMQUICKBOOT_POWERONaction was removed fromBootUpReceiverbecause it is not delivered to a non-exported receiver.Testing
Unit testing
No new unit tests were added because this is a manifest-only component exposure change.
Manual testing
Automated verification run locally:
./gradlew :OneSignal:notifications:spotlessXmlCheck :OneSignal:notifications:processDebugManifest :OneSignal:notifications:processReleaseManifest :OneSignal:notifications:assembleReleaseonesignal/notifications/build/outputs/aar/notifications-release.aarand confirmed the affected components are packaged with the intendedandroid:exportedvalues,NotificationOpenedActivityHMSremains exported,FCMBroadcastReceiverremains exported withcom.google.android.c2dm.permission.SEND, andQUICKBOOT_POWERONis no longer present../gradlew :OneSignal:notifications:testDebugUnitTestDevice smoke testing has not been run yet. Recommended follow-up coverage: notification open, action button open, dismiss handling, boot restore, package upgrade restore, and HMS notification open.
Affected code checklist
Checklist
Overview
Testing
Final pass
Made with Cursor