Fix #1156: Enable CDN detection to prevent privacy guideline violations#1179
Open
faisalahammad wants to merge 1 commit intoWordPress:trunkfrom
Open
Fix #1156: Enable CDN detection to prevent privacy guideline violations#1179faisalahammad wants to merge 1 commit intoWordPress:trunkfrom
faisalahammad wants to merge 1 commit intoWordPress:trunkfrom
Conversation
… external CDN usage This commit enables the existing EnqueuedResourceOffloadingSniff in the plugin-check ruleset to detect when plugins load external assets from third-party CDNs without user consent, addressing a privacy guideline violation (WordPress.org guideline WordPress#7). The sniff already existed and was fully functional with comprehensive unit tests, but was not included in the plugin-check.ruleset.xml file. This simple addition now ensures PCP will flag CDN usage from jsdelivr, unpkg, cloudflare, bootstrapcdn, and 20+ other known CDN services.
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
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.
Summary
This PR fixes issue #1156 by enabling the existing
EnqueuedResourceOffloadingSniffin the plugin-check ruleset. This allows Plugin Check to detect when plugins load external assets from third-party CDNs without user consent, which violates WordPress.org plugin guideline #7 (privacy requirements).The fix is simple: Add the existing, fully tested sniff to the ruleset configuration file (just 5 lines of code).
Problem Description
What Was Happening
Plugin Check was not detecting when plugins loaded CSS and JS files from external CDN services like jsdelivr, unpkg, or bootstrapcdn. This is a violation of WordPress.org Plugin Guideline #7 which states:
Loading assets from external servers can expose user data (IP addresses, browsing behavior) to third parties without the user's knowledge or permission.
Example Code That Was Not Being Detected
When plugin developers ran Plugin Check on code like this, they received no warnings or errors, even though the code violates privacy guidelines.
Root Cause
After investigating the codebase, I found that:
phpcs-sniffs/PluginCheck/Sniffs/CodeAnalysis/EnqueuedResourceOffloadingSniff.phpphpcs-sniffs/PluginCheck/Tests/CodeAnalysis/EnqueuedResourceOffloadingUnitTest.phpphpcs-rulesets/plugin-check.ruleset.xml, so Plugin Check never ran itI verified this by running the sniff directly:
./vendor/bin/phpcs test-file.php --standard=PluginCheck \ --sniffs=PluginCheck.CodeAnalysis.EnqueuedResourceOffloading # Result: Successfully detected both CDN violationsSolution
What This PR Does
This PR adds the
EnqueuedResourceOffloadingSniffto the plugin-check ruleset configuration file. No new code is written. We are simply enabling an existing, tested feature.Changes Made
File:
phpcs-rulesets/plugin-check.ruleset.xmlBefore:
After:
That's it! Just 5 lines of XML configuration.
How It Works
The
EnqueuedResourceOffloadingSniffworks by:Detecting WordPress asset enqueue functions during static code analysis:
wp_enqueue_script()wp_register_script()wp_enqueue_style()wp_register_style()Extracting the source URL from the second parameter (
$src)Checking against known CDN patterns using the
OffloadingServicesTrait, which includes over 20 known CDN services:cdn.jsdelivr.net(the one from issue PCP does not detect external CDN asset loading (privacy guideline violation) #1156)unpkg.combootstrapcdn.comcloudflare.comajax.googleapis.comrawgit.comamazonaws.comfontawesomeCDNsReporting an error when external CDN usage is detected
Special Cases Handled
fonts.gstatic.comsince Google Fonts are commonly used and generally acceptedBOOTSTRAPCDN.COMorcdn.jsdelivr.netTesting
Test 1: Code From Issue #1156
I created a test file with the exact code reported in the issue:
Before this PR:
./vendor/bin/phpcs test-file.php --standard=phpcs-rulesets/plugin-check.ruleset.xml # Result: No errors foundAfter this PR:
✅ Both violations are now properly detected!
Test 2: Existing Unit Tests
The sniff already has comprehensive unit tests that verify detection of multiple CDN services:
✅ All existing tests pass correctly!
These test results match the expected errors defined in the unit test file, confirming the sniff is working as designed.
Test 3: Edge Cases
The existing unit tests cover important edge cases:
Why This Approach
I considered three potential solutions:
Option 1: Enable Existing Sniff (This PR)
Option 2: Create New Runtime Check
Option 3: Hybrid Approach (Both Static and Runtime)
Option 1 is clearly the best choice because it is simple, uses proven code, requires no new maintenance, and solves the reported issue completely.
Impact
Benefits
Risk Assessment
Related Files
phpcs-sniffs/PluginCheck/Sniffs/CodeAnalysis/EnqueuedResourceOffloadingSniff.phpphpcs-sniffs/PluginCheck/Helpers/OffloadingServicesTrait.phpphpcs-sniffs/PluginCheck/Tests/CodeAnalysis/EnqueuedResourceOffloadingUnitTest.phpphpcs-sniffs/PluginCheck/Tests/CodeAnalysis/EnqueuedResourceOffloadingUnitTest.1.incChecklist
Closes
Fixes #1156