Validates that the user-provided `--speedtest-server` parameter does not start with a hyphen (`-`) before passing it to `asyncio.create_subprocess_exec` in `tier_ookla`. This prevents option injection vulnerabilities where a malicious user or external script could supply flags (e.g. `--hacked`) that the `speedtest` binary would process. Also adds a test case to prevent regression.
Co-authored-by: Horizontal42 <75383519+Horizontal42@users.noreply.github.com>
- Rename and consolidate `test_speed_math.py`, `test_speed_fastcom.py`, and `test_speed_cloudflare.py` into a single, comprehensive `tests/test_speed.py`.
- Add test coverage for `tier_ndt7` testing happy paths, network errors, and websocket exceptions via mocks.
- Ensure 100% pass rate with no regressions.
Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
* test: add tests for ValueError handling in ip_geo.py
This commit adds tests to ensure `ValueError` exception paths
in `ip_geo.py` are properly covered, specifically for `_ip_version`
and the ripestat HTTP JSON parsing fallback.
Coverage for `ip_geo.py` is now at 100%.
Co-authored-by: Horizontal42 <75383519+Horizontal42@users.noreply.github.com>
* drop redundant pytest-cov dep addition
---------
Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
* 🧪 Add tests for primary_interface_ip
Added unit tests to fully cover `primary_interface_ip` in `netinfo.py`, handling IPv4, IPv6 and error conditions using mock sockets.
Coverage for `primary_interface_ip` is now 100%.
Co-authored-by: Horizontal42 <75383519+Horizontal42@users.noreply.github.com>
* drop redundant pytest-cov dep addition
---------
Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
* Add initial test suite for cli.py
Co-authored-by: Horizontal42 <75383519+Horizontal42@users.noreply.github.com>
* drop redundant pytest-cov dep addition
---------
Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
This addresses the missing coverage for local network parameter discovery. Tests cover happy paths, CGNAT scenarios, and cases with no network components. The additions improve reliability and prevent regression of core host capability detection.
Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
Added tests for the `detect_capabilities` function in `src/netsleuth/netinfo.py` to cover both Linux and Windows OS flows, improving test coverage and reliability.
Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
* 🧪 Add tests for check_fresh and improve coverage in reputation.py
* Extract `check_fresh` to a module-level function for testability.
* Add comprehensive tests for `check_fresh` covering missing, fresh, and stale cache files.
* Add a missing test for `reverse_ip` with IPv6 to hit 100% coverage on `reputation.py`.
Co-authored-by: Horizontal42 <75383519+Horizontal42@users.noreply.github.com>
* drop redundant pytest-cov dep addition
---------
Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
* 🧪 Add tests for render_dashboard in watch.py
Added comprehensive tests in `tests/test_watch.py` to cover the `render_dashboard` function from `src/netsleuth/watch.py`.
Included tests for:
- Empty sessions
- Sessions populated with valid ping results
- Error states with missing data (e.g., 100% loss) ensuring dashes ("—") render properly
- Dashboard captions for speed test results
Improved test coverage for `netsleuth.watch`.
Co-authored-by: Horizontal42 <75383519+Horizontal42@users.noreply.github.com>
* drop redundant pytest-cov dep addition
---------
Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
* Remove hardcoded API token for Fast.com
Co-authored-by: Horizontal42 <75383519+Horizontal42@users.noreply.github.com>
* keep fastcom probe working by default, just make the token overridable
---------
Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
This commit adds a new test file, `tests/test_cli.py`, containing
unit tests for the `_os_timezone` function in `netsleuth.cli`.
The tests cover:
- Happy paths (TZ variable, /etc/localtime symlink, /etc/timezone file)
- The edge case of an `OSError` exception being thrown when reading the
/etc/timezone file.
Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
Filenames only carry a time-of-day now (report_<asn>_HH-MM-SSZ)
since the date lives in the directory path; history lookups walk
the tree with rglob and sort by full path instead of basename,
since the basename alone no longer encodes recency across days.
The only feature in this batch adding a real dependency, approved up front
and landed the same way tcptrace/ndt7 already do: an optional extra
(uv sync --extra quic), imported lazily inside measure_quic() so a missing
install degrades to a clear skip warning instead of a crash.
Verified the actually-installed aioquic 1.3.0 API directly rather than
assuming. connect() takes local_port but no local_host/source-address
parameter at all, so --quic structurally cannot honor --interface -
_quic_section() says so explicitly rather than silently measuring the
OS-default path under a report claiming a forced adapter. Its context
manager doesn't yield until the handshake finishes, so unlike the TLS RTT
probe's two-connection subtraction trick, handshake time here is timed
directly. HandshakeCompleted's fields matched what was expected; dropped
a planned quic_version field since aioquic has no clean public API for
it rather than reach into a private attribute.
quic_verdict(quic_ok, tcp_ok) is the actual payoff: QUIC failing while
plain TCP to the same host succeeds means the network specifically
drops or throttles UDP/443, silently degrading Chrome/YouTube's HTTP/3
preference in a way no TCP-based diagnostic would ever catch. Both
failing is ordinary unreachability already covered elsewhere, so it
stays silent rather than duplicating that finding.
Opt-in --quic, excluded from --full - an optionally-installed extra must
never become a silent requirement. Added the quic extra to the release
workflow's wheel-install verification.
Live-tested against real cloudflare.com/google.com: real handshakes,
ALPN h3 negotiated, correct end-to-end timing.
This completes the 16-feature extension batch from
plans/zesty-zooming-wilkinson.md (gitignored, local planning doc).
Stayed unprivileged on both platforms deliberately. Windows: echo_once()
gained a df: bool param setting IP_FLAG_DF on the IP_OPTION_INFORMATION
struct's Flags field (present but unused until now), and classify_status()
learned IP_PACKET_TOO_BIG (11009) - the same unprivileged IcmpSendEcho2
API the rest of the ICMP tier already uses, no new capability needed.
Unix deliberately does not hand-roll raw ICMP echo requests - unprivileged
SOCK_DGRAM/IPPROTO_ICMP checksum and identifier handling varies enough
across kernel versions that shipping it without a Linux/macOS box to
verify on felt like the wrong risk to take. Shells out to system
ping -M do -s <size> (Linux) / ping -D -s <size> (macOS/BSD) instead,
the same subprocess-and-parse pattern traceroute.py's system tiers
already use. Matches "frag"/"too long" loosely in the output rather than
an exact phrase, since a locally-cached-PMTU failure and a genuine
on-path ICMP response word themselves differently across ping
implementations - same "parse by shape" principle traceparse.py already
established.
classify_pmtu() distinguishes a path that's just narrower (a frag-needed
signal was seen - PMTUD is working, normal behind a VPN) from an actual
blackhole (large packets vanish with no ICMP reply at all - the "SSH
connects then hangs" symptom). path.pmtu_below_iface_mtu flags separately
when the path MTU is narrower than the local NIC's own MTU, which is
exactly what makes this class of problem invisible locally.
Opt-in --pmtud, excluded from --full. Live-verified on Windows against
real cloudflare.com/google.com - found the full 1500-byte MTU in 11
probes each, matching the expected binary-search depth, correct probe
history in the JSON output.
Part of plans/zesty-zooming-wilkinson.md (gitignored).
Scoped down from the original plan: traceroute and --path-diversity's v6
legs are deferred as a documented follow-up, since extending the trace
cascade to a second address family is real additional work that doesn't
share the trick that made latency cheap. Latency alone captures the core
value (a tunneled or poorly-peered IPv6 path is otherwise invisible to
tools that only report whichever stack the OS picked) at a fraction of
the risk.
Turns out no family-detection code was needed at all. icmp_win's
ping_samples_win() calls the IPv4-only socket.gethostbyname(), which
raises on a v6 literal - already caught by ping_host()'s existing bare
except-fallback to TCP, which handles v6 fine via asyncio.open_connection.
icmplib handles v6 natively on Linux/macOS. So v6 hosts ride the existing
ping_fanout() completely unmodified, tagged with a "<label>-v6" PingResult
label instead of a parallel report section - exporter, metrics.py and
compare.py needed zero changes since they already key everything off
label.
New probing.reference_hosts_v6/probing.ipv6 (auto/on/off) config,
--ipv6/--no-ipv6 override, interpret.dual_family_findings() for
net.ipv6_unreachable and latency.v6_much_slower.
Live-tested with --ipv6 forced on a v6-unreachable host: every row fell
back to method=tcp with 100% loss as expected. Caught and fixed a wording
bug in the same pass - the unreachable finding said "on this dual-stack
connection" unconditionally, which is wrong when --ipv6 forces the check
on a host that isn't actually dual-stack.
Part of plans/zesty-zooming-wilkinson.md (gitignored).
Shipped half of the original design on purpose: generalizing BSD's alt:
same-TTL annotation across backends is impossible - mtr's JSON aggregates
cycles and discards per-cycle next-hop variance, icmp_win/icmplib send one
probe per TTL by construction, and the Linux system-traceroute parser
doesn't model multi-IP hops even though the text can contain them. A
finding that only ever fires on macOS/Linux-system-binary would be worse
than not having it.
New probes/ecmp.py::detect_ecmp() takes N independently-run TraceResults
for the same target and diffs which IP showed up at each TTL - pure and
backend-agnostic, since it never touches capability detection. Flags
path.ecmp (info, multiple next hops exist - normal on a load-balanced
network) and path.ecmp_asymmetric (warn, branches differ by >20ms - the
diagnostic case, since one degraded branch of a load-balanced pair causes
intermittent slowness a single traceroute would never catch).
--ecmp is opt-in and deliberately excluded from --full: it multiplies
traceroute traffic against the same targets by ecmp.runs (capped at 5)
across ecmp.max_targets (capped at 3), sequential and sharing one
semaphore, only after the main fan-out has finished.
Live-tested against 2 real targets x 3 runs each - ran cleanly, correctly
reported no divergence on this network.
Part of plans/zesty-zooming-wilkinson.md (gitignored).
watch.py never imported interpret and had no notion of a verdict at all -
summarize_cycle() now runs the same latency_findings/speed_findings/
overall_verdict pipeline the one-shot report uses, so every tick carries
status/score/finding_ids and the dashboard caption shows a verdict line.
Real improvement on its own, independent of whether a webhook exists.
New alerting.py carries the whole notification contract as pure functions:
should_fire() only fires on an actual transition, into a state the user
opted into via watch.webhook_on, debounced by
watch.webhook_min_interval_seconds (floored at 30 in a validator) so a
flapping link can't spam an endpoint every tick. Recovery is its own
opt-in "recovered" token, separate from configuring "crit", since wanting
outage alerts doesn't imply wanting a second ping when they clear.
build_payload() is flat JSON, not shaped for Slack/Discord - that's the
receiver's problem. post_webhook() swallows every failure and returns a
bool; a dead endpoint must never interrupt the loop watching it.
New --webhook URL flag overrides watch.webhook_url per run.
Verified the tick sequence end-to-end outside the Rich Live loop -
sending real OS signals to test --watch's Ctrl-C handling in this shell
wasn't reliable, so exercised should_fire/build_payload/render_dashboard/
to_report in the same order run_watch calls them.
Part of plans/zesty-zooming-wilkinson.md (gitignored).
New metrics.py: collect_metrics() does the per-section extraction a flat
dict walk can't (latency needs label/host/method, TLS label/host, path hops
target/ttl/ip/asn), producing a flat Metric list that render_prometheus()
and render_csv() each format in about 15 lines. Simpler than treating them
as two separate renderers. FORMAT_EXTENSIONS gained "prom"/"csv" as the
single wiring point both parse_formats() and Output._normalize_formats()
already key off of.
New history.py finds a previous JSON report for the same ASN by reusing
the exact filename convention exporter.report_filename() already writes,
so the lookup key can't drift from what's on disk. Wired into diagnose()
before write_report() - a compact vs-previous-run summary
(compare.py::render_diff_brief(), new) prints automatically whenever this
run writes JSON and an earlier one exists for the same network; silent on
every other case (first run, prior run wrote no JSON prints one note
instead of guessing). New --no-auto-diff escapes it per run,
history.auto_diff in config disables it entirely.
New trend.py aggregates N saved JSON reports into per-label sparkline
series via --trend N - read-only, and it doesn't touch the network to find
"this network's" history either: history.latest_key() just reads it off
the newest report filename already on disk.
Growing FORMAT_EXTENSIONS to 5 entries broke three pre-existing tests that
hardcoded the old 3-format set; fixed in this commit.
Live-tested: prom/csv artifacts render correctly, auto-diff fired on a
second run, --trend 3 produced readable output, --compare unaffected.
Part of plans/zesty-zooming-wilkinson.md (gitignored).
tls_context() now defaults to real certificate verification instead of the
tool's original always-CERT_NONE posture; measure_tls() only falls back to
unverified on ssl.SSLCertVerificationError, retiming the retry since the
failed attempt's duration isn't representative. This is also the only way
to get subject/issuer/notAfter at all - getpeercert() returns {} for a
chain that never validated.
Every TLS result now carries cert_sha256 (SHA-256 of the full DER cert, not
the SPKI - that would need the cryptography package, a dependency this
doesn't need), cert_issuer, cert_days_remaining, and the long-dead
cert_verified field finally gets written. New tls.pinned_fingerprints
config (host -> expected hash, capped and hex-validated) drives three new
findings: a pin mismatch is crit, an unpinned validation failure is warn,
an expiring cert is warn.
Live-tested with --tls against real Cloudflare/Google endpoints - verified
handshake, correct issuer and expiry both times.
Part of plans/zesty-zooming-wilkinson.md (gitignored).
New Phase 0 in diagnose(): three generate_204-style probes run before every
other module, using detect-and-tag rather than short-circuit — a portal is
rarely total (DNS/ICMP often pass while only HTTP is intercepted), and an
aborted run would look identical to a dead link. On detection every later
phase still runs, tagged with a crit finding and a banner at the top of the
report.
Caught live before it shipped: detectportal.firefox.com/success.txt (one of
the original three default check URLs) returns 200 "success" on a genuinely
clean connection, which the generate_204-shaped classifier misread as a
portal redirect. Swapped for a second real generate_204 endpoint instead of
teaching the classifier a second success shape.
Also redesigned the speedtest server geo-sanity check after checking the
bundled Ookla CLI's actual JSON output: its server object never carries
lat/lon (documented schema, not a version gap), so speed.server_other_country
compares server_country against the client's own country instead of a
haversine distance that no tier can actually supply.
Part of plans/zesty-zooming-wilkinson.md (gitignored).
TraceHop.asn/as_name existed in the model since the start but nothing ever
populated them. New probes/hop_asn.py fires one Team Cymru DNS query pair
per unique hop IP (in-run deduped, no rate limit unlike the cached_json()
providers) after the latency/path/TLS fan-out, plus a PTR lookup for hops
whose reverse_dns is still empty because the backend that produced them
doesn't resolve hostnames itself (icmp_win/icmplib/mtr, unlike system
traceroute text).
New path.detour_country finding flags a route that spends two or more
consecutive hops in a country that's neither the client's nor the
destination's. Wires up providers.cymru_asn_zone, which was configured but
referenced nowhere, and adds cymru_origin6_zone for the IPv6 branch Phase 8
will need.
Part of plans/zesty-zooming-wilkinson.md (gitignored).
- net.cgnat: three evidence paths (local IP, gateway, traceroute hop) for
RFC 6598 100.64.0.0/10, since the local address alone misses the common
home-router case
- net.nat64: RFC 7050 ipv4only.arpa AAAA probe (new probes/nat64.py) instead
of checking the client's own address against 64:ff9b::/96, which never
fires since that prefix is a synthesis target, not something the client's
egress address falls into
- path.first_hop_loss/slow/unexpected: path_findings() takes an optional
LocalNet/Thresholds so loss or latency at hop 1 is attributed to the
user's own router/Wi-Fi instead of blending into the ISP-facing mid-path
logic
All four findings are always-on additions to existing report sections, no
new CLI flags. Part of the larger extension batch tracked in
plans/zesty-zooming-wilkinson.md (gitignored, local planning doc).
