Skip to content

build: Verify the PHARs signature before using them#44

Open
theofidry wants to merge 1 commit intomasterfrom
chore/php-cs-fixer-dev
Open

build: Verify the PHARs signature before using them#44
theofidry wants to merge 1 commit intomasterfrom
chore/php-cs-fixer-dev

Conversation

@theofidry
Copy link
Member

@theofidry theofidry commented Jan 5, 2026

There is a few issues with the way we use the PHAR tools currently:

  • We do not verify them before using them. Neither the signature, neither if they work at all (i.e. were correctly downloaded, or maybe the shipped PHAR is crap).
  • Whenever we change the Makefile (regardless of whether it's the "download PHAR section", we invalidate the PHARs triggering their reload.
  • It is hard to update (we need to constantly update the URL in 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:

  • We define the version of the tool in .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.
  • the makefile:
    • downloads the PHP-CS-Fixer PHAR and its signature (.asc)
    • use gpg to verify the signing
    • makes it executable and tries to display the version as a smoke test
    • moves it to the desired location

I 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:

wget ...
gh attestation verify --owner php pie.phar

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:

  1. Having to define the PHAR version in a separate file and use that to know if the PHAR needs to be downloaded again or not instead of the Makefile itself.
  2. Having the download + any verification done in a separate directly, so that if it fails and you had a working PHAR, it does not get replaced.
  3. Having the GPG/gh attestation verify check
  4. Having the --version check
  5. Having the update workflow to keep the PHARs up to date.
  6. Having a composite GitHub Action to cache the PHAR in the CI.

Which 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.

@theofidry theofidry self-assigned this Jan 5, 2026
@theofidry
Copy link
Member Author

theofidry commented Jan 8, 2026

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.

@maks-rafalko
Copy link
Member

maks-rafalko commented Jan 10, 2026

  • Whenever we change the Makefile (regardless of whether it's the "download PHAR section", we invalidate the PHARs triggering their reload.

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?

  1. Having the update workflow to keep the PHARs up to date.

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.

Having the GPG/gh attestation verify check

why would we want it?

Having the --version check

also, why is it needed?

@theofidry
Copy link
Member Author

theofidry commented Jan 10, 2026

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?

Because of the rule:

$(INFECTION): Makefile

And it makes sense: the PHAR URL is configured in Makefile so without this there would be no way to know the PHAR URL changed (so long as the URL/version stays in Makefile).

I don't like it on phpspec adapter repo to be honest.

Ah :/

It spams and I don't see why we need it for let's say PHP-CS-Fixer.

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).

Having the GPG/gh attestation verify check

why would we want it?

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.

Having the --version check

also, why is it needed?

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).

@theofidry
Copy link
Member Author

Looks like this was not exactly a new problem... infection/infection#852

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants