T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/setup_cron.py:49
- Finding
- Persistent Shell Command Injection in Generated Crontab Entries<![CDATA[ ## Vulnerability Details **File Location**: `scripts/setup_cron.py:49-96` **Vulnerability Type**: Persistent shell command injection through unquoted cron parameters **Risk Level**: High ### Vulnerable Code ```python def build_job_defs(python: str, profile: str, sf: str, log_dir: str) -> dict[str, dict]: """Return cron job definitions keyed by job name.""" strategy = SCRIPT_DIR / "strategy_engine.py" daily_pick = SCRIPT_DIR / "daily_pick.py" new_releases = SCRIPT_DIR / "new_releases.sh" health = SCRIPT_DIR / "playlist_health.py" return { "weekly-mix": { "description": "Generate a fresh playlist every Monday at 7 AM", "schedule": "0 7 * * 1", "command": ( f"{python} {strategy} --strategy trend " f"--profile {profile} --storefront {sf} --create" ), "log": f"{log_dir}/weekly-mix.log", }, "new-releases": { "description": "Check for new releases every Wednesday at 8 AM", "schedule": "0 8 * * 3", "command": f"bash {new_releases} {sf}", "log": f"{log_dir}/new-releases.log", }, "daily-drop": { "description": "Surface a daily pick every day at 8:30 AM", "schedule": "30 8 * * *", "command": ( f"{python} {daily_pick} daily --profile {profile}" ), "log": f"{log_dir}/daily-drop.log", }, "health-check": { "description": "Scan playlists for issues on the 1st of each month", "schedule": "0 9 1 * *", "command": ( f"{python} {health} check all --profile {profile}" ), "log": f"{log_dir}/health-check.log", }, } def format_cron_line(job: dict, name: str) -> str: """Format a single crontab line with logging and marker.""" return ( f"{job['schedule']} {job[ ...[truncated 2892 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Apply `shlex.quote()` independently to every executable path, file path, storefront value, and redirection target before constructing the cron command. 2. Validate storefront values against a strict allowlist or `^[a-z]{2}$`. 3. Reject carriage returns, newline characters, NUL bytes, and other control characters in every value written to a crontab. 4. Resolve and validate executable and script paths before installation: - Require absolute paths. - Ensure expected files exist. - Reject unexpected symlinks where appropriate. 5. Construct each job from a list of arguments and use a dedicated serialization function rather than accepting an already assembled shell command. 6. Consider installing a fixed wrapper script and placing only a quoted wrapper path plus a fixed job identifier in the crontab. 7. Display the exact escaped crontab lines and require explicit confirmation before calling `crontab -`. 8. Add tests covering spaces, quotes, semicolons, command substitutions, redirections, and newline injection in all configurable fields. ]]>