Add tests for path traversal and invalid ID encoding methods

.changeset/fix-dot-segment-path-parameter-encoding.md

@@ -0,0 +1,5 @@
+---
+"@fingerprint/php-sdk": patch
+---
+
+Fixed `event_id`/`visitor_id` values of exactly `.` or `..` being collapsed by curl's URL normalization before the request is sent, causing `getEvent`, `updateEvent`, and `deleteVisitorData` to hit the wrong endpoint instead of the requested resource.

src/Api/FingerprintApi.php

@@ -1690,7 +1690,17 @@ public function updateEventRequest(string $event_id, EventUpdate $event_update):
      */
     protected function createHttpClientOption(): array
     {
-        $options = [];
+        // Path parameters are percent-encoded (see ObjectSerializer::toPathValue()),
+        // but curl still decodes and collapses RFC 3986 dot-segments (e.g. a
+        // literal '.' or '..' path parameter) before the request is sent unless
+        // explicitly told not to. CURLOPT_PATH_AS_IS makes curl transmit the URL
+        // exactly as built, so an event/visitor ID can never be misrouted to a
+        // different endpoint via path normalization.
+        $options = [
+            'curl' => [
+                \CURLOPT_PATH_AS_IS => true,
+            ],
+        ];
         if ($this->config->getDebug()) {
             $options[RequestOptions::DEBUG] = fopen($this->config->getDebugFile(), 'a');
             if (!$options[RequestOptions::DEBUG]) {

src/ObjectSerializer.php

@@ -145,7 +145,18 @@ public static function sanitizeTimestamp(string $timestamp): string
      */
     public static function toPathValue(string $value): string
     {
-        return rawurlencode(self::toString($value));
+        $encoded = rawurlencode(self::toString($value));
+
+        // '.' and '..' are RFC 3986 dot-segments: rawurlencode() leaves the
+        // literal dots untouched, but URL normalizers (e.g. curl, before the
+        // request ever hits the wire) collapse a path segment consisting
+        // solely of dots into '' or 'up one level'. Escaping the dots here
+        // keeps the segment inert without changing any other encoded value.
+        if ('.' === $encoded || '..' === $encoded) {
+            $encoded = str_replace('.', '%2E', $encoded);
+        }
+
+        return $encoded;
     }
 
     /**

template/ObjectSerializer.mustache

@@ -132,7 +132,18 @@ class ObjectSerializer
      */
     public static function toPathValue(string $value): string
     {
-        return rawurlencode(self::toString($value));
+        $encoded = rawurlencode(self::toString($value));
+
+        // '.' and '..' are RFC 3986 dot-segments: rawurlencode() leaves the
+        // literal dots untouched, but URL normalizers (e.g. curl, before the
+        // request ever hits the wire) collapse a path segment consisting
+        // solely of dots into '' or 'up one level'. Escaping the dots here
+        // keeps the segment inert without changing any other encoded value.
+        if ('.' === $encoded || '..' === $encoded) {
+            $encoded = str_replace('.', '%2E', $encoded);
+        }
+
+        return $encoded;
     }
 
     /**

template/api.mustache

@@ -474,7 +474,17 @@ use {{invokerPackage}}\ObjectSerializer;
      */
     protected function createHttpClientOption(): array
     {
-        $options = [];
+        // Path parameters are percent-encoded (see ObjectSerializer::toPathValue()),
+        // but curl still decodes and collapses RFC 3986 dot-segments (e.g. a
+        // literal '.' or '..' path parameter) before the request is sent unless
+        // explicitly told not to. CURLOPT_PATH_AS_IS makes curl transmit the URL
+        // exactly as built, so an event/visitor ID can never be misrouted to a
+        // different endpoint via path normalization.
+        $options = [
+            'curl' => [
+                \CURLOPT_PATH_AS_IS => true,
+            ],
+        ];
         if ($this->config->getDebug()) {
             $options[RequestOptions::DEBUG] = fopen($this->config->getDebugFile(), 'a');
             if (!$options[RequestOptions::DEBUG]) {

test/Api/FingerprintApiTest.php

@@ -26,6 +26,7 @@
 use Fingerprint\ServerSdk\Model\SearchEventsVpnConfidence;
 use Fingerprint\ServerSdk\Model\SupplementaryIDHighRecall;
 use Fingerprint\ServerSdk\Test\MockHelper;
+use Fingerprint\ServerSdk\Test\Support\RawRequestCapture;
 use GuzzleHttp\Client;
 use GuzzleHttp\Exception\ConnectException;
 use GuzzleHttp\Exception\GuzzleException;
@@ -35,6 +36,8 @@
 use GuzzleHttp\Psr7\Response;
 use GuzzleHttp\Utils;
 use PHPUnit\Framework\Attributes\CoversClass;
+use PHPUnit\Framework\Attributes\DataProvider;
+use PHPUnit\Framework\Attributes\Group;
 use PHPUnit\Framework\TestCase;
 use Psr\Http\Message\RequestInterface;
 
@@ -1514,6 +1517,90 @@ public function testUpdateEventNon2xxStatusCode(): void
         $this->api->updateEvent('test', new EventUpdate());
     }
 
+    /**
+     * Verifies a malformed path parameter (path traversal, an absolute URL,
+     * a bare dot-segment, or an empty string) is always percent-encoded into
+     * a single opaque path segment rather than being interpreted as part of
+     * the path structure, across every operation that takes an ID in the path.
+     */
+    #[DataProvider('pathParameterEncodingProvider')]
+    public function testPathParameterIsEncodedAsSingleOpaqueSegment(\Closure $buildRequest, string $value, string $expectedPath): void
+    {
+        $request = $buildRequest($this->api, $value);
+
+        $this->assertSame('api.fpjs.io', $request->getUri()->getHost());
+        $this->assertSame($expectedPath, $request->getUri()->getPath());
+    }
+
+    public static function pathParameterEncodingProvider(): iterable
+    {
+        $endpoints = [
+            'getEventRequest' => ['/v4/events/', static fn (FingerprintApi $api, string $id) => $api->getEventRequest($id)],
+            'updateEventRequest' => ['/v4/events/', static fn (FingerprintApi $api, string $id) => $api->updateEventRequest($id, new EventUpdate())],
+            'deleteVisitorDataRequest' => ['/v4/visitors/', static fn (FingerprintApi $api, string $id) => $api->deleteVisitorDataRequest($id)],
+        ];
+
+        $values = [
+            'path traversal' => ['../events', '..%2Fevents'],
+            'nested path traversal' => ['../../events', '..%2F..%2Fevents'],
+            'absolute url' => ['https://domain.tld/evil', 'https%3A%2F%2Fdomain.tld%2Fevil'],
+            'dot segment' => ['.', '%2E'],
+            'parent dot segment' => ['..', '%2E%2E'],
+            'empty' => ['', ''],
+        ];
+
+        foreach ($endpoints as $endpoint => [$prefix, $call]) {
+            foreach ($values as $case => [$value, $encoded]) {
+                yield "{$endpoint}: {$case}" => [$call, $value, $prefix.$encoded];
+            }
+        }
+    }
+
+    /**
+     * Regression test for the actual wire-level bug: a PSR-7 Uri never
+     * normalizes dot-segments (asserting on $request->getUri()->getPath()
+     * alone would pass even without ObjectSerializer's encoding fix), but
+     * curl decodes and collapses them just before sending unless
+     * CURLOPT_PATH_AS_IS is set. This spins up a real local TCP listener and
+     * checks the literal bytes a real Guzzle+curl request puts on the wire,
+     * so it fails if either half of the fix (percent-encoding in
+     * ObjectSerializer::toPathValue, or CURLOPT_PATH_AS_IS in
+     * createHttpClientOption) is reverted.
+     */
+    #[DataProvider('dotSegmentWireProvider')]
+    #[Group('wire')]
+    public function testDotSegmentIsNotCollapsedOnTheWire(\Closure $call, string $expectedPrefix): void
+    {
+        $capture = RawRequestCapture::start();
+
+        try {
+            $config = new Configuration('test-api-key');
+            $config->setHost($capture->baseUri().'/v4');
+            $api = new FingerprintApi($config, new Client(['timeout' => 10]));
+
+            try {
+                $call($api);
+            } catch (\Throwable $e) {
+                // Only the request line on the wire matters for this test.
+            }
+
+            $this->assertStringStartsWith($expectedPrefix, $capture->requestLine());
+        } finally {
+            $capture->stop();
+        }
+    }
+
+    public static function dotSegmentWireProvider(): iterable
+    {
+        yield 'getEvent: dot segment' => [static fn (FingerprintApi $api) => $api->getEvent('.'), 'GET /v4/events/%2E?'];
+
+        yield 'getEvent: parent dot segment' => [static fn (FingerprintApi $api) => $api->getEvent('..'), 'GET /v4/events/%2E%2E?'];
+
+        yield 'updateEvent: dot segment' => [static fn (FingerprintApi $api) => $api->updateEvent('.', new EventUpdate()), 'PATCH /v4/events/%2E?'];
+
+        yield 'deleteVisitorData: dot segment' => [static fn (FingerprintApi $api) => $api->deleteVisitorData('.'), 'DELETE /v4/visitors/%2E?'];
+    }
+
     private function parseQueryString(string $query): array
     {
         $queryArray = [];

test/ObjectSerializerTest.php

@@ -302,6 +302,69 @@ public function testToPathValue(): void
         $this->assertSame('simple', ObjectSerializer::toPathValue('simple'));
     }
 
+    /**
+     * Verifies path traversal sequences are encoded so a single path segment
+     * cannot escape into a sibling resource (e.g. `../events`).
+     */
+    public function testToPathValueEncodesPathTraversal(): void
+    {
+        $this->assertSame('..%2Fevents', ObjectSerializer::toPathValue('../events'));
+        $this->assertSame('..%2F..%2Fevents', ObjectSerializer::toPathValue('../../events'));
+    }
+
+    /**
+     * Verifies slashes are always encoded, since an un-encoded slash would let
+     * a path parameter inject extra path segments.
+     */
+    public function testToPathValueEncodesSlash(): void
+    {
+        $this->assertSame('abc%2Fdef', ObjectSerializer::toPathValue('abc/def'));
+    }
+
+    /**
+     * A value that looks like an absolute URL must not be able to redirect
+     * the request elsewhere; its scheme and slashes are encoded so it stays
+     * a single, inert path segment.
+     */
+    public function testToPathValueEncodesAbsoluteUrlValue(): void
+    {
+        $this->assertSame('https%3A%2F%2Fdomain.tld%2Fevil', ObjectSerializer::toPathValue('https://domain.tld/evil'));
+    }
+
+    public function testToPathValueWithEmptyString(): void
+    {
+        $this->assertSame('', ObjectSerializer::toPathValue(''));
+    }
+
+    /**
+     * A path parameter of exactly '.' or '..' is an RFC 3986 dot-segment:
+     * left as a literal dot, URL normalizers (including curl, before the
+     * request ever reaches the wire — see RawRequestCapture-based tests in
+     * FingerprintApiTest) collapse it into the parent/current path instead
+     * of treating it as an opaque resource identifier. The dots must be
+     * percent-encoded so no normalizer can mistake the segment for one.
+     */
+    public function testToPathValueEncodesDotSegment(): void
+    {
+        $this->assertSame('%2E', ObjectSerializer::toPathValue('.'));
+    }
+
+    public function testToPathValueEncodesDotDotSegment(): void
+    {
+        $this->assertSame('%2E%2E', ObjectSerializer::toPathValue('..'));
+    }
+
+    /**
+     * Only a segment consisting solely of dots is special under RFC 3986;
+     * anything else containing a dot (e.g. a real event ID) must pass
+     * through unencoded, since '.' is otherwise a safe, unreserved character.
+     */
+    public function testToPathValueDoesNotEncodeDotsInOtherwiseNormalValues(): void
+    {
+        $this->assertSame('1708102555327.NLOjmg', ObjectSerializer::toPathValue('1708102555327.NLOjmg'));
+        $this->assertSame('...', ObjectSerializer::toPathValue('...'));
+    }
+
     // -- toHeaderValue --
 
     public function testToHeaderValueWithString(): void

test/Support/RawRequestCapture.php

@@ -0,0 +1,130 @@
+<?php
+
+namespace Fingerprint\ServerSdk\Test\Support;
+
+/**
+ * Captures the literal request-line a real HTTP client puts on the wire.
+ *
+ * PSR-7's Uri never normalizes dot-segments, so asserting against
+ * $request->getUri()->getPath() cannot reveal that curl collapses a bare
+ * '.' or '..' path segment before the bytes leave the process. This spins
+ * up a plain TCP listener in a child process and hands back the raw
+ * request line it received, so tests can assert on what the transport
+ * actually sent rather than what the PHP object model says it sent.
+ *
+ * @internal
+ */
+final class RawRequestCapture
+{
+    private $process;
+
+    /** @var resource */
+    private $stdout;
+
+    /** @var resource */
+    private $stderr;
+
+    private string $buffer = '';
+
+    private int $port;
+
+    /**
+     * @param resource $process
+     * @param resource $stdout
+     * @param resource $stderr
+     */
+    private function __construct($process, $stdout, $stderr)
+    {
+        $this->process = $process;
+        $this->stdout = $stdout;
+        $this->stderr = $stderr;
+    }
+
+    public static function start(): self
+    {
+        $process = proc_open(
+            [PHP_BINARY, __DIR__.'/raw_request_listener.php'],
+            [1 => ['pipe', 'w'], 2 => ['pipe', 'w']],
+            $pipes
+        );
+
+        if (!\is_resource($process)) {
+            throw new \RuntimeException('Failed to start raw request listener.');
+        }
+
+        foreach ($pipes as $pipe) {
+            stream_set_blocking($pipe, false);
+        }
+
+        $capture = new self($process, $pipes[1], $pipes[2]);
+
+        try {
+            $ready = $capture->readLine(10.0);
+            if (!str_starts_with($ready, 'READY ')) {
+                throw new \RuntimeException("Unexpected listener handshake: {$ready}");
+            }
+            $capture->port = (int) substr($ready, 6);
+        } catch (\Throwable $e) {
+            $capture->stop();
+
+            throw $e;
+        }
+
+        return $capture;
+    }
+
+    public function baseUri(): string
+    {
+        return "http://127.0.0.1:{$this->port}";
+    }
+
+    /**
+     * Returns the raw request line (e.g. "GET /v4/events/. HTTP/1.1")
+     * once the listener has accepted and read one request.
+     */
+    public function requestLine(): string
+    {
+        return $this->readLine(10.0);
+    }
+
+    public function stop(): void
+    {
+        if (\is_resource($this->process)) {
+            proc_terminate($this->process);
+            proc_close($this->process);
+        }
+    }
+
+    private function readLine(float $timeout): string
+    {
+        $deadline = microtime(true) + $timeout;
+
+        while (false === ($eol = strpos($this->buffer, "\n"))) {
+            $chunk = fread($this->stdout, 8192);
+            if (false !== $chunk && '' !== $chunk) {
+                $this->buffer .= $chunk;
+
+                continue;
+            }
+            if (feof($this->stdout)) {
+                throw new \RuntimeException('Raw request listener exited early. stderr: '.$this->stderrTail());
+            }
+            if (microtime(true) >= $deadline) {
+                throw new \RuntimeException('Timed out reading from the raw request listener. stderr: '.$this->stderrTail());
+            }
+            usleep(5000);
+        }
+
+        $line = substr($this->buffer, 0, $eol);
+        $this->buffer = substr($this->buffer, $eol + 1);
+
+        return rtrim($line, "\r");
+    }
+
+    private function stderrTail(): string
+    {
+        $stderr = (string) @stream_get_contents($this->stderr);
+
+        return '' === $stderr ? '(empty)' : $stderr;
+    }
+}