fix: [SDK-4753] narrow overly broad consumer ProGuard/R8 keep rules

[abdulraqeeb33]

Problem

A developer ran Android's R8 Configuration Analyzer on their app and found that 3 of the top 5 sources of the broadest -keep rules in their final app came from OneSignal (reported against core-5.9.2). Our consumer rules shipped whole-package member keeps like:

-keepclassmembers class com.onesignal.core.** { *; }
-keepclassmembers class com.onesignal.notifications.** { *; }
...

These keep all members of all classes across nearly the entire SDK, blocking R8 from obfuscating and member-shrinking consumer apps that depend on OneSignal.

Note: the real consumer-facing rules live in each module's consumer-rules.pro (wired via consumerProguardFiles). The proguard-rules.pro files are the modules' own internal build config and are empty.

What changed (per module)

core/consumer-rules.pro

• Removed the six broad -keepclassmembers class com.onesignal.{core,session,user,internal,debug,common}.** { *; } rules.
• Added a constructor-only keep for com.onesignal.** (service impls are instantiated via reflective constructor selection in ServiceRegistrationReflection).
• Added a Model getter keep without allowobfuscation (*** get*(); + *** is*();) — Model.initializeFromJson matches getter method names via reflection, so getters must keep their names.
• Narrowed the IModule rule from { *; } to { <init>(); } (modules are loaded by FQN string via Class.forName(...).newInstance()).
• Kept the @KeepStub constructor rule as-is.
• Kept -dontwarn com.amazon.**; removed the blanket -dontwarn com.onesignal.**.

notifications/consumer-rules.pro

• Replaced -keepclassmembers class com.onesignal.notifications.** { *; } with a constructor-only keep.
• Tightened the NSE rule to also keep <init>() (resolved by name from manifest <meta-data> and newInstance()d).
• Collapsed the 15 per-class badger keeps into one implements Badger { <init>(...); } rule.
• Kept the WorkManager worker + InputMerger rules.
• Removed the dead -dontwarn com.onesignal.notification.** (wrong package — the package is notifications).
• Narrowed (not deleted) the risky rules: ADM ADMMessageHandler/ADMMessageHandlerJob and JobIntentService$* changed from {*;} to { <init>(...); }.
• Deleted the stale com.google.firebase.iid.FirebaseInstanceId and com.google.android.gms.common.api.GoogleApiClient keeps and the three consumer observer keeps (IPermissionObserver, IPushSubscriptionObserver, IUserStateObserver) — no main-source references found (FCM now uses FirebaseMessaging; observers are invoked through their interfaces so R8 keeps reachable overrides automatically). See Validation needed below.

in-app-messages/consumer-rules.pro

• Replaced { *; } with constructor-only keep; removed dead -dontwarn com.onesignal.iam.**.

location/consumer-rules.pro

• Replaced { *; } with constructor-only keep; removed -dontwarn com.onesignal.location.**.

otel/consumer-rules.pro — no change (only -dontwarn for optional Jackson/AutoValue; no shrinking impact).

Narrowed vs deleted

• Narrowed (cannot regress): all package member keeps → constructor-only; NSE; badgers; ADM handlers; JobIntentService$*; IModule.
• Deleted (needs validation): stale FirebaseInstanceId/GoogleApiClient keeps; the three observer keeps.

Build

./gradlew :onesignal:core:assemble :onesignal:notifications:assemble :onesignal:in-app-messages:assemble :onesignal:location:assemble :onesignal:otel:assemble → BUILD SUCCESSFUL (consumer-rules are merged at build time; only pre-existing Kotlin warnings remain). Consumer ProGuard/R8 rules are ultimately validated at the consuming app's R8 step, hence the checklist below. Spotless/detekt run in CI but do not apply to .pro files.

Validation needed (R8 full-mode sample-app smoke test)

Build a sample app with android.enableR8.fullMode=true + minifyEnabled true and verify:

• Notification receive + open (FCM)
• NSE (INotificationServiceExtension) resolved from manifest <meta-data>
• ADM path (Amazon device) — confirms narrowed ADM handler rules
• Notification badges (shortcut badger)
• Login / logout + operation persistence round-trip across an app restart (exercises Model.initializeFromJson getter-name reflection)
• IAM triggers
• Location updates
• Observer callbacks fire after deleting the observer keeps (IPermissionObserver, IPushSubscriptionObserver, IUserStateObserver)

References

• GitHub issue: [Feedback]: Overly broad ProGuard/R8 rules used throughout the library #2658
• Linear: SDK-4753

Do not merge until the R8 full-mode validation above is complete.

Made with Cursor

[github-actions]

📊 Diff Coverage Report

✓ Coverage check passed (no source files changed)

📥 View workflow run

[fadi-george]

One release-risk item to address before merge: the PR deletes the GoogleApiClient keep rule from notifications, but location still reflects on GMS methods by string in GoogleApiClientCompatProxy (blockingConnect, connect, disconnect). In a minified consumer app those methods may be renamed/stripped, silently breaking GMS location.
Can we move/add a targeted rule in location/consumer-rules.pro for those reflected GoogleApiClient methods?

[abdulraqeeb33]

@fadi-george good catch — you're right, that keep was not stale, it was just in the wrong module. The GoogleApiClient methods are reflected on by string name in the location module's GoogleApiClientCompatProxy, not notifications:

• getMethod("blockingConnect") — GoogleApiClientCompatProxy.kt:41
• getMethod("connect") — GoogleApiClientCompatProxy.kt:51
• getMethod("disconnect") — GoogleApiClientCompatProxy.kt:59

All three are no-arg methods invoked reflectively for cross-GMS-version compatibility, so in a minified consumer app R8 could rename/strip them and silently break GMS location.

Fixed in 3fb7e2126: I relocated the keep to location/consumer-rules.pro and scoped it to exactly those reflected methods (name-preserving, no allowobfuscation, no { *; }):

# GoogleApiClientCompatProxy invokes these GMS methods reflectively by string name (for
# compatibility across Google Play services versions), so their names must be preserved.
-keepclassmembers class com.google.android.gms.common.api.GoogleApiClient {
    com.google.android.gms.common.ConnectionResult blockingConnect();
    void connect();
    void disconnect();
}

Nothing was re-added to notifications/consumer-rules.pro — the rule belongs in location. Confirmed the full set of reflected GoogleApiClient methods is exactly these three (no isConnected/Builder/getters). :onesignal:location:assemble (plus notifications/core) builds green. This addresses the GMS-location regression risk.

[abdulraqeeb33]

New CI gate: R8 consumer keep-rules diff (bb70e5fa6)

Following the discussion on the dropped GoogleApiClient keep, I added a build-time gate so a silently dropped/renamed consumer keep can't slip through again.

Why the existing minified build didn't catch the GoogleApiClient regression: the CI demo release build runs full-mode R8 over the merged consumer-rules.pro, but R8 only hard-fails on missing classes (that's why the Huawei/Firebase -dontwarn issue was caught). When a keep for a method reached only by a reflection string is dropped, R8 just renames/removes the method — the build stays green and it breaks at runtime. Nothing in CI runs the minified app, so it was invisible until a human caught it.

What the gate does

• examples/demo/app/proguard-r8-report.pro (wired into the release build type) makes R8 emit -printseeds / -printconfiguration under build/outputs/r8-report/ (gitignored).
• scripts/r8-keep-check.sh normalizes the seeds to a stable, high-signal set and diffs it against a checked-in baseline (examples/demo/app/r8-keep-baseline.txt, 1136 entries).
• .github/workflows/ci.yml runs it right after :app:assembleGmsRelease (before the Huawei build), failing the PR on drift. No extra R8 build — it reuses the GMS release that already runs.

