From 0211dc877b962111a3b2050231f86b18d42d34bf Mon Sep 17 00:00:00 2001
From: Ankit Pramanik <59945244+ankit98040@users.noreply.github.com>
Date: Thu, 28 May 2026 12:12:56 +0530
Subject: [PATCH] fix(engine): propagate and handle chart accessor errors

Ensure that errors returned by NewAccessor are propagated and handled correctly rather than ignored, which prevents a potential nil-pointer dereference panic in recAllTpls. Added TestRenderInvalidChartType to prevent future regressions.

Signed-off-by: Ankit Pramanik <59945244+ankit98040@users.noreply.github.com>
---
 pkg/engine/engine.go      | 31 +++++++++++++++++++++----------
 pkg/engine/engine_test.go | 14 +++++++++++++-
 2 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go
index 6fd2beed8..00d1a5a64 100644
--- a/pkg/engine/engine.go
+++ b/pkg/engine/engine.go
@@ -77,7 +77,10 @@ func New(config *rest.Config) Engine {
 // section contains a value named "bar", that value will be passed on to the
 // bar chart during render time.
 func (e Engine) Render(chrt ci.Charter, values common.Values) (map[string]string, error) {
-	tmap := allTemplates(chrt, values)
+	tmap, err := allTemplates(chrt, values)
+	if err != nil {
+		return nil, err
+	}
 	return e.render(tmap)
 }
 
@@ -521,22 +524,24 @@ func (p byPathLen) Less(i, j int) bool {
 // allTemplates returns all templates for a chart and its dependencies.
 //
 // As it goes, it also prepares the values in a scope-sensitive manner.
-func allTemplates(c ci.Charter, vals common.Values) map[string]renderable {
+func allTemplates(c ci.Charter, vals common.Values) (map[string]renderable, error) {
 	templates := make(map[string]renderable)
-	recAllTpls(c, templates, vals)
-	return templates
+	if _, err := recAllTpls(c, templates, vals); err != nil {
+		return nil, err
+	}
+	return templates, nil
 }
 
 // recAllTpls recurses through the templates in a chart.
 //
 // As it recurses, it also sets the values to be appropriate for the template
 // scope.
-func recAllTpls(c ci.Charter, templates map[string]renderable, values common.Values) map[string]any {
+func recAllTpls(c ci.Charter, templates map[string]renderable, values common.Values) (map[string]any, error) {
 	vals := values.AsMap()
 	subCharts := make(map[string]any)
 	accessor, err := ci.NewAccessor(c)
 	if err != nil {
-		slog.Error("error accessing chart", "error", err)
+		return nil, fmt.Errorf("error accessing chart: %w", err)
 	}
 	chartMetaData := accessor.MetadataAsMap()
 	chartMetaData["IsRoot"] = accessor.IsRoot()
@@ -559,9 +564,15 @@ func recAllTpls(c ci.Charter, templates map[string]renderable, values common.Val
 	}
 
 	for _, child := range accessor.Dependencies() {
-		// TODO: Handle error
-		sub, _ := ci.NewAccessor(child)
-		subCharts[sub.Name()] = recAllTpls(child, templates, next)
+		sub, err := ci.NewAccessor(child)
+		if err != nil {
+			return nil, fmt.Errorf("error accessing dependency chart: %w", err)
+		}
+		subTpls, err := recAllTpls(child, templates, next)
+		if err != nil {
+			return nil, err
+		}
+		subCharts[sub.Name()] = subTpls
 	}
 
 	newParentID := accessor.ChartFullPath()
@@ -579,7 +590,7 @@ func recAllTpls(c ci.Charter, templates map[string]renderable, values common.Val
 		}
 	}
 
-	return next
+	return next, nil
 }
 
 // isTemplateValid returns true if the template is valid for the chart type
diff --git a/pkg/engine/engine_test.go b/pkg/engine/engine_test.go
index 869b5d202..c87932555 100644
--- a/pkg/engine/engine_test.go
+++ b/pkg/engine/engine_test.go
@@ -590,12 +590,24 @@ func TestAllTemplates(t *testing.T) {
 	}
 	dep1.AddDependency(dep2)
 
-	tpls := allTemplates(ch1, common.Values{})
+	tpls, err := allTemplates(ch1, common.Values{})
+	if err != nil {
+		t.Fatalf("Unexpected error: %s", err)
+	}
 	if len(tpls) != 5 {
 		t.Errorf("Expected 5 charts, got %d", len(tpls))
 	}
 }
 
+func TestRenderInvalidChartType(t *testing.T) {
+	_, err := Render(struct{}{}, common.Values{})
+	if err == nil {
+		t.Error("Expected error when rendering invalid chart type, got nil")
+	} else if !strings.Contains(err.Error(), "unsupported chart type") {
+		t.Errorf("Expected error to contain 'unsupported chart type', got: %v", err)
+	}
+}
+
 func TestChartValuesContainsIsRoot(t *testing.T) {
 	modTime := time.Now()
 	ch1 := &chart.Chart{