From 53850c96db0748769aa1c66af6d1900c9d5cbd36 Mon Sep 17 00:00:00 2001 From: Vijay Date: Wed, 7 May 2025 00:11:35 +0530 Subject: [PATCH] fix: sonar security hotspots (https, --ignore-scripts, api_key, math.random) (#5538) Co-authored-by: Matthias Nannt --- apps/web/Dockerfile | 8 +++---- apps/web/lib/constants.ts | 2 +- .../lib/survey/tests/__mock__/survey.mock.ts | 2 +- apps/web/lib/telemetry.ts | 2 +- .../editor/components/open-question-form.tsx | 2 +- .../src/lib/common/tests/utils.test.ts | 23 ++++++++++++++----- packages/js-core/src/lib/common/utils.ts | 8 ++++++- packages/surveys/src/lib/utils.ts | 10 ++++++-- 8 files changed, 40 insertions(+), 17 deletions(-) diff --git a/apps/web/Dockerfile b/apps/web/Dockerfile index 447f012845..7506e4fc19 100644 --- a/apps/web/Dockerfile +++ b/apps/web/Dockerfile @@ -18,7 +18,7 @@ FROM node:22-alpine3.21 AS base FROM base AS installer # Enable corepack and prepare pnpm -RUN npm install -g corepack@latest +RUN npm install --ignore-scripts -g corepack@latest RUN corepack enable RUN corepack prepare pnpm@9.15.0 --activate @@ -60,7 +60,7 @@ COPY . . RUN touch apps/web/.env # Install the dependencies -RUN pnpm install +RUN pnpm install --ignore-scripts # Build the project using our secret reader script # This mounts the secrets only during this build step without storing them in layers @@ -76,7 +76,7 @@ RUN jq -r '.devDependencies.prisma' packages/database/package.json > /prisma_ver # FROM base AS runner -RUN npm install -g corepack@latest +RUN npm install --ignore-scripts -g corepack@latest RUN corepack enable RUN corepack prepare pnpm@9.15.0 --activate @@ -143,7 +143,7 @@ RUN chmod -R 755 ./node_modules/@noble/hashes COPY --from=installer /app/node_modules/zod ./node_modules/zod RUN chmod -R 755 ./node_modules/zod -RUN npm install -g tsx typescript prisma pino-pretty +RUN npm install --ignore-scripts -g tsx typescript prisma pino-pretty EXPOSE 3000 ENV HOSTNAME "0.0.0.0" diff --git a/apps/web/lib/constants.ts b/apps/web/lib/constants.ts index 5e8432e13a..ecae3f2373 100644 --- a/apps/web/lib/constants.ts +++ b/apps/web/lib/constants.ts @@ -82,7 +82,7 @@ export const AIRTABLE_CLIENT_ID = env.AIRTABLE_CLIENT_ID; export const SMTP_HOST = env.SMTP_HOST; export const SMTP_PORT = env.SMTP_PORT; -export const SMTP_SECURE_ENABLED = env.SMTP_SECURE_ENABLED === "1"; +export const SMTP_SECURE_ENABLED = env.SMTP_SECURE_ENABLED === "1" || env.SMTP_PORT === "465"; export const SMTP_USER = env.SMTP_USER; export const SMTP_PASSWORD = env.SMTP_PASSWORD; export const SMTP_AUTHENTICATED = env.SMTP_AUTHENTICATED !== "0"; diff --git a/apps/web/lib/survey/tests/__mock__/survey.mock.ts b/apps/web/lib/survey/tests/__mock__/survey.mock.ts index c682356eaf..7b1ec38e36 100644 --- a/apps/web/lib/survey/tests/__mock__/survey.mock.ts +++ b/apps/web/lib/survey/tests/__mock__/survey.mock.ts @@ -202,7 +202,7 @@ const baseSurveyProperties = { autoComplete: 7, runOnDate: null, closeOnDate: currentDate, - redirectUrl: "http://github.com/formbricks/formbricks", + redirectUrl: "https://github.com/formbricks/formbricks", recontactDays: 3, displayLimit: 3, welcomeCard: mockWelcomeCard, diff --git a/apps/web/lib/telemetry.ts b/apps/web/lib/telemetry.ts index da96a81103..da9043865c 100644 --- a/apps/web/lib/telemetry.ts +++ b/apps/web/lib/telemetry.ts @@ -22,7 +22,7 @@ export const captureTelemetry = async (eventName: string, properties = {}) => { method: "POST", headers: { "Content-Type": "application/json" }, body: JSON.stringify({ - api_key: "phc_SoIFUJ8b9ufDm0YOnoOxJf6PXyuHpO7N6RztxFdZTy", + api_key: "phc_SoIFUJ8b9ufDm0YOnoOxJf6PXyuHpO7N6RztxFdZTy", // NOSONAR // This is a public API key for telemetry and not a secret event: eventName, properties: { distinct_id: getTelemetryId(), diff --git a/apps/web/modules/survey/editor/components/open-question-form.tsx b/apps/web/modules/survey/editor/components/open-question-form.tsx index c527d43daf..aad6f2772e 100644 --- a/apps/web/modules/survey/editor/components/open-question-form.tsx +++ b/apps/web/modules/survey/editor/components/open-question-form.tsx @@ -232,7 +232,7 @@ const getPlaceholderByInputType = (inputType: TSurveyOpenTextQuestionInputType) case "email": return "example@email.com"; case "url": - return "http://..."; + return "https://..."; case "number": return "42"; case "phone": diff --git a/packages/js-core/src/lib/common/tests/utils.test.ts b/packages/js-core/src/lib/common/tests/utils.test.ts index 86d1ca812c..acb4689352 100644 --- a/packages/js-core/src/lib/common/tests/utils.test.ts +++ b/packages/js-core/src/lib/common/tests/utils.test.ts @@ -399,16 +399,27 @@ describe("utils.ts", () => { // --------------------------------------------------------------------------------- describe("shouldDisplayBasedOnPercentage()", () => { test("returns true if random number <= displayPercentage", () => { - // We'll mock Math.random to return something - const mockedRandom = vi.spyOn(Math, "random").mockReturnValue(0.2); // 0.2 => 20% - // displayPercentage = 30 => 30% => we should display + const mockGetRandomValues = vi + .spyOn(crypto, "getRandomValues") + .mockImplementation((array: T): T => { + if (array instanceof Uint32Array) { + array[0] = Math.floor((20 / 100) * 2 ** 32); + return array; + } + return array; + }); expect(shouldDisplayBasedOnPercentage(30)).toBe(true); - mockedRandom.mockReturnValue(0.5); // 50% + mockGetRandomValues.mockImplementation((array: T): T => { + if (array instanceof Uint32Array) { + array[0] = Math.floor((80 / 100) * 2 ** 32); + return array; + } + return array; + }); expect(shouldDisplayBasedOnPercentage(30)).toBe(false); - // restore - mockedRandom.mockRestore(); + mockGetRandomValues.mockRestore(); }); }); diff --git a/packages/js-core/src/lib/common/utils.ts b/packages/js-core/src/lib/common/utils.ts index 667ac511dc..2d23de1445 100644 --- a/packages/js-core/src/lib/common/utils.ts +++ b/packages/js-core/src/lib/common/utils.ts @@ -183,8 +183,14 @@ export const getLanguageCode = (survey: TEnvironmentStateSurvey, language?: stri return selectedLanguage.language.code; }; +export const getSecureRandom = (): number => { + const u32 = new Uint32Array(1); + crypto.getRandomValues(u32); + return u32[0] / 2 ** 32; // Normalized to [0, 1) +}; + export const shouldDisplayBasedOnPercentage = (displayPercentage: number): boolean => { - const randomNum = Math.floor(Math.random() * 10000) / 100; // NOSONAR typescript:S2245 // Math.random() is not used in a security context + const randomNum = Math.floor(getSecureRandom() * 10000) / 100; return randomNum <= displayPercentage; }; diff --git a/packages/surveys/src/lib/utils.ts b/packages/surveys/src/lib/utils.ts index f10325c9d4..7ae4507330 100644 --- a/packages/surveys/src/lib/utils.ts +++ b/packages/surveys/src/lib/utils.ts @@ -15,9 +15,15 @@ export const cn = (...classes: string[]) => { return classes.filter(Boolean).join(" "); }; +export const getSecureRandom = (): number => { + const u32 = new Uint32Array(1); + crypto.getRandomValues(u32); + return u32[0] / 2 ** 32; // Normalized to [0, 1) +}; + const shuffle = (array: unknown[]) => { - for (let i = 0; i < array.length; i++) { - const j = Math.floor(Math.random() * (i + 1)); // NOSONAR typescript:S2245 // Math.random() is not used in a security context + for (let i = array.length - 1; i > 0; i--) { + const j = Math.floor(getSecureRandom() * (i + 1)); [array[i], array[j]] = [array[j], array[i]]; } };