Skip to content

Fixed Harden profiler, media replacement, and dashboard widget vulnerabilities - #1160

Merged
kushh23 merged 9 commits into
masterfrom
bugfix/optimole-service/1805
Sep 28, 2026
Merged

kushh23 merged 9 commits into
masterfrom
bugfix/optimole-service/1805

Conversation

@girishpanchal30

Copy link
Copy Markdown
Contributor

All Submissions:

Changes proposed in this Pull Request:

Three security issues reported against 4.2.14:

  • Profiler CSS selectors were concatenated into a :not() rule with strip_tags, allowing unauthenticated stored CSS injection. Any client selector carrying CSS metacharacters now voids that device's hide rule, as the browser already did, so nothing is injected and behaviour is unchanged.
  • The file-replace path skipped SVG sanitization; it now runs the same sanitizer as normal uploads before writing the file.
  • The dashboard widget exposed the full service_data (CDN key/secret) to any logged-in user; it is now gated on manage_options and only the stats fields are sent to the page.

Closes https://github.com/Codeinwp/optimole-service/issues/1805

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully ran tests with your changes locally?

@pirate-bot

pirate-bot commented Sep 24, 2026 •

Copy link
Copy Markdown
Collaborator

Plugin build for 1ea0fea is ready 🛎️!

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

The unresolved SVG sanitization bypass and breakpoint behavior must be fixed before approval.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 High severity

Open (1)
What changed in this PR

Hardens profiler CSS handling, SVG replacement, and dashboard widget data exposure.

Changes:

  • Validates profiler selectors.
  • Sanitizes replacement SVGs.
  • Restricts widget access and filters sensitive data.
  • Adds regression tests.
File Review
tests/​test-dashboard-widget.php Tests widget authorization and data filtering.
tests/​test-bg-selector.php Tests selector validation, but lacks a mixed safe/unsafe device case.
tests/​media_rename/​test-attachment-replace.php Tests SVG replacement sanitization.
inc/​v2/​BgOptimizer/​Lazyload.php Unsafe selectors can cause safe rules to apply at the wrong breakpoint.
inc/​media_rename/​attachment_replace.php A sanitizer write failure can still allow unsanitized SVG replacement.
inc/​dashboard_widget.php Restricts widget access and filters localized data.
inc/​admin.php Exposes the shared SVG sanitizer.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread inc/media_rename/attachment_replace.php

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Critical partial-write handling and moderate selector-scoping and test-portability issues remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 High severity

Open (1)
Resolved since last review (1)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Keep surviving selectors scoped to their device

inc/​v2/​BgOptimizer/​Lazyload.php:103

Clearing only this device's selectors does not keep the surviving rule device-specific. The rendering logic below treats an empty mobile/desktop selector list as absent data and returns the other device's selectors without a media query, so an unsafe mobile submission with valid desktop data makes the desktop hide rule apply on mobile (and vice versa), potentially hiding an above-fold background. Preserve the tainted-device state through rendering and media-scope the remaining device; add an asymmetric-device regression case.

Comment thread inc/admin.php Outdated

@pirate-bot pirate-bot left a comment •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🟡 Changes requested

The security fixes hold, but an unsafe selector on one device now leaks the other device's hide rule to all screen widths.

Validation details
  • Files reviewed: 7/7 changed files.
  • PR tests at HEAD pass on PHP 8.3 with WordPress 7.1.2: 32 tests, 1 skipped as root.
  • On the base tree, the injection, SVG replace and credential tests fail as expected.
  • A read-only SVG test ran as www-data. sanitize_svg() returns false at HEAD and true at base.
  • The security pass found no bypass in the selector check, SVG replace or widget data.
  • The widget scripts read only the six fields in the allow-list.
Untested areas
  • The write-failure test skips as root. An isolated run covered it instead.
  • Jest and Playwright suites were not run.
  • The widget rendering was not checked in a browser.

🤖 Automated review · run code-review-agent_6ab4ed214db4a7.48343246.


🤖 Review agent — review posted ✅ on 2bfd6607 · changes requested · 3 findings · 10 min

Run code-review-agent_6ab4ed214db4a7.48343246 · trail

Comment thread inc/v2/BgOptimizer/Lazyload.php Outdated
Comment thread inc/v2/BgOptimizer/Lazyload.php Outdated
Comment thread tests/test-bg-selector.php

@pirate-bot pirate-bot left a comment •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🟡 Changes requested

The media-query fix works, but the new desktop-voided branch has no test.

