From c6061b5363e1a7a3c682ad635666737d7528b080 Mon Sep 17 00:00:00 2001 From: simonfaltum Date: Tue, 9 Jun 2026 22:09:56 +0200 Subject: [PATCH 1/2] Error on cyclic YAML anchor references instead of stack overflow Co-authored-by: Isaac --- libs/dyn/yamlloader/loader.go | 18 +++++++++++++++- libs/dyn/yamlloader/testdata/anchor_09.yml | 2 ++ libs/dyn/yamlloader/testdata/anchor_10.yml | 5 +++++ libs/dyn/yamlloader/yaml_anchor_test.go | 25 ++++++++++++++++++++++ 4 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 libs/dyn/yamlloader/testdata/anchor_09.yml create mode 100644 libs/dyn/yamlloader/testdata/anchor_10.yml diff --git a/libs/dyn/yamlloader/loader.go b/libs/dyn/yamlloader/loader.go index 7ff4303d8ee..f65916020ef 100644 --- a/libs/dyn/yamlloader/loader.go +++ b/libs/dyn/yamlloader/loader.go @@ -23,6 +23,11 @@ func (e *LocationError) Error() string { type loader struct { path string + + // Aliases currently being expanded. A self-referential anchor (e.g. + // "a: &x {b: *x}") makes the yaml.v3 node graph cyclic; without this + // bookkeeping, expansion would recurse until stack overflow. + activeAliases map[*yaml.Node]bool } func errorf(loc dyn.Location, format string, args ...any) error { @@ -31,7 +36,8 @@ func errorf(loc dyn.Location, format string, args ...any) error { func newLoader(path string) *loader { return &loader{ - path: path, + path: path, + activeAliases: make(map[*yaml.Node]bool), } } @@ -271,5 +277,15 @@ func (d *loader) loadScalar(node *yaml.Node, loc dyn.Location) (dyn.Value, error } func (d *loader) loadAlias(node *yaml.Node, loc dyn.Location) (dyn.Value, error) { + if d.activeAliases[node] { + return dyn.InvalidValue, errorf(loc, "cyclic reference to anchor %q", node.Value) + } + + // Remove the alias from the set once expanded: the same alias node may be + // reached again through another alias to an enclosing anchor, which is not + // a cycle. + d.activeAliases[node] = true + defer delete(d.activeAliases, node) + return d.load(node.Alias) } diff --git a/libs/dyn/yamlloader/testdata/anchor_09.yml b/libs/dyn/yamlloader/testdata/anchor_09.yml new file mode 100644 index 00000000000..8005bf445ef --- /dev/null +++ b/libs/dyn/yamlloader/testdata/anchor_09.yml @@ -0,0 +1,2 @@ +anchortest: &x + child: *x diff --git a/libs/dyn/yamlloader/testdata/anchor_10.yml b/libs/dyn/yamlloader/testdata/anchor_10.yml new file mode 100644 index 00000000000..013832ab3ac --- /dev/null +++ b/libs/dyn/yamlloader/testdata/anchor_10.yml @@ -0,0 +1,5 @@ +leaf: &leaf 1 +shared: &shared + value: *leaf +use1: *shared +use2: *shared diff --git a/libs/dyn/yamlloader/yaml_anchor_test.go b/libs/dyn/yamlloader/yaml_anchor_test.go index 05beb5401d8..d3e4553c70e 100644 --- a/libs/dyn/yamlloader/yaml_anchor_test.go +++ b/libs/dyn/yamlloader/yaml_anchor_test.go @@ -1,10 +1,14 @@ package yamlloader_test import ( + "bytes" + "os" "testing" "github.com/databricks/cli/libs/dyn" + "github.com/databricks/cli/libs/dyn/yamlloader" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestYAMLAnchor01(t *testing.T) { @@ -115,3 +119,24 @@ func TestYAMLAnchor08(t *testing.T) { assert.Equal(t, true, active.AsAny()) assert.Equal(t, dyn.Location{File: file, Line: 2, Column: 11}, active.Location()) } + +func TestYAMLAnchor09(t *testing.T) { + // A self-referential anchor must return an error instead of + // recursing until stack overflow. + file := "testdata/anchor_09.yml" + input, err := os.ReadFile(file) + require.NoError(t, err) + + _, err = yamlloader.LoadYAML(file, bytes.NewBuffer(input)) + assert.ErrorContains(t, err, `cyclic reference to anchor "x"`) +} + +func TestYAMLAnchor10(t *testing.T) { + // Expanding the same alias node again on a different path is not a cycle. + file := "testdata/anchor_10.yml" + self := loadYAML(t, file) + assert.NotEqual(t, dyn.NilValue, self) + + assert.Equal(t, 1, self.Get("use1").Get("value").AsAny()) + assert.Equal(t, 1, self.Get("use2").Get("value").AsAny()) +} From ff32c312c444c2b84469292681a7c0a6a45200c4 Mon Sep 17 00:00:00 2001 From: simonfaltum Date: Wed, 10 Jun 2026 00:08:30 +0200 Subject: [PATCH 2/2] Remove redundant comments Drop per-test-case explanation comments from yaml_anchor_test.go and trim the loadAlias comment to the why only. No behavior changes. Co-authored-by: Isaac --- libs/dyn/yamlloader/loader.go | 5 ++--- libs/dyn/yamlloader/yaml_anchor_test.go | 3 --- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/libs/dyn/yamlloader/loader.go b/libs/dyn/yamlloader/loader.go index f65916020ef..0f69f0c638b 100644 --- a/libs/dyn/yamlloader/loader.go +++ b/libs/dyn/yamlloader/loader.go @@ -281,9 +281,8 @@ func (d *loader) loadAlias(node *yaml.Node, loc dyn.Location) (dyn.Value, error) return dyn.InvalidValue, errorf(loc, "cyclic reference to anchor %q", node.Value) } - // Remove the alias from the set once expanded: the same alias node may be - // reached again through another alias to an enclosing anchor, which is not - // a cycle. + // The same alias node may be reached again through another alias to an + // enclosing anchor, which is not a cycle. d.activeAliases[node] = true defer delete(d.activeAliases, node) diff --git a/libs/dyn/yamlloader/yaml_anchor_test.go b/libs/dyn/yamlloader/yaml_anchor_test.go index d3e4553c70e..53cd6945f93 100644 --- a/libs/dyn/yamlloader/yaml_anchor_test.go +++ b/libs/dyn/yamlloader/yaml_anchor_test.go @@ -121,8 +121,6 @@ func TestYAMLAnchor08(t *testing.T) { } func TestYAMLAnchor09(t *testing.T) { - // A self-referential anchor must return an error instead of - // recursing until stack overflow. file := "testdata/anchor_09.yml" input, err := os.ReadFile(file) require.NoError(t, err) @@ -132,7 +130,6 @@ func TestYAMLAnchor09(t *testing.T) { } func TestYAMLAnchor10(t *testing.T) { - // Expanding the same alias node again on a different path is not a cycle. file := "testdata/anchor_10.yml" self := loadYAML(t, file) assert.NotEqual(t, dyn.NilValue, self)