From 098bf4be05ccd687e5d5caa4e8b8b81642624c39 Mon Sep 17 00:00:00 2001 From: ajmallesh Date: Thu, 27 Aug 2026 18:40:31 -0700 Subject: [PATCH] fix(sast): align Capella export with the submit-time code-path contract The export gate required every code_paths entry to be file:line, but submit only requires the primary sink to be file:line and accepts bare trace steps. A single malformed trace step therefore dropped an otherwise-valid finding at export. - add isValidPrimaryCodePath as the one shared primary-sink contract - validate only the primary at export; buildResult already drops unusable steps - route the submit-time validator through the same helper so the two cannot drift --- apps/worker/src/ai/sast/capella/collectors.ts | 9 ++++++--- apps/worker/src/ai/sast/capella/paths.ts | 11 +++++++++++ apps/worker/src/ai/sast/capella/sarif-exporter.ts | 15 ++++++++------- 3 files changed, 25 insertions(+), 10 deletions(-) diff --git a/apps/worker/src/ai/sast/capella/collectors.ts b/apps/worker/src/ai/sast/capella/collectors.ts index 809e38cd..585121d4 100644 --- a/apps/worker/src/ai/sast/capella/collectors.ts +++ b/apps/worker/src/ai/sast/capella/collectors.ts @@ -55,7 +55,7 @@ import { type TriageChecklist, type TriageRuleKey, } from './finding-types.js'; -import { isNormalizedRepositoryPath, parseCodePath } from './paths.js'; +import { isValidPrimaryCodePath, parseCodePath } from './paths.js'; // === Result Helpers === @@ -265,8 +265,7 @@ export function validateCodePaths( error: `code_paths[0] must be a sink location ":", not a URL (got ${JSON.stringify(first)}).`, }; } - const parsed = parseCodePath(first); - if (!parsed || !isNormalizedRepositoryPath(parsed.file)) { + if (!isValidPrimaryCodePath(first)) { return { ok: false, error: @@ -275,6 +274,10 @@ export function validateCodePaths( 'contain traversal, be a bare file, a symbol or an offset.', }; } + const parsed = parseCodePath(first); + if (!parsed) { + return { ok: false, error: 'code_paths[0] must be a ":" locator.' }; + } return { ok: true, codePaths, file: parsed.file, line: parsed.line }; } diff --git a/apps/worker/src/ai/sast/capella/paths.ts b/apps/worker/src/ai/sast/capella/paths.ts index cf58de32..54af6f5f 100644 --- a/apps/worker/src/ai/sast/capella/paths.ts +++ b/apps/worker/src/ai/sast/capella/paths.ts @@ -21,6 +21,17 @@ export function parseCodePath(entry: string): ParsedCodePath | undefined { return { file, line }; } +/** + * The primary (sink) code-path contract, shared by the submit-time collector and the + * export gate so the two ends enforce the same rule and cannot drift. `code_paths[0]` + * becomes the finding's SARIF location, so it must be a normalized repository-relative + * `file:line`. Trace steps are held to no such requirement on either side. + */ +export function isValidPrimaryCodePath(entry: string): boolean { + const parsed = parseCodePath(entry); + return parsed !== undefined && isNormalizedRepositoryPath(parsed.file); +} + /** * The normalized repository-relative POSIX path contract shared by SARIF * locations and code-path scoping. Rejects absolute, drive-letter, encoded, diff --git a/apps/worker/src/ai/sast/capella/sarif-exporter.ts b/apps/worker/src/ai/sast/capella/sarif-exporter.ts index 585948b9..46ac9bda 100644 --- a/apps/worker/src/ai/sast/capella/sarif-exporter.ts +++ b/apps/worker/src/ai/sast/capella/sarif-exporter.ts @@ -22,7 +22,7 @@ import type { AgenticSastOmission, AgenticSastReduction, SarifRef } from '../typ import { atomicPublishBytes, sha256Bytes, stableJson } from './artifacts.js'; import { SastContractError } from './errors.js'; import type { CapellaFinding, CapellaSeverity } from './finding-types.js'; -import { isExcludedCodePath, isNormalizedRepositoryPath, parseCodePath } from './paths.js'; +import { isExcludedCodePath, isNormalizedRepositoryPath, isValidPrimaryCodePath, parseCodePath } from './paths.js'; import { renderCapellaReport } from './report.js'; import type { AtomicPublishOptions } from './types.js'; import { isCapellaFinding } from './validation.js'; @@ -102,14 +102,15 @@ function classifyExportCandidate( if (!isCapellaFinding(value)) { return { omission: buildOmission(value, 'invalid_finding_record') }; } - if (value.code_paths.length === 0) { + const primary = value.code_paths[0]; + if (value.code_paths.length === 0 || primary === undefined) { return { omission: buildOmission(value, 'missing_code_path') }; } - const codePathsAreValid = value.code_paths.every((entry) => { - const parsed = parseCodePath(entry); - return parsed !== undefined && isNormalizedRepositoryPath(parsed.file); - }); - if (!codePathsAreValid) { + // Only the primary (sink) must be a `file:line` locator: it becomes the SARIF location. + // Trace steps are held to no such rule at submit time, and `buildResult` already drops any + // that are not `file:line`, so gating on them here would discard a whole finding for a + // malformed step the two ends never agreed to require. + if (!isValidPrimaryCodePath(primary)) { return { omission: buildOmission(value, 'invalid_code_path') }; } return { finding: value };