From 38bf3c2ead74ff2750dc314e1ee9f049e12f19bc Mon Sep 17 00:00:00 2001 From: Aksel Blekastad Date: Mon, 2 Nov 2026 08:29:42 +0100 Subject: [PATCH 1/3] Don't walk nested worktrees or submodules --- cmd/gh-actions-lock/migrate_test.go | 28 ++++++ internal/workflowfile/rewrite.go | 7 +- internal/workflowfile/workflowfile.go | 99 +++++++++++++++++----- internal/workflowfile/workflowfile_test.go | 68 +++++++++++++++ 4 files changed, 177 insertions(+), 25 deletions(-) diff --git a/cmd/gh-actions-lock/migrate_test.go b/cmd/gh-actions-lock/migrate_test.go index a6b7513e..ebbecf67 100644 --- a/cmd/gh-actions-lock/migrate_test.go +++ b/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 { diff --git a/internal/workflowfile/rewrite.go b/internal/workflowfile/rewrite.go index 016c62e9..ee09caa6 100644 --- a/internal/workflowfile/rewrite.go +++ b/internal/workflowfile/rewrite.go @@ -178,11 +178,14 @@ 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) - if !isWithinRoot(repoRoot, actionDir) { + if !isWithinRoot(repoRoot, actionDir) || inNestedCheckout(repoRoot, actionDir) { return false } for _, name := range []string{"action.yml", "action.yaml"} { diff --git a/internal/workflowfile/workflowfile.go b/internal/workflowfile/workflowfile.go index dd887678..26224b36 100644 --- a/internal/workflowfile/workflowfile.go +++ b/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,41 @@ 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) + w.paths = append(w.paths, p) } - return nil - }) - if err != nil { - return nil, fmt.Errorf("discovering action files under %s: %w", root, err) } - sort.Strings(paths) - return paths, 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 path lies in a separate checkout below +// root. As in DiscoverCompositeActionFiles, this only applies when root is +// itself a checkout. +func inNestedCheckout(root, path string) bool { + if !isCheckout(root) { + return false + } + rel, err := filepath.Rel(root, path) + if err != nil || !filepath.IsLocal(rel) { + return false + } + for dir := rel; dir != "."; dir = filepath.Dir(dir) { + if isCheckout(filepath.Join(root, dir)) { + return true + } + } + return false } // walkUses visits `uses:` values at the only places GitHub Actions honors @@ -539,7 +592,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) diff --git a/internal/workflowfile/workflowfile_test.go b/internal/workflowfile/workflowfile_test.go index a6343d92..4f468d60 100644 --- a/internal/workflowfile/workflowfile_test.go +++ b/internal/workflowfile/workflowfile_test.go @@ -4,6 +4,7 @@ import ( "errors" "os" "path/filepath" + "slices" "strings" "testing" @@ -642,6 +643,28 @@ func TestMigrateLocalActionsToSelfRepository(t *testing.T) { assert.Contains(t, string(out), "uses: ./missing-action") } +func TestMigrateLocalActionsToSelfRepository_NestedCheckout(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)) + + workflowPath := filepath.Join(repoRoot, ".github", "workflows", "ci.yml") + content := []byte("jobs:\n build:\n steps:\n - uses: ./vendor/sub/action\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 +736,51 @@ 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) { + require.NoError(t, os.Symlink("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_EmptyRoot(t *testing.T) { got, err := DiscoverCompositeActionFiles("") require.NoError(t, err) From 2c449bc6bb4221f5d06f12ea4d328b3366bf05cc Mon Sep 17 00:00:00 2001 From: Aksel Blekastad Date: Mon, 2 Nov 2026 09:00:16 +0100 Subject: [PATCH 2/3] Address review --- internal/workflowfile/rewrite.go | 5 +- internal/workflowfile/workflowfile.go | 32 ++++++-- internal/workflowfile/workflowfile_test.go | 86 ++++++++++++++++++---- 3 files changed, 98 insertions(+), 25 deletions(-) diff --git a/internal/workflowfile/rewrite.go b/internal/workflowfile/rewrite.go index ee09caa6..43fecf05 100644 --- a/internal/workflowfile/rewrite.go +++ b/internal/workflowfile/rewrite.go @@ -185,7 +185,7 @@ func (f *File) validateSelfRepositoryMigration(scan RefScan) error { func localActionExists(repoRoot, localPath string) bool { relPath := strings.TrimPrefix(localPath, "./") actionDir := filepath.Join(repoRoot, relPath) - if !isWithinRoot(repoRoot, actionDir) || inNestedCheckout(repoRoot, actionDir) { + if !isWithinRoot(repoRoot, actionDir) { return false } for _, name := range []string{"action.yml", "action.yaml"} { @@ -193,6 +193,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 diff --git a/internal/workflowfile/workflowfile.go b/internal/workflowfile/workflowfile.go index 26224b36..8606abe0 100644 --- a/internal/workflowfile/workflowfile.go +++ b/internal/workflowfile/workflowfile.go @@ -407,6 +407,11 @@ func (w *actionFileWalker) walk(dir string) error { if !info.Mode().IsRegular() { return fmt.Errorf("action definition %s is not a regular file", 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) } } @@ -425,19 +430,30 @@ func isCheckout(dir string) bool { return err == nil } -// inNestedCheckout reports whether path lies in a separate checkout below -// root. As in DiscoverCompositeActionFiles, this only applies when root is -// itself a checkout. -func inNestedCheckout(root, path string) bool { +// 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. +func inNestedCheckout(root, file string) bool { if !isCheckout(root) { return false } - rel, err := filepath.Rel(root, path) + resolvedRoot, err := filepath.EvalSymlinks(root) + if err != nil { + return true + } + resolvedFile, err := filepath.EvalSymlinks(file) + if err != nil { + return true + } + rel, err := filepath.Rel(resolvedRoot, resolvedFile) if err != nil || !filepath.IsLocal(rel) { - return false + return true } - for dir := rel; dir != "."; dir = filepath.Dir(dir) { - if isCheckout(filepath.Join(root, dir)) { + for dir := filepath.Dir(rel); dir != "."; dir = filepath.Dir(dir) { + if isCheckout(filepath.Join(resolvedRoot, dir)) { return true } } diff --git a/internal/workflowfile/workflowfile_test.go b/internal/workflowfile/workflowfile_test.go index 4f468d60..d16daf0e 100644 --- a/internal/workflowfile/workflowfile_test.go +++ b/internal/workflowfile/workflowfile_test.go @@ -644,25 +644,55 @@ func TestMigrateLocalActionsToSelfRepository(t *testing.T) { } func TestMigrateLocalActionsToSelfRepository_NestedCheckout(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)) + 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) { + require.NoError(t, os.Symlink(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)) + require.NoError(t, os.Symlink(filepath.Join("..", "vendor", "sub", "action", "action.yml"), filepath.Join(shim, "action.yml"))) + }, + }, + } + 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: ./vendor/sub/action\n") - require.NoError(t, os.WriteFile(workflowPath, content, 0o644)) - f, err := Load(workflowPath) - require.NoError(t, err) + 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() + 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) + 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) { @@ -781,6 +811,30 @@ func TestDiscoverCompositeActionFiles_NestedCheckouts(t *testing.T) { } } +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)) + require.NoError(t, os.Symlink(filepath.Join("..", "vendor", "sub", "action.yml"), filepath.Join(shim, "action.yml"))) + inRepo := filepath.Join(root, "alias") + require.NoError(t, os.Mkdir(inRepo, 0o755)) + require.NoError(t, os.Symlink(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") +} + func TestDiscoverCompositeActionFiles_EmptyRoot(t *testing.T) { got, err := DiscoverCompositeActionFiles("") require.NoError(t, err) From 833ac7fa5f9a254bea9c870f3d312815fe891439 Mon Sep 17 00:00:00 2001 From: Aksel Blekastad Date: Thu, 5 Nov 2026 08:35:48 +0100 Subject: [PATCH 3/3] Treat Windows junctions as nested checkouts filepath.EvalSymlinks doesn't resolve junctions, so a junction pointing into a submodule slipped past inNestedCheckout and the action was migrated to `$/`. Any non-directory on the resolved path now counts as nested. This also leaves a junction that stays inside the checkout unmigrated, which is the safe side. Tests that create symlinks now skip on Windows when the privilege is missing, and fail elsewhere as before. Co-Authored-By: Claude Opus 5.5 --- internal/workflowfile/workflowfile.go | 14 +++++++- internal/workflowfile/workflowfile_test.go | 42 +++++++++++++++++++--- 2 files changed, 50 insertions(+), 6 deletions(-) diff --git a/internal/workflowfile/workflowfile.go b/internal/workflowfile/workflowfile.go index 8606abe0..2c7b90eb 100644 --- a/internal/workflowfile/workflowfile.go +++ b/internal/workflowfile/workflowfile.go @@ -436,6 +436,11 @@ func isCheckout(dir string) bool { // 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 @@ -453,13 +458,20 @@ func inNestedCheckout(root, file string) bool { return true } for dir := filepath.Dir(rel); dir != "."; dir = filepath.Dir(dir) { - if isCheckout(filepath.Join(resolvedRoot, 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 // them: `jobs..uses`, `jobs..steps[*].uses` in a workflow, and // `runs.steps[*].uses` in a composite action definition. A `uses` key diff --git a/internal/workflowfile/workflowfile_test.go b/internal/workflowfile/workflowfile_test.go index d16daf0e..e1bd8d80 100644 --- a/internal/workflowfile/workflowfile_test.go +++ b/internal/workflowfile/workflowfile_test.go @@ -3,7 +3,9 @@ package workflowfile import ( "errors" "os" + "os/exec" "path/filepath" + "runtime" "slices" "strings" "testing" @@ -654,7 +656,7 @@ func TestMigrateLocalActionsToSelfRepository_NestedCheckout(t *testing.T) { name: "leaves symlinked dir pointing into submodule", uses: "./alias", setup: func(t *testing.T, repoRoot string) { - require.NoError(t, os.Symlink(filepath.Join("vendor", "sub", "action"), filepath.Join(repoRoot, "alias"))) + symlinkOrSkip(t, filepath.Join("vendor", "sub", "action"), filepath.Join(repoRoot, "alias")) }, }, { @@ -663,7 +665,14 @@ func TestMigrateLocalActionsToSelfRepository_NestedCheckout(t *testing.T) { setup: func(t *testing.T, repoRoot string) { shim := filepath.Join(repoRoot, "shim") require.NoError(t, os.Mkdir(shim, 0o755)) - require.NoError(t, os.Symlink(filepath.Join("..", "vendor", "sub", "action", "action.yml"), filepath.Join(shim, "action.yml"))) + 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")) }, }, } @@ -774,7 +783,7 @@ func TestDiscoverCompositeActionFiles_NestedCheckouts(t *testing.T) { require.NoError(t, os.Mkdir(filepath.Join(dir, ".git"), 0o755)) } danglingGit := func(t *testing.T, dir string) { - require.NoError(t, os.Symlink("missing", filepath.Join(dir, ".git"))) + symlinkOrSkip(t, "missing", filepath.Join(dir, ".git")) } tests := []struct { name string @@ -823,10 +832,10 @@ func TestDiscoverCompositeActionFiles_SymlinkIntoNestedCheckout(t *testing.T) { 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)) - require.NoError(t, os.Symlink(filepath.Join("..", "vendor", "sub", "action.yml"), filepath.Join(shim, "action.yml"))) + 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)) - require.NoError(t, os.Symlink(filepath.Join("..", "own", "action.yml"), filepath.Join(inRepo, "action.yml"))) + symlinkOrSkip(t, filepath.Join("..", "own", "action.yml"), filepath.Join(inRepo, "action.yml")) got, err := DiscoverCompositeActionFiles(root) @@ -835,6 +844,29 @@ func TestDiscoverCompositeActionFiles_SymlinkIntoNestedCheckout(t *testing.T) { 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)