commit 62e0be5127a2dd33e15dc954974ce95768498719
author Luca Milanesio <luca.milanesio@gmail.com> Thu Sep 16 00:09:15 2021 +0100
committer Luca Milanesio <luca.milanesio@gmail.com> Thu Sep 23 00:35:08 2021 +0100
tree ce230eb97097ab9da3b3e9c303386e4d244dc15e
parent 59e701d960f7cce36522230b994bb19169081146

Filter out edit refs

User's private edits are not part of an open change
and should not be served or advertised.

The refs could be in theory made invisible by playing
with the ACLs. However, having a complex ACL would have
a detrimental impact on the overall performance of
the security evaluation. It does not make sense to
expose the user's private edits and they would risk
to make a clone failing with a wants-not-valid error.

Change-Id: Idbae7ed339515daa44df3f0093a51f6343b4e8d5

README.md

diff --git a/README.md b/README.md
index e8c373c..3cecec3 100644
--- a/README.md
+++ b/README.md
@@ -35,6 +35,7 @@
 - Abandoned changes and all their patch-sets
 - Corrupted changes and all their patch-sets
 - All '/meta' refs of all changes
+- All non-published edits of any changes
 
 To enable a group of users of getting a "filtered list" of refs (e.g. CI jobs):
 - Define a new group of users (e.g. Builders)

src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/ForProjectWrapper.java

diff --git a/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/ForProjectWrapper.java b/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/ForProjectWrapper.java
index 6136498..2860870 100644
--- a/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/ForProjectWrapper.java
+++ b/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/ForProjectWrapper.java
@@ -17,6 +17,7 @@
 import com.google.common.flogger.FluentLogger;
 import com.google.gerrit.entities.Change;
 import com.google.gerrit.entities.Project;
+import com.google.gerrit.entities.RefNames;
 import com.google.gerrit.extensions.api.access.CoreOrPluginProjectPermission;
 import com.google.gerrit.extensions.conditions.BooleanCondition;
 import com.google.gerrit.extensions.restapi.AuthException;
@@ -75,11 +76,15 @@
   @Override
   public Collection<Ref> filter(Collection<Ref> refs, Repository repo, RefFilterOptions opts)
       throws PermissionBackendException {
-    return defaultForProject.filter(refs, repo, opts).parallelStream()
+    return defaultForProject
+        .filter(refs, repo, opts)
+        .parallelStream()
+        .filter((ref) -> !ref.getName().startsWith(RefNames.REFS_USERS))
         .filter(
             (ref) -> {
               String refName = ref.getName();
-              return (!isChangeRef(refName) || (!isChangeMetaRef(refName) && isOpen(repo, refName)));
+              return (!isChangeRef(refName)
+                  || (!isChangeMetaRef(refName) && isOpen(repo, refName)));
             })
         .collect(Collectors.toList());
   }

src/test/java/com/googlesource/gerrit/libmodule/plugins/test/GitRefsFilterTest.java

diff --git a/src/test/java/com/googlesource/gerrit/libmodule/plugins/test/GitRefsFilterTest.java b/src/test/java/com/googlesource/gerrit/libmodule/plugins/test/GitRefsFilterTest.java
index 854ac6e..d9d458b 100644
--- a/src/test/java/com/googlesource/gerrit/libmodule/plugins/test/GitRefsFilterTest.java
+++ b/src/test/java/com/googlesource/gerrit/libmodule/plugins/test/GitRefsFilterTest.java
@@ -16,10 +16,22 @@
 
 import static com.google.common.truth.Truth.assertThat;
 
+import com.google.gerrit.acceptance.GitUtil;
 import com.google.gerrit.acceptance.NoHttpd;
 import com.google.gerrit.acceptance.Sandboxed;
+import com.google.gerrit.acceptance.TestAccount;
+import com.google.gerrit.entities.RefNames;
 import com.google.inject.Module;
 import com.googlesource.gerrit.modules.gitrefsfilter.RefsFilterModule;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.eclipse.jgit.internal.storage.dfs.DfsRepositoryDescription;
+import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
+import org.eclipse.jgit.junit.TestRepository;
+import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.transport.FetchResult;
+import org.eclipse.jgit.util.FS;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -45,6 +57,19 @@
   }
 
   @Test
+  public void testUserWithFilterOutCapabilityShouldNotSeeUserEdits() throws Exception {
+    createChange();
+    int changeNum = changeNumOfRef(getChangesRefsAs(admin).get(0));
+    gApi.changes().id(changeNum).edit().create();
+
+    assertThat(
+            fetchAllRefs(user)
+                .filter((ref) -> ref.startsWith(RefNames.REFS_USERS))
+                .collect(Collectors.toSet()))
+        .isEmpty();
+  }
+
+  @Test
   public void testUserWithFilterOutCapabilityShouldSeeOpenChangesRefs() throws Exception {
     createChange();
 
@@ -57,4 +82,21 @@
 
     assertThat(getRefs(cloneProjectChangesRefs(admin))).isNotEmpty();
   }
+
+  protected Stream<String> fetchAllRefs(TestAccount testAccount) throws Exception {
+    DfsRepositoryDescription desc = new DfsRepositoryDescription("clone of " + project.get());
+
+    FS fs = FS.detect();
+    fs.setUserHome(null);
+
+    InMemoryRepository dest =
+        new InMemoryRepository.Builder().setRepositoryDescription(desc).setFS(fs).build();
+    Config cfg = dest.getConfig();
+    String uri = registerRepoConnection(project, testAccount);
+    cfg.setString("remote", "origin", "url", uri);
+    cfg.setString("remote", "origin", "fetch", "+refs/*:refs/remotes/origin/*");
+    TestRepository<InMemoryRepository> testRepo = GitUtil.newTestRepository(dest);
+    FetchResult result = testRepo.git().fetch().setRemote("origin").call();
+    return result.getAdvertisedRefs().stream().map(Ref::getName);
+  }
 }