From 7f5327d5bb4db97edb6407bfdc1ee8f19d9a7b6a Mon Sep 17 00:00:00 2001 From: Andrey Smirnov Date: Fri, 11 Sep 2026 20:31:09 +0400 Subject: [PATCH] fix: guard against nil config document slices This imports the gen library helper https://github.com/siderolabs/gen/pull/34, and uses that to decode any machine config document, guarding against various `[]*T` places being nil. Remove now unneded validation (which was incomplete anyways). This protection applies to all code paths, including config patches. Signed-off-by: Andrey Smirnov --- go.mod | 2 +- go.sum | 4 +- .../configloader/internal/decoder/decoder.go | 5 ++ .../internal/decoder/decoder_test.go | 59 +++++++++++++++++++ .../types/v1alpha1/v1alpha1_validation.go | 20 +------ pkg/machinery/go.mod | 2 +- pkg/machinery/go.sum | 4 +- 7 files changed, 71 insertions(+), 25 deletions(-) diff --git a/go.mod b/go.mod index 885838f7c3a..8cab0514022 100644 --- a/go.mod +++ b/go.mod @@ -145,7 +145,7 @@ require ( github.com/siderolabs/crypto v0.6.5 github.com/siderolabs/discovery-api v0.1.8 github.com/siderolabs/discovery-client v0.1.15 - github.com/siderolabs/gen v0.8.7 + github.com/siderolabs/gen v0.8.8 github.com/siderolabs/go-adv v1.0.0 github.com/siderolabs/go-blockdevice/v2 v2.0.32 github.com/siderolabs/go-circular v0.2.3 diff --git a/go.sum b/go.sum index add230be459..21341ea5c0d 100644 --- a/go.sum +++ b/go.sum @@ -1451,8 +1451,8 @@ github.com/siderolabs/discovery-client v0.1.15/go.mod h1:iUpFYp40CNTnqshG7d2r9zj github.com/siderolabs/ethtool v0.6.0-sidero h1:0LVd+VWgwGo1d5DEJiJSY+rW3R6P5Sp1qHKKqMahDJI= github.com/siderolabs/ethtool v0.6.0-sidero/go.mod h1:pZdvfpIRgnKmcS2kDJSqE9AjyPEQORZ3O3tKsJzkcwA= github.com/siderolabs/gen v0.8.6/go.mod h1:J9IbusbES2W6QWjtSHpDV9iPGZHc978h1+KJ4oQRspQ= -github.com/siderolabs/gen v0.8.7 h1:Nu31kL0ln/facRHBfNX7zcB7w9VZ9tifXKsP4lUtWHw= -github.com/siderolabs/gen v0.8.7/go.mod h1:J9IbusbES2W6QWjtSHpDV9iPGZHc978h1+KJ4oQRspQ= +github.com/siderolabs/gen v0.8.8 h1:wDDlHAT7q4JYr1nD6I8iJml2jUgMKKuCpd05W5UoM+A= +github.com/siderolabs/gen v0.8.8/go.mod h1:iOCzLzCgiHas+4KIJ0KaLJUjIXL/SVH1ZXZUXBt04tc= github.com/siderolabs/go-adv v1.0.0 h1:ZWXnoGq1GKeEIkLSR4o6oKcayFsowZkJsWcyQqwPk6c= github.com/siderolabs/go-adv v1.0.0/go.mod h1:nR6YwduJv56mZI1D3ow1Zok5lwefiM94hS0o1d6KmNc= github.com/siderolabs/go-api-signature v0.3.13 h1:1u3vOWpn4PJJcQZCQXXXxeZk+HcoRdXCpvojE3q5Q9k= diff --git a/pkg/machinery/config/configloader/internal/decoder/decoder.go b/pkg/machinery/config/configloader/internal/decoder/decoder.go index 278a00fbb8d..efa4a59bd80 100644 --- a/pkg/machinery/config/configloader/internal/decoder/decoder.go +++ b/pkg/machinery/config/configloader/internal/decoder/decoder.go @@ -199,5 +199,10 @@ func decode(manifest *yaml.Node) (target config.Document, err error) { return nil, err } + // a null entry in a list (or a map) of pointers decodes to a nil element which the code doesn't expect + if err = xyaml.CheckNullElements(target, manifest); err != nil { + return nil, err + } + return target, nil } diff --git a/pkg/machinery/config/configloader/internal/decoder/decoder_test.go b/pkg/machinery/config/configloader/internal/decoder/decoder_test.go index 61656b7fe55..7bd43956646 100644 --- a/pkg/machinery/config/configloader/internal/decoder/decoder_test.go +++ b/pkg/machinery/config/configloader/internal/decoder/decoder_test.go @@ -50,6 +50,7 @@ type MockV2 struct { Meta Slice []Mock `yaml:"slice"` + Ptrs []*Mock `yaml:"ptrs"` Map map[string]*Mock `yaml:"map"` } @@ -227,6 +228,64 @@ map: expected: nil, expectedErr: "error decoding document v1alpha2/mock/ (line 2): unknown keys found during decoding:\nmap:\n second:\n a:\n b: {}\n", }, + { + name: "null in slice of pointers", + source: []byte(`--- +kind: mock +apiVersion: v1alpha2 +ptrs: + - test: true + - + - null +`), + expected: nil, + expectedErr: "error decoding document v1alpha2/mock/ (line 2): null value is not allowed at \"ptrs[1]\" (line 6)\nnull value is not allowed at \"ptrs[2]\" (line 7)", + }, + { + name: "null in map of pointers", + source: []byte(`--- +kind: mock +apiVersion: v1alpha2 +map: + first: null + second: + test: true +`), + expected: nil, + expectedErr: "error decoding document v1alpha2/mock/ (line 2): null value is not allowed at \"map.first\" (line 5)", + }, + { + // the YAML library drops a null item when decoding into a slice of values, so there's nothing to reject + name: "null in slice of values", + source: []byte(`--- +kind: mock +apiVersion: v1alpha2 +slice: + - null + - test: true +`), + expected: []config.Document{ + &MockV2{ + Slice: []Mock{{Test: true}}, + }, + }, + }, + { + name: "null in v1alpha1 network interfaces", + source: []byte(`--- +version: v1alpha1 +machine: + network: + interfaces: + - interface: eth0 + vlans: + - null + - null +`), + expected: nil, + expectedErr: "error decoding document /v1alpha1/ (line 2): null value is not allowed at \"machine.network.interfaces[0].vlans[0]\"" + + " (line 8)\nnull value is not allowed at \"machine.network.interfaces[1]\" (line 9)", + }, { name: "valid nested", source: []byte(`--- diff --git a/pkg/machinery/config/types/v1alpha1/v1alpha1_validation.go b/pkg/machinery/config/types/v1alpha1/v1alpha1_validation.go index 609453020e3..8e3b8b9f7ca 100644 --- a/pkg/machinery/config/types/v1alpha1/v1alpha1_validation.go +++ b/pkg/machinery/config/types/v1alpha1/v1alpha1_validation.go @@ -224,13 +224,7 @@ func (c *Config) Validate(mode validation.RuntimeMode, options ...validation.Opt } } - for i, disk := range c.MachineConfig.MachineDisks { - if disk == nil { - result = multierror.Append(result, fmt.Errorf("machine.disks[%d] is null", i)) - - continue - } - + for _, disk := range c.MachineConfig.MachineDisks { for i, pt := range disk.DiskPartitions { if pt.DiskSize == 0 && i != len(disk.DiskPartitions)-1 { result = multierror.Append(result, fmt.Errorf("partition for disk %q is set to occupy full disk, but it's not the last partition in the list", disk.Device())) @@ -354,18 +348,6 @@ func (c *Config) Validate(mode validation.RuntimeMode, options ...validation.Opt } } - for key, val := range c.MachineConfig.MachineRegistries.RegistryConfig { - if val == nil { - result = multierror.Append(result, fmt.Errorf("registries.config[%q] is null", key)) - } - } - - for key, val := range c.MachineConfig.MachineRegistries.RegistryMirrors { - if val == nil { - result = multierror.Append(result, fmt.Errorf("registries.mirrors[%q] is null", key)) - } - } - // don't validate Kubernetes version in local mode, as it depends on Talos version if !opts.Local { result = multierror.Append(result, c.ValidateKubernetesVersions()) diff --git a/pkg/machinery/go.mod b/pkg/machinery/go.mod index 90f052a48aa..5acc6c0d833 100644 --- a/pkg/machinery/go.mod +++ b/pkg/machinery/go.mod @@ -24,7 +24,7 @@ require ( github.com/ryanuber/go-glob v1.0.0 github.com/santhosh-tekuri/jsonschema/v6 v6.0.3 github.com/siderolabs/crypto v0.6.5 - github.com/siderolabs/gen v0.8.7 + github.com/siderolabs/gen v0.8.8 github.com/siderolabs/go-api-signature v0.3.13 github.com/siderolabs/go-pointer v1.0.1 github.com/siderolabs/net v0.4.0 diff --git a/pkg/machinery/go.sum b/pkg/machinery/go.sum index 818678290b2..d9ecc484afc 100644 --- a/pkg/machinery/go.sum +++ b/pkg/machinery/go.sum @@ -102,8 +102,8 @@ github.com/siderolabs/crypto v0.6.5 h1:Elq5tpWP2ApZ4Y+Kg+eIDiiWbmriCPI1mjYIMwvsY github.com/siderolabs/crypto v0.6.5/go.mod h1:QjVcrdJQE1sxhjHqieCgwGdlIYq/xCP2DL53Up8nbU4= github.com/siderolabs/ethtool v0.6.0-sidero h1:0LVd+VWgwGo1d5DEJiJSY+rW3R6P5Sp1qHKKqMahDJI= github.com/siderolabs/ethtool v0.6.0-sidero/go.mod h1:pZdvfpIRgnKmcS2kDJSqE9AjyPEQORZ3O3tKsJzkcwA= -github.com/siderolabs/gen v0.8.7 h1:Nu31kL0ln/facRHBfNX7zcB7w9VZ9tifXKsP4lUtWHw= -github.com/siderolabs/gen v0.8.7/go.mod h1:J9IbusbES2W6QWjtSHpDV9iPGZHc978h1+KJ4oQRspQ= +github.com/siderolabs/gen v0.8.8 h1:wDDlHAT7q4JYr1nD6I8iJml2jUgMKKuCpd05W5UoM+A= +github.com/siderolabs/gen v0.8.8/go.mod h1:iOCzLzCgiHas+4KIJ0KaLJUjIXL/SVH1ZXZUXBt04tc= github.com/siderolabs/go-api-signature v0.3.13 h1:1u3vOWpn4PJJcQZCQXXXxeZk+HcoRdXCpvojE3q5Q9k= github.com/siderolabs/go-api-signature v0.3.13/go.mod h1:gfAm/sYbkxAR6YAH+72dhK46nGwNM/O985y8btLjMhQ= github.com/siderolabs/go-pointer v1.0.1 h1:f7Yi4IK1jptS8yrT9GEbwhmGcVxvPQgBUG/weH3V3DM=