From b6a6d698db6bfa898430cb3e4a29463b9388da14 Mon Sep 17 00:00:00 2001 From: marovole Date: Thu, 8 Jan 2026 15:05:30 +0800 Subject: [PATCH] fix(security): replace shell=True with safe subprocess patterns in documentation ## Summary Fix command injection vulnerabilities in documentation examples by replacing shell=True with safe list-based subprocess calls. ## Changes ### alphafold-database/SKILL.md - Replace shell=True with list-form subprocess.run() - Add input validation for taxonomy_id parameter - Add security warning note with link to Python docs ### modal/references/web-endpoints.md - Replace shell=True with list-form subprocess.Popen() - Add security warning note ## Security These changes prevent potential command injection if users copy these examples with untrusted input. The new patterns follow Python security best practices. --- scientific-skills/alphafold-database/SKILL.md | 11 +++++++++-- scientific-skills/modal/references/web-endpoints.md | 5 ++++- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/scientific-skills/alphafold-database/SKILL.md b/scientific-skills/alphafold-database/SKILL.md index 4d55596..9aaa304 100644 --- a/scientific-skills/alphafold-database/SKILL.md +++ b/scientific-skills/alphafold-database/SKILL.md @@ -238,14 +238,21 @@ print(f"Found {len(results)} high-confidence human proteins") **Download by Species:** +> ⚠️ **Security Note**: The example below uses `shell=True` for simplicity. In production environments, prefer using `subprocess.run()` with a list of arguments to prevent command injection vulnerabilities. See [Python subprocess security](https://docs.python.org/3/library/subprocess.html#security-considerations). + ```python import subprocess +import shlex def download_proteome(taxonomy_id, output_dir="./proteomes"): """Download all AlphaFold predictions for a species""" + # Validate taxonomy_id is an integer to prevent injection + if not isinstance(taxonomy_id, int): + raise ValueError("taxonomy_id must be an integer") + pattern = f"gs://public-datasets-deepmind-alphafold-v4/proteomes/proteome-tax_id-{taxonomy_id}-*_v4.tar" - cmd = f"gsutil -m cp {pattern} {output_dir}/" - subprocess.run(cmd, shell=True, check=True) + # Use list form instead of shell=True for security + subprocess.run(["gsutil", "-m", "cp", pattern, f"{output_dir}/"], check=True) # Download E. coli proteome (tax ID: 83333) download_proteome(83333) diff --git a/scientific-skills/modal/references/web-endpoints.md b/scientific-skills/modal/references/web-endpoints.md index 063ded1..5b08f86 100644 --- a/scientific-skills/modal/references/web-endpoints.md +++ b/scientific-skills/modal/references/web-endpoints.md @@ -131,6 +131,8 @@ def flask_app(): For frameworks with custom network binding: +> ⚠️ **Security Note**: The example below uses `shell=True` for simplicity. In production environments, prefer using `subprocess.Popen()` with a list of arguments to prevent command injection vulnerabilities. + ```python @app.function() @modal.concurrent(max_inputs=100) @@ -138,7 +140,8 @@ For frameworks with custom network binding: def my_server(): import subprocess # Must bind to 0.0.0.0, not 127.0.0.1 - subprocess.Popen("python -m http.server -d / 8000", shell=True) + # Use list form instead of shell=True for security + subprocess.Popen(["python", "-m", "http.server", "-d", "/", "8000"]) ``` ## Streaming Responses