From e98a25134a6af1fd452e98a9e37a8a193181b935 Mon Sep 17 00:00:00 2001 From: Kartik Suryavanshi <158498247+KartikSuryavanshi@users.noreply.github.com> Date: Thu, 25 Jun 2026 12:09:55 +0530 Subject: [PATCH 1/5] fix(sympath): skip broken symlinks instead of erroring When sympath.Walk encounters a broken symlink whose target does not exist, silently skip it instead of returning an error. Other EvalSymlinks errors (e.g. permission denied, symlink loops) are still propagated. Broken symlinks cannot contribute useful content, and this prevents errors when they are listed in .helmignore patterns. Fixes #13284 Signed-off-by: Kartik Suryavanshi <158498247+KartikSuryavanshi@users.noreply.github.com> --- internal/chart/v3/loader/load_test.go | 50 +++++++++++++++++++++++++++ internal/sympath/walk.go | 10 ++++-- pkg/chart/v2/loader/load_test.go | 50 +++++++++++++++++++++++++++ 3 files changed, 108 insertions(+), 2 deletions(-) diff --git a/internal/chart/v3/loader/load_test.go b/internal/chart/v3/loader/load_test.go index de9219a1b..9b8762e51 100644 --- a/internal/chart/v3/loader/load_test.go +++ b/internal/chart/v3/loader/load_test.go @@ -90,6 +90,56 @@ func TestLoadDirWithSymlink(t *testing.T) { verifyDependenciesLock(t, c) } +func TestLoadDirWithBrokenSymlinkInHelmignore(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("symlink tests require unix") + } + + tmpDir := t.TempDir() + + // Create minimal chart structure + chartYAML := `apiVersion: v2 +name: test +version: 0.1.0 +` + valuesYAML := `{} +` + tpl := `config: {} +` + if err := os.WriteFile(filepath.Join(tmpDir, "Chart.yaml"), []byte(chartYAML), 0644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(tmpDir, "values.yaml"), []byte(valuesYAML), 0644); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(filepath.Join(tmpDir, "templates"), 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(tmpDir, "templates", "config.yaml"), []byte(tpl), 0644); err != nil { + t.Fatal(err) + } + + // Create a broken symlink in templates/ + brokenLink := filepath.Join(tmpDir, "templates", "broken") + if err := os.Symlink("nonexistent", brokenLink); err != nil { + t.Fatal(err) + } + + // Add the broken symlink to .helmignore + if err := os.WriteFile(filepath.Join(tmpDir, ".helmignore"), []byte("templates/broken\n"), 0644); err != nil { + t.Fatal(err) + } + + // Loading should succeed despite the broken symlink + l, err := Loader(tmpDir) + if err != nil { + t.Fatal(err) + } + if _, err := l.Load(); err != nil { + t.Fatalf("loading chart with broken symlink in .helmignore should not fail: %s", err) + } +} + func TestBomTestData(t *testing.T) { testFiles := []string{"frobnitz_with_bom/.helmignore", "frobnitz_with_bom/templates/template.tpl", "frobnitz_with_bom/Chart.yaml"} for _, file := range testFiles { diff --git a/internal/sympath/walk.go b/internal/sympath/walk.go index 812bb68ce..4aca38693 100644 --- a/internal/sympath/walk.go +++ b/internal/sympath/walk.go @@ -21,7 +21,6 @@ limitations under the License. package sympath import ( - "fmt" "log/slog" "os" "path/filepath" @@ -68,7 +67,14 @@ func symwalk(path string, info os.FileInfo, walkFn filepath.WalkFunc) error { if IsSymlink(info) { resolved, err := filepath.EvalSymlinks(path) if err != nil { - return fmt.Errorf("error evaluating symlink %s: %w", path, err) + if os.IsNotExist(err) { + // If a symlink's target does not exist (broken symlink), + // silently skip it. Broken symlinks do not contribute + // content and should not cause errors, especially when + // they match .helmignore patterns. + return nil + } + return err } // This log message is to highlight a symlink that is being used within a chart, symlinks can be used for nefarious reasons. slog.Info("found symbolic link in path. Contents of linked file included and used", "path", path, "resolved", resolved) diff --git a/pkg/chart/v2/loader/load_test.go b/pkg/chart/v2/loader/load_test.go index be7041386..56954b16a 100644 --- a/pkg/chart/v2/loader/load_test.go +++ b/pkg/chart/v2/loader/load_test.go @@ -90,6 +90,56 @@ func TestLoadDirWithSymlink(t *testing.T) { verifyDependenciesLock(t, c) } +func TestLoadDirWithBrokenSymlinkInHelmignore(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("symlink tests require unix") + } + + tmpDir := t.TempDir() + + // Create minimal chart structure + chartYAML := `apiVersion: v2 +name: test +version: 0.1.0 +` + valuesYAML := `{} +` + tpl := `config: {} +` + if err := os.WriteFile(filepath.Join(tmpDir, "Chart.yaml"), []byte(chartYAML), 0644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(tmpDir, "values.yaml"), []byte(valuesYAML), 0644); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(filepath.Join(tmpDir, "templates"), 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(tmpDir, "templates", "config.yaml"), []byte(tpl), 0644); err != nil { + t.Fatal(err) + } + + // Create a broken symlink in templates/ + brokenLink := filepath.Join(tmpDir, "templates", "broken") + if err := os.Symlink("nonexistent", brokenLink); err != nil { + t.Fatal(err) + } + + // Add the broken symlink to .helmignore + if err := os.WriteFile(filepath.Join(tmpDir, ".helmignore"), []byte("templates/broken\n"), 0644); err != nil { + t.Fatal(err) + } + + // Loading should succeed despite the broken symlink + l, err := Loader(tmpDir) + if err != nil { + t.Fatal(err) + } + if _, err := l.Load(); err != nil { + t.Fatalf("loading chart with broken symlink in .helmignore should not fail: %s", err) + } +} + func TestBomTestData(t *testing.T) { testFiles := []string{"frobnitz_with_bom/.helmignore", "frobnitz_with_bom/templates/template.tpl", "frobnitz_with_bom/Chart.yaml"} for _, file := range testFiles { From 3d42cb4c6a46c04bc4e50b69dffa2aacc8b5810f Mon Sep 17 00:00:00 2001 From: Kartik Suryavanshi <158498247+KartikSuryavanshi@users.noreply.github.com> Date: Wed, 15 Jul 2026 19:58:20 +0530 Subject: [PATCH 2/5] fix: only skip broken symlinks when matched by .helmignore Instead of silently skipping all broken symlinks unconditionally, pass the error to the walkFn callback so chart loaders can check .helmignore patterns before deciding whether to error. - internal/sympath/walk.go: call walkFn with the error instead of returning nil for broken symlinks - pkg/chart/v2/loader/directory.go: skip broken symlink errors only when the file matches a .helmignore rule - internal/chart/v3/loader/directory.go: same fix for v3 charts - Add TestLoadDirWithBrokenSymlinkNotInHelmignore to verify that broken symlinks not in .helmignore still cause an error Signed-off-by: Kartik Suryavanshi Signed-off-by: Kartik Suryavanshi <158498247+KartikSuryavanshi@users.noreply.github.com> --- internal/chart/v3/loader/directory.go | 4 +++ internal/chart/v3/loader/load_test.go | 40 +++++++++++++++++++++++++++ internal/sympath/walk.go | 9 +++--- pkg/chart/v2/loader/directory.go | 4 +++ pkg/chart/v2/loader/load_test.go | 40 +++++++++++++++++++++++++++ 5 files changed, 92 insertions(+), 5 deletions(-) diff --git a/internal/chart/v3/loader/directory.go b/internal/chart/v3/loader/directory.go index dfe3af3b2..ff3926935 100644 --- a/internal/chart/v3/loader/directory.go +++ b/internal/chart/v3/loader/directory.go @@ -77,6 +77,10 @@ func LoadDir(dir string) (*chart.Chart, error) { n = filepath.ToSlash(n) if err != nil { + // For broken symlinks, respect .helmignore before erroring. + if os.IsNotExist(err) && fi != nil && rules.Ignore(n, fi) { + return nil + } return err } if fi.IsDir() { diff --git a/internal/chart/v3/loader/load_test.go b/internal/chart/v3/loader/load_test.go index 9b8762e51..8f14d56f1 100644 --- a/internal/chart/v3/loader/load_test.go +++ b/internal/chart/v3/loader/load_test.go @@ -90,6 +90,46 @@ func TestLoadDirWithSymlink(t *testing.T) { verifyDependenciesLock(t, c) } +func TestLoadDirWithBrokenSymlinkNotInHelmignore(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("symlink tests require unix") + } + + tmpDir := t.TempDir() + + chartYAML := `apiVersion: v2 +name: test +version: 0.1.0 +` + valuesYAML := `{} +` + if err := os.WriteFile(filepath.Join(tmpDir, "Chart.yaml"), []byte(chartYAML), 0644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(tmpDir, "values.yaml"), []byte(valuesYAML), 0644); err != nil { + t.Fatal(err) + } + + // Create a broken symlink NOT listed in .helmignore + brokenLink := filepath.Join(tmpDir, "broken") + if err := os.Symlink("nonexistent", brokenLink); err != nil { + t.Fatal(err) + } + + // Empty .helmignore — broken symlink is not ignored + if err := os.WriteFile(filepath.Join(tmpDir, ".helmignore"), []byte(""), 0644); err != nil { + t.Fatal(err) + } + + l, err := Loader(tmpDir) + if err != nil { + t.Fatal(err) + } + if _, err := l.Load(); err == nil { + t.Fatal("loading chart with broken symlink not in .helmignore should fail") + } +} + func TestLoadDirWithBrokenSymlinkInHelmignore(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("symlink tests require unix") diff --git a/internal/sympath/walk.go b/internal/sympath/walk.go index 4aca38693..8eca7da47 100644 --- a/internal/sympath/walk.go +++ b/internal/sympath/walk.go @@ -68,11 +68,10 @@ func symwalk(path string, info os.FileInfo, walkFn filepath.WalkFunc) error { resolved, err := filepath.EvalSymlinks(path) if err != nil { if os.IsNotExist(err) { - // If a symlink's target does not exist (broken symlink), - // silently skip it. Broken symlinks do not contribute - // content and should not cause errors, especially when - // they match .helmignore patterns. - return nil + // Pass the broken symlink error to walkFn so callers + // (e.g. chart loaders) can decide whether to skip it + // based on .helmignore rules. + return walkFn(path, info, err) } return err } diff --git a/pkg/chart/v2/loader/directory.go b/pkg/chart/v2/loader/directory.go index 82578d924..8fd0b516b 100644 --- a/pkg/chart/v2/loader/directory.go +++ b/pkg/chart/v2/loader/directory.go @@ -77,6 +77,10 @@ func LoadDir(dir string) (*chart.Chart, error) { n = filepath.ToSlash(n) if err != nil { + // For broken symlinks, respect .helmignore before erroring. + if os.IsNotExist(err) && fi != nil && rules.Ignore(n, fi) { + return nil + } return err } if fi.IsDir() { diff --git a/pkg/chart/v2/loader/load_test.go b/pkg/chart/v2/loader/load_test.go index 56954b16a..8f61b3632 100644 --- a/pkg/chart/v2/loader/load_test.go +++ b/pkg/chart/v2/loader/load_test.go @@ -90,6 +90,46 @@ func TestLoadDirWithSymlink(t *testing.T) { verifyDependenciesLock(t, c) } +func TestLoadDirWithBrokenSymlinkNotInHelmignore(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("symlink tests require unix") + } + + tmpDir := t.TempDir() + + chartYAML := `apiVersion: v2 +name: test +version: 0.1.0 +` + valuesYAML := `{} +` + if err := os.WriteFile(filepath.Join(tmpDir, "Chart.yaml"), []byte(chartYAML), 0644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(tmpDir, "values.yaml"), []byte(valuesYAML), 0644); err != nil { + t.Fatal(err) + } + + // Create a broken symlink NOT listed in .helmignore + brokenLink := filepath.Join(tmpDir, "broken") + if err := os.Symlink("nonexistent", brokenLink); err != nil { + t.Fatal(err) + } + + // Empty .helmignore — broken symlink is not ignored + if err := os.WriteFile(filepath.Join(tmpDir, ".helmignore"), []byte(""), 0644); err != nil { + t.Fatal(err) + } + + l, err := Loader(tmpDir) + if err != nil { + t.Fatal(err) + } + if _, err := l.Load(); err == nil { + t.Fatal("loading chart with broken symlink not in .helmignore should fail") + } +} + func TestLoadDirWithBrokenSymlinkInHelmignore(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("symlink tests require unix") From 8bc1c04c11dd7c5210831237d716f18130d7a530 Mon Sep 17 00:00:00 2001 From: Kartik Suryavanshi <158498247+KartikSuryavanshi@users.noreply.github.com> Date: Wed, 15 Jul 2026 21:07:40 +0530 Subject: [PATCH 3/5] fix: address review feedback - Pass all symlink evaluation errors through walkFn for consistency with the Walk contract (all errors are filtered by walkFn) - Tighten test assertions to verify the error is specifically os.IsNotExist, not just any error Signed-off-by: Kartik Suryavanshi Signed-off-by: Kartik Suryavanshi <158498247+KartikSuryavanshi@users.noreply.github.com> --- internal/chart/v3/loader/load_test.go | 4 ++-- internal/sympath/walk.go | 10 +++------- pkg/chart/v2/loader/load_test.go | 4 ++-- 3 files changed, 7 insertions(+), 11 deletions(-) diff --git a/internal/chart/v3/loader/load_test.go b/internal/chart/v3/loader/load_test.go index 8f14d56f1..70d30b645 100644 --- a/internal/chart/v3/loader/load_test.go +++ b/internal/chart/v3/loader/load_test.go @@ -125,8 +125,8 @@ version: 0.1.0 if err != nil { t.Fatal(err) } - if _, err := l.Load(); err == nil { - t.Fatal("loading chart with broken symlink not in .helmignore should fail") + if _, err := l.Load(); err == nil || !os.IsNotExist(err) { + t.Fatalf("expected broken symlink error (os.IsNotExist), got: %v", err) } } diff --git a/internal/sympath/walk.go b/internal/sympath/walk.go index 8eca7da47..0da7b1fc8 100644 --- a/internal/sympath/walk.go +++ b/internal/sympath/walk.go @@ -67,13 +67,9 @@ func symwalk(path string, info os.FileInfo, walkFn filepath.WalkFunc) error { if IsSymlink(info) { resolved, err := filepath.EvalSymlinks(path) if err != nil { - if os.IsNotExist(err) { - // Pass the broken symlink error to walkFn so callers - // (e.g. chart loaders) can decide whether to skip it - // based on .helmignore rules. - return walkFn(path, info, err) - } - return err + // Pass the error to walkFn so callers (e.g. chart loaders) + // can decide whether to skip it based on .helmignore rules. + return walkFn(path, info, err) } // This log message is to highlight a symlink that is being used within a chart, symlinks can be used for nefarious reasons. slog.Info("found symbolic link in path. Contents of linked file included and used", "path", path, "resolved", resolved) diff --git a/pkg/chart/v2/loader/load_test.go b/pkg/chart/v2/loader/load_test.go index 8f61b3632..33893450d 100644 --- a/pkg/chart/v2/loader/load_test.go +++ b/pkg/chart/v2/loader/load_test.go @@ -125,8 +125,8 @@ version: 0.1.0 if err != nil { t.Fatal(err) } - if _, err := l.Load(); err == nil { - t.Fatal("loading chart with broken symlink not in .helmignore should fail") + if _, err := l.Load(); err == nil || !os.IsNotExist(err) { + t.Fatalf("expected broken symlink error (os.IsNotExist), got: %v", err) } } From 67e71e18edbd6f49d401d4707bdfe1628df8dabc Mon Sep 17 00:00:00 2001 From: Kartik Suryavanshi <158498247+KartikSuryavanshi@users.noreply.github.com> Date: Wed, 15 Jul 2026 21:29:36 +0530 Subject: [PATCH 4/5] fix: address review feedback on error routing - Route os.Lstat(resolved) errors through walkFn with original symlink info, so callers can decide whether to skip based on .helmignore - Keep info reassignment from Lstat to prevent infinite recursion on symlink-to-directory paths Signed-off-by: Kartik Suryavanshi Signed-off-by: Kartik Suryavanshi <158498247+KartikSuryavanshi@users.noreply.github.com> --- internal/sympath/walk.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/sympath/walk.go b/internal/sympath/walk.go index 0da7b1fc8..caacec306 100644 --- a/internal/sympath/walk.go +++ b/internal/sympath/walk.go @@ -74,7 +74,9 @@ func symwalk(path string, info os.FileInfo, walkFn filepath.WalkFunc) error { // This log message is to highlight a symlink that is being used within a chart, symlinks can be used for nefarious reasons. slog.Info("found symbolic link in path. Contents of linked file included and used", "path", path, "resolved", resolved) if info, err = os.Lstat(resolved); err != nil { - return err + // Route through walkFn with the original symlink info so callers + // can decide whether to skip it based on .helmignore rules. + return walkFn(path, info, err) } if err := symwalk(path, info, walkFn); err != nil && err != filepath.SkipDir { return err From bf2c8c4af07251393df4ba7e65644b2f8ee6ebab Mon Sep 17 00:00:00 2001 From: Kartik Suryavanshi <158498247+KartikSuryavanshi@users.noreply.github.com> Date: Wed, 15 Jul 2026 22:03:27 +0530 Subject: [PATCH 5/5] fix: preserve original symlink FileInfo on Lstat failure Save the original symlink info before os.Lstat(resolved) overwrites it, so walkFn receives the symlink's FileInfo (not nil) on error. This allows .helmignore pattern matching to work correctly for broken symlinks. Signed-off-by: Kartik Suryavanshi Signed-off-by: Kartik Suryavanshi <158498247+KartikSuryavanshi@users.noreply.github.com> --- internal/sympath/walk.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/sympath/walk.go b/internal/sympath/walk.go index caacec306..41b6cd6f0 100644 --- a/internal/sympath/walk.go +++ b/internal/sympath/walk.go @@ -73,10 +73,11 @@ func symwalk(path string, info os.FileInfo, walkFn filepath.WalkFunc) error { } // This log message is to highlight a symlink that is being used within a chart, symlinks can be used for nefarious reasons. slog.Info("found symbolic link in path. Contents of linked file included and used", "path", path, "resolved", resolved) + originalInfo := info if info, err = os.Lstat(resolved); err != nil { // Route through walkFn with the original symlink info so callers // can decide whether to skip it based on .helmignore rules. - return walkFn(path, info, err) + return walkFn(path, originalInfo, err) } if err := symwalk(path, info, walkFn); err != nil && err != filepath.SkipDir { return err