diff --git a/.github/workflows/callable-pr-policy.yaml b/.github/workflows/callable-pr-policy.yaml index aefe26b..35dde29 100644 --- a/.github/workflows/callable-pr-policy.yaml +++ b/.github/workflows/callable-pr-policy.yaml @@ -488,6 +488,28 @@ jobs: 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 ────────────────────────────────────────────────────────── // Set PR_POLICY_TEST=1 to extract pure functions without hitting any API. if (process.env.PR_POLICY_TEST === '1') { @@ -507,6 +529,7 @@ jobs: filterNewViolations, fnv1a32, stripHash, + classifyFileStatus, }; } @@ -724,7 +747,13 @@ jobs: if (!filesLink.includes('rel="next"') || filesResp.data.length === 0) filesMore = false; for (const file of filesResp.data) { 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. let headContent; @@ -740,23 +769,15 @@ jobs: 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. + if (cls.action === 'full') { + // No baseline — added, copied, or renamed-from-non-workflow. 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; + // diff — modified, changed, or renamed-from-workflow. + // Both head and base violations use file.filename so fingerprints match. let baseErrs = []; 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 baseEnc = baseRaw.encoding === 'base64' ? 'base64' : 'utf8'; const baseContent = Buffer.from(baseRaw.content, baseEnc).toString('utf8'); diff --git a/test/pr-policy.test.mjs b/test/pr-policy.test.mjs index c6f4247..de4636d 100644 --- a/test/pr-policy.test.mjs +++ b/test/pr-policy.test.mjs @@ -95,7 +95,7 @@ async function loadValidators() { 'validateCommitSubject', 'detectAiFooter', 'isWorkflowFilename', 'validateWorkflowContent', 'parseRetryAfterMs', 'checkCommitLimit', 'checkFilesLimit', 'normalizeViolationFingerprint', 'filterNewViolations', - 'fnv1a32', 'stripHash', + 'fnv1a32', 'stripHash', 'classifyFileStatus', ]) { assert.equal(typeof v[fn], 'function', fn + ' must be exported'); } @@ -1735,3 +1735,102 @@ describe('isWorkflowFilename: rename routing', () => { 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); + }); +});