fix(artifact): confine folder exchange locking

Folder targets may be writable by other participants, so opening the coordination file by absolute path lets a substituted symlink escape the marked namespace. Bind locking to a root-opened, identity-checked descriptor while preserving native Unix and Windows lock semantics.\n\nExercise corruption through journaled publication so the end-to-end quarantine coverage follows the transport contract.

Author: wesm

internal/artifact/transport_folder.go

@@ -13,9 +13,6 @@ import (
 	"path/filepath"
 	"strings"
 	"sync"
-	"time"
-
-	"github.com/gofrs/flock"
 )
 
 const (
@@ -188,28 +185,6 @@ func (t *folderTransport) Exchange(
 	return result, nil
 }
 
-func (t *folderTransport) acquireExchangeLockLocked(
-	ctx context.Context,
-) (*flock.Flock, error) {
-	lock := flock.New(filepath.Join(t.target, folderExchangeLockName))
-	locked, err := lock.TryLockContext(ctx, 100*time.Millisecond)
-	if err != nil {
-		_ = lock.Close()
-		if ctxErr := ctx.Err(); ctxErr != nil {
-			return nil, ctxErr
-		}
-		return nil, fmt.Errorf("acquiring artifact folder exchange lock: %w", err)
-	}
-	if !locked {
-		_ = lock.Close()
-		if ctxErr := ctx.Err(); ctxErr != nil {
-			return nil, ctxErr
-		}
-		return nil, errors.New("artifact folder exchange is already running")
-	}
-	return lock, nil
-}
-
 func (t *folderTransport) Close() error {
 	if t == nil {
 		return nil

internal/artifact/transport_folder_lock.go

@@ -0,0 +1,99 @@
+package artifact
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"io/fs"
+	"os"
+	"time"
+)
+
+const folderExchangeLockRetryDelay = 100 * time.Millisecond
+
+type folderExchangeLock struct {
+	file *os.File
+}
+
+func (t *folderTransport) acquireExchangeLockLocked(
+	ctx context.Context,
+) (*folderExchangeLock, error) {
+	file, err := openFolderExchangeLockFile(t.root)
+	if err != nil {
+		return nil, fmt.Errorf("acquiring artifact folder exchange lock: %w", err)
+	}
+	for {
+		locked, lockErr := tryLockFolderFile(file)
+		if lockErr != nil {
+			return nil, errors.Join(
+				fmt.Errorf("acquiring artifact folder exchange lock: %w", lockErr),
+				file.Close(),
+			)
+		}
+		if locked {
+			return &folderExchangeLock{file: file}, nil
+		}
+
+		timer := time.NewTimer(folderExchangeLockRetryDelay)
+		select {
+		case <-ctx.Done():
+			timer.Stop()
+			return nil, errors.Join(ctx.Err(), file.Close())
+		case <-timer.C:
+		}
+	}
+}
+
+func openFolderExchangeLockFile(root *os.Root) (*os.File, error) {
+	for range 8 {
+		before, err := root.Lstat(folderExchangeLockName)
+		if err != nil && !errors.Is(err, fs.ErrNotExist) {
+			return nil, err
+		}
+		if err == nil && !before.Mode().IsRegular() {
+			return nil, errors.New("artifact folder exchange lock is not a regular file")
+		}
+
+		flags := os.O_RDWR
+		if errors.Is(err, fs.ErrNotExist) {
+			before = nil
+			flags |= os.O_CREATE | os.O_EXCL
+		}
+		file, openErr := root.OpenFile(folderExchangeLockName, flags, 0o600)
+		if errors.Is(openErr, fs.ErrExist) || errors.Is(openErr, fs.ErrNotExist) {
+			continue
+		}
+		if openErr != nil {
+			return nil, openErr
+		}
+
+		opened, statErr := file.Stat()
+		if statErr != nil {
+			return nil, errors.Join(statErr, file.Close())
+		}
+		after, statErr := root.Lstat(folderExchangeLockName)
+		if statErr != nil {
+			return nil, errors.Join(statErr, file.Close())
+		}
+		if !opened.Mode().IsRegular() ||
+			!after.Mode().IsRegular() ||
+			(before != nil && !os.SameFile(before, opened)) ||
+			!os.SameFile(opened, after) {
+			return nil, errors.Join(
+				errors.New("artifact folder exchange lock changed while opening"),
+				file.Close(),
+			)
+		}
+		return file, nil
+	}
+	return nil, errors.New("artifact folder exchange lock changed while opening")
+}
+
+func (l *folderExchangeLock) Close() error {
+	if l == nil || l.file == nil {
+		return nil
+	}
+	file := l.file
+	l.file = nil
+	return errors.Join(unlockFolderFile(file), file.Close())
+}

internal/artifact/transport_folder_lock_other.go

@@ -0,0 +1,16 @@
+//go:build !darwin && !dragonfly && !freebsd && !illumos && !linux && !netbsd && !openbsd && !windows
+
+package artifact
+
+import (
+	"errors"
+	"os"
+)
+
+func tryLockFolderFile(*os.File) (bool, error) {
+	return false, errors.ErrUnsupported
+}
+
+func unlockFolderFile(*os.File) error {
+	return errors.ErrUnsupported
+}

internal/artifact/transport_folder_lock_unix.go

@@ -0,0 +1,26 @@
+//go:build darwin || dragonfly || freebsd || illumos || linux || netbsd || openbsd
+
+package artifact
+
+import (
+	"errors"
+	"os"
+
+	"golang.org/x/sys/unix"
+)
+
+func tryLockFolderFile(file *os.File) (bool, error) {
+	err := unix.Flock(int(file.Fd()), unix.LOCK_EX|unix.LOCK_NB)
+	switch {
+	case errors.Is(err, unix.EWOULDBLOCK), errors.Is(err, unix.EAGAIN):
+		return false, nil
+	case err != nil:
+		return false, err
+	default:
+		return true, nil
+	}
+}
+
+func unlockFolderFile(file *os.File) error {
+	return unix.Flock(int(file.Fd()), unix.LOCK_UN)
+}

internal/artifact/transport_folder_lock_windows.go

@@ -0,0 +1,44 @@
+//go:build windows
+
+package artifact
+
+import (
+	"errors"
+	"os"
+
+	"golang.org/x/sys/windows"
+)
+
+func tryLockFolderFile(file *os.File) (bool, error) {
+	err := windows.LockFileEx(
+		windows.Handle(file.Fd()),
+		windows.LOCKFILE_EXCLUSIVE_LOCK|windows.LOCKFILE_FAIL_IMMEDIATELY,
+		0,
+		1,
+		0,
+		&windows.Overlapped{},
+	)
+	switch {
+	case errors.Is(err, windows.ERROR_LOCK_VIOLATION),
+		errors.Is(err, windows.ERROR_IO_PENDING):
+		return false, nil
+	case err != nil && !errors.Is(err, windows.Errno(0)):
+		return false, err
+	default:
+		return true, nil
+	}
+}
+
+func unlockFolderFile(file *os.File) error {
+	err := windows.UnlockFileEx(
+		windows.Handle(file.Fd()),
+		0,
+		1,
+		0,
+		&windows.Overlapped{},
+	)
+	if errors.Is(err, windows.Errno(0)) {
+		return nil
+	}
+	return err
+}

internal/artifact/transport_folder_test.go

@@ -999,6 +999,30 @@ func TestFolderTransportExchangeLockProtectsActivePublishTemp(t *testing.T) {
 	assert.FileExists(t, active)
 }
 
+func TestFolderTransportExchangeLockRejectsSymlinkEscape(t *testing.T) {
+	t.Parallel()
+
+	target := t.TempDir()
+	transport, err := OpenFolderTransport(target, FolderTransportOptions{})
+	require.NoError(t, err)
+	t.Cleanup(func() { require.NoError(t, transport.Close()) })
+
+	escaped := filepath.Join(t.TempDir(), "escaped.lock")
+	lockPath := filepath.Join(target, folderExchangeLockName)
+	if err := os.Symlink(escaped, lockPath); err != nil {
+		t.Skipf("creating lock symlink: %v", err)
+	}
+
+	_, err = transport.Exchange(
+		t.Context(),
+		&transportRecordingStore{ArtifactStore: newTestArtifactStore(t)},
+		testFolderPublishOrigin,
+	)
+	require.Error(t, err)
+	assert.ErrorContains(t, err, "exchange lock is not a regular file")
+	assert.NoFileExists(t, escaped)
+}
+
 func TestFolderTransportRejectsOversizedStoreEntryBeforePublication(
 	t *testing.T,
 ) {

internal/e2e/artifact_sync_test.go

@@ -5,6 +5,8 @@ package e2e_test
 import (
 	"bytes"
 	"context"
+	"crypto/sha256"
+	"encoding/hex"
 	"os"
 	"path/filepath"
 	"testing"
@@ -110,7 +112,7 @@ func TestArtifactSyncTwoInstanceFolder(t *testing.T) {
 		3,
 	)
 
-	writeCompleteCorruptCheckpoint(t, target)
+	publishCompleteCorruptCheckpoint(t, target)
 	afterCorruptionA := syncArtifactNode(t, nodeA, target)
 	assert.Equal(t, 1, afterCorruptionA.Quarantined)
 	afterCorruptionB := syncArtifactNode(t, nodeB, target)
@@ -268,39 +270,60 @@ func writeArtifactSyncSource(
 	require.NoError(t, os.WriteFile(path, content, 0o600))
 }
 
-func writeCompleteCorruptCheckpoint(
+func publishCompleteCorruptCheckpoint(
 	t *testing.T,
 	target string,
 ) {
 	t.Helper()
 	origin := "broken-a7b8c9"
+	body := []byte(`{"v":1}`)
 	ref, err := artifact.NewRef(
 		origin,
 		artifact.KindCheckpoints,
 		"cp-0000000001.json",
 	)
 	require.NoError(t, err)
-	wire, err := artifact.ToWireRef(ref)
+	sum := sha256.Sum256(body)
+	identity, err := artifact.NewIdentity(
+		hex.EncodeToString(sum[:]),
+		int64(len(body)),
+	)
 	require.NoError(t, err)
-	directory := filepath.Join(
-		target,
-		origin,
-		string(artifact.KindCheckpoints),
+	repository, err := artifact.OpenRepository(t.Context(), t.TempDir())
+	require.NoError(t, err)
+	t.Cleanup(func() { require.NoError(t, repository.Close()) })
+	_, err = repository.Content().Create(
+		t.Context(),
+		ref,
+		identity,
+		"application/json",
+		bytes.NewReader(body),
 	)
-	require.NoError(t, os.MkdirAll(directory, 0o755))
-	file, err := os.OpenFile(
-		filepath.Join(directory, wire.Name),
-		os.O_WRONLY|os.O_CREATE|os.O_EXCL,
-		0o600,
+	require.NoError(t, err)
+
+	transport, err := artifact.OpenFolderTransport(
+		target,
+		artifact.FolderTransportOptions{},
 	)
 	require.NoError(t, err)
-	encodeErr := artifact.EncodeWire(
-		context.Background(),
-		ref,
-		bytes.NewBufferString(`{"v":1}`),
-		file,
+	t.Cleanup(func() { require.NoError(t, transport.Close()) })
+	result, err := transport.Exchange(
+		t.Context(),
+		artifactSyncTransportStore{ArtifactStore: repository.Content()},
+		origin,
 	)
-	closeErr := file.Close()
-	require.NoError(t, encodeErr)
-	require.NoError(t, closeErr)
+	require.NoError(t, err)
+	assert.Equal(t, 1, result.Published)
+	assert.False(t, result.More)
+}
+
+type artifactSyncTransportStore struct {
+	artifact.ArtifactStore
+}
+
+func (artifactSyncTransportStore) RecordTransportChanged(
+	context.Context,
+	artifact.Entry,
+) error {
+	return nil
 }