Workflow pin: refuses success() and any if: continued past its line, the way block scalars are refused

Soul s2 found 'success() || c' and plain or quoted if: values continued on a
following line passing the pin. Any status function now refuses, and stepCondition
reports an if: whose value continues (block scalar or a deeper-indented next line)
as multi-line, which the pin refuses. Soul's four probe forms are asserted.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Meta Vens 2026-10-03 13:26:43 +02:00
commit 942e4033b2
2 changed files with 37 additions and 19 deletions

View file

@ -284,10 +284,12 @@ Each of those runs the check's own tests first, and the tests pin that order.
In `publish-packages.yml` the test step must run whenever the job's Publish
step does. The pin proves that from the text of the two `if:` conditions, so it
accepts only three shapes: no `if:` on the test step, the same `if:` on both,
or Publish's `if:` as the test step's followed by ` && ` with no `||`. A
block-scalar `if:` (`>-`, `|`) is refused, and so is a Publish `if:` calling
`always()`, `cancelled()` or `failure()`, which would publish after the tests
failed. Matching text rather than parsing GitHub's expression language also
or Publish's `if:` as the test step's followed by ` && ` with no `||`. The pin
reads one line, so a multi-line `if:` is refused: a block scalar (`>-`, `|`),
or a plain or quoted value continued on the next line. So is a Publish `if:`
calling `always()`, `cancelled()`, `failure()` or `success()`: any status
function replaces the implicit `success()`, so `success() || c` would publish
after the tests failed, and `success() && c` says nothing `c` alone does not. Matching text rather than parsing GitHub's expression language also
refuses some sound conditions: a parenthesised `S && (c || d)`, conjuncts in
another order, and a condition wrapped in `${{ }}` or spaced differently on one
step only. Rewrite those into an accepted shape; a wrong pass would publish a

View file

