Reject invalid label ranges for permission when pushing a project.config file
It should not be possible to add permissions with a range where min is
greater than max, where min is present but max is missing or where max
is present but min is missing.
Adding a permission where min is present but max is missing or where max
is present but min is missing was already not possible, but we didn't
have tests for this.
Release-Notes: Invalid label ranges for permissions are rejected when pushing a project.config file
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: Ia6dfd182b18eeb41880f26da1d93374693446699
Bug: Issue Google b/260961794
Forward-Compatible: checked
diff --git a/java/com/google/gerrit/entities/PermissionRule.java b/java/com/google/gerrit/entities/PermissionRule.java
index 9a2d31e..1665c1c 100644
--- a/java/com/google/gerrit/entities/PermissionRule.java
+++ b/java/com/google/gerrit/entities/PermissionRule.java
@@ -202,6 +202,9 @@
int dotdot = range.indexOf("..");
int min = parseInt(range.substring(0, dotdot));
int max = parseInt(range.substring(dotdot + 2));
+ if (min > max) {
+ throw new IllegalArgumentException("Invalid range in rule: " + orig);
+ }
rule.setRange(min, max);
} else {
throw new IllegalArgumentException("Invalid range in rule: " + orig);
diff --git a/javatests/com/google/gerrit/acceptance/api/project/ProjectConfigIT.java b/javatests/com/google/gerrit/acceptance/api/project/ProjectConfigIT.java
index 5fd2159..28a0196 100644
--- a/javatests/com/google/gerrit/acceptance/api/project/ProjectConfigIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/project/ProjectConfigIT.java
@@ -18,6 +18,7 @@
import static com.google.gerrit.acceptance.GitUtil.fetch;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
@@ -32,6 +33,8 @@
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.ChangeInput;
import com.google.gerrit.git.ObjectIds;
+import com.google.gerrit.server.group.SystemGroupBackend;
+import com.google.gerrit.server.project.GroupList;
import com.google.gerrit.server.project.LabelConfigValidator;
import com.google.gerrit.server.project.ProjectConfig;
import com.google.inject.Inject;
@@ -155,6 +158,81 @@
}
@Test
+ public void rejectCreatingLabelPermissionWithInvalidRange_minGreaterThanMax() throws Exception {
+ fetchRefsMetaConfig();
+ PushOneCommit push =
+ pushFactory.create(
+ admin.newIdent(),
+ testRepo,
+ "Test Change",
+ ImmutableMap.of(
+ ProjectConfig.PROJECT_CONFIG,
+ "[access \"refs/heads/*\"]\n label-Code-Review = 1..-1 group Registered-Users",
+ GroupList.FILE_NAME,
+ String.format("%s\tRegistered-Users", SystemGroupBackend.REGISTERED_USERS.get())));
+ PushOneCommit.Result r = push.to(RefNames.REFS_CONFIG);
+ r.assertErrorStatus(
+ String.format("commit %s: invalid project configuration", abbreviateName(r.getCommit())));
+ r.assertMessage(
+ String.format(
+ "ERROR: commit %s: invalid project configuration:\n"
+ + "ERROR: commit %s: project.config: invalid rule in"
+ + " access.refs/heads/*.label-Code-Review:"
+ + " invalid range in rule: 1..-1 group Registered-Users",
+ abbreviateName(r.getCommit()), abbreviateName(r.getCommit())));
+ }
+
+ @Test
+ public void rejectCreatingLabelPermissionWithInvalidRange_minSetMaxMissing() throws Exception {
+ fetchRefsMetaConfig();
+ PushOneCommit push =
+ pushFactory.create(
+ admin.newIdent(),
+ testRepo,
+ "Test Change",
+ ImmutableMap.of(
+ ProjectConfig.PROJECT_CONFIG,
+ "[access \"refs/heads/*\"]\n label-Code-Review = -1.. group Registered-Users",
+ GroupList.FILE_NAME,
+ String.format("%s\tRegistered-Users", SystemGroupBackend.REGISTERED_USERS.get())));
+ PushOneCommit.Result r = push.to(RefNames.REFS_CONFIG);
+ r.assertErrorStatus(
+ String.format("commit %s: invalid project configuration", abbreviateName(r.getCommit())));
+ r.assertMessage(
+ String.format(
+ "ERROR: commit %s: invalid project configuration:\n"
+ + "ERROR: commit %s: project.config: invalid rule in"
+ + " access.refs/heads/*.label-Code-Review:"
+ + " invalid range in rule: -1.. group Registered-Users",
+ abbreviateName(r.getCommit()), abbreviateName(r.getCommit())));
+ }
+
+ @Test
+ public void rejectCreatingLabelPermissionWithInvalidRange_maxSetMinMissing() throws Exception {
+ fetchRefsMetaConfig();
+ PushOneCommit push =
+ pushFactory.create(
+ admin.newIdent(),
+ testRepo,
+ "Test Change",
+ ImmutableMap.of(
+ ProjectConfig.PROJECT_CONFIG,
+ "[access \"refs/heads/*\"]\n label-Code-Review = ..1 group Registered-Users",
+ GroupList.FILE_NAME,
+ String.format("%s\tRegistered-Users", SystemGroupBackend.REGISTERED_USERS.get())));
+ PushOneCommit.Result r = push.to(RefNames.REFS_CONFIG);
+ r.assertErrorStatus(
+ String.format("commit %s: invalid project configuration", abbreviateName(r.getCommit())));
+ r.assertMessage(
+ String.format(
+ "ERROR: commit %s: invalid project configuration:\n"
+ + "ERROR: commit %s: project.config: invalid rule in"
+ + " access.refs/heads/*.label-Code-Review:"
+ + " invalid range in rule: ..1 group Registered-Users",
+ abbreviateName(r.getCommit()), abbreviateName(r.getCommit())));
+ }
+
+ @Test
public void rejectSettingCopyMinScore() throws Exception {
testRejectSettingLabelFlag(
LabelConfigValidator.KEY_COPY_MIN_SCORE, /* value= */ true, "is:MIN");
