diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b97bf65..2c7ae6b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -30,7 +30,7 @@ jobs: # Create logs directory with absolute path mkdir -p "$GITHUB_WORKSPACE/logs" - chmod 777 "$GITHUB_WORKSPACE/logs" + chmod 755 "$GITHUB_WORKSPACE/logs" # Generate log file name TIMESTAMP=$(date +"%Y%m%dT%H%M") @@ -44,7 +44,7 @@ jobs: # Create and verify log file touch "${LOG_FILE}" - chmod 666 "${LOG_FILE}" + chmod 644 "${LOG_FILE}" # Write initial content echo "Starting CI run at $(date)" | tee -a "${LOG_FILE}" @@ -89,7 +89,10 @@ jobs: conda install -y -c conda-forge pytest pytest-cov pytest-xdist pluggy coverage execnet 2>&1 | tee -a "${LOG_FILE}" echo "" | tee -a "${LOG_FILE}" echo "Installing ruff and mypy..." | tee -a "${LOG_FILE}" - pip install ruff==0.8.4 mypy 2>&1 | tee -a "${LOG_FILE}" + # mypy pinned to match .pre-commit-config.yaml so CI agrees with local + # hooks. Must be >= the dev floor (mypy>=1.15.0); older versions report + # false positives the codebase is not written against. + pip install ruff==0.8.4 mypy==2.1.0 2>&1 | tee -a "${LOG_FILE}" echo "" | tee -a "${LOG_FILE}" echo "Installing mpasdiag..." | tee -a "${LOG_FILE}" pip install -e . 2>&1 | tee -a "${LOG_FILE}" @@ -244,3 +247,22 @@ jobs: - name: Lint with ruff run: | ruff check mpasdiag/ tests/ + + security: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + + - name: Set up Python + uses: actions/setup-python@ece7cb06caefa5fff74198d8649806c4678c61a1 # v6.3.0 + with: + python-version: '3.11' + + - name: Install security tooling + run: pip install bandit==1.8.0 pip-audit==2.7.3 + + - name: Static analysis with bandit + run: bandit -r mpasdiag/ -ll + + - name: Audit dependencies for known vulnerabilities + run: pip-audit --progress-spinner=off || true diff --git a/.gitignore b/.gitignore index c201c5c..920ddfb 100644 --- a/.gitignore +++ b/.gitignore @@ -57,3 +57,9 @@ examples/benchmark_results_*.csv # Misc *.pyc + +# Internal security audit documents (DO NOT COMMIT / PUBLISH) +SECURITY_AUDIT.md +SECURITY_AUDIT_*.md +security_audit*.md +scripts/ diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 9c89056..2549e42 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -30,8 +30,16 @@ repos: files: ^(mpasdiag|tests)/ - repo: https://github.com/pre-commit/mirrors-mypy - rev: v1.13.0 + rev: v2.1.0 hooks: - id: mypy files: ^mpasdiag/ additional_dependencies: ['types-PyYAML'] + + # Static security analysis (security audit finding MPAS-004). + - repo: https://github.com/PyCQA/bandit + rev: 1.8.0 + hooks: + - id: bandit + args: ['-ll'] + files: ^mpasdiag/ diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 952ac4a..697afa2 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -107,6 +107,7 @@ also enforced in CI, so please run them before pushing: ruff check mpasdiag/ tests/ # linting black --check mpasdiag/ tests/ # formatting (use `black mpasdiag/ tests/` to apply) mypy mpasdiag/ # static type checking +bandit -r mpasdiag/ -ll # static security analysis ``` Guidelines: @@ -114,6 +115,10 @@ Guidelines: - Add type annotations to all new functions (mypy runs in strict-ish mode). - Write clear docstrings (NumPy style) for public functions and classes. - Keep changes consistent with the surrounding code. +- **Never use `assert` for input validation or any security/correctness check.** + Python strips `assert` statements when run under `python -O`, so a validation + written as an assertion silently disappears. Use an explicit `raise` (e.g. + `ValueError`) instead. Reserve `assert` for internal invariants only. ## Testing diff --git a/MANIFEST.in b/MANIFEST.in index 944d410..8d30112 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -23,6 +23,7 @@ prune tests prune examples prune data prune output +prune scripts prune .github # Exclude caches and compiled files diff --git a/README.md b/README.md index b3b5eba..439a433 100644 --- a/README.md +++ b/README.md @@ -1019,4 +1019,4 @@ If you use this package in your research, please cite: --- **Version**: 1.0.0 -**Last Updated**: June 06, 2026 +**Last Updated**: June 28, 2026 diff --git a/SECURITY.md b/SECURITY.md index 80b0f35..a7858a2 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -9,6 +9,50 @@ Security updates are provided for the most recent release line of MPASdiag. | 1.0.x | :white_check_mark: | | < 1.0 | :x: | +## Security considerations for users + +MPASdiag is a **local command-line tool and Python library**. It runs with the +privileges of the user who invokes it and is not a network service. Please keep +the following trust boundaries in mind: + +### Only process data you trust + +MPASdiag reads NetCDF/HDF5 model output, grid files, and pre-computed remapping +weight caches. Parsing of these binary files is delegated to the underlying +`netCDF4`, `h5netcdf`/`HDF5`, and `xarray` libraries. As with any scientific data +tool, **opening a maliciously crafted or corrupt file can crash the process or, +through a vulnerability in those underlying libraries, be unsafe.** Only process +files obtained from sources you trust, and keep your scientific stack updated. + +As a defense-in-depth measure, MPASdiag rejects inputs whose declared dimensions +exceed generous safety limits before allocating large arrays, to avoid +out-of-memory crashes on malformed files. If you legitimately work with very +large grids, you can raise these limits via environment variables: +`MPASDIAG_MAX_SOURCE_CELLS`, `MPASDIAG_MAX_TARGET_POINTS`, +`MPASDIAG_MAX_WEIGHTS_NNZ`, and `MPASDIAG_MAX_NUM_POINTS`. + +The remapping **weights cache directory** (`weights_dir`) is treated as trusted +input: only point it at a location you control. A tampered cache file is +validated for internal consistency before use, but should not be shared across +trust boundaries. + +### Output, log, and config paths + +Output directories, log files (`--log-file`), configuration files (`--config`), +and weights paths are taken from the operator and are honored as given — this is +intended behavior for a local tool that writes to your own filesystem. +Configuration paths are confined to the working directory (or an explicit +`base_dir`) and must be `.yaml`/`.yml` files; this guards against accidental +path traversal, not against a user who deliberately targets their own files. + +### Reproducible installation + +Lower bounds in `pyproject.toml`/`requirements.txt` are set above known-vulnerable +releases, but `pip` will otherwise resolve to the latest compatible versions. For +a fully reproducible environment, install into the provided conda `environment.yml`, +or generate a pinned constraints file from a known-good environment +(`pip freeze > constraints.txt`) and install with `pip install mpasdiag -c constraints.txt`. + ## Reporting a vulnerability We take the security of MPASdiag seriously. If you discover a security diff --git a/data/grids/x1.10242.static.nc b/data/grids/x1.10242.static.nc index 89e32e1..d583d5c 100644 Binary files a/data/grids/x1.10242.static.nc and b/data/grids/x1.10242.static.nc differ diff --git a/data/u240k/diag/diag.2024-09-17_00.00.00.nc b/data/u240k/diag/diag.2024-09-17_00.00.00.nc index 1c6c5b1..265af82 100644 Binary files a/data/u240k/diag/diag.2024-09-17_00.00.00.nc and b/data/u240k/diag/diag.2024-09-17_00.00.00.nc differ diff --git a/data/u240k/diag/diag.2024-09-17_01.00.00.nc b/data/u240k/diag/diag.2024-09-17_01.00.00.nc index b173601..60c5a01 100644 Binary files a/data/u240k/diag/diag.2024-09-17_01.00.00.nc and b/data/u240k/diag/diag.2024-09-17_01.00.00.nc differ diff --git a/data/u240k/diag/diag.2024-09-17_02.00.00.nc b/data/u240k/diag/diag.2024-09-17_02.00.00.nc index 0ab1faa..1b19d9e 100644 Binary files a/data/u240k/diag/diag.2024-09-17_02.00.00.nc and b/data/u240k/diag/diag.2024-09-17_02.00.00.nc differ diff --git a/data/u240k/diag/diag.2024-09-17_03.00.00.nc b/data/u240k/diag/diag.2024-09-17_03.00.00.nc index d112577..7c254f0 100644 Binary files a/data/u240k/diag/diag.2024-09-17_03.00.00.nc and b/data/u240k/diag/diag.2024-09-17_03.00.00.nc differ diff --git a/data/u240k/diag/diag.2024-09-17_04.00.00.nc b/data/u240k/diag/diag.2024-09-17_04.00.00.nc index 7228ecf..7ce91e7 100644 Binary files a/data/u240k/diag/diag.2024-09-17_04.00.00.nc and b/data/u240k/diag/diag.2024-09-17_04.00.00.nc differ diff --git a/data/u240k/diag/diag.2024-09-17_05.00.00.nc b/data/u240k/diag/diag.2024-09-17_05.00.00.nc index 1cc4850..3a2ec45 100644 Binary files a/data/u240k/diag/diag.2024-09-17_05.00.00.nc and b/data/u240k/diag/diag.2024-09-17_05.00.00.nc differ diff --git a/data/u240k/diag/diag.2024-09-17_06.00.00.nc b/data/u240k/diag/diag.2024-09-17_06.00.00.nc index b30d89d..9cecf79 100644 Binary files a/data/u240k/diag/diag.2024-09-17_06.00.00.nc and b/data/u240k/diag/diag.2024-09-17_06.00.00.nc differ diff --git a/data/u240k/mpasout/mpasout.2024-09-17_00.00.00.nc b/data/u240k/mpasout/mpasout.2024-09-17_00.00.00.nc index d0a2819..284e590 100644 Binary files a/data/u240k/mpasout/mpasout.2024-09-17_00.00.00.nc and b/data/u240k/mpasout/mpasout.2024-09-17_00.00.00.nc differ diff --git a/data/u240k/mpasout/mpasout.2024-09-17_01.00.00.nc b/data/u240k/mpasout/mpasout.2024-09-17_01.00.00.nc index 3beed13..ef09529 100644 Binary files a/data/u240k/mpasout/mpasout.2024-09-17_01.00.00.nc and b/data/u240k/mpasout/mpasout.2024-09-17_01.00.00.nc differ diff --git a/data/u240k/mpasout/mpasout.2024-09-17_02.00.00.nc b/data/u240k/mpasout/mpasout.2024-09-17_02.00.00.nc index db008d6..c592f21 100644 Binary files a/data/u240k/mpasout/mpasout.2024-09-17_02.00.00.nc and b/data/u240k/mpasout/mpasout.2024-09-17_02.00.00.nc differ diff --git a/data/u240k/mpasout/mpasout.2024-09-17_03.00.00.nc b/data/u240k/mpasout/mpasout.2024-09-17_03.00.00.nc index c333e03..2b8cfdb 100644 Binary files a/data/u240k/mpasout/mpasout.2024-09-17_03.00.00.nc and b/data/u240k/mpasout/mpasout.2024-09-17_03.00.00.nc differ diff --git a/data/u240k/mpasout/mpasout.2024-09-17_04.00.00.nc b/data/u240k/mpasout/mpasout.2024-09-17_04.00.00.nc index 7231e40..775b1f3 100644 Binary files a/data/u240k/mpasout/mpasout.2024-09-17_04.00.00.nc and b/data/u240k/mpasout/mpasout.2024-09-17_04.00.00.nc differ diff --git a/data/u240k/mpasout/mpasout.2024-09-17_05.00.00.nc b/data/u240k/mpasout/mpasout.2024-09-17_05.00.00.nc index 628ebfe..22a402e 100644 Binary files a/data/u240k/mpasout/mpasout.2024-09-17_05.00.00.nc and b/data/u240k/mpasout/mpasout.2024-09-17_05.00.00.nc differ diff --git a/data/u240k/mpasout/mpasout.2024-09-17_06.00.00.nc b/data/u240k/mpasout/mpasout.2024-09-17_06.00.00.nc index 009ea8e..ee056c2 100644 Binary files a/data/u240k/mpasout/mpasout.2024-09-17_06.00.00.nc and b/data/u240k/mpasout/mpasout.2024-09-17_06.00.00.nc differ diff --git a/mpasdiag/processing/constants.py b/mpasdiag/processing/constants.py index 4adcbf8..15df0b2 100644 --- a/mpasdiag/processing/constants.py +++ b/mpasdiag/processing/constants.py @@ -69,6 +69,15 @@ Rv_OVER_Rd = 1.608 # Ratio of gas constants: water vapour / dry air EPSILON_RD_RV = 0.622 # Ratio of molar masses: Rd / Rv +# --------------------------------------------------------------------------- +# Untrusted-input safety limits (security audit finding MPAS-001) +# --------------------------------------------------------------------------- + +MAX_SOURCE_CELLS = 500_000_000 # source mesh cells / points +MAX_TARGET_POINTS = 500_000_000 # target lat-lon grid points (n_lat * n_lon) +MAX_WEIGHTS_NNZ = 2_000_000_000 # non-zero remap weight entries +MAX_NUM_POINTS = 1_000_000 # interpolation points along a cross-section transect + # --------------------------------------------------------------------------- # Dimension names # --------------------------------------------------------------------------- diff --git a/mpasdiag/processing/remapping.py b/mpasdiag/processing/remapping.py index 9b1523b..d5e054b 100644 --- a/mpasdiag/processing/remapping.py +++ b/mpasdiag/processing/remapping.py @@ -22,6 +22,8 @@ from typing import Any, Optional, Union, List, Tuple from .utils_logger import get_logger +from .utils_validator import DataValidator +from .utils_path import safe_resolve_within logger = get_logger(__name__) @@ -227,6 +229,10 @@ def create_target_grid( lon = np.arange(lon_min, lon_max + dlon / 2, dlon) lat = np.arange(lat_min, lat_max + dlat / 2, dlat) + DataValidator.enforce_size_limits( + n_tgt=len(lon) * len(lat), context="creating the target grid" + ) + target_grid = xr.Dataset( { "lon": xr.DataArray(lon, dims=["lon"]), @@ -335,14 +341,16 @@ def _resolve_weights_path( Returns: Optional[Path]: The resolved file path for caching weights, or None if caching is not configured. """ - if self.weights_dir is not None and filename is None: + if self.weights_dir is None: + return None + + if filename is None: src_shape = len(source_grid["lon"]) tgt_shape = f"{len(target_grid['lon'])}x{len(target_grid['lat'])}" filename = f"weights_{self.method}_{src_shape}to{tgt_shape}.nc" - return ( - self.weights_dir / filename - if self.weights_dir is not None and filename is not None - else None + + return safe_resolve_within( + filename, self.weights_dir, allowed_suffixes=(".nc",) ) def _try_load_cached_weights( @@ -1244,23 +1252,59 @@ def _load_weights_netcdf( Tuple[Any, Tuple[int, int], Optional[np.ndarray]]: A tuple containing the sparse weight matrix as a scipy.sparse.csr_matrix, the shape of the target grid as a tuple (n_lat, n_lon), and an optional array mapping mesh elements to their parent cells if it was included in the dataset. """ ds = xr.open_dataset(path) + try: + row = ds["row"].values.astype(np.intp) - 1 + col = ds["col"].values.astype(np.intp) - 1 + vals = ds["S"].values.astype(np.float64) - row = ds["row"].values.astype(np.intp) - 1 - col = ds["col"].values.astype(np.intp) - 1 - vals = ds["S"].values.astype(np.float64) + n_src = int(ds.attrs["n_src"]) + n_dst = int(ds.attrs["n_dst"]) - n_src = int(ds.attrs["n_src"]) - n_dst = int(ds.attrs["n_dst"]) + tgt_shape = (int(ds.attrs["shape_tgt_lat"]), int(ds.attrs["shape_tgt_lon"])) - tgt_shape = (int(ds.attrs["shape_tgt_lat"]), int(ds.attrs["shape_tgt_lon"])) + cell_of_element: Optional[np.ndarray] = ( + ds["cell_of_element"].values.astype(np.int64) + if "cell_of_element" in ds + else None + ) + finally: + ds.close() + + nnz = int(vals.shape[0]) - cell_of_element: Optional[np.ndarray] = ( - ds["cell_of_element"].values.astype(np.int64) - if "cell_of_element" in ds - else None + DataValidator.enforce_size_limits( + n_src=n_src, + n_tgt=n_dst, + nnz=nnz, + context=f"loading cached weights from {path.name}", ) - ds.close() + if n_src <= 0 or n_dst <= 0: + raise ValueError( + f"Invalid weights cache '{path.name}': non-positive dimensions " + f"(n_src={n_src}, n_dst={n_dst})" + ) + + if tgt_shape[0] * tgt_shape[1] != n_dst: + raise ValueError( + f"Inconsistent weights cache '{path.name}': target shape " + f"{tgt_shape} does not match n_dst={n_dst}" + ) + + if not (row.shape[0] == col.shape[0] == vals.shape[0]): + raise ValueError( + f"Corrupt weights cache '{path.name}': row/col/value lengths " + f"differ ({row.shape[0]}, {col.shape[0]}, {vals.shape[0]})" + ) + + if nnz and ( + row.min() < 0 or row.max() >= n_dst or col.min() < 0 or col.max() >= n_src + ): + raise ValueError( + f"Corrupt weights cache '{path.name}': row/col indices fall " + f"outside the declared ({n_dst}, {n_src}) matrix bounds" + ) + weight_matrix = coo_matrix((vals, (row, col)), shape=(n_dst, n_src)).tocsr() return weight_matrix, tgt_shape, cell_of_element diff --git a/mpasdiag/processing/utils_config.py b/mpasdiag/processing/utils_config.py index 6c2cd20..22bed50 100644 --- a/mpasdiag/processing/utils_config.py +++ b/mpasdiag/processing/utils_config.py @@ -20,6 +20,7 @@ from dataclasses import dataclass, asdict from .utils_logger import get_logger +from .utils_path import safe_resolve_within logger = get_logger(__name__) @@ -41,19 +42,12 @@ def _validate_config_path( Returns: Path: The resolved, validated absolute path. """ - base = (Path(base_dir) if base_dir else Path.cwd()).resolve() - resolved = (base / filepath).resolve() - - if not resolved.is_relative_to(base): - raise ValueError(f"Refusing to access config path outside '{base}': {filepath}") - - if resolved.suffix.lower() not in (".yaml", ".yml"): - raise ValueError(f"Config file must be a .yaml or .yml file: {filepath}") - - if must_exist and not resolved.is_file(): - raise FileNotFoundError(f"Configuration file not found: {filepath}") - - return resolved + return safe_resolve_within( + filepath, + base_dir, + allowed_suffixes=(".yaml", ".yml"), + must_exist=must_exist, + ) @dataclass diff --git a/mpasdiag/processing/utils_path.py b/mpasdiag/processing/utils_path.py new file mode 100644 index 0000000..c01e5bc --- /dev/null +++ b/mpasdiag/processing/utils_path.py @@ -0,0 +1,56 @@ +#!/usr/bin/env python3 +# -*- coding: utf-8 -*- +# SPDX-License-Identifier: MIT + +""" +MPASdiag Core Processing Module: Path Containment Utilities + +This module provides a single, dependency-free helper for safely resolving a user- or caller-supplied file path against a trusted base directory. It guards against path-traversal escapes (e.g. ``'../../etc/passwd'`` or absolute paths outside the working tree) and optionally enforces an allowed file-extension set. It is shared by the configuration loader and the remapping weights cache so that both use the same proven containment logic (security audit findings MPAS-005). This module intentionally imports nothing from the rest of the package, keeping it free of import cycles so any module may depend on it. + +Author: Rubaiat Islam +Institution: Mesoscale & Microscale Meteorology Laboratory, NCAR +Email: mrislam@ucar.edu +Date: November 2025 +Version: 1.0.0 +""" + +from pathlib import Path +from typing import Iterable, Optional, Union + + +def safe_resolve_within( + filepath: Union[str, Path], + base_dir: Optional[Union[str, Path]], + *, + allowed_suffixes: Optional[Iterable[str]] = None, + must_exist: bool = False, +) -> Path: + """ + This function resolves a user-supplied file path against a trusted base directory, ensuring that the resolved path does not escape the base directory and optionally enforcing allowed file extensions. It also checks for the existence of the file if required. + + Parameters: + filepath (Union[str, Path]): Raw path supplied by the caller (relative or absolute). + base_dir (Optional[Union[str, Path]]): Directory the path must stay within. Defaults to the current working directory when None. + allowed_suffixes (Optional[Iterable[str]]): If given, the resolved path's suffix (case-insensitive) must be one of these (e.g. ``(".yaml", ".yml")``). + must_exist (bool): When True, require the resolved path to be an existing regular file. + + Returns: + Path: The resolved, validated absolute path. + """ + base = (Path(base_dir) if base_dir else Path.cwd()).resolve() + resolved = (base / filepath).resolve() + + if not resolved.is_relative_to(base): + raise ValueError(f"Refusing to access path outside '{base}': {filepath}") + + if allowed_suffixes is not None: + allowed = {suffix.lower() for suffix in allowed_suffixes} + if resolved.suffix.lower() not in allowed: + raise ValueError( + f"Path must have one of {sorted(allowed)} extensions: {filepath}" + ) + + if must_exist and not resolved.is_file(): + raise FileNotFoundError(f"File not found: {filepath}") + + return resolved diff --git a/mpasdiag/processing/utils_validator.py b/mpasdiag/processing/utils_validator.py index 3c9c23a..fbfaf24 100644 --- a/mpasdiag/processing/utils_validator.py +++ b/mpasdiag/processing/utils_validator.py @@ -14,13 +14,96 @@ Version: 1.0.0 """ +import os import numpy as np from typing import Dict, Any, Optional +from .constants import ( + MAX_SOURCE_CELLS, + MAX_TARGET_POINTS, + MAX_WEIGHTS_NNZ, + MAX_NUM_POINTS, +) + class DataValidator: """Data validation utilities for MPAS coordinate arrays and numerical data quality assurance with comprehensive sanity checking capabilities.""" + @staticmethod + def _resolve_size_limit(env_var: str, default: int) -> int: + """ + This method resolves the effective size limit for a given parameter by checking if an environment variable is set to override the compiled-in default. If the environment variable is set, it attempts to convert its value to an integer and checks if it is a positive integer. If the conversion fails or the value is not positive, a ValueError is raised. If the environment variable is not set, the method returns the default limit. + + Parameters: + env_var (str): Name of the MPASDIAG_MAX_* environment variable that may override the compiled-in default. + default (int): Default limit from constants.py to use when the environment variable is unset. + + Returns: + int: The effective positive integer limit. + """ + raw = os.environ.get(env_var) + if raw is None: + return default + try: + value = int(raw) + except (TypeError, ValueError): + raise ValueError(f"{env_var} must be a positive integer (got {raw!r})") + if value <= 0: + raise ValueError(f"{env_var} must be a positive integer (got {raw!r})") + return value + + @staticmethod + def enforce_size_limits( + *, + n_src: Optional[int] = None, + n_tgt: Optional[int] = None, + nnz: Optional[int] = None, + num_points: Optional[int] = None, + context: str = "", + ) -> None: + """ + This method enforces safety limits on various parameters related to MPAS data processing, such as the number of source mesh cells, target grid points, non-zero remap weight entries, and cross-section interpolation points. It checks if the provided values are negative or exceed their respective safety limits, which can be overridden by environment variables. If any value is invalid, a ValueError is raised with a descriptive message indicating the issue and suggesting how to raise the limit if the input is trusted. + + Parameters: + n_src (Optional[int]): Number of source mesh cells / points, checked against MAX_SOURCE_CELLS (env MPASDIAG_MAX_SOURCE_CELLS). + n_tgt (Optional[int]): Number of target grid points, checked against MAX_TARGET_POINTS (env MPASDIAG_MAX_TARGET_POINTS). + nnz (Optional[int]): Number of non-zero remap weight entries, checked against MAX_WEIGHTS_NNZ (env MPASDIAG_MAX_WEIGHTS_NNZ). + num_points (Optional[int]): Number of cross-section interpolation points, checked against MAX_NUM_POINTS (env MPASDIAG_MAX_NUM_POINTS). + context (str): Optional short description of the operation, included in error messages for clarity. + + Returns: + None + """ + checks = ( + (n_src, "MPASDIAG_MAX_SOURCE_CELLS", MAX_SOURCE_CELLS, "source grid cells"), + ( + n_tgt, + "MPASDIAG_MAX_TARGET_POINTS", + MAX_TARGET_POINTS, + "target grid points", + ), + (nnz, "MPASDIAG_MAX_WEIGHTS_NNZ", MAX_WEIGHTS_NNZ, "remap weight entries"), + ( + num_points, + "MPASDIAG_MAX_NUM_POINTS", + MAX_NUM_POINTS, + "cross-section points", + ), + ) + where = f" while {context}" if context else "" + for value, env_var, default, label in checks: + if value is None: + continue + if value < 0: + raise ValueError(f"Invalid negative {label}: {value}") + limit = DataValidator._resolve_size_limit(env_var, default) + if value > limit: + raise ValueError( + f"{label} ({int(value):,}) exceeds the safety limit " + f"({limit:,}){where}. If this input is trusted, raise the " + f"limit via the {env_var} environment variable." + ) + @staticmethod def validate_coordinates(lon: np.ndarray, lat: np.ndarray) -> bool: """ diff --git a/mpasdiag/visualization/cross_section.py b/mpasdiag/visualization/cross_section.py index 86c7189..862268b 100644 --- a/mpasdiag/visualization/cross_section.py +++ b/mpasdiag/visualization/cross_section.py @@ -35,6 +35,7 @@ from ..processing.utils_unit import UnitConverter from ..processing.utils_metadata import MPASFileMetadata from ..processing.utils_logger import get_logger +from ..processing.utils_validator import DataValidator logger = get_logger(__name__) @@ -594,6 +595,10 @@ def create_vertical_cross_section( "save": 0.0, } + DataValidator.enforce_size_limits( + num_points=num_points, context="building the cross-section transect" + ) + cross_section_data = self._generate_cross_section_data( mpas_3d_processor, var_name, diff --git a/pyproject.toml b/pyproject.toml index d2df91e..2ac9401 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -34,22 +34,25 @@ keywords = [ "weather prediction", ] dependencies = [ - "numpy>=1.20.0", - "scipy>=1.7.0", + # Lower bounds are deliberately set above the newest *known-vulnerable* release + # of each security-relevant dependency (audit finding MPAS-003). CI installs the + # latest versions, so these floors only constrain the minimum a user may resolve. + "numpy>=1.22.0", # >=1.22 clears CVE-2021-33430/34141/41495/41496 + "scipy>=1.8.0", "xarray>=0.19.0", - "pandas>=1.3.0", + "pandas>=1.4.0", # matplotlib capped <3.11: cartopy<=0.25.0 is incompatible with matplotlib 3.11 "matplotlib>=3.5.0,<3.11", - "cartopy>=0.20.0", - "netCDF4>=1.5.0", - "h5netcdf>=1.0.0", - "dask>=2021.6.0", + "cartopy>=0.21.0", + "netCDF4>=1.6.0", # bundles patched libhdf5/libnetcdf (MPAS-001) + "h5netcdf>=1.1.0", + "dask>=2021.10.0", # >=2021.10.0 clears CVE-2021-42343 (distributed RCE) "numba>=0.55.0", "llvmlite>=0.38.0", "mpi4py>=3.1.0", "uxarray>=2024.01.0", - "PyYAML>=5.4.0", - "psutil>=5.8.0", + "PyYAML>=6.0", # loading uses yaml.safe_load; floor raised for hygiene + "psutil>=5.8.0", # >=5.6.6 clears CVE-2019-18874 (double free) "metpy>=1.4.0", ] @@ -70,6 +73,9 @@ dev = [ "mypy>=1.15.0", "sphinx>=4.0", "sphinx-rtd-theme>=0.5", + # Security tooling (audit finding MPAS-004) + "bandit>=1.7.0", + "pip-audit>=2.6.0", ] performance = [ "bottleneck>=1.3.0", @@ -89,7 +95,12 @@ mpasdiag = "mpasdiag.cli:main" "Documentation" = "https://github.com/mrixlam/MPASdiag#readme" [tool.uv.sources] -esmpy = { git = "https://github.com/esmf-org/esmf", subdirectory = "src/addon/esmpy" } +# esmpy is not published on PyPI for all Python versions, so it is fetched from the +# upstream ESMF repository. The ref is PINNED to an immutable release tag (audit +# finding MPAS-002): an unpinned default-branch checkout would execute arbitrary, +# non-reproducible upstream build code at install time. Bump deliberately and +# re-verify when upgrading. Prefer `conda install -c conda-forge esmpy` where possible. +esmpy = { git = "https://github.com/esmf-org/esmf", subdirectory = "src/addon/esmpy", rev = "v8.9.1" } [tool.setuptools] include-package-data = true diff --git a/requirements.txt b/requirements.txt index e292328..6d5e8de 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,13 +1,17 @@ # MPASdiag Package Dependencies +# +# Lower bounds are kept in sync with pyproject.toml and are deliberately set above +# the newest known-vulnerable release of each security-relevant dependency +# (security audit findings MPAS-001/003). # Core scientific computing -numpy>=1.20.0 -scipy>=1.7.0 +numpy>=1.22.0 +scipy>=1.8.0 xarray>=0.19.0 -pandas>=1.3.0 -netCDF4>=1.5.0 -h5netcdf>=1.0.0 -dask>=2021.6.0 +pandas>=1.4.0 +netCDF4>=1.6.0 +h5netcdf>=1.1.0 +dask>=2021.10.0 # Performance acceleration numba>=0.55.0 @@ -22,11 +26,11 @@ uxarray>=2024.01.0 # Visualization libraries # matplotlib capped <3.11: cartopy<=0.25.0 is incompatible with matplotlib 3.11 matplotlib>=3.5.0,<3.11 -cartopy>=0.20.0 +cartopy>=0.21.0 seaborn>=0.11.0 # Configuration and utilities -PyYAML>=5.4.0 +PyYAML>=6.0 psutil>=5.8.0 # Testing dependencies diff --git a/tests/processing/test_security_limits.py b/tests/processing/test_security_limits.py new file mode 100644 index 0000000..ccab1be --- /dev/null +++ b/tests/processing/test_security_limits.py @@ -0,0 +1,294 @@ +#!/usr/bin/env python3 +# -*- coding: utf-8 -*- +# SPDX-License-Identifier: MIT + +""" +Tests for the security-hardening helpers added in the pre-release audit: + +This module contains unit tests for the security-hardening helpers introduced in the pre-release audit of the MPASdiag package. The tests focus on verifying the functionality of the enforce_size_limits function and the safe_resolve_within function, ensuring that they correctly enforce input size limits and path containment, respectively. +""" + +import pytest + +from mpasdiag.processing.utils_validator import DataValidator +from mpasdiag.processing.utils_path import safe_resolve_within +from mpasdiag.processing import constants +from pathlib import Path + + +class TestEnforceSizeLimits: + """Generous, configurable input-size caps (MPAS-001).""" + + def test_within_limits_passes(self: "TestEnforceSizeLimits") -> None: + """ + This test verifies that the enforce_size_limits function does not raise an exception when the provided values for source grid cells, target grid points, non-zero remap weight entries, and cross-section interpolation points are all well below their respective default limits. It ensures that valid inputs are accepted without any issues. + + Parameters: + None + + Returns: + None + """ + # Well below every default limit -> no exception. + DataValidator.enforce_size_limits( + n_src=1000, n_tgt=2000, nnz=5000, num_points=100 + ) + + def test_none_values_are_skipped(self: "TestEnforceSizeLimits") -> None: + """ + This test checks that the enforce_size_limits function correctly handles None values for its parameters. When None is passed for any of the parameters (n_src, n_tgt, nnz, num_points), the function should skip the corresponding checks and not raise any exceptions. This allows callers to only specify the dimensions relevant to their allocation without being forced to provide values for all parameters. + + Parameters: + None + + Returns: + None + """ + # Passing nothing relevant must never raise. + DataValidator.enforce_size_limits() + + def test_source_cells_over_default_raises(self: "TestEnforceSizeLimits") -> None: + """ + This test verifies that the enforce_size_limits function raises a ValueError when the number of source grid cells (n_src) exceeds the default limit defined in constants.MAX_SOURCE_CELLS. It ensures that the function correctly enforces the safety limit for source grid cells and provides an appropriate error message indicating the issue. + + Parameters: + None + + Returns: + None + """ + with pytest.raises(ValueError, match="source grid cells"): + DataValidator.enforce_size_limits(n_src=constants.MAX_SOURCE_CELLS + 1) + + def test_target_points_over_default_raises(self: "TestEnforceSizeLimits") -> None: + """ + This test checks that the enforce_size_limits function raises a ValueError when the number of target grid points (n_tgt) exceeds the default limit defined in constants.MAX_TARGET_POINTS. It ensures that the function correctly enforces the safety limit for target grid points and provides an appropriate error message indicating the issue. + + Parameters: + None + + Returns: + None + """ + with pytest.raises(ValueError, match="target grid points"): + DataValidator.enforce_size_limits(n_tgt=constants.MAX_TARGET_POINTS + 1) + + def test_nnz_over_default_raises(self: "TestEnforceSizeLimits") -> None: + """ + This test verifies that the enforce_size_limits function raises a ValueError when the number of non-zero remap weight entries (nnz) exceeds the default limit defined in constants.MAX_WEIGHTS_NNZ. It ensures that the function correctly enforces the safety limit for non-zero remap weight entries and provides an appropriate error message indicating the issue. + + Parameters: + None + + Returns: + None + """ + with pytest.raises(ValueError, match="weight entries"): + DataValidator.enforce_size_limits(nnz=constants.MAX_WEIGHTS_NNZ + 1) + + def test_num_points_over_default_raises(self: "TestEnforceSizeLimits") -> None: + """ + This test checks that the enforce_size_limits function raises a ValueError when the number of cross-section interpolation points (num_points) exceeds the default limit defined in constants.MAX_NUM_POINTS. It ensures that the function correctly enforces the safety limit for cross-section interpolation points and provides an appropriate error message indicating the issue. + + Parameters: + None + + Returns: + None + """ + with pytest.raises(ValueError, match="cross-section points"): + DataValidator.enforce_size_limits(num_points=constants.MAX_NUM_POINTS + 1) + + def test_negative_value_raises(self: "TestEnforceSizeLimits") -> None: + """ + This test checks that the enforce_size_limits function raises a ValueError when a negative value is provided for the number of source grid points (n_src). It ensures that the function correctly enforces the safety limit for non-negative values and provides an appropriate error message indicating the issue. + + Parameters: + None + + Returns: + None + """ + with pytest.raises(ValueError, match="negative"): + DataValidator.enforce_size_limits(n_src=-1) + + def test_env_override_relaxes_limit( + self: "TestEnforceSizeLimits", monkeypatch: pytest.MonkeyPatch + ) -> None: + """ + This test checks that the enforce_size_limits function allows an override of the default limit for the number of source grid points (n_src) through an environment variable (MPASDIAG_MAX_SOURCE_CELLS). It ensures that the function correctly respects the environment variable and allows values above the default limit when the override is set. + + Parameters: + monkeypatch: pytest.MonkeyPatch + + Returns: + None + """ + # An override above the requested size lets an otherwise-rejected input pass. + monkeypatch.setenv( + "MPASDIAG_MAX_SOURCE_CELLS", str(constants.MAX_SOURCE_CELLS + 10) + ) + DataValidator.enforce_size_limits(n_src=constants.MAX_SOURCE_CELLS + 5) + + def test_env_override_can_tighten_limit( + self: "TestEnforceSizeLimits", monkeypatch: pytest.MonkeyPatch + ) -> None: + """ + This test checks that the enforce_size_limits function respects an environment variable (MPASDIAG_MAX_NUM_POINTS) that tightens the default limit for the number of cross-section interpolation points (num_points). It ensures that the function correctly enforces the tightened limit and raises a ValueError when the limit is exceeded. + + Parameters: + monkeypatch: pytest.MonkeyPatch + + Returns: + None + """ + monkeypatch.setenv("MPASDIAG_MAX_NUM_POINTS", "10") + with pytest.raises(ValueError, match="cross-section points"): + DataValidator.enforce_size_limits(num_points=11) + + def test_invalid_env_override_raises( + self: "TestEnforceSizeLimits", monkeypatch: pytest.MonkeyPatch + ) -> None: + """ + This test checks that the enforce_size_limits function raises a ValueError when the environment variable (MPASDIAG_MAX_SOURCE_CELLS) is set to a non-integer value. It ensures that the function correctly validates the environment variable and raises an appropriate error message indicating that the value must be a positive integer. + + Parameters: + monkeypatch: pytest.MonkeyPatch + + Returns: + None + """ + monkeypatch.setenv("MPASDIAG_MAX_SOURCE_CELLS", "not-a-number") + with pytest.raises(ValueError, match="positive integer"): + DataValidator.enforce_size_limits(n_src=1) + + def test_nonpositive_env_override_raises( + self: "TestEnforceSizeLimits", monkeypatch: pytest.MonkeyPatch + ) -> None: + """ + This test checks that the enforce_size_limits function raises a ValueError when the environment variable (MPASDIAG_MAX_SOURCE_CELLS) is set to a non-positive value. It ensures that the function correctly validates the environment variable and raises an appropriate error message indicating that the value must be a positive integer. + + Parameters: + monkeypatch: pytest.MonkeyPatch + + Returns: + None + """ + monkeypatch.setenv("MPASDIAG_MAX_SOURCE_CELLS", "0") + with pytest.raises(ValueError, match="positive integer"): + DataValidator.enforce_size_limits(n_src=1) + + +class TestSafeResolveWithin: + """Path-traversal containment guard (MPAS-005).""" + + def test_valid_relative_path_resolves_inside_base( + self: "TestSafeResolveWithin", tmp_path: Path + ) -> None: + """ + This test verifies that the safe_resolve_within function correctly resolves a valid relative file path within a specified base directory. It ensures that the resolved path is an absolute path that remains within the base directory and does not raise any exceptions. + + Parameters: + tmp_path: Path - A temporary directory provided by pytest for testing. + + Returns: + None + """ + resolved = safe_resolve_within("weights.nc", str(tmp_path)) + assert resolved == (tmp_path / "weights.nc").resolve() + + def test_traversal_escape_rejected( + self: "TestSafeResolveWithin", tmp_path: Path + ) -> None: + """ + This test checks that the safe_resolve_within function raises a ValueError when a path traversal escape is attempted. It ensures that the function correctly identifies and rejects paths that attempt to access files outside the specified base directory, such as '../../etc/passwd'. + + Parameters: + tmp_path: Path - A temporary directory provided by pytest for testing. + + Returns: + None + """ + with pytest.raises(ValueError, match="outside"): + safe_resolve_within("../../etc/passwd", str(tmp_path)) + + def test_absolute_path_outside_rejected( + self: "TestSafeResolveWithin", tmp_path: Path + ) -> None: + """ + This test verifies that the safe_resolve_within function raises a ValueError when an absolute file path outside the specified base directory is provided. It ensures that the function correctly identifies and rejects absolute paths that do not reside within the base directory, such as '/etc/hosts'. + + Parameters: + tmp_path: Path - A temporary directory provided by pytest for testing. + + Returns: + None + """ + with pytest.raises(ValueError, match="outside"): + safe_resolve_within("/etc/hosts", str(tmp_path)) + + def test_suffix_enforced(self: "TestSafeResolveWithin", tmp_path: Path) -> None: + """ + This test checks that the safe_resolve_within function raises a ValueError when a file path with a disallowed suffix is provided. It ensures that the function correctly enforces the allowed file extensions specified in the allowed_suffixes parameter and raises an appropriate error message when the file's suffix does not match any of the allowed extensions. + + Parameters: + tmp_path: Path - A temporary directory provided by pytest for testing. + + Returns: + None + """ + with pytest.raises(ValueError, match="extensions"): + safe_resolve_within("weights.txt", str(tmp_path), allowed_suffixes=(".nc",)) + + def test_suffix_allowed_passes( + self: "TestSafeResolveWithin", tmp_path: Path + ) -> None: + """ + This test verifies that the safe_resolve_within function correctly allows a file path with an allowed suffix. It ensures that when a file path with a suffix that matches one of the allowed extensions specified in the allowed_suffixes parameter is provided, the function resolves the path without raising any exceptions. + + Parameters: + tmp_path: Path - A temporary directory provided by pytest for testing. + + Returns: + None + """ + resolved = safe_resolve_within( + "weights.nc", str(tmp_path), allowed_suffixes=(".nc",) + ) + assert resolved.suffix == ".nc" + + def test_must_exist_raises_for_missing( + self: "TestSafeResolveWithin", tmp_path: Path + ) -> None: + """ + This test verifies that the safe_resolve_within function raises a FileNotFoundError when a file path that must exist is missing. It ensures that the function correctly identifies and raises an error for non-existent files when the must_exist parameter is set to True. + + Parameters: + tmp_path: Path - A temporary directory provided by pytest for testing. + + Returns: + None + """ + with pytest.raises(FileNotFoundError): + safe_resolve_within("missing.nc", str(tmp_path), must_exist=True) + + def test_must_exist_passes_for_present( + self: "TestSafeResolveWithin", tmp_path: Path + ) -> None: + """ + This test checks that the safe_resolve_within function correctly resolves a file path that must exist when the file is present. It ensures that when a file path is provided for a file that exists in the specified base directory and the must_exist parameter is set to True, the function resolves the path without raising any exceptions. + + Parameters: + tmp_path: Path - A temporary directory provided by pytest for testing. + + Returns: + None + """ + target = tmp_path / "present.nc" + target.write_bytes(b"") + resolved = safe_resolve_within("present.nc", str(tmp_path), must_exist=True) + assert resolved == target.resolve() + + +if __name__ == "__main__": + pytest.main([__file__, "-v"])