Disallow ref creation when passed object is neither Commit or Tag
The checkCreateRef works by throwing an AuthException if permission is
denied. It explicitly checks for RevCommit and RevTag, which means if
the object argument is neither no checks are made (ie. ref creation is
allowed), which is not desired.
Google-Bug-Id: b/355739238
Release-Notes: Disallow ref creation when passed object is neither Commit or Tag
Change-Id: Ie4bdd12c6c76ae78e01f926cb69c142a539d0e75
diff --git a/java/com/google/gerrit/server/project/CreateRefControl.java b/java/com/google/gerrit/server/project/CreateRefControl.java
index ab134b5..c3d3978 100644
--- a/java/com/google/gerrit/server/project/CreateRefControl.java
+++ b/java/com/google/gerrit/server/project/CreateRefControl.java
@@ -112,7 +112,9 @@
RefPermission.READ.describeForException()));
throw e;
}
- } else if (object instanceof RevTag) {
+ return;
+ }
+ if (object instanceof RevTag) {
RevTag tag = (RevTag) object;
try (RevWalk rw = new RevWalk(repo)) {
rw.parseBody(tag);
@@ -145,7 +147,11 @@
} else {
forRef.check(RefPermission.CREATE_TAG);
}
+ return;
}
+ throw new AuthException(
+ String.format(
+ "Ref creation not allowed. Object %s is neither Commit or Tag.", object.getId()));
}
/**
diff --git a/javatests/com/google/gerrit/acceptance/git/PushPermissionsIT.java b/javatests/com/google/gerrit/acceptance/git/PushPermissionsIT.java
index f94aa12..fa6d2e4 100644
--- a/javatests/com/google/gerrit/acceptance/git/PushPermissionsIT.java
+++ b/javatests/com/google/gerrit/acceptance/git/PushPermissionsIT.java
@@ -45,6 +45,7 @@
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.transport.PushResult;
import org.eclipse.jgit.transport.RefSpec;
import org.eclipse.jgit.transport.RemoteRefUpdate;
@@ -405,6 +406,33 @@
assertThat(r).hasProcessed(ImmutableMap.of("refs", 1));
}
+ @Test
+ public void pushTreeIsNotAllowed() throws Exception {
+ RevCommit commit = testRepo.branch("HEAD").commit().create();
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(allow(Permission.CREATE).ref("refs/*").group(REGISTERED_USERS))
+ .add(allow(Permission.PUSH).ref("refs/*").group(REGISTERED_USERS))
+ .update();
+
+ // We use "refs/main" instead of "refs/heads/main", because the latter only allows commits.
+ {
+ // An extra colon (:) makes it a tree reference
+ PushResult r = push(commit.getId().getName() + "::refs/main");
+ RemoteRefUpdate refUpdate = r.getRemoteUpdates().stream().findFirst().get();
+ assertThat(refUpdate.getStatus()).isEqualTo(Status.REJECTED_OTHER_REASON);
+ assertThat(refUpdate.getMessage()).contains("is neither Commit or Tag");
+ }
+
+ {
+ PushResult r = push(commit.getTree().getId().getName() + ":refs/main");
+ RemoteRefUpdate refUpdate = r.getRemoteUpdates().stream().findFirst().get();
+ assertThat(refUpdate.getStatus()).isEqualTo(Status.REJECTED_OTHER_REASON);
+ assertThat(refUpdate.getMessage()).contains("is neither Commit or Tag");
+ }
+ }
+
private static void removeAllBranchPermissions(ProjectConfig cfg, String... permissions) {
for (AccessSection s : cfg.getAccessSections()) {
if (s.getName().startsWith("refs/heads/")
