repository: fix uri scheme restriction for ssh repo uris (#1226)

Author: benbroadaway

repository/src/main/java/com/walmartlabs/concord/repository/GitClient.java

@@ -300,10 +300,15 @@ private static String getRefSpec(Ref ref) {
         }
     }
 
-    private String updateUrl(String url, Secret secret) {
-        url = url.trim();
+    String updateUrl(String url, Secret secret) {
+        if (!url.matches("^[A-Za-z][A-Za-z0-9+.-]+://.*")) {
+            // no scheme. Assume ssh, e.g. from GitHub like '[email protected]:owner/repo.git'
+            assertUriAllowed("ssh://" + url);
 
-        URI uri = URI.create(url);
+            return url; // return un-modified. git cli doesn't want the ssh scheme
+        }
+
+        URI uri = assertUriAllowed(url);
 
         List<String> allowedDefaultHosts = cfg.authorizedGitHosts();
 
@@ -324,6 +329,39 @@ private String updateUrl(String url, Secret secret) {
         return "https://" + cfg.oauthToken() + "@" + url.substring("https://".length());
     }
 
+    private URI assertUriAllowed(String rawUri) {
+        // make sure it's in valid format
+        URI uri = URI.create(rawUri);
+        assertUriAllowed(uri);
+
+        return uri;
+    }
+
+    private void assertUriAllowed(URI uri) {
+        String providedScheme = uri.getScheme();
+        Set<String> allowedSchemes = cfg.allowedSchemes();
+        boolean hasScheme = providedScheme != null && (!providedScheme.isEmpty());
+
+        if (allowedSchemes.isEmpty()) {
+            return; // allow all
+        }
+
+        // the provided repo string is definitely an allowed protocol.
+        if (hasScheme && allowedSchemes.contains(providedScheme)) {
+            return;
+        }
+
+        // the provided repo string has no explicit scheme, should be understood to use an ssh connection
+        if (!hasScheme && uri.getUserInfo() != null) {
+            return;
+        }
+
+        String msg = String.format("Provided repository ('%s') contains an unsupported URI scheme: '%s'.",
+                uri, uri.getScheme());
+        log.warn(msg);
+        throw new RepositoryException(msg);
+    }
+
     private void updateSubmodules(Path workDir, Secret secret) {
         exec(Command.builder()
                 .workDir(workDir)

repository/src/main/java/com/walmartlabs/concord/repository/GitClientConfiguration.java

@@ -25,6 +25,7 @@
 import javax.annotation.Nullable;
 import java.time.Duration;
 import java.util.List;
+import java.util.Set;
 
 @Value.Immutable
 @Value.Style(jdkOnly = true)
@@ -36,6 +37,11 @@ public interface GitClientConfiguration {
     @Nullable
     List<String> authorizedGitHosts();
 
+    @Value.Default
+    default Set<String> allowedSchemes() {
+        return Set.of("https", "http", "ssh", "classpath");
+    }
+
     @Value.Default
     default Duration defaultOperationTimeout() {
         return Duration.ofMinutes(10L);

repository/src/test/java/com/walmartlabs/concord/repository/GitClientFetchTest.java

@@ -47,6 +47,7 @@ public class GitClientFetchTest {
     @BeforeEach
     public void init() {
         client = new GitClient(GitClientConfiguration.builder()
+                .addAllowedSchemes("file", "ssh")
                 .sshTimeout(Duration.ofMinutes(10))
                 .sshTimeoutRetryCount(1)
                 .httpLowSpeedLimit(1)

repository/src/test/java/com/walmartlabs/concord/repository/GitUriTest.java

@@ -0,0 +1,67 @@
+package com.walmartlabs.concord.repository;
+
+import com.walmartlabs.concord.common.secret.BinaryDataSecret;
+import com.walmartlabs.concord.sdk.Secret;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+class GitUriTest {
+
+    private static final GitClientConfiguration cfg = GitClientConfiguration.builder()
+            .oauthToken("mock-token")
+            .build();
+    private static final GitClient client = new GitClient(cfg);
+    private static Secret secret = new BinaryDataSecret(new byte[0]);
+
+    @Test
+    void testSsh() {
+        var sshWithUser = client.updateUrl("[email protected]:my-org/my-repo.git", null);
+        assertEquals("[email protected]:my-org/my-repo.git", sshWithUser);
+
+        var sshNoUser = client.updateUrl("gitserver.local:my-org/my-repo.git", null);
+        assertEquals("gitserver.local:my-org/my-repo.git",
+                sshNoUser);
+    }
+
+    @Test
+    void testHttps() {
+        var httpsDefault = client.updateUrl("https://gitserver.local/my-org/my-repo.git", null);
+        assertEquals("https://[email protected]/my-org/my-repo.git", httpsDefault);
+    }
+
+    @Test
+    void testHttpWithSecret() {
+        var httpsSecret = client.updateUrl("https://gitserver.local/my-org/my-repo.git", secret);
+        assertEquals("https://gitserver.local/my-org/my-repo.git", httpsSecret);
+    }
+
+    @Test
+    void testUnrestrictedHost() {
+        var anonAuth = client.updateUrl("https://elsewhere.local/my-org/my-repo.git", null);
+        // backwards-compat, auth added
+        assertEquals("https://[email protected]/my-org/my-repo.git", anonAuth);
+
+        var url2 = client.updateUrl("https://gitserver.local/my-org/my-repo.git", null);
+        // auth added
+        assertEquals("https://[email protected]/my-org/my-repo.git", url2);
+
+    }
+
+    @Test
+    void testGitHostRestriction() {
+        var restrictedClient = new GitClient(GitClientConfiguration.builder()
+                .from(cfg)
+                .addAuthorizedGitHosts("gitserver.local")
+                .build());
+
+        var anonAuth = restrictedClient.updateUrl("https://elsewhere.local/my-org/my-repo.git", null);
+        // unchanged
+        assertEquals("https://elsewhere.local/my-org/my-repo.git", anonAuth);
+
+        var url2 = restrictedClient.updateUrl("https://gitserver.local/my-org/my-repo.git", null);
+        // auth added
+        assertEquals("https://[email protected]/my-org/my-repo.git", url2);
+    }
+
+}