Skip to content

Commit 85989fb

Browse files
🔥 enforce allowedTrackingOrigin config when initializing the SDK from an extension (#3885)
1 parent f8d28fc commit 85989fb

File tree

7 files changed

+24
-65
lines changed

7 files changed

+24
-65
lines changed

‎developer-extension/src/content-scripts/main.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,11 @@ function overrideInitConfiguration(global: GlobalInstrumentation, configurationO
110110
if ('init' in sdkInstance) {
111111
const originalInit = sdkInstance.init
112112
sdkInstance.init = (config: any) => {
113-
originalInit({ ...config, ...configurationOverride })
113+
originalInit({
114+
...config,
115+
...configurationOverride,
116+
allowedTrackingOrigins: [location.origin],
117+
})
114118
}
115119
}
116120
})

‎packages/core/src/domain/allowedTrackingOrigins.spec.ts‎

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@ import { STACK_WITH_INIT_IN_EXTENSION, STACK_WITH_INIT_IN_PAGE } from '../../tes
22
import { display } from '../tools/display'
33
import {
44
isAllowedTrackingOrigins,
5-
WARN_DOES_NOT_HAVE_ALLOWED_TRACKING_ORIGIN,
65
ERROR_NOT_ALLOWED_TRACKING_ORIGIN,
6+
ERROR_DOES_NOT_HAVE_ALLOWED_TRACKING_ORIGIN,
77
} from './allowedTrackingOrigins'
88

99
const DEFAULT_CONFIG = {
@@ -13,17 +13,14 @@ const DEFAULT_CONFIG = {
1313
}
1414

1515
describe('checkForAllowedTrackingOrigins', () => {
16-
let displayWarnSpy: jasmine.Spy
1716
let displayErrorSpy: jasmine.Spy
1817

1918
beforeEach(() => {
20-
displayWarnSpy = spyOn(display, 'warn')
2119
displayErrorSpy = spyOn(display, 'error')
2220
})
2321

2422
it('should not warn if not in extension environment', () => {
2523
const result = isAllowedTrackingOrigins(DEFAULT_CONFIG, STACK_WITH_INIT_IN_PAGE, 'https://app.example.com')
26-
expect(displayWarnSpy).not.toHaveBeenCalled()
2724
expect(displayErrorSpy).not.toHaveBeenCalled()
2825
expect(result).toBe(true)
2926
})
@@ -38,7 +35,6 @@ describe('checkForAllowedTrackingOrigins', () => {
3835
STACK_WITH_INIT_IN_PAGE,
3936
'https://app.example.com'
4037
)
41-
expect(displayWarnSpy).not.toHaveBeenCalled()
4238
expect(displayErrorSpy).not.toHaveBeenCalled()
4339
expect(result).toBe(true)
4440
})
@@ -52,7 +48,6 @@ describe('checkForAllowedTrackingOrigins', () => {
5248
STACK_WITH_INIT_IN_PAGE,
5349
'https://app.example.com'
5450
)
55-
expect(displayWarnSpy).not.toHaveBeenCalled()
5651
expect(displayErrorSpy).not.toHaveBeenCalled()
5752
expect(result).toBe(true)
5853
})
@@ -66,7 +61,6 @@ describe('checkForAllowedTrackingOrigins', () => {
6661
STACK_WITH_INIT_IN_PAGE,
6762
'https://app.example.com'
6863
)
69-
expect(displayWarnSpy).not.toHaveBeenCalled()
7064
expect(displayErrorSpy).not.toHaveBeenCalled()
7165
expect(result).toBe(true)
7266
})
@@ -84,7 +78,6 @@ describe('checkForAllowedTrackingOrigins', () => {
8478
STACK_WITH_INIT_IN_PAGE,
8579
'https://app.example.com'
8680
)
87-
expect(displayWarnSpy).not.toHaveBeenCalled()
8881
expect(displayErrorSpy).not.toHaveBeenCalled()
8982
expect(result).toBe(true)
9083
})
@@ -175,7 +168,7 @@ describe('checkForAllowedTrackingOrigins', () => {
175168
})
176169

