hooks: Refactor directory locks

Currently the `RestoreCustomHooks` RPC uses `safe.LockingDirectory` to
prevent concurrent modification of a repository `custom_hooks`
directory. When the `custom_hooks` directory is locked, a `.lock` file
is created inside and subsequent calls to `Lock()` will fail until the
directory is unlocked.

In future commits, having this `.lock` file exist inside the
`custom_hooks` directory will result in vote hashes being constructed
with the lock file. Also it will make atomic directory swap operations
vulnerable to concurrent modification.

To remediate this, indstead of creating a `.lock` file in the
`custom_hooks` directory, a named lock file is created inside the
repository directory. By giving the lock a name the lock is associated
with the `custom_hooks` directory without existing inside of it.

3 files changed, 13 insertions, 9 deletions

diff --git a/internal/gitaly/service/repository/restore_custom_hooks.go b/internal/gitaly/service/repository/restore_custom_hooks.go
index 80edad305..ece1449e8 100644
--- a/internal/gitaly/service/repository/restore_custom_hooks.go
+++ b/internal/gitaly/service/repository/restore_custom_hooks.go
@@ -98,7 +98,7 @@ func (s *server) restoreCustomHooksWithVoting(stream gitalypb.RepositoryService_
 		return structerr.NewInternal("making custom hooks directory %w", err)
 	}
 
-	lockDir, err := safe.NewLockingDirectory(customHooksPath)
+	lockDir, err := safe.NewLockingDirectory(repoPath, customHooksDir)
 	if err != nil {
 		return structerr.NewInternal("RestoreCustomHooks: creating locking directory: %w", err)
 	}
diff --git a/internal/safe/locking_directory.go b/internal/safe/locking_directory.go
index b15483bce..2f51e5a8e 100644
--- a/internal/safe/locking_directory.go
+++ b/internal/safe/locking_directory.go
@@ -20,10 +20,11 @@ const (
 type LockingDirectory struct {
 	state lockingDirectoryState
 	path  string
+	name  string
 }
 
 // NewLockingDirectory creates a new LockingDirectory.
-func NewLockingDirectory(path string) (*LockingDirectory, error) {
+func NewLockingDirectory(path, name string) (*LockingDirectory, error) {
 	fi, err := os.Stat(path)
 	if err != nil {
 		return nil, fmt.Errorf("creating new locking directory: %w", err)
@@ -36,6 +37,7 @@ func NewLockingDirectory(path string) (*LockingDirectory, error) {
 	ld := &LockingDirectory{
 		state: lockingDirectoryStateUnlocked,
 		path:  path,
+		name:  name,
 	}
 
 	return ld, nil
@@ -91,5 +93,5 @@ func (ld *LockingDirectory) Unlock() error {
 }
 
 func (ld *LockingDirectory) lockPath() string {
-	return filepath.Join(ld.path, ".lock")
+	return filepath.Join(ld.path, ld.name+".lock")
 }
diff --git a/internal/safe/locking_directory_test.go b/internal/safe/locking_directory_test.go
index 98ab2d8fd..4d005f8d3 100644
--- a/internal/safe/locking_directory_test.go
+++ b/internal/safe/locking_directory_test.go
@@ -16,12 +16,14 @@ import (
 func TestLockingDirectory(t *testing.T) {
 	t.Parallel()
 
+	lockname := "foo"
+
 	t.Run("normal lifecycle", func(t *testing.T) {
 		path := testhelper.TempDir(t)
-		lockingDir, err := safe.NewLockingDirectory(path)
+		lockingDir, err := safe.NewLockingDirectory(path, lockname)
 		require.NoError(t, err)
 		require.NoError(t, lockingDir.Lock())
-		secondLockingDir, err := safe.NewLockingDirectory(path)
+		secondLockingDir, err := safe.NewLockingDirectory(path, lockname)
 		require.NoError(t, err)
 		require.NoError(t, os.WriteFile(
 			filepath.Join(path, "somefile"),
@@ -34,7 +36,7 @@ func TestLockingDirectory(t *testing.T) {
 
 	t.Run("multiple locks fail", func(t *testing.T) {
 		path := testhelper.TempDir(t)
-		lockingDir, err := safe.NewLockingDirectory(path)
+		lockingDir, err := safe.NewLockingDirectory(path, lockname)
 		require.NoError(t, err)
 		require.NoError(t, lockingDir.Lock())
 		assert.Equal(
@@ -46,7 +48,7 @@ func TestLockingDirectory(t *testing.T) {
 
 	t.Run("unlock without lock fails", func(t *testing.T) {
 		path := testhelper.TempDir(t)
-		lockingDir, err := safe.NewLockingDirectory(path)
+		lockingDir, err := safe.NewLockingDirectory(path, lockname)
 		require.NoError(t, err)
 		assert.Equal(
 			t,
@@ -57,7 +59,7 @@ func TestLockingDirectory(t *testing.T) {
 
 	t.Run("multiple unlocks fail", func(t *testing.T) {
 		path := testhelper.TempDir(t)
-		lockingDir, err := safe.NewLockingDirectory(path)
+		lockingDir, err := safe.NewLockingDirectory(path, lockname)
 		require.NoError(t, err)
 		require.NoError(t, lockingDir.Lock())
 		require.NoError(t, lockingDir.Unlock())
@@ -72,7 +74,7 @@ func TestLockingDirectory(t *testing.T) {
 		path := testhelper.TempDir(t)
 		require.NoError(t, os.RemoveAll(path))
 
-		_, err := safe.NewLockingDirectory(path)
+		_, err := safe.NewLockingDirectory(path, lockname)
 		assert.True(t, errors.Is(err, fs.ErrNotExist))
 	})
 }