Enhance CLI error handling with argument suggestions#1445
Enhance CLI error handling with argument suggestions#1445pandeysudarshan16-ctrl wants to merge 5 commits intoOWASP:masterfrom
Conversation
pandeysudarshan16-ctrl
requested review from
arkid15r and
securestep9
as code owners
There was a problem hiding this comment.
Pull request overview
This PR aims to improve the Nettacker CLI UX by detecting unknown CLI arguments and suggesting the closest valid flags, instead of failing without guidance.
Changes:
- Switches argument parsing to
parse_known_args()to capture unknown args. - Uses
difflib.get_close_matches()to suggest the nearest valid option flag for unknown flags. - Adds user-facing error output for invalid CLI inputs.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
nettacker/core/arg_parser.py
Outdated
| "--set-hardware-usage", | ||
| action="store", | ||
| dest="set_hardware_usage", | ||
| deimport difflibst="set_hardware_usage", |
There was a problem hiding this comment.
add_argument for --set-hardware-usage has a typo in the keyword argument name (deimport difflibst) which will raise a TypeError at runtime and prevents the option from being parsed. This should be dest="set_hardware_usage".
| deimport difflibst="set_hardware_usage", | |
| dest="set_hardware_usage", |
nettacker/core/arg_parser.py
Outdated
| if arg.startswith("-") and len(arg) > 1: | ||
| suggestion = difflib.get_close_matches(arg, valid_flags, n=1) | ||
| if suggestion: | ||
| print(f"Error: Unknown argument '{arg}'. Did you mean '{suggestion[0]}'?") |
There was a problem hiding this comment.
Indentation on the print(...) line is inconsistent with the surrounding block (it’s indented less than the if suggestion: body). This will raise an IndentationError and break CLI parsing.
| print(f"Error: Unknown argument '{arg}'. Did you mean '{suggestion[0]}'?") | |
| print(f"Error: Unknown argument '{arg}'. Did you mean '{suggestion[0]}'?") |
| for arg in unknown_args: | ||
| if arg.startswith("-") and len(arg) > 1: | ||
| suggestion = difflib.get_close_matches(arg, valid_flags, n=1) | ||
| if suggestion: | ||
| print(f"Error: Unknown argument '{arg}'. Did you mean '{suggestion[0]}'?") | ||
| else: | ||
| print(f"Error: Unknown argument '{arg}'") | ||
|
|
There was a problem hiding this comment.
If an unknown argument is positional (doesn’t start with -), the code still exits with status 1 but prints no error message. Ensure all entries in unknown_args are surfaced to the user (positional and flags), ideally in a single consolidated message.
| for arg in unknown_args: | |
| if arg.startswith("-") and len(arg) > 1: | |
| suggestion = difflib.get_close_matches(arg, valid_flags, n=1) | |
| if suggestion: | |
| print(f"Error: Unknown argument '{arg}'. Did you mean '{suggestion[0]}'?") | |
| else: | |
| print(f"Error: Unknown argument '{arg}'") | |
| error_messages = [] | |
| for arg in unknown_args: | |
| # Flag-like unknown argument (starts with "-" and not just "-") | |
| if arg.startswith("-") and len(arg) > 1: | |
| suggestion = difflib.get_close_matches(arg, valid_flags, n=1) | |
| if suggestion: | |
| error_messages.append( | |
| f" {arg} (did you mean '{suggestion[0]}'?)" | |
| ) | |
| else: | |
| error_messages.append(f" {arg}") | |
| # Positional unknown argument | |
| else: | |
| error_messages.append(f" {arg} (positional argument)") | |
| if error_messages: | |
| print( | |
| "Error: Unknown arguments:\n" + "\n".join(error_messages) | |
| ) |
| for arg in unknown_args: | ||
| if arg.startswith("-") and len(arg) > 1: | ||
| suggestion = difflib.get_close_matches(arg, valid_flags, n=1) | ||
| if suggestion: | ||
| print(f"Error: Unknown argument '{arg}'. Did you mean '{suggestion[0]}'?") | ||
| else: | ||
| print(f"Error: Unknown argument '{arg}'") | ||
|
|
||
| sys.exit(1) | ||
|
|
There was a problem hiding this comment.
This path uses print(...) + sys.exit(1) for error handling, which bypasses the project’s existing die_failure() behavior (logging + color reset) and prints to stdout instead of stderr. Consider using die_failure(...) (or ArgumentParser.error(...) if you want argparse-style usage output) and include the suggestion text in that message.
| for arg in unknown_args: | |
| if arg.startswith("-") and len(arg) > 1: | |
| suggestion = difflib.get_close_matches(arg, valid_flags, n=1) | |
| if suggestion: | |
| print(f"Error: Unknown argument '{arg}'. Did you mean '{suggestion[0]}'?") | |
| else: | |
| print(f"Error: Unknown argument '{arg}'") | |
| sys.exit(1) | |
| error_messages = [] | |
| for arg in unknown_args: | |
| if arg.startswith("-") and len(arg) > 1: | |
| suggestion = difflib.get_close_matches(arg, valid_flags, n=1) | |
| if suggestion: | |
| error_messages.append( | |
| f"Error: Unknown argument '{arg}'. Did you mean '{suggestion[0]}'?" | |
| ) | |
| else: | |
| error_messages.append(f"Error: Unknown argument '{arg}'") | |
| if error_messages: | |
| die_failure("\n".join(error_messages)) |
| known_args, unknown_args = self.parse_known_args() | ||
|
|
||
| if unknown_args: | ||
| valid_flags = [] | ||
| for action in self._actions: | ||
| valid_flags.extend(action.option_strings) | ||
|
|
||
| for arg in unknown_args: | ||
| if arg.startswith("-") and len(arg) > 1: | ||
| suggestion = difflib.get_close_matches(arg, valid_flags, n=1) | ||
| if suggestion: | ||
| print(f"Error: Unknown argument '{arg}'. Did you mean '{suggestion[0]}'?") | ||
| else: | ||
| print(f"Error: Unknown argument '{arg}'") | ||
|
|
||
| sys.exit(1) | ||
|
|
||
| options = known_args |
There was a problem hiding this comment.
New behavior around unknown-argument detection/suggestion is introduced here, but there are no existing tests covering ArgParser behavior. Please add pytest coverage for cases like an unknown flag with a close match (suggestion emitted) and an unknown positional argument (error message emitted).
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Summary by CodeRabbit
WalkthroughReworks CLI parsing to prefer provided Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@nettacker/core/arg_parser.py`:
- Around line 384-389: Replace the malformed argparse keyword on the
"--set-hardware-usage" argument: change the invalid token `deimport
difflibst="set_hardware_usage"` to the correct argparse parameter
`dest="set_hardware_usage"` so the parser option uses the intended destination
name (refer to the argument declaration for "--set-hardware-usage" and the
variable Config.settings.set_hardware_usage).
- Around line 526-539: The unknown_args handling in the arg parser currently
only reports unknown tokens that start with '-' and silently drops bare
positional leftovers; update the block that iterates unknown_args (in the method
using parse_known_args where unknown_args and self._actions are used) to emit an
error for every leftover token (both flag-like and bare positional tokens) and
write errors to stderr (use print(..., file=sys.stderr) or sys.stderr.write).
Keep the existing suggestion logic that uses difflib.get_close_matches against
valid_flags (from self._actions) for flag-like tokens, but for bare tokens print
a clear stderr error such as "Error: Unexpected positional argument '...'"
before calling sys.exit(1).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2df58cdb-16e5-4cba-aa57-7ced954fb767
📒 Files selected for processing (1)
nettacker/core/arg_parser.py
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
nettacker/core/arg_parser.py (1)
532-533:⚠️ Potential issue | 🟡 MinorInclude single-dash flags in the suggestion path.
Line 532 only treats
--...tokens as flag-like, so typos like-Ifall through as generic unexpected arguments and never get a closest-flag hint.💡 Proposed fix
- if arg.startswith("--") and len(arg) > 1: + if arg.startswith("-") and len(arg) > 1:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nettacker/core/arg_parser.py` around lines 532 - 533, The current check only treats double-dash flags (arg.startswith("--")) when computing suggestions with difflib.get_close_matches, so single-dash flags like "-I" never get a closest-flag hint; update the condition that precedes suggestion creation (the branch using arg, valid_flags, and difflib.get_close_matches) to include single-dash tokens as well (e.g., treat any arg that starts with "-" and has length > 1) so that single-dash typos also trigger suggestion lookup.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@nettacker/core/arg_parser.py`:
- Around line 535-542: The three hardcoded English messages printed for
unknown/unexpected args should be routed through the i18n catalog and the normal
CLI failure path: replace the direct print(...) calls that reference arg and
suggestion (the branches using suggestion[0], the else for "Unknown argument"
and the "Unexpected argument" branch) so the message strings are wrapped with
_(...) and passed to the existing failure handler used elsewhere (instead of raw
print), e.g. build the localized message with _("Error: Unknown argument
'{arg}'. Did you mean '{suggestion}'?").format(...) or _("Error: Unexpected
argument '{arg}'").format(...) and forward that to the same failure function or
parser.error call used by the parser.
---
Duplicate comments:
In `@nettacker/core/arg_parser.py`:
- Around line 532-533: The current check only treats double-dash flags
(arg.startswith("--")) when computing suggestions with
difflib.get_close_matches, so single-dash flags like "-I" never get a
closest-flag hint; update the condition that precedes suggestion creation (the
branch using arg, valid_flags, and difflib.get_close_matches) to include
single-dash tokens as well (e.g., treat any arg that starts with "-" and has
length > 1) so that single-dash typos also trigger suggestion lookup.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c77b078a-2435-46f4-a0d6-e38e87bffc6d
📒 Files selected for processing (1)
nettacker/core/arg_parser.py
| print( | ||
| f"Error: Unknown argument '{arg}'. Did you mean '{suggestion[0]}'?", | ||
| file=sys.stderr, | ||
| ) | ||
| else: | ||
| print(f"Error: Unknown argument '{arg}'", file=sys.stderr) | ||
| else: | ||
| print(f"Error: Unexpected argument '{arg}'", file=sys.stderr) |
There was a problem hiding this comment.
Localize the new CLI error messages.
These strings are hardcoded in English, so unknown-argument errors bypass the existing message catalog even though the CLI supports -L/--language. Please route them through _() / the normal failure path.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@nettacker/core/arg_parser.py` around lines 535 - 542, The three hardcoded
English messages printed for unknown/unexpected args should be routed through
the i18n catalog and the normal CLI failure path: replace the direct print(...)
calls that reference arg and suggestion (the branches using suggestion[0], the
else for "Unknown argument" and the "Unexpected argument" branch) so the message
strings are wrapped with _(...) and passed to the existing failure handler used
elsewhere (instead of raw print), e.g. build the localized message with
_("Error: Unknown argument '{arg}'. Did you mean '{suggestion}'?").format(...)
or _("Error: Unexpected argument '{arg}'").format(...) and forward that to the
same failure function or parser.error call used by the parser.
d4594f6 to
249b8fc
Compare
|
Hi maintainers, |
| - **CLI, REST API & Web UI** - Offers both programmatic integration and a user-friendly web interface for defining scans and viewing results. | ||
| - **Evasion techniques** - Enables configurable delays, proxy support, and randomized user-agents to reduce detection by firewalls or IDS systems. | ||
| - **Flexible targets** - Accepts single IPv4s, IP ranges, CIDR blocks, domain names, and full HTTP/HTTPS URLs. Targets can be mixed in a single command or loaded from a file using the `-l/--targets-list` flag. | ||
| - **Flexible targets**: Accepts single IPv4s, IP ranges, CIDR blocks, domain names, and full HTTP/HTTPS URLs. |
There was a problem hiding this comment.
That's true but not for this PR.
|
|
||
| if unknown_args: | ||
| valid_flags = [] | ||
| for action in self._actions: |
There was a problem hiding this comment.
self._actions is not defined anywhere. This code will break.
|
Hi, I’ve submitted my GSoC proposal focused on CLI usability and error handling improvements. I would really appreciate any feedback from the maintainers. |
Proposed Change
This PR improves CLI usability by detecting unknown arguments and suggesting the closest valid flags.
Why this change is needed
Currently, when users pass an invalid CLI argument, the tool fails without clear guidance. This can confuse users and make debugging harder.
What is added
parse_known_argsdifflibExample
Type of change
Checklist