-
Notifications
You must be signed in to change notification settings - Fork 115
Ensure read access to the run image selected by extensions #1364
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
Ensure read access to the run image selected by extensions #1364
Conversation
7032ccb to
a1dc432
Compare
Co-authored-by: Nicolas Bender <nicolas.bender@sap.com> Signed-off-by: Pavel Busko <pavel.busko@sap.com> Co-authored-by: Pavel Busko <pavel.busko@sap.com>
a1dc432 to
499f875
Compare
|
@jabrown85 With your approval, what's missing to get it merged and released? Do we need @natalieparellano's approval in addition? |
|
@loewenstein-sap yeah - I think I'd like a second set of eyes on this. What comes to mind now is that this phase may need registry auth, which isn't typically a thing in the untrusted phase executions. There are other registry related settings like |
This is true. Can we move this check to the restore phase? |
Signed-off-by: Pavel Busko <pavel.busko@sap.com>
67cf4bc to
f4f38a2
Compare
|
the check has been moved to the restore phase, however I couldn't find any unit tests for the file, and the acceptance tests are using docker daemon only, which makes it hard to test without mock registry. |
|
#1338 would have made this PR much easier to test :) unfortunately, that PR is blocked on me having some bandwidth to add units. Apologies for being slow here, I've just returned from vacation. I'll take a look today. |
cmd/lifecycle/restorer.go
Outdated
| cli.FlagBuildImage(&r.BuildImageRef) | ||
| } | ||
| cli.FlagAnalyzedPath(&r.AnalyzedPath) | ||
| cli.FlagRunPath(&r.RunPath) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am sorry to raise the specter of another Platform API version bump, but I think we need to gate this on Platform 0.14 also (an raise another spec PR).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
b1a2cce to
af38ac5
Compare
Signed-off-by: Pavel Busko <pavel.busko@sap.com>
af38ac5 to
c252f36
Compare
|
I may have been too hasty in merging this one and #1346, as we're due for a go version / dependencies patch and this should be in the next minor... |
This implements the missing feature from Platform API 0.14 where the restorer should accept a -run flag to enable read access validation for run images selected by extensions during the restore phase. When extensions switch the run image to one listed in run.toml, the restorer needs to verify accessibility using the platform's authentication context (CNB_REGISTRY_AUTH). This prevents builds from proceeding with images the system cannot actually access. Changes: - Add -run flag to restorer when Platform API >= 0.14 - Write run.toml file via WriteRunToml operation - Add tests verifying flag is present for Platform API >= 0.14 - Add tests verifying flag is absent for Platform API < 0.14 Fixes #2515 References: - Spec PR: buildpacks/spec#408 - Lifecycle PR: buildpacks/lifecycle#1364 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This implements the missing feature from Platform API 0.14 where the restorer should accept a -run flag to enable read access validation for run images selected by extensions during the restore phase. When extensions switch the run image to one listed in run.toml, the restorer needs to verify accessibility using the platform's authentication context (CNB_REGISTRY_AUTH). This prevents builds from proceeding with images the system cannot actually access. Changes: - Add -run flag to restorer when Platform API >= 0.14 - Write run.toml file via WriteRunToml operation - Add tests verifying flag is present for Platform API >= 0.14 - Add tests verifying flag is absent for Platform API < 0.14 Fixes #2515 References: - Spec PR: buildpacks/spec#408 - Lifecycle PR: buildpacks/lifecycle#1364 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Juan Bustamante <bustamantejj@gmail.com>
Summary
When an extension switches the run image to one listed in the
run.toml, the read access check is not triggered, as it is done in the analyze phase. This change will iterate over the requested run image and it's mirrors, to select the first accessible one.Release notes
Related
Resolves #___
Context