Python review hooks: list args, refuse pickle

A Python backdoor demo is usually subprocess with shell=True, or pickle, or an eval of untrusted text.

CPython will not pick a shell for you unless you ask. Bandit already has an id for that ask. Production code that still interpolates a filename into os.system is the real backdoor.

The usual mistake is treating these posts as a how-to. They are a list of sinks to refuse and to grep.

This page names those sinks, the Bandit ids, and the tests that prove a user-controlled string cannot become a command.

CPython 3.13 still will not pick a shell for you. Bandit B602 is still the id for subprocess_popen_with_shell_equals_true. A reviewer who accepts a string plus shell=True, or a new pickle.loads, undoes both of those.This page is the review hook that blocks those two shapes before merge.

The sibling page on inventory and runtime deny is the other half. Keep the Python hatch list for the four everyday misses. Read injection when the string reaches SQL or the shell, input validation when the value is still a request field, and the secure coding checklist for the rest of the surface.

The merge is the miss, not a later scan

A nightly Bandit job that pages after deploy is late. The line already shipped. The gate that matches this page is a required check on the PR, plus a required reviewer on the files that can spawn a child or load a stream. Style lint does not see either hatch.

3.13 subprocess security section on 22 August 2026. The library will not implicitly choose a shell. Metacharacters stay characters when args is a sequence. The hatch is shell=True, or a string you interpolate and then hand to Popen. 3.12 changed one Windows case: with shell=True, the search order no longer prefers the current directory for cmd.exe. That is a belt. It is not a reason to keep a shell string on a user filename.

CWE-78 is the ticket when untrusted text reaches that shell. CWE-502 is the ticket when the same PR adds a loader. Treat both as merge blockers. A # nosec without a ticket id is a reject.

subprocess wants a list, then a path check

A list is necessary and not sufficient. The kernel sees a vector. It does not see whether the first element is the binary you meant, or whether a filename walked out of the upload directory. blorgle said the second half in 2021.

run_convert is the named helper. The binary is a constant path. The two files must resolve inside directories you listed. shell stays false. check=True so a nonzero exit is an exception, not a silent string.

import subprocess
from pathlib import Path

CONVERT = Path("/usr/bin/convert")
INBOX = Path("/var/app/inbox")
OUTBOX = Path("/var/app/outbox")

def _inside(root: Path, candidate: Path) -> Path:
 resolved = candidate.resolve()
 root_res = root.resolve()
 if root_res not in resolved.parents and resolved != root_res:
 raise ValueError("path leaves the root")
 return resolved

def run_convert(src: Path, dest: Path) -> None:
 src_ok = _inside(INBOX, src)
 dest_ok = _inside(OUTBOX, dest)
 if not CONVERT.is_file():
 raise ValueError("missing convert")
 if not src_ok.is_file():
 raise ValueError("missing source")
 subprocess.run(
 [str(CONVERT), str(src_ok), str(dest_ok)],
 check=True,
 shell=False,
 timeout=30,
 )

The miss is the interpolation people still paste:

# BAD: do not ship
# subprocess.run(f"convert {src} {dest}", shell=True)
# os.system("convert " + src)

shlex.quote is leftover work for a pipeline you have not split yet. The 3.13 docs mention it when you already invoked a shell. A list with shell=False does not need that quote. If you need a pipe, run two list calls and pass bytes between them. Do not rebuild a shell line so the pipe character works.

Windows batch files are the other footgun. The 3.13 security section says *.bat and *.cmd may still be launched through a system shell even when you passed a list. Do not hand a user filename to a batch wrapper. Call the real .exe by absolute path, or do the work in Python.

Refuse pickle at review, not after deploy

3.13.15 pickle page the same day. The banner is still the first thing after the title: the module is not secure. The next sentence says a crafted stream can run during load. JSON is the format that page names. A PR that adds pickle.loads to a cache, a queue body, or a celery result is a reject. Sign the JSON with HMAC if you need tamper evidence. Do not keep the loader.

import json

