OWASP
Make Unix/Linux path handling OS-independent for Windows compatibility
This PR makes the Unix/Linux path handling OS-independent to ensure Nettacker works equally well on both Windows and Unix-like operating systems.
I've requested assignment for issue #933 and have completed the implementation with full testing on Windows.
Changes Made Refactored path operations in 5 core files to use OS-independent methods:
Fixed graph discovery, language loading, and module loading in arg_parser Fixed language discovery in messages module Fixed file download headers in API engine Added UTF-8 encoding to YAML file operations in template module for Windows compatibility Added clarifying comment in common utils to distinguish URL parsing from file path operations All changes replace hardcoded forward slash operations with pathlib properties and os.path functions.
Testing Performed Tested comprehensively on Windows 10 with Python 3.10.0:
Scan Engine: All 117 modules discovered and loaded successfully API: File download headers working correctly with Windows paths WebUI: Language discovery (23 languages) and graph discovery (2 graphs) working Module Loading: All YAML files parsed without encoding errors Path Handling: Windows backslash paths working correctly Database: SQLite connection and operations working All changes are cross-platform and fully backwards compatible with Unix/Linux systems.
Fixes #933
Type of change Bugfix (non-breaking change which fixes an issue) Checklist I've followed the contributing guidelines I've run make pre-commit, it didn't generate any changes I've run make test, all tests passed locally
[!NOTE]
Other AI code review bot(s) detected
CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.
Summary by CodeRabbit
-
Refactor
- Improved internal path and file-name handling across core modules for more consistent cross-platform behavior.
- Streamlined language and module metadata extraction.
-
Documentation
- Added and clarified docstrings and descriptive comments throughout the codebase.
-
Bug Fixes
- Ensured template files are read using UTF-8 to prevent character encoding issues.
✏️ Tip: You can customize this high-level summary in your review settings.
Walkthrough
Replaced string-based path manipulations with platform-agnostic pathlib/os.path calls, added UTF-8 file encoding and a few docstrings/comments, and a minor whitespace change. No public interfaces or control flow changes; behavior preserved.
Changes
| Cohort / File(s) | Summary |
|---|---|
API filename extraction nettacker/api/engine.py |
Use os.path.basename(filename) for Content-Disposition filename instead of filename.split("/")[-1]; added a blank line after a multiprocessing loop (no functional change). |
Argument, graph and module path parsing nettacker/core/arg_parser.py |
Replace string-splitting with pathlib.Path properties: derive graph name from parent.name, use Path.stem for language/module names, and get module category from parent.name. |
Language name extraction nettacker/core/messages.py |
Use Path.stem to obtain language identifiers from locale file paths instead of manual string parsing. |
Template file handling and docs nettacker/core/template.py |
Open YAML files with encoding="utf-8" and add docstrings to open() and format(); loading/formatting logic unchanged. |
Clarifying docstring/comment nettacker/core/utils/common.py |
Added a descriptive docstring to re_address_repeaters_key_name clarifying slash-delimited key behavior; no signature change. |
Estimated code review effort
🎯 2 (Simple) | ⏱️ ~10 minutes
- Verify Path-based extractions match previous behavior for edge cases (filenames without extensions, names with multiple dots, Windows backslashes).
- Confirm Content-Disposition filename quoting/escaping is still correct.
- Ensure YAML files load correctly with explicit
utf-8encoding.
Pre-merge checks and finishing touches
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | ⚠️ Warning | Docstring coverage is 72.73% which is insufficient. The required threshold is 80.00%. | You can run @coderabbitai generate docstrings to improve docstring coverage. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title clearly and specifically describes the main objective of the PR: making Unix/Linux path handling OS-independent for Windows compatibility, which directly aligns with the core changes. |
| Description check | ✅ Passed | The description is detailed and directly related to the changeset, explaining the path handling refactoring across five core files, testing performed on Windows, and how changes ensure cross-platform compatibility. |
| Linked Issues check | ✅ Passed | The PR fulfills the linked issue #933 requirements by replacing Unix/Linux-style path operations with OS-independent methods across all affected modules (arg_parser, messages, engine, template, common.py), and testing confirms functionality on both Windows and Unix-like systems. |
| Out of Scope Changes check | ✅ Passed | All changes are scoped to OS-independent path handling refactoring across five core files. Minor additions like docstrings and blank lines are supporting changes; no unrelated functionality modifications detected. |
✨ Finishing touches
- [ ] 📝 Generate docstrings
🧪 Generate unit tests (beta)
- [ ] Create PR with unit tests
- [ ] Post copyable unit tests in a comment
📜 Recent review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between 4ba29e7572267d7f10079eea82bb630595cc8fa1 and d1e8544fc22f2a8b42ecd5e4e2387f47a26a460d.
📒 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
🔇 Additional comments (1)
nettacker/core/utils/common.py (1)
168-183: Excellent clarification in the docstring.The docstring clearly documents that "/" is used as a key delimiter for nested dictionary access, not as a file path separator. This distinction is important for the PR's cross-platform objectives and prevents confusion when reviewing path-handling code.
[!TIP]
✨ Issue Enrichment is now available for GitHub issues!
CodeRabbit can now help you manage issues more effectively:
- Duplicate Detection — Identify similar or duplicate issues
- Related Issues & PRs — Find relevant issues and PR's from your repository
- Suggested Assignees — Find the best person to work on the issue
- Implementation Planning — Generate detailed coding plans for engineers and agents
Disable automatic issue enrichment
To disable automatic issue enrichment, add the following to your
.coderabbit.yaml:issue_enrichment: auto_enrich: enabled: false
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 help to get the list of available commands and usage tips.
![coderabbitai[bot] avatar](https://avatars.githubusercontent.com/in/347564?v=4 "Published by a pub.dev verified publisher"){kind=small}
✅ Actions performed
Initiated docstring generation; will generate only if new commits exist.
[!NOTE] Docstrings generation - SUCCESS Generated docstrings for this pull request at https://github.com/OWASP/Nettacker/pull/1174
Hi maintainers
This PR fixes Windows path compatibility issues by replacing Unix-specific path operations with OS-independent methods (os.path.basename(), .stem, .parent.name).
Changes:
- Replaced
split("/")with proper pathlib/os.path methods - Added docstrings per CodeRabbit review
- All formatting fixes applied
The CI workflow is awaiting approval to run. Could a maintainer please approve the workflow? Thanks!
Fixes #1039
Hi @subhash-0000,
I tested PR #1173 locally on macOS. The path-handling changes work as expected, module resolution and the updated directory structure behave correctly.
The only failures I encountered were due to Python 3.12 removing ssl.wrap_socket,which affects the test suite but isn’t related to your changes.
To address this and make the test suite compatible with Python 3.12+, I opened a separate test-only compatibility PR here: https://github.com/OWASP/Nettacker/pull/1180
With that shim applied, all 214 tests pass locally, and PR #1173 runs cleanly.
Happy to test anything else if needed!
@deekshithaby Thank you so much for testing this locally
Great to hear the path-handling changes work correctly on macOS and that module resolution is functioning as expected. I appreciate you opening #1180 to address the Python 3.12 SSL compatibility issue separately - that's very helpful!
Looking forward to getting this merged once the maintainers approve the workflow and review the changes. Thanks again for your thorough testing!