Skip to content

feat(python): add encode_path_params config to percent-encode path parameters - #17295

Merged
aditya-arolkar-swe merged 5 commits into
mainfrom
devin/1785416411-python-encode-path-params
Jul 31, 2026
Merged

feat(python): add encode_path_params config to percent-encode path parameters#17295
aditya-arolkar-swe merged 5 commits into
mainfrom
devin/1785416411-python-encode-path-params

Conversation

@fern-api

@fern-api fern-api Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Description

Refs Auth0 SEC-16202 (Pylon #22523)

Follow-up to #17294 (PHP). While investigating the reported path traversal in the generated PHP SDK,
Python turned out to have the same weakness: generated clients do route path parameters through
encode_path_param, but that helper only stringified the value — it never percent-encoded it. So a
value like ../connections still reaches the URL intact and, once the HTTP client and server
normalize it, the request can resolve to a different endpoint than the method targets.

Changes Made

  • generators/python/core_utilities/shared/jsonable_encoder.py: encode_path_param now returns
    quote(str(jsonable_encoder(obj)), safe=""). Booleans keep their true/false rendering.
  • Added test_encode_path_param to generators/python/tests/sdk/test_jsonable_encoder.py.
  • Propagated the updated core utility to the seed/python-sdk fixtures.
  • Changelog: generators/python/sdk/changes/unreleased/encode-path-parameters.yml (fix).

Behavior note for reviewers: same caveat as the PHP PR — APIs that intentionally accept
slash-containing path parameter values will now send %2F. This matches TypeScript, Go, Java, and
C#, but it is a visible change for existing Python users.

Testing

  • Unit tests added/updated (test_encode_path_param)
  • Manual testing completed — verified quote(..., safe="") output for the reported payload
    (../connections..%2Fconnections, user id?user%20id%3F, user_1user_1).
    poetry install / pytest could not run in this environment (pypi.org is not reachable from
    it), so CI is the first full run of the Python test suite for this change.

Open in Devin Review

Update: gated behind encode_path_params (opt-in)

Per review, encoding is now off by default and opted into with a generator config flag:

- name: fernapi/fern-python-sdk
  config:
    encode_path_params: true
  • SDKCustomConfig: new encode_path_params (default false).
  • encode_path_param is restored to its original stringify-only behavior; percent-encoding lives in a
    new quote_path_param in the same core utility. CoreUtilities.encode_path_param emits a call to
    whichever one the flag selects, so call sites are unchanged.
  • Tests cover both helpers (test_encode_path_param, test_quote_path_param).
  • Changelog reclassified fix -> feat.

Caveat, unchanged from before: the seed/python-sdk fixtures were updated by propagating the core
utility rather than by running python seed (pypi is unreachable from my box, so poetry install for
the python generator fails). Default output is unchanged, but CI seed is the first real regeneration —
worth a look. There is also no seed fixture exercising the flag on for the same reason.

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

@nitpickybot nitpickybot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI Review Summary

Correctly fixes the missing percent-encoding in encode_path_param and propagates it to all seed fixtures. The implementation matches TS/Go/Java semantics (safe=""encodeURIComponent). Main concerns are release-note framing (this is behaviorally breaking for APIs with slash-bearing path params) and making sure no other copies of this helper were missed.

  • 🟡 1 warning(s)
  • 🔵 2 suggestion(s)

Comment thread generators/python/sdk/changes/unreleased/encode-path-parameters.yml Outdated
Comment thread generators/python/core_utilities/shared/jsonable_encoder.py
Comment thread generators/python/tests/sdk/test_jsonable_encoder.py

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

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

View in Devin Review to see 1 additional finding.

Open in Devin Review

@github-actions

github-actions Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

SDK Generation Benchmark Results

Comparing PR branch against median of 5 nightly run(s) on main (latest: 2026-07-31T05:08:23Z).

Full benchmark table (click to expand)
Generator Spec main (generator) main (E2E) PR (generator) Delta
python-sdk square 143s (n=5) 259s (n=5) 148s +5s (+3.5%)

main (generator): generator-only time via --skip-scripts (includes Docker image build, container startup, IR parsing, and code generation — this is the same Docker-based flow customers use via fern generate). main (E2E): full customer-observable time including build/test scripts (nightly baseline, informational). Delta is computed against generator-only baseline.
⚠️ = generation exited with a non-zero exit code (timing may not reflect a successful run).
Baseline from nightly runs on main (latest: 2026-07-31T05:08:23Z). Trigger benchmark-baseline to refresh.
Last updated: 2026-07-31 16:59 UTC

… config

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

Claude Code Review is paused for this repository. To reconnect it, an admin of this repository's GitHub organization (or the account owner, for personal repositories) who can also manage your Claude organization's Code Review settings needs to re-link GitHub in Code Review settings. This is a one-time step.

Tip: disable this comment in your organization's Code Review settings.

@fern-api fern-api Bot changed the title fix(python): percent-encode path parameters feat(python): add encode_path_params config to percent-encode path parameters Jul 30, 2026
willkendall01 and others added 3 commits July 31, 2026 11:29
Regenerate the path-parameters fixture via the generator (not hand-copied)
into two config folders:
- no-custom-config: default output (byte-identical relocation of old root)
- encode-path-params: encode_path_params=true, so path params route through
  quote_path_param() and percent-encode "/" and "..".

Covers the flag-on codegen path that previously had no seed snapshot.

Co-Authored-By: Claude <noreply@anthropic.com>
…angelog

Co-Authored-By: Claude <noreply@anthropic.com>
… in tests

Addresses review feedback: assert that every "/" is percent-encoded and that
already-encoded input is encoded again (a%2Fb -> a%252Fb).

Co-Authored-By: Claude <noreply@anthropic.com>
@aditya-arolkar-swe
aditya-arolkar-swe merged commit bbe385c into main Jul 31, 2026
82 checks passed
@aditya-arolkar-swe
aditya-arolkar-swe deleted the devin/1785416411-python-encode-path-params branch July 31, 2026 20:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants