Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion server/src/__tests__/projects.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,11 @@ describe('Projects API', () => {
});

it('rejects an invalid repository URL without persisting a project', async () => {
for (const repoUrl of ['not-a-url', 'javascript:alert(document.domain)']) {
for (const repoUrl of [
'not-a-url',
'javascript:alert(document.domain)',
'https://token@github.com/org/repo',
]) {
await request(app)
.post(`/api/companies/${companyId}/projects`)
.send({ name: 'Invalid repository', repoUrl })
Comment thread
qodo-code-review[bot] marked this conversation as resolved.
Expand Down
9 changes: 6 additions & 3 deletions server/src/routes/projects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,17 @@ import eventBus from '../realtime/events.js';
import type { DbInstance } from '../types.js';
import { routeParams } from '../utils/route-params.js';

// Userinfo is rejected alongside non-http(s) schemes: a credential-bearing URL such as
// https://token@host/org/repo would be persisted and rendered verbatim, leaking the secret.
const HttpUrl = z.string().url().refine((value) => {
try {
const protocol = new URL(value).protocol;
return protocol === 'http:' || protocol === 'https:';
const url = new URL(value);
const isHttpProtocol = url.protocol === 'http:' || url.protocol === 'https:';
return isHttpProtocol && url.username === '' && url.password === '';
Comment thread
qodo-code-review[bot] marked this conversation as resolved.
Outdated
} catch {
return false;
}
}, 'Repository URL must start with http:// or https://');
}, 'Repository URL must start with http:// or https:// and must not embed credentials');

const CreateProjectBody = z.object({
name: z.string().min(1).max(255),
Expand Down
5 changes: 4 additions & 1 deletion ui/src/components/projects/ProjectFormModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,10 @@ const projectSchema = z.object({
z.literal(""),
z
.url("Enter a complete repository URL, such as https://github.com/org/repo.")
.refine(isHttpUrl, "Repository URL must start with http:// or https://."),
.refine(
isHttpUrl,
"Use an http(s) repository URL without embedded credentials, such as https://github.com/org/repo.",
),
]),
});

Expand Down
10 changes: 8 additions & 2 deletions ui/src/lib/urls.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
/**
* True when the value is an absolute http(s) URL with no embedded credentials.
* Userinfo is rejected because a repository URL such as https://token@host/org/repo
* would otherwise be persisted and rendered verbatim, leaking the secret.
*/
export function isHttpUrl(value: string): boolean {
try {
const protocol = new URL(value).protocol;
return protocol === "http:" || protocol === "https:";
const url = new URL(value);
const isHttpProtocol = url.protocol === "http:" || url.protocol === "https:";
return isHttpProtocol && url.username === "" && url.password === "";
} catch {
return false;
}
Expand Down
12 changes: 6 additions & 6 deletions ui/test/CreateProjectModal.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,10 @@ describe("CreateProjectModal", () => {
expect(mocks.createProject).not.toHaveBeenCalled();
});

it("rejects repository URLs with executable schemes", async () => {
it.each([
["executable schemes", "javascript:alert(document.domain)"],
["embedded credentials", "https://token@github.com/org/repo"],
])("rejects repository URLs with %s", async (_label, value) => {
const user = userEvent.setup();
render(
<CreateProjectModal
Expand All @@ -85,14 +88,11 @@ describe("CreateProjectModal", () => {
);

await user.type(screen.getByLabelText("Project name"), "Runtime reliability");
await user.type(
screen.getByLabelText("Repository URL"),
"javascript:alert(document.domain)",
);
await user.type(screen.getByLabelText("Repository URL"), value);
await user.click(screen.getByRole("button", { name: "Create Project" }));

expect(screen.getByRole("alert")).toHaveTextContent(
"Repository URL must start with http:// or https://.",
"Use an http(s) repository URL without embedded credentials, such as https://github.com/org/repo.",
);
expect(mocks.createProject).not.toHaveBeenCalled();
});
Expand Down