Validate integrate URL prefixes on URL boundaries

java/com/google/copybara/git/GitHubPrIntegrateLabel.java

@@ -123,7 +123,7 @@ static Optional<IntegrateLabel> maybeParse(
     if (label == null) {
       return Optional.empty();
     }
-    if (allowedRefPrefixes.stream().anyMatch(label.gitHubIdentifier.getUrl()::startsWith)
+    if (IntegrateUrlPrefixes.matchesAny(label.gitHubIdentifier.getUrl(), allowedRefPrefixes)
         || label.matchesDestinationUrl(repoUrl)) {
       return Optional.of(label);
     }

java/com/google/copybara/git/GitModule.java

@@ -543,14 +543,24 @@ public GitIntegrateChanges integrate(
       Boolean allowProtocolRef,
       Sequence<?> allowedRefPrefixes)
       throws EvalException {
+    ImmutableList<String> prefixes =
+        ImmutableList.copyOf(
+            Sequence.cast(allowedRefPrefixes, String.class, "allowed_ref_prefixes"));
+    for (String prefix : prefixes) {
+      try {
+        IntegrateUrlPrefixes.validateAllowedRefPrefix(prefix);
+      } catch (ValidationException e) {
+        throw new EvalException(e.getMessage(), e);
+      }
+    }
     return new GitIntegrateChanges(
         label,
         stringToEnum("strategy", strategy, Strategy.class),
         ignoreErrors,
         allowUnrelatedHistory != null && allowUnrelatedHistory,
         new LabelTemplate(mergeCommitMessage),
         allowProtocolRef != null && allowProtocolRef,
-        Sequence.cast(allowedRefPrefixes, String.class, "allowed_ref_prefixes"));
+        prefixes);
   }
 
   @SuppressWarnings("unused")

java/com/google/copybara/git/IntegrateLabel.java

@@ -16,13 +16,16 @@
 
 package com.google.copybara.git;
 
+import com.google.common.base.Ascii;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import com.google.copybara.ChangeMessage;
 import com.google.copybara.GeneralOptions;
 import com.google.copybara.LabelFinder;
 import com.google.copybara.exception.RepoException;
 import com.google.copybara.exception.ValidationException;
+import java.net.URI;
+import java.net.URISyntaxException;
 import java.util.List;
 import java.util.Optional;
 
@@ -67,7 +70,7 @@ static IntegrateLabel genericGitRevision(
                   rawLabel, repoUrl));
         }
         ValidationException.checkCondition(
-            allowedRefPrefixes.stream().anyMatch(labelUrl::startsWith),
+            IntegrateUrlPrefixes.matchesAny(labelUrl, allowedRefPrefixes),
             "Integrate URL in '%s' does not match destination repository URL '%s' or any allowed"
                 + " prefix (%s).",
             rawLabel,
@@ -120,3 +123,120 @@ static String withLabels(String msg, ImmutableList<LabelFinder> labelsToAdd)
     return result.toString();
   }
 }
