T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/interactive_commit.py:21
- Finding
- Git Option Injection Through Repository-Controlled Filenames<![CDATA[ ## Vulnerability Details **File Locations**: - `scripts/interactive_commit.py:21-50` - `scripts/categorize_changes.py:174-186` - `scripts/generate_commit_message.py:283-296` **Vulnerability Type**: Git command option injection **Risk Level**: High ### Vulnerable Code ```python def stage_files(files: List[str]) -> bool: """Stage files for commit.""" if not files: return True try: subprocess.run( ['git', 'add'] + files, capture_output=True, check=True ) return True except subprocess.CalledProcessError as e: print(f"暂存文件时出错: {e}", file=sys.stderr) return False def unstage_files(files: List[str]) -> bool: """Unstage files to reorganize commits.""" if not files: return True try: subprocess.run( ['git', 'reset', 'HEAD'] + files, capture_output=True, check=True ) return True except subprocess.CalledProcessError as e: print(f"取消暂存文件时出错: {e}", file=sys.stderr) return False ``` The same unsafe path handling occurs during diff analysis: ```python def detect_code_change_type(filepath: str) -> str: """ Detect if a source code change is feat, fix, refactor, or style. This analyzes the git diff content. """ try: result = subprocess.run( ['git', 'diff', '--cached', filepath], capture_output=True, text=True, check=True ) ``` ```python def get_file_diff(filepath: str) -> str: """Get git diff for a specific file (cached).""" if filepath in _diff_cache: return _diff_cache[filepath] try: result = subprocess.run( ['git', 'diff', '--cached', filepath], capture_output=True, text=True, ) _diff_cache[filepath] = result.stdout return result.stdout except Exception: return "" ``` ### Technical A ...[truncated 3106 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions Place Git's `--` option terminator before every repository-controlled path: ```python def stage_files(files: List[str]) -> bool: if not files: return True try: subprocess.run( ['git', 'add', '--', *files], capture_output=True, check=True ) return True except subprocess.CalledProcessError as e: print(f"Error staging files: {e}", file=sys.stderr) return False def unstage_files(files: List[str]) -> bool: if not files: return True try: subprocess.run( ['git', 'reset', 'HEAD', '--', *files], capture_output=True, check=True ) return True except subprocess.CalledProcessError as e: print(f"Error unstaging files: {e}", file=sys.stderr) return False ``` Apply the same protection to all per-file diff commands: ```python subprocess.run( ['git', 'diff', '--cached', '--', filepath], capture_output=True, text=True, check=True ) ``` Use NUL-delimited Git output to handle all valid filenames safely: ```python result = subprocess.run( ['git', 'diff', '--cached', '--name-only', '-z'], capture_output=True, check=True ) files = [ value.decode('utf-8', errors='surrogateescape') for value in result.stdout.split(b'\0') if value ] ``` Additional hardening should include: 1. Add tests covering filenames such as `--hard`, `--all`, names containing spaces, and names containing newline characters. 2. Preserve the original index state before unstaging all files. 3. If any group fails, restore or restage the remaining original entries rather than leaving the index partially modified. 4. Validate that every grouped path is present in the original staged-file set before invoking Git. 5. Continue using subprocess argument arrays and do not replace them with shell command strings. ]]>