From 1542fbab574e751a6747d935bc56333cbc6f86bb Mon Sep 17 00:00:00 2001 From: David <33458145+davidmcpowell@users.noreply.github.com> Date: Tue, 9 Jun 2026 09:59:47 +0100 Subject: [PATCH] Merge pull request #33770 from overleaf/mj-git-bridge-hook-readbranch [git-bridge] Ensure pushed branches match branch on disk GitOrigin-RevId: 9fb6b6f6475790ad83b560a84b625e6a2b806254 --- .../git/handler/hook/WriteLatexPutHook.java | 15 ++- .../hook/exception/WrongBranchException.java | 13 ++- .../handler/hook/WriteLatexPutHookTest.java | 104 +++++++++++++++++- .../exception/WrongBranchExceptionTest.java | 29 +++++ 4 files changed, 152 insertions(+), 9 deletions(-) create mode 100644 services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/git/handler/hook/exception/WrongBranchExceptionTest.java diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/git/handler/hook/WriteLatexPutHook.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/git/handler/hook/WriteLatexPutHook.java index 4137a9240d..f5bda49823 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/git/handler/hook/WriteLatexPutHook.java +++ b/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 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); } } diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/git/handler/hook/exception/WrongBranchException.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/git/handler/hook/exception/WrongBranchException.java index e216781f12..22dac09a4f 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/git/handler/hook/exception/WrongBranchException.java +++ b/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 getDescriptionLines() { - return Arrays.asList(DESCRIPTION_LINES); + return Arrays.asList( + "You can't push any new branches.", "Please use the " + branchName + " branch."); } @Override diff --git a/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/git/handler/hook/WriteLatexPutHookTest.java b/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/git/handler/hook/WriteLatexPutHookTest.java index 71746e06bf..28063b2317 100644 --- a/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/git/handler/hook/WriteLatexPutHookTest.java +++ b/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 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; + } } diff --git a/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/git/handler/hook/exception/WrongBranchExceptionTest.java b/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/git/handler/hook/exception/WrongBranchExceptionTest.java new file mode 100644 index 0000000000..c78414191e --- /dev/null +++ b/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 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 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)); + } +}