Page MenuHomePhabricator

Application Security Review Request : CommunityRequests Extension
Closed, ResolvedPublic

Description

Project Information:

  • Name of tool/project: CommunityRequests
  • Project home page: https://www.mediawiki.org/wiki/Extension:CommunityRequests
  • Name of team requesting review: Community Tech
  • Primary contact: @JWheeler-WMF and @KSiebert
  • Target date for deployment: 1 August 2025
  • Link to code repository / patchset: https://gerrit.wikimedia.org/r/c/mediawiki/extensions/CommunityRequests/+/1167986
  • Link to scc output for general sizing of codebases (https://github.com/boyter/scc):
    1───────────────────────────────────────────────────────────────────────────────
    2Language Files Lines Blanks Comments Code Complexity
    3───────────────────────────────────────────────────────────────────────────────
    4JSON 63 22611 0 0 22611 0
    5PHP 49 7758 783 1445 5530 189
    6JavaScript 17 1406 144 313 949 49
    7Vue 13 2379 149 404 1826 68
    8SQL 3 248 56 12 180 0
    9Markdown 2 4 1 0 3 0
    10Plain Text 2 139 8 0 131 0
    11LESS 1 50 7 4 39 0
    12License 1 339 58 0 281 0
    13XML 1 7 0 0 7 0
    14gitignore 1 8 0 0 8 0
    15───────────────────────────────────────────────────────────────────────────────
    16Total 153 34949 1206 2178 31565 306
    17───────────────────────────────────────────────────────────────────────────────
    18Estimated Cost to Develop (organic) $1,013,354
    19Estimated Schedule Effort (organic) 13.82 months
    20Estimated People Required (organic) 6.51
    21───────────────────────────────────────────────────────────────────────────────
    22Processed 1346533 bytes, 1.347 megabytes (SI)
    23───────────────────────────────────────────────────────────────────────────────

Description of the tool/project:
The CommunityRequests extension is a system for managing a wiki community's requests and suggestions for technical development. What used to be the community wishlist survey and was run once a year will now be an extension that will be run all year around and allow collecting wishes from technical communities continuously.

Description of how the tool will be used at WMF:
The tool will allow us to collect wishes, group them and prioritise them to be able to plan technical needs.

Dependencies

Has this project been reviewed before?
No.

Working test environment

Post-deployment
Community Tech // @KSiebert or @JWheeler-WMF

Details

Risk Rating
Low

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
sbassett changed the task status from Open to In Progress.Jul 3 2024, 5:12 PM
sbassett claimed this task.
sbassett triaged this task as Medium priority.
sbassett moved this task from Upcoming Quarter Planning Queue to In Progress on the secscrum board.

@sbassett As you may have noticed, work on this extension stalled. We had to go the MediaWiki-extensions-Gadgets route for the initial launch, but we are planning to extensionize the code.

My question for you: If we (i.e. my manager) agree to accept all security risks, how likely if at all would it be to ship the extension with just translations, then we slowly add code to it over time? This would make the transition considerably easier, and allow us to address some issues we have now such as T370230: Migrate translations to Community Requests. When we have enough code moved over for the extension to run standalone, we can ask for a proper security review. Until then, we could have all code (other than the translations) behind a feature flag, which we will not enable until the security review is complete. How does that sound? I know this is highly unusual but I thought it was at least worth inquiring :)

My question for you: If we (i.e. my manager) agree to accept all security risks, how likely if at all would it be to ship the extension with just translations, then we slowly add code to it over time? This would make the transition considerably easier, and allow us to address some issues we have now such as T370230: Migrate translations to Community Requests. When we have enough code moved over for the extension to run standalone, we can ask for a proper security review. Until then, we could have all code (other than the translations) behind a feature flag, which we will not enable until the security review is complete. How does that sound? I know this is highly unusual but I thought it was at least worth inquiring :)

So the extension would be deployed to wikimedia production, but only translations would be made available for it? e.g. anything in its i18n directory? If I'm understanding this correctly, that sounds fine. Keeping feature development on a separate, undeployed branch would probably be preferred to a wikimedia production feature flag, but I'm not sure if that would suit your development needs. Regardless, it sounds like we should place this review request into our backlog again, for now? Until we have a better sense of when most of the code will be ready for the extension?

So the extension would be deployed to wikimedia production, but only translations would be made available for it? e.g. anything in its i18n directory? If I'm understanding this correctly, that sounds fine.

Awesome! We will start migrating messages, then. Note currently the repo has other files in it, including a Hooks.php, but they all appear to be no-ops. We can remove this code for now if it is of concern.

Keeping feature development on a separate, undeployed branch would probably be preferred to a wikimedia production feature flag, but I'm not sure if that would suit your development needs.

Yes, we could use a different branch but for Gerrit I think that means we have to rely on relation chains and just not merge, right? Indeed that would seriously hamper development. So I think we will go with a feature flag -- but I believe that is going to be necessary anyway because in order to do the security review, we need the extension deployed to the Beta cluster, but also simultaneously not deploy to production.

Regardless, it sounds like we should place this review request into our backlog again, for now? Until we have a better sense of when most of the code will be ready for the extension?

I suppose for now, yes. My wild estimate is some months before we're ready for a security review. I understand you do your reviews quarterly, so we can still aim for the end of Q1 2024-25 (~2 months from now).

Yes, we could use a different branch but for Gerrit I think that means we have to rely on relation chains and just not merge, right? Indeed that would seriously hamper development. So I think we will go with a feature flag -- but I believe that is going to be necessary anyway because in order to do the security review, we need the extension deployed to the Beta cluster, but also simultaneously not deploy to production.

Ok, that's fair.

I suppose for now, yes. My wild estimate is some months before we're ready for a security review. I understand you do your reviews quarterly, so we can still aim for the end of Q1 2024-25 (~2 months from now).

Since this was already assigned at the beginning of this quarter, yes, it should be possible. If you could just keep this task updated with any key progress, that would be great.

Hey @JWheeler-WMF and @KSiebert -

Any update on when all relevant code might be ready for a security-review? Since we only have a week and a half or so left in the current quarter, I'm guessing we'd now be targeting a date next quarter?

Any update on when all relevant code might be ready for a security-review? Since we only have a week and a half or so left in the current quarter, I'm guessing we'd now be targeting a date next quarter?

I was just going to write you!

We are racing to get things migrated over this week and early next week, in hopes of still getting a review this quarter. While the bulk of the code will be present (but not necessarily merged), it will not be a fully functioning extension quite yet. Allow me to explain :)

We are in a unique situation here where the Wishlist is an ongoing, self-evolving product that is powered by a combination of gadgets, bot tasks, and on-wiki templates/modules. A slow, piecemeal migration to an extension I think is the only reasonable way for this to ever happen given given we're working with a moving target. That migration started with the translations, which as you know are deployed now. Next is data persistence (parser function + hook to persist to db), then finally the Vue application to render a UI to the user. The stuff that lives on wiki pages now will probably stay that way, so visually there's not a lot to the extension that isn't already live, if that means anything.

My thought was, since most of the code that powers the Wishlist is a Vue application (and other on-wiki content not subject to code review), having the Vue code ported over and running minimally would be enough for security review? I.e. I'd like to get the [[ meta.wikimedia.org/wiki/Community_Wishlist/Intake | intake form ]] loading on a Patch Demo, and I think we'll also have some of the MinT integration ready to demo as well. The code will be available on Gerrit for static analysis etc., but won't necessarily be merged yet. The remaining code that is yet to be written will largely be the same as what we did for MediaWiki-extensions-PageAssessments and MediaWiki-extensions-Phonos (other parser function extensions) – that is to say, we aren't treading into technical unknown territory, and thus perhaps willing to accept any implicit security risks.

We can of course postpone the target date for this task, but I hope that doesn't mean we will need to wait another 3 months for review. The longer we wait, the more fragmented the two implementations of the Wishlist become. I think, as irregular or counter-intuitive as it may seem with respect to security and release engineering, it's most practical to migrate the software to the extension in piecemeal chunks. It's not an issue yet, but eventually https://meta.wikimedia.org/wiki/Community_Wishlist/Wishes will contain too much content for a wiki page, necessitating pagination, and that's the point where we really need to have an extension (or hacky Toolforge tool with CORS errors in the JS console…). Hence why I outlined the proposed road map (T366194) in its stated order. I still think within just a few days, we can have most of those checkboxes at T366194 ticked, it just won't be production-ready per-se due to the unusual circumstances we're working with.

