Page MenuHomePhabricator

Upgrade phan to 0.10.2 and remove phan-taint-check-plugin
Closed, ResolvedPublic

Description

This now includes taint-check, per T235390. A few notes:

  • The 'extra' key should be removed from composer.json (or actually, taint-check should be removed from there)
  • Upgrading core will be painful due to T231311 not being done yet. We can use the $disableTaintCheck hack for now, and finish it later.
  • The seccheck-specific job can be killed in CI for the master branch of all the upgraded repos. This can be done at the end of the upgrade for all repos together.

Related Objects

Event Timeline

Daimona created this task.Mar 26 2020, 9:43 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 26 2020, 9:43 PM
Jdforrester-WMF renamed this task from Upgrade phan to 0.10.0 to Upgrade phan to 0.10.0 and remove phan-taint-check-plugin.Mar 26 2020, 9:57 PM

Change 583696 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[labs/libraryupgrader@master] Remove phan-taint-check-plugin from extras if new mediawiki-phan-config present

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

Change 583774 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] build: Bump phan to 0.10.0, remove taint-check

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

Daimona renamed this task from Upgrade phan to 0.10.0 and remove phan-taint-check-plugin to Upgrade phan to 0.10.1 and remove phan-taint-check-plugin.Mar 26 2020, 11:33 PM

Ouch: https://integration.wikimedia.org/ci/job/mwext-php72-phan-docker/43938/console some are weird:

<file name="includes/AbuseFilterRunner.php">
  <error line="651" severity="warning" message="Variable $AbuseFilter is undeclared" source="PhanUndeclaredVariable"/>
$filterPublicComments = AbuseFilter::getFilter( $filter )->af_public_comments; // Line 651

I need to check what's going wrong. This might just be the new version of phan being buggy, or taint-check might be doing something very wrong.

Also, we'll have to exclude the "SecurityCheck-PHPSerializeInjection" warning, and also "SecurityCheck-LikelyFalsePositive" because they're very experimental.

Meh, it's taint-check messing up with phan. I'll take a look tomorrowish... Also:

Change 584122 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/tools/phan@master] Adjust taint-check settings, require new version

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

Legoktm added a subscriber: Legoktm.Apr 6 2020, 9:34 AM

@Daimona could you clarify what you want LibUp to do here? If we need to do something custom, example patches would be helpful for me.

Daimona changed the task status from Open to Stalled.Apr 6 2020, 9:38 AM

@Daimona could you clarify what you want LibUp to do here? If we need to do something custom, example patches would be helpful for me.

Yeah, I think we talked about this with James on IRC last week but I didn't follow up here on phab. More precisely, LibUp should:

  • Remove phan-taint-check-plugin' from the 'extra' key in composer.json, if present
  • Also remove the 'extra' key if it only contained taint-check
  • Bump mediawiki-phan-config to 0.10.1 as usual

An example patch is https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/AbuseFilter/+/583774/

However, we're not ready: there's a showstopper in taint-check which causes lots of false positives (see patch above). We need to fix T248742, release a new version of taint-check, release mediawiki-phan-config 0.10.2 with the new version of taint-check, then go straight to 0.10.2 with LibUp.

Jdforrester-WMF renamed this task from Upgrade phan to 0.10.1 and remove phan-taint-check-plugin to Upgrade phan to 0.10.2 and remove phan-taint-check-plugin.Apr 6 2020, 5:10 PM

(FTR, there are other showstoppers popping up. In addition to the subtasks I've created so far, I have to investigate a few warnings about array offsets, a PhanTypeMismatchArgument, and a few PhanUndeclaredMethod. Also, note that taint-check is more intensive and deeper than phan, so we may also get some new non-security true positives).

Change 584122 merged by jenkins-bot:
[mediawiki/tools/phan@master] Adjust taint-check settings, require new version

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

Daimona changed the task status from Stalled to Open.Apr 15 2020, 4:14 PM

Change 589051 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[mediawiki/extensions/CirrusSearch@master] build: Bump phan to 0.10.2, remove taint-check

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

Change 589052 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[mediawiki/extensions/Wikibase@master] build: Bump phan to 0.10.2, remove taint-check

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

Change 589053 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[mediawiki/extensions/VisualEditor@master] build: Bump phan to 0.10.2, remove taint-check

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

Change 589054 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[mediawiki/extensions/Linter@master] build: Bump phan to 0.10.2, remove taint-check

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

Change 589055 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[mediawiki/extensions/MassMessage@master] build: Bump phan to 0.10.2, remove taint-check

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

Change 589056 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[oojs/ui@master] build: Bump phan to 0.10.2

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

Change 589057 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[mediawiki/skins/MonoBook@master] build: Bump phan to 0.10.2, remove taint-check

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

Change 589062 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/CentralAuth@master] build: Bump phan to 0.10.2, remove taint-check

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

Change 589054 merged by jenkins-bot:
[mediawiki/extensions/Linter@master] build: Bump phan to 0.10.2, remove taint-check

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

Change 589057 merged by jenkins-bot:
[mediawiki/skins/MonoBook@master] build: Bump phan to 0.10.2, remove taint-check

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

Change 586317 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/core@master] Upgrade phan to 0.10.2

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

Change 589055 merged by jenkins-bot:
[mediawiki/extensions/MassMessage@master] build: Bump phan to 0.10.2, remove taint-check

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

Change 589056 merged by jenkins-bot:
[oojs/ui@master] build: Bump phan to 0.10.2

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

Change 589051 merged by jenkins-bot:
[mediawiki/extensions/CirrusSearch@master] build: Bump phan to 0.10.2, remove taint-check

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

Change 589053 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] build: Bump phan to 0.10.2, remove taint-check

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

Change 586317 merged by jenkins-bot:
[mediawiki/core@master] Upgrade phan to 0.10.2

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

Change 589052 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] build: Bump phan to 0.10.2, remove taint-check

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

Change 593819 had a related patch set uploaded (by VolkerE; owner: VolkerE):
[mediawiki/core@master] Update OOUI to v0.38.1

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

Change 593819 merged by jenkins-bot:
[mediawiki/core@master] Update OOUI to v0.38.1

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

Jdforrester-WMF triaged this task as Medium priority.May 13 2020, 5:21 PM

Change 601390 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[labs/libraryupgrader/config@master] Bump mediawiki/mediawiki-phan-config to 0.10.2

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

Change 601390 merged by jenkins-bot:
[labs/libraryupgrader/config@master] Bump mediawiki/mediawiki-phan-config to 0.10.2

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

Change 583696 merged by jenkins-bot:
[labs/libraryupgrader@master] Remove phan-taint-check-plugin from extra if new mediawiki-phan-config present

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

Change 601500 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[integration/config@master] layout: Drop the seccheck job from all repos with phan

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

Change 583774 abandoned by Daimona Eaytoy:
build: Bump phan to 0.10.2, remove taint-check

Reason:
Ib63be75df4bfdbd2c5b97de5f80dbec715108c01

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

Some extensions only using seccheck are still open: http://libraryupgrader2.wmflabs.org/library/composer/mediawiki/phan-taint-check-plugin
Some extras remain and will be cleaned up on next libup run

Change 601500 merged by jenkins-bot:
[integration/config@master] layout: Drop the seccheck job from all repos with phan

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

Jdforrester-WMF closed this task as Resolved.Jun 4 2020, 8:40 PM

Declaring this Resolved. Will continue work on final removal of seccheck elsewhere.