Skip to content

bug: Dead validation code: local ignition config files never validated #2280

Description

@yasminvalim

Created originally in Butane by @deepak0x: coreos/butane#724




name: release checklist
about: release checklist template
title: Dead validation code: local ignition config files never validated
labels: jira,kind/bug
warning: |
⚠️ Template generated by https://github.com/coreos/repo-templates; do not edit downstream

Bug Report

File: base/v0_8_exp/translate.go, line 157 (same pattern at base/v0_7/translate.go:153)

Summary:

translateResource contains dead validation code. The strings.HasPrefix guard at line 157 is always false, so ValidateIgnitionConfig is never called for local files — even when those files are used in ignition.config.merge or ignition.config.replace contexts. Invalid or corrupt ignition config data in a local field passes through translation silently with no warning or error.


Buggy code

https://github.com/coreos/butane/blob/cb34e120e5267bfd5bdfa83fa8c2e44e06dedda2/base/v0_8_exp/translate.go#L157

if from.Local != nil {
    c := path.New("yaml", "local")
    contents, err := baseutil.ReadLocalFile(*from.Local, options.FilesDir)
    if err != nil {
        r.AddOnError(c, err)
        return
    }
    // Validating the contents of the local file from here since there is no way to
    // get both the filename and filedirectory in the Validate context
    if strings.HasPrefix(c.String(), "$.ignition.config") {   // ← always false
        rp, err := ValidateIgnitionConfig(c, contents)
        r.Merge(rp)
        if err != nil {
            return
        }
    }
    ...
}

Root cause

c is constructed as path.New("yaml", "local"). The first argument is the tag; the second is a path element.

path.ContextPath.String() (defined in vendor/github.com/coreos/vcontext/path/path.go:36) builds its output by joining path elements with dots, prefixed with $. The tag is not included in the string representation.

Therefore:

path.New("yaml", "local").String()  →  "$.local"
strings.HasPrefix("$.local", "$.ignition.config")  →  false

The condition can never be true regardless of where translateResource is called from. ValidateIgnitionConfig is dead code for all local field inputs.


Impact

Local files referenced via the local field inside ignition.config.merge or ignition.config.replace blocks are never validated as Ignition configs at translation time. Malformed or invalid ignition config JSON/YAML in those files is silently accepted and encoded into the output without any diagnostic.


Reproduction

Confirmed by evaluating the vendored vcontext library directly:

import "github.com/coreos/vcontext/path"
import "strings"

c := path.New("yaml", "local")
fmt.Println(c.String())                                         // "$.local"
fmt.Println(strings.HasPrefix(c.String(), "$.ignition.config")) // false

path.New("yaml", "local").String() returns "$.local", which does not satisfy strings.HasPrefix(..., "$.ignition.config").

The same dead-code pattern exists independently at base/v0_7/translate.go:153.


Tagging:

  • Confirm path.ContextPath.String() does not include the tag in any vcontext version vendored by this repo
  • Confirm ValidateIgnitionConfig is unreachable for local field inputs in both base/v0_7 and base/v0_8_exp
  • Confirm no test currently exercises the ValidateIgnitionConfig branch via a local field

Fedora packaging:

  • No response

GitHub release:

  • No response

Quay release:

  • No response

Metadata

Metadata

Assignees

No one assigned

    Labels

    butaneIssues originating from the Butane repository.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions