fix(lint): restrict data-plan detector to known event builder types - #817
Conversation
…ents DataplanDetector resolved MPEvent.Builder/CommerceEvent.Builder chains via unrestricted reflection (Class.forName/newInstance/invoke) driven by class and method names found in the analyzed project's own source, including anything reachable through a local variable's initializer chain. Restrict resolution to an explicit allowlist of the mParticle DTO/builder types this detector actually needs, and bound expression resolution depth so an unrelated failure mode can't escape the existing exception handling as a stack overflow. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
PR SummaryLow Risk Overview Key changes include:
Reviewed by Cursor Bugbot for commit 7a82061. Bugbot is set up for automated code reviews on this repo. Configure here. |
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Nickolas Dimitrakas <nickolas.dimitrakas@rokt.com>
📦 SDK Size Impact ReportWhat the SDK adds to a minified release APK. Measured against an empty baseline app. Unlike the Rokt kit, android-core ships no Compose and no resources, so there is nothing here that a host app would already provide. mParticle Core SDK
➡️ SDK size impact change is minimal. Raw measurementsTarget branch: {"baseline_dex_bytes": 0, "baseline_download_bytes": 2513, "baseline_install_bytes": 7529, "core_dex_bytes": 218884, "core_download_bytes": 122911, "core_install_bytes": 130161}This PR: {"baseline_dex_bytes": 0, "baseline_download_bytes": 2511, "baseline_install_bytes": 7527, "core_dex_bytes": 218884, "core_download_bytes": 122916, "core_install_bytes": 130166} |
…class allowlist Three follow-ups from review: - MethodCall.resolve() invoked any method matching name+arity on whatever its parent resolved to, with no check of its own. An allowed instance's inherited getClass() returns a java.lang.Class, which is not itself allowed, so chaining a further call onto that (e.g. Class.forName, reachable via ordinary instance-call syntax) went through unchecked. Gate the resolved instance before searching for a matching method, not just at construction time. - Constructor.resolve() still returned the raw Class object when no matching constructor was found or newInstance() threw, instead of null - the same java.lang.Class leak as above, reachable without even needing getClass(). - StaticFactory checked the allowlist using the class name a compiled reference resolves to, but Kotlin multifile-class functions like mapOf/listOf resolve to a part class (e.g. kotlin.collections.MapsKt__MapsKt), not the bare facade (kotlin.collections.MapsKt) - which also declares no methods of its own, so the check could never pass. Normalize the "__Part" suffix for the allowlist lookup (Class.forName still uses the original name), and restrict to the specific factory methods actually needed instead of every declared method on the class. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Background
The Data Plan lint detector evaluates
MPEvent.Builder/CommerceEvent.Buildercalls found in analyzed source to check them against a data plan. This change tightens what it's allowed to touch while doing that evaluation.What Has Changed
Screenshots/Video
N/A — lint tooling change, no UI.
Checklist
Additional Notes
./gradlew :tooling:custom-lint-rules:testand:tooling:custom-lint-rules:ktlintCheckpass.DataplanDetectorTestconfirm the allowlist is enforced and that a normal builder chain still resolves as before.🤖 Generated with Claude Code