fix: URL-encode path params to prevent SSRF/path traversal (GHSA-vv7q-7jx5-f767) by jlowin · Pull Request #3507 · PrefectHQ/fastmcp

jlowin · 2026-03-14T20:40:15Z

RequestDirector._build_url() substituted path parameter values directly into URL templates without URL-encoding. Since urllib.parse.urljoin() interprets ../ sequences, an attacker who controls a path parameter value (e.g. ../../../admin/delete-all?) can traverse out of the intended API prefix and reach arbitrary backend endpoints — inheriting any auth credentials the provider is configured with.

The fix applies urllib.parse.quote(str(param_value), safe="") to every path parameter value before substitution. Using safe="" ensures all special characters — including /, ., ?, and # — are percent-encoded, so urljoin treats the substituted value as a single opaque path segment rather than interpreting path navigation sequences.

# Before (vulnerable):
url_path = url_path.replace(placeholder, str(param_value))

# After (safe):
safe_value = quote(str(param_value), safe="")
url_path = url_path.replace(placeholder, safe_value)

Query and header parameters are not affected — they are passed to httpx.Request via params= and headers=, which handle encoding internally.

Ref: GHSA-vv7q-7jx5-f767

…traversal Co-authored-by: Claude <[email protected]>

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3583c26b55

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-03-14T20:42:18Z

            placeholder = f"{{{param_name}}}"
            if placeholder in url_path:
-                url_path = url_path.replace(placeholder, str(param_value))
+                safe_value = quote(str(param_value), safe="")


Escape dot-segment path params before URL join

quote(str(param_value), safe="") does not escape . characters, so attacker input like ".." is still substituted as a real dot segment. When urljoin() combines the URL, it normalizes /../ and can climb out of the intended route prefix (for example, /api/v1/users/{id}/profile becomes /api/v1/profile), so path traversal remains possible for this input class.

Useful? React with 👍 / 👎.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8b18db9fff

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-03-14T20:52:38Z

+            path_route, {"id": "../admin"}, "https://api.example.com"
+        )
+        url = str(request.url)
+        assert "..%2Fadmin" in url or "..%2fadmin" in url


Update dot-dot test to match encoded dot segments

After _build_url started encoding . as %2E, this assertion no longer matches the produced URL (%2E%2E%2Fadmin), so uv run pytest tests/utilities/openapi/test_director.py now fails in test_dot_dot_slash_is_encoded. This leaves the new security regression suite red and blocks the required test workflow for the commit.

Useful? React with 👍 / 👎.

marvin-context-protocol · 2026-03-14T20:56:28Z

Test Failure Analysis

✏️ Edited to reflect the latest CI run (#23096236334).

Summary: A single test in tests/cli/test_run.py is timing out on Windows / Python 3.10 only. All other matrix jobs (Ubuntu Python 3.10, Ubuntu Python 3.13, lowest-direct deps, integration) are green.

Root Cause: TestMCPConfig::test_run_mcp_config spawns a subprocess via StdioMCPServer(command="python", args=[script_path]), then connects to it using FastMCP's stdio transport to call list_tools(). The test is hitting the 5-second unit-test timeout while blocked in asyncio.selector_events waiting on the subprocess's stdout. This failure is unrelated to the PR's changes (URL-encoding path params in director.py) — nothing in the diff touches subprocess or stdio transport handling.

This looks like a pre-existing Windows subprocess/stdio flakiness. The Python subprocess appears to connect (the debug log shows Stdio transport connected) but never sends back a list_tools response within the timeout window.

Suggested Solution: This is not caused by this PR. Options:

Re-run the failed job — if it's a transient flake, it should pass.
If it's consistently failing on Windows for this test, the test may need a longer timeout or a @pytest.mark.skip on Windows (sys.platform == "win32"), but that's a separate issue from this PR.

Detailed Analysis

Failing job: Tests: Python 3.10 on windows-latest (job ID 67089143832)

Relevant log excerpt:

tests\cli\test_run.py ........+++++++++++++++++++++++++++++++++++ Timeout +++++++++++++++++++++++++++++++++++

The stderr captured at timeout shows the sequence:

MCPConfigTransport inferred, stdio subprocess started
Stdio transport connected logged
[ProxyClient] called list_tools — sent but no response received before 5s timeout
Thread stack frozen at selectors.select(timeout)

PR changes (none of these touch the failing code path):

src/fastmcp/utilities/openapi/director.py: URL-encode path params via urllib.parse.quote
tests/utilities/openapi/test_director.py: New TestPathTraversalPrevention tests
.github/workflows/marvin-label-triage.yml: Workflow label update

The previous failure I reported (test_dot_dot_slash_is_encoded) appears to have been fixed by a follow-up commit.

Related Files

tests/cli/test_run.py:104 — test_run_mcp_config: the timing-out test; spawns a Python subprocess via stdio
src/fastmcp/utilities/openapi/director.py — the PR's actual change (not related to the failure)

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 20d0a9e31f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-03-14T21:01:30Z

            placeholder = f"{{{param_name}}}"
            if placeholder in url_path:
-                url_path = url_path.replace(placeholder, str(param_value))
+                safe_value = quote(str(param_value), safe="").replace("..", "%2E%2E")


Encode single-dot path segments before urljoin

quote(str(param_value), safe="") still leaves . unescaped, and this patch only rewrites .., so a path param of "." is substituted literally. In urljoin, a "/./" segment is normalized away (e.g. /api/v1/users/{id}/profile becomes /api/v1/users/profile), which means caller-controlled params can still rewrite the target route and bypass path-segment boundaries the fix is meant to enforce.

Useful? React with 👍 / 👎.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 66693f068e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-03-15T13:51:45Z

+            "https://api.example.com",
+        )
+        url = str(request.url)
+        assert "/admin" not in unquote(unquote(url)).split("/api/v1/users/")[0]


