Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions apps/storybook/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
"eslint-plugin-react-refresh": "0.4.26",
"eslint-plugin-storybook": "10.3.6",
"storybook": "10.3.6",
"typescript": "5.9.3",
"vite": "7.3.5"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { useTranslation } from "react-i18next";
import { type TI18nString } from "@formbricks/types/i18n";
import { TSurveyElementTypeEnum } from "@formbricks/types/surveys/elements";
import { TSurvey, TSurveyElementSummaryPictureSelection } from "@formbricks/types/surveys/types";
import { isExternalImageSrc } from "@/lib/image-hosts";
import { getChoiceIdByValue } from "@/lib/response/utils";
import { IdBadge } from "@/modules/ui/components/id-badge";
import { ProgressBar } from "@/modules/ui/components/progress-bar";
Expand Down Expand Up @@ -68,6 +69,7 @@ export const PictureChoiceSummary = ({ elementSummary, survey, setFilter }: Pict
layout="fill"
objectFit="cover"
className="rounded-md"
unoptimized={isExternalImageSrc(result.imageUrl)}
/>
</div>
<div className="self-end">{choiceId && <IdBadge id={choiceId} />}</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const mocks = vi.hoisted(() => ({
validateClientFileUploads: vi.fn(),
validateOtherOptionLengthForMultipleChoice: vi.fn(),
validateResponseData: vi.fn(),
verifyLinkSurveyPinToken: vi.fn(),
}));

vi.mock("@formbricks/logger", () => ({
Expand Down Expand Up @@ -65,6 +66,10 @@ vi.mock("./validated-response-update-input", () => ({
getValidatedResponseUpdateInput: mocks.getValidatedResponseUpdateInput,
}));

vi.mock("@/modules/survey/link/lib/pin-token", () => ({
verifyLinkSurveyPinToken: mocks.verifyLinkSurveyPinToken,
}));

const workspaceId = "workspace_a";
const responseId = "response_123";
const surveyId = "survey_123";
Expand Down Expand Up @@ -139,6 +144,7 @@ describe("putResponseHandler", () => {
mocks.validateClientFileUploads.mockReturnValue(true);
mocks.validateOtherOptionLengthForMultipleChoice.mockReturnValue(null);
mocks.validateResponseData.mockReturnValue(null);
mocks.verifyLinkSurveyPinToken.mockReturnValue(true);
});

test("returns a bad request response when the response id is missing", async () => {
Expand Down Expand Up @@ -335,6 +341,53 @@ describe("putResponseHandler", () => {
expect(mocks.updateResponseWithQuotaEvaluation).not.toHaveBeenCalled();
});

test("rejects updates to a PIN-protected survey without a valid PIN token", async () => {
mocks.getSurvey.mockResolvedValue({
...getBaseSurvey(),
pin: "1234",
});
mocks.getValidatedResponseUpdateInput.mockResolvedValue({
responseUpdateInput: { ...getBaseResponseUpdateInput(), pinAuthToken: "invalid" },
});
mocks.verifyLinkSurveyPinToken.mockReturnValue(false);

const result = await putResponseHandler(createHandlerParams());

expect(result.response.status).toBe(403);
await expect(result.response.json()).resolves.toEqual({
code: "forbidden",
message: "Survey is protected by a PIN",
details: { surveyId },
});
expect(mocks.verifyLinkSurveyPinToken).toHaveBeenCalledWith("invalid", surveyId);
expect(mocks.updateResponseWithQuotaEvaluation).not.toHaveBeenCalled();
});

test("allows updates to a PIN-protected survey with a valid PIN token", async () => {
mocks.getSurvey.mockResolvedValue({
...getBaseSurvey(),
pin: "1234",
});
mocks.getValidatedResponseUpdateInput.mockResolvedValue({
responseUpdateInput: { ...getBaseResponseUpdateInput(), pinAuthToken: "valid-token" },
});
mocks.verifyLinkSurveyPinToken.mockReturnValue(true);

const result = await putResponseHandler(createHandlerParams());

expect(result.response.status).toBe(200);
expect(mocks.verifyLinkSurveyPinToken).toHaveBeenCalledWith("valid-token", surveyId);
expect(mocks.updateResponseWithQuotaEvaluation).toHaveBeenCalled();
});

test("does not require a PIN token when the survey has no PIN", async () => {
const result = await putResponseHandler(createHandlerParams());

expect(result.response.status).toBe(200);
expect(mocks.verifyLinkSurveyPinToken).not.toHaveBeenCalled();
expect(mocks.updateResponseWithQuotaEvaluation).toHaveBeenCalled();
});

test("rejects invalid file upload updates", async () => {
mocks.validateClientFileUploads.mockReturnValue(false);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { formatValidationErrorsForV1Api, validateResponseData } from "@/modules/
import { validateOtherOptionLengthForMultipleChoice } from "@/modules/api/v2/lib/element";
import { createQuotaFullObject } from "@/modules/ee/quotas/lib/helpers";
import { validateClientFileUploads } from "@/modules/storage/utils";
import { verifyLinkSurveyPinToken } from "@/modules/survey/link/lib/pin-token";
import { updateResponseWithQuotaEvaluation } from "./response";
import { getValidatedResponseUpdateInput } from "./validated-response-update-input";

Expand Down Expand Up @@ -278,6 +279,17 @@ export const putResponseHandler = async ({
};
}

// Mirror the POST endpoint: PIN-protected link surveys require a server-verifiable PIN token.
// Without this, an unfinished response of a PIN survey could be updated/finalized without the PIN
// (CWE-602, ENG-1579).
if (survey.pin && !verifyLinkSurveyPinToken(responseUpdateInput.pinAuthToken, survey.id)) {
return {
response: responses.forbiddenResponse("Survey is protected by a PIN", true, {
surveyId: survey.id,
}),
};
}

const validationResult = validateUpdateRequest(existingResponse, survey, responseUpdateInput, workspaceId);
if (validationResult) {
return validationResult;
Expand Down
7 changes: 7 additions & 0 deletions apps/web/app/api/v1/client/[workspaceId]/responses/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { formatValidationErrorsForV1Api, validateResponseData } from "@/modules/
import { getIsContactsEnabled } from "@/modules/ee/license-check/lib/utils";
import { createQuotaFullObject } from "@/modules/ee/quotas/lib/helpers";
import { validateClientFileUploads } from "@/modules/storage/utils";
import { verifyLinkSurveyPinToken } from "@/modules/survey/link/lib/pin-token";
import { createResponseWithQuotaEvaluation } from "./lib/response";

export const OPTIONS = async (): Promise<Response> => {
Expand Down Expand Up @@ -146,6 +147,12 @@ export const POST = withV1ApiWrapper({
};
}

if (survey.pin && !verifyLinkSurveyPinToken(responseInputData.pinAuthToken, survey.id)) {
return {
response: responses.forbiddenResponse("Survey is protected by a PIN", true, { surveyId: survey.id }),
};
}

const singleUseValidationResult = validateSingleUseResponseInput(survey, workspaceId, responseInputData);
if (singleUseValidationResult) {
if ("response" in singleUseValidationResult) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { symmetricDecrypt } from "@/lib/crypto";
import { getOrganizationIdFromWorkspaceId } from "@/lib/utils/helper";
import { validateSurveySingleUseLinkParams } from "@/lib/utils/single-use-surveys";
import { getIsSpamProtectionEnabled } from "@/modules/ee/license-check/lib/utils";
import { verifyLinkSurveyPinToken } from "@/modules/survey/link/lib/pin-token";

export const RECAPTCHA_VERIFICATION_ERROR_CODE = "recaptcha_verification_failed";

Expand All @@ -33,6 +34,10 @@ export const checkSurveyValidity = async (
});
}

if (survey.pin && !verifyLinkSurveyPinToken(responseInput.pinAuthToken, survey.id)) {
return responses.forbiddenResponse("Survey is protected by a PIN", true, { surveyId: survey.id });
}

if (survey.type === "link" && survey.singleUse?.enabled) {
if (!responseInput.singleUseId) {
return responses.badRequestResponse("Missing single use id", {
Expand Down
1 change: 1 addition & 0 deletions apps/web/lib/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ export const MAIL_FROM = env.MAIL_FROM;
export const MAIL_FROM_NAME = env.MAIL_FROM_NAME;

export const NEXTAUTH_SECRET = env.NEXTAUTH_SECRET;
export const BETTER_AUTH_SECRET = env.BETTER_AUTH_SECRET;
export const ITEMS_PER_PAGE = 30;
export const SURVEYS_PER_PAGE = 12;
export const RESPONSES_PER_PAGE = 25;
Expand Down
43 changes: 43 additions & 0 deletions apps/web/lib/image-hosts.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import { type StaticImageData } from "next/image";
import { describe, expect, test } from "vitest";
import { OPTIMIZABLE_IMAGE_HOSTS, isExternalImageSrc } from "./image-hosts";

describe("isExternalImageSrc", () => {
test("treats relative first-party paths as optimizable (not external)", () => {
expect(isExternalImageSrc("/storage/ws/public/logo.png")).toBe(false);
expect(isExternalImageSrc("/images/hero.png")).toBe(false);
});

test("treats data URIs as optimizable (not external)", () => {
expect(isExternalImageSrc("data:image/png;base64,iVBORw0KGgo=")).toBe(false);
});

test("treats nullish / non-string (StaticImageData) srcs as not external", () => {
expect(isExternalImageSrc(undefined)).toBe(false);
expect(isExternalImageSrc(null)).toBe(false);
expect(isExternalImageSrc("")).toBe(false);
expect(isExternalImageSrc({ src: "/x.png", height: 1, width: 1 } as StaticImageData)).toBe(false);
});

test("treats absolute URLs on allowlisted provider hosts as optimizable", () => {
expect(isExternalImageSrc("https://images.unsplash.com/photo-1.jpg")).toBe(false);
expect(isExternalImageSrc("https://avatars.githubusercontent.com/u/1")).toBe(false);
});

test("treats arbitrary external URLs as external (must bypass the optimizer)", () => {
expect(isExternalImageSrc("https://evil.example.com/x.png")).toBe(true);
expect(isExternalImageSrc("http://random-host.test/logo.svg")).toBe(true);
// Cloud-specific infra is not in the universal allowlist — treated as external.
expect(isExternalImageSrc("https://formbricks-cdn.s3.eu-central-1.amazonaws.com/x.png")).toBe(true);
});

test("treats an absolute URL to the deployment's own domain as external (relative is the supported first-party form)", () => {
// The running domain is intentionally not in the allowlist; first-party images must be relative.
expect(isExternalImageSrc("https://app.formbricks.com/storage/ws/public/logo.png")).toBe(true);
expect(isExternalImageSrc("https://survey.company.com/storage/ws/public/logo.png")).toBe(true);
});

test("does not include the deployment domain in the optimizable host allowlist", () => {
expect(OPTIMIZABLE_IMAGE_HOSTS).not.toContain("app.formbricks.com");
});
});
34 changes: 34 additions & 0 deletions apps/web/lib/image-hosts.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import { type StaticImageData } from "next/image";
import { OPTIMIZABLE_IMAGE_HOSTS } from "./optimizable-image-hosts.mjs";

// Re-exported from the plain `.mjs` source of truth (also imported by next.config.mjs) so
// remotePatterns and the runtime check below share one list. See ./optimizable-image-hosts.mjs.
export { OPTIMIZABLE_IMAGE_HOSTS };

const OPTIMIZABLE_IMAGE_HOSTS_SET: ReadonlySet<string> = new Set(OPTIMIZABLE_IMAGE_HOSTS);

/**
* Whether an `<Image>` src must be rendered with `unoptimized` because the Next.js image optimizer
* would not (and should not) serve it.
*
* Returns `false` (i.e. optimize) for:
* - relative paths (`/storage/...`, `/images/...`) — local images, optimized via `localPatterns`;
* - `data:` URIs, `StaticImageData` imports, or empty/nullish values;
* - absolute URLs whose host is in {@link OPTIMIZABLE_IMAGE_HOSTS}.
*
* Returns `true` (i.e. bypass the optimizer, serve directly) for any other absolute `http(s)` URL —
* i.e. arbitrary user-provided external images. This keeps the optimizer from acting as an open
* proxy for hosts we don't control, without breaking rendering of those images.
*
* The decision depends only on the src string, so it is identical on the server and client (no
* hydration mismatch).
*/
export const isExternalImageSrc = (src: string | StaticImageData | null | undefined): boolean => {
if (!src || typeof src !== "string") return false;
if (!/^https?:\/\//i.test(src)) return false; // relative path or data: URI → local/optimizable
try {
return !OPTIMIZABLE_IMAGE_HOSTS_SET.has(new URL(src).hostname);
} catch {
return false;
}
};
28 changes: 28 additions & 0 deletions apps/web/lib/optimizable-image-hosts.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/**
* Hosts whose images are safe to run through the Next.js image optimizer (ENG-1678).
*
* Plain `.mjs` so `next.config.mjs` can statically import it at config-eval time (no jiti/TS needed)
* while `lib/image-hosts.ts` re-exports it for the runtime `isExternalImageSrc` check — one source of
* truth, so `images.remotePatterns` and the per-`<Image>` `unoptimized` decision can never drift.
*
* Two hard constraints shape this list:
* - `next.config` (and therefore `remotePatterns`) is frozen into the build. The same Docker image
* serves multiple domains (app.formbricks.com, ksa.formbricks.com, and every self-hoster), so the
* deployment's own domain can NOT be baked in here and is intentionally absent.
* - First-party uploads are served from same-origin `/storage/...` (relative) paths, which Next
* treats as local images (default: optimize all) and never checks against `remotePatterns`.
*
* It therefore contains only *universal* provider hosts that real features rely on and that are
* identical on every deployment.
*/
export const OPTIMIZABLE_IMAGE_HOSTS = [
// OAuth profile avatars
"avatars.githubusercontent.com",
"avatars.slack-edge.com",
"lh3.googleusercontent.com",
// survey editor's Unsplash background picker
"images.unsplash.com",
// local development
"localhost",
"127.0.0.1",
];
3 changes: 2 additions & 1 deletion apps/web/lib/turbo-build-outputs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ describe("turbo.json web build excludes transient Next.js dirs", () => {
};

// Turbo uses the package-specific task when defined, otherwise the generic one.
const resolvedOutputs = turboJson.tasks["@formbricks/web#build"]?.outputs ?? turboJson.tasks.build.outputs ?? [];
const resolvedOutputs =
turboJson.tasks["@formbricks/web#build"]?.outputs ?? turboJson.tasks.build.outputs ?? [];

test("resolved @formbricks/web#build outputs exclude .next/cache and .next/dev", () => {
const missing = REQUIRED_EXCLUSIONS.filter((exclusion) => !resolvedOutputs.includes(exclusion));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { TAllowedFileExtension } from "@formbricks/types/storage";
import { TUser } from "@formbricks/types/user";
import { SettingsCard } from "@/app/(app)/workspaces/[workspaceId]/settings/components/SettingsCard";
import { cn } from "@/lib/cn";
import { isExternalImageSrc } from "@/lib/image-hosts";
import { getFormattedErrorMessage } from "@/lib/utils/helper";
import {
removeOrganizationEmailLogoUrlAction,
Expand Down Expand Up @@ -219,6 +220,7 @@ export const EmailCustomizationSettings = ({
className="max-h-24 max-w-full object-contain"
width={192}
height={192}
unoptimized={isExternalImageSrc(logoUrl)}
/>
</div>

Expand Down Expand Up @@ -287,6 +289,7 @@ export const EmailCustomizationSettings = ({
className="mx-auto max-h-[100px] max-w-full object-contain"
width={192}
height={192}
unoptimized={isExternalImageSrc(logoUrl || fbLogoUrl)}
/>
<P className="font-bold">
{t("workspace.settings.general.email_customization_preview_email_heading", {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { useTranslation } from "react-i18next";
import { TOrganization } from "@formbricks/types/organizations";
import { TAllowedFileExtension } from "@formbricks/types/storage";
import { SettingsCard } from "@/app/(app)/workspaces/[workspaceId]/settings/components/SettingsCard";
import { isExternalImageSrc } from "@/lib/image-hosts";
import { getFormattedErrorMessage } from "@/lib/utils/helper";
import {
removeOrganizationFaviconUrlAction,
Expand Down Expand Up @@ -172,6 +173,7 @@ export const FaviconCustomizationSettings = ({
width={64}
height={64}
className="-mb-2 size-16 rounded-lg border object-contain p-1"
unoptimized={isExternalImageSrc(faviconUrl)}
/>
) : (
<FileInput
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { useTranslation } from "react-i18next";
import { TSurveyStyling } from "@formbricks/types/surveys/types";
import { TWorkspaceStyling } from "@formbricks/types/workspace";
import { cn } from "@/lib/cn";
import { isExternalImageSrc } from "@/lib/image-hosts";
import { handleFileUpload } from "@/modules/storage/file-upload";
import { showFileUploadErrorToast } from "@/modules/storage/file-upload-error";
import { AdvancedOptionToggle } from "@/modules/ui/components/advanced-option-toggle";
Expand Down Expand Up @@ -189,6 +190,7 @@ export const LogoSettingsCard = ({
height={56}
style={{ backgroundColor: logoBgColor || undefined }}
className="h-20 w-auto max-w-64 rounded-lg border object-contain p-1"
unoptimized={isExternalImageSrc(logoUrl)}
/>
</div>

Expand Down
2 changes: 2 additions & 0 deletions apps/web/modules/survey/editor/components/option-ids.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { useTranslation } from "react-i18next";
import { TSurveyElement, TSurveyElementTypeEnum } from "@formbricks/types/surveys/elements";
import { TSurveyVariable } from "@formbricks/types/surveys/types";
import { getLocalizedValue } from "@/lib/i18n/utils";
import { isExternalImageSrc } from "@/lib/image-hosts";
import { IdBadge } from "@/modules/ui/components/id-badge";
import { Label } from "@/modules/ui/components/label";

Expand Down Expand Up @@ -54,6 +55,7 @@ export const OptionIds = (props: OptionIdsProps) => {
style={{ objectFit: "cover" }}
quality={75}
className="rounded-lg transition-opacity duration-200"
unoptimized={isExternalImageSrc(imageUrl)}
/>
</div>
<IdBadge id={choice.id} />
Expand Down
2 changes: 2 additions & 0 deletions apps/web/modules/survey/editor/components/unsplash-images.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { useEffect, useRef, useState } from "react";
import toast from "react-hot-toast";
import { useTranslation } from "react-i18next";
import { TSurveyBackgroundBgType } from "@formbricks/types/surveys/types";
import { isExternalImageSrc } from "@/lib/image-hosts";
import { debounce } from "@/lib/utils/debounce";
import { Button } from "@/modules/ui/components/button";
import { Input } from "@/modules/ui/components/input";
Expand Down Expand Up @@ -213,6 +214,7 @@ export const ImageFromUnsplashSurveyBg = ({ handleBgChange }: ImageFromUnsplashS
alt={image.alt_description}
onClick={() => handleImageSelected(image.urls.regularWithAttribution, image.urls.download)}
className="h-full cursor-pointer rounded-lg object-cover"
unoptimized={isExternalImageSrc(image.urls.regularWithAttribution)}
/>
{image.authorName && (
<span className="absolute right-1 bottom-1 hidden rounded-sm bg-black/75 px-2 py-1 text-xs text-white group-hover:block">
Expand Down
Loading
Loading