Skip to content

Fix stale TODO comment around hidden WPA handshake --strip flag#470

Merged
kimocoder merged 2 commits intomasterfrom
copilot/fix-stale-todo-wpa-handshake
Mar 19, 2026
Merged

Fix stale TODO comment around hidden WPA handshake --strip flag#470
kimocoder merged 2 commits intomasterfrom
copilot/fix-stale-todo-wpa-handshake

Conversation

Copy link
Copy Markdown

Copilot AI commented Mar 19, 2026

The # TODO: Uncomment the --strip option once it works comment in wifite/args.py was misleading — the feature is fully implemented (Handshake.strip() exists, config.py parses wpa_strip_handshake, and attack/wpa.py calls it), but the public --strip flag is deliberately hidden due to a known aircrack-ng compatibility risk documented in Handshake.strip() itself.

Changes

  • wifite/args.py: Replace the stale TODO with an accurate comment explaining why the public --strip flag stays hidden — stripped captures may break aircrack-ng and other downstream tools — and that the hidden -strip alias is retained intentionally pending broader validation.
# Before
# TODO: Uncomment the --strip option once it works

# After
# Handshake stripping is implemented (see Handshake.strip()), but the public
# --strip flag remains hidden because stripped captures may break compatibility
# with aircrack-ng and other downstream tools (Handshake.strip() warns of this).
# Keep only the hidden compatibility alias (-strip) until the behavior is
# validated across all supported cracking tools.

No runtime behavior is changed.

Original prompt

Open a pull request in kimocoder/wifite2 to fix the stale TODO/comment around the WPA handshake stripping CLI flag.

Context

In wifite/args.py, there is a stale comment near the hidden -strip flag:

# TODO: Uncomment the --strip option once it works
'''
wpa.add_argument('--strip',
    action='store_true',
    dest='wpa_strip_handshake',
    default=False,
    help=Color.s('Strip unnecessary packets from handshake capture using tshark'))
'''
wpa.add_argument('-strip', help=argparse.SUPPRESS, action='store_true', dest='wpa_strip_handshake')

Repo inspection shows the feature is already implemented internally:

  • wifite/config.py parses args.wpa_strip_handshake
  • wifite/model/handshake.py implements Handshake.strip()
  • wifite/attack/wpa.py uses Configuration.wpa_strip_handshake when saving handshakes

However, Handshake.strip() contains this warning:

  • # XXX: This method might break aircrack-ng, use at own risk.

That means the existing TODO is misleading. The likely intent is not “uncomment once it works” but rather “keep hidden until compatibility is validated/documented.”

Requested change

Make the smallest safe change that fixes this stale/misleading comment.

Requirements

  1. Update the comment in wifite/args.py so it accurately reflects the current state.
  2. Do not expose the public --strip flag unless you can prove from the code/tests that it is safe and intended.
  3. Preserve current behavior.
  4. Keep the hidden -strip compatibility alias as-is unless there is a very strong reason to change it.
  5. If helpful, add a brief clarifying comment referencing that Handshake.strip() may affect downstream tool compatibility.

Preferred outcome

A minimal PR that replaces the stale TODO with an accurate explanatory comment, without changing functionality.

Validation

  • Ensure no behavior changes are introduced.
  • If there are relevant tests, run them; otherwise keep the change comment-only/minimal.

Deliverable

Open a PR with the stale TODO/comment corrected.

The following is the prior conversation context from the user's chat exploration (may be truncated):

User: Scan the project for issues, problems or improvements to add. Summary me the findings

Assistant:

I'll scan the repository to understand its structure, current issues, and code quality. Let me gather information from multiple angles.

Assistant: Now let me scan the codebase for potential issues, code quality problems, and improvement opportunities.

Assistant: Now let me also look at the existing TODO items and check for test coverage:

Assistant: Here's a comprehensive summary of my findings after scanning the kimocoder/wifite2 project:


📋 Project Scan Summary

📊 Overview

  • Language: Python | License: GPLv2 | ⭐ 1,426 stars | 🍴 254 forks
  • Fork of: derv82/wifite2 — actively maintained with significant new features (Evil Twin, TUI, WPA3/SAE, dual-interface, wpa-sec upload, etc.)
  • Open Issues: 2 | Open PRs: 0

🐛 Open Issues

# Title Summary
#363 aircrack-ng NOT utilising "neon" SIMD on Raspberry Pi 5 On RPi5, wifite uses --simd=generic instead of neon, resulting in ~13x slower hash rates (210 k/s vs 2800 k/s). Wifite may need to detect SIMD capabilities and pass --simd=neon to aircrack-ng explicitly.
#160 WPS always "No" on WiFi Pineapple MK7 On OpenWRT-based devices, WPS always shows as "No" or "Lock". Likely a parsing/compatibility issue with older tool versions on embedded platforms.

🔧 Code-Level Findings

1. Acknowledged TODOs & HACKs in Code 🟡

Multiple TODO, FIXME, XXX, and HACK markers exist throughout the codebase:

  • wifite/model/handshake.pyHACK: Arbitrarily selects the first BSSID/ESSID pair when neither is known; a user selection menu is missing (TODO: Display menu for user to select from list).
  • wifite/model/handshake.pyXXX: The strip() method warns it may break aircrack-ng.
  • wifite/util/crack.pyTODO: Bring back the 'print' option, for easy copy/pasting.
  • wifite/config.pyTODO: categorize configuration into separate classes (under config/*.py) — the Configuration class is a ~1,440-line monolith holding all settings as class variables.
  • wifite/args.pyTODO: Uncomment the --strip option once it works — feature disabled/commented out.
  • wifite/tools/airmon.pyTODO: add support for other unorthodox init systems.
  • wifite/attack/wpa.pyTODO: Potentially add a fallback or user choice for aircrack-ng for non-SAE.
  • docs/EVILTWIN.md — Multiple TODOs for HTTPS spoofing, password validation, CGI server state management, and Apple/Google captive portal detection.
  • docs/TODO.md...

This pull request was created from Copilot chat.


💬 Send tasks to Copilot coding agent from Slack and Teams to turn conversations into code. Copilot posts an update in your thread when it's finished.

Co-authored-by: kimocoder <4252297+kimocoder@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix stale TODO comment for WPA handshake stripping flag Fix stale TODO comment around hidden WPA handshake --strip flag Mar 19, 2026
Copilot AI requested a review from kimocoder March 19, 2026 03:31
@kimocoder kimocoder marked this pull request as ready for review March 19, 2026 03:31
Copilot AI review requested due to automatic review settings March 19, 2026 03:31
@kimocoder kimocoder merged commit e6a1455 into master Mar 19, 2026
9 of 12 checks passed
@kimocoder kimocoder deleted the copilot/fix-stale-todo-wpa-handshake branch March 19, 2026 03:31
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates a stale/misleading TODO in the WPA CLI argument definitions to accurately document why the handshake “strip” functionality is kept non-public, aligning the CLI help/comments with the already-implemented internal behavior.

Changes:

  • Replace the outdated “uncomment once it works” TODO with an explanation that stripping is implemented but not publicly exposed due to downstream tool compatibility risk.
  • Clarify that the hidden -strip alias is intentionally retained pending broader validation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +814 to +818
# Handshake stripping is implemented (see Handshake.strip()), but the public
# --strip flag remains hidden because stripped captures may break compatibility
# with aircrack-ng and other downstream tools (Handshake.strip() warns of this).
# Keep only the hidden compatibility alias (-strip) until the behavior is
# validated across all supported cracking tools.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants