Update BR-01 to split CI/CD security into 3 areas#443
Update BR-01 to split CI/CD security into 3 areas#443evankanderson wants to merge 6 commits intoossf:mainfrom
Conversation
funnelfiasco
left a comment
There was a problem hiding this comment.
This seems significant enough that I'm going to leave a fake nack to block merging in order to make sure there's enough time for discussion.
|
I'm fine adding more requirement statements, as Evan requests here. I ask that changes like this get merged into the crosswalk spreadsheet as we implement them, as some stakeholders use that as a prime source. I think we have the yaml --> website covered through our automation. We want to ensure all paths into the catalog are consistent for the user. |
You're talking about filling in the Scorecard -> Dangerous Workflows mapping for BR-01? And you want me to update docs/Compliance%20Crosswalk%20Matrix-17Nov2025.xlsx, or something else (possibly not in source control)? |
| CI/CD pipelines which accept collaborator input MUST sanitize and | ||
| validate that input prior to use in the pipeline. |
There was a problem hiding this comment.
This reads quite similar to the text removed above "When a CI/CD pipeline accepts an input parameter, that parameter MUST be sanitized and validated prior to use in the pipeline."
There was a problem hiding this comment.
This explicitly targets trusted-collaborator inputs to CI workflows, e.g. branch names in an explicitly-run GitHub workflow.
For that reason, it is Maturity Level 3 (protect project CI from insiders) compared with maturity level 1 (prevent drive-by pwning) of the other two requirements.
No, our Compliance Crosswalk(1) - after each update of the yaml files I've been trying to keep it current. Beyond the yaml files, if we had some way to get this file into git and still allow edits, that would be a dream. As it goes today after I update the xls, i output a pdf and store in our osps repo. |
|
@evankanderson can you please address Eddie's feedback? |
Addressed, I think. |
e62baf2 to
e22a781
Compare
Signed-off-by: Evan Anderson <evan.k.anderson@gmail.com>
Signed-off-by: Evan Anderson <evan.k.anderson@gmail.com>
…rusted. Signed-off-by: Evan Anderson <evan.k.anderson@gmail.com>
* Add UKSSCOP reference-ids and claims to OSPS-DO.yaml Dependent upon merge of ossf#426 DO mappings to UKSSCOP framework Signed-off-by: CRob <69357996+SecurityCRob@users.noreply.github.com> * Update baseline/OSPS-DO.yaml Co-authored-by: Ben Cotton <ben@kusari.dev> Signed-off-by: Travis Truman <trumant@gmail.com> --------- Signed-off-by: CRob <69357996+SecurityCRob@users.noreply.github.com> Signed-off-by: Travis Truman <trumant@gmail.com> Co-authored-by: Travis Truman <trumant@gmail.com> Co-authored-by: Ben Cotton <ben@kusari.dev> Signed-off-by: Evan Anderson <evan.k.anderson@gmail.com>
Signed-off-by: Evan Anderson <evan.k.anderson@gmail.com>
e22a781 to
1526f46
Compare
… subdivide-ci;validation Signed-off-by: Evan Anderson <evan.k.anderson@gmail.com>
| values) all collaborator inputs on explicit workflow executions. | ||
| While collaborators are generally trusted, manual inputs to a | ||
| workflow cannot be reviewed and could be abused by an account | ||
| takeover. |
There was a problem hiding this comment.
| takeover. | |
| takeover or insider threat. |
| - Maturity Level 2 | ||
| - Maturity Level 3 | ||
| recommendation: # TODO | ||
| - id: OSPS-BR-01.02 |
There was a problem hiding this comment.
We need to mark this as removed or retired, remove the applicability items, and change the text to say Retired in https://github.com/ossf/security-baseline/pull/443.
| recommendation: | | ||
| CI/CD pipelines should sanitize (quote, escape or exit on expected | ||
| values) all metadata inputs which correspond to untrusted sources. | ||
| This includes data such as commit messages, tags, pull request titles, |
There was a problem hiding this comment.
| This includes data such as commit messages, tags, pull request titles, | |
| This includes data such as branch names, commit messages, tags, pull request titles, |
|
@evankanderson I think I'm good with shipping this once you apply the suggestions you noted a couple of weeks ago. |
As discussed in the 2025-11-25 meeting and on Slack.
The BR-01 controls was originally lifted from the Scorecard
Dangerous-Workflowcheck. When this control was refactored into assessment criteria, we ended up with some ambiguity and possible overlap:workflow_runtrigger, which supports user-selected explicit values. It could also be read to cover input metadata (e.g. PR title), but it's not clear.I unified the current assessments into BR-01.01, which covers all untrusted metadata executed without contributor review.
Both of these missed the "Untrusted Code Checkout" check from Dangerous-Workflow, which I've revived as BR-01.03 (to avoid re-using BR-01.02 with a different meaning).
I revised the plain meaning of BR-01.01 to BR-01.04 as a level 3 control for projects with higher levels of assurances.