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] 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) } }