@ -747,19 +747,29 @@ test("workflow: the Caching Unity prefix is its own tag family, not another pack
// before it calls the checker.
const SUITE = /node\s+--test[^\n]*check-changelog-semver\.test\.mjs/;
const CHECKER = /check-changelog-semver\.mjs/;
// A status function replaces a step's implicit success(), so Publish would run after the suite failed:
// the same bypass as continue-on-error. "!cancelled()" matches through "cancelled(".
const STATUS_FUNCTION = /\b(?:always|cancelled|failure)\s*\(/;
// Any status function replaces a step's implicit success(), so Publish could run after the suite failed:
// the same bypass as continue-on-error. success() is refused too: "success() || c" publishes on c alone,
// and "success() && c" is sound but says nothing the same condition without it does not. "!cancelled()"
// matches through "cancelled(".
const STATUS_FUNCTION = /\b(?:always|cancelled|failure|success)\s*\(/;
const code = (text) => text.split("\n").filter((line) => !line.trim().startsWith("#")).join("\n");
// The if: of the step that holds offset `at` in a job's text, or null when the step has none. A step
// runs from its "- name:" line to the next one. Only the if: line is read, so a block scalar
// (if: >- or if: |) comes back as its indicator; workflowSuiteProblems refuses it.
// runs from its "- name:" line to the next one. Only the if: line is read, so an if: whose value goes on
// past that line comes back as MULTI_LINE, which workflowSuiteProblems refuses: a block scalar (if: >-,
// if: |), or a plain or quoted scalar continued on a following line indented deeper than the key, where
// always() or "|| true" would hide from a one-line read.
const MULTI_LINE = Symbol("multi-line if:");
function stepCondition(job, at) {
const names = [...job.matchAll(/^\s+- name:/gm)].map((match) => match.index);
const start = names.filter((index) => index <= at).at(-1) ?? 0;
const end = names.find((index) => index > at) ?? job.length;
return job.slice(start, end).match(/^\s+if:\s*(.*?)\s*$/m)?.[1] ?? null;
const step = job.slice(start, end);
const condition = step.match(/^([ \t]*)if:[ \t]*(.*?)[ \t]*\r?$/m);
if (condition === null) return null;
const next = step.slice(condition.index + condition[0].length).split("\n").find((line) => line.trim() !== "");
const continued = next !== undefined && next.match(/^[ \t]*/)[0].length > condition[1].length;
return continued || /^[>|]/.test(condition[2]) ? MULTI_LINE : condition[2];
}
// The suite must run whenever Publish does, so Publish's condition has to imply the suite's. Three
@ -796,10 +806,10 @@ function workflowSuiteProblems(workflow) {
const publish = at(/^\s+- name: Publish\b/m);
if (publish < 0) return;
const [suiteIf, publishIf] = [stepCondition(job, suite), stepCondition(job, publish)];
if ([suiteIf, publishIf].some((condition) => /^[>|]/.test(condition ?? ""))) {
problems.push(`${label} gives the suite or Publish a block-scalar if:, which the pin does not read; write it on one line`);
if ([suiteIf, publishIf].includes(MULTI_LINE)) {
problems.push(`${label} gives the suite or Publish a multi-line if: (a block scalar, or a value continued on the next line), which the pin does not read; write it on one line`);
} else if (STATUS_FUNCTION.test(publishIf ?? "")) {
problems.push(`${label} lets Publish run after the suite fails or is skipped: its if: calls always(), cancelled() or failure()`);
problems.push(`${label} lets Publish run after the suite fails or is skipped: its if: calls always(), cancelled(), failure() or success()`);
} else if (!suiteConditionCovers(suiteIf, publishIf)) {
problems.push(`${label} runs the suite under a condition the pin cannot prove Publish implies; it accepts no suite if:, an equal if:, or Publish's if: as the suite's followed by " && " with no "||"`);
}
@ -853,16 +863,22 @@ test("workflow: every job runs the suite first, with every tag fetched; removing
// a status function in Publish's if: runs it after the suite failed or was skipped
const npmPublishIf = (prefix) => workflow.replace(/(- name: Publish\r?\n\s+if: )(startsWith\(github\.ref, 'refs\/tags\/cultcache-ts-v'\))/, `$1${prefix}$2`);
const status = /lets Publish run after the suite fails or is skipped/;
for (const prefix of ["always() && ", "!cancelled() && ", "cancelled() || ", "failure() && "]) {
for (const prefix of ["always() && ", "!cancelled() && ", "cancelled() || ", "failure() && ", "success() || "]) {
assert.match(workflowSuiteProblems(npmPublishIf(prefix)).join("\n"), status, prefix);
}
assert.match(workflowSuiteProblems(pythonPublishIf((condition) => condition.replace(" && ", " && always() && "))).join("\n"), status);
// a block-scalar if: on either step is refused rather than compared by its indicator
const blockScalar = /gives the suite or Publish a block-scalar if:/;
// a multi-line if: on either step is refused rather than read by its first line: a block scalar...
const multiLine = /gives the suite or Publish a multi-line if:/;
const npmBlockSuite = npmSuiteIf(">-\n false");
assert.match(workflowSuiteProblems(npmBlockSuite.replace(/(- name: Publish\r?\n\s+if: )(startsWith\(github\.ref, 'refs\/tags\/cultcache-ts-v'\))/, "$1>-\n $2")).join("\n"), blockScalar);
assert.match(workflowSuiteProblems(npmSuiteIf("|\n true")).join("\n"), blockScalar);
assert.match(workflowSuiteProblems(npmPublishIf(">-\n ")).join("\n"), blockScalar);
assert.match(workflowSuiteProblems(npmBlockSuite.replace(/(- name: Publish\r?\n\s+if: )(startsWith\(github\.ref, 'refs\/tags\/cultcache-ts-v'\))/, "$1>-\n $2")).join("\n"), multiLine);
assert.match(workflowSuiteProblems(npmSuiteIf("|\n true")).join("\n"), multiLine);
assert.match(workflowSuiteProblems(npmPublishIf(">-\n ")).join("\n"), multiLine);
// ...or a plain or quoted scalar continued on a following line, where always() or "|| true" would hide
const npmPublishTail = (tail) => workflow.replace(/(- name: Publish\r?\n\s+if: startsWith\(github\.ref, 'refs\/tags\/cultcache-ts-v'\))/, `$1${tail}`);
assert.match(workflowSuiteProblems(npmPublishTail(" &&\n always()")).join("\n"), multiLine);
assert.match(workflowSuiteProblems(pythonPublishIf((condition) => `${condition}\n || true`)).join("\n"), multiLine);
assert.match(workflowSuiteProblems(npmPublishIf("\"").replace(/(if: "startsWith\(github\.ref, 'refs\/tags\/cultcache-ts-v'\))/, "$1 &&\n always()\"")).join("\n"), multiLine);
assert.match(workflowSuiteProblems(npmSuiteIf("startsWith(github.ref,\n 'refs/tags/cultcache-ts-v')")).join("\n"), multiLine);
// a suite condition equal to Publish's passes
assert.deepEqual(workflowSuiteProblems(npmSuiteIf("startsWith(github.ref, 'refs/tags/cultcache-ts-v')")), []);
});