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
| Status | Assigned | Task | ||
|---|---|---|---|---|
| Resolved | Daimona | T207344 Phan-taint-check-plugin not available for PHP > 7.0 | ||
| Resolved | sbassett | T227171 Release taint-check 2.0.0 | ||
| Resolved | Daimona | T216974 Update phan-taint-check-plugin to a newer phan (1.3.2) | ||
| Resolved | Daimona | T218721 Have CI run seccheck tests |
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)
@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.
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.
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.
@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.
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.