feat: 006 authority-aware cross-tool correlation 구현 - #273
Conversation
📝 WalkthroughWalkthroughThis change adds authority-aware SAST finding correlation. It defines shared contracts, persists scoped correlation ledgers with provenance, validates durable observation context, creates deterministic edges, supports exact replay, fences late batches, and exposes only the correlation service. ChangesSAST finding correlation
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Observations
participant SastFindingCorrelationService
participant PrismaSastFindingCorrelationStore
participant Prisma
Observations->>SastFindingCorrelationService: Submit observation results
SastFindingCorrelationService->>PrismaSastFindingCorrelationStore: Load and validate durable context
SastFindingCorrelationService->>SastFindingCorrelationService: Build deterministic edges and provenance
PrismaSastFindingCorrelationStore->>Prisma: Persist batch, sources, edges, provenance, and audit event
Prisma-->>PrismaSastFindingCorrelationStore: Return persisted counts or replay state
PrismaSastFindingCorrelationStore-->>SastFindingCorrelationService: Return correlated or rejected outcome
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d13250bb4d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (9)
apps/api/prisma/migrations/20260802120000_sast_finding_correlation/migration.sql (1)
298-311: 🗄️ Data Integrity & Integration | 🔵 TrivialDocument the mandatory online-schema step for the occurrence foreign keys.
This migration creates the batch-scope and edge-scope foreign keys, but not the occurrence foreign keys.
apps/api/scripts/apply-online-sast-runtime-schema.mjsaddsSastFindingCorrelationEdge_source_occurrence_scope_fkey,SastFindingCorrelationEdge_target_occurrence_scope_fkey, andSastFindingCorrelationProvenance_occurrence_scope_fkeyat lines 546-566, because they depend onSastFindingOccurrence_correlation_scope_key, which the same script creates concurrently at lines 89-94.Until that script runs, correlation edges and provenance rows have no referential integrity against
SastFindingOccurrence. Add a comment at the top of this migration that names the required follow-up script, so an operator who applies migrations alone sees the dependency.📝 Proposed comment header
+-- The correlation occurrence foreign keys are NOT created here. They depend on +-- "SastFindingOccurrence_correlation_scope_key", which is built concurrently by +-- apps/api/scripts/apply-online-sast-runtime-schema.mjs. Run that script after +-- this migration to complete referential integrity for correlation edges and +-- correlation provenance rows. CREATE TYPE "SastFindingCorrelationKind" AS ENUM (🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/prisma/migrations/20260802120000_sast_finding_correlation/migration.sql` around lines 298 - 311, Add a header comment at the top of this migration documenting that applying migrations alone does not create the occurrence foreign keys. Name apply-online-sast-runtime-schema.mjs as the mandatory follow-up script, and mention that it creates the SastFindingOccurrence correlation key and the three occurrence-scope foreign keys for SastFindingCorrelationEdge and SastFindingCorrelationProvenance.packages/shared/src/types/sast-finding-correlation.ts (1)
948-961: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winPrecompute the correlation-ID patterns instead of building a
RegExpper call.
isCorrelationIdcompiles a newRegExpon every invocation. The validators call it several times per edge and per provenance record, andSAST_FINDING_CORRELATION_LIMITS.maximumEdgesallows 100,000 edges. A frozen lookup table removes the repeated compilation.The
prefixparameter is a closed literal union and the function is not exported, so the ast-grep ReDoS hint is a false positive. The precomputed table also removes that hint.♻️ Proposed refactor
+const CORRELATION_ID_PATTERNS = Object.freeze({ + 'finding-observation': /^finding-observation:\/\/[a-f0-9]{64}$/u, + 'finding-occurrence': /^finding-occurrence:\/\/[a-f0-9]{64}$/u, + 'finding-lineage': /^finding-lineage:\/\/[a-f0-9]{64}$/u, + 'normalized-finding': /^normalized-finding:\/\/[a-f0-9]{64}$/u, + 'finding-correlation': /^finding-correlation:\/\/[a-f0-9]{64}$/u +}); + function isCorrelationId( value: unknown, - prefix: - | 'finding-observation' - | 'finding-occurrence' - | 'finding-lineage' - | 'normalized-finding' - | 'finding-correlation' + prefix: keyof typeof CORRELATION_ID_PATTERNS ): value is string { return ( typeof value === 'string' && - new RegExp(`^${prefix}://[a-f0-9]{64}$`, 'u').test(value) + CORRELATION_ID_PATTERNS[prefix].test(value) ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/shared/src/types/sast-finding-correlation.ts` around lines 948 - 961, Precompute the correlation-ID regular expressions in a frozen lookup table keyed by the literal prefixes, then update isCorrelationId to select the cached pattern instead of constructing a new RegExp per call. Preserve the existing string and 64-character hexadecimal validation behavior and the current prefix union.Source: Linters/SAST tools
packages/shared/test/sast-finding-correlation.test.mjs (1)
112-122: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a safety-only negative case that keeps the valid
edgeDigest.The current negative case changes
safetyandedgeDigesttogether. The forged digest alone makesisSastFindingCorrelationEdgeShapeValidreturnfalse, so the assertion does not prove thatsafetytampering is rejected.This matters because
canonicalizeSastFindingCorrelationEdgesubstitutescanonicalSafety()and ignores the caller'ssafetyobject. A tamperedsafetyfield therefore does not change the recomputed digest. OnlyisSafetyValidrejects it, and that path is currently untested.💚 Proposed additional assertion
assert.equal( isSastFindingCorrelationEdgeShapeValid( { ...edge, safety: { ...edge.safety, severityInheritanceAllowed: true }, edgeDigest: digest('forged') }, digest ), false ); + assert.equal( + isSastFindingCorrelationEdgeShapeValid( + { + ...edge, + safety: { ...edge.safety, severityInheritanceAllowed: true } + }, + digest + ), + false + ); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/shared/test/sast-finding-correlation.test.mjs` around lines 112 - 122, Update the negative test for isSastFindingCorrelationEdgeShapeValid to tamper only with safety.severityInheritanceAllowed while preserving the original valid edgeDigest. Keep the assertion expecting false, ensuring the test exercises safety validation rather than digest mismatch.apps/api/src/scan-plane/prisma-sast-finding-correlation.store.ts (2)
208-215: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAvoid the double cast on
this.prisma.
as unknown as CorrelationReaderremoves all structural checking.PrismaServiceextendsPrismaClient, which already supplies the four delegates inCorrelationReader. If a model is renamed inschema.prisma, this cast hides the break until runtime.Pass
this.prismadirectly, or use a singleas CorrelationReadercast so the compiler still verifies the shape.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/scan-plane/prisma-sast-finding-correlation.store.ts` around lines 208 - 215, Update loadContext to pass this.prisma directly to readContext, removing the as unknown as CorrelationReader double cast; if a cast is required, use only as CorrelationReader so structural delegate compatibility remains compiler-checked.
563-580: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winBound the occurrence read with
take.
expectedOccurrenceCountis checked againstSAST_FINDING_CORRELATION_LIMITS.maximumOccurrenceson Lines 557-562. That check uses thefindingCountcolumns of the observation batches. It does not bound the number ofsastFindingOccurrencerows the query returns. If the durable table holds more occurrence rows than the batch columns declare, this query materializes all of them inside a serializable transaction before Line 578 rejects.Add
take: expectedOccurrenceCount + 1. The count check on Line 578 then still rejects the mismatch, and the read stays bounded.♻️ Proposed change
select: OCCURRENCE_SELECT, orderBy: [ { observationBatchId: 'asc' }, { ordinal: 'asc' } - ] + ], + take: expectedOccurrenceCount + 1 });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/scan-plane/prisma-sast-finding-correlation.store.ts` around lines 563 - 580, Bound the occurrence query in the correlation flow by adding a take limit of expectedOccurrenceCount + 1 to the findMany call on sastFindingOccurrence. Keep the existing ordering and length-mismatch rejection unchanged so valid counts continue to work while oversized results are detected without materializing all rows.apps/api/src/scan-plane/sast-finding-correlation.service.ts (1)
322-324: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winLog unmapped errors before converting them to a rejection.
The catch converts every error into a rejection reason code.
mapStoreErrorreturnsFINDING_CORRELATION_PERSISTENCE_FAILEDfor any error it does not recognize. A programming defect, such as aTypeError, therefore becomes a silent, well-formed rejection. Persistence and correlation defects then produce no signal for operators.Inject a
Loggerand record the unrecognized error before you map it. Keep the rejection payload free of identifiers.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/scan-plane/sast-finding-correlation.service.ts` around lines 322 - 324, Update the catch block in the finding-correlation service to inject and use a Logger, logging errors that mapStoreError cannot recognize before returning the rejection. Preserve the existing mapped rejection behavior and ensure log messages contain no identifiers or sensitive correlation data.apps/api/test/scan-plane/sast-finding-correlation.e2e-spec.ts (1)
309-347: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDerive the occurrence count from
SAST_FINDING_CORRELATION_LIMITS.yieldOccurrenceInterval.The literal
65appears in the test name and in four assertions. The expectationyieldCount === 1holds only whileyieldOccurrenceIntervalequals64. If the shared limit changes, this test fails without indicating the cause.Import the limit and compute the count as
yieldOccurrenceInterval + 1. Compute the expectedexactFingerprintCountasyieldOccurrenceInterval.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/test/scan-plane/sast-finding-correlation.e2e-spec.ts` around lines 309 - 347, The test case around YieldCountingCorrelationService hardcodes the bounded occurrence count and expected exact fingerprint count. Import SAST_FINDING_CORRELATION_LIMITS, derive the fixture, source/observation counts, and occurrence array length from yieldOccurrenceInterval + 1, and derive exactFingerprintCount from yieldOccurrenceInterval while preserving the yieldCount assertion.apps/api/test/scan-plane/prisma-sast-finding-correlation.store.e2e-spec.ts (1)
309-323: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftAdd coverage for
readContextand the in-transaction re-read.
mockCorrelationContextstubs the privatereadContextmethod.correlateInTransactioncallsreadContexta second time inside the transaction and compares the result withinput.contextthroughsameContext(apps/api/src/scan-plane/prisma-sast-finding-correlation.store.ts, Lines 230-236). Stubbing the method disables both the durable read and that TOCTOU fence.As a result, no test in this file exercises
readContext(Lines 426-640). That function enforces the plan-digest binding, the attempt stage and scan-request status checks, the exact observation-set completeness check, andverifySourceCapabilities. The linked issue requires these authority checks.Add at least one test that supplies the observation, occurrence, and attempt delegates on the transaction double and lets
readContextrun. Add one negative case where the in-transaction re-read returns a changed context, and assertSastFindingCorrelationDurableScopeError.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/test/scan-plane/prisma-sast-finding-correlation.store.e2e-spec.ts` around lines 309 - 323, Add tests in the correlation store suite that avoid stubbing readContext, provide observation, occurrence, and attempt delegates on the transaction double, and exercise its authority checks. Cover a successful durable read and a negative correlateInTransaction case where the in-transaction re-read differs from input.context, asserting SastFindingCorrelationDurableScopeError; retain mockCorrelationContext only for tests that do not require this coverage.apps/api/test/support/sast-finding-correlation-fixtures.ts (1)
149-179: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDerive the profile capabilities and the capability order from the shared contracts.
Two constants duplicate shared source data:
- Lines 169-179 hardcode
requiredCapabilitiesper profile.contextAuthorityMatchesProfileinapps/api/src/scan-plane/sast-finding-correlation.service.ts(Lines 658-670) compares this list for exact ordered equality againstSAST_SCAN_PROFILES[profileId].requiredCapabilitieswithSBOMremoved. If a profile definition changes, these tests fail withFINDING_CORRELATION_AUTHORITY_INVALIDinstead of a clear fixture error.- Lines 407-415 re-declare the capability order.
SAST_CAPABILITIESalready defines it, andisCanonicalCapabilitiesinapps/api/src/scan-plane/prisma-sast-finding-correlation.store.ts(Lines 1322-1339) derives the order from that constant.indexOfalso returns-1for an unknown capability, which sorts it first without an error.Derive both from the shared constants.
♻️ Proposed refactor
+const FINDING_CAPABILITY_ORDER = SAST_CAPABILITIES.filter( + (capability) => capability !== 'SBOM' +); + function capabilityOrder(left: string, right: string): number { - const order = [ - 'SAST', - 'DEPENDENCY_VULNERABILITY', - 'SECRET_DETECTION', - 'IAC_MISCONFIGURATION' - ]; - return order.indexOf(left) - order.indexOf(right); + const leftIndex = FINDING_CAPABILITY_ORDER.indexOf(left as never); + const rightIndex = FINDING_CAPABILITY_ORDER.indexOf(right as never); + if (leftIndex < 0 || rightIndex < 0) { + throw new Error(`Unknown correlation fixture capability.`); + } + return leftIndex - rightIndex; }Apply the same approach to
requiredCapabilities:- requiredCapabilities: options.supportingOpenGrep - ? [ - 'DEPENDENCY_VULNERABILITY', - 'SECRET_DETECTION', - 'IAC_MISCONFIGURATION' - ] - : [ - 'SAST', - 'DEPENDENCY_VULNERABILITY', - 'SECRET_DETECTION' - ], + requiredCapabilities: SAST_SCAN_PROFILES[ + profileId + ].requiredCapabilities.filter( + (capability) => capability !== 'SBOM' + ),Also applies to: 407-415
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/test/support/sast-finding-correlation-fixtures.ts` around lines 149 - 179, Update the fixture’s profile setup to derive requiredCapabilities directly from SAST_SCAN_PROFILES[profileId].requiredCapabilities, removing SBOM as required by the correlation contract, instead of hardcoding per-profile arrays. Update the capability ordering logic near the fixture’s canonical capability handling to reuse SAST_CAPABILITIES and validate unknown capabilities rather than sorting indexOf(-1) values first; preserve the shared contract’s exact ordering.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/api/src/scan-plane/prisma-sast-finding-lineage.store.ts`:
- Around line 246-263: The correlation fence in the transaction flow must fail
closed when transaction.sastFindingCorrelationBatch is unavailable. Update the
correlationDelegate guard to throw SastFindingLineageReplayConflictError when
the delegate is absent; when present, continue querying findFirst with the
existing scope filters and throw the same error when a matching record exists.
---
Nitpick comments:
In
`@apps/api/prisma/migrations/20260802120000_sast_finding_correlation/migration.sql`:
- Around line 298-311: Add a header comment at the top of this migration
documenting that applying migrations alone does not create the occurrence
foreign keys. Name apply-online-sast-runtime-schema.mjs as the mandatory
follow-up script, and mention that it creates the SastFindingOccurrence
correlation key and the three occurrence-scope foreign keys for
SastFindingCorrelationEdge and SastFindingCorrelationProvenance.
In `@apps/api/src/scan-plane/prisma-sast-finding-correlation.store.ts`:
- Around line 208-215: Update loadContext to pass this.prisma directly to
readContext, removing the as unknown as CorrelationReader double cast; if a cast
is required, use only as CorrelationReader so structural delegate compatibility
remains compiler-checked.
- Around line 563-580: Bound the occurrence query in the correlation flow by
adding a take limit of expectedOccurrenceCount + 1 to the findMany call on
sastFindingOccurrence. Keep the existing ordering and length-mismatch rejection
unchanged so valid counts continue to work while oversized results are detected
without materializing all rows.
In `@apps/api/src/scan-plane/sast-finding-correlation.service.ts`:
- Around line 322-324: Update the catch block in the finding-correlation service
to inject and use a Logger, logging errors that mapStoreError cannot recognize
before returning the rejection. Preserve the existing mapped rejection behavior
and ensure log messages contain no identifiers or sensitive correlation data.
In `@apps/api/test/scan-plane/prisma-sast-finding-correlation.store.e2e-spec.ts`:
- Around line 309-323: Add tests in the correlation store suite that avoid
stubbing readContext, provide observation, occurrence, and attempt delegates on
the transaction double, and exercise its authority checks. Cover a successful
durable read and a negative correlateInTransaction case where the in-transaction
re-read differs from input.context, asserting
SastFindingCorrelationDurableScopeError; retain mockCorrelationContext only for
tests that do not require this coverage.
In `@apps/api/test/scan-plane/sast-finding-correlation.e2e-spec.ts`:
- Around line 309-347: The test case around YieldCountingCorrelationService
hardcodes the bounded occurrence count and expected exact fingerprint count.
Import SAST_FINDING_CORRELATION_LIMITS, derive the fixture, source/observation
counts, and occurrence array length from yieldOccurrenceInterval + 1, and derive
exactFingerprintCount from yieldOccurrenceInterval while preserving the
yieldCount assertion.
In `@apps/api/test/support/sast-finding-correlation-fixtures.ts`:
- Around line 149-179: Update the fixture’s profile setup to derive
requiredCapabilities directly from
SAST_SCAN_PROFILES[profileId].requiredCapabilities, removing SBOM as required by
the correlation contract, instead of hardcoding per-profile arrays. Update the
capability ordering logic near the fixture’s canonical capability handling to
reuse SAST_CAPABILITIES and validate unknown capabilities rather than sorting
indexOf(-1) values first; preserve the shared contract’s exact ordering.
In `@packages/shared/src/types/sast-finding-correlation.ts`:
- Around line 948-961: Precompute the correlation-ID regular expressions in a
frozen lookup table keyed by the literal prefixes, then update isCorrelationId
to select the cached pattern instead of constructing a new RegExp per call.
Preserve the existing string and 64-character hexadecimal validation behavior
and the current prefix union.
In `@packages/shared/test/sast-finding-correlation.test.mjs`:
- Around line 112-122: Update the negative test for
isSastFindingCorrelationEdgeShapeValid to tamper only with
safety.severityInheritanceAllowed while preserving the original valid
edgeDigest. Keep the assertion expecting false, ensuring the test exercises
safety validation rather than digest mismatch.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f3b5df04-3be0-49cd-b609-d27bed345698
📒 Files selected for processing (26)
apps/api/prisma/migrations/20260802120000_sast_finding_correlation/migration.sqlapps/api/prisma/schema.prismaapps/api/scripts/apply-online-sast-runtime-schema.mjsapps/api/src/scan-plane/prisma-sast-finding-correlation.store.tsapps/api/src/scan-plane/prisma-sast-finding-lineage.store.tsapps/api/src/scan-plane/sast-finding-correlation.service.tsapps/api/src/scan-plane/sast-finding-correlation.store.tsapps/api/src/scan-plane/scan-plane.module.tsapps/api/test/scan-plane/prisma-sast-finding-correlation.store.e2e-spec.tsapps/api/test/scan-plane/sast-finding-correlation-persistence.e2e-spec.tsapps/api/test/scan-plane/sast-finding-correlation.e2e-spec.tsapps/api/test/scan-plane/sast-finding-lineage-persistence.e2e-spec.tsapps/api/test/support/sast-finding-correlation-fixtures.tspackages/shared/src/index.tspackages/shared/src/types/sast-finding-correlation.tspackages/shared/test/sast-finding-correlation.test.mjsspecs/006-production-sast-runtime-design/contracts/sast-runtime.mdspecs/006-production-sast-runtime-design/data-model.mdspecs/006-production-sast-runtime-design/plan.mdspecs/006-production-sast-runtime-design/quality-gates.mdspecs/006-production-sast-runtime-design/quickstart.mdspecs/006-production-sast-runtime-design/research.mdspecs/006-production-sast-runtime-design/spec.mdspecs/006-production-sast-runtime-design/tasks.mdspecs/006-production-sast-runtime-design/threat-model.mdtest/github-actions/active-feature.test.mjs
|
CodeRabbit 리뷰 본문의 nitpick을 현재 계약과 대조해 처리했습니다. 적용한 유효 지적 8건 (
적용하지 않은 제안:
검증: workspace lint/test/typecheck/build, Prisma validate, runtime layout, |
🎋 작업 중인 브랜치 및 이슈
feat/272-006-finding-correlation🔎 주요 변경 사항
sast-finding-correlation-v1shared 계약과 canonical source-set/batch/basis/provenance/edge/result/rejection digest 및 exact shape validator를 추가했습니다.AUTHORITATIVE | SUPPORTING_ONLY권한을 platform-owned 방식으로 결정합니다.EXACT_FINGERPRINT, canonical ecosystem/package/installed-version/CVE 기반SAME_DEPENDENCY_CVE, cross-capability CVE/same-file CWE 기반SUPPORTING_EVIDENCE | POSSIBLE_OVERLAP만 허용합니다.ScanPlaneModule은 T037을 내부화하고 T038 correlation service만 T039에 export하며 coverage/evidence/policy/publication/AI authority와 사용자 route는 계속 차단합니다.✅ 컨벤션 확인
type/issue-number-short-feature형식을 따르나요?<type>: <description>형식을 따르나요?Check List
검증
corepack pnpm lintcorepack pnpm test(API 91 suites / 561 tests 포함 전체 workspace 통과)corepack pnpm typecheckcorepack pnpm buildcorepack pnpm --filter @aegisai/api prisma:validatenode --test test/runtime/*.test.mjsgit diff --checkCI) 성공006 진행 상태
Closes #272
Summary by CodeRabbit
New Features
Documentation
Tests