feat(audit): a report nobody finishes reading teaches nothing

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

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,16 +234,23 @@ 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())
         print_lesson(finding, is_new)
 
     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"
     )

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)