Skip to content

fix: reject RestApiTool path params with '.' or '..' segments - #7066

Open
artemkulyk wants to merge 2 commits into
google:mainfrom
artemkulyk:fix/rest-api-tool-dot-segment-path
Open

fix: reject RestApiTool path params with '.' or '..' segments#7066
artemkulyk wants to merge 2 commits into
google:mainfrom
artemkulyk:fix/rest-api-tool-dot-segment-path

Conversation

@artemkulyk

Copy link
Copy Markdown

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

1. Link to an existing issue (if applicable):

Problem:
RestApiTool percent-encodes path parameters with quote(value, safe="") and documents that this prevents a model-supplied value from steering the request onto an undeclared path. quote() never encodes ., so ../../admin/secret is sent as ..%2F..%2Fadmin%2Fsecret. Backends that decode %2F and then merge dot-segments can dispatch that request — with the tool's credentials — to a path the OpenAPI spec never declared.

Solution:
Reject a path parameter if any /- or \-separated segment is exactly . or .., then keep quote(safe=""). Slash-containing IDs such as foo/bar are still encoded as foo%2Fbar. Invalid values fail closed in call() as an {"error": ...} result; no HTTP request is sent.

Testing Plan

Unit Tests:

  • I have added or updated unit tests for my change.
  • All unit tests pass locally.
.venv/bin/pytest tests/unittests/tools/openapi_tool/openapi_spec_parser/test_rest_api_tool.py -q
76 passed

New coverage: _encode_path_param rejects ., .., ../../admin/v1/tenants, foo/../bar, ..\admin, ./x; still encodes foo/bar, ?/#, file..txt, .gitignore, ok.txt. call() with a traversal name returns an error and does not call _request.

Manual End-to-End (E2E) Tests:

OpenAPI spec GET /files/{name} against a local HTTP server:

  • args={"name": "../../admin/secret"}{"error": "... parent-directory segments."}, no wire request.
  • args={"name": "ok.txt"}GET /files/ok.txt succeeds.

Checklist

  • I have read the CONTRIBUTING.md document.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.
  • I have manually tested my changes end-to-end.
  • Any dependent changes have been merged and published in downstream modules.

Additional context

Google VRP 557701521 was closed as Infeasible (not tracked as a security bug) with a request to file this publicly. I have signed the Google CLA.

Made with Cursor

@google-cla

google-cla Bot commented Sep 9, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

quote(safe="") leaves '.' literal, so model-supplied '..' still reaches
the wire as encoded-slash traversal. Reject those segments before encoding
so backends that decode %2F cannot leave the declared path.
@artemkulyk
artemkulyk force-pushed the fix/rest-api-tool-dot-segment-path branch from 249777b to 4cba603 Compare September 9, 2026 00:23
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.

RestApiTool path parameters with '.' / '..' still reach the wire after quote(safe="")

2 participants