Merge pull request #56 from advaitpatel/ci/enforce-tests-on-pr

advaitpatel · web-flow · commit af26c369bf65 · 2026-04-02T03:00:41.000-05:00
ci: enforce pytest as required gate on all PRs, fix pre-existing test failures
diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml
@@ -11,30 +11,28 @@ jobs:
   coverage:
     runs-on: ubuntu-latest
     name: Test Coverage Report
-    
+
     steps:
     - uses: actions/checkout@v4
-    
+
     - name: Set up Python
       uses: actions/setup-python@v4
       with:
         python-version: '3.12'
-    
+
     - name: Install dependencies
       run: |
         python -m pip install --upgrade pip
         pip install pytest pytest-cov
-        if [ -f requirements.txt ]; then pip install -r requirements.txt; fi
-    
+        pip install -r requirements.txt
+
     - name: Install package
-      run: |
-        pip install -e .
-    
+      run: pip install -e .
+
     - name: Run tests with coverage
       run: |
-        pytest tests/ --cov=. --cov-report=xml --cov-report=html --cov-report=term-missing || echo "Tests completed with coverage"
-      continue-on-error: true
-    
+        pytest tests/ --cov=. --cov-report=xml --cov-report=html --cov-report=term-missing
+
     - name: Upload coverage to Codecov
       uses: codecov/codecov-action@v4
       with:
@@ -44,7 +42,7 @@ jobs:
         fail_ci_if_error: false
       env:
         CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
-    
+
     - name: Upload coverage reports as artifact
       uses: actions/upload-artifact@v4
       with:
@@ -53,26 +51,3 @@ jobs:
           coverage.xml
           htmlcov/
         if-no-files-found: ignore
-    
-    - name: Generate Coverage Summary
-      run: |
-        echo "## Coverage Report" >> $GITHUB_STEP_SUMMARY
-        echo "" >> $GITHUB_STEP_SUMMARY
-        if [ -f coverage.xml ]; then
-          echo "Coverage report generated successfully!" >> $GITHUB_STEP_SUMMARY
-          echo "" >> $GITHUB_STEP_SUMMARY
-          echo "📊 View detailed HTML report in artifacts" >> $GITHUB_STEP_SUMMARY
-          echo "" >> $GITHUB_STEP_SUMMARY
-          # Extract coverage percentage if available
-          if command -v coverage &> /dev/null; then
-            echo "### Coverage Details:" >> $GITHUB_STEP_SUMMARY
-            coverage report --format=markdown >> $GITHUB_STEP_SUMMARY 2>/dev/null || echo "Run completed" >> $GITHUB_STEP_SUMMARY
-          fi
-        else
-          echo "⚠️ No coverage data generated" >> $GITHUB_STEP_SUMMARY
-        fi
-    
-    - name: Coverage Badge
-      run: |
-        echo "Add this badge to your README.md:"
-        echo "[![codecov](https://codecov.io/gh/advaitpatel/DockSec/branch/main/graph/badge.svg)](https://codecov.io/gh/advaitpatel/DockSec)"
diff --git a/.github/workflows/python-app.yml b/.github/workflows/python-app.yml
@@ -10,81 +10,61 @@ on:
 jobs:
   build-and-test:
     runs-on: ubuntu-latest
-    
+
     steps:
     - uses: actions/checkout@v4
-    
+
     - name: Set up Python
       uses: actions/setup-python@v4
       with:
         python-version: '3.12'
-    
+
+    - name: Install dependencies
+      run: |
+        python -m pip install --upgrade pip
+        pip install -r requirements.txt
+
+    - name: Build and install package
+      run: |
+        pip install build
+        python -m build
+        pip install -e .
+
     - name: Setup Hadolint
       run: |
         curl -sL -o hadolint https://github.com/hadolint/hadolint/releases/latest/download/hadolint-Linux-x86_64
         chmod +x hadolint
         sudo mv hadolint /usr/local/bin/
-    
+
     - name: Setup Trivy
       run: |
-        sudo apt-get update
+        sudo apt-get update -qq
         sudo apt-get install -y wget apt-transport-https gnupg lsb-release
         wget -qO - https://aquasecurity.github.io/trivy-repo/deb/public.key | sudo gpg --dearmor -o /usr/share/keyrings/trivy.gpg
         echo "deb [signed-by=/usr/share/keyrings/trivy.gpg] https://aquasecurity.github.io/trivy-repo/deb $(lsb_release -sc) main" | sudo tee /etc/apt/sources.list.d/trivy.list
-        sudo apt-get update
+        sudo apt-get update -qq
         sudo apt-get install -y trivy
-    
-    - name: Install dependencies
-      run: |
-        python -m pip install --upgrade pip
-        if [ -f requirements.txt ]; then pip install -r requirements.txt; fi
-    
-    - name: Build package
-      run: |
-        pip install build
-        python -m build
-    
-    - name: Install package in development mode
+
+    - name: Run unit tests
       run: |
-        pip install -e .
-        # Ensure all dependencies are installed (including from setup.py)
-        pip install -r requirements.txt
-    
-    - name: Run Hadolint on Dockerfiles
+        pip install pytest
+        pytest tests/ -v
+
+    - name: Lint Dockerfiles with Hadolint
       run: |
         find . -name "Dockerfile*" -exec hadolint {} \;
-    
-    - name: Run Trivy for vulnerability scanning
+
+    - name: Scan Dockerfiles with Trivy
       run: |
         find . -name "Dockerfile*" -exec trivy config {} \;
-      
-    - name: Debug folder structure
-      run: |
-        echo "Current directory: $(pwd)"
-        ls -R
-    
-    - name: Run tests
-      env:
-        OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}
+
+    - name: Verify CLI installs correctly
       run: |
-        # Test 1: Version check
-        echo "=========================================="
-        echo "Test 1: Version check"
-        echo "=========================================="
         docksec --version
-        
-        # Test 2: Help command
-        echo ""
-        echo "=========================================="
-        echo "Test 2: Help command"
-        echo "=========================================="
-        docksec -h
-        
-        # Test 3: Create a test Dockerfile
-        echo ""
-        echo "=========================================="
-        echo "Test 3: Creating test Dockerfile"
-        echo "=========================================="
+        docksec --help
+
+    - name: Run scan-only mode (no AI required)
+      run: |
         mkdir -p test_dir
         cat > test_dir/Dockerfile << 'EOF'
         FROM alpine:latest
@@ -93,68 +73,18 @@ jobs:
         COPY . .
         CMD ["sh"]
         EOF
-        cat test_dir/Dockerfile
-        
-        # Test 4: Pull Docker image for testing
-        echo ""
-        echo "=========================================="
-        echo "Test 4: Pull test Docker image"
-        echo "=========================================="
         docker pull alpine:latest
-        echo "✅ Alpine image pulled successfully"
-        
-        # Test 5: Scan-only mode (no AI)
-        echo ""
-        echo "=========================================="
-        echo "Test 5: Scan-only mode (no AI)"
-        echo "=========================================="
-        docksec test_dir/Dockerfile --scan-only -i alpine:latest || echo "Scan completed"
-        
-        # Test 6: Image-only scan
-        echo ""
-        echo "=========================================="
-        echo "Test 6: Image-only scan"
-        echo "=========================================="
-        docksec --image-only -i alpine:latest || echo "Image scan completed"
-        
-        # Test 7: Verify results directory was created
-        echo ""
-        echo "=========================================="
-        echo "Test 7: Checking results"
-        echo "=========================================="
-        if [ -d "results" ]; then
-          echo "✅ Results directory created successfully"
-          echo "Contents:"
-          ls -lh results/ 2>/dev/null || echo "No files in results (may be expected)"
-        else
-          echo "⚠️  Results directory not found"
-          echo "This may be expected if scans failed due to missing API key"
-        fi
-        
-        # Cleanup
-        echo ""
-        echo "=========================================="
-        echo "Cleanup"
-        echo "=========================================="
+        docksec test_dir/Dockerfile --scan-only -i alpine:latest
         rm -rf test_dir
-        echo "Test directory cleaned up"
-        
-        echo ""
-        echo "=========================================="
-        echo "✅ All DockSec CLI tests passed!"
-        echo "=========================================="
-        echo ""
-        echo "Summary:"
-        echo "- Version flag: ✅ Working"
-        echo "- Help command: ✅ Working"
-        echo "- CLI installation: ✅ Working"
-        echo "- Scan modes: ⚠️  Working (API key warnings expected)"
-        echo ""
-        echo "Note: Full AI features require OPENAI_API_KEY to be set in GitHub Secrets"
-    
-    - name: Upload coverage report
+
+    - name: Run image-only scan
+      run: |
+        docksec --image-only -i alpine:latest
+
+    - name: Upload scan results
       uses: actions/upload-artifact@v4
+      if: always()
       with:
