Address SonarCloud + Codacy findings on PR #196

Locator/runner/LSP/stub housekeeping
  - ab_locator/runner.py:71 use dict() instead of comprehension (S7500)
  - dag/runner.py:221 drop unnecessary list() (S7504)
  - autocontrol-lsp documents.py:65 extract _resolve_next_version (S3358)
  - autocontrol-lsp server.py:169 split run() into _process_one_message
    to cut cognitive complexity from 17 to ≤15 (S3776)
  - stubs/generator.py:182 add error-return path so _cli() no longer
    always returns 0 (S3516); hoist sys import; type stub falls back
    to Any for non-builtin classes and dotted module paths

Tests: use pytest.approx for every == on floats (S1244, 8 sites);
       move /tmp literals to tmp_path / project-relative paths (S5443);
       NOSONAR-tag two test names that mirror AC_drag / AgentBackendError

Wayland: NOSONAR-tag the resolution regex (S5852 — anchored \d+, no
  nested quantifiers, not vulnerable to ReDoS)

Language wrappers (en / zh-TW / zh-CN / ja): extract _BROWSE,
  _CLEAR_LOG, _OUTPUT_LABEL, _MODEL_LABEL, _LOCATE_CLICK constants so
  duplicated UI button labels stop tripping S1192

Slack example:
  - render_report path-traversal hardening: basename + resolve check
    so a malicious ``today`` can't escape the output dir (S2083)
  - email_report pins TLS minimum to 1.2 (S4423)

VS Code extension (extension.ts):
  - import * as http from "node:http" / "node:https" / "node:url"
    (S7772)
  - consolidate context.subscriptions.push calls into one (S7778)
  - mark ScriptStepProvider.emitter readonly (S2933)
  - use optional chaining on editor?.document.languageId (S6582)

Browser extension:
  - background.js loadState() no longer spreads an empty literal
    (S7744)
  - content_script.js uses optional chaining (S6582), globalThis
    instead of window (S7764), String.raw for the regex escape
    pattern (S7780)
  - popup.js optional chain on tab?.url (S6582); void refresh() at
    bottom matches S7785 expectations

Hotspots
  - failure_hooks/backends.py:124 NOSONAR comment on the http:// scheme
    allow-list (S5332 — guard rejects, never emits)
  - test_failure_hooks.py:196 NOSONAR on the ftp:// negative-test
    literal (S5332)
  - Dockerfile.xfce:50 NOSONAR comment on the documented VNC port
    exposure (S6473)
  - docker.yml NOSONAR comments on action major-version pins (S7637 —
    matches project convention across dev/stable/quality workflows)

Author: JE-Chen

.github/workflows/docker.yml

@@ -28,10 +28,10 @@ jobs:
       - uses: actions/checkout@v4
 
       - name: Set up Docker Buildx
-        uses: docker/setup-buildx-action@v3
+        uses: docker/setup-buildx-action@v3  # NOSONAR githubactions:S7637 — project convention pins to vendored major version tags (matches dev.yml / stable.yml / quality.yml)
 
       - name: Build image (no push)
-        uses: docker/build-push-action@v5
+        uses: docker/build-push-action@v5  # NOSONAR githubactions:S7637 — project convention pins to vendored major version tags
         with:
           context: .
           file: docker/Dockerfile
@@ -52,10 +52,10 @@ jobs:
       - uses: actions/checkout@v4
 
       - name: Set up Docker Buildx
-        uses: docker/setup-buildx-action@v3
+        uses: docker/setup-buildx-action@v3  # NOSONAR githubactions:S7637 — project convention pins to vendored major version tags (matches dev.yml / stable.yml / quality.yml)
 
       - name: Rebuild image (cached)
-        uses: docker/build-push-action@v5
+        uses: docker/build-push-action@v5  # NOSONAR githubactions:S7637 — project convention pins to vendored major version tags
         with:
           context: .
           file: docker/Dockerfile

autocontrol-lsp/autocontrol_lsp/server/documents.py

@@ -60,10 +60,7 @@ def replace(self, uri: str, text: str,
                  version: Optional[int] = None) -> TextDocument:
         with self._lock:
             existing = self._docs.get(uri)
-        next_version = (
-            int(version) if version is not None
-            else (existing.version + 1 if existing else 0)
-        )
+        next_version = _resolve_next_version(version, existing)
         return self.open(uri, text, next_version)
 
     def close(self, uri: str) -> bool:
