[CRAFTING] Add Screenshot Updater CLI tool for updating widgets e2e screenshots locally

[leonardomendix]

Pull request type

Test related change (New E2E test, test automation, etc.)

---

Description

Implemented a new CLI tool for updating Playwright screenshots using Docker and Mendix runtime.

[github-actions]

AI Code Review

⚠️ Approved with suggestions — low-severity items only, safe to merge

---

What was reviewed

File	Change
automation/scripts/update-screenshots-local.mjs	New CLI tool for updating Playwright screenshot baselines locally via Docker + Mendix runtime

Skipped (out of scope): pnpm-lock.yaml

---

Findings

⚠️ Low — widgetName user input passed directly into shell command string

File: automation/scripts/update-screenshots-local.mjs line 604 and line 725
Note: widgetName (from CLI positional arg) is interpolated directly into a shell command string passed to execSync (line 604) and into a bash script written to disk and executed (line 725). If a user passes a crafted widget name like foo; rm -rf /, it could be interpreted by the shell. The risk is developer-tool-only, so not blocking, but worth guarding.
Fix: Validate widgetName against the discovered allWidgets list before use (this already happens at line 877 via allWidgets.includes(widgetName) — just ensure this check always precedes any shell use, which it does). For the shell script generation, also consider shell-quoting the interpolated path explicitly:

`cd /workspace/packages/pluggableWidgets/${widgetName.replace(/[^a-z0-9_-]/gi, "")}`

Since the validation at line 877 already whitelist-checks the name against the scanned directories, the actual risk is minimal.

---

⚠️ Low — No --add-host flag for Linux Docker networking

File: automation/scripts/update-screenshots-local.mjs line 706–711
Note: The tool resolves hostIp by scanning os.networkInterfaces() for non-internal IPv4, then falls back to host.docker.internal. On Linux, host.docker.internal only works if --add-host=host.docker.internal:host-gateway is passed to docker run. The fallback will silently fail on Linux Docker Engine without Docker Desktop, and the runtime health check will pass but Playwright won't actually reach the runtime.
Fix: When running on Linux, add --add-host=host.docker.internal:host-gateway to the dockerCmd args array (lines 741–756), or document this limitation prominently in the usage help.

---

⚠️ Low — onExit handler leaks containerId closure on normal completion

File: automation/scripts/update-screenshots-local.mjs line 848–851
Note: process.once("SIGINT", onExit) and process.once("SIGTERM", onExit) are registered but never removed after a successful run. If the process emits a late signal after cleanup() already ran (line 937–939), the handler will call cleanup(containerId, tmpDir) again where containerId is now null (line 938) — harmless but sloppy. More importantly the tmpDir reference may point to a path that's already been deleted.
Fix: Remove the signal handlers after successful cleanup:

process.removeListener("SIGINT", onExit);
process.removeListener("SIGTERM", onExit);

Add these two lines right before or after line 938.

---

Positives

• The existing allWidgets.includes(widgetName) guard at line 877 naturally prevents unknown/crafted widget names from reaching shell commands — good defensive ordering.
• Using a random container label (e2e.mxruntime=${labelValue}) for polling instead of hardcoding a name avoids collisions when running multiple instances in parallel.
• The redirect-following in downloadToFile correctly drops the Authorization header on non-GitHub hostnames (line 278–280), preventing token leakage to CDN redirect targets.
• The findFreePort() implementation (line 426–435) uses OS port binding to avoid races — much more reliable than a hardcoded port.
• The spinner gracefully degrades for non-TTY environments (line 82–84), making CI log output readable.

[github-actions]

AI Code Review

🔶 Changes requested — one or more medium-severity items must be addressed

---

What was reviewed

File	Change
automation/scripts/update-screenshots-local.mjs	New ~980-line CLI tool — Docker + Mendix runtime screenshot updater
automation/scripts/package.json	New update-screenshots-local script entry
package.json	Root-level script alias for the new tool
automation/utils/bin/rui-merge-changelogs-pr.ts	File mode change only (chmod +x)

Skipped (out of scope): dist/, pnpm-lock.yaml

⚠️ CI check status unavailable — gh pr checks required interactive approval in this environment. Please verify lint and test jobs are green before merging.

---

Findings

🔶 Medium — Shell command built by string join breaks on paths with spaces and accepts unvalidated user input

File: automation/scripts/update-screenshots-local.mjs lines 620–628

Problem: buildDeploymentBundle constructs a Docker command by joining an array with " " and passing the result as a shell string to execSync. Three separate values are embedded without quoting or validation:

1. REPO_ROOT — not shell-quoted; breaks silently if the repo is checked out under a path with spaces (e.g. ~/My Projects/web-widgets).
2. mxbuildImage (mxbuild:${mendixVersion}) — mendixVersion comes directly from the --mendix-version CLI flag with no format validation. A value like 10.0 --rm -v /:/host ubuntu reshapes the Docker invocation; a value with ; can inject host-side shell commands.
3. mprPath embedded inside bash -c "…" — unquoted inside the double-quoted string; a filename with spaces or $ will corrupt the argument.

startMendixRuntime on line 671 correctly uses spawn("docker", args, …) — apply the same pattern here and in runPlaywrightTests (line 781).

Fix:

// buildDeploymentBundle — use spawnSync with an args array
const result = spawnSync(
    "docker",
    [
        "run", "--tty", "--rm",
        "--volume", `${REPO_ROOT}:/source`,
        mxbuildImage,                          // safe: no shell expansion on array items
        "bash", "-c",
        `mx update-widgets --loose-version-check '${mprPath}' && mxbuild --output=/source/automation.mda '${mprPath}'`
    ],
    { stdio: VERBOSE ? "inherit" : "pipe" }
);
if (result.status !== 0) throw new Error("mxbuild failed");

// runPlaywrightTests — replace execSync(dockerCmd.join(" "), …) with:
const result = spawnSync(dockerCmd[0], dockerCmd.slice(1), { stdio: "inherit" });
if (result.status !== 0) throw new Error(`Docker exited ${result.status}`);

Add a simple guard on the mendixVersion input to reject obviously malformed values:

if (!/^[\d.]+$/.test(mendixVersion)) {
    bail(`Invalid Mendix version format: ${mendixVersion}`);
}

---

⚠️ Low — PR title does not follow JIRA key format

File: PR metadata

Note: The title [CRAFTING] Add Screenshot Updater CLI tool… uses an informal label. The repo checklist requires either [XX-000]: description (JIRA) or a conventional commit prefix (feat:, fix:, etc.). Consider renaming to something like feat(automation): add local screenshot updater CLI tool or link to a real JIRA ticket.

---

⚠️ Low — No validation that --mendix-version matches a known version

File: automation/scripts/update-screenshots-local.mjs line 864

Note: versions.latest is used when no flag is provided, but when --mendix-version is given the value bypasses the versions map entirely and is used as-is. A version that is valid syntactically but not present in mendix-versions.json will cause a confusing Docker image-not-found error rather than a clear message. Consider checking that the supplied version is a key in the versions file (or at least matches \d+\.\d+\.\d+\.\d+).

---

Positives

• startMendixRuntime correctly uses spawn("docker", args, …) with an args array — the safe pattern that avoids all shell expansion. The medium finding above is specifically about the two places that diverge from this established pattern in the same file.
• widgetName is validated against the live widget list before being embedded in any shell script, preventing path traversal from that input.
• SIGINT/SIGTERM cleanup handlers prevent orphaned Docker containers and temp files when the user presses Ctrl-C — a common pain point for local automation scripts.
• isTTY guard on the spinner produces clean output when piped, which makes the tool work well in CI contexts as well.
• Graceful fallback from unzip to python3 for ZIP extraction is a nice portability touch for teams that don't have unzip on their PATH.