Disallow editing the Change-Id during inline edits

Editing an existing Change-Id is checked when a new patch-set
is uploaded via Git protocol. However, when the commit message
is modified through an inline edit, the commit is created
directly on the Git repository and therefore any change was
allowed, including the editing of the Change-Id.

Return HTTP status code 409 (conflict) error when editing Change-Id
which is consistent to the error raised when the same operation
is attempted on more recent versions of the inline edit with
PolyGerrit.

Bug: Issue 13931
Change-Id: I6ba424e09d5a38fd474bf3b2373ff3209b7baf74

2 files changed, 43 insertions, 1 deletions

diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/edit/ChangeEditIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/edit/ChangeEditIT.java
index 9cb0b318b1..82f91cbe20 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/edit/ChangeEditIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/edit/ChangeEditIT.java
@@ -329,6 +329,33 @@ public class ChangeEditIT extends AbstractDaemonTest {
   }
 
   @Test
+  public void updateMessageEditChangeIdShouldThrowResourceConflictException() throws Exception {
+    createEmptyEditFor(changeId);
+    String commitMessage = gApi.changes().id(changeId).edit().getCommitMessage();
+
+    exception.expect(ResourceConflictException.class);
+    exception.expectMessage("Editing of the Change-Id footer is not allowed");
+    gApi.changes()
+        .id(changeId)
+        .edit()
+        .modifyCommitMessage(commitMessage.replaceAll(changeId, changeId2));
+  }
+
+  @Test
+  public void updateMessageEditRemoveChangeIdShouldThrowResourceConflictException()
+      throws Exception {
+    createEmptyEditFor(changeId);
+    String commitMessage = gApi.changes().id(changeId).edit().getCommitMessage();
+
+    exception.expect(ResourceConflictException.class);
+    exception.expectMessage("Editing of the Change-Id footer is not allowed");
+    gApi.changes()
+        .id(changeId)
+        .edit()
+        .modifyCommitMessage(commitMessage.replaceAll("(Change-Id:).*", ""));
+  }
+
+  @Test
   public void updateMessage() throws Exception {
     createEmptyEditFor(changeId);
     String msg = String.format("New commit message\n\nChange-Id: %s\n", changeId);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditModifier.java b/gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditModifier.java
index 3d1fb79a4e..43b8d5dfa1 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditModifier.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditModifier.java
@@ -17,10 +17,12 @@ package com.google.gerrit.server.edit;
 import static com.google.common.base.Preconditions.checkState;
 
 import com.google.common.base.Strings;
+import com.google.gerrit.common.FooterConstants;
 import com.google.gerrit.common.TimeUtil;
 import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.extensions.restapi.MergeConflictException;
 import com.google.gerrit.extensions.restapi.RawInput;
+import com.google.gerrit.extensions.restapi.ResourceConflictException;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.reviewdb.client.RefNames;
@@ -194,10 +196,12 @@ public class ChangeEditModifier {
    * @param newCommitMessage the new commit message
    * @throws AuthException if the user isn't authenticated or not allowed to use change edits
    * @throws UnchangedCommitMessageException if the commit message is the same as before
+   * @throws ResourceConflictException if the commit message has a Change-Id modification
    */
   public void modifyMessage(
       Repository repository, ChangeControl changeControl, String newCommitMessage)
-      throws AuthException, IOException, UnchangedCommitMessageException, OrmException {
+      throws AuthException, IOException, UnchangedCommitMessageException, OrmException,
+          ResourceConflictException {
     ensureAuthenticatedAndPermitted(changeControl);
     newCommitMessage = getWellFormedCommitMessage(newCommitMessage);
 
@@ -217,6 +221,17 @@ public class ChangeEditModifier {
     ObjectId newEditCommit =
         createCommit(repository, basePatchSetCommit, baseTree, newCommitMessage, nowTimestamp);
 
+    if (changeControl.getProjectControl().getProjectState().isRequireChangeID()) {
+      try (RevWalk revWalk = new RevWalk(repository)) {
+        if (!revWalk
+            .parseCommit(newEditCommit)
+            .getFooterLines(FooterConstants.CHANGE_ID)
+            .contains(changeControl.getChange().getKey().get())) {
+          throw new ResourceConflictException("Editing of the Change-Id footer is not allowed");
+        }
+      }
+    }
+
     if (optionalChangeEdit.isPresent()) {
       updateEditReference(repository, optionalChangeEdit.get(), newEditCommit, nowTimestamp);
     } else {