Threat model chapter 3

[Dreamsorcerer]

No description provided.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.95%. Comparing base (feb0405) to head (4c47d29).
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #12677   +/-   ##
=======================================
  Coverage   98.95%   98.95%           
=======================================
  Files         131      131           
  Lines       46688    46688           
  Branches     2421     2421           
=======================================
  Hits        46200    46200           
  Misses        366      366           
  Partials      122      122           

Flag	Coverage Δ
Autobahn	22.42% <ø> (ø)
CI-GHA	98.92% <ø> (ø)
OS-Linux	98.68% <ø> (ø)
OS-Windows	97.03% <ø> (ø)
OS-macOS	97.93% <ø> (ø)
Py-3.10	98.15% <ø> (ø)
Py-3.11	98.41% <ø> (-0.01%)	⬇️
Py-3.12	98.50% <ø> (ø)
Py-3.13	98.47% <ø> (-0.01%)	⬇️
Py-3.14	98.50% <ø> (+<0.01%)	⬆️
Py-3.14t	97.55% <ø> (+<0.01%)	⬆️
Py-pypy-3.11	97.42% <ø> (-0.02%)	⬇️
VM-macos	97.93% <ø> (ø)
VM-ubuntu	98.68% <ø> (ø)
VM-windows	97.03% <ø> (ø)
cython-coverage	37.94% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[codspeed-hq]

Merging this PR will not alter performance

✅ 72 untouched benchmarks
⏩ 72 skipped benchmarks¹

---

_{Comparing threat-model3 (4c47d29) with master (feb0405)}

Footnotes

1. 72 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

[bdraco]

@aiolibsbot review

[aiolibsbot]

PR Review — Threat model chapter 3

Substantively this is a strong, accurate threat-model chapter. I verified every technical claim against the source: the absent client-mask enforcement (3.1), the random.getrandbits(32) mask source at writer.py:63 (3.2), RSV gating (3.3), control-frame/fragmentation rules (3.4–3.7), the max_msg_size + 1 post-decompress check and the isal overshoot caveat with PR #11898 (3.9), context-takeover via Z_SYNC_FLUSH/Z_FULL_FLUSH (3.10), strict UTF-8 decode (3.11), the < 3000 close-code validation (3.12), no writer auto-fragmentation (3.13), and the single-backend test import vs. test_http_parser's dual parametrization (3.15) — all correct, including the 4 MiB default max_msg_size. Two documentation defects hold it back from clean: (1) four cross-reference anchors (§5.5, §5.11, §5.14, and §6.1×3) point to sections that don't exist yet in the file, so they render as dead links; and (2) the components list cites reader_c.pyx, which doesn't exist — it should be reader_c.pxd (the Cython backend is a symlinked .py + .pxd, not a .pyx). Neither is a correctness problem in the analysis itself; both are accuracy/link-hygiene issues the author should consciously resolve given this is part of an incremental series.

---

🟡 Important

1. Dead cross-reference anchors — §5.5, §5.11, §5.14, §6.1 don't exist yet (`THREAT_MODEL.md`, L442-443)

§5.3 links to four sections that are not present in THREAT_MODEL.md. The file currently ends at §5.2 (HTTP/1 writer); there is no §5.4+, and no §6 at all. The following anchors will not resolve when the markdown is rendered (e.g. on GitHub):

• [§5.11](#511-server-side-websocket-handler) and [§5.14](#514-client-side-websocket) (line ~442)
• [§5.5](#55-compression-codecs) (line ~443)
• [§6.1](#61-highest-leverage-recommendations) — referenced 3 times in the Mitigations table (rows 3.14 and 3.15) as the canonical home of "recommendations The WSGI applications which are generators can now use "yield" #3", but §6 does not exist.

Verified: grep -nE '^### 5\.(4|5|11|14)|^## 6|^### 6\.1' THREAT_MODEL.md returns nothing. For an incrementally-built, multi-chapter document these are presumably forward references to future chapters, so this may be an accepted tradeoff — but the links are objectively broken in this PR's rendered output. Options: (a) drop the parenthetical links until those sections land, (b) keep the §-number text without the […](#…) anchor, or (c) acknowledge that they're intentional placeholders. Your call given the series structure, but worth a deliberate decision rather than shipping dead anchors.

lifecycle (covered in [§5.11](#511-server-side-websocket-handler) server, [§5.14](#514-client-side-websocket) client) and the underlying
compression codecs themselves ([§5.5](#55-compression-codecs)).

🟢 Suggestions

1. `reader_c.pyx` does not exist — should be `reader_c.pxd` (`THREAT_MODEL.md`, L453)

The components list reads reader.py / reader_c.pyx / reader_c.py / reader_py.py. There is no reader_c.pyx in aiohttp/_websocket/. Unlike mask (which has a real mask.pyx), the Cython reader is built from reader_c.py — a symlink to reader_py.py — augmented by the type-declaration file reader_c.pxd. So the single Python source is shared by both backends and Cython-typed via the .pxd.

Actual files: reader.py (import shim), reader_py.py (source), reader_c.py → reader_py.py (symlink), reader_c.pxd (Cython augmentation). Replace reader_c.pyx with reader_c.pxd. In a document whose value is accuracy about the codebase, this filename error is worth fixing — it also slightly misrepresents the build mechanism (shared .py source, not a separate .pyx).

  - `reader.py` / `reader_c.pyx` / `reader_c.py` / `reader_py.py` — frame
    reader (Cython preferred, pure-Python fallback).

---

Checklist

• Threat descriptions match actual code behavior
• File/function/line references are accurate — suggestion ＃1
• Internal cross-reference links resolve — warning ＃1
• PR/CVE attribution (PR Use decompressor max_length parameter #11898) verified
• Default values cited correctly (max_msg_size 4 MiB)

---

Summary

Substantively this is a strong, accurate threat-model chapter. I verified every technical claim against the source: the absent client-mask enforcement (3.1), the random.getrandbits(32) mask source at writer.py:63 (3.2), RSV gating (3.3), control-frame/fragmentation rules (3.4–3.7), the max_msg_size + 1 post-decompress check and the isal overshoot caveat with PR #11898 (3.9), context-takeover via Z_SYNC_FLUSH/Z_FULL_FLUSH (3.10), strict UTF-8 decode (3.11), the < 3000 close-code validation (3.12), no writer auto-fragmentation (3.13), and the single-backend test import vs. test_http_parser's dual parametrization (3.15) — all correct, including the 4 MiB default max_msg_size. Two documentation defects hold it back from clean: (1) four cross-reference anchors (§5.5, §5.11, §5.14, and §6.1×3) point to sections that don't exist yet in the file, so they render as dead links; and (2) the components list cites reader_c.pyx, which doesn't exist — it should be reader_c.pxd (the Cython backend is a symlinked .py + .pxd, not a .pyx). Neither is a correctness problem in the analysis itself; both are accuracy/link-hygiene issues the author should consciously resolve given this is part of an incremental series.

---

To rebase specific severity levels, mention me: @aiolibsbot rebase critical (fixes 🔴 only), @aiolibsbot rebase important (fixes 🔴 + 🟡), or just @aiolibsbot rebase for all.

---

Automated review by Kōan6555d4a
4c47d29