From 08e4553001da146f1d80a9b620aef0ef0db04bd4 Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Thu, 31 Oct 2024 13:53:27 -0500 Subject: [PATCH] Signal improvements (#1178) --- .changeset/wise-coats-refuse.md | 7 + packages/signals/signals/README.md | 18 ++- packages/signals/signals/package.json | 3 +- .../__tests__/network-generator.test.ts | 20 +++ .../signal-generators/network-gen/helpers.ts | 10 +- .../network-gen/network-signals-filter.ts | 10 +- .../signals/src/core/signals/signals.ts | 3 - packages/signals/signals/src/index.umd.ts | 6 + .../standalone-playground/package.json | 3 +- .../pages/index-signals.html | 126 ++++++++++++++++++ yarn.lock | 1 + 11 files changed, 181 insertions(+), 26 deletions(-) create mode 100644 .changeset/wise-coats-refuse.md create mode 100644 playgrounds/standalone-playground/pages/index-signals.html diff --git a/.changeset/wise-coats-refuse.md b/.changeset/wise-coats-refuse.md new file mode 100644 index 000000000..c09155d1c --- /dev/null +++ b/.changeset/wise-coats-refuse.md @@ -0,0 +1,7 @@ +--- +'@segment/analytics-signals': patch +--- + +* Refactor disallowList logic to never allow api.segment.io +* Update README +* Export SignalsPlugin in umd as well as global diff --git a/packages/signals/signals/README.md b/packages/signals/signals/README.md index 25f19c69e..f63eeb0de 100644 --- a/packages/signals/signals/README.md +++ b/packages/signals/signals/README.md @@ -12,22 +12,20 @@ See: [settings.ts](src/types/settings.ts) My Website - - - - + - - + + + ``` diff --git a/packages/signals/signals/package.json b/packages/signals/signals/package.json index 38697a1c7..5b72bc8b1 100644 --- a/packages/signals/signals/package.json +++ b/packages/signals/signals/package.json @@ -31,10 +31,9 @@ "build:esm": "yarn tsc -p tsconfig.build.json", "build:cjs": "yarn tsc -p tsconfig.build.json --outDir ./dist/cjs --module commonjs", "build:bundle": "NODE_ENV=production yarn run webpack", - "build:bundle-dev": "NODE_ENV=development yarn run webpack", "workerbox": "node scripts/build-workerbox.js", "assert-workerbox-built": "sh scripts/assert-workerbox-built.sh", - "watch": "yarn concurrently 'yarn build:bundle-dev --watch' 'yarn build:esm --watch'", + "watch": "yarn concurrently 'yarn build:bundle --watch' 'yarn build:esm --watch'", "version": "sh scripts/version.sh", "watch:test": "yarn test --watch", "tsc": "yarn run -T tsc", diff --git a/packages/signals/signals/src/core/signal-generators/network-gen/__tests__/network-generator.test.ts b/packages/signals/signals/src/core/signal-generators/network-gen/__tests__/network-generator.test.ts index 8975a12b6..36815f63b 100644 --- a/packages/signals/signals/src/core/signal-generators/network-gen/__tests__/network-generator.test.ts +++ b/packages/signals/signals/src/core/signal-generators/network-gen/__tests__/network-generator.test.ts @@ -323,4 +323,24 @@ describe(NetworkGenerator, () => { expect(mockEmitter.emit.mock.calls).toEqual([]) unregister() }) + + it('always disallows segment api network signals', async () => { + const mockEmitter = { emit: jest.fn() } + const networkGenerator = new TestNetworkGenerator({ + networkSignalsAllowList: ['.*'], + }) + const unregister = networkGenerator.register( + mockEmitter as unknown as SignalEmitter + ) + + await window.fetch(`https://api.segment.io`, { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ key: 'value' }), + }) + + await sleep(100) + expect(mockEmitter.emit.mock.calls).toEqual([]) + unregister() + }) }) diff --git a/packages/signals/signals/src/core/signal-generators/network-gen/helpers.ts b/packages/signals/signals/src/core/signal-generators/network-gen/helpers.ts index 18428ef35..d3319513d 100644 --- a/packages/signals/signals/src/core/signal-generators/network-gen/helpers.ts +++ b/packages/signals/signals/src/core/signal-generators/network-gen/helpers.ts @@ -43,15 +43,7 @@ export const containsContentType = ( return false } const normalizedHeaders = normalizeHeaders(headers) - - // format the content-type header to remove charset -- this is non-standard behavior that is somewhat common - // e.g. application/json;charset=utf-8 => application/json - const removeCharset = (header: string | null): string | null => - header?.split(';')[0].trim() ?? null - - return match.some((t) => - removeCharset(normalizedHeaders.get('content-type'))?.includes(t) - ) + return match.some((t) => normalizedHeaders.get('content-type')?.includes(t)) } export const containsJSONContentType = ( diff --git a/packages/signals/signals/src/core/signal-generators/network-gen/network-signals-filter.ts b/packages/signals/signals/src/core/signal-generators/network-gen/network-signals-filter.ts index 925d2dd6b..c71ea59c6 100644 --- a/packages/signals/signals/src/core/signal-generators/network-gen/network-signals-filter.ts +++ b/packages/signals/signals/src/core/signal-generators/network-gen/network-signals-filter.ts @@ -50,6 +50,7 @@ class NetworkFilterListItem { export class NetworkSignalsFilterList { public allowed: NetworkFilterListItem public disallowed: NetworkFilterListItem + private disallowedDefaults: NetworkFilterListItem constructor( allowList: RegexLike[] | undefined, @@ -57,10 +58,16 @@ export class NetworkSignalsFilterList { ) { this.allowed = new NetworkFilterListItem(allowList || []) this.disallowed = new NetworkFilterListItem(disallowList || []) + this.disallowedDefaults = new NetworkFilterListItem([ + 'api.segment.io', + 'signals.segment.io', + 'cdn.segment.com', + ]) } isAllowed(url: string): boolean { - const disallowed = this.disallowed.test(url) + const disallowed = + this.disallowed.test(url) || this.disallowedDefaults.test(url) const allowed = this.allowed.test(url) return allowed && !disallowed } @@ -85,6 +92,7 @@ export class NetworkSignalsFilter { isAllowed(url: string): boolean { const { networkSignalsFilterList, networkSignalsAllowSameDomain } = this.settings + const passesNetworkFilter = networkSignalsFilterList.isAllowed(url) const allowedBecauseSameDomain = networkSignalsAllowSameDomain && isSameDomain(url) diff --git a/packages/signals/signals/src/core/signals/signals.ts b/packages/signals/signals/src/core/signals/signals.ts index 8dcb7e424..207ace283 100644 --- a/packages/signals/signals/src/core/signals/signals.ts +++ b/packages/signals/signals/src/core/signals/signals.ts @@ -91,9 +91,6 @@ export class Signals implements ISignals { disallowListURLs: [ analyticsService.instance.settings.apiHost, analyticsService.instance.settings.cdnURL, - 'api.segment.io', - 'signals.segment.io', - 'cdn.segment.com', ], sampleRate: analyticsService.instance.settings.cdnSettings diff --git a/packages/signals/signals/src/index.umd.ts b/packages/signals/signals/src/index.umd.ts index 388d009a9..a3047fae1 100644 --- a/packages/signals/signals/src/index.umd.ts +++ b/packages/signals/signals/src/index.umd.ts @@ -3,3 +3,9 @@ */ import { SignalsPlugin } from './index' export { SignalsPlugin } // in case someone wants to use the umd module directly + +// this will almost certainly be executed in the browser, but since this is UMD, +// we are checking just for the sake of being thorough +if (typeof window !== 'undefined') { + ;(window as any).SignalsPlugin = SignalsPlugin +} diff --git a/playgrounds/standalone-playground/package.json b/playgrounds/standalone-playground/package.json index 4cb22b5d5..140c87124 100644 --- a/playgrounds/standalone-playground/package.json +++ b/playgrounds/standalone-playground/package.json @@ -12,7 +12,8 @@ }, "dependencies": { "@segment/analytics-consent-wrapper-onetrust": "workspace:^", - "@segment/analytics-next": "workspace:^" + "@segment/analytics-next": "workspace:^", + "@segment/analytics-signals": "workspace:^" }, "devDependencies": { "http-server": "14.1.1" diff --git a/playgrounds/standalone-playground/pages/index-signals.html b/playgrounds/standalone-playground/pages/index-signals.html new file mode 100644 index 000000000..48c05d39e --- /dev/null +++ b/playgrounds/standalone-playground/pages/index-signals.html @@ -0,0 +1,126 @@ + + + + + +
+ + +
+ + + + + + +
+ +
+ + +
+
+ + 

+  

+
+  
+
+
+
\ No newline at end of file
diff --git a/yarn.lock b/yarn.lock
index ee1fc0ef5..2e78059ec 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -3948,6 +3948,7 @@ __metadata:
   dependencies:
     "@segment/analytics-consent-wrapper-onetrust": "workspace:^"
     "@segment/analytics-next": "workspace:^"
+    "@segment/analytics-signals": "workspace:^"
     http-server: 14.1.1
   languageName: unknown
   linkType: soft