Security review by alexander-b-clean · Pull Request #838 · CleanTalk/wordpress-antispam

alexander-b-clean · 2026-07-24T17:40:46Z

https://app.doboard.com/1/task/52113

codecov · 2026-07-24T17:44:20Z

Codecov Report

❌ Patch coverage is 47.05882% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 28.45%. Comparing base (07f04cc) to head (7690b40).

Files with missing lines	Patch %	Lines
lib/Cleantalk/ApbctWP/RemoteCalls.php	47.05%	9 Missing ⚠️

❌ Your patch check has failed because the patch coverage (47.05%) is below the target coverage (70.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

@@             Coverage Diff              @@
##                dev     #838      +/-   ##
============================================
- Coverage     28.47%   28.45%   -0.02%     
- Complexity     6192     6195       +3     
============================================
  Files           286      286              
  Lines         25630    25636       +6     
============================================
- Hits           7299     7296       -3     
- Misses        18331    18340       +9

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot

Pull request overview

This PR hardens security around remote-call execution and front-end modal rendering by restricting sensitive remote actions to verified CleanTalk RC servers and reducing XSS risk when inserting remotely loaded modal HTML.

Changes:

Add a centralized CleanTalk RC servers allowlist and require verified RC origin for plugin lifecycle remote-call actions.
Sanitize modal HTML before assigning to innerHTML, including when linkifying URLs.
Propagate the modal sanitization changes into prebuilt and minified bundle artifacts.

Reviewed changes

Copilot reviewed 10 out of 20 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
lib/Cleantalk/ApbctWP/RemoteCalls.php	Adds RC server allowlist and enforces verified-origin checks for sensitive plugin lifecycle remote calls.
js/src/common-cleantalk-modal.js	Sanitizes loaded modal HTML before injecting into the DOM.
js/prebuild/apbct-public-bundle.js	Prebuilt bundle updated to include modal HTML sanitization.
js/prebuild/apbct-public-bundle_int-protection.js	Prebuilt bundle variant updated to include modal HTML sanitization.
js/prebuild/apbct-public-bundle_int-protection_gathering.js	Prebuilt bundle variant updated to include modal HTML sanitization.
js/prebuild/apbct-public-bundle_gathering.js	Prebuilt bundle variant updated to include modal HTML sanitization.
js/prebuild/apbct-public-bundle_full-protection.js	Prebuilt bundle variant updated to include modal HTML sanitization.
js/prebuild/apbct-public-bundle_full-protection_gathering.js	Prebuilt bundle variant updated to include modal HTML sanitization.
js/prebuild/apbct-public-bundle_ext-protection.js	Prebuilt bundle variant updated to include modal HTML sanitization.
js/prebuild/apbct-public-bundle_ext-protection_gathering.js	Prebuilt bundle variant updated to include modal HTML sanitization.
js/common-cleantalk-modal.min.js.map	Source map updated to reflect modal sanitization changes.
js/common-cleantalk-modal.min.js	Minified modal script updated to include modal HTML sanitization.
js/apbct-public-bundle.min.js	Minified public bundle updated to include modal HTML sanitization.

Comments suppressed due to low confidence (1)

lib/Cleantalk/ApbctWP/RemoteCalls.php:134

Same strict hostname comparison issue as in checkWithoutToken(): consider normalizing the verified hostname (lowercase + trim trailing '.') before checking against RC_SERVERS to prevent accidental lockouts if DNS returns different casing or a trailing dot.

        if ( in_array($action, self::$pluginLifecycleActions, true) ) {
            // Resolve the client IP to a hostname (reverse DNS) and check it against the RC servers allowlist.
            $client_ip = Helper::ipGet('remote_addr');
            $verified_hostname = $client_ip ? \Cleantalk\Common\Helper::ipResolve($client_ip) : false;
            if ( $verified_hostname === false || ! in_array($verified_hostname, self::RC_SERVERS, true) ) {
                die('FAIL ' . json_encode(array('error' => 'FORBIDDEN_SOURCE')));
            }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

alexander-b-clean · 2026-08-12T11:03:52Z

        // Resolve IP of the client making the request and verify hostname from it to be in the list of RC servers hostnames
        $client_ip = Helper::ipGet('remote_addr');
        $verified_hostname = $client_ip ? \Cleantalk\Common\Helper::ipResolve($client_ip) : false;
        $is_noc_request = ! $apbct->key_is_ok &&
            Request::get('spbc_remote_call_action') &&
            in_array(Request::get('plugin_name'), array('antispam', 'anti-spam', 'apbct')) &&
            $verified_hostname !== false &&
-            in_array($verified_hostname, $rc_servers, true);
+            in_array($verified_hostname, self::RC_SERVERS, true);


alexander-b-clean · 2026-08-12T11:04:25Z

+        let forbiddenTags = ['SCRIPT', 'IFRAME', 'OBJECT', 'EMBED', 'BASE', 'LINK', 'META'];
+
+        template.content.querySelectorAll( '*' ).forEach( function( el ) {
+            if ( forbiddenTags.indexOf( el.tagName ) !== -1 ) {
+                el.remove();
+                return;
+            }


# Conflicts: # js/apbct-public-bundle.min.js # js/apbct-public-bundle_ext-protection.min.js # js/apbct-public-bundle_ext-protection_gathering.min.js # js/apbct-public-bundle_full-protection.min.js # js/apbct-public-bundle_full-protection_gathering.min.js # js/apbct-public-bundle_gathering.min.js # js/apbct-public-bundle_int-protection.min.js # js/apbct-public-bundle_int-protection_gathering.min.js # lib/Cleantalk/ApbctWP/RemoteCalls.php

Copilot

Pull request overview

Copilot reviewed 10 out of 20 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

js/src/common-cleantalk-modal.js:157

Links generated with target="_blank" should include rel="noopener noreferrer" to prevent reverse-tabnabbing (and it is already allowed by the sanitizer via the "rel" attribute allowlist).

                    this.loaded.replace(urlRegex, '<a href="$1" target="_blank">$1</a>'),

lib/Cleantalk/ApbctWP/RemoteCalls.php:145

The docstring says “an unauthorized request must never be able to trigger a DNS resolving”, but any external request can satisfy the “cheap checks” by sending spbc_remote_call_action + plugin_name, so the guarantee is stronger than what the code enforces. Rewording avoids misleading future maintainers about the threat model.

     * Performs DNS lookups, so it has to be called only after all the cheap checks passed -
     * an unauthorized request must never be able to trigger a DNS resolving.

alexander-b-clean · 2026-08-12T11:06:52Z

+    isSafeCss: function( value ) {
+        let normalized = String( value ).replace( /[\u0000-\u0020\u007F]+/g, '' ).toLowerCase();
+
+        return ! /(expression\(|javascript:|vbscript:|url\(|@import|-moz-binding|behavior:)/.test( normalized );
+    },


I think this is unnecessary.

AntonV1211

piggy bank comments if you decide, then click on Resolve conversation
The name of the commit consists of 3 blocks:

this is the general nature of the edits (example Mod, Fix, Upd, New),
where the edits were made (you can briefly specify the functionality - for example, SFW or the interface - for example, Settings, or if there are a lot of edits, you can simply write Code),
What exactly was done in the edits
Currently, neither the commits in the edits nor the general name correspond to the names that are allowed in the release changelog.

There is not even an attempt to make unit tests

datorik added 2 commits July 24, 2026 18:56

Code. Security review

e6a71e8

Code. Security review

62b4b6e

alexander-b-clean requested a review from Copilot July 24, 2026 17:40

Copilot started reviewing on behalf of alexander-b-clean July 24, 2026 17:41 View session

Copilot AI reviewed Jul 24, 2026

View reviewed changes

datorik added 2 commits August 10, 2026 13:44

Udp.Code.Security review

167e86c

Udp.Code.Security review. JS build

13e024f

alexander-b-clean requested review from AntonV1211 and Glomberg August 12, 2026 09:26

datorik added 2 commits August 12, 2026 13:37

Udp.Code.Security review.Resolved conflicts

7690b40

alexander-b-clean requested a lite review from Copilot August 12, 2026 10:47

Copilot started reviewing on behalf of alexander-b-clean August 12, 2026 10:47 View session

Copilot AI reviewed Aug 12, 2026

View reviewed changes

Glomberg approved these changes Aug 12, 2026

View reviewed changes

AntonV1211 requested changes Aug 13, 2026

View reviewed changes

Navigation Menu

Uh oh!

Security review - #838

Security review#838
alexander-b-clean wants to merge 6 commits into
devfrom
security-review.ab

alexander-b-clean commented Jul 24, 2026

Uh oh!

codecov Bot commented Jul 24, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

alexander-b-clean Aug 12, 2026

Uh oh!

Uh oh!

alexander-b-clean Aug 12, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

alexander-b-clean Aug 12, 2026

Uh oh!

AntonV1211 left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

alexander-b-clean commented Jul 24, 2026

Uh oh!

codecov Bot commented Jul 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

alexander-b-clean Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

alexander-b-clean Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

alexander-b-clean Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

AntonV1211 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov Bot commented Jul 24, 2026 •

edited

Loading