Skip to content

Abilities: Strip internal schema keywords from abilities REST responses#11451

Open
jorgefilipecosta wants to merge 12 commits intoWordPress:trunkfrom
jorgefilipecosta:fix/strip-internal-schema-keywords-from-abilities-response
Open

Abilities: Strip internal schema keywords from abilities REST responses#11451
jorgefilipecosta wants to merge 12 commits intoWordPress:trunkfrom
jorgefilipecosta:fix/strip-internal-schema-keywords-from-abilities-response

Conversation

@jorgefilipecosta
Copy link
Copy Markdown
Member

@jorgefilipecosta jorgefilipecosta commented Apr 6, 2026

Track ticket: https://core.trac.wordpress.org/ticket/65035

Ability input_schema and output_schema may contain WordPress-internal properties like sanitize_callback, validate_callback, and arg_options that are not valid JSON Schema keywords. These cause client-side JSON Schema validators to fail.

This adds recursive stripping of internal keywords in prepare_item_for_response(), using a denylist approach (array_diff_key removing specific internal keywords defined in WP_REST_Abilities_Controller::INTERNAL_SCHEMA_KEYWORDS). This differs from the allowlist approach (rest_get_allowed_schema_keywords() + array_intersect_key()) used in WP_REST_Server::get_data_for_route() — the denylist is the correct choice here because rest_get_allowed_schema_keywords() is missing valid JSON Schema keywords (allOf, not, definitions, dependencies, additionalItems) that would get silently stripped with the allowlist approach.

Related Gutenberg PR: WordPress/gutenberg#77029

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 6, 2026

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props jorgefilipecosta, gziolo, ocean90.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 6, 2026

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

  • All changes will be lost when closing a tab with a Playground instance.
  • All changes will be lost when refreshing the page.
  • A fresh instance is created each time the link below is clicked.
  • Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
    it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

@jorgefilipecosta
Copy link
Copy Markdown
Member Author

jorgefilipecosta commented Apr 6, 2026

Client side clean up at WordPress/gutenberg#77029.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the Abilities REST API responses to remove WordPress-internal (non–JSON Schema) keywords from input_schema/output_schema so that client-side JSON Schema validators don’t fail.

Changes:

  • Add recursive schema-keyword stripping in WP_REST_Abilities_V1_List_Controller::prepare_item_for_response().
  • Add PHPUnit coverage to ensure internal schema keywords are removed from ability REST responses.

Reviewed changes

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

File Description
src/wp-includes/rest-api/endpoints/class-wp-rest-abilities-v1-list-controller.php Adds recursive filtering of schema arrays before returning them in REST responses.
tests/phpunit/tests/rest-api/wpRestAbilitiesV1ListController.php Adds a test ensuring internal schema keywords are stripped from input_schema/output_schema.

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

