Merge pull request #1589 from buildkite/pipeline-upload-reject-secrets

Make secret value rejection on pipeline upload optional

clicommand/pipeline_upload.go

@@ -57,6 +57,7 @@ type PipelineUploadConfig struct {
 	DryRun          bool     `cli:"dry-run"`
 	NoInterpolation bool     `cli:"no-interpolation"`
 	RedactedVars    []string `cli:"redacted-vars" normalize:"list"`
+	RejectSecrets   bool     `cli:"reject-secrets"`
 
 	// Global flags
 	Debug       bool     `cli:"debug"`
@@ -73,7 +74,7 @@ type PipelineUploadConfig struct {
 
 var PipelineUploadCommand = cli.Command{
 	Name:        "upload",
-	Usage:       "Uploads a description of a build pipeline adds it to the currently running build after the current job.",
+	Usage:       "Uploads a description of a build pipeline adds it to the currently running build after the current job",
 	Description: PipelineUploadHelpDescription,
 	Flags: []cli.Flag{
 		cli.BoolFlag{
@@ -97,6 +98,11 @@ var PipelineUploadCommand = cli.Command{
 			Usage:  "Skip variable interpolation the pipeline when uploaded",
 			EnvVar: "BUILDKITE_PIPELINE_NO_INTERPOLATION",
 		},
+		cli.BoolFlag{
+			Name:   "reject-secrets",
+			Usage:  "When true, fail the pipeline upload early if the the pipeline contains secrets",
+			EnvVar: "BUILDKITE_AGENT_PIPELINE_UPLOAD_REJECT_SECRETS",
+		},
 
 		// API Flags
 		AgentAccessTokenFlag,
@@ -227,20 +233,6 @@ var PipelineUploadCommand = cli.Command{
 			l.Fatal("Pipeline parsing of \"%s\" failed (%s)", src, err)
 		}
 
-		// In dry-run mode we just output the generated pipeline to stdout
-		if cfg.DryRun {
-			enc := json.NewEncoder(os.Stdout)
-			enc.SetIndent("", "  ")
-
-			// Dump json indented to stdout. All logging happens to stderr
-			// this can be used with other tools to get interpolated json
-			if err := enc.Encode(result); err != nil {
-				l.Fatal("%#v", err)
-			}
-
-			return
-		}
-
 		if len(cfg.RedactedVars) > 0 {
 			needles := redaction.GetKeyValuesToRedact(shell.StderrLogger, cfg.RedactedVars, env.FromSlice(os.Environ()).ToMap())
 			serialisedPipeline, err := result.MarshalJSON()
@@ -251,11 +243,36 @@ var PipelineUploadCommand = cli.Command{
 
 			stringifiedserialisedPipeline := string(serialisedPipeline)
 
+			secretsFound := make([]string, 0, len(needles))
 			for needleKey, needle := range needles {
 				if strings.Contains(stringifiedserialisedPipeline, needle) {
-					l.Fatal("Couldn’t upload the %q pipeline. Refusing to upload pipeline containing the value interpolated from the %q environment variable. Ensure your pipeline does not include secret values or interpolated secret values.", src, needleKey)
+					secretsFound = append(secretsFound, needleKey)
 				}
 			}
+
+			if len(secretsFound) > 0 {
+				if cfg.RejectSecrets {
+					l.Fatal("Pipeline %q contains values interpolated from the following secret environment variables: %v, and cannot be uploaded to Buildkite", src, secretsFound)
+				} else {
+					l.Warn("Pipeline %q contains values interpolated from the following secret environment variables: %v, which could leak sensitive information into the Buildkite UI.", src, secretsFound)
+					l.Warn("This pipeline will still be uploaded, but if you'd like to to prevent this from happening, you can use the `--reject-secrets` cli flag, or the `BUILDKITE_AGENT_PIPELINE_UPLOAD_REJECT_SECRETS` environment variable, which will make the `buildkite-agent pipeline upload` command fail if it finds secrets in the pipeline.")
+					l.Warn("The behaviour in the above flags will become default in Buildkite Agent v4")
+				}
+			}
+		}
+
+		// In dry-run mode we just output the generated pipeline to stdout
+		if cfg.DryRun {
+			enc := json.NewEncoder(os.Stdout)
+			enc.SetIndent("", "  ")
+
+			// Dump json indented to stdout. All logging happens to stderr
+			// this can be used with other tools to get interpolated json
+			if err := enc.Encode(result); err != nil {
+				l.Fatal("%#v", err)
+			}
+
+			return
 		}
 
 		// Check we have a job id set if not in dry run

redaction/redactor.go

@@ -321,7 +321,7 @@ func GetKeyValuesToRedact(logger shell.Logger, patterns []string, environment ma
 
 			if matched {
 				if len(varValue) < RedactLengthMin {
-					logger.Warningf("Value of %s below minimum length and will not be redacted", varName)
+					logger.Warningf("Value of %s below minimum length (%d bytes) and will not be redacted", varName, RedactLengthMin)
 				} else {
 					valuesToRedact[varName] = varValue
 				}