fix: address code review issues

- Add file type validation for logo/favicon uploads
- Add error state handling instead of alert()
- Style file inputs with proper button appearance
- Add SSRF protection to favicon API (URL validation)
- Add fetch timeout (5 seconds)
- Add content-type validation
- Reduce cache duration to 5 minutes
- Validate image data size before serving

package-lock.json

@@ -10707,6 +10707,7 @@
       "version": "2.3.2",
       "resolved": "https://registry.npmjs.org/fsevents/-/fsevents-2.3.2.tgz",
       "integrity": "sha512-xiqMQR4xAeHTuB9uWm+fFRcIOgKBMiOBP+eXiyT7jsgVCq1bkVygt00oASowB7EdtpOHaaPgKt812P9ab+DDKA==",
+      "dev": true,
       "hasInstallScript": true,
       "license": "MIT",
       "optional": true,

src/app/(dashboard)/dashboard/settings/components/AppearanceTab.tsx

@@ -21,6 +21,7 @@ export default function AppearanceTab() {
   const tSidebar = useTranslations("sidebar");
   const [settings, setSettings] = useState<Record<string, any>>({});
   const [loading, setLoading] = useState(true);
+  const [uploadError, setUploadError] = useState<string | null>(null);
   const [customThemeColor, setCustomThemeColor] = useState(customColor || "#3b82f6");
   const isValidHex = /^#([0-9a-fA-F]{6})$/.test(
     customThemeColor.startsWith("#") ? customThemeColor : `#${customThemeColor}`
@@ -342,26 +343,47 @@ export default function AppearanceTab() {
             <div className="flex flex-col gap-2">
               <p className="font-medium">{t("uploadLogo")}</p>
               <div className="flex items-center gap-2">
-                <input
-                  type="file"
-                  accept="image/*"
-                  onChange={(e) => {
-                    const file = e.target.files?.[0];
-                    if (file) {
-                      if (file.size > 500 * 1024) {
-                        alert("Logo file must be less than 500KB");
-                        return;
+                <label className="flex items-center gap-2 px-4 py-2 rounded-lg bg-surface border border-border text-sm text-text-main cursor-pointer hover:bg-surface/80 transition-colors">
+                  <input
+                    type="file"
+                    accept="image/png,image/jpeg,image/svg+xml,image/gif,image/webp"
+                    onChange={(e) => {
+                      const file = e.target.files?.[0];
+                      if (file) {
+                        if (file.size > 500 * 1024) {
+                          setUploadError("Logo file must be less than 500KB");
+                          return;
+                        }
+                        const validTypes = [
+                          "image/png",
+                          "image/jpeg",
+                          "image/svg+xml",
+                          "image/gif",
+                          "image/webp",
+                        ];
+                        if (!validTypes.includes(file.type)) {
+                          setUploadError(
+                            "Invalid file type. Please upload PNG, JPG, SVG, GIF, or WebP."
+                          );
+                          return;
+                        }
+                        setUploadError(null);
+                        const reader = new FileReader();
+                        reader.onerror = () => {
+                          setUploadError("Failed to read file");
+                        };
+                        reader.onload = (event) => {
+                          const base64 = event.target?.result as string;
+                          updateSetting("customLogoBase64", base64);
+                        };
+                        reader.readAsDataURL(file);
                       }
-                      const reader = new FileReader();
-                      reader.onload = (event) => {
-                        const base64 = event.target?.result as string;
-                        updateSetting("customLogoBase64", base64);
-                      };
-                      reader.readAsDataURL(file);
-                    }
-                  }}
-                  className="text-sm text-text-muted"
-                />
+                    }}
+                    className="hidden"
+                  />
+                  <span className="material-symbols-outlined text-[18px]">upload</span>
+                  <span>{t("uploadLogo")}</span>
+                </label>
                 <Button
                   variant="secondary"
                   onClick={() => {
@@ -372,6 +394,7 @@ export default function AppearanceTab() {
                   {t("resetLogo")}
                 </Button>
               </div>
+              {uploadError && <p className="text-sm text-red-500">{uploadError}</p>}
               {(settings.customLogoBase64 || settings.customLogoUrl) && (
                 <div className="mt-2 p-3 bg-black/5 dark:bg-white/5 rounded-lg">
                   <p className="text-xs text-text-muted mb-2">{t("logoPreview")}</p>
@@ -414,26 +437,47 @@ export default function AppearanceTab() {
             <div className="flex flex-col gap-2">
               <p className="font-medium">{t("uploadFavicon")}</p>
               <div className="flex items-center gap-2">
-                <input
-                  type="file"
-                  accept="image/*"
-                  onChange={(e) => {
-                    const file = e.target.files?.[0];
-                    if (file) {
-                      if (file.size > 50 * 1024) {
-                        alert("Favicon file must be less than 50KB");
-                        return;
+                <label className="flex items-center gap-2 px-4 py-2 rounded-lg bg-surface border border-border text-sm text-text-main cursor-pointer hover:bg-surface/80 transition-colors">
+                  <input
+                    type="file"
+                    accept="image/png,image/x-icon,image/svg+xml,image/gif,image/webp"
+                    onChange={(e) => {
+                      const file = e.target.files?.[0];
+                      if (file) {
+                        if (file.size > 50 * 1024) {
+                          setUploadError("Favicon file must be less than 50KB");
+                          return;
+                        }
+                        const validTypes = [
+                          "image/png",
+                          "image/x-icon",
+                          "image/svg+xml",
+                          "image/gif",
+                          "image/webp",
+                        ];
+                        if (!validTypes.includes(file.type)) {
+                          setUploadError(
+                            "Invalid file type. Please upload PNG, ICO, SVG, GIF, or WebP."
+                          );
+                          return;
+                        }
+                        setUploadError(null);
+                        const reader = new FileReader();
+                        reader.onerror = () => {
+                          setUploadError("Failed to read file");
+                        };
+                        reader.onload = (event) => {
+                          const base64 = event.target?.result as string;
+                          updateSetting("customFaviconBase64", base64);
+                        };
+                        reader.readAsDataURL(file);
                       }
-                      const reader = new FileReader();
-                      reader.onload = (event) => {
-                        const base64 = event.target?.result as string;
-                        updateSetting("customFaviconBase64", base64);
-                      };
-                      reader.readAsDataURL(file);
-                    }
-                  }}
-                  className="text-sm text-text-muted"
-                />
+                    }}
+                    className="hidden"
+                  />
+                  <span className="material-symbols-outlined text-[18px]">upload</span>
+                  <span>{t("uploadFavicon")}</span>
+                </label>
                 <Button
                   variant="secondary"
                   onClick={() => {
@@ -444,6 +488,9 @@ export default function AppearanceTab() {
                   {t("resetFavicon")}
                 </Button>
               </div>
+              {uploadError && !uploadError.includes("Logo") && (
+                <p className="text-sm text-red-500">{uploadError}</p>
+              )}
               {(settings.customFaviconBase64 || settings.customFaviconUrl) && (
                 <div className="mt-2 p-3 bg-black/5 dark:bg-white/5 rounded-lg">
                   <p className="text-xs text-text-muted mb-2">{t("faviconPreview")}</p>

src/app/api/settings/favicon/route.ts

@@ -3,29 +3,115 @@ import { getSettings } from "@/lib/db/settings";
 
 export const dynamic = "force-dynamic";
 
+const ALLOWED_IMAGE_TYPES = [
+  "image/png",
+  "image/x-icon",
+  "image/svg+xml",
+  "image/gif",
+  "image/webp",
+  "image/jpeg",
+];
+const MAX_FAVICON_SIZE = 50 * 1024; // 50KB
+const FETCH_TIMEOUT = 5000; // 5 seconds
+const CACHE_DURATION = 300; // 5 minutes
+
+function isAllowedUrl(url: string): boolean {
+  try {
+    const parsedUrl = new URL(url);
+    // Only allow https (or http for local development)
+    if (parsedUrl.protocol !== "https:" && parsedUrl.protocol !== "http:") {
+      return false;
+    }
+    // Block private/internal IPs
+    const hostname = parsedUrl.hostname;
+    if (
+      hostname === "localhost" ||
+      hostname === "127.0.0.1" ||
+      hostname === "0.0.0.0" ||
+      hostname.startsWith("192.168.") ||
+      hostname.startsWith("10.") ||
+      hostname.startsWith("172.") ||
+      hostname.endsWith(".local") ||
+      hostname === "localhost"
+    ) {
+      return false;
+    }
+    return true;
+  } catch {
+    return false;
+  }
+}
+
+function validateImageData(base64Data: string, contentType: string): boolean {
+  if (!ALLOWED_IMAGE_TYPES.includes(contentType)) {
+    console.error("Invalid content type:", contentType);
+    return false;
+  }
+  // Check for obvious image magic bytes
+  const matches = base64Data.match(/^data:[^;]+;base64,(.+)$/);
+  if (!matches) return false;
+
+  const binaryData = Buffer.from(matches[1], "base64");
+  if (binaryData.length > MAX_FAVICON_SIZE) {
+    console.error("Favicon too large:", binaryData.length);
+    return false;
+  }
+
+  return true;
+}
+
 export async function GET() {
   try {
     const settings = await getSettings();
 
-    const customFaviconBase64 = settings?.customFaviconBase64;
-    const customFaviconUrl = settings?.customFaviconUrl;
+    const customFaviconBase64 = settings?.customFaviconBase64 as string | undefined;
+    const customFaviconUrl = settings?.customFaviconUrl as string | undefined;
 
     let faviconData: string | null = null;
 
     if (customFaviconBase64) {
-      faviconData = customFaviconBase64;
+      // Validate stored Base64 data
+      const match = customFaviconBase64.match(/^data:([^;]+);base64,(.+)$/);
+      if (match && validateImageData(customFaviconBase64, match[1])) {
+        faviconData = customFaviconBase64;
+      }
     } else if (customFaviconUrl) {
-      try {
-        const response = await fetch(customFaviconUrl);
-        if (response.ok) {
-          const arrayBuffer = await response.arrayBuffer();
-          const uint8Array = new Uint8Array(arrayBuffer);
-          const base64 = Buffer.from(uint8Array).toString("base64");
-          const contentType = response.headers.get("content-type") || "image/png";
-          faviconData = `data:${contentType};base64,${base64}`;
+      // Validate URL before fetching (SSRF protection)
+      if (!isAllowedUrl(customFaviconUrl)) {
+        console.error("Blocked invalid favicon URL:", customFaviconUrl);
+      } else {
+        try {
+          const controller = new AbortController();
+          const timeoutId = setTimeout(() => controller.abort(), FETCH_TIMEOUT);
+
+          const response = await fetch(customFaviconUrl, {
+            signal: controller.signal,
+            headers: {
+              "User-Agent": "OmniRoute/1.0",
+            },
+          });
+          clearTimeout(timeoutId);
+
+          if (response.ok) {
+            const contentType = response.headers.get("content-type") || "";
+            const arrayBuffer = await response.arrayBuffer();
+            const uint8Array = new Uint8Array(arrayBuffer);
+
+            // Validate size before processing
+            if (uint8Array.length > MAX_FAVICON_SIZE) {
+              console.error("Favicon exceeds max size:", uint8Array.length);
+            } else {
+              const base64 = Buffer.from(uint8Array).toString("base64");
+              const fullData = `data:${contentType};base64,${base64}`;
+
+              if (validateImageData(fullData, contentType)) {
+                faviconData = fullData;
+              }
+            }
+          }
+        } catch (error) {
+          console.error("Failed to fetch custom favicon:", error);
         }
-      } catch (error) {
-        console.error("Failed to fetch custom favicon:", error);
       }
     }
 
@@ -45,7 +131,7 @@ export async function GET() {
     return new NextResponse(buffer, {
       headers: {
         "Content-Type": contentType,
-        "Cache-Control": "public, max-age=3600",
+        "Cache-Control": `public, max-age=${CACHE_DURATION}`,
       },
     });
   } catch (error) {