Skip to content

Commit 3f7173f

Browse files
committed
repository: fix uri scheme restriction for ssh repos
1 parent 142a873 commit 3f7173f

File tree

3 files changed

+114
-3
lines changed

3 files changed

+114
-3
lines changed

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

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -300,10 +300,15 @@ private static String getRefSpec(Ref ref) {
300300
}
301301
}
302302

303-
private String updateUrl(String url, Secret secret) {
304-
url = url.trim();
303+
String updateUrl(String url, Secret secret) {
304+
if (!url.matches("^[A-Za-z][A-Za-z0-9+.-]+://.*")) {
305+
// no scheme. Assume ssh, e.g. from GitHub like '[email protected]:owner/repo.git'
306+
assertUriAllowed("ssh://" + url);
305307

306-
URI uri = URI.create(url);
308+
return url; // return un-modified. git cli doesn't want the ssh scheme
309+
}
310+
311+
URI uri = assertUriAllowed(url);
307312

308313
List<String> allowedDefaultHosts = cfg.authorizedGitHosts();
309314

@@ -324,6 +329,39 @@ private String updateUrl(String url, Secret secret) {
324329
return "https://" + cfg.oauthToken() + "@" + url.substring("https://".length());
325330
}
326331

332+
private URI assertUriAllowed(String rawUri) {
333+
// make sure it's in valid format
334+
URI uri = URI.create(rawUri);
335+
assertUriAllowed(uri);
336+
337+
return uri;
338+
}
339+
340+
private void assertUriAllowed(URI uri) {
341+
String providedScheme = uri.getScheme();
342+
Set<String> allowedSchemes = cfg.allowedSchemes();
343+
boolean hasScheme = providedScheme != null && (!providedScheme.isEmpty());
344+
345+
if (allowedSchemes.isEmpty()) {
346+
return; // allow all
347+
}
348+
349+
// the provided repo string is definitely an allowed protocol.
350+
if (hasScheme && allowedSchemes.contains(providedScheme)) {
351+
return;
352+
}
353+
354+
// the provided repo string has no explicit scheme, should be understood to use an ssh connection
355+
if (!hasScheme && uri.getUserInfo() != null) {
356+
return;
357+
}
358+
359+
String msg = String.format("Provided repository ('%s') contains an unsupported URI scheme: '%s'.",
360+
uri, uri.getScheme());
361+
log.warn(msg);
362+
throw new RepositoryException(msg);
363+
}
364+
327365
private void updateSubmodules(Path workDir, Secret secret) {
328366
exec(Command.builder()
329367
.workDir(workDir)

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import javax.annotation.Nullable;
2626
import java.time.Duration;
2727
import java.util.List;
28+
import java.util.Set;
2829

2930
@Value.Immutable
3031
@Value.Style(jdkOnly = true)
@@ -36,6 +37,11 @@ public interface GitClientConfiguration {
3637
@Nullable
3738
List<String> authorizedGitHosts();
3839

40+
@Value.Default
41+
default Set<String> allowedSchemes() {
42+
return Set.of("https", "http", "ssh", "classpath");
43+
}
44+
3945
@Value.Default
4046
default Duration defaultOperationTimeout() {
4147
return Duration.ofMinutes(10L);
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
package com.walmartlabs.concord.repository;
2+
3+
import com.walmartlabs.concord.common.secret.BinaryDataSecret;
4+
import com.walmartlabs.concord.sdk.Secret;
5+
import org.junit.jupiter.api.Test;
6+
7+
import static org.junit.jupiter.api.Assertions.assertEquals;
8+
9+
class GitUriTest {
10+
11+
private static final GitClientConfiguration cfg = GitClientConfiguration.builder()
12+
.oauthToken("mock-token")
13+
.build();
14+
private static final GitClient client = new GitClient(cfg);
15+
private static Secret secret = new BinaryDataSecret(new byte[0]);
16+
17+
@Test
18+
void testSsh() {
19+
var sshWithUser = client.updateUrl("[email protected]:my-org/my-repo.git", null);
20+
assertEquals("[email protected]:my-org/my-repo.git", sshWithUser);
21+
22+
var sshNoUser = client.updateUrl("gitserver.local:my-org/my-repo.git", null);
23+
assertEquals("gitserver.local:my-org/my-repo.git",
24+
sshNoUser);
25+
}
26+
27+
@Test
28+
void testHttps() {
29+
var httpsDefault = client.updateUrl("https://gitserver.local/my-org/my-repo.git", null);
30+
assertEquals("https://[email protected]/my-org/my-repo.git", httpsDefault);
31+
}
32+
33+
@Test
34+
void testHttpWithSecret() {
35+
var httpsSecret = client.updateUrl("https://gitserver.local/my-org/my-repo.git", secret);
36+
assertEquals("https://gitserver.local/my-org/my-repo.git", httpsSecret);
37+
}
38+
39+
@Test
40+
void testUnrestrictedHost() {
41+
var anonAuth = client.updateUrl("https://elsewhere.local/my-org/my-repo.git", null);
42+
// backwards-compat, auth added
43+
assertEquals("https://[email protected]/my-org/my-repo.git", anonAuth);
44+
45+
var url2 = client.updateUrl("https://gitserver.local/my-org/my-repo.git", null);
46+
// auth added
47+
assertEquals("https://[email protected]/my-org/my-repo.git", url2);
48+
49+
}
50+
51+
@Test
52+
void testGitHostRestriction() {
53+
var restrictedClient = new GitClient(GitClientConfiguration.builder()
54+
.from(cfg)
55+
.addAuthorizedGitHosts("gitserver.local")
56+
.build());
57+
58+
var anonAuth = restrictedClient.updateUrl("https://elsewhere.local/my-org/my-repo.git", null);
59+
// unchanged
60+
assertEquals("https://elsewhere.local/my-org/my-repo.git", anonAuth);
61+
62+
var url2 = restrictedClient.updateUrl("https://gitserver.local/my-org/my-repo.git", null);
63+
// auth added
64+
assertEquals("https://[email protected]/my-org/my-repo.git", url2);
65+
}
66+
67+
}

0 commit comments

Comments
 (0)