PCG: follow redirects on probe loopback to fix "Too many redirects" (#48897)

* PCG: follow redirects on probe loopback to fix "Too many redirects"

The probe was firing with redirects=0, so any 3xx response from the
loopback (force_ssl_admin's http->https bounce on the admin probe,
canonical http->https / trailing-slash on the front-end probe) threw
"Too many redirects" — about 11k of these in logstash. Switching to
redirects=3 lets Requests follow the bounce, re-sending the forwarded
Cookie header on the followed request, so admin auth still validates
and the probe handler still fires at the canonical front-end URL
(WP emits full-URL Location headers so pcg_probe/token survives).

parse_response now classifies:

- Too-many-redirects exception (budget exceeded) -> ok-inconclusive,
  matched on WpOrg\Requests\Exception::getType() === 'toomanyredirects'
  so a future upstream wording change can't reclassify these.
- 3xx safety net (Requests refused to follow — cross-scheme downgrade,
  malformed Location, etc.) -> ok-inconclusive, broadened from the
  admin-only 301/302 check that's no longer reachable for the common
  cases.
- 2xx with redirect_count > 0 -> ok-inconclusive (destination dropped
  our pcg_probe query string), distinguished from the no-redirects 2xx
  which still classifies as fatal (real exit/die during bootstrap).

Drop the now-unused $is_admin param on parse_response.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* PCG: bump redirect budget from 3 to 5 to match wp_remote_get default

Absorbs multilingual / 4-hop canonical chains (http -> https -> www ->
trailing-slash -> locale-prefix) without changing the failure mode at
the cap (still ok-inconclusive, non-blocking). Aligns with WP core's
own loopback convention.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* PCG: tighten 'redirects' rationale comment

Drop a few lines while keeping the load-bearing facts: budget aligns
with wp_remote_get's default, what kinds of hops it absorbs, and why
the probe query + auth survive the bounce.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* PCG: update changelog blurb to reflect 5-redirect budget

Stale "up to 3" from the initial commit; bumped to 5 to match
wp_remote_get's default.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* PCG: defer probe transient delete until verdict emits

If force_ssl_admin redirects http->https before admin_init runs, the
loopback follows with the same token but the transient was already
deleted on the first hit, so the follow-up bailed with "Invalid or
expired probe token". Stash the key and delete only inside the
terminal responder; the 30s TTL caps any lingering entries.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* PCG: strip forwarded Cookie on cross-host probe redirect

Now that the loopback follows up to 5 redirects, a misconfigured admin
URL that bounces off-host would leak the forwarded auth cookies to the
third party. Register a per-request requests.before_redirect hook that
drops the Cookie header whenever the redirect target's host differs
from the original admin host.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* PCG: trim probe comments and simplify redirect cookie strip

Drop diff-narrating comments around the redirect/2xx/3xx branches in
parse_response and the redirect budget block. Collapse the case-insensitive
Cookie-header scan in the cross-host redirect hook to a direct unset
(Requests passes through the caller's exact-case key). Rename the closure's
$return param to $return_value to satisfy PHPCS, and realign equals signs.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* PCG: also strip Cookie on same-host https→http redirect

Address review: relative redirects inherit the original origin/scheme,
and https→http on the same host now drops the forwarded Cookie too.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* PCG: case-insensitive Cookie strip in redirect hook

Future-proof against a Requests change that normalizes header keys
before invoking before_redirect: scan req_headers case-insensitively
so the cross-host / scheme-downgrade Cookie strip can't silently
no-op if the key arrives as 'cookie' or 'COOKIE'.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: arthur791004

projects/packages/jetpack-mu-wpcom/changelog/pcg-follow-redirects

@@ -0,0 +1,4 @@
+Significance: patch
+Type: changed
+
+Plugin Conflicts Guardian: follow up to 5 redirects on the probe loopback so canonical http→https and force_ssl_admin scheme bounces stop returning "Too many redirects".

projects/packages/jetpack-mu-wpcom/src/features/plugin-conflicts-guardian/README.md

@@ -26,7 +26,7 @@ Two independent filters:
 2. `activation-guard.php` intercepts on `load-plugins.php` / `load-update.php` priority 0, verifies the nonce, filters the request down to eligible plugins (passes `validate_file`, not already active, file exists), and calls `PCG_Load_Tester::test()` once with the full batch.
 3. The load tester stashes `{ plugins, mode }` in a short-lived transient keyed by a random token, then fires the probe against `?pcg_probe=1&token=…` on this same site. Activation flows pass `mode = activation`; the post-update health check passes `mode = update` (see "Post-update health check" below).
 4. `probe-endpoint.php` runs synchronously at require time (already inside `plugins_loaded` priority 10 via `load_features()`), validates + consumes the token, gates on the per-mode filter (`pcg_guard_activation` for activation, `pcg_guard_updates` for update), defines `WP_SANDBOX_SCRAPING` so core's fatal handler steps aside, arms a shutdown handler, and in activation mode `require_once`s each plugin's main file in order under that single request. Probe cost is constant regardless of how many plugins are activated, and conflicts that only fire when two plugins load together (duplicate class, shared global) are caught — which a per-plugin probe model couldn't see. In update mode the files are already loaded by WP's normal bootstrap and re-requiring would fatal with "Cannot redeclare class/function" — the probe just verifies that bootstrap completed cleanly.
-5. Two probes fire in parallel via `\WpOrg\Requests\Requests::request_multiple()`: one against `home_url('/')` (front-end) and one against `admin_url('index.php')` with `pcg_admin=1` and the admin's WP auth cookies forwarded so `auth_redirect()` clears. The admin probe defers its verdict to `admin_init` priority `PHP_INT_MAX`; the front-end probe emits on `wp_loaded`. A captured `fatal` / `throwable` from either probe wins; otherwise the front-end verdict is returned. A 302 on the admin probe (cookies missing/expired) becomes a distinct `ok-inconclusive` status that's still treated as a non-blocking pass — that way transport quirks don't break activation, but the signal can be measured separately from a clean `ok`.
+5. Two probes fire in parallel via `\WpOrg\Requests\Requests::request_multiple()`: one against `home_url('/')` (front-end) and one against `admin_url('index.php')` with `pcg_admin=1` and the admin's WP auth cookies forwarded so `auth_redirect()` clears. Requests follows up to 5 redirects (matching `wp_remote_get`'s default), so canonical http→https / trailing-slash / non-www→www on the front-end and `force_ssl_admin`'s http→https bounce on the admin probe both reach a real verdict (WP emits full-URL `Location:` headers from `home_url`/`set_url_scheme` so `pcg_probe`/`token` survives, and Requests re-sends the forwarded `Cookie:` header on the followed request so admin auth still validates after the scheme bounce). The admin probe defers its verdict to `admin_init` priority `PHP_INT_MAX`; the front-end probe emits on `wp_loaded`. A captured `fatal` / `throwable` from either probe wins; otherwise the front-end verdict is returned. Inconclusive verdicts (exceeded redirect budget, destination dropped the probe query, or unfollowed 3xx) become `ok-inconclusive` — a non-blocking pass since the bootstrap was healthy enough to redirect or render — so they're distinguishable from a clean `ok` in logs without breaking activation.
 6. On a fatal/throwable the guard attributes the failure to one plugin in the batch — preferring the explicit `plugin` field (set when a `Throwable` is caught around the `require`), then falling back to matching the captured `file` against each plugin's directory. The whole batch is blocked as a unit; the notice tells the admin which plugin caused the fatal so they can retry without it.
 
 ```

projects/packages/jetpack-mu-wpcom/src/features/plugin-conflicts-guardian/class-pcg-load-tester.php

@@ -60,7 +60,11 @@ public function test( array $plugin_mains, $mode = self::MODE_ACTIVATION ) {
 				),
 				array(
 					'timeout'   => self::PROBE_TIMEOUT,
-					'redirects' => 0,
+					// Match `wp_remote_get`'s default; covers http->https,
+					// force_ssl_admin's scheme bounce, and locale redirects.
+					// `build_same_host_cookie_hook` keeps admin auth from
+					// leaking if the redirect points off-host.
+					'redirects' => 5,
 				)
 			);
 		} catch ( \Throwable $t ) {
@@ -73,8 +77,8 @@ public function test( array $plugin_mains, $mode = self::MODE_ACTIVATION ) {
 			delete_transient( self::transient_key( $admin['token'] ) );
 		}
 
-		$front_result = $this->parse_response( $responses['front'], false );
-		$admin_result = $this->parse_response( $responses['admin'], true );
+		$front_result = $this->parse_response( $responses['front'] );
+		$admin_result = $this->parse_response( $responses['admin'] );
 
 		// Log transport-level errors (most often timeouts at PROBE_TIMEOUT)
 		// so we can see how often they fire before deciding whether to
@@ -209,11 +213,13 @@ protected function prepare_probe( array $plugin_mains, $base_url, $is_admin, $mo
 			'token'     => $token,
 		);
 		$headers = array();
+		$options = array();
 		if ( $is_admin ) {
 			$query['pcg_admin'] = '1';
 			$cookie_header      = $this->collect_auth_cookie_header();
 			if ( '' !== $cookie_header ) {
 				$headers['Cookie'] = $cookie_header;
+				$options['hooks']  = $this->build_same_host_cookie_hook( $base_url );
 			}
 		}
 
@@ -223,6 +229,7 @@ protected function prepare_probe( array $plugin_mains, $base_url, $is_admin, $mo
 				'url'     => add_query_arg( $query, $base_url ),
 				'type'    => 'GET',
 				'headers' => $headers,
+				'options' => $options,
 			),
 		);
 	}
@@ -232,31 +239,41 @@ protected function prepare_probe( array $plugin_mains, $base_url, $is_admin, $mo
 	 *
 	 * @param mixed $response A `WpOrg\Requests\Response`, or an exception
 	 *                        thrown for that single request.
-	 * @param bool  $is_admin True when this was the admin probe.
 	 * @return array{status:string,reason?:string,errno?:int,class?:string,message?:string,file?:string,line?:int,plugin?:string}
 	 */
-	protected function parse_response( $response, $is_admin ) {
+	protected function parse_response( $response ) {
 		if ( $response instanceof \Throwable ) {
+			// Bootstrap was healthy enough to issue several redirects in
+			// a row, so treat redirect-budget exhaustion as inconclusive
+			// rather than an error.
+			if ( $response instanceof \WpOrg\Requests\Exception && 'toomanyredirects' === $response->getType() ) {
+				return array(
+					'status' => 'ok-inconclusive',
+					'reason' => 'Probe exceeded redirect budget; treating as inconclusive ok.',
+				);
+			}
 			return array(
 				'status' => 'error',
 				'reason' => sprintf( 'Probe request failed: %s', $response->getMessage() ),
 			);
 		}
 
-		$code = (int) ( $response->status_code ?? 0 );
-		$body = (string) ( $response->body ?? '' );
+		$code           = (int) ( $response->status_code ?? 0 );
+		$body           = (string) ( $response->body ?? '' );
+		$redirect_count = (int) ( $response->redirects ?? 0 );
 
 		$decoded = json_decode( $body, true );
 		if ( is_array( $decoded ) && isset( $decoded['status'] ) ) {
 			return $decoded;
 		}
 
-		// Admin probe bounced to login (no/expired cookie). Distinct status so
-		// we can measure how often it fires; treated as ok by callers.
-		if ( $is_admin && ( 301 === $code || 302 === $code ) ) {
+		// 3xx that Requests refused to follow (cross-scheme downgrade,
+		// malformed Location). Treat as ok — bootstrap completed enough
+		// to emit one.
+		if ( $code >= 300 && $code < 400 ) {
 			return array(
 				'status' => 'ok-inconclusive',
-				'reason' => 'Admin probe redirected; treating as inconclusive ok.',
+				'reason' => sprintf( 'Probe redirected (HTTP %d); treating as inconclusive ok.', $code ),
 			);
 		}
 
@@ -267,10 +284,18 @@ protected function parse_response( $response, $is_admin ) {
 			);
 		}
 
-		// Probe endpoint always emits JSON; a 2xx without one means the
-		// bootstrap was terminated mid-flight (exit/die during load/init/admin_init).
-		// Block, since the same termination would affect matching future requests.
 		if ( $code >= 200 && $code < 300 ) {
+			// Followed a redirect whose destination dropped the probe
+			// query. Bootstrap rendered cleanly, so don't block.
+			if ( $redirect_count > 0 ) {
+				return array(
+					'status' => 'ok-inconclusive',
+					'reason' => sprintf( 'Probe followed %d redirect(s) but destination dropped the probe query; treating as inconclusive ok.', $redirect_count ),
+				);
+			}
+			// Probe endpoint always emits JSON; a 2xx without one and no
+			// redirect means the bootstrap was terminated mid-flight
+			// (exit/die during load/init/admin_init).
 			return array(
 				'status'  => 'fatal',
 				'message' => sprintf(
@@ -309,6 +334,47 @@ protected function collect_auth_cookie_header() {
 		return implode( '; ', $pairs );
 	}
 
+	/**
+	 * Build a Hooks instance that strips the forwarded `Cookie:` header on
+	 * any redirect that leaves the original origin: off-host, or an
+	 * https→http scheme downgrade on the same host. Defends against
+	 * leaking admin auth cookies (we forward `Cookie:` manually, so
+	 * Requests won't enforce browser `Secure` semantics for us).
+	 * Relative redirects inherit the original origin and pass through.
+	 *
+	 * @param string $original_url Initial probe URL whose origin is the trust boundary.
+	 * @return \WpOrg\Requests\Hooks
+	 */
+	protected function build_same_host_cookie_hook( $original_url ) {
+		$original        = wp_parse_url( $original_url );
+		$original_host   = isset( $original['host'] ) ? strtolower( (string) $original['host'] ) : '';
+		$original_scheme = isset( $original['scheme'] ) ? strtolower( (string) $original['scheme'] ) : '';
+
+		$hooks = new \WpOrg\Requests\Hooks();
+		$hooks->register(
+			'requests.before_redirect',
+			static function ( &$location, &$req_headers, &$req_data, &$options, $return_value ) use ( $original_host, $original_scheme ) {
+				unset( $req_data, $options, $return_value );
+				if ( ! is_array( $req_headers ) ) {
+					return;
+				}
+				$next             = wp_parse_url( (string) $location );
+				$next_host        = isset( $next['host'] ) ? strtolower( (string) $next['host'] ) : $original_host;
+				$next_scheme      = isset( $next['scheme'] ) ? strtolower( (string) $next['scheme'] ) : $original_scheme;
+				$same_host        = '' !== $next_host && $next_host === $original_host;
+				$scheme_downgrade = 'https' === $original_scheme && 'https' !== $next_scheme;
+				if ( ! $same_host || $scheme_downgrade ) {
+					foreach ( array_keys( $req_headers ) as $key ) {
+						if ( 0 === strcasecmp( (string) $key, 'Cookie' ) ) {
+							unset( $req_headers[ $key ] );
+						}
+					}
+				}
+			}
+		);
+		return $hooks;
+	}
+
 	/**
 	 * Transient key for a probe token. Shared with the endpoint.
 	 *

projects/packages/jetpack-mu-wpcom/src/features/plugin-conflicts-guardian/probe-endpoint.php

@@ -30,11 +30,17 @@ function pcg_maybe_handle_probe() {
 
 	$key     = PCG_Load_Tester::transient_key( $token );
 	$payload = get_transient( $key );
-	delete_transient( $key );
 
 	if ( ! is_array( $payload ) || ! isset( $payload['plugins'] ) || ! isset( $payload['mode'] ) ) {
 		pcg_probe_bail_error( 'Invalid or expired probe token.', 403 );
 	}
+
+	// Defer deletion until we actually emit a verdict. If something redirects
+	// (e.g. force_ssl_admin's http->https bounce) before admin_init, Requests
+	// follows with the same token; deleting upfront would make the follow-up
+	// fail with "Invalid or expired probe token". The 30s transient TTL caps
+	// lingering entries when no terminal response runs.
+	pcg_probe_pending_key( $key );
 	$plugin_mains = is_array( $payload['plugins'] ) ? array_values(
 		array_filter(
 			array_map( static fn( $p ) => (string) $p, $payload['plugins'] ),
@@ -145,13 +151,31 @@ function pcg_probe_shutdown() {
  * @return never
  */
 function pcg_probe_respond( $payload, $status = 200 ) {
+	$key = pcg_probe_pending_key();
+	if ( '' !== $key ) {
+		delete_transient( $key );
+	}
 	while ( ob_get_level() > 0 ) {
 		ob_end_clean();
 	}
 	wp_send_json( $payload, (int) $status, JSON_UNESCAPED_SLASHES );
 	exit;
 }
 
+/**
+ * Get/set the transient key to delete when we emit a verdict.
+ *
+ * @param string|null $set Key to remember; omit to read the current value.
+ * @return string
+ */
+function pcg_probe_pending_key( $set = null ) {
+	static $key = '';
+	if ( null !== $set ) {
+		$key = (string) $set;
+	}
+	return $key;
+}
+
 /**
  * Emit an `error` verdict with the given reason + HTTP status, and terminate.
  *