Skip to content

fix(playscan): close the Android scanning gaps from review - #22

Merged
ethanzhoucool merged 3 commits into
mainfrom
fix/playscan-android-gaps
Jul 28, 2026
Merged

fix(playscan): close the Android scanning gaps from review#22
ethanzhoucool merged 3 commits into
mainfrom
fix/playscan-android-gaps

Conversation

@ethanzhoucool

@ethanzhoucool ethanzhoucool commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

The Android fixes that were meant to be in #21. That PR merged with only the README commit — the code commits had been made on a different local branch and never reached the PR head, so #21 shipped the attribution work alone. These are the same three commits, cherry-picked onto the current main and re-verified.

Five findings from a review of the Play support, all reproduced before fixing.

  • preflight could not inspect an Android artifact. --ipa existed, --apk / --aab did not. Both added, and preflight now runs the source and archive scans together since they see different things.
  • Artifact scans silently skipped the Gradle-backed checks. ScanArchive passes an empty GradleInfo, so Play Billing version, ads-SDK detection, and auth-SDK detection could not fire. Their absence read as a pass. The scan now reports the gap and names the three checks it skipped.
  • AABs false-passed native checks. Native code was only read from base/lib/, so a misaligned library in a feature module passed the 16 KB check. Every module is scanned now. Added the missing 64-bit parity rule: a 32-bit ABI without its 64-bit counterpart is CRITICAL, skipped on config splits, which legitimately carry one ABI.
  • Deadline logic ignored form factors. Play runs a separate target API schedule for Wear OS, TV, Automotive and XR, so those apps could get a CRITICAL that Play would not produce. They now get a WARN naming their own track, and android:required="false" is honoured so a phone app that also ships to TV keeps its CRITICAL.
  • preflight --verify hardcoded iOS. An Android-only project ran its flows against the wrong store and had its .apk rejected. Platform now follows the artifact extension, then the project layout.

Three further defects were found while reviewing the first pass:

  • The form-factor fix was inert on the artifact path: applyElement had no uses-feature case, so compiled manifests never populated it and only source scans ever saw the declaration.
  • frameworkAttrIDs had no entry for android:required. aapt2 emits empty string-pool names for framework attributes, so it was dropped on both compiled paths and a phone app declaring leanback required="false" was still read as a TV app. 0x0101028e was read back from aapt2 dump xmltree against android-34, -35 and -36 rather than recalled, per the rule that every entry in that table is verified.
  • A failing archive scan discarded source findings that had already succeeded.

Verified end to end against real aapt2-linked APKs: a watch app at targetSdk 33 reports WARN naming Wear OS; a phone app declaring leanback required="false" still reports CRITICAL.

Every fix has a regression test, and each test was confirmed to fail with its fix reverted. Full suite, -race, vet and gofmt pass.

Known follow-ups, deliberately not here: BundleConfig.pb PAGE_ALIGNMENT_16K, per-library rather than per-ABI 64-bit parity, and cutting a release so playscan actually ships (v0.1.0 predates it, so Homebrew and go install users still get an Apple-only binary).


Note

Medium Risk
Touches Play policy gating and preflight CI behavior (merged scans, incomplete runs, verify platform); changes are well covered by regression tests but could shift severity or exit-code outcomes for Wear/TV and artifact-only workflows.

Overview
Closes several Android playscan and preflight gaps so artifact scans are honest, native policy checks are complete, and runtime verify targets the right store.

preflight gains --apk / --aab (mutually exclusive) and runs source playscan plus archive scan when both apply, merging findings and keeping source results if the artifact scan fails (Incomplete). preflight --verify no longer hardcodes iOS: platform follows artifact extension (.apk / .app) then project layout.

Artifact scans always emit an INFO “Scan coverage” finding for Gradle-only checks (Play Billing, ads-SDK, auth-SDK) so a clean APK/AAB is not mistaken for a full scan. AAB native libs are collected from every module (<module>/lib/), not only base/. New 64-bit ABI parity rule (CRITICAL) skips config split manifests. Target API uses manifest form factor from required <uses-feature>: Wear/TV/Automotive/XR get a WARN on the phone schedule instead of a blocking CRITICAL; android:required="false" stays on the phone track. Archive decoders now handle uses-feature, split, and framework attr required (0x0101028e) in binary XML.

README documents complementary source vs artifact scans, 64-bit requirement, and form-factor deadlines.

Reviewed by Cursor Bugbot for commit 2d95f52. Bugbot is set up for automated code reviews on this repo. Configure here.

Five issues, all reproduced before fixing.

Native code coverage:
- An AAB carries native code at <module>/lib/, but collectNativeLibs only
  read base/lib/, so a misaligned library shipping in a feature module
  passed the 16 KB check silently. Every module is scanned now.
- No 64-bit parity rule existed. Play requires a 64-bit library for each
  32-bit ABI shipped; armeabi-v7a without arm64-v8a (or x86 without
  x86_64) is now CRITICAL.

