From 65b339a86258934728e55978f98ca63922598bd3 Mon Sep 17 00:00:00 2001 From: ChristopherHX Date: Sun, 11 Dec 2022 21:59:30 +0100 Subject: [PATCH 1/5] fix: GITHUB_ENV / PATH handling --- pkg/runner/action_composite.go | 1 + pkg/runner/run_context.go | 18 +++++++-- pkg/runner/runner_test.go | 4 +- pkg/runner/step.go | 39 +++++++++++-------- pkg/runner/step_action_local_test.go | 16 +++----- pkg/runner/step_action_remote_test.go | 16 ++++---- pkg/runner/step_docker_test.go | 12 ++---- pkg/runner/step_run_test.go | 6 +-- pkg/runner/step_test.go | 4 -- .../GITHUB_ENV-use-in-env-ctx/push.yml | 27 +++++++++++++ .../docker-action-custom-path/push.yml | 12 ++++++ .../remote-action-js-node-user/push.yml | 15 +++++++ 12 files changed, 113 insertions(+), 57 deletions(-) create mode 100644 pkg/runner/testdata/GITHUB_ENV-use-in-env-ctx/push.yml create mode 100644 pkg/runner/testdata/docker-action-custom-path/push.yml create mode 100644 pkg/runner/testdata/remote-action-js-node-user/push.yml diff --git a/pkg/runner/action_composite.go b/pkg/runner/action_composite.go index c1e94fcd2cd..348568c641e 100644 --- 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 index c653cc7e89a..91acb22e216 100644 --- 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 index faa13ced30a..991507c5f19 100644 --- 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 index f8a192fdc30..9f40240a018 100644 --- 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 index 9ece06a4172..ecac7818bf5 100644 --- a/pkg/runner/step_action_local_test.go +++ b/pkg/runner/step_action_local_test.go @@ -69,7 +69,7 @@ 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 }) @@ -77,10 +77,6 @@ func TestStepActionLocalTest(t *testing.T) { 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 index 829e3864485..a8d29fc3720 100644 --- 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,10 +592,6 @@ 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 }) @@ -603,6 +599,10 @@ func TestStepActionRemotePost(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_docker_test.go b/pkg/runner/step_docker_test.go index c26ffd685a9..3d90ac34f9e 100644 --- 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 index 324ed5fcc8f..4ca2eb97456 100644 --- a/pkg/runner/step_run_test.go +++ b/pkg/runner/step_run_test.go @@ -55,7 +55,7 @@ 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 }) @@ -63,10 +63,6 @@ func TestStepRun(t *testing.T) { 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 index 8930cca3e7f..86e5acc4a26 100644 --- 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 new file mode 100644 index 00000000000..c7b75a02003 --- /dev/null +++ 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 }} \ No newline at end of file diff --git a/pkg/runner/testdata/docker-action-custom-path/push.yml b/pkg/runner/testdata/docker-action-custom-path/push.yml new file mode 100644 index 00000000000..37bbf417344 --- /dev/null +++ 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: ./ \ No newline at end of file diff --git a/pkg/runner/testdata/remote-action-js-node-user/push.yml b/pkg/runner/testdata/remote-action-js-node-user/push.yml new file mode 100644 index 00000000000..cede7b0a204 --- /dev/null +++ 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/slash-command-dispatch@0.14.0 From 9fe22f73eb15c3a33f9712867dffd6118b8a01c8 Mon Sep 17 00:00:00 2001 From: ChristopherHX Date: Mon, 12 Dec 2022 20:14:21 +0000 Subject: [PATCH 2/5] apply workaround --- pkg/runner/action_composite.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/pkg/runner/action_composite.go b/pkg/runner/action_composite.go index 348568c641e..4f57021681a 100644 --- a/pkg/runner/action_composite.go +++ b/pkg/runner/action_composite.go @@ -99,7 +99,12 @@ func execAsComposite(step actionStep) common.Executor { rc.Masks = append(rc.Masks, compositeRC.Masks...) rc.ExtraPath = compositeRC.ExtraPath - rc.Env = compositeRC.Env + // Drop INPUT_... envs, otherwise could just assign rc.Env = compositeRC.Env + for k, v := range compositeRC.Env { + if !strings.HasPrefix(k, "INPUT_") { + rc.Env[k] = v + } + } return err } From f69646740fb33770d0c5ca6b51299ad4aa8b3bd8 Mon Sep 17 00:00:00 2001 From: ChristopherHX Date: Tue, 13 Dec 2022 19:17:38 +0000 Subject: [PATCH 3/5] add ctx to ApplyExtraPath --- pkg/runner/action.go | 6 +++--- pkg/runner/run_context.go | 5 +---- pkg/runner/step_run.go | 2 +- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/pkg/runner/action.go b/pkg/runner/action.go index ec7ac7cd68d..c729d4201e2 100644 --- a/pkg/runner/action.go +++ b/pkg/runner/action.go @@ -154,7 +154,7 @@ func runActionImpl(step actionStep, actionDir string, remoteAction *remoteAction containerArgs := []string{"node", path.Join(containerActionDir, action.Runs.Main)} logger.Debugf("executing remote job container: %s", containerArgs) - rc.ApplyExtraPath(step.getEnv()) + rc.ApplyExtraPath(ctx, step.getEnv()) return rc.execJobContainer(containerArgs, *step.getEnv(), "", "")(ctx) case model.ActionRunsUsingDocker: @@ -488,7 +488,7 @@ func runPreStep(step actionStep) common.Executor { containerArgs := []string{"node", path.Join(containerActionDir, action.Runs.Pre)} logger.Debugf("executing remote job container: %s", containerArgs) - rc.ApplyExtraPath(step.getEnv()) + rc.ApplyExtraPath(ctx, step.getEnv()) return rc.execJobContainer(containerArgs, *step.getEnv(), "", "")(ctx) @@ -577,7 +577,7 @@ func runPostStep(step actionStep) common.Executor { containerArgs := []string{"node", path.Join(containerActionDir, action.Runs.Post)} logger.Debugf("executing remote job container: %s", containerArgs) - rc.ApplyExtraPath(step.getEnv()) + rc.ApplyExtraPath(ctx, step.getEnv()) return rc.execJobContainer(containerArgs, *step.getEnv(), "", "")(ctx) diff --git a/pkg/runner/run_context.go b/pkg/runner/run_context.go index 91acb22e216..f3f5715ef7a 100644 --- a/pkg/runner/run_context.go +++ b/pkg/runner/run_context.go @@ -15,7 +15,6 @@ import ( "regexp" "runtime" "strings" - "time" "github.com/mitchellh/go-homedir" "github.com/opencontainers/selinux/go-selinux" @@ -286,14 +285,12 @@ func (rc *RunContext) execJobContainer(cmd []string, env map[string]string, user } } -func (rc *RunContext) ApplyExtraPath(env *map[string]string) { +func (rc *RunContext) ApplyExtraPath(ctx context.Context, env *map[string]string) { if rc.ExtraPath != nil && len(rc.ExtraPath) > 0 { path := rc.JobContainer.GetPathVariableName() if (*env)[path] == "" { 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 diff --git a/pkg/runner/step_run.go b/pkg/runner/step_run.go index 17db77af6ec..f833fc1a928 100644 --- a/pkg/runner/step_run.go +++ b/pkg/runner/step_run.go @@ -30,7 +30,7 @@ func (sr *stepRun) main() common.Executor { return runStepExecutor(sr, stepStageMain, common.NewPipelineExecutor( sr.setupShellCommandExecutor(), func(ctx context.Context) error { - sr.getRunContext().ApplyExtraPath(&sr.env) + sr.getRunContext().ApplyExtraPath(ctx, &sr.env) return sr.getRunContext().JobContainer.Exec(sr.cmd, sr.env, "", sr.Step.WorkingDirectory)(ctx) }, )) From 7df11256c4a0d0cc05989b727f8f516d6b1c8bf9 Mon Sep 17 00:00:00 2001 From: ChristopherHX Date: Thu, 26 Jan 2023 23:04:47 +0000 Subject: [PATCH 4/5] fix: Do not leak step env in composite See https://github.com/nektos/act/pull/1585 for a test --- pkg/runner/action_composite.go | 11 +++++++---- pkg/runner/command.go | 10 ++++++++-- pkg/runner/run_context.go | 1 + 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/pkg/runner/action_composite.go b/pkg/runner/action_composite.go index 4f57021681a..0b6fbe55272 100644 --- a/pkg/runner/action_composite.go +++ b/pkg/runner/action_composite.go @@ -66,6 +66,7 @@ func newCompositeRunContext(ctx context.Context, parent *RunContext, step action JobContainer: parent.JobContainer, ActionPath: actionPath, Env: env, + GlobalEnv: parent.GlobalEnv, Masks: parent.Masks, ExtraPath: parent.ExtraPath, Parent: parent, @@ -99,11 +100,13 @@ func execAsComposite(step actionStep) common.Executor { rc.Masks = append(rc.Masks, compositeRC.Masks...) rc.ExtraPath = compositeRC.ExtraPath - // Drop INPUT_... envs, otherwise could just assign rc.Env = compositeRC.Env - for k, v := range compositeRC.Env { - if !strings.HasPrefix(k, "INPUT_") { - rc.Env[k] = v + // compositeRC.Env is dirty, contains INPUT_ and merged step env, only rely on compositeRC.GlobalEnv + for k, v := range compositeRC.GlobalEnv { + rc.Env[k] = v + if rc.GlobalEnv == nil { + rc.GlobalEnv = map[string]string{} } + rc.GlobalEnv[k] = v } return err diff --git a/pkg/runner/command.go b/pkg/runner/command.go index 53e167cd921..f14eb7aad5c 100644 --- a/pkg/runner/command.go +++ b/pkg/runner/command.go @@ -82,11 +82,17 @@ func (rc *RunContext) commandHandler(ctx context.Context) common.LineHandler { } func (rc *RunContext) setEnv(ctx context.Context, kvPairs map[string]string, arg string) { - common.Logger(ctx).Infof(" \U00002699 ::set-env:: %s=%s", kvPairs["name"], arg) + name := kvPairs["name"] + common.Logger(ctx).Infof(" \U00002699 ::set-env:: %s=%s", name, arg) if rc.Env == nil { rc.Env = make(map[string]string) } - rc.Env[kvPairs["name"]] = arg + rc.Env[name] = arg + // for composite action GITHUB_ENV and set-env passing + if rc.GlobalEnv == nil { + rc.GlobalEnv = map[string]string{} + } + rc.GlobalEnv[name] = arg } func (rc *RunContext) setOutput(ctx context.Context, kvPairs map[string]string, arg string) { logger := common.Logger(ctx) diff --git a/pkg/runner/run_context.go b/pkg/runner/run_context.go index 9113c7fc11b..d9321ff83c0 100644 --- a/pkg/runner/run_context.go +++ b/pkg/runner/run_context.go @@ -35,6 +35,7 @@ type RunContext struct { Run *model.Run EventJSON string Env map[string]string + GlobalEnv map[string]string // to pass env changes of GITHUB_ENV and set-env correctly, due to dirty Env field ExtraPath []string CurrentStep string StepResults map[string]*model.StepResult From b44d6e3a705fdc1e46ef516b5b309fcb79604697 Mon Sep 17 00:00:00 2001 From: ChristopherHX Date: Tue, 31 Jan 2023 17:17:22 +0100 Subject: [PATCH 5/5] add more tests --- pkg/runner/runner_test.go | 4 ++++ .../push.yml | 1 + .../set-env-new-env-file-per-step/push.yml | 15 ++++++++++++ .../set-env-step-env-override/push.yml | 24 +++++++++++++++++++ 4 files changed, 44 insertions(+) create mode 100644 pkg/runner/testdata/set-env-new-env-file-per-step/push.yml create mode 100644 pkg/runner/testdata/set-env-step-env-override/push.yml diff --git a/pkg/runner/runner_test.go b/pkg/runner/runner_test.go index 5af9686a7b3..0a83537ebc0 100644 --- a/pkg/runner/runner_test.go +++ b/pkg/runner/runner_test.go @@ -206,6 +206,8 @@ func TestRunEvent(t *testing.T) { {"../model/testdata", "container-volumes", "push", "", platforms, secrets}, {workdir, "path-handling", "push", "", platforms, secrets}, {workdir, "do-not-leak-step-env-in-composite", "push", "", platforms, secrets}, + {workdir, "set-env-step-env-override", "push", "", platforms, secrets}, + {workdir, "set-env-new-env-file-per-step", "push", "", platforms, secrets}, } for _, table := range tables { @@ -306,6 +308,8 @@ func TestRunEventHostEnvironment(t *testing.T) { {workdir, "nix-prepend-path", "push", "", platforms, secrets}, {workdir, "inputs-via-env-context", "push", "", platforms, secrets}, {workdir, "do-not-leak-step-env-in-composite", "push", "", platforms, secrets}, + {workdir, "set-env-step-env-override", "push", "", platforms, secrets}, + {workdir, "set-env-new-env-file-per-step", "push", "", platforms, secrets}, }...) } diff --git a/pkg/runner/testdata/do-not-leak-step-env-in-composite/push.yml b/pkg/runner/testdata/do-not-leak-step-env-in-composite/push.yml index df5aab7f177..1bebab0bf2f 100644 --- a/pkg/runner/testdata/do-not-leak-step-env-in-composite/push.yml +++ b/pkg/runner/testdata/do-not-leak-step-env-in-composite/push.yml @@ -8,6 +8,7 @@ jobs: using: composite steps: - run: exit 1 + shell: bash if: env.LEAK_ENV != 'val' shell: cp {0} action.yml - uses: ./ diff --git a/pkg/runner/testdata/set-env-new-env-file-per-step/push.yml b/pkg/runner/testdata/set-env-new-env-file-per-step/push.yml new file mode 100644 index 00000000000..34f4bad9253 --- /dev/null +++ b/pkg/runner/testdata/set-env-new-env-file-per-step/push.yml @@ -0,0 +1,15 @@ +on: push +jobs: + _: + runs-on: ubuntu-latest + env: + MY_ENV: test + steps: + - run: exit 1 + if: env.MY_ENV != 'test' + - run: echo "MY_ENV=test2" > $GITHUB_ENV + - run: exit 1 + if: env.MY_ENV != 'test2' + - run: echo "MY_ENV=returnedenv" > $GITHUB_ENV + - run: exit 1 + if: env.MY_ENV != 'returnedenv' \ No newline at end of file diff --git a/pkg/runner/testdata/set-env-step-env-override/push.yml b/pkg/runner/testdata/set-env-step-env-override/push.yml new file mode 100644 index 00000000000..f35ef8758cc --- /dev/null +++ b/pkg/runner/testdata/set-env-step-env-override/push.yml @@ -0,0 +1,24 @@ +on: push +jobs: + _: + runs-on: ubuntu-latest + env: + MY_ENV: test + steps: + - run: exit 1 + if: env.MY_ENV != 'test' + - run: | + runs: + using: composite + steps: + - run: exit 1 + shell: bash + if: env.MY_ENV != 'val' + - run: echo "MY_ENV=returnedenv" > $GITHUB_ENV + shell: bash + shell: cp {0} action.yml + - uses: ./ + env: + MY_ENV: val + - run: exit 1 + if: env.MY_ENV != 'returnedenv' \ No newline at end of file