Change #1269034 had a related patch set uploaded (by HakanIST; author: HakanIST):

[mediawiki/extensions/GrowthExperiments@master] fix: Gracefully handle invalid userId in NewcomerTasksCacheRefreshJob

https://gerrit.wikimedia.org/r/1269034

There are 7+ callers of suggest(), most of which should only be called for logged-in users. ApiQueryGrowthTasks is safe ( isNamed() guard), but other callers like HomepageHooks::onBeforePageDisplay (line 425) or SuggestedEdits::resetTaskCache could potentially pass an anonymous user.

Root cause identified and fix uploaded.

CacheDecorator::suggest() enters the cache path for all users without checking userId. For userId=0 this creates a shared cache key and enqueues a NewcomerTasksCacheRefreshJob that fails because UserIdentityLookup cannot resolve userId=0.

Fix in https://gerrit.wikimedia.org/r/c/mediawiki/extensions/GrowthExperiments/+/1269034 (PS4): bypass the cache path entirely for userId=0 at the CacheDecorator level. The LogicException in the job is preserved as a safety net.

Edit: PS8 adds diagnostic logging with stack traces at both job scheduling sites instead. Should reveal which caller reaches this code with userId=0.

Change #1269034 had a related patch set uploaded (by HakanIST; author: HakanIST):

[mediawiki/extensions/GrowthExperiments@master] Add diagnostic logging for T419172

https://gerrit.wikimedia.org/r/1269034

Change #1269034 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] NewcomerTasks: Add logging for unexpected anon users

https://gerrit.wikimedia.org/r/1269034

After the diagnostic logging, the Logstash hits I found all point to HomepageHooks::onBeforePageDisplay() line 425. It looks like anonymous users can reach URLs with GE parameters (geclickid, gesuggestededit), and the click-ID block ends up calling suggest() with userId=0 without a registration check.

We seem to have data from the logging that was added:

from /srv/mediawiki/php-1.47.0-wmf.1/extensions/GrowthExperiments/includes/NewcomerTasks/TaskSuggester/CacheDecorator.php(156)
#0 /srv/mediawiki/php-1.47.0-wmf.1/includes/libs/ObjectCache/WANObjectCache.php(1830): GrowthExperiments\NewcomerTasks\TaskSuggester\CacheDecorator->GrowthExperiments\NewcomerTasks\TaskSuggester\{closure}(bool, int, array, null, array)
#1 /srv/mediawiki/php-1.47.0-wmf.1/includes/libs/ObjectCache/WANObjectCache.php(1640): Wikimedia\ObjectCache\WANObjectCache->fetchOrRegenerate(string, int, Closure, array, array)
#2 /srv/mediawiki/php-1.47.0-wmf.1/extensions/GrowthExperiments/includes/NewcomerTasks/TaskSuggester/CacheDecorator.php(78): Wikimedia\ObjectCache\WANObjectCache->getWithSetCallback(string, int, Closure, array)
#3 /srv/mediawiki/php-1.47.0-wmf.1/extensions/GrowthExperiments/includes/NewcomerTasks/TaskSuggester/QualityGateDecorator.php(73): GrowthExperiments\NewcomerTasks\TaskSuggester\CacheDecorator->suggest(MediaWiki\User\User, GrowthExperiments\NewcomerTasks\Task\TaskSetFilters, int, null, array)
#4 /srv/mediawiki/php-1.47.0-wmf.1/extensions/GrowthExperiments/includes/HomepageHooks.php(385): GrowthExperiments\NewcomerTasks\TaskSuggester\QualityGateDecorator->suggest(MediaWiki\User\User, GrowthExperiments\NewcomerTasks\Task\TaskSetFilters, int)
#5 /srv/mediawiki/php-1.47.0-wmf.1/includes/HookContainer/HookContainer.php(127): GrowthExperiments\HomepageHooks->onBeforePageDisplay(MediaWiki\Output\OutputPage, MediaWiki\Minerva\Skins\SkinMinerva)
#6 /srv/mediawiki/php-1.47.0-wmf.1/includes/HookContainer/HookRunner.php(1025): MediaWiki\HookContainer\HookContainer->run(string, array, array)
#7 /srv/mediawiki/php-1.47.0-wmf.1/includes/Output/OutputPage.php(3304): MediaWiki\HookContainer\HookRunner->onBeforePageDisplay(MediaWiki\Output\OutputPage, MediaWiki\Minerva\Skins\SkinMinerva)
#8 /srv/mediawiki/php-1.47.0-wmf.1/includes/Actions/ActionEntryPoint.php(162): MediaWiki\Output\OutputPage->output(bool)
#9 /srv/mediawiki/php-1.47.0-wmf.1/includes/MediaWikiEntryPoint.php(180): MediaWiki\Actions\ActionEntryPoint->execute()
#10 /srv/mediawiki/php-1.47.0-wmf.1/index.php(44): MediaWiki\MediaWikiEntryPoint->run()
#11 /srv/mediawiki/w/index.php(3): require(string)
#12 {main}

If I'm looking at the code around HomepageHooks.php(385), then my guess is that someone bookmarked or shared a URL with getasktype and geclickid URL params and then this URL is visited by users that are not logged-in.

So, an immediate first fix would probably be to introuce a check for User->isNamed() at the beginning of \GrowthExperiments\HomepageHooks::onBeforePageDisplay and if they are not then to exit that method early. The homepage exists exclusively for named users, so none of that code should run for anon or temp users.

A more substantial fix would be to not only have a getasktype and a geclickid, but also a CSRF-token in the url, and if the token does not check out to discard all the rest. That should prevent bookmarked and shared links to create noise in our setup. But this is maybe a bit of a bigger lift.

In T419172#11890448, @HakanIST wrote:

After the diagnostic logging, the Logstash hits I found all point to HomepageHooks::onBeforePageDisplay() line 425. It looks like anonymous users can reach URLs with GE parameters (geclickid, gesuggestededit), and the click-ID block ends up calling suggest() with userId=0 without a registration check.

Sorry, I totally missed your comment here! I think in my comment above I came to the same conclusion as you did. Do you want to take on the next step of fixing this?

Change #1289353 had a related patch set uploaded (by HakanIST; author: HakanIST):

[mediawiki/extensions/GrowthExperiments@master] HomepageHooks: Skip onBeforePageDisplay for non-named users

https://gerrit.wikimedia.org/r/1289353

In T419172#11933150, @Michael wrote:

Sorry, I totally missed your comment here! I think in my comment above I came to the same conclusion as you did. Do you want to take on the next step of fixing this?

No problem, added the isNamed() guard at the top of onBeforePageDisplay() as you suggested.

Change #1289353 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] HomepageHooks: Skip onBeforePageDisplay for non-named users

https://gerrit.wikimedia.org/r/1289353

Looking at Logstash, the LogicException dropped to zero on Jun 11 after the remaining delayed jobs from before the fix expired.