Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the "Python security update available" version check #1569

Merged
merged 1 commit into from
Apr 18, 2024

Conversation

edmorley
Copy link
Member

@edmorley edmorley commented Apr 18, 2024

Previously the "Python security update is available" message was shown if the requested Python version does not exactly equal what the buildpack believes to be the latest Python patch version for that major release.

However, this means that the message is then incorrectly shown if the current version is actually newer than the latest version the buildpack knows about.

This scenario can occur:
(a) During the small time window when a new Python version has been built and uploaded to S3 but the buildpack updates have not yet been released.
(b) If an app pins to an older buildpack version but manually requests a newer Python patch release.

It's not possible to add an integration test for this specific case, since using a fake future version (like Python 3.12.999) will fail prior to the version check due to it not existing on S3. (The scenario being fixed is effectively a race condition that we can't emulate.)

However, the security version numbers are tested in general:
https://github.com/heroku/heroku-buildpack-python/blob/main/spec/hatchet/python_update_warning_spec.rb

GUS-W-15541305.

@edmorley edmorley added the bug label Apr 18, 2024
@edmorley edmorley self-assigned this Apr 18, 2024
@edmorley edmorley marked this pull request as ready for review April 18, 2024 09:15
@edmorley edmorley requested a review from a team as a code owner April 18, 2024 09:15
@edmorley edmorley enabled auto-merge (squash) April 18, 2024 09:15
@edmorley edmorley changed the title Fix the "security update available" version check Fix the "Python security update available" version check Apr 18, 2024
@edmorley edmorley disabled auto-merge April 18, 2024 09:17
@edmorley edmorley force-pushed the fix-patch-update-version-comparison branch from 9752d4d to 403de41 Compare April 18, 2024 14:05
Previously the "Python security update is available" message was shown
if the requested Python version does not exactly equal what the
buildpack believes to be the latest Python patch version for that major
release.

However, this means that the message is then incorrectly shown if the
current version is actually newer than the latest version the buildpack
knows about.

This scenario can occur:
(a) During the small time window when a new Python version has been
    built and uploaded to S3 but the buildpack updates have not yet
    been released.
(b) If an app pins to an older buildpack version and then later
    updates to a new Python patch release.

It's not possible to add an integration test for this specific case, since
using a fake future version (like Python 3.12.999) will fail prior to the
version check due to it not existing on S3. (The scenario being fixed is
effectively a race condition that we can't emulate.)

However, the security version numbers are tested in general:
https://github.com/heroku/heroku-buildpack-python/blob/main/spec/hatchet/python_update_warning_spec.rb

GUS-W-15541305.
@edmorley edmorley force-pushed the fix-patch-update-version-comparison branch from 403de41 to b4fa786 Compare April 18, 2024 14:06
@edmorley edmorley enabled auto-merge (squash) April 18, 2024 14:06
Copy link
Contributor

@dzuelke dzuelke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just as a general note, I think this check will fail if we ever decide to throw alpha, beta, RC versions into the mix.

@edmorley
Copy link
Member Author

edmorley commented Apr 18, 2024

Just as a general note, I think this check will fail if we ever decide to throw alpha, beta, RC versions into the mix.

We don't support these for Python. And if we ever added support, we'd add a test for these versions, which would catch this. (Also in the event we ever add support in the future, it will likely be for the CNB only, which will have switched to the manifest by then, and also won't be using a bash implementation for version comparison.)

@edmorley edmorley merged commit a9bcb10 into main Apr 18, 2024
5 checks passed
@edmorley edmorley deleted the fix-patch-update-version-comparison branch April 18, 2024 15:43
@heroku-linguist heroku-linguist bot mentioned this pull request Apr 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants