Abilities API: check_permissions() passes raw input to permission callbacks when called directly

[zaguiini]

Summary: Abilities API: check_permissions() passes raw input to permission callbacks when called directly

Since [65044] (#66383), WP_Ability::execute() validates the input and converts it to the schema types with coerce_input() before it calls check_permissions(). The permission callback gets typed input on that path.

check_permissions() is public, and callers also use it on its own. The MCP adapter calls it with the raw tool arguments before it calls execute():

• McpTool::check_permission()
• McpPrompt::check_permission()
• ExecuteAbilityAbility::check_permission()

On that path the permission callback gets the input as the client sent it. It gets no schema default, no validation, and no conversion. So one ability gets different input in its permission callback, depending on who calls it:

Caller	Input {"id": "123"} reaches the callback as
execute()	array( 'id' => 123 )
check_permissions() from the MCP adapter	array( 'id' => '123' )

The same happens to defaults. The adapter passes {} as null. execute() applies the schema default and turns it into array(), but check_permissions() passes on null.

A permission callback written for execute() can then fail on a direct call. With declare( strict_types=1 ), function get_id( array $input ): int { return $input['id'] ?? 0; } throws a TypeError when id is "123". The adapter reports it as mcp_permission_check_failed, so the tool call fails for the user.

The docblock of check_permissions() says that callers should run validate_input() first. Every caller then has to repeat the execute() steps in the same order: normalize_input(), validate_input(), coerce_input(). The MCP adapter does not, and nothing tells a callback author which input to expect.

Proposal

Make check_permissions() prepare the input the same way execute() does: apply the default, validate, and convert. Return the validation error when the input is invalid. The permission callback then gets the same input on every path.

execute() already prepares the input before it calls check_permissions(). It could call a private method that skips the second preparation, so execute() still validates once.

[zaguiini]

@gziolo what do you think?

[zaguiini]

Related: #66383 and the discussion in ​PR #14722. That discussion placed coercion between validate_input() and check_permissions() inside execute(), and made coerce_input() public so the REST controller and the MCP adapter can call it too.

So the open question for this ticket is who prepares the input when a caller uses check_permissions() on its own:

1. check_permissions() prepares it, as proposed in the description. Every caller then gets the same input in the permission callback.
2. Each direct caller prepares it with normalize_input(), validate_input() and coerce_input(). Then the docblock of check_permissions() should say so, because today it mentions only validate_input().

The MCP adapter does neither today. I opened ​mcp-adapter#438 for that.

[euvoor]

I'm working on the adapter side in ​mcp-adapter#438 and can reproduce this on WordPress trunk a99e724. The same ability passes through execute(), but all three adapter permission entry points lose the schema default, trigger a TypeError when a permission callback uses a strict integer-returning helper, and invoke the permission callback for invalid input.

I have ​an adapter candidate with regression tests. Its input preparation follows normalize_input(), validate_input(), then coerce_input() when available, and propagates errors before checking permissions. The adapter suite passes on PHP 8.4 with WordPress 6.9, 7.1 and trunk, with version-dependent and multisite skips.

I'll adapt that patch to the contract agreed here. If check_permissions() takes over preparation, the adapter must avoid preparing input again within the same permission check.

[justlevine]

Thanks for opening this @euvoor 🙇.

I've shared a similar concern about the premise for coerce_input() in https://core.trac.wordpress.org/ticket/66383#comment:10

[euvoor]

Thanks @justlevine. I'd use the callback behavior as the acceptance criterion while #66383 is reviewed: defaults applied, invalid input rejected before the permission callback, and accepted input handled consistently with execute().

The adapter bypasses normalization and validation as well as coercion, so its defaults and invalid-input failures need addressing whichever API we settle on. The ​candidate tests cover these separately across all three adapter entry points, including preservation of permission denials and errors.

I'll retain that coverage when adapting the patch to the agreed Core contract. If preparation moves into check_permissions(), the adapter should avoid running the normalization and validation filters again within the same permission check.

[gziolo]

@zaguiini, thanks for raising the issue.

We should at minimum update PHPDoc to recommend also running normalization and coercion here:

​https://github.com/WordPress/wordpress-develop/blob/7516662a3b47c73a87cb64ef49d5c8fa21cd3ec4/src/wp-includes/abilities-api/class-wp-ability.php#L754-L755

@euvoor, the proposed fix in MCP adapter looks great. It exactly mirror how it is handled in REST API in WordPress core:

​https://github.com/WordPress/wordpress-develop/blob/7516662a3b47c73a87cb64ef49d5c8fa21cd3ec4/src/wp-includes/rest-api/endpoints/class-wp-rest-abilities-v1-run-controller.php#L174-L194

I'd be happy to see explorations how to move these steps into check_permissions() while ensuring it doesn't run twice inside execute(). That was the primary concern we had knowing that check_permission() is lower-level for the majority of consumers, as they will interact with execute().

[euvoor]

@gziolo I'd like to explore moving input preparation into check_permissions(), with regression coverage ensuring execute() prepares input only once and preserves existing filter and error behavior. I'll bring back a candidate for review and keep the adapter fix coordinated with that discussion.

[gziolo]

@euvoor, excellent! Go for it, so we can asses pros and cons. It's a very small group of consumers (libraries) that will run into this issue where there is need to run permissions explicitly.

Direct calls to WP_Ability::check_permissions() currently skip input preparation. With an integer schema, a permission callback receives the string "7", while execute() supplies the integer 7. Schema defaults and invalid-input handling also differ.

This candidate makes the default permission check normalize, validate, and coerce input before checking capabilities or invoking the permission callback.

Implementation and compatibility

execute() prepares input once and calls a new check_permissions_for_prepared_input() method. The REST controller uses the same method after its existing preparation steps, preserving its separate handling of input errors and permission errors. Early execution hooks and short-circuit behavior remain in place.

Existing subclasses can override check_permissions(). The prepared-input method dispatches to those overrides, and calls to parent::check_permissions() from an override retain their previous prepared-input contract. Subclasses inheriting the default method receive automatic preparation.

That compatibility handling uses reflection to identify overrides. This is the main design tradeoff for review, together with adding a public method for prepared input. Custom overrides still own their input-preparation contract. Existing consumers that already prepare input should use the new method to avoid running preparation filters again.

Validation

Tested on PHP 8.4.24 with PHPUnit 9.6.36:

• Unchanged trunk at 7516662: 879 Abilities API tests passed.
• Candidate, single-site: 900 tests and 3,088 assertions passed.
• Candidate, multisite: 905 tests and 3,110 assertions passed.
• Core PHPCS ruleset, PHP syntax checks, and git diff --check passed for the changed files.

The 21 added cases cover coercion and defaults, preparation errors, repeated and nested calls, custom overrides, preparation counts, and REST input-error statuses. The direct-input regression cases fail against unchanged trunk. An existing callback test now expects floats for its number schema, matching the prepared input.

The full Core suite and other PHP versions have not been run locally.

[euvoor]

@gziolo the candidate is ready for review: ​https://github.com/WordPress/wordpress-develop/pull/15504

It preserves existing permission overrides and prepares input once inside execute(). The new method for already-prepared input and the compatibility handling are the main tradeoffs to assess.

[zaguiini]

Claude and I tested ​https://github.com/WordPress/wordpress-develop/pull/15504 with WooCommerce trunk and MCP adapter 0.7.1. It ran 45 checks of the WooCommerce abilities over MCP and REST, with input such as "123" for an integer, "false" for a boolean, "a,b" for an array and {}.

• Without the patch: 11 MCP calls fail with a TypeError in a strict permission callback.
• With the patch: all 45 pass.
• With the patch and a subclass that overrides check_permissions() to prepare its own input: all 45 pass. In that case execute() prepares the input twice, which is harmless.

WooCommerce has that override today to work around this gap, and it also backports coerce_input() for 7.0 and 7.1. If this patch ships in 7.2 together with coerce_input(), plugins can remove both workarounds when they require 7.2. A later release would make them keep the override for one more version, so it would be awesome to see this in 7.2.

[gziolo]

Thanks @euvoor for exploring this and putting together the patch and tests!

After revisiting the original Abilities API discussion, I'm leaning towards keeping check_permissions() independent of input preparation. This separation was intentional and came from feedback by @jorbin ​during the WordPress 6.9 Core review, with @jason_the_adams supporting the approach. The main concern was mixing validation with authorization, which complicated error handling. We agreed that execute() should orchestrate the full lifecycle, while integrations needing individual steps should compose them explicitly.

Keeping these methods focused and independent also benefits extensibility, allowing developers to subclass WP_Ability and override specific lifecycle methods without introducing implicit dependencies.

The proposed PR highlights the complexity of reversing that decision, requiring another public method and reflection-based handling for overridden methods.

Given this context, I'd prefer to fix the MCP adapter to follow the existing lifecycle, as the REST controller already does, and clarify the check_permissions() PHPDoc to cover normalization, validation, and coercion.

@zaguiini, would that create issues for WooCommerce when coerce_input() has to be called explicitly when invoked outside of usual execute() path?

[zaguiini]

No, it doesn't create issues for Woo, @gziolo. WooCommerce runs its abilities only through execute(), so it never calls check_permissions() directly. The MCP adapter was the only caller that skipped the preparation, and the adapter fix covers that.

WooCommerce will keep its own check_permissions() override, which prepares the input before the permission callback runs, as protection against callers that skip that step. Clarifying the PHPDoc sounds good to me.

WP_Ability::check_permissions() does not prepare the input itself. Callers that use it outside of WP_Ability::execute() need to call normalize_input(), validate_input(), and coerce_input() first, in this order. Otherwise, the permission callback may get different input than during execute(). For example, a schema default may be missing, or a value may be a string instead of an integer.

This PR updates the docs to make that clear:

• The check_permissions() docblock now explains the steps, the order, and why they matter. It also points to execute() as the usual way to run an ability.
• The Abilities API overview in abilities-api.php now mentions input coercion in the execution lifecycle. It points integrations to the check_permissions() docs instead of repeating the steps.

The REST run controller already follows this order, so this is a docs-only change.

Use of AI Tools

Claude Code helped review and edit the docblocks and wrote the commit message and this description. I reviewed all changes.

🤖 Generated with ​Claude Code

[euvoor]

Thanks @gziolo, that makes sense. I see #15514 now covers the PHPDoc clarification, so I'll close the exploratory PR #15504 and finish the adapter fix against the existing lifecycle.

@zaguiini, thanks for testing the patch with WooCommerce and confirming the adapter approach works for you.

[gziolo]

@euvoor, I opened PR with documentation edits: ​https://github.com/WordPress/wordpress-develop/pull/15514. We can use your PR or mine. Both works for me, your call.

Let us also know, when MCP fix is ready so we can help to land it.

[euvoor]

@gziolo let's use #15514 for the docs; I've closed #15504. The adapter fix is ready: ​https://github.com/WordPress/mcp-adapter/pull/444

The full adapter suite passes on PHP 8.4 with WordPress trunk, 7.1, and 6.9. PHPCS and PHPStan also pass; version-dependent and multisite skips are listed in the PR.

One note outside the diff: coerce_input() has a bare @see validate_input() at line 580 that could be qualified in the same pass.

Makes perfect sense, adjusted in ​https://github.com/WordPress/wordpress-develop/pull/15514/changes/58b3cf82f5500f95f95c3fc52349da7e9d51057a.

[gziolo]

In 65306:

Abilities API: Document input preparation for direct permission checks

WP_Ability::check_permissions() does not prepare the input. Callers outside of WP_Ability::execute() must run normalize_input(), validate_input(), and coerce_input() first, or the permission callback may get different input than during execute().

Developed in ​https://github.com/WordPress/wordpress-develop/pull/15514.

Props euvoor, gziolo, zaguiini, jorbin.
See #66482.

Committed in https://core.trac.wordpress.org/changeset/65306.

[!NOTE]
This PR is stacked on #15684. It includes that PR's two commits. Only the last commit, "Abilities API: Add WP_Ability::prepare_input()", is new here. The diff will get smaller once #15684 lands.

What

Adds a public WP_Ability::prepare_input() method. It normalizes, validates, and sanitizes the input in one call, and returns the prepared input or the first WP_Error.

• execute() uses prepare_input() instead of calling the three steps itself.
• The REST run controller calls prepare_input() in both places that prepare input: sanitize_input_for_ability() and the null input branch of check_ability_permissions().
• coerce_input() is renamed to the protected sanitize_input(). Sanitizing input that was not validated can hide validation errors, so prepare_input() is now the public way to run it. coerce_input() was added in 7.2 and has not been released yet, so this is not a breaking change.

Why

Before this change, every caller had to run normalize_input(), validate_input(), and coerce_input() in the right order, and stop at the first error. This was repeated in execute() and twice in the REST controller. check_permissions() also documented the three steps for integrations that call it directly. Now there is one method that does it correctly.

Documentation

The docs now refer to prepare_input() instead of listing each step. This covers the Abilities API overview, check_permissions(), execute(), and the hook docs. Tests about the sanitization step now use "sanitization" in their names, to match the method name. Tests about type conversion keep "coerce".

Testing

• npm run test:php -- --filter=Abilit passes.
• Linting and PHPStan pass.

Trac ticket: https://core.trac.wordpress.org/ticket/66482, https://core.trac.wordpress.org/ticket/66383

Use of AI Tools

AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5.5
Used for: Implementation, documentation updates, and test renames, iterated on together with me. I reviewed the final code and tests.