Skip to content

Commit 84f3e42

Browse files
committed
Phase 3.3: centralize deserialization policy and enforce trusted roots
1 parent cd6f22a commit 84f3e42

12 files changed

Lines changed: 288 additions & 57 deletions

File tree

README.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,7 @@ Optional API environment variables:
161161
# Root directories used by API path constraints
162162
export CATPRED_API_INPUT_ROOT="/absolute/path/for/input-csvs"
163163
export CATPRED_API_RESULTS_ROOT="/absolute/path/for/results"
164+
export CATPRED_API_CHECKPOINT_ROOT="/absolute/path/for/checkpoints"
164165
165166
# Enable only for trusted local workflows (not recommended for public deployments)
166167
export CATPRED_API_ALLOW_INPUT_FILE=1
@@ -171,6 +172,17 @@ export CATPRED_API_MAX_INPUT_ROWS=1000
171172
export CATPRED_API_MAX_INPUT_FILE_BYTES=5000000
172173
```
173174

175+
Deserialization hardening controls:
176+
177+
```bash
178+
# Trusted roots used by secure loaders (colon-separated list on Unix)
179+
export CATPRED_TRUSTED_DESERIALIZATION_ROOTS="/srv/catpred:/srv/catpred-data"
180+
181+
# Backward-compatible default is enabled (1). Set to 0 to block unsafe pickle-based loading.
182+
# Use 0 only after validating your artifacts are safe-load compatible.
183+
export CATPRED_ALLOW_UNSAFE_DESERIALIZATION=1
184+
```
185+
174186
### 🧪 Fine-Tuning On Custom Data
175187

176188
You can fine-tune CatPred on your own regression targets using `train.py`.

catpred/__init__.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
"models",
1414
"nn_utils",
1515
"rdkit",
16+
"security",
1617
"train",
1718
"uncertainty",
1819
"utils",

catpred/args.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import json
22
import os
33
from tempfile import TemporaryDirectory
4-
import pickle
54
from typing import List, Optional
65
from typing_extensions import Literal
76
from packaging import version
@@ -15,6 +14,7 @@
1514
import catpred.data.utils
1615
from catpred.data import set_cache_mol, empty_cache
1716
from catpred.features import get_available_features_generators
17+
from catpred.security import load_index_artifact
1818

1919

2020
Metric = Literal['auc', 'prc-auc', 'rmse', 'mae', 'mse', 'r2', 'accuracy', 'cross_entropy', 'binary_cross_entropy', 'sid', 'wasserstein', 'f1', 'mcc', 'bounded_rmse', 'bounded_mae', 'bounded_mse']
@@ -815,8 +815,10 @@ def process_args(self) -> None:
815815
raise ValueError('When using crossval or index_predetermined split type, must provide crossval_index_file.')
816816

817817
if self.split_type in ['crossval', 'index_predetermined']:
818-
with open(self.crossval_index_file, 'rb') as rf:
819-
self._crossval_index_sets = pickle.load(rf)
818+
self._crossval_index_sets = load_index_artifact(
819+
self.crossval_index_file,
820+
purpose="cross-validation index file",
821+
)
820822
self.num_folds = len(self.crossval_index_sets)
821823
self.seed = 0
822824

catpred/data/cache_utils.py

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import hashlib
55
from functools import wraps
66
from pathlib import Path
7+
from catpred.security import load_torch_artifact
78

89
def exists(val):
910
return val is not None
@@ -27,13 +28,6 @@ def md5_hash_fn(s):
2728
encoded = s.encode('utf-8')
2829
return hashlib.md5(encoded).hexdigest()
2930

30-
31-
def _torch_load_compat(path):
32-
try:
33-
return torch.load(path, weights_only=False)
34-
except TypeError:
35-
return torch.load(path)
36-
3731
# run once function
3832

3933
GLOBAL_RUN_RECORDS = dict()
@@ -99,7 +93,11 @@ def inner(t, *args, __cache_key = None, **kwargs):
9993

10094
if entry_path.exists():
10195
log(f'cache hit: fetching {t} from {str(entry_path)}')
102-
return _torch_load_compat(str(entry_path))
96+
return load_torch_artifact(
97+
str(entry_path),
98+
purpose="esm cache entry",
99+
roots=[CACHE_PATH],
100+
)
103101

104102
out = fn(t, *args, **kwargs)
105103

catpred/data/utils.py

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
import csv
44
import ctypes
55
from logging import Logger
6-
import pickle
76
from random import Random
87
from typing import List, Set, Tuple, Union
98
import os
@@ -22,6 +21,7 @@
2221
from catpred.args import PredictArgs, TrainArgs
2322
from catpred.features import load_features, load_valid_atom_or_bond_features, is_mol
2423
from catpred.rdkit import make_mol
24+
from catpred.security import load_index_artifact, load_pickle_artifact
2525

2626
# Increase maximum size of field in the csv processing for the current architecture
2727
csv.field_size_limit(int(ctypes.c_ulong(-1).value // 2))
@@ -791,8 +791,12 @@ def split_data(data: MoleculeDataset,
791791
for split in range(3):
792792
split_indices = []
793793
for index in index_set[split]:
794-
with open(os.path.join(args.crossval_index_dir, f'{index}.pkl'), 'rb') as rf:
795-
split_indices.extend(pickle.load(rf))
794+
split_indices.extend(
795+
load_index_artifact(
796+
os.path.join(args.crossval_index_dir, f"{index}.pkl"),
797+
purpose="cross-validation fold index file",
798+
)
799+
)
796800
data_split.append([data[i] for i in split_indices])
797801
train, val, test = tuple(data_split)
798802
return MoleculeDataset(train), MoleculeDataset(val), MoleculeDataset(test)
@@ -841,12 +845,10 @@ def split_data(data: MoleculeDataset,
841845
if test_fold_index is None:
842846
raise ValueError('arg "test_fold_index" can not be None!')
843847

844-
try:
845-
with open(folds_file, 'rb') as f:
846-
all_fold_indices = pickle.load(f)
847-
except UnicodeDecodeError:
848-
with open(folds_file, 'rb') as f:
849-
all_fold_indices = pickle.load(f, encoding='latin1') # in case we're loading indices from python2
848+
all_fold_indices = load_pickle_artifact(
849+
folds_file,
850+
purpose="predetermined folds file",
851+
)
850852

851853
log_scaffold_stats(data, all_fold_indices, logger=logger)
852854

catpred/features/utils.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
import csv
22
import os
3-
import pickle
43
from typing import List
54

65
import numpy as np
76
import pandas as pd
87
from rdkit.Chem import PandasTools
8+
from catpred.security import load_pickle_artifact
99

1010

1111
def save_features(path: str, features: List[np.ndarray]) -> None:
@@ -49,8 +49,8 @@ def load_features(path: str) -> np.ndarray:
4949
next(reader) # skip header
5050
features = np.array([[float(value) for value in row] for row in reader])
5151
elif extension in ['.pkl', '.pckl', '.pickle']:
52-
with open(path, 'rb') as f:
53-
features = np.array([np.squeeze(np.array(feat.todense())) for feat in pickle.load(f)])
52+
payload = load_pickle_artifact(path, purpose="feature matrix pickle")
53+
features = np.array([np.squeeze(np.array(feat.todense())) for feat in payload])
5454
else:
5555
raise ValueError(f'Features path extension {extension} not supported.')
5656

catpred/models/model.py

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,7 @@
1313
from catpred.args import TrainArgs
1414
from catpred.features import BatchMolGraph
1515
from catpred.nn_utils import initialize_weights
16-
17-
18-
def _torch_load_compat(path):
19-
try:
20-
return torch.load(path, weights_only=False)
21-
except TypeError:
22-
return torch.load(path)
16+
from catpred.security import load_torch_artifact
2317

2418

2519
def exists(val):
@@ -126,7 +120,10 @@ def create_protein_model(self, args: TrainArgs) -> None:
126120
self.seq_embedder = nn.Embedding(21, args.seq_embed_dim, padding_idx=20) #last index is for padding
127121

128122
if self.args.add_pretrained_egnn_feats:
129-
self.pretrained_egnn_feats_dict = _torch_load_compat(self.args.pretrained_egnn_feats_path)
123+
self.pretrained_egnn_feats_dict = load_torch_artifact(
124+
self.args.pretrained_egnn_feats_path,
125+
purpose="pretrained EGNN features",
126+
)
130127
x = list(self.pretrained_egnn_feats_dict.values())
131128
self.pretrained_egnn_feats_avg = torch.stack(x).mean(dim=0)
132129

catpred/security/__init__.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
from .deserialization import (
2+
DeserializationSecurityError,
3+
ensure_trusted_path,
4+
load_index_artifact,
5+
load_pickle_artifact,
6+
load_torch_artifact,
7+
unsafe_deserialization_enabled,
8+
)
9+
10+
__all__ = [
11+
"DeserializationSecurityError",
12+
"ensure_trusted_path",
13+
"load_index_artifact",
14+
"load_pickle_artifact",
15+
"load_torch_artifact",
16+
"unsafe_deserialization_enabled",
17+
]
Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,168 @@
1+
from __future__ import annotations
2+
3+
import gzip
4+
import json
5+
import os
6+
import pickle
7+
from pathlib import Path
8+
from typing import Any, Iterable
9+
10+
11+
class DeserializationSecurityError(RuntimeError):
12+
"""Raised when deserialization is blocked by policy."""
13+
14+
15+
_ALLOW_UNSAFE_ENV = "CATPRED_ALLOW_UNSAFE_DESERIALIZATION"
16+
_TRUSTED_ROOTS_ENV = "CATPRED_TRUSTED_DESERIALIZATION_ROOTS"
17+
18+
19+
def _env_flag(name: str, default: bool = False) -> bool:
20+
raw = os.environ.get(name)
21+
if raw is None:
22+
return default
23+
return raw.strip().lower() in {"1", "true", "yes", "y", "on"}
24+
25+
26+
def _dedupe_paths(paths: list[Path]) -> list[Path]:
27+
unique: list[Path] = []
28+
seen = set()
29+
for path in paths:
30+
resolved = path.resolve()
31+
key = str(resolved)
32+
if key not in seen:
33+
unique.append(resolved)
34+
seen.add(key)
35+
return unique
36+
37+
38+
def _default_trusted_roots() -> list[Path]:
39+
raw = os.environ.get(_TRUSTED_ROOTS_ENV)
40+
if raw:
41+
candidates = [Path(item) for item in raw.split(os.pathsep) if item.strip()]
42+
else:
43+
cwd = Path.cwd().resolve()
44+
candidates = [cwd, cwd.parent]
45+
return _dedupe_paths(candidates)
46+
47+
48+
def trusted_roots(extra_roots: Iterable[str | Path] | None = None) -> list[Path]:
49+
roots = _default_trusted_roots()
50+
if extra_roots:
51+
roots.extend(Path(path) for path in extra_roots)
52+
return _dedupe_paths(roots)
53+
54+
55+
def is_trusted_path(path: str | Path, roots: Iterable[str | Path] | None = None) -> bool:
56+
resolved = Path(path).resolve()
57+
candidate_roots = trusted_roots(roots)
58+
for root in candidate_roots:
59+
try:
60+
resolved.relative_to(root)
61+
return True
62+
except ValueError:
63+
continue
64+
return False
65+
66+
67+
def ensure_trusted_path(
68+
path: str | Path,
69+
*,
70+
purpose: str,
71+
roots: Iterable[str | Path] | None = None,
72+
) -> Path:
73+
resolved = Path(path).resolve()
74+
candidate_roots = trusted_roots(roots)
75+
if is_trusted_path(resolved, candidate_roots):
76+
return resolved
77+
roots_display = ", ".join(str(item) for item in candidate_roots)
78+
raise DeserializationSecurityError(
79+
f'Refusing to load untrusted {purpose} from "{resolved}". '
80+
f"Allowed roots: {roots_display}. "
81+
f"Use {_TRUSTED_ROOTS_ENV} to expand trusted roots."
82+
)
83+
84+
85+
def unsafe_deserialization_enabled(default: bool = True) -> bool:
86+
return _env_flag(_ALLOW_UNSAFE_ENV, default=default)
87+
88+
89+
def load_pickle_artifact(
90+
path: str | Path,
91+
*,
92+
purpose: str,
93+
roots: Iterable[str | Path] | None = None,
94+
allow_unsafe: bool | None = None,
95+
encoding: str | None = None,
96+
) -> Any:
97+
resolved = ensure_trusted_path(path, purpose=purpose, roots=roots)
98+
unsafe = unsafe_deserialization_enabled() if allow_unsafe is None else allow_unsafe
99+
if not unsafe:
100+
raise DeserializationSecurityError(
101+
f"Pickle deserialization is disabled for {purpose}. "
102+
f"Set {_ALLOW_UNSAFE_ENV}=1 only for trusted artifacts."
103+
)
104+
105+
with resolved.open("rb") as handle:
106+
if encoding is None:
107+
try:
108+
return pickle.load(handle)
109+
except UnicodeDecodeError:
110+
handle.seek(0)
111+
return pickle.load(handle, encoding="latin1")
112+
return pickle.load(handle, encoding=encoding)
113+
114+
115+
def load_index_artifact(
116+
path: str | Path,
117+
*,
118+
purpose: str,
119+
roots: Iterable[str | Path] | None = None,
120+
allow_unsafe: bool | None = None,
121+
) -> Any:
122+
resolved = ensure_trusted_path(path, purpose=purpose, roots=roots)
123+
suffixes = tuple(s.lower() for s in resolved.suffixes)
124+
if suffixes and suffixes[-1] == ".json":
125+
with resolved.open("rt", encoding="utf-8") as handle:
126+
return json.load(handle)
127+
if suffixes[-2:] == (".json", ".gz"):
128+
with gzip.open(resolved, "rt", encoding="utf-8") as handle:
129+
return json.load(handle)
130+
return load_pickle_artifact(
131+
resolved,
132+
purpose=purpose,
133+
roots=roots,
134+
allow_unsafe=allow_unsafe,
135+
)
136+
137+
138+
def load_torch_artifact(
139+
path: str | Path,
140+
*,
141+
purpose: str,
142+
map_location=None,
143+
roots: Iterable[str | Path] | None = None,
144+
allow_unsafe: bool | None = None,
145+
) -> Any:
146+
resolved = ensure_trusted_path(path, purpose=purpose, roots=roots)
147+
unsafe = unsafe_deserialization_enabled() if allow_unsafe is None else allow_unsafe
148+
149+
import torch
150+
151+
if unsafe:
152+
try:
153+
return torch.load(str(resolved), map_location=map_location, weights_only=False)
154+
except TypeError:
155+
return torch.load(str(resolved), map_location=map_location)
156+
157+
try:
158+
return torch.load(str(resolved), map_location=map_location, weights_only=True)
159+
except TypeError as exc:
160+
raise DeserializationSecurityError(
161+
"Safe torch deserialization requires a torch version that supports weights_only loading. "
162+
f"Set {_ALLOW_UNSAFE_ENV}=1 for trusted legacy checkpoints."
163+
) from exc
164+
except Exception as exc:
165+
raise DeserializationSecurityError(
166+
f"Safe torch deserialization rejected {purpose}. "
167+
f"If this checkpoint is trusted, set {_ALLOW_UNSAFE_ENV}=1."
168+
) from exc

0 commit comments

Comments
 (0)