feat(commands): detect vendored charts/talm drift by content

lexfrei · lexfrei · commit dbc2a57f05da · 2026-06-05T14:31:10.000+03:00
CheckChartDrift compares a project's vendored charts/talm/ against the
binary's built-in copy via the chart-tree digest. The vendored tree is
read through an os.Root scoped to charts/talm/, confining traversal to
that subtree so a symlink inside it cannot redirect a read elsewhere.

The match is by content, with the Chart.yaml version stamp normalized
away on both sides: a binary version bump that left the library byte-
identical is not reported as drift. A project with no vendored library
stays silent rather than erroring, and a charts/talm that is a file
rather than a directory surfaces as an error (callers degrade it to a
non-fatal warning) instead of a crash.

The drift message points operators at `talm init --update --preset
&lt;preset&gt;` — the bare form cannot resolve the preset from an init'd
project's Chart.yaml and would not re-vendor. Config gains an opt-in
StrictCharts field (Chart.yaml strictCharts) for turning drift from a
warning into a hard error; absent keeps today's behavior.

Assisted-By: Claude &lt;noreply@anthropic.com&gt;
Signed-off-by: Aleksei Sviridkin &lt;f@lex.la&gt;
diff --git a/pkg/commands/chart_drift.go b/pkg/commands/chart_drift.go
@@ -0,0 +1,125 @@
+// Copyright Cozystack Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package commands
+
+import (
+	"fmt"
+	"io/fs"
+	"os"
+	"path"
+	"path/filepath"
+
+	"github.com/cockroachdb/errors"
+
+	"github.com/cozystack/talm/pkg/generated"
+)
+
+// vendoredTalmFiles reads a project's vendored talm library from
+// rootDir/charts/talm/, keyed by forward-slash path relative to that
+// directory (so the keys line up with the embedded TalmLibraryFiles output)
+// and with Chart.yaml metadata normalized the same way. The bool is false
+// when no vendored library exists — an unconfigured or freshly cloned
+// project the drift check should simply skip rather than treat as an error.
+//
+// Reads go through an os.Root rooted at charts/talm/, confining traversal to
+// that subtree so a symlink inside it cannot redirect a read outside the
+// project (gosec G122).
+func vendoredTalmFiles(rootDir string) (map[string]string, bool, error) {
+	base := filepath.Join(rootDir, "charts", "talm")
+
+	root, err := os.OpenRoot(base)
+	if err != nil {
+		if os.IsNotExist(err) {
+			return nil, false, nil
+		}
+
+		return nil, false, errors.Wrapf(err, "opening vendored talm library %q", base)
+	}
+	defer root.Close()
+
+	fsys := root.FS()
+	filesMap := make(map[string]string)
+
+	err = fs.WalkDir(fsys, ".", func(filePath string, entry fs.DirEntry, err error) error {
+		if err != nil {
+			return errors.Wrapf(err, "walking vendored talm library at %q", filePath)
+		}
+
+		if entry.IsDir() {
+			return nil
+		}
+
+		data, err := fs.ReadFile(fsys, filePath)
+		if err != nil {
+			return errors.Wrapf(err, "reading vendored talm file %q", filePath)
+		}
+
+		// root.FS() keys are already "/"-separated and relative to base, so
+		// they match the embedded TalmLibraryFiles output on every platform.
+		filesMap[filePath] = generated.NormalizeChartMeta(path.Base(filePath), string(data))
+
+		return nil
+	})
+	if err != nil {
+		return nil, false, errors.Wrap(err, "collecting vendored talm library")
+	}
+
+	return filesMap, true, nil
+}
+
+// CheckChartDrift reports whether a project's vendored charts/talm/ library
+// diverges, by content, from the copy built into this talm binary.
+//
+// talm vendors its library chart into the project at `talm init` time;
+// rendering (template/apply/upgrade) reads that local copy, never the
+// binary's embedded charts. Upgrading the binary therefore leaves
+// charts/talm/ frozen at the version that last ran init. CheckChartDrift
+// surfaces that staleness so the operator can re-run
+// `talm init --update --preset <preset>`.
+//
+// The comparison is by content: the Chart.yaml version stamp is normalized
+// away on both sides, so a pure version bump that left the library
+// byte-identical is NOT reported as drift. binaryVersion is used only for
+// the operator-facing message.
+//
+// It returns (false, "", nil) — staying silent — when the project has no
+// vendored library to compare. A read or walk failure is returned as an
+// error; callers treat drift detection as best-effort and must not block a
+// command on it.
+func CheckChartDrift(rootDir, binaryVersion string) (bool, string, error) {
+	vendored, ok, err := vendoredTalmFiles(rootDir)
+	if err != nil {
+		return false, "", err
+	}
+
+	if !ok {
+		return false, "", nil
+	}
+
+	embedded, err := generated.TalmLibraryFiles()
+	if err != nil {
+		return false, "", errors.Wrap(err, "loading embedded talm library")
+	}
+
+	if generated.HashChartFiles(vendored) == generated.HashChartFiles(embedded) {
+		return false, "", nil
+	}
+
+	return true, fmt.Sprintf(
+		"project's vendored charts/talm/ library differs from the copy built into talm %s; "+
+			"run `talm init --update --preset <preset>` to re-sync (or ignore if this is intentional)",
+		binaryVersion,
+	), nil
+}
diff --git a/pkg/commands/chart_drift_test.go b/pkg/commands/chart_drift_test.go
@@ -0,0 +1,160 @@
+// Copyright Cozystack Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package commands
+
+import (
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/cozystack/talm/pkg/generated"
+)
+
+// writeVendoredTalmLibrary materializes a project's charts/talm/ tree from
+// the binary's embedded library, stamping version into the library
+// Chart.yaml so the result looks like a real `talm init` would produce. It
+// returns the project root.
+func writeVendoredTalmLibrary(t *testing.T, version string) string {
+	t.Helper()
+
+	files, err := generated.TalmLibraryFiles()
+	if err != nil {
+		t.Fatalf("TalmLibraryFiles: %v", err)
+	}
+
+	root := t.TempDir()
+	for rel, content := range files {
+		// TalmLibraryFiles normalizes Chart.yaml name/version to %s; fill
+		// them back in (name, version) so the vendored copy mirrors init.
+		if filepath.Base(rel) == "Chart.yaml" {
+			content = strings.ReplaceAll(content, "name: %s", "name: talm")
+			content = strings.ReplaceAll(content, "version: %s", "version: "+version)
+		}
+
+		dest := filepath.Join(root, "charts", "talm", filepath.FromSlash(rel))
+		if err := os.MkdirAll(filepath.Dir(dest), 0o755); err != nil {
+			t.Fatalf("mkdir %q: %v", dest, err)
+		}
+		if err := os.WriteFile(dest, []byte(content), 0o644); err != nil {
+			t.Fatalf("write %q: %v", dest, err)
+		}
+	}
+
+	return root
+}
+
+// TestCheckChartDrift_MatchingCopy_NoDrift pins the happy path: a project
+// whose vendored charts/talm/ matches the embedded library reports no
+// drift, even though its Chart.yaml carries a concrete version stamp.
+func TestCheckChartDrift_MatchingCopy_NoDrift(t *testing.T) {
+	root := writeVendoredTalmLibrary(t, "0.30.0")
+
+	drift, msg, err := CheckChartDrift(root, "0.30.0")
+	if err != nil {
+		t.Fatalf("CheckChartDrift: %v", err)
+	}
+	if drift {
+		t.Errorf("reported drift for a matching vendored copy: %s", msg)
+	}
+}
+
+// TestCheckChartDrift_VersionStampOnly_NoDrift is the core regression
+// guard. A project vendored by an older release carries an older version
+// stamp in charts/talm/Chart.yaml, but its helpers are byte-identical to
+// the running binary's. That MUST NOT be reported as drift — flagging a
+// pure version difference is exactly the false positive the version-number
+// comparison approach produced.
+func TestCheckChartDrift_VersionStampOnly_NoDrift(t *testing.T) {
+	root := writeVendoredTalmLibrary(t, "0.1.0")
+
+	drift, msg, err := CheckChartDrift(root, "0.30.0")
+	if err != nil {
+		t.Fatalf("CheckChartDrift: %v", err)
+	}
+	if drift {
+		t.Errorf("reported drift for a version-only difference; this is a false positive: %s", msg)
+	}
+}
+
+// TestCheckChartDrift_ContentChange_Drift pins detection: a vendored
+// helpers template that diverges from the embedded copy is real drift and
+// must be surfaced.
+func TestCheckChartDrift_ContentChange_Drift(t *testing.T) {
+	root := writeVendoredTalmLibrary(t, "0.30.0")
+
+	helpers := filepath.Join(root, "charts", "talm", "templates", "_helpers.tpl")
+	data, err := os.ReadFile(helpers)
+	if err != nil {
+		t.Fatalf("read helpers: %v", err)
+	}
+	if err := os.WriteFile(helpers, append(data, []byte("\n{{- /* stale local edit */ -}}\n")...), 0o644); err != nil {
+		t.Fatalf("write helpers: %v", err)
+	}
+
+	drift, msg, err := CheckChartDrift(root, "0.30.0")
+	if err != nil {
+		t.Fatalf("CheckChartDrift: %v", err)
+	}
+	if !drift {
+		t.Error("did not detect a divergent vendored helpers template; real drift went unreported")
+	}
+	// The remediation must carry --preset: bare `talm init --update` cannot
+	// resolve the preset from an init'd project's Chart.yaml and errors out
+	// without re-vendoring, so pointing at it would send operators to a
+	// command that does not clear the drift.
+	if !strings.Contains(msg, "talm init --update --preset") {
+		t.Errorf("drift message must point at `talm init --update --preset`; got %q", msg)
+	}
+}
+
+// TestCheckChartDrift_VendoredTalmIsFile_ErrorsGracefully pins that a
+// charts/talm that exists as a file (not a directory) is surfaced as an
+// error rather than a panic or a spurious drift result. Callers
+// (evaluateChartDrift) downgrade this to a non-fatal warning, so a
+// malformed project never crashes a command.
+func TestCheckChartDrift_VendoredTalmIsFile_ErrorsGracefully(t *testing.T) {
+	root := t.TempDir()
+	if err := os.MkdirAll(filepath.Join(root, "charts"), 0o755); err != nil {
+		t.Fatalf("mkdir: %v", err)
+	}
+	if err := os.WriteFile(filepath.Join(root, "charts", "talm"), []byte("not a directory"), 0o644); err != nil {
+		t.Fatalf("write file: %v", err)
+	}
+
+	drift, _, err := CheckChartDrift(root, "0.30.0")
+	if err == nil {
+		t.Error("expected an error when charts/talm is a file, got nil")
+	}
+	if drift {
+		t.Error("must not report drift when the vendored tree cannot be read")
+	}
+}
+
+// TestCheckChartDrift_MissingVendoredDir_NoDriftNoError pins graceful
+// handling: a project without a charts/talm/ directory (nothing vendored
+// yet) yields no drift and no error, so the check never blocks a command
+// on a tree it cannot compare.
+func TestCheckChartDrift_MissingVendoredDir_NoDriftNoError(t *testing.T) {
+	root := t.TempDir()
+
+	drift, msg, err := CheckChartDrift(root, "0.30.0")
+	if err != nil {
+		t.Fatalf("CheckChartDrift returned an error for a missing vendored dir: %v", err)
+	}
+	if drift {
+		t.Errorf("reported drift with no vendored library to compare: %s", msg)
+	}
+}
diff --git a/pkg/commands/root.go b/pkg/commands/root.go
@@ -42,7 +42,12 @@ var GlobalArgs global.Args
 var Config struct {
 	RootDir         string
 	RootDirExplicit bool // true if --root was explicitly set
-	GlobalOptions   struct {
+	// StrictCharts turns vendored-chart drift into a hard error instead of a
+	// warning. Opt-in per project via Chart.yaml (strictCharts: true) so a
+	// whole team/CI inherits it; absent means a warning only (the historical
+	// behavior). The --strict-charts flag forces it on for a single run.
+	StrictCharts  bool `yaml:"strictCharts"`
+	GlobalOptions struct {
 		Talosconfig string `yaml:"talosconfig"`
 		Kubeconfig  string `yaml:"kubeconfig"`
 	} `yaml:"globalOptions"`
