Skip to content

SMB: add optional share report output under --shares#1167

Open
Dovendyrr wants to merge 1 commit intoPennyw0rth:mainfrom
Dovendyrr:feat/smb-share-report-enhancements
Open

SMB: add optional share report output under --shares#1167
Dovendyrr wants to merge 1 commit intoPennyw0rth:mainfrom
Dovendyrr:feat/smb-share-report-enhancements

Conversation

@Dovendyrr
Copy link

@Dovendyrr Dovendyrr commented Mar 20, 2026

Description

Adds SMB share reporting enhancements under --shares so NetExec can generate aggregated share reports without requiring a separate standalone action.

AI Usage Disclosure

  • AI tool(s) used: ChatGPT Codex (GPT-5)
  • AI-assisted scope:
    • design iteration for CLI/report UX
    • implementation support for reporting and HTML filtering logic
    • drafting/refinement of contribution text
  • Human verification performed:
    • reviewed and edited all generated code and text
    • ran lint checks (ruff) on changed files
    • manually validated behavior in local SMB test environment

Implemented

  • Added --shares-output [path] as an optional output switch tied to --shares
  • Added --shares-output-format {markdown,json,html}
  • Added --shares-output-high-risk <names...>
  • Added report engine in nxc/protocols/smb/share_report.py
  • Integrated report generation into SMB share enumeration flow in nxc/protocols/smb.py
  • Added host-aware keying (host:port) to avoid localhost multi-port overwrite collisions
  • Added enhanced HTML report UX:
    • Summary cards
    • Share pivot (click share -> see hosts)
    • Host breakdown collapsibles
    • Detailed shares host dropdown filter

Files changed

  • nxc/protocols/smb/proto_args.py
  • nxc/protocols/smb.py
  • nxc/protocols/smb/share_report.py (new)
  • tests/test_smb_share_report.py (new)

Type of change

  • New feature (non-breaking change)
  • Refactor (CLI behavior: reporting now option under --shares)
  • Test updates

Setup guide for the review

  1. Run SMB share enumeration with report output:

    nxc smb <targets> -u <user> -p <pass> \
      --shares \
      --shares-output /tmp/smb_report \
      --shares-output-format html
  2. Validate output behavior:

  • Summary renders host/share/read/write/high-risk totals
  • Share Pivot section allows clicking a share name to display affected hosts
  • Host Breakdown section shows per-host share details
  • Detailed Shares dropdown filters rows by host
  1. Optional high-risk tuning:

    nxc smb <targets> -u <user> -p <pass> \
      --shares \
      --shares-output /tmp/smb_report \
      --shares-output-format html \
      --shares-output-high-risk c$ admin$ wwwroot inetpub backups
  2. Validation commands used:

    /Users/safetynet/Tools/contributions/venv/bin/ruff check --isolated \
      nxc/protocols/smb.py \
      nxc/protocols/smb/proto_args.py \
      nxc/protocols/smb/share_report.py \
      tests/test_smb_share_report.py

Checklist

  • I have performed a self-review of my own code
  • I have added or updated tests where appropriate
  • Ruff lint checks pass for changed files
  • Existing --shares behavior remains functional
  • Help text and usage flow were updated to match CLI behavior
  • AI usage is fully disclosed (tool/model/scope)

@NeffIsBack
Copy link
Member

Hi and thanks for the PR.

This looks highly AI generated. What parts were done by you? As per the template:

If you have used AI in any form, please state the tool you used (e.g. Claude Code, Cursor, Amp) along with the extent that the work was AI-assisted. See the project's AI policy for more details: https://github.com/Pennyw0rth/NetExec/blob/main/AI_POLICY.md

Second thing, i don't really understand what the benefit of adding such a report is. What is a "high risk" share? And what does this functionality provide what isn't already provided by nxc or the nxcdb?

@Dovendyrr
Copy link
Author

Dovendyrr commented Mar 20, 2026 via email

@NeffIsBack
Copy link
Member

first time attempting to contribute on github so getting the
hang of things.

Welcome to the Open Source world then :)

This was vibed for sure (not the most knowledgable, wanting
to change this however and learn to code better!), I had used Codex 5.3 and
majority of the changes were done by it, with some minor tweaks I did where
it would overwrite data due to ingesting based off of the host only

So i know everyone is screaming "Do everything with AI" right now, but if you want my opinion (yes you didn't ask, but i give it to you anyway lol) learning how to code is better done without AI or at least not with prompting. AI tends to produce a lot of bloat and that can be seen in this code as well. Also, using prompts to generate code often skips the step where you need to think about the structure of the solution, which is one of the most important parts of coding.

The problem with AI generated code is that the code needs heavy reviewing and the bloat consumes a lot of time to trim down. In addition, AI most often does not respect the structure of a project and just places the code somewhere into existing functionality, where it does not make a lot of sense. So all in all, I don't think this will be merged in the current state. We do have spidering as you said as well as the nxcdb that contains and is able to export this kind of information. Not as HTML, but as CSV. We should first have a discussion what exactly is needed and where and in which form we would integrate it. For this a feature request issue would likely be better suited than a PR.

@Dovendyrr
Copy link
Author

Dovendyrr commented Mar 21, 2026 via email

@NeffIsBack
Copy link
Member

Sounds good! What would be your goal? I guess an HTML, but what should that contain? Are write/read privs enough data or do you want/need something else?

My initial intuition would be to use the data from the nxcdb since it is already stored in there when enumerating shares (and prone to changes for future data collection). We also already have some kind of export functionality for a csv and iirc text format that could be extended.

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.

2 participants