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

Next major version should support the 99% case #260

Open
keithpitt opened this issue Dec 29, 2023 · 1 comment
Open

Next major version should support the 99% case #260

keithpitt opened this issue Dec 29, 2023 · 1 comment

Comments

@keithpitt
Copy link

I'd like us to revisit the decision made here: #164

While I 100% agree that having propagate-environment on by default is somewhat a security issue, I think the point of this plugin is that it makes the 99% case easy mode.

All of the steps I've been writing lately have had both of these options turned on:

steps:
  - command: "ruby my-script.rb"
    plugins:
      - docker#v5.4.0:
          image: "ruby:latest"
          propagate-environment: true #<-- this
          mount-buildkite-agent: true #<-- and this

I think if folks want to go down a "secure docker" route, they have the option to call out to docker directly. But I think we should optimise this plugin for "everything just works" by default.

Are there any other options that we should consider flipping the defaults for?

@toote
Copy link
Contributor

toote commented Dec 29, 2023

My opinion has always been: security by default, that way you would have to (ideally) read the documentation, understand the risks and opt-in to a less secure execution. Realistically, though, I know that most just add options until things work (the equivalent of click through OK without readying the messages) 🤷 .

I believe that the propagate-environment is not that much of an issue as the shared values are not secrets. The mounting of the buildkite agent is definitely not secure as that would share your agent's token to the container by default. I am pretty sure that such a thing is on-par with the arbitrary code execution implications of the options to expand variables (expand-image-vars and expand-volume-vars).

That said, I can think of two alternatives to avoid code repetition:
1- setting BUILDKITE_PLUGIN_DOCKER_PROPAGATE_ENVIRONMENT and BUILDKITE_PLUGIN_DOCKER_MOUNT_BUILDKITE_AGENT as environment variables at the pipeline level
2- YAML anchors and merge tags so that the shared (or base) configuration for the plugin is only specified once and then re-used and shared in all steps

We should probably add some documentation about those alternatives in the meantime while we continue to discuss how to best tackle this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants