hclsyntax: Template expression produces too large value refinements

[wata727]

See #590
See https://github.com/zclconf/go-cty/pull/153

HCL v2.17 introduced value refinements. However, this can result in template expressions that produce too large refinements.

module example.com/m

go 1.20

require (
        github.com/hashicorp/hcl/v2 v2.17.0
        github.com/zclconf/go-cty v1.13.2
)

require (
        github.com/agext/levenshtein v1.2.1 // indirect
        github.com/apparentlymart/go-textseg/v13 v13.0.0 // indirect
        github.com/mitchellh/go-wordwrap v0.0.0-20150314170334-ad45545899c7 // indirect
        github.com/vmihailenco/msgpack/v5 v5.3.5 // indirect
        github.com/vmihailenco/tagparser/v2 v2.0.0 // indirect
        golang.org/x/text v0.3.8 // indirect
)

package main

import (
        "fmt"
        "strings"

        "github.com/hashicorp/hcl/v2"
        "github.com/hashicorp/hcl/v2/hclsyntax"
        "github.com/zclconf/go-cty/cty"
        "github.com/zclconf/go-cty/cty/msgpack"
)

func main() {
        src := strings.Repeat("a", 1024) + "${unknown}"

        expr, diags := hclsyntax.ParseTemplate([]byte(src), "", hcl.InitialPos)
        if diags.HasErrors() {
                panic(diags)
        }

        val, diags := expr.Value(&hcl.EvalContext{Variables: map[string]cty.Value{"unknown": cty.UnknownVal(cty.String)}})
        if diags.HasErrors() {
                panic(diags)
        }
        fmt.Println(val.GoString())

        bytes, err := msgpack.Marshal(val, cty.String)
        if err != nil {
                panic(err)
        }
        restored, err := msgpack.Unmarshal(bytes, cty.String)
        if err != nil {
                panic(err)
        }
        fmt.Println(restored.GoString())

}

$ go run main.go
cty.UnknownVal(cty.String).Refine().NotNull().StringPrefixFull("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa").NewValue()
panic: oversize unknown value refinement

goroutine 1 [running]:
main.main()
        /workspaces/tflint/work/refine/main.go:33 +0x305
exit status 2

There are two issues with too large refinements. One, as shown above, is hitting the upper limit of go-cty's MessagePack serialization. The other is that it can be nonsense for user feedback purposes.

Such issues mainly occur in Terraform's templatefile function. Imagine an example with an unknown value at the end of a huge JSON file. terraform-linters/tflint#1791 is an actual reported issue in TFLint.

To be honest, I struggled with whether to report this issue to HCL or go-cty. I believe the solutions to this issue are:

• Set a reasonable upper limit on the size of refinements in HCL
• Remove the upper limit of go-cty's MessagePack serialization
• Add API to remove refinements from the value
  • As far as I can see, there doesn't seem to be a way to discard refinements that have already been set. It probably looks like it can be discarded converted to JSON format, but it would be nice to have an intuitive API like value.WithoutRefinements.

[apparentlymart]

Hi @wata727! Thanks for reporting this.

I think for the moment it's probably most pragmatic to just set a reasonable upper limit in HCL's template implementation where after a certain size it truncates the known prefix. That should still produce a valid refinement because it will broaden rather than tighten the range of the result.

The question of whether and how to have limits in cty is also valid but will require some more consideration. Changing the HCL template logic is less risky for an immediate fix because it only affects one use-case where we know that a problem exists; large strings can potentially appear in other places too, but templates seem like the most likely origin.

[apparentlymart]

Oh and on the subject of removing refinements: today we have some code in Terraform doing that to work around some historical implementation mistakes in Terraform's rules for validating plans and we're doing it by using cty.Transform with a transformer that checks if the value is unknown and if so returns cty.UnknownVal(v.Type()), therefore constructing a new unknown value of the same type without any refinements.

That could potentially be a helper function in cty but I'd like to see if we can avoid the need for it by fixing these rough edges in the main refinements handling.

[wata727]

Thank you for answering my question.

I think for the moment it's probably most pragmatic to just set a reasonable upper limit in HCL's template implementation where after a certain size it truncates the known prefix.

Agreed. I've opened a PR for this issue #617
I struggled with what a "reasonable upper limit" is, but decided on 128 B here. Please let me know if there is a better upper limit. Also, if you have a proper implementation other than this, you can ignore this PR.

we're doing it by using cty.Transform with a transformer that checks if the value is unknown and if so returns cty.UnknownVal(v.Type()), therefore constructing a new unknown value of the same type without any refinements.

Thank you for telling me how to do this. The unrefineValue is indeed what I was looking for. For the time being, I will try #617 to see if the issue is mitigated. If it is difficult to fix, consider implementing a function equivalent to unrefinedValue.