[WIP] Address feedback on 'The Tale of the Stabilized Deployment' PR

frontend/.gitignore

@@ -9,6 +9,7 @@
 !.yarn/plugins
 !.yarn/releases
 !.yarn/versions
+package-lock.json
 
 # testing
 /coverage

frontend/lib/__tests__/url-validation.test.ts

@@ -0,0 +1,80 @@
+import { describe, it, expect } from "vitest"
+import { isSafeRedirectUrl } from "../url-validation"
+
+describe("isSafeRedirectUrl", () => {
+  describe("should return true for safe relative URLs", () => {
+    it("accepts simple paths", () => {
+      expect(isSafeRedirectUrl("/dashboard")).toBe(true)
+      expect(isSafeRedirectUrl("/configure")).toBe(true)
+      expect(isSafeRedirectUrl("/settings")).toBe(true)
+    })
+
+    it("accepts paths with query parameters", () => {
+      expect(isSafeRedirectUrl("/dashboard?tab=active")).toBe(true)
+      expect(isSafeRedirectUrl("/configure?step=2")).toBe(true)
+    })
+
+    it("accepts paths with hash fragments", () => {
+      expect(isSafeRedirectUrl("/dashboard#section")).toBe(true)
+      expect(isSafeRedirectUrl("/page#top")).toBe(true)
+    })
+
+    it("accepts nested paths", () => {
+      expect(isSafeRedirectUrl("/dashboard/deployments/123")).toBe(true)
+      expect(isSafeRedirectUrl("/a/b/c/d/e")).toBe(true)
+    })
+  })
+
+  describe("should return false for unsafe URLs", () => {
+    it("rejects absolute URLs with http protocol", () => {
+      expect(isSafeRedirectUrl("http://evil.com")).toBe(false)
+      expect(isSafeRedirectUrl("http://example.com/path")).toBe(false)
+    })
+
+    it("rejects absolute URLs with https protocol", () => {
+      expect(isSafeRedirectUrl("https://evil.com")).toBe(false)
+      expect(isSafeRedirectUrl("https://example.com/path")).toBe(false)
+    })
+
+    it("rejects protocol-relative URLs", () => {
+      expect(isSafeRedirectUrl("//evil.com")).toBe(false)
+      expect(isSafeRedirectUrl("//example.com/path")).toBe(false)
+    })
+
+    it("rejects javascript: URLs", () => {
+      expect(isSafeRedirectUrl("javascript:alert(1)")).toBe(false)
+      expect(isSafeRedirectUrl("javascript:void(0)")).toBe(false)
+    })
+
+    it("rejects data: URLs", () => {
+      expect(isSafeRedirectUrl("data:text/html,<script>alert(1)</script>")).toBe(false)
+    })
+
+    it("rejects URLs not starting with /", () => {
+      expect(isSafeRedirectUrl("dashboard")).toBe(false)
+      expect(isSafeRedirectUrl("./dashboard")).toBe(false)
+      expect(isSafeRedirectUrl("../dashboard")).toBe(false)
+    })
+
+    it("rejects other malicious protocols", () => {
+      expect(isSafeRedirectUrl("file:///etc/passwd")).toBe(false)
+      expect(isSafeRedirectUrl("ftp://server.com/file")).toBe(false)
+    })
+  })
+
+  describe("edge cases", () => {
+    it("handles empty strings", () => {
+      expect(isSafeRedirectUrl("")).toBe(false)
+    })
+
+    it("handles URLs with encoded characters", () => {
+      expect(isSafeRedirectUrl("/path?url=http%3A%2F%2Fevil.com")).toBe(true) // encoded as query param is OK
+      expect(isSafeRedirectUrl("/path%2F..%2F..%2Fetc")).toBe(true) // path traversal encoded is still a relative path
+    })
+
+    it("rejects URLs trying to bypass with protocols in path", () => {
+      // These should still be rejected as they contain ://
+      expect(isSafeRedirectUrl("/redirect?url=https://evil.com")).toBe(false)
+    })
+  })
+})

frontend/lib/url-validation.ts

@@ -0,0 +1,41 @@
+/**
+ * Validates if a URL is safe for redirecting within the application.
+ * 
+ * A URL is considered safe if it:
+ * 1. Is a relative path starting with "/"
+ * 2. Does not contain protocol markers (like "://") which indicate absolute URLs
+ * 3. Does not start with "//" (protocol-relative URLs)
+ * 
+ * This prevents open redirect vulnerabilities where attackers could redirect
+ * users to malicious external sites.
+ * 
+ * @param url - The URL string to validate
+ * @returns true if the URL is safe for internal redirects, false otherwise
+ * 
+ * @example
+ * ```ts
+ * isSafeRedirectUrl("/dashboard") // true
+ * isSafeRedirectUrl("/dashboard?tab=active") // true
+ * isSafeRedirectUrl("https://evil.com") // false
+ * isSafeRedirectUrl("//evil.com") // false
+ * isSafeRedirectUrl("javascript:alert(1)") // false
+ * ```
+ */
+export function isSafeRedirectUrl(url: string): boolean {
+  // Must start with "/" to be a relative path
+  if (!url.startsWith("/")) {
+    return false
+  }
+
+  // Must not start with "//" (protocol-relative URL)
+  if (url.startsWith("//")) {
+    return false
+  }
+
+  // Must not contain "://" which indicates an absolute URL with protocol
+  if (url.includes("://")) {
+    return false
+  }
+
+  return true
+}

frontend/middleware.ts

@@ -1,5 +1,6 @@
 import { auth } from "@/auth"
 import { NextResponse } from "next/server"
+import { isSafeRedirectUrl } from "@/lib/url-validation"
 
 /**
  * Middleware to protect routes requiring authentication
@@ -38,6 +39,13 @@ export default auth((req) => {
   // Redirect to intended destination if already signed in and visiting sign-in page
   if (pathname === "/signin" && isAuthenticated) {
     const callbackUrl = req.nextUrl.searchParams.get("callbackUrl") || "/dashboard"
+
+    // Validate callbackUrl to prevent open redirect vulnerability
+    if (!isSafeRedirectUrl(callbackUrl)) {
+      console.warn(`[Middleware] Rejected unsafe callbackUrl: ${callbackUrl}`)
+      return NextResponse.redirect(new URL("/dashboard", req.url))
+    }
+
     console.log(`[Middleware] Authenticated user on signin page, redirecting to ${callbackUrl}`)
     return NextResponse.redirect(new URL(callbackUrl, req.url))
   }

frontend/package.json

@@ -49,8 +49,9 @@
     "@vitejs/plugin-react": "^5.1.2",
     "eslint": "^9",
     "eslint-config-next": "16.0.10",
+    "jsdom": "^27.4.0",
     "tailwindcss": "^4",
     "typescript": "^5",
     "vitest": "^4.0.16"
   }
-}
+}