docs: verify threat models for completeness and fix silent gaps#1330
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (9)
WalkthroughThis PR renumbers Winget-related asset identifiers across threat-model sources and generated documentation, adds threat coverage validation in the renderer, and updates OSCAL version references and component-definition metadata. It also adds a reserved-control-IDs section and new threat entries for command injection and credential persistence. ChangesThreat model updates
OSCAL compliance documentation update
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant TM as TM._threats
participant Render as _render_threat_rows
participant Validate as _validate_threat_coverage
participant Output as Rendered Table
Render->>TM: build threat catalog
Render->>Validate: validate(findings, responses)
Validate-->>Render: raise ValueError or pass
Render->>Output: render findings and catalog-only threats
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 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 `@security/tm_render.py`:
- Around line 178-197: The early return in tm_render’s findings handling skips
_validate_threat_coverage() and prevents response-only rows from being rendered.
In the tm_render flow, move coverage validation before the empty-findings exit
and only return immediately when both tm.findings and responses are empty. Keep
the existing representative-building logic in tm_render so response threat IDs
not present in findings still get catalog-backed rows and unknown IDs still
trigger validation.
🪄 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: 709dfd35-5fa0-4670-9c26-7e6764cd5156
📒 Files selected for processing (9)
doc/explanation/security.rstdoc/explanation/threat_model_supply_chain.rstdoc/explanation/threat_model_usage.rstsecurity/README.mdsecurity/dfetch.component-definition.jsonsecurity/tm_controls_data.pysecurity/tm_render.pysecurity/tm_supply_chain.pysecurity/tm_usage.py
c4baf59 to
be98353
Compare
Cross-checking the threat catalog, responses, controls, and generated reports against each other and the codebase surfaced several gaps: - DFT-06 (command injection) and DFT-34 (long-lived stored credential) had documented responses and mitigating controls (C-006/C-007, C-015/C-017, C-042) but never appeared in any rendered threat table: DFT-06's condition no longer matches once parameterized subprocess input is modelled, and pytm's resolve() never evaluates threats against Data assets (it iterates TM._elements only). Render such responses from the threat catalog so every documented risk decision stays visible, and fail generation when a finding has no response or a response names an unknown threat ID. - Asset IDs A-09/A-10 meant different assets in the two models (Remote VCS Server / Archive HTTP Server in the usage model, Winget repo / WINGET_TOKEN PAT in the supply-chain model). Renumber the Winget assets to A-28/A-29 so every asset ID is unique across the doc set. - Gap control IDs C-018/C-019/C-020/C-035 cited by the usage model were defined nowhere; add an 'Identified Gaps' section to the security page so each reserved ID resolves to a description. - Fix the SBOM cross-reference in the usage model (A-04 -> A-03) and the stale OSCAL version (1.1.2 -> 1.2.2) in security.rst and security/README.md. - Regenerate both threat-model reports (also picks up the stale C-045/C-036 control-row ordering in the usage report).
be98353 to
c389926
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
Cross-checking the threat catalog, responses, controls, and generated
reports against each other and the codebase surfaced several gaps:
had documented responses and mitigating controls (C-006/C-007,
C-015/C-017, C-042) but never appeared in any rendered threat table:
DFT-06's condition no longer matches once parameterized subprocess
input is modelled, and pytm's resolve() never evaluates threats
against Data assets (it iterates TM._elements only). Render such
responses from the threat catalog so every documented risk decision
stays visible, and fail generation when a finding has no response or
a response names an unknown threat ID.
VCS Server / Archive HTTP Server in the usage model, Winget repo /
WINGET_TOKEN PAT in the supply-chain model). Renumber the Winget
assets to A-28/A-29 so every asset ID is unique across the doc set.
defined nowhere; add an 'Identified Gaps' section to the security
page so each reserved ID resolves to a description.
the stale OSCAL version (1.1.2 -> 1.2.2) in security.rst and
security/README.md.
C-045/C-036 control-row ordering in the usage report).
Summary by CodeRabbit
New Features
Bug Fixes
Documentation