From b236cb64f91c7289b6ef7c79eb85db1226a8aa1b Mon Sep 17 00:00:00 2001 From: Earl Warren Date: Sun, 10 Aug 2025 16:24:19 +0000 Subject: [PATCH] fix: composite action input pollution (#818) refuses to use the default for an nodejs input when an composite action has an input with the same name. clean cherry-pick (except for trivial context conflict) of two related pull requests - https://github.com/nektos/act/pull/2348 - https://github.com/nektos/act/pull/2473 - bug fixes - [PR](https://code.forgejo.org/forgejo/runner/pulls/818): fix: composite action input pollution Co-authored-by: ChristopherHX Reviewed-on: https://code.forgejo.org/forgejo/runner/pulls/818 Reviewed-by: Gusted Co-authored-by: Earl Warren Co-committed-by: Earl Warren --- act/runner/action.go | 1 + act/runner/expression.go | 16 +++++-- act/runner/runner_test.go | 3 ++ act/runner/step.go | 12 ++++- .../action-with-pre-and-post/action.yml | 16 +++++++ .../action-with-pre-and-post/main.js | 14 ++++++ .../action-with-pre-and-post/post.js | 14 ++++++ .../action-with-pre-and-post/pre.js | 12 +++++ .../composite_action/action.yml | 16 +++++++ .../push.yml | 10 ++++ .../composite_action/action.yml | 47 +++++++++++++++++++ .../push.yml | 24 ++++++++++ .../action-with-pre-and-post/action.yml | 16 +++++++ .../action-with-pre-and-post/main.js | 14 ++++++ .../action-with-pre-and-post/post.js | 14 ++++++ .../action-with-pre-and-post/pre.js | 12 +++++ .../composite_action/action.yml | 18 +++++++ .../push.yml | 12 +++++ 18 files changed, 267 insertions(+), 4 deletions(-) create mode 100644 act/runner/testdata/uses-composite-check-for-input-collision/action-with-pre-and-post/action.yml create mode 100644 act/runner/testdata/uses-composite-check-for-input-collision/action-with-pre-and-post/main.js create mode 100644 act/runner/testdata/uses-composite-check-for-input-collision/action-with-pre-and-post/post.js create mode 100644 act/runner/testdata/uses-composite-check-for-input-collision/action-with-pre-and-post/pre.js create mode 100644 act/runner/testdata/uses-composite-check-for-input-collision/composite_action/action.yml create mode 100644 act/runner/testdata/uses-composite-check-for-input-collision/push.yml create mode 100644 act/runner/testdata/uses-composite-check-for-input-in-if-uses/composite_action/action.yml create mode 100644 act/runner/testdata/uses-composite-check-for-input-in-if-uses/push.yml create mode 100644 act/runner/testdata/uses-composite-check-for-input-shadowing/action-with-pre-and-post/action.yml create mode 100644 act/runner/testdata/uses-composite-check-for-input-shadowing/action-with-pre-and-post/main.js create mode 100644 act/runner/testdata/uses-composite-check-for-input-shadowing/action-with-pre-and-post/post.js create mode 100644 act/runner/testdata/uses-composite-check-for-input-shadowing/action-with-pre-and-post/pre.js create mode 100644 act/runner/testdata/uses-composite-check-for-input-shadowing/composite_action/action.yml create mode 100644 act/runner/testdata/uses-composite-check-for-input-shadowing/push.yml diff --git a/act/runner/action.go b/act/runner/action.go index c777573d..93016f1d 100644 --- a/act/runner/action.go +++ b/act/runner/action.go @@ -711,6 +711,7 @@ func runPostStep(step actionStep) common.Executor { case model.ActionRunsUsingNode12, model.ActionRunsUsingNode16, model.ActionRunsUsingNode20: populateEnvsFromSavedState(step.getEnv(), step, rc) + populateEnvsFromInput(ctx, step.getEnv(), step.getActionModel(), rc) containerArgs := []string{"node", path.Join(containerActionDir, action.Runs.Post)} logger.Debugf("executing remote job container: %s", containerArgs) diff --git a/act/runner/expression.go b/act/runner/expression.go index a81c7d39..1eade240 100644 --- a/act/runner/expression.go +++ b/act/runner/expression.go @@ -108,6 +108,19 @@ var hashfiles string // NewStepExpressionEvaluator creates a new evaluator func (rc *RunContext) NewStepExpressionEvaluator(ctx context.Context, step step) ExpressionEvaluator { + return rc.NewStepExpressionEvaluatorExt(ctx, step, false) +} + +// NewStepExpressionEvaluatorExt creates a new evaluator +func (rc *RunContext) NewStepExpressionEvaluatorExt(ctx context.Context, step step, rcInputs bool) ExpressionEvaluator { + ghc := rc.getGithubContext(ctx) + if rcInputs { + return rc.newStepExpressionEvaluator(ctx, step, ghc, getEvaluatorInputs(ctx, rc, nil, ghc)) + } + return rc.newStepExpressionEvaluator(ctx, step, ghc, getEvaluatorInputs(ctx, rc, step, ghc)) +} + +func (rc *RunContext) newStepExpressionEvaluator(ctx context.Context, step step, ghc *model.GithubContext, inputs map[string]interface{}) ExpressionEvaluator { // todo: cleanup EvaluationEnvironment creation job := rc.Run.Job() strategy := make(map[string]interface{}) @@ -127,9 +140,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(), diff --git a/act/runner/runner_test.go b/act/runner/runner_test.go index 86f06fcf..24d18546 100644 --- a/act/runner/runner_test.go +++ b/act/runner/runner_test.go @@ -252,7 +252,10 @@ func TestRunner_RunEvent(t *testing.T) { // Uses {workdir, "uses-composite", "push", "", platforms, secrets}, {workdir, "uses-composite-with-error", "push", "Job 'failing-composite-action' failed", platforms, secrets}, + {workdir, "uses-composite-check-for-input-collision", "push", "", platforms, secrets}, + {workdir, "uses-composite-check-for-input-shadowing", "push", "", platforms, secrets}, {workdir, "uses-nested-composite", "push", "", platforms, secrets}, + {workdir, "uses-composite-check-for-input-in-if-uses", "push", "", platforms, secrets}, // {workdir, "remote-action-composite-js-pre-with-defaults", "push", "", platforms, secrets}, {workdir, "remote-action-composite-action-ref", "push", "", platforms, secrets}, // reusable workflow not fully implemented yet diff --git a/act/runner/step.go b/act/runner/step.go index 0d2de3fd..56352baa 100644 --- a/act/runner/step.go +++ b/act/runner/step.go @@ -262,6 +262,16 @@ func mergeEnv(ctx context.Context, step step) { } rc.withGithubEnv(ctx, step.getGithubContext(ctx), *env) + + if step.getStepModel().Uses != "" { + // prevent uses action input pollution of unset parameters, skip this for run steps + // due to design flaw + for key := range *env { + if strings.HasPrefix(key, "INPUT_") { + delete(*env, key) + } + } + } } func isStepEnabled(ctx context.Context, expr string, step step, stage stepStage) (bool, error) { @@ -274,7 +284,7 @@ func isStepEnabled(ctx context.Context, expr string, step step, stage stepStage) defaultStatusCheck = exprparser.DefaultStatusCheckSuccess } - runStep, err := EvalBool(ctx, rc.NewStepExpressionEvaluator(ctx, step), expr, defaultStatusCheck) + runStep, err := EvalBool(ctx, rc.NewStepExpressionEvaluatorExt(ctx, step, stage == stepStageMain), expr, defaultStatusCheck) if err != nil { return false, fmt.Errorf(" \u274C Error in if-expression: \"if: %s\" (%s)", expr, err) } diff --git a/act/runner/testdata/uses-composite-check-for-input-collision/action-with-pre-and-post/action.yml b/act/runner/testdata/uses-composite-check-for-input-collision/action-with-pre-and-post/action.yml new file mode 100644 index 00000000..1e9a8122 --- /dev/null +++ b/act/runner/testdata/uses-composite-check-for-input-collision/action-with-pre-and-post/action.yml @@ -0,0 +1,16 @@ +name: "Action with pre and post" +description: "Action with pre and post" + +inputs: + step: + description: "step" + required: true + cache: + required: false + default: false + +runs: + using: "node16" + pre: pre.js + main: main.js + post: post.js diff --git a/act/runner/testdata/uses-composite-check-for-input-collision/action-with-pre-and-post/main.js b/act/runner/testdata/uses-composite-check-for-input-collision/action-with-pre-and-post/main.js new file mode 100644 index 00000000..5a58515a --- /dev/null +++ b/act/runner/testdata/uses-composite-check-for-input-collision/action-with-pre-and-post/main.js @@ -0,0 +1,14 @@ +const { appendFileSync } = require('fs'); +const step = process.env['INPUT_STEP']; +appendFileSync(process.env['GITHUB_ENV'], `TEST=${step}`, { encoding:'utf-8' }) + +var cache = process.env['INPUT_CACHE'] +try { + var cache = JSON.parse(cache) +} catch { + +} +if(typeof cache !== 'boolean') { + console.log("Input Polluted boolean true/false expected, got " + cache) + process.exit(1); +} \ No newline at end of file diff --git a/act/runner/testdata/uses-composite-check-for-input-collision/action-with-pre-and-post/post.js b/act/runner/testdata/uses-composite-check-for-input-collision/action-with-pre-and-post/post.js new file mode 100644 index 00000000..2f06cfe8 --- /dev/null +++ b/act/runner/testdata/uses-composite-check-for-input-collision/action-with-pre-and-post/post.js @@ -0,0 +1,14 @@ +const { appendFileSync } = require('fs'); +const step = process.env['INPUT_STEP']; +appendFileSync(process.env['GITHUB_ENV'], `TEST=${step}-post`, { encoding:'utf-8' }) + +var cache = process.env['INPUT_CACHE'] +try { + var cache = JSON.parse(cache) +} catch { + +} +if(typeof cache !== 'boolean') { + console.log("Input Polluted boolean true/false expected, got " + cache) + process.exit(1); +} \ No newline at end of file diff --git a/act/runner/testdata/uses-composite-check-for-input-collision/action-with-pre-and-post/pre.js b/act/runner/testdata/uses-composite-check-for-input-collision/action-with-pre-and-post/pre.js new file mode 100644 index 00000000..5d545140 --- /dev/null +++ b/act/runner/testdata/uses-composite-check-for-input-collision/action-with-pre-and-post/pre.js @@ -0,0 +1,12 @@ +console.log('pre'); + +var cache = process.env['INPUT_CACHE'] +try { + var cache = JSON.parse(cache) +} catch { + +} +if(typeof cache !== 'boolean') { + console.log("Input Polluted boolean true/false expected, got " + cache) + process.exit(1); +} \ No newline at end of file diff --git a/act/runner/testdata/uses-composite-check-for-input-collision/composite_action/action.yml b/act/runner/testdata/uses-composite-check-for-input-collision/composite_action/action.yml new file mode 100644 index 00000000..d9683b77 --- /dev/null +++ b/act/runner/testdata/uses-composite-check-for-input-collision/composite_action/action.yml @@ -0,0 +1,16 @@ +name: "Test Composite Action" +description: "Test action uses composite" + +inputs: + cache: + default: none + +runs: + using: "composite" + steps: + - uses: ./uses-composite-check-for-input-collision/action-with-pre-and-post + with: + step: step1 + - uses: ./uses-composite-check-for-input-collision/action-with-pre-and-post + with: + step: step2 diff --git a/act/runner/testdata/uses-composite-check-for-input-collision/push.yml b/act/runner/testdata/uses-composite-check-for-input-collision/push.yml new file mode 100644 index 00000000..059d8215 --- /dev/null +++ b/act/runner/testdata/uses-composite-check-for-input-collision/push.yml @@ -0,0 +1,10 @@ +name: uses-composite-with-pre-and-post-steps +on: push + +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: https://data.forgejo.org/actions/checkout@v4 + - run: echo -n "STEP_OUTPUT_TEST=empty" >> $GITHUB_ENV + - uses: ./uses-composite-check-for-input-collision/composite_action diff --git a/act/runner/testdata/uses-composite-check-for-input-in-if-uses/composite_action/action.yml b/act/runner/testdata/uses-composite-check-for-input-in-if-uses/composite_action/action.yml new file mode 100644 index 00000000..b743bf85 --- /dev/null +++ b/act/runner/testdata/uses-composite-check-for-input-in-if-uses/composite_action/action.yml @@ -0,0 +1,47 @@ +name: "Test Composite Action" +description: "Test action uses composite" + +inputs: + b: + default: true + b2: {} + +runs: + using: "composite" + steps: + - uses: https://github.com/actions/github-script@v7 + if: inputs.b == 'true' + with: + script: | + console.log(${{ tojson(inputs) }}) + if( ${{ tojson(inputs.b) }} != 'true' ) { + process.exit(-1); + } + github-token: noop + - uses: https://github.com/actions/github-script@v7 + if: inputs.b != 'true' + with: + script: | + console.log(${{ tojson(inputs) }}) + if( ${{ tojson(inputs.b) }} == 'true' ) { + process.exit(-1); + } + github-token: noop + - uses: https://github.com/actions/github-script@v7 + if: inputs.b2 == 'false' + with: + script: | + console.log(${{ tojson(inputs) }}) + if( ${{ tojson(inputs.b2) }} != 'false' ) { + process.exit(-1); + } + github-token: noop + - uses: https://github.com/actions/github-script@v7 + if: inputs.b2 != 'false' + with: + script: | + console.log(${{ tojson(inputs) }}) + if( ${{ tojson(inputs.b2) }} == 'false' ) { + process.exit(-1); + } + github-token: noop diff --git a/act/runner/testdata/uses-composite-check-for-input-in-if-uses/push.yml b/act/runner/testdata/uses-composite-check-for-input-in-if-uses/push.yml new file mode 100644 index 00000000..068ba833 --- /dev/null +++ b/act/runner/testdata/uses-composite-check-for-input-in-if-uses/push.yml @@ -0,0 +1,24 @@ +name: uses-composite-check-for-input-in-if-uses +on: push + +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: https://data.forgejo.org/actions/checkout@v4 + - uses: ./uses-composite-check-for-input-in-if-uses/composite_action + with: + b: true + b2: true + - uses: ./uses-composite-check-for-input-in-if-uses/composite_action + with: + b: false + b2: false + - uses: ./uses-composite-check-for-input-in-if-uses/composite_action + with: + b: true + b2: false + - uses: ./uses-composite-check-for-input-in-if-uses/composite_action + with: + b: false + b2: true diff --git a/act/runner/testdata/uses-composite-check-for-input-shadowing/action-with-pre-and-post/action.yml b/act/runner/testdata/uses-composite-check-for-input-shadowing/action-with-pre-and-post/action.yml new file mode 100644 index 00000000..1e9a8122 --- /dev/null +++ b/act/runner/testdata/uses-composite-check-for-input-shadowing/action-with-pre-and-post/action.yml @@ -0,0 +1,16 @@ +name: "Action with pre and post" +description: "Action with pre and post" + +inputs: + step: + description: "step" + required: true + cache: + required: false + default: false + +runs: + using: "node16" + pre: pre.js + main: main.js + post: post.js diff --git a/act/runner/testdata/uses-composite-check-for-input-shadowing/action-with-pre-and-post/main.js b/act/runner/testdata/uses-composite-check-for-input-shadowing/action-with-pre-and-post/main.js new file mode 100644 index 00000000..5a58515a --- /dev/null +++ b/act/runner/testdata/uses-composite-check-for-input-shadowing/action-with-pre-and-post/main.js @@ -0,0 +1,14 @@ +const { appendFileSync } = require('fs'); +const step = process.env['INPUT_STEP']; +appendFileSync(process.env['GITHUB_ENV'], `TEST=${step}`, { encoding:'utf-8' }) + +var cache = process.env['INPUT_CACHE'] +try { + var cache = JSON.parse(cache) +} catch { + +} +if(typeof cache !== 'boolean') { + console.log("Input Polluted boolean true/false expected, got " + cache) + process.exit(1); +} \ No newline at end of file diff --git a/act/runner/testdata/uses-composite-check-for-input-shadowing/action-with-pre-and-post/post.js b/act/runner/testdata/uses-composite-check-for-input-shadowing/action-with-pre-and-post/post.js new file mode 100644 index 00000000..2f06cfe8 --- /dev/null +++ b/act/runner/testdata/uses-composite-check-for-input-shadowing/action-with-pre-and-post/post.js @@ -0,0 +1,14 @@ +const { appendFileSync } = require('fs'); +const step = process.env['INPUT_STEP']; +appendFileSync(process.env['GITHUB_ENV'], `TEST=${step}-post`, { encoding:'utf-8' }) + +var cache = process.env['INPUT_CACHE'] +try { + var cache = JSON.parse(cache) +} catch { + +} +if(typeof cache !== 'boolean') { + console.log("Input Polluted boolean true/false expected, got " + cache) + process.exit(1); +} \ No newline at end of file diff --git a/act/runner/testdata/uses-composite-check-for-input-shadowing/action-with-pre-and-post/pre.js b/act/runner/testdata/uses-composite-check-for-input-shadowing/action-with-pre-and-post/pre.js new file mode 100644 index 00000000..5d545140 --- /dev/null +++ b/act/runner/testdata/uses-composite-check-for-input-shadowing/action-with-pre-and-post/pre.js @@ -0,0 +1,12 @@ +console.log('pre'); + +var cache = process.env['INPUT_CACHE'] +try { + var cache = JSON.parse(cache) +} catch { + +} +if(typeof cache !== 'boolean') { + console.log("Input Polluted boolean true/false expected, got " + cache) + process.exit(1); +} \ No newline at end of file diff --git a/act/runner/testdata/uses-composite-check-for-input-shadowing/composite_action/action.yml b/act/runner/testdata/uses-composite-check-for-input-shadowing/composite_action/action.yml new file mode 100644 index 00000000..2cb5b4e9 --- /dev/null +++ b/act/runner/testdata/uses-composite-check-for-input-shadowing/composite_action/action.yml @@ -0,0 +1,18 @@ +name: "Test Composite Action" +description: "Test action uses composite" + +inputs: + cache: + default: true + +runs: + using: "composite" + steps: + - uses: ./uses-composite-check-for-input-shadowing/action-with-pre-and-post + with: + step: step1 + cache: ${{ inputs.cache || 'none' }} + - uses: ./uses-composite-check-for-input-shadowing/action-with-pre-and-post + with: + step: step2 + cache: ${{ inputs.cache || 'none' }} diff --git a/act/runner/testdata/uses-composite-check-for-input-shadowing/push.yml b/act/runner/testdata/uses-composite-check-for-input-shadowing/push.yml new file mode 100644 index 00000000..0a11a995 --- /dev/null +++ b/act/runner/testdata/uses-composite-check-for-input-shadowing/push.yml @@ -0,0 +1,12 @@ +name: uses-composite-with-pre-and-post-steps +on: push + +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: https://data.forgejo.org/actions/checkout@v4 + - run: echo -n "STEP_OUTPUT_TEST=empty" >> $GITHUB_ENV + - uses: ./uses-composite-check-for-input-shadowing/composite_action + # with: + # cache: other