Skip to content

fix: keep local environment file access in workspace - #5870

Closed
he-yufeng wants to merge 1 commit into
google:mainfrom
he-yufeng:fix/local-environment-path-boundary
Closed

fix: keep local environment file access in workspace#5870
he-yufeng wants to merge 1 commit into
google:mainfrom
he-yufeng:fix/local-environment-path-boundary

Conversation

@he-yufeng

@he-yufeng he-yufeng commented May 27, 2026

Copy link
Copy Markdown
Contributor

Fixes #5869

Summary

  • resolve LocalEnvironment read/write paths against the workspace root
  • reject relative traversal and absolute paths outside working_dir
  • keep absolute paths inside working_dir working
  • add regression coverage for relative and absolute escape attempts

Tests

  • python -m pytest tests\unittests\tools\test_local_environment.py -q -p no:cacheprovider --basetemp .tmp\pytest
  • python -m ruff check src\google\adk\environment\_local_environment.py tests\unittests\tools\test_local_environment.py
  • python -m pyink --check src\google\adk\environment\_local_environment.py tests\unittests\tools\test_local_environment.py
  • python -m py_compile src\google\adk\environment\_local_environment.py tests\unittests\tools\test_local_environment.py
  • git diff --check

@adk-bot adk-bot added the tools [Component] This issue is related to tools label May 27, 2026
@er3b07 er3b07 mentioned this pull request May 27, 2026
Closed
@he-yufeng
he-yufeng force-pushed the fix/local-environment-path-boundary branch from 04a49ff to 34870c2 Compare May 27, 2026 18:11
@rohityan rohityan self-assigned this May 27, 2026
@rohityan rohityan added the needs review [Status] The PR/issue is awaiting review from the maintainer label May 27, 2026
@rohityan
rohityan requested a review from wyf7107 May 27, 2026 19:22
@rohityan

rohityan commented May 27, 2026

Copy link
Copy Markdown
Collaborator

Hi @he-yufeng , Thank you for your contribution! We appreciate you taking the time to submit this pull request. Your PR has been received by the team and is currently under review. We will provide feedback as soon as we have an update to share.

@rohityan

rohityan commented May 27, 2026

Copy link
Copy Markdown
Collaborator

Hi @wyf7107 , can you please review this.

@er3b07

er3b07 commented May 28, 2026 •
edited
Loading

Copy link
Copy Markdown

Hi @he-yufeng and team, just verified that the fix corrects successfully the path traversal
#5869

@he-yufeng

he-yufeng commented Jun 12, 2026

Copy link
Copy Markdown
Contributor Author

Gentle ping on this path-boundary fix. The branch is green, and #5869 verified the path traversal behavior after the patch.

The change stays limited to keeping local environment file access inside the workspace root. Happy to make any naming or API-shape adjustment if maintainers prefer a different boundary helper.

@he-yufeng
he-yufeng force-pushed the fix/local-environment-path-boundary branch 2 times, most recently from 34870c2 to 3f2af35 Compare June 12, 2026 01:15
@he-yufeng

he-yufeng commented Jun 12, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto the current upstream/main and resolved the LocalEnvironment path-resolution conflict.

The conflict was caused by upstream changing the helper to return Path; the branch now keeps that return type while preserving the workspace-boundary check.

Validation:

  • python -m py_compile src/google/adk/environment/_local_environment.py tests/unittests/tools/test_local_environment.py
  • PYTHONPATH=src python -m pytest tests/unittests/tools/test_local_environment.py -q (10 passed)
  • git diff --check upstream/main..HEAD

Current head: 3f2af35e

@he-yufeng
he-yufeng force-pushed the fix/local-environment-path-boundary branch 2 times, most recently from 931452c to 29d160b Compare June 12, 2026 13:28
@he-yufeng

he-yufeng commented Jun 12, 2026

Copy link
Copy Markdown
Contributor Author

Rebased this on current main and re-ran the focused local checks:

  • PYTHONPATH=src python -m pytest tests\unittests\tools\test_local_environment.py -q -p no:cacheprovider --basetemp .tmp\pytest-5870 -> 10 passed
  • python -m py_compile on the touched files
  • python -m ruff check on the touched files
  • python -m pyink --check on the touched files
  • git diff --check upstream/main..HEAD

