Plugin Directory: Validate the security scan callback contract at the route

[obenland]

Summary

The Gandalf callback route currently validates nothing: the handler trusts every field of the body, the plugin is resolved via $request['plugin_slug'] — where JSON body params outrank the URL segment, so a body field named plugin_slug overrides the routed identity — and an unresolvable slug reaches the handler as a null plugin. This PR moves contract enforcement to the route, ahead of #777, whose scan policy acts on these fields (max_risk_score deciding whether a release is blocked deserves a validated type).

What it does

• The route args carry the callback body schema: types, the status/subject_type enums, required identity fields. Gandalf_Scan::validate_callback_data() adds the per-status required sets (a completed callback must carry its verdict fields; a failed one its error).
• The plugin is resolved from the URL segment via get_url_params(), a payload asserting a different plugin than it was routed to is rejected and recorded via record_invalid_callback(), and an unknown slug is rejected at URL-arg validation.

Deliberately lenient

The contract is strict only about what the directory acts on. Everything descriptive is open, so Gandalf can evolve without voiding deliveries:

• No unknown-field rejection at any level — new top-level fields, new finding fields, new investigation fields all flow through.
• No enums on display-only fields — a new severity or investigation status/result is accepted as a plain string.
• No upper bound on scores — a recalibrated scale doesn't 400; the threshold comparison is the policy layer's job.
• No string length caps — an over-long model-generated title must not void a verdict.

Testing

tests/Gandalf_Scan_Endpoint_Test.php dispatches a production-shaped callback body (three findings, multi-paragraph explanations, code snippets, skipped investigations) through rest_do_request(), covering the layers unit tests bypass: bearer authentication (missing and wrong token), routing, schema rejections (mistyped and missing fields), unknown-plugin rejection, slug reconciliation, and — the leniency regression test — contract additions at every level plus a novel severity and an out-of-range score flowing through to a 200.

Notes

Split out of #777, which will be rebased on top: its scan policy reads callback fields on the promise that the route validated them.

🤖 Generated with Claude Code

[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.

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

Props obenland, lucasbustamante.

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

[Copilot]

Pull request overview

This PR tightens the Plugin Directory’s Gandalf security-scan callback endpoint by enforcing a request contract at the REST route layer (schema + cross-field validation), preventing plugin identity confusion between URL and body payload, and adding end-to-end REST tests to exercise authentication, routing, validation, and acceptance/leniency behavior.

Changes:

• Adds a detailed REST arg schema for the Gandalf callback payload and a validate_callback_data() cross-field validator.
• Resolves the plugin strictly from the URL segment and rejects callbacks whose payload asserts a different plugin slug.
• Introduces a comprehensive REST-dispatch test suite for valid callbacks, auth failures, schema failures, slug reconciliation, and leniency toward unknown fields.

Reviewed changes

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

File	Description
wordpress.org/public_html/wp-content/plugins/plugin-directory/api/routes/class-gandalf-scan.php	Adds route-level schema + cross-field validation and tightens plugin identity resolution to the URL param.
wordpress.org/public_html/wp-content/plugins/plugin-directory/tests/Gandalf_Scan_Endpoint_Test.php	New end-to-end REST tests for callback auth, routing, validation, identity checks, and leniency regression.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

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

Suppressed comments (2)

wordpress.org/public_html/wp-content/plugins/plugin-directory/api/routes/class-gandalf-scan.php:255

• This WP_Error message is currently a raw string (not translatable), while other endpoint error messages use __() with the wporg-plugins textdomain. Consider localizing it for consistency.

		// The payload must assert the same plugin the callback was routed to.
		if ( ! $error && ( $data['slug'] ?? '' ) !== $plugin->post_name ) {
			$error = new WP_Error( 'invalid_gandalf_scan', 'Security scan callback slug does not match the plugin.', [ 'status' => WP_Http::BAD_REQUEST ] );
		}

wordpress.org/public_html/wp-content/plugins/plugin-directory/api/routes/class-gandalf-scan.php:214

• The missing-field error message here is not run through i18n, but other REST errors in this handler use __() with the wporg-plugins textdomain. Please make this string translatable as well (and keep the sprintf() placeholder).

This issue also appears on line 252 of the same file.

			if ( ! isset( $data[ $field ] ) ) {
				return new WP_Error(
					'invalid_gandalf_scan_callback',
					sprintf( 'Invalid security scan callback: missing %s.', $field ),
					[ 'status' => WP_Http::BAD_REQUEST ]
				);

[obenland]

@Luc45 Could you take a look at this when you get a chance?

The intention is to be a bit more specific about what the endpoint expects, so that consumers down the stack can rely on certain information to be available, without constraining future changes to the payload too much.
As in, you should always be able to add stuff without breaking the callback endpoint.

[Luc45]

I agree with validating at the boundary so that downstream consumers can rely on it without having to check for isset, adding nullables, fallbacks, etc.

I can add automated tests on Gandalf to make sure I never break the contract expected by this callback endpoint. To keep it up-to-date, I'd fetch the actual file wordpress.org/public_html/wp-content/plugins/plugin-directory/api/routes/class-gandalf-scan.php from trunk as part of the test.

And I ack that this is a guarantee of presence and format of expected fields, not a strict limitation for new fields.

I'll approve it as soon as I verify Gandalf sends the callback as expected.

[Luc45]

I ran this PR locally and pointed Gandalf locally against it, the callbacks were accepted, both for success and failed scans, and also asserted that a mistyped max_risk_score was rejected.