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

Sensitive outputs shouldn't be exposed when using terraform.OutputRequired #476

Open
soerenmartius opened this issue Mar 16, 2020 · 10 comments
Assignees
Labels
enhancement New feature or request trial project Suitable for trial projects

Comments

@soerenmartius
Copy link

Hi,

We are working on a test-case that creates some IAM users and access keys that we use for authenticating with some AWS resources within that test. To be able to use the credentials we pull them from the outputs, e.G:

outputs.tf:

output "aws_iam_access_key_secret" {
  description = "The acccess key secret."
  value       = aws_iam_access_key.docker.secret
  sensitive   = true
}

test.go:

accessKeySecret := terraform.OutputRequired(t, terraformOptions, 'aws_iam_access_key_secret')

Using the helper methods to retrieve outputs with terraform will dump the outputs to the logs. I believe we should align the behavior of terratest with terraform and only write <sensitive> instead of the actual values to the logs.

@brikis98
Copy link
Member

Oh, good point! Would you be up for a PR to fix this?

@soerenmartius
Copy link
Author

soerenmartius commented Mar 17, 2020

Ok, so running terraform output will mask any outputs that have sensitive = true set. If one calls terraform output on a specific output ( e.g. terraform output aws_iam_access_key_secret terraform returns the value of the output as clear text, which makes kind of sense to me also.

It seems to me that there is no easy way to be aware if we deal with sensitive data or not when calling terraform output on a specific output. The log line is written in the command.go file though. It might be a reasonable and simpler approach to make the logging of the output configurable.

wdyt @brikis98 ?

@brikis98
Copy link
Member

Try terraform output -json. I believe that shows you which ones are sensitive.

@soerenmartius
Copy link
Author

yeah sure, terraform output also shows you which ones are sensitive. That isn't very helpful though since the outputs methods in terratest would end up calling terraform output key, which will always return clear text.

@brikis98
Copy link
Member

Yea, what I meant was that the terraform.Output(xxx) helper would always run terraform output -json with no logging, read output xxx from the JSON, and only log what it saw if it wasn't sensitive. So it's making the logging optional in command.go as you said, but also making the terraform.Output helper in how it uses it.

@soerenmartius
Copy link
Author

Thank you, that makes sense to me! I will take a proper look in a bit.

@burythehammer
Copy link

We've been seeing this issue as well. We apply some terraform, and then use terratest's output functions to fetch out a randomly generated password. We need the password to ensure that the applied resources have been set up, but by getting the output, it also prints out the password to our logs - not ideal.

It feels like by default logging should be turned off for any sensitive output.

Has there been any progress on this issue?

@yorinasub17
Copy link
Contributor

Here is a workaround for this that we recently identified:

func fetchSensitiveOutput(t *testing.T, options *terraform.Options, name string) string {
    defer func() {
        options.Logger = nil
    }()
    options.Logger = logger.Discard  // import path is github.com/gruntwork-io/terratest/modules/logger
    return terraform.Output(t, options, name)
}

@estahn
Copy link
Contributor

estahn commented Sep 30, 2021

I started using terratest to test my Kubernetes Webhook (k8s-image-swapper). The IAM keys and docker credentials were exposed in the logs, which is not ideal for CI purposes. I came up with the following solution:

Usage:

func TestHelmDeployment(t *testing.T) {
	workingDir, _ := filepath.Abs("..")

	awsAccountID := os.Getenv("AWS_ACCOUNT_ID")
	awsRegion := os.Getenv("AWS_DEFAULT_REGION")
	awsAccessKeyID := os.Getenv("AWS_ACCESS_KEY_ID")
	awsSecretAccessKey := os.Getenv("AWS_SECRET_ACCESS_KEY")
	ecrRegistry := awsAccountID + ".dkr.ecr." + awsRegion + ".amazonaws.com"
	ecrRepository := "docker.io/library/nginx"

	logger.Default = logger.New(newSensitiveLogger(
		logger.Default,
		[]*regexp.Regexp{
			regexp.MustCompile(awsAccountID),
			regexp.MustCompile(awsAccessKeyID),
			regexp.MustCompile(awsSecretAccessKey),
			regexp.MustCompile(`(--docker-password=)\S+`),
		},
	))

    ...

Implementation::

type sensitiveLogger struct{
	logger logger.TestLogger
	patterns []*regexp.Regexp
}

func newSensitiveLogger(logger *logger.Logger, patterns []*regexp.Regexp) *sensitiveLogger {
	return &sensitiveLogger{
		logger: logger,
		patterns: patterns,
	}
}

func (l *sensitiveLogger) Logf(t terratesttesting.TestingT, format string, args ...interface{}) {
	var redactedArgs []interface{}

	obfuscateWith := "$1*******"

	redactedArgs = args

	for _, pattern := range l.patterns {
		for i, arg := range redactedArgs {
			switch arg := arg.(type) {
			case string:
				redactedArgs[i] = pattern.ReplaceAllString(arg, obfuscateWith)
			case []string:
				var result []string
				for _, s := range arg {
					result = append(result, pattern.ReplaceAllString(s, obfuscateWith))
				}
				redactedArgs[i] = result
			default:
				panic("type needs implementation")
			}
		}
	}

	l.logger.Logf(t, format, redactedArgs...)
}

@brikis98 What are your thoughts on this? Do you think this is something we can add to terratest itself?

Copy link
Contributor

After some exploration, I haven't found a straightforward solution.

The most ideal approach would be to implement a custom logger that can detect sensitive values as logs are being written. This would involve creating a streaming JSON decoder that can tolerate invalid tokens, identify JSON objects representing Terraform output values, check if they are marked as sensitive based on the sensitive field, and then censor the value field accordingly. However, I haven’t found any library that supports this out of the box, so it would require significant research and development time, with a relatively high risk of not being completed.

An alternative would be to create a logger that first stores all log values in memory, and only writes them after analyzing the parsed output to identify which values are sensitive. It would then filter every occurrence of those sensitive values before writing the logs. This approach increases memory usage since all logs need to be held in memory temporarily, and also risks censoring non-sensitive variables that coincidentally share the same value as a sensitive one.

As a fallback, users of the library can simply set logger.Discard in the options to prevent any logging altogether. This requires no code changes, as it's already supported in the current implementation.

@james03160927 james03160927 self-assigned this Mar 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request trial project Suitable for trial projects
Projects
None yet
Development

No branches or pull requests

7 participants