177170
describe('when configuration does not have allowedTrackingOrigins', () => {
178-
it('should warn when in extension environment and allowedTrackingOrigins is undefined', () => {
171+
it('should log an error when in extension environment and allowedTrackingOrigins is undefined', () => {
179172
const result = isAllowedTrackingOrigins(
180173
{
181174
...DEFAULT_CONFIG,
@@ -184,8 +177,8 @@ describe('checkForAllowedTrackingOrigins', () => {
184177
STACK_WITH_INIT_IN_EXTENSION,
185178
'https://example.com'
186179
)
187-
expect(displayWarnSpy).toHaveBeenCalledWith(WARN_DOES_NOT_HAVE_ALLOWED_TRACKING_ORIGIN)
188-
expect(result).toBe(true)
180+
expect(displayErrorSpy).toHaveBeenCalledWith(ERROR_DOES_NOT_HAVE_ALLOWED_TRACKING_ORIGIN)
181+
expect(result).toBe(false)
189182
})
190183

191184
it('should error when in extension environment and allowedTrackingOrigins is an empty array', () => {
@@ -210,7 +203,6 @@ describe('checkForAllowedTrackingOrigins', () => {
210203
STACK_WITH_INIT_IN_PAGE,
211204
'https://example.com'
212205
)
213-
expect(displayWarnSpy).not.toHaveBeenCalled()
214206
expect(displayErrorSpy).not.toHaveBeenCalled()
215207
expect(result).toBe(true)
216208
})

‎packages/core/src/domain/allowedTrackingOrigins.ts‎

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
11
import { display } from '../tools/display'
22
import { matchList } from '../tools/matchOption'
33
import type { InitConfiguration } from './configuration'
4-
import { extractExtensionUrlFromStack, isUnsupportedExtensionEnvironment } from './extension/extensionUtils'
5-
import { addTelemetryDebug } from './telemetry'
4+
import { isUnsupportedExtensionEnvironment } from './extension/extensionUtils'
65

7-
export const WARN_DOES_NOT_HAVE_ALLOWED_TRACKING_ORIGIN =
8-
'Running the Browser SDK in a Web extension content script is discouraged and will be forbidden in a future major release unless the `allowedTrackingOrigins` option is provided.'
6+
export const ERROR_DOES_NOT_HAVE_ALLOWED_TRACKING_ORIGIN =
7+
'Running the Browser SDK in a Web extension content script is forbidden unless the `allowedTrackingOrigins` option is provided.'
98
export const ERROR_NOT_ALLOWED_TRACKING_ORIGIN = 'SDK initialized on a non-allowed domain.'
109

1110
export function isAllowedTrackingOrigins(
@@ -16,14 +15,9 @@ export function isAllowedTrackingOrigins(
1615
const allowedTrackingOrigins = configuration.allowedTrackingOrigins
1716
if (!allowedTrackingOrigins) {
1817
if (isUnsupportedExtensionEnvironment(windowOrigin, errorStack)) {
19-
display.warn(WARN_DOES_NOT_HAVE_ALLOWED_TRACKING_ORIGIN)
18+
display.error(ERROR_DOES_NOT_HAVE_ALLOWED_TRACKING_ORIGIN)
2019

21-
const extensionUrl = extractExtensionUrlFromStack(errorStack)
22-
// monitor-until: 2026-01-01
23-
addTelemetryDebug(WARN_DOES_NOT_HAVE_ALLOWED_TRACKING_ORIGIN, {
24-
extensionUrl: extensionUrl || 'unknown',
25-
})
26-
// TODO(next major): make `allowedTrackingOrigins` required in unsupported extension environments
20+
return false
2721
}
2822
return true
2923
}

‎packages/core/src/domain/extension/extensionUtils.spec.ts‎

Lines changed: 1 addition & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,7 @@ import {
33
STACK_WITH_INIT_IN_EXTENSION_FIREFOX,
44
STACK_WITH_INIT_IN_PAGE,
55
} from '../../../test'
6-
import {
7-
containsExtensionUrl,
8-
EXTENSION_PREFIXES,
9-
extractExtensionUrlFromStack,
10-
isUnsupportedExtensionEnvironment,
11-
} from './extensionUtils'
6+
import { containsExtensionUrl, EXTENSION_PREFIXES, isUnsupportedExtensionEnvironment } from './extensionUtils'
127

138
describe('extensionUtils', () => {
149
describe('containsExtensionUrl', () => {
@@ -49,19 +44,4 @@ describe('extensionUtils', () => {
4944
expect(isUnsupportedExtensionEnvironment('https://example.com', STACK_WITH_INIT_IN_EXTENSION)).toBe(true)
5045
})
5146
})
52-
53-
describe('extract init caller', () => {
54-
it('should extract extension URL from stack trace', () => {
55-
const stack = `Error
56-
at foo (<anonymous>:549:44)
57-
at bar (<anonymous>:701:91)
58-
at e.init (chrome-extension://abcd/content-script-main.js:1:1009)`
59-
expect(extractExtensionUrlFromStack(stack)).toBe('chrome-extension://abcd')
60-
})
61-
62-
it('should return undefined when no extension URL found', () => {
63-
const stack = 'Error at https://example.com/script.js:10:15'
64-
expect(extractExtensionUrlFromStack(stack)).toBeUndefined()
65-
})
66-
})
6747
})

‎packages/core/src/domain/extension/extensionUtils.ts‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,3 @@ export function isUnsupportedExtensionEnvironment(windowLocation: string, stack:
2626

2727
return containsExtensionUrl(target)
2828
}
29-
30-
export function extractExtensionUrlFromStack(stack: string = ''): string | undefined {
31-
for (const prefix of EXTENSION_PREFIXES) {
32-
const match = stack.match(new RegExp(`${prefix}[^/]+`))
33-
34-
if (match) {
35-
return match[0]
36-
}
37-
}
38-
}

‎packages/core/src/index.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,4 +162,3 @@ export * from './domain/connectivity'
162162
export * from './tools/stackTrace/handlingStack'
163163
export * from './tools/abstractHooks'
164164
export * from './domain/tags'
165-
export { WARN_DOES_NOT_HAVE_ALLOWED_TRACKING_ORIGIN } from './domain/allowedTrackingOrigins'

‎test/e2e/scenario/browser-extensions/browserExtensions.scenario.ts‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@ import { test, expect } from '@playwright/test'
33
import type { BrowserLog } from '../../lib/framework'
44
import { createTest, createExtension, createCrossOriginScriptUrls, formatConfiguration } from '../../lib/framework'
55

6-
const WARNING_MESSAGE =
7-
'Datadog Browser SDK: Running the Browser SDK in a Web extension content script is discouraged and will be forbidden in a future major release unless the `allowedTrackingOrigins` option is provided.'
6+
const ERROR_DOES_NOT_HAVE_ALLOWED_TRACKING_ORIGIN =
7+
'Datadog Browser SDK: Running the Browser SDK in a Web extension content script is forbidden unless the `allowedTrackingOrigins` option is provided.'
88
const ERROR_MESSAGE = 'Datadog Browser SDK: SDK initialized on a non-allowed domain.'
99

1010
const BASE_PATH = path.join(process.cwd(), 'test/apps')
@@ -18,22 +18,22 @@ const isNotSdkLoadedMoreThanOnce = (log: BrowserLog) => !log.message.includes('S
1818
test.describe('browser extensions', () => {
1919
for (const name of EXTENSIONS) {
2020
test.describe(`with ${name} extension`, () => {
21-
createTest('should warn and start tracking when SDK is initialized in an unsupported environment')
21+
createTest('should not start tracking and log an error when SDK is initialized in an unsupported environment')
2222
.withExtension(createExtension(path.join(BASE_PATH, name)).withRum().withLogs())
2323
.run(async ({ withBrowserLogs, flushEvents, intakeRegistry }) => {
2424
await flushEvents()
2525

26-
expect(intakeRegistry.rumViewEvents).toHaveLength(1)
26+
expect(intakeRegistry.rumViewEvents).toHaveLength(0)
2727

2828
withBrowserLogs((logs) => {
2929
const filteredLogs = logs.filter(isNotSdkLoadedMoreThanOnce)
3030

31-
// Two warnings, one for RUM and one for LOGS SDK
31+
// Two errors, one for RUM and one for LOGS SDK
3232
expect(filteredLogs).toHaveLength(2)
3333
filteredLogs.forEach((log) => {
3434
expect(log).toMatchObject({
35-
level: 'warning',
36-
message: WARNING_MESSAGE,
35+
level: 'error',
36+
message: ERROR_DOES_NOT_HAVE_ALLOWED_TRACKING_ORIGIN,
3737
})
3838
})
3939
})
@@ -53,8 +53,8 @@ test.describe('browser extensions', () => {
5353
withBrowserLogs((logs) =>
5454
expect(logs).not.toContainEqual(
5555
expect.objectContaining({
56-
level: 'warning',
57-
message: WARNING_MESSAGE,
56+
level: 'error',
57+
message: ERROR_DOES_NOT_HAVE_ALLOWED_TRACKING_ORIGIN,
5858
})
5959
)
6060
)

0 commit comments

Comments
 (0)