Cflite fuzzing adding #165
Conversation
Signed-off-by: Shahm Najeeb <[email protected]>
DefinetlyNotAI
added
type/Github Actions
WalkthroughThis PR introduces several GitHub Actions workflows for ClusterFuzzLite, automating various tasks related to fuzz testing. It adds three new workflow files: one for executing batch fuzzing every six hours, another for continuous integration builds triggered by code pushes, and a third for daily pruning tasks. Additionally, updates are made to the debug and checks code to enhance logging practices and functionality. Changes
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Signed-off-by: Shahm Najeeb <[email protected]>
Signed-off-by: Shahm Najeeb <[email protected]>
pull-request-size
bot
added
size/L
Signed-off-by: Shahm Najeeb <[email protected]>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 4
🧹 Nitpick comments (4)
.github/workflows/cflite_build.yml (1)
16-20: Hey! Let's power up your fuzzing game! 🚀You've only got the address sanitizer enabled rn, but there's more firepower available! The undefined and memory sanitizers can catch different types of bugs. Consider uncommenting them for better coverage.
sanitizer: - address -# Override this with the sanitizers you want. -# - undefined -# - memory +- undefined +- memory.github/workflows/cflite_pr.yml (3)
1-6: Hey! Let's make the path filter more specific! 🎯Right now, your workflow runs on literally every file change (
'**'). That's like checking your entire backpack when you only need to look in the front pocket! Let's make it run only when relevant files change.Try this instead:
paths: - - '**' + - '**/*.py' # Python files + - '**/fuzz_*.py' # Fuzzing targets + - '.github/workflows/cflite_*.yml' # Fuzzing workflows
13-20: Yo! Why not level up with more sanitizers? 🚀The address sanitizer is cool, but you've got other sanitizers commented out that could catch different types of bugs:
undefinedcatches undefined behavior (like when you divide by zero 😱)memoryfinds memory leaks (when your code forgets to clean up after itself 🧹)Consider uncommenting them if your code is ready for that extra security check:
sanitizer: - address - # - undefined - # - memory + - undefined + - memory
35-44: 10 minutes might not be enough time to find all the bugs! ⏰Your fuzzers only run for 600 seconds (that's 10 minutes). For a bigger codebase, that might not be enough time to find the tricky bugs hiding in the corners!
Maybe bump it up a bit:
- fuzz-seconds: 600 + fuzz-seconds: 1800 # 30 minutesBTW, nice job enabling SARIF output! That'll show your fuzzing results right in GitHub's security dashboard. That's super cool! 🎉
🛑 Comments failed to post (4)
.github/workflows/cflite_build.yml (1)
26-28: 💡 Codebase verification
⚠️ Potential issueHey! Your builds might go poof into thin air! 🌪️
So here's the deal - you've got
upload-build: truein your workflow, but I checked and all the storage-repo stuff is just sitting there in comments (you know, those lines starting with #). That's like telling your phone to save pics but not giving it any storage! Not cool!You've got two options to fix this:
- Either uncomment and set up that storage-repo (just remove the # and fill in your actual repo details)
- Or set
upload-build: falseif you don't need to save the builds🔗 Analysis chain
Heads up about that upload-build setting! 📦
You've got
upload-build: truebut no storage repo configured. This might cause your builds to disappear into the void! You might wanna set up a storage repo like in the other workflow files.
Let me check if this is a commented example or if there are any actual storage repo configurations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if storage repo is configured in any of the workflow files rg "storage-repo:" .github/workflows/Length of output: 602
Script:
#!/bin/bash # Check the actual content of these workflow files to see if the storage-repo is actually configured or just commented out cat .github/workflows/cflite_pr.yml .github/workflows/cflite_cron.yml .github/workflows/cflite_batch.yml .github/workflows/cflite_build.yml | grep -A 2 -B 2 "storage-repo:"Length of output: 1615
.github/workflows/cflite_cron.yml (2)
24-27: 🛠️ Refactor suggestion
Storage repo setup needs some love! 💾
Those commented storage repo settings are super important for keeping your fuzzing history and coverage data! Without them, you won't be able to track your progress over time.
Here's what you need to do:
- Create a storage repo
- Generate a Personal Access Token with repo scope
- Add it to your repo's secrets
- Uncomment and configure these lines
13-14:
⚠️ Potential issueYo! Where's your sanitizer config at? 🤔
The build step is missing the sanitizer config that's in the other workflow files. This might limit what kinds of bugs you can catch!
with: language: python # Change this to the language you are fuzzing + sanitizer: address📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.with: language: python # Change this to the language you are fuzzing sanitizer: address.github/workflows/cflite_batch.yml (1)
33-37: 🛠️ Refactor suggestion
Time to get that storage repo going! 📊
Just like in the other workflows, you'll want to uncomment and configure these storage settings. This is super important for:
- Tracking your fuzzing progress
- Storing coverage data
- Keeping historical findings
Without this, you're missing out on valuable insights about your fuzzing effectiveness!
Signed-off-by: Shahm Najeeb <[email protected]>
pull-request-size
bot
added
size/M
|
@ski-sketch approve tmrw, after I commit, k |
Now its better and more efficient, as well as less redundant, new function added to Checks.py which allows you to check the execution policy Signed-off-by: Shahm Najeeb <[email protected]>
## Pull Request Template ### Prerequisites <!-- Take a couple of minutes to help our maintainers work faster by checking of the pre-requisites. --> <!-- To tick the checkboxes replace the space with an 'x', so [ ] becomes [x] . --> - [x] I have [searched](https://github.com/DefinetlyNotAI/Logicytics/pulls) for duplicate or closed issues. - [x] I have read the [contributing guidelines](https://github.com/DefinetlyNotAI/Logicytics/blob/main/CONTRIBUTING.md). - [x] I have followed the instructions in the [wiki](https://github.com/DefinetlyNotAI/Logicytics/wiki) about contributions. - [ ] I have updated the documentation accordingly, if required. - [x] I have tested my code with the `--dev` flag, if required. ### PR Type <!-- Take a couple of minutes to help our maintainers work faster by telling us what is the PR guided on. --> <!-- To tick the checkboxes replace the space with an 'x', so [ ] becomes [x] . --> - [x] Bug fix <!-- Non-Breaking Bug Fix - Usually relates to fixing an issue --> - [x] New feature <!-- Non-Breaking Change that adds a new feature --> - [ ] Refactoring <!-- Non-Breaking Change that modifies existing code to refactor it to become more organised --> - [ ] Documentation update <!-- Non-Breaking Change that modifies existing documentation to refactor it or add extra comments - either wiki, md files or code is included here --> - [ ]⚠️ Breaking change⚠️ <!-- Breaking Bug Fix / New Addition that changes how Logicytics works --> ### Description This pull request includes several changes to the `CODE/_debug.py` and `CODE/logicytics/Checks.py` files to improve logging and refactor code. The most significant changes involve modifying methods to use logging instead of returning status messages, adding new methods for checking Python version and execution policy, and removing the return of status messages from several methods. #### Logging improvements: * [`CODE/_debug.py`](diffhunk://#diff-cddd7a4c003554a2fce729e954458aa39d397bf2910c94cdd093f39747848061L6-L36): Modified the `HealthCheck.check_files`, `HealthCheck.compare_versions`, and `DebugCheck.sys_internal_binaries` methods to use logging instead of returning status messages. [[1]](diffhunk://#diff-cddd7a4c003554a2fce729e954458aa39d397bf2910c94cdd093f39747848061L6-L36) [[2]](diffhunk://#diff-cddd7a4c003554a2fce729e954458aa39d397bf2910c94cdd093f39747848061L71-L78) [[3]](diffhunk://#diff-cddd7a4c003554a2fce729e954458aa39d397bf2910c94cdd093f39747848061L95-L135) [[4]](diffhunk://#diff-cddd7a4c003554a2fce729e954458aa39d397bf2910c94cdd093f39747848061L148-R144) * [`CODE/_debug.py`](diffhunk://#diff-cddd7a4c003554a2fce729e954458aa39d397bf2910c94cdd093f39747848061R161-R174): Added a new method `python_version` to log the Python version and its compatibility status. #### Code refactoring: * [`CODE/_debug.py`](diffhunk://#diff-cddd7a4c003554a2fce729e954458aa39d397bf2910c94cdd093f39747848061L6-L36): Removed the return of status messages from the `HealthCheck.check_files`, `HealthCheck.compare_versions`, `DebugCheck.sys_internal_binaries`, and `DebugCheck.execution_policy` methods. [[1]](diffhunk://#diff-cddd7a4c003554a2fce729e954458aa39d397bf2910c94cdd093f39747848061L6-L36) [[2]](diffhunk://#diff-cddd7a4c003554a2fce729e954458aa39d397bf2910c94cdd093f39747848061L71-L78) [[3]](diffhunk://#diff-cddd7a4c003554a2fce729e954458aa39d397bf2910c94cdd093f39747848061L95-L135) [[4]](diffhunk://#diff-cddd7a4c003554a2fce729e954458aa39d397bf2910c94cdd093f39747848061L148-R144) * [`CODE/_debug.py`](diffhunk://#diff-cddd7a4c003554a2fce729e954458aa39d397bf2910c94cdd093f39747848061L204-R194): Simplified the `debug` method by removing redundant logging calls and using the new `python_version` method. [[1]](diffhunk://#diff-cddd7a4c003554a2fce729e954458aa39d397bf2910c94cdd093f39747848061L204-R194) [[2]](diffhunk://#diff-cddd7a4c003554a2fce729e954458aa39d397bf2910c94cdd093f39747848061L225-R208) #### New method addition: * [`CODE/logicytics/Checks.py`](diffhunk://#diff-83131f7977935512c18a758ef3ff85db0152c6f8277d99eb94b0bb6830c70a08R25-R33): Added a new method `Check.execution_policy` to check if the execution policy is unrestricted. These changes enhance the logging capabilities and simplify the code by removing unnecessary return values and redundant logging calls. Now its better and more efficient, as well as less redundant, new function added to Checks.py which allows you to check the execution policy ### Motivation and Context <!-- REQUIRED: Why is this PR required? What problem does it solve? Why do you want to do it? --> Fixing huge bug with `--dev` and `--debug` where the logic was both broken and failed ### Credit <!-- If this PR is a contribution, please mention the contributors here using the appropriate syntax. --> <!-- ### File-Created/CONTRIBUTION by MAIN-Username What you did, created, removed, refactored, fixed, or discovered. - [Your GitHub Username](https://github.com/YourGitHubLink) - [Your GitHub Username](https://github.com/YourGitHubLink) etc... --> _N/A_ ### Issues Fixed <!-- REQUIRED: What issues will be fixed? (Format: "#50, #23" etc.) if none exist type _N/A_ --> _N/A_ <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Enhanced logging for health and debug checks. - Added a new function to check the Python version. - Introduced a method to retrieve and check the system's execution policy. - **Bug Fixes** - Removed return values from several methods to streamline functionality. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
pull-request-size
bot
added
size/L
DefinetlyNotAI
added
bug/High
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
CODE/_debug.py (1)
Line range hint
23-72: Watch out for sneaky path tricks! 🕵️♂️The way we're handling file paths could let someone access files they shouldn't! Also, we could make this faster!
def check_files(directory: str, required_files: list[str]): + directory = os.path.abspath(directory) # Prevent path traversal try: log_debug.debug(f"Checking directory: {directory}") - if not os.path.exists(directory): + if not os.path.exists(directory) or not os.path.isdir(directory): log_debug.error(f"Directory {directory} does not exist or is not a directory.") + return + # Use set for faster lookups + actual_files = set() - actual_files = []
🧹 Nitpick comments (2)
CODE/logicytics/Checks.py (1)
Line range hint
1-85: Hey! Let's make this code more beginner-friendly! 📚Some methods are missing those cool docstrings that explain what they do. Also, we should handle errors better in case things go wrong!
Add docstrings to
execution_policylike this:@staticmethod def execution_policy() -> bool: """ Check if PowerShell execution policy is set to unrestricted. Returns: bool: True if execution policy is unrestricted, False otherwise. Note: This method requires PowerShell to be available on the system. """CODE/_debug.py (1)
161-173: Let's make version checking more flexible! 🐍Hardcoding Python 3.11 isn't cool - what happens when Python 3.12 drops? Also, we should check if all our required packages work!
def python_version(): + MIN_VERSION = (3, 11) + MAX_VERSION = (3, 12) version = sys.version.split()[0] try: major, minor = map(int, version.split(".")[:2]) - if (major, minor) == (3, 11): + if MIN_VERSION <= (major, minor) < MAX_VERSION: log_debug.info(f"Python Version: {version} - Perfect") - elif major == 3: + elif (major, minor) < MIN_VERSION: log_debug.warning(f"Python Version: {version} - Recommended: {MIN_VERSION[0]}.{MIN_VERSION[1]}.x") else: log_debug.error(f"Python Version: {version} - Incompatible")
🛑 Comments failed to post (3)
CODE/logicytics/Checks.py (1)
25-32:
⚠️ Potential issueYo, heads up! We need to make this PowerShell command more secure! 🔒
Running PowerShell commands without input validation is kinda risky - like leaving your front door wide open! Let's add some safety checks:
@staticmethod def execution_policy() -> bool: + # Validate PowerShell exists first + if not os.path.exists("C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe"): + return False try: result = subprocess.run( ["powershell", "-Command", "Get-ExecutionPolicy"], capture_output=True, text=True, + timeout=5 # Don't hang forever ) + return result.returncode == 0 and result.stdout.strip().lower() == "unrestricted" + except (subprocess.TimeoutExpired, subprocess.SubprocessError) as e: + return False - return result.stdout.strip().lower() == "unrestricted"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.@staticmethod def execution_policy() -> bool: # Validate PowerShell exists first if not os.path.exists("C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe"): return False try: result = subprocess.run( ["powershell", "-Command", "Get-ExecutionPolicy"], capture_output=True, text=True, timeout=5 # Don't hang forever ) return result.returncode == 0 and result.stdout.strip().lower() == "unrestricted" except (subprocess.TimeoutExpired, subprocess.SubprocessError) as e: return FalseCODE/_debug.py (2)
14-18:
⚠️ Potential issueTime to level up our logging game! 📝
Leaving debug logs around is like posting your diary online - not a great idea! Also, these logs could eat up your disk space!
Let's make it better:
+ from logging.handlers import RotatingFileHandler + import os.path log_debug = Log( {"log_level": DEBUG, - "filename": "../ACCESS/LOGS/DEBUG/DEBUG.log", + "filename": os.path.join(os.path.dirname(__file__), "..", "ACCESS", "LOGS", "DEBUG", "DEBUG.log"), + "max_bytes": 10485760, # 10MB + "backup_count": 3, "truncate_message": False} )Committable suggestion skipped: line range outside the PR's diff.
96-113: 🛠️ Refactor suggestion
Let's fix how we compare versions! 🔢
Just comparing version strings like regular text can give wrong results. Like, "2.10" would look smaller than "2.9" - oops!
+ from packaging import version @staticmethod def compare_versions(local_version: str, remote_version: str): + try: + local = version.parse(local_version) + remote = version.parse(remote_version) - if local_version == remote_version: + if local == remote: log_debug.info(f"Version is up to date. Your Version: {local_version}") - elif local_version > remote_version: + elif local > remote:Committable suggestion skipped: line range outside the PR's diff.
Signed-off-by: Shahm Najeeb <[email protected]>
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 4
🧹 Nitpick comments (2)
CODE/_debug.py (2)
34-44: Let's make sure our version strings are legit! 🎯The version parser is pretty smart, but it might crash with weird version strings. Let's add some checks!
Here's how to make it safer:
def __version_tuple(version: str) -> tuple[int, int, int | str, str]: + if not version or not isinstance(version, str): + log_debug.error(f"Invalid version format: {version}") + return 0, 0, 0, "error" + try:
180-193: Make those Python versions easy to update! 🐍Right now, the version numbers are hardcoded. What if we need to change them later?
Let's make them configurable:
+# At the top of the file with other constants +PYTHON_VERSION_CONFIG = { + "min": (3, 11), + "max": (3, 13), + "recommended": "3.11.x" +} + def python_version(): version = sys.version.split()[0] - MIN_VERSION = (3, 11) - MAX_VERSION = (3, 13) + MIN_VERSION = PYTHON_VERSION_CONFIG["min"] + MAX_VERSION = PYTHON_VERSION_CONFIG["max"]
🛑 Comments failed to post (4)
CODE/logicytics/Checks.py (2)
46-47:
⚠️ Potential issueYo! Don't ignore those errors! 🚨
When you catch an exception but don't use it, you're basically throwing away important info about what went wrong. That's not cool!
Let's fix it:
- except (subprocess.TimeoutExpired, subprocess.SubprocessError) as e: + except (subprocess.TimeoutExpired, subprocess.SubprocessError) as e: + log_debug.error(f"Failed to check execution policy: {e}") return FalseCommittable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Ruff (0.8.2)
46-46: Local variable
eis assigned to but never usedRemove assignment to unused variable
e(F841)
36-37: 🛠️ Refactor suggestion
Hey! The PowerShell path should be more flexible! 🔧
Using a hardcoded path isn't cool because PowerShell might be installed in different places on different Windows versions. Let's make it smarter!
Here's a better way to do it:
- if not os.path.exists("C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe"): + powershell = "powershell.exe" + if not any(os.path.exists(os.path.join(path, powershell)) + for path in os.environ["PATH"].split(os.pathsep)):📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.powershell = "powershell.exe" if not any(os.path.exists(os.path.join(path, powershell)) for path in os.environ["PATH"].split(os.pathsep)): return FalseCODE/_debug.py (2)
219-222:
⚠️ Potential issueBe careful when deleting files! 🗑️
What if we can't delete the log file? We should handle that gracefully!
Here's a safer way:
# Clear Debug Log log_path = "../ACCESS/LOGS/DEBUG/DEBUG.log" - if os.path.exists(log_path): - os.remove(log_path) + try: + if os.path.exists(log_path): + os.remove(log_path) + except OSError as e: + log_debug.error(f"Failed to clear debug log: {e}")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.log_path = "../ACCESS/LOGS/DEBUG/DEBUG.log" try: if os.path.exists(log_path): os.remove(log_path) except OSError as e: log_debug.error(f"Failed to clear debug log: {e}")
196-211: 🛠️ Refactor suggestion
Watch out for network problems! 🌐
Getting stuff from the internet can be tricky - what if GitHub is down or the connection is bad?
Let's make it more reliable:
def get_online_config() -> dict | None: + MAX_RETRIES = 3 + BASE_URL = "https://raw.githubusercontent.com/DefinetlyNotAI/Logicytics/main/CODE/config.ini" + try: - url = "https://raw.githubusercontent.com/DefinetlyNotAI/Logicytics/main/CODE/config.ini" - config = configparser.ConfigParser() - config.read_string(requests.get(url, timeout=15).text) + session = requests.Session() + session.mount('https://', requests.adapters.HTTPAdapter(max_retries=MAX_RETRIES)) + response = session.get(BASE_URL, timeout=15, verify=True) + response.raise_for_status() + + config = configparser.ConfigParser() + config.read_string(response.text) return config📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.def get_online_config() -> dict | None: MAX_RETRIES = 3 BASE_URL = "https://raw.githubusercontent.com/DefinetlyNotAI/Logicytics/main/CODE/config.ini" try: session = requests.Session() session.mount('https://', requests.adapters.HTTPAdapter(max_retries=MAX_RETRIES)) response = session.get(BASE_URL, timeout=15, verify=True) response.raise_for_status() config = configparser.ConfigParser() config.read_string(response.text) return config except requests.exceptions.RequestException as e: log_debug.error(f"Connection error: {e}") return None
Signed-off-by: Shahm Najeeb <[email protected]>
|
Code Climate has analyzed commit 391a6ed and detected 0 issues on this pull request. View more on Code Climate. |
Pull Request Template
Prerequisites
--devflag, if required.PR Type
update
Description
This pull request introduces several new GitHub workflows for ClusterFuzzLite and refactors the
CODE/_debug.pyfile to improve logging and error handling. The most important changes include adding new workflows for batch fuzzing, continuous builds, and cron tasks, as well as refactoring methods in_debug.pyto enhance readability and functionality.New GitHub Workflows:
.github/workflows/cflite_batch.yml: Added a workflow for batch fuzzing with ClusterFuzzLite, scheduled to run every 6 hours..github/workflows/cflite_build.yml: Added a workflow for continuous builds with ClusterFuzzLite, triggered on pushes to the main branch..github/workflows/cflite_cron.yml: Added a workflow for daily cron tasks with ClusterFuzzLite, scheduled to run once a day at midnight.Refactoring in
CODE/_debug.py:HealthCheckandDebugCheckclasses, including adding detailed docstrings and restructuring methods for better readability. [1] [2] [3] [4] [5]execution_policymethod fromDebugChecktoCheckclass inCODE/logicytics/Checks.pyfor better organization. [1] [2]Motivation and Context
CI Testing, added fuzzing and fixed bugs
Credit
Issues Fixed
Summary by CodeRabbit
New Features
Bug Fixes
Documentation