Page MenuHomePhabricator

Security concept review for WikiUnit extension
Closed, ResolvedPublic

Description

Project Information

  • Name of project: Extension:WikiUnit (working title, may change)
  • Project home page: none yet
  • Name of team which owns the project: none yet
  • Primary contact for the project: @Evad37
  • Target date for deployment: none yet
  • Link to code repository: none yet (I want to check if the concept is workable, or how it could be made workable, before starting anything)
  • Is this a brand-new project: yes
  • Has this project ever been reviewed before: (Phab tasks, etc.) no
  • Has any risk assessment (STRIDE, etc.) been performed: no
  • Is there an existing RFC or has this been presented to the community: enwiki discussion, T240647, T39230 (more generally, not for this specific concept: T234661)
  • Is this project tied to a team quarterly goal: no
  • Does this project require its own privacy policy: no

Description of the project and how it will be used

Per T240647 (more details there):

An extension that provides unit testing capability for gadgets/scripts, using QUnit. Unit tests are defined on a script's /testcases.js subpage.
Tests are run when editing, after previewing or showing changes (action=submit) -- or perhaps a new button "Run unit tests".
To run tests, load QUnit js and css, concatenate the editor textbox content and the content of the testcases subpages (so they are within the same outer scope), and execute the resulting text as javascript.

There may need to restrictions on where this can operate for security:

  • MediaWiki namespace should be okay - editing is restricted to interface admins
  • contentmodel:javascript pages in your userspace should be okay - editing is restricted to yourself and interface admins
  • Elsewhere could be problematic. The safest way would be to just not operate on these pages... but perhaps just having a confirmation box with a "scary" message (similar to enwiki's MediaWiki:Jswarning) would be good enough, given that it's not really any worse than using importScript on your common.js
  • Perhaps there should also be a note or warning that code entered into the textbox will be executed to run unit tests

Additionally, per T39230, there should be a Special page to run tests for the current saved version of a script (without needing to edit it).

Description of any sensitive data to be collected or exposed

None

Technologies employed

JavaScript and PHP

Dependencies and vendor code

QUnit, and probably mediawiki.api, mediawiki.util, and oojs

Working test environment

Proof-of-concept: WikiUnit userscript (source)

Event Timeline

Evad37 created this task.Dec 14 2019, 4:41 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptDec 14 2019, 4:41 AM

Thanks! :) To clarify, the request here is to perform a security review on the ~200 lines in https://en.wikipedia.org/wiki/User:Evad37/WikiUnit.js ?

Evad37 added a comment.EditedDec 14 2019, 4:45 PM

Yes, a security concept review (per mw:Security/SOP/Security Readiness Reviews) for that code -- in the context of it, or substantially similar code, being made into an extension.

sbassett triaged this task as Medium priority.EditedDec 16 2019, 10:11 PM
sbassett added a subscriber: sbassett.

@Evad37 - Thanks for submitting this request. I assume this at least partially ties into the concerns from T71445 and related tasks? It's an interesting idea - to run qunit tests on-wiki, though I think the Security-Team might have a few concerns here, most of which would center around introducing qunit into the Wikimedia production environment and (presumably) running it through a web interface (Vuln-DoS, etc.) I'm wondering if such a solution could be partially leveraged by gerrit, which is obviously designed to run tests like this, though I understand a clean pipeline between on-wiki JS and gerrit would be non-trivial to solve.

Anyhow, did you have a deployment date in mind and is there a sponsoring WMF group or person for this? If not, we'll probably need to de-prioritize this request a bit within Security Readiness Reviews.

Reedy added a subscriber: Reedy.EditedDec 17 2019, 5:20 PM

I note that Special:JavaScriptTest exists (not on Production wikis)... This seems very related/similar...

Which is hidden behind $wgEnableJavaScriptTest being true

@sbassett It is related to T71445 and the surrounding issues, but mainly just based on my own experiences of trying to do unit testing for my scripts in a somewhat sane way. I don't have any particular deployment date in mind - maybe sometime next year, I'm not really sure how long it would to take to write an extension, get it reviewed, and resolve whatever concerns arise. There isn't a sponsoring WMF group or person (though I'd be more than happy to collaborate if anyone wants to).

