Retry JS verification with approved version's lock files on mismatch

[potiuk]

When the initial JS build verification fails and a previously approved version exists, retry the Docker build using the approved commit's package.json and lock files. This handles the common case where a version bump updates devDependencies (e.g. rollup, ncc) but the committed dist/ was built with the old toolchain.

If the retry matches, the result is reported as a warning ("matches with approved lock files, devDeps changed") rather than a hard failure, with guidance to review the devDependency changes before approving.

Generated-by: Claude Opus 4.6 (1M context) noreply@anthropic.com

[potiuk]

This one addresees #669 -> I am also opening PR to address it in SonarSource as apparently they build the .javascript with version of dev tooling from the previous version.

[dave2wave]

Looks reasonable to me, but wait for another review

[potiuk]

Rebased after merging the split refactor. CC: @raboof -> that is a weird case but it happened already once that action's javascript was actually built before the dependencies from previous run where upgraded - resulting in non-consistent javascript (but it actually shows that our javascript reproducibility checks are good because essentially anyone could commit any modification to compiled javascript - this one is at least reproducible with previously approved version of dependencies - so it is somewhat reproducible.

The funny thing - I would have probably never guessed that this happened, it was entirely figured out by Claude - I just asked it - what's wrong with this PR and can you produce a fix - and it did - with very reasonable explanation :)

[potiuk]

Also I asked Claude - and it diagnosed the Sonar Qube action code and produced a fix that I submitted as PR SonarSource/sonarqube-scan-action#228

[dave2wave]

@potiuk So if someone is building their action in this way is this not a bug on their part and we should reject this action completely? Seems like they are producing a product that were it an ASF release it would be rejected.

[potiuk]

@potiuk So if someone is building their action in this way is this not a bug on their part and we should reject this action completely? Seems like they are producing a product that were it an ASF release it would be rejected.

We might - yes - depending on our criteria and goals

• if we care about "correctness" - yes, we should reject it
• if we care anout makicng it ready for people who want to do it fast and to only check safety - no, we should not

We know it's a bit sloppy but should be safe (because it is reproducible). Especially if it is followed (which is what I did) with an issue or PR fixing it - this is I think the best approach - when we notice an issue and let the third party know so they fix it, and then continue with a workaround (which this check really is).

They might not be super sloppy, they might made an innocent mistake in their pipeline (I think this is what happened in this case). I think we should be quite careful what we "punish" by not accepting actions - because we essentially punish our PMCs. And I find it much more open-source friendly to work with our dependencies rather than reject them because they made a mistake - and even do not tell them that they did. If we don't tell them, they might not ever be aware of it.

This is also folllowing what Astral team explained in the https://astral.sh/blog/open-source-security-at-astral astral, we could have similar approach as them). They worked with they downstream dependencies to make sure all their composite dependencies are hashed - which likely should also be our criteria - I was thinking about adding such criteria. This might be a good reason for rejection (because of security)

My personal goal with current way we handle actions (and this is why I developed verify_build_actions.p) was to unblock our PMCs as soon as possible, while making sure we are safe. I did not even have the "licence" criteria in mind (this is why I missed it when reviewing Gradle, but that is another one to consider, if it should be our review criteria as well).

If we would like to add "correctness" of building the action and reject those actions based on it, we can - but It was not my primary concern so far, and for me "freeing" actions so that our PMCs can upgrade to latest "safe" ones as fast as possible (minus cooldown for security reasons) was the primary criteria (and the only so far).

What's your criteria for accepting / rejecting the actions @dave2wave - maybe we have different ones?

[potiuk]

I created #686 -> maybe a discussion can be held there.