-
Notifications
You must be signed in to change notification settings - Fork 0
fix(worker): make the Google Ads conversion tag env-driven instead of a dead placeholder #136
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
b5c87c5
aaf3afd
e46a81e
60379b7
32e765b
cc097aa
071505f
0dd0885
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,8 @@ | ||
| window.dataLayer = window.dataLayer || []; | ||
| function gtag(){dataLayer.push(arguments);} | ||
| gtag('js', new Date()); | ||
| gtag('config', 'AW-XXXXXXXXX'); | ||
| gtag('event', 'conversion', { | ||
| 'send_to': 'AW-XXXXXXXXX/YYYYYYYYYYYYYYYYYYY' | ||
| }); | ||
| // Google Ads conversion script. | ||
| // | ||
| // This file's real content is generated by the worker at request time from | ||
| // the GOOGLE_ADS_CONVERSION_ID and GOOGLE_ADS_CONVERSION_LABEL env values — | ||
| // see specs/003-wellness-clinic-launch/tracking-setup.md. A hardcoded tag | ||
| // with a placeholder id is dead by construction (it can never record a | ||
| // conversion), so without those env values this script stays a no-op and | ||
| // nothing fires. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1065,3 +1065,77 @@ test("retired API host frames the current offer as The Website Appraisal, not th | |
| assert.match(body.message, /free leak audit of high-ticket service homepages/, "retired API host message must state the current offer truth"); | ||
| assert.doesNotMatch(body.message, /self-serve Agent Desk/, "retired API host message must not point at the retired Agent Desk as the current offer"); | ||
| }); | ||
|
|
||
| // ---- Google Ads conversion tag (funnel measurement) ------------------------ | ||
| // The funnel's only conversion measurement was dead by construction: the | ||
| // brief-requested page hardcoded the AW-XXXXXXXXX placeholder, and the | ||
| // production CSP blocked googletagmanager.com, so the event could never | ||
| // record. The tag must therefore be generated by the worker from env values | ||
| // and only emitted on /brief-requested when both are configured; with either | ||
| // missing or malformed, the page ships with no tag at all. | ||
| const BRIEF_REQUESTED_HTML = '<!doctype html><html><head><title>brief</title></head><body>ok</body></html>'; | ||
|
|
||
| function adsEnv(overrides = {}) { | ||
| return { | ||
| ASSETS: { | ||
| fetch: async () => | ||
| new Response(BRIEF_REQUESTED_HTML, { | ||
| status: 200, | ||
| headers: { "Content-Type": "text/html; charset=utf-8" } | ||
| }) | ||
| }, | ||
| GOOGLE_ADS_CONVERSION_ID: "AW-1234567890", | ||
| GOOGLE_ADS_CONVERSION_LABEL: "AbCdEfGhIjKlMnOpQrSt", | ||
| ...overrides | ||
| }; | ||
| } | ||
|
|
||
| test("brief-requested ships no Google Ads tag while the conversion env is not configured", async () => { | ||
| const env = adsEnv({ GOOGLE_ADS_CONVERSION_ID: "", GOOGLE_ADS_CONVERSION_LABEL: "" }); | ||
| const res = await worker.fetch(new Request("https://tinystudio.io/brief-requested"), env); | ||
| assert.equal(res.status, 200); | ||
| const body = await res.text(); | ||
| assert.ok(!body.includes("googletagmanager.com"), "no tag may ship while unconfigured"); | ||
| const csp = res.headers.get("Content-Security-Policy") || ""; | ||
| assert.ok(!csp.includes("googletagmanager.com"), "CSP must stay strict while unconfigured"); | ||
| const js = await (await worker.fetch(new Request("https://tinystudio.io/brief-requested.js"), env)).text(); | ||
| assert.ok(!js.includes("gtag("), "no conversion event may fire while unconfigured"); | ||
| }); | ||
|
|
||
| test("worker injects the configured Google Ads conversion tag on /brief-requested only", async () => { | ||
| const env = adsEnv(); | ||
| const res = await worker.fetch(new Request("https://tinystudio.io/brief-requested"), env); | ||
| assert.equal(res.status, 200); | ||
| const body = await res.text(); | ||
| assert.ok( | ||
| body.includes('src="https://www.googletagmanager.com/gtag/js?id=AW-1234567890"'), | ||
| "gtag loader must be injected with the real conversion id" | ||
| ); | ||
| assert.ok(body.includes("</head>"), "injected loader must sit inside the head"); | ||
| const csp = res.headers.get("Content-Security-Policy") || ""; | ||
| assert.ok(csp.includes("https://www.googletagmanager.com"), "brief-requested CSP must allow the gtag script origin"); | ||
| assert.ok(csp.includes("googleads.g.doubleclick.net"), "brief-requested CSP must allow the conversion beacon"); | ||
|
|
||
| const js = await (await worker.fetch(new Request("https://tinystudio.io/brief-requested.js"), env)).text(); | ||
| assert.ok(js.includes("gtag('config', 'AW-1234567890')"), "generated script must configure the real conversion id"); | ||
| assert.ok(js.includes("AW-1234567890/AbCdEfGhIjKlMnOpQrSt"), "generated script must send the real conversion label"); | ||
|
|
||
| // Every other page keeps the strict CSP even when the tag is configured. | ||
| const other = await worker.fetch(new Request("https://tinystudio.io/pricing.html"), env); | ||
| const otherCsp = other.headers.get("Content-Security-Policy") || ""; | ||
| assert.ok(!otherCsp.includes("googletagmanager.com"), "other pages must keep the strict CSP"); | ||
| }); | ||
|
|
||
| test("worker refuses to emit a Google Ads tag for a partial or malformed conversion config", async () => { | ||
| const malformed = [ | ||
| adsEnv({ GOOGLE_ADS_CONVERSION_LABEL: "" }), | ||
| adsEnv({ GOOGLE_ADS_CONVERSION_ID: "AW-123" }), | ||
| adsEnv({ GOOGLE_ADS_CONVERSION_ID: "javascript:alert(1)" }) | ||
| ]; | ||
| for (const env of malformed) { | ||
| const res = await worker.fetch(new Request("https://tinystudio.io/brief-requested"), env); | ||
| const body = await res.text(); | ||
| assert.ok(!body.includes("googletagmanager.com"), "no tag may emit for a partial or malformed config"); | ||
| assert.equal(body, BRIEF_REQUESTED_HTML, "unconfigured page must pass through untouched"); | ||
| } | ||
|
Comment on lines
+1129
to
+1140
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win Add a malformed conversion-label fixture. The fixtures test a missing label, but none tests a non-empty invalid 🤖 Prompt for AI Agents |
||
| }); | ||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -66,8 +66,17 @@ I cannot create this without account access. Exact steps: | |||||||||
| - Click-through window: **30 days** | ||||||||||
| - Attribution: **Data-driven**, or last-click if data-driven is unavailable | ||||||||||
| 4. Tag setup → copy the **conversion ID** (`AW-…`) and the **conversion label** | ||||||||||
| 5. Paste both into `brief-requested.html`, replacing `AW-XXXXXXXXX` and the | ||||||||||
| `send_to` label. **Four placeholder occurrences — replace all of them.** | ||||||||||
| 5. Set both on the Worker so the tag is emitted at request time — no code | ||||||||||
| change, and no placeholder can ever ship as a dead conversion: | ||||||||||
| - `wrangler secret put GOOGLE_ADS_CONVERSION_ID` → the `AW-…` ID | ||||||||||
| - `wrangler secret put GOOGLE_ADS_CONVERSION_LABEL` → the conversion label | ||||||||||
| - (For `wrangler dev --remote`, put both in `.dev.vars` instead.) | ||||||||||
| The Worker validates both (`AW-` + digits; a 10+ character alphanumeric | ||||||||||
| label) and injects the gtag loader + conversion event into | ||||||||||
|
Comment on lines
+74
to
+75
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Match the documented label rule to The text says the label is “10+ character alphanumeric.” Proposed fix- The Worker validates both (`AW-` + digits; a 10+ character alphanumeric
- label) and injects the gtag loader + conversion event into
+ The Worker validates both (`AW-` + 6-15 digits; a 10-50 character label
+ containing letters, digits, `_`, or `-`) and injects the gtag loader + conversion event into📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||
| `/brief-requested` **only** when both are set and well-formed. With either | ||||||||||
| missing or malformed, the page ships with no tag at all — a dead tag is | ||||||||||
| never served. The CSP allowances for gtag are scoped to that one noindex | ||||||||||
| page's response; every other page keeps the strict CSP. | ||||||||||
|
|
||||||||||
| ## 4. GA4 + Ads link | ||||||||||
|
|
||||||||||
|
|
@@ -94,7 +103,7 @@ works is a day of data you cannot use. | |||||||||
| - [ ] `htmlRedirect()` in `src/worker.js` points at `/brief-requested` | ||||||||||
| - [ ] JS success path redirects to `/brief-requested` | ||||||||||
| - [ ] Conversion action created — Submit lead form, no value, count One, 30 days | ||||||||||
| - [ ] `AW-…` ID and label pasted into `brief-requested.html` (all four spots) | ||||||||||
| - [ ] `GOOGLE_ADS_CONVERSION_ID` and `GOOGLE_ADS_CONVERSION_LABEL` set on the Worker (`wrangler secret put`; `.dev.vars` for dev) | ||||||||||
| - [ ] GA4 linked to Ads | ||||||||||
| - [ ] Live test submission verified as a recorded conversion | ||||||||||
| - [ ] Negatives loaded, ads approved | ||||||||||
|
|
||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,6 +10,17 @@ const SECURITY_HEADERS = { | |
| "default-src 'self'; img-src 'self' data:; style-src 'self' https://fonts.googleapis.com; font-src 'self' https://fonts.gstatic.com; script-src 'self' https://static.cloudflareinsights.com; connect-src 'self' https://cloudflareinsights.com; base-uri 'self'; frame-ancestors 'none'; form-action 'self'" | ||
| }; | ||
|
|
||
| // Page-scoped CSP for /brief-requested ONLY when the Google Ads conversion | ||
| // tag is configured. gtag.js loads from googletagmanager.com and beacons to | ||
| // Google's measurement endpoints; the global CSP above blocks both, which | ||
| // made even a real conversion id dead on arrival. The allowances are scoped | ||
| // to this one noindex page's response so every other page keeps the strict | ||
| // CSP. Only reachable when GOOGLE_ADS_CONVERSION_ID / _LABEL are configured | ||
| // (see googleAdsConversion below); when they are not, the page ships with | ||
| // the strict CSP and no tag at all. | ||
| const GOOGLE_ADS_CSP = | ||
| "default-src 'self'; img-src 'self' data: https://www.googleadservices.com; style-src 'self' https://fonts.googleapis.com; font-src 'self' https://fonts.gstatic.com; script-src 'self' https://static.cloudflareinsights.com https://www.googletagmanager.com; connect-src 'self' https://cloudflareinsights.com https://www.googletagmanager.com https://googleads.g.doubleclick.net https://www.googleadservices.com https://www.google-analytics.com https://stats.g.doubleclick.net; base-uri 'self'; frame-ancestors 'none'; form-action 'self'"; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When gtag sends the conversion or view-through pixel as an image request to Useful? React with 👍 / 👎. |
||
|
|
||
| const PUBLIC_ASSET_PATHS = new Set([ | ||
| "/", | ||
| "/index.html", | ||
|
|
@@ -94,10 +105,10 @@ const WEEKLY_METRIC_LABELS = [ | |
| "Cash collected" | ||
| ]; | ||
|
|
||
| function withSecurityHeaders(response) { | ||
| function withSecurityHeaders(response, contentSecurityPolicy) { | ||
| const headers = new Headers(response.headers); | ||
| for (const [key, value] of Object.entries(SECURITY_HEADERS)) { | ||
| headers.set(key, value); | ||
| headers.set(key, key === "Content-Security-Policy" && contentSecurityPolicy ? contentSecurityPolicy : value); | ||
| } | ||
| return new Response(response.body, { | ||
| status: response.status, | ||
|
|
@@ -1316,6 +1327,41 @@ function isHtmlNavigation(request) { | |
| return (request.method === "GET" || request.method === "HEAD") && accept.includes("text/html"); | ||
| } | ||
|
|
||
| // ---- Google Ads conversion tag (funnel measurement) ----------------------- | ||
| // The funnel's only conversion measurement used to be dead by construction: | ||
| // brief-requested.html shipped a hardcoded gtag loader with a placeholder | ||
| // conversion id, and the production CSP blocked googletagmanager.com | ||
| // entirely, so the event could never record. The tag is now generated at | ||
| // request time from env values and only emitted on /brief-requested when | ||
| // BOTH are configured and well-formed; a partial or malformed config emits | ||
| // nothing rather than a dead tag. The strict patterns also mean the values | ||
| // are safe to interpolate into the generated script. | ||
| const GOOGLE_ADS_ID_PATTERN = /^AW-\d{6,15}$/; | ||
| const GOOGLE_ADS_LABEL_PATTERN = /^[A-Za-z0-9_-]{10,50}$/; | ||
|
|
||
| function googleAdsConversion(env) { | ||
| const id = String(env.GOOGLE_ADS_CONVERSION_ID || "").trim(); | ||
| const label = String(env.GOOGLE_ADS_CONVERSION_LABEL || "").trim(); | ||
| if (!GOOGLE_ADS_ID_PATTERN.test(id) || !GOOGLE_ADS_LABEL_PATTERN.test(label)) return null; | ||
| return { id, label }; | ||
| } | ||
|
|
||
| function googleAdsLoader({ id }) { | ||
| return `<!-- Google Ads conversion: injected by the worker from env (fires once, on this noindex page only) --> | ||
| <script async src="https://www.googletagmanager.com/gtag/js?id=${id}"></script>`; | ||
| } | ||
|
|
||
| function googleAdsScript({ id, label }) { | ||
| return `window.dataLayer = window.dataLayer || []; | ||
| function gtag(){dataLayer.push(arguments);} | ||
| gtag('js', new Date()); | ||
| gtag('config', '${id}'); | ||
| gtag('event', 'conversion', { | ||
| 'send_to': '${id}/${label}' | ||
| }); | ||
| `; | ||
| } | ||
|
|
||
| export default { | ||
| async fetch(request, env) { | ||
| const url = new URL(request.url); | ||
|
|
@@ -1342,6 +1388,36 @@ export default { | |
| } | ||
|
|
||
| if (PUBLIC_ASSET_PATHS.has(url.pathname)) { | ||
| const ads = googleAdsConversion(env); | ||
| const isBriefRequestedPage = | ||
| url.pathname === "/brief-requested" || url.pathname === "/brief-requested.html"; | ||
| const isBriefRequestedScript = url.pathname === "/brief-requested.js"; | ||
|
|
||
| if (ads && request.method === "GET" && (isBriefRequestedPage || isBriefRequestedScript)) { | ||
| if (isBriefRequestedScript) { | ||
| return withSecurityHeaders( | ||
| new Response(googleAdsScript(ads), { | ||
| headers: { "Content-Type": "text/javascript;charset=UTF-8" } | ||
| }) | ||
| ); | ||
| } | ||
| const assetResponse = await env.ASSETS.fetch(request); | ||
| if (assetResponse.ok) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When tracking is enabled after an unconfigured visit, the browser can revalidate its cached static Useful? React with 👍 / 👎. |
||
| const html = await assetResponse.text(); | ||
| const rewritten = html.includes("</head>") | ||
| ? html.replace("</head>", `${googleAdsLoader(ads)}\n</head>`) | ||
| : html; | ||
| return withSecurityHeaders( | ||
| new Response(rewritten, { | ||
| status: assetResponse.status, | ||
| statusText: assetResponse.statusText, | ||
| headers: { "Content-Type": "text/html; charset=utf-8" } | ||
| }), | ||
| GOOGLE_ADS_CSP | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| const assetResponse = await env.ASSETS.fetch(request); | ||
| return withSecurityHeaders(assetResponse); | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Scan all public assets for static tracking code.
Lines 637-643 inspect only two files under
public/. A hardcoded tag in another public asset will pass CI. The exactgtag(check also misses whitespace variants such asgtag (.Enumerate all files under
public/. Use syntax-tolerant patterns forgtagcalls anddataLayerreferences.🤖 Prompt for AI Agents