@Reedy I guess it is a similar concept, at least regarding the special page aspect, though it is part of/for core.
( Convenience links for others:
https://phabricator.wikimedia.org/source/mediawiki/browse/master/includes/specials/SpecialJavaScriptTest.php
https://phabricator.wikimedia.org/source/mediawiki/browse/master/tests/qunit/QUnitTestResources.php
https://phabricator.wikimedia.org/source/mediawiki/browse/master/tests/qunit/data/testrunner.js )

chasemp renamed this task from Security Concept Review For WikiUnit to Security Readiness Review For WikiUnit.Jan 7 2020, 6:34 PM
Evad37 added a subscriber: chasemp.Jan 10 2020, 9:07 AM

@chasemp I'm not sure why you changed this task from a concept review to a readiness review. Should I go ahead and start working on this, or should I wait for further discussion/review of this concept, or is there something else I should be doing?

@chasemp I'm not sure why you changed this task from a concept review to a readiness review. Should I go ahead and start working on this, or should I wait for further discussion/review of this concept, or is there something else I should be doing?

Apologies if I've misconstrued what this is. I imagine this is more appropriately a concept review than a readiness review even though code is available.

Thanks! :) To clarify, the request here is to perform a security review on the ~200 lines in https://en.wikipedia.org/wiki/User:Evad37/WikiUnit.js ?

This was the catalyst for me switching. Let's switch back :) I'll talk to the team during our weekly meeting next tue as well.

Restricted Application added a project: Security-Team. · View Herald TranscriptJan 10 2020, 4:10 PM
chasemp renamed this task from Security Readiness Review For WikiUnit to Security Concept Review For WikiUnit.Jan 13 2020, 4:24 PM
chasemp moved this task from Incoming to In Progress on the Security-Team board.