+
+/**
+ * Matches an integrate label URL against the URL prefixes allowed by the configuration.
+ *
+ * <p>Matching is done on URL boundaries instead of plain string prefixes, so a prefix of {@code
+ * https://github.com} does not allow {@code https://github.com.evil.example}, and a prefix of
+ * {@code https://github.com/google} does not allow {@code https://github.com/google-evil}.
+ */
+final class IntegrateUrlPrefixes {
+
+  private IntegrateUrlPrefixes() {}
+
+  /** Returns true if {@code url} is contained in at least one of {@code allowedRefPrefixes}. */
+  static boolean matchesAny(String url, Iterable<String> allowedRefPrefixes)
+      throws ValidationException {
+    for (String prefix : allowedRefPrefixes) {
+      if (matches(url, prefix)) {
+        return true;
+      }
+    }
+    return false;
+  }
+
+  /**
+   * Returns true if {@code url} is contained in {@code prefix} on a URL boundary. Both values are
+   * parsed as URIs, and the scheme, host, effective port and path are compared.
+   */
+  static boolean matches(String url, String prefix) throws ValidationException {
+    URI urlUri = parseUrl(url);
+    URI prefixUri = parseAllowedRefPrefix(prefix);
+    return equalsIgnoreCase(urlUri.getScheme(), prefixUri.getScheme())
+        && equalsIgnoreCase(urlUri.getHost(), prefixUri.getHost())
+        && effectivePort(urlUri) == effectivePort(prefixUri)
+        && pathMatches(urlUri.getPath(), prefixUri.getPath());
+  }
+
+  /**
+   * Validates an allowed prefix used in the configuration.
+   *
+   * <p>User information, queries and fragments are rejected because they are not part of a
+   * repository identity and would make prefix matching ambiguous.
+   */
+  static void validateAllowedRefPrefix(String prefix) throws ValidationException {
+    parseAllowedRefPrefix(prefix);
+  }
+
+  private static URI parseAllowedRefPrefix(String prefix) throws ValidationException {
+    ValidationException.checkCondition(
+        prefix != null && !prefix.trim().isEmpty(),
+        "Allowed integrate URL prefix cannot be empty.");
+    URI uri;
+    try {
+      uri = new URI(prefix.trim());
+    } catch (URISyntaxException e) {
+      throw new ValidationException(
+          String.format("Invalid allowed integrate URL prefix '%s': %s", prefix, e.getMessage()),
+          e);
+    }
+    ValidationException.checkCondition(
+        uri.isAbsolute() && !uri.isOpaque(),
+        "Allowed integrate URL prefix '%s' must be an absolute hierarchical URL.",
+        prefix);
+    ValidationException.checkCondition(
+        uri.getRawUserInfo() == null,
+        "Allowed integrate URL prefix '%s' must not contain user information.",
+        prefix);
+    ValidationException.checkCondition(
+        uri.getRawQuery() == null && uri.getRawFragment() == null,
+        "Allowed integrate URL prefix '%s' must not contain a query or fragment.",
+        prefix);
+    return uri;
+  }
+
+  private static URI parseUrl(String url) throws ValidationException {
+    try {
+      return new URI(url);
+    } catch (URISyntaxException e) {
+      throw new ValidationException(
+          String.format("Invalid integrate URL '%s': %s", url, e.getMessage()), e);
+    }
+  }
+
+  private static boolean pathMatches(String urlPath, String prefixPath) {
+    String normalizedUrlPath = urlPath == null ? "" : urlPath;
+    String normalizedPrefixPath = prefixPath == null ? "" : prefixPath;
+    if (normalizedUrlPath.equals(normalizedPrefixPath)) {
+      return true;
+    }
+    if (!normalizedUrlPath.startsWith(normalizedPrefixPath)) {
+      return false;
+    }
+    return normalizedPrefixPath.endsWith("/")
+        || normalizedUrlPath.charAt(normalizedPrefixPath.length()) == '/';
+  }
+
+  private static boolean equalsIgnoreCase(String left, String right) {
+    if (left == null || right == null) {
+      return left == right;
+    }
+    return Ascii.equalsIgnoreCase(left, right);
+  }
+
+  private static int effectivePort(URI uri) {
+    if (uri.getPort() != -1) {
+      return uri.getPort();
+    }
+    String scheme = uri.getScheme();
+    if (scheme == null) {
+      return -1;
+    }
+    return switch (Ascii.toLowerCase(scheme)) {
+      case "http" -> 80;
+      case "https" -> 443;
+      default -> -1;
+    };
+  }
+}

javatests/com/google/copybara/git/GitHubPrIntegrateLabelTest.java

@@ -205,6 +205,50 @@ public void maybeParse_nonGitHubDestinationUrl_allowedWithAllowedRefPrefixes() t
     assertThat(parsedMatchingDest.get().toString()).isEqualTo(rawLabel);
   }
 
+  @Test
+  public void maybeParse_hostSuffixSpoof_rejected() throws Exception {
+    GeneralOptions options =
+        new GeneralOptions(ImmutableMap.of(), FileSystems.getDefault(), console);
+    String rawLabel = "https://github.com.evil.example/foo/bar/pull/18 from copybarrista:main";
+
+    assertThrows(
+        ValidationException.class,
+        () ->
+            GitHubPrIntegrateLabel.maybeParse(
+                rawLabel, repo, "rpc://foo", options, ImmutableList.of("https://github.com")));
+  }
+
+  @Test
+  public void maybeParse_orgSuffixSpoof_rejected() throws Exception {
+    GeneralOptions options =
+        new GeneralOptions(ImmutableMap.of(), FileSystems.getDefault(), console);
+    String rawLabel = "https://github.com/foo-evil/bar/pull/18 from copybarrista:main";
+
+    assertThrows(
+        ValidationException.class,
+        () ->
+            GitHubPrIntegrateLabel.maybeParse(
+                rawLabel,
+                repo,
+                "rpc://foo",
+                options,
+                ImmutableList.of("https://github.com/foo")));
+  }
+
+  @Test
+  public void maybeParse_allowedOrgPrefix() throws Exception {
+    GeneralOptions options =
+        new GeneralOptions(ImmutableMap.of(), FileSystems.getDefault(), console);
+    String rawLabel = "https://github.com/foo/bar/pull/18 from copybarrista:main";
+
+    Optional<IntegrateLabel> parsed =
+        GitHubPrIntegrateLabel.maybeParse(
+            rawLabel, repo, "rpc://foo", options, ImmutableList.of("https://github.com/foo"));
+
+    assertThat(parsed).isPresent();
+    assertThat(parsed.get().toString()).isEqualTo(rawLabel);
+  }
+
   @Test
   public void parseLabel_hyphenatedUser() throws Exception {
     GeneralOptions options =
@@ -232,4 +276,134 @@ public void parseLabel_sha256() throws Exception {
                 options))
         .isNotNull();
   }
