lacs-project / lacs-project/sysknife
The action-pin check reads uses: with a line grep, so a flow-style mapping is never checked
- Dominant language
- Rust
- Stars
- 12
- Forks
- 19
- Avg merge
- 18h 57m
- Merged PRs (30d)
- 116
Description
`tests/release/release-rehearsal.test.sh` states the strongest supply-chain invariant in this repo: every `uses:` in every workflow pins a full 40-hex SHA. It reads those lines with a line-oriented grep:
```
$ grep -n "grep -E '\^\[\[:space:\]\]\*(-\[\[:space:\]\]+)?uses:'" tests/release/release-rehearsal.test.sh
```
That pattern reads `uses:` at the start of a line, with an optional `- ` in front. YAML also allows a flow mapping, and GitHub Actions accepts it:
```yaml
- { uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 }
```
The grep does not match `- { uses:`, so an action written that way is never handed to the SHA assertion.
#432 is open against this file and adds a floor: the extraction has to produce at least twenty lines or the run fails. That closes the case the extraction breaks wholesale. It does not close this one, because fifty-five real lines clear a floor of twenty however many flow-style entries sit beside them.
## Measured
Against `31b9f2d6`, which is #432's head with the floor in place. One flow-style, unpinned entry added to `docs.yml` next to the fifty-five ordinary ones:
```
$ grep -n 'attacker/exfil' .github/workflows/docs.yml
39: - { uses: attacker/exfil@main }
$ echo "uses: lines the extraction still sees: $(cat .github/workflows/*.yml | grep -cE '^[[:space:]]*(-[[:space:]]+)?uses:')"
uses: lines the extraction still sees: 55
$ podman run --rm --network=none -v "$PWD:/repo:z" -v "$HOME/.cargo:/cargo:O" -v "$HOME/.rustup:/rustup:O" -w /repo -e HOME=/tmp -e CARGO_HOME=/cargo -e RUSTUP_HOME=/rustup -e CARGO_NET_OFFLINE=true -e PATH=/cargo/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin localhost/sk-reh2:1 bash -c 'bash tests/release/release-rehearsal.test.sh; echo "rc=$?"'
Release rehearsal contract passed.
rc=0
```
The same mutation against `main`'s copy of the test gives the same result, so this predates #432 and #432 neither creates it nor widens it.
## Why it matters
Every other pin in this tree is checked. This one line is the difference between "every action is pinned, and a gate says so" and "every action a grep happened to notice is pinned". A workflow diff still gets a human read before it merges, which is the control that actually stands between a fork and the token, so this is a gate with a hole rather than a live hole. A gate with a hole is worth closing on its own terms: the whole reason it exists is so the human read is not the only thing.
## The fix
Parse the YAML instead of grepping it. `docs-and-hygiene` already sets up Python 3.11 and #377 is landing a PyYAML-based workflow reader for a neighbouring check, so the dependency and the pattern are both about to exist here:
```python
for job in doc.get("jobs", {}).values():
for step in job.get("steps", []) or []:
if "uses" in step:
...
```
That reads block and flow mappings identically, because by the time PyYAML is done there is no difference between them. Keep #432's floor: a parser that returns an empty list for a file it could not read has the same failure mode the floor was written for.
Two things to hold on to while replacing the grep:
- The `./...` exemption. A reusable workflow in this repository is referenced by path and resolves at the caller's commit, so it cannot carry a SHA. The exemption must stay anchored to `./` so a third-party `owner/repo/.github/workflows/x.yml@ref` is still required to pin.
- A negative fixture per spelling. #432 added one for the flow mapping; the replacement should keep it and watch it turn green for the right reason, which means asserting the parser finds that `uses:` rather than asserting the run fails.
Blocked on #432 landing, so the floor and the fixture are in the file first.
Found while reviewing #432.
Contributor guide
Research direction
Start in tests/release/release-rehearsal.test.sh, then read the PyYAML workflow-reader setup from #377 and the extraction floor and flow-mapping fixture from #432. Run the release rehearsal with block- and flow-style workflow entries; done means both are parsed, SHA pinning and the ./ exemption remain enforced, and the minimum extraction floor still fails unreadable or empty results.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- bash, github-actions, python, yaml
- Domain
- ci-cd, security, testing
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 65/100