Windows compatibility support #1151
Conversation
uvloop does not support Windows. This change makes it conditional using platform markers, only installing on non-Windows platforms.
- Make uvloop optional using platform markers (Windows unsupported) - Add win32 to allowed platforms in check_dependencies() - Replace os.geteuid() with cross-platform privilege checking - Add explicit UTF-8 encoding for file operations - Fix hardcoded path separators using pathlib in: - load_graphs() - load_modules() - load_languages() - get_result_content() This enables Nettacker to run on Windows systems.
- Make uvloop optional using platform markers (Windows unsupported) - Add win32 to allowed platforms in check_dependencies() - Replace os.geteuid() with cross-platform privilege checking - Add explicit UTF-8 encoding for file operations - Fix hardcoded path separators using pathlib in: - load_graphs() - load_modules() - load_languages() - get_result_content() Resolves #[933]
Summary by CodeRabbit
WalkthroughReplaces string-based path handling with pathlib, adds a cross-platform is_running_with_privileges() helper and uses it for ICMP gating, ensures UTF-8 when reading templates, extends platform checks to Windows, and marks uvloop as non-Windows-specific in pyproject.toml. (≤50 words) Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
nettacker/core/arg_parser.py (2)
68-68: Remove redundant Path() wrapper.Since
Config.path.locale_dir.glob("*.yaml")already yieldsPathobjects, thePath()wrapper is unnecessary.Apply this diff:
- languages_list.append(Path(language).stem) + languages_list.append(language.stem)
85-87: Remove redundant Path() wrapper.Since
Config.path.modules_dir.glob("**/*.yaml")already yieldsPathobjects, wrappingmodule_nameinPath()again is unnecessary.Apply this diff:
- module_path = Path(module_name) - library = module_path.stem - category = module_path.parent.name + library = module_name.stem + category = module_name.parent.name
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
nettacker/api/engine.py(2 hunks)nettacker/core/app.py(3 hunks)nettacker/core/arg_parser.py(4 hunks)nettacker/core/template.py(1 hunks)pyproject.toml(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use 4-space indents in Python code
Limit lines to 99 characters (ruff/ruff-format/isort profile=black)
Module and file names should use lower_snake_case
Function and variable names should use lower_snake_case
Class names should use PascalCase
Constants should use UPPER_SNAKE_CASE
Keep functions small and add type hints where practical
Files:
nettacker/core/app.pynettacker/core/template.pynettacker/api/engine.pynettacker/core/arg_parser.py
nettacker/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Add docstrings for public APIs in the nettacker package
Files:
nettacker/core/app.pynettacker/core/template.pynettacker/api/engine.pynettacker/core/arg_parser.py
nettacker/core/**
📄 CodeRabbit inference engine (AGENTS.md)
Place core libraries under nettacker/core/
Files:
nettacker/core/app.pynettacker/core/template.pynettacker/core/arg_parser.py
nettacker/api/**
📄 CodeRabbit inference engine (AGENTS.md)
Place API-related code under nettacker/api/
Files:
nettacker/api/engine.py
🧬 Code graph analysis (1)
nettacker/core/template.py (1)
nettacker/config.py (1)
Config(183-187)
🪛 Ruff (0.14.0)
nettacker/core/app.py
52-52: Do not catch blind exception: Exception
(BLE001)
🔇 Additional comments (7)
nettacker/core/template.py (1)
35-37: LGTM! Good cross-platform practice.Explicitly specifying UTF-8 encoding ensures consistent YAML parsing across platforms, particularly on Windows where the default encoding differs from Unix systems.
nettacker/api/engine.py (1)
8-8: LGTM! Improved cross-platform path handling.Using
Path(filename).namecorrectly extracts the basename on both Unix and Windows, replacing the Unix-specificfilename.split("/")[-1]approach.Also applies to: 396-396
pyproject.toml (1)
65-65: LGTM! Correct platform-specific dependency.The platform marker correctly excludes uvloop on Windows since it relies on Unix-specific APIs (libuv) that are not available on Windows.
nettacker/core/arg_parser.py (1)
54-54: LGTM! Good use of pathlib for cross-platform compatibility.Using
.parent.namecorrectly extracts the directory name in a cross-platform manner.nettacker/core/app.py (3)
40-56: LGTM! Well-implemented cross-platform privilege detection.The function correctly detects elevated privileges on both Windows (Administrator) and Unix (root). The broad exception handler on Line 52 is appropriate here to ensure graceful degradation if privilege detection fails, though you may want to log the exception for debugging purposes.
Optional enhancement for better observability:
try: import ctypes return ctypes.windll.shell32.IsUserAnAdmin() != 0 - except Exception: + except Exception as e: + log.debug(f"Failed to check Windows admin privileges: {e}") return False
87-87: LGTM! Correct platform support addition.Adding
"win32"to the supported platforms list enables Windows compatibility as intended by this PR.
186-186: LGTM! Correct cross-platform privilege check.Replacing the Unix-only
os.geteuid() == 0with the newis_running_with_privileges()function maintains the existing behavior while adding Windows support.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
nettacker/core/utils/common.py (2)
465-465: Remove redundant ctypes import.The
ctypesmodule is already imported at the module level (line 2), so the local import inside this function is unnecessary.Apply this diff:
if sys.platform == "win32": try: - import ctypes - return ctypes.windll.shell32.IsUserAnAdmin() != 0 except Exception:
468-469: Catch specific exceptions instead of generic Exception.Catching a broad
Exceptioncan mask unexpected errors. Consider catching more specific exceptions such asAttributeErrororOSErrorthat might occur when accessingctypes.windll.shell32.IsUserAnAdmin().Based on static analysis hints.
Apply this diff:
return ctypes.windll.shell32.IsUserAnAdmin() != 0 - except Exception: + except (AttributeError, OSError): return False
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
nettacker/core/app.py(3 hunks)nettacker/core/utils/common.py(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use 4-space indents in Python code
Limit lines to 99 characters (ruff/ruff-format/isort profile=black)
Module and file names should use lower_snake_case
Function and variable names should use lower_snake_case
Class names should use PascalCase
Constants should use UPPER_SNAKE_CASE
Keep functions small and add type hints where practical
Files:
nettacker/core/utils/common.pynettacker/core/app.py
nettacker/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Add docstrings for public APIs in the nettacker package
Files:
nettacker/core/utils/common.pynettacker/core/app.py
nettacker/core/**
📄 CodeRabbit inference engine (AGENTS.md)
Place core libraries under nettacker/core/
Files:
nettacker/core/utils/common.pynettacker/core/app.py
🧬 Code graph analysis (1)
nettacker/core/app.py (1)
nettacker/core/utils/common.py (2)
wait_for_threads_to_finish(55-72)is_running_with_privileges(456-471)
🪛 Ruff (0.14.0)
nettacker/core/utils/common.py
468-468: Do not catch blind exception: Exception
(BLE001)
🔇 Additional comments (3)
nettacker/core/app.py (3)
30-30: LGTM!The import of
is_running_with_privilegesis correctly added to support cross-platform privilege checking.
69-69: LGTM!Adding
"win32"to the platform check correctly enables Windows compatibility.
168-168: LGTM!Replacing the Unix-specific
os.geteuid() == 0with the cross-platformis_running_with_privileges()correctly enables privilege checking on Windows while maintaining backward compatibility with Unix-like systems.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
nettacker/core/utils/common.py (1)
456-469: Cross-platform privilege check implemented correctly.The function successfully achieves Windows compatibility by using platform-specific APIs. The logic is sound:
IsUserAnAdmin()for Windows andos.geteuid() == 0for Unix systems.Recommended refinements:
- Narrow the exception handler to catch specific exceptions rather than bare
Exception. This prevents masking unexpected errors while still failing gracefully:if sys.platform == "win32": try: return ctypes.windll.shell32.IsUserAnAdmin() != 0 - except Exception: + except (AttributeError, OSError): return False
- Add type hint for the return value:
-def is_running_with_privileges(): +def is_running_with_privileges() -> bool:
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
nettacker/core/utils/common.py(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use 4-space indents in Python code
Limit lines to 99 characters (ruff/ruff-format/isort profile=black)
Module and file names should use lower_snake_case
Function and variable names should use lower_snake_case
Class names should use PascalCase
Constants should use UPPER_SNAKE_CASE
Keep functions small and add type hints where practical
Files:
nettacker/core/utils/common.py
nettacker/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Add docstrings for public APIs in the nettacker package
Files:
nettacker/core/utils/common.py
nettacker/core/**
📄 CodeRabbit inference engine (AGENTS.md)
Place core libraries under nettacker/core/
Files:
nettacker/core/utils/common.py
🪛 Ruff (0.14.0)
nettacker/core/utils/common.py
466-466: Do not catch blind exception: Exception
(BLE001)
🔇 Additional comments (1)
nettacker/core/utils/common.py (1)
8-8: LGTM! Import necessary for Unix privilege check.The
osmodule import is correctly placed and required for theos.geteuid()call in the new cross-platform privilege check function.
|
@arkid15r @securestep9 , Just following up on this PR. Happy to address any feedback or make additional changes if needed. Thanks! |
2 participants
Proposed change
This PR adds Windows compatibility to OWASP Nettacker by addressing platform-specific issues that previously prevented it from running on Windows systems.
Changes:
Related: Replaces #932 with properly signed commits as requested.
Type of change
Checklist
make pre-commit, it didn't generate any changesmake test, all tests passed locallyTested manually on windows since make commands are unix specific
[contributing-guidelines]: https://nettacker.readthedocs.io/en/latest/Developers/