fix: ChimeraX --cmd quoting breaks rendering

scripts/plotting/chimerax_plot.py

@@ -17,20 +17,18 @@
 logger = daiquiri.getLogger(__logger_name__ + ".plotting.chimerax_plot")
 
 
-# Characters that would break the ChimeraX command parser (or, worse, allow
-# injecting extra ChimeraX commands) if interpolated raw into the --cmd script.
-# Double quotes are how we delimit path arguments; newlines / carriage returns
-# would let an attacker (or a malformed config) end a command and start new ones.
-_CHIMERAX_UNSAFE_PATH_CHARS = ('"', '\n', '\r')
+# Paths are interpolated unquoted into the ';'-separated --cmd script (ChimeraX
+# rejects quoted paths in such a --cmd on some versions), so reject characters
+# that would split a path (space, tab) or terminate the command and inject new
+# ones (';', quote, newline, CR).
+_CHIMERAX_UNSAFE_PATH_CHARS = ('"', ' ', '\t', ';', '\n', '\r')
 
 
 def _validate_chimerax_path(path, role):
-    """Reject paths that contain characters unsafe for the ChimeraX --cmd script.
+    """Reject paths with characters that would break the ChimeraX --cmd script.
 
-    Real-world paths in this codebase don't contain these characters (build-datasets
-    rejects them, and HPC/output paths are normally clean), but interpolating them
-    raw would silently produce broken or attacker-controlled ChimeraX command
-    strings. Surface a clear failure instead.
+    Real-world paths here are normally clean; this just surfaces a clear failure
+    instead of a silently broken or injected command.
     """
     text = str(path)
     bad = [c for c in _CHIMERAX_UNSAFE_PATH_CHARS if c in text]
@@ -42,13 +40,12 @@ def _validate_chimerax_path(path, role):
 
 
 def _run_chimerax(command, label):
-    """Run a ChimeraX subprocess. Surface failures as warnings so one bad image
-    doesn't abort the whole loop, but the user actually sees something went wrong.
+    """Run a ChimeraX subprocess. Failures become warnings so one bad image
+    doesn't abort the loop while the user still sees something went wrong.
 
-    `command` is an argv list passed with shell=False, so the OS shell never
-    interprets the tokens (spaces, metacharacters, etc. in argv elements are
-    preserved as-is). Path quoting *inside* the ChimeraX --cmd script is
-    handled separately by `get_chimerax_command`.
+    Paths inside the --cmd script are unquoted, so `get_chimerax_command`
+    validates them to be free of spaces/quotes/newlines that would break the
+    ChimeraX parser.
     """
     # Discard stdout (never read) but keep stderr for the failure warning;
     # capture_output=True would buffer both into memory across many genes.
@@ -159,22 +156,15 @@ def get_chimerax_command(chimerax_bin,
     palette = get_palette(intervals, type="diverging") if attribute == "logscore" else get_palette(intervals, type="sequential")
     transparent_bg_suffix = " transparentBackground  true" if transparent_bg else ""
 
-    # Validate paths *before* interpolating them: the double-quote wrapping below
-    # protects against whitespace splitting but doesn't escape embedded " or
-    # newlines, which would otherwise let a path inject extra ChimeraX commands.
+    # Validate before interpolating: paths go unquoted into the --cmd script.
     _validate_chimerax_path(pdb_path, "PDB")
     _validate_chimerax_path(attr_file_path, "attribute file")
 
-    # Quote paths so ChimeraX's own command parser treats each as a single token
-    # even when the path contains spaces.
-    pdb_arg = f'"{pdb_path}"'
-    attr_arg = f'"{attr_file_path}"'
-
     chimerax_script = (
-        f"open {pdb_arg}; "
+        f"open {pdb_path}; "
         "set bgColor white; "
         "color lightgray; "
-        f"open {attr_arg}; "
+        f"open {attr_file_path}; "
         f"color byattribute {attribute} palette {palette}; "
         f"key {palette} :{intervals[0]} :{intervals[1]} :{intervals[2]} :{intervals[3]} :{intervals[4]} pos 0.35,0.03 fontSize 4 size 0.3,0.02;"
         f"2dlabels create label text '{labels[attribute]}' size 6 color darkred xpos 0.34 ypos 0.065;"
@@ -209,11 +199,9 @@ def get_chimerax_command(chimerax_bin,
 
     output_path = os.path.join(chimera_output_path, f"{cohort}_{i}_{gene}_{attribute}{cluster_tag}.png")
     _validate_chimerax_path(output_path, "output")
-    chimerax_script += f'save "{output_path}" pixelSize {pixelsize} supersample 3{transparent_bg_suffix};exit'
+    chimerax_script += f'save {output_path} pixelSize {pixelsize} supersample 3{transparent_bg_suffix};exit'
 
-    # Return as an argv list so the caller can use subprocess.run(..., shell=False),
-    # bypassing the OS shell entirely. Paths embedded inside the ChimeraX
-    # --cmd script are also quoted so ChimeraX's own parser handles spaces.
+    # argv list for subprocess.run(shell=False) — the OS shell never sees it.
     return [chimerax_bin, "--nogui", "--offscreen", "--silent", "--cmd", chimerax_script]
 
 
@@ -240,7 +228,8 @@ def generate_chimerax_plot(output_dir,
     if "Ratio_obs_sim" in result.columns:
         result = result.rename(columns={"Ratio_obs_sim" : "Score_obs_sim"})
     result = cap_inf_scores(result)
-    result["Logscore_obs_sim"] = np.log(result["Score_obs_sim"])
+    with np.errstate(divide="ignore"):
+        result["Logscore_obs_sim"] = np.log(result["Score_obs_sim"])
 
     # Detect the AlphaFold version actually present in the dataset; the
     # user-supplied --af_version is used as a tiebreaker only.

scripts/plotting/utils.py

@@ -55,7 +55,7 @@ def cap_inf_scores(pos_result, col="Score_obs_sim"):
     n_inf = int(inf_mask.sum())
     logger.warning(
         f"Capping {n_inf} `{col}` value(s) at {cap:.4f} for plotting "
-        f"(originally +inf — extreme hotspot positions)"
+        f"(originally +inf: extreme hotspot positions)"
     )
     # Write back the whole numeric column so the returned frame is numeric dtype.
     # .copy() is required: pd.to_numeric can share the caller's buffer, so an