Skip to content

Pattern directory: Validate the fields the directory renders - #769

Merged
obenland merged 2 commits into
trunkfrom
pattern-directory/validate-rendered-fields
Sep 4, 2026
Merged

Pattern directory: Validate the fields the directory renders#769
obenland merged 2 commits into
trunkfrom
pattern-directory/validate-rendered-fields

Conversation

@obenland

@obenland obenland commented Sep 4, 2026

Copy link
Copy Markdown
Member

validate_block_directives() scans post_content for Interactivity API attributes, and its docblock explains why kses cannot do that job. The field to scan is hard-coded, so the title and excerpt — which the directory renders the same way — are never checked.

is_title_valid() has the matching gap on the other side: it compares the title against a five-word disallow list, which says nothing about what the title contains.

  • Scan post_title and post_excerpt for directives alongside post_content.
  • Require a title to survive strip_shortcodes() and wp_strip_all_tags() unchanged, and to carry no directives, before the disallow list sees it.

The new checks run ahead of the disallow list, so the existing rejections keep their error codes.

Testing steps

  1. Submit a pattern titled Example [caption width="1" caption="x"]y[/caption]. It is rejected; on trunk it is accepted and stored intact.
  2. Submit one titled <span data-wp-interactive="x">Hi</span>. Rejected.
  3. Default Paragraph, Testimonial and Stylized Quote and Citation are still accepted; Testing, My Pattern and Test are still rejected with rest_pattern_invalid_title.

🤖 Generated with Claude Code

The directive guard read post_content only, and the title validator checked a
five-word disallow list. Both leave the other fields the directory renders
unchecked, even though a title is emitted as markup the same way content is.

Scan the title and excerpt for directives alongside the content, and require a
title to survive strip_shortcodes() and wp_strip_all_tags() unchanged before the
disallow list sees it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings September 4, 2026 16:30

Copilot AI 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.

🟡 Changes recommended

The new directive check does not match the PR’s stated behavior for entity-escaped markup, and the added validation paths lack corresponding unit test coverage.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR tightens server-side validation for fields the Pattern Directory renders (content, title, excerpt) by expanding Interactivity API directive scanning beyond post_content, and by making title validation reject shortcodes/HTML and directives before applying the existing disallowed-word list.

Changes:

  • Scan post_title and post_excerpt for data-wp-* Interactivity directives in addition to post_content.
  • Strengthen is_title_valid() to reject titles that change under strip_shortcodes() / wp_strip_all_tags(), and to reject titles containing interactivity directives before the disallow list check.
File summaries
File Description
public_html/wp-content/plugins/pattern-directory/includes/pattern-validation.php Expands directive scanning to multiple rendered fields and hardens title validation rules.
Review details

Suppressed comments (1)

public_html/wp-content/plugins/pattern-directory/includes/pattern-validation.php:853

  • content_has_block_directives() only detects real HTML tags via WP_HTML_Tag_Processor. When $title contains entity-escaped markup (e.g. &lt;span data-wp-interactive=...&gt; as described in the PR), stripos() matches but the tag processor finds no tags, so the title will not be rejected. This also makes the directive check redundant with the preceding wp_strip_all_tags() check for non-escaped tags.
	if ( content_has_block_directives( $title ) ) {
		return false;
	}
  • Files reviewed: 1/1 changed files
  • Comments generated: 2
  • Review effort level: Lite

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

The message said 'Pattern content' while the check now covers the title and
excerpt too, so a submitter was told to look in the wrong place.

Also covers the new title rules: a shortcode and markup are rejected by the
title validator, and a directive by the directive check, which runs first.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot AI 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.

🟡 Changes recommended

Entity-encoded directives (e.g. &lt;span data-wp-interactive="x"&gt;) can currently bypass the directive detector, so the implementation does not match the PR’s stated validation behavior.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 2
  • Review effort level: Lite

@obenland
obenland merged commit eda70dc into trunk Sep 4, 2026
5 checks passed
@obenland
obenland deleted the pattern-directory/validate-rendered-fields branch September 4, 2026 17:44
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