In T311337#8735156, @Dreamy_Jazz wrote:

@sbassett it seems that the CVE description is wrong.

This information should not allow public viewing: it is supposed to be viewable only by users with checkuser access.

It should be suppress not checkuser.

Can this be updated?

In T305082#8731667, @thcipriani wrote:

My goals would be:

1. No free-for-all private repos
2. Allow users to request private repos through some process (a phab form? Dunno if there's any analogous process that exists that we could copy?)
   1. Ideally the form would require users to acknowledge that any data in a repo is only private-ish
3. Ensure there's a list of known private repositories that can be reviewed by administrators on some cadence

I chatted with @sbassett, he suggested I loop in Privacy Engineering for some discussion/assistance (👋 @JFishback_WMF ).

In T332850#8721037, @Bugreporter wrote:

If the issue is severe enough as T257062: Lilypond seemingly not subject to restrictions (CVE-2020-29007), it should be emergency disabled but reenabled once it is fixed (and there would be a task for reenable it).

Declining this as I think it is likely a relic of the past. We still have the the mediawiki-i18n-check-docker running for twn messages, phan-taint-check running under phan for most php repos (which can find problematic message usage like this), bugs like T200997 seemingly mostly resolved and still-active efforts like T2212.

In T328277#8699633, @Yaron_Koren wrote:

@thiemowmde - thanks for finding these, and sorry for the long delay in handling this. I just checked in this fix - I don't know if I got all the necessary escaping there, but hopefully I got most of it:

https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Cargo/+/899598

It might not be terrible to port Whispers' rule regular expressions over to client-side js, but that kind of defeats the point of leveraging an upstream project IMO. As far as JS-ish things that currently exist, this Node fork of Yelp's detect-secrets looks pretty unmaintained at this point. And this eslint plugin seems more entropy-focused than problematic, basic string patterns (e.g. password=).

In T301044#8716327, @Doc_James wrote:

By "experienced developer review our current nodejs service documentation and begin specifying and prototyping a new project" do you mean someone within the WMF technical team?

In T301044#8716117, @Doc_James wrote:

Okay so what would be the steps to "develop a microservice to deliver any relevant OWID content which could live within Wikimedia production"? Best

Sure, it's just that, as also mentioned within the security review (T324989), we can't iframe external content within Wikimedia production. And in this case, anything under wmcs or toolforge would be considered external content. So that would be a blocker to getting OWID into Wikimedia production. Hence the suggestion of developing a microservice to deliver any relevant OWID content which could live within Wikimedia production.

In T301044#8715648, @Tim-moody wrote:

Wikimedia production supports a number of node/js (some examples here) and golang microservices these days

I didn't mean to imply that it is a nodejs app. Once converted it is straight up html, but most of the work is done by runtime javascript on the front end that draws the graphs.

In T301044#8715389, @Tim-moody wrote:

This would be ideal, but keep in mind that the graphs rendered use javascript and svg, so they are not your typical php app,

So there are at least a few CLI tools - git secrets, whispers, gitleaks - that we (sec team) have experimented with for our manual reviews. It would be nice to maybe leverage one of these established tools with a hypothetical Phab plugin.

Confirmed 2fa enabled for @nfraison:

ext:StopForumSpam also echos back a user's IP address via the following error message (if they're disallowed access by the extension): https://github.com/wikimedia/mediawiki-extensions-StopForumSpam/blob/master/i18n/en.json#L13. It is unclear to me if this sort of thing will be disallowed under the new IP Masking policies.

@Aklapper - seems like there are no objections here, with at least one blessing from the Release-Engineering-Team?

The Security-Team are the ostensible drivers of this work, but we have no resources or plans to work on it, so I'll mark it declined for now.

Added an update to remove a noisy rule: https://github.com/wikimedia/eslint-config-wikimedia/pull/490#issuecomment-1468547357.

In T332003#8693081, @Xaosflux wrote:

Can a project determine what actions are being stopped by this? (Log?)

Thanks, @MarcoAurelio. Some initial notes on the proposed questions/metrics:

Ping @thcipriani @brennen et al to see if there are any objections to this request. Thanks.

Ok, I've revised the policy description (the warning, specifically): https://phabricator.wikimedia.org/project/manage/4570/. Let me know if there are any additional concerns.

Done. Logstash seems happy and I confirmed the verb switch in the log messages.

In T331928#8689603, @Dzahn wrote:

On T306708#8661705 you said your team can't handle 2fa reset requests while on this ticket you request admin access to handle 2fa requests. This is confusing me a bit.

In T331928#8689597, @Dzahn wrote:

@Aklapper Looks like this could solve T306708

@Aklapper - see T331928

In T331492#8689467, @Legoktm wrote:

Hmm, that's weird. We don't require that for other security ACLs. I think the idea of self-management for those (members performing these updates) was built into that architecture, but I guess that's another matter. I mean, there's nothing stopping members from managing those ACLs AFAIK. Anyhow, the Security-Team approves any verified, newly-elected stewards for Phabricator security issue access, assuming they have 2fa enabled.

