Skip to content

fix(cli): sanitize test_name on GET, DELETE, and rebuild - #7034

Open
Oskii wants to merge 1 commit into
google:mainfrom
Oskii:fix/dev-test-path-sanitizer
Open

fix(cli): sanitize test_name on GET, DELETE, and rebuild#7034
Oskii wants to merge 1 commit into
google:mainfrom
Oskii:fix/dev-test-path-sanitizer

Conversation

@Oskii

@Oskii Oskii commented Sep 6, 2026

Copy link
Copy Markdown

Fixes #7033

What

adk web can create, read, delete, and rebuild agent test JSON files under <agent>/tests/. create_test already ran os.path.basename on test_name so a name cannot leave that folder. GET, DELETE, and rebuild did not.

On main @ b018062, this DELETE removes a file next to tests/, not inside it:

DELETE /dev/apps/<app>/tests/%2e%2e%2foutside.json

The path param decodes to ../outside.json. os.path.join(tests_dir, test_name) follows it. GET reads the same way. POST .../tests/rebuild?test_name=../outside.json passes that path into rebuild_tests.

This is the local unauthenticated dev server. Default bind is loopback. It still matters if someone uses --host 0.0.0.0, or if anything else can hit those routes.

After the fix, those names are stripped to outside.json and resolved only under tests/. A missing file is 404. The file outside tests/ is left alone.

Why

I was reading the test-file routes next to test_create_test. The create handler has an explicit comment about directory traversal. I tried the same name on DELETE with a percent-encoded ../ and the file outside tests/ was gone.

How

One helper, _sanitize_test_filename, used by create, get, delete, and rebuild. Same basename plus the existing .json suffix rule. No new public API.

Testing

Added test_delete_test_rejects_path_traversal and test_rebuild_single_test_rejects_path_traversal. Existing create/get/delete/rebuild cases still pass.

pytest tests/unittests/cli/test_adk_web_server_tests.py::test_delete_test_rejects_path_traversal tests/unittests/cli/test_adk_web_server_tests.py::test_rebuild_single_test_rejects_path_traversal tests/unittests/cli/test_adk_web_server_tests.py::test_delete_test tests/unittests/cli/test_adk_web_server_tests.py::test_get_test_content tests/unittests/cli/test_adk_web_server_tests.py::test_create_test tests/unittests/cli/test_adk_web_server_tests.py::test_rebuild_single_test -q

6 passed.

Testing plan

  • Unit tests above (TestClient, no live adk web).
  • I did not run the full tox matrix on this machine.

create_test already used basename so a test name cannot leave tests/.
The other three endpoints joined the name as given. Share one helper.

Fixes google#7033
@google-cla

google-cla Bot commented Sep 6, 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.

@Oskii

Oskii commented Sep 6, 2026

Copy link
Copy Markdown
Author

I added the required CLA from this github account :)

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.

adk web GET/DELETE test endpoints skip the create_test path sanitiser

2 participants