Programming Languages Bug Report

Python mutable default argument leaked checkout warnings between customers

Customers started seeing checkout warnings that belonged to other people's orders. The cause was a list used as a default argument, created once per worker process and appended to on every request. Here is how we traced it and made it impossible to reintroduce.

Daniel Tran Daniel Tran @fun_dev · October 29, 2024 · 9 min read · 8 Views
Banner: one Python list shared by many function calls, leaking items between unrelated requests

The first report came through support on a Tuesday: a customer reviewing her basket saw the message "Shipping to PO boxes is not available for bulky items" although she had a single paperback in her cart and a street address. A day later another customer saw five warnings stacked on top of each other, one of which mentioned a discount code he had never typed.

Our checkout API is a Flask app served by Gunicorn with 8 sync workers behind an nginx proxy. The response that the frontend rendered looked like this:

{
  "order_id": "ord_7Q2K",
  "valid": true,
  "warnings": [
    "Shipping to PO boxes is not available for bulky items",
    "Coupon AUTUMN10 expired on 2024-10-20",
    "Coupon AUTUMN10 expired on 2024-10-20",
    "Gift wrap is unavailable for digital products"
  ]
}

A valid order with four warnings, two of them duplicates, none of them true for that basket.

Reproducing it

Locally with flask run and a single process we could reproduce it every time once we knew what to look for: validate order A (which has a bulky item to a PO box), then validate order B (a plain book). Order B came back with A's warning. Validate B again and it still had it. Restart the server and B was clean again until another order with a warning came along.

In production it looked random because each Gunicorn worker is a separate process with its own memory. A customer only saw stale warnings if their request landed on a worker that had previously served an order with warnings, and the list grew until that worker was recycled by max_requests = 2000.

What we ruled out

  • Frontend state. The React checkout page keeps warnings in a store, so we first suspected it was not clearing between orders. The raw API response in the network tab already contained the extra warnings.
  • Caching in nginx or the CDN. The endpoint is a POST with Cache-Control: no-store, and the order_id in the response matched the request, so it was not someone else's cached payload.
  • A shared Redis key. We store coupon state in Redis, and a key collision would have explained the coupon warning. The warnings were never written to Redis at all.

Root cause

A refactor two weeks earlier had split validation into small helpers. Each helper accepted an optional warnings list so they could be chained:

def check_shipping(order, warnings=[]):
    if order.has_bulky_items and order.address.is_po_box:
        warnings.append("Shipping to PO boxes is not available for bulky items")
    return warnings

The top-level function called check_shipping(order) without passing a list. Python evaluates [] once, when the def statement runs at import time, and stores that single list object on the function. Every call that omitted the argument appended to the same list, for the lifetime of the worker process. That is why warnings accumulated and duplicated across unrelated customers.

What is happening under the hood

In Python, default values are expressions evaluated when the function object is created, not when it is called. The resulting objects are stored on the function in __defaults__ (and __kwdefaults__ for keyword-only parameters). You can see the shared list directly:

>>> def add(item, bucket=[]):
...     bucket.append(item)
...     return bucket
...
>>> add("a")
['a']
>>> add("b")
['a', 'b']
>>> add.__defaults__
(['a', 'b'],)

Immutable defaults such as None, 0, "" or a tuple are safe because nobody can change them in place. Lists, dicts, sets, and instances of most classes are not. The same trap applies to datetime.now() used as a default: it is evaluated once at import, so every call gets the server start time.

In a web app this becomes a data-isolation bug. A module-level function lives as long as the worker process, so its default list is effectively a global variable shared by every request that worker handles. With threaded workers or async servers it is also shared between concurrent requests.

Environment

Component Version / value
OS Ubuntu 22.04 LTS (container base python:3.12-slim-bookworm)
Runtime CPython 3.12.6
Framework Flask 3.0.3
App server Gunicorn 23.0.0, 8 sync workers, max_requests = 2000
Lint flake8 with default plugins only (no bugbear)

Minimal reproduction

# shared_default.py
def collect(order_id, has_issue, warnings=[]):
    if has_issue:
        warnings.append(f"issue on {order_id}")
    return warnings

print(collect("A", True))    # ['issue on A']
print(collect("B", False))   # ['issue on A']  <- B sees A's warning
print(collect("C", True))    # ['issue on A', 'issue on C']

Who is affected

Every Python version behaves this way; it is defined language semantics, not a bug in CPython. The risk is highest in long-running processes (web workers, Celery workers, Lambda containers that are reused between invocations) and in helpers that "accumulate" results, build headers, or merge options into a dict passed by default.

The fix

Use None as the sentinel and create a fresh object inside the function. We applied this to all nine validation helpers:

-def check_shipping(order, warnings=[]):
+def check_shipping(order, warnings: list[str] | None = None) -> list[str]:
+    if warnings is None:
+        warnings = []
     if order.has_bulky_items and order.address.is_po_box:
         warnings.append("Shipping to PO boxes is not available for bulky items")
     return warnings

We also changed the design so helpers no longer mutate a caller's list. Each helper returns its own warnings and the orchestrator concatenates them. That removes the temptation to share state at all:

def check_shipping(order) -> list[str]:
    if order.has_bulky_items and order.address.is_po_box:
        return ["Shipping to PO boxes is not available for bulky items"]
    return []

VALIDATORS = (check_shipping, check_coupon, check_gift_wrap, check_stock)

def validate_order(order) -> list[str]:
    warnings: list[str] = []
    for validator in VALIDATORS:
        warnings.extend(validator(order))
    return warnings

Alternatives and trade-offs

  1. Dataclasses with default_factory. For structured results we now use a dataclass. The standard library refuses an unhashable default outright, which is a nice safety net:

    from dataclasses import dataclass, field
    
    @dataclass
    class ValidationResult:
        order_id: str
        warnings: list[str] = field(default_factory=list)
    

    Writing warnings: list[str] = [] instead raises ValueError: mutable default <class 'list'> for field warnings is not allowed: use default_factory when the class is defined.

  2. Immutable defaults. If the function only reads the value, a tuple or frozenset default is fine and avoids the None check: def tag_filter(tags=()):.

  3. Pydantic models. Pydantic copies mutable field defaults per instance, so tags: list[str] = [] is safe on a BaseModel. We still prefer Field(default_factory=list) for consistency, since the same line in a plain class or dataclass would not be safe.

How to test the fix

We added a test that calls the validator twice with different orders in the same process, which is exactly what the production workers did:

def test_warnings_do_not_leak_between_orders(make_order):
    bulky_po_box = make_order(bulky=True, po_box=True)
    plain = make_order(bulky=False, po_box=False)

    assert validate_order(bulky_po_box) == [
        "Shipping to PO boxes is not available for bulky items"
    ]
    assert validate_order(plain) == []
    assert validate_order(plain) == []  # still empty on a repeat call
pytest tests/checkout/test_validation.py -q

Before the fix the second assertion failed with the PO box warning in the list.

Confirming in production

We added a temporary structured log field, warning_count, and compared it with the number of validators that actually fired for each order. Before the deploy, 3.1% of validations returned more warnings than validators that fired, with a maximum of 41 on one long-lived worker. In the 72 hours after the deploy that number was zero across roughly 160,000 validations. Support closed the four open tickets and no new reports came in.

We also reviewed the access logs for the 13 days the bug was live. The leaked strings contained coupon codes and product categories but no names, addresses or payment details, which we confirmed by listing every message template the validators can produce.

Regression guard

We switched our linting to Ruff and enabled the flake8-bugbear rules. B006 flags mutable default arguments and B008 flags function calls in defaults, such as datetime.now():

# pyproject.toml
[tool.ruff.lint]
extend-select = ["B"]
ruff check src/
src/checkout/validation.py:14:39: B006 Do not use mutable data structures for argument defaults

Pylint users get the same coverage from W0102 dangerous-default-value. The check runs in CI as a required status, so a new def f(x=[]) cannot be merged.

Security implications

This bug crossed a user boundary: data generated for one customer was returned to another. In our case the content was low sensitivity, but the same pattern in a function that builds email context, audit metadata or permission lists could disclose personal data or grant one user another's permissions. We logged it as a privacy incident internally, documented which fields could have leaked, and confirmed none were personal data. Treat any state that outlives a request as a potential cross-tenant leak.

Prevention checklist

  • Never use [], {}, set() or a call such as datetime.now() as a default; use None and build the value inside.
  • Prefer functions that return new values over helpers that mutate an argument.
  • Use field(default_factory=...) in dataclasses and Field(default_factory=...) in Pydantic.
  • Enable Ruff B006 and B008 (or Pylint W0102) and make lint a required CI check.
  • Test request-scoped functions by calling them twice in one process with different inputs.

References

Add a label

#python #flask #gunicorn #ruff #dataclasses
Written by
Daniel Tran @fun_dev

I have spent most of my career on the side of software that pages people at night: payment integrations, web platforms with real traffic, CI/CD pipelines, Kubernetes clusters and the databases underneath them. Somewhere along the way I started keeping a notebook of every bug that cost more than an hour — the exact error, what we ruled out, the real root cause and how we proved the fix. Hibug is where those notes live. Every post follows the same path: Investigate, Explain, Fix, Verify — with the environment spelled out, the fix tested, and the security angle checked.

Advertisement

Comments

0

No comments yet. Be the first to comment!

Sign In to post a comment.