All passed locally.

@he-yufeng
he-yufeng force-pushed the fix/local-environment-path-boundary branch 2 times, most recently from 1d61969 to 77f9a9b Compare June 12, 2026 21:58
@GWeale GWeale assigned GWeale and unassigned rohityan Jun 15, 2026
@he-yufeng
he-yufeng force-pushed the fix/local-environment-path-boundary branch 3 times, most recently from e5c32d2 to 3c3cdd3 Compare June 18, 2026 06:16
@he-yufeng
he-yufeng force-pushed the fix/local-environment-path-boundary branch from 3c3cdd3 to f08eadc Compare June 18, 2026 10:14
@er3b07

er3b07 commented Jun 30, 2026

Copy link
Copy Markdown

Hi @GWeale @wyf7107 — gentle ping on #5870 (Fixes #5869).

I re-tested today against google-adk 2.3.0 on PyPI and current main: the path traversal behavior described in #5869 is still reproducible. I also confirmed the fix on this branch blocks the PoC as expected.

Whenever there is an update on review timing, I am happy to help re-test or provide any additional details.

Thank you for your time.

@wuliang229 wuliang229 self-assigned this Jul 9, 2026
@wuliang229

wuliang229 commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator

Merged in commit f41bc79

@wuliang229 wuliang229 closed this Jul 13, 2026
sasha-gitg added a commit to sasha-gitg/adk-python-public that referenced this pull request Aug 5, 2026
@sasha-gitg
fix: keep local environment file access in workspace (backport)
aa13ba3 
Backport of the `_local_environment.py` / `test_local_environment.py`
portion of the following commit from `main`, released in v2.5.0:

  f41bc79
    (squashed copybara commit; upstream PR google#5870, fixes google#5869)

Only the two files above are taken from that commit -- the rest of it is
unrelated (a2a sample, PR-triage agent, transfer_to_agent_tool).

Adapted rather than cherry-picked directly: on `main` `_resolve_path`
returns a `Path`, while on v1 it returns `str` and `_sync_read` /
`_sync_write` take `str`. The containment check is the same; the v1
return type is preserved to keep the change minimal.

`_resolve_path` returned absolute paths verbatim:

    if os.path.isabs(path):
      return path

so `working_dir` was advisory only, and `read_file` / `write_file` --
and therefore the ReadFile, WriteFile and EditFile tools -- could reach
any path the process could access.

Paths are now resolved against `working_dir` and rejected with a
`ValueError` if they escape it. Absolute paths that stay inside
`working_dir` keep working.
sasha-gitg added a commit to sasha-gitg/adk-python-public that referenced this pull request Aug 5, 2026
@sasha-gitg
fix: keep local environment file access in workspace (backport)
d4fde85 
Backport of the `_local_environment.py` / `test_local_environment.py`
portion of the following commit from `main`, released in v2.5.0:

  f41bc79
    (squashed copybara commit; upstream PR google#5870, fixes google#5869)

Only the two files above are taken from that commit -- the rest of it is
unrelated (a2a sample, PR-triage agent, transfer_to_agent_tool).

Adapted rather than cherry-picked directly: on `main` `_resolve_path`
returns a `Path`, while on v1 it returns `str` and `_sync_read` /
`_sync_write` take `str`. The containment check is the same; the v1
return type is preserved to keep the change minimal.

`_resolve_path` returned absolute paths verbatim:

    if os.path.isabs(path):
      return path

so `working_dir` was advisory only, and `read_file` / `write_file` --
and therefore the ReadFile, WriteFile and EditFile tools -- could reach
any path the process could access.

Paths are now resolved against `working_dir` and rejected with a
`ValueError` if they escape it. Absolute paths that stay inside
`working_dir` keep working.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wyf7107 wyf7107 Awaiting requested review from wyf7107

Assignees

@wuliang229 wuliang229

@GWeale GWeale

Labels

needs review [Status] The PR/issue is awaiting review from the maintainer tools [Component] This issue is related to tools

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

(CWE-22) Path traversal in experimental LocalEnvironment allows file access outside workspace

6 participants

@he-yufeng @rohityan @er3b07 @wuliang229 @GWeale @adk-bot