Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Allow passing a nonce to google tag manager related script #73370

Merged
merged 4 commits into from
Dec 3, 2024

Conversation

abdelrahmanAbouelkheir
Copy link
Contributor

What

Adding nonce in script in google tag manager

Why

Prevent from using csp header inline script or adding url manually in csp headers by package users.

How

passing nonce to the script.

What 

Adding nonce in script in google tag manager

Why 

Prevent from using csp header inline script or adding url manually in csp headers by package users.

How

passing nonce to the script.
@ijjk
Copy link
Member

ijjk commented Nov 30, 2024

Allow CI Workflow Run

  • approve CI run for commit: 3298c0e

Note: this should only be enabled once the PR is ready to go and can only be enabled by a maintainer

@abdelrahmanAbouelkheir abdelrahmanAbouelkheir changed the title Add nonce to gtm script Allow passing a nonce to google tag manager related script Nov 30, 2024
@abdelrahmanAbouelkheir
Copy link
Contributor Author

Sorry, I don't understand exactly, does anyone contributing to code is a maintainer? and how to enable it if I am considered as a maintainer

@Matthew-Wise
Copy link

Hi,

Hit this problem today and came to do the same PR. For any maintainer out there who wants some more context the suggestion from googles inline script adds the nonce attribute to the include allowing strict-dynamic to work with the csp

https://developers.google.com/tag-platform/security/guides/csp

Thanks
Matt

@ijjk
Copy link
Member

ijjk commented Dec 3, 2024

@abdelrahmanAbouelkheir no need to merge canary, this blocks the tests from running

@ijjk
Copy link
Member

ijjk commented Dec 3, 2024

Failing test suites

Commit: 3298c0e

pnpm test test/integration/amp-export-validation/test/index.test.js (turbopack)

  • AMP Validation on Export > production mode > should have shown errors during build
Expand output

● AMP Validation on Export › production mode › should have shown errors during build

expect(received).toMatch(expected)

Expected pattern: /error.*The mandatory attribute 'height' is missing in tag 'amp-video'\./
Received string:  "   Loading config from /root/actions-runner/_work/next.js/next.js/test/integration/amp-export-validation/next.config.js
   Loading config from /root/actions-runner/_work/next.js/next.js/test/integration/amp-export-validation/next.config.js
   ▲ Next.js 15.0.4-canary.36 (Turbopack)·
   Checking validity of types ...
   Creating an optimized production build ...
   Loading config from /root/actions-runner/_work/next.js/next.js/test/integration/amp-export-validation/next.config.js
   Building (0/7) ...
   Building (1/7)··
   Building (3/7)··
   Building (5/7)··
 ✓ Building (7/7)
 ✓ Compiled successfully in 2.8s
   Collecting page data ...
   Generating static pages (0/8) ...
   Generating static pages (2/8)··
 ⚠ Linting is disabled.
Error occurred prerendering page \"/first\". Read more: https://nextjs.org/docs/messages/prerender-error
AssertionError: Assertion failed: WebAssembly is uninitialized

  at new module$contents$goog$asserts_AssertionError (../evalmachine.<anonymous>:102:1441)
  at module$contents$goog$asserts_doAssertFailure (../evalmachine.<anonymous>:103:354)
  at goog.asserts.assertExists (../evalmachine.<anonymous>:104:374)
  at Object.module$contents$amp$validator_validateString [as validateString] (../evalmachine.<anonymous>:2238:108)
  at Validator.validateString (../packages/next/dist/compiled/amphtml-validator/index.js:1:20650)
  at validateAmp (../packages/next/dist/export/routes/pages.js:100:34)
  at async exportPagesPage (../packages/next/dist/export/routes/pages.js:117:13)
  at async Span.traceAsyncFn (../packages/next/dist/trace/trace.js:153:20)
  at async exportPage (../packages/next/dist/export/worker.js:334:18)
  Export encountered an error on /first, exiting the build.
   ⨯ Static worker exited with code: 1 and signal: null
  "
  at Object.toMatch (integration/amp-export-validation/test/index.test.js:28:29)

Read more about building and testing Next.js in contributing.md.

@ijjk ijjk merged commit 3fcd1a1 into vercel:canary Dec 3, 2024
104 of 106 checks passed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 18, 2024
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants