diff --git a/go.mod b/go.mod index 1f2e923ce21..9230df0154c 100644 --- a/go.mod +++ b/go.mod @@ -87,6 +87,7 @@ require ( github.com/prometheus/client_golang v1.23.2 github.com/quasoft/websspi v1.1.2 github.com/redis/go-redis/v9 v9.21.0 + github.com/rhysd/actionlint v1.7.12 github.com/robfig/cron/v3 v3.0.1 github.com/santhosh-tekuri/jsonschema/v6 v6.0.2 github.com/sassoftware/go-rpmutils v0.4.0 @@ -244,7 +245,6 @@ require ( github.com/prometheus/common v0.68.1 // indirect github.com/prometheus/procfs v0.20.1 // indirect github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec // indirect - github.com/rhysd/actionlint v1.7.12 // indirect github.com/rs/xid v1.6.0 // indirect github.com/russross/blackfriday/v2 v2.1.0 // indirect github.com/shopspring/decimal v1.4.0 // indirect diff --git a/modules/actions/jobparser/evaluator.go b/modules/actions/jobparser/evaluator.go index 3e314afb62b..9cccaf4f101 100644 --- a/modules/actions/jobparser/evaluator.go +++ b/modules/actions/jobparser/evaluator.go @@ -6,7 +6,10 @@ package jobparser import ( "errors" "fmt" + "math" + "reflect" "regexp" + "strconv" "strings" "gitea.com/gitea/runner/act/exprparser" @@ -23,12 +26,6 @@ func NewExpressionEvaluator(interpreter exprparser.Interpreter) *ExpressionEvalu return &ExpressionEvaluator{interpreter: interpreter} } -func (ee ExpressionEvaluator) evaluate(in string, defaultStatusCheck exprparser.DefaultStatusCheck) (any, error) { - evaluated, err := ee.interpreter.Evaluate(in, defaultStatusCheck) - - return evaluated, err -} - func (ee ExpressionEvaluator) evaluateScalarYamlNode(node *yaml.Node) error { var in string if err := node.Decode(&in); err != nil { @@ -37,17 +34,17 @@ func (ee ExpressionEvaluator) evaluateScalarYamlNode(node *yaml.Node) error { if !strings.Contains(in, "${{") || !strings.Contains(in, "}}") { return nil } - expr, _ := rewriteSubExpression(in, false) - res, err := ee.evaluate(expr, exprparser.DefaultStatusCheckNone) + res, err := ee.evaluateScalar(in) if err != nil { return err } return node.Encode(res) } +// GitHub has this undocumented feature to merge maps, called insert directive +var insertDirective = regexp.MustCompile(`\${{\s*insert\s*}}`) + func (ee ExpressionEvaluator) evaluateMappingYamlNode(node *yaml.Node) error { - // GitHub has this undocumented feature to merge maps, called insert directive - insertDirective := regexp.MustCompile(`\${{\s*insert\s*}}`) for i := 0; i < len(node.Content)/2; { k := node.Content[i*2] v := node.Content[i*2+1] @@ -102,88 +99,170 @@ func (ee ExpressionEvaluator) EvaluateYamlNode(node *yaml.Node) error { } } -func (ee ExpressionEvaluator) Interpolate(in string) string { - if !strings.Contains(in, "${{") || !strings.Contains(in, "}}") { - return in - } - - expr, _ := rewriteSubExpression(in, true) - evaluated, err := ee.evaluate(expr, exprparser.DefaultStatusCheckNone) +// interpolate evaluates every part on its own, so a malformed one cannot restructure its neighbours +func (ee ExpressionEvaluator) interpolate(in string) (string, error) { + parts, err := splitSubExpressions(in) if err != nil { - return "" + return "", err } - - value, ok := evaluated.(string) - if !ok { - panic(fmt.Sprintf("Expression %s did not evaluate to a string", expr)) - } - - return value -} - -func escapeFormatString(in string) string { - return strings.ReplaceAll(strings.ReplaceAll(in, "{", "{{"), "}", "}}") -} - -func rewriteSubExpression(in string, forceFormat bool) (string, error) { - if !strings.Contains(in, "${{") || !strings.Contains(in, "}}") { + if len(parts) == 1 && !parts[0].isExpr { return in, nil } + var out strings.Builder + out.Grow(len(in)) + for _, part := range parts { + if !part.isExpr { + out.WriteString(part.text) + continue + } + evaluated, err := ee.interpreter.Evaluate(part.text, exprparser.DefaultStatusCheckNone) + if err != nil { + return "", err + } + out.WriteString(coerceToString(evaluated)) + } + return out.String(), nil +} - strPattern := regexp.MustCompile("(?:''|[^'])*'") - pos := 0 - exprStart := -1 - strStart := -1 - var results []string - var formatOut strings.Builder - for pos < len(in) { - if strStart > -1 { - matches := strPattern.FindStringIndex(in[pos:]) - if matches == nil { - return "", errors.New("unclosed string") - } +// evaluateScalar keeps the type of a lone expression, so `${{ fromJSON('[1,2]') }}` stays an array +func (ee ExpressionEvaluator) evaluateScalar(in string) (any, error) { + parts, err := splitSubExpressions(in) + if err != nil { + return nil, err + } + if len(parts) == 1 && parts[0].isExpr { + return ee.interpreter.Evaluate(parts[0].text, exprparser.DefaultStatusCheckNone) + } + return ee.interpolate(in) +} - strStart = -1 - pos += matches[1] - } else if exprStart > -1 { - exprEnd := strings.Index(in[pos:], "}}") - strStart = strings.Index(in[pos:], "'") +// evaluateCondition evaluates an `if:`, an expression even without `${{ }}`. Mixed content +// interpolates to a string, so the success() default applies to it separately. +func (ee ExpressionEvaluator) evaluateCondition(in string) (bool, error) { + parts, err := splitSubExpressions(in) + if err != nil { + return false, err + } + if len(parts) == 1 { + evaluated, err := ee.interpreter.Evaluate(parts[0].text, exprparser.DefaultStatusCheckSuccess) + if err != nil { + return false, err + } + return exprparser.IsTruthy(evaluated), nil + } - if exprEnd > -1 && strStart > -1 { - if exprEnd < strStart { - strStart = -1 - } else { - exprEnd = -1 - } - } - - if exprEnd > -1 { - fmt.Fprintf(&formatOut, "{%d}", len(results)) - results = append(results, strings.TrimSpace(in[exprStart:pos+exprEnd])) - pos += exprEnd + 2 - exprStart = -1 - } else if strStart > -1 { - pos += strStart + 1 - } else { - panic("unclosed expression.") - } - } else { - exprStart = strings.Index(in[pos:], "${{") - if exprStart != -1 { - formatOut.WriteString(escapeFormatString(in[pos : pos+exprStart])) - exprStart = pos + exprStart + 3 - pos = exprStart - } else { - formatOut.WriteString(escapeFormatString(in[pos:])) - pos = len(in) - } + // mixed content is a string, so the success() default applies to it separately + if !expressionCallsFunction(in, "success", "always", "failure", "cancelled") { + status, err := ee.interpreter.Evaluate("success()", exprparser.DefaultStatusCheckNone) + if err != nil { + return false, err + } + if !exprparser.IsTruthy(status) { + return false, nil } } + interpolated, err := ee.interpolate(in) + if err != nil { + return false, err + } + return exprparser.IsTruthy(interpolated), nil +} - if len(results) == 1 && formatOut.String() == "{0}" && !forceFormat { - return in, nil +// coerceToString converts an evaluated expression value to a string the way GitHub does, +// see https://docs.github.com/en/actions/reference/workflows-and-actions/expressions#operators +// An already reflected value is accepted as-is, since Interface() would panic on an invalid one. +func coerceToString(v any) string { + value, ok := v.(reflect.Value) + if !ok { + value = reflect.ValueOf(v) } - out := fmt.Sprintf("format('%s', %s)", strings.ReplaceAll(formatOut.String(), "'", "''"), strings.Join(results, ", ")) - return out, nil + switch value.Kind() { + case reflect.Invalid: + return "" + + case reflect.Bool: + return strconv.FormatBool(value.Bool()) + + case reflect.String: + return value.String() + + case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64: + return strconv.FormatInt(value.Int(), 10) + + case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64: + return strconv.FormatUint(value.Uint(), 10) + + case reflect.Float32, reflect.Float64: + if math.IsInf(value.Float(), 1) { + return "Infinity" + } else if math.IsInf(value.Float(), -1) { + return "-Infinity" + } + return fmt.Sprintf("%.15G", value.Float()) + + case reflect.Slice, reflect.Array: + return "Array" + + // contexts such as `github` are pointers to structs, so they stringify as objects too + case reflect.Map, reflect.Struct: + return "Object" + + case reflect.Interface, reflect.Pointer: + if value.IsNil() { + return "" + } + return coerceToString(value.Elem()) + } + + return fmt.Sprintf("%v", value) +} + +type exprPart struct { + text string + isExpr bool +} + +// splitSubExpressions splits in the way GitHub's template reader does, leaving a value without a +// complete expression literal. +func splitSubExpressions(in string) ([]exprPart, error) { + if !strings.Contains(in, "${{") || !strings.Contains(in, "}}") { + return []exprPart{{text: in}}, nil + } + + parts := make([]exprPart, 0, 2*strings.Count(in, "${{")+1) + for { + start := strings.Index(in, "${{") + if start < 0 { + if in != "" { + parts = append(parts, exprPart{text: in}) + } + return parts, nil + } + if start > 0 { + parts = append(parts, exprPart{text: in[:start]}) + } + rest := in[start+len("${{"):] + end := indexExprEnd(rest) + if end < 0 { + return nil, errors.New("unclosed expression") + } + parts = append(parts, exprPart{text: strings.TrimSpace(rest[:end]), isExpr: true}) + in = rest[end+len("}}"):] + } +} + +// indexExprEnd returns the offset of the `}}` ending an expression, or -1. A quote toggles string +// state, so a `}}` inside a string does not end it. +func indexExprEnd(in string) int { + inString := false + for i := range len(in) { + switch { + case in[i] == '\'': + inString = !inString + case !inString && in[i] == '}' && i+1 < len(in) && in[i+1] == '}': + return i + } + } + return -1 } diff --git a/modules/actions/jobparser/jobparser.go b/modules/actions/jobparser/jobparser.go index 79c7b7b433e..1f843c125b6 100644 --- a/modules/actions/jobparser/jobparser.go +++ b/modules/actions/jobparser/jobparser.go @@ -6,11 +6,13 @@ package jobparser import ( "bytes" "fmt" + "slices" "sort" "strings" "gitea.com/gitea/runner/act/exprparser" "gitea.com/gitea/runner/act/model" + "github.com/rhysd/actionlint" "go.yaml.in/yaml/v4" ) @@ -48,7 +50,9 @@ func Parse(content []byte, options ...ParseOption) ([]*SingleWorkflow, error) { } evaluator := NewExpressionEvaluator(exprparser.NewInterpeter(&exprparser.EvaluationEnvironment{Github: pc.gitContext, Vars: pc.vars, Inputs: pc.inputs}, exprparser.Config{})) - workflow.RunName = evaluator.Interpolate(workflow.RunName) + if workflow.RunName, err = evaluator.interpolate(workflow.RunName); err != nil { + return nil, fmt.Errorf("interpolate run-name: %w", err) + } for i, id := range ids { job := jobs[i] @@ -63,10 +67,14 @@ func Parse(content []byte, options ...ParseOption) ([]*SingleWorkflow, error) { } job.Strategy.RawMatrix = encodeMatrix(matrix) evaluator := NewExpressionEvaluator(NewInterpeter(id, origin.GetJob(id), matrix, pc.gitContext, results, pc.vars, pc.inputs)) - job.Name = nameWithMatrix(job.Name, matrix, evaluator) + if job.Name, err = nameWithMatrix(job.Name, matrix, evaluator); err != nil { + return nil, fmt.Errorf("interpolate name for job %q: %w", id, err) + } runsOn := origin.GetJob(id).RunsOn() for i, v := range runsOn { - runsOn[i] = evaluator.Interpolate(v) + if runsOn[i], err = evaluator.interpolate(v); err != nil { + return nil, fmt.Errorf("interpolate runs-on for job %q: %w", id, err) + } } job.RawRunsOn = encodeRunsOn(runsOn) if err := evaluator.EvaluateYamlNode(&job.RawContinueOnError); err != nil { @@ -150,16 +158,45 @@ func encodeRunsOn(runsOn []string) yaml.Node { return node } -func nameWithMatrix(name string, m map[string]any, evaluator *ExpressionEvaluator) string { +func nameWithMatrix(name string, m map[string]any, evaluator *ExpressionEvaluator) (string, error) { if len(m) == 0 { - return name + return name, nil } if !strings.Contains(name, "${{") || !strings.Contains(name, "}}") { - return name + " " + matrixName(m) + return name + " " + matrixName(m), nil } - return evaluator.Interpolate(name) + return evaluator.interpolate(name) +} + +// expressionCallsFunction reports whether any ${{ }} expression in value calls one of the functions. +func expressionCallsFunction(value string, names ...string) bool { + parts, err := splitSubExpressions(value) + if err != nil { + return true // unparseable here, let the expansion report it against the real values + } + for _, part := range parts { + if !part.isExpr { + continue + } + // The lexer needs the closing `}}` that the scanner strips. + expr, err := actionlint.NewExprParser().Parse(actionlint.NewExprLexer(part.text + "}}")) + if err != nil { + return true // unparseable here, let the expansion report it against the real values + } + found := false + actionlint.VisitExprNode(expr, func(node, _ actionlint.ExprNode, entering bool) { + call, ok := node.(*actionlint.FuncCallNode) + if entering && ok && slices.Contains(names, strings.ToLower(call.Callee)) { + found = true + } + }) + if found { + return true + } + } + return false } func matrixName(m map[string]any) string { diff --git a/modules/actions/jobparser/jobparser_test.go b/modules/actions/jobparser/jobparser_test.go index 05bb3151b7d..ca8493ef1cb 100644 --- a/modules/actions/jobparser/jobparser_test.go +++ b/modules/actions/jobparser/jobparser_test.go @@ -7,6 +7,7 @@ import ( "strings" "testing" + "gitea.com/gitea/runner/act/model" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.yaml.in/yaml/v4" @@ -107,3 +108,42 @@ func TestParse(t *testing.T) { }) } } + +func TestParseInterpolatesRunName(t *testing.T) { + workflow := func(runName string) []byte { + return []byte("name: t\nrun-name: \"" + runName + "\"\non: push\njobs:\n build:\n runs-on: ubuntu-latest\n steps: [{run: echo}]\n") + } + + for _, tt := range []struct{ name, runName, want string }{ + {"bool", "${{ true }}", "true"}, + {"int", "${{ 1 }}", "1"}, + {"float", "${{ 1.0 }}", "1"}, + {"null", "${{ null }}", ""}, + {"object", `${{ fromJSON('{\"a\":1}') }}`, "Object"}, + {"array", "${{ fromJSON('[1,2]') }}", "Array"}, + {"context", "${{ github }}", "Object"}, + {"surrounding literals", "run ${{ 1 }} now", "run 1 now"}, + {"two expressions", "${{ 1 }}-${{ true }}", "1-true"}, + {"closing brace inside a string", "${{ 'a}}b' }}", "a}}b"}, + {"incomplete expression stays literal", "${{ 1", "${{ 1"}, + } { + t.Run(tt.name, func(t *testing.T) { + result, err := Parse(workflow(tt.runName), WithGitContext(&model.GithubContext{EventName: "push"})) + require.NoError(t, err) + require.Len(t, result, 1) + assert.Equal(t, tt.want, result[0].RunName) + }) + } + + // a malformed part must not restructure the surrounding expression + for _, runName := range []string{"${{ 1) && (2 }}", "run ${{ 1) && (2 }} now", "${{ 'a' }} ${{ b", "${{ 'a }}"} { + _, err := Parse(workflow(runName), WithGitContext(&model.GithubContext{EventName: "push"})) + assert.ErrorContains(t, err, "interpolate run-name") + } + + // callers such as commit status parse without a git context, leaving `github` a nil pointer + result, err := Parse(workflow("${{ github }}")) + require.NoError(t, err) + require.Len(t, result, 1) + assert.Empty(t, result[0].RunName) +} diff --git a/modules/actions/jobparser/model.go b/modules/actions/jobparser/model.go index 1df8d89350d..0953fe1229a 100644 --- a/modules/actions/jobparser/model.go +++ b/modules/actions/jobparser/model.go @@ -8,7 +8,6 @@ import ( "errors" "fmt" - "gitea.com/gitea/runner/act/exprparser" "gitea.com/gitea/runner/act/model" "go.yaml.in/yaml/v4" ) @@ -518,15 +517,7 @@ func EvaluateJobIfExpression(jobID string, job *Job, gitCtx map[string]any, resu matrix = matrixes[0] } evaluator := NewExpressionEvaluator(NewInterpeter(jobID, actJob, matrix, toGitContext(gitCtx), results, vars, inputs)) - expr, err := rewriteSubExpression(job.If.Value, false) - if err != nil { - return false, err - } - result, err := evaluator.evaluate(expr, exprparser.DefaultStatusCheckSuccess) - if err != nil { - return false, err - } - return exprparser.IsTruthy(result), nil + return evaluator.evaluateCondition(job.If.Value) } // parseMappingNode parse a mapping node and preserve order. diff --git a/modules/actions/jobparser/model_test.go b/modules/actions/jobparser/model_test.go index 23d7489fedb..cc05871df77 100644 --- a/modules/actions/jobparser/model_test.go +++ b/modules/actions/jobparser/model_test.go @@ -527,6 +527,10 @@ func TestEvaluateJobIfExpression(t *testing.T) { {name: "cancelled", ifCond: "${{ cancelled() }}", needResult: "success", expected: false}, {name: "not cancelled or failure", ifCond: "${{ !(cancelled() || failure()) }}", needResult: "success", expected: true}, {name: "not cancelled or failure, need failed", ifCond: "${{ !(cancelled() || failure()) }}", needResult: "failure", expected: false}, + // a condition is an expression with or without `${{ }}`, literal text around one makes it a string + {name: "bare expression", ifCond: "always()", needResult: "failure", expected: true}, + {name: "literal text keeps the success() default", ifCond: "x ${{ 1 }}", needResult: "failure", expected: false}, + {name: "literal text around a status function drops it", ifCond: "x ${{ always() }}", needResult: "failure", expected: true}, } for _, kase := range kases { t.Run(kase.name, func(t *testing.T) { diff --git a/modules/actions/jobparser/workflow_call.go b/modules/actions/jobparser/workflow_call.go index 7c534dca3c0..6f725dd175d 100644 --- a/modules/actions/jobparser/workflow_call.go +++ b/modules/actions/jobparser/workflow_call.go @@ -260,7 +260,7 @@ func MatchCallerInputsAgainstSpec(spec *WorkflowCallSpec, evaluated map[string]a func parseWorkflowCallInput(name string, typ InputType, v any) (any, error) { switch typ { case InputTypeString: - return toString(v), nil + return coerceToString(v), nil case InputTypeBoolean: // strict type matching: a boolean input only accepts a native bool, not a "true"/"false" string if b, ok := v.(bool); ok { @@ -361,11 +361,11 @@ func EvaluateWorkflowCallOutputs(spec *WorkflowCallSpec, gitCtx *model.GithubCon Vars: vars, Inputs: inputs, } - interpreter := exprparser.NewInterpeter(env, exprparser.Config{}) + evaluator := NewExpressionEvaluator(exprparser.NewInterpeter(env, exprparser.Config{})) out := make(map[string]string, len(spec.Outputs)) for name, o := range spec.Outputs { - v, err := evaluateWorkflowCallOutputValue(interpreter, o.Value) + v, err := evaluator.interpolate(o.Value) if err != nil { return nil, fmt.Errorf("workflow_call output %q: %w", name, err) } @@ -373,29 +373,3 @@ func EvaluateWorkflowCallOutputs(spec *WorkflowCallSpec, gitCtx *model.GithubCon } return out, nil } - -func evaluateWorkflowCallOutputValue(interpreter exprparser.Interpreter, value string) (string, error) { - if !strings.Contains(value, "${{") || !strings.Contains(value, "}}") { - return value, nil - } - expr, err := rewriteSubExpression(value, true) - if err != nil { - return "", err - } - evaluated, err := interpreter.Evaluate(expr, exprparser.DefaultStatusCheckNone) - if err != nil { - return "", err - } - return toString(evaluated), nil -} - -func toString(v any) string { - switch s := v.(type) { - case string: - return s - case nil: - return "" - default: - return fmt.Sprintf("%v", s) - } -}