From eb4e0d8e95e4c9957f1f1b48ad12f6efcbac9042 Mon Sep 17 00:00:00 2001 From: Gabriel Lluch Date: Sat, 1 Aug 2026 15:57:23 -0700 Subject: [PATCH] feat(audit): a report nobody finishes reading teaches nothing --- README.md | 22 ++++++- netdiff/cli.py | 28 ++++++++- tests/test_cli_output.py | 130 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 177 insertions(+), 3 deletions(-) create mode 100644 tests/test_cli_output.py diff --git a/README.md b/README.md index 611389e..7a2dfd0 100644 --- a/README.md +++ b/README.md @@ -28,6 +28,23 @@ Most people assume NAT is a firewall - nothing outside can reach in unless they ```console $ netdiff audit 192.168.1.0/24 +audit 12: 192.168.1.0/24 - 1 critical, 2 high, 1 medium, 1 info + + critical nas.local (192.168.1.23:8080) is reachable from the internet on port 8080 [NEW] + high Telnet on port 23 sends usernames, passwords and every keystroke of the + session in cleartext + high port 8080 asks for a password over unencrypted HTTP + medium the router lets any device on the LAN open its firewall + info 7 open port(s) observed, and not reported as problems + +-v adds the evidence each line rests on, why it matters, how to fix it, +and a command you can run yourself to confirm it. +``` + +A report nobody finishes reading teaches nothing, so depth is something you ask for. `-v` expands every line above into the finding it stands for: + +```console +$ netdiff audit 192.168.1.0/24 -v audit 12: 192.168.1.0/24 - 1 critical, 2 high, 1 medium, 1 info CRITICAL @@ -50,8 +67,9 @@ CRITICAL Every finding carries the observation that produced it, what an attacker gains, how to fix it, and **a command you run yourself to confirm it**. You should not have to take a scanner's word for anything. ```bash -netdiff audit 192.168.1.0/24 # full report -netdiff audit 192.168.1.0/24 --json # same thing, machine-readable +netdiff audit 192.168.1.0/24 # a headline per finding +netdiff audit 192.168.1.0/24 -v # each one expanded into its lesson +netdiff audit 192.168.1.0/24 --json # every field, machine-readable netdiff audit 192.168.1.0/24 --no-upnp # skip the router check netdiff audit --explain upnp-control-open # read a lesson without scanning netdiff audit 192.168.1.0/24 --fail-on-finding # exit 1 on critical/high, for cron diff --git a/netdiff/cli.py b/netdiff/cli.py index dd7f3cf..6c9d221 100644 --- a/netdiff/cli.py +++ b/netdiff/cli.py @@ -135,6 +135,19 @@ def print_field(label: str, text: str, wrap: bool = True) -> None: print(f"{pad}{line}") +def print_headline(finding, is_new: bool = False) -> None: + """One finding, one line, severity first. + + The default view. Fifteen findings rendered as fifteen full lessons is a + wall of text people stop reading, and a lesson nobody reads teaches nothing + - so depth is something you ask for with `-v` rather than something you have + to wade through. The severity rides on the line rather than in a heading + above a group, so any single line still says what it is once it has been + copied somewhere else. + """ + print_field(finding.severity, f"{finding.title}{' [NEW]' if is_new else ''}") + + def print_lesson(finding, is_new: bool = False) -> None: """Render one finding as the lesson it is, not as a severity-coloured row.""" print(f" {finding.title}{' [NEW]' if is_new else ''}") @@ -221,6 +234,9 @@ def placeholders(text): print() severity = "" for finding, is_new in annotated: + if not args.verbose: + print_headline(finding, is_new) + continue if finding.severity != severity: severity = finding.severity print(severity.upper()) @@ -228,9 +244,13 @@ def placeholders(text): if not findings: print("nothing to report - no devices answered, or none had open ports") - else: + elif args.verbose: print("Every finding above quotes the observation it rests on. Run the verify") print("command yourself - do not take a scanner's word for anything.") + else: + print() + print("-v adds the evidence each line rests on, why it matters, how to fix it,") + print("and a command you can run yourself to confirm it.") serious = any(f.severity in ("critical", "high") for f in findings) return 1 if args.fail_on_finding and serious else 0 @@ -313,6 +333,12 @@ def build_parser() -> argparse.ArgumentParser: aud.add_argument( "--no-upnp", action="store_true", help="skip the router port-forward check" ) + aud.add_argument( + "-v", + "--verbose", + action="store_true", + help="expand every finding into its evidence, why, fix and verify", + ) aud.add_argument( "--explain", metavar="RULE", help="print the lesson for a rule and exit" ) diff --git a/tests/test_cli_output.py b/tests/test_cli_output.py new file mode 100644 index 0000000..b5b83be --- /dev/null +++ b/tests/test_cli_output.py @@ -0,0 +1,130 @@ +"""How the audit renders, which is the difference between teaching and wallpaper. + +A report nobody finishes reading teaches nothing, so the default view is one +line per finding and the lesson is something you ask for. These tests pin that +ladder: the headline view stays short, `-v` still carries every field, and +neither one drops a finding. + +Nothing here scans. `cmd_audit` is driven with a stubbed `discover` and no UPnP, +so the only thing under test is the rendering. +""" + +import re + +import pytest + +from netdiff import cli +from netdiff.audit import RULES, Finding +from netdiff.scan import Device + +DEVICES = [ + Device(mac="aa:bb:cc:00:00:01", ip="192.168.1.10", ports=(23, 80)), + Device(mac="aa:bb:cc:00:00:02", ip="192.168.1.11", ports=(21, 5900)), +] + +BANNERS = { + ("192.168.1.10", 23): "Welcome to the router", + ("192.168.1.10", 80): "HTTP/1.0 401\r\nWWW-Authenticate: Basic realm=x", + ("192.168.1.11", 21): "220 FTP server ready", + ("192.168.1.11", 5900): "RFB 003.008", +} + + +@pytest.fixture +def audit_output(tmp_path, monkeypatch, capsys): + """Run `netdiff audit` against canned observations, return its stdout.""" + monkeypatch.setattr(cli, "discover", lambda *a, **k: DEVICES) + monkeypatch.setattr(cli, "grab_banners", lambda pairs, **k: BANNERS) + monkeypatch.setattr(cli.mdns, "discover", dict) + + def run(*flags): + db = str(tmp_path / "history.db") + code = cli.main(["--db", db, "audit", "192.168.1.0/24", "--no-upnp", *flags]) + assert code == 0 + return capsys.readouterr().out + + return run + + +HEADLINE = re.compile(r"^ (critical|high|medium|info) ") + + +def headlines(out): + """The lines that open a finding, not the ones a long title wrapped onto.""" + return [line for line in out.splitlines() if HEADLINE.match(line)] + + +def test_the_default_view_opens_one_headline_per_finding(audit_output): + assert len(headlines(audit_output())) == 5, "4 plaintext/auth plus the ports note" + + +def test_the_default_view_is_short_enough_to_actually_read(audit_output): + """The flaw being fixed: five findings used to be sixty lines of wallpaper.""" + short, long = audit_output(), audit_output("-v") + assert len(short.splitlines()) < len(long.splitlines()) / 3 + + +def test_the_default_view_names_the_severity_of_every_finding(audit_output): + found = {HEADLINE.match(line).group(1) for line in headlines(audit_output())} + assert found == {"high", "info"} + + +def test_verbose_carries_every_field_of_every_finding(audit_output): + out = audit_output("-v") + for field in ("evidence", "why", "fix", "verify"): + assert out.count(f" {field:<9}") == 5, f"{field} missing from a finding" + + +def test_both_views_report_the_same_findings(audit_output): + """Brevity is allowed to drop detail. It is not allowed to drop a finding.""" + short, long = audit_output(), audit_output("-v") + for title in ("Telnet on port 23", "FTP on port 21", "VNC on port 5900"): + assert title in short and title in long + # Same counts either way; only the scan id differs between the two runs. + assert short.splitlines()[0].endswith("4 high, 1 info") + assert long.splitlines()[0].endswith("4 high, 1 info") + + +def test_the_default_view_says_how_to_get_the_lesson(audit_output): + """Otherwise the teaching layer is there and nobody ever finds it.""" + assert "-v" in audit_output().splitlines()[-2] + + +def test_verbose_does_not_advertise_itself(audit_output): + out = audit_output("-v") + assert "-v adds" not in out + assert "do not take a scanner's word for anything" in out + + +def test_new_findings_are_marked_in_both_views(audit_output): + audit_output() # first audit: nothing is new, everything would be + assert "[NEW]" not in audit_output() + assert "[NEW]" not in audit_output("-v") + + +def test_explain_is_unaffected_by_the_verbosity_flag(capsys): + assert cli.main(["audit", "--explain", "ssh-v1"]) == 0 + plain = capsys.readouterr().out + assert cli.main(["audit", "-v", "--explain", "ssh-v1"]) == 0 + assert capsys.readouterr().out == plain + + +def test_an_unknown_rule_lists_the_known_ones(capsys): + assert cli.main(["audit", "--explain", "no-such-rule"]) == 2 + err = capsys.readouterr().err + assert all(rule in err for rule in RULES) + + +def test_a_headline_survives_a_finding_with_no_title_padding(): + """`print_field` is shared with the lesson view; severity is its label here.""" + finding = Finding( + rule="r", + severity="critical", + device="d", + title="x" * 200, + evidence="e", + why="w", + fix="f", + verify="v", + ) + cli.print_headline(finding, is_new=True)