Skip to content

Commit f00fc5a

Browse files
adilburaksencopybara-github
authored andcommitted
fix: confine docs-agent file tools to the managed repos directory
Merge #5996 The `adk_documentation` sample tools (`read_local_git_repo_file_content`, `list_directory_contents`, `search_local_git_repo`, and the file-writing step of `create_pull_request_from_changes`) are invoked by autonomous agents (`adk_docs_updater`, `adk_release_analyzer`) that run non-interactively in GitHub Actions and process untrusted input — GitHub issue bodies and release diffs. This change keeps the tools within their intended sandbox by resolving symlinks and `..` segments, then requiring results to stay inside the managed repositories directory. PiperOrigin-RevId: 947146756
1 parent 0bd7ea0 commit f00fc5a

2 files changed

Lines changed: 176 additions & 1 deletion

File tree

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
# Copyright 2026 Google LLC
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
15+
"""Tests that the file/search tools stay confined to LOCAL_REPOS_DIR_PATH.
16+
17+
These tools are driven by an LLM that processes untrusted input (issue bodies,
18+
release diffs), so a path that escapes the managed repositories directory must
19+
be rejected even though it is absolute.
20+
"""
21+
22+
import os
23+
import tempfile
24+
import unittest
25+
from unittest import mock
26+
27+
# Set GITHUB_TOKEN before import since settings.py validates it at import time.
28+
os.environ.setdefault("GITHUB_TOKEN", "test-token")
29+
30+
# E402 is required because GITHUB_TOKEN must be set before this import.
31+
from adk_documentation import tools # noqa: E402
32+
33+
34+
class PathConfinementTest(unittest.TestCase):
35+
36+
@classmethod
37+
def setUpClass(cls):
38+
cls.sandbox = tempfile.mkdtemp(prefix="adk_repos_")
39+
40+
def setUp(self):
41+
self.repo = os.path.join(self.sandbox, "adk-docs")
42+
os.makedirs(self.repo, exist_ok=True)
43+
self.inside_file = os.path.join(self.repo, "index.md")
44+
with open(self.inside_file, "w", encoding="utf-8") as f:
45+
f.write("hello")
46+
47+
# Dynamically mock the constant to isolate setting from sys.modules cache
48+
self.patcher = mock.patch(
49+
"adk_documentation.tools.LOCAL_REPOS_DIR_PATH", self.sandbox
50+
)
51+
self.patcher.start()
52+
53+
def tearDown(self):
54+
self.patcher.stop()
55+
56+
def test_read_inside_sandbox_succeeds(self):
57+
res = tools.read_local_git_repo_file_content(self.inside_file)
58+
self.assertEqual(res["status"], "success")
59+
60+
def test_read_outside_sandbox_is_denied(self):
61+
for path in ("/etc/passwd", "/proc/self/environ"):
62+
res = tools.read_local_git_repo_file_content(path)
63+
self.assertEqual(res["status"], "error", path)
64+
self.assertIn("Access denied", res["error_message"])
65+
66+
def test_read_symlink_escape_is_denied(self):
67+
link = os.path.join(self.repo, "sneaky")
68+
if not os.path.lexists(link):
69+
os.symlink("/etc/passwd", link)
70+
res = tools.read_local_git_repo_file_content(link)
71+
self.assertEqual(res["status"], "error")
72+
self.assertIn("Access denied", res["error_message"])
73+
74+
def test_list_and_search_outside_sandbox_are_denied(self):
75+
self.assertEqual(tools.list_directory_contents("/etc")["status"], "error")
76+
self.assertEqual(
77+
tools.search_local_git_repo("/etc", "root")["status"], "error"
78+
)
79+
80+
def test_clone_or_pull_outside_sandbox_is_denied(self):
81+
res = tools.clone_or_pull_repo(
82+
repo_owner="google",
83+
repo_name="adk-docs",
84+
local_path="/etc",
85+
)
86+
self.assertEqual(res["status"], "error")
87+
self.assertIn("Access denied", res["error_message"])
88+
89+
def test_create_pr_rejects_path_traversal_in_changes(self):
90+
# Reach the file-writing step with the git/network calls stubbed out, then
91+
# assert a traversing change key is rejected before any write.
92+
with (
93+
mock.patch.object(tools, "_run_git_command"),
94+
mock.patch.object(tools, "post_request"),
95+
):
96+
res = tools.create_pull_request_from_changes(
97+
repo_owner="google",
98+
repo_name="adk-docs",
99+
local_path=self.repo,
100+
base_branch="main",
101+
changes={"../../../../tmp/evil.txt": "owned"},
102+
commit_message="m",
103+
pr_title="t",
104+
pr_body="b",
105+
)
106+
self.assertEqual(res["status"], "error")
107+
self.assertIn("escapes the repository", res["error_message"])
108+
self.assertFalse(os.path.exists("/tmp/evil.txt"))
109+
110+
111+
if __name__ == "__main__":
112+
unittest.main()

‎contributing/samples/adk_team/adk_documentation/tools.py‎

Lines changed: 64 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
from typing import Optional
2323

