From ce0890578ac10a0953965b1184ba8e74db5a5072 Mon Sep 17 00:00:00 2001 From: ChristopherHX Date: Fri, 19 Dec 2025 16:24:48 +0000 Subject: [PATCH] refactor: move getContainerActionPaths into step interface (#13) * allows to specialize local vs remote action implementation * allow paths like ./.. for local action Closes https://gitea.com/actions-oss/act-cli/issues/12 Reviewed-on: https://gitea.com/actions-oss/act-cli/pulls/13 Co-authored-by: ChristopherHX Co-committed-by: ChristopherHX --- pkg/container/docker_build.go | 8 +- pkg/runner/action.go | 172 ++++-------------- pkg/runner/action_test.go | 13 +- pkg/runner/runner_test.go | 3 + pkg/runner/step.go | 17 +- pkg/runner/step_action_local.go | 43 ++++- pkg/runner/step_action_local_test.go | 6 +- pkg/runner/step_action_remote.go | 41 ++++- pkg/runner/step_action_remote_test.go | 25 +-- pkg/runner/step_test.go | 27 +++ .../uses-local-dot-dot-dir-symlink/push.yml | 19 ++ 11 files changed, 196 insertions(+), 178 deletions(-) create mode 100644 pkg/runner/testdata/uses-local-dot-dot-dir-symlink/push.yml diff --git a/pkg/container/docker_build.go b/pkg/container/docker_build.go index fa041669..5b2e041e 100644 --- a/pkg/container/docker_build.go +++ b/pkg/container/docker_build.go @@ -4,6 +4,7 @@ package container import ( "context" + "errors" "io" "os" "path/filepath" @@ -61,11 +62,8 @@ func NewDockerBuildExecutor(input NewDockerBuildExecutorInput) common.Executor { logger.Debugf("Creating image from context dir '%s' with tag '%s' and platform '%s'", input.ContextDir, input.ImageTag, input.Platform) resp, err := cli.ImageBuild(ctx, buildContext, options) - err = logDockerResponse(logger, resp.Body, err != nil) - if err != nil { - return err - } - return nil + err = errors.Join(err, logDockerResponse(logger, resp.Body, err != nil)) + return err } } func createBuildContext(ctx context.Context, contextDir string, relDockerfile string) (io.ReadCloser, error) { diff --git a/pkg/runner/action.go b/pkg/runner/action.go index 46b3837c..0fed7694 100644 --- a/pkg/runner/action.go +++ b/pkg/runner/action.go @@ -27,6 +27,10 @@ type actionStep interface { getActionModel() *model.Action getCompositeRunContext(context.Context) *RunContext getCompositeSteps() *compositeSteps + getContainerActionPaths() (actionName string, containerActionDir string) + getTarArchive(ctx context.Context, src string) (io.ReadCloser, error) + getActionPath() string + maybeCopyToActionDir(ctx context.Context) error } type readAction func(ctx context.Context, step *model.Step, actionDir string, actionPath string, readFile actionYamlReader, writeFile fileWriter) (*model.Action, error) @@ -35,7 +39,7 @@ type actionYamlReader func(filename string) (io.Reader, io.Closer, error) type fileWriter func(filename string, data []byte, perm fs.FileMode) error -type runAction func(step actionStep, actionDir string, remoteAction *remoteAction) common.Executor +type runAction func(step actionStep) common.Executor //go:embed res/trampoline.js var trampoline embed.FS @@ -116,60 +120,30 @@ func readActionImpl(ctx context.Context, step *model.Step, actionDir string, act return action, err } -func maybeCopyToActionDir(ctx context.Context, step actionStep, actionPath string, containerActionDir string) error { - logger := common.Logger(ctx) - rc := step.getRunContext() - stepModel := step.getStepModel() - - if stepModel.Type() != model.StepTypeUsesActionRemote { - return nil - } - - var containerActionDirCopy string - containerActionDirCopy = strings.TrimSuffix(containerActionDir, actionPath) - logger.Debug(containerActionDirCopy) - - if !strings.HasSuffix(containerActionDirCopy, `/`) { - containerActionDirCopy += `/` - } - - raction := step.(*stepActionRemote) - ta, err := rc.getActionCache().GetTarArchive(ctx, raction.cacheDir, raction.resolvedSha, "") - if err != nil { - return err - } - defer ta.Close() - return rc.JobContainer.CopyTarStream(ctx, containerActionDirCopy, ta) -} - -func runActionImpl(step actionStep, actionDir string, remoteAction *remoteAction) common.Executor { +func runActionImpl(step actionStep) common.Executor { rc := step.getRunContext() stepModel := step.getStepModel() return func(ctx context.Context) error { logger := common.Logger(ctx) - actionPath := "" - if remoteAction != nil && remoteAction.Path != "" { - actionPath = remoteAction.Path - } + actionPath := step.getActionPath() action := step.getActionModel() logger.Debugf("About to run action %v", action) - err := setupActionEnv(ctx, step, remoteAction) + err := setupActionEnv(ctx, step) if err != nil { return err } - actionLocation := path.Join(actionDir, actionPath) - actionName, containerActionDir := getContainerActionPaths(stepModel, actionLocation, rc) + actionName, containerActionDir := step.getContainerActionPaths() - logger.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) + logger.Debugf("type=%v actionPath=%s workdir=%s actionCacheDir=%s actionName=%s containerActionDir=%s", stepModel.Type(), actionPath, rc.Config.Workdir, rc.ActionCacheDir(), actionName, containerActionDir) x := action.Runs.Using switch { case x.IsNode(): - if err := maybeCopyToActionDir(ctx, step, actionPath, containerActionDir); err != nil { + if err := step.maybeCopyToActionDir(ctx); err != nil { return err } containerArgs := []string{rc.GetNodeToolFullPath(ctx), path.Join(containerActionDir, action.Runs.Main)} @@ -179,13 +153,9 @@ func runActionImpl(step actionStep, actionDir string, remoteAction *remoteAction return rc.execJobContainer(containerArgs, *step.getEnv(), "", "")(ctx) case x.IsDocker(): - if remoteAction == nil { - actionDir = "" - actionPath = containerActionDir - } - return execAsDocker(ctx, step, actionName, actionDir, actionPath, remoteAction == nil, "entrypoint") + return execAsDocker(ctx, step, actionName, actionPath, "entrypoint") case x.IsComposite(): - if err := maybeCopyToActionDir(ctx, step, actionPath, containerActionDir); err != nil { + if err := step.maybeCopyToActionDir(ctx); err != nil { return err } @@ -200,7 +170,7 @@ func runActionImpl(step actionStep, actionDir string, remoteAction *remoteAction } } -func setupActionEnv(ctx context.Context, step actionStep, _ *remoteAction) error { +func setupActionEnv(ctx context.Context, step actionStep) error { rc := step.getRunContext() // A few fields in the environment (e.g. GITHUB_ACTION_REPOSITORY) @@ -217,7 +187,7 @@ func setupActionEnv(ctx context.Context, step actionStep, _ *remoteAction) error // TODO: break out parts of function to reduce complexicity // //nolint:gocyclo -func execAsDocker(ctx context.Context, step actionStep, actionName, basedir, subpath string, localAction bool, entrypointType string) error { +func execAsDocker(ctx context.Context, step actionStep, actionName, subpath string, entrypointType string) error { logger := common.Logger(ctx) rc := step.getRunContext() action := step.getActionModel() @@ -230,7 +200,7 @@ func execAsDocker(ctx context.Context, step actionStep, actionName, basedir, sub // Apply forcePull only for prebuild docker images forcePull = rc.Config.ForcePull } else { - // "-dockeraction" enshures that "./", "./test " won't get converted to "act-:latest", "act-test-:latest" which are invalid docker image names + // "-dockeraction" ensures that "./", "./test " won't get converted to "act-:latest", "act-test-:latest" which are invalid docker image names image = fmt.Sprintf("%s-dockeraction:%s", regexp.MustCompile("[^a-zA-Z0-9]").ReplaceAllString(actionName, "-"), "latest") image = fmt.Sprintf("act-%s", strings.TrimLeft(image, "-")) image = strings.ToLower(image) @@ -258,23 +228,12 @@ func execAsDocker(ctx context.Context, step actionStep, actionName, basedir, sub if !correctArchExists || rc.Config.ForceRebuild { logger.Debugf("image '%s' for architecture '%s' will be built from context '%s", image, rc.Config.ContainerArchitecture, contextDir) - var buildContext io.ReadCloser - if localAction { - buildContext, err = rc.JobContainer.GetContainerArchive(ctx, contextDir+"/.") - if err != nil { - return err - } - defer buildContext.Close() - } else { - rstep := step.(*stepActionRemote) - buildContext, err = rc.getActionCache().GetTarArchive(ctx, rstep.cacheDir, rstep.resolvedSha, contextDir) - if err != nil { - return err - } - defer buildContext.Close() + buildContext, err := step.getTarArchive(ctx, contextDir+".") + if err != nil { + return err } + defer buildContext.Close() prepImage = container.NewDockerBuildExecutor(container.NewDockerBuildExecutorInput{ - ContextDir: filepath.Join(basedir, contextDir), Dockerfile: fileName, ImageTag: image, BuildContext: buildContext, @@ -432,32 +391,16 @@ func populateEnvsFromInput(ctx context.Context, env *map[string]string, action * } } -func getContainerActionPaths(step *model.Step, actionDir string, rc *RunContext) (string, string) { - actionName := "" - containerActionDir := "." - if step.Type() != model.StepTypeUsesActionRemote { - actionName = getOsSafeRelativePath(actionDir, rc.Config.Workdir) - containerActionDir = rc.JobContainer.ToContainerPath(rc.Config.Workdir) + "/" + actionName - actionName = "./" + actionName - } else if step.Type() == model.StepTypeUsesActionRemote { - actionName = getOsSafeRelativePath(actionDir, rc.ActionCacheDir()) - containerActionDir = rc.JobContainer.GetActPath() + "/actions/" + actionName +func normalizePath(s string) string { + if runtime.GOOS == "windows" { + return strings.ReplaceAll(s, "\\", "/") } - - if actionName == "" { - actionName = filepath.Base(actionDir) - if runtime.GOOS == "windows" { - actionName = strings.ReplaceAll(actionName, "\\", "/") - } - } - return actionName, containerActionDir + return s } func getOsSafeRelativePath(s, prefix string) string { actionName := strings.TrimPrefix(s, prefix) - if runtime.GOOS == "windows" { - actionName = strings.ReplaceAll(actionName, "\\", "/") - } + actionName = normalizePath(actionName) actionName = strings.TrimPrefix(actionName, "/") return actionName @@ -493,38 +436,19 @@ func runPreStep(step actionStep) common.Executor { logger.Debugf("run pre step for '%s'", step.getStepModel()) rc := step.getRunContext() - stepModel := step.getStepModel() action := step.getActionModel() // defaults in pre steps were missing, however provided inputs are available populateEnvsFromInput(ctx, step.getEnv(), action, rc) - // todo: refactor into step - var actionDir string - var actionPath string - var remoteAction *stepActionRemote - if remote, ok := step.(*stepActionRemote); ok { - actionPath = newRemoteAction(stepModel.Uses).Path - actionDir = fmt.Sprintf("%s/%s", rc.ActionCacheDir(), safeFilename(stepModel.Uses)) - remoteAction = remote - } else { - actionDir = filepath.Join(rc.Config.Workdir, stepModel.Uses) - actionPath = "" - } + actionPath := step.getActionPath() - actionLocation := "" - if actionPath != "" { - actionLocation = path.Join(actionDir, actionPath) - } else { - actionLocation = actionDir - } - - actionName, containerActionDir := getContainerActionPaths(stepModel, actionLocation, rc) + actionName, containerActionDir := step.getContainerActionPaths() x := action.Runs.Using switch { case x.IsNode(): - if err := maybeCopyToActionDir(ctx, step, actionPath, containerActionDir); err != nil { + if err := step.maybeCopyToActionDir(ctx); err != nil { return err } @@ -536,11 +460,7 @@ func runPreStep(step actionStep) common.Executor { return rc.execJobContainer(containerArgs, *step.getEnv(), "", "")(ctx) case x.IsDocker(): - if remoteAction == nil { - actionDir = "" - actionPath = containerActionDir - } - return execAsDocker(ctx, step, actionName, actionDir, actionPath, remoteAction == nil, "pre-entrypoint") + return execAsDocker(ctx, step, actionName, actionPath, "pre-entrypoint") case x.IsComposite(): if step.getCompositeSteps() == nil { @@ -600,34 +520,18 @@ func runPostStep(step actionStep) common.Executor { logger.Debugf("run post step for '%s'", step.getStepModel()) rc := step.getRunContext() - stepModel := step.getStepModel() action := step.getActionModel() - // todo: refactor into step - var actionDir string - var actionPath string - var remoteAction *stepActionRemote - if remote, ok := step.(*stepActionRemote); ok { - actionPath = newRemoteAction(stepModel.Uses).Path - actionDir = fmt.Sprintf("%s/%s", rc.ActionCacheDir(), safeFilename(stepModel.Uses)) - remoteAction = remote - } else { - actionDir = filepath.Join(rc.Config.Workdir, stepModel.Uses) - actionPath = "" - } + actionPath := step.getActionPath() - actionLocation := "" - if actionPath != "" { - actionLocation = path.Join(actionDir, actionPath) - } else { - actionLocation = actionDir - } - - actionName, containerActionDir := getContainerActionPaths(stepModel, actionLocation, rc) + actionName, containerActionDir := step.getContainerActionPaths() x := action.Runs.Using switch { case x.IsNode(): + if err := step.maybeCopyToActionDir(ctx); err != nil { + return err + } populateEnvsFromSavedState(step.getEnv(), step, rc) populateEnvsFromInput(ctx, step.getEnv(), step.getActionModel(), rc) @@ -639,14 +543,10 @@ func runPostStep(step actionStep) common.Executor { return rc.execJobContainer(containerArgs, *step.getEnv(), "", "")(ctx) case x.IsDocker(): - if remoteAction == nil { - actionDir = "" - actionPath = containerActionDir - } - return execAsDocker(ctx, step, actionName, actionDir, actionPath, remoteAction == nil, "post-entrypoint") + return execAsDocker(ctx, step, actionName, actionPath, "post-entrypoint") case x.IsComposite(): - if err := maybeCopyToActionDir(ctx, step, actionPath, containerActionDir); err != nil { + if err := step.maybeCopyToActionDir(ctx); err != nil { return err } diff --git a/pkg/runner/action_test.go b/pkg/runner/action_test.go index 59f4c3e0..b19df463 100644 --- a/pkg/runner/action_test.go +++ b/pkg/runner/action_test.go @@ -141,6 +141,7 @@ runs: } func TestActionRunner(t *testing.T) { + remoteAction := newRemoteAction("org/repo/path@ref") table := []struct { name string step actionStep @@ -176,7 +177,8 @@ func TestActionRunner(t *testing.T) { Using: "node16", }, }, - env: map[string]string{}, + remoteAction: remoteAction, + env: map[string]string{}, }, expectedEnv: map[string]string{"INPUT_KEY": "default value"}, }, @@ -216,7 +218,8 @@ func TestActionRunner(t *testing.T) { Using: "node16", }, }, - env: map[string]string{}, + remoteAction: remoteAction, + env: map[string]string{}, }, expectedEnv: map[string]string{"STATE_name": "state value"}, }, @@ -227,7 +230,7 @@ func TestActionRunner(t *testing.T) { ctx := context.Background() cm := &containerMock{} - cm.Mock.On("CopyTarStream", ctx, "/var/run/act/actions/dir/", mock.Anything).Return(nil) + cm.Mock.On("CopyTarStream", ctx, "/var/run/act/actions/org-repo-path@ref/", mock.Anything).Return(nil) cacheMock := &TestRepositoryCache{} cacheMock.Mock.On("GetTarArchive", ctx, "", "", "").Return(io.NopCloser(io.MultiReader())) @@ -241,12 +244,12 @@ func TestActionRunner(t *testing.T) { return true }) - cm.On("Exec", []string{"node", "/var/run/act/actions/dir/path"}, envMatcher, "", "").Return(func(_ context.Context) error { return nil }) + cm.On("Exec", []string{"node", "/var/run/act/actions/org-repo-path@ref/path"}, envMatcher, "", "").Return(func(_ context.Context) error { return nil }) tt.step.getRunContext().JobContainer = cm tt.step.getRunContext().Config.ActionCache = cacheMock - err := runActionImpl(tt.step, "dir", newRemoteAction("org/repo/path@ref"))(ctx) + err := runActionImpl(tt.step)(ctx) assert.Nil(t, err) cm.AssertExpectations(t) diff --git a/pkg/runner/runner_test.go b/pkg/runner/runner_test.go index 1fe6e21d..501130aa 100644 --- a/pkg/runner/runner_test.go +++ b/pkg/runner/runner_test.go @@ -330,6 +330,9 @@ func TestRunEvent(t *testing.T) { // local remote action overrides {workdir, "local-remote-action-overrides", "push", "", platforms, secrets}, + + // local folder symlink in ./../action-sym + {workdir, "uses-local-dot-dot-dir-symlink", "push", "", platforms, secrets}, } if _, ok := os.LookupEnv("DOOD"); !ok { diff --git a/pkg/runner/step.go b/pkg/runner/step.go index 46b39ec0..8749cfd0 100644 --- a/pkg/runner/step.go +++ b/pkg/runner/step.go @@ -362,12 +362,19 @@ func mergeIntoMapCaseInsensitive(target map[string]string, maps ...map[string]st } } -func symlinkJoin(filename, sym, parent string) (string, error) { +func symlinkJoin(filename, sym string, parents ...string) (string, error) { dir := path.Dir(filename) dest := path.Join(dir, sym) - prefix := path.Clean(parent) + "/" - if strings.HasPrefix(dest, prefix) || prefix == "./" { - return dest, nil + var builder strings.Builder + for _, parent := range parents { + prefix := strings.TrimSuffix(path.Clean(parent), "/") + "/" + if strings.HasPrefix(dest, prefix) || prefix == "./" { + return dest, nil + } + if builder.Len() != 0 { + builder.WriteString(", ") + } + builder.WriteString(strings.ReplaceAll(dest, "'", "''")) } - return "", fmt.Errorf("symlink tries to access file '%s' outside of '%s'", strings.ReplaceAll(dest, "'", "''"), strings.ReplaceAll(parent, "'", "''")) + return "", fmt.Errorf("symlink tries to access file '%s' outside of '%s'", strings.ReplaceAll(dest, "'", "''"), builder.String()) } diff --git a/pkg/runner/step_action_local.go b/pkg/runner/step_action_local.go index e8bf18fe..89206a10 100644 --- a/pkg/runner/step_action_local.go +++ b/pkg/runner/step_action_local.go @@ -40,10 +40,18 @@ func (sal *stepActionLocal) main() common.Executor { return nil } - actionDir := filepath.Join(sal.getRunContext().Config.Workdir, sal.Step.Uses) + workdir := sal.getRunContext().Config.Workdir + actionDir := filepath.Join(workdir, sal.Step.Uses) localReader := func(ctx context.Context) actionYamlReader { - _, cpath := getContainerActionPaths(sal.Step, path.Join(actionDir, ""), sal.RunContext) + // In case we want to limit resolving symlinks, folders are resolved by archive function + // _, cpath = sal.getContainerActionPathsExt(".") + roots := []string{ + ".", // Allow everything, other code permits it already + // path.Dir(cpath), // Allow RUNNER_WORKSPACE e.g. GITHUB_WORKSPACE/../ + // sal.RunContext.JobContainer.GetActPath(), // Allow remote action folders + } + _, cpath := sal.getContainerActionPaths() return func(filename string) (io.Reader, io.Closer, error) { spath := path.Join(cpath, filename) for i := 0; i < maxSymlinkDepth; i++ { @@ -61,7 +69,7 @@ func (sal *stepActionLocal) main() common.Executor { return nil, nil, err } if header.FileInfo().Mode()&os.ModeSymlink == os.ModeSymlink { - spath, err = symlinkJoin(spath, header.Linkname, cpath) + spath, err = symlinkJoin(spath, header.Linkname, roots...) if err != nil { return nil, nil, err } @@ -80,7 +88,7 @@ func (sal *stepActionLocal) main() common.Executor { sal.action = actionModel - return sal.runAction(sal, actionDir, nil)(ctx) + return sal.runAction(sal)(ctx) }) } @@ -118,10 +126,33 @@ func (sal *stepActionLocal) getActionModel() *model.Action { return sal.action } +func (sal *stepActionLocal) getContainerActionPathsExt(subPath string) (string, string) { + workdir := sal.RunContext.Config.Workdir + actionName := normalizePath(subPath) + containerActionDir := path.Join(sal.RunContext.JobContainer.ToContainerPath(workdir), actionName) + return actionName, containerActionDir +} + +func (sal *stepActionLocal) getContainerActionPaths() (string, string) { + return sal.getContainerActionPathsExt(sal.Step.Uses) +} + +func (sal *stepActionLocal) getTarArchive(ctx context.Context, src string) (io.ReadCloser, error) { + return sal.RunContext.JobContainer.GetContainerArchive(ctx, src) +} + +func (sal *stepActionLocal) getActionPath() string { + return sal.RunContext.JobContainer.ToContainerPath(path.Join(sal.RunContext.Config.Workdir, sal.Step.Uses)) +} + +func (sal *stepActionLocal) maybeCopyToActionDir(_ context.Context) error { + // nothing to do + return nil +} + func (sal *stepActionLocal) getCompositeRunContext(ctx context.Context) *RunContext { if sal.compositeRunContext == nil { - actionDir := filepath.Join(sal.RunContext.Config.Workdir, sal.Step.Uses) - _, containerActionDir := getContainerActionPaths(sal.getStepModel(), actionDir, sal.RunContext) + _, containerActionDir := sal.getContainerActionPaths() sal.compositeRunContext = newCompositeRunContext(ctx, sal.RunContext, sal, containerActionDir) sal.compositeSteps = sal.compositeRunContext.compositeExecutor(sal.action) diff --git a/pkg/runner/step_action_local_test.go b/pkg/runner/step_action_local_test.go index 845ef5a5..6d0688fa 100644 --- a/pkg/runner/step_action_local_test.go +++ b/pkg/runner/step_action_local_test.go @@ -19,8 +19,8 @@ type stepActionLocalMocks struct { mock.Mock } -func (salm *stepActionLocalMocks) runAction(step actionStep, actionDir string, remoteAction *remoteAction) common.Executor { - args := salm.Called(step, actionDir, remoteAction) +func (salm *stepActionLocalMocks) runAction(step actionStep) common.Executor { + args := salm.Called(step) return args.Get(0).(func(context.Context) error) } @@ -88,7 +88,7 @@ func TestStepActionLocalTest(t *testing.T) { cm.On("GetContainerArchive", ctx, "/var/run/act/workflow/SUMMARY.md").Return(io.NopCloser(&bytes.Buffer{}), nil) cm.On("GetContainerArchive", ctx, "/var/run/act/workflow/pathcmd.txt").Return(io.NopCloser(&bytes.Buffer{}), nil) - salm.On("runAction", sal, filepath.Clean("/tmp/path/to/action"), (*remoteAction)(nil)).Return(func(_ context.Context) error { + salm.On("runAction", sal).Return(func(_ context.Context) error { return nil }) diff --git a/pkg/runner/step_action_remote.go b/pkg/runner/step_action_remote.go index 8f9df270..742e576e 100644 --- a/pkg/runner/step_action_remote.go +++ b/pkg/runner/step_action_remote.go @@ -121,9 +121,7 @@ func (sar *stepActionRemote) main() common.Executor { return sar.RunContext.JobContainer.CopyDir(copyToPath, sar.RunContext.Config.Workdir+string(filepath.Separator)+".", sar.RunContext.Config.UseGitIgnore)(ctx) } - actionDir := fmt.Sprintf("%s/%s", sar.RunContext.ActionCacheDir(), safeFilename(sar.Step.Uses)) - - return sar.runAction(sar, actionDir, sar.remoteAction)(ctx) + return sar.runAction(sar)(ctx) }), ) } @@ -178,11 +176,42 @@ func (sar *stepActionRemote) getActionModel() *model.Action { return sar.action } +func (sar *stepActionRemote) getContainerActionPaths() (string, string) { + return sar.getContainerActionPathsExt(sar.getActionPath()) +} + +func (sar *stepActionRemote) getContainerActionPathsExt(subPath string) (string, string) { + cacheDir := sar.RunContext.ActionCacheDir() + actionDir := filepath.Join(cacheDir, safeFilename(sar.Step.Uses), subPath) + actionName := getOsSafeRelativePath(actionDir, cacheDir) + containerActionDir := sar.RunContext.JobContainer.GetActPath() + "/actions/" + actionName + return actionName, containerActionDir +} + +func (sar *stepActionRemote) getTarArchive(ctx context.Context, src string) (io.ReadCloser, error) { + return sar.RunContext.getActionCache().GetTarArchive(ctx, sar.cacheDir, sar.resolvedSha, src) +} + +func (sar *stepActionRemote) getActionPath() string { + return sar.remoteAction.Path +} + +func (sar *stepActionRemote) maybeCopyToActionDir(ctx context.Context) error { + rc := sar.getRunContext() + + _, containerRootPath := sar.getContainerActionPathsExt("") + + ta, err := sar.getTarArchive(ctx, "") + if err != nil { + return err + } + defer ta.Close() + return rc.JobContainer.CopyTarStream(ctx, strings.TrimSuffix(containerRootPath, "/")+"/", ta) +} + func (sar *stepActionRemote) getCompositeRunContext(ctx context.Context) *RunContext { if sar.compositeRunContext == nil { - actionDir := fmt.Sprintf("%s/%s", sar.RunContext.ActionCacheDir(), safeFilename(sar.Step.Uses)) - actionLocation := path.Join(actionDir, sar.remoteAction.Path) - _, containerActionDir := getContainerActionPaths(sar.getStepModel(), actionLocation, sar.RunContext) + _, containerActionDir := sar.getContainerActionPaths() sar.compositeRunContext = newCompositeRunContext(ctx, sar.RunContext, sar, containerActionDir) sar.compositeSteps = sar.compositeRunContext.compositeExecutor(sar.action) diff --git a/pkg/runner/step_action_remote_test.go b/pkg/runner/step_action_remote_test.go index bf9d93bf..ffedd2dc 100644 --- a/pkg/runner/step_action_remote_test.go +++ b/pkg/runner/step_action_remote_test.go @@ -5,7 +5,6 @@ import ( "context" "errors" "io" - "strings" "testing" "github.com/stretchr/testify/assert" @@ -25,8 +24,8 @@ func (sarm *stepActionRemoteMocks) readAction(_ context.Context, step *model.Ste return args.Get(0).(*model.Action), args.Error(1) } -func (sarm *stepActionRemoteMocks) runAction(step actionStep, actionDir string, remoteAction *remoteAction) common.Executor { - args := sarm.Called(step, actionDir, remoteAction) +func (sarm *stepActionRemoteMocks) runAction(step actionStep) common.Executor { + args := sarm.Called(step) return args.Get(0).(func(context.Context) error) } @@ -200,11 +199,6 @@ func TestStepActionRemote(t *testing.T) { } cacheMock.Mock.On("Fetch", ctx, mock.AnythingOfType("string"), serverURL+"/remote/action", "v1", "").Return("someval") - suffixMatcher := func(suffix string) interface{} { - return mock.MatchedBy(func(actionDir string) bool { - return strings.HasSuffix(actionDir, suffix) - }) - } if tt.mocks.read { sarm.Mock.On("readAction", sar.Step, "someval", "", mock.Anything, mock.Anything).Return(&model.Action{}, nil) @@ -212,7 +206,7 @@ func TestStepActionRemote(t *testing.T) { if tt.mocks.run { remoteAction := newRemoteAction(sar.Step.Uses) remoteAction.URL = serverURL - sarm.On("runAction", sar, suffixMatcher("act/remote-action@v1"), remoteAction).Return(func(_ context.Context) error { return tt.runError }) + sarm.On("runAction", sar).Return(func(_ context.Context) error { return tt.runError }) cm.On("Copy", "/var/run/act", mock.AnythingOfType("[]*container.FileEntry")).Return(func(_ context.Context) error { return nil @@ -550,12 +544,14 @@ func TestStepActionRemotePost(t *testing.T) { ctx := context.Background() cm := &containerMock{} + cacheMock := &TestRepositoryCache{} sar := &stepActionRemote{ env: map[string]string{}, RunContext: &RunContext{ Config: &Config{ GitHubInstance: "https://github.com", + ActionCache: cacheMock, }, JobContainer: cm, Run: &model.Run{ @@ -570,8 +566,9 @@ func TestStepActionRemotePost(t *testing.T) { IntraActionState: tt.IntraActionState, nodeToolFullPath: "node", }, - Step: tt.stepModel, - action: tt.actionModel, + Step: tt.stepModel, + action: tt.actionModel, + remoteAction: newRemoteAction(tt.stepModel.Uses), } sar.RunContext.ExprEval = sar.RunContext.NewExpressionEvaluator(ctx) @@ -596,6 +593,10 @@ func TestStepActionRemotePost(t *testing.T) { cm.On("GetContainerArchive", ctx, "/var/run/act/workflow/SUMMARY.md").Return(io.NopCloser(&bytes.Buffer{}), nil) cm.On("GetContainerArchive", ctx, "/var/run/act/workflow/pathcmd.txt").Return(io.NopCloser(&bytes.Buffer{}), nil) + + actionArchive := io.NopCloser(&bytes.Buffer{}) + cacheMock.On("GetTarArchive", mock.Anything, "", "", "").Return(actionArchive, nil) + cm.On("CopyTarStream", mock.Anything, "/var/run/act/actions/remote-action@v1/", actionArchive).Return(nil) } err := sar.post()(ctx) @@ -606,7 +607,7 @@ func TestStepActionRemotePost(t *testing.T) { assert.Equal(t, value, sar.env[key]) } } - // Enshure that StepResults is nil in this test + // Ensure that StepResults is nil in this test assert.Equal(t, sar.RunContext.StepResults["post-step"], (*model.StepResult)(nil)) cm.AssertExpectations(t) }) diff --git a/pkg/runner/step_test.go b/pkg/runner/step_test.go index 2b97a89c..4f7b336a 100644 --- a/pkg/runner/step_test.go +++ b/pkg/runner/step_test.go @@ -345,3 +345,30 @@ func TestIsContinueOnError(t *testing.T) { assertObject.False(continueOnError) assertObject.NotNil(err) } + +func TestSymlinkJoin(t *testing.T) { + table := []struct { + from string + target string + root []string + result string + err string + }{ + {"/some/file/somewhere/action.yml", "../../../", []string{"/"}, "/", ""}, + {"/some/file/somewhere/action.yml", "../../../var/lib/act/action.yml", []string{"/some/file/somewhere", "/var/lib/act"}, "/var/lib/act/action.yml", ""}, + {"/some/file/somewhere/action.yml", "../../../var/lib/act/action.yml", []string{"/"}, "/var/lib/act/action.yml", ""}, + {"/some/file/somewhere/action.yml", "../../var/lib/act/action.yml", []string{"/some/file/somewhere", "/var/lib/act"}, "", "Not allowed"}, + {"/some/file/somewhere/action.yml", "../../var/lib/act/action.yml", []string{"/some/file", "/var/lib/act"}, "", "Not allowed"}, + {"/some/file/somewhere/action.yml", "../../var/lib/act/action.yml", []string{"/some/", "/var/lib/act"}, "/some/var/lib/act/action.yml", ""}, + } + + for _, entry := range table { + result, err := symlinkJoin(entry.from, entry.target, entry.root...) + if entry.err == "" { + assert.NoError(t, err) + } else { + assert.Error(t, err, entry.err) + } + assert.Equal(t, entry.result, result) + } +} diff --git a/pkg/runner/testdata/uses-local-dot-dot-dir-symlink/push.yml b/pkg/runner/testdata/uses-local-dot-dot-dir-symlink/push.yml new file mode 100644 index 00000000..38b2318f --- /dev/null +++ b/pkg/runner/testdata/uses-local-dot-dot-dir-symlink/push.yml @@ -0,0 +1,19 @@ +on: push +jobs: + test-symlinks: + runs-on: ubuntu-latest + steps: + - name: Create symlink + run: | + mkdir action-dir + cat << 'EOF' > action-dir/action.yml + runs: + using: composite + steps: + - run: echo test + shell: bash + EOF + ln -s $PWD/action-dir ../action-sym + - run: cat ../action-sym/action.yml + - name: Verify symlink + uses: ./../action-sym \ No newline at end of file