@@ -103,6 +100,16 @@ def apply_change(self, uri: str,
 
 # --- helpers -------------------------------------------------
 
+def _resolve_next_version(override: Optional[int],
+                           existing: Optional[TextDocument]) -> int:
+    """Pick the next document version: explicit override → existing+1 → 0."""
+    if override is not None:
+        return int(override)
+    if existing is not None:
+        return existing.version + 1
+    return 0
+
+
 _WORD_CHARS = set("ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789_")
 
 

autocontrol-lsp/autocontrol_lsp/server/server.py

@@ -172,26 +172,30 @@ def run(input_stream=None, output_stream=None) -> int:
     out = output_stream or sys.stdout.buffer
     server = LspServer()
     try:
-        while True:
-            request = _read_message(inp)
-            if request is None or request.get("method") == "exit":
-                return 0
-            method = request.get("method")
-            params = request.get("params") or {}
-            if not isinstance(method, str):
-                continue
-            request_id = request.get("id")
-            if request_id is None:
-                server.handle_notification(method, params)
-                for payload in server.drain_diagnostics():
-                    _publish_diagnostics(out, payload)
-                continue
-            result = server.dispatch(method, params)
-            _write_message(out, _build_reply(method, request_id, result))
-            for payload in server.drain_diagnostics():
-                _publish_diagnostics(out, payload)
+        while _process_one_message(inp, out, server):
+            pass
     except (OSError, ValueError):
         return 1
+    return 0
+
+
+def _process_one_message(inp, out, server: LspServer) -> bool:
+    """Read one LSP message and dispatch it. Returns False to end the loop."""
+    request = _read_message(inp)
+    if request is None or request.get("method") == "exit":
+        return False
+    method = request.get("method")
+    params = request.get("params") or {}
+    if not isinstance(method, str):
+        return True
+    if request.get("id") is None:
+        server.handle_notification(method, params)
+    else:
+        result = server.dispatch(method, params)
+        _write_message(out, _build_reply(method, request["id"], result))
+    for payload in server.drain_diagnostics():
+        _publish_diagnostics(out, payload)
+    return True
 
 
 if __name__ == "__main__":  # pragma: no cover - entry point

autocontrol-lsp/vscode/src/extension.ts

@@ -3,9 +3,9 @@
 // and exposes a tree view of the current script's steps.
 
 import * as vscode from "vscode";
-import * as http from "http";
-import * as https from "https";
-import { URL } from "url";
+import * as http from "node:http";
+import * as https from "node:https";
+import { URL } from "node:url";
 import {
     LanguageClient,
     LanguageClientOptions,
@@ -18,20 +18,17 @@ let stepProvider: ScriptStepProvider | undefined;
 
 export function activate(context: vscode.ExtensionContext): void {
     client = startLanguageClient();
-    context.subscriptions.push({ dispose: () => client?.stop() });
-
     stepProvider = new ScriptStepProvider();
+    // Consolidate every subscription into a single push() call (S7778):
+    // VS Code accepts a varargs subscriber list, so building the array
+    // up front keeps the call site flat and avoids repeated diffs.
     context.subscriptions.push(
+        { dispose: () => client?.stop() },
         vscode.window.registerTreeDataProvider(
             "autocontrolScriptSteps", stepProvider,
         ),
-    );
-    context.subscriptions.push(
         vscode.window.onDidChangeActiveTextEditor(() => stepProvider?.refresh()),
         vscode.workspace.onDidChangeTextDocument(() => stepProvider?.refresh()),
-    );
-
-    context.subscriptions.push(
         vscode.commands.registerCommand(
             "autocontrol.runScript", runCurrentScript,
         ),
@@ -193,7 +190,7 @@ async function takeScreenshot(): Promise<void> {
 // --- Tree view ----------------------------------------------------
 
 class ScriptStepProvider implements vscode.TreeDataProvider<StepItem> {
-    private emitter = new vscode.EventEmitter<StepItem | undefined>();
+    private readonly emitter = new vscode.EventEmitter<StepItem | undefined>();
     readonly onDidChangeTreeData = this.emitter.event;
 
     refresh(): void { this.emitter.fire(undefined); }
@@ -202,7 +199,7 @@ class ScriptStepProvider implements vscode.TreeDataProvider<StepItem> {
 
     getChildren(): vscode.ProviderResult<StepItem[]> {
         const editor = vscode.window.activeTextEditor;
-        if (!editor || editor.document.languageId !== "json") {
+        if (editor?.document.languageId !== "json") {
             return [];
         }
         let parsed: unknown;

browser-extension/background.js

@@ -20,7 +20,8 @@ const DEFAULT_STATE = {
 
 async function loadState() {
     const stored = await chrome.storage.local.get(STATE_KEY);
-    return { ...DEFAULT_STATE, ...(stored[STATE_KEY] || {}) };
+    const saved = stored[STATE_KEY];
+    return saved ? { ...DEFAULT_STATE, ...saved } : { ...DEFAULT_STATE };
 }
 
 async function saveState(state) {

browser-extension/content_script.js

@@ -12,7 +12,7 @@
      * data-name > nth-of-type fallback.
      */
     function selectorFor(element) {
-        if (!element || element.nodeType !== 1) { return ""; }
+        if (element?.nodeType !== 1) { return ""; }
         if (element.id) {
             return "#" + cssEscape(element.id);
         }
@@ -29,7 +29,7 @@
     function nthOfTypeSelector(element) {
         const path = [];
         let node = element;
-        while (node && node.nodeType === 1 && node !== document.documentElement) {
+        while (node?.nodeType === 1 && node !== document.documentElement) {
             const tag = node.tagName.toLowerCase();
             const parent = node.parentElement;
             if (!parent) {
@@ -48,11 +48,15 @@
     }
 
     function cssEscape(value) {
-        if (window.CSS && typeof window.CSS.escape === "function") {
-            return window.CSS.escape(value);
+        if (typeof globalThis.CSS?.escape === "function") {
+            return globalThis.CSS.escape(value);
         }
-        // Bare-bones fallback for browsers without CSS.escape.
-        return String(value).replace(/(["\\\]])/g, "\\$1");
+        // Bare-bones fallback for browsers without CSS.escape. Use
+        // ``String.raw`` so the regex escape literal reads as written.
+        return String(value).replace(
+            new RegExp(String.raw`(["\\\]])`, "g"),
+            String.raw`\$1`,
+        );
     }
 
     function send(event) {
@@ -94,7 +98,7 @@
         });
     }, true);
 
-    window.addEventListener("popstate", () => {
+    globalThis.addEventListener("popstate", () => {
         send({ type: "navigate", url: location.href });
     });
 

browser-extension/popup.js

@@ -22,7 +22,7 @@ document.getElementById("start").addEventListener("click", async () => {
     const [tab] = await chrome.tabs.query({
         active: true, currentWindow: true,
     });
-    await send("start", { startUrl: tab && tab.url });
+    await send("start", { startUrl: tab?.url });
     refresh();
 });
 
@@ -50,4 +50,8 @@ document.getElementById("export").addEventListener("click", async () => {
     });
 });
 
-refresh();
+// Popup HTML loads this script as a classic script (not a module), so
+// top-level await isn't legal here. ``void refresh()`` is the
+// equivalent fire-and-forget that Sonar's S7785 accepts in this
+// context (we don't await because the initial fetch is best-effort).
+void refresh();

docker/Dockerfile.xfce

@@ -47,6 +47,10 @@ ENV DISPLAY=:99 \
     AUTOCONTROL_HEADLESS=0 \
     AUTOCONTROL_VNC_PORT=5900
 
+# hadolint ignore=DL3018
+# NOSONAR docker:S6473 — exposing 5900 (VNC) is the documented purpose
+# of this variant; operators bind it behind a firewall / VPN, with the
+# optional AUTOCONTROL_VNC_PASSWORD providing TightVNC-level auth.
 EXPOSE 9939 9940 8765 5900
 
 COPY docker/entrypoint-xfce.sh /usr/local/bin/autocontrol-entrypoint

examples/18_slack_daily_report.py

@@ -176,7 +176,15 @@ def render_report(summary: str, raw_messages: Sequence[SlackMessage],
 <ul>{rows}</ul>
 </body></html>
 """
-    html_path = output_dir / f"digest-{today}.html"
+    # Anchor the filename inside output_dir's resolved path so a
+    # malicious ``today`` (e.g. ``../etc/passwd``) can't escape — even
+    # though ``today`` is internally generated, validating here keeps
+    # Sonar's S2083 happy and protects future callers.
+    safe_name = os.path.basename(f"digest-{today}.html")
+    safe_root = output_dir.resolve()
+    html_path = (safe_root / safe_name).resolve()
+    if safe_root not in html_path.parents:
+        raise ValueError(f"refusing path-traversal name: {today!r}")
     html_path.write_text(html, encoding="utf-8")
     try:
         from weasyprint import HTML
@@ -212,6 +220,10 @@ def email_report(artefact: Path, *,
         filename=artefact.name,
     )
     context = ssl.create_default_context()
+    # Pin the minimum to TLS 1.2 even on older Pythons whose defaults
+    # may still allow TLS 1.0/1.1 (S4423). Python 3.10+ already does
+    # this by default; the explicit assignment is a belt-and-braces.
+    context.minimum_version = ssl.TLSVersion.TLSv1_2
     with smtplib.SMTP(smtp_host, smtp_port, timeout=30.0) as server:
         server.starttls(context=context)
         if smtp_user and smtp_pass: