Skip to content

Harden auditor-reported output and term counting - #1235

Merged
repile[bot] merged 3 commits into
developmentfrom
codex/auditor-output-and-cache
Oct 14, 2026
Merged

repile[bot] merged 3 commits into
developmentfrom
codex/auditor-output-and-cache

Conversation

@stevejburge

Copy link
Copy Markdown
Member

Summary

  • sanitize SVG fragments with an explicit allowlist before rendering checklist tabs
  • escape generated hidden attribute text
  • avoid overwriting the WordPress $post_type global in the checklist template
  • replace cache-key base64_encode() calls with SHA-256 fingerprints
  • use current wp_count_terms() argument form
  • use wp_safe_redirect() for activation redirect

Auditor coverage

Addresses confirmed High Output Escaping, High Discouraged PHP Functions, Medium Deprecated Function Parameters, and one Medium Global Variable Override finding from publishpress-checklists-free-2.29.0-plugin-auditor-report.pdf.

The $submenu assignment is required by the custom_menu_order filter and remains unchanged.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-05T16:47:14.384589Z 675b757 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 675b757ad3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

$total_categories = get_transient($cache_key);
if ($total_categories === false) {
$total_categories = wp_count_terms('category', $args);
$total_categories = wp_count_terms(array_merge(['taxonomy' => 'category'], $args));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Keep term counting compatible with WordPress 5.5

On WordPress 5.5, which the plugin still declares as its minimum supported version, wp_count_terms() expects the taxonomy as its first argument and the query arguments as its second; the array-only signature was introduced in WordPress 5.6. Consequently, this call passes an array where 5.5 expects a taxonomy and breaks the AJAX response whenever the category selector calculates pagination. The same incompatible change appears in the prohibited/required tag and required-category implementations, so retain the legacy call on 5.5 or add a version-compatible wrapper.

Useful? React with 👍 / 👎.

@stevejburge stevejburge added this to the 2.30 milestone Oct 5, 2026
@repile

repile Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Review sweep before 2.30.0: verdict Not yet

Setup: fresh SandyWP sandboxes (WP 7.1.3, PHP 8.3, WP_DEBUG_LOG on, Yoast SEO active so the SVG tab icon renders), not cloned from any customer site. I installed development @ 358d914 first, then this PR @ 675b757, with 16 categories and 15 tags. The PR merges cleanly into current development, and all four sweep PRs apply together without conflicts.

On WP 7.1.3 everything works

Category picker (browser) Yoast SEO tab and SVG icon (browser)
picker yoast

Recording

Check development This PR
Category/tag AJAX (4 endpoints): page 1 / has_next / page 2 10 / true / 6 10 / true / 6 ✅
Count cache keys …_count_V29tZW5cJ3M= (base64) …_count_<sha256> ✅
Activation redirect (ppch_activated) 302 → admin.php?page=ppch-checklists same, via wp_safe_redirect ✅
Checklists page rows by data-post-type (post / page) 54 / 47 54 / 47 ✅ (template rename is safe)
Full Checklists page HTML diff – only the tab <svg> changes ✅
PHP debug log – no plugin entries ✅

wp_kses lowercases viewBox to viewbox and reduces the already-malformed xmlns="[http://…"](…") in core/Utils/FieldsTabs.php to //www.w3.org/2000/svg. HTML parsing maps viewbox back to viewBox, so the icon renders the same (screenshot above). The [http://…](…) markdown corruption in those FieldsTabs.php SVG strings is worth a separate cleanup.

Why "Not yet": the Codex P1 is real on WP 5.5

The plugin still declares Requires at least: 5.5. SandyWP can't create a WP 5.5 site, so I ran WP 5.5 core against the same DB and called both forms with the plugin's arguments:

WP 5.5:  wp_count_terms("category", $args)                           => '16'
WP 5.5:  wp_count_terms(array_merge(['taxonomy'=>'category'], $args)) => WP_Error invalid_taxonomy
has_next (page 1, 16 categories): legacy => true, this PR => false

So on WP 5.5 the category/tag pickers stop paginating after the first 10 terms, PHP logs an int-conversion warning, and the WP_Error gets cached in the transient. The legacy two-argument call still works on WP 7.1.3 with no deprecation notice. Fix: keep wp_count_terms('category', $args) (and 'post_tag'), or bump Requires at least to 5.6 in a deliberate release decision.

Verdict: Not yet, because of WP 5.5 pagination. Everything else checks out.

root and others added 2 commits October 14, 2026 19:34
wp_count_terms($args) returns WP_Error invalid_taxonomy before WP 5.6,
and the legacy two-argument form is deprecated afterwards. Also avoid
caching a WP_Error as the total.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@repile

repile Bot commented Oct 14, 2026

Copy link
Copy Markdown
Contributor

Follow-up: WP 5.5 term counting fixed, verdict Safe to merge

What I changed

  • 4248faf merges current development into this branch (no conflicts).
  • f6885b7: the four get_total_count() methods (Required/Prohibited categories and tags) now count with get_terms(array_merge($args, ['taxonomy' => …, 'fields' => 'count'])) instead of wp_count_terms(). This is the array form wp_count_terms() calls internally on 5.6+, and it has worked unchanged since WP 4.5. So it works on 5.5 (where wp_count_terms($args) returns invalid_taxonomy) and on current WP (where the two-argument form is deprecated). A WP_Error now becomes 0 instead of being cached in the transient.

How I tested

I used fresh SandyWP sandboxes, not cloned from any customer site, with 16 categories and 15 tags. I drove the real settings page in Chromium (playwright): the four AJAX endpoints with the page's nonce, then each select2 picker scrolled to the end.

SandyWP can't create a WP 5.5 site, so I created a PHP 7.4 sandbox and downgraded core in place with wp core download --version=5.5 (db_version 48748, Twenty Twenty 1.5). The test-only mu-plugin there loads wp-admin/includes/plugin.php early. That works around the unrelated #1238 fatal, which #1239 fixes.

AJAX page 1 → page 2 (all 4 endpoints) Options in each picker after scrolling debug.log Cached count
WP 5.5, PR before (675b757) 10 items, has_next=false ❌ 10 / 10 / 10 / 10 ❌ Object of class WP_Error could not be converted to int ×8 serialized WP_Error ❌
WP 5.5, this fix 10, has_next=true → 6 (tags 5), false ✅ 16 / 16 / 15 / 15 ✅ empty ✅ 16 / 15 ✅
WP 7.1.3 / PHP 8.3, this fix 10, has_next=true → 6 (tags 5), false ✅ 16 / 16 / 15 / 15 ✅ empty, no deprecation notice ✅ 16 / 15 ✅
WP 5.5 before: the list ends at "Cat 10" WP 5.5 after: it scrolls through to "Cat 15" and "Uncategorized"
screenshot · recording screenshot · recording

(The links need a Repile login.) php -l passes on all four files under PHP 7.4 and 8.3.

Verdict: ✅ Safe to merge. Pagination works from the declared minimum (5.5) through 7.1.3, and the rest of the PR checked out in the earlier review.

@repile
repile Bot merged commit 9bf2725 into development Oct 14, 2026
@rizaardiyanto1412
rizaardiyanto1412 deleted the codex/auditor-output-and-cache branch October 26, 2026 18:42
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.

2 participants