From 9707ebacbcfe239df2ac3b657db4f282c8e3b9d1 Mon Sep 17 00:00:00 2001 From: "Garen J. Torikian" Date: Fri, 1 May 2026 16:24:59 -0400 Subject: [PATCH 1/3] fix: reject path-traversal sequences in HttpClient::resolveUrl WorkOS service methods interpolate caller-supplied IDs directly into the request path with PHP string interpolation (e.g. "connections/{$id}") without URL-encoding. libcurl normalizes "../" before transmission, so a value like "../webhook_endpoints/wh_target" causes the SDK to issue a request against a different WorkOS endpoint while still presenting the application's API key. This adds a defense-in-depth runtime guard. The structural fix (per-segment rawurlencode in the generator) lands in oagen-emitters separately; this guard remains valuable even after that ships, and protects existing PHP SDK users on the current generator output. Reject paths whose segments are "." or "..", and paths containing "?", "#", or any \\x00-\\x1f control character. Throw \\InvalidArgumentException without echoing the offending path to avoid log injection. Co-Authored-By: Claude Opus 4.7 (1M context) --- lib/HttpClient.php | 27 ++++++++++++++++ tests/HttpClientTest.php | 69 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 96 insertions(+) diff --git a/lib/HttpClient.php b/lib/HttpClient.php index 5d7a4823..07033146 100644 --- a/lib/HttpClient.php +++ b/lib/HttpClient.php @@ -236,11 +236,38 @@ private function resolveUrl(string $path, ?RequestOptions $options): string return $path; } + $this->assertSafePath($path); + $baseUrl = $options !== null && $options->baseUrl !== null ? $options->baseUrl : $this->baseUrl; $baseUrl = rtrim($baseUrl, '/'); return $baseUrl . '/' . ltrim($path, '/'); } + /** + * Reject paths whose segments could escape the intended endpoint once + * normalized by the HTTP transport. Service methods interpolate caller- + * supplied IDs into path templates without per-segment URL-encoding, so + * an unencoded "../" or embedded "?"/"#"/CRLF in a single ID would + * silently re-target the request at a different WorkOS resource under + * the application's authenticated API key. + */ + private function assertSafePath(string $path): void + { + if (preg_match('/[\x00-\x1f?#]/', $path) === 1) { + throw new \InvalidArgumentException( + 'WorkOS request path contains a forbidden character (control character, "?", or "#"). Pass query parameters via the $query argument rather than embedding them in the path.', + ); + } + + foreach (explode('/', $path) as $segment) { + if ($segment === '.' || $segment === '..') { + throw new \InvalidArgumentException( + 'WorkOS request path contains a relative segment ("." or ".."). Refusing to send the request to avoid cross-resource redirection.', + ); + } + } + } + private function resolveTimeout(?RequestOptions $options): int { return $options !== null && $options->timeout !== null ? $options->timeout : $this->timeout; diff --git a/tests/HttpClientTest.php b/tests/HttpClientTest.php index 34af98a6..61198002 100644 --- a/tests/HttpClientTest.php +++ b/tests/HttpClientTest.php @@ -85,6 +85,75 @@ public function testBuildUrlAppendsQueryString(): void $this->assertSame('code', $query['response_type']); } + /** + * @return array + */ + public static function unsafePathProvider(): array + { + return [ + 'parent traversal segment' => ['connections/../webhook_endpoints/wh_target'], + 'leading parent traversal' => ['../webhook_endpoints/wh_target'], + 'current directory segment' => ['connections/./id'], + 'embedded query character' => ['connections/conn_123?override=1'], + 'embedded fragment character' => ['connections/conn_123#frag'], + 'embedded carriage return' => ["connections/conn_123\r\nHost: evil"], + 'embedded newline' => ["connections/conn_123\nfoo"], + 'embedded null byte' => ["connections/conn_123\x00"], + ]; + } + + /** + * @dataProvider unsafePathProvider + */ + public function testRequestRejectsUnsafePaths(string $path): void + { + $client = new HttpClient( + apiKey: 'test_key', + clientId: null, + baseUrl: 'https://api.workos.com', + timeout: 10, + maxRetries: 0, + ); + + $this->expectException(\InvalidArgumentException::class); + $client->request('DELETE', $path); + } + + /** + * @dataProvider unsafePathProvider + */ + public function testBuildUrlRejectsUnsafePaths(string $path): void + { + $client = new HttpClient( + apiKey: 'test_key', + clientId: null, + baseUrl: 'https://api.workos.com', + timeout: 10, + maxRetries: 0, + ); + + $this->expectException(\InvalidArgumentException::class); + $client->buildUrl($path); + } + + public function testRequestAllowsSafePathsWithDotsInsideSegments(): void + { + $mock = new MockHandler([ + new Response(200, ['Content-Type' => 'application/json'], '{}'), + ]); + + $client = new HttpClient( + apiKey: 'test_key', + clientId: null, + baseUrl: 'https://api.workos.com', + timeout: 10, + maxRetries: 0, + handler: HandlerStack::create($mock), + ); + + $this->assertSame([], $client->request('GET', 'users/user.with.dots')); + } + public function testErrorResponseIncludesCodeAndError(): void { $body = json_encode([ From f0343b59bad28d50414c9be0270b5def9576b8eb Mon Sep 17 00:00:00 2001 From: "Garen J. Torikian" Date: Fri, 1 May 2026 17:31:06 -0400 Subject: [PATCH 2/3] fix: percent-decode path before traversal checks in HttpClient Encoded variants like %2e%2e and %2f could bypass the literal "."/".." and "?"/"#"/CRLF checks if a downstream proxy or the receiving server normalized them. Decode the path once with rawurldecode() before both checks so encoded dots, slashes, query/fragment markers, and control characters are all caught. Co-Authored-By: Claude Opus 4.7 (1M context) --- lib/HttpClient.php | 19 ++++++++++++------- tests/HttpClientTest.php | 9 +++++++++ 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/lib/HttpClient.php b/lib/HttpClient.php index 07033146..d6ef598f 100644 --- a/lib/HttpClient.php +++ b/lib/HttpClient.php @@ -245,21 +245,26 @@ private function resolveUrl(string $path, ?RequestOptions $options): string /** * Reject paths whose segments could escape the intended endpoint once - * normalized by the HTTP transport. Service methods interpolate caller- - * supplied IDs into path templates without per-segment URL-encoding, so - * an unencoded "../" or embedded "?"/"#"/CRLF in a single ID would - * silently re-target the request at a different WorkOS resource under - * the application's authenticated API key. + * normalized by the HTTP transport or the receiving server. Service + * methods interpolate caller-supplied IDs into path templates without + * per-segment URL-encoding, so an unencoded "../" or embedded "?"/"#"/CRLF + * in a single ID would silently re-target the request at a different + * WorkOS resource under the application's authenticated API key. + * + * The check runs against the percent-decoded path so that encoded + * variants (`%2e%2e`, `%2f`, `%3f`, `%0d%0a`, ...) cannot bypass it. */ private function assertSafePath(string $path): void { - if (preg_match('/[\x00-\x1f?#]/', $path) === 1) { + $decoded = rawurldecode($path); + + if (preg_match('/[\x00-\x1f?#]/', $decoded) === 1) { throw new \InvalidArgumentException( 'WorkOS request path contains a forbidden character (control character, "?", or "#"). Pass query parameters via the $query argument rather than embedding them in the path.', ); } - foreach (explode('/', $path) as $segment) { + foreach (explode('/', $decoded) as $segment) { if ($segment === '.' || $segment === '..') { throw new \InvalidArgumentException( 'WorkOS request path contains a relative segment ("." or ".."). Refusing to send the request to avoid cross-resource redirection.', diff --git a/tests/HttpClientTest.php b/tests/HttpClientTest.php index 61198002..bdeed3b4 100644 --- a/tests/HttpClientTest.php +++ b/tests/HttpClientTest.php @@ -99,6 +99,15 @@ public static function unsafePathProvider(): array 'embedded carriage return' => ["connections/conn_123\r\nHost: evil"], 'embedded newline' => ["connections/conn_123\nfoo"], 'embedded null byte' => ["connections/conn_123\x00"], + 'percent-encoded parent traversal lowercase' => ['connections/%2e%2e/webhook_endpoints/wh_target'], + 'percent-encoded parent traversal uppercase' => ['connections/%2E%2E/webhook_endpoints/wh_target'], + 'percent-encoded current directory segment' => ['connections/%2e/id'], + 'percent-encoded slash hides traversal' => ['connections%2F..%2Fwebhook_endpoints'], + 'percent-encoded slash hides encoded traversal' => ['connections%2F%2e%2e%2Fwebhook_endpoints'], + 'percent-encoded query character' => ['connections/conn_123%3Foverride=1'], + 'percent-encoded fragment character' => ['connections/conn_123%23frag'], + 'percent-encoded CRLF injection' => ['connections/conn_123%0D%0AHost:%20evil'], + 'percent-encoded null byte' => ['connections/conn_123%00'], ]; } From d9591723b16d4077b64e97bd4bd420fba71d38d1 Mon Sep 17 00:00:00 2001 From: "Garen J. Torikian" Date: Sat, 2 May 2026 10:19:57 -0400 Subject: [PATCH 3/3] fix(http): close double-encoding bypass in path traversal guard Replace single rawurldecode() with a decode-until-stable loop so that double-encoded sequences like %252e%252e cannot slip past the assertSafePath check. Add test cases for double-encoded traversal, slash, query character, and null byte variants. --- lib/HttpClient.php | 21 ++++++++++++++++++--- tests/HttpClientTest.php | 4 ++++ 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/lib/HttpClient.php b/lib/HttpClient.php index d6ef598f..deb1868d 100644 --- a/lib/HttpClient.php +++ b/lib/HttpClient.php @@ -251,12 +251,13 @@ private function resolveUrl(string $path, ?RequestOptions $options): string * in a single ID would silently re-target the request at a different * WorkOS resource under the application's authenticated API key. * - * The check runs against the percent-decoded path so that encoded - * variants (`%2e%2e`, `%2f`, `%3f`, `%0d%0a`, ...) cannot bypass it. + * The check runs against the fully percent-decoded path so that encoded + * variants (`%2e%2e`, `%2f`, `%3f`, `%0d%0a`, ...) and double-encoded + * variants (`%252e%252e`, `%252f`, ...) cannot bypass it. */ private function assertSafePath(string $path): void { - $decoded = rawurldecode($path); + $decoded = self::decodeUntilStable($path); if (preg_match('/[\x00-\x1f?#]/', $decoded) === 1) { throw new \InvalidArgumentException( @@ -273,6 +274,20 @@ private function assertSafePath(string $path): void } } + /** + * Decode percent-encoded characters in a loop until the value stabilizes, + * closing double-encoding bypass vectors like `%252e%252e`. + */ + private static function decodeUntilStable(string $value): string + { + do { + $prev = $value; + $value = rawurldecode($value); + } while ($value !== $prev); + + return $value; + } + private function resolveTimeout(?RequestOptions $options): int { return $options !== null && $options->timeout !== null ? $options->timeout : $this->timeout; diff --git a/tests/HttpClientTest.php b/tests/HttpClientTest.php index bdeed3b4..429c5ec4 100644 --- a/tests/HttpClientTest.php +++ b/tests/HttpClientTest.php @@ -108,6 +108,10 @@ public static function unsafePathProvider(): array 'percent-encoded fragment character' => ['connections/conn_123%23frag'], 'percent-encoded CRLF injection' => ['connections/conn_123%0D%0AHost:%20evil'], 'percent-encoded null byte' => ['connections/conn_123%00'], + 'double-encoded parent traversal' => ['connections/%252e%252e/webhook_endpoints'], + 'double-encoded slash hides traversal' => ['connections%252F..%252Fwebhook_endpoints'], + 'double-encoded query character' => ['connections/conn_123%253Foverride=1'], + 'double-encoded null byte' => ['connections/conn_123%2500'], ]; }