Skip to content

fix(plugin): skip sockets when copying repositories - #32637

Open
ruslan-shaydullin wants to merge 1 commit into
helm:mainfrom
ruslan-shaydullin:codex/fix-plugin-copy-sockets-current
Open

fix(plugin): skip sockets when copying repositories#32637
ruslan-shaydullin wants to merge 1 commit into
helm:mainfrom
ruslan-shaydullin:codex/fix-plugin-copy-sockets-current

Conversation

@ruslan-shaydullin

Copy link
Copy Markdown
Contributor

What this PR does / why we need it:

Git's FSMonitor daemon can leave a Unix-domain socket inside a plugin repository's .git directory. Copying that repository currently fails when CopyFile attempts to open the socket, which prevents plugin installation.

Skip directory entries marked os.ModeSocket during recursive CopyDir operations. Add a regression test with a real Unix-domain socket that verifies the regular file reaches the destination while the socket is omitted.

Fixes #12125.

Special notes for your reviewer:

The change preserves .git copying, the CopyDir API, and existing handling of regular files, directories and symlinks. It only filters socket entries.

Validation on Go 1.27.0:

  • Regression fails on the original main implementation and passes on this patch.
  • macOS: full make test-unit, filesystem/installer -race tests, make test-style (0 issues; golangci-lint 2.13.2), go mod tidy -diff, and CLI build pass.
  • Native Linux and Windows run: Linux filesystem/installer tests pass; the socket regression also passes on Windows Server 2025.
  • The broader Windows suite exposes existing failures: filesystem handle/symlink tests and installer tests using syscall.Umask. A baseline-versus-patch Windows comparison confirms identical failing test sets on main and this patch, while the new socket test and production package builds pass. The full Windows suite is not claimed green.

The native checks explicitly test contribution commit c2689c955; the validation-only workflow stays in the fork and is not part of this PR.

AI assistance: Codex assisted with implementation, review, testing and preparation of this description.

If applicable:

  • this PR contains user facing changes (the docs needed label should be applied if so)
  • this PR contains unit tests
  • this PR has been tested for backwards compatibility

Signed-off-by: Ruslan Shaydullin <shaydullin.r.d@outlook.com>
@pull-request-size pull-request-size Bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Sep 9, 2026
@github-actions github-actions Bot added the v4.x Issues and Pull Requests related to the major version v4 label Sep 9, 2026
@AbdulKreemShah2408

AbdulKreemShah2408 commented Sep 10, 2026

Copy link
Copy Markdown

Thanks @ruslan-shaydullin for the clear fix! Reviewed the change—skipping os.ModeSocket during CopyDir makes total sense to prevent FSMonitor socket errors. The regression test looks solid too.

@Karthik-Chowdary Karthik-Chowdary 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.

The narrow behavior is appropriate for #12125: a Unix-domain socket is live IPC state and cannot be meaningfully copied, while checking the DirEntry type before the existing directory/file paths preserves regular files, directories, and symlinks. The regression uses a real AF_UNIX listener and verifies both the positive copy and socket omission. I also checked the prior broader #12372 approach, CopyDir call sites/history, and special-file/symlink implications; I found no new traversal or dereference behavior here. Locally, go test -race ./internal/third_party/dep/fs ./internal/plugin/installer and 100 repetitions of the socket regression pass at c2689c9. Upstream unit/build, lint, CodeQL, and DCO checks are green for the same SHA.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/S Denotes a PR that changes 10-29 lines, ignoring generated files. v4.x Issues and Pull Requests related to the major version v4

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cannot add plugins with git fsmonitor daemon enabled

3 participants