fix(actions): reject non-mapping matrix include/exclude (#38933)

modules/actions/jobparser/jobparser.go

@@ -125,6 +125,9 @@ type parseContext struct {
 type ParseOption func(c *parseContext)
 
 func getMatrixes(job *model.Job) ([]map[string]any, error) {
+	if err := validateMatrixFilters(job); err != nil {
+		return nil, err
+	}
 	ret, err := job.GetMatrixes()
 	if err != nil {
 		return nil, fmt.Errorf("GetMatrixes: %w", err)
@@ -135,6 +138,35 @@ func getMatrixes(job *model.Job) ([]map[string]any, error) {
 	return ret, nil
 }
 
+// validateMatrixFilters rejects an `include`/`exclude` that is not a list of mappings, so that the
+// usual way to get there, an unevaluated ${{ }} expression that is still a scalar, is named as such
+// instead of panicking inside the expansion.
+func validateMatrixFilters(job *model.Job) error {
+	if job.Strategy == nil || job.Strategy.RawMatrix.Kind != yaml.MappingNode {
+		return nil
+	}
+	content := job.Strategy.RawMatrix.Content
+	for i := 0; i+1 < len(content); i += 2 {
+		name, value := content[i].Value, content[i+1]
+		if name != "include" && name != "exclude" {
+			continue
+		}
+		entries := []*yaml.Node{value}
+		if value.Kind == yaml.SequenceNode {
+			entries = value.Content
+		}
+		for _, entry := range entries {
+			if entry.Kind == yaml.AliasNode {
+				entry = entry.Alias
+			}
+			if entry.Kind != yaml.MappingNode {
+				return fmt.Errorf("matrix %s must be a list of mappings", name)
+			}
+		}
+	}
+	return nil
+}
+
 func encodeMatrix(matrix map[string]any) yaml.Node {
 	if len(matrix) == 0 {
 		return yaml.Node{}

modules/actions/jobparser/jobparser_test.go

@@ -4,6 +4,7 @@
 package jobparser
 
 import (
+	"fmt"
 	"strings"
 	"testing"
 
@@ -147,3 +148,47 @@ func TestParseInterpolatesRunName(t *testing.T) {
 	require.Len(t, result, 1)
 	assert.Empty(t, result[0].RunName)
 }
+
+func TestRejectsUnevaluatedMatrixFilters(t *testing.T) {
+	// An unevaluated ${{ }} expression is still a scalar, which act cannot apply as a filter: it used
+	// to reach the expansion and panic there on an unchecked type assertion, taking down the file view
+	// and the push_update queue.
+	const workflow = `
+name: t
+on: push
+jobs:
+  setup:
+    runs-on: ubuntu-latest
+    outputs:
+      m: ${{ steps.s.outputs.m }}
+    steps: [{id: s, run: echo}]
+  build:
+    needs: setup
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        %s
+    steps: [{run: echo}]
+`
+	for _, tt := range []struct {
+		name   string
+		matrix string
+	}{
+		{name: "include expression", matrix: "include: ${{ fromJson(needs.setup.outputs.m) }}"},
+		{name: "exclude expression", matrix: "os: [a, b]\n        exclude: ${{ fromJson(vars.MATRIX) }}"},
+		{name: "include scalar", matrix: "include: whatever"},
+		{name: "include list of scalars", matrix: "include: [a, b]"},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			require.NotPanics(t, func() {
+				_, err := Parse(fmt.Appendf(nil, workflow, tt.matrix))
+				require.ErrorContains(t, err, "must be a list of mappings")
+			})
+		})
+	}
+
+	// a well-formed include/exclude keeps expanding
+	planned, err := Parse(fmt.Appendf(nil, workflow, "os: [a, b]\n        include:\n          - os: c\n        exclude:\n          - os: b"))
+	require.NoError(t, err)
+	assert.Len(t, planned, 3) // setup, plus build for os a and c
+}