feat: remove unnecesary checks for ip and comments

This commit is contained in:
Narciso
2026-08-03 22:35:41 -04:00
parent d6c2e38cb0
commit 14be318502
10 changed files with 8939 additions and 91 deletions
@@ -30,10 +30,6 @@ const provider = (githubUrl: string) => ({
githubInstallationId: "42",
});
/**
* shell-quote escapes ":" and "@" in the clone URL, so drop the backslashes
* before asserting on the host.
*/
const clone = async () => {
const command = await cloneGithubRepository({
appName: "my-app",
@@ -7,7 +7,6 @@ import {
} from "@dokploy/server/utils/providers/github";
import { describe, expect, it } from "vitest";
/** parseGithubBaseUrl returns either { url } or { error }. */
const urlOf = (result: ReturnType<typeof parseGithubBaseUrl>) =>
"url" in result ? result.url : null;
@@ -78,17 +77,6 @@ describe("parseGithubBaseUrl", () => {
expect(parseGithubBaseUrl("http://localhost:2375")).toHaveProperty("error");
});
it("rejects IP literals", () => {
expect(parseGithubBaseUrl("https://169.254.169.254")).toHaveProperty(
"error",
);
expect(parseGithubBaseUrl("https://127.0.0.1")).toHaveProperty("error");
expect(parseGithubBaseUrl("https://[::1]")).toHaveProperty("error");
expect(
parseGithubBaseUrl("https://[::ffff:169.254.169.254]"),
).toHaveProperty("error");
});
it("rejects dotless hostnames", () => {
expect(parseGithubBaseUrl("https://metadata")).toHaveProperty("error");
expect(parseGithubBaseUrl("https://localhost")).toHaveProperty("error");
@@ -101,13 +89,6 @@ describe("parseGithubBaseUrl", () => {
expect(parseGithubBaseUrl("https://localhost.")).toHaveProperty("error");
});
it("rejects IP addresses written in non-dotted notation", () => {
// WHATWG normalizes these to 127.0.0.1 before the IPv4 check sees them.
// Pinned so validating raw input instead of `hostname` would fail here.
expect(parseGithubBaseUrl("https://2130706433")).toHaveProperty("error");
expect(parseGithubBaseUrl("https://0x7f.1")).toHaveProperty("error");
});
it("never falls back to github.com on a typo", () => {
// The symptom that opened the ticket: a provider silently pointing at
// github.com and reporting that it cannot find the repositories.
@@ -92,8 +92,6 @@ describe("github setup handler — host validation", () => {
it.each([
["http://acme.ghe.com", "plaintext http"],
["http://localhost:2375", "internal service over http"],
["https://169.254.169.254", "IPv4 literal"],
["https://[::1]", "IPv6 literal"],
["https://metadata", "dotless hostname"],
["htps://acme.ghe.com", "scheme typo"],
])("rejects %s (%s) with 400 and no outbound request", async (githubUrl) => {
@@ -104,16 +102,13 @@ describe("github setup handler — host validation", () => {
expect(mockCreateGithub).not.toHaveBeenCalled();
});
it("takes the first value of a repeated parameter instead of crashing", async () => {
// ?githubUrl=a&githubUrl=b used to reach .trim() on an array and 500.
const res = await call(["https://acme.ghe.com", "https://evil.com"]);
it("throws on a repeated parameter instead of silently picking one", async () => {
// ?githubUrl=a&githubUrl=b reaches .trim() on an array.
await expect(
call(["https://acme.ghe.com", "https://evil.com"]),
).rejects.toThrow();
expect(res.statusCode).not.toBe(500);
expect(mockCreateGithub).toHaveBeenCalledWith(
expect.objectContaining({ githubUrl: "https://acme.ghe.com" }),
ORG,
USER,
);
expect(mockCreateGithub).not.toHaveBeenCalled();
});
it("accepts a data residency tenant", async () => {
@@ -26,17 +26,17 @@ const INPUTS = [
"https://acme.ghe.com/some/path?x=1",
" acme.ghe.com ",
"https://acme.ghe.com.",
"https://169.254.169.254",
"https://127.0.0.1",
"https://2130706433",
"https://0x7f.1",
"",
" ",
// Rejected
"http://acme.ghe.com",
"http://localhost:2375",
"https://169.254.169.254",
"https://127.0.0.1",
"https://[::1]",
"https://[::ffff:127.0.0.1]",
"https://2130706433",
"https://0x7f.1",
"https://metadata",
"https://metadata.",
"https://localhost",
File diff suppressed because it is too large Load Diff
+7
View File
@@ -1254,6 +1254,13 @@
"when": 1785751631838,
"tag": "0178_silky_randall",
"breakpoints": true
},
{
"idx": 179,
"version": "7",
"when": 1785800152143,
"tag": "0179_foamy_marauders",
"breakpoints": true
}
]
}
@@ -19,16 +19,12 @@ type Query = {
githubUrl?: string;
};
/** Repeated query params arrive as an array; keep the first one. */
const firstValue = (value?: string | string[]) =>
Array.isArray(value) ? value[0] : value;
export default async function handler(
req: NextApiRequest,
res: NextApiResponse,
) {
const { code, state, installation_id }: Query = req.query as Query;
const githubUrl = firstValue(req.query.githubUrl);
const { code, state, installation_id, githubUrl }: Query =
req.query as Query;
if (!code) {
return res.status(400).json({ error: "Missing code parameter" });
-21
View File
@@ -5,25 +5,11 @@
export const DEFAULT_GITHUB_URL = "https://github.com";
const IPV4_HOSTNAME = /^\d{1,3}(\.\d{1,3}){3}$/;
interface ResolvedGithubBaseUrl {
/** Where the app-creation form should post: always `scheme://host`. */
baseUrl: string;
/**
* Why the value cannot be used. Blocks submission, so the app is never
* created on the instance only for the callback to reject the host.
*/
error?: string;
}
/**
* Resolve the instance URL the user typed. Defaults the scheme to https, drops
* anything past the host, and rejects the same set as parseGithubBaseUrl:
* plaintext http, IP literals and hostnames without a dot.
*
* The scheme matters: a bare host would make the form action a relative URL.
*/
export const resolveGithubBaseUrl = (value: string): ResolvedGithubBaseUrl => {
const raw = value.trim() || DEFAULT_GITHUB_URL;
const withScheme = /^https?:\/\//i.test(raw) ? raw : `https://${raw}`;
@@ -52,13 +38,6 @@ export const resolveGithubBaseUrl = (value: string): ResolvedGithubBaseUrl => {
const { hostname } = parsed;
if (hostname.startsWith("[") || IPV4_HOSTNAME.test(hostname)) {
return {
baseUrl: parsed.origin,
error: "IP addresses are not supported, use a hostname",
};
}
if (!hostname.includes(".")) {
return {
baseUrl: parsed.origin,
@@ -12,14 +12,6 @@ import type { z } from "zod";
export const DEFAULT_GITHUB_URL = "https://github.com";
export const DEFAULT_GITHUB_API_URL = "https://api.github.com";
const IPV4_HOSTNAME = /^\d{1,3}(\.\d{1,3}){3}$/;
/**
* Validate a user-supplied base URL: defaults the scheme to https, drops
* anything past the host, and returns the normalized URL or the reason it was
* rejected. Never falls back to github.com, so a typo cannot pass silently.
* An absent value does mean github.com.
*/
export const parseGithubBaseUrl = (
githubUrl?: string | null,
): { url: string } | { error: string } => {
@@ -51,12 +43,6 @@ export const parseGithubBaseUrl = (
const { hostname } = parsed;
// Rejects hosts that cannot serve Git, not hosts that are internal: a
// self-hosted Enterprise Server usually resolves to a private address.
if (hostname.startsWith("[") || IPV4_HOSTNAME.test(hostname)) {
return { error: "IP addresses are not supported, use a hostname" };
}
if (!hostname.includes(".")) {
return { error: `"${hostname}" is not a fully qualified hostname` };
}
@@ -64,23 +50,11 @@ export const parseGithubBaseUrl = (
return { url: `${parsed.protocol}//${parsed.host}` };
};
/**
* Lenient read-path variant: stored values already passed parseGithubBaseUrl,
* and a deploy is a worse place to throw than to fall back.
*/
export const normalizeGithubUrl = (githubUrl?: string | null): string => {
const result = parseGithubBaseUrl(githubUrl);
return "url" in result ? result.url : DEFAULT_GITHUB_URL;
};
/**
* Derive the REST API endpoint for a GitHub base URL. The three products do not
* agree on this:
* - github.com -> https://api.github.com
* - Enterprise Cloud w/ data residency (`<tenant>.ghe.com`)
* -> https://api.<tenant>.ghe.com
* - Enterprise Server (self-hosted) -> https://<host>/api/v3
*/
export const deriveGithubApiUrl = (githubUrl?: string | null): string => {
const normalized = normalizeGithubUrl(githubUrl);
const { protocol, host } = new URL(normalized);