Skip to content

Conversation

@HeshamHM28
Copy link
Contributor

@HeshamHM28 HeshamHM28 commented Feb 2, 2026

Summary

This PR addresses critical security vulnerabilities and improves error handling in the CodeFlash Java implementation.

Security Fixes

  • Command Injection Prevention: Added validation for test class names before passing to Maven, preventing shell injection attacks
  • XXE Attack Prevention: Implemented safe XML parsing to prevent XML External Entity attacks when parsing POM files and Surefire reports
  • Input Sanitization: Added comprehensive validation for Maven test filters

Error Handling Improvements

  • Robust error handling for malformed XML in Surefire reports
  • Safe handling of invalid numeric values in test result attributes
  • Try-catch blocks around integer conversions to prevent crashes

Changes

test_runner.py:

  • Added _validate_java_class_name() to validate Java identifier syntax
  • Added _validate_test_filter() to sanitize test filter strings
  • Validate test class names in get_test_run_command() before passing to Maven
  • Use regex pattern to ensure only valid Java identifiers are accepted

build_tools.py:

  • Added _safe_parse_xml() for secure XML parsing
  • Replaced all ET.parse() calls with secure version
  • Added validation and error handling for numeric XML attributes in Surefire reports
  • Handle malformed XML gracefully with logging

Test Coverage

Added comprehensive test suite (test_security.py) with 16 tests covering:

  • Input validation (positive and negative cases)
  • Command injection prevention
  • XML parsing security
  • Error handling edge cases
  • Special characters and wildcards

All tests pass ✅

Impact

  • Security: Prevents command injection and XXE attacks
  • Reliability: Handles malformed input gracefully instead of crashing
  • Maintainability: Clear validation logic with comprehensive tests

Test plan

  • All existing tests pass
  • New security tests pass (16/16)
  • Manual testing with valid and malicious input

Ubuntu and others added 2 commits February 2, 2026 23:13
Security fixes:
- Add validation for test class names to prevent command injection (CVE-level)
- Implement safe XML parsing to prevent XXE attacks
- Add input sanitization for Maven test filters

Error handling improvements:
- Add robust error handling for malformed XML in Surefire reports
- Handle invalid numeric values in test result attributes
- Add try-catch blocks around integer conversions

Changes:
- test_runner.py: Add _validate_java_class_name() and _validate_test_filter()
- test_runner.py: Validate test class names before passing to Maven
- build_tools.py: Add _safe_parse_xml() for secure XML parsing
- build_tools.py: Replace all ET.parse() calls with secure version
- build_tools.py: Add validation for numeric XML attributes

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Added test coverage for:
- Input validation (command injection prevention)
- Test class name validation with positive and negative cases
- Test filter validation including wildcards
- XML parsing security (XXE attack prevention)
- Error handling for malformed XML
- Error handling for invalid numeric attributes
- Edge cases (empty strings, whitespace, special characters)

All tests pass. This ensures the security fixes work correctly
and prevents regressions.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Ubuntu seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@HeshamHM28 HeshamHM28 merged commit 906cd0c into omni-java Feb 2, 2026
17 of 27 checks passed
@HeshamHM28 HeshamHM28 deleted the fix/java-security-improvements branch February 2, 2026 23:40
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.

4 participants