From c0e7d8cf9dd14784c0b6860639e37f7a194f2b7c Mon Sep 17 00:00:00 2001 From: Harsh Joshi Date: Wed, 17 Jun 2026 15:15:57 +0530 Subject: [PATCH 01/10] [ms-bing-ads-audiences] Add temporary debug logging of API responses Non-bulk destinations do not persist the responses received from the destination, which makes debugging the MS Bing Ads Audiences sync difficult. Add temporary debug logging, gated behind the 'actions-ms-bing-ads-audiences-debug-logging' feature flag, that logs the sent request payloads and the raw Bing Ads API responses (including error bodies) to the centralized logging pipeline. Mirrors the existing 'salesforce-advanced-logging' pattern. The error path clones the response before reading it so handleHttpError can still consume the body. To be removed once debugging is complete. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../ms-bing-ads-audiences/metadata.json | 342 ++++++++++++++++++ .../syncAudiences/index.ts | 67 +++- 2 files changed, 403 insertions(+), 6 deletions(-) create mode 100644 packages/destination-actions/src/destinations/ms-bing-ads-audiences/metadata.json diff --git a/packages/destination-actions/src/destinations/ms-bing-ads-audiences/metadata.json b/packages/destination-actions/src/destinations/ms-bing-ads-audiences/metadata.json new file mode 100644 index 00000000000..d53bda62dd3 --- /dev/null +++ b/packages/destination-actions/src/destinations/ms-bing-ads-audiences/metadata.json @@ -0,0 +1,342 @@ +{ + "slug": "actions-ms-bing-ads-audiences", + "name": "Ms Bing Ads Audiences", + "mode": "cloud", + "authentication": { + "scheme": "oauth2", + "fields": { + "customerAccountId": { + "label": "Customer Account ID", + "description": "The account ID of the Microsoft Advertising account you want to manage. You can find it in the URL when viewing the account in the Microsoft Ads User Interface. Not to be confused with Account Number.", + "type": "string", + "required": true, + "multiple": false, + "choices": null, + "default": null, + "depends_on": null + }, + "customerId": { + "label": "Customer ID", + "description": "The customer (parent) ID associated with your Microsoft Advertising account. You can find this in the URL when viewing your account in the Microsoft Ads User Interface.", + "type": "string", + "required": true, + "multiple": false, + "choices": null, + "default": null, + "depends_on": null + } + } + }, + "audienceConfig": { + "mode": { + "type": "synced", + "full_audience_sync": false + }, + "audienceFields": {}, + "supportsAudienceFunctions": true + }, + "actions": { + "syncAudiences": { + "title": "Sync Audiences", + "description": "Sync users to Microsoft Bing Ads Audiences", + "platform": "cloud", + "defaultSubscription": null, + "hidden": false, + "hasPerformBatch": true, + "syncMode": null, + "hooks": null, + "dynamicFields": null, + "fields": { + "audience_id": { + "label": "Audience ID", + "description": "The ID of the audience to which you want to add or remove users.", + "type": "string", + "required": true, + "multiple": false, + "allowNull": false, + "dynamic": false, + "default": { + "@path": "$.context.personas.external_audience_id" + }, + "choices": null, + "placeholder": null, + "properties": null, + "category": null, + "depends_on": null, + "readOnly": null, + "hidden": true, + "minimum": null, + "maximum": null, + "defaultObjectUI": null, + "disabledInputMethods": null, + "displayMode": null, + "format": null, + "additionalProperties": false + }, + "traits_or_props": { + "label": "[Hidden] Traits or Properties", + "description": "[Hidden] properties object from track() payloads. Note: identify calls are not handled and are disabled in the Partner Portal.", + "type": "object", + "required": true, + "multiple": false, + "allowNull": false, + "dynamic": false, + "default": { + "@path": "$.properties" + }, + "choices": null, + "placeholder": null, + "properties": null, + "category": null, + "depends_on": null, + "readOnly": null, + "hidden": true, + "minimum": null, + "maximum": null, + "defaultObjectUI": null, + "disabledInputMethods": null, + "displayMode": null, + "format": null, + "additionalProperties": false + }, + "audience_key": { + "label": "[Hidden] Audience Key", + "description": "[Hidden]: The Engage Audience Key / Slug.", + "type": "string", + "required": true, + "multiple": false, + "allowNull": false, + "dynamic": false, + "default": { + "@path": "$.context.personas.computation_key" + }, + "choices": null, + "placeholder": null, + "properties": null, + "category": null, + "depends_on": null, + "readOnly": null, + "hidden": true, + "minimum": null, + "maximum": null, + "defaultObjectUI": null, + "disabledInputMethods": null, + "displayMode": null, + "format": null, + "additionalProperties": false + }, + "identifier_type": { + "label": "Identifier Type", + "description": "The type of identifier you are using to sync users.", + "type": "string", + "required": true, + "multiple": false, + "allowNull": false, + "dynamic": false, + "default": "Email", + "choices": [ + { + "label": "Email", + "value": "Email" + }, + { + "label": "CRM ID", + "value": "CRM" + } + ], + "placeholder": null, + "properties": null, + "category": null, + "depends_on": null, + "readOnly": null, + "hidden": null, + "minimum": null, + "maximum": null, + "defaultObjectUI": null, + "disabledInputMethods": null, + "displayMode": null, + "format": null, + "additionalProperties": false + }, + "email": { + "label": "Email", + "description": "The email address of the user to add or remove from the audience.", + "type": "string", + "required": { + "conditions": [ + { + "fieldKey": "identifier_type", + "operator": "is", + "value": "Email" + } + ] + }, + "multiple": false, + "allowNull": false, + "dynamic": false, + "default": { + "@if": { + "exists": { + "@path": "$.context.traits.email" + }, + "then": { + "@path": "$.context.traits.email" + }, + "else": { + "@path": "$.properties.email" + } + } + }, + "choices": null, + "placeholder": null, + "properties": null, + "category": "hashedPII", + "depends_on": { + "conditions": [ + { + "fieldKey": "identifier_type", + "operator": "is", + "value": "Email" + } + ] + }, + "readOnly": null, + "hidden": null, + "minimum": null, + "maximum": null, + "defaultObjectUI": null, + "disabledInputMethods": null, + "displayMode": null, + "format": null, + "additionalProperties": false + }, + "crm_id": { + "label": "CRM ID", + "description": "The CRM ID of the user to add or remove from the audience.", + "type": "string", + "required": { + "conditions": [ + { + "fieldKey": "identifier_type", + "operator": "is", + "value": "CRM" + } + ] + }, + "multiple": false, + "allowNull": false, + "dynamic": false, + "default": { + "@path": "$.userId" + }, + "choices": null, + "placeholder": null, + "properties": null, + "category": null, + "depends_on": { + "conditions": [ + { + "fieldKey": "identifier_type", + "operator": "is", + "value": "CRM" + } + ] + }, + "readOnly": null, + "hidden": null, + "minimum": null, + "maximum": null, + "defaultObjectUI": null, + "disabledInputMethods": null, + "displayMode": null, + "format": null, + "additionalProperties": false + }, + "enable_batching": { + "label": "Enable Batching", + "description": "Enable batching of user syncs to optimize performance. When enabled, user syncs will be sent in batches based on the specified batch size.", + "type": "boolean", + "required": true, + "multiple": false, + "allowNull": false, + "dynamic": false, + "default": true, + "choices": null, + "placeholder": null, + "properties": null, + "category": null, + "depends_on": null, + "readOnly": true, + "hidden": null, + "minimum": null, + "maximum": null, + "defaultObjectUI": null, + "disabledInputMethods": null, + "displayMode": null, + "format": null, + "additionalProperties": false + }, + "batch_size": { + "label": "[Hidden] Batch Size", + "description": "[Hidden] The number of user syncs to include in each batch when batching is enabled. Must be between 1 and 1000.", + "type": "number", + "required": true, + "multiple": false, + "allowNull": false, + "dynamic": false, + "default": 1000, + "choices": null, + "placeholder": null, + "properties": null, + "category": null, + "depends_on": null, + "readOnly": null, + "hidden": true, + "minimum": 1, + "maximum": 1000, + "defaultObjectUI": null, + "disabledInputMethods": null, + "displayMode": null, + "format": null, + "additionalProperties": false + }, + "computation_class": { + "label": "Computation Class", + "description": "Hidden field: The computation class for the audience.", + "type": "string", + "required": true, + "multiple": false, + "allowNull": false, + "dynamic": false, + "default": { + "@path": "$.context.personas.computation_class" + }, + "choices": [ + { + "label": "audience", + "value": "audience" + }, + { + "label": "journey_step", + "value": "journey_step" + } + ], + "placeholder": null, + "properties": null, + "category": null, + "depends_on": null, + "readOnly": null, + "hidden": true, + "minimum": null, + "maximum": null, + "defaultObjectUI": null, + "disabledInputMethods": null, + "displayMode": null, + "format": null, + "additionalProperties": false + } + } + } + }, + "presets": [] +} diff --git a/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/index.ts b/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/index.ts index e93d5fe65d8..fb5da724b55 100644 --- a/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/index.ts +++ b/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/index.ts @@ -1,4 +1,4 @@ -import { ActionDefinition, RequestClient, HTTPError } from '@segment/actions-core' +import { ActionDefinition, RequestClient, HTTPError, Logger, ModifiedResponse } from '@segment/actions-core' import { MultiStatusResponse } from '@segment/actions-core' import type { Settings } from '../generated-types' import type { Payload } from './generated-types' @@ -36,15 +36,61 @@ const action: ActionDefinition = { batch_size, computation_class }, - perform: async (request, { payload }) => { - return await syncUser(request, [payload], false) + perform: async (request, { payload, features, logger }) => { + return await syncUser(request, [payload], false, isDebugLoggingEnabled(features), logger) }, - performBatch: async (request, { payload }) => { - return await syncUser(request, payload, true) + performBatch: async (request, { payload, features, logger }) => { + return await syncUser(request, payload, true, isDebugLoggingEnabled(features), logger) } } +// TEMPORARY DEBUG LOGGING — gated behind the 'actions-ms-bing-ads-audiences-debug-logging' +// feature flag. Enables logging of the raw request payloads and Bing Ads API responses +// (including error bodies) so they can be inspected in the centralized logging pipeline. +// Remove once debugging is complete. +const DEBUG_LOGGING_FLAG = 'actions-ms-bing-ads-audiences-debug-logging' + +const isDebugLoggingEnabled = (features: Record | undefined): boolean => + Boolean(features && features[DEBUG_LOGGING_FLAG]) + +// TEMPORARY: logs the request payload sent to Bing Ads and the raw response body. +const logBingAdsResponse = ( + logger: Logger | undefined, + debugLogging: boolean, + action: string, + audienceId: string, + sentPayload: SyncAudiencePayload, + response: ModifiedResponse +): void => { + if (!debugLogging || !logger) { + return + } + logger.info( + `[ms-bing-ads-audiences][DEBUG] ${action} audienceId=${audienceId} status=${response.status} ` + + `sent=${JSON.stringify(sentPayload)} response=${JSON.stringify(response.data)}` + ) +} + +// TEMPORARY: logs the error body returned by Bing Ads when an Apply call fails. +const logBingAdsError = async ( + logger: Logger | undefined, + debugLogging: boolean, + audienceId: string, + error: unknown +): Promise => { + if (!debugLogging || !logger) { + return + } + let body: string + try { + body = error instanceof HTTPError ? JSON.stringify(await error.response?.clone().json()) : String(error) + } catch { + body = error instanceof Error ? error.message : String(error) + } + logger.error(`[ms-bing-ads-audiences][DEBUG] Apply failed audienceId=${audienceId} error=${body}`) +} + /** * Synchronizes user audience data with Microsoft Bing Ads. * @@ -58,7 +104,13 @@ const action: ActionDefinition = { * @returns A promise that resolves to a `MultiStatusResponse` object summarizing the results. * @throws Will throw an error if a non-batch operation fails, or rethrows non-HTTP errors in batch mode. */ -const syncUser = async (request: RequestClient, payload: Payload[], isBatch: boolean) => { +const syncUser = async ( + request: RequestClient, + payload: Payload[], + isBatch: boolean, + debugLogging = false, + logger?: Logger +) => { const msResponse = new MultiStatusResponse() if (!Array.isArray(payload) || payload.length === 0) { @@ -87,13 +139,16 @@ const syncUser = async (request: RequestClient, payload: Payload[], isBatch: boo // Send data to Microsoft Bing Ads for both Add and Remove actions if they have entries if (addMap.size > 0) { const response = await sendDataToMicrosoftBingAds(request, addPayload) + logBingAdsResponse(logger, debugLogging, 'Add', audienceId, addPayload, response) handleMultistatusResponse(msResponse, response, addItems, addMap, payload, isBatch) } if (removeMap.size > 0) { const response = await sendDataToMicrosoftBingAds(request, removePayload) + logBingAdsResponse(logger, debugLogging, 'Remove', audienceId, removePayload, response) handleMultistatusResponse(msResponse, response, removeItems, removeMap, payload, isBatch) } } catch (error) { + await logBingAdsError(logger, debugLogging, audienceId, error) if (!isBatch) { throw error } From 9ac9829d4e32bf6345409f5037947587acf74d72 Mon Sep 17 00:00:00 2001 From: Harsh Joshi Date: Wed, 17 Jun 2026 15:38:24 +0530 Subject: [PATCH 02/10] [ms-bing-ads-audiences] Address Copilot review feedback - Stop logging CustomerListItems (hashed emails / CRM IDs). Log only identifierType + itemCount alongside the response body to avoid leaking customer-match identifiers into centralized logs. - Read error bodies as text (not assumed JSON) and size-cap all logged bodies to avoid oversized log entries. - Add unit tests covering flag-off (no logging), flag-on (logs metadata, no PII) and error logging not interfering with handleHttpError. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../syncAudiences/__tests__/index.test.ts | 86 +++++++++++++++++++ .../syncAudiences/index.ts | 20 ++++- 2 files changed, 102 insertions(+), 4 deletions(-) diff --git a/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/__tests__/index.test.ts b/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/__tests__/index.test.ts index 596193a2677..4aa7d4feb35 100644 --- a/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/__tests__/index.test.ts +++ b/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/__tests__/index.test.ts @@ -264,6 +264,92 @@ describe('MS Bing Ads Audiences syncAudiences', () => { expect(payloadArg[0].email).toBe('add1@segment.com') }) + describe('debug logging (actions-ms-bing-ads-audiences-debug-logging flag)', () => { + const DEBUG_FLAG = 'actions-ms-bing-ads-audiences-debug-logging' + + const makeLogger = () => + ({ + info: jest.fn(), + error: jest.fn(), + warn: jest.fn(), + debug: jest.fn(), + crit: jest.fn(), + log: jest.fn(), + withTags: jest.fn(), + level: 'info', + name: 'test' + } as any) + + const addEvent = () => + createTestEvent({ + type: 'identify', + properties: { aud_key: true }, + context: { traits: { email: 'demo@segment.com' } } + }) + + it('does not log when the flag is off', async () => { + nock(BASE_URL).post('/CustomerListUserData/Apply').reply(200, {}) + const logger = makeLogger() + + await testDestination.testAction('syncAudiences', { + event: addEvent(), + mapping: baseMapping, + useDefaultMappings: true, + settings, + logger, + features: { [DEBUG_FLAG]: false } + }) + + expect(logger.info).not.toHaveBeenCalled() + expect(logger.error).not.toHaveBeenCalled() + }) + + it('logs response metadata when the flag is on, without leaking CustomerListItems', async () => { + nock(BASE_URL).post('/CustomerListUserData/Apply').reply(200, { PartialErrors: [] }) + const logger = makeLogger() + + await testDestination.testAction('syncAudiences', { + event: addEvent(), + mapping: baseMapping, + useDefaultMappings: true, + settings, + logger, + features: { [DEBUG_FLAG]: true } + }) + + expect(logger.info).toHaveBeenCalledTimes(1) + const logged = (logger.info as jest.Mock).mock.calls[0][0] as string + expect(logged).toContain('[ms-bing-ads-audiences][DEBUG]') + expect(logged).toContain('identifierType=Email') + expect(logged).toContain('itemCount=1') + // The hashed identifier must never be logged. + expect(logged).not.toContain('5a95f052958dac8ed1d66d74eb481b3ccdbbc953b583c5ff0325be6b091d6281') + }) + + it('logs the error body and still lets handleHttpError consume the response', async () => { + nock(BASE_URL).post('/CustomerListUserData/Apply').reply(500, { message: 'boom' }) + const logger = makeLogger() + + const response = await testDestination.testBatchAction('syncAudiences', { + events: [addEvent()], + mapping: baseMapping, + useDefaultMappings: true, + settings, + logger, + features: { [DEBUG_FLAG]: true } + }) + + expect(logger.error).toHaveBeenCalledTimes(1) + const logged = (logger.error as jest.Mock).mock.calls[0][0] as string + expect(logged).toContain('[ms-bing-ads-audiences][DEBUG] Apply failed') + expect(logged).toContain('boom') + + // handleHttpError must still be able to read the (cloned) response body. + expect(utils.handleHttpError).toHaveBeenCalled() + expect(response[0].status).toBe(500) + }) + }) + it('should throw non-HTTP errors in batch mode', async () => { // Create a custom error that is NOT an HTTPError const customError = new Error('Custom non-HTTP error') diff --git a/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/index.ts b/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/index.ts index fb5da724b55..e1068d8fcda 100644 --- a/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/index.ts +++ b/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/index.ts @@ -54,7 +54,15 @@ const DEBUG_LOGGING_FLAG = 'actions-ms-bing-ads-audiences-debug-logging' const isDebugLoggingEnabled = (features: Record | undefined): boolean => Boolean(features && features[DEBUG_LOGGING_FLAG]) -// TEMPORARY: logs the request payload sent to Bing Ads and the raw response body. +// Cap logged bodies so a large or malformed response can't produce oversized log entries. +const MAX_LOGGED_BODY_LENGTH = 4096 + +const truncate = (value: string): string => + value.length > MAX_LOGGED_BODY_LENGTH ? `${value.slice(0, MAX_LOGGED_BODY_LENGTH)}…[truncated]` : value + +// TEMPORARY: logs the Bing Ads response body, plus non-sensitive metadata about the request. +// We intentionally do NOT log CustomerListItems (hashed emails / CRM IDs) — only the +// identifier type and item count — to avoid leaking customer-match identifiers into logs. const logBingAdsResponse = ( logger: Logger | undefined, debugLogging: boolean, @@ -66,13 +74,17 @@ const logBingAdsResponse = ( if (!debugLogging || !logger) { return } + const { CustomerListItemSubType, CustomerListItems } = sentPayload.CustomerListUserData logger.info( `[ms-bing-ads-audiences][DEBUG] ${action} audienceId=${audienceId} status=${response.status} ` + - `sent=${JSON.stringify(sentPayload)} response=${JSON.stringify(response.data)}` + `identifierType=${CustomerListItemSubType} itemCount=${CustomerListItems.length} ` + + `response=${truncate(JSON.stringify(response.data))}` ) } // TEMPORARY: logs the error body returned by Bing Ads when an Apply call fails. +// The body is read as text (not assumed to be JSON) and size-capped, then the response is +// cloned so handleHttpError can still consume the original body. const logBingAdsError = async ( logger: Logger | undefined, debugLogging: boolean, @@ -84,11 +96,11 @@ const logBingAdsError = async ( } let body: string try { - body = error instanceof HTTPError ? JSON.stringify(await error.response?.clone().json()) : String(error) + body = error instanceof HTTPError ? (await error.response?.clone().text()) ?? '' : String(error) } catch { body = error instanceof Error ? error.message : String(error) } - logger.error(`[ms-bing-ads-audiences][DEBUG] Apply failed audienceId=${audienceId} error=${body}`) + logger.error(`[ms-bing-ads-audiences][DEBUG] Apply failed audienceId=${audienceId} error=${truncate(body)}`) } /** From 8f2b03f85c76fa1fdc8d6c097396ac081fdf8acb Mon Sep 17 00:00:00 2001 From: Harsh Joshi Date: Wed, 17 Jun 2026 16:57:38 +0530 Subject: [PATCH 03/10] [ms-bing-ads-audiences] Log response.content instead of response.data response.data can be undefined for an empty/non-JSON body, and JSON.stringify(undefined) returns undefined, which would throw inside truncate(). Log response.content (always a string) instead, and add a test asserting debug logging does not crash on an empty response body. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../syncAudiences/__tests__/index.test.ts | 18 ++++++++++++++++++ .../syncAudiences/index.ts | 4 +++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/__tests__/index.test.ts b/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/__tests__/index.test.ts index 4aa7d4feb35..a206da14ee1 100644 --- a/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/__tests__/index.test.ts +++ b/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/__tests__/index.test.ts @@ -326,6 +326,24 @@ describe('MS Bing Ads Audiences syncAudiences', () => { expect(logged).not.toContain('5a95f052958dac8ed1d66d74eb481b3ccdbbc953b583c5ff0325be6b091d6281') }) + it('does not crash when the response body is empty', async () => { + // Empty body => response.data is undefined; logging must still succeed. + nock(BASE_URL).post('/CustomerListUserData/Apply').reply(200) + const logger = makeLogger() + + const response = await testDestination.testAction('syncAudiences', { + event: addEvent(), + mapping: baseMapping, + useDefaultMappings: true, + settings, + logger, + features: { [DEBUG_FLAG]: true } + }) + + expect(logger.info).toHaveBeenCalledTimes(1) + expect(response[0].status).toBe(200) + }) + it('logs the error body and still lets handleHttpError consume the response', async () => { nock(BASE_URL).post('/CustomerListUserData/Apply').reply(500, { message: 'boom' }) const logger = makeLogger() diff --git a/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/index.ts b/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/index.ts index e1068d8fcda..5e0c459a2e4 100644 --- a/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/index.ts +++ b/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/index.ts @@ -75,10 +75,12 @@ const logBingAdsResponse = ( return } const { CustomerListItemSubType, CustomerListItems } = sentPayload.CustomerListUserData + // Use `response.content` (always a string) rather than `response.data` (can be undefined + // for an empty/non-JSON body, which would make truncate() throw). logger.info( `[ms-bing-ads-audiences][DEBUG] ${action} audienceId=${audienceId} status=${response.status} ` + `identifierType=${CustomerListItemSubType} itemCount=${CustomerListItems.length} ` + - `response=${truncate(JSON.stringify(response.data))}` + `response=${truncate(response.content ?? '')}` ) } From f08f4a95ae2ac1618b34d35b88495cf11762bb1d Mon Sep 17 00:00:00 2001 From: Harsh Joshi Date: Wed, 17 Jun 2026 17:17:31 +0530 Subject: [PATCH 04/10] [ms-bing-ads-audiences] Correct debug-logging header comment The comment claimed it logs "raw request payloads", but the implementation deliberately omits CustomerListItems and logs only non-sensitive request metadata (identifier type + item count). Update the comment to match. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../ms-bing-ads-audiences/syncAudiences/index.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/index.ts b/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/index.ts index 5e0c459a2e4..4047c47659f 100644 --- a/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/index.ts +++ b/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/index.ts @@ -46,9 +46,9 @@ const action: ActionDefinition = { } // TEMPORARY DEBUG LOGGING — gated behind the 'actions-ms-bing-ads-audiences-debug-logging' -// feature flag. Enables logging of the raw request payloads and Bing Ads API responses -// (including error bodies) so they can be inspected in the centralized logging pipeline. -// Remove once debugging is complete. +// feature flag. Logs non-sensitive request metadata (identifier type + item count) and the +// Bing Ads API response/error bodies to the centralized logging pipeline. It intentionally +// does NOT log CustomerListItems (hashed emails / CRM IDs). Remove once debugging is complete. const DEBUG_LOGGING_FLAG = 'actions-ms-bing-ads-audiences-debug-logging' const isDebugLoggingEnabled = (features: Record | undefined): boolean => From 96f770f56b915d7a9a38a26bb5101df2afa2e641 Mon Sep 17 00:00:00 2001 From: Harsh Joshi Date: Wed, 17 Jun 2026 17:35:23 +0530 Subject: [PATCH 05/10] [ms-bing-ads-audiences] Include request metadata in error debug log The error log only carried audienceId, so a failure couldn't be correlated with the attempted operation. Track the in-flight Apply payload and include the same non-sensitive metadata as the success log (action, identifier type, item count) in logBingAdsError. CustomerListItems are still never logged. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../syncAudiences/__tests__/index.test.ts | 4 +++ .../syncAudiences/index.ts | 26 ++++++++++++++----- 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/__tests__/index.test.ts b/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/__tests__/index.test.ts index a206da14ee1..78c4001f9e3 100644 --- a/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/__tests__/index.test.ts +++ b/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/__tests__/index.test.ts @@ -361,6 +361,10 @@ describe('MS Bing Ads Audiences syncAudiences', () => { const logged = (logger.error as jest.Mock).mock.calls[0][0] as string expect(logged).toContain('[ms-bing-ads-audiences][DEBUG] Apply failed') expect(logged).toContain('boom') + // Error log carries the same non-sensitive request metadata as the success log. + expect(logged).toContain('action=Add') + expect(logged).toContain('identifierType=Email') + expect(logged).toContain('itemCount=1') // handleHttpError must still be able to read the (cloned) response body. expect(utils.handleHttpError).toHaveBeenCalled() diff --git a/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/index.ts b/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/index.ts index 4047c47659f..162217f0afd 100644 --- a/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/index.ts +++ b/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/index.ts @@ -84,14 +84,18 @@ const logBingAdsResponse = ( ) } -// TEMPORARY: logs the error body returned by Bing Ads when an Apply call fails. -// The body is read as text (not assumed to be JSON) and size-capped, then the response is -// cloned so handleHttpError can still consume the original body. +// TEMPORARY: logs the error body returned by Bing Ads when an Apply call fails, plus the same +// non-sensitive metadata as the success log so the failure can be correlated with the attempted +// operation. `attempted` is the payload of the in-flight Apply call (undefined if it failed +// before a call was made). The body is read as text (not assumed to be JSON) and size-capped, +// then the response is cloned so handleHttpError can still consume the original body. We never +// log CustomerListItems. const logBingAdsError = async ( logger: Logger | undefined, debugLogging: boolean, audienceId: string, - error: unknown + error: unknown, + attempted?: SyncAudiencePayload ): Promise => { if (!debugLogging || !logger) { return @@ -102,7 +106,11 @@ const logBingAdsError = async ( } catch { body = error instanceof Error ? error.message : String(error) } - logger.error(`[ms-bing-ads-audiences][DEBUG] Apply failed audienceId=${audienceId} error=${truncate(body)}`) + const data = attempted?.CustomerListUserData + const context = data + ? `action=${data.ActionType} identifierType=${data.CustomerListItemSubType} itemCount=${data.CustomerListItems.length} ` + : '' + logger.error(`[ms-bing-ads-audiences][DEBUG] Apply failed audienceId=${audienceId} ${context}error=${truncate(body)}`) } /** @@ -149,20 +157,26 @@ const syncUser = async ( const removePayload: SyncAudiencePayload = preparePayload(audienceId, 'Remove', identifierType, removeItems) + // Tracks the Apply payload currently in flight so an error can be correlated with the + // operation (Add vs Remove) that failed. + let attemptedPayload: SyncAudiencePayload | undefined + try { // Send data to Microsoft Bing Ads for both Add and Remove actions if they have entries if (addMap.size > 0) { + attemptedPayload = addPayload const response = await sendDataToMicrosoftBingAds(request, addPayload) logBingAdsResponse(logger, debugLogging, 'Add', audienceId, addPayload, response) handleMultistatusResponse(msResponse, response, addItems, addMap, payload, isBatch) } if (removeMap.size > 0) { + attemptedPayload = removePayload const response = await sendDataToMicrosoftBingAds(request, removePayload) logBingAdsResponse(logger, debugLogging, 'Remove', audienceId, removePayload, response) handleMultistatusResponse(msResponse, response, removeItems, removeMap, payload, isBatch) } } catch (error) { - await logBingAdsError(logger, debugLogging, audienceId, error) + await logBingAdsError(logger, debugLogging, audienceId, error, attemptedPayload) if (!isBatch) { throw error } From 3f1653645399e5bfc5d2ff50524c5b9e1aabbd16 Mon Sep 17 00:00:00 2001 From: Harsh Joshi Date: Wed, 17 Jun 2026 18:13:13 +0530 Subject: [PATCH 06/10] [ms-bing-ads-audiences] Harden debug logging (review feedback) Addresses deepreview findings on the debug-logging path: - PII: stop logging raw response/error bodies. PartialErrors are reduced to a PII-safe summary (ErrorCode/Code/Index/Type) and the free-text fields (Message/Details/FieldPath), which can echo unhashed CRM IDs, are dropped. The HTTP error path now logs status + redacted PartialErrors only. - Log injection: strip ASCII control chars (incl. CR/LF) from logged content via sanitizeForLog so a crafted body can't forge log lines. - Robustness: wrap logger.info/logger.error in safeLog() so a throwing logger can never mask the original error or skip handleHttpError. - Truncation: avoid splitting a UTF-16 surrogate pair when capping length. Tests updated for the redacted format; added cases for PartialError redaction, control-char stripping, and a throwing logger not breaking the action. 36/36 pass. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../syncAudiences/__tests__/index.test.ts | 90 +++++++++++++++- .../syncAudiences/index.ts | 101 +++++++++++++----- 2 files changed, 164 insertions(+), 27 deletions(-) diff --git a/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/__tests__/index.test.ts b/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/__tests__/index.test.ts index 78c4001f9e3..35ef2356736 100644 --- a/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/__tests__/index.test.ts +++ b/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/__tests__/index.test.ts @@ -322,10 +322,94 @@ describe('MS Bing Ads Audiences syncAudiences', () => { expect(logged).toContain('[ms-bing-ads-audiences][DEBUG]') expect(logged).toContain('identifierType=Email') expect(logged).toContain('itemCount=1') + expect(logged).toContain('partialErrors=[]') // The hashed identifier must never be logged. expect(logged).not.toContain('5a95f052958dac8ed1d66d74eb481b3ccdbbc953b583c5ff0325be6b091d6281') }) + it('redacts PartialError free-text fields that can echo identifiers', async () => { + // Bing can echo the offending identifier in Message/Details/FieldPath. Only codes/index + // should be logged, never the free-text fields. + nock(BASE_URL) + .post('/CustomerListUserData/Apply') + .reply(200, { + PartialErrors: [ + { + ErrorCode: 'InvalidCustomerListItem', + Code: 4001, + Index: 0, + Type: 'BatchError', + Message: 'Invalid value crm_secret_12345', + Details: 'crm_secret_12345', + FieldPath: 'CustomerListItems[0]=crm_secret_12345' + } + ] + }) + const logger = makeLogger() + + await testDestination.testBatchAction('syncAudiences', { + events: [addEvent()], + mapping: baseMapping, + useDefaultMappings: true, + settings, + logger, + features: { [DEBUG_FLAG]: true } + }) + + const logged = (logger.info as jest.Mock).mock.calls[0][0] as string + expect(logged).toContain('ErrorCode') + expect(logged).toContain('InvalidCustomerListItem') + // The free-text fields (and any identifier they echo) must not be logged. + expect(logged).not.toContain('crm_secret_12345') + expect(logged).not.toContain('Message') + expect(logged).not.toContain('FieldPath') + }) + + it('strips control characters from logged content to prevent log injection', async () => { + // A crafted response body with newlines must not be able to forge log lines. + nock(BASE_URL) + .post('/CustomerListUserData/Apply') + .reply(200, { + PartialErrors: [ + { ErrorCode: 'X\nInjected', Code: 1, Index: 0, Type: 'T', Message: null, Details: null, FieldPath: null } + ] + }) + const logger = makeLogger() + + await testDestination.testBatchAction('syncAudiences', { + events: [addEvent()], + mapping: baseMapping, + useDefaultMappings: true, + settings, + logger, + features: { [DEBUG_FLAG]: true } + }) + + const logged = (logger.info as jest.Mock).mock.calls[0][0] as string + expect(logged).not.toContain('\n') + }) + + it('does not let a throwing logger break the action', async () => { + nock(BASE_URL).post('/CustomerListUserData/Apply').reply(500, { message: 'boom' }) + const logger = makeLogger() + ;(logger.error as jest.Mock).mockImplementation(() => { + throw new Error('logger exploded') + }) + + // The throwing logger must be swallowed so handleHttpError still runs. + const response = await testDestination.testBatchAction('syncAudiences', { + events: [addEvent()], + mapping: baseMapping, + useDefaultMappings: true, + settings, + logger, + features: { [DEBUG_FLAG]: true } + }) + + expect(utils.handleHttpError).toHaveBeenCalled() + expect(response[0].status).toBe(500) + }) + it('does not crash when the response body is empty', async () => { // Empty body => response.data is undefined; logging must still succeed. nock(BASE_URL).post('/CustomerListUserData/Apply').reply(200) @@ -344,7 +428,7 @@ describe('MS Bing Ads Audiences syncAudiences', () => { expect(response[0].status).toBe(200) }) - it('logs the error body and still lets handleHttpError consume the response', async () => { + it('logs error status/metadata and still lets handleHttpError consume the response', async () => { nock(BASE_URL).post('/CustomerListUserData/Apply').reply(500, { message: 'boom' }) const logger = makeLogger() @@ -360,11 +444,13 @@ describe('MS Bing Ads Audiences syncAudiences', () => { expect(logger.error).toHaveBeenCalledTimes(1) const logged = (logger.error as jest.Mock).mock.calls[0][0] as string expect(logged).toContain('[ms-bing-ads-audiences][DEBUG] Apply failed') - expect(logged).toContain('boom') + expect(logged).toContain('status=500') // Error log carries the same non-sensitive request metadata as the success log. expect(logged).toContain('action=Add') expect(logged).toContain('identifierType=Email') expect(logged).toContain('itemCount=1') + // The raw error body is not logged verbatim (it can echo identifiers). + expect(logged).not.toContain('boom') // handleHttpError must still be able to read the (cloned) response body. expect(utils.handleHttpError).toHaveBeenCalled() diff --git a/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/index.ts b/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/index.ts index 162217f0afd..9d2f8563288 100644 --- a/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/index.ts +++ b/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/index.ts @@ -20,7 +20,7 @@ import { handleHttpError, categorizePayloadByAction } from '../utils' -import { Identifier, SyncAudiencePayload } from '../types' +import { Identifier, SyncAudiencePayload, PartialError } from '../types' const action: ActionDefinition = { title: 'Sync Audiences', @@ -46,9 +46,11 @@ const action: ActionDefinition = { } // TEMPORARY DEBUG LOGGING — gated behind the 'actions-ms-bing-ads-audiences-debug-logging' -// feature flag. Logs non-sensitive request metadata (identifier type + item count) and the -// Bing Ads API response/error bodies to the centralized logging pipeline. It intentionally -// does NOT log CustomerListItems (hashed emails / CRM IDs). Remove once debugging is complete. +// feature flag. Logs non-sensitive request metadata (identifier type + item count) plus a +// redacted summary of the Bing Ads API response/errors to the centralized logging pipeline. +// It intentionally does NOT log CustomerListItems (hashed emails / CRM IDs) and strips the +// PartialError free-text fields (Message/Details/FieldPath) that can echo back identifiers — +// CRM IDs in particular are unhashed. Remove once debugging is complete. const DEBUG_LOGGING_FLAG = 'actions-ms-bing-ads-audiences-debug-logging' const isDebugLoggingEnabled = (features: Record | undefined): boolean => @@ -57,12 +59,47 @@ const isDebugLoggingEnabled = (features: Record | undefined): b // Cap logged bodies so a large or malformed response can't produce oversized log entries. const MAX_LOGGED_BODY_LENGTH = 4096 -const truncate = (value: string): string => - value.length > MAX_LOGGED_BODY_LENGTH ? `${value.slice(0, MAX_LOGGED_BODY_LENGTH)}…[truncated]` : value +// Truncate without splitting a surrogate pair (which would emit a lone surrogate), and strip +// control characters so logged content can't forge log lines or inject control sequences. +const sanitizeForLog = (value: string): string => { + let out = value + if (out.length > MAX_LOGGED_BODY_LENGTH) { + let end = MAX_LOGGED_BODY_LENGTH + const code = out.charCodeAt(end - 1) + // If the cut lands on the high half of a surrogate pair, drop it. + if (code >= 0xd800 && code <= 0xdbff) { + end -= 1 + } + out = `${out.slice(0, end)}…[truncated]` + } + // Stripping control chars is the intent here, so no-control-regex is expected. + // eslint-disable-next-line no-control-regex + return out.replace(/[\u0000-\u001f\u007f]/g, ' ') +} + +// Build a PII-safe summary of the Bing Ads response. PartialErrors are reduced to their codes +// and index — the free-text Message/Details/FieldPath fields can echo back the offending +// identifier value, so they are dropped. +const summarizeErrors = (errors: PartialError[] | undefined): string => { + if (!errors || errors.length === 0) { + return '[]' + } + return JSON.stringify(errors.map((e) => ({ ErrorCode: e.ErrorCode, Code: e.Code, Index: e.Index, Type: e.Type }))) +} -// TEMPORARY: logs the Bing Ads response body, plus non-sensitive metadata about the request. -// We intentionally do NOT log CustomerListItems (hashed emails / CRM IDs) — only the -// identifier type and item count — to avoid leaking customer-match identifiers into logs. +// Run a logging side effect, swallowing any error: temporary debug logging must never alter +// the action's control flow (e.g. by masking the original error in the catch path). +const safeLog = (fn: () => void): void => { + try { + fn() + } catch { + // Intentionally ignored — debug logging is best-effort. + } +} + +// TEMPORARY: logs a redacted summary of the Bing Ads response, plus non-sensitive metadata about +// the request. We intentionally do NOT log CustomerListItems (hashed emails / CRM IDs) or the +// PartialError free-text fields — only identifier type, item count, status and error codes. const logBingAdsResponse = ( logger: Logger | undefined, debugLogging: boolean, @@ -75,21 +112,23 @@ const logBingAdsResponse = ( return } const { CustomerListItemSubType, CustomerListItems } = sentPayload.CustomerListUserData - // Use `response.content` (always a string) rather than `response.data` (can be undefined - // for an empty/non-JSON body, which would make truncate() throw). - logger.info( - `[ms-bing-ads-audiences][DEBUG] ${action} audienceId=${audienceId} status=${response.status} ` + - `identifierType=${CustomerListItemSubType} itemCount=${CustomerListItems.length} ` + - `response=${truncate(response.content ?? '')}` + const partialErrors = (response.data as { PartialErrors?: PartialError[] } | undefined)?.PartialErrors + safeLog(() => + logger.info( + `[ms-bing-ads-audiences][DEBUG] ${action} audienceId=${audienceId} status=${response.status} ` + + `identifierType=${CustomerListItemSubType} itemCount=${CustomerListItems.length} ` + + `partialErrors=${sanitizeForLog(summarizeErrors(partialErrors))}` + ) ) } -// TEMPORARY: logs the error body returned by Bing Ads when an Apply call fails, plus the same +// TEMPORARY: logs the error returned by Bing Ads when an Apply call fails, plus the same // non-sensitive metadata as the success log so the failure can be correlated with the attempted // operation. `attempted` is the payload of the in-flight Apply call (undefined if it failed -// before a call was made). The body is read as text (not assumed to be JSON) and size-capped, -// then the response is cloned so handleHttpError can still consume the original body. We never -// log CustomerListItems. +// before a call was made). When the body parses as JSON, PartialErrors are reduced to a +// PII-safe summary; otherwise only the HTTP status is logged (the raw body can echo back +// identifiers, so it is not logged verbatim). The response is cloned so handleHttpError can +// still consume the original body. const logBingAdsError = async ( logger: Logger | undefined, debugLogging: boolean, @@ -100,17 +139,29 @@ const logBingAdsError = async ( if (!debugLogging || !logger) { return } - let body: string - try { - body = error instanceof HTTPError ? (await error.response?.clone().text()) ?? '' : String(error) - } catch { - body = error instanceof Error ? error.message : String(error) + let detail: string + if (error instanceof HTTPError) { + const status = error.response?.status + let summary = '' + try { + const parsed = (await error.response?.clone().json()) as { PartialErrors?: PartialError[] } | undefined + summary = summarizeErrors(parsed?.PartialErrors) + } catch { + // Non-JSON or already-consumed body — fall back to status only, never the raw body. + } + detail = `status=${status ?? 'unknown'} partialErrors=${summary}` + } else { + detail = `error=${error instanceof Error ? error.message : String(error)}` } const data = attempted?.CustomerListUserData const context = data ? `action=${data.ActionType} identifierType=${data.CustomerListItemSubType} itemCount=${data.CustomerListItems.length} ` : '' - logger.error(`[ms-bing-ads-audiences][DEBUG] Apply failed audienceId=${audienceId} ${context}error=${truncate(body)}`) + safeLog(() => + logger.error( + `[ms-bing-ads-audiences][DEBUG] Apply failed audienceId=${audienceId} ${context}${sanitizeForLog(detail)}` + ) + ) } /** From 4b77498e47cbb5eaac431176040efafd80190708 Mon Sep 17 00:00:00 2001 From: Harsh Joshi Date: Wed, 17 Jun 2026 18:23:06 +0530 Subject: [PATCH 07/10] [ms-bing-ads-audiences] Log Microsoft tracking id The earlier redaction dropped the raw response body, which also removed the Microsoft tracking/request id needed for Bing Ads support tickets. Add an explicit extractTrackingId() that reads it from the response header (TrackingId / x-ms-trackingid / RequestId) or a top-level body id field, and log it as its own dedicated field on both the success and error paths. The tracking id is a short opaque value (not PII) so it is control-char sanitized but never truncated. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../syncAudiences/__tests__/index.test.ts | 36 ++++++++++++++++++ .../syncAudiences/index.ts | 38 +++++++++++++++++-- 2 files changed, 70 insertions(+), 4 deletions(-) diff --git a/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/__tests__/index.test.ts b/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/__tests__/index.test.ts index 35ef2356736..8a39b0cc154 100644 --- a/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/__tests__/index.test.ts +++ b/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/__tests__/index.test.ts @@ -327,6 +327,42 @@ describe('MS Bing Ads Audiences syncAudiences', () => { expect(logged).not.toContain('5a95f052958dac8ed1d66d74eb481b3ccdbbc953b583c5ff0325be6b091d6281') }) + it('logs the Microsoft tracking id from the response header', async () => { + nock(BASE_URL) + .post('/CustomerListUserData/Apply') + .reply(200, { PartialErrors: [] }, { TrackingId: 'abc-123-track' }) + const logger = makeLogger() + + await testDestination.testAction('syncAudiences', { + event: addEvent(), + mapping: baseMapping, + useDefaultMappings: true, + settings, + logger, + features: { [DEBUG_FLAG]: true } + }) + + const logged = (logger.info as jest.Mock).mock.calls[0][0] as string + expect(logged).toContain('trackingId=abc-123-track') + }) + + it('falls back to a body-level tracking id when no header is present', async () => { + nock(BASE_URL).post('/CustomerListUserData/Apply').reply(200, { TrackingId: 'body-track-789', PartialErrors: [] }) + const logger = makeLogger() + + await testDestination.testAction('syncAudiences', { + event: addEvent(), + mapping: baseMapping, + useDefaultMappings: true, + settings, + logger, + features: { [DEBUG_FLAG]: true } + }) + + const logged = (logger.info as jest.Mock).mock.calls[0][0] as string + expect(logged).toContain('trackingId=body-track-789') + }) + it('redacts PartialError free-text fields that can echo identifiers', async () => { // Bing can echo the offending identifier in Message/Details/FieldPath. Only codes/index // should be logged, never the free-text fields. diff --git a/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/index.ts b/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/index.ts index 9d2f8563288..cd011feb070 100644 --- a/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/index.ts +++ b/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/index.ts @@ -87,6 +87,31 @@ const summarizeErrors = (errors: PartialError[] | undefined): string => { return JSON.stringify(errors.map((e) => ({ ErrorCode: e.ErrorCode, Code: e.Code, Index: e.Index, Type: e.Type }))) } +// Extract Microsoft's request/tracking identifier so it can be quoted to Bing Ads support. +// It is normally returned as a response header (TrackingId / x-ms-trackingid / RequestId) and +// is sometimes also echoed in the body as a top-level id field. It is a short opaque id, not +// PII, so it is logged in full (control chars stripped, never truncated). +const TRACKING_HEADER_NAMES = ['trackingid', 'x-ms-trackingid', 'requestid', 'x-ms-requestid'] +const TRACKING_BODY_KEYS = ['TrackingId', 'RequestId'] + +const extractTrackingId = (headers: Headers | undefined, body: unknown): string => { + for (const name of TRACKING_HEADER_NAMES) { + const value = headers?.get(name) + if (value) { + return value + } + } + if (body && typeof body === 'object') { + for (const key of TRACKING_BODY_KEYS) { + const value = (body as Record)[key] + if (typeof value === 'string' && value) { + return value + } + } + } + return 'none' +} + // Run a logging side effect, swallowing any error: temporary debug logging must never alter // the action's control flow (e.g. by masking the original error in the catch path). const safeLog = (fn: () => void): void => { @@ -98,8 +123,9 @@ const safeLog = (fn: () => void): void => { } // TEMPORARY: logs a redacted summary of the Bing Ads response, plus non-sensitive metadata about -// the request. We intentionally do NOT log CustomerListItems (hashed emails / CRM IDs) or the -// PartialError free-text fields — only identifier type, item count, status and error codes. +// the request and Microsoft's tracking id (for support tickets). We intentionally do NOT log +// CustomerListItems (hashed emails / CRM IDs) or the PartialError free-text fields — only +// identifier type, item count, status, tracking id and error codes. const logBingAdsResponse = ( logger: Logger | undefined, debugLogging: boolean, @@ -113,9 +139,11 @@ const logBingAdsResponse = ( } const { CustomerListItemSubType, CustomerListItems } = sentPayload.CustomerListUserData const partialErrors = (response.data as { PartialErrors?: PartialError[] } | undefined)?.PartialErrors + const trackingId = extractTrackingId(response.headers, response.data) safeLog(() => logger.info( `[ms-bing-ads-audiences][DEBUG] ${action} audienceId=${audienceId} status=${response.status} ` + + `trackingId=${sanitizeForLog(trackingId)} ` + `identifierType=${CustomerListItemSubType} itemCount=${CustomerListItems.length} ` + `partialErrors=${sanitizeForLog(summarizeErrors(partialErrors))}` ) @@ -143,13 +171,15 @@ const logBingAdsError = async ( if (error instanceof HTTPError) { const status = error.response?.status let summary = '' + let parsed: { PartialErrors?: PartialError[] } | undefined try { - const parsed = (await error.response?.clone().json()) as { PartialErrors?: PartialError[] } | undefined + parsed = (await error.response?.clone().json()) as { PartialErrors?: PartialError[] } | undefined summary = summarizeErrors(parsed?.PartialErrors) } catch { // Non-JSON or already-consumed body — fall back to status only, never the raw body. } - detail = `status=${status ?? 'unknown'} partialErrors=${summary}` + const trackingId = extractTrackingId(error.response?.headers, parsed) + detail = `status=${status ?? 'unknown'} trackingId=${sanitizeForLog(trackingId)} partialErrors=${summary}` } else { detail = `error=${error instanceof Error ? error.message : String(error)}` } From d153c1bc0397428aa49c018ef733eb6cbdeb1bc9 Mon Sep 17 00:00:00 2001 From: Harsh Joshi Date: Wed, 17 Jun 2026 18:34:07 +0530 Subject: [PATCH 08/10] [ms-bing-ads-audiences] Sanitize audienceId; make truncation cap strict MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the latest Copilot review: - Run audienceId through sanitizeForLog on both the success and error log lines so a crafted audience id can't inject newlines/control chars. - Make the truncation strict: reserve the suffix length so the returned string (incl. '…[truncated]') never exceeds MAX_LOGGED_BODY_LENGTH. Added tests for audienceId sanitization and the strict cap. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../syncAudiences/__tests__/index.test.ts | 46 +++++++++++++++++++ .../syncAudiences/index.ts | 15 ++++-- 2 files changed, 56 insertions(+), 5 deletions(-) diff --git a/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/__tests__/index.test.ts b/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/__tests__/index.test.ts index 8a39b0cc154..367ab4d840a 100644 --- a/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/__tests__/index.test.ts +++ b/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/__tests__/index.test.ts @@ -425,6 +425,52 @@ describe('MS Bing Ads Audiences syncAudiences', () => { expect(logged).not.toContain('\n') }) + it('sanitizes a crafted audience id to prevent log injection', async () => { + nock(BASE_URL).post('/CustomerListUserData/Apply').reply(200, { PartialErrors: [] }) + const logger = makeLogger() + + await testDestination.testAction('syncAudiences', { + event: addEvent(), + mapping: { ...baseMapping, audience_id: 'aud\n_injected' }, + useDefaultMappings: true, + settings, + logger, + features: { [DEBUG_FLAG]: true } + }) + + const logged = (logger.info as jest.Mock).mock.calls[0][0] as string + expect(logged).not.toContain('\n') + }) + + it('truncates oversized values without exceeding the cap', async () => { + // A long PartialError code forces truncation; the resulting log line's summary segment + // (including the suffix) must stay within the configured cap. + const longCode = 'C'.repeat(10000) + nock(BASE_URL) + .post('/CustomerListUserData/Apply') + .reply(200, { + PartialErrors: [ + { ErrorCode: longCode, Code: 1, Index: 0, Type: 'T', Message: null, Details: null, FieldPath: null } + ] + }) + const logger = makeLogger() + + await testDestination.testBatchAction('syncAudiences', { + events: [addEvent()], + mapping: baseMapping, + useDefaultMappings: true, + settings, + logger, + features: { [DEBUG_FLAG]: true } + }) + + const logged = (logger.info as jest.Mock).mock.calls[0][0] as string + const partialErrors = logged.split('partialErrors=')[1] + expect(partialErrors).toContain('[truncated]') + // The truncated segment (suffix included) must not exceed the 4096 cap. + expect(partialErrors.length).toBeLessThanOrEqual(4096) + }) + it('does not let a throwing logger break the action', async () => { nock(BASE_URL).post('/CustomerListUserData/Apply').reply(500, { message: 'boom' }) const logger = makeLogger() diff --git a/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/index.ts b/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/index.ts index cd011feb070..95610bc2ad0 100644 --- a/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/index.ts +++ b/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/index.ts @@ -56,21 +56,24 @@ const DEBUG_LOGGING_FLAG = 'actions-ms-bing-ads-audiences-debug-logging' const isDebugLoggingEnabled = (features: Record | undefined): boolean => Boolean(features && features[DEBUG_LOGGING_FLAG]) -// Cap logged bodies so a large or malformed response can't produce oversized log entries. +// Cap logged values so a large or malformed response can't produce oversized log entries. The +// cap is strict: the returned string (including the truncation suffix) never exceeds it. const MAX_LOGGED_BODY_LENGTH = 4096 +const TRUNCATION_SUFFIX = '…[truncated]' // Truncate without splitting a surrogate pair (which would emit a lone surrogate), and strip // control characters so logged content can't forge log lines or inject control sequences. const sanitizeForLog = (value: string): string => { let out = value if (out.length > MAX_LOGGED_BODY_LENGTH) { - let end = MAX_LOGGED_BODY_LENGTH + // Reserve room for the suffix so the total stays within the cap. + let end = MAX_LOGGED_BODY_LENGTH - TRUNCATION_SUFFIX.length const code = out.charCodeAt(end - 1) // If the cut lands on the high half of a surrogate pair, drop it. if (code >= 0xd800 && code <= 0xdbff) { end -= 1 } - out = `${out.slice(0, end)}…[truncated]` + out = `${out.slice(0, end)}${TRUNCATION_SUFFIX}` } // Stripping control chars is the intent here, so no-control-regex is expected. // eslint-disable-next-line no-control-regex @@ -142,7 +145,7 @@ const logBingAdsResponse = ( const trackingId = extractTrackingId(response.headers, response.data) safeLog(() => logger.info( - `[ms-bing-ads-audiences][DEBUG] ${action} audienceId=${audienceId} status=${response.status} ` + + `[ms-bing-ads-audiences][DEBUG] ${action} audienceId=${sanitizeForLog(audienceId)} status=${response.status} ` + `trackingId=${sanitizeForLog(trackingId)} ` + `identifierType=${CustomerListItemSubType} itemCount=${CustomerListItems.length} ` + `partialErrors=${sanitizeForLog(summarizeErrors(partialErrors))}` @@ -189,7 +192,9 @@ const logBingAdsError = async ( : '' safeLog(() => logger.error( - `[ms-bing-ads-audiences][DEBUG] Apply failed audienceId=${audienceId} ${context}${sanitizeForLog(detail)}` + `[ms-bing-ads-audiences][DEBUG] Apply failed audienceId=${sanitizeForLog(audienceId)} ${context}${sanitizeForLog( + detail + )}` ) ) } From 41a1d336d6cc47d3e59198180eca9739a3db01c2 Mon Sep 17 00:00:00 2001 From: Harsh Joshi Date: Thu, 18 Jun 2026 11:02:32 +0530 Subject: [PATCH 09/10] [ms-bing-ads-audiences] Never truncate the tracking id MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The tracking id was routed through sanitizeForLog (which caps at 4096) and, on the error path, re-capped via sanitizeForLog(detail) — contradicting the stated "logged in full, never truncated" guarantee. Split out stripControlChars() (no cap) and apply it to trackingId (and audienceId), while keeping the length cap only for the potentially-large PartialErrors summary and error message. Added a test asserting a long tracking id is logged verbatim. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../syncAudiences/__tests__/index.test.ts | 22 ++++++++++++ .../syncAudiences/index.ts | 35 ++++++++++++------- 2 files changed, 45 insertions(+), 12 deletions(-) diff --git a/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/__tests__/index.test.ts b/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/__tests__/index.test.ts index 367ab4d840a..0a2d75a87c1 100644 --- a/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/__tests__/index.test.ts +++ b/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/__tests__/index.test.ts @@ -363,6 +363,28 @@ describe('MS Bing Ads Audiences syncAudiences', () => { expect(logged).toContain('trackingId=body-track-789') }) + it('logs the tracking id in full, never truncated', async () => { + // Even an unexpectedly long tracking id must be quotable to Microsoft support verbatim. + const longTrackingId = `T-${'9'.repeat(6000)}` + nock(BASE_URL) + .post('/CustomerListUserData/Apply') + .reply(200, { PartialErrors: [] }, { TrackingId: longTrackingId }) + const logger = makeLogger() + + await testDestination.testAction('syncAudiences', { + event: addEvent(), + mapping: baseMapping, + useDefaultMappings: true, + settings, + logger, + features: { [DEBUG_FLAG]: true } + }) + + const logged = (logger.info as jest.Mock).mock.calls[0][0] as string + expect(logged).toContain(`trackingId=${longTrackingId}`) + expect(logged).not.toContain('[truncated]') + }) + it('redacts PartialError free-text fields that can echo identifiers', async () => { // Bing can echo the offending identifier in Message/Details/FieldPath. Only codes/index // should be logged, never the free-text fields. diff --git a/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/index.ts b/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/index.ts index 95610bc2ad0..6a77ac404fc 100644 --- a/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/index.ts +++ b/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/index.ts @@ -61,8 +61,16 @@ const isDebugLoggingEnabled = (features: Record | undefined): b const MAX_LOGGED_BODY_LENGTH = 4096 const TRUNCATION_SUFFIX = '…[truncated]' -// Truncate without splitting a surrogate pair (which would emit a lone surrogate), and strip -// control characters so logged content can't forge log lines or inject control sequences. +// Strip control characters so logged content can't forge log lines or inject control +// sequences. Applied to every interpolated value; does NOT cap length. +const stripControlChars = (value: string): string => + // Stripping control chars is the intent here, so no-control-regex is expected. + // eslint-disable-next-line no-control-regex + value.replace(/[\u0000-\u001f\u007f]/g, ' ') + +// Strip control chars AND cap length (truncating without splitting a surrogate pair). Use for +// values that can be arbitrarily large (response/error bodies); prefer stripControlChars alone +// for short identifiers that must be logged in full. const sanitizeForLog = (value: string): string => { let out = value if (out.length > MAX_LOGGED_BODY_LENGTH) { @@ -75,9 +83,7 @@ const sanitizeForLog = (value: string): string => { } out = `${out.slice(0, end)}${TRUNCATION_SUFFIX}` } - // Stripping control chars is the intent here, so no-control-regex is expected. - // eslint-disable-next-line no-control-regex - return out.replace(/[\u0000-\u001f\u007f]/g, ' ') + return stripControlChars(out) } // Build a PII-safe summary of the Bing Ads response. PartialErrors are reduced to their codes @@ -145,8 +151,11 @@ const logBingAdsResponse = ( const trackingId = extractTrackingId(response.headers, response.data) safeLog(() => logger.info( - `[ms-bing-ads-audiences][DEBUG] ${action} audienceId=${sanitizeForLog(audienceId)} status=${response.status} ` + - `trackingId=${sanitizeForLog(trackingId)} ` + + `[ms-bing-ads-audiences][DEBUG] ${action} audienceId=${stripControlChars(audienceId)} status=${ + response.status + } ` + + // trackingId is only control-char stripped, never truncated, so it can be quoted in full. + `trackingId=${stripControlChars(trackingId)} ` + `identifierType=${CustomerListItemSubType} itemCount=${CustomerListItems.length} ` + `partialErrors=${sanitizeForLog(summarizeErrors(partialErrors))}` ) @@ -182,9 +191,13 @@ const logBingAdsError = async ( // Non-JSON or already-consumed body — fall back to status only, never the raw body. } const trackingId = extractTrackingId(error.response?.headers, parsed) - detail = `status=${status ?? 'unknown'} trackingId=${sanitizeForLog(trackingId)} partialErrors=${summary}` + // Only the (potentially large) PartialErrors summary is capped; trackingId is stripped but + // logged in full so it can be quoted to Microsoft support. + detail = `status=${status ?? 'unknown'} trackingId=${stripControlChars(trackingId)} partialErrors=${sanitizeForLog( + summary + )}` } else { - detail = `error=${error instanceof Error ? error.message : String(error)}` + detail = `error=${sanitizeForLog(error instanceof Error ? error.message : String(error))}` } const data = attempted?.CustomerListUserData const context = data @@ -192,9 +205,7 @@ const logBingAdsError = async ( : '' safeLog(() => logger.error( - `[ms-bing-ads-audiences][DEBUG] Apply failed audienceId=${sanitizeForLog(audienceId)} ${context}${sanitizeForLog( - detail - )}` + `[ms-bing-ads-audiences][DEBUG] Apply failed audienceId=${stripControlChars(audienceId)} ${context}${detail}` ) ) } From aa5b94b1d35dfe0be3e6e725323f76547bd8dbb1 Mon Sep 17 00:00:00 2001 From: Harsh Joshi Date: Thu, 18 Jun 2026 11:23:32 +0530 Subject: [PATCH 10/10] [ms-bing-ads-audiences] Only log successful responses; drop error logging Remove logBingAdsError entirely. The success path already captures what we need (tracking id + per-item PartialErrors on the 200 response), and the error path was the sole source of the PII leak (raw IntegrationError.message in the non-batch path), the response-clone/stream handling, and the attemptedPayload tracking. Dropping it removes all of that complexity. The HTTP error path now just flows through to handleHttpError unchanged. Tests updated: error path asserts nothing is logged; throwing-logger test moved to the success path. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../syncAudiences/__tests__/index.test.ts | 31 ++++------- .../syncAudiences/index.ts | 55 ------------------- 2 files changed, 11 insertions(+), 75 deletions(-) diff --git a/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/__tests__/index.test.ts b/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/__tests__/index.test.ts index 0a2d75a87c1..0260dad8dc3 100644 --- a/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/__tests__/index.test.ts +++ b/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/__tests__/index.test.ts @@ -494,15 +494,15 @@ describe('MS Bing Ads Audiences syncAudiences', () => { }) it('does not let a throwing logger break the action', async () => { - nock(BASE_URL).post('/CustomerListUserData/Apply').reply(500, { message: 'boom' }) + nock(BASE_URL).post('/CustomerListUserData/Apply').reply(200, { PartialErrors: [] }) const logger = makeLogger() - ;(logger.error as jest.Mock).mockImplementation(() => { + ;(logger.info as jest.Mock).mockImplementation(() => { throw new Error('logger exploded') }) - // The throwing logger must be swallowed so handleHttpError still runs. - const response = await testDestination.testBatchAction('syncAudiences', { - events: [addEvent()], + // A throwing logger must be swallowed so the action still succeeds. + const response = await testDestination.testAction('syncAudiences', { + event: addEvent(), mapping: baseMapping, useDefaultMappings: true, settings, @@ -510,8 +510,7 @@ describe('MS Bing Ads Audiences syncAudiences', () => { features: { [DEBUG_FLAG]: true } }) - expect(utils.handleHttpError).toHaveBeenCalled() - expect(response[0].status).toBe(500) + expect(response[0].status).toBe(200) }) it('does not crash when the response body is empty', async () => { @@ -532,7 +531,7 @@ describe('MS Bing Ads Audiences syncAudiences', () => { expect(response[0].status).toBe(200) }) - it('logs error status/metadata and still lets handleHttpError consume the response', async () => { + it('does not log on the error path (only success responses are logged)', async () => { nock(BASE_URL).post('/CustomerListUserData/Apply').reply(500, { message: 'boom' }) const logger = makeLogger() @@ -545,18 +544,10 @@ describe('MS Bing Ads Audiences syncAudiences', () => { features: { [DEBUG_FLAG]: true } }) - expect(logger.error).toHaveBeenCalledTimes(1) - const logged = (logger.error as jest.Mock).mock.calls[0][0] as string - expect(logged).toContain('[ms-bing-ads-audiences][DEBUG] Apply failed') - expect(logged).toContain('status=500') - // Error log carries the same non-sensitive request metadata as the success log. - expect(logged).toContain('action=Add') - expect(logged).toContain('identifierType=Email') - expect(logged).toContain('itemCount=1') - // The raw error body is not logged verbatim (it can echo identifiers). - expect(logged).not.toContain('boom') - - // handleHttpError must still be able to read the (cloned) response body. + // We only log successful responses; the error path emits nothing. + expect(logger.info).not.toHaveBeenCalled() + expect(logger.error).not.toHaveBeenCalled() + // The HTTP error is still handled normally. expect(utils.handleHttpError).toHaveBeenCalled() expect(response[0].status).toBe(500) }) diff --git a/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/index.ts b/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/index.ts index 6a77ac404fc..12ee3bce235 100644 --- a/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/index.ts +++ b/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/index.ts @@ -162,54 +162,6 @@ const logBingAdsResponse = ( ) } -// TEMPORARY: logs the error returned by Bing Ads when an Apply call fails, plus the same -// non-sensitive metadata as the success log so the failure can be correlated with the attempted -// operation. `attempted` is the payload of the in-flight Apply call (undefined if it failed -// before a call was made). When the body parses as JSON, PartialErrors are reduced to a -// PII-safe summary; otherwise only the HTTP status is logged (the raw body can echo back -// identifiers, so it is not logged verbatim). The response is cloned so handleHttpError can -// still consume the original body. -const logBingAdsError = async ( - logger: Logger | undefined, - debugLogging: boolean, - audienceId: string, - error: unknown, - attempted?: SyncAudiencePayload -): Promise => { - if (!debugLogging || !logger) { - return - } - let detail: string - if (error instanceof HTTPError) { - const status = error.response?.status - let summary = '' - let parsed: { PartialErrors?: PartialError[] } | undefined - try { - parsed = (await error.response?.clone().json()) as { PartialErrors?: PartialError[] } | undefined - summary = summarizeErrors(parsed?.PartialErrors) - } catch { - // Non-JSON or already-consumed body — fall back to status only, never the raw body. - } - const trackingId = extractTrackingId(error.response?.headers, parsed) - // Only the (potentially large) PartialErrors summary is capped; trackingId is stripped but - // logged in full so it can be quoted to Microsoft support. - detail = `status=${status ?? 'unknown'} trackingId=${stripControlChars(trackingId)} partialErrors=${sanitizeForLog( - summary - )}` - } else { - detail = `error=${sanitizeForLog(error instanceof Error ? error.message : String(error))}` - } - const data = attempted?.CustomerListUserData - const context = data - ? `action=${data.ActionType} identifierType=${data.CustomerListItemSubType} itemCount=${data.CustomerListItems.length} ` - : '' - safeLog(() => - logger.error( - `[ms-bing-ads-audiences][DEBUG] Apply failed audienceId=${stripControlChars(audienceId)} ${context}${detail}` - ) - ) -} - /** * Synchronizes user audience data with Microsoft Bing Ads. * @@ -254,26 +206,19 @@ const syncUser = async ( const removePayload: SyncAudiencePayload = preparePayload(audienceId, 'Remove', identifierType, removeItems) - // Tracks the Apply payload currently in flight so an error can be correlated with the - // operation (Add vs Remove) that failed. - let attemptedPayload: SyncAudiencePayload | undefined - try { // Send data to Microsoft Bing Ads for both Add and Remove actions if they have entries if (addMap.size > 0) { - attemptedPayload = addPayload const response = await sendDataToMicrosoftBingAds(request, addPayload) logBingAdsResponse(logger, debugLogging, 'Add', audienceId, addPayload, response) handleMultistatusResponse(msResponse, response, addItems, addMap, payload, isBatch) } if (removeMap.size > 0) { - attemptedPayload = removePayload const response = await sendDataToMicrosoftBingAds(request, removePayload) logBingAdsResponse(logger, debugLogging, 'Remove', audienceId, removePayload, response) handleMultistatusResponse(msResponse, response, removeItems, removeMap, payload, isBatch) } } catch (error) { - await logBingAdsError(logger, debugLogging, audienceId, error, attemptedPayload) if (!isBatch) { throw error }