security(PER-8550): drop unused @amplitude production dependencies

[pranavz28]

What

Moves the two Amplitude packages out of production dependencies:

   "dependencies": {
-    "@amplitude/analytics-browser": "2.35.4",
-    "@amplitude/plugin-session-replay-browser": "1.25.16",
     "@percy/cli-command": "^1.31.10",
   ...
   "devDependencies": {
+    "@amplitude/analytics-browser": "2.35.4",
+    "@amplitude/plugin-session-replay-browser": "1.25.16",
     "@babel/cli": "^7.23.0",

package.json is the only file changed. yarn.lock is untouched — a yarn v1 lockfile does not record the dev/prod distinction, and the version specifiers are unchanged, so yarn install --frozen-lockfile still validates against the existing lockfile.

Effect: consumers installing @percy/storybook no longer pull 22 @amplitude/* packages (~48 MB) into node_modules.

Correcting the ticket's threat model

PER-8635 describes a "credential exfiltration chain (Combined CVSS 8.4)" in which Amplitude session replay captures the BrowserStack access key from the connect form. That chain does not exist in the shipped artifact, but the reasoning needs correcting in both directions — the ticket overstates the risk, and the obvious "these are unused, just delete them" reading is also wrong.

The packages are not unreferenced

git grep -i amplitude across src/ returns zero hits — this repo's own code never imports, initializes, or configures Amplitude. But they are not stray, either. @browserstack/utils@5.10.0 declares both as peerDependencies, and @browserstack/utils is pulled in transitively by @browserstack/design-stack and @browserstack/review-viewer. Its barrel entry point contains real side-effect imports:

// node_modules/@browserstack/utils/dist/index.js
import "./amplitude.js";
import "@amplitude/analytics-browser";
import "@amplitude/plugin-session-replay-browser";

dist/amplitude.js there does contain genuine init / sessionReplayPlugin / setUserId code. So the two entries in package.json were satisfying a peer dependency, not sitting unused.

Deleting them outright is therefore not viable — it breaks the build:

[vite]: Rollup failed to resolve import "@amplitude/analytics-browser"
from "node_modules/@browserstack/utils/dist/index.js"

Hence the move to devDependencies rather than a deletion: the specifier must resolve at bundle time, but nothing needs it at consumer install time.

Why no Amplitude code reaches the browser

@browserstack/utils sets "sideEffects": false, so Rollup tree-shakes those side-effect-only imports away — nothing in this addon references any Amplitude export. Rollup must still resolve the specifier before it can shake it, which is exactly why the build fails when the package is absent but the output stays clean when it is present.

Verified empirically — grep -ri amplitude dist/ returns zero hits across all 56 emitted files, both before and after this change. The bundle is also byte-for-byte identical before and after (sha256 over all of dist/), confirming the change is a pure packaging move with no runtime effect.

So there is no Amplitude initialization, no session replay recorder, and no analytics network path in the distributed dist/manager.js. The ticket's claim that the packages "ride along in the bundle and can be activated in a consumer's browser context" is not supported.

Does moving to devDependencies break consumers? No — verified end-to-end

Reasonable reviewer concern: @browserstack/utils declares the two Amplitude packages as non-optional peerDependencies, so if a consumer received @browserstack/utils but not Amplitude, resolution could fail. Checked all three links in that chain.

1. Are the peers declared optional? No. @browserstack/utils@5.10.0 has no peerDependenciesMeta block at all, so none of its peers are optional:

"peerDependencies": {
  "@amplitude/analytics-browser": "2.11.7",
  "@amplitude/plugin-session-replay-browser": "1.12.1",
  "axios": "1.12.0", "react": "18.3.1", "react-dom": "18.3.1", "lodash": "4.17.21"
}
// peerDependenciesMeta: undefined

2. Does a consumer ever receive @browserstack/utils? No — and this is what makes the peer question moot. @browserstack/utils is a devDependency of @percy/storybook, as are the @browserstack/design-stack and @browserstack/review-viewer packages that pull it in. The published dependencies are only:

"@percy/cli-command", "@percy/config", "axios",
"cross-spawn", "glob-to-regexp", "qs", "react-router-dom"

No @browserstack/* package is a production dependency, so no consumer install ever creates a peer relationship for Amplitude to satisfy.

3. Is it bundled or resolved at runtime? Bundled. Vite inlines @browserstack/utils (and design-stack / review-viewer) into the emitted chunks. Grepping the packed tarball, zero @browserstack specifiers survive in any published code file — the only matches anywhere are the devDependencies entries in package.json, which npm ignores for consumers. The complete set of bare (non-relative) imports in the shipped browser chunks is require("util") (a Node builtin), plus the entry point:

// dist/manager.js — the entry point
import "./chunks/manager-BWgDX38l.js";
import "react";                    // declared peerDependency of @percy/storybook
import "storybook/manager-api";    // provided by the consumer's Storybook

Note also that package.json files excludes dist/components and dist/hooks — the Babel output that does carry bare @browserstack/design-stack imports is never published. Only the self-contained Vite chunks and the server-side files ship.

Clean-room consumer install. Packed the built package with npm pack and installed the tarball into an empty throwaway project:

Check	Result
npm install <tarball>	exit 0, 183 packages added, no errors
Unmet-peer-dependency warnings	none
npm ls @percy/storybook	clean, no UNMET PEER DEPENDENCY markers
@amplitude/* in consumer tree	absent (0 packages)
@browserstack/* in consumer tree	absent (0 packages)
@browserstack refs in installed @percy/storybook code	none

So the peer requirement binds only at build time in this repo, where Amplitude is still installed as a devDependency — and yarn build passes with no peer warnings.

Pre-existing peer-version mismatch (unchanged by this PR, but relevant here)

@browserstack/utils peer-wants exact @amplitude/analytics-browser@2.11.7 and @amplitude/plugin-session-replay-browser@1.12.1, while this repo pins 2.35.4 and 1.25.16. That mismatch predates this PR — the versions are carried over untouched. Yarn v1 does not enforce peer ranges (and emits no warning here), and since the Amplitude code is tree-shaken out entirely, the divergence has no effect on the output. Flagging it for whoever evaluates the peer question, since it means the declared peer contract was already not being honoured.

Recommended severity change

• PER-8635 should be downgraded from "credential exfiltration chain (Combined CVSS 8.4)" to "unused production dependency" — an install-footprint / supply-chain surface issue (22 extra packages in every consumer's node_modules, implying telemetry that does not exist), not an active exfiltration path.
• PER-8544 / PER-8545, the other legs of the chain, are already fixed and should be closed. Confirmed on master at c0b6dd4 (PR security: keep BrowserStack creds server-side + validate/gate privileged server channels (PER-8544, PER-8545) #1330): src/server/projectConfig.cjs no longer returns credentials to the browser, and carries the comment "Do NOT send the BrowserStack username/access key back to the browser." The PROJECT_CONFIG_LOADED payload now emits only credentialsValid, project, projectDetails, hasValidToken, and lastBuild. No change was made to that file here.

Deliberately not changed

• The show/hide access-key toggle in src/components/BrowserStackConnect.jsx (type={showAccessKey ? 'text' : 'password'}) is left as-is. The ticket flags it as an exposure vector only because it assumed a session replay recorder was capturing the DOM. With no recorder running, it is a conventional UX affordance that every major product ships, and removing it would be a usability regression bought with no security gain.
• No maskSelector / privacySelector configuration was added — there is no Amplitude initialization in this codebase to configure.
• src/server/projectConfig.cjs is untouched (already fixed by security: keep BrowserStack creds server-side + validate/gate privileged server channels (PER-8544, PER-8545) #1330, see above).

Validation

Check	Result
@amplitude in package.json dependencies	0
yarn install --frozen-lockfile	passes against unmodified yarn.lock
yarn list --production amplitude packages	0 (was 22)
yarn build	passes, incl. assert-prod-bundle guard
dist/ bundle hashes vs. pre-change	byte-for-byte identical
grep -ri amplitude dist/	0 hits (before and after)
yarn lint	clean
git diff scope	package.json only

Tests: yarn test runs 405 specs with 42 failures — but these are pre-existing and environmental, not caused by this change. Every failure traces to a Chromium download 404 on this Apple Silicon machine (storage.googleapis.com/chromium-browser-snapshots/Mac_Arm/1300314/chrome-mac.zip). Running the identical suite against unmodified master produces exactly the same result (405 specs, 42 failed, same 102 Chromium-404 log lines), so the suite is unaffected. CI should be treated as the authority here.

🤖 Generated with Claude Code