1
0
Fork 0
mirror of https://code.forgejo.org/forgejo/runner.git synced 2025-08-16 18:01:34 +00:00

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

<!--start release-notes-assistant-->
<!--URL:https://code.forgejo.org/forgejo/runner-->
- bug fixes
  - [PR](https://code.forgejo.org/forgejo/runner/pulls/818): <!--number 818 --><!--line 0 --><!--description Zml4OiBjb21wb3NpdGUgYWN0aW9uIGlucHV0IHBvbGx1dGlvbg==-->fix: composite action input pollution<!--description-->
<!--end release-notes-assistant-->

Co-authored-by: ChristopherHX <christopher.homberger@web.de>
Reviewed-on: https://code.forgejo.org/forgejo/runner/pulls/818
Reviewed-by: Gusted <gusted@noreply.code.forgejo.org>
Co-authored-by: Earl Warren <contact@earl-warren.org>
Co-committed-by: Earl Warren <contact@earl-warren.org>
This commit is contained in:
Earl Warren 2025-08-10 16:24:19 +00:00 committed by earl-warren
parent 931c2c0ac3
commit b236cb64f9
No known key found for this signature in database
GPG key ID: F128CBE6AB3A7201
18 changed files with 267 additions and 4 deletions

View file

@ -711,6 +711,7 @@ func runPostStep(step actionStep) common.Executor {
case model.ActionRunsUsingNode12, model.ActionRunsUsingNode16, model.ActionRunsUsingNode20: case model.ActionRunsUsingNode12, model.ActionRunsUsingNode16, model.ActionRunsUsingNode20:
populateEnvsFromSavedState(step.getEnv(), step, rc) populateEnvsFromSavedState(step.getEnv(), step, rc)
populateEnvsFromInput(ctx, step.getEnv(), step.getActionModel(), rc)
containerArgs := []string{"node", path.Join(containerActionDir, action.Runs.Post)} containerArgs := []string{"node", path.Join(containerActionDir, action.Runs.Post)}
logger.Debugf("executing remote job container: %s", containerArgs) logger.Debugf("executing remote job container: %s", containerArgs)

View file

@ -108,6 +108,19 @@ var hashfiles string
// NewStepExpressionEvaluator creates a new evaluator // NewStepExpressionEvaluator creates a new evaluator
func (rc *RunContext) NewStepExpressionEvaluator(ctx context.Context, step step) ExpressionEvaluator { 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 // todo: cleanup EvaluationEnvironment creation
job := rc.Run.Job() job := rc.Run.Job()
strategy := make(map[string]interface{}) 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{ ee := &exprparser.EvaluationEnvironment{
Github: step.getGithubContext(ctx), Github: step.getGithubContext(ctx),
Env: *step.getEnv(), Env: *step.getEnv(),

View file

@ -252,7 +252,10 @@ func TestRunner_RunEvent(t *testing.T) {
// Uses // Uses
{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-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-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-js-pre-with-defaults", "push", "", platforms, secrets},
{workdir, "remote-action-composite-action-ref", "push", "", platforms, secrets}, {workdir, "remote-action-composite-action-ref", "push", "", platforms, secrets},
// reusable workflow not fully implemented yet // reusable workflow not fully implemented yet

View file

@ -262,6 +262,16 @@ func mergeEnv(ctx context.Context, step step) {
} }
rc.withGithubEnv(ctx, step.getGithubContext(ctx), *env) 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) { 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 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 { if err != nil {
return false, fmt.Errorf(" \u274C Error in if-expression: \"if: %s\" (%s)", expr, err) return false, fmt.Errorf(" \u274C Error in if-expression: \"if: %s\" (%s)", expr, err)
} }

View file

@ -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

View file

@ -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);
}

View file

@ -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);
}

View file

@ -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);
}

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -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);
}

View file

@ -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);
}

View file

@ -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);
}

View file

@ -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' }}

View file

@ -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