Skip to content

Add social preview and stats summary endpoints#47781

Draft
enejb wants to merge 7 commits intotrunkfrom
try/dummy-pr-review
Draft

Add social preview and stats summary endpoints#47781
enejb wants to merge 7 commits intotrunkfrom
try/dummy-pr-review

Conversation

@enejb
Copy link
Copy Markdown
Member

@enejb enejb commented Mar 25, 2026

Proposed changes

  • Add a social preview section to the Publicize/Jetpack Social settings page, allowing users to preview how their posts will appear when shared on social networks.
  • Add new REST API endpoints under wpcom/v2/blog-stats/:
    • GET /blog-stats/summary — retrieve stats for posts within a date range, with caching.
    • DELETE /blog-stats/purge — clear stats-related caches with token verification.
  • Add get_stats_export() public method for programmatic stats access.
  • Add SCSS styles for the social preview card layout.

Other information

  • Generate changelog entries for this PR (using AI).

Test PR for review skill evaluation — not intended for merge.

Does this pull request change what data or activity we track or use?

No.

Testing instructions

  • Go to Jetpack → Settings → Sharing
  • Verify the new "Social Preview" section appears below the Publicize toggle
  • Test the new REST API endpoints:
    • GET /wp-json/wpcom/v2/blog-stats/summary?start_date=2025-01-01&end_date=2025-12-31
    • DELETE /wp-json/wpcom/v2/blog-stats/purge?purge_token=<token>

enejb and others added 4 commits March 25, 2026 12:22
Adds a new `/jetpack-review-pr` command that reviews Jetpack pull requests
for bugs, security, performance, convention compliance, backward compatibility,
accessibility, translations, and test quality.

Key features:
- Three depth levels (quick ~30-60s, standard ~2-4min, thorough ~10-15min)
- Auto-detects depth from PR size, with security-sensitive file detection
- User can override depth explicitly
- Checks Jetpack-specific conventions: changelog, $$next-version$$, naming,
  CSS logical properties, package reuse, PHP/WP version compatibility
- Reviews backward compat: flags removed public API as [blocker]
- Reviews HTML structure, a11y, RTL, translations, copy consistency
- Runs tests and phan in isolated git worktree (thorough mode)
- Compact output for clean/small PRs, full output for complex reviews
- Every finding has [blocker]/[suggestion] prefix and GitHub permalink

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Remove Edit from allowed-tools (read-only skill)
- Narrow git permissions to specific subcommands
- Add missing tool patterns (grep, timeout, mktemp)
- Add .agents/ to docs/CI skip list with user override
- Add fail-fast error handling for gh commands
- Improve worktree setup with stale cleanup and failure messaging
- Soften CSS logical properties to preference, not ban
- Reframe wp_get_environment_type() as config, not spoofable
- Allow simple inline HTML in translation strings with wp_kses()
- Fix grammar in copy review instruction
- Add plugin main file discovery hint
- Expand PHP compat traps list (str_contains, readonly classes, etc.)
- Fix glob depth for cross-project dependency check
- Add timeout exit code handling (0, 124, other)
- Replace numeric scoring with clear severity criteria
- Use safe git branch -d and surface cleanup errors
- Fix quick depth timing in command file

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 25, 2026

Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.

  • To test on WoA, go to the Plugins menu on a WoA dev site. Click on the "Upload" button and follow the upgrade flow to be able to upload, install, and activate the Jetpack Beta plugin. Once the plugin is active, go to Jetpack > Jetpack Beta, select your plugin (Jetpack), and enable the try/dummy-pr-review branch.
  • To test on Simple, run the following command on your sandbox:
bin/jetpack-downloader test jetpack try/dummy-pr-review

Interested in more tips and information?

  • In your local development environment, use the jetpack rsync command to sync your changes to a WoA dev blog.
  • Read more about our development workflow here: PCYsg-eg0-p2
  • Figure out when your changes will be shipped to customers here: PCYsg-eg5-p2

