From 71327371c33222dd3cc5e88d0fb14fc301d41488 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 6 Apr 2026 12:25:51 +0000 Subject: [PATCH] Audit fixes: security, bugs, code quality improvements MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix HTTP → HTTPS for ip-api.com in macOS and Windows to prevent man-in-the-middle interception of public IP/ISP lookups - Fix falsy-check bug in headless_config: use `is not None` for poll and threshold so explicit value 0 is honoured - Add input validation in interactive_setup: replace bare int() casts with _prompt_int() which re-prompts on invalid input - Add HTML escaping (html.escape) for ISP name, public IP, target names, and drop data injected into HTML reports - Move import datetime from inside functions to module level in metrics.py - Use try/finally for socket cleanup in network.py get_local_ip - Rename single-letter variables (l, a, j) in get_health_score to loss_pct, avg_lat, jitter_ms for readability - Fix confusingly-named variables now_s/end_s → start_s/end_s in macos/lib/logging.sh log_drop and log_threshold_breach Agent-Logs-Url: https://github.com/nexuspcs/ConnectivityMonitor/sessions/36342ce1-c124-412a-9d81-904bbde9071a Co-authored-by: nexuspcs <69493073+nexuspcs@users.noreply.github.com> --- macos/lib/logging.sh | 16 +++++------ macos/lib/network.sh | 2 +- python/connectivity_monitor/config.py | 32 +++++++++++++--------- python/connectivity_monitor/html_report.py | 22 +++++++++++---- python/connectivity_monitor/metrics.py | 24 ++++++++-------- python/connectivity_monitor/network.py | 9 +++--- windows/ConnectivityDropMonitor.ps1 | 2 +- 7 files changed, 62 insertions(+), 45 deletions(-) diff --git a/macos/lib/logging.sh b/macos/lib/logging.sh index 4abb92c..ef5411d 100755 --- a/macos/lib/logging.sh +++ b/macos/lib/logging.sh @@ -47,10 +47,10 @@ log_ping() { log_drop() { local start="$1" end="$2" target="$3" diagnosis="$4" - local now_s end_s dur - now_s=$(date -j -f "%Y-%m-%d %H:%M:%S" "$end" +%s 2>/dev/null || date +%s) - end_s=$(date -j -f "%Y-%m-%d %H:%M:%S" "$start" +%s 2>/dev/null || date +%s) - dur=$(awk "BEGIN {printf \"%.2f\", $now_s - $end_s}") + local start_s end_s dur + start_s=$(date -j -f "%Y-%m-%d %H:%M:%S" "$start" +%s 2>/dev/null || date +%s) + end_s=$(date -j -f "%Y-%m-%d %H:%M:%S" "$end" +%s 2>/dev/null || date +%s) + dur=$(awk "BEGIN {printf \"%.2f\", $end_s - $start_s}") if awk "BEGIN {exit !($dur < 0)}"; then dur="0"; fi drop_starts+=("$start") @@ -67,10 +67,10 @@ log_drop() { log_threshold_breach() { local start="$1" end="$2" avg_lat="$3" - local now_s end_s dur - now_s=$(date -j -f "%Y-%m-%d %H:%M:%S" "$end" +%s 2>/dev/null || date +%s) - end_s=$(date -j -f "%Y-%m-%d %H:%M:%S" "$start" +%s 2>/dev/null || date +%s) - dur=$(awk "BEGIN {printf \"%.2f\", $now_s - $end_s}") + local start_s end_s dur + start_s=$(date -j -f "%Y-%m-%d %H:%M:%S" "$start" +%s 2>/dev/null || date +%s) + end_s=$(date -j -f "%Y-%m-%d %H:%M:%S" "$end" +%s 2>/dev/null || date +%s) + dur=$(awk "BEGIN {printf \"%.2f\", $end_s - $start_s}") if awk "BEGIN {exit !($dur < 0)}"; then dur="0"; fi breach_starts+=("$start") diff --git a/macos/lib/network.sh b/macos/lib/network.sh index 99e2872..4b1a19c 100755 --- a/macos/lib/network.sh +++ b/macos/lib/network.sh @@ -78,7 +78,7 @@ get_local_ip() { # ================================================================ detect_public_ip() { local resp - resp=$(curl -s --connect-timeout 5 --max-time 8 "http://ip-api.com/json" 2>/dev/null) + resp=$(curl -s --connect-timeout 5 --max-time 8 "https://ip-api.com/json" 2>/dev/null) if [[ -n "$resp" ]]; then public_ip=$(echo "$resp" | python3 -c "import sys,json; d=json.load(sys.stdin); print(d.get('query','N/A'))" 2>/dev/null) isp_name=$(echo "$resp" | python3 -c "import sys,json; d=json.load(sys.stdin); print(d.get('isp','N/A'))" 2>/dev/null) diff --git a/python/connectivity_monitor/config.py b/python/connectivity_monitor/config.py index c6b5aed..54a9cbc 100644 --- a/python/connectivity_monitor/config.py +++ b/python/connectivity_monitor/config.py @@ -58,19 +58,30 @@ def save_config(cfg): def prompt_default(prompt, default): - """Prompt user with a default value.""" + """Prompt user with a default value. Returns the user's input or the default.""" val = input("{} [{}]: ".format(prompt, default)).strip() return val if val else str(default) def prompt_yes_no(prompt, default="Y"): - """Prompt user for yes/no.""" + """Prompt user for yes/no. Returns True for yes, False for no.""" val = input("{} [{}]: ".format(prompt, default)).strip() if not val: val = default return val.upper().startswith("Y") +def _prompt_int(prompt, default): + """Prompt user for an integer, re-prompting on invalid input.""" + while True: + raw = input("{} [{}]: ".format(prompt, default)).strip() + val = raw if raw else str(default) + try: + return int(val) + except ValueError: + print(" Invalid value '{}' — please enter a whole number.".format(val)) + + def interactive_setup(saved_cfg=None): """Run interactive configuration and return config dict.""" print() @@ -97,24 +108,19 @@ def interactive_setup(saved_cfg=None): return cfg cfg = dict(DEFAULTS) - poll = prompt_default(" Poll interval (seconds)", cfg["poll"]) - cfg["poll"] = int(poll) - - threshold = prompt_default(" Failure threshold for drop", cfg["threshold"]) - cfg["threshold"] = int(threshold) + cfg["poll"] = _prompt_int(" Poll interval (seconds)", cfg["poll"]) + cfg["threshold"] = _prompt_int(" Failure threshold for drop", cfg["threshold"]) targets = prompt_default(" Ping targets (comma-sep)", cfg["targets"]) cfg["targets"] = targets - lat_warn = prompt_default(" Latency warning (ms)", cfg["lat_warn"]) - cfg["lat_warn"] = int(lat_warn) + cfg["lat_warn"] = _prompt_int(" Latency warning (ms)", cfg["lat_warn"]) cfg["enable_dns"] = prompt_yes_no(" Enable DNS health check?", "Y") if cfg["enable_dns"]: cfg["dns_target"] = prompt_default(" DNS test hostname", cfg["dns_target"]) - web_port = prompt_default(" Web dashboard port", cfg["web_port"]) - cfg["web_port"] = int(web_port) + cfg["web_port"] = _prompt_int(" Web dashboard port", cfg["web_port"]) save_config(cfg) print(" Config saved to {}".format(get_config_path())) @@ -130,9 +136,9 @@ def headless_config(args): if args.targets: cfg["targets"] = args.targets - if args.poll: + if args.poll is not None: cfg["poll"] = args.poll - if args.threshold: + if args.threshold is not None: cfg["threshold"] = args.threshold if args.web_port is not None: cfg["web_port"] = args.web_port diff --git a/python/connectivity_monitor/html_report.py b/python/connectivity_monitor/html_report.py index ecdebad..251dc50 100644 --- a/python/connectivity_monitor/html_report.py +++ b/python/connectivity_monitor/html_report.py @@ -4,6 +4,7 @@ """ import datetime +import html import os from . import metrics @@ -141,7 +142,11 @@ def generate_html_report(state, report_file): report_gen_str = datetime.datetime.now().strftime("%Y-%m-%d %H:%M:%S") total_lost = state.total_pings - state.total_success - html = _build_html( + # Escape external/user-supplied strings before injecting into HTML + safe_isp = html.escape(str(state.isp_name)) + safe_public_ip = html.escape(str(state.public_ip)) + + html_out = _build_html( labels_js=labels_js, data_js=data_js, gw_js=gw_js, hist_js=hist_js, target_rows=target_rows, drops_table=drops_table, breach_table=breach_table, heatmap_html=heatmap_html, report_date=report_date, @@ -152,14 +157,14 @@ def generate_html_report(state, report_file): total_downtime=total_downtime, baseline_text=baseline_text, health_css=health_css, uptime_css=uptime_css, avg_css=avg_css, loss_css=loss_css, jitter_css=jitter_css, drops_css=drops_css, - drops_count=len(state.drops), isp=state.isp_name, public_ip=state.public_ip, + drops_count=len(state.drops), isp=safe_isp, public_ip=safe_public_ip, ) d = os.path.dirname(report_file) if d: os.makedirs(d, exist_ok=True) with open(report_file, "w", encoding="utf-8") as f: - f.write(html) + f.write(html_out) def _score_color(value, thresholds, default): @@ -190,7 +195,7 @@ def _build_target_rows(state): rows += ( "{}{}{}{}%" "{}ms{}ms{}ms\n" - ).format(cls, t, info["sent"], info["ok"], t_loss, t_avg, t_min, t_max) + ).format(cls, html.escape(t), info["sent"], info["ok"], t_loss, t_avg, t_min, t_max) return rows @@ -206,7 +211,14 @@ def _build_drop_rows(state): cls = ' class="warn"' rows += ( "{}{}{}s{}{}\n" - ).format(cls, d["Start"], d["End"], d["Duration"], d["Target"], d["Diagnosis"]) + ).format( + cls, + html.escape(str(d["Start"])), + html.escape(str(d["End"])), + d["Duration"], + html.escape(str(d["Target"])), + html.escape(str(d["Diagnosis"])), + ) return rows diff --git a/python/connectivity_monitor/metrics.py b/python/connectivity_monitor/metrics.py index 0ba42ca..27793ba 100644 --- a/python/connectivity_monitor/metrics.py +++ b/python/connectivity_monitor/metrics.py @@ -1,5 +1,6 @@ """Metrics engine — loss, avg, min, max, percentile, jitter, health, trend.""" +import datetime import math @@ -72,23 +73,23 @@ def uptime(state): def get_health_score(state): """Health score 0-100 with letter grade. Returns {score, grade, color}.""" - l = loss(state) - a = avg(state) - j = jitter(state) + loss_pct = loss(state) + avg_lat = avg(state) + jitter_ms = jitter(state) score = 100 - score -= l * 3 - if a > 100: + score -= loss_pct * 3 + if avg_lat > 100: score -= 20 - elif a > 50: + elif avg_lat > 50: score -= 10 - elif a > 30: + elif avg_lat > 30: score -= 5 - if j > 30: + if jitter_ms > 30: score -= 15 - elif j > 15: + elif jitter_ms > 15: score -= 8 - elif j > 5: + elif jitter_ms > 5: score -= 3 score = max(0, min(100, round(score))) @@ -171,7 +172,6 @@ def get_network_weather(state): def update_history(state, lat, target): """Add a ping result to history and update per-target stats.""" - import datetime entry = { "time": datetime.datetime.now(), "latency": lat, @@ -205,7 +205,6 @@ def update_history(state, lat, target): def update_gw_history(state, gw_lat): """Add a gateway ping result to gateway history.""" - import datetime entry = {"time": datetime.datetime.now(), "latency": gw_lat} state.gw_history.append(entry) while len(state.gw_history) > 200: @@ -216,7 +215,6 @@ def update_hourly_data(state, lat): """Record latency data for hourly heatmap.""" if lat is None: return - import datetime hour = datetime.datetime.now().hour if hour not in state.hourly_data: state.hourly_data[hour] = [] diff --git a/python/connectivity_monitor/network.py b/python/connectivity_monitor/network.py index 2395455..d635dd8 100644 --- a/python/connectivity_monitor/network.py +++ b/python/connectivity_monitor/network.py @@ -91,10 +91,11 @@ def get_local_ip(): """Get local IP address of the machine.""" try: s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM) - s.connect(("8.8.8.8", 80)) - ip = s.getsockname()[0] - s.close() - return ip + try: + s.connect(("8.8.8.8", 80)) + return s.getsockname()[0] + finally: + s.close() except Exception: return "N/A" diff --git a/windows/ConnectivityDropMonitor.ps1 b/windows/ConnectivityDropMonitor.ps1 index 8c9efdd..7ebffa2 100644 --- a/windows/ConnectivityDropMonitor.ps1 +++ b/windows/ConnectivityDropMonitor.ps1 @@ -195,7 +195,7 @@ function GetLocalIP($alias) { function DetectPublicIP { try { - $resp = Invoke-RestMethod -Uri "http://ip-api.com/json" -TimeoutSec 5 -ErrorAction Stop + $resp = Invoke-RestMethod -Uri "https://ip-api.com/json" -TimeoutSec 5 -ErrorAction Stop $script:publicIP = $resp.query $script:ispName = $resp.isp }