Compare commits

...

4 Commits

Author SHA1 Message Date
Cursor Agent
207ad3c7bd fix: wrap custom script execution in try-catch to prevent ReferenceError
Fixes FORMBRICKS-TK

Custom scripts injected via CustomScriptsInjector can attempt to access
undefined global variables (like zp_token) before they're defined, causing
a ReferenceError that breaks the survey page.

This fix wraps inline script content in an IIFE with try-catch to gracefully
handle errors from undefined variables. Also adds error handlers for external
scripts that fail to load.

The survey page will now continue to function even when custom scripts
encounter runtime errors, with warnings logged to the console for debugging.
2026-03-17 18:30:40 +00:00
Dhruwang Jariwala
1e7817fb69 fix: pre-strip style attributes before DOMPurify to prevent CSP violations (#7489)
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: pandeymangg <anshuman.pandey9999@gmail.com>
2026-03-17 15:33:44 +00:00
Anshuman Pandey
f250bc7e88 fix: fixes race between setUserId and trigger (#7498)
Co-authored-by: Dhruwang <dhruwangjariwala18@gmail.com>
2026-03-17 08:57:07 +00:00
Santosh
c7faa29437 fix: derive organizationId from resources in server actions to prevent cross-org IDOR (#7409)
Co-authored-by: Dhruwang <dhruwangjariwala18@gmail.com>
2026-03-17 05:36:58 +00:00
22 changed files with 561 additions and 55 deletions

View File

@@ -64,15 +64,17 @@ export const sendEmbedSurveyPreviewEmailAction = authenticatedActionClient
const ZResetSurveyAction = z.object({
surveyId: ZId,
organizationId: ZId,
projectId: ZId,
});
export const resetSurveyAction = authenticatedActionClient.inputSchema(ZResetSurveyAction).action(
withAuditLogging("updated", "survey", async ({ ctx, parsedInput }) => {
const organizationId = await getOrganizationIdFromSurveyId(parsedInput.surveyId);
const projectId = await getProjectIdFromSurveyId(parsedInput.surveyId);
await checkAuthorizationUpdated({
userId: ctx.user.id,
organizationId: parsedInput.organizationId,
organizationId,
access: [
{
type: "organization",
@@ -81,12 +83,12 @@ export const resetSurveyAction = authenticatedActionClient.inputSchema(ZResetSur
{
type: "projectTeam",
minPermission: "readWrite",
projectId: parsedInput.projectId,
projectId,
},
],
});
ctx.auditLoggingCtx.organizationId = parsedInput.organizationId;
ctx.auditLoggingCtx.organizationId = organizationId;
ctx.auditLoggingCtx.surveyId = parsedInput.surveyId;
ctx.auditLoggingCtx.oldObject = null;

View File

@@ -64,7 +64,7 @@ export const SurveyAnalysisCTA = ({
const [isResetModalOpen, setIsResetModalOpen] = useState(false);
const [isResetting, setIsResetting] = useState(false);
const { organizationId, project } = useEnvironment();
const { project } = useEnvironment();
const { refreshSingleUseId } = useSingleUseId(survey, isReadOnly);
const appSetupCompleted = survey.type === "app" && environment.appSetupCompleted;
@@ -128,7 +128,6 @@ export const SurveyAnalysisCTA = ({
setIsResetting(true);
const result = await resetSurveyAction({
surveyId: survey.id,
organizationId: organizationId,
projectId: project.id,
});
if (result?.data) {

View File

@@ -217,7 +217,7 @@ describe("utils", () => {
});
describe("logApiError", () => {
test("logs API error details", () => {
test("logs API error details with method and path", () => {
// Mock the withContext method and its returned error method
const errorMock = vi.fn();
const withContextMock = vi.fn().mockReturnValue({
@@ -228,7 +228,7 @@ describe("utils", () => {
const originalWithContext = logger.withContext;
logger.withContext = withContextMock;
const mockRequest = new Request("http://localhost/api/test");
const mockRequest = new Request("http://localhost/api/v2/management/surveys", { method: "POST" });
mockRequest.headers.set("x-request-id", "123");
const error: ApiErrorResponseV2 = {
@@ -238,9 +238,11 @@ describe("utils", () => {
logApiError(mockRequest, error);
// Verify withContext was called with the expected context
// Verify withContext was called with the expected context including method and path
expect(withContextMock).toHaveBeenCalledWith({
correlationId: "123",
method: "POST",
path: "/api/v2/management/surveys",
error,
});
@@ -275,6 +277,8 @@ describe("utils", () => {
// Verify withContext was called with the expected context
expect(withContextMock).toHaveBeenCalledWith({
correlationId: "",
method: "GET",
path: "/api/test",
error,
});
@@ -285,7 +289,7 @@ describe("utils", () => {
logger.withContext = originalWithContext;
});
test("log API error details with SENTRY_DSN set", () => {
test("log API error details with SENTRY_DSN set includes method and path tags", () => {
// Mock the withContext method and its returned error method
const errorMock = vi.fn();
const withContextMock = vi.fn().mockReturnValue({
@@ -295,11 +299,23 @@ describe("utils", () => {
// Mock Sentry's captureException method
vi.mocked(Sentry.captureException).mockImplementation((() => {}) as any);
// Capture the scope mock for tag verification
const scopeSetTagMock = vi.fn();
vi.mocked(Sentry.withScope).mockImplementation((callback: (scope: any) => void) => {
const mockScope = {
setTag: scopeSetTagMock,
setContext: vi.fn(),
setLevel: vi.fn(),
setExtra: vi.fn(),
};
callback(mockScope);
});
// Replace the original withContext with our mock
const originalWithContext = logger.withContext;
logger.withContext = withContextMock;
const mockRequest = new Request("http://localhost/api/test");
const mockRequest = new Request("http://localhost/api/v2/management/surveys", { method: "DELETE" });
mockRequest.headers.set("x-request-id", "123");
const error: ApiErrorResponseV2 = {
@@ -309,20 +325,60 @@ describe("utils", () => {
logApiError(mockRequest, error);
// Verify withContext was called with the expected context
// Verify withContext was called with the expected context including method and path
expect(withContextMock).toHaveBeenCalledWith({
correlationId: "123",
method: "DELETE",
path: "/api/v2/management/surveys",
error,
});
// Verify error was called on the child logger
expect(errorMock).toHaveBeenCalledWith("API V2 Error Details");
// Verify Sentry scope tags include method and path
expect(scopeSetTagMock).toHaveBeenCalledWith("correlationId", "123");
expect(scopeSetTagMock).toHaveBeenCalledWith("method", "DELETE");
expect(scopeSetTagMock).toHaveBeenCalledWith("path", "/api/v2/management/surveys");
// Verify Sentry.captureException was called
expect(Sentry.captureException).toHaveBeenCalled();
// Restore the original method
logger.withContext = originalWithContext;
});
test("does not send to Sentry for non-internal_server_error types", () => {
// Mock the withContext method and its returned error method
const errorMock = vi.fn();
const withContextMock = vi.fn().mockReturnValue({
error: errorMock,
});
vi.mocked(Sentry.captureException).mockClear();
// Replace the original withContext with our mock
const originalWithContext = logger.withContext;
logger.withContext = withContextMock;
const mockRequest = new Request("http://localhost/api/v2/management/surveys");
mockRequest.headers.set("x-request-id", "456");
const error: ApiErrorResponseV2 = {
type: "not_found",
details: [{ field: "survey", issue: "not found" }],
};
logApiError(mockRequest, error);
// Verify Sentry.captureException was NOT called for non-500 errors
expect(Sentry.captureException).not.toHaveBeenCalled();
// But structured logging should still happen
expect(errorMock).toHaveBeenCalledWith("API V2 Error Details");
// Restore the original method
logger.withContext = originalWithContext;
});
});
});

View File

@@ -6,13 +6,18 @@ import { ApiErrorResponseV2 } from "@/modules/api/v2/types/api-error";
export const logApiErrorEdge = (request: Request, error: ApiErrorResponseV2): void => {
const correlationId = request.headers.get("x-request-id") ?? "";
const method = request.method;
const url = new URL(request.url);
const path = url.pathname;
// Send the error to Sentry if the DSN is set and the error type is internal_server_error
// This is useful for tracking down issues without overloading Sentry with errors
if (SENTRY_DSN && IS_PRODUCTION && error.type === "internal_server_error") {
// Use Sentry scope to add correlation ID as a tag for easy filtering
// Use Sentry scope to add correlation ID and request context as tags for easy filtering
Sentry.withScope((scope) => {
scope.setTag("correlationId", correlationId);
scope.setTag("method", method);
scope.setTag("path", path);
scope.setLevel("error");
scope.setExtra("originalError", error);
@@ -24,6 +29,8 @@ export const logApiErrorEdge = (request: Request, error: ApiErrorResponseV2): vo
logger
.withContext({
correlationId,
method,
path,
error,
})
.error("API V2 Error Details");

View File

@@ -97,14 +97,13 @@ export const createSegmentAction = authenticatedActionClient.inputSchema(ZSegmen
);
const ZUpdateSegmentAction = z.object({
environmentId: ZId,
segmentId: ZId,
data: ZSegmentUpdateInput,
});
export const updateSegmentAction = authenticatedActionClient.inputSchema(ZUpdateSegmentAction).action(
withAuditLogging("updated", "segment", async ({ ctx, parsedInput }) => {
const organizationId = await getOrganizationIdFromEnvironmentId(parsedInput.environmentId);
const organizationId = await getOrganizationIdFromSegmentId(parsedInput.segmentId);
await checkAuthorizationUpdated({
userId: ctx.user.id,
organizationId,

View File

@@ -75,7 +75,6 @@ export function SegmentSettings({
try {
setIsUpdatingSegment(true);
const data = await updateSegmentAction({
environmentId,
segmentId: segment.id,
data: {
title: segment.title,

View File

@@ -124,7 +124,7 @@ export function TargetingCard({
};
const handleSaveAsNewSegmentUpdate = async (segmentId: string, data: TSegmentUpdateInput) => {
const updatedSegment = await updateSegmentAction({ segmentId, environmentId, data });
const updatedSegment = await updateSegmentAction({ segmentId, data });
return updatedSegment?.data as TSegment;
};
@@ -136,7 +136,7 @@ export function TargetingCard({
const handleSaveSegment = async (data: TSegmentUpdateInput) => {
try {
if (!segment) throw new Error(t("environments.segments.invalid_segment"));
const result = await updateSegmentAction({ segmentId: segment.id, environmentId, data });
const result = await updateSegmentAction({ segmentId: segment.id, data });
if (result?.serverError) {
toast.error(getFormattedErrorMessage(result));
return;

View File

@@ -21,7 +21,6 @@ import { getOrganizationBilling } from "@/modules/survey/lib/survey";
const ZDeleteQuotaAction = z.object({
quotaId: ZId,
surveyId: ZId,
});
const checkQuotasEnabled = async (organizationId: string) => {
@@ -37,7 +36,7 @@ const checkQuotasEnabled = async (organizationId: string) => {
export const deleteQuotaAction = authenticatedActionClient.inputSchema(ZDeleteQuotaAction).action(
withAuditLogging("deleted", "quota", async ({ ctx, parsedInput }) => {
const organizationId = await getOrganizationIdFromSurveyId(parsedInput.surveyId);
const organizationId = await getOrganizationIdFromQuotaId(parsedInput.quotaId);
await checkQuotasEnabled(organizationId);
await checkAuthorizationUpdated({
userId: ctx.user.id,
@@ -49,7 +48,7 @@ export const deleteQuotaAction = authenticatedActionClient.inputSchema(ZDeleteQu
},
{
type: "projectTeam",
projectId: await getProjectIdFromSurveyId(parsedInput.surveyId),
projectId: await getProjectIdFromQuotaId(parsedInput.quotaId),
minPermission: "readWrite",
},
],
@@ -72,7 +71,7 @@ const ZUpdateQuotaAction = z.object({
export const updateQuotaAction = authenticatedActionClient.inputSchema(ZUpdateQuotaAction).action(
withAuditLogging("updated", "quota", async ({ ctx, parsedInput }) => {
const organizationId = await getOrganizationIdFromSurveyId(parsedInput.quota.surveyId);
const organizationId = await getOrganizationIdFromQuotaId(parsedInput.quotaId);
await checkQuotasEnabled(organizationId);
await checkAuthorizationUpdated({
userId: ctx.user.id,
@@ -84,7 +83,7 @@ export const updateQuotaAction = authenticatedActionClient.inputSchema(ZUpdateQu
},
{
type: "projectTeam",
projectId: await getProjectIdFromSurveyId(parsedInput.quota.surveyId),
projectId: await getProjectIdFromQuotaId(parsedInput.quotaId),
minPermission: "readWrite",
},
],

View File

@@ -85,7 +85,6 @@ export const QuotasCard = ({
setIsDeletingQuota(true);
const deleteQuotaActionResult = await deleteQuotaAction({
quotaId: quotaId,
surveyId: localSurvey.id,
});
if (deleteQuotaActionResult?.data) {
toast.success(t("environments.surveys.edit.quotas.quota_deleted_successfull_toast"));

View File

@@ -10,6 +10,7 @@ import { getUserManagementAccess } from "@/lib/membership/utils";
import { getOrganization } from "@/lib/organization/service";
import { authenticatedActionClient } from "@/lib/utils/action-client";
import { checkAuthorizationUpdated } from "@/lib/utils/action-client/action-client-middleware";
import { getOrganizationIdFromInviteId } from "@/lib/utils/helper";
import { withAuditLogging } from "@/modules/ee/audit-logs/lib/handler";
import { getAccessControlPermission } from "@/modules/ee/license-check/lib/utils";
import { updateInvite } from "@/modules/ee/role-management/lib/invite";
@@ -31,7 +32,6 @@ export const checkRoleManagementPermission = async (organizationId: string) => {
const ZUpdateInviteAction = z.object({
inviteId: ZUuid,
organizationId: ZId,
data: ZInviteUpdateInput,
});
@@ -39,17 +39,16 @@ export type TUpdateInviteAction = z.infer<typeof ZUpdateInviteAction>;
export const updateInviteAction = authenticatedActionClient.inputSchema(ZUpdateInviteAction).action(
withAuditLogging("updated", "invite", async ({ ctx, parsedInput }) => {
const currentUserMembership = await getMembershipByUserIdOrganizationId(
ctx.user.id,
parsedInput.organizationId
);
const organizationId = await getOrganizationIdFromInviteId(parsedInput.inviteId);
const currentUserMembership = await getMembershipByUserIdOrganizationId(ctx.user.id, organizationId);
if (!currentUserMembership) {
throw new AuthenticationError("User not a member of this organization");
}
await checkAuthorizationUpdated({
userId: ctx.user.id,
organizationId: parsedInput.organizationId,
organizationId,
access: [
{
data: parsedInput.data,
@@ -68,9 +67,9 @@ export const updateInviteAction = authenticatedActionClient.inputSchema(ZUpdateI
throw new OperationNotAllowedError("Managers can only invite members");
}
await checkRoleManagementPermission(parsedInput.organizationId);
await checkRoleManagementPermission(organizationId);
ctx.auditLoggingCtx.organizationId = parsedInput.organizationId;
ctx.auditLoggingCtx.organizationId = organizationId;
ctx.auditLoggingCtx.inviteId = parsedInput.inviteId;
ctx.auditLoggingCtx.oldObject = { ...(await getInvite(parsedInput.inviteId)) };

View File

@@ -65,7 +65,7 @@ export function EditMembershipRole({
}
if (inviteId) {
await updateInviteAction({ inviteId: inviteId, organizationId, data: { role } });
await updateInviteAction({ inviteId: inviteId, data: { role } });
}
} catch (error) {
toast.error(t("common.something_went_wrong_please_try_again"));

View File

@@ -27,14 +27,15 @@ import { deleteInvite, getInvite, inviteUser, refreshInviteExpiration, resendInv
const ZDeleteInviteAction = z.object({
inviteId: ZUuid,
organizationId: ZId,
});
export const deleteInviteAction = authenticatedActionClient.inputSchema(ZDeleteInviteAction).action(
withAuditLogging("deleted", "invite", async ({ ctx, parsedInput }) => {
const organizationId = await getOrganizationIdFromInviteId(parsedInput.inviteId);
await checkAuthorizationUpdated({
userId: ctx.user.id,
organizationId: parsedInput.organizationId,
organizationId,
access: [
{
type: "organization",
@@ -42,7 +43,7 @@ export const deleteInviteAction = authenticatedActionClient.inputSchema(ZDeleteI
},
],
});
ctx.auditLoggingCtx.organizationId = parsedInput.organizationId;
ctx.auditLoggingCtx.organizationId = organizationId;
ctx.auditLoggingCtx.inviteId = parsedInput.inviteId;
ctx.auditLoggingCtx.oldObject = { ...(await getInvite(parsedInput.inviteId)) };
return await deleteInvite(parsedInput.inviteId);

View File

@@ -41,7 +41,7 @@ export const MemberActions = ({ organization, member, invite, showDeleteButton }
if (!member && invite) {
// This is an invite
const result = await deleteInviteAction({ inviteId: invite?.id, organizationId: organization.id });
const result = await deleteInviteAction({ inviteId: invite?.id });
if (result?.serverError) {
toast.error(getFormattedErrorMessage(result));
setIsDeleting(false);

View File

@@ -56,11 +56,26 @@ export const CustomScriptsInjector = ({
newScript.setAttribute(attr.name, attr.value);
});
// Copy inline script content
// Copy inline script content with error handling
if (script.textContent) {
newScript.textContent = script.textContent;
// Wrap inline scripts in try-catch to prevent undefined variable errors
const wrappedContent = `
(function() {
try {
${script.textContent}
} catch (error) {
console.warn("[Formbricks] Custom script error:", error);
}
})();
`;
newScript.textContent = wrappedContent;
}
// Add error handler for external scripts
newScript.onerror = (error) => {
console.warn("[Formbricks] Custom script failed to load:", error);
};
document.head.appendChild(newScript);
});

View File

@@ -43,6 +43,17 @@ vi.mock("@/lib/common/utils", () => ({
handleHiddenFields: vi.fn(),
}));
const mockUpdateQueue = {
hasPendingWork: vi.fn().mockReturnValue(false),
waitForPendingWork: vi.fn().mockResolvedValue(true),
};
vi.mock("@/lib/user/update-queue", () => ({
UpdateQueue: {
getInstance: vi.fn(() => mockUpdateQueue),
},
}));
describe("widget-file", () => {
let getInstanceConfigMock: MockInstance<() => Config>;
let getInstanceLoggerMock: MockInstance<() => Logger>;
@@ -249,4 +260,265 @@ describe("widget-file", () => {
widget.removeWidgetContainer();
expect(document.getElementById("formbricks-container")).toBeFalsy();
});
test("renderWidget waits for pending identification before rendering", async () => {
mockUpdateQueue.hasPendingWork.mockReturnValue(true);
mockUpdateQueue.waitForPendingWork.mockResolvedValue(true);
const mockConfigValue = {
get: vi.fn().mockReturnValue({
appUrl: "https://fake.app",
environmentId: "env_123",
environment: {
data: {
project: {
clickOutsideClose: true,
overlay: "none",
placement: "bottomRight",
inAppSurveyBranding: true,
},
},
},
user: {
data: {
userId: "user_abc",
contactId: "contact_abc",
displays: [],
responses: [],
lastDisplayAt: null,
language: "en",
},
},
}),
update: vi.fn(),
};
getInstanceConfigMock.mockReturnValue(mockConfigValue as unknown as Config);
widget.setIsSurveyRunning(false);
// @ts-expect-error -- mock window.formbricksSurveys
window.formbricksSurveys = {
renderSurvey: vi.fn(),
};
vi.useFakeTimers();
await widget.renderWidget({
...mockSurvey,
delay: 0,
} as unknown as TEnvironmentStateSurvey);
expect(mockUpdateQueue.hasPendingWork).toHaveBeenCalled();
expect(mockUpdateQueue.waitForPendingWork).toHaveBeenCalled();
vi.advanceTimersByTime(0);
expect(window.formbricksSurveys.renderSurvey).toHaveBeenCalledWith(
expect.objectContaining({
contactId: "contact_abc",
})
);
vi.useRealTimers();
});
test("renderWidget does not wait when no identification is pending", async () => {
mockUpdateQueue.hasPendingWork.mockReturnValue(false);
const mockConfigValue = {
get: vi.fn().mockReturnValue({
appUrl: "https://fake.app",
environmentId: "env_123",
environment: {
data: {
project: {
clickOutsideClose: true,
overlay: "none",
placement: "bottomRight",
inAppSurveyBranding: true,
},
},
},
user: {
data: {
userId: "user_abc",
contactId: "contact_abc",
displays: [],
responses: [],
lastDisplayAt: null,
language: "en",
},
},
}),
update: vi.fn(),
};
getInstanceConfigMock.mockReturnValue(mockConfigValue as unknown as Config);
widget.setIsSurveyRunning(false);
// @ts-expect-error -- mock window.formbricksSurveys
window.formbricksSurveys = {
renderSurvey: vi.fn(),
};
vi.useFakeTimers();
await widget.renderWidget({
...mockSurvey,
delay: 0,
} as unknown as TEnvironmentStateSurvey);
expect(mockUpdateQueue.hasPendingWork).toHaveBeenCalled();
expect(mockUpdateQueue.waitForPendingWork).not.toHaveBeenCalled();
vi.advanceTimersByTime(0);
expect(window.formbricksSurveys.renderSurvey).toHaveBeenCalled();
vi.useRealTimers();
});
test("renderWidget reads contactId after identification wait completes", async () => {
let callCount = 0;
const mockConfigValue = {
get: vi.fn().mockImplementation(() => {
callCount++;
return {
appUrl: "https://fake.app",
environmentId: "env_123",
environment: {
data: {
project: {
clickOutsideClose: true,
overlay: "none",
placement: "bottomRight",
inAppSurveyBranding: true,
},
},
},
user: {
data: {
// Simulate contactId becoming available after identification
userId: "user_abc",
contactId: callCount > 2 ? "contact_after_identification" : undefined,
displays: [],
responses: [],
lastDisplayAt: null,
language: "en",
},
},
};
}),
update: vi.fn(),
};
getInstanceConfigMock.mockReturnValue(mockConfigValue as unknown as Config);
mockUpdateQueue.hasPendingWork.mockReturnValue(true);
mockUpdateQueue.waitForPendingWork.mockResolvedValue(true);
widget.setIsSurveyRunning(false);
// @ts-expect-error -- mock window.formbricksSurveys
window.formbricksSurveys = {
renderSurvey: vi.fn(),
};
vi.useFakeTimers();
await widget.renderWidget({
...mockSurvey,
delay: 0,
} as unknown as TEnvironmentStateSurvey);
vi.advanceTimersByTime(0);
// The contactId passed to renderSurvey should be read after the wait
expect(window.formbricksSurveys.renderSurvey).toHaveBeenCalledWith(
expect.objectContaining({
contactId: "contact_after_identification",
})
);
vi.useRealTimers();
});
test("renderWidget skips survey when identification fails and survey has segment filters", async () => {
mockUpdateQueue.hasPendingWork.mockReturnValue(true);
mockUpdateQueue.waitForPendingWork.mockResolvedValue(false);
widget.setIsSurveyRunning(false);
// @ts-expect-error -- mock window.formbricksSurveys
window.formbricksSurveys = {
renderSurvey: vi.fn(),
};
await widget.renderWidget({
...mockSurvey,
delay: 0,
segment: { id: "seg_1", filters: [{ type: "attribute", value: "plan" }] },
} as unknown as TEnvironmentStateSurvey);
expect(mockUpdateQueue.waitForPendingWork).toHaveBeenCalled();
expect(mockLogger.debug).toHaveBeenCalledWith(
"User identification failed. Skipping survey with segment filters."
);
expect(window.formbricksSurveys.renderSurvey).not.toHaveBeenCalled();
});
test("renderWidget proceeds when identification fails but survey has no segment filters", async () => {
mockUpdateQueue.hasPendingWork.mockReturnValue(true);
mockUpdateQueue.waitForPendingWork.mockResolvedValue(false);
const mockConfigValue = {
get: vi.fn().mockReturnValue({
appUrl: "https://fake.app",
environmentId: "env_123",
environment: {
data: {
project: {
clickOutsideClose: true,
overlay: "none",
placement: "bottomRight",
inAppSurveyBranding: true,
},
},
},
user: {
data: {
userId: null,
contactId: null,
displays: [],
responses: [],
lastDisplayAt: null,
language: "en",
},
},
}),
update: vi.fn(),
};
getInstanceConfigMock.mockReturnValue(mockConfigValue as unknown as Config);
widget.setIsSurveyRunning(false);
// @ts-expect-error -- mock window.formbricksSurveys
window.formbricksSurveys = {
renderSurvey: vi.fn(),
};
vi.useFakeTimers();
await widget.renderWidget({
...mockSurvey,
delay: 0,
segment: undefined,
} as unknown as TEnvironmentStateSurvey);
expect(mockLogger.debug).toHaveBeenCalledWith(
"User identification failed but survey has no segment filters. Proceeding."
);
vi.advanceTimersByTime(0);
expect(window.formbricksSurveys.renderSurvey).toHaveBeenCalled();
vi.useRealTimers();
});
});

View File

@@ -11,6 +11,7 @@ import {
handleHiddenFields,
shouldDisplayBasedOnPercentage,
} from "@/lib/common/utils";
import { UpdateQueue } from "@/lib/user/update-queue";
import { type TEnvironmentStateSurvey, type TUserState } from "@/types/config";
import { type TTrackProperties } from "@/types/survey";
@@ -60,6 +61,24 @@ export const renderWidget = async (
setIsSurveyRunning(true);
// Wait for pending user identification to complete before rendering
const updateQueue = UpdateQueue.getInstance();
if (updateQueue.hasPendingWork()) {
logger.debug("Waiting for pending user identification before rendering survey");
const identificationSucceeded = await updateQueue.waitForPendingWork();
if (!identificationSucceeded) {
const hasSegmentFilters = Array.isArray(survey.segment?.filters) && survey.segment.filters.length > 0;
if (hasSegmentFilters) {
logger.debug("User identification failed. Skipping survey with segment filters.");
setIsSurveyRunning(false);
return;
}
logger.debug("User identification failed but survey has no segment filters. Proceeding.");
}
}
if (survey.delay) {
logger.debug(`Delaying survey "${survey.name}" by ${survey.delay.toString()} seconds.`);
}

View File

@@ -169,4 +169,104 @@ describe("UpdateQueue", () => {
"Formbricks can't set attributes without a userId! Please set a userId first with the setUserId function"
);
});
test("hasPendingWork returns false when no updates and no flush in flight", () => {
expect(updateQueue.hasPendingWork()).toBe(false);
});
test("hasPendingWork returns true when updates are queued", () => {
updateQueue.updateUserId(mockUserId1);
expect(updateQueue.hasPendingWork()).toBe(true);
});
test("hasPendingWork returns true while processUpdates flush is in flight", () => {
(sendUpdates as Mock).mockReturnValue({
ok: true,
data: { hasWarnings: false },
});
updateQueue.updateUserId(mockUserId1);
// Start processing but don't await — the debounce means the flush is in-flight
void updateQueue.processUpdates();
expect(updateQueue.hasPendingWork()).toBe(true);
});
test("waitForPendingWork returns true immediately when no pending work", async () => {
const result = await updateQueue.waitForPendingWork();
expect(result).toBe(true);
});
test("waitForPendingWork returns true when processUpdates succeeds", async () => {
(sendUpdates as Mock).mockReturnValue({
ok: true,
data: { hasWarnings: false },
});
updateQueue.updateUserId(mockUserId1);
void updateQueue.processUpdates();
const result = await updateQueue.waitForPendingWork();
expect(result).toBe(true);
expect(updateQueue.hasPendingWork()).toBe(false);
expect(sendUpdates).toHaveBeenCalled();
});
test("waitForPendingWork returns false when processUpdates rejects", async () => {
loggerMock.mockReturnValue(mockLogger as unknown as Logger);
(sendUpdates as Mock).mockRejectedValue(new Error("network error"));
updateQueue.updateUserId(mockUserId1);
// eslint-disable-next-line @typescript-eslint/no-empty-function -- intentionally swallowing rejection to avoid unhandled promise
const processPromise = updateQueue.processUpdates().catch(() => {});
const result = await updateQueue.waitForPendingWork();
expect(result).toBe(false);
await processPromise;
});
test("waitForPendingWork returns false when flush hangs past timeout", async () => {
vi.useFakeTimers();
// sendUpdates returns a promise that never resolves, simulating a network hang
// eslint-disable-next-line @typescript-eslint/no-empty-function -- intentionally never-resolving promise
(sendUpdates as Mock).mockReturnValue(new Promise(() => {}));
updateQueue.updateUserId(mockUserId1);
void updateQueue.processUpdates();
const resultPromise = updateQueue.waitForPendingWork();
// Advance past the debounce delay (500ms) so the handler fires and hangs on sendUpdates
await vi.advanceTimersByTimeAsync(500);
// Advance past the pending work timeout (5000ms)
await vi.advanceTimersByTimeAsync(5000);
const result = await resultPromise;
expect(result).toBe(false);
vi.useRealTimers();
});
test("processUpdates reuses pending flush instead of creating orphaned promises", async () => {
(sendUpdates as Mock).mockReturnValue({
ok: true,
data: { hasWarnings: false },
});
updateQueue.updateUserId(mockUserId1);
// First call creates the flush promise
const firstPromise = updateQueue.processUpdates();
// Second call while first is still pending should not create a new flush
updateQueue.updateAttributes({ name: mockAttributes.name });
const secondPromise = updateQueue.processUpdates();
// Both promises should resolve (second is not orphaned)
await Promise.all([firstPromise, secondPromise]);
expect(updateQueue.hasPendingWork()).toBe(false);
});
});

View File

@@ -8,7 +8,9 @@ export class UpdateQueue {
private static instance: UpdateQueue | null = null;
private updates: TUpdates | null = null;
private debounceTimeout: NodeJS.Timeout | null = null;
private pendingFlush: Promise<void> | null = null;
private readonly DEBOUNCE_DELAY = 500;
private readonly PENDING_WORK_TIMEOUT = 5000;
private constructor() {}
@@ -63,17 +65,45 @@ export class UpdateQueue {
return !this.updates;
}
public hasPendingWork(): boolean {
return this.updates !== null || this.pendingFlush !== null;
}
public async waitForPendingWork(): Promise<boolean> {
if (!this.hasPendingWork()) return true;
const flush = this.pendingFlush ?? this.processUpdates();
try {
const succeeded = await Promise.race([
flush.then(() => true as const),
new Promise<false>((resolve) => {
setTimeout(() => {
resolve(false);
}, this.PENDING_WORK_TIMEOUT);
}),
]);
return succeeded;
} catch {
return false;
}
}
public async processUpdates(): Promise<void> {
const logger = Logger.getInstance();
if (!this.updates) {
return;
}
// If a flush is already in flight, reuse it instead of creating a new promise
if (this.pendingFlush) {
return this.pendingFlush;
}
if (this.debounceTimeout) {
clearTimeout(this.debounceTimeout);
}
return new Promise((resolve, reject) => {
const flushPromise = new Promise<void>((resolve, reject) => {
const handler = async (): Promise<void> => {
try {
let currentUpdates = { ...this.updates };
@@ -147,8 +177,10 @@ export class UpdateQueue {
}
this.clearUpdates();
this.pendingFlush = null;
resolve();
} catch (error: unknown) {
this.pendingFlush = null;
logger.error(
`Failed to process updates: ${error instanceof Error ? error.message : "Unknown error"}`
);
@@ -158,5 +190,8 @@ export class UpdateQueue {
this.debounceTimeout = setTimeout(() => void handler(), this.DEBOUNCE_DELAY);
});
this.pendingFlush = flushPromise;
return flushPromise;
}
}

View File

@@ -1,4 +1,4 @@
import DOMPurify from "isomorphic-dompurify";
import { sanitize } from "isomorphic-dompurify";
import * as React from "react";
import { cn, stripInlineStyles } from "@/lib/utils";
@@ -39,7 +39,7 @@ function Label({
const isHtml = childrenString ? isValidHTML(strippedContent) : false;
const safeHtml =
isHtml && strippedContent
? DOMPurify.sanitize(strippedContent, {
? sanitize(strippedContent, {
ADD_ATTR: ["target"],
FORBID_ATTR: ["style"],
})

View File

@@ -1,5 +1,5 @@
import { type ClassValue, clsx } from "clsx";
import DOMPurify from "isomorphic-dompurify";
import { sanitize } from "isomorphic-dompurify";
import { extendTailwindMerge } from "tailwind-merge";
const twMerge = extendTailwindMerge({
@@ -27,14 +27,16 @@ export function cn(...inputs: ClassValue[]): string {
export const stripInlineStyles = (html: string): string => {
if (!html) return html;
// Use DOMPurify to safely remove style attributes
// This is more secure than regex-based approaches and handles edge cases properly
return DOMPurify.sanitize(html, {
// Pre-strip style attributes from the raw string BEFORE DOMPurify parses it.
// DOMPurify internally uses innerHTML to parse HTML, which triggers CSP
// `style-src` violations at parse time — before FORBID_ATTR can strip them.
// The regex is O(n) safe: [^"]* and [^']* are negated classes bounded by
// fixed quote delimiters, so no backtracking can occur.
const preStripped = html.replaceAll(/ style="[^"]*"| style='[^']*'/gi, "");
return sanitize(preStripped, {
FORBID_ATTR: ["style"],
// Preserve the target attribute (e.g. target="_blank" on links) which is not
// in DOMPurify's default allow-list but is explicitly required downstream.
ADD_ATTR: ["target"],
// Keep other attributes and tags as-is, only remove style attributes
KEEP_CONTENT: true,
});
};

View File

@@ -4,6 +4,7 @@
"baseUrl": ".",
"isolatedModules": true,
"jsx": "react-jsx",
"lib": ["DOM", "DOM.Iterable", "ES2020", "ES2021.String"],
"noEmit": true,
"paths": {
"@/*": ["./src/*"]

View File

@@ -10,14 +10,16 @@ import DOMPurify from "isomorphic-dompurify";
export const stripInlineStyles = (html: string): string => {
if (!html) return html;
// Use DOMPurify to safely remove style attributes
// This is more secure than regex-based approaches and handles edge cases properly
return DOMPurify.sanitize(html, {
// Pre-strip style attributes from the raw string BEFORE DOMPurify parses it.
// DOMPurify internally uses innerHTML to parse HTML, which triggers CSP
// `style-src` violations at parse time — before FORBID_ATTR can strip them.
// The regex is O(n) safe: [^"]* and [^']* are negated classes bounded by
// fixed quote delimiters, so no backtracking can occur.
const preStripped = html.replaceAll(/ style="[^"]*"| style='[^']*'/gi, "");
return DOMPurify.sanitize(preStripped, {
FORBID_ATTR: ["style"],
// Preserve the target attribute (e.g. target="_blank" on links) which is not
// in DOMPurify's default allow-list but is explicitly required downstream.
ADD_ATTR: ["target"],
// Keep other attributes and tags as-is, only remove style attributes
KEEP_CONTENT: true,
});
};