Improve the threat model docs#1208
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (6)
WalkthroughAdds diagram rendering helpers to threat-model reporting, extends supply-chain and usage models to return and connect additional actors/processes/datastores via new completion flows, inserts generated DFD/sequence sections into RST docs, and updates the CLI allowlist to run the new modules. ChangesThreat-Model Diagram Rendering and Documentation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@doc/explanation/threat_model_supply_chain.rst`:
- Line 300: The generated RST contains hardcoded absolute image paths coming
from pytm's diagram output called via run_model() in security/tm_supply_chain.py
and security/tm_usage.py; fix by post-processing the generated RST in the code
path that writes the diagrams (the run_model() caller) to replace absolute
site-package image paths (e.g., "/home/dev/.local/.../pytm/images/*.png") with
portable references (relative paths or data URIs), or add a small monkey-patch
in the module that invokes pytm to override pytm's image path generation before
run_model() is called so the produced DOT/RST uses relative names; ensure the
replacement runs for all occurrences (threat_model_supply_chain.rst and
threat_model_usage.rst) after diagram generation.
In `@doc/explanation/threat_model_usage.rst`:
- Around line 425-557: The DOT output embeds non-portable absolute image paths
for nodes (e.g., datastore_AdfetchManifest_9345ab4c19,
datastore_AFetchedSourceCode_86e4604564,
datastore_AIntegrityHashRecord_b2e5892d06,
datastore_ASBOMOutputCycloneDX_990b886585,
process_SVNExportsvnexport_7eb89910ee, datastore_ALocalVCSCachetemp_ae6e32d09a,
datastore_AAuditCheckReports_d0c0ca0a3b,
externalentity_ARemoteVCSServer_d2006ce1bb), which breaks rendering off your
machine; update the diagram renderer that emits these nodes so it uses portable
Graphviz-native shapes (e.g., cylinder, box, none) or repo-relative/static asset
URLs instead of absolute filesystem paths, then regenerate the DOT output to
remove the /home/.../pytm/images/... references and verify the nodes above now
use the portable styling.
In `@security/tm_common.py`:
- Line 316: The current indentation code assigns indented = "\n".join(" " +
line for line in wrapped.splitlines()), which prepends spaces to blank lines;
update the generator used to build indented (and the similar assignment at the
other occurrence) to only prepend the three-space prefix when the line is not
empty (e.g., use a conditional in the comprehension: prefix + line if line else
"" or check line.strip()), keeping blank lines unindented to avoid trailing
whitespace/linter warnings while preserving existing behavior for non-empty
lines.
- Around line 312-318: The DFD/sequence rendering helpers should avoid emitting
headers and an empty ".. uml::" block when tm.dfd() or tm.seq() returns None or
an empty string; in _render_dfd_section (and likewise in _render_seq_section)
check the output of tm.dfd() / tm.seq() for falsy/whitespace-only content and
return an empty string (or skip rendering) when there's no diagram to display,
otherwise proceed to wrap, indent and return the full section; reference the
functions _render_dfd_section, _render_seq_section and the methods tm.dfd() and
tm.seq() when making the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 963f2c4c-7671-4dc3-a9fe-1964ba94a854
📒 Files selected for processing (4)
.claude/settings.jsondoc/explanation/threat_model_supply_chain.rstdoc/explanation/threat_model_usage.rstsecurity/tm_common.py
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@doc/explanation/threat_model_usage.rst`:
- Around line 357-807: The threat_model_usage.rst block is stale: the generated
UML output is missing the injected "skinparam defaultFontSize 16" and the
fullscreen raw-HTML wrapper emitted by security/tm_common.py; fix by
regenerating the usage output with the current renderer (security.tm_usage) so
the DF/Sequence diagram sections include the fullscreen raw-HTML container and
the injected skinparam, or alternatively update the diagram block to match the
output produced by security.tm_common.py (ensure the wrapper and the "skinparam
defaultFontSize 16" appear around the lines corresponding to the DF/Sequence UML
output).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: b78ea30e-ebb4-47fa-9c24-b97903882fd7
📒 Files selected for processing (6)
.claude/settings.jsondoc/explanation/threat_model_supply_chain.rstdoc/explanation/threat_model_usage.rstsecurity/tm_common.pysecurity/tm_supply_chain.pysecurity/tm_usage.py
Summary by CodeRabbit
Documentation
New Features
Chores