Security: harden HDF5 deserialization and executable lookup

[jacquelinegarrahan]

Security hardening: HDF5 deserialization and executable lookup

Branch: security/hardening → main

This PR addresses two security findings from a scan of the codebase. Both are
in code paths that handle attacker-influenceable input (a serialized archive
file; the process working directory). All 121 existing tests pass unchanged.

Scan scope & tooling

• Static analysis with bandit -r lume (only low-severity assert/subprocess
  informational hits, addressed contextually below).
• Dependency audit with pip-audit — no known vulnerable dependencies.
• Review of GitHub Actions workflows — publishing uses OIDC/trusted publishing
  and pull_request (not pull_request_target); no secrets exposure found.
• Manual review of all deserialization, subprocess, filesystem, and dynamic
  import paths.

Finding 1 — Deserialization gadget in HDF5Serializer.deserialize (CWE-502)

lume/serializers/hdf5.py

deserialize() reads the object attribute from the HDF5 file (a dotted
module.ClassName string), imports that module, fetches the named attribute,
instantiates it, and calls load_archive() on it — all driven entirely by
values inside the file.

Confirmed exploit. A crafted archive naming antigravity.foo caused
import antigravity to run its module-level side effect (opening a web page)
purely from opening the file:

with h5py.File("evil.h5", "w") as f:
    f.attrs["_version"] = (0, 0, 0)
    f.attrs["object"] = "antigravity.foo"   # any importable dotted path
    f.attrs["_pkg_version"] = "0.0.0"
HDF5Serializer.deserialize("evil.h5")        # imports antigravity, side effect fires

Fix (defense in depth):

• Validate the object attribute is present and well-formed before use.
• After resolving the attribute, require it to be a class exposing a callable
  load_archive before instantiating it. This blocks the
  "instantiate an arbitrary class and call load_archive on it" gadget
  (e.g. os.system, pathlib.Path, builtins.dict are now rejected with
  ClassInitError). The check is intentionally interface-based rather than
  issubclass(Base, ...) because the serializer's documented contract is
  duck-typed (see test_hdf5.py, which round-trips a non-Base object).
• Add a prominent security warning to the docstring.

Residual risk (documented, not fully closable here): the module import
still executes before any class check, so import-time side effects of an
arbitrary installed module remain possible. This is inherent to a
self-describing archive format that names its own class. The robust mitigation
is operational — only deserialize archives from trusted sources — now stated
explicitly in the API docs. A future hardening could add an opt-in module
allowlist.

Finding 2 — Untrusted search path in find_executable (CWE-426)

lume/tools.py

The executable search built its path as [os.getcwd(), PATH], so
shutil.which returned a match in the current working directory in preference
to PATH. A binary planted in a working directory (e.g. a shared scratch or
download dir) named after the expected simulation code would run instead of the
trusted one on PATH.

Fix: search PATH before the cwd. An executable that legitimately lives
only in the working directory is still found; the only behavior change is that a
same-named binary on PATH now wins over one in the cwd. Also guards against
PATH being unset (previously os.pathsep.join([...None...]) would raise
TypeError).

Not changed (reviewed, acceptable)

• subprocess usage in tools.py (execute, execute2) uses list-form
  args with shell=False — no shell injection. Bandit's B603/B404 are
  informational for this pattern.
• assert statements in base.py/tools.py (bandit B101) are argument
  validation, not security controls; left as-is to avoid behavior changes.
• yaml.safe_load is already used (not yaml.load) — safe.

Verification

pytest -q   # 121 passed

Plus the confirmed-exploit archives above are now rejected with ClassInitError
before instantiation.

---

🤖 Generated with Claude Code