-        name: coverage-report
-        path: coverage.xml
-        if-no-files-found: ignore
+        name: scan-results
+        path: results/
+        if-no-files-found: ignore
diff --git a/docker_scanner.py b/docker_scanner.py
@@ -35,12 +35,14 @@ def _validate_file_path(file_path: str) -> Path:
         """
         if not file_path:
             raise ValueError("File path cannot be empty")
-        
+
+        # Check the raw string before resolution — Path.resolve() removes '..'
+        # so checking the resolved path would silently allow traversal attempts.
+        if '..' in file_path:
+            raise ValueError(f"Invalid path: path traversal detected in '{file_path}'")
+
         try:
             path = Path(file_path).resolve()
-            # Check for path traversal (parent directory access)
-            if '..' in str(path):
-                raise ValueError(f"Invalid path: path traversal detected in '{file_path}'")
             return path
         except (OSError, ValueError) as e:
             raise ValueError(f"Invalid file path '{file_path}': {str(e)}")
@@ -71,11 +73,10 @@ def _validate_image_name(image_name: str) -> str:
         if '..' in image_name or image_name.startswith('/'):
             raise ValueError(f"Image name contains path traversal or absolute path: '{image_name}'")
         
-        # Check for dangerous characters that could be used in command injection
-        dangerous_chars = [';', '&', '|', '`', '$', '(', ')', '<', '>', '\n', '\r']
-        for char in dangerous_chars:
-            if char in image_name:
-                raise ValueError(f"Image name contains invalid character: '{char}'")
+        # Whitelist: Docker image names allow alphanumeric, '/', ':', '-', '_', '.', '@'
+        # Anything outside this set (spaces, shell metacharacters, etc.) is rejected.
+        if not re.match(r'^[a-zA-Z0-9/:._\-@]+$', image_name):
+            raise ValueError(f"Image name contains invalid characters: '{image_name}'")
         
         return image_name.strip()
     
diff --git a/tests/test_docker_scanner.py b/tests/test_docker_scanner.py
@@ -47,7 +47,9 @@ def test_init_with_valid_inputs(self, mock_llm, mock_subprocess):
         from docker_scanner import DockerSecurityScanner
         
         scanner = DockerSecurityScanner(dockerfile, "test:latest")
-        self.assertEqual(scanner.dockerfile_path, dockerfile)
+        # Compare resolved paths — on macOS tempfile returns /var/... but
+        # _validate_file_path resolves it to /private/var/... via symlink.
+        self.assertEqual(scanner.dockerfile_path, str(Path(dockerfile).resolve()))
         self.assertEqual(scanner.image_name, "test:latest")
         self.assertIsNone(scanner.analysis_score)
     
diff --git a/tests/test_integration.py b/tests/test_integration.py
@@ -3,6 +3,7 @@
 import os
 import tempfile
 import shutil
+from pathlib import Path
 from unittest.mock import patch, Mock
 
 # Import after mocking external dependencies
@@ -54,7 +55,9 @@ def subprocess_side_effect(*args, **kwargs):
         
         # Verify initialization
         self.assertIsNotNone(scanner)
-        self.assertEqual(scanner.dockerfile_path, self.test_dockerfile)
+        # Compare resolved paths — on macOS tempfile returns /var/... but
+        # _validate_file_path resolves it to /private/var/... via symlink.
+        self.assertEqual(scanner.dockerfile_path, str(Path(self.test_dockerfile).resolve()))
         self.assertEqual(scanner.image_name, "test:latest")
     
     @patch('docker_scanner.subprocess.run')
diff --git a/tests/test_utils.py b/tests/test_utils.py
@@ -43,28 +43,43 @@ def test_load_docker_file_not_found(self):
         result = load_docker_file("/nonexistent/path/Dockerfile")
         self.assertIsNone(result)
     
-    @patch('utils.get_openai_api_key')
     @patch('utils.ChatOpenAI')
-    def test_get_llm(self, mock_chatopenai, mock_api_key):
-        """Test LLM initialization."""
+    @patch('config_manager.get_config')
+    def test_get_llm(self, mock_get_config, mock_chatopenai):
+        """Test LLM initialization with a mocked config and mocked ChatOpenAI."""
         from utils import get_llm
-        
-        mock_api_key.return_value = "test-api-key"
+
+        mock_config = Mock()
+        mock_config.llm_provider = "openai"
+        mock_config.llm_model = "gpt-4o"
+        mock_config.llm_temperature = 0.0
+        mock_config.timeout_llm = 60
+        mock_config.max_retries_llm = 2
+        mock_config.get_api_key_for_provider.return_value = "test-api-key"
+        mock_get_config.return_value = mock_config
+
         mock_llm_instance = Mock()
         mock_chatopenai.return_value = mock_llm_instance
-        
+
         llm = get_llm()
-        
+
         mock_chatopenai.assert_called_once()
         self.assertIsNotNone(llm)
-    
-    @patch('utils.get_openai_api_key')
-    def test_get_llm_no_api_key(self, mock_api_key):
-        """Test LLM initialization without API key."""
+
+    @patch('config_manager.get_config')
+    def test_get_llm_no_api_key(self, mock_get_config):
+        """Test LLM initialization raises EnvironmentError when API key is missing."""
         from utils import get_llm
-        
-        mock_api_key.side_effect = EnvironmentError("API key not found")
-        
+
+        mock_config = Mock()
+        mock_config.llm_provider = "openai"
+        mock_config.llm_model = "gpt-4o"
+        mock_config.llm_temperature = 0.0
+        mock_config.timeout_llm = 60
+        mock_config.max_retries_llm = 2
+        mock_config.get_api_key_for_provider.side_effect = EnvironmentError("API key not found")
+        mock_get_config.return_value = mock_config
+
         with self.assertRaises(EnvironmentError):
             get_llm()
 
