Page MenuHomePhabricator

Phan-taint-check-plugin not available for PHP > 7.0
Closed, ResolvedPublic

Description

In an effort to investigate T207343 I tried to install phan-taint-check-plugin for the first time locally, by running composer require --dev mediawiki/phan-taint-check-plugin and I got the following error message:

[InvalidArgumentException]                                                 
Could not find package mediawiki/phan-taint-check-plugin at any version matching your PHP version 7.1.17.0

Event Timeline

Huji created this task.Oct 17 2018, 10:49 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 17 2018, 10:49 PM
Huji renamed this task from Phan not available for PHP 7.1.17 to Phan-taint-check-plugin not available for PHP 7.1.17.Oct 17 2018, 10:49 PM
Huji added a comment.Oct 27 2018, 1:17 PM

I updated the git working copy of SecurePoll and reran the above and now I get this error message:

[InvalidArgumentException]                                                                            
  Package mediawiki/phan-taint-check-plugin at version  has a PHP requirement incompatible with your PHP version (7.2.10)

While the primary goal of this task to help me figure out how to install phan :) I think it is worth noting that it says ... at version has a PHP requirement ... (note that version is missing and this needs to be fixed too)

Huji added a subscriber: Reedy.Oct 27 2018, 1:18 PM

@Reedy can I ask you to kindly take a look at this?

It currently has a hard dependency on php7.0

Daimona renamed this task from Phan-taint-check-plugin not available for PHP 7.1.17 to Phan-taint-check-plugin not available for PHP > 7.0.Mar 18 2019, 11:05 AM

@Bawolff Is there a specific reason to require PHP 7.0.0, or it's just because the plugin is untested with other versions?

@Daimona I believe it's just a matter of being developed against a specific version of phan and php-ast. It could stand to be updated, but @Bawolff is the only maintainer right now (I'm trying to get up to speed to help out) so it's slow going. Regardless, the CI docker image builds out the dependencies nicely and the sec-check plugin _should_ be able to run against any PHP 7.x code. I've got a local, modified version of it that I run manually against codebases (along with some other automated security tools) when I perform security audits.

@sbassett Thanks for the reply. I'm trying to understand how phan, AST etc. work to see if I can start bumping the phan version. For now I'm trying to get to phan ^0.8 and ast ^0.1.5, although it won't be fast and I cannot guarantee anything.

@Daimona I've played around with version-bumping minimally without any success. Specific versions of phan and php-ast seem fairly tightly coupled, unfortunately. Good luck!

yeah, its tied pretty heavily to phan 0.8, which in turn is tied to php 7. There's an upcoming goal to move it the plugin to a modern version of phan.

@sbassett Well, actually I'm facing several breaking changes even with 0.8.0 => 0.8.13. The most important is the addition of UnaddressableTypedElement (which also doesn't have a true context but just a FileRef), which breaks several things in TaintednessBaseVisitor. More specifically, I'm talking about this commit.

I'm giving up with the upgrade. Phan doesn't comply with semver at all, and thus as I was saying above you'll face plenty of breaking changes even for x.x.y => x.x.z upgrades. At this point, I think seccheck needs a major rewrite in order to work with 1.2.6. If instead, you want to do it gradually, I suggest bumping to 0.9.6 first (which is roughly the same as 0.8.13), then switch to PluginV2 and slowly move on to 1.2.6.

Daimona claimed this task.May 13 2019, 5:42 PM

This is essentially the same as the child task, given that the strict PHP requirement is imposed by the old version of phan. Moreover, one could force-install seccheck via --ignore-platform-reqs.

Jdforrester-WMF added a subscriber: Jdforrester-WMF.

This is now the last remaining blocker to moving CI to newer versions of PHP (and dropping PHP support in MediaWiki). Prioritisation would be appreciated.

As I probably wrote in the child task (which is almost a duplicate of this one), the last patch to merge before releasing 2.0 is https://gerrit.wikimedia.org/r/#/c/mediawiki/tools/phan/SecurityCheckPlugin/+/507619/ (and the ~15 patches it depends on). That version will be production-ready and it will support php ^7.0.0

Oh, and all that code has been written into a feature branch so jenkins is blind to it. Fun. :-(

Yes, that's because intermediate versions have some incompatibilities. The first working version is the one above anyway, so it wouldn't be possible to use the 2.x branch in a job, not even experimental.

sbassett triaged this task as Normal priority.EditedJun 21 2019, 2:27 PM

@Daimona @Jdforrester-WMF - apologies for the disappearing act from the Security-Team on this. @Bawolff and I have been working through some of the outstanding patch sets in Gerrit for the plugin and hope to make good progress on them this week and next. I'm optimistic we can have a proper 2.x release once that work is completed and have it ready for CI shortly after that. I'll plan to provide another update here next week.

A bigger picture note: the Security-Team is still working through some internal issues around our stewardship of the plugin. Within the coming weeks, there's a decent chance I'll be assuming most/all stewardship-related duties, though that is yet to be officially decided. We appreciate the patience.

Thank you! I'm happy to help in whatever way I can.

Daimona closed this task as Resolved.Jul 4 2019, 4:49 PM

2.0.0 was released and is PHP70+ compatible. We still have to set up CI and do some final testing, but it's already usable.

sbassett moved this task from Backlog to Done on the Security-Team board.Mon, Jul 22, 1:47 PM