Skip to content

Threat model chapter 5#13098

Open
Dreamsorcerer wants to merge 3 commits into
masterfrom
threat-model5
Open

Threat model chapter 5#13098
Dreamsorcerer wants to merge 3 commits into
masterfrom
threat-model5

Conversation

@Dreamsorcerer

Copy link
Copy Markdown
Member

No description provided.

@Dreamsorcerer Dreamsorcerer added bot:chronographer:skip This PR does not need to include a change note backport-3.15 Trigger automatic backporting to the 3.15 release branch by Patchback robot labels Jul 8, 2026
@codecov

codecov Bot commented Jul 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.97%. Comparing base (9e00085) to head (b0d098e).
⚠️ Report is 4 commits behind head on master.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #13098   +/-   ##
=======================================
  Coverage   98.97%   98.97%           
=======================================
  Files         131      131           
  Lines       48635    48635           
  Branches     2525     2525           
=======================================
  Hits        48135    48135           
  Misses        376      376           
  Partials      124      124           
Flag Coverage Δ
Autobahn 22.12% <ø> (ø)
CI-GHA 98.92% <ø> (ø)
OS-Linux 98.68% <ø> (+<0.01%) ⬆️
OS-Windows 97.07% <ø> (ø)
OS-macOS 97.96% <ø> (-0.01%) ⬇️
Py-3.10 98.16% <ø> (-0.01%) ⬇️
Py-3.11 98.43% <ø> (ø)
Py-3.12 98.51% <ø> (ø)
Py-3.13 98.49% <ø> (+<0.01%) ⬆️
Py-3.14 98.50% <ø> (-0.01%) ⬇️
Py-3.14t 97.61% <ø> (+<0.01%) ⬆️
Py-pypy-3.11 97.47% <ø> (+<0.01%) ⬆️
VM-macos 97.96% <ø> (-0.01%) ⬇️
VM-ubuntu 98.68% <ø> (+<0.01%) ⬆️
VM-windows 97.07% <ø> (ø)
cython-coverage 37.93% <ø> (ø)

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

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

@codspeed-hq

codspeed-hq Bot commented Jul 8, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 83 untouched benchmarks
⏩ 83 skipped benchmarks1


Comparing threat-model5 (74ee3d4) with master (9e00085)

Open in CodSpeed

Footnotes

  1. 83 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.

@Dreamsorcerer

Copy link
Copy Markdown
Member Author

@aiolibsbot review

@aiolibsbot

Copy link
Copy Markdown
Contributor

PR Review — Threat model chapter 5

Well-researched threat-model addition (§5.5 Compression codecs); merge-ready with two doc-accuracy nits.

Strengths:

  • Every technical claim I spot-checked holds: the 32 MiB→256 KiB cap evolution, feed_eof's assert not chunk residual check (http_parser.py:1184), the isal _last_empty "no further data" workaround, and the set_zlib_backend/protocol surface all match the code.
  • Threat-ID numbering (5.x) correctly follows the section's own convention (§5.3→3.x, §5.4→4.x).
  • STRIDE table and mitigations are grounded in real call sites, and the cancellation-hazard note aligns with the WARNING docstrings in ZLibCompressor.compress.

Needs attention (both non-blocking suggestions):

  • DEFAULT_MAX_DECOMPRESS_SIZE was renamed to DEFAULT_CHUNK_SIZE in Use DEFAULT_CHUNK_SIZE global #12356 and no longer exists in the tree — values are right, symbol is stale for the audit trail.
  • Forward anchors §5.6/§5.9/§5.12 don't resolve yet (matches the existing incremental convention, so informational).

🟢 Suggestions

1. Stale constant name: `DEFAULT_MAX_DECOMPRESS_SIZE` was renamed to `DEFAULT_CHUNK_SIZE`
THREAT_MODEL.md:777-790

The recap names DEFAULT_MAX_DECOMPRESS_SIZE in two places (the CVE-2025-69223 bullet: DEFAULT_MAX_DECOMPRESS_SIZE = 2**25, and the #11966 bullet: 256 KiB (DEFAULT_MAX_DECOMPRESS_SIZE ...)).

I verified the history:

Why it matters: the values quoted are all correct, and the two bullets are describing point-in-time history so they aren't wrong. But AGENTS.md designates this recap as "the audit trail for what has been verified-in-place," and someone auditing the current code who greps for DEFAULT_MAX_DECOMPRESS_SIZE finds nothing. A one-line note that the constant is now DEFAULT_CHUNK_SIZE (same 256 KiB value) keeps the trail greppable.

cap `DEFAULT_MAX_DECOMPRESS_SIZE = 2**25`, i.e. 32 MiB per `decompress()` call

Checklist

  • Technical claims match the code (caps, asserts, backend quirks)
  • Referenced symbols/constants exist in current tree — suggestion #1
  • Internal cross-reference anchors resolve
  • Threat-ID numbering consistent with section convention
  • CVE/GHSA identifiers and PR numbers accurate

Automated review by Kōan (Claude) HEAD=74ee3d4 4 min 16s

Comment thread THREAT_MODEL.md
Comment on lines +800 to +802
1. Is the cancellation hazard on `ZLibCompressor.compress()` (threat 5.3)
adequately covered by the existing connection-write-lock pattern at
every call site, or is there an unaudited path?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aiolibsbot Can you answer this by performing the audit?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-3.15 Trigger automatic backporting to the 3.15 release branch by Patchback robot bot:chronographer:skip This PR does not need to include a change note

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants