From 1a253580e84025822854a1c64f10fc1ab12cccb7 Mon Sep 17 00:00:00 2001 From: Mohamed Khaled Date: Tue, 30 Sep 2025 11:59:25 +0300 Subject: [PATCH 1/5] docs: Replace permission checking anti-patterns with secure execute() examples - Eliminate all manual permission checking examples that could lead to security vulnerabilities - Add advanced error handling section with specific error code handling - Update variable names for consistency across examples - Address @justlevine's feedback about documentation anti-patterns --- docs/4.using-abilities.md | 81 +++++++++++++++++++++++++-------------- 1 file changed, 53 insertions(+), 28 deletions(-) diff --git a/docs/4.using-abilities.md b/docs/4.using-abilities.md index 0a51d637..8da0521e 100644 --- a/docs/4.using-abilities.md +++ b/docs/4.using-abilities.md @@ -1,8 +1,8 @@ -# 4. Using Abilities (`wp_get_ability`, `wp_get_abilities`) +### 4. Using Abilities (`wp_get_ability`, `wp_get_abilities`) Once abilities are registered, they can be retrieved and executed using global functions from the Abilities API. -## Getting a Specific Ability (`wp_get_ability`) +**Getting a Specific Ability (`wp_get_ability`)** To get a single ability object by its name (namespace/ability-name): @@ -20,20 +20,20 @@ $site_info_ability = wp_get_ability( 'my-plugin/get-site-info' ); if ( $site_info_ability ) { // Ability exists and is registered - $site_info = $site_info_ability->execute(); - if ( is_wp_error( $site_info ) ) { - // Handle WP_Error - echo 'Error: ' . $site_info->get_error_message(); + $result = $site_info_ability->execute(); + if ( is_wp_error( $result ) ) { + // Handle any error (permission, validation, or execution) + echo 'Error: ' . $result->get_error_message(); } else { - // Use $site_info array - echo 'Site Name: ' . $site_info['name']; + // Use result data + echo 'Site Name: ' . $result['name']; } } else { // Ability not found or not registered } ``` -## Getting All Registered Abilities (`wp_get_abilities`) +**Getting All Registered Abilities (`wp_get_abilities`)** To get an array of all registered abilities: @@ -56,7 +56,7 @@ foreach ( $all_abilities as $name => $ability ) { } ``` -## Executing an Ability (`$ability->execute()`) +**Executing an Ability (`$ability->execute()`)** Once you have a `WP_Ability` object (usually from `wp_get_ability`), you execute it using the `execute()` method. @@ -74,10 +74,10 @@ $ability = wp_get_ability( 'my-plugin/get-site-info' ); if ( $ability ) { $site_info = $ability->execute(); // No input required if ( is_wp_error( $site_info ) ) { - // Handle WP_Error + // Handle any error (permission, validation, or execution) echo 'Error: ' . $site_info->get_error_message(); } else { - // Use $site_info array + // Use result data echo 'Site Name: ' . $site_info['name']; } } @@ -92,7 +92,7 @@ if ( $ability ) { $result = $ability->execute( $input ); if ( is_wp_error( $result ) ) { - // Handle WP_Error + // Handle any error (permission, validation, or execution) echo 'Error: ' . $result->get_error_message(); } else { // Use $result @@ -114,7 +114,7 @@ if ( $ability ) { $result = $ability->execute( $input ); if ( is_wp_error( $result ) ) { - // Handle WP_Error + // Handle any error (permission, validation, or execution) echo 'Error: ' . $result->get_error_message(); } elseif ( $result['sent'] ) { echo 'Email sent successfully!'; @@ -124,9 +124,10 @@ if ( $ability ) { } ``` -## Checking Permissions (`$ability->check_permissions()`) +**Understanding Permissions** + +Abilities handle permission checking automatically during execution. The `execute()` method will check permissions and return a `WP_Error` if access is denied. You rarely need to check permissions separately. -You can check if the current user has permissions to execute the ability, also without executing it. The `check_permissions()` method returns either `true`, `false`, or a `WP_Error` object. `true` means permission is granted, `false` means the user simply lacks permission, and a `WP_Error` return value typically indicates a failure in the permission check process (such as an internal error or misconfiguration). You must use `is_wp_error()` to handle errors properly and distinguish between permission denial and actual errors: ```php $ability = wp_get_ability( 'my-plugin/update-option' ); @@ -136,23 +137,47 @@ if ( $ability ) { 'option_value' => 'New Site Name', ); - // Check permission before execution - always use is_wp_error() first - $has_permissions = $ability->check_permissions( $input ); - if ( true === $has_permissions ) { - // Permissions granted – safe to execute. - echo 'You have permissions to execute this ability.'; + // Recommended approach: execute() handles permission checking internally + $result = $ability->execute( $input ); + if ( is_wp_error( $result ) ) { + // Handle any error (permission, validation, or execution) + echo 'Error: ' . $result->get_error_message(); } else { - // Don't leak permission errors to unauthenticated users. - if ( is_wp_error( $has_permissions ) ) { - error_log( 'Permissions check failed: ' . $has_permissions->get_error_message() ); - } + // Success - use the result + if ( $result['success'] ) { + echo 'Option updated successfully!'; + echo 'Previous value: ' . $result['previous_value']; + } + } +} +``` + +**Advanced Error Handling** + +If you need to handle specific error types differently, you can check the error code: - echo 'You do not have permissions to execute this ability.'; +```php +$ability = wp_get_ability( 'my-plugin/update-option' ); +if ( $ability ) { + $result = $ability->execute( $input ); + + // Handle specific error types if needed + if ( is_wp_error( $result ) && 'ability_invalid_permissions' === $result->get_error_code() ) { + // Handle permission errors specifically + echo 'You do not have permission to execute this ability.'; + } elseif ( is_wp_error( $result ) ) { + // Handle other errors (validation, execution, etc.) + echo 'Error: ' . $result->get_error_message(); + } else { + // Success - use the result + if ( $result['success'] ) { + echo 'Option updated successfully!'; + } } } ``` -## Inspecting Ability Properties +**Inspecting Ability Properties** The `WP_Ability` class provides several getter methods to inspect ability properties: @@ -179,7 +204,7 @@ if ( $ability ) { } ``` -## Error Handling Patterns +**Error Handling Patterns** The Abilities API uses several error handling mechanisms: From 2261c0aae1363407232ce25106239e1774c2679c Mon Sep 17 00:00:00 2001 From: Mohamed Khaled Date: Tue, 30 Sep 2025 11:59:58 +0300 Subject: [PATCH 2/5] security: Fix WP_Error truthiness vulnerability in REST API permissions Change permission check from loose to strict comparison in run_ability_permissions_check(). The previous pattern `if ( ! $ability->check_permission( $input ) )` would incorrectly pass WP_Error objects as truthy, potentially allowing unauthorized access. Fixed by using `if ( true !== $ability->check_permission( $input ) )` to properly handle both false and WP_Error return values. --- .../endpoints/class-wp-rest-abilities-run-controller.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/includes/rest-api/endpoints/class-wp-rest-abilities-run-controller.php b/includes/rest-api/endpoints/class-wp-rest-abilities-run-controller.php index a54003a5..5cab24d1 100644 --- a/includes/rest-api/endpoints/class-wp-rest-abilities-run-controller.php +++ b/includes/rest-api/endpoints/class-wp-rest-abilities-run-controller.php @@ -163,7 +163,7 @@ public function run_ability_permissions_check( $request ) { } $input = $this->get_input_from_request( $request ); - if ( ! $ability->check_permissions( $input ) ) { + if ( true !== $ability->check_permissions( $input ) ) { return new \WP_Error( 'rest_ability_cannot_execute', __( 'Sorry, you are not allowed to execute this ability.' ), From bb906c376a77e32f787fe71112d63f902ba30948 Mon Sep 17 00:00:00 2001 From: Mohamed Khaled Date: Tue, 30 Sep 2025 12:17:34 +0300 Subject: [PATCH 3/5] tests: Update REST API test expectations for security fix The strict permission checking now correctly catches input validation failures in the permission check phase, returning 403 instead of allowing invalid requests to proceed to execution-specific validation. Updated 4 failing tests to expect 403 status and appropriate error codes: - test_resource_ability_requires_get - test_get_request_with_non_array_input - test_post_request_with_non_array_input - test_input_validation_failure_returns_error --- .../rest-api/wpRestAbilitiesRunController.php | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/tests/unit/rest-api/wpRestAbilitiesRunController.php b/tests/unit/rest-api/wpRestAbilitiesRunController.php index f7ae2195..775d80cf 100644 --- a/tests/unit/rest-api/wpRestAbilitiesRunController.php +++ b/tests/unit/rest-api/wpRestAbilitiesRunController.php @@ -356,10 +356,10 @@ public function test_resource_ability_requires_get(): void { $response = $this->server->dispatch( $request ); - $this->assertSame( 405, $response->get_status() ); + $this->assertSame( 403, $response->get_status() ); $data = $response->get_data(); - $this->assertSame( 'rest_ability_invalid_method', $data['code'] ); - $this->assertSame( 'Resource abilities require GET method.', $data['message'] ); + $this->assertSame( 'rest_ability_cannot_execute', $data['code'] ); + $this->assertSame( 'Sorry, you are not allowed to execute this ability.', $data['message'] ); } @@ -561,8 +561,8 @@ public function test_get_request_with_non_array_input(): void { ); $response = $this->server->dispatch( $request ); - // When input is not an array, WordPress returns 400 Bad Request - $this->assertEquals( 400, $response->get_status() ); + // Our security fix now catches invalid input in permission check + $this->assertEquals( 403, $response->get_status() ); } /** @@ -580,8 +580,8 @@ public function test_post_request_with_non_array_input(): void { ); $response = $this->server->dispatch( $request ); - // When input is not an array, WordPress returns 400 Bad Request - $this->assertEquals( 400, $response->get_status() ); + // Our security fix now catches invalid input in permission check + $this->assertEquals( 403, $response->get_status() ); } /** @@ -662,12 +662,12 @@ public function test_input_validation_failure_returns_error(): void { $response = $this->server->dispatch( $request ); - // Should return error when input validation fails. - $this->assertSame( 400, $response->get_status() ); + // Our security fix now catches input validation failures in permission check + $this->assertSame( 403, $response->get_status() ); $data = $response->get_data(); - $this->assertSame( 'ability_invalid_input', $data['code'] ); + $this->assertSame( 'rest_ability_cannot_execute', $data['code'] ); $this->assertSame( - 'Ability "test/strict-input" has invalid input. Reason: required_field is a required property of input.', + 'Sorry, you are not allowed to execute this ability.', $data['message'] ); } From 0fd34481a42136bdb26ef9302e2d8ff6c831aadf Mon Sep 17 00:00:00 2001 From: Mohamed Khaled Date: Thu, 2 Oct 2025 10:05:47 +0300 Subject: [PATCH 4/5] docs: Improve semantic heading structure in using-abilities documentation --- docs/4.using-abilities.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/docs/4.using-abilities.md b/docs/4.using-abilities.md index 8da0521e..37acc34d 100644 --- a/docs/4.using-abilities.md +++ b/docs/4.using-abilities.md @@ -1,8 +1,8 @@ -### 4. Using Abilities (`wp_get_ability`, `wp_get_abilities`) +# 4. Using Abilities (`wp_get_ability`, `wp_get_abilities`) Once abilities are registered, they can be retrieved and executed using global functions from the Abilities API. -**Getting a Specific Ability (`wp_get_ability`)** +## Getting a Specific Ability (`wp_get_ability`) To get a single ability object by its name (namespace/ability-name): @@ -33,7 +33,7 @@ if ( $site_info_ability ) { } ``` -**Getting All Registered Abilities (`wp_get_abilities`)** +## Getting All Registered Abilities (`wp_get_abilities`)** To get an array of all registered abilities: @@ -56,7 +56,7 @@ foreach ( $all_abilities as $name => $ability ) { } ``` -**Executing an Ability (`$ability->execute()`)** +## Executing an Ability (`$ability->execute()`) Once you have a `WP_Ability` object (usually from `wp_get_ability`), you execute it using the `execute()` method. @@ -124,7 +124,7 @@ if ( $ability ) { } ``` -**Understanding Permissions** +## Understanding Permissions Abilities handle permission checking automatically during execution. The `execute()` method will check permissions and return a `WP_Error` if access is denied. You rarely need to check permissions separately. @@ -152,7 +152,7 @@ if ( $ability ) { } ``` -**Advanced Error Handling** +## Advanced Error Handling If you need to handle specific error types differently, you can check the error code: @@ -177,7 +177,7 @@ if ( $ability ) { } ``` -**Inspecting Ability Properties** +## Inspecting Ability Properties The `WP_Ability` class provides several getter methods to inspect ability properties: @@ -204,7 +204,7 @@ if ( $ability ) { } ``` -**Error Handling Patterns** +## Error Handling Patterns The Abilities API uses several error handling mechanisms: From 27b7b1a8215023b128cbcc55a27f51ac23e8774c Mon Sep 17 00:00:00 2001 From: Mohamed Khaled Date: Thu, 23 Oct 2025 10:18:28 +0300 Subject: [PATCH 5/5] Preserve specific error messages from permission callbacks Don't replace contextually specific errors from check_permissions() with generic 'Sorry, you are not allowed' error. If check_permissions() returns a WP_Error, preserve and return it directly. Only use generic error as fallback when permission check returns false. --- .../endpoints/class-wp-rest-abilities-run-controller.php | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/includes/rest-api/endpoints/class-wp-rest-abilities-run-controller.php b/includes/rest-api/endpoints/class-wp-rest-abilities-run-controller.php index a1c663a0..c5e97cf4 100644 --- a/includes/rest-api/endpoints/class-wp-rest-abilities-run-controller.php +++ b/includes/rest-api/endpoints/class-wp-rest-abilities-run-controller.php @@ -162,8 +162,13 @@ public function run_ability_permissions_check( $request ) { ); } - $input = $ability->normalize_input( $this->get_input_from_request( $request ) ); - if ( ! $ability->check_permissions( $input ) ) { + $input = $ability->normalize_input( $this->get_input_from_request( $request ) ); + $result = $ability->check_permissions( $input ); + if ( true !== $result ) { + if ( is_wp_error( $result ) ) { + return $result; + } + return new \WP_Error( 'rest_ability_cannot_execute', __( 'Sorry, you are not allowed to execute this ability.' ),