go get -u github.com/mitchellh/cli

[finnigja]

This updates to a newer github.com/mitchellh/cli version, bringing in several indirect dependency updates that include fixes for security issues (specifically - github.com/Masterminds/goutils CVE-2021-4238 and GHSA-xg2h-wx96-xgxr, and golang.org/x/text CVE-2022-32149 and GHSA-69ch-w2m2-3vjp). These vulnerabilities are unlikely exposed in Terraform itself, but adoption reduces vulnerability scan noise, and may ease adoption of future updates.

Target Release

1.4.x, 1.3.x

Draft CHANGELOG entry

BUG FIXES

• Updated to newer github.com/mitchellh/cli version, in turn bringing in updates for several indirect dependencies with known security issues.

[apparentlymart]

Thanks!

This does seem reasonable to do, but I just want to note that we don't typically put this sort of thing in Terraform's changelog unless we know the upgrade will change Terraform's user-observable behavior somehow. It seems like the idea here is that this change should be entirely neutral from a user perspective -- it has fixes to codepaths that Terraform doesn't use and nothing else -- so that would be a situation where we'd typically not mention it.

We should still make an effort to confirm whether that is true though; if there's anything coming along for the ride with these upgrades that would affect Terraform behavior then we should consider adding those to our changelog as if they were direct changes to Terraform. We don't expect readers of the changelog to know what Terraform's dependencies are and so I wouldn't expect to mention "mitchellh/cli" or any of these other packages in the changelog; similar principle as over in #32135 where we reframed the relevant entries from the Go release notes in terms that make sense for Terraform.

[apparentlymart]

I looked through the changes to each of the libraries that would be upgraded by this change. I found a few changes that do seem to change Terraform's observable behavior in ways that might affect end-users. Some hypothetical changelog entries below.

Changes that I've confirmed:

• config: The textencodebase64 function when called with encoding "GB18030" will now encode the euro symbol € as the two-byte sequence 0xA2,0xE3, as required by the GB18030 standard, before applying base64 encoding.

• config: The textencodebase64 function when called with encoding "GBK" or "CP936" will now encode the euro symbol € as the single byte 0x80 before applying base64 encoding. This matches the behavior on Windows when encoding to this Windows-specific character encoding.

• terraform init: When interpreting the hostname portion of a provider source address or the address of a module in a module registry, Terraform will now use non-transitional IDNA2008 mapping rules instead of the transitional mapping rules previously used.

  This matches a change to the WHATWG URL spec's rules for interpreting non-ASCII domain names which is being gradually adopted by web browsers. Terraform aims to follow the interpretation of hostnames used by web browsers for consistency. For some hostnames containing non-ASCII characters this may cause Terraform to now request a different "punycode" hostname when resolving.

Possible changes, not yet checked:

• cli: Terraform will now respect the NO_COLOR environment variable and disable any colored output to the terminal if set.

That last one I've not confirmed yet because it's a change made in a terminal coloring library used only indirectly by mitchellh/cli and I'm not familiar enough with it to confirm whether mitchellh/cli is somehow overriding that default setting and imposing its own instead. I think we'll want to confirm that before merging because if that does change the behavior in this way then that was one of the proposed approaches for #23708 and so we'll need to revisit that issue and see if that is the direction we intend to take to resolve it, or if we want to somehow disable this change in behavior until we have an opportunity to consider it more deeply.

Also, the terraform init hostname change and the NO_COLOR change above seem to be enhancements rather than bug fixes and so would not typically be eligible for backport into an active release branch. I wonder if it would be possible to perform a more surgical upgrade which avoids those changes so that we can make this more palatable to backport into the v1.3 branch. Alternatively, we could land it in main and have it go out with all of these changes in the v1.4.0 release.

[finnigja]

Thanks for digging into these, @apparentlymart! If the changes are significant / potentially-breaking, then - given that we have no evidence the security issues in indirect dependencies are actually exposed in Terraform context and we're doing this largely to avoid scanner noise & stay current - I think it'd be okay to just merge into main & ship with v1.4.0, as you say.

[apparentlymart]

I reviewed our use of github.com/fatih/color more deeply and have confirmed that Terraform does not use that package, even though it's in our dependency graph.

The github.com/mitchellh/cli package offers a UiColor type which implements the Ui interface in terms of github.com/fatih/color. However, Terraform has its own ColorizeUi which achieves the same effect using the helper library github.com/mitchellh/colorstring instead.

Terraform therefore doesn't use cli.UiColor and indirectly doesn't use github.com/fatih/color, so the NO_COLOR change in that dependency will not affect Terraform's behavior.

With all of that said then: I think we're good to land this in the main branch for inclusion in the forthcoming v1.4.0 release, with the changelog entries I mentioned above aside from the one about NO_COLOR.

[github-actions]

Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch.

[github-actions]

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.