Skip to content

fix: Fix command injection vulnerability in upload_package_repo.py#6789

Open
raramakr wants to merge 1 commit into
mainfrom
users/raramakr/pkg-upload
Open

fix: Fix command injection vulnerability in upload_package_repo.py#6789
raramakr wants to merge 1 commit into
mainfrom
users/raramakr/pkg-upload

Conversation

@raramakr

Copy link
Copy Markdown
Contributor

Refactor run_command() to prevent shell injection attacks:

  • Accept list[str] instead of string commands with shell=True
  • Add stdout parameter for file redirects instead of shell redirection
  • Add type hints and docstring

Additional improvements:

  • Add encoding="utf-8" to all text file operations
  • Convert os.path usage to pathlib.Path for consistency

JIRA ID: https://amd-hub.atlassian.net/browse/ROCM-26880

…0354)

  Refactor run_command() to prevent shell injection attacks:
  - Accept list[str] instead of string commands with shell=True
  - Add stdout parameter for file redirects instead of shell redirection
  - Add type hints and docstring

  Additional improvements:
  - Add encoding="utf-8" to all text file operations
  - Convert os.path usage to pathlib.Path for consistency
@therock-pr-bot

therock-pr-bot Bot commented Jul 22, 2026

Copy link
Copy Markdown

❌ PR Check — Action Required

Check Status Details
🌿 Branch Name ✅ Pass
📝 PR Title/Description ✅ Pass
Forbidden Files ✅ Pass
🧪 Unit Test ❌ Fail Error: Source/code files changed without an accompanying unit test.
Expected: add at least one test file named like test_<name>.py / test_<name>.cpp (or <name>_test.*).
Current: code file(s) changed: build_tools/packaging/linux/upload_package_repo.py; no test file found
🔎 pre-commit ✅ Pass
🚫 Draft PR 🔜 To Be Enabled
🚩 Feature Flag 🔜 To Be Enabled
📊 Code Coverage 🔜 To Be Enabled

⚠️ 1 policy check(s) failed. Please address the issues above before this PR can be Reviewed.

🚫 Please fix the failed policies

  • ❌ Unit Test

The Not ready to Review label was added to this PR. Once all policies pass, the label is removed automatically.

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

@therock-pr-bot therock-pr-bot Bot added the Not ready to Review PR has unresolved policy failures — reviews blocked label Jul 22, 2026
@therock-pr-bot

Copy link
Copy Markdown

🚫 Please fix the failed policies before requesting reviews.

The following policy checks failed:

  • ❌ Unit Test

The Not ready to Review label has been added to this PR.
Once all policies pass, the label will be removed automatically.

@nunnikri nunnikri left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good to me. Lets complete the ci run

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

Labels

Not ready to Review PR has unresolved policy failures — reviews blocked

Projects

Status: TODO

Development

Successfully merging this pull request may close these issues.

2 participants