Skip to content

Detect self-referencing values instead of crashing with a stack overflow - #1007

Open
pujitha24 wants to merge 1 commit into
carvel-dev:developfrom
pujitha24:auto/issue-1006
Open

Detect self-referencing values instead of crashing with a stack overflow#1007
pujitha24 wants to merge 1 commit into
carvel-dev:developfrom
pujitha24:auto/issue-1006

Conversation

@pujitha24

Copy link
Copy Markdown

Motivation:
A Starlark list or dict that (directly or indirectly) contains itself
made ytt's Starlark-to-Go value conversion recurse forever, aborting
the process with an unrecoverable Go runtime "fatal error: stack
overflow" after allocating up to 1GB of stack. This has no ytt-level
error message and exit code 2, and because it is a Go runtime fatal
error (not a panic), it cannot be recovered by an embedding program
using ytt as a library. This affects YAML/text template rendering as
well as @ytt:json's json.encode() and @ytt:yaml's yaml.encode(), all
of which route through the same conversion path.

Approach:
pkg/template/core/starlark_value.go's asInterface/dictAsInterface/
itearableAsInterface now thread a path argument through the recursive
conversion, recording the *starlark.Dict and *starlark.List values
currently being converted (i.e. the ancestors in the current
recursion). Encountering a dict or list already present in path now
returns a normal ytt error instead of recursing further. Only Dict
and List are checked: starlark.Tuple is immutable after construction,
*starlark.Set can only hold hashable elements so it can never hold a
Dict/List/itself, and ytt's own *StarlarkStruct is built once from a
fixed map with no field-mutation exposed to Starlark code, so none of
those three can participate in a cycle. This mirrors the cycle
detection already used by the vendored starlark-go library itself in
starlark/value.go's writeValue/pathContains (used by str() on
Starlark values), which uses the same append(path, x) idiom and also
only checks List and Dict.

The remainder of the diff (extracting scalarAsInterface,
dictItemAsInterface, and extendPathWithList, wrapping long function
signatures, and a dictItemKeyValueLen constant) is a mechanical,
behavior-preserving split of the existing type switch, needed to stay
under this repo's golangci-lint (revive, enable-all-rules) complexity
and line-length limits once the path parameter touched those lines.

Validation:

  • go build ./... succeeds.
  • go test ./... passes for the whole repo, including a new test,
    TestSelfReferencingValueReturnsError in
    pkg/cmd/template/cmd_test.go, covering a direct self-referencing
    list, a direct self-referencing dict, and an indirect list->dict->
    list cycle, using the exact repro from the issue. Confirmed this
    test fails before the fix: with only the fix reverted (via git
    stash on starlark_value.go), the list subtest reproduces the actual
    "fatal error: stack overflow" crash from the issue; restoring the
    fix makes all subtests pass.
  • golangci-lint run ./... (v2.12.2, matching the version pinned in
    .github/workflows/golangci-lint.yml) reports 0 issues.
  • Manually ran the built ytt binary against the issue's exact repro
    (self-referencing list and dict), against @ytt:json's json.encode()
    on a self-referencing list, and against a non-cyclic case where the
    same dict value is legitimately shared by reference at multiple
    points in the output, to confirm sharing (as opposed to a cycle) is
    still converted correctly and not falsely flagged.

Report: #1006
Signed-off-by: Pujitha Paladugu 10557236+pujitha24@users.noreply.github.com
Assisted-by: claude-sonnet-5 (via Claude Code)

Fixes #1006

Motivation:
A Starlark list or dict that (directly or indirectly) contains itself
made ytt's Starlark-to-Go value conversion recurse forever, aborting
the process with an unrecoverable Go runtime "fatal error: stack
overflow" after allocating up to 1GB of stack. This has no ytt-level
error message and exit code 2, and because it is a Go runtime fatal
error (not a panic), it cannot be recovered by an embedding program
using ytt as a library. This affects YAML/text template rendering as
well as @ytt:json's json.encode() and @ytt:yaml's yaml.encode(), all
of which route through the same conversion path.

Approach:
pkg/template/core/starlark_value.go's asInterface/dictAsInterface/
itearableAsInterface now thread a path argument through the recursive
conversion, recording the *starlark.Dict and *starlark.List values
currently being converted (i.e. the ancestors in the current
recursion). Encountering a dict or list already present in path now
returns a normal ytt error instead of recursing further. Only Dict
and List are checked: starlark.Tuple is immutable after construction,
*starlark.Set can only hold hashable elements so it can never hold a
Dict/List/itself, and ytt's own *StarlarkStruct is built once from a
fixed map with no field-mutation exposed to Starlark code, so none of
those three can participate in a cycle. This mirrors the cycle
detection already used by the vendored starlark-go library itself in
starlark/value.go's writeValue/pathContains (used by str() on
Starlark values), which uses the same append(path, x) idiom and also
only checks List and Dict.

The remainder of the diff (extracting scalarAsInterface,
dictItemAsInterface, and extendPathWithList, wrapping long function
signatures, and a dictItemKeyValueLen constant) is a mechanical,
behavior-preserving split of the existing type switch, needed to stay
under this repo's golangci-lint (revive, enable-all-rules) complexity
and line-length limits once the path parameter touched those lines.

Validation:
- go build ./... succeeds.
- go test ./... passes for the whole repo, including a new test,
  TestSelfReferencingValueReturnsError in
  pkg/cmd/template/cmd_test.go, covering a direct self-referencing
  list, a direct self-referencing dict, and an indirect list->dict->
  list cycle, using the exact repro from the issue. Confirmed this
  test fails before the fix: with only the fix reverted (via git
  stash on starlark_value.go), the list subtest reproduces the actual
  "fatal error: stack overflow" crash from the issue; restoring the
  fix makes all subtests pass.
- golangci-lint run ./... (v2.12.2, matching the version pinned in
  .github/workflows/golangci-lint.yml) reports 0 issues.
- Manually ran the built ytt binary against the issue's exact repro
  (self-referencing list and dict), against @ytt:json's json.encode()
  on a self-referencing list, and against a non-cyclic case where the
  same dict value is legitimately shared by reference at multiple
  points in the output, to confirm sharing (as opposed to a cycle) is
  still converted correctly and not falsely flagged.

Report: carvel-dev#1006
Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>
Assisted-by: claude-sonnet-5 (via Claude Code)
@carvel-bot carvel-bot added this to Carvel Sep 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

Self-referencing value crashes ytt with a Go stack overflow instead of an error

2 participants