Assert traversal check on decoded path, not URL prefix

The test_double_encoded_traversal assertion currently inspects split("/api/v1/users/")[0], which is only the part before that marker (typically just the scheme/host). That means a decoded URL like /api/v1/users/../../admin/profile would still satisfy this check, so the test does not actually validate the traversal condition it is meant to cover. Checking the decoded path portion after /api/v1/users/ (or the full decoded path) would make this regression test effective.

Useful? React with 👍 / 👎.

@tool

Co-authored-by: Claude Opus 4.6 <[email protected]> Co-authored-by: Jeremiah Lowin <[email protected]> Co-authored-by: Marvin Context Protocol <41898282+Marvin Context [email protected]> Co-authored-by: voidborne-d <[email protected]> Co-authored-by: marvin-context-protocol[bot] <225465937+marvin-context-protocol[bot]@users.noreply.github.com> Co-authored-by: Claude <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: d 🔹 <[email protected]> Co-authored-by: Jeremiah Lowin <[email protected]> Co-authored-by: nightcityblade <[email protected]> Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]> Co-authored-by: Claude Sonnet 4.6 <[email protected]> Co-authored-by: Bill Easton <[email protected]> Co-authored-by: Sumanshu Nankana <[email protected]> Co-authored-by: Eric Robinson <[email protected]> Co-authored-by: Martim Santos <[email protected]> Co-authored-by: d 🔹 <[email protected]> Co-authored-by: Matthieu B <[email protected]> Co-authored-by: Sascha Buehrle <[email protected]> Co-authored-by: Hakancan <[email protected]> Co-authored-by: nightcityblade <[email protected]> Co-authored-by: Matt Hallowell <[email protected]> Co-authored-by: nate nowack <[email protected]> Co-authored-by: Bill Easton <[email protected]> Co-authored-by: Marcus Shu <[email protected]> Co-authored-by: Rushabh Doshi <[email protected]> Co-authored-by: AIKAWA Shigechika <[email protected]> Co-authored-by: Jeremy Simon <[email protected]> Co-authored-by: Miguel Miranda Dias <[email protected]> Co-authored-by: Anthony James Padavano <[email protected]> Co-authored-by: Mostafa Kamal <[email protected]> Fix auto-close MRE script posting comment without closing (#3386) Fix WorkOS token scope verification bypass 🤖 Generated with Codex (#3407) Fix initialize McpError fallthrough 🤖 Generated with Codex (#3413) Fix transform arg collisions with passthrough params (#3431) Fix get_* returning None when latest version is disabled (#3439) Fix get_* returning None when latest version is disabled (#3421) Fix server lifespan overlap teardown (#3415) Fix $ref output schema object detection regression (#3420) resolved annotations (#3429) Fix async partial callables rejected by iscoroutinefunction (#3438) Fix async partial callables rejected by iscoroutinefunction (#3423) fix: add version to components (#3458) fix: use intent-based flag for OIDC scope patch in load_access_token (#3465) Fixes #3461 fix: normalize Google scope shorthands and surface valid_scopes (#3477) fix: resolve ty 0.0.23 type-checking errors and bump pin (#3481) fix: shield lifespan teardown from cancellation (#3480) fix: forward custom_route endpoints from mounted servers (#3462) fix updates _get_additional_http_routes() to traverse providers, Fixes #3457 fix: remove hardcoded version from CLI help text (#3456) fix: monty 0.0.8 compatibility, drop external_functions from constructor (#3468) fix: task test teardown hanging 5s per test (#3499) Closes #3498 fix: validate workspace path is a directory before cursor install (#3440) Fixes #3426 fix: handle re.error from malformed URI templates in build_regex (#3501) fix: reject empty/OIDC-only required_scopes in AzureProvider (#3503) fix: restrict $ref resolution to local refs only (SSRF/LFI) (#3502) fix warnings and timeouts (#3504) close upgrade check issue when build passes (#3505) Closes #3484 fix: URL-encode path params to prevent SSRF/path traversal (GHSA-vv7q-7jx5-f767) (#3507) fix: prevent path traversal in skill download (#3493) fix: prefer IdP-granted scopes over client-requested scopes in OAuthProxy (#3492) fix: remove unrelated transform and http.py changes from PR scope fix: remove forced follow_redirects from httpx_client_factory calls (#3496) fix: stop passing follow_redirects to httpx_client_factory fix: restore follow_redirects=True for custom httpx client factories Closes #3509 fix: CSRF double-submit cookie check in consent flow (#3519) fix: validate server names in install commands (#3522) fix: use raw strings for regex in pytest.raises match (#3523) fix: reject refresh tokens used as Bearer access tokens (#3524) fix: route ResourcesAsTools/PromptsAsTools through server middleware (#3495) fix: resolve Pyright "Module is not callable" on @tool, @resource, @prompt decorators (#3540) fix: filter warnings by message in KEY_PREFIX test (#3549) fix: suppress output schema for ToolResult subclass annotations (#3548) fix: increase sleep duration in proxy cache tests (#3567) fix: store absolute token expiry to prevent stale expires_in on reload (#3572) fix: preserve tool properties named 'title' during schema compression (#3582) Fix loopback redirect URI port matching per RFC 8252 §7.3 (#3589) Fix app tool routing: visibility check and middleware propagation (#3591) Fix query parameter serialization to respect OpenAPI explode/style settings (#3595) Fix dev apps form: union types, textarea support, JSON parsing (#3597) fix(google): replace deprecated /oauth2/v1/tokeninfo with /oauth2/v3/userinfo (#3603) fix: resolve EntraOBOToken dependency injection through MultiAuth (#3609) fix(docs): correct misleading stateless_http header (#3622) fix: filesystem provider import machinery (#3626) Closes #3625 (issues 2, 3, 6) fix: recover StdioTransport after subprocess exits (#3630) fix(server): preserve mounted tool task metadata (#3632) fix: scope deprecation warning filter to FastMCPDeprecationWarning (#3649) fix imports, add PrefabAppConfig (#3650) fix: resolve CurrentFastMCP/ctx.fastmcp to child server in mounted background tasks (#3651) Fix blocking docs issues: chart imports, Select API, Rx consistency (#3652) closed by default (#3657) Fix prompt caching middleware missing wrap/unwrap round-trip (#3666) fix: serialize object query params per OpenAPI style/explode rules (#3662) Fixes #2857 fix: HTTP request headers not accessible in background task workers (#3631) fix: restore HTTP headers in worker execution path for background tasks (#3681) fix: strip discriminator after dereferencing schemas (#3682) fix: remove stale ty:ignore directives for ty 0.0.26 (#3684) Fix docs gaps in app provider pages (#3690) fix: dev apps log panel UX improvements (#3698) fix dev server empty string args (#3700)

fix: URL-encode path params in OpenAPI provider to prevent SSRF/path …

3583c26

…traversal Co-authored-by: Claude <[email protected]>

jlowin added the bug Something isn't working. Reports of errors, unexpected behavior, or broken functionality. label Mar 14, 2026

marvin-context-protocol Bot added high-priority http Related to HTTP transport, networking, or web server functionality. labels Mar 14, 2026

chatgpt-codex-connector Bot reviewed Mar 14, 2026

View reviewed changes

jlowin added 2 commits March 14, 2026 16:47

Exempt too-long from core-category requirement in triage

6b829a7

fix: also encode dots in path params to prevent bare .. traversal

8b18db9

chatgpt-codex-connector Bot reviewed Mar 14, 2026

View reviewed changes

fix: only encode .. (not all dots) to preserve valid dotted values

20d0a9e

chatgpt-codex-connector Bot reviewed Mar 14, 2026

View reviewed changes

fix: encode all dots in path params to prevent single-dot normalization

66693f0

chatgpt-codex-connector Bot reviewed Mar 15, 2026

View reviewed changes

fix: check decoded path stays within prefix in double-encoding test

4b4f119

jlowin merged commit 40bdfb6 into main Mar 15, 2026
7 checks passed

jlowin deleted the security/openapi-path-traversal-ssrf branch March 15, 2026 14:45

jlowin added the security Security fixes: input validation, SSRF/LFI prevention, auth hardening, injection defenses. label Mar 15, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: URL-encode path params to prevent SSRF/path traversal (GHSA-vv7q-7jx5-f767)#3507

fix: URL-encode path params to prevent SSRF/path traversal (GHSA-vv7q-7jx5-f767)#3507
jlowin merged 6 commits intomainfrom
security/openapi-path-traversal-ssrf

jlowin commented Mar 14, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Mar 14, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Mar 14, 2026

Uh oh!

marvin-context-protocol Bot commented Mar 14, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Mar 14, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Mar 15, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

jlowin commented Mar 14, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Mar 14, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Mar 14, 2026

Choose a reason for hiding this comment

Uh oh!

marvin-context-protocol Bot commented Mar 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Failure Analysis

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Mar 14, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Mar 15, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

marvin-context-protocol Bot commented Mar 14, 2026 •

edited

Loading