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
27 changes: 27 additions & 0 deletions packages/targets/chat-whatsapp/src/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,23 @@ describe('WhatsApp Business Cloud API target', () => {
})).rejects.toThrow('placeholders must be contiguous from {{1}}');
});

it('rejects unsafe Graph API identifiers and endpoints', async () => {
await expect(adapter.build(fakeBuildContext() as any, {
...baseConfig,
phoneNumberId: '123/456',
})).rejects.toThrow('phoneNumberId must be a URL-safe Graph API id');

await expect(adapter.build(fakeBuildContext() as any, {
...baseConfig,
graphApiVersion: 'beta',
})).rejects.toThrow('graphApiVersion must look like v25.0');

await expect(adapter.build(fakeBuildContext() as any, {
...baseConfig,
graphApiBaseUrl: 'http://graph.facebook.com',
})).rejects.toThrow('graphApiBaseUrl must use HTTPS');
});

it('keeps dry-run shipping side-effect free', async () => {
await expect(adapter.ship(fakeShipContext({ dryRun: true }) as any, baseConfig))
.resolves.toMatchObject({
Expand All @@ -89,6 +106,16 @@ describe('WhatsApp Business Cloud API target', () => {
});
});

it('rejects blank secret key names before Graph API calls', async () => {
await expect(adapter.ship(fakeShipContext({
dryRun: false,
secret: makeVault({ WHATSAPP_BUSINESS_TOKEN: 'mock-token' }),
}) as any, {
...baseConfig,
tokenKey: ' ',
})).rejects.toThrow('chat-whatsapp requires tokenKey');
});
Comment on lines +109 to +117

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Missing test for blank verifyTokenKey

This PR changed the verifyTokenKey path to wrap it with requireText before passing to requireSecret, but no test exercises a blank verifyTokenKey value. Since ' ' is truthy, the if (config.verifyTokenKey) guard lets it through and requireText is the only safeguard — a regression there would be silent. A test mirroring the tokenKey blank case (with a non-dry-run context and verifyTokenKey: ' ') would pin this behaviour.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!


it('submits templates and subscribes WABA webhooks through Graph API', async () => {
const fetchMock = vi.fn()
.mockResolvedValueOnce({ ok: true, status: 200, json: async () => ({ id: 'template_1' }) })
Expand Down
50 changes: 39 additions & 11 deletions packages/targets/chat-whatsapp/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ interface TemplateManifestEntry extends WhatsAppTemplateBody {

const TEMPLATE_NAME_RE = /^[a-z0-9_]+$/;
const LANGUAGE_RE = /^[a-z]{2,3}(_[A-Z]{2})?$/;
const GRAPH_ID_RE = /^[A-Za-z0-9_-]+$/;
const GRAPH_API_VERSION_RE = /^v\d+(?:\.\d+)?$/;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 The regex accepts a version without a minor segment (e.g. v25), but the error message tells the user it "must look like v25.0". If the intent is to require the MAJOR.MINOR form that Meta Graph API versions use, the ? on the minor group should be dropped. If bare integers like v25 are intentionally allowed, the error message should reflect that.

Suggested change
const GRAPH_API_VERSION_RE = /^v\d+(?:\.\d+)?$/;
const GRAPH_API_VERSION_RE = /^v\d+\.\d+$/;

const DEFAULT_GRAPH_API_VERSION = 'v25.0';
const DEFAULT_GRAPH_API_BASE_URL = 'https://graph.facebook.com';

Expand All @@ -66,22 +68,48 @@ function requireText(value: string | undefined, field: string): string {
return text;
}

function optionalText(value: string | undefined, field: string): string | undefined {
return value === undefined ? undefined : requireText(value, field);
}

function requireGraphId(value: string | undefined, field: string): string {
const id = requireText(value, field);
if (!GRAPH_ID_RE.test(id)) {
throw new Error(`chat-whatsapp ${field} must be a URL-safe Graph API id`);
}
return id;
}

function graphApiVersion(config: Config): string {
const version = config.graphApiVersion?.trim() || DEFAULT_GRAPH_API_VERSION;
return version.startsWith('v') ? version : `v${version}`;
const rawVersion = optionalText(config.graphApiVersion, 'graphApiVersion') ?? DEFAULT_GRAPH_API_VERSION;
const version = rawVersion.startsWith('v') ? rawVersion : `v${rawVersion}`;
if (!GRAPH_API_VERSION_RE.test(version)) {
throw new Error('chat-whatsapp graphApiVersion must look like v25.0');
}
return version;
}

function graphApiBaseUrl(config: Config): string {
return (config.graphApiBaseUrl?.trim() || DEFAULT_GRAPH_API_BASE_URL).replace(/\/+$/, '');
const baseUrl = optionalText(config.graphApiBaseUrl, 'graphApiBaseUrl') ?? DEFAULT_GRAPH_API_BASE_URL;
let parsed: URL;
try {
parsed = new URL(baseUrl);
} catch {
throw new Error('chat-whatsapp graphApiBaseUrl must be a valid HTTPS URL');
}
if (parsed.protocol !== 'https:') throw new Error('chat-whatsapp graphApiBaseUrl must use HTTPS');
return baseUrl.replace(/\/+$/, '');
}

function graphUrl(config: Config, path: string): string {
return `${graphApiBaseUrl(config)}/${graphApiVersion(config)}/${path}`;
}

function validateBaseConfig(config: Config): void {
requireText(config.phoneNumberId, 'phoneNumberId');
requireText(config.wabaId, 'wabaId');
requireGraphId(config.phoneNumberId, 'phoneNumberId');
requireGraphId(config.wabaId, 'wabaId');
graphApiVersion(config);
graphApiBaseUrl(config);
const webhookUrl = requireText(config.webhookUrl, 'webhookUrl');
let parsed: URL;
try {
Expand Down Expand Up @@ -140,8 +168,8 @@ function validateTemplate(template: NonNullable<Config['templates']>[number]): T

function templateManifest(config: Config, version: string) {
validateBaseConfig(config);
const phoneNumberId = requireText(config.phoneNumberId, 'phoneNumberId');
const wabaId = requireText(config.wabaId, 'wabaId');
const phoneNumberId = requireGraphId(config.phoneNumberId, 'phoneNumberId');
const wabaId = requireGraphId(config.wabaId, 'wabaId');
const webhookUrl = requireText(config.webhookUrl, 'webhookUrl');
const templates = (config.templates ?? []).map(validateTemplate);
return {
Expand Down Expand Up @@ -234,7 +262,7 @@ export default defineTarget<Config>({

if (typeof fetch !== 'function') throw new Error('global fetch is not available for WhatsApp Graph API calls');

const tokenKey = config.tokenKey ?? 'WHATSAPP_BUSINESS_TOKEN';
const tokenKey = optionalText(config.tokenKey, 'tokenKey') ?? 'WHATSAPP_BUSINESS_TOKEN';
const token = requireSecret(ctx, tokenKey);
const submittedTemplates = [];
for (const template of manifest.templates) {
Expand All @@ -247,13 +275,13 @@ export default defineTarget<Config>({
ctx.log('whatsapp · subscribe app to WABA webhooks');
subscription = await callGraph(manifest.endpoints.subscribedApps, token, {
override_callback_uri: manifest.webhookUrl,
...(config.verifyTokenKey ? { verify_token: requireSecret(ctx, config.verifyTokenKey) } : {}),
...(config.verifyTokenKey ? { verify_token: requireSecret(ctx, requireText(config.verifyTokenKey, 'verifyTokenKey')) } : {}),
});
}

return {
id: `${config.phoneNumberId}@${ctx.version}`,
url: `https://business.facebook.com/wa/manage/phone-numbers/?waba_id=${config.wabaId}`,
id: `${manifest.phoneNumberId}@${ctx.version}`,
url: `https://business.facebook.com/wa/manage/phone-numbers/?waba_id=${manifest.wabaId}`,
meta: {
templates: submittedTemplates,
subscription: subscription ? { success: subscription.success ?? true } : undefined,
Expand Down
Loading