fix: composite action input pollution (#2348)

* fix: composite action input pollution

* fix run steps

* fix missing defaults in post after env cleanup

* fix test to make more sense

* Add tests and simplify change

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

pkg/runner/action.go

@@ -625,6 +625,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)

pkg/runner/runner_test.go

@@ -242,6 +242,8 @@ func TestRunEvent(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, "remote-action-composite-js-pre-with-defaults", "push", "", platforms, secrets},
 		{workdir, "remote-action-composite-action-ref", "push", "", platforms, secrets},

pkg/runner/step.go

@@ -239,6 +239,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.Contains(key, "INPUT_") {
+				delete(*env, key)
+			}
+		}
+	}
 }
 
 func isStepEnabled(ctx context.Context, expr string, step step, stage stepStage) (bool, error) {

pkg/runner/step_test.go

@@ -139,6 +139,7 @@ func TestSetupEnv(t *testing.T) {
 		JobContainer: cm,
 	}
 	step := &model.Step{
+		Uses: "./",
 		With: map[string]string{
 			"STEP_WITH": "with-value",
 		},

pkg/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

pkg/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);
+}

pkg/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);
+}

pkg/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);
+}

pkg/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

pkg/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: actions/checkout@v4
+      - run: echo -n "STEP_OUTPUT_TEST=empty" >> $GITHUB_ENV
+      - uses: ./uses-composite-check-for-input-collision/composite_action

pkg/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

pkg/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);
+}

pkg/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);
+}

pkg/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);
+}

pkg/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' }}

pkg/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: actions/checkout@v4
+      - run: echo -n "STEP_OUTPUT_TEST=empty" >> $GITHUB_ENV
+      - uses: ./uses-composite-check-for-input-shadowing/composite_action
+        # with:
+          # cache: other