Skip to content

Commit 441d01a

Browse files
committed
Sanitize redirect paths in the gh installation and auth flow
1 parent 6609e3c commit 441d01a

File tree

4 files changed

+32
-10
lines changed

4 files changed

+32
-10
lines changed

apps/webapp/app/routes/_app.github.callback/route.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { redirectWithErrorMessage, redirectWithSuccessMessage } from "~/models/m
77
import { tryCatch } from "@trigger.dev/core";
88
import { $replica } from "~/db.server";
99
import { requireUser } from "~/services/session.server";
10+
import { sanitizeRedirectPath } from "~/utils";
1011

1112
const QuerySchema = z.discriminatedUnion("setup_action", [
1213
z.object({
@@ -52,7 +53,8 @@ export async function loader({ request }: LoaderFunctionArgs) {
5253
return redirectWithErrorMessage("/", request, "Failed to install GitHub App");
5354
}
5455

55-
const { organizationId, redirectTo } = sessionResult;
56+
const { organizationId, redirectTo: unsafeRedirectTo } = sessionResult;
57+
const redirectTo = sanitizeRedirectPath(unsafeRedirectTo);
5658

5759
const user = await requireUser(request);
5860
const org = await $replica.organization.findFirst({

apps/webapp/app/routes/_app.github.install/route.tsx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,13 @@ import { createGitHubAppInstallSession } from "~/services/gitHubSession.server";
66
import { requireUser } from "~/services/session.server";
77
import { newOrganizationPath } from "~/utils/pathBuilder";
88
import { logger } from "~/services/logger.server";
9+
import { sanitizeRedirectPath } from "~/utils";
910

1011
const QuerySchema = z.object({
1112
org_slug: z.string(),
12-
redirect_to: z.string(),
13+
redirect_to: z.string().refine((value) => value === sanitizeRedirectPath(value), {
14+
message: "Invalid redirect path",
15+
}),
1316
});
1417

1518
export const loader = async ({ request }: LoaderFunctionArgs) => {

apps/webapp/app/routes/auth.github.callback.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,12 @@ import { getSession, redirectWithErrorMessage } from "~/models/message.server";
55
import { authenticator } from "~/services/auth.server";
66
import { commitSession } from "~/services/sessionStorage.server";
77
import { redirectCookie } from "./auth.github";
8+
import { sanitizeRedirectPath } from "~/utils";
89

910
export let loader: LoaderFunction = async ({ request }) => {
1011
const cookie = request.headers.get("Cookie");
1112
const redirectValue = await redirectCookie.parse(cookie);
12-
const redirectTo = redirectValue ?? "/";
13+
const redirectTo = sanitizeRedirectPath(redirectValue);
1314

1415
const auth = await authenticator.authenticate("github", request, {
1516
failureRedirect: "/login", // If auth fails, the failureRedirect will be thrown as a Response

apps/webapp/app/utils.ts

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,22 +7,38 @@ const DEFAULT_REDIRECT = "/";
77
* This should be used any time the redirect path is user-provided
88
* (Like the query string on our login/signup pages). This avoids
99
* open-redirect vulnerabilities.
10-
* @param {string} to The redirect destination
10+
* @param {string} path The redirect destination
1111
* @param {string} defaultRedirect The redirect to use if the to is unsafe.
1212
*/
13-
export function safeRedirect(
14-
to: FormDataEntryValue | string | null | undefined,
13+
export function sanitizeRedirectPath(
14+
path: string | undefined | null,
1515
defaultRedirect: string = DEFAULT_REDIRECT
16-
) {
17-
if (!to || typeof to !== "string") {
16+
): string {
17+
if (!path || typeof path !== "string") {
1818
return defaultRedirect;
1919
}
2020

21-
if (!to.startsWith("/") || to.startsWith("//")) {
21+
if (!path.startsWith("/") || path.startsWith("//")) {
2222
return defaultRedirect;
2323
}
2424

25-
return to;
25+
try {
26+
// should not parse as a full URL
27+
new URL(path);
28+
return defaultRedirect;
29+
} catch {}
30+
31+
try {
32+
// ensure it's a valid relative path
33+
const url = new URL(path, "https://example.com");
34+
if (url.hostname !== "example.com") {
35+
return defaultRedirect;
36+
}
37+
} catch {
38+
return defaultRedirect;
39+
}
40+
41+
return path;
2642
}
2743

2844
/**

0 commit comments

Comments
 (0)