DC-492 - Add Meta Pixel support to Enrollment app#352
Conversation
|
it's not clear to me that this will fire on each route change, since this is a SPA. I am not sure if that is desirable either. |
| setError(null) | ||
| setIsSubmitting(true) | ||
| try { | ||
| if (process.env.NEXT_PUBLIC_META_PIXEL && process.env.NEXT_PUBLIC_META_PIXEL_ACTION) { |
There was a problem hiding this comment.
Invokes window.fbq contingent on an externally loaded Meta Pixel script that isn't version-pinned or checksum-verified. Ensure the Pixel script is pinned and integrity-checked before relying on fbq.
Details
✨ AI Reasoning
The new submit handler calls the global fbq function if environment variables are present. This runtime call depends on an external Meta Pixel script being loaded and untampered. Since the script is added elsewhere in this PR without checksum or version pinning, this change increases risk: it couples application behavior to an unpinned external dependency and lacks verification that fbq originates from an expected, immutable artifact.
🔧 How do I fix it?
Include lockfiles in your repo and CI. Pin exact versions or SHAs in manifests. Enable checksum verification. Set ignore-scripts in npm or equivalent. Enforce a 72hr minimum package age policy to avoid newly published malicious packages.
Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info
| {process.env.NEXT_PUBLIC_BUILD_SHA && ( | ||
| <meta name="build-sha" content={process.env.NEXT_PUBLIC_BUILD_SHA} /> | ||
| )} | ||
| {process.env.NEXT_PUBLIC_META_PIXEL && ( |
There was a problem hiding this comment.
Loads meta_pixel.js from a runtime path without pinning or integrity checks. Pin the script version and include checksum verification (or vendor the file) to avoid silent/unauthorized updates.
Details
✨ AI Reasoning
The new code injects an external JavaScript asset at runtime using an environment-controlled path. This introduces an unmanaged third-party dependency into the production build: its exact contents, version, and integrity are not pinned, nor is there any checksum or package-lock-style verification. According to strict dependency management practices, externally loaded scripts should be versioned and verified (lockfile/integrity) to prevent silent or malicious upgrades. This change therefore worsens the repository's dependency management posture by adding an unpinned, unverifiable script load.
🔧 How do I fix it?
Include lockfiles in your repo and CI. Pin exact versions or SHAs in manifests. Enable checksum verification. Set ignore-scripts in npm or equivalent. Enforce a 72hr minimum package age policy to avoid newly published malicious packages.
Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info
| !function(f,b,e,v,n,t,s){if(f.fbq)return;n=f.fbq=function(){n.callMethod? | ||
| n.callMethod.apply(n,arguments):n.queue.push(arguments)};if(!f._fbq)f._fbq=n; | ||
| n.push=n;n.loaded=!0;n.version='2.0';n.queue=[];t=b.createElement(e);t.async=!0; | ||
| t.src=v;s=b.getElementsByTagName(e)[0];s.parentNode.insertBefore(t,s)}(window, |
There was a problem hiding this comment.
Sets t.src to an unpinned external URL ('https://connect.facebook.net/en_US/fbevents.js') without checksum or version pinning; include a pinned version and integrity verification.
Details
✨ AI Reasoning
The new code injects an external script by setting t.src to a public CDN URL. This introduces an unmanaged runtime dependency that can silently change (no pinned version or checksum), which violates strict dependency management expectations. It increases risk of unexpected behavior or supply-chain changes because there is no integrity or pinned reference for the fetched asset.
🔧 How do I fix it?
Include lockfiles in your repo and CI. Pin exact versions or SHAs in manifests. Enable checksum verification. Set ignore-scripts in npm or equivalent. Enforce a 72hr minimum package age policy to avoid newly published malicious packages.
Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info
|
Added myself as a reviewer and also re-applied the "do not merge" label as this PR should not be merged, as-is, without further discussion. |
Add Meta Pixel support to Enrollment app
🔗 Jira ticket
✍️ Description
If the
NEXT_PUBLIC_META_PIXELenv var is set at build time, inserts the FB Meta Pixel JS snippet into the page head.If the
NEXT_PUBLIC_META_PIXEL_ACTIONenv var is set, calls a trigger action on the final Review page Submit button click.Expects that the
public/meta_pixel.jsfile is updated at build time to replaceNEXT_PUBLIC_META_PIXELstring with the Meta account id. This is left as an exercise to the builder, but in our Makefile it's:🔗 Links to related PRs
✅ Completion tasks