Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .act-replies-44.tsv
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
PRRT_kwDOTyoI9s6YxjZz Fixed — `verifyPolicyAgainstGraph` now normalizes `checks/` prefixes, so bare check ids like `typecheck` match steps like `checks/typecheck` and align with `evaluatePolicy`.
PRRT_kwDOTyoI9s6Yxmn2 Fixed — the rule iteration now guards `checkIds` against `null`, `undefined`, and non-array values, treating them as no check filter.
PRRT_kwDOTyoI9s6Yxn9P Fixed — bare check ids are canonical; `verifyPolicyAgainstGraph` and `evaluatePolicy` normalize `checks/` prefixes and `evaluatePolicy` matches rule-qualified finding ids (`<checkId>:<ruleId>`). `extractFindings` also strips `checks/` before normalization.
PRRT_kwDOTyoI9s6Yxn9R Fixed — `verifyPolicyAgainstGraph` now validates `policy.failOn` and `graph.project.pipelines` and returns structural errors in `PolicyVerification.errors` without throwing.
PRRT_kwDOTyoI9s6Y3M-8 Fixed — `specs/15-findings/spec.md` now declares `compareBaseline(findings, baseline)` matching the implementation and tests.
PRRT_kwDOTyoI9s6Y3M_A Fixed — `specs/16-policy/spec.md` now defines that `failOn[].checkIds` refer to check steps (IDs starting with `checks/`), with normalization for `checks/` prefix and rule-qualified findings; implementation and tests are aligned.
1 change: 1 addition & 0 deletions bun.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions packages/checks/src/__tests__/extract.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,16 @@ describe("extractFindings — SARIF", () => {
expect(findings.length).toBeGreaterThan(0);
expect(findings[0]!.checkId).toContain("mycheck");
});

it("strips checks/ prefix from checkId when extracting", async () => {
const dir = makeDir();
writeFileSync(join(dir, "out.sarif"), JSON.stringify(sampleSarif()));
const outputs: CheckOutput[] = [{ path: "out.sarif", format: "sarif" }];
const findings = await extractFindings(outputs, dir, "checks/mycheck");
expect(findings.length).toBeGreaterThan(0);
expect(findings[0]!.checkId.startsWith("checks/")).toBe(false);
expect(findings[0]!.checkId).toContain("mycheck");
});
});

