From e3608115706382233c3b943b4dabea09bab9d7d2 Mon Sep 17 00:00:00 2001 From: Markus Wolf Date: Wed, 11 May 2022 21:06:05 +0200 Subject: [PATCH] refactor: remove composite action runcontext workaround (#1085) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor: remove composite action runcontext workaround The RunContext is cloned to execute a composite action with all its steps in a similar context. This required some workaround, since the command handler has kept a reference to the original RunContext. This is solved now, by replacing the docker LogWriter with a proper scoped LogWriter. This prepares for a simpler setup of composite actions to be able to create and re-create the composite RunContext for pre/main/post action steps. * test: check env-vars for local js and docker actions * test: test remote docker and js actions * fix: merge github context into env when read and setup * refacotr: simplify composite context setup * test: use a map matcher to test input setup * fix: restore composite log output Since we create a new line writer, we need to log the raw_output as well. Otherwise no output will be available from the log-writer * fix: add RunContext JobName to fill GITHUB_JOBNAME * test: use nektos/act-test-actions * fix: allow masking values in composite actions To allow masking of values from composite actions, we need to use a custom job logger with a reference to the masked values for the composite run context. * refactor: keep existing logger for composite actions To not introduce another new logger while still be able to use the masking from the composite action, we add the masks to the go context. To leverage that context, we also add the context to the log entries where the valueMasker then could get the actual mask values. With this way to 'inject' the masked values into the logger, we do - keep the logger - keep the coloring - stay away from inconsistencies due to parallel jobs * fix: re-add removed color increase This one should have never removed :-) * fix: add missing ExtraPath attribute * fix: merge run context env into composite run context env This adds a test and fix for the parent environment. It should be inherited by the composite environment. * test: add missing test case * fix: store github token next to secrets We must not expose the secrets to composite actions, but the `github.token` is available inside composite actions. To provide this we store the token in the config and create it in the GithubContext from there. The token can be used with `github.token` but is not available as `secrets.GITHUB_TOKEN`. This implements the same behavior as on GitHub. Co-authored-by: Björn Brauer Co-authored-by: Marcus Noll * fixup! fix: allow masking values in composite actions * style: use tabs instead of spaces to fix linter errors Co-authored-by: Björn Brauer Co-authored-by: Marcus Noll Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> --- cmd/root.go | 1 + pkg/container/docker_run.go | 11 + pkg/runner/action.go | 196 ++++++++++++------ pkg/runner/action_test.go | 31 +-- pkg/runner/expression.go | 14 +- pkg/runner/logger.go | 36 +++- pkg/runner/run_context.go | 55 +---- pkg/runner/run_context_test.go | 12 +- pkg/runner/runner.go | 1 + pkg/runner/runner_test.go | 33 +++ pkg/runner/step.go | 10 +- pkg/runner/step_run.go | 15 +- .../act-composite-env-test/action1/action.yml | 21 ++ .../act-composite-env-test/action2/action.yml | 21 ++ .../testdata/act-composite-env-test/push.yml | 13 ++ .../docker/Dockerfile | 5 + .../docker/action.yml | 5 + .../docker/entrypoint.sh | 26 +++ .../js/action.yml | 5 + .../js/index.js | 15 ++ .../push.yml | 13 ++ .../testdata/mask-values/composite/action.yml | 12 ++ pkg/runner/testdata/mask-values/push.yml | 12 ++ 23 files changed, 396 insertions(+), 167 deletions(-) create mode 100644 pkg/runner/testdata/act-composite-env-test/action1/action.yml create mode 100644 pkg/runner/testdata/act-composite-env-test/action2/action.yml create mode 100644 pkg/runner/testdata/act-composite-env-test/push.yml create mode 100644 pkg/runner/testdata/actions-environment-and-context-tests/docker/Dockerfile create mode 100644 pkg/runner/testdata/actions-environment-and-context-tests/docker/action.yml create mode 100755 pkg/runner/testdata/actions-environment-and-context-tests/docker/entrypoint.sh create mode 100644 pkg/runner/testdata/actions-environment-and-context-tests/js/action.yml create mode 100644 pkg/runner/testdata/actions-environment-and-context-tests/js/index.js create mode 100644 pkg/runner/testdata/actions-environment-and-context-tests/push.yml create mode 100644 pkg/runner/testdata/mask-values/composite/action.yml create mode 100644 pkg/runner/testdata/mask-values/push.yml diff --git a/cmd/root.go b/cmd/root.go index a94f692..568c00a 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -366,6 +366,7 @@ func newRunCommand(ctx context.Context, input *Input) func(*cobra.Command, []str JSONLogger: input.jsonLogger, Env: envs, Secrets: secrets, + Token: secrets["GITHUB_TOKEN"], InsecureSecrets: input.insecureSecrets, Platforms: input.newPlatforms(), Privileged: input.privileged, diff --git a/pkg/container/docker_run.go b/pkg/container/docker_run.go index 5334bfe..1948c31 100644 --- a/pkg/container/docker_run.go +++ b/pkg/container/docker_run.go @@ -77,6 +77,7 @@ type Container interface { UpdateFromPath(env *map[string]string) common.Executor Remove() common.Executor Close() common.Executor + ReplaceLogWriter(io.Writer, io.Writer) (io.Writer, io.Writer) } // NewContainer creates a reference to a container @@ -195,6 +196,16 @@ func (cr *containerReference) Remove() common.Executor { ).IfNot(common.Dryrun) } +func (cr *containerReference) ReplaceLogWriter(stdout io.Writer, stderr io.Writer) (io.Writer, io.Writer) { + out := cr.input.Stdout + err := cr.input.Stderr + + cr.input.Stdout = stdout + cr.input.Stderr = stderr + + return out, err +} + type containerReference struct { cli *client.Client id string diff --git a/pkg/runner/action.go b/pkg/runner/action.go index 6a8d31a..510e947 100644 --- a/pkg/runner/action.go +++ b/pkg/runner/action.go @@ -102,30 +102,31 @@ func runActionImpl(step actionStep, actionDir string, remoteAction *remoteAction rc := step.getRunContext() stepModel := step.getStepModel() return func(ctx context.Context) error { - // Backup the parent composite action path and restore it on continue - parentActionPath := rc.ActionPath - parentActionRepository := rc.ActionRepository - parentActionRef := rc.ActionRef - defer func() { - rc.ActionPath = parentActionPath - rc.ActionRef = parentActionRef - rc.ActionRepository = parentActionRepository - }() - actionPath := "" - if remoteAction != nil { - rc.ActionRef = remoteAction.Ref - rc.ActionRepository = remoteAction.Repo - if remoteAction.Path != "" { - actionPath = remoteAction.Path - } - } else { - rc.ActionRef = "" - rc.ActionRepository = "" + if remoteAction != nil && remoteAction.Path != "" { + actionPath = remoteAction.Path } action := step.getActionModel() log.Debugf("About to run action %v", action) + if remoteAction != nil { + rc.ActionRepository = fmt.Sprintf("%s/%s", remoteAction.Org, remoteAction.Repo) + rc.ActionRef = remoteAction.Ref + } else { + rc.ActionRepository = "" + rc.ActionRef = "" + } + defer (func() { + // cleanup after the action is done, to avoid side-effects in + // the next step/action + rc.ActionRepository = "" + rc.ActionRef = "" + })() + + // we need to merge with github-env again, since at the step setup + // time, we don't have all environment prepared + mergeIntoMap(step.getEnv(), rc.withGithubEnv(map[string]string{})) + populateEnvsFromInput(step.getEnv(), action, rc) actionLocation := path.Join(actionDir, actionPath) @@ -134,7 +135,6 @@ func runActionImpl(step actionStep, actionDir string, remoteAction *remoteAction log.Debugf("type=%v actionDir=%s actionPath=%s workdir=%s actionCacheDir=%s actionName=%s containerActionDir=%s", stepModel.Type(), actionDir, actionPath, rc.Config.Workdir, rc.ActionCacheDir(), actionName, containerActionDir) maybeCopyToActionDir := func() error { - rc.ActionPath = containerActionDir if stepModel.Type() != model.StepTypeUsesActionRemote { return nil } @@ -170,7 +170,7 @@ func runActionImpl(step actionStep, actionDir string, remoteAction *remoteAction if err := maybeCopyToActionDir(); err != nil { return err } - return execAsComposite(step)(ctx) + return execAsComposite(step, containerActionDir)(ctx) default: return fmt.Errorf(fmt.Sprintf("The runs.using key must be one of: %v, got %s", []string{ model.ActionRunsUsingDocker, @@ -359,7 +359,7 @@ func newStepContainer(ctx context.Context, step step, image string, cmd []string return stepContainer } -func execAsComposite(step actionStep) common.Executor { +func execAsComposite(step actionStep, containerActionDir string) common.Executor { rc := step.getRunContext() action := step.getActionModel() @@ -370,9 +370,10 @@ func execAsComposite(step actionStep) common.Executor { return err } } + + eval := rc.NewExpressionEvaluator() + inputs := make(map[string]interface{}) - eval := step.getRunContext().NewExpressionEvaluator() - // Set Defaults for k, input := range action.Inputs { inputs[k] = eval.Interpolate(input.Default) } @@ -381,63 +382,128 @@ func execAsComposite(step actionStep) common.Executor { inputs[k] = eval.Interpolate(v) } } - // Doesn't work with the command processor has a pointer to the original rc - // compositerc := rc.Clone() - // Workaround start - backup := *rc - defer func() { *rc = backup }() - *rc = *rc.Clone() - scriptName := backup.CurrentStep - for rcs := &backup; rcs.Parent != nil; rcs = rcs.Parent { - scriptName = fmt.Sprintf("%s-composite-%s", rcs.Parent.CurrentStep, scriptName) - } - compositerc := rc - compositerc.Parent = &RunContext{ - CurrentStep: scriptName, - } - // Workaround end - compositerc.Composite = action - envToEvaluate := mergeMaps(compositerc.Env, step.getStepModel().Environment()) - compositerc.Env = make(map[string]string) - // origEnvMap: is used to pass env changes back to parent runcontext - origEnvMap := make(map[string]string) - for k, v := range envToEvaluate { - ev := eval.Interpolate(v) - origEnvMap[k] = ev - compositerc.Env[k] = ev - } - compositerc.Inputs = inputs - compositerc.ExprEval = compositerc.NewExpressionEvaluator() - err := compositerc.compositeExecutor()(ctx) + env := make(map[string]string) + for k, v := range rc.Env { + env[k] = eval.Interpolate(v) + } + for k, v := range step.getStepModel().Environment() { + env[k] = eval.Interpolate(v) + } - // Map outputs to parent rc - eval = compositerc.NewStepExpressionEvaluator(step) + // run with the global config but without secrets + configCopy := *rc.Config + configCopy.Secrets = nil + + // create a run context for the composite action to run in + compositerc := &RunContext{ + Name: rc.Name, + JobName: rc.JobName, + Run: &model.Run{ + JobID: "composite-job", + Workflow: &model.Workflow{ + Name: rc.Run.Workflow.Name, + Jobs: map[string]*model.Job{ + "composite-job": {}, + }, + }, + }, + Config: &configCopy, + StepResults: map[string]*model.StepResult{}, + JobContainer: rc.JobContainer, + Inputs: inputs, + ActionPath: containerActionDir, + ActionRepository: rc.ActionRepository, + ActionRef: rc.ActionRef, + Env: env, + Masks: rc.Masks, + ExtraPath: rc.ExtraPath, + } + + ctx = WithCompositeLogger(ctx, &compositerc.Masks) + + // We need to inject a composite RunContext related command + // handler into the current running job container + // We need this, to support scoping commands to the composite action + // executing. + rawLogger := common.Logger(ctx).WithField("raw_output", true) + logWriter := common.NewLineWriter(compositerc.commandHandler(ctx), func(s string) bool { + if rc.Config.LogOutput { + rawLogger.Infof("%s", s) + } else { + rawLogger.Debugf("%s", s) + } + return true + }) + oldout, olderr := compositerc.JobContainer.ReplaceLogWriter(logWriter, logWriter) + defer (func() { + rc.JobContainer.ReplaceLogWriter(oldout, olderr) + })() + + err := compositerc.compositeExecutor(action)(ctx) + + // Map outputs from composite RunContext to job RunContext + eval = compositerc.NewExpressionEvaluator() for outputName, output := range action.Outputs { - backup.setOutput(ctx, map[string]string{ + rc.setOutput(ctx, map[string]string{ "name": outputName, }, eval.Interpolate(output.Value)) } - backup.Masks = append(backup.Masks, compositerc.Masks...) - // Test if evaluated parent env was altered by this composite step - // Known Issues: - // - you try to set an env variable to the same value as a scoped step env, will be discared - for k, v := range compositerc.Env { - if ov, ok := origEnvMap[k]; !ok || ov != v { - backup.Env[k] = v - } - } + rc.Masks = compositerc.Masks + rc.ExtraPath = compositerc.ExtraPath + return err } } +// Executor returns a pipeline executor for all the steps in the job +func (rc *RunContext) compositeExecutor(action *model.Action) common.Executor { + steps := make([]common.Executor, 0) + + sf := &stepFactoryImpl{} + + for i, step := range action.Runs.Steps { + if step.ID == "" { + step.ID = fmt.Sprintf("%d", i) + } + + // create a copy of the step, since this composite action could + // run multiple times and we might modify the instance + stepcopy := step + + step, err := sf.newStep(&stepcopy, rc) + if err != nil { + return common.NewErrorExecutor(err) + } + stepExec := common.NewPipelineExecutor(step.pre(), step.main(), step.post()) + + steps = append(steps, func(ctx context.Context) error { + err := stepExec(ctx) + if err != nil { + common.Logger(ctx).Errorf("%v", err) + common.SetJobError(ctx, err) + } else if ctx.Err() != nil { + common.Logger(ctx).Errorf("%v", ctx.Err()) + common.SetJobError(ctx, ctx.Err()) + } + return nil + }) + } + + steps = append(steps, common.JobError) + return func(ctx context.Context) error { + return common.NewPipelineExecutor(steps...)(common.WithJobErrorContainer(ctx)) + } +} + func populateEnvsFromInput(env *map[string]string, action *model.Action, rc *RunContext) { + eval := rc.NewExpressionEvaluator() for inputID, input := range action.Inputs { envKey := regexp.MustCompile("[^A-Z0-9-]").ReplaceAllString(strings.ToUpper(inputID), "_") envKey = fmt.Sprintf("INPUT_%s", envKey) if _, ok := (*env)[envKey]; !ok { - (*env)[envKey] = rc.ExprEval.Interpolate(input.Default) + (*env)[envKey] = eval.Interpolate(input.Default) } } } diff --git a/pkg/runner/action_test.go b/pkg/runner/action_test.go index 833e8b7..7e1c129 100644 --- a/pkg/runner/action_test.go +++ b/pkg/runner/action_test.go @@ -25,8 +25,8 @@ func TestActionReader(t *testing.T) { yaml := strings.ReplaceAll(` name: 'name' runs: - using: 'node16' - main: 'main.js' + using: 'node16' + main: 'main.js' `, "\t", " ") table := []struct { @@ -136,16 +136,6 @@ runs: } } -type exprEvalMock struct { - ExpressionEvaluator - mock.Mock -} - -func (e *exprEvalMock) Interpolate(expr string) string { - args := e.Called(expr) - return args.String(0) -} - func TestActionRunner(t *testing.T) { table := []struct { name string @@ -158,10 +148,7 @@ func TestActionRunner(t *testing.T) { Uses: "repo@ref", }, RunContext: &RunContext{ - ActionRepository: "repo", - ActionPath: "path", - ActionRef: "ref", - Config: &Config{}, + Config: &Config{}, Run: &model.Run{ JobID: "job", Workflow: &model.Workflow{ @@ -194,17 +181,17 @@ func TestActionRunner(t *testing.T) { cm := &containerMock{} cm.On("CopyDir", "/var/run/act/actions/dir/", "dir/", false).Return(func(ctx context.Context) error { return nil }) - cm.On("Exec", []string{"node", "/var/run/act/actions/dir/path"}, map[string]string{"INPUT_KEY": "default value"}, "", "").Return(func(ctx context.Context) error { return nil }) - tt.step.getRunContext().JobContainer = cm - ee := &exprEvalMock{} - ee.On("Interpolate", "default value").Return("default value") - tt.step.getRunContext().ExprEval = ee + envMatcher := mock.MatchedBy(func(env map[string]string) bool { + return env["INPUT_KEY"] == "default value" + }) + cm.On("Exec", []string{"node", "/var/run/act/actions/dir/path"}, envMatcher, "", "").Return(func(ctx context.Context) error { return nil }) + + tt.step.getRunContext().JobContainer = cm err := runActionImpl(tt.step, "dir", newRemoteAction("org/repo/path@ref"))(ctx) assert.Nil(t, err) - ee.AssertExpectations(t) cm.AssertExpectations(t) }) } diff --git a/pkg/runner/expression.go b/pkg/runner/expression.go index 9dbb16f..5ff311d 100644 --- a/pkg/runner/expression.go +++ b/pkg/runner/expression.go @@ -37,11 +37,6 @@ func (rc *RunContext) NewExpressionEvaluator() ExpressionEvaluator { } } - secrets := rc.Config.Secrets - if rc.Composite != nil { - secrets = nil - } - ee := &exprparser.EvaluationEnvironment{ Github: rc.getGithubContext(), Env: rc.GetEnv(), @@ -54,7 +49,7 @@ func (rc *RunContext) NewExpressionEvaluator() ExpressionEvaluator { "temp": "/tmp", "tool_cache": "/opt/hostedtoolcache", }, - Secrets: secrets, + Secrets: rc.Config.Secrets, Strategy: strategy, Matrix: rc.Matrix, Needs: using, @@ -89,11 +84,6 @@ func (rc *RunContext) NewStepExpressionEvaluator(step step) ExpressionEvaluator } } - secrets := rc.Config.Secrets - if rc.Composite != nil { - secrets = nil - } - ee := &exprparser.EvaluationEnvironment{ Github: rc.getGithubContext(), Env: *step.getEnv(), @@ -104,7 +94,7 @@ func (rc *RunContext) NewStepExpressionEvaluator(step step) ExpressionEvaluator "temp": "/tmp", "tool_cache": "/opt/hostedtoolcache", }, - Secrets: secrets, + Secrets: rc.Config.Secrets, Strategy: strategy, Matrix: rc.Matrix, Needs: using, diff --git a/pkg/runner/logger.go b/pkg/runner/logger.go index a5751b0..f4217a1 100644 --- a/pkg/runner/logger.go +++ b/pkg/runner/logger.go @@ -37,6 +37,26 @@ func init() { } } +type masksContextKey string + +const masksContextKeyVal = masksContextKey("logrus.FieldLogger") + +// Logger returns the appropriate logger for current context +func Masks(ctx context.Context) *[]string { + val := ctx.Value(masksContextKeyVal) + if val != nil { + if masks, ok := val.(*[]string); ok { + return masks + } + } + return &[]string{} +} + +// WithLogger adds a value to the context for the logger +func WithMasks(ctx context.Context, masks *[]string) context.Context { + return context.WithValue(ctx, masksContextKeyVal, masks) +} + // WithJobLogger attaches a new logger to context that is aware of steps func WithJobLogger(ctx context.Context, jobName string, config *Config, masks *[]string) context.Context { mux.Lock() @@ -46,26 +66,32 @@ func WithJobLogger(ctx context.Context, jobName string, config *Config, masks *[ if config.JSONLogger { formatter = &jobLogJSONFormatter{ formatter: &logrus.JSONFormatter{}, - masker: valueMasker(config.InsecureSecrets, config.Secrets, masks), + masker: valueMasker(config.InsecureSecrets, config.Secrets), } } else { formatter = &jobLogFormatter{ color: colors[nextColor%len(colors)], - masker: valueMasker(config.InsecureSecrets, config.Secrets, masks), + masker: valueMasker(config.InsecureSecrets, config.Secrets), } } nextColor++ + ctx = WithMasks(ctx, masks) logger := logrus.New() logger.SetFormatter(formatter) logger.SetOutput(os.Stdout) logger.SetLevel(logrus.GetLevel()) - rtn := logger.WithFields(logrus.Fields{"job": jobName, "dryrun": common.Dryrun(ctx)}) + rtn := logger.WithFields(logrus.Fields{"job": jobName, "dryrun": common.Dryrun(ctx)}).WithContext(ctx) return common.WithLogger(ctx, rtn) } +func WithCompositeLogger(ctx context.Context, masks *[]string) context.Context { + ctx = WithMasks(ctx, masks) + return common.WithLogger(ctx, common.Logger(ctx).WithFields(logrus.Fields{}).WithContext(ctx)) +} + func withStepLogger(ctx context.Context, stepName string) context.Context { rtn := common.Logger(ctx).WithFields(logrus.Fields{"step": stepName}) return common.WithLogger(ctx, rtn) @@ -73,12 +99,14 @@ func withStepLogger(ctx context.Context, stepName string) context.Context { type entryProcessor func(entry *logrus.Entry) *logrus.Entry -func valueMasker(insecureSecrets bool, secrets map[string]string, masks *[]string) entryProcessor { +func valueMasker(insecureSecrets bool, secrets map[string]string) entryProcessor { return func(entry *logrus.Entry) *logrus.Entry { if insecureSecrets { return entry } + masks := Masks(entry.Context) + for _, v := range secrets { if v != "" { entry.Message = strings.ReplaceAll(entry.Message, v, "***") diff --git a/pkg/runner/run_context.go b/pkg/runner/run_context.go index 07f62d8..5b4e126 100644 --- a/pkg/runner/run_context.go +++ b/pkg/runner/run_context.go @@ -43,7 +43,6 @@ type RunContext struct { ActionPath string ActionRef string ActionRepository string - Composite *model.Action Inputs map[string]interface{} Parent *RunContext Masks []string @@ -53,16 +52,6 @@ func (rc *RunContext) AddMask(mask string) { rc.Masks = append(rc.Masks, mask) } -func (rc *RunContext) Clone() *RunContext { - clone := *rc - clone.CurrentStep = "" - clone.Composite = nil - clone.Inputs = nil - clone.StepResults = make(map[string]*model.StepResult) - clone.Parent = rc - return &clone -} - type MappableOutput struct { StepID string OutputName string @@ -309,46 +298,6 @@ func (rc *RunContext) Executor() common.Executor { } } -// Executor returns a pipeline executor for all the steps in the job -func (rc *RunContext) compositeExecutor() common.Executor { - steps := make([]common.Executor, 0) - - sf := &stepFactoryImpl{} - - for i, step := range rc.Composite.Runs.Steps { - if step.ID == "" { - step.ID = fmt.Sprintf("%d", i) - } - - // create a copy of the step, since this composite action could - // run multiple times and we might modify the instance - stepcopy := step - - step, err := sf.newStep(&stepcopy, rc) - if err != nil { - return common.NewErrorExecutor(err) - } - stepExec := common.NewPipelineExecutor(step.pre(), step.main(), step.post()) - - steps = append(steps, func(ctx context.Context) error { - err := stepExec(ctx) - if err != nil { - common.Logger(ctx).Errorf("%v", err) - common.SetJobError(ctx, err) - } else if ctx.Err() != nil { - common.Logger(ctx).Errorf("%v", ctx.Err()) - common.SetJobError(ctx, ctx.Err()) - } - return nil - }) - } - - steps = append(steps, common.JobError) - return func(ctx context.Context) error { - return common.NewPipelineExecutor(steps...)(common.WithJobErrorContainer(ctx)) - } -} - func (rc *RunContext) platformImage() string { job := rc.Run.Job() @@ -492,7 +441,7 @@ func (rc *RunContext) getGithubContext() *model.GithubContext { EventName: rc.Config.EventName, Workspace: rc.Config.ContainerWorkdir(), Action: rc.CurrentStep, - Token: rc.Config.Secrets["GITHUB_TOKEN"], + Token: rc.Config.Token, ActionPath: rc.ActionPath, ActionRef: rc.ActionRef, ActionRepository: rc.ActionRepository, @@ -639,8 +588,6 @@ func (rc *RunContext) withGithubEnv(env map[string]string) map[string]string { env["GITHUB_SERVER_URL"] = "https://github.com" env["GITHUB_API_URL"] = "https://api.github.com" env["GITHUB_GRAPHQL_URL"] = "https://api.github.com/graphql" - env["GITHUB_ACTION_REF"] = github.ActionRef - env["GITHUB_ACTION_REPOSITORY"] = github.ActionRepository env["GITHUB_BASE_REF"] = github.BaseRef env["GITHUB_HEAD_REF"] = github.HeadRef env["GITHUB_JOB"] = rc.JobName diff --git a/pkg/runner/run_context_test.go b/pkg/runner/run_context_test.go index 1deb45d..401a21a 100644 --- a/pkg/runner/run_context_test.go +++ b/pkg/runner/run_context_test.go @@ -310,7 +310,17 @@ func TestRunContext_GetBindsAndMounts(t *testing.T) { }) assert.NoError(t, err) - rc := rctemplate.Clone() + rc := &RunContext{ + Name: "TestRCName", + Run: &model.Run{ + Workflow: &model.Workflow{ + Name: "TestWorkflowName", + }, + }, + Config: &Config{ + BindWorkdir: false, + }, + } rc.Run.JobID = "job1" rc.Run.Workflow.Jobs = map[string]*model.Job{"job1": job} diff --git a/pkg/runner/runner.go b/pkg/runner/runner.go index 751ee8a..026f2ac 100644 --- a/pkg/runner/runner.go +++ b/pkg/runner/runner.go @@ -36,6 +36,7 @@ type Config struct { JSONLogger bool // use json or text logger Env map[string]string // env for containers Secrets map[string]string // list of secrets + Token string // GitHub token InsecureSecrets bool // switch hiding output when printing to terminal Platforms map[string]string // list of platforms Privileged bool // use privileged mode diff --git a/pkg/runner/runner_test.go b/pkg/runner/runner_test.go index bc13d49..fa27995 100644 --- a/pkg/runner/runner_test.go +++ b/pkg/runner/runner_test.go @@ -138,6 +138,7 @@ func TestRunEvent(t *testing.T) { {workdir, "uses-nested-composite", "push", "", platforms}, {workdir, "uses-workflow", "push", "reusable workflows are currently not supported (see https://github.com/nektos/act/issues/826 for updates)", platforms}, {workdir, "uses-docker-url", "push", "", platforms}, + {workdir, "act-composite-env-test", "push", "", platforms}, // Eval {workdir, "evalmatrix", "push", "", platforms}, @@ -171,6 +172,7 @@ func TestRunEvent(t *testing.T) { {workdir, "steps-context/outcome", "push", "", platforms}, {workdir, "job-status-check", "push", "job 'fail' failed", platforms}, {workdir, "if-expressions", "push", "Job 'mytest' failed", platforms}, + {workdir, "actions-environment-and-context-tests", "push", "", platforms}, {"../model/testdata", "strategy", "push", "", platforms}, // TODO: move all testdata into pkg so we can validate it with planner and runner // {"testdata", "issue-228", "push", "", platforms, }, // TODO [igni]: Remove this once everything passes {"../model/testdata", "container-volumes", "push", "", platforms}, @@ -199,6 +201,37 @@ func TestRunDifferentArchitecture(t *testing.T) { tjfi.runTest(context.Background(), t, &Config{ContainerArchitecture: "linux/arm64"}) } +func TestMaskValues(t *testing.T) { + assertNoSecret := func(text string, secret string) { + index := strings.Index(text, "composite secret") + if index > -1 { + fmt.Printf("\nFound Secret in the given text:\n%s\n", text) + } + assert.False(t, strings.Contains(text, "composite secret")) + } + + if testing.Short() { + t.Skip("skipping integration test") + } + + log.SetLevel(log.DebugLevel) + + tjfi := TestJobFileInfo{ + workdir: workdir, + workflowPath: "mask-values", + eventName: "push", + errorMessage: "", + platforms: platforms, + } + + output := captureOutput(t, func() { + tjfi.runTest(context.Background(), t, &Config{}) + }) + + assertNoSecret(output, "secret value") + assertNoSecret(output, "composite secret") +} + func TestRunEventSecrets(t *testing.T) { if testing.Short() { t.Skip("skipping integration test") diff --git a/pkg/runner/step.go b/pkg/runner/step.go index 0f63558..b6ed65a 100644 --- a/pkg/runner/step.go +++ b/pkg/runner/step.go @@ -51,14 +51,18 @@ func runStepExecutor(step step, executor common.Executor) common.Executor { return nil } - common.Logger(ctx).Infof("\u2B50 Run %s", stepModel) + stepString := stepModel.String() + if strings.Contains(stepString, "::add-mask::") { + stepString = "add-mask command" + } + common.Logger(ctx).Infof("\u2B50 Run %s", stepString) err = executor(ctx) if err == nil { - common.Logger(ctx).Infof(" \u2705 Success - %s", stepModel) + common.Logger(ctx).Infof(" \u2705 Success - %s", stepString) } else { - common.Logger(ctx).Errorf(" \u274C Failure - %s", stepModel) + common.Logger(ctx).Errorf(" \u274C Failure - %s", stepString) rc.StepResults[rc.CurrentStep].Outcome = model.StepStatusFailure if stepModel.ContinueOnError { diff --git a/pkg/runner/step_run.go b/pkg/runner/step_run.go index 68ebd90..a26b31a 100644 --- a/pkg/runner/step_run.go +++ b/pkg/runner/step_run.go @@ -9,7 +9,6 @@ import ( "github.com/nektos/act/pkg/common" "github.com/nektos/act/pkg/container" "github.com/nektos/act/pkg/model" - log "github.com/sirupsen/logrus" ) type stepRun struct { @@ -56,7 +55,7 @@ func (sr *stepRun) getEnv() *map[string]string { func (sr *stepRun) setupShellCommandExecutor() common.Executor { return func(ctx context.Context) error { - scriptName, script, err := sr.setupShellCommand() + scriptName, script, err := sr.setupShellCommand(ctx) if err != nil { return err } @@ -82,7 +81,7 @@ func getScriptName(rc *RunContext, step *model.Step) string { // so we return proper errors before any execution or spawning containers // it will error anyway with: // OCI runtime exec failed: exec failed: container_linux.go:380: starting container process caused: exec: "${{": executable file not found in $PATH: unknown -func (sr *stepRun) setupShellCommand() (name, script string, err error) { +func (sr *stepRun) setupShellCommand(ctx context.Context) (name, script string, err error) { sr.setupShell() sr.setupWorkingDirectory() @@ -114,7 +113,11 @@ func (sr *stepRun) setupShellCommand() (name, script string, err error) { script = fmt.Sprintf("%s\n%s\n%s", runPrepend, script, runAppend) - log.Debugf("Wrote command \n%s\n to '%s'", script, name) + if !strings.Contains(script, "::add-mask::") && !sr.RunContext.Config.InsecureSecrets { + common.Logger(ctx).Debugf("Wrote command \n%s\n to '%s'", script, name) + } else { + common.Logger(ctx).Debugf("Wrote add-mask command to '%s'", name) + } scriptPath := fmt.Sprintf("%s/%s", ActPath, name) sr.cmd, err = shellquote.Split(strings.Replace(scCmd, `{0}`, scriptPath, 1)) @@ -130,7 +133,7 @@ func (sr *stepRun) setupShell() { step.Shell = rc.Run.Job().Defaults.Run.Shell } - step.Shell = rc.ExprEval.Interpolate(step.Shell) + step.Shell = rc.NewExpressionEvaluator().Interpolate(step.Shell) if step.Shell == "" { step.Shell = rc.Run.Workflow.Defaults.Run.Shell @@ -157,7 +160,7 @@ func (sr *stepRun) setupWorkingDirectory() { } // jobs can receive context values, so we interpolate - step.WorkingDirectory = rc.ExprEval.Interpolate(step.WorkingDirectory) + step.WorkingDirectory = rc.NewExpressionEvaluator().Interpolate(step.WorkingDirectory) // but top level keys in workflow file like `defaults` or `env` can't if step.WorkingDirectory == "" { diff --git a/pkg/runner/testdata/act-composite-env-test/action1/action.yml b/pkg/runner/testdata/act-composite-env-test/action1/action.yml new file mode 100644 index 0000000..e027ac6 --- /dev/null +++ b/pkg/runner/testdata/act-composite-env-test/action1/action.yml @@ -0,0 +1,21 @@ +name: action1 +description: action1 +runs: + using: composite + steps: + - name: env.COMPOSITE_OVERRIDE != '1' + run: exit 1 + if: env.COMPOSITE_OVERRIDE != '1' + shell: bash + - name: env.JOB != '1' + run: exit 1 + if: env.JOB != '1' + shell: bash + - name: env.GLOBAL != '1' + run: exit 1 + if: env.GLOBAL != '1' + shell: bash + - uses: ./act-composite-env-test/action2 + env: + COMPOSITE_OVERRIDE: "2" + COMPOSITE: "1" diff --git a/pkg/runner/testdata/act-composite-env-test/action2/action.yml b/pkg/runner/testdata/act-composite-env-test/action2/action.yml new file mode 100644 index 0000000..8716838 --- /dev/null +++ b/pkg/runner/testdata/act-composite-env-test/action2/action.yml @@ -0,0 +1,21 @@ +name: action2 +description: actions2 +runs: + using: composite + steps: + - name: env.COMPOSITE_OVERRIDE != '2' + run: exit 1 + if: env.COMPOSITE_OVERRIDE != '2' + shell: bash + - name: env.COMPOSITE != '1' + run: exit 1 + if: env.COMPOSITE != '1' + shell: bash + - name: env.JOB != '1' + run: exit 1 + if: env.JOB != '1' + shell: bash + - name: env.GLOBAL != '1' + run: exit 1 + if: env.GLOBAL != '1' + shell: bash diff --git a/pkg/runner/testdata/act-composite-env-test/push.yml b/pkg/runner/testdata/act-composite-env-test/push.yml new file mode 100644 index 0000000..da7109e --- /dev/null +++ b/pkg/runner/testdata/act-composite-env-test/push.yml @@ -0,0 +1,13 @@ +on: push +env: + GLOBAL: "1" +jobs: + test: + runs-on: ubuntu-latest + env: + JOB: "1" + steps: + - uses: actions/checkout@v2 + - uses: ./act-composite-env-test/action1 + env: + COMPOSITE_OVERRIDE: "1" diff --git a/pkg/runner/testdata/actions-environment-and-context-tests/docker/Dockerfile b/pkg/runner/testdata/actions-environment-and-context-tests/docker/Dockerfile new file mode 100644 index 0000000..bd8fcb2 --- /dev/null +++ b/pkg/runner/testdata/actions-environment-and-context-tests/docker/Dockerfile @@ -0,0 +1,5 @@ +FROM alpine:3 + +COPY entrypoint.sh /entrypoint.sh + +ENTRYPOINT [ "/entrypoint.sh" ] diff --git a/pkg/runner/testdata/actions-environment-and-context-tests/docker/action.yml b/pkg/runner/testdata/actions-environment-and-context-tests/docker/action.yml new file mode 100644 index 0000000..0b0b9f0 --- /dev/null +++ b/pkg/runner/testdata/actions-environment-and-context-tests/docker/action.yml @@ -0,0 +1,5 @@ +name: 'Test' +description: 'Test' +runs: + using: 'docker' + image: 'Dockerfile' diff --git a/pkg/runner/testdata/actions-environment-and-context-tests/docker/entrypoint.sh b/pkg/runner/testdata/actions-environment-and-context-tests/docker/entrypoint.sh new file mode 100755 index 0000000..a954fd4 --- /dev/null +++ b/pkg/runner/testdata/actions-environment-and-context-tests/docker/entrypoint.sh @@ -0,0 +1,26 @@ +#!/bin/sh + +checkEnvVar() { + name=$1 + value=$2 + allowEmpty=$3 + + if [ -z "$value" ]; then + echo "Missing environment variable: $name" + exit 1 + fi + + if [ "$allowEmpty" != "true" ]; then + test=$(echo "$value" |cut -f 2 -d "=") + if [ -z "$test" ]; then + echo "Environment variable is empty: $name" + exit 1 + fi + fi + + echo "$value" +} + +checkEnvVar "GITHUB_ACTION" "$(env |grep "GITHUB_ACTION=")" false +checkEnvVar "GITHUB_ACTION_REPOSITORY" "$(env |grep "GITHUB_ACTION_REPOSITORY=")" true +checkEnvVar "GITHUB_ACTION_REF" "$(env |grep "GITHUB_ACTION_REF=")" true diff --git a/pkg/runner/testdata/actions-environment-and-context-tests/js/action.yml b/pkg/runner/testdata/actions-environment-and-context-tests/js/action.yml new file mode 100644 index 0000000..c29a6df --- /dev/null +++ b/pkg/runner/testdata/actions-environment-and-context-tests/js/action.yml @@ -0,0 +1,5 @@ +name: 'Test' +description: 'Test' +runs: + using: 'node12' + main: 'index.js' diff --git a/pkg/runner/testdata/actions-environment-and-context-tests/js/index.js b/pkg/runner/testdata/actions-environment-and-context-tests/js/index.js new file mode 100644 index 0000000..79d8d06 --- /dev/null +++ b/pkg/runner/testdata/actions-environment-and-context-tests/js/index.js @@ -0,0 +1,15 @@ +function checkEnvVar({ name, allowEmpty }) { + if ( + process.env[name] === undefined || + (allowEmpty === false && process.env[name] === "") + ) { + throw new Error( + `${name} is undefined` + (allowEmpty === false ? " or empty" : "") + ); + } + console.log(`${name}=${process.env[name]}`); +} + +checkEnvVar({ name: "GITHUB_ACTION", allowEmpty: false }); +checkEnvVar({ name: "GITHUB_ACTION_REPOSITORY", allowEmpty: true /* allows to be empty for local actions */ }); +checkEnvVar({ name: "GITHUB_ACTION_REF", allowEmpty: true /* allows to be empty for local actions */ }); diff --git a/pkg/runner/testdata/actions-environment-and-context-tests/push.yml b/pkg/runner/testdata/actions-environment-and-context-tests/push.yml new file mode 100644 index 0000000..db3c341 --- /dev/null +++ b/pkg/runner/testdata/actions-environment-and-context-tests/push.yml @@ -0,0 +1,13 @@ +name: actions-with-environment-and-context-tests +description: "Actions with environment (env vars) and context (expression) tests" +on: push + +jobs: + check: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - uses: './actions-environment-and-context-tests/js' + - uses: './actions-environment-and-context-tests/docker' + - uses: 'nektos/act-test-actions/js@main' + - uses: 'nektos/act-test-actions/docker@main' diff --git a/pkg/runner/testdata/mask-values/composite/action.yml b/pkg/runner/testdata/mask-values/composite/action.yml new file mode 100644 index 0000000..cbeb229 --- /dev/null +++ b/pkg/runner/testdata/mask-values/composite/action.yml @@ -0,0 +1,12 @@ +name: composite +description: composite + +runs: + using: composite + steps: + - run: echo "secret value" + shell: bash + - run: echo "::add-mask::composite secret" + shell: bash + - run: echo "composite secret" + shell: bash diff --git a/pkg/runner/testdata/mask-values/push.yml b/pkg/runner/testdata/mask-values/push.yml new file mode 100644 index 0000000..fb7e837 --- /dev/null +++ b/pkg/runner/testdata/mask-values/push.yml @@ -0,0 +1,12 @@ +name: mask-values +on: push + +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - run: echo "::add-mask::secret value" + - run: echo "secret value" + - uses: ./mask-values/composite + - run: echo "composite secret"