Slack: Stop unauthenticated requests from fataling the webhook endpoints

[obenland]

announce.php and committers.php pass $_POST['token'] straight into 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 where PHP 7 only warned about the non-string argument:

E_ERROR: Uncaught TypeError: hash_equals(): Argument #2 ($user_string) must be
of type string, null given in dotorg/slack/announce.php:32
Source: GET https://api.wordpress.org/dotorg/slack/announce.php

Nothing leaks — the token comparison can never succeed — but every probe of these URLs writes a fatal to the error log.

Changes

Guard both endpoints with an isset() / is_string() / non-empty check before the comparison:

• is_string() matters because an array token (token[]=x) would otherwise trigger the same TypeError that a ?? '' fallback alone would not catch.
• The non-empty check avoids matching a misconfigured empty constant.
• The superglobal stays inside hash_equals() rather than being copied to a local, so the comparison remains timing-safe and PHPCS continues to credit hash_equals() as the sanitizer.

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; harmless while the tokens are distinct, but it would post the announcement twice if two were ever configured with the same value.

Unauthenticated requests now get the same empty 200 that a wrong token already produced.

Notes

• The WordPress.Security.ValidatedSanitizedInput.InputNotValidated error PHPCS already reported on both files — "possibly undefined superglobal array index", literally this bug — is gone, and no new sniffs are introduced. The remaining MissingUnslash errors are pre-existing and not fixable here: these endpoints don't bootstrap WordPress, so there is no wp_unslash(). The rest of dotorg/slack/ carries them too.
• I swept the other endpoints in the directory for the same pattern. trac-bot.php, security-team.php, and community-deputies-calendly-webhook.php already guard with ?? '' / empty(), and subgroup.php guards its signature headers. These two files were the only remaining instances.
• There is no test coverage under api.wordpress.org/public_html/dotorg/ — these files bootstrap outside WordPress and have no PHPUnit setup. Verification was php -l, a before/after PHPCS comparison, and an isolated check that return at the top level of a braced namespace {} block exits cleanly.

🤖 Generated with Claude Code

Coding standards

The repository's branch linter checks changed lines only, and the fix above tripped it, so a second commit takes both files to zero PHPCS errors and warnings against the full ruleset — not just on the changed lines.

• announce.php namespace flattened. The braced namespace {} / namespace Dotorg\Slack\Announce {} pair left the entire file body at zero indentation inside a scope PHPCS expects indented, so every line in the file tripped Generic.WhiteSpace.ScopeIndent. 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. Collapsing to one unbraced declaration clears that plus DisallowCurlyBraceSyntax, DisallowDeclarationWithoutName, and OneDeclarationPerFile — without reindenting a single body line.
• File-level docblocks on both handlers documenting the actual authentication mechanism, with phpcs:disable for the two sniffs that cannot apply: these endpoints never load WordPress, so there is no wp_unslash() to call and no nonce to verify.
• Ordinary fixes: function docblock on get_avatar(), multi-line call formatting on the two $wpdb->prepare() calls, single quotes on the SQL that needs no interpolation, brackets around ++$i in string concatenation, and dirname( __DIR__, 2 ) for the 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 control characters are stripped and the value defaults when absent.

Gravatar lookup

A third commit fixes two defects in get_avatar(), both surfaced while reviewing the reformatting above.

Unlinked accounts raised a deprecation and collided. 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 cleaning up — and hash( 'sha256', '' ) returned the constant e3b0c442… for every unlinked user, giving them all the same avatar. It now bails as soon as the slack_users lookup comes up empty, so that case no longer passes null into the second prepare() for a WHERE ID = 0 round-trip that can never match, with a second check for a linked account that has no email on file. run() only consults this hook when Slack has no profile image and gates on the result, so an empty return leaves the icon unset.

The query string was missing a separator. ?s=96d=mm&r=G parses as:

array( 's' => '96d=mm', 'r' => 'G' )   // no 'd' at all

So the size was a garbage value and the mm default-avatar fallback never reached Gravatar. Corrected to ?s=96&d=mm&r=G, which parses as s=96, d=mm, r=G.

Verification

No test coverage exists under api.wordpress.org/public_html/dotorg/ — these files bootstrap outside WordPress and have no PHPUnit setup. Verification was:

• php -l on both files, and full-file phpcs reporting zero errors and zero warnings.
• A harness with stand-ins for hyperdb, slack-config.php, and lib.php, exercising the flattened namespace: no token, empty token, array token, valid token matching the first constant, valid token matching the second, and a wrong token. Confirmed the guard rejects every malformed case without a fatal; that run() fires on a match with the get_avatar() hook resolving in the right namespace and global $wpdb seeing the value set by the un-namespaced include; that break stops the loop at the matching iteration; and that ( ++$i ) preserves the pre-increment.
• get_avatar() re-run against null, '', and a real address under error_reporting( E_ALL ): no deprecation, empty return on the first two, and the corrected URL parsing back to the intended four parameters.

Relationship to #802

#802 overlaps this PR on both files — it independently flattens the same namespace, converts the same dirname() calls, and adds an equivalent token guard. Whichever lands first, the other needs a rebase. Unique here are the break after run( $_POST ), the rejection of an empty-string token, and the get_avatar() fixes above; #802 additionally hardens the five other handlers in this directory.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

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

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