feat: Separate vulnerability types from code#29
Open
farnaboldi wants to merge 3 commits intopsyray:masterfrom
Open
feat: Separate vulnerability types from code#29farnaboldi wants to merge 3 commits intopsyray:masterfrom
farnaboldi wants to merge 3 commits intopsyray:masterfrom
Conversation
Major architectural enhancement that makes vulnerability definitions modular and extensible: BREAKING CHANGES: - Vulnerability definitions moved from hardcoded config to JSON files - New vulnerability/ directory structure with 25 vulnerability types - Dynamic loading system with backward compatibility fallback NEW FEATURES: - 9 additional vulnerability types: lfi, rfi, redirect, cmdi, debug, deser, upload, cors, jwt - Extensible system allowing custom vulnerability definitions - JSON-based vulnerability schema for easy maintenance - Automatic detection and loading of custom vulnerability files ENHANCEMENTS: - Users can now add custom vulnerabilities by creating JSON files in vulnerability/ - Vulnerability definitions separated from application logic for better maintainability - Support for organization-specific and domain-specific vulnerability patterns - Enhanced help system displaying all available vulnerability types FILES ADDED: - vulnerability/: New directory with 25 vulnerability definition files - vulnerability/*.json: Individual JSON files for each vulnerability type FILES MODIFIED: - oasis/config.py: Replaced hardcoded VULNERABILITY_MAPPING with dynamic loading system USAGE: - Existing: oasis -i ./code -v sqli,xss,rce (unchanged) - New types: oasis -i ./app -v jwt,cors,upload,debug - Custom: oasis -i ./code -v customvuln (if vulnerability/customvuln.json exists) - Mixed: oasis -i ./api -v sqli,jwt,upload,cors This change enables OASIS to be customized for specific security requirements while maintaining full backward compatibility.
…AdaptiveAnalysis classes Resolves AttributeError where BatchAdaptiveAnalysis object was missing 'args' attribute needed for _should_disable_progress() function calls. CHANGES: - AdaptiveAnalysisPipeline.__init__: Added self.args = analyzer.args - BatchAdaptiveAnalysis.__init__: Added self.args = pipeline.analyzer.args This ensures proper args propagation through the class hierarchy: SecurityAnalyzer.args -> AdaptiveAnalysisPipeline.args -> BatchAdaptiveAnalysis.args Fixes error: "'BatchAdaptiveAnalysis' object has no attribute 'args'"
Contributor
Reviewer's GuideThis PR externalizes the vulnerability type definitions into separate JSON files and implements a dynamic loader with a legacy fallback in config.py, while also propagating CLI arguments into analyzer and pipeline classes. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `oasis/config.py:260` </location>
<code_context>
- ],
- 'impact': 'Unauthorized actions performed on behalf of authenticated users',
- 'mitigation': 'Use CSRF tokens, SameSite cookies, and verify request origins'
+def load_vulnerability_definitions() -> Dict[str, Any]:
+ """
+ Load vulnerability definitions from JSON files in the vulnerability/ directory.
</code_context>
<issue_to_address>
Consider using logging instead of print for error reporting.
The logging module provides configurable log levels and integrates with standard logging systems, making it more suitable for production error reporting than print.
Suggested implementation:
```python
import logging
def load_vulnerability_definitions() -> Dict[str, Any]:
```
```python
vulnerabilities = {}
# Optionally set up basic logging configuration if not already set elsewhere
logging.basicConfig(level=logging.INFO)
# Get the directory containing this config.py file
config_dir = Path(__file__).parent
# Look for vulnerability directory relative to the project root
```
If there are any `print` statements for error reporting in the rest of the `load_vulnerability_definitions` function (or elsewhere in this file), replace them with `logging.error`, `logging.warning`, or `logging.info` as appropriate. For example:
`print("Error loading vulnerability definitions")` should become `logging.error("Error loading vulnerability definitions")`.
</issue_to_address>
### Comment 2
<location> `oasis/config.py:280` </location>
<code_context>
+ return _get_legacy_vulnerability_mapping()
+
+ # Load all JSON files from vulnerability directory
+ for json_file in vuln_dir.glob("*.json"):
+ try:
+ vuln_key = json_file.stem # filename without extension
+ with open(json_file, 'r', encoding='utf-8') as f:
</code_context>
<issue_to_address>
Consider handling duplicate vulnerability keys from JSON files.
Currently, entries with the same key are overwritten without notice. Consider implementing a check or warning to prevent silent overwrites when duplicate keys are encountered.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
for json_file in vuln_dir.glob("*.json"):
try:
vuln_key = json_file.stem # filename without extension
with open(json_file, 'r', encoding='utf-8') as f:
vuln_data = json.load(f)
vulnerabilities[vuln_key] = vuln_data
except (json.JSONDecodeError, FileNotFoundError, PermissionError) as e:
# Log error but continue loading other vulnerabilities
print(f"Warning: Could not load vulnerability definition from {json_file}: {e}")
continue
=======
for json_file in vuln_dir.glob("*.json"):
try:
vuln_key = json_file.stem # filename without extension
if vuln_key in vulnerabilities:
print(f"Warning: Duplicate vulnerability key '{vuln_key}' found in {json_file}. Skipping this file to avoid overwriting existing entry.")
continue
with open(json_file, 'r', encoding='utf-8') as f:
vuln_data = json.load(f)
vulnerabilities[vuln_key] = vuln_data
except (json.JSONDecodeError, FileNotFoundError, PermissionError) as e:
# Log error but continue loading other vulnerabilities
print(f"Warning: Could not load vulnerability definition from {json_file}: {e}")
continue
>>>>>>> REPLACE
</suggested_fix>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| ], | ||
| 'impact': 'Unauthorized actions performed on behalf of authenticated users', | ||
| 'mitigation': 'Use CSRF tokens, SameSite cookies, and verify request origins' | ||
| def load_vulnerability_definitions() -> Dict[str, Any]: |
Contributor
There was a problem hiding this comment.
suggestion: Consider using logging instead of print for error reporting.
The logging module provides configurable log levels and integrates with standard logging systems, making it more suitable for production error reporting than print.
Suggested implementation:
import logging
def load_vulnerability_definitions() -> Dict[str, Any]: vulnerabilities = {}
# Optionally set up basic logging configuration if not already set elsewhere
logging.basicConfig(level=logging.INFO)
# Get the directory containing this config.py file
config_dir = Path(__file__).parent
# Look for vulnerability directory relative to the project rootIf there are any print statements for error reporting in the rest of the load_vulnerability_definitions function (or elsewhere in this file), replace them with logging.error, logging.warning, or logging.info as appropriate. For example:
print("Error loading vulnerability definitions") should become logging.error("Error loading vulnerability definitions").
Owner
|
Thanks for the PR |
psyray
changed the title
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| analyzer: SecurityAnalyzer instance with required methods and attributes | ||
| """ | ||
| self.analyzer = analyzer | ||
| self.args = analyzer.args |
There was a problem hiding this comment.
Bug: Analyzer Args Missing, Pipeline Initialization Error
AdaptiveAnalysisPipeline.__init__ tries to access analyzer.args, but SecurityAnalyzer never stores the args parameter as an instance attribute. The SecurityAnalyzer.__init__ only extracts specific attributes from args but doesn't save args itself, causing an AttributeError when AdaptiveAnalysisPipeline is instantiated.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Separates vulnerability types from the code, allowing easier modification of existing types, the addition of new ones, and to effortless import private ones
Summary by Sourcery
Separate vulnerability type metadata from code by moving definitions into a vulnerability/ directory of JSON files with a legacy fallback, and update analysis classes to capture and store args.
New Features:
Enhancements:
Note
Externalizes vulnerability type metadata into
vulnerability/*.jsonand dynamically loads them at runtime, with pipelines now storingargs.load_vulnerability_definitions()and_get_legacy_vulnerability_mapping(); setVULNERABILITY_MAPPING = load_vulnerability_definitions().vulnerability/JSON files):sqli,xss,auth,session,csrf,crypto,ssrf,rce,cmdi,pathtra,idor,deser,rfi,lfi,redirect,logging,data,cors,jwt,upload,debug,xxe.argsonAdaptiveAnalysisPipelineandBatchAdaptiveAnalysisconstructors for downstream use.Written by Cursor Bugbot for commit 4ae588b. This will update automatically on new commits. Configure here.