fix(ci): grandfather unchanged workflow policy debt

Refs: PLAT-62
This commit is contained in:
Adam Moussa 2026-08-03 19:32:18 -04:00
parent 5954557ef0
commit d3daa9579e
No known key found for this signature in database
3 changed files with 362 additions and 16 deletions

View file

@ -194,6 +194,23 @@ jobs:
/^workflow-templates\/[^/]+\.ya?ml$/.test(filename);
}
// FNV-1a 32-bit hash of a string — used to produce content fingerprints
// for line-specific violations so changing the payload changes the
// fingerprint while shifting the same payload to a different line does not.
function fnv1a32(str) {
let h = 2166136261;
for (let i = 0; i < str.length; i++) {
h = Math.imul(h ^ str.charCodeAt(i), 16777619) >>> 0;
}
return h.toString(16).padStart(8, '0');
}
// Strip the trailing [fnv:XXXXXXXX] content-hash token from a violation
// string before emitting it to users. The hash is purely internal.
function stripHash(msg) {
return msg.replace(/ \[fnv:[0-9a-f]{8}\]$/, '');
}
// Deterministic line scanner for workflow YAML content.
//
// Block-scalar tracking: any YAML key whose value begins with | or >
@ -250,7 +267,7 @@ jobs:
// Content of block scalar.
// Only flag expression injection for run: block scalars.
if (blockIsRun && line.includes(EXPR_OPEN)) {
errs.push(filename + ':' + (i + 1) + ': run: block contains ' + EXPR_OPEN + ' }} — expressions must go through env:');
errs.push(filename + ':' + (i + 1) + ': run: block contains ' + EXPR_OPEN + ' }} — expressions must go through env: [fnv:' + fnv1a32(line.trim()) + ']');
}
continue;
}
@ -266,7 +283,7 @@ jobs:
// the line scanner (e.g. "u\u0073es" parses as "uses" in YAML).
// Reject any such key at the structural position.
if (/^[ \t]*(?:-[ \t]+)?"[^"]*\\[^"]*":/.test(line)) {
errs.push(filename + ':' + (i + 1) + ': escaped key in double-quoted string is not supported — use literal key names (run:, uses:, permissions:)');
errs.push(filename + ':' + (i + 1) + ': escaped key in double-quoted string is not supported — use literal key names (run:, uses:, permissions:) [fnv:' + fnv1a32(line.trim()) + ']');
continue;
}
@ -281,7 +298,7 @@ jobs:
const braceIdx = line.indexOf('{');
const flowContent = line.slice(braceIdx);
if (/[{,]\s*(?:"uses"|'uses'|uses|"run"|'run'|run|"[^"]*\\[^"]*")\s*:/.test(flowContent)) {
errs.push(filename + ':' + (i + 1) + ': flow-style step mapping with structural "run:" or "uses:" key is not supported — use block mapping style');
errs.push(filename + ':' + (i + 1) + ': flow-style step mapping with structural "run:" or "uses:" key is not supported — use block mapping style [fnv:' + fnv1a32(line.trim()) + ']');
}
continue;
}
@ -313,11 +330,11 @@ jobs:
// A YAML alias is *name; an anchor is &name (non-whitespace after &).
// Ordinary shell & like 'echo "R&D build"' does not start with * or &word.
if (runVal[0] === '*' || /^&\S/.test(runVal)) {
errs.push(filename + ':' + (i + 1) + ': YAML alias/anchor in "run:" value is not supported — inline the run script');
errs.push(filename + ':' + (i + 1) + ': YAML alias/anchor in "run:" value is not supported — inline the run script [fnv:' + fnv1a32(line.trim()) + ']');
continue;
}
if (inlineRunM[1].includes(EXPR_OPEN)) {
errs.push(filename + ':' + (i + 1) + ': run: value contains ' + EXPR_OPEN + ' }} — expressions must go through env:');
errs.push(filename + ':' + (i + 1) + ': run: value contains ' + EXPR_OPEN + ' }} — expressions must go through env: [fnv:' + fnv1a32(line.trim()) + ']');
}
continue;
}
@ -333,7 +350,7 @@ jobs:
// Reject YAML alias/anchor in uses: value.
// An alias is *name; an anchor is &name (non-whitespace after &).
if (rawVal[0] === '*' || /^&\S/.test(rawVal)) {
errs.push(filename + ':' + (i + 1) + ': YAML alias/anchor in "uses:" value is not supported — inline the action ref');
errs.push(filename + ':' + (i + 1) + ': YAML alias/anchor in "uses:" value is not supported — inline the action ref [fnv:' + fnv1a32(line.trim()) + ']');
continue;
}
@ -414,6 +431,39 @@ jobs:
return null;
}
// Normalize a validateWorkflowContent error string to a line-agnostic
// fingerprint. Removes the `:LINE:` segment produced by per-line errors
// so that a violation shifted to a different line in the head file does
// not appear new relative to the base. Content-identifying tokens (action
// ref, expression text) are preserved.
function normalizeViolationFingerprint(err) {
return err.replace(/:\d+:/, ':');
}
// Diff head vs base violations using normalized fingerprints with
// multiplicity. For each fingerprint, up to base-count head violations
// of that fingerprint are considered pre-existing; the remainder are new.
// Violations are returned in head-file order.
function filterNewViolations(headErrs, baseErrs) {
const baseCounts = new Map();
for (const e of baseErrs) {
const fp = normalizeViolationFingerprint(e);
baseCounts.set(fp, (baseCounts.get(fp) || 0) + 1);
}
const remaining = new Map(baseCounts);
const result = [];
for (const e of headErrs) {
const fp = normalizeViolationFingerprint(e);
const rem = remaining.get(fp) || 0;
if (rem > 0) {
remaining.set(fp, rem - 1);
} else {
result.push(e);
}
}
return result;
}
// ── Test escape ──────────────────────────────────────────────────────────
// Set PR_POLICY_TEST=1 to extract pure functions without hitting any API.
if (process.env.PR_POLICY_TEST === '1') {
@ -429,6 +479,10 @@ jobs:
parseRetryAfterMs,
checkCommitLimit,
checkFilesLimit,
normalizeViolationFingerprint,
filterNewViolations,
fnv1a32,
stripHash,
};
}
@ -624,10 +678,14 @@ jobs:
}
}
// 8 — workflow file supply-chain checks
// GitHub REST API caps listFiles at 3000. Fail infra immediately when
// pr.changed_files exceeds that limit; compare fetched count to detect
// truncation at lower file counts.
// 8 — workflow file supply-chain checks (diff-mode)
// Only NEW violations relative to the base branch are reported.
// Added files have no baseline and must be fully compliant.
// Modified/renamed files are diffed: base content is fetched at
// pr.base.sha (using previous_filename for renames). Base fetch
// failures are POLICY-INFRA — partial history is never silently
// grandfathered. Deleted files are skipped.
// GitHub REST API caps listFiles at 3000.
try {
const filesLimitErr = checkFilesLimit(pr.changed_files);
if (filesLimitErr) addInfra(filesLimitErr);
@ -643,14 +701,47 @@ jobs:
for (const file of filesResp.data) {
if (!isWorkflowFilename(file.filename)) continue;
if (file.status !== 'added' && file.status !== 'modified' && file.status !== 'renamed') continue;
// Fetch HEAD content via blob SHA.
let headContent;
try {
const blobResp = await github.rest.git.getBlob({ owner: repoOwner, repo: repoName, file_sha: file.sha });
const raw = blobResp.data;
const encoding = raw.encoding === 'base64' ? 'base64' : 'utf8';
const fileContent = Buffer.from(raw.content, encoding).toString('utf8');
for (const e of validateWorkflowContent(fileContent, file.filename)) addViolation(e);
const enc = raw.encoding === 'base64' ? 'base64' : 'utf8';
headContent = Buffer.from(raw.content, enc).toString('utf8');
} catch (blobErr) {
addInfra('POLICY-INFRA: Cannot fetch blob for ' + file.filename + ': ' + blobErr.message);
continue;
}
const headErrs = validateWorkflowContent(headContent, file.filename);
// A rename from a non-workflow path into a workflow directory is
// treated as an addition — the previous file was not a workflow, so
// there is no valid baseline to compare against. Only rename FROM
// an existing workflow file uses baseline diffing.
const isRenameFromWorkflow = file.status === 'renamed' && isWorkflowFilename(file.previous_filename);
if (file.status === 'added' || (file.status === 'renamed' && !isRenameFromWorkflow)) {
// No baseline exists — every violation is new.
for (const e of headErrs) addViolation(e);
} else {
// modified or renamed-from-workflow — fetch base and report only new violations.
// Renamed files are fetched at previous_filename; both head and base
// violations use file.filename so fingerprints are comparable.
const basePath = isRenameFromWorkflow ? file.previous_filename : file.filename;
let baseErrs = [];
try {
const baseResp = await github.rest.repos.getContent({ owner: repoOwner, repo: repoName, path: basePath, ref: pr.base.sha });
const baseRaw = baseResp.data;
const baseEnc = baseRaw.encoding === 'base64' ? 'base64' : 'utf8';
const baseContent = Buffer.from(baseRaw.content, baseEnc).toString('utf8');
baseErrs = validateWorkflowContent(baseContent, file.filename);
} catch (baseErr) {
addInfra('POLICY-INFRA: Cannot fetch base content for ' + file.filename + ' at ' + pr.base.sha + ': ' + baseErr.message);
continue;
}
for (const e of filterNewViolations(headErrs, baseErrs)) addViolation(e);
}
}
filesPage++;
@ -666,7 +757,7 @@ jobs:
// Both annotations and summary entries are capped at MAX_ANNOTATIONS
// to prevent oversized outputs on PRs with many violations.
const annotated = violations.slice(0, MAX_ANNOTATIONS);
for (const msg of annotated) core.error(msg);
for (const msg of annotated) core.error(stripHash(msg));
for (const msg of infraCodes.slice(0, MAX_ANNOTATIONS)) core.error(msg);
if (violations.length > MAX_ANNOTATIONS) {
core.warning((violations.length - MAX_ANNOTATIONS) + ' additional violation(s) suppressed (max ' + MAX_ANNOTATIONS + ' annotations)');
@ -677,7 +768,7 @@ jobs:
if (violations.length > 0) {
summaryParts.push('', '### Policy violations');
const shownV = violations.slice(0, MAX_ANNOTATIONS);
for (const msg of shownV) summaryParts.push('- ' + msg);
for (const msg of shownV) summaryParts.push('- ' + stripHash(msg));
if (violations.length > MAX_ANNOTATIONS) {
summaryParts.push('- _...and ' + (violations.length - MAX_ANNOTATIONS) + ' more violation(s) not shown_');
}

View file

@ -52,6 +52,8 @@ The two sanctioned deploy paths are merge to `main` triggering the pipeline and
**`.github/workflows/callable-pr-policy.yaml`** — Reusable PR metadata gate. Validates PR title convention (type/scope/Jira key), branch naming, four-section body, commit subjects, AI attribution footers, and workflow file pin compliance — all via GitHub API, no checkout. Emits `policy / pr` when the caller job is named `policy`. Optional secrets `JIRA_CLOUD_ID`, `JIRA_SERVICE_ACCOUNT_EMAIL`, and `JIRA_API_TOKEN` must all be set for human PRs; Dependabot skips Jira/branch/body but still runs commit and workflow supply-chain checks. Emergency `revert` PRs may skip Jira with the `emergency-revert` label applied by a human collaborator with `maintain` or `admin` permission.
The supply-chain check operates in **diff mode**: for modified or renamed workflow files, the gate fetches the base-branch version at `pr.base.sha` and reports only violations whose normalized fingerprint is absent from the base. Added files must be fully compliant. Historical drift already present in the base branch is handled by the drift audit/remediation backlog, not by this gate. A failure to fetch the base version is a `POLICY-INFRA` error and the file is not silently grandfathered.
> **Workflow file constraints enforced by the supply-chain scanner.** Changed workflow files scanned by this policy must use block-style structural keys and inline `run:`/`uses:` values. The scanner fails closed on YAML forms it cannot safely resolve: flow-style step mappings (`- { uses: ... }`, `- { run: ... }`), escaped or Unicode-encoded structural keys in double-quoted strings (`"u\u0073es"`, `"r\u0075n"`), and YAML aliases or anchors on `run:`, `uses:`, or `permissions:` values (`run: *cmd`, `uses: &anchor ...`). Use the literal unquoted key forms and inline values in all workflow steps.
**`.github/workflows/callable-labeler.yaml`** — Org-wide PR auto-labeler. Label rules live inline here (single source of truth) — consumer repos need only a thin caller with `contents: read`, `pull-requests: write`, and `issues: write`; no per-repo labeler.yml.

View file

@ -94,7 +94,8 @@ async function loadValidators() {
'validateTitle', 'getJiraKey', 'validateBranch', 'validateBody',
'validateCommitSubject', 'detectAiFooter', 'isWorkflowFilename',
'validateWorkflowContent', 'parseRetryAfterMs', 'checkCommitLimit',
'checkFilesLimit',
'checkFilesLimit', 'normalizeViolationFingerprint', 'filterNewViolations',
'fnv1a32', 'stripHash',
]) {
assert.equal(typeof v[fn], 'function', fn + ' must be exported');
}
@ -1355,3 +1356,255 @@ describe('static YAML assertions', () => {
);
});
});
// ── normalizeViolationFingerprint ────────────────────────────────────────────
describe('normalizeViolationFingerprint', () => {
it('strips :LINE: from per-line errors', () => {
const err = 'f.yaml:42: run: block contains expression — expressions must go through env:';
assert.equal(v.normalizeViolationFingerprint(err), 'f.yaml: run: block contains expression — expressions must go through env:');
});
it('leaves uses: violations unchanged (no line number in error)', () => {
const err = 'f.yaml: "uses: actions/checkout@v4" must be pinned to a 40-char SHA with "# vX.Y.Z" comment';
assert.equal(v.normalizeViolationFingerprint(err), err);
});
it('leaves missing-permissions unchanged (no line number)', () => {
const err = 'f.yaml: missing top-level "permissions:" key';
assert.equal(v.normalizeViolationFingerprint(err), err);
});
it('removes only the first :LINE: occurrence', () => {
const err = 'f.yaml:10: escaped key: something';
assert.equal(v.normalizeViolationFingerprint(err), 'f.yaml: escaped key: something');
});
it('line 10 and line 200 normalize to the same fingerprint', () => {
const a = v.normalizeViolationFingerprint('f.yaml:10: run: block contains expression');
const b = v.normalizeViolationFingerprint('f.yaml:200: run: block contains expression');
assert.equal(a, b);
});
});
// ── filterNewViolations ───────────────────────────────────────────────────────
describe('filterNewViolations', () => {
const FP_UNPIN = (f, ref) => `${f}: "uses: ${ref}" must be pinned to a 40-char SHA with "# vX.Y.Z" comment`;
const FP_PERM = (f) => `${f}: missing top-level "permissions:" key`;
const FP_EXPR = (f, ln) => `${f}:${ln}: run: block contains expression — expressions must go through env:`;
it('unchanged floating action is ignored', () => {
const err = FP_UNPIN('w.yaml', 'actions/checkout@v4');
assert.deepEqual(v.filterNewViolations([err], [err]), []);
});
it('changed floating ref is treated as new', () => {
const head = FP_UNPIN('w.yaml', 'actions/setup-node@v4');
const base = FP_UNPIN('w.yaml', 'actions/checkout@v4');
assert.deepEqual(v.filterNewViolations([head], [base]), [head]);
});
it('new floating action (no base violation) is blocked', () => {
const err = FP_UNPIN('w.yaml', 'actions/checkout@v4');
assert.deepEqual(v.filterNewViolations([err], []), [err]);
});
it('unchanged missing permissions is ignored', () => {
const err = FP_PERM('w.yaml');
assert.deepEqual(v.filterNewViolations([err], [err]), []);
});
it('added file missing permissions is blocked (empty base)', () => {
const err = FP_PERM('w.yaml');
assert.deepEqual(v.filterNewViolations([err], []), [err]);
});
it('unchanged run expression at same line is ignored', () => {
const err = FP_EXPR('w.yaml', 10);
assert.deepEqual(v.filterNewViolations([err], [err]), []);
});
it('line shift ignored — same run expression moved to different line', () => {
const head = FP_EXPR('w.yaml', 20);
const base = FP_EXPR('w.yaml', 10);
assert.deepEqual(v.filterNewViolations([head], [base]), []);
});
it('newly added run expression is blocked', () => {
const e1 = FP_EXPR('w.yaml', 10);
const e2 = FP_EXPR('w.yaml', 20);
const result = v.filterNewViolations([e1, e2], [e1]);
assert.equal(result.length, 1);
});
it('unchanged run expression ignored; a second new one blocked', () => {
const base = FP_EXPR('w.yaml', 10);
const head1 = FP_EXPR('w.yaml', 12);
const head2 = FP_EXPR('w.yaml', 50);
const result = v.filterNewViolations([head1, head2], [base]);
assert.equal(result.length, 1);
assert.equal(result[0], head2);
});
it('multiple violation types: unchanged ones are ignored', () => {
const perm = FP_PERM('w.yaml');
const unpin = FP_UNPIN('w.yaml', 'actions/checkout@v4');
const newUnpin = FP_UNPIN('w.yaml', 'actions/upload-artifact@v4');
const result = v.filterNewViolations([perm, unpin, newUnpin], [perm, unpin]);
assert.deepEqual(result, [newUnpin]);
});
it('renamed file: fingerprints use head filename for both sides', () => {
const headV = FP_PERM('new-name.yaml');
const baseV = FP_PERM('new-name.yaml');
assert.deepEqual(v.filterNewViolations([headV], [baseV]), []);
});
it('returns empty array when head has no violations', () => {
const base = FP_UNPIN('w.yaml', 'actions/checkout@v4');
assert.deepEqual(v.filterNewViolations([], [base]), []);
});
it('returns all head violations when base is empty (added file)', () => {
const v1 = FP_PERM('w.yaml');
const v2 = FP_UNPIN('w.yaml', 'actions/checkout@v4');
assert.deepEqual(v.filterNewViolations([v1, v2], []), [v1, v2]);
});
});
// ── fnv1a32 ───────────────────────────────────────────────────────────────────
describe('fnv1a32', () => {
it('returns 8 hex chars', () => {
assert.match(v.fnv1a32('hello'), /^[0-9a-f]{8}$/);
});
it('is deterministic', () => {
assert.equal(v.fnv1a32('run: echo hi'), v.fnv1a32('run: echo hi'));
});
it('different strings produce different hashes', () => {
assert.notEqual(v.fnv1a32('run: echo ${{ github.ref }}'), v.fnv1a32('run: echo ${{ github.event.pull_request.title }}'));
});
it('empty string returns known value', () => {
assert.equal(v.fnv1a32(''), '811c9dc5');
});
});
// ── stripHash ─────────────────────────────────────────────────────────────────
describe('stripHash', () => {
it('strips [fnv:XXXXXXXX] at end of message', () => {
assert.equal(
v.stripHash('f.yaml:42: run: block contains ${{ }} [fnv:a1b2c3d4]'),
'f.yaml:42: run: block contains ${{ }}'
);
});
it('is a no-op when no hash suffix present', () => {
const msg = 'f.yaml: missing top-level "permissions:" key';
assert.equal(v.stripHash(msg), msg);
});
it('does not strip [fnv:...] appearing in the middle of a message', () => {
const msg = 'f.yaml: some [fnv:a1b2c3d4] middle text';
assert.equal(v.stripHash(msg), msg);
});
});
// ── Content fingerprint integration (Finding 1) ───────────────────────────────
describe('content fingerprint: changed payload is new', () => {
const PIN = 'actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b3 # v7.0.1';
const BASE_WF = (runLine) => [
'on:',
' workflow_call:',
'permissions:',
' contents: read',
'jobs:',
' j:',
' runs-on: ubuntu-latest',
' steps:',
' - uses: ' + PIN,
' - run: ' + runLine,
].join('\n');
it('changed run expression is treated as new (block scalar)', () => {
const wfRef = [
'on:',
' workflow_call:',
'permissions:',
' contents: read',
'jobs:',
' j:',
' runs-on: ubuntu-latest',
' steps:',
' - uses: ' + PIN,
' - run: |',
' echo ${{ github.ref }}',
].join('\n');
const wfTitle = wfRef.replace('echo ${{ github.ref }}', 'echo ${{ github.event.pull_request.title }}');
const headErrs = v.validateWorkflowContent(wfTitle, 'f.yaml');
const baseErrs = v.validateWorkflowContent(wfRef, 'f.yaml');
assert.equal(v.filterNewViolations(headErrs, baseErrs).length, 1, 'changed expression must be reported as new');
});
it('unchanged run expression at a shifted line is grandfathered', () => {
const wfA = BASE_WF('echo ${{ github.ref }}');
// wfB inserts a blank step before the run step, shifting its line number
const wfB = [
'on:',
' workflow_call:',
'permissions:',
' contents: read',
'jobs:',
' j:',
' runs-on: ubuntu-latest',
' steps:',
' - uses: ' + PIN,
' - name: placeholder',
' run: echo noop',
' - run: echo ${{ github.ref }}',
].join('\n');
const headErrs = v.validateWorkflowContent(wfB, 'f.yaml');
const baseErrs = v.validateWorkflowContent(wfA, 'f.yaml');
assert.deepEqual(v.filterNewViolations(headErrs, baseErrs), [], 'same expression on a different line must be grandfathered');
});
it('flow-style run changed to flow-style uses is new', () => {
const wfRun = 'permissions: {}\n - { run: echo hi }';
const wfUses = 'permissions: {}\n - { uses: actions/checkout@v4 }';
const headErrs = v.validateWorkflowContent(wfUses, 'f.yaml');
const baseErrs = v.validateWorkflowContent(wfRun, 'f.yaml');
assert.equal(v.filterNewViolations(headErrs, baseErrs).length, 1, 'changed flow mapping must be reported as new');
});
});
// ── Renamed from non-workflow path (Finding 2) ───────────────────────────────
describe('isWorkflowFilename: rename routing', () => {
it('non-workflow source path is not a workflow filename', () => {
assert.equal(v.isWorkflowFilename('scripts/deploy.yaml'), false);
assert.equal(v.isWorkflowFilename('infra/template.yml'), false);
assert.equal(v.isWorkflowFilename('.github/deploy.yaml'), false);
});
it('workflow target paths are workflow filenames', () => {
assert.equal(v.isWorkflowFilename('.github/workflows/deploy.yaml'), true);
assert.equal(v.isWorkflowFilename('workflow-templates/ci.yml'), true);
});
it('rename from non-workflow treated as added: filterNewViolations with empty base captures all', () => {
// When previous_filename is not a workflow file, the runtime uses [] as baseErrs.
// This test verifies that all head violations are surfaced (same as added file).
const noncompliantWf = [
'on:',
' workflow_call:',
'jobs:',
' j:',
' runs-on: ubuntu-latest',
' steps:',
' - uses: actions/checkout@v4',
].join('\n');
const headErrs = v.validateWorkflowContent(noncompliantWf, '.github/workflows/new.yaml');
assert.ok(headErrs.length > 0, 'noncompliant file must have violations');
// With empty base (simulating rename from non-workflow), all violations are new
assert.deepEqual(v.filterNewViolations(headErrs, []), headErrs);
});
});