Azure / Azure/golden

locals can be paired with the wrong preserved expression due to independent map iteration

Open
#100 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
3
Forks
13
PR merge metrics
No merged PRs in 30d

Description

## Summary

`AsHclBlocks` can attach the `hclwrite` expression of one local attribute to the `hclsyntax` attribute of another local. The resulting `LocalBlock` evaluates the correct syntax expression, but `ExprString()` can return a different local's expression.

This is nondeterministic because the syntax and write attributes are collected by two independent Go map iterations and then paired by slice index.

## Impact

Consumers that persist or replay `HclAttribute.ExprString()` can save the wrong expression under a local's name. The original plan may evaluate correctly, while a later evaluation of the saved expression changes type or value.

In one downstream case, a local whose value was a string was saved with a sibling local's `list(string)` expression. A deferred HCL template then failed during apply with:

```text
Invalid template interpolation value; string required, but have list of string.
```

## Reproduction

This test uses only Golden's parsing/conversion path. Each expression contains its local's numeric suffix, so the assertion verifies that the syntax name and preserved write expression still refer to the same source attribute.

```go
package golden

import (
"fmt"
"strconv"
"strings"
"testing"

"github.com/hashicorp/hcl/v2"
"github.com/hashicorp/hcl/v2/hclsyntax"
"github.com/hashicorp/hcl/v2/hclwrite"
"github.com/stretchr/testify/require"
)

func TestAsHclBlocksPreservesLocalExpressionNames(t *testing.T) {
var source strings.Builder
source.WriteString("locals {\n")
for index := range 64 {
fmt.Fprintf(&source, " local_%02d = %q\n", index, fmt.Sprintf("expression_%02d", index))
}
source.WriteString("}\n")

syntaxFile, diagnostics := hclsyntax.ParseConfig([]byte(source.String()), "locals.hcl", hcl.InitialPos)
require.False(t, diagnostics.HasErrors(), diagnostics.Error())
writeFile, diagnostics := hclwrite.ParseConfig([]byte(source.String()), "locals.hcl", hcl.InitialPos)
require.False(t, diagnostics.HasErrors(), diagnostics.Error())

for range 1024 {
blocks := AsHclBlocks(
syntaxFile.Body.(*hclsyntax.Body).Blocks,
writeFile.Body().Blocks(),
)
require.Len(t, blocks, 64)
for _, block := range blocks {
name := block.Labels[1]
index := strings.TrimPrefix(name, "local_")
require.Equal(t, strconv.Quote("expression_"+index), block.Attributes()["value"].ExprString(), name)
}
}
}
```

On `70803d52d853639e7f9cfce1fb87cffa0e50a1bb` with Go `1.26.5 windows/amd64`, the test fails immediately. Example:

```text
expected: "\"expression_08\""
actual : "\"expression_20\""
Messages: local_08
```

I also ran the test in 50 independent `go test` processes to refresh runtime map iteration state:

```text
failed=50 passed=0
```

## Root cause

For a `locals` block, `readRawHclSyntaxBlock` ranges over `b.Body.Attributes` and returns a slice. Separately, `readRawHclWriteBlock` ranges over `b.Body().Attributes()` and returns another slice. Both attribute collections are Go maps.

`AsHclBlocks` then pairs those independently ordered slices by index:

```go
var rbs = readRawHclSyntaxBlock(b)
var wbs = readRawHclWriteBlock(writeBlocks[i])
for i, hb := range rbs {
blocks = append(blocks, NewHclBlock(hb, wbs[i], nil))
}
```

`NewHclBlock` consequently combines the syntax attribute from `rbs[i]` with the write attribute from an unrelated `wbs[i]`. `HclAttribute.Value()` uses the syntax expression while `HclAttribute.ExprString()` uses the write expression, which explains why planning can succeed but saved-expression replay fails.

## Suggested fix

Please pair local attributes by attribute name, not iteration position.

The safest approach is to special-case `locals` conversion in `AsHclBlocks` (or introduce one helper receiving both source blocks):

1. Read the syntax and write attribute maps.
2. Build one stable list of attribute names, preferably sorted for deterministic block order.
3. For each name, construct both synthetic `local` blocks from `syntaxAttributes[name]` and `writeAttributes[name]`.
4. Return an explicit error or diagnostic if either representation is missing that name.

Sorting the two currently independent slices by their local label before index pairing would also repair the immediate bug, but direct name-based lookup makes the invariant explicit and prevents a future helper change from reintroducing positional coupling.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with AsHclBlocks and the readRawHclSyntaxBlock/readRawHclWriteBlock helpers described in the issue, then run TestAsHclBlocksPreservesLocalExpressionNames using the supplied reproduction. Trace how syntax and write attributes are paired and verify that the test passes repeatedly with each local's ExprString matching its name.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
tooling
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.