Skip to content

feat(core)!: finding identity computed once and transported (finding_governance Task 1.3a) - #171

Open
cagodoy wants to merge 30 commits into
mainfrom
feat/finding_governance
Open

cagodoy wants to merge 30 commits into
mainfrom
feat/finding_governance

Conversation

@cagodoy

@cagodoy cagodoy commented Sep 13, 2026

Copy link
Copy Markdown
Member

Summary

Epic finding_governance, Cycle 1, Task 1.3a — core and agents. The finding identity is computed once, by createFinding, and transported everywhere else. This PR closes the code side of the task and the audit that followed it; the SaaS projection and backfill (Task 1.3b, private) start once @gitgov/core is published from this branch.

What changes in @gitgov/core (major)

Identity (AUDIT-K1..K6)

  • fingerprint = sha256("gitgov-fp/2|file|category|normalizeAnchor(anchor)), computed only by createFinding. rehydrateFinding transports it byte for byte. Neither accepts a caller-supplied fingerprint.
  • The three generateFingerprint copies in the detectors and the two positional fallbacks in the orchestrator and policy evaluator are gone. SarifBuilder writes the identity to fingerprints["gitgov/v2"] (SARIF-N1); partialFingerprints only when it can read the line (N2).
  • The orchestrator consolidates by the transported key, reports unmatched waivers (AuditSummary.unmatchedWaivers, required) and warns when it discards a result with no identity (AORCH-B12/B15).

One domain per concept (AUDIT-J4, J5, M1, B4)

  • SCAN_SCOPES / ScanScope = diff | full | baseline, referenced by AuditOrchestrationOptions, AgentAuditInput, Scan, the SARIF scanScope properties and the CLI --scope. It used to be a two-value union with four inline three-value copies.
  • SeverityCounts + countBySeverity replace five spellings of the severity map and five hand-rolled loops. AuditSummary is SeverityCounts & {...}: same flat shape, one declaration.
  • BASE_FINDING_CATEGORIES is a runtime tuple of the 38 built-in categories; FindingCategory stays open.
  • createFinding, rehydrateFinding and verifySnippet share the one sha256 from crypto/checksum.

Removed

  • source_auditor: AuditTarget, CodeScope, JiraScope, GitgovScope, AuditScope, GroupByOption, OutputFormat, FailOnSeverity (zero consumers; 'none' was accepted nowhere) and ScoringEngine (a pass-through with no requirement). ScopeSelector stays exported. AuditSummary of this module is now SourceAuditSummary; WaiverApplicationCounts.unmatched is required.
  • redaction: the as unknown as cast in redact().

Redaction behaviour

  • redact() carries finding.snippetHash unchanged at every level and never recomputes it (RLDX-B3). redactSarif keeps a transported gitgov/snippetHash and computes one only when absent (RLDX-B9).
  • A sensitive finding at L1 comes back with the fixes key omitted, not set to undefined (RLDX-B4).
  • security-vulnerability is sensitive and code-quality is safe; every built-in category is classified and RLDX-A6 fails the day one is added unclassified. RedactionConfig lists are FindingCategory[]. isSensitiveCategory is public.

Also in this branch

  • audit() in source_auditor reads and detects in batches of 100 (EARS-E4) and warns on unreadable files (EARS-B3).
  • policy_evaluator rehydrates from the transported identity (PEVAL-F6).
  • Every live EARS of source_auditor and redaction has a // [EARS-XX] marker.
  • CLI: AuditCommandOptions.scope and failOn are core's ScanScope and FindingSeverity; the DI test wires the real redactor.
  • MCP: gitgov_audit_scan spreads changedSince conditionally; its test mocks against the real AuditResult.
  • Agents: semgrep and security-audit call countBySeverity; source-audit gains a tsconfig.

Verification

  • tsc --noEmit: 0 errors in core, cli, mcp-server, semgrep, security-audit, source-audit.
  • core: 3137/3137 (133 suites), including the clean-exports guardrail. cli: dependency-injection + audit-command 70/70. mcp-server audit tools 10/10. semgrep 27/27, security-audit 29/29.
  • Negative controls, run and reverted: five independent mutations (a category removed, hasWaiver ignoring the fingerprint, countBySeverity ignoring the severity, redactSarif overwriting a transported hash, redact() recomputing the hash) turn 12 named tests red.
  • Not run here: packages/e2e (needs Postgres in globalSetup); it is the Task 1.4 gate.

Audit

/triad:audit source_auditor redaction produced 85 findings; 75 are closed in this branch and its private counterpart, 10 are criteria of Task 1.3b/1.4 with their origin ID. Record: private/…/epics/active/finding_governance/audits/audit_2026-09-11_source_auditor_redaction.md. Four decisions carry a PM verdict and await the owner's seal: ScanScope unification, ScoringEngine removal, retirement of the multi-target types, and the SAST classification.

Coordination

This major also carries ProjectInitError (FE's branch, core/src/index.ts). The two public constructors of this PR (createFinding, rehydrateFinding) and that error type are the surface that changes in @gitgov/core; whichever merges second rebases the export line.

…not the GitGov SarifLog

`SemgrepAgent.run()` declared its second parameter as `Sarif.SarifLog`, whose
result `properties` bag requires `gitgov/category`, `gitgov/detector` and
`gitgov/confidence`. What semgrep emits is SARIF 2.1.0 with its own metadata
(`properties.metadata.cwe`), which is exactly what the agent reads to map
categories. Every test fixture passing raw semgrep output therefore failed
`tsc --noEmit` (8 errors in agent.test.ts, reproduced on main); jest never
noticed because it does not type-check.

- `SemgrepRawResult` / `SemgrepRawSarif` in types.ts describe the input shape;
  `run()`, `mapResultsToFindings()`, `mapCategory()` and `buildSummary()` use it.
- The fixtures stay as they are: they are honest raw semgrep output. Giving
  them GitGov properties would have made them lie about what semgrep produces.
- config.test.ts `[SGP-D3]` passes the required `scope` to `buildSemgrepArgs`
  (the ninth pre-existing error).

`pnpm typecheck` 0 errors, jest 27/27. No runtime change.
…nts["gitgov/v2"] (SARIF-N1/N2)

Task 1.3a of finding_governance, first slice. The finding identity is
`Finding.fingerprint`; the builder now carries it as-is under SARIF §3.27.16
`fingerprints["gitgov/v2"]`, with or without source access, and never
recalculates it.

- [SARIF-N1] every result carries `fingerprints["gitgov/v2"] = finding.fingerprint`.
- [SARIF-N2] `partialFingerprints["primaryLocationLineHash/v1"]` is emitted only
  when `getLineContent` is provided, and only for GitHub code scanning interop.
  Without source access it is omitted entirely: the SARIF-C5 fallback that put
  `Finding.fingerprint` under the GitHub key is removed. That fallback welded two
  different values under one key, so no consumer could tell which one it read
  (input #19 §0.2).
- [SARIF-F1] waivers are matched by `finding.fingerprint`, never by the line hash;
  F1..F5 tests rebuilt on that key.

Consumers still reading `primaryLocationLineHash/v1` (audit_orchestrator,
policy_evaluator, saas-api projection) are untouched here: they switch to the
new key in the next slices (AORCH-B6/B12, PEVAL-F6, AP-K1), which need
`computeFingerprint` from the identity block.

sarif 50/50, core 3104/3104, tsc 0 errors.
…g, and never recomputed

BREAKING CHANGE: createFinding no longer accepts `fingerprint`. Producers pass `anchor` —
the text they matched — and the factory derives the identity. Consumers rebuilding a
transported Finding use the new rehydrateFinding, which keeps the value byte for byte.

Until now six places computed a finding's identity their own way: three copies of
generateFingerprint in the detectors, two of buildFallbackFingerprint in the consumers, and
the SARIF line hash that silently overwrote all of them in transport. The value that
actually reached the database was sha256(normalized line)[0:16]:occurrence, carrying
neither file nor category — so the same line in two files was one finding, one waiver
silenced both, and a reformat that split the line lost the identity altogether.

  fingerprint = sha256("gitgov-fp/2|" + file + "|" + category + "|" + normalizeAnchor(anchor))

AUDIT-K1 to K6 in core/src/audit/fingerprint.ts and types.ts. The scheme tag is hashed
inside the preimage rather than stored in a column, so identities of different natures
coexist without runtime branching and a formula change recomputes everything once.
normalizeAnchor trims and collapses whitespace, and deliberately does not lowercase or
strip comments: case is semantic in a secret and comment syntax is language-dependent.

EARS-16, 31, 32 and 33 in finding_detector: the three generateFingerprint copies are gone,
regex and heuristic detectors pass match[0], the LLM detector passes its snippet, and the
semgrep agent stops emitting an empty fingerprint. HEURISTIC_PATTERNS is exported indexed
by ruleId because the backfill has to re-run the detector's own rule to derive the anchor
of a stored finding.

AUDIT-J1 to J3 close as well. They were red in the spec since s78b-37 with a note saying
the code did not exist; measured at the start of this pass, both the code and the tests
were there and passing. Only the spec vertex was missing.

audit_orchestrator and policy_evaluator switch to rehydrateFinding. That is mechanical
adaptation, not their pass: AORCH-B6/B12/B14 and PEVAL-F6 stay open, and the code says so.

Every EARS carries an observed red and a measured negative control. The controls reproduce
the defects the block closes — dropping `file` from the preimage collapses two files,
dropping `category` collapses a secret and a PII hit on the same text. Verified by mutation
that the tests discriminate rather than merely pass: removing `file` turns two red, adding
toLowerCase to normalizeAnchor turns one red.

tsc clean in core, agents and cli. core 3121/3121, semgrep 27/27.
…y and reports unmatched waivers

BREAKING CHANGE: AuditSummary gains a required `unmatchedWaivers`. Three sites build one —
the orchestrator twice and createScan — so requiring it costs three lines and buys a number
that cannot be silently omitted.

AORCH-B6: consolidation keys on fingerprints["gitgov/v2"], compared by equality, and rebuilds
each Finding with rehydrateFinding. Reading partialFingerprints here was the defect: that key
is GitHub's line hash, it carries neither file nor category, and it quietly replaced whatever
the detector had computed. With the new key, regex and semgrep matching the same token under
different rules produce ONE finding with both agents in reportedBy — no rule mapping needed,
because ruleId never enters the identity.

AORCH-B12: a result without the key — an external tool, or a SARIF written before the cut —
gets its identity from computeFingerprint, the same function the detectors use.
buildFallbackFingerprint is deleted: `fallback:{ruleId}:{file}:{line}` changed the moment
anyone inserted a line above the finding, so the same finding became a new one between runs.
A result with neither key nor snippet is skipped rather than given a fabricated identity.

AORCH-B14: two results sharing the key but declaring different categories are not merged. The
first wins, the second is rejected with a warning naming both categories and the fingerprint.
With category inside the preimage this is only reachable from a malformed SARIF, and the point
is that the consolidated finding must not go quiet wearing whichever category arrived first.

AORCH-B15: summary.unmatchedWaivers counts active waivers that matched no consolidated
finding, and the CLI prints the line only when that count is above zero. After the identity
cut every waiver written with the old fingerprint stops matching at once, and the user's only
other signal is "0 waived" — which reads exactly like having had no waivers at all.

Five test fixtures still built their SARIF with the old key. One had no snippet, so B12 now
skips its result and the test went red — correctly. The other four kept passing for the wrong
reason: they fell through to B12's fallback instead of transporting an identity. All five
moved to the new key; the helper keeps the old one behind a `legacyKeyOnly` flag, which is
what exercises the fallback path deliberately.

core 3125/3125, cli 622/622, tsc clean in core, cli and both agents.
…tector tests

STRIPE_KEY_B belongs to finding_detector.test.ts, where two DIFFERENT secrets in one file
prove the collapse is by anchor rather than by file. The banner that introduced the split
fixtures declared it in both files, and the regex detector tests never use it.

jest let it through — ts-jest does not apply noUnusedLocals — and tsc caught it. Worth
recording which instrument saw it: a green suite is not a typecheck.
… (PEVAL-F6)

extractFindingsFromSarif is a rehydrator and now behaves like the orchestrator: it keys on
fingerprints["gitgov/v2"], keeps the transported value, and derives one with
computeFingerprint only when the key is absent. Its private copy of buildFallbackFingerprint
is deleted, which was the second of the two the epic set out to remove — there are now none
left in core.

This path reads L1, where the snippet may already be [REDACTED], so recomputing here would
diverge from what the producer wrote even more readily than in the orchestrator.

The red had to be provoked. makeSarifLogForReeval emitted the identity under the OLD key, the
one the old code read, so the F6 test passed against unchanged code and discriminated
nothing. Moving the fixture to the new key turned four re-evaluation tests red, and only then
did the red mean something. Same finding as in audit_orchestrator: a fixture on the old key
turns an identity test into a test that measures nothing.

core 3126/3126, tsc clean.
…o finding (EARS-C6)

BREAKING CHANGE: WaiverApplicationCounts gains a required `unmatched` field. It is public
API through the SourceAuditor namespace of the main barrel, so anyone constructing an
AuditResult has to supply it.

After the identity cut (AUDIT-K1..K6) every waiver in .gitgov/feedbacks/ written with the
old value stops matching at once. The SaaS re-keys them with the AP-K2 backfill; the CLI
without SaaS has no backfill, so those waivers silently apply to nothing. The two counts
that existed cannot report it: a run with a stale waiver and a run with no waivers at all
agree on both `acknowledged` and `new`. Only the new counter separates them, and that is
what its negative control asserts.

createEmptyResult reports `unmatched: 0` deliberately, and the asymmetry with AORCH-B15 is
declared in the code and in the spec. Nothing was scanned there, so no waiver was tested
against anything, which is a different fact from "the scan ran and no waiver matched".
AORCH-B15's early return does count them because its trigger is a misconfiguration, while a
scope selecting no files is ordinary in incremental mode. Counting them here would also give
two answers for one situation, since audit() reaches that return before loading waivers at
all while auditContents() receives them in its input.

The radius was measured by adding the type before the code: exactly two tsc errors, at
source_auditor.ts:87 and :250. The second is inside createEmptyResult, where the object is
a literal and a grep for acknowledgedCount does not find it.

Verified by mutation that the tests discriminate rather than merely pass. The first run
exposed a defect in the test, not the code: with one matching waiver and one stale one,
counting the waivers that matched and counting those that did not return the same number, so
an inverted predicate kept it green. Rewritten asymmetric, one matching and two stale. Both
mutations now turn two red.

tsc 0 in core, core 3128/3128, source_auditor 46/46. Triad verified with the sweep scoped to
the module directory: [EARS-C6] also exists in ten files of core outside source_auditor, so
a repo-wide grep reports code present over someone else's code.
…s the fixes guard

RedactableInput read `Finding | Finding`, a leftover from when detectors and the
orchestrator had two separate finding types. The union of a type with itself is that type,
so the declaration described a distinction that no longer existed. Section 3.2 of the spec
already said `Finding`; the code was the deviant vertex, so no EARS changes. Six fossil
comments went with it, across two files. The first sweep found five and missed the one
written in Spanish ("un Finding o Finding"); a search by term does not see a paraphrase.

The audit then measured that the two runtime guards in redact() were not pinned by any test:
dropping both left all 3128 tests of core green. They were not equivalent. The guard on
`snippet` was dead, because `snippet` is required on Finding and the condition could never be
false. The guard on `fixes` is load-bearing: under exactOptionalPropertyTypes, assigning
`undefined` to an absent optional key adds the key, so a finding with no `fixes` came back
carrying `fixes: undefined`. The dead guard is removed and the live one now has a second
RLDX-B4 test whose discriminating assertion is `'fixes' in result` — `toBeUndefined()`
passes either way, which is why the guard sat uncovered. Verified by mutation: without the
guard, one red.

tsc 0, redaction 22/22, core 3131/3131.
…files and batches reads (EARS-B3, EARS-E4)

BREAKING CHANGE: `SourceAuditor.AuditSummary` is now `SourceAuditor.SourceAuditSummary`.
`audit/types.ts` already exported a different `AuditSummary` — the orchestrator's, whose
`total` counts findings including the waived ones, while this one counts post-waiver. Two
sibling modules of one package exported the same name with opposite semantics on a shared
field. The compiler shows the two are not unifiable: neither is assignable to the other and
the only shared key is `total`. audit_record_types designates audit/types.ts as canonical,
so this is the side that renames. Radius measured with the real grep: 12 files, all in the
monorepo, none in saas-api.

EARS-B3 asked for a warning on an unreadable file and the catch was silent. The test could
not see it: it deleted the file before audit(), so scope selection never listed it and the
catch was never entered — it passed because one file existed, not because one failed. The
read now fails after selection, both files are asserted in scope first, and the warning is
asserted by content. The waiver-loading catch, equally silent, now warns too: continuing
without waivers makes every waived finding reappear as new. Mutation: a rethrowing catch
turns both new tests red where the old one stayed green.

EARS-E4 was green with a test that measured nothing, and the requirement was not
implemented. createBatches produced slices that a sequential await loop walked one at a time
— iterating [[a,b],[c,d]] and [a,b,c,d] is the same execution — and audit() had already
loaded every file before any of it ran. The fixture used 152 files against a 1000 threshold,
so it produced one batch and asserted `scannedFiles`, which is unrelated. audit() now reads
and detects in batches of BATCH_SIZE with each batch going out of scope before the next, and
finishAudit() carries the shared tail so the waiver counters and the summary stay in one
place. The inert inner batching is removed. The EARS is amended to the clause that makes it
verifiable: detection of the first batch happens before the last read. Measured on the old
pipeline, firstDetect was 252 of 252 reads. Mutation with batch = filePaths.length: one red.

tsc 0 in core, both agents and the CLI. source_auditor 48/48, core 3131/3131.
… and are typed

Three AuditResult stubs were missing `unmatched` after WaiverApplicationCounts gained it as
a required field, and tsc in this package still exited 0: the stub factory had no return
type and took `Record<string, unknown>` overrides, so nothing was checked against anything.
The factory is now typed as SourceAuditor.AuditResult and the cascade that annotation
exposed was three real fictions — `bySeverity: {}` and `byDetector: {}` where the type
requires every key, and a hand-built Finding missing snippetHash, executionId, reportedBy
and isWaived with invented fingerprints.

That Finding is not rebuilt with createFinding, and the reason is recorded in the test: this
package's jest cannot load core's ESM at runtime, so no test here can call a core factory.
The literal is typed as Finding so the compiler enforces completeness, and it is marked as
an opaque pass-through fixture, which is all the test reads it as.

security-audit 29/29 (the suite did not load before), tsc 0.
…the canonical audit/types

Six files in the module imported `Finding` (and in one case `DetectorName`) from
`finding_detector/types`, which only re-exports them from `audit/types`. Type-identical, so
nothing broke, but the spec's Appendix B claimed the correction had already been applied on
2026-09-07 and the file still carried the old path — a spec asserting a false state of the
code. AUDIT-B1 names `audit/types` as the canonical source; it now holds for the whole module
rather than for `types.ts` alone.

Found by the dependency auditor (findings #17 and the Appendix B item). No behaviour change.
tsc 0 in core, both agents and the CLI; core 3131/3131.
… the L2 hash value (RLDX-B8, B9, B10)

The three tests each used a single SARIF result, always of a sensitive category, and B9/B10
ran at l1 only. B10 checked the hash with a 64-hex regex. Measured before this change: dropping
the category decision, processing only results[0], or hashing '[REDACTED]' at l2 each left all
four redactSarif tests green.

B8 now puts a safe result next to the sensitive one in the same SarifLog and asserts the safe
snippet survives — the word "sensitive" in the requirement was never exercised without that.
B9 uses two results at both levels and asserts each hash against its own snippet. B10 asserts
the hash value, not its shape; the regex accepted sha256('[REDACTED]') as readily as the real
digest, and audit_projection reads that hash into GitgovFinding.snippetHash, so a wrong L2
value flipped every RLDX-F4 verification to unverified with nothing failing.

Measured after: each mutation turns red exactly the test that covers it. No production code
changed. core 3131/3131, tsc 0.
…action on request as O1..O4

normalizeLineContent is now a re-export of normalizeAnchor from audit/fingerprint. It was a
second copy of "trim and collapse whitespace"; the identity formula and the GitHub interop
line hash normalize the same way and must keep doing so, so the copy is gone. B1/B2 stay green
against the single implementation.

N1/N2 and F1..F5 had code and tests since b9b014d and were red because they were written
outside the skill's flow. Verified here by mutation: reintroducing the C5 fallback turns red
exactly the N2 omission test; altering the transported identity by one character turns red
exactly N1; matching waivers by inequality turns red F1..F4 (F5 pins the empty-list guard and
correctly does not depend on the predicate). Markers for F2..F5 added; only F1 had one.

The four tests carrying SARIF-M1..M4 collided with the real M1/M2 (fixes, section 4.13) and
had no EARS — the spec called them "4 tests adicionales sin EARS". They are the test vertex
of the builder's redaction-on-request behaviour, which saas-api uses in production on the
export path. Specified as SARIF-O1..O4 and renamed to match; no production code changed.
Mutations: ignoring the supplied config turns red O4; forcing l1 turns red O2; skipping the
redactor turns red O1 and O4.

sarif 50/50, core 3131/3131, tsc 0 in core, both agents and the CLI.
…on discard, B6 gains its negative control

The count of active waivers matching no finding was computed in three places: source_auditor's
filterByWaivers, the orchestrator's buildSummary, and the orchestrator's no-agents branch,
which hard-coded it as waivers.length with the reasoning in a comment. It is now one function
in audit/types.ts, AUDIT-L1, and the edge case is its property: with no findings every waiver
is unmatched, which the empty-set comparison yields on its own. Mutation: inverting the
predicate in the helper turns red five tests in three files — L1, EARS-C6 and AORCH-B15 — so
all three sites demonstrably route through it. The two field names stay: `unmatched` inside a
type that is all about waivers is not ambiguous, `unmatchedWaivers` inside AuditSummary needs
the suffix.

AORCH-B12 discarded a result with neither identity key nor snippet with a bare continue,
while B14 warns in the analogous case. It now warns, in B14's format, and the EARS says so.
Mutation: removing the warning turns red exactly the new test.

AORCH-B6 had three green tests and none was the negative control of its own bug: that a
result carrying only partialFingerprints["primaryLocationLineHash/v1"] must not be keyed by
it. The legacyKeyOnly fixture flag existed for this and no test used it. It does now, and
the mutation that reintroduces the pre-cut fallback turns red exactly that test — the other
three did not see it.

Two vacuous tests removed on PM's verdict: RLDX-A1 asserted a typed literal against itself
with the type erased at compile time, and EARS-C3/C4 tested expiry in a function that never
reads expiresAt (their requirement lives in WaiverReader as F3/F4).

core 3132/3132, tsc 0 in core, both agents and the CLI.
…t-in categories (AUDIT-J4, J5, M1, B4)

ScanScope is declared once as a tuple with three values (diff, full, baseline)
and every copy — AuditOrchestrationOptions, AgentAuditInput, Scan, the SARIF
scanScope properties, the CLI --scope option — references it. It used to be a
two-value union that could not record a baseline run while four inline copies
carried three values and saas-api bridged the gap with a cast.

SeverityCounts and countBySeverity replace five spellings of the severity map
and three hand-rolled loops (createScan, the orchestrator's buildSummary,
source_auditor's calculateSummary; the semgrep and security-audit agents now
call the same function). AuditSummary is the intersection SeverityCounts & {...},
structurally identical to the flat shape it always had.

BASE_FINDING_CATEGORIES is a runtime tuple of the 38 built-in categories and
BaseFindingCategory derives from it; FindingCategory stays open. AUDIT-B4
generalizes the import rule that source_auditor and redaction were citing B1 for.

The multi-target block of source_auditor (AuditTarget, CodeScope, JiraScope,
GitgovScope, AuditScope, GroupByOption, OutputFormat, FailOnSeverity) is removed:
zero consumers outside the module, and FailOnSeverity carried a 'none' no caller
accepted. The CLI types failOn as FindingSeverity from core.

createFinding, rehydrateFinding and verifySnippet call the one sha256 from
crypto/checksum instead of reimplementing createHash three times; imported from
the file and not the barrel so @gitgov/core/audit stays clean of util (EARS-CI02).

The MCP audit tool spreads changedSince conditionally, its test mocks
sourceAuditorModule.audit against the real AuditResult, source-audit gains a
tsconfig, and the five namespace accesses of IWaiverReader use the root import.

Audit 2026-09-11: cross-spec F-5, F-6, F-7, F-11; dependency #4, #7, #12, #15,
#18, #20; dep-red F1, F4; M3b. PM verdicts recorded in the audit record.
…ker for every live EARS

ScoringEngine was a pass-through with no EARS, tested under a [Step-5] ID that
exists in no spec, and exported as public API while the spec declared it
internal three times. Removed with its tests, its field and the Score step of
the pipeline; ScopeSelector stays exported (it has behaviour, EARS-A1..A5, and
saas-api reasons about it). PM verdict, 2026-09-13.

Every live EARS of the module (36 of 38; C3/C4 are retired) now has a
// [EARS-XX] marker at the line that implements it — 37 of 38 had none.

EARS-F6's negative control loads an active waiver for a different fingerprint
instead of an empty list, so a hasWaiver that ignored the fingerprint turns it
red (measured: it did).

Audit 2026-09-11: dependency #13, #14; M1; H5.
…ssifies every built-in category

redact() carries finding.snippetHash byte for byte at every level and never
recomputes it (RLDX-B3 rewritten): createFinding computes it once, everyone
else transports it — the rule AUDIT-K5 established for fingerprint. redactSarif
keeps a transported gitgov/snippetHash and computes one only for SARIF that
arrived without it (RLDX-B9 precised); it used to overwrite the value
SarifBuilder had just transported.

RLDX-B4 is "omit the fixes key", not "set it to undefined": Finding.fixes?:
Fix[] admits no explicit undefined under exactOptionalPropertyTypes, and the
old assignment only compiled through an `as unknown as` that disabled every
check on the returned object. The cast is gone; build() takes a typed partial.

security-vulnerability (sensitive) and code-quality (safe) are classified —
two built-ins fell to defaultBehavior for months. RLDX-A6 iterates
BASE_FINDING_CATEGORIES and fails the day a built-in is added unclassified;
A2/A3 compare the lists by value. RedactionConfig types its lists as
FindingCategory[]. isSensitiveCategory is public so the E2E asks the module
instead of re-deriving one of its three steps.

Tests strengthened where mutation showed they could not fail: A5/B1/B5/B7
structural, B6 asserts full redaction, B11 both levels whole-object, C1..C4
behavioural, C5 all three fields. The CLI DI test wires the real redactor
instead of a no-op mock and asserts it redacts.

Negative controls run: five independent mutations (category removed, recompute
in redact, overwrite in redactSarif, severity ignored, fingerprint ignored)
turn 12 tests red; reverted, 3137/3137 green.

Audit 2026-09-11: cross-spec F-5, F-15, F-16, F-17; dep-red F2, F6, F7, F13,
F14; test-red F3, F6, F8, F9, F12, F13; M2, M3.
…ts own ID

[RLDX-E1] was written on five agent_runner and CLI sites to mean "the runner
is pure"; that requirement is ARUN-H3, and E1 is the caller's side. The sites
now carry ARUN-H3. The orchestrator test that covers RLDX-E3 is tagged with it,
so the delegation in redaction_module §4.5.2 is greppable from both ends.

Audit 2026-09-11: dep-red F10 (agent_runner part), F11.
…utions

The docstring said "Number of agents executed" while buildSummary computes
agentResults.filter(r => r.status === "success").length. The false wording
was copied into Decision 12 as agentsRun === agentsFailed, which is never true
when every agent fails; the condition is agentsRun === 0 && agentsFailed > 0.
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Sep 13, 2026

Copy link
Copy Markdown

Deploying monorepo with  Cloudflare Pages  Cloudflare Pages

Latest commit: 0a09a24
Status: ✅  Deploy successful!
Preview URL: https://74646c95.monorepo-2lb.pages.dev
Branch Preview URL: https://feat-finding-governance.monorepo-2lb.pages.dev

View logs

…missing runtime in the load warning (AORCH-G1)

Decision 12: with executeAgent propagating agent errors, a run where every
agent fails to load reached the evaluator with an empty list, got "pass"
(correct for an empty list) and exited 0 over a repository nobody scanned.
The CLI now exits 1 when summary.agentsRun === 0 && agentsFailed > 0 and says
so on the Exit code line. agentsRun counts SUCCESSES, so the first wording of
the decision, agentsRun === agentsFailed, was never true in that case.

Decision 13: AORCH-G1/G2 recognise RuntimeNotFoundError ("RuntimeNotFound:
<runtime>") next to the MODULE_NOT_FOUND family. A specialist registered with
runtime: 'typescript' fails that way in production, and until now ended in
status: error with no re-registration guidance.

The CLI e2e helper drops NODE_PATH for every spawned CLI: pnpm exported the
workspace's node_modules through it, so an agent not installed in the fixture
still resolved from the workspace and the audit e2e measured pnpm, not the
user's repo. 118/118 e2e green without it.
…e, REDACTED_SNIPPET

SarifBuilderOptions.redactionConfig (SARIF-O4) is removed: a third entry point
for the redaction policy, which now enters through FindingRedactor's
constructor only. redactionLevel stays — the export-from-DB path reconstructs
SARIF from rows without the orchestrator and redacts in the builder (O1..O3,
verified by RLDX-G4/G5). The Sarif.RedactionLevel alias goes with zero
consumers measured; RedactionLevel and RedactedFinding are root-exported
because consumers extend them.

isScanScope(value) is the runtime guard a persisted string goes through
instead of a cast (AUDIT-J4). REDACTED_SNIPPET is the one declaration of the
sentinel the SaaS projection compares against.

The public E2E redaction suite is tagged with the EARS each test asserts
(RLDX-B8, B10, G1; SARIF-N2, J1) instead of [CI1]..[CI4], which no spec knew.

Audit 2026-09-11 (delegated to 1.4, closed on the owner's order): dep-red F5,
F9, F12, F17; cross-spec F-6 (saas-api side).
…ntity and rule tables exported at the root

audit.prisma: snippetHash is required, like the canonical Finding.snippetHash
(AUDIT-K6) — the projection always writes it (RLDX-F2) and the saas-api
migration sets '' on legacy rows, the value RLDX-F4 reads as unverifiable.

The package root exports rehydrateFinding, computeFingerprint, normalizeAnchor
and FINGERPRINT_SCHEME as values (saas-api's projection and backfill call
them across the boundary), and the finding_detector barrel exports
HEURISTIC_PATTERNS so the backfill re-derives HEUR-* anchors with the same
rule the detector used (AP-K2).

finding_governance Task 1.3b.
… it upserts

Finding.snippetHash is NOT NULL since 4d8fe96 and the generated Prisma
input requires it; the two hand-written upserts in this public e2e were the
producers the private-side sweep did not cover. CI Typecheck was red on #171
from that commit; local recipe (prisma:generate + typecheck) is 0 again.
…d agents reported as failed

Review of the finding_governance major, before merge.

- AORCH-B5/F4: executeAgent and executeReviewAgent read the runner's
  resolved status "error" instead of reporting success with an empty
  SARIF. AORCH-C9 and the RuntimeNotFound branch of AORCH-G1 were
  unreachable in production; G1/G2 tests now mock the runner's real
  shape, and a CLI e2e drives a crashing agent through FsAgentRunner.
- AUDIT-K7: createFinding no longer hashes an empty, placeholder
  ("requires login") or redacted anchor as text. It falls back to the
  snippet, and without text to a region identity with its own scheme
  tag (gitgov-fp/2+pos) and a warning.
- AUDIT-K2: the preimage is JSON-encoded instead of "|"-joined, so a
  separator inside file or category cannot merge two identities.
- AUDIT-N1..N3: identifySarifResult, transportedSnippetHash and
  rehydrateSarifResult are the one way back from a SARIF result. The
  orchestrator and policy re-evaluation use them; a result without the
  key and without snippet text is discarded with a warning, and the
  transported snippetHash is kept (PEVAL-F6..F8, AORCH-B6/B12).
- AORCH-B15/AUDIT-L2: AuditSummary.unmatchedWaivers is number | null,
  null when the run did not cover every file a waiver can point at;
  source_auditor counts only waivers on files it read.
- AUDIT-J6 DETECTOR_NAMES/isDetectorName; EARS-34 heuristic patterns
  exported as source and flags; AUDIT-M1 reaches PolicyDecision.summary.
- security-audit and semgrep peer @gitgov/core >=5.0.0.

BREAKING CHANGE: AuditSummary.unmatchedWaivers is number | null;
HEURISTIC_PATTERNS values are { source, flags }; fingerprints computed
by earlier builds of this branch differ (no release carried them).
…ecisions

Comments added by this branch no longer reference dates, tasks, audit finding IDs or decision numbers, per the TypeScript preset. No behavior change.
…urrence, and a failed audit agent fails the step

- AUDIT-K8: a fingerprint is written <scheme>:<digest>; parseFingerprint,
  isCurrentFingerprint and fingerprintDigest read it. Short forms (CLI
  output, `audit waive <prefix>`) use the digest.
- AUDIT-L1/L3, AORCH-B16, EARS-C7: waivers under an earlier scheme are
  counted as outdated, apart from unmatched.
- EARS-35: every regex and heuristic rule yields two findings for two
  distinct occurrences in a file and one for a repeated one. SEC-003 anchors
  on the whole key block, PII-005 and HEUR-001 on the line, LOG-001,
  XFER-001, HEUR-002 and HEUR-003 through the end of the line. The
  HEURISTIC_PATTERNS export is gone with its only consumer.
- AORCH-C9: `gitgov audit` exits 1 when no audit agent was found or any
  audit agent failed.

BREAKING CHANGE: fingerprint values carry a scheme prefix; AuditSummary
gains outdatedWaivers; WaiverApplicationCounts gains outdated; a run with
a failed audit agent exits 1; HEURISTIC_PATTERNS is no longer exported.
…(EARS-35)

SEC-003 matched a key cut before its END marker lazily up to the next END
in the file, so a truncated key followed by a complete one produced one
finding spanning both keys. The block body now stops at another BEGIN;
the truncated key falls back to its bounded base64 tail and the complete
key keeps its own block and identity.

Also corrects a source_auditor comment that claimed the orchestrator's
no-agent early return counts unmatched waivers; it reports null.
…CH-C9)

Covers the review side of the exit rule: every audit agent completed, the
policy passed and a review agent failed, so the CI/CD step passes.

The exit assertions of AORCH-C2 and AORCH-C9 now read the last
process.exit call. The project guard in this suite calls process.exit(1)
before the audit runs, so toHaveBeenCalledWith(1) held whatever the
command decided; the block test stayed green with its exit mutated to 0.
Every test ran on a project the AORCH-P5 guard rejects. With process.exit
stubbed to return, the guard called exit(1) and the test went on along a
path production never takes, so any exit assertion on 1 held whatever the
command decided.

The suite now builds the command inside an initialized project; the
project guard tests build the missing project explicitly. Exit assertions
check a single call. The initialization error test no longer trips the
working repo guard, and the AORCH-D5 test uses a passing run with a
rejecting reporter instead of in-test vi.mock calls that are hoisted.
…HLP-A1, HLP-A4, HLP-A5)

runGitgovCli and spawnGitgovCli ran `gitgov` from PATH. On a machine with a
globally linked CLI that is whatever checkout was linked last, so E2E runs
from a worktree exercised another branch's code while the preflight checked
this checkout's build. Both helpers now run
node <checkout>/packages/cli/build/dist/gitgov.mjs, resolved by
resolveGitgovCli from the helpers' own real location, with a GITGOV_CLI_BIN
override, and log the binary's realpath once per process.

projection_audit no longer spawns `gitgov` on its own.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant