Skip to content

Commit a9d4a1c

Browse files
spoorccclaude
andauthored
fix: SHA-pin slsa_with_provenance action; explicit shell=False; fix C-009/C-024 doc errors (#1278)
* fix: SHA-pin slsa_with_provenance action; explicit shell=False; fix C-009/C-024 doc errors - Pin slsa-framework/source-actions/slsa_with_provenance to commit SHA dea965cdca5e0cb422bf7b2653c9d15f678ad01c (was tag v0.1.0, violating the C-009 commit-SHA pinning control for the most security-sensitive workflow) - Add explicit shell=False to subprocess.run() in cmdline.py; the default is False but the code did not state it, undermining the C-007 claim - Rename duplicate C-009 "Plaintext transport detection" (usage model) to C-045 to eliminate the control-ID conflict with C-009 "Actions commit-SHA pinning" (supply-chain model); update all references in compliance_data.py, compliance_track.rst, threat_model_usage.rst, control_register.rst - Rename C-024 from "secrets: inherit scope" to "Explicit secret forwarding" — the implementation deliberately avoids secrets: inherit and uses selective named forwarding; the old name was the inverse of the actual behaviour - Fix DFT-15 (VCS externals) accept rationale: the "Manifest under code review" assumption does not cover nested submodule/external URLs that come from the upstream repo, not dfetch.yaml; rebase on dfetch scope boundary assumption with explicit residual-risk statement - Add Risk Rating Methodology section to security.rst documenting the Sev/Risk scale (L/M/H/VH/C) aligned with BSI TR-03183-1 §5.3; add cross-reference from both threat model pages and update report_template - Clarify ECR-C SO.Updateability and ECR-M SO.SecureDataDeletion: add explanatory notes and gaps column text so "✓ Implemented" rows with no control listed are not misleading to conformity assessors - Update C-009 description from absolute "Every third-party Action" to "All third-party Actions" (accurate after the SHA-pin fix above) https://claude.ai/code/session_01CtU2HEmkrr4gBHvZMRT3U5 * fix: restore nosec B603 on subprocess.run with explicit scope B603 is bandit's reminder-check for subprocess calls regardless of shell=False; the suppression acknowledges that cmd is list-form args constructed entirely from internal code, not from untrusted user input. Scoped to B603 only (was unscoped # nosec before). https://claude.ai/code/session_01CtU2HEmkrr4gBHvZMRT3U5 * fix: add RST section label for Risk Rating Methodology; fix stale C-009 in DFT-26 - Add .. _risk-rating-methodology: label above the section heading in security.rst so :ref: cross-references resolve to the section itself rather than the page-level anchor - Update all three reference sites to use <risk-rating-methodology>: security/report_template.rst, threat_model_supply_chain.rst, threat_model_usage.rst - Replace stale C-009 with C-045 in the DFT-26 note in tm_usage.py (C-009 is commit-SHA pinning; C-045 is plaintext transport detection) https://claude.ai/code/session_01CtU2HEmkrr4gBHvZMRT3U5 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent b721b05 commit a9d4a1c

11 files changed

Lines changed: 158 additions & 46 deletions

.github/workflows/source-provenance.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ jobs:
4141
persist-credentials: true
4242

4343
- name: Attest source governance (SLSA Source Track)
44-
uses: slsa-framework/source-actions/slsa_with_provenance@v0.1.0
44+
uses: slsa-framework/source-actions/slsa_with_provenance@dea965cdca5e0cb422bf7b2653c9d15f678ad01c # v0.1.0
4545
with:
4646
version: v0.6.3
4747

dfetch/util/cmdline.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,8 @@ def run_on_cmdline(
4646
logger.debug(f"Running {cmd}")
4747

4848
try:
49-
proc = subprocess.run( # nosec
50-
cmd, env=env, input=input_data, capture_output=True, check=True
49+
proc = subprocess.run( # nosec B603 — shell=False, list-form args from internal code
50+
cmd, shell=False, env=env, input=input_data, capture_output=True, check=True
5151
)
5252
except subprocess.CalledProcessError as exc:
5353
raise SubprocessCommandError(

doc/explanation/compliance_track.rst

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ The table below summarises dfetch's implementation of each prEN 40000-1-4 Securi
141141
* - **ECR-C** — Ensure that vulnerabilities can be addressed through security updates, including automatic updates enabled by default, with an opt-out mechanism, user notification, and the option to postpone updates.
142142
- SO.Updateability
143143
- —
144-
-
144+
- Satisfied by pip distribution (``pip install --upgrade dfetch``); no dfetch-specific control required — see note below table.
145145
- ✓ Implemented
146146
* -
147147
- SO.AutomaticUpdates
@@ -165,7 +165,7 @@ The table below summarises dfetch's implementation of each prEN 40000-1-4 Securi
165165
- ⚠ Partial
166166
* -
167167
- SO.AccessControlReport
168-
- :ref:`C-009 <c-009>`
168+
- :ref:`C-045 <c-045>`
169169
- No persistent log of unauthorised access attempts
170170
- ⚠ Partial
171171
* - **ECR-E** — Protect the confidentiality of stored, transmitted or otherwise processed data by state-of-the-art mechanisms such as encryption at rest and in transit.
@@ -180,12 +180,12 @@ The table below summarises dfetch's implementation of each prEN 40000-1-4 Securi
180180
- ✓ Implemented
181181
* -
182182
- SO.DataTransmittedConfidentiality
183-
- :ref:`C-005 <c-005>`, :ref:`C-009 <c-009>`
183+
- :ref:`C-005 <c-005>`, :ref:`C-045 <c-045>`
184184
- —
185185
- ✓ Implemented
186186
* -
187187
- SO.ComAuth
188-
- :ref:`C-003 <c-003>`, :ref:`C-004 <c-004>`, :ref:`C-009 <c-009>`
188+
- :ref:`C-003 <c-003>`, :ref:`C-004 <c-004>`, :ref:`C-045 <c-045>`
189189
- —
190190
- ✓ Implemented
191191
* -
@@ -210,7 +210,7 @@ The table below summarises dfetch's implementation of each prEN 40000-1-4 Securi
210210
- ⚠ Partial
211211
* -
212212
- SO.IntegrityReport
213-
- :ref:`C-009 <c-009>`
213+
- :ref:`C-045 <c-045>`
214214
- No persistent integrity-violation log
215215
- ⚠ Partial
216216
* - **ECR-G** — Process only data, personal or other, that are adequate, relevant and limited to what is necessary in relation to the intended purpose of the product with digital elements (data minimisation).
@@ -260,7 +260,7 @@ The table below summarises dfetch's implementation of each prEN 40000-1-4 Securi
260260
- ⚠ Partial
261261
* -
262262
- SO.MonitorSecurityRelevantActivities
263-
- :ref:`C-009 <c-009>`
263+
- :ref:`C-045 <c-045>`
264264
- —
265265
- ⚠ Partial
266266
* -
@@ -276,7 +276,7 @@ The table below summarises dfetch's implementation of each prEN 40000-1-4 Securi
276276
* - **ECR-M** — Provide the possibility for users to securely and easily remove on a permanent basis all data and settings and, where such data can be transferred to other products or systems, ensure that this is done in a secure manner.
277277
- SO.SecureDataDeletion
278278
- —
279-
-
279+
- No dfetch-specific control required — satisfied by design (no personal data stored); standard OS file deletion suffices — see note below table.
280280
- ✓ Implemented
281281
* -
282282
- SO.DataTransmittedConfidentiality
@@ -294,6 +294,21 @@ The table below summarises dfetch's implementation of each prEN 40000-1-4 Securi
294294
- —
295295
- — N/A
296296

297+
.. rubric:: Notes on "Implemented" rows with no control listed
298+
299+
**ECR-C SO.Updateability** — No dfetch-specific control is needed. SUM-1/SUM-2
300+
are satisfied by the PyPI distribution mechanism (``pip install --upgrade dfetch``
301+
and GitHub Releases binary packages). pip's TLS-protected download channel provides
302+
the required secure update path. The update mechanism is the responsibility of the
303+
user's package manager, not of dfetch itself.
304+
305+
**ECR-M SO.SecureDataDeletion** — No dfetch-specific control is needed. DLM-1 is
306+
satisfied by design: dfetch stores no personal data, credentials, or cryptographic
307+
keying material on disk. The only on-disk state is ``.dfetch_data.yaml`` (non-sensitive
308+
dependency metadata — credentials stripped by :ref:`C-036 <c-036>`) and vendored
309+
source files (third-party code). Standard OS file deletion (``rm`` / ``del``) is
310+
sufficient to remove all dfetch data; no secure-wipe facility is warranted.
311+
297312
----
298313

299314
Part II — Vulnerability Handling (prEN 40000-1-3)

doc/explanation/control_register.rst

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,9 +127,9 @@ requirements not independently surfaced by the risk analysis.
127127
* - .. _c-024:
128128

129129
C-024
130-
- ``secrets: inherit`` scope
130+
- Explicit secret forwarding
131131
- Risk-driven
132-
-
132+
- `.github/workflows/ci.yml <https://github.com/dfetch-org/dfetch/blob/main/.github/workflows/ci.yml>`_
133133
* - .. _c-026:
134134

135135
C-026
@@ -208,6 +208,13 @@ requirements not independently surfaced by the risk analysis.
208208
- Data minimisation policy
209209
- Compliance-only
210210
- :doc:`compliance_track`
211+
* - .. _c-045:
212+
213+
C-045
214+
- Plaintext transport detection
215+
- Risk-driven
216+
- `dfetch/manifest/project.py <https://github.com/dfetch-org/dfetch/blob/main/dfetch/manifest/project.py>`_,
217+
`dfetch/project/subproject.py <https://github.com/dfetch-org/dfetch/blob/main/dfetch/project/subproject.py>`_
211218
* - .. _c-046:
212219

213220
C-046

doc/explanation/security.rst

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,61 @@ For an overview of how this documentation set is produced — the threat-model
108108
pipeline, compliance pipeline, release attestations, and the full artifact
109109
inventory — see :doc:`security_pipeline`.
110110

111+
.. _risk-rating-methodology:
112+
113+
Risk Rating Methodology
114+
-----------------------
115+
116+
Both threat models use a qualitative two-axis risk framework aligned with
117+
`BSI TR-03183-1 §5.3 <https://www.bsi.bund.de/SharedDocs/Downloads/EN/BSI/Publications/TechGuidelines/TR03183/BSI-TR-03183-1.pdf>`_
118+
(Cyber Resilience Requirements for Manufacturers, Chapter 5).
119+
120+
Each threat entry carries two independent ratings:
121+
122+
**Severity (Sev)** — the maximum potential impact if the threat is fully
123+
realised, assessed without regard to likelihood or existing controls.
124+
Determined from the Confidentiality / Integrity / Availability ratings of
125+
the targeted asset and the scope of harm to downstream consumers.
126+
127+
**Risk** — the *residual* risk after factoring in the likelihood of
128+
successful exploitation (attacker capability, required access, known
129+
exploitation evidence) and any controls already partially in place.
130+
Risk can therefore be lower than Severity when the attack path is
131+
constrained, and must be reassessed whenever the control set changes.
132+
133+
.. list-table::
134+
:header-rows: 1
135+
:widths: 12 20 68
136+
137+
* - Rating
138+
- Label
139+
- Guidance (BSI TR-03183-1 §5.3)
140+
141+
* - 🟢L
142+
- Low
143+
- Negligible impact or extremely unlikely. No mandatory control action; document and monitor.
144+
145+
* - 🟡M
146+
- Medium
147+
- Moderate impact or realistic but non-trivial exploit path. Controls advisable; track in backlog.
148+
149+
* - 🟠H
150+
- High
151+
- Significant impact or credible attack path with meaningful probability. Controls required before next release.
152+
153+
* - 🔴VH
154+
- Very High
155+
- Severe impact (system compromise, data exfiltration) or well-documented exploit path. Priority controls required.
156+
157+
* - 🔴C
158+
- Critical
159+
- Catastrophic, near-certain potential (e.g. unencrypted channel with no compensating control). Immediate mitigation required; accept decision requires explicit sign-off.
160+
161+
The risk treatment decisions (Mitigate / Accept / Transfer) in each threat
162+
table follow the same vocabulary as ISO/IEC 27005 and BSI TR-03183-1: an
163+
**Accept** decision requires explicit rationale citing the assumption under
164+
which the residual risk is acceptable, documented alongside the threat entry.
165+
111166
Threat Models
112167
-------------
113168

doc/explanation/threat_model_supply_chain.rst

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ Risk Context
1717
This report follows the risk-based approach of `BSI TR-03183-1
1818
<https://www.bsi.bund.de/SharedDocs/Downloads/EN/BSI/Publications/TechGuidelines/TR03183/BSI-TR-03183-1.pdf>`_
1919
Chapter 5.
20+
The Sev / Risk rating scale and treatment vocabulary (Mitigate / Accept / Transfer)
21+
are defined in the :ref:`Risk Rating Methodology <risk-rating-methodology>` section of the main security page.
2022

2123
Threat model for dfetch. Covers the pre-install lifecycle: code contribution, CI/CD, build (wheel / sdist), PyPI distribution, Winget manifest submission, and consumer installation. The installed dfetch package is the handoff point to tm_usage.py.
2224

@@ -879,7 +881,7 @@ Controls
879881
* - C-009
880882
- Actions commit-SHA pinning
881883
- DFT-07
882-
- Every third-party GitHub Action is pinned to a full commit SHA, preventing tag-mutable supply-chain substitution. ``.github/workflows/*.yml``
884+
- All third-party GitHub Actions are pinned to a full commit SHA (``@<40-hex>``), preventing tag-mutable supply-chain substitution. ``.github/workflows/*.yml``
883885
* - C-010
884886
- OIDC trusted publishing
885887
- DFT-07
@@ -917,9 +919,9 @@ Controls
917919
- DFT-02
918920
- A CycloneDX SBOM is generated during the build and published alongside the PyPI release, satisfying CRA Article 13 requirements.
919921
* - C-024
920-
- ``secrets: inherit`` scope
922+
- Explicit secret forwarding
921923
- DFT-07
922-
- ``ci.yml`` only passes required repository secrets to the test and docs workflows, preventing malicious PR steps from exfiltrating unrelated secrets.
924+
- ``ci.yml`` explicitly names only the secrets each child workflow requires (``CODACY_PROJECT_TOKEN`` → ``test.yml``; ``GH_DFETCH_ORG_DEPLOY`` → ``docs.yml``); all other repository secrets are not forwarded. ``secrets: inherit`` is deliberately not used so that malicious PR steps cannot exfiltrate unrelated secrets. ``.github/workflows/ci.yml``
923925
* - C-026
924926
- Consumer-side package provenance verification
925927
- DFT-17, DFT-25

doc/explanation/threat_model_usage.rst

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ Risk Context
1717
This report follows the risk-based approach of `BSI TR-03183-1
1818
<https://www.bsi.bund.de/SharedDocs/Downloads/EN/BSI/Publications/TechGuidelines/TR03183/BSI-TR-03183-1.pdf>`_
1919
Chapter 5.
20+
The Sev / Risk rating scale and treatment vocabulary (Mitigate / Accept / Transfer)
21+
are defined in the :ref:`Risk Rating Methodology <risk-rating-methodology>` section of the main security page.
2022

2123
Threat model for dfetch. Covers the post-install lifecycle: reading the manifest, fetching dependencies from VCS and archive sources, applying patches, writing vendored files, and generating reports (SBOM, SARIF, check output). The installed dfetch package - produced by the supply chain in tm_supply_chain.py - is the entry point.
2224

@@ -1013,7 +1015,7 @@ Threats
10131015
| **Risk:** 🟠H
10141016
| **STRIDE:** T
10151017
| **Status:** Accept
1016-
- Git submodules are followed: ``git submodule update --init --recursive`` is called unconditionally during every Git fetch (``dfetch/vcs/git.py``), and each submodule is recorded as a ``Dependency`` with ``source_type='git-submodule'`` (``gitsubproject.py``). SVN ``export`` is invoked without ``--ignore-externals``; each ``svn:externals`` entry triggers an additional fetch, and ``SvnSubProject._fetch_externals()`` records it as a ``Dependency`` with ``source_type='svn-external'`` (``svnsubproject.py``). Both behaviours are intentional — dfetch vendors submodule and external trees and surfaces them in metadata — but the fetched URLs come from the upstream repository (``.gitmodules`` / ``svn:externals``), not from ``dfetch.yaml``, and therefore bypass manifest code review and carry no integrity hash. Suppressing these fetches (e.g. passing ``--no-recurse-submodules`` or ``--ignore-externals``) would be a design change that removes intentional vendoring behaviour. Accepted based on the **Manifest under code review** assumption: the choice to vendor an upstream that contains submodules or svn:externals is declared in ``dfetch.yaml`` and subject to code review; the decision to trust those nested URLs is made at the manifest-review boundary.
1018+
- Git submodules are followed: ``git submodule update --init --recursive`` is called unconditionally during every Git fetch (``dfetch/vcs/git.py``), and each submodule is recorded as a ``Dependency`` with ``source_type='git-submodule'`` (``gitsubproject.py``). SVN ``export`` is invoked without ``--ignore-externals``; each ``svn:externals`` entry triggers an additional fetch, and ``SvnSubProject._fetch_externals()`` records it as a ``Dependency`` with ``source_type='svn-external'`` (``svnsubproject.py``). Both behaviours are intentional — dfetch vendors submodule and external trees and surfaces them in metadata — but the fetched URLs come from the upstream repository (``.gitmodules`` / ``svn:externals``), not from ``dfetch.yaml``, and therefore bypass manifest code review and carry no integrity hash. Suppressing these fetches (e.g. passing ``--no-recurse-submodules`` or ``--ignore-externals``) would be a design change that removes intentional vendoring behaviour. The initial decision to vendor a given upstream repository (which may contain submodules or SVN externals) is declared in ``dfetch.yaml`` and subject to code review. However, the specific nested URLs in ``.gitmodules`` or ``svn:externals`` are not visible in ``dfetch.yaml`` and are not independently reviewed; if an upstream maintainer adds a new submodule after the initial review, dfetch will fetch it on the next ``dfetch update`` without a new ``dfetch.yaml`` change triggering review. Residual risk: a compromised upstream maintainer could inject a malicious submodule URL that bypasses the manifest review boundary. Accepted based on the **dfetch scope boundary** assumption: the security of fetched third-party source code and its nested dependencies is the responsibility of the manifest author who selects and pins each upstream; ``dfetch check`` version-drift notifications prompt review before any upstream change (including new submodules) is vendored.
10171019
* - DFT-16
10181020
- Configured destination path allows writes to security-sensitive project directories
10191021
- A-22: dfetch Process
@@ -1101,7 +1103,7 @@ Threats
11011103
| **Risk:** 🟠H
11021104
| **STRIDE:** T I
11031105
| **Status:** Mitigate
1104-
- C-009 emits a visible warning immediately before the VCS command when a plaintext scheme (``http://``, ``git://``, ``svn://``) is detected, with credentials redacted and ``https://`` / ``svn+ssh://`` recommended. Detection only — dfetch does not reject or upgrade plaintext URLs; scheme selection remains the manifest author's responsibility.
1106+
- C-045 emits a visible warning immediately before the VCS command when a plaintext scheme (``http://``, ``git://``, ``svn://``) is detected, with credentials redacted and ``https://`` / ``svn+ssh://`` recommended. Detection only — dfetch does not reject or upgrade plaintext URLs; scheme selection remains the manifest author's responsibility.
11051107
* - DFT-28
11061108
- CI/CD build cache poisoned to silently substitute a malicious compiled artifact
11071109
- A-20: Local VCS Cache (temp)
@@ -1188,7 +1190,7 @@ Controls
11881190
- Manifest input validation
11891191
- DFT-04, DFT-08
11901192
- StrictYAML schema with ``SAFE_STR = Regex(r"^[^\x00-\x1F\x7F-\x9F]*$")`` rejects control characters in all string fields. ``dfetch/manifest/schema.py``
1191-
* - C-009
1193+
* - C-045
11921194
- Plaintext transport detection
11931195
- DFT-26
11941196
- ``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``

0 commit comments

Comments
 (0)