Page MenuHomePhabricator

Security Readiness Review For the MediaSearch extension
Open, LowPublic

Description

Project Information

Description of the tool/project:

MediaSearch is a new interface for searching media on Wikimedia Commons with a Vue JS UI.

Description of how the tool will be used at WMF:

This code has been reviewed and merged into the WikibaseMediaInfo (WBMI) extension (see the commit message for the patch linked above for a list of the changes, e.g. swapping out CSS class name prefixes). The Structured Data team has chosen to pull this code into a separate extension for various reasons (increased maintainability and extensibility, lack of dependence on WBMI, clean slate so we can add a modern JS testing framework).

Dependencies

Wikibase, Elastica, CirrusSearch, WikibaseCirrusSearch

Has this project been reviewed before?

The WBMI extension was reviewed years ago, but this specific code has not been reviewed by the security team.

Working test environment

This code as it stands in WBMI already exists on production and beta Commons, but we do not have a working test environment for the new extension.

Post-deployment

Structured Data team: manager Mark Holmquist, tech lead Cormac Parle

Event Timeline

AnneT created this task.Oct 26 2020, 9:57 PM
Restricted Application added a project: secscrum. · View Herald TranscriptOct 26 2020, 9:57 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
sbassett triaged this task as Low priority.Oct 28 2020, 4:14 PM
sbassett moved this task from Incoming to Back Orders on the secscrum board.

Hi @sbassett, we're hoping to get some information on how thorough a review this new extension will need given that it's comprised of existing production code from WBMI. This will help us set a timeline for deployment. Could you let me know your thoughts on this?

sbassett added a comment.EditedOct 29 2020, 5:08 PM

Hey @AnneT -

If the code comprising the new extension is verbatim what existed within the WikibaseMediaInfo extension then I don't believe we'd need to complete much of a security review and would likely rate this as a low risk. WikibaseMediaInfo appears to have been around for about 5 years now, seems stable in production and has even been security-reviewed a couple of times (T159709, T200279). So again, if the code is mostly identical to how it existed in late 2018, then we don't need to perform a security review. If significant new code has been added or there have been major refactors of existing code since then, we should perform a security review. If this is indeed the case, it'd also be helpful to call out specific gerrit change sets/git commits where these additions or changes have been made.

AnneT added a comment.Oct 29 2020, 8:14 PM

Thanks @sbassett. All of the code in this new extension was added to WBMI post-2018, so it seems a security review is in order and it would make sense for that to cover all of the code in the MediaSearch extension.

Follow-up question: Since this code is already in production as part of WBMI, does the security review for MediaSearch need to block deployment of MediaSearch since there will be no additional risk?

Thanks @sbassett. All of the code in this new extension was added to WBMI post-2018, so it seems a security review is in order and it would make sense for that to cover all of the code in the MediaSearch extension.

Ok, we can get this scheduled during our weekly appsec triage meeting. Is there a desired deployment date or time period at the moment? We can't always guarantee that we an accommodate such dates, but it does help us with scheduling.

Follow-up question: Since this code is already in production as part of WBMI, does the security review for MediaSearch need to block deployment of MediaSearch since there will be no additional risk?

Well, the risk would be indeterminate for now, since we haven't performed a recent security review. Typically reviews with critical or high risks would need to be owned by a c-level+ with a mitigation plan and an entry within our risk registry. Please see T249039#6309061 for more information and let me know if you have any additional questions.

AnneT added a comment.Nov 4 2020, 7:23 PM

@sbassett My apologies, I was speaking of risk more generally—that this code already exists on production, so moving it to a new extension doesn't alter the existing level of risk the code presents—not specific to the security review ranking framework.

As far as timeline, I'd like to draw a distinction between our ideal timeline for deploying the new extension and formally releasing the MediaSearch feature. Though the code is already live on Commons as part of WBMI, the MediaSearch feature has not been "released"—it is not yet the default search on Commons, nor is it widely advertised or linked to. We would like to roll it out on a more widespread basis sometime next quarter after adding more features and further refining the search backend.

That said, we would prefer to deploy the new extension as soon as possible because of the benefits it would pose to our workflow (the most pressing of which is the ability to add a modern JavaScript testing framework for the Vue front-end, which is prohibitive in the current WBMI setup), and because we don't believe it adds risk, although we defer to your team on that matter, of course. Could we move forward with deployment, but not release of the feature, in this circumstance? We don't want to presume to be exempt from the usual review process, and if we need to go through the normal process, we are committed to doing so. We just want to point out the distinctions here in case it's possible for us to deploy the new extension before the full review process is completed.

@AnneT -

So the official guidelines for these sorts of things (which, to be fair, were actually just reworked a bit by me) don't list any review as being a hard blocker for deployment to the beta cluster. Production would be a bit different. According to your comment in T266513#6589831 - "All of the code in this new extension was added to WBMI post-2018" - we would consider this to be unreviewed code, even if it has existed in production within a different code base for a while. So the risk in our mind would still be undetermined, and if we were going to assign a default risk without first conducting a review, it would likely be high for now, given all of the potential unknowns.

CBogen added a subscriber: CBogen.Nov 5 2020, 10:32 PM
Jcross added a subscriber: Jcross.EditedTue, Jan 5, 8:14 PM

Hi @AnneT - we're in the process of finalizing our queue for Q3 and we're wondering if you can let us know how close this is to production ready/what your timeline is? Thank you!

AnneT added a comment.EditedThu, Jan 7, 1:26 AM

Hi @Jcross (and @sbassett—thanks for your helpful comments on this process earlier, and I'm sorry for my lack of response!), we have been deliberating how to move forward on this as a team. I'm working on getting you a more concrete timeline on when we plan to formally release this feature, but I can tell you that the extension itself will be ready for review within a few weeks. I'll get back to you with more info as soon as possible (hopefully tomorrow Monday next week).

Hi @Jcross, here's an outline of our timeline:

  • The new MediaSearch extension will be ready for security review by the end of January, possibly sooner
  • The MediaSearch feature, which is currently live on Commons (with the code currently in the WikibaseMediaInfo extension), will be rolled out more publicly (e.g. announced, publicly linked to) over the course of this quarter
  • The MediaSearch feature will become the default search on Commons around the end of the quarter, possibly later pending community discussions

I've been chatting with @CBogen, our program manager, and we were hoping to have a brief chat with you about this timeline to make sure we're on the same page. If that's alright with you, Carly will get something on the calendar. Thanks!

Jcross added a subscriber: Reedy.EditedThu, Jan 21, 9:51 PM

Hi @AnneT -as we mentioned in our call, we're revamping our scheduling process a bit to provide more clarity and transparency around our workload management and prioritization process. You'll see that we've placed you into queue for this quarter: https://phabricator.wikimedia.org/tag/secscrum/ and @sbassett or @Reedy will be in touch with any questions or concerns. Thanks for your patience, and please feel free to reach out at any time!

AnneT added a comment.Fri, Jan 22, 7:27 PM

@Jcross this is great news, thank you!

Jcross assigned this task to sbassett.Mon, Jan 25, 7:23 PM