Merge pull request #33770 from overleaf/mj-git-bridge-hook-readbranch

[git-bridge] Ensure pushed branches match branch on disk

GitOrigin-RevId: 9fb6b6f6475790ad83b560a84b625e6a2b806254

services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/git/handler/hook/WriteLatexPutHook.java

@@ -112,7 +112,7 @@ public class WriteLatexPutHook implements PreReceiveHook {
   private void handleReceiveCommand(
       Optional<Credential> oauth2, Repository repository, ReceiveCommand receiveCommand)
       throws IOException, GitUserException, CannotAcquireLockException {
-    checkBranch(receiveCommand);
+    checkBranch(receiveCommand, repository);
     checkForcedPush(receiveCommand);
     bridge.push(
         oauth2,
@@ -122,9 +122,16 @@ public class WriteLatexPutHook implements PreReceiveHook {
         hostname);
   }
 
-  private void checkBranch(ReceiveCommand receiveCommand) throws WrongBranchException {
-    if (!receiveCommand.getRefName().equals("refs/heads/master")) {
-      throw new WrongBranchException();
+  private void checkBranch(ReceiveCommand receiveCommand, Repository repository)
+      throws WrongBranchException, IOException {
+    String expectedRef = repository.getFullBranch();
+    if (expectedRef == null) {
+      throw new IllegalStateException(
+          "Cannot determine expected branch: repository HEAD is unresolved"
+              + " (detached or unborn ref)");
+    }
+    if (!receiveCommand.getRefName().equals(expectedRef)) {
+      throw new WrongBranchException(expectedRef);
     }
   }
 

services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/git/handler/hook/exception/WrongBranchException.java

@@ -3,6 +3,7 @@ package uk.ac.ic.wlgitbridge.git.handler.hook.exception;
 import com.google.gson.JsonElement;
 import java.util.Arrays;
 import java.util.List;
+import java.util.Objects;
 import uk.ac.ic.wlgitbridge.snapshot.push.exception.SnapshotPostException;
 
 /*
@@ -10,9 +11,12 @@ import uk.ac.ic.wlgitbridge.snapshot.push.exception.SnapshotPostException;
  */
 public class WrongBranchException extends SnapshotPostException {
 
-  private static final String[] DESCRIPTION_LINES = {
-    "You can't push any new branches.", "Please use the master branch."
-  };
+  private final String branchName;
+
+  public WrongBranchException(String expectedRef) {
+    Objects.requireNonNull(expectedRef, "expectedRef must not be null");
+    this.branchName = expectedRef.substring(expectedRef.lastIndexOf('/') + 1);
+  }
 
   @Override
   public String getMessage() {
@@ -21,7 +25,8 @@ public class WrongBranchException extends SnapshotPostException {
 
   @Override
   public List<String> getDescriptionLines() {
-    return Arrays.asList(DESCRIPTION_LINES);
+    return Arrays.asList(
+        "You can't push any new branches.", "Please use the " + branchName + " branch.");
   }
 
   @Override

services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/git/handler/hook/WriteLatexPutHookTest.java

@@ -1,7 +1,10 @@
 package uk.ac.ic.wlgitbridge.git.handler.hook;
 
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.mock;
@@ -16,17 +19,24 @@ import java.io.File;
 import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
+import org.eclipse.jgit.api.Git;
+import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.storage.file.FileRepositoryBuilder;
 import org.eclipse.jgit.transport.ReceiveCommand;
 import org.eclipse.jgit.transport.ReceiveCommand.Result;
 import org.eclipse.jgit.transport.ReceivePack;
 import org.junit.After;
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
 import org.slf4j.LoggerFactory;
 import uk.ac.ic.wlgitbridge.application.GitBridgeApp;
 import uk.ac.ic.wlgitbridge.bridge.Bridge;
+import uk.ac.ic.wlgitbridge.bridge.repo.ProjectRepo;
 import uk.ac.ic.wlgitbridge.bridge.repo.RepoStore;
+import uk.ac.ic.wlgitbridge.data.filestore.RawDirectory;
 
 public class WriteLatexPutHookTest {
 
@@ -38,8 +48,10 @@ public class WriteLatexPutHookTest {
   private ListAppender<ILoggingEvent> listAppender;
   private Logger logger;
 
+  @Rule public TemporaryFolder tmpFolder = new TemporaryFolder();
+
   @Before
-  public void setup() {
+  public void setup() throws Exception {
     bridge = mock(Bridge.class);
     repoStore = mock(RepoStore.class);
     hook = new WriteLatexPutHook(repoStore, bridge, "localhost", Optional.empty());
@@ -49,11 +61,17 @@ public class WriteLatexPutHookTest {
     Repository repository = mock(Repository.class);
     when(receivePack.getRepository()).thenReturn(repository);
     when(repository.getDirectory()).thenReturn(new File("/tmp/test-repo/.git"));
+    when(repository.getFullBranch()).thenReturn("refs/heads/master");
 
     logger = (Logger) LoggerFactory.getLogger(GitBridgeApp.class);
     listAppender = new ListAppender<>();
     listAppender.start();
     logger.addAppender(listAppender);
+
+    ProjectRepo projectRepo = mock(ProjectRepo.class);
+    when(projectRepo.getDirectory()).thenReturn(mock(RawDirectory.class));
+    when(repoStore.useJGitRepo(any(Repository.class), any(ObjectId.class))).thenReturn(projectRepo);
+    hook = new WriteLatexPutHook(repoStore, bridge, "localhost", Optional.empty());
   }
 
   @After
@@ -125,4 +143,88 @@ public class WriteLatexPutHookTest {
     assertTrue("Expected a WARN log for ForcedPushException", hasWarnLog);
     assertFalse("Expected no ERROR log for ForcedPushException", hasErrorLog);
   }
+
+  @Test
+  public void pushToMainAcceptedOnMainRepo() throws Exception {
+    Repository repo = createRepoWithBranch("refs/heads/main");
+    ObjectId head = repo.resolve("HEAD");
+    ReceiveCommand cmd =
+        new ReceiveCommand(head, head, "refs/heads/main", ReceiveCommand.Type.UPDATE);
+    ReceivePack rp = new ReceivePack(repo);
+
+    hook.onPreReceive(rp, Collections.singletonList(cmd));
+
+    // checkBranch passes; the command fails later on bridge.push (mocked),
+    // but it should NOT be REJECTED_OTHER_REASON with "wrong branch"
+    assertNotEquals("wrong branch", cmd.getMessage());
+  }
+
+  @Test
+  public void pushToMasterRejectedOnMainRepo() throws Exception {
+    Repository repo = createRepoWithBranch("refs/heads/main");
+    ObjectId head = repo.resolve("HEAD");
+    ReceiveCommand cmd =
+        new ReceiveCommand(head, head, "refs/heads/master", ReceiveCommand.Type.UPDATE);
+    ReceivePack rp = new ReceivePack(repo);
+
+    hook.onPreReceive(rp, Collections.singletonList(cmd));
+
+    assertEquals(Result.REJECTED_OTHER_REASON, cmd.getResult());
+    assertEquals("wrong branch", cmd.getMessage());
+  }
+
+  @Test
+  public void pushToMasterAcceptedOnMasterRepo() throws Exception {
+    Repository repo = createRepoWithBranch("refs/heads/master");
+    ObjectId head = repo.resolve("HEAD");
+    ReceiveCommand cmd =
+        new ReceiveCommand(head, head, "refs/heads/master", ReceiveCommand.Type.UPDATE);
+    ReceivePack rp = new ReceivePack(repo);
+
+    hook.onPreReceive(rp, Collections.singletonList(cmd));
+
+    assertNotEquals("wrong branch", cmd.getMessage());
+  }
+
+  @Test
+  public void pushToMainRejectedOnMasterRepo() throws Exception {
+    Repository repo = createRepoWithBranch("refs/heads/master");
+    ObjectId head = repo.resolve("HEAD");
+    ReceiveCommand cmd =
+        new ReceiveCommand(head, head, "refs/heads/main", ReceiveCommand.Type.UPDATE);
+    ReceivePack rp = new ReceivePack(repo);
+
+    hook.onPreReceive(rp, Collections.singletonList(cmd));
+
+    assertEquals(Result.REJECTED_OTHER_REASON, cmd.getResult());
+    assertEquals("wrong branch", cmd.getMessage());
+  }
+
+  @Test
+  public void pushToFeatureBranchRejected() throws Exception {
+    Repository repo = createRepoWithBranch("refs/heads/main");
+    ObjectId head = repo.resolve("HEAD");
+    ReceiveCommand cmd =
+        new ReceiveCommand(head, head, "refs/heads/feature", ReceiveCommand.Type.UPDATE);
+    ReceivePack rp = new ReceivePack(repo);
+
+    hook.onPreReceive(rp, Collections.singletonList(cmd));
+
+    assertEquals(Result.REJECTED_OTHER_REASON, cmd.getResult());
+    assertEquals("wrong branch", cmd.getMessage());
+  }
+
+  private Repository createRepoWithBranch(String branchRef) throws Exception {
+    File repoDir = tmpFolder.newFolder();
+    Repository repo = new FileRepositoryBuilder().setWorkTree(repoDir).build();
+    repo.create();
+    // Point HEAD at the desired branch
+    repo.updateRef("HEAD").link(branchRef);
+    // Create an initial commit so the branch ref exists
+    Git git = new Git(repo);
+    new File(repoDir, "test.txt").createNewFile();
+    git.add().addFilepattern("test.txt").call();
+    git.commit().setMessage("initial").call();
+    return repo;
+  }
 }

services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/git/handler/hook/exception/WrongBranchExceptionTest.java

@@ -0,0 +1,29 @@
+package uk.ac.ic.wlgitbridge.git.handler.hook.exception;
+
+import static org.junit.Assert.assertEquals;
+
+import java.util.List;
+import org.junit.Test;
+
+public class WrongBranchExceptionTest {
+
+  @Test
+  public void messageIncludesMainBranch() {
+    WrongBranchException e = new WrongBranchException("refs/heads/main");
+    assertEquals("wrong branch", e.getMessage());
+    List<String> lines = e.getDescriptionLines();
+    assertEquals(2, lines.size());
+    assertEquals("You can't push any new branches.", lines.get(0));
+    assertEquals("Please use the main branch.", lines.get(1));
+  }
+
+  @Test
+  public void messageIncludesMasterBranch() {
+    WrongBranchException e = new WrongBranchException("refs/heads/master");
+    assertEquals("wrong branch", e.getMessage());
+    List<String> lines = e.getDescriptionLines();
+    assertEquals(2, lines.size());
+    assertEquals("You can't push any new branches.", lines.get(0));
+    assertEquals("Please use the master branch.", lines.get(1));
+  }
+}