@W-23025048 fix(order-tracking): make carrier tracking links external - #3898
Conversation
|
Git2Gus App is installed but the |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
A carrier tracking URL can arrive without a scheme (e.g. `www.carrier.com/t`). Rendered directly in an `href`, the browser treats it as a relative path and resolves it against the current page, so the tracking-number link navigated inside the app (`/account/orders/www.carrier.com/t`) instead of going to the carrier. Add `ensureExternalUrl` (app/utils/url.js): prepend `https://` to a scheme-less URL, keep already-absolute http(s) URLs, and reject unsafe or non-web values (`javascript:`, `data:`, `mailto:`, app-internal paths, …) by returning `undefined` so the link renders inactive rather than executable or wrong. Apply it to the OrderTracking tracking-number link.
444a0dd to
f351bdf
Compare
sf-jie-dai
left a comment
There was a problem hiding this comment.
Code-review findings (2 substantive). Both verified empirically against the WHATWG URL parser in Node.
| } | ||
|
|
||
| // Require a host-like value so a bare word ("track") isn't externalized. | ||
| const host = sanitized.replace(/^\/+/, '').split(/[/?#]/)[0] |
There was a problem hiding this comment.
Authority-confusion bypass — link reads like the carrier but navigates elsewhere.
The host-like guard approves a string because it contains a ., but the WHATWG parser resolves the actual host to something else. Verified in Node:
ensureExternalUrl('ups.com:@evil.com/track') // -> 'https://ups.com@evil.com/track' (host = evil.com)
ensureExternalUrl('@evil.com/track') // -> 'https://evil.com/track'
Here ups.com: parses as a (dotted) "protocol" → falls through to the prepend path; host extraction yields ups.com:@evil.com, which passes .includes('.'). The produced URL's userinfo is ups.com and the host is evil.com. Since the whole point of this helper is to safely externalize merchant/OMS-sourced tracking URLs, a value that renders as a UPS-looking link but navigates to an attacker host defeats the guard. Consider validating the parsed URL's host/hostname (and rejecting any username/password) rather than the raw substring.
| export const ensureExternalUrl = (input) => { | ||
| if (!input) return undefined | ||
| // eslint-disable-next-line no-control-regex -- strip control chars so they can't smuggle past the scheme checks | ||
| const sanitized = input.replace(/[\x00-\x1f\x7f]/g, '').trim() |
There was a problem hiding this comment.
Non-string input throws, breaking the documented "never throws" contract.
trackingUrl is PropTypes.string (dev-only) and flows from API data (omsShipment?.trackingUrl / shipment.trackingUrl). A malformed response that yields a number crashes the render:
ensureExternalUrl(1234) // TypeError: input.replace is not a function
!1234 is false, so the falsy guard is skipped and .replace throws — the OrderTracking render tree crashes. The old href={trackingUrl} coerced harmlessly. Suggest a typeof input !== 'string' guard (or String(input ?? '')) right after the falsy check.
Address review findings on the carrier-tracking-link normalizer. The function trusted the parsed scheme but never validated the resulting host, so several inputs produced an href that read like the carrier yet navigated elsewhere. - Reject userinfo: `https://www.ups.com@evil.com` parses to host `evil.com` with `www.ups.com` as the username — now rejected on both parse branches. - Reject backslash forms (`\` is treated as `/` by the URL parser) and run the relative-path guard on the raw input so a control char between slashes can't collapse `/x/host` into a protocol-relative `//host`. - Reject non-string input (was throwing on a non-string and crashing the render). - Reject bare filenames / empty-label hosts (e.g. `data.html`, `a..b`) that aren't real external hosts. - Drop the unreachable http branch and the dead IPv4 check. Centralize the checks in `isSafeExternalUrl` (http(s) only, no userinfo, plausible host), applied to both the absolute and prepended candidates. Add regression tests asserting the resolved host for each case.
|
Thanks for the review — both were good catches. Pushed fixes:
Since this is the same helper as the storefront-next side, I also folded in the equivalent hardening flagged there — reject backslash-authority forms, run the relative-path guard on the raw input, reject bare-filename hosts, and drop a dead branch — so both stay in parity. Added regression tests asserting the resolved host for each case. |
| // protocol; a scheme-less `host:port` (`carrier.com:8080`) mis-parses to a | ||
| // dotted protocol — only the former should be validated/rejected as-is. | ||
| const parsed = new URL(sanitized) | ||
| if (!parsed.protocol.replace(/:$/, '').includes('.')) { |
There was a problem hiding this comment.
Residual host-confusion bypass — same class as the userinfo spoof, via the dotted-protocol fallthrough.
The intent here is "a scheme-less host:port (carrier.com:8080) mis-parses to a dotted protocol → fall through and prepend https://." But an input shaped label.tld://… also parses to a dotted protocol, so it skips isSafeExternalUrl and falls through to be re-prepended — smuggling the fake scheme in as the real host. Verified against Node's WHATWG URL:
ensureExternalUrl('attacker.com://ups.com/track/12345')
// -> 'https://attacker.com//ups.com/track/12345' (host = attacker.com)
ensureExternalUrl('foo.bar://evil.com')
// -> 'https://foo.bar//evil.com' (host = foo.bar)
The href navigates to attacker.com while the string reads like ups.com — exactly the "reads like the carrier, navigates elsewhere" defect this helper documents itself as rejecting (the suite already asserts https://www.ups.com@evil.com → undefined; this is the same outcome through a different door).
Severity is moderate, not critical: the rendered link text is the tracking number, not the URL, and label.tld://… isn't a shape any real carrier URL takes. But it's an open hole in a function whose sole job is host-confusion-safe externalization.
The discriminator between the legit and malicious cases is the // authority: a scheme-less host:port has no // after the colon, whereas attacker.com://… does. So reject when the first parse yields a dotted protocol that carries an authority, and only treat the no-// host:port form as scheme-less. (Note: simply "always run isSafeExternalUrl on the first parse" would wrongly reject carrier.com:8080, since its protocol isn't http/https.)
There was a problem hiding this comment.
Good catch — fixed in d80fc161f1 with the //-authority discriminator you suggested. After the first parse, a dotted "protocol" that carries an authority is now rejected, while a genuine scheme-less host:port (empty host on that parse) still falls through and resolves:
const parsed = new URL(sanitized)
if (!parsed.protocol.replace(/:$/, '').includes('.')) {
return isSafeExternalUrl(parsed) ? parsed.toString() : undefined
}
// Dotted protocol WITH an authority (`attacker.com://ups.com/t`) is a host-confusion spoof.
if (parsed.host) return undefinedAdded regression tests: attacker.com://ups.com/track/12345 and foo.bar://evil.com → undefined; carrier.com:8080 (and /track) still → https://carrier.com:8080/... so the legit host:port form isn't over-rejected. Ready for another look.
sf-jie-dai
left a comment
There was a problem hiding this comment.
Re-review: both findings from the previous pass are fixed — verified empirically against Node's WHATWG URL parser.
- Userinfo spoof (
https://www.ups.com@evil.com,ups.com:@evil.com) → nowundefined(isSafeExternalUrlrejectsusername/password). ✅ - Non-string crash (
1234,{},null) → nowundefined, never throws (thetypeof input !== 'string'guard). ✅
One residual issue in the same host-confusion class, so holding approval — see the inline comment.
A scheme-less value shaped `label.tld://host` parses to a dotted protocol, so it skipped the dot-less validation and fell through to be re-prepended into `https://label.tld//host` — a link that reads like one host but navigates to another (same class as the userinfo spoof). Reject a dotted-protocol parse that carries an authority; a genuine `host:port` has an empty host on that parse and still resolves. Review feedback.
sf-jie-dai
left a comment
There was a problem hiding this comment.
Approving — reviewed the ensureExternalUrl normalizer and its order-tracking usage at high effort. The security spoof vectors are genuinely closed and tested (userinfo @, backslash authority, dotted-protocol label.tld://host, javascript:/data:), and since the result is a client-side href opened with rel=noopener noreferrer, the host-confusion/decimal-IP cases don't apply here. No blocking issues.
Non-blocking notes for a follow-up (optional):
FILENAME_HOSTis an enumerated file-extension blocklist run againsturl.hostname— fragile and largely redundant;isSafeExternalUrl(http(s) + no userinfo + dotted multi-label host) already covers the intent. Consider dropping it.- Slight over-strictness: trailing-dot FQDN (
https://tracking.ups.com.) and IPv6 literals are rejected. Low real-world risk for retail carriers. - The
if (parsed.host) return undefinedguard is load-bearing on a WHATWG quirk (scheme-lesshost:port→ empty host). Correct today; worth keeping the regression tests that pin it. - The two try/catch parse branches duplicate the same validate-and-return tail.
…to t/cc-sharks/W-23091033/tracking-url-external/main
e546e0d
into
feature/264-order-management
* @W-22931003 Add cancelOmsOrder and getOmsMetaData hooks to commerce-sdk-react (#3864) * @W-22931003@ Add cancelOmsOrder and getOmsMetaData hooks to commerce-sdk-react - Add CancelOmsOrder to ShopperOrdersMutations enum (mutation.ts) - Add useOmsMetaData query hook for fetching cancel/return reason codes (query.ts) - Add getOmsMetaData query key helper (queryKeyHelpers.ts) - Add cancelOmsOrder cache invalidation: invalidates order query on success (cache.ts) - Update query test map with useOmsMetaData - Add returnOmsOrder to unimplemented endpoints list (available in SDK but no hook yet) Requires commerce-sdk-isomorphic with shopper-orders-oas v1.14.1 (branch: update-shopper-orders-1.14.1, not yet published to npm) * Add changelog entry for cancelOmsOrder and getOmsMetaData hooks * Address PR review feedback - Invalidate getCustomerOrders cache on cancel (order history shows updated status) - Add comments for unimplemented endpoints in test file - Bump commerce-sdk-isomorphic to 5.1.0-unstable-20260611093359 (preview with OAS v1.14.1) * Update package-lock.json for commerce-sdk-isomorphic unstable version * Fix prettier: inline single-element array in cancelOmsOrder cache * Fix tests for unstable SDK: remove endpoints no longer in preview version - Remove setupCustomerPaymentMethodReference from ShopperCustomers unimplemented list - Remove resolveQualifiers from ShopperExperience unimplemented list * Remove changelog entry (will add when merging to develop) * Add unit tests for cancelOmsOrder mutation and cache invalidation * @W-22821835@ Add returnOmsOrder mutation to commerce-sdk-react (#3869) Add returnOmsOrder mutation * @W-22918057 Extract standalone OrderTracking component (#3865) @W-22918057 Extract standalone OrderTracking component Pure refactor: move the inline tracking render from renderShippingMethod in order-detail.jsx into a standalone, testable app/components/order-tracking component, wired at both call sites (delivery/OMS-fallback and OMS-multi). The OMS-multi call site no longer needs a wrapping React.Fragment (key moves onto OrderTracking), and the now-unused ChakraLink import is removed from order-detail.jsx. DOM output is identical and i18n message IDs are unchanged (shipping-status descriptors are kept as inline formatMessage literals so babel-plugin-formatjs still statically extracts them). The existing orders.test.js suite plus new isolated component tests pass (96/96); lint clean. First of the Order Tracking epic work items. * @W-22918455 Render estimated delivery date on order tracking (#3867) @W-22918455 Render estimated delivery date on order tracking Render expectedDeliveryDate and actualDeliveryDate on the OrderTracking component as locale-formatted "Expected delivery: <date>" / "Delivered: <date>" lines, shown when present and omitted when absent (or when the date is missing or malformed — formatTrackingDate returns null on an unparseable value so it never renders "Invalid Date" or throws). Wired at both order-detail call sites; these fields are OMS-only (no ECOM fallback — they exist only on omsData.shipments[]). Adds two i18n keys (extracted + compiled). Unit and page tests cover the present, absent, and malformed cases across single, multi-shipment, and ECOM-fallback paths. * @W-22918457 Guard null delivery dates in OrderTracking (#3872) * @W-22918457 Guard null delivery dates in OrderTracking QA on W-22918455 found that new Date(null) returns the epoch (1970-01-01), not Invalid Date, so a null expectedDeliveryDate/actualDeliveryDate slipped past the isNaN guard and rendered '31 Dec 1969' to the shopper. Add a falsy guard (catches null, undefined, '') to formatTrackingDate, with a regression test for the explicit-null case. Defensive regardless of whether OMS serializes null on the wire. * @W-22918457 Re-trigger CI (flaky e2e SLAS login) * @W-22918462 Add error/fallback state to order-detail fetch (#3871) Consume useOrder's isError and render a full-card error (OrderNotFoundCard-style: square corners, title + description + Back to Order History) instead of hanging on the loading skeleton forever (the AC6 bug). A successful order with no omsData is NOT an error — it renders normally via the ECOM fallback. Adds the OrderLoadError component, i18n keys, and unit tests for the error, back-link, and no-omsData paths. Partial (products-only) failure keeps graceful degradation per the team's decision. * @W-22952388@ Sync payment-instrument amount before createOrder so OMS can ingest(#3875) @W-22952388@ Sync payment-instrument amount before createOrder so OMS can ingest * fix: invalidate `useCustomerOrders` after `createOrder` @W-22821836 (#3878) * @W-22821836@ Invalidate customer orders cache after createOrder After a registered shopper places an order, useCustomerOrders kept serving the stale (pre-order) result and only refreshed on a fresh login, because createOrder's cache-update matrix only invalidated getCustomerBaskets. Add getCustomerOrders to the same invalidate list so the account order-history page reflects the new order immediately. Guest checkouts (no customerId) remain a no-op. * @W-22806925@ Integrate SCAPI Cancel Order API with UI (#3873) * @W-22806925@ Integrate SCAPI Cancel Order API with UI - Replace dummy cancel handler with real useShopperOrdersMutation(CancelOmsOrder) - Fetch reason codes from useOmsMetaData API when modal opens - Update cancel eligibility: omsData must exist + quantityAvailableToCancel === quantityOrdered for all items - Remove oms.enabled config flag — cancel visibility driven by order.omsData presence - Remove hardcoded CANCELLATION_REASONS — dropdown populated from API response - Hide dropdown when no reason codes available (allows cancel without reason) - Pass isSubmitting from mutation state to disable modal buttons during request - Show skeleton while reason codes are loading - Remove extracted translation messages for hardcoded reasons - Add mock for useShopperOrdersMutation/useOmsMetaData in order detail tests * Fix: keep modal open during API call, close only after response * Fix CI: prettier formatting and add changelog entry * Revert changelog entry — will add when merging to develop * Fix prettier: correct indentation for map, revert formatMessage to multiline * Pre-fetch OMS metadata on page load instead of on modal open * Fix prettier: inline useOmsMetaData and formatMessage calls * Fix prettier conflict: extract formatMessage to variables (under 100 char width) * Fix prettier: match showCancelSuccess format with showCancelError * Address Jie's review: initialize default reason and add no-reason test - Pre-select the default reason code when modal opens (from API response) - Add test: passes empty string when no reason codes provided and confirm clicked - Update existing tests to expect default reason pre-selection * Fix lint and tighten canCancel check - Add item.omsData != null guard to prevent undefined === undefined passing - Inline formatMessage in showCancelSuccess (satisfies generated-project prettier) - Inline test props (satisfies generated-project prettier) * Fix prettier: revert showCancelSuccess formatMessage to multiline (matches showCancelError) * feat: surface OMS return eligibility on order detail @W-22821836 (#3880) feat: surface OMS return eligibility on order detail * @W-22930993 Handle cancel order API error scenarios with specific messages (#3884) * @W-22930993@ Handle cancel order API error scenarios with specific messages - 404: "Unable to cancel order" / "We could not find this order. Please refresh and try again." - 409: "Unable to cancel order" / "This order is already being processed and cannot be canceled. Please reach out to the Merchant." - Everything else (400, 401, 500, network): "Something went wrong" / "We couldn't process your cancellation right now. Please wait a moment and try again." Error status is read from error.response.status (commerce-sdk-isomorphic ResponseError shape). Cancel button remains enabled on error to allow retry. * Fix prettier: inline defaultMessage values * Address review: disable button on terminal errors, fix copy - Disable cancel button after 404/409 (terminal — retrying won't help) - Keep button enabled for transient errors (500/network) to allow retry - Fix spelling: "canceled" → "cancelled", "Merchant" → "merchant" - Reword 409 copy: "Please contact the merchant for assistance." * Address review: revert copy change, fix spelling only, add error tests - Revert message rewording — keep original copy, only fix "canceled" → "cancelled" and "Merchant" → "merchant" - Add unit tests for cancel error scenarios (404, 409, 500, network failure) - Tests verify correct title and description for each error code * Fix lint: split long defaultMessage lines, multiline isDisabled, remove unused act * @W-22930989 Test and polish cancel modal behavior when OMS metadata is unavailable (#3883) * @W-22930989@ Hide cancellation reason dropdown when OMS metadata API fails The dropdown is already hidden when reasonCodes is undefined (metadata fetch failed). Add retry: 1 to the useOmsMetaData query so React Query automatically retries once before giving up. If both attempts fail, the modal renders without the dropdown and the shopper can still confirm cancellation — the server applies the default reason. * Add unit tests for metadata API failure scenarios * Address review: remove retry override, conditional modal text - Remove retry: 1 — let TanStack default (3) handle resilience - Show "Confirm cancellation below." when dropdown is hidden (no reason codes) - Show "Select a reason and confirm cancellation." when dropdown is visible - Update test for conditional text * feat: return-item-selection modal on order detail @W-22821837 (#3886) feat: return-item-selection modal on order detail * feat: add return review step + returnOmsOrder submission @W-22821838 (#3895) feat: add return review step + returnOmsOrder submission * @W-22806929 Add comprehensive cancel order integration tests (#3896) * @W-22806929@ Add comprehensive cancel order integration tests 12 tests covering: - Eligibility: OMS order shows cancel, non-OMS hides it, partially shipped disables, already-cancelled disables, wrong customer disables - Happy path: modal opens, API called with correct payload, success feedback - Reason selection: submits selected reason to API - Error states: button disabled after 409 (terminal), enabled after 500 (retry) - Modal UX: keep order closes without API call, no-reason-codes shows alternative text * Address review: fix vacuous tests and lint errors - Fix "cancel submits with selected reason": rewrite as "cancel submits empty body when no reason codes available" — tests the actual contract (cancelReasonCodes: [] means no dropdown, empty body sent to API) - Fix "does not show cancel button for different customer": rename to "disables cancel button", assert unconditionally (button IS rendered but disabled for OMS orders with wrong customerId) - Rename "quantityAvailableToCancel < quantityOrdered" to "not fully cancellable" — clearer wording matching the strict-equality contract - Fix prettier: multiline mock resolved values - Remove conditional expects (jest/no-conditional-expect lint rule) * chore: merge develop into feature/264-order-management (#3901) merge develop into feature/264-order-management * feat: order-level status matrix util from item omsData @W-23163246 (#3900) order-level status matrix util from item omsData * feat: item-level return error states @W-22821839 (#3897) item-level return error states * @W-23093717 wire getOrderDisplayStatus for cancelled status into order badges (#3902) * feat: wire getOrderDisplayStatus into order badges @W-23093717 Use item-level status derivation to show Cancelled badge (red with × icon) on both order-detail and order-history pages when all items are cancelled. Non-cancelled orders retain the existing raw status display. * Address review: hoist isCancelled out of IIFEs, add negative tests - Hoist `isCancelled` derivation above the JSX return in order-detail and to the top of the .map() callback in order-history, removing the inline IIFEs. - Add negative tests asserting a partially-cancelled order does NOT render the "Cancelled" badge (regression guard). * Address review: memoize isCancelled, add OMS-only scope comment - Wrap isCancelled in useMemo on order-detail to avoid recomputing on every render (matches canCancel/returnableItems pattern). - Add comment clarifying the derivation is OMS-only; pure ECOM cancellations fall through to the raw status string in the badge. - Fix broken import from merge (return-error-utils missing `import {`). * Extract shared OrderStatusBadge component De-duplicate the cancelled badge JSX from order-detail and order-history into a reusable component with memoized status derivation. Unifies the message ID (order_status_badge.label.cancelled) so translators only localize the string once. Prepares a clean extension point for follow-up status WIs. --------- Signed-off-by: sf-madhuri-uppu <madhuri.uppu@salesforce.com> * feat: show return status in order status badge @W-23163243 (#3904) feat: show return status in order status badge * @W-23025048 fix(order-tracking): make carrier tracking links external (#3898) * fix(order-tracking): make carrier tracking links external A carrier tracking URL can arrive without a scheme (e.g. `www.carrier.com/t`). Rendered directly in an `href`, the browser treats it as a relative path and resolves it against the current page, so the tracking-number link navigated inside the app (`/account/orders/www.carrier.com/t`) instead of going to the carrier. Add `ensureExternalUrl` (app/utils/url.js): prepend `https://` to a scheme-less URL, keep already-absolute http(s) URLs, and reject unsafe or non-web values (`javascript:`, `data:`, `mailto:`, app-internal paths, …) by returning `undefined` so the link renders inactive rather than executable or wrong. Apply it to the OrderTracking tracking-number link. * chore: trigger CI re-run * harden ensureExternalUrl per review feedback Address review findings on the carrier-tracking-link normalizer. The function trusted the parsed scheme but never validated the resulting host, so several inputs produced an href that read like the carrier yet navigated elsewhere. - Reject userinfo: `https://www.ups.com@evil.com` parses to host `evil.com` with `www.ups.com` as the username — now rejected on both parse branches. - Reject backslash forms (`\` is treated as `/` by the URL parser) and run the relative-path guard on the raw input so a control char between slashes can't collapse `/x/host` into a protocol-relative `//host`. - Reject non-string input (was throwing on a non-string and crashing the render). - Reject bare filenames / empty-label hosts (e.g. `data.html`, `a..b`) that aren't real external hosts. - Drop the unreachable http branch and the dead IPv4 check. Centralize the checks in `isSafeExternalUrl` (http(s) only, no userinfo, plausible host), applied to both the absolute and prepended candidates. Add regression tests asserting the resolved host for each case. * harden ensureExternalUrl against dotted-protocol host confusion A scheme-less value shaped `label.tld://host` parses to a dotted protocol, so it skipped the dot-less validation and fell through to be re-prepended into `https://label.tld//host` — a link that reads like one host but navigates to another (same class as the userinfo spoof). Reject a dotted-protocol parse that carries an authority; a genuine `host:port` has an empty host on that parse and still resolves. Review feedback. * feat: order-detail tracking cards + Track Shipment action @W-23091033 (#3906) * feat: add Track Shipment button to order detail @W-23091033 Render a "Track Shipment" order action per OMS shipment that has a tracking URL — each button opens its own shipment's carrier URL in a new tab (same destination as the tracking-number link), with no numeric suffix. When no shipment has a tracking URL, a single disabled button is shown. The Order Actions row is now full-width stacked on mobile and inline from the sm breakpoint up, so additional buttons no longer overflow on narrow screens. * feat: per-shipment tracking cards + single track action @W-23091033 Align the order-detail tracking UI with storefront-next and the order tracking epic's "flat list, no address association" scope: - Render tracking as a flat list of per-shipment cards (one card per omsData.shipments[] entry, ECOM order.shipments[] fallback), each with carrier, status, tracking-number link, and delivery dates. No address on the cards and no positional OMS-to-ECOM index join. - Consolidate the Track Shipment order action to a single button that links to the first shipment with a carrier URL (disabled when none), matching storefront-next getTrackShipmentHref. - Move the shipping address to a single order-level block shown only for single-shipment delivery orders; multi-shipment orders no longer associate an address per shipment. Updates the order detail tests and extracts the new Tracking heading message. * feat: per-shipment item boxes on order detail @W-23091033 Restructure the Items Ordered area to mirror storefront-next: render one bordered box per ECOM delivery shipment, each with a "Shipment N" header, that shipment's status, its items (grouped strictly by shipmentId; all items when there's a single delivery shipment), and its own native shipping address and method. Pickup-only orders (no delivery shipment) fall back to a flat item list. The top summary card no longer carries a standalone shipping-address block — the address now lives inside each shipment box. The Tracking cards stay in a separate flat section (OMS-preferred, ECOM fallback) and carry no address: there is no reliable join key between an OMS shipment and a specific ECOM shipment, so a per-shipment box never index-joins OMS data (the deferred TD). Reconcile the multi-shipment / BOPIS tests to the new per-box behavior: they previously asserted addresses were hidden and only passed because the products endpoint was unmocked (which suppressed the box render); they now mock products and assert each shipment's own address renders in its box, plus strict item-to-box placement by shipmentId. * feat: move tracking inside the first shipment box @W-23091033 Match the storefront-next design branch: the flat tracking list now renders INSIDE the first shipment box, after its items and before its shipping address, instead of as a standalone section above the items. It is still the whole flat list (rendered in box 1 only, never split per shipment), so there is no positional OMS↔ECOM index-join. Pickup-only orders (no delivery box) append the flat tracking list after the flat item list so tracking is never lost. Update the regression-lock test: it previously asserted tracking was a separate section; it now asserts tracking sits inside the first shipment box with the shipping address as a sibling block (no address inside a tracking card). * feat: tracking as flat section below shipment boxes @W-23091033 OMS tracking (omsData.shipments[]) has no join key back to a specific ECOM shipment (deferred TD-0326366), so the layout must not imply which tracking belongs to which shipment. Move the flat tracking list out of the first shipment box and render it as a single section BELOW all the boxes — a peer of the boxes, not nested in one. Each shipment box still shows its own products (grouped by ECOM shipmentId) and its own native address, both of which ARE known; only the tracking↔shipment link is unknown, and the flat list is honest about that. One code path covers single, multi, and pickup-only orders. Update the regression-lock test: it now asserts tracking renders OUTSIDE (not nested in) the shipment boxes, with no address/payment text in the section. * feat: Track Shipment dropdown for multi-shipment orders @W-23091033 UX option A for multi-shipment tracking (one of two branches for review). When an order has more than one shipment with a carrier tracking URL, the Track Shipment order action becomes a dropdown (Popover — the shared UI barrel exposes no Chakra Menu) listing each tracking number as an external link, so the shopper can pick which carrier link to open. Single-shipment orders keep the simple external-link button; orders with no tracking URL keep the single disabled button. This is interim: we still cannot say which tracking maps to which shipment (deferred TD-0326366), so options are labeled by tracking number, not by shipment contents. * chore(order-detail): self-contained rationale in tracking comments Rewrite the order-detail and test comments to explain the multi-shipment tracking behavior in project-internal terms (OMS and ECOM shipment arrays share no join key, so a tracking entry can't be tied to a specific shipment) instead of referencing tracking artifacts or another product. Customer-facing code should not carry process/cross-product references; the rationale lives in the spec and PR instead. * fix(order-detail): address review findings on tracking layout @W-23091033 - Surface items that match no shipment box (untagged, or a shipmentId naming no delivery shipment) in an "Other items" box, so multi-shipment orders never silently drop purchased items while the item-count header still counts them. - Localize the per-shipment box status pill via a shared formatter, so the box header and the tracking card render the same localized status instead of one showing a raw snake_case token in non-English locales. - Make the disabled Track Shipment button use aria-disabled + a hidden disabled-reason hint (keyboard/SR focusable), matching the Cancel/Return buttons in the same row, instead of a native disabled button. * fix(order-detail): meet AA contrast on shipment status pill @W-23091033 The "Shipment N" status pill rendered gray.700 text on gray.200 (4.04:1), which fails WCAG AA (4.5:1) and tripped the registered happy-path a11y snapshot (the color-contrast violation captured on the order detail page). Darken the text to gray.800 on the same gray.200 background for 5.88:1. * docs: order-management feature README + strip internal GUS refs @W-22821845 (#3907) order-management feature README + strip internal GUS refs * @W-22821845 fix(order-return): return focus to the trigger when the return modal closes (#3910) return focus to the trigger when the return modal closes * fix(oms-ui): apply CX UI text guidelines to order & account UI (#3912) apply CX UI text guidelines to order & account UI * docs(commerce-sdk-react): changelog for OMS order-action hooks The cancelOmsOrder/returnOmsOrder mutations and getOmsMetaData hook (#3864, #3869) landed without a commerce-sdk-react CHANGELOG entry, so the integration PR failed the per-package changelog check. Add it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(commerce-sdk-react): pin commerce-sdk-isomorphic to stable 5.4.0 Replace the temporary 5.1.0-unstable-* dev build with stable 5.4.0, the first release carrying the OMS Shopper Orders endpoints (oms-return-order, oms-cancel-order, oms-meta-data). develop's 5.2.0 has no OMS endpoints, so this is required for the OMS hooks to work rather than a plain bump. Build + tsc clean; ShopperOrders hooks 33/33 pass against 5.4.0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(commerce-sdk-react): list new 5.4.0 endpoints as unimplemented The isomorphic 5.2.0 -> 5.4.0 bump adds endpoints with no React hooks yet, so the 'all endpoints have hooks' guard tests failed. Add each new endpoint (requestOtp/verifyOtp, setupCustomerPaymentMethodReference, promoteTemporaryBasket, getComponent/resolveQualifiers, getProduct Images/Prices/Promotions) to the expected unimplemented list with a TODO, matching the test's documented convention. Implementing the hooks is out of scope for the OMS integration. Full commerce-sdk-react suite: 778/778 pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(oms-ui): bump vendor.js bundle-size budget to 398 kB The commerce-sdk-isomorphic 5.2.0 -> 5.4.0 upgrade pushes vendor.js to 397.07KB, just over the 397 kB budget. Bump to 398 kB (main.js unchanged). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: address PR #3914 follow-up review comments @W-23254713 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(oms-ui): apply CX UI text guidelines to cancel order modal @W-23254713 CX UI text guideline caps for buttons Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Signed-off-by: sf-madhuri-uppu <madhuri.uppu@salesforce.com> Co-authored-by: sf-madhuri-uppu <madhuri.uppu@salesforce.com> Co-authored-by: sf-shikhar-prasoon <214730309+sf-shikhar-prasoon@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
What
A carrier tracking URL can arrive without a scheme (e.g.
www.carrier.com/track). Rendered directly in anhref, the browser treats it as a relative path and resolves it against the current page — so the tracking-number link on Order Details navigated inside the app (/account/orders/www.carrier.com/track) instead of going to the carrier.This adds
ensureExternalUrland applies it where the URL becomes an href.Changes
app/utils/url.js— newensureExternalUrl(input):https://to a scheme-less URL (www.carrier.com→https://www.carrier.com/)http(s)URLs (normalized by the nativeURLparser)//host,host:port, and IPv4 hostsjavascript:,data:,vbscript:,mailto:,tel:, app-internal/relative paths) by returningundefinedso the caller renders an inactive link rather than an executable or wrong hrefapp/components/order-tracking/index.jsx— the tracking-numberChakraLinkuses the normalized href; falls back to plain text when there's no safe external URL.Testing / Self-QA
ensureExternalUrlunit tests (url.test.js): scheme-less, already-absolute, protocol-relative,host:port, IPv4, dangerous/non-web schemes, internal/relative paths, control-char stripping, empty/nullish.order-tracking/index.test.js): a scheme-lesstrackingUrlresolves to an absolute external href (regression lock for the relative-path bug).url.test.js+order-tracking/index.test.js— 139 tests). Lint clean on the touched files.https://...) instead of an internal route.