fix(connect): close TOCTOU race in connect sync lock acquisition

internal/assets/commands/text/errors.yaml

@@ -649,6 +649,8 @@ err.hub.duplicate-project:
   short: 'project already registered: %q'
 err.hub.invalid-peer-action:
   short: "action must be 'add' or 'remove', got %q"
+err.hub.connect-sync-locked:
+  short: 'connect sync already in progress; if no sync is running, remove the stale lock %s: %w'
 err.serve.no-running-hub:
   short: 'no running hub: %w'
 err.serve.invalid-pid:

internal/cli/connection/core/sync/state.go

@@ -14,6 +14,7 @@ import (
 	"github.com/ActiveMemory/ctx/internal/config/fs"
 	cfgHub "github.com/ActiveMemory/ctx/internal/config/hub"
 	cfgWarn "github.com/ActiveMemory/ctx/internal/config/warn"
+	errHub "github.com/ActiveMemory/ctx/internal/err/hub"
 	"github.com/ActiveMemory/ctx/internal/io"
 	logWarn "github.com/ActiveMemory/ctx/internal/log/warn"
 	"github.com/ActiveMemory/ctx/internal/rc"
@@ -41,18 +42,23 @@ func loadState() (state, func(), error) {
 		return s, nil, mkErr
 	}
 
-	// Acquire lock: fail if another sync is running.
-	if _, statErr := os.Stat(lockPath); statErr == nil {
-		return s, nil, os.ErrExist
+	// Acquire lock: fail if another sync is running. The
+	// O_CREATE|O_EXCL create-or-fail is a single syscall, so
+	// there is no check-to-write window for a concurrent sync
+	// to slip through.
+	acquired, lockErr := io.SafeTryLock(lockPath, fs.PermFile)
+	if lockErr != nil {
+		return s, nil, lockErr
 	}
-	if writeErr := io.SafeWriteFile(
-		lockPath, []byte(cfgHub.LockSentinel), fs.PermFile,
-	); writeErr != nil {
-		return s, nil, writeErr
+	if !acquired {
+		// Another sync holds the lock — or a crashed sync left it
+		// stale. Name the path so the wedge is self-documenting;
+		// the error still wraps os.ErrExist for errors.Is callers.
+		return s, nil, errHub.ConnectSyncLocked(lockPath)
 	}
 
 	release := func() {
-		if rmErr := os.Remove(lockPath); rmErr != nil {
+		if rmErr := io.SafeUnlock(lockPath); rmErr != nil {
 			logWarn.Warn(cfgWarn.Remove, lockPath, rmErr)
 		}
 	}

internal/cli/connection/core/sync/state_test.go

@@ -0,0 +1,188 @@
+//   /    ctx:                         https://ctx.ist
+// ,'`./    do you remember?
+// `.,'\
+//   \    Copyright 2026-present Context contributors.
+//                 SPDX-License-Identifier: Apache-2.0
+
+package sync
+
+import (
+	"errors"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/ActiveMemory/ctx/internal/config/fs"
+	cfgHub "github.com/ActiveMemory/ctx/internal/config/hub"
+	"github.com/ActiveMemory/ctx/internal/io"
+	"github.com/ActiveMemory/ctx/internal/testutil/testctx"
+)
+
+// declareContext positions the test in a temp project with a
+// materialized .context/ directory and returns its path.
+func declareContext(t *testing.T) string {
+	t.Helper()
+	dir := t.TempDir()
+	ctxDir := testctx.Declare(t, dir)
+	if mkErr := os.MkdirAll(ctxDir, fs.PermExec); mkErr != nil {
+		t.Fatal(mkErr)
+	}
+	return ctxDir
+}
+
+func TestLoadState_RejectsConcurrentSyncs(t *testing.T) {
+	declareContext(t)
+
+	const attempts = 16
+
+	type outcome struct {
+		release func()
+		err     error
+	}
+
+	start := make(chan struct{})
+	results := make(chan outcome, attempts)
+	for i := 0; i < attempts; i++ {
+		go func() {
+			<-start
+			_, release, lockErr := loadState()
+			results <- outcome{release: release, err: lockErr}
+		}()
+	}
+	close(start)
+
+	// Winners must not release until every attempt has
+	// finished, or a late goroutine could legitimately
+	// re-acquire the freed lock and skew the count.
+	var winners []func()
+	var contended int
+	for i := 0; i < attempts; i++ {
+		got := <-results
+		switch {
+		case got.err == nil:
+			winners = append(winners, got.release)
+		case errors.Is(got.err, os.ErrExist):
+			contended++
+		default:
+			t.Fatalf("unexpected error: %v", got.err)
+		}
+	}
+
+	if len(winners) != 1 {
+		t.Errorf("winners = %d, want exactly 1", len(winners))
+	}
+	if contended != attempts-1 {
+		t.Errorf(
+			"contended = %d, want %d", contended, attempts-1,
+		)
+	}
+	for _, release := range winners {
+		release()
+	}
+}
+
+func TestLoadState_ReleaseRemovesLock(t *testing.T) {
+	ctxDir := declareContext(t)
+	lockPath := filepath.Join(
+		ctxDir, cfgHub.DirHub, cfgHub.FileSyncLock,
+	)
+
+	_, release, lockErr := loadState()
+	if lockErr != nil {
+		t.Fatalf("first loadState: %v", lockErr)
+	}
+	if _, statErr := os.Stat(lockPath); statErr != nil {
+		t.Fatalf("lock file should exist while held: %v", statErr)
+	}
+
+	release()
+
+	if _, statErr := os.Stat(lockPath); !os.IsNotExist(statErr) {
+		t.Errorf(
+			"lock file should be gone after release: %v", statErr,
+		)
+	}
+
+	// The next sync must be able to proceed.
+	_, release, lockErr = loadState()
+	if lockErr != nil {
+		t.Fatalf("loadState after release: %v", lockErr)
+	}
+	release()
+}
+
+func TestLoadState_ReleasesLockOnCorruptState(t *testing.T) {
+	ctxDir := declareContext(t)
+	hubDir := filepath.Join(ctxDir, cfgHub.DirHub)
+	if mkErr := io.SafeMkdirAll(hubDir, fs.PermKeyDir); mkErr != nil {
+		t.Fatal(mkErr)
+	}
+	statePath := filepath.Join(hubDir, cfgHub.FileSyncState)
+	if writeErr := os.WriteFile(
+		statePath, []byte("{not json"), fs.PermFile,
+	); writeErr != nil {
+		t.Fatal(writeErr)
+	}
+
+	_, _, loadErr := loadState()
+	if loadErr == nil {
+		t.Fatal("loadState should fail on corrupt state")
+	}
+
+	lockPath := filepath.Join(hubDir, cfgHub.FileSyncLock)
+	if _, statErr := os.Stat(lockPath); !os.IsNotExist(statErr) {
+		t.Errorf(
+			"lock must not leak after a failed load: %v", statErr,
+		)
+	}
+}
+
+func TestLoadState_LockedErrorIsActionable(t *testing.T) {
+	ctxDir := declareContext(t)
+	lockPath := filepath.Join(
+		ctxDir, cfgHub.DirHub, cfgHub.FileSyncLock,
+	)
+
+	_, release, lockErr := loadState()
+	if lockErr != nil {
+		t.Fatalf("first loadState: %v", lockErr)
+	}
+	defer release()
+
+	_, _, contendErr := loadState()
+	if contendErr == nil {
+		t.Fatal("second loadState should fail while lock is held")
+	}
+	// Contract preserved: still matches the pre-existing sentinel
+	// so any errors.Is(err, os.ErrExist) caller keeps working.
+	if !errors.Is(contendErr, os.ErrExist) {
+		t.Errorf(
+			"contended error must wrap os.ErrExist, got: %v",
+			contendErr,
+		)
+	}
+	// Actionable: names the lock path so a wedged stale lock is
+	// self-documenting instead of an opaque "file already exists".
+	if !strings.Contains(contendErr.Error(), lockPath) {
+		t.Errorf(
+			"error should name the lock path %q, got: %v",
+			lockPath, contendErr,
+		)
+	}
+}
+
+func TestLoadState_LockFileLocation(t *testing.T) {
+	ctxDir := declareContext(t)
+
+	_, release, lockErr := loadState()
+	if lockErr != nil {
+		t.Fatalf("loadState: %v", lockErr)
+	}
+	defer release()
+
+	want := filepath.Join(ctxDir, cfgHub.DirHub, cfgHub.FileSyncLock)
+	if _, statErr := os.Stat(want); statErr != nil {
+		t.Errorf("lock file not at %s: %v", want, statErr)
+	}
+}

internal/cli/connection/core/sync/testmain_test.go

@@ -0,0 +1,22 @@
+//   /    ctx:                         https://ctx.ist
+// ,'`./    do you remember?
+// `.,'\
+//   \    Copyright 2026-present Context contributors.
+//                 SPDX-License-Identifier: Apache-2.0
+
+package sync
+
+import (
+	"os"
+	"testing"
+
+	"github.com/ActiveMemory/ctx/internal/assets/read/lookup"
+)
+
+// TestMain initialises the embedded asset lookup so the
+// lock-contention error (errHub.ConnectSyncLocked) renders its
+// parsed format string rather than the empty default.
+func TestMain(m *testing.M) {
+	lookup.Init()
+	os.Exit(m.Run())
+}

internal/config/embed/text/err_hub.go

@@ -20,4 +20,8 @@ const (
 	// DescKeyErrHubInvalidPeerAction is the text key for
 	// unrecognized peer action errors.
 	DescKeyErrHubInvalidPeerAction = "err.hub.invalid-peer-action"
+	// DescKeyErrHubConnectSyncLocked is the text key for a
+	// connect-sync lock-contention failure; names the lock path
+	// so a wedged stale lock is self-documenting.
+	DescKeyErrHubConnectSyncLocked = "err.hub.connect-sync-locked"
 )

internal/config/hub/doc.go

@@ -59,7 +59,7 @@
 //     connection config files
 //   - FilePID ("hub.pid"), FileAdminToken,
 //     DirHubData: daemon management files
-//   - JSONIndent, LockSentinel, SuffixPluralMD:
+//   - JSONIndent, SuffixPluralMD:
 //     formatting and naming helpers
 //
 // # Raft Cluster Configuration

internal/config/hub/hub.go

@@ -167,8 +167,6 @@ const (
 	FileConnect = ".connect.enc"
 	// JSONIndent is the indentation string for JSON marshaling.
 	JSONIndent = "  "
-	// LockSentinel is the content written to lock files.
-	LockSentinel = "lock"
 	// SuffixPluralMD is the suffix for typed hub markdown
 	// filenames (e.g. "decisions.md").
 	SuffixPluralMD = "s.md"

internal/err/hub/hub.go

@@ -8,6 +8,7 @@ package hub
 
 import (
 	"fmt"
+	"os"
 
 	"github.com/ActiveMemory/ctx/internal/assets/read/desc"
 	"github.com/ActiveMemory/ctx/internal/config/embed/text"
@@ -67,3 +68,22 @@ func InvalidPeerAction(action string) error {
 		action,
 	)
 }
+
+// ConnectSyncLocked returns the error surfaced when connect
+// sync cannot acquire its lock because another sync holds it.
+// It names the lock path so a wedged stale lock (e.g. left by
+// a crashed sync) is self-documenting, and wraps os.ErrExist
+// so callers matching the pre-existing contract via
+// errors.Is(err, os.ErrExist) still match.
+//
+// Parameters:
+//   - lockPath: absolute path to the contended lock file
+//
+// Returns:
+//   - error: lock-contention error wrapping os.ErrExist
+func ConnectSyncLocked(lockPath string) error {
+	return fmt.Errorf(
+		desc.Text(text.DescKeyErrHubConnectSyncLocked),
+		lockPath, os.ErrExist,
+	)
+}

internal/io/security.go

@@ -139,6 +139,13 @@ func SafeCreateFile(path string, perm os.FileMode) (*os.File, error) {
 // holder). The handle is closed before returning; the lock is held by
 // the file's existence until SafeUnlock removes it.
 //
+// If the create succeeds but closing the handle fails, the freshly
+// created lock file is removed and (false, err) is returned: callers
+// treat a non-nil error as "not acquired" and get no release func, so
+// leaving the file behind would wedge every future caller with a lock
+// nobody owns. Removing it keeps the on-disk state consistent with the
+// reported outcome.
+//
 // Parameters:
 //   - path: lock file path
 //   - perm: file permission bits
@@ -161,7 +168,12 @@ func SafeTryLock(path string, perm os.FileMode) (bool, error) {
 		}
 		return false, openErr
 	}
-	return true, f.Close()
+	if closeErr := f.Close(); closeErr != nil {
+		// Best-effort cleanup so a half-acquired lock cannot leak.
+		_ = os.Remove(clean)
+		return false, closeErr
+	}
+	return true, nil
 }
 
 // SafeUnlock releases a lock acquired by SafeTryLock by removing the