Harden atomicWriteGitattributes writability check against TOCTOU symlink swap

Add fstatFn injectable and compare lstat vs fstat via os.SameFile after
opening the existing target file. If the path is swapped to a symlink
between lstatFile and os.OpenFile, the opened fd's inode will differ from
the lstat result, and the function returns an error rather than silently
writing through the link.

Add TestAtomicWriteGitattributes_FstatFails and
TestAtomicWriteGitattributes_LstatFdMismatch tests covering the new paths.

https://claude.ai/code/session_01UFDupDRxvbPgETqngjCta7

Author: claude

internal/githooks/githooks.go

@@ -798,26 +798,36 @@ func writeGitattributesFile(path, content string) error {
 	return nil
 }
 
-// createTempFn, syncTempFn, closeTempFn, and chmodFn are variables so tests
-// can inject failures into atomicWriteGitattributes without OS-level tricks.
+// createTempFn, syncTempFn, closeTempFn, chmodFn, and fstatFn are variables
+// so tests can inject failures into atomicWriteGitattributes without OS tricks.
 var createTempFn = os.CreateTemp
 var syncTempFn = (*os.File).Sync
 var closeTempFn = (*os.File).Close
 var chmodFn = os.Chmod
+var fstatFn = (*os.File).Stat
 
 // atomicWriteGitattributes writes data to a temp file in the same directory
 // as path, sets its permissions, then renames it over path. The rename
 // replaces the directory entry atomically, so it cannot follow a symlink
 // that might have been introduced between an earlier lstat check and the write.
 func atomicWriteGitattributes(path string, data []byte, mode os.FileMode) error {
-	// Verify an existing target is writable. os.Rename can replace read-only
-	// files when the directory is writable, so we check explicitly.
-	if _, err := lstatFile(path); err == nil {
+	// Verify an existing target is writable and has not been swapped to a
+	// symlink. os.Rename can replace read-only files when the directory is
+	// writable, so we check writability explicitly. We then compare lstat and
+	// fstat to detect a TOCTOU swap between the lstat and the open.
+	if lstatInfo, err := lstatFile(path); err == nil {
 		f, err := os.OpenFile(path, os.O_WRONLY, 0)
 		if err != nil {
 			return err
 		}
+		fdInfo, statErr := fstatFn(f)
 		_ = f.Close()
+		if statErr != nil {
+			return statErr
+		}
+		if !os.SameFile(lstatInfo, fdInfo) {
+			return fmt.Errorf("%s: file changed since lstat", path)
+		}
 	} else if !os.IsNotExist(err) {
 		return err
 	}

internal/githooks/githooks_test.go

@@ -1224,6 +1224,44 @@ func TestAtomicWriteGitattributes_LstatNonENOENTError_ReturnsError(t *testing.T)
 	assert.Contains(t, err.Error(), "mock lstat failure")
 }
 
+func TestAtomicWriteGitattributes_FstatFails_ReturnsError(t *testing.T) {
+	dir := t.TempDir()
+	path := filepath.Join(dir, ".gitattributes")
+	require.NoError(t, os.WriteFile(path, []byte("existing"), 0o644))
+
+	orig := fstatFn
+	t.Cleanup(func() { fstatFn = orig })
+	fstatFn = func(*os.File) (os.FileInfo, error) {
+		return nil, fmt.Errorf("mock fstat failure")
+	}
+
+	err := atomicWriteGitattributes(path, []byte("content"), 0o644)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "mock fstat failure")
+}
+
+func TestAtomicWriteGitattributes_LstatFdMismatch_ReturnsError(t *testing.T) {
+	dir := t.TempDir()
+	path := filepath.Join(dir, ".gitattributes")
+	other := filepath.Join(dir, "other")
+	require.NoError(t, os.WriteFile(path, []byte("existing"), 0o644))
+	require.NoError(t, os.WriteFile(other, []byte("other"), 0o644))
+
+	otherInfo, err := os.Lstat(other)
+	require.NoError(t, err)
+
+	// Inject lstatFile to return info for 'other' (different inode than path).
+	orig := lstatFile
+	t.Cleanup(func() { lstatFile = orig })
+	lstatFile = func(string) (os.FileInfo, error) {
+		return otherInfo, nil
+	}
+
+	err = atomicWriteGitattributes(path, []byte("content"), 0o644)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "changed since lstat")
+}
+
 func TestWriteGitattributes_LstatNonENOENTError_ReturnsError(t *testing.T) {
 	orig := lstatFile
 	t.Cleanup(func() { lstatFile = orig })