diff --git a/server/src/__tests__/projects.test.ts b/server/src/__tests__/projects.test.ts index f6127f7..4103cc8 100644 --- a/server/src/__tests__/projects.test.ts +++ b/server/src/__tests__/projects.test.ts @@ -45,7 +45,13 @@ 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', + 'https://@github.com/org/repo', + 'https://:@github.com/org/repo', + ]) { await request(app) .post(`/api/companies/${companyId}/projects`) .send({ name: 'Invalid repository', repoUrl }) @@ -115,10 +121,16 @@ describe('Projects API', () => { .send({ name: 'Keep canonical state', status: 'planning', repoUrl: null }) .expect(201); - await request(app) - .patch(`/api/companies/${companyId}/projects/${created.body.data.id}`) - .send({ name: 'Do not persist', repoUrl: 'javascript:alert(document.domain)' }) - .expect(400); + for (const repoUrl of [ + 'javascript:alert(document.domain)', + 'https://token@github.com/org/repo', + 'https://@github.com/org/repo', + ]) { + await request(app) + .patch(`/api/companies/${companyId}/projects/${created.body.data.id}`) + .send({ name: 'Do not persist', repoUrl }) + .expect(400); + } await request(app) .get(`/api/companies/${companyId}/projects/${created.body.data.id}`) diff --git a/server/src/routes/projects.ts b/server/src/routes/projects.ts index 74d1066..b280673 100644 --- a/server/src/routes/projects.ts +++ b/server/src/routes/projects.ts @@ -7,14 +7,22 @@ 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:'; + if (!isHttpProtocol || url.username !== '' || url.password !== '') return false; + // URL parsing discards an empty userinfo section, so https://@host and + // https://:@host would survive the username/password check above. + const afterScheme = value.trim().slice(url.protocol.length).replace(/^[/\\]*/, ''); + const authority = afterScheme.split(/[/\\?#]/, 1)[0] ?? ''; + return !authority.includes('@'); } 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), diff --git a/ui/src/components/projects/ProjectFormModal.tsx b/ui/src/components/projects/ProjectFormModal.tsx index d4ff5ee..f3a5b51 100644 --- a/ui/src/components/projects/ProjectFormModal.tsx +++ b/ui/src/components/projects/ProjectFormModal.tsx @@ -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.", + ), ]), }); diff --git a/ui/src/lib/urls.ts b/ui/src/lib/urls.ts index c4faf99..3295f2b 100644 --- a/ui/src/lib/urls.ts +++ b/ui/src/lib/urls.ts @@ -1,8 +1,23 @@ +/** + * 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:"; + if (!isHttpProtocol || url.username !== "" || url.password !== "") return false; + // URL parsing discards an empty userinfo section, so https://@host and + // https://:@host would survive the username/password check above. + return !rawAuthority(value, url.protocol).includes("@"); } catch { return false; } } + +/** Authority of the raw input, before URL parsing normalizes empty userinfo away. */ +function rawAuthority(value: string, protocol: string): string { + const afterScheme = value.trim().slice(protocol.length).replace(/^[/\\]*/, ""); + return afterScheme.split(/[/\\?#]/, 1)[0] ?? ""; +} diff --git a/ui/test/CreateProjectModal.test.tsx b/ui/test/CreateProjectModal.test.tsx index c307a38..8b1abf2 100644 --- a/ui/test/CreateProjectModal.test.tsx +++ b/ui/test/CreateProjectModal.test.tsx @@ -73,7 +73,12 @@ 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"], + ["an empty userinfo section", "https://@github.com/org/repo"], + ["an empty password userinfo section", "https://:@github.com/org/repo"], + ])("rejects repository URLs with %s", async (_label, value) => { const user = userEvent.setup(); render( { ); 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(); });