Repository navigation
adk web GET/DELETE test endpoints skip the create_test path sanitiser #7033
Description
Expected Behavior
create_test strips directories from test_name with os.path.basename so a name cannot leave the app tests/ folder.
GET, DELETE, and rebuild of a single test should use the same rule.
Actual Behavior
On main @ b018062, only create_test calls os.path.basename. delete_test, get_test_content, and rebuild_app_tests join test_name as given.
A percent-encoded path segment ../outside.json (%2e%2e%2foutside.json) on DELETE/GET is joined onto tests/ and can read or remove a JSON file in the agent directory, outside tests/.
rebuild?test_name=../outside.json does the same for the rebuild path.
This is the local adk web server. It is unauthenticated. Default bind is loopback. It still matters when --host 0.0.0.0 is used, or when anything else can hit those routes.
Steps to Reproduce
adk web(or the TestClient intests/unittests/cli/test_adk_web_server_tests.py)- Put
outside.jsonin the agent directory, not intests/ DELETE /dev/apps/<app>/tests/%2e%2e%2foutside.json- On current
main, that file is removed. After sanitising withbasename, the request 404s and the file stays.
I can send a PR that shares one helper with create_test and adds those cases to test_adk_web_server_tests.py.
Activity
surajksharma07 commentedon Sep 7, 2026
@Oskii Delete/get/rebuild all skip the basename check create_test already does, confirmed it lands one directory above tests/. Tried the shared-helper approach from #7034 locally and it closes the gap cleanly, traversal attempts now just 404 instead of touching anything outside tests/. Only thing worth double checking before review and merge is a dedicated traversal test on the GET path specifically since your testing plan calls out delete and rebuild but not get by name.
Oskii commentedon Sep 7, 2026
@surajksharma07 Added a dedicated
test_get_test_rejects_path_traversalon #7034 (same encoded../as DELETE, own test so GET is not only a side assertion). Rebuild already had its own case.surajksharma07 commentedon Sep 10, 2026
@Oskii Heads up before you wait on that CI approval. This gap got closed independently on main (a separate internal commit landed 2026-09-09, adds a shared path-resolving helper across create/get/delete/rebuild, same effect as your fix plus a resolve()-based containment check). It hasn't shipped in a tagged release yet so nothing wrong with your diagnosis or patch, just means #7034 would be redundant now. Will flag #7034 for closing in favor of the landed fix rather than have you keep chasing CI on it.
Oskii commentedon Sep 11, 2026
Huh, so someone internally took my work and opened their own PR, so now I won't get attribution for finding it?
I reported this vulnerability, provided a working fix, and added further tests at your request. Using that contribution to produce an internal replacement without acknowledgment is pretty brutal.
surajksharma07 commentedon Sep 16, 2026
@Oskii Fair and you're right to push back. Double-checked the actual timestamps and your issue/PR went up 2026-09-06, the internal fix landed 2026-09-09, so it came after your report not before like I implied.
Will make sure the comment closing #7034 explicitly credits you by name for finding this and writing the fix and raising the sequencing with the team so next time an external PR beats us to it we review and merge that instead of quietly reimplementing it. Appreciate you holding us to this.
Team is looking more into it.
Oskii commentedon Sep 29, 2026
Thank you @surajksharma07 I appreciate the response
GWeale commentedon Nov 9, 2026
fixed in 4c2bafb, which shipped in v2.10.0. get, delete and rebuild now use the same path check as create, so a test name can't escape the tests folder anymore. thanks again @Oskii for the report and for #7034. please open a new issue if you find another way around it.
github-actions commentedon Nov 9, 2026
Are you satisfied with the resolution of your issue?
Yes
No