To be discussed in our Security Readiness Reviews meeting this week. (even though concept review but it's the right folks in the right place)

Hey @Krinkle - we were wondering if you had any thoughts about the viability, potential redundancy and performance of this proposed extension. Thanks!

sbassett changed the task status from Open to Stalled.Jan 14 2020, 6:29 PM
sbassett moved this task from In Progress to Watching on the Security-Team board.
Krinkle renamed this task from Security Concept Review For WikiUnit to Security concept review for WikiUnit extension.Jan 15 2020, 2:18 AM
Bias / other direction

My initial reaction is that I'm slightly biased against this direction, for a number of reasons:

There is nothing preventing gadget developers from using QUnit today (or other unit test methodologies). As such, it would not be a fundamentally enabling effort to provide such an extension. It would take some effort to build a test runner as user script, but I believe it would take considerably less effort to do as user script than as an extension.

Doing it as an extension would additionally bring in significant limitations and complexities. One being the version of QUnit being hard to control (when to upgrade? how long to keep an old version for?). We've tries a centralised approach to this kind of problem in the past and I don't recall it ever working out long-term. It seems that no matter how hard we try, this always ends up being something individual developers prefer to have control over. And that's probably fair, but it means we're much better off investing in a generic means to do something like this, than to hardcode a solution to one specific way to do one specific aspect of the picture.

The generic means being that if you're a developer familiar with testing and are interested in writing and enforcing such tests, that you're likely interested in a lot of other developer capabilities as well (using a desktop text editor, git-blame and other versioning features, reviewing change requests, linting/testing and other CI features).

At TechConf 2019 the topic of gadgets came up and while there is not yet a clear owner and roadmap, it seems there is at least growing interest for offering a way for gadgets to be deployed from a Git repository (e.g. Gerrit repo with code review and Jenkins CI). This would be entirely optional of course, but for those interested in getting translations via translatewiki, code review in Gerrit, linting and unit testing and other testing abilities via Jenkins etc, ability to automatically have the gadget in sync on multiple wikis etc., this would be an option. See also T171577 and T71445.

This direction is particularly of interest to me as I see that numerous gadgets are in fact already developed outside the wiki (e.g. on GitHub), where they are already enjoying the ability to lint and unit test their code. I expect that that the gap between beginning developers and developers comfortable with using Git is likely to shrink over time as Git interfaces become easier to use and with more support around them (other users helping out, automatic fixing of lint errors, real-time feedback in your text editor etc.). In fact I believe that for a significant number of junior engineers, the Git-based approach is in fact easier to use than working with the in-browser experience we offer today. In particular due to it being difficult to make multiple changes at once, and with no place to safely safe an edit while still working on it. With a local repo one can "save" the file at any time and then safely copy the build output to the browser console for testing.

Performance

As for performance specifically with the extension as proposed, it is hard to say exactly as it depends a lot on the details.

If the source code were to be loaded as ResourceLoader modules, that would likely cause a performance regression for all users due to startup module overhead. The current proposal suggests fetching it directly from a wiki sub page and sending it out in an inline script tag during the preview action. I don't foresee any insurmountable performance issues with that approach.

However as short version of the above, I think this would be much better fitted as a site script enabled via a button and query parameter (useJS) through local means. That seems very minimal to set up. That also allows versioning and expansion of the logic to be controlled on a per-wiki basis.

I would like to know what major benefits are expected to come from it being an extension. For example, is there something we can do in core to better accomodate this use case better in general?

Thanks for the feedback, @Krinkle !

There is nothing preventing gadget developers from using QUnit today (or other unit test methodologies).

True, though there are issues around Gadget developers sometimes being less familiar with unit-testing and its benefits and the current Gadgets architecture not mandating version-control, code review, etc. within common supporting systems like gerrit or github. These do create at least soft barriers to more ubiquitous testing and quality control IMO.

It would take some effort to build a test runner as user script, but I believe it would take considerably less effort to do as user script than as an extension.

I wonder where staff and/or volunteer time would be best spent. Is it in developing an imperfect but possibly useful solution like the above or providing better Gadget process documentation and training for both staff and volunteer developers? Both?

The generic means being that if you're a developer familiar with testing and are interested in writing and enforcing such tests, that you're likely interested in a lot of other developer capabilities as well (using a desktop text editor, git-blame and other versioning features, reviewing change requests, linting/testing and other CI features).

I'd concur, which makes me question if even a userJS-based solution would have a low enough cost-benefit ratio.

At TechConf 2019 the topic of gadgets came up and while there is not yet a clear owner and roadmap, it seems there is at least growing interest for offering a way for gadgets to be deployed from a Git repository (e.g. Gerrit repo with code review and Jenkins CI).

Just an additional note: the Security-Team is currently working on a risk assessment of the Gadgets ecosystem in an effort to eventually drive incremental change which may or may not encompass elements of the Gadgets 2.0 and 3.0 proposals.

This direction is particularly of interest to me as I see that numerous gadgets are in fact already developed outside the wiki (e.g. on GitHub)

Just curious if we have some real numbers on this as I had assumed most Gadget code wasn't being developed in this way, and was more akin to one-off copypasta userJS development and "deployment".

@Evad37 -

Were there any further questions for this concept review? From the discussions above, it seems like your existing UserJS proof-of-concept from the description would probably be the easiest way to get/keep this functionality on production wikis as opposed to writing a new extension, which would probably be difficult to get into Wikimedia production without the backing of a development group.

Yeah, if there isn't support for writing a new extension, and it isn't likely to be deployed, and the existing setup works for my needs, then there's not much point of me working on this. It's just a shame that anyone wanting to do some unit testing for scripts has to basically reinvent the wheel each time (unless they happen to stumble upon someone else's solution).

@Evad37 - yes, this is a very frustrating part of the Wikimedia developer community, though I might suggest:

  1. Shopping the extension around to a relevant WMF development group and see if there's more interest. Some obvious candidate teams IMO would be:
    1. Community Tech
    2. Product Infrastructure
    3. Readers Web/Mobile
    4. Quality and Test Engineering Team
  2. Proposing your current user-script as a Gadget for wider use at a relevant village pump. Though there definitely is no guarantee that it will be approved as a Gadget and it may involve cleaning up your user-script and documentation a bit.
  3. Expanding and copying your current user-script documentation to mediawiki.org as more "official" documentation, and maybe reference it under relevant pages. That way there's a place to point folks to who are trying to solve the same problem or would like some example code. I'm not entirely certain if anyone would be persnickety about doing this, but I feel like it's a reasonable request which I would personally help support.
  4. Once any of the above happen, maybe send out an email to wikitech-l or write a Wikimedia Space post to further promote your user-script.
sbassett closed this task as Resolved.Feb 12 2020, 4:07 PM
sbassett claimed this task.
sbassett moved this task from Watching to Our Part Is Done on the Security-Team board.
chasemp moved this task from Incoming to Our Part Is Done on the secscrum board.Mar 10 2020, 8:18 PM