Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Align worktree validation with upstream and remove build warnings #958

Merged
merged 2 commits into from
Dec 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions .github/workflows/git.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,14 @@ jobs:
GIT_DIST_PATH: .git-dist/${{ matrix.git[0] }}

steps:
- name: Checkout code
uses: actions/checkout@v4

- name: Install Go
uses: actions/setup-go@v4
with:
go-version: 1.21.x

- name: Checkout code
uses: actions/checkout@v4

- name: Install build dependencies
run: sudo apt-get update && sudo apt-get install gettext libcurl4-openssl-dev

Expand Down
6 changes: 3 additions & 3 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,13 @@ jobs:

runs-on: ${{ matrix.platform }}
steps:
- name: Checkout code
uses: actions/checkout@v4

- name: Install Go
uses: actions/setup-go@v4
with:
go-version: ${{ matrix.go-version }}

- name: Checkout code
uses: actions/checkout@v4

- name: Configure known hosts
if: matrix.platform != 'ubuntu-latest'
Expand Down
107 changes: 107 additions & 0 deletions worktree.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"io"
"os"
"path/filepath"
"runtime"
"strings"

"github.com/go-git/go-billy/v5"
Expand Down Expand Up @@ -394,6 +395,9 @@ func (w *Worktree) resetWorktree(t *object.Tree) error {
b := newIndexBuilder(idx)

for _, ch := range changes {
if err := w.validChange(ch); err != nil {
return err
}
if err := w.checkoutChange(ch, t, b); err != nil {
return err
}
Expand All @@ -403,6 +407,104 @@ func (w *Worktree) resetWorktree(t *object.Tree) error {
return w.r.Storer.SetIndex(idx)
}

// worktreeDeny is a list of paths that are not allowed
// to be used when resetting the worktree.
var worktreeDeny = map[string]struct{}{
// .git
GitDirName: {},

// For other historical reasons, file names that do not conform to the 8.3
// format (up to eight characters for the basename, three for the file
// extension, certain characters not allowed such as `+`, etc) are associated
// with a so-called "short name", at least on the `C:` drive by default.
// Which means that `git~1/` is a valid way to refer to `.git/`.
"git~1": {},
}

// validPath checks whether paths are valid.
// The rules around invalid paths could differ from upstream based on how
// filesystems are managed within go-git, but they are largely the same.
//
// For upstream rules:
// https://github.com/git/git/blob/564d0252ca632e0264ed670534a51d18a689ef5d/read-cache.c#L946
// https://github.com/git/git/blob/564d0252ca632e0264ed670534a51d18a689ef5d/path.c#L1383
func validPath(paths ...string) error {
for _, p := range paths {
parts := strings.FieldsFunc(p, func(r rune) bool { return (r == '\\' || r == '/') })
if _, denied := worktreeDeny[strings.ToLower(parts[0])]; denied {
return fmt.Errorf("invalid path prefix: %q", p)
}

if runtime.GOOS == "windows" {
// Volume names are not supported, in both formats: \\ and <DRIVE_LETTER>:.
if vol := filepath.VolumeName(p); vol != "" {
return fmt.Errorf("invalid path: %q", p)
}

if !windowsValidPath(parts[0]) {
return fmt.Errorf("invalid path: %q", p)
}
}

for _, part := range parts {
if part == ".." {
return fmt.Errorf("invalid path %q: cannot use '..'", p)
}
}
}
return nil
}

// windowsPathReplacer defines the chars that need to be replaced
// as part of windowsValidPath.
var windowsPathReplacer *strings.Replacer

func init() {
windowsPathReplacer = strings.NewReplacer(" ", "", ".", "")
}

func windowsValidPath(part string) bool {
if len(part) > 3 && strings.EqualFold(part[:4], GitDirName) {
// For historical reasons, file names that end in spaces or periods are
// automatically trimmed. Therefore, `.git . . ./` is a valid way to refer
// to `.git/`.
if windowsPathReplacer.Replace(part[4:]) == "" {
return false
}

// For yet other historical reasons, NTFS supports so-called "Alternate Data
// Streams", i.e. metadata associated with a given file, referred to via
// `<filename>:<stream-name>:<stream-type>`. There exists a default stream
// type for directories, allowing `.git/` to be accessed via
// `.git::$INDEX_ALLOCATION/`.
//
// For performance reasons, _all_ Alternate Data Streams of `.git/` are
// forbidden, not just `::$INDEX_ALLOCATION`.
if len(part) > 4 && part[4:5] == ":" {
return false
}
}
return true
}

func (w *Worktree) validChange(ch merkletrie.Change) error {
action, err := ch.Action()
if err != nil {
return nil
}

switch action {
case merkletrie.Delete:
return validPath(ch.From.String())
case merkletrie.Insert:
return validPath(ch.To.String())
case merkletrie.Modify:
return validPath(ch.From.String(), ch.To.String())
}

return nil
}

func (w *Worktree) checkoutChange(ch merkletrie.Change, t *object.Tree, idx *indexBuilder) error {
a, err := ch.Action()
if err != nil {
Expand Down Expand Up @@ -575,6 +677,11 @@ func (w *Worktree) checkoutFile(f *object.File) (err error) {
}

func (w *Worktree) checkoutFileSymlink(f *object.File) (err error) {
// https://github.com/git/git/commit/10ecfa76491e4923988337b2e2243b05376b40de
if strings.EqualFold(f.Name, gitmodulesFile) {
return ErrGitModulesSymlink
}

from, err := f.Reader()
if err != nil {
return
Expand Down
75 changes: 75 additions & 0 deletions worktree_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"
"context"
"errors"
"fmt"
"io"
"os"
"path/filepath"
Expand Down Expand Up @@ -2747,3 +2748,77 @@ func (s *WorktreeSuite) TestLinkedWorktree(c *C) {
c.Assert(err, Equals, ErrRepositoryIncomplete)
}
}

func TestValidPath(t *testing.T) {
type testcase struct {
path string
wantErr bool
}

tests := []testcase{
{".git", true},
{".git/b", true},
{".git\\b", true},
{"git~1", true},
{"a/../b", true},
{"a\\..\\b", true},
{".gitmodules", false},
{".gitignore", false},
{"a..b", false},
{".", false},
{"a/.git", false},
{"a\\.git", false},
{"a/.git/b", false},
{"a\\.git\\b", false},
}

if runtime.GOOS == "windows" {
tests = append(tests, []testcase{
{"\\\\a\\b", true},
{"C:\\a\\b", true},
{".git . . .", true},
{".git . . ", true},
{".git ", true},
{".git.", true},
{".git::$INDEX_ALLOCATION", true},
}...)
}

for _, tc := range tests {
t.Run(fmt.Sprintf("%s", tc.path), func(t *testing.T) {
err := validPath(tc.path)
if tc.wantErr {
assert.Error(t, err)
} else {
assert.NoError(t, err)
}
})
}
}

func TestWindowsValidPath(t *testing.T) {
tests := []struct {
path string
want bool
}{
{".git", false},
{".git . . .", false},
{".git ", false},
{".git ", false},
{".git . .", false},
{".git . .", false},
{".git::$INDEX_ALLOCATION", false},
{".git:", false},
{"a", true},
{"a\\b", true},
{"a/b", true},
{".gitm", true},
}

for _, tc := range tests {
t.Run(fmt.Sprintf("%s", tc.path), func(t *testing.T) {
got := windowsValidPath(tc.path)
assert.Equal(t, tc.want, got)
})
}
}