Zizmor, Linting, documentation - #738
Merged
Merged
Conversation
The post_comment activity never touches the Contents API: it reads the workflow run and its artifacts (actions: read), lists the PR (pull-requests: read) and creates or edits an issue comment (pull-requests: write). Getting the repository only needs the metadata scope, which is always granted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The e2e PATs were repository secrets, so every job in every workflow could read them -- including the pytest run of any collaborator's pull request. Scoping them to an `e2e` environment means only the jobs that declare it can reach them, and it allows a deployment branch rule to restrict them to main. The workflows that need them all run on the default branch's ref (workflow_run, pull_request_target, issue_comment), so such a rule doesn't affect them. ci.yml runs on refs/pull/N/merge and cannot use the environment, so it no longer receives the tokens at all; the e2e tests skip without them, and external runs go through the /e2e approval flow. This only takes effect once the secrets are actually moved to the environment in the repository settings; until then the repository secrets still resolve. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Nothing consumes ewjoachim/python-coverage-comment-action-base:v7. The runtime Dockerfile moved to ghcr.io in 90c8df8, released in v3.31; every version that still pulls from Docker Hub pins :v6 or older, and those tags keep existing. The push was write-only, so drop it along with DOCKER_PASSWORD. While here, fix the stale pointer in Dockerfile.build: the base image version lives in ci.yml, not in a release.yml that no longer exists. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three of zizmor's pedantic/auditor audits, all on the repo's own workflows: - undocumented-permissions: each requested permission now says what it's for. Note zizmor only accepts a trailing comment, not one on the line above. - anonymous-definition: name the two jobs that were just `test:`. - concurrency-limits: without a group, concurrent runs race on shared state -- two runs can each decide no coverage comment exists yet and both post one. On workflow_run, github.ref is always the default branch, so those group on the triggering run's head branch instead; the cleanup and invite workflows group per PR and never cancel, since they must run to completion. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The README examples had drifted: they recommended persist-credentials: true,
carried permission comments zizmor doesn't accept, and had no top-level
permissions or concurrency. Nothing linted them, because nothing could -- they
only existed inside Markdown fences.
They're now real workflow files that zizmor lints, pinact pins and renovate
keeps current, copied into the README by a pre-commit hook. The fence carries
the source path, and GitHub drops everything after the language, so it stays
invisible when rendered. `lines=` shows a slice, for snippets that would be
noise as a whole workflow.
Renovate doesn't look outside .github/ on its own, hence the config addition;
without it the new pins would silently rot.
The examples themselves now use persist-credentials: false, a top-level
permissions: {}, a concurrency group, and real pinned SHAs.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The action stopped relying on the credentials actions/checkout writes into .git/config in 12255bc, released in v4.1: it passes the GITHUB_TOKEN it is given straight to git via http.extraheader for every network operation. Say so explicitly, and say which version, so that readers still pinned to a v3.x release don't follow the new examples and break their coverage push. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Now that the repository is clean at that level. It's worth the step up: artipacked -- an explicit persist-credentials: true, the thing that started all this -- is only reported at the auditor persona. The default persona only catches a checkout with no persist-credentials at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
shellcheck's SC2086. The values already come in through env: rather than being interpolated by Actions, so this is the second half of the same rule -- read them back as "$VAR" so a value with whitespace or a glob character can't be split or expanded. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Complements zizmor, which only looks at security: actionlint validates expressions and contexts, so a typo like github.event_nam or a reference to an undefined step output is caught. That matters for the examples, which document 20+ steps.coverage_comment.outputs.* names. shellcheck-py comes along because actionlint's shell linting is a no-op when shellcheck isn't on PATH -- silently, with no warning that the check was skipped. Two limits worth knowing. actionlint checks `with:` input names against a bundled database keyed by tag, so it never fires on a SHA-pinned `uses:` -- which is all of them here. And it reads every file it's given as a workflow, so action.yml is excluded from the hook. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Added in 717fea2 and released in v3.36, but never listed in "All options", so it has been undiscoverable for close to a year. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
USE_GH_PAGES_HTML_URL went undocumented for nearly a year, and nothing was in a position to notice. GitHub Actions only logs an "Unexpected input(s)" warning for a name that doesn't exist, actionlint's input database is keyed by tag so it never fires on a SHA-pinned `uses:`, and zizmor ignores inputs. So compare both directions: inputs used in docs/examples/ must exist in action.yml, and every input in action.yml must appear in "All options". The script locates things by structure -- a heading, a key -- and a checker that silently finds nothing reads as a pass, which is worse than having no checker. Each lookup therefore asserts it found something and fails loudly if the shape of action.yml or the README moves underneath it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Admin commands cheatsheet:
|
Correction to ce3be58, which took the tokens away from ci.yml on the grounds that member pull requests could fall back to the /e2e approval flow. They can't: e2e-external-phase-1 gates on author_association != 'MEMBER', because that flow exists precisely for the PRs ci.yml doesn't cover. So that commit left our own PRs with no end-to-end run at all, and GitHub doesn't allow approving your own PR, so no condition tweak fixes it either. Declare the environment on the job instead and give it no protection rules. The tokens stay reachable from refs/pull/N/merge, which a deployment branch rule would refuse, and the scoping still holds: a job can only read them by naming the environment, which is a visible line in the diff. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Coverage report
This PR does not seem to contain any modification to coverable code. |
Three things fall out of the split. The e2e tokens stop being in scope for the unit tests, the coverage comment and the artifact upload, which is a tighter grant than putting the environment on the whole job. The `e2e` deployment stops appearing on fork pull requests, where the suite only skips for want of secrets, and where a deployment implied work that never happened. And an end-to-end failure is now distinguishable from a unit test failure. The end-to-end tests import nothing from coverage_comment, so ignoring them in the main job leaves the coverage figure unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Now that the job declares an environment, GitHub shows a deployment on the pull request, and it can carry the link that two workflows existed to post. The URL is the owner's repository list filtered by pull request number rather than one repository: a deployment gets a single URL, and this covers both the public and the private repo, while listing nothing instead of 404ing when a run created neither. So e2e-public-link-in-pr.yml goes, taking a pull_request_target trigger with it, and the private workflow keeps only the half a URL can't replace -- the invitation itself -- and is renamed to match. The admin commands it used to advertise move to CONTRIBUTING. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Member
Author
|
/invite |
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
1 participant
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.
Documentation was stale (still advertizing
persist-credentials: true), Zizmor was still finding things (but without the auditor persona it didn't surface it)`.Fixing this for good: adding zizmor in auditor mode, make it run on README code examples, add actionlint.
Closes #
Checklist: