From eb61df1c93522715a57006da873309bb1f0960c3 Mon Sep 17 00:00:00 2001 From: x01dc Date: Mon, 27 Jul 2026 17:06:10 +0200 Subject: [PATCH] fix(OMNY): use requests instead of shelling out to wget for tomo ID TomoIDManager.register() called wget via subprocess with shell=True, interpolating sample_name/eaccount/etc. unescaped into the command string -- broke outright wherever wget isn't installed (as hit while testing: "wget: command not found", silently falling back to tomo ID 0), and was a latent shell-injection risk. Use requests.get() with a params dict instead: no external binary dependency, proper URL encoding, and the same self-signed-cert/SSRF handling already used by the samples PDF upload. Drops the now-unused OMNYToolsError class and TMP_FILE/subprocess, which only existed to support the wget call. --- .../plugins/omny/omny_general_tools.py | 57 ++++++++++--------- 1 file changed, 31 insertions(+), 26 deletions(-) diff --git a/csaxs_bec/bec_ipython_client/plugins/omny/omny_general_tools.py b/csaxs_bec/bec_ipython_client/plugins/omny/omny_general_tools.py index c41d5e6..8f8de0d 100644 --- a/csaxs_bec/bec_ipython_client/plugins/omny/omny_general_tools.py +++ b/csaxs_bec/bec_ipython_client/plugins/omny/omny_general_tools.py @@ -4,7 +4,6 @@ import fcntl import json import os import socket -import subprocess import sys import termios import threading @@ -36,10 +35,6 @@ def umvr(*args): return scans.umv(*args, relative=True) -class OMNYToolsError(Exception): - pass - - class OMNYTools: HEADER = "\033[95m" @@ -354,7 +349,6 @@ class TomoIDManager: OMNY_URL = "https://v1p0zyg2w9n2k9c1.myfritz.net/samples/newmeasurement.php" TEST_OMNY_URL = "https://omny-test.psi.ch/samples/newmeasurement.php" - TMP_FILE = "~/currsamplesnr.txt" FALLBACK_TOMO_ID = 0 @staticmethod @@ -389,28 +383,39 @@ class TomoIDManager: "production -- the eaccount recorded there won't be real." ) - url = ( - f"{omny_url}" - f"?sample={sample_name}" - f"&date={date}" - f"&eaccount={eaccount}" - f"&scannr={scan_number}" - f"&setup={setup}" - f"&additional={additional_info}" - f"&user={user}" - ) + params = { + "sample": sample_name, + "date": date, + "eaccount": eaccount, + "scannr": scan_number, + "setup": setup, + "additional": additional_info, + "user": user, + } - tmp_file = os.path.expanduser(self.TMP_FILE) try: - result = subprocess.run(f"wget -q -O {tmp_file} '{url}'", shell=True, timeout=30) - if result.returncode != 0: - raise OMNYToolsError( - f"wget failed (exit code {result.returncode}) fetching tomo ID from {omny_url}" - ) - with open(tmp_file) as f: - content = f.read().strip() - return int(content) - except (subprocess.TimeoutExpired, FileNotFoundError, ValueError, OMNYToolsError) as exc: + import requests + import urllib3 + + urllib3.disable_warnings(urllib3.exceptions.InsecureRequestWarning) + except ImportError as exc: + logger.warning( + f"Could not obtain tomo ID from OMNY database ('requests' library not " + f"installed: {exc}); falling back to tomo ID {self.FALLBACK_TOMO_ID}." + ) + return self.FALLBACK_TOMO_ID + + try: + response = requests.get( + omny_url, + params=params, + timeout=30, + verify=False, # accept self-signed certs + allow_redirects=False, # SSRF hardening + ) + response.raise_for_status() + return int(response.text.strip()) + except (requests.RequestException, ValueError) as exc: logger.warning( f"Could not obtain tomo ID from OMNY database ({exc}); " f"falling back to tomo ID {self.FALLBACK_TOMO_ID}."