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

pack build not correctly treating trusted builders as trusted #2198

Closed
edmorley opened this issue Jul 3, 2024 · 1 comment · Fixed by #2205
Closed

pack build not correctly treating trusted builders as trusted #2198

edmorley opened this issue Jul 3, 2024 · 1 comment · Fixed by #2205
Labels
status/ready Issue ready to be worked on. type/bug Issue that reports an unexpected behaviour.
Milestone

Comments

@edmorley
Copy link
Contributor

edmorley commented Jul 3, 2024

Summary

The pack build command is not correctly treating builders as trusted after #2043. Specifically, if a builder is "trusted but not suggested" then pack build treats it as untrusted, even though the builder still shows up as trusted in the output of pack config trusted-builders.


Reproduction

Steps
  1. Update to latest Pack CLI (0.34.2)
  2. Remove the Pack CLI config (to reset any manually added trusted builders): rm ~/.pack/config.toml
  3. mkdir testcase && touch testcase/requirements.txt
  4. pack build --builder heroku/builder:22 --path testcase testapp --verbose
Current behavior

The heroku/builder:22 builder is treated as untrusted by pack build:

$ pack build --builder heroku/builder:22 --path testcase testapp --verbose
Builder heroku/builder:22 is untrusted
As a result, the phases of the lifecycle which require root access will be run in separate trusted ephemeral containers.
For more information, see https://medium.com/buildpacks/faster-more-secure-builds-with-pack-0-11-0-4d0c633ca619
Pulling image index.docker.io/heroku/builder:22
22: Pulling from heroku/builder
...
Expected behavior

For heroku/builder:22 to be treated as a trusted builder, given that the pack config trusted-builders Pack CLI command lists it under trusted builders:

$ pack config trusted-builders
Trusted Builders:
  gcr.io/buildpacks/builder:v1
  heroku/builder:20
  heroku/builder:22
  heroku/builder:24
...

...and that it's marked as trusted here:

Image: "heroku/builder:22",
DefaultDescription: "Ubuntu 22.04 AMD64 base image with buildpacks for Go, Java, Node.js, PHP, Python, Ruby & Scala.",
Suggested: false,
Trusted: true,

Notes

This appears to be caused by a bug here:

func isTrustedBuilder(cfg config.Config, builder string) bool {
for _, trustedBuilder := range cfg.TrustedBuilders {
if builder == trustedBuilder.Name {
return true
}
}
return isSuggestedBuilder(builder)
}

In that function, the return isSuggestedBuilder(builder) line should instead be return isTrustedBuilder(builder).

It looks like when the concept of "suggested" vs "trusted" builders was added in 1b68d12 some places in the codebase weren't updated along with the others.

This is why pack config trusted-builders says the builder is trusted, but pack build says it is not.

In addition to fixing this bug, it seems worth reducing the number of places that implement the same "is this a trusted builder" check to avoid issues like this (where the behaviour of different Pack subcommands for common functionality can diverge). Plus this comment needs updating too.

cc @colincasey @schneems


Environment

pack info
$ pack report
Pack:
  Version:  0.34.2+git-ce8db3c.build-6005
  OS/Arch:  darwin/arm64

Default Lifecycle Version:  0.19.6

Supported Platform APIs:  0.3, 0.4, 0.5, 0.6, 0.7, 0.8, 0.9, 0.10, 0.11, 0.12, 0.13

Config:
  default-builder-image = "[REDACTED]"
docker info

N/A

@edmorley edmorley added status/triage Issue or PR that requires contributor attention. type/bug Issue that reports an unexpected behaviour. labels Jul 3, 2024
@natalieparellano natalieparellano added status/ready Issue ready to be worked on. and removed status/triage Issue or PR that requires contributor attention. labels Jul 3, 2024
@natalieparellano natalieparellano added this to the 0.36.0 milestone Jul 3, 2024
@edmorley
Copy link
Contributor Author

edmorley commented Jul 3, 2024

I also think separately this error wording needs updating too:

// Builder is not in the trusted builder list
if len(existingTrustedBuilders) == len(cfg.TrustedBuilders) {
if isSuggestedBuilder(builder) {
// Attempted to untrust a suggested builder
return errors.Errorf("Builder %s is a suggested builder, and is trusted by default. Currently pack doesn't support making these builders untrusted", style.Symbol(builder))

(Since it's not only suggested builders that are trusted by default, but any of the "built-in trusted builders")

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/ready Issue ready to be worked on. type/bug Issue that reports an unexpected behaviour.
Projects
None yet
3 participants