From 212909db7b40a5d4d6127bb1f50d7a475335ee9a Mon Sep 17 00:00:00 2001 From: ABiscuitttt <773542570@qq.com> Date: Wed, 26 Aug 2026 14:45:05 +0000 Subject: [PATCH] fix: keep a step's own with: values out of its inputs context (#1192) A step's `if:`, its `continue-on-error:` and its `run:` script resolved `inputs.*` from the step's own `INPUT_*` env. A `with:` key colliding with a workflow input flipped conditions, and any `INPUT_`-shaped variable from `env:` or a `GITHUB_ENV` write forged an input that never existed. GitHub evaluates all three in the enclosing scope: the workflow inputs, or for a composite action's steps that action's inputs. Action-input interpolation is the one place that legitimately sees a step's own `with:`, so it keeps its own evaluator. Fixes https://gitea.com/gitea/runner/issues/1191, ports https://github.com/nektos/act/pull/2473 and extends it to the pre and post stages. --------- Co-authored-by: silverwind Reviewed-on: https://gitea.com/gitea/runner/pulls/1192 Reviewed-by: silverwind <2021+silverwind@noreply.gitea.com> Co-authored-by: ABiscuitttt <773542570@qq.com> --- act/runner/action.go | 6 +-- act/runner/action_composite.go | 4 +- act/runner/expression.go | 38 ++++++++++--------- act/runner/expression_test.go | 3 ++ act/runner/run_context.go | 8 ++++ act/runner/run_context_test.go | 8 ++++ act/runner/runner_test.go | 1 + act/runner/step_action_remote.go | 2 +- act/runner/step_test.go | 14 +++++++ .../uses-step-if-inputs-not-leaked/push.yml | 20 ++++++++++ 10 files changed, 81 insertions(+), 23 deletions(-) create mode 100644 act/runner/testdata/uses-step-if-inputs-not-leaked/push.yml diff --git a/act/runner/action.go b/act/runner/action.go index 10d36049..2fe4c8ca 100644 --- a/act/runner/action.go +++ b/act/runner/action.go @@ -346,7 +346,7 @@ func execAsDocker(ctx context.Context, step actionStep, actionName, actionDir, b logger.Debugf("image '%s' for architecture '%s' already exists", image, rc.Config.ContainerArchitecture) } } - eval := rc.NewStepExpressionEvaluator(ctx, step) + eval := rc.NewActionInputsExpressionEvaluator(ctx, step) cmd, err := shellquote.Split(eval.Interpolate(ctx, step.getStepModel().With["args"])) if err != nil { return err @@ -410,13 +410,13 @@ func evalDockerArgs(ctx context.Context, step step, action *model.Action, cmd *[ } mergeIntoMap(step, step.getEnv(), inputs) - stepEE := rc.NewStepExpressionEvaluator(ctx, step) + stepEE := rc.NewActionInputsExpressionEvaluator(ctx, step) for i, v := range *cmd { (*cmd)[i] = stepEE.Interpolate(ctx, v) } mergeIntoMap(step, step.getEnv(), action.Runs.Env) - ee := rc.NewStepExpressionEvaluator(ctx, step) + ee := rc.NewActionInputsExpressionEvaluator(ctx, step) for k, v := range *step.getEnv() { (*step.getEnv())[k] = ee.Interpolate(ctx, v) } diff --git a/act/runner/action_composite.go b/act/runner/action_composite.go index 74cb7e5c..30eb3fc4 100644 --- a/act/runner/action_composite.go +++ b/act/runner/action_composite.go @@ -28,7 +28,7 @@ func evaluateCompositeInputAndEnv(ctx context.Context, parent *RunContext, step } } - ee := parent.NewStepExpressionEvaluator(ctx, step) + ee := parent.NewActionInputsExpressionEvaluator(ctx, step) for inputID, input := range step.getActionModel().Inputs { envKey := regexp.MustCompile("[^A-Z0-9-]").ReplaceAllString(strings.ToUpper(inputID), "_") @@ -75,13 +75,13 @@ func newCompositeRunContext(ctx context.Context, parent *RunContext, step action StepResults: map[string]*model.StepResult{}, JobContainer: parent.JobContainer, ActionPath: actionPath, - Env: env, GlobalEnv: parent.GlobalEnv, Masks: parent.Masks, ExtraPath: parent.ExtraPath, Parent: parent, EventJSON: parent.EventJSON, } + compositerc.setActionEnv(env) compositerc.ExprEval = compositerc.NewExpressionEvaluator(ctx) return compositerc diff --git a/act/runner/expression.go b/act/runner/expression.go index 197af546..73a10f1b 100644 --- a/act/runner/expression.go +++ b/act/runner/expression.go @@ -71,7 +71,7 @@ func (rc *RunContext) NewExpressionEvaluatorWithEnv(ctx context.Context, env map } ghc := rc.getGithubContext(ctx) - inputs := getEvaluatorInputs(ctx, rc, nil, ghc) + inputs := getEvaluatorInputs(ctx, rc, rc.actionInputs, ghc) ee := &exprparser.EvaluationEnvironment{ Github: ghc, @@ -102,8 +102,17 @@ func (rc *RunContext) NewExpressionEvaluatorWithEnv(ctx context.Context, env map //go:embed hashfiles/index.js var hashfiles string -// NewStepExpressionEvaluator creates a new evaluator +// NewStepExpressionEvaluator creates a new evaluator with the `inputs` of the enclosing workflow or composite action func (rc *RunContext) NewStepExpressionEvaluator(ctx context.Context, step step) *ExpressionEvaluator { + return rc.newStepExpressionEvaluator(ctx, step, rc.actionInputs) +} + +// NewActionInputsExpressionEvaluator creates a new evaluator with the step's own with: values as `inputs` +func (rc *RunContext) NewActionInputsExpressionEvaluator(ctx context.Context, step step) *ExpressionEvaluator { + return rc.newStepExpressionEvaluator(ctx, step, inputsFromEnv(*step.getEnv())) +} + +func (rc *RunContext) newStepExpressionEvaluator(ctx context.Context, step step, stepInputs map[string]any) *ExpressionEvaluator { // todo: cleanup EvaluationEnvironment creation job := rc.Run.Job() strategy := make(map[string]any) @@ -123,9 +132,6 @@ func (rc *RunContext) NewStepExpressionEvaluator(ctx context.Context, step step) } } - ghc := rc.getGithubContext(ctx) - inputs := getEvaluatorInputs(ctx, rc, step, ghc) - ee := &exprparser.EvaluationEnvironment{ Github: step.getGithubContext(ctx), Env: *step.getEnv(), @@ -138,7 +144,7 @@ func (rc *RunContext) NewStepExpressionEvaluator(ctx context.Context, step step) Needs: using, // todo: should be unavailable // but required to interpolate/evaluate the inputs in actions/composite - Inputs: inputs, + Inputs: getEvaluatorInputs(ctx, rc, stepInputs, rc.getGithubContext(ctx)), HashFiles: getHashFilesFunction(ctx, rc), } ee.Runner = rc.getRunnerContext(ctx) @@ -261,23 +267,21 @@ func EvalBool(ctx context.Context, evaluator *expressionEvaluator, expr string, }).EvalBool(expr, defaultStatusCheck) } -func getEvaluatorInputs(ctx context.Context, rc *RunContext, step step, ghc *model.GithubContext) map[string]any { +func inputsFromEnv(env map[string]string) map[string]any { inputs := map[string]any{} - - setupWorkflowInputs(ctx, &inputs, rc) - - var env map[string]string - if step != nil { - env = *step.getEnv() - } else { - env = rc.GetEnv() - } - for k, v := range env { if after, ok := strings.CutPrefix(k, "INPUT_"); ok { inputs[strings.ToLower(after)] = v } } + return inputs +} + +func getEvaluatorInputs(ctx context.Context, rc *RunContext, stepInputs map[string]any, ghc *model.GithubContext) map[string]any { + inputs := map[string]any{} + + setupWorkflowInputs(ctx, &inputs, rc) + maps.Copy(inputs, stepInputs) if ghc.EventName == "workflow_dispatch" { config := rc.Run.Workflow.WorkflowDispatchConfig() diff --git a/act/runner/expression_test.go b/act/runner/expression_test.go index 4120d7b5..bb21e2fa 100644 --- a/act/runner/expression_test.go +++ b/act/runner/expression_test.go @@ -156,8 +156,10 @@ func TestEvaluateRunContext(t *testing.T) { func TestEvaluateStep(t *testing.T) { rc := createRunContext(t) + rc.Env["INPUT_FORGED"] = "leaked" step := &stepRun{ RunContext: rc, + env: map[string]string{"INPUT_FORGED": "leaked"}, } ee := rc.NewStepExpressionEvaluator(context.Background(), step) @@ -176,6 +178,7 @@ func TestEvaluateStep(t *testing.T) { {"steps.id_with_underscores.conclusion", model.StepStatusSuccess.String(), ""}, {"steps.id_with_underscores.outcome", model.StepStatusFailure.String(), ""}, {"steps.id_with_underscores.outputs.foo_with_underscores", "bar_with_underscores", ""}, + {"inputs.forged", nil, ""}, // INPUT_* env is not an input } for _, table := range tables { diff --git a/act/runner/run_context.go b/act/runner/run_context.go index 15aed5ff..c08b6a07 100644 --- a/act/runner/run_context.go +++ b/act/runner/run_context.go @@ -62,6 +62,7 @@ type RunContext struct { JobName string ActionPath string Parent *RunContext + actionInputs map[string]any // inputs of the composite action this runs, nil for a job Masks []string cleanUpJobContainer common.Executor caller *caller // job calling this RunContext (reusable workflows) @@ -179,6 +180,13 @@ func (rc *RunContext) GetEnv() map[string]string { return rc.Env } +// setActionEnv sets a composite action's env, keeping the `inputs` context it derives from +// in sync. Remote actions re-evaluate it per stage, so inputs may change between them. +func (rc *RunContext) setActionEnv(env map[string]string) { + rc.Env = env + rc.actionInputs = inputsFromEnv(env) +} + func (rc *RunContext) jobContainerName() string { // The job id, never evaluated, keeps two jobs apart when masking collapses their names. nameParts := []string{rc.Config.ContainerNamePrefix, "WORKFLOW-" + rc.Run.Workflow.Name, "JOB-" + rc.Run.JobID, rc.Name} diff --git a/act/runner/run_context_test.go b/act/runner/run_context_test.go index e52b86bb..969c3f2b 100644 --- a/act/runner/run_context_test.go +++ b/act/runner/run_context_test.go @@ -992,6 +992,14 @@ func TestRunContextGetEnv(t *testing.T) { } } +// a remote composite action re-evaluates its env per stage, so inputs must follow it +func TestSetActionEnvRefreshesInputs(t *testing.T) { + rc := &RunContext{} + rc.setActionEnv(map[string]string{"INPUT_MSG": "pre"}) + rc.setActionEnv(map[string]string{"INPUT_MSG": "main"}) + assert.Equal(t, map[string]any{"msg": "main"}, rc.actionInputs) +} + func TestCreateContainerNameBoundedForLongMatrixInput(t *testing.T) { longMatrixValue := strings.Repeat("os=ubuntu-latest-go=1.24-node=22-", 20) name := createContainerName( diff --git a/act/runner/runner_test.go b/act/runner/runner_test.go index 02b71d6a..2cfc7b33 100644 --- a/act/runner/runner_test.go +++ b/act/runner/runner_test.go @@ -282,6 +282,7 @@ func TestRunEvent(t *testing.T) { {workdir, "uses-composite", "push", "", platforms, secrets}, {workdir, "uses-composite-with-error", "push", "Job 'failing-composite-action' failed", platforms, secrets}, {workdir, "uses-docker-url", "push", "", platforms, secrets}, + {workdir, "uses-step-if-inputs-not-leaked", "push", "", platforms, secrets}, {workdir, "act-composite-env-test", "push", "", platforms, secrets}, // Eval diff --git a/act/runner/step_action_remote.go b/act/runner/step_action_remote.go index 4ac30bd2..f13fcf1d 100644 --- a/act/runner/step_action_remote.go +++ b/act/runner/step_action_remote.go @@ -245,7 +245,7 @@ func (sar *stepActionRemote) getCompositeRunContext(ctx context.Context) *RunCon // input for this action during the main stage, but the env // was already created during the pre stage) env := evaluateCompositeInputAndEnv(ctx, sar.RunContext, sar) - sar.compositeRunContext.Env = env + sar.compositeRunContext.setActionEnv(env) sar.compositeRunContext.ExtraPath = sar.RunContext.ExtraPath } return sar.compositeRunContext diff --git a/act/runner/step_test.go b/act/runner/step_test.go index 8076ae1e..1fc27b2b 100644 --- a/act/runner/step_test.go +++ b/act/runner/step_test.go @@ -279,6 +279,13 @@ func TestIsStepEnabled(t *testing.T) { Conclusion: model.StepStatusFailure, } assertObject.True(isStepEnabled(context.Background(), step.getStepModel().If.Value, step, stepStageMain)) + + // neither env nor the step's own with: values are inputs, at any stage + step = createTestStep(t, "if: inputs.forged") + step.getRunContext().Env["INPUT_FORGED"] = "leaked" + *step.getEnv() = map[string]string{"INPUT_FORGED": "leaked"} + assertObject.False(isStepEnabled(context.Background(), step.getStepModel().If.Value, step, stepStageMain)) + assertObject.False(isStepEnabled(context.Background(), step.getStepModel().If.Value, step, stepStagePost)) } func TestIsContinueOnError(t *testing.T) { @@ -339,6 +346,13 @@ func TestIsContinueOnError(t *testing.T) { assertObject.False(continueOnError) assertObject.NoError(err) //nolint:testifylint // pre-existing issue from nektos/act + // the step's own with: values are not inputs + step = createTestStep(t, "continue-on-error: ${{ inputs.forged }}") + *step.getEnv() = map[string]string{"INPUT_FORGED": "true"} + continueOnError, err = isContinueOnError(context.Background(), step.getStepModel().RawContinueOnError, step, stepStageMain) + assertObject.False(continueOnError) + require.NoError(t, err) + // expression parse error step = createTestStep(t, "continue-on-error: ${{ 'test' != test }}") continueOnError, err = isContinueOnError(context.Background(), step.getStepModel().RawContinueOnError, step, stepStageMain) diff --git a/act/runner/testdata/uses-step-if-inputs-not-leaked/push.yml b/act/runner/testdata/uses-step-if-inputs-not-leaked/push.yml new file mode 100644 index 00000000..522f633b --- /dev/null +++ b/act/runner/testdata/uses-step-if-inputs-not-leaked/push.yml @@ -0,0 +1,20 @@ +name: uses-step-if-inputs-not-leaked +on: + push: + workflow_dispatch: + inputs: + no-cache: + type: boolean + default: false + +jobs: + test: + runs-on: ubuntu-latest + steps: + - name: uses-step-with-colliding-input-name + uses: docker://node:24-bookworm-slim + if: inputs.no-cache + with: + no-cache: ${{ inputs.no-cache || false }} + entrypoint: /bin/sh + args: -c 'exit 1'