Skip to content

Conversation

@gilluminate
Copy link
Contributor

@gilluminate gilluminate commented Oct 30, 2025

Ticket ENG-1668

Description Of Changes

Fixed a race condition in FidesJS TCF initialization where gdprApplies could temporarily be undefined, causing the IAB CMP API to incorrectly report that GDPR does not apply. This could result in assets being served without proper consent checks for returning users with existing TCF cookies.

The issue occurred because:

  1. fidesTcfGdprApplies was not guaranteed to have a default value during initialization
  2. The check used truthy/falsy logic that treated undefined as false

Code Changes

  • Updated fides-tcf.ts to ensure fidesTcfGdprApplies defaults to true for TCF experiences during initialization, with proper override handling
  • Updated lib/tcf/events.ts to use strict equality check (=== false) instead of falsy check, so only explicitly false values set gdprApplies = false
  • Ensured the TCF stub receives the correct default value on creation

Steps to Confirm

  1. Initialize FidesJS with TCF enabled but without explicitly setting fides_tcf_gdpr_applies
  2. Verify that returning users with existing TCF cookies receive gdprApplies = true consistently
  3. Verify that assets requiring consent are properly gated
  4. Test that explicitly setting fides_tcf_gdpr_applies: false still works correctly

Pre-Merge Checklist

  • Issue requirements met
  • All CI pipelines succeeded
  • CHANGELOG.md updated
    • Add a db-migration This indicates that a change includes a database migration label to the entry if your change includes a DB migration
    • Add a high-risk This issue suggests changes that have a high-probability of breaking existing code label to the entry if your change includes a high-risk change (i.e. potential for performance impact or unexpected regression) that should be flagged
    • Updates unreleased work already in Changelog, no new entry necessary
  • UX feedback:
    • All UX related changes have been reviewed by a designer
    • No UX review needed
  • Followup issues:
    • Followup issues created
    • No followup issues
  • Database migrations:
    • Ensure that your downrev is up to date with the latest revision on main
    • Ensure that your downgrade() migration is correct and works
      • If a downgrade migration is not possible for this change, please call this out in the PR description!
    • No migrations
  • Documentation:
    • Documentation complete, PR opened in fidesdocs
    • Documentation issue created in fidesdocs
    • If there are any new client scopes created as part of the pull request, remember to update public-facing documentation that references our scope registry
    • No documentation updates required

@vercel
Copy link

vercel bot commented Oct 30, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
fides-plus-nightly Ignored Ignored Preview Oct 31, 2025 4:03pm
fides-privacy-center Ignored Ignored Oct 31, 2025 4:03pm

@gilluminate gilluminate force-pushed the gill/ENG-1668/fides-js-gdpr-applies-can-be branch 2 times, most recently from aadcac2 to a23f75c Compare October 30, 2025 21:37
@gilluminate gilluminate marked this pull request as ready for review October 30, 2025 21:48
@gilluminate gilluminate requested a review from a team as a code owner October 30, 2025 21:48
@gilluminate gilluminate requested review from jpople and removed request for a team October 30, 2025 21:48
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.

Greptile Overview

Greptile Summary

This PR fixes a race condition in FidesJS TCF initialization where gdprApplies could incorrectly be set to false when it should be true. The bug occurred because fidesTcfGdprApplies could be undefined during initialization, and the check used falsy logic that treated undefined as false.

The fix has two parts:

  • Changed the check in extractTCStringForCmpApi from falsy (!window.Fides?.options?.fidesTcfGdprApplies) to strict equality (=== false), so only explicitly false values trigger the GDPR-does-not-apply behavior
  • Ensured fidesTcfGdprApplies always has a defined value by defaulting to true in two locations during initialization: when creating the TCF stub and when merging config options

This ensures returning users with existing TCF cookies receive the correct gdprApplies = true value consistently, preventing assets from being served without proper consent checks.

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The changes are surgical and address a specific race condition with clear logic. The fix uses defensive programming (strict equality and default values) to prevent undefined behavior. The changes are well-commented and the logic is sound: defaulting to true for TCF experiences is the correct behavior per GDPR requirements.
  • No files require special attention

Important Files Changed

File Analysis

Filename Score Overview
clients/fides-js/src/lib/tcf/events.ts 5/5 Changed falsy check to strict equality check (=== false) to prevent undefined from being treated as false, fixing the core race condition bug
clients/fides-js/src/fides-tcf.ts 5/5 Added default value of true for fidesTcfGdprApplies in two locations to ensure it's always defined during initialization, preventing race condition

3 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@gilluminate gilluminate force-pushed the gill/ENG-1668/fides-js-gdpr-applies-can-be branch from a23f75c to f3be891 Compare October 31, 2025 16:03
@gilluminate gilluminate enabled auto-merge October 31, 2025 16:09
Copy link
Contributor

@jpople jpople left a comment

Choose a reason for hiding this comment

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

Tested locally, confirming bug is fixed.

@gilluminate gilluminate added this pull request to the merge queue Nov 4, 2025
Merged via the queue into main with commit fa0f8d6 Nov 4, 2025
42 checks passed
@gilluminate gilluminate deleted the gill/ENG-1668/fides-js-gdpr-applies-can-be branch November 4, 2025 07:56
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