Skip to content

security: prevent command injection in /contribute endpoint by removing shell=True (fixes #360)#407

Merged
pradeeban merged 1 commit into
ControlCore-Project:devfrom
GaneshPatil7517:security/remove-shell-from-contribute
Feb 19, 2026
Merged

security: prevent command injection in /contribute endpoint by removing shell=True (fixes #360)#407
pradeeban merged 1 commit into
ControlCore-Project:devfrom
GaneshPatil7517:security/remove-shell-from-contribute

Conversation

@GaneshPatil7517
Copy link
Copy Markdown
Contributor

@GaneshPatil7517 GaneshPatil7517 commented Feb 18, 2026

Hello @pradeeban Sir
This PR resolves Issue #360 by eliminating the command injection vulnerability in the /contribute endpoint of fri/server/main.py.

Previously, the non-Windows code path used check_output() which could be susceptible to shell injection if shell=True were ever reintroduced. The Windows path already used subprocess.run() but the code was inconsistent across platforms.

Changes Made

  • Replaced check_output() with subprocess.run() on the non-Windows code path
  • Unified both Windows and non-Windows paths to use a single subprocess.run() call with capture_output=True, text=True, and check=True
  • No shell=True is used anywhere in the /contribute endpoint
  • Arguments are passed strictly as a list, preventing shell interpretation of user-controlled inputs

Security Impact

  • Eliminates command injection vector — payloads like "; rm -rf / #" in PR title, body, branch name, etc. are treated as literal string arguments
  • User inputs are never passed through a shell interpreter
  • Existing input validation (validate_input, validate_text_field) provides an additional defense layer

Scope

  • Single file modified: fri/server/main.py
  • No changes to concore-lite
  • No Verilog file changes
  • No behavior change for valid requests

Testing

  • Verified fri/server/main.py compiles without syntax errors
  • Verified injection payloads (e.g., ; rm -rf /, && cat /etc/passwd, | whoami) are blocked by input validation and treated as literal arguments
  • Verified no shell=True exists in the /contribute endpoint
  • Verified subprocess.run() with check=True and capture_output=True is used
  • Verified check_output is no longer used in the /contribute endpoint

Copilot AI review requested due to automatic review settings February 18, 2026 10:55
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses a command injection vulnerability in the /contribute endpoint by standardizing the use of subprocess.run() across both Windows and non-Windows platforms and ensuring shell=True is never used with user-controlled inputs.

Changes:

  • Unified subprocess invocation using subprocess.run() with capture_output=True, text=True, and check=True for both Windows and non-Windows code paths
  • Eliminated check_output() calls in favor of the standardized subprocess.run() approach
  • Ensured command arguments are always passed as a list without shell interpretation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread fri/server/main.py Outdated
@GaneshPatil7517 GaneshPatil7517 force-pushed the security/remove-shell-from-contribute branch from faed353 to e2c9853 Compare February 18, 2026 10:59
@pradeeban pradeeban merged commit b4fdcf1 into ControlCore-Project:dev Feb 19, 2026
6 checks passed
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.

3 participants