describe("extractFindings — missing file", () => {
Expand Down
3 changes: 2 additions & 1 deletion packages/checks/src/extract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,10 @@ export async function extractFindings(
);
}
try {
const checkIdPrefix = checkId.startsWith("checks/") ? checkId.slice(7) : checkId;
const ctx: NormalizeContext = {
root: artifactDir,
checkIdPrefix: checkId,
checkIdPrefix,
defaultConfidence: 0.5,
};
const result = normalizeSarif(parsed as SarifLog, ctx);
Expand Down
3 changes: 2 additions & 1 deletion packages/policy/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@
"typecheck": "tsc --noEmit"
},
"dependencies": {
"@sverka/findings": "workspace:*"
"@sverka/findings": "workspace:*",
"@sverka/core": "workspace:*"
},
"devDependencies": {
"tsdown": "^0.22.0",
Expand Down
21 changes: 21 additions & 0 deletions packages/policy/src/__tests__/evaluator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,27 @@ describe("failOn rules", () => {
expect(r.triggered).toHaveLength(0);
});

it("checkIds filter matches rule-qualified finding checkIds", () => {
const f = makeFinding({ checkId: "check-a:rule-1", severity: "high" });
const policy = createPolicy({
failOn: [{ severity: "high", onlyNew: false, checkIds: ["check-a"] }],
});
const r = evaluatePolicy([f], policy, []);
expect(r.verdict).toBe("fail");
expect(r.triggered).toHaveLength(1);
expect(r.triggered[0]?.finding.checkId).toBe("check-a:rule-1");
});

it("checkIds filter accepts checks/ prefix", () => {
const f = makeFinding({ checkId: "check-a:rule-1", severity: "high" });
const policy = createPolicy({
failOn: [{ severity: "high", onlyNew: false, checkIds: ["checks/check-a"] }],
});
const r = evaluatePolicy([f], policy, []);
expect(r.verdict).toBe("fail");
expect(r.triggered[0]?.finding.checkId).toBe("check-a:rule-1");
});

it("multiple failOn rules evaluated independently", () => {
const low = findingAt("low");
const crit = findingAt("critical");
Expand Down
8 changes: 8 additions & 0 deletions packages/policy/src/__tests__/public-api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
DEFAULT_POLICY,
createPolicy,
evaluatePolicy,
verifyPolicyAgainstGraph,
PolicyError,
type Verdict,
type Policy,
Expand All @@ -14,6 +15,7 @@ import {
type RuleResult,
type PolicyResult,
type PolicyConfig,
type PolicyVerification,
type PolicyErrorCode,
} from "../index.js";

Expand All @@ -30,6 +32,10 @@ describe("public API — functions and constants", () => {
it("exports evaluatePolicy function", () => {
expect(typeof evaluatePolicy).toBe("function");
});

it("exports verifyPolicyAgainstGraph function", () => {
expect(typeof verifyPolicyAgainstGraph).toBe("function");
});
});

describe("public API — error class", () => {
Expand Down Expand Up @@ -66,6 +72,7 @@ describe("public API — types (compile-time check)", () => {
summary: "",
};
const _config: PolicyConfig = {};
const _verification: PolicyVerification = { valid: true, unknownCheckIds: [] };
const _code: PolicyErrorCode = "INVALID_POLICY";
// Touch all to avoid unused warnings.
expect(_verdict).toBe("pass");
Expand All @@ -75,6 +82,7 @@ describe("public API — types (compile-time check)", () => {
expect(_ruleResult.triggered).toBe(false);
expect(_result.verdict).toBe("pass");
expect(_config).toBeDefined();
expect(_verification.valid).toBe(true);
expect(_code).toBe("INVALID_POLICY");
});
});
Expand Down
129 changes: 129 additions & 0 deletions packages/policy/src/__tests__/verify.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
import { describe, it, expect } from "vitest";
import { verifyPolicyAgainstGraph } from "../verify.js";
import { DEFAULT_POLICY } from "../policy.js";
import type { Policy } from "../types.js";
import type { DefinitionGraph } from "@sverka/core";

function makeGraph(stepIds: string[]): DefinitionGraph {
return {
project: {
id: "test",
pipelines: [{
id: "ci",
inputs: {},
entries: [],
steps: stepIds.map((id) => ({
id,
runtime: { mode: "host" as const },
operations: [{ kind: "shell" as const, command: "echo hi" }],
inputs: [],
outputs: [],
dependencies: [],
})),
outputs: [],
}],
},
};
}

function makePolicy(checkIds?: string[]): Policy {
return {
name: "test",
default: "pass",
failOn: checkIds
? [{ severity: "high", onlyNew: false, checkIds }]
: [{ severity: "high", onlyNew: false }],
};
}

describe("verifyPolicyAgainstGraph", () => {
it("valid policy (all checkIds match) → valid=true", () => {
const graph = makeGraph(["checks/typecheck", "checks/lint"]);
const policy = makePolicy(["checks/typecheck", "checks/lint"]);
const result = verifyPolicyAgainstGraph(policy, graph);
expect(result.valid).toBe(true);
expect(result.unknownCheckIds).toEqual([]);
});

it("unknown checkId → valid=false, listed", () => {
const graph = makeGraph(["checks/typecheck"]);
const policy = makePolicy(["checks/typecheck", "checks/unknown"]);
const result = verifyPolicyAgainstGraph(policy, graph);
expect(result.valid).toBe(false);
expect(result.unknownCheckIds).toEqual(["checks/unknown"]);
});

it("policy with no checkIds → valid=true", () => {
const graph = makeGraph(["checks/typecheck"]);
const policy = makePolicy();
const result = verifyPolicyAgainstGraph(policy, graph);
expect(result.valid).toBe(true);
expect(result.unknownCheckIds).toEqual([]);
});

it("multiple unknown checkIds → all listed", () => {
const graph = makeGraph(["checks/typecheck"]);
const policy = makePolicy(["checks/typecheck", "checks/foo", "checks/bar"]);
const result = verifyPolicyAgainstGraph(policy, graph);
expect(result.valid).toBe(false);
expect(result.unknownCheckIds).toHaveLength(2);
expect(result.unknownCheckIds).toContain("checks/foo");
expect(result.unknownCheckIds).toContain("checks/bar");
});

it("DEFAULT_POLICY (no checkIds) → valid=true", () => {
const graph = makeGraph([]);
const result = verifyPolicyAgainstGraph(DEFAULT_POLICY, graph);
expect(result.valid).toBe(true);
});

it("empty graph → unknown checkIds reported", () => {
const graph = makeGraph([]);
const policy = makePolicy(["checks/typecheck"]);
const result = verifyPolicyAgainstGraph(policy, graph);
expect(result.valid).toBe(false);
expect(result.unknownCheckIds).toEqual(["checks/typecheck"]);
});

it("deduplicates checkIds across rules", () => {
const graph = makeGraph(["checks/typecheck"]);
const policy: Policy = {
name: "test",
default: "pass",
failOn: [
{ severity: "high", onlyNew: false, checkIds: ["checks/unknown"] },
{ severity: "medium", onlyNew: false, checkIds: ["checks/unknown"] },
],
};
const result = verifyPolicyAgainstGraph(policy, graph);
expect(result.valid).toBe(false);
// "checks/unknown" appears in both rules but should be listed once.
expect(result.unknownCheckIds).toEqual(["checks/unknown"]);
});

it("accepts bare checkIds that match checks/<id> steps", () => {
const graph = makeGraph(["checks/typecheck", "checks/lint"]);
const policy = makePolicy(["typecheck", "lint"]);
const result = verifyPolicyAgainstGraph(policy, graph);
expect(result.valid).toBe(true);
expect(result.unknownCheckIds).toEqual([]);
});

it("ignores non-check steps when validating checkIds", () => {
const graph = makeGraph(["build/compile"]);
const policy = makePolicy(["compile"]);
const result = verifyPolicyAgainstGraph(policy, graph);
expect(result.valid).toBe(false);
expect(result.unknownCheckIds).toEqual(["compile"]);
});

it("returns errors for malformed policy or graph without throwing", () => {
const badPolicy = { name: "x" } as unknown as Policy;
const badGraph = { project: {} } as unknown as DefinitionGraph;
const result = verifyPolicyAgainstGraph(badPolicy, badGraph);
expect(result.valid).toBe(false);
expect(result.unknownCheckIds).toEqual([]);
expect(result.errors).toBeDefined();
expect(result.errors!.length).toBeGreaterThanOrEqual(1);
});
});
24 changes: 22 additions & 2 deletions packages/policy/src/evaluator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,19 @@
import { PolicyError } from "./errors.js";
import { severityRank, assertValidSeverity } from "./policy.js";

/** Strip the `checks/` prefix so `checks/typecheck` and `typecheck` compare equal. */
function normalizeCheckId(id: string): string {
return id.startsWith("checks/") ? id.slice(7) : id;
}

/** Match a finding checkId against a policy checkId, allowing rule-qualified ids. */
function matchesCheckId(findingCheckId: string, policyCheckId: string): boolean {
const bareFinding = normalizeCheckId(findingCheckId);
const barePolicy = normalizeCheckId(policyCheckId);
if (bareFinding === barePolicy) return true;
return bareFinding.startsWith(`${barePolicy}:`);
}

/** Severity display order for summary counts (most severe first). */
const SUMMARY_SEVERITY_ORDER: Severity[] = [
"critical",
Expand Down Expand Up @@ -60,9 +73,16 @@

for (let i = 0; i < policy.failOn.length; i++) {
const rule = policy.failOn[i]!;
const rawCheckIds = rule.checkIds as string | string[] | undefined | null;
const checkIds = rawCheckIds === undefined || rawCheckIds === null
? []
: Array.isArray(rawCheckIds)
? rawCheckIds
: [rawCheckIds];

Check warning on line 81 in packages/policy/src/evaluator.ts

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Extract this nested ternary operation into an independent statement.

See more on https://sonarcloud.io/project/issues?id=sverka-dev_sverka&issues=AZ_9DRFmIeMYNSpD2iEI&open=AZ_9DRFmIeMYNSpD2iEI&pullRequest=44

let matched: Finding[] = findings.filter((f) => {
// checkIds filter (exact match)
if (rule.checkIds !== undefined && !rule.checkIds.includes(f.checkId)) {
// checkIds filter (supports bare, checks/ prefix, and rule-qualified ids)
if (checkIds.length > 0 && !checkIds.some((id) => matchesCheckId(f.checkId, id))) {
return false;
}
// onlyNew filter
Expand Down
4 changes: 3 additions & 1 deletion packages/policy/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// @sverka/policy — public API
// @sverka/policy — public API. Spec 16.
export type { Verdict, Policy, FailOnRule, TriggeredFinding,
RuleResult, PolicyResult, PolicyConfig } from "./types.js";
export { DEFAULT_POLICY, createPolicy } from "./policy.js";
export { evaluatePolicy } from "./evaluator.js";
export { verifyPolicyAgainstGraph } from "./verify.js";
export type { PolicyVerification } from "./verify.js";
export { PolicyError, type PolicyErrorCode } from "./errors.js";
93 changes: 93 additions & 0 deletions packages/policy/src/verify.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
// Policy verification against a Definition Graph.
// Spec 16 — §26, §27. Verifies that policy checkIds reference check
// steps that exist in the graph.

import type { DefinitionGraph } from "@sverka/core";
import type { Policy } from "./types.js";

export interface PolicyVerification {
readonly valid: boolean;
readonly unknownCheckIds: readonly string[];
readonly errors?: readonly string[];
}

/** Strip the `checks/` prefix so `checks/typecheck` and `typecheck` compare equal. */
function normalizeCheckId(id: string): string {
return id.startsWith("checks/") ? id.slice(7) : id;
}

/**
* Verify that all checkIds referenced in a policy's failOn rules exist
* as check step IDs in the Definition Graph. A step is treated as a check
* when its ID starts with `checks/`. Returns a PolicyVerification
* with valid=false and the list of unknown check IDs if any are found.
*
* Does not throw for structural errors — returns them in `errors`.
*/
export function verifyPolicyAgainstGraph(
policy: Policy,
graph: DefinitionGraph,
): PolicyVerification {
const errors = collectValidationErrors(policy, graph);
if (errors.length > 0) {
return { valid: false, unknownCheckIds: [], errors };
}

const knownCheckIds = collectKnownCheckIds(graph);
const referencedCheckIds = collectReferencedCheckIds(policy);
const unknownCheckIds = findUnknownCheckIds(referencedCheckIds, knownCheckIds);

return {
valid: unknownCheckIds.length === 0,
unknownCheckIds,
};
}

function collectValidationErrors(policy: Policy, graph: DefinitionGraph): string[] {
const errors: string[] = [];
if (!policy || typeof policy !== "object") {
errors.push("invalid policy: expected object");
} else if (!Array.isArray(policy.failOn)) {
errors.push("invalid policy: failOn must be an array");
}
if (!graph || typeof graph !== "object" || !Array.isArray(graph.project?.pipelines)) {
errors.push("invalid graph: project.pipelines must be an array");
}
return errors;
}

function collectKnownCheckIds(graph: DefinitionGraph): Set<string> {
const knownCheckIds = new Set<string>();
for (const pipeline of graph.project.pipelines) {
if (!Array.isArray(pipeline.steps)) continue;
for (const step of pipeline.steps) {
if (typeof step.id === "string" && step.id.startsWith("checks/")) {
knownCheckIds.add(normalizeCheckId(step.id));
}
}
Comment thread
qodo-code-review[bot] marked this conversation as resolved.
}
return knownCheckIds;
}

function collectReferencedCheckIds(policy: Policy): Set<string> {
const referencedCheckIds = new Set<string>();
for (const rule of policy.failOn) {
const raw = (rule as { checkIds?: unknown }).checkIds;
if (raw === undefined || raw === null) continue;
const ids: string[] = Array.isArray(raw) ? (raw as string[]) : [String(raw)];

Check warning on line 77 in packages/policy/src/verify.ts

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

'raw' will use Object's default stringification format ('[object Object]') when stringified.

See more on https://sonarcloud.io/project/issues?id=sverka-dev_sverka&issues=AZ_9DRETIeMYNSpD2iEH&open=AZ_9DRETIeMYNSpD2iEH&pullRequest=44
for (const id of ids) {
if (typeof id === "string") referencedCheckIds.add(id);
}
}
return referencedCheckIds;
}

function findUnknownCheckIds(referenced: Set<string>, known: Set<string>): string[] {
const unknown: string[] = [];
for (const id of referenced) {
if (!known.has(normalizeCheckId(id))) {
unknown.push(id);
}
}
return unknown;
}
Loading