Regardless, I will set a personal deadline of Tuesday, September 24 or sooner to have a mostly working (but static) survey on Patch Demo. I'll keep this task updated along the while, and come Tuesday/Wednesday you let me know if we have enough for the review. How does that sound? :)

Many thanks in advance!

@MusikAnimal - Ideally we'd have any and all relevant code for this project completed (or at least in a very stable state) for the security review. Our team would typically like to avoid performing multiple, piecemeal reviews of different codebases and/or re-reviewing more volatile codebases. If we can have most of that in place by September 24th, that's great, though I cannot guarnatee the review will be completed before the quarter ends, a few days later. Though it could very likely be completed within the first week or two of the next quarter, for sure. If we can't guarnatee that all relevant code is in a stable or near-complete state by September 24th, then I think it would be best to discuss a different completion date. However, we are happy to try to accommodate various engineering team milestones and deadlines, and would try to work with you to complete the review earlier next quarter (as opposed to the last week or two). Let me know how you'd like to proceed.

I don't think we'll get it done by the 24th (which is already today in some parts of the world). But if we can aim for a date within the first couple of weeks of next quarter that sounds great, I think! We're making progress now with moving all the functionality from the gadget, so hopefully the code will be representative of the completed extension (e.g. it'll show what we're aiming for with the special pages, database tables, parser functions, font-end access of remote APIs, etc.). Perhaps not perfect, and we'll be continuing to work on it, but the skeleton (and a fair bit of the detail) will be done.

Ok, thanks @Samwilson. I'm going to move this task back to our quarterly planning column where it will be re-reviewed during our next scheduled planning session (October 1st, 2024). I'll note that we'll plan to complete this review sometime within the month of October 2024.

Thanks all, I set the target date of the task to first of November 2024.

sbassett claimed this task.
sbassett added a project: user-sbassett.
sbassett moved this task from Backlog to In Progress on the user-sbassett board.
sbassett added a subscriber: mmartorana.
sbassett removed a subscriber: mmartorana.

Hey @Samwilson @KSiebert @MusikAnimal - I've got this assigned to me for this quarter and plan to begin the review as soon as the code is ready this month. If you could let me know when that happens on this task, that would be great.

sbassett changed the task status from Open to In Progress.

Hey all - I'm going to move this review request into our backlog for now since I've heard no response or updates on the development of this code, since my last comment over a month and a half ago. We can likely plan the review for next quarter, if progress has been made.

sbassett changed the task status from In Progress to Open.Nov 22 2024, 3:54 PM
sbassett lowered the priority of this task from Medium to Low.
sbassett moved this task from In Progress to Upcoming Quarter Planning Queue on the secscrum board.
sbassett removed a project: user-sbassett.

Yes, I'm terribly sorry for the silence. I had read your every inquiry, and kept thinking we'd have something to show in just another few days/weeks… but obviously it's not moving very quickly! :(

Meanwhile https://meta.wikimedia.org/wiki/Community_Wishlist/Wishes still loads fine. That's our main indicator for when the Wishlist would essentially need to be extensionized. It sounds like we have probably have a few more months to spare.

So I guess we'll be back in touch next quarter :)

Thanks again for your patience and leniency in working with us!

@MusikAnimal - Hey, no problem. We'll get this done when you're ready. I'm going to backlog it for now though. But we can easily prioritize it for any upcoming quarter when it gets close to production-ready.

@MusikAnimal - Any update on this work? Are we looking to get this reviewed this quarter? Thanks.

@MusikAnimal - Any update on this work? Are we looking to get this reviewed this quarter? Thanks.

Yes, actually! We have just started on development. I expect it to be complete by the end of the quarter, hopefully in as little as a month.

Yes, actually! We have just started on development. I expect it to be complete by the end of the quarter, hopefully in as little as a month.

Ok. Timing-wise, it would probably make sense to schedule the security review for early next quarter (July - Sept) since we're already almost halfway through the current quarter and the Security Team has already scheduled our reviews for the current quarter. Would that work?

