INTER-2499 fix: url encode path parameters

.changeset/encode-path-parameters.md

@@ -0,0 +1,5 @@
+---
+'@fingerprint/node-sdk': patch
+---
+
+URL-encode path parameters; reject "." and ".." path parameters with a TypeError.

src/urlUtils.ts

@@ -43,6 +43,39 @@ function serializeQueryStringParams(params: QueryStringParameters): string {
   return urlSearchParams.toString()
 }
 
+/**
+ * Confines a value to a single URL path segment. `.` is deliberately left unencoded because
+ * the Server API does not decode path parameters, and valid parameter values can contain dots.
+ *
+ * A value of `.` or `..` is rejected: `new URL()` drops such a segment even when the dots are
+ * encoded, so it cannot be expressed. See https://url.spec.whatwg.org/#double-dot-path-segment
+ */
+function encodePathParam(placeholder: string, value: unknown): string {
+  // Coerce before comparing, because an untyped caller can pass something that is not a string
+  // but stringifies to one. Both conversions throw on values only such a caller could pass:
+  // `String` when the value has no primitive representation, `encodeURIComponent` on a lone
+  // surrogate. Neither should escape as its own error type.
+  let param: string
+  let encoded: string
+  try {
+    // eslint-disable-next-line @typescript-eslint/no-base-to-string -- runtime validation
+    param = String(value ?? '')
+    encoded = encodeURIComponent(param)
+  } catch (cause) {
+    throw new TypeError(`Invalid path parameter for ${placeholder}`, { cause })
+  }
+
+  if (param === '') {
+    throw new TypeError(`Missing path parameter for ${placeholder}`)
+  }
+
+  if (param === '.' || param === '..') {
+    throw new TypeError(`Invalid path parameter for ${placeholder}: ${param}`)
+  }
+
+  return encoded
+}
+
 function getServerApiUrl(region: Region): string {
   switch (region) {
     case Region.EU:
@@ -67,7 +100,7 @@ export interface GetRequestPathOptions {
 }
 
 /**
- * Formats a URL for the FingerprintJS server API by replacing placeholders and
+ * Formats a URL for the Fingerprint Server API by replacing placeholders and
  * appending query string parameters.
  *
  * @internal
@@ -92,19 +125,12 @@ export function getRequestPath({
   // eslint-disable-next-line @typescript-eslint/no-unused-vars
   method: _,
 }: GetRequestPathOptions): string {
-  // Step 1: Extract the path parameters (placeholders) from the path
-  const placeholders = Array.from(path.matchAll(/{(.*?)}/g)).map((match) => match[1])
-
-  // Step 2: Replace the placeholders with provided pathParams
-  let formattedPath: string = `${apiVersion}${path}`
-  placeholders.forEach((placeholder, index) => {
-    const param = pathParams?.[index]
-    if (param !== undefined && param !== '') {
-      formattedPath = formattedPath.replace(`{${placeholder}}`, param)
-    } else {
-      throw new Error(`Missing path parameter for ${placeholder}`)
-    }
-  })
+  // Replace each `{placeholder}` with its path parameter. The replacement is a function so
+  // that `$&` and friends in a parameter are not read as replacement patterns.
+  let index = 0
+  const formattedPath = `${apiVersion}${path}`.replace(/{(.*?)}/g, (_, placeholder: string) =>
+    encodePathParam(placeholder, pathParams?.[index++])
+  )
 
   const queryStringParameters: QueryStringParameters = {
     ...(queryParams ?? {}),
@@ -115,5 +141,9 @@ export function getRequestPath({
   url.pathname = formattedPath
   url.search = serializeQueryStringParams(queryStringParameters)
 
+  if (url.pathname !== `/${formattedPath}`) {
+    throw new TypeError('Invalid path: path changed during normalization')
+  }
+
   return url.toString()
 }

tests/mocked-responses-tests/pathParamEncodingTests.spec.ts

@@ -0,0 +1,72 @@
+import { FingerprintServerApiClient, Region } from '../../src'
+import { getIntegrationInfo } from '../../src/urlUtils'
+import { describe, expect, it } from 'vitest'
+import { mockFetch } from './mockFetch'
+
+/**
+ * A path parameter must never be able to change which endpoint or which host the SDK talks
+ * to, no matter what the caller passes in.
+ */
+describe('[Mocked response] Path parameter encoding', () => {
+  const apiKey = 'dummy_api_key'
+  const ii = `ii=${encodeURIComponent(getIntegrationInfo())}`
+
+  const client = new FingerprintServerApiClient({ region: Region.EU, apiKey })
+
+  const emptyResponse = () => new Response(undefined, { headers: { 'content-length': '0' } })
+
+  const operations = [
+    {
+      name: 'getEvent',
+      prefix: 'v4/events',
+      placeholder: 'event_id',
+      paramName: 'eventId',
+      call: (param: string) => client.getEvent(param),
+    },
+    {
+      name: 'updateEvent',
+      prefix: 'v4/events',
+      placeholder: 'event_id',
+      paramName: 'eventId',
+      call: (param: string) => client.updateEvent(param, { suspect: true }),
+    },
+    {
+      name: 'deleteVisitorData',
+      prefix: 'v4/visitors',
+      placeholder: 'visitor_id',
+      paramName: 'visitorId',
+      call: (param: string) => client.deleteVisitorData(param),
+    },
+  ] as const
+
+  describe.each(operations)('$name', ({ prefix, placeholder, paramName, call }) => {
+    // The full encoding table lives in the unit tests; these are the cases INTER-2499 asks to
+    // be pinned at the wire level for every method.
+    it.each([
+      ['../events', '..%2Fevents'],
+      ['evil.com', 'evil.com'],
+      ['//evil.com', '%2F%2Fevil.com'],
+    ])('requests a single path segment for %j', async (param, encoded) => {
+      mockFetch.mockReturnValue(Promise.resolve(emptyResponse()))
+
+      // `getEvent` rejects on the empty body; only the requested URL matters here.
+      await call(param).catch(() => undefined)
+
+      const requestedUrl = mockFetch.mock.calls[0]?.[0] as string
+      expect(requestedUrl).toEqual(`https://eu.api.fpjs.io/${prefix}/${encoded}?${ii}`)
+      expect(new URL(requestedUrl).host).toEqual('eu.api.fpjs.io')
+    })
+
+    it.each(['.', '..'])('does not send a request for %j', async (param) => {
+      await expect(call(param)).rejects.toThrow(new TypeError(`Invalid path parameter for ${placeholder}: ${param}`))
+
+      expect(mockFetch).not.toHaveBeenCalled()
+    })
+
+    it('does not send a request for an empty parameter', async () => {
+      await expect(call('')).rejects.toThrow(new TypeError(`${paramName} is not set`))
+
+      expect(mockFetch).not.toHaveBeenCalled()
+    })
+  })
+})

tests/unit-tests/urlUtilsTests.spec.ts

@@ -1,6 +1,7 @@
 import { describe, expect, it } from 'vitest'
 import { Region, SearchEventsFilter } from '../../src'
 import { version } from '../../package.json'
+import type { paths } from '../../src/generatedApiTypes'
 import { getRequestPath } from '../../src/urlUtils'
 
 const visitorId = 'TaDnMBz9XCpZNuSzFUqP'
@@ -184,6 +185,16 @@ describe('getRequestPath', () => {
     }).toThrow('Missing path parameter for event_id')
   })
 
+  it('disallows normalized path segments', () => {
+    expect(() => {
+      getRequestPath({
+        path: '/visitors/../events' as keyof paths,
+        method: 'get',
+        pathParams: [],
+      })
+    }).toThrow('Invalid path: path changed during normalization')
+  })
+
   it('encodes special characters', () => {
     const actual = getRequestPath({
       path: '/events',
@@ -197,3 +208,86 @@ describe('getRequestPath', () => {
     expect(actual).toEqual(expected)
   })
 })
+
+// Encoding does not depend on which parameter is being replaced, so these run against one
+// path. That every operation routes through it is covered by the mocked-response tests.
+describe('path parameter encoding', () => {
+  const eventPath = (param: unknown) =>
+    getRequestPath({
+      path: '/events/{event_id}',
+      method: 'get',
+      pathParams: [param] as string[],
+      region: Region.Global,
+    })
+
+  it.each([
+    ['../events', '..%2Fevents'],
+    ['../', '..%2F'],
+    ['/../../events', '%2F..%2F..%2Fevents'],
+    ['evil.com', 'evil.com'],
+    ['//evil.com', '%2F%2Fevil.com'],
+    ['https://evil.com', 'https%3A%2F%2Fevil.com'],
+    ['a b#c?d', 'a%20b%23c%3Fd'],
+    ['%2e%2e', '%252e%252e'],
+    ['..%2fevents', '..%252fevents'],
+    ['..\\..', '..%5C..'],
+    ['$&', '%24%26'],
+    ['...', '...'],
+    // A placeholder in a parameter must not reach the next replacement, which would matter
+    // for a path with two placeholders.
+    ['{event_id}', '%7Bevent_id%7D'],
+    ['1626550679751.cVc5Pm', '1626550679751.cVc5Pm'],
+    // An untyped caller can pass something that is not a string but stringifies to one.
+    [new String('../events'), '..%2Fevents'],
+  ])('keeps %j inside a single path segment', (param, encoded) => {
+    expect(eventPath(param)).toEqual(`https://api.fpjs.io/v4/events/${encoded}?${ii}`)
+  })
+
+  it.each([
+    ['.', '.', 'Invalid path parameter for event_id: .'],
+    ['..', '..', 'Invalid path parameter for event_id: ..'],
+    ['a String object', new String('..'), 'Invalid path parameter for event_id: ..'],
+    ['an object with a toString', { toString: () => '..' }, 'Invalid path parameter for event_id: ..'],
+    ['an array', ['..'], 'Invalid path parameter for event_id: ..'],
+    // A lone surrogate makes `encodeURIComponent` throw a `URIError`
+    ['a lone surrogate', '\ud800', 'Invalid path parameter for event_id'],
+    // These have no primitive representation, so `String` itself throws
+    ['an object without a prototype', Object.create(null), 'Invalid path parameter for event_id'],
+    [
+      'an object whose toString throws',
+      {
+        toString: () => {
+          throw new Error('boom')
+        },
+      },
+      'Invalid path parameter for event_id',
+    ],
+  ])('rejects %s', (_, param, message) => {
+    expect(() => eventPath(param)).toThrow(new TypeError(message))
+  })
+
+  it('preserves the cause when string coercion fails', () => {
+    const cause = new Error('boom')
+    const param = {
+      toString: () => {
+        throw cause
+      },
+    }
+
+    expect(() => eventPath(param)).toThrow(
+      expect.objectContaining({
+        message: 'Invalid path parameter for event_id',
+        cause,
+      })
+    )
+  })
+
+  it.each([
+    ['an empty string', ''],
+    ['an empty String object', new String('')],
+    ['null', null],
+    ['undefined', undefined],
+  ])('rejects %s as missing', (_, param) => {
+    expect(() => eventPath(param)).toThrow(new TypeError('Missing path parameter for event_id'))
+  })
+})