From 265c43fa743696f9c434ef4d9bab6cbcf876440b Mon Sep 17 00:00:00 2001 From: ethan zhou <231755529+ethanzhoucool@users.noreply.github.com> Date: Mon, 27 Jul 2026 14:39:30 -0700 Subject: [PATCH 1/3] feat(playscan): scan Android projects against Google Play policies Adds a Google Play tier alongside the existing App Store checks. Android projects are detected automatically, including the android/ directory of an Expo or React Native app, so a cross-platform repo is checked against both stores in one pass. Also available standalone as `greenlight playscan`. Deadline checks, both landing 2026-08-31: - Target API level: CRITICAL below API 35, HIGH below 36, with a live day countdown - Play Billing Library 7 and below, which has no direct v7 to v9 path Policy checks: - Restricted permissions: SMS and Call Log (including the 2026-07-15 change dropping phone-call account verification), All files access, QUERY_ALL_PACKAGES, REQUEST_INSTALL_PACKAGES, background location, broad photo/video access, accessibility service, overlays, device admin, usage stats, contacts - Foreground service types missing their FOREGROUND_SERVICE_* permission, which throws at startForeground() on Android 14+ - android:exported missing on components with an intent filter, debuggable, cleartext traffic - Ads SDK without the AD_ID permission, which silently zeroes the ad ID - Account creation without both in-app and web deletion paths Every finding carries a Doc link to the policy page it comes from. Play policies are named rather than numbered and change often, so the citation is worth more than a section reference. The Apple scanners now skip only when a project is unambiguously Android-only. Previously every pure-Android repo led with a CRITICAL for a missing PrivacyInfo.xcprivacy, an iOS-only requirement. iOS-only and cross-platform projects are unaffected. targetSdk is resolved from the app module, convention plugins, version catalogs, gradle.properties, and named constants, since most real projects reach it through a reference rather than a literal. lint.targetSdk and testOptions.targetSdk are explicitly excluded. When it cannot be resolved the scan says so rather than reporting a pass. Scope is documented in the README and command help: this reads the pre-merge manifest, so permissions contributed by library manifests are not visible until the build merges them. Validated against nowinandroid, AnkiDroid, Thunderbird, Pocket Casts, DuckDuckGo, and Mattermost, with every finding checked against source. --- README.md | 57 +- internal/cli/playscan.go | 240 ++++++++ internal/cli/preflight.go | 45 +- internal/cli/root.go | 8 +- internal/playscan/gradle.go | 307 ++++++++++ internal/playscan/manifest.go | 179 ++++++ internal/playscan/rules.go | 561 ++++++++++++++++++ internal/playscan/scanner.go | 272 +++++++++ internal/playscan/scanner_test.go | 916 ++++++++++++++++++++++++++++++ internal/playscan/types.go | 51 ++ internal/preflight/platform.go | 72 +++ internal/preflight/runner.go | 211 ++++--- internal/preflight/runner_test.go | 135 ++++- 13 files changed, 2972 insertions(+), 82 deletions(-) create mode 100644 internal/cli/playscan.go create mode 100644 internal/playscan/gradle.go create mode 100644 internal/playscan/manifest.go create mode 100644 internal/playscan/rules.go create mode 100644 internal/playscan/scanner.go create mode 100644 internal/playscan/scanner_test.go create mode 100644 internal/playscan/types.go create mode 100644 internal/preflight/platform.go diff --git a/README.md b/README.md index 2cf45c1..7e89c5e 100644 --- a/README.md +++ b/README.md @@ -1,8 +1,8 @@ # greenlight -**Know before you submit.** Pre-submission compliance scanner for the Apple App Store. +**Know before you submit.** Pre-submission compliance scanner for the Apple App Store and Google Play. -Greenlight scans your app — source code, privacy manifests, IPA binaries, and App Store Connect metadata — against Apple's Review Guidelines, catching rejection risks before Apple does. Fully offline, no account, runs in under a second. +Greenlight scans your app — source code, privacy manifests, Android manifests and Gradle builds, IPA binaries, and App Store Connect metadata — against Apple's Review Guidelines and Google Play's Developer Program Policies, catching rejection risks before the stores do. Fully offline, no account, runs in under a second. > **Optional runtime tier:** want to confirm flow-dependent guidelines (account deletion, restore purchases, Sign in with Apple) actually *work*, not just exist in source? `greenlight verify` validates them on a cloud device via [Revyl](https://revyl.com). It's entirely separate and opt-in — the static scanner above never needs it. See [`greenlight verify`](#greenlight-verify-path--runtime-flow-validation-via-revyl). @@ -53,6 +53,7 @@ greenlight preflight . --output report.json # write to file | **metadata** | app.json / Info.plist: name, version, bundle ID format, icon, privacy policy URL, purpose strings | | **codescan** | 30+ code patterns: private APIs, secrets, payment violations, missing ATT, social login, placeholders | | **privacy** | PrivacyInfo.xcprivacy completeness, Required Reason APIs, tracking SDKs vs ATT implementation | +| **playscan** | Google Play: target API level deadline, restricted permissions, foreground service types, Play Billing version, manifest requirements (Android projects only) | | **ipa** | Binary: Info.plist keys, launch storyboard, app icons, app size, framework privacy manifests | ### `greenlight codescan [path]` — Code pattern scan @@ -80,6 +81,58 @@ Scans Swift, Objective-C, React Native, and Expo projects for: - Missing encryption export-compliance declaration - Expo config issues (§2.1) +### `greenlight playscan [path]` — Google Play policy scan + +```bash +greenlight playscan /path/to/project +greenlight playscan . --format json +greenlight playscan . --exit-code # CI gating +``` + +Checks an Android app against Google Play's Developer Program Policies and its +published distribution deadlines. Android projects are also picked up +automatically by `greenlight preflight`, including the `android/` directory of +an Expo or React Native app, so a cross-platform repo is checked against both +stores in one pass. + +**Deadlines** +- **Target API level** — new apps and updates must target API 36 from + August 31, 2026; apps below API 35 already lose distribution to new users on + newer devices — **CRITICAL / HIGH** +- **Play Billing Library** — v7 and below lose support August 31, 2026, and + there is no direct v7 → v9 upgrade path — **HIGH** + +**Restricted permissions** (each needs an approved use case or declaration form) +- SMS and Call Log — including the July 2026 change that drops phone-call + account verification as a permitted `READ_CALL_LOG` use +- `MANAGE_EXTERNAL_STORAGE` (All files access) +- `QUERY_ALL_PACKAGES`, `REQUEST_INSTALL_PACKAGES` +- `ACCESS_BACKGROUND_LOCATION` (declaration + demo video) +- Broad photo/video access over the system Photo Picker (API 33+) +- Accessibility Service, overlays, device admin, usage stats +- Contacts, ahead of the 2026 Contact Permissions policy + +**Manifest and build** +- Foreground service types missing their required `FOREGROUND_SERVICE_*` + permission — crashes at `startForeground()` on Android 14+ — **CRITICAL** +- `specialUse` foreground services needing a Console justification +- `android:exported` missing on components with an intent filter (API 31+) — + the package fails to install — **CRITICAL** +- `android:debuggable="true"` — **CRITICAL** +- `android:usesCleartextTraffic="true"` +- Ads SDK shipped without `com.google.android.gms.permission.AD_ID`, which + silently returns a zeroed advertising ID +- Account creation without the required in-app **and** web deletion paths + +Every finding cites the policy page it comes from. + +**Scope.** This reads the `AndroidManifest.xml` and Gradle files in your repo, +which is the *pre-merge* manifest. Permissions contributed by library manifests +only appear once the build merges them, so a clean scan is not proof of a clean +merged manifest. `targetSdk` is resolved from the app module, convention +plugins under `build-logic/`/`buildSrc/`, and version catalogs; when it cannot +be resolved the scan says so rather than reporting a pass. + ### `greenlight privacy [path]` — Privacy manifest validator ```bash diff --git a/internal/cli/playscan.go b/internal/cli/playscan.go new file mode 100644 index 0000000..2ae866e --- /dev/null +++ b/internal/cli/playscan.go @@ -0,0 +1,240 @@ +package cli + +import ( + "encoding/json" + "fmt" + "os" + "sort" + "strings" + "time" + + "github.com/RevylAI/greenlight/internal/playscan" + "github.com/fatih/color" + "github.com/spf13/cobra" +) + +var ( + playscanFormat string + playscanOutput string + playscanExitCode bool +) + +var playscanCmd = &cobra.Command{ + Use: "playscan [path]", + Short: "Scan an Android project against Google Play's policies", + Long: `Check an Android app against Google Play's Developer Program Policies +and its published distribution deadlines, before you upload. + +Checks: + • Target API level — the annual requirement that silently pulls apps + from distribution when missed + • Play Billing Library — versions past their support window + • Restricted permissions — SMS, Call Log, All files access, QUERY_ALL_PACKAGES, + background location, broad photo/video access + • Foreground services — service types missing their required permission + • Manifest requirements — android:exported, debuggable, cleartext traffic + • Advertising ID — ads SDK shipped without the AD_ID permission + • Account deletion — the in-app and web deletion requirement + +Runs entirely offline. No Play Console account needed. + +Scope: this reads the AndroidManifest.xml and Gradle files in your repo, which +is the pre-merge manifest. Permissions contributed by library manifests only +appear after the build merges them, so a clean scan is not proof of a clean +merged manifest. + +Usage: + greenlight playscan . + greenlight playscan ./android --format json + greenlight playscan . --exit-code`, + Args: cobra.MaximumNArgs(1), + RunE: runPlayscan, +} + +func init() { + playscanCmd.Flags().StringVar(&playscanFormat, "format", "terminal", "output format: terminal, json") + playscanCmd.Flags().StringVar(&playscanOutput, "output", "", "write report to file (stdout if omitted)") + playscanCmd.Flags().BoolVar(&playscanExitCode, "exit-code", false, "exit non-zero on any CRITICAL or HIGH finding — for CI gating") + rootCmd.AddCommand(playscanCmd) +} + +func runPlayscan(cmd *cobra.Command, args []string) error { + path := "." + if len(args) > 0 { + path = args[0] + } + + info, err := os.Stat(path) + if err != nil { + return fmt.Errorf("cannot access path: %w", err) + } + if !info.IsDir() { + return fmt.Errorf("path must be a directory: %s", path) + } + + isJSON := strings.ToLower(playscanFormat) == "json" + if !isJSON { + purple.Println("\n greenlight playscan — know before you upload to Google Play.") + fmt.Printf(" Project: %s\n\n", path) + } + + start := time.Now() + result, err := playscan.Scan(path) + if err != nil { + return fmt.Errorf("play scan failed: %w", err) + } + elapsed := time.Since(start) + + out := os.Stdout + if playscanOutput != "" { + out, err = os.Create(playscanOutput) + if err != nil { + return fmt.Errorf("failed to create output file: %w", err) + } + defer out.Close() + } + + if isJSON { + enc := json.NewEncoder(out) + enc.SetIndent("", " ") + if err := enc.Encode(result); err != nil { + return err + } + } else { + printPlayscanReport(out, result, elapsed) + } + + if playscanExitCode { + for _, f := range result.Findings { + if f.Severity == "CRITICAL" || f.Severity == "HIGH" { + return ErrThreshold + } + } + } + return nil +} + +func printPlayscanReport(w *os.File, result *playscan.ScanResult, elapsed time.Duration) { + red := color.New(color.FgRed, color.Bold) + hiYellow := color.New(color.FgHiYellow, color.Bold) + yellow := color.New(color.FgYellow) + green := color.New(color.FgGreen, color.Bold) + greenC := color.New(color.FgGreen) + bold := color.New(color.Bold) + + if result.ManifestPath == "" && result.TargetSDK == 0 { + dim.Fprintln(w, " No Android project found at this path.") + dim.Fprintln(w, " Looked for AndroidManifest.xml and Gradle build files.") + fmt.Fprintln(w) + return + } + + // Project context + if result.PackageName != "" { + fmt.Fprintf(w, " Package: %s\n", result.PackageName) + } + if result.TargetSDK > 0 { + fmt.Fprintf(w, " targetSdk: %d\n", result.TargetSDK) + } + if result.ManifestPath != "" { + fmt.Fprintf(w, " Manifest: %s\n", result.ManifestPath) + } + fmt.Fprintln(w) + + // Sort by severity, then policy, so the output is stable run to run. + sevRank := map[string]int{"CRITICAL": 4, "HIGH": 3, "WARN": 2, "INFO": 1} + findings := append([]playscan.Finding{}, result.Findings...) + sort.SliceStable(findings, func(i, j int) bool { + if ri, rj := sevRank[findings[i].Severity], sevRank[findings[j].Severity]; ri != rj { + return ri > rj + } + return findings[i].Policy < findings[j].Policy + }) + + var criticals, highs, warns, infos int + for _, f := range findings { + switch f.Severity { + case "CRITICAL": + criticals++ + case "HIGH": + highs++ + case "WARN": + warns++ + case "INFO": + infos++ + } + } + + lastSeverity := "" + for _, f := range findings { + if f.Severity != lastSeverity { + switch f.Severity { + case "CRITICAL": + red.Fprintln(w, " CRITICAL — Will be rejected or already losing distribution") + case "HIGH": + hiYellow.Fprintln(w, " HIGH — Likely rejection") + case "WARN": + yellow.Fprintln(w, " WARNING — Worth fixing") + case "INFO": + dim.Fprintln(w, " INFO — Best practices") + } + fmt.Fprintln(w) + lastSeverity = f.Severity + } + + switch f.Severity { + case "CRITICAL": + red.Fprintf(w, " [CRITICAL] ") + case "HIGH": + hiYellow.Fprintf(w, " [HIGH] ") + case "WARN": + yellow.Fprintf(w, " [WARN] ") + case "INFO": + dim.Fprintf(w, " [INFO] ") + } + if f.Policy != "" { + bold.Fprintf(w, "%s: ", f.Policy) + } + bold.Fprintln(w, f.Title) + + if f.File != "" { + loc := f.File + if f.Line > 0 { + loc = fmt.Sprintf("%s:%d", f.File, f.Line) + } + dim.Fprintf(w, " %s\n", loc) + } + fmt.Fprintf(w, " %s\n", f.Detail) + if f.Fix != "" { + greenC.Fprintf(w, " Fix: ") + fmt.Fprintln(w, f.Fix) + } + if f.Doc != "" { + dim.Fprintf(w, " %s\n", f.Doc) + } + fmt.Fprintln(w) + } + + dim.Fprintln(w, " ─────────────────────────────────────────────") + fmt.Fprintln(w) + + switch { + case criticals > 0: + red.Fprint(w, " NOT READY") + fmt.Fprintf(w, " — %d critical, %d high, %d warnings\n", criticals, highs, warns) + case highs > 0: + hiYellow.Fprint(w, " NEEDS REVIEW") + fmt.Fprintf(w, " — %d high, %d warnings\n", highs, warns) + default: + green.Fprint(w, " LOOKS GOOD") + fmt.Fprintf(w, " — %d warnings, %d notes\n", warns, infos) + } + + fmt.Fprintf(w, " Scanned in %s\n\n", elapsed.Round(time.Millisecond)) + + // Console-side obligations no static scan can see. + dim.Fprintln(w, " Not checkable from source — verify in Play Console:") + dim.Fprintln(w, " • Data safety form matches what the app actually collects") + dim.Fprintln(w, " • Advertising ID declaration, content rating, target audience") + dim.Fprintf(w, " • %s\n\n", playscan.DocAppContent) +} diff --git a/internal/cli/preflight.go b/internal/cli/preflight.go index dda34d4..5f7ce6b 100644 --- a/internal/cli/preflight.go +++ b/internal/cli/preflight.go @@ -41,8 +41,13 @@ Combines: • Code scan — private APIs, hardcoded secrets, missing ATT, etc. • Privacy scan — Required Reason APIs, PrivacyInfo.xcprivacy, tracking SDKs • Metadata scan — app.json / Info.plist completeness, icons, version, bundle ID + • Play scan — target API level, restricted permissions, Play Billing (Android) • IPA inspect — binary analysis (if --ipa is provided) +Android projects are detected automatically, including the android/ directory +of an Expo or React Native app, so a cross-platform repo is checked against +both stores in one pass. + Add --verify to continue past the static checks and validate your flow-dependent guidelines (account deletion, restore purchases, Sign in with Apple) on a REAL device via Revyl — catching broken flows static analysis structurally can't. @@ -99,7 +104,16 @@ func runPreflight(cmd *cobra.Command, args []string) error { if preflightIPA != "" { fmt.Printf(" IPA: %s\n", preflightIPA) } - scanners := []string{"metadata", "codescan", "privacy"} + // Mirrors the gating in preflight.Run so the banner never advertises a + // scanner that will not run. + isIOS, isAndroid := preflight.DetectPlatforms(path) + var scanners []string + if isIOS || !isAndroid { + scanners = append(scanners, "metadata", "codescan", "privacy") + } + if isAndroid { + scanners = append(scanners, "playscan") + } if preflightIPA != "" { scanners = append(scanners, "ipa") } @@ -337,9 +351,14 @@ func printPreflightFinding(w *os.File, f preflight.Finding) { // Source tag dim.Fprintf(w, "[%s] ", f.Source) - // Guideline + title + // Guideline + title. Apple guidelines are numbered sections and read well + // with a § prefix; Play policies are named, where § would be wrong. if f.Guideline != "" { - bold.Fprintf(w, "§%s ", f.Guideline) + if isNumberedGuideline(f.Guideline) { + bold.Fprintf(w, "§%s ", f.Guideline) + } else { + bold.Fprintf(w, "%s: ", f.Guideline) + } } bold.Fprintln(w, f.Title) @@ -366,9 +385,23 @@ func printPreflightFinding(w *os.File, f preflight.Finding) { fmt.Fprintln(w, f.Fix) } + // Policy source, so the developer can go read the rule themselves. + if f.Doc != "" { + dim.Fprintf(w, " %s\n", f.Doc) + } + fmt.Fprintln(w) } +// isNumberedGuideline reports whether a guideline reference is an Apple-style +// numbered section ("5.1.1") rather than a named Play policy. +func isNumberedGuideline(g string) bool { + if g == "" { + return false + } + return g[0] >= '0' && g[0] <= '9' +} + func printPreflightFooter(w *os.File, result *preflight.Result) { red := color.New(color.FgRed, color.Bold) green := color.New(color.FgGreen, color.Bold) @@ -447,6 +480,9 @@ func preflightJSONObject(result *preflight.Result) interface{} { HasPrivacyInfo bool `json:"has_privacy_info"` DetectedAPIs []string `json:"detected_apis,omitempty"` TrackingSDKs []string `json:"tracking_sdks,omitempty"` + IsAndroid bool `json:"is_android"` + PackageName string `json:"package_name,omitempty"` + TargetSDK int `json:"target_sdk,omitempty"` Findings []preflight.Finding `json:"findings"` Summary preflight.Summary `json:"summary"` Elapsed string `json:"elapsed"` @@ -458,6 +494,9 @@ func preflightJSONObject(result *preflight.Result) interface{} { HasPrivacyInfo: result.HasPrivacyInfo, DetectedAPIs: result.DetectedAPIs, TrackingSDKs: result.TrackingSDKs, + IsAndroid: result.IsAndroid, + PackageName: result.PackageName, + TargetSDK: result.TargetSDK, Findings: result.Findings, Summary: result.Summary, Elapsed: result.Elapsed.Round(time.Millisecond).String(), diff --git a/internal/cli/root.go b/internal/cli/root.go index e012e04..42bf5ca 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -23,15 +23,17 @@ var dim = color.New(color.Faint) var rootCmd = &cobra.Command{ Use: "greenlight", - Short: "Pre-submission compliance scanner for the Apple App Store", + Short: "Pre-submission compliance scanner for the App Store and Google Play", Long: fmt.Sprintf(`%s -Greenlight scans your app against Apple's App Store Review Guidelines -before you submit, catching rejection risks so you ship with confidence. +Greenlight scans your app against Apple's App Store Review Guidelines and +Google Play's Developer Program Policies before you submit, catching +rejection risks so you ship with confidence. Get started: greenlight preflight . Run ALL checks — one command, zero uploads greenlight preflight . --ipa X Include IPA binary analysis + greenlight playscan . Google Play policies + target API deadline greenlight scan --app-id ID Check App Store Connect metadata (needs API key) greenlight guidelines search Browse Apple's review guidelines`, purple.Sprint("greenlight — know before you submit.")), diff --git a/internal/playscan/gradle.go b/internal/playscan/gradle.go new file mode 100644 index 0000000..1345d60 --- /dev/null +++ b/internal/playscan/gradle.go @@ -0,0 +1,307 @@ +package playscan + +import ( + "os" + "path/filepath" + "regexp" + "strconv" + "strings" +) + +// GradleInfo is what the policy rules need out of the Gradle build files. +type GradleInfo struct { + TargetSDK int + CompileSDK int + MinSDK int + // TargetSDKFile / TargetSDKLine locate the declaration so the report can + // point at the line to edit. + TargetSDKFile string + TargetSDKLine int + + // BillingVersion is the Play Billing Library major version, 0 if absent. + BillingVersion int + BillingVersionRaw string + BillingFile string + BillingLine int + Dependencies []string + HasAdsSDK bool + HasAuthSDK bool + AdsSDKFile string + AdsSDKLine int + + // HasAndroidPlugin is true when a build file actually configures Android. + // A Gradle file alone proves nothing — plenty of pure-JVM Kotlin projects + // have one — so this is what separates an Android project from any other + // Gradle project. + HasAndroidPlugin bool + + // Namespace is the application ID declared in Gradle, used when the + // manifest omits the legacy package attribute. + Namespace string +} + +// Gradle DSL comes in Groovy and Kotlin flavours, and every one of these forms +// appears in real projects: +// +// targetSdkVersion 34 targetSdk 34 +// targetSdkVersion(34) targetSdk = 34 +// targetSdk.set(34) targetSdkVersion = 34 +// +// A value that is not an integer literal (e.g. `targetSdkVersion +// rootProject.ext.targetSdkVersion`) is skipped here and picked up from +// whichever file defines the literal, since ext blocks are scanned too. +var ( + reTargetSDK = regexp.MustCompile(`\btargetSdk(?:Version)?\b\s*(?:=|\.set\(|\()?\s*["']?(\d{1,2})["']?`) + reCompileSDK = regexp.MustCompile(`\bcompileSdk(?:Version)?\b\s*(?:=|\.set\(|\()?\s*["']?(\d{1,2})["']?`) + reMinSDK = regexp.MustCompile(`\bminSdk(?:Version)?\b\s*(?:=|\.set\(|\()?\s*["']?(\d{1,2})["']?`) + + // Matches com.android.billingclient:billing:8.0.0 and the -ktx artifact, + // including version catalog style quoting. + reBilling = regexp.MustCompile(`com\.android\.billingclient:billing(?:-ktx)?:["']?v?(\d+)(?:\.(\d+))?`) + + reAdsSDK = regexp.MustCompile(`(?i)(com\.google\.android\.gms:play-services-ads|com\.google\.android\.ads|applovin|com\.facebook\.android:audience-network|com\.unity3d\.ads|ironsource|com\.mbridge|com\.vungle|adcolony|com\.chartboost)`) + + // Authentication SDKs, used as the signal that an app creates accounts. + // + // The artifact name must terminate at the match: play-services-auth is an + // auth SDK, but play-services-auth-blockstore (credential backup) and + // play-services-auth-api-phone (SMS retrieval) are not, and treating them + // as one is a false positive on apps with no accounts at all. + reAuthSDK = regexp.MustCompile(`(?i)(firebase-auth(?:-ktx)?["':]|play-services-auth(?:-ktx)?["':]|com\.auth0|supabase.*gotrue|supabase-kt.*auth|com\.okta|com\.amplifyframework:aws-auth|@react-native-google-signin|react-native-fbsdk)`) + + // Modern AGP moved the application ID out of the manifest, so the package + // name usually lives in the Gradle files now. + reNamespace = regexp.MustCompile(`\b(?:namespace|applicationId)\s*=?\s*["']([A-Za-z][A-Za-z0-9_]*(?:\.[A-Za-z][A-Za-z0-9_]*)+)["']`) + + // A targetSdk whose value is an expression rather than a literal. Real + // projects reach the SDK level through a named constant far more often + // than they inline it, in at least these shapes: + // + // targetSdk target_sdk (properties variable) + // targetSdk = ProjectConfig.Android.sdkTarget (Kotlin object) + // targetSdk = project.property("targetSdkVersion") as Int + // targetSdk = libs.versions.targetSdk.get().toInt() + // + // Capturing the expression lets it be resolved against the symbols + // collected from every other build file. + reTargetSDKRef = regexp.MustCompile(`\btargetSdk(?:Version)?\b\s*(?:=|\.set\(|\()?\s*([A-Za-z_"'][^\n]*)`) + + // Integer constants a targetSdk expression can refer to, in the forms + // Gradle, Kotlin DSL, and gradle.properties each use. + reSymbolAssign = regexp.MustCompile(`(?:^|\s)(?:const\s+)?(?:val|var|ext\.)?\s*([A-Za-z_][A-Za-z0-9_]*)\s*=\s*(\d{1,2})\s*$`) + reSymbolSet = regexp.MustCompile(`set\(\s*["']([A-Za-z_][A-Za-z0-9_]*)["']\s*,\s*(\d{1,2})\s*\)`) + reSymbolTo = regexp.MustCompile(`["']([A-Za-z_][A-Za-z0-9_]*)["']\s+to\s+(\d{1,2})\b`) + + // The identifier a targetSdk expression ultimately names: a quoted + // property key if there is one, else the last segment of a dotted path. + reQuotedIdent = regexp.MustCompile(`["']([A-Za-z_][A-Za-z0-9_]*)["']`) + reDottedIdent = regexp.MustCompile(`([A-Za-z_][A-Za-z0-9_]*)`) + + // Matches the Android Gradle plugin being applied, or an android {} + // configuration block, in either Groovy or Kotlin DSL. + reAndroidPlugin = regexp.MustCompile(`(com\.android\.(application|library)|\bandroid\s*\{|\bandroid\s*\(|AndroidApplicationConventionPlugin|com\.android\.tools\.build:gradle)`) +) + +// isGradleFile reports whether a filename is a Gradle build script. +func isGradleFile(name string) bool { + lower := strings.ToLower(name) + return strings.HasSuffix(lower, ".gradle") || strings.HasSuffix(lower, ".gradle.kts") +} + +// isVersionCatalog reports whether a file is a Gradle version catalog, where +// modern projects keep the SDK levels and dependency versions the build +// scripts interpolate. +func isVersionCatalog(name string) bool { + return strings.EqualFold(name, "libs.versions.toml") +} + +// isConventionPlugin reports whether a path is a Kotlin convention plugin. +// +// Projects using build-logic or buildSrc set targetSdk in precompiled plugins +// rather than in any .gradle file, so without these the scan cannot resolve the +// value on a large share of modern Android repos. +func isConventionPlugin(path string) bool { + if !strings.HasSuffix(strings.ToLower(path), ".kt") { + return false + } + p := filepath.ToSlash(path) + for _, dir := range conventionPluginDirs { + if strings.Contains(p, dir) { + return true + } + } + return false +} + +// conventionPluginDirs are the directory names projects use for precompiled +// build logic. There is no single convention, so each real variant seen in the +// wild is listed rather than guessed at. +var conventionPluginDirs = []string{ + "/build-logic/", "/buildSrc/", "/build-plugin/", + "/build-conventions/", "/gradle/plugins/", "/build-logic-", "/buildlogic/", +} + +// isGradleProperties reports whether a file is a gradle.properties, where many +// projects keep the SDK levels their build scripts interpolate. +func isGradleProperties(name string) bool { + return strings.EqualFold(name, "gradle.properties") +} + +// targetSdkIsNotTheApps reports whether a targetSdk assignment belongs to +// something other than the shipped application — the lint tool's target or the +// instrumentation test target. Reading those as the app's target would report a +// compliant app as non-compliant, or worse, the reverse. +func targetSdkIsNotTheApps(line string) bool { + return strings.Contains(line, "lint.targetSdk") || + strings.Contains(line, "testOptions.targetSdk") || + strings.Contains(line, "lint {") || + strings.Contains(line, "targetSdkPreview") +} + +// parseGradleFiles reads every Gradle script found and folds them into one view. +// +// Paths must be ordered most-specific-first (the app module before the root +// project), because a value already resolved from the app module wins: the app +// module's android {} block is what actually ships, and a root ext block only +// supplies the default it interpolates. +func parseGradleFiles(paths []string, relTo func(string) string) *GradleInfo { + g := &GradleInfo{} + + // Integer constants seen anywhere in the build, used to resolve a + // targetSdk that is expressed as a reference. First definition wins, + // matching the most-authoritative-first file ordering. + symbols := map[string]int{} + // The first unresolved targetSdk expression, kept in case no literal turns up. + var refExpr, refFile string + var refLine int + + for _, p := range paths { + data, err := os.ReadFile(p) + if err != nil { + continue + } + rel := relTo(p) + for i, line := range strings.Split(string(data), "\n") { + lineNo := i + 1 + code := stripGradleComment(line) + if code == "" { + continue + } + + for _, re := range []*regexp.Regexp{reSymbolAssign, reSymbolSet, reSymbolTo} { + if m := re.FindStringSubmatch(code); m != nil { + if v, err := strconv.Atoi(m[2]); err == nil { + if _, seen := symbols[m[1]]; !seen { + symbols[m[1]] = v + } + } + } + } + + if g.TargetSDK == 0 && !targetSdkIsNotTheApps(code) { + if m := reTargetSDK.FindStringSubmatch(code); m != nil { + if v, err := strconv.Atoi(m[1]); err == nil { + g.TargetSDK, g.TargetSDKFile, g.TargetSDKLine = v, rel, lineNo + } + } else if refExpr == "" { + if m := reTargetSDKRef.FindStringSubmatch(code); m != nil { + refExpr, refFile, refLine = m[1], rel, lineNo + } + } + } + if g.CompileSDK == 0 { + if m := reCompileSDK.FindStringSubmatch(code); m != nil { + if v, err := strconv.Atoi(m[1]); err == nil { + g.CompileSDK = v + } + } + } + if g.MinSDK == 0 { + if m := reMinSDK.FindStringSubmatch(code); m != nil { + if v, err := strconv.Atoi(m[1]); err == nil { + g.MinSDK = v + } + } + } + if g.BillingVersion == 0 { + if m := reBilling.FindStringSubmatch(code); m != nil { + if v, err := strconv.Atoi(m[1]); err == nil { + g.BillingVersion = v + g.BillingVersionRaw = m[1] + if m[2] != "" { + g.BillingVersionRaw = m[1] + "." + m[2] + } + g.BillingFile, g.BillingLine = rel, lineNo + } + } + } + if g.Namespace == "" { + if m := reNamespace.FindStringSubmatch(code); m != nil { + g.Namespace = m[1] + } + } + if !g.HasAndroidPlugin && reAndroidPlugin.MatchString(code) { + g.HasAndroidPlugin = true + } + if !g.HasAdsSDK && reAdsSDK.MatchString(code) { + g.HasAdsSDK, g.AdsSDKFile, g.AdsSDKLine = true, rel, lineNo + } + if !g.HasAuthSDK && reAuthSDK.MatchString(code) { + g.HasAuthSDK = true + } + } + } + + if g.TargetSDK == 0 && refExpr != "" { + if v, ok := resolveSDKRef(refExpr, symbols); ok { + g.TargetSDK, g.TargetSDKFile, g.TargetSDKLine = v, refFile, refLine + } + } + return g +} + +// resolveSDKRef resolves a targetSdk expression against the collected symbols. +// +// The name is taken from a quoted property key when the expression has one +// (project.property("targetSdkVersion")), otherwise from the last identifier +// in a dotted path (ProjectConfig.Android.sdkTarget -> sdkTarget). Values +// below API 14 are rejected: no shipping app targets them, so a match that low +// means the wrong symbol was resolved and reporting it would be worse than +// admitting the value is unknown. +func resolveSDKRef(expr string, symbols map[string]int) (int, bool) { + var candidates []string + if m := reQuotedIdent.FindStringSubmatch(expr); m != nil { + candidates = append(candidates, m[1]) + } + if idents := reDottedIdent.FindAllString(expr, -1); len(idents) > 0 { + // Walk from the most specific segment outward. + for i := len(idents) - 1; i >= 0; i-- { + candidates = append(candidates, idents[i]) + } + } + for _, name := range candidates { + if v, ok := symbols[name]; ok && v >= 14 { + return v, true + } + } + return 0, false +} + +// stripGradleComment removes a trailing // comment and drops whole-line +// comments, so a commented-out dependency is not read as a live one. Block +// comments spanning lines are not tracked; a `/*` line is dropped wholesale, +// which errs toward skipping rather than inventing a match. +func stripGradleComment(line string) string { + trimmed := strings.TrimSpace(line) + if trimmed == "" || strings.HasPrefix(trimmed, "//") || strings.HasPrefix(trimmed, "*") || strings.HasPrefix(trimmed, "/*") { + return "" + } + if idx := strings.Index(trimmed, "//"); idx >= 0 { + // Only strip when the // is not inside a quoted string, which is the + // common case for URLs in maven { url "https://..." } lines. + if !strings.Contains(trimmed[:idx], "\"") && !strings.Contains(trimmed[:idx], "'") { + trimmed = strings.TrimSpace(trimmed[:idx]) + } + } + return trimmed +} diff --git a/internal/playscan/manifest.go b/internal/playscan/manifest.go new file mode 100644 index 0000000..4032db6 --- /dev/null +++ b/internal/playscan/manifest.go @@ -0,0 +1,179 @@ +package playscan + +import ( + "encoding/xml" + "os" + "strconv" + "strings" +) + +// Manifest is the subset of AndroidManifest.xml the policy rules need. +// +// Attribute tags intentionally omit the android XML namespace. Go's decoder +// matches an unqualified `,attr` tag against any namespace, which keeps parsing +// working on manifests that bind the android prefix unusually or omit the +// xmlns declaration entirely (both appear in real generated manifests). +type Manifest struct { + XMLName xml.Name `xml:"manifest"` + Package string `xml:"package,attr"` + Permissions []UsesPermission `xml:"uses-permission"` + // PermissionsSDK23 covers , which grants the same + // policy obligations as a plain . + PermissionsSDK23 []UsesPermission `xml:"uses-permission-sdk-23"` + UsesSDK *UsesSDK `xml:"uses-sdk"` + Application *Application `xml:"application"` +} + +type UsesPermission struct { + Name string `xml:"name,attr"` +} + +type UsesSDK struct { + MinSDKVersion string `xml:"minSdkVersion,attr"` + TargetSDKVersion string `xml:"targetSdkVersion,attr"` +} + +type Application struct { + Name string `xml:"name,attr"` + Debuggable string `xml:"debuggable,attr"` + AllowBackup string `xml:"allowBackup,attr"` + UsesCleartextTraffic string `xml:"usesCleartextTraffic,attr"` + NetworkSecurityConf string `xml:"networkSecurityConfig,attr"` + Activities []Component `xml:"activity"` + ActivityAliases []Component `xml:"activity-alias"` + Services []Component `xml:"service"` + Receivers []Component `xml:"receiver"` + Providers []Component `xml:"provider"` +} + +// Component covers the four manifest component types. Only the attributes the +// policy rules read are modelled. +type Component struct { + Name string `xml:"name,attr"` + Exported string `xml:"exported,attr"` + Permission string `xml:"permission,attr"` + ForegroundSvcType string `xml:"foregroundServiceType,attr"` + IntentFilters []IntentFilter `xml:"intent-filter"` +} + +type IntentFilter struct { + Actions []struct { + Name string `xml:"name,attr"` + } `xml:"action"` + Categories []struct { + Name string `xml:"name,attr"` + } `xml:"category"` +} + +// HasLauncherActivity reports whether the manifest declares the app's launcher +// entry point. In a multi-module repo this is what distinguishes the shipped +// application's manifest from those of its libraries, benchmark harnesses, and +// test modules — none of which carry the app's permissions. +func (m *Manifest) HasLauncherActivity() bool { + if m.Application == nil { + return false + } + for _, comp := range append(append([]Component{}, m.Application.Activities...), m.Application.ActivityAliases...) { + for _, f := range comp.IntentFilters { + mainAction, launcherCategory := false, false + for _, a := range f.Actions { + if a.Name == "android.intent.action.MAIN" { + mainAction = true + } + } + for _, c := range f.Categories { + if c.Name == "android.intent.category.LAUNCHER" { + launcherCategory = true + } + } + if mainAction && launcherCategory { + return true + } + } + } + return false +} + +// HasIntentFilter reports whether the component declares any intent filter, +// which is what makes android:exported mandatory from API 31. +func (c Component) HasIntentFilter() bool { return len(c.IntentFilters) > 0 } + +// ParseManifest reads and decodes an AndroidManifest.xml. +func ParseManifest(path string) (*Manifest, error) { + data, err := os.ReadFile(path) + if err != nil { + return nil, err + } + var m Manifest + if err := xml.Unmarshal(data, &m); err != nil { + return nil, err + } + return &m, nil +} + +// AllPermissions returns every declared permission name, de-duplicated and in +// declaration order. +func (m *Manifest) AllPermissions() []string { + seen := make(map[string]bool) + var out []string + for _, p := range append(append([]UsesPermission{}, m.Permissions...), m.PermissionsSDK23...) { + name := strings.TrimSpace(p.Name) + if name == "" || seen[name] { + continue + } + seen[name] = true + out = append(out, name) + } + return out +} + +// HasPermission reports whether the manifest declares the given permission. +// Matching is on the full name, so "android.permission.READ_SMS" does not +// match "com.example.READ_SMS". +func (m *Manifest) HasPermission(name string) bool { + for _, p := range m.AllPermissions() { + if p == name { + return true + } + } + return false +} + +// Components returns every component across all four types. +func (a *Application) Components() []Component { + if a == nil { + return nil + } + var out []Component + out = append(out, a.Activities...) + out = append(out, a.ActivityAliases...) + out = append(out, a.Services...) + out = append(out, a.Receivers...) + out = append(out, a.Providers...) + return out +} + +// attrIsTrue reports whether a manifest boolean attribute is literally true. +// +// Manifest booleans are frequently resource references ("@bool/is_debug") or +// Gradle manifest placeholders ("${debuggable}"), whose value is not knowable +// without running the build. Those resolve to false here so the scan never +// invents a finding it cannot stand behind. +func attrIsTrue(v string) bool { + return strings.EqualFold(strings.TrimSpace(v), "true") +} + +// attrIsSet reports whether an attribute was present at all, distinguishing +// android:exported="false" from a missing android:exported. +func attrIsSet(v string) bool { return strings.TrimSpace(v) != "" } + +// parseSDKInt reads an SDK level attribute. Values can be a plain integer, a +// codename for preview releases, or a Gradle placeholder; only integers are +// usable and everything else returns 0 ("unknown"). +func parseSDKInt(v string) int { + n, err := strconv.Atoi(strings.TrimSpace(v)) + if err != nil || n <= 0 { + return 0 + } + return n +} diff --git a/internal/playscan/rules.go b/internal/playscan/rules.go new file mode 100644 index 0000000..6d6b4a0 --- /dev/null +++ b/internal/playscan/rules.go @@ -0,0 +1,561 @@ +package playscan + +import ( + "fmt" + "strings" + "time" +) + +// Policy documentation URLs. Only pages verified to exist are referenced — +// a link that 404s is worse than no link, since the whole point of citing is +// that the developer can go read the rule themselves. +const ( + docTargetAPI = "https://support.google.com/googleplay/android-developer/answer/11926878" + docBillingDeprecat = "https://developer.android.com/google/play/billing/deprecation-faq" + docRestrictedPerms = "https://support.google.com/googleplay/android-developer/answer/14115180" + docForegroundSvc = "https://support.google.com/googleplay/android-developer/answer/13392821" + docAccountDeletion = "https://support.google.com/googleplay/android-developer/answer/13327111" + docAdvertisingID = "https://support.google.com/googleplay/android-developer/answer/6048248" + docAppContent = "https://support.google.com/googleplay/android-developer/answer/9859455" + docProgramPolicy = "https://support.google.com/googleplay/android-developer/answer/16810878" + docJul2026Policy = "https://support.google.com/googleplay/android-developer/answer/17134731" +) + +// Google Play's published requirements, as of the 2026 cycle. +const ( + // requiredTargetSDKNew is what new apps and updates must target from + // targetAPIDeadline onward (Android 16). + requiredTargetSDKNew = 36 + // requiredTargetSDKExisting is the floor an already-published app must + // meet to stay available to new users on newer devices (Android 15). + // This one is already in force. + requiredTargetSDKExisting = 35 + // minSupportedBillingMajor is the lowest Play Billing Library major + // version still supported after billingDeadline. + minSupportedBillingMajor = 8 +) + +var ( + targetAPIDeadline = time.Date(2026, time.August, 31, 0, 0, 0, 0, time.UTC) + // Play Billing Library 7 loses support on the same date. There is no + // v7-to-v9 upgrade path; v8 is a required stop. + billingDeadline = targetAPIDeadline + + // now is a package variable so deadline wording is testable without + // waiting for the calendar. + now = time.Now +) + +// ruleContext is the parsed project state every rule reads. +type ruleContext struct { + manifest *Manifest + gradle *GradleInfo + targetSDK int + manifestFile string +} + +func (c *ruleContext) hasPermission(name string) bool { + return c.manifest != nil && c.manifest.HasPermission(name) +} + +type rule func(*ruleContext) []Finding + +func allRules() []rule { + return []rule{ + ruleTargetAPILevel, + rulePlayBillingVersion, + ruleRestrictedPermissions, + ruleForegroundServiceTypes, + ruleDebuggable, + ruleExportedComponents, + ruleCleartextTraffic, + ruleAdvertisingID, + ruleAccountDeletion, + } +} + +// deadlinePhrase renders a deadline as urgency a developer can act on, and +// stays correct after the date passes. +func deadlinePhrase(deadline time.Time) string { + days := int(deadline.Sub(now()).Hours() / 24) + switch { + case days < 0: + return fmt.Sprintf("The %s deadline has passed", deadline.Format("January 2, 2006")) + case days == 0: + return fmt.Sprintf("The deadline is today (%s)", deadline.Format("January 2, 2006")) + case days == 1: + return fmt.Sprintf("The deadline is tomorrow (%s)", deadline.Format("January 2, 2006")) + default: + return fmt.Sprintf("%d days left until %s", days, deadline.Format("January 2, 2006")) + } +} + +// ruleTargetAPILevel checks the annual target API level requirement, which is +// the single most common cause of an app silently losing distribution. +func ruleTargetAPILevel(c *ruleContext) []Finding { + file, line := c.gradle.TargetSDKFile, c.gradle.TargetSDKLine + if file == "" { + file = c.manifestFile + line = 0 + } + + if c.targetSDK == 0 { + // Nothing to check against, but silence would be misleading: the value + // exists somewhere the scan could not resolve (an ext property, a + // version catalog, a convention plugin). + return []Finding{{ + Severity: sevWarn, + Policy: "Target API level", + Title: "Could not determine targetSdk", + Detail: "No integer targetSdk was found in the Gradle files or the manifest, so the target API level requirement could not be checked. " + + "This usually means the value comes from an ext property, a version catalog, or a convention plugin.", + Fix: fmt.Sprintf("Confirm your app targets API %d or higher before submitting. %s.", requiredTargetSDKNew, deadlinePhrase(targetAPIDeadline)), + Doc: docTargetAPI, + File: file, + }} + } + + switch { + case c.targetSDK < requiredTargetSDKExisting: + return []Finding{{ + Severity: sevCritical, + Policy: "Target API level", + Title: fmt.Sprintf("targetSdk %d is below the API %d floor for existing apps", c.targetSDK, requiredTargetSDKExisting), + Detail: fmt.Sprintf( + "Apps already on Google Play must target API %d or higher to stay available to new users on devices running a newer Android version than the app targets. "+ + "At targetSdk %d the app is not discoverable or installable for those users. New submissions and updates must target API %d. %s.", + requiredTargetSDKExisting, c.targetSDK, requiredTargetSDKNew, deadlinePhrase(targetAPIDeadline)), + Fix: fmt.Sprintf("Set targetSdk = %d in your app module's android {} block and re-test on Android 16.", requiredTargetSDKNew), + Doc: docTargetAPI, + File: file, + Line: line, + }} + + case c.targetSDK < requiredTargetSDKNew: + return []Finding{{ + Severity: sevHigh, + Policy: "Target API level", + Title: fmt.Sprintf("targetSdk %d is below API %d, required for new submissions", c.targetSDK, requiredTargetSDKNew), + Detail: fmt.Sprintf( + "New apps and updates to existing apps must target Android %d (API %d) or higher to be published. %s. "+ + "An extension can be requested in Play Console, but it only defers the requirement to November 1, 2026.", + requiredTargetSDKNew-20, requiredTargetSDKNew, deadlinePhrase(targetAPIDeadline)), + Fix: fmt.Sprintf("Set targetSdk = %d in your app module's android {} block and re-test behaviour changes for Android 16.", requiredTargetSDKNew), + Doc: docTargetAPI, + File: file, + Line: line, + }} + } + return nil +} + +// rulePlayBillingVersion checks the Play Billing Library support window. +func rulePlayBillingVersion(c *ruleContext) []Finding { + if c.gradle.BillingVersion == 0 { + return nil + } + if c.gradle.BillingVersion >= minSupportedBillingMajor { + return nil + } + return []Finding{{ + Severity: sevHigh, + Policy: "Play Billing Library", + Title: fmt.Sprintf("Play Billing Library %s is past its support window", c.gradle.BillingVersionRaw), + Detail: fmt.Sprintf( + "Play Billing Library 7 and below lose support and updates using them can no longer be published. %s. "+ + "There is no direct 7-to-9 upgrade: the version 8 migration has to be done first.", + deadlinePhrase(billingDeadline)), + Fix: fmt.Sprintf("Upgrade com.android.billingclient:billing to %d.x or higher, migrating through 8 if you are on 7.", minSupportedBillingMajor), + Doc: docBillingDeprecat, + File: c.gradle.BillingFile, + Line: c.gradle.BillingLine, + }} +} + +// restrictedPermission is a permission whose mere presence creates a Play +// obligation — a declaration form, an approved use case, or a demo video. +type restrictedPermission struct { + names []string + severity string + policy string + title string + detail string + fix string + doc string + // minTargetSDK gates policies that only bind above a target level; 0 means + // the policy applies regardless. + minTargetSDK int +} + +var restrictedPermissions = []restrictedPermission{ + { + names: []string{ + "android.permission.READ_SMS", "android.permission.SEND_SMS", + "android.permission.RECEIVE_SMS", "android.permission.RECEIVE_MMS", + "android.permission.RECEIVE_WAP_PUSH", "android.permission.WRITE_SMS", + }, + severity: sevHigh, + policy: "SMS and Call Log permissions", + title: "Restricted SMS permission requires an approved use case", + detail: "SMS permissions are restricted. The app must have a Play-approved core use case and a completed Permissions Declaration Form, " + + "or the update is rejected. Most one-time-password flows do not qualify because the SMS Retriever API covers them without the permission.", + fix: "Remove the permission and use the SMS Retriever API, or file the Permissions Declaration Form with your approved use case.", + doc: docRestrictedPerms, + }, + { + names: []string{"android.permission.READ_CALL_LOG", "android.permission.WRITE_CALL_LOG", "android.permission.PROCESS_OUTGOING_CALLS"}, + severity: sevHigh, + policy: "SMS and Call Log permissions", + title: "Restricted Call Log permission requires an approved use case", + detail: "Call Log permissions are restricted and need a Play-approved core use case plus a Permissions Declaration Form. " + + "As of the July 15, 2026 policy update, account verification by phone call is no longer a permitted use of READ_CALL_LOG, with compliance required by August 14, 2026.", + fix: "Drop the permission, or move verification to the Digital Credentials API or SMS Retriever API. Keep it only with an approved declaration.", + doc: docJul2026Policy, + }, + { + names: []string{"android.permission.MANAGE_EXTERNAL_STORAGE"}, + severity: sevHigh, + policy: "All files access", + title: "MANAGE_EXTERNAL_STORAGE (All files access) requires approval", + detail: "All files access is a restricted permission granted only to apps whose core purpose requires broad file management, such as file managers and backup tools. " + + "Requesting it without an approved declaration is a common rejection.", + fix: "Use the Storage Access Framework, MediaStore, or scoped directories instead. If the app genuinely needs it, file the declaration.", + doc: docRestrictedPerms, + }, + { + names: []string{"android.permission.QUERY_ALL_PACKAGES"}, + severity: sevHigh, + policy: "Package visibility", + title: "QUERY_ALL_PACKAGES requires an approved use case", + detail: "Broad package visibility is restricted to apps that must discover any installed app, such as launchers, antivirus, and accessibility tools. " + + "Other apps are expected to declare the specific packages they interact with.", + fix: "Replace with a element listing the packages or intents you actually need. Keep the permission only with an approved declaration.", + doc: docRestrictedPerms, + }, + { + names: []string{"android.permission.REQUEST_INSTALL_PACKAGES"}, + severity: sevHigh, + policy: "Device and Network Abuse", + title: "REQUEST_INSTALL_PACKAGES needs a permitted use case", + detail: "Installing other packages is restricted to a narrow set of use cases such as app stores and enterprise device management. " + + "It is also read as a signal of distributing code outside Play, which the Device and Network Abuse policy prohibits.", + fix: "Remove the permission unless the app's core purpose is app distribution or file management with an approved declaration.", + doc: docProgramPolicy, + }, + { + names: []string{"android.permission.ACCESS_BACKGROUND_LOCATION"}, + severity: sevHigh, + policy: "Background location", + title: "Background location requires a declaration and a demo video", + detail: "Background location access is reviewed individually. The app must show the feature delivers clear user benefit, get user-facing consent, " + + "and submit a video demonstrating the in-app flow that uses it. Review of this permission routinely takes multiple rounds.", + fix: "Confirm foreground location is genuinely insufficient. If it is not, remove the permission. Otherwise budget review time and prepare the demo video.", + doc: docRestrictedPerms, + }, + { + names: []string{"android.permission.READ_MEDIA_IMAGES", "android.permission.READ_MEDIA_VIDEO"}, + severity: sevWarn, + policy: "Photo and Video Permissions", + title: "Broad photo/video permission needs justification over the Photo Picker", + detail: "Apps targeting API 33 and above may request READ_MEDIA_IMAGES or READ_MEDIA_VIDEO only when the system photo picker cannot support the app's core functionality. One-off uploads such as an avatar picker do not qualify.", + fix: "Switch to the Android Photo Picker, which needs no permission. Keep broad access only if the app's core purpose requires a persistent full-library view.", + doc: docRestrictedPerms, + minTargetSDK: 33, + }, + { + names: []string{"android.permission.BIND_ACCESSIBILITY_SERVICE"}, + severity: sevHigh, + policy: "Accessibility API", + title: "Accessibility Service use is tightly restricted", + detail: "The Accessibility APIs may only be used to help users with disabilities, and the app must disclose the use in the store listing and in-app. " + + "Using accessibility for automation, overlays, or ad interaction is one of the most common causes of app suspension rather than simple rejection.", + fix: "Confirm the service exists to support users with disabilities and disclose it prominently. Otherwise use a purpose-built API.", + doc: docProgramPolicy, + }, + { + names: []string{"android.permission.SYSTEM_ALERT_WINDOW"}, + severity: sevWarn, + policy: "Overlay permissions", + title: "SYSTEM_ALERT_WINDOW draws over other apps", + detail: "Overlay windows are scrutinised because they are used to obscure disclosures and interfere with other apps. Overlays that hide consent dialogs or system UI violate policy.", + fix: "Use in-app UI, notifications, or picture-in-picture where possible. If the overlay is core, ensure it never obscures a permission or consent prompt.", + doc: docProgramPolicy, + }, + { + names: []string{"android.permission.BIND_DEVICE_ADMIN"}, + severity: sevWarn, + policy: "Device admin", + title: "Device administrator API requires a permitted use case", + detail: "Device admin is limited to genuine device management use cases. Using it to make the app hard to uninstall is an abuse signal.", + fix: "Confirm the app is an enterprise or family management tool, and that admin rights can be revoked normally.", + doc: docProgramPolicy, + }, + { + names: []string{"android.permission.PACKAGE_USAGE_STATS"}, + severity: sevWarn, + policy: "User Data", + title: "Usage access reads other apps' activity", + detail: "App usage data is personal and sensitive under the User Data policy. It requires prominent in-app disclosure and consent before collection, in addition to the Data safety declaration.", + fix: "Add a prominent disclosure before requesting usage access and declare the collection in the Data safety form.", + doc: docProgramPolicy, + }, + { + names: []string{"android.permission.READ_CONTACTS", "android.permission.WRITE_CONTACTS"}, + severity: sevInfo, + policy: "Contact Permissions", + title: "Contacts access is narrowing under the 2026 Contact Permissions policy", + detail: "A Contact Permissions policy introduced in April 2026 restricts READ_CONTACTS for apps targeting API 37 and above to cases where the system Contact Picker is insufficient. Contacts also require prominent disclosure and a Data safety declaration today.", + fix: "Plan a move to the system Contact Picker before targeting API 37, and confirm contacts are declared in the Data safety form.", + doc: docRestrictedPerms, + }, +} + +func ruleRestrictedPermissions(c *ruleContext) []Finding { + if c.manifest == nil { + return nil + } + var findings []Finding + for _, rp := range restrictedPermissions { + if rp.minTargetSDK > 0 && c.targetSDK > 0 && c.targetSDK < rp.minTargetSDK { + continue + } + var hit []string + for _, name := range rp.names { + if c.manifest.HasPermission(name) { + hit = append(hit, shortPermission(name)) + } + } + if len(hit) == 0 { + continue + } + findings = append(findings, Finding{ + Severity: rp.severity, + Policy: rp.policy, + Title: rp.title, + Detail: fmt.Sprintf("Declared: %s. %s", strings.Join(hit, ", "), rp.detail), + Fix: rp.fix, + Doc: rp.doc, + File: c.manifestFile, + }) + } + return findings +} + +// foregroundServicePermissions maps each foreground service type to the +// permission Android 14 requires alongside it. A type declared without its +// permission throws SecurityException at startForeground() on API 34+. +var foregroundServicePermissions = map[string]string{ + "camera": "android.permission.FOREGROUND_SERVICE_CAMERA", + "connectedDevice": "android.permission.FOREGROUND_SERVICE_CONNECTED_DEVICE", + "dataSync": "android.permission.FOREGROUND_SERVICE_DATA_SYNC", + "health": "android.permission.FOREGROUND_SERVICE_HEALTH", + "location": "android.permission.FOREGROUND_SERVICE_LOCATION", + "mediaPlayback": "android.permission.FOREGROUND_SERVICE_MEDIA_PLAYBACK", + "mediaProjection": "android.permission.FOREGROUND_SERVICE_MEDIA_PROJECTION", + "microphone": "android.permission.FOREGROUND_SERVICE_MICROPHONE", + "phoneCall": "android.permission.FOREGROUND_SERVICE_PHONE_CALL", + "remoteMessaging": "android.permission.FOREGROUND_SERVICE_REMOTE_MESSAGING", + "shortService": "", + "specialUse": "android.permission.FOREGROUND_SERVICE_SPECIAL_USE", + "systemExempted": "android.permission.FOREGROUND_SERVICE_SYSTEM_EXEMPTED", + "mediaProcessing": "android.permission.FOREGROUND_SERVICE_MEDIA_PROCESSING", + "fileManagement": "android.permission.FOREGROUND_SERVICE_FILE_MANAGEMENT", +} + +// ruleForegroundServiceTypes checks that every declared foreground service type +// has its matching permission, and flags specialUse, which needs a separate +// Play Console declaration. +func ruleForegroundServiceTypes(c *ruleContext) []Finding { + if c.manifest == nil || c.manifest.Application == nil { + return nil + } + var findings []Finding + reported := make(map[string]bool) + + for _, svc := range c.manifest.Application.Services { + if svc.ForegroundSvcType == "" { + continue + } + // A service may declare multiple types, pipe-separated. + for _, t := range strings.Split(svc.ForegroundSvcType, "|") { + t = strings.TrimSpace(t) + perm, known := foregroundServicePermissions[t] + if !known || perm == "" || reported[t] { + continue + } + if c.manifest.HasPermission(perm) { + continue + } + reported[t] = true + findings = append(findings, Finding{ + Severity: sevCritical, + Policy: "Foreground services", + Title: fmt.Sprintf("Foreground service type %q is missing its required permission", t), + Detail: fmt.Sprintf( + "Service %s declares foregroundServiceType=%q but the manifest does not request %s. "+ + "On Android 14 and above the app throws SecurityException the moment it calls startForeground(), so the feature crashes on every current device.", + displayComponentName(svc.Name), t, perm), + Fix: fmt.Sprintf("Add to the manifest.", perm), + Doc: docForegroundSvc, + File: c.manifestFile, + }) + } + + if strings.Contains(svc.ForegroundSvcType, "specialUse") && !reported["specialUse-decl"] { + reported["specialUse-decl"] = true + findings = append(findings, Finding{ + Severity: sevHigh, + Policy: "Foreground services", + Title: "specialUse foreground service requires a Play Console declaration", + Detail: "The specialUse foreground service type exists for cases none of the defined types cover, and Play requires a written justification in Console describing the use case. " + + "Submitting without it is rejected.", + Fix: "Declare the specialUse justification in Play Console under App content, or move the service to a defined foreground service type.", + Doc: docForegroundSvc, + File: c.manifestFile, + }) + } + } + return findings +} + +func ruleDebuggable(c *ruleContext) []Finding { + if c.manifest == nil || c.manifest.Application == nil { + return nil + } + if !attrIsTrue(c.manifest.Application.Debuggable) { + return nil + } + return []Finding{{ + Severity: sevCritical, + Policy: "Malicious Behavior", + Title: "android:debuggable is set to true", + Detail: "Google Play rejects uploads whose manifest is marked debuggable. It also exposes the app's internals to any process on the device, " + + "which is a security finding in its own right.", + Fix: "Remove android:debuggable from the manifest and let the build type control it. Release builds must never set it.", + Doc: docProgramPolicy, + File: c.manifestFile, + }} +} + +// ruleExportedComponents catches the API 31 android:exported requirement. This +// is a hard install failure, not merely a policy matter. +func ruleExportedComponents(c *ruleContext) []Finding { + if c.manifest == nil || c.manifest.Application == nil { + return nil + } + // The requirement binds when targeting API 31+. An unknown target is not + // assumed to be affected. + if c.targetSDK != 0 && c.targetSDK < 31 { + return nil + } + + var missing []string + for _, comp := range c.manifest.Application.Components() { + if comp.HasIntentFilter() && !attrIsSet(comp.Exported) { + missing = append(missing, displayComponentName(comp.Name)) + } + } + if len(missing) == 0 { + return nil + } + if len(missing) > 5 { + missing = append(missing[:5], fmt.Sprintf("and %d more", len(missing)-5)) + } + return []Finding{{ + Severity: sevCritical, + Policy: "Manifest requirements", + Title: "Component with an intent filter is missing android:exported", + Detail: fmt.Sprintf( + "Apps targeting API 31 and above must set android:exported explicitly on every activity, service, and receiver that declares an intent filter. "+ + "Without it the package fails to install. Missing on: %s.", + strings.Join(missing, ", ")), + Fix: "Add android:exported=\"true\" or \"false\" to each component listed, choosing false unless another app genuinely needs to start it.", + Doc: docProgramPolicy, + File: c.manifestFile, + }} +} + +func ruleCleartextTraffic(c *ruleContext) []Finding { + if c.manifest == nil || c.manifest.Application == nil { + return nil + } + if !attrIsTrue(c.manifest.Application.UsesCleartextTraffic) { + return nil + } + return []Finding{{ + Severity: sevWarn, + Policy: "User Data", + Title: "android:usesCleartextTraffic is enabled", + Detail: "The app opts in to unencrypted HTTP for all destinations. Play requires personal and sensitive user data to be transmitted securely, " + + "and cleartext traffic is a standard finding in pre-launch security reports.", + Fix: "Remove the attribute and use HTTPS. If specific legacy hosts need cleartext, allow only those in a network security config.", + Doc: docProgramPolicy, + File: c.manifestFile, + }} +} + +// ruleAdvertisingID checks the AD_ID permission that ads SDKs require from +// API 33. Without it the advertising ID silently returns zeros rather than +// failing loudly, so this usually ships unnoticed and quietly breaks +// attribution. +func ruleAdvertisingID(c *ruleContext) []Finding { + if !c.gradle.HasAdsSDK { + return nil + } + if c.targetSDK != 0 && c.targetSDK < 33 { + return nil + } + if c.hasPermission("com.google.android.gms.permission.AD_ID") { + return nil + } + return []Finding{{ + Severity: sevHigh, + Policy: "Advertising ID", + Title: "Ads SDK present without the AD_ID permission", + Detail: "An advertising or monetization SDK is a dependency, but the manifest does not declare com.google.android.gms.permission.AD_ID. " + + "Apps targeting API 33 and above must declare it to read the advertising ID; without it the ID comes back as all zeros and attribution and ad revenue degrade silently. " + + "The app's use of the advertising ID must also be declared in the Play Console Advertising ID section.", + Fix: "Add and confirm the Advertising ID declaration in Play Console.", + Doc: docAdvertisingID, + File: c.gradle.AdsSDKFile, + Line: c.gradle.AdsSDKLine, + }} +} + +// ruleAccountDeletion surfaces the account deletion requirement when the app +// ships an auth SDK. Play requires deletion to be reachable two ways, and the +// web URL half is the one teams forget because nothing in the app references it. +func ruleAccountDeletion(c *ruleContext) []Finding { + if !c.gradle.HasAuthSDK { + return nil + } + return []Finding{{ + Severity: sevHigh, + Policy: "Account deletion", + Title: "Account creation requires in-app AND web account deletion", + Detail: "An authentication SDK is a dependency, so the app appears to let users create accounts. Play requires such apps to offer account deletion from inside the app " + + "and through a web URL that works without reinstalling, with that URL entered in the Data safety form. Deactivating or freezing an account does not satisfy this, " + + "and associated user data must actually be deleted.", + Fix: "Ship an in-app deletion flow, publish a deletion web page, and enter its URL in the Data safety section of Play Console.", + Doc: docAccountDeletion, + }} +} + +// shortPermission trims the well-known android.permission prefix so findings +// read cleanly, leaving custom and vendor permissions fully qualified. +func shortPermission(name string) string { + return strings.TrimPrefix(name, "android.permission.") +} + +// displayComponentName renders a manifest component name, which is commonly a +// relative ".MainActivity" form. +func displayComponentName(name string) string { + if name == "" { + return "(unnamed)" + } + return name +} + +// DocAppContent is exported so callers can point users at the Play Console +// declarations that no static scan can verify. +const DocAppContent = docAppContent diff --git a/internal/playscan/scanner.go b/internal/playscan/scanner.go new file mode 100644 index 0000000..3b87faf --- /dev/null +++ b/internal/playscan/scanner.go @@ -0,0 +1,272 @@ +package playscan + +import ( + "errors" + "io/fs" + "path/filepath" + "sort" + "strings" +) + +// skipDirs are never walked when looking for Android sources. Build outputs in +// particular contain generated and merged manifests that would otherwise be +// scanned as if a developer had written them. +var skipDirs = map[string]bool{ + "node_modules": true, ".git": true, "build": true, ".gradle": true, + "dist": true, ".expo": true, "Pods": true, "vendor": true, + ".next": true, "DerivedData": true, "captures": true, ".idea": true, +} + +// Detect reports whether the given directory looks like an Android project. +// +// A React Native or Expo project with a prebuilt android/ directory counts: +// those ship a real manifest and Gradle build, and are subject to every Play +// policy a native project is. +// +// This short-circuits on the first Android file it sees rather than reusing +// discover(), because callers use it to decide whether to run the scan at all +// and should not pay for a second full walk of the tree. +func Detect(root string) bool { + found := false + errStop := errors.New("found") + _ = filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error { + if err != nil { + return nil + } + if d.IsDir() { + if skipDirs[d.Name()] { + return filepath.SkipDir + } + return nil + } + if d.Name() == "AndroidManifest.xml" || isGradleFile(d.Name()) { + found = true + return errStop + } + return nil + }) + return found +} + +// Scan runs every Play policy rule against an Android project. +// +// A directory with no Android sources is not an error; it returns a result with +// no manifest and no findings, so the caller can run this unconditionally. +func Scan(root string) (*ScanResult, error) { + result := &ScanResult{ProjectPath: root} + + manifestPath, gradlePaths := discover(root) + if manifestPath == "" && len(gradlePaths) == 0 { + return result, nil + } + + relTo := func(p string) string { + rel, err := filepath.Rel(root, p) + if err != nil { + return p + } + return rel + } + + gradle := parseGradleFiles(gradlePaths, relTo) + + // A Gradle build with no Android manifest and no Android plugin is some + // other kind of JVM project. Returning empty keeps Play findings off repos + // that never ship to Play. + if manifestPath == "" && !gradle.HasAndroidPlugin { + return result, nil + } + + var manifest *Manifest + if manifestPath != "" { + m, err := ParseManifest(manifestPath) + if err != nil { + // A manifest that does not parse is worth reporting rather than + // swallowing: the developer's build would fail on it too, and a + // silent skip would look like a clean scan. + result.Findings = append(result.Findings, Finding{ + Severity: sevWarn, + Policy: "Scan coverage", + Title: "AndroidManifest.xml could not be parsed", + Detail: "The manifest is not well-formed XML, so manifest-based policy checks were skipped: " + err.Error(), + Fix: "Fix the XML syntax error, then re-run the scan.", + File: relTo(manifestPath), + }) + result.ManifestPath = relTo(manifestPath) + return result, nil + } + manifest = m + result.ManifestPath = relTo(manifestPath) + result.PackageName = m.Package + result.Permissions = m.AllPermissions() + } + + // AGP 8 removed the manifest package attribute in favour of the Gradle + // namespace, so fall back to it rather than showing no package at all. + if result.PackageName == "" { + result.PackageName = gradle.Namespace + } + + // Gradle's android {} block wins over the manifest's , matching + // what AGP does: the DSL value overwrites the manifest attribute at merge. + result.TargetSDK = gradle.TargetSDK + result.MinSDK = gradle.MinSDK + if manifest != nil && manifest.UsesSDK != nil { + if result.TargetSDK == 0 { + result.TargetSDK = parseSDKInt(manifest.UsesSDK.TargetSDKVersion) + } + if result.MinSDK == 0 { + result.MinSDK = parseSDKInt(manifest.UsesSDK.MinSDKVersion) + } + } + + ctx := &ruleContext{ + manifest: manifest, + gradle: gradle, + targetSDK: result.TargetSDK, + manifestFile: result.ManifestPath, + } + for _, rule := range allRules() { + result.Findings = append(result.Findings, rule(ctx)...) + } + + return result, nil +} + +// discover locates the AndroidManifest.xml and Gradle scripts to read. +// +// Gradle paths come back most-specific-first so parseGradleFiles resolves the +// app module's values before any root-project ext defaults. +func discover(root string) (manifestPath string, gradlePaths []string) { + var manifests, gradles []string + + _ = filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error { + if err != nil { + return nil + } + if d.IsDir() { + if skipDirs[d.Name()] { + return filepath.SkipDir + } + return nil + } + switch { + case d.Name() == "AndroidManifest.xml": + manifests = append(manifests, path) + case isGradleFile(d.Name()), isVersionCatalog(d.Name()), + isConventionPlugin(path), isGradleProperties(d.Name()): + gradles = append(gradles, path) + } + return nil + }) + + sort.SliceStable(manifests, func(i, j int) bool { + si, sj := manifestScore(manifests[i]), manifestScore(manifests[j]) + if si != sj { + return si > sj + } + return manifests[i] < manifests[j] + }) + sort.Slice(gradles, func(i, j int) bool { + si, sj := gradleScore(gradles[i]), gradleScore(gradles[j]) + if si != sj { + return si > sj + } + return gradles[i] < gradles[j] + }) + + if len(manifests) > 0 { + manifestPath = selectAppManifest(manifests) + } + return manifestPath, gradles +} + +// selectAppManifest picks the shipped application's manifest out of every +// candidate in the repo. +// +// Path shape alone is not enough: multi-module projects routinely name the app +// module something other than "app" (AnkiDroid's is "AnkiDroid"), which lets a +// benchmark or library module win on path score and silently hide the real +// app's permissions. The launcher activity is the definitive signal, so +// candidates are parsed and one declaring MAIN/LAUNCHER wins outright. +func selectAppManifest(candidates []string) string { + best, bestScore := candidates[0], -1<<30 + for _, path := range candidates { + score := manifestScore(path) + if m, err := ParseManifest(path); err == nil && m.HasLauncherActivity() { + score += 1000 + } + if score > bestScore { + best, bestScore = path, score + } + } + return best +} + +// manifestScore ranks candidate manifests by path shape, used to break ties +// once the launcher-activity signal has been applied. +func manifestScore(path string) int { + p := filepath.ToSlash(path) + score := 0 + if strings.Contains(p, "/src/main/") { + score += 10 + } + if strings.Contains(p, "/app/") { + score += 5 + } + // Non-shipping source sets never hold the app's real configuration. + for _, sourceSet := range []string{"/src/debug/", "/src/androidTest/", "/src/test/", "/src/benchmark/", "/src/nightly/"} { + if strings.Contains(p, sourceSet) { + score -= 20 + } + } + // Modules that exist to measure or test the app, not to ship it. + for _, module := range []string{"/baselineprofile/", "/benchmark/", "/macrobenchmark/", "/lint/", "/testlib/"} { + if strings.Contains(p, module) { + score -= 30 + } + } + // Shallower manifests are more likely to be the application's own. + score -= strings.Count(p, "/") + return score +} + +// gradleScore ranks build files by how authoritative they are for the shipped +// application's configuration, most authoritative first. +// +// The app module's own android {} block wins, then the convention plugin that +// configures application modules, then other build-logic sources, then the +// version catalog those files interpolate from. +func gradleScore(path string) int { + p := filepath.ToSlash(path) + base := strings.ToLower(filepath.Base(p)) + score := 0 + + switch { + case isGradleProperties(base): + // Properties only hold values other files reference. + score += 2 + case isVersionCatalog(base): + // A catalog only holds the values other files reference, so it is the + // last resort rather than a statement about the app module. + score += 2 + case isConventionPlugin(path): + score += 6 + // The plugin applied to application modules is the one whose targetSdk + // actually ships; library and test plugins configure other things. + if strings.Contains(strings.ToLower(base), "application") { + score += 4 + } + if strings.Contains(strings.ToLower(base), "library") || strings.Contains(strings.ToLower(base), "test") { + score -= 4 + } + case strings.HasPrefix(base, "build.gradle"): + score += 5 + if strings.Contains(p, "/app/") { + score += 10 + } + case strings.HasPrefix(base, "settings.gradle"): + score -= 5 + } + return score +} diff --git a/internal/playscan/scanner_test.go b/internal/playscan/scanner_test.go new file mode 100644 index 0000000..27cca51 --- /dev/null +++ b/internal/playscan/scanner_test.go @@ -0,0 +1,916 @@ +package playscan + +import ( + "os" + "path/filepath" + "strings" + "testing" + "time" +) + +// writeProject materializes a map of relative paths to contents in a temp dir. +func writeProject(t *testing.T, files map[string]string) string { + t.Helper() + root := t.TempDir() + for rel, content := range files { + full := filepath.Join(root, rel) + if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil { + t.Fatalf("mkdir %s: %v", rel, err) + } + if err := os.WriteFile(full, []byte(content), 0o644); err != nil { + t.Fatalf("write %s: %v", rel, err) + } + } + return root +} + +// findByPolicy returns the first finding for a policy, or nil. +func findByPolicy(findings []Finding, policy string) *Finding { + for i := range findings { + if findings[i].Policy == policy { + return &findings[i] + } + } + return nil +} + +func countByPolicy(findings []Finding, policy string) int { + n := 0 + for _, f := range findings { + if f.Policy == policy { + n++ + } + } + return n +} + +// freezeClock pins the deadline clock so day-count wording is deterministic. +func freezeClock(t *testing.T, at time.Time) { + t.Helper() + prev := now + now = func() time.Time { return at } + t.Cleanup(func() { now = prev }) +} + +const manifestHeader = ` +` + +func TestParseManifestReadsNamespacedAttributes(t *testing.T) { + root := writeProject(t, map[string]string{ + "app/src/main/AndroidManifest.xml": manifestHeader + ` + + + + + + + + + + + + +`, + }) + + m, err := ParseManifest(filepath.Join(root, "app/src/main/AndroidManifest.xml")) + if err != nil { + t.Fatalf("parse: %v", err) + } + + if m.Package != "com.example.app" { + t.Errorf("package = %q, want com.example.app", m.Package) + } + if !m.HasPermission("android.permission.READ_SMS") { + t.Error("READ_SMS not detected") + } + // uses-permission-sdk-23 carries the same obligations as uses-permission. + if !m.HasPermission("android.permission.CAMERA") { + t.Error("uses-permission-sdk-23 CAMERA not detected") + } + if m.HasPermission("android.permission.READ_CONTACTS") { + t.Error("undeclared permission reported as present") + } + if got := parseSDKInt(m.UsesSDK.TargetSDKVersion); got != 33 { + t.Errorf("targetSdkVersion = %d, want 33", got) + } + if !attrIsTrue(m.Application.Debuggable) { + t.Error("debuggable attribute not read") + } + if len(m.Application.Services) != 1 || m.Application.Services[0].ForegroundSvcType != "dataSync" { + t.Errorf("foregroundServiceType not read: %+v", m.Application.Services) + } + if !m.Application.Activities[0].HasIntentFilter() { + t.Error("intent-filter not read") + } +} + +func TestHasPermissionRequiresExactName(t *testing.T) { + root := writeProject(t, map[string]string{ + "app/src/main/AndroidManifest.xml": manifestHeader + ` + + +`, + }) + m, err := ParseManifest(filepath.Join(root, "app/src/main/AndroidManifest.xml")) + if err != nil { + t.Fatalf("parse: %v", err) + } + // A custom permission that merely ends in READ_SMS must not trip the + // restricted-permission rule. + if m.HasPermission("android.permission.READ_SMS") { + t.Error("custom permission matched the platform permission") + } +} + +func TestGradleTargetSDKForms(t *testing.T) { + cases := []struct { + name string + line string + want int + }{ + {"groovy bare", "targetSdkVersion 34", 34}, + {"groovy paren", "targetSdkVersion(34)", 34}, + {"kts assign", "targetSdk = 35", 35}, + {"kts short", "targetSdk 36", 36}, + {"kts set", "targetSdk.set(33)", 33}, + {"quoted", `targetSdkVersion "34"`, 34}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + root := writeProject(t, map[string]string{ + "app/build.gradle": "android {\n defaultConfig {\n " + tc.line + "\n }\n}\n", + }) + res, err := Scan(root) + if err != nil { + t.Fatalf("scan: %v", err) + } + if res.TargetSDK != tc.want { + t.Errorf("TargetSDK = %d, want %d", res.TargetSDK, tc.want) + } + }) + } +} + +// An interpolated targetSdk in the app module must resolve from the root ext +// block, which is the standard Expo / React Native layout. +func TestGradleTargetSDKFromRootExtBlock(t *testing.T) { + root := writeProject(t, map[string]string{ + "android/build.gradle": `buildscript { + ext { + minSdkVersion = 24 + targetSdkVersion = 34 + } +}`, + "android/app/build.gradle": `android { + defaultConfig { + targetSdkVersion rootProject.ext.targetSdkVersion + } +}`, + }) + res, err := Scan(root) + if err != nil { + t.Fatalf("scan: %v", err) + } + if res.TargetSDK != 34 { + t.Errorf("TargetSDK = %d, want 34 resolved from the root ext block", res.TargetSDK) + } +} + +func TestGradleIgnoresCommentedDependency(t *testing.T) { + root := writeProject(t, map[string]string{ + "app/build.gradle": `dependencies { + // implementation 'com.android.billingclient:billing:5.0.0' + implementation 'androidx.core:core-ktx:1.13.0' +}`, + }) + res, err := Scan(root) + if err != nil { + t.Fatalf("scan: %v", err) + } + if f := findByPolicy(res.Findings, "Play Billing Library"); f != nil { + t.Errorf("commented-out billing dependency produced a finding: %s", f.Title) + } +} + +func TestTargetAPILevelSeverityTiers(t *testing.T) { + freezeClock(t, time.Date(2026, time.July, 27, 0, 0, 0, 0, time.UTC)) + + cases := []struct { + targetSDK int + wantSeverity string + wantFinding bool + }{ + {34, sevCritical, true}, // below the existing-app floor + {35, sevHigh, true}, // meets the floor, below the new-submission bar + {36, "", false}, // compliant + {37, "", false}, // ahead of the requirement + } + for _, tc := range cases { + root := writeProject(t, map[string]string{ + "app/build.gradle": "android { defaultConfig { targetSdk = " + itoa(tc.targetSDK) + " } }", + }) + res, err := Scan(root) + if err != nil { + t.Fatalf("scan: %v", err) + } + f := findByPolicy(res.Findings, "Target API level") + if !tc.wantFinding { + if f != nil { + t.Errorf("targetSdk %d: unexpected finding %q", tc.targetSDK, f.Title) + } + continue + } + if f == nil { + t.Errorf("targetSdk %d: expected a finding, got none", tc.targetSDK) + continue + } + if f.Severity != tc.wantSeverity { + t.Errorf("targetSdk %d: severity = %s, want %s", tc.targetSDK, f.Severity, tc.wantSeverity) + } + if !strings.Contains(f.Detail, "35 days left") { + t.Errorf("targetSdk %d: detail missing the day countdown: %s", tc.targetSDK, f.Detail) + } + } +} + +func TestDeadlinePhraseAfterDeadline(t *testing.T) { + freezeClock(t, time.Date(2026, time.September, 15, 0, 0, 0, 0, time.UTC)) + got := deadlinePhrase(targetAPIDeadline) + if !strings.Contains(got, "has passed") { + t.Errorf("after the deadline the phrase should say it passed, got %q", got) + } +} + +func TestUnresolvableTargetSDKWarnsRatherThanPasses(t *testing.T) { + root := writeProject(t, map[string]string{ + "app/build.gradle": `android { + defaultConfig { + targetSdkVersion libs.versions.targetSdk.get().toInteger() + } +}`, + }) + res, err := Scan(root) + if err != nil { + t.Fatalf("scan: %v", err) + } + f := findByPolicy(res.Findings, "Target API level") + if f == nil || f.Severity != sevWarn { + t.Fatalf("an unresolvable targetSdk must warn rather than silently pass, got %+v", f) + } +} + +func TestRestrictedPermissionsFire(t *testing.T) { + root := writeProject(t, map[string]string{ + "app/build.gradle": "android { defaultConfig { targetSdk = 36 } }", + "app/src/main/AndroidManifest.xml": manifestHeader + ` + + + + + + +`, + }) + res, err := Scan(root) + if err != nil { + t.Fatalf("scan: %v", err) + } + + for _, policy := range []string{ + "SMS and Call Log permissions", "All files access", + "Package visibility", "Background location", + } { + if findByPolicy(res.Findings, policy) == nil { + t.Errorf("expected a finding for policy %q", policy) + } + } + // SMS and Call Log are separate entries under one policy name. + if n := countByPolicy(res.Findings, "SMS and Call Log permissions"); n != 2 { + t.Errorf("SMS + Call Log should produce 2 findings, got %d", n) + } + // Every finding must cite a policy page. + for _, f := range res.Findings { + if f.Doc == "" { + t.Errorf("finding %q has no Doc link", f.Title) + } + } +} + +// The photo/video policy only binds at targetSdk 33+, so a lower target must +// not produce the finding. +func TestPhotoPermissionGatedByTargetSDK(t *testing.T) { + manifest := manifestHeader + ` + + +` + + low := writeProject(t, map[string]string{ + "app/build.gradle": "android { defaultConfig { targetSdk = 32 } }", + "app/src/main/AndroidManifest.xml": manifest, + }) + res, err := Scan(low) + if err != nil { + t.Fatalf("scan: %v", err) + } + if f := findByPolicy(res.Findings, "Photo and Video Permissions"); f != nil { + t.Error("photo permission policy fired below targetSdk 33") + } + + high := writeProject(t, map[string]string{ + "app/build.gradle": "android { defaultConfig { targetSdk = 36 } }", + "app/src/main/AndroidManifest.xml": manifest, + }) + res, err = Scan(high) + if err != nil { + t.Fatalf("scan: %v", err) + } + if f := findByPolicy(res.Findings, "Photo and Video Permissions"); f == nil { + t.Error("photo permission policy did not fire at targetSdk 36") + } +} + +func TestForegroundServiceTypeMissingPermission(t *testing.T) { + root := writeProject(t, map[string]string{ + "app/build.gradle": "android { defaultConfig { targetSdk = 36 } }", + "app/src/main/AndroidManifest.xml": manifestHeader + ` + + + + +`, + }) + res, err := Scan(root) + if err != nil { + t.Fatalf("scan: %v", err) + } + f := findByPolicy(res.Findings, "Foreground services") + if f == nil { + t.Fatal("missing FOREGROUND_SERVICE_LOCATION was not reported") + } + if f.Severity != sevCritical { + t.Errorf("severity = %s, want CRITICAL (it crashes at startForeground)", f.Severity) + } + if !strings.Contains(f.Detail, "FOREGROUND_SERVICE_LOCATION") { + t.Errorf("detail should name the missing permission: %s", f.Detail) + } +} + +func TestForegroundServiceTypeWithPermissionIsClean(t *testing.T) { + root := writeProject(t, map[string]string{ + "app/build.gradle": "android { defaultConfig { targetSdk = 36 } }", + "app/src/main/AndroidManifest.xml": manifestHeader + ` + + + + + +`, + }) + res, err := Scan(root) + if err != nil { + t.Fatalf("scan: %v", err) + } + if f := findByPolicy(res.Findings, "Foreground services"); f != nil { + t.Errorf("declared permission should satisfy the rule, got %q", f.Title) + } +} + +// Multiple pipe-separated types each need their own permission. +func TestForegroundServiceMultipleTypes(t *testing.T) { + root := writeProject(t, map[string]string{ + "app/build.gradle": "android { defaultConfig { targetSdk = 36 } }", + "app/src/main/AndroidManifest.xml": manifestHeader + ` + + + + +`, + }) + res, err := Scan(root) + if err != nil { + t.Fatalf("scan: %v", err) + } + f := findByPolicy(res.Findings, "Foreground services") + if f == nil || !strings.Contains(f.Detail, "FOREGROUND_SERVICE_CAMERA") { + t.Fatalf("the camera half of location|camera was not checked, got %+v", f) + } +} + +func TestDebuggableIsCritical(t *testing.T) { + root := writeProject(t, map[string]string{ + "app/build.gradle": "android { defaultConfig { targetSdk = 36 } }", + "app/src/main/AndroidManifest.xml": manifestHeader + ` + +`, + }) + res, err := Scan(root) + if err != nil { + t.Fatalf("scan: %v", err) + } + f := findByPolicy(res.Findings, "Malicious Behavior") + if f == nil || f.Severity != sevCritical { + t.Fatalf("debuggable=true must be CRITICAL, got %+v", f) + } +} + +// A placeholder or resource reference is not knowable statically and must not +// be reported as a violation. +func TestDebuggablePlaceholderIsNotReported(t *testing.T) { + for _, value := range []string{"${debuggable}", "@bool/is_debuggable", "false"} { + root := writeProject(t, map[string]string{ + "app/build.gradle": "android { defaultConfig { targetSdk = 36 } }", + "app/src/main/AndroidManifest.xml": manifestHeader + ` + +`, + }) + res, err := Scan(root) + if err != nil { + t.Fatalf("scan: %v", err) + } + if f := findByPolicy(res.Findings, "Malicious Behavior"); f != nil { + t.Errorf("debuggable=%q should not be reported, got %q", value, f.Title) + } + } +} + +func TestExportedMissingOnIntentFilterComponent(t *testing.T) { + root := writeProject(t, map[string]string{ + "app/build.gradle": "android { defaultConfig { targetSdk = 36 } }", + "app/src/main/AndroidManifest.xml": manifestHeader + ` + + + + + + + + + + + + +`, + }) + res, err := Scan(root) + if err != nil { + t.Fatalf("scan: %v", err) + } + f := findByPolicy(res.Findings, "Manifest requirements") + if f == nil { + t.Fatal("missing android:exported was not reported") + } + if !strings.Contains(f.Detail, ".MainActivity") { + t.Errorf("should name the offending component: %s", f.Detail) + } + // The receiver sets exported explicitly, so it must not be listed. + if strings.Contains(f.Detail, ".BootReceiver") { + t.Errorf("component with an explicit exported was listed: %s", f.Detail) + } +} + +func TestExportedRuleSkippedBelowAPI31(t *testing.T) { + root := writeProject(t, map[string]string{ + "app/build.gradle": "android { defaultConfig { targetSdk = 30 } }", + "app/src/main/AndroidManifest.xml": manifestHeader + ` + + + + + +`, + }) + res, err := Scan(root) + if err != nil { + t.Fatalf("scan: %v", err) + } + if f := findByPolicy(res.Findings, "Manifest requirements"); f != nil { + t.Error("the exported requirement does not bind below API 31") + } +} + +func TestAdvertisingIDPermissionMissing(t *testing.T) { + root := writeProject(t, map[string]string{ + "app/build.gradle": `android { defaultConfig { targetSdk = 36 } } +dependencies { + implementation 'com.google.android.gms:play-services-ads:23.0.0' +}`, + "app/src/main/AndroidManifest.xml": manifestHeader + ` + +`, + }) + res, err := Scan(root) + if err != nil { + t.Fatalf("scan: %v", err) + } + f := findByPolicy(res.Findings, "Advertising ID") + if f == nil { + t.Fatal("ads SDK without AD_ID was not reported") + } + if f.Line == 0 { + t.Error("finding should point at the dependency line") + } +} + +func TestAdvertisingIDPermissionPresentIsClean(t *testing.T) { + root := writeProject(t, map[string]string{ + "app/build.gradle": `android { defaultConfig { targetSdk = 36 } } +dependencies { implementation 'com.google.android.gms:play-services-ads:23.0.0' }`, + "app/src/main/AndroidManifest.xml": manifestHeader + ` + + +`, + }) + res, err := Scan(root) + if err != nil { + t.Fatalf("scan: %v", err) + } + if f := findByPolicy(res.Findings, "Advertising ID"); f != nil { + t.Errorf("AD_ID is declared, expected no finding, got %q", f.Title) + } +} + +func TestBillingVersionBelowSupported(t *testing.T) { + freezeClock(t, time.Date(2026, time.July, 27, 0, 0, 0, 0, time.UTC)) + root := writeProject(t, map[string]string{ + "app/build.gradle": `android { defaultConfig { targetSdk = 36 } } +dependencies { implementation 'com.android.billingclient:billing:7.1.1' }`, + }) + res, err := Scan(root) + if err != nil { + t.Fatalf("scan: %v", err) + } + f := findByPolicy(res.Findings, "Play Billing Library") + if f == nil { + t.Fatal("Play Billing 7 was not reported") + } + if !strings.Contains(f.Title, "7.1") { + t.Errorf("title should carry the detected version, got %q", f.Title) + } +} + +func TestBillingVersionSupportedIsClean(t *testing.T) { + root := writeProject(t, map[string]string{ + "app/build.gradle": `dependencies { implementation("com.android.billingclient:billing-ktx:8.0.0") }`, + }) + res, err := Scan(root) + if err != nil { + t.Fatalf("scan: %v", err) + } + if f := findByPolicy(res.Findings, "Play Billing Library"); f != nil { + t.Errorf("Billing 8 is supported, got %q", f.Title) + } +} + +func TestAccountDeletionFiresOnAuthSDK(t *testing.T) { + root := writeProject(t, map[string]string{ + "app/build.gradle": `android { defaultConfig { targetSdk = 36 } } +dependencies { implementation 'com.google.firebase:firebase-auth:23.0.0' }`, + }) + res, err := Scan(root) + if err != nil { + t.Fatalf("scan: %v", err) + } + if findByPolicy(res.Findings, "Account deletion") == nil { + t.Error("an auth SDK should raise the account deletion requirement") + } +} + +// A Gradle project with no Android manifest and no Android plugin is a JVM +// project, and must not collect Play findings. +func TestPureJVMGradleProjectIsNotScanned(t *testing.T) { + root := writeProject(t, map[string]string{ + "build.gradle.kts": `plugins { kotlin("jvm") version "2.0.0" } +dependencies { implementation("org.jetbrains.kotlinx:kotlinx-coroutines-core:1.8.0") }`, + "gradle/libs.versions.toml": "[versions]\nkotlin = \"2.0.0\"\n", + }) + res, err := Scan(root) + if err != nil { + t.Fatalf("scan: %v", err) + } + if len(res.Findings) != 0 { + t.Errorf("a pure JVM Gradle project produced %d Play findings: %+v", len(res.Findings), res.Findings) + } +} + +// Convention plugins are where many modern projects set targetSdk, and the +// library plugin's lint.targetSdk must never be read as the app's target. +func TestTargetSDKFromConventionPlugin(t *testing.T) { + root := writeProject(t, map[string]string{ + "app/build.gradle.kts": `plugins { id("nowinandroid.android.application") }`, + "build-logic/convention/src/main/kotlin/AndroidApplicationConventionPlugin.kt": `class AndroidApplicationConventionPlugin : Plugin { + override fun apply(target: Project) { + with(target) { + defaultConfig.targetSdk = 36 + } + } +}`, + "build-logic/convention/src/main/kotlin/AndroidLibraryConventionPlugin.kt": `class AndroidLibraryConventionPlugin : Plugin { + override fun apply(target: Project) { + lint.targetSdk = 30 + testOptions.targetSdk = 30 + } +}`, + }) + res, err := Scan(root) + if err != nil { + t.Fatalf("scan: %v", err) + } + if res.TargetSDK != 36 { + t.Errorf("TargetSDK = %d, want 36 from the application convention plugin", res.TargetSDK) + } + if f := findByPolicy(res.Findings, "Target API level"); f != nil { + t.Errorf("targetSdk 36 is compliant, got %q", f.Title) + } +} + +func TestTargetSDKFromVersionCatalog(t *testing.T) { + root := writeProject(t, map[string]string{ + "app/build.gradle.kts": `android { + defaultConfig { + targetSdk = libs.versions.targetSdk.get().toInt() + } +}`, + "gradle/libs.versions.toml": `[versions] +targetSdk = "34" +minSdk = "24" +`, + }) + res, err := Scan(root) + if err != nil { + t.Fatalf("scan: %v", err) + } + if res.TargetSDK != 34 { + t.Errorf("TargetSDK = %d, want 34 resolved from the version catalog", res.TargetSDK) + } +} + +// lint.targetSdk must never be mistaken for the app's target, since reading a +// low lint target would report a compliant app as non-compliant. +func TestLintTargetSDKIsIgnored(t *testing.T) { + root := writeProject(t, map[string]string{ + "app/build.gradle.kts": `android { + lint.targetSdk = 30 + testOptions.targetSdk = 30 + defaultConfig { + targetSdk = 36 + } +}`, + }) + res, err := Scan(root) + if err != nil { + t.Fatalf("scan: %v", err) + } + if res.TargetSDK != 36 { + t.Errorf("TargetSDK = %d, want 36; a lint/test target was read as the app's", res.TargetSDK) + } +} + +func TestCleanProjectProducesNoFindings(t *testing.T) { + root := writeProject(t, map[string]string{ + "app/build.gradle": `android { + defaultConfig { + minSdk = 24 + targetSdk = 36 + } +} +dependencies { + implementation 'androidx.core:core-ktx:1.13.0' +}`, + "app/src/main/AndroidManifest.xml": manifestHeader + ` + + + + + + +`, + }) + res, err := Scan(root) + if err != nil { + t.Fatalf("scan: %v", err) + } + if len(res.Findings) != 0 { + for _, f := range res.Findings { + t.Errorf("unexpected finding: [%s] %s", f.Severity, f.Title) + } + } + if res.PackageName != "com.example.app" { + t.Errorf("PackageName = %q", res.PackageName) + } +} + +func TestNonAndroidProjectScansClean(t *testing.T) { + root := writeProject(t, map[string]string{ + "App.swift": "import SwiftUI", + "app.json": `{"expo":{"name":"x"}}`, + }) + if Detect(root) { + t.Error("an iOS-only project must not be detected as Android") + } + res, err := Scan(root) + if err != nil { + t.Fatalf("scan: %v", err) + } + if len(res.Findings) != 0 || res.ManifestPath != "" { + t.Errorf("expected an empty result, got %+v", res) + } +} + +func TestDetectFindsExpoAndroidDirectory(t *testing.T) { + root := writeProject(t, map[string]string{ + "app.json": `{"expo":{"name":"x"}}`, + "android/app/build.gradle": "android { defaultConfig { targetSdk = 36 } }", + }) + if !Detect(root) { + t.Error("an Expo project with a prebuilt android/ directory is an Android project") + } +} + +// Build outputs contain generated manifests that must never be scanned in +// place of the developer's own. +func TestDiscoverSkipsBuildOutput(t *testing.T) { + root := writeProject(t, map[string]string{ + "app/src/main/AndroidManifest.xml": manifestHeader + ``, + "app/build/intermediates/merged_manifests/release/AndroidManifest.xml": manifestHeader + ` + + `, + }) + res, err := Scan(root) + if err != nil { + t.Fatalf("scan: %v", err) + } + if filepath.ToSlash(res.ManifestPath) != "app/src/main/AndroidManifest.xml" { + t.Errorf("ManifestPath = %q, want the source manifest", res.ManifestPath) + } + if findByPolicy(res.Findings, "SMS and Call Log permissions") != nil { + t.Error("a permission from a build-output manifest leaked into the scan") + } +} + +// In a multi-module repo the app module is often not named "app", so path +// shape alone picks the wrong manifest and silently hides the real app's +// permissions. The launcher activity is what identifies the shipped app. +func TestSelectsAppManifestInMultiModuleProject(t *testing.T) { + root := writeProject(t, map[string]string{ + "app/build.gradle": "android { defaultConfig { targetSdk = 36 } }", + // A benchmark module whose path scores well but ships nothing. + "baselineprofile/src/main/AndroidManifest.xml": manifestHeader + ` + +`, + // A library module, also no launcher. + "common/src/main/AndroidManifest.xml": manifestHeader + ` + +`, + // The real app, in a module named after the product. + "MyProduct/src/main/AndroidManifest.xml": manifestHeader + ` + + + + + + + + + +`, + }) + res, err := Scan(root) + if err != nil { + t.Fatalf("scan: %v", err) + } + if filepath.ToSlash(res.ManifestPath) != "MyProduct/src/main/AndroidManifest.xml" { + t.Fatalf("ManifestPath = %q, want the module with the launcher activity", res.ManifestPath) + } + if findByPolicy(res.Findings, "All files access") == nil { + t.Error("the real app's restricted permission was missed") + } +} + +func TestMalformedManifestIsReportedNotSwallowed(t *testing.T) { + root := writeProject(t, map[string]string{ + "app/src/main/AndroidManifest.xml": manifestHeader + ` + +`, + }) + res, err := Scan(root) + if err != nil { + t.Fatalf("scan should not hard-fail on a bad manifest: %v", err) + } + f := findByPolicy(res.Findings, "Scan coverage") + if f == nil { + t.Fatal("a malformed manifest must surface as a finding, not a silent pass") + } +} + +// itoa avoids pulling strconv into the test file for one call. +func itoa(n int) string { + if n == 0 { + return "0" + } + var b []byte + for n > 0 { + b = append([]byte{byte('0' + n%10)}, b...) + n /= 10 + } + return string(b) +} + +// play-services-auth-blockstore is credential backup and +// play-services-auth-api-phone is SMS retrieval; neither means the app creates +// accounts. Matching them raised a HIGH on an app with no accounts at all. +func TestAuthSDKDetectionExcludesNonAuthArtifacts(t *testing.T) { + notAuth := []string{ + `implementation "com.google.android.gms:play-services-auth-blockstore:_"`, + `implementation "com.google.android.gms:play-services-auth-api-phone:18.0.1"`, + } + for _, dep := range notAuth { + root := writeProject(t, map[string]string{ + "app/build.gradle": "android { defaultConfig { targetSdk = 36 } }\ndependencies {\n " + dep + "\n}", + }) + res, err := Scan(root) + if err != nil { + t.Fatalf("scan: %v", err) + } + if f := findByPolicy(res.Findings, "Account deletion"); f != nil { + t.Errorf("%s is not an auth SDK but raised the account deletion rule", dep) + } + } + + isAuth := []string{ + `implementation "com.google.android.gms:play-services-auth:21.6.0"`, + `implementation "com.google.firebase:firebase-auth-ktx:23.0.0"`, + } + for _, dep := range isAuth { + root := writeProject(t, map[string]string{ + "app/build.gradle": "android { defaultConfig { targetSdk = 36 } }\ndependencies {\n " + dep + "\n}", + }) + res, err := Scan(root) + if err != nil { + t.Fatalf("scan: %v", err) + } + if findByPolicy(res.Findings, "Account deletion") == nil { + t.Errorf("%s is an auth SDK and should raise the account deletion rule", dep) + } + } +} + +// Real projects reach targetSdk through a named constant far more often than +// they inline it. Each shape here came from a shipping open-source app. +func TestTargetSDKResolvedThroughReferences(t *testing.T) { + cases := []struct { + name string + files map[string]string + want int + }{ + { + name: "gradle ext variable (DuckDuckGo)", + files: map[string]string{ + "build.gradle": "ext {\n target_sdk = 35\n}", + "app/build.gradle": "android {\n defaultConfig {\n targetSdk target_sdk\n }\n}", + }, + want: 35, + }, + { + name: "kotlin object constant (Thunderbird)", + files: map[string]string{ + "build-plugin/src/main/kotlin/ProjectConfig.kt": "object ProjectConfig {\n object Android {\n const val sdkTarget = 35\n }\n}", + "build-plugin/src/main/kotlin/app.android.gradle.kts": "android {\n defaultConfig {\n targetSdk = ProjectConfig.Android.sdkTarget\n }\n}", + }, + want: 35, + }, + { + name: "extra property set() (Pocket Casts)", + files: map[string]string{ + "dependencies.gradle.kts": `extra.apply { + set("targetSdkVersion", 36) + set("targetSdkVersionWear", 34) +}`, + "app/build.gradle.kts": "android {\n defaultConfig {\n targetSdk = project.property(\"targetSdkVersion\") as Int\n }\n}", + }, + want: 36, + }, + { + name: "gradle.properties", + files: map[string]string{ + "gradle.properties": "android.useAndroidX=true\nTARGET_SDK=33\n", + "app/build.gradle": "android {\n defaultConfig {\n targetSdk TARGET_SDK\n }\n}", + }, + want: 33, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + root := writeProject(t, tc.files) + res, err := Scan(root) + if err != nil { + t.Fatalf("scan: %v", err) + } + if res.TargetSDK != tc.want { + t.Errorf("TargetSDK = %d, want %d", res.TargetSDK, tc.want) + } + }) + } +} diff --git a/internal/playscan/types.go b/internal/playscan/types.go new file mode 100644 index 0000000..ad05695 --- /dev/null +++ b/internal/playscan/types.go @@ -0,0 +1,51 @@ +// Package playscan checks an Android project against Google Play's Developer +// Program Policies and its published distribution deadlines. +// +// Everything here is static: it reads AndroidManifest.xml and the Gradle build +// files that ship in the repo. That is deliberate — the scan runs offline with +// no Play Console credentials — but it also bounds what the scan can see. The +// manifest in source is the *pre-merge* manifest, so permissions contributed by +// library manifests are invisible until the merge runs at build time. Findings +// here are therefore sound but not complete: what it reports is real, and a +// clean scan is not proof of a clean merged manifest. APK/AAB inspection reads +// the merged result and closes that gap. +package playscan + +// Finding is a single Play policy issue. Severity strings match the unified +// preflight vocabulary ("CRITICAL", "HIGH", "WARN", "INFO"). +type Finding struct { + Severity string `json:"severity"` + // Policy is the human-readable policy or requirement name. Play policies + // are named rather than numbered, so this is not an Apple-style section. + Policy string `json:"policy,omitempty"` + Title string `json:"title"` + Detail string `json:"detail"` + Fix string `json:"fix,omitempty"` + // Doc links the policy page the finding is based on. Play policy pages + // change far more often than Apple's guidelines, so every finding cites + // the source a developer can re-read. + Doc string `json:"doc,omitempty"` + File string `json:"file,omitempty"` + Line int `json:"line,omitempty"` +} + +// ScanResult holds the full Android scan output. +type ScanResult struct { + ProjectPath string `json:"project_path"` + // ManifestPath is the AndroidManifest.xml the scan read, relative to the + // project root. Empty when no manifest was found. + ManifestPath string `json:"manifest_path,omitempty"` + PackageName string `json:"package_name,omitempty"` + TargetSDK int `json:"target_sdk,omitempty"` + MinSDK int `json:"min_sdk,omitempty"` + // Permissions lists every permission declared in the source manifest. + Permissions []string `json:"permissions,omitempty"` + Findings []Finding `json:"findings"` +} + +const ( + sevCritical = "CRITICAL" + sevHigh = "HIGH" + sevWarn = "WARN" + sevInfo = "INFO" +) diff --git a/internal/preflight/platform.go b/internal/preflight/platform.go new file mode 100644 index 0000000..a3bc220 --- /dev/null +++ b/internal/preflight/platform.go @@ -0,0 +1,72 @@ +package preflight + +import ( + "io/fs" + "path/filepath" + "strings" + + "github.com/RevylAI/greenlight/internal/playscan" +) + +// iosSkipDirs mirrors the directories the other scanners ignore, so a vendored +// dependency's Xcode project cannot make a repo look like an iOS app. +var iosSkipDirs = map[string]bool{ + "node_modules": true, ".git": true, "build": true, ".gradle": true, + "dist": true, ".expo": true, "Pods": true, "vendor": true, + ".next": true, "DerivedData": true, ".idea": true, +} + +// DetectPlatforms reports which app platforms a project targets. +// +// This exists to keep store-specific findings off projects that do not ship to +// that store: an Android-only repo should never be told it is missing an Apple +// privacy manifest. Detection is deliberately generous on the iOS side, since +// the cost of a missed iOS signal (silently skipping the Apple checks) is far +// worse than the cost of running them on a repo that does not need them. +func DetectPlatforms(root string) (ios, android bool) { + android = playscan.Detect(root) + ios = detectIOS(root) + return ios, android +} + +func detectIOS(root string) bool { + found := false + _ = filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error { + if err != nil { + return nil + } + name := d.Name() + if d.IsDir() { + if iosSkipDirs[name] { + return filepath.SkipDir + } + // An .xcodeproj / .xcworkspace bundle is itself a directory. + if strings.HasSuffix(name, ".xcodeproj") || strings.HasSuffix(name, ".xcworkspace") { + found = true + return filepath.SkipAll + } + return nil + } + + switch { + case strings.EqualFold(name, "Info.plist"), + strings.EqualFold(name, "Podfile"), + strings.EqualFold(name, "Package.swift"), + strings.EqualFold(name, "project.pbxproj"), + strings.HasSuffix(name, ".xcprivacy"), + strings.HasSuffix(strings.ToLower(name), ".swift"), + strings.HasSuffix(strings.ToLower(name), ".xcodeproj"): + found = true + return filepath.SkipAll + } + + // An Expo / React Native config with an ios section ships to the App + // Store even when the repo holds no native iOS sources. + if strings.EqualFold(name, "app.json") || strings.EqualFold(name, "app.config.js") || strings.EqualFold(name, "app.config.ts") { + found = true + return filepath.SkipAll + } + return nil + }) + return found +} diff --git a/internal/preflight/runner.go b/internal/preflight/runner.go index 4348589..713ed17 100644 --- a/internal/preflight/runner.go +++ b/internal/preflight/runner.go @@ -7,20 +7,28 @@ import ( "github.com/RevylAI/greenlight/internal/codescan" "github.com/RevylAI/greenlight/internal/ipa" + "github.com/RevylAI/greenlight/internal/playscan" "github.com/RevylAI/greenlight/internal/privacy" ) // Finding is the unified finding type across all scanners. type Finding struct { - Source string `json:"source"` // "codescan", "privacy", "ipa", "metadata" - Severity string `json:"severity"` // "CRITICAL", "WARN", "INFO" + Source string `json:"source"` // "codescan", "privacy", "ipa", "metadata", "playscan" + Severity string `json:"severity"` // "CRITICAL", "HIGH", "WARN", "INFO" + // Guideline is an Apple review guideline section ("5.1.1") for iOS + // findings, or a named Google Play policy ("Target API level") for Android + // ones. Play policies are named rather than numbered. Guideline string `json:"guideline,omitempty"` Title string `json:"title"` Detail string `json:"detail"` Fix string `json:"fix,omitempty"` - File string `json:"file,omitempty"` - Line int `json:"line,omitempty"` - Code string `json:"code,omitempty"` + // Doc links the policy or guideline page the finding is based on. Play + // policy pages change often enough that citing the source is worth more + // than a section number alone. + Doc string `json:"doc,omitempty"` + File string `json:"file,omitempty"` + Line int `json:"line,omitempty"` + Code string `json:"code,omitempty"` } // Result holds the combined output from all scanners. @@ -41,6 +49,11 @@ type Result struct { HasPrivacyInfo bool `json:"has_privacy_info"` DetectedAPIs []string `json:"detected_apis,omitempty"` TrackingSDKs []string `json:"tracking_sdks,omitempty"` + + // Android context, populated when an Android project is detected. + IsAndroid bool `json:"is_android,omitempty"` + PackageName string `json:"package_name,omitempty"` + TargetSDK int `json:"target_sdk,omitempty"` } // Summary provides aggregate counts. @@ -71,81 +84,133 @@ func Run(projectPath string, ipaPath string, verbose bool) (*Result, error) { wg sync.WaitGroup ) - // Channel for collecting errors (non-fatal; we report what we can) - errs := make(chan error, 4) + // The Apple scanners run unless the project is unambiguously Android-only. + // Skipping them there is what stops an Android repo being told it is + // missing a PrivacyInfo.xcprivacy; anywhere else — including a project + // that is both, and a project that is neither — they run exactly as before. + isIOS, isAndroid := DetectPlatforms(projectPath) + runApple := isIOS || !isAndroid + result.IsAndroid = isAndroid + + // Channel for collecting errors (non-fatal; we report what we can). + // Buffered above the number of possible senders: the channel is drained + // only after wg.Wait(), so a full buffer would deadlock the scan. + errs := make(chan error, 8) // 1. Local metadata checks - wg.Add(1) - go func() { - defer wg.Done() - findings, meta := CheckLocalMetadata(projectPath) - mu.Lock() - result.Findings = append(result.Findings, findings...) - if meta.AppName != "" { - result.AppName = meta.AppName - } - if meta.BundleID != "" { - result.BundleID = meta.BundleID - } - mu.Unlock() - }() + if runApple { + wg.Add(1) + go func() { + defer wg.Done() + findings, meta := CheckLocalMetadata(projectPath) + mu.Lock() + result.Findings = append(result.Findings, findings...) + if meta.AppName != "" { + result.AppName = meta.AppName + } + if meta.BundleID != "" { + result.BundleID = meta.BundleID + } + mu.Unlock() + }() + + } // 2. Code scan - wg.Add(1) - go func() { - defer wg.Done() - scanner := codescan.NewScannerWithConfig(projectPath, verbose, cfg) - findings, err := scanner.Scan() - if err != nil { - errs <- err - return - } - mu.Lock() - for _, f := range findings { - result.Findings = append(result.Findings, Finding{ - Source: "codescan", - Severity: f.Severity.String(), - Guideline: f.Guideline, - Title: f.Title, - Detail: f.Detail, - Fix: f.Fix, - File: f.File, - Line: f.Line, - Code: f.Code, - }) - } - mu.Unlock() - }() + if runApple { + wg.Add(1) + go func() { + defer wg.Done() + scanner := codescan.NewScannerWithConfig(projectPath, verbose, cfg) + findings, err := scanner.Scan() + if err != nil { + errs <- err + return + } + mu.Lock() + for _, f := range findings { + result.Findings = append(result.Findings, Finding{ + Source: "codescan", + Severity: f.Severity.String(), + Guideline: f.Guideline, + Title: f.Title, + Detail: f.Detail, + Fix: f.Fix, + File: f.File, + Line: f.Line, + Code: f.Code, + }) + } + mu.Unlock() + }() + + } // 3. Privacy scan - wg.Add(1) - go func() { - defer wg.Done() - privResult, err := privacy.Scan(projectPath) - if err != nil { - errs <- err - return - } - mu.Lock() - result.HasPrivacyInfo = privResult.HasPrivacyInfo - result.DetectedAPIs = privResult.DetectedAPIs - result.TrackingSDKs = privResult.TrackingSDKs - for _, f := range privResult.Findings { - result.Findings = append(result.Findings, Finding{ - Source: "privacy", - Severity: f.Severity, - Guideline: f.Guideline, - Title: f.Title, - Detail: f.Detail, - Fix: f.Fix, - File: f.File, - Line: f.Line, - }) - } - mu.Unlock() - }() + if runApple { + wg.Add(1) + go func() { + defer wg.Done() + privResult, err := privacy.Scan(projectPath) + if err != nil { + errs <- err + return + } + mu.Lock() + result.HasPrivacyInfo = privResult.HasPrivacyInfo + result.DetectedAPIs = privResult.DetectedAPIs + result.TrackingSDKs = privResult.TrackingSDKs + for _, f := range privResult.Findings { + result.Findings = append(result.Findings, Finding{ + Source: "privacy", + Severity: f.Severity, + Guideline: f.Guideline, + Title: f.Title, + Detail: f.Detail, + Fix: f.Fix, + File: f.File, + Line: f.Line, + }) + } + mu.Unlock() + }() + + } + + // 4. Google Play policy scan (Android projects only). + // + // A cross-platform repo satisfies both this and runApple, so it is checked + // against both stores in a single pass. + if isAndroid { + wg.Add(1) + go func() { + defer wg.Done() + playResult, err := playscan.Scan(projectPath) + if err != nil { + errs <- err + return + } + mu.Lock() + result.PackageName = playResult.PackageName + result.TargetSDK = playResult.TargetSDK + for _, f := range playResult.Findings { + result.Findings = append(result.Findings, Finding{ + Source: "playscan", + Severity: f.Severity, + Guideline: f.Policy, + Title: f.Title, + Detail: f.Detail, + Fix: f.Fix, + Doc: f.Doc, + File: f.File, + Line: f.Line, + }) + } + mu.Unlock() + }() + } - // 4. IPA inspection (if path provided) + // 5. IPA inspection (if path provided) if ipaPath != "" { wg.Add(1) go func() { diff --git a/internal/preflight/runner_test.go b/internal/preflight/runner_test.go index 98bf5c2..26ca3cf 100644 --- a/internal/preflight/runner_test.go +++ b/internal/preflight/runner_test.go @@ -1,6 +1,10 @@ package preflight -import "testing" +import ( + "os" + "path/filepath" + "testing" +) // HIGH findings are counted separately, but Passed stays "no criticals" so a // HIGH-only result is still passed=true (the headline shows NEEDS REVIEW). @@ -40,3 +44,132 @@ func TestDedupKeepsHighestSeverity(t *testing.T) { } } } + +// An Android project must pick up Play findings from a plain preflight run, +// without the user naming a platform. +func TestRunDetectsAndroidProject(t *testing.T) { + root := t.TempDir() + mustWrite(t, root, "app/build.gradle", "android {\n defaultConfig {\n targetSdk = 30\n }\n}\n") + mustWrite(t, root, "app/src/main/AndroidManifest.xml", ` + + + +`) + + result, err := Run(root, "", false) + if err != nil { + t.Fatalf("Run: %v", err) + } + if !result.IsAndroid { + t.Fatal("Android project was not detected") + } + if result.PackageName != "com.example.app" { + t.Errorf("PackageName = %q", result.PackageName) + } + + var sawPlayFinding bool + for _, f := range result.Findings { + if f.Source == "playscan" { + sawPlayFinding = true + if f.Doc == "" { + t.Errorf("play finding %q lost its Doc link through the preflight mapping", f.Title) + } + } + } + if !sawPlayFinding { + t.Error("no playscan findings surfaced from preflight") + } +} + +// An iOS-only project must behave exactly as before: no Android state, no +// Play findings. +func TestRunLeavesIOSProjectsUnchanged(t *testing.T) { + root := t.TempDir() + mustWrite(t, root, "app.json", `{"expo":{"name":"Demo","description":"d","version":"1.0.0","icon":"./icon.png","ios":{"bundleIdentifier":"com.example.demo"}}}`) + mustWrite(t, root, "App.swift", "import SwiftUI\n") + + result, err := Run(root, "", false) + if err != nil { + t.Fatalf("Run: %v", err) + } + if result.IsAndroid { + t.Error("an iOS-only project was flagged as Android") + } + for _, f := range result.Findings { + if f.Source == "playscan" { + t.Errorf("unexpected Play finding on an iOS-only project: %s", f.Title) + } + } +} + +func mustWrite(t *testing.T, root, rel, content string) { + t.Helper() + full := filepath.Join(root, rel) + if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + if err := os.WriteFile(full, []byte(content), 0o644); err != nil { + t.Fatalf("write: %v", err) + } +} + +// An Android-only repo must not be told it is missing an Apple privacy +// manifest. This was a real false positive: every pure-Android project led +// with a CRITICAL for an iOS-only requirement. +func TestAndroidOnlyProjectGetsNoAppleFindings(t *testing.T) { + root := t.TempDir() + mustWrite(t, root, "app/build.gradle", "android {\n defaultConfig {\n targetSdk = 36\n }\n}\n") + mustWrite(t, root, "app/src/main/AndroidManifest.xml", ` + + +`) + mustWrite(t, root, "app/src/main/java/com/example/app/MainActivity.kt", "package com.example.app\n") + + result, err := Run(root, "", false) + if err != nil { + t.Fatalf("Run: %v", err) + } + for _, f := range result.Findings { + switch f.Source { + case "privacy", "metadata", "codescan": + t.Errorf("Apple scanner ran on an Android-only project: [%s] %s", f.Source, f.Title) + } + } +} + +// A cross-platform repo must be checked against both stores in one pass. +func TestCrossPlatformProjectRunsBothScanners(t *testing.T) { + root := t.TempDir() + mustWrite(t, root, "app.json", `{"expo":{"name":"Demo"}}`) + mustWrite(t, root, "ios/Demo/Info.plist", "") + mustWrite(t, root, "android/app/build.gradle", "android {\n defaultConfig {\n targetSdk = 30\n }\n}\n") + mustWrite(t, root, "android/app/src/main/AndroidManifest.xml", ` + + +`) + + ios, android := DetectPlatforms(root) + if !ios || !android { + t.Fatalf("DetectPlatforms = (ios=%v, android=%v), want both", ios, android) + } + + result, err := Run(root, "", false) + if err != nil { + t.Fatalf("Run: %v", err) + } + var sawPlay, sawApple bool + for _, f := range result.Findings { + switch f.Source { + case "playscan": + sawPlay = true + case "privacy", "metadata", "codescan": + sawApple = true + } + } + if !sawPlay { + t.Error("no Play findings on a cross-platform project") + } + if !sawApple { + t.Error("no Apple findings on a cross-platform project") + } +} From 8fc835bb8a42e53055faf64e7f8ebe6a82895d35 Mon Sep 17 00:00:00 2001 From: ethan zhou <231755529+ethanzhoucool@users.noreply.github.com> Date: Mon, 27 Jul 2026 14:58:25 -0700 Subject: [PATCH 2/3] feat(playscan): scan built APKs and AABs, with 16 KB page size checks Adds `greenlight playscan --apk` / `--aab`, closing the gap the source scan documents: a repo's AndroidManifest.xml is the pre-merge manifest, so permissions contributed by library manifests are invisible until build time. A built artifact carries the merged manifest, so its permission list is complete. Every existing policy rule runs unchanged against the decoded manifest. The archive scan adds the native code checks, which only exist at this layer: - 16 KB page size: ELF LOAD segment alignment on arm64-v8a, 16 KB zip alignment of uncompressed libraries, and GNU_RELRO presence. Google Play requires apps targeting Android 15+ to support 16 KB memory pages; an unaligned library fails to load and crashes at startup on those devices. Both container formats are decoded in pure Go with no new dependencies, so no Android SDK, aapt2, or bundletool is required: - APK: Android binary XML, including the string pool in both UTF-8 and UTF-16 form and the resource ID map. aapt2 emits an empty string-pool entry for framework attributes and identifies them only by resource ID, so without the map every android:* attribute reads as unnamed. - AAB: the protobuf-encoded manifest, walked directly off the wire format. foregroundServiceType is a bitmask once compiled; it is rendered back to the pipe-separated names the source manifest uses so the foreground service rule needs no second code path. Only arm64-v8a is assessed for 16 KB alignment. 16 KB page size devices are 64-bit ARM, so flagging a 32-bit library at 4 KB would be noise. Verified against real APKs. Termux 0.118.3 decodes to exactly the 15 permissions in its published source manifest, and its 4 KB aligned libraries are flagged; F-Droid's 16 KB aligned library is not. Alignment findings were cross-checked by parsing the ELF program headers independently. --- README.md | 33 +- internal/cli/playscan.go | 63 +++- internal/playscan/archive.go | 337 +++++++++++++++++ internal/playscan/archive_test.go | 600 ++++++++++++++++++++++++++++++ internal/playscan/axml.go | 446 ++++++++++++++++++++++ internal/playscan/protoxml.go | 245 ++++++++++++ internal/playscan/rules.go | 1 + internal/playscan/types.go | 9 +- 8 files changed, 1714 insertions(+), 20 deletions(-) create mode 100644 internal/playscan/archive.go create mode 100644 internal/playscan/archive_test.go create mode 100644 internal/playscan/axml.go create mode 100644 internal/playscan/protoxml.go diff --git a/README.md b/README.md index 7e89c5e..7de67d9 100644 --- a/README.md +++ b/README.md @@ -85,8 +85,10 @@ Scans Swift, Objective-C, React Native, and Expo projects for: ```bash greenlight playscan /path/to/project +greenlight playscan --apk app-release.apk # built artifact: merged manifest +greenlight playscan --aab app-release.aab # app bundle greenlight playscan . --format json -greenlight playscan . --exit-code # CI gating +greenlight playscan . --exit-code # CI gating ``` Checks an Android app against Google Play's Developer Program Policies and its @@ -126,12 +128,29 @@ stores in one pass. Every finding cites the policy page it comes from. -**Scope.** This reads the `AndroidManifest.xml` and Gradle files in your repo, -which is the *pre-merge* manifest. Permissions contributed by library manifests -only appear once the build merges them, so a clean scan is not proof of a clean -merged manifest. `targetSdk` is resolved from the app module, convention -plugins under `build-logic/`/`buildSrc/`, and version catalogs; when it cannot -be resolved the scan says so rather than reporting a pass. +**Scanning a built artifact** (`--apk` / `--aab`) + +Passing a build reads the *merged* manifest, so it sees permissions contributed +by library manifests that a source scan structurally cannot. Every policy check +above runs against it, plus the native code checks: + +- **16 KB page size** — Google Play requires apps targeting Android 15+ to + support 16 KB memory pages. Checks ELF `LOAD` segment alignment on + `arm64-v8a` libraries, 16 KB zip alignment of uncompressed libraries, and + `GNU_RELRO` presence — **CRITICAL / HIGH** + +Both formats are read directly: an APK's compiled binary XML and an AAB's +protobuf manifest are decoded in pure Go, so no Android SDK, `aapt2`, or +`bundletool` is needed. + +**Scope.** Scanning *source* reads the `AndroidManifest.xml` and Gradle files in +your repo, which is the *pre-merge* manifest. Permissions contributed by library +manifests only appear once the build merges them, so a clean source scan is not +proof of a clean merged manifest — scan the built artifact to close that gap. +`targetSdk` is resolved from the app module, convention plugins +(`build-logic/`, `buildSrc/`, `build-plugin/`), version catalogs, +`gradle.properties`, and named constants; when it cannot be resolved the scan +says so rather than reporting a pass. ### `greenlight privacy [path]` — Privacy manifest validator diff --git a/internal/cli/playscan.go b/internal/cli/playscan.go index 2ae866e..2d4f768 100644 --- a/internal/cli/playscan.go +++ b/internal/cli/playscan.go @@ -17,6 +17,7 @@ var ( playscanFormat string playscanOutput string playscanExitCode bool + playscanArchive string ) var playscanCmd = &cobra.Command{ @@ -36,15 +37,23 @@ Checks: • Advertising ID — ads SDK shipped without the AD_ID permission • Account deletion — the in-app and web deletion requirement +Pass --apk or --aab to scan a BUILT artifact instead of source. That reads the +merged manifest, so it sees permissions contributed by library manifests that a +source scan structurally cannot, and adds the native code checks: + + • 16 KB page size — ELF LOAD alignment, zip alignment, GNU_RELRO + Runs entirely offline. No Play Console account needed. -Scope: this reads the AndroidManifest.xml and Gradle files in your repo, which -is the pre-merge manifest. Permissions contributed by library manifests only -appear after the build merges them, so a clean scan is not proof of a clean -merged manifest. +Scope: scanning source reads the AndroidManifest.xml and Gradle files in your +repo, which is the pre-merge manifest. Permissions contributed by library +manifests only appear after the build merges them, so a clean source scan is +not proof of a clean merged manifest. Scan the built artifact to close that gap. Usage: greenlight playscan . + greenlight playscan --apk app-release.apk + greenlight playscan --aab app-release.aab greenlight playscan ./android --format json greenlight playscan . --exit-code`, Args: cobra.MaximumNArgs(1), @@ -55,6 +64,8 @@ func init() { playscanCmd.Flags().StringVar(&playscanFormat, "format", "terminal", "output format: terminal, json") playscanCmd.Flags().StringVar(&playscanOutput, "output", "", "write report to file (stdout if omitted)") playscanCmd.Flags().BoolVar(&playscanExitCode, "exit-code", false, "exit non-zero on any CRITICAL or HIGH finding — for CI gating") + playscanCmd.Flags().StringVar(&playscanArchive, "apk", "", "scan a built .apk (merged manifest + native code checks) instead of source") + playscanCmd.Flags().StringVar(&playscanArchive, "aab", "", "scan a built .aab (merged manifest + native code checks) instead of source") rootCmd.AddCommand(playscanCmd) } @@ -64,22 +75,44 @@ func runPlayscan(cmd *cobra.Command, args []string) error { path = args[0] } - info, err := os.Stat(path) - if err != nil { - return fmt.Errorf("cannot access path: %w", err) - } - if !info.IsDir() { - return fmt.Errorf("path must be a directory: %s", path) + // --apk / --aab share one variable: they select the same archive scan and + // the format is detected from the file, so passing the wrong one is not an + // error the user needs to care about. + archive := playscanArchive + if archive != "" { + if info, err := os.Stat(archive); err != nil { + return fmt.Errorf("cannot access archive: %w", err) + } else if info.IsDir() { + return fmt.Errorf("--apk/--aab expects a file, not a directory: %s", archive) + } + } else { + info, err := os.Stat(path) + if err != nil { + return fmt.Errorf("cannot access path: %w", err) + } + if !info.IsDir() { + return fmt.Errorf("path must be a directory: %s", path) + } } isJSON := strings.ToLower(playscanFormat) == "json" if !isJSON { purple.Println("\n greenlight playscan — know before you upload to Google Play.") - fmt.Printf(" Project: %s\n\n", path) + if archive != "" { + fmt.Printf(" Archive: %s\n\n", archive) + } else { + fmt.Printf(" Project: %s\n\n", path) + } } start := time.Now() - result, err := playscan.Scan(path) + var result *playscan.ScanResult + var err error + if archive != "" { + result, err = playscan.ScanArchive(archive) + } else { + result, err = playscan.Scan(path) + } if err != nil { return fmt.Errorf("play scan failed: %w", err) } @@ -130,6 +163,9 @@ func printPlayscanReport(w *os.File, result *playscan.ScanResult, elapsed time.D } // Project context + if result.ArchiveKind != "" { + fmt.Fprintf(w, " Format: %s (merged manifest)\n", result.ArchiveKind) + } if result.PackageName != "" { fmt.Fprintf(w, " Package: %s\n", result.PackageName) } @@ -139,6 +175,9 @@ func printPlayscanReport(w *os.File, result *playscan.ScanResult, elapsed time.D if result.ManifestPath != "" { fmt.Fprintf(w, " Manifest: %s\n", result.ManifestPath) } + if result.IsArchive { + fmt.Fprintf(w, " Native libs: %d\n", result.NativeLibCount) + } fmt.Fprintln(w) // Sort by severity, then policy, so the output is stable run to run. diff --git a/internal/playscan/archive.go b/internal/playscan/archive.go new file mode 100644 index 0000000..65bde7f --- /dev/null +++ b/internal/playscan/archive.go @@ -0,0 +1,337 @@ +package playscan + +import ( + "archive/zip" + "debug/elf" + "fmt" + "io" + "path" + "sort" + "strings" +) + +// pageSize16KB is the alignment Google Play requires of native code so it can +// load on 16 KB page size devices. +const pageSize16KB = 16384 + +// ArchiveKind distinguishes the two upload formats. +type ArchiveKind string + +const ( + KindAPK ArchiveKind = "APK" + KindAAB ArchiveKind = "AAB" +) + +// nativeLib is one shared library found in the archive. +type nativeLib struct { + Path string + ABI string + // SegmentAlign is the smallest alignment across the ELF LOAD segments. + SegmentAlign uint64 + HasRelro bool + // ZipOffset is where the entry's data begins in the archive, which must be + // 16 KB aligned for the loader to map the library directly. + ZipOffset int64 + Stored bool + ParseErr error +} + +// ScanArchive checks a built APK or AAB. +// +// The manifest inside a build is the merged one, so unlike a source scan this +// sees permissions contributed by library manifests. Every policy rule runs +// against it unchanged, plus the native code checks that only exist here. +func ScanArchive(archivePath string) (*ScanResult, error) { + zr, err := zip.OpenReader(archivePath) + if err != nil { + return nil, fmt.Errorf("cannot open archive: %w", err) + } + defer zr.Close() + + kind, manifestEntry := classifyArchive(&zr.Reader) + if kind == "" { + return nil, fmt.Errorf("not an APK or AAB: no AndroidManifest.xml found in %s", archivePath) + } + + result := &ScanResult{ + ProjectPath: archivePath, + ArchiveKind: string(kind), + ManifestPath: manifestEntry, + IsArchive: true, + } + + var manifest *Manifest + if manifestEntry != "" { + data, err := readZipEntry(&zr.Reader, manifestEntry) + if err != nil { + return nil, fmt.Errorf("read manifest: %w", err) + } + switch kind { + case KindAPK: + manifest, err = DecodeBinaryXML(data) + case KindAAB: + manifest, err = DecodeProtoXML(data) + } + if err != nil { + result.Findings = append(result.Findings, Finding{ + Severity: sevWarn, + Policy: "Scan coverage", + Title: "Could not decode the bundled AndroidManifest.xml", + Detail: "The archive's manifest could not be decoded, so manifest-based policy checks were skipped: " + err.Error() + + ". Native code checks below are unaffected.", + Fix: "Run the scan against the project source as well, which reads the manifest before it is compiled.", + File: manifestEntry, + }) + } + } + + if manifest != nil { + result.PackageName = manifest.Package + result.Permissions = manifest.AllPermissions() + if manifest.UsesSDK != nil { + result.TargetSDK = parseSDKInt(manifest.UsesSDK.TargetSDKVersion) + result.MinSDK = parseSDKInt(manifest.UsesSDK.MinSDKVersion) + } + + ctx := &ruleContext{ + manifest: manifest, + // A built archive carries no Gradle state; rules that read it are + // source-only and correctly stay silent. + gradle: &GradleInfo{}, + targetSDK: result.TargetSDK, + manifestFile: manifestEntry, + } + for _, rule := range allRules() { + result.Findings = append(result.Findings, rule(ctx)...) + } + } + + libs := collectNativeLibs(&zr.Reader, kind) + result.NativeLibCount = len(libs) + result.Findings = append(result.Findings, checkPageAlignment(libs, kind)...) + + return result, nil +} + +// classifyArchive determines the format and locates the manifest entry. +func classifyArchive(zr *zip.Reader) (ArchiveKind, string) { + var hasBundleLayout bool + for _, f := range zr.File { + switch f.Name { + case "AndroidManifest.xml": + return KindAPK, f.Name + case "base/manifest/AndroidManifest.xml": + hasBundleLayout = true + } + } + if hasBundleLayout { + return KindAAB, "base/manifest/AndroidManifest.xml" + } + return "", "" +} + +func readZipEntry(zr *zip.Reader, name string) ([]byte, error) { + for _, f := range zr.File { + if f.Name != name { + continue + } + rc, err := f.Open() + if err != nil { + return nil, err + } + defer rc.Close() + return io.ReadAll(io.LimitReader(rc, 32<<20)) + } + return nil, fmt.Errorf("entry not found: %s", name) +} + +// collectNativeLibs finds every packaged .so and reads what the alignment +// checks need. Both archive layouts are handled. +func collectNativeLibs(zr *zip.Reader, kind ArchiveKind) []nativeLib { + prefix := "lib/" + if kind == KindAAB { + prefix = "base/lib/" + } + + var libs []nativeLib + for _, f := range zr.File { + if !strings.HasPrefix(f.Name, prefix) || !strings.HasSuffix(f.Name, ".so") { + continue + } + lib := nativeLib{ + Path: f.Name, + ABI: abiFromPath(f.Name, prefix), + Stored: f.Method == zip.Store, + } + if off, err := f.DataOffset(); err == nil { + lib.ZipOffset = off + } + + rc, err := f.Open() + if err != nil { + lib.ParseErr = err + libs = append(libs, lib) + continue + } + data, err := io.ReadAll(io.LimitReader(rc, 256<<20)) + rc.Close() + if err != nil { + lib.ParseErr = err + libs = append(libs, lib) + continue + } + readELFAlignment(&lib, data) + libs = append(libs, lib) + } + + sort.Slice(libs, func(i, j int) bool { return libs[i].Path < libs[j].Path }) + return libs +} + +func abiFromPath(name, prefix string) string { + rest := strings.TrimPrefix(name, prefix) + if i := strings.Index(rest, "/"); i > 0 { + return rest[:i] + } + return path.Dir(rest) +} + +// readELFAlignment records the minimum LOAD segment alignment and whether the +// library has GNU_RELRO, both of which the 16 KB requirement depends on. +func readELFAlignment(lib *nativeLib, data []byte) { + f, err := elf.NewFile(newByteReaderAt(data)) + if err != nil { + lib.ParseErr = err + return + } + defer f.Close() + + var minAlign uint64 + for _, p := range f.Progs { + switch p.Type { + case elf.PT_LOAD: + if minAlign == 0 || p.Align < minAlign { + minAlign = p.Align + } + case elf.PT_GNU_RELRO: + lib.HasRelro = true + } + } + lib.SegmentAlign = minAlign +} + +// checkPageAlignment produces the 16 KB page size findings. +// +// Only arm64-v8a is assessed for the requirement itself: 16 KB page size +// devices are 64-bit ARM, so a 32-bit library being 4 KB aligned is not a +// violation and reporting it would be noise. +func checkPageAlignment(libs []nativeLib, kind ArchiveKind) []Finding { + if len(libs) == 0 { + return nil + } + + var unaligned, missingRelro, badZipOffset []string + for _, lib := range libs { + if lib.ABI != "arm64-v8a" { + continue + } + if lib.ParseErr != nil { + continue + } + if lib.SegmentAlign > 0 && lib.SegmentAlign < pageSize16KB { + unaligned = append(unaligned, fmt.Sprintf("%s (align %s)", path.Base(lib.Path), formatAlign(lib.SegmentAlign))) + } + if !lib.HasRelro { + missingRelro = append(missingRelro, path.Base(lib.Path)) + } + // Zip alignment only applies to an APK, where the loader maps the + // library straight out of the archive. An AAB is repackaged by Play, + // so its entry offsets say nothing about the delivered APK. + if kind == KindAPK && lib.Stored && lib.ZipOffset%pageSize16KB != 0 { + badZipOffset = append(badZipOffset, path.Base(lib.Path)) + } + } + + var findings []Finding + + if len(unaligned) > 0 { + findings = append(findings, Finding{ + Severity: sevCritical, + Policy: "16 KB page size", + Title: "Native libraries are not 16 KB aligned", + Detail: fmt.Sprintf( + "Google Play requires apps targeting Android 15 and above to support 16 KB page sizes, and these arm64-v8a libraries have LOAD segments aligned below 16384 bytes: %s. "+ + "On a 16 KB page size device the app fails to load them and crashes at startup.", + strings.Join(truncateList(unaligned, 6), ", ")), + Fix: "Rebuild with AGP 8.5.1+ and NDK r28+, or add -Wl,-z,max-page-size=16384 to the linker flags. Prebuilt dependencies must be updated to a 16 KB compatible release.", + Doc: docPageSizes, + }) + } + + if len(badZipOffset) > 0 { + findings = append(findings, Finding{ + Severity: sevHigh, + Policy: "16 KB page size", + Title: "Uncompressed native libraries are not 16 KB zip-aligned", + Detail: fmt.Sprintf( + "These libraries are stored uncompressed but do not begin on a 16384-byte boundary in the archive: %s. "+ + "The loader maps them straight out of the APK, so a misaligned offset defeats 16 KB support even when the ELF segments themselves are aligned.", + strings.Join(truncateList(badZipOffset, 6), ", ")), + Fix: "Repackage with zipalign -P 16 4, or let AGP 8.5.1+ handle it.", + Doc: docPageSizes, + }) + } + + if len(missingRelro) > 0 { + findings = append(findings, Finding{ + Severity: sevWarn, + Policy: "16 KB page size", + Title: "Native libraries are missing GNU_RELRO", + Detail: fmt.Sprintf( + "These libraries have no GNU_RELRO segment: %s. Combining a RELRO-enabled section with a non-RELRO one on the same page crashes the app on 16 KB devices.", + strings.Join(truncateList(missingRelro, 6), ", ")), + Fix: "Rebuild with a current NDK, which emits GNU_RELRO by default.", + Doc: docPageSizes, + }) + } + + return findings +} + +func formatAlign(align uint64) string { + switch align { + case 4096: + return "4 KB" + case 8192: + return "8 KB" + case 16384: + return "16 KB" + case 65536: + return "64 KB" + } + return fmt.Sprintf("%d bytes", align) +} + +func truncateList(items []string, max int) []string { + if len(items) <= max { + return items + } + out := append([]string{}, items[:max]...) + return append(out, fmt.Sprintf("and %d more", len(items)-max)) +} + +// byteReaderAt adapts a byte slice to io.ReaderAt for debug/elf. +type byteReaderAt struct{ b []byte } + +func newByteReaderAt(b []byte) *byteReaderAt { return &byteReaderAt{b: b} } + +func (r *byteReaderAt) ReadAt(p []byte, off int64) (int, error) { + if off < 0 || off >= int64(len(r.b)) { + return 0, io.EOF + } + n := copy(p, r.b[off:]) + if n < len(p) { + return n, io.EOF + } + return n, nil +} diff --git a/internal/playscan/archive_test.go b/internal/playscan/archive_test.go new file mode 100644 index 0000000..cdf092d --- /dev/null +++ b/internal/playscan/archive_test.go @@ -0,0 +1,600 @@ +package playscan + +import ( + "archive/zip" + "bytes" + "encoding/binary" + "os" + "path/filepath" + "strings" + "testing" +) + +// --- AXML construction ------------------------------------------------- +// +// Building compiled manifests by hand keeps the decoder's tests hermetic; the +// alternative is committing a real APK. The byte layouts here mirror +// ResourceTypes.h exactly, so a regression in the decoder shows up as a +// decode failure rather than a fixture mismatch. + +type axmlBuilder struct { + strings []string + resIDs []uint32 + body bytes.Buffer +} + +func (b *axmlBuilder) str(s string) uint32 { + for i, existing := range b.strings { + if existing == s { + return uint32(i) + } + } + b.strings = append(b.strings, s) + b.resIDs = append(b.resIDs, 0) + return uint32(len(b.strings) - 1) +} + +// attr registers an attribute name that carries a framework resource ID and an +// empty pool entry, which is what aapt2 actually emits. +func (b *axmlBuilder) attrRef(resID uint32) uint32 { + b.strings = append(b.strings, "") + b.resIDs = append(b.resIDs, resID) + return uint32(len(b.strings) - 1) +} + +type buildAttr struct { + nameIdx uint32 + dataType byte + data uint32 + rawIdx uint32 +} + +func (b *axmlBuilder) startTag(name string, attrs []buildAttr) { + nameIdx := b.str(name) + var buf bytes.Buffer + // ResXMLTree_node: lineNumber, comment + binary.Write(&buf, binary.LittleEndian, uint32(1)) + binary.Write(&buf, binary.LittleEndian, uint32(0xFFFFFFFF)) + // ResXMLTree_attrExt + binary.Write(&buf, binary.LittleEndian, uint32(0xFFFFFFFF)) // ns + binary.Write(&buf, binary.LittleEndian, nameIdx) + binary.Write(&buf, binary.LittleEndian, uint16(20)) // attributeStart, from attrExt + binary.Write(&buf, binary.LittleEndian, uint16(20)) // attributeSize + binary.Write(&buf, binary.LittleEndian, uint16(len(attrs))) + binary.Write(&buf, binary.LittleEndian, uint16(0)) // idIndex + binary.Write(&buf, binary.LittleEndian, uint16(0)) // classIndex + binary.Write(&buf, binary.LittleEndian, uint16(0)) // styleIndex + for _, a := range attrs { + binary.Write(&buf, binary.LittleEndian, uint32(0xFFFFFFFF)) // ns + binary.Write(&buf, binary.LittleEndian, a.nameIdx) + binary.Write(&buf, binary.LittleEndian, a.rawIdx) + binary.Write(&buf, binary.LittleEndian, uint16(8)) // typed value size + buf.WriteByte(0) // res0 + buf.WriteByte(a.dataType) + binary.Write(&buf, binary.LittleEndian, a.data) + } + b.writeChunk(chunkXMLStartTag, buf.Bytes()) +} + +func (b *axmlBuilder) endTag(name string) { + nameIdx := b.str(name) + var buf bytes.Buffer + binary.Write(&buf, binary.LittleEndian, uint32(1)) + binary.Write(&buf, binary.LittleEndian, uint32(0xFFFFFFFF)) + binary.Write(&buf, binary.LittleEndian, uint32(0xFFFFFFFF)) + binary.Write(&buf, binary.LittleEndian, nameIdx) + b.writeChunk(chunkXMLEndTag, buf.Bytes()) +} + +func (b *axmlBuilder) writeChunk(chunkType uint16, payload []byte) { + binary.Write(&b.body, binary.LittleEndian, chunkType) + binary.Write(&b.body, binary.LittleEndian, uint16(8)) + binary.Write(&b.body, binary.LittleEndian, uint32(8+len(payload))) + b.body.Write(payload) +} + +// build emits the full document: header, string pool, resource map, then tags. +func (b *axmlBuilder) build() []byte { + var pool bytes.Buffer + offsets := make([]uint32, len(b.strings)) + var data bytes.Buffer + for i, s := range b.strings { + offsets[i] = uint32(data.Len()) + // UTF-16 form: u16 length, UTF-16LE units, u16 terminator. + binary.Write(&data, binary.LittleEndian, uint16(len(s))) + for _, r := range s { + binary.Write(&data, binary.LittleEndian, uint16(r)) + } + binary.Write(&data, binary.LittleEndian, uint16(0)) + } + headerLen := 28 + len(offsets)*4 + binary.Write(&pool, binary.LittleEndian, uint32(len(b.strings))) // stringCount + binary.Write(&pool, binary.LittleEndian, uint32(0)) // styleCount + binary.Write(&pool, binary.LittleEndian, uint32(0)) // flags: UTF-16 + binary.Write(&pool, binary.LittleEndian, uint32(headerLen)) // stringsStart + binary.Write(&pool, binary.LittleEndian, uint32(0)) // stylesStart + for _, o := range offsets { + binary.Write(&pool, binary.LittleEndian, o) + } + pool.Write(data.Bytes()) + + var resMap bytes.Buffer + for _, id := range b.resIDs { + binary.Write(&resMap, binary.LittleEndian, id) + } + + var out bytes.Buffer + var chunks bytes.Buffer + writeChunkTo(&chunks, chunkStringPool, pool.Bytes()) + writeChunkTo(&chunks, chunkXMLResource, resMap.Bytes()) + chunks.Write(b.body.Bytes()) + + binary.Write(&out, binary.LittleEndian, uint16(chunkXMLDocument)) + binary.Write(&out, binary.LittleEndian, uint16(8)) + binary.Write(&out, binary.LittleEndian, uint32(8+chunks.Len())) + out.Write(chunks.Bytes()) + return out.Bytes() +} + +func writeChunkTo(w *bytes.Buffer, chunkType uint16, payload []byte) { + binary.Write(w, binary.LittleEndian, chunkType) + binary.Write(w, binary.LittleEndian, uint16(8)) + binary.Write(w, binary.LittleEndian, uint32(8+len(payload))) + w.Write(payload) +} + +// buildTestManifest produces a compiled manifest exercising the attribute +// forms that matter: a string permission name, a boolean, an integer SDK +// level, and a foregroundServiceType bitmask. +func buildTestManifest() []byte { + b := &axmlBuilder{} + nameAttr := b.attrRef(0x01010003) + debuggableAttr := b.attrRef(0x0101000f) + targetSdkAttr := b.attrRef(0x01010270) + exportedAttr := b.attrRef(0x01010010) + fgsAttr := b.attrRef(0x01010596) + + pkgIdx := b.str("com.example.built") + permIdx := b.str("android.permission.READ_SMS") + perm2Idx := b.str("android.permission.FOREGROUND_SERVICE") + svcIdx := b.str("com.example.SyncService") + actIdx := b.str("com.example.MainActivity") + mainIdx := b.str("android.intent.action.MAIN") + launcherIdx := b.str("android.intent.category.LAUNCHER") + packageAttr := b.str("package") + + b.startTag("manifest", []buildAttr{ + {nameIdx: packageAttr, dataType: typeString, data: pkgIdx, rawIdx: pkgIdx}, + }) + b.startTag("uses-permission", []buildAttr{ + {nameIdx: nameAttr, dataType: typeString, data: permIdx, rawIdx: permIdx}, + }) + b.endTag("uses-permission") + b.startTag("uses-permission", []buildAttr{ + {nameIdx: nameAttr, dataType: typeString, data: perm2Idx, rawIdx: perm2Idx}, + }) + b.endTag("uses-permission") + b.startTag("uses-sdk", []buildAttr{ + {nameIdx: targetSdkAttr, dataType: typeIntDec, data: 34, rawIdx: 0xFFFFFFFF}, + }) + b.endTag("uses-sdk") + b.startTag("application", []buildAttr{ + {nameIdx: debuggableAttr, dataType: typeIntBoolean, data: 0xFFFFFFFF, rawIdx: 0xFFFFFFFF}, + }) + b.startTag("activity", []buildAttr{ + {nameIdx: nameAttr, dataType: typeString, data: actIdx, rawIdx: actIdx}, + {nameIdx: exportedAttr, dataType: typeIntBoolean, data: 0xFFFFFFFF, rawIdx: 0xFFFFFFFF}, + }) + b.startTag("intent-filter", nil) + b.startTag("action", []buildAttr{ + {nameIdx: nameAttr, dataType: typeString, data: mainIdx, rawIdx: mainIdx}, + }) + b.endTag("action") + b.startTag("category", []buildAttr{ + {nameIdx: nameAttr, dataType: typeString, data: launcherIdx, rawIdx: launcherIdx}, + }) + b.endTag("category") + b.endTag("intent-filter") + b.endTag("activity") + // foregroundServiceType="location" is bit 3 in compiled form. + b.startTag("service", []buildAttr{ + {nameIdx: nameAttr, dataType: typeString, data: svcIdx, rawIdx: svcIdx}, + {nameIdx: fgsAttr, dataType: typeIntHex, data: 1 << 3, rawIdx: 0xFFFFFFFF}, + }) + b.endTag("service") + b.endTag("application") + b.endTag("manifest") + return b.build() +} + +func TestDecodeBinaryXML(t *testing.T) { + m, err := DecodeBinaryXML(buildTestManifest()) + if err != nil { + t.Fatalf("decode: %v", err) + } + + if m.Package != "com.example.built" { + t.Errorf("Package = %q", m.Package) + } + if !m.HasPermission("android.permission.READ_SMS") { + t.Error("READ_SMS not decoded") + } + if m.UsesSDK == nil || m.UsesSDK.TargetSDKVersion != "34" { + t.Errorf("targetSdkVersion not decoded: %+v", m.UsesSDK) + } + if m.Application == nil || !attrIsTrue(m.Application.Debuggable) { + t.Error("debuggable boolean not decoded as true") + } + if len(m.Application.Activities) != 1 || m.Application.Activities[0].Name != "com.example.MainActivity" { + t.Fatalf("activity not decoded: %+v", m.Application.Activities) + } + if !attrIsTrue(m.Application.Activities[0].Exported) { + t.Error("exported boolean not decoded") + } + if !m.HasLauncherActivity() { + t.Error("launcher intent filter not decoded") + } + // The compiled bitmask must render back to the source attribute name so + // the foreground service rule needs no second code path. + if len(m.Application.Services) != 1 || m.Application.Services[0].ForegroundSvcType != "location" { + t.Errorf("foregroundServiceType bitmask not decoded: %+v", m.Application.Services) + } +} + +func TestForegroundServiceTypeNames(t *testing.T) { + cases := []struct { + mask uint32 + want string + }{ + {1 << 0, "dataSync"}, + {1 << 3, "location"}, + {1<<3 | 1<<6, "location|camera"}, + {1 << 30, "specialUse"}, + {0, ""}, + } + for _, tc := range cases { + if got := foregroundServiceTypeNames(tc.mask); got != tc.want { + t.Errorf("mask %#x = %q, want %q", tc.mask, got, tc.want) + } + } +} + +func TestDecodeBinaryXMLRejectsNonAXML(t *testing.T) { + if _, err := DecodeBinaryXML([]byte("")); err == nil { + t.Error("text XML should be rejected as not binary XML") + } + if _, err := DecodeBinaryXML([]byte{1, 2}); err == nil { + t.Error("truncated input should error") + } +} + +// --- ELF + archive ----------------------------------------------------- + +// buildTestELF produces a minimal but valid 64-bit ARM shared object with one +// PT_LOAD segment at the requested alignment, optionally with GNU_RELRO. +func buildTestELF(align uint64, relro bool) []byte { + const phoff = 64 + phnum := 1 + if relro { + phnum = 2 + } + phentsize := 56 + buf := make([]byte, phoff+phnum*phentsize) + + copy(buf[0:], []byte{0x7f, 'E', 'L', 'F'}) + buf[4] = 2 // 64-bit + buf[5] = 1 // little endian + buf[6] = 1 // version + binary.LittleEndian.PutUint16(buf[16:], 3) // ET_DYN + binary.LittleEndian.PutUint16(buf[18:], 0xB7) // EM_AARCH64 + binary.LittleEndian.PutUint32(buf[20:], 1) + binary.LittleEndian.PutUint64(buf[32:], phoff) + binary.LittleEndian.PutUint16(buf[52:], 64) + binary.LittleEndian.PutUint16(buf[54:], uint16(phentsize)) + binary.LittleEndian.PutUint16(buf[56:], uint16(phnum)) + + writeProg := func(idx int, ptype uint32, palign uint64) { + off := phoff + idx*phentsize + binary.LittleEndian.PutUint32(buf[off:], ptype) + binary.LittleEndian.PutUint32(buf[off+4:], 4) // flags + binary.LittleEndian.PutUint64(buf[off+48:], palign) + } + writeProg(0, 1, align) // PT_LOAD + if relro { + writeProg(1, 0x6474e552, 1) // PT_GNU_RELRO + } + return buf +} + +// buildTestAPK writes an APK containing a compiled manifest and the given +// native libraries. +func buildTestAPK(t *testing.T, libs map[string][]byte) string { + t.Helper() + path := filepath.Join(t.TempDir(), "app.apk") + f, err := os.Create(path) + if err != nil { + t.Fatalf("create: %v", err) + } + defer f.Close() + + zw := zip.NewWriter(f) + mw, err := zw.Create("AndroidManifest.xml") + if err != nil { + t.Fatalf("zip create: %v", err) + } + if _, err := mw.Write(buildTestManifest()); err != nil { + t.Fatalf("write manifest: %v", err) + } + for name, data := range libs { + w, err := zw.Create(name) + if err != nil { + t.Fatalf("zip create %s: %v", name, err) + } + if _, err := w.Write(data); err != nil { + t.Fatalf("write %s: %v", name, err) + } + } + if err := zw.Close(); err != nil { + t.Fatalf("zip close: %v", err) + } + return path +} + +func TestScanArchiveRunsPolicyRulesOnMergedManifest(t *testing.T) { + apk := buildTestAPK(t, map[string][]byte{ + "lib/arm64-v8a/libgood.so": buildTestELF(16384, true), + }) + + res, err := ScanArchive(apk) + if err != nil { + t.Fatalf("ScanArchive: %v", err) + } + if res.ArchiveKind != string(KindAPK) { + t.Errorf("ArchiveKind = %q", res.ArchiveKind) + } + if !res.IsArchive { + t.Error("IsArchive not set") + } + if res.PackageName != "com.example.built" { + t.Errorf("PackageName = %q", res.PackageName) + } + if res.TargetSDK != 34 { + t.Errorf("TargetSDK = %d, want 34 from the merged manifest", res.TargetSDK) + } + + // The same source-tree rules must fire against the compiled manifest. + for _, policy := range []string{ + "Target API level", // targetSdk 34 + "SMS and Call Log permissions", // READ_SMS + "Malicious Behavior", // debuggable=true + "Foreground services", // location without its permission + } { + if findByPolicy(res.Findings, policy) == nil { + t.Errorf("policy %q did not fire on the archive scan", policy) + } + } + // A correctly aligned library must not be reported. + if f := findByPolicy(res.Findings, "16 KB page size"); f != nil { + t.Errorf("aligned library reported: %s", f.Title) + } +} + +func TestScanArchiveDetects16KBMisalignment(t *testing.T) { + apk := buildTestAPK(t, map[string][]byte{ + "lib/arm64-v8a/libold.so": buildTestELF(4096, true), + }) + res, err := ScanArchive(apk) + if err != nil { + t.Fatalf("ScanArchive: %v", err) + } + f := findByPolicy(res.Findings, "16 KB page size") + if f == nil { + t.Fatal("4 KB aligned arm64 library was not reported") + } + if f.Severity != sevCritical { + t.Errorf("severity = %s, want CRITICAL", f.Severity) + } + if !strings.Contains(f.Detail, "libold.so") || !strings.Contains(f.Detail, "4 KB") { + t.Errorf("detail should name the library and its alignment: %s", f.Detail) + } +} + +// 16 KB page size devices are 64-bit ARM, so a 32-bit library at 4 KB is not a +// violation and reporting it would be noise. +func TestScanArchiveIgnoresNon64BitABIs(t *testing.T) { + apk := buildTestAPK(t, map[string][]byte{ + "lib/armeabi-v7a/libold.so": buildTestELF(4096, true), + "lib/x86/libold.so": buildTestELF(4096, true), + }) + res, err := ScanArchive(apk) + if err != nil { + t.Fatalf("ScanArchive: %v", err) + } + if f := findByPolicy(res.Findings, "16 KB page size"); f != nil { + t.Errorf("32-bit ABI reported for 16 KB alignment: %s", f.Title) + } +} + +func TestScanArchiveFlagsMissingRelro(t *testing.T) { + apk := buildTestAPK(t, map[string][]byte{ + "lib/arm64-v8a/libnorelro.so": buildTestELF(16384, false), + }) + res, err := ScanArchive(apk) + if err != nil { + t.Fatalf("ScanArchive: %v", err) + } + var found bool + for _, f := range res.Findings { + if f.Policy == "16 KB page size" && strings.Contains(f.Title, "GNU_RELRO") { + found = true + } + } + if !found { + t.Error("library without GNU_RELRO was not reported") + } +} + +func TestScanArchiveRejectsNonAndroidZip(t *testing.T) { + path := filepath.Join(t.TempDir(), "notanapk.zip") + f, _ := os.Create(path) + zw := zip.NewWriter(f) + w, _ := zw.Create("readme.txt") + w.Write([]byte("hello")) + zw.Close() + f.Close() + + if _, err := ScanArchive(path); err == nil { + t.Error("a zip with no AndroidManifest.xml should be rejected") + } +} + +func TestScanArchiveDetectsBundleLayout(t *testing.T) { + path := filepath.Join(t.TempDir(), "app.aab") + f, _ := os.Create(path) + zw := zip.NewWriter(f) + // A bundle's manifest is protobuf, which this fixture does not supply; + // the point here is that the AAB layout is recognised and the undecodable + // manifest is surfaced rather than silently passing. + w, _ := zw.Create("base/manifest/AndroidManifest.xml") + w.Write([]byte{0x00}) + lw, _ := zw.Create("base/lib/arm64-v8a/libold.so") + lw.Write(buildTestELF(4096, true)) + zw.Close() + f.Close() + + res, err := ScanArchive(path) + if err != nil { + t.Fatalf("ScanArchive: %v", err) + } + if res.ArchiveKind != string(KindAAB) { + t.Errorf("ArchiveKind = %q, want AAB", res.ArchiveKind) + } + if findByPolicy(res.Findings, "Scan coverage") == nil { + t.Error("an undecodable manifest must be surfaced, not silently skipped") + } + // Native checks still run against the bundle's lib/ layout. + if findByPolicy(res.Findings, "16 KB page size") == nil { + t.Error("bundle native libraries were not checked") + } +} + +// --- AAB protobuf manifest --------------------------------------------- + +// pbField encodes one length-delimited protobuf field. +func pbField(num int, payload []byte) []byte { + var out []byte + out = binary.AppendUvarint(out, uint64(num)<<3|2) + out = binary.AppendUvarint(out, uint64(len(payload))) + return append(out, payload...) +} + +func pbString(num int, s string) []byte { return pbField(num, []byte(s)) } + +// pbAttr builds an XmlAttribute { name = 2, value = 3 }. +func pbAttr(name, value string) []byte { + return append(pbString(2, name), pbString(3, value)...) +} + +// pbElement builds an XmlElement { name = 3, attribute = 4, child = 5 }, +// wrapped in the XmlNode { element = 1 } the format nests everything in. +func pbElement(name string, attrs [][]byte, children [][]byte) []byte { + elem := pbString(3, name) + for _, a := range attrs { + elem = append(elem, pbField(4, a)...) + } + for _, c := range children { + elem = append(elem, pbField(5, c)...) + } + return pbField(1, elem) +} + +func TestDecodeProtoXML(t *testing.T) { + manifest := pbElement("manifest", + [][]byte{pbAttr("package", "com.example.bundle")}, + [][]byte{ + pbElement("uses-permission", [][]byte{pbAttr("name", "android.permission.READ_SMS")}, nil), + pbElement("uses-sdk", [][]byte{pbAttr("targetSdkVersion", "34")}, nil), + pbElement("application", + [][]byte{pbAttr("debuggable", "true")}, + [][]byte{ + pbElement("service", [][]byte{ + pbAttr("name", "com.example.Sync"), + pbAttr("foregroundServiceType", "location"), + }, nil), + }), + }) + + m, err := DecodeProtoXML(manifest) + if err != nil { + t.Fatalf("decode: %v", err) + } + if m.Package != "com.example.bundle" { + t.Errorf("Package = %q", m.Package) + } + if !m.HasPermission("android.permission.READ_SMS") { + t.Error("permission not decoded from protobuf manifest") + } + if m.UsesSDK == nil || m.UsesSDK.TargetSDKVersion != "34" { + t.Errorf("targetSdkVersion not decoded: %+v", m.UsesSDK) + } + if m.Application == nil || !attrIsTrue(m.Application.Debuggable) { + t.Error("debuggable not decoded") + } + if len(m.Application.Services) != 1 || m.Application.Services[0].ForegroundSvcType != "location" { + t.Errorf("nested service not decoded: %+v", m.Application) + } +} + +func TestDecodeProtoXMLRejectsGarbage(t *testing.T) { + if _, err := DecodeProtoXML([]byte{0xFF, 0xFF, 0xFF}); err == nil { + t.Error("malformed protobuf should error rather than yield an empty manifest") + } +} + +// A full AAB with a real protobuf manifest must run the policy rules just as +// an APK does. +func TestScanArchiveAABEndToEnd(t *testing.T) { + manifest := pbElement("manifest", + [][]byte{pbAttr("package", "com.example.bundle")}, + [][]byte{ + pbElement("uses-permission", [][]byte{pbAttr("name", "android.permission.QUERY_ALL_PACKAGES")}, nil), + pbElement("uses-sdk", [][]byte{pbAttr("targetSdkVersion", "34")}, nil), + pbElement("application", nil, nil), + }) + + path := filepath.Join(t.TempDir(), "app.aab") + f, err := os.Create(path) + if err != nil { + t.Fatalf("create: %v", err) + } + zw := zip.NewWriter(f) + w, _ := zw.Create("base/manifest/AndroidManifest.xml") + w.Write(manifest) + lw, _ := zw.Create("base/lib/arm64-v8a/libok.so") + lw.Write(buildTestELF(16384, true)) + zw.Close() + f.Close() + + res, err := ScanArchive(path) + if err != nil { + t.Fatalf("ScanArchive: %v", err) + } + if res.ArchiveKind != string(KindAAB) { + t.Errorf("ArchiveKind = %q", res.ArchiveKind) + } + if res.PackageName != "com.example.bundle" { + t.Errorf("PackageName = %q", res.PackageName) + } + if res.TargetSDK != 34 { + t.Errorf("TargetSDK = %d", res.TargetSDK) + } + if findByPolicy(res.Findings, "Package visibility") == nil { + t.Error("QUERY_ALL_PACKAGES from the bundle manifest was not flagged") + } + if findByPolicy(res.Findings, "Scan coverage") != nil { + t.Error("a valid protobuf manifest should decode cleanly") + } + if f := findByPolicy(res.Findings, "16 KB page size"); f != nil { + t.Errorf("aligned bundle library reported: %s", f.Title) + } +} diff --git a/internal/playscan/axml.go b/internal/playscan/axml.go new file mode 100644 index 0000000..f618cfc --- /dev/null +++ b/internal/playscan/axml.go @@ -0,0 +1,446 @@ +package playscan + +import ( + "encoding/binary" + "fmt" + "strconv" + "strings" + "unicode/utf16" +) + +// Android binary XML (AXML) chunk types. A compiled AndroidManifest.xml inside +// an APK is a sequence of these chunks rather than text, so reading the merged +// manifest that actually ships means decoding the format directly. +const ( + chunkStringPool = 0x0001 + chunkXMLDocument = 0x0003 + chunkXMLResource = 0x0180 + chunkXMLStartTag = 0x0102 + chunkXMLEndTag = 0x0103 +) + +// Typed attribute value formats, from ResourceTypes.h. +const ( + typeReference = 0x01 + typeString = 0x03 + typeIntDec = 0x10 + typeIntHex = 0x11 + typeIntBoolean = 0x12 + typeNull = 0x00 + attrRecordBytes = 20 +) + +// stringPoolUTF8Flag marks a pool whose strings are UTF-8 rather than UTF-16. +const stringPoolUTF8Flag = 1 << 8 + +// axmlAttr is one decoded attribute of an element. +type axmlAttr struct { + Name string + Value string +} + +// DecodeBinaryXML decodes a compiled AndroidManifest.xml into the same Manifest +// the source-tree parser produces, so every existing policy rule runs unchanged +// against the merged manifest. +func DecodeBinaryXML(data []byte) (*Manifest, error) { + if len(data) < 8 { + return nil, fmt.Errorf("binary XML too short (%d bytes)", len(data)) + } + if t := binary.LittleEndian.Uint16(data[0:2]); t != chunkXMLDocument { + return nil, fmt.Errorf("not Android binary XML (chunk type 0x%04x)", t) + } + + var pool []string + var resMap []uint32 + // Element stack, used to attach parsed children to the right parent. + m := &Manifest{} + var path []string + var currentActivity *Component + var currentComponentKind string + + offset := int(binary.LittleEndian.Uint16(data[2:4])) // header size + for offset+8 <= len(data) { + chunkType := binary.LittleEndian.Uint16(data[offset : offset+2]) + chunkSize := int(binary.LittleEndian.Uint32(data[offset+4 : offset+8])) + if chunkSize < 8 || offset+chunkSize > len(data) { + break + } + chunk := data[offset : offset+chunkSize] + + switch chunkType { + case chunkStringPool: + p, err := decodeStringPool(chunk) + if err != nil { + return nil, err + } + pool = p + + case chunkXMLStartTag: + name, attrs, err := decodeStartTag(chunk, pool, resMap) + if err != nil { + return nil, err + } + path = append(path, name) + applyElement(m, name, attrs, path, ¤tActivity, ¤tComponentKind) + + case chunkXMLEndTag: + if len(path) > 0 { + closing := path[len(path)-1] + path = path[:len(path)-1] + if closing == currentComponentKind { + currentActivity, currentComponentKind = nil, "" + } + } + + case chunkXMLResource: + resMap = decodeResourceMap(chunk) + } + + offset += chunkSize + } + + if m.Application == nil && len(m.Permissions) == 0 { + return nil, fmt.Errorf("no manifest content decoded") + } + return m, nil +} + +// applyElement folds one decoded element into the Manifest being built. +// +// Nesting matters: an belongs to whichever component is open, +// and only components directly inside are the app's own. +func applyElement(m *Manifest, name string, attrs []axmlAttr, path []string, current **Component, currentKind *string) { + get := func(key string) string { + for _, a := range attrs { + if a.Name == key { + return a.Value + } + } + return "" + } + + switch name { + case "manifest": + m.Package = get("package") + + case "uses-permission": + m.Permissions = append(m.Permissions, UsesPermission{Name: get("name")}) + case "uses-permission-sdk-23": + m.PermissionsSDK23 = append(m.PermissionsSDK23, UsesPermission{Name: get("name")}) + + case "uses-sdk": + m.UsesSDK = &UsesSDK{ + MinSDKVersion: get("minSdkVersion"), + TargetSDKVersion: get("targetSdkVersion"), + } + + case "application": + m.Application = &Application{ + Name: get("name"), + Debuggable: get("debuggable"), + AllowBackup: get("allowBackup"), + UsesCleartextTraffic: get("usesCleartextTraffic"), + NetworkSecurityConf: get("networkSecurityConfig"), + } + + case "activity", "activity-alias", "service", "receiver", "provider": + if m.Application == nil { + return + } + comp := Component{ + Name: get("name"), + Exported: get("exported"), + Permission: get("permission"), + ForegroundSvcType: get("foregroundServiceType"), + } + switch name { + case "activity": + m.Application.Activities = append(m.Application.Activities, comp) + *current = &m.Application.Activities[len(m.Application.Activities)-1] + case "activity-alias": + m.Application.ActivityAliases = append(m.Application.ActivityAliases, comp) + *current = &m.Application.ActivityAliases[len(m.Application.ActivityAliases)-1] + case "service": + m.Application.Services = append(m.Application.Services, comp) + *current = &m.Application.Services[len(m.Application.Services)-1] + case "receiver": + m.Application.Receivers = append(m.Application.Receivers, comp) + *current = &m.Application.Receivers[len(m.Application.Receivers)-1] + case "provider": + m.Application.Providers = append(m.Application.Providers, comp) + *current = &m.Application.Providers[len(m.Application.Providers)-1] + } + *currentKind = name + + case "intent-filter": + if *current != nil { + (*current).IntentFilters = append((*current).IntentFilters, IntentFilter{}) + } + + case "action", "category": + if *current == nil || len((*current).IntentFilters) == 0 { + return + } + f := &(*current).IntentFilters[len((*current).IntentFilters)-1] + entry := struct { + Name string `xml:"name,attr"` + }{Name: get("name")} + if name == "action" { + f.Actions = append(f.Actions, entry) + } else { + f.Categories = append(f.Categories, entry) + } + } +} + +// decodeStartTag reads one RES_XML_START_ELEMENT chunk. +// +// Layout: ResXMLTree_node is header(8) + lineNumber(4) + comment(4) = 16 bytes, +// followed by ResXMLTree_attrExt. attributeStart is an offset from the start of +// attrExt, NOT from the start of the chunk, so it has to be rebased by 16. +func decodeStartTag(chunk []byte, pool []string, resMap []uint32) (string, []axmlAttr, error) { + const attrExtOffset = 16 + if len(chunk) < 36 { + return "", nil, fmt.Errorf("start tag chunk too short") + } + nameIdx := binary.LittleEndian.Uint32(chunk[20:24]) + attrStart := int(binary.LittleEndian.Uint16(chunk[24:26])) + attrSize := int(binary.LittleEndian.Uint16(chunk[26:28])) + attrCount := int(binary.LittleEndian.Uint16(chunk[28:30])) + + // The record stride is declared per file rather than fixed. + if attrSize <= 0 { + attrSize = attrRecordBytes + } + + name := poolAt(pool, nameIdx) + + attrs := make([]axmlAttr, 0, attrCount) + for i := 0; i < attrCount; i++ { + off := attrExtOffset + attrStart + i*attrSize + if off < 0 || off+attrRecordBytes > len(chunk) { + break + } + rec := chunk[off : off+attrRecordBytes] + attrNameIdx := binary.LittleEndian.Uint32(rec[4:8]) + rawValueIdx := binary.LittleEndian.Uint32(rec[8:12]) + dataType := rec[15] + data := binary.LittleEndian.Uint32(rec[16:20]) + + attrs = append(attrs, axmlAttr{ + Name: attrName(pool, resMap, attrNameIdx), + Value: attrValue(pool, rawValueIdx, dataType, data), + }) + } + return name, attrs, nil +} + +// frameworkAttrIDs maps android framework attribute resource IDs to their +// names. aapt2 commonly emits an empty string-pool entry for a framework +// attribute and identifies it only by resource ID, so without this table every +// android:* attribute in a compiled manifest reads as unnamed. +var frameworkAttrIDs = map[uint32]string{ + 0x01010003: "name", + 0x01010006: "permission", + 0x0101000f: "debuggable", + 0x01010010: "exported", + 0x0101020c: "minSdkVersion", + 0x01010270: "targetSdkVersion", + 0x01010280: "allowBackup", + 0x010104ec: "usesCleartextTraffic", + 0x0101048f: "networkSecurityConfig", + 0x01010507: "networkSecurityConfig", + 0x01010596: "foregroundServiceType", +} + +// attrName resolves an attribute's name, preferring the string pool and +// falling back to the resource ID map when the pool entry is empty. +func attrName(pool []string, resMap []uint32, idx uint32) string { + if s := poolAt(pool, idx); s != "" { + return s + } + if int(idx) < len(resMap) { + if name, ok := frameworkAttrIDs[resMap[idx]]; ok { + return name + } + } + return "" +} + +// decodeResourceMap reads RES_XML_RESOURCE_MAP, an array of resource IDs +// indexed in parallel with the string pool. +func decodeResourceMap(chunk []byte) []uint32 { + headerSize := int(binary.LittleEndian.Uint16(chunk[2:4])) + if headerSize < 8 || headerSize > len(chunk) { + headerSize = 8 + } + body := chunk[headerSize:] + ids := make([]uint32, 0, len(body)/4) + for i := 0; i+4 <= len(body); i += 4 { + ids = append(ids, binary.LittleEndian.Uint32(body[i:i+4])) + } + return ids +} + +// attrValue renders a typed attribute value as the string the rules expect. +func attrValue(pool []string, rawIdx uint32, dataType byte, data uint32) string { + // A present raw value is the literal source text and is always preferred. + if rawIdx != 0xFFFFFFFF { + if s := poolAt(pool, rawIdx); s != "" { + return s + } + } + switch dataType { + case typeString: + return poolAt(pool, data) + case typeIntBoolean: + if data != 0 { + return "true" + } + return "false" + case typeIntDec: + return strconv.FormatUint(uint64(data), 10) + case typeIntHex: + // foregroundServiceType is a flags bitmask in compiled form; render it + // back to the pipe-separated names the source manifest would carry so + // the policy rules do not need a second code path. + if names := foregroundServiceTypeNames(data); names != "" { + return names + } + return "0x" + strconv.FormatUint(uint64(data), 16) + case typeReference: + if data == 0 { + return "" + } + return "@" + strconv.FormatUint(uint64(data), 16) + case typeNull: + return "" + } + return "" +} + +// foregroundServiceTypeBits maps the compiled bitmask back to the manifest +// attribute names, from android.content.pm.ServiceInfo. +var foregroundServiceTypeBits = []struct { + bit uint32 + name string +}{ + {1 << 0, "dataSync"}, + {1 << 1, "mediaPlayback"}, + {1 << 2, "phoneCall"}, + {1 << 3, "location"}, + {1 << 4, "connectedDevice"}, + {1 << 5, "mediaProjection"}, + {1 << 6, "camera"}, + {1 << 7, "microphone"}, + {1 << 8, "health"}, + {1 << 9, "remoteMessaging"}, + {1 << 10, "systemExempted"}, + {1 << 11, "shortService"}, + {1 << 12, "fileManagement"}, + {1 << 13, "mediaProcessing"}, + {1 << 30, "specialUse"}, +} + +func foregroundServiceTypeNames(mask uint32) string { + var names []string + for _, b := range foregroundServiceTypeBits { + if mask&b.bit != 0 { + names = append(names, b.name) + } + } + return strings.Join(names, "|") +} + +func poolAt(pool []string, idx uint32) string { + if idx == 0xFFFFFFFF || int(idx) >= len(pool) { + return "" + } + return pool[idx] +} + +// decodeStringPool reads a RES_STRING_POOL chunk in either encoding. +func decodeStringPool(chunk []byte) ([]string, error) { + if len(chunk) < 28 { + return nil, fmt.Errorf("string pool chunk too short") + } + count := int(binary.LittleEndian.Uint32(chunk[8:12])) + flags := binary.LittleEndian.Uint32(chunk[16:20]) + stringsStart := int(binary.LittleEndian.Uint32(chunk[20:24])) + utf8 := flags&stringPoolUTF8Flag != 0 + + if count < 0 || 28+count*4 > len(chunk) { + return nil, fmt.Errorf("string pool count %d out of range", count) + } + + out := make([]string, count) + for i := 0; i < count; i++ { + offAt := 28 + i*4 + rel := int(binary.LittleEndian.Uint32(chunk[offAt : offAt+4])) + pos := stringsStart + rel + if pos < 0 || pos >= len(chunk) { + continue + } + if utf8 { + out[i] = decodeUTF8String(chunk, pos) + } else { + out[i] = decodeUTF16String(chunk, pos) + } + } + return out, nil +} + +// decodeUTF8String reads a length-prefixed UTF-8 string. Both the UTF-16 length +// and the byte length are stored, each as one or two bytes depending on a high +// bit, so both prefixes have to be stepped over to reach the bytes. +func decodeUTF8String(b []byte, pos int) string { + if pos >= len(b) { + return "" + } + _, pos = readUTF8Len(b, pos) + byteLen, pos := readUTF8Len(b, pos) + if pos+byteLen > len(b) || byteLen < 0 { + return "" + } + return string(b[pos : pos+byteLen]) +} + +func readUTF8Len(b []byte, pos int) (int, int) { + if pos >= len(b) { + return 0, pos + } + n := int(b[pos]) + pos++ + if n&0x80 != 0 { + if pos >= len(b) { + return 0, pos + } + n = ((n & 0x7F) << 8) | int(b[pos]) + pos++ + } + return n, pos +} + +// decodeUTF16String reads a length-prefixed UTF-16LE string. +func decodeUTF16String(b []byte, pos int) string { + if pos+2 > len(b) { + return "" + } + n := int(binary.LittleEndian.Uint16(b[pos : pos+2])) + pos += 2 + if n&0x8000 != 0 { + if pos+2 > len(b) { + return "" + } + n = ((n & 0x7FFF) << 16) | int(binary.LittleEndian.Uint16(b[pos:pos+2])) + pos += 2 + } + if pos+n*2 > len(b) || n < 0 { + return "" + } + units := make([]uint16, n) + for i := 0; i < n; i++ { + units[i] = binary.LittleEndian.Uint16(b[pos+i*2 : pos+i*2+2]) + } + return string(utf16.Decode(units)) +} diff --git a/internal/playscan/protoxml.go b/internal/playscan/protoxml.go new file mode 100644 index 0000000..da3ff09 --- /dev/null +++ b/internal/playscan/protoxml.go @@ -0,0 +1,245 @@ +package playscan + +import ( + "encoding/binary" + "fmt" + "strconv" +) + +// An AAB stores its manifest as a protobuf-encoded XmlNode (aapt2's +// Resources.proto) rather than the binary XML an APK uses. Only a handful of +// fields are needed, and the protobuf wire format is self-describing enough to +// walk directly, which avoids taking a protobuf dependency for one file. +// +// The relevant subset of the schema: +// +// XmlNode { XmlElement element = 1; string text = 2 } +// XmlElement { XmlNamespace namespace_declaration = 1; string namespace_uri = 2; +// string name = 3; XmlAttribute attribute = 4; XmlNode child = 5 } +// XmlAttribute { string namespace_uri = 1; string name = 2; string value = 3; +// Source source = 4; uint32 resource_id = 5; Item compiled_item = 6 } +// Item { ... Prim prim = 2 ... } // typed fallback when value is empty +const ( + fieldElement = 1 + fieldElemName = 3 + fieldElemAttr = 4 + fieldElemChild = 5 + fieldAttrName = 2 + fieldAttrValue = 3 + fieldAttrCompiled = 6 +) + +// Protobuf wire types. +const ( + wireVarint = 0 + wireI64 = 1 + wireBytes = 2 + wireI32 = 5 +) + +// DecodeProtoXML decodes an AAB's protobuf AndroidManifest.xml into the same +// Manifest the other parsers produce. +func DecodeProtoXML(data []byte) (*Manifest, error) { + root, err := findElement(data) + if err != nil { + return nil, err + } + + m := &Manifest{} + var current *Component + var currentKind string + if err := walkProtoElement(root, m, ¤t, ¤tKind, nil); err != nil { + return nil, err + } + if m.Application == nil && len(m.Permissions) == 0 { + return nil, fmt.Errorf("no manifest content decoded") + } + return m, nil +} + +// findElement pulls the XmlElement out of the top-level XmlNode wrapper. +func findElement(node []byte) ([]byte, error) { + var elem []byte + err := eachField(node, func(num int, wire int, val []byte) error { + if num == fieldElement && wire == wireBytes { + elem = val + } + return nil + }) + if err != nil { + return nil, err + } + if elem == nil { + return nil, fmt.Errorf("no XmlElement in manifest root") + } + return elem, nil +} + +// walkProtoElement decodes one element and recurses into its children. +func walkProtoElement(elem []byte, m *Manifest, current **Component, currentKind *string, _ []string) error { + var name string + var attrs []axmlAttr + var children [][]byte + + err := eachField(elem, func(num, wire int, val []byte) error { + switch { + case num == fieldElemName && wire == wireBytes: + name = string(val) + case num == fieldElemAttr && wire == wireBytes: + a, err := decodeProtoAttr(val) + if err != nil { + return err + } + attrs = append(attrs, a) + case num == fieldElemChild && wire == wireBytes: + children = append(children, val) + } + return nil + }) + if err != nil { + return err + } + if name == "" { + return nil + } + + applyElement(m, name, attrs, nil, current, currentKind) + + for _, childNode := range children { + childElem, err := findElement(childNode) + if err != nil { + // A text node has no element; that is normal, not an error. + continue + } + if err := walkProtoElement(childElem, m, current, currentKind, nil); err != nil { + return err + } + } + + // Closing this element ends whichever component it opened. + if name == *currentKind { + *current, *currentKind = nil, "" + } + return nil +} + +// decodeProtoAttr reads one XmlAttribute, falling back to the compiled value +// when the source text was not retained (booleans and ints commonly are not). +func decodeProtoAttr(b []byte) (axmlAttr, error) { + var a axmlAttr + var compiled []byte + err := eachField(b, func(num, wire int, val []byte) error { + switch { + case num == fieldAttrName && wire == wireBytes: + a.Name = string(val) + case num == fieldAttrValue && wire == wireBytes: + a.Value = string(val) + case num == fieldAttrCompiled && wire == wireBytes: + compiled = val + } + return nil + }) + if err != nil { + return a, err + } + if a.Value == "" && compiled != nil { + a.Value = decodeCompiledItem(compiled) + } + return a, nil +} + +// decodeCompiledItem extracts a scalar from a compiled Item. The nested Prim +// message carries the value in a field whose number identifies its type; +// booleans and integers are the ones that matter for policy checks. +func decodeCompiledItem(b []byte) string { + var out string + _ = eachField(b, func(num, wire int, val []byte) error { + if wire != wireBytes { + return nil + } + // Descend into any nested message looking for a scalar. + _ = eachField(val, func(pnum, pwire int, pval []byte) error { + switch pwire { + case wireVarint: + v, _ := binary.Uvarint(pval) + // Prim field 3 is boolean_value in aapt2's schema. + if pnum == 3 { + if v != 0 { + out = "true" + } else { + out = "false" + } + return nil + } + if out == "" { + out = strconv.FormatUint(v, 10) + } + case wireI32: + if len(pval) == 4 && out == "" { + out = strconv.FormatUint(uint64(binary.LittleEndian.Uint32(pval)), 10) + } + } + return nil + }) + return nil + }) + return out +} + +// eachField walks a protobuf message, invoking fn per field. Varint payloads +// are handed back as their raw encoding so the caller can decode them. +func eachField(b []byte, fn func(num, wire int, val []byte) error) error { + for i := 0; i < len(b); { + key, n := binary.Uvarint(b[i:]) + if n <= 0 { + return fmt.Errorf("malformed protobuf field key") + } + i += n + num := int(key >> 3) + wire := int(key & 0x7) + + switch wire { + case wireVarint: + _, vn := binary.Uvarint(b[i:]) + if vn <= 0 { + return fmt.Errorf("malformed varint") + } + if err := fn(num, wire, b[i:i+vn]); err != nil { + return err + } + i += vn + case wireI64: + if i+8 > len(b) { + return fmt.Errorf("truncated 64-bit field") + } + if err := fn(num, wire, b[i:i+8]); err != nil { + return err + } + i += 8 + case wireBytes: + l, ln := binary.Uvarint(b[i:]) + if ln <= 0 { + return fmt.Errorf("malformed length prefix") + } + i += ln + if l > uint64(len(b)-i) { + return fmt.Errorf("length-delimited field overruns message") + } + if err := fn(num, wire, b[i:i+int(l)]); err != nil { + return err + } + i += int(l) + case wireI32: + if i+4 > len(b) { + return fmt.Errorf("truncated 32-bit field") + } + if err := fn(num, wire, b[i:i+4]); err != nil { + return err + } + i += 4 + default: + return fmt.Errorf("unsupported protobuf wire type %d", wire) + } + } + return nil +} diff --git a/internal/playscan/rules.go b/internal/playscan/rules.go index 6d6b4a0..7751bd6 100644 --- a/internal/playscan/rules.go +++ b/internal/playscan/rules.go @@ -19,6 +19,7 @@ const ( docAppContent = "https://support.google.com/googleplay/android-developer/answer/9859455" docProgramPolicy = "https://support.google.com/googleplay/android-developer/answer/16810878" docJul2026Policy = "https://support.google.com/googleplay/android-developer/answer/17134731" + docPageSizes = "https://developer.android.com/guide/practices/page-sizes" ) // Google Play's published requirements, as of the 2026 cycle. diff --git a/internal/playscan/types.go b/internal/playscan/types.go index ad05695..d2f55d9 100644 --- a/internal/playscan/types.go +++ b/internal/playscan/types.go @@ -38,9 +38,16 @@ type ScanResult struct { PackageName string `json:"package_name,omitempty"` TargetSDK int `json:"target_sdk,omitempty"` MinSDK int `json:"min_sdk,omitempty"` - // Permissions lists every permission declared in the source manifest. + // Permissions lists every permission declared in the manifest that was read. Permissions []string `json:"permissions,omitempty"` Findings []Finding `json:"findings"` + + // Archive context, set when scanning a built APK or AAB rather than source. + // A built archive carries the MERGED manifest, so its permission list is + // complete in a way a source scan's cannot be. + IsArchive bool `json:"is_archive,omitempty"` + ArchiveKind string `json:"archive_kind,omitempty"` + NativeLibCount int `json:"native_lib_count,omitempty"` } const ( From c078a3168e955dba2e6b16bc9ce2c71b89f1e569 Mon Sep 17 00:00:00 2001 From: ethan zhou <231755529+ethanzhoucool@users.noreply.github.com> Date: Mon, 27 Jul 2026 15:42:35 -0700 Subject: [PATCH 3/3] fix(playscan): address code review findings Two denial-of-service bugs in the binary parsers, both reachable through the stated threat model of scanning an archive someone handed you, and both reproduced before and after the fix: - protoxml: the decoder recursed once per manifest nesting level with no bound. A deeply nested AAB drove a Go stack overflow, which is a process abort recover() cannot catch. An 8.9 MB file was enough. Now capped at 100 levels, matching Android's own parser; the repro drops to 0.35s / 22 MB and reports an undecodable manifest. - axml: attrCount is a uint16 read straight from the file and need not match the bytes present, but it was used directly to size an allocation. A 97 KB APK of 36-byte chunks each claiming 65535 attributes cost 65s CPU and 1.46 GB RSS. Capped by the chunk's actual capacity; the repro drops to 0.04s / 110 MB. Correctness: - protoxml decoded compiled values against a guessed schema. Primitive's oneof is non-contiguous in Resources.proto (float=3, int_decimal=6, int_hex=7, boolean=8), and the code treated field 3 as the boolean, so every compiled boolean in a bundle was misread and an AAB's debuggable=true was missed entirely. Now decodes Item.str and Item.prim against the real field numbers. - The BIND_* permissions are never requested via ; the framework requires them as the bound component's android:permission. Those rules could therefore never fire. Component permissions are now inspected, and BIND_VPN_SERVICE is covered alongside accessibility and device admin. - Every foreground service needs the base FOREGROUND_SERVICE permission from API 28 regardless of type. Only the type-specific permissions were checked, so an app missing the base one was reported clean. - Gradle file ranking assumed the app module is named "app", the same assumption already fixed for manifest selection. Ranking is now tied to the module the selected manifest belongs to, so a library module's targetSdk cannot be reported as the app's. - The Billing Library version had to be a literal, but most builds reach it through a variable or a version catalog version.ref, so the support-window check silently never fired on them. Both forms now resolve. This surfaces a real finding on Thunderbird, which ships Billing 7.1.1 via version.ref. - detectIOS missed Objective-C sources and app.config.json, either of which could classify a cross-platform repo as Android-only and skip the Apple scanners. Also drops GradleInfo.CompileSDK and .Dependencies, which were written or declared but never read, and serializes an empty findings list as [] rather than null. --- README.md | 12 +- internal/playscan/archive.go | 1 + internal/playscan/archive_test.go | 121 ++++++++++++++++++++ internal/playscan/axml.go | 12 ++ internal/playscan/gradle.go | 70 +++++++++--- internal/playscan/manifest.go | 19 ++++ internal/playscan/protoxml.go | 123 +++++++++++++++----- internal/playscan/rules.go | 65 +++++++++-- internal/playscan/scanner.go | 44 ++++++-- internal/playscan/scanner_test.go | 179 +++++++++++++++++++++++++++++- internal/preflight/platform.go | 18 ++- internal/preflight/runner_test.go | 52 +++++++++ 12 files changed, 644 insertions(+), 72 deletions(-) diff --git a/README.md b/README.md index 7de67d9..31b8a28 100644 --- a/README.md +++ b/README.md @@ -102,7 +102,8 @@ stores in one pass. August 31, 2026; apps below API 35 already lose distribution to new users on newer devices — **CRITICAL / HIGH** - **Play Billing Library** — v7 and below lose support August 31, 2026, and - there is no direct v7 → v9 upgrade path — **HIGH** + there is no direct v7 → v9 upgrade path. Versions reached through a variable + or a version catalog `version.ref` are resolved — **HIGH** **Restricted permissions** (each needs an approved use case or declaration form) - SMS and Call Log — including the July 2026 change that drops phone-call @@ -111,12 +112,15 @@ stores in one pass. - `QUERY_ALL_PACKAGES`, `REQUEST_INSTALL_PACKAGES` - `ACCESS_BACKGROUND_LOCATION` (declaration + demo video) - Broad photo/video access over the system Photo Picker (API 33+) -- Accessibility Service, overlays, device admin, usage stats +- Accessibility Service, VPN service, device admin — declared as a component's + `android:permission` rather than `` +- Overlays, usage stats - Contacts, ahead of the 2026 Contact Permissions policy **Manifest and build** -- Foreground service types missing their required `FOREGROUND_SERVICE_*` - permission — crashes at `startForeground()` on Android 14+ — **CRITICAL** +- Foreground services missing the base `FOREGROUND_SERVICE` permission, or a + declared type missing its `FOREGROUND_SERVICE_*` permission — both throw at + `startForeground()` — **CRITICAL** - `specialUse` foreground services needing a Console justification - `android:exported` missing on components with an intent filter (API 31+) — the package fails to install — **CRITICAL** diff --git a/internal/playscan/archive.go b/internal/playscan/archive.go index 65bde7f..b04a02a 100644 --- a/internal/playscan/archive.go +++ b/internal/playscan/archive.go @@ -58,6 +58,7 @@ func ScanArchive(archivePath string) (*ScanResult, error) { ArchiveKind: string(kind), ManifestPath: manifestEntry, IsArchive: true, + Findings: []Finding{}, } var manifest *Manifest diff --git a/internal/playscan/archive_test.go b/internal/playscan/archive_test.go index cdf092d..92cfaae 100644 --- a/internal/playscan/archive_test.go +++ b/internal/playscan/archive_test.go @@ -598,3 +598,124 @@ func TestScanArchiveAABEndToEnd(t *testing.T) { t.Errorf("aligned bundle library reported: %s", f.Title) } } + +// --- review regressions ------------------------------------------------ + +// Primitive's oneof field numbers are non-contiguous in Resources.proto +// (float=3, int_decimal=6, int_hex=7, boolean=8). Treating an early number as +// the boolean silently misread every compiled boolean in a bundle, so +// android:debuggable="true" decoded as neither true nor false. +func TestDecodeCompiledPrimitiveUsesRealSchema(t *testing.T) { + // Item { Primitive prim = 7 { bool boolean_value = 8 } } + varint := func(num int, v uint64) []byte { + out := binary.AppendUvarint(nil, uint64(num)<<3|0) + return binary.AppendUvarint(out, v) + } + boolItem := pbField(7, varint(8, 1)) + if got := decodeCompiledItem(boolItem); got != "true" { + t.Errorf("boolean_value(field 8)=1 decoded as %q, want \"true\"", got) + } + falseItem := pbField(7, varint(8, 0)) + if got := decodeCompiledItem(falseItem); got != "false" { + t.Errorf("boolean_value(field 8)=0 decoded as %q, want \"false\"", got) + } + + // int_decimal_value = 6 uses zigzag (int32). + zig := func(num int, v int64) []byte { + out := binary.AppendUvarint(nil, uint64(num)<<3|0) + return binary.AppendVarint(out, v) + } + intItem := pbField(7, zig(6, 34)) + if got := decodeCompiledItem(intItem); got != "34" { + t.Errorf("int_decimal_value=34 decoded as %q", got) + } + + // int_hexadecimal_value = 7 carries the foregroundServiceType bitmask. + hexItem := pbField(7, varint(7, 1<<3)) + if got := decodeCompiledItem(hexItem); got != "location" { + t.Errorf("int_hex bitmask decoded as %q, want \"location\"", got) + } + + // Item { String str = 2 { string value = 1 } } + strItem := pbField(2, pbString(1, "com.example.Thing")) + if got := decodeCompiledItem(strItem); got != "com.example.Thing" { + t.Errorf("String item decoded as %q", got) + } +} + +// A compiled boolean must reach the policy rules, so an AAB with +// debuggable stored as a Primitive is still caught. +func TestAABCompiledBooleanReachesRules(t *testing.T) { + varint := func(num int, v uint64) []byte { + out := binary.AppendUvarint(nil, uint64(num)<<3|0) + return binary.AppendUvarint(out, v) + } + // XmlAttribute { name = 2, compiled_item = 6 } with no source text. + debuggableAttr := append(pbString(2, "debuggable"), pbField(6, pbField(7, varint(8, 1)))...) + + manifest := pbElement("manifest", + [][]byte{pbAttr("package", "com.example.bundle")}, + [][]byte{ + pbElement("uses-sdk", [][]byte{pbAttr("targetSdkVersion", "36")}, nil), + pbElement("application", [][]byte{debuggableAttr}, nil), + }) + + m, err := DecodeProtoXML(manifest) + if err != nil { + t.Fatalf("decode: %v", err) + } + if m.Application == nil || !attrIsTrue(m.Application.Debuggable) { + t.Fatalf("compiled debuggable boolean not decoded: %+v", m.Application) + } +} + +// The protobuf decoder recurses per nesting level, and a Go stack overflow is +// an unrecoverable process abort. A deeply nested bundle must be rejected, not +// crash the tool. +func TestDecodeProtoXMLRejectsDeepNesting(t *testing.T) { + // Each level is XmlElement{ name, child = XmlNode{ element = } }. + // The whole thing is wrapped in one XmlNode at the end, matching what + // DecodeProtoXML expects at the root. + elem := pbString(3, "manifest") + for i := 0; i < maxXMLDepth+50; i++ { + elem = append(pbString(3, "m"), pbField(5, pbField(1, elem))...) + } + _, err := DecodeProtoXML(pbField(1, elem)) + if err == nil { + t.Fatal("deeply nested manifest should be rejected") + } + if !strings.Contains(err.Error(), "nesting") { + t.Errorf("error should name the nesting limit, got %v", err) + } +} + +// attrCount is a uint16 read straight from the file and need not match the +// bytes present. Uncapped, a 36-byte chunk claiming 65535 attributes forced a +// 2 MB allocation, and a file packed with them burned gigabytes. +func TestDecodeStartTagCapsAttrCountToChunkSize(t *testing.T) { + var chunk bytes.Buffer + binary.Write(&chunk, binary.LittleEndian, uint16(chunkXMLStartTag)) + binary.Write(&chunk, binary.LittleEndian, uint16(8)) + binary.Write(&chunk, binary.LittleEndian, uint32(36)) + binary.Write(&chunk, binary.LittleEndian, uint32(1)) // lineNumber + binary.Write(&chunk, binary.LittleEndian, uint32(0xFFFFFFFF)) // comment + binary.Write(&chunk, binary.LittleEndian, uint32(0xFFFFFFFF)) // ns + binary.Write(&chunk, binary.LittleEndian, uint32(0)) // name + binary.Write(&chunk, binary.LittleEndian, uint16(20)) // attributeStart + binary.Write(&chunk, binary.LittleEndian, uint16(20)) // attributeSize + binary.Write(&chunk, binary.LittleEndian, uint16(65535)) // attributeCount, a lie + binary.Write(&chunk, binary.LittleEndian, uint16(0)) + binary.Write(&chunk, binary.LittleEndian, uint16(0)) + binary.Write(&chunk, binary.LittleEndian, uint16(0)) + + _, attrs, err := decodeStartTag(chunk.Bytes(), []string{"tag"}, nil) + if err != nil { + t.Fatalf("decode: %v", err) + } + if len(attrs) != 0 { + t.Errorf("got %d attributes from a chunk with room for none", len(attrs)) + } + if c := cap(attrs); c > 8 { + t.Errorf("allocated capacity %d for a chunk with room for 0 attributes", c) + } +} diff --git a/internal/playscan/axml.go b/internal/playscan/axml.go index f618cfc..85393ea 100644 --- a/internal/playscan/axml.go +++ b/internal/playscan/axml.go @@ -215,6 +215,18 @@ func decodeStartTag(chunk []byte, pool []string, resMap []uint32) (string, []axm name := poolAt(pool, nameIdx) + // attrCount is attacker-controlled (a uint16 read straight from the file) + // and need not match the bytes actually present. Cap it by what the chunk + // can physically hold before allocating, otherwise a 36-byte chunk claiming + // 65535 attributes costs a 2 MB allocation, and a file packed with such + // chunks turns a tiny APK into gigabytes of allocation churn. + if room := (len(chunk) - attrExtOffset - attrStart) / attrSize; attrCount > room { + if room < 0 { + room = 0 + } + attrCount = room + } + attrs := make([]axmlAttr, 0, attrCount) for i := 0; i < attrCount; i++ { off := attrExtOffset + attrStart + i*attrSize diff --git a/internal/playscan/gradle.go b/internal/playscan/gradle.go index 1345d60..882e0d5 100644 --- a/internal/playscan/gradle.go +++ b/internal/playscan/gradle.go @@ -10,9 +10,8 @@ import ( // GradleInfo is what the policy rules need out of the Gradle build files. type GradleInfo struct { - TargetSDK int - CompileSDK int - MinSDK int + TargetSDK int + MinSDK int // TargetSDKFile / TargetSDKLine locate the declaration so the report can // point at the line to edit. TargetSDKFile string @@ -23,7 +22,6 @@ type GradleInfo struct { BillingVersionRaw string BillingFile string BillingLine int - Dependencies []string HasAdsSDK bool HasAuthSDK bool AdsSDKFile string @@ -51,14 +49,26 @@ type GradleInfo struct { // rootProject.ext.targetSdkVersion`) is skipped here and picked up from // whichever file defines the literal, since ext blocks are scanned too. var ( - reTargetSDK = regexp.MustCompile(`\btargetSdk(?:Version)?\b\s*(?:=|\.set\(|\()?\s*["']?(\d{1,2})["']?`) - reCompileSDK = regexp.MustCompile(`\bcompileSdk(?:Version)?\b\s*(?:=|\.set\(|\()?\s*["']?(\d{1,2})["']?`) - reMinSDK = regexp.MustCompile(`\bminSdk(?:Version)?\b\s*(?:=|\.set\(|\()?\s*["']?(\d{1,2})["']?`) + reTargetSDK = regexp.MustCompile(`\btargetSdk(?:Version)?\b\s*(?:=|\.set\(|\()?\s*["']?(\d{1,2})["']?`) + reMinSDK = regexp.MustCompile(`\bminSdk(?:Version)?\b\s*(?:=|\.set\(|\()?\s*["']?(\d{1,2})["']?`) // Matches com.android.billingclient:billing:8.0.0 and the -ktx artifact, // including version catalog style quoting. reBilling = regexp.MustCompile(`com\.android\.billingclient:billing(?:-ktx)?:["']?v?(\d+)(?:\.(\d+))?`) + // The billing artifact with its version supplied indirectly, which is how + // most real builds declare it. Without these the version is unreadable and + // the support-window check silently never fires: + // + // implementation "com.android.billingclient:billing:$billingVersion" + // billing = { module = "com.android.billingclient:billing", version.ref = "billing" } + reBillingInterp = regexp.MustCompile(`com\.android\.billingclient:billing(?:-ktx)?:\$\{?([A-Za-z_][A-Za-z0-9_.]*)\}?`) + reBillingRef = regexp.MustCompile(`com\.android\.billingclient:billing(?:-ktx)?["'].*version\.ref\s*=\s*["']([A-Za-z_][A-Za-z0-9_-]*)["']`) + + // A version string constant, e.g. billingVersion = "7.1.1" or a + // [versions] entry in a catalog. + reVersionString = regexp.MustCompile(`(?:^|\s)(?:const\s+)?(?:val|var|ext\.)?\s*([A-Za-z_][A-Za-z0-9_-]*)\s*=\s*["']v?(\d+)(?:\.(\d+))?[^"']*["']`) + reAdsSDK = regexp.MustCompile(`(?i)(com\.google\.android\.gms:play-services-ads|com\.google\.android\.ads|applovin|com\.facebook\.android:audience-network|com\.unity3d\.ads|ironsource|com\.mbridge|com\.vungle|adcolony|com\.chartboost)`) // Authentication SDKs, used as the signal that an app creates accounts. @@ -175,6 +185,11 @@ func parseGradleFiles(paths []string, relTo func(string) string) *GradleInfo { var refExpr, refFile string var refLine int + // Version strings, and the first unresolved billing version reference. + versionStrings := map[string]string{} + var billingRef, billingRefFile string + var billingRefLine int + for _, p := range paths { data, err := os.ReadFile(p) if err != nil { @@ -188,6 +203,16 @@ func parseGradleFiles(paths []string, relTo func(string) string) *GradleInfo { continue } + if m := reVersionString.FindStringSubmatch(code); m != nil { + if _, seen := versionStrings[m[1]]; !seen { + v := m[2] + if m[3] != "" { + v = m[2] + "." + m[3] + } + versionStrings[m[1]] = v + } + } + for _, re := range []*regexp.Regexp{reSymbolAssign, reSymbolSet, reSymbolTo} { if m := re.FindStringSubmatch(code); m != nil { if v, err := strconv.Atoi(m[2]); err == nil { @@ -209,13 +234,6 @@ func parseGradleFiles(paths []string, relTo func(string) string) *GradleInfo { } } } - if g.CompileSDK == 0 { - if m := reCompileSDK.FindStringSubmatch(code); m != nil { - if v, err := strconv.Atoi(m[1]); err == nil { - g.CompileSDK = v - } - } - } if g.MinSDK == 0 { if m := reMinSDK.FindStringSubmatch(code); m != nil { if v, err := strconv.Atoi(m[1]); err == nil { @@ -233,6 +251,12 @@ func parseGradleFiles(paths []string, relTo func(string) string) *GradleInfo { } g.BillingFile, g.BillingLine = rel, lineNo } + } else if billingRef == "" { + if mm := reBillingInterp.FindStringSubmatch(code); mm != nil { + billingRef, billingRefFile, billingRefLine = lastIdentSegment(mm[1]), rel, lineNo + } else if mm := reBillingRef.FindStringSubmatch(code); mm != nil { + billingRef, billingRefFile, billingRefLine = mm[1], rel, lineNo + } } } if g.Namespace == "" { @@ -252,6 +276,15 @@ func parseGradleFiles(paths []string, relTo func(string) string) *GradleInfo { } } + if g.BillingVersion == 0 && billingRef != "" { + if raw, ok := versionStrings[billingRef]; ok { + if major, err := strconv.Atoi(strings.SplitN(raw, ".", 2)[0]); err == nil { + g.BillingVersion, g.BillingVersionRaw = major, raw + g.BillingFile, g.BillingLine = billingRefFile, billingRefLine + } + } + } + if g.TargetSDK == 0 && refExpr != "" { if v, ok := resolveSDKRef(refExpr, symbols); ok { g.TargetSDK, g.TargetSDKFile, g.TargetSDKLine = v, refFile, refLine @@ -260,6 +293,15 @@ func parseGradleFiles(paths []string, relTo func(string) string) *GradleInfo { return g } +// lastIdentSegment returns the final segment of a dotted reference, e.g. +// "libs.versions.billing" -> "billing". +func lastIdentSegment(ref string) string { + if i := strings.LastIndex(ref, "."); i >= 0 && i+1 < len(ref) { + return ref[i+1:] + } + return ref +} + // resolveSDKRef resolves a targetSdk expression against the collected symbols. // // The name is taken from a quoted property key when the expression has one diff --git a/internal/playscan/manifest.go b/internal/playscan/manifest.go index 4032db6..094b96d 100644 --- a/internal/playscan/manifest.go +++ b/internal/playscan/manifest.go @@ -177,3 +177,22 @@ func parseSDKInt(v string) int { } return n } + +// HasComponentPermission reports whether any component is guarded by the given +// android:permission. +// +// The BIND_* permissions are declared this way rather than through +// : the framework requires the bound service or receiver to +// name the permission the system holds. Checking only the uses-permission list +// would miss every accessibility service, device admin, and VPN service. +func (m *Manifest) HasComponentPermission(name string) bool { + if m.Application == nil { + return false + } + for _, comp := range m.Application.Components() { + if strings.TrimSpace(comp.Permission) == name { + return true + } + } + return false +} diff --git a/internal/playscan/protoxml.go b/internal/playscan/protoxml.go index da3ff09..402518f 100644 --- a/internal/playscan/protoxml.go +++ b/internal/playscan/protoxml.go @@ -3,6 +3,7 @@ package playscan import ( "encoding/binary" "fmt" + "math" "strconv" ) @@ -18,7 +19,7 @@ import ( // string name = 3; XmlAttribute attribute = 4; XmlNode child = 5 } // XmlAttribute { string namespace_uri = 1; string name = 2; string value = 3; // Source source = 4; uint32 resource_id = 5; Item compiled_item = 6 } -// Item { ... Prim prim = 2 ... } // typed fallback when value is empty +// Item { String str = 2; Primitive prim = 7 } // used when value is empty const ( fieldElement = 1 fieldElemName = 3 @@ -48,7 +49,7 @@ func DecodeProtoXML(data []byte) (*Manifest, error) { m := &Manifest{} var current *Component var currentKind string - if err := walkProtoElement(root, m, ¤t, ¤tKind, nil); err != nil { + if err := walkProtoElement(root, m, ¤t, ¤tKind, 0); err != nil { return nil, err } if m.Application == nil && len(m.Permissions) == 0 { @@ -76,7 +77,10 @@ func findElement(node []byte) ([]byte, error) { } // walkProtoElement decodes one element and recurses into its children. -func walkProtoElement(elem []byte, m *Manifest, current **Component, currentKind *string, _ []string) error { +func walkProtoElement(elem []byte, m *Manifest, current **Component, currentKind *string, depth int) error { + if depth > maxXMLDepth { + return fmt.Errorf("manifest nesting exceeds %d levels", maxXMLDepth) + } var name string var attrs []axmlAttr var children [][]byte @@ -111,7 +115,7 @@ func walkProtoElement(elem []byte, m *Manifest, current **Component, currentKind // A text node has no element; that is normal, not an error. continue } - if err := walkProtoElement(childElem, m, current, currentKind, nil); err != nil { + if err := walkProtoElement(childElem, m, current, currentKind, depth+1); err != nil { return err } } @@ -148,39 +152,98 @@ func decodeProtoAttr(b []byte) (axmlAttr, error) { return a, nil } -// decodeCompiledItem extracts a scalar from a compiled Item. The nested Prim -// message carries the value in a field whose number identifies its type; -// booleans and integers are the ones that matter for policy checks. +// Item and Primitive field numbers, from aapt2's Resources.proto. +// +// These are not guessable — Primitive's oneof is deliberately non-contiguous +// (float is 3, int_decimal is 6, boolean is 8), so treating an early field +// number as the boolean silently misreads every compiled boolean in a bundle. +const ( + itemFieldStr = 2 + itemFieldPrim = 7 + + primFloat = 3 + primIntDecimal = 6 + primIntHex = 7 + primBoolean = 8 + + stringFieldValue = 1 +) + +// maxXMLDepth bounds manifest nesting. The decoder walks the tree recursively, +// and a Go stack overflow is a process abort that recover() cannot catch, so an +// attacker-supplied bundle must not be able to drive the depth. Android's own +// XML parser caps nesting at 100; a real manifest is under 10 levels deep. +const maxXMLDepth = 100 + +// decodeCompiledItem extracts a scalar from a compiled Item, which is how an +// AAB stores an attribute value whose source text was not retained. func decodeCompiledItem(b []byte) string { var out string _ = eachField(b, func(num, wire int, val []byte) error { - if wire != wireBytes { + if wire != wireBytes || out != "" { return nil } - // Descend into any nested message looking for a scalar. - _ = eachField(val, func(pnum, pwire int, pval []byte) error { - switch pwire { - case wireVarint: - v, _ := binary.Uvarint(pval) - // Prim field 3 is boolean_value in aapt2's schema. - if pnum == 3 { - if v != 0 { - out = "true" - } else { - out = "false" - } - return nil - } - if out == "" { - out = strconv.FormatUint(v, 10) + switch num { + case itemFieldStr: + // String { string value = 1 } + _ = eachField(val, func(snum, swire int, sval []byte) error { + if snum == stringFieldValue && swire == wireBytes { + out = string(sval) } - case wireI32: - if len(pval) == 4 && out == "" { - out = strconv.FormatUint(uint64(binary.LittleEndian.Uint32(pval)), 10) - } - } + return nil + }) + case itemFieldPrim: + out = decodePrimitive(val) + } + return nil + }) + return out +} + +// decodePrimitive renders a Primitive as the string the policy rules expect. +func decodePrimitive(b []byte) string { + var out string + _ = eachField(b, func(num, wire int, val []byte) error { + if out != "" { return nil - }) + } + switch num { + case primBoolean: + if wire != wireVarint { + return nil + } + v, _ := binary.Uvarint(val) + if v != 0 { + out = "true" + } else { + out = "false" + } + case primIntDecimal: + if wire != wireVarint { + return nil + } + v, n := binary.Varint(val) + if n > 0 { + out = strconv.FormatInt(v, 10) + } + case primIntHex: + if wire != wireVarint { + return nil + } + v, _ := binary.Uvarint(val) + // foregroundServiceType arrives here as a flags bitmask; render it + // back to the names the source manifest would carry. + if names := foregroundServiceTypeNames(uint32(v)); names != "" { + out = names + } else { + out = strconv.FormatUint(v, 10) + } + case primFloat: + if wire != wireI32 || len(val) != 4 { + return nil + } + out = strconv.FormatFloat(float64(math.Float32frombits(binary.LittleEndian.Uint32(val))), 'g', -1, 32) + } return nil }) return out diff --git a/internal/playscan/rules.go b/internal/playscan/rules.go index 7751bd6..60ff815 100644 --- a/internal/playscan/rules.go +++ b/internal/playscan/rules.go @@ -186,6 +186,11 @@ type restrictedPermission struct { // minTargetSDK gates policies that only bind above a target level; 0 means // the policy applies regardless. minTargetSDK int + // componentBound marks a BIND_* permission that is never requested via + // . The framework requires it as the android:permission + // attribute of the or being bound, so checking only + // the uses-permission list would never fire. + componentBound bool } var restrictedPermissions = []restrictedPermission{ @@ -270,8 +275,9 @@ var restrictedPermissions = []restrictedPermission{ title: "Accessibility Service use is tightly restricted", detail: "The Accessibility APIs may only be used to help users with disabilities, and the app must disclose the use in the store listing and in-app. " + "Using accessibility for automation, overlays, or ad interaction is one of the most common causes of app suspension rather than simple rejection.", - fix: "Confirm the service exists to support users with disabilities and disclose it prominently. Otherwise use a purpose-built API.", - doc: docProgramPolicy, + fix: "Confirm the service exists to support users with disabilities and disclose it prominently. Otherwise use a purpose-built API.", + doc: docProgramPolicy, + componentBound: true, }, { names: []string{"android.permission.SYSTEM_ALERT_WINDOW"}, @@ -283,13 +289,24 @@ var restrictedPermissions = []restrictedPermission{ doc: docProgramPolicy, }, { - names: []string{"android.permission.BIND_DEVICE_ADMIN"}, - severity: sevWarn, - policy: "Device admin", - title: "Device administrator API requires a permitted use case", - detail: "Device admin is limited to genuine device management use cases. Using it to make the app hard to uninstall is an abuse signal.", - fix: "Confirm the app is an enterprise or family management tool, and that admin rights can be revoked normally.", - doc: docProgramPolicy, + names: []string{"android.permission.BIND_DEVICE_ADMIN"}, + severity: sevWarn, + policy: "Device admin", + title: "Device administrator API requires a permitted use case", + detail: "Device admin is limited to genuine device management use cases. Using it to make the app hard to uninstall is an abuse signal.", + fix: "Confirm the app is an enterprise or family management tool, and that admin rights can be revoked normally.", + doc: docProgramPolicy, + componentBound: true, + }, + { + names: []string{"android.permission.BIND_VPN_SERVICE"}, + severity: sevWarn, + policy: "VPN Service", + title: "VPN service requires the VpnService declaration", + detail: "Only apps whose core functionality is a VPN may use VpnService, and the use must be declared in Play Console. Using it to monitor or redirect other apps' traffic for analytics or ad injection violates policy.", + fix: "Confirm a VPN is the app's core purpose and complete the VPN declaration in Play Console.", + doc: docProgramPolicy, + componentBound: true, }, { names: []string{"android.permission.PACKAGE_USAGE_STATS"}, @@ -324,6 +341,12 @@ func ruleRestrictedPermissions(c *ruleContext) []Finding { for _, name := range rp.names { if c.manifest.HasPermission(name) { hit = append(hit, shortPermission(name)) + continue + } + // BIND_* permissions are enforced on the component, not requested + // by the app, so they appear as a component's android:permission. + if rp.componentBound && c.manifest.HasComponentPermission(name) { + hit = append(hit, shortPermission(name)) } } if len(hit) == 0 { @@ -373,6 +396,30 @@ func ruleForegroundServiceTypes(c *ruleContext) []Finding { var findings []Finding reported := make(map[string]bool) + // Every foreground service needs the base FOREGROUND_SERVICE permission + // from API 28, independent of its type. Without it startForeground() + // throws just as surely as a missing type-specific permission, so checking + // only the type-specific ones misses the case entirely. + declaresForegroundService := false + for _, svc := range c.manifest.Application.Services { + if svc.ForegroundSvcType != "" { + declaresForegroundService = true + break + } + } + if declaresForegroundService && !c.manifest.HasPermission("android.permission.FOREGROUND_SERVICE") { + findings = append(findings, Finding{ + Severity: sevCritical, + Policy: "Foreground services", + Title: "Foreground service declared without the FOREGROUND_SERVICE permission", + Detail: "The manifest declares a foreground service but does not request android.permission.FOREGROUND_SERVICE, which every foreground service has required since API 28. " + + "startForeground() throws SecurityException, so the feature fails on every supported device.", + Fix: "Add to the manifest.", + Doc: docForegroundSvc, + File: c.manifestFile, + }) + } + for _, svc := range c.manifest.Application.Services { if svc.ForegroundSvcType == "" { continue diff --git a/internal/playscan/scanner.go b/internal/playscan/scanner.go index 3b87faf..f4ae698 100644 --- a/internal/playscan/scanner.go +++ b/internal/playscan/scanner.go @@ -53,7 +53,9 @@ func Detect(root string) bool { // A directory with no Android sources is not an error; it returns a result with // no manifest and no findings, so the caller can run this unconditionally. func Scan(root string) (*ScanResult, error) { - result := &ScanResult{ProjectPath: root} + // Findings starts non-nil so a clean scan serializes as [] rather than + // null, which consumers parsing the JSON should not have to special-case. + result := &ScanResult{ProjectPath: root, Findings: []Finding{}} manifestPath, gradlePaths := discover(root) if manifestPath == "" && len(gradlePaths) == 0 { @@ -167,20 +169,40 @@ func discover(root string) (manifestPath string, gradlePaths []string) { } return manifests[i] < manifests[j] }) + + // The manifest is chosen first so Gradle ranking can be tied to the module + // that manifest belongs to. Without that link, a repo whose app module is + // not named "app" ranks its build files no higher than a library's, and the + // scan can report a library module's targetSdk as the app's. + if len(manifests) > 0 { + manifestPath = selectAppManifest(manifests) + } + appModule := moduleDirOf(manifestPath) + sort.Slice(gradles, func(i, j int) bool { - si, sj := gradleScore(gradles[i]), gradleScore(gradles[j]) + si, sj := gradleScore(gradles[i], appModule), gradleScore(gradles[j], appModule) if si != sj { return si > sj } return gradles[i] < gradles[j] }) - if len(manifests) > 0 { - manifestPath = selectAppManifest(manifests) - } return manifestPath, gradles } +// moduleDirOf returns the Gradle module directory a manifest belongs to, i.e. +// the path above its source set. Empty when there is no manifest to anchor to. +func moduleDirOf(manifestPath string) string { + if manifestPath == "" { + return "" + } + p := filepath.ToSlash(manifestPath) + if i := strings.Index(p, "/src/"); i > 0 { + return p[:i] + } + return filepath.ToSlash(filepath.Dir(p)) +} + // selectAppManifest picks the shipped application's manifest out of every // candidate in the repo. // @@ -237,11 +259,17 @@ func manifestScore(path string) int { // The app module's own android {} block wins, then the convention plugin that // configures application modules, then other build-logic sources, then the // version catalog those files interpolate from. -func gradleScore(path string) int { +func gradleScore(path string, appModule string) int { p := filepath.ToSlash(path) base := strings.ToLower(filepath.Base(p)) score := 0 + // A build file sitting in the same module as the app's manifest is the + // app's own, whatever that module happens to be called. + if appModule != "" && strings.HasPrefix(p, appModule+"/") { + score += 20 + } + switch { case isGradleProperties(base): // Properties only hold values other files reference. @@ -262,7 +290,9 @@ func gradleScore(path string) int { } case strings.HasPrefix(base, "build.gradle"): score += 5 - if strings.Contains(p, "/app/") { + // Fallback for the conventional module name when there is no manifest + // to anchor to (a Gradle-only scan). + if appModule == "" && strings.Contains(p, "/app/") { score += 10 } case strings.HasPrefix(base, "settings.gradle"): diff --git a/internal/playscan/scanner_test.go b/internal/playscan/scanner_test.go index 27cca51..53a5a5e 100644 --- a/internal/playscan/scanner_test.go +++ b/internal/playscan/scanner_test.go @@ -382,6 +382,7 @@ func TestForegroundServiceMultipleTypes(t *testing.T) { root := writeProject(t, map[string]string{ "app/build.gradle": "android { defaultConfig { targetSdk = 36 } }", "app/src/main/AndroidManifest.xml": manifestHeader + ` + @@ -392,9 +393,114 @@ func TestForegroundServiceMultipleTypes(t *testing.T) { if err != nil { t.Fatalf("scan: %v", err) } - f := findByPolicy(res.Findings, "Foreground services") - if f == nil || !strings.Contains(f.Detail, "FOREGROUND_SERVICE_CAMERA") { - t.Fatalf("the camera half of location|camera was not checked, got %+v", f) + var sawCamera bool + for _, f := range res.Findings { + if f.Policy == "Foreground services" && strings.Contains(f.Detail, "FOREGROUND_SERVICE_CAMERA") { + sawCamera = true + } + } + if !sawCamera { + t.Fatalf("the camera half of location|camera was not checked: %+v", res.Findings) + } + // location's permission is declared, so only camera should be reported. + for _, f := range res.Findings { + if f.Policy == "Foreground services" && strings.Contains(f.Detail, "FOREGROUND_SERVICE_LOCATION") { + t.Errorf("location permission is declared but was still reported: %s", f.Detail) + } + } +} + +// Every foreground service needs the base FOREGROUND_SERVICE permission, not +// just the type-specific one. Checking only the type-specific permissions +// missed this case entirely. +func TestForegroundServiceBasePermissionRequired(t *testing.T) { + root := writeProject(t, map[string]string{ + "app/build.gradle": "android { defaultConfig { targetSdk = 36 } }", + "app/src/main/AndroidManifest.xml": manifestHeader + ` + + + + +`, + }) + res, err := Scan(root) + if err != nil { + t.Fatalf("scan: %v", err) + } + var sawBase bool + for _, f := range res.Findings { + if f.Policy == "Foreground services" && strings.Contains(f.Title, "without the FOREGROUND_SERVICE permission") { + sawBase = true + if f.Severity != sevCritical { + t.Errorf("severity = %s, want CRITICAL", f.Severity) + } + } + } + if !sawBase { + t.Error("missing base FOREGROUND_SERVICE permission was not reported") + } +} + +// BIND_* permissions are never requested via ; the framework +// requires them as the component's android:permission. Checking only the +// uses-permission list meant these rules could never fire. +func TestComponentBoundPermissionsDetected(t *testing.T) { + root := writeProject(t, map[string]string{ + "app/build.gradle": "android { defaultConfig { targetSdk = 36 } }", + "app/src/main/AndroidManifest.xml": manifestHeader + ` + + + + + +`, + }) + res, err := Scan(root) + if err != nil { + t.Fatalf("scan: %v", err) + } + for _, policy := range []string{"Accessibility API", "VPN Service", "Device admin"} { + if findByPolicy(res.Findings, policy) == nil { + t.Errorf("component-declared permission for %q was not detected", policy) + } + } +} + +// The app module is frequently not named "app", so Gradle files must be ranked +// against the module the selected manifest belongs to. Otherwise a library +// module's targetSdk can be reported as the app's. +func TestGradleSelectionFollowsAppModule(t *testing.T) { + root := writeProject(t, map[string]string{ + // A library module that would otherwise win on path score. + "app/build.gradle": "android { defaultConfig { targetSdk = 30 } }", + "app/src/main/AndroidManifest.xml": manifestHeader + ` + +`, + // The real application module, named after the product. + "MyProduct/build.gradle": "android { defaultConfig { targetSdk = 36 } }", + "MyProduct/src/main/AndroidManifest.xml": manifestHeader + ` + + + + + + + + +`, + }) + res, err := Scan(root) + if err != nil { + t.Fatalf("scan: %v", err) + } + if res.TargetSDK != 36 { + t.Errorf("TargetSDK = %d, want 36 from the app module's own build.gradle", res.TargetSDK) + } + if f := findByPolicy(res.Findings, "Target API level"); f != nil { + t.Errorf("app module targets 36 and is compliant, got %q", f.Title) } } @@ -914,3 +1020,70 @@ func TestTargetSDKResolvedThroughReferences(t *testing.T) { }) } } + +// Most real builds reach the Billing Library version through a variable or a +// version catalog. Requiring a literal meant the support-window check silently +// never fired on them. +func TestBillingVersionResolvedThroughReferences(t *testing.T) { + freezeClock(t, time.Date(2026, time.July, 27, 0, 0, 0, 0, time.UTC)) + cases := []struct { + name string + files map[string]string + want string + }{ + { + name: "groovy string interpolation", + files: map[string]string{ + "app/build.gradle": `ext { billingVersion = "7.1.1" } +android { defaultConfig { targetSdk = 36 } } +dependencies { implementation "com.android.billingclient:billing:$billingVersion" }`, + }, + want: "7.1", + }, + { + name: "version catalog version.ref", + files: map[string]string{ + "app/build.gradle": `android { defaultConfig { targetSdk = 36 } } +dependencies { implementation libs.billing }`, + "gradle/libs.versions.toml": `[versions] +billing = "6.2.0" + +[libraries] +billing = { module = "com.android.billingclient:billing", version.ref = "billing" }`, + }, + want: "6.2", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + root := writeProject(t, tc.files) + res, err := Scan(root) + if err != nil { + t.Fatalf("scan: %v", err) + } + f := findByPolicy(res.Findings, "Play Billing Library") + if f == nil { + t.Fatalf("an out-of-support billing version reached by reference was not detected") + } + if !strings.Contains(f.Title, tc.want) { + t.Errorf("title = %q, want it to carry version %s", f.Title, tc.want) + } + }) + } +} + +// A supported version reached by reference must stay clean. +func TestBillingVersionByReferenceSupportedIsClean(t *testing.T) { + root := writeProject(t, map[string]string{ + "app/build.gradle": `ext { billingVersion = "8.0.0" } +android { defaultConfig { targetSdk = 36 } } +dependencies { implementation "com.android.billingclient:billing:$billingVersion" }`, + }) + res, err := Scan(root) + if err != nil { + t.Fatalf("scan: %v", err) + } + if f := findByPolicy(res.Findings, "Play Billing Library"); f != nil { + t.Errorf("Billing 8 is supported, got %q", f.Title) + } +} diff --git a/internal/preflight/platform.go b/internal/preflight/platform.go index a3bc220..46d4db5 100644 --- a/internal/preflight/platform.go +++ b/internal/preflight/platform.go @@ -48,21 +48,29 @@ func detectIOS(root string) bool { return nil } + lower := strings.ToLower(name) switch { case strings.EqualFold(name, "Info.plist"), strings.EqualFold(name, "Podfile"), strings.EqualFold(name, "Package.swift"), strings.EqualFold(name, "project.pbxproj"), strings.HasSuffix(name, ".xcprivacy"), - strings.HasSuffix(strings.ToLower(name), ".swift"), - strings.HasSuffix(strings.ToLower(name), ".xcodeproj"): + strings.HasSuffix(lower, ".swift"), + // Objective-C sources: codescan reads these for private-API use, so + // missing them would silently drop real Apple findings. + strings.HasSuffix(lower, ".m"), + strings.HasSuffix(lower, ".mm"), + strings.HasSuffix(lower, ".h"), + strings.HasSuffix(lower, ".xcodeproj"): found = true return filepath.SkipAll } - // An Expo / React Native config with an ios section ships to the App - // Store even when the repo holds no native iOS sources. - if strings.EqualFold(name, "app.json") || strings.EqualFold(name, "app.config.js") || strings.EqualFold(name, "app.config.ts") { + // An Expo / React Native config ships to the App Store even when the + // repo holds no native iOS sources. All four spellings count; missing + // one classifies a cross-platform app as Android-only. + switch lower { + case "app.json", "app.config.js", "app.config.ts", "app.config.json": found = true return filepath.SkipAll } diff --git a/internal/preflight/runner_test.go b/internal/preflight/runner_test.go index 26ca3cf..f923fc9 100644 --- a/internal/preflight/runner_test.go +++ b/internal/preflight/runner_test.go @@ -173,3 +173,55 @@ func TestCrossPlatformProjectRunsBothScanners(t *testing.T) { t.Error("no Apple findings on a cross-platform project") } } + +// detectIOS decides whether the Apple scanners run at all, so a missed signal +// silently drops real findings. Each entry here is a file that codescan or the +// metadata scanner actually reads. +func TestDetectIOSRecognizesAllAppleSignals(t *testing.T) { + signals := []string{ + "App.swift", + "Legacy.m", + "Legacy.mm", + "Header.h", + "MyApp/Info.plist", + "Podfile", + "Package.swift", + "MyApp.xcodeproj/project.pbxproj", + "app.json", + "app.config.js", + "app.config.ts", + "app.config.json", + } + for _, signal := range signals { + t.Run(signal, func(t *testing.T) { + root := t.TempDir() + // Paired with a real Android project, so a missed iOS signal would + // classify the repo Android-only and skip the Apple scanners. + mustWrite(t, root, "android/app/build.gradle", "android { defaultConfig { targetSdk = 36 } }") + mustWrite(t, root, "android/app/src/main/AndroidManifest.xml", + ``) + mustWrite(t, root, signal, "{}") + + ios, android := DetectPlatforms(root) + if !android { + t.Fatal("android side not detected") + } + if !ios { + t.Errorf("%s did not mark the project as iOS, so Apple checks would be skipped", signal) + } + }) + } +} + +// A vendored dependency's Xcode project must not make an Android-only repo +// look like an iOS app. +func TestDetectIOSSkipsVendoredDirectories(t *testing.T) { + root := t.TempDir() + mustWrite(t, root, "android/app/build.gradle", "android { defaultConfig { targetSdk = 36 } }") + mustWrite(t, root, "node_modules/some-lib/ios/Thing.swift", "import Foundation") + mustWrite(t, root, "Pods/Other/Info.plist", "") + + if ios, _ := DetectPlatforms(root); ios { + t.Error("vendored iOS sources should not classify the repo as iOS") + } +}