Strengthening security for TH repo - #235
Merged
Merged
Conversation
albertocarpentieri
approved these changes
Aug 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Harden the 2D3DS dataset downloader and CI supply chain
Problem
Two security issues were reported against
torch_harmonics/examples/stanford_2d3ds_dataset.py,which ships as part of the installable
torch_harmonics.examplespackage:Path traversal on extraction.
_extract_tar()calledtar.extractall()with no memberfiltering. A tar entry with
../components — or a symlink pointing outside the tree — couldwrite anywhere the process has permission.
base_urlis user-controllable, so a maliciousmirror or a compromised upstream server turns this into arbitrary file write.
No download integrity check.
_download_file()fetched multi-GB tars over HTTPS with nochecksum or signature, so a compromised server or an intercepted connection (TLS interception
is common in corporate environments) could substitute a crafted archive. Combined with (1),
this is a supply-chain path into a user's filesystem.
While fixing these, three latent bugs surfaced in the same download path, plus some CI hardening
that addresses the same class of risk on the build side.
Changes
Dataset downloader
tar.extractall(filter="data")where available.data_filterwas backported to 3.10.12+, so 3.10.0–3.10.11 remain in the supported range andfall back to
_check_tar_members(), which validates each member path against the destinationand additionally checks symlink/hardlink targets (a name-only check misses
area_1/x -> ../../../etc/passwd).DEFAULT_TAR_FILE_CHECKSUMStable covering all seven archives,verified after download and on the "file already exists" path, so a poisoned or truncated
file left from an earlier run is not silently trusted. New
checksumsconstructor argumentaccepts a custom mapping for mirrors, or
Noneto opt out.raise_for_status(). HTTP error bodies were written to disk as.tarfilesand handed to
tarfile; the visible symptom wasReadError: not a gzip filefrom a saved 404 page..partpath.local_path.split(".")[0] + ".part"breaks on anylocal_dircontaininga dot —
./datawrote the partial file into the current working directory,v1.0/dataproduced
v1.part. Now usesos.path.splitext.Rangeresume path appended unconditionally. If the serverignored the header and replied
200with a full body, the partial file was corrupted. Resumenow requires a
206, otherwise the download restarts cleanly.Packaging
requires-python, trove classifier, cibuildwheelbuild,the wheel matrix in
build_wheels.yml, setup-python intests/style, CONTRIBUTING).3.9 reached end of life in October 2025.
tarfilefrom the2d3dsextra.tarfileis stdlib and the name is unregisteredon PyPI, so
pip install torch-harmonics[2d3ds]failed at resolution with"No matching distribution found for tarfile".
CI supply chain
permissions: contents: readadded tobuild_wheels.yml,tests.yml, andstyle.yml.The repo default was "read and write", so every job — including the one producing published
wheels — ran with a token that could push commits and move tags.
docs.yamlalready scopedits own permissions and is unchanged. (The repo-level default and "Allow GitHub Actions to
create and approve pull requests" were also turned off in repo settings.)
.github/requirements/:cibuildwheel,twine,auditwheel,wheel(host);pip,setuptools,wheel,setuptools-scm(inside the manylinuxcontainers);
pre-commit(style). These previously resolved to whatever was latest at buildtime and run inside the job that produces the published wheels. Pinned to the versions CI was
already resolving, so this is behavior-neutral today.
pipentry on/.github/requirementsso the pins don't go stale.Testing
No GPU or kernel code is touched, so the kernel/benchmark workflow does not apply.
Dataset downloader — exercised end to end against a local HTTP server serving crafted tars
(Python 3.12, torch 2.9.0), rather than the 225 GB real dataset:
RuntimeErrorbefore extraction; target dir never created../../pwned.txt), checksum matchesRange.partin cwdHTTPError, nothing written to diskchecksums=NoneBoth extraction paths were tested — the stdlib
data_filterand, withtarfile.data_filterdeleted to simulate 3.10.0–3.10.11, the manual fallback.
The
Rangefix was additionally confirmed against the real ETH server:area_4resumed from94% after an interrupted run and hashed to the expected digest.
Checksums — all seven archives (225 GB) were downloaded from
cvg-data.inf.ethz.chon2026-08-05 via the library's own
_download_file(), hashed, and deleted. The committed tablewas verified to match the recorded digests exactly and to cover every entry in
DEFAULT_TAR_FILE_PAIRS, so no file falls through to the unverified warning path.Packaging / CI —
pyproject.tomland all four workflow YAMLs parse; the three requirementsfiles resolve with no version conflicts;
cibuildwheel4.2.0's{project}placeholdersubstitution in
before_buildwas confirmed in its source (platforms/linux.py:287).Behavior changes
Stanford2D3DSDownloader.__init__gains achecksumskeyword argument (defaulted; existingpositional calls are unaffected).
RuntimeErrorinstead of extracting.local_dirnow raise instead of being written.Requires-Python, no cp39 wheelsare built, and 3.9 users on
pip install torch-harmonicssilently resolve to the last 3.9-compatible release.
Notes and limitations
tampering, MITM, and silent re-uploads, but cannot retroactively prove those bytes are the
2D-3D-S authors' originals; cross-checking a second mirror would be needed for that.
lockfiles (
--generate-hashes+--require-hashes) would close that and can follow separately.pip install numpyinCIBW_BEFORE_BUILD/BEFORE_TESTwas left unpinned deliberately: ittests the declared runtime dependency and should track current numpy.
workflow_dispatchrun ofbuild_wheels.ymlbefore the next release tag toconfirm the pinned toolchain in the real manylinux containers.