fix: GITHUB_ENV / PATH handling

nektos · Dec 12, 2022 · 65b339a · 65b339a
1 parent d281230
commit 65b339a
Show file tree

Hide file tree

Showing 12 changed files with 113 additions and 57 deletions.
diff --git a/pkg/runner/action_composite.go b/pkg/runner/action_composite.go
@@ -99,6 +99,7 @@ func execAsComposite(step actionStep) common.Executor {
 
 		rc.Masks = append(rc.Masks, compositeRC.Masks...)
 		rc.ExtraPath = compositeRC.ExtraPath
+		rc.Env = compositeRC.Env
 
 		return err
 	}

diff --git a/pkg/runner/run_context.go b/pkg/runner/run_context.go
@@ -15,6 +15,7 @@ import (
 	"regexp"
 	"runtime"
 	"strings"
+	"time"
 
 	"github.com/mitchellh/go-homedir"
 	"github.com/opencontainers/selinux/go-selinux"
@@ -266,8 +267,6 @@ func (rc *RunContext) startJobContainer() common.Executor {
 			rc.stopJobContainer(),
 			rc.JobContainer.Create(rc.Config.ContainerCapAdd, rc.Config.ContainerCapDrop),
 			rc.JobContainer.Start(false),
-			rc.JobContainer.UpdateFromImageEnv(&rc.Env),
-			rc.JobContainer.UpdateFromEnv("/etc/environment", &rc.Env),
 			rc.JobContainer.Copy(rc.JobContainer.GetActPath()+"/", &container.FileEntry{
 				Name: "workflow/event.json",
 				Mode: 0644,
@@ -291,7 +290,19 @@ func (rc *RunContext) ApplyExtraPath(env *map[string]string) {
 	if rc.ExtraPath != nil && len(rc.ExtraPath) > 0 {
 		path := rc.JobContainer.GetPathVariableName()
 		if (*env)[path] == "" {
-			(*env)[path] = rc.JobContainer.DefaultPathVariable()
+			cenv := map[string]string{}
+			var cpath string
+			ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
+			defer cancel()
+			if err := rc.JobContainer.UpdateFromImageEnv(&cenv)(ctx); err == nil {
+				if p, ok := cenv[path]; ok {
+					cpath = p
+				}
+			}
+			if len(cpath) == 0 {
+				cpath = rc.JobContainer.DefaultPathVariable()
+			}
+			(*env)[path] = cpath
 		}
 		(*env)[path] = rc.JobContainer.JoinPathVariable(append(rc.ExtraPath, (*env)[path])...)
 	}
@@ -668,7 +679,6 @@ func nestedMapLookup(m map[string]interface{}, ks ...string) (rval interface{})
 
 func (rc *RunContext) withGithubEnv(ctx context.Context, github *model.GithubContext, env map[string]string) map[string]string {
 	env["CI"] = "true"
-	env["GITHUB_ENV"] = rc.JobContainer.GetActPath() + "/workflow/envs.txt"
 	env["GITHUB_WORKFLOW"] = github.Workflow
 	env["GITHUB_RUN_ID"] = github.RunID
 	env["GITHUB_RUN_NUMBER"] = github.RunNumber

diff --git a/pkg/runner/runner_test.go b/pkg/runner/runner_test.go
@@ -162,7 +162,7 @@ func TestRunEvent(t *testing.T) {
 		{workdir, "container-hostname", "push", "", platforms},
 		{workdir, "remote-action-docker", "push", "", platforms},
 		{workdir, "remote-action-js", "push", "", platforms},
-		{workdir, "remote-action-js", "push", "", map[string]string{"ubuntu-latest": "catthehacker/ubuntu:runner-latest"}}, // Test if this works with non root container
+		{workdir, "remote-action-js-node-user", "push", "", platforms}, // Test if this works with non root container
 		{workdir, "matrix", "push", "", platforms},
 		{workdir, "matrix-include-exclude", "push", "", platforms},
 		{workdir, "commands", "push", "", platforms},
@@ -186,6 +186,8 @@ func TestRunEvent(t *testing.T) {
 		{workdir, "actions-environment-and-context-tests", "push", "", platforms},
 		{workdir, "uses-action-with-pre-and-post-step", "push", "", platforms},
 		{workdir, "evalenv", "push", "", platforms},
+		{workdir, "docker-action-custom-path", "push", "", platforms},
+		{workdir, "GITHUB_ENV-use-in-env-ctx", "push", "", platforms},
 		{workdir, "ensure-post-steps", "push", "Job 'second-post-step-should-fail' failed", platforms},
 		{workdir, "workflow_dispatch", "workflow_dispatch", "", platforms},
 		{workdir, "workflow_dispatch_no_inputs_mapping", "workflow_dispatch", "", platforms},

diff --git a/pkg/runner/step.go b/pkg/runner/step.go
@@ -56,6 +56,18 @@ func (s stepStage) getStepName(stepModel *model.Step) string {
 	return "unknown"
 }
 
+func processRunnerEnvFileCommand(ctx context.Context, fileName string, rc *RunContext, setter func(context.Context, map[string]string, string)) error {
+	env := map[string]string{}
+	err := rc.JobContainer.UpdateFromEnv(path.Join(rc.JobContainer.GetActPath(), fileName), &env)(ctx)
+	if err != nil {
+		return err
+	}
+	for k, v := range env {
+		setter(ctx, map[string]string{"name": k}, v)
+	}
+	return nil
+}
+
 func runStepExecutor(step step, stage stepStage, executor common.Executor) common.Executor {
 	return func(ctx context.Context) error {
 		logger := common.Logger(ctx)
@@ -101,9 +113,11 @@ func runStepExecutor(step step, stage stepStage, executor common.Executor) commo
 		outputFileCommand := path.Join("workflow", "outputcmd.txt")
 		stateFileCommand := path.Join("workflow", "statecmd.txt")
 		pathFileCommand := path.Join("workflow", "pathcmd.txt")
+		envFileCommand := path.Join("workflow", "envs.txt")
 		(*step.getEnv())["GITHUB_OUTPUT"] = path.Join(actPath, outputFileCommand)
 		(*step.getEnv())["GITHUB_STATE"] = path.Join(actPath, stateFileCommand)
 		(*step.getEnv())["GITHUB_PATH"] = path.Join(actPath, pathFileCommand)
+		(*step.getEnv())["GITHUB_ENV"] = path.Join(actPath, envFileCommand)
 		_ = rc.JobContainer.Copy(actPath, &container.FileEntry{
 			Name: outputFileCommand,
 			Mode: 0666,
@@ -113,6 +127,9 @@ func runStepExecutor(step step, stage stepStage, executor common.Executor) commo
 		}, &container.FileEntry{
 			Name: pathFileCommand,
 			Mode: 0666,
+		}, &container.FileEntry{
+			Name: envFileCommand,
+			Mode: 0666,
 		})(ctx)
 
 		err = executor(ctx)
@@ -140,21 +157,17 @@ func runStepExecutor(step step, stage stepStage, executor common.Executor) commo
 		}
 		// Process Runner File Commands
 		orgerr := err
-		state := map[string]string{}
-		err = rc.JobContainer.UpdateFromEnv(path.Join(actPath, stateFileCommand), &state)(ctx)
+		err = processRunnerEnvFileCommand(ctx, envFileCommand, rc, rc.setEnv)
 		if err != nil {
 			return err
 		}
-		for k, v := range state {
-			rc.saveState(ctx, map[string]string{"name": k}, v)
-		}
-		output := map[string]string{}
-		err = rc.JobContainer.UpdateFromEnv(path.Join(actPath, outputFileCommand), &output)(ctx)
+		err = processRunnerEnvFileCommand(ctx, stateFileCommand, rc, rc.saveState)
 		if err != nil {
 			return err
 		}
-		for k, v := range output {
-			rc.setOutput(ctx, map[string]string{"name": k}, v)
+		err = processRunnerEnvFileCommand(ctx, outputFileCommand, rc, rc.setOutput)
+		if err != nil {
+			return err
 		}
 		err = rc.UpdateExtraPath(ctx, path.Join(actPath, pathFileCommand))
 		if err != nil {
@@ -171,14 +184,6 @@ func setupEnv(ctx context.Context, step step) error {
 	rc := step.getRunContext()
 
 	mergeEnv(ctx, step)
-	err := rc.JobContainer.UpdateFromImageEnv(step.getEnv())(ctx)
-	if err != nil {
-		return err
-	}
-	err = rc.JobContainer.UpdateFromEnv((*step.getEnv())["GITHUB_ENV"], step.getEnv())(ctx)
-	if err != nil {
-		return err
-	}
 	// merge step env last, since it should not be overwritten
 	mergeIntoMap(step.getEnv(), step.getStepModel().GetEnv())
 

diff --git a/pkg/runner/step_action_local_test.go b/pkg/runner/step_action_local_test.go
@@ -69,18 +69,14 @@ func TestStepActionLocalTest(t *testing.T) {
 	salm.On("readAction", sal.Step, filepath.Clean("/tmp/path/to/action"), "", mock.Anything, mock.Anything).
 		Return(&model.Action{}, nil)
 
-	cm.On("UpdateFromImageEnv", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error {
+	cm.On("Copy", "/var/run/act", mock.AnythingOfType("[]*container.FileEntry")).Return(func(ctx context.Context) error {
 		return nil
 	})
 
 	cm.On("UpdateFromEnv", "/var/run/act/workflow/envs.txt", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error {
 		return nil
 	})
 
-	cm.On("Copy", "/var/run/act", mock.AnythingOfType("[]*container.FileEntry")).Return(func(ctx context.Context) error {
-		return nil
-	})
-
 	cm.On("UpdateFromEnv", "/var/run/act/workflow/statecmd.txt", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error {
 		return nil
 	})
@@ -213,7 +209,7 @@ func TestStepActionLocalPost(t *testing.T) {
 				env  bool
 				exec bool
 			}{
-				env:  true,
+				env:  false,
 				exec: false,
 			},
 		},
@@ -277,10 +273,6 @@ func TestStepActionLocalPost(t *testing.T) {
 			}
 			sal.RunContext.ExprEval = sal.RunContext.NewExpressionEvaluator(ctx)
 
-			if tt.mocks.env {
-				cm.On("UpdateFromImageEnv", &sal.env).Return(func(ctx context.Context) error { return nil })
-				cm.On("UpdateFromEnv", "/var/run/act/workflow/envs.txt", &sal.env).Return(func(ctx context.Context) error { return nil })
-			}
 			if tt.mocks.exec {
 				suffixMatcher := func(suffix string) interface{} {
 					return mock.MatchedBy(func(array []string) bool {
@@ -293,6 +285,10 @@ func TestStepActionLocalPost(t *testing.T) {
 					return nil
 				})
 
+				cm.On("UpdateFromEnv", "/var/run/act/workflow/envs.txt", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error {
+					return nil
+				})
+
 				cm.On("UpdateFromEnv", "/var/run/act/workflow/statecmd.txt", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error {
 					return nil
 				})

diff --git a/pkg/runner/step_action_remote_test.go b/pkg/runner/step_action_remote_test.go
@@ -165,10 +165,6 @@ func TestStepActionRemote(t *testing.T) {
 				})
 			}
 
-			if tt.mocks.env {
-				cm.On("UpdateFromImageEnv", &sar.env).Return(func(ctx context.Context) error { return nil })
-				cm.On("UpdateFromEnv", "/var/run/act/workflow/envs.txt", &sar.env).Return(func(ctx context.Context) error { return nil })
-			}
 			if tt.mocks.read {
 				sarm.On("readAction", sar.Step, suffixMatcher("act/remote-action@v1"), "", mock.Anything, mock.Anything).Return(&model.Action{}, nil)
 			}
@@ -179,6 +175,10 @@ func TestStepActionRemote(t *testing.T) {
 					return nil
 				})
 
+				cm.On("UpdateFromEnv", "/var/run/act/workflow/envs.txt", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error {
+					return nil
+				})
+
 				cm.On("UpdateFromEnv", "/var/run/act/workflow/statecmd.txt", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error {
 					return nil
 				})
@@ -592,17 +592,17 @@ func TestStepActionRemotePost(t *testing.T) {
 			}
 			sar.RunContext.ExprEval = sar.RunContext.NewExpressionEvaluator(ctx)
 
-			if tt.mocks.env {
-				cm.On("UpdateFromImageEnv", &sar.env).Return(func(ctx context.Context) error { return nil })
-				cm.On("UpdateFromEnv", "/var/run/act/workflow/envs.txt", &sar.env).Return(func(ctx context.Context) error { return nil })
-			}
 			if tt.mocks.exec {
 				cm.On("Exec", []string{"node", "/var/run/act/actions/remote-action@v1/post.js"}, sar.env, "", "").Return(func(ctx context.Context) error { return tt.err })
 
 				cm.On("Copy", "/var/run/act", mock.AnythingOfType("[]*container.FileEntry")).Return(func(ctx context.Context) error {
 					return nil
 				})
 
+				cm.On("UpdateFromEnv", "/var/run/act/workflow/envs.txt", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error {
+					return nil
+				})
+
 				cm.On("UpdateFromEnv", "/var/run/act/workflow/statecmd.txt", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error {
 					return nil
 				})

diff --git a/pkg/runner/step_docker_test.go b/pkg/runner/step_docker_test.go
@@ -57,14 +57,6 @@ func TestStepDockerMain(t *testing.T) {
 	}
 	sd.RunContext.ExprEval = sd.RunContext.NewExpressionEvaluator(ctx)
 
-	cm.On("UpdateFromImageEnv", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error {
-		return nil
-	})
-
-	cm.On("UpdateFromEnv", "/var/run/act/workflow/envs.txt", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error {
-		return nil
-	})
-
 	cm.On("Pull", false).Return(func(ctx context.Context) error {
 		return nil
 	})
@@ -89,6 +81,10 @@ func TestStepDockerMain(t *testing.T) {
 		return nil
 	})
 
+	cm.On("UpdateFromEnv", "/var/run/act/workflow/envs.txt", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error {
+		return nil
+	})
+
 	cm.On("UpdateFromEnv", "/var/run/act/workflow/statecmd.txt", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error {
 		return nil
 	})

diff --git a/pkg/runner/step_run_test.go b/pkg/runner/step_run_test.go
@@ -55,18 +55,14 @@ func TestStepRun(t *testing.T) {
 		return nil
 	})
 
-	cm.On("UpdateFromImageEnv", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error {
+	cm.On("Copy", "/var/run/act", mock.AnythingOfType("[]*container.FileEntry")).Return(func(ctx context.Context) error {
 		return nil
 	})
 
 	cm.On("UpdateFromEnv", "/var/run/act/workflow/envs.txt", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error {
 		return nil
 	})
 
-	cm.On("Copy", "/var/run/act", mock.AnythingOfType("[]*container.FileEntry")).Return(func(ctx context.Context) error {
-		return nil
-	})
-
 	cm.On("UpdateFromEnv", "/var/run/act/workflow/statecmd.txt", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error {
 		return nil
 	})

diff --git a/pkg/runner/step_test.go b/pkg/runner/step_test.go
@@ -148,9 +148,6 @@ func TestSetupEnv(t *testing.T) {
 	sm.On("getStepModel").Return(step)
 	sm.On("getEnv").Return(&env)
 
-	cm.On("UpdateFromImageEnv", &env).Return(func(ctx context.Context) error { return nil })
-	cm.On("UpdateFromEnv", "/var/run/act/workflow/envs.txt", &env).Return(func(ctx context.Context) error { return nil })
-
 	err := setupEnv(context.Background(), sm)
 	assert.Nil(t, err)
 
@@ -174,7 +171,6 @@ func TestSetupEnv(t *testing.T) {
 		"GITHUB_ACTION_REPOSITORY": "",
 		"GITHUB_API_URL":           "https:///api/v3",
 		"GITHUB_BASE_REF":          "",
-		"GITHUB_ENV":               "/var/run/act/workflow/envs.txt",
 		"GITHUB_EVENT_NAME":        "",
 		"GITHUB_EVENT_PATH":        "/var/run/act/workflow/event.json",
 		"GITHUB_GRAPHQL_URL":       "https:///api/graphql",

diff --git a/pkg/runner/testdata/GITHUB_ENV-use-in-env-ctx/push.yml b/pkg/runner/testdata/GITHUB_ENV-use-in-env-ctx/push.yml
@@ -0,0 +1,27 @@
+on: push
+jobs:
+  _:
+    runs-on: ubuntu-latest
+    env:
+      MYGLOBALENV3: myglobalval3
+    steps:
+    - run: |
+        echo MYGLOBALENV1=myglobalval1 > $GITHUB_ENV
+        echo "::set-env name=MYGLOBALENV2::myglobalval2"
+    - uses: nektos/act-test-actions/script@main
+      with:
+        main: |
+          env
+          [[ "$MYGLOBALENV1" = "${{ env.MYGLOBALENV1 }}" ]]
+          [[ "$MYGLOBALENV1" = "${{ env.MYGLOBALENV1ALIAS }}" ]]
+          [[ "$MYGLOBALENV1" = "$MYGLOBALENV1ALIAS" ]]
+          [[ "$MYGLOBALENV2" = "${{ env.MYGLOBALENV2 }}" ]]
+          [[ "$MYGLOBALENV2" = "${{ env.MYGLOBALENV2ALIAS }}" ]]
+          [[ "$MYGLOBALENV2" = "$MYGLOBALENV2ALIAS" ]]
+          [[ "$MYGLOBALENV3" = "${{ env.MYGLOBALENV3 }}" ]]
+          [[ "$MYGLOBALENV3" = "${{ env.MYGLOBALENV3ALIAS }}" ]]
+          [[ "$MYGLOBALENV3" = "$MYGLOBALENV3ALIAS" ]]
+      env:
+        MYGLOBALENV1ALIAS: ${{ env.MYGLOBALENV1 }}
+        MYGLOBALENV2ALIAS: ${{ env.MYGLOBALENV2 }}
+        MYGLOBALENV3ALIAS: ${{ env.MYGLOBALENV3 }}
diff --git a/pkg/runner/testdata/docker-action-custom-path/push.yml b/pkg/runner/testdata/docker-action-custom-path/push.yml
@@ -0,0 +1,12 @@
+on: push
+jobs:
+  _:
+    runs-on: ubuntu-latest
+    steps:
+    - run: |
+        FROM ubuntu:latest
+        ENV PATH="/opt/texlive/texdir/bin/x86_64-linuxmusl:${PATH}"
+        ENV ORG_PATH="${PATH}"
+        ENTRYPOINT [ "bash", "-c", "echo \"PATH=$PATH\" && echo \"ORG_PATH=$ORG_PATH\" && [[ \"$PATH\" = \"$ORG_PATH\" ]]" ]
+      shell: mv {0} Dockerfile
+    - uses: ./
diff --git a/pkg/runner/testdata/remote-action-js-node-user/push.yml b/pkg/runner/testdata/remote-action-js-node-user/push.yml
@@ -0,0 +1,15 @@
+name: remote-action-js
+on: push
+
+jobs:
+  test:
+    runs-on: ubuntu-latest
+    container:
+      image: node:16-buster-slim
+      options: --user node
+    steps:
+    - uses: actions/hello-world-javascript-action@v1
+      with:
+        who-to-greet: 'Mona the Octocat'
+
+    - uses: cloudposse/actions/github/[email protected]