Security hardening: input validation and fail-closed permissions

[galatanovidiu]

Why

Three gaps flagged during the v0.5.0 code review and PR #151 review:

1. arguments parameter not type-checked. tools/call and prompts/get accept whatever the client sends as arguments — a string, integer, or boolean passes silently into the execution layer. The MCP spec requires arguments to be an object (associative array in PHP).

2. Permission callback failures fail open. When a custom transport_permission_callback returns WP_Error or throws an exception, HttpTransport::check_permission() falls through to the weaker default capability check instead of denying access. A broken permission callback should not silently degrade security.

3. Inconsistent pre-filter error types. The mcp_adapter_pre_prompt_get filter returns permission_denied (-32008) on WP_Error short-circuit, while mcp_adapter_pre_resource_read returns internal_error (-32603). Pre-filter short-circuits represent generic blocks (rate limiting, maintenance mode, content policy), not permission failures. Fixes Normalize WP_Error short-circuit error types across pre-execution filters #152.

What

Input validation (ToolsHandler, PromptsHandler):

• Added is_array() check on arguments before use in both call_tool() and get_prompt()
• Non-array arguments return invalid_params (-32602) with message "arguments must be an object"
• Null and missing arguments continue to default to [] — no behavior change on the happy path

Fail-closed permissions (HttpTransport):

• check_permission() now returns false when the custom callback returns WP_Error or throws
• Errors are still logged via the error handler
• Updated filter PHPDoc to remove the "or when the custom callback fails" clause since it no longer falls through

Pre-filter error normalization (PromptsHandler):

• Changed mcp_adapter_pre_prompt_get WP_Error handling from permission_denied() to internal_error()
• Now consistent with mcp_adapter_pre_resource_read

Test plan

• 8 new unit tests covering string/integer arguments and null/missing arguments for both handlers
• Updated integration tests for WP_Error and exception permission callback scenarios
• Updated prompt handler test to verify -32603 error code on pre-filter short-circuit
• Full suite: 983 tests, 3792 assertions, 0 failures
• PHPStan Level 8: 0 errors
• PHPCS: 0 violations

[github-actions]

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.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: galatanovidiu <ovidiu-galatan@git.wordpress.org>
Co-authored-by: gziolo <gziolo@git.wordpress.org>

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.76%. Comparing base (1111068) to head (2551f5c).
⚠️ Report is 3 commits behind head on trunk.

Additional details and impacted files

@@             Coverage Diff              @@
##              trunk     #153      +/-   ##
============================================
+ Coverage     87.74%   87.76%   +0.01%     
- Complexity     1230     1234       +4     
============================================
  Files            54       54              
  Lines          3990     3996       +6     
============================================
+ Hits           3501     3507       +6     
  Misses          489      489              

Flag	Coverage Δ
unit	87.76% <100.00%> (+0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Copilot]

Pull request overview

This PR hardens the MCP adapter’s request handling by tightening input validation, making transport permissions fail closed, and normalizing pre-filter short-circuit error types to be consistent with the resource handler behavior.

Changes:

• Validate arguments is an array for tools/call and prompts/get, returning invalid_params when not.
• Change HttpTransport::check_permission() to deny access when the custom permission callback throws or returns WP_Error (fail-closed), while logging the failure.
• Normalize mcp_adapter_pre_prompt_get short-circuit behavior to return internal_error (-32603) instead of permission_denied.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
includes/Handlers/Tools/ToolsHandler.php	Adds arguments type validation for tools/call.
includes/Handlers/Prompts/PromptsHandler.php	Adds arguments type validation and normalizes pre-filter WP_Error to internal_error.
includes/Transport/HttpTransport.php	Changes permission callback failure handling to fail closed and updates filter PHPDoc accordingly.
tests/Unit/Handlers/ToolsHandlerCallTest.php	Adds unit tests for invalid/non-array arguments and null/missing arguments.
tests/Unit/Handlers/PromptsHandlerTest.php	Updates pre-filter short-circuit assertions and adds unit tests for argument validation.
tests/Integration/HttpTransportTest.php	Updates integration coverage to assert fail-closed behavior and logging for permission callback failures.

---

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

[gziolo]

All checks pass locally — 983 tests, PHPCS clean, PHPStan Level 8 clean.

The three fixes are correct and well-motivated:

1. Arguments type validation — proper isset() && !is_array() guard in both handlers, correct error code (-32602), null/missing arguments unaffected.
2. Fail-closed permissions — WP_Error and exception paths now deny access instead of silently falling through to the weaker default check.
3. Pre-filter error normalization — aligns mcp_adapter_pre_prompt_get with mcp_adapter_pre_resource_read behavior.

Previously raised inconsistencies were properly addressed. Great refactor overall.