Page MenuHomePhabricator

[Task] Security review of Wikibase-Quality-External-Validation branch master
Closed, DeclinedPublic

Description

Please do a security review of Wikibase-Quality-External-Validation.
Thank you very much!

Gerrit: https://gerrit.wikimedia.org/r/#/admin/projects/mediawiki/extensions/WikibaseQualityExternalValidation

Event Timeline

Tamslo set Security to None.
csteipp added a project: Security-Team.
csteipp moved this task from Incoming to Ready on the Security-Team board.

@csteipp: are you still working on it? And on T99352? :)

@Tamslo, are you asking if this can be closed? Definitely not. Both of the other extensions have serious issues that need to be addressed before they can be deployed, and I've only started reviewing this one. If plans change on wmde's side, please let me know.

Could you be so kind and clarify the blocking issues for the other two extensions? For Constraints we have seen only one open ticket (T101467), for which we have uploaded a fix.
Same for the Quality extension. Only one blocking task for which we have uploaded some patches. I'm not sure if we can close it on our own or if you could have another look at it?

As far as the plans of wmde are concerned:
The current plan is to do a Beta deploy about June, 19th 2015 0800 PDT.
On June, 30th there should be a deploy on test and July, 8th on wikidata.org. So far the current plans.

Do you have, additional to the questions above, something that would block the planned beta deploy?

Short clarification: Please review branch v1 on gerrit.

The URL of the repository changed, see the description.

@Tamslo, what is the expected size of wbqev_dump_information?

@csteipp, wbqev_dump_information contains a row for each database dump, that was imported. For now, there are only 3 ones. When new databases are integrated for cross-checks, the number will increase, but this table will always just have a few rows.

JanZerebecki renamed this task from Security review of Wikibase-Quality-External-Validation to Security review of Wikibase-Quality-External-Validation branch v1.Jun 25 2015, 11:02 AM

SpecialCrossCheck::buildResultTable
$referenceStatus = $this->msg( "wbqev-crosscheck-status-" . $result->getReferenceResult()->getStatus() )->text();

Either user escaped() or don't use rawhtml in the table cell.

In ComparisonResult you guard setting the result to a list of constant strings, but in ReferenceResult that only happens on object creation. ReferenceResult should do the same as ComparisonResult.

SpecialCrossCheck::buildResultTable
$referenceStatus = $this->msg( "wbqev-crosscheck-status-" . $result->getReferenceResult()->getStatus() )->text();

Either user escaped() or don't use rawhtml in the table cell.

That already got fixed. Sorry about that.

Change 221104 had a related patch set uploaded (by Dominic.sauer):
T99358 guard setting the result to a list of constant strings in ReferenceResult similar to the ComparisonResult

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

Change 221107 had a related patch set uploaded (by Dominic.sauer):
T99358 guard setting the result to a list of constant strings in ReferenceResult similar to the ComparisonResult

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

Change 221104 merged by jenkins-bot:
T99358 guard setting the result to a list of constant strings in ReferenceResult similar to the ComparisonResult

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

Change 221107 merged by jenkins-bot:
T99358 guard setting the result to a list of constant strings in ReferenceResult similar to the ComparisonResult

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

Lydia_Pintscher renamed this task from Security review of Wikibase-Quality-External-Validation branch v1 to [Task] Security review of Wikibase-Quality-External-Validation branch v1.Aug 17 2015, 4:18 PM

@csteipp: Is this good to go from your side once T103912 is closed?

JanZerebecki renamed this task from [Task] Security review of Wikibase-Quality-External-Validation branch v1 to [Task] Security review of Wikibase-Quality-External-Validation branch master.Sep 29 2015, 10:53 AM
csteipp moved this task from Incoming to Scheduled on the deprecated-security-team-reviews board.

@csteipp: Is this good to go from your side once T103912 is closed?

Yes

Lydia_Pintscher changed the task status from Open to Stalled.Apr 6 2017, 3:15 PM

Sorry. For now you can ignore this. I'll mark it as stalled and reopen it when it becomes relevant again.

Aklapper lowered the priority of this task from High to Low.
Aklapper removed a project: Patch-For-Review.

Lowering priority per last comment by Lydia; resetting assignee.