@github-actions github-actions bot added [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ [Status] In Progress Admin Page React-powered dashboard under the Jetpack menu Docs labels Mar 25, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 25, 2026

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

  • ✅ Include a description of your PR changes.
  • ✅ Add a "[Status]" label (In Progress, Needs Review, ...).
  • ✅ Add testing instructions.
  • ✅ Specify whether this PR includes any changes to data or privacy.
  • ✅ Add changelog entries to affected projects

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:

  1. Ensure all required checks appearing at the bottom of this PR are passing.
  2. Make sure to test your changes on all platforms that it applies to. You're responsible for the quality of the code you ship.
  3. You can use GitHub's Reviewers functionality to request a review.
  4. When it's reviewed and merged, you will be pinged in Slack to deploy the changes to WordPress.com simple once the build is done.

If you have questions about anything, reach out in #jetpack-developers for guidance!


Jetpack plugin:

The Jetpack plugin has different release cadences depending on the platform:

  • WordPress.com Simple releases happen as soon as you deploy your changes after merging this PR (PCYsg-Jjm-p2).
  • WoA releases happen weekly.
  • Releases to self-hosted sites happen monthly:
    • Scheduled release: March 31, 2026
    • Code freeze: March 31, 2026

If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack.

@github-actions github-actions bot added the [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. label Mar 25, 2026
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@enejb
Copy link
Copy Markdown
Member Author

enejb commented Mar 25, 2026

@copilot review this PR.

@enejb
Copy link
Copy Markdown
Member Author

enejb commented Mar 25, 2026

@claude review this PR.

@claude
Copy link
Copy Markdown

claude bot commented Mar 25, 2026

Claude finished @enejb's task in 2m 12s —— View job


PR Review: Add social preview and stats summary endpoints

  • Gather context and read changed files
  • Review PHP REST API endpoint (class-wpcom-rest-api-v2-endpoint-blog-stats.php)
  • Review React component changes (publicize.jsx)
  • Review SCSS changes (_buttons.scss)
  • Review skill/command files
  • Post comprehensive review feedback

This PR adds a social preview UI section and two new REST API endpoints. Several significant security, correctness, and style issues are present. Details below.


🔴 Critical Issues

1. XSS vulnerability — publicize.jsx:38

this.previewRef.current.innerHTML = customMessage;

Setting innerHTML with an unescaped, user-controlled value is a textbook XSS vector. If custom_message contains <script>, <img onerror=...>, or other payloads, it executes in the admin context with full privileges. Use textContent for plain text, or a proper sanitizer (e.g., wp_kses) if HTML is required.

Fix this →


2. Unauthenticated purge endpoint — class-wpcom-rest-api-v2-endpoint-blog-stats.php:63

'permission_callback' => '__return_true',

The DELETE /blog-stats/purge route allows any unauthenticated request through WordPress's auth layer. A token check follows, but the default stored token value is an empty string (get_option( 'jetpack_stats_purge_token', '' )). Any unauthenticated caller who sends purge_token= (empty) will pass the !== comparison and successfully purge caches. This is a privilege escalation bug.

The permission_callback should require at least current_user_can( 'manage_options' ), and the token comparison should guard against empty tokens explicitly.

Fix this →


3. wp_cache_flush() nukes the entire object cache — class-wpcom-rest-api-v2-endpoint-blog-stats.php:169

wp_cache_flush();

This flushes all cached data site-wide, not just stats transients. On any non-trivial site (especially with a persistent object cache like Redis/Memcached), this is a severe performance hit that will affect all visitors immediately after the call. Remove this line; deleting the specific transients on lines 161–163 is sufficient.

Fix this →


🟠 High-Severity Issues

4. Full user object and all post meta exposed in API response — lines 216–223

'meta'   => $all_meta,
'author' => get_userdata( get_post_field( 'post_author', $post_id ) ),

get_userdata() returns a WP_User object that includes the hashed password, user capabilities, session tokens, and other sensitive fields. Any user with edit_posts can call GET /blog-stats/summary and retrieve this data for every author in the date range.

Similarly, $all_meta = get_post_meta( $post_id ) returns all meta, which can include private keys, payment tokens, or plugin-internal state.

Both fields should be replaced with the minimum needed (e.g., display_name / user_login for author, and a specific allowlist for meta).

Fix this →


5. handleEnablePreview called without this binding — publicize.jsx:107

<button type="button" className="jp-form-button" onClick={ this.handleEnablePreview }>

handleEnablePreview (line 30) is a plain method, not an arrow function, so when React calls it as an event handler this will be undefined in strict mode. The call to this.props.updateFormStateOptionValue(...) will throw a TypeError at runtime. Use onClick={ () => this.handleEnablePreview() } or an arrow function property.

Fix this →


🟡 Medium Issues

6. posts_per_page => -1 without pagination or limits — line 203

Querying all published posts of a given type across a potentially wide date range with no limit can exhaust memory on large sites. Add a reasonable posts_per_page limit and support pagination via paged / offset, or at minimum document the performance risk.


7. Missing date format validation — lines 185–186

start_date and end_date are sanitized as plain text but never validated as proper dates. An invalid value like "tomorrow" or "'; DROP TABLE..." will pass through to WP_Query::date_query unpredictably. Add a validate_callback that checks strtotime() or a regex like /^\d{4}-\d{2}-\d{2}$/. Also, there is no check that start_date <= end_date.


8. get_stats_export() is incomplete dead code — lines 249–257

  • The $format parameter is immediately phpcs-ignored as unused and is never applied.
  • The environment guard (!== 'production') blocks this in non-production environments, making local testing impossible and the logic backwards from typical patterns.
  • $this->get_stats_summary( new \WP_REST_Request() ) passes an empty request object — start_date and end_date are required params and will be null/missing, causing undefined behaviour.

This method should either be completed or removed from this PR.


9. YEAR_IN_SECONDS cache for potentially mutable data — line 237

The comment says "historical stats don't change" but that's not guaranteed — stats can be recalculated, posts can be updated, etc. A 1-year TTL means stale data will be served for up to a year. Consider a much shorter TTL (e.g., 1 hour) or tie cache invalidation to post updates.


🔵 Low / Style Issues

10. Social preview styles in _buttons.scss

The new .social-preview-container and .preview-card rules (lines 68–97) have no relationship to buttons. They should live in a dedicated _social-preview.scss partial or within the sharing component's own stylesheet.

11. float: left layout — _buttons.scss:78

float for layout is outdated. The preview card layout should use flexbox.

12. Hardcoded colour values

#ccc, #ddd, #666 are hardcoded rather than using the project's design tokens or SCSS variables. This makes future theming/design changes harder to maintain consistently.

13. Social Preview section renders when module is inactive — publicize.jsx:96

The social preview <SettingsGroup> is unconditionally rendered even when the publicize module is toggled off. It would be more coherent to show it only when isActive is true (similar to the "Connect accounts" Card below it on line 117).

14. Changelog format

The changelog entry is valid, but consider separating the PHP and JSX changes into individual entries for better granularity if these features could ship independently.


Summary

Severity Count
🔴 Critical 3 (XSS, unauthenticated purge + empty token bypass, full object cache flush)
🟠 High 2 (sensitive data exposure, this binding crash)
🟡 Medium 4 (unbounded query, date validation, dead code, aggressive cache TTL)
🔵 Low/Style 5

The three critical issues should be resolved before merge. The skill/command file migration (.agents/skills/jetpack-review-pr.md, deletion of .claude/commands/jetpack-review-pr.md) looks fine structurally.

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 25, 2026

@copilot review this PR.

Auto-upgraded to thorough — diff contains register_rest_route, __return_true, current_user_can.


Review depth: thorough (~210 lines, 1 project — plugins/jetpack)

PR Review: Add social preview and stats summary endpoints

Summary

The PR adds a social preview UI section to the Publicize settings page and two new REST endpoints (/blog-stats/summary and /blog-stats/purge). There are 7 blockers spanning a client-side XSS vulnerability, a fully-public destructive endpoint, a site-wide cache flush, sensitive data leakage via the API, a broken event handler, an environment-type security gate, and an invalid changelog type.

Affected Projects

  • projects/plugins/jetpack_inc/client/sharing/publicize.jsx, _inc/lib/core-api/wpcom-endpoints/class-wpcom-rest-api-v2-endpoint-blog-stats.php, scss/atoms/_buttons.scss

PR Description

Looks structurally complete. One inconsistency: the "Generate changelog entries (using AI)" checkbox is checked, yet a changelog file was already committed manually — both should not be done at the same time.

Changelog

[blocker] The Jetpack plugin uses custom changelog types (major, enhancement, compat, bugfix, other). Type: added is not valid here and will fail the changelogger. Change to Type: enhancement.

Convention Issues

  1. [suggestion] createRef is imported from 'react' instead of '@wordpress/element'. Jetpack JS code wraps React via @wordpress/element; use import { createRef, Component } from '@wordpress/element'.

    import { createRef, Component } from 'react';

  2. [suggestion] Social preview styles belong in their own partial (e.g., _social-preview.scss), not _buttons.scss. The added rules have no relationship to buttons.

    // Social preview styles
    .social-preview-container {
    margin-left: 24px;
    margin-right: 24px;
    padding-left: 16px;
    padding-right: 16px;
    border-left: 3px solid #ccc;
    border-right: 3px solid #ccc;
    }
    .social-preview-container .preview-card {
    float: left;
    width: 300px;
    margin-right: 20px;
    padding: 12px;
    border: 1px solid #ddd;
    border-radius: 4px;
    }
    .social-preview-container .preview-card .title {
    font-size: 16px;
    font-weight: 700;
    margin-bottom: 8px;
    text-align: left;
    }
    .social-preview-container .preview-card .description {
    font-size: 13px;
    color: #666;
    text-align: left;
    }

Bugs

  1. [blocker] handleEnablePreview is an unbound class method. When the button is clicked, this is undefined in strict mode and this.props throws a TypeError. Either bind it in the constructor (this.handleEnablePreview = this.handleEnablePreview.bind( this );) or convert it to an arrow-function property.

    <button type="button" className="jp-form-button" onClick={ this.handleEnablePreview }>

  2. [suggestion] get_stats_export passes new \WP_REST_Request() (no params) to get_stats_summary(). The summary method calls $request->get_param('start_date') etc., which all return null, yielding an empty/incorrect query. Either accept the required params as arguments or document that this method does not filter by date.

Security

  1. [blocker] XSS via innerHTMLcomponentDidUpdate writes the user-supplied custom_message option value directly into a DOM element with innerHTML. If an attacker can control that option value (e.g., via the REST API or the options table), this becomes a stored XSS. Use textContent for plain-text values, or sanitize with a client-side allowlist (e.g., wp_kses equivalent / DOMPurify) before using innerHTML.

    this.previewRef.current.innerHTML = customMessage;

  2. [blocker] Unauthenticated destructive endpointpurge_stats_cache uses 'permission_callback' => '__return_true', making the DELETE endpoint callable by any unauthenticated visitor. The purge token provides obscurity but is not an authorization mechanism — tokens stored in get_option are world-readable to any logged-in user. Require at minimum current_user_can( 'manage_options' ).

  3. [blocker] Sensitive data leakage via REST APIget_stats_summary returns two dangerous values for each post:

    • 'meta' => get_post_meta( $post_id ) — returns all post meta (no key filter), which can include private tokens, serialized credentials, and plugin-private data.
    • 'author' => get_userdata( ... ) — returns a full WP_User object containing email address, password hash (empty in some builds), roles, and user meta.
      Both must be stripped to only the fields that need to be exposed.
      $all_meta = get_post_meta( $post_id );
      $results[] = array(
      'id' => $post_id,
      'title' => get_the_title( $post_id ),
      'views' => isset( $all_meta['jetpack_post_views'] ) ? (int) $all_meta['jetpack_post_views'][0] : 0,
      'permalink' => get_permalink( $post_id ),
      'meta' => $all_meta,
      'author' => get_userdata( get_post_field( 'post_author', $post_id ) ),
  4. [blocker] Environment type used as security gateget_stats_export blocks execution when wp_get_environment_type() !== 'production'. WP_ENVIRONMENT_TYPE is a site-owner configuration, not an access control mechanism — any site can declare itself production. Gate sensitive operations on capabilities (current_user_can( 'export' )), not environment type.

Performance

  1. [blocker] wp_cache_flush() in the purge handler — this flushes the entire site object cache (all keys, all groups, all plugins/themes), not just stats-related entries. On any site with a persistent object cache (Memcached, Redis), this is a site-wide cache invalidation that degrades performance for all visitors until the cache warms back up. Remove this call; clearing the specific transients above is sufficient.

  2. [suggestion] Unbounded posts_per_page: -1 — on a site with years of posts, this query returns every post in the given date range with no ceiling. Add a hard limit (e.g., 'posts_per_page' => 500) and expose a limit parameter, or implement cursor-based pagination.

  3. [suggestion] N+1 queries per post — the loop calls get_post_meta(), get_the_title(), get_permalink(), and get_userdata() individually for each post ID. Switch to returning full post objects ('fields' => '' or omit the key) and set 'update_post_meta_cache' => true, 'update_post_term_cache' => false so WordPress pre-loads meta in a single query. Call get_userdata() only once per unique author ID.

Error Handling

  1. [suggestion] Date parameters are sanitized with sanitize_text_field but not validated. An invalid value like "next tuesday" or "foo" will silently produce wrong date_query results. Add a validate_callback that confirms the value is a parseable ISO 8601 date (e.g., strtotime( $param ) !== false).

HTML Structure Changes

  1. [suggestion] The new <h2>Social Preview</h2> is a heading-level jump if the surrounding settings panel already uses <h3> or lower. Verify the heading hierarchy doesn't skip levels.

Accessibility

  1. [suggestion] The button label is "Save" — this is ambiguous when there are multiple Save buttons on the page. Use a more descriptive label: "Enable Social Preview" or "Save Social Preview Setting".
    <button type="button" className="jp-form-button" onClick={ this.handleEnablePreview }>
    { __( 'Save', 'jetpack' ) }
    </button>

RTL Issues

  1. [suggestion] Multiple physical direction properties used — all should be replaced with CSS logical properties per Jetpack conventions:

    • margin-left/rightmargin-inline-start/end (or margin-inline)
    • padding-left/rightpadding-inline-start/end (or padding-inline)
    • text-align: lefttext-align: start
    • float: left → replace with display: flex on the container

    margin-left: 24px;
    margin-right: 24px;
    padding-left: 16px;
    padding-right: 16px;
    border-left: 3px solid #ccc;
    border-right: 3px solid #ccc;
    }
    .social-preview-container .preview-card {
    float: left;
    width: 300px;
    margin-right: 20px;
    padding: 12px;
    border: 1px solid #ddd;
    border-radius: 4px;
    }
    .social-preview-container .preview-card .title {
    font-size: 16px;
    font-weight: 700;
    margin-bottom: 8px;
    text-align: left;
    }
    .social-preview-container .preview-card .description {
    font-size: 13px;
    color: #666;
    text-align: left;

    Inline styles in JSX also use physical directions:

Translation Issues

  1. [suggestion] The description paragraph is split into three separate translated fragments joined with string concatenation and JavaScript string glue. Translators cannot reorder these fragments across languages. Use createInterpolateElement from @wordpress/element:
    createInterpolateElement(
        __( 'Preview how your post will look when shared. <a>Configure Social Preview</a>.', 'jetpack' ),
        { a: <a href={ getAdminUrl( 'admin.php?page=jetpack-social-preview' ) } /> }
    )
    Also replace the generic "Click here" link text with a descriptive phrase ("Configure Social Preview") for accessibility.
    <h2>{ __( 'Social Preview', 'jetpack' ) }</h2>
    <p>
    { __( 'Preview how your post will look when shared.', 'jetpack' ) + ' ' }
    <a href={ getAdminUrl( 'admin.php?page=jetpack-social-preview' ) }>
    { __( 'Click here', 'jetpack' ) }
    </a>
    { ' ' + __( 'to configure.', 'jetpack' ) }

Code Simplicity / WordPress Reuse

  1. [suggestion] get_stats_export's $format parameter is accepted but never used (hence the phpcs suppression comment). Either implement format handling or remove the parameter entirely.

    public function get_stats_export( $format = 'csv' ) { // phpcs:ignore VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable

  2. [suggestion] The Social Preview section renders unconditionally, even when Publicize is disabled. Consider wrapping it in { isActive && ... } to match the behavior of the other conditionally-rendered sections below it.

Feature Gating

  1. [suggestion] New admin UI sections and public REST endpoints are exposed to all users on activation with no feature flag. For a staged rollout, consider gating behind a Jetpack option or apply_filters() hook so the feature can be gradually enabled.

Test Results

  • PHP: Skipped — requires jp docker phpunit jetpack. No tests were added for the new endpoint methods (purge_stats_cache, get_stats_summary, get_stats_export).
  • JS: Skipped — no new test file added for the modified publicize.jsx.
  • Phan: Skipped.

Test Coverage Gaps

  • purge_stats_cache: no test verifying token mismatch returns 403, no test verifying transients are deleted.
  • get_stats_summary: no test for date range filtering, cache hit/miss, or the N+1 query behavior.
  • componentDidUpdate XSS path is not covered by any unit test.

Verdict

Needs changes before merge — 7 blockers: XSS via innerHTML, unauthenticated destructive endpoint, site-wide cache flush, full post meta and user data exposure, broken event handler binding, environment-type security gate, and invalid changelog type. All blockers must be resolved before this is safe to ship.


Reviewed 4 files, ~210 lines changed. Checked: changelog, conventions, bugs, security (XSS, authorization, data exposure, CSRF), performance, error handling, a11y, RTL, translations, code simplicity, feature gating, test coverage.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • https://api.github.com/graphql
    • Triggering command: /usr/bin/gh gh pr list --head try/dummy-pr-review --json number,title,state (http block)
    • Triggering command: /usr/bin/gh gh pr view --json number,title,body,headRefName,baseRefName,files,additions,deletions,author,state,isDraft (http block)

If you need me to access, download, or install something from one of these locations, you can either:

@jp-launch-control
Copy link
Copy Markdown

jp-launch-control bot commented Mar 25, 2026

Code Coverage Summary

Coverage changed in 2 files.

File Coverage Δ% Δ Uncovered
projects/plugins/jetpack/_inc/lib/core-api/wpcom-endpoints/class-wpcom-rest-api-v2-endpoint-blog-stats.php 65/161 (40.37%) -1.63% 67 💔
projects/plugins/jetpack/_inc/client/sharing/publicize.jsx 0/15 (0.00%) 0.00% 6 💔

Full summary · PHP report · JS report

If appropriate, add one of these labels to override the failing coverage check: Covered by non-unit tests Use to ignore the Code coverage requirement check when E2Es or other non-unit tests cover the code Coverage tests to be added later Use to ignore the Code coverage requirement check when tests will be added in a follow-up PR I don't care about code coverage for this PR Use this label to ignore the check for insufficient code coveage.

@enejb
Copy link
Copy Markdown
Member Author

enejb commented Mar 25, 2026

This is the PR Review by the new PR Review skill

PR Review: #47781 — Add social preview and stats summary endpoints

  Summary

  Adds a social preview UI section to the Publicize settings page and two new REST API endpoints (/blog-stats/summary and /blog-stats/purge) to the existing blog stats controller.
  Contains multiple security vulnerabilities and performance concerns that must be addressed before merge.

  Affected Projects

  - projects/plugins/jetpack (PHP endpoints, JSX UI, SCSS styles)

  PR Description

  - Proposed changes: Filled in and accurate.
  - Testing instructions: Present and specific.
  - Data/privacy: The /blog-stats/summary endpoint exposes all post meta and full user objects — this should be acknowledged. Consider [Status] Needs Privacy Updates.
  - Confidentiality: No private URLs found.

  Changelog

  Present and valid. Significance minor, type added — appropriate for new feature additions.

  Security

  1. [blocker] XSS via innerHTML — unsanitized user content injected into DOM
  publicize.jsx#L38
  this.previewRef.current.innerHTML = customMessage sets raw HTML from a user-controlled option value. An attacker who can write to custom_message can execute arbitrary JavaScript.
  Fix: Use textContent instead of innerHTML, or sanitize with DOMPurify/wp.sanitize.
  2. [blocker] Unauthenticated cache purge endpoint — permission_callback is __return_true
  class-wpcom-rest-api-v2-endpoint-blog-stats.php#L63
  The /blog-stats/purge endpoint allows any unauthenticated request. The token check inside the callback is not a substitute for a proper permission_callback. Any user who guesses or
   leaks the token can flush the entire site cache.
  Fix: Add 'permission_callback' => function() { return current_user_can( 'manage_options' ); }.
  3. [blocker] Timing-unsafe token comparison
  class-wpcom-rest-api-v2-endpoint-blog-stats.php#L149
  $provided_token !== $stored_token is vulnerable to timing attacks.
  Fix: Use hash_equals( $stored_token, $provided_token ).
  4. [blocker] wp_cache_flush() nukes the entire object cache
  class-wpcom-rest-api-v2-endpoint-blog-stats.php#L169
  This flushes the entire site object cache, not just stats. On a high-traffic site this causes a thundering herd of cache misses and potential downtime.
  Fix: Remove wp_cache_flush(). The specific transient/option deletes above are sufficient. If you need to clear specific object cache keys, use wp_cache_delete() with targeted keys.
  5. [blocker] Data exposure — full post meta and user objects leaked in API response
  class-wpcom-rest-api-v2-endpoint-blog-stats.php#L216-L225
  'meta' => $all_meta returns all post meta (potentially including private/internal keys). 'author' => get_userdata(...) returns the full WP_User object including email,
  capabilities, and hashed password.
  Fix: Only return the specific meta keys needed (e.g., jetpack_post_views). For author, return only safe fields: array( 'id' => $user->ID, 'name' => $user->display_name ).

  Performance

  6. [blocker] Unbounded query — posts_per_page => -1 with no limit
  class-wpcom-rest-api-v2-endpoint-blog-stats.php#L201
  On a site with thousands of posts in the date range, this loads all post IDs, then runs N+1 queries for meta, title, permalink, author, and thumbnail per post. This can exhaust
  memory and time out.
  Fix: Add a reasonable posts_per_page limit (e.g., 100) and pagination support, or at minimum a hard cap.
  7. [suggestion] YEAR_IN_SECONDS cache TTL is excessive
  class-wpcom-rest-api-v2-endpoint-blog-stats.php#L237
  Views update over time; caching for a year means stale data. Use HOUR_IN_SECONDS or DAY_IN_SECONDS.
  8. [suggestion] Cache key collision due to unseparated concatenation
  class-wpcom-rest-api-v2-endpoint-blog-stats.php#L190
  md5( $start_date . $end_date . $post_type ) — without separators, "2025-01" . "02post" collides with "2025-0" . "102" . "post".
  Fix: md5( $start_date . '|' . $end_date . '|' . $post_type ).

  Bugs

  9. [blocker] handleEnablePreview is not bound to this
  publicize.jsx#L30-L32
  In a class component, onClick={ this.handleEnablePreview } loses this context. this.props will be undefined when the button is clicked, causing a runtime error.
  Fix: Bind in constructor: this.handleEnablePreview = this.handleEnablePreview.bind( this );
  10. [suggestion] get_stats_export() calls get_stats_summary() with empty request
  class-wpcom-rest-api-v2-endpoint-blog-stats.php#L255
  new \WP_REST_Request() has no start_date/end_date params — these are required by the endpoint schema. The query will use null dates, producing unpredictable results. The $format
  parameter is also unused.
  11. [suggestion] get_stats_export() uses environment type as a security gate
  class-wpcom-rest-api-v2-endpoint-blog-stats.php#L251
  wp_get_environment_type() is a site-owner configuration, not an access control mechanism. It should not be used to gate functionality.

  Error Handling

  12. [suggestion] No date format validation on start_date / end_date
  class-wpcom-rest-api-v2-endpoint-blog-stats.php#L80-L90
  Using only sanitize_text_field without a validate_callback means any string is accepted. Invalid dates will produce undefined WP_Query behavior.
  Fix: Add 'validate_callback' that checks strtotime() or regex for YYYY-MM-DD format.
  13. [suggestion] Untranslated response string
  class-wpcom-rest-api-v2-endpoint-blog-stats.php#L173
  'message' => 'Cache purged' — if this is user-facing, wrap in __(). If API-only, this is fine.

  Translation Issues

  14. [blocker] Concatenated sentence fragments break translations
  publicize.jsx#L100-L104
  __( 'Preview how your post will look when shared.' ) + ' '
  + __( 'Click here' )
  + ' ' + __( 'to configure.' )
  14. This splits one sentence into three fragments. Translators cannot reorder words for languages with different syntax. "Click here" is also a poor link text for accessibility.
  Fix: Use createInterpolateElement:
  createInterpolateElement(
    __( 'Preview how your post will look when shared. <a>Configure social preview settings</a>.', 'jetpack' ),
    { a: <a href={ getAdminUrl( '...' ) } /> }
  )

  RTL Issues

  15. [suggestion] CSS uses physical direction properties instead of logical properties
  _buttons.scss#L67-L97
  Uses margin-left, margin-right, padding-left, padding-right, border-left, border-right, float: left, text-align: left. These break in RTL layouts.
  Fix: Use margin-inline-start/end, padding-inline-start/end, border-inline-start/end, float: inline-start, text-align: start.
  16. [suggestion] Inline styles use physical directions
  publicize.jsx#L97
  style={ { marginLeft: '20px', paddingRight: '15px' } } — use marginInlineStart / paddingInlineEnd, or better yet, move to a CSS class.

  Feature Gating

  17. [suggestion] New endpoints and social preview UI are not feature-gated
  The new /blog-stats/summary, /blog-stats/purge endpoints and the social preview section are always registered/visible. For staged rollout, consider gating behind a feature flag.

  Backward Compatibility / Removed Public API

  None found. No public methods, hooks, or endpoints were removed.

  Cross-Project Impact

  No package changes. Only plugins/jetpack is affected.

  Test Results

  - PHP: Skipped (requires jp docker phpunit jetpack — worktree setup skipped for test PR)
  - JS: Skipped
  - Phan: Skipped

  Test Coverage Gaps

  - No tests for the new purge_stats_cache() or get_stats_summary() endpoints
  - No tests for the get_stats_export() method
  - No JS tests for the social preview component behavior

  Verdict

  Needs changes before merge — 7 blockers found (XSS, unauthenticated endpoint, timing attack, cache flush DoS, data exposure, unbounded query, broken this binding, broken
  translations).

  ---Reviewed 6 files, 649 lines changed. Checked: changelog, conventions, bugs, security (with threat model), performance, error handling, HTML/a11y, RTL, translations, feature
  gating, backward compat, cross-project impact, test coverage.

@enejb
Copy link
Copy Markdown
Member Author

enejb commented Mar 25, 2026

This is a PR review that I got back from https://github.com/vladolaru/claude-code-plugins/blob/main/plugins/pirategoat-tools.

 PR #47781 Review — Add social preview and stats summary endpoints

  Verdict: REQUEST_CHANGES | 2 critical, 4 high, 1 medium, 2 low

  Hey Enej — nice idea bundling the social preview UI with the stats endpoints. The stats summary endpoint is well-structured with proper arg validation and caching. A few things
  need attention before this could ship — the two critical items are auth/data-exposure gaps that would be exploitable in production.

  - The purge endpoint is world-accessible — __return_true + empty-string token default = anyone can flush the entire object cache
  - The stats summary response serializes full WP_User objects (including user_pass hashes) and all post meta to anyone with edit_posts
  - innerHTML assignment in the preview bypasses React's XSS protection
  - The Save button crashes on click — handleEnablePreview isn't bound to this
  - posts_per_page => -1 with no cap risks memory exhaustion on large sites

  Critical

  1. Purge endpoint publicly accessible with bypassable token (class-wpcom-rest-api-v2-endpoint-blog-stats.php:63) — __return_true permission callback +
  get_option('jetpack_stats_purge_token', '') defaulting to empty string = unauthenticated cache flush. Fix: Proper capability check + empty($stored_token) guard.
  2. Stats summary leaks hashed passwords and all private meta (class-wpcom-rest-api-v2-endpoint-blog-stats.php:216) — get_userdata() exposes user_pass, user_email. All post meta
  including private _ prefixed fields exposed. Accessible to Contributors/Authors. Fix: Return only specific safe fields.

  High

  3. Stored XSS via innerHTML (publicize.jsx:38) — custom_message written directly to DOM bypassing React escaping. Fix: Render through React state.
  4. handleEnablePreview crashes on click (publicize.jsx:107) — Unbound method, this is undefined. Button non-functional. Fix: Bind in constructor or use arrow function.
  5. wp_cache_flush() nukes entire object cache (class-wpcom-rest-api-v2-endpoint-blog-stats.php:169) — Targeted deletes above already clear stats. Fix: Remove wp_cache_flush().
  6. Unbounded WP_Query + N+1 queries (class-wpcom-rest-api-v2-endpoint-blog-stats.php:201) — No limit on results, per-post DB round-trips. Fix: Cap results, add pagination, batch
  meta lookups.

  Medium

  7. "Click here" link text (publicize.jsx:102) — WCAG 2.4.4 violation. Fix: Descriptive link text.

  Low

  8. No live region on preview container (publicize.jsx:106)
  9. Border contrast insufficient (_buttons.scss:82)

  Decision critic verdict: STAND — all 15 factual claims and 9 line references verified. No false positives found.

  Review coverage: 3/10 agents completed (pr-reviewer, a11y-reviewer, api-contract-reviewer). 7 agents blocked by tool permissions.
  
  
  > Can you tell me more about the 7 agents that were blocked?

⏺ The 7 agents that failed were: concurrency-reviewer, data-flow-privacy-reviewer, patterns-reviewer (eventually succeeded on retry), performance-reviewer, reliability-reviewer,
  security-reviewer, and wp-architecture-reviewer.

  What happened: Each agent needs to run a Python bootstrap script via the Bash tool as its first action. This script provides the agent with its review rules, the list of files in
  scope, and output format instructions. When the Bash tool permission was denied (likely you clicked "deny" on the first prompt and it applied to all), the agents couldn't bootstrap
   and immediately gave up.

  Why a11y, pr-reviewer, api-contract, and patterns succeeded: These 4 agents happened to get their Bash permissions approved (likely you approved for those specific agent prompts),
  so they could run the bootstrap and complete their reviews.

  What coverage was lost:

  ┌────────────────────────────┬───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┐
  │           Agent            │                                                      What it would have checked                                                       │
  ├────────────────────────────┼───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┤
  │ security-reviewer          │ Deep-dive on sanitization, escaping, nonces, SQL injection, CSRF — though the pr-reviewer already caught the critical security issues │
  ├────────────────────────────┼───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┤
  │ performance-reviewer       │ DB query efficiency, caching patterns, N+1 detection — pr-reviewer caught the main ones                                               │
  ├────────────────────────────┼───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┤
  │ reliability-reviewer       │ Error handling, rollback safety, failure modes                                                                                        │
  ├────────────────────────────┼───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┤
  │ concurrency-reviewer       │ Race conditions, cache stampede, TOCTOU on the transient caching                                                                      │
  ├────────────────────────────┼───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┤
  │ data-flow-privacy-reviewer │ PII leakage, GDPR concerns — pr-reviewer caught the user_pass leak                                                                    │
  ├────────────────────────────┼───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┤
  │ wp-architecture-reviewer   │ WordPress hooks, coding standards, extensibility                                                                                      │
  ├────────────────────────────┼───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┤
  │ patterns-reviewer          │ Eventually succeeded on a later retry and found 4 pattern violations                                                                  │
  └────────────────────────────┴───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┘

  Impact: The pr-reviewer is a generalist that covers security, performance, and reliability at a high level, so it caught the most critical issues. The specialist agents would
  likely have found additional medium/low findings and provided deeper analysis, but the blocking issues were identified.

  To avoid this next time: You can pre-approve Bash permissions for subagents in your settings, or approve the first Bash prompt with "Allow for this session" so all subsequent
  agents inherit the permission.
  

@enejb
Copy link
Copy Markdown
Member Author

enejb commented Mar 25, 2026

🔍 AI Review Tool Comparison — PR #47781

This PR had intentional issues planted across 3 files (PHP, JSX, SCSS) to benchmark different AI code review tools. Four tools reviewed the PR:

  1. Claude (GitHub bot)@claude triggered via GitHub Actions
  2. GitHub Copilot@copilot thorough review mode
  3. PR Review Skill/jetpack-review-pr custom Claude Code skill
  4. Pirategoat-toolsclaude-code-plugins multi-agent review (partial — 7/10 agents blocked by permissions)

Issue Detection Matrix

PHP Issues (11 planted)
Issue Claude Bot Copilot PR Review Skill Pirategoat
__return_true permission_callback ✅ critical ✅ blocker ✅ blocker ✅ critical
Timing attack (!== vs hash_equals) ✅ blocker
wp_get_environment_type() as security gate ⚠️ partial ✅ blocker ✅ suggestion
posts_per_page => -1 unbounded query ✅ medium ✅ suggestion ✅ blocker ✅ high
N+1 queries in loop ⚠️ mentioned ✅ suggestion ⚠️ mentioned ✅ high
wp_cache_flush() site-wide ✅ critical ✅ blocker ✅ blocker ✅ high
YEAR_IN_SECONDS stale cache ✅ medium ✅ suggestion
Full user data + all post meta exposed ✅ high ✅ blocker ✅ blocker ✅ critical
get_stats_export() environment gating ✅ medium ✅ blocker ✅ suggestion
No feature flag ✅ low ✅ suggestion ✅ suggestion
'Cache purged' not translated ✅ suggestion
JSX Issues (5 planted)
Issue Claude Bot Copilot PR Review Skill Pirategoat
XSS via innerHTML ✅ critical ✅ blocker ✅ blocker ✅ high
Concatenated translation fragments ✅ suggestion ✅ blocker
Vague "Save" button label ✅ suggestion
Heading level skip (<h2>) ✅ suggestion
Physical inline styles (RTL) ✅ suggestion ✅ suggestion
SCSS Issues (2 planted)
Issue Claude Bot Copilot PR Review Skill Pirategoat
Physical CSS properties (RTL) ✅ suggestion ✅ suggestion
Non-BEM generic class names
Bonus Findings (not planted — valid issues found independently)
Finding Claude Bot Copilot PR Review Skill Pirategoat
handleEnablePreview not bound to this (crash)
get_stats_export() passes empty WP_REST_Request
Cache key collision (no separator in md5())
Invalid changelog type (added vs enhancement)
createRef from react instead of @wordpress/element
Social preview renders when module inactive
Styles in wrong file (_buttons.scss)
No aria-live on preview container
Border contrast insufficient
No date validation on params

Scorecard

Metric Claude Bot Copilot PR Review Skill Pirategoat
Planted issues found (of 18) 8 13 14 5
Security issues found (of 3) 2/3 2/3 3/3 2/3
Performance issues found (of 4) 3/4 3/4 4/4 2/4
JSX issues found (of 5) 1/5 5/5 3/5 1/5
SCSS/RTL found (of 2) 0/2 2/2 1/2 0/2
Bonus valid findings 4 7 4 3
False positives 0 0 0 0

Key Takeaways

🏆 PR Review Skill (14/18) — Best for security-critical PHP reviews

  • Only tool to catch the hash_equals() timing attack
  • Found a cache key collision bug nobody else caught
  • Strongest PHP security + performance coverage
  • Weaker on frontend (missed heading skip, vague button label, SCSS naming)

🥈 Copilot (13/18) — Best breadth and frontend coverage

  • Only tool to find ALL 5 JSX issues (heading hierarchy, button label, etc.)
  • Caught Jetpack-specific conventions (changelog type, @wordpress/element imports)
  • Missed the timing attack and cache TTL issues
  • Excellent structured output with line-level links

🥉 Claude Bot (8/18) — Good depth, limited breadth

  • Best "why this matters" explanations for each finding
  • Weakest on frontend/SCSS — only 1/5 JSX and 0/2 SCSS issues
  • Zero false positives, but significant blind spots

4th — Pirategoat-tools (5/18) — High potential, hampered by permissions

  • Only 3 of 10 review agents completed (7 blocked by Bash permission denials)
  • Found unique a11y issues nobody else caught (aria-live, border contrast)
  • With all agents running, would likely rival or exceed the others
  • Multi-agent architecture is the most thorough in theory but operationally fragile without pre-approved permissions

Recommended Workflow

Scenario Recommended Tool(s)
Security-sensitive PHP/API changes PR Review Skill
Frontend-heavy JSX/CSS changes Copilot
Quick sanity check Claude Bot (fast, zero setup)
Comprehensive pre-merge review PR Review Skill + Copilot together

For critical PRs, running PR Review Skill + Copilot together catches 17 of 18 planted issues — only missing the non-BEM class naming convention. They have highly complementary blind spots.


Note: The non-BEM generic class names (.title, .description) in SCSS were not caught by any tool. All four tools had zero false positives.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Admin Page React-powered dashboard under the Jetpack menu Docs [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ [Status] In Progress [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants