Submit requirements: always hide applicability expressions on the API
Applicability expressions migth contain sensitive information that we
don't want to reveal, for example branch names. With this change, we're
always hiding the applicability expressions (expression, passing/failing
atoms) on the API.
Bug: Google b/204960013
Change-Id: I4ee34a1635836202b92b63e354aa3ec986485b83
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index 0a203cc..33e7804 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -8263,19 +8263,19 @@
The `SubmitRequirementExpressionInfo` describes the result of evaluating a
single submit requirement expression, for example `label:code-review=+2`.
-[options="header",cols="1,6"]
+[options="header",cols="1,^1,5"]
|===========================
-|Field Name |Description
-|`expression`|
+|Field Name ||Description
+|`expression`|optional|
The submit requirement expression as a string, for example
`branch:refs/heads/foo and label:verified=+1`.
-|`fulfilled`|
+|`fulfilled`||
True if the submit requirement is fulfilled for the change.
-|`passing_atoms`|
+|`passing_atoms`|optional|
A list of passing atoms as strings. For the above expression,
`passing_atoms` can contain ["branch:refs/heads/foo"] if the branch predicate is
fulfilled for the change.
-|`failing_atoms`|
+|`failing_atoms`|optional|
A list of failing atoms. This is similar to `passing_atoms` except that it
contains the list of predicates that are not fulfilled for the change.
|===========================
@@ -8329,6 +8329,8 @@
A link:#submit-requirement-expression-info[SubmitRequirementExpressionInfo]
containing the result of evaluating the applicability expression. Not set if the
submit requirement did not define an applicability expression.
+Note that fields `expression`, `passing_atoms` and `failing_atoms` are always
+omitted for the `applicability_expression_result`.
|`submittability_expression_result`||
A link:#submit-requirement-expression-info[SubmitRequirementExpressionInfo]
containing the result of evaluating the submittability expression.
diff --git a/java/com/google/gerrit/server/change/SubmitRequirementsJson.java b/java/com/google/gerrit/server/change/SubmitRequirementsJson.java
index 8eeec62..4833197 100644
--- a/java/com/google/gerrit/server/change/SubmitRequirementsJson.java
+++ b/java/com/google/gerrit/server/change/SubmitRequirementsJson.java
@@ -36,28 +36,36 @@
if (req.applicabilityExpression().isPresent()) {
info.applicabilityExpressionResult =
submitRequirementExpressionToInfo(
- req.applicabilityExpression().get(), result.applicabilityExpressionResult().get());
+ req.applicabilityExpression().get(),
+ result.applicabilityExpressionResult().get(),
+ /* hide= */ true); // Always hide applicability expressions on the API
}
if (req.overrideExpression().isPresent()) {
info.overrideExpressionResult =
submitRequirementExpressionToInfo(
- req.overrideExpression().get(), result.overrideExpressionResult().get());
+ req.overrideExpression().get(),
+ result.overrideExpressionResult().get(),
+ /* hide= */ false);
}
info.submittabilityExpressionResult =
submitRequirementExpressionToInfo(
- req.submittabilityExpression(), result.submittabilityExpressionResult());
+ req.submittabilityExpression(),
+ result.submittabilityExpressionResult(),
+ /* hide= */ false);
info.status = SubmitRequirementResultInfo.Status.valueOf(result.status().toString());
info.isLegacy = result.isLegacy();
return info;
}
private static SubmitRequirementExpressionInfo submitRequirementExpressionToInfo(
- SubmitRequirementExpression expression, SubmitRequirementExpressionResult result) {
+ SubmitRequirementExpression expression,
+ SubmitRequirementExpressionResult result,
+ boolean hide) {
SubmitRequirementExpressionInfo info = new SubmitRequirementExpressionInfo();
- info.expression = expression.expressionString();
+ info.expression = hide ? null : expression.expressionString();
info.fulfilled = result.status().equals(SubmitRequirementExpressionResult.Status.PASS);
- info.passingAtoms = result.passingAtoms();
- info.failingAtoms = result.failingAtoms();
+ info.passingAtoms = hide ? null : result.passingAtoms();
+ info.failingAtoms = hide ? null : result.failingAtoms();
return info;
}
}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 74407c0..50098d7 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -89,6 +89,7 @@
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
import com.google.gerrit.common.FooterConstants;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.RawInputUtil;
import com.google.gerrit.common.data.GlobalCapability;
import com.google.gerrit.entities.Account;
@@ -153,6 +154,7 @@
import com.google.gerrit.extensions.common.LegacySubmitRequirementInfo;
import com.google.gerrit.extensions.common.RevisionInfo;
import com.google.gerrit.extensions.common.SubmitRecordInfo;
+import com.google.gerrit.extensions.common.SubmitRequirementExpressionInfo;
import com.google.gerrit.extensions.common.SubmitRequirementInput;
import com.google.gerrit.extensions.common.SubmitRequirementResultInfo;
import com.google.gerrit.extensions.common.SubmitRequirementResultInfo.Status;
@@ -4934,6 +4936,35 @@
}
@Test
+ @GerritConfig(
+ name = "experiments.enabled",
+ value = ExperimentFeaturesConstants.GERRIT_BACKEND_REQUEST_FEATURE_ENABLE_SUBMIT_REQUIREMENTS)
+ public void submitRequirement_applicabilityExpressionIsAlwaysHidden() throws Exception {
+ configSubmitRequirement(
+ project,
+ SubmitRequirement.builder()
+ .setName("Code-Review")
+ .setApplicabilityExpression(SubmitRequirementExpression.of("branch:refs/heads/master"))
+ .setSubmittabilityExpression(SubmitRequirementExpression.create("label:Code-Review=+2"))
+ .setAllowOverrideInChildProjects(false)
+ .build());
+
+ PushOneCommit.Result r = createChange();
+ String changeId = r.getChangeId();
+
+ voteLabel(changeId, "Code-Review", 2);
+ ChangeInfo changeInfo = gApi.changes().id(changeId).get();
+ SubmitRequirementResultInfo requirement =
+ changeInfo.submitRequirements.stream().collect(MoreCollectors.onlyElement());
+ assertSubmitRequirementExpression(
+ requirement.applicabilityExpressionResult,
+ /* expression= */ null,
+ /* passingAtoms= */ null,
+ /* failingAtoms= */ null,
+ /* fulfilled= */ true);
+ }
+
+ @Test
public void submitRequirements_notServedIfExperimentNotEnabled() throws Exception {
configSubmitRequirement(
project,
@@ -5550,6 +5581,26 @@
.collect(toImmutableList())));
}
+ private void assertSubmitRequirementExpression(
+ SubmitRequirementExpressionInfo result,
+ @Nullable String expression,
+ @Nullable List<String> passingAtoms,
+ @Nullable List<String> failingAtoms,
+ boolean fulfilled) {
+ assertThat(result.expression).isEqualTo(expression);
+ if (passingAtoms == null) {
+ assertThat(result.passingAtoms).isNull();
+ } else {
+ assertThat(result.passingAtoms).containsExactlyElementsIn(passingAtoms);
+ }
+ if (failingAtoms == null) {
+ assertThat(result.failingAtoms).isNull();
+ } else {
+ assertThat(result.failingAtoms).containsExactlyElementsIn(failingAtoms);
+ }
+ assertThat(result.fulfilled).isEqualTo(fulfilled);
+ }
+
private Project.NameKey createProjectForPush(SubmitType submitType) throws Exception {
Project.NameKey project = projectOperations.newProject().submitType(submitType).create();
projectOperations
