Make Git hook configuration idempotent to avoid config.lock churn

src/main/java/jenkins/plugins/git/DisableHooks.java

@@ -23,8 +23,11 @@ class DisableHooks implements RepositoryCallback<Object> {
     public Object invoke(Repository repo, VirtualChannel channel) throws IOException, InterruptedException {
         final String VAL = Functions.isWindows() ? DISABLED_WIN : DISABLED_NIX;
         final StoredConfig repoConfig = repo.getConfig();
-        repoConfig.setString("core", null, "hooksPath", VAL);
-        repoConfig.save();
+        // Skip the rewrite, and its config.lock, when hooksPath is already disabled. JENKINS-71349
+        if (!VAL.equals(repoConfig.getString("core", null, "hooksPath"))) {
+            repoConfig.setString("core", null, "hooksPath", VAL);
+            repoConfig.save();
+        }
         return null;
     }
 }

src/main/java/jenkins/plugins/git/UnsetHooks.java

@@ -21,7 +21,10 @@
     public Object invoke(Repository repo, VirtualChannel channel) throws IOException, InterruptedException {
         final StoredConfig repoConfig = repo.getConfig();
         final String val = repoConfig.getString("core", null, "hooksPath");
-        if (val != null && !val.isEmpty() && !DisableHooks.DISABLED_NIX.equals(val) && !DisableHooks.DISABLED_WIN.equals(val)) {
+        if (val == null) {
+            return null; // nothing to unset; avoid a redundant config.lock write. JENKINS-71349
+        }
+        if (!val.isEmpty() && !DisableHooks.DISABLED_NIX.equals(val) && !DisableHooks.DISABLED_WIN.equals(val)) {
             LOGGER.warning(() -> "core.hooksPath explicitly set to %s and will be left intact on %s.".formatted(val, repo.getDirectory()));
         } else {
             repoConfig.unset("core", null, "hooksPath");

src/test/java/jenkins/plugins/git/GitHooksConfigurationTest.java

@@ -26,6 +26,8 @@
 import hudson.EnvVars;
 import hudson.model.TaskListener;
 
+import java.io.File;
+import java.nio.file.Files;
 import java.util.Random;
 import org.eclipse.jgit.lib.StoredConfig;
 import org.jenkinsci.plugins.gitclient.Git;
@@ -203,4 +205,74 @@ void testConfigure_3argsFalse() throws Exception {
         String hooksPath = getCoreHooksPath();
         assertThat(hooksPath, is(NULL_HOOKS_PATH));
     }
+
+    private File createConfigLock() throws Exception {
+        return client.withRepository((repo, channel) -> {
+            File lock = new File(repo.getDirectory(), "config.lock");
+            Files.deleteIfExists(lock.toPath());
+            assertTrue(lock.createNewFile());
+            return lock;
+        });
+    }
+
+    // JENKINS-71349: re-disabling an already-disabled repo must not save(). A planted stale
+    // config.lock would otherwise make save() throw LockFailedException.
+    @Test
+    void testConfigureDisabledIsNoOpWhenAlreadyDisabled() throws Exception {
+        GitHooksConfiguration.configure(client, false);
+        assertThat(getCoreHooksPath(), is(NULL_HOOKS_PATH));
+
+        File configLock = createConfigLock();
+        try {
+            GitHooksConfiguration.configure(client, false);
+            assertThat(getCoreHooksPath(), is(NULL_HOOKS_PATH));
+            assertTrue(configLock.exists(), "DisableHooks must not touch an unrelated stale lock");
+        } finally {
+            Files.deleteIfExists(configLock.toPath());
+        }
+    }
+
+    // The guard must skip only the disabled sentinel, never a different value (SECURITY-2754).
+    @Test
+    void testConfigureDisabledOverwritesNonDisabledValue() throws Exception {
+        setCoreHooksPath(ALTERNATE_HOOKS_PATH);
+        assertThat(getCoreHooksPath(), is(ALTERNATE_HOOKS_PATH));
+
+        GitHooksConfiguration.configure(client, false);
+
+        assertThat(getCoreHooksPath(), is(NULL_HOOKS_PATH));
+    }
+
+    @Test
+    void testConfigureDisabledRepeatedlyIsNoOp() throws Exception {
+        GitHooksConfiguration.configure(client, false);
+        assertThat(getCoreHooksPath(), is(NULL_HOOKS_PATH));
+
+        File configLock = createConfigLock();
+        try {
+            for (int i = 0; i < 5; i++) {
+                GitHooksConfiguration.configure(client, false);
+            }
+            assertThat(getCoreHooksPath(), is(NULL_HOOKS_PATH));
+            assertTrue(configLock.exists(), "repeated disable must not touch the stale lock");
+        } finally {
+            Files.deleteIfExists(configLock.toPath());
+        }
+    }
+
+    // JENKINS-71349, allow-hooks path: UnsetHooks must not save() when hooksPath is already absent.
+    @Test
+    void testConfigureAllowedIsNoOpWhenHooksPathAbsent() throws Exception {
+        GitHooksConfiguration.configure(client, true);
+        assertThat(getCoreHooksPath(), is(nullValue()));
+
+        File configLock = createConfigLock();
+        try {
+            GitHooksConfiguration.configure(client, true);
+            assertThat(getCoreHooksPath(), is(nullValue()));
+            assertTrue(configLock.exists(), "UnsetHooks must not touch the stale lock when hooksPath is absent");
+        } finally {
+            Files.deleteIfExists(configLock.toPath());
+        }
+    }
 }