In T275669#6862906, @Urbanecm wrote:

Merged them.

I kept these as separate patches for the backports so as to (hopefully) make reverting the first patch easier, if and when that's needed. These don't cleanly apply to REL1_35 and REL1_31, mainly due to directory/file name refactoring, but I can work on new patches for those, post them here and then push them up to gerrit for review/merge.

In T275704#6861757, @Urbanecm wrote:

I can add a query to detect whether there are any broken entries, to prevent going it through everything.

+1 to the updated patch above, I assume that'll go through gerrit once T275669 is public (which I plan to do today, along with the backports). Do we have any idea what other projects this might need to be run on besides loginwiki, testwiki and enwiki?

+1 to the patches above. I assume Linker::userLink( $row->cul_target_id, $row->cul_target_text ) in LogPager doesn't need a trim because of https://gerrit.wikimedia.org/g/mediawiki/core/+/21ab535b83b97866cb9b79dcede95e8b7c32858f/includes/Linker.php#914. I guess feel free to deploy these unless you want @Reedy or I to do so instead.

In T274107#6855644, @Majavah wrote:

Upstream does not treat this as a security issue, can this task be made public?

@Aklapper - a number of items from the Security-Team's perspective. For starters: there's no estimated deployment date which is a hard requirement for us to schedule reviews. It also doesn't satisfy a number of requirements for higher priority or status as described within the "What type of project or code triggers this review process?" and "How are these requests prioritized?" sections of the current security readiness review SOP. There are also questions above as to whether this extension would be the best approach for the current indicated problem. Given all of that, this review will have to be very low-priority for the Security-Team. If you'd like to change the task to "Open", I guess that's fine, though it won't really change how the Security-Team currently views this task.

Pull request created: https://github.com/MatmaRex/patchdemo/pull/241

The first patch (0001-SECURITY-Escape-the-wikitext-of-parse-warning-messag.patch) has been deployed to wmf.31. Logstash seems fine and the issue seems resolved testing the formerly problematic previews on enwiki Wikipedia:Sandbox. I'm going to track this issue on the next security release tracking task, just so we don't forget about it, in case it causes any issues later. @matmarex - it should be fine to push the second patch up to gerrit now, I'll make this task public shortly.

In T125289#6847728, @Aklapper wrote:

Seeking input from Security-Team hence adding tag

In T242821#6844336, @suffusion_of_yellow wrote:

Not saying anything is broken now, or even with this patch.

In T242821#6844239, @suffusion_of_yellow wrote:

As a reminder, Special:AbuseFilter/test still allows you to view private filters with a URL like https://en.wikipedia.org/wiki/Special:AbuseFilter/test/2.

In T257066#6843508, @Xover wrote:

So… we're currently waiting for a suitable volunteer to materialize out of thin air to address an issue whose details are not public for security reasons? And in the mean time we have many thousand broken pages across multiple projects and all we can do is bleed contributors in those areas?

I reviewed this task for completeness and PII. Making public now per request from @RhinosF1.

Some initial thoughts:

1. I think the initial standard is: don't. Given the outcomes of the VueJS task force and commitments of various teams at this point, Rollup is likely to be the low-risk, paved-road approach for any JS-related build steps, e.g. T272879 and also a part of SX's current risk mitigation plan at T260236#6825798.
2. I'm not saying this is a perfect approach, but requiring human-readable (kinda) webpack artifacts with any relevant gerrit cs with the steps I performed and outlined here should be mostly feasible for now. A couple of questions remain as to 1) how long we plan to support this kind of stuff until we... mandate? migration to Rollup and 2) how similar a manual review process Rollup artifacts might require.

Hey all - thanks for submitting this review request. As discussed a bit with @Jdforrester-WMF, the security readiness review of the WikiLambda extension will be my primary focus/deliverable for Q3 for the Abstract Wikipedia project. The code currently seems to be in a reasonable state of completion for such a review, though as a lot of code for this project is likely to be quite volatile, I imagine this and the related services might undergo a few different reviews depending upon various deltas. Of course I'd like to keep those to a minimum as much as possible. For the forthcoming node services (orchestrator, executor), I'd imagine those to be ready for review sometime in Q4. Since they are based upon the existing (and what we believe to be reasonably-mature) service-template-node code, I'll likely be most concerned with the various measures to best protect against potential vulnerabilities specifically related to the execution of user-submitted code - though it is important to note that any system which allows for such a feature will always be inherently vulnerable, at least from a conceptual standpoint.

Added to Q4 planning column for Q4 review.

Added to Q4 planning column for Q4 review.

In T274883#6835461, @matmarex wrote:

The bug was introduced in https://gerrit.wikimedia.org/r/c/mediawiki/core/+/597262, which was not in a MediaWiki release yet, so we can probably just deploy the first patch to Wikimedia wikis, and then make this task public again and submit the patches to Gerrit?

@Arrbee et al - Great! I'll link the mitigation plan document within the relevant risk registry entry. Hopefully said registry will become a bit cleaner and feature better automation once it is migrated to our new GRC platform (Archer). But for now this is still a very manual process for the Security-Team. The mitigation plan the Language Team has put together looks good (thanks!) though I did want to provide feedback for a few items:

1. It would be great if @Reedy and myself could be added to any gerrit change sets which include unminified webpack artifacts and any other relevant JS code as was recently done for WVUI, pre-merge/pre-deployment.
2. For any vuln-checking/SAST/etc that may occur in CI/CD for SX and its dependencies, it would be great if those results could be 1) protected somewhere and/or 2) relevant change sets were not merged until any resultant risk was either mitigated (by bumping dependencies to known secure versions) or by formal risk acceptance. I understand the first suggestion is, at best, extremely inconvenient given gerrit and its current configuration.
3. I would imagine that SX could leverage T272879, once completed, though I'm not sure what the timeline estimate is for that task.

For a bit more clarification on the comment above and per our security readiness review SOP, the Security-Team would be happy to re-triage this if a few more details can be provided regarding:

1. A more specific target deployment date.
2. An intended production support plan, including any potential Foundation team sponsorship.
3. A working test environment, be that in Mediawiki-Docker, a standalone docker, a cloud installation, patchdemo, perhaps even a beta deployment, etc. While we can in theory just manually install the extension against a local copy of mediawiki, it helps us quite a bit to have an existing development environment with potentially real data to test against.

Thanks.

In T272770#6814046, @Majavah wrote:

This isn't an issue on any release branch.

...

In T272770#6814891, @Ladsgroup wrote:

Yeah the issue has not been released to 3rd parties at all.

All - I believe there are some fairly significant misunderstanding around items related to:

1. How the Security-Team performs various security reviews
2. What is included within Security-Team review deliverables
3. What the Security-Team will and will not review based upon current resources and priorities
4. The concepts of risk assessment and risk ownership as they relate to various Security-Team reviews
5. How to best communicate with the Security-Team through our official, documented processes

and likely a few others. We're going to have a few team discussions on some of the comments here with the hope of providing some clarifications to reset certain incorrect expectations and assumptions.

In T272770#6810803, @Ladsgroup wrote:

This is deployed now.

Could there be a cache-related security issue here?

In T6845#6811866, @MJL wrote:

I'm talking about the line included in challenges:

• "Audio captchas present a language barrier problem (next to the language script barrier of normal captchas)"

It seems to imply audio captchas are an option but have been dismissed for this reason. If that's the case, could we not just implement them as a stop-gap measure?

Resolving for now. The current fix (tx @Reedy) will likely need some adjustment if/when this repo migrates to Gitlab.

Security Review Summary - T257579 - 2020-02-05
Last commit reviewed: 7fd111ad5ed3165683a76f68c3d3504f24cc179f

In T272130#6802796, @Addshore wrote:

So, this will be deployed via a build in jenkins (ideally), so that it uses the same process and the query gui.
This is just about to be created by the campsite as a push button trigger in https://phabricator.wikimedia.org/T210286
I guess it's only for a similar job to exist fetching code from github to create the build that would then be deployed?

Another alternative would be github actions to make the build and push a change to gerrit?
I don't see a big difference between the two as either way the build is triggered by a human, and the change is still 2ed by a human.
The one difference would be that npm install is running in a different place for each.

@SBisson - Thanks for the update. We're pretty much booked this quarter for reviews (with our new SOP, we're trying to be realistic given current resources and limit the total volume of security reviews to 6 per quarter) so perhaps this could be scheduled for next quarter (cc @Jcross).

Update: I'm still hopeful to have this review completed by the end of this week (2021-02-05). @Volker_E - thanks for the version bumps, I'll pull those down for the review if they haven't been merged yet. Also - with the webpack build step still being in place, I'm going to rate the overall risk of wvui in its current state as at least medium, which will require managerial/directory acceptance of the risk, per our risk managment policy, as publicly summarized here: T249039#6309061.

In T101017#6795359, @Aklapper wrote:

Just for clarification, that means Oct-Dec 2021?

Note: I committed the deletion of the two wmf.28 Wikibase patches under /srv/patches on the deployment server (5578144525) since wmf.28 was rolled back and as noted by gerritbot above, https://gerrit.wikimedia.org/r/658323 and https://gerrit.wikimedia.org/r/658324 were merged.

In T260236#6790822, @ngkountas wrote:

I believe that it is still unclear what it the future of our UI lib. I believe that it will eventually be merged into Wikimedia Vue UI. After all the reason that we built our own UI library was the fact that we were moving too fast relatively to Wikimedia Vue UI lib and we would be blocked if not creating our own.

Update: unfortunately, it looks like the pilot program mentioned above likely will not happen until Q4 2021.

Placing into back orders for now. The Security-Team needs to discuss how useful the continued maintenance of peek is for our project management function.

@Ladsgroup @Michael

In T271729#6790352, @Aklapper wrote:

@sbassett: Hmm, why were Edit Policy and Join Policy of SecTeam-Processed and SecTeam-wikimedia-project-event changed to acl*security ? Which problem does that solve, apart from blocking interested people to join/follow that project?