Comment on lines +232 to +238
private function strip_internal_schema_keywords( array $schema ): array {
$allowed_keywords = rest_get_allowed_schema_keywords();
$allowed_keywords[] = 'required';
$allowed_keywords = array_flip( $allowed_keywords );

$schema = array_intersect_key( $schema, $allowed_keywords );

Copy link

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

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

strip_internal_schema_keywords() currently filters schemas using rest_get_allowed_schema_keywords(), which is a WordPress-supported subset and does not include several standard JSON Schema draft-04 keywords (e.g. $schema, $ref, allOf, not, definitions). As a result, any ability schema that legitimately uses those keywords will have them removed from REST responses, changing the meaning of the schema and potentially breaking existing consumers. If the intent is only to remove WordPress-internal keys like sanitize_callback/validate_callback/arg_options, consider switching to a targeted denylist for those internal keys, or expanding the allowlist to cover the JSON Schema keywords you want to preserve (and ensuring recursion covers those keywords too).

Copilot uses AI. Check for mistakes.
Comment on lines +232 to +245
private function strip_internal_schema_keywords( array $schema ): array {
$allowed_keywords = rest_get_allowed_schema_keywords();
$allowed_keywords[] = 'required';
$allowed_keywords = array_flip( $allowed_keywords );

$schema = array_intersect_key( $schema, $allowed_keywords );

// Sub-schema maps: keys are user-defined, values are sub-schemas.
foreach ( array( 'properties', 'patternProperties' ) as $keyword ) {
if ( isset( $schema[ $keyword ] ) && is_array( $schema[ $keyword ] ) ) {
foreach ( $schema[ $keyword ] as $key => $child_schema ) {
if ( is_array( $child_schema ) ) {
$schema[ $keyword ][ $key ] = $this->strip_internal_schema_keywords( $child_schema );
}
Copy link

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

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

strip_internal_schema_keywords() rebuilds and flips the allowed-keywords array on every recursive call. For deep or repetitive schemas this adds avoidable overhead. Consider computing the flipped allowlist once (e.g., in the top-level call or as a static/private property) and passing it into the recursive calls to keep the per-node work to just filtering + traversal.

Copilot uses AI. Check for mistakes.
Comment on lines +818 to +819
$data = $response->get_data();

Copy link

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

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

The test does not assert that the REST request succeeded (e.g. status 200) before indexing into $data['input_schema']['properties']['content']. If the endpoint returns an error (or an unexpected payload), this can produce undefined-index notices that obscure the real failure. Add an explicit status assertion (and/or assertArrayHasKey checks) before accessing nested keys.

Suggested change
$data = $response->get_data();
$this->assertEquals( 200, $response->get_status() );
$data = $response->get_data();
$this->assertIsArray( $data );
$this->assertArrayHasKey( 'input_schema', $data );
$this->assertIsArray( $data['input_schema'] );
$this->assertArrayHasKey( 'properties', $data['input_schema'] );
$this->assertIsArray( $data['input_schema']['properties'] );
$this->assertArrayHasKey( 'content', $data['input_schema']['properties'] );
$this->assertIsArray( $data['input_schema']['properties']['content'] );
$this->assertArrayHasKey( 'output_schema', $data );
$this->assertIsArray( $data['output_schema'] );

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

The fix is correct and well-tested. One clarification and a note for future work.

The description says this uses "the same approach (rest_get_allowed_schema_keywords() + array_intersect_key()) that WP_REST_Server::get_data_for_route() already uses." That's not quite right — get_data_for_route() uses an allowlist (array_intersect_key keeping only known-good keywords), while this PR uses a denylist (array_diff_key removing 3 specific internal keywords). The denylist is the correct choice here because rest_get_allowed_schema_keywords() is missing valid JSON Schema keywords (allOf, not, definitions, dependencies, additionalItems) that would get silently stripped with the allowlist approach. Worth updating the description to avoid confusion for future readers.

Looking ahead, the recursive internal-keyword stripping logic is generic and not abilities-specific. In a future release, it could be extracted into a shared utility, alongside updating the set of allowed schema keywords to cover the full JSON Schema spec so an allowlist approach becomes viable. That's out of scope for an RC fix though, and it's being tracked more broadly in Abilities API: Add schema compiler for AI tool calling compatibility.

Implementation correctly traverses all JSON Schema sub-schema locations and tests cover both flat and deeply nested schemas across all keyword types.

Ability `input_schema` and `output_schema` may contain WordPress-internal
properties like `sanitize_callback`, `validate_callback`, and `arg_options`
that are not valid JSON Schema keywords. These cause client-side JSON Schema
validators to fail.

Strip non-standard keywords recursively using the same allowlist approach
(`rest_get_allowed_schema_keywords()`) that `WP_REST_Server::get_data_for_route()`
already uses for endpoint arguments.
…eywords.

Use a targeted denylist of the three known WordPress-internal keys
(sanitize_callback, validate_callback, arg_options) instead of an
allowlist based on rest_get_allowed_schema_keywords(). The allowlist
approach was too aggressive — it stripped legitimate JSON Schema
keywords like $schema, $ref, allOf, not, and definitions that are
valid but not in WordPress's supported subset.
The blind recursion into all array values would incorrectly strip
denylist keys from data-holding keywords like `default` and `enum`.
Restrict recursion to known sub-schema locations: properties,
patternProperties, definitions, items, additionalProperties,
additionalItems, not, anyOf, oneOf, allOf.
…ing test coverage.

Defines INTERNAL_SCHEMA_KEYWORDS as an associative array so array_flip()
is no longer called on every recursive invocation. Adds test coverage for
additionalItems, definitions, and tuple-style items sub-schema locations.
… stripping.

Add wp_is_numeric_array() guard to the dependencies handler so
property-dependency arrays (numeric arrays of strings) are explicitly
skipped rather than relying on array_diff_key being a no-op.
@jorgefilipecosta jorgefilipecosta force-pushed the fix/strip-internal-schema-keywords-from-abilities-response branch from 5291a90 to e89f03c Compare April 7, 2026 13:13
jorgefilipecosta and others added 3 commits April 7, 2026 14:30
…list-controller.php

Co-authored-by: Dominik Schilling <dominikschilling+git@gmail.com>
Co-authored-by: Dominik Schilling <dominikschilling+git@gmail.com>
@jorgefilipecosta jorgefilipecosta changed the title REST API: Strip internal schema keywords from ability REST responses Abilities: Strip internal schema keywords from abilities REST responses Apr 7, 2026
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.

4 participants