Weird dependabot update for for `pypa/cibuildwheel` v2.23.2

[ogrisel]

Description

We just observed that, a few days ago, dependabot updated our GHA workflow unexpectedly as following:

https://github.com/scikit-learn/scikit-learn/pull/31125/files

Note that it changed the commit hash for v2.23.2 which was previously correct for another commit (without changing the version number): it changed the commit hash to 6c426a3 instead of keeping the correct d04cacb.

As this behavior (bug) is quite concerning from a security point of view, we were wondering what could explain this. Did you re-push the v2.23.2 tag several times?

We manually undid this change today to fix the problem here:

scikit-learn/scikit-learn#31145

[henryiii]

That commit is over a week newer than the release itself, so while @joerick did the release, I'm pretty sure he didn't set the v2.23.2 tag to a commit that hadn't been written yet. That's not even on the right branch, that includes all the v3 work. That looks a bit more like a dependabot bug.

[henryiii]

As for the failing build, cibuildwheel 3.0 changes the way it runs tests. You really should use a src folder (notice it is broken locally too!), but if you don't, you can use the #2062 test-sources to ensure you don't import files that should not be importable. The cd technique would not support iOS/Android, and had issues even with regular packages, and complicated running tests. You could set PYTHONSAFEPATH for Python 3.11+ (I wonder if we should do that?)

(I think this was expected, though I wasn't following this change closely.)

[henryiii]

@joerick would setting PYTHONSAFEPATH makes sense for tests? That would only help 3.11+, but SPEC 0 packages are 3.11+ already and they tend to be old packages that didn't use src folders and don't want to change. Maybe that would be too invasive, but it seems like a possible idea.

Edit: even though scikit-learn is listed on SPEC 0, they still support 3.10+. Though if they are actually still on the NEP 29 schedule instead, today is the drop day for 3.10.

[joerick]

We just observed that, a few days ago, dependabot updated our GHA workflow unexpectedly as following:

https://github.com/scikit-learn/scikit-learn/pull/31125/files

Note that it changed the commit hash for v2.23.2 which was previously correct for another commit (without changing the version number): it changed the commit hash to 6c426a3 instead of keeping the correct d04cacb.

As this behavior (bug) is quite concerning from a security point of view, we were wondering what could explain this. Did you re-push the v2.23.2 tag several times?

This is indeed, very concerning. I can confirm that I did not touch that tag or release after pushing it a couple of weeks ago.

I suppose it's possible the tag moved in the meantime, but I don't know of a way to check that.

It seems to me this is likely an issue with dependabot. Mislabeling commit hashes is unacceptable from a security point of view. Perhaps a Github admin could verify that the tag never moved when they look into this bug.

[lesteve]

Maybe I got too fancy with @dependabot <command> without understanding in detail the consequence. I first did @dependabot rebase and because it did not seem enough (I don't remember why exactly) a @dependabot recreate.

You can see that @dependabot recreate force-pushed the change and replaced the right commit hash for 2.23.2 55bd91a (no the right commit hash for 2.23.2 is d04cacb) with the wrong one (likely the one from main at the time) 6c426a3, see the comparison of the force-push:
https://github.com/scikit-learn/scikit-learn/compare/c1b2336761e2960198e27ff70e4199bc526ee48c..b39696ac9bdcc2851c07a79304c2cbc89c149474

[lesteve]

Actually sorry for the noise, I was partly wrong in my previous message, I need to look into it more ...

[lesteve]

I looked at it a little bit more and I think actually dependabot updated the commit hash with a commit hash from the main branch in https://github.com/scikit-learn/scikit-learn/pull/31125/files, for some unknown reason.

I tried to reproduce a similar behaviour in a test repo and was not able to reproduce, the dependabot PR seems fine.

Here are the dependabot build logs, in case there is some useful information there: original dependabot PR log, build log after the @dependabot rebase.

[ogrisel]

Thanks, @lesteve, for your reproduction attempt. Since we cannot reproduce the bug, I am not sure what else we can do, w.r.t. the original dependabot problem.

Shall we close?

The failure to build scikit-learn wheels with cibuildwheel main will probably be addressed on the scikit-learn side when we need it (e.g. via the cd trick as long as we support Python 3.10, or maybe via reorganizing the source tree to use the recommended src/ subfolder but that might be disruptive for some people / workflows).

[henryiii]

You can also use pytest instead of python -m pytest, though IIRC there was an issue doing that for Pyodide. Also, you don't need to cd, you can use the new test-sources once 3.0 comes out.

[joerick]

Looks a lot like a dependabot issue. Closing from cibuildwheel's point of view.