Sanitize waypoint fields before sending to API

src/libs/actions/IOU/index.ts

@@ -240,7 +240,7 @@
 import {buildOptimisticPolicyRecentlyUsedTags} from '@userActions/Policy/Tag';
 import type {GuidedSetupData} from '@userActions/Report';
 import {buildInviteToRoomOnyxData, completeOnboarding, notifyNewAction, optimisticReportLastData} from '@userActions/Report';
-import {mergeTransactionIdsHighlightOnSearchRoute, sanitizeRecentWaypoints} from '@userActions/Transaction';
+import {mergeTransactionIdsHighlightOnSearchRoute, sanitizeWaypointsForAPI, stringifyWaypointsForAPI} from '@userActions/Transaction';
 import {removeDraftTransaction, removeDraftTransactions, removeDraftTransactionsByIDs} from '@userActions/TransactionEdit';
 import {getOnboardingMessages} from '@userActions/Welcome/OnboardingFlow';
 import type {OnboardingCompanySize} from '@userActions/Welcome/OnboardingFlow';
@@ -771,7 +771,7 @@
     betas: OnyxEntry<OnyxTypes.Beta[]>;
 };
 
 type GetTrackExpenseInformationTransactionParams = {
     comment: string;
     amount: number;
     currency: string;
@@ -871,7 +871,7 @@
     chatReportID: string | undefined;
     chatReport: OnyxEntry<OnyxTypes.Report> | undefined;
     transactionID: string | undefined;
     reportAction: OnyxTypes.ReportAction;
     iouReport: OnyxEntry<OnyxTypes.Report>;
     chatIOUReport: OnyxEntry<OnyxTypes.Report>;
     transactions: OnyxCollection<OnyxTypes.Transaction>;
@@ -885,7 +885,7 @@
 
 type DeleteMoneyRequestFunctionParams = {
     transactionID: string | undefined;
     reportAction: OnyxTypes.ReportAction;
     transactions: OnyxCollection<OnyxTypes.Transaction>;
     violations: OnyxCollection<OnyxTypes.TransactionViolations>;
     iouReport: OnyxEntry<OnyxTypes.Report>;
@@ -894,7 +894,7 @@
     isSingleTransactionView?: boolean;
     transactionIDsPendingDeletion?: string[];
     selectedTransactionIDs?: string[];
     allTransactionViolationsParam: OnyxCollection<OnyxTypes.TransactionViolations>;
     currentUserAccountID: number;
 };
 
@@ -908,7 +908,7 @@
     userBillingGraceEndPeriods: OnyxCollection<OnyxTypes.BillingGraceEndPeriod>;
     paymentPolicyID?: string;
     full?: boolean;
     activePolicy?: OnyxEntry<OnyxTypes.Policy>;
     policy?: OnyxEntry<OnyxTypes.Policy>;
     betas: OnyxEntry<OnyxTypes.Beta[]>;
     isSelfTourViewed: boolean | undefined;
@@ -921,7 +921,7 @@
     waitForCollectionCallback: true,
     callback: (value) => {
         if (!value) {
             allTransactions = {};
             return;
         }
 
@@ -930,7 +930,7 @@
 });
 
 let allTransactionDrafts: NonNullable<OnyxCollection<OnyxTypes.Transaction>> = {};
 Onyx.connect({
     key: ONYXKEYS.COLLECTION.TRANSACTION_DRAFT,
     waitForCollectionCallback: true,
     callback: (value) => {
@@ -940,7 +940,7 @@
 
 let allTransactionViolations: NonNullable<OnyxCollection<OnyxTypes.TransactionViolations>> = {};
 Onyx.connect({
     key: ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS,
     waitForCollectionCallback: true,
     callback: (value) => {
         if (!value) {
@@ -949,7 +949,7 @@
         }
 
         allTransactionViolations = value;
     },
 });
 
 let allPolicyTags: OnyxCollection<OnyxTypes.PolicyTagLists> = {};
@@ -957,7 +957,7 @@
     key: ONYXKEYS.COLLECTION.POLICY_TAGS,
     waitForCollectionCallback: true,
     callback: (value) => {
         if (!value) {
             allPolicyTags = {};
             return;
         }
@@ -4611,8 +4611,7 @@
     const transactionDetails = getTransactionDetails(updatedTransaction, undefined, undefined, allowNegative);
 
     if (transactionDetails?.waypoints) {
-        // This needs to be a JSON string since we're sending this to the MapBox API
-        transactionDetails.waypoints = JSON.stringify(transactionDetails.waypoints);
+        transactionDetails.waypoints = stringifyWaypointsForAPI(transactionDetails.waypoints as WaypointCollection);
     }
 
     const dataToIncludeInParams: Partial<TransactionDetails> = Object.fromEntries(Object.entries(transactionDetails ?? {}).filter(([key]) => key in transactionChanges));
@@ -5159,8 +5158,7 @@
     const transactionDetails = getTransactionDetails(updatedTransaction);
 
     if (transactionDetails?.waypoints) {
-        // This needs to be a JSON string since we're sending this to the MapBox API
-        transactionDetails.waypoints = JSON.stringify(transactionDetails.waypoints);
+        transactionDetails.waypoints = stringifyWaypointsForAPI(transactionDetails.waypoints as WaypointCollection);
     }
 
     const dataToIncludeInParams: Partial<TransactionDetails> = Object.fromEntries(Object.entries(transactionDetails ?? {}).filter(([key]) => key in transactionChanges));
@@ -5749,7 +5747,9 @@
     parentReportNextStep,
 }: UpdateMoneyRequestDistanceParams) {
     const transactionChanges: TransactionChanges = {
-        ...(waypoints && {waypoints: sanitizeRecentWaypoints(waypoints)}),
+        // Don't sanitize waypoints here - keep all fields for Onyx optimistic data (e.g., keyForList)
+        // Sanitization happens when building API params
+        ...(waypoints && {waypoints}),
         routes,
         ...(distance && {distance}),
         ...(odometerStart !== undefined && {odometerStart}),
@@ -6413,7 +6413,7 @@
 
         const isDistanceRequest = isDistanceRequestTransactionUtils(transaction);
         const transactionWaypoints = getWaypoints(transaction);
-        const sanitizedWaypointsForBulk = transactionWaypoints ? JSON.stringify(sanitizeRecentWaypoints(transactionWaypoints)) : undefined;
+        const sanitizedWaypointsForBulk = transactionWaypoints ? stringifyWaypointsForAPI(transactionWaypoints) : undefined;
 
         const convertParams: ConvertTrackedExpenseToRequestParams = {
             payerParams: {
@@ -6697,7 +6697,7 @@
 
     const testDriveCommentReportActionID = isTestDrive ? NumberUtils.rand64() : undefined;
 
-    const sanitizedWaypoints = waypoints ? JSON.stringify(sanitizeRecentWaypoints(waypoints)) : undefined;
+    const sanitizedWaypoints = waypoints ? stringifyWaypointsForAPI(waypoints) : undefined;
 
     // If the report is iou or expense report, we should get the linked chat report to be passed to the getMoneyRequestInformation function
     const isMoneyRequestReport = isMoneyRequestReportReportUtils(report);
@@ -7496,7 +7496,7 @@
 
     // Pass an open receipt so the distance expense will show a map with the route optimistically
     const trackedReceipt = validWaypoints ? {source: ReceiptGeneric as ReceiptSource, state: CONST.IOU.RECEIPT_STATE.OPEN, name: 'receipt-generic.png'} : receipt;
-    const sanitizedWaypoints = validWaypoints ? JSON.stringify(sanitizeRecentWaypoints(validWaypoints)) : undefined;
+    const sanitizedWaypoints = validWaypoints ? stringifyWaypointsForAPI(validWaypoints) : undefined;
 
     const retryParams: CreateTrackExpenseParams = {
         ...params,
@@ -8440,7 +8440,7 @@
 
     let parameters: CreateDistanceRequestParams;
     let onyxData: OnyxData<BuildOnyxDataForMoneyRequestKeys | typeof ONYXKEYS.NVP_LAST_DISTANCE_EXPENSE_TYPE | typeof ONYXKEYS.NVP_RECENT_WAYPOINTS | typeof ONYXKEYS.GPS_DRAFT_DETAILS>;
-    const sanitizedWaypoints = !isManualDistanceRequest ? sanitizeRecentWaypoints(validWaypoints) : null;
+    const sanitizedWaypoints = !isManualDistanceRequest ? sanitizeWaypointsForAPI(validWaypoints) : null;
     if (iouType === CONST.IOU.TYPE.SPLIT) {
         const {
             splitData,

src/libs/actions/Transaction.ts

@@ -347,23 +347,47 @@ function getOnyxDataForRouteRequest(
 }
 
 /**
- * Sanitizes the waypoints by removing the pendingAction property.
+ * Sanitizes the waypoints data to only include allowed fields for API requests.
+ * Only keeps: name (optional), address, lat, lng
  *
  * @param waypoints - The collection of waypoints to sanitize.
- * @returns The sanitized collection of waypoints.
+ * @returns The sanitized collection of waypoints with only allowed fields.
  */
-function sanitizeRecentWaypoints(waypoints: WaypointCollection): WaypointCollection {
+function sanitizeWaypointsForAPI(waypoints: WaypointCollection): WaypointCollection {
     return Object.entries(waypoints).reduce((acc: WaypointCollection, [key, waypoint]) => {
-        if ('pendingAction' in waypoint) {
-            const {pendingAction, ...rest} = waypoint;
-            acc[key] = rest;
-        } else {
-            acc[key] = waypoint;
+        if (!waypoint) {
+            return acc;
+        }
+
+        const sanitizedWaypoint: Record<string, string | number> = {};
+
+        if (waypoint.name !== undefined) {
+            sanitizedWaypoint.name = waypoint.name;
+        }
+        if (waypoint.address !== undefined) {
+            sanitizedWaypoint.address = waypoint.address;
+        }
+        if (waypoint.lat !== undefined) {
+            sanitizedWaypoint.lat = waypoint.lat;
         }
+        if (waypoint.lng !== undefined) {
+            sanitizedWaypoint.lng = waypoint.lng;
+        }
+
+        acc[key] = sanitizedWaypoint;
         return acc;
     }, {});
 }
 
+/**
+ * Sanitizes waypoints and serializes them to a JSON string for API params.
+ * Preserves keyForList and other Onyx-only fields by sanitizing at the serialization boundary
+ * rather than when building transactionChanges.
+ */
+function stringifyWaypointsForAPI(waypoints: WaypointCollection): string {
+    return JSON.stringify(sanitizeWaypointsForAPI(waypoints));
+}
+
 /**
  * Gets the route for a set of waypoints
  * Used so we can generate a map view of the provided waypoints
@@ -372,7 +396,7 @@ function sanitizeRecentWaypoints(waypoints: WaypointCollection): WaypointCollect
 function getRoute(transactionID: string, waypoints: WaypointCollection, routeType: TransactionState = CONST.TRANSACTION.STATE.CURRENT) {
     const parameters: GetRouteParams = {
         transactionID,
-        waypoints: JSON.stringify(sanitizeRecentWaypoints(waypoints)),
+        waypoints: stringifyWaypointsForAPI(waypoints),
     };
 
     let command;
@@ -1682,7 +1706,8 @@ export {
     setReviewDuplicatesKey,
     abandonReviewDuplicateTransactions,
     openDraftDistanceExpense,
-    sanitizeRecentWaypoints,
+    sanitizeWaypointsForAPI,
+    stringifyWaypointsForAPI,
     getLastModifiedExpense,
     revert,
     changeTransactionsReport,

tests/unit/TransactionTest.ts

@@ -3,7 +3,7 @@ import type {OnyxEntry} from 'react-native-onyx';
 import Onyx from 'react-native-onyx';
 import OnyxUtils from 'react-native-onyx/dist/OnyxUtils';
 import useOnyx from '@hooks/useOnyx';
-import {changeTransactionsReport, dismissDuplicateTransactionViolation, markAsCash, saveWaypoint} from '@libs/actions/Transaction';
+import {changeTransactionsReport, dismissDuplicateTransactionViolation, markAsCash, sanitizeWaypointsForAPI, saveWaypoint} from '@libs/actions/Transaction';
 import DateUtils from '@libs/DateUtils';
 import {getAllNonDeletedTransactions} from '@libs/MoneyRequestReportUtils';
 import type {buildOptimisticNextStep} from '@libs/NextStepUtils';
@@ -1101,6 +1101,126 @@ describe('Transaction', () => {
         });
     });
 
+    describe('sanitizeWaypointsForAPI', () => {
+        it('should only include allowed fields (name, address, lat, lng)', () => {
+            // Given waypoints with extra fields that should be stripped out
+            const waypointsWithExtraFields = {
+                waypoint0: {
+                    name: 'Start Location',
+                    address: '123 Main St',
+                    lat: 40.7128,
+                    lng: -74.006,
+                    city: 'New York',
+                    state: 'NY',
+                    zipCode: '10001',
+                    country: 'US',
+                    street: '123 Main St',
+                    street2: 'Apt 4B',
+                    keyForList: 'unique-key-1',
+                    pendingAction: 'add',
+                    extraField: 'should be removed',
+                },
+                waypoint1: {
+                    address: '456 Oak Ave',
+                    lat: 40.7589,
+                    lng: -73.9851,
+                    city: 'New York',
+                    state: 'NY',
+                    zipCode: '10002',
+                    keyForList: 'unique-key-2',
+                    anotherExtraField: 'should also be removed',
+                },
+            };
+
+            // When sanitizing the waypoints
+            // Test intentionally passes extra fields not in WaypointCollection to verify they are stripped
+            // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-argument
+            const sanitizedWaypoints = sanitizeWaypointsForAPI(waypointsWithExtraFields as any);
+
+            // Then only allowed fields should remain
+            expect(sanitizedWaypoints.waypoint0).toEqual({
+                name: 'Start Location',
+                address: '123 Main St',
+                lat: 40.7128,
+                lng: -74.006,
+            });
+
+            // waypoint1 has no name field, so it should not be included
+            expect(sanitizedWaypoints.waypoint1).toEqual({
+                address: '456 Oak Ave',
+                lat: 40.7589,
+                lng: -73.9851,
+            });
+        });
+
+        it('should handle waypoints with only some allowed fields', () => {
+            // Given waypoints with only address
+            const waypointsWithPartialFields = {
+                waypoint0: {
+                    address: 'Partial Address Only',
+                },
+            };
+
+            // When sanitizing the waypoints
+            // Test uses a partial waypoint object to verify sanitization handles missing fields
+            // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-argument
+            const sanitizedWaypoints = sanitizeWaypointsForAPI(waypointsWithPartialFields as any);
+
+            // Then only the address should be present
+            expect(sanitizedWaypoints.waypoint0).toEqual({
+                address: 'Partial Address Only',
+            });
+        });
+
+        it('should handle empty waypoints', () => {
+            // Given empty waypoints
+            const emptyWaypoints = {};
+
+            // When sanitizing the waypoints
+            const sanitizedWaypoints = sanitizeWaypointsForAPI(emptyWaypoints);
+
+            // Then the result should also be empty
+            expect(sanitizedWaypoints).toEqual({});
+        });
+
+        it('should skip null waypoint entries without crashing', () => {
+            // Given waypoints where some entries are null (can happen during rollback of failed distance edits)
+            const waypointsWithNulls = {
+                waypoint0: {
+                    address: '123 Main St',
+                    lat: 40.7128,
+                    lng: -74.006,
+                },
+                waypoint1: null,
+                waypoint2: {
+                    address: '789 Pine Rd',
+                    lat: 40.73,
+                    lng: -73.99,
+                },
+            };
+
+            // When sanitizing the waypoints
+            // Null entries can occur at runtime even though WaypointCollection type doesn't include null
+            // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-argument
+            const sanitizedWaypoints = sanitizeWaypointsForAPI(waypointsWithNulls as any);
+
+            // Then null entries should be dropped and valid entries sanitized
+            expect(sanitizedWaypoints).toEqual({
+                waypoint0: {
+                    address: '123 Main St',
+                    lat: 40.7128,
+                    lng: -74.006,
+                },
+                waypoint2: {
+                    address: '789 Pine Rd',
+                    lat: 40.73,
+                    lng: -73.99,
+                },
+            });
+            expect(sanitizedWaypoints.waypoint1).toBeUndefined();
+        });
+    });
+
     describe('markAsCash', () => {
         it('should optimistically remove RTER violation and add dismissed violation report action', async () => {
             // Given a transaction with an RTER violation