def load_cache(path: Path) -> dict:
 raw = path.read_text(encoding="utf-8")
 data = json.loads(raw)
 if not isinstance(data, dict):
 raise ValueError("cache must be an object")
 return data

def save_cache(path: Path, data: dict) -> None:
 path.write_text(json.dumps(data, separators=(",", ":")), encoding="utf-8")

load_cache and save_cache are the named pair. A reviewer can print the file in a ticket. The cousin hatches are dill.loads, shelve.open, jsonpickle.decode, and yaml.load( without SafeLoader. Bandit B301 and B506 already name those. A find_class override is not a substitute. The next vendor pickle still calls loads.

For a cache you already wrote with pickle, do not “just sign it” and keep the loader. Add a version byte, write JSON beside the old key, read JSON first, and delete the pickle key after one TTL. The point is the loader goes away, not that HMAC makes REDUCE safe.

The hook file and the owners file

review_hatches is the named check. It reads the diff or the tree, and exits 1 on a new token. Put it in the same job that runs tests. A pre-commit hook that only formats will not see the line.

from pathlib import Path
import sys

TOKENS = (
 "pickle.loads",
 "pickle.load(",
 "dill.loads",
 "shell=True",
 "os.system",
 "os.popen",
 "yaml.load(",
)

SKIP = {".venv", "venv", "__pycache__", "tests"}

def review_hatches(root: Path) -> list[str]:
 hits: list[str] = []
 for path in root.rglob("*.py"):
 if any(part in SKIP for part in path.parts):
 continue
 text = path.read_text(encoding="utf-8", errors="replace")
 for i, line in enumerate(text.splitlines(), start=1):
 stripped = line.strip()
 if stripped.startswith("#"):
 continue
 if any(token in stripped for token in TOKENS):
 hits.append(f"{path}:{i}:{stripped}")
 return hits

if __name__ == "__main__":
 found = review_hatches(Path("."))
 if found:
 print("\n".join(found))
 sys.exit(1)

A comment-only line is skipped so a reviewer can write the token in a note. A string in production code is not skipped. If you need one exception, put the path in review_hatches.allow and require two owners to change that file. Do not put # nosec on the helper itself.

DIFF review_hatches(root)
 shell=True or pickle.loads
 exit 1, no merge

OWN CODEOWNERS on subprocess imports
 two reviewers, not one
 a nosec needs a ticket id

KEEP run_convert([bin, *argv])
 load_cache via json.loads
 path stays inside INBOX

CODEOWNERS is the human belt. Name the people who already understand run_convert on every path that imports subprocess or pickle. GitHub and GitLab both honor that file on the default branch. A required review from that group means a drive-by approval from someone who has never seen B602 cannot land the line.

#.github/CODEOWNERS
/app/jobs/** @platform-review
/app/workers/** @platform-review
/scripts/review_hatches.py @platform-review
/review_hatches.allow @platform-review

Branch protection should require the review_hatches check and a review from @platform-review. Skipping those two for a hotfix is how a hatch lands on Friday. If you must skip, open a follow-up ticket in the same hour and do not close it until the hook is green on main.

Prove the hook fails the PR

You are not walking an implant. You are proving review_hatches exits 1 on a planted token and exits 0 on the current tree.

python scripts/review_hatches.py
# Expect: exit 0 on main

printf '%s\n' 'x = pickle.loads(b"")' >> /tmp/probe_hatch.py
# do not commit that file
python -c "from review_hatches import review_hatches; print(review_hatches(Path('/tmp')))"
# Expect: a hit on probe_hatch.py, then delete the file

The probe is a token in a throwaway path. It does not load a stream or spawn a child. If the function returns empty on that file, the scanner is wrong. Then open a draft PR that adds shell=True on a throwaway branch you own. Expect the required check to fail. A green check on that draft means the job is not running the script.

Bandit in the same job is extra signal. Keep B301, B602, B605, and B506 as errors. A warning that nobody reads is not a gate. If you already run it, wire those four ids to fail. If you do not, review_hatches is the five-minute proof.