terratest icon indicating copy to clipboard operation
terratest copied to clipboard

Filter out secrets from logs

Open mkyc opened this issue 4 years ago • 9 comments

While using docker module I find it quite problematic that env variables being passed to docker command are being logged as they are. It all comes down to this line where all command parameters are considered ... well just parameters, while docker env variables are used also for secrets.

If I pass CLIENT_SECRET env variable to docker.RunOptions EnvironmentVariables it gets transformed into shell.Command Args but I believe there should be also shell.Command Env involved.

Is there solution for that? I believe that it should be possible to limit some strings being logged.

mkyc avatar Nov 30 '20 11:11 mkyc

More control over logging would indeed be handy. Suggestions on how to improve this without blowing up the API, and corresponding PRs, are very welcome!

brikis98 avatar Nov 30 '20 13:11 brikis98

So there is two ideas I have in mind for now:

First one would be implementation of secretsSensitiveLogger that would be configured with []string with secrets values and would search for those strings in each outputted lines and substitute it with *** when found. That is approach used by some CI products.

Second option would be adding SecretEnvironmentVariables []string to docker.RunOptions and SectetArgs []string to shell.Command. Then shell.runCommand would need to append those secret args after log line.

First approach is for sure more compute intensive because requires checking of each of outputted lines. Second approach is bit more complicated and might introduce changes in API though.

I think first approach makes much more sense and in fact is possible to introduce even outside of terratest library by passing this secretsSensitiveLogger to Logger options field.

If that is something interesting for you, I could also prepare PR with it in free time.

mkyc avatar Nov 30 '20 13:11 mkyc

Hm, I worry that a secretsSensitiveLogger would be error prone. You'd add a secret in one place, forget to update the logger to strip that secret out, and not notice until it's too late.

Would it make sense to be able to suppress logging the command being executed with a single SensitiveVars boolean flag?

brikis98 avatar Dec 01 '20 10:12 brikis98

Would it make sense to be able to suppress logging the command being executed with a single SensitiveVars boolean flag?

FWIW, we have this functionality built in gruntwork-cli. Probably makes sense to port that over: https://github.com/gruntwork-io/gruntwork-cli/blob/master/shell/cmd.go#L16

yorinasub17 avatar Dec 01 '20 15:12 yorinasub17

Ah, yea, seems like a good idea to keep that consistent behavior. @mkyc WDYT?

brikis98 avatar Dec 02 '20 12:12 brikis98

Doesn't this solution lack granularity? It's kind of binary approach. In general it's very good thing to log parameters so making it completely deaf because of one secret is kind of throwing the baby out with the bathwater, isn't it?

mkyc avatar Dec 02 '20 13:12 mkyc

Another option here is to fix docker.Run to support setting environment variables on the shell runtime. That way, you can use the "inherit from runtime env var" feature to set the secrets. The idea would be:

  • Add a new property RuntimeEnvironmentVariables to the RunOptions, which get passed through to the shell command Env property.
  • When setting secrets, you set it in RuntimeEnvironmentVariables (so it isn't logged), and in EnvironmentVariables you just set the key (so even if it is logged, it only logs the secret key not the value).

FWIW, we generally recommend managing real secrets outside terratest and setting it in the env var of the shell before calling go test, as it is the safest way to avoid logging your secrets. You can use the existing code to do a similar set up as above, only you have the env var already set in the parent shell running go test, bypassing the need for RuntimeEnvironmentVariables. Basically something like:

export CLIENT_SECRET=something
go test ...
>> docker run -e CLIENT_SECRET ...

yorinasub17 avatar Dec 02 '20 15:12 yorinasub17

That sounds a lot better IMHO. Do you have any example of such approach in another package or repo?

mkyc avatar Dec 02 '20 22:12 mkyc

I don't think we have an example in our public repos, but I can describe what we do.

We use CircleCI contexts to set the env vars in the build job, and then set the docker run environment variables to only have the key. For example, if we had a secret called GITHUB_OAUTH_TOKEN, that would be set in the CircleCI context and passed to the container in terratest with:

opts := docker.RunOptions{
    // This will be inherited from the test runner environment variables.
    EnvironmentVariables: []string{"GITHUB_OAUTH_TOKEN"},
}

You can probably find similar mechanisms for managing secrets with other CI servers.

For local, we recommend our team to use pass to create a multiline secret that contains the export calls and do eval "$(pass github-secrets)".

Another way we manage secrets outside of terratest is using AWS SecretsManager, and using the aws CLI to grab the secret value to set the env var, e.g.:

export GITHUB_OAUTH_TOKEN="$(aws secretsmanager get-secret-value --region $REGION --secret-id $SECRET_ARN --query 'SecretString' --output text)"

yorinasub17 avatar Dec 02 '20 22:12 yorinasub17