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 d6ce23daa4f..7e851d03a30 100644 --- a/modules/actions/jobparser/jobparser.go +++ b/modules/actions/jobparser/jobparser.go @@ -72,9 +72,14 @@ func ExpressionReadsMatrix(ifValue string) bool { // the status functions that run a job whatever its needs did rather than under the implicit success(). // Keep in sync with act's exprparser, which owns the same list for the evaluation itself. func ExpressionIgnoresNeedResults(ifValue string) bool { - return expressionsMatch(asIfExpression(ifValue), func(node actionlint.ExprNode) bool { + return expressionCallsFunction(asIfExpression(ifValue), "always", "failure", "cancelled") +} + +// expressionCallsFunction reports whether any ${{ }} expression in value calls one of the functions. +func expressionCallsFunction(value string, names ...string) bool { + return expressionsMatch(value, func(node actionlint.ExprNode) bool { call, ok := node.(*actionlint.FuncCallNode) - return ok && slices.Contains([]string{"always", "failure", "cancelled"}, strings.ToLower(call.Callee)) + return ok && slices.Contains(names, strings.ToLower(call.Callee)) }) } @@ -98,14 +103,16 @@ func expressionReadsContext(value, contextName string) bool { // expressionsMatch reports whether any ${{ }} expression in value holds a node the predicate accepts. func expressionsMatch(value string, match func(node actionlint.ExprNode) bool) bool { - for rest := value; ; { - _, after, found := strings.Cut(rest, "${{") - if !found { - return false + 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 } - rest = after - // The lexer ends the expression at its closing `}}`, so it can be handed the whole remainder. - expr, err := actionlint.NewExprParser().Parse(actionlint.NewExprLexer(rest)) + // 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 } @@ -119,6 +126,7 @@ func expressionsMatch(value string, match func(node actionlint.ExprNode) bool) b return true } } + return false } func Parse(content []byte, options ...ParseOption) ([]*SingleWorkflow, error) { @@ -155,7 +163,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] @@ -289,6 +299,7 @@ func validateMatrixFilters(job *model.Job) error { func buildMatrixCombos(jobID string, src *Job, matrixes []map[string]any, actJob *model.Job, gitCtx *model.GithubContext, results map[string]*JobResult, vars map[string]string, inputs map[string]any) ([]*Job, error) { srcRunsOn := src.RunsOn() combos := make([]*Job, 0, len(matrixes)) + var err error for _, matrix := range matrixes { combo := src.Clone() if combo.Name == "" { @@ -296,10 +307,14 @@ func buildMatrixCombos(jobID string, src *Job, matrixes []map[string]any, actJob } combo.Strategy.RawMatrix = encodeMatrix(matrix) evaluator := NewExpressionEvaluator(NewInterpeter(jobID, actJob, matrix, gitCtx, results, vars, inputs)) - combo.Name = nameWithMatrix(combo.Name, matrix, evaluator) + if combo.Name, err = nameWithMatrix(combo.Name, matrix, evaluator); err != nil { + return nil, fmt.Errorf("interpolate name for job %q: %w", jobID, err) + } runsOn := slices.Clone(srcRunsOn) for i := range runsOn { - runsOn[i] = evaluator.Interpolate(runsOn[i]) + if runsOn[i], err = evaluator.interpolate(runsOn[i]); err != nil { + return nil, fmt.Errorf("interpolate runs-on for job %q: %w", jobID, err) + } } combo.RawRunsOn = encodeRunsOn(runsOn) if err := evaluator.EvaluateYamlNode(&combo.RawContinueOnError); err != nil { @@ -371,16 +386,16 @@ 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) } func matrixName(m map[string]any) string { diff --git a/modules/actions/jobparser/jobparser_test.go b/modules/actions/jobparser/jobparser_test.go index 00fc4bdc22e..c612937e469 100644 --- a/modules/actions/jobparser/jobparser_test.go +++ b/modules/actions/jobparser/jobparser_test.go @@ -160,6 +160,45 @@ jobs: } } +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) +} + func TestExpandMatrixWithNeeds(t *testing.T) { // matrixYAML is the YAML value of the `matrix:` key, so a case can replace the whole node. expandMax := func(t *testing.T, matrixYAML string, maxCombinations int) ([]*Job, error) { diff --git a/modules/actions/jobparser/model.go b/modules/actions/jobparser/model.go index 4406bb77153..e08fb2a72e9 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" ) @@ -526,15 +525,7 @@ func EvaluateJobIfExpression(jobID string, job *Job, gitCtx map[string]any, resu } } 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 3ceb3f196f5..48ee90f28d0 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 1b01192af13..b6a7e3fa97c 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) - } -}