Skip to content

🛡️ Sentinel: [MEDIUM] Fix shell injection vulnerability #669

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
badMade wants to merge 2 commits into
base: main
Choose a base branch
from
+7 −3
Open

🛡️ Sentinel: [MEDIUM] Fix shell injection vulnerability #669

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
12b3482
🛡️ Sentinel: [MEDIUM] Fix shell injection vulnerability in tools_conf…
badMade May 31, 2026
64eeea5
🛡️ Sentinel: [MEDIUM] Fix shell injection vulnerability in tools_conf…
badMade May 31, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .jules/sentinel.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,7 @@
**Vulnerability:** SQL Injection via string-interpolated subqueries with LIMIT. The `query` function in `optional-skills/mcp/fastmcp/templates/database_server.py` wrapped user SQL in a subquery: `SELECT * FROM ({sql}) LIMIT N`. This allowed malicious users to bypass simple checks (e.g. ensuring it starts with SELECT) and inject additional clauses or statements by manipulating the closing parenthesis.
**Learning:** SQLite does not natively support parameterization for the FROM clause (e.g., subqueries or table names). Attempting to string-interpolate user input into a subquery creates an injection vector, especially when trying to enforce a LIMIT clause on user-provided queries.
**Prevention:** To prevent SQL injection when applying limits to user-provided SQL queries, execute the raw user query directly and restrict the output rows in Python using `cursor.fetchmany(limit)` instead of trying to wrap the query in another SELECT with a LIMIT clause.
## 2024-05-24 - Security Enhancement: Shell Injection Prevention
**Vulnerability:** Use of `subprocess.run(shell=True)` in `hermes_cli/tools_config.py` for cua-driver installation.
**Learning:** Using `shell=True` can introduce shell injection vulnerabilities, especially if any parts of the command are dynamic. Although this specific case was a hardcoded URL string, it's best practice to replace `shell=True` with an argument list for defense in depth.
**Prevention:** Avoid `shell=True` in `subprocess.run` and pass the command and its arguments as a list. When using `bash -c`, pass the script content as an argument to `-c` rather than interpolating it into a single string with `shell=True`.
6 changes: 3 additions & 3 deletions hermes_cli/tools_config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -786,11 +786,11 @@ def _run_post_setup(post_setup_key: str):
_print_info(" Installing cua-driver (macOS background computer-use)...")
try:
install_cmd = (
"/bin/bash -c \"$(curl -fsSL "
"curl -fsSL "
"https://raw.githubusercontent.com/trycua/cua/main/"
"libs/cua-driver/scripts/install.sh)\""
"libs/cua-driver/scripts/install.sh | /bin/bash"
)
result = subprocess.run(install_cmd, shell=True, timeout=300)
result = subprocess.run(["/bin/bash", "-c", install_cmd], timeout=300)
Comment on lines 788 to +793
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot May 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using subprocess.run(["/bin/bash", "-c", install_cmd]) still invokes a shell to parse and execute the command string (including the pipe |), which does not fully eliminate shell evaluation or potential injection risks if any part of the command were to become dynamic in the future.

Instead of spawning a shell to run curl and pipe its output to bash, you can download the script directly in Python using urllib.request and pass the script content directly to /bin/bash via stdin (using the input parameter of subprocess.run). This completely avoids shell execution and removes the dependency on the external curl binary during execution.

            import urllib.request
            url = (
                "https://raw.githubusercontent.com/trycua/cua/main/"
                "libs/cua-driver/scripts/install.sh"
            )
            with urllib.request.urlopen(url, timeout=30) as response:
                script_content = response.read()
            result = subprocess.run(["/bin/bash"], input=script_content, timeout=300)

Copy link
Copy Markdown
Owner Author

@badMade badMade May 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gemini-code-assist, is this suggestion still valid? If so, what are the steps and line numbers to make this fix?

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot May 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the suggestion is still valid. Using urllib.request to fetch the script content and passing it directly to subprocess.run via the input parameter is the most secure approach because it avoids invoking a shell entirely, thereby eliminating the risk of shell injection.

To implement this in hermes_cli/tools_config.py, you would replace the lines 788-793 with the following code:

Suggested change
install_cmd = (
"/bin/bash -c \"$(curl -fsSL "
"curl -fsSL "
"https://raw.githubusercontent.com/trycua/cua/main/"
"libs/cua-driver/scripts/install.sh)\""
"libs/cua-driver/scripts/install.sh | /bin/bash"
)
result = subprocess.run(install_cmd, shell=True, timeout=300)
result = subprocess.run(["/bin/bash", "-c", install_cmd], timeout=300)
import urllib.request
url = (
"https://raw.githubusercontent.com/trycua/cua/main/"
"libs/cua-driver/scripts/install.sh"
)
with urllib.request.urlopen(url, timeout=30) as response:
script_content = response.read()
result = subprocess.run(["/bin/bash"], input=script_content, timeout=300)

Comment on lines +789 to +793
if result.returncode == 0 and shutil.which("cua-driver"):
_print_success(" cua-driver installed.")
_print_info(" IMPORTANT — grant macOS permissions now:")
Expand Down