From f4c52ceed6eebb3aa8d85c56811b318cf87ac21b Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Sat, 21 Oct 2023 15:02:57 +0200 Subject: [PATCH 1/3] fix: heroku subdomain check --- src/__tests__/utils.js | 13 +++++++++++++ src/posthog-core.ts | 3 ++- src/utils.ts | 10 ++++++++++ 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/__tests__/utils.js b/src/__tests__/utils.js index 57afc1d8b..00e85ba08 100644 --- a/src/__tests__/utils.js +++ b/src/__tests__/utils.js @@ -12,6 +12,7 @@ import { DEFAULT_BLOCKED_UA_STRS, loadScript, _isUrlMatchingRegex, + isCrossDomainCookie, } from '../utils' function userAgentFor(botString) { @@ -272,4 +273,16 @@ describe('loadScript', () => { expect(_isUrlMatchingRegex('https://example.com/something/test', 'example.com/(.*.)/test')).toEqual(true) }) }) + + describe('check for cross domain cookies', () => { + it.each([ + [false, 'https://test.herokuapp.com'], + // ensure it isn't matching herokuapp anywhere in the domain + [true, 'https://test.herokuapp.com.impersonator.io'], + [false, undefined], + [true, 'https://bbc.co.uk'], + ])('should return %s when hostname is %s', (expectedResult, hostname) => { + expect(isCrossDomainCookie(hostname)).toEqual(expectedResult) + }) + }) }) diff --git a/src/posthog-core.ts b/src/posthog-core.ts index 5cc9d94f7..18147ce1a 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -16,6 +16,7 @@ import { userAgent, window, logger, + isCrossDomainCookie, } from './utils' import { autocapture } from './autocapture' import { PostHogFeatureFlags } from './posthog-featureflags' @@ -109,7 +110,7 @@ const defaultConfig = (): PostHogConfig => ({ token: '', autocapture: true, rageclick: true, - cross_subdomain_cookie: document?.location?.hostname?.indexOf('herokuapp.com') === -1, + cross_subdomain_cookie: isCrossDomainCookie(document?.location?.hostname), persistence: 'cookie', persistence_name: '', cookie_name: '', diff --git a/src/utils.ts b/src/utils.ts index e9524cf74..06d6c50fa 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -955,4 +955,14 @@ export const _info = { }, } +export function isCrossDomainCookie(hostname: string | undefined) { + if (!_isString(hostname)) { + return false + } + // split and slice isn't a great way to match arbitrary domains, + // but it's good enough for ensuring we only match herokuapp.com when it is the TLD + // for the hostname + return hostname.split('.').slice(-2).join('.').indexOf('herokuapp.com') === -1 +} + export { win as window, userAgent, document } From fba26da9e17507671fc10be62b4bed2444af4fa7 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Mon, 23 Oct 2023 09:22:38 +0100 Subject: [PATCH 2/3] satisfy codeql like this? --- src/__tests__/utils.js | 2 +- src/posthog-core.ts | 2 +- src/utils.ts | 4 +++- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/__tests__/utils.js b/src/__tests__/utils.js index 00e85ba08..641f7920b 100644 --- a/src/__tests__/utils.js +++ b/src/__tests__/utils.js @@ -282,7 +282,7 @@ describe('loadScript', () => { [false, undefined], [true, 'https://bbc.co.uk'], ])('should return %s when hostname is %s', (expectedResult, hostname) => { - expect(isCrossDomainCookie(hostname)).toEqual(expectedResult) + expect(isCrossDomainCookie({ hostname })).toEqual(expectedResult) }) }) }) diff --git a/src/posthog-core.ts b/src/posthog-core.ts index 18147ce1a..5f5fa12d0 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -110,7 +110,7 @@ const defaultConfig = (): PostHogConfig => ({ token: '', autocapture: true, rageclick: true, - cross_subdomain_cookie: isCrossDomainCookie(document?.location?.hostname), + cross_subdomain_cookie: isCrossDomainCookie(document?.location), persistence: 'cookie', persistence_name: '', cookie_name: '', diff --git a/src/utils.ts b/src/utils.ts index 06d6c50fa..7cc651b96 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -955,7 +955,9 @@ export const _info = { }, } -export function isCrossDomainCookie(hostname: string | undefined) { +export function isCrossDomainCookie(documentLocation: Location | undefined) { + const hostname = documentLocation?.hostname + if (!_isString(hostname)) { return false } From ce0c5d3e304182c20f982923cee1b23f13670039 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Mon, 23 Oct 2023 09:23:33 +0100 Subject: [PATCH 3/3] more tests --- src/__tests__/utils.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/__tests__/utils.js b/src/__tests__/utils.js index 641f7920b..b9a3e2c24 100644 --- a/src/__tests__/utils.js +++ b/src/__tests__/utils.js @@ -277,10 +277,14 @@ describe('loadScript', () => { describe('check for cross domain cookies', () => { it.each([ [false, 'https://test.herokuapp.com'], + [false, 'test.herokuapp.com'], + [false, 'herokuapp.com'], // ensure it isn't matching herokuapp anywhere in the domain [true, 'https://test.herokuapp.com.impersonator.io'], [false, undefined], [true, 'https://bbc.co.uk'], + [true, 'bbc.co.uk'], + [true, 'www.bbc.co.uk'], ])('should return %s when hostname is %s', (expectedResult, hostname) => { expect(isCrossDomainCookie({ hostname })).toEqual(expectedResult) })