Skip to content

Commit 57c01db

Browse files
committed
fix: address review edge cases in shell capture and cd handling
- Windows: fall back to pipe capture when PTY is unavailable - PTY: catch asyncio.CancelledError alongside KeyboardInterrupt to prevent Ctrl+C from tearing down the shell loop - Security: neutralise <system-reminder> tags in captured output to prevent injection via untrusted shell output - cd: preserve shell expansion for ~, $VAR, and - targets instead of quoting them with shlex.quote
1 parent 6537e0c commit 57c01db

2 files changed

Lines changed: 24 additions & 12 deletions

File tree

src/kimi_cli/ui/shell/__init__.py

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import contextlib
55
import os
66
import shlex
7+
import sys
78
import time
89
from collections import deque
910
from collections.abc import Awaitable, Callable, Coroutine
@@ -23,7 +24,11 @@
2324
from kimi_cli.soul import LLMNotSet, LLMNotSupported, MaxStepsReached, RunCancelled, Soul, run_soul
2425
from kimi_cli.soul.kimisoul import KimiSoul
2526
from kimi_cli.ui.shell import update as _update_mod
26-
from kimi_cli.ui.shell.capture import execute_with_pty_capture, inject_to_context
27+
from kimi_cli.ui.shell.capture import (
28+
execute_with_pipe_capture,
29+
execute_with_pty_capture,
30+
inject_to_context,
31+
)
2732
from kimi_cli.ui.shell.console import console
2833
from kimi_cli.ui.shell.echo import render_user_echo_text
2934
from kimi_cli.ui.shell.mcp_status import render_mcp_prompt
@@ -521,9 +526,10 @@ async def _run_shell_command(self, command: str) -> None:
521526
exit_code: int | None = None
522527
raw_output: str | None = None
523528
try:
524-
exit_code, raw_output = await execute_with_pty_capture(
525-
command, env=get_clean_env(), cwd=os.getcwd()
529+
execute = (
530+
execute_with_pty_capture if sys.platform != "win32" else execute_with_pipe_capture
526531
)
532+
exit_code, raw_output = await execute(command, env=get_clean_env(), cwd=os.getcwd())
527533
except Exception as e:
528534
logger.exception("Failed to run shell command:")
529535
console.print(f"[red]Failed to run shell command: {e}[/red]")
@@ -544,19 +550,21 @@ async def _handle_cd(self, args: list[str]) -> None:
544550
"""
545551
target = args[1] if len(args) > 1 else "~"
546552

547-
# Expand ~ on Python side so shlex.quote won't suppress tilde expansion.
548-
if target.startswith("~"):
549-
target = os.path.expanduser(target)
550-
551553
# Provide OLDPWD so `cd -` works across invocations.
552554
env = get_clean_env()
553555
old_cwd = os.getcwd()
554556
if "OLDPWD" not in env:
555557
env["OLDPWD"] = old_cwd
556558

557-
# Let the shell resolve -, $HOME, CDPATH, etc.
559+
# Use shlex.quote for safety, but NOT for targets that need shell
560+
# expansion: ~, -, and $VAR references. For those we let the real
561+
# shell handle expansion directly.
562+
needs_shell_expansion = target.startswith("~") or target.startswith("$") or target == "-"
563+
quoted_target = target if needs_shell_expansion else shlex.quote(target)
564+
565+
# Let the shell resolve ~, -, $HOME, CDPATH, etc.
558566
probe = await asyncio.create_subprocess_shell(
559-
f"cd {shlex.quote(target)} && pwd",
567+
f"cd {quoted_target} && pwd",
560568
stdout=asyncio.subprocess.PIPE,
561569
stderr=asyncio.subprocess.PIPE,
562570
env=env,

src/kimi_cli/ui/shell/capture.py

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -163,14 +163,14 @@ def _on_master_readable() -> None:
163163
# Drain any remaining buffered output after process exits.
164164
with contextlib.suppress(TimeoutError):
165165
await asyncio.wait_for(eof_event.wait(), timeout=0.5)
166-
except KeyboardInterrupt:
166+
except (KeyboardInterrupt, asyncio.CancelledError):
167167
# Forward SIGINT to child if it is still running.
168-
logger.debug("PTY capture: KeyboardInterrupt, forwarding SIGINT to child")
168+
logger.debug("PTY capture: interrupted, forwarding SIGINT to child")
169169
if proc.returncode is None:
170170
proc.send_signal(2) # SIGINT
171171
try:
172172
await asyncio.wait_for(proc.wait(), timeout=3.0)
173-
except (TimeoutError, ProcessLookupError):
173+
except (TimeoutError, ProcessLookupError, asyncio.CancelledError):
174174
logger.debug("PTY capture: child did not exit after SIGINT, killing")
175175
proc.kill()
176176
finally:
@@ -254,6 +254,10 @@ async def inject_to_context(
254254
"""
255255
output = clean_output(raw_output)
256256

257+
# Neutralise any system-reminder tags in the output to prevent injection.
258+
output = output.replace("<system-reminder>", "&lt;system-reminder&gt;")
259+
output = output.replace("</system-reminder>", "&lt;/system-reminder&gt;")
260+
257261
# Truncate, keeping the tail (usually more informative).
258262
truncated = False
259263
encoded = output.encode("utf-8")

0 commit comments

Comments
 (0)