chore: add local testing extension - #1931
Conversation
size-limit report 📦
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Done
Or push these changes by commenting:
@cursor push 998428230a
Preview (998428230a)
diff --git a/test-server/configurator-extension/csp.js b/test-server/configurator-extension/csp.js
--- a/test-server/configurator-extension/csp.js
+++ b/test-server/configurator-extension/csp.js
@@ -42,7 +42,14 @@
// nobody's attention if session replay is off.
function requirementsFor({ analytics = {}, sessionReplay, engagement }) {
const zone = ZONES[analytics.serverZone] ?? ZONES.US;
- const events = analytics.serverUrl ? new URL(analytics.serverUrl).origin : zone.events;
+ let events = zone.events;
+ if (analytics.serverUrl) {
+ try {
+ events = new URL(analytics.serverUrl).origin;
+ } catch {
+ // Free-form configurator input; an invalid URL must not abort injection.
+ }
+ }
const requirements = [{ directive: 'connect-src', target: events, reason: 'event uploads' }];
if (fetchesRemoteConfig(analytics)) {
requirements.push({ directive: 'connect-src', target: zone.remoteConfig, reason: 'remote configuration' });
@@ -73,11 +80,14 @@
// Tabs whose rule is in place. Rehydrated from the rules themselves, since they are the state that
// survives the service worker being torn down between a run and the navigation it opened.
+// Await before registering onHeadersReceived so a waking navigation cannot race an empty set.
const relaxed = new Set();
-chrome.declarativeNetRequest
- .getSessionRules()
- .then((rules) => rules.forEach(({ id }) => relaxed.add(id)))
- .catch(() => undefined);
+try {
+ const rules = await chrome.declarativeNetRequest.getSessionRules();
+ rules.forEach(({ id }) => relaxed.add(id));
+} catch {
+ // Leave relaxed empty; rules can still be added via relaxCsp.
+}
// Only the policy of a tab under instrumentation is worth keeping, and only until the tab goes away.
const policies = new Map();
diff --git a/test-server/configurator-extension/sync-vendor.mjs b/test-server/configurator-extension/sync-vendor.mjs
--- a/test-server/configurator-extension/sync-vendor.mjs
+++ b/test-server/configurator-extension/sync-vendor.mjs
@@ -10,8 +10,9 @@
// Run from the repository root: node test-server/configurator-extension/sync-vendor.mjs
import { mkdir, readFile, writeFile } from 'node:fs/promises';
import path from 'node:path';
+import { fileURLToPath } from 'node:url';
-const HERE = path.dirname(new URL(import.meta.url).pathname);
+const HERE = path.dirname(fileURLToPath(import.meta.url));
const PACKAGES = path.resolve(HERE, '../../packages');
const BUNDLES = [You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 4126acd. Configure here.
| chrome.declarativeNetRequest | ||
| .getSessionRules() | ||
| .then((rules) => rules.forEach(({ id }) => relaxed.add(id))) | ||
| .catch(() => undefined); |
There was a problem hiding this comment.
CSP state races service worker wake
Medium Severity
relaxed is rehydrated with a fire-and-forget promise, so a waking onHeadersReceived can run while the set is still empty and skip recording the policy. CSP removal still works via session rules, but the console and toolbar report that the design depends on is lost after the service worker sleeps.
Reviewed by Cursor Bugbot for commit 4126acd. Configure here.
| import { mkdir, readFile, writeFile } from 'node:fs/promises'; | ||
| import path from 'node:path'; | ||
|
|
||
| const HERE = path.dirname(new URL(import.meta.url).pathname); |
There was a problem hiding this comment.
Vendor sync breaks on Windows paths
Medium Severity
HERE is derived from new URL(import.meta.url).pathname instead of fileURLToPath. On Windows that yields a leading-slash path such as /C:/..., and on any OS spaces or non-ASCII segments stay percent-encoded, so the vendor copy step fails before the extension can load bundles.
Reviewed by Cursor Bugbot for commit 4126acd. Configure here.
| } | ||
| // Read before injecting: by the time a navigation commits the response headers have arrived, which is | ||
| // where the policy the page was sent is still visible. | ||
| const csp = cspReport(tabId, payload); |
There was a problem hiding this comment.
Bad server URL aborts injection
Low Severity
cspReport runs outside the injection try/catch and calls new URL(analytics.serverUrl). An invalid custom server URL from the configurator throws before any script is injected, leaves the badge as on, and skips the error path that would mark err.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 4126acd. Configure here.
1 participant
Summary
Checklist
Note
Low Risk
Changes are confined to test-server dev tooling; the extension is explicitly a spike with broad host permissions intended for local use only.
Overview
Adds a local Chrome extension spike so configurator settings can be exercised on any http(s) site, not only the in-repo run page.
The configurator gains a Run on URL flow:
extension-bridge.jsposts the built runtime config (viabuildRuntimeConfig+toJsonSafefor regexes) to a content script onconfigurator.html, which opens the target tab and injects vendored analytics (and optional session replay) in the main world onwebNavigation.onCommitted. A toolbar action toggles instrumentation on the current tab using the last payload or a debug default.csp.jsstrips CSP headers per tab with declarativeNetRequest, restores them when instrumentation ends, and heuristically reports what the original policy would have blocked for Amplitude endpoints.inject.jsrevives config, logs events, and restores any pre-existingwindow.amplitude.sync-vendor.mjscopies built SDK bundles intovendor/with UTF-8 non-character escaping for Chromium. Includes README, manifest, and a strict-CSP target page for manual testing.Reviewed by Cursor Bugbot for commit 4126acd. Bugbot is set up for automated code reviews on this repo. Configure here.