mirror of
https://gitea.com/gitea/runner.git
synced 2026-08-29 15:27:45 +00:00
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 <me@silverwind.io> Reviewed-on: https://gitea.com/gitea/runner/pulls/1192 Reviewed-by: silverwind <2021+silverwind@noreply.gitea.com> Co-authored-by: ABiscuitttt <773542570@qq.com>
This commit is contained in:
@@ -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)
|
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"]))
|
cmd, err := shellquote.Split(eval.Interpolate(ctx, step.getStepModel().With["args"]))
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return err
|
return err
|
||||||
@@ -410,13 +410,13 @@ func evalDockerArgs(ctx context.Context, step step, action *model.Action, cmd *[
|
|||||||
}
|
}
|
||||||
mergeIntoMap(step, step.getEnv(), inputs)
|
mergeIntoMap(step, step.getEnv(), inputs)
|
||||||
|
|
||||||
stepEE := rc.NewStepExpressionEvaluator(ctx, step)
|
stepEE := rc.NewActionInputsExpressionEvaluator(ctx, step)
|
||||||
for i, v := range *cmd {
|
for i, v := range *cmd {
|
||||||
(*cmd)[i] = stepEE.Interpolate(ctx, v)
|
(*cmd)[i] = stepEE.Interpolate(ctx, v)
|
||||||
}
|
}
|
||||||
mergeIntoMap(step, step.getEnv(), action.Runs.Env)
|
mergeIntoMap(step, step.getEnv(), action.Runs.Env)
|
||||||
|
|
||||||
ee := rc.NewStepExpressionEvaluator(ctx, step)
|
ee := rc.NewActionInputsExpressionEvaluator(ctx, step)
|
||||||
for k, v := range *step.getEnv() {
|
for k, v := range *step.getEnv() {
|
||||||
(*step.getEnv())[k] = ee.Interpolate(ctx, v)
|
(*step.getEnv())[k] = ee.Interpolate(ctx, v)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -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 {
|
for inputID, input := range step.getActionModel().Inputs {
|
||||||
envKey := regexp.MustCompile("[^A-Z0-9-]").ReplaceAllString(strings.ToUpper(inputID), "_")
|
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{},
|
StepResults: map[string]*model.StepResult{},
|
||||||
JobContainer: parent.JobContainer,
|
JobContainer: parent.JobContainer,
|
||||||
ActionPath: actionPath,
|
ActionPath: actionPath,
|
||||||
Env: env,
|
|
||||||
GlobalEnv: parent.GlobalEnv,
|
GlobalEnv: parent.GlobalEnv,
|
||||||
Masks: parent.Masks,
|
Masks: parent.Masks,
|
||||||
ExtraPath: parent.ExtraPath,
|
ExtraPath: parent.ExtraPath,
|
||||||
Parent: parent,
|
Parent: parent,
|
||||||
EventJSON: parent.EventJSON,
|
EventJSON: parent.EventJSON,
|
||||||
}
|
}
|
||||||
|
compositerc.setActionEnv(env)
|
||||||
compositerc.ExprEval = compositerc.NewExpressionEvaluator(ctx)
|
compositerc.ExprEval = compositerc.NewExpressionEvaluator(ctx)
|
||||||
|
|
||||||
return compositerc
|
return compositerc
|
||||||
|
|||||||
+21
-17
@@ -71,7 +71,7 @@ func (rc *RunContext) NewExpressionEvaluatorWithEnv(ctx context.Context, env map
|
|||||||
}
|
}
|
||||||
|
|
||||||
ghc := rc.getGithubContext(ctx)
|
ghc := rc.getGithubContext(ctx)
|
||||||
inputs := getEvaluatorInputs(ctx, rc, nil, ghc)
|
inputs := getEvaluatorInputs(ctx, rc, rc.actionInputs, ghc)
|
||||||
|
|
||||||
ee := &exprparser.EvaluationEnvironment{
|
ee := &exprparser.EvaluationEnvironment{
|
||||||
Github: ghc,
|
Github: ghc,
|
||||||
@@ -102,8 +102,17 @@ func (rc *RunContext) NewExpressionEvaluatorWithEnv(ctx context.Context, env map
|
|||||||
//go:embed hashfiles/index.js
|
//go:embed hashfiles/index.js
|
||||||
var hashfiles string
|
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 {
|
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
|
// todo: cleanup EvaluationEnvironment creation
|
||||||
job := rc.Run.Job()
|
job := rc.Run.Job()
|
||||||
strategy := make(map[string]any)
|
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{
|
ee := &exprparser.EvaluationEnvironment{
|
||||||
Github: step.getGithubContext(ctx),
|
Github: step.getGithubContext(ctx),
|
||||||
Env: *step.getEnv(),
|
Env: *step.getEnv(),
|
||||||
@@ -138,7 +144,7 @@ func (rc *RunContext) NewStepExpressionEvaluator(ctx context.Context, step step)
|
|||||||
Needs: using,
|
Needs: using,
|
||||||
// todo: should be unavailable
|
// todo: should be unavailable
|
||||||
// but required to interpolate/evaluate the inputs in actions/composite
|
// but required to interpolate/evaluate the inputs in actions/composite
|
||||||
Inputs: inputs,
|
Inputs: getEvaluatorInputs(ctx, rc, stepInputs, rc.getGithubContext(ctx)),
|
||||||
HashFiles: getHashFilesFunction(ctx, rc),
|
HashFiles: getHashFilesFunction(ctx, rc),
|
||||||
}
|
}
|
||||||
ee.Runner = rc.getRunnerContext(ctx)
|
ee.Runner = rc.getRunnerContext(ctx)
|
||||||
@@ -261,23 +267,21 @@ func EvalBool(ctx context.Context, evaluator *expressionEvaluator, expr string,
|
|||||||
}).EvalBool(expr, defaultStatusCheck)
|
}).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{}
|
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 {
|
for k, v := range env {
|
||||||
if after, ok := strings.CutPrefix(k, "INPUT_"); ok {
|
if after, ok := strings.CutPrefix(k, "INPUT_"); ok {
|
||||||
inputs[strings.ToLower(after)] = v
|
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" {
|
if ghc.EventName == "workflow_dispatch" {
|
||||||
config := rc.Run.Workflow.WorkflowDispatchConfig()
|
config := rc.Run.Workflow.WorkflowDispatchConfig()
|
||||||
|
|||||||
@@ -156,8 +156,10 @@ func TestEvaluateRunContext(t *testing.T) {
|
|||||||
|
|
||||||
func TestEvaluateStep(t *testing.T) {
|
func TestEvaluateStep(t *testing.T) {
|
||||||
rc := createRunContext(t)
|
rc := createRunContext(t)
|
||||||
|
rc.Env["INPUT_FORGED"] = "leaked"
|
||||||
step := &stepRun{
|
step := &stepRun{
|
||||||
RunContext: rc,
|
RunContext: rc,
|
||||||
|
env: map[string]string{"INPUT_FORGED": "leaked"},
|
||||||
}
|
}
|
||||||
|
|
||||||
ee := rc.NewStepExpressionEvaluator(context.Background(), step)
|
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.conclusion", model.StepStatusSuccess.String(), ""},
|
||||||
{"steps.id_with_underscores.outcome", model.StepStatusFailure.String(), ""},
|
{"steps.id_with_underscores.outcome", model.StepStatusFailure.String(), ""},
|
||||||
{"steps.id_with_underscores.outputs.foo_with_underscores", "bar_with_underscores", ""},
|
{"steps.id_with_underscores.outputs.foo_with_underscores", "bar_with_underscores", ""},
|
||||||
|
{"inputs.forged", nil, ""}, // INPUT_* env is not an input
|
||||||
}
|
}
|
||||||
|
|
||||||
for _, table := range tables {
|
for _, table := range tables {
|
||||||
|
|||||||
@@ -62,6 +62,7 @@ type RunContext struct {
|
|||||||
JobName string
|
JobName string
|
||||||
ActionPath string
|
ActionPath string
|
||||||
Parent *RunContext
|
Parent *RunContext
|
||||||
|
actionInputs map[string]any // inputs of the composite action this runs, nil for a job
|
||||||
Masks []string
|
Masks []string
|
||||||
cleanUpJobContainer common.Executor
|
cleanUpJobContainer common.Executor
|
||||||
caller *caller // job calling this RunContext (reusable workflows)
|
caller *caller // job calling this RunContext (reusable workflows)
|
||||||
@@ -179,6 +180,13 @@ func (rc *RunContext) GetEnv() map[string]string {
|
|||||||
return rc.Env
|
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 {
|
func (rc *RunContext) jobContainerName() string {
|
||||||
// The job id, never evaluated, keeps two jobs apart when masking collapses their names.
|
// 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}
|
nameParts := []string{rc.Config.ContainerNamePrefix, "WORKFLOW-" + rc.Run.Workflow.Name, "JOB-" + rc.Run.JobID, rc.Name}
|
||||||
|
|||||||
@@ -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) {
|
func TestCreateContainerNameBoundedForLongMatrixInput(t *testing.T) {
|
||||||
longMatrixValue := strings.Repeat("os=ubuntu-latest-go=1.24-node=22-", 20)
|
longMatrixValue := strings.Repeat("os=ubuntu-latest-go=1.24-node=22-", 20)
|
||||||
name := createContainerName(
|
name := createContainerName(
|
||||||
|
|||||||
@@ -282,6 +282,7 @@ func TestRunEvent(t *testing.T) {
|
|||||||
{workdir, "uses-composite", "push", "", platforms, secrets},
|
{workdir, "uses-composite", "push", "", platforms, secrets},
|
||||||
{workdir, "uses-composite-with-error", "push", "Job 'failing-composite-action' failed", platforms, secrets},
|
{workdir, "uses-composite-with-error", "push", "Job 'failing-composite-action' failed", platforms, secrets},
|
||||||
{workdir, "uses-docker-url", "push", "", 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},
|
{workdir, "act-composite-env-test", "push", "", platforms, secrets},
|
||||||
|
|
||||||
// Eval
|
// Eval
|
||||||
|
|||||||
@@ -245,7 +245,7 @@ func (sar *stepActionRemote) getCompositeRunContext(ctx context.Context) *RunCon
|
|||||||
// input for this action during the main stage, but the env
|
// input for this action during the main stage, but the env
|
||||||
// was already created during the pre stage)
|
// was already created during the pre stage)
|
||||||
env := evaluateCompositeInputAndEnv(ctx, sar.RunContext, sar)
|
env := evaluateCompositeInputAndEnv(ctx, sar.RunContext, sar)
|
||||||
sar.compositeRunContext.Env = env
|
sar.compositeRunContext.setActionEnv(env)
|
||||||
sar.compositeRunContext.ExtraPath = sar.RunContext.ExtraPath
|
sar.compositeRunContext.ExtraPath = sar.RunContext.ExtraPath
|
||||||
}
|
}
|
||||||
return sar.compositeRunContext
|
return sar.compositeRunContext
|
||||||
|
|||||||
@@ -279,6 +279,13 @@ func TestIsStepEnabled(t *testing.T) {
|
|||||||
Conclusion: model.StepStatusFailure,
|
Conclusion: model.StepStatusFailure,
|
||||||
}
|
}
|
||||||
assertObject.True(isStepEnabled(context.Background(), step.getStepModel().If.Value, step, stepStageMain))
|
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) {
|
func TestIsContinueOnError(t *testing.T) {
|
||||||
@@ -339,6 +346,13 @@ func TestIsContinueOnError(t *testing.T) {
|
|||||||
assertObject.False(continueOnError)
|
assertObject.False(continueOnError)
|
||||||
assertObject.NoError(err) //nolint:testifylint // pre-existing issue from nektos/act
|
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
|
// expression parse error
|
||||||
step = createTestStep(t, "continue-on-error: ${{ 'test' != test }}")
|
step = createTestStep(t, "continue-on-error: ${{ 'test' != test }}")
|
||||||
continueOnError, err = isContinueOnError(context.Background(), step.getStepModel().RawContinueOnError, step, stepStageMain)
|
continueOnError, err = isContinueOnError(context.Background(), step.getStepModel().RawContinueOnError, step, stepStageMain)
|
||||||
|
|||||||
@@ -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'
|
||||||
Reference in New Issue
Block a user