Skip to content

Commit 38d238d

Browse files
committed
Tighten Sonar/Codacy markers and clear remaining PR #182 findings
The previous sweep used ``# NOSONAR python:S1234`` style rationales — Sonar's S7632 rule classifies that as malformed because the colon form isn't part of its accepted suppression syntax. Reformat all markers as plain ``# NOSONAR — <reason>`` placed on the violating line, which is the form Sonar actually honours. Also fix a few residuals the first sweep missed: Real fixes (not suppressions): - swagger.html: replace the verbose <!-- … --> rationale block (which AvoidCommentedOutCodeCheck mistook for commented-out code) with proper ``integrity="sha512-…"`` SRI hashes fetched from cdnjs.api/libraries/swagger-ui/5.17.14, closing the three S5725 hotspots properly instead of suppressing them. - mic-worklet.js: collapse process() to a single ``return true`` exit point so S3516 stops firing — same behaviour, no marker needed. - web_viewer/index.html setLanguage: extract _resolveLanguageChoice + _refreshDynamicLabels to push cognitive complexity below 15 (S3776:412 was the only remaining cog hit). - app.js clearChildren: use ``firstChild.remove()`` instead of ``removeChild`` (S7762). - signaling_server validators: route-level ``responses=`` already documents the 400/404 contract; mark the helper raises NOSONAR (S8415 false positive across helper-call boundary). - webrtc_transport.wait_for_ice_gathering: NOSONAR S7483 with rationale (asyncio.timeout requires Python 3.11; we still support 3.10). Suppression-syntax repairs (line-targeted plain ``# NOSONAR`` form): - admin_client.py, usb_browser_tab.py, webrtc_stats.py: TimeoutError-OSError catch tuples (Python 3.10 keeps them distinct). - config_bundle/__main__.py: CLI export path (S2083 by-design). - host_service.py: stub-config token placeholder (S6418). - lan_discovery.py: 8.8.8.8 routing probe literal (S1313). - usb_passthrough_prompt.py: cross-thread ``result`` mutation hidden from Sonar by Q_ARG queued slot (S2583). - admin_client.py / usb_browser_tab.py / rest_server.py: scheme allowlist checks and loopback-bound base_url (S5332 hotspots). - test_admin_client.py / test_usb_browser_tab.py: loopback test fixture URLs (S5332 hotspots). - web_viewer/index.html: serviceWorker .catch() in non-module script (S7785). - mic-worklet.js: TypedArray index access — ``i`` is a numeric loop counter, no user-controlled key path (Codacy/ESLint detect-object-injection ×2; same eslint-disable-next-line markers retained). app.js: rename rationale comment so Codacy/Semgrep's hardcoded- password Semgrep rule recognises the ``nosemgrep:`` directive instead of only seeing ``NOSONAR``.
1 parent 81b841f commit 38d238d

16 files changed

Lines changed: 121 additions & 115 deletions

File tree

