gessnerfl/terraform-provider-instana

Provider shows used API-token in cleartext upon failure

Closed this issue · 4 comments

Due to some misconfiguration in my Terraform-configuration I had used the wrong token for the Instana-API.

I found out the provider logged the used apiToken header in cleartext in the error-message.

This could be helpful for debugging, but at the same time I wonder if this output could get prevented.

apiToken is marked as XXXXXXXXXXXXXXXXXX

│ Error: failed to send HTTP GET request to Instana API; status code = 0; status message = ; Headers map[], Get "https://instana.example.com/api/application-monitoring/settings/application/yyyyyyyyyyyyyyyy": net/http: invalid header field value "apiToken XXXXXXXXXXXXXXXXXX\n" for key Authorization
│
│   with instana_application_config.applications["team_dev1"],
│   on applications.tf line 1, in resource "instana_application_config" "applications":
│    1: resource "instana_application_config" "applications" {
│
╵

Provider-version: v1.2.0
Terraform: v1.1.7

Hi @ppuschmann,

I also think the API Token ideally is not logged in any case. However in the code this is hard to filter as the log is used to provide details about failed HTTP calls to Instana API in general. The API token is not logged explicitly by the provider but from the core go http module.

return emptyResponse, fmt.Errorf("failed to send HTTP %s request to Instana API; status code = %d; status message = %s; Headers %s, %s", method, resp.StatusCode(), resp.Status(), resp.Header(), err)

For the particular case, I could extend the code to trim whitespaces to ensure that the api key is valid.

Thank you for acknowledging this issue.

As you might have seen, I had accidentally introduced a \n to the token.
I achieved this by encrypting the token in a wrong way with Google KMS.

I provided release v1.2.1 where I added trimming of whitespaces for the API token and endpoint in the provider configuration. In both cases, any whitespace is not expected so that this operation is safe to apply and should address the reported issue.

@ppuschmann Can you please cross check and confirm?

Considering this issue as solved after one month of inactivity.