Seems likely fall out from one of these two:

• https://gerrit.wikimedia.org/r/c/mediawiki/core/+/673795 "Add an ActionFactory and start converting to DI", T253078, @DannyS712
• https://gerrit.wikimedia.org/r/c/mediawiki/core/+/708537 "Add a hook 'GetActionName' to allow overriding the action", T282204, @matmarex

&action=new-topic is a legitimate URL parameter used by Flow's no-JavaScript interface.

It seems that on some pages, it works fine: https://www.mediawiki.org/w/index.php?title=Talk:Talk_pages_project&action=new-topic

But on others, it causes the error you describe: https://www.mediawiki.org/w/index.php?title=Talk:MediaWiki&action=new-topic

1 of these in 1.37.0-wmf.18.

I triggered a couple more while testing, please ignore those

At first I assumed this was because of ActionFactory, but in theory that should have been a no-op for extension actions, and its not happening on all pages, so I'm at a loss as to why that would cause this

Of interest, Talk:MediaWiki (https://www.mediawiki.org/w/index.php?title=Talk:MediaWiki) is a redirect to Project:Support desk, and there the action=new-topic works fine: https://www.mediawiki.org/wiki/Project:Support_desk?action=new-topic
My hunch is that its something with Flow's support of redirects, because https://www.mediawiki.org/wiki/Talk:MediaWiki displays the support desk content, but is not redirected in the url, so its a wikitext page (https://www.mediawiki.org/w/index.php?title=Talk:MediaWiki&action=info) where the new-topic action doesn't exist because thats just for flow pages

Can reproduce at the same page (https://www.mediawiki.org/wiki/Talk:MediaWiki) with some other flow actions that shouldn't work because its not a flow page:

https://www.mediawiki.org/wiki/Talk:MediaWiki?action=edit-header
https://www.mediawiki.org/wiki/Talk:MediaWiki?action=undo-edit-header
https://www.mediawiki.org/wiki/Talk:MediaWiki?action=edit-topic-summary
(and thats where I stopped testing)

Specifically, from https://gerrit.wikimedia.org/g/mediawiki/extensions/Flow/+/a308f15536c232c6585b263d0ffe7b695a374b01/FlowActions.php its the entries that have handler-class set, because those get added to $wgActions (https://gerrit.wikimedia.org/g/mediawiki/extensions/Flow/+/a308f15536c232c6585b263d0ffe7b695a374b01/includes/Hooks.php#91)

I think I figured out the cause, which is that the old Action::factory code had a check for returning false if given a string for a class that didn't exist, but the ActionFactory doesn't. Patch coming momentarily on gerrit with an inconspicuous message

Patch at https://gerrit.wikimedia.org/r/c/mediawiki/core/+/713525, tagged with T253078 only. Hope it was okay to do publicly, I figured the patch doesn't reveal which class was missing and thus causing issues, and it would be easier to review on gerrit.

So the root cause is that flow registers a bunch of actions by name, setting the value to true, all having no class at the correct location, and then for flow pages, use ContentHandler::getActionOverrides() to replace the true (which maps to using the class based on the action name) with the correct thing to use (an instance of FlowAction). See https://gerrit.wikimedia.org/g/mediawiki/extensions/Flow/+/a308f15536c232c6585b263d0ffe7b695a374b01/includes/Content/BoardContentHandler.php#131

I can work to clean that up now that we have DI for Actions, because it shouldn't be needed anymore. I'll wait until this security issue is resolved though, so that we can do the flow cleanup publicly (plan is to switch from handler-class to handler-spec, as well as to stop registering them in $wgActions because they only work for flow pages, so they should be controlled entirely via getActionOverrides(), which has the added benefit of allowing us to inject dependencies into core ViewAction without flow overwriting our spec with true)

Flagged as UBN until we can confirm that this does not break Flow in a critical way, and does not pose a security risk.

This error was caused by the introduction of ActionFactory.

Analysis:

• Flow puts all actions it defines into $wgActions, using the aciton name as the key, and true as the value.
• The handler class for flow actions does not have a name that matches the action name
• The handler class for flow actions cannot be instantiated via ObjectFactory, since it needs to have a Page passed in, besides other things.
• Flow actions are normally instantiated via ContentHandler::getActionOverrides() (which always constructs handlers for all actions).
• The contents of $wgActions ends up in ActionFactory::actionConfig.
• ActionFactory::getActionSpec will return true for all actions defined by Flow, because the action is found in ActionFactory::actionConfig. So the action is considered to "exist".
• When a Flow action is invoked on a page that has a Flow content model, ContentHandler::getActionOverrides() will return a handler, resulting in correct completion of the request.
• When a Flow action is invoked on a page that does not have a Flow content model, ContentHandler::getActionOverrides() will not return a handler for the action. So $spec remains true in ActionFactory::getAction. But true means "use the action name to build the class name", causing it to try an instantiate a class with a name like "Edit-headerAction", which fails hard.
• Interestingly, this problem did not exist before the introduction of ActionFactory: Action::factory was checking class_exists(), and returning false if the class did not exist.

Remedy:

• ActionFactory could check whether the class exists before attempting to instantiate it, and return false if it does not.

This task can probably be made public now (it seems I don't have the permissions to do it any more).

In T289190#7295143, @matmarex wrote:

This task can probably be made public now (it seems I don't have the permissions to do it any more).

Done. Not really sure why you can't edit the appropriate task policies. You're in acl*security, which I thought provided the permissions to do that. Might be a good question for @mmodell, @Reedy et al as to why that's not currently working for you.

@sbassett do we want to backport this?

In T289190#7296026, @DannyS712 wrote:

@sbassett do we want to backport this?

This isn't really a security issue, so that wouldn't be driving it. Though being that this was a nasty bug, I believe the best practice would be to backport to any relevant, supported release branches. I'd guess that would at least be 1.36. Not sure about 1.35 or 1.31 as that would depend upon when the issue was first introduced via ActionFactory.

In T289190#7296081, @sbassett wrote:

In T289190#7296026, @DannyS712 wrote:

@sbassett do we want to backport this?

This isn't really a security issue, so that wouldn't be driving it. Though being that this was a nasty bug, I believe the best practice would be to backport to any relevant, supported release branches. I'd guess that would at least be 1.36. Not sure about 1.35 or 1.31 as that would depend upon when the issue was first introduced via ActionFactory.

rMW20293e4c9918: Add an ActionFactory and start converting to DI was merged in 1.37 (July 28, 2021) - I was asking about backporting for the train, though I guess I should have addressed that question to @brennen

In T289190#7296115, @DannyS712 wrote:

rMW20293e4c9918: Add an ActionFactory and start converting to DI was merged in 1.37 (July 28, 2021) - I was asking about backporting for the train, though I guess I should have addressed that question to @brennen

Yes, Release-Engineering-Team are definitely more knowledgeable about that. If this can't wait until next week, then it probably needs a port to wmf.19 and deploy.

Still happening, wasn't the fix supposed to be in 1.37.0-wmf.19?

The patch was https://gerrit.wikimedia.org/r/c/mediawiki/core/+/713525, it wasn't backported, it's in 1.37.0-wmf.20.