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..34c755b7c6a 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 @@ -567,7 +637,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 +744,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..96438c4e55d 100644 --- a/pkg/runner/action_test.go +++ b/pkg/runner/action_test.go @@ -4,6 +4,8 @@ import ( "context" "io" "io/fs" + "os" + "path/filepath" "strings" "testing" @@ -49,6 +51,7 @@ runs: PreIf: "always()", PostIf: "always()", }, + ActionPath: "actionPath", }, }, { @@ -64,6 +67,7 @@ runs: PreIf: "always()", PostIf: "always()", }, + ActionPath: "actionPath", }, }, { @@ -77,6 +81,7 @@ runs: Using: "docker", Image: "Dockerfile", }, + ActionPath: "actionPath", }, }, { @@ -104,6 +109,7 @@ runs: Using: "node12", Main: "trampoline.js", }, + ActionPath: "actionPath", }, }, } @@ -140,6 +146,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 @@ -249,3 +287,70 @@ func TestActionRunner(t *testing.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, + ) + } +}