diff --git a/README.md b/README.md index 2cf45c1..31b8a28 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,81 @@ 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 --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 +``` + +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. 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 + 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, 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 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** +- `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. + +**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 ```bash diff --git a/internal/cli/playscan.go b/internal/cli/playscan.go new file mode 100644 index 0000000..2d4f768 --- /dev/null +++ b/internal/cli/playscan.go @@ -0,0 +1,279 @@ +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 + playscanArchive string +) + +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 + +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: 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), + 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") + 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) +} + +func runPlayscan(cmd *cobra.Command, args []string) error { + path := "." + if len(args) > 0 { + path = args[0] + } + + // --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.") + if archive != "" { + fmt.Printf(" Archive: %s\n\n", archive) + } else { + fmt.Printf(" Project: %s\n\n", path) + } + } + + start := time.Now() + 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) + } + 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.ArchiveKind != "" { + fmt.Fprintf(w, " Format: %s (merged manifest)\n", result.ArchiveKind) + } + 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) + } + 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. + 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/archive.go b/internal/playscan/archive.go new file mode 100644 index 0000000..b04a02a --- /dev/null +++ b/internal/playscan/archive.go @@ -0,0 +1,338 @@ +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, + Findings: []Finding{}, + } + + 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..92cfaae --- /dev/null +++ b/internal/playscan/archive_test.go @@ -0,0 +1,721 @@ +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) + } +} + +// --- 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 new file mode 100644 index 0000000..85393ea --- /dev/null +++ b/internal/playscan/axml.go @@ -0,0 +1,458 @@ +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) + + // 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 + 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/gradle.go b/internal/playscan/gradle.go new file mode 100644 index 0000000..882e0d5 --- /dev/null +++ b/internal/playscan/gradle.go @@ -0,0 +1,349 @@ +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 + 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 + 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})["']?`) + 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. + // + // 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 + + // 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 { + continue + } + rel := relTo(p) + for i, line := range strings.Split(string(data), "\n") { + lineNo := i + 1 + code := stripGradleComment(line) + if code == "" { + 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 { + 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.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 + } + } 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 == "" { + 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.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 + } + } + 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 +// (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..094b96d --- /dev/null +++ b/internal/playscan/manifest.go @@ -0,0 +1,198 @@ +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 +} + +// 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 new file mode 100644 index 0000000..402518f --- /dev/null +++ b/internal/playscan/protoxml.go @@ -0,0 +1,308 @@ +package playscan + +import ( + "encoding/binary" + "fmt" + "math" + "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 { String str = 2; Primitive prim = 7 } // used 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, 0); 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, depth int) error { + if depth > maxXMLDepth { + return fmt.Errorf("manifest nesting exceeds %d levels", maxXMLDepth) + } + 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, depth+1); 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 +} + +// 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 || out != "" { + return nil + } + 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) + } + 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 +} + +// 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 new file mode 100644 index 0000000..60ff815 --- /dev/null +++ b/internal/playscan/rules.go @@ -0,0 +1,609 @@ +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" + docPageSizes = "https://developer.android.com/guide/practices/page-sizes" +) + +// 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 + // 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{ + { + 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, + componentBound: true, + }, + { + 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, + 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"}, + 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)) + 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 { + 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) + + // 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 + } + // 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..f4ae698 --- /dev/null +++ b/internal/playscan/scanner.go @@ -0,0 +1,302 @@ +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) { + // 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 { + 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] + }) + + // 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], appModule), gradleScore(gradles[j], appModule) + if si != sj { + return si > sj + } + return gradles[i] < gradles[j] + }) + + 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. +// +// 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, 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. + 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 + // 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"): + score -= 5 + } + return score +} diff --git a/internal/playscan/scanner_test.go b/internal/playscan/scanner_test.go new file mode 100644 index 0000000..53a5a5e --- /dev/null +++ b/internal/playscan/scanner_test.go @@ -0,0 +1,1089 @@ +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) + } + 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) + } +} + +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) + } + }) + } +} + +// 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/playscan/types.go b/internal/playscan/types.go new file mode 100644 index 0000000..d2f55d9 --- /dev/null +++ b/internal/playscan/types.go @@ -0,0 +1,58 @@ +// 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 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 ( + 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..46d4db5 --- /dev/null +++ b/internal/preflight/platform.go @@ -0,0 +1,80 @@ +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 + } + + 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(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 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 + } + 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..f923fc9 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,184 @@ 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") + } +} + +// 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") + } +}