Ok. Timing-wise, it would probably make sense to schedule the security review for early next quarter (July - Sept) since we're already almost halfway through the current quarter and the Security Team has already scheduled our reviews for the current quarter. Would that work?

I think that will be fine. Thank you!! :)

@sbassett is it possible to prioritize and / or escalate the CommunityRequests Extension security review? We plan to be finished in May, and then intend to build incremental UX enhancements. Delaying until Mid-July would delay these feature enhancements the community has asked for, and it adds additional security risk.

@sbassett is it possible to prioritize and / or escalate the CommunityRequests Extension security review?

@JWheeler-WMF We've discussed internally and may be able to support an additional expedited review. For us to do that, please add to this ticket some more detail below (see my prompts), to make the rationale more clear.

We plan to be finished in May, and then intend to build incremental UX enhancements.

How confident are you that you'll be finished in May?

Delaying until Mid-July would delay these feature enhancements the community has asked for, and it adds additional security risk.

It's not obvious how delaying would add security risk - if you have a specific point you want to make there, please do. More generally, could you say something about why it's particularly important to avoid a delay to mid-July?

@EMill-WMF apologies for the delay. The CommTech team had to delay work due to our wishathon, and I don't want to add undue pressure on any teams. A review in early to Mid-July would be okay, but I don't think we can go much past that... All of this is assuming that we'd be ready for review by July 1.

@EMill-WMF apologies for the delay. The CommTech team had to delay work due to our wishathon, and I don't want to add undue pressure on any teams. A review in early to Mid-July would be okay, but I don't think we can go much past that... All of this is assuming that we'd be ready for review by July 1.

Security-Team will prioritize this review to be completed, at the latest, by mid-July 2025.

sbassett changed the task status from Open to In Progress.Jul 8 2025, 3:25 PM
sbassett assigned this task to Jly.
sbassett raised the priority of this task from Low to Medium.
sbassett moved this task from Upcoming Quarter Planning Queue to In Progress on the secscrum board.

@JWheeler-WMF Can I check if the codebase is ready to be reviewed? We'd like to ensure it's as close to production-ready as possible, so there aren't any major features or changes introduced after the review.

@JWheeler-WMF Can I check if the codebase is ready to be reviewed? We'd like to ensure it's as close to production-ready as possible, so there aren't any major features or changes introduced after the review.

I was waiting for a ping… but the "at the latest, by mid-July" would actually be doing us a favour :)

I think we can have it all ready by Monday, July 14, if that's okay?

No worries, no rush! Please give me a ping when it's ready

@Jly This is now ready for review! The task description has been updated accordingly. Thanks :)

@Jly @sbassett just wanted to confirm that this is currently under review by Security? Double-checking since it was moved to "In Progress" on the secscrum board on July 8th, but @MusikAnimal mentioned it was ready for review a few days later on July 14th. I simply saw that the last couple of weekly reports in the correlated Asana hypothesis project mentioned this work was awaiting Security review, and want to make sure the folks involved are aligned and see if there's anything I can do to support unblocking. :)

@Jly @sbassett just wanted to confirm that this is currently under review by Security? Double-checking since it was moved to "In Progress" on the secscrum board on July 8th, but @MusikAnimal mentioned it was ready for review a few days later on July 14th. I simply saw that the last couple of weekly reports in the correlated Asana hypothesis project mentioned this work was awaiting Security review, and want to make sure the folks involved are aligned and see if there's anything I can do to support unblocking. :)

Yes, this review is in progress and assigned to @Jly this quarter. We typically schedule and complete security reviews on a quarterly basis. If there is a need for an expedited timeline, we can try to accommodate that, but cannot always guarantee that level of service. Let @Jly or I know if you have any additional questions or concerns.

Hey @MBinder_WMF - Sorry for the confusion here. I think there were some mixed messages going on in these comments, and my previous comment likely isn't very helpful. I think we were targeting mid- / end-of-July for the completion of the review if we had the codebase ready a bit sooner than July 14th, and I should have been more clear about that. I've reached out to @Jly to understand what might be a reasonable turnaround date over the next several weeks for this review. I'll provide an update on this soon and hope that can be an acceptable, non-blocking solution.

