fix : ReadFileTool shells out on ranged reads and interpolates untrusted path

[hamdi-sakly]

Link to Issue or Description of Change

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

• Closes: ReadFileTool shells out on ranged reads and interpolates untrusted path #5530

2. Description of change:

Problem:

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

# Vulnerable codecmd=f"cat -n '{path}' | sed -n '{sed_range}p'"res=awaitself._environment.execute(cmd)

The user-supplied path is interpolated directly into the shell command without
any sanitization. A path containing shell-sensitive characters (e.g. single quotes,
semicolons) is interpreted as shell syntax, allowing unintended commands to be
executed as a side effect of a file-read operation.

Solution:

Used Python's built-in shlex.quote() to safely escape the path before embedding
it in the shell command. shlex.quote() ensures the shell always treats the entire
path as a single string argument regardless of its content.

# Fixed codeimportshlexsafe_path=shlex.quote(path)
cmd=f"cat -n {safe_path} | sed -n '{sed_range}p'"res=awaitself._environment.execute(cmd)

shlex is a Python standard library module — no new dependencies are required.

---

Testing Plan

Unit Tests:

• I have added or updated unit tests for my change.
• All unit tests pass locally.

importasynciofrompathlibimportPathimporttempfilefromgoogle.adk.environment._local_environmentimportLocalEnvironmentfromgoogle.adk.tools.environment._toolsimportReadFileToolasyncdefmain():
withtempfile.TemporaryDirectory() astd:
env=LocalEnvironment(working_dir=Path(td))
awaitenv.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=awaittool.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())
awaitenv.close()
asyncio.run(main())

The marker.txt file is no longer created, confirming the injected shell
command is no longer executed.

[sasha-gitg]

This is great, thanks! Can you add a few unit tests?

[milaforge]

Even if path is safely shell-quoted, the ranged-read path still interpolates end_line into sed -n '...p'.

[milaforge]

Now if you replace the previous POC with the

Updated POC

importasynciofrompathlibimportPathimporttempfilefromgoogle.adk.environment._local_environmentimportLocalEnvironmentfromgoogle.adk.tools.environment._toolsimportReadFileToolasyncdefmain():
withtempfile.TemporaryDirectory() astd:
env=LocalEnvironment(working_dir=Path(td))
awaitenv.initialize()
target=Path(td) /"sample.txt"target.write_text("line1\nline2\nline3\n", encoding="utf-8")
marker=Path(td) /"marker.txt"injected_end_line=f"1'; touch {marker}; echo '"tool=ReadFileTool(env)
result=awaittool.run_async(
args={"path": "sample.txt", "end_line": injected_end_line},
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())
awaitenv.close()
asyncio.run(main())

You could see that the current implementation of the file-read API could cause unintended side effects.

#5558 is how I would fix it.