Triage SonarCloud security hotspots on PR #182

S1313 hardcoded IPs:
- test_rest_auth.py: switch test fixture IPs from arbitrary
  literals (1.1.1.1, 2.2.2.2, …) to RFC 5737 documentation ranges
  (192.0.2.0/24 = TEST-NET-1) via _TEST_IP_A..F module constants.
  These are reserved for documentation and never appear on the
  public internet, which is exactly what the rule is meant to
  encourage.
- lan_discovery.py: NOSONAR on the 8.8.8.8 anycast probe with
  rationale (UDP no-traffic interface-discovery trick — the literal
  is the well-known address being probed for; parameterising it
  would only obscure intent).

S5332 cleartext HTTP:
- admin_client._http_request: NOSONAR — this is a scheme allow-
  list check, not URL emission.
- rest_server.base_url: NOSONAR with deployment note (loopback
  bind + operator-managed TLS reverse proxy is the documented
  shape).
- admin_console_tab placeholder text, test_admin_client/_url +
  validator-empty literals, test_usb_browser_tab fixtures: NOSONAR
  with reasons (placeholder UI, validator-only literals, loopback
  test server).

Web:S5725 SRI on swagger.html: per-tag NOSONAR with rationale —
already handled in the JS-smells commit; included here for
completeness.

S107 webrtc_host.__init__: NOSONAR with rationale — public
constructor; the discrete kwargs are clearer at the call sites
(GUI panel + multi_viewer) than a callbacks-bag dataclass would
be, and breaking the kwarg names would force every operator's
external embedding to change.

Author: JE-Chen

je_auto_control/gui/admin_console_tab.py

@@ -52,7 +52,11 @@ def __init__(self, parent: Optional[QWidget] = None) -> None:
         self._client = default_admin_console()
         self._label_input = QLineEdit()
         self._url_input = QLineEdit()
-        self._url_input.setPlaceholderText("http://host:9939")
+        # Placeholder text only — the operator types the real URL.
+        # Default scheme is http to match the bundled local server;
+        # production deployments should put TLS in front via a reverse
+        # proxy and the operator can paste an https://… URL here.
+        self._url_input.setPlaceholderText("http://host:9939")  # NOSONAR python:S5332
         self._token_input = QLineEdit()
         self._token_input.setEchoMode(QLineEdit.Password)
         self._table = QTableWidget(0, 5)

je_auto_control/utils/admin/admin_client.py

@@ -169,6 +169,10 @@ def _http_request(self, host: AdminHost, path: str, *,
                       method: str, body: Optional[Dict[str, Any]],
                       ) -> Dict[str, Any]:
         url = f"{host.base_url}{path}"
+        # NOSONAR python:S5332 — this is a scheme allowlist check, not
+        # a URL emission. Both http:// and https:// must be accepted
+        # because the operator points the admin console at whatever
+        # the host is actually listening on.
         if not url.startswith(("http://", "https://")):
             raise ValueError(f"unsupported URL scheme: {url}")
         headers = {"Authorization": f"Bearer {host.token}"}

je_auto_control/utils/remote_desktop/lan_discovery.py

@@ -41,7 +41,13 @@ def _local_ip() -> str:
     try:
         sock = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
         try:
-            sock.connect(("8.8.8.8", 80))  # nosec B113  # reason: UDP no-traffic probe; no actual packet sent
+            # Connect-to-public-IP trick to discover the local interface
+            # the kernel would pick for outbound traffic; UDP, so no
+            # packet is sent and 8.8.8.8 (Google DNS) just stands in for
+            # "any reachable public IP". NOSONAR python:S1313 because
+            # the literal IS the well-known anycast probe address —
+            # parameterising it would obscure intent.
+            sock.connect(("8.8.8.8", 80))  # nosec B113  # NOSONAR python:S1313
             return sock.getsockname()[0]
         finally:
             sock.close()

je_auto_control/utils/remote_desktop/webrtc_host.py

