buildpacks / pack

CLI for building apps using Cloud Native Buildpacks
https://buildpacks.io
Apache License 2.0
2.47k stars 278 forks source link

pack build not correctly treating trusted builders as trusted #2198

Open edmorley opened 5 days ago

edmorley commented 5 days ago

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: https://github.com/buildpacks/pack/blob/ce8db3c5d11b8e040fd565632668a697e53e4f87/internal/builder/known_builder.go#L28-L31

Notes

This appears to be caused by a bug here: https://github.com/buildpacks/pack/blob/ce8db3c5d11b8e040fd565632668a697e53e4f87/internal/commands/commands.go#L108-L116

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 https://github.com/buildpacks/pack/commit/1b68d1261eb8e8f01aade2737d5268e54f4a4604 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 commented 5 days ago

I also think separately this error wording needs updating too: https://github.com/buildpacks/pack/blob/ce8db3c5d11b8e040fd565632668a697e53e4f87/internal/commands/config_trusted_builder.go#L81-L85

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