fix: reject path-traversal sequences in HttpClient::resolveUrl

[gjtorikian]

Summary

Defense-in-depth runtime guard against a path-traversal / cross-resource-API-call vulnerability.

WorkOS service methods interpolate caller-supplied ids directly into request paths with PHP string interpolation (e.g. "connections/{$id}") without rawurlencode-ing them. libcurl normalizes ../ segments before transmission, so a value like ../webhook_endpoints/wh_target causes the SDK to issue a request against a different WorkOS endpoint while still authenticated with the application's API key — i.e. forged cross-resource API requests under the application's bearer token.

This PR hardens HttpClient::resolveUrl() to reject paths containing:

• . or .. segments (between / boundaries) — blocks connections/../webhook_endpoints/...-style cross-resource redirection
• ? or # — blocks query/fragment injection (queries go through the $query argument)
• \x00–\x1f control characters — blocks CRLF / null-byte injection

Throws \InvalidArgumentException without echoing the offending path back, to avoid log injection from CRLF-laden inputs.

Why this lands separately from the structural fix

The structural fix (per-segment rawurlencode at code-generation time) lands in workos/oagen-emitters and will require a regen of this SDK. This runtime guard is valuable independently:

• Protects existing PHP SDK users immediately, before the regen ships.
• Stays valuable as defense-in-depth even after the generator fix lands — the guard applies to any future hand-written path code paths or RequestOptions::baseUrl overrides as well.
• Both files modified (lib/HttpClient.php, tests/HttpClientTest.php) carry @oagen-ignore-file, so the upcoming regen won't clobber the guard.

Versioning

Patch release. Public API surface (method signatures, return types, exception types) is unchanged. The new exception path only fires on inputs that were either being exploited or already producing wrong/404 behavior — no legitimate caller passes .. or control characters as a WorkOS id.

Test plan

• vendor/bin/phpunit — 272 passed, including 17 new tests for the guard (8 unsafe-input cases × 2 entrypoints + 1 positive test that legitimate ids with dots inside a segment still work)
• vendor/bin/phpstan analyse on changed files — clean

🤖 Generated with Claude Code

---

[devin-ai-integration]

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 3 additional findings.

[greptile-apps]

Greptile Summary

This PR adds a defense-in-depth path-traversal guard to HttpClient::resolveUrl() by introducing assertSafePath() and decodeUntilStable(). The guard rejects dot-segments (., ..), control characters, and ?/# injection after fully unwinding any percent- or double-percent-encoding, blocking the cross-resource API-call vector where caller-supplied IDs are interpolated into path templates without rawurlencode.

The implementation correctly addresses the two bypass vectors raised in previous review threads — single-encoded (%2e%2e) and double-encoded (%252e%252e) dot segments — via the iterative decode loop. The test suite covers 20 distinct unsafe inputs tested through both public entrypoints, plus a positive case for legitimate dotted IDs.

Confidence Score: 4/5

Safe to merge — the guard is correctly implemented and all previously flagged encoding bypass vectors are resolved.

No P0 or P1 findings. The security-sensitive nature of the change and the absence of a positive test for percent-encoded-but-safe paths keep this at 4 rather than 5, but the implementation logic is sound.

No files require special attention beyond normal review of the guard logic in lib/HttpClient.php.

Important Files Changed

Filename	Overview
lib/HttpClient.php	Adds assertSafePath() and decodeUntilStable() to resolveUrl() — guards against path-traversal via literal, percent-encoded, and double-encoded dot segments, control characters, and query/fragment injection. Guard is applied only to relative paths (absolute URL early-return bypasses it, by design).
tests/HttpClientTest.php	Adds 20-case data-provider covering literal, percent-encoded, and double-encoded traversal/injection inputs tested against both request() and buildUrl(), plus one positive test for dots-inside-segment IDs.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["resolveUrl(path, options)"] --> B{Absolute URL?\npreg_match https?://}
    B -- Yes --> C["Return path as-is\n(guard skipped)"]
    B -- No --> D["assertSafePath(path)"]
    D --> E["decodeUntilStable(path)\nLoop rawurldecode until stable\n(handles %2e, %252e, etc.)"]
    E --> F{Control chars\nor ? or #?}
    F -- Yes --> G["throw InvalidArgumentException\n(forbidden character)"]
    F -- No --> H{Any segment\n== '.' or '..'?}
    H -- Yes --> I["throw InvalidArgumentException\n(relative segment)"]
    H -- No --> J["Assemble: baseUrl + '/' + ltrim(path)"]
    J --> K["Return resolved URL"]

Loading

_{Reviews (4): Last reviewed commit: "fix(http): close double-encoding bypass ..." | Re-trigger Greptile}

[greptile-apps]

Percent-encoded dot-segment bypass

The guard splits on / and checks for literal '.' and '..', but RFC 3986-compliant HTTP clients (including libcurl, which Guzzle uses by default) normalize percent-encoded equivalents: %2e → . and %2E%2E → ... An attacker who controls the ID can pass %2e%2e and produce connections/%2e%2e/webhook_endpoints/wh_target, which satisfies this check but is resolved by libcurl to webhook_endpoints/wh_target — the same cross-resource redirect the guard is meant to prevent.

Decoding the path before the segment check closes the gap:

foreach (explode('/', $path) as $segment) {
    $decoded = rawurldecode($segment);
    if ($decoded === '.' || $decoded === '..') {
        throw new \InvalidArgumentException(
            'WorkOS request path contains a relative segment ("." or ".."). Refusing to send the request to avoid cross-resource redirection.',
        );
    }
}

[nicknisi]

Agree we should address this

[gjtorikian]

@greptile review