From fcb39da3ac5984ee715caea45fb6df97b43726fc Mon Sep 17 00:00:00 2001 From: daniel-rdt Date: Thu, 2 Jul 2026 14:42:48 +0200 Subject: [PATCH 01/11] feat: add sanitization of cli inputs in launch_explorer.py --- scripts/sb/launch_explorer.py | 123 +++++++++++++++++++++++++++++++++- 1 file changed, 120 insertions(+), 3 deletions(-) diff --git a/scripts/sb/launch_explorer.py b/scripts/sb/launch_explorer.py index 33282af75b..718922a273 100644 --- a/scripts/sb/launch_explorer.py +++ b/scripts/sb/launch_explorer.py @@ -10,6 +10,7 @@ """ import logging +import re import sys import webbrowser from pathlib import Path @@ -20,10 +21,117 @@ logger = logging.getLogger(__name__) +# Allow-list of characters permitted in a command-line file path +ALLOWED_PATH = re.compile(r"[A-Za-z0-9._/\\-]+") + + +def sanitize_path(path_str: str, label: str) -> Path: + """ + Validate an untrusted path and resolve it inside the working directory. + + Guards against path manipulation: only allow-listed characters are accepted, + absolute paths and ``..`` traversal are rejected, and the resolved path must + stay within the current working directory. + + Parameters + ---------- + path_str : str + Untrusted path, expected to be relative to the working directory. + label : str + Human-readable input name, used in error messages. + + Returns + ------- + pathlib.Path + The validated path resolved against the working directory. + + Raises + ------ + ValueError + If the path is empty, contains disallowed characters, is absolute or + contains ``..``, or resolves outside the working directory. + """ + if not path_str or not ALLOWED_PATH.fullmatch(path_str): + raise ValueError(f"Invalid characters in {label}: {path_str!r}") + path = Path(path_str) + if path.is_absolute() or ".." in path.parts: + raise ValueError(f"{label} must be relative and without '..': {path_str!r}") + base_dir = Path.cwd().resolve() + resolved = (base_dir / path).resolve() + if not resolved.is_relative_to(base_dir): + raise ValueError(f"{label} escapes the working directory: {path_str!r}") + return resolved + + +def sanitize_port(port_str: str) -> int: + """ + Parse an untrusted TCP port and ensure it lies in the valid range. + + Parameters + ---------- + port_str : str + Untrusted port value to validate. + + Returns + ------- + int + The parsed port number, guaranteed to be within 1-65535. + + Raises + ------ + ValueError + If the value is not an integer within the range 1-65535. + """ + if not (port_str.isdigit() and 1 <= (port := int(port_str)) <= 65535): + raise ValueError(f"Port must be an integer in 1-65535, got: {port_str!r}") + return port + + +def sanitize_input_files(path_strs: list[str]) -> list[str]: + """ + Validate untrusted network file paths. + + Each path must resolve inside the working directory (see ``sanitize_path``), + carry a ``.nc`` suffix, and refer to an existing file. + + Parameters + ---------- + path_strs : list[str] + Untrusted network file paths, expected to be relative to the working + directory. + + Returns + ------- + list[str] + The validated paths resolved against the working directory. + + Raises + ------ + ValueError + If any path fails validation, lacks a ``.nc`` suffix, or does not exist. + """ + files = [] + for path_str in path_strs: + path = sanitize_path(path_str, "network file") + if path.suffix != ".nc" or not path.is_file(): + raise ValueError(f"Not an existing '.nc' file: {path_str!r}") + files.append(str(path)) + return files + def import_network(fn: str): """ Import PyPSA network and set default color for 'none' carrier. + + Parameters + ---------- + fn : str + Path to the PyPSA network file to load. + + Returns + ------- + pypsa.Network + The loaded network with a default color assigned to the 'none' carrier. """ n = pypsa.Network(fn) n.carriers.loc["none", "color"] = "#000000" @@ -33,6 +141,15 @@ def import_network(fn: str): def open_browser(port): """ Opens browser with chosen port. + + Parameters + ---------- + port : int + Port on which the PyPSA-Explorer app is launched. + + Returns + ------- + None """ webbrowser.open_new(f"http://127.0.0.1:{port}") @@ -65,9 +182,9 @@ def open_browser(port): else: # Running from command line logging.basicConfig(level=logging.INFO) - output_log = sys.argv[1] - port = int(sys.argv[2]) - files = sys.argv[3:] + output_log = sanitize_path(sys.argv[1], "log path") + port = sanitize_port(sys.argv[2]) + files = sanitize_input_files(sys.argv[3:]) print("Running from command line.") # Add file handler to write to explorer_launched.log From 7d20c5d556b54ce1e3e8c7555aeaf737e8f7b61c Mon Sep 17 00:00:00 2001 From: daniel-rdt Date: Thu, 2 Jul 2026 14:58:58 +0200 Subject: [PATCH 02/11] feat: make max port as constant variable --- scripts/sb/launch_explorer.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/scripts/sb/launch_explorer.py b/scripts/sb/launch_explorer.py index 718922a273..95d871f3f4 100644 --- a/scripts/sb/launch_explorer.py +++ b/scripts/sb/launch_explorer.py @@ -24,6 +24,9 @@ # Allow-list of characters permitted in a command-line file path ALLOWED_PATH = re.compile(r"[A-Za-z0-9._/\\-]+") +# Highest valid TCP port (2**16 - 1) +MAX_PORT = 65535 + def sanitize_path(path_str: str, label: str) -> Path: """ @@ -75,15 +78,15 @@ def sanitize_port(port_str: str) -> int: Returns ------- int - The parsed port number, guaranteed to be within 1-65535. + The parsed port number, guaranteed to be within 1-``MAX_PORT``. Raises ------ ValueError - If the value is not an integer within the range 1-65535. + If the value is not an integer within the range 1-``MAX_PORT``. """ - if not (port_str.isdigit() and 1 <= (port := int(port_str)) <= 65535): - raise ValueError(f"Port must be an integer in 1-65535, got: {port_str!r}") + if not (port_str.isdigit() and 1 <= (port := int(port_str)) <= MAX_PORT): + raise ValueError(f"Port must be an integer in 1-{MAX_PORT}, got: {port_str!r}") return port From fa086038a36bd41f692e1a324d9fde9c99968012 Mon Sep 17 00:00:00 2001 From: daniel-rdt Date: Thu, 2 Jul 2026 15:06:08 +0200 Subject: [PATCH 03/11] doc: add release note --- doc/release_notes.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/doc/release_notes.rst b/doc/release_notes.rst index 05c2ce4933..300b60e413 100644 --- a/doc/release_notes.rst +++ b/doc/release_notes.rst @@ -35,6 +35,8 @@ Upcoming Open-TYNDP Release **Bugfixes and Compatibility** +* Add sanitization of cli inputs passed to launch_explorer (https://github.com/open-energy-transition/open-tyndp/pull/776). + **Documentation** * Update benchmarking documentation tables and figures for v0.7.1 (https://github.com/open-energy-transition/open-tyndp/pull/711). From 32daa617060cbb8b2cc8496223004b70d52ea9c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20R=C3=BCdt?= <117752024+daniel-rdt@users.noreply.github.com> Date: Mon, 6 Jul 2026 17:06:03 +0200 Subject: [PATCH 04/11] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Thomas Gilon Co-authored-by: Daniel Rüdt <117752024+daniel-rdt@users.noreply.github.com> --- doc/release_notes.rst | 2 +- scripts/sb/launch_explorer.py | 10 +++------- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/doc/release_notes.rst b/doc/release_notes.rst index 300b60e413..c58eef8879 100644 --- a/doc/release_notes.rst +++ b/doc/release_notes.rst @@ -35,7 +35,7 @@ Upcoming Open-TYNDP Release **Bugfixes and Compatibility** -* Add sanitization of cli inputs passed to launch_explorer (https://github.com/open-energy-transition/open-tyndp/pull/776). +* Add sanitization of CLI inputs passed to `launch_explorer` (https://github.com/open-energy-transition/open-tyndp/pull/776). **Documentation** diff --git a/scripts/sb/launch_explorer.py b/scripts/sb/launch_explorer.py index 95d871f3f4..2f999cccf7 100644 --- a/scripts/sb/launch_explorer.py +++ b/scripts/sb/launch_explorer.py @@ -85,12 +85,12 @@ def sanitize_port(port_str: str) -> int: ValueError If the value is not an integer within the range 1-``MAX_PORT``. """ - if not (port_str.isdigit() and 1 <= (port := int(port_str)) <= MAX_PORT): + if not (port_str.isdecimal() and 1 <= (port := int(port_str)) <= MAX_PORT): raise ValueError(f"Port must be an integer in 1-{MAX_PORT}, got: {port_str!r}") return port -def sanitize_input_files(path_strs: list[str]) -> list[str]: +def sanitize_input_nc_files(path_strs: list[str]) -> list[str]: """ Validate untrusted network file paths. @@ -149,10 +149,6 @@ def open_browser(port): ---------- port : int Port on which the PyPSA-Explorer app is launched. - - Returns - ------- - None """ webbrowser.open_new(f"http://127.0.0.1:{port}") @@ -187,7 +183,7 @@ def open_browser(port): logging.basicConfig(level=logging.INFO) output_log = sanitize_path(sys.argv[1], "log path") port = sanitize_port(sys.argv[2]) - files = sanitize_input_files(sys.argv[3:]) + files = sanitize_input_nc_files(sys.argv[3:]) print("Running from command line.") # Add file handler to write to explorer_launched.log From 77d174485c9a16fdf521146229bc95956f8e3549 Mon Sep 17 00:00:00 2001 From: daniel-rdt Date: Mon, 6 Jul 2026 17:15:30 +0200 Subject: [PATCH 05/11] refac: remove oudated import function --- scripts/sb/launch_explorer.py | 21 +-------------------- 1 file changed, 1 insertion(+), 20 deletions(-) diff --git a/scripts/sb/launch_explorer.py b/scripts/sb/launch_explorer.py index 2f999cccf7..0f280357b8 100644 --- a/scripts/sb/launch_explorer.py +++ b/scripts/sb/launch_explorer.py @@ -122,25 +122,6 @@ def sanitize_input_nc_files(path_strs: list[str]) -> list[str]: return files -def import_network(fn: str): - """ - Import PyPSA network and set default color for 'none' carrier. - - Parameters - ---------- - fn : str - Path to the PyPSA network file to load. - - Returns - ------- - pypsa.Network - The loaded network with a default color assigned to the 'none' carrier. - """ - n = pypsa.Network(fn) - n.carriers.loc["none", "color"] = "#000000" - return n - - def open_browser(port): """ Opens browser with chosen port. @@ -194,7 +175,7 @@ def open_browser(port): logger.addHandler(file_handler) # Load networks into a dictionary for PyPSA-Explorer - networks = {fn.split("_")[-1].split(".")[0]: import_network(fn) for fn in files} + networks = {fn.split("_")[-1].split(".")[0]: pypsa.Network(fn) for fn in files} logger.info(f"Successfully loaded {len(networks)} networks: {networks}") # Create the PyPSA-Explorer dash app From 8c7c9c976cf8aa748dd7370b6b0f94f53768b9aa Mon Sep 17 00:00:00 2001 From: daniel-rdt Date: Tue, 7 Jul 2026 09:58:37 +0200 Subject: [PATCH 06/11] fix: define MIN_PORT to exclude privileged ports --- scripts/sb/launch_explorer.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/scripts/sb/launch_explorer.py b/scripts/sb/launch_explorer.py index 0f280357b8..2dc430f375 100644 --- a/scripts/sb/launch_explorer.py +++ b/scripts/sb/launch_explorer.py @@ -24,6 +24,8 @@ # Allow-list of characters permitted in a command-line file path ALLOWED_PATH = re.compile(r"[A-Za-z0-9._/\\-]+") +# Exclude privileged ports +MIN_PORT = 1024 # Highest valid TCP port (2**16 - 1) MAX_PORT = 65535 @@ -85,8 +87,10 @@ def sanitize_port(port_str: str) -> int: ValueError If the value is not an integer within the range 1-``MAX_PORT``. """ - if not (port_str.isdecimal() and 1 <= (port := int(port_str)) <= MAX_PORT): - raise ValueError(f"Port must be an integer in 1-{MAX_PORT}, got: {port_str!r}") + if not (port_str.isdecimal() and MIN_PORT <= (port := int(port_str)) <= MAX_PORT): + raise ValueError( + f"Port must be an integer in {MIN_PORT}-{MAX_PORT}, got: {port_str!r}" + ) return port From 0c8d7f75e1d540dd991829590ec69319a65884a1 Mon Sep 17 00:00:00 2001 From: daniel-rdt Date: Tue, 7 Jul 2026 10:06:25 +0200 Subject: [PATCH 07/11] doc: improve comment --- scripts/sb/launch_explorer.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/sb/launch_explorer.py b/scripts/sb/launch_explorer.py index 2dc430f375..f54a563086 100644 --- a/scripts/sb/launch_explorer.py +++ b/scripts/sb/launch_explorer.py @@ -153,7 +153,7 @@ def open_browser(port): # Get files and output path from snakemake or command line arguments if "snakemake" in globals(): - # Running from Snakemake directly for debugging + # Running with Mock Snakemake directly for debugging # Add parent directory to path to find scripts module sys.path.insert(0, str(Path(__file__).parent.parent.parent)) from scripts._helpers import configure_logging @@ -164,7 +164,7 @@ def open_browser(port): port = snakemake.params.port print("Running from Snakemake directly.") else: - # Running from command line + # CLI entry point, allows the app to be launched as a subprocess logging.basicConfig(level=logging.INFO) output_log = sanitize_path(sys.argv[1], "log path") port = sanitize_port(sys.argv[2]) From b23efdff32cf30bfd40d4cffc77c80ba468bb0cd Mon Sep 17 00:00:00 2001 From: daniel-rdt Date: Tue, 7 Jul 2026 11:45:58 +0200 Subject: [PATCH 08/11] feat: simplify and improve path validation with resolve(strict=True) --- scripts/sb/launch_explorer.py | 31 ++++++++++++++----------------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/scripts/sb/launch_explorer.py b/scripts/sb/launch_explorer.py index f54a563086..8e23efdde2 100644 --- a/scripts/sb/launch_explorer.py +++ b/scripts/sb/launch_explorer.py @@ -10,7 +10,6 @@ """ import logging -import re import sys import webbrowser from pathlib import Path @@ -21,9 +20,6 @@ logger = logging.getLogger(__name__) -# Allow-list of characters permitted in a command-line file path -ALLOWED_PATH = re.compile(r"[A-Za-z0-9._/\\-]+") - # Exclude privileged ports MIN_PORT = 1024 # Highest valid TCP port (2**16 - 1) @@ -32,11 +28,11 @@ def sanitize_path(path_str: str, label: str) -> Path: """ - Validate an untrusted path and resolve it inside the working directory. + Resolve an untrusted path and confirm it stays inside the working directory. - Guards against path manipulation: only allow-listed characters are accepted, - absolute paths and ``..`` traversal are rejected, and the resolved path must - stay within the current working directory. + Returns the absolute, resolved path, guaranteed to exist and to lie within + the working directory. Absolute inputs, ``..`` traversal, and symlinks + pointing outside the directory are all rejected. Parameters ---------- @@ -48,23 +44,24 @@ def sanitize_path(path_str: str, label: str) -> Path: Returns ------- pathlib.Path - The validated path resolved against the working directory. + The resolved path, guaranteed to exist and lie within the working directory. Raises ------ ValueError - If the path is empty, contains disallowed characters, is absolute or - contains ``..``, or resolves outside the working directory. + If the path cannot be resolved (missing, unreadable, or a symlink loop) or + if it resolves outside the working directory. """ - if not path_str or not ALLOWED_PATH.fullmatch(path_str): - raise ValueError(f"Invalid characters in {label}: {path_str!r}") path = Path(path_str) - if path.is_absolute() or ".." in path.parts: - raise ValueError(f"{label} must be relative and without '..': {path_str!r}") base_dir = Path.cwd().resolve() - resolved = (base_dir / path).resolve() + try: + resolved = (base_dir / path).resolve(strict=True) + except OSError as e: + raise ValueError(f"Invalid {label}: {e}") from e + if not resolved.is_relative_to(base_dir): raise ValueError(f"{label} escapes the working directory: {path_str!r}") + return resolved @@ -119,7 +116,7 @@ def sanitize_input_nc_files(path_strs: list[str]) -> list[str]: """ files = [] for path_str in path_strs: - path = sanitize_path(path_str, "network file") + path = sanitize_path(path_str, "network file path") if path.suffix != ".nc" or not path.is_file(): raise ValueError(f"Not an existing '.nc' file: {path_str!r}") files.append(str(path)) From 479b120b6667f9f323ed5f3055619d14aac94f05 Mon Sep 17 00:00:00 2001 From: daniel-rdt Date: Tue, 7 Jul 2026 11:46:34 +0200 Subject: [PATCH 09/11] doc: improve log file creation with comment --- rules/sb.smk | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/rules/sb.smk b/rules/sb.smk index c6c7c121ab..a4e4e68b8e 100644 --- a/rules/sb.smk +++ b/rules/sb.smk @@ -1120,13 +1120,10 @@ rule launch_explorer: import platform import subprocess import sys - from pathlib import Path output_log = str(output[0]) input_files = list(input) - Path(output_log).touch() - # Define command line executable cmd = [ sys.executable, @@ -1137,6 +1134,7 @@ rule launch_explorer: print(params.launch_msg) + # Open logfile before Popen so the log exists when the subprocess validates its path popen_kwargs = { "stdout": open(output_log, "w"), "stderr": subprocess.STDOUT, From a121d90dae704c39f9fafe42905a0ab0fbed8879 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20R=C3=BCdt?= <117752024+daniel-rdt@users.noreply.github.com> Date: Thu, 9 Jul 2026 13:29:50 +0200 Subject: [PATCH 10/11] Apply suggestions from code review Co-authored-by: Thomas Gilon --- doc/release_notes.md | 2 +- scripts/sb/launch_explorer.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/release_notes.md b/doc/release_notes.md index 49afbb5aba..20dfc8e378 100644 --- a/doc/release_notes.md +++ b/doc/release_notes.md @@ -47,7 +47,7 @@ * Add missing regex anchors with `re.fullmatch` to `create_zenodo_deposition_cli` utils script ([778](https://github.com/open-energy-transition/open-tyndp/pull/778)). -* Add sanitization of CLI inputs passed to `launch_explorer` ([776](https://github.com/open-energy-transition/open-tyndp/pull/776)). +* Add sanitization of CLI inputs passed to `launch_explorer` ([#776](https://github.com/open-energy-transition/open-tyndp/pull/776)). ## Upcoming PyPSA-Eur Release diff --git a/scripts/sb/launch_explorer.py b/scripts/sb/launch_explorer.py index 8e23efdde2..963c5d33c1 100644 --- a/scripts/sb/launch_explorer.py +++ b/scripts/sb/launch_explorer.py @@ -161,7 +161,7 @@ def open_browser(port): port = snakemake.params.port print("Running from Snakemake directly.") else: - # CLI entry point, allows the app to be launched as a subprocess + # CLI entry point for Snakemake, allows the app to be launched as a subprocess logging.basicConfig(level=logging.INFO) output_log = sanitize_path(sys.argv[1], "log path") port = sanitize_port(sys.argv[2]) From 51b65940465a5971ba64852f260b0ec2cfbc77ef Mon Sep 17 00:00:00 2001 From: daniel-rdt Date: Thu, 9 Jul 2026 13:33:20 +0200 Subject: [PATCH 11/11] doc: update docstring to reflect MIN_PORT --- scripts/sb/launch_explorer.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/sb/launch_explorer.py b/scripts/sb/launch_explorer.py index 963c5d33c1..49f207e5ef 100644 --- a/scripts/sb/launch_explorer.py +++ b/scripts/sb/launch_explorer.py @@ -77,12 +77,12 @@ def sanitize_port(port_str: str) -> int: Returns ------- int - The parsed port number, guaranteed to be within 1-``MAX_PORT``. + The parsed port number, guaranteed to be within ``MIN_PORT``-``MAX_PORT``. Raises ------ ValueError - If the value is not an integer within the range 1-``MAX_PORT``. + If the value is not an integer within the range ``MIN_PORT``-``MAX_PORT``. """ if not (port_str.isdecimal() and MIN_PORT <= (port := int(port_str)) <= MAX_PORT): raise ValueError(