diff --git a/pkg/model/action.go b/pkg/model/action.go index 4478271c39c..80e0935b5df 100644 --- a/pkg/model/action.go +++ b/pkg/model/action.go @@ -93,6 +93,7 @@ type Action struct { Inputs map[string]Input `yaml:"inputs"` Outputs map[string]Output `yaml:"outputs"` Runs ActionRuns `yaml:"runs"` + ActionPath string `yaml:"-"` Branding struct { Color string `yaml:"color"` Icon string `yaml:"icon"` diff --git a/pkg/runner/action.go b/pkg/runner/action.go index b46a4d03a75..13580a0baf0 100644 --- a/pkg/runner/action.go +++ b/pkg/runner/action.go @@ -67,6 +67,7 @@ func readActionImpl(ctx context.Context, step *model.Step, actionDir string, act Using: "docker", Image: "Dockerfile", }, + ActionPath: actionPath, } logger.Debugf("Using synthetic action %v for Dockerfile", action) return action, nil @@ -99,6 +100,7 @@ func readActionImpl(ctx context.Context, step *model.Step, actionDir string, act Using: "node12", Main: "trampoline.js", }, + ActionPath: actionPath, } logger.Debugf("Using synthetic action %v", action) return action, nil @@ -107,11 +109,75 @@ func readActionImpl(ctx context.Context, step *model.Step, actionDir string, act } } if allErrors != nil { + baseDir := filepath.Join(actionDir, actionPath) + // ActionCache case: actionDir is a SHA hash, not a real directory. + // os.Stat/ReadDir won't work with tar-backed readers. + // We explicitly check and return a clear error so users know why discovery failed. + info, statErr := os.Stat(baseDir) + if statErr != nil { + // baseDir doesn't exist at all — likely ActionCache is enabled. + // Return original errors plus a clear explanation. + return nil, errors.Join( + errors.Join(allErrors...), + fmt.Errorf("subdir discovery failed for %q: directory not accessible (ActionCache may be enabled, which uses a tar-backed reader incompatible with filesystem discovery): %w", baseDir, statErr), + ) + } + if info.IsDir() { + entries, readErr := os.ReadDir(baseDir) + if readErr != nil { + return nil, errors.Join( + errors.Join(allErrors...), + fmt.Errorf("failed to read action directory %q: %w", baseDir, readErr), + ) + } + var discoveredFile string + var discoveredSubdir string + for _, entry := range entries { + if !entry.IsDir() { + continue + } + subdir := entry.Name() + actionYml := filepath.Join(baseDir, subdir, "action.yml") + actionYaml := filepath.Join(baseDir, subdir, "action.yaml") + if _, err := os.Stat(actionYml); err == nil { + if discoveredFile != "" { + return nil, fmt.Errorf("ambiguous action: found action.yml in multiple subdirectories: %q and %q (in %q)", discoveredFile, actionYml, baseDir) + } + discoveredFile = actionYml + discoveredSubdir = subdir + continue + } + if _, err := os.Stat(actionYaml); err == nil { + if discoveredFile != "" { + return nil, fmt.Errorf("ambiguous action: found action.yml in multiple subdirectories: %q and %q (in %q)", discoveredFile, actionYaml, baseDir) + } + discoveredFile = actionYaml + discoveredSubdir = subdir + } + } + if discoveredFile != "" { + file, openErr := os.Open(discoveredFile) + if openErr != nil { + return nil, fmt.Errorf("%s failed to open discovered action file %q (action path %q, base dir %q): %w", step.String(), discoveredFile, actionPath, baseDir, openErr) + } + defer file.Close() + action, readErr := model.ReadAction(file) + if readErr != nil { + return nil, fmt.Errorf("%s failed to read discovered action file %q (action path %q, base dir %q): %w", step.String(), discoveredFile, actionPath, baseDir, readErr) + } + action.ActionPath = path.Join(actionPath, discoveredSubdir) + logger.Debugf("Read action %v from '%s'", action, discoveredFile) + return action, nil + } + } return nil, errors.Join(allErrors...) } defer closer.Close() action, err := model.ReadAction(reader) + if err == nil { + action.ActionPath = actionPath + } logger.Debugf("Read action %v from '%s'", action, "Unknown") return action, err } @@ -191,7 +257,11 @@ func runActionImpl(step actionStep, actionDir string, remoteAction *remoteAction actionDir = "" actionPath = containerActionDir } - return execAsDocker(ctx, step, actionName, actionDir, actionPath, remoteAction == nil, "entrypoint") + dockerSubpath := actionPath + if remoteAction != nil && action.ActionPath != "" { + dockerSubpath = action.ActionPath + } + return execAsDocker(ctx, step, actionName, actionDir, dockerSubpath, remoteAction == nil, "entrypoint") case x.IsComposite(): if err := maybeCopyToActionDir(ctx, step, actionDir, actionPath, containerActionDir); err != nil { return err @@ -463,31 +533,24 @@ func getContainerActionPaths(step *model.Step, actionDir string, rc *RunContext) actionName := "" containerActionDir := "." if step.Type() != model.StepTypeUsesActionRemote { - actionName = getOsSafeRelativePath(actionDir, rc.Config.Workdir) - containerActionDir = rc.JobContainer.ToContainerPath(rc.Config.Workdir) + "/" + actionName - actionName = "./" + actionName + actionName = "./" + getOsSafeRelativePath(actionDir, rc.Config.Workdir) + containerActionDir = rc.JobContainer.ToContainerPath(actionDir) } else if step.Type() == model.StepTypeUsesActionRemote { actionName = getOsSafeRelativePath(actionDir, rc.ActionCacheDir()) containerActionDir = rc.JobContainer.GetActPath() + "/actions/" + actionName } - - if actionName == "" { - actionName = filepath.Base(actionDir) - if runtime.GOOS == "windows" { - actionName = strings.ReplaceAll(actionName, "\\", "/") - } - } return actionName, containerActionDir } -func getOsSafeRelativePath(s, prefix string) string { - actionName := strings.TrimPrefix(s, prefix) - if runtime.GOOS == "windows" { - actionName = strings.ReplaceAll(actionName, "\\", "/") - } - actionName = strings.TrimPrefix(actionName, "/") - return actionName +func getOsSafeRelativePath(s, basepath string) string { + if relpath, err := filepath.Rel(basepath, s); err == nil { + if runtime.GOOS == "windows" { + relpath = strings.ReplaceAll(relpath, "\\", "/") + } + return relpath + } + return filepath.Base(s) } func shouldRunPreStep(step actionStep) common.Conditional { @@ -567,7 +630,11 @@ func runPreStep(step actionStep) common.Executor { actionDir = "" actionPath = containerActionDir } - return execAsDocker(ctx, step, actionName, actionDir, actionPath, remoteAction == nil, "pre-entrypoint") + dockerSubpath := actionPath + if remoteAction != nil && action.ActionPath != "" { + dockerSubpath = action.ActionPath + } + return execAsDocker(ctx, step, actionName, actionDir, dockerSubpath, remoteAction == nil, "pre-entrypoint") case x.IsComposite(): if step.getCompositeSteps() == nil { @@ -670,7 +737,11 @@ func runPostStep(step actionStep) common.Executor { actionDir = "" actionPath = containerActionDir } - return execAsDocker(ctx, step, actionName, actionDir, actionPath, remoteAction == nil, "post-entrypoint") + dockerSubpath := actionPath + if remoteAction != nil && action.ActionPath != "" { + dockerSubpath = action.ActionPath + } + return execAsDocker(ctx, step, actionName, actionDir, dockerSubpath, remoteAction == nil, "post-entrypoint") case x.IsComposite(): if err := maybeCopyToActionDir(ctx, step, actionDir, actionPath, containerActionDir); err != nil { diff --git a/pkg/runner/action_test.go b/pkg/runner/action_test.go index bc55bd7716e..c0e0a992091 100644 --- a/pkg/runner/action_test.go +++ b/pkg/runner/action_test.go @@ -2,8 +2,11 @@ package runner import ( "context" + "fmt" "io" "io/fs" + "os" + "path/filepath" "strings" "testing" @@ -49,6 +52,7 @@ runs: PreIf: "always()", PostIf: "always()", }, + ActionPath: "actionPath", }, }, { @@ -64,6 +68,7 @@ runs: PreIf: "always()", PostIf: "always()", }, + ActionPath: "actionPath", }, }, { @@ -77,6 +82,7 @@ runs: Using: "docker", Image: "Dockerfile", }, + ActionPath: "actionPath", }, }, { @@ -104,6 +110,7 @@ runs: Using: "node12", Main: "trampoline.js", }, + ActionPath: "actionPath", }, }, } @@ -140,6 +147,38 @@ runs: } } +func TestActionReaderDiscoverSubdir(t *testing.T) { + yaml := ` +name: 'name' +runs: + using: 'node16' + main: 'main.js' +` + + baseDir := t.TempDir() + subdir := filepath.Join(baseDir, "github-action") + if err := os.MkdirAll(subdir, 0o755); err != nil { + t.Fatalf("failed to create subdir: %v", err) + } + if err := os.WriteFile(filepath.Join(subdir, "action.yml"), []byte(yaml), 0o644); err != nil { + t.Fatalf("failed to write action.yml: %v", err) + } + + readFile := func(filename string) (io.Reader, io.Closer, error) { + return nil, nil, fs.ErrNotExist + } + + writeFile := func(filename string, _ []byte, perm fs.FileMode) error { + t.Fatalf("unexpected writeFile call: %s %v", filename, perm) + return nil + } + + action, err := readActionImpl(context.Background(), &model.Step{}, baseDir, "", readFile, writeFile) + + assert.Nil(t, err) + assert.Equal(t, "github-action", action.ActionPath) +} + func TestActionRunner(t *testing.T) { table := []struct { name string @@ -225,9 +264,10 @@ func TestActionRunner(t *testing.T) { for _, tt := range table { t.Run(tt.name, func(t *testing.T) { ctx := context.Background() + actionDir := fmt.Sprintf("%s/dir", tt.step.getRunContext().ActionCacheDir()) cm := &containerMock{} - cm.On("CopyDir", "/var/run/act/actions/dir/", "dir/", false).Return(func(_ context.Context) error { return nil }) + cm.On("CopyDir", "/var/run/act/actions/dir/", actionDir+"/", false).Return(func(_ context.Context) error { return nil }) envMatcher := mock.MatchedBy(func(env map[string]string) bool { for k, v := range tt.expectedEnv { @@ -242,10 +282,77 @@ func TestActionRunner(t *testing.T) { tt.step.getRunContext().JobContainer = cm - err := runActionImpl(tt.step, "dir", newRemoteAction("org/repo/path@ref"))(ctx) + err := runActionImpl(tt.step, actionDir, newRemoteAction("org/repo/path@ref"))(ctx) assert.Nil(t, err) cm.AssertExpectations(t) }) } } + +func TestActionReaderDiscoverSubdirMultipleMatches(t *testing.T) { + yaml := ` +name: 'name' +runs: + using: 'node16' + main: 'main.js' +` + baseDir := t.TempDir() + for _, dir := range []string{"github-action", "nested-action"} { + subdir := filepath.Join(baseDir, dir) + if err := os.MkdirAll(subdir, 0o755); err != nil { + t.Fatalf("failed to create subdir %q: %v", dir, err) + } + if err := os.WriteFile(filepath.Join(subdir, "action.yml"), []byte(yaml), 0o644); err != nil { + t.Fatalf("failed to write action.yml in %q: %v", dir, err) + } + } + + readFile := func(filename string) (io.Reader, io.Closer, error) { + return nil, nil, fs.ErrNotExist + } + + writeFile := func(filename string, _ []byte, perm fs.FileMode) error { + t.Fatalf("unexpected writeFile call: %s %v", filename, perm) + return nil + } + + action, err := readActionImpl(context.Background(), &model.Step{}, baseDir, "", readFile, writeFile) + + assert.Nil(t, action) + if assert.Error(t, err) { + assert.True(t, + strings.Contains(strings.ToLower(err.Error()), "multiple") || + strings.Contains(strings.ToLower(err.Error()), "ambiguous"), + "expected a clear error for multiple discovered action subdirectories, got: %v", err, + ) + } +} + +func TestActionReaderDiscoverSubdirActionCacheUnsupported(t *testing.T) { + // When ActionCache is enabled, actionDir is a SHA hash (not a real path). + // Discovery should fail with a clear, actionable error — not silently. + nonExistentDir := filepath.Join(t.TempDir(), "sha-abc123def456") + + readFile := func(filename string) (io.Reader, io.Closer, error) { + return nil, nil, fs.ErrNotExist + } + + writeFile := func(filename string, _ []byte, perm fs.FileMode) error { + t.Fatalf("unexpected writeFile call: %s %v", filename, perm) + return nil + } + + action, err := readActionImpl(context.Background(), &model.Step{}, nonExistentDir, "", readFile, writeFile) + + assert.Nil(t, action) + if assert.Error(t, err) { + assert.True(t, + strings.Contains(strings.ToLower(err.Error()), "actioncache") || + strings.Contains(strings.ToLower(err.Error()), "tar") || + strings.Contains(strings.ToLower(err.Error()), "subdir") || + strings.Contains(strings.ToLower(err.Error()), "accessible"), + "expected a clear error about ActionCache/discovery failure, got: %v", err, + ) + } +} diff --git a/pkg/runner/runner_test.go b/pkg/runner/runner_test.go index 5c3959af2cd..144384ff3b2 100644 --- a/pkg/runner/runner_test.go +++ b/pkg/runner/runner_test.go @@ -243,6 +243,7 @@ func TestRunEvent(t *testing.T) { {workdir, "local-action-docker-url", "push", "", platforms, secrets}, {workdir, "local-action-dockerfile", "push", "", platforms, secrets}, {workdir, "local-action-via-composite-dockerfile", "push", "", platforms, secrets}, + {workdir, "local-action-outside-workspace", "push", "", platforms, secrets}, {workdir, "local-action-js", "push", "", platforms, secrets}, // Uses diff --git a/pkg/runner/testdata/local-action-outside-workspace/push.yml b/pkg/runner/testdata/local-action-outside-workspace/push.yml new file mode 100644 index 00000000000..d9c0b63a899 --- /dev/null +++ b/pkg/runner/testdata/local-action-outside-workspace/push.yml @@ -0,0 +1,17 @@ +name: local-action-outside-workspace +on: push +jobs: + test: + runs-on: ubuntu-latest + steps: + - run: mkdir ../action-outside-workspace + - run: | + cat <<-ACTIONEOF > ../action-outside-workspace/action.yml + name: test + runs: + using: composite + steps: + - run: echo hello + shell: bash + ACTIONEOF + - uses: ./../action-outside-workspace