fix(ci): scan copied workflow files

Refs: PLAT-62
This commit is contained in:
Adam Moussa 2026-08-03 19:52:30 -04:00
parent f5925d3fd2
commit 0bc443e766
No known key found for this signature in database
2 changed files with 135 additions and 15 deletions

View file

@ -488,6 +488,28 @@ jobs:
return result; return result;
} }
// Classify the status of a pull-request file for workflow scanning.
// Returns one of four action objects:
// { action: 'skip' } — removed or unchanged; no validation
// { action: 'full' } — added or copied; full validation, no baseline
// { action: 'diff', basePath: string } — modified/changed or renamed-from-workflow;
// validate head, diff against base at basePath
// { action: 'infra', reason: string } — unknown status; report POLICY-INFRA
// isWfFn must be the isWorkflowFilename predicate (injectable for testing).
function classifyFileStatus(file, isWfFn) {
const s = file.status;
if (s === 'removed' || s === 'unchanged') return { action: 'skip' };
if (s === 'added' || s === 'copied') return { action: 'full' };
if (s === 'modified' || s === 'changed') return { action: 'diff', basePath: file.filename };
if (s === 'renamed') {
if (file.previous_filename && isWfFn(file.previous_filename)) {
return { action: 'diff', basePath: file.previous_filename };
}
return { action: 'full' };
}
return { action: 'infra', reason: 'unknown file status "' + s + '" for ' + file.filename };
}
// ── Test escape ────────────────────────────────────────────────────────── // ── Test escape ──────────────────────────────────────────────────────────
// Set PR_POLICY_TEST=1 to extract pure functions without hitting any API. // Set PR_POLICY_TEST=1 to extract pure functions without hitting any API.
if (process.env.PR_POLICY_TEST === '1') { if (process.env.PR_POLICY_TEST === '1') {
@ -507,6 +529,7 @@ jobs:
filterNewViolations, filterNewViolations,
fnv1a32, fnv1a32,
stripHash, stripHash,
classifyFileStatus,
}; };
} }
@ -724,7 +747,13 @@ jobs:
if (!filesLink.includes('rel="next"') || filesResp.data.length === 0) filesMore = false; if (!filesLink.includes('rel="next"') || filesResp.data.length === 0) filesMore = false;
for (const file of filesResp.data) { for (const file of filesResp.data) {
if (!isWorkflowFilename(file.filename)) continue; if (!isWorkflowFilename(file.filename)) continue;
if (file.status !== 'added' && file.status !== 'modified' && file.status !== 'renamed') continue;
const cls = classifyFileStatus(file, isWorkflowFilename);
if (cls.action === 'skip') continue;
if (cls.action === 'infra') {
addInfra('POLICY-INFRA: ' + cls.reason + '; skipping workflow validation');
continue;
}
// Fetch HEAD content via blob SHA. // Fetch HEAD content via blob SHA.
let headContent; let headContent;
@ -740,23 +769,15 @@ jobs:
const headErrs = validateWorkflowContent(headContent, file.filename); const headErrs = validateWorkflowContent(headContent, file.filename);
// A rename from a non-workflow path into a workflow directory is if (cls.action === 'full') {
// treated as an addition — the previous file was not a workflow, so // No baseline — added, copied, or renamed-from-non-workflow.
// 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); for (const e of headErrs) addViolation(e);
} else { } else {
// modified or renamed-from-workflow — fetch base and report only new violations. // diff — modified, changed, or renamed-from-workflow.
// Renamed files are fetched at previous_filename; both head and base // Both head and base violations use file.filename so fingerprints match.
// violations use file.filename so fingerprints are comparable.
const basePath = isRenameFromWorkflow ? file.previous_filename : file.filename;
let baseErrs = []; let baseErrs = [];
try { try {
const baseResp = await github.rest.repos.getContent({ owner: repoOwner, repo: repoName, path: basePath, ref: pr.base.sha }); const baseResp = await github.rest.repos.getContent({ owner: repoOwner, repo: repoName, path: cls.basePath, ref: pr.base.sha });
const baseRaw = baseResp.data; const baseRaw = baseResp.data;
const baseEnc = baseRaw.encoding === 'base64' ? 'base64' : 'utf8'; const baseEnc = baseRaw.encoding === 'base64' ? 'base64' : 'utf8';
const baseContent = Buffer.from(baseRaw.content, baseEnc).toString('utf8'); const baseContent = Buffer.from(baseRaw.content, baseEnc).toString('utf8');

View file

@ -95,7 +95,7 @@ async function loadValidators() {
'validateCommitSubject', 'detectAiFooter', 'isWorkflowFilename', 'validateCommitSubject', 'detectAiFooter', 'isWorkflowFilename',
'validateWorkflowContent', 'parseRetryAfterMs', 'checkCommitLimit', 'validateWorkflowContent', 'parseRetryAfterMs', 'checkCommitLimit',
'checkFilesLimit', 'normalizeViolationFingerprint', 'filterNewViolations', 'checkFilesLimit', 'normalizeViolationFingerprint', 'filterNewViolations',
'fnv1a32', 'stripHash', 'fnv1a32', 'stripHash', 'classifyFileStatus',
]) { ]) {
assert.equal(typeof v[fn], 'function', fn + ' must be exported'); assert.equal(typeof v[fn], 'function', fn + ' must be exported');
} }
@ -1735,3 +1735,102 @@ describe('isWorkflowFilename: rename routing', () => {
assert.deepEqual(v.filterNewViolations(headErrs, []), headErrs); assert.deepEqual(v.filterNewViolations(headErrs, []), headErrs);
}); });
}); });
// ── classifyFileStatus ────────────────────────────────────────────────────────
describe('classifyFileStatus', () => {
function isWf(f) { return v.isWorkflowFilename(f); }
const wfFile = '.github/workflows/deploy.yaml';
const nonWfFile = 'scripts/setup.yaml';
function file(status, filename, previous_filename) {
return { status, filename: filename || wfFile, previous_filename };
}
it('removed → skip', () => {
assert.deepEqual(v.classifyFileStatus(file('removed'), isWf), { action: 'skip' });
});
it('unchanged → skip', () => {
assert.deepEqual(v.classifyFileStatus(file('unchanged'), isWf), { action: 'skip' });
});
it('added → full', () => {
assert.deepEqual(v.classifyFileStatus(file('added'), isWf), { action: 'full' });
});
it('copied → full (even with previous_filename)', () => {
assert.deepEqual(v.classifyFileStatus(file('copied', wfFile, wfFile), isWf), { action: 'full' });
});
it('modified → diff with same filename as basePath', () => {
assert.deepEqual(v.classifyFileStatus(file('modified'), isWf), { action: 'diff', basePath: wfFile });
});
it('changed → diff with same filename as basePath', () => {
assert.deepEqual(v.classifyFileStatus(file('changed'), isWf), { action: 'diff', basePath: wfFile });
});
it('renamed from workflow path → diff with previous_filename as basePath', () => {
const prev = '.github/workflows/old.yaml';
assert.deepEqual(v.classifyFileStatus(file('renamed', wfFile, prev), isWf), { action: 'diff', basePath: prev });
});
it('renamed from non-workflow path → full (treat as added)', () => {
assert.deepEqual(v.classifyFileStatus(file('renamed', wfFile, nonWfFile), isWf), { action: 'full' });
});
it('renamed with no previous_filename → full', () => {
assert.deepEqual(v.classifyFileStatus(file('renamed', wfFile, undefined), isWf), { action: 'full' });
});
it('unknown status → infra with reason string', () => {
const result = v.classifyFileStatus(file('bogus'), isWf);
assert.equal(result.action, 'infra');
assert.ok(result.reason.includes('bogus'), 'reason must name the unknown status: ' + result.reason);
assert.ok(result.reason.includes(wfFile), 'reason must include filename: ' + result.reason);
});
it('copied noncompliant workflow gets full validation (no baseline)', () => {
// Simulate: copied file has violations in head, previous_filename also exists.
// classifyFileStatus returns full, so filterNewViolations is called with empty base.
const noncompliantWf = [
'on:',
' workflow_call:',
'jobs:',
' j:',
' runs-on: ubuntu-latest',
' steps:',
' - uses: actions/checkout@v4',
].join('\n');
const cls = v.classifyFileStatus({ status: 'copied', filename: wfFile, previous_filename: wfFile }, isWf);
assert.equal(cls.action, 'full', 'copied must be full');
const headErrs = v.validateWorkflowContent(noncompliantWf, wfFile);
assert.ok(headErrs.length > 0, 'noncompliant file must have violations');
assert.deepEqual(v.filterNewViolations(headErrs, []), headErrs, 'all violations reported with empty base');
});
it('copied compliant workflow produces no violations', () => {
const PIN = 'actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b3 # v7.0.1';
const compliantWf = [
'on:',
' workflow_call:',
'permissions:',
' contents: read',
'jobs:',
' j:',
' runs-on: ubuntu-latest',
' steps:',
' - uses: ' + PIN,
].join('\n');
const cls = v.classifyFileStatus({ status: 'copied', filename: wfFile }, isWf);
assert.equal(cls.action, 'full');
assert.deepEqual(v.validateWorkflowContent(compliantWf, wfFile), []);
});
it('unknown status result reason is usable as POLICY-INFRA message', () => {
const result = v.classifyFileStatus(file('merge'), isWf);
assert.equal(result.action, 'infra');
const infra = 'POLICY-INFRA: ' + result.reason + '; skipping workflow validation';
assert.ok(infra.includes('POLICY-INFRA'), 'infra message must have prefix: ' + infra);
});
});