mirror of
https://github.com/formbricks/formbricks.git
synced 2026-05-16 19:48:48 -05:00
fix: harden storage presigned URL issuance
This commit is contained in:
@@ -0,0 +1,243 @@
|
||||
import { NextRequest } from "next/server";
|
||||
import { beforeEach, describe, expect, test, vi } from "vitest";
|
||||
import { getOrganizationByEnvironmentId } from "@/lib/organization/service";
|
||||
import { getSurvey } from "@/lib/survey/service";
|
||||
import { getBiggerUploadFileSizePermission } from "@/modules/ee/license-check/lib/utils";
|
||||
import { getSignedUrlForUpload } from "@/modules/storage/service";
|
||||
import { POST } from "./route";
|
||||
|
||||
vi.mock("@/app/lib/api/with-api-logging", async () => {
|
||||
return {
|
||||
withV1ApiWrapper:
|
||||
({ handler }: { handler: any }) =>
|
||||
async (req: NextRequest, props: any) => {
|
||||
const result = await handler({ req, props });
|
||||
return result.response;
|
||||
},
|
||||
};
|
||||
});
|
||||
|
||||
vi.mock("@formbricks/logger", () => ({
|
||||
logger: {
|
||||
error: vi.fn(),
|
||||
},
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/organization/service", () => ({
|
||||
getOrganizationByEnvironmentId: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/survey/service", () => ({
|
||||
getSurvey: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("@/modules/ee/license-check/lib/utils", () => ({
|
||||
getBiggerUploadFileSizePermission: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("@/modules/storage/service", () => ({
|
||||
getSignedUrlForUpload: vi.fn(),
|
||||
}));
|
||||
|
||||
const environmentId = "ck12345678901234567890123";
|
||||
const surveyId = "clq5n7p1q0000m7z0h5p6g3r2";
|
||||
|
||||
const props = {
|
||||
params: Promise.resolve({
|
||||
environmentId,
|
||||
}),
|
||||
};
|
||||
|
||||
const signedUploadResponse = {
|
||||
ok: true,
|
||||
data: {
|
||||
signedUrl: "https://s3.example.com/upload",
|
||||
presignedFields: { key: "value" },
|
||||
fileUrl: `/storage/${environmentId}/private/report.pdf`,
|
||||
},
|
||||
} as const;
|
||||
|
||||
const createRequest = (body: Record<string, unknown>) =>
|
||||
new NextRequest(`http://localhost/api/v1/client/${environmentId}/storage`, {
|
||||
method: "POST",
|
||||
headers: {
|
||||
"Content-Type": "application/json",
|
||||
},
|
||||
body: JSON.stringify(body),
|
||||
});
|
||||
|
||||
const createRequestBody = (overrides: Record<string, unknown> = {}) => ({
|
||||
fileName: "report.pdf",
|
||||
fileType: "application/pdf",
|
||||
surveyId,
|
||||
...overrides,
|
||||
});
|
||||
|
||||
const createSurvey = (overrides: Record<string, unknown> = {}) => ({
|
||||
id: surveyId,
|
||||
environmentId,
|
||||
blocks: [],
|
||||
questions: [],
|
||||
...overrides,
|
||||
});
|
||||
|
||||
describe("POST /api/v1/client/[environmentId]/storage", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
vi.mocked(getOrganizationByEnvironmentId).mockResolvedValue({ id: "org-123" } as any);
|
||||
vi.mocked(getBiggerUploadFileSizePermission).mockResolvedValue(false);
|
||||
vi.mocked(getSignedUrlForUpload).mockResolvedValue(signedUploadResponse as any);
|
||||
});
|
||||
|
||||
test("creates a signed upload URL when a block file-upload element allows the extension", async () => {
|
||||
vi.mocked(getSurvey).mockResolvedValue(
|
||||
createSurvey({
|
||||
blocks: [
|
||||
{
|
||||
id: "block-1",
|
||||
name: "Block 1",
|
||||
elements: [
|
||||
{
|
||||
id: "element-1",
|
||||
type: "fileUpload",
|
||||
allowMultipleFiles: false,
|
||||
allowedFileExtensions: ["pdf"],
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
}) as any
|
||||
);
|
||||
|
||||
const response = await POST(createRequest(createRequestBody()), props);
|
||||
|
||||
expect(response.status).toBe(200);
|
||||
await expect(response.json()).resolves.toEqual({ data: signedUploadResponse.data });
|
||||
expect(getSignedUrlForUpload).toHaveBeenCalledWith(
|
||||
"report.pdf",
|
||||
environmentId,
|
||||
"application/pdf",
|
||||
"private",
|
||||
expect.any(Number)
|
||||
);
|
||||
});
|
||||
|
||||
test("creates a signed upload URL when a legacy file-upload question allows the extension", async () => {
|
||||
vi.mocked(getSurvey).mockResolvedValue(
|
||||
createSurvey({
|
||||
questions: [
|
||||
{
|
||||
id: "question-1",
|
||||
type: "fileUpload",
|
||||
allowMultipleFiles: false,
|
||||
allowedFileExtensions: ["png"],
|
||||
},
|
||||
],
|
||||
}) as any
|
||||
);
|
||||
|
||||
const response = await POST(
|
||||
createRequest(createRequestBody({ fileName: "screenshot.png", fileType: "image/png" })),
|
||||
props
|
||||
);
|
||||
|
||||
expect(response.status).toBe(200);
|
||||
expect(getSignedUrlForUpload).toHaveBeenCalledWith(
|
||||
"screenshot.png",
|
||||
environmentId,
|
||||
"image/png",
|
||||
"private",
|
||||
expect.any(Number)
|
||||
);
|
||||
});
|
||||
|
||||
test("rejects uploads when the survey has no file-upload elements or questions", async () => {
|
||||
vi.mocked(getSurvey).mockResolvedValue(
|
||||
createSurvey({
|
||||
blocks: [
|
||||
{
|
||||
id: "block-1",
|
||||
name: "Block 1",
|
||||
elements: [
|
||||
{
|
||||
id: "element-1",
|
||||
type: "openText",
|
||||
headline: { default: "Question" },
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
}) as any
|
||||
);
|
||||
|
||||
const response = await POST(createRequest(createRequestBody()), props);
|
||||
|
||||
expect(response.status).toBe(400);
|
||||
await expect(response.json()).resolves.toMatchObject({
|
||||
code: "bad_request",
|
||||
message: "Survey does not allow file uploads",
|
||||
});
|
||||
expect(getSignedUrlForUpload).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test("rejects uploads when the survey file-upload config does not allow the extension", async () => {
|
||||
vi.mocked(getSurvey).mockResolvedValue(
|
||||
createSurvey({
|
||||
blocks: [
|
||||
{
|
||||
id: "block-1",
|
||||
name: "Block 1",
|
||||
elements: [
|
||||
{
|
||||
id: "element-1",
|
||||
type: "fileUpload",
|
||||
allowMultipleFiles: false,
|
||||
allowedFileExtensions: ["png"],
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
}) as any
|
||||
);
|
||||
|
||||
const response = await POST(createRequest(createRequestBody({ allowedFileExtensions: ["pdf"] })), props);
|
||||
|
||||
expect(response.status).toBe(400);
|
||||
await expect(response.json()).resolves.toMatchObject({
|
||||
code: "bad_request",
|
||||
message: "File extension is not allowed for this survey",
|
||||
});
|
||||
expect(getSignedUrlForUpload).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test("allows globally safe extensions when a survey file-upload entry has no extension restriction", async () => {
|
||||
vi.mocked(getSurvey).mockResolvedValue(
|
||||
createSurvey({
|
||||
blocks: [
|
||||
{
|
||||
id: "block-1",
|
||||
name: "Block 1",
|
||||
elements: [
|
||||
{
|
||||
id: "element-1",
|
||||
type: "fileUpload",
|
||||
allowMultipleFiles: false,
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
}) as any
|
||||
);
|
||||
|
||||
const response = await POST(createRequest(createRequestBody()), props);
|
||||
|
||||
expect(response.status).toBe(200);
|
||||
expect(getSignedUrlForUpload).toHaveBeenCalledWith(
|
||||
"report.pdf",
|
||||
environmentId,
|
||||
"application/pdf",
|
||||
"private",
|
||||
expect.any(Number)
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -9,7 +9,7 @@ import { getSurvey } from "@/lib/survey/service";
|
||||
import { rateLimitConfigs } from "@/modules/core/rate-limit/rate-limit-configs";
|
||||
import { getBiggerUploadFileSizePermission } from "@/modules/ee/license-check/lib/utils";
|
||||
import { getSignedUrlForUpload } from "@/modules/storage/service";
|
||||
import { getErrorResponseFromStorageError } from "@/modules/storage/utils";
|
||||
import { getErrorResponseFromStorageError, validateSurveyAllowsFileUpload } from "@/modules/storage/utils";
|
||||
|
||||
export const OPTIONS = async (): Promise<Response> => {
|
||||
return responses.successResponse(
|
||||
@@ -79,6 +79,24 @@ export const POST = withV1ApiWrapper({
|
||||
};
|
||||
}
|
||||
|
||||
const fileUploadPermission = validateSurveyAllowsFileUpload({
|
||||
fileName,
|
||||
blocks: survey.blocks,
|
||||
questions: survey.questions,
|
||||
});
|
||||
|
||||
if (!fileUploadPermission.ok) {
|
||||
return {
|
||||
response: responses.badRequestResponse(
|
||||
fileUploadPermission.reason === "no_file_upload_question"
|
||||
? "Survey does not allow file uploads"
|
||||
: "File extension is not allowed for this survey",
|
||||
undefined,
|
||||
true
|
||||
),
|
||||
};
|
||||
}
|
||||
|
||||
const isBiggerFileUploadAllowed = await getBiggerUploadFileSizePermission(organization.id);
|
||||
const maxFileUploadSize = isBiggerFileUploadAllowed
|
||||
? MAX_FILE_UPLOAD_SIZES.big
|
||||
|
||||
@@ -1,7 +1,9 @@
|
||||
import "server-only";
|
||||
import { StorageError, StorageErrorCode } from "@formbricks/storage";
|
||||
import { StorageErrorCode } from "@formbricks/storage";
|
||||
import { TResponseData } from "@formbricks/types/responses";
|
||||
import { TAllowedFileExtension, ZAllowedFileExtension } from "@formbricks/types/storage";
|
||||
import { TSurveyBlock } from "@formbricks/types/surveys/blocks";
|
||||
import { TSurveyElementTypeEnum } from "@formbricks/types/surveys/elements";
|
||||
import { TSurveyQuestion, TSurveyQuestionTypeEnum } from "@formbricks/types/surveys/types";
|
||||
import { responses } from "@/app/lib/api/response";
|
||||
import { WEBAPP_URL } from "@/lib/constants";
|
||||
@@ -11,6 +13,10 @@ import { getOriginalFileNameFromUrl } from "./url-helpers";
|
||||
// Re-export for backward compatibility with server-side code
|
||||
export { getOriginalFileNameFromUrl } from "./url-helpers";
|
||||
|
||||
type TStorageError = {
|
||||
code: (typeof StorageErrorCode)[keyof typeof StorageErrorCode];
|
||||
};
|
||||
|
||||
/**
|
||||
* Sanitize a provided file name to a safe subset.
|
||||
* - Removes path separators and backslashes to avoid implicit prefixes
|
||||
@@ -100,6 +106,77 @@ export const validateFileUploads = (data?: TResponseData, questions?: TSurveyQue
|
||||
return true;
|
||||
};
|
||||
|
||||
type TSurveyFileUploadConfig = {
|
||||
allowedFileExtensions?: TAllowedFileExtension[];
|
||||
};
|
||||
|
||||
export type TSurveyFileUploadPermissionResult =
|
||||
| {
|
||||
ok: true;
|
||||
}
|
||||
| {
|
||||
ok: false;
|
||||
reason: "no_file_upload_question" | "file_extension_not_allowed";
|
||||
};
|
||||
|
||||
const getAllowedFileExtensionFromFileName = (fileName: string): TAllowedFileExtension | null => {
|
||||
const extension = fileName.split(".").pop()?.toLowerCase();
|
||||
|
||||
if (!extension || extension === fileName.toLowerCase()) {
|
||||
return null;
|
||||
}
|
||||
|
||||
const extensionValidation = ZAllowedFileExtension.safeParse(extension);
|
||||
|
||||
return extensionValidation.success ? extensionValidation.data : null;
|
||||
};
|
||||
|
||||
export const validateSurveyAllowsFileUpload = ({
|
||||
fileName,
|
||||
blocks,
|
||||
questions,
|
||||
}: {
|
||||
fileName: string;
|
||||
blocks?: TSurveyBlock[] | null;
|
||||
questions?: TSurveyQuestion[] | null;
|
||||
}): TSurveyFileUploadPermissionResult => {
|
||||
const fileUploadConfigs: TSurveyFileUploadConfig[] = [
|
||||
...(blocks ?? [])
|
||||
.flatMap((block) => block.elements)
|
||||
.filter((element) => element.type === TSurveyElementTypeEnum.FileUpload),
|
||||
...(questions ?? []).filter((question) => question.type === TSurveyQuestionTypeEnum.FileUpload),
|
||||
];
|
||||
|
||||
if (fileUploadConfigs.length === 0) {
|
||||
return {
|
||||
ok: false,
|
||||
reason: "no_file_upload_question",
|
||||
};
|
||||
}
|
||||
|
||||
const fileExtension = getAllowedFileExtensionFromFileName(fileName);
|
||||
|
||||
if (!fileExtension) {
|
||||
return {
|
||||
ok: false,
|
||||
reason: "file_extension_not_allowed",
|
||||
};
|
||||
}
|
||||
|
||||
const isFileExtensionAllowed = fileUploadConfigs.some((fileUploadConfig) => {
|
||||
const { allowedFileExtensions } = fileUploadConfig;
|
||||
|
||||
return allowedFileExtensions === undefined || allowedFileExtensions.includes(fileExtension);
|
||||
});
|
||||
|
||||
return isFileExtensionAllowed
|
||||
? { ok: true }
|
||||
: {
|
||||
ok: false,
|
||||
reason: "file_extension_not_allowed",
|
||||
};
|
||||
};
|
||||
|
||||
export const isValidImageFile = (fileUrl: string): boolean => {
|
||||
const fileName = getOriginalFileNameFromUrl(fileUrl);
|
||||
if (!fileName || fileName.endsWith(".")) return false;
|
||||
@@ -112,7 +189,7 @@ export const isValidImageFile = (fileUrl: string): boolean => {
|
||||
};
|
||||
|
||||
export const getErrorResponseFromStorageError = (
|
||||
error: StorageError,
|
||||
error: TStorageError,
|
||||
details?: Record<string, string>
|
||||
): Response => {
|
||||
switch (error.code) {
|
||||
|
||||
Reference in New Issue
Block a user