Skip to content

SEC-774: fix(utils): percent-encode path parameters so an ID can't change which API route is called - #1230

Open
alexb-openrouter wants to merge 2 commits into
mainfrom
devin/1793837556-sec-774-encode-path-params
Open

alexb-openrouter wants to merge 2 commits into
mainfrom
devin/1793837556-sec-774-encode-path-params

Conversation

@alexb-openrouter

Copy link
Copy Markdown
Contributor

The risk in plain terms

  • Apps often pass an ID from an end user (or from a model's tool call) straight into an SDK method, for example client.files.download(file_id).
  • The SDK put that ID into the URL as-is. httpx then cleans up ../ before sending, so an ID like ../keys?x= turned GET /api/v1/files/{file_id}/content into GET /api/v1/keys. files.delete("../keys/<hash>") became DELETE /api/v1/keys/<hash>.
  • That request goes out with the app's own API key. Whoever controls the ID can reach any other /api/v1 route that key can use. It doesn't cross into other accounts, but it does let an outside party pick which route the app calls.
  • This affects every operation that has a path parameter (86 path params across 48 path templates). The TypeScript SDK already percent-encodes path params, so it doesn't have this problem.

How this PR fixes it

  • generate_url() now percent-encodes every path parameter value with urllib.parse.quote(value, safe=""), the same encoding as the TypeScript SDK's encodeSimple with charEncoding: "percent". /, ?, #, %, spaces and non-ASCII characters are encoded, so a value always stays inside its own path segment.
  • "." and ".." have no characters to encode and would still be treated as dot segments, so they now raise ValueError before any request is sent. No real ID has either value.
# before
path = path.replace("{" + key + "}", value, 1)
# after
path = path.replace("{" + key + "}", _encode_path_param(value), 1)

Keeping the fix when the SDK is regenerated

src/openrouter/utils/url.py is a Speakeasy-generated file. The fix is kept as a Speakeasy patch file at .speakeasy/patches/src/openrouter/utils/url.py.patch, which Speakeasy re-applies after every generation. If a future template change conflicts with it, generation fails with conflict markers instead of silently dropping the fix.

Options I checked and why I didn't use them:

  • gen.yaml option: the Python generator has none. The pinned CLI (1.803.2) only supports x-speakeasy-path-encoding-override for other languages; for Python it reports "not currently supported for this language".
  • SDK hook (_hooks/): a before_request hook gets the httpx.Request after httpx has already removed the ../, so the original value is gone by then.
  • Editing the generated file (persistent edits): gen.yaml has persistentEdits: {}, which isn't enabled. A local regeneration with only the edit, and no patch file, reverted the file to the unencoded version.
  • .genignore: this would freeze the whole file and drop future generator updates to it.

What changed

  • src/openrouter/utils/url.py: adds _encode_path_param() and uses it in generate_url().
  • .speakeasy/patches/src/openrouter/utils/url.py.patch: the same change as a patch file, so regeneration keeps it.
  • tests/test_path_param_encoding.py: regression tests that use a local 127.0.0.1 mock server and a fake key.

Testing

  • tests/test_path_param_encoding.py checks the exact request line the mock server receives:
    • test_traversal_in_file_id_stays_on_files_download_route: files.download("../keys?x=") sends GET /api/v1/files/..%2Fkeys%3Fx%3D/content.
    • test_traversal_in_file_id_stays_on_files_delete_route: files.delete("../keys/abc") sends DELETE /api/v1/files/..%2Fkeys%2Fabc.
    • test_normal_file_id_is_unchanged, test_spaces_unicode_and_fragment_are_percent_encoded, test_dot_segment_ids_are_rejected_before_sending, test_generate_url_percent_encodes_path_params (also covers %2e%2e becoming %252e%252e).
  • On main, 8 of the 10 tests fail. For example, the download test receives ('GET', '/api/v1/keys'). The two that pass on main are the normal-ID test and the space/unicode case in the generate_url test. With this PR, all 10 pass.
  • Ran mypy src, pyright src, pylint src --rcfile pylintrc (10.00/10) and pytest (15 passed), the same steps as PR Validation.
  • Regeneration check: ran the pinned Speakeasy CLI 1.803.2 (speakeasy run --skip-versioning --skip-upload-spec --skip-compile, registry upload removed from the local workflow copy) starting from the unpatched url.py. It logged Applying patch files..., and the generated url.py was byte-identical to this PR's version. The only other changes were the usual gen.lock and workflow.lock updates, which are not committed here.

Existing-PR search: searched open PRs and PRs from the last 90 days for path param, encode, url.py, percent, generate_url, traversal and SEC-774. None make this change. #1127 and #1118 touch publish workflows and overlays, not url.py or generation config, so they don't conflict with this PR.

Fixes SEC-774

Link to Devin session: https://openrouter.devinenterprise.com/sessions/7437b67a506642b38fc4a5ffd44c4f6c
Open in Devin Desktop: https://openrouter.devinenterprise.com/desktop/session/7437b67a506642b38fc4a5ffd44c4f6c?variant=devin
Requested by: @alexb-openrouter

alexb-openrouter and others added 2 commits November 5, 2026 00:15
…quest route

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…n with a patch file

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

devin-ai-integration Bot commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

I'll fix CI failures and address comments from users with write access that start with 'Devin'.

  • Disable automatic comment, CI, and merge conflict monitoring

This branch has not been deployed

No deployments
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.

1 participant