@Arrbee - that's good to know, thanks. Again, I think this code is looking pretty good overall with @santhosh's recent patch set. I was merely planning to perform some additional (kind of optional) analysis and pen-testing of the code for good measure. It shouldn't really be anything that prevents a full production deployment.

Update: I did have a more in-depth look at the JavaScript under modules/ yesterday - again, I didn't find anything particularly concerning. I still wanted to pen-test it a bit further (was having some vagrant issues yesterday) so if I find anything from that, I'll post it here. Additionally, I see that the extension was deployed to simple.wikipedia.org and id.wikipedia.org earlier today, per T213076.

@Eevans, @Clarakosi - just booked a quick hangout for this Friday (2/15) to discuss potential security concerns for this service.

Security-Team approved at weekly this morning. Tagging @JBennett and @chasemp in case there are any further questions.

These changes look good and phan-taint-check likes them now. I still want to review the aforementioned JS a bit more in depth, but I personally don't think that should stop your deployment timeline of 2/12.

Some additional follow-up:

lodash <= 4.17.5

EventGate uses ^4.17.11

@santhosh - I think we'd like to (or at least I'd like to) review this code a bit more. We didn't have our weekly security review scrum during All-hands or the week after, so this is still kind of in limbo at the moment though we wanted to get you some initial feedback. We should be able to schedule a full review by next Tuesday (2/12) when we have our next scrum. Is there a more specific launch date other than "February 2019" you had in mind that would make this more pressing?

*Update:* Sorry for the crazy delays on all of this. I'm officially starting in on this today (2/8).

Security Review Summary - February 2019
Overall, this looks pretty good to me. Issues detailed below:

This is still technically unassigned, but I had a cursory look at it. Some initial findings:

1. package.lock looks fine from a quick run of security-checker.
2. I assume any potential privacy (and related) issues w/ the Google MT service are already addressed by WMF's agreement with Google for cxserver.
3. In SiteMapper::getPageURL, it looks like $title isn't being validated/sanitized and is just replacing $2 within //$1.wikipedia.org/wiki/$2. Given that Html::rawElement is used within specials/SpecialExternalGuidance.php and it's populating $sourcePage from the request via getVal(), I'm thinking there could be a potential for injection here, unless I'm just missing where this is being sent to the parser or sanitized, etc.
4. I'm not seeing anything egregious within the JS modules under modules/, but I (or whomever officially gets assigned this review) should definitely spend more time reviewing those.

Update: review will be posted here by Friday (2/8/2019) at the latest.

Thanks, @Tgr. Unless @Reedy has strong objections, we'll go with the bjeavons repo and get this scheduled for a security review.

But you served as the first reviewer, what am I getting wrong?

Pinging @Reedy and @Tgr per @charlotteportero comment above.

@Ottomata et al - just fyi, this is officially in-progress and I hope to have a review completed either just before or just after all-hands.

I'd imagine you'd probably need deployment and analytics-privatedata-users access for now - this is what @Bawolff and I have. Here's the patch for when I was added to these groups.

Some follow-up here - apologies for the stop/go on this one:

1. Did the mirroring issue with gerrit ever get addressed? It still looks to be an empty repo.
2. I was curious if the tool is actually working in production. On wikidata.org, I added the gadget and selected "All Sources" within the config and the interstitial just kind of hangs indefinitely for me. (Chrome 71.0.3578.98 on Mac Mojave.) Is this expected for now?
3. Is the current version (gadget/backend at pst.wmflabs.org) the indefinite production version for now? Looking at wikidata.org/wiki/Special:Version, I'm not seeing a deployed extension, so I assume that might be part of a forthcoming development cycle?

@Niharika - I believe I tagged Community-Tech as it was thought that they might be a viable champion/owner of this project, when the Security-Team recently discussed this task. If that's not the case, then we can remove the tag.

I think it should be a larger effort between RelEng and the SRE/Service Operations team.

The Security-Team should be able to get a review scheduled for this soon. Just a few initial questions/observations, in addition to some of the concerns already being brought up within other comments:

1. We'd want to have a look at the eventgate code as well as any additional dependencies within package.json as opposed to just Ajv, at bare minimum scanning for existing vulnerabilities and any obviously-dangerous functionality.
2. Given some of the recent unpleasantness with npm, has there been any plan for Analytics to host their own npm registry, only allowing vetted modules to be installed for various wikimedia Node applications? From various conversations I've had and doc I've read, it seems that having local repositories (e.g. apt) of vetted packages/modules are simultaneously 1) expected 2) not enforced in any way for things like npm, pip, etc. I'd imagine this is something the Security-Team would want to begin calling out within our reviews, as again, this seems a fairly lax policy within the context of wikimedia application development.

Here's some current documentation for our security review process and here's the Phab request form. We're currently revamping the process a bit and updating the documentation with the goal of socializing it a bit better in the future.

Right, I just meant more for sanity's sake and in case discussion from any of those tickets accidentally wandered over here :)

So there's a tag where a lot of password/credential-related tasks are tracked: https://phabricator.wikimedia.org/project/board/148/. But similar to Security, it's fairly noisy. Some recent password/credential-related tasks have been public (e.g. the proposed haveibeenpwned service T189641), though many others are security-protected for obvious reasons. If we'd like to track those here, we may want to consider making this a security-protected task as well, at least for the time being. There's also a fairly enormous body of password/credential-related tasks in various states of decay from the past decade or so. Some of these do seem to have recent, relevant discussions on them, but many are probably too stale for what we would want to track here.

There's a private ticket (T212679) where similar issues on a couple of other wikis were being addressed over the holiday break. It looks like this issue subsided a bit after 12/19/2018, according to Special:RecentChanges.

Looks like this is for travis CI's integration w/ slack:

@Ladsgroup - I think the Security-Team would be fine with that, thanks.

All (@santhosh, @Nikerabbit, @KartikMistry, etc) - I'll plan to resolve this ticket today or tomorrow unless there any additional concerns.

@Nikerabbit - This all sounds reasonable. From your analysis (thanks!) it appears the two sinks we eventually arrive at (from CXServer's perspective) are ve.init.target.parseDocument and ve.createDocumentFromHtml, both of which are trusted and do not use jQuery in any way. Furthermore, VE seems unconcerned with the flag within its own sanitization module: https://gerrit.wikimedia.org/r/plugins/gitiles/VisualEditor/VisualEditor/+/master/src/ve.sanitize.js#23

@santhosh - abandoning r477459 should be fine, but I would note that sending any DOMPurified content to jQuery's dom-writing functions (html(), etc) should be thoroughly vetted so as to ensure any potentially-dangerous payloads are being sanitized (via some method similar to DOMPurify's SAFE_FOR_JQUERY feature) as expected.

SWAT today sounds fine to me.

@Ladsgroup - this looks fine to me. Were you pinging @Bawolff and @Reedy to deploy this during the security window (2200 UTC Mondays) or did you want to deploy? I'd guess either would most likely be fine.

@Krenair - yes, looks to be a dupe. Can we merge this one or resolve it and have @Sasheto claim T204506?

Ok, thanks for the update, @Hjfocs.

Not seeing anything in master or REL1_32 for this. Is it somewhere else? If not, is there an estimate for completion?

@KartikMistry - Ok, that sounds good. I just wanted to ensure apertium.wmflabs.org was only used for testing purposes and not in any production capacity. Thanks.

I'm going to resolve this ticket now. The last item was just to add the following to your /etc/hosts if wikitech,w.o/DNS ever goes down:

192.237.162.200 wikitech-static.wikimedia.org
2001:4801:7821:77:be76:4eff:fe10:2ed5 wikitech-static.wikimedia.org

If you really feel like doing that, go for it, but that's fairly optional IMO.

@santhosh et al -

I'm not sure how worthwhile this would be if we ever got a decent password strength meter deployed. Though digging through the history, that might be a big if.

If we're looking to abandon https://gerrit.wikimedia.org/r/475798/, my vote would be to instead go with https://gerrit.wikimedia.org/r/478395/, as it does a good job of balancing both security and usability IMO.

Thanks for all of the quick follow-up on this. https://gerrit.wikimedia.org/r/477972 looks good as additional hardening and the new test within test/mt/Yandex.test.js runs well. I think this is looking pretty good, and would like to leave the SAFE_FOR_JQUERY flag implementation up to you and the Language Team to discuss further, as I believe I've probably provided as much relevant feedback within https://gerrit.wikimedia.org/r/477459/ as I can. Also, thanks for the information regarding the specific implementation of the Youdao service. I'm going to review that a bit more, but for now I'm not seeing any issues there, as it seems to be a more restrictive (fewer html tags/markup) version of the 1) reduce html 2) send to MT service 3) expand translated html process.

Interesting - thanks for the context and history, @Anomie.

Trust-and-Safety might have some additional thoughts here, as they currently manage the operational work around OATHAuth. Though the tasks @Tgr mentioned (T166622#4802577) should alleviate most of their concerns, I'd imagine.

A bit out of scope for this task, but have we ever considered creating alternative captchas (math, image classifying, etc?)

A summary of where I think we're at right now:

This need to fix via: https://github.com/gwicke/kad/pull/1
This is coming from service-runner and affecting all services. We have asked services team to update the dependencies

@Bawolff and I are deploying this now.

@santhosh et al-

@Daimona, @Huji - I checked that T210329.patch applies locally to the abusefilter master branch. Looks good. Not sure if @Bawolff or @Reedy are around right now, but we do have the security deployment window today at 22:00 UTC, so just a shade under two hours away (https://wikitech.wikimedia.org/wiki/Deployments#Monday,_December_03). I've got deployment rights and have done config deployments before, so I could probably do this, but:

1. I don't have CU anywhere, so the best I'd be able to test is locally. The patch doesn't look volatile, but if anything looked amiss in the logs, I'd have to revert immediately.
2. I've never done a full security patch and deploy before, by myself.

Hey everybody-

Hey Jon-

Thanks, @Aklapper.

@Jdforrester-WMF The Security-Team discussed that item today as well, and perhaps filing it as a separate task related to this issue. Given some of the feedback above, it might be wiser to pursue that approach as opposed to this one.

@Catrope - sounds good, thanks.

Well you are proposing removal of functionality that's only displaying public data. This does need to be balanced against the value of that functionality.

Is that a bad thing? Increasing obscurity and potentially deterring certain behaviors while reassuring legitimate users seems like a good thing.