ink/act_runner
1
1
Fork 0
mirror of https://gitea.com/gitea/act_runner.git synced 2026-08-07 01:14:22 +02:00
Code Issues Packages Projects Releases Wiki Activity

fix: evaluate each ${{ }} part on its own (#1146)

Browse Source 
Every `${{ }}` part was spliced as raw text into a synthesized `format('...', <raw>)` call and re-parsed, so unbalanced parentheses restructured the whole expression:

```yaml
run: echo ${{ 1) && (2 }}      # panicked with "did not evaluate to a string"
if: ${{ 1 }} ${{ 0) && (0 }}   # silently skipped the step
```

One scanner shaped like GitHub's template reader now splits every value, and each part is evaluated on its own, so nothing builds an expression out of text.

An empty input to `Evaluate` asked `success()` whatever the caller requested, so a post step running under `always()` asked the wrong question.

Same fix as https://github.com/go-gitea/gitea/pull/38754 on the Gitea side.

Written by Claude Opus 5.

---------

Co-authored-by: bircni <bircni@icloud.com>
Reviewed-on: https://gitea.com/gitea/runner/pulls/1146
Reviewed-by: bircni <bircni@icloud.com>
Co-authored-by: silverwind <me@silverwind.io>
This commit is contained in:
silverwind
2026-08-06 16:05:39 +00:00
committed by bircni
parent 09b643bc14
commit 20497aaf4f
4 changed files with 202 additions and 154 deletions
Download Patch File Download Diff File Expand all files Collapse all files

52
act/exprparser/interpreter.go
View File

@@ -85,37 +85,26 @@ func NewInterpeter(env *EvaluationEnvironment, config Config) Interpreter {
}
}
// Evaluate evaluates one expression. An empty input asks defaultStatusCheck on its own, which is
// what a value that carries no expression of its own runs under.
func (impl *interperterImpl) Evaluate(input string, defaultStatusCheck DefaultStatusCheck) (any, error) {
input = strings.TrimPrefix(input, "${{")
if defaultStatusCheck != DefaultStatusCheckNone && input == "" {
input = "success()"
if input == "" && defaultStatusCheck != DefaultStatusCheckNone {
return impl.evaluateNode(statusCheckNode(defaultStatusCheck))
}
parser := actionlint.NewExprParser()
exprNode, err := parser.Parse(actionlint.NewExprLexer(input + "}}"))
if err != nil {
return nil, fmt.Errorf("Failed to parse: %s", err.Message)
}
if defaultStatusCheck != DefaultStatusCheckNone {
hasStatusCheckFunction := false
actionlint.VisitExprNode(exprNode, func(node, _ actionlint.ExprNode, entering bool) {
if funcCallNode, ok := node.(*actionlint.FuncCallNode); entering && ok {
switch strings.ToLower(funcCallNode.Callee) {
case "success", "always", "cancelled", "failure":
hasStatusCheckFunction = true
}
}
})
if !hasStatusCheckFunction {
exprNode = &actionlint.LogicalOpNode{
Kind: actionlint.LogicalOpNodeKindAnd,
Left: &actionlint.FuncCallNode{
Callee: defaultStatusCheck.String(),
Args: []actionlint.ExprNode{},
},
Right: exprNode,
}
if defaultStatusCheck != DefaultStatusCheckNone && !CallsStatusFunction(exprNode) {
exprNode = &actionlint.LogicalOpNode{
Kind: actionlint.LogicalOpNodeKindAnd,
Left: statusCheckNode(defaultStatusCheck),
Right: exprNode,
}
}
@@ -124,6 +113,25 @@ func (impl *interperterImpl) Evaluate(input string, defaultStatusCheck DefaultSt
return result, err2
}
func statusCheckNode(defaultStatusCheck DefaultStatusCheck) *actionlint.FuncCallNode {
return &actionlint.FuncCallNode{Callee: defaultStatusCheck.String(), Args: []actionlint.ExprNode{}}
}
// CallsStatusFunction reports whether the expression calls a status function, which counts as the
// expression asking its own status question instead of the default one.
func CallsStatusFunction(exprNode actionlint.ExprNode) bool {
found := false
actionlint.VisitExprNode(exprNode, func(node, _ actionlint.ExprNode, entering bool) {
if funcCallNode, ok := node.(*actionlint.FuncCallNode); entering && ok {
switch strings.ToLower(funcCallNode.Callee) {
case "success", "always", "cancelled", "failure":
found = true
}
}
})
return found
}
func (impl *interperterImpl) evaluateNode(exprNode actionlint.ExprNode) (any, error) {
switch node := exprNode.(type) {
case *actionlint.VariableNode:

7
act/exprparser/interpreter_test.go
View File

@@ -682,3 +682,10 @@ func TestCoerceToString(t *testing.T) {
})
}
}
func TestEvaluateEmptyInputAsksItsOwnStatusCheck(t *testing.T) {
// always() needs no job or step context, so it shows which function an empty input asks for
output, err := NewInterpeter(&EvaluationEnvironment{}, Config{}).Evaluate("", DefaultStatusCheckAlways)
require.NoError(t, err)
assert.Equal(t, true, output)
}

211
act/runner/expression.go
View File

@@ -7,6 +7,7 @@ package runner
import (
"bytes"
"context"
"errors"
"fmt"
"maps"
"path"
@@ -22,12 +23,14 @@ import (
_ "embed"
"github.com/rhysd/actionlint"
"go.yaml.in/yaml/v4"
)
// ExpressionEvaluator is the interface for evaluating expressions
type ExpressionEvaluator interface {
evaluate(context.Context, string, exprparser.DefaultStatusCheck) (any, error)
interpolate(context.Context, string) (string, error)
EvaluateYamlNode(context.Context, *yaml.Node) error
Interpolate(context.Context, string) string
}
@@ -240,8 +243,7 @@ func (ee expressionEvaluator) evaluateScalarYamlNode(ctx context.Context, node *
if !strings.Contains(in, "${{") || !strings.Contains(in, "}}") {
return nil, nil //nolint:nilnil // pre-existing issue from nektos/act
}
expr, _ := rewriteSubExpression(ctx, in, false)
res, err := ee.evaluate(ctx, expr, exprparser.DefaultStatusCheckNone)
res, err := ee.evaluateScalar(ctx, in)
if err != nil {
return nil, err
}
@@ -367,105 +369,146 @@ func (ee expressionEvaluator) EvaluateYamlNode(ctx context.Context, node *yaml.N
}
func (ee expressionEvaluator) Interpolate(ctx context.Context, in string) string {
if !strings.Contains(in, "${{") || !strings.Contains(in, "}}") {
return in
}
expr, _ := rewriteSubExpression(ctx, in, true)
evaluated, err := ee.evaluate(ctx, expr, exprparser.DefaultStatusCheckNone)
out, err := ee.interpolate(ctx, in)
if err != nil {
common.Logger(ctx).Errorf("Unable to interpolate expression '%s': %s", expr, err)
common.Logger(ctx).Errorf("Unable to interpolate expression '%s': %s", in, err)
return ""
}
value, ok := evaluated.(string)
if !ok {
panic(fmt.Sprintf("Expression %s did not evaluate to a string", expr))
}
return value
return out
}
// EvalBool evaluates an expression against given evaluator
func EvalBool(ctx context.Context, evaluator ExpressionEvaluator, expr string, defaultStatusCheck exprparser.DefaultStatusCheck) (bool, error) {
nextExpr, _ := rewriteSubExpression(ctx, expr, false)
// interpolate evaluates every part on its own, so a malformed one cannot restructure its neighbours
func (ee expressionEvaluator) interpolate(ctx context.Context, in string) (string, error) {
parts, err := splitSubExpressions(in)
if err != nil {
return "", err
}
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.evaluate(ctx, part.text, exprparser.DefaultStatusCheckNone)
if err != nil {
return "", err
}
out.WriteString(exprparser.CoerceToString(evaluated))
}
return out.String(), nil
}
evaluated, err := evaluator.evaluate(ctx, nextExpr, defaultStatusCheck)
// evaluateScalar keeps the type of a lone expression, so `${{ fromJSON('[1,2]') }}` stays an array
func (ee expressionEvaluator) evaluateScalar(ctx context.Context, in string) (any, error) {
parts, err := splitSubExpressions(in)
if err != nil {
return nil, err
}
if len(parts) == 1 && parts[0].isExpr {
return ee.evaluate(ctx, parts[0].text, exprparser.DefaultStatusCheckNone)
}
return ee.interpolate(ctx, in)
}
// EvalBool evaluates an expression against given evaluator. An `if:` is an expression even without
// `${{ }}`, while literal text around one makes the whole value a string.
func EvalBool(ctx context.Context, evaluator ExpressionEvaluator, expr string, defaultStatusCheck exprparser.DefaultStatusCheck) (bool, error) {
parts, err := splitSubExpressions(expr)
if err != nil {
return false, err
}
return exprparser.IsTruthy(evaluated), nil
}
func escapeFormatString(in string) string {
return strings.ReplaceAll(strings.ReplaceAll(in, "{", "{{"), "}", "}}")
}
func rewriteSubExpression(ctx context.Context, in string, forceFormat bool) (string, error) { //nolint:unparam // pre-existing issue from nektos/act
if !strings.Contains(in, "${{") || !strings.Contains(in, "}}") {
return in, nil
if len(parts) == 1 {
evaluated, err := evaluator.evaluate(ctx, parts[0].text, defaultStatusCheck)
if err != nil {
return false, err
}
return exprparser.IsTruthy(evaluated), 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 {
panic("unclosed string.")
}
strStart = -1
pos += matches[1]
} else if exprStart > -1 {
exprEnd := strings.Index(in[pos:], "}}")
strStart = strings.Index(in[pos:], "'")
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 status check applies to it separately
if defaultStatusCheck != exprparser.DefaultStatusCheckNone && !callsStatusFunction(parts) {
status, err := evaluator.evaluate(ctx, "", defaultStatusCheck)
if err != nil {
return false, err
}
if !exprparser.IsTruthy(status) {
return false, nil
}
}
interpolated, err := evaluator.interpolate(ctx, expr)
if err != nil {
return false, err
}
return exprparser.IsTruthy(interpolated), nil
}
if len(results) == 1 && formatOut.String() == "{0}" && !forceFormat {
return in, nil
// callsStatusFunction reports whether any part calls a status function. A part that does not parse
// counts as one, so the evaluation reports it against the real values.
func callsStatusFunction(parts []exprPart) bool {
for _, part := range parts {
if !part.isExpr {
continue
}
// The lexer needs the closing `}}` that the scanner strips.
exprNode, err := actionlint.NewExprParser().Parse(actionlint.NewExprLexer(part.text + "}}"))
if err != nil || exprparser.CallsStatusFunction(exprNode) {
return true
}
}
return false
}
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
}
out := fmt.Sprintf("format('%s', %s)", strings.ReplaceAll(formatOut.String(), "'", "''"), strings.Join(results, ", "))
if in != out {
common.Logger(ctx).Debugf("expression '%s' rewritten to '%s'", in, out)
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("}}"):]
}
return out, nil
}
// 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
}
func getEvaluatorInputs(ctx context.Context, rc *RunContext, step step, ghc *model.GithubContext) map[string]any {

86
act/runner/expression_test.go
View File

@@ -259,6 +259,15 @@ func TestInterpolate(t *testing.T) {
{"${{ env.SOMETHING_FALSE || false }}", "false"},
{"${{ env.SOMETHING_FALSE }} && ${{ env.SOMETHING_TRUE }}", "false && true"},
{"${{ fromJSON('{}') < 2 }}", "false"},
{"${{ 1 }}", "1"},
{"${{ 1.0 }}", "1"},
{"${{ null }}", ""},
{"${{ fromJSON('[1,2]') }}", "Array"},
{"${{ fromJSON('{\"a\":1}') }}", "Object"},
// a malformed part must not restructure its neighbours, and it interpolates to nothing
{"${{ 1) && (2 }}", ""},
{"run ${{ 1) && (2 }} now", ""},
{"${{ 1", "${{ 1"},
}
for _, table := range tables {
@@ -270,57 +279,38 @@ func TestInterpolate(t *testing.T) {
}
}
func TestRewriteSubExpression(t *testing.T) {
table := []struct {
in string
out string
func TestSplitSubExpressions(t *testing.T) {
expr := func(text string) exprPart { return exprPart{text: text, isExpr: true} }
literal := func(text string) exprPart { return exprPart{text: text} }
for _, tt := range []struct {
in string
want []exprPart
}{
{in: "Hello World", out: "Hello World"},
{in: "${{ true }}", out: "${{ true }}"},
{in: "${{ true }} ${{ true }}", out: "format('{0} {1}', true, true)"},
{in: "${{ true || false }} ${{ true && true }}", out: "format('{0} {1}', true || false, true && true)"},
{in: "${{ '}}' }}", out: "${{ '}}' }}"},
{in: "${{ '''}}''' }}", out: "${{ '''}}''' }}"},
{in: "${{ '''' }}", out: "${{ '''' }}"},
{in: `${{ fromJSON('"}}"') }}`, out: `${{ fromJSON('"}}"') }}`},
{in: `${{ fromJSON('"\"}}\""') }}`, out: `${{ fromJSON('"\"}}\""') }}`},
{in: `${{ fromJSON('"''}}"') }}`, out: `${{ fromJSON('"''}}"') }}`},
{in: "Hello ${{ 'World' }}", out: "format('Hello {0}', 'World')"},
{"Hello World", []exprPart{literal("Hello World")}},
{"${{ true }}", []exprPart{expr("true")}},
{"${{ true }} ${{ false }}", []exprPart{expr("true"), literal(" "), expr("false")}},
{"Hello ${{ 'World' }}", []exprPart{literal("Hello "), expr("'World'")}},
// a quote toggles string state, so a `}}` inside a string does not end the expression
{"${{ '}}' }}", []exprPart{expr("'}}'")}},
{"${{ '''}}''' }}", []exprPart{expr("'''}}'''")}},
{"${{ '''' }}", []exprPart{expr("''''")}},
{`${{ fromJSON('"}}"') }}`, []exprPart{expr(`fromJSON('"}}"')`)}},
{`${{ fromJSON('"\"}}\""') }}`, []exprPart{expr(`fromJSON('"\"}}\""')`)}},
{`${{ fromJSON('"''}}"') }}`, []exprPart{expr(`fromJSON('"''}}"')`)}},
// without a complete literal the value stays text, as GitHub's template reader leaves it
{"${{ 1", []exprPart{literal("${{ 1")}},
// a malformed part stays one part, so it cannot restructure its neighbours
{"${{ 1) && (2 }}", []exprPart{expr("1) && (2")}},
} {
got, err := splitSubExpressions(tt.in)
require.NoError(t, err, tt.in)
assert.Equal(t, tt.want, got, tt.in)
}
for _, table := range table {
t.Run("TestRewriteSubExpression", func(t *testing.T) {
assertObject := assert.New(t)
out, err := rewriteSubExpression(context.Background(), table.in, false)
if err != nil {
t.Fatal(err)
}
assertObject.Equal(table.out, out, table.in)
})
}
}
func TestRewriteSubExpressionForceFormat(t *testing.T) {
table := []struct {
in string
out string
}{
{in: "Hello World", out: "Hello World"},
{in: "${{ true }}", out: "format('{0}', true)"},
{in: "${{ '}}' }}", out: "format('{0}', '}}')"},
{in: `${{ fromJSON('"}}"') }}`, out: `format('{0}', fromJSON('"}}"'))`},
{in: "Hello ${{ 'World' }}", out: "format('Hello {0}', 'World')"},
}
for _, table := range table {
t.Run("TestRewriteSubExpressionForceFormat", func(t *testing.T) {
assertObject := assert.New(t)
out, err := rewriteSubExpression(context.Background(), table.in, true)
if err != nil {
t.Fatal(err)
}
assertObject.Equal(table.out, out, table.in)
})
for _, in := range []string{"${{ 'a' }} ${{ b", "${{ 'a }}"} {
_, err := splitSubExpressions(in)
assert.ErrorContains(t, err, "unclosed expression", in)
}
}