Thanks @sbassett ! I know you folks are in high demand, and I trust you to tell us what's reasonable. If @HMonroy has any concerns, we'll bring them here! :)

Hey all, yes, exactly, just like Scott mentioned. I've been reviewing it on and off, and I should have something by the end of next week or the week after at the latest. I will keep you posted on any delays.

@Jly is there any way to prioritize this review to be completed by end of next week? We discussed having a review in mid-July, and cannot release the wishlist extension until is complete.

@JWheeler-WMF No worries, I can aim for the end of next week.

Security Review Summary - T365525 - 2025-08-02
Last commit reviewed: 72b33ae

Summary
The codebase is generally well-structured and defined. There are vulnerable packages that need to be addressed and a few findings related to XSS and modification of data. Please refer to the Dynamic Application Scanning Findings section below.

Overall risk rating: medium.

Vulnerable Packages
high

Vulnerability namePackageReference URLRemediationSeverity
form-data uses unsafe random function in form-data for choosing boundaryform-datahttps://github.com/advisories/GHSA-fjxv-7rqg-78g4Manual update required critical
Server-Side Request Forgery in Requestrequesthttps://github.com/advisories/GHSA-p8p7-x288-28g6Manual update required critical
tough-cookie Prototype Pollution vulnerabilitytough-cookiehttps://github.com/advisories/GHSA-72xf-g2v4-qvf3Manual update required medium

Outdated Packages
As reported via npm outdated:

PackageCurrentWantedLatestLocation
@babel/preset-env7.25.47.25.47.28.0node_modules/@babel/preset-env
@wdio/cli9.15.09.15.09.18.4node_modules/@wdio/cli
@wdio/junit-reporter9.15.09.15.09.18.0node_modules/@wdio/junit-reporter
@wdio/local-runner9.15.09.15.09.18.4node_modules/@wdio/local-runner
@wdio/mocha-framework9.15.09.15.09.18.0node_modules/@wdio/mocha-framework
@wdio/spec-reporter9.15.09.15.09.18.0node_modules/@wdio/spec-reporter
jest29.7.029.7.030.0.5node_modules/jest
jest-environment-jsdom29.7.029.7.030.0.5node_modules/jest-environment-jsdom
vue3.5.133.5.133.5.18node_modules/vue
wdio-mediawiki4.1.34.1.35.1.0node_modules/wdio-mediawiki

