Skip to content

Commit c099024

Browse files
dguidoclaude
authored andcommitted
Replace AGENTS.md with comprehensive CLAUDE.md
Renames AGENTS.md to CLAUDE.md and expands it from vulnerability reporting guidance into full project context: commands, architecture, design rules, testing patterns, code style, and CI. Design rules distilled from PR review history (#79, #87, #195, #207, #210, #220). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent fc814e2 commit c099024

File tree

2 files changed

+132
-40
lines changed

2 files changed

+132
-40
lines changed

AGENTS.md

Lines changed: 0 additions & 40 deletions
This file was deleted.

CLAUDE.md

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
# Fickling
2+
3+
Static analyzer, decompiler, and bytecode rewriter for Python pickle serializations. Detects malicious pickle files including PyTorch models. Maintained by Trail of Bits.
4+
5+
## Commands
6+
7+
```bash
8+
make dev # uv sync --all-extras
9+
make test # pytest --cov=fickling test/ + coverage report
10+
make test-quick # pytest -q test/ (no coverage)
11+
make lint # ruff format --check + ruff check + mypy
12+
make format # ruff check --fix + ruff format
13+
```
14+
15+
CI uses `ty check fickling/` for type checking (not mypy). Run `uv run ty check fickling/` locally to match CI.
16+
17+
## Architecture
18+
19+
```
20+
fickling/
21+
fickle.py # Core: pickle AST, opcode classes, interpreter, UNSAFE_IMPORTS blocklist
22+
analysis.py # Security analysis framework: Analysis base class, Severity enum, check_safety()
23+
ml.py # ML-specific safety: UnsafeImportsML, MLAllowlist with UNSAFE_MODULES dict
24+
polyglot.py # Multi-format: TAR, ZIP, 7z, NumPy NPZ archive scanning
25+
pytorch.py # PyTorch model injection via BaseInjection
26+
loader.py # Public API: fickling.load() / fickling.loads() with safety checks
27+
hook.py # Runtime protection: activate_safe_ml_environment()
28+
cli.py # CLI argument parsing and output (all print/stdout goes here, not in library code)
29+
constants.py # ClamAV-compatible exit codes (EXIT_CLEAN=0, EXIT_UNSAFE=1, EXIT_ERROR=2)
30+
exception.py # Custom exceptions: WrongMethodError, InterpretationError, PickleDecodeError
31+
```
32+
33+
### Key design rules
34+
35+
- **`check_safety()` is the primary API.** It returns an `AnalysisResult` with a `severity` field. Library code returns data; CLI code prints it.
36+
- **No stdout/stderr in library code.** Print statements, result formatting, and JSON output belong in `cli.py` only. If someone uses fickling programmatically, they get structured results, not console noise.
37+
- **Two parallel blocklists exist.** `UNSAFE_IMPORTS` in `fickle.py` (general) and `UNSAFE_MODULES`/`UNSAFE_IMPORTS` in `analysis.py` (ML-specific with descriptions). Both must be updated when adding dangerous modules.
38+
- **`fickling.load()` re-serializes before loading.** It calls `pickle.loads(pickled_data.dumps())` rather than `pickle.load(file)` to prevent TOCTOU race conditions where the file changes between analysis and load.
39+
- **Graceful degradation for optional deps.** Features gated on optional dependencies (torch, py7zr) must work when the dependency is missing. Check availability at import time, not at call time.
40+
41+
## Testing
42+
43+
All tests use `unittest.TestCase`. Test files are in `test/`.
44+
45+
```bash
46+
uv run pytest -q test/ # all tests
47+
uv run pytest -q test/test_bypasses.py # just bypass regression tests
48+
```
49+
50+
### Test patterns
51+
52+
**Bypass/vulnerability tests** build pickles using the opcode API and assert severity:
53+
54+
```python
55+
def test_example_bypass(self):
56+
pickled = Pickled([
57+
op.Proto.create(4),
58+
op.ShortBinUnicode("dangerous_module"),
59+
op.ShortBinUnicode("dangerous_func"),
60+
op.StackGlobal(),
61+
op.EmptyTuple(),
62+
op.Reduce(),
63+
op.Stop(),
64+
])
65+
self.assertGreater(check_safety(pickled).severity, Severity.LIKELY_SAFE)
66+
```
67+
68+
Build payloads using fickling's opcode API (`op.Proto`, `op.ShortBinUnicode`, `op.StackGlobal`, `op.Reduce`, etc.) or Python's `pickle` module. Do not submit raw bytes or hand-assembled byte strings.
69+
70+
### Test organization
71+
72+
| File | Purpose |
73+
|------|---------|
74+
| `test_bypasses.py` | Regression tests for CVEs and GHSAs (each links to advisory) |
75+
| `test_attack_vectors.py` | Malicious payload detection |
76+
| `test_benign_edge_cases.py` | Safe pickle variants (false positive prevention) |
77+
| `test_cve_patterns.py` | CVE pattern detection |
78+
| `test_archive_scanning.py` | Archive/polyglot format tests |
79+
| `test_crashes.py` | Malformed input handling |
80+
| `test_pickle.py` | Core pickle module tests |
81+
| `test_hook.py` | Hook functionality |
82+
83+
## Code style
84+
85+
- **Formatter/linter:** ruff (line-length 100, double quotes, target py310)
86+
- **Type checker:** ty (CI runs with `continue-on-error` — 55 remaining errors being addressed incrementally)
87+
- **Imports:** `import fickling.fickle as op` is the convention for opcode access in tests
88+
- **AST visitor methods** in `fickle.py` use `visit_*` naming (suppresses N802)
89+
- **Error handling:** Malformed pickles raise `InterpretationError` (subclass of `PickleDecodeError`), not `ValueError`. The `has_interpretation_error` flag on `Pickled` signals downstream analysis to report `LIKELY_UNSAFE`.
90+
- **Exception design:** Exception messages are constructor arguments, not hardcoded in the class.
91+
- **Functions return data, not print.** Separation between computation and presentation is strict.
92+
93+
## CI
94+
95+
| Workflow | What it checks |
96+
|----------|---------------|
97+
| `tests.yml` | pytest across Python 3.10-3.14 |
98+
| `lint.yml` | ruff format + ruff check + ty check |
99+
| `pip-audit.yml` | Dependency vulnerability scan (runs daily + on PRs) |
100+
| `release.yml` | PyPI publishing on tags |
101+
102+
Actions are SHA-pinned with version comments. uv is used for dependency caching.
103+
104+
## Vulnerability reporting
105+
106+
### Include a minimal reproducing test case
107+
108+
Every report must include a test case for `test/test_bypasses.py` following the existing pattern. Link the GHSA/CVE in a comment above the test method.
109+
110+
### Use non-offensive payloads
111+
112+
Use `echo` or `print` to demonstrate code execution. Do not spawn shells, read sensitive files, or execute remote scripts.
113+
114+
### Keep the impact section minimal
115+
116+
Do not write elaborate exploitation scenarios. A brief explanation is sufficient — e.g., "module X is not in the unsafe imports list and can be used for code execution."
117+
118+
### Update both blocklists
119+
120+
When adding dangerous modules, update `UNSAFE_IMPORTS` in `fickle.py` **and** the relevant dict in `analysis.py` (`UNSAFE_MODULES` or `UNSAFE_IMPORTS`). Match only specific dangerous names (e.g., `_io.FileIO` not all of `_io`) to avoid false positives.
121+
122+
### Match all components of dotted module paths
123+
124+
Import matching checks all components of the module path: `any(component in UNSAFE_IMPORTS for component in module.split("."))`. This prevents bypasses via re-exports like `foo.bar.os`.
125+
126+
### Out of scope: UnusedVariables heuristic
127+
128+
We are **not** interested in bypasses of the `UnusedVariables` analysis. This is an intentionally weak, supplementary heuristic. Bypassing it alone is not a meaningful finding.
129+
130+
### No suggested fixes without human review
131+
132+
Do not include suggested code fixes unless they have been reviewed and approved by a human operator first.

0 commit comments

Comments
 (0)