(For completeness, the above error was at https://www.mediawiki.org/w/index.php?api=lift-wing&title=Special%3ARestSandbox , so the problem was that we were hitting the API from www.mediawiki.org but the ACAO header was for test.wikipedia.org)

In a private message to Cormac I explained that we're about to announce a new, more restrictive policy for security task access. Based on the new policy, he agreed that he would not need proactive access to all security tasks, but could be CCed on tasks as they are assigned to him, and said I could decline this request.

In T427023#11956681, @Jdlrobson-WMF wrote:

In T427023#11956350, @Catrope wrote:

I agree with @mszwarc's comments: I think it makes more sense to group all sitewide scripts together (Common.js + gadgets) and all sitewide styles together.

I agree it should be possible to disable JS gadgets but not CSS gadgets. Instead of grouping together it might be a good idea to separating these out as well for consistency (either as part of this phase or a later phase).
e.g.

• Gadget CSS
• Gadget JS
• Site CSS
• Site JS

OK, as long as it's consistent with what we're already doing for other types of content, I'm fine with it.

Thanks for pointing me in the right direction. The issue was that we were requesting conditional mediation (for passwordless login) on the second page of the login process, in parallel with the main (non-conditional) WebAuthn request. We had a check to prevent this code from running when the username field isn't present, but I accidentally deleted that check recently.

Right now its function is to prevent WebAuthn creds from being registered when you're on an SUL wiki but not on the central domain (auth.wm.o). This is done through a confusing layered config: InitialiseSettings sets it to false on non-SUL wikis and true on SUL wikis, but then CommonSettings sets it back to false when on the central domain. However, we now have code always redirecting Special:AccountSecurity to the central domain, so we probably don't need this anymore.

A more user-friendly view of $wgRestrictedGroups is available at https://meta.wikimedia.org/wiki/Special:ListGroupRights#restricted_groups (note this is only for local groups, so it differs from wiki to wiki)

What does fr_deleted=1 mean? It's not super clear to me from reading the code, but it seems to me that it might mean that the file's entire history is deleted. If that's the case, should we still exclude rows with fr_deleted=1 entirely?

I agree with @mszwarc's comments: I think it makes more sense to group all sitewide scripts together (Common.js + gadgets) and all sitewide styles together.

https://gerrit.wikimedia.org/r/c/mediawiki/extensions/WikimediaCustomizations/+/1293764

Whoops, you're right, I forgot that that existed. And it looks like editing these does indeed require the editsitejs right. I'll write a patch for this today then.

That's right, reauthentication is currently only required for editing .js pages in the MW namespace. We'll consider whether to expand reauth to the entire MW namespace in the near future. I don't think it would be feasible to require it only for raw HTML messages, because MW doesn't really track those as such.

In T427023#11949515, @EMill-WMF wrote:

Changing the Safe mode preference should require 2FA when enabled.

@Catrope Would this better said as, "If a user has 2FA enabled, then disabling the Safe Mode checkbox should be considered a privilege-elevation-worthy check"? A couple things in that are 1) not sure why we'd need to check anything for enabling Safe Mode

@mszwarc has already done this: https://gitlab.wikimedia.org/msz2001/2fa_less

Pushing a new change did not produce this error (trace ID 778187236011-bbfaed23). But pushing https://gerrit.wikimedia.org/r/c/mediawiki/core/+/471664/29 and https://gerrit.wikimedia.org/r/c/mediawiki/core/+/471664/30 did (trace IDs 1778190246679-3985a3f5 and 1778190340140-35c006fa).

This will be somewhat improved next week with this patch, which includes a link to the reauthentication form in the error message. That's still not exactly what we want, and we'll keep working on more improvements, but it should be slightly better and slightly less confusing than the current situation,

Since the key is always required, could we simplify $cache->newCacheCall()->key( $key )->... to $cache->newCacheCall( $key )->... ? Or were you thinking you would handle the local/global key distinction by having the caller call either ->key() or ->globalKey()?

I don't think these vulnerabilities affect us directly, but it doesn't hurt to upgrade. I've submitted a public patch to upgrade Handlebars at https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Flow/+/1277782 , without reference to security vulnerabilities or to this task. Once that has ridden the train I think we can make this task public.

Thank you, this was really helpful! It allowed me to track down the bug and upload a fix. I'll get this reviewed soon and do a follow-up release (2.5.1).

Furthermore, I verified that the correct CSP headers appear at https://en.wikipedia.beta.wmcloud.org/w/?oldid=2 (I remember a bug where the VCL logic doesn't apply to URLs like that), but the old ones appear at https://en.wikipedia.beta.wmcloud.org/w/index.php?oldid=2

Relatedly, we plan to do T206012, which in my view means that CentralAuthTokenSessionProvider should never allow editing site JS.

In T197137#11822948, @Od1n wrote:

I still prefer the "disable execution of JS from wiki pages" approach (though would it be sufficient?), but if "re-auth" is kept, we could reduce its friction:

• Ask for re-auth only when publishing, not when opening the edit page.

This is a good idea, and probably the best solution to T423193. This would be a bit complicated because we have to preserve/stash the submitted edit while going through the reauth process, but we will have to do that regardless (to handle the case where your reauth times out after you click edit but before you click submit), and we're already planning to do it soon. I think we should be able to do this in the next month or two.

There are probably other bugs like this, all caused by the fact that the current mechanism for requiring reauth to edit sitewide JS pages is hacky. Integrating reauth into the permissions system (T197136) should fix this category of bugs, and we plan to work on this in the next few months.