2424
from adk_documentation.settings import GITHUB_BASE_URL
25+
from adk_documentation.settings import LOCAL_REPOS_DIR_PATH
2526
from adk_documentation.utils import error_response
2627
from adk_documentation.utils import get_paginated_request
2728
from adk_documentation.utils import get_request
@@ -30,6 +31,44 @@
3031
import requests
3132

3233

34+
def _resolve_within_repos_dir(path: str) -> Optional[str]:
35+
"""Resolves `path` and verifies it stays within LOCAL_REPOS_DIR_PATH.
36+
37+
Returns the resolved absolute path if it is inside the sandbox, else None.
38+
"""
39+
allowed_root = os.path.realpath(LOCAL_REPOS_DIR_PATH)
40+
resolved = os.path.realpath(path)
41+
if resolved == allowed_root or resolved.startswith(allowed_root + os.sep):
42+
return resolved
43+
return None
44+
45+
46+
def _confine_path(
47+
path: str, param_name: str
48+
) -> tuple[Optional[str], Optional[Dict[str, Any]]]:
49+
"""Resolves `path` and verifies it stays within LOCAL_REPOS_DIR_PATH.
50+
51+
The file/search tools are only meant to operate on the repositories cloned
52+
under LOCAL_REPOS_DIR_PATH. Because these tools are driven by an LLM that
53+
processes untrusted input (issue bodies, release diffs), an `os.path.isabs`
54+
check alone lets a crafted instruction read or write arbitrary paths (e.g.
55+
the credentials file pointed to by GOOGLE_APPLICATION_CREDENTIALS, or
56+
/proc/self/environ). Resolve symlinks and `..` segments first, then require
57+
the result to stay inside the managed directory.
58+
59+
Returns:
60+
A tuple of (safe_path, error_response_dict). If the path is outside the
61+
sandbox, safe_path is None and error_response_dict contains the error.
62+
"""
63+
safe_path = _resolve_within_repos_dir(path)
64+
if safe_path is None:
65+
return None, error_response(
66+
f"Access denied: {param_name} is outside the managed repositories "
67+
f"directory ({LOCAL_REPOS_DIR_PATH}): {path}"
68+
)
69+
return safe_path, None
70+
71+
3372
def list_releases(repo_owner: str, repo_name: str) -> Dict[str, Any]:
3473
"""Lists all releases for a repository.
3574
@@ -135,6 +174,10 @@ def clone_or_pull_repo(
135174
A dictionary indicating the status of the operation, output message, and
136175
the head commit hash.
137176
"""
177+
local_path, err = _confine_path(local_path, "local_path")
178+
if err:
179+
return err
180+
138181
repo_url = f"git@github.com:{repo_owner}/{repo_name}.git"
139182

140183
try:
@@ -188,6 +231,10 @@ def read_local_git_repo_file_content(file_path: str) -> Dict[str, Any]:
188231
f"file_path must be an absolute path, got: {file_path}"
189232
)
190233

234+
file_path, err = _confine_path(file_path, "file_path")
235+
if err:
236+
return err
237+
191238
try:
192239
dir_path = os.path.dirname(file_path)
193240
head_commit_sha = _find_head_commit_sha(dir_path)
@@ -229,6 +276,9 @@ def list_directory_contents(directory_path: str) -> Dict[str, Any]:
229276
print(
230277
f"Attempting to recursively list contents of directory: {directory_path}"
231278
)
279+
directory_path, err = _confine_path(directory_path, "directory_path")
280+
if err:
281+
return err
232282
if not os.path.isdir(directory_path):
233283
return error_response(f"Error: Directory not found at {directory_path}")
234284

@@ -276,6 +326,9 @@ def search_local_git_repo(
276326
f"Attempting to search for pattern: {pattern} in directory:"
277327
f" {directory_path}, with extensions: {extensions}"
278328
)
329+
directory_path, err = _confine_path(directory_path, "directory_path")
330+
if err:
331+
return err
279332
try:
280333
grep_process = _git_grep(directory_path, pattern, extensions, ignored_dirs)
281334
if grep_process.returncode > 1:
@@ -350,8 +403,18 @@ def create_pull_request_from_changes(
350403
if not changes:
351404
return error_response("No changes provided to apply.")
352405

406+
repo_root = os.path.realpath(local_path)
353407
for relative_path, new_content in changes.items():
354-
full_path = os.path.join(local_path, relative_path)
408+
full_path = os.path.realpath(os.path.join(local_path, relative_path))
409+
# Confine writes to the repository: a crafted relative_path such as
410+
# "../../etc/x" would otherwise escape `local_path`.
411+
if not (
412+
full_path == repo_root or full_path.startswith(repo_root + os.sep)
413+
):
414+
return error_response(
415+
"Access denied: change path escapes the repository:"
416+
f" {relative_path}"
417+
)
355418
os.makedirs(os.path.dirname(full_path), exist_ok=True)
356419
with open(full_path, "w", encoding="utf-8") as f:
357420
f.write(new_content)

0 commit comments

Comments
 (0)