Normalization (keeps the baseline low-noise): member-level seeds only (the constructors / getters / enum constants / methods that keep rules actually protect), owned by com.onesignal.* or third-party classes auto-derived from the -keep targets in consumer-rules.pro (so today it picks up com.google.android.gms.common.api.GoogleApiClient automatically, no hardcoding). It excludes the demo app's own com.onesignal.example.* and churny anonymous/lambda classes so unrelated UI/code edits don't rewrite the baseline.

Proof it would have caught this bug — removing the GoogleApiClient keep from location/consumer-rules.pro and rebuilding: the build still succeeds, but the gate fails with:

R8 keep-rules gate FAILED — the set of kept consumer members drifted from the baseline.

Members NO LONGER kept (a keep rule may have been dropped or mis-scoped —
verify none of these are reached by reflection before accepting):
  - com.google.android.gms.common.api.GoogleApiClient: com.google.android.gms.common.ConnectionResult blockingConnect()
  - com.google.android.gms.common.api.GoogleApiClient: void connect()
  - com.google.android.gms.common.api.GoogleApiClient: void disconnect()

Restoring the keep returns it to green.

Updating the baseline intentionally (when you legitimately add/change a keep or SDK surface):

UPDATE_R8_BASELINE=1 scripts/r8-keep-check.sh

and commit the updated r8-keep-baseline.txt — the diff shows reviewers exactly which kept members changed.

Notes / watch-items

• Added CI time is ~a few seconds (the gate is a shell diff; GMS release is unchanged, Huawei is now a separate step but built once).
• The baseline was generated locally (macOS, Java 17, AGP 8.8.2). R8 seeds are JVM-deterministic for identical inputs, so Linux CI should match; if the first CI run shows benign drift, regenerate the baseline on CI via the command above.
• Currently gates the GMS release variant (it has GoogleApiClient + all modules on the classpath); Huawei could be added later with a variant-specific seeds path.

[abdulraqeeb33]

R8 keep-rules gate failure was benign host drift — now made deterministic

Investigation: Pulled the failing step's diff. It was 5 lines, all constructors/enum-constants, none of the reflective keeps the gate guards:

Members NO LONGER kept:
  - com.onesignal.common.threading.ThreadingMode: ThreadingMode()
  - com.onesignal.core.internal.features.FeatureFlag: ... SDK_BACKGROUND_THREADING
  - com.onesignal.internal.OneSignalImp: OneSignalImp(CoroutineDispatcher)
  - com.onesignal.internal.OneSignalImp: OneSignalImp(CoroutineDispatcher,int,DefaultConstructorMarker)
Newly kept:
  + com.onesignal.common.threading.OneSignalDispatchers$Pools: OneSignalDispatchers$Pools()

The GoogleApiClient trio, Model getters, and NSE callback were not in the diff — i.e. still kept on CI. A clean local rebuild reproduced the macOS baseline byte-for-byte, so this is host-dependent R8 behavior (Kotlin default-arg / DefaultConstructorMarker constructor synthesis and enum unboxing on ubuntu vs macOS), not a dropped keep.

Fix: Scoped the gate's normalization to method seeds only (a return type + an argument list), which excludes the churny constructor/enum-constant categories that drifted while retaining the name-matched reflective keeps the gate exists for. Regenerated the committed baseline (297 deterministic method seeds; still contains GoogleApiClient.blockingConnect/connect/disconnect and the Model getters).

Re-proof: Dropping the GoogleApiClient keep still fails the gate listing exactly those 3 methods; restoring it passes. Pushed in da9b3a110.

[fadi-george]

Thanks for the change.
One follow-up: the new R8 gate now normalizes to method seeds only, so it won’t catch dropped constructor-only keeps even though the comments mention service constructors. Could we either narrow the wording to “method keeps” or add targeted coverage for the high-risk constructors?