fix: don't mutate shared matrix AST when resolving ref: rows

taskfile/ast/matrix.go

@@ -93,6 +93,21 @@ func (matrix *Matrix) DeepCopy() *Matrix {
 	}
 }
 
+// DeepCopy returns a copy of the MatrixRow. Without this, deepcopy.OrderedMap
+// falls back to copying the *MatrixRow pointer as-is, so every "copy" of a
+// Matrix would still share the same underlying rows - see #2890, where
+// concurrent invocations of a task with a `ref:` matrix row raced on
+// resolveMatrixRefs mutating that shared row.
+func (row *MatrixRow) DeepCopy() *MatrixRow {
+	if row == nil {
+		return nil
+	}
+	return &MatrixRow{
+		Ref:   row.Ref,
+		Value: deepcopy.Slice(row.Value),
+	}
+}
+
 func (matrix *Matrix) UnmarshalYAML(node *yaml.Node) error {
 	switch node.Kind {
 	case yaml.MappingNode:

variables.go

@@ -347,13 +347,14 @@ func itemsFromFor(
 	var values []any  // The list of values to loop over
 	// Get the list from a matrix
 	if f.Matrix.Len() != 0 {
-		if err := resolveMatrixRefs(f.Matrix, cache); err != nil {
+		resolvedMatrix, err := resolveMatrixRefs(f.Matrix, cache)
+		if err != nil {
 			return nil, nil, errors.TaskfileInvalidError{
 				URI: location.Taskfile,
 				Err: err,
 			}
 		}
-		return asAnySlice(product(f.Matrix)), nil, nil
+		return asAnySlice(product(resolvedMatrix)), nil, nil
 	}
 	// Get the list from the explicit for list
 	if len(f.List) > 0 {
@@ -425,22 +426,43 @@ func itemsFromFor(
 	return values, keys, nil
 }
 
-func resolveMatrixRefs(matrix *ast.Matrix, cache *templater.Cache) error {
+// resolveMatrixRefs resolves any `ref:` rows in matrix and returns a new
+// Matrix with those rows populated. It must not mutate the matrix passed in:
+// that matrix is part of the shared, cached Task AST, and concurrent
+// invocations of the same task (e.g. via parallel deps) call this with the
+// same *ast.Matrix and would otherwise race on the row.Value assignment
+// below, intermittently leaking a value resolved for one caller into another
+// caller's expansion. See #2890.
+func resolveMatrixRefs(matrix *ast.Matrix, cache *templater.Cache) (*ast.Matrix, error) {
 	if matrix.Len() == 0 {
-		return nil
+		return matrix, nil
 	}
+	hasRef := false
 	for _, row := range matrix.All() {
+		if row.Ref != "" {
+			hasRef = true
+			break
+		}
+	}
+	if !hasRef {
+		return matrix, nil
+	}
+	resolved := matrix.DeepCopy()
+	for _, row := range resolved.All() {
 		if row.Ref != "" {
 			v := templater.ResolveRef(row.Ref, cache)
+			if cache.Err() != nil {
+				return nil, cache.Err()
+			}
 			switch value := v.(type) {
 			case []any:
 				row.Value = value
 			default:
-				return fmt.Errorf("matrix reference %q must resolve to a list", row.Ref)
+				return nil, fmt.Errorf("matrix reference %q must resolve to a list", row.Ref)
 			}
 		}
 	}
-	return nil
+	return resolved, nil
 }
 
 func resolveEnumRefs(requires *ast.Requires, cache *templater.Cache) error {

variables_internal_test.go

@@ -0,0 +1,93 @@
+package task
+
+import (
+	"fmt"
+	"sync"
+	"testing"
+
+	"github.com/go-task/task/v3/internal/templater"
+	"github.com/go-task/task/v3/taskfile/ast"
+)
+
+// TestResolveMatrixRefsDoesNotMutateSharedMatrix is a regression test for
+// #2890. The *ast.Matrix passed to resolveMatrixRefs is part of the shared,
+// cached Task AST, and concurrent invocations of the same task (e.g. via
+// parallel deps) all call resolveMatrixRefs with the very same *ast.Matrix.
+// If resolveMatrixRefs mutates that matrix in place, concurrent callers race
+// on the mutation and can observe a value resolved for a different caller
+// (cross-contamination), or trip the race detector.
+//
+// Run with `go test -race` to catch the race deterministically; it is not
+// guaranteed to corrupt output on every unsynchronized run.
+func TestResolveMatrixRefsDoesNotMutateSharedMatrix(t *testing.T) {
+	t.Parallel()
+
+	sharedMatrix := ast.NewMatrix(
+		&ast.MatrixElement{Key: "ARCH", Value: &ast.MatrixRow{Ref: ".ARCH_VAR"}},
+	)
+
+	const n = 200
+	var wg sync.WaitGroup
+	results := make([]string, n)
+	errs := make([]error, n)
+
+	for i := 0; i < n; i++ {
+		wg.Add(1)
+		go func(i int) {
+			defer wg.Done()
+
+			want := "amd64"
+			if i%2 != 0 {
+				want = "arm64"
+			}
+
+			vars := ast.NewVars()
+			vars.Set("ARCH_VAR", ast.Var{Value: []any{want}})
+			cache := &templater.Cache{Vars: vars}
+
+			resolved, err := resolveMatrixRefs(sharedMatrix, cache)
+			if err != nil {
+				errs[i] = err
+				return
+			}
+			row, ok := resolved.Get("ARCH")
+			if !ok {
+				errs[i] = fmt.Errorf("call %d: ARCH row missing from resolved matrix", i)
+				return
+			}
+			if len(row.Value) != 1 {
+				errs[i] = fmt.Errorf("call %d: ARCH.Value = %v, want a single-element slice", i, row.Value)
+				return
+			}
+			got, _ := row.Value[0].(string)
+			results[i] = got
+		}(i)
+	}
+	wg.Wait()
+
+	for i, err := range errs {
+		if err != nil {
+			t.Fatalf("call %d: unexpected error: %v", i, err)
+		}
+		want := "amd64"
+		if i%2 != 0 {
+			want = "arm64"
+		}
+		if results[i] != want {
+			t.Errorf("call %d: got ARCH=%q, want %q (cross-contamination between concurrent callers)", i, results[i], want)
+		}
+	}
+
+	// The original, shared matrix must remain unresolved: resolveMatrixRefs
+	// must operate on a copy, not the input.
+	origRow, ok := sharedMatrix.Get("ARCH")
+	if !ok {
+		t.Fatal("ARCH row missing from original matrix")
+	}
+	if origRow.Value != nil {
+		t.Errorf("shared input matrix was mutated: ARCH.Value = %v, want nil (Ref rows must be resolved into a copy)", origRow.Value)
+	}
+	if origRow.Ref != ".ARCH_VAR" {
+		t.Errorf("shared input matrix Ref was altered: got %q, want %q", origRow.Ref, ".ARCH_VAR")
+	}
+}