From 09c27bce49b4e728a7c1befd701c589a687e8e02 Mon Sep 17 00:00:00 2001 From: "red-hat-konflux[bot]" <126015336+red-hat-konflux[bot]@users.noreply.github.com> Date: Sun, 2 Aug 2026 22:17:50 +0000 Subject: [PATCH] Update module github.com/go-git/go-git/v5 to v5.19.2 Signed-off-by: red-hat-konflux <126015336+red-hat-konflux[bot]@users.noreply.github.com> --- go.mod | 2 +- go.sum | 4 +- vendor/github.com/go-git/go-git/v5/.gitignore | 1 + .../go-git/go-git/v5/plumbing/reference.go | 42 +++++++ .../v5/storage/filesystem/dotgit/dotgit.go | 57 +++++++++ .../github.com/go-git/go-git/v5/worktree.go | 67 ++++++++++ .../go-git/go-git/v5/worktree_fs.go | 115 +++++++++++++++--- .../go-git/go-git/v5/worktree_status.go | 2 +- vendor/modules.txt | 2 +- 9 files changed, 272 insertions(+), 20 deletions(-) diff --git a/go.mod b/go.mod index 7ed1f1566..8fc07d2e3 100644 --- a/go.mod +++ b/go.mod @@ -8,7 +8,7 @@ require ( github.com/argoproj-labs/argocd-operator v0.17.0 github.com/bradleyfalzon/ghinstallation/v2 v2.19.0 github.com/go-errors/errors v1.5.1 - github.com/go-git/go-git/v5 v5.19.1 + github.com/go-git/go-git/v5 v5.19.2 github.com/go-logr/logr v1.4.4 github.com/google/uuid v1.6.1-0.20241114170450-2d3c2a9cc518 github.com/onsi/ginkgo/v2 v2.32.0 diff --git a/go.sum b/go.sum index 3d1cf1e54..ad10725f2 100644 --- a/go.sum +++ b/go.sum @@ -151,8 +151,8 @@ github.com/go-git/go-billy/v5 v5.9.1 h1:8U73XiOTfINdItHVa6z4Gv7ToObcZ6grkqQbLryL github.com/go-git/go-billy/v5 v5.9.1/go.mod h1:ExsU+jcGwXTBOnyilvAnEM1wug1IxHr4yP2ZXsNRtV0= github.com/go-git/go-git-fixtures/v4 v4.3.2-0.20231010084843-55a94097c399 h1:eMje31YglSBqCdIqdhKBW8lokaMrL3uTkpGYlE2OOT4= github.com/go-git/go-git-fixtures/v4 v4.3.2-0.20231010084843-55a94097c399/go.mod h1:1OCfN199q1Jm3HZlxleg+Dw/mwps2Wbk9frAWm+4FII= -github.com/go-git/go-git/v5 v5.19.1 h1:nX27AnaU43/K5bKktKwgBmR9lawoYVe1Ckg0rgzzN00= -github.com/go-git/go-git/v5 v5.19.1/go.mod h1:Pb1v0c7/g8aGQJwx9Us09W85yGoyvSwuhEGMH7zjDKQ= +github.com/go-git/go-git/v5 v5.19.2 h1:wkfn7vOlUBu8ivAWKBWisTiwJK4jYHzTF8Ndv1LyGqY= +github.com/go-git/go-git/v5 v5.19.2/go.mod h1:QqCBE1EFN5ddFmrliLQ3/ntRCUjZU3EJuwuB/jWEHjk= github.com/go-jose/go-jose/v4 v4.1.4 h1:moDMcTHmvE6Groj34emNPLs/qtYXRVcd6S7NHbHz3kA= github.com/go-jose/go-jose/v4 v4.1.4/go.mod h1:x4oUasVrzR7071A4TnHLGSPpNOm2a21K9Kf04k1rs08= github.com/go-logr/logr v1.2.3/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= diff --git a/vendor/github.com/go-git/go-git/v5/.gitignore b/vendor/github.com/go-git/go-git/v5/.gitignore index b7f2c5807..62dae67e8 100644 --- a/vendor/github.com/go-git/go-git/v5/.gitignore +++ b/vendor/github.com/go-git/go-git/v5/.gitignore @@ -5,3 +5,4 @@ profile.out .tmp/ .git-dist/ .vscode +build/tools/ diff --git a/vendor/github.com/go-git/go-git/v5/plumbing/reference.go b/vendor/github.com/go-git/go-git/v5/plumbing/reference.go index 4daa34164..9d34135b2 100644 --- a/vendor/github.com/go-git/go-git/v5/plumbing/reference.go +++ b/vendor/github.com/go-git/go-git/v5/plumbing/reference.go @@ -110,6 +110,48 @@ func (r ReferenceName) IsTag() bool { return strings.HasPrefix(string(r), refTagPrefix) } +// IsSafe reports whether the reference name can be safely turned into a path +// under the .git directory, mirroring Git's refname_is_safe (refs.c). A name +// is safe when it is either: +// +// - under "refs/", non-empty after the prefix, containing no backslash and +// no empty, "." or ".." path component (so it cannot escape the refs/ +// sub-tree, or alias another name, once turned into a path); or +// - a one-level pseudo-ref whose spelling is restricted to [A-Z_] +// (e.g. HEAD, ORIG_HEAD, FETCH_HEAD). +// +// Everything else — a lowercase or mixed one-level name such as "config" or +// "index", an absolute or drive-prefixed name, or a refs/ name that escapes — +// is unsafe, because it could resolve onto unrelated repository metadata. +func (r ReferenceName) IsSafe() bool { + s := string(r) + if s == "" { + return false + } + + if rest, ok := strings.CutPrefix(s, refPrefix); ok { + // '\' is a path separator on Windows, so a refs/ name containing one + // could escape the sub-tree or alias another name once turned into a + // path; reject it outright (check_refname_format forbids '\' too). + if rest == "" || strings.Contains(rest, "\\") { + return false + } + for part := range strings.SplitSeq(rest, "/") { + if part == "" || part == "." || part == ".." { + return false + } + } + return true + } + + for i := 0; i < len(s); i++ { + if (s[i] < 'A' || s[i] > 'Z') && s[i] != '_' { + return false + } + } + return true +} + func (r ReferenceName) String() string { return string(r) } diff --git a/vendor/github.com/go-git/go-git/v5/storage/filesystem/dotgit/dotgit.go b/vendor/github.com/go-git/go-git/v5/storage/filesystem/dotgit/dotgit.go index eb85a1145..d57a6dc82 100644 --- a/vendor/github.com/go-git/go-git/v5/storage/filesystem/dotgit/dotgit.go +++ b/vendor/github.com/go-git/go-git/v5/storage/filesystem/dotgit/dotgit.go @@ -16,6 +16,7 @@ import ( "strings" "time" + "github.com/go-git/go-git/v5/internal/pathutil" "github.com/go-git/go-git/v5/plumbing" "github.com/go-git/go-git/v5/plumbing/hash" "github.com/go-git/go-git/v5/storage" @@ -79,8 +80,52 @@ var ( // resolve outside the modules/ subtree, mirroring canonical Git's // "ignoring suspicious submodule name" defence. ErrModuleNameEscape = errors.New("submodule name escapes modules/ directory") + // ErrReferenceNameEscape is returned when a reference name would + // resolve outside its reference sub-tree once turned into a path + // under the .git directory (e.g. a name with a ".." component). + ErrReferenceNameEscape = errors.New("reference name escapes the reference storage") ) +// isPathSep reports whether r is a path separator in reference names. +// It treats both '/' and '\\' as separators to harden against cross-OS paths. +func isPathSep(r rune) bool { return r == '/' || r == '\\' } + +// validReferenceName rejects reference names that cannot be safely turned into +// a path under the .git directory. A loose reference is stored verbatim at +// ".git/", so a crafted name — for instance one advertised by a malicious +// remote — could climb out of its reference sub-tree and read, overwrite, or +// delete unrelated metadata such as .git/config. +// +// The storage-safety gate is plumbing.ReferenceName.IsSafe, mirroring Git's +// refname_is_safe: a name must be under refs/ without escaping it, or be a +// [A-Z_] pseudo-ref. This alone rejects absolute, drive-prefixed, escaping and +// single-level metadata names. On top of it, this adds filesystem-specific +// hardening that IsSafe's literal check does not cover: control characters, and +// components a case-insensitive/NTFS/HFS+ filesystem would fold back to "." or +// ".." (trailing dots/spaces, Alternate Data Streams, ignorable Unicode code +// points), delegated to pathutil.IsHFSDot and pathutil.IsNTFSDot with "." as +// the needle — as validSubmoduleName does — and run regardless of host OS. +func validReferenceName(name plumbing.ReferenceName) error { + if !name.IsSafe() { + return fmt.Errorf("%w: %q is not under refs/ nor a valid pseudo-ref", ErrReferenceNameEscape, string(name)) + } + + s := string(name) + for i := 0; i < len(s); i++ { + if s[i] < 0x20 || s[i] == 0x7f { + return fmt.Errorf("%w: %q", ErrReferenceNameEscape, s) + } + } + for _, part := range strings.FieldsFunc(s, isPathSep) { + // IsNTFSDot/IsHFSDot with a "." needle match ".." and its disguises + // but not a bare ".", so reject that component explicitly too. + if part == "." || pathutil.IsHFSDot(part, ".") || pathutil.IsNTFSDot(part, ".", "") { + return fmt.Errorf("%w: %q", ErrReferenceNameEscape, s) + } + } + return nil +} + // Options holds configuration for the storage. type Options struct { // ExclusiveAccess means that the filesystem is not modified externally @@ -706,6 +751,10 @@ func (d *DotGit) checkReferenceAndTruncate(f billy.File, old *plumbing.Reference } func (d *DotGit) SetRef(r, old *plumbing.Reference) error { + if err := validReferenceName(r.Name()); err != nil { + return err + } + var content string switch r.Type() { case plumbing.SymbolicReference: @@ -741,6 +790,10 @@ func (d *DotGit) Refs() ([]*plumbing.Reference, error) { // Ref returns the reference for a given reference name. func (d *DotGit) Ref(name plumbing.ReferenceName) (*plumbing.Reference, error) { + if err := validReferenceName(name); err != nil { + return nil, err + } + ref, err := d.readReferenceFile(".", name.String()) if err == nil { return ref, nil @@ -804,6 +857,10 @@ func (d *DotGit) packedRef(name plumbing.ReferenceName) (*plumbing.Reference, er // RemoveRef removes a reference by name. func (d *DotGit) RemoveRef(name plumbing.ReferenceName) error { + if err := validReferenceName(name); err != nil { + return err + } + path := d.fs.Join(".", name.String()) _, err := d.fs.Stat(path) if err == nil { diff --git a/vendor/github.com/go-git/go-git/v5/worktree.go b/vendor/github.com/go-git/go-git/v5/worktree.go index d8ee9fdd1..b935c6b78 100644 --- a/vendor/github.com/go-git/go-git/v5/worktree.go +++ b/vendor/github.com/go-git/go-git/v5/worktree.go @@ -583,6 +583,10 @@ func (w *Worktree) checkoutChangeSubmodule(name string, return err } + if err := w.clearBlockingSymlinks(name); err != nil { + return err + } + if err := w.Filesystem.MkdirAll(name, mode); err != nil { return err } @@ -626,7 +630,70 @@ func (w *Worktree) checkoutChangeRegularFile(name string, return nil } +// clearBlockingSymlinks removes a symlink that is in the way of +// materialising name, so the checkout writes a real entry in its place +// instead of following the link out of the worktree. Two cases: +// +// - a leading directory component that is a symlink (e.g. "s" while +// writing "s/config", where "s" links to ".git"): OpenFile/MkdirAll +// would traverse it, so the write would land under the link's target. +// - the final component itself being a symlink (e.g. writing "s" while +// "s" links to ".git/config"): OpenFile with O_TRUNC, or Symlink, +// would follow/replace through it and clobber the target. +// +// A symlink can never be a legitimate parent of, or the destination for, +// a tracked entry, so removing it is always correct. This mirrors upstream +// Git's forced checkout, which unlinks a blocking symlink in the leading +// path (create_directories) and unlinks an existing entry before +// write_entry. +// https://github.com/git/git/blob/v2.54.0/entry.c#L50 +func (w *Worktree) clearBlockingSymlinks(name string) error { + var dirs []string + for dir := filepath.Dir(name); dir != "." && dir != "" && dir != string(filepath.Separator); dir = filepath.Dir(dir) { + dirs = append(dirs, dir) + } + // Leading components, shallowest-first: removing the shallowest symlink + // invalidates every component beneath it, so a single removal is enough. + for i := len(dirs) - 1; i >= 0; i-- { + fi, err := w.Filesystem.Lstat(dirs[i]) + if err != nil { + // A missing component is created as a real directory by the + // checkout. Any other error means we cannot tell whether it is + // a symlink, so surface it instead of leaving a blocking link in + // place and failing later in a harder-to-diagnose way. + if os.IsNotExist(err) { + continue + } + return err + } + if fi.Mode()&os.ModeSymlink != 0 { + return w.Filesystem.Remove(dirs[i]) + } + } + // Final component: an existing symlink here would be followed by the + // subsequent OpenFile/Symlink/MkdirAll, so replace it. + fi, err := w.Filesystem.Lstat(name) + if err != nil { + if os.IsNotExist(err) { + return nil + } + return err + } + if fi.Mode()&os.ModeSymlink != 0 { + return w.Filesystem.Remove(name) + } + return nil +} + func (w *Worktree) checkoutFile(f *object.File) (err error) { + // checkoutFile is the materialisation boundary for tracked entries. + // Remove any blocking symlink first so the subsequent OpenFile or + // Symlink call writes the entry itself instead of following a planted + // final-component link in the underlying filesystem. + if err := w.clearBlockingSymlinks(f.Name); err != nil { + return err + } + mode, err := f.Mode.ToOSFileMode() if err != nil { return diff --git a/vendor/github.com/go-git/go-git/v5/worktree_fs.go b/vendor/github.com/go-git/go-git/v5/worktree_fs.go index 9bc2fd97d..cf16de746 100644 --- a/vendor/github.com/go-git/go-git/v5/worktree_fs.go +++ b/vendor/github.com/go-git/go-git/v5/worktree_fs.go @@ -35,10 +35,25 @@ func defaultProtectNTFS() bool { return true } -// worktreeFilesystem wraps a billy.Filesystem and validates every path passed -// to a mutating operation. This prevents writing to, or deleting from, -// dangerous locations (e.g. .git/*, ../) regardless of which worktree -// code path triggers the operation. +// worktreeFilesystem wraps a billy.Filesystem and validates every path it +// is handed, so worktree operations cannot use dangerous paths at the +// boundary. Two layers apply: +// +// - validPath rejects dangerous path *strings*: .git and its HFS+/NTFS +// variants, "..", control characters, volume names. +// - validNoLeadingSymlink rejects paths whose leading directories +// already exist on disk as symlinks, so a write or delete cannot +// follow a planted link out of the tree. +// +// Both layers run on every mutating operation (validWritePath) and every +// read (validReadPath). Chroot additionally refuses a symlink as the final +// component, so a sub-filesystem such as a submodule worktree cannot be +// scoped to a redirected target. +// +// The wrapper intentionally stops at leading-component traversal. Callers +// that need final-component no-follow semantics for materialisation +// (checkoutFile) enforce that directly by removing the blocking symlink +// before opening the destination path. type worktreeFilesystem struct { billy.Filesystem protectNTFS bool @@ -50,7 +65,7 @@ func newWorktreeFilesystem(fs billy.Filesystem, protectNTFS, protectHFS bool) *w } func (sfs *worktreeFilesystem) Create(filename string) (billy.File, error) { - if err := sfs.validPath(filename); err != nil { + if err := sfs.validWritePath(filename); err != nil { return nil, fmt.Errorf("create: %w", err) } return sfs.Filesystem.Create(filename) @@ -64,7 +79,7 @@ func (sfs *worktreeFilesystem) Open(filename string) (billy.File, error) { } func (sfs *worktreeFilesystem) OpenFile(filename string, flag int, perm os.FileMode) (billy.File, error) { - if err := sfs.validPath(filename); err != nil { + if err := sfs.validWritePath(filename); err != nil { return nil, fmt.Errorf("openfile: %w", err) } return sfs.Filesystem.OpenFile(filename, flag, perm) @@ -78,14 +93,14 @@ func (sfs *worktreeFilesystem) Stat(filename string) (os.FileInfo, error) { } func (sfs *worktreeFilesystem) Remove(filename string) error { - if err := sfs.validPath(filename); err != nil { + if err := sfs.validWritePath(filename); err != nil { return fmt.Errorf("remove: %w", err) } return sfs.Filesystem.Remove(filename) } func (sfs *worktreeFilesystem) Rename(from, to string) error { - if err := sfs.validPath(from, to); err != nil { + if err := sfs.validWritePath(from, to); err != nil { return fmt.Errorf("rename: %w", err) } return sfs.Filesystem.Rename(from, to) @@ -106,7 +121,7 @@ func (sfs *worktreeFilesystem) Lstat(filename string) (os.FileInfo, error) { } func (sfs *worktreeFilesystem) Symlink(target, link string) error { - if err := sfs.validPath(link); err != nil { + if err := sfs.validWritePath(link); err != nil { return fmt.Errorf("symlink: %w", err) } if err := sfs.validSymlinkName(link); err != nil { @@ -131,7 +146,7 @@ func (sfs *worktreeFilesystem) MkdirAll(path string, perm os.FileMode) error { if path == "" || path == "." || path == "/" { return nil } - if err := sfs.validPath(path); err != nil { + if err := sfs.validWritePath(path); err != nil { return fmt.Errorf("mkdirall: %w", err) } return sfs.Filesystem.MkdirAll(path, perm) @@ -145,18 +160,39 @@ func (sfs *worktreeFilesystem) Chroot(path string) (billy.Filesystem, error) { if err := sfs.validReadPath(path); err != nil { return nil, fmt.Errorf("chroot: %w", err) } + // Chroot scopes a sub-filesystem to path, so the final component must + // be a real directory too: a symlink there would silently redirect the + // scope (e.g. a submodule worktree) to a target outside the tree. This + // is the "valid path, wrong target" case that validNoLeadingSymlink, + // which only inspects leading components, does not cover. + // + // A non-existent target is fine: Chroot creates it as a real + // directory. Any other Lstat error means we cannot prove the target + // is not a symlink, so fail closed rather than scope through it. + if fi, err := sfs.Filesystem.Lstat(path); err != nil { + if !os.IsNotExist(err) { + return nil, fmt.Errorf("chroot: cannot stat %q: %w", path, err) + } + } else if fi.Mode()&os.ModeSymlink != 0 { + return nil, fmt.Errorf("chroot: invalid path %q: is a symlink", path) + } return sfs.Filesystem.Chroot(path) } -// validReadPath is like validPath but treats the empty string and "." as -// valid references to the worktree root. Read-side operations on the root -// (e.g. ReadDir(""), Lstat(".")) are legitimate; mutating the root itself -// is not, so write-side operations continue to use validPath directly. +// validReadPath is like validWritePath but treats the empty string and "." +// as valid references to the worktree root. Read-side operations on the +// root (e.g. ReadDir(""), Lstat(".")) are legitimate. Mutating the root +// itself is not, so write-side operations reject it via validPath. Reads +// are still refused through a leading symlink, so the wrapper never +// follows a planted link even on the read surface. func (sfs *worktreeFilesystem) validReadPath(p string) error { if p == "" || p == "." || p == "/" { return nil } - return sfs.validPath(p) + if err := sfs.validPath(p); err != nil { + return err + } + return sfs.validNoLeadingSymlink(p) } var errUnsupportedOperation = errors.New("unsupported operation") @@ -232,6 +268,55 @@ func (sfs *worktreeFilesystem) validPath(paths ...string) error { return nil } +// validWritePath validates paths for mutating operations. It layers the +// filesystem-state check validNoLeadingSymlink on top of the string-only +// checks in validPath, so a write can neither name a dangerous path nor +// reach one by traversing an existing symlink. Every mutating method on +// the wrapper funnels through here, so the leading-symlink invariant holds +// for all worktree writers without each call site having to remember it. +func (sfs *worktreeFilesystem) validWritePath(paths ...string) error { + if err := sfs.validPath(paths...); err != nil { + return err + } + return sfs.validNoLeadingSymlink(paths...) +} + +// validNoLeadingSymlink rejects paths whose leading directory components +// resolve through a symlink that already exists on the underlying +// filesystem. validPath guards the path string. This guards the on-disk +// state, so a write or delete cannot reach outside the worktree by +// traversing a symlink that a tree or an earlier step left in place. +// +// This is the fail-closed backstop for the whole class. Callers that want +// upstream's replace-and-continue behaviour (checkout) remove the blocking +// symlink first via clearBlockingSymlinks, so no symlink remains when the +// write reaches the wrapper. Callers that do not get a safe error, +// matching upstream Git refusing rather than following the link. See +// has_symlink_leading_path (symlinks.c) and the check_leading_path guard +// in unlink_entry (entry.c). +func (sfs *worktreeFilesystem) validNoLeadingSymlink(paths ...string) error { + for _, p := range paths { + for dir := filepath.Dir(p); dir != "." && dir != "" && dir != string(filepath.Separator); dir = filepath.Dir(dir) { + fi, err := sfs.Filesystem.Lstat(dir) + if err != nil { + // A missing ancestor is materialised as a real directory, + // so it cannot be a symlink and is safe to skip. Any other + // error (permission, I/O) means we cannot prove the + // component is not a symlink, so fail closed rather than + // let the operation traverse an unverified component. + if os.IsNotExist(err) { + continue + } + return fmt.Errorf("invalid path %q: cannot stat leading component %q: %w", p, dir, err) + } + if fi.Mode()&os.ModeSymlink != 0 { + return fmt.Errorf("invalid path %q: leading component %q is a symlink", p, dir) + } + } + } + return nil +} + // validSymlinkName checks the per-component name of a symlink for // dotfile names that attackers can use to trick a checkout into // writing a dangerous symlink. Each path component is compared diff --git a/vendor/github.com/go-git/go-git/v5/worktree_status.go b/vendor/github.com/go-git/go-git/v5/worktree_status.go index ecc3d7ab8..8a28231d1 100644 --- a/vendor/github.com/go-git/go-git/v5/worktree_status.go +++ b/vendor/github.com/go-git/go-git/v5/worktree_status.go @@ -371,7 +371,7 @@ func (w *Worktree) doAdd(path string, ignorePattern []gitignore.Pattern, skipSta } } - path = filepath.Clean(path) + path = filepath.ToSlash(filepath.Clean(path)) if err != nil || !fi.IsDir() { added, h, err = w.doAddFile(idx, s, path, ignorePattern) diff --git a/vendor/modules.txt b/vendor/modules.txt index d185fc385..14b658064 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -332,7 +332,7 @@ github.com/go-git/go-billy/v5/helper/polyfill github.com/go-git/go-billy/v5/memfs github.com/go-git/go-billy/v5/osfs github.com/go-git/go-billy/v5/util -# github.com/go-git/go-git/v5 v5.19.1 +# github.com/go-git/go-git/v5 v5.19.2 ## explicit; go 1.25.0 github.com/go-git/go-git/v5 github.com/go-git/go-git/v5/config