From ae9477c708f588e92776c538deda64fd8e1e7176 Mon Sep 17 00:00:00 2001 From: Michael I Chen Date: Tue, 15 Sep 2026 02:08:20 -0700 Subject: [PATCH 1/2] chore: unwrap Markdown prose Adopts michen00/markdown-prose-hooks, which needed no porting: the hooks ship as a pre-commit repository and the PR-body halves as reusable workflows, so this consumes them rather than vendoring a copy that would then drift. Localization came to the two checks its README asks for, and both already held. markdownlint's `line-length` is `false` here and prettier's `proseWrap` is unset, so nothing wraps the prose back -- a rule that wraps and a hook that unwraps would each undo the other on every run. The `-py` mirror, not `-rs`: a `language: rust` hook builds from source, so pre-commit would install a Rust toolchain before checking the first commit, and nothing here needs cargo otherwise. The two PR-body workflows take different trigger types on purpose. Sharing an event fires both, and the report then describes a body the rewrite is about to replace. Both are held in bot-automerge's actions-major list, re-derived as that file asks. prose-body-write.yml runs from the default branch under `pull_request_target`, and prose-body.yml fires only on `synchronize` and `edited`, so neither is exercised by the pull request proposing its own bump. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/bot-automerge.yml | 10 ++++++ .github/workflows/prose-body-write.yml | 31 ++++++++++++++++ .github/workflows/prose-body.yml | 26 ++++++++++++++ .pre-commit-config.yaml | 12 +++++++ CLAUDE.md | 5 +-- CONTRIBUTING.md | 49 +++++++------------------- 6 files changed, 93 insertions(+), 40 deletions(-) create mode 100644 .github/workflows/prose-body-write.yml create mode 100644 .github/workflows/prose-body.yml diff --git a/.github/workflows/bot-automerge.yml b/.github/workflows/bot-automerge.yml index 33bff9c..233fada 100644 --- a/.github/workflows/bot-automerge.yml +++ b/.github/workflows/bot-automerge.yml @@ -289,6 +289,14 @@ jobs: # exercised either. release-tag.yml is the one that matters: it is part of the # release path. # + # The two prose-body workflows are held for the same reason, re-derived + # when they were added. prose-body-write.yml triggers on + # `pull_request_target`, so it runs from the default branch and a pull + # request never executes the version it proposes. prose-body.yml triggers + # on `pull_request` but only for `synchronize` and `edited`, neither of + # which a freshly opened bot pull request fires, so green says nothing + # about it either. + # # Listed rather than inferred, and that is this branch's known weakness: a # workflow's triggers can change without this file noticing, and a stale list # fails by arming. Re-derive it when a trigger changes. @@ -312,6 +320,8 @@ jobs: -e '.github/workflows/pre-commit-autoupdate.yml' \ -e '.github/workflows/delete-bot-branches-for-closed-prs.yml' \ -e '.github/workflows/bot-automerge-disarm.yml' \ + -e '.github/workflows/prose-body.yml' \ + -e '.github/workflows/prose-body-write.yml' \ || true) if [ "$held" -ne 0 ]; then echo "::warning::actions major reaches a workflow no PR runs; holding." diff --git a/.github/workflows/prose-body-write.yml b/.github/workflows/prose-body-write.yml new file mode 100644 index 0000000..08fec36 --- /dev/null +++ b/.github/workflows/prose-body-write.yml @@ -0,0 +1,31 @@ +--- +# Rewrites a hard-wrapped pull request body rather than reporting it, and after +# a rewrite deletes any report prose-body.yml left. The report cannot withdraw +# its own: GitHub starts no workflow run for an event caused by its own +# `GITHUB_TOKEN`, so nothing tells it the body has changed. +# +# Only on open, reopen and ready-for-review. Running it on every push would +# overwrite whatever the author had typed since. Drafts are not edited, which +# is what `ready_for_review` is here for. +# +# `pull_request_target` is the only trigger this one accepts, and it is why the +# file has to be on the default branch: under `pull_request` the workflow file +# would come from the pull request itself, and that file is what grants the +# token. It checks out nothing. A consequence worth knowing when editing it -- +# a pull request cannot try this out; it takes effect once merged. +# +# Unlike a file, a body carries no evidence of intent: there is no hard-break +# marker to distinguish a break the author wanted from one their editor made, +# so this can join a break that was meant to stay. `` on +# the line above a paragraph keeps it. +name: Prose body write + +on: + pull_request_target: + types: [opened, reopened, ready_for_review] + +jobs: + edit: + permissions: + pull-requests: write + uses: michen00/markdown-prose-hooks/.github/workflows/unwrap-pr-body.yml@v0.4.0 diff --git a/.github/workflows/prose-body.yml b/.github/workflows/prose-body.yml new file mode 100644 index 0000000..0e22bc4 --- /dev/null +++ b/.github/workflows/prose-body.yml @@ -0,0 +1,26 @@ +--- +# Reports a hard-wrapped pull request body. GitHub renders every newline in a +# body as a `
`, so a body wrapped in an editor or a heredoc reaches readers +# at whatever width it was typed to. A file has the opposite problem -- the +# break renders as a space and costs a reflowed diff -- and the pre-commit hook +# covers that side. +# +# Deliberately narrower than the upstream example, which also carries `opened` +# and `reopened`. Those belong to prose-body-write.yml: sharing an event fires +# both at once, and this report would then describe a body the rewrite is about +# to replace. `edited` is not in the default type set and is the one that +# matters -- editing a body fires it alone, so without it a report would stand +# on a body the author has already fixed. +name: Prose body + +on: + pull_request: + types: [synchronize, edited] + +jobs: + report: + # The reusable workflow declares no permissions of its own; only the + # comment needs one, and the scope belongs on the calling job. + permissions: + pull-requests: write + uses: michen00/markdown-prose-hooks/.github/workflows/unwrap-pr-body-check.yml@v0.4.0 diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 8249d45..d51fca8 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -65,6 +65,18 @@ repos: hooks: - id: trailing-whitespace exclude: ^tests/test-conventional-merge-commit\.csv$ + # Before prettier, not after: this rewrites prose, and prettier is what + # normalizes the result. markdownlint's `line-length` is `false` here and + # prettier's `proseWrap` is left at `preserve`, so nothing downstream wraps + # the prose back -- which is the one thing that would make these fight. + # + # The `-py` mirror rather than `-rs`: a `language: rust` hook builds from + # source, so pre-commit would install a whole Rust toolchain before checking + # the first commit, and nothing here needs cargo otherwise. + - repo: https://github.com/michen00/markdown-prose-hooks-py + rev: v0.4.0 + hooks: + - id: unwrap-markdown-prose-py - repo: https://github.com/rbubley/mirrors-prettier rev: v3.9.6 hooks: diff --git a/CLAUDE.md b/CLAUDE.md index 5da78b2..9781d22 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -37,10 +37,7 @@ See @README.md for project overview and features. - Common types: `feat`, `fix`, `docs`, `chore`, `test`, `refactor` - Merge commits should be prefixed with `chore: merge` - Keep commit messages clear and descriptive -- **Land PRs with squash merge only** — the repo allows no other method. Rebase merge - replays commits unsigned, which loses the signature the `main-protect` ruleset asks for; - GitHub signs the commit it creates for a squash. Write PR titles squash-ready, since the - title becomes the squash subject and the branch's commit messages become its body. +- **Land PRs with squash merge only** — the repo allows no other method. Rebase merge replays commits unsigned, which loses the signature the `main-protect` ruleset asks for; GitHub signs the commit it creates for a squash. Write PR titles squash-ready, since the title becomes the squash subject and the branch's commit messages become its body. ## Testing Requirements diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1c4c5d2..48ec0a7 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -73,23 +73,12 @@ Default flow (automated): 1. **Release PR** (`.github/workflows/release-pr.yml`) opens the release PR by itself when a commit worth releasing lands on `main`. Every conventional type bumps at least the patch version, so the version cannot decide that on its own; the workflow gates on the group `cliff.toml` parsed each commit into — features, fixes, performance and reverts, plus anything marked breaking. A `chore`, `docs`, `build`, `ci`, `test`, `refactor` or `style` merge — the weekly hook autoupdate and Dependabot among them — rides along in the next release without proposing one. Run the workflow by hand, or `make release-pr`, to pin the version or to release a batch containing none of those types; a manual run skips the worthiness gate. Leave `version` empty to derive it via `git cliff --bumped-version`, or pass `X.Y.Z` / `vX.Y.Z`. 1. Review and merge the generated PR (`chore(release): prepare vX.Y.Z`). -1. **Release Tag** workflow (`.github/workflows/release-tag.yml`) runs on merge of a - `release/*` branch. It creates a GPG-signed annotated tag, pushes it, and dispatches - **Release Publish**. -1. **Release Publish** workflow (`.github/workflows/release-publish.yml`) builds and - uploads signed artifacts to the GitHub release. - -No local step is required after the release PR merges, but the release is not automatic: -**Release Tag** runs in the protected `release` environment and waits for a maintainer to -approve the run. Approve from the run page, or from the PR's checks tab, to mint the tag and -publish. Rejecting the approval leaves no tag behind. - -That is the only approval a normal release needs. **Release Publish** also declares the -`release` environment, but when **Release Tag** dispatches it the deployment is created by -`github-actions[bot]` and the reviewer rule is skipped, so it proceeds without a second -prompt. The declaration still gates a Release Publish run dispatched by hand, which is the -manual fallback path. Treat the Release Tag approval as the release decision — no tag means -no publish. +1. **Release Tag** workflow (`.github/workflows/release-tag.yml`) runs on merge of a `release/*` branch. It creates a GPG-signed annotated tag, pushes it, and dispatches **Release Publish**. +1. **Release Publish** workflow (`.github/workflows/release-publish.yml`) builds and uploads signed artifacts to the GitHub release. + +No local step is required after the release PR merges, but the release is not automatic: **Release Tag** runs in the protected `release` environment and waits for a maintainer to approve the run. Approve from the run page, or from the PR's checks tab, to mint the tag and publish. Rejecting the approval leaves no tag behind. + +That is the only approval a normal release needs. **Release Publish** also declares the `release` environment, but when **Release Tag** dispatches it the deployment is created by `github-actions[bot]` and the reviewer rule is skipped, so it proceeds without a second prompt. The declaration still gates a Release Publish run dispatched by hand, which is the manual fallback path. Treat the Release Tag approval as the release decision — no tag means no publish. Two guards follow from that. Because the tag is what marks a release finished, **Release PR** refuses to prepare a second one while the last prepared version is still untagged — on a push it says so and stops, and a manual run fails. That covers the approval window: a `fix` merged while **Release Tag** waits would otherwise propose a duplicate PR for the version already on its way out. It also latches when an approval is _rejected_, since that leaves a prepared version that never gets a tag; clear it with the manual fallback below, which both publishes that release and satisfies the check. @@ -100,34 +89,25 @@ Manual fallback: 1. Tag and push by hand from `main`: - `git switch main && git pull` - `git tag -a vX.Y.Z -m vX.Y.Z -s` - - `git push origin vX.Y.Z` - A tag pushed this way triggers **Release Publish** directly on tag push. + - `git push origin vX.Y.Z` A tag pushed this way triggers **Release Publish** directly on tag push. 1. If needed, run **Release Publish** via `workflow_dispatch` with an existing `tag`. -> **Note:** Release Tag dispatches Release Publish explicitly rather than relying on the -> tag push, because a tag pushed with `GITHUB_TOKEN` does not trigger `on: push: tags`. +> **Note:** Release Tag dispatches Release Publish explicitly rather than relying on the tag push, because a tag pushed with `GITHUB_TOKEN` does not trigger `on: push: tags`. Signing model: - Sigstore keyless signatures are generated in CI for every release artifact. - GPG detached signatures are also generated for compatibility. -- Release tags are annotated and GPG-signed. When **Release Tag** creates the tag, it is - signed with the CI release key rather than a maintainer's personal key. The protected - `release` environment is what keeps that key from being usable by anyone who merges a - `release/*` PR: the tagging job waits for maintainer approval before it runs. +- Release tags are annotated and GPG-signed. When **Release Tag** creates the tag, it is signed with the CI release key rather than a maintainer's personal key. The protected `release` environment is what keeps that key from being usable by anyone who merges a `release/*` PR: the tagging job waits for maintainer approval before it runs. - Required repository secrets for GPG signing in CI: - `RELEASE_GPG_PRIVATE_KEY` (ASCII-armored private key) - `RELEASE_GPG_PASSPHRASE` (passphrase for the private key) -Without both secrets, **Release Tag** and **Release Publish** fail at the GPG import step, -so no tag is created and no artifacts are published. A GitHub App token does not substitute -for them: a token authenticates git and API calls but cannot produce a GPG signature, and -GitHub signs only commits it creates via the API, never annotated tag objects. +Without both secrets, **Release Tag** and **Release Publish** fail at the GPG import step, so no tag is created and no artifacts are published. A GitHub App token does not substitute for them: a token authenticates git and API calls but cannot produce a GPG signature, and GitHub signs only commits it creates via the API, never annotated tag objects. ##### One-time release key setup -Run these locally as a maintainer; never paste private key material into an issue, a PR, or -a chat transcript. +Run these locally as a maintainer; never paste private key material into an issue, a PR, or a chat transcript. ```bash # 1. Pick a passphrase and generate a dedicated release key (not your personal key). @@ -158,12 +138,9 @@ fi gpg --armor --export ``` -Confirm both secrets landed with `gh secret list`. The key expires in two years; rotate by -repeating these steps. +Confirm both secrets landed with `gh secret list`. The key expires in two years; rotate by repeating these steps. -Neither removal above guarantees the bytes are gone: copy-on-write filesystems and SSD wear -levelling can leave the export recoverable. Treat the passphrase as the real protection for -that file, and prefer a passphrase over an empty one for exactly this reason. +Neither removal above guarantees the bytes are gone: copy-on-write filesystems and SSD wear levelling can leave the export recoverable. Treat the passphrase as the real protection for that file, and prefer a passphrase over an empty one for exactly this reason. Verification examples: From ca01cb4a60158f98af7053c60a5a0fdccf3a281f Mon Sep 17 00:00:00 2001 From: Michael I Chen Date: Tue, 15 Sep 2026 02:21:14 -0700 Subject: [PATCH 2/2] fix(prose-body): report from pull_request_target A fork's token is read-only whatever the caller's `permissions:` block asks for, so under `pull_request` the report's comment was refused for every outside contributor -- the people least likely to know this repository's prose convention, and the ones the report is for. Not a failure, and the report was not lost: the reusable workflow falls back to the job summary and does not fail the check. But a summary has to be opened to be read, and a comment does not. Safe here because that workflow checks out nothing -- it carries no `actions/checkout` step at all, reading the body from the event payload and posting a comment. The usual `pull_request_target` hazard is a writable token handed to unreviewed code, and there is no code here for it to attach to. Upstream accepts either trigger and names this one for fork coverage. The cost matches the write half: the workflow is read from the default branch, so a pull request cannot test a change to it. Reported by Qodo on #85. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/bot-automerge.yml | 10 ++++------ .github/workflows/prose-body.yml | 22 +++++++++++++++++++++- 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/.github/workflows/bot-automerge.yml b/.github/workflows/bot-automerge.yml index 233fada..0a40abe 100644 --- a/.github/workflows/bot-automerge.yml +++ b/.github/workflows/bot-automerge.yml @@ -290,12 +290,10 @@ jobs: # release path. # # The two prose-body workflows are held for the same reason, re-derived - # when they were added. prose-body-write.yml triggers on - # `pull_request_target`, so it runs from the default branch and a pull - # request never executes the version it proposes. prose-body.yml triggers - # on `pull_request` but only for `synchronize` and `edited`, neither of - # which a freshly opened bot pull request fires, so green says nothing - # about it either. + # when they were added and again when the report moved to + # `pull_request_target`. Both now trigger on that event, so both run + # from the default branch and a pull request never executes the version + # it proposes -- green here would say nothing about either. # # Listed rather than inferred, and that is this branch's known weakness: a # workflow's triggers can change without this file noticing, and a stale list diff --git a/.github/workflows/prose-body.yml b/.github/workflows/prose-body.yml index 0e22bc4..4da5a09 100644 --- a/.github/workflows/prose-body.yml +++ b/.github/workflows/prose-body.yml @@ -11,10 +11,30 @@ # to replace. `edited` is not in the default type set and is the one that # matters -- editing a body fires it alone, so without it a report would stand # on a body the author has already fixed. +# +# `pull_request_target`, not `pull_request`, and the difference is who the +# report reaches. A fork's token is read-only whatever a caller's +# `permissions:` block asks for, so under `pull_request` the comment is refused +# for every outside contributor -- the people least likely to know this +# repository's prose convention, and the ones the report exists for. The run +# does not fail there and the report is not lost: the reusable workflow falls +# back to the job summary. But a summary is something you have to open a check +# to find, and a comment is not. +# +# What makes that safe here is that the reusable workflow checks out nothing -- +# it has no `actions/checkout` step at all. It reads the body from the event +# payload and posts a comment, so the usual `pull_request_target` hazard, a +# writable token handed to unreviewed code, has nothing to attach to. Keep it +# that way: adding a checkout of the pull request head under this trigger is +# what would turn it into the vulnerability the pattern is known for. +# +# The cost is the same one prose-body-write.yml pays. `pull_request_target` +# reads the workflow from the default branch, so a pull request cannot test a +# change to this file; it takes effect once merged. name: Prose body on: - pull_request: + pull_request_target: types: [synchronize, edited] jobs: