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

Make secret value rejection on pipeline upload optional #1589

Merged
merged 2 commits into from
Mar 23, 2022

Conversation

moskyb
Copy link
Contributor

@moskyb moskyb commented Mar 22, 2022

🤔 Problem: In #1523, we made it so that the buildkite-agent pipeline upload command would reject any pipeline yaml that had sensitive values in it. We shipped this as part of v3.34. This is an awesome security feature, and prevents a bunch of footguns, but unfortunately it's a breaking change, and we accidentally shipped it as part of a minor release.

✅ Solution: This PR reverts default redaction behaviour to how it was previously, with some slight changes:

  • The newer behaviour (rejecting a pipeline upload if there's juicy secrets in it) is available with the --reject-secrets flag
  • Even when the --reject-secrets flag isn't used, the command issues a warning that:
    • There are unredacted secrets in the pipeline
    • and that default behaviour in agent v4 will be to reject these pipeline uploads

Slightly related, it makes the --dry-run flag for the pipeline upload command work with redaction.

Screenshots:
Without --reject-secrets:

Screen Shot 2022-03-22 at 2 59 50 PM

With --reject-secrets:

Screen Shot 2022-03-22 at 3 00 10 PM

These are from my local machine using the --dry-run flag, using this pipeline:

steps:
  - label: ":hammer: Example Script"
    command: "echo $COOL_PASSWORD"
    artifact_paths: "artifacts/*"
    agents:
      queue: "${BUILDKITE_AGENT_META_DATA_QUEUE:-default}"

Closes #1578
Closes PIP-309

@moskyb moskyb requested review from pda and eleanorakh March 22, 2022 02:02
@moskyb moskyb force-pushed the pipeline-upload-reject-secrets branch from bd36e0c to 684d9a5 Compare March 22, 2022 02:09
@moskyb
Copy link
Contributor Author

moskyb commented Mar 22, 2022

i've also tested that this works in a non-dry run environment, using an agent on my machine:

With --reject-secrets:

Screen Shot 2022-03-23 at 10 20 35 AM

Without --reject-secrets:

Screen Shot 2022-03-23 at 10 19 51 AM

clicommand/pipeline_upload.go Outdated Show resolved Hide resolved
clicommand/pipeline_upload.go Outdated Show resolved Hide resolved
clicommand/pipeline_upload.go Outdated Show resolved Hide resolved
clicommand/pipeline_upload.go Outdated Show resolved Hide resolved
Copy link
Member

@pda pda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good plan 👍🏼

clicommand/pipeline_upload.go Outdated Show resolved Hide resolved
redaction/redactor.go Show resolved Hide resolved
@moskyb moskyb requested review from pda and ticky March 23, 2022 01:22
Copy link
Member

@pda pda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good, up to you if you want to fix #1589 (comment)

@moskyb moskyb force-pushed the pipeline-upload-reject-secrets branch 2 times, most recently from 50c34d3 to bb17702 Compare March 23, 2022 01:45
@moskyb moskyb force-pushed the pipeline-upload-reject-secrets branch from bb17702 to 99ce182 Compare March 23, 2022 01:47
@moskyb moskyb merged commit 42053d7 into main Mar 23, 2022
@moskyb moskyb deleted the pipeline-upload-reject-secrets branch March 23, 2022 02:12
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

Successfully merging this pull request may close these issues.

Feature flag to reject pipelines with redacted variables
4 participants