Skip to content

Trim leading and trailing whitespace when setting secrets from stdin#5086

Merged
samcoe merged 2 commits intotrunkfrom
trim-whitespace
Jan 29, 2022
Merged

Trim leading and trailing whitespace when setting secrets from stdin#5086
samcoe merged 2 commits intotrunkfrom
trim-whitespace

Conversation

@samcoe
Copy link
Contributor

@samcoe samcoe commented Jan 24, 2022

This PR changes the behavior of the secret set command to trim leading and trailing whitespace from secrets that are input from stdin. The issue this solves only mentions trailing whitespace but I could not think of a reason to not trim leading whitespace as well.

closes #5031

@samcoe samcoe requested a review from a team as a code owner January 24, 2022 09:11
@samcoe samcoe requested review from vilmibm and removed request for a team January 24, 2022 09:11
@samcoe samcoe self-assigned this Jan 24, 2022
}

return body, nil
return bytes.TrimSpace(body), nil
Copy link
Contributor

@mislav mislav Jan 24, 2022

Choose a reason for hiding this comment

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

Since we're trying to mitigate a problem that only happens when using stdin, and is caused by stdin being typically followed by a newline character, could we scope the solution to only those conditions?

  1. Only trim when reading from stdin;
  2. Only trim a single trailing \r?\n and nothing else.

I don't know of realistic scenarios in which preserving all surrounding whitespace will be necessary, but I would like us to avoid doing unnecessary undocumented transformations on the user's input. For example, if someone uses --body "..." to pass a value, I don't see why we should ever change a single byte of that value. With the <<<"..." syntax, however, if the shell magically adds a newline then we can compensate for that by stripping it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup agreed on both points. The location of the current fix is limited to just stdin input so I think that is covered but I will pair it down to just the trailing newline characters.

@samcoe samcoe requested a review from mislav January 24, 2022 20:28
Copy link
Contributor

@mislav mislav left a comment

Choose a reason for hiding this comment

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

Fantastic.

@samcoe samcoe merged commit 3be4b99 into trunk Jan 29, 2022
@samcoe samcoe deleted the trim-whitespace branch January 29, 2022 07:32
@Pencab

This comment was marked as spam.

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.

Secrets don't work when set via stdin

3 participants