je_auto_control/gui/usb_browser_tab.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ def fetch_remote_devices(*, base_url: str,
4848
if not base_url:
4949
raise ValueError("base_url is required")
5050
base = base_url.rstrip("/")
51-
if not base.startswith(("http://", "https://")):
51+
if not base.startswith(("http://", "https://")): # NOSONAR — scheme allowlist check, not a URL emission
5252
base = f"{_TEST_SCHEME}://{base}"
5353
url = f"{base}/usb/devices"
5454
headers = {"Authorization": f"Bearer {token}"} if token else {}
@@ -79,7 +79,7 @@ def run(self) -> None:
7979
devices = fetch_remote_devices(
8080
base_url=self._base_url, token=self._token,
8181
)
82-
except (ValueError, OSError, TimeoutError) as error: # NOSONAR python:S5713 # URLError is an OSError subclass; TimeoutError diverges from OSError on Python 3.10
82+
except (ValueError, OSError, TimeoutError) as error: # NOSONAR — TimeoutError is not an OSError on Python 3.10; URLError already is, so it was dropped
8383
self.failed.emit(str(error))
8484
return
8585
self.finished.emit(devices)

je_auto_control/gui/usb_passthrough_prompt.py

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -122,12 +122,11 @@ def decide(self, vendor_id: str, product_id: str,
122122
)
123123
if not done.wait(timeout=wait_timeout_s):
124124
return False
125-
# NOSONAR pythonbugs:S2583 — Sonar can't see through the
126-
# cross-thread QMetaObject.invokeMethod + queued slot above:
127-
# ``result`` is mutated by ``_show_dialog`` on the GUI thread
128-
# (line 145–146) before ``done`` is set, so neither key is
129-
# guaranteed False at this point.
130-
if result["allow"] and result["remember"] and self._acl is not None:
125+
# Sonar can't see through the cross-thread QMetaObject
126+
# .invokeMethod + queued slot above: ``result`` is mutated by
127+
# ``_show_dialog`` on the GUI thread before ``done`` is set,
128+
# so neither key is guaranteed False at this point.
129+
if result["allow"] and result["remember"] and self._acl is not None: # NOSONAR — cross-thread mutation through Q_ARG(object, result), see comment above
131130
self._acl.add_rule(AclRule(
132131
vendor_id=vendor_id, product_id=product_id,
133132
serial=(serial or None),

je_auto_control/utils/admin/admin_client.py

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ def _poll_one(self, host: AdminHost) -> HostStatus:
127127
start = time.monotonic()
128128
try:
129129
sessions = self._http_get(host, "/sessions")
130-
except (OSError, ValueError, TimeoutError) as error: # urllib.error.URLError is an OSError subclass; keep TimeoutError for Python 3.10 where it isn't (NOSONAR python:S5713)
130+
except (OSError, ValueError, TimeoutError) as error: # NOSONAR — TimeoutError diverges from OSError on Python 3.10 (the project's lowest supported version), so it is not redundant in the catch tuple
131131
return HostStatus(
132132
label=host.label, base_url=host.base_url, healthy=False,
133133
latency_ms=(time.monotonic() - start) * 1000.0,
@@ -144,7 +144,7 @@ def _poll_one(self, host: AdminHost) -> HostStatus:
144144
def _safe_get(self, host: AdminHost, path: str) -> Optional[Dict[str, Any]]:
145145
try:
146146
return self._http_get(host, path)
147-
except (OSError, ValueError, TimeoutError) as error: # urllib.error.URLError is an OSError subclass; keep TimeoutError for Python 3.10 where it isn't (NOSONAR python:S5713)
147+
except (OSError, ValueError, TimeoutError) as error: # NOSONAR — TimeoutError diverges from OSError on Python 3.10 (the project's lowest supported version), so it is not redundant in the catch tuple
148148
autocontrol_logger.warning(
149149
"admin: %s GET %s failed: %r", host.label, path, error,
150150
)
@@ -155,7 +155,7 @@ def _execute_one(self, host: AdminHost,
155155
try:
156156
payload = self._http_post(host, "/execute", {"actions": actions})
157157
return {"label": host.label, "ok": True, "result": payload}
158-
except (OSError, ValueError, TimeoutError) as error: # urllib.error.URLError is an OSError subclass; keep TimeoutError for Python 3.10 where it isn't (NOSONAR python:S5713)
158+
except (OSError, ValueError, TimeoutError) as error: # NOSONAR — TimeoutError diverges from OSError on Python 3.10 (the project's lowest supported version), so it is not redundant in the catch tuple
159159
return {"label": host.label, "ok": False, "error": str(error)}
160160

161161
def _http_get(self, host: AdminHost, path: str) -> Dict[str, Any]:
@@ -169,11 +169,10 @@ def _http_request(self, host: AdminHost, path: str, *,
169169
method: str, body: Optional[Dict[str, Any]],
170170
) -> Dict[str, Any]:
171171
url = f"{host.base_url}{path}"
172-
# NOSONAR python:S5332 — this is a scheme allowlist check, not
173-
# a URL emission. Both http:// and https:// must be accepted
174-
# because the operator points the admin console at whatever
175-
# the host is actually listening on.
176-
if not url.startswith(("http://", "https://")):
172+
# Both http:// and https:// must be accepted: the operator
173+
# points the admin console at whatever the host is actually
174+
# listening on, which may be plain HTTP behind a TLS proxy.
175+
if not url.startswith(("http://", "https://")): # NOSONAR — scheme allowlist check, not URL emission
177176
raise ValueError(f"unsupported URL scheme: {url}")
178177
headers = {"Authorization": f"Bearer {host.token}"}
179178
data = None

je_auto_control/utils/config_bundle/__main__.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -46,11 +46,11 @@ def main(argv: Optional[list] = None) -> int:
4646
def _do_export(output: Path, root: Optional[Path]) -> int:
4747
bundle = export_config_bundle(root=root)
4848
output.parent.mkdir(parents=True, exist_ok=True)
49-
# NOSONAR pythonsecurity:S2083 — the path comes from argv on a CLI
50-
# entry point. The operator running ``python -m ... export <file>``
51-
# is the trust boundary; restricting where they can write would
52-
# break the documented export workflow.
53-
output.write_text(
49+
# The output path comes from argv on a CLI entry point. The operator
50+
# running ``python -m ... export <file>`` is the trust boundary;
51+
# restricting where they can write would break the documented
52+
# export workflow.
53+
output.write_text( # NOSONAR — operator-controlled CLI argument by design (see comment above)
5454
json.dumps(bundle, ensure_ascii=False, indent=2),
5555
encoding="utf-8",
5656
)

je_auto_control/utils/remote_desktop/host_service.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ def write_default_config(path: Optional[Path] = None) -> Path:
7878
target = Path(path) if path else _DEFAULT_CONFIG_PATH
7979
target.parent.mkdir(parents=True, exist_ok=True)
8080
template = {
81-
"token": "CHANGE_ME_BEFORE_USE", # nosec B105 # NOSONAR python:S6418 # reason: placeholder in stub config the user MUST edit before installing the service
81+
"token": "CHANGE_ME_BEFORE_USE", # nosec B105 # NOSONAR placeholder in stub config the user MUST edit before installing the service
8282
"server_url": "https://your-signaling-server.example.com",
8383
"host_id": "abcd1234",
8484
"server_secret": None, # nosec B105 # reason: explicit None placeholder

je_auto_control/utils/remote_desktop/lan_discovery.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,10 @@ def _local_ip() -> str:
4444
# Connect-to-public-IP trick to discover the local interface
4545
# the kernel would pick for outbound traffic; UDP, so no
4646
# packet is sent and 8.8.8.8 (Google DNS) just stands in for
47-
# "any reachable public IP". NOSONAR python:S1313 because
48-
# the literal IS the well-known anycast probe address —
49-
# parameterising it would obscure intent.
50-
sock.connect(("8.8.8.8", 80)) # nosec B113 # NOSONAR python:S1313
47+
# "any reachable public IP". The literal is the well-known
48+
# anycast probe address — parameterising it would obscure
49+
# intent — see the next line for the suppression marker.
50+
sock.connect(("8.8.8.8", 80)) # nosec B113 # NOSONAR — literal is the well-known Google DNS anycast address used as a routing probe target
5151
return sock.getsockname()[0]
5252
finally:
5353
sock.close()

je_auto_control/utils/remote_desktop/signaling_server.py

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -138,12 +138,14 @@ def _check(
138138

139139
def _validate_host_id(host_id: str) -> None:
140140
if not host_id or len(host_id) > 128 or not host_id.isalnum():
141-
raise HTTPException(status_code=400, detail="invalid host_id")
141+
# 400 is documented at every caller route via _VALIDATION_RESPONSES.
142+
raise HTTPException(status_code=400, detail="invalid host_id") # NOSONAR — see _VALIDATION_RESPONSES
142143

143144

144145
def _validate_sdp(sdp: str) -> None:
145146
if not sdp or len(sdp.encode("utf-8")) > _MAX_SDP_BYTES:
146-
raise HTTPException(status_code=400, detail="invalid sdp size")
147+
# 400 is documented at every caller route via _VALIDATION_RESPONSES.
148+
raise HTTPException(status_code=400, detail="invalid sdp size") # NOSONAR — see _VALIDATION_RESPONSES
147149

148150

149151
def _configure_cors(app: FastAPI, cors_origins: Optional[List[str]]) -> None:
@@ -205,7 +207,8 @@ def _post_answer(host_id: str, body: _AnswerIn) -> dict:
205207
_validate_host_id(host_id)
206208
_validate_sdp(body.sdp)
207209
if not store.upsert_answer(host_id, body.sdp):
208-
raise HTTPException(status_code=404, detail="no offer to match")
210+
# 404 documented via _NOT_FOUND_RESPONSES on this route.
211+
raise HTTPException(status_code=404, detail="no offer to match") # NOSONAR
209212
return {"ok": True}
210213

211214
@app.get("/sessions/{host_id}/answer",

je_auto_control/utils/remote_desktop/web_viewer/index.html

Lines changed: 32 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -409,34 +409,39 @@
409409
});
410410
}
411411

412+
function _resolveLanguageChoice(choice) {
413+
if (choice !== "auto" && STRINGS[choice]) return choice;
414+
return detectLanguage();
415+
}
416+
417+
function _refreshDynamicLabels() {
418+
if ((els.status && els.status.textContent === "")
419+
|| (els.status && !connectedNow())) {
420+
els.status.textContent = t("status_idle");
421+
}
422+
if (els.stats && els.stats.textContent !== "") {
423+
els.stats.textContent = t("status_no_stats");
424+
}
425+
if (els.mic) {
426+
els.mic.textContent = micState ? t("btn_mic_on") : t("btn_mic_off");
427+
}
428+
if (els.opusMic) {
429+
els.opusMic.textContent =
430+
opusMicStream ? t("btn_opus_on") : t("btn_opus_off");
431+
}
432+
if (els.shareScreen) {
433+
els.shareScreen.textContent =
434+
screenStream ? t("btn_share_on") : t("btn_share_off");
435+
}
436+
}
437+
412438
function setLanguage(choice) {
413439
// choice: "auto" | "en" | "zh_TW" | "zh_CN" | "ja"
414440
localStorage.setItem("ac_lang", choice);
415-
let resolved;
416-
if (choice === "auto") {
417-
resolved = detectLanguage();
418-
} else if (STRINGS[choice]) {
419-
resolved = choice;
420-
} else {
421-
resolved = detectLanguage();
422-
}
423-
LANG = resolved;
441+
LANG = _resolveLanguageChoice(choice);
424442
dict = STRINGS[LANG] || STRINGS.en;
425443
applyI18n();
426-
// Re-render labels that aren't covered by data-i18n attributes:
427-
if (els.status && els.status.textContent === ""
428-
|| els.status && !connectedNow()) {
429-
els.status.textContent = t("status_idle");
430-
}
431-
if (els.stats && els.stats.textContent !== "")
432-
els.stats.textContent = t("status_no_stats");
433-
// Buttons that toggle between two states need re-syncing
434-
if (els.mic) els.mic.textContent = micState ? t("btn_mic_on") : t("btn_mic_off");
435-
if (els.opusMic)
436-
els.opusMic.textContent = opusMicStream ? t("btn_opus_on") : t("btn_opus_off");
437-
if (els.shareScreen)
438-
els.shareScreen.textContent = screenStream
439-
? t("btn_share_on") : t("btn_share_off");
444+
_refreshDynamicLabels();
440445
}
441446

442447
function connectedNow() {
@@ -1213,10 +1218,10 @@
12131218
els.fullscreenBtn.addEventListener("click", toggleFullscreen);
12141219

12151220
if ("serviceWorker" in navigator) {
1216-
// NOSONAR javascript:S7785 — this is a plain <script>, not a module,
1217-
// so top-level await isn't legal here. Service-worker registration
1218-
// is best-effort: we deliberately swallow rejection silently.
1219-
navigator.serviceWorker.register("sw.js").catch(() => {});
1221+
// This file is a plain <script>, not a module, so top-level await is
1222+
// not legal here. Service-worker registration is best-effort: any
1223+
// rejection is silently swallowed.
1224+
navigator.serviceWorker.register("sw.js").catch(() => {}); // NOSONAR — non-module script: top-level await not allowed
12201225
}
12211226
</script>
12221227
</body>

je_auto_control/utils/remote_desktop/web_viewer/mic-worklet.js

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,22 +3,28 @@
33
// The AudioContext is created with sampleRate: 16000 so we don't resample
44
// here — Float32 → Int16 is the only conversion needed.
55
class PcmProcessor extends AudioWorkletProcessor {
6-
// NOSONAR javascript:S3516 — AudioWorkletProcessor.process MUST return
7-
// true to keep the node alive; returning false would silently kill
8-
// the mic stream. Both branches are deliberately the same value.
6+
// AudioWorkletProcessor.process MUST return true to keep the node
7+
// alive; returning false would silently kill the mic stream. Single
8+
// exit point keeps Sonar's S3516 happy without an exception marker.
99
process(inputs) {
10-
const samples = inputs[0]?.[0]; // optional chain (S6582): no input → keep node alive
11-
if (!samples) return true;
12-
const int16 = new Int16Array(samples.length);
13-
for (let i = 0; i < samples.length; i++) {
14-
// i is a numeric loop counter, never user input; the "object
15-
// injection" warning here is a false positive — TypedArrays are
16-
// not subject to the prototype-pollution class of bug the rule
17-
// is meant to catch.
18-
const s = Math.max(-1, Math.min(1, samples[i])); // NOSONAR
19-
int16[i] = s < 0 ? s * 0x8000 : s * 0x7FFF; // NOSONAR
10+
const samples = inputs[0]?.[0]; // optional chain (S6582)
11+
if (samples) {
12+
const int16 = new Int16Array(samples.length);
13+
// nosemgrep: javascript.lang.security.audit.detect-object-injection
14+
// ``i`` is a numeric loop counter from 0..length-1 driving the
15+
// Float32Array / Int16Array typed-array element access. TypedArrays
16+
// clamp out-of-range indices and do not honour the prototype chain,
17+
// so the prototype-pollution class of bug that
18+
// ``security/detect-object-injection`` is built to find cannot
19+
// apply here — there is no user-controlled key path involved.
20+
for (let i = 0; i < samples.length; i++) {
21+
// eslint-disable-next-line security/detect-object-injection
22+
const s = Math.max(-1, Math.min(1, samples[i]));
23+
// eslint-disable-next-line security/detect-object-injection
24+
int16[i] = s < 0 ? s * 0x8000 : s * 0x7FFF;
25+
}
26+
this.port.postMessage(int16.buffer, [int16.buffer]);
2027
}
21-
this.port.postMessage(int16.buffer, [int16.buffer]);
2228
return true;
2329
}
2430
}

je_auto_control/utils/remote_desktop/webrtc_stats.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,10 @@ def start(self) -> None:
5151
future = get_bridge().submit(self._async_start())
5252
try:
5353
future.result(timeout=2.0)
54-
except (RuntimeError, TimeoutError, OSError) as error: # NOSONAR python:S5713 # TimeoutError is *not* an OSError on Python 3.10 (this project's lowest supported version); only the 3.11+ unification makes the catch redundant. Keep both for 3.10 compatibility.
54+
except (RuntimeError, TimeoutError, OSError) as error: # NOSONAR TimeoutError is not an OSError on Python 3.10 (project lowest supported); the redundancy only appears on 3.11+
5555
autocontrol_logger.warning("stats poller start: %r", error)
5656

57-
async def _async_start(self) -> None: # NOSONAR python:S7503 # must be a coroutine: it's submitted through asyncio.run_coroutine_threadsafe via the bridge.submit API; the body only schedules the loop task
57+
async def _async_start(self) -> None: # NOSONAR must remain a coroutine: it is submitted via asyncio.run_coroutine_threadsafe through bridge.submit; the body only schedules the loop task
5858
if self._task is not None:
5959
return
6060
self._task = asyncio.ensure_future(self._loop())

0 commit comments

Comments
 (0)