Skip to content

Commit 3c0fb65

Browse files
wuliang229copybara-github
authored andcommitted
fix(environment): add integer validation in ReadFileTool
Add runtime integer type validation for start_line and end_line arguments (including boolean exclusion) to prevent slice type errors and ensure safe execution. Co-authored-by: Liang Wu <wuliang@google.com> PiperOrigin-RevId: 945188902
1 parent e5fdb51 commit 3c0fb65

2 files changed

Lines changed: 112 additions & 1 deletion

File tree

src/google/adk/tools/environment/_read_file_tool.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,11 @@
3737
logger = logging.getLogger('google_adk.' + __name__)
3838

3939

40+
def _is_valid_line_number(value: Any) -> bool:
41+
"""Returns True when *value* is a non-bool integer."""
42+
return isinstance(value, int) and not isinstance(value, bool)
43+
44+
4045
@experimental
4146
class ReadFileTool(BaseTool):
4247
"""Read a file from the environment."""
@@ -101,6 +106,12 @@ async def run_async(
101106
return {'status': 'error', 'error': '`path` is required.'}
102107
start_line = args.get('start_line')
103108
end_line = args.get('end_line')
109+
for name, value in (('start_line', start_line), ('end_line', end_line)):
110+
if value is not None and not _is_valid_line_number(value):
111+
return {
112+
'status': 'error',
113+
'error': f'`{name}` must be an integer if provided.',
114+
}
104115

105116
try:
106117
# TODO: Avoid loading the entire file into memory to prevent OOM on large files.

tests/unittests/tools/environment/test_read_file_tool.py

Lines changed: 101 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,17 @@
1212
# See the License for the specific language governing permissions and
1313
# limitations under the License.
1414

15+
"""Tests for ReadFileTool."""
16+
1517
from pathlib import Path
1618
from typing import Optional
1719

1820
from google.adk.environment._base_environment import BaseEnvironment
1921
from google.adk.environment._base_environment import ExecutionResult
20-
from google.adk.tools.environment._tools import ReadFileTool
22+
from google.adk.environment._local_environment import LocalEnvironment
23+
from google.adk.tools.environment._read_file_tool import ReadFileTool
2124
import pytest
25+
import pytest_asyncio
2226

2327

2428
class _StubEnvironment(BaseEnvironment):
@@ -94,3 +98,99 @@ async def test_read_file_with_line_range_treats_shell_payload_as_literal_path():
9498
'error': f'File not found: {payload}',
9599
}
96100
assert environment.execute_calls == []
101+
102+
103+
@pytest_asyncio.fixture(name='env')
104+
async def _env(tmp_path: Path):
105+
"""Create and initialize a LocalEnvironment backed by a temp directory."""
106+
environment = LocalEnvironment(working_dir=tmp_path)
107+
await environment.initialize()
108+
yield environment
109+
await environment.close()
110+
111+
112+
class TestReadFileTool:
113+
"""Tests for ReadFileTool behavior."""
114+
115+
@pytest.mark.asyncio
116+
async def test_read_file_with_line_range_returns_selected_lines(
117+
self, env: LocalEnvironment
118+
):
119+
"""Reads the requested line range and preserves line numbers."""
120+
await env.write_file('sample.txt', 'line1\nline2\nline3\n')
121+
122+
tool = ReadFileTool(env)
123+
result = await tool.run_async(
124+
args={'path': 'sample.txt', 'start_line': 2, 'end_line': 3},
125+
tool_context=None,
126+
)
127+
128+
assert result == {
129+
'status': 'ok',
130+
'content': ' 2\tline2\n 3\tline3\n',
131+
'total_lines': 3,
132+
}
133+
134+
@pytest.mark.asyncio
135+
async def test_read_file_with_line_range_missing_file_returns_error(
136+
self, env: LocalEnvironment
137+
):
138+
"""Returns a missing-file error for ranged reads."""
139+
tool = ReadFileTool(env)
140+
141+
result = await tool.run_async(
142+
args={'path': 'missing.txt', 'start_line': 2},
143+
tool_context=None,
144+
)
145+
146+
assert result == {
147+
'status': 'error',
148+
'error': 'File not found: missing.txt',
149+
}
150+
151+
@pytest.mark.asyncio
152+
async def test_read_file_rejects_non_integer_end_line(
153+
self, env: LocalEnvironment
154+
):
155+
"""Rejects non-integer line numbers without executing shell syntax."""
156+
await env.write_file('sample.txt', 'line1\nline2\n')
157+
marker = env.working_dir / 'marker.txt'
158+
injected_end_line = f"1'; touch {marker}; echo '"
159+
160+
tool = ReadFileTool(env)
161+
result = await tool.run_async(
162+
args={'path': 'sample.txt', 'end_line': injected_end_line},
163+
tool_context=None,
164+
)
165+
166+
assert result == {
167+
'status': 'error',
168+
'error': '`end_line` must be an integer if provided.',
169+
}
170+
assert not marker.exists()
171+
172+
@pytest.mark.asyncio
173+
async def test_read_file_rejects_boolean_line_numbers(
174+
self, env: LocalEnvironment
175+
):
176+
"""Rejects boolean values for start_line and end_line."""
177+
await env.write_file('sample.txt', 'line1\nline2\n')
178+
179+
tool = ReadFileTool(env)
180+
res_start = await tool.run_async(
181+
args={'path': 'sample.txt', 'start_line': True},
182+
tool_context=None,
183+
)
184+
res_end = await tool.run_async(
185+
args={'path': 'sample.txt', 'end_line': False},
186+
tool_context=None,
187+
)
188+
189+
assert res_start == {
190+
'status': 'error',
191+
'error': '`start_line` must be an integer if provided.',
192+
}
193+
assert res_end == {
194+
'status': 'error',
195+
'error': '`end_line` must be an integer if provided.',
196+
}

0 commit comments

Comments
 (0)