diff --git a/plugins/codex-security/mcp-app/src/artifact-deep-reducer.ts b/plugins/codex-security/mcp-app/src/artifact-deep-reducer.ts index 4782fc485..b8e147ed9 100644 --- a/plugins/codex-security/mcp-app/src/artifact-deep-reducer.ts +++ b/plugins/codex-security/mcp-app/src/artifact-deep-reducer.ts @@ -6,9 +6,7 @@ import scanDraftSchema from "../../schemas/tools/scan-draft.schema.json"; import type { ArtifactContext, DeepReducerContext } from "./artifact-context.js"; import { parsePersistedScanDraft, - parseScanDraft, saveScanDraftCheckpoint, - type ScanDraftInput } from "./artifact-scan-draft.js"; import { loadArtifactZodSchema, @@ -21,7 +19,12 @@ import { writeJsonAtomic, type DeepScanArtifacts } from "./deep-scan/artifacts.js"; -import { reconcileDeepReduction } from "./deep-scan/artifact-validation.js"; +import { + parseDeepReduction, + reconcileDeepReduction, + type DeepReductionInput, + type DeepReductionSources, +} from "./deep-scan/artifact-validation.js"; const schemaDocuments = [ commonSchema, @@ -39,15 +42,7 @@ export const deepReductionInputSchema = loadArtifactZodSchema( schemaDocuments, reducerSchema.$id, "reductionInput" -) as ZodType; - -interface DeepReducerInputs { - discoveries: { - workerId: string; - result: ScanDraftInput; - }[]; - previous: ScanDraftInput | null; -} +) as ZodType; interface BoundReducer { artifacts: DeepScanArtifacts; @@ -56,10 +51,10 @@ interface BoundReducer { scanId?: string; } -/** Return complete Standard results without exposing their artifact locations. */ +/** Read the findings and scan context assigned to this reducer. */ export async function getCodexSecurityDeepReducerInputs( context: ArtifactContext -): Promise { +): Promise { return withLogicalReducerErrors(context, async () => { const bound = bindDeepReducer(context); const discoveries = await Promise.all(bound.state.claimedWorkers.map(async (worker) => { @@ -67,7 +62,8 @@ export async function getCodexSecurityDeepReducerInputs( const result = parseStoredScanDraft( await readJsonObject(worker.resultPath), "Accepted Standard worker " + worker.id, - bound.scanId + bound.scanId, + parsePersistedScanDraft ); if (result.complete === false) throw new Error("An assigned Standard worker wrote only a checkpoint, not a complete result."); result.findings = result.findings.map((finding, index) => ({ @@ -77,7 +73,8 @@ export async function getCodexSecurityDeepReducerInputs( sourceFindingIds: [`${worker.id}:${index}`], }, })); - return { workerId: worker.id, result }; + const { coverage: _coverage, ...reduction } = result; + return { workerId: worker.id, result: reduction }; })); const previous = await readPreviousReduction(bound); const scanId = bound.scanId ?? previous?.scanId ?? discoveries[0]?.result.scanId; @@ -97,7 +94,7 @@ export async function getCodexSecurityDeepReducerInputs( }); } -/** Validate and durably replace this reducer's complete semantic Standard result. */ +/** Check and save the reducer's finished result. */ export async function recordCodexSecurityDeepReduction( context: ArtifactContext, input: unknown @@ -107,7 +104,8 @@ export async function recordCodexSecurityDeepReduction( }> { return withLogicalReducerErrors(context, async () => { const bound = bindDeepReducer(context); - let reduction = parseScanDraft(input as ScanDraftInput); + const submitted = deepReductionInputSchema.parse(input); + let reduction = parseDeepReduction(submitted); if (reduction.complete === false) throw new Error("Deep reduction is only a checkpoint, not a complete result."); const inputs = await getCodexSecurityDeepReducerInputs(context); const expectedScanId = bound.scanId @@ -164,25 +162,27 @@ function bindDeepReducer(context: ArtifactContext): BoundReducer { async function readPreviousReduction( bound: BoundReducer -): Promise { +): Promise { const { previousReducerResultPath } = bound.state; if (!previousReducerResultPath) return null; await requireRegularFile(previousReducerResultPath, bound.artifacts.dedupRoot); return parseStoredScanDraft( await readJsonObject(previousReducerResultPath), "The previous accepted Deep reduction", - bound.scanId + bound.scanId, + (value) => parseDeepReduction(value, true) ); } -function parseStoredScanDraft( +function parseStoredScanDraft( value: Record, label: string, - expectedScanId?: string -): ScanDraftInput { - let parsed: ScanDraftInput; + expectedScanId: string | undefined, + parse: (input: Record) => Result +): Result { + let parsed: Result; try { - parsed = parsePersistedScanDraft(value); + parsed = parse(value); } catch (error) { const detail = error instanceof Error ? error.message : String(error); throw new Error(label + " has an invalid Standard scan result: " + detail, { cause: error }); diff --git a/plugins/codex-security/mcp-app/src/artifact-scan-draft.ts b/plugins/codex-security/mcp-app/src/artifact-scan-draft.ts index 5b9ce3504..4c560822c 100644 --- a/plugins/codex-security/mcp-app/src/artifact-scan-draft.ts +++ b/plugins/codex-security/mcp-app/src/artifact-scan-draft.ts @@ -57,7 +57,7 @@ interface PreparedScanDraft { type PublishScanDraft = ( draft: PreparedScanDraft, - expectedDigest: string, + expectedDigest: string | undefined, checkpoint: ScanDraftInput, ) => Promise; @@ -88,7 +88,11 @@ export async function recordCodexSecurityScanDraft( for (;;) { signal?.throwIfAborted(); - const preserved = await preserveScanDraft(context, parsed, false); + // Deep results are ready to save. Do not merge older drafts or + // checkpoints into them. + const preserved = context.mode === "deep" && parsed.complete !== false + ? { input: parsed, previousDigest: undefined } + : await preserveScanDraft(context, parsed, false); const reconciled = preserved.input; const contract = requireObject( context.targetContract, @@ -104,7 +108,7 @@ export async function recordCodexSecurityScanDraft( ); const target = buildTarget(context, contract, trustedTarget); const scope = buildScope(context, trustedScope, reconciled.scope); - const findings = buildFindings(reconciled.findings); + const findings = buildFindings(reconciled.findings, context.mode); const coverage = buildCoverage( context, contract, @@ -190,9 +194,10 @@ export async function recordCodexSecurityScanDraftViaWorkbench( draftPath, "--checkpoint-path", checkpointPath, - "--expected-draft-digest", - expectedDigest, ]; + if (expectedDigest !== undefined) { + arguments_.push("--expected-draft-digest", expectedDigest); + } if (context.handoffClaimToken) { arguments_.push("--claim-token", context.handoffClaimToken); } @@ -268,7 +273,7 @@ export async function recordCodexSecurityWorkerScanDraft( /** Keep the semantic input before any replaceable worker or canonical artifact. */ export async function saveScanDraftCheckpoint( context: ArtifactContext, - input: ScanDraftInput, + input: Omit, updateHead = true, ): Promise { const { handoffClaimToken: _claim, ...snapshot } = input; @@ -514,7 +519,7 @@ async function readCurrentCheckpoints( return checkpoints.map(({ input }) => input); } -function scanDraftCheckpointName(input: ScanDraftInput): string { +function scanDraftCheckpointName(input: Omit): string { const { handoffClaimToken: _claim, ...snapshot } = input; return createHash("sha256").update(JSON.stringify(snapshot)).digest("hex") + ".json"; } @@ -1388,7 +1393,7 @@ function buildScope( }; } -function buildFindings(findings: JsonObject[]): JsonObject[] { +function buildFindings(findings: JsonObject[], mode?: string): JsonObject[] { const generatedIdentities = findings.map((finding, index) => { if (finding.identity !== undefined) return undefined; const candidateId = (finding.extensions as JsonObject | undefined) @@ -1421,7 +1426,7 @@ function buildFindings(findings: JsonObject[]): JsonObject[] { ); } - return findings.map((finding, index) => { + const identified: JsonObject[] = findings.map((finding, index) => { const generatedIdentity = generatedIdentities[index]; if (generatedIdentity === undefined) return { ...finding }; const identity: JsonObject = { anchor: generatedIdentity.anchor }; @@ -1441,6 +1446,36 @@ function buildFindings(findings: JsonObject[]): JsonObject[] { identity, }; }); + if (mode !== "deep") return identified; + + // Keep both findings when workers reuse an ID. + // Add a numeric suffix to make each ID unique. + const reserved = new Set(identified.map(scanFindingIdentity)); + const used = new Set(); + return identified.map((finding) => { + const key = scanFindingIdentity(finding); + if (!used.has(key)) { + used.add(key); + return finding; + } + const identity = finding.identity as JsonObject; + const baseInstance = identity.instance ?? "saved"; + let suffix = 2; + const distinct: JsonObject & { identity: JsonObject } = { + ...finding, identity: { ...identity }, + }; + do { + distinct.identity.instance = `${baseInstance}-${suffix}`; + suffix += 1; + } while (reserved.has(scanFindingIdentity(distinct)) || used.has(scanFindingIdentity(distinct))); + const provenance = finding.provenance as JsonObject; + distinct.provenance = { + ...provenance, + preservedIdentity: provenance.preservedIdentity ?? structuredClone(identity), + }; + used.add(scanFindingIdentity(distinct)); + return distinct; + }); } function buildCoverage( diff --git a/plugins/codex-security/mcp-app/src/deep-scan/artifact-validation.ts b/plugins/codex-security/mcp-app/src/deep-scan/artifact-validation.ts index e26dd1cb3..291557a9e 100644 --- a/plugins/codex-security/mcp-app/src/deep-scan/artifact-validation.ts +++ b/plugins/codex-security/mcp-app/src/deep-scan/artifact-validation.ts @@ -1,8 +1,8 @@ import { isDeepStrictEqual } from "node:util"; import { parsePersistedScanDraft, + parseScanDraft, preserveFindingDetails, - preserveScanCoverage, saveScanDraftCheckpoint, scanFindingIdentity, type ScanDraftInput, @@ -10,13 +10,39 @@ import { import { readJsonObject, requireRegularFile, writeJsonAtomic } from "./artifacts.js"; import type { DeepScanArtifacts } from "./artifacts.js"; +export type DeepReductionInput = Omit; + export interface DeepReductionSources { - discoveries: { workerId: string; result: ScanDraftInput }[]; - previous: ScanDraftInput | null; + discoveries: { workerId: string; result: DeepReductionInput }[]; + previous: DeepReductionInput | null; } export interface ReducerArtifactValidation { newFindings: number; + result: DeepReductionInput; +} + +/** + * Check reducer findings with the Standard scan validator. + * It requires coverage, so add an empty value and remove it after validation. + */ +export function parseDeepReduction( + input: Record, + persisted = false, +): DeepReductionInput { + const standard = { + ...input, + coverage: { + completeness: "complete", + surfaces: [], + explicitExclusions: [], + deferred: [], + }, + }; + const { coverage: _coverage, ...parsed } = persisted + ? parsePersistedScanDraft(standard) + : parseScanDraft(standard as unknown as ScanDraftInput); + return parsed; } /** Admit exactly the complete semantic result written by an ordinary Standard scan. */ @@ -29,7 +55,8 @@ export async function validateDiscoveryArtifacts( const result = parseStoredScanDraft( await readJsonObject(resultPath), "Standard scan worker", - expectedScanId + expectedScanId, + parsePersistedScanDraft ); if (result.complete === false) throw new Error("Standard scan worker wrote only a checkpoint; its audit is not complete."); return result; @@ -56,7 +83,8 @@ export async function validateReducerArtifacts(input: { let result = parseStoredScanDraft( await readJsonObject(resultPath), reducerId, - expectedScanId + expectedScanId, + (value) => parseDeepReduction(value, true) ); if (result.complete === false) throw new Error("Deep reduction wrote only a checkpoint; its audit is not complete."); @@ -66,7 +94,8 @@ export async function validateReducerArtifacts(input: { previous = parseStoredScanDraft( await readJsonObject(previousReducerResultPath), "Previous successful reducer", - result.scanId + result.scanId, + (value) => parseDeepReduction(value, true) ); } @@ -79,6 +108,7 @@ export async function validateReducerArtifacts(input: { } const previousFindingIds = new Set((previous?.findings ?? []).map(scanFindingIdentity)); return { + result, newFindings: result.findings.filter((finding) => ( !previousFindingIds.has(scanFindingIdentity(finding)) )).length @@ -87,10 +117,10 @@ export async function validateReducerArtifacts(input: { /** Reconcile a reducer output against the immutable inputs captured before dispatch. */ export function reconcileDeepReduction( - input: ScanDraftInput, + input: DeepReductionInput, discoveries: DeepReductionSources["discoveries"], - previous: ScanDraftInput | null, -): ScanDraftInput { + previous: DeepReductionInput | null, +): DeepReductionInput { const result = structuredClone(input); if (result.complete === false) throw new Error("Deep reduction is only a checkpoint, not a complete result."); for (const source of [...discoveries.map((discovery) => discovery.result), ...(previous ? [previous] : [])]) { @@ -117,10 +147,6 @@ export function reconcileDeepReduction( } } retainSourceFindings(result, { discoveries, previous }); - result.coverage = preserveScanCoverage(result.coverage, [ - ...discoveries.map((discovery) => discovery.result.coverage), - ...(previous ? [previous.coverage] : []), - ]); if (result.threatModel === undefined) { const sourceModels = [ ...discoveries.map((discovery) => discovery.result.threatModel), @@ -167,7 +193,7 @@ function findingSourceIds(finding: Record): string[] { )); } -function retainSourceFindings(result: ScanDraftInput, inputs: DeepReductionSources): void { +function retainSourceFindings(result: DeepReductionInput, inputs: DeepReductionSources): void { type Finding = Record; const sources = new Map(); for (const discovery of inputs.discoveries) { @@ -215,9 +241,9 @@ function retainSourceFindings(result: ScanDraftInput, inputs: DeepReductionSourc /** Preserve previously accepted identities and never discard every reported finding. */ export function validateRetainedFindings( - result: ScanDraftInput, - sources: ScanDraftInput[], - previous?: ScanDraftInput + result: DeepReductionInput, + sources: DeepReductionInput[], + previous?: DeepReductionInput ): void { if ( result.findings.length === 0 @@ -238,14 +264,15 @@ export function validateRetainedFindings( } } -function parseStoredScanDraft( +function parseStoredScanDraft( value: Record, label: string, - expectedScanId?: string -): ScanDraftInput { - let parsed: ScanDraftInput; + expectedScanId: string | undefined, + parse: (input: Record) => Result +): Result { + let parsed: Result; try { - parsed = parsePersistedScanDraft(value); + parsed = parse(value); } catch (error) { const detail = error instanceof Error ? error.message : String(error); throw new Error(label + " returned an invalid Standard scan result: " + detail, { diff --git a/plugins/codex-security/mcp-app/src/deep-scan/coordinator.ts b/plugins/codex-security/mcp-app/src/deep-scan/coordinator.ts index 5e0d985b1..905c0c9a8 100644 --- a/plugins/codex-security/mcp-app/src/deep-scan/coordinator.ts +++ b/plugins/codex-security/mcp-app/src/deep-scan/coordinator.ts @@ -3,13 +3,10 @@ import { promises as fs } from "node:fs"; import { basename, dirname, join } from "node:path"; import { createDeepScanArtifacts, - ensureDeepScanDirectories, - readJsonObject, - requireRegularFile + ensureDeepScanDirectories } from "./artifacts.js"; -import { validateDiscoveryArtifacts, validateReducerArtifacts } from "./artifact-validation.js"; +import { validateDiscoveryArtifacts, validateReducerArtifacts, type DeepReductionInput } from "./artifact-validation.js"; import { - parsePersistedScanDraft, scanDraftInputSchema, type ScanDraftInput } from "../artifact-scan-draft.js"; @@ -51,13 +48,16 @@ type SchedulerSettlement = | { status: "fulfilled"; outcome: SchedulerOutcome } | { status: "rejected"; error: unknown }; +type AcceptedReducer = Omit; + interface SchedulerResult { reason: DeepScanTerminalReason; omittedWorkerIds: string[]; canceledWorkerIds: string[]; accepted: AcceptedDiscovery[]; mergedWorkerIds: string[]; - reducers: SuccessfulDedupOutcome[]; + reducers: AcceptedReducer[]; + result?: DeepReductionInput; } type CoordinatorPhase = "setup" | "discovery" | "terminal"; @@ -68,7 +68,7 @@ interface SchedulerAudit { omittedWorkerIds: string[]; canceledWorkerIds: string[]; bufferedWorkerIds: string[]; - reducers: SuccessfulDedupOutcome[]; + reducers: AcceptedReducer[]; executions: WorkerExecutionAudit[]; } @@ -292,9 +292,18 @@ export class DeepScanCoordinator { const schedulerResult = await this.runScheduler(); if (this.canceled || this.externallyFailed) return; this.phase = "terminal"; - const latestReducer = schedulerResult.reducers.at(-1); - const draft = latestReducer - ? parsePersistedScanDraft(await readJsonObject(latestReducer.resultPath)) + const draft = schedulerResult.result + ? { + ...structuredClone(schedulerResult.result), + // Readers require coverage.json. The coordinator has accepted this + // result, so mark it complete and leave review notes empty. + coverage: { + completeness: "complete", + surfaces: [], + explicitExclusions: [], + deferred: [] + } + } : scanDraftInputSchema.parse({ scanId: this.state.scanId, findings: [], @@ -605,7 +614,9 @@ export class DeepScanCoordinator { this.audit.mergedWorkerIds = mergedDiscoveries.map((worker) => worker.id); this.audit.canceledWorkerIds = [...canceledWorkerIds]; this.audit.executions = await this.recoverPersistedExecutions(); - const reducerOutcomes = await this.recoverCompletedReducers(recovered); + const recoveredReducers = await this.recoverCompletedReducers(recovered); + const reducerOutcomes = recoveredReducers.reducers; + let latestResult = recoveredReducers.result; let buffer: AcceptedDiscovery[] = recovered.filter((worker) => !mergedIds.has(worker.id)); let reducer: Promise | undefined; let previousReducerResultPath = reducerOutcomes.at(-1)?.resultPath; @@ -745,7 +756,9 @@ export class DeepScanCoordinator { this.state = outcome.run; previousReducerResultPath = outcome.resultPath; mergedDiscoveries.push(...outcome.consumed); - reducerOutcomes.push(outcome); + const { result: acceptedResult, ...metadata } = outcome; + latestResult = acceptedResult; + reducerOutcomes.push(metadata); this.audit.reducers = [...reducerOutcomes]; this.audit.mergedWorkerIds = unique(mergedDiscoveries.map((worker) => worker.id)); this.audit.bufferedWorkerIds = buffer.map((worker) => worker.id); @@ -907,7 +920,9 @@ export class DeepScanCoordinator { this.state = outcome.run; previousReducerResultPath = outcome.resultPath; mergedDiscoveries.push(...outcome.consumed); - reducerOutcomes.push(outcome); + const { result: acceptedResult, ...metadata } = outcome; + latestResult = acceptedResult; + reducerOutcomes.push(metadata); this.audit.reducers = [...reducerOutcomes]; this.audit.mergedWorkerIds = unique(mergedDiscoveries.map((worker) => worker.id)); this.audit.bufferedWorkerIds = buffer.map((worker) => worker.id); @@ -934,7 +949,8 @@ export class DeepScanCoordinator { this.audit.canceledWorkerIds = unique(canceledWorkerIds); this.audit.bufferedWorkerIds = buffer.map((worker) => worker.id); - if (lateFailure) throw lateFailure; + // Once Deep reaches saturation, late worker errors cannot fail the scan. + if (lateFailure && stopReason !== "saturated") throw lateFailure; if ( !previousReducerResultPath @@ -948,7 +964,8 @@ export class DeepScanCoordinator { canceledWorkerIds: unique(canceledWorkerIds), accepted, mergedWorkerIds: unique(mergedDiscoveries.map((worker) => worker.id)), - reducers: reducerOutcomes + reducers: reducerOutcomes, + result: latestResult, }; } @@ -981,10 +998,11 @@ export class DeepScanCoordinator { private async recoverCompletedReducers( discoveries: AcceptedDiscovery[] - ): Promise { + ): Promise<{ reducers: AcceptedReducer[]; result?: DeepReductionInput }> { const discoveriesById = new Map(discoveries.map((worker) => [worker.id, worker])); const inputs = this.state.persistedDedupInputs ?? []; - const outcomes: SuccessfulDedupOutcome[] = []; + const outcomes: AcceptedReducer[] = []; + let latestResult: DeepReductionInput | undefined; const completedReducers = (this.state.persistedWorkers ?? []) .filter((worker) => worker.kind === "dedup" && worker.status === "succeeded") .sort((left, right) => ( @@ -1004,13 +1022,14 @@ export class DeepScanCoordinator { throw new Error(`Completed reducer ${worker.id} has incomplete persisted inputs.`); } const accepted = consumed as AcceptedDiscovery[]; - const { newFindings } = await validateReducerArtifacts({ + const { newFindings, result } = await validateReducerArtifacts({ artifacts: this.artifacts, artifactDir: worker.artifactDir, resultPath: worker.resultManifestPath, reducerId: worker.id, previousReducerResultPath: outcomes.at(-1)?.resultPath }, this.state.scanId); + latestResult = result; noNewStreak = newFindings > 0 ? 0 : noNewStreak + accepted.length; const evidence = await persistedWorkerEvidence(worker); outcomes.push({ @@ -1025,7 +1044,7 @@ export class DeepScanCoordinator { run: { ...this.state, noNewStreak } }); } - return outcomes; + return { reducers: outcomes, result: latestResult }; } private async recoverPersistedExecutions(): Promise { diff --git a/plugins/codex-security/mcp-app/src/deep-scan/worker-runner.ts b/plugins/codex-security/mcp-app/src/deep-scan/worker-runner.ts index 4c13886ca..82f119979 100644 --- a/plugins/codex-security/mcp-app/src/deep-scan/worker-runner.ts +++ b/plugins/codex-security/mcp-app/src/deep-scan/worker-runner.ts @@ -6,7 +6,7 @@ import { validateDiscoveryArtifacts, validateReducerArtifacts } from "./artifact-validation.js"; -import type { ReducerArtifactValidation } from "./artifact-validation.js"; +import type { DeepReductionInput, ReducerArtifactValidation } from "./artifact-validation.js"; import { archiveDirectory, discoveryArtifacts, @@ -62,6 +62,7 @@ export interface SuccessfulDedupOutcome { id: string; consumed: AcceptedDiscovery[]; resultPath: string; + result: DeepReductionInput; newFindings: number; attempt: number; threadId?: string; @@ -442,6 +443,7 @@ export class DeepScanWorkerRunner { id: reducerId, consumed, resultPath, + result: reducerValidation.result, newFindings: reducerValidation.newFindings, attempt: outcome.attempt, threadId: outcome.threadId, @@ -796,7 +798,7 @@ function standardScanCompletionContinuation(attempt: number): string { function reducerCompletionContinuation(attempt: number): string { return [ `Continue the existing Deep Scan reducer after attempt ${attempt} ended without its required result.`, - "Use your existing Standard scan analysis and call record_codex_security_deep_reduction({ scanId, findings, coverage, threatModel?, scope? }).", + "Submit the aggregate with record_codex_security_deep_reduction({ scanId, findings, threatModel?, scope? }).", "If the tool rejects the arguments, use its error to correct them and retry the call until it succeeds.", "Do not end your turn, write the result directly, or call the tool again after it succeeds." ].join("\n"); diff --git a/plugins/codex-security/mcp-app/src/server/compact-artifact-tools.ts b/plugins/codex-security/mcp-app/src/server/compact-artifact-tools.ts index f9a3dbe1c..69e931e76 100644 --- a/plugins/codex-security/mcp-app/src/server/compact-artifact-tools.ts +++ b/plugins/codex-security/mcp-app/src/server/compact-artifact-tools.ts @@ -271,7 +271,7 @@ export function registerCompactWorkerArtifactTools( registerCompactTool(server, { name: "get_codex_security_deep_reducer_inputs", title: "Get Codex Security Deep Reducer Inputs", - description: "Read the complete Standard scan results assigned to this reducer.", + description: "Read the assigned findings, context, and previous aggregate.", inputSchema: deepReducerInputsInputSchema, readOnly: true, handler: async () => getCodexSecurityDeepReducerInputs(context) @@ -280,12 +280,12 @@ export function registerCompactWorkerArtifactTools( registerCompactTool(server, { name: "record_codex_security_deep_reduction", title: "Record Codex Security Deep Reduction", - description: "Record this reducer's complete aggregated Standard scan result.", + description: "Record the merged findings and context for this Deep scan.", inputSchema: deepReductionInputSchema, readOnly: false, handler: async (value) => recordCodexSecurityDeepReduction( context, - value as ScanDraftInput + value ) }); } diff --git a/plugins/codex-security/mcp-app/templates/deep-scan/dedup.md b/plugins/codex-security/mcp-app/templates/deep-scan/dedup.md index b32585c22..5f6271a29 100644 --- a/plugins/codex-security/mcp-app/templates/deep-scan/dedup.md +++ b/plugins/codex-security/mcp-app/templates/deep-scan/dedup.md @@ -6,7 +6,7 @@ Use this exact reducer configuration: {{DEDUP_CONTEXT_JSON}} ``` -Call `get_codex_security_deep_reducer_inputs({})` to read each assigned worker's complete, already-validated Standard scan result and the previous aggregate, if any. The scan identity, findings, coverage, threat model, and scope already use the Standard semantic scan-draft contract. +Call `get_codex_security_deep_reducer_inputs({})` to read the findings and context from each assigned, already-validated Standard scan and the previous aggregate, if any. Read every finding and the previous aggregate. Merge only the same actionable root issue using remediation-subsumption: fixing the retained finding must also fix every absorbed finding. Preserve distinct reachable vulnerable instances, proof tuples, useful evidence, uncertainty, locations, provenance, severity, validation, attack paths, and remediation. @@ -16,6 +16,6 @@ For a valid merge, synthesize one stronger finding while preserving every materi Account for every input finding using the host-supplied `provenance.sourceFindingIds`. Copy the refs for retained findings; union them for a valid merge, preserving previous refs. Never invent, omit, or reuse a ref across independent output findings. The host retains original source payloads and rejects unaccounted input. Identity collisions do not establish that findings are duplicates. -Combine the complete coverage, exclusions, deferred work, open questions, threat-model context, and optional scope from the Standard results without dropping meaningful information. Keep coverage partial whenever any deferred work or follow-up surface requires it. You cannot resolve or reject a source finding without inspecting code, which is outside this reducer's role. +Preserve the threat-model context and scope as needed. You cannot resolve or reject a source finding without inspecting code, which is outside this reducer's role. -Call `record_codex_security_deep_reduction({ scanId, findings, coverage, threatModel?, scope? })` with one complete Standard semantic result until it succeeds; correct a reported validation error and retry in the same conversation. After the first successful call, do not call it again. The host derives convergence and worker attribution from its existing state. +Call `record_codex_security_deep_reduction({ scanId, findings, threatModel?, scope? })` until it succeeds; correct a reported validation error and retry in the same conversation. After the first successful call, do not call it again. The host derives convergence and worker attribution from its existing state. diff --git a/plugins/codex-security/mcp-app/tests/deep_scan_publication_cases.mjs b/plugins/codex-security/mcp-app/tests/deep_scan_publication_cases.mjs new file mode 100644 index 000000000..5e44d1796 --- /dev/null +++ b/plugins/codex-security/mcp-app/tests/deep_scan_publication_cases.mjs @@ -0,0 +1,183 @@ +import assert from "node:assert/strict"; +import { readFile, rm, writeFile } from "node:fs/promises"; +import path from "node:path"; + +export async function testDeepScanPublication({ + fixtureRun, FakeStore, FakeExecutor, DeepScanCoordinator, deferred, + immediateClock, eventually, +}) { + async function testSaturationOmitsWorkerAcceptedDuringCancellation() { + const fixture = await fixtureRun({ workers: 3, subagents: 0, stopAfterNoNew: 2, maxDiscoveryRuns: 3 }); + const store = new FakeStore(fixture.run); + const releaseLateWorker = deferred(); + const lateAcceptance = deferred(); + const releaseAcceptance = deferred(); + const updateWorker = store.updateWorker.bind(store); + let acceptedLateWorker; + store.updateWorker = async (update) => { + const persisted = await updateWorker(update); + if (update.kind === "discovery" && update.status === "succeeded" + && path.basename(path.dirname(update.promptPath)) === "discovery-0003") { + acceptedLateWorker = persisted; + // This worker finishes too late to be included in the final result. + await rm(update.resultManifestPath); + lateAcceptance.resolve(); + await releaseAcceptance.promise; + } + return persisted; + }; + const executor = new FakeExecutor({ + blockDedup: true, + discoveryGates: { "discovery-0003": releaseLateWorker.promise }, + discoveryCandidates: { "discovery-0003": "late-accepted-finding" }, + }); + const completed = []; + const coordinator = new DeepScanCoordinator({ + run: fixture.run, store, executor, pluginRoot: fixture.pluginRoot, + clock: immediateClock, + onComplete: async (draft) => completed.push(structuredClone(draft)), + }); + coordinator.start(); + await executor.dedupStarted; + releaseLateWorker.resolve(); + await lateAcceptance.promise; + executor.releaseDedup(); + await eventually(() => executor.dedupSignal?.aborted === true); + releaseAcceptance.resolve(); + const terminal = await coordinator.wait(undefined, 5_000); + assert.equal(terminal?.status, "succeeded", terminal?.error); + assert.equal(terminal.terminalReason, "saturated"); + assert.equal(executor.discoveryCalls, 3); + assert.equal(executor.dedupCalls, 1, "late accepted results do not restart convergence"); + assert.deepEqual(store.finishCalls[0].omittedWorkerIds, [acceptedLateWorker.id]); + assert.equal(completed.length, 1); + assert.equal(completed[0].coverage.completeness, "complete"); + assert.deepEqual(completed[0].findings, [], "late worker findings are not appended to the saturated aggregate"); + } + + async function testSuccessfulDeepCoverageIgnoresWorkerAndReducerReviewStatus() { + const fixture = await fixtureRun({ workers: 2, subagents: 0, stopAfterNoNew: 2, maxDiscoveryRuns: 2 }); + const store = new FakeStore(fixture.run); + const executor = new FakeExecutor(); + const run = executor.run.bind(executor); + const reviewed = { label: "Reviewed query", disposition: "no_issue_found" }; + const workerReviewed = { ...reviewed, receiptRefs: ["artifacts/missing-worker-receipt.md"] }; + const followUp = { label: "Worker follow-up", disposition: "needs_follow_up" }; + executor.run = async (request) => { + const outcome = await run(request); + const resultPath = path.join(request.artifactContext.root, "result.json"); + const draft = JSON.parse(await readFile(resultPath, "utf8")); + draft.coverage = { + completeness: request.kind === "discovery" && request.promptPath.includes("discovery-0002") + ? "unknown" : "partial", + surfaces: [workerReviewed, followUp], + explicitExclusions: [], + deferred: [{ reason: "An independent review left this question unresolved." }], + }; + await writeFile(resultPath, JSON.stringify(draft)); + return outcome; + }; + const completed = []; + const coordinator = new DeepScanCoordinator({ + run: fixture.run, store, executor, pluginRoot: fixture.pluginRoot, + clock: immediateClock, + onComplete: async (draft) => completed.push(structuredClone(draft)), + }); + coordinator.start(); + const terminal = await coordinator.wait(undefined, 5_000); + assert.equal(terminal?.status, "succeeded", terminal?.error); + assert.equal(completed.length, 1); + assert.deepEqual(completed[0].coverage, { + completeness: "complete", surfaces: [], explicitExclusions: [], deferred: [], + }); + for (const worker of store.workers.values()) { + if (worker.kind !== "discovery") continue; + const draft = JSON.parse(await readFile(worker.resultManifestPath, "utf8")); + assert.notEqual(draft.coverage.completeness, "complete"); + assert.deepEqual(draft.coverage.surfaces, [workerReviewed, followUp]); + assert.equal(draft.coverage.deferred.length, 1); + } + } + + async function testSaturationIgnoresDiscoveryCancellationWriteFailure() { + const fixture = await fixtureRun({ workers: 2, subagents: 0, stopAfterNoNew: 2, maxDiscoveryRuns: 6 }); + const store = new FakeStore(fixture.run); + const executor = new FakeExecutor({ blockDedup: true, blockDiscoveryAfterCalls: 2 }); + const updateWorker = store.updateWorker.bind(store); + const rejectedCancellations = new Set(); + store.updateWorker = async (update) => { + if (update.kind === "discovery" && update.status === "canceled") { + assert.equal(executor.dedupSignal?.aborted, true); + assert.equal(store.run.noNewStreak, 2); + rejectedCancellations.add(update.id); + throw new Error("fixture cancellation persistence failure"); + } + return updateWorker(update); + }; + const completed = []; + const coordinator = new DeepScanCoordinator({ + run: fixture.run, store, executor, pluginRoot: fixture.pluginRoot, + clock: immediateClock, + onComplete: async (draft) => completed.push(structuredClone(draft)), + }); + coordinator.start(); + await executor.dedupStarted; + await eventually(() => executor.discoveryCalls === 4 && executor.runningDiscovery === 2); + executor.releaseDedup(); + + const terminal = await coordinator.wait(undefined, 5_000); + assert.equal(rejectedCancellations.size, 2, "the redundant discoveries reached the failing cancellation write"); + assert.equal(terminal?.status, "succeeded", terminal?.error); + assert.equal(terminal.terminalReason, "saturated"); + assert.equal(store.failCalls, 0); + assert.equal(store.finishCalls.length, 1); + assert.equal(store.finishCalls[0].reason, "saturated"); + assert.equal(executor.discoveryCalls, 4, "cancellation persistence failures must not dispatch replacement reviews after saturation"); + assert.equal(executor.dedupCalls, 1, "cancellation persistence failures must not restart reduction after saturation"); + assert.equal(executor.runningDiscovery, 0); + assert.equal(completed.length, 1); + assert.equal(completed[0].coverage.completeness, "complete"); + const acceptedReducer = [...store.workers.values()].find((worker) => ( + worker.kind === "dedup" && worker.status === "succeeded" + )); + const { coverage, ...publishedReduction } = completed[0]; + assert.deepEqual( + publishedReduction, + JSON.parse(await readFile(acceptedReducer.resultManifestPath, "utf8")), + "the accepted aggregate still reaches publication when redundant cancellation writes fail", + ); + } + + async function testPublicationUsesAcceptedReducerSnapshot() { + const fixture = await fixtureRun({ workers: 1, subagents: 0, stopAfterNoNew: 1, maxDiscoveryRuns: 1 }); + const store = new FakeStore(fixture.run); + const commitDedup = store.commitDedup.bind(store); + store.commitDedup = async (commit) => { + const accepted = await commitDedup(commit); + await rm(commit.resultManifestPath); + return accepted; + }; + store.finish = async (input) => { + store.finishCalls.push(input); + Object.assign(store.run, { status: "succeeded", terminalReason: input.reason, manifestPath: input.manifestPath }); + return structuredClone(store.run); + }; + const completed = []; + const coordinator = new DeepScanCoordinator({ + run: fixture.run, store, + executor: new FakeExecutor({ discoveryCandidateId: "accepted-finding" }), + pluginRoot: fixture.pluginRoot, clock: immediateClock, + onComplete: async (draft) => completed.push(structuredClone(draft)), + }); + coordinator.start(); + const terminal = await coordinator.wait(undefined, 5_000); + assert.equal(terminal?.status, "succeeded", terminal?.error); + assert.equal(completed[0].findings[0].provenance.candidateId, "accepted-finding"); + assert.equal(completed[0].coverage.completeness, "complete"); + } + + await testSaturationOmitsWorkerAcceptedDuringCancellation(); + await testSuccessfulDeepCoverageIgnoresWorkerAndReducerReviewStatus(); + await testSaturationIgnoresDiscoveryCancellationWriteFailure(); + await testPublicationUsesAcceptedReducerSnapshot(); +} diff --git a/plugins/codex-security/mcp-app/tests/test_artifact_deep_reducer.mjs b/plugins/codex-security/mcp-app/tests/test_artifact_deep_reducer.mjs index 2dd7c4cac..5466999cc 100644 --- a/plugins/codex-security/mcp-app/tests/test_artifact_deep_reducer.mjs +++ b/plugins/codex-security/mcp-app/tests/test_artifact_deep_reducer.mjs @@ -1,5 +1,5 @@ import assert from "node:assert/strict"; -import { mkdir, mkdtemp, readFile, realpath, rm, writeFile } from "node:fs/promises"; +import { mkdir, mkdtemp, readFile, readdir, realpath, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import path from "node:path"; import { build } from "esbuild"; @@ -22,17 +22,19 @@ const { ); const scanId = "7fc17317-9594-49e0-b06a-d72fd7e14bba"; -const validDraft = draft([]); +const validReduction = reduction([]); assert.equal(deepReducerInputsInputSchema.safeParse({}).success, true); assert.equal(deepReducerInputsInputSchema.safeParse({ path: "/tmp" }).success, false); -assert.equal(deepReductionInputSchema.safeParse(validDraft).success, true); +assert.equal(deepReductionInputSchema.safeParse(validReduction).success, true); +assert.equal(deepReductionInputSchema.safeParse(workerDraft([])).success, false, + "Deep reducer submissions no longer accept coverage"); assert.equal( - deepReductionInputSchema.safeParse({ ...validDraft, resultPath: "/tmp" }).success, + deepReductionInputSchema.safeParse({ ...validReduction, resultPath: "/tmp" }).success, false ); assert.equal( - deepReductionInputSchema.safeParse({ ...validDraft, consumedWorkerIds: ["spoofed"] }).success, + deepReductionInputSchema.safeParse({ ...validReduction, consumedWorkerIds: ["spoofed"] }).success, false ); assert.equal( @@ -54,12 +56,24 @@ try { const shared = finding("shared", "src/shared.ts"); const independent = finding("independent", "src/independent.ts"); + const rejectedCoverage = { + completeness: "partial", + surfaces: [ + { label: "SQL route", disposition: "rejected", notes: "Parameterized queries prevent injection.", + receiptRefs: ["artifacts/missing-worker-receipt.md"] }, + { label: "Archive upload", disposition: "needs_follow_up", notes: "The guard still needs review." }, + ], + explicitExclusions: [{ pattern: "vendor", reason: "Outside the requested source scope." }], + deferred: [{ candidateId: "candidate-upload", reason: "Review the guard.", paths: ["src/upload.ts"] }], + openQuestions: ["Does the alternate upload handler use the guard?"], + }; const first = await createWorker({ workersRoot, label: "discovery-0001", id: "worker-001", - result: draft([shared], { - threatModel: { summary: "Requests may reach shared code." } + result: workerDraft([shared], { + threatModel: { summary: "Requests may reach shared code." }, + coverage: rejectedCoverage, }), completionSequence: 1 }); @@ -67,11 +81,15 @@ try { workersRoot, label: "discovery-0002", id: "worker-002", - result: draft([shared, independent], { - scope: { summary: "Shared and independent request handling." } + result: workerDraft([shared, independent], { + scope: { summary: "Shared and independent request handling." }, + coverage: { ...workerDraft([]).coverage, completeness: "unknown" }, }), completionSequence: 2 }); + const originalWorkerArtifacts = await Promise.all( + [first, second].map((worker) => readFile(worker.resultPath, "utf8")), + ); const outputRoot = path.join(dedupRoot, "dedup-0001", "output"); await mkdir(outputRoot, { recursive: true }); const context = { @@ -87,15 +105,28 @@ try { const inputs = await getCodexSecurityDeepReducerInputs(context); await assert.rejects( - recordCodexSecurityDeepReduction(context, draft([shared])), + recordCodexSecurityDeepReduction(context, reduction([], { complete: false })), + /only a checkpoint/, + "a reducer submission must contain a complete result", + ); + await assert.rejects( + recordCodexSecurityDeepReduction(context, reduction([shared])), /unaccounted|discarded.*finding/, "a successful reduction must account for every fresh finding, not just one", ); await assert.rejects( - recordCodexSecurityDeepReduction(context, draft([shared, independent, finding("invented", "src/unreviewed.ts")])), + recordCodexSecurityDeepReduction(context, reduction([shared, independent, finding("invented", "src/unreviewed.ts")])), /no assigned source finding/, "a reducer cannot introduce an unvalidated finding outside its assigned sources", ); + await assert.rejects( + recordCodexSecurityDeepReduction(context, reduction([ + { ...shared, validation: { evidenceRefs: ["missing-evidence"] } }, + independent, + ], { complete: true })), + /evidenceRefs must refer/, + "live reducer submissions reject unknown evidence references instead of silently removing them", + ); assert.deepEqual(inputs, { discoveries: [ { workerId: first.id, result: withSourceRefs(first) }, @@ -106,27 +137,21 @@ try { assert.equal(JSON.stringify(inputs).includes(root), false); assert.equal(JSON.stringify(inputs).includes("result.json"), false); - const invalidCoverage = { - ...first.result, - coverage: { - ...first.result.coverage, - deferred: [{ reason: "A related path needs follow-up." }] - } - }; - await assert.rejects( - recordCodexSecurityDeepReduction(context, invalidCoverage), - /complete coverage cannot contain deferred/ - ); await assert.rejects( readFile(path.join(outputRoot, "result.json"), "utf8"), { code: "ENOENT" } ); await assert.rejects( - recordCodexSecurityDeepReduction(context, draft([])), + readdir(path.join(outputRoot, "checkpoints")), + { code: "ENOENT" }, + "invalid reducer submissions do not save a checkpoint", + ); + await assert.rejects( + recordCodexSecurityDeepReduction(context, reduction([])), /discarded every accepted Standard scan finding/ ); - const merged = draft([shared, independent], { + const merged = reduction([shared, independent], { threatModel: { summary: "Requests reach shared and independent code." }, scope: { summary: "Shared and independent request handling." } }); @@ -146,38 +171,24 @@ try { JSON.parse(await readFile(path.join(outputRoot, "result.json"), "utf8")), mergedWithSources ); + const checkpointNames = await readdir(path.join(outputRoot, "checkpoints")); + assert.equal(checkpointNames.length, 1); + assert.deepEqual( + JSON.parse(await readFile(path.join(outputRoot, "checkpoints", checkpointNames[0]), "utf8")), + mergedWithSources, + "reducer checkpoints retain the accepted findings and scope without coverage", + ); - const rejectedCoverage = { - completeness: "partial", - surfaces: [ - { label: "SQL route", disposition: "rejected", notes: "Parameterized queries prevent injection." }, - { label: "Archive upload", disposition: "needs_follow_up", notes: "The guard still needs review." }, - ], - explicitExclusions: [{ pattern: "vendor", reason: "Outside the requested source scope." }], - deferred: [{ candidateId: "candidate-upload", reason: "Review the guard.", paths: ["src/upload.ts"] }], - openQuestions: ["Does the alternate upload handler use the guard?"], - }; - const coverageWorker = await createWorker({ - workersRoot, label: "discovery-coverage", id: "worker-coverage", - result: draft([], { coverage: rejectedCoverage }), completionSequence: 4, - }); - const coverageRoot = path.join(dedupRoot, "dedup-coverage", "output"); - await mkdir(coverageRoot, { recursive: true }); - const coverageContext = { - ...context, root: coverageRoot, - deepReducer: { scanRoot, claimedWorkers: [coverageWorker] }, - }; - await recordCodexSecurityDeepReduction(coverageContext, draft([])); assert.deepEqual( - JSON.parse(await readFile(path.join(coverageRoot, "result.json"), "utf8")).coverage, - rejectedCoverage, - "reduction preserves rejection reasons and unfinished review even when the model omits them", + await Promise.all([first, second].map((worker) => readFile(worker.resultPath, "utf8"))), + originalWorkerArtifacts, + "reduction must not rewrite raw Standard worker coverage evidence", ); const collision = { ...independent, ruleId: shared.ruleId, identity: shared.identity }; const collisionWorker = await createWorker({ workersRoot, label: "discovery-collision", id: "worker-collision", - result: draft([shared, collision]), completionSequence: 5, + result: workerDraft([shared, collision]), completionSequence: 5, }); const collisionRoot = path.join(dedupRoot, "dedup-collision", "output"); await mkdir(collisionRoot, { recursive: true }); @@ -185,13 +196,13 @@ try { ...context, root: collisionRoot, deepReducer: { scanRoot, claimedWorkers: [collisionWorker] }, }; - await assert.rejects(recordCodexSecurityDeepReduction(collisionContext, draft([shared])), /ambiguous|unaccounted/); + await assert.rejects(recordCodexSecurityDeepReduction(collisionContext, reduction([shared])), /ambiguous|unaccounted/); const collisionInputs = await getCodexSecurityDeepReducerInputs(collisionContext); const sourceFindingIds = collisionInputs.discoveries[0].result.findings.flatMap( finding => finding.provenance.sourceFindingIds, ); assert.deepEqual(sourceFindingIds, ["worker-collision:0", "worker-collision:1"]); - await recordCodexSecurityDeepReduction(collisionContext, draft([{ + await recordCodexSecurityDeepReduction(collisionContext, reduction([{ ...shared, provenance: { ...shared.provenance, sourceFindingIds }, }])); const collisionOutput = JSON.parse(await readFile(path.join(collisionRoot, "result.json"), "utf8")); @@ -199,7 +210,7 @@ try { { id: "worker-collision:0", finding: shared }, { id: "worker-collision:1", finding: collision }, ]); - await assert.rejects(recordCodexSecurityDeepReduction(collisionContext, draft([{ + await assert.rejects(recordCodexSecurityDeepReduction(collisionContext, reduction([{ ...shared, provenance: { ...shared.provenance, sourceFindingIds: ["unassigned:0"] }, }])), /unknown source finding/); await assert.rejects( @@ -211,7 +222,7 @@ try { workersRoot, label: "discovery-0003", id: "worker-003", - result: draft([shared]), + result: workerDraft([shared]), completionSequence: 3 }); const nextOutputRoot = path.join(dedupRoot, "dedup-0002", "output"); @@ -233,7 +244,7 @@ try { previous: mergedWithSources }); await assert.rejects( - recordCodexSecurityDeepReduction(nextContext, draft([shared])), + recordCodexSecurityDeepReduction(nextContext, reduction([shared])), (error) => error.code === "merge_traceability_unstable_candidate_id" ); assert.deepEqual( @@ -258,9 +269,37 @@ try { const enrichedPrevious = structuredClone(mergedWithSources); enrichedPrevious.findings[0].summary = "The earlier reduction established an additional reachable output route."; enrichedPrevious.findings[0].validation = { summary: "Both output routes bypass the same encoding control." }; - await writeFile(path.join(outputRoot, "result.json"), JSON.stringify(enrichedPrevious)); + for (const legacyCoverage of [ + rejectedCoverage, + { completeness: "outdated", surfaces: [null], explicitExclusions: false, deferred: 42 }, + "legacy coverage is no longer structured", + ]) { + const previousArtifact = JSON.stringify({ ...enrichedPrevious, coverage: legacyCoverage }); + await writeFile(path.join(outputRoot, "result.json"), previousArtifact); + assert.deepEqual( + (await getCodexSecurityDeepReducerInputs(nextContext)).previous, + enrichedPrevious, + "previous reducer coverage is ignored even when malformed; findings and scope remain intact", + ); + assert.equal( + await readFile(path.join(outputRoot, "result.json"), "utf8"), + previousArtifact, + "reading a previous reduction does not rewrite its legacy coverage", + ); + } + const previousArtifact = await readFile(path.join(outputRoot, "result.json"), "utf8"); await recordCodexSecurityDeepReduction(nextContext, merged); const preservedEnrichment = JSON.parse(await readFile(path.join(nextOutputRoot, "result.json"), "utf8")); + assert.equal( + Object.hasOwn(preservedEnrichment, "coverage"), + false, + "a subsequent accepted reduction omits the previous reducer's legacy coverage", + ); + assert.equal( + await readFile(path.join(outputRoot, "result.json"), "utf8"), + previousArtifact, + "the original previous reduction remains available without rewriting its coverage", + ); assert.equal( preservedEnrichment.findings[0].provenance.previousFindings[0].summary, enrichedPrevious.findings[0].summary, @@ -293,6 +332,22 @@ try { /repeats assigned Standard scan worker/ ); + await writeFile(first.resultPath, JSON.stringify({ ...first.result, complete: false })); + await assert.rejects( + getCodexSecurityDeepReducerInputs(context), + /only a checkpoint/, + "unfinished Standard worker results are not reducer inputs", + ); + + for (const invalidCoverage of [undefined, { ...workerDraft([]).coverage, completeness: "outdated" }]) { + await writeFile(first.resultPath, JSON.stringify({ ...first.result, coverage: invalidCoverage })); + await assert.rejects( + getCodexSecurityDeepReducerInputs(context), + /coverage|completeness/, + "Standard worker coverage remains required and validated before projection", + ); + } + await writeFile(first.resultPath, "{invalid Standard scan\n"); await assert.rejects( getCodexSecurityDeepReducerInputs(context), @@ -325,7 +380,11 @@ async function createWorker({ workersRoot, label, id, result, completionSequence return { id, resultPath, completionSequence, result }; } -function draft(findings, extra = {}) { +function reduction(findings, extra = {}) { + return { scanId, findings, ...extra }; +} + +function workerDraft(findings, extra = {}) { return { scanId, findings, @@ -340,8 +399,9 @@ function draft(findings, extra = {}) { } function withSourceRefs(worker) { + const { coverage: _coverage, ...result } = worker.result; return { - ...worker.result, + ...result, findings: worker.result.findings.map((finding, index) => ({ ...finding, provenance: { ...finding.provenance, sourceFindingIds: [`${worker.id}:${index}`] }, diff --git a/plugins/codex-security/mcp-app/tests/test_artifact_scan_draft.mjs b/plugins/codex-security/mcp-app/tests/test_artifact_scan_draft.mjs index 77ff8a93d..72f815fc3 100644 --- a/plugins/codex-security/mcp-app/tests/test_artifact_scan_draft.mjs +++ b/plugins/codex-security/mcp-app/tests/test_artifact_scan_draft.mjs @@ -500,6 +500,118 @@ try { assert.deepEqual(carriedParentManifest.scan.scope.summary, input.scope.summary); assert.deepEqual(carriedParentManifest.scan.threatModel, input.threatModel); + const deepParentRoot = path.join(root, "accepted-deep-parent"); + await mkdir(deepParentRoot); + const deepParentContext = { + ...context, + root: deepParentRoot, + mode: "deep", + scope: "src", + targetContract: { + ...context.targetContract, + scope: { + requiredIncludePaths: ["src", "lib"], + requiredExcludePaths: ["vendor"], + }, + }, + }; + const obsoleteFinding = { + ...finding, + identity: { anchor: "obsolete-parent-finding" }, + provenance: { ...finding.provenance, candidateId: "obsolete-parent-candidate" }, + extensions: { candidateId: "obsolete-parent-candidate" }, + }; + const obsoleteDeepDraft = { + ...input, + complete: false, + findings: [obsoleteFinding], + coverage: { + ...coverage, + completeness: "partial", + surfaces: [{ + label: "Old upload handler", + disposition: "needs_follow_up", + notes: "An earlier parent draft left this review unfinished.", + }], + deferred: [{ candidateId: "obsolete-review", reason: "Earlier review work." }], + }, + }; + await recordCodexSecurityScanDraft(deepParentContext, obsoleteDeepDraft); + await saveScanDraftCheckpoint(deepParentContext, { + ...obsoleteDeepDraft, + findings: [interruptedFinding], + }); + await recordCodexSecurityScanDraft(deepParentContext, { + ...input, + complete: false, + findings: [], + }); + assert.deepEqual( + new Set((await readJson(deepParentRoot, "findings.json")).findings.map( + (item) => item.provenance.candidateId, + )), + new Set(["obsolete-parent-candidate", "interrupted-checkpoint"]), + "an unfinished Deep parent checkpoint still preserves earlier validated findings", + ); + assert.equal((await readJson(deepParentRoot, "scan-manifest.json")).scan.complete, false); + assert.equal((await readJson(deepParentRoot, "coverage.json")).completeness, "partial"); + const savedDeepCheckpoints = await Promise.all( + (await readdir(path.join(deepParentRoot, "checkpoints"))).map(async (name) => [ + name, + await readFile(path.join(deepParentRoot, "checkpoints", name), "utf8"), + ]), + ); + const acceptedDeepDraft = { + ...input, + complete: true, + coverage: { completeness: "complete", surfaces: [], explicitExclusions: [], deferred: [] }, + }; + await recordCodexSecurityScanDraft(deepParentContext, acceptedDeepDraft); + const acceptedDeepFindings = await readJson(deepParentRoot, "findings.json"); + const acceptedDeepCoverage = await readJson(deepParentRoot, "coverage.json"); + const acceptedDeepManifest = await readJson(deepParentRoot, "scan-manifest.json"); + assert.equal(acceptedDeepFindings.findings.length, 1); + assert.deepEqual(acceptedDeepFindings.findings[0].provenance, finding.provenance); + assert.equal(acceptedDeepCoverage.completeness, "complete"); + assert.deepEqual(acceptedDeepCoverage.deferred, []); + assert.deepEqual( + acceptedDeepCoverage.surfaces, + [], + "accepted Deep coverage does not inherit obsolete parent review work", + ); + assert.deepEqual(acceptedDeepCoverage.explicitExclusions, []); + assert.deepEqual(acceptedDeepCoverage.includePaths, ["src", "lib"]); + assert.deepEqual(acceptedDeepCoverage.excludePaths, ["vendor"]); + assert.deepEqual(acceptedDeepManifest.scan.scope.includePaths, ["src", "lib"]); + assert.deepEqual(acceptedDeepManifest.scan.scope.excludePaths, ["vendor"]); + for (const [name, contents] of savedDeepCheckpoints) { + assert.equal(await readFile(path.join(deepParentRoot, "checkpoints", name), "utf8"), contents); + } + + const obsoleteCheckpointPath = path.join(deepParentRoot, "checkpoints", "obsolete.json"); + await writeFile(obsoleteCheckpointPath, "{malformed obsolete checkpoint\n"); + let deepWorkbenchWrites = 0; + await recordCodexSecurityScanDraftViaWorkbench( + deepParentContext, + acceptedDeepDraft, + async (arguments_) => { + deepWorkbenchWrites += 1; + assert.deepEqual(arguments_.slice(0, 3), ["write-scan-draft", "--scan-id", scanId]); + assert.equal(arguments_.includes("--expected-draft-digest"), false); + assert.deepEqual(arguments_.slice(-2), ["--claim-token", claimToken]); + const draftPath = arguments_[arguments_.indexOf("--draft-path") + 1]; + const checkpointPath = arguments_[arguments_.indexOf("--checkpoint-path") + 1]; + const staged = JSON.parse(await readFile(draftPath, "utf8")); + const stagedCheckpoint = JSON.parse(await readFile(checkpointPath, "utf8")); + assert.deepEqual(staged.findings, acceptedDeepFindings); + assert.deepEqual(staged.coverage, acceptedDeepCoverage); + assert.deepEqual(stagedCheckpoint.findings, acceptedDeepDraft.findings); + assert.equal(stagedCheckpoint.handoffClaimToken, undefined); + }, + ); + assert.equal(deepWorkbenchWrites, 1, "terminal Deep drafts still publish through the workbench lock despite obsolete malformed checkpoints"); + assert.deepEqual(await readdir(path.join(deepParentRoot, "drafts")), []); + const pendingRoot = path.join(root, "pending-worker"); await mkdir(pendingRoot); const pendingContext = { ...workerContext, root: pendingRoot }; @@ -2188,6 +2300,94 @@ try { "duplicate stable instance sources remain collisions for finalization", ); + const collisionFindings = ["src/upload.py", "src/import.py", "src/restore.py"].map((location) => ({ + ...finding, + locations: [{ path: location, startLine: 41, endLine: 44 }], + provenance: { source: "local_plugin" }, + extensions: {}, + })); + const authoredCollisionIdentity = { anchor: "shared-archive-review" }; + const reservedCollisionIdentity = { ...authoredCollisionIdentity, instance: "parser" }; + const collisionCases = [ + { + label: "authored", + findings: collisionFindings.map((item) => ({ ...item, identity: authoredCollisionIdentity })), + originalIdentity: authoredCollisionIdentity, + }, + { + label: "generated", + findings: collisionFindings, + originalIdentity: { anchor: "unsafe-archive-extraction", instance: "unsafe-archive-extraction" }, + }, + { + label: "authored with reserved suffixes", + findings: [ + ...collisionFindings.map((item) => ({ ...item, identity: reservedCollisionIdentity })), + ...collisionFindings.slice(0, 2).map((item, index) => ({ + ...item, + identity: { ...reservedCollisionIdentity, instance: `parser-${index + 2}` }, + })), + ], + originalIdentity: reservedCollisionIdentity, + expectedIdentities: [ + reservedCollisionIdentity, + { ...reservedCollisionIdentity, instance: "parser-4" }, + { ...reservedCollisionIdentity, instance: "parser-5" }, + { ...reservedCollisionIdentity, instance: "parser-2" }, + { ...reservedCollisionIdentity, instance: "parser-3" }, + ], + }, + ]; + for (const collisionCase of collisionCases) { + const collisionInput = { ...input, findings: collisionCase.findings }; + await recordFreshScanDraft(context, collisionInput); + const standardFindings = (await readJson(root, "findings.json")).findings; + assert.deepEqual( + standardFindings.map((item) => item.identity), + collisionCase.findings.map((item) => item.identity ?? collisionCase.originalIdentity), + `${collisionCase.label} identity collisions retain the existing Standard shape`, + ); + assert.deepEqual( + standardFindings.map((item) => item.provenance), + collisionCase.findings.map((item) => item.provenance), + ); + + const deepIdentityContext = { ...context, mode: "deep" }; + await recordFreshScanDraft(deepIdentityContext, collisionInput); + const deepFindings = (await readJson(root, "findings.json")).findings; + const deepIdentities = deepFindings.map((item) => item.identity); + assert.equal(deepFindings.length, collisionCase.findings.length, `${collisionCase.label} identity collisions must retain every Deep finding`); + assert.deepEqual( + deepIdentities, + collisionCase.expectedIdentities ?? collisionCase.findings.map((_, index) => ( + index === 0 ? collisionCase.originalIdentity : { + ...collisionCase.originalIdentity, + instance: `${collisionCase.originalIdentity.instance ?? "saved"}-${index + 1}`, + } + )), + `${collisionCase.label} collisions receive successive numeric suffixes without replacing authored identities`, + ); + assert.deepEqual( + deepFindings.map((item) => item.provenance), + collisionCase.findings.map((item, index) => ( + index === 1 || index === 2 ? { + ...item.provenance, preservedIdentity: collisionCase.originalIdentity, + } : item.provenance + )), + ); + assert.deepEqual( + deepFindings.map((item) => item.locations), + collisionCase.findings.map((item) => item.locations), + ); + + await recordCodexSecurityScanDraft(deepIdentityContext, collisionInput); + assert.deepEqual( + (await readJson(root, "findings.json")).findings.map((item) => item.identity), + deepIdentities, + `${collisionCase.label} Deep finding identities remain stable when the accepted aggregate is republished`, + ); + } + const completeCodeEvidence = { ...incompleteCodeEvidence, code: "extract_archive_entry(untrusted_entry, output_path)", diff --git a/plugins/codex-security/mcp-app/tests/test_compact_artifact_server.mjs b/plugins/codex-security/mcp-app/tests/test_compact_artifact_server.mjs index d1af7e3da..153bfaf9f 100644 --- a/plugins/codex-security/mcp-app/tests/test_compact_artifact_server.mjs +++ b/plugins/codex-security/mcp-app/tests/test_compact_artifact_server.mjs @@ -1151,10 +1151,13 @@ async function testReducerWorkerToolList(bundle) { assert.equal( Object.hasOwn(tool.inputSchema.properties ?? {}, "scanId"), tool.name === "record_codex_security_deep_reduction", - `${tool.name} must expose scanId only when submitting its complete Standard draft.` + `${tool.name} must expose scanId only when submitting its complete reduction.` ); if (tool.name === "record_codex_security_deep_reduction") { - assert.equal(tool.inputSchema.required?.includes("scanId"), true); + assert.deepEqual(tool.inputSchema.required, ["scanId", "findings"]); + assert.equal(tool.inputSchema.additionalProperties, false); + assert.equal(Object.hasOwn(tool.inputSchema.properties, "coverage"), false, + "The reducer must not be asked to submit coverage."); } for (const forbidden of [ "path", diff --git a/plugins/codex-security/mcp-app/tests/test_deep_scan_artifact_validation.mjs b/plugins/codex-security/mcp-app/tests/test_deep_scan_artifact_validation.mjs index e5c3e6b65..2b0bd4391 100644 --- a/plugins/codex-security/mcp-app/tests/test_deep_scan_artifact_validation.mjs +++ b/plugins/codex-security/mcp-app/tests/test_deep_scan_artifact_validation.mjs @@ -146,6 +146,19 @@ async function testDiscoveryValidation(root) { /complete coverage cannot contain deferred/ ); + await writeResult(worker.resultPath, { + ...result, + coverage: { + ...result.coverage, + surfaces: [{ label: "Unfinished Standard review", disposition: "needs_follow_up" }], + }, + }); + await assert.rejects( + validateDiscoveryArtifacts(artifacts, worker.resultPath, scanId), + /complete coverage cannot contain needs_follow_up/, + "reducer coverage normalization must not relax Standard discovery semantics", + ); + await writeResult(worker.resultPath, { ...result, findings: [{ @@ -210,13 +223,20 @@ async function testReducerValidation(root) { await assert.rejects(validateSnapshot(), /unaccounted source findings/); await writeResult(resultPath, draft([firstFinding, secondFinding])); await writeFile(first.resultPath, "{source changed after dispatch"); - assert.deepEqual(await validateSnapshot(), { newFindings: 2 }); + const validatedSnapshot = await validateSnapshot(); + assert.equal(validatedSnapshot.newFindings, 2); const admitted = JSON.parse(await readFile(resultPath, "utf8")); + assert.deepEqual( + validatedSnapshot.result, + admitted, + "validation returns the same reconciled result that was accepted on disk", + ); + assert.equal(Object.hasOwn(admitted, "coverage"), false); assert.deepEqual(admitted.findings[1].provenance.sourceFindingIds, ["worker-001:1"]); sources.previous = structuredClone(admitted); sources.previous.findings[0].summary = "Additional proof established by the previous reducer."; await writeResult(resultPath, draft([firstFinding, secondFinding])); - assert.deepEqual(await validateSnapshot(), { newFindings: 0 }); + assert.equal((await validateSnapshot()).newFindings, 0); assert.equal( JSON.parse(await readFile(resultPath, "utf8")).findings[0].provenance.previousFindings[0].summary, sources.previous.findings[0].summary, @@ -286,46 +306,6 @@ async function testReducerValidation(root) { riskArea: "filesystem", notes: "The reducer completed the extraction review.", }; - await writeResult(resultPath, draft([firstFinding], { - coverage: { - completeness: "complete", - surfaces: [resolvedCoverageSurface], - explicitExclusions: [], - deferred: [], - }, - })); - await validateReducerArtifacts({ - artifacts, - artifactDir, - resultPath, - reducerId: "dedup-updated-coverage", - sources: { - discoveries: [{ - workerId: first.id, - result: draft([firstFinding], { - coverage: { - completeness: "partial", - surfaces: [{ - riskArea: "filesystem", - notes: "The discovery worker still needed runtime validation.", - disposition: "needs_follow_up", - label: "Archive extraction", - }], - explicitExclusions: [], - deferred: [], - }, - }), - }], - previous: null, - }, - }, scanId); - const reconciledCoverage = JSON.parse(await readFile(resultPath, "utf8")).coverage; - assert.deepEqual( - reconciledCoverage.surfaces, - [resolvedCoverageSurface], - "the reducer's updated semantic surface must supersede stale source coverage", - ); - assert.equal(reconciledCoverage.completeness, "complete"); await writeResult(resultPath, draft([])); await assert.rejects( @@ -411,16 +391,51 @@ async function testReducerValidation(root) { ...(previousReducerResultPath ? { previousReducerResultPath } : {}) }, scanId); - assert.deepEqual(await validate(), { newFindings: 1 }); + assert.equal((await validate()).newFindings, 1); + + const legacyPartial = draft([firstFinding], { + coverage: { + completeness: "partial", + surfaces: [ + { ...resolvedCoverageSurface, receiptRefs: ["artifacts/missing-worker-receipt.md"] }, + { label: "Legacy follow-up", disposition: "needs_follow_up" }, + ], + explicitExclusions: [{ pattern: "vendor", reason: "Outside the requested source scope." }], + deferred: [{ reason: "A previous reducer retained worker follow-up work." }], + openQuestions: ["Should a future review include generated handlers?"], + }, + }); + for (const [label, legacyCoverage] of [ + ["partial", legacyPartial.coverage], + ["complete with pending work", { ...legacyPartial.coverage, completeness: "complete" }], + ["malformed", null], + ]) { + const legacyReducer = { + ...legacyPartial, + coverage: legacyCoverage, + }; + await writeResult(resultPath, legacyReducer); + const legacyArtifact = await readFile(resultPath, "utf8"); + const resumed = await validate(); + assert.equal(resumed.newFindings, 1); + assert.deepEqual(resumed.result, { scanId, findings: [firstFinding] }); + assert.equal( + await readFile(resultPath, "utf8"), + legacyArtifact, + `resuming a reducer with ${label} coverage ignores it without rewriting the original artifact`, + ); + } + await writeResult(resultPath, { ...legacyPartial, complete: false }); + await assert.rejects(validate(), /only a checkpoint|not complete/); await writeResult(resultPath, draft([])); - assert.deepEqual(await validate(), { newFindings: 0 }); + assert.equal((await validate()).newFindings, 0); await writeResult(resultPath, { ...draft([firstFinding]), resultPath: "/tmp/result.json" }); - await assert.rejects(validate(), /invalid Standard scan result/); + await assert.rejects(validate(), /resultPath/); await writeResult(resultPath, draft([firstFinding, secondFinding])); - assert.deepEqual(await validate(), { newFindings: 2 }); + assert.equal((await validate()).newFindings, 2); const previousReducerResultPath = path.join( artifacts.dedupRoot, @@ -429,17 +444,27 @@ async function testReducerValidation(root) { "result.json" ); await mkdir(path.dirname(previousReducerResultPath), { recursive: true }); - await writeResult(previousReducerResultPath, draft([firstFinding])); - assert.deepEqual( - await validate(previousReducerResultPath), - { newFindings: 1 } + await writeResult(previousReducerResultPath, { + ...legacyPartial, + coverage: "malformed legacy coverage", + }); + const previousArtifact = await readFile(previousReducerResultPath, "utf8"); + assert.equal( + (await validate(previousReducerResultPath)).newFindings, + 1, + "malformed previous reducer coverage is ignored and does not change finding novelty", + ); + assert.equal( + await readFile(previousReducerResultPath, "utf8"), + previousArtifact, + "reading a previous reducer must not rewrite its original coverage", ); const renamedTitle = { ...firstFinding, title: "Stronger explanation of the same finding." }; await writeResult(resultPath, draft([renamedTitle, secondFinding])); - assert.deepEqual( - await validate(previousReducerResultPath), - { newFindings: 1 } + assert.equal( + (await validate(previousReducerResultPath)).newFindings, + 1 ); await writeResult(previousReducerResultPath, draft([firstFinding, secondFinding])); @@ -451,9 +476,9 @@ async function testReducerValidation(root) { const replacement = finding("replacement", "src/c.js"); await writeResult(resultPath, draft([firstFinding, secondFinding, replacement])); - assert.deepEqual( - await validate(previousReducerResultPath), - { newFindings: 1 } + assert.equal( + (await validate(previousReducerResultPath)).newFindings, + 1 ); const implicitFirst = { ...firstFinding }; @@ -461,9 +486,9 @@ async function testReducerValidation(root) { const implicitRenamed = { ...implicitFirst, summary: "More complete evidence." }; await writeResult(previousReducerResultPath, draft([implicitFirst])); await writeResult(resultPath, draft([implicitRenamed])); - assert.deepEqual( - await validate(previousReducerResultPath), - { newFindings: 0 } + assert.equal( + (await validate(previousReducerResultPath)).newFindings, + 0 ); await writeResult(resultPath, draft([{ ...implicitRenamed, @@ -472,9 +497,9 @@ async function testReducerValidation(root) { { path: "src/another-affected-location.js", startLine: 4 } ] }])); - assert.deepEqual( - await validate(previousReducerResultPath), - { newFindings: 0 }, + assert.equal( + (await validate(previousReducerResultPath)).newFindings, + 0, "An existing finding without an explicit identity may gain affected locations." ); @@ -487,15 +512,15 @@ async function testReducerValidation(root) { await writeResult(resultPath, draft([firstFinding])); await writeResult(first.resultPath, { ...draft([firstFinding]), scanId: otherScanId }); - assert.deepEqual( - await validate(), - { newFindings: 1 }, + assert.equal( + (await validate()).newFindings, + 1, "A completed aggregate must not reread already-consumed Standard results." ); await writeFile(first.resultPath, "{invalid Standard scan\n"); - assert.deepEqual( - await validate(), - { newFindings: 1 }, + assert.equal( + (await validate()).newFindings, + 1, "Accepted Standard inputs were already validated by the reducer writer." ); await writeResult(first.resultPath, draft([firstFinding])); @@ -533,7 +558,7 @@ async function testEmptyDiscoveryAndReduction(root) { const artifactDir = path.join(artifacts.dedupRoot, "dedup-empty", "output"); const resultPath = path.join(artifactDir, "result.json"); await mkdir(artifactDir, { recursive: true }); - await writeResult(resultPath, draft([])); + await writeResult(resultPath, { scanId, findings: [] }); const result = await validateReducerArtifacts({ artifacts, artifactDir, @@ -541,6 +566,11 @@ async function testEmptyDiscoveryAndReduction(root) { reducerId: "dedup-empty" }); assert.equal(result.newFindings, 0); + assert.deepEqual( + result.result, + { scanId, findings: [] }, + "reducers submit and return results without coverage", + ); } async function createLayout(scanDir) { diff --git a/plugins/codex-security/mcp-app/tests/test_deep_scan_coordinator.mjs b/plugins/codex-security/mcp-app/tests/test_deep_scan_coordinator.mjs index a27a03dde..0625bbbb1 100644 --- a/plugins/codex-security/mcp-app/tests/test_deep_scan_coordinator.mjs +++ b/plugins/codex-security/mcp-app/tests/test_deep_scan_coordinator.mjs @@ -4,6 +4,7 @@ import { mkdir, mkdtemp, readFile, readdir, realpath, rename, rm, writeFile } fr import { tmpdir } from "node:os"; import path from "node:path"; import { build } from "esbuild"; +import { testDeepScanPublication } from "./deep_scan_publication_cases.mjs"; const bundle = await build({ bundle: true, @@ -790,7 +791,7 @@ async function testDiscoveryDeadlineWithoutAcceptedWorkersPublishesEmptyResults( assert.deepEqual(JSON.parse(await readFile(terminal.manifestPath, "utf8")).findings, []); } -async function testSaturationDoesNotHideTerminalWorkerFailure() { +async function testSaturationIgnoresWorkerFailureSettledAfterStop() { const fixture = await fixtureRun({ workers: 3, subagents: 0, stopAfterNoNew: 2, maxDiscoveryRuns: 3 }); const store = new FakeStore(fixture.run); store.blockDiscoveryFailure = true; @@ -814,21 +815,24 @@ async function testSaturationDoesNotHideTerminalWorkerFailure() { await Promise.all([executor.dedupStarted, store.discoveryFailureBlocked.promise]); executor.releaseDedup(); - await eventually(() => coordinator.snapshot().noNewStreak === 2); + await eventually(() => executor.dedupSignal?.aborted === true); store.releaseDiscoveryFailure(); const terminal = await coordinator.wait(undefined, 5_000); - assert.equal(terminal?.status, "failed"); - assert.equal(terminal?.terminalReason, undefined); - assert.equal(completedDrafts.length, 0); - assert.match(terminal.error, /fixture configuration failure/); + assert.equal(terminal?.status, "succeeded", terminal?.error); + assert.equal(terminal?.terminalReason, "saturated"); + assert.equal(completedDrafts.length, 1); + assert.equal(completedDrafts[0].coverage.completeness, "complete"); + assert.deepEqual(completedDrafts[0].findings, []); const failedWorker = [...store.workers.values()].find((worker) => ( worker.status === "failed" && path.basename(path.dirname(worker.promptPath)) === "discovery-0003" )); assert.ok(failedWorker); assert.equal(failedWorker.status, "failed"); - assert.equal(store.failCalls, 1); - assert.equal(store.finishCalls.length, 0); + assert.equal(store.failCalls, 0); + assert.equal(store.finishCalls.length, 1); + assert.equal(executor.discoveryCalls, 3); + assert.equal(executor.dedupCalls, 1); } async function testSettledReducerIsNotStarvedByDiscoveryBacklog() { @@ -1743,7 +1747,7 @@ async function testMissingReducerResultResumesExistingThread() { assert.equal(new Set(executor.dedupThreadIds).size, 1); assert.match( executor.dedupContinuationPrompts[1] ?? "", - /record_codex_security_deep_reduction\(\{ scanId, findings, coverage, threatModel\?, scope\? \}\)/ + /record_codex_security_deep_reduction\(\{ scanId, findings, threatModel\?, scope\? \}\)/ ); assert.doesNotMatch(executor.dedupContinuationPrompts[1] ?? "", /\{ candidates, merges \}/); assert.match(executor.dedupContinuationPrompts[1] ?? "", /retry the call until it succeeds/); @@ -2157,7 +2161,7 @@ async function testReducerTraceabilityRetryNamesExactMissingSource() { ]); assert.doesNotMatch(basePrompt, /artifact_validation_failed/); assert.match(retryPrompt, /artifact_validation_failed/); - assert.match(retryPrompt, /coverage.*completeness/s); + assert.match(retryPrompt, /findings/s); assert.equal(context.claimedWorkerIds.includes(missingWorkerId), true); } @@ -3848,7 +3852,8 @@ async function writeDedupArtifacts(request, consumedOverride, options = {}) { if (options.canonicalCandidateId && draft.findings.length > 0) { draft.findings[0].provenance.candidateId = options.canonicalCandidateId; } - if (consumedOverride || options.omitLastWorkerSource) draft.coverage.completeness = "invalid"; + delete draft.coverage; + if (consumedOverride || options.omitLastWorkerSource) draft.findings = "invalid"; if (options.dropLastFinding) draft.findings.pop(); const resultPath = path.join(artifactContext.root, "result.json"); await mkdir(path.dirname(resultPath), { recursive: true }); @@ -3962,6 +3967,10 @@ try { await testSandboxDiagnosticSurvivesArtifactRetries(); await testCompletionOrdering(); await testSaturationPreservesFindingAlreadyBuffered(); + await testDeepScanPublication({ + fixtureRun, FakeStore, FakeExecutor, DeepScanCoordinator, deferred, + immediateClock, eventually, + }); await testDirectReducerCannotDropAcceptedFinding(); await testSaturationDrainsBufferedAndCancelsInflight(); await testDiscoveryDeadlineDrainsActiveReducerAndPreservesFindings(); @@ -3970,7 +3979,7 @@ try { await testDiscoveryDeadlineWithoutAcceptedWorkersReturnsPartialEvidence(); await testDiscoveryDeadlineBeforeWorkerDispatchReturnsPartialEvidence(); await testDiscoveryDeadlineWithoutAcceptedWorkersPublishesEmptyResults(); - await testSaturationDoesNotHideTerminalWorkerFailure(); + await testSaturationIgnoresWorkerFailureSettledAfterStop(); await testSettledReducerIsNotStarvedByDiscoveryBacklog(); await testSingletonHardCapReduction(); await testExhaustedRetryFailsScan(); diff --git a/plugins/codex-security/mcp-app/tests/test_deep_scan_templates.mjs b/plugins/codex-security/mcp-app/tests/test_deep_scan_templates.mjs index b2c6930bb..86c2fc28f 100644 --- a/plugins/codex-security/mcp-app/tests/test_deep_scan_templates.mjs +++ b/plugins/codex-security/mcp-app/tests/test_deep_scan_templates.mjs @@ -25,6 +25,7 @@ const rendered = renderDiscoveryPrompt({ assert.doesNotMatch(rendered, /false_positive_feedback\.json/); assert.match(rendered, /preserve literal \{\{DISCOVERY_CONTEXT_JSON\}\} text/); assert.match(rendered, /record_codex_security_scan_draft/); +assert.match(rendered, /coverage\.deferred/); const discoveryContext = firstJsonBlock(rendered); assert.deepEqual(discoveryContext, { scanId: "a0d89285-66b7-4e4f-b51a-e21b93b7081b", @@ -68,6 +69,8 @@ const dedup = renderDedupPrompt({ }] }); const dedupContext = firstJsonBlock(dedup); +assert.doesNotMatch(dedup, /\bcoverage\b/i); +assert.match(dedup, /record_codex_security_deep_reduction\(\{ scanId, findings, threatModel\?, scope\? \}\)/); assert.deepEqual(dedupContext, { reducerLabel: "dedup-0001", claimedWorkerIds: ["worker-001"] diff --git a/plugins/codex-security/references/final-report.md b/plugins/codex-security/references/final-report.md index 8fa17bd1e..bf7c6eefa 100644 --- a/plugins/codex-security/references/final-report.md +++ b/plugins/codex-security/references/final-report.md @@ -16,11 +16,11 @@ Use `report.md` as the primary readable entry point. Explain report-relevant art In the final response, link the generated markdown report path as the primary readable artifact. -Every scan mode uses the same final report pipeline. Workbench-owned Standard and workbench-backed diff scans submit canonical semantics with `record_codex_security_scan_draft({ scanId, handoffClaimToken?, scope?, threatModel?, findings, coverage })`; the workbench supplies authoritative metadata and writes the unsealed canonical draft. Deep Standard scan workers submit complete semantic results through their bound draft tool, and the coordinator aggregates them and writes the parent scan's unsealed canonical draft. SDK-owned Standard scans instead write unsealed canonical files with the exact SDK-provided metadata and leave finalization to the SDK. Terminal scans without a `scanId` retain their existing canonical JSON workflow. No mode authors, repairs, or treats an existing `report.md` as input. Finalization validates and enriches the canonical JSON, seals the canonical JSON and evidence artifacts, then deterministically generates `report.md`. Supply report prose through structured canonical semantics rather than a separately authored report. +Every scan mode uses the same final report pipeline. Workbench-owned Standard and workbench-backed diff scans submit canonical semantics with `record_codex_security_scan_draft({ scanId, handoffClaimToken?, scope?, threatModel?, findings, coverage })`; the workbench supplies authoritative metadata and writes the unsealed canonical draft. For Deep scans, the coordinator writes the parent scan's unsealed canonical draft from its accepted aggregate. SDK-owned Standard scans instead write unsealed canonical files with the exact SDK-provided metadata and leave finalization to the SDK. Terminal scans without a `scanId` retain their existing canonical JSON workflow. No mode authors, repairs, or treats an existing `report.md` as input. Finalization validates and enriches the canonical JSON, seals the canonical JSON and evidence artifacts, then deterministically generates `report.md`. Supply report prose through structured canonical semantics rather than a separately authored report. For each finding, supply an evidence-supported lowercase vulnerability-family `ruleId`; `taxonomy: { category, cwe }` using its exact known CWEs; verified locations; and `provenance.source`, using `"local_plugin"` only when this plugin actually discovered the finding. Preserve genuine worker or source provenance and any existing canonical candidate identity in the finding extensions. A finding with no known CWE retains `cwe: []`; never invent a classification. Include optional `codeEvidence` only when its actual code is nonempty and every referenced evidence ID is present. -Supply semantic coverage as `{ completeness, surfaces, explicitExclusions, deferred }`, with each surface using the actual `label` and one existing `disposition`. Mark coverage `partial` when a deferred item or `needs_follow_up` surface remains; preserve its real reason and supporting context. Each deferred item needs a meaningful reason; preserve any existing `id` or `candidateId`. The workbench derives a missing ID from its candidate identity or stable deferred-work details. Open questions may be nonempty strings or `{ question, followUpPrompt? }` objects. The workbench derives target and scope metadata, scope include and exclude paths, coverage mode and inventory strategy, finding identities and fingerprints, and surface IDs. Do not put those workbench-owned values or top-level coverage receipt references into the semantic draft. +For Standard scan drafts, including Deep worker results, and diff drafts, supply semantic coverage as `{ completeness, surfaces, explicitExclusions, deferred }`, with each surface using the actual `label` and one existing `disposition`. Mark coverage `partial` when a deferred item or `needs_follow_up` surface remains; preserve its real reason and supporting context. Each deferred item needs a meaningful reason; preserve any existing `id` or `candidateId`. The workbench derives a missing ID from its candidate identity or stable deferred-work details. Open questions may be nonempty strings or `{ question, followUpPrompt? }` objects. The workbench derives target and scope metadata, scope include and exclude paths, coverage mode and inventory strategy, finding identities and fingerprints, and surface IDs. Do not put those workbench-owned values or top-level coverage receipt references into the semantic draft. During a host-backed scan, checkpoint saved findings and pending candidates with `complete: false`; a checkpoint is not a completed audit. After a final workbench-owned Standard or workbench-backed diff draft is accepted with `complete: true` (or the backwards-compatible omitted flag), or the Deep coordinator returns its parent scan's canonical manifest, call `complete_codex_security_scan({ scanId, handoffClaimToken? })` and use its returned completion metadata. An SDK-owned scan returns its unsealed canonical files without calling a completion tool or finalizer; the SDK owns completion and report generation. Read full canonical results only when explicitly requested. For a terminal/chat workflow without a `scanId` or completion tool, retain `python /scripts/finalize_scan_contract.py --scan-dir --source-root ` after writing the canonical JSON. Outside the SDK path, do not mark the scan goal complete until finalization succeeds and the generated report exists. @@ -59,7 +59,7 @@ Group observations only when they share the same broken security control and eff Set the finding category and CWE from the primary broken control. Do not add secondary support-impact CWEs, such as data exposure or missing authentication, to an injection/RCE/path/file/parser finding merely because they make exploitation worse; mention those impacts in prose or emit a separate finding if that secondary control is independently vulnerable. -Workbench-owned Standard scans submit their source-backed final findings and coverage directly through `record_codex_security_scan_draft`; SDK-owned Standard scans write the same semantics into unsealed canonical files. Deep scans semantically aggregate complete Standard worker results, preserving validated findings, attack-path analysis, affected locations, threat-model context, and honest coverage. The coordinator writes the parent scan's canonical draft; the parent does not read candidate ledgers, run separate validation or attack-path phases, or submit another draft. Canonical `severity.changeConditions` must be one non-empty string; when `attack_path.change_conditions` contains multiple strings, join them into one prose string before writing `findings.json`. +Canonical `severity.changeConditions` must be one non-empty string; when `attack_path.change_conditions` contains multiple strings, join them into one prose string before writing `findings.json`. For workbench-backed diff candidates, apply row outcomes in this order: validation disposition `reportable` plus attack-path decision `reportable` becomes a finding with its distinct instance and all relevant entrypoint, root-control, sink, and supporting locations; otherwise, a `deferred` result from either phase becomes `needs_follow_up` coverage and a `coverage.deferred` entry using the recorded uncertainty or proof gap; otherwise, validation disposition `not_applicable` becomes `not_applicable` coverage; otherwise, validation disposition `suppressed` or attack-path decision `ignore` becomes `rejected` coverage. A missing required phase record leaves the candidate unresolved and prevents complete coverage. Do not require phase receipts, per-candidate narratives, or another reconciliation pass. @@ -73,6 +73,8 @@ Use this report structure: `## Scope` +Deep reports present the configured include and exclude paths and execution outcome alongside aggregated findings, with their validation, attack paths, affected locations, and any supplied scope and threat-model context. When the discovery time limit expires before any source review completes, the report explains the incomplete review. + Populate `scan.scope` with in-scope context, artifacts reviewed, runtime or test status, validation mode, and explicit limitations. Include/exclude paths and coverage fields supply the remaining projected scope content. If the threat model was generated during Phase 1 rather than provided by the user, say that in canonical scope context. Do not call generated threat-model material an external input. After the scope bullets, include a compact `### Scan Summary` table when the scan has findings or repository-wide coverage. Use columns `Field` and `Value`. Include the count of reportable findings, severity mix, confidence mix, coverage, and validation mode when those values are known. Keep artifact paths below this table. diff --git a/plugins/codex-security/references/scan-artifacts.md b/plugins/codex-security/references/scan-artifacts.md index e4fabb441..9d58246d9 100644 --- a/plugins/codex-security/references/scan-artifacts.md +++ b/plugins/codex-security/references/scan-artifacts.md @@ -40,7 +40,11 @@ End each repository-scoped threat model with these two lines: ### Compact Deep And Workbench-Backed Diff Discovery -Workbench-owned Standard scans submit findings and coverage through `record_codex_security_scan_draft`; SDK-owned Standard scans write unsealed canonical files directly. Deep scans run complete Standard scan workers, each of which saves `complete: false` checkpoints and submits its final validated findings, coverage, threat model, and optional scope through its bound `record_codex_security_scan_draft` tool. Pending candidates retain their original evidence in `coverage.deferred`. Host-owned immutable checkpoints survive retries, cancellation, and failure; a checkpoint alone is never an accepted complete worker result. The coordinator semantically reduces those complete results and writes the parent scan's unsealed `scan-manifest.json`, `findings.json`, and `coverage.json`. The parent does not list candidates, rerun validation or attack-path phases, or submit another draft. Workbench-backed diff scans retain the compact artifacts described below. +Workbench-owned Standard scans submit findings and coverage through `record_codex_security_scan_draft`; SDK-owned Standard scans write unsealed canonical files directly. Workbench-backed diff scans use the compact artifacts described below. + +Deep scans run ordinary Standard scan workers. Workers save progress with `complete: false` and submit final results through `record_codex_security_scan_draft`. Their checkpoints and results contain findings and coverage, with optional scope and threat-model context. Pending candidates and their evidence are recorded in worker coverage. Immutable checkpoints store progress across retries, cancellation, and failure. The coordinator passes completed workers' findings and context to the reducer. + +Deep reducer inputs, results, and checkpoints contain findings and optional scope and threat-model context. The host writes the parent scan's unsealed `scan-manifest.json` and `findings.json` from the accepted aggregate, and derives `coverage.json` from the configured include and exclude paths and the coordinator's outcome. The parent completes the scan from these artifacts. If the discovery time limit expires before any source review completes, the parent records partial coverage with that reason. See `scan-contract.md` for canonical field definitions. - A workbench-backed diff scan records all candidates once with `record_codex_security_discovery_candidates({ scanId, candidates })` and reads the canonical candidates with `list_codex_security_candidates({ scanId, cursor?, limit? })`. - The writer validates candidates against assigned source paths, merges rows with the same CWE ids, locations, and optional instance, preserves their text, and assigns deterministic `candidate_id` values. diff --git a/plugins/codex-security/references/scan-contract.md b/plugins/codex-security/references/scan-contract.md index f49de0975..d5f0b105e 100644 --- a/plugins/codex-security/references/scan-contract.md +++ b/plugins/codex-security/references/scan-contract.md @@ -104,9 +104,13 @@ Use CWE taxonomy separately. Do not include file names, line numbers, scan IDs, ## Coverage -`coverage.json` prevents downstream consumers from confusing `not observed` with `not scanned`. +`coverage.json` records scan scope and completion information. Standard and diff summaries also describe reviewed surfaces and outstanding work. -Record: +For a Deep parent scan, the host copies the configured paths into `includePaths` and `excludePaths` and sets `completeness` from the coordinator's outcome. A successful aggregate uses `complete`; `surfaces`, `explicitExclusions`, and `deferred` are empty arrays, and `openQuestions` is omitted. If the configured time limit expires before any review completes, the coordinator writes `partial` and records the explanation in `deferred`. Stopped outcomes follow the [stopped-result recovery rules](#stopped-result-recovery). + +Each Deep worker writes an ordinary Standard result, including its own coverage. A reducer submits `record_codex_security_deep_reduction({ scanId, findings, scope?, threatModel? })`; its saved results and checkpoints contain the accepted findings and optional scope and threat-model context. + +For Standard and diff scans, record: - scan mode and inventory strategy - included and excluded paths @@ -140,7 +144,7 @@ For a whole-repository Deep scan, keep `inventoryStrategy` as `repository`; repe | `directory` | Deterministic non-Git directory inventory | | `custom` | Producer-defined inventory described by detailed receipts | -Use `complete` when the requested scope was fully reviewed, `partial` when in-scope work was deferred, and `unknown` when the producer cannot establish enough coverage to make that distinction. +For Standard and diff scans, use `complete` when the requested scope was fully reviewed, `partial` when in-scope work was deferred, and `unknown` when the producer cannot establish enough coverage to make that distinction. Map detailed ledger closure into completed surface summaries in this order: diff --git a/plugins/codex-security/schemas/tools/deep-reducer.schema.json b/plugins/codex-security/schemas/tools/deep-reducer.schema.json index d9ca75ecb..69726ccdc 100644 --- a/plugins/codex-security/schemas/tools/deep-reducer.schema.json +++ b/plugins/codex-security/schemas/tools/deep-reducer.schema.json @@ -9,7 +9,33 @@ "additionalProperties": false }, "reductionInput": { - "$ref": "codex-security://schemas/tools/scan-draft.schema.json#/$defs/scanDraftInput" + "type": "object", + "properties": { + "complete": { + "type": "boolean", + "description": "Omit or set true when submitting the finished aggregate." + }, + "scanId": { + "$ref": "codex-security://schemas/tools/scan-draft.schema.json#/$defs/scanId" + }, + "handoffClaimToken": { + "$ref": "codex-security://schemas/tools/scan-draft.schema.json#/$defs/handoffClaimToken" + }, + "scope": { + "$ref": "codex-security://schemas/tools/scan-draft.schema.json#/$defs/scope" + }, + "threatModel": { + "$ref": "codex-security://schemas/tools/scan-draft.schema.json#/$defs/threatModel" + }, + "findings": { + "$ref": "codex-security://schemas/tools/scan-draft.schema.json#/$defs/scanDraftInput/properties/findings" + } + }, + "required": [ + "scanId", + "findings" + ], + "additionalProperties": false } } } diff --git a/plugins/codex-security/scripts/deep_scan_workbench.py b/plugins/codex-security/scripts/deep_scan_workbench.py index 80f679524..853094a0a 100644 --- a/plugins/codex-security/scripts/deep_scan_workbench.py +++ b/plugins/codex-security/scripts/deep_scan_workbench.py @@ -1877,6 +1877,7 @@ def finish_deep_scan_locked( """ SELECT 1 FROM deep_scan_workers AS failed WHERE failed.scan_id = ? AND failed.status = 'failed' + AND (? != 'saturated' OR failed.kind != 'discovery') AND ( failed.kind != 'dedup' OR NOT EXISTS ( @@ -1902,10 +1903,14 @@ def finish_deep_scan_locked( ) LIMIT 1 """, - (scan_id,), + (scan_id, args.terminal_reason), ).fetchone() if failed_worker is not None and not failure_capped: raise SystemExit("Deep Scan cannot finish after a worker has failed.") + if args.terminal_reason == "saturated": + # Mark any remaining workers canceled, including those whose own + # cancellation writes failed, so they cannot block completion. + cancel_active_workers(connection, scan_id, now()) active_worker = connection.execute( """ SELECT 1 FROM deep_scan_workers diff --git a/plugins/codex-security/scripts/finalize_scan_contract.py b/plugins/codex-security/scripts/finalize_scan_contract.py index e82dc36f4..58cac8f1f 100644 --- a/plugins/codex-security/scripts/finalize_scan_contract.py +++ b/plugins/codex-security/scripts/finalize_scan_contract.py @@ -1252,12 +1252,9 @@ def _normalize_unsealed_deep_repository_inventory_strategy( *, expected_coverage_mode: str | None, ) -> None: - """Normalize the old Deep workflow label to the ordinary repository inventory.""" + """Label whole-repository Deep scans as using the repository inventory.""" - if ( - expected_coverage_mode == "deep_repository" - and coverage.get("inventoryStrategy") == "deep_repository_repeated_discovery" - ): + if expected_coverage_mode == "deep_repository": coverage["inventoryStrategy"] = "repository" diff --git a/plugins/codex-security/scripts/report_projection.py b/plugins/codex-security/scripts/report_projection.py index 697dda8bd..98b4f00d1 100644 --- a/plugins/codex-security/scripts/report_projection.py +++ b/plugins/codex-security/scripts/report_projection.py @@ -814,7 +814,7 @@ def build_report_markdown( "", _text( scope.get("summary"), - "The scan reviewed the canonical include paths and exclusions listed below.", + "The scan was configured for the include paths and exclusions listed below.", ), "", f"- Scan mode: {coverage['mode']}", diff --git a/plugins/codex-security/scripts/workbench_db.py b/plugins/codex-security/scripts/workbench_db.py index b5227826f..702fcd6bc 100644 --- a/plugins/codex-security/scripts/workbench_db.py +++ b/plugins/codex-security/scripts/workbench_db.py @@ -1534,7 +1534,9 @@ def add_warning() -> None: scan_dir, expected_coverage_mode=expected_coverage_mode(scan), completion_binding=completion_binding, - completion_warnings=warnings, + # Save the finished Deep result as submitted. Worker drafts and + # recovery repairs belong to the stopped-scan path. + completion_warnings=warnings if scan["mode"] != "deep" else None, draft_documents=saved_results.merge_saved_results( scan_dir, scan["id"], @@ -1547,7 +1549,7 @@ def add_warning() -> None: stopped=False, reason="", ) - if current_manifest_path is not None and not already_sealed + if scan["mode"] != "deep" and current_manifest_path is not None and not already_sealed else None, ) add_warning() diff --git a/plugins/codex-security/scripts/workbench_saved_results.py b/plugins/codex-security/scripts/workbench_saved_results.py index 56288ee4c..8c015a65d 100644 --- a/plugins/codex-security/scripts/workbench_saved_results.py +++ b/plugins/codex-security/scripts/workbench_saved_results.py @@ -112,13 +112,13 @@ def _latest_successful_reducer(workers: list[Any]) -> Any | None: ) -def _saved_result_paths(scan_dir: Path, workers: list[Any]) -> Iterator[str]: +def _saved_result_paths(scan_dir: Path, workers: list[Any]) -> Iterator[tuple[str, str | None]]: latest_reducer = _latest_successful_reducer(workers) - def checkpoints(directory: str) -> Iterator[str]: + def checkpoints(directory: str, kind: str | None = None) -> Iterator[tuple[str, str | None]]: for name in _children(scan_dir, directory): if re.fullmatch(r"[0-9a-f]{64}\.json", name): - yield f"{directory}/{name}" + yield f"{directory}/{name}", kind yield from checkpoints("checkpoints") for worker in workers: @@ -137,25 +137,32 @@ def checkpoints(directory: str) -> Iterator[str]: if re.fullmatch(r"attempt-\d+", name) ] for directory in directories: - checkpoint_paths = list(checkpoints(f"{directory}/checkpoints")) + checkpoint_paths = list(checkpoints(f"{directory}/checkpoints", worker["kind"])) if worker["kind"] == "discovery" or checkpoint_paths: - yield f"{directory}/result.json" + yield f"{directory}/result.json", worker["kind"] yield from checkpoint_paths if worker["result_manifest_path"] and ( worker["kind"] == "discovery" or (latest_reducer is not None and worker["id"] == latest_reducer["id"]) ): try: - yield Path(worker["result_manifest_path"]).relative_to(scan_dir).as_posix() + yield ( + Path(worker["result_manifest_path"]).relative_to(scan_dir).as_posix(), + worker["kind"], + ) except ValueError: continue -def _read_saved_result(scan_dir: Path, relative: str, scan_id: str) -> tuple[dict[str, Any], str]: +def _read_saved_result( + scan_dir: Path, relative: str, scan_id: str, *, kind: str | None = None +) -> tuple[dict[str, Any], str]: draft = _read_scan_local_json(scan_dir, relative, "Saved scan checkpoint") if draft.get("scanId") != scan_id: raise ContractError("checkpoint belongs to a different scan") - if not isinstance(draft.get("findings"), list) or not isinstance(draft.get("coverage"), dict): + if not isinstance(draft.get("findings"), list) or not isinstance( + draft.get("coverage", {} if kind == "dedup" else None), dict + ): raise ContractError("checkpoint has no semantic findings or coverage") return draft, _digest(draft) @@ -207,13 +214,13 @@ def _saved_results_changed(db: Any, connection: Any, scan: Any) -> bool: "FROM deep_scan_workers WHERE scan_id = ?", (scan["id"],), ).fetchall() - paths = set(_saved_result_paths(scan_dir, workers)) + paths = dict(_saved_result_paths(scan_dir, workers)) frozen_sources = scan["retained_source_digests_json"] def has_saved_source() -> bool: for path in paths: try: - _read_saved_result(scan_dir, path, scan["id"]) + _read_saved_result(scan_dir, path, scan["id"], kind=paths[path]) return True except (ContractError, OSError, ValueError): continue @@ -244,7 +251,9 @@ def has_saved_source() -> bool: current_sources = dict(published_sources) for path in paths: try: - _, current_sources[path] = _read_saved_result(scan_dir, path, scan["id"]) + _, current_sources[path] = _read_saved_result( + scan_dir, path, scan["id"], kind=paths[path] + ) except (ContractError, OSError, ValueError): continue return current_sources != published_sources @@ -290,23 +299,26 @@ def _recovery_source_digests(db: Any, connection: Any, scan: Any) -> tuple[dict[ else: include_parent = True + workers = connection.execute( + "SELECT id, kind, status, completed_at, artifact_dir, result_manifest_path " + "FROM deep_scan_workers WHERE scan_id = ?", + (scan["id"],), + ).fetchall() + paths = dict(_saved_result_paths(scan_dir, workers)) recovery_sources = dict(frozen_sources or {}) for relative, expected_digest in recovery_sources.items(): try: - _, digest = _read_saved_result(scan_dir, relative, scan["id"]) + _, digest = _read_saved_result(scan_dir, relative, scan["id"], kind=paths.get(relative)) except (ContractError, OSError, ValueError) as exc: raise ContractError("Frozen stopped-scan checkpoint set is incomplete.") from exc if digest != expected_digest: raise ContractError("checkpoint changed after the scan stopped") - workers = connection.execute( - "SELECT id, kind, status, completed_at, artifact_dir, result_manifest_path " - "FROM deep_scan_workers WHERE scan_id = ?", - (scan["id"],), - ).fetchall() - for relative in set(_saved_result_paths(scan_dir, workers)) - recovery_sources.keys(): + for relative in paths.keys() - recovery_sources.keys(): try: - _, recovery_sources[relative] = _read_saved_result(scan_dir, relative, scan["id"]) + _, recovery_sources[relative] = _read_saved_result( + scan_dir, relative, scan["id"], kind=paths[relative] + ) except (ContractError, OSError, ValueError): continue return recovery_sources, include_parent @@ -489,6 +501,7 @@ def merge_saved_results( parent_preserved_sources = recorded source_digests.update(parent_preserved_sources) paths: dict[str, str | None] = {} + reducer_paths: set[str] = set() current_results: set[str] = set() reducer_outputs: list[tuple[Any, str, list[str], int]] = [] reducer = _latest_successful_reducer(workers) @@ -497,6 +510,7 @@ def merge_saved_results( try: latest_reducer = Path(reducer["result_manifest_path"]).relative_to(scan_dir).as_posix() paths[latest_reducer] = None + reducer_paths.add(latest_reducer) except ValueError: warnings.append("Skipped a reducer result outside the scan directory.") @@ -526,6 +540,7 @@ def reducer_output(directory: str, attempt: int, reducer_worker: Any) -> None: paths[result_path] = None for checkpoint_path in checkpoint_paths: paths[checkpoint_path] = None + reducer_paths.update([result_path, *checkpoint_paths]) reducer_outputs.append((reducer_worker, result_path, checkpoint_paths, attempt)) reducer_output(output, int(worker["attempt"] or 0), worker) @@ -570,11 +585,15 @@ def reducer_output(directory: str, attempt: int, reducer_worker: Any) -> None: for relative, worker_id in paths.items(): try: - draft, digest = _read_saved_result(scan_dir, relative, scan_id) + draft, digest = _read_saved_result( + scan_dir, relative, scan_id, kind="dedup" if relative in reducer_paths else None + ) if frozen_source_digests is not None and frozen_source_digests[relative] != digest: raise ContractError("checkpoint changed after the scan stopped") source_digests[relative] = digest - sources.append((relative, draft, worker_id)) + # Recovery expects coverage, but reducer results only contain findings + # and context. Add an empty value after hashing the original result. + sources.append((relative, {"coverage": {}, **draft}, worker_id)) except (ContractError, OSError, ValueError) as exc: if (scan_dir / relative).exists(): warnings.append(f"Preserved unreadable checkpoint {relative}: {exc}") @@ -645,7 +664,7 @@ def reducer_output(directory: str, attempt: int, reducer_worker: Any) -> None: manifest["scan"]["preservedSources"] = source_digests coverage = ( copy.deepcopy(parent["coverage"]) - if parent + if parent and parent["coverage"] else { "completeness": "partial", "mode": binding["coverageMode"], diff --git a/plugins/codex-security/skills/deep-security-scan/SKILL.md b/plugins/codex-security/skills/deep-security-scan/SKILL.md index 72007f5c8..ef9b63035 100644 --- a/plugins/codex-security/skills/deep-security-scan/SKILL.md +++ b/plugins/codex-security/skills/deep-security-scan/SKILL.md @@ -5,7 +5,9 @@ description: Use when the user asks for a deep, exhaustive, multi-pass, or varia # Deep Security Scan -Use `start_codex_security_deep_scan` to run repeated independent workers against the exact requested target and scope. Each worker loads `../../references/core-scan.md` directly and performs the complete ordinary Standard audit, including its own threat map, investigation, source-backed validation, and attack-path reasoning, saving worker-bound checkpoints as results arrive and one final semantic scan draft when its audit finishes. The coordinator aggregates the finished Standard results and writes the parent scan's unsealed `scan-manifest.json`, `findings.json`, and `coverage.json` before returning `{ manifestPath }`. +Use `start_codex_security_deep_scan` to run repeated independent workers against the exact requested target and scope. Each worker reads `../../references/core-scan.md` directly and completes the ordinary Standard audit, saving checkpoints as results arrive and a final scan draft when the audit finishes. + +The coordinator combines the finished findings and writes the parent scan's unsealed `scan-manifest.json`, `findings.json`, and `coverage.json` before returning `{ manifestPath }`. The final report identifies the configured directories and exclusions alongside the findings. ## Phase Ownership diff --git a/plugins/codex-security/tests/test_deep_scan_stop_conditions.py b/plugins/codex-security/tests/test_deep_scan_stop_conditions.py new file mode 100644 index 000000000..253297eab --- /dev/null +++ b/plugins/codex-security/tests/test_deep_scan_stop_conditions.py @@ -0,0 +1,152 @@ +from __future__ import annotations + +import json +from argparse import Namespace + +import pytest +from test_deep_scan_successful_publication import ( + add_worker, + assert_published_aggregate, + complete, +) +from test_deep_scan_successful_publication import ( + publication_scan as publication_scan, +) + + +@pytest.fixture +def saturated_scan(publication_scan, workbench_db): + scan = publication_scan() + completed_result = add_worker(workbench_db, scan) + completed_result.write_text( + json.dumps( + { + "scanId": scan.scan_id, + "complete": True, + "findings": scan.findings, + "coverage": scan.coverage, + } + ) + ) + reducer_result = add_worker(workbench_db, scan) + reducer_result.write_bytes(completed_result.read_bytes()) + with workbench_db: + workbench_db.execute( + "UPDATE deep_scan_workers SET completion_sequence = 1 WHERE id = ?", + (completed_result.parent.name,), + ) + workbench_db.execute( + "UPDATE deep_scan_workers SET kind = 'dedup', merge_state = 'none' WHERE id = ?", + (reducer_result.parent.name,), + ) + workbench_db.execute( + "INSERT INTO deep_scan_dedup_inputs " + "(scan_id, dedup_worker_id, discovery_worker_id, input_order) VALUES (?, ?, ?, 0)", + (scan.scan_id, reducer_result.parent.name, completed_result.parent.name), + ) + workbench_db.execute( + "UPDATE deep_scan_runs SET status = 'running', phase = 'discovery', " + "consecutive_no_new = stop_after_no_new, completion_sequence = 1, " + "max_discovery_runs = 3, discovery_runs_dispatched = 1, " + "manifest_path = NULL, terminal_reason = NULL, completed_at = NULL WHERE scan_id = ?", + (scan.scan_id,), + ) + scan.completed_worker_id = completed_result.parent.name + scan.reducer_id = reducer_result.parent.name + return scan + + +def finish(workbench_api, connection, scan, *, terminal_reason="saturated"): + return workbench_api["deep_scan"].finish_deep_scan( + connection, + Namespace( + scan_id=scan.scan_id, + terminal_reason=terminal_reason, + manifest_path=str(scan.scan_dir / "scan-manifest.json"), + staged_manifest_path=None, + omitted_worker_id=[], + ), + )["deepScan"] + + +def test_saturated_finish_cancels_unfinished_discovery_without_using_its_drafts( + workbench_api, workbench_db, saturated_scan +): + scan = saturated_scan + unfinished = [] + for status in ("queued", "running"): + result = add_worker(workbench_db, scan, status=status) + result.write_text("{unfinished worker draft") + unfinished.append(result) + + finished = finish(workbench_api, workbench_db, scan) + + assert finished["status"] == "succeeded" + assert finished["terminalReason"] == "saturated" + workers = {worker["id"]: worker for worker in finished["workers"]} + assert workers[scan.completed_worker_id]["status"] == "succeeded" + assert workers[scan.reducer_id]["status"] == "succeeded" + for result in unfinished: + worker = workers[result.parent.name] + assert worker["status"] == "canceled" + assert worker["completedAt"] is not None + assert worker["completionSequence"] is None + assert worker["mergeState"] == "none" + assert result.read_text() == "{unfinished worker draft" + + complete(workbench_api, workbench_db, scan) + assert_published_aggregate(scan) + + +def test_saturated_finish_retains_failed_discovery_diagnostic( + workbench_api, workbench_db, saturated_scan +): + scan = saturated_scan + result = add_worker(workbench_db, scan, status="failed") + error = "Worker stopped while persisting its result." + with workbench_db: + workbench_db.execute( + "UPDATE deep_scan_workers SET error_message = ? WHERE id = ?", + (error, result.parent.name), + ) + + finished = finish(workbench_api, workbench_db, scan) + + assert finished["status"] == "succeeded" + failed_worker = next( + worker for worker in finished["workers"] if worker["id"] == result.parent.name + ) + assert failed_worker["status"] == "failed" + assert failed_worker["error"] == error + complete(workbench_api, workbench_db, scan) + assert_published_aggregate(scan) + + +@pytest.mark.parametrize( + ("terminal_reason", "kind"), + [("saturated", "setup"), ("saturated", "dedup"), ("capped", "discovery")], + ids=["saturated-setup", "saturated-reducer", "capped-discovery"], +) +def test_finish_preserves_other_worker_failure_checks( + workbench_api, workbench_db, saturated_scan, terminal_reason, kind +): + scan = saturated_scan + result = add_worker(workbench_db, scan, status="failed") + with workbench_db: + workbench_db.execute( + "UPDATE deep_scan_workers SET kind = ? WHERE id = ?", (kind, result.parent.name) + ) + if terminal_reason == "capped": + workbench_db.execute( + "UPDATE deep_scan_runs SET discovery_runs_dispatched = max_discovery_runs " + "WHERE scan_id = ?", + (scan.scan_id,), + ) + + with pytest.raises(SystemExit, match="after a worker has failed"): + finish(workbench_api, workbench_db, scan, terminal_reason=terminal_reason) + + run = workbench_db.execute( + "SELECT status, terminal_reason FROM deep_scan_runs WHERE scan_id = ?", (scan.scan_id,) + ).fetchone() + assert tuple(run) == ("running", None) diff --git a/plugins/codex-security/tests/test_deep_scan_successful_publication.py b/plugins/codex-security/tests/test_deep_scan_successful_publication.py new file mode 100644 index 000000000..8a144ece4 --- /dev/null +++ b/plugins/codex-security/tests/test_deep_scan_successful_publication.py @@ -0,0 +1,462 @@ +from __future__ import annotations + +import copy +import json +import uuid +from argparse import Namespace +from pathlib import Path +from types import SimpleNamespace + +import pytest +from workbench_test_support import write_checkpoint, write_completed_contract + + +@pytest.fixture +def publication_scan(workbench_api, workbench_db, tmp_path, monkeypatch): + monkeypatch.setenv("CODEX_SECURITY_STATE_DIR", str(tmp_path / "state")) + monkeypatch.setenv("CODEX_HOME", str(tmp_path / "codex-home")) + deep = workbench_api["deep_scan"] + monkeypatch.setattr( + deep, + "_dependencies", + deep.DeepScanDependencies( + **{ + name: workbench_api[ + "preserve_stopped_results_after_transition" + if name == "preserve_stopped_results" + else name + ] + for name in deep.DeepScanDependencies.__dataclass_fields__ + } + ), + ) + + def create(*, mode="deep", scope="."): + target = tmp_path / "target" + target.mkdir() + (target / "subdir").mkdir() + (target / "subdir" / "extract.py").write_text("# Synthetic scan target\n") + scan_dir = tmp_path / "scan" + scan_dir.mkdir(mode=0o700) + registered = workbench_api["register_cli_scan"]( + workbench_db, + Namespace( + repository=str(target), + scan_dir=str(scan_dir), + recipe_json=json.dumps( + { + "config": {}, + "mode": mode, + "repository": str(target), + "target": { + "kind": "repository" if scope == "." else "paths", + "paths": [] if scope == "." else [scope], + }, + } + ), + registration_json_stdin=False, + recipe_json_stdin=False, + parent_scan_id=None, + archive_existing=False, + archived_scan_dir=None, + ), + ) + scan_id = registered["scanId"] + timestamp = workbench_db.execute( + "SELECT started_at FROM scans WHERE id = ?", (scan_id,) + ).fetchone()[0] + if mode == "deep": + # Start with a finished Deep result so these tests only need to save it. + with workbench_db: + workbench_db.execute( + "INSERT INTO deep_scan_runs (scan_id, schema_version, workflow_version, " + "status, phase, workers, subagents, stop_after_no_new, max_discovery_runs, " + "manifest_path, terminal_reason, created_at, updated_at, completed_at) " + "VALUES (?, 1, 'publication-test', 'succeeded', 'terminal', 1, 0, 1, 1, " + "?, 'saturated', ?, ?, ?)", + ( + scan_id, + str(scan_dir / "scan-manifest.json"), + timestamp, + timestamp, + timestamp, + ), + ) + coverage_mode = ( + "scoped_path" if scope != "." else "deep_repository" if mode == "deep" else "repository" + ) + write_completed_contract( + scan_dir, + scan_id, + target, + include_paths=[scope], + relative_path="subdir/extract.py", + coverage_mode=coverage_mode, + inventory_strategy="scoped_path" if scope != "." else "repository", + ) + manifest = json.loads((scan_dir / "scan-manifest.json").read_text()) + findings = json.loads((scan_dir / "findings.json").read_text())["findings"] + coverage = json.loads((scan_dir / "coverage.json").read_text()) + if mode == "deep": + coverage.update(surfaces=[], explicitExclusions=[], deferred=[]) + else: + coverage["openQuestions"] = [{"question": "Which deployment controls apply?"}] + for field in ("documentType", "schemaVersion", "scanId"): + coverage.pop(field) + # Use the same draft format as the writer, before finalization adds metadata. + manifest = {"scan": {key: manifest["scan"][key] for key in ("target", "scope")}} + findings[0]["severity"]["changeConditions"] = "Reassess if the upload route is removed." + findings[0]["provenance"]["sourceFindings"] = [ + {"id": "review-1:candidate-1", "finding": {"summary": "Retained original wording."}} + ] + for name, value in ( + ("scan-manifest.json", manifest), + ("findings.json", {"findings": findings}), + ("coverage.json", coverage), + ): + (scan_dir / name).write_text(json.dumps(value)) + return SimpleNamespace( + scan_id=scan_id, + scan_dir=scan_dir, + timestamp=timestamp, + findings=findings, + coverage=coverage, + ) + + return create + + +def add_worker(connection, scan, *, status="succeeded") -> Path: + worker_id = str(uuid.uuid4()) + output = scan.scan_dir / "workers" / worker_id + output.mkdir(parents=True) + result = output / "result.json" + with connection: + connection.execute( + "INSERT INTO deep_scan_workers (id, scan_id, kind, status, merge_state, " + "prompt_path, artifact_dir, result_manifest_path, attempt, created_at, " + "updated_at, completed_at) VALUES (?, ?, 'discovery', ?, ?, ?, ?, ?, 1, ?, ?, ?)", + ( + worker_id, + scan.scan_id, + status, + "merged" if status == "succeeded" else "none", + str(output / "prompt.md"), + str(output), + str(result), + scan.timestamp, + scan.timestamp, + scan.timestamp, + ), + ) + return result + + +def complete(workbench_api, connection, scan, *, prepare_only=False): + return workbench_api["complete_scan"]( + connection, + Namespace(scan_id=scan.scan_id, claim_token=None, cost_json=None), + prepare_only=prepare_only, + )["scan"] + + +def assert_published_aggregate(scan): + findings = json.loads((scan.scan_dir / "findings.json").read_text())["findings"] + for finding in findings: + for field in ("findingId", "occurrenceId", "fingerprints"): + assert finding.pop(field) + assert findings == scan.findings + coverage = json.loads((scan.scan_dir / "coverage.json").read_text()) + for field in ("documentType", "schemaVersion", "scanId"): + coverage.pop(field) + assert coverage == scan.coverage + assert (scan.scan_dir / "report.md").is_file() + + +@pytest.mark.parametrize("scope", [".", "subdir"], ids=["repository", "scoped"]) +def test_deep_publication_keeps_configured_scope_without_worker_observations( + workbench_api, workbench_db, publication_scan, scope +): + scan = publication_scan(scope=scope) + result = add_worker(workbench_db, scan) + worker_coverage = { + "completeness": "partial", + "surfaces": [ + { + "id": "worker-surface", + "label": "Worker review", + "disposition": "needs_follow_up", + "receiptRefs": [], + } + ], + "explicitExclusions": [{"pattern": "docs/", "reason": "Worker-local exclusion."}], + "deferred": [{"id": "worker-follow-up", "reason": "Review this path again."}], + "openQuestions": [{"question": "Which deployment controls apply?"}], + } + result.write_text( + json.dumps( + { + "scanId": scan.scan_id, + "complete": True, + "findings": [], + "coverage": worker_coverage, + } + ) + ) + source_bytes = result.read_bytes() + + completed = complete(workbench_api, workbench_db, scan) + + assert completed["progress"]["status"] == "complete" + assert_published_aggregate(scan) + assert result.read_bytes() == source_bytes + coverage = json.loads((scan.scan_dir / "coverage.json").read_text()) + assert coverage["mode"] == ("deep_repository" if scope == "." else "scoped_path") + assert coverage["includePaths"] == [scope] + assert coverage["excludePaths"] == [] + report = (scan.scan_dir / "report.md").read_text() + assert f"- Included paths: {scope}" in report + assert "- Excluded paths: none" in report + assert "## Reviewed Surfaces" not in report + assert "Which deployment controls apply?" not in report + + +def test_deep_publication_ignores_empty_canceled_checkpoint( + workbench_api, workbench_db, publication_scan +): + scan = publication_scan() + result = add_worker(workbench_db, scan, status="canceled") + checkpoint = write_checkpoint( + result.parent / "checkpoints", + { + "scanId": scan.scan_id, + "complete": False, + "findings": [], + "coverage": { + "completeness": "partial", + "surfaces": [], + "explicitExclusions": [], + "deferred": [], + }, + }, + ) + source_bytes = checkpoint.read_bytes() + + complete(workbench_api, workbench_db, scan) + + assert_published_aggregate(scan) + assert checkpoint.read_bytes() == source_bytes + assert not result.exists() + + +@pytest.mark.parametrize("old_result", ["unreadable", "removed"]) +def test_deep_publication_does_not_require_old_worker_files( + workbench_api, workbench_db, publication_scan, old_result +): + scan = publication_scan() + result = add_worker(workbench_db, scan) + result.write_text("{old worker output is unavailable") + if old_result == "removed": + result.unlink() + + completed = complete(workbench_api, workbench_db, scan) + + assert completed["warnings"] == [] + assert_published_aggregate(scan) + if old_result == "removed": + assert not result.exists() + else: + assert result.read_text() == "{old worker output is unavailable" + + +def test_deep_prepare_and_complete_preserve_the_same_aggregate( + workbench_api, workbench_db, publication_scan +): + scan = publication_scan() + prepared = complete(workbench_api, workbench_db, scan, prepare_only=True) + assert prepared["progress"]["status"] == "running" + names = ("scan-manifest.json", "findings.json", "coverage.json") + published = {name: (scan.scan_dir / name).read_bytes() for name in names} + + complete(workbench_api, workbench_db, scan, prepare_only=True) + complete(workbench_api, workbench_db, scan) + repeated = complete(workbench_api, workbench_db, scan) + + assert repeated["progress"]["status"] == "complete" + assert {name: (scan.scan_dir / name).read_bytes() for name in names} == published + assert_published_aggregate(scan) + + +@pytest.mark.parametrize( + ("source", "scope", "has_parent"), + [ + ("standard-worker-checkpoint", ".", True), + ("deep-reducer-checkpoint", ".", True), + ("deep-reducer-archived-checkpoint", ".", True), + ("deep-reducer-result", ".", True), + ("deep-reducer-result", "subdir", False), + ], + ids=[ + "standard-worker-checkpoint", + "deep-reducer-checkpoint", + "deep-reducer-archived-checkpoint", + "deep-reducer-result", + "scoped-reducer-without-parent", + ], +) +def test_stopped_deep_scan_still_salvages_saved_findings( + workbench_api, workbench_db, publication_scan, source, scope, has_parent +): + scan = publication_scan(scope=scope) + if not has_parent: + for name in ("scan-manifest.json", "findings.json", "coverage.json"): + (scan.scan_dir / name).unlink() + with workbench_db: + workbench_db.execute( + "UPDATE deep_scan_runs SET status = 'running', phase = 'reducing', " + "terminal_reason = NULL, completed_at = NULL WHERE scan_id = ?", + (scan.scan_id,), + ) + result = add_worker( + workbench_db, scan, status="succeeded" if source == "deep-reducer-result" else "running" + ) + if source.startswith("deep-reducer"): + with workbench_db: + workbench_db.execute( + "UPDATE deep_scan_workers SET kind = 'dedup', merge_state = 'none' WHERE id = ?", + (result.parent.name,), + ) + later_finding = copy.deepcopy(scan.findings[0]) + later_finding["identity"]["anchor"] = "later-checkpoint-finding" + later_finding["summary"] = "Finding saved after the last completed aggregate." + saved = { + "scanId": scan.scan_id, + "complete": source == "deep-reducer-result", + "findings": [later_finding], + } + if source == "standard-worker-checkpoint": + saved["coverage"] = { + "completeness": "partial", + "surfaces": [], + "explicitExclusions": [], + "deferred": [], + } + if source == "deep-reducer-result": + checkpoint = None + result.write_text(json.dumps(saved)) + else: + checkpoint_root = ( + result.parent / "attempts" / "attempt-01" + if source == "deep-reducer-archived-checkpoint" + else result.parent + ) + checkpoint = write_checkpoint(checkpoint_root / "checkpoints", saved) + result.write_text("{interrupted worker output") + result_bytes = result.read_bytes() + + stopped = workbench_api["fail_scan"]( + workbench_db, + Namespace( + scan_id=scan.scan_id, claim_token=None, cost_json=None, message="Scan interrupted." + ), + )["scan"] + + assert stopped["progress"]["status"] == "failed" + manifest = json.loads((scan.scan_dir / "scan-manifest.json").read_text()) + coverage = json.loads((scan.scan_dir / "coverage.json").read_text()) + assert manifest["scan"]["status"] == "failed" + assert coverage["completeness"] == "partial" + findings = json.loads((scan.scan_dir / "findings.json").read_text())["findings"] + expected_summaries = {later_finding["summary"]} + if has_parent: + expected_summaries.add(scan.findings[0]["summary"]) + assert {finding["summary"] for finding in findings} == expected_summaries + + artifact_names = ("scan-manifest.json", "findings.json", "coverage.json") + published = {name: (scan.scan_dir / name).read_bytes() for name in artifact_names} + recovered = workbench_api["recover_scan_results"]( + workbench_db, Namespace(scan_id=scan.scan_id) + )["scan"] + assert recovered["findingCount"] == len(expected_summaries) + assert {name: (scan.scan_dir / name).read_bytes() for name in artifact_names} == published + if checkpoint is not None: + assert json.loads(checkpoint.read_text()) == saved + assert result.read_bytes() == result_bytes + + +@pytest.mark.parametrize("source", ["result", "checkpoint", "parent-checkpoint"]) +def test_stopped_deep_scan_ignores_non_reducer_sources_without_coverage( + workbench_api, workbench_db, publication_scan, source +): + scan = publication_scan() + with workbench_db: + workbench_db.execute( + "UPDATE deep_scan_runs SET status = 'running', phase = 'discovery', " + "terminal_reason = NULL, completed_at = NULL WHERE scan_id = ?", + (scan.scan_id,), + ) + invalid_finding = copy.deepcopy(scan.findings[0]) + invalid_finding["identity"]["anchor"] = "non-reducer-without-coverage" + invalid_finding["summary"] = "Finding from a non-reducer artifact missing required coverage." + saved = { + "scanId": scan.scan_id, + "complete": source == "result", + "findings": [invalid_finding], + } + if source == "parent-checkpoint": + source_path = write_checkpoint(scan.scan_dir / "checkpoints", saved) + else: + result = add_worker( + workbench_db, scan, status="succeeded" if source == "result" else "running" + ) + if source == "result": + result.write_text(json.dumps(saved)) + source_path = result + else: + source_path = write_checkpoint(result.parent / "checkpoints", saved) + source_bytes = source_path.read_bytes() + source_relative = source_path.relative_to(scan.scan_dir).as_posix() + + stopped = workbench_api["fail_scan"]( + workbench_db, + Namespace( + scan_id=scan.scan_id, claim_token=None, cost_json=None, message="Scan interrupted." + ), + )["scan"] + + assert stopped["progress"]["status"] == "failed" + assert stopped["resultsRecoveryNeeded"] is False + findings = json.loads((scan.scan_dir / "findings.json").read_text())["findings"] + assert [finding["summary"] for finding in findings] == [scan.findings[0]["summary"]] + manifest = json.loads((scan.scan_dir / "scan-manifest.json").read_text()) + frozen = json.loads( + workbench_db.execute( + "SELECT retained_source_digests_json FROM scans WHERE id = ?", (scan.scan_id,) + ).fetchone()[0] + ) + assert manifest["scan"]["preservedSources"] == frozen + assert source_relative not in frozen + artifact_names = ("scan-manifest.json", "findings.json", "coverage.json") + published = {name: (scan.scan_dir / name).read_bytes() for name in artifact_names} + + recovered = workbench_api["recover_scan_results"]( + workbench_db, Namespace(scan_id=scan.scan_id) + )["scan"] + + assert recovered["findingCount"] == 1 + assert recovered["resultsRecoveryNeeded"] is False + assert {name: (scan.scan_dir / name).read_bytes() for name in artifact_names} == published + assert source_path.read_bytes() == source_bytes + + +def test_standard_publication_preserves_deliberately_partial_coverage( + workbench_api, workbench_db, publication_scan +): + scan = publication_scan(mode="standard") + scan.coverage["completeness"] = "partial" + scan.coverage["deferred"] = [{"id": "remaining-review", "reason": "Another surface remains."}] + (scan.scan_dir / "coverage.json").write_text(json.dumps(scan.coverage)) + + complete(workbench_api, workbench_db, scan) + + assert_published_aggregate(scan) diff --git a/plugins/codex-security/tests/test_finalize_scan_contract.py b/plugins/codex-security/tests/test_finalize_scan_contract.py index c9a50e501..9cedd21ad 100644 --- a/plugins/codex-security/tests/test_finalize_scan_contract.py +++ b/plugins/codex-security/tests/test_finalize_scan_contract.py @@ -618,7 +618,7 @@ def test_finalize_rejects_deep_inventory_strategy_alias_outside_deep(self) -> No expected_coverage_mode=expected_mode, ) - def test_finalize_rejects_unknown_deep_inventory_strategy_alias(self) -> None: + def test_finalize_rejects_unknown_inventory_without_selected_deep_mode(self) -> None: self.coverage["mode"] = "deep_repository" self.coverage["inventoryStrategy"] = "deep_repository_repeated_discovery_v2" self.write_scan() @@ -627,10 +627,7 @@ def test_finalize_rejects_unknown_deep_inventory_strategy_alias(self) -> None: FINALIZER.ContractError, "coverage.schema.inventoryStrategy: unsupported value", ): - FINALIZER.finalize_scan( - self.scan_dir, - expected_coverage_mode="deep_repository", - ) + FINALIZER.finalize_scan(self.scan_dir) def test_finalize_rejects_sealed_deep_inventory_strategy_alias(self) -> None: self.coverage["mode"] = "deep_repository" diff --git a/plugins/codex-security/tests/test_scan_contract_examples.py b/plugins/codex-security/tests/test_scan_contract_examples.py index 26e43286b..f3e27451b 100644 --- a/plugins/codex-security/tests/test_scan_contract_examples.py +++ b/plugins/codex-security/tests/test_scan_contract_examples.py @@ -54,10 +54,14 @@ def test_schemas_are_valid_draft_2020_12(self) -> None: with self.subTest(schema=schema_path.name): Draft202012Validator.check_schema(read_json(schema_path)) - def test_deep_reducer_schema_preserves_complete_standard_results(self) -> None: + def test_deep_reducer_schema_accepts_standard_findings_without_coverage(self) -> None: common_schema = read_json(SCHEMA_DIR / "definitions" / "artifact-common.schema.json") scan_draft_schema = read_json(SCHEMA_DIR / "tools" / "scan-draft.schema.json") reducer_schema = read_json(SCHEMA_DIR / "tools" / "deep-reducer.schema.json") + reduction_input = reducer_schema["$defs"]["reductionInput"] + self.assertNotIn("coverage", reduction_input["properties"]) + self.assertEqual(set(reduction_input["required"]), {"scanId", "findings"}) + self.assertFalse(reduction_input["additionalProperties"]) registry = Registry().with_resources( (schema["$id"], Resource.from_contents(schema)) for schema in (common_schema, scan_draft_schema) @@ -78,10 +82,8 @@ def test_deep_reducer_schema_preserves_complete_standard_results(self) -> None: "provenance": {"source": "local_plugin"}, } coverage = { - "completeness": "complete", - "surfaces": [], - "explicitExclusions": [], - "deferred": [], + "surfaces": [{"label": "HTTP responses", "disposition": "reported"}], + "explicitExclusions": [{"pattern": "docs/", "reason": "Documentation only."}], } Draft202012Validator.check_schema(reducer_schema) @@ -93,19 +95,64 @@ def test_deep_reducer_schema_preserves_complete_standard_results(self) -> None: request = { "scanId": "7fc17317-9594-49e0-b06a-d72fd7e14bba", "findings": [finding], - "coverage": coverage, } validator.validate(request) validator.validate( { **request, + "complete": True, + "handoffClaimToken": "2ea75b4f-f9b2-49b4-a5a9-2a8de8ca9047", "scope": {"summary": "HTTP responses"}, "threatModel": {"summary": "Untrusted requests reach responses."}, } ) validator.validate({**request, "findings": []}) + self.assertFalse(validator.is_valid({**request, "coverage": coverage})) + standard_validator = Draft202012Validator( + scan_draft_schema, registry=registry, format_checker=FormatChecker() + ) + standard_request = { + **request, + "coverage": {**coverage, "completeness": "complete", "deferred": []}, + } + standard_validator.validate(standard_request) + self.assertFalse(standard_validator.is_valid(request)) + self.assertFalse(standard_validator.is_valid({**request, "coverage": coverage})) + for completeness in ("complete", "partial", "unknown"): + with self.subTest(standard_completeness=completeness): + standard_coverage_request = { + **standard_request, + "coverage": { + **standard_request["coverage"], + "completeness": completeness, + }, + } + standard_validator.validate(standard_coverage_request) + self.assertFalse(validator.is_valid(standard_coverage_request)) + self.assertFalse( + standard_validator.is_valid( + { + **standard_request, + "coverage": {**standard_request["coverage"], "completeness": "invalid"}, + } + ) + ) + for missing_field in ("completeness", "surfaces", "explicitExclusions", "deferred"): + with self.subTest(standard_missing_coverage_field=missing_field): + self.assertFalse( + standard_validator.is_valid( + { + **standard_request, + "coverage": { + field: value + for field, value in standard_request["coverage"].items() + if field != missing_field + }, + } + ) + ) for extra_field in ( "source_worker_id", "unknown_field", @@ -128,22 +175,17 @@ def test_deep_reducer_schema_preserves_complete_standard_results(self) -> None: "provenance", ): with self.subTest(missing_field=missing_field): + incomplete_finding = { + field: value for field, value in finding.items() if field != missing_field + } + self.assertFalse(validator.is_valid({**request, "findings": [incomplete_finding]})) self.assertFalse( - validator.is_valid( - { - **request, - "findings": [ - { - field: value - for field, value in finding.items() - if field != missing_field - } - ], - } + standard_validator.is_valid( + {**standard_request, "findings": [incomplete_finding]} ) ) - for missing_field in ("scanId", "findings", "coverage"): + for missing_field in ("scanId", "findings"): with self.subTest(missing_field=missing_field): self.assertFalse( validator.is_valid( diff --git a/plugins/codex-security/tests/test_workbench_completion_binding.py b/plugins/codex-security/tests/test_workbench_completion_binding.py index 632a4f0e8..5eed724c6 100644 --- a/plugins/codex-security/tests/test_workbench_completion_binding.py +++ b/plugins/codex-security/tests/test_workbench_completion_binding.py @@ -462,7 +462,7 @@ def test_completion_keeps_recoverable_prewrite_failures_resumable( assert completed["findingCount"] == 1 -def test_deep_completion_recovers_malformed_inventory_without_dropping_findings( +def test_deep_completion_derives_inventory_without_downgrading_coverage( tmp_path: Path, ) -> None: for index, inventory in enumerate((None, "", "invalid_strategy")): @@ -481,13 +481,11 @@ def test_deep_completion_recovers_malformed_inventory_without_dropping_findings( assert completed["scan"]["progress"]["status"] == "complete" assert completed["scan"]["findingCount"] == 1 - assert completed["scan"]["warnings"] == [ - "Recovered malformed Deep Scan inventory strategy; marked coverage as partial." - ] + assert completed["scan"]["warnings"] == [] sealed_coverage = json.loads(coverage_path.read_text()) assert sealed_coverage["mode"] == "deep_repository" assert sealed_coverage["inventoryStrategy"] == "repository" - assert sealed_coverage["completeness"] == "partial" + assert sealed_coverage["completeness"] == "complete" assert len(json.loads((scan_dir / "findings.json").read_text())["findings"]) == 1 assert (scan_dir / "report.md").is_file() diff --git a/plugins/codex-security/tests/test_workbench_deep_scan.py b/plugins/codex-security/tests/test_workbench_deep_scan.py index b8111a6f4..f7cf02a00 100644 --- a/plugins/codex-security/tests/test_workbench_deep_scan.py +++ b/plugins/codex-security/tests/test_workbench_deep_scan.py @@ -3944,20 +3944,6 @@ def test_discovery_buffer_prefix_dedup_and_saturation_are_transactional( manifest = scan_dir / "artifacts" / "deep_discovery" / "coordinator-manifest.json" manifest.write_text("{}\n") - active_rejected = run_workbench( - state_dir, - "finish-deep-scan", - "--scan-id", - scan_id, - "--terminal-reason", - "saturated", - "--manifest-path", - str(manifest), - environment=deep_environment(codex_home), - check=False, - ) - assert "while workers are active" in str(active_rejected["stderr"]) - late_result.write_text("{}\n") accepted_late = upsert_worker( state_dir, diff --git a/sdk/typescript/tests-ts/deep-scan-reducer-recovery.test.ts b/sdk/typescript/tests-ts/deep-scan-reducer-recovery.test.ts index c33104c52..b3905c4ab 100644 --- a/sdk/typescript/tests-ts/deep-scan-reducer-recovery.test.ts +++ b/sdk/typescript/tests-ts/deep-scan-reducer-recovery.test.ts @@ -14,7 +14,7 @@ function bundledFunction(runtime: string, name: string): string { return source; } -test("keeps every advertised Deep worker tool within Codex's name limit", async () => { +test("advertises distinct Standard worker and Deep reducer contracts", async () => { const runtime = await loadBundledRuntime(); const method = / compactArtifactServer\(request\) \{[\s\S]*?\n \}/u.exec( runtime, @@ -83,7 +83,12 @@ test("keeps every advertised Deep worker tool within Codex's name limit", async .split("\n") .map((line) => JSON.parse(line) as { id?: number; result?: unknown }) .find((message) => message.id === 2)?.result as - | { tools: Array<{ name: string }> } + | { + tools: Array<{ + name: string; + inputSchema: { properties: Record }; + }>; + } | undefined; expect(response).toBeDefined(); expect(response!.tools.length).toBeGreaterThan(0); @@ -92,11 +97,22 @@ test("keeps every advertised Deep worker tool within Codex's name limit", async 64, ); } - if (layout === "reducer") { - expect(response!.tools.map((tool) => tool.name)).toContain( - "record_codex_security_deep_reduction", + const recordTool = response!.tools.find( + (tool) => + tool.name === + (layout === "reducer" + ? "record_codex_security_deep_reduction" + : "record_codex_security_scan_draft"), + ); + expect(recordTool).toBeDefined(); + expect(recordTool!.inputSchema.properties).toHaveProperty("findings"); + expect(recordTool!.inputSchema.properties).toHaveProperty("scope"); + if (layout === "reducer") + expect(recordTool!.inputSchema.properties).not.toHaveProperty( + "coverage", ); - } + else + expect(recordTool!.inputSchema.properties).toHaveProperty("coverage"); } } finally { rmSync(root, { recursive: true, force: true }); diff --git a/sdk/typescript/tests-ts/runtime.test.ts b/sdk/typescript/tests-ts/runtime.test.ts index 07c142f1f..d086a6295 100644 --- a/sdk/typescript/tests-ts/runtime.test.ts +++ b/sdk/typescript/tests-ts/runtime.test.ts @@ -294,9 +294,10 @@ describe("plugin runtime preparation", () => { ), ); const runtime = brotliDecompressSync(Buffer.concat(parts)).toString("utf8"); - const source = /function buildFindings\(findings\) \{[\s\S]*?\n\}/u.exec( - runtime, - )?.[0]; + const source = + /function buildFindings\(findings, mode\) \{[\s\S]*?\n\}/u.exec( + runtime, + )?.[0]; expect(source).toBeDefined(); const buildFindings = new Function( "semanticIdentifier",