Artifact scan honesty:
- ScanArchive passes an empty GradleInfo, so Play Billing version,
  ads-SDK detection, and auth-SDK detection cannot fire. They were
  silently absent, which reads as a pass. The scan now reports the gap as
  a finding naming the three checks it skipped.
- preflight had no way to scan an Android artifact at all: --ipa existed,
  --apk/--aab did not. Both are added, and preflight runs the source and
  archive scans together since they see different things.

False positives and wrong-platform runs:
- Play publishes a separate target API schedule per form factor. A Wear,
  TV, Automotive, or XR app was held to the phone schedule and could get
  a CRITICAL that Play would not produce. Those now get a WARN naming
  their own track.
- preflight --verify hardcoded Platform: "ios", so an Android-only
  project ran its flows against the wrong store and had its .apk
  rejected by artifact validation. The platform follows the artifact
  extension when given, then the project layout.

README: the "every check runs against the artifact" claim was wrong and
is corrected, and the new rules are documented.

Tests cover each fix, including the 32/64 parity matrix and the
form-factor table. Full suite and -race pass.
Three defects found reviewing the previous commit, all reproduced.

- The form-factor fix was inert on the path it was added for. applyElement
  had no uses-feature case, so a compiled manifest (APK binary XML, AAB
  protobuf) never populated UsesFeatures and a Wear or TV artifact still
  got the phone schedule's CRITICAL. Only the source scan, which decodes
  with encoding/xml, ever saw the declaration.
- android:required was ignored. A phone app that also ships to TV declares
  leanback with required="false", and that was being read as a TV app,
  downgrading a genuine phone CRITICAL to WARN. Unrequired features no
  longer move an app off the phone track.
- A failing archive scan discarded the source findings that had already
  succeeded. The goroutine returned on error after playscan.Scan had
  populated results but before the merge, so a broken --apk silently
  erased real findings. Both scans now merge whatever completed, and the
  error still marks the run incomplete.

Smaller items from the same review:

- A non-phone app with an unresolvable targetSdk returned nothing; it now
  falls through to the shared "could not determine" finding, matching the
  rule's own principle that silence is misleading.
- The Gradle coverage finding sat inside the manifest-decoded branch, so
  an artifact with an undecodable manifest did not report the gap. That
  gap exists either way, so it moved out.
- 64-bit parity no longer runs on a config split. A split legitimately
  carries one ABI, so scanning one would report a violation that does not
  exist in the release it belongs to. Needs android:split, which is now
  decoded.

Each fix has a regression test, and each test was confirmed to fail with
its fix reverted. Full suite, -race, and vet pass.
The required=false fix only worked on source scans. frameworkAttrIDs had
no entry for android:required, and aapt2 emits an empty string-pool name
for framework attributes, so the attribute was dropped on every APK and
AAB. A phone app declaring leanback required="false" was still read as a
TV app and had its CRITICAL downgraded to WARN.

Adds 0x0101028e -> "required", read back from `aapt2 dump xmltree` on
android-34, -35 and -36 (all three agree), per the rule that every entry
in that table is verified rather than recalled.

The same aapt2 dump confirms `split` is unqualified on <manifest>, so it
resolves through the string pool and needs no table entry.

Tests: the binary-XML path now covers required=false, required=true, and
an absent attribute defaulting to true. Verified end to end against real
aapt2-linked APKs: a watch app at targetSdk 33 reports WARN naming Wear
OS, and a phone app declaring leanback required=false still reports
CRITICAL.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 2d95f52. Configure here.

"android.hardware.type.automotive": FormFactorAutomotive,
"android.software.xr.immersive": FormFactorXR,
"android.hardware.xr.head_tracking": FormFactorXR,
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Wrong XR form-factor feature names

High Severity

featureFormFactors maps XR to android.software.xr.immersive and android.hardware.xr.head_tracking, but real Android XR apps declare android.software.xr.api.spatial or android.software.xr.api.openxr. XR packages never match, so they still get the phone track’s blocking target API finding — the exact false CRITICAL this change aimed to stop for form factors. The regression test uses the same incorrect names, so it cannot catch this.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 2d95f52. Configure here.

@ethanzhoucool
ethanzhoucool merged commit ec41b7c into main Jul 28, 2026
4 checks passed
@ethanzhoucool
ethanzhoucool deleted the fix/playscan-android-gaps branch July 28, 2026 19:56
ethanzhoucool added a commit that referenced this pull request Jul 28, 2026
…ure names (#24)

featureFormFactors mapped XR to two feature constants that do not exist, so no XR app ever matched and XR packages kept getting the phone track's blocking target API finding. XR is now matched on the android.software.xr. / android.hardware.xr. namespace, which no android.jar for API 34-36 lets us pin to an exact name.

Reported by Cursor Bugbot on #22.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant