From 6e268f8b86891e1df81b1111fce123715e08de40 Mon Sep 17 00:00:00 2001 From: leemour Date: Fri, 16 Oct 2026 00:43:24 +0200 Subject: [PATCH] fix(security): retain rule and parser gaps in comparison coverage --- CHANGELOG.md | 3 +++ docs/security/METHOD.md | 2 ++ src/observations.test.ts | 55 +++++++++++++++++++++++++++++++++++++++- src/observations.ts | 25 ++++++++++++++++++ src/runs.test.ts | 13 ++++++++++ src/runs.ts | 6 ++--- 6 files changed, 100 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b7d4e43..5eb7438 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,9 @@ Every entry says what changed as a caller sees it, why, and what to watch for ### Fixed +- Security comparison coverage includes Semgrep rule/file gaps and partial-parsing spans. A new + gap cannot make a missing observation appear fixed merely because the scanner names match. + Existing records retain their original coverage; older and new fingerprints must not be mixed. - `cli-testing security scan` runs its Semgrep stage from an installed package: the stage ships as compiled `dist/semgrep.js`, because Node refuses to strip types under `node_modules`. Before, every scan from an install failed at Semgrep; it worked only from a checkout. diff --git a/docs/security/METHOD.md b/docs/security/METHOD.md index 8508693..9ee0511 100644 --- a/docs/security/METHOD.md +++ b/docs/security/METHOD.md @@ -24,6 +24,8 @@ Start with the read-only preflight in [RUNBOOK](RUNBOOK.md#preflight-and-baselin baseline without `--previous` when no complete comparable run exists. Historical gaps stay gaps; scanner completion never substitutes for the manual/Socket/A7 stages. The [regression checklist](REGRESSIONS.md) defines the evidence and stage ledger for a complete audit. +Comparison coverage includes Semgrep rule/file gaps and partial-parsing positions. A change in +those warnings refuses automatic fixed-observation claims, even when scanner names are unchanged. 1. `cli-testing security scan ... [--previous ]` creates the run folder, refuses tool versions other than the pins (or pins younger than two weeks), and calls diff --git a/src/observations.test.ts b/src/observations.test.ts index 2baa950..8ea436a 100644 --- a/src/observations.test.ts +++ b/src/observations.test.ts @@ -1,7 +1,60 @@ import { describe, expect, it } from "vitest" -import { scannerObservations } from "./observations.js" +import { compareRuns } from "./compare.js" +import { scannerCoverage, scannerObservations } from "./observations.js" describe("scanner observations", () => { + it("refuses fixed claims when a rule/file gap or partial-parsing span changes", () => { + const report = { + results: [], + coverageGaps: [{ rule: "synthetic-rule", path: "src/fixture.ts" }], + errors: [ + { + type: ["PartialParsing", [{ path: "src/fixture.ts", start: { line: 1, col: 2 }, end: { line: 1, col: 5 } }]], + }, + ], + } + const coverage = scannerCoverage("fixture", "semgrep", report) + expect(coverage).toHaveLength(3) + const before = { + schema: 1 as const, + suite: "security", + complete: true, + observations: [{ id: "synthetic" }], + known: ["synthetic"], + coverage: scannerCoverage("fixture", "semgrep", { results: [] }), + } + expect(() => compareRuns("security", before, { ...before, observations: [], coverage })).toThrow( + "coverage must match", + ) + const moved = { + ...report, + errors: [ + { + type: ["PartialParsing", [{ path: "src/fixture.ts", start: { line: 2, col: 2 }, end: { line: 2, col: 5 } }]], + }, + ], + } + expect(scannerCoverage("fixture", "semgrep", moved)).not.toEqual(coverage) + expect(scannerCoverage("fixture", "codeql", [])).toEqual(['["fixture","codeql"]']) + }) + + it("rejects malformed gap metadata rather than silently dropping its coverage", () => { + for (const report of [ + { coverageGaps: [{ rule: "", path: "fixture.ts" }] }, + { + errors: [ + { + type: ["PartialParsing", [{ path: "fixture.ts", start: { line: "1", col: 1 }, end: { line: 1, col: 2 } }]], + }, + ], + }, + { errors: [{ type: ["PartialParsing", null] }] }, + ]) + expect(() => scannerCoverage("fixture", "semgrep", report)).toThrow() + expect(scannerCoverage("fixture", "semgrep", { errors: [{ type: "other warning" }] })).toEqual([ + '["fixture","semgrep"]', + ]) + }) it.each([ [ "osv", diff --git a/src/observations.ts b/src/observations.ts index a534d8b..379e79c 100644 --- a/src/observations.ts +++ b/src/observations.ts @@ -13,6 +13,31 @@ const string = (value: unknown): string => { return value } +export const scannerCoverage = (repo: string, tool: string, value: unknown): string[] => { + const keys = new Set([JSON.stringify([repo, tool])]) + if (tool !== "semgrep") return [...keys] + const report = object(value) + for (const item of list(report.coverageGaps ?? [])) { + const gap = object(item) + keys.add(JSON.stringify([repo, tool, "rule-gap", string(gap.rule), string(gap.path)])) + } + const point = (value: unknown) => { + const position = object(value) + if (!Number.isSafeInteger(position.line) || !Number.isSafeInteger(position.col)) + throw new Error("invalid partial parsing position") + return [position.line, position.col] + } + for (const item of list(report.errors ?? [])) { + const type = object(item).type + if (!Array.isArray(type) || type[0] !== "PartialParsing") continue + for (const item of list(type[1])) { + const span = object(item) + keys.add(JSON.stringify([repo, tool, "partial", string(span.path), point(span.start), point(span.end)])) + } + } + return [...keys].sort() +} + /** These are scanner observations, not verdicts on whether a vulnerability is reachable. */ export const scannerObservations = (repo: string, tool: string, value: unknown): Observation[] => { const ids: string[] = [] diff --git a/src/runs.test.ts b/src/runs.test.ts index 4e7a297..15ece60 100644 --- a/src/runs.test.ts +++ b/src/runs.test.ts @@ -101,4 +101,17 @@ describe("run folders", () => { expect(() => readRun(root)).toThrow("invalid run record") } }) + + it("derives explicit rule coverage from retained raw reports without rewriting them", () => { + const root = createRun(join(fixture(), "coverage")), + repo = join(root, "raw", "fixture") + mkdirSync(repo) + const original = JSON.stringify({ results: [], coverageGaps: [{ rule: "synthetic-rule", path: "fixture.ts" }] }) + const path = join(repo, "semgrep.json") + writeFileSync(path, original) + const coverage = new Set() + expect(collectObservations(root, coverage)).toEqual([]) + expect(coverage.has('["fixture","semgrep","rule-gap","synthetic-rule","fixture.ts"]')).toBe(true) + expect(readFileSync(path, "utf8")).toBe(original) + }) }) diff --git a/src/runs.ts b/src/runs.ts index d29246c..f604722 100644 --- a/src/runs.ts +++ b/src/runs.ts @@ -1,7 +1,7 @@ import { existsSync, mkdirSync, readdirSync, readFileSync, realpathSync, writeFileSync } from "node:fs" import { dirname, join, resolve } from "node:path" import type { Observation, RunRecord } from "./compare.js" -import { scannerObservations } from "./observations.js" +import { scannerCoverage, scannerObservations } from "./observations.js" export const readRun = (directory: string): RunRecord => { if (!existsSync(join(directory, "run.json"))) { @@ -48,7 +48,6 @@ export const collectObservations = (directory: string, coverage?: Set): for (const file of readdirSync(join(directory, "raw", repo.name), { withFileTypes: true })) { if (!file.isFile() || !file.name.endsWith(".json")) continue const tool = file.name.slice(0, -5) - coverage?.add(JSON.stringify([repo.name, tool])) const raw = readFileSync(join(directory, "raw", repo.name, file.name), "utf8") let value: unknown try { @@ -60,7 +59,8 @@ export const collectObservations = (directory: string, coverage?: Set): .split("\n") .map((line) => JSON.parse(line)) } - observations.push(...scannerObservations(repo.name, tool, value)) + for (const key of scannerCoverage(repo.name, tool, value)) coverage?.add(key) + for (const observation of scannerObservations(repo.name, tool, value)) observations.push(observation) } } return observations