From 01252816093bfd696d4dfc71c3121e51baf6d4ee Mon Sep 17 00:00:00 2001 From: f1aggo_macair Date: Mon, 3 Aug 2026 11:09:15 +0530 Subject: [PATCH] fix(tools): allow Unicode letters in workdir validation The workdir allowlist regex was ASCII-only, so perfectly normal non-ASCII workdirs (Chinese Obsidian vault paths, accented dirnames) were rejected with 'disallowed character'. Replace the regex with a per-character check that accepts Unicode letters/digits (str.isalnum) plus the same safe ASCII punctuation set, while still rejecting shell metacharacters, control characters (newlines/tabs), and NUL. Salvaged from PR #54314. Co-authored-by: kshitij <82637225+kshitijk4poor@users.noreply.github.com> --- tests/tools/test_terminal_tool.py | 20 +++++++++++++++++ tools/terminal_tool.py | 37 ++++++++++++++++++++----------- 2 files changed, 44 insertions(+), 13 deletions(-) diff --git a/tests/tools/test_terminal_tool.py b/tests/tools/test_terminal_tool.py index bbb29c04d0f27..958245fd12aeb 100644 --- a/tests/tools/test_terminal_tool.py +++ b/tests/tools/test_terminal_tool.py @@ -83,6 +83,26 @@ def test_validate_workdir_blocks_shell_metacharacters_in_windows_paths(): assert terminal_tool._validate_workdir("C:\\Users\\Alice\\project\nwhoami") +def test_validate_workdir_allows_unicode_filesystem_paths(): + assert terminal_tool._validate_workdir( + "/Users/alice/Documents/Obs_Hermes_Data/项目-projects/客户拜访" + ) is None + assert terminal_tool._validate_workdir("/tmp/テスト") is None + assert terminal_tool._validate_workdir("/home/jürgen/über projekt") is None + + +def test_validate_workdir_still_blocks_metachars_in_unicode_paths(): + # Widening to Unicode letters must not open the injection boundary: + # shell metacharacters and control chars stay rejected even when mixed + # with non-ASCII path segments. + assert terminal_tool._validate_workdir("/tmp/テスト; rm -rf /") + assert terminal_tool._validate_workdir("/tmp/项目$(whoami)") + assert terminal_tool._validate_workdir("/tmp/über`id`") + assert terminal_tool._validate_workdir("/tmp/テスト\nwhoami") + assert terminal_tool._validate_workdir("/tmp/项目|cat /etc/passwd") + assert terminal_tool._validate_workdir("/tmp/ü\x00ber") + + def test_count_real_sudo_invocations_ignores_mentions(monkeypatch): assert terminal_tool._count_real_sudo_invocations("grep sudo README.md") == 0 assert terminal_tool._count_real_sudo_invocations("sudo a; sudo b") == 2 diff --git a/tools/terminal_tool.py b/tools/terminal_tool.py index 4191c48ac2ae9..d929947f41e07 100644 --- a/tools/terminal_tool.py +++ b/tools/terminal_tool.py @@ -374,10 +374,24 @@ def _check_all_guards(command: str, env_type: str, # Allowlist: characters that can legitimately appear in directory paths. -# Covers alphanumeric, path separators, Windows drive/UNC separators, tilde, -# dot, hyphen, underscore, space, plus, at, equals, and comma. Everything -# else is rejected. -_WORKDIR_SAFE_RE = re.compile(r'^[A-Za-z0-9/\\:_\-.~ +@=,]+$') +# Covers Unicode letters/digits, path separators, Windows drive/UNC separators, +# tilde, dot, hyphen, underscore, space, plus, at, equals, and comma. Shell +# metacharacters remain rejected. This intentionally fixes the old ASCII-only +# guard that blocked perfectly normal workdirs such as Chinese Obsidian vault +# paths while preserving the injection boundary around command execution +# (the cwd is additionally shlex-quoted before it reaches the shell; this +# allowlist is defense-in-depth). +_WORKDIR_SAFE_ASCII_CHARS = frozenset('/\\:_-.~ +@=,') + + +def _is_safe_workdir_char(ch: str) -> bool: + if not ch: + return False + # Reject control characters (including newlines/tabs) and NUL bytes before + # considering Unicode categories. + if ord(ch) < 32 or ord(ch) == 127: + return False + return ch.isalnum() or ch in _WORKDIR_SAFE_ASCII_CHARS def _validate_workdir(workdir: str) -> str | None: @@ -390,15 +404,12 @@ def _validate_workdir(workdir: str) -> str | None: """ if not workdir: return None - if not _WORKDIR_SAFE_RE.match(workdir): - # Find the first offending character for a helpful message. - for ch in workdir: - if not _WORKDIR_SAFE_RE.match(ch): - return ( - f"Blocked: workdir contains disallowed character {repr(ch)}. " - "Use a simple filesystem path without shell metacharacters." - ) - return "Blocked: workdir contains disallowed characters." + for ch in workdir: + if not _is_safe_workdir_char(ch): + return ( + f"Blocked: workdir contains disallowed character {repr(ch)}. " + "Use a simple filesystem path without shell metacharacters." + ) return None