Skip to content

Commit 2ffa717

Browse files
Improve repository finder search URL validation (#20287)
* Improve repository finder search URL validation * Add one more test
1 parent b429e93 commit 2ffa717

File tree

3 files changed

+76
-6
lines changed

3 files changed

+76
-6
lines changed

components/dashboard/src/data/git-providers/unified-repositories-search-query.test.ts

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
*/
66

77
import { SuggestedRepository } from "@gitpod/public-api/lib/gitpod/v1/scm_pb";
8-
import { deduplicateAndFilterRepositories } from "./unified-repositories-search-query";
8+
import { deduplicateAndFilterRepositories, isValidGitUrl } from "./unified-repositories-search-query";
99

1010
function repo(name: string, project?: string): SuggestedRepository {
1111
return new SuggestedRepository({
@@ -95,3 +95,27 @@ test("it should return all repositories without duplicates when excludeProjects
9595
expect(deduplicated[0].repoName).toEqual("foo");
9696
expect(deduplicated[1].repoName).toEqual("bar");
9797
});
98+
99+
test("should perform weak validation for git URLs", () => {
100+
expect(isValidGitUrl("a:")).toEqual(false);
101+
expect(isValidGitUrl("a:b")).toEqual(false);
102+
expect(isValidGitUrl("https://b")).toEqual(false);
103+
expect(isValidGitUrl("https://b/repo.git")).toEqual(false);
104+
expect(isValidGitUrl("https://b.com/repo.git")).toEqual(true);
105+
expect(isValidGitUrl("[email protected]:")).toEqual(false);
106+
expect(isValidGitUrl("[email protected]:")).toEqual(false);
107+
expect(isValidGitUrl("[email protected]:22:")).toEqual(false);
108+
expect(isValidGitUrl("[email protected]:g/g")).toEqual(true);
109+
110+
// some "from the wild" cases
111+
expect(isValidGitUrl("https://github.com/gitpod-io/gitpod/pull/20281")).toEqual(true);
112+
expect(isValidGitUrl("https://gitlab.com/filiptronicek/gitpod.git")).toEqual(true);
113+
expect(isValidGitUrl("[email protected]:gitpod-io/gitpod.git")).toEqual(true);
114+
expect(isValidGitUrl("[email protected]:filiptronicek/gitpod.git")).toEqual(true);
115+
expect(isValidGitUrl("ssh://[email protected]:12345/~/repository.git")).toBe(true);
116+
expect(isValidGitUrl("https://bitbucket.gitpod-dev.com/scm/~geropl/test-user-repo.git")).toBe(true);
117+
expect(isValidGitUrl("git://gitlab.com/gitpod/spring-petclinic")).toBe(true);
118+
expect(isValidGitUrl("[email protected]:v3/services-azure/open-to-edit-project2/open-to-edit-project2")).toBe(
119+
true,
120+
);
121+
});

components/dashboard/src/data/git-providers/unified-repositories-search-query.ts

Lines changed: 50 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { useMemo } from "react";
1111
import { useListConfigurations } from "../configurations/configuration-queries";
1212
import type { UseInfiniteQueryResult } from "@tanstack/react-query";
1313
import { Configuration } from "@gitpod/public-api/lib/gitpod/v1/configuration_pb";
14+
import { parseUrl } from "../../utils";
1415

1516
export const flattenPagedConfigurations = (
1617
data: UseInfiniteQueryResult<{ configurations: Configuration[] }>["data"],
@@ -125,17 +126,62 @@ export function deduplicateAndFilterRepositories(
125126
}
126127

127128
if (results.length === 0) {
128-
try {
129-
// If the searchString is a URL, and it's not present in the proposed results, "artificially" add it here.
130-
new URL(searchString);
129+
// If the searchString is a URL, and it's not present in the proposed results, "artificially" add it here.
130+
if (isValidGitUrl(searchString)) {
131+
console.log("It's valid man");
131132
results.push(
132133
new SuggestedRepository({
133134
url: searchString,
134135
}),
135136
);
136-
} catch {}
137+
}
138+
139+
console.log("Valid after man");
137140
}
138141

139142
// Limit what we show to 200 results
140143
return results.slice(0, 200);
141144
}
145+
146+
const ALLOWED_GIT_PROTOCOLS = ["ssh:", "git:", "http:", "https:"];
147+
/**
148+
* An opionated git URL validator
149+
*
150+
* Assumptions:
151+
* - Git hosts are not themselves TLDs (like .com) or reserved names like `localhost`
152+
* - Git clone URLs can operate over ssh://, git:// and http(s)://
153+
* - Git clone URLs (both SSH and HTTP ones) must have a nonempty path
154+
*/
155+
export const isValidGitUrl = (input: string): boolean => {
156+
const url = parseUrl(input);
157+
if (!url) {
158+
// SSH URLs with no protocol, such as [email protected]:gitpod-io/gitpod.git
159+
const sshMatch = input.match(/^\w+@([^:]+):(.+)$/);
160+
if (!sshMatch) return false;
161+
162+
const [, host, path] = sshMatch;
163+
164+
// Check if the path is not empty
165+
if (!path || path.trim().length === 0) return false;
166+
167+
if (path.includes(":")) return false;
168+
169+
return isHostValid(host);
170+
}
171+
172+
if (!url) return false;
173+
174+
if (!ALLOWED_GIT_PROTOCOLS.includes(url.protocol)) return false;
175+
if (url.pathname.length <= 1) return false; // make sure we have some path
176+
177+
return isHostValid(url.host);
178+
};
179+
180+
const isHostValid = (input?: string): boolean => {
181+
if (!input) return false;
182+
183+
const hostSegments = input.split(".");
184+
if (hostSegments.length < 2 || hostSegments.some((chunk) => chunk === "")) return false; // check that there are no consecutive periods as well as no leading or trailing ones
185+
186+
return true;
187+
};

components/dashboard/src/utils.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,7 @@ export class ReplayableEventEmitter<EventTypes extends EventMap> extends EventEm
214214
}
215215
}
216216

217-
function parseUrl(url: string): URL | null {
217+
export function parseUrl(url: string): URL | null {
218218
try {
219219
return new URL(url);
220220
} catch (_) {

0 commit comments

Comments
 (0)