#64311 closed enhancement (fixed)
Add filters for input and output validation
| Reported by: | priethor | Owned by: | priethor |
|---|---|---|---|
| Priority: | normal | Milestone: | 7.1 |
| Component: | Abilities API | Version: | 6.9 |
| Severity: | normal | Keywords: | has-patch has-unit-tests has-dev-note |
| Cc: | Focuses: |
Description
The Abilities API currently validates ability input and output against JSON Schema using WordPress's built-in rest_validate_value_from_schema(), which supports only a subset of JSON Schema Draft 4 (aligning with WordPress core). This approach is reliable for core compatibility but limits extenders to dated JSON Schema features, missing newer ones like $ref references for composability and reusability, and not keyword to exclude patterns.
Developers extending the Abilities API that need more expressive schema validation have no way to override the default validator without forking or monkey-patching.
Proposed Solution
Introduce two hooks to allow custom validation:
/** * Filters the input validation result for an ability. * * @since 7.0.0 * * @param true|WP_Error $is_valid Validation result (true or WP_Error). * @param mixed $input The input being validated. * @param string $name The ability name. */ apply_filters( 'wp_ability_validate_input', $is_valid, $input, $name ); /** * Filters the output validation result for an ability. * * @since 7.0.0 * * @param true|WP_Error $is_valid Validation result (true or WP_Error). * @param mixed $output The output being validated. * @param string $name The ability name. */ apply_filters( 'wp_ability_validate_output', $is_valid, $output, $name );
Related
Change History (26)
This ticket was mentioned in PR #10557 on WordPress/wordpress-develop by @priethor.
10 months ago
#1
- Keywords has-unit-tests added
#2
@
10 months ago
- Keywords has-unit-tests removed
- Summary Abilities API: add filters for input and ouput validation → Abilities API: add filters for input and output validation
#4
@
7 months ago
- Milestone 7.0 → Future Release
Because of the lack of activity in the last 3 months and due to the Beta1 freeze happening in 24hrs I'm punting this to "Future Release"
#5
@
6 months ago
- Milestone Future Release → 7.1
Worth noting that similar extensibility was explored earlier in https://github.com/WordPress/abilities-api/pull/37, which proposed ability_input_schema and ability_output_schema filters. However, the filters proposed here are more powerful — they operate on the validation result rather than the schema itself. Schema-level filtering can already happen at registration time, whereas these hooks give developers control over the validation logic, which is the actual constraint point for supporting newer JSON Schema features.
That same PR also proposed ability_permission_result and ability_execute_result filters. It might be worth considering whether those should be part of the parallel effort as well, to provide a complete set of extensibility points across the ability lifecycle.
I'd be happy to see this proposal included in WordPress 7.1.
@westonruter commented on PR #10557:
6 months ago
#7
One thing to address: the
add_filter()calls in the new tests use anonymous closures and are never cleaned up withremove_filter(). Since PHPUnit runs tests in the same process, filters added in one test can leak into subsequent tests and silently affect results. Each test should store the closure reference and remove it after assertions, e.g.:
@gziolo Actually, this isn't a concern because the hooks get reset after each test is run, regardless of whether the tests run in a separate processor not.
In set_up:
In tear_down:
@gziolo commented on PR #10557:
6 months ago
#8
@westonruter, thank you so much for pointing me to that logic. That’s perfect. I’m glad it existed as this is the very well designed default behavior 👍
I intend to land this PR as soon as 7.1 cycle starts.
#9
@
6 months ago
I proactively followed up with #64989 to expand filtering in other aspects of the execution lifecycle for individual abilities.
#10
@
5 months ago
- Component AI → Abilities API
Moving tickets related to the Abilities API to a new sub-component.
#11
@
4 months ago
- Keywords abilities removed
Removing abilities and abilities-api custom keywords. This is now indicated by the Abilities API component.
#12
@
4 months ago
- Keywords has-patch, has-unit-tests → has-patch has-unit-tests
- Summary Abilities API: add filters for input and output validation → Add filters for input and output validation
@gziolo commented on PR #10557:
4 months ago
#13
Merged the latest changes from trunk and resolved the conflicts. Since trunk introduced the new execution lifecycle filters (wp_ability_normalize_input, wp_ability_permission_result, wp_pre_execute_ability, wp_ability_execute_result), the only real conflict was in the test file — resolved by keeping both the upstream pipeline tests and this PR's input/output validation filter tests.
Also addressed the review feedback:
- Corrected the
@sincetags forwp_ability_validate_inputandwp_ability_validate_outputto7.1.0. - Refactored
validate_input()andvalidate_output()to return early instead of reassigning$validity, while preserving the hardening that coerces afalsefilter return into aWP_Error.
Validation passing locally: PHPCS, PHP compatibility, PHPStan, and the abilities-api tests (57 tests, 89 assertions).
@gziolo commented on PR #10557:
4 months ago
#14
Note for a follow-up: as decided in https://github.com/WordPress/wordpress-develop/pull/10557#discussion_r2575710801, esc_html() was removed from $this->name in the input/output validation error messages (the entities would be wrong when rendered into JSON). The same reasoning applies to the other error messages in WP_Ability that still wrap $this->name in esc_html() (ability_invalid_permission_callback, ability_invalid_execute_callback, ability_callback_exception, and the permission denial in execute()). Since those messages predate this PR, it'd make sense to address them in a separate follow-up commit rather than expand this PR's scope.
cc @westonruter
This ticket was mentioned in PR #11917 on WordPress/wordpress-develop by @gziolo.
4 months ago
#17
Follow-up to https://github.com/WordPress/wordpress-develop/pull/10557.
Preserves custom REST response statuses returned by wp_ability_validate_input filter errors. The REST run controller already preserves custom statuses from wp_ability_normalize_input; this applies the same behavior to validation errors by only defaulting to 400 when the WP_Error does not already include a status value.
Testing:
- npm run test:php -- --filter Tests_REST_API_WpRestAbilitiesV1RunController
- npm run test:php -- --group abilities-api
This ticket was mentioned in PR #11918 on WordPress/wordpress-develop by @gziolo.
4 months ago
#18
## Description
WP_Ability renders the ability name into several WP_Error messages that are commonly serialized to JSON (e.g. through the REST API), where HTML entities produced by esc_html() would be incorrect. This removes esc_html() from the ability name in those messages:
ability_callback_exceptionability_invalid_permission_callbackability_invalid_execute_callbackability_invalid_permissions
This aligns them with validate_input() / validate_output(), which already pass $this->name unescaped (see the decision in https://github.com/WordPress/wordpress-develop/pull/10557#discussion_r2575710801).
### Why this is safe
Ability names are pattern-validated at registration to ^[a-z0-9-]+\/[a-z0-9-]+$, so they can never contain HTML special characters — esc_html() on the name is a no-op in every case. The change is purely about correctness/consistency for JSON-bound messages.
### What is intentionally left unchanged
- The
_doing_it_wrong()calls render into HTML admin output, so they keepesc_html()(including<code>-wrapped values and the arbitrary$property_name). - The arbitrary exception text (
$e->getMessage()) and permission error text ($has_permissions->get_error_message()) keepesc_html(), since those can contain arbitrary content.
## Testing
tests/phpunit/tests/abilities-api/wpAbility.phppasses (57 tests, 89 assertions).- PHPCS, PHP compatibility, and PHPStan all pass.
---
Trac ticket: https://core.trac.wordpress.org/ticket/64311
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request.
@gziolo commented on PR #11917:
4 months ago
#20
A commit was made that fixes the Trac ticket referenced in the description of this pull request.
SVN changeset: 62397
GitHub commit: https://github.com/WordPress/wordpress-develop/commit/1d51ae58054360b749e8f5714d0071e2c2a6318d
This PR will be closed, but please confirm the accuracy of this and reopen if there is more work to be done.
@gziolo commented on PR #10557:
4 months ago
#21
@gziolo commented on PR #11918:
4 months ago
#23
A commit was made that fixes the Trac ticket referenced in the description of this pull request.
SVN changeset: 62401
GitHub commit: https://github.com/WordPress/wordpress-develop/commit/4ddc905a5ea4be6971ef7d8a4fed227f3b0a2635
This PR will be closed, but please confirm the accuracy of this and reopen if there is more work to be done.
![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png)
Trac ticket: https://core.trac.wordpress.org/ticket/64311