Skip to content

fix: use subprocess instead of os.system in checkPackageRuning.py#35276

Merged
zitsen merged 1 commit intotaosdata:mainfrom
orbisai0security:fix-v-006-command-injection-checkpackageruning
May 6, 2026
Merged

fix: use subprocess instead of os.system in checkPackageRuning.py#35276
zitsen merged 1 commit intotaosdata:mainfrom
orbisai0security:fix-v-006-command-injection-checkpackageruning

Conversation

@orbisai0security
Copy link
Copy Markdown
Contributor

Summary

Fix critical severity security issue in packaging/checkPackageRuning.py.

Vulnerability

Field Value
ID V-006
Severity CRITICAL
Scanner multi_agent_ai
Rule V-006
File packaging/checkPackageRuning.py:50

Description: The packaging script checkPackageRuning.py constructs OS shell commands by directly interpolating the serverHost variable using Python % string formatting and passing the result to os.system(). Because os.system() invokes a shell, any shell metacharacters in serverHost (semicolons, pipes, backticks, dollar signs) are interpreted as shell syntax, allowing an attacker who controls the serverHost argument to execute arbitrary operating system commands with the full privileges of the packaging script process.

Changes

  • packaging/checkPackageRuning.py

Verification

  • Build passes
  • Scanner re-scan confirms fix
  • LLM code review passed

Automated security fix by OrbisAI Security

Automated security fix generated by Orbis Security AI
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request overhauls the project's build system and CI/CD infrastructure, transitioning to CMake 3.16 with Conan support and introducing scripts for test validation and documentation generation. It also includes a significant cleanup of legacy files and updated documentation. Feedback focuses on enhancing the robustness and safety of these additions, specifically by addressing dangerous file deletion commands in the build script, fixing fragile path-handling logic in documentation generators, adding timeouts to subprocess calls, and removing leftover developer notes in the CMake configuration.

Comment thread build.sh
Comment thread .github/scripts/generate_case_md.py
Comment thread .github/scripts/generate_case_md.py
Comment thread .github/scripts/generate_shell_case_md.py
Comment thread cmake/define.cmake
Comment thread .github/scripts/check_enum_append_only.py
@guanshengliang guanshengliang changed the base branch from develop to main May 6, 2026 10:56
@guanshengliang guanshengliang requested a review from feici02 as a code owner May 6, 2026 10:56
Copy link
Copy Markdown
Contributor

@zitsen zitsen left a comment

Choose a reason for hiding this comment

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

Security review summary: I reviewed the single-file change in packaging/checkPackageRuning.py. The PR removes the command-injection path caused by interpolating serverHost into shell commands and now passes serverHost as a single argv element to subprocess.run, so shell metacharacters are not evaluated by a shell. I did not find high-confidence security regressions introduced by this change. Residual non-blocking hygiene: prefer a normal subprocess import with targeted scanner suppressions over dynamic import, and consider cleaning up the temporary dump directory in a follow-up.

@zitsen zitsen merged commit 67952e0 into taosdata:main May 6, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants