SECURITY.md and the assurance case say CI never runs check_no_secrets.sh, but it does #653

Activity

1. vladimirrott

   added

   enhancementNew feature or requestNew feature or request

   help wantedExtra attention is neededExtra attention is needed

   easyDifficulty: self-contained, no deep context neededDifficulty: self-contained, no deep context needed

   on Oct 24, 2026
2. vladimirrott commented on Oct 24, 2026

   

   vladimirrott

   on Oct 24, 2026

   MemberAuthor

   More actions

   @mikevillari your #446 parsed workflow action references as YAML instead of grepping them. This one needs the same kind of contract test: assert that docs-and-hygiene runs check_no_secrets.sh, watch it fail, then add the step.

   I'm holding it for you. Say so here and I'll assign it. If it isn't for you, no reply needed.
3. vladimirrott commented on Oct 26, 2026

   

   vladimirrott

   on Oct 26, 2026

   MemberAuthor

   More actions

   I got the premise of this issue wrong. CI already runs check_no_secrets.sh over the whole tracked tree, one level down from where I looked:

   $ grep -n "no-secrets" .github/workflows/ci.yml
   119:        run: bash tests/release/no-secrets.test.sh
   $ sed -n '256p;276p' tests/release/no-secrets.test.sh
   # --- 3. The whole tracked tree must be clean --------------------------------
       elif ! scan_out="$("$CHECK" "${tracked[@]}" 2>&1)"; then
   $ grep -n '^CHECK=' tests/release/no-secrets.test.sh
   21:CHECK="$ROOT/scripts/check_no_secrets.sh"
   303:CHECK="$mutant"
   309:CHECK="$real_check"
   $ git log --format='%h %ad %s' --date=short -S'The whole tracked tree must be clean' -- tests/release/no-secrets.test.sh
   bf8d412 2026-08-08 build: refuse to commit a provider credential (#188)
   

   Line 119 sits in the docs-and-hygiene job, and section 3 of the test (with CHECK at its line-21 value; 303 and 309 belong to the mutation self-test) passes every git ls-files path to the scanner. grep -rn check_no_secrets .github/ finds nothing because the workflow calls the test, and the test calls the script. A fork PR or a --no-verify commit carrying a key the scanner recognises therefore fails CI today.

   The step this issue asks for would scan the tree a second time. Please don't start on it as written. I'm leaving it open for now rather than closing it in the same breath as admitting the mistake.
4. vladimirrott commented on Nov 3, 2026

   

   vladimirrott

   on Nov 3, 2026

   MemberAuthor

   More actions

   I repeated my own mistake from the comment above in #734, three days later. Two new lines say CI does not run the scanner, and both link here as the fix:

   $ grep -n 'check_no_secrets\|653' SECURITY.md docs/assurance-case.md
   SECURITY.md:82:opt-in pre-commit hook runs `scripts/check_no_secrets.sh` before a commit
   SECURITY.md:84:[#653](https://github.com/lacs-project/sysknife/issues/653). The project's credentials
   docs/assurance-case.md:205:| CWE-798 | Use of hard-coded credentials | trufflehog on every pull request and push; GitHub push protection; `scripts/check_no_secrets.sh` in the opt-in pre-commit hook ([#653](https://github.com/lacs-project/sysknife/issues/653) moves it into CI) | `tests/release/no-secrets.test.sh` |
   $ grep -n "no-secrets" .github/workflows/ci.yml
   135:        run: bash tests/release/no-secrets.test.sh
   

   Line 135 is still inside docs-and-hygiene (the job starts at line 19), and section 3 of tests/release/no-secrets.test.sh (line 256 on) still passes every tracked path to $CHECK, which line 21 sets to scripts/check_no_secrets.sh.

   That settles what this issue should be. The work left is a docs fix, and I'm narrowing the scope to it:

   • SECURITY.md:80-84: say that CI runs scripts/check_no_secrets.sh over every tracked file through tests/release/no-secrets.test.sh in the docs-and-hygiene job, and drop the link to this issue.
   • docs/assurance-case.md:205: the same correction in the CWE-798 row, keeping trufflehog and push protection.

   Leave the CI workflow alone; adding a second scan step is out of scope. To check your change, run grep -n '653' SECURITY.md docs/assurance-case.md (no output once both links are gone) and bash tests/release/no-secrets.test.sh (it still passes).
5. vladimirrott commented on Nov 3, 2026

   

   vladimirrott

   on Nov 3, 2026

   MemberAuthor

   More actions

   @mikevillari I offered you this on October 24 and haven't heard back, which is fine. I'm opening it to other contributors now so it isn't held for you while you're busy elsewhere. If you want it later, say so here and it's yours again. Thanks again for #446.
6. vladimirrott

   changed the title [-]check_no_secrets.sh never runs in CI, only in the opt-in pre-commit hook[/-] [+]SECURITY.md and the assurance case say CI never runs check_no_secrets.sh, but it does[/+] on Nov 3, 2026
7. vladimirrott

   added

   good first issueGood for newcomersGood for newcomers

   documentationImprovements or additions to documentationImprovements or additions to documentation

   twir-listedListed in a This Week in Rust CfP. Do not offer to anyone until that issue publishes.Listed in a This Week in Rust CfP. Do not offer to anyone until that issue publishes.

   and removed

   enhancementNew feature or requestNew feature or request

   on Nov 3, 2026