build: Verify the PHARs signature before using them#44
build: Verify the PHARs signature before using them#44
Conversation
|
ping @infection/core: it's a bit boring, but I would like us to make a decision on what is the canonical way we want to deal with PHARs so that I can update the main repo as well with that approach. |
could you please explain why it happens? So, do you say if I add some comment to a Makefile that all PHARs will be invalidated? Why?
I don't like it on phpspec adapter repo to be honest. It spams and I don't see why we need it for let's say PHP-CS-Fixer. I'm personally ok to update it even once a year and do 1 PR to fix all new issues (if any) or introduce a new rule. Otherwise I will be spammed and likely many of such PRs with PHP-CS-FIxer version upgrade will be failed if new rule is introduced. We already see it for PHPStan, Rector and I can't say I like it. I would avoid adding more notifications to my email, especially when it's daily, multiplied by many tools - it started to be annoying.
why would we want it?
also, why is it needed? |
Because of the rule: And it makes sense: the PHAR URL is configured in
Ah :/
Depending on the tool it is not as important I agree, it is one thing that is easier to configure with say dependabot (where it would be easy to tell it to check X every 2 weeks and Y every 6 months). That said, I configured the check to happen on every week, we could easily extend it. On the other hand if there is no level of automation, it is not an update once a week... We saw it fore both adapters. For PHPSpec the last update before I changed it was like 4-5 years? For Codeception it was 3 years ago... So whilst I agree with the sentiment of not wanting spams, I also don't feel it's healthy to leave it take dust so longer either as otherwise it easily gets to the point of you come over here to fix something or contribute (or a new contributor) and nothing works (this is not hypothetical, for both adapters both the pipeline, tests, e2e tests, Makefile – none were green; I'll say the codeception one was less efforts to get back green though).
Security: you download binary locally, it's good hygiene to check that you're getting what you thought. Traditionally for PHARs it has been done with GPG, but gh attestation is taking momentum. For example PIE is using it. It certainly more elegant than GPG + signing key see a complete example with Box.
I found it's an easy way to check that the PHAR actually works, it fastens the feedback loop as if it doesn't, you would otherwise have the error somewhere else and potentially a lot less explicit. For example https://github.com/jakzal/phpqa did that at some point as part of their automation and found out the Behat PHAR never worked... (and it was bad of Behat to never check that either but that's a different story). |
|
Looks like this was not exactly a new problem... infection/infection#852 |
There is a few issues with the way we use the PHAR tools currently:
Makefile(regardless of whether it's the "download PHAR section", we invalidate the PHARs triggering their reload.Makefile)Typically when working on the train this happened several times: either I had no connection or it wasn't stable and I couldn't use the make commands directly due to the PHARs being deleted and requiring to be downloaded again.
This PR is an attempt to fix this:
.tools/tool-name-version. This is what defines which version we used, and its easier to update. I experimented with an update workflow in the phpspec-adapter repository to do that. It will trigger regularly (it is configured as a cron) and will do a PR if necessary triggering the checks, mimicking the experience we would have with dependabot or equivalent..asc)gpgto verify the signingI must say I'm not happy with the gpg check: it is verbose and annoying. For reference, it is now possible to use gh attestation verify which makes it only need:
But not all tools switched to that yet.
Anyway this was more to report on the issue and see what it takes. So to recap, we have the following:
Makefileitself.--versioncheckWhich ones you're unhappy with and think we should remove?
I at the very least feel strongly about 1) and 5). For 3) I also feel strongly if gh attestation is available, I don't like the implementation for the GPG check so I'm fine skipping it although I don't like to run untrusted PHARs. 4. is also cheap, but i don't feel overly strongly about it.
Depending on your stance on this I'll update each repo accordingly.