Security review for Extension:JADE
Closed, ResolvedPublic
Actions

Description

Project Information

Name of tool/project: Extension:JADE
Project home page: https://www.mediawiki.org/wiki/JADE
Name of team requesting review: WMF Scoring Platform Team
Primary contact: @awight
Target date for deployment: ASAP—but we're not blocking anything external.
Link to code repository / patchset: https://phabricator.wikimedia.org/diffusion/EJAD/

Description of the tool/project ..

This extension creates two namespaces, and adds a content handler for one of them. That's about all.
https://www.mediawiki.org/wiki/JADE

Description of how the tool will be used at WMF ..

Mostly, third-party patrolling tools like Huggle will be making API edits into the Jade: namespace, in order to coordinate between patrollers.

Dependencies

No dependencies.

Has this project been reviewed before?

No, only intra-team code review in GitHub and Gerrit.

Working test environment

A mw-vagrant role is available: vagrant roles enable jade

Post-deployment

https://www.mediawiki.org/wiki/Wikimedia_Scoring_Platform_team
IRC: awight

Details

	Subject	Repo	Branch	Lines +/-
	Security review followups	mediawiki/extensions/JADE	master	+3 -9

Customize query in gerrit

Related Objects
Search...

Status	Assigned	Task
Resolved	None	T176324 Scoring platform team FY18 Q2
Resolved	awight	T176333 Deploy JADE prototype in Beta Cluster
Resolved	Bawolff	T188308 Security review for Extension:JADE

Event Timeline

awight created this task.Feb 26 2018, 8:25 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 26 2018, 8:25 PM

awight mentioned this in T188307: Extension review for JADE.Feb 26 2018, 8:27 PM

awight added a parent task: T176333: Deploy JADE prototype in Beta Cluster.

awight added subscribers: Bawolff, Legoktm.Feb 27 2018, 5:04 PM

Halfak moved this task from Parked to Review on the Machine-Learning-Team (Active Tasks) board.Feb 28 2018, 3:25 PM

This task needs more info. See https://www.mediawiki.org/wiki/Wikimedia_Security_Team/Security_reviews#Requesting_a_review

awight updated the task description. (Show Details)Mar 2 2018, 5:52 PM

awight updated the task description. (Show Details)

Bawolff claimed this task.Mar 10 2018, 7:00 AM

Bawolff moved this task from Incoming to In Progress on the deprecated-security-team-reviews board.

Review passed.

Some minor comments

"JudgmentValidator.php" line 172 - should probably be $wgContLang->ucfirst() unless you are very sure it will only be english text fed to this as I believe php's ucfirst doesn't work well with non-ascii characters.
includes/ContentHandlers/JudgmentEditAction.php - This seems unused... If its not used it should probably be deleted. If there are plans to alter the edit action process in significant ways, I would like to review that just to make sure its all safe.

In T188308#4040037, @Bawolff wrote:

Review passed.

Some minor comments

Thanks, good points!

"JudgmentValidator.php" line 172 - should probably be $wgContLang->ucfirst() unless you are very sure it will only be english text fed to this as I believe php's ucfirst doesn't work well with non-ascii characters.

In theory, the only correct workflow validates the data, so we know the string is a member of an ASCII enum—however, the validation functions are public, so it's possible for a really determined programmer such as future me to run the workflow out-of-order :-)

includes/ContentHandlers/JudgmentEditAction.php - This seems unused... If its not used it should probably be deleted. If there are plans to alter the edit action process in significant ways, I would like to review that just to make sure its all safe.

Oops!

gerritbot added a project: Patch-For-Review.Mar 13 2018, 3:53 PM

awight mentioned this in rEJAD90d5ea771a1a: Security review followups.Mar 13 2018, 3:54 PM

Change 419211 merged by jenkins-bot:
[mediawiki/extensions/JADE@master] Security review followups

https://gerrit.wikimedia.org/r/419211

ReleaseTaggerBot added a project: MW-1.31-release-notes (WMF-deploy-2018-03-20 (1.31.0-wmf.26)).Mar 14 2018, 9:00 AM

Halfak mentioned this in T200297: Review Jade data storage and architecture proposal [RFC].Jul 30 2018, 8:17 PM

sbassett moved this task from Waiting to Done on the deprecated-security-team-reviews board.Jul 9 2019, 8:20 PM

• chasemp removed a project: deprecated-security-team-reviews.Jan 8 2020, 4:12 PM

• chasemp added a project: Application Security Reviews.Jan 8 2020, 4:38 PM

• chasemp moved this task from Incoming to Our Part Is Done on the Application Security Reviews board.Jan 8 2020, 4:43 PM

• chasemp added a project: secscrum.Mar 10 2020, 8:11 PM