Skip to content

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

Open
aksestok wants to merge 4 commits into
github:mainfrom
aksestok:dont-walk-worktrees-or-submodules
Open

aksestok wants to merge 4 commits into
github:mainfrom
aksestok:dont-walk-worktrees-or-submodules

Conversation

@aksestok

@aksestok aksestok commented Nov 2, 2026 •

Copy link
Copy Markdown

--migrate-local-actions swept action files in every checkout below the repo root, including nested worktrees (e.g. .gwt/<name>/), submodules and nested clones. It rewrote files that aren't part of this repository and could fail the whole run because of them.

  • The action-file sweep skips any subdirectory that is its own checkout, meaning it has a .git file (worktree, submodule) or a .git directory (nested clone).
  • A ./… ref that points into a nested checkout is no longer treated as a same-repo action, so it stays ./ instead of being rewritten to a $/… ref that can't reach it.
  • When GH_ACTIONS_LOCK_REPO_ROOT points at a directory without .git that holds the real checkout below it (as with actions/checkout path: src), nothing is skipped, so that checkout is still migrated.
  • The sweep reads each directory once and finds .git in that listing, so the check adds no extra syscall per directory.

Fixes #253

🤖 Generated with Claude Code

Copilot AI auto review requested due to automatic review settings November 2, 2026 07:44
@aksestok
aksestok requested a review from a team as a code owner November 2, 2026 07:44

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Symlink resolution gaps allow local actions to bypass nested-checkout safeguards, permitting incorrect $/ rewrites and unintended modifications inside nested checkouts.

2 open issues
What changed in this PR

This PR limits --migrate-local-actions to the current checkout, addressing unintended migration of files in nested worktrees, submodules, and clones described in #253.

Changes:

  • Skips nested checkouts during action-file discovery.
  • Leaves local references into nested checkouts unchanged.
  • Adds regression tests while preserving migration when an explicit repository root sits above the checkout.
File Description
internal/​workflowfile/​workflowfile.go Adds checkout detection and nested-checkout filtering during discovery.
internal/​workflowfile/​workflowfile_test.go Tests nested-checkout exclusion and root-handling edge cases.
internal/​workflowfile/​rewrite.go Excludes nested-checkout actions from local-reference migration.
cmd/​gh-actions-lock/​migrate_test.go Tests migration with an explicit root above the checkout.

Review effort: Balanced (auto)

Comment thread internal/workflowfile/rewrite.go Resolved
Comment thread internal/workflowfile/workflowfile.go Resolved
jessehouwing

This comment was marked as outdated.

aksestok and others added 2 commits November 5, 2026 08:36
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 <noreply@anthropic.com>
@aksestok

aksestok commented Nov 5, 2026 •

Copy link
Copy Markdown
Author

This needs a windows equivalent at least in terms of tests.

@jessehouwing I made a stab at Windows, but I don't have a machine to test on, and Windows test don't run as part of CI here. I don't think I should add Windows CI runners here either.

EDIT: I see you have something incoming in #259 🙏

@aksestok
aksestok requested a review from jessehouwing November 5, 2026 07:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

--migrate-local-actions rewrites action files in nested worktrees and submodules

3 participants