fix: allows local ip images (#7189)

Co-authored-by: pandeymangg <pandeyman@Anshumans-MacBook-Air.local>
Co-authored-by: Dhruwang <dhruwangjariwala18@gmail.com>
Co-authored-by: Matti Nannt <matti@formbricks.com>

apps/web/modules/storage/file-upload.test.ts

@@ -72,13 +72,13 @@ describe("fileUpload", () => {
   test("should handle successful file upload with presigned fields", async () => {
     const file = createMockFile("test.jpg", "image/jpeg", 1000);
 
-    // Mock successful API response
+    // Mock successful API response - now returns relative path
     mockFetch.mockResolvedValueOnce({
       ok: true,
       json: async () => ({
         data: {
           signedUrl: "https://s3.example.com/upload",
-          fileUrl: "https://s3.example.com/file.jpg",
+          fileUrl: "/storage/test-env/public/file.jpg",
           presignedFields: {
             key: "value",
           },
@@ -98,18 +98,18 @@ describe("fileUpload", () => {
 
     const result = await fileUploadModule.handleFileUpload(file, "test-env");
     expect(result.error).toBeUndefined();
-    expect(result.url).toBe("https://s3.example.com/file.jpg");
+    expect(result.url).toBe("/storage/test-env/public/file.jpg");
   });
 
   test("should handle upload error with presigned fields", async () => {
     const file = createMockFile("test.jpg", "image/jpeg", 1000);
-    // Mock successful API response
+    // Mock successful API response - now returns relative path
     mockFetch.mockResolvedValueOnce({
       ok: true,
       json: async () => ({
         data: {
           signedUrl: "https://s3.example.com/upload",
-          fileUrl: "https://s3.example.com/file.jpg",
+          fileUrl: "/storage/test-env/public/file.jpg",
           presignedFields: {
             key: "value",
           },
@@ -134,13 +134,13 @@ describe("fileUpload", () => {
   test("should handle upload error", async () => {
     const file = createMockFile("test.jpg", "image/jpeg", 1000);
 
-    // Mock successful API response
+    // Mock successful API response - now returns relative path
     mockFetch.mockResolvedValueOnce({
       ok: true,
       json: async () => ({
         data: {
           signedUrl: "https://s3.example.com/upload",
-          fileUrl: "https://s3.example.com/file.jpg",
+          fileUrl: "/storage/test-env/public/file.jpg",
           presignedFields: {
             key: "value",
           },

apps/web/modules/storage/service.test.ts

@@ -5,7 +5,7 @@ import { TAccessType } from "@formbricks/types/storage";
 import {
   deleteFile,
   deleteFilesByEnvironmentId,
-  getSignedUrlForDownload,
+  getFileStreamForDownload,
   getSignedUrlForUpload,
 } from "./service";
 
@@ -14,14 +14,6 @@ vi.mock("crypto", () => ({
   randomUUID: vi.fn(),
 }));
 
-vi.mock("@/lib/constants", () => ({
-  WEBAPP_URL: "https://webapp.example.com",
-}));
-
-vi.mock("@/lib/getPublicUrl", () => ({
-  getPublicDomain: vi.fn(),
-}));
-
 vi.mock("@formbricks/logger", () => ({
   logger: {
     error: vi.fn(),
@@ -38,22 +30,22 @@ vi.mock("@formbricks/storage", () => ({
   },
   deleteFile: vi.fn(),
   deleteFilesByPrefix: vi.fn(),
+  getFileStream: vi.fn(),
   getSignedDownloadUrl: vi.fn(),
   getSignedUploadUrl: vi.fn(),
 }));
 
 // Import mocked dependencies
 const { logger } = await import("@formbricks/logger");
-const { getPublicDomain } = await import("@/lib/getPublicUrl");
+const storageModule = await import("@formbricks/storage");
 const {
   deleteFile: deleteFileFromS3,
   deleteFilesByPrefix,
-  getSignedDownloadUrl,
   getSignedUploadUrl,
-} = await import("@formbricks/storage");
-
+  getFileStream,
+} = storageModule;
 type MockedSignedUploadReturn = Awaited<ReturnType<typeof getSignedUploadUrl>>;
-type MockedSignedDownloadReturn = Awaited<ReturnType<typeof getSignedDownloadUrl>>;
+type MockedFileStreamReturn = Awaited<ReturnType<typeof getFileStream>>;
 type MockedDeleteFileReturn = Awaited<ReturnType<typeof deleteFile>>;
 type MockedDeleteFilesByPrefixReturn = Awaited<ReturnType<typeof deleteFilesByPrefix>>;
 
@@ -63,7 +55,6 @@ describe("storage service", () => {
   beforeEach(() => {
     vi.clearAllMocks();
     vi.mocked(randomUUID).mockReturnValue(mockUUID);
-    vi.mocked(getPublicDomain).mockReturnValue("https://public.example.com");
   });
 
   describe("getSignedUrlForUpload", () => {
@@ -90,7 +81,7 @@ describe("storage service", () => {
         expect(result.data).toEqual({
           signedUrl: "https://s3.example.com/upload",
           presignedFields: { key: "value" },
-          fileUrl: `https://public.example.com/storage/env-123/public/test-image--fid--${mockUUID}.jpg`,
+          fileUrl: `/storage/env-123/public/test-image--fid--${mockUUID}.jpg`,
         });
       }
 
@@ -102,7 +93,7 @@ describe("storage service", () => {
       );
     });
 
-    test("should use WEBAPP_URL for private files", async () => {
+    test("should return relative URL for private files", async () => {
       const mockSignedUrlResponse = {
         ok: true,
         data: {
@@ -122,9 +113,7 @@ describe("storage service", () => {
 
       expect(result.ok).toBe(true);
       if (result.ok) {
-        expect(result.data.fileUrl).toBe(
-          `https://webapp.example.com/storage/env-123/private/test-doc--fid--${mockUUID}.pdf`
-        );
+        expect(result.data.fileUrl).toBe(`/storage/env-123/private/test-doc--fid--${mockUUID}.pdf`);
       }
     });
 
@@ -149,9 +138,7 @@ describe("storage service", () => {
       expect(result.ok).toBe(true);
       if (result.ok) {
         // The filename should be URL-encoded to prevent # from being treated as a URL fragment
-        expect(result.data.fileUrl).toBe(
-          `https://public.example.com/storage/env-123/public/testfile--fid--${mockUUID}.txt`
-        );
+        expect(result.data.fileUrl).toBe(`/storage/env-123/public/testfile--fid--${mockUUID}.txt`);
       }
 
       expect(getSignedUploadUrl).toHaveBeenCalledWith(
@@ -276,86 +263,6 @@ describe("storage service", () => {
     });
   });
 
-  describe("getSignedUrlForDownload", () => {
-    test("should generate signed URL for download", async () => {
-      const mockSignedUrlResponse = {
-        ok: true,
-        data: "https://s3.example.com/download?signature=abc123",
-      } as MockedSignedDownloadReturn;
-
-      vi.mocked(getSignedDownloadUrl).mockResolvedValue(mockSignedUrlResponse);
-
-      const result = await getSignedUrlForDownload("test-file.jpg", "env-123", "public" as TAccessType);
-
-      expect(result.ok).toBe(true);
-      if (result.ok) {
-        expect(result.data).toBe("https://s3.example.com/download?signature=abc123");
-      }
-      expect(getSignedDownloadUrl).toHaveBeenCalledWith("env-123/public/test-file.jpg");
-    });
-
-    test("should decode URI-encoded filename", async () => {
-      const mockSignedUrlResponse = {
-        ok: true,
-        data: "https://s3.example.com/download?signature=abc123",
-      } as MockedSignedDownloadReturn;
-
-      vi.mocked(getSignedDownloadUrl).mockResolvedValue(mockSignedUrlResponse);
-
-      const encodedFileName = encodeURIComponent("file with spaces.jpg");
-      await getSignedUrlForDownload(encodedFileName, "env-123", "private" as TAccessType);
-
-      expect(getSignedDownloadUrl).toHaveBeenCalledWith("env-123/private/file with spaces.jpg");
-    });
-
-    test("should return error when getSignedDownloadUrl fails", async () => {
-      const mockErrorResponse = {
-        ok: false,
-        error: {
-          code: StorageErrorCode.S3ClientError,
-        },
-      } as MockedSignedDownloadReturn;
-
-      vi.mocked(getSignedDownloadUrl).mockResolvedValue(mockErrorResponse);
-
-      const result = await getSignedUrlForDownload("missing-file.jpg", "env-123", "public" as TAccessType);
-
-      expect(result.ok).toBe(false);
-      if (!result.ok) {
-        expect(result.error.code).toBe(StorageErrorCode.S3ClientError);
-      }
-    });
-
-    test("should handle unexpected errors and return unknown error", async () => {
-      vi.mocked(getSignedDownloadUrl).mockRejectedValue(new Error("Network error"));
-
-      const result = await getSignedUrlForDownload("test-file.jpg", "env-123", "public" as TAccessType);
-
-      expect(result.ok).toBe(false);
-      if (!result.ok) {
-        expect(result.error.code).toBe(StorageErrorCode.Unknown);
-      }
-      expect(logger.error).toHaveBeenCalledWith(
-        { error: expect.any(Error) },
-        "Error getting signed url for download"
-      );
-    });
-
-    test("should handle files with special characters", async () => {
-      const mockSignedUrlResponse = {
-        ok: true,
-        data: "https://s3.example.com/download?signature=abc123",
-      } as MockedSignedDownloadReturn;
-
-      vi.mocked(getSignedDownloadUrl).mockResolvedValue(mockSignedUrlResponse);
-
-      const specialFileName = "file%20with%20%26%20symbols.jpg";
-      await getSignedUrlForDownload(specialFileName, "env-123", "public" as TAccessType);
-
-      expect(getSignedDownloadUrl).toHaveBeenCalledWith("env-123/public/file with & symbols.jpg");
-    });
-  });
-
   describe("deleteFile", () => {
     test("should call deleteFileFromS3 with correct file key", async () => {
       const mockSuccessResult = {
@@ -433,4 +340,147 @@ describe("storage service", () => {
       expect(deleteFilesByPrefix).toHaveBeenCalledWith("env-123");
     });
   });
+
+  describe("getFileStreamForDownload", () => {
+    test("should return file stream for public file", async () => {
+      const mockStream = new ReadableStream();
+      const mockStreamResult = {
+        ok: true,
+        data: {
+          body: mockStream,
+          contentType: "image/jpeg",
+          contentLength: 12345,
+        },
+      } as MockedFileStreamReturn;
+
+      vi.mocked(getFileStream).mockResolvedValue(mockStreamResult);
+
+      const result = await getFileStreamForDownload("test-image.jpg", "env-123", "public" as TAccessType);
+
+      expect(result.ok).toBe(true);
+      if (result.ok) {
+        expect(result.data.body).toBe(mockStream);
+        expect(result.data.contentType).toBe("image/jpeg");
+        expect(result.data.contentLength).toBe(12345);
+      }
+      expect(getFileStream).toHaveBeenCalledWith("env-123/public/test-image.jpg");
+    });
+
+    test("should return file stream for private file", async () => {
+      const mockStream = new ReadableStream();
+      const mockStreamResult = {
+        ok: true,
+        data: {
+          body: mockStream,
+          contentType: "application/pdf",
+          contentLength: 54321,
+        },
+      } as MockedFileStreamReturn;
+
+      vi.mocked(getFileStream).mockResolvedValue(mockStreamResult);
+
+      const result = await getFileStreamForDownload("document.pdf", "env-456", "private" as TAccessType);
+
+      expect(result.ok).toBe(true);
+      if (result.ok) {
+        expect(result.data.contentType).toBe("application/pdf");
+      }
+      expect(getFileStream).toHaveBeenCalledWith("env-456/private/document.pdf");
+    });
+
+    test("should decode URL-encoded filename", async () => {
+      const mockStream = new ReadableStream();
+      const mockStreamResult = {
+        ok: true,
+        data: {
+          body: mockStream,
+          contentType: "image/png",
+          contentLength: 1000,
+        },
+      } as MockedFileStreamReturn;
+
+      vi.mocked(getFileStream).mockResolvedValue(mockStreamResult);
+
+      // URL-encoded filename with spaces: "my file.png" -> "my%20file.png"
+      const result = await getFileStreamForDownload("my%20file.png", "env-123", "public" as TAccessType);
+
+      expect(result.ok).toBe(true);
+      // Should decode %20 to space before passing to getFileStream
+      expect(getFileStream).toHaveBeenCalledWith("env-123/public/my file.png");
+    });
+
+    test("should return error when getFileStream fails with FileNotFoundError", async () => {
+      const mockErrorResult = {
+        ok: false,
+        error: {
+          code: StorageErrorCode.FileNotFoundError,
+        },
+      } as MockedFileStreamReturn;
+
+      vi.mocked(getFileStream).mockResolvedValue(mockErrorResult);
+
+      const result = await getFileStreamForDownload("missing-file.jpg", "env-123", "public" as TAccessType);
+
+      expect(result.ok).toBe(false);
+      if (!result.ok) {
+        expect(result.error.code).toBe(StorageErrorCode.FileNotFoundError);
+      }
+    });
+
+    test("should return error when getFileStream fails with S3ClientError", async () => {
+      const mockErrorResult = {
+        ok: false,
+        error: {
+          code: StorageErrorCode.S3ClientError,
+        },
+      } as MockedFileStreamReturn;
+
+      vi.mocked(getFileStream).mockResolvedValue(mockErrorResult);
+
+      const result = await getFileStreamForDownload("some-file.jpg", "env-123", "public" as TAccessType);
+
+      expect(result.ok).toBe(false);
+      if (!result.ok) {
+        expect(result.error.code).toBe(StorageErrorCode.S3ClientError);
+      }
+    });
+
+    test("should handle unexpected errors and return unknown error", async () => {
+      vi.mocked(getFileStream).mockRejectedValue(new Error("Unexpected S3 error"));
+
+      const result = await getFileStreamForDownload("test-file.jpg", "env-123", "public" as TAccessType);
+
+      expect(result.ok).toBe(false);
+      if (!result.ok) {
+        expect(result.error.code).toBe(StorageErrorCode.Unknown);
+      }
+      expect(logger.error).toHaveBeenCalledWith(
+        { error: expect.any(Error) },
+        "Error getting file stream for download"
+      );
+    });
+
+    test("should handle filename with fid pattern", async () => {
+      const mockStream = new ReadableStream();
+      const mockStreamResult = {
+        ok: true,
+        data: {
+          body: mockStream,
+          contentType: "image/jpeg",
+          contentLength: 5000,
+        },
+      } as MockedFileStreamReturn;
+
+      vi.mocked(getFileStream).mockResolvedValue(mockStreamResult);
+
+      const result = await getFileStreamForDownload(
+        "photo--fid--abc123-def456.jpg",
+        "env-123",
+        "public" as TAccessType
+      );
+
+      expect(result.ok).toBe(true);
+      expect(getFileStream).toHaveBeenCalledWith("env-123/public/photo--fid--abc123-def456.jpg");
+    });
+  });
 });

apps/web/modules/storage/service.ts

@@ -1,17 +1,16 @@
 import { randomUUID } from "crypto";
 import { logger } from "@formbricks/logger";
 import {
+  type FileStreamResult,
   type StorageError,
   StorageErrorCode,
   deleteFile as deleteFileFromS3,
   deleteFilesByPrefix,
-  getSignedDownloadUrl,
+  getFileStream,
   getSignedUploadUrl,
 } from "@formbricks/storage";
 import { Result, err, ok } from "@formbricks/types/error-handlers";
 import { TAccessType } from "@formbricks/types/storage";
-import { WEBAPP_URL } from "@/lib/constants";
-import { getPublicDomain } from "@/lib/getPublicUrl";
 import { sanitizeFileName } from "./utils";
 
 export const getSignedUrlForUpload = async (
@@ -51,15 +50,11 @@ export const getSignedUrlForUpload = async (
       return signedUrlResult;
     }
 
-    // Use PUBLIC_URL for public files, WEBAPP_URL for private files
-    const baseUrl = accessType === "public" ? getPublicDomain() : WEBAPP_URL;
-
+    // Return relative path - can be resolved to absolute URL at runtime when needed
     return ok({
       signedUrl: signedUrlResult.data.signedUrl,
       presignedFields: signedUrlResult.data.presignedFields,
-      fileUrl: new URL(
-        `${baseUrl}/storage/${environmentId}/${accessType}/${encodeURIComponent(updatedFileName)}`
-      ).href,
+      fileUrl: `/storage/${environmentId}/${accessType}/${encodeURIComponent(updatedFileName)}`,
     });
   } catch (error) {
     logger.error({ error }, "Error getting signed url for upload");
@@ -70,24 +65,28 @@ export const getSignedUrlForUpload = async (
   }
 };
 
-export const getSignedUrlForDownload = async (
+/**
+ * Get a file stream for downloading/streaming files directly
+ * Use this instead of signed URL redirect for Next.js Image component compatibility
+ */
+export const getFileStreamForDownload = async (
   fileName: string,
   environmentId: string,
   accessType: TAccessType
-): Promise<Result<string, StorageError>> => {
+): Promise<Result<FileStreamResult, StorageError>> => {
   try {
     const fileNameDecoded = decodeURIComponent(fileName);
     const fileKey = `${environmentId}/${accessType}/${fileNameDecoded}`;
 
-    const signedUrlResult = await getSignedDownloadUrl(fileKey);
+    const streamResult = await getFileStream(fileKey);
 
-    if (!signedUrlResult.ok) {
-      return signedUrlResult;
+    if (!streamResult.ok) {
+      return streamResult;
     }
 
-    return signedUrlResult;
+    return streamResult;
   } catch (error) {
-    logger.error({ error }, "Error getting signed url for download");
+    logger.error({ error }, "Error getting file stream for download");
 
     return err({
       code: StorageErrorCode.Unknown,

apps/web/modules/storage/url-helpers.ts

@@ -0,0 +1,30 @@
+/**
+ * Client-safe URL helper utilities for storage files.
+ * These functions can be used in both server and client components.
+ */
+
+/**
+ * Extracts the original file name from a storage URL.
+ * Handles both relative paths (/storage/...) and absolute URLs.
+ * @param fileURL The storage URL to parse
+ * @returns The original file name, or empty string if parsing fails
+ */
+export const getOriginalFileNameFromUrl = (fileURL: string): string => {
+  try {
+    const lastSegment = fileURL.startsWith("/storage/")
+      ? (fileURL.split("/").pop() ?? "")
+      : (new URL(fileURL).pathname.split("/").pop() ?? "");
+    const fileNameFromURL = lastSegment.split(/[?#]/)[0];
+
+    const [namePart, fidPart] = fileNameFromURL.split("--fid--");
+    if (!fidPart) return namePart ? decodeURIComponent(namePart) : "";
+
+    const dotIdx = fileNameFromURL.lastIndexOf(".");
+    const hasExt = dotIdx > fileNameFromURL.indexOf("--fid--");
+    const ext = hasExt ? fileNameFromURL.slice(dotIdx + 1) : "";
+
+    return decodeURIComponent(ext ? `${namePart}.${ext}` : namePart);
+  } catch {
+    return "";
+  }
+};

apps/web/modules/storage/utils.test.ts

@@ -7,6 +7,7 @@ import {
   isAllowedFileExtension,
   isValidFileTypeForExtension,
   isValidImageFile,
+  resolveStorageUrl,
   sanitizeFileName,
   validateFileUploads,
   validateSingleFile,
@@ -145,7 +146,8 @@ describe("storage utils", () => {
       const { getOriginalFileNameFromUrl } =
         await vi.importActual<typeof import("@/modules/storage/utils")>("@/modules/storage/utils");
       const path = "/storage/env/public/Document%20Name.pdf";
-      expect(getOriginalFileNameFromUrl(path)).toBe("/storage/env/public/Document Name.pdf");
+      // Function extracts filename, not full path
+      expect(getOriginalFileNameFromUrl(path)).toBe("Document Name.pdf");
     });
 
     test("returns empty string on invalid URL input", async () => {
@@ -396,4 +398,38 @@ describe("storage utils", () => {
       expect(isValidImageFile("https://example.com/image.JPG")).toBe(true);
     });
   });
+
+  describe("resolveStorageUrl", () => {
+    test("should return empty string for null or undefined input", () => {
+      expect(resolveStorageUrl(null)).toBe("");
+      expect(resolveStorageUrl(undefined)).toBe("");
+      expect(resolveStorageUrl("")).toBe("");
+    });
+
+    test("should return absolute URL unchanged (backward compatibility)", () => {
+      const httpsUrl = "https://example.com/storage/env-123/public/image.jpg";
+      const httpUrl = "http://example.com/storage/env-123/public/image.jpg";
+
+      expect(resolveStorageUrl(httpsUrl)).toBe(httpsUrl);
+      expect(resolveStorageUrl(httpUrl)).toBe(httpUrl);
+    });
+
+    test("should resolve relative /storage/ path to absolute URL", async () => {
+      // Use actual implementation with mocked dependencies
+      const { resolveStorageUrl: actualResolveStorageUrl } =
+        await vi.importActual<typeof import("@/modules/storage/utils")>("@/modules/storage/utils");
+
+      const relativePath = "/storage/env-123/public/image.jpg";
+      const result = actualResolveStorageUrl(relativePath);
+
+      // Should prepend the base URL (from mocked WEBAPP_URL or getPublicDomain)
+      expect(result).toContain("/storage/env-123/public/image.jpg");
+      expect(result.startsWith("http")).toBe(true);
+    });
+
+    test("should return non-storage paths unchanged", () => {
+      expect(resolveStorageUrl("/some/other/path")).toBe("/some/other/path");
+      expect(resolveStorageUrl("relative/path.jpg")).toBe("relative/path.jpg");
+    });
+  });
 });

apps/web/modules/storage/utils.ts

@@ -1,30 +1,15 @@
-import { logger } from "@formbricks/logger";
+import "server-only";
 import { StorageError, StorageErrorCode } from "@formbricks/storage";
 import { TResponseData } from "@formbricks/types/responses";
 import { TAllowedFileExtension, ZAllowedFileExtension, mimeTypes } from "@formbricks/types/storage";
 import { TSurveyQuestion, TSurveyQuestionTypeEnum } from "@formbricks/types/surveys/types";
 import { responses } from "@/app/lib/api/response";
+import { WEBAPP_URL } from "@/lib/constants";
+import { getPublicDomain } from "@/lib/getPublicUrl";
+import { getOriginalFileNameFromUrl } from "./url-helpers";
 
-export const getOriginalFileNameFromUrl = (fileURL: string) => {
-  try {
-    const lastSegment = fileURL.startsWith("/storage/")
-      ? fileURL
-      : (new URL(fileURL).pathname.split("/").pop() ?? "");
-    const fileNameFromURL = lastSegment.split(/[?#]/)[0];
-
-    const [namePart, fidPart] = fileNameFromURL.split("--fid--");
-    if (!fidPart) return namePart ? decodeURIComponent(namePart) : "";
-
-    const dotIdx = fileNameFromURL.lastIndexOf(".");
-    const hasExt = dotIdx > fileNameFromURL.indexOf("--fid--");
-    const ext = hasExt ? fileNameFromURL.slice(dotIdx + 1) : "";
-
-    return decodeURIComponent(ext ? `${namePart}.${ext}` : namePart);
-  } catch (error) {
-    logger.error({ error, fileURL }, "Error parsing file URL");
-    return "";
-  }
-};
+// Re-export for backward compatibility with server-side code
+export { getOriginalFileNameFromUrl } from "./url-helpers";
 
 /**
  * Sanitize a provided file name to a safe subset.
@@ -163,3 +148,31 @@ export const getErrorResponseFromStorageError = (
     }
   }
 };
+
+/**
+ * Resolves a storage URL to an absolute URL.
+ * - If already absolute, returns as-is (backward compatibility for old data)
+ * - If relative (/storage/...), prepends the appropriate base URL
+ * @param url The storage URL (relative or absolute)
+ * @param accessType The access type to determine which base URL to use (defaults to "public")
+ * @returns The resolved absolute URL, or empty string if url is falsy
+ */
+export const resolveStorageUrl = (
+  url: string | undefined | null,
+  accessType: "public" | "private" = "public"
+): string => {
+  if (!url) return "";
+
+  // Already absolute URL - return as-is (backward compatibility for old data)
+  if (url.startsWith("http://") || url.startsWith("https://")) {
+    return url;
+  }
+
+  // Relative path - resolve with base URL
+  if (url.startsWith("/storage/")) {
+    const baseUrl = accessType === "public" ? getPublicDomain() : WEBAPP_URL;
+    return `${baseUrl}${url}`;
+  }
+
+  return url;
+};