As reported via composer outdated:
(no direct vulnerabilities reported, only transitive packages were found and simply noting for completeness' sake.)

PackageCurrentWantedDescription
netresearch/jsonmapper4.5.05.0.0Map nested JSON structures onto PHP classes
phpcsstandards/phpcsextra1.2.11.4.0A collection of sniffs and standards for use with PHP_CodeSniffer.
phpcsstandards/phpcsutils1.0.121.1.0A suite of utility functions for use with PHP_CodeSniffer
sabre/event5.1.76.0.1sabre/event is a library for lightweight event-based programming
squizlabs/php_codesniffer3.12.23.13.2PHP_CodeSniffer tokenizes PHP, JavaScript and CSS files and detects violations of a defined set of coding stan...

Static Analysis Findings
Risk: low.
A handful of popular, FOSS static analysis tools were run against the codebase. A summary of some of these tools' output exists below:

  1. semgrep - False positive findings were found related to dynamic rendering of abritrary HTML which were found to be either fixed messages or have been sanitized/formatted.
  2. Bearer - False positive findings were found related to SQL injection which were parameterised via query builders.
  3. phan-taint-check - No SAST results.
  4. gitleaks - No issues found.
  5. trufflehog - No issues found.

General code health score

  1. The Wikimedia code health check tool returned a weighted risk score of 23.30.
PROJECT: CommunityRequests@master
REPOSITORY: https://gerrit.wikimedia.org/r/mediawiki/extensions/CommunityRequests

+-----------+----------+----------+------+---------------+---------------+--------------+-------------+------------+--------------+-----------+---------------+
| Vuln Pkgs | Pkg Mgmt | Test Cov | SAST | Non-auto Cmts | Uniq Contribs | Contrib Conc | Lang Guides | Staff Supp | Task Backlog | Code Stew | Weighted Risk |
+===========+==========+==========+======+===============+===============+==============+=============+============+==============+===========+===============+
|         0 |        0 |        2 |    2 |             5 |            10 |            2 |           7 |         10 |            0 |         0 |         23.30 |
+-----------+----------+----------+------+---------------+---------------+--------------+-------------+------------+--------------+-----------+---------------+

Dynamic Application Scanning Findings
Risk: medium.

Since this extension is currently used in Meta-Wiki, please refer to P80545 instead.

Hi @Jly, thank you for working on this so promptly.

Since we did not introduce any new packages in this extension, the only issues that need addressing on our side are those listed in P80545. Is that correct?

Since this extension is currently used in Meta-Wiki, please refer to P80545 instead.

The extension is not deployed anywhere yet, so I want to confirm that it is ok to create Phab tasks without tagging security, and submit patches to Gerrit as if they were regular tasks

Since we did not introduce any new packages in this extension, the only issues that need addressing on our side are those listed in P80545. Is that correct?

I would still strongly encourage updating vulnerable/outdated packages where possible.

The extension is not deployed anywhere yet, so I want to confirm that it is ok to create Phab tasks without tagging security, and submit patches to Gerrit as if they were regular tasks

My apologies, thanks for the correction. If it's not deployed anywhere, please go ahead and do that.

Vulnerable Packages
high

Vulnerability namePackageReference URLRemediationSeverity
form-data uses unsafe random function in form-data for choosing boundaryform-datahttps://github.com/advisories/GHSA-fjxv-7rqg-78g4Manual update required critical
Server-Side Request Forgery in Requestrequesthttps://github.com/advisories/GHSA-p8p7-x288-28g6Manual update required critical
tough-cookie Prototype Pollution vulnerabilitytough-cookiehttps://github.com/advisories/GHSA-72xf-g2v4-qvf3Manual update required medium

For the record, these are all from wdio-mediawiki / mwbot which is in every repo with Selenium tests, including Core. All versions of the package have these vulnerabilities, and they've been present for years now ¯\_(ツ)_/¯

For the record, these are all from wdio-mediawiki / mwbot which is in every repo with Selenium tests, including Core. All versions of the package have these vulnerabilities, and they've been present for years now ¯\_(ツ)_/¯

This would likely involve an upstream request for mwbot which, somewhat tragically, looks like it's basically unmaintained at this point.

@MusikAnimal @dmaza It does look to be unmaintained. We would need your manager/director's approval to accept these risks and to be entered in the risk registry.

@Jly The normal EM would be @KSiebert but she's away. @Arrbee can you weigh in as the director? :)

@MusikAnimal @dmaza It does look to be unmaintained. We would need your manager/director's approval to accept these risks and to be entered in the risk registry.

This is the same package that is in MediaWiki Core and dozens of extensions that are already deployed to production. It's also only a dev-dependency, i.e. only used in CI and on our local environments. This means the risk has already been taken, many thousands of times over, by the entire organization :) I don't think we need manager approval here specifically for CommunityRequests – especially given the risk does not effect production.

For the record, these are all from wdio-mediawiki / mwbot which is in every repo with Selenium tests, including Core. All versions of the package have these vulnerabilities, and they've been present for years now ¯\_(ツ)_/¯

This would likely involve an upstream request for mwbot which, somewhat tragically, looks like it's basically unmaintained at this point.

Not for this task, but my thoughts are to move to https://mwn.toolforge.org/ which is a much nicer library and lacks most of those security problems (though I think it still has one similar lower-risk vulnerability if I remember correctly).

I am approving to go ahead with this.

Thanks, I'll go ahead and put this into the risk register as it's something we still want to keep track of in the future.

Thank you all for being so helpful and accommodating. With this task marked as Resolved, does that mean we are free to start deploying the extension to Beta, and Meta when we are ready?

Thank you all for being so helpful and accommodating. With this task marked as Resolved, does that mean we are free to start deploying the extension to Beta, and Meta when we are ready?

Yes, you should be good to go as @Arrbee has accepted the medium risk for the dependency vulnerabilities.