Skip to content

fix: escape path in find command to prevent shell injection#402

Open
hobostay wants to merge 1 commit into
bytedance:mainfrom
hobostay:fix/shell-injection-find-command
Open

fix: escape path in find command to prevent shell injection#402
hobostay wants to merge 1 commit into
bytedance:mainfrom
hobostay:fix/shell-injection-find-command

Conversation

@hobostay
Copy link
Copy Markdown

Summary

  • Fix shell injection vulnerability in edit_tool.py:162 where the path parameter is passed unsanitized to a find shell command

Problem

In TextEditorTool._view(), when viewing a directory, the path is interpolated directly into a shell command:

return_code, stdout, stderr = await run(rf"find {path} -maxdepth 2 -not -path '*/\.*'")

The run() function uses asyncio.create_subprocess_shell(), so a path containing shell metacharacters could execute arbitrary commands.

Fix

Use shlex.quote() to properly escape the path before passing it to the shell command.

Test plan

  • Verify normal directory viewing still works
  • Test with paths containing spaces, semicolons, or other shell metacharacters

🤖 Generated with Claude Code

The `find` command in `_view()` passes the `path` parameter directly to
a shell via `asyncio.create_subprocess_shell`. A path containing shell
metacharacters (e.g., `/tmp/repo; rm -rf /`) could lead to command
injection. Use `shlex.quote()` to properly escape the path.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@CLAassistant
Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Test User seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants