Don't walk nested worktrees or submodules in --migrate-local-actions

cmd/gh-actions-lock/migrate_test.go

@@ -56,6 +56,34 @@ func TestMigrateLocalActions_CompositeActionFiles(t *testing.T) {
 	assert.Contains(t, read("my-action/action.yml"), "uses: ./nonexistent")
 }
 
+// TestMigrateLocalActions_RepoRootEnvAboveCheckout covers a RepoRootEnv root
+// without .git that holds the real checkout in a subdirectory, as with
+// actions/checkout `path: src`. That checkout must not be skipped as nested.
+func TestMigrateLocalActions_RepoRootEnvAboveCheckout(t *testing.T) {
+	workspace := t.TempDir()
+	write := func(rel, content string) {
+		full := filepath.Join(workspace, rel)
+		require.NoError(t, os.MkdirAll(filepath.Dir(full), 0o755))
+		require.NoError(t, os.WriteFile(full, []byte(content), 0o644))
+	}
+	require.NoError(t, os.MkdirAll(filepath.Join(workspace, "src", ".git"), 0o755))
+	write("src/.github/workflows/ci.yml",
+		"name: CI\non: push\njobs:\n  b:\n    runs-on: ubuntu-latest\n    steps:\n      - uses: actions/checkout@v4\n")
+	write("src/my-action/action.yml",
+		"name: My Action\nruns:\n  using: composite\n  steps:\n    - uses: ./src/helper\n")
+	write("src/helper/action.yml",
+		"name: Helper\nruns:\n  using: composite\n  steps:\n    - run: echo hi\n      shell: bash\n")
+	t.Setenv("GH_ACTIONS_LOCK_REPO_ROOT", workspace)
+
+	total, _, err := migrateLocalActions([]string{filepath.Join(workspace, "src", ".github", "workflows", "ci.yml")})
+
+	require.NoError(t, err)
+	assert.Equal(t, 1, total)
+	b, err := os.ReadFile(filepath.Join(workspace, "src", "my-action", "action.yml"))
+	require.NoError(t, err)
+	assert.Contains(t, string(b), "uses: $/src/helper")
+}
+
 func TestMigrateLocalActions_StructuralErrorPreventsAllWrites(t *testing.T) {
 	dir := t.TempDir()
 	write := func(rel, content string) string {

internal/workflowfile/rewrite.go

@@ -179,7 +179,10 @@ func (f *File) validateSelfRepositoryMigration(scan RefScan) error {
 }
 
 // localActionExists reports whether a `./…` path resolves to an action file
-// (action.yml or action.yaml) within the repo root.
+// (action.yml or action.yaml) within the repo root. An action inside a
+// submodule or other nested checkout doesn't count: it isn't part of this
+// repository, so a `$/` ref can't reach it, and DiscoverCompositeActionFiles
+// doesn't migrate its own `./…` steps.
 func localActionExists(repoRoot, localPath string) bool {
 	relPath := strings.TrimPrefix(localPath, "./")
 	actionDir := filepath.Join(repoRoot, relPath)
@@ -191,6 +194,9 @@ func localActionExists(repoRoot, localPath string) bool {
 		if err := ValidatePathWithinRoot(repoRoot, actionPath); err != nil {
 			continue
 		}
+		if inNestedCheckout(repoRoot, actionPath) {
+			continue
+		}
 		info, err := os.Stat(actionPath)
 		if err == nil && info.Mode().IsRegular() {
 			return true

internal/workflowfile/workflowfile.go

@@ -351,26 +351,53 @@ func FindRepoRoot(startPath string) (string, error) {
 
 // DiscoverCompositeActionFiles walks the repository rooted at root and returns
 // the paths of all action definition files (action.yml / action.yaml). The
-// .git directory is skipped. Non-composite action files are included; callers
-// migrate them with MigrateLocalActionsToSelfRepository, which no-ops when a file has
-// no local `./…` steps.
+// .git directory is skipped. When root is itself a checkout, so is any
+// subdirectory that is a separate checkout (worktree, submodule or nested
+// clone); see inNestedCheckout. A RepoRootEnv root without .git may hold the
+// real checkout below it, so nothing is skipped there. Non-composite action
+// files are included; callers migrate them with
+// MigrateLocalActionsToSelfRepository, which no-ops when a file has no local
+// `./…` steps.
 func DiscoverCompositeActionFiles(root string) ([]string, error) {
 	if root == "" {
 		return nil, nil
 	}
-	var paths []string
-	err := filepath.WalkDir(root, func(p string, d fs.DirEntry, walkErr error) error {
-		if walkErr != nil {
-			return walkErr
-		}
-		if d.IsDir() {
-			if d.Name() == ".git" {
-				return filepath.SkipDir
+	w := actionFileWalker{root: root, skipNested: isCheckout(root)}
+	if err := w.walk(root); err != nil {
+		return nil, fmt.Errorf("discovering action files under %s: %w", root, err)
+	}
+	sort.Strings(w.paths)
+	return w.paths, nil
+}
+
+type actionFileWalker struct {
+	root       string
+	skipNested bool
+	paths      []string
+}
+
+// walk reads each directory once and checks its listing for a .git entry,
+// so detecting nested checkouts costs no extra syscall per directory.
+func (w *actionFileWalker) walk(dir string) error {
+	entries, err := os.ReadDir(dir)
+	if err != nil {
+		return err
+	}
+	if w.skipNested && dir != w.root && hasEntry(entries, ".git") && isCheckout(dir) {
+		return nil
+	}
+	for _, e := range entries {
+		p := filepath.Join(dir, e.Name())
+		switch {
+		case e.IsDir():
+			if e.Name() == ".git" {
+				continue
 			}
-			return nil
-		}
-		if d.Name() == "action.yml" || d.Name() == "action.yaml" {
-			if err := ValidatePathWithinRoot(root, p); err != nil {
+			if err := w.walk(p); err != nil {
+				return err
+			}
+		case e.Name() == "action.yml" || e.Name() == "action.yaml":
+			if err := ValidatePathWithinRoot(w.root, p); err != nil {
 				return err
 			}
 			info, err := os.Stat(p)
@@ -380,15 +407,69 @@ func DiscoverCompositeActionFiles(root string) ([]string, error) {
 			if !info.Mode().IsRegular() {
 				return fmt.Errorf("action definition %s is not a regular file", p)
 			}
-			paths = append(paths, p)
+			// The walk already skipped nested checkouts, so only a symlinked
+			// action file can still point into one.
+			if w.skipNested && e.Type()&fs.ModeSymlink != 0 && inNestedCheckout(w.root, p) {
+				continue
+			}
+			w.paths = append(w.paths, p)
 		}
-		return nil
-	})
+	}
+	return nil
+}
+
+func hasEntry(entries []fs.DirEntry, name string) bool {
+	return slices.ContainsFunc(entries, func(e fs.DirEntry) bool { return e.Name() == name })
+}
+
+// isCheckout reports whether dir is the top of a git checkout. A worktree or
+// submodule has a .git file and a clone has a .git directory. Stat follows
+// symlinks, so a dangling .git link does not count.
+func isCheckout(dir string) bool {
+	_, err := os.Stat(filepath.Join(dir, ".git"))
+	return err == nil
+}
+
+// inNestedCheckout reports whether file, after resolving symlinks, lies in a
+// separate checkout below root. Resolving matters because a symlink outside
+// the nested checkout can point into it. As in DiscoverCompositeActionFiles,
+// this only applies when root is itself a checkout. Callers have already
+// validated file with ValidatePathWithinRoot, so a resolution error here
+// means the tree changed underneath us; treat it as nested to stay safe.
+//
+// EvalSymlinks doesn't resolve Windows junctions (Go reports them as
+// ModeIrregular), so a junction on the path hides where it leads. Those
+// count as nested too, at the cost of not migrating a junction that stays
+// inside this checkout.
+func inNestedCheckout(root, file string) bool {
+	if !isCheckout(root) {
+		return false
+	}
+	resolvedRoot, err := filepath.EvalSymlinks(root)
 	if err != nil {
-		return nil, fmt.Errorf("discovering action files under %s: %w", root, err)
+		return true
 	}
-	sort.Strings(paths)
-	return paths, nil
+	resolvedFile, err := filepath.EvalSymlinks(file)
+	if err != nil {
+		return true
+	}
+	rel, err := filepath.Rel(resolvedRoot, resolvedFile)
+	if err != nil || !filepath.IsLocal(rel) {
+		return true
+	}
+	for dir := filepath.Dir(rel); dir != "."; dir = filepath.Dir(dir) {
+		p := filepath.Join(resolvedRoot, dir)
+		if !isPlainDir(p) || isCheckout(p) {
+			return true
+		}
+	}
+	return false
+}
+
+// isPlainDir uses Lstat so a junction, which Stat would follow, doesn't count.
+func isPlainDir(p string) bool {
+	info, err := os.Lstat(p)
+	return err == nil && info.IsDir()
 }
 
 // walkUses visits `uses:` values at the only places GitHub Actions honors
@@ -539,7 +620,7 @@ func findRepoRoot(startPath string) (string, error) {
 		return overrideRepoRoot(root, absPath)
 	}
 	for {
-		if _, err := os.Stat(filepath.Join(absPath, ".git")); err == nil {
+		if isCheckout(absPath) {
 			return absPath, nil
 		}
 		parent := filepath.Dir(absPath)

internal/workflowfile/workflowfile_test.go

@@ -3,7 +3,10 @@ package workflowfile
 import (
 	"errors"
 	"os"
+	"os/exec"
 	"path/filepath"
+	"runtime"
+	"slices"
 	"strings"
 	"testing"
 
@@ -642,6 +645,65 @@ func TestMigrateLocalActionsToSelfRepository(t *testing.T) {
 	assert.Contains(t, string(out), "uses: ./missing-action")
 }
 
+func TestMigrateLocalActionsToSelfRepository_NestedCheckout(t *testing.T) {
+	tests := []struct {
+		name  string
+		uses  string
+		setup func(t *testing.T, repoRoot string)
+	}{
+		{name: "leaves action inside submodule", uses: "./vendor/sub/action"},
+		{
+			name: "leaves symlinked dir pointing into submodule",
+			uses: "./alias",
+			setup: func(t *testing.T, repoRoot string) {
+				symlinkOrSkip(t, filepath.Join("vendor", "sub", "action"), filepath.Join(repoRoot, "alias"))
+			},
+		},
+		{
+			name: "leaves action.yml symlinked into submodule",
+			uses: "./shim",
+			setup: func(t *testing.T, repoRoot string) {
+				shim := filepath.Join(repoRoot, "shim")
+				require.NoError(t, os.Mkdir(shim, 0o755))
+				symlinkOrSkip(t, filepath.Join("..", "vendor", "sub", "action", "action.yml"), filepath.Join(shim, "action.yml"))
+			},
+		},
+		{
+			name: "leaves junction pointing into submodule",
+			uses: "./alias",
+			setup: func(t *testing.T, repoRoot string) {
+				junction(t, filepath.Join(repoRoot, "vendor", "sub", "action"), filepath.Join(repoRoot, "alias"))
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			repoRoot := t.TempDir()
+			require.NoError(t, os.Mkdir(filepath.Join(repoRoot, ".git"), 0o755))
+			require.NoError(t, os.MkdirAll(filepath.Join(repoRoot, ".github", "workflows"), 0o755))
+			subAction := filepath.Join(repoRoot, "vendor", "sub", "action")
+			require.NoError(t, os.MkdirAll(subAction, 0o755))
+			require.NoError(t, os.WriteFile(filepath.Join(repoRoot, "vendor", "sub", ".git"), []byte("gitdir: ../../.git/modules/sub\n"), 0o644))
+			require.NoError(t, os.WriteFile(filepath.Join(subAction, "action.yml"), []byte("runs:\n  using: composite\n"), 0o644))
+			if tt.setup != nil {
+				tt.setup(t, repoRoot)
+			}
+
+			workflowPath := filepath.Join(repoRoot, ".github", "workflows", "ci.yml")
+			content := []byte("jobs:\n  build:\n    steps:\n      - uses: " + tt.uses + "\n")
+			require.NoError(t, os.WriteFile(workflowPath, content, 0o644))
+			f, err := Load(workflowPath)
+			require.NoError(t, err)
+
+			out, changed, err := f.MigrateLocalActionsToSelfRepository()
+
+			require.NoError(t, err)
+			assert.Zero(t, changed, "a submodule action isn't part of this repository")
+			assert.Equal(t, content, out)
+		})
+	}
+}
+
 func TestMigrateLocalActionsToSelfRepository_NoLocalPaths(t *testing.T) {
 	content := []byte("jobs:\n  build:\n    steps:\n      - uses: actions/checkout@v4\n")
 	f, err := Parse("ci.yml", content)
@@ -713,6 +775,98 @@ func TestDiscoverCompositeActionFiles(t *testing.T) {
 	assert.Equal(t, want, got)
 }
 
+func TestDiscoverCompositeActionFiles_NestedCheckouts(t *testing.T) {
+	gitFile := func(t *testing.T, dir string) {
+		require.NoError(t, os.WriteFile(filepath.Join(dir, ".git"), []byte("gitdir: /elsewhere\n"), 0o644))
+	}
+	gitDir := func(t *testing.T, dir string) {
+		require.NoError(t, os.Mkdir(filepath.Join(dir, ".git"), 0o755))
+	}
+	danglingGit := func(t *testing.T, dir string) {
+		symlinkOrSkip(t, "missing", filepath.Join(dir, ".git"))
+	}
+	tests := []struct {
+		name     string
+		markRoot func(*testing.T, string)
+		nested   string
+		markDir  func(*testing.T, string)
+		wantSeen bool
+	}{
+		{name: "skips nested worktree", markRoot: gitDir, nested: ".gwt/some-wt", markDir: gitFile},
+		{name: "skips nested clone", markRoot: gitDir, nested: "vendor/clone", markDir: gitDir},
+		{name: "walks root that is itself a worktree", markRoot: gitFile, nested: "sub", wantSeen: true},
+		{name: "walks checkout below a root without .git", nested: "src", markDir: gitDir, wantSeen: true},
+		{name: "walks dir whose .git is a dangling symlink", markRoot: gitDir, nested: "fixture", markDir: danglingGit, wantSeen: true},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			root := t.TempDir()
+			if tt.markRoot != nil {
+				tt.markRoot(t, root)
+			}
+			dir := filepath.Join(root, tt.nested)
+			require.NoError(t, os.MkdirAll(dir, 0o755))
+			if tt.markDir != nil {
+				tt.markDir(t, dir)
+			}
+			action := filepath.Join(dir, "action.yml")
+			require.NoError(t, os.WriteFile(action, []byte("runs:\n  using: composite\n"), 0o644))
+
+			got, err := DiscoverCompositeActionFiles(root)
+
+			require.NoError(t, err)
+			assert.Equal(t, tt.wantSeen, slices.Contains(got, action))
+		})
+	}
+}
+
+func TestDiscoverCompositeActionFiles_SymlinkIntoNestedCheckout(t *testing.T) {
+	root := t.TempDir()
+	require.NoError(t, os.Mkdir(filepath.Join(root, ".git"), 0o755))
+	sub := filepath.Join(root, "vendor", "sub")
+	require.NoError(t, os.MkdirAll(sub, 0o755))
+	require.NoError(t, os.WriteFile(filepath.Join(sub, ".git"), []byte("gitdir: /elsewhere\n"), 0o644))
+	require.NoError(t, os.WriteFile(filepath.Join(sub, "action.yml"), []byte("runs:\n  using: composite\n"), 0o644))
+	own := filepath.Join(root, "own")
+	require.NoError(t, os.Mkdir(own, 0o755))
+	require.NoError(t, os.WriteFile(filepath.Join(own, "action.yml"), []byte("runs:\n  using: composite\n"), 0o644))
+	shim := filepath.Join(root, "shim")
+	require.NoError(t, os.Mkdir(shim, 0o755))
+	symlinkOrSkip(t, filepath.Join("..", "vendor", "sub", "action.yml"), filepath.Join(shim, "action.yml"))
+	inRepo := filepath.Join(root, "alias")
+	require.NoError(t, os.Mkdir(inRepo, 0o755))
+	symlinkOrSkip(t, filepath.Join("..", "own", "action.yml"), filepath.Join(inRepo, "action.yml"))
+
+	got, err := DiscoverCompositeActionFiles(root)
+
+	require.NoError(t, err)
+	assert.NotContains(t, got, filepath.Join(shim, "action.yml"), "symlink into a nested checkout")
+	assert.Contains(t, got, filepath.Join(inRepo, "action.yml"), "symlink within this checkout")
+}
+
+// symlinkOrSkip skips on Windows, where creating a symlink needs a privilege
+// the user may lack (Developer Mode or admin). Elsewhere a failure is a bug in
+// the test setup.
+func symlinkOrSkip(t *testing.T, oldname, newname string) {
+	t.Helper()
+	err := os.Symlink(oldname, newname)
+	if err != nil && runtime.GOOS == "windows" {
+		t.Skipf("creating symlink: %v", err)
+	}
+	require.NoError(t, err)
+}
+
+// junction creates a Windows directory junction: unlike a symlink it needs no
+// privilege, and filepath.EvalSymlinks doesn't resolve it.
+func junction(t *testing.T, target, link string) {
+	t.Helper()
+	if runtime.GOOS != "windows" {
+		t.Skip("junctions are Windows-only")
+	}
+	out, err := exec.Command("cmd", "/c", "mklink", "/J", link, target).CombinedOutput()
+	require.NoError(t, err, string(out))
+}
+
 func TestDiscoverCompositeActionFiles_EmptyRoot(t *testing.T) {
 	got, err := DiscoverCompositeActionFiles("")
 	require.NoError(t, err)