+
+  @Test
+  public void matches_hostSuffixSpoof_rejected() throws Exception {
+    assertThat(
+            IntegrateUrlPrefixes.matches(
+                "https://github.com.evil.example/google/repo", "https://github.com"))
+        .isFalse();
+  }
+
+  @Test
+  public void matches_orgSuffixSpoof_rejected() throws Exception {
+    assertThat(
+            IntegrateUrlPrefixes.matches(
+                "https://github.com/google-evil/repo", "https://github.com/google"))
+        .isFalse();
+  }
+
+  @Test
+  public void matches_schemeMismatch_rejected() throws Exception {
+    assertThat(
+            IntegrateUrlPrefixes.matches(
+                "https://github.com/google/repo", "http://github.com/google"))
+        .isFalse();
+    assertThat(
+            IntegrateUrlPrefixes.matches(
+                "http://github.com/google/repo", "https://github.com/google"))
+        .isFalse();
+  }
+
+  @Test
+  public void matches_hostMismatch_rejected() throws Exception {
+    assertThat(
+            IntegrateUrlPrefixes.matches(
+                "https://github.com/google/repo", "https://gitlab.com/google"))
+        .isFalse();
+  }
+
+  @Test
+  public void matches_emptyPrefix_throws() {
+    ValidationException thrown =
+        assertThrows(
+            ValidationException.class,
+            () -> IntegrateUrlPrefixes.matches("https://github.com/google/repo", ""));
+
+    assertThat(thrown).hasMessageThat().contains("Allowed integrate URL prefix cannot be empty");
+  }
+
+  @Test
+  public void matches_pathPrefixWithoutSlashBoundary_rejected() throws Exception {
+    assertThat(
+            IntegrateUrlPrefixes.matches(
+                "https://github.com/google/repo", "https://github.com/goog"))
+        .isFalse();
+    assertThat(
+            IntegrateUrlPrefixes.matches(
+                "https://github.com/google/repo", "https://github.com/google/repo-extra"))
+        .isFalse();
+  }
+
+  @Test
+  public void matches_exactRepo_allowed() throws Exception {
+    assertThat(
+            IntegrateUrlPrefixes.matches(
+                "https://github.com/google/repo", "https://github.com/google/repo"))
+        .isTrue();
+  }
+
+  @Test
+  public void matches_allowedOrgPrefix_allowed() throws Exception {
+    assertThat(
+            IntegrateUrlPrefixes.matches(
+                "https://github.com/google/repo", "https://github.com/google"))
+        .isTrue();
+    assertThat(
+            IntegrateUrlPrefixes.matches(
+                "https://github.com/google/repo", "https://github.com/google/"))
+        .isTrue();
+    assertThat(IntegrateUrlPrefixes.matches("https://github.com/google/repo", "https://github.com"))
+        .isTrue();
+  }
+
+  @Test
+  public void matches_hostIsCaseInsensitive() throws Exception {
+    assertThat(
+            IntegrateUrlPrefixes.matches(
+                "https://GitHub.com/google/repo", "https://github.com/google"))
+        .isTrue();
+  }
+
+  @Test
+  public void matches_defaultPortsAreNormalized() throws Exception {
+    assertThat(
+            IntegrateUrlPrefixes.matches(
+                "https://github.com:443/google/repo", "https://github.com/google"))
+        .isTrue();
+    assertThat(
+            IntegrateUrlPrefixes.matches(
+                "https://github.com:8443/google/repo", "https://github.com/google"))
+        .isFalse();
+    assertThat(
+            IntegrateUrlPrefixes.matches(
+                "https://github.com/google/repo", "https://github.com:8443/google"))
+        .isFalse();
+    assertThat(
+            IntegrateUrlPrefixes.matches(
+                "https://github.com:8443/google/repo", "https://github.com:8443/google"))
+        .isTrue();
+    assertThat(
+            IntegrateUrlPrefixes.matches(
+                "https://github.com:443/google/repo", "https://github.com:443/google"))
+        .isTrue();
+  }
+
+  @Test
+  public void matches_invalidAllowedPrefixes_throw(
+      @TestParameter({
+            "https://github.com/google?owner=evil",
+            "https://github.com/google#fragment",
+            "https://user:pass@github.com/google",
+            "github.com/google",
+            "relative/path"
+          })
+          String prefix) {
+    ValidationException thrown =
+        assertThrows(
+            ValidationException.class,
+            () -> IntegrateUrlPrefixes.matches("https://github.com/google/repo", prefix));
+
+    assertThat(thrown).hasMessageThat().contains("Allowed integrate URL prefix");
+  }
 }