test(release): assert workflow injection safety against parsed YAML

The line-based guards allow-listed known-safe spellings, so bracket forms
like ${{ steps.version.outputs['VERSION'] }} never entered the candidate
list and passed, and the ordering check read indexOf over raw text, which
a comment naming the step could satisfy.

Parse the workflows instead and assert on step structure: no template
expression may appear in any run body, every step reading $RELEASE_VERSION
or $TAG_NAME must declare it in env, and the prerelease notes check must
precede the publishing step in the release job's step list.

Verified each guard fails on the three evasions it now covers.
This commit is contained in:
sudacode
2026-08-23 03:45:29 -07:00
parent 7b403bf8ad
commit 615a785703
3 changed files with 101 additions and 40 deletions
+23 -21
View File
@@ -2,9 +2,16 @@ import test from 'node:test';
import assert from 'node:assert/strict'; import assert from 'node:assert/strict';
import { readFileSync } from 'node:fs'; import { readFileSync } from 'node:fs';
import { resolve } from 'node:path'; import { resolve } from 'node:path';
import {
jobSteps,
readWorkflow,
stepsMissingEnvDeclaration,
templateExpressionsInRunBodies,
} from './workflow-test-helpers';
const prereleaseWorkflowPath = resolve(__dirname, '../.github/workflows/prerelease.yml'); const prereleaseWorkflowPath = resolve(__dirname, '../.github/workflows/prerelease.yml');
const prereleaseWorkflow = readFileSync(prereleaseWorkflowPath, 'utf8').replace(/\r\n/g, '\n'); const prereleaseWorkflow = readFileSync(prereleaseWorkflowPath, 'utf8').replace(/\r\n/g, '\n');
const parsedPrereleaseWorkflow = readWorkflow(prereleaseWorkflowPath);
const packageJsonPath = resolve(__dirname, '../package.json'); const packageJsonPath = resolve(__dirname, '../package.json');
const packageJson = JSON.parse(readFileSync(packageJsonPath, 'utf8')) as { const packageJson = JSON.parse(readFileSync(packageJsonPath, 'utf8')) as {
scripts: Record<string, string>; scripts: Record<string, string>;
@@ -128,29 +135,24 @@ test('prerelease workflow rejects committed notes generated for a different beta
packageJson.scripts['changelog:check-prerelease-notes'], packageJson.scripts['changelog:check-prerelease-notes'],
'bun run scripts/build-changelog.ts check-prerelease-notes', 'bun run scripts/build-changelog.ts check-prerelease-notes',
); );
assert.match(
prereleaseWorkflow,
/bun run changelog:check-prerelease-notes --version "\$RELEASE_VERSION"/,
);
// The staleness check has to run before the release is created or edited, const steps = jobSteps(parsedPrereleaseWorkflow, 'release');
// otherwise stale notes are already published by the time it fails. const checkIndex = steps.findIndex((step) =>
const checkIndex = prereleaseWorkflow.indexOf('changelog:check-prerelease-notes'); step.run?.includes('changelog:check-prerelease-notes'),
const createIndex = prereleaseWorkflow.indexOf('gh release create'); );
const editIndex = prereleaseWorkflow.indexOf('gh release edit'); const publishIndex = steps.findIndex((step) => /gh release (create|edit)/.test(step.run ?? ''));
assert.notEqual(checkIndex, -1); assert.notEqual(checkIndex, -1);
assert.ok(checkIndex < createIndex); assert.notEqual(publishIndex, -1);
assert.ok(checkIndex < editIndex); // Stale notes are already published if the check runs after the release.
assert.ok(checkIndex < publishIndex);
assert.match(
steps[checkIndex]!.run!,
/changelog:check-prerelease-notes --version "\$RELEASE_VERSION"/,
);
}); });
// GitHub substitutes ${{ }} into the run script before the shell parses it, so a test('prerelease workflow keeps tag-derived values out of shell bodies', () => {
// tag-derived value used that way is executed as script rather than read as data. assert.deepEqual(templateExpressionsInRunBodies(parsedPrereleaseWorkflow), []);
test('tag-derived values reach shell bodies through env, not template interpolation', () => { assert.deepEqual(stepsMissingEnvDeclaration(parsedPrereleaseWorkflow, 'RELEASE_VERSION'), []);
const rawVersionUses = prereleaseWorkflow
.split('\n')
.filter((line) => line.includes('steps.version.outputs.VERSION'))
.filter(
(line) => !/^\s*RELEASE_VERSION: \$\{\{ steps\.version\.outputs\.VERSION \}\}$/.test(line),
);
assert.deepEqual(rawVersionUses, []);
}); });
+14 -20
View File
@@ -2,11 +2,18 @@ import test from 'node:test';
import assert from 'node:assert/strict'; import assert from 'node:assert/strict';
import { readFileSync } from 'node:fs'; import { readFileSync } from 'node:fs';
import { resolve } from 'node:path'; import { resolve } from 'node:path';
import {
readWorkflow,
stepsMissingEnvDeclaration,
templateExpressionsInRunBodies,
} from './workflow-test-helpers';
const releaseWorkflowPath = resolve(__dirname, '../.github/workflows/release.yml'); const releaseWorkflowPath = resolve(__dirname, '../.github/workflows/release.yml');
const releaseWorkflow = readFileSync(releaseWorkflowPath, 'utf8'); const releaseWorkflow = readFileSync(releaseWorkflowPath, 'utf8');
const docsPagesWorkflowPath = resolve(__dirname, '../.github/workflows/docs-pages.yml'); const docsPagesWorkflowPath = resolve(__dirname, '../.github/workflows/docs-pages.yml');
const docsPagesWorkflow = readFileSync(docsPagesWorkflowPath, 'utf8'); const docsPagesWorkflow = readFileSync(docsPagesWorkflowPath, 'utf8');
const parsedReleaseWorkflow = readWorkflow(releaseWorkflowPath);
const parsedDocsPagesWorkflow = readWorkflow(docsPagesWorkflowPath);
const makefilePath = resolve(__dirname, '../Makefile'); const makefilePath = resolve(__dirname, '../Makefile');
const makefile = readFileSync(makefilePath, 'utf8'); const makefile = readFileSync(makefilePath, 'utf8');
const packageJsonPath = resolve(__dirname, '../package.json'); const packageJsonPath = resolve(__dirname, '../package.json');
@@ -279,26 +286,13 @@ test('Makefile uninstall targets remove bundled runtime plugin app-data copies',
assert.match(makefile, /Removed:[\s\S]*\$\(MACOS_DATA_DIR\)\/plugin\/subminer/); assert.match(makefile, /Removed:[\s\S]*\$\(MACOS_DATA_DIR\)\/plugin\/subminer/);
}); });
// GitHub substitutes ${{ }} into the run script before the shell parses it, so a test('release and docs workflows keep tag-derived values out of shell bodies', () => {
// tag-derived value used that way is executed as script rather than read as data. assert.deepEqual(templateExpressionsInRunBodies(parsedReleaseWorkflow), []);
// The release and docs workflows must route tag values through env and assert.deepEqual(templateExpressionsInRunBodies(parsedDocsPagesWorkflow), []);
// reference them as shell variables. assert.deepEqual(stepsMissingEnvDeclaration(parsedReleaseWorkflow, 'RELEASE_VERSION'), []);
test('tag-derived values reach shell bodies through env, not template interpolation', () => { assert.deepEqual(stepsMissingEnvDeclaration(parsedDocsPagesWorkflow, 'TAG_NAME'), []);
const rawVersionUses = releaseWorkflow
.split('\n')
.filter((line) => line.includes('steps.version.outputs.VERSION'))
.filter(
(line) => !/^\s*RELEASE_VERSION: \$\{\{ steps\.version\.outputs\.VERSION \}\}$/.test(line),
);
assert.deepEqual(rawVersionUses, []);
const rawRefNameUses = docsPagesWorkflow
.split('\n')
.filter((line) => line.includes('github.ref_name'))
.filter((line) => !/^\s*TAG_NAME: \$\{\{ github\.ref_name \}\}$/.test(line))
// `if:` conditions are evaluated by Actions itself, never handed to a shell.
.filter((line) => !/^\s*if:/.test(line));
assert.deepEqual(rawRefNameUses, []);
// The docs tag guard must test the shell variable, not an interpolated value
// that would be substituted into the condition before the shell reads it.
assert.match(docsPagesWorkflow, /if \[\[ ! "\$TAG_NAME" =~/); assert.match(docsPagesWorkflow, /if \[\[ ! "\$TAG_NAME" =~/);
}); });
+65
View File
@@ -0,0 +1,65 @@
import { readFileSync } from 'node:fs';
export type WorkflowStep = {
name?: string;
run?: string;
env?: Record<string, unknown>;
};
export type ParsedWorkflow = {
jobs?: Record<string, { steps?: WorkflowStep[] } | undefined>;
};
// Workflow tests only ever run under `bun test`, which parses YAML natively.
function parseWorkflowYaml(source: string): ParsedWorkflow {
const bunRuntime = globalThis as typeof globalThis & {
Bun?: { YAML?: { parse?: (input: string) => unknown } };
};
const parse = bunRuntime.Bun?.YAML?.parse;
if (!parse) {
throw new Error('Bun.YAML.parse is unavailable; workflow tests must run under bun.');
}
return parse(source) as ParsedWorkflow;
}
export function readWorkflow(workflowPath: string): ParsedWorkflow {
return parseWorkflowYaml(readFileSync(workflowPath, 'utf8'));
}
// Steps of one job, in declaration order. Throws on an unknown job so a renamed
// job fails loudly instead of silently emptying an ordering assertion.
export function jobSteps(workflow: ParsedWorkflow, jobName: string): WorkflowStep[] {
const job = workflow.jobs?.[jobName];
if (!job) {
throw new Error(`Workflow has no job named ${jobName}.`);
}
return job.steps ?? [];
}
function allSteps(workflow: ParsedWorkflow): Array<{ job: string; step: WorkflowStep }> {
return Object.entries(workflow.jobs ?? {}).flatMap(([job, definition]) =>
(definition?.steps ?? []).map((step) => ({ job, step })),
);
}
// GitHub substitutes ${{ }} into a run script before the shell parses it, so any
// value used that way is executed as script rather than read as data. Reporting
// every expression (rather than allow-listing known-safe ones) also covers
// alternate spellings such as ${{ steps.version.outputs['VERSION'] }}.
export function templateExpressionsInRunBodies(workflow: ParsedWorkflow): string[] {
return allSteps(workflow).flatMap(({ job, step }) =>
(typeof step.run === 'string' ? (step.run.match(/\$\{\{[\s\S]*?\}\}/g) ?? []) : []).map(
(expression) => `${job}/${step.name ?? '<unnamed>'}: ${expression}`,
),
);
}
// Steps whose shell body reads $NAME without the step declaring it in env, which
// would silently expand to an empty string at run time.
export function stepsMissingEnvDeclaration(workflow: ParsedWorkflow, name: string): string[] {
const reference = new RegExp(`\\$${name}\\b|\\$\\{${name}\\b`);
return allSteps(workflow)
.filter(({ step }) => typeof step.run === 'string' && reference.test(step.run))
.filter(({ step }) => !Object.prototype.hasOwnProperty.call(step.env ?? {}, name))
.map(({ job, step }) => `${job}/${step.name ?? '<unnamed>'}`);
}