From 1b85d90d8e948896dc1d5ca6a887f08008b0e166 Mon Sep 17 00:00:00 2001 From: AI Healer Bot Date: Fri, 1 May 2026 17:03:41 +0530 Subject: [PATCH] fix: resolve Dockerfile context from action.yml location for remote actions When a remote action declares runs.using: docker with a relative runs.image (e.g. image: Dockerfile), the build context must be resolved relative to the directory containing action.yml, not from the repository root. Previously readActionImpl dropped the actionPath after reading metadata, so execAsDocker always used the uses: subpath to resolve runs.image. If action.yml lives in a subdirectory but uses: has no explicit path component, the Dockerfile was resolved from the wrong location. Changes: - Add ActionPath string (yaml:"-") to model.Action to carry the resolved subpath through the runner without affecting YAML parsing - Set action.ActionPath in all return paths of readActionImpl - When action.yml is not found at the repo root, scan immediate subdirectories of actionDir/actionPath and read from the unique match; return an error if zero or multiple matches are found - Use action.ActionPath as dockerSubpath at all three execAsDocker call sites when running remote actions (main, pre, post) Local actions and the ActionCache tar-stream path are unaffected. Added TestActionReaderDiscoverSubdir to cover the new discovery path. Fixes #739 --- pkg/model/action.go | 1 + pkg/runner/action.go | 84 ++++++++++++++++++++++++++++-- pkg/runner/action_test.go | 105 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 187 insertions(+), 3 deletions(-) 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, + ) + } +}