Use proper repo permissions when creating directories

Honor the repository permssions set with the umask or core.sharedRepository when creating directories by using tools.MkdirAll. Pass an appropriate configuration or filesystem object through as needed to ensure that we can query the proper permissions. Add a test that these code paths work by cloning a new repository and then performing some operations on it. Note that in the test, we match patterns with either the setgid bit clear or set, since it may be set or not depending on whether the system has SysV or BSD group semantics.
git-lfs · Dec 5, 2018 · cc815f59e60e113d378fc28a5ce3211079eebbd0 · cc815f5
1 parent d9e4683
commit cc815f59e60e113d378fc28a5ce3211079eebbd0
diff --git a/commands/command_checkout.go b/commands/command_checkout.go
@@ -50,7 +50,7 @@ func checkoutCommand(cmd *cobra.Command, args []string) {
 	logger := tasklog.NewLogger(os.Stdout,
 		tasklog.ForceProgress(cfg.ForceProgress()),
 	)
-  
-	meter := tq.NewMeter()
+  
+	meter := tq.NewMeter(cfg)
 	meter.Direction = tq.Checkout
 	meter.Logger = meter.LoggerFromEnv(cfg.Os)
 	logger.Enqueue(meter)

diff --git a/commands/command_fsck.go b/commands/command_fsck.go
@@ -10,6 +10,7 @@ import (
 	"github.com/git-lfs/git-lfs/filepathfilter"
 	"github.com/git-lfs/git-lfs/git"
 	"github.com/git-lfs/git-lfs/lfs"
+  
+	"github.com/git-lfs/git-lfs/tools"
 	"github.com/spf13/cobra"
 )
@@ -76,7 +77,7 @@ func fsckCommand(cmd *cobra.Command, args []string) {
 	badDir := filepath.Join(cfg.LFSStorageDir(), "bad")
 	Print("Moving corrupt objects to %s", badDir)
-  
-	if err := os.MkdirAll(badDir, 0755); err != nil {
+  
+	if err := tools.MkdirAll(badDir, cfg); err != nil {
 		ExitWithError(err)
 	}

diff --git a/commands/command_pull.go b/commands/command_pull.go
@@ -41,7 +41,7 @@ func pull(filter *filepathfilter.Filter) {
 	logger := tasklog.NewLogger(os.Stdout,
 		tasklog.ForceProgress(cfg.ForceProgress()),
 	)
-  
-	meter := tq.NewMeter()
+  
+	meter := tq.NewMeter(cfg)
 	meter.Logger = meter.LoggerFromEnv(cfg.Os)
 	logger.Enqueue(meter)
 	remote := cfg.Remote()

diff --git a/commands/commands.go b/commands/commands.go
@@ -90,9 +90,9 @@ func closeAPIClient() error {
 }
 func newLockClient() *locking.Client {
-  
-	lockClient, err := locking.NewClient(cfg.PushRemote(), getAPIClient())
+  
+	lockClient, err := locking.NewClient(cfg.PushRemote(), getAPIClient(), cfg)
 	if err == nil {
-  
-		os.MkdirAll(cfg.LFSStorageDir(), 0755)
+  
+		tools.MkdirAll(cfg.LFSStorageDir(), cfg)
 		err = lockClient.SetupFileCache(cfg.LFSStorageDir())
 	}
@@ -142,7 +142,7 @@ func getHookInstallSteps() string {
 	if err != nil {
 		ExitWithError(err)
 	}
-  
-	hooks := lfs.LoadHooks(hookDir)
+  
+	hooks := lfs.LoadHooks(hookDir, cfg)
 	steps := make([]string, 0, len(hooks))
 	for _, h := range hooks {
 		steps = append(steps, fmt.Sprintf(
@@ -158,7 +158,7 @@ func installHooks(force bool) error {
 	if err != nil {
 		return err
 	}
-  
-	hooks := lfs.LoadHooks(hookDir)
+  
+	hooks := lfs.LoadHooks(hookDir, cfg)
 	for _, h := range hooks {
 		if err := h.Install(force); err != nil {
 			return err
@@ -178,7 +178,7 @@ func uninstallHooks() error {
 	if err != nil {
 		return err
 	}
-  
-	hooks := lfs.LoadHooks(hookDir)
+  
+	hooks := lfs.LoadHooks(hookDir, cfg)
 	for _, h := range hooks {
 		if err := h.Uninstall(); err != nil {
 			return err
@@ -343,7 +343,7 @@ func logPanic(loggedError error) string {
 	name := now.Format("20060102T150405.999999999")
 	full := filepath.Join(cfg.LocalLogDir(), name+".log")
-  
-	if err := os.MkdirAll(cfg.LocalLogDir(), 0755); err != nil {
+  
+	if err := tools.MkdirAll(cfg.LocalLogDir(), cfg); err != nil {
 		full = ""
 		fmt.Fprintf(fmtWriter, "Unable to log panic to %s: %s\n\n", cfg.LocalLogDir(), err.Error())
 	} else if file, err := os.Create(full); err != nil {
@@ -464,7 +464,7 @@ func determineIncludeExcludePaths(config *config.Configuration, includeArg, excl
 }
 func buildProgressMeter(dryRun bool, d tq.Direction) *tq.Meter {
-  
-	m := tq.NewMeter()
+  
+	m := tq.NewMeter(cfg)
 	m.Logger = m.LoggerFromEnv(cfg.Os)
 	m.DryRun = dryRun
 	m.Direction = d

diff --git a/commands/run.go b/commands/run.go
@@ -10,6 +10,7 @@ import (
 	"time"
 	"github.com/git-lfs/git-lfs/config"
+  
+	"github.com/git-lfs/git-lfs/tools"
 	"github.com/spf13/cobra"
 )
@@ -117,7 +118,7 @@ func setupHTTPLogger(cmd *cobra.Command, args []string) {
 	}
 	logBase := filepath.Join(cfg.LocalLogDir(), "http")
-  
-	if err := os.MkdirAll(logBase, 0755); err != nil {
+  
+	if err := tools.MkdirAll(logBase, cfg); err != nil {
 		fmt.Fprintf(os.Stderr, "Error logging http stats: %s\n", err)
 		return
 	}

diff --git a/fs/fs.go b/fs/fs.go
@@ -66,7 +66,7 @@ func (f *Filesystem) ObjectExists(oid string, size int64) bool {
 func (f *Filesystem) ObjectPath(oid string) (string, error) {
 	dir := f.localObjectDir(oid)
-  
-	if err := os.MkdirAll(dir, 0755); err != nil {
+  
+	if err := tools.MkdirAll(dir, f); err != nil {
 		return "", fmt.Errorf("Error trying to create local storage directory in %q: %s", dir, err)
 	}
 	return filepath.Join(dir, oid), nil
@@ -141,7 +141,7 @@ func (f *Filesystem) LFSObjectDir() string {
 	if len(f.lfsobjdir) == 0 {
 		f.lfsobjdir = filepath.Join(f.LFSStorageDir, "objects")
-  
-		os.MkdirAll(f.lfsobjdir, 0755)
+  
+		tools.MkdirAll(f.lfsobjdir, f)
 	}
 	return f.lfsobjdir
@@ -153,7 +153,7 @@ func (f *Filesystem) LogDir() string {
 	if len(f.logdir) == 0 {
 		f.logdir = filepath.Join(f.LFSStorageDir, "logs")
-  
-		os.MkdirAll(f.logdir, 0755)
+  
+		tools.MkdirAll(f.logdir, f)
 	}
 	return f.logdir
@@ -165,7 +165,7 @@ func (f *Filesystem) TempDir() string {
 	if len(f.tmpdir) == 0 {
 		f.tmpdir = filepath.Join(f.LFSStorageDir, "tmp")
-  
-		os.MkdirAll(f.tmpdir, 0755)
+  
+		tools.MkdirAll(f.tmpdir, f)
 	}
 	return f.tmpdir

diff --git a/lfs/gitfilter_smudge.go b/lfs/gitfilter_smudge.go
@@ -15,7 +15,7 @@ import (
 )
 func (f *GitFilter) SmudgeToFile(filename string, ptr *Pointer, download bool, manifest *tq.Manifest, cb tools.CopyCallback) error {
-  
-	os.MkdirAll(filepath.Dir(filename), 0755)
+  
+	tools.MkdirAll(filepath.Dir(filename), f.cfg)
 	if stat, _ := os.Stat(filename); stat != nil && stat.Mode()&0200 == 0 {
 		if err := os.Chmod(filename, stat.Mode()|0200); err != nil {

diff --git a/lfs/hook.go b/lfs/hook.go
@@ -8,6 +8,7 @@ import (
 	"path/filepath"
 	"strings"
+  
+	"github.com/git-lfs/git-lfs/config"
 	"github.com/git-lfs/git-lfs/tools"
 	"github.com/rubyist/tracerx"
 )
@@ -25,30 +26,32 @@ type Hook struct {
 	Contents     string
 	Dir          string
 	upgradeables []string
+  
+	cfg          *config.Configuration
 }
-  
-func LoadHooks(hookDir string) []*Hook {
+  
+func LoadHooks(hookDir string, cfg *config.Configuration) []*Hook {
 	return []*Hook{
 		NewStandardHook("pre-push", hookDir, []string{
 			"#!/bin/sh\ngit lfs push --stdin $*",
 			"#!/bin/sh\ngit lfs push --stdin \"$@\"",
 			"#!/bin/sh\ngit lfs pre-push \"$@\"",
 			"#!/bin/sh\ncommand -v git-lfs >/dev/null 2>&1 || { echo >&2 \"\\nThis repository has been set up with Git LFS but Git LFS is not installed.\\n\"; exit 0; }\ngit lfs pre-push \"$@\"",
 			"#!/bin/sh\ncommand -v git-lfs >/dev/null 2>&1 || { echo >&2 \"\\nThis repository has been set up with Git LFS but Git LFS is not installed.\\n\"; exit 2; }\ngit lfs pre-push \"$@\"",
-  
-		}),
-  
-		NewStandardHook("post-checkout", hookDir, []string{}),
-  
-		NewStandardHook("post-commit", hookDir, []string{}),
-  
-		NewStandardHook("post-merge", hookDir, []string{}),
+  
+		}, cfg),
+  
+		NewStandardHook("post-checkout", hookDir, []string{}, cfg),
+  
+		NewStandardHook("post-commit", hookDir, []string{}, cfg),
+  
+		NewStandardHook("post-merge", hookDir, []string{}, cfg),
 	}
 }
 // NewStandardHook creates a new hook using the template script calling 'git lfs theType'
-  
-func NewStandardHook(theType, hookDir string, upgradeables []string) *Hook {
+  
+func NewStandardHook(theType, hookDir string, upgradeables []string, cfg *config.Configuration) *Hook {
 	return &Hook{
 		Type:         theType,
 		Contents:     strings.Replace(hookBaseContent, "{{Command}}", theType, -1),
 		Dir:          hookDir,
 		upgradeables: upgradeables,
+  
+		cfg:          cfg,
 	}
 }
@@ -71,7 +74,7 @@ func (h *Hook) Path() string {
 func (h *Hook) Install(force bool) error {
 	msg := fmt.Sprintf("Install hook: %s, force=%t, path=%s", h.Type, force, h.Path())
-  
-	if err := os.MkdirAll(h.Dir, 0755); err != nil {
+  
+	if err := tools.MkdirAll(h.Dir, h.cfg); err != nil {
 		return err
 	}

diff --git a/lfs/util.go b/lfs/util.go
@@ -40,7 +40,7 @@ func (f *GitFilter) CopyCallbackFile(event, filename string, index, totalFiles i
 	}
 	cbDir := filepath.Dir(logPath)
-  
-	if err := os.MkdirAll(cbDir, 0755); err != nil {
+  
+	if err := tools.MkdirAll(cbDir, f.cfg); err != nil {
 		return nil, nil, wrapProgressError(err, event, logPath)
 	}

diff --git a/locking/locks.go b/locking/locks.go
@@ -9,6 +9,7 @@ import (
 	"sync"
 	"time"
+  
+	"github.com/git-lfs/git-lfs/config"
 	"github.com/git-lfs/git-lfs/errors"
 	"github.com/git-lfs/git-lfs/filepathfilter"
 	"github.com/git-lfs/git-lfs/git"
@@ -43,6 +44,7 @@ type Client struct {
 	client    *lockClient
 	cache     LockCacher
 	cacheDir  string
+  
+	cfg       *config.Configuration
 	lockablePatterns []string
 	lockableFilter   *filepathfilter.Filter
@@ -56,11 +58,12 @@ type Client struct {
 // NewClient creates a new locking client with the given configuration
 // You must call the returned object's `Close` method when you are finished with
 // it
-  
-func NewClient(remote string, lfsClient *lfsapi.Client) (*Client, error) {
+  
+func NewClient(remote string, lfsClient *lfsapi.Client, cfg *config.Configuration) (*Client, error) {
 	return &Client{
 		Remote: remote,
 		client: &lockClient{Client: lfsClient},
 		cache:  &nilLockCacher{},
+  
+		cfg:    cfg,
 	}, nil
 }
@@ -433,7 +436,7 @@ func (c *Client) prepareCacheDirectory() (string, error) {
 			return cacheDir, errors.New("init cache directory " + cacheDir + " failed: already exists, but is no directory")
 		}
 	} else if os.IsNotExist(err) {
-  
-		err = os.MkdirAll(cacheDir, os.ModePerm)
+  
+		err = tools.MkdirAll(cacheDir, c.cfg)
 		if err != nil {
 			return cacheDir, errors.Wrap(err, "init cache directory "+cacheDir+" failed: directory creation failed")
 		}

diff --git a/locking/locks_test.go b/locking/locks_test.go
@@ -10,6 +10,7 @@ import (
 	"testing"
 	"time"
+  
+	"github.com/git-lfs/git-lfs/config"
 	"github.com/git-lfs/git-lfs/git"
 	"github.com/git-lfs/git-lfs/lfsapi"
 	"github.com/git-lfs/git-lfs/lfshttp"
@@ -57,7 +58,7 @@ func TestRemoteLocksWithCache(t *testing.T) {
 	}))
 	require.Nil(t, err)
-  
-	client, err := NewClient("", lfsclient)
+  
+	client, err := NewClient("", lfsclient, config.New())
 	assert.Nil(t, err)
 	assert.Nil(t, client.SetupFileCache(tempDir))
@@ -163,7 +164,7 @@ func TestRefreshCache(t *testing.T) {
 	}))
 	require.Nil(t, err)
-  
-	client, err := NewClient("", lfsclient)
+  
+	client, err := NewClient("", lfsclient, config.New())
 	assert.Nil(t, err)
 	assert.Nil(t, client.SetupFileCache(tempDir))
@@ -235,7 +236,7 @@ func TestGetVerifiableLocks(t *testing.T) {
 	}))
 	require.Nil(t, err)
-  
-	client, err := NewClient("", lfsclient)
+  
+	client, err := NewClient("", lfsclient, config.New())
 	assert.Nil(t, err)
 	ourLocks, theirLocks, err := client.VerifiableLocks(nil, 0)

diff --git a/t/cmd/util/testutils.go b/t/cmd/util/testutils.go
@@ -104,6 +104,10 @@ func (r *Repo) Filesystem() *fs.Filesystem {
 	return r.fs
 }
+  
+func (r *Repo) Configuration() *config.Configuration {
+  
+	return r.cfg
+  
+}
+  
+
 func (r *Repo) GitConfig() *git.Configuration {
 	return r.cfg.GitConfig()
 }
@@ -180,10 +184,6 @@ func newRepo(callback RepoCallback, settings *RepoCreateSettings) *Repo {
 		ret.GitDir = filepath.Join(ret.Path, ".git")
 	}
-  
-	ret.cfg = config.NewIn(ret.Path, ret.GitDir)
-  
-	ret.fs = ret.cfg.Filesystem()
-  
-	ret.gitfilter = lfs.NewGitFilter(ret.cfg)
-  
-
 	args = append(args, path)
 	cmd := exec.Command("git", args...)
 	err = cmd.Run()
@@ -192,6 +192,10 @@ func newRepo(callback RepoCallback, settings *RepoCreateSettings) *Repo {
 		callback.Fatalf("Unable to create git repo at %v: %v", path, err)
 	}
+  
+	ret.cfg = config.NewIn(ret.Path, ret.GitDir)
+  
+	ret.fs = ret.cfg.Filesystem()
+  
+	ret.gitfilter = lfs.NewGitFilter(ret.cfg)
+  
+
 	// Configure default user/email so not reliant on env
 	ret.Pushd()
 	RunGitCommand(callback, true, "config", "user.name", "Git LFS Tests")

diff --git a/t/git-lfs-test-server-api/main.go b/t/git-lfs-test-server-api/main.go
@@ -185,7 +185,7 @@ func buildTestData(repo *t.Repo, manifest *tq.Manifest) (oidsExist, oidsMissing
 	logger := tasklog.NewLogger(os.Stdout,
 		tasklog.ForceProgress(false),
 	)
-  
-	meter := tq.NewMeter()
+  
+	meter := tq.NewMeter(repo.Configuration())
 	meter.Logger = meter.LoggerFromEnv(repo.OSEnv())
 	logger.Enqueue(meter)
 	commit := t.CommitInput{CommitterName: "A N Other", CommitterEmail: "noone@somewhere.com"}