Add RemoveAll to Basic Interface? #77
Comments
|
Hey @cipherboy! The However, a native As for the |
|
\o hey @pjbgf -- regarding the last point, I ran across this when working across multiple repos: go-git/go-git#1155 (comment) It seems this only works if I think the issue with this is it's unsafe for sandbox execution/bypass; you don't want to necessarily allow or follow symlinks to arbitrary places on disk if you're a git repo provider. But if you're using known repositories it probably isn't as much an issue. Let me see if I can create a better reproducer. |
Signed-off-by: Alexander Scheel <[email protected]>
|
@pjbgf Hmm, looks like it was simpler than I thought. Check out this code: https://github.com/cipherboy/testbed/tree/master/go-git-repo-symlink Script and codeScript: #!/bin/bash
create_repo() {(
rm -rf repos/ repos.bak/
mkdir -p repos/example
cd repos/example
git init .
git status
echo "initial" > README.md
git add README.md && git commit -m "initial"
git status
echo "lib symlink" > README.md
mkdir lib64
echo "whoami" > lib64/code.sh
ln -s lib64 lib
git add lib64 lib README.md && git commit -m "lib symlink"
git status
echo "absolute symlink" > README.md
ln -s /bin/bash lib64/shell
git add lib64/shell README.md && git commit -m "absolute symlink"
git status
echo "latest change" > README.md
git add README.md && git commit -m "latest change"
git log | cat -
)}
backup_repo() {(
cp -pr repos/ repos.bak
)}
create_repo
backup_repo
go mod tidy
# go run ./main.go
(
cd repos/example && go run ../../main.go .
)Code: package main
import (
"fmt"
"os"
"github.com/go-git/go-billy/v5/osfs"
"github.com/go-git/go-git/v5"
"github.com/go-git/go-git/v5/plumbing/cache"
"github.com/go-git/go-git/v5/plumbing/object"
"github.com/go-git/go-git/v5/storage/filesystem"
)
func getRepo(dir string) (*git.Repository, error) {
// See https://github.com/go-git/go-git/issues/1155#issuecomment-2242778023
wt := osfs.New(dir, osfs.WithBoundOS())
dotfs, err := wt.Chroot(git.GitDirName)
if err != nil {
return nil, err
}
store := filesystem.NewStorage(dotfs, cache.NewObjectLRUDefault())
// Clone -> Open
return git.Open(store, wt)
}
func walkCommits(repo *git.Repository) error {
iter, err := repo.CommitObjects()
if err != nil {
return fmt.Errorf("failed getting commit objects: %v", err)
}
repoW, err := repo.Worktree()
if err != nil {
return fmt.Errorf("failed to get repo worktree: %v", err)
}
if err := iter.ForEach(func(commit *object.Commit) error {
if err := repoW.Checkout(&git.CheckoutOptions{Hash: commit.Hash, Force: true}); err != nil {
return fmt.Errorf("failed to checkout commit (%#v): %v", commit.Hash.String(), err)
}
return nil
}); err != nil {
return fmt.Errorf("failed iterating over commit object: %v", err)
}
return nil
}
func main() {
dir := "repos/example"
if len(os.Args) > 1 {
dir = os.Args[1]
}
fmt.Printf("using dir=%v\n", dir)
repo, err := getRepo(dir)
if err != nil {
panic(fmt.Sprintf("failed getting repository: %v", err))
}
if err := walkCommits(repo); err != nil {
panic(fmt.Sprintf("failed walking commits: %v", err))
}
}In particular, the current mode ( or similar depending on what file gets executed first. But the other one has the error I remember: Both of these hashes are (the freshly generated) repo's initial commit. With my hacky filesystem, both methods pass. The implementation essentially looks like this: Implementation//go:build !js
// +build !js
package osfs
import (
"os"
"path/filepath"
"strings"
securejoin "github.com/cyphar/filepath-securejoin"
"github.com/go-git/go-billy/v5"
)
// WorkingDirOS is a legacyfilesystem without a chroot of the OS filesystem,
// but instead treats relative directories as if they were specific to a
// given underlying path, even if CWD changes.
type WorkingDirOS struct {
baseDir string
}
func newWorkingDirOS(baseDir string) billy.Filesystem {
return &WorkingDirOS{baseDir}
}
func (fs *WorkingDirOS) Chroot(path string) (billy.Filesystem, error) {
joined, err := securejoin.SecureJoin(fs.baseDir, path)
if err != nil {
return nil, err
}
return New(joined), nil
}
func (fs *WorkingDirOS) Root() string {
return fs.baseDir
}
func (fs *WorkingDirOS) Create(filename string) (billy.File, error) {
if !filepath.IsAbs(filename) && !strings.HasPrefix(filename, fs.baseDir) {
filename = filepath.Join(fs.baseDir, filename)
}
return fs.OpenFile(filename, os.O_RDWR|os.O_CREATE|os.O_TRUNC, defaultCreateMode)
}
func (fs *WorkingDirOS) OpenFile(filename string, flag int, perm os.FileMode) (billy.File, error) {
if !filepath.IsAbs(filename) && !strings.HasPrefix(filename, fs.baseDir) {
filename = filepath.Join(fs.baseDir, filename)
}
return openFile(filename, flag, perm, fs.createDir)
}
func (fs *WorkingDirOS) createDir(fullpath string) error {
if !filepath.IsAbs(fullpath) && !strings.HasPrefix(fullpath, fs.baseDir) {
fullpath = filepath.Join(fs.baseDir, fullpath)
}
dir := filepath.Dir(fullpath)
if dir != "." {
if err := os.MkdirAll(dir, defaultDirectoryMode); err != nil {
return err
}
}
return nil
}
func (fs *WorkingDirOS) ReadDir(dir string) ([]os.FileInfo, error) {
if !filepath.IsAbs(dir) && !strings.HasPrefix(dir, fs.baseDir) {
dir = filepath.Join(fs.baseDir, dir)
}
return readDir(dir)
}
func (fs *WorkingDirOS) Rename(from, to string) error {
if !filepath.IsAbs(from) && !strings.HasPrefix(from, fs.baseDir) {
from = filepath.Join(fs.baseDir, from)
}
if err := fs.createDir(to); err != nil {
return err
}
if !filepath.IsAbs(to) && !strings.HasPrefix(to, fs.baseDir) {
to = filepath.Join(fs.baseDir, to)
}
return rename(from, to)
}
func (fs *WorkingDirOS) MkdirAll(path string, perm os.FileMode) error {
if !filepath.IsAbs(path) && !strings.HasPrefix(path, fs.baseDir) {
path = filepath.Join(fs.baseDir, path)
}
return os.MkdirAll(path, defaultDirectoryMode)
}
func (fs *WorkingDirOS) Open(filename string) (billy.File, error) {
return fs.OpenFile(filename, os.O_RDONLY, 0)
}
func (fs *WorkingDirOS) Stat(filename string) (os.FileInfo, error) {
if !filepath.IsAbs(filename) && !strings.HasPrefix(filename, fs.baseDir) {
filename = filepath.Join(fs.baseDir, filename)
}
return os.Stat(filename)
}
func (fs *WorkingDirOS) Remove(filename string) error {
if !filepath.IsAbs(filename) && !strings.HasPrefix(filename, fs.baseDir) {
filename = filepath.Join(fs.baseDir, filename)
}
return os.Remove(filename)
}
func (fs *WorkingDirOS) TempFile(dir, prefix string) (billy.File, error) {
if err := fs.createDir(dir + string(os.PathSeparator)); err != nil {
return nil, err
}
return tempFile(dir, prefix)
}
func (fs *WorkingDirOS) Join(elem ...string) string {
return filepath.Join(elem...)
}
func (fs *WorkingDirOS) RemoveAll(path string) error {
if !filepath.IsAbs(path) && !strings.HasPrefix(path, fs.baseDir) {
path = filepath.Join(fs.baseDir, path)
}
return os.RemoveAll(filepath.Clean(path))
}
func (fs *WorkingDirOS) Lstat(filename string) (os.FileInfo, error) {
if !filepath.IsAbs(filename) && !strings.HasPrefix(filename, fs.baseDir) {
filename = filepath.Join(fs.baseDir, filename)
}
return os.Lstat(filepath.Clean(filename))
}
func (fs *WorkingDirOS) Symlink(target, link string) error {
if err := fs.createDir(link); err != nil {
return err
}
if !filepath.IsAbs(link) && !strings.HasPrefix(link, fs.baseDir) {
link = filepath.Join(fs.baseDir, link)
}
return os.Symlink(target, link)
}
func (fs *WorkingDirOS) Readlink(link string) (string, error) {
if !filepath.IsAbs(link) && !strings.HasPrefix(link, fs.baseDir) {
link = filepath.Join(fs.baseDir, link)
}
return os.Readlink(link)
}
// Capabilities implements the Capable interface.
func (fs *WorkingDirOS) Capabilities() billy.Capability {
return billy.DefaultCapabilities
}i.e., a very simple one with a bunch of: if !filepath.IsAbs(link) && !strings.HasPrefix(link, fs.baseDir) {
link = filepath.Join(fs.baseDir, link)
}in relevant places. |
|
@cipherboy thanks for clarifying and sharing the code. I think the current implementation is mistakenly trying to remove the target instead of the symlink. However, the safeguards in place to restrict deletion of things outside of the repository dir are blocking that from taking place. The bug here is that we are doing a Would you be keen to propose a PR to fix this? |
I was wanting to call the equivalent of
os.RemoveAll(...)on a worktree's filesystem and ended up implementing support for it here and ingo-git(the latter was a trivial change only inRepositoryFilesystem; would this change be welcomed here?I suspect ideally this would be implemented as an optional additional interface to not break backwards compatibility (existing user would have to implement the interface on upgrade), but maybe it would be best for a v6 to simplify the interface? Thoughts?
I also ended up implementing a non-chroot'd version (similar to
osfs/os_bound.go, but taking into account an arbitrary$PWDdifferent from the actual$PWDat the time of execution), but I'm not sure that changeset has relevance outside of my use case. :-) Let me know if it is also of interest though!The text was updated successfully, but these errors were encountered: