Skip to content

Commit c389926

Browse files
claudespoorcc
authored andcommitted
docs: verify threat models for completeness and fix silent gaps
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).
1 parent b96dae4 commit c389926

9 files changed

Lines changed: 162 additions & 40 deletions

doc/explanation/security.rst

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,46 @@ for regeneration instructions.
173173
threat_model_supply_chain
174174
threat_model_usage
175175

176+
.. _identified-gaps:
177+
178+
Identified Gaps (Reserved Control IDs)
179+
--------------------------------------
180+
181+
The runtime-usage threat model references four control IDs that are reserved
182+
for improvements not yet implemented. Reserving the IDs keeps the control
183+
numbering stable and makes each gap a first-class part of the risk analysis —
184+
you can trace every "gap C-0xx" mention in the threat models back to this
185+
table:
186+
187+
.. list-table::
188+
:header-rows: 1
189+
:widths: 10 90
190+
191+
* - ID
192+
- Planned control
193+
* - C-018
194+
- Integrity-hash enforcement for archive dependencies. The
195+
``integrity.hash`` manifest field is optional today; when it is absent no
196+
content verification occurs. You can bootstrap the hash for an existing
197+
manifest with ``dfetch freeze``.
198+
* - C-019
199+
- End-to-end content integrity for Git and SVN dependencies. Authenticity
200+
currently relies on transport security (TLS / SSH) and the integrity of
201+
the upstream repository; pinning to an immutable commit SHA is the
202+
strongest mitigation available today.
203+
* - C-020
204+
- Patch-file integrity verification. ``patch:`` files referenced by the
205+
manifest carry no hash, and the destination paths inside a patch are
206+
validated by ``patch-ng`` rather than independently by dfetch.
207+
* - C-035
208+
- Upstream source-attestation verification. dfetch has no mechanism yet to
209+
request or verify SLSA Source Provenance Attestations (:term:`VSA`) for
210+
fetched repositories, and the manifest schema has no field to declare an
211+
expected SLSA source level.
212+
213+
When one of these controls is implemented, it moves to the
214+
:doc:`control_register` under its reserved ID.
215+
176216
CRA Compliance
177217
--------------
178218

@@ -235,7 +275,7 @@ OSCAL
235275

236276
:term:`OSCAL` (Open Security Controls Assessment Language) is a set of
237277
NIST-published JSON/XML schemas for machine-readable security documentation.
238-
dfetch uses OSCAL 1.1.2 for two artifacts:
278+
dfetch uses OSCAL 1.2.2 for two artifacts:
239279

240280
- `OSCAL Catalog model <https://pages.nist.gov/OSCAL/reference/latest/catalog/>`_ —
241281
used by ``cra_pren_4000014_oscal_catalog.json`` to represent the prEN 40000-1-4

doc/explanation/threat_model_supply_chain.rst

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ Boundaries
8282
- Python Package Index and its staging registry. dfetch publishes via OIDC trusted publishing - no long-lived API token stored.
8383

