fix(security): replace os.popen/os.system with subprocess for command injection prevention

Replace all deprecated and unsafe command execution methods with
secure subprocess.run() calls using list arguments.

Changes:
- Replace os.popen() with subprocess.run() in tisbackup_gui.py
- Replace os.system() with subprocess.run() in tasks.py and backup_xva.py
- Add input validation for device/partition names (regex-based)
- Fix file operations to use context managers (with statement)
- Remove wildcard import from shutil
- Add timeout protection to all subprocess calls (5-30s)
- Improve error handling with proper try/except blocks

Security improvements:
- Prevent command injection vulnerabilities in USB disk operations
- Validate device paths with regex before system calls
- Use list arguments instead of shell=True to prevent injection
- Add proper error handling instead of silent failures

Code quality improvements:
- Replace deprecated os.popen() (deprecated since Python 2.6)
- Use context managers for file operations
- Remove wildcard imports for cleaner namespace
- Add comprehensive error handling and logging

Documentation:
- Add SECURITY_IMPROVEMENTS.md documenting all changes
- Document remaining security issues and recommendations
- Include testing recommendations and migration notes

BREAKING CHANGE: None - all changes are backward compatible

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

SECURITY_IMPROVEMENTS.md

@@ -0,0 +1,156 @@
+# Security and Code Quality Improvements
+
+This document summarizes the security and code quality improvements made to TISBackup.
+
+## Completed Improvements (High Priority)
+
+### 1. Replaced `os.popen()` with `subprocess.run()`
+**Files Modified:** [tisbackup_gui.py](tisbackup_gui.py)
+
+**Changes:**
+- Replaced deprecated `os.popen()` calls with modern `subprocess.run()`
+- All subprocess calls now use list arguments instead of shell strings
+- Added timeout protection (5-30 seconds depending on operation)
+- Proper error handling with try/except blocks
+
+**Before:**
+```python
+for line in os.popen("udevadm info -q env -n %s" % name):
+    # Process output
+```
+
+**After:**
+```python
+result = subprocess.run(
+    ["udevadm", "info", "-q", "env", "-n", name],
+    capture_output=True,
+    text=True,
+    check=True,
+    timeout=5
+)
+for line in result.stdout.splitlines():
+    # Process output
+```
+
+**Security Impact:** Prevents command injection vulnerabilities
+
+### 2. Replaced `os.system()` with `subprocess.run()`
+**Files Modified:** [tasks.py](tasks.py), [libtisbackup/backup_xva.py](libtisbackup/backup_xva.py)
+
+**Changes:**
+- [tasks.py:37](tasks.py#L37): Changed `os.system("/bin/umount %s")` to `subprocess.run(["/bin/umount", mount_point])`
+- [backup_xva.py:199](libtisbackup/backup_xva.py#L199): Changed `os.system('tar tf "%s"')` to `subprocess.run(["tar", "tf", filename_temp])`
+- Added proper error handling and logging
+
+**Security Impact:** Eliminates command injection risk from potentially user-controlled mount points and filenames
+
+### 3. Added Input Validation
+**Files Modified:** [tisbackup_gui.py](tisbackup_gui.py)
+
+**Changes:**
+- Added regex validation for device/partition names: `^/dev/sd[a-z]1?$`
+- Validates partition names before using in mount/unmount operations
+- Prevents path traversal and command injection attacks
+
+**Example:**
+```python
+# Validate partition name to prevent command injection
+if not re.match(r"^/dev/sd[a-z]1$", partition):
+    continue
+```
+
+**Security Impact:** Prevents malicious input from reaching system commands
+
+### 4. Fixed File Operations with Context Managers
+**Files Modified:** [tisbackup_gui.py](tisbackup_gui.py)
+
+**Before:**
+```python
+line = open(elem).readline()
+```
+
+**After:**
+```python
+with open(elem) as f:
+    line = f.readline()
+```
+
+**Impact:** Ensures files are properly closed, prevents resource leaks
+
+### 5. Improved `run_command()` Function
+**Files Modified:** [tisbackup_gui.py:415-453](tisbackup_gui.py#L415)
+
+**Changes:**
+- Now accepts list arguments for safe command execution
+- Backward compatible with string commands (marked as legacy)
+- Added timeout protection (30 seconds)
+- Better error handling and reporting
+
+**Security Impact:** Provides safe command execution interface while maintaining backward compatibility
+
+### 6. Removed Wildcard Import
+**Files Modified:** [tisbackup_gui.py](tisbackup_gui.py)
+
+**Before:**
+```python
+from shutil import *
+```
+
+**After:**
+```python
+import shutil
+import subprocess
+```
+
+**Impact:** Cleaner namespace, easier to track dependencies
+
+## Remaining Security Issues (Critical - Not Fixed)
+
+### 1. **Hardcoded Secret Key** ([tisbackup_gui.py:64](tisbackup_gui.py#L64))
+```python
+app.secret_key = "fsiqefiuqsefARZ4Zfesfe34234dfzefzfe"
+```
+**Recommendation:** Load from environment variable or secure config file
+
+### 2. **No Authentication on Flask Routes**
+All routes are publicly accessible without authentication.
+
+**Recommendation:** Implement Flask-Login or similar authentication
+
+### 3. **Insecure SSH Host Key Policy** ([libtisbackup/common.py:649](libtisbackup/common.py#L649))
+```python
+ssh.set_missing_host_key_policy(paramiko.AutoAddPolicy())
+```
+**Recommendation:** Use proper host key verification with known_hosts
+
+### 4. **Command Injection in Legacy Code**
+Multiple files still use `subprocess.call(shell_string, shell=True)` and `subprocess.Popen(..., shell=True)`:
+- [libtisbackup/common.py:128](libtisbackup/common.py#L128)
+- [libtisbackup/common.py:883](libtisbackup/common.py#L883)
+- [libtisbackup/common.py:986](libtisbackup/common.py#L986)
+- [libtisbackup/backup_rsync.py:176](libtisbackup/backup_rsync.py#L176)
+- [libtisbackup/backup_rsync_btrfs.py](libtisbackup/backup_rsync_btrfs.py) (multiple locations)
+
+**Recommendation:** Refactor to use list arguments without shell=True
+
+## Code Quality Issues Remaining
+
+1. **Global State Management** - Use Flask application context instead
+2. **Wildcard imports from common** - `from libtisbackup.common import *`
+3. **Configuration loaded at module level** - Should use application factory pattern
+4. **Duplicated code** - `read_config()` and `read_all_configs()` share significant logic
+
+## Testing Recommendations
+
+Before deploying these changes:
+1. Test USB disk detection and mounting functionality
+2. Test backup export operations
+3. Verify XVA backup tar validation
+4. Test error handling for invalid device names
+5. Verify backward compatibility with existing configurations
+
+## Migration Notes
+
+All changes are backward compatible. The `run_command()` function accepts both:
+- New format: `run_command(["/bin/command", "arg1", "arg2"])`
+- Legacy format: `run_command("/bin/command arg1 arg2")` (less secure, marked for deprecation)