refactor: GITHUB_ENV command / remove env.PATH (#1503)

* fix: GITHUB_ENV / PATH handling

* apply workaround

* add ctx to ApplyExtraPath

* fix: Do not leak step env in composite

See https://github.com/nektos/act/pull/1585 for a test

* add more tests

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
This commit is contained in:
ChristopherHX 2023-02-04 14:35:13 +01:00 committed by GitHub
parent 24c16fbf25
commit e775fea265
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
18 changed files with 176 additions and 64 deletions

View file

@ -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:
@ -491,7 +491,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)
@ -580,7 +580,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)

View file

@ -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,6 +100,14 @@ func execAsComposite(step actionStep) common.Executor {
rc.Masks = append(rc.Masks, compositeRC.Masks...)
rc.ExtraPath = compositeRC.ExtraPath
// 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
}

View file

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

View file

@ -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
@ -275,8 +276,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,
@ -296,11 +295,21 @@ 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] == "" {
(*env)[path] = rc.JobContainer.DefaultPathVariable()
cenv := map[string]string{}
var cpath string
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])...)
}
@ -664,7 +673,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

View file

@ -168,7 +168,7 @@ func TestRunEvent(t *testing.T) {
{workdir, "container-hostname", "push", "", platforms, secrets},
{workdir, "remote-action-docker", "push", "", platforms, secrets},
{workdir, "remote-action-js", "push", "", platforms, secrets},
{workdir, "remote-action-js", "push", "", map[string]string{"ubuntu-latest": "catthehacker/ubuntu:runner-latest"}, secrets}, // Test if this works with non root container
{workdir, "remote-action-js-node-user", "push", "", platforms, secrets}, // Test if this works with non root container
{workdir, "matrix", "push", "", platforms, secrets},
{workdir, "matrix-include-exclude", "push", "", platforms, secrets},
{workdir, "matrix-exitcode", "push", "Job 'test' failed", platforms, secrets},
@ -193,6 +193,8 @@ func TestRunEvent(t *testing.T) {
{workdir, "actions-environment-and-context-tests", "push", "", platforms, secrets},
{workdir, "uses-action-with-pre-and-post-step", "push", "", platforms, secrets},
{workdir, "evalenv", "push", "", platforms, secrets},
{workdir, "docker-action-custom-path", "push", "", platforms, secrets},
{workdir, "GITHUB_ENV-use-in-env-ctx", "push", "", platforms, secrets},
{workdir, "ensure-post-steps", "push", "Job 'second-post-step-should-fail' failed", platforms, secrets},
{workdir, "workflow_dispatch", "workflow_dispatch", "", platforms, secrets},
{workdir, "workflow_dispatch_no_inputs_mapping", "workflow_dispatch", "", platforms, secrets},
@ -204,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 {
@ -304,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},
}...)
}

View file

@ -44,6 +44,18 @@ func (s stepStage) String() 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)
@ -92,9 +104,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,
@ -104,6 +118,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)
@ -131,21 +148,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 {
@ -162,14 +175,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())

View file

@ -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
})
@ -197,7 +193,7 @@ func TestStepActionLocalPost(t *testing.T) {
env bool
exec bool
}{
env: true,
env: false,
exec: false,
},
},
@ -260,10 +256,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 {
@ -276,6 +268,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
})

View file

@ -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
})
@ -579,10 +579,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 })
@ -590,6 +586,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
})

View file

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

View file

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

View file

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

View file

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

View file

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

View file

@ -8,6 +8,7 @@ jobs:
using: composite
steps:
- run: exit 1
shell: bash
if: env.LEAK_ENV != 'val'
shell: cp {0} action.yml
- uses: ./

View file

@ -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: ./

View file

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

View file

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

View file

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