Skip to content

Add concept of sensitive args #14

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

Merged
merged 2 commits into from
Nov 28, 2018
Merged

Add concept of sensitive args #14

merged 2 commits into from
Nov 28, 2018

Conversation

yorinasub17
Copy link
Contributor

Right now we log the command string when we call commands using RunShellCommand, which is really nice for debugging purposes. However, sometimes this should be suppressed because the args might include sensitive data. For example, when scripting calls to openssl to create certs, the password can only be passed in via the command line when avoiding the prompt.

The solution proposed and implemented here is to add a new shell option that marks the commands as having sensitive args, which is used to suppress logging the args.

Copy link
Member

@brikis98 brikis98 left a comment

Choose a reason for hiding this comment

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

I like this idea. It would be even better if we could mark specific arguments as sensitive, but that's prob too error prone.


assert.Nil(t, RunShellCommand(options, "echo", "hi"))
assert.False(t, strings.Contains(buffer.String(), "hi"))
assert.True(t, strings.Contains(buffer.String(), "echo"))
Copy link
Member

Choose a reason for hiding this comment

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

Use assert.Contains and assert.NotContains

@yorinasub17
Copy link
Contributor Author

It would be even better if we could mark specific arguments as sensitive, but that's prob too error prone.

Yea I thought about this, but it was hard to come up with a good API for the general case when the target command could be anything.

@yorinasub17
Copy link
Contributor Author

Merging and releasing!

@yorinasub17 yorinasub17 merged commit f56d6e3 into master Nov 28, 2018
@yorinasub17 yorinasub17 deleted the yori-sensitive branch November 28, 2018 18:00
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.

2 participants