Release guard tidy: threat model stated, preload route pinned shut, dated changelog headings read
- The npm Publish step starts the guard under `env -u NODE_OPTIONS`, and the pin's exact text carries it, so a workflow or job NODE_OPTIONS preload cannot run inside the guard (preload-fakes-guard). - The pin's shell/defaults ban and its test are gone: it defended only a deliberate edit, which is review's job (suite-gate-structural-not-parsed), and refused a harmless defaults.run.working-directory. A step-level shell still fails the exact comparison (shell-ban-deliberate-only). - docs/semver-policy.md states that the guard and pin stop accidental edits, that the cleaned environment removes known variables rather than proving no inherited one matters (env-denylist-overclaim), recommends a required-reviewer environment for the npm job, and gives the first-release note with the valid next versions. - check-changelog-semver accepts `## [x.y.z] - YYYY-MM-DD` and `## x.y.z - YYYY-MM-DD` beside the plain forms (changelog-first-entry). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
082aec7962
commit
49a5526ede
4 changed files with 70 additions and 25 deletions
5
.github/workflows/publish-packages.yml
vendored
5
.github/workflows/publish-packages.yml
vendored
|
|
@ -74,10 +74,11 @@ jobs:
|
|||
# One process checks that the tag is the manifest version, runs the
|
||||
# release-checks suite, the semver policy check (docs/semver-policy.md)
|
||||
# and npm test, and only then runs npm publish --access public. Only
|
||||
# npm publish receives the token.
|
||||
# npm publish receives the token. It starts without NODE_OPTIONS, so no
|
||||
# preloaded module runs inside it.
|
||||
- name: Publish
|
||||
if: startsWith(github.ref, 'refs/tags/cultcache-ts-v')
|
||||
run: node scripts/release-after-suite.mjs npm-publish packages/cultcache-ts "$GITHUB_REF"
|
||||
run: env -u NODE_OPTIONS node scripts/release-after-suite.mjs npm-publish packages/cultcache-ts "$GITHUB_REF"
|
||||
env:
|
||||
NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}
|
||||
|
||||
|
|
|
|||
|
|
@ -126,7 +126,9 @@ considered stable), not something the checker infers on its own.
|
|||
Every published package keeps a `CHANGELOG.md` beside its manifest
|
||||
(`unity/org.gamecult.cultlib/CHANGELOG.md` and
|
||||
`src/GameCult.Unity/Assets/Caching/CHANGELOG.md` are the existing precedent).
|
||||
Each release adds a `## [<version>]` section containing:
|
||||
Each release adds a `## [<version>]` section (the check also reads
|
||||
`## <version>`, and either form followed by Keep a Changelog's
|
||||
` - YYYY-MM-DD` date, such as `## [0.4.0] - 2026-10-03`) containing:
|
||||
|
||||
- a category subsection per change (`### Added`, `### Changed`, `### Fixed`,
|
||||
etc., following Keep a Changelog style);
|
||||
|
|
@ -140,6 +142,22 @@ A version with no changelog entry at all fails the check below, the same as
|
|||
a mislabelled one — an undocumented release is not a smaller sin than a
|
||||
mislabelled one.
|
||||
|
||||
### First release of a package
|
||||
|
||||
A package with no `CHANGELOG.md` yet cannot release: the check refuses with
|
||||
"changelog not found". Its first release writes the file at the declared path,
|
||||
`packages/<package>/CHANGELOG.md`, with one section for the version being
|
||||
released. That version must still be exactly the next one after the package's
|
||||
nearest ancestor tag. As of 2026-10-03 the valid next versions are:
|
||||
|
||||
- `cultcache-ts`, after `cultcache-ts-v0.14.0`: `0.14.1`, `0.15.0` or `1.0.0`;
|
||||
- `cultcache-py`, after `cultcache-py-v0.3.0`: `0.3.1`, `0.4.0` or `1.0.0`;
|
||||
- `cultnet-py` and `cultmesh-py`, which have no tags: `0.1.0`, their manifest
|
||||
version, as a first release. Run it from a clone with the tags fetched, or
|
||||
the check cannot tell a first release from a missing record.
|
||||
|
||||
On `0.y.z` a `### Breaking` section needs at least a minor bump.
|
||||
|
||||
## Enforcement
|
||||
|
||||
`scripts/check-changelog-semver.mjs` is the mechanical check. Given a
|
||||
|
|
@ -289,15 +307,32 @@ check for the tagged version, runs the package's own tests (`npm test`, or the
|
|||
Python package's unit tests), and only then the release action: `npm publish`
|
||||
for `cultcache-ts`, which it refuses on any ref but a release tag, and for the
|
||||
Python packages the sdist and wheel build, whose output is all the PyPI upload
|
||||
step uploads. The checks run without `NODE_OPTIONS`, `NODE_TEST_CONTEXT`, any
|
||||
other `NODE_TEST_*` variable and `NODE_AUTH_TOKEN`, so no inherited variable can
|
||||
make the tests run nothing and pass; only the release action receives the token.
|
||||
step uploads. The checks run with the known test-runner and token variables
|
||||
removed (`NODE_OPTIONS`, `NODE_TEST_CONTEXT`, any other `NODE_TEST_*` variable,
|
||||
and `NODE_AUTH_TOKEN`), so none of those can make the tests run nothing and
|
||||
pass, and only the release action receives the token. That list names known
|
||||
variables; it does not prove that no other inherited variable can affect a
|
||||
check. The npm Publish step starts the guard under `env -u NODE_OPTIONS`, so a
|
||||
`NODE_OPTIONS` preload set in the workflow's or job's `env:` cannot run code
|
||||
inside the guard before its first line.
|
||||
The gate is that call, not a step condition, so no `if:` or `continue-on-error`
|
||||
can release what a check refused. The tests compare each job's release steps
|
||||
with their expected text exactly, less the steps' `if:` lines, and refuse `shell`
|
||||
and `defaults` anywhere in the workflow, which could run a release step's line
|
||||
through another command. Removing the call is still possible, but only as a visible edit to those
|
||||
steps and to the tests.
|
||||
with their expected text exactly, less the steps' `if:` lines. Removing the call
|
||||
is still possible, but only as a visible edit to those steps and to the tests.
|
||||
|
||||
The guard and that pin stop accidental workflow edits. They do not stop a
|
||||
deliberate one: an extra step that publishes, a workflow-level `defaults:` or
|
||||
`env:`, or a fake `node` on `$GITHUB_PATH` all pass them. Deliberate edits are
|
||||
review's job, as ruled in `suite-gate-structural-not-parsed`, and the pin
|
||||
does not try to parse the workflow for them. For the same reason it has no ban
|
||||
on `shell` or `defaults`: such a ban defends only against a deliberate edit,
|
||||
and it also refused harmless settings such as `defaults.run.working-directory`.
|
||||
A step-level `shell:` on a release step still fails the exact comparison.
|
||||
A recommendation for the operator, not something this repository can set: the
|
||||
Python job already runs in the `pypi` GitHub environment, as PyPI trusted
|
||||
publishing expects. Putting the npm job in an environment with required
|
||||
reviewers would make every npm release wait for a person's approval. That is a
|
||||
GitHub settings change.
|
||||
|
||||
`org.gamecult.cultlib` compiles CultMath from source and ships beside
|
||||
`org.gamecult.cultmath`, so its build refuses unless it declares exactly the
|
||||
|
|
|
|||
|
|
@ -73,12 +73,13 @@ export function classifyBump(prev, next) {
|
|||
return { invalid: compareVersions(next, prev) < 0 ? "reverse" : "skip" };
|
||||
}
|
||||
|
||||
// Extracts the `## [<version>]` section of a changelog and reports whether it
|
||||
// carries a BREAKING_HEADING subsection. Throws if the version has no entry
|
||||
// Extracts the `## [<version>]` section of a changelog (also `## <version>`,
|
||||
// and either with Keep a Changelog's ` - YYYY-MM-DD` date) and reports whether
|
||||
// it carries a BREAKING_HEADING subsection. Throws if the version has no entry
|
||||
// at all — an undocumented release is refused the same as a mislabelled one.
|
||||
export function hasBreakingSection(changelogText, version) {
|
||||
const sectionHeading = new RegExp(
|
||||
`^##\\s*\\[?${version.replace(/\./g, "\\.")}\\]?\\s*$`,
|
||||
`^##\\s*\\[?${version.replace(/\./g, "\\.")}\\]?(\\s+-\\s+\\d{4}-\\d{2}-\\d{2})?\\s*$`,
|
||||
"m",
|
||||
);
|
||||
const startMatch = sectionHeading.exec(changelogText);
|
||||
|
|
|
|||
|
|
@ -84,6 +84,19 @@ test("hasBreakingSection scopes to the named version's section only", () => {
|
|||
assert.throws(() => hasBreakingSection(text, "0.9.0"), /no "## \[0\.9\.0\]" entry/);
|
||||
});
|
||||
|
||||
test("hasBreakingSection reads Keep a Changelog's dated heading beside the plain forms", () => {
|
||||
const text = ["## [0.4.0] - 2026-10-03", "### Breaking", "- x", "## 0.3.1 - 2026-09-01", "### Breaking", "## [0.3.0]", "### Fixed", ""].join("\n");
|
||||
assert.equal(hasBreakingSection(text, "0.4.0"), true);
|
||||
assert.equal(hasBreakingSection(text, "0.3.1"), true);
|
||||
assert.equal(hasBreakingSection(text, "0.3.0"), false);
|
||||
// a dated section ends at the next heading, dated or not
|
||||
assert.equal(hasBreakingSection(["## [0.4.0] - 2026-10-03", "### Fixed", "## [0.3.9] - 2026-09-01", "### Breaking"].join("\n"), "0.4.0"), false);
|
||||
// only a date may follow the version
|
||||
for (const heading of ["## [0.4.0] - soon", "## [0.4.0] 2026-10-03", "## [0.4.0] - 2026-10-03 extra", "## [0.4.0.1] - 2026-10-03"]) {
|
||||
assert.throws(() => hasBreakingSection(`${heading}\n### Breaking\n`, "0.4.0"), /no "## \[0\.4\.0\]" entry/, heading);
|
||||
}
|
||||
});
|
||||
|
||||
// Table-driven per the operator's required cases (spec: breaking+major=pass,
|
||||
// breaking+patch=fail, breaking+minor=fail, non-breaking+minor=pass,
|
||||
// 0.y.z breaking+minor=pass, skipped version=fail, downgrade=fail).
|
||||
|
|
@ -746,16 +759,16 @@ test("workflow: the Caching Unity prefix is its own tag family, not another pack
|
|||
// suite, the semver policy check and the package's tests and then the release action from one process, so no
|
||||
// if: or continue-on-error on any step can release what those checks did not pass. This pin reads no
|
||||
// condition. It compares each job's release steps whole, less their own if: line, with the text below, so a
|
||||
// release step that does anything else is a mismatch, and it refuses shell and defaults anywhere, which could
|
||||
// run the pinned line through another command. Each Unity release script runs the suite itself, before it
|
||||
// calls the checker.
|
||||
// release step that does anything else is a mismatch. It stops accidental edits; a deliberate one, such as a
|
||||
// workflow-level defaults or env or an extra step, is review's job (ruling suite-gate-structural-not-parsed).
|
||||
// Each Unity release script runs the suite itself, before it calls the checker.
|
||||
const SUITE = /node\s+--test[^\n]*check-changelog-semver\.test\.mjs/;
|
||||
const CHECKER = /check-changelog-semver\.mjs/;
|
||||
const code = (text) => text.split("\n").filter((line) => !line.trim().startsWith("#")).join("\n");
|
||||
|
||||
const RELEASE_STEPS = {
|
||||
"cultcache-ts": [
|
||||
["- name: Publish", " run: node scripts/release-after-suite.mjs npm-publish packages/cultcache-ts \"$GITHUB_REF\"", " env:", " NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}"],
|
||||
["- name: Publish", " run: env -u NODE_OPTIONS node scripts/release-after-suite.mjs npm-publish packages/cultcache-ts \"$GITHUB_REF\"", " env:", " NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}"],
|
||||
],
|
||||
// Publish uploads only the dist the guarded step built, so it has nothing to upload unless the suite passed.
|
||||
python: [
|
||||
|
|
@ -805,10 +818,6 @@ function workflowReleaseProblems(workflow) {
|
|||
from = at + 1;
|
||||
}
|
||||
});
|
||||
// A job's or the workflow's defaults.run.shell would run the pinned release line through any command.
|
||||
if (/\bshell\b|\bdefaults\b/.test(code(workflow))) {
|
||||
problems.push("the workflow says shell or defaults: a release step must run its pinned line through the default shell");
|
||||
}
|
||||
return problems;
|
||||
}
|
||||
|
||||
|
|
@ -821,7 +830,7 @@ function scriptSuiteProblems(script) {
|
|||
test("workflow: each job releases only through release-after-suite.mjs", () => {
|
||||
const workflow = workflowText();
|
||||
assert.deepEqual(workflowReleaseProblems(workflow), []);
|
||||
const npmRun = 'run: node scripts/release-after-suite.mjs npm-publish packages/cultcache-ts "$GITHUB_REF"';
|
||||
const npmRun = 'run: env -u NODE_OPTIONS node scripts/release-after-suite.mjs npm-publish packages/cultcache-ts "$GITHUB_REF"';
|
||||
const pythonRun = 'run: node scripts/release-after-suite.mjs python-build packages/${{ matrix.package }} "$GITHUB_REF"';
|
||||
const exact = /is not exactly the release call/;
|
||||
for (const run of [npmRun, pythonRun]) {
|
||||
|
|
@ -833,15 +842,14 @@ test("workflow: each job releases only through release-after-suite.mjs", () => {
|
|||
assert.match(edit("run: |\n node --test scripts/check-changelog-semver.test.mjs\n true"), exact, run);
|
||||
assert.match(edit(run.replace("release-after-suite.mjs", "check-changelog-semver.mjs")), exact, run);
|
||||
assert.match(edit(run.replace('"$GITHUB_REF"', "refs/tags/cultcache-ts-v0.14.0")), exact, run);
|
||||
assert.match(edit(`${run}\n shell: bash`), /another command|default shell/, run);
|
||||
assert.match(edit(`${run}\n shell: bash`), exact, run);
|
||||
assert.match(edit(`${run}\n continue-on-error: true`), exact, run);
|
||||
}
|
||||
// the guard runs every release check itself, so neither the release steps' conditions nor a job allowed
|
||||
// to fail can release what a check refused
|
||||
assert.deepEqual(workflowReleaseProblems(workflow.replace(/if: startsWith\(github\.ref, 'refs\/tags\/cultcache-ts-v'\)(\r?\n\s+run: node)/, "if: always()$1")), []);
|
||||
assert.deepEqual(workflowReleaseProblems(workflow.replace(/( runs-on: ubuntu-latest\r?\n)/, "$1 continue-on-error: true\n")), []);
|
||||
// a job's default shell, a release step removed, renamed or doubled, a Publish that uploads another dist, an unknown job
|
||||
assert.match(workflowReleaseProblems(workflow.replace(/( runs-on: ubuntu-latest\r?\n)/, "$1 defaults:\n run:\n shell: sh -c 'npm publish' {0}\n")).join("\n"), /default shell/);
|
||||
// a release step removed, renamed or doubled, a Publish that uploads another dist, an unknown job
|
||||
assert.match(workflowReleaseProblems(workflow.replace("- name: Build sdist and wheel", "- name: Build")).join("\n"), /needs exactly one step "- name: Build sdist and wheel"/);
|
||||
assert.match(workflowReleaseProblems(workflow.replace("- name: Build and test", "- name: Publish\n run: npm publish\n\n - name: Build and test")).join("\n"), /needs exactly one step "- name: Publish"/);
|
||||
assert.match(workflowReleaseProblems(workflow.replace("packages-dir: packages/${{ matrix.package }}/dist", "packages-dir: dist")).join("\n"), exact);
|
||||
|
|
|
|||
Loading…
Reference in a new issue