Skip to content

fix: quote paths inside ChimeraX --cmd script to handle spaces #106

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
St3451 merged 2 commits into from
May 28, 2026
Merged

fix: quote paths inside ChimeraX --cmd script to handle spaces #106

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
55a5b70
fix: quote paths inside ChimeraX --cmd script to handle spaces
St3451 May 28, 2026
cbe61af
fix: validate paths for ChimeraX commands to prevent injection of uns…
St3451 May 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
111 changes: 79 additions & 32 deletions scripts/plotting/chimerax_plot.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,38 @@
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')


def _validate_chimerax_path(path, role):
"""Reject paths that contain characters unsafe for 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.
"""
text = str(path)
bad = [c for c in _CHIMERAX_UNSAFE_PATH_CHARS if c in text]
if bad:
raise ValueError(
f"{role} path contains characters unsafe for the ChimeraX command "
f"script ({', '.join(repr(c) for c in bad)}): {text!r}"
)


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.

`command` is an argv list (no shell interpretation), so paths and labels
with spaces or shell metacharacters pass through verbatim.
`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`.
"""
# Discard stdout (never read) but keep stderr for the failure warning;
# capture_output=True would buffer both into memory across many genes.
Expand Down Expand Up @@ -132,11 +158,22 @@ 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_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_path}; "
f"open {pdb_arg}; "
"set bgColor white; "
"color lightgray; "
f"open {attr_file_path}; "
f"open {attr_arg}; "
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;"
Expand All @@ -156,10 +193,12 @@ def get_chimerax_command(chimerax_bin,
cluster_tag = ""

output_path = os.path.join(chimera_output_path, f"{cohort}_{i}_{gene}_{attribute}{cluster_tag}.png")
chimerax_script += f"save {output_path} pixelSize {pixelsize} supersample 3{transparent_bg_suffix};exit"
_validate_chimerax_path(output_path, "output")
chimerax_script += f'save "{output_path}" pixelSize {pixelsize} supersample 3{transparent_bg_suffix};exit'

Comment thread
St3451 marked this conversation as resolved.
# Return as an argv list so the caller can use subprocess.run(..., shell=False)
# and avoid shell interpretation of paths/labels with spaces or metacharacters.
# 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.
return [chimerax_bin, "--nogui", "--offscreen", "--silent", "--cmd", chimerax_script]


Expand Down Expand Up @@ -251,39 +290,47 @@ def generate_chimerax_plot(output_dir,
intervals = get_intervals(attribute_vector, attribute)

logger.debug("Generating 3D images..")
chimerax_command = get_chimerax_command(chimerax_bin,
pdb_path,
chimera_plots_path,
attr_file_path,
attribute,
intervals,
gene,
uni_id,
labels,
i,
f,
cohort,
pixelsize=pixel_size,
transparent_bg=transparent_bg)
_run_chimerax(chimerax_command, f"{gene} ({attribute})")
logger.debug(shlex.join(chimerax_command))

if attribute == "score" or attribute == "logscore":
chimerax_command = get_chimerax_command(chimerax_bin,
pdb_path,
chimera_plots_path,
attr_file_path,
attribute,
intervals,
try:
chimerax_command = get_chimerax_command(chimerax_bin,
pdb_path,
chimera_plots_path,
attr_file_path,
attribute,
intervals,
gene,
uni_id,
labels,
i,
f,
cohort,
clusters=clusters,
pixelsize=pixel_size,
transparent_bg=transparent_bg)
except ValueError as exc:
logger.warning(f"Skipping {gene} ({attribute}): {exc}")
continue
_run_chimerax(chimerax_command, f"{gene} ({attribute})")
logger.debug(shlex.join(chimerax_command))

if attribute == "score" or attribute == "logscore":
try:
chimerax_command = get_chimerax_command(chimerax_bin,
pdb_path,
chimera_plots_path,
attr_file_path,
attribute,
intervals,
gene,
uni_id,
labels,
i,
f,
cohort,
clusters=clusters,
pixelsize=pixel_size,
transparent_bg=transparent_bg)
except ValueError as exc:
logger.warning(f"Skipping {gene} ({attribute}, clusters): {exc}")
continue
_run_chimerax(chimerax_command, f"{gene} ({attribute}, clusters)")
logger.debug(shlex.join(chimerax_command))

Expand Down