Skip to content

fix(classifiers): preserve finding kind through report intake - #238

Merged
smith-xyz merged 2 commits into
developmentfrom
feat/classifier-finding-kind-7_13hll2
Oct 27, 2026
Merged

smith-xyz merged 2 commits into
developmentfrom
feat/classifier-finding-kind-7_13hll2

Conversation

@chubirka

@chubirka chubirka commented Oct 27, 2026 •

Copy link
Copy Markdown
Member

Classifier plan 05 — finding_kind only

Target development, not main. Required reviewer @smith-xyz. Keep the merge hold; no auto-merge.

Problem and fix

A schema-valid report with kind: weakness records successfully, but the report -> FindingCandidate -> FindingIntake path discards the declared kind and stores null. The actual Context/services reproduction failed before this change and passes afterward.

Carry the existing optional finding_kind through the existing candidate and intake owner. Explicit report kinds populate only a newly created finding; an omitted kind remains unknown and a later observation does not overwrite an existing finding's classification. Validate the kind at the actual intake boundary, including model_copy-mutated batches, before any rows are added.

No new vocabulary, artifact-schema or database-shape change, migration, policy or parallel normalizer. The existing FindingCandidate gains the optional kind already declared on reports and Finding rows. No validity answer, evidence grade or signature is inferred from a finding kind. Existing triage and review owners remain unchanged.

Acceptance

  • Real report recording preserves weakness/vulnerability/omission and exact artifact bytes, does not create decisions, and replay keeps the assessment identity.
  • Copied candidates with hardening/unknown kinds are refused without partial batch rows.
  • A later unclassified observation preserves the stored kind.
  • Installed App reporting record exercised container and RPM roles on fresh SQLite and PostgreSQL: kinds preserved, omission unknown, no invented decision, both replays stable, invalid kind exits 2 without adding an assessment, source bytes unchanged.

Changed tests

Existing expectations are unchanged; only behavioral regressions were added. README documents the existing-owner behavior. No query/schema/performance shape change.

Rollout

Merge Core first, then the paired App producer/adoption PR. App's existing direct Core override avoids an unrelated Engine pin PR. This completes this finding-kind writer batch, not every plan 05 family or the whole classifier plan.

Completed gates

Gate Result
Core unit suite 6874 passed, 10 skipped
Core PostgreSQL integration suite 2418 passed, 3 skipped
Core coverage 97.02% (80% required)
Adopting App full suite, PostgreSQL enabled 2786 passed, 2 skipped
App generated reference, docs/license and version checks Passed

Core unit/coverage execution uses a controlled tool PATH so the existing missing-YARA refusal scenario genuinely lacks that binary, while retaining Git/GPG/cosign/Go/Node. Existing expectations were not edited or selectively skipped. PostgreSQL integration uses explicit UTC session options. With a non-UTC default session, the unmodified freshness age model separately rendered a 150-day assessment as 151 days; that unrelated view issue is not bundled into this finding-kind PR. The actual installed finding-kind CLI scenarios also passed against the original database-session configuration.

Current-development reconciliation

Retains Core development through ae5585d29933530562bbee297555600abbab321b, including storage-free spend/product lookup and error-detail redaction. Reconciled Core candidate: 0b9e50f6378ccc725b07dab6ded8194dd4a118eb. Repeated installed App CLI acceptance on both databases passed with that exact candidate. Paired adoption: App #293.

@chubirka
chubirka requested a review from smith-xyz as a code owner October 27, 2026 19:07
@chubirka chubirka added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 27, 2026
@chubirka

Copy link
Copy Markdown
Member Author

Paired producer/adoption PR: traust-security/traust#293 (development). It pins this exact Core candidate and has passed the installed container/RPM CLI scenarios on SQLite/PostgreSQL plus the full PostgreSQL-enabled App suite (2713 passed, 2 skipped). Merge Core #238 first, then App #293, after Shaun review; both are review-ready with explicit merge holds and no auto-merge. No Engine PR is required because the App already owns the direct Core override. This is the finding-kind batch of plan 05, not a claim of complete classifier rollout.

@smith-xyz

Copy link
Copy Markdown
Contributor

lgtm

@smith-xyz
smith-xyz enabled auto-merge October 27, 2026 19:21
@chubirka

Copy link
Copy Markdown
Member Author

Final reconciled review head: 0b9e50f, retaining development through ae5585d. Core unit gates: 6874 passed, 10 skipped; PostgreSQL integration (explicit UTC sessions): 2418 passed, 3 skipped; coverage: 97.02%. The installed App container/RPM record paths passed again on SQLite and PostgreSQL with this exact Core commit. Paired App #293 now retains development through a5c5c3d, is conflict-free, and passes 2786 tests with 2 documented non-database skips. Core #238 is also mergeable; review/merge holds and Shaun review requests remain. Merge Core first, then App. The unrelated non-UTC freshness issue and test-environment boundaries are explicit in the PR body; no unrelated policy/source fix was bundled.

@smith-xyz
smith-xyz merged commit 5e06eb9 into development Oct 27, 2026
6 checks passed
@chubirka

Copy link
Copy Markdown
Member Author

Post-merge correction is separately reviewable in Core #242: #242. Duplicate fingerprints now reconcile their explicit kind before insertion and reject conflicting kinds atomically, without overwriting previously stored findings. New order-permutation regressions failed before and all 20 cases passed after across SQLite/PostgreSQL. Full corrected gates: 6884 unit passed/10 skipped, 2428 PostgreSQL passed/3 skipped, 97.02% coverage; installed container/RPM CLI permutations and conflict refusals passed on both databases. App #293 pins the correction and must wait for Core #242, not merely this already-merged #238.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants