Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| /** Google ad-serving domains whose URLs should be proxied (exact match). */ | ||
| const GPT_DOMAINS = [ | ||
| 'securepubads.g.doubleclick.net', |
Check failure
Code scanning / CodeQL
Incomplete regular expression for hostnames
Copilot Autofix
AI about 2 hours ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
| /** Google ad-serving domains whose URLs should be proxied (exact match). */ | ||
| const GPT_DOMAINS = [ | ||
| 'securepubads.g.doubleclick.net', | ||
| 'pagead2.googlesyndication.com', |
Check failure
Code scanning / CodeQL
Incomplete regular expression for hostnames
Copilot Autofix
AI about 2 hours ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
| const GPT_DOMAINS = [ | ||
| 'securepubads.g.doubleclick.net', | ||
| 'pagead2.googlesyndication.com', | ||
| 'tpc.googlesyndication.com', |
Check failure
Code scanning / CodeQL
Incomplete regular expression for hostnames
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 days ago
In general, to avoid incomplete hostname regular expressions, any string used to build a regex should be run through a generic “escape for regex literal” function that escapes all regex metacharacters, not only dots. This ensures future additions to GPT_DOMAINS cannot accidentally introduce patterns that match more than the literal hostname.
Concretely, in crates/js/lib/src/integrations/gpt/script_guard.ts, the fallback block in rewriteUrl currently builds the regex with:
new RegExp(`https?://(?:www\\.)?${domain.replace(/\./g, '\\.')}`, 'i')This manually escapes only dots in domain. Replace this with a helper escapeRegex (defined in this file) that escapes every regex metacharacter: \ ^ $ * + ? . ( ) | { } [ ]. Use that helper both for domain and for any other future regex constructions based on literal strings if needed. The change is localized to this file: add the helper function (near the top, after constants or helpers) and change the new RegExp(...) call to use escapeRegex(domain) instead of domain.replace(/\./g, '\\.'). No behavior changes for current values, but it becomes robust and satisfies the security rule.
| @@ -54,6 +54,13 @@ | ||
| /** Integration route prefix on the first-party domain. */ | ||
| const PROXY_PREFIX = '/integrations/gpt'; | ||
|
|
||
| /** | ||
| * Escape a string so it can be safely used inside a RegExp literal. | ||
| */ | ||
| function escapeRegex(value: string): string { | ||
| return value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); | ||
| } | ||
|
|
||
| // --------------------------------------------------------------------------- | ||
| // URL matching and rewriting | ||
| // --------------------------------------------------------------------------- | ||
| @@ -98,7 +105,7 @@ | ||
| if (lower.includes(domain)) { | ||
| const prefix = hostPrefixForDomain(domain); | ||
| return originalUrl.replace( | ||
| new RegExp(`https?://(?:www\\.)?${domain.replace(/\./g, '\\.')}`, 'i'), | ||
| new RegExp(`https?://(?:www\\.)?${escapeRegex(domain)}`, 'i'), | ||
| `${window.location.protocol}//${window.location.host}${PROXY_PREFIX}${prefix}`, | ||
| ); | ||
| } |
| 'pagead2.googlesyndication.com', | ||
| 'tpc.googlesyndication.com', | ||
| 'googletagservices.com', | ||
| 'www.googletagservices.com', |
Check failure
Code scanning / CodeQL
Incomplete regular expression for hostnames
Copilot Autofix
AI about 2 hours ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
| 'tpc.googlesyndication.com', | ||
| 'googletagservices.com', | ||
| 'www.googletagservices.com', | ||
| 'cm.g.doubleclick.net', |
Check failure
Code scanning / CodeQL
Incomplete regular expression for hostnames
Copilot Autofix
AI about 2 hours ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
| 'cm.g.doubleclick.net', | ||
| 'ep1.adtrafficquality.google', | ||
| 'ep2.adtrafficquality.google', | ||
| 'www.googleadservices.com', |
Check failure
Code scanning / CodeQL
Incomplete regular expression for hostnames
Copilot Autofix
AI about 2 hours ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
ChristianPavilonis
force-pushed
the
feature/gpt-integration
branch
from
b6ec286 to
b811668
Compare
|
We should consider the scope of this integration. Currently this is doing a lot of rewriting and proxying it's not catching everything but many scripts and 3rd party calls are proxied through the 1st party context. |
ChristianPavilonis
force-pushed
the
feature/gpt-integration
branch
from
a800ad7 to
264f64f
Compare
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
ChristianPavilonis
force-pushed
the
feature/gpt-integration
branch
from
264f64f to
597bef9
Compare
| .with_header( | ||
| header::CACHE_CONTROL, | ||
| format!( | ||
| "public, max-age={}, immutable", |
| ), | ||
| ) | ||
| .with_header("X-GPT-Proxy", "true") | ||
| .with_header("X-Script-Source", script_url) |
There was a problem hiding this comment.
❓ Do we need to pass back X-Script-Source all the time
| function patchCommandQueue(tag: Partial<GoogleTag>): void { | ||
| const pending = Array.isArray(tag.cmd) ? [...tag.cmd] : []; | ||
| const queue: Array<() => void> = []; | ||
| tag.cmd = queue; |
There was a problem hiding this comment.
🔧 tag.cmd = queue replaces the googletag.cmd object. If GPT is already loaded, this can drop GPT’s custom cmd.push behavior (often immediate execution) and cause queued callbacks to stop executing. Please preserve googletag.cmd identity and patch cmd.push in place), then re-wrap any already queued callbacks.
| } | ||
|
|
||
| fn build(settings: &Settings) -> Option<Arc<GptIntegration>> { | ||
| let config = match settings.integration_config::<GptConfig>(GPT_INTEGRATION_ID) { |
There was a problem hiding this comment.
🔧 GPT shim self-initializes unconditionally on bundle load. This ignores server integration config ([integrations.gpt].enabled) and can rewrite GPT URLs to /integrations/gpt/* even when GPT proxy routes are not registered, causing failed script loads.
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { |
There was a problem hiding this comment.
🔧 Coverage gap: tests in this module validate URL matching/rewriter config, but there are no tests for handle_script_serving (gpt.rs (line 144)) or handle_pagead_proxy (gpt.rs (line 207)). Please add handler-level tests for path/query forwarding, non-2xx upstream behavior, and content-encoding/vary header propagation.
| * Any commands already queued before the shim loads are re-queued through the | ||
| * wrapper so GPT executes them later via its normal queue-drain behavior. | ||
| */ | ||
| function patchCommandQueue(tag: Partial<GoogleTag>): void { |
There was a problem hiding this comment.
🔧 Coverage gap: patchCommandQueue has no direct tests. Please add tests that cover preserving googletag.cmd behavior when GPT is already loaded, idempotent install (no double wrapping), and pending callback wrapping semantics.
|
|
||
| // Self-initialise on import (guarded for SSR safety) | ||
| if (typeof window !== 'undefined') { | ||
| installGptShim(); |
There was a problem hiding this comment.
🔧 Coverage gap: self-init path is untested. Please add tests for enabled/disabled runtime gating and verify we do not install the shim when GPT integration is disabled.
2 participants
What this does
Proxies gpt.js script and additional scripts loaded by gpt.js that do the heavy lifting for ad calls for google.
Change Summary
Closes #227
Verifying GPT Script Proxying in the Browser
To confirm that GPT scripts are being proxied through the trusted server domain: