diff --git a/internal/privacy/scanner.go b/internal/privacy/scanner.go index 0fc7744..5641212 100644 --- a/internal/privacy/scanner.go +++ b/internal/privacy/scanner.go @@ -6,6 +6,7 @@ import ( "os" "path/filepath" "regexp" + "sort" "strings" ) @@ -93,6 +94,9 @@ var requiredReasonAPIs = []RequiredReasonAPI{ }, } +// attPattern matches an App Tracking Transparency implementation. +var attPattern = regexp.MustCompile(`(?i)(ATTrackingManager|requestTrackingAuthorization|AppTrackingTransparency|expo-tracking-transparency)`) + // Known tracking/advertising SDKs var trackingSDKPatterns = []struct { Pattern *regexp.Regexp @@ -138,6 +142,7 @@ func Scan(projectPath string) (*ScanResult, error) { // 2. Scan code for Required Reason API usage detectedAPIs := make(map[string][]FileHit) trackingSDKsFound := make(map[string]bool) + trackingSDKHits := make(map[string]FileHit) hasATT := false skipDirs := map[string]bool{ @@ -165,17 +170,32 @@ func Scan(projectPath string) (*ScanResult, error) { return nil } - fullContent := strings.Join(lines, "\n") + // Tracking SDKs and the ATT call are matched line by line, skipping comments. + // A commented-out reference is not an integration: counting one as a tracking + // SDK produces a CRITICAL for an app that does not track, and counting one as + // an ATT implementation hides a real CRITICAL from an app that does. + for lineNum, line := range lines { + if isCommentLine(line) { + continue + } - // Check for ATT implementation - if regexp.MustCompile(`(?i)(ATTrackingManager|requestTrackingAuthorization|AppTrackingTransparency|expo-tracking-transparency)`).MatchString(fullContent) { - hasATT = true - } + if attPattern.MatchString(line) { + hasATT = true + } - // Check for tracking SDKs - for _, sdk := range trackingSDKPatterns { - if sdk.Pattern.MatchString(fullContent) { + for _, sdk := range trackingSDKPatterns { + if !sdk.Pattern.MatchString(line) { + continue + } trackingSDKsFound[sdk.Name] = true + if _, seen := trackingSDKHits[sdk.Name]; !seen { + trackingSDKHits[sdk.Name] = FileHit{ + File: relPath, + Line: lineNum + 1, + Code: strings.TrimSpace(line), + API: sdk.Name, + } + } } } @@ -185,8 +205,7 @@ func Scan(projectPath string) (*ScanResult, error) { continue } for lineNum, line := range lines { - trimmed := strings.TrimSpace(line) - if strings.HasPrefix(trimmed, "//") || strings.HasPrefix(trimmed, "/*") || strings.HasPrefix(trimmed, "*") { + if isCommentLine(line) { continue } for _, p := range api.Patterns { @@ -242,16 +261,26 @@ func Scan(projectPath string) (*ScanResult, error) { for sdk := range trackingSDKsFound { result.TrackingSDKs = append(result.TrackingSDKs, sdk) } + // Map iteration is randomised, so sort for a stable report. + sort.Strings(result.TrackingSDKs) if len(trackingSDKsFound) > 0 && !hasATT { sdkList := strings.Join(result.TrackingSDKs, ", ") - result.Findings = append(result.Findings, Finding{ + finding := Finding{ Severity: "CRITICAL", Guideline: "5.1.2", Title: "Tracking SDKs detected without ATT implementation", Detail: "Found: " + sdkList + ". App Tracking Transparency prompt is required before any tracking.", Fix: "Import AppTrackingTransparency and call requestTrackingAuthorization() before initializing any tracking SDK.", - }) + } + // Cite where the first SDK was matched, so the finding can be checked + // against the source the way the Required Reason findings already can be. + if hit, ok := earliestHit(trackingSDKHits); ok { + finding.File = hit.File + finding.Line = hit.Line + finding.Detail += " First seen at " + hit.File + ":" + fmt.Sprint(hit.Line) + "." + } + result.Findings = append(result.Findings, finding) } // 5. Check if privacy manifest declares tracking but no tracking SDKs found @@ -335,6 +364,31 @@ func parsePrivacyManifest(content string) []string { return apis } +// earliestHit returns the hit that comes first in the project, ordered by file +// then line. Ranging over the map directly would pick an arbitrary SDK, so the +// cited location — and "first seen" — would change between runs. +func earliestHit(hits map[string]FileHit) (FileHit, bool) { + var earliest FileHit + found := false + for _, hit := range hits { + if !found || hit.File < earliest.File || (hit.File == earliest.File && hit.Line < earliest.Line) { + earliest = hit + found = true + } + } + return earliest, found +} + +// isCommentLine reports whether a source line is a single-line or block comment. +// It is deliberately conservative: it only skips lines that begin with a comment +// marker, so a trailing comment on a line of real code is still scanned. +func isCommentLine(line string) bool { + trimmed := strings.TrimSpace(line) + return strings.HasPrefix(trimmed, "//") || + strings.HasPrefix(trimmed, "/*") || + strings.HasPrefix(trimmed, "*") +} + func detectLang(path string) string { ext := strings.ToLower(filepath.Ext(path)) switch ext { diff --git a/internal/privacy/scanner_test.go b/internal/privacy/scanner_test.go index 1335846..1d0948b 100644 --- a/internal/privacy/scanner_test.go +++ b/internal/privacy/scanner_test.go @@ -1,8 +1,11 @@ package privacy import ( + "fmt" "os" "path/filepath" + "slices" + "strings" "testing" ) @@ -39,3 +42,142 @@ func TestRequiredReasonDetectsRealCallSites(t *testing.T) { } } } + +// Tracking SDKs and the ATT call used to be matched against the whole file with +// MatchString(fullContent), which reads commented-out code as if it were live. The +// Required Reason scan in the same walk already skipped comments; these two did not. +func TestTrackingScanIgnoresComments(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, "Notes.swift", strings.Join([]string{ + "// We deliberately do not use mixpanel here.", + "/* AppsFlyer was removed in v2.0 */", + " * and so was applovin", + "func body() { render() }", + }, "\n")+"\n") + + res, err := Scan(dir) + if err != nil { + t.Fatalf("Scan: %v", err) + } + if len(res.TrackingSDKs) != 0 { + t.Errorf("commented-out references counted as tracking SDKs: %v", res.TrackingSDKs) + } + for _, f := range res.Findings { + if f.Guideline == "5.1.2" { + t.Errorf("commented-out references produced a §5.1.2 finding: %+v", f) + } + } +} + +// The inverse, and the more dangerous direction: an ATT call that only exists in a +// comment used to suppress the CRITICAL for an app that really does track. +func TestCommentedOutATTDoesNotSuppressFinding(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, "Tracking.swift", strings.Join([]string{ + "import AppsFlyerLib", + "// TODO: call ATTrackingManager.requestTrackingAuthorization before this ships", + "func start() { AppsFlyerLib.shared().start() }", + }, "\n")+"\n") + + res, err := Scan(dir) + if err != nil { + t.Fatalf("Scan: %v", err) + } + + var found bool + for _, f := range res.Findings { + if f.Guideline == "5.1.2" { + found = true + } + } + if !found { + t.Errorf("a commented-out ATT call suppressed the §5.1.2 finding; findings=%+v", res.Findings) + } +} + +// A trailing comment must not hide the code in front of it. +func TestTrackingScanStillReadsCodeWithTrailingComment(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, "A.swift", "import AppsFlyerLib // analytics\n") + + res, err := Scan(dir) + if err != nil { + t.Fatalf("Scan: %v", err) + } + if len(res.TrackingSDKs) == 0 { + t.Error("real SDK on a line with a trailing comment was not detected") + } +} + +// The §5.1.2 finding is the only CRITICAL that carried no location, which is exactly +// what makes a false positive hard to disprove. Finding already has File and Line. +func TestTrackingFindingCitesFileAndLine(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, "Analytics.swift", strings.Join([]string{ + "import Foundation", + "import AppsFlyerLib", + "func boot() {}", + }, "\n")+"\n") + + res, err := Scan(dir) + if err != nil { + t.Fatalf("Scan: %v", err) + } + + for _, f := range res.Findings { + if f.Guideline != "5.1.2" { + continue + } + if f.File != "Analytics.swift" { + t.Errorf("File = %q, want %q", f.File, "Analytics.swift") + } + if f.Line != 2 { + t.Errorf("Line = %d, want 2", f.Line) + } + return + } + t.Fatalf("no §5.1.2 finding produced; findings=%+v", res.Findings) +} + +// TrackingSDKs is built by ranging over a map, and Go randomises map iteration, so +// both the reported SDK list and the cited location used to change between runs on +// a project using more than one SDK. The list is sorted, and the citation is the +// genuinely earliest hit by file then line rather than whichever key came out first. +func TestTrackingFindingIsDeterministic(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, "A.swift", "import AppsFlyerLib\n") + writeFile(t, dir, "B.swift", "import Mixpanel\n") + writeFile(t, dir, "C.swift", "import AppLovinSDK\n") + + var first string + for i := 0; i < 25; i++ { + res, err := Scan(dir) + if err != nil { + t.Fatalf("Scan: %v", err) + } + + if want := []string{"AppLovin", "AppsFlyer", "Mixpanel"}; !slices.Equal(res.TrackingSDKs, want) { + t.Fatalf("TrackingSDKs = %v, want %v (sorted)", res.TrackingSDKs, want) + } + + var got string + for _, f := range res.Findings { + if f.Guideline == "5.1.2" { + got = fmt.Sprintf("%s:%d|%s", f.File, f.Line, f.Detail) + } + } + if got == "" { + t.Fatalf("no §5.1.2 finding; findings=%+v", res.Findings) + } + if i == 0 { + first = got + if !strings.HasPrefix(got, "A.swift:1|") { + t.Errorf("citation = %q, want the earliest hit A.swift:1", got) + } + continue + } + if got != first { + t.Fatalf("finding changed between runs:\n run 0: %s\n run %d: %s", first, i, got) + } + } +}