Major improvements across debugger, vulnscan, and logging #225
Conversation
- Added explicit variable type annotations across most files - Rewrote `dump_memory` to include TODO logic and improve robustness - Enhanced debugger version check: - Now supports custom Git repo URLs for update checks - Improved forced exit handling in `Logicytics.py` with basic cleanup - vulnscan module upgrades: - Implemented batch file reading - Switched to asynchronous file scanning - Optimized model loading with caching - Improved feature extraction pipeline - Introduced `MAX_FILE_SIZE` global (default 50MB; None to ignore) - Added `max_workers` and `UNREADABLE_EXTENSIONS` to `config.ini` - Replaced `print` statements with structured logging across logicytics lib Signed-off-by: Shahm Najeeb <[email protected]>
DefinetlyNotAI
added
type/Dependencies
WalkthroughThis update brings a bunch of changes: it adds and tweaks config settings, switches some file scanning code from threads to async for faster scans, makes error handling and logging more consistent, and adds type hints to several functions. There are also some improvements to how files and memory dumps are managed, plus a few project config file updates. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant VulnScan
participant FileSystem
participant Model
participant Logger
User->>VulnScan: scan_directory(scan_paths)
VulnScan->>FileSystem: Collect files (sync)
VulnScan->>VulnScan: Start async _async_scan(files)
loop For each file (async, limited by max_workers)
VulnScan->>FileSystem: Read file (async)
VulnScan->>Model: Predict sensitive content
Model-->>VulnScan: Prediction result
VulnScan->>Logger: Log result
end
VulnScan-->>User: Scan complete
Possibly related PRs
Suggested labels
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
![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. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 12
🔭 Outside diff range comments (1)
CODE/bluetooth_logger.py (1)
71-102:⚠️ Potential issueFix the missing return statement in the exception handler.
The
parse_outputfunction has inconsistent return behavior - it returnsresultsin the try block but doesn't explicitly return anything in the except block (which means it returnsNone). This could cause issues when the calling code expects a list.except Exception as err: log.error(f"Parsing output failed: {err}") + return []🧰 Tools
🪛 Pylint (3.3.7)
[warning] 100-100: Catching too general exception Exception
(W0718)
[refactor] 71-71: Either all return statements in a function should return an expression, or none of them should.
(R1710)
🧹 Nitpick comments (9)
CODE/Logicytics.py (1)
533-536: Good cleanup improvement, but let's make the messages clearer! 💪Love that you're actually doing cleanup now instead of just having a TODO! Removing the emojis also fits better with the structured logging mentioned in the PR.
One small suggestion - the warning messages could be a bit more specific about what's happening:
- log.warning("Force shutdown detected! Some temporary files might be left behind.") - log.warning("Pro tip: Next time, let the program finish naturally.") - # Attempt cleanup -> Zip generated files + log.warning("Force shutdown detected! Attempting to zip generated files before exit.") + log.warning("Pro tip: Next time, let the program finish naturally for complete cleanup.") + # Emergency cleanup - zip generated filesThis way users know exactly what cleanup is happening during the forced exit!
CODE/_dev.py (1)
22-26: Clean up this if-else structureHey, you can make this code cleaner! Since you're already returning something in the
ifblock, you don't need theelsepart. Just move the print and return statements to the same level as the if.if is_input: return input(f"{color_code}{text}{colors['reset']}") -else: - print(f"{color_code}{text}{colors['reset']}") - return None +print(f"{color_code}{text}{colors['reset']}") +return None🧰 Tools
🪛 Pylint (3.3.7)
[refactor] 22-26: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
CODE/logicytics/Logger.py (1)
337-337: These lines are getting pretty longYour lines are going over 100 characters, which can make them hard to read. Try breaking them up into shorter chunks.
- This method processes a list of messages, where each message is associated with a specific log type. It is designed for scenarios where multiple log entries need to be processed simultaneously, such as logging script execution results. + This method processes a list of messages, where each message is associated with a specific log type. + It is designed for scenarios where multiple log entries need to be processed simultaneously, + such as logging script execution results.- message_log (list[list[str]]): A list of message entries. Each entry is a list containing two elements: + message_log (list[list[str]]): A list of message entries. + Each entry is a list containing two elements:Also applies to: 340-340
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 337-337: Line too long (243/100)
(C0301)
CODE/logicytics/Execute.py (1)
9-9: Type change looks consistent with your logging updatesYou changed this to match what you did in Logger.py - making the inner lists more flexible instead of requiring exactly 2 strings. Just make sure this actually matches how you're using the returned data in your code.
Also, that line 19 is pretty long - maybe break it up:
- list[list[str]] | None: A list of message-ID pairs for PowerShell scripts, or None for Python scripts. + list[list[str]] | None: A list of message-ID pairs for PowerShell scripts, + or None for Python scripts.Also applies to: 19-19
CODE/config.ini (1)
103-103: Consider documenting the file extension formatThe
unreadable_extensionslist is super long but doesn't show if extensions need dots or not. Looking at the values, they all have dots, but it'd be cleaner to add a comment.Add a clarifying comment:
[VulnScan Settings] # Following extensions to be skipped by the model +# Format: comma-separated list with dots (e.g., .exe,.dll) unreadable_extensions = .exe,.dll,.so,.zip,.tar,.gz,.7z,.rar,.jpg,.jpeg,.png,.gif,.bmp,.tiff,.webp,.mp3,.wav,.flac,.aac,.ogg,.mp4,.mkv,.avi,.mov,.wmv,.flv,.pdf,.doc,.docx,.xls,.xlsx,.ppt,.pptx,.odt,.ods,.odp,.bin,.dat,.iso,.class,.pyc,.o,.obj,.sqlite,.db,.ttf,.otf,.woff,.woff2,.lnk,.urlCODE/dump_memory.py (1)
163-166: Redundant directory creationYou're creating
DUMP_DIRtwice - once at module level (line 20) and again here. The second one is unnecessary.Since you already create it at module level, you can remove this or just check if it exists:
- try: - os.makedirs(DUMP_DIR, exist_ok=True) - except Exception as e: - log.critical(f"Failed to create dump directory '{DUMP_DIR}': {e}") - return + if not os.path.exists(DUMP_DIR): + log.critical(f"Dump directory '{DUMP_DIR}' does not exist and couldn't be created") + return🧰 Tools
🪛 Pylint (3.3.7)
[warning] 164-164: Catching too general exception Exception
(W0718)
CODE/logicytics/Config.py (1)
30-30: Good cleanup, but let's make it even better!Nice job consolidating the error handling into a single line! That's way cleaner than having separate print and exit statements.
However, we can make this even more solid by following Python best practices:
- exit("The config.ini file is not found in the expected location.") + sys.exit("The config.ini file is not found in the expected location.")Also, since you're returning in the
ifblock above, you don't actually need theelse- you can just put the exit at the same indentation level as theif.if os.path.exists(configs_path): return configs_path - else: - sys.exit("The config.ini file is not found in the expected location.") + sys.exit("The config.ini file is not found in the expected location.")Don't forget to add
import sysat the top if it's not already there!🧰 Tools
🪛 Pylint (3.3.7)
[refactor] 27-30: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
[refactor] 30-30: Consider using 'sys.exit' instead
(R1722)
CODE/logicytics/Checks.py (1)
45-45: Same improvement here - nice consistency!Great job keeping the error handling consistent across files! This matches the cleanup in Config.py perfectly.
Just like in the Config.py file, let's use
sys.exitinstead of bareexit()for better clarity:- exit(f"Failed to check execution policy: {e}") + sys.exit(f"Failed to check execution policy: {e}")Make sure to add
import sysat the top if it's not already imported!🧰 Tools
🪛 Pylint (3.3.7)
[refactor] 45-45: Consider using 'sys.exit' instead
(R1722)
CODE/logicytics/Flag.py (1)
691-695: Simplify the unnecessary else statement.There's an unnecessary
elseblock after areturnstatement. Since the function already returns in the if block, we can remove the else and just have the code execute normally.if return_output: return parser.format_help() - else: - parser.print_help() - return None + parser.print_help() + return None🧰 Tools
🪛 Pylint (3.3.7)
[refactor] 691-695: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
🛑 Comments failed to post (12)
requirements.txt (1)
24-25: 💡 Verification agent
🧩 Analysis chain
New dependencies added - let's double-check these versions! 🔍
The
aiofilesdependency makes sense for the async file scanning improvements mentioned in the PR. However,configparser~=7.1.0seems unusually high since configparser is usually part of Python's standard library.Let me check if these versions actually exist and are secure:
🏁 Script executed:
#!/bin/bash # Check if these package versions exist on PyPI echo "Checking configparser version..." curl -s https://pypi.org/pypi/configparser/json | jq '.releases | keys[]' | grep "7.1" echo "Checking aiofiles version..." curl -s https://pypi.org/pypi/aiofiles/json | jq '.info.version' # Check for any known security advisories echo "Checking for security advisories..." gh api graphql -f query=' { securityVulnerabilities(first: 5, ecosystem: PIP, package: "configparser") { nodes { advisory { summary severity } vulnerableVersionRange } } }'Length of output: 648
Drop
configparser—it’s already in Python’s standard library 🔥You’ve added
configparser~=7.1.0even though this package on PyPI is just a backport for older Pythons. For modern Python 3 builds, you don’t need to install it—and having it inrequirements.txtcan actually cause confusion or version conflicts.On the other hand,
aiofiles~=24.1.0is spot-on for async file I/O.Please update
requirements.txtas follows:- configparser~=7.1.0 aiofiles~=24.1.0📝 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.aiofiles~=24.1.0🤖 Prompt for AI Agents
In requirements.txt at lines 24 to 25, remove the line specifying configparser~=7.1.0 because configparser is included in the Python standard library and does not need to be installed separately. Keep the aiofiles~=24.1.0 dependency as is. This will prevent potential version conflicts and confusion.CODE/logicytics/Logger.py (1)
333-333: 💡 Verification agent
🧩 Analysis chain
Check if this type change is really what you want
You changed the type from
list[list[str, str]](which means each inner list has exactly 2 strings) tolist[list[str]](which means each inner list can have any number of strings).This is more flexible, but make sure this matches how you're actually using this function everywhere else in your code. If you always expect exactly 2 strings per inner list, the old type was more accurate.
Let me check how this function is being used across the codebase:
Also applies to: 340-340
🏁 Script executed:
#!/bin/bash # Check how the execution method is called throughout the codebase rg -A 3 -B 3 "\.execution\(" --type pyLength of output: 921
Let’s peek at how
Execute.scriptis defined and howexecution_listis shaped so we know what types we’re really dealing with:
🏁 Script executed:
#!/bin/bash # Find where Execute is defined rg -n "class Execute" --type py # Find the implementation of `script` rg -n "def script" --type py # See how execution_list is initialized or used rg -n "execution_list" CODE/Logicytics.pyLength of output: 1997
Let’s inspect the full
scriptmethod to see what shape of lists it actually returns:
🏁 Script executed:
#!/bin/bash rg -n "def script" -A 200 CODE/logicytics/Execute.pyLength of output: 3303
Let’s check how
Logger.executionactually usesmessage_logso we know if it really expects exactly two items per inner list:
🏁 Script executed:
#!/bin/bash rg -n "def execution" -A50 -B10 CODE/logicytics/Logger.pyLength of output: 2967
Use tuples for fixed two-element message entries
The
executionmethod always processes pairs of strings (message and log type), but annotating it aslist[list[str]]allows lists of any length. Let’s tighten the contract by using two-element tuples:• In CODE/logicytics/Logger.py (around line 333):
- def execution(self, message_log: list[list[str]]): + def execution(self, message_log: list[tuple[str, str]]):• In CODE/logicytics/Execute.py (script signature, line 9):
- def script(cls, script_path: str) -> list[list[str]] | None: + def script(cls, script_path: str) -> list[tuple[str, str]] | None:• In
Execute.__run_other_script(around lines 89–92):- for line in lines: - if ":" in line: - id_part, message_part = line.split(":", 1) - messages.append([message_part.strip(), id_part.strip()]) + for line in lines: + if ":" in line: + id_part, message_part = line.split(":", 1) + messages.append((message_part.strip(), id_part.strip()))These changes ensure each entry is exactly two strings, matching how the code actually runs.
🤖 Prompt for AI Agents
In CODE/logicytics/Logger.py around line 333 and similarly in CODE/logicytics/Execute.py at the script method signature and in Execute.__run_other_script around lines 89-92, change the type annotation for the execution method parameter from list[list[str]] to list[tuple[str, str]] to reflect that each inner element is a fixed pair of strings (message and log type). This tightens the type contract to ensure each entry has exactly two strings, matching actual usage.CODE/logicytics/__init__.py (1)
10-10: 🛠️ Refactor suggestion
Hold up - are you actually using this import?
The static analysis tool is saying you imported
logbut never used it in this file. If you're just importing it to make it available when people import from this module (called re-exporting), you should add it to an__all__list to make that clear.If you want to re-export the log for other modules to use, add this near the top:
+__all__ = ['log', 'Check', 'DEBUG', 'VERSION', 'CURRENT_FILES', 'DELETE_LOGS', + 'config', 'Execute', 'FileManagement', 'Flag', 'Get']Or if you're not actually using it here, remove the import:
-from logicytics.Logger import logCommittable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Ruff (0.11.9)
10-10:
logicytics.Logger.logimported but unused; consider removing, adding to__all__, or using a redundant alias(F401)
🤖 Prompt for AI Agents
In CODE/logicytics/__init__.py at line 10, the imported 'log' from logicytics.Logger is not used in this file. If the intention is to re-export 'log' for other modules, add it to an __all__ list near the top of the file like __all__ = ['log']. Otherwise, if it is not used or re-exported, remove the import statement to clean up unused imports.CODE/wifi_stealer.py (1)
83-83:
⚠️ Potential issueFix the inconsistent return type
While adding
return Noneis good for clarity, there's an issue:get_wifi_names()is supposed to return a list according to its docstring, but now returnsNoneon error. This could break code that expects a list.Fix it by returning an empty list instead:
- return None + return []This way the function always returns a list type, preventing potential
TypeErrorwhen callers try to iterate over the result.📝 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.return []🤖 Prompt for AI Agents
In CODE/wifi_stealer.py at line 83, the function get_wifi_names() currently returns None on error, which conflicts with its docstring stating it returns a list. To fix this, replace the return None statement with return an empty list ([]). This ensures the function consistently returns a list type, avoiding potential TypeErrors when the caller iterates over the result.CODE/vulnscan.py (4)
45-62:
⚠️ Potential issueThread safety concern with model caching
You're using thread locks but then storing PyTorch models in a class-level cache. If multiple VulnScan instances exist, they'll share the same cache dict which could cause issues.
Move the cache to instance level:
class _SensitiveDataScanner: def __init__(self, model_path: str, vectorizer_path: str): self.model_path = model_path self.vectorizer_path = vectorizer_path - self.model_cache = {} - self.vectorizer_cache = {} self.model_lock = threading.Lock() self.vectorizer_lock = threading.Lock()Then use a class-level cache if you really need shared caching:
class _SensitiveDataScanner: _model_cache = {} _vectorizer_cache = {} _cache_lock = threading.Lock()🤖 Prompt for AI Agents
In CODE/vulnscan.py around lines 45 to 62, the model_cache is currently a class-level dictionary shared across all VulnScan instances, which can cause thread safety issues when storing PyTorch models. To fix this, move the model_cache to be an instance attribute instead of a class attribute to avoid shared mutable state. If shared caching is necessary, implement a class-level cache dictionary with a dedicated class-level threading lock to synchronize access safely across instances.
173-174:
⚠️ Potential issueAdd encoding to file write
The static analysis caught this - you're opening a file without specifying encoding.
- with open("Sensitive_File_Paths.txt", "a") as out: + with open("Sensitive_File_Paths.txt", "a", encoding="utf-8") as out:📝 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 open("Sensitive_File_Paths.txt", "a", encoding="utf-8") as out: out.write("\n".join(sensitive_files) + "\n" if sensitive_files else "No sensitive files detected.\n")🧰 Tools
🪛 Pylint (3.3.7)
[convention] 174-174: Line too long (113/100)
(C0301)
[warning] 173-173: Using open without explicitly specifying an encoding
(W1514)
🤖 Prompt for AI Agents
In CODE/vulnscan.py around lines 173 to 174, the file is opened for writing without specifying the encoding, which can cause issues on different systems. Fix this by adding the encoding parameter to the open() call, for example encoding="utf-8", to ensure consistent file writing behavior.
136-137: 🛠️ Refactor suggestion
Potential event loop conflict
Getting the current event loop with
get_event_loop()can fail if no loop exists in the current thread.Use the safer approach:
- loop = asyncio.get_event_loop() - loop.run_until_complete(self._async_scan(all_files)) + asyncio.run(self._async_scan(all_files))This creates a new event loop if needed and properly cleans up after.
📝 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.- loop = asyncio.get_event_loop() - loop.run_until_complete(self._async_scan(all_files)) + asyncio.run(self._async_scan(all_files))🤖 Prompt for AI Agents
In CODE/vulnscan.py around lines 136 to 137, replace the use of asyncio.get_event_loop() with creating a new event loop using asyncio.new_event_loop(), set it as the current event loop with asyncio.set_event_loop(), run the coroutine with run_until_complete(), and then close the loop to avoid potential event loop conflicts when no loop exists in the current thread.
20-29:
⚠️ Potential issueValidate the max_workers configuration
The code doesn't check if the user input for
max_workersis valid when it's not "auto". A negative number or zero would cause issues.Add validation after parsing:
raw_workers = config.get("VulnScan Settings", "max_workers", fallback="auto") max_workers = min(32, os.cpu_count() * 2) if raw_workers == "auto" else int(raw_workers) + +if not raw_workers == "auto" and max_workers < 1: + log.critical(f"Invalid max_workers value: {max_workers}. Must be positive integer or 'auto'") + exit(1)📝 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.UNREADABLE_EXTENSIONS = config.get("VulnScan Settings", "unreadable_extensions").split(",") MAX_FILE_SIZE_MB = config.get("VulnScan Settings", "max_file_size_mb", fallback="None") raw_workers = config.get("VulnScan Settings", "max_workers", fallback="auto") max_workers = min(32, os.cpu_count() * 2) if raw_workers == "auto" else int(raw_workers) if not raw_workers == "auto" and max_workers < 1: log.critical(f"Invalid max_workers value: {max_workers}. Must be positive integer or 'auto'") exit(1) if MAX_FILE_SIZE_MB != "None": MAX_FILE_SIZE_MB = max(int(MAX_FILE_SIZE_MB), 1) else: MAX_FILE_SIZE_MB = None🧰 Tools
🪛 Ruff (0.11.9)
25-28: Use ternary operator
MAX_FILE_SIZE_MB = max(int(MAX_FILE_SIZE_MB), 1) if MAX_FILE_SIZE_MB != "None" else Noneinstead ofif-else-blockReplace
if-else-block withMAX_FILE_SIZE_MB = max(int(MAX_FILE_SIZE_MB), 1) if MAX_FILE_SIZE_MB != "None" else None(SIM108)
🤖 Prompt for AI Agents
In CODE/vulnscan.py around lines 20 to 29, the max_workers value parsed from the config is not validated when it is not "auto", which can lead to invalid values like zero or negative numbers causing issues. After converting raw_workers to an integer, add a check to ensure max_workers is at least 1; if it is less than 1, set it to a default safe value such as 1 or raise an error to prevent invalid configurations.CODE/dump_memory.py (3)
15-17: 🛠️ Refactor suggestion
Use sys.exit for cleaner exits
The static analyzer is right -
sys.exit()is cleaner than plainexit().+import sys if SAFETY_MARGIN < 1: log.critical("Invalid Safety Margin Inputted - Cannot proceed with dump memory") - exit(1) + sys.exit(1)📝 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.import sys if SAFETY_MARGIN < 1: log.critical("Invalid Safety Margin Inputted - Cannot proceed with dump memory") sys.exit(1)🧰 Tools
🪛 Pylint (3.3.7)
[refactor] 17-17: Consider using 'sys.exit' instead
(R1722)
🤖 Prompt for AI Agents
In CODE/dump_memory.py around lines 15 to 17, replace the use of plain exit(1) with sys.exit(1) for a cleaner and more explicit program termination. First, ensure that the sys module is imported at the top of the file if it is not already. Then change exit(1) to sys.exit(1) in the error handling block.
57-59:
⚠️ Potential issueSpecify encoding for text files
You're writing text but didn't specify encoding, which could cause issues on different systems.
- with open(os.path.join(DUMP_DIR, "Ram_Snapshot.txt"), "w") as file: + with open(os.path.join(DUMP_DIR, "Ram_Snapshot.txt"), "w", encoding="utf-8") as file:📝 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 open(os.path.join(DUMP_DIR, "Ram_Snapshot.txt"), "w", encoding="utf-8") as file: memory_helper(memory, "RAM") memory_helper(swap, "Swap Memory", use_free_rather_than_available=True)🧰 Tools
🪛 Pylint (3.3.7)
[warning] 57-57: Using open without explicitly specifying an encoding
(W1514)
🤖 Prompt for AI Agents
In CODE/dump_memory.py around lines 57 to 59, the open() call for writing "Ram_Snapshot.txt" does not specify the file encoding, which can lead to compatibility issues across different systems. Modify the open() function to include the encoding parameter, such as encoding="utf-8", to explicitly set the text encoding when opening the file for writing.
7-7:
⚠️ Potential issueAvoid importing private modules
You're importing from
psutil._commonwhich is a private module (the underscore means it's internal). This could break in future psutil versions.Since you're only using this for type hints, you can handle it differently:
-from psutil._common import sswap +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from psutil._common import sswapThen in your type hint:
-def memory_helper(mem_var: psutil.svmem | sswap, flavor_text: str, use_free_rather_than_available: bool = False): +def memory_helper(mem_var: psutil.svmem | "sswap", flavor_text: str, use_free_rather_than_available: bool = False):📝 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.# replace the private import with a TYPE_CHECKING guard -from psutil._common import sswap +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from psutil._common import sswap # ... other imports ... import psutil # update the signature to use a forward‐reference for sswap -def memory_helper(mem_var: psutil.svmem | sswap, flavor_text: str, use_free_rather_than_available: bool = False): +def memory_helper(mem_var: psutil.svmem | "sswap", flavor_text: str, use_free_rather_than_available: bool = False): ...🧰 Tools
🪛 Pylint (3.3.7)
[error] 7-7: Unable to import 'psutil._common'
(E0401)
🤖 Prompt for AI Agents
In CODE/dump_memory.py at line 7, avoid importing the private module psutil._common as it is internal and may change. Instead, remove the direct import and use a string literal for the type hint referencing sswap, or import the public psutil types if available. This prevents future breakage while maintaining type hinting.CODE/logicytics/Flag.py (1)
180-183: 🛠️ Refactor suggestion
Inconsistent logging - some print statements still hanging around!
You've done a great job converting most print statements to log calls, but there are still some regular
print()calls mixed in. For consistency, these should also use the logging system.- print(f" Average Accuracy: {average_accuracy:.2f}%") - print(f" Most Common Device Name: {most_common_device}") - print(f" Most Common User Input: {average_user_input}") + log.info(f" Average Accuracy: {average_accuracy:.2f}%") + log.info(f" Most Common Device Name: {most_common_device}") + log.info(f" Most Common User Input: {average_user_input}")- print(f" {flag}: {count} times") + log.info(f" {flag}: {count} times")Also applies to: 188-188
🤖 Prompt for AI Agents
In CODE/logicytics/Flag.py around lines 180 to 183 and line 188, replace the remaining print() statements with appropriate logging calls using the existing logging system to maintain consistency. Change each print statement to a logger.info or another suitable log level call, formatting the message string similarly to the current print statements.
Also bug fix to ignore pyc files (pycache) from the --dev menu Signed-off-by: Shahm Najeeb <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
CODE/_dev.py (1)
25-26: Hey, this explicit return looks good but there's a cleaner way to write it!Adding the explicit
return Nonemakes the code clearer about what it's doing - that's awesome! But there's a neat trick here: since you're already returning in theifblock above, you don't need theelseblock at all.Here's how you can make it even cleaner:
def color_print(text, color="reset", is_input=False) -> None | str: colors = { "reset": "\033[0m", "red": "\033[31m", "green": "\033[32m", "yellow": "\033[33m", "cyan": "\033[36m", } color_code = colors.get(color.lower(), colors["reset"]) if is_input: return input(f"{color_code}{text}{colors['reset']}") - else: - print(f"{color_code}{text}{colors['reset']}") - return None + print(f"{color_code}{text}{colors['reset']}") + return None🧰 Tools
🪛 Pylint (3.3.7)
[refactor] 22-26: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
CODE/_dev.py(3 hunks)CODE/config.ini(3 hunks)CODE/logicytics/__init__.py(1 hunks)SECURITY.md(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- SECURITY.md
🚧 Files skipped from review as they are similar to previous changes (1)
- CODE/config.ini
🧰 Additional context used
🧬 Code Graph Analysis (1)
CODE/logicytics/__init__.py (1)
CODE/logicytics/Logger.py (1)
Log(16-419)
🪛 Pylint (3.3.7)
CODE/_dev.py
[refactor] 22-26: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
🪛 Ruff (0.11.9)
CODE/logicytics/__init__.py
10-10: logicytics.Logger.log imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
10-10: logicytics.Logger.Log imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
🔇 Additional comments (2)
CODE/logicytics/__init__.py (1)
10-10: Hey! Your imports are just hanging out doing nothing! 🤔So you've got both
logandLogimported here, but they're not actually being used in this file. The static analysis tool is basically saying "yo, these imports are just sitting there!"Here's what's probably going on - you want these to be available when people do
from logicytics import log, Log, right? That's totally cool and makes sense for a package API! But you need to tell Python that's what you're doing.Try one of these fixes:
Option 1: Add them to
__all__(recommended)+__all__ = ['log', 'Log', 'Execute', 'Get', 'Check', 'Flag', 'FileManagement'] + from logicytics.Logger import log, LogOption 2: Use them in this file somehow (if that makes sense)
Option 3: Remove them if they're really not needed
Can you double-check if these imports are meant to be part of the public API that other files will import from this package?
🧰 Tools
🪛 Ruff (0.11.9)
10-10:
logicytics.Logger.logimported but unused; consider removing, adding to__all__, or using a redundant alias(F401)
10-10:
logicytics.Logger.Logimported but unused; consider removing, adding to__all__, or using a redundant alias(F401)
CODE/_dev.py (1)
121-122: Nice addition with the .pyc exclusion!Adding
.pycfiles to the exclusion list is smart - these are Python bytecode files that get generated automatically and definitely shouldn't be tracked. This fits perfectly with the other exclusions you already have.The change makes total sense and keeps the file list clean by filtering out compiled Python files that would just clutter things up.
Removed 2 commands from bluetooth_logger.py to fix the bug where is just fails due to the event log not actually existing Fully remade packet_sniffer.py to remove the already buggy code and deprecation warnings Signed-off-by: Shahm Najeeb <[email protected]>
pull-request-size
bot
added
size/XXL
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (2)
CODE/logicytics/Flag.py (2)
168-187: 🛠️ Refactor suggestionGood logging conversion but check the mixed approach.
The conversion to
log.info()is great, but I notice lines 180-183 and 188 still useprint()statements. This creates inconsistent output handling.Consider converting all remaining print statements to maintain consistency:
- print(f" Average Accuracy: {average_accuracy:.2f}%") - print(f" Most Common Device Name: {most_common_device}") - print(f" Most Common User Input: {average_user_input}") + log.info(f" Average Accuracy: {average_accuracy:.2f}%") + log.info(f" Most Common Device Name: {most_common_device}") + log.info(f" Most Common User Input: {average_user_input}")- print(f" {flag}: {count} times") + log.info(f" {flag}: {count} times")
651-666: 🛠️ Refactor suggestionFix line length issues but good logging approach.
The logging conversion is great, but several lines exceed the 100-character limit.
Break up the long lines for better readability:
- log.warning(f"Invalid flag '{user_input}', Did you mean '--{closest_matches[0].replace('_', '-')}'?") + log.warning(f"Invalid flag '{user_input}', Did you mean " + f"'--{closest_matches[0].replace('_', '-')}'?")- log.info( - f"User input: {user_input_desc}\nMatched flag: {flag_received.replace('_', '-')}\nAccuracy: {accuracy_received:.2f}%\n") + log.info(f"User input: {user_input_desc}\n" + f"Matched flag: {flag_received.replace('_', '-')}\n" + f"Accuracy: {accuracy_received:.2f}%\n")- log.info(f"Matched flag: {flag_received.replace('_', '-')} (Accuracy: {accuracy_received:.2f}%)\n") + log.info(f"Matched flag: {flag_received.replace('_', '-')} " + f"(Accuracy: {accuracy_received:.2f}%)\n")🧰 Tools
🪛 Pylint (3.3.7)
[convention] 651-651: Line too long (113/100)
(C0301)
[convention] 660-660: Line too long (102/100)
(C0301)
[convention] 663-663: Line too long (136/100)
(C0301)
[convention] 665-665: Line too long (111/100)
(C0301)
[refactor] 652-652: Consider using 'sys.exit' instead
(R1722)
🧹 Nitpick comments (9)
CODE/logicytics/Config.py (1)
29-29: Hey, this cleanup looks good but let's make it even better!You consolidated the error message and exit into one line, which is way cleaner than before. But there's a small improvement we can make - using
sys.exit()instead of the built-inexit()is more appropriate for scripts like this.+import sys ... - exit("The config.ini file is not found in the expected location.") + sys.exit("The config.ini file is not found in the expected location.")🧰 Tools
🪛 Pylint (3.3.7)
[refactor] 29-29: Consider using 'sys.exit' instead
(R1722)
CODE/logicytics/Logger.py (1)
342-342: Line's getting a bit long here!This docstring line is over the 100-character limit. Let's break it up to keep things readable.
- This method processes a list of messages, where each message is associated with a specific log type. It is designed for scenarios where multiple log entries need to be processed simultaneously, such as logging script execution results. + This method processes a list of messages, where each message is associated with a specific log type. + It is designed for scenarios where multiple log entries need to be processed simultaneously, + such as logging script execution results.🧰 Tools
🪛 Pylint (3.3.7)
[convention] 342-342: Line too long (243/100)
(C0301)
CODE/bluetooth_details.py (1)
110-110: Consider being more specific with exception handling.While catching
Exceptionworks, it might be better to catch specific exceptions likeIOErrororOSErrorfor file operations. This gives you more control over error handling.- except Exception as e: + except (IOError, OSError) as e:🧰 Tools
🪛 Pylint (3.3.7)
[warning] 110-110: Catching too general exception Exception
(W0718)
CODE/logicytics/__init__.py (1)
10-10: Import looks good but fix the static analysis warning!The import statement brings in both
logandLogwhich is smart, but static analysis is complaining that they're unused. Since you're adding them to__all__, you should include them there to fix this warning.-__all__ = ['execute', 'get', 'check', 'flag', 'file_management', 'deprecated', 'ObjectLoadError'] +__all__ = ['execute', 'get', 'check', 'flag', 'file_management', 'log', 'Log', 'deprecated', 'ObjectLoadError']🧰 Tools
🪛 Ruff (0.11.9)
10-10:
logicytics.Logger.logimported but unused; consider removing, adding to__all__, or using a redundant alias(F401)
10-10:
logicytics.Logger.Logimported but unused; consider removing, adding to__all__, or using a redundant alias(F401)
CODE/dump_memory.py (1)
82-86: Fix the encoding warning!The static analysis tool is right - you should specify an encoding when opening files. This prevents weird issues on different systems.
- with open(os.path.join(DUMP_DIR, "SystemRam_Info.txt"), "w") as file: + with open(os.path.join(DUMP_DIR, "SystemRam_Info.txt"), "w", encoding="utf-8") as file:🧰 Tools
🪛 Pylint (3.3.7)
[warning] 85-85: Catching too general exception Exception
(W0718)
[warning] 82-82: Using open without explicitly specifying an encoding
(W1514)
CODE/packet_sniffer.py (4)
21-26: Great class rename and cleaner initialization!
PacketSnifferis way more descriptive thanSniff! The simplified__init__with clear variable names (packet_data,G) makes the code more readable.However, consider adding a class docstring and fixing the attribute naming convention:
+class PacketSniffer: + """ + A network packet sniffer that captures, analyzes, and visualizes network traffic. + + Uses Scapy for packet capture and NetworkX for graph visualization of network connections. + """ def __init__(self): conf.verb = 0 self.packet_data = [] - self.G = nx.Graph() + self.graph = nx.Graph() # Use snake_case naming🧰 Tools
🪛 Pylint (3.3.7)
[convention] 21-21: Missing class docstring
(C0115)
[convention] 25-25: Attribute name "G" doesn't conform to snake_case naming style
(C0103)
28-35: Cleaner protocol detection but fix the elif chain.The simplified protocol detection is much cleaner! However, you can make it even more efficient.
Remove unnecessary
elifstatements after return (as suggested by static analysis):@staticmethod def _get_protocol(packet: IP) -> str: if packet.haslayer(TCP): return "TCP" - elif packet.haslayer(UDP): + if packet.haslayer(UDP): return "UDP" - elif packet.haslayer(ICMP): + if packet.haslayer(ICMP): return "ICMP" return "Other"🧰 Tools
🪛 Pylint (3.3.7)
[refactor] 29-34: Unnecessary "elif" after "return", remove the leading "el" from "elif"
(R1705)
38-43: Clean port extraction but fix line length and elif.The port extraction logic is much cleaner than before! But there are some style issues to fix.
Fix the line length violations and unnecessary elif:
@staticmethod def _get_port(packet: IP, port_type: str) -> int | None: if port_type == "sport": - return getattr(packet[TCP], "sport", None) if packet.haslayer(TCP) else getattr(packet[UDP], "sport", None) - elif port_type == "dport": - return getattr(packet[TCP], "dport", None) if packet.haslayer(TCP) else getattr(packet[UDP], "dport", None) + if packet.haslayer(TCP): + return getattr(packet[TCP], "sport", None) + return getattr(packet[UDP], "sport", None) if packet.haslayer(UDP) else None + if port_type == "dport": + if packet.haslayer(TCP): + return getattr(packet[TCP], "dport", None) + return getattr(packet[UDP], "dport", None) if packet.haslayer(UDP) else None return None🧰 Tools
🪛 Pylint (3.3.7)
[convention] 40-40: Line too long (119/100)
(C0301)
[convention] 42-42: Line too long (119/100)
(C0301)
[refactor] 39-42: Unnecessary "elif" after "return", remove the leading "el" from "elif"
(R1705)
80-94: Great graph visualization but fix line length.The graph visualization with edge validation is awesome! The matplotlib configuration looks good too.
Just fix this line length issue:
- nx.draw(self.G, pos, with_labels=True, node_color="skyblue", node_size=3000, font_size=10, font_weight="bold") + nx.draw(self.G, pos, with_labels=True, node_color="skyblue", + node_size=3000, font_size=10, font_weight="bold")🧰 Tools
🪛 Pylint (3.3.7)
[convention] 87-87: Line too long (118/100)
(C0301)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
.idea/csv-editor.xml(1 hunks)CODE/Logicytics.py(12 hunks)CODE/_debug.py(3 hunks)CODE/_dev.py(5 hunks)CODE/bluetooth_details.py(3 hunks)CODE/bluetooth_logger.py(4 hunks)CODE/cmd_commands.py(2 hunks)CODE/config.ini(3 hunks)CODE/dir_list.py(2 hunks)CODE/dump_memory.py(4 hunks)CODE/logicytics/Config.py(1 hunks)CODE/logicytics/Execute.py(1 hunks)CODE/logicytics/Flag.py(12 hunks)CODE/logicytics/Logger.py(6 hunks)CODE/logicytics/__init__.py(2 hunks)CODE/network_psutil.py(2 hunks)CODE/packet_sniffer.py(1 hunks)CODE/wifi_stealer.py(3 hunks)CODE/wmic.py(3 hunks)requirements.txt(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- CODE/network_psutil.py
🚧 Files skipped from review as they are similar to previous changes (5)
- requirements.txt
- .idea/csv-editor.xml
- CODE/config.ini
- CODE/_dev.py
- CODE/logicytics/Execute.py
🧰 Additional context used
🧬 Code Graph Analysis (8)
CODE/dir_list.py (1)
CODE/logicytics/Execute.py (1)
command(37-48)
CODE/bluetooth_logger.py (2)
CODE/cmd_commands.py (1)
command(5-25)CODE/logicytics/Execute.py (1)
command(37-48)
CODE/_debug.py (2)
CODE/logicytics/Logger.py (2)
Log(16-425)info(243-251)CODE/logicytics/Checks.py (3)
admin(13-23)uac(48-61)execution_policy(26-45)
CODE/bluetooth_details.py (1)
CODE/logicytics/Logger.py (2)
info(243-251)error(263-271)
CODE/cmd_commands.py (1)
CODE/logicytics/Execute.py (1)
command(37-48)
CODE/logicytics/__init__.py (5)
CODE/logicytics/Logger.py (1)
Log(16-425)CODE/logicytics/Execute.py (1)
Execute(7-96)CODE/logicytics/Get.py (1)
Get(6-55)CODE/logicytics/Flag.py (2)
flag(311-367)Flag(370-770)CODE/logicytics/FileManagement.py (2)
FileManagement(12-220)mkdir(35-54)
CODE/wifi_stealer.py (2)
CODE/logicytics/Execute.py (1)
command(37-48)CODE/logicytics/Logger.py (2)
error(263-271)info(243-251)
CODE/wmic.py (1)
CODE/logicytics/Execute.py (1)
command(37-48)
🪛 Pylint (3.3.7)
CODE/bluetooth_logger.py
[refactor] 71-71: Either all return statements in a function should return an expression, or none of them should.
(R1710)
CODE/dump_memory.py
[warning] 57-57: Catching too general exception Exception
(W0718)
[convention] 108-108: Line too long (105/100)
(C0301)
[convention] 137-137: Line too long (117/100)
(C0301)
[convention] 138-138: Line too long (108/100)
(C0301)
[warning] 78-78: Catching too general exception Exception
(W0718)
[warning] 85-85: Catching too general exception Exception
(W0718)
[warning] 82-82: Using open without explicitly specifying an encoding
(W1514)
[warning] 147-147: Catching too general exception Exception
(W0718)
[warning] 119-119: Catching too general exception Exception
(W0718)
[warning] 142-142: Catching too general exception Exception
(W0718)
[warning] 161-161: Catching too general exception Exception
(W0718)
CODE/packet_sniffer.py
[convention] 40-40: Line too long (119/100)
(C0301)
[convention] 42-42: Line too long (119/100)
(C0301)
[convention] 87-87: Line too long (118/100)
(C0301)
[error] 6-6: Unable to import 'matplotlib.pyplot'
(E0401)
[error] 7-7: Unable to import 'networkx'
(E0401)
[error] 8-8: Unable to import 'pandas'
(E0401)
[error] 9-9: Unable to import 'cryptography.utils'
(E0401)
[error] 15-15: Unable to import 'scapy.all'
(E0401)
[convention] 15-15: Import "from scapy.all import sniff, conf" should be placed at the top of the module
(C0413)
[error] 16-16: Unable to import 'scapy.layers.inet'
(E0401)
[convention] 16-16: Import "from scapy.layers.inet import IP, TCP, UDP, ICMP" should be placed at the top of the module
(C0413)
[convention] 18-18: Import "from logicytics import log, config" should be placed at the top of the module
(C0413)
[convention] 21-21: Missing class docstring
(C0115)
[convention] 25-25: Attribute name "G" doesn't conform to snake_case naming style
(C0103)
[refactor] 29-34: Unnecessary "elif" after "return", remove the leading "el" from "elif"
(R1705)
[refactor] 39-42: Unnecessary "elif" after "return", remove the leading "el" from "elif"
(R1705)
[warning] 70-70: Catching too general exception Exception
(W0718)
[convention] 100-100: Missing function or method docstring
(C0116)
[warning] 115-115: Catching too general exception Exception
(W0718)
[convention] 124-124: Missing function or method docstring
(C0116)
[convention] 136-136: Missing function or method docstring
(C0116)
[warning] 146-146: Catching too general exception Exception
(W0718)
CODE/Logicytics.py
[convention] 104-104: Line too long (106/100)
(C0301)
CODE/_debug.py
[convention] 14-14: Line too long (118/100)
(C0301)
[convention] 215-215: Line too long (120/100)
(C0301)
[convention] 217-217: Line too long (109/100)
(C0301)
CODE/bluetooth_details.py
[warning] 110-110: Catching too general exception Exception
(W0718)
CODE/cmd_commands.py
[convention] 1-1: Missing module docstring
(C0114)
CODE/logicytics/Config.py
[refactor] 29-29: Consider using 'sys.exit' instead
(R1722)
CODE/logicytics/Flag.py
[convention] 211-211: Line too long (117/100)
(C0301)
[convention] 583-583: Line too long (102/100)
(C0301)
[convention] 587-587: Line too long (104/100)
(C0301)
[convention] 591-591: Line too long (105/100)
(C0301)
[refactor] 584-584: Consider using 'sys.exit' instead
(R1722)
[refactor] 588-588: Consider using 'sys.exit' instead
(R1722)
[convention] 651-651: Line too long (113/100)
(C0301)
[convention] 663-663: Line too long (136/100)
(C0301)
[convention] 665-665: Line too long (111/100)
(C0301)
[refactor] 725-725: Consider using 'sys.exit' instead
(R1722)
CODE/logicytics/Logger.py
[convention] 342-342: Line too long (243/100)
(C0301)
CODE/logicytics/__init__.py
[refactor] 14-14: Consider using 'sys.exit' instead
(R1722)
CODE/wifi_stealer.py
[warning] 34-34: Catching too general exception Exception
(W0718)
[warning] 81-81: Catching too general exception Exception
(W0718)
CODE/wmic.py
[convention] 1-1: Missing module docstring
(C0114)
🪛 Ruff (0.11.9)
CODE/logicytics/__init__.py
10-10: logicytics.Logger.log imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
10-10: logicytics.Logger.Log imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
🔇 Additional comments (55)
CODE/logicytics/Logger.py (3)
13-13: Nice addition! This import makes total sense.Adding the DEBUG import from Config is perfect for configuring the log level dynamically. This fits well with the overall refactoring to centralize configuration.
338-338: Smart type annotation fix!Changing from
list[list[str, str]]tolist[tuple[str, str]]is way more accurate. Tuples are immutable, which better represents that these message-type pairs shouldn't be modified once created. This also matches the actual usage pattern better.
428-428: Love this centralized logging setup!Creating a module-level log instance with the DEBUG configuration is super smart. This gives the whole codebase a consistent way to log messages and makes it easy to control log levels from one place.
CODE/bluetooth_details.py (4)
5-5: Good type import!Adding
TextIOfrom typing is perfect for properly typing the file parameter. This makes the code way more clear about what kind of file object is expected.
88-88: These type annotations are on point!Adding explicit types for
devices: list[dict[str, str]]andfilename: strmakes the function signature super clear. Anyone reading this code will immediately know what data structure to pass in.
109-109: Nice logging addition!Adding this success log is really helpful for tracking when file operations complete successfully. It's great for debugging and monitoring.
114-114: Perfect type annotation!The
file: TextIOanddevice_info: dict[str, str]annotations are spot on. This makes it crystal clear what this function expects and helps catch type errors early.CODE/bluetooth_logger.py (4)
4-4: Smart import choice!Using
LiteralStringfrom typing is perfect for subprocess output. This type hints that the strings come from a trusted source (the subprocess), which is exactly what you want here.
10-10: Clean type annotations!Adding explicit types for all the string parameters makes this function super clear to use. No more guessing what data types to pass in!
39-39: Good return type annotation!The
None | list[LiteralString]return type clearly shows this function can either succeed with a list or fail with None. That's really helpful for anyone calling this function.
105-105: Excellent return type annotation!The
list[str]return type is perfect and matches exactly what the function returns. This makes the function's contract super clear.CODE/wmic.py (1)
1-1: Nice refactoring! The changes look solid.The switch from
Executetoexecuteis clean and consistent. All the function calls got updated properly too, so everything should work the same way as before.Just a heads up - you might want to add a module docstring at the top to explain what this file does. It's not a big deal, but it helps other developers understand the code better.
Also applies to: 19-19, 31-31
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 1-1: Missing module docstring
(C0114)
CODE/cmd_commands.py (1)
1-1: Solid refactoring work!The import change and function call update are perfectly done. Everything's consistent with the new naming pattern you're using across the project.
Same small suggestion as before - adding a module docstring would be a nice touch for documentation.
Also applies to: 20-20
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 1-1: Missing module docstring
(C0114)
CODE/dir_list.py (1)
4-4: Perfect refactoring!The changes are clean and follow the same pattern as the other files. Nice job keeping everything consistent across the codebase!
Also applies to: 29-29
CODE/wifi_stealer.py (2)
3-3: Great refactoring work!The import and function call changes are spot-on and consistent with the rest of the codebase.
Also applies to: 25-26, 78-78
33-33: Nice error handling improvements!Adding explicit
return Noneandreturn []statements makes the code way clearer about what happens when things go wrong. This is much better than leaving it unclear what the functions return on failure.The static analysis tool is complaining about catching general
Exception, but honestly, for a utility like this that needs to be robust, it's probably fine to catch everything and log it rather than crashing.Also applies to: 36-36, 83-83
CODE/_debug.py (5)
12-12: Nice import cleanup!The switch from
Checktocheckmakes the code way more consistent with the rest of the codebase. Addingconfighere is smart too since you're using it right below.
16-16: Smart move using config for the URL!Instead of hardcoding URLs everywhere, pulling it from config makes this way more flexible. Now if someone needs to change the repository URL, they just update the config file instead of hunting through code.
153-155: Good variable renaming to avoid conflicts!Renaming the local
configvariable to_configprevents it from clashing with the importedconfig. This is exactly what you should do when you have naming conflicts.
197-199: Consistent variable naming throughout!You kept the same
_confignaming pattern here too, which makes the code easier to follow. The logic flow is clear and consistent.
209-217: Import standardization looks solid!All the
Check→checkchanges match the new lowercase naming convention. This makes the codebase way more consistent and easier to understand.🧰 Tools
🪛 Pylint (3.3.7)
[convention] 215-215: Line too long (120/100)
(C0301)
[convention] 217-217: Line too long (109/100)
(C0301)
CODE/Logicytics.py (8)
15-15: Awesome import standardization!Switching to lowercase instances (
execute,check,get, etc.) makes the code way more readable and follows Python naming conventions better. This is a solid improvement across the board.
69-71: Clean file listing logic!Using
get.list_of_fileswith the updated parameters makes this much cleaner. The exclusion logic is clear and the new lowercase naming is consistent with the rest of the changes.
104-106: Consistent method usage!Same clean pattern here with
get.list_of_files. The code is way more readable now that all the method calls follow the same lowercase convention.🧰 Tools
🪛 Pylint (3.3.7)
[convention] 104-104: Line too long (106/100)
(C0301)
[convention] 105-105: Line too long (112/100)
(C0301)
147-147: Method call updates look good!All the
execute.scriptcalls are updated consistently. This keeps everything in line with the new naming scheme.Also applies to: 210-210
355-355: Flag data access updated correctly!The
flag.data()call fits perfectly with the new lowercase instance pattern. Nice and consistent!
385-385: All check method calls updated!Every
check.admin(),check.sys_internal_zip(),check.uac()call is properly updated to use the lowercase instance. This shows great attention to detail in the refactoring.Also applies to: 410-410, 441-441, 450-450
465-465: File management call updated!The
file_management.Zip.and_hashcall follows the new pattern perfectly. Everything is consistent now.
533-537: Smart emergency cleanup on force shutdown!Adding
ZIP.files()to the KeyboardInterrupt handler is brilliant! Now if someone force-quits the program, it still tries to zip up the generated files before exiting. That's way better than just leaving everything scattered around. The warning messages are clear too.🧰 Tools
🪛 Pylint (3.3.7)
[refactor] 537-537: Consider using 'sys.exit' instead
(R1722)
CODE/logicytics/__init__.py (3)
15-20: Perfect instance naming standardization!Switching from uppercase class names to lowercase instances (
Execute()→execute, etc.) makes the code way more Pythonic and consistent. This is exactly how singletons should be named!
21-21: Consistent method call update!Using
file_management.mkdir()instead of the old uppercase version keeps everything in line with the new naming scheme. Nice attention to detail!
107-107: Good idea adding all for explicit exports!Having an explicit
__all__list makes it super clear what this module is supposed to export. Just remember to addlogandLogto fix those static analysis warnings.CODE/dump_memory.py (9)
10-17: Smart configuration setup with validation!Loading all the settings from config instead of hardcoding them is a huge improvement! The safety margin validation is really smart too - if someone puts in a bad value, the program won't just crash mysteriously later on.
🧰 Tools
🪛 Pylint (3.3.7)
[refactor] 17-17: Consider using 'sys.exit' instead
(R1722)
41-48: Clean helper function!The
memory_helperfunction is a great way to avoid repeating the same code over and over. It makes the memory writing logic way cleaner and easier to understand.
51-58: Better error handling structure!Wrapping everything in a single try-except block instead of having multiple scattered ones makes the error handling way cleaner. Much easier to follow what's happening.
🧰 Tools
🪛 Pylint (3.3.7)
[warning] 57-57: Catching too general exception Exception
(W0718)
75-75: More descriptive fallback text!Changing from 'N/A' to 'Unavailable' is more user-friendly. People will understand what that means better.
104-109: Brilliant disk space checking!Doing the disk space check once at the beginning instead of checking every single memory region is way more efficient. And using the safety margin from config makes it super flexible.
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 108-108: Line too long (105/100)
(C0301)
112-113: Smart permission filtering!Skipping memory regions without read permission right away prevents a bunch of errors later. This is much cleaner than trying to read them and handling the errors.
115-121: Robust address parsing!Adding proper error handling for address parsing is really smart. Memory addresses can be weird formats sometimes, so this prevents the whole program from crashing on one bad address.
🧰 Tools
🪛 Pylint (3.3.7)
[warning] 119-119: Catching too general exception Exception
(W0718)
123-141: Much cleaner metadata handling!The new metadata structure is way more organized and readable. Including the region size calculation is really useful too - that's information people actually want to see.
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 137-137: Line too long (117/100)
(C0301)
[convention] 138-138: Line too long (108/100)
(C0301)
159-163: Better directory creation with error handling!Adding proper error handling for directory creation is essential. If the dump directory can't be created, there's no point trying to continue. The early return prevents weird errors later.
🧰 Tools
🪛 Pylint (3.3.7)
[warning] 161-161: Catching too general exception Exception
(W0718)
CODE/logicytics/Flag.py (6)
12-12: Great addition of structured logging!Adding the log import from logicytics.Logger is exactly what this codebase needed. This will make debugging and monitoring so much easier!
67-70: Perfect error logging implementation!Using
log.critical()andlog.error()for model loading failures is spot-on. Critical errors should definitely use the highest log level, and the additional context about checking the config file is super helpful for users.🧰 Tools
🪛 Pylint (3.3.7)
[refactor] 70-70: Consider using 'sys.exit' instead
(R1722)
204-212: Solid logging for file operations!Using
log.info()for successful file saves is perfect - users need to know where their files are being saved. The multiple fallback paths with appropriate logging make this robust.🧰 Tools
🪛 Pylint (3.3.7)
[convention] 211-211: Line too long (117/100)
(C0301)
364-366: Smart use of warning level for suggestions.Using
log.warning()for flag suggestions when no match is found is the right choice - it's important info that users should notice, but not an error.
693-694: Good explicit return handling.Adding the explicit
return Nonemakes the method behavior crystal clear. The help menu printing logic is well-structured.
724-731: Excellent error handling with logging.The validation logic with appropriate error logging is well done. The early return when preferences are disabled is also a good optimization.
Also applies to: 739-739
🧰 Tools
🪛 Pylint (3.3.7)
[refactor] 725-725: Consider using 'sys.exit' instead
(R1722)
[refactor] 731-731: Consider using 'sys.exit' instead
(R1722)
CODE/packet_sniffer.py (8)
3-14: Smart handling of deprecation warnings!Adding warning suppression for cryptography deprecation warnings is a good call - these warnings can clutter the output and aren't actionable for most users. The global suppression approach keeps the code clean.
🧰 Tools
🪛 Pylint (3.3.7)
[error] 6-6: Unable to import 'matplotlib.pyplot'
(E0401)
[error] 7-7: Unable to import 'networkx'
(E0401)
[error] 8-8: Unable to import 'pandas'
(E0401)
[error] 9-9: Unable to import 'cryptography.utils'
(E0401)
45-72: Excellent packet logging consolidation!Combining all the packet processing into one method is brilliant! The error handling and structured logging make this super robust. The data extraction and graph building logic is clean and easy to follow.
However, consider making the exception handling more specific if possible, and add a docstring:
+ def _log_packet(self, packet: IP): + """Log packet information and update network graph.""" if not packet.haslayer(IP): return try: # ... existing code ... - except Exception as err: + except (AttributeError, KeyError) as err: log.error(f"Error logging packet: {err}")🧰 Tools
🪛 Pylint (3.3.7)
[warning] 70-70: Catching too general exception Exception
(W0718)
73-79: Perfect CSV saving with proper validation!The data validation before saving and the clear logging messages make this method super user-friendly. Users will know exactly what's happening!
96-99: Clever interface name correction!This interface correction method is a neat solution for common naming inconsistencies between different systems. Simple but effective!
100-123: Robust sniffing with retry logic - impressive!The retry mechanism with timeout handling is really well thought out! This will make the sniffer much more reliable in real-world conditions.
Consider adding a docstring and making exception handling more specific:
+ def sniff_packets(self, iface: str, count: int, timeout: int, retry_max: int): + """ + Sniff packets with retry logic and interface correction. + + Args: + iface: Network interface name + count: Number of packets to capture + timeout: Timeout for each sniff attempt + retry_max: Maximum retry time in seconds + """ # ... existing code ... try: # ... sniff code ... - except Exception as e: + except (OSError, RuntimeError) as e: log.warning(f"Sniff failed on {iface}: {e}")🧰 Tools
🪛 Pylint (3.3.7)
[convention] 100-100: Missing function or method docstring
(C0116)
[warning] 115-115: Catching too general exception Exception
(W0718)
124-135: Excellent configuration-driven approach!Using fallback values and validation makes this super robust! The configuration validation with early return is exactly the right pattern.
Just add a docstring to complete it:
+ def run(self): + """Run packet sniffing with configuration from config file."""🧰 Tools
🪛 Pylint (3.3.7)
[convention] 124-124: Missing function or method docstring
(C0116)
137-139: Clean and simple cleanup!The simplified cleanup method does exactly what it needs to do with clear logging. Perfect!
Add a docstring for completeness:
+ def cleanup(self): + """Clean up resources and close matplotlib figures."""
143-149: Perfect main execution block!The try-catch with finally block ensures cleanup always happens. The error logging is spot-on for debugging issues.
🧰 Tools
🪛 Pylint (3.3.7)
[warning] 146-146: Catching too general exception Exception
(W0718)
Signed-off-by: Shahm Najeeb <[email protected]>
|
Code Climate has analyzed commit 8a93c7a and detected 0 issues on this pull request. View more on Code Climate. |
DefinetlyNotAI
added
bug/Medium
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
CODE/dump_memory.py (1)
108-108: Consider breaking up these long linesSome lines are getting pretty long (over 100 characters). While the code works fine, breaking them up would make it easier to read, especially on smaller screens.
For example, you could break line 108 like this:
- if free_space < required_space: - log.error(f"Not enough disk space. Need at least {required_space / (1024 ** 2):.2f} MiB") + if free_space < required_space: + required_mb = required_space / (1024 ** 2) + log.error(f"Not enough disk space. Need at least {required_mb:.2f} MiB")Also applies to: 137-138
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 108-108: Line too long (105/100)
(C0301)
CODE/logicytics/__init__.py (2)
14-14: Quick fix: Usesys.exit()instead ofexit()The built-in
exit()function isn't meant for production code - it's more of an interactive shell thing.sys.exit()is the proper way to exit from scripts.+import sys ... - exit("This is a library, Please import rather than directly run.") + sys.exit("This is a library, Please import rather than directly run.")🧰 Tools
🪛 Pylint (3.3.7)
[refactor] 14-14: Consider using 'sys.exit' instead
(R1722)
107-107: Line's getting a bit long hereThis line is 111 characters when the limit is 100. Easy fix - just split it across multiple lines to keep things readable.
-__all__ = ['execute', 'get', 'check', 'flag', 'file_management', 'deprecated', 'ObjectLoadError', 'log', 'Log'] +__all__ = [ + 'execute', 'get', 'check', 'flag', 'file_management', + 'deprecated', 'ObjectLoadError', 'log', 'Log' +]🧰 Tools
🪛 Pylint (3.3.7)
[convention] 107-107: Line too long (111/100)
(C0301)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.idea/csv-editor.xml(0 hunks)CODE/dump_memory.py(4 hunks)CODE/logicytics/__init__.py(2 hunks)
💤 Files with no reviewable changes (1)
- .idea/csv-editor.xml
🧰 Additional context used
🪛 Pylint (3.3.7)
CODE/dump_memory.py
[warning] 57-57: Catching too general exception Exception
(W0718)
[convention] 108-108: Line too long (105/100)
(C0301)
[convention] 137-137: Line too long (117/100)
(C0301)
[convention] 138-138: Line too long (108/100)
(C0301)
[warning] 78-78: Catching too general exception Exception
(W0718)
[warning] 85-85: Catching too general exception Exception
(W0718)
[warning] 147-147: Catching too general exception Exception
(W0718)
[warning] 119-119: Catching too general exception Exception
(W0718)
[warning] 142-142: Catching too general exception Exception
(W0718)
[warning] 161-161: Catching too general exception Exception
(W0718)
CODE/logicytics/__init__.py
[refactor] 14-14: Consider using 'sys.exit' instead
(R1722)
[convention] 107-107: Line too long (111/100)
(C0301)
🔇 Additional comments (16)
CODE/dump_memory.py (12)
10-13: Great job making this configurable!Moving from hardcoded values to config-based settings is a smart move. This makes the code way more flexible and easier to maintain. The variable names are clear and the fallback for
DUMP_DIRis a nice touch.
41-48: Smart helper function - reduces code duplication!This helper function is a clever way to avoid repeating the same memory info writing logic. The parameter
use_free_rather_than_availableis a bit long but it's clear what it does. Good job making this reusable!
51-58: Much cleaner error handling!The unified try-except block is way better than having scattered error handling. Using the helper function makes this code super clean and easy to read. The logging is consistent too.
🧰 Tools
🪛 Pylint (3.3.7)
[warning] 57-57: Catching too general exception Exception
(W0718)
75-75: Better user experience with "Unavailable"Changing from 'N/A' to 'Unavailable' makes it clearer for users when CPU frequency info isn't available. Small change but good attention to detail!
81-86: Good addition of file writing protection!Adding error handling specifically for file writing is smart. This prevents the whole function from crashing if there's a disk issue or permission problem when saving the system info.
🧰 Tools
🪛 Pylint (3.3.7)
[warning] 85-85: Catching too general exception Exception
(W0718)
104-109: Smart upfront disk space check!This is way better than checking space for each memory region. Doing one check at the beginning saves time and prevents wasted work. The math looks correct too - multiplying by safety margin makes sense.
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 108-108: Line too long (105/100)
(C0301)
112-113: Good early filtering for permissions!Skipping regions without read permissions right away is efficient. No point trying to process memory you can't actually read from.
115-121: Solid address parsing with error handling!The address parsing logic handles both single addresses and ranges correctly. Adding exception handling here prevents crashes from weird address formats. The warning log is helpful for debugging too.
🧰 Tools
🪛 Pylint (3.3.7)
[warning] 119-119: Catching too general exception Exception
(W0718)
123-131: Nice metadata structure!Adding the region size calculation (
end - start) gives more useful info. The metadata dictionary is well organized and includes all the important details about each memory region.
133-143: Better file size limiting and error handling!The truncation logic is much cleaner now. Writing a clear truncation message helps users understand why the dump stopped. The try-except around metadata writing prevents one bad region from breaking the whole dump.
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 137-137: Line too long (117/100)
(C0301)
[convention] 138-138: Line too long (108/100)
(C0301)
[warning] 142-142: Catching too general exception Exception
(W0718)
146-148: Improved error messages!Separating psutil errors from general errors gives better debugging info. The error messages are clear and help identify whether it's a memory access issue or something else.
🧰 Tools
🪛 Pylint (3.3.7)
[warning] 147-147: Catching too general exception Exception
(W0718)
159-163: Critical directory creation protection!Adding error handling for directory creation is super important. If you can't create the dump directory, there's no point continuing. The critical log level and early return make this fail gracefully.
🧰 Tools
🪛 Pylint (3.3.7)
[warning] 161-161: Catching too general exception Exception
(W0718)
CODE/logicytics/__init__.py (4)
10-10: Nice import expansion! 👍Good call adding both
logandLogto the import. This gives users flexibility to either use the pre-configuredloginstance or create their ownLoginstances if needed.
15-19: Smart refactoring with the lowercase instances! 🔥Converting from
Executetoexecute(and same for the others) follows Python naming conventions way better. Instance variables should definitely be lowercase - this makes the code much more readable and consistent.
21-21: Perfect update to use the new lowercase instance! ✨You correctly updated this to use
file_management.mkdir()instead of the old uppercase version. This consistency throughout the refactoring is exactly what we want to see.
107-107: Great addition of__all__! 📦Adding the
__all__list is awesome practice for a library module. It clearly defines what users can import and use from your package, making the API much cleaner and more explicit.🧰 Tools
🪛 Pylint (3.3.7)
[convention] 107-107: Line too long (111/100)
(C0301)
github-project-automation
bot
moved this from Waiting on Review
to In Progress
in Issue Board
Removed 2 commands from bluetooth_logger.py to fix the bug where is just fails due to the event log not actually existing Fully remade packet_sniffer.py to remove the already buggy code and deprecation warnings Signed-off-by: Shahm Najeeb <[email protected]>
Pull Request Template
Prerequisites
--devflag, if required.PR Type
update
Description
dump_memoryto include TODO logic and improve robustnessLogicytics.pywith basic clean-upMAX_FILE_SIZEglobal (default 50MB; None to ignore)max_workersandUNREADABLE_EXTENSIONStoconfig.iniprintstatements with structured logging across Logicytics libMotivation and Context
Based on
PLANS.mdI have worked on thisCredit
N/A
Issues Fixed
N/A
Summary by CodeRabbit
configparser,aiofiles, andcryptography.