Should the description be updated based on this? That as long as an admin confirms 2FA status, the group can be self-managed by stewards?

In T331492#8687004, @Aklapper wrote:

Any Phab admin could check (the Security team also has Phab admin members). :)

Note that on ShoutWiki this table is shared between all wikis; WMF may or may not want to do the same.

In T271108#8629416, @Wangombe wrote:

This ticket is currently awaiting security review. We shall resume after reviews are complete.

In T331492#8683061, @Urbanecm wrote:

So, in that case, the description of the project's actually correct, as Security-Team approval is needed each time the ACL membership changes (in bulk after elections, or individually if a need arises during the term), and there's actually nothing to do differently than I did here? Or am I missing something?

In T331492#8677457, @Urbanecm wrote:

Can you clarify what "verified" means in this context? Does that mean simply verifying that the Phabricator accounts actually belong to a duly elected steward? Or some other kind of verification that needs to happen?

In T331554#8678974, @MatthewVernon wrote:

@sbassett I've opened a CR to update your ssh key - if you can confirm it's correct and +1 the CR, I'll merge it.

In T331554#8678099, @Dzahn wrote:

Normally these are handled by the SRE on clinic duty but since it's late in Europe and to be on the safe side I just revoked the existing key and ran puppet on bast* hosts for right now.

In T331492#8677279, @Urbanecm wrote:

Hi, I need the Security team to add the newly-elected stewards to the ACL group (acl*security_steward description says Security-Team needs to approve each addition).

In T297839#8677172, @Bawolff wrote:

Just as a note, a lot of these cves for extensions seem to have the product field set to mediawiki, and the version numbers of affected versions set to mediawiki core versions. This is pretty confusing for the extensions not bundled with mediawiki core. I think its causing confusion in downstream packagers of mediawiki - e.g. https://bugzilla.redhat.com/show_bug.cgi?id=2161180 which doesn't make much sense

@dancy @kostajh et al - Can we resolve this for now? The issue was addressed but I guess technically the problem wasn't solved, IIUC? So not sure if we want to leave this open or stalled or something.

Hey @hashar et al - Any reason not to make this task public at this point? Thanks.

In T328163#8665299, @Catrope wrote:

That's fine with us. We just wanted to make sure we were doing everything by the book and that we're in the clear, security policy-wise.

In T149488#8675142, @Bawolff wrote:

I noticed that the CVE ( https://www.cvedetails.com/cve/CVE-2023-22911/ ) has the product marked as "mediawiki". Normally I would assume this would imply mediawiki-core or at least an extension distributed with MediaWiki. It also has language like "An issue was discovered in MediaWiki before 1.35.9, 1.36.x through 1.38.x before 1.38.5, and 1.39.x before 1.39.1" which is pretty misleading.

In T273220#8665225, @MarcoAurelio wrote:

I wonder if we should not add some sort of privacy warning for onOtherBlockLogLink in rESFS includes/Hooks.php for stopforumspam-is-blocked to indicate that the link leads you to stopforumspam, a external website which has a different Privacy Policy than ours.

Thanks, @sguebo_WMF for the privacy review. I'm going to block this review for now on the above issues being acknowledged and either accepted by the correct level of WMF staff per our current risk management framework or mitigated in some other fashion to reduce the assessed risk level.

Regardless of T309900#8462082 (and the currently open, related change set), I'm personally fine putting es.wikiversity in enforce mode, given its relatively lower traffic and the fact that it's fairly trivial to disable ext:SFS if it causes problems or proves mostly ineffective. Thoughts, @Reedy?

In T306708#8657402, @Aklapper wrote:

@sbassett: Thanks. Any updates? This is a bottleneck, e.g. I missed T330073 (and nobody else picked it up either)...

Tagging Privacy Engineering for an opinion/risk rating about the following. I'm not certain there's precedent for this on Wikimedia production or that wmcs would completely satisfy any privacy concerns for proposed, embedded content like this.

The vast majority of these seem to have been fixed over the years? A quick code search reveals a few rawParams(...)->text(...) calls. Sourcegraph finds some others with its structural search of our github mirrors. A few of these are likely true positives, maybe, though the rest have either been noted as exceptions via // @phan-suppress-next-line SecurityCheck-XSS comments or are in unit test files.

To be honest, I don't know if this and similar issues matter much unless a more formal strategy around Technical-Debt is implemented and code stewardship/maintenance is reassessed. Phabricator in aggregate is pretty noisy and, as others have noted, is without commonly-shared standards around its usage for things like task priority, which I'm not sure is even solvable. I do like @Bawolff's suggestion (in his wikitech-l response) - as an initial step - to separate various teams (wmf, community, both) from actual codebases or projects within Phabricator, in relation to what is being prioritized and worked upon. I believe that would help with various misunderstandings like "the Security-Team works on every task tagged with Security". This would be a significant change though and likely difficult to propagate.