Skip to content

feature: add GTM + update CSP to allow it#5507

Merged
carlosmonastyrski merged 4 commits intomainfrom
feat/ENG-4565
Feb 24, 2026
Merged

feature: add GTM + update CSP to allow it#5507
carlosmonastyrski merged 4 commits intomainfrom
feat/ENG-4565

Conversation

@carlosmonastyrski
Copy link
Contributor

Context

Added Google Tag Manager to cloud instances, plus product updates message on sign up page

Screenshots

Steps to verify the change

Type

  • Fix
  • Feature
  • Improvement
  • Breaking
  • Docs
  • Chore

Checklist

  • Title follows the conventional commit format: type(scope): short description (scope is optional, e.g., fix: prevent crash on sync or fix(api): handle null response).
  • Tested locally
  • Updated docs (if needed)
  • Read the contributing guide

@linear
Copy link

linear bot commented Feb 17, 2026

@maidul98
Copy link
Collaborator

maidul98 commented Feb 17, 2026

Snyk checks have passed. No issues have been found so far.

Status Scanner Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 17, 2026

Greptile Summary

This PR adds Google Tag Manager (GTM) to Infisical Cloud deployments and a "product updates" notice on the sign-up page.

Key changes:

  • A GTM inline script is added to index.html with a hostname guard that restricts it to app.infisical.com, eu.infisical.com, us.infisical.com, and gamma.infisical.com — correct approach.
  • The backend (serve-ui.ts) conditionally extends the CSP with analytics/ad-tech domains only when appCfg.isCloud is true, preventing CSP expansion for self-hosted deployments — good design.
  • A <noscript> GTM iframe is statically embedded in index.html without a hostname guard, meaning it is present in the HTML served by all deployments (self-hosted too). On self-hosted instances it is blocked by CSP, so there is no functional tracking leak, but it represents a silent no-op on every self-hosted deployment rather than being cleanly scoped. The backend's conditional injection pattern (already used for CDN and CSP) is the appropriate way to handle this.
  • The PR also fixes a pre-existing bug where connect-src was duplicated in the CSP (browsers only honor the first occurrence), consolidating it into a single correct directive.
  • A product-updates-notice translation key is added and correctly used via t() in InitialSignupStep.tsx. Since i18n is hardcoded to lng: "en" with fallbackLng: "en", the key not being present in other locale files is not a functional issue.

Confidence Score: 4/5

  • This PR is safe to merge with one minor logical issue to address: the unconditional noscript GTM iframe in index.html.
  • The core implementation is sound — the GTM JS script has a proper hostname guard, and the CSP injection is correctly gated behind isCloud in the backend. The one real issue is the noscript iframe being present unconditionally in the static HTML, which is served to self-hosted instances as well. On self-hosted, this is benign (blocked by CSP) but is technically incorrect. The CSP double connect-src fix is a positive improvement. No security data leaks or breaking API changes were introduced.
  • frontend/index.html — the noscript GTM iframe lacks a hostname guard and is unconditionally embedded in all deployments.

Important Files Changed

Filename Overview
backend/src/server/plugins/serve-ui.ts Adds conditional CSP extension for cloud deployments using RE2 regex injection. The pattern correctly uses RE2 to avoid ReDoS, the isCloud check properly gates the analytics domains, and the logic integrates cleanly with the existing CDN injection. Minor: the regex uses [^;]+ which is greedy across newlines, but this is safe given the current CSP format.
frontend/index.html Adds GTM script with hostname guard (correct) and a noscript fallback iframe without a hostname guard (problematic). The noscript iframe is unconditionally present in all deployments and will attempt to contact googletagmanager.com even on self-hosted instances. Also fixes a pre-existing bug where connect-src was duplicated in the old CSP.
frontend/public/locales/en/translations.json Adds a new translation key 'product-updates-notice' to the signup section. The key is only added to the English locale, but since i18n is hardcoded to English with fallbackLng: 'en', this is not a functional issue.
frontend/src/components/auth/InitialSignupStep.tsx Adds a product-updates-notice paragraph to the signup step using the i18n t() function with the newly added translation key. The lg:w-1/4 width difference from the adjacent create-policy div (lg:w-1/6) was confirmed intentional per PR comments.

Last reviewed commit: 27c5c5c

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 files reviewed, 5 comments

Edit Code Review Agent Settings | Greptile

@carlosmonastyrski
Copy link
Contributor Author

@greptile please review this PR again with the fixes made and update the summary to match the new state of the PR

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

@carlosmonastyrski carlosmonastyrski merged commit fb1747c into main Feb 24, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants