Cookie Consent: Add consent log IP handling modes#49997
Draft
chihsuan wants to merge 8 commits into
Draft
Conversation
Summary: - Add log.ip_mode with drop as the default cookie-consent config. - Store consent log IP values according to raw, hash, truncate, or drop. - Add PHP unit coverage and package PHPUnit config for the new behavior. Rationale: - The consent log should avoid persisting full visitor IP addresses unless a consumer explicitly configures raw storage. - The hash format uses a salted SHA-256 digest encoded to fit the existing IP address column. Tests: - composer --working-dir=projects/packages/cookie-consent test-php - composer validate --working-dir=projects/packages/cookie-consent --strict - php -l on changed PHP files - git diff --check - not run: composer phpcs:changed, root tooling dependency phpcs-changed is not installed in this workspace
Summary: - Match fallback IPv6 truncation to WordPress's /64 anonymization mask. - Allow null in the consent log read schema for dropped IP addresses. - Cover the nullable schema contract in the PHP unit suite. Rationale: - The fallback truncation should describe the same storage behavior as the WordPress runtime path. - The REST schema should match the default drop mode, which stores no IP value. Tests: - composer --working-dir=projects/packages/cookie-consent test-php - composer validate --working-dir=projects/packages/cookie-consent --strict - php -l on changed PHP files - git diff --check - not run: composer phpcs:changed, root tooling dependency phpcs-changed is not installed in this workspace
Summary: - Add a namespaced WordPress anonymizer test double for consent-log IP tests. - Cover truncate mode through both the wp_privacy_anonymize_ip path and the controller fallback path. - Keep the branch rebuilt on origin/trunk with only cookie-consent changes. Rationale: - Production uses WordPress's anonymizer when it is available, so the tests should assert that path directly. - The fallback remains useful defensive code and should keep the same /24 and /64 behavior if WordPress's helper is unavailable. Tests: - composer --working-dir=projects/packages/cookie-consent test-php - composer validate --working-dir=projects/packages/cookie-consent --strict - php -l on changed PHP files - git diff --check - git diff --name-only origin/trunk...HEAD - not run: composer phpcs:changed, root tooling dependency phpcs-changed is not installed in this workspace
Code Coverage SummaryThis PR did not change code coverage! That could be good or bad, depending on the situation. Everything covered before, and still is? Great! Nothing was covered before? Not so great. 🤷 |
Contributor
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! |
github-actions
Bot
added
the
[Status] Needs Author Reply
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the Cookie Consent package’s consent-log IP storage behavior to be configurable, with a privacy-preserving default that no longer stores visitor IP addresses unless explicitly enabled via config.
Changes:
- Added
log.ip_modeconfiguration withdrop(default),truncate,hash, andrawmodes for consent-log IP persistence. - Updated the consent-log REST schema so
ip_addresscan benullwhen IP storage is dropped. - Added PHPUnit coverage for the new IP-mode behavior, including WordPress anonymizer and fallback truncation paths.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| projects/packages/cookie-consent/src/class-consent-log-controller.php | Implements log.ip_mode resolution and IP formatting logic; updates REST schema to allow null IPs. |
| projects/packages/cookie-consent/src/class-cookie-consent.php | Adds default config for log.ip_mode and exposes config accessor for controller use. |
| projects/packages/cookie-consent/tests/php/Consent_Log_Controller_Test.php | Adds test coverage for default/configured IP modes, anonymizer behavior, and nullable schema. |
| projects/packages/cookie-consent/changelog/wooa7s-1605-ip-mode | Documents the user-facing change in consent-log IP handling. |
Document Cookie_Consent::get_config() as internal API and punctuate its docblock so external consumers are discouraged from relying on it. Addresses review comment 3480401427.
Summary: - Move namespaced consent log test double functions into a functions-only helper include. - Fix IP mode test case array alignment in the PHPUnit class file. Rationale: - Keeps the test coverage for the WordPress anonymizer and fallback paths while satisfying the sniffer rule that forbids mixing function declarations and OO declarations in one file. Tests: - php -l projects/packages/cookie-consent/tests/php/Consent_Log_Controller_Test.php - php -l projects/packages/cookie-consent/tests/php/consent-log-controller-test-functions.php - composer --working-dir=projects/packages/cookie-consent test-php - composer validate --working-dir=projects/packages/cookie-consent --strict - git diff --check Review-comment: 3480401427
Summary: - Guard the test-only ReflectionMethod::setAccessible() call so it only runs on PHP versions where private method invocation still requires it. - Keep the existing private helper coverage compatible with PHP 7.2+. Rationale: - PHP 8.5 deprecates ReflectionMethod::setAccessible() because it has no effect since PHP 8.1, and PHPUnit fails the package test job when that deprecation is triggered. - Version-gating matches the existing repository pattern and avoids changing the production cookie-consent behavior. Tests: - php -l projects/packages/cookie-consent/tests/php/Consent_Log_Controller_Test.php - php -l projects/packages/cookie-consent/tests/php/consent-log-controller-test-functions.php - composer --working-dir=projects/packages/cookie-consent test-php - composer validate --working-dir=projects/packages/cookie-consent --strict - git diff --check - composer phpcs:changed -- <changed PHP files> blocked locally: phpcs-changed is not installed - composer phpcs:lint:required -- <changed PHP files> blocked locally: phpcs is not installed - jp test php packages/cookie-consent blocked locally: jp is not installed
Clarify the @internal tag on Cookie_Consent::get_config() so the public method is documented as package-only API. Addresses review comment 3480401427.
Member
Author
|
Symphony AI is treating the Copilot review overview as informational and made no change for that review summary because it only summarized the PR and did not request a specific code change. |
Use the standard hex HMAC output for consent-log hash mode so PHPCS no longer flags base64_encode() as discouraged obfuscation. Clarify that Cookie_Consent::get_config() is package-internal API. Addresses reviewer comment 3480401427.
Member
Author
|
Symphony AI is treating the Copilot pull request overview as informational and made no change specifically for that summary because it only describes the PR and does not request a change. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes WOOA7S-1605
Proposed changes
log.ip_modesupport for consent-log IP storage withraw,hash,truncate, anddropmodes.log.ip_modetodrop, so the legacy path no longer stores visitor IP addresses unless configured otherwise.ip_addresscan benull.Related product discussion/links
Does this pull request change what data or activity we track or use?
Yes. This changes consent-log IP storage behavior. The default now stores no IP address, and configured non-raw modes avoid persisting the full visitor IP.
Testing instructions
composer --working-dir=projects/packages/cookie-consent test-php.composer validate --working-dir=projects/packages/cookie-consent --strict.composer phpcs:lint:required -- projects/packages/cookie-consent/src/class-cookie-consent.php projects/packages/cookie-consent/src/class-consent-log-controller.php projects/packages/cookie-consent/tests/php/Consent_Log_Controller_Test.php projects/packages/cookie-consent/tests/php/consent-log-controller-test-functions.php.composer phpcs:changed -- projects/packages/cookie-consent/src/class-cookie-consent.php projects/packages/cookie-consent/src/class-consent-log-controller.php projects/packages/cookie-consent/tests/php/Consent_Log_Controller_Test.php projects/packages/cookie-consent/tests/php/consent-log-controller-test-functions.php.php -lon the changed PHP files.git diff --name-only origin/trunk...HEADonly lists files underprojects/packages/cookie-consent/.Validation performed:
composer --working-dir=projects/packages/cookie-consent test-phppassed: 31 tests, 61 assertions.composer validate --working-dir=projects/packages/cookie-consent --strictpassed.composer phpcs:lint:required -- projects/packages/cookie-consent/src/class-cookie-consent.php projects/packages/cookie-consent/src/class-consent-log-controller.php projects/packages/cookie-consent/tests/php/Consent_Log_Controller_Test.php projects/packages/cookie-consent/tests/php/consent-log-controller-test-functions.phppassed.composer phpcs:changed -- projects/packages/cookie-consent/src/class-cookie-consent.php projects/packages/cookie-consent/src/class-consent-log-controller.php projects/packages/cookie-consent/tests/php/Consent_Log_Controller_Test.php projects/packages/cookie-consent/tests/php/consent-log-controller-test-functions.phppassed.php -lpassed on changed PHP files.git diff --checkpassed.origin/trunk...HEADpath outsideprojects/packages/cookie-consent/.Not run:
jp test php packages/cookie-consent: blocked becausejpis not installed in this workspace.jp phan packages/cookie-consent: blocked becausejpis not installed in this workspace.vendor/bin/phan --allow-polyfill-parser --config-file projects/packages/cookie-consent/.phan/config.php: stopped after several minutes without completing locally; normal direct Phan was blocked by missingphp-ast.