Validation details
  • Files reviewed: 7/7 changed files. New checks cover only the Lazyload.php and tests/test-bg-selector.php changes made after 2bfd660.
  • Test_Bg_Selector passes at HEAD on PHP 8.3 and WordPress 7.1.2: 23 tests.
  • With pr-base source files, the four new CSS tests fail on their assertions.
  • With the 2bfd660 source file, the new media-query scope test fails.
  • A desktop-voided run at HEAD returned only the mobile media query.
  • Reused security pass from run code-review-agent_6ab4ed214db4a7.48343246, not rerun: no bypass found.
Untested areas
  • Missing native types, repeated media-query strings and redundant assertions were raised but not confirmed. They were not posted.
  • Jest and end-to-end tests did not run. No JavaScript changed.

🤖 Automated review · run code-review-agent_6ab4fc345aa068.92423204.


🤖 Review agent — review posted ✅ on 63a51b57 · changes requested · 1 finding · 11 min

Run code-review-agent_6ab4fc345aa068.92423204 · trail

Comment thread tests/test-bg-selector.php Outdated

@pirate-bot pirate-bot left a comment •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🟡 Changes requested

The test fixes work. The new mobile scoping test needs a : void return type.

Validation details
  • Files reviewed: 1/7 changed files. Only tests/test-bg-selector.php changed after 63a51b5.
  • PHP 8.3 with the WordPress 7.1.2 test library: the three PR test classes at HEAD pass with 33 tests and 1 root-only skip.
  • With pr-base source and PR tests, all five test_personalized_css* tests fail on their assertions.
  • With 2bfd660 Lazyload.php, both scoping tests fail on the media query assertion.
  • Run code-review-agent_6ab4fc345aa068.92423204 checked the other six files. Its results were reused and not rerun.
Untested areas
  • Background lazyload rendering was not checked in a browser.

🤖 Automated review · run code-review-agent_6ab503cc1942f4.69205567.


🤖 Review agent — review posted ✅ on dc13197f · changes requested · 1 finding · 5 min

Run code-review-agent_6ab503cc1942f4.69205567 · trail

Comment thread tests/test-bg-selector.php Outdated

@pirate-bot pirate-bot left a comment •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔵 Needs a closer look

QA check: The code checks pass and the prior finding is fixed. Confirm the dashboard widget and background lazy loading on a real site.

Validation details
  • Files reviewed: 1/7 changed files. Only tests/test-bg-selector.php changed after dc13197.
  • PHP 8.3 with the WordPress 7.1.2 test library: the three PR test classes pass at HEAD with 33 tests and 1 root-only skip.
  • With pr-base source and PR tests, all five test_personalized_css* tests fail on their assertions.
  • Runs code-review-agent_6ab4ed214db4a7.48343246 and code-review-agent_6ab503cc1942f4.69205567 checked the other six files. Their results were reused and not rerun.
QA steps
  1. Log in as an administrator on a connected site with at least ten visits. Open Dashboard. Expect the Optimole widget to show visits, the visit limit, compression and traffic.
  2. Log in as a subscriber and open Dashboard. Expect no Optimole widget.
  3. Enable viewport lazy loading and background lazy loading in Optimole > Settings. Open a page with a background image above the fold on a phone and on a desktop. Expect the background to appear without delay on both.
  4. Scroll to a background image below the fold on both devices. Expect it to load when it enters the screen.
Untested areas
  • The widget was not rendered because the repository has no built assets and JS dependencies are absent.
  • Background lazy loading was not checked in a browser.
  • Jest and Playwright suites were not run.

🤖 Automated review · run code-review-agent_6ab5092b391357.13357155.


🤖 Review agent — review posted ✅ on 1ea0feaa · commented · 0 findings · 3 min

Run code-review-agent_6ab5092b391357.13357155 · trail

@girishpanchal30 girishpanchal30 changed the title Fixed harden profiler, replace, widget Fixed harden profiler, media replace, dashboard widget Sep 24, 2026
@girishpanchal30 girishpanchal30 changed the title Fixed harden profiler, media replace, dashboard widget Fixed Harden profiler, media replacement, and dashboard widget vulnerabilities Sep 24, 2026
@kushh23
kushh23 changed the base branch from development to master September 28, 2026 23:43
@kushh23
kushh23 merged commit 43e105d into master Sep 28, 2026
19 of 20 checks passed
@kushh23
kushh23 deleted the bugfix/optimole-service/1805 branch September 28, 2026 23:57
@pirate-bot

Copy link
Copy Markdown
Collaborator

🎉 This PR is included in version 4.2.15 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@pirate-bot pirate-bot added the released Indicate that an issue has been resolved and released in a particular version of the product. label Sep 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

released Indicate that an issue has been resolved and released in a particular version of the product.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants