From 8a6c0e398a2674e7ebd9a48653852cc0efe823e0 Mon Sep 17 00:00:00 2001 From: Konstantin Obenland Date: Sun, 16 Aug 2026 17:43:53 -0700 Subject: [PATCH 1/3] Slack: Stop unauthenticated requests from fataling the webhook endpoints. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `announce.php` and `committers.php` pass `$_POST['token']` straight to `hash_equals()`. Both are Slack outgoing-webhook endpoints, so they assume a POST body carrying a token. A bare GET — which scanners send regularly — leaves `$_POST` empty, and PHP 8 raises an uncaught `TypeError` instead of the PHP 7 non-string warning, filling the error log with fatals: Uncaught TypeError: hash_equals(): Argument #2 ($user_string) must be of type string, null given in .../dotorg/slack/announce.php:32 Guard both with an `isset()`/`is_string()`/non-empty check before comparing. An array token (`token[]=x`) would otherwise trigger the same `TypeError`, and the non-empty check avoids matching a misconfigured empty constant. The superglobal stays inside `hash_equals()` so the comparison remains timing-safe. Also `break` out of the token loop in `announce.php` after a match. `run()` returns rather than exits, so the loop kept testing the remaining `WEBHOOK_TOKEN_N` constants and would fire twice if two ever held the same value. Unauthenticated requests now get the same empty 200 that a wrong token already produced. Co-Authored-By: Claude Opus 5 (1M context) --- api.wordpress.org/public_html/dotorg/slack/announce.php | 6 ++++++ api.wordpress.org/public_html/dotorg/slack/committers.php | 5 +++++ 2 files changed, 11 insertions(+) diff --git a/api.wordpress.org/public_html/dotorg/slack/announce.php b/api.wordpress.org/public_html/dotorg/slack/announce.php index 04a7c8918b..01d5ca5469 100644 --- a/api.wordpress.org/public_html/dotorg/slack/announce.php +++ b/api.wordpress.org/public_html/dotorg/slack/announce.php @@ -26,11 +26,17 @@ function get_avatar( $username, $slack_id, $team_id ) { return sprintf( 'https://secure.gravatar.com/avatar/%s?s=96d=mm&r=G&%s', $hash, time() ); } +// Slack sends the token as POST data; anything else is not a webhook request. +if ( ! isset( $_POST['token'] ) || ! is_string( $_POST['token'] ) || '' === $_POST['token'] ) { + return; +} + $i = 0; // WEBHOOK_TOKEN_1, WEBHOOK_TOKEN_2, etc. while ( defined( __NAMESPACE__ . '\\WEBHOOK_TOKEN_' . ++$i ) ) { if ( hash_equals( constant( __NAMESPACE__ . '\\WEBHOOK_TOKEN_' . $i ), $_POST['token'] ) ) { run( $_POST ); + break; } } diff --git a/api.wordpress.org/public_html/dotorg/slack/committers.php b/api.wordpress.org/public_html/dotorg/slack/committers.php index e89f6a820b..491b502cef 100644 --- a/api.wordpress.org/public_html/dotorg/slack/committers.php +++ b/api.wordpress.org/public_html/dotorg/slack/committers.php @@ -6,6 +6,11 @@ require dirname( dirname( __DIR__ ) ) . '/includes/slack-config.php'; +// Slack sends the token as POST data; anything else is not a webhook request. +if ( ! isset( $_POST['token'] ) || ! is_string( $_POST['token'] ) || '' === $_POST['token'] ) { + return; +} + if ( ! hash_equals( WEBHOOK_TOKEN, $_POST['token'] ) ) { return; } From e498319b07c04909d610c398ef3e29d0b729326b Mon Sep 17 00:00:00 2001 From: Konstantin Obenland Date: Sun, 16 Aug 2026 18:13:27 -0700 Subject: [PATCH 2/3] Slack: Announce, Committers: Bring both handlers up to the coding standards. Flatten `announce.php`'s braced `namespace {}` / `namespace Dotorg\Slack\Announce {}` pair into a single unbraced declaration. The global block was never necessary: an included file's namespace comes from its own declaration, not the include site, and variable scope is namespace-independent, so `$wpdb` stays global either way. This also clears the `ScopeIndent` errors that the whole file body carried, without reindenting a single line of it. Both handlers now document their actual authentication mechanism in a file-level docblock and disable the two sniffs that cannot apply to them: these are standalone endpoints that never load WordPress, so there is no `wp_unslash()` to call and no nonce to verify. Slack authenticates with a shared token instead. The rest are ordinary fixes: file and function docblocks, multi-line call formatting in `get_avatar()`, single quotes on the SQL that needs no interpolation, brackets around `++$i` in string concatenation, and `dirname( __DIR__, 2 )` in place of nested `dirname()` calls, matching `props.php` and the Calendly webhook in this directory. `committers.php` echoes `$_POST['user_name']` back in its JSON response, so strip control characters from it and default it when absent. Both files now report zero PHPCS errors and warnings against the full ruleset. Co-Authored-By: Claude Opus 5 (1M context) --- .../public_html/dotorg/slack/announce.php | 60 ++++++++++++------- .../public_html/dotorg/slack/committers.php | 31 +++++++--- 2 files changed, 63 insertions(+), 28 deletions(-) diff --git a/api.wordpress.org/public_html/dotorg/slack/announce.php b/api.wordpress.org/public_html/dotorg/slack/announce.php index 01d5ca5469..acc74ad8c7 100644 --- a/api.wordpress.org/public_html/dotorg/slack/announce.php +++ b/api.wordpress.org/public_html/dotorg/slack/announce.php @@ -1,26 +1,49 @@ get_var( $wpdb->prepare( - "SELECT user_id FROM slack_users WHERE slack_id = %s", - $slack_id - ) ); + $wp_user_id = $wpdb->get_var( + $wpdb->prepare( + 'SELECT user_id FROM slack_users WHERE slack_id = %s', + $slack_id + ) + ); - $email = $wpdb->get_var( $wpdb->prepare( - "SELECT user_email FROM $wpdb->users WHERE ID = %d", - $wp_user_id - ) ); + $email = $wpdb->get_var( + $wpdb->prepare( + "SELECT user_email FROM $wpdb->users WHERE ID = %d", + $wp_user_id + ) + ); $hash = hash( 'sha256', strtolower( trim( $email ) ) ); return sprintf( 'https://secure.gravatar.com/avatar/%s?s=96d=mm&r=G&%s', $hash, time() ); @@ -33,12 +56,9 @@ function get_avatar( $username, $slack_id, $team_id ) { $i = 0; // WEBHOOK_TOKEN_1, WEBHOOK_TOKEN_2, etc. -while ( defined( __NAMESPACE__ . '\\WEBHOOK_TOKEN_' . ++$i ) ) { +while ( defined( __NAMESPACE__ . '\\WEBHOOK_TOKEN_' . ( ++$i ) ) ) { if ( hash_equals( constant( __NAMESPACE__ . '\\WEBHOOK_TOKEN_' . $i ), $_POST['token'] ) ) { run( $_POST ); break; } } - -} - diff --git a/api.wordpress.org/public_html/dotorg/slack/committers.php b/api.wordpress.org/public_html/dotorg/slack/committers.php index 491b502cef..a4e11150f9 100644 --- a/api.wordpress.org/public_html/dotorg/slack/committers.php +++ b/api.wordpress.org/public_html/dotorg/slack/committers.php @@ -1,10 +1,19 @@ 'wordpressdotorg', - 'link_names' => 1, - 'text' => sprintf( '@%s: Use the `/committers` command.', $_POST['user_name'] ), -) ); +// The Slack user name of whoever triggered the webhook, echoed back in the JSON response below. +$user_name = (string) filter_var( $_POST['user_name'] ?? '', FILTER_UNSAFE_RAW, FILTER_FLAG_STRIP_LOW ); + +// phpcs:ignore WordPress.WP.AlternativeFunctions.json_encode_json_encode -- No WP loaded. +echo json_encode( + array( + 'username' => 'wordpressdotorg', + 'link_names' => 1, + 'text' => sprintf( '@%s: Use the `/committers` command.', $user_name ), + ) +); exit; From c148fe93d589152689666b7dcada5c1241a18a4b Mon Sep 17 00:00:00 2001 From: Konstantin Obenland Date: Sun, 16 Aug 2026 18:27:50 -0700 Subject: [PATCH 3/3] Slack: Announce: Fix the Gravatar lookup for unlinked Slack accounts. `get_avatar()` hashed whatever `slack_users` returned. When a Slack ID has no linked WordPress.org account both queries return null, so `trim( null )` raised a PHP 8.1+ deprecation into the same error log this branch is clearing out, and `hash( 'sha256', '' )` handed every unlinked user the identical `e3b0c442...` Gravatar. Bail as soon as the `slack_users` lookup comes up empty, so the unlinked case no longer passes null into the second `prepare()` for a `WHERE ID = 0` round-trip that can never match, and keep a second check for a linked account with no email on file. `run()` in lib.php only consults this hook when Slack has no profile image, and gates on the result, so an empty return simply leaves the icon unset. The query string was also missing a separator: `?s=96d=mm&r=G` parses as `s=96d=mm` with no `d` at all, so the size was garbage and the `mm` default-avatar fallback never reached Gravatar. Co-Authored-By: Claude Opus 5 (1M context) --- .../public_html/dotorg/slack/announce.php | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/api.wordpress.org/public_html/dotorg/slack/announce.php b/api.wordpress.org/public_html/dotorg/slack/announce.php index acc74ad8c7..b9bc0c5f02 100644 --- a/api.wordpress.org/public_html/dotorg/slack/announce.php +++ b/api.wordpress.org/public_html/dotorg/slack/announce.php @@ -26,7 +26,7 @@ * @param string $username The Slack user name. Unused, part of the hook signature. * @param string $slack_id The Slack user ID to look up. * @param string $team_id The Slack team ID. Unused, part of the hook signature. - * @return string The Gravatar URL for the linked account. + * @return string The Gravatar URL, or an empty string when the Slack account is not linked. */ function get_avatar( $username, $slack_id, $team_id ) { // phpcs:ignore Generic.CodeAnalysis.UnusedFunctionParameter -- Signature is fixed by the call in lib.php. global $wpdb; @@ -38,6 +38,10 @@ function get_avatar( $username, $slack_id, $team_id ) { // phpcs:ignore Generic. ) ); + if ( ! $wp_user_id ) { + return ''; + } + $email = $wpdb->get_var( $wpdb->prepare( "SELECT user_email FROM $wpdb->users WHERE ID = %d", @@ -45,8 +49,12 @@ function get_avatar( $username, $slack_id, $team_id ) { // phpcs:ignore Generic. ) ); + if ( ! $email ) { + return ''; + } + $hash = hash( 'sha256', strtolower( trim( $email ) ) ); - return sprintf( 'https://secure.gravatar.com/avatar/%s?s=96d=mm&r=G&%s', $hash, time() ); + return sprintf( 'https://secure.gravatar.com/avatar/%s?s=96&d=mm&r=G&%s', $hash, time() ); } // Slack sends the token as POST data; anything else is not a webhook request.