Reduce false positives: Amplitude, placeholder, HTTP namespace URIs#1
Conversation
- Amplitude: require SDK-specific patterns (import, init, instance) instead of bare word match. The word "amplitude" is a common math/physics term (e.g. `let amplitude: CGFloat = 8`) and was triggering false positives on variable names and comments. - Placeholder: add ignore patterns for Swift/WidgetKit protocol method signatures like `func placeholder(in context:)` which is a required AppIntentTimelineProvider conformance, not placeholder content. - HTTP URLs: add w3.org, xmlns, DTD, and doctype to the ignore list. XML/SVG namespace URIs (e.g. `http://www.w3.org/2000/svg`) are spec-defined identifiers, not network requests, and are unaffected by App Transport Security.
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed everything up to d400171 in 6 seconds. Click for details.
- Reviewed
45lines of code in2files - Skipped
0files when reviewing. - Skipped posting
0draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
Workflow ID: wflow_mIeAdur2WsO2D7uj
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d4001713a4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| patterns: []*regexp.Regexp{ | ||
| regexp.MustCompile(`(?i)(firebase.*analytics|google.*analytics|facebook.*sdk|fbsdk|adjust.*sdk|appsflyer|amplitude|mixpanel)`), | ||
| regexp.MustCompile(`(?i)(firebase.*analytics|google.*analytics|facebook.*sdk|fbsdk|adjust.*sdk|appsflyer|mixpanel)`), | ||
| regexp.MustCompile(`(?i)(import\s+Amplitude|AmplitudeSwift|amplitude\.init|Amplitude\.instance|amplitude-js|@amplitude/)`), |
There was a problem hiding this comment.
Restore Objective-C Amplitude SDK matching
This pattern now only recognizes Swift/JS-specific forms (import Amplitude, Amplitude.instance, @amplitude/) and misses common Objective-C integrations such as #import <Amplitude/Amplitude.h> and [Amplitude instance]. In Objective-C apps, missing-att will no longer detect Amplitude usage, so the scanner can skip a real ATT compliance warning (5.1.2); this is a regression from the previous broader amplitude match, and the same narrowed pattern is also present in internal/privacy/scanner.go.
Useful? React with 👍 / 👎.
Summary
(?i)(amplitude)regex with SDK-specific patterns (import Amplitude,Amplitude.instance,@amplitude/, etc.). The word "amplitude" is a common math/physics term (e.g.let amplitude: CGFloat = 8for wave rendering) and was causing false positives on variable names and comments.func placeholder(in context:), which is a requiredAppIntentTimelineProviderconformance — not actual placeholder text.w3.org,xmlns,DTD, anddoctypeto the ignore list. XML/SVG namespace URIs likehttp://www.w3.org/2000/svgare spec-defined identifiers, not network requests, and are unaffected by App Transport Security.Context
All three false positives were encountered while running
greenlight preflighton a SwiftUI iOS app with WidgetKit extensions and embedded HTML5 games using inline SVG.Test plan
go build ./...passesgo test ./...passesimport Amplitude)func placeholder(in context:)no longer triggers, but"placeholder text"in strings still doeshttp://www.w3.org/2000/svgno longer triggers, buthttp://example-api.com/datastill doesImportant
Refines detection patterns for Amplitude SDK, placeholder content, and HTTP URLs to reduce false positives in code scanning.
(?i)(amplitude)regex with specific patterns likeimport Amplitude,Amplitude.instance,@amplitude/inrules.goandscanner.go.func placeholder(in context:)inrules.go.w3.org,xmlns,DTD, anddoctypeto ignore list inrules.gofor XML/SVG namespace URIs.This description was created by
for d400171. You can customize this summary. It will automatically update as commits are pushed.