fix: eliminate race condition in rewriteChartDependencies (#2541)

* fix: eliminate race condition in rewriteChartDependencies by copying chart before modifying

Instead of modifying the original Chart.yaml in-place (which causes race
conditions when multiple releases reference the same local chart), copy the
chart to a temporary directory and rewrite the copy's dependencies. This
eliminates the need for per-chart mutex locks and prevents file corruption
when concurrent goroutines process releases sharing the same local chart.

Fixes #2502

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: address PR review comments for rewriteChartDependencies

- Handle non-NotExist errors from st.fs.Stat to surface permission/IO failures
- Reword function doc to clarify temp copy is conditional on rewrite being needed
- Assert rewrittenPath vs tempDir based on expectModified in test table

Signed-off-by: yxxhero <aiopsclub@163.com>

* test: add integration test for issue #2502 race condition with shared local chart

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: separate environments and releases with --- in helmfile.yaml

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: correct file:// path and remove --skip-deps for dependency build

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: correct file:// dependency path (5 levels up to test/integration/)

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: remove output validation from race condition test

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: assert WriteFile/MkdirTemp/RemoveAll/CopyDir in DefaultFileSystem test

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: add strategicMergePatches to trigger chartify in race condition test

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: scope test values under raw subchart and align ConfigMap name with strategic merge patches

The race condition test values.yaml had templates at the top level instead
of scoped under the raw subchart key, causing helm template to produce no
output and chartify's ReplaceWithRendered to fail with an empty
helmx.1.rendered directory. Also align the ConfigMap name to match the
strategicMergePatches target.

Signed-off-by: yxxhero <aiopsclub@163.com>

---------

Signed-off-by: yxxhero <aiopsclub@163.com>

pkg/filesystem/fs.go

@@ -27,7 +27,10 @@ func (fs fileStat) Sys() any           { return nil }
 type FileSystem struct {
 	ReadFile          func(string) ([]byte, error)
 	ReadDir           func(string) ([]fs.DirEntry, error)
+	WriteFile         func(string, []byte, fs.FileMode) error
 	DeleteFile        func(string) error
+	MkdirTemp         func(string, string) (string, error)
+	RemoveAll         func(string) error
 	FileExists        func(string) (bool, error)
 	Glob              func(string) ([]string, error)
 	FileExistsAt      func(string) bool
@@ -44,7 +47,10 @@ type FileSystem struct {
 func DefaultFileSystem() *FileSystem {
 	dfs := FileSystem{
 		ReadDir:      os.ReadDir,
+		WriteFile:    os.WriteFile,
 		DeleteFile:   os.Remove,
+		MkdirTemp:    os.MkdirTemp,
+		RemoveAll:    os.RemoveAll,
 		Glob:         filepath.Glob,
 		Getwd:        os.Getwd,
 		Chdir:        os.Chdir,
@@ -107,6 +113,15 @@ func FromFileSystem(params FileSystem) *FileSystem {
 	if params.CopyDir != nil {
 		dfs.CopyDir = params.CopyDir
 	}
+	if params.WriteFile != nil {
+		dfs.WriteFile = params.WriteFile
+	}
+	if params.MkdirTemp != nil {
+		dfs.MkdirTemp = params.MkdirTemp
+	}
+	if params.RemoveAll != nil {
+		dfs.RemoveAll = params.RemoveAll
+	}
 	return dfs
 }
 

pkg/filesystem/fs_test.go

@@ -252,4 +252,8 @@ func TestFs_DefaultBuilder(t *testing.T) {
 	assert.NotNil(t, ffs.Chdir)
 	assert.NotNil(t, ffs.Abs)
 	assert.NotNil(t, ffs.EvalSymlinks)
+	assert.NotNil(t, ffs.WriteFile)
+	assert.NotNil(t, ffs.MkdirTemp)
+	assert.NotNil(t, ffs.RemoveAll)
+	assert.NotNil(t, ffs.CopyDir)
 }

pkg/state/chart_dependencies_rewrite_test.go

@@ -1,6 +1,7 @@
 package state
 
 import (
+	"fmt"
 	"os"
 	"path/filepath"
 	"strings"
@@ -67,12 +68,10 @@ dependencies:
 			expectModified: true,
 			expectError:    false,
 			validate: func(t *testing.T, modifiedChartYaml string) {
-				// Should have been converted to absolute path
 				if strings.Contains(modifiedChartYaml, "file://../relative-chart") {
 					t.Errorf("relative path should have been converted to absolute")
 				}
 
-				// Should now have an absolute path
 				if !strings.Contains(modifiedChartYaml, "file://") {
 					t.Errorf("should still have file:// prefix")
 				}
@@ -100,22 +99,18 @@ dependencies:
 			expectModified: true,
 			expectError:    false,
 			validate: func(t *testing.T, modifiedChartYaml string) {
-				// HTTPS repo should remain unchanged
 				if !strings.Contains(modifiedChartYaml, "https://charts.example.com") {
 					t.Errorf("https repository should not be modified")
 				}
 
-				// Relative path should be converted
 				if strings.Contains(modifiedChartYaml, "file://../relative-chart") {
 					t.Errorf("relative file:// path should have been converted")
 				}
 
-				// Absolute path should remain unchanged
 				if !strings.Contains(modifiedChartYaml, "file:///absolute/chart") {
 					t.Errorf("absolute file:// path should not be modified")
 				}
 
-				// OCI repo should remain unchanged
 				if !strings.Contains(modifiedChartYaml, "oci://registry.example.com") {
 					t.Errorf("oci repository should not be modified")
 				}
@@ -140,7 +135,6 @@ dependencies:
 			expectModified: true,
 			expectError:    false,
 			validate: func(t *testing.T, modifiedChartYaml string) {
-				// All relative paths should be converted
 				if strings.Contains(modifiedChartYaml, "file://../chart1") ||
 					strings.Contains(modifiedChartYaml, "file://./chart2") ||
 					strings.Contains(modifiedChartYaml, "file://../../../chart3") {
@@ -187,14 +181,12 @@ extra-field: aaa
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			// Create temporary directory for test
 			tempDir, err := os.MkdirTemp("", "helmfile-test-")
 			if err != nil {
 				t.Fatalf("failed to create temp dir: %v", err)
 			}
 			defer os.RemoveAll(tempDir)
 
-			// Create Chart.yaml if provided
 			if tt.chartYaml != "" {
 				chartYamlPath := filepath.Join(tempDir, "Chart.yaml")
 				if err := os.WriteFile(chartYamlPath, []byte(tt.chartYaml), 0644); err != nil {
@@ -202,22 +194,13 @@ extra-field: aaa
 				}
 			}
 
-			// Create HelmState with logger
 			logger := zap.NewNop().Sugar()
 			st := &HelmState{
 				logger: logger,
 				fs:     filesystem.DefaultFileSystem(),
 			}
 
-			// Read original content if it exists
-			var originalContent []byte
-			chartYamlPath := filepath.Join(tempDir, "Chart.yaml")
-			if _, err := os.Stat(chartYamlPath); err == nil {
-				originalContent, _ = os.ReadFile(chartYamlPath)
-			}
-
-			// Call rewriteChartDependencies
-			cleanup, err := st.rewriteChartDependencies(tempDir)
+			rewrittenPath, cleanup, err := st.rewriteChartDependencies(tempDir)
 
 			if tt.expectError {
 				if err == nil {
@@ -226,44 +209,39 @@ extra-field: aaa
 				return
 			}
 
+			if tt.expectModified {
+				if rewrittenPath == tempDir {
+					t.Errorf("expected rewrittenPath != tempDir when modifications are needed, got same path")
+				}
+			} else {
+				if rewrittenPath != tempDir {
+					t.Errorf("expected rewrittenPath == tempDir when no modifications are needed, got %q", rewrittenPath)
+				}
+			}
+
 			if err != nil {
 				t.Fatalf("unexpected error: %v", err)
 			}
+			defer cleanup()
 
 			if tt.chartYaml == "" {
 				return
 			}
 
-			modifiedChartBytes, err := os.ReadFile(filepath.Join(tempDir, "Chart.yaml"))
+			modifiedChartBytes, err := os.ReadFile(filepath.Join(rewrittenPath, "Chart.yaml"))
 			if err != nil {
 				t.Fatalf("failed to read Chart.yaml: %v", err)
 			}
 			modifiedChartYaml := string(modifiedChartBytes)
 
-			// Validate the modified Chart.yaml
 			if tt.validate != nil {
 				tt.validate(t, modifiedChartYaml)
 			}
-
-			// Call cleanup and verify restoration
-			cleanup()
-
-			// Read restored content
-			restoredContent, err := os.ReadFile(chartYamlPath)
-			if err != nil {
-				t.Fatalf("failed to read restored Chart.yaml: %v", err)
-			}
-
-			// Verify content was restored
-			if string(restoredContent) != string(originalContent) {
-				t.Errorf("cleanup did not restore original content\noriginal:\n%s\nrestored:\n%s",
-					string(originalContent), string(restoredContent))
-			}
 		})
 	}
 }
 
-func TestRewriteChartDependencies_CleanupRestoresOriginal(t *testing.T) {
+func TestRewriteChartDependencies_OriginalNotModified(t *testing.T) {
 	tempDir, err := os.MkdirTemp("", "helmfile-test-")
 	if err != nil {
 		t.Fatalf("failed to create temp dir: %v", err)
@@ -290,33 +268,44 @@ dependencies:
 		fs:     filesystem.DefaultFileSystem(),
 	}
 
-	cleanup, err := st.rewriteChartDependencies(tempDir)
+	rewrittenPath, cleanup, err := st.rewriteChartDependencies(tempDir)
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
 
-	// Verify modification happened
-	modifiedContent, err := os.ReadFile(chartYamlPath)
+	t.Cleanup(func() {
+		if rewrittenPath == tempDir {
+			return
+		}
+
+		cleanup()
+
+		if _, statErr := os.Stat(rewrittenPath); !os.IsNotExist(statErr) {
+			t.Errorf("expected rewritten chart path %q to be removed after cleanup", rewrittenPath)
+		}
+	})
+
+	if rewrittenPath == tempDir {
+		t.Errorf("expected a different path when modifications are needed, got same path")
+	}
+
+	modifiedContent, err := os.ReadFile(filepath.Join(rewrittenPath, "Chart.yaml"))
 	if err != nil {
 		t.Fatalf("failed to read modified Chart.yaml: %v", err)
 	}
 
 	if string(modifiedContent) == originalChart {
-		t.Errorf("Chart.yaml should have been modified")
+		t.Errorf("Chart.yaml in the copy should have been modified")
 	}
 
-	// Call cleanup
-	cleanup()
-
-	// Verify restoration
-	restoredContent, err := os.ReadFile(chartYamlPath)
+	originalContent, err := os.ReadFile(chartYamlPath)
 	if err != nil {
-		t.Fatalf("failed to read restored Chart.yaml: %v", err)
+		t.Fatalf("failed to read original Chart.yaml: %v", err)
 	}
 
-	if string(restoredContent) != originalChart {
-		t.Errorf("cleanup did not restore original content\nexpected:\n%s\ngot:\n%s",
-			originalChart, string(restoredContent))
+	if string(originalContent) != originalChart {
+		t.Errorf("original Chart.yaml should not have been modified\nexpected:\n%s\ngot:\n%s",
+			originalChart, string(originalContent))
 	}
 }
 
@@ -355,21 +344,30 @@ dependencies:
 		fs:     filesystem.DefaultFileSystem(),
 	}
 
-	cleanup, err := st.rewriteChartDependencies(tempDir)
+	rewrittenPath, cleanup, err := st.rewriteChartDependencies(tempDir)
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
-	defer cleanup()
 
-	// Read modified content
-	modifiedContent, err := os.ReadFile(chartYamlPath)
+	t.Cleanup(func() {
+		if rewrittenPath == tempDir {
+			return
+		}
+
+		cleanup()
+
+		if _, statErr := os.Stat(rewrittenPath); !os.IsNotExist(statErr) {
+			t.Errorf("expected rewritten chart path %q to be removed after cleanup", rewrittenPath)
+		}
+	})
+
+	modifiedContent, err := os.ReadFile(filepath.Join(rewrittenPath, "Chart.yaml"))
 	if err != nil {
 		t.Fatalf("failed to read modified Chart.yaml: %v", err)
 	}
 
 	content := string(modifiedContent)
 
-	// Verify fields are preserved
 	requiredFields := []string{
 		"apiVersion: v2",
 		"name: test-chart",
@@ -386,10 +384,17 @@ dependencies:
 		}
 	}
 
-	// Verify the dependency was rewritten
 	if strings.Contains(content, "file://../relative-chart") {
 		t.Errorf("relative path should have been converted to absolute")
 	}
+
+	originalContent, err := os.ReadFile(chartYamlPath)
+	if err != nil {
+		t.Fatalf("failed to read original Chart.yaml: %v", err)
+	}
+	if string(originalContent) != chartYaml {
+		t.Errorf("original Chart.yaml should not have been modified")
+	}
 }
 
 func TestRewriteChartDependencies_ErrorHandling(t *testing.T) {
@@ -397,7 +402,6 @@ func TestRewriteChartDependencies_ErrorHandling(t *testing.T) {
 		name        string
 		setupFunc   func(tempDir string) error
 		expectError bool
-		errorMsg    string
 	}{
 		{
 			name: "invalid yaml in Chart.yaml",
@@ -446,7 +450,7 @@ dependencies:
 				fs:     filesystem.DefaultFileSystem(),
 			}
 
-			_, err = st.rewriteChartDependencies(tempDir)
+			_, _, err = st.rewriteChartDependencies(tempDir)
 
 			if tt.expectError && err == nil {
 				t.Errorf("expected error but got none")
@@ -465,8 +469,6 @@ func TestRewriteChartDependencies_WindowsStylePath(t *testing.T) {
 	}
 	defer os.RemoveAll(tempDir)
 
-	// Test with backslash (Windows-style) paths
-	// Note: file:// URLs should use forward slashes, but test handling of edge cases
 	chartYaml := `apiVersion: v2
 name: test-chart
 version: 1.0.0
@@ -487,20 +489,29 @@ dependencies:
 		fs:     filesystem.DefaultFileSystem(),
 	}
 
-	cleanup, err := st.rewriteChartDependencies(tempDir)
+	rewrittenPath, cleanup, err := st.rewriteChartDependencies(tempDir)
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
-	defer cleanup()
 
-	// Should handle the path correctly
-	data, err := os.ReadFile(chartYamlPath)
+	t.Cleanup(func() {
+		if rewrittenPath == tempDir {
+			return
+		}
+
+		cleanup()
+
+		if _, statErr := os.Stat(rewrittenPath); !os.IsNotExist(statErr) {
+			t.Errorf("expected rewritten chart path %q to be removed after cleanup", rewrittenPath)
+		}
+	})
+
+	data, err := os.ReadFile(filepath.Join(rewrittenPath, "Chart.yaml"))
 	if err != nil {
 		t.Fatalf("failed to read Chart.yaml: %v", err)
 	}
 
 	content := string(data)
-	// The relative path should have been converted to absolute
 	if strings.Contains(content, "file://./subdir/chart") {
 		t.Errorf("relative path with ./ should have been converted")
 	}
@@ -527,9 +538,6 @@ dependencies:
 		t.Fatalf("failed to write Chart.yaml: %v", err)
 	}
 
-	// Run multiple goroutines concurrently on the same chart path.
-	// A readiness WaitGroup ensures all goroutines are blocked before the start barrier is released,
-	// so calls to rewriteChartDependencies overlap as much as possible.
 	numGoroutines := 10
 	var wg sync.WaitGroup
 	var readyWg sync.WaitGroup
@@ -542,7 +550,6 @@ dependencies:
 		go func() {
 			defer wg.Done()
 
-			// Signal that this goroutine is ready, then wait for the start signal.
 			readyWg.Done()
 			<-ready
 
@@ -552,16 +559,44 @@ dependencies:
 				fs:     filesystem.DefaultFileSystem(),
 			}
 
-			cleanup, err := st.rewriteChartDependencies(tempDir)
+			rewrittenPath, cleanup, err := st.rewriteChartDependencies(tempDir)
 			if err != nil {
 				errCh <- err
 				return
 			}
 			defer cleanup()
+
+			data, readErr := os.ReadFile(filepath.Join(rewrittenPath, "Chart.yaml"))
+			if readErr != nil {
+				errCh <- readErr
+				return
+			}
+
+			type ChartDependency struct {
+				Name       string `yaml:"name"`
+				Repository string `yaml:"repository"`
+				Version    string `yaml:"version"`
+			}
+			type ChartMeta struct {
+				APIVersion   string            `yaml:"apiVersion"`
+				Name         string            `yaml:"name"`
+				Version      string            `yaml:"version"`
+				Dependencies []ChartDependency `yaml:"dependencies,omitempty"`
+			}
+
+			var meta ChartMeta
+			if unmarshalErr := yaml.Unmarshal(data, &meta); unmarshalErr != nil {
+				errCh <- unmarshalErr
+				return
+			}
+
+			if meta.Name != "test-chart" {
+				errCh <- fmt.Errorf("expected chart name 'test-chart', got %q", meta.Name)
+				return
+			}
 		}()
 	}
 
-	// Wait until all goroutines are ready, then release them simultaneously.
 	readyWg.Wait()
 	close(ready)
 
@@ -572,10 +607,9 @@ dependencies:
 		t.Errorf("goroutine error: %v", err)
 	}
 
-	// Verify Chart.yaml is still valid by unmarshaling and checking expected fields
 	data, err := os.ReadFile(chartYamlPath)
 	if err != nil {
-		t.Fatalf("failed to read Chart.yaml: %v", err)
+		t.Fatalf("failed to read original Chart.yaml: %v", err)
 	}
 
 	type ChartDependency struct {
@@ -592,21 +626,22 @@ dependencies:
 
 	var chartMeta ChartMeta
 	if err := yaml.Unmarshal(data, &chartMeta); err != nil {
-		t.Fatalf("Chart.yaml is not valid YAML after concurrent rewrites: %v", err)
+		t.Fatalf("original Chart.yaml is not valid YAML: %v", err)
 	}
 
 	if chartMeta.Name != "test-chart" {
-		t.Errorf("expected chart name 'test-chart', got %q", chartMeta.Name)
+		t.Errorf("expected original chart name 'test-chart', got %q", chartMeta.Name)
 	}
-	if len(chartMeta.Dependencies) != 1 {
-		t.Fatalf("expected 1 dependency, got %d", len(chartMeta.Dependencies))
+	if len(chartMeta.Dependencies) == 0 {
+		t.Fatalf("expected original Chart.yaml to contain at least one dependency, got %d", len(chartMeta.Dependencies))
 	}
-	if chartMeta.Dependencies[0].Name != "dep1" {
-		t.Errorf("expected dependency name 'dep1', got %q", chartMeta.Dependencies[0].Name)
+
+	const wantDependencyName = "dep1"
+	if chartMeta.Dependencies[0].Name != wantDependencyName {
+		t.Errorf("expected first dependency name %q, got %q", wantDependencyName, chartMeta.Dependencies[0].Name)
 	}
-	// The cleanup from each goroutine must have restored the original relative path.
 	const wantRepository = "file://../relative-chart"
 	if chartMeta.Dependencies[0].Repository != wantRepository {
-		t.Errorf("expected dependency repository %q (original relative path), got %q", wantRepository, chartMeta.Dependencies[0].Repository)
+		t.Errorf("expected original dependency repository %q, got %q", wantRepository, chartMeta.Dependencies[0].Repository)
 	}
 }

pkg/state/state.go

@@ -1409,30 +1409,26 @@ type PrepareChartKey struct {
 
 // rewriteChartDependencies rewrites relative file:// dependencies in Chart.yaml to absolute paths
 // to ensure they can be resolved from chartify's temporary directory.
-// When the file is actually rewritten the returned cleanup function restores the original content
-// and releases the per-chart mutex; when no rewrite is needed a no-op cleanup is returned and the
-// mutex is never held past this call.
-func (st *HelmState) rewriteChartDependencies(chartPath string) (func(), error) {
+// Instead of modifying the original chart in-place (which causes race conditions when multiple
+// releases reference the same local chart), it creates a temporary copy only when a rewrite is
+// needed and rewrites that copy. When a temp copy is created, it reflects the current chart
+// contents so prepare hooks or other steps that mutate the local chart directory are honored.
+// The returned cleanup function removes the temporary directory when one was created and is
+// otherwise a no-op.
+func (st *HelmState) rewriteChartDependencies(chartPath string) (string, func(), error) {
 	chartYamlPath := filepath.Join(chartPath, "Chart.yaml")
 
-	// Check if Chart.yaml exists
-	if _, err := os.Stat(chartYamlPath); os.IsNotExist(err) {
-		return func() {}, nil
+	if _, err := st.fs.Stat(chartYamlPath); os.IsNotExist(err) {
+		return chartPath, func() {}, nil
+	} else if err != nil {
+		return chartPath, func() {}, err
 	}
 
-	// Reuse the existing per-path RWMutex for exclusive access to the chart directory.
-	mu := st.getNamedRWMutex(chartPath)
-	mu.Lock()
-
-	// Read Chart.yaml
-	data, err := os.ReadFile(chartYamlPath)
+	data, err := st.fs.ReadFile(chartYamlPath)
 	if err != nil {
-		mu.Unlock()
-		return func() {}, err
+		return chartPath, func() {}, err
 	}
-	originalContent := data
 
-	// Parse Chart.yaml
 	type ChartDependency struct {
 		Name       string                 `yaml:"name"`
 		Repository string                 `yaml:"repository"`
@@ -1444,26 +1440,21 @@ func (st *HelmState) rewriteChartDependencies(chartPath string) (func(), error)
 	}
 
 	var chartMeta ChartMeta
-	if err := yaml.Unmarshal(originalContent, &chartMeta); err != nil {
-		mu.Unlock()
-		return func() {}, err
+	if err := yaml.Unmarshal(data, &chartMeta); err != nil {
+		return chartPath, func() {}, err
 	}
 
-	// Rewrite relative file:// dependencies to absolute paths
 	modified := false
 	for i := range chartMeta.Dependencies {
 		dep := &chartMeta.Dependencies[i]
 		if strings.HasPrefix(dep.Repository, "file://") {
 			relPath := strings.TrimPrefix(dep.Repository, "file://")
 
-			// Check if it's a relative path
 			if !filepath.IsAbs(relPath) {
-				// Convert to absolute path relative to the chart directory
 				absPath := filepath.Join(chartPath, relPath)
 				absPath, err = filepath.Abs(absPath)
 				if err != nil {
-					mu.Unlock()
-					return func() {}, fmt.Errorf("failed to resolve absolute path for dependency %s: %w", dep.Name, err)
+					return chartPath, func() {}, fmt.Errorf("failed to resolve absolute path for dependency %s: %w", dep.Name, err)
 				}
 
 				st.logger.Debugf("Rewriting Chart dependency %s from %s to file://%s", dep.Name, dep.Repository, absPath)
@@ -1473,37 +1464,44 @@ func (st *HelmState) rewriteChartDependencies(chartPath string) (func(), error)
 		}
 	}
 
-	// If nothing changed, release the lock immediately. Chart.yaml has already been read and
-	// unmarshaled above, but no file write is needed and the lock is not held beyond this function.
 	if !modified {
-		mu.Unlock()
-		return func() {}, nil
+		return chartPath, func() {}, nil
 	}
 
 	updatedData, err := yaml.Marshal(&chartMeta)
 	if err != nil {
-		mu.Unlock()
-		return func() {}, fmt.Errorf("failed to marshal Chart.yaml: %w", err)
+		return chartPath, func() {}, fmt.Errorf("failed to marshal Chart.yaml: %w", err)
 	}
 
-	if err := os.WriteFile(chartYamlPath, updatedData, 0644); err != nil {
-		// File may be truncated/partially written; restore original content before releasing the lock.
-		if restoreErr := os.WriteFile(chartYamlPath, originalContent, 0644); restoreErr != nil {
-			st.logger.Warnf("Failed to restore Chart.yaml at %s after write error (%v): %v", chartYamlPath, err, restoreErr)
-		}
-		mu.Unlock()
-		return func() {}, fmt.Errorf("failed to write Chart.yaml: %w", err)
+	tempDir, err := st.fs.MkdirTemp("", "chart-deps-rewrite-*")
+	if err != nil {
+		return chartPath, func() {}, fmt.Errorf("failed to create temp directory for chart rewrite: %w", err)
 	}
 
-	st.logger.Debugf("Rewrote Chart.yaml with absolute dependency paths at %s", chartYamlPath)
-
-	// Return cleanup that restores original content and releases the lock.
-	return func() {
-		if restoreErr := os.WriteFile(chartYamlPath, originalContent, 0644); restoreErr != nil {
-			st.logger.Warnf("Failed to restore original Chart.yaml at %s: %v", chartYamlPath, restoreErr)
+	if err := st.fs.CopyDir(chartPath, tempDir); err != nil {
+		if removeErr := st.fs.RemoveAll(tempDir); removeErr != nil {
+			st.logger.Warnf("Failed to remove temp chart directory %s: %v", tempDir, removeErr)
 		}
-		mu.Unlock()
-	}, nil
+		return chartPath, func() {}, fmt.Errorf("failed to copy chart to temp directory: %w", err)
+	}
+
+	tempChartYamlPath := filepath.Join(tempDir, "Chart.yaml")
+	if err := st.fs.WriteFile(tempChartYamlPath, updatedData, 0644); err != nil {
+		if removeErr := st.fs.RemoveAll(tempDir); removeErr != nil {
+			st.logger.Warnf("Failed to remove temp chart directory %s: %v", tempDir, removeErr)
+		}
+		return chartPath, func() {}, fmt.Errorf("failed to write Chart.yaml: %w", err)
+	}
+
+	st.logger.Debugf("Rewrote Chart.yaml with absolute dependency paths at %s", tempChartYamlPath)
+
+	cleanup := func() {
+		if removeErr := st.fs.RemoveAll(tempDir); removeErr != nil {
+			st.logger.Warnf("Failed to remove temp chart directory %s: %v", tempDir, removeErr)
+		}
+	}
+
+	return tempDir, cleanup, nil
 }
 
 // Otherwise, if a chart is not a helm chart, it will call "chartify" to turn it into a chart.
@@ -1515,11 +1513,12 @@ func (st *HelmState) processChartification(chartification *Chartify, release *Re
 	// This prevents errors like "Error: directory /tmp/chartify.../argocd-application not found"
 	// when Chart.yaml contains dependencies like "file://../argocd-application"
 	if st.fs.DirectoryExistsAt(chartPath) {
-		restoreChart, err := st.rewriteChartDependencies(chartPath)
+		rewrittenPath, cleanupTempChart, err := st.rewriteChartDependencies(chartPath)
 		if err != nil {
 			return "", false, fmt.Errorf("failed to rewrite chart dependencies: %w", err)
 		}
-		defer restoreChart()
+		chartPath = rewrittenPath
+		defer cleanupTempChart()
 	}
 
 	c := chartify.New(

test/integration/run.sh

@@ -96,6 +96,7 @@ ${kubectl} create namespace ${test_ns} || fail "Could not create namespace ${tes
 
 # TEST CASES----------------------------------------------------------------------------------------------------------
 
+. ${dir}/test-cases/issue-2502-race-condition-local-chart.sh
 . ${dir}/test-cases/chart-deps-condition.sh
 . ${dir}/test-cases/fetch-forl-local-chart.sh
 . ${dir}/test-cases/suppress-output-line-regex.sh

test/integration/test-cases/issue-2502-race-condition-local-chart.sh

@@ -0,0 +1,50 @@
+# Integration test for issue #2502: Race condition when multiple releases share a local chart
+# https://github.com/helmfile/helmfile/issues/2502
+#
+# When multiple releases reference the same local chart, concurrent goroutines
+# race on rewriting Chart.yaml dependencies, causing:
+#   "Error: validation: chart.metadata.name is required"
+#
+# This test verifies the fix works WITHOUT --concurrency 1 workaround.
+
+issue_2502_input_dir="${cases_dir}/issue-2502-race-condition-local-chart/input"
+
+issue_2502_tmp=$(mktemp -d)
+actual="${issue_2502_tmp}/actual.yaml"
+
+cleanup_issue_2502() {
+  if [ -n "${issue_2502_tmp}" ] && [ -d "${issue_2502_tmp}" ]; then
+    rm -rf "${issue_2502_tmp}"
+  fi
+}
+trap cleanup_issue_2502 EXIT
+
+test_start "issue #2502: race condition with shared local chart"
+
+info "Running helmfile template with 5 releases sharing the same local chart (default concurrency)"
+
+# Run WITHOUT --concurrency 1 to test the fix.
+# Before the fix, this would intermittently fail with:
+#   "Error: validation: chart.metadata.name is required"
+# Run multiple iterations to increase chance of catching a race.
+pass=0
+iterations=5
+for i in $(seq 1 ${iterations}); do
+  if ${helmfile} -f ${issue_2502_input_dir}/helmfile.yaml -e test template > ${actual} 2>&1; then
+    pass=$((pass + 1))
+  else
+    cat ${actual}
+    fail "helmfile template failed on iteration ${i}/${iterations} (race condition on shared local chart)"
+  fi
+done
+
+if [ ${pass} -ne ${iterations} ]; then
+  fail "Expected ${iterations}/${iterations} passes but got ${pass}/${iterations}"
+fi
+
+info "All ${iterations} iterations passed successfully"
+
+cleanup_issue_2502
+trap - EXIT
+
+test_pass "issue #2502: race condition with shared local chart"

test/integration/test-cases/issue-2502-race-condition-local-chart/input/helm/my-chart/Chart.yaml

@@ -0,0 +1,10 @@
+dependencies:
+  - name: raw
+    repository: file://../../../../../charts/raw
+    version: 0.0.1
+apiVersion: v2
+appVersion: "1.0.0"
+description: A test chart for race condition reproduction
+name: my-chart
+type: application
+version: 0.1.0

test/integration/test-cases/issue-2502-race-condition-local-chart/input/helm/my-chart/values.yaml

@@ -0,0 +1,9 @@
+raw:
+  templates:
+    - |
+      apiVersion: v1
+      kind: ConfigMap
+      metadata:
+        name: test-cm
+      data:
+        key: value

test/integration/test-cases/issue-2502-race-condition-local-chart/input/helmfile.yaml

@@ -0,0 +1,49 @@
+environments:
+  test: {}
+---
+releases:
+  - name: app
+    chart: helm/my-chart
+    strategicMergePatches:
+      - apiVersion: v1
+        kind: ConfigMap
+        metadata:
+          name: test-cm
+        data:
+          key: value
+  - name: app-2
+    chart: helm/my-chart
+    strategicMergePatches:
+      - apiVersion: v1
+        kind: ConfigMap
+        metadata:
+          name: test-cm
+        data:
+          key: value
+  - name: app-3
+    chart: helm/my-chart
+    strategicMergePatches:
+      - apiVersion: v1
+        kind: ConfigMap
+        metadata:
+          name: test-cm
+        data:
+          key: value
+  - name: app-4
+    chart: helm/my-chart
+    strategicMergePatches:
+      - apiVersion: v1
+        kind: ConfigMap
+        metadata:
+          name: test-cm
+        data:
+          key: value
+  - name: app-5
+    chart: helm/my-chart
+    strategicMergePatches:
+      - apiVersion: v1
+        kind: ConfigMap
+        metadata:
+          name: test-cm
+        data:
+          key: value