Skip to content

Security: Unified Package Manager with Shell Injection Protection #530

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

Closed
goodluxiao2 wants to merge 4 commits into from
Closed

Security: Unified Package Manager with Shell Injection Protection #530

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a1afbf8
docs: unify package manager terminology
goodluxiao2 Jan 5, 2026
0779dd9
Security: implemented shlex.quote for shell injection protection
goodluxiao2 Jan 5, 2026
c603ce4
Security: full implementation of shlex.quote for shell injection prot…
goodluxiao2 Jan 5, 2026
3717fea
docs: sign CLA for goodluxiao2 via API
goodluxiao2 Jan 5, 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
13 changes: 10 additions & 3 deletions .github/cla-signers.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,9 +167,16 @@
],
"signed_date": "2026-01-03",
"cla_version": "1.0"
},
{
"name": "Aether-AGI-Operator",
"github_username": "goodluxiao2",
"emails": [
"goodluxiao@gmail.com"
],
"signed_date": "2026-01-05",
"cla_version": "1.0"
}
Comment on lines +171 to 179
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 5, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical: Security implementation file missing from review.

The PR title and description claim to implement "Shell Injection Protection" in cortex/packages.py using shlex.quote() for package names. However, that file is not included in this review—only the CLA signers file is provided.

Security-critical changes must be reviewed to verify:

  • Correct usage of shlex.quote() for all external command invocations
  • Coverage of all vulnerable code paths (apt-cache, yum, dnf commands mentioned in AI summary)
  • Proper handling of edge cases and error conditions
  • Validation that the fix actually prevents shell injection attacks

Please include cortex/packages.py in this PR for review, or clarify why the primary security implementation is not shown.

🤖 Prompt for AI Agents 
In @.github/cla-signers.json around lines 171-179, The PR claims a security fix
in cortex/packages.py (using shlex.quote for package names) but that file is
missing from the diff; add cortex/packages.py to the PR or update the branch so
the implementation is included, and in that file ensure every external command
invocation (e.g., functions like install_packages, _build_command,
run_subprocess or any helper that constructs apt-cache, apt-get, yum, dnf, or
dnf/yum/distro-specific command strings) applies shlex.quote() to all untrusted
inputs (package names, repos, options), handles empty/null inputs and encoding,
and surfaces subprocess errors (capture stderr/exit codes) instead of swallowing
them; also add unit/integration tests that exercise apt-cache/yum/dnf code paths
with malicious input to verify shell injection is prevented and error conditions
are covered.



],
"corporations": {
"AI Venture Holdings LLC": {
Expand All @@ -187,4 +194,4 @@
"emails": []
}
}
}
}
5 changes: 3 additions & 2 deletions cortex/packages.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

import re
import subprocess
import shlex
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 5, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Import added but misused in subprocess calls.

While the import is syntactically correct, shlex is being misused in this file. The shlex.quote() function is intended for escaping strings in shell commands, but this code uses subprocess.run() with list arguments (not shell=True), where quoting is both unnecessary and harmful.

🤖 Prompt for AI Agents 
In @cortex/packages.py around line 11, The added shlex import is misused: remove
shlex.quote() from calls to subprocess.run that pass arguments as a list (e.g.,
any subprocess.run([...]) usages) and stop importing shlex; instead pass raw
argument strings as separate list elements (or switch to a single shell string
with shell=True if you really need shell quoting). Locate references to
shlex.quote and replace them with the unquoted argument values so subprocess.run
receives a proper argv list, and remove the unused import line "import shlex".

from enum import Enum


Expand Down Expand Up @@ -439,7 +440,7 @@ def get_package_info(self, package_name: str) -> dict[str, str] | None:
try:
if self.pm_type == PackageManagerType.APT:
result = subprocess.run(
["apt-cache", "show", package_name], capture_output=True, text=True, timeout=10
["apt-cache", "show", shlex.quote(package_name)], capture_output=True, text=True, timeout=10
)
if result.returncode == 0:
info = {}
Expand All @@ -452,7 +453,7 @@ def get_package_info(self, package_name: str) -> dict[str, str] | None:
elif self.pm_type in (PackageManagerType.YUM, PackageManagerType.DNF):
pm_cmd = "yum" if self.pm_type == PackageManagerType.YUM else "dnf"
result = subprocess.run(
[pm_cmd, "info", package_name], capture_output=True, text=True, timeout=10
[pm_cmd, "info", shlex.quote(package_name)], capture_output=True, text=True, timeout=10
)
if result.returncode == 0:
info = {}
Expand Down
Loading