@@ -55,7 +55,7 @@ class WebRTCDesktopHost:
     "one person controls my machine" workflow and keeps the GUI simple.
     """
 
-    def __init__(self, *, token: str,
+    def __init__(self, *, token: str,  # NOSONAR python:S107  # public constructor; callbacks/permissions are kept as discrete kwargs to keep the call site readable at the GUI layer (see gui/remote_desktop/webrtc_panel.py + utils/remote_desktop/multi_viewer.py)
                  config: Optional[WebRTCConfig] = None,
                  on_state_change: Optional[StateCallback] = None,
                  on_authenticated: Optional[Callable[[], None]] = None,

je_auto_control/utils/rest_api/rest_server.py

@@ -346,6 +346,11 @@ def is_running(self) -> bool:
 
     @property
     def base_url(self) -> str:
+        # NOSONAR python:S5332 — the embedded HTTP server binds to
+        # localhost and is meant to sit behind an operator-managed
+        # reverse proxy that terminates TLS. Returning http:// here
+        # reflects what's actually listening; admins compose the
+        # public https:// URL upstream.
         host, port = self._address
         return f"http://{host}:{port}"
 

test/unit_test/headless/test_admin_client.py

@@ -26,6 +26,9 @@ def client(tmp_path):
 
 
 def _url(server):
+    # NOSONAR python:S5332 — tests run against a stub localhost HTTP
+    # server fixture; TLS would force every test to mint certs and
+    # offers no real coverage benefit here.
     host, port = server.address
     return f"http://{host}:{port}"
 
@@ -39,6 +42,9 @@ def test_add_host_round_trip(client, two_servers):
 
 
 def test_add_host_validates_required_fields(client):
+    # NOSONAR python:S5332 — these literals are placeholder URL strings
+    # passed to a validator that only checks emptiness; no traffic is
+    # ever sent to "http://x".
     with pytest.raises(ValueError):
         client.add_host(label="", base_url="http://x", token="t")
     with pytest.raises(ValueError):

test/unit_test/headless/test_rest_auth.py

@@ -1,9 +1,23 @@
-"""Tests for the REST bearer-token + per-IP rate-limit gate (round 23)."""
+"""Tests for the REST bearer-token + per-IP rate-limit gate (round 23).
+
+Test IPs use the RFC 5737 documentation ranges (192.0.2.0/24 = TEST-NET-1,
+198.51.100.0/24 = TEST-NET-2, 203.0.113.0/24 = TEST-NET-3) so static
+analysis tools that flag hardcoded IPs (Sonar S1313) recognise them as
+intentional test fixtures rather than real-world routable addresses.
+"""
 from je_auto_control.utils.rest_api.rest_auth import (
     RestAuthGate, constant_time_equal, generate_token,
 )
 
 
+_TEST_IP_A = "192.0.2.1"
+_TEST_IP_B = "192.0.2.2"
+_TEST_IP_C = "192.0.2.3"
+_TEST_IP_D = "192.0.2.4"
+_TEST_IP_E = "192.0.2.5"
+_TEST_IP_F = "192.0.2.6"
+
+
 def test_generate_token_is_url_safe_and_unique():
     a = generate_token()
     b = generate_token()
@@ -22,51 +36,51 @@ def test_constant_time_equal_matches():
 
 def test_check_accepts_correct_bearer():
     gate = RestAuthGate(expected_token="real")
-    verdict = gate.check(client_ip="1.1.1.1", header_value="Bearer real")
+    verdict = gate.check(client_ip=_TEST_IP_A, header_value="Bearer real")
     assert verdict == "ok"
 
 
 def test_check_rejects_wrong_token():
     gate = RestAuthGate(expected_token="real")
-    verdict = gate.check(client_ip="1.1.1.1", header_value="Bearer wrong")
+    verdict = gate.check(client_ip=_TEST_IP_A, header_value="Bearer wrong")
     assert verdict == "unauthorized"
 
 
 def test_check_rejects_missing_header():
     gate = RestAuthGate(expected_token="real")
-    assert gate.check(client_ip="1.1.1.1", header_value=None) == "unauthorized"
-    assert gate.check(client_ip="1.1.1.1", header_value="") == "unauthorized"
+    assert gate.check(client_ip=_TEST_IP_A, header_value=None) == "unauthorized"
+    assert gate.check(client_ip=_TEST_IP_A, header_value="") == "unauthorized"
 
 
 def test_check_rejects_non_bearer_scheme():
     gate = RestAuthGate(expected_token="real")
-    verdict = gate.check(client_ip="1.1.1.1", header_value="Basic real")
+    verdict = gate.check(client_ip=_TEST_IP_A, header_value="Basic real")
     assert verdict == "unauthorized"
 
 
 def test_lockout_after_repeated_failures():
     gate = RestAuthGate(expected_token="real")
     for _ in range(8):
-        gate.check(client_ip="2.2.2.2", header_value="Bearer wrong")
-    verdict = gate.check(client_ip="2.2.2.2", header_value="Bearer wrong")
+        gate.check(client_ip=_TEST_IP_B, header_value="Bearer wrong")
+    verdict = gate.check(client_ip=_TEST_IP_B, header_value="Bearer wrong")
     assert verdict in ("locked_out", "rate_limited"), verdict
 
 
 def test_lockout_is_per_ip():
     """A bad client must NOT lock out a different IP."""
     gate = RestAuthGate(expected_token="real")
     for _ in range(20):
-        gate.check(client_ip="3.3.3.3", header_value="Bearer wrong")
+        gate.check(client_ip=_TEST_IP_C, header_value="Bearer wrong")
     # different client should still be evaluated normally
-    verdict = gate.check(client_ip="4.4.4.4", header_value="Bearer real")
+    verdict = gate.check(client_ip=_TEST_IP_D, header_value="Bearer real")
     assert verdict == "ok"
 
 
 def test_rate_limit_kicks_in():
     """Burst is 30 by default — 50 requests in a row should get rate-limited."""
     gate = RestAuthGate(expected_token="real")
     verdicts = [
-        gate.check(client_ip="5.5.5.5", header_value="Bearer real")
+        gate.check(client_ip=_TEST_IP_E, header_value="Bearer real")
         for _ in range(50)
     ]
     assert "rate_limited" in verdicts
@@ -75,10 +89,10 @@ def test_rate_limit_kicks_in():
 def test_successful_auth_resets_failure_counter():
     gate = RestAuthGate(expected_token="real")
     for _ in range(3):
-        gate.check(client_ip="6.6.6.6", header_value="Bearer wrong")
+        gate.check(client_ip=_TEST_IP_F, header_value="Bearer wrong")
     # Successful login clears the failure window.
-    assert gate.check(client_ip="6.6.6.6", header_value="Bearer real") == "ok"
+    assert gate.check(client_ip=_TEST_IP_F, header_value="Bearer real") == "ok"
     # Now a few more failures should not lock out immediately.
     for _ in range(3):
-        verdict = gate.check(client_ip="6.6.6.6", header_value="Bearer wrong")
+        verdict = gate.check(client_ip=_TEST_IP_F, header_value="Bearer wrong")
         assert verdict == "unauthorized"

test/unit_test/headless/test_usb_browser_tab.py

@@ -23,6 +23,9 @@ def rest_server():
 
 
 def test_fetch_returns_list_against_real_server(rest_server):
+    # NOSONAR python:S5332 — the rest_server fixture binds to localhost
+    # without TLS; production deployments terminate TLS at a reverse
+    # proxy. These test URLs never leave the loopback interface.
     host, port = rest_server.address
     devices = fetch_remote_devices(
         base_url=f"http://{host}:{port}", token=rest_server.token,
@@ -42,6 +45,7 @@ def test_fetch_rejects_missing_url():
 
 def test_fetch_propagates_http_error(rest_server):
     """Wrong token surfaces the 401 as a urllib HTTPError."""
+    # NOSONAR python:S5332 — see test_fetch_returns_list_against_real_server.
     host, port = rest_server.address
     with pytest.raises(urllib.error.HTTPError):
         fetch_remote_devices(
@@ -59,6 +63,7 @@ def test_fetch_accepts_url_without_scheme(rest_server):
 
 
 def test_fetch_strips_trailing_slash(rest_server):
+    # NOSONAR python:S5332 — see test_fetch_returns_list_against_real_server.
     host, port = rest_server.address
     devices = fetch_remote_devices(
         base_url=f"http://{host}:{port}/", token=rest_server.token,