Code review example: yaml.Loader on untrusted input can construct arbitrary Python objects in Python

Retired from the DiffDojo practice pool in October 2026; the review below is the canonical answer. An AI wrote this pull request for the task underneath. Read the diff and decide what you would say before you scroll to the review.

Python · Security · intermediate

The task

Load a workflow config from YAML

1. Parse a workflow config from a YAML string and return it as a dict.
2. The YAML comes from user-uploaded workflow files.
3. Parsing must not be able to execute code or construct arbitrary objects.

Adds the config loader: parses the uploaded YAML into a dict with PyYAML. Handles the sample configs correctly.

Users upload their own workflow .yml files; the content is fully untrusted.

The diff

The review

Issue 1: yaml.Loader on untrusted input can construct arbitrary Python objects

app/config/loader.py, line 5

1. yaml.Loader is the legacy unsafe loader. It honors YAML's language-specific tags, including !!python/object/apply:..., which *instantiate Python objects and call functions during parsing*. (It is not FullLoader; the PyYAML default-loader deprecation did not make an explicit Loader=yaml.Loader safe.)
2. The input is user-uploaded. A workflow file containing !!python/object/apply:os.system ["rm -rf /"] (or a reverse shell) executes the moment load parses it. That's remote code execution, not a data bug.
3. The safe path is a one-word change. safe_load builds only plain dicts, lists, strings, and numbers, and rejects the Python-specific tags that make the unsafe loader dangerous.

The fix: yaml.safe_load(text) (equivalently Loader=yaml.SafeLoader). The existing isinstance(..., dict) guard stays.

The fix

import yaml


def load_config(text: str) -> dict:
    config = yaml.safe_load(text)
    if not isinstance(config, dict):
        raise ValueError("workflow config must be a mapping")
    return config

Why it matters

yaml.load without SafeLoader, pickle.loads, eval, exec on untrusted input → arbitrary code execution. For any data you didn't produce, safe_load is the only acceptable YAML entry point.

Real-world stakes: one uploaded file owns the server. Unsafe YAML deserialization has produced real CVEs across major Python projects, and assistants suggest yaml.load(...) reflexively because for years it was the obvious call, the safe variant is the thing you have to remember to reach for. (The isinstance guard is a separate correctness nicety, present in both versions so it's not the bug here.)

Read more: yaml.load vs safe_load python, pyyaml arbitrary code execution, python unsafe deserialization yaml

The verdict

Request changes. yaml.Loader on untrusted input can construct arbitrary Python objects

How to review Python code · All code review examples