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>
This commit is contained in:
parent
72e8e2983a
commit
0125281609
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue