ReadFileTool shells out on ranged reads and interpolates untrusted path

[milaforge]

ReadFileTool shells out for ranged reads (start_line > 1 or end_line is set) instead of reading the file directly.

adk-python/src/google/adk/tools/environment/_tools.py

Lines 190 to 191 in 3e282d2

	cmd = f"cat -n '{path}' | sed -n '{sed_range}p'"
	res = await self._environment.execute(cmd)

As a result, the ranged-read path is not a pure file-read operation, and paths containing shell-sensitive characters can be interpreted unexpectedly.

Steps to Reproduce:

1. Install ADK.
2. Run the minimal reproduction below.
3. Call ReadFileTool with start_line=2 and a path containing a single quote.
4. Observe that the ranged-read path invokes shell execution instead of direct file I/O.

Expected Behavior:
ReadFileTool should always read file contents directly and should not invoke shell commands based on the requested line range.

Observed Behavior:
When start_line or end_line is provided, ReadFileTool builds and runs a shell command using the provided path.

Environment Details:

• ADK Library Version (pip show google-adk): 1.31.0
• Desktop OS:** Linux
• Python Version (python -V): Python 3.14.4

Model Information:

• Are you using LiteLLM: No
• Which model is being used: N/A

---

🟡 Optional Information

Regression:
Unknown / N/A

Logs:
text file.

No special logs required. The issue is reproducible from the current
ReadFileTool implementation.

Screenshots / Video:
N/A

Additional Context:
This looks fixable by removing the shell-based ranged-read path and handling line slicing entirely in Python.

Minimal Reproduction Code:
Please provide a code snippet or a link to a Gist/repo that isolates the issue.

import asyncio
from pathlib import Path
import tempfile

from google.adk.environment._local_environment import LocalEnvironment
from google.adk.tools.environment._tools import ReadFileTool


async def main():
 with tempfile.TemporaryDirectory() as td:
 env = LocalEnvironment(working_dir=Path(td))
 await env.initialize()

 target = Path(td) / "sample.txt"
 target.write_text("line1\nline2\nline3\n", encoding="utf-8")

 marker = Path(td) / "marker.txt"
 # ReadFileTool is expected to treat `path` as data, not as shell syntax.
 # This payload closes the quoted filename used by the ranged-read path,
 # injects an extra command, and reopens the quote to keep the shell happy.
 injected_path = f"sample.txt'; touch {marker}; echo '"

 tool = ReadFileTool(env)
 result = await tool.run_async(
 # `start_line=2` is important because it triggers the code path that
 # shells out through `cat ... | sed ...` instead of doing pure Python
 # file I/O.
 args={"path": injected_path, "start_line": 2},
 tool_context=None,
 )

 print(result)
 # If this prints True, a file-read API caused an unintended side effect.
 # That demonstrates the bug: the path was interpreted by the shell.
 print("marker exists:", marker.exists())
 await env.close()


asyncio.run(main())

How often has this issue occurred?:

• Always (100%)

[surajksharma07]

@failuresmith The cat -n '{path}' | sed -n ... path is interpolating the untrusted path directly into a shell command whenever start_line > 1 or end_line is set.

The pure-Python fallback below it already handles ranged reads correctly, so the fix is straightforward: remove the sed-based block entirely and let all reads go through self._environment.read_file(path).

PR #5537 has been linked and looks like the right direction — could you give it a review and test once it lands and confirm it resolves the injection on your side?
Please do thorough testing before we push it through!

[milaforge]

@surajksharma07 I verified this blocks the original path-based PoC, but I don’t think it fully fixes the issue. The ranged-read path still shells out, and end_line is still interpolated into the sed command; I was able to reproduce command execution through that path after this patch.

I think the right fix is to remove the cat | sed branch entirely and always use read_file() plus the existing Python slicing logic. That eliminates the injection surface and also fixes the current missing-file behavior for ranged reads.

I’ve put together a follow-up patch with tests here.

[surajksharma07]

@failuresmith PR #5537 only fixes the path vector.

The end_line injection is still open because sed_range is built as f'{start},{end_line}' and dropped directly into the shell command unquoted: sed -n '{sed_range}p'.

Since end_line has no type validation before it hits execute(), passing something like end_line="999; touch /tmp/pwned" still executes.

Quoting path shifted the attack surface — it didn't close it.

PR #5558 is the correct fix.
Removing the shell branch entirely means there's nothing left to inject into.
The pure-Python path with read_file() + list slicing already handles all the same cases: start_line, end_line, range validation, line numbering, and missing-file errors.
The end_line integer check you added on top is the right defensive layer — returning 'end_line must be an integer if provided' before any execution path is reached is the cleanest way to close that last ambiguity.

The test coverage in #5558 also looks solid — regression for normal ranged reads, correct missing-file behavior (which the sed path was silently swallowing by returning ok with empty content), and the non-integer end_line input all covered.

Worth flagging: this is still unpatched as of v1.32.0 — neither #5537 nor #5558 has landed yet.

Since this is arbitrary command execution reachable through a core tool, #5558 should take priority.

Team is looking more into it.
Thanks !