fix(attribution): ignore shared/NAT IPs in fingerprint matching + CDN client-IP header#26
Merged
Merged
Conversation
…port CDN client-IP header Two root-cause fixes for cross-tenant install mis-attribution: 1. Shared-IP filter: calculateConfidenceScore no longer awards the IP score when either IP is in a shared/non-routable range (CGNAT 100.64.0.0/10, RFC1918, loopback, link-local, IPv6 ULA/link-local). Those IPs don't identify a single device, so unrelated users behind the same NAT could collide on IP (40pts) + UA (30pts) = the 70 threshold and cross orgs. Without the IP score such installs top out at 60 and no longer match. New exported isAttributableIp() with tests. 2. getClientIp honors a configurable authoritative client-IP header (TRUSTED_CLIENT_IP_HEADER, e.g. cf-connecting-ip) so installs behind Cloudflare are fingerprinted on the real device IP instead of a CDN/NAT hop. Opt-in; default behavior unchanged. Existing /24-match tests repointed from private to public IPs (their intent was to verify /24 matching, which now correctly requires routable IPs). Full suite green (125 tests).
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Cross-tenant install mis-attribution: coarse, org-blind fingerprint matching attributed one app's installs to a different tenant's link. Because the events page scopes in-app events by the matched link's org, that surfaced one app's in-app events under another workspace's feed (observed: a baby-store app's events under an unrelated customer's link, all from shared
100.64.0.xCGNAT IPs).Changes
Shared-IP filter —
calculateConfidenceScoreno longer awards the IP score when either IP is in a shared/non-routable range (CGNAT100.64.0.0/10, RFC1918, loopback, link-local, IPv6 ULA/link-local). Without the IP score such installs top out at 60 and no longer reach the 70 threshold. New exportedisAttributableIp().CDN client IP —
getClientIphonorsTRUSTED_CLIENT_IP_HEADER(e.g.cf-connecting-ip) so installs behind Cloudflare are fingerprinted on the real device IP, not a CDN/NAT hop. Opt-in; default behavior unchanged.Compatibility / risk