fix(campaigns): wizard sent an empty payload; harden preview URL extraction

Critical: only the active wizard step is mounted, so on the Review step no
Form.Items are registered and form.validateFields() resolved to {}. Every
field was dropped and every submit failed validation server-side. Read the
preserved store with getFieldsValue(true) instead, and validate each step's
fields when leaving it so the required rules actually run.

Security: extractSrc matched src=" anywhere in the body with no scheme or
host check, so a data-src attribute or an earlier <img> could hijack the
match, and javascript:/data:/protocol-relative URLs passed through. Combined
with sandbox="allow-same-origin" a javascript: URL would have inherited the
app origin. It now anchors on the iframe tag and accepts only https URLs on
facebook.com/fbcdn.net, failing closed otherwise. Verified against 14 inputs
including both hijack shapes.

Also: memoise the creative object so PreviewStep does not refetch on every
parent render, type the form values, guard the preview effect against stale
responses, reset the wizard on close, and show an explicit message when no
placement renders.

Moving extractSrc to its own module removes the react-refresh lint
suppression rather than hiding it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
AFFAANh
2026-08-02 18:48:20 +05:30
co-authored by Claude Opus 5
parent 07e4c8997e
commit 39119163c1
3 changed files with 178 additions and 55 deletions
+73 -14
View File
@@ -1,4 +1,4 @@
import { useState } from "react";
import { useMemo, useState } from "react";
import {
Button,
Descriptions,
@@ -32,33 +32,92 @@ const OBJECTIVES = [
"OUTCOME_APP_PROMOTION",
];
interface WizardValues {
name?: string;
objective?: string;
daily_budget?: number;
age_min?: number;
age_max?: number;
countries?: string[];
primary_text?: string;
headline?: string;
description?: string;
link?: string;
auto_pause_if_spend_reaches?: number;
max_cost_per_lead?: number;
max_cost_per_click?: number;
require_approval_for_budget_increase?: boolean;
stop_loss_enabled?: boolean;
}
// Stable identity so `values` does not churn on every render while useWatch
// is still undefined.
const EMPTY_VALUES: WizardValues = {};
// Fields validated when leaving each step. The Preview step (index 3) has no
// inputs of its own.
const STEP_FIELDS: string[][] = [
["name", "objective", "daily_budget"],
["age_min", "age_max", "countries"],
["primary_text", "headline", "description", "link"],
[],
["auto_pause_if_spend_reaches", "max_cost_per_lead", "max_cost_per_click"],
];
export default function CampaignWizard({
open,
adAccount,
onClose,
onCreated,
}: Props) {
const [form] = Form.useForm();
const [form] = Form.useForm<WizardValues>();
const [step, setStep] = useState(0);
const [saving, setSaving] = useState(false);
const values = Form.useWatch([], form) ?? {};
const values: WizardValues = Form.useWatch([], form) ?? EMPTY_VALUES;
const creative = {
object_story_spec: {
link_data: {
message: values.primary_text ?? "",
name: values.headline ?? "",
description: values.description ?? "",
link: values.link ?? "https://example.com",
// Memoised so PreviewStep's effect does not refire on every parent render.
const creative = useMemo(
() => ({
object_story_spec: {
link_data: {
message: values.primary_text ?? "",
name: values.headline ?? "",
description: values.description ?? "",
link: values.link ?? "https://example.com",
},
},
},
}),
[values.primary_text, values.headline, values.description, values.link],
);
const closeAndReset = () => {
form.resetFields();
setStep(0);
onClose();
};
const next = async () => {
try {
await form.validateFields(STEP_FIELDS[step]);
setStep(step + 1);
} catch {
// antd renders the field errors inline.
}
};
const submit = async () => {
setSaving(true);
try {
const raw = await form.validateFields();
// Only the active step is mounted, so validateFields() would resolve to
// {} here and silently drop every value. getFieldsValue(true) returns
// the whole preserved store. Required fields are enforced per step by
// `next`, so by the time Review is reachable they are already valid.
const raw = form.getFieldsValue(true) as WizardValues;
if (!raw.name) {
message.error("Campaign name is required. Go back to the first step.");
return;
}
const payload: CampaignPayload = {
name: raw.name,
objective: raw.objective,
@@ -228,7 +287,7 @@ export default function CampaignWizard({
<Drawer
title="New campaign"
open={open}
onClose={onClose}
onClose={closeAndReset}
width={720}
footer={
<Space>
@@ -236,7 +295,7 @@ export default function CampaignWizard({
Back
</Button>
{step < steps.length - 1 ? (
<Button type="primary" onClick={() => setStep(step + 1)}>
<Button type="primary" onClick={() => void next()}>
Next
</Button>
) : (
+73 -41
View File
@@ -1,6 +1,7 @@
import { useCallback, useEffect, useState } from "react";
import { useEffect, useMemo, useState } from "react";
import { Alert, Button, Spin, Tabs } from "antd";
import api from "../../api";
import { extractSrc } from "./extractSrc";
const FORMATS: { key: string; label: string }[] = [
{ key: "DESKTOP_FEED_STANDARD", label: "Desktop feed" },
@@ -11,55 +12,69 @@ const FORMATS: { key: string; label: string }[] = [
{ key: "RIGHT_COLUMN_STANDARD", label: "Right column" },
];
const AD_FORMATS = FORMATS.map((f) => f.key);
interface Props {
adAccountId: string;
creative: Record<string, unknown>;
}
/**
* Meta returns a full `<iframe src="...">` string per placement. Render a
* real sandboxed iframe from the extracted URL rather than injecting Meta's
* markup into the DOM.
*/
// extractSrc is exported alongside the component intentionally so its
// regex-based URL extraction (the XSS-safe alternative to
// dangerouslySetInnerHTML) is directly testable in isolation.
// eslint-disable-next-line react-refresh/only-export-components
export function extractSrc(body: string | undefined): string | null {
if (!body) return null;
const match = body.match(/src="([^"]+)"/);
if (!match) return null;
return match[1].replace(/&amp;/g, "&");
}
export default function PreviewStep({ adAccountId, creative }: Props) {
const [previews, setPreviews] = useState<Record<string, string>>({});
const [loading, setLoading] = useState(false);
const [error, setError] = useState<string | null>(null);
const load = useCallback(async () => {
if (!adAccountId) return;
setLoading(true);
setError(null);
try {
const result = await api.previewCampaign({
ad_account_id: adAccountId,
creative,
ad_formats: FORMATS.map((f) => f.key),
});
setPreviews(result.previews);
} catch (err) {
setError(err instanceof Error ? err.message : "Preview failed");
} finally {
setLoading(false);
}
}, [adAccountId, creative]);
const [reloadKey, setReloadKey] = useState(0);
useEffect(() => {
void load();
}, [load]);
if (!adAccountId) return;
// Guards against a stale response overwriting a newer one, and against
// setting state after unmount.
let ignore = false;
setLoading(true);
setError(null);
if (loading) return <Spin tip="Rendering previews" />;
api
.previewCampaign({
ad_account_id: adAccountId,
creative,
ad_formats: AD_FORMATS,
})
.then((result) => {
if (!ignore) setPreviews(result.previews);
})
.catch((err: unknown) => {
if (!ignore) {
setError(err instanceof Error ? err.message : "Preview failed");
}
})
.finally(() => {
if (!ignore) setLoading(false);
});
return () => {
ignore = true;
};
}, [adAccountId, creative, reloadKey]);
const retry = () => setReloadKey((key) => key + 1);
// Resolve each placement once per render rather than calling extractSrc
// twice per tab.
const renderable = useMemo(
() =>
FORMATS.map((f) => ({ ...f, src: extractSrc(previews[f.key]) })).filter(
(f): f is { key: string; label: string; src: string } => f.src !== null,
),
[previews],
);
if (loading) {
return (
<Spin tip="Rendering previews">
<div style={{ height: 620 }} />
</Spin>
);
}
if (error) {
return (
@@ -68,21 +83,38 @@ export default function PreviewStep({ adAccountId, creative }: Props) {
showIcon
message="Could not render preview"
description={error}
action={<Button onClick={() => void load()}>Retry</Button>}
action={<Button onClick={retry}>Retry</Button>}
/>
);
}
if (renderable.length === 0) {
return (
<Alert
type="info"
showIcon
message="No preview available"
description={
"Meta returned no renderable preview for this creative. Check the " +
"primary text, headline and destination link, then try again."
}
action={<Button onClick={retry}>Retry</Button>}
/>
);
}
return (
<Tabs
items={FORMATS.filter((f) => extractSrc(previews[f.key])).map((f) => ({
items={renderable.map((f) => ({
key: f.key,
label: f.label,
children: (
<iframe
title={f.label}
src={extractSrc(previews[f.key]) as string}
src={f.src}
sandbox="allow-scripts allow-same-origin"
referrerPolicy="no-referrer"
loading="lazy"
style={{ width: "100%", height: 620, border: 0 }}
/>
),
+32
View File
@@ -0,0 +1,32 @@
/**
* Meta's preview endpoint returns a full `<iframe src="...">` HTML string per
* placement. We never inject that markup into the DOM; we extract the URL and
* render a real sandboxed iframe from it.
*
* The extraction fails closed. Anything that is not an https URL on a Meta
* host returns null, and the caller filters that placement out. This matters
* because the iframe is rendered with `allow-same-origin`: a `javascript:` URL
* would otherwise inherit the application's own origin.
*/
const META_HOST = /(^|\.)(facebook\.com|fbcdn\.net)$/;
export function extractSrc(body: string | undefined): string | null {
if (!body) return null;
// Anchor on the iframe tag so an earlier `data-src` or `<img src>` in the
// body cannot hijack the match.
const match = body.match(/<iframe\b[^>]*?\ssrc="([^"]+)"/i);
if (!match) return null;
const raw = match[1].replace(/&amp;/g, "&");
try {
const url = new URL(raw);
if (url.protocol !== "https:") return null;
if (!META_HOST.test(url.hostname)) return null;
return url.toString();
} catch {
// Relative or protocol-relative URLs throw here, which is the desired
// fail-closed outcome.
return null;
}
}