Skip to content

Fix: Bug fixes and code quality improvements#15

Merged
advaitpatel merged 3 commits into
mainfrom
Fix-Bug-fixes-and-code-quality-improvements
Nov 1, 2025
Merged

Fix: Bug fixes and code quality improvements#15
advaitpatel merged 3 commits into
mainfrom
Fix-Bug-fixes-and-code-quality-improvements

Conversation

@advaitpatel

@advaitpatel advaitpatel commented Nov 1, 2025

Copy link
Copy Markdown
Collaborator

Overview

This PR addresses 9 critical bugs and code quality issues to improve production readiness, security, and maintainability of DockSec.


  • Initialize self.analysis_score = None in __init__ method
  • Added _validate_file_path() to prevent path traversal attacks
  • Added _validate_image_name() to prevent command injection
  • Added _validate_severity() to validate severity levels
  • All inputs validated before use
  • All subprocess calls now use shell=False explicitly
  • Added timeout parameters (10s for tool checks, 300-600s for scans)
  • Input validation prevents dangerous characters
  • Added proper error handling for timeouts
  • Implemented lazy loading via get_openai_api_key() function
  • API key only checked when AI features are actually needed
  • Scan-only mode (--scan-only) works without API key
  • Returns proper Dict structure with success/error information
  • Renamed class and updated all imports
  • Removed debug print statements from config.py and docker_scanner.py
  • Replaced with os.path.join() for cross-platform compatibility
  • Improved error handling in validation methods and subprocess calls with proper

Advait Patel added 3 commits October 31, 2025 22:50
## Fix: Bug Fixes and Code Quality Improvements (Items #2-10)

**Related to:** Items #2-10 from CODE_REVIEW_AND_IMPROVEMENTS.md

### Overview
This PR addresses 9 critical bugs and code quality issues to improve production readiness, security, and maintainability of DockSec.

---

### Security Fixes

#### Item #2: Fix missing attribute error
- **Problem**: `self.analysis_score` was accessed before initialization, causing AttributeError
- **Solution**: Initialize `self.analysis_score = None` in `__init__` method
- **Files**: `docker_scanner.py`

#### Item #4: Add input validation and sanitization
- **Problem**: No validation for file paths, image names, or severity inputs - security risk
- **Solution**:
  - Added `_validate_file_path()` to prevent path traversal attacks
  - Added `_validate_image_name()` to prevent command injection
  - Added `_validate_severity()` to validate severity levels
  - All inputs validated before use
- **Files**: `docker_scanner.py`

#### Item #5: Fix subprocess command injection vulnerabilities
- **Problem**: User input directly passed to subprocess without sanitization
- **Solution**:
  - All subprocess calls now use `shell=False` explicitly
  - Added timeout parameters (10s for tool checks, 300-600s for scans)
  - Input validation prevents dangerous characters
  - Added proper error handling for timeouts
- **Files**: `docker_scanner.py`

---

### Functionality Fixes

#### Item #3: Improve API key error handling
- **Problem**: API key check happened on import, breaking scan-only mode without API key
- **Solution**:
  - Implemented lazy loading via `get_openai_api_key()` function
  - API key only checked when AI features are actually needed
  - Scan-only mode (`--scan-only`) works without API key
- **Files**: `config.py`, `utils.py`

#### Item #10: Fix missing return statement in `advanced_scan()`
- **Problem**: Method signature declared `Dict` return but returned `int(0)` on error
- **Solution**: Returns proper `Dict` structure with success/error information
- **Files**: `docker_scanner.py`

---

### Code Quality Improvements

#### Item #6: Fix typo in class name
- **Problem**: Class named `AnalsesResponse` (typo) instead of `AnalyzesResponse`
- **Solution**: Renamed class and updated all imports
- **Files**: `utils.py`, `docksec.py`, `main.py`

#### Item #8: Remove debug print statements
- **Problem**: Debug prints left in production code
- **Solution**: Removed debug print statements from `config.py` and `docker_scanner.py`
- **Files**: `config.py`, `docker_scanner.py`

#### Item #9: Fix Windows path handling
- **Problem**: Hardcoded Windows path (`r"testfiles\1\Dockerfile"`) breaks on Unix systems
- **Solution**: Replaced with `os.path.join()` for cross-platform compatibility
- **Files**: `main.py`

#### Item #7: Improve error handling consistency
- **Problem**: Inconsistent error handling throughout codebase
- **Solution**: Improved error handling in validation methods and subprocess calls with proper exceptions and timeouts
- **Files**: `docker_scanner.py`

---

### Changes Summary

**Files Modified:**
- `docker_scanner.py` - Major security and validation improvements
- `config.py` - Lazy API key loading
- `utils.py` - Typo fix, lazy loading integration
- `main.py` - Cross-platform path handling, typo fix
- `docksec.py` - Typo fix

**Lines Changed:** ~200+ lines

---

### Testing

✅ Comprehensive test suite created and all tests passing:
- Item #2: `analysis_score` initialization verified
- Item #3: API key lazy loading verified
- Item #4: Input validation methods verified
- Item #5: Subprocess security verified (6 calls with `shell=False` and timeouts)
- Item #6: Typo fix verified across all files
- Item #8: Debug prints removal verified
- Item #9: Windows path fix verified
- Item #10: Return type fix verified

**Test Results:**
@advaitpatel
advaitpatel merged commit 3d05256 into main Nov 1, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant