Reject pipeline uploads containing redacted vars#1523
Conversation
Co-Authored-By: Eleanor <eleanor@buildkite.com> Co-Authored-By: Paula <paula@buildkite.com>
Co-Authored-By: Eleanor <eleanor@buildkite.com> Co-Authored-By: Paula <paula@buildkite.com>
Co-Authored-By: Eleanor <eleanor@buildkite.com> Co-Authored-By: Paula <paula@buildkite.com>
Co-Authored-By: Eleanor <eleanor@buildkite.com> Co-Authored-By: Paula <paula@buildkite.com>
a4dd9e5 to
01ef8d9
Compare
01ef8d9 to
6da0424
Compare
6da0424 to
7e10854
Compare
keithduncan
left a comment
There was a problem hiding this comment.
@eleanorakh I’ve taken another look at this and had some ideas about error messages before we merge, let me know what you think 🙇 😃
| serialisedPipeline, err := result.MarshalJSON() | ||
|
|
||
| if err != nil { | ||
| l.Fatal("Pipeline serialization of \"%s\" failed (%s)", src, err) |
There was a problem hiding this comment.
What do you think about tweaking this error to say something like:
"Couldn’t scan the %q pipeline for redacted variables. This parsed pipeline could not be serialized, ensure the pipeline YAML is valid, or disable secret redaction for this upload by passing --redacted-vars=''. (%s)", src, err
There was a problem hiding this comment.
Yea, I like that. It's hitting the three points of what makes a good error message. I'll update! :)
|
|
||
| for _, needle := range needles { | ||
| if strings.Contains(stringifiedserialisedPipeline, needle) { | ||
| l.Fatal("Couldn't upload %q pipeline. Refusing to upload pipeline containing redacted vars. Ensure your pipeline does not include secret values or interpolated secret values", src) |
There was a problem hiding this comment.
I did wonder about whether GetValuesToRedact could return tuples of (env_key, env_value) so that this error could say "Whoops your pipeline interpolated the value of the $env_key" and point folks closer to the answer. What do you think?
There was a problem hiding this comment.
Yea, a bit more specificity never hurts!
How about this?
Couldn't upload %q pipeline. Refusing to upload pipeline containing interpolated values of the $env_key. Ensure your pipeline does not include secret values or interpolated secret values
There was a problem hiding this comment.
Yeah I think that would be great 😄
|
Here’s how this looks when your pipeline interpolates one of the environment variables matched by the |
Work in progress with @eleanorakh and @pzeballos to catch and prevent pipeline uploads from including interpolated redacted vars.
Fixes #1469