Repository navigation
test: assert every action is hash-pinned and no checkout persists credentials - #41
Merged
Merged
Conversation
…dentials docs/security.md promised that the checkout step is pinned by commit hash and does not persist credentials, but nothing enforced either. test_workflows.py now reads every `uses:` line of both workflows and requires `owner/repo@<40 hex>`, optionally followed by a `# vX.Y.Z` comment, and reads every step under a `steps:` list (indentation kept, `name:`-first and quoted forms included) to require `persist-credentials: false` in the `with:` block of each `actions/checkout` step. Two counts keep the readers honest: every spelling of a `uses` key YAML accepts is counted against the lines the strict pattern read, and the steps that carry an action against the `uses:` lines, so a step the readers did not see fails the test instead of slipping past it. Seen failing on tests.yml with a tag instead of a hash, a checkout without a `with:` block, `persist-credentials: true`, and a short hash with a bare comment; a `name:`-first, quoted, pinned checkout with the flag passes. The same shapes, with a two-job sample, are kept as a fixture test so the readers cannot loosen unnoticed. docs/security.md says the test enforces the promise; docs/testing.md counts 142 Python tests. Closes #34
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #34
What changed
tests/python/test_workflows.pytest_every_action_is_pinned_to_a_commit: everyuses:in both workflows isowner/repo@<40 hex>, optionally followed by a# vX.Y.Zcomment. Every spelling of auseskey YAML accepts (quoted key, space before the colon, flow-style step) is counted against the lines the strict pattern read, so a spelling it cannot read fails rather than passing unseen.test_every_checkout_step_leaves_no_credentials_behind: a step reader (steps(), indentation kept, soname:-first and quoted forms count) finds everyactions/checkoutstep, case-insensitively, and requirespersist-credentials: falseinside itswith:block. The steps that carry an action are counted against theuses:lines of the file.test_the_pin_and_step_readers_on_the_shapes_they_must_tell_apart: the shapes verified by hand (below) as an inline two-job fixture, so the readers cannot loosen unnoticed.docs/security.md: the promise now says every action is pinned and that the test enforces it.docs/testing.md: the Python worker count is 154 (was 151 after the warm worker pool merge) and lists action pins and checkout credentials.Tests that ran
tests.yml, each mutation alone and reverted: a tag instead of a hash (actions/checkout@v7.0.1) fails the pin test; a checkout without itswith:block fails the credentials test ('persist-credentials: false' not found in []);persist-credentials: truefails it too; a short hash with a bare# v7comment fails the pin test. Aname:-first checkout with a quoted pinned value and the flag inside a longerwith:block passes both new tests.python -m unittest tests.python.test_workflows: 10 tests OK on the host.docker compose -f docker/compose.yaml run --rm --no-deps python-tests: 142 tests OK on 8cd6cb8, and 154 tests OK on the merge commit f1ee066 aftermainwas merged in for a conflict on the count line in docs/testing.md (the warm worker pool PR changed it). The PHP, integration and MCP layers run in CI on this PR; only test code and docs changed.Independent review
delivery-reviewer on the first draft of this commit: nothing blocking. Folded into the one commit before pushing: the checkout filter is now case-insensitive (GitHub reads owner and repository names that way); every spelling of a
useskey is counted against the strict pattern (flow-style, quoted key,uses :); the hand-run red cases became a fixture test, which found and fixed one hole of its own (./local@<sha>matched the pin pattern because a dot could start the owner part; the owner part now starts with an alphanumeric); a comment that overstated what the version comment says; two over-long lines.Deliberate strictness left as is:
docker://images, subpath actions (owner/repo/path@sha), job-level reusable workflows, a trailing comment after the flag and extra text after the version comment all fail the tests until a workflow needs one. The pre-existing checkout-before-compose test still expects- uses: actions/checkout@as a step's first line and errors loudly on aname:-first checkout; that is onmainand untouched here.