mirror of
https://github.com/Sea-Haven-Industries/.github.git
synced 2026-09-30 08:13:12 +00:00
fix(policy): preserve line-specific violation fingerprints
Co-authored-by: Adam Moussa <amoussa1229@users.noreply.github.com>
This commit is contained in:
parent
7399fb6577
commit
9aa67bd806
2 changed files with 21 additions and 22 deletions
21
.github/workflows/callable-pr-policy.yaml
vendored
21
.github/workflows/callable-pr-policy.yaml
vendored
|
|
@ -214,7 +214,7 @@ jobs:
|
|||
|
||||
// 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.
|
||||
// fingerprint even when the violation remains on the same line.
|
||||
function fnv1a32(str) {
|
||||
let h = 2166136261;
|
||||
for (let i = 0; i < str.length; i++) {
|
||||
|
|
@ -514,19 +514,18 @@ 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.
|
||||
// Normalize a validateWorkflowContent error string to a diff fingerprint.
|
||||
// Per-line locations are intentionally preserved so moving a grandfathered
|
||||
// violation to a different execution path is treated as a new violation.
|
||||
// Content-identifying tokens (action ref, expression text) are preserved.
|
||||
function normalizeViolationFingerprint(err) {
|
||||
return err.replace(/:\d+:/, ':');
|
||||
return err;
|
||||
}
|
||||
|
||||
// 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.
|
||||
// Diff head vs base violations using location-preserving 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) {
|
||||
|
|
|
|||
|
|
@ -1507,9 +1507,9 @@ describe('static YAML assertions', () => {
|
|||
|
||||
// ── normalizeViolationFingerprint ────────────────────────────────────────────
|
||||
describe('normalizeViolationFingerprint', () => {
|
||||
it('strips :LINE: from per-line errors', () => {
|
||||
it('preserves :LINE: in 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:');
|
||||
assert.equal(v.normalizeViolationFingerprint(err), err);
|
||||
});
|
||||
|
||||
it('leaves uses: violations unchanged (no line number in error)', () => {
|
||||
|
|
@ -1522,15 +1522,15 @@ describe('normalizeViolationFingerprint', () => {
|
|||
assert.equal(v.normalizeViolationFingerprint(err), err);
|
||||
});
|
||||
|
||||
it('removes only the first :LINE: occurrence', () => {
|
||||
it('does not rewrite line-like message content', () => {
|
||||
const err = 'f.yaml:10: escaped key: something';
|
||||
assert.equal(v.normalizeViolationFingerprint(err), 'f.yaml: escaped key: something');
|
||||
assert.equal(v.normalizeViolationFingerprint(err), err);
|
||||
});
|
||||
|
||||
it('line 10 and line 200 normalize to the same fingerprint', () => {
|
||||
it('line 10 and line 200 remain distinct fingerprints', () => {
|
||||
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);
|
||||
assert.notEqual(a, b);
|
||||
});
|
||||
});
|
||||
|
||||
|
|
@ -1571,10 +1571,10 @@ describe('filterNewViolations', () => {
|
|||
assert.deepEqual(v.filterNewViolations([err], [err]), []);
|
||||
});
|
||||
|
||||
it('line shift ignored — same run expression moved to different line', () => {
|
||||
it('line shift is blocked when the same run expression moves', () => {
|
||||
const head = FP_EXPR('w.yaml', 20);
|
||||
const base = FP_EXPR('w.yaml', 10);
|
||||
assert.deepEqual(v.filterNewViolations([head], [base]), []);
|
||||
assert.deepEqual(v.filterNewViolations([head], [base]), [head]);
|
||||
});
|
||||
|
||||
it('newly added run expression is blocked', () => {
|
||||
|
|
@ -1586,7 +1586,7 @@ describe('filterNewViolations', () => {
|
|||
|
||||
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 head1 = FP_EXPR('w.yaml', 10);
|
||||
const head2 = FP_EXPR('w.yaml', 50);
|
||||
const result = v.filterNewViolations([head1, head2], [base]);
|
||||
assert.equal(result.length, 1);
|
||||
|
|
@ -1694,7 +1694,7 @@ describe('content fingerprint: changed payload is new', () => {
|
|||
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', () => {
|
||||
it('unchanged run expression at a shifted line is blocked', () => {
|
||||
const wfA = BASE_WF('echo ${{ github.ref }}');
|
||||
// wfB inserts a blank step before the run step, shifting its line number
|
||||
const wfB = [
|
||||
|
|
@ -1713,7 +1713,7 @@ describe('content fingerprint: changed payload is new', () => {
|
|||
].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');
|
||||
assert.equal(v.filterNewViolations(headErrs, baseErrs).length, 1, 'same expression on a different line must be reported as new');
|
||||
});
|
||||
|
||||
it('flow-style run changed to flow-style uses is new', () => {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue