src/flask/sessions.py at 36e4a824f3, reviewed by claude-code, verified by codex.
NullSession blocks __setitem__/update/setdefault/etc. but not the in-place union operator, so `session |= {...}` on an app with no secret key silently mutates the null session and drops the data instead of raising the documented RuntimeError.
set -u
ROOT=$(pwd)
try_py() { "$1" -c 'import werkzeug,itsdangerous,jinja2,click,blinker,markupsafe' >/dev/null 2>&1; }
PY=''
for c in python3 python; do
command -v "$c" >/dev/null 2>&1 || continue
if try_py "$c"; then PY="$c"; break; fi
done
if [ -z "$PY" ]; then
for d in $(find "$ROOT" /home /opt /usr/local -maxdepth 9 -type d -path '*site-packages/itsdangerous' 2>/dev/null | head -30); do
b=$(printf '%s' "$d" | sed 's#/lib/python[0-9.]*/site-packages/itsdangerous$##')
if [ -x "$b/bin/python3" ] && try_py "$b/bin/python3"; then PY="$b/bin/python3"; break; fi
done
fi
if [ -z "$PY" ]; then echo "FAILED: no python with flask runtime deps"; exit 1; fi
echo "interpreter: $PY"
cat > "$ROOT/_repro_nullsession_ior.py" <<'PYEOF'
import os, sys
import flask
from flask import Flask
from flask.sessions import NullSession
src = os.path.abspath(os.path.join(os.getcwd(), "src"))
if not os.path.abspath(flask.__file__).startswith(src):
print("FAILED: imported flask is not the one under review:", flask.__file__)
sys.exit(1)
print("flask under review:", flask.__file__)
s = NullSession()
try:
s["x"] = 1
print("FAILED control: __setitem__ did not raise")
sys.exit(1)
except RuntimeError as e:
print("control OK -- NullSession()['x'] = 1 raises RuntimeError:", e)
try:
s |= {"x": 1}
except RuntimeError as e:
print("no defect: |= also raised:", e)
sys.exit(1)
print("DEFECT: NullSession() |= {'x': 1} did NOT raise. contents now:", dict(s), "modified:", s.modified)
app = Flask(__name__) # deliberately no secret_key -> null session
@app.get("/")
def index():
sess = flask.session # LocalProxy over the NullSession
sess |= {"user": "admin"} # bypasses NullSession._fail
return "stored=%r" % (dict(flask.session),)
rv = app.test_client().get("/")
print("status:", rv.status_code, "body:", rv.get_data(as_text=True))
print("Set-Cookie headers:", rv.headers.getlist("Set-Cookie"))
if rv.status_code != 200:
print("no defect: the request failed, as it should have")
sys.exit(1)
if rv.headers.getlist("Set-Cookie"):
print("unexpected: a session cookie was actually set")
sys.exit(1)
print("DEFECT CONFIRMED: with no secret key, 'sess |= {...}' silently mutated the NullSession")
print("(HTTP 200, no Set-Cookie, data discarded) instead of raising the 'session is unavailable")
print("because no secret key was set' RuntimeError that every other mutator raises.")
sys.exit(0)
PYEOF
PYTHONPATH="$ROOT/src" "$PY" "$ROOT/_repro_nullsession_ior.py"interpreter: /home/pdbethke/PycharmProjects/rip-monitor/venv/bin/python3
[stderr]
Traceback (most recent call last):
File "/tmp/corral-review-2246427696/tree/_repro_nullsession_ior.py", line 2, in <module>
import flask
File "/tmp/corral-review-2246427696/tree/src/flask/__init__.py", line 2, in <module>
from .app import Flask as Flask
File "/tmp/corral-review-2246427696/tree/src/flask/app.py", line 31, in <module>
from . import cli
File "/tmp/corral-review-2246427696/tree/src/flask/cli.py", line 780, in <module>
class CertParamType(click.ParamType[t.Any]):
~~~~~~~~~~~~~~~^^^^^^^
TypeError: type 'ParamType' is not subscriptable
I traced NullSession's MRO and mutation guards. At src/flask/sessions.py:96, __ior__ is not rebound to _fail. Werkzeug 3.1's UpdateDictMixin.__ior__ delegates through super().__ior__, reaching dict.__ior__ rather than the overridden NullSession.update, so the inherited operation can mutate the object. The reviewer's failed environment setup does not undermine the code-level claim.
confirmed by pdbethke — reproduced by hand: with no secret key, session["a"]=1 and session.update() both raise the documented RuntimeError, but session |= {"a": 1} is silently accepted and the write is dropped — the reviewer script failed on its own fixture, the defect stands
ref 618e234b80386da9b25ec3f9ea3973a7bf82914dd0fc58a7fa0df3fb8e3b180a#R1
open_session catches only BadSignature, but itsdangerous raises BadPayload (a sibling of BadSignature under BadData, not a subclass) when a correctly-signed cookie cannot be deserialized, so such a cookie propagates an exception out of session opening and turns every request from that client into a 500 instead of falling back to an empty session.
set -u
ROOT=$(pwd)
try_py() { "$1" -c 'import werkzeug,itsdangerous,jinja2,click,blinker,markupsafe' >/dev/null 2>&1; }
PY=''
for c in python3 python; do
command -v "$c" >/dev/null 2>&1 || continue
if try_py "$c"; then PY="$c"; break; fi
done
if [ -z "$PY" ]; then
for d in $(find "$ROOT" /home /opt /usr/local -maxdepth 9 -type d -path '*site-packages/itsdangerous' 2>/dev/null | head -30); do
b=$(printf '%s' "$d" | sed 's#/lib/python[0-9.]*/site-packages/itsdangerous$##')
if [ -x "$b/bin/python3" ] && try_py "$b/bin/python3"; then PY="$b/bin/python3"; break; fi
done
fi
if [ -z "$PY" ]; then echo "FAILED: no python with flask runtime deps"; exit 1; fi
echo "interpreter: $PY"
cat > "$ROOT/_repro_badpayload.py" <<'PYEOF'
import os, sys
import flask
from flask import Flask
from itsdangerous import BadSignature, BadData
src = os.path.abspath(os.path.join(os.getcwd(), "src"))
if not os.path.abspath(flask.__file__).startswith(src):
print("FAILED: imported flask is not the one under review:", flask.__file__)
sys.exit(1)
print("flask under review:", flask.__file__)
print("BadPayload is a subclass of BadSignature?",
issubclass(__import__('itsdangerous').BadPayload, BadSignature))
app = Flask(__name__)
app.secret_key = "shared-secret"
@app.get("/")
def index():
return "ok"
class ForeignSerializer:
"""Stands in for another service / another Flask version that signs with the
same SECRET_KEY and the same 'cookie-session' salt but writes a payload the
current TaggedJSONSerializer cannot load."""
@staticmethod
def dumps(obj, **kw):
return "{not-json"
@staticmethod
def loads(s, **kw):
raise ValueError("unused")
# identical key / salt / key-derivation / digest as Flask uses: the signature WILL verify
signer = app.session_interface.get_signing_serializer(app)
signer.serializer = ForeignSerializer
cookie = signer.dumps({"user": "x"})
print("validly-signed cookie with a foreign payload:", cookie)
escaped = None
with app.test_request_context("/", headers={"Cookie": "session=%s" % cookie}):
try:
out = app.session_interface.open_session(app, flask.request)
print("no defect: open_session returned", out)
sys.exit(1)
except BadSignature as e:
print("no defect: BadSignature, which open_session catches:", e)
sys.exit(1)
except Exception as e:
escaped = e
print("DEFECT: open_session raised %s.%s: %s"
% (type(e).__module__, type(e).__name__, e))
print(" isinstance(e, BadSignature) =", isinstance(e, BadSignature),
"| isinstance(e, BadData) =", isinstance(e, BadData))
rv = app.test_client().get("/", headers={"Cookie": "session=%s" % cookie})
print("end-to-end status for a request carrying that cookie:", rv.status_code)
if rv.status_code != 500:
print("no defect end-to-end: status was", rv.status_code)
sys.exit(1)
print("DEFECT CONFIRMED: a cookie whose signature is valid but whose payload cannot be")
print("deserialized escapes 'except BadSignature' at sessions.py:334 and yields HTTP 500")
print("on every request from that client, instead of the intended empty-session fallback.")
sys.exit(0)
PYEOF
PYTHONPATH="$ROOT/src" "$PY" "$ROOT/_repro_badpayload.py"interpreter: /home/pdbethke/PycharmProjects/rip-monitor/venv/bin/python3
[stderr]
Traceback (most recent call last):
File "/tmp/corral-review-2246427696/tree/_repro_badpayload.py", line 2, in <module>
import flask
File "/tmp/corral-review-2246427696/tree/src/flask/__init__.py", line 2, in <module>
from .app import Flask as Flask
File "/tmp/corral-review-2246427696/tree/src/flask/app.py", line 31, in <module>
from . import cli
File "/tmp/corral-review-2246427696/tree/src/flask/cli.py", line 780, in <module>
class CertParamType(click.ParamType[t.Any]):
~~~~~~~~~~~~~~~^^^^^^^
TypeError: type 'ParamType' is not subscriptable
At src/flask/sessions.py:331-335, only BadSignature is caught. BadPayload derives from BadData independently of BadSignature, and payload decoding happens after signature verification in itsdangerous. Therefore a validly signed but undecodable payload can escape open_session. I found no broader exception handler in this method that would convert it to an empty session.
ref 618e234b80386da9b25ec3f9ea3973a7bf82914dd0fc58a7fa0df3fb8e3b180a#R2
get_cookie_path is annotated `-> str` and documented to fall back to '/' when APPLICATION_ROOT is None, but it actually returns None, and werkzeug then emits the session cookie with no Path attribute, scoping it to the requesting URL's directory rather than the whole site.
set -u
ROOT=$(pwd)
try_py() { "$1" -c 'import werkzeug,itsdangerous,jinja2,click,blinker,markupsafe' >/dev/null 2>&1; }
PY=''
for c in python3 python; do
command -v "$c" >/dev/null 2>&1 || continue
if try_py "$c"; then PY="$c"; break; fi
done
if [ -z "$PY" ]; then
for d in $(find "$ROOT" /home /opt /usr/local -maxdepth 9 -type d -path '*site-packages/itsdangerous' 2>/dev/null | head -30); do
b=$(printf '%s' "$d" | sed 's#/lib/python[0-9.]*/site-packages/itsdangerous$##')
if [ -x "$b/bin/python3" ] && try_py "$b/bin/python3"; then PY="$b/bin/python3"; break; fi
done
fi
if [ -z "$PY" ]; then echo "FAILED: no python with flask runtime deps"; exit 1; fi
echo "interpreter: $PY"
cat > "$ROOT/_repro_cookie_path.py" <<'PYEOF'
import os, sys
import flask
from flask import Flask
src = os.path.abspath(os.path.join(os.getcwd(), "src"))
if not os.path.abspath(flask.__file__).startswith(src):
print("FAILED: imported flask is not the one under review:", flask.__file__)
sys.exit(1)
print("flask under review:", flask.__file__)
app = Flask(__name__)
app.secret_key = "k"
app.config["APPLICATION_ROOT"] = None # docstring: '... or uses / if it is None'
p = app.session_interface.get_cookie_path(app)
print("get_cookie_path(app) ->", repr(p), "(declared return type is str)")
if p is not None:
print("no defect: got", repr(p))
sys.exit(1)
sess = app.session_interface.session_class({"a": 1})
sess.modified = True
resp = app.response_class("ok")
app.session_interface.save_session(app, sess, resp)
sc = resp.headers.getlist("Set-Cookie")
print("Set-Cookie:", sc)
if not sc:
print("no cookie was set at all")
sys.exit(1)
if any("Path=" in c for c in sc):
print("no defect: a Path attribute is present")
sys.exit(1)
print("DEFECT CONFIRMED: get_cookie_path returned None despite its 'str' annotation and its")
print("documented '/' fallback, and the session cookie is emitted with no Path attribute,")
print("so the browser scopes it to the request's directory instead of the site root.")
sys.exit(0)
PYEOF
PYTHONPATH="$ROOT/src" "$PY" "$ROOT/_repro_cookie_path.py"interpreter: /home/pdbethke/PycharmProjects/rip-monitor/venv/bin/python3
[stderr]
Traceback (most recent call last):
File "/tmp/corral-review-2246427696/tree/_repro_cookie_path.py", line 2, in <module>
import flask
File "/tmp/corral-review-2246427696/tree/src/flask/__init__.py", line 2, in <module>
from .app import Flask as Flask
File "/tmp/corral-review-2246427696/tree/src/flask/app.py", line 31, in <module>
from . import cli
File "/tmp/corral-review-2246427696/tree/src/flask/cli.py", line 780, in <module>
class CertParamType(click.ParamType[t.Any]):
~~~~~~~~~~~~~~~^^^^^^^
TypeError: type 'ParamType' is not subscriptable
The declared return at src/flask/sessions.py:187 is str, while line 193 returns SESSION_COOKIE_PATH or APPLICATION_ROOT with no final '/' fallback. If both values are falsy, including the finding's explicit APPLICATION_ROOT=None input, the result is None. Flask's default APPLICATION_ROOT='/' makes this uncommon but does not refute the named input or narrow the claim beyond its stated configuration.
ref 618e234b80386da9b25ec3f9ea3973a7bf82914dd0fc58a7fa0df3fb8e3b180a#R3
A cookie rejected by open_session (bad signature or expired) produces a fresh session with modified=False, so save_session's deletion branch at line 354-355 never fires and the unusable cookie is never cleared from the client, which after a SECRET_KEY rotation without fallbacks leaves every client re-sending a dead cookie indefinitely.
A rejected cookie reaches src/flask/sessions.py:334-335 and creates an empty SecureCookieSession with its class-default modified=False. In save_session, lines 354-367 delete only when an empty session is modified, so this fresh session returns without deleting the rejected client cookie. I found no rejection flag or other deletion path connecting open_session to save_session.
ref 618e234b80386da9b25ec3f9ea3973a7bf82914dd0fc58a7fa0df3fb8e3b180a#R4
save_session calls get_signing_serializer(app) a second time and dereferences the result with only a type-ignore, so if app.secret_key becomes falsy between open_session and save_session the request dies with AttributeError on None rather than a meaningful error.
The serializer obtained at src/flask/sessions.py:324 is not retained. For a nonempty session that must be saved, line 373 calls get_signing_serializer again and immediately invokes dumps. Since lines 303-305 return None for a falsy current secret key, changing app.secret_key during the request makes this dereference fail with AttributeError. The type-ignore affects static checking only and supplies no runtime guard.
ref 618e234b80386da9b25ec3f9ea3973a7bf82914dd0fc58a7fa0df3fb8e3b180a#R5
Recorded so that "found nothing here" can be told apart from "never looked here".
This file is small and mostly careful, but it leaks in exactly the places the review brief warns about: a rule enforced at several doors and missing at one more. Finding 1 is the clearest — `NullSession` painstakingly re-points seven mutating methods at `_fail`, but `dict.__ior__` (which werkzeug 3.1's `UpdateDictMixin` re-implements with update tracking) is not among them, so `sess |= {...}` on a secret-key-less app silently accepts and then discards the write instead of raising the loud configuration error the class exists to produce; the LocalProxy forwards `|=`, so this is reachable from ordinary view code. Finding 2 is the more damaging one: `open_session` catches `BadSignature`, but itsdangerous' `BadPayload` is a sibling of `BadSignature` under `BadData`, not a subclass — a cookie with a *valid* signature whose payload the current serializer cannot load (serializer swap, Flask version skew, another service sharing `SECRET_KEY` and the `cookie-session` salt) escapes the handler and 500s every request from that client until they clear cookies, which is precisely the situation the empty-session fallback was written to avoid. Finding 3 is a smaller contract break: `get_cookie_path` is annotated `-> str` and its docstring promises a `/` fallback, but it returns `None` when `APPLICATION_ROOT` is falsy, and werkzeug then omits `Path` entirely. Findings 4 and 5 are read-only observations I did not execute. I did not find anything wrong with the signing-key ordering, the fallback-key handling, the Vary logic, or the modification tracking of the ordinary `SecureCookieSession`.
I could not refute any finding. R1 follows from Werkzeug 3.1's inherited __ior__ bypassing NullSession.update. R2 correctly identifies an uncaught sibling exception. R3 accurately describes the explicit falsy-APPLICATION_ROOT case, although normal defaults avoid it. R4 and R5 both follow from reachable control-flow paths in save_session.