8484
* - Winget Community Repository
85-
- The Windows Package Manager Community Repository (https://github.com/microsoft/winget-pkgs) where dfetch's Winget manifest is hosted. Manifest PRs are submitted automatically by the CI release pipeline (winget-publish.yml) using the stored WINGET_TOKEN PAT (A-10). Consumer installations resolve manifests from this repository; winget downloads the MSI installer from the URL declared in the manifest (pointing to GitHub Releases, A-01) and verifies its SHA256 hash.
85+
- The Windows Package Manager Community Repository (https://github.com/microsoft/winget-pkgs) where dfetch's Winget manifest is hosted. Manifest PRs are submitted automatically by the CI release pipeline (winget-publish.yml) using the stored WINGET_TOKEN PAT (A-29). Consumer installations resolve manifests from this repository; winget downloads the MSI installer from the URL declared in the manifest (pointing to GitHub Releases, A-01) and verifies its SHA256 hash.
8686

8787

8888
Data Flow Diagram
@@ -258,7 +258,7 @@ Data Flow Diagram
258258
shape = square;
259259
color = black;
260260
fontcolor = black;
261-
label = "A-09: Winget\nCommunity\nRepository\n(microsoft/winget-\npkgs)";
261+
label = "A-28: Winget\nCommunity\nRepository\n(microsoft/winget-\npkgs)";
262262
margin = 0.02;
263263
]
264264

@@ -458,7 +458,7 @@ Sequence Diagram
458458
entity process_APythonBuildwheelsdist_b2e5892d06 as "A-08: Python\nBuild (wheel\n/ sdist)"
459459
database datastore_AdfetchBuildDevDependencies_990b886585 as "A-07: dfetch\nBuild / Dev\nDependencies"
460460
database datastore_AbGitHubActionsBuildCache_9df04f8dae as "A-08b:\nGitHub\nActions\nBuild Cache"
461-
entity externalentity_AWingetCommunityRepositorymicrosoftwingetpkgs_7113ed0f48 as "A-09: Winget\nCommunity\nRepository\n(microsoft/winget-pkgs)"
461+
entity externalentity_AWingetCommunityRepositorymicrosoftwingetpkgs_7113ed0f48 as "A-28: Winget\nCommunity\nRepository\n(microsoft/winget-pkgs)"
462462

463463
actor_DeveloperContributor_d2006ce1bb -> externalentity_AbGitHubRepositoryfeaturebranchesPRs_0291419f72: DF-11: Push commits / open PR
464464
externalentity_AbGitHubRepositoryfeaturebranchesPRs_0291419f72 -> process_AReleaseGateCodeReview_9345ab4c19: DF-22: PR enters code review
@@ -554,7 +554,7 @@ Asset Identification
554554
- Data
555555
- High / High / High
556556
* - A-06: GitHub Actions Workflow
557-
- CI/CD pipelines: test, build (wheel/msi/deb/rpm), lint, CodeQL, Scorecard, dependency-review, docs, release, winget-publish. All actions pinned by commit SHA. harden-runner used in every workflow that executes steps on a runner (egress: block with endpoint allowlist); ci.yml is a dispatcher-only workflow with no runner steps and does not include harden-runner. winget-publish.yml uses a stored PAT (WINGET_TOKEN, A-10) to submit manifest PRs to the Winget Community Repository (A-09).
557+
- CI/CD pipelines: test, build (wheel/msi/deb/rpm), lint, CodeQL, Scorecard, dependency-review, docs, release, winget-publish. All actions pinned by commit SHA. harden-runner used in every workflow that executes steps on a runner (egress: block with endpoint allowlist); ci.yml is a dispatcher-only workflow with no runner steps and does not include harden-runner. winget-publish.yml uses a stored PAT (WINGET_TOKEN, A-29) to submit manifest PRs to the Winget Community Repository (A-28).
558558
- Process
559559
- Medium / Medium / Medium
560560
* - A-07: dfetch Build / Dev Dependencies
@@ -569,11 +569,11 @@ Asset Identification
569569
- GitHub Actions cache entries written and restored across pipeline runs. Used to speed up dependency installation (pip, gem) and incremental builds. Cache-poisoning from forked PRs (DFT-28, SLSA E6: poison the build cache) is mitigated by ref-scoped cache keys: build.yml includes ``${{ github.ref_name }}`` in both ``key`` and ``restore-keys`` (C-033), which isolates PR and release caches per branch so a fork cannot write into the release cache namespace.
570570
- Datastore
571571
- High / High / —
572-
* - A-09: Winget Community Repository (microsoft/winget-pkgs)
573-
- The Windows Package Manager Community Repository where the dfetch ``DFetch-org.DFetch`` manifest is hosted (https://github.com/microsoft/winget-pkgs). CI submits manifest update PRs via ``vedantmgoyal9/winget-releaser`` using a stored PAT (A-10); PRs are reviewed by ``microsoft/winget-pkgs`` maintainers before merging (C-041). Manifests contain SHA256 hashes of the installer binary; winget verifies the hash before installation. A compromised PAT or a fraudulent PR that passes review could redirect consumers to a malicious installer (DFT-35).
572+
* - A-28: Winget Community Repository (microsoft/winget-pkgs)
573+
- The Windows Package Manager Community Repository where the dfetch ``DFetch-org.DFetch`` manifest is hosted (https://github.com/microsoft/winget-pkgs). CI submits manifest update PRs via ``vedantmgoyal9/winget-releaser`` using a stored PAT (A-29); PRs are reviewed by ``microsoft/winget-pkgs`` maintainers before merging (C-041). Manifests contain SHA256 hashes of the installer binary; winget verifies the hash before installation. A compromised PAT or a fraudulent PR that passes review could redirect consumers to a malicious installer (DFT-35).
574574
- ExternalEntity
575575
- High / High / —
576-
* - A-10: WINGET_TOKEN PAT
576+
* - A-29: WINGET_TOKEN PAT
577577
- Long-lived classic GitHub Personal Access Token (fine-grained PATs are not supported by the ``vedantmgoyal9/winget-releaser`` action) with ``public_repo`` scope, stored as a GitHub Actions environment secret in the ``winget`` environment. Used by ``winget-publish.yml`` to push a manifest branch to the pre-existing fork ``dfetch-org/winget-pkgs`` and open a PR against ``microsoft/winget-pkgs``. Unlike the PyPI OIDC token (A-05) which is short-lived and not stored, this PAT persists indefinitely until rotated. If exfiltrated from the CI environment, an attacker could submit fraudulent manifest PRs from outside the project's pipeline.
578578
- Data
579579
- High / High / —
@@ -677,16 +677,16 @@ Dataflows
677677

678678
* - DF-27: Winget manifest PR submission
679679
- A-02: GitHub Actions Infrastructure
680-
- A-09: Winget Community Repository (microsoft/winget-pkgs)
680+
- A-28: Winget Community Repository (microsoft/winget-pkgs)
681681
- HTTPS
682682

683683
* - DF-28: winget install dfetch
684684
- Consumer / End User
685-
- A-09: Winget Community Repository (microsoft/winget-pkgs)
685+
- A-28: Winget Community Repository (microsoft/winget-pkgs)
686686
- HTTPS
687687

688688
* - DF-29: Consumer downloads MSI via winget
689-
- A-09: Winget Community Repository (microsoft/winget-pkgs)
689+
- A-28: Winget Community Repository (microsoft/winget-pkgs)
690690
- Consumer / End User
691691
- HTTPS
692692

@@ -728,6 +728,14 @@ Threats
728728
| **STRIDE:** T S
729729
| **Status:** Mitigate
730730
- C-021
731+
* - DFT-06
732+
- Command injection via unsanitised subprocess input
733+
- A-01: GitHub Repository (main / protected)
734+
- | **Sev:** 🟠H
735+
| **Risk:** 🟠H
736+
| **STRIDE:** T E
737+
| **Status:** Mitigate
738+
- C-015 (CodeQL) and C-017 (bandit) perform static analysis that detects command injection patterns before code reaches production.
731739
* - DFT-07
732740
- CI/CD secret exfiltration via supply-chain attack on build environment
733741
- A-08: Python Build (wheel / sdist)
@@ -856,9 +864,17 @@ Threats
856864
| **STRIDE:** T
857865
| **Status:** Mitigate
858866
- C-038
867+
* - DFT-34
868+
- Long-lived stored credential enables persistent unauthorised publication after exfiltration
869+
- A-29: WINGET_TOKEN PAT
870+
- | **Sev:** 🟠H
871+
| **Risk:** 🟠H
872+
| **STRIDE:** I T
873+
| **Status:** Mitigate
874+
- C-042
859875
* - DFT-35
860876
- Compromised publish credential enables malicious installer URL injection via package manifest submission
861-
- A-09: Winget Community Repository (microsoft/winget-pkgs)
877+
- A-28: Winget Community Repository (microsoft/winget-pkgs)
862878
- | **Sev:** 🟠H
863879
| **Risk:** 🟠H
864880
| **STRIDE:** T S

doc/explanation/threat_model_usage.rst

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -679,7 +679,7 @@ Asset Identification
679679
- Datastore
680680
- Critical / Critical / High
681681
* - A-15: SBOM Output (CycloneDX)
682-
- CycloneDX JSON/XML produced by ``dfetch report -t sbom``. Enumerates vendored components with PURL, license, and hash. Falsification hides actual dependencies from downstream CVE scanners. NOTE: this SBOM covers vendored deps only - dfetch itself has a separate machine-readable SBOM published on PyPI (see A-04 in tm_supply_chain.py).
682+
- CycloneDX JSON/XML produced by ``dfetch report -t sbom``. Enumerates vendored components with PURL, license, and hash. Falsification hides actual dependencies from downstream CVE scanners. NOTE: this SBOM covers vendored deps only - dfetch itself has a separate machine-readable SBOM published on PyPI (see A-03 in tm_supply_chain.py).
683683
- Datastore
684684
- High / High / —
685685
* - A-16: VCS Credentials
@@ -952,6 +952,14 @@ Threats
952952
| **STRIDE:** T S
953953
| **Status:** Mitigate
954954
- C-005 mitigates archive deps when hash present. Git/SVN: no integrity mechanism; pinning to an immutable commit SHA is recommended but not enforced by dfetch.
955+
* - DFT-06
956+
- Command injection via unsanitised subprocess input
957+
- A-22: dfetch Process
958+
- | **Sev:** 🟠H
959+
| **Risk:** 🟠H
960+
| **STRIDE:** T E
961+
| **Status:** Mitigate
962+
- C-006 suppresses interactive credential prompts for Git and SVN; C-007 invokes all external commands with ``shell=False`` and list-form arguments, eliminating the shell-injection vector for non-interactive VCS operations.
955963
* - DFT-07
956964
- CI/CD secret exfiltration via supply-chain attack on build environment
957965
- A-25: Patch Application (patch-ng)
@@ -1190,10 +1198,6 @@ Controls
11901198
- Manifest input validation
11911199
- DFT-04, DFT-08
11921200
- StrictYAML schema with ``SAFE_STR = Regex(r"^[^\x00-\x1F\x7F-\x9F]*$")`` rejects control characters in all string fields. ``dfetch/manifest/schema.py``
1193-
* - C-045
1194-
- Plaintext transport detection
1195-
- DFT-26
1196-
- ``plaintext_warning()`` (``dfetch/manifest/project.py``) inspects the resolved remote URL immediately before each VCS command is issued (inside the ``check_for_update`` and ``update`` spinners in ``subproject.py``). If the scheme is ``http://``, ``git://``, or ``svn://``, a visible warning is emitted naming the redacted URL (credentials stripped from the userinfo component) and recommending ``https://`` or ``svn+ssh://``. Detection only — dfetch still proceeds with the plaintext connection; the control raises user awareness but does not enforce scheme selection. ``dfetch/manifest/project.py, dfetch/project/subproject.py``
11971201
* - C-034
11981202
- Hash algorithm allowlist (SHA-256/384/512 only)
11991203
- DFT-30
@@ -1202,3 +1206,7 @@ Controls
12021206
- Persisted-metadata credential redaction
12031207
- DFT-13
12041208
- ``Metadata.dump()`` rebuilds the netloc of every persisted URL from ``parsed.hostname`` and ``parsed.port`` via ``urllib.parse.urlsplit`` / ``urlunsplit``, dropping any ``user:password@`` userinfo before writing ``.dfetch_data.yaml``. The same stripper is applied to each ``dependencies[].remote_url`` entry (git submodule, svn:external) so a credential in a nested upstream URL also never reaches disk. The in-memory ``Metadata`` object held by the running command keeps the original URL — only the on-disk representation is redacted, so an in-flight authenticated fetch is unaffected. ``dfetch/project/metadata.py``
1209+
* - C-045
1210+
- Plaintext transport detection
1211+
- DFT-26
1212+
- ``plaintext_warning()`` (``dfetch/manifest/project.py``) inspects the resolved remote URL immediately before each VCS command is issued (inside the ``check_for_update`` and ``update`` spinners in ``subproject.py``). If the scheme is ``http://``, ``git://``, or ``svn://``, a visible warning is emitted naming the redacted URL (credentials stripped from the userinfo component) and recommending ``https://`` or ``svn+ssh://``. Detection only — dfetch still proceeds with the plaintext connection; the control raises user awareness but does not enforce scheme selection. ``dfetch/manifest/project.py, dfetch/project/subproject.py``

security/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ python -m security.compliance \
3737

3838
This produces:
3939

40-
- `security/dfetch.component-definition.json` — OSCAL 1.1.2 Component Definition implementing
40+
- `security/dfetch.component-definition.json` — OSCAL 1.2.2 Component Definition implementing
4141
prEN 40000-1-4 Security Objectives for dfetch
4242
- `doc/explanation/compliance_track.rst` — human-readable RST (built into the Sphinx docs)
4343

security/dfetch.component-definition.json

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33
"uuid": "eddc2e8c-a2ae-5a42-8d35-88f5c2af55b2",
44
"metadata": {
55
"title": "dfetch CRA Compliance Component Definition",
6-
"last-modified": "2026-06-17T00:00:00Z",
7-
"version": "0.15.0",
6+
"last-modified": "2026-07-02T00:00:00Z",
7+
"version": "0.14.0",
88
"oscal-version": "1.2.2",
99
"document-ids": [
1010
{
@@ -82,7 +82,7 @@
8282
"props": [
8383
{
8484
"name": "software-version",
85-
"value": "0.15.0"
85+
"value": "0.14.0"
8686
},
8787
{
8888
"name": "asset-type",
@@ -1367,4 +1367,4 @@
13671367
]
13681368
}
13691369
}
1370-
}
1370+
}

security/tm_controls_data.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -301,7 +301,7 @@ def to_pytm(self) -> str:
301301
Control(
302302
id="C-041",
303303
name="Winget manifest PRs reviewed by community maintainers",
304-
assets=["A-09"],
304+
assets=["A-28"],
305305
threats=["DFT-35"],
306306
reference=".github/workflows/winget-publish.yml",
307307
description=(
@@ -319,7 +319,7 @@ def to_pytm(self) -> str:
319319
Control(
320320
id="C-042",
321321
name="WINGET_TOKEN scoped to dedicated Winget environment",
322-
assets=["A-10"],
322+
assets=["A-29"],
323323
threats=["DFT-34"],
324324
reference=".github/workflows/winget-publish.yml",
325325
description=(

security/tm_render.py

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import re
99
import sys
1010
from collections.abc import Callable
11+
from dataclasses import dataclass
1112
from typing import Any
1213

1314
from pytm import TM, Data, Element
@@ -120,6 +121,46 @@ def _render_asset_rows(
120121
return "".join(_asset_row(e, cia) for e in assets)
121122

122123

124+
@dataclass
125+
class _CatalogRow:
126+
"""Stand-in finding for a threat documented via ``ThreatResponse`` only.
127+
128+
Two situations leave a responded-to threat without any pytm finding:
129+
the mitigation is already modelled in the element properties, so the
130+
threat's condition no longer matches (e.g. DFT-06 with parameterized
131+
subprocess input), or the threat targets ``Data`` assets, which pytm's
132+
``resolve()`` never evaluates (it iterates ``TM._elements`` only, while
133+
``Data`` lives in ``TM._data`` — e.g. DFT-34). Rendering these rows
134+
from the threat catalog keeps the decision visible in the report instead
135+
of silently dropping it.
136+
"""
137+
138+
threat_id: str
139+
description: str
140+
severity: str
141+
target: str = "—"
142+
143+
144+
def _validate_threat_coverage(
145+
findings: Any,
146+
responses: list[ThreatResponse],
147+
catalog: dict[str, Any],
148+
) -> None:
149+
"""Fail generation when findings and responses have drifted apart.
150+
151+
Every pytm finding must have a documented ``ThreatResponse``, and every
152+
response must reference a threat that exists in the catalog. Raising here
153+
means an undocumented threat can never silently render as an empty row.
154+
"""
155+
resp_ids = {r.threat_id for r in responses}
156+
unknown = sorted(resp_ids - set(catalog))
157+
if unknown:
158+
raise ValueError(f"Responses reference unknown threat IDs: {unknown}")
159+
unhandled = sorted({f.threat_id for f in findings} - resp_ids)
160+
if unhandled:
161+
raise ValueError(f"Findings without a ThreatResponse: {unhandled}")
162+
163+
123164
def _render_threat_rows(
124165
tm: Any, controls: list[Control], responses: list[ThreatResponse]
125166
) -> str:
@@ -129,16 +170,31 @@ def _render_threat_rows(
129170
threat that applies to many elements produces many byte-identical rows.
130171
Collapse them to a single row documented against the threat's canonical
131172
target (``ThreatResponse.target`` when set, otherwise the matched element).
173+
174+
Responses whose threat produced no pytm finding are rendered from the
175+
threat catalog instead (see ``_CatalogRow``), so every documented risk
176+
decision appears in the table.
132177
"""
133178
findings = tm.findings
134-
if not findings:
179+
if not findings and not responses:
135180
return ""
136181

182+
catalog: dict[str, Any] = {t.id: t for t in getattr(TM, "_threats", [])}
183+
_validate_threat_coverage(findings, responses, catalog)
184+
137185
resp_map: dict[str, ThreatResponse] = {r.threat_id: r for r in responses}
138186

139187
representative: dict[str, Any] = {}
140188
for f in findings:
141189
representative.setdefault(f.threat_id, f)
190+
for r in responses:
191+
if r.threat_id not in representative:
192+
threat = catalog[r.threat_id]
193+
representative[r.threat_id] = _CatalogRow(
194+
threat_id=r.threat_id,
195+
description=str(threat.description),
196+
severity=str(threat.severity),
197+
)
142198

143199
def _row(f: Any) -> str:
144200
resp = resp_map.get(f.threat_id)

0 commit comments

Comments
 (0)