From 76b433b6889f049d7aa8ef3f9b4fd7ada56e9f44 Mon Sep 17 00:00:00 2001 From: Rudy Celekli Date: Mon, 21 Sep 2026 11:52:26 -0400 Subject: [PATCH 1/5] fix(security): preserve scan execution evidence through CLI and MCP --- docs/security/scan-execution-evidence.md | 103 +++++++ src/cli/commands/security.ts | 272 ++++++++++------- src/cli/utils/ci-output.ts | 55 +++- .../handlers/security-handlers.ts | 249 ++++++++++----- src/coordination/protocols/security-audit.ts | 100 +++--- src/coordination/result-saver.ts | 92 ++++-- src/domains/security-compliance/interfaces.ts | 5 + .../security-compliance/scan-discovery.ts | 110 +++++++ .../security-compliance/scan-evidence.ts | 59 ++++ .../services/scanners/sast-scanner.ts | 198 +++++++----- .../services/semgrep-integration.ts | 97 ++++-- src/mcp/handlers/domain-handler-configs.ts | 60 +++- src/mcp/protocol-server.ts | 5 +- src/mcp/types.ts | 2 + .../security-scan-execution-receipts.test.ts | 289 ++++++++++++++++++ .../security/vulnerability-detection.test.ts | 27 +- tests/unit/cli/ci-output.test.ts | 4 +- .../cli/commands/security-evidence.test.ts | 238 +++++++++++++++ .../unit/cli/security-evidence-output.test.ts | 32 ++ .../security-audit-evidence.test.ts | 130 ++++++++ .../scanners/scan-evidence.test.ts | 183 +++++++++++ .../security-discovery.test.ts | 75 +++++ .../security-scanner.test.ts | 46 +-- .../semgrep-evidence.test.ts | 144 +++++++++ 24 files changed, 2166 insertions(+), 409 deletions(-) create mode 100644 docs/security/scan-execution-evidence.md create mode 100644 src/domains/security-compliance/scan-discovery.ts create mode 100644 src/domains/security-compliance/scan-evidence.ts create mode 100644 tests/integration/security-scan-execution-receipts.test.ts create mode 100644 tests/unit/cli/commands/security-evidence.test.ts create mode 100644 tests/unit/cli/security-evidence-output.test.ts create mode 100644 tests/unit/coordination/security-audit-evidence.test.ts create mode 100644 tests/unit/domains/security-compliance/scanners/scan-evidence.test.ts create mode 100644 tests/unit/domains/security-compliance/security-discovery.test.ts create mode 100644 tests/unit/domains/security-compliance/semgrep-evidence.test.ts diff --git a/docs/security/scan-execution-evidence.md b/docs/security/scan-execution-evidence.md new file mode 100644 index 000000000..1480d4992 --- /dev/null +++ b/docs/security/scan-execution-evidence.md @@ -0,0 +1,103 @@ +# Security scan execution evidence + +Security findings and execution completeness are separate results. A completed +scan may contain critical findings. An empty finding list may mean that no +analysis ran. + +This implements the execution-truth portion of [#694](https://github.com/proffesor-for-testing/agentic-qe/issues/694). +It does not provide full SAST assurance, add a secret-scanning engine, or close +all of that issue's scope and reuse requirements. + +## Receipt and counting semantics + +`SASTResult.evidence` is additive and uses `schemaVersion: 1`. The production +SAST scanner supplies it. Consumers treat a legacy result without evidence as +unverified; they must not infer successful analysis from its finding count or +old coverage numbers. + +- `requestedPaths` is captured before engine execution. Paths are normalized + absolute paths and deduplicated lexically; symlink aliases are not resolved. + `duplicateInputs` counts repeated normalized inputs. +- Each built-in file analysis records its disposition, SHA-256 of the exact + bytes read when readable, `readLines`, and `analyzedLines`. Unsupported + readable inputs have a digest and read count but zero analyzed lines. +- `coverage.filesScanned` and `linesScanned` count completed built-in JS/TS + pattern analyses. `rulesApplied` counts unique selected rules actually + evaluated, with `rulesAppliedScope: 'built-in-patterns'`. It is independent + of the number of findings. Unknown requested rule-set IDs are rejected. +- Engines carry their requested/required flags, execution status, declared + scope, known rule IDs/digest, errors, and limitations. Unknown external + file membership and rule coverage remain unknown. +- `completeness: complete` means the **declared required scope** completed; + `partial` retains completed work alongside gaps; `none` establishes no + completed required file analysis. These values never mean a system is safe. + +Line counts retain the existing `content.split('\n').length` convention. +The per-engine digest identifies bytes read, not an atomic repository snapshot: +files can change between discovery and reads, or between independent engines. + +## Discovery and engine boundaries + +CLI and comprehensive MCP scanning share the `aqe-security-source-files@1` +discovery policy. Directory scans select established source extensions, exclude +known dependency/build/private-state directories, and do not follow observed +symlinks. Direct file targets are retained so unsupported inputs are visible. +Missing/unreadable roots, unreadable subtrees, and file/depth limits have +separate discovery dispositions. A completely inventoried empty directory does +not imply any analysis executed. Limits default to 5,000 files and 64 levels. + +The SAST domain runs its existing JavaScript/TypeScript patterns. Comprehensive +MCP scanning also runs its existing generic source-text patterns and optional +manifest-name advisories. Its `filesScanned` counts unique requested source +paths with a completed analysis; the nested `coverage` retains the narrower +built-in SAST counts. Generic patterns on Python or other source languages do +not establish language-specific SAST coverage. `deepAnalysisPerformed` now +requires a returned built-in SAST execution receipt; it is not a claim of +semantic completeness. + +Semgrep remains optional (`required: false`). Its absence, process failure, +malformed output, partial errors, or clean completion are distinct. Valid +findings survive partial output. Semgrep still targets the common parent +directory and can return findings outside the requested file list; its receipt +makes that scope explicit and does not invent per-file/rule coverage. An +optional Semgrep failure does not invalidate completed required built-in +checks. Callers requiring Semgrep must inspect that engine's receipt. + +## Consumer changes + +- `aqe security` now actually runs SAST by default and passes `FilePath` values + to the domain API. Incomplete, unavailable, failed, or unverified requested + checks exit nonzero. Complete scans retain the existing severity codes: + critical/high = 1, medium = 2, otherwise 0. +- CLI text, JSON, Markdown, and SARIF retain execution status. SARIF + `executionSuccessful` describes execution, independently of findings. The + CLI's existing `--dast` placeholder is explicitly not-run. Compliance checks + preserve skipped/unverified/failed execution separately from violations. +- `security_scan_comprehensive` retains findings, execution receipts, + discovery, scope limitations, and actual counts through the registered MCP + path. Its result can be `partial`, `unavailable`, or `unverified` in addition + to `completed`. Transport/task success does not mean scan completeness. + `targetUrl` describes the DAST target; current receiptless DAST output remains + unverified. Requested compliance that this task does not implement is + explicitly not-run. Saved security reports also retain execution status. +- The exported TypeScript `SecurityAuditProtocol` no longer estimates a clean + secret scan. It reports that implementation unavailable and blocks its + deployment recommendation when requested checks are failed, incomplete, or + unverified, including the protocol's placeholder compliance reports. Trigger + scope is honored: pre-release requires secrets; dependency-update requests + dependency scanning. This protocol is tested directly; the registered comprehensive + MCP route uses a different task handler. + +Security receipt errors use bounded codes/static explanations rather than raw +provider error text. Findings retain the existing source snippets; receipts +are not a redaction mechanism for findings or user-supplied paths/URLs. + +## Remaining work in #694 + +This change does not add an atomic source snapshot, alias identity resolution, +per-file Semgrep execution proof, a mandatory-external-engine policy, or new +DAST/secret-scanner implementations. It does not add cross-run receipt reuse +or integration with a general release gate. The comprehensive MCP tool already +bypasses the session result cache; integration tests verify that changing a +file and repeating the same call returns a fresh digest and findings. That is +a freshness control, not a new cache invalidation mechanism. diff --git a/src/cli/commands/security.ts b/src/cli/commands/security.ts index 73ca387d3..ffe9caea6 100644 --- a/src/cli/commands/security.ts +++ b/src/cli/commands/security.ts @@ -7,8 +7,11 @@ import { Command } from 'commander'; import chalk from 'chalk'; import type { CLIContext } from '../handlers/interfaces.js'; -import { walkSourceFiles } from '../utils/file-discovery.js'; -import { type OutputFormat, type SecurityScanResult, type SecurityVulnerability, writeOutput, toJSON, toSARIF, securityToMarkdown } from '../utils/ci-output.js'; +import { discoverSecurityFiles } from '../../domains/security-compliance/scan-discovery.js'; +import { FilePath } from '../../shared/value-objects/index.js'; +import type { SecurityComplianceAPI } from '../../domains/security-compliance/plugin.js'; +import type { ComplianceReport, Vulnerability } from '../../domains/security-compliance/interfaces.js'; +import { type OutputFormat, type SecurityScanResult, type SecurityVulnerability, type SecurityCheckResult, writeOutput, toJSON, toSARIF, securityToMarkdown } from '../utils/ci-output.js'; export function createSecurityCommand( context: CLIContext, @@ -96,137 +99,172 @@ export function createSecurityCommand( return; // Don't fall through to SAST/DAST path } - try { - const format = options.format as OutputFormat; - - if (format === 'text') { - console.log(chalk.blue(`\n Running security scan on ${options.target}...\n`)); - } - - const securityAPI = await context.kernel!.getDomainAPIAsync!<{ - runSASTScan(files: string[]): Promise<{ success: boolean; value?: unknown; error?: Error }>; - runDASTScan(urls: string[]): Promise<{ success: boolean; value?: unknown; error?: Error }>; - runComplianceCheck(standardId: string): Promise<{ success: boolean; value?: unknown; error?: Error }>; - }>('security-compliance'); + const format = options.format as OutputFormat; + const runSAST = options.sast || (!options.dast && !options.compliance); + const frameworks: string[] = options.compliance + ? options.compliance.split(',').map((value: string) => value.trim()).filter(Boolean) + : []; + const scanResult: SecurityScanResult = { + vulnerabilities: [], target: options.target, + scanType: [runSAST && 'SAST', options.dast && 'DAST', frameworks.length > 0 && 'Compliance'].filter(Boolean).join('+'), + checks: [], status: 'not-run', + }; + const checks = scanResult.checks!; + try { + const securityAPI = await context.kernel!.getDomainAPIAsync!('security-compliance'); if (!securityAPI) { - console.log(chalk.red('Security domain not available')); - return; - } - - const path = await import('path'); - const targetPath = path.resolve(options.target); - - // Fix #280: Use shared file discovery supporting all languages - const files = walkSourceFiles(targetPath, { includeTests: true }); - - if (files.length === 0) { - console.log(chalk.yellow('No files found to scan')); - return; - } - - if (format === 'text') { - console.log(chalk.gray(` Scanning ${files.length} files...\n`)); - } - const scanResult: SecurityScanResult = { - vulnerabilities: [], - target: options.target, - scanType: [options.sast && 'SAST', options.dast && 'DAST', options.compliance && 'Compliance'].filter(Boolean).join('+') || 'SAST', - }; - - // Run SAST if requested - if (options.sast) { - if (format === 'text') console.log(chalk.blue(' SAST Scan:')); - const sastResult = await securityAPI.runSASTScan(files); - if (sastResult.success && sastResult.value) { - const result = sastResult.value as { vulnerabilities?: Array<{ severity: string; type: string; file: string; line: number; message: string }> }; - const vulns = result.vulnerabilities || []; - scanResult.vulnerabilities = vulns as SecurityVulnerability[]; - if (format === 'text') { - if (vulns.length === 0) { - console.log(chalk.green(' * No vulnerabilities found')); - } else { - console.log(chalk.yellow(` ! Found ${vulns.length} potential issues:`)); - for (const v of vulns.slice(0, 10)) { - const color = v.severity === 'high' ? chalk.red : v.severity === 'medium' ? chalk.yellow : chalk.gray; - console.log(color(` [${v.severity}] ${v.type}: ${v.file}:${v.line}`)); - console.log(chalk.gray(` ${v.message}`)); - } - if (vulns.length > 10) { - console.log(chalk.gray(` ... and ${vulns.length - 10} more`)); + checks.push({ name: scanResult.scanType, status: 'failed', reason: 'Security domain not available' }); + } else { + if (runSAST) { + const { files, discovery } = await discoverSecurityFiles(options.target); + scanResult.discovery = discovery; + if (files.length === 0) { + checks.push({ name: 'SAST', status: 'not-run', reason: 'No analyzable input was discovered; no SAST scan ran.' }); + } else { + try { + const sastResult = await securityAPI.runSASTScan(files.map(file => FilePath.create(file))); + if (sastResult.success && sastResult.value) { + const result = sastResult.value; + scanResult.vulnerabilities = result.vulnerabilities.map(formatVulnerability); + scanResult.coverage = result.evidence ? result.coverage : undefined; + scanResult.evidence = result.evidence; + checks.push({ + name: 'SAST', status: result.evidence?.completeness ?? 'unverified', + ...(!result.evidence ? { reason: 'The scanner returned no execution receipts.' } : {}), + }); + } else { + checks.push({ name: 'SAST', status: 'failed', reason: !sastResult.success ? securityFailureReason('SAST scan', sastResult.error) : 'SAST scan returned no result' }); } + } catch (error) { + checks.push({ name: 'SAST', status: 'failed', reason: securityFailureReason('SAST scan', error) }); } } - } else if (format === 'text') { - console.log(chalk.red(` x SAST failed: ${sastResult.error?.message || 'Unknown error'}`)); + if (discovery.status !== 'complete') { + checks.push({ name: 'Source discovery', status: discovery.status, reason: 'Requested source discovery did not complete; inspect discovery issues.' }); + } } - if (format === 'text') console.log(''); - } - // Run compliance check if requested - if (options.compliance) { - const frameworks = options.compliance.split(','); - if (format === 'text') console.log(chalk.blue(` Compliance Check (${frameworks.join(', ')}):`)); - // Run compliance check for each framework - const compResults = await Promise.all( - frameworks.map((f: string) => securityAPI.runComplianceCheck(f.trim())) - ); - const compResult = compResults[0]; // Primary result for display - if (compResult.success && compResult.value) { - const result = compResult.value as { compliant: boolean; issues?: Array<{ framework: string; issue: string }> }; - scanResult.compliance = result; - if (format === 'text') { - if (result.compliant) { - console.log(chalk.green(' * Compliant with all frameworks')); - } else { - console.log(chalk.yellow(' ! Compliance issues found:')); - for (const issue of (result.issues || []).slice(0, 5)) { - console.log(chalk.yellow(` [${issue.framework}] ${issue.issue}`)); + if (frameworks.length > 0) { + const issues: Array<{ framework: string; issue: string }> = []; + let compliant = true; + for (const framework of frameworks) { + try { + const outcome = await securityAPI.runComplianceCheck(framework); + if (!outcome.success || !outcome.value) { + compliant = false; + checks.push({ name: `Compliance:${framework}`, status: 'failed', reason: !outcome.success ? securityFailureReason('Compliance check', outcome.error) : 'Compliance check returned no result' }); + continue; } + const report = outcome.value; + const legacy = report as ComplianceReport & { compliant?: boolean; issues?: Array<{ framework: string; issue: string }> }; + const violations = report.violations ?? []; + const skipped = report.skippedRules ?? []; + const passed = report.passedRules ?? []; + const measured = Array.isArray(report.violations) && Array.isArray(report.passedRules) && Array.isArray(report.skippedRules); + const status = !measured ? 'unverified' : skipped.length > 0 ? 'partial' : passed.length + violations.length > 0 ? 'complete' : 'none'; + checks.push({ name: `Compliance:${framework}`, status, ...(skipped.length ? { reason: `${skipped.length} compliance rules were skipped.` } : {}) }); + issues.push(...violations.map(violation => ({ framework, issue: violation.details }))); + if (legacy.issues) issues.push(...legacy.issues); + if (status !== 'complete' || violations.length > 0 || legacy.compliant === false) compliant = false; + } catch (error) { + compliant = false; + checks.push({ name: `Compliance:${framework}`, status: 'failed', reason: securityFailureReason('Compliance check', error) }); } } - } else if (format === 'text') { - console.log(chalk.red(` x Compliance check failed: ${compResult.error?.message || 'Unknown error'}`)); + scanResult.compliance = { compliant, issues }; } - if (format === 'text') console.log(''); } - - // DAST note - if (options.dast && format === 'text') { - console.log(chalk.gray('Note: DAST requires running application URLs. Use --target with URLs for DAST scanning.')); - } - - // Format-aware output - if (format === 'json') { - writeOutput(toJSON(scanResult), options.output); - } else if (format === 'sarif') { - writeOutput(toSARIF(scanResult), options.output); - } else if (format === 'markdown') { - writeOutput(securityToMarkdown(scanResult), options.output); - } else { - console.log(chalk.green(' Security scan complete\n')); - } - - // Exit codes: 1 = critical/high vulns, 2 = medium-only vulns, 0 = low/none - const hasHighSeverity = scanResult.vulnerabilities.some(v => - v.severity === 'high' || v.severity === 'critical' - ); - const hasMediumSeverity = scanResult.vulnerabilities.some(v => - v.severity === 'medium' - ); - if (hasHighSeverity) { - await cleanupAndExit(1); - } else if (hasMediumSeverity) { - await cleanupAndExit(2); + if (options.dast) { + checks.push({ name: 'DAST', status: 'not-run', reason: 'This CLI command does not execute DAST. A running application URL and DAST execution path are required.' }); } + } catch (error) { + checks.push({ name: scanResult.scanType, status: 'failed', reason: securityFailureReason('Security scan', error) }); + } - await cleanupAndExit(0); - - } catch (err) { - console.error(chalk.red('\nFailed:'), err); - await cleanupAndExit(1); + scanResult.status = summarizeChecks(checks); + try { + if (format === 'json') writeOutput(toJSON(scanResult), options.output); + else if (format === 'sarif') writeOutput(toSARIF(scanResult), options.output); + else if (format === 'markdown') writeOutput(securityToMarkdown(scanResult), options.output); + else printSecurityResult(scanResult); + } catch (error) { + console.error(chalk.red('Failed to write security report:'), error); + return cleanupAndExit(1); } + + // Incomplete execution never has a clean exit, even if it found no vulnerabilities. + if (scanResult.status !== 'complete' || scanResult.compliance?.compliant === false) return cleanupAndExit(1); + if (scanResult.vulnerabilities.some(v => v.severity === 'high' || v.severity === 'critical')) return cleanupAndExit(1); + if (scanResult.vulnerabilities.some(v => v.severity === 'medium')) return cleanupAndExit(2); + return cleanupAndExit(0); }); return securityCmd; } + +/** Provider exceptions can contain credentials or source; expose only an operation and errno. */ +function securityFailureReason(operation: string, error: unknown): string { + const code = error && typeof error === 'object' && 'code' in error ? error.code : undefined; + return typeof code === 'string' && /^[A-Z0-9_]{1,32}$/.test(code) + ? `${operation} failed (${code}).` + : `${operation} failed.`; +} + +function summarizeChecks(checks: SecurityCheckResult[]): SecurityScanResult['status'] { + if (checks.length === 0) return 'not-run'; + if (checks.every(check => check.status === 'complete')) return 'complete'; + if (checks.some(check => check.status === 'partial' || check.status === 'complete')) return 'partial'; + if (checks.some(check => check.status === 'failed')) return 'failed'; + if (checks.some(check => check.status === 'unverified')) return 'unverified'; + if (checks.some(check => check.status === 'none')) return 'none'; + return 'not-run'; +} + +function formatVulnerability(value: Vulnerability | SecurityVulnerability): SecurityVulnerability { + if ('location' in value) { + return { + severity: value.severity, type: value.title || value.category, + file: value.location.file, line: value.location.line ?? 1, + message: value.description, + }; + } + return value; +} + +function printSecurityResult(result: SecurityScanResult): void { + console.log(chalk.blue(`\n Security scan on ${result.target}\n`)); + for (const check of result.checks ?? []) { + const color = check.status === 'complete' ? chalk.green : chalk.yellow; + console.log(color(` ${check.name}: ${check.status}${check.reason ? ` — ${check.reason}` : ''}`)); + } + if (result.coverage) { + console.log(` Analyzed ${result.coverage.filesScanned} files, ${result.coverage.linesScanned} lines; ${result.coverage.rulesApplied} rules applied${result.coverage.rulesAppliedScope ? ` (${result.coverage.rulesAppliedScope})` : ''}.`); + } + if (result.vulnerabilities.length === 0) { + console.log(result.status === 'complete' + ? ' No vulnerabilities found in completed checks.' + : ' No vulnerabilities reported; requested analysis is incomplete or unverified.'); + } else { + console.log(chalk.yellow(` Found ${result.vulnerabilities.length} potential issues:`)); + for (const vulnerability of result.vulnerabilities.slice(0, 10)) { + console.log(` [${vulnerability.severity}] ${vulnerability.type}: ${vulnerability.file}:${vulnerability.line}`); + console.log(` ${vulnerability.message}`); + } + if (result.vulnerabilities.length > 10) console.log(` ... and ${result.vulnerabilities.length - 10} more`); + } + if (result.compliance) { + console.log(` Compliance: ${result.compliance.compliant ? 'compliant in completed checks' : 'failed or incomplete'}`); + for (const issue of result.compliance.issues ?? []) console.log(` [${issue.framework}] ${issue.issue}`); + } + const limitations = [ + ...(result.evidence?.limitations ?? []), + ...(result.evidence?.engines.flatMap(engine => engine.status === 'completed' ? [] : [`${engine.id}: ${engine.status}`, ...engine.errors]) ?? []), + ...(result.evidence?.files.filter(file => file.status !== 'analyzed' && file.status !== 'excluded').map(file => `${file.path}: ${file.status}${file.reason ? ` (${file.reason})` : ''}`) ?? []), + ...(result.discovery?.issues.map(issue => `${issue.path}: ${issue.reason}`) ?? []), + ]; + for (const limitation of limitations.slice(0, 10)) console.log(chalk.yellow(` ${limitation}`)); + if (limitations.length > 10) console.log(` ... ${limitations.length - 10} additional limitations; use JSON, Markdown, or SARIF for full receipts.`); + if (result.evidence) console.log(' Scope: completed checks of the declared required engines; this is not full SAST assurance.'); + console.log(`\n Security analysis: ${result.status}\n`); +} diff --git a/src/cli/utils/ci-output.ts b/src/cli/utils/ci-output.ts index 4a380d279..c2fb7cb5d 100644 --- a/src/cli/utils/ci-output.ts +++ b/src/cli/utils/ci-output.ts @@ -9,6 +9,9 @@ import { writeFileSync, readFileSync } from 'node:fs'; import { resolve, dirname, join } from 'node:path'; import { mkdirSync } from 'node:fs'; import { fileURLToPath } from 'node:url'; +import type { SecurityCoverage } from '../../domains/security-compliance/interfaces.js'; +import type { SecurityScanEvidence } from '../../domains/security-compliance/scan-evidence.js'; +import type { discoverSecurityFiles } from '../../domains/security-compliance/scan-discovery.js'; /** Read version from package.json at build time — no hardcoded strings. */ function getPackageVersion(): string { @@ -55,6 +58,25 @@ export interface SecurityScanResult { compliance?: { compliant: boolean; issues?: Array<{ framework: string; issue: string }> }; target: string; scanType: string; + /** Execution completeness is independent of finding severity. */ + status?: SecurityExecutionStatus; + checks?: SecurityCheckResult[]; + coverage?: SecurityCoverage; + evidence?: SecurityScanEvidence; + discovery?: Awaited>['discovery']; +} + +export type SecurityExecutionStatus = 'complete' | 'partial' | 'none' | 'failed' | 'not-run' | 'unverified'; + +export interface SecurityCheckResult { + name: string; + status: SecurityExecutionStatus; + reason?: string; +} + +/** Legacy finding-only reports carry no evidence that the requested work ran. */ +export function securityExecutionStatus(result: SecurityScanResult): SecurityExecutionStatus { + return result.status ?? result.evidence?.completeness ?? 'unverified'; } export interface TestResult { @@ -159,6 +181,7 @@ export function toJSON(data: unknown): string { const SARIF_SCHEMA = 'https://raw.githubusercontent.com/oasis-tcs/sarif-spec/main/sarif-2.1/schema/sarif-schema-2.1.0.json'; export function toSARIF(result: SecurityScanResult): string { + const status = securityExecutionStatus(result); const severityToLevel = (severity: string): string => { switch (severity.toLowerCase()) { case 'critical': @@ -218,9 +241,20 @@ export function toSARIF(result: SecurityScanResult): string { }, }, results, + properties: { + securityScan: { + status, target: result.target, scanType: result.scanType, coverage: result.evidence ? result.coverage : undefined, + evidence: result.evidence, discovery: result.discovery, checks: result.checks, + }, + }, invocations: [{ - executionSuccessful: true, - commandLine: `aqe security --sast --format sarif -t ${result.target}`, + executionSuccessful: status === 'complete', + ...(status === 'complete' ? {} : { + toolExecutionNotifications: [{ + level: 'warning', + message: { text: `Security analysis ${status}; findings do not establish complete coverage.` }, + }], + }), }], }], }; @@ -347,8 +381,25 @@ export function securityToMarkdown(result: SecurityScanResult): string { let md = `# Security Scan Report\n\n`; md += `**Target:** ${result.target}\n`; md += `**Scan Type:** ${result.scanType}\n`; + md += `**Execution:** ${securityExecutionStatus(result)}\n`; md += `**Vulnerabilities Found:** ${result.vulnerabilities.length}\n\n`; + if (result.checks?.length) { + md += `## Check execution\n\n`; + for (const check of result.checks) { + md += `- **${check.name}:** ${check.status}${check.reason ? ` — ${check.reason}` : ''}\n`; + } + md += '\n'; + } + if (result.coverage && result.evidence) { + md += `**Analyzed files:** ${result.coverage.filesScanned}\n`; + md += `**Analyzed lines:** ${result.coverage.linesScanned}\n`; + md += `**Rules applied:** ${result.coverage.rulesApplied}${result.coverage.rulesAppliedScope ? ` (${result.coverage.rulesAppliedScope})` : ''}\n\n`; + } + if (result.evidence || result.discovery) { + md += `## Execution receipts\n\n\`\`\`json\n${toJSON({ evidence: result.evidence, discovery: result.discovery })}\n\`\`\`\n\n`; + } + if (result.vulnerabilities.length > 0) { md += `## Vulnerabilities\n\n`; md += `| Severity | Type | File | Line | Message |\n`; diff --git a/src/coordination/handlers/security-handlers.ts b/src/coordination/handlers/security-handlers.ts index acfdb4979..bf02e1e5a 100644 --- a/src/coordination/handlers/security-handlers.ts +++ b/src/coordination/handlers/security-handlers.ts @@ -8,10 +8,14 @@ import * as fs from 'fs/promises'; import * as path from 'path'; import { ok, err } from '../../shared/types'; +import { createHash } from 'node:crypto'; import { toError } from '../../shared/error-utils.js'; import { FilePath } from '../../shared/value-objects/index.js'; import type { TaskHandlerContext } from './handler-types'; -import { discoverSourceFiles, generateSecurityRecommendations } from './handler-utils'; +import { generateSecurityRecommendations } from './handler-utils'; +import { discoverSecurityFiles, isSecuritySourceFile } from '../../domains/security-compliance/scan-discovery.js'; +import type { SecurityScanEvidence, SecurityEngineEvidence, SecurityFileEvidence } from '../../domains/security-compliance/scan-evidence.js'; +import type { SASTResult, DASTResult } from '../../domains/security-compliance/interfaces.js'; export function registerSecurityHandlers(ctx: TaskHandlerContext): void { // Register security scan handler - REAL IMPLEMENTATION @@ -25,33 +29,21 @@ export function registerSecurityHandlers(ctx: TaskHandlerContext): void { }; try { - const scanner = ctx.getSecurityScanner(); - const targetPath = payload.target || process.cwd(); - - // Discover files to scan - const filesToScan = await discoverSourceFiles(targetPath); - - if (filesToScan.length === 0) { - return ok({ - vulnerabilities: 0, - critical: 0, - high: 0, - medium: 0, - low: 0, - informational: 0, - topVulnerabilities: [], - recommendations: ['No source files found to scan'], - scanTypes: { - sast: payload.sast !== false, - dast: payload.dast || false, - }, - warning: `No source files found in ${targetPath}`, - }); - } + const targetPath = path.resolve(payload.target || process.cwd()); + const sastRequested = payload.sast !== false; + const discoveryResult = sastRequested ? await discoverSecurityFiles(targetPath) : undefined; + const filesToScan = discoveryResult?.files ?? []; + const fileEvidence: SecurityFileEvidence[] = []; + const engines: SecurityEngineEvidence[] = []; + const genericRules = new Map(); + const genericErrors: string[] = []; + const manifestErrors: string[] = []; + const manifestRules = new Map(); + const limitations: string[] = []; // Separate files by language capability - const jstsFiles = filesToScan.filter(f => /\.(ts|tsx|js|jsx|mjs|cjs)$/.test(f)); - const otherFiles = filesToScan.filter(f => !/\.(ts|tsx|js|jsx|mjs|cjs)$/.test(f)); + const jstsFiles = filesToScan.filter(f => /\.(ts|tsx|js|jsx|mjs|cjs)$/i.test(f)); + const otherFiles = filesToScan.filter(f => !/\.(ts|tsx|js|jsx|mjs|cjs)$/i.test(f)); // Run basic cross-language security patterns on non-JS/TS files const crossLangVulns: Array<{ @@ -61,12 +53,19 @@ export function registerSecurityHandlers(ctx: TaskHandlerContext): void { // Run secret/CORS patterns on ALL files (not just otherFiles) to catch JS/TS secrets too for (const filePath of filesToScan) { + if (!isSecuritySourceFile(filePath)) { + fileEvidence.push({ path: filePath, engineId: 'generic-patterns', status: 'unsupported', + readLines: 0, analyzedLines: 0, reason: 'Outside the declared source-language policy.' }); + continue; + } try { - const content = await fs.readFile(filePath, 'utf-8'); + const source = await fs.readFile(filePath); + const content = source.toString('utf-8'); const lines = content.split('\n'); - const relPath = filePath.startsWith(targetPath) - ? filePath.slice(targetPath.length).replace(/^\//, '') - : filePath; + fileEvidence.push({ path: filePath, engineId: 'generic-patterns', status: 'analyzed', + sourceDigest: createHash('sha256').update(source).digest('hex'), + readLines: lines.length, analyzedLines: lines.length }); + const relPath = path.relative(targetPath, filePath) || path.basename(filePath); // Pattern: Hardcoded secrets/keys // Fix #287: Use \w* around keywords to match SECRET_KEY, JWT_SECRET, API_TOKEN, etc. @@ -76,7 +75,8 @@ export function registerSecurityHandlers(ctx: TaskHandlerContext): void { { regex: /(?:AWS_SECRET|GITHUB_TOKEN|SLACK_TOKEN|OPENAI_API_KEY)\s*[=:]\s*['"][^'"]+['"]/gi, title: 'Hardcoded cloud credential', severity: 'critical' as const }, ]; - for (const pattern of secretPatterns) { + for (const [index, pattern] of secretPatterns.entries()) { + genericRules.set(`secret-${index}`, pattern.regex.toString()); for (let i = 0; i < lines.length; i++) { // Use matchAll to find ALL secrets on a single line (not just first) const matches = [...lines[i].matchAll(pattern.regex)]; @@ -94,6 +94,7 @@ export function registerSecurityHandlers(ctx: TaskHandlerContext): void { // Pattern: SQL injection risks const sqlPatterns = /(?:execute|query|cursor\.execute)\s*\(\s*(?:f['"]|['"].*%s|['"].*\+\s*\w)/gi; + genericRules.set('sql-interpolation', sqlPatterns.toString()); for (let i = 0; i < lines.length; i++) { if (sqlPatterns.test(lines[i])) { crossLangVulns.push({ @@ -115,7 +116,8 @@ export function registerSecurityHandlers(ctx: TaskHandlerContext): void { /@CrossOrigin\(\s*origins?\s*=\s*["']\*["']/i, // Spring Boot /\.Header\(\)\.Set\(["']Access-Control-Allow-Origin["'],\s*["']\*["']/i, // Go ]; - for (const corsPattern of corsPatterns) { + for (const [index, corsPattern] of corsPatterns.entries()) { + genericRules.set(`cors-${index}`, corsPattern.toString()); if (corsPattern.test(content)) { crossLangVulns.push({ title: 'CORS wildcard origin', @@ -129,6 +131,7 @@ export function registerSecurityHandlers(ctx: TaskHandlerContext): void { } // Pattern: Debug/development mode enabled + genericRules.set('debug-mode', /(?:DEBUG|debug)\s*[=:]\s*(?:True|true|1)/i.toString()); if (/(?:DEBUG|debug)\s*[=:]\s*(?:True|true|1)/i.test(content)) { crossLangVulns.push({ title: 'Debug mode enabled', @@ -140,6 +143,7 @@ export function registerSecurityHandlers(ctx: TaskHandlerContext): void { } // Pattern: Eval/exec usage + genericRules.set('eval-exec', /\b(?:eval|exec)\s*\(/i.toString()); if (/\b(?:eval|exec)\s*\(/i.test(content)) { crossLangVulns.push({ title: 'Dangerous eval/exec usage', @@ -149,17 +153,25 @@ export function registerSecurityHandlers(ctx: TaskHandlerContext): void { category: 'injection', }); } - } catch { - // Skip unreadable files + } catch (error) { + const reason = `Source read failed: ${safeErrorCode(error)}`; + genericErrors.push(`${filePath}: ${reason}`); + fileEvidence.push({ path: filePath, engineId: 'generic-patterns', status: 'unreadable', + readLines: 0, analyzedLines: 0, reason }); } } // Also check dependency manifests for known vulnerable packages const depManifests = ['requirements.txt', 'pyproject.toml', 'Gemfile', 'go.mod', 'Cargo.toml']; - for (const manifest of depManifests) { + for (const manifest of sastRequested ? depManifests : []) { const manifestPath = path.join(targetPath, manifest); try { - const manifestContent = await fs.readFile(manifestPath, 'utf-8'); + const manifestSource = await fs.readFile(manifestPath); + const manifestContent = manifestSource.toString('utf-8'); + const manifestLines = manifestContent.split('\n').length; + fileEvidence.push({ path: manifestPath, engineId: 'dependency-manifest-patterns', status: 'analyzed', + sourceDigest: createHash('sha256').update(manifestSource).digest('hex'), readLines: manifestLines, analyzedLines: manifestLines }); + manifestRules.set('dependency-audit-advisory', 'Presence of a supported dependency manifest'); crossLangVulns.push({ title: 'Dependency audit recommended', severity: 'informational', @@ -177,6 +189,7 @@ export function registerSecurityHandlers(ctx: TaskHandlerContext): void { ]; for (const known of knownCVEs) { + manifestRules.set(known.cve, known.pattern.toString()); if (known.pattern.test(manifestContent)) { crossLangVulns.push({ title: known.title, @@ -188,36 +201,110 @@ export function registerSecurityHandlers(ctx: TaskHandlerContext): void { } } } - } catch { - // Manifest doesn't exist + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + if (code !== 'ENOENT' && code !== 'ENOTDIR') { + const reason = `Manifest read failed: ${safeErrorCode(error)}`; + manifestErrors.push(`${manifestPath}: ${reason}`); + fileEvidence.push({ path: manifestPath, engineId: 'dependency-manifest-patterns', status: 'unreadable', + readLines: 0, analyzedLines: 0, reason }); + } } } + const genericAnalyzed = fileEvidence.filter(file => file.engineId === 'generic-patterns' && file.status === 'analyzed').length; + const manifestsAnalyzed = fileEvidence.filter(file => file.engineId === 'dependency-manifest-patterns' && file.status === 'analyzed').length; + engines.push({ id: 'dependency-manifest-patterns', requested: sastRequested, required: false, + status: !sastRequested ? 'disabled' : manifestErrors.length > 0 ? (manifestsAnalyzed > 0 ? 'partial' : 'failed') : manifestsAnalyzed > 0 ? 'completed' : 'not-run', + scope: 'parent-directory', target: targetPath, analyzedFiles: manifestsAnalyzed, + ruleIds: [...manifestRules.keys()].sort(), rulesetDigest: createHash('sha256').update(JSON.stringify([...manifestRules.entries()].sort())).digest('hex'), + ruleCoverage: 'known', errors: manifestErrors, limitations: ['Manifest name patterns are advisories, not a resolved dependency or version audit.'] }); + const genericLimitations = ['Generic text patterns do not establish language-specific or dependency vulnerability coverage.']; + engines.push({ id: 'generic-patterns', requested: sastRequested, required: sastRequested, + status: !sastRequested ? 'disabled' : genericErrors.length > 0 ? (genericAnalyzed > 0 ? 'partial' : 'failed') + : genericAnalyzed === filesToScan.length && genericAnalyzed > 0 ? 'completed' + : genericAnalyzed > 0 ? 'partial' : filesToScan.length > 0 ? 'unavailable' : 'not-run', + scope: 'requested-files', analyzedFiles: genericAnalyzed, + ruleIds: [...genericRules.keys()].sort(), + rulesetDigest: createHash('sha256').update(JSON.stringify([...genericRules.entries()].sort())).digest('hex'), + ruleCoverage: 'known', errors: genericErrors, limitations: genericLimitations }); + // Convert JS/TS file paths to FilePath value objects for the SAST scanner const filePathObjects = jstsFiles.map(filePath => FilePath.create(filePath)); - // Run SAST scan on JS/TS files if requested and files exist - let sastResult = null; - if (payload.sast !== false && filePathObjects.length > 0) { - const result = await scanner.scanFiles(filePathObjects); - if (result.success) { - sastResult = result.value; + // Only returned execution receipts can establish SAST coverage. + let sastResult: SASTResult | null = null; + if (sastRequested && filePathObjects.length > 0) { + try { + const result = await ctx.getSecurityScanner().scanFiles(filePathObjects); + if (result.success) { + sastResult = result.value; + if (sastResult.evidence) { + fileEvidence.push(...sastResult.evidence.files); + engines.push(...sastResult.evidence.engines); + limitations.push(...sastResult.evidence.limitations); + } else { + engines.push(unverifiedEngine('sast', 'requested-files')); + } + } else { + engines.push(failedEngine('sast', 'requested-files', result.error)); + } + } catch (error) { + engines.push(failedEngine('sast', 'requested-files', error)); } + } else { + engines.push({ id: 'sast', requested: sastRequested, required: false, + status: sastRequested ? 'not-run' : 'disabled', scope: 'requested-files', analyzedFiles: 0, + ruleCoverage: 'unknown', errors: [], limitations: sastRequested ? ['No JS/TS files were available for language-specific SAST.'] : [] }); } - // Run DAST scan if URL provided and dast is enabled - let dastResult = null; + let dastResult: DASTResult | null = null; if (payload.dast && payload.targetUrl) { - const result = await scanner.scanUrl(payload.targetUrl, { - activeScanning: true, - maxDepth: 3, - timeout: 30000, - }); - if (result.success) { - dastResult = result.value; + try { + const result = await ctx.getSecurityScanner().scanUrl(payload.targetUrl, { + activeScanning: true, maxDepth: 3, timeout: 30000, + }); + if (result.success) { + dastResult = result.value; + // Legacy DAST results contain findings, but no execution coverage receipt. + engines.push({ ...unverifiedEngine('dast', 'url'), target: payload.targetUrl }); + } else { + engines.push({ ...failedEngine('dast', 'url', result.error), target: payload.targetUrl }); + } + } catch (error) { + engines.push({ ...failedEngine('dast', 'url', error), target: payload.targetUrl }); } + } else { + engines.push({ id: 'dast', requested: payload.dast === true, required: payload.dast === true, + status: payload.dast ? 'not-run' : 'disabled', scope: 'url', ruleCoverage: 'unknown', + errors: payload.dast ? ['DAST was requested without targetUrl.'] : [], limitations: [] }); + } + + if (payload.compliance?.length) { + engines.push({ id: 'compliance', requested: true, required: true, status: 'not-run', + scope: 'requested-files', ruleCoverage: 'unknown', errors: [], + limitations: ['Requested compliance checks are not executed by this task handler.'] }); } + const requestedPaths = new Set(filesToScan); + const analyzedPaths = new Set(fileEvidence.filter(file => + file.status === 'analyzed' && requestedPaths.has(file.path)).map(file => file.path)); + const requiredEngines = engines.filter(engine => engine.requested && engine.required); + const hasExecuted = analyzedPaths.size > 0; + const complete = hasExecuted && requiredEngines.every(engine => engine.status === 'completed') && + (!discoveryResult || discoveryResult.discovery.status === 'complete'); + const evidence: SecurityScanEvidence = { + schemaVersion: 1, completeness: complete ? 'complete' : hasExecuted ? 'partial' : 'none', + requestedFiles: filesToScan.length, requestedPaths: filesToScan, duplicateInputs: 0, + files: fileEvidence, engines, + limitations: [...new Set([...limitations, ...engines.flatMap(engine => [...engine.limitations, ...engine.errors]), + ...(discoveryResult?.discovery.issues.map(issue => `${issue.path}: ${issue.reason}`) ?? [])])], + ...(discoveryResult ? { discovery: discoveryResult.discovery } : {}), + }; + const deepAnalysisPerformed = Boolean(sastResult?.evidence?.engines.some(engine => + engine.id === 'patterns' && (engine.status === 'completed' || engine.status === 'partial') && + (engine.analyzedFiles ?? 0) > 0)); + // Combine results from all scan sources - SAST, DAST, and cross-language patterns const crossLangSeverityCounts = { critical: crossLangVulns.filter(v => v.severity === 'critical').length, @@ -257,7 +344,9 @@ export function registerSecurityHandlers(ctx: TaskHandlerContext): void { })); // Generate recommendations based on findings - const recommendations = generateSecurityRecommendations(allVulns); + const recommendations = allVulns.length === 0 && evidence.completeness !== 'complete' + ? ['No findings reported; requested analysis is incomplete or unverified. Inspect execution receipts.'] + : generateSecurityRecommendations(allVulns); return ok({ vulnerabilities: allVulns.length, @@ -267,36 +356,44 @@ export function registerSecurityHandlers(ctx: TaskHandlerContext): void { low: summary.low, informational: summary.informational, topVulnerabilities, + findings: allVulns, recommendations, scanTypes: { sast: payload.sast !== false, dast: payload.dast || false, }, - filesScanned: filesToScan.length, - jstsFilesScanned: jstsFiles.length, - otherFilesScanned: otherFiles.length, - coverage: sastResult?.coverage, - // #569 (companion note): "zero findings" from a scan that ran no - // language-specific analysis is not evidence of absence, and combined - // with a coverage number it reads as "this code is clean and covered". - // Say plainly which depth of analysis actually ran. - deepAnalysisPerformed: jstsFiles.length > 0, - analysisDepth: jstsFiles.length > 0 - ? (otherFiles.length > 0 ? 'full-sast-on-js-ts; pattern-matching-on-other-languages' : 'full-sast') - : 'pattern-matching-only', - ...(jstsFiles.length === 0 ? { - note: otherFiles.length > 0 - ? 'NO LANGUAGE-SPECIFIC ANALYSIS RAN. Only cross-language pattern matching ' + - '(secrets, CORS, eval/exec) was applied to these files — full SAST is ' + - 'implemented for JS/TS only. A zero-finding result here means "nothing ' + - 'matched a generic pattern", NOT "no vulnerabilities". Run a ' + - 'language-specific tool (cargo audit / clippy, bandit, gosec, semgrep) ' + - 'for real coverage of this codebase.' - : 'No source files were analyzed.', - } : {}), + status: evidence.completeness === 'complete' ? 'completed' : evidence.completeness === 'partial' ? 'partial' : 'unavailable', + evidence, + limitations: evidence.limitations, + filesScanned: analyzedPaths.size, + jstsFilesScanned: jstsFiles.filter(file => analyzedPaths.has(file)).length, + otherFilesScanned: otherFiles.filter(file => analyzedPaths.has(file)).length, + ...(sastResult?.evidence ? { coverage: sastResult.coverage } : {}), + deepAnalysisPerformed, + analysisDepth: deepAnalysisPerformed + ? (otherFiles.length > 0 ? 'sast-patterns-on-js-ts; generic-patterns-on-other-languages' : 'sast-patterns') + : genericAnalyzed > 0 ? 'pattern-matching-only' : 'none', + ...(!deepAnalysisPerformed ? { note: 'No verified language-specific SAST ran. Generic pattern matches and partial findings do not establish that the target is free of vulnerabilities.' } : {}), }); } catch (error) { return err(toError(error)); } }); } + +function failedEngine(id: string, scope: SecurityEngineEvidence['scope'], error: unknown): SecurityEngineEvidence { + const reason = id === 'sast' && error instanceof Error && error.message.startsWith('No valid rule sets found:') + ? 'No valid rule sets configured.' : `${id} execution failed (${safeErrorCode(error)}).`; + return { id, requested: true, required: true, status: 'failed', scope, + ruleCoverage: 'unknown', errors: [reason], limitations: [] }; +} + +function safeErrorCode(error: unknown): string { + const code = (error as NodeJS.ErrnoException | undefined)?.code; + return typeof code === 'string' && /^[A-Z0-9_]{1,32}$/.test(code) ? code : 'UNKNOWN'; +} + +function unverifiedEngine(id: string, scope: SecurityEngineEvidence['scope']): SecurityEngineEvidence { + return { id, requested: true, required: true, status: 'unverified', scope, + ruleCoverage: 'unknown', errors: [], limitations: [`${id} returned no execution receipt; coverage is unverified.`] }; +} diff --git a/src/coordination/protocols/security-audit.ts b/src/coordination/protocols/security-audit.ts index 3aa1db9cb..5d6591332 100644 --- a/src/coordination/protocols/security-audit.ts +++ b/src/coordination/protocols/security-audit.ts @@ -133,6 +133,8 @@ export interface SecurityAuditResult { readonly overallRiskScore: RiskScore; readonly recommendations: string[]; readonly deploymentDecision: DeploymentDecision; + /** Requested checks without complete execution evidence; findings remain usable. */ + readonly incompleteChecks?: readonly string[]; } /** @@ -306,18 +308,26 @@ export class SecurityAuditProtocol { triagedFindings: this.createEmptyTriagedFindings(), overallRiskScore: RiskScore.create(0), recommendations: [], - deploymentDecision: { allowed: true, reason: '', blockingIssues: [], warnings: [] }, + incompleteChecks: [], + deploymentDecision: { allowed: false, reason: 'Audit not completed', blockingIssues: [], warnings: [] }, }; // Adjust scope based on trigger const auditOptions = this.getAuditOptionsForTrigger(trigger); // Phase 1: Vulnerability Scan (SAST) - this.updatePhase('vulnerability-scan'); - const sastResult = await this.scanVulnerabilities(auditOptions); - if (sastResult.success) { - this.currentAudit = { ...this.currentAudit, sastResult: sastResult.value }; - await this.publishVulnerabilities(sastResult.value.vulnerabilities); + if (auditOptions.includeSAST) { + this.updatePhase('vulnerability-scan'); + const sastResult = await this.scanVulnerabilities(auditOptions); + if (sastResult.success) { + this.currentAudit = { ...this.currentAudit, sastResult: sastResult.value }; + await this.publishVulnerabilities(sastResult.value.vulnerabilities); + if (sastResult.value.evidence?.completeness !== 'complete') { + this.recordIncompleteCheck('SAST coverage is incomplete or unverified'); + } + } else { + this.recordIncompleteCheck('SAST scan did not complete'); + } } // Phase 2: Dependency Scan @@ -326,36 +336,50 @@ export class SecurityAuditProtocol { if (depResult.success) { this.currentAudit = { ...this.currentAudit, dependencyResult: depResult.value }; await this.publishDependencyVulnerabilities(depResult.value.vulnerabilities); + } else { + this.recordIncompleteCheck('Dependency scan did not complete'); } // Phase 3: Secret Scan (if enabled) - if (this.config.enableSecretScan) { + if (auditOptions.includeSecrets) { this.updatePhase('secret-scan'); const secretResult = await this.auditSecrets(); if (secretResult.success) { this.currentAudit = { ...this.currentAudit, secretResult: secretResult.value }; await this.publishSecretExposures(secretResult.value.secretsFound); + } else { + this.recordIncompleteCheck('Secret scan is unavailable'); } } // Phase 4: DAST Scan (if enabled and URL provided) - if (this.config.enableDAST && this.config.targetUrl) { + if (auditOptions.includeDAST && this.config.targetUrl) { const dastResult = await this.runDASTScan(this.config.targetUrl); if (dastResult.success) { this.currentAudit = { ...this.currentAudit, dastResult: dastResult.value }; + this.recordIncompleteCheck('DAST coverage is unverified: the scanner returned no execution receipt'); await this.publishVulnerabilities(dastResult.value.vulnerabilities); + } else { + this.recordIncompleteCheck('DAST scan did not complete'); } + } else if (auditOptions.includeDAST) { + this.recordIncompleteCheck('DAST requested without a target URL'); } // Phase 5: Compliance Validation - this.updatePhase('compliance-validation'); - const complianceResult = await this.validateCompliance(); - if (complianceResult.success) { - this.currentAudit = { - ...this.currentAudit, - complianceReports: complianceResult.value, - }; - await this.publishComplianceResults(complianceResult.value); + if (this.config.complianceStandards.length > 0) { + this.updatePhase('compliance-validation'); + const complianceResult = await this.validateCompliance(); + if (complianceResult.success) { + this.currentAudit = { + ...this.currentAudit, + complianceReports: complianceResult.value, + }; + await this.publishComplianceResults(complianceResult.value); + this.recordIncompleteCheck('Protocol compliance checks have no verified execution receipt'); + } else { + this.recordIncompleteCheck('Compliance validation did not complete'); + } } // Phase 6: Triage Findings @@ -527,28 +551,9 @@ export class SecurityAuditProtocol { * Audit for exposed secrets/credentials */ async auditSecrets(): Promise> { - try { - const agentId = await this.spawnAgent('secret-scanner', ['secret-scan', 'credential-audit']); - if (!agentId.success) { - return err(agentId.error); - } - - const secretsFound: DetectedSecret[] = []; - - // In production, this would scan actual files with patterns like: - // - API keys: /(?:api[_-]?key|apikey)/gi - // - Passwords: /(?:password|passwd|pwd)/gi - // - Tokens: /(?:secret|token|bearer)/gi - // - Private keys: /-----BEGIN\s+(?:RSA\s+)?PRIVATE\s+KEY-----/gi - // For now, report no secrets found (clean scan) - - return ok({ - secretsFound, - filesScanned: this.config.scanPaths.length * 10, // Estimate - }); - } catch (error) { - return err(toError(error)); - } + // Agent allocation is not scan execution. Until this protocol has a real + // secret-scanner adapter, retain an explicit unavailable result. + return err(new Error('Secret scanning is unavailable in SecurityAuditProtocol: no scanner is implemented.')); } /** @@ -632,7 +637,7 @@ export class SecurityAuditProtocol { return { riskScore: RiskScore.create(0), recommendations: [], - deploymentDecision: { allowed: true, reason: 'No audit data', blockingIssues: [], warnings: [] }, + deploymentDecision: { allowed: false, reason: 'No audit data', blockingIssues: ['No audit evidence'], warnings: [] }, }; } @@ -972,6 +977,15 @@ export class SecurityAuditProtocol { } } + private recordIncompleteCheck(reason: string): void { + if (this.currentAudit) { + this.currentAudit = { + ...this.currentAudit, + incompleteChecks: [...(this.currentAudit.incompleteChecks ?? []), reason], + }; + } + } + private createEmptyTriagedFindings(): TriagedFindings { return { critical: [], @@ -1047,6 +1061,10 @@ export class SecurityAuditProtocol { if (!this.currentAudit) return recommendations; + for (const check of this.currentAudit.incompleteChecks ?? []) { + recommendations.push(`Complete the required security check before approving deployment: ${check}`); + } + const { triagedFindings, complianceReports } = this.currentAudit; // Critical findings @@ -1095,11 +1113,11 @@ export class SecurityAuditProtocol { } private determineDeploymentDecision(_riskScore: RiskScore): DeploymentDecision { - const blockingIssues: string[] = []; + const blockingIssues: string[] = [...(this.currentAudit?.incompleteChecks ?? [])]; const warnings: string[] = []; if (!this.currentAudit) { - return { allowed: true, reason: 'No audit data', blockingIssues, warnings }; + return { allowed: false, reason: 'No audit data', blockingIssues: ['No audit evidence'], warnings }; } const { triagedFindings, complianceReports } = this.currentAudit; diff --git a/src/coordination/result-saver.ts b/src/coordination/result-saver.ts index b3aa7e7cb..637aff09f 100644 --- a/src/coordination/result-saver.ts +++ b/src/coordination/result-saver.ts @@ -8,11 +8,44 @@ import * as path from 'path'; import { createHash } from 'crypto'; import { TaskType } from './queen-coordinator'; import { safeJsonParse } from '../shared/safe-json.js'; +import type { SecurityScanEvidence } from '../domains/security-compliance/scan-evidence.js'; +import type { SecurityCoverage } from '../domains/security-compliance/interfaces.js'; // ============================================================================ // Types // ============================================================================ +interface SecurityReportFinding { + type?: string; + title?: string; + severity: string; + file?: string; + line?: number; + location?: { file: string; line?: number }; + description?: string; +} + +interface SecurityReportData { + vulnerabilities: number; + critical: number; + high: number; + medium: number; + low: number; + topVulnerabilities: Array<{ type: string; severity: string; file: string; line: number }>; + findings?: readonly SecurityReportFinding[]; + recommendations: string[]; + evidence?: SecurityScanEvidence; + coverage?: SecurityCoverage; + limitations?: readonly string[]; + analysisDepth?: string; +} + +function securityReportExecution(data: SecurityReportData): string { + return data.evidence?.completeness === 'complete' ? 'completed' + : data.evidence?.completeness === 'partial' ? 'partial' + : data.evidence ? 'unavailable' : 'unverified'; +} + export interface SaveOptions { /** Target language for test generation */ language?: string; @@ -323,15 +356,7 @@ export class ResultSaver { options: SaveOptions ): Promise { const files: SavedFile[] = []; - const data = result as { - vulnerabilities: number; - critical: number; - high: number; - medium: number; - low: number; - topVulnerabilities: Array<{ type: string; severity: string; file: string; line: number }>; - recommendations: string[]; - }; + const data = result as SecurityReportData; const securityDir = path.join(this.resultsDir, 'security'); @@ -513,10 +538,9 @@ end_of_record `; } - private generateSarif(data: { - vulnerabilities: number; - topVulnerabilities: Array<{ type: string; severity: string; file: string; line: number }>; - }): string { + private generateSarif(data: SecurityReportData): string { + const status = securityReportExecution(data); + const findings: readonly SecurityReportFinding[] = data.findings ?? data.topVulnerabilities; return JSON.stringify({ $schema: 'https://raw.githubusercontent.com/oasis-tcs/sarif-spec/master/Schemata/sarif-schema-2.1.0.json', version: '2.1.0', @@ -528,14 +552,23 @@ end_of_record informationUri: 'https://github.com/ruvnet/agentic-qe', }, }, - results: data.topVulnerabilities.map((v, i) => ({ + properties: { securityScan: { status, evidence: data.evidence, coverage: data.coverage, + limitations: data.limitations, analysisDepth: data.analysisDepth } }, + invocations: [{ + executionSuccessful: data.evidence?.completeness === 'complete', + ...(data.evidence?.completeness === 'complete' ? {} : { + toolExecutionNotifications: [{ level: 'warning', + message: { text: `Security analysis ${status}; findings do not establish complete coverage.` } }], + }), + }], + results: findings.map((v, i) => ({ ruleId: `VULN-${String(i + 1).padStart(3, '0')}`, level: v.severity === 'critical' ? 'error' : v.severity === 'high' ? 'error' : 'warning', - message: { text: v.type }, + message: { text: v.type ?? v.title ?? 'Security finding' }, locations: [{ physicalLocation: { - artifactLocation: { uri: v.file }, - region: { startLine: v.line }, + artifactLocation: { uri: v.file ?? v.location?.file ?? '' }, + ...((v.line ?? v.location?.line ?? 0) > 0 ? { region: { startLine: v.line ?? v.location?.line } } : {}), }, }], })), @@ -628,19 +661,19 @@ ${data.gaps.length === 0 ? 'No significant gaps detected.' : data.gaps.map(g => `; } - private generateSecurityReport(data: { - vulnerabilities: number; - critical: number; - high: number; - medium: number; - low: number; - topVulnerabilities: Array<{ type: string; severity: string; file: string; line: number }>; - recommendations: string[]; - }): string { + private generateSecurityReport(data: SecurityReportData): string { return `# Security Scan Report **Generated:** ${new Date().toISOString()} **Scanner:** Agentic QE v3 Security +**Execution:** ${securityReportExecution(data)} + +## Execution receipts + +\`\`\`json +${JSON.stringify({ evidence: data.evidence, coverage: data.coverage, limitations: data.limitations, + analysisDepth: data.analysisDepth }, null, 2)} +\`\`\` ## Summary @@ -660,6 +693,12 @@ ${data.topVulnerabilities.map(v => `### ${v.type} - **Line:** ${v.line} `).join('\n')} +## All findings + +\`\`\`json +${JSON.stringify(data.findings ?? data.topVulnerabilities, null, 2)} +\`\`\` + ## Recommendations ${data.recommendations.map((r, i) => `${i + 1}. ${r}`).join('\n')} @@ -744,6 +783,7 @@ ${data.recommendations.length === 0 ? 'No recommendations - all quality gates pa switch (taskType) { case 'scan-security': return { + status: securityReportExecution(data as unknown as SecurityReportData), vulnerabilities: data.vulnerabilities, critical: data.critical, high: data.high, diff --git a/src/domains/security-compliance/interfaces.ts b/src/domains/security-compliance/interfaces.ts index 0c4699eb1..4d8f809ab 100644 --- a/src/domains/security-compliance/interfaces.ts +++ b/src/domains/security-compliance/interfaces.ts @@ -1,3 +1,4 @@ +import type { SecurityScanEvidence } from './scan-evidence.js'; /** * Agentic QE v3 - Security & Compliance Domain Interfaces * @@ -179,6 +180,8 @@ export interface SASTResult { readonly vulnerabilities: Vulnerability[]; readonly summary: ScanSummary; readonly coverage: SecurityCoverage; + /** Absent on legacy adapters; absence does not prove completed analysis. */ + readonly evidence?: SecurityScanEvidence; } export interface RuleSet { @@ -196,6 +199,8 @@ export interface FalsePositiveCheck { } export interface SecurityCoverage { + /** Counter scope; external engine rule coverage is described in evidence. */ + readonly rulesAppliedScope?: 'built-in-patterns'; readonly filesScanned: number; readonly linesScanned: number; readonly rulesApplied: number; diff --git a/src/domains/security-compliance/scan-discovery.ts b/src/domains/security-compliance/scan-discovery.ts new file mode 100644 index 000000000..dec749de4 --- /dev/null +++ b/src/domains/security-compliance/scan-discovery.ts @@ -0,0 +1,110 @@ +import * as fs from 'node:fs/promises'; +import * as path from 'node:path'; +import type { SecurityDiscoveryEvidence } from './scan-evidence.js'; + +// The established security task's source-language scope. A direct file target +// is retained even when unsupported, so its scanner disposition is observable. +const SOURCE_EXTENSIONS = new Set([ + '.ts', '.tsx', '.js', '.jsx', '.mjs', '.cjs', '.py', '.pyw', '.go', '.rs', + '.java', '.kt', '.kts', '.rb', '.cs', '.php', '.swift', '.c', '.h', '.cpp', + '.hpp', '.cc', '.scala', +]); +const EXCLUDED_DIRECTORIES = new Set([ + 'node_modules', '.git', 'dist', 'build', 'coverage', '.nyc_output', + '__pycache__', '.venv', 'venv', '.tox', '.mypy_cache', 'target', '.gradle', + 'vendor', '.bundle', '.agentic-qe', '.claude', '.cache', '.npm', '.yarn', + '.next', '.nuxt', '.svelte-kit', 'out', '.turbo', 'tmp', 'temp', '.tmp', +]); + +export function isSecuritySourceFile(filePath: string): boolean { + return SOURCE_EXTENSIONS.has(path.extname(filePath).toLowerCase()); +} + +export interface SecurityDiscoveryOptions { + readonly maxFiles?: number; + readonly maxDepth?: number; +} + +/** Discover source inputs without turning failures or truncation into empty success. */ +export async function discoverSecurityFiles( + target: string, + options: SecurityDiscoveryOptions = {}, +): Promise<{ files: string[]; discovery: SecurityDiscoveryEvidence }> { + const maxFiles = options.maxFiles ?? 5000; + const maxDepth = options.maxDepth ?? 64; + if (!Number.isSafeInteger(maxFiles) || maxFiles < 1 + || !Number.isSafeInteger(maxDepth) || maxDepth < 0) { + throw new Error('Security discovery requires a positive file limit and non-negative depth limit.'); + } + const files: string[] = []; + const issues: { path: string; reason: string }[] = []; + const excludedPaths: string[] = []; + const root = path.resolve(target); + let rootFailed = false; + let truncated = false; + + function recordError(location: string, error: unknown): void { + const code = (error as NodeJS.ErrnoException | undefined)?.code; + // Raw errors can include source text, arguments, URLs, or credentials. + issues.push({ path: location, reason: typeof code === 'string' && /^[A-Z0-9_]{1,32}$/.test(code) + ? code : 'DISCOVERY_FAILED' }); + } + + async function walk(directory: string, depth: number): Promise { + if (depth > maxDepth) { + issues.push({ path: directory, reason: 'DEPTH_LIMIT' }); + return; + } + let entries; + try { + entries = await fs.readdir(directory, { withFileTypes: true }); + } catch (error) { + recordError(directory, error); + if (directory === root) rootFailed = true; + return; + } + entries.sort((a, b) => a.name < b.name ? -1 : a.name > b.name ? 1 : 0); + for (const entry of entries) { + if (truncated) return; + const entryPath = path.join(directory, entry.name); + if (entry.isSymbolicLink()) { + // Observed symlinks are explicitly excluded; their targets are not + // claimed as scanned. Discovery does not guarantee an atomic snapshot. + excludedPaths.push(entryPath); + } else if (entry.isDirectory()) { + if (EXCLUDED_DIRECTORIES.has(entry.name)) excludedPaths.push(entryPath); + else await walk(entryPath, depth + 1); + } else if (entry.isFile() && isSecuritySourceFile(entry.name)) { + if (files.length === maxFiles) { + issues.push({ path: entryPath, reason: 'FILE_LIMIT' }); + truncated = true; + return; + } + files.push(entryPath); + } + } + } + + try { + const stat = await fs.lstat(root); + if (stat.isFile()) files.push(root); + else if (stat.isDirectory()) await walk(root, 0); + else { + rootFailed = true; + issues.push({ path: root, reason: stat.isSymbolicLink() ? 'SYMLINK_TARGET' : 'NOT_REGULAR_INPUT' }); + } + } catch (error) { + rootFailed = true; + recordError(root, error); + } + + return { + files, + discovery: { + status: rootFailed ? 'failed' : issues.length > 0 ? 'partial' : 'complete', + policy: 'aqe-security-source-files@1', + issues, + excludedPaths, + }, + }; +} diff --git a/src/domains/security-compliance/scan-evidence.ts b/src/domains/security-compliance/scan-evidence.ts new file mode 100644 index 000000000..89073f981 --- /dev/null +++ b/src/domains/security-compliance/scan-evidence.ts @@ -0,0 +1,59 @@ +/** Evidence of executed security analysis; findings alone do not establish coverage. */ +export type SecurityFileDisposition = + | 'analyzed' | 'unsupported' | 'unreadable' | 'excluded' + | 'failed' | 'unavailable' | 'not-run'; + +export interface SecurityFileEvidence { + /** Normalized absolute path; normalization does not resolve symlinks. */ + readonly path: string; + readonly engineId: string; + readonly status: SecurityFileDisposition; + /** SHA-256 of the exact source bytes read by this engine. */ + readonly sourceDigest?: string; + readonly readLines: number; + readonly analyzedLines: number; + /** A bounded error code or static description; never source contents. */ + readonly reason?: string; +} + +export interface SecurityEngineEvidence { + readonly id: string; + readonly requested: boolean; + /** Whether this engine is required for the declared analysis scope. */ + readonly required: boolean; + readonly status: 'completed' | 'partial' | 'failed' | 'unavailable' | 'disabled' | 'unverified' | 'not-run'; + readonly scope: 'requested-files' | 'parent-directory' | 'url'; + readonly target?: string; + /** Only populated when input membership was actually established. */ + readonly analyzedFiles?: number; + readonly ruleIds?: readonly string[]; + /** SHA-256 of selected rule definitions, when these are locally known. */ + readonly rulesetDigest?: string; + readonly ruleCoverage: 'known' | 'unknown'; + readonly version?: string; + readonly errors: readonly string[]; + readonly limitations: readonly string[]; +} + +export interface SecurityScanEvidence { + readonly schemaVersion: 1; + /** Completeness of required declared scope, not absence of vulnerabilities. */ + readonly completeness: 'complete' | 'partial' | 'none'; + /** Unique normalized absolute requested paths, without resolving symlinks. */ + readonly requestedFiles: number; + readonly requestedPaths: readonly string[]; + readonly duplicateInputs: number; + /** A path may have separate receipts for independently executed engines. */ + readonly files: readonly SecurityFileEvidence[]; + readonly engines: readonly SecurityEngineEvidence[]; + readonly limitations: readonly string[]; + /** Discovery describes the declared source-file policy, not every repository file. */ + readonly discovery?: SecurityDiscoveryEvidence; +} + +export interface SecurityDiscoveryEvidence { + readonly status: 'complete' | 'partial' | 'failed'; + readonly policy: 'aqe-security-source-files@1'; + readonly issues: readonly { readonly path: string; readonly reason: string }[]; + readonly excludedPaths: readonly string[]; +} diff --git a/src/domains/security-compliance/services/scanners/sast-scanner.ts b/src/domains/security-compliance/services/scanners/sast-scanner.ts index 1c8da2b75..479f8eaaf 100644 --- a/src/domains/security-compliance/services/scanners/sast-scanner.ts +++ b/src/domains/security-compliance/services/scanners/sast-scanner.ts @@ -3,10 +3,13 @@ * Performs static code analysis to detect security vulnerabilities */ +import { createHash } from 'node:crypto'; +import * as path from 'node:path'; +import { FilePath } from '@shared/value-objects/index.js'; +import type { SecurityEngineEvidence, SecurityFileEvidence, SecurityScanEvidence } from '../../scan-evidence.js'; import { LoggerFactory } from '../../../../logging/index.js'; import { v4 as uuidv4 } from 'uuid'; import { Result, ok, err } from '@shared/types/index.js'; -import type { FilePath } from '@shared/value-objects/index.js'; import type { SecurityPattern, SecurityScannerConfig, @@ -90,7 +93,6 @@ export class SASTScanner { return err(new Error('No files provided for scanning')); } - this.activeScans.set(scanId, 'running'); const startTime = Date.now(); // Get applicable rule sets @@ -102,34 +104,68 @@ export class SASTScanner { return err(new Error(`No valid rule sets found: ${ruleSetIds.join(', ')}`)); } - // Run pattern-based scanning and semgrep in parallel - const [patternResult, semgrepVulns] = await Promise.all([ - this.runPatternScanning(files, ruleSets), - this.runSemgrepScanning(files, ruleSetIds), + const unknownRuleSets = ruleSetIds.filter(id => !BUILT_IN_RULE_SETS.some(ruleSet => ruleSet.id === id)); + if (unknownRuleSets.length > 0) { + return err(new Error('Unknown rule sets requested; no scan executed.')); + } + this.activeScans.set(scanId, 'running'); + + // Deduplicate lexical absolute paths; do not follow symlinks to invent identity. + const requestedPaths = [...new Set(files.map(file => path.resolve(file.value)))]; + const uniqueFiles = requestedPaths.map(value => FilePath.create(value)); + + // Run independent engines while retaining each engine's execution outcome. + const [patternResult, semgrepResult] = await Promise.all([ + this.runPatternScanning(uniqueFiles, ruleSets), + this.runSemgrepScanning(uniqueFiles, ruleSetIds), ]); // Merge pattern-based and semgrep findings, deduplicating by file+line const vulnerabilities = this.mergeVulnerabilities( patternResult.vulnerabilities, - semgrepVulns + semgrepResult.vulnerabilities ); - const linesScanned = patternResult.linesScanned; + const analyzedFiles = patternResult.files.filter(file => file.status === 'analyzed'); + const linesScanned = analyzedFiles.reduce((total, file) => total + file.analyzedLines, 0); const scanDurationMs = Date.now() - startTime; // Calculate summary const summary = this.calculateSummary( vulnerabilities, - files.length, + analyzedFiles.length, scanDurationMs ); - // Calculate coverage — include semgrep rules when they ran - const patternRules = ruleSets.reduce((acc, rs) => acc + rs.ruleCount, 0); + const patterns = this.getApplicablePatterns(ruleSets); + const ruleIds = analyzedFiles.length > 0 ? [...new Set(patterns.map(pattern => pattern.id))] : []; const coverage: SecurityCoverage = { - filesScanned: files.length, + filesScanned: analyzedFiles.length, linesScanned, - rulesApplied: patternRules + (semgrepVulns.length > 0 ? semgrepVulns.length : 0), + rulesApplied: ruleIds.length, + rulesAppliedScope: 'built-in-patterns', + }; + const patternEngine: SecurityEngineEvidence = { + id: 'patterns', requested: true, required: true, scope: 'requested-files', + status: analyzedFiles.length === uniqueFiles.length ? 'completed' : analyzedFiles.length > 0 ? 'partial' : 'failed', + analyzedFiles: analyzedFiles.length, + ruleIds, + rulesetDigest: createHash('sha256').update(JSON.stringify(patterns.map(pattern => ({ + id: pattern.id, category: pattern.category, expression: pattern.pattern.source, flags: pattern.pattern.flags, + })))).digest('hex'), + ruleCoverage: 'known', + errors: patternResult.files.filter(file => file.status === 'unreadable').map(file => file.reason || 'File unreadable'), + limitations: ['Built-in pattern matching covers supported JavaScript and TypeScript inputs only.'], + }; + const evidence: SecurityScanEvidence = { + schemaVersion: 1, + completeness: analyzedFiles.length === uniqueFiles.length ? 'complete' : analyzedFiles.length > 0 ? 'partial' : 'none', + requestedFiles: uniqueFiles.length, + requestedPaths, + duplicateInputs: files.length - uniqueFiles.length, + files: patternResult.files, + engines: [patternEngine, semgrepResult.evidence], + limitations: [...patternEngine.limitations, ...semgrepResult.evidence.limitations], }; // Store scan results in memory @@ -142,6 +178,7 @@ export class SASTScanner { vulnerabilities, summary, coverage, + evidence, }); } catch (error) { this.activeScans.set(scanId, 'failed'); @@ -155,48 +192,54 @@ export class SASTScanner { private async runPatternScanning( files: FilePath[], ruleSets: RuleSet[] - ): Promise<{ vulnerabilities: Vulnerability[]; linesScanned: number }> { + ): Promise<{ vulnerabilities: Vulnerability[]; files: SecurityFileEvidence[] }> { const vulnerabilities: Vulnerability[] = []; - let linesScanned = 0; - + const receipts: SecurityFileEvidence[] = []; for (const file of files) { - const fileVulns = await this.analyzeFile(file, ruleSets); - vulnerabilities.push(...fileVulns.vulnerabilities); - linesScanned += fileVulns.linesScanned; + const result = await this.analyzeFile(file, ruleSets); + vulnerabilities.push(...result.vulnerabilities); + receipts.push(result.evidence); } - - return { vulnerabilities, linesScanned }; + return { vulnerabilities, files: receipts }; } /** * Run semgrep scanning when enabled and available. - * Returns converted vulnerabilities or empty array on failure/unavailability. + * Optional engine failure preserves completed pattern analysis and its findings. */ private async runSemgrepScanning( files: FilePath[], ruleSetIds: string[] - ): Promise { + ): Promise<{ vulnerabilities: Vulnerability[]; evidence: SecurityEngineEvidence }> { + const targetDir = this.resolveTargetDirectory(files); + const base: SecurityEngineEvidence = { + id: 'semgrep', requested: !!this.config.enableSemgrep, required: false, + status: 'disabled', scope: 'parent-directory', target: targetDir, + ruleCoverage: 'unknown', errors: [], limitations: [], + }; if (!this.config.enableSemgrep) { - return []; + return { vulnerabilities: [], evidence: base }; } - + const limitations = [ + 'Optional Semgrep scans a parent directory; requested-file membership and executed-rule coverage are unverified.', + ]; try { - const available = await isSemgrepAvailable(); - if (!available) { - return []; + if (!await isSemgrepAvailable()) { + return { vulnerabilities: [], evidence: { ...base, status: 'unavailable', limitations, + errors: ['Semgrep is unavailable.'] } }; } - - // Determine target directory from files (use common parent) - const targetDir = this.resolveTargetDirectory(files); - const semgrepResult = await runSemgrepWithRules(targetDir, ruleSetIds); - if (!semgrepResult.success || semgrepResult.findings.length === 0) { - return []; - } - + const evidence: SecurityEngineEvidence = { + ...base, + status: semgrepResult.status || 'unverified', + version: semgrepResult.version, + errors: semgrepResult.errors.length > 0 ? [`Semgrep reported ${semgrepResult.errors.length} execution error(s).`] : [], + limitations: [...limitations, ...(semgrepResult.diagnostics ?? []), + ...(!semgrepResult.status ? ['Legacy Semgrep adapter returned no execution disposition.'] : [])], + }; // Convert semgrep findings to our Vulnerability format const converted = convertSemgrepFindings(semgrepResult.findings); - return converted.map(f => ({ + const vulnerabilities = converted.map(f => ({ id: uuidv4(), cveId: undefined, title: f.title, @@ -216,9 +259,10 @@ export class SASTScanner { }, references: f.references, })); + return { vulnerabilities, evidence }; } catch { - // Semgrep failure is non-fatal — pattern scanning still covers us - return []; + return { vulnerabilities: [], evidence: { ...base, status: 'failed', limitations, + errors: ['Semgrep execution failed.'] } }; } } @@ -226,26 +270,17 @@ export class SASTScanner { * Resolve the common parent directory from a set of file paths */ private resolveTargetDirectory(files: FilePath[]): string { - if (files.length === 0) return '.'; - if (files.length === 1) return files[0].directory || '.'; - - // Find common prefix of all directories - const dirs = files.map(f => f.directory || '.'); - const first = dirs[0]; - let commonLen = first.length; - - for (let i = 1; i < dirs.length; i++) { - const dir = dirs[i]; - const maxLen = Math.min(commonLen, dir.length); - let j = 0; - while (j < maxLen && first[j] === dir[j]) j++; - commonLen = j; + if (files.length === 0) return process.cwd(); + let common = path.dirname(path.resolve(files[0].value)); + for (const file of files.slice(1)) { + const directory = path.dirname(path.resolve(file.value)); + while (directory !== common && !directory.startsWith(common.endsWith(path.sep) ? common : common + path.sep)) { + const parent = path.dirname(common); + if (parent === common) break; + common = parent; + } } - - const common = first.substring(0, commonLen); - // Trim to last path separator - const lastSep = common.lastIndexOf('/'); - return lastSep > 0 ? common.substring(0, lastSep) : common || '.'; + return common; } /** @@ -349,41 +384,38 @@ export class SASTScanner { /** * Analyze a file for security vulnerabilities using pattern-based detection */ + private getApplicablePatterns(ruleSets: RuleSet[]): SecurityPattern[] { + const categories = new Set(ruleSets.flatMap(ruleSet => ruleSet.categories)); + return ALL_SECURITY_PATTERNS.filter(pattern => categories.has(pattern.category)); + } + private async analyzeFile( file: FilePath, ruleSets: RuleSet[] - ): Promise<{ vulnerabilities: Vulnerability[]; linesScanned: number }> { + ): Promise<{ vulnerabilities: Vulnerability[]; evidence: SecurityFileEvidence }> { const vulnerabilities: Vulnerability[] = []; const filePath = file.value; - const extension = file.extension; - - // Read file content + const receipt = { path: filePath, engineId: 'patterns', readLines: 0, analyzedLines: 0 }; let content: string; - let lines: string[]; + let sourceDigest: string; try { const fs = await import('fs/promises'); - content = await fs.readFile(filePath, 'utf-8'); - lines = content.split('\n'); - } catch { - // File not accessible - return empty results - return { vulnerabilities: [], linesScanned: 0 }; + const bytes = await fs.readFile(filePath); + sourceDigest = createHash('sha256').update(bytes).digest('hex'); + content = bytes.toString('utf8'); + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + return { vulnerabilities: [], evidence: { ...receipt, status: 'unreadable', + reason: typeof code === 'string' && /^[A-Z0-9_]{1,32}$/.test(code) ? code : 'File unreadable' } }; } - - const linesScanned = lines.length; - - // Only scan supported file types + const lines = content.split('\n'); + const readReceipt = { ...receipt, sourceDigest, readLines: lines.length }; const supportedExtensions = ['ts', 'tsx', 'js', 'jsx', 'mjs', 'cjs']; - if (!supportedExtensions.includes(extension)) { - return { vulnerabilities: [], linesScanned }; + if (!supportedExtensions.includes(file.extension)) { + return { vulnerabilities: [], evidence: { ...readReceipt, status: 'unsupported', + reason: 'Unsupported language for built-in SAST patterns' } }; } - - // Get applicable categories from rule sets - const applicableCategories = new Set(ruleSets.flatMap((rs) => rs.categories)); - - // Filter patterns to only those matching applicable categories - const applicablePatterns = ALL_SECURITY_PATTERNS.filter((pattern) => - applicableCategories.has(pattern.category) - ); + const applicablePatterns = this.getApplicablePatterns(ruleSets); // Scan content for each pattern for (const securityPattern of applicablePatterns) { @@ -405,7 +437,7 @@ export class SASTScanner { } } - return { vulnerabilities, linesScanned }; + return { vulnerabilities, evidence: { ...readReceipt, status: 'analyzed', analyzedLines: lines.length } }; } /** diff --git a/src/domains/security-compliance/services/semgrep-integration.ts b/src/domains/security-compliance/services/semgrep-integration.ts index e13320bf0..ee7939cbc 100644 --- a/src/domains/security-compliance/services/semgrep-integration.ts +++ b/src/domains/security-compliance/services/semgrep-integration.ts @@ -13,7 +13,6 @@ import { execFile } from 'child_process'; import { promisify } from 'util'; import * as path from 'path'; -import { toErrorMessage } from '../../../shared/error-utils.js'; import { safeJsonParse } from '../../../shared/safe-json.js'; const execFileAsync = promisify(execFile); @@ -22,6 +21,11 @@ const execFileAsync = promisify(execFile); // Types // ============================================================================ +// Semgrep's match_severity schema retains legacy/deprecated values as well as +// CRITICAL/HIGH/MEDIUM/LOW introduced in 1.72.0. +const SEMGREP_SEVERITIES = ['ERROR', 'WARNING', 'INFO', 'CRITICAL', 'HIGH', 'MEDIUM', 'LOW', 'EXPERIMENT', 'INVENTORY'] as const; +export type SemgrepSeverity = typeof SEMGREP_SEVERITIES[number]; + export interface SemgrepFinding { check_id: string; path: string; @@ -29,7 +33,7 @@ export interface SemgrepFinding { end: { line: number; col: number }; extra: { message: string; - severity: 'ERROR' | 'WARNING' | 'INFO'; + severity: SemgrepSeverity; lines: string; metadata?: { cwe?: string[]; @@ -45,8 +49,12 @@ export interface SemgrepFinding { export interface SemgrepResult { success: boolean; + /** Optional only for compatibility with legacy adapters. */ + status?: 'completed' | 'partial' | 'failed' | 'unavailable'; findings: SemgrepFinding[]; errors: string[]; + /** Sanitized non-fatal diagnostics, separate from analysis failures. */ + diagnostics?: string[]; version?: string; } @@ -163,8 +171,9 @@ export async function runSemgrep(config: Partial): Promise): Promise 0) { + const result = parseSemgrepOutput(execError.stdout); + // Exit 1 can represent findings with --error. Other process failures cannot + // be converted into success just because stdout contains valid JSON. + if (execError.code === 1 && !execError.killed && !execError.signal && result.success && result.findings.length > 0) { return result; - } catch { - // Parse failed, return error } + return { ...result, success: false, status: 'failed', + errors: [...result.errors, 'Semgrep process did not complete successfully.'] }; } - return { - success: false, - findings: [], - errors: [execError.message ?? String(error)], + success: false, status: 'failed', findings: [], + errors: ['Semgrep execution failed.'], }; } } @@ -228,13 +235,45 @@ export async function runSemgrep(config: Partial): Promise e.message || String(e)) || []; + if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) { + throw new Error('Expected a JSON object'); + } + const rawResults = parsed.results ?? parsed.findings; + if (!Array.isArray(rawResults) || (parsed.errors !== undefined && !Array.isArray(parsed.errors))) { + throw new Error('Missing or invalid result collection'); + } + const isRecord = (value: unknown): value is Record => + !!value && typeof value === 'object' && !Array.isArray(value); + const optionalText = (value: unknown): boolean => value === undefined || typeof value === 'string'; + const validSeverity = (value: unknown): boolean => value === undefined + || (typeof value === 'string' && (SEMGREP_SEVERITIES as readonly string[]).includes(value)); + const validPosition = (value: unknown): boolean => value === undefined || (isRecord(value) + && ['line', 'col'].every(key => value[key] === undefined + || (typeof value[key] === 'number' && Number.isSafeInteger(value[key]) && (value[key] as number) > 0))); + const validMetadata = (value: unknown): boolean => value === undefined || (isRecord(value) + && ['cwe', 'owasp', 'references'].every(key => value[key] === undefined + || (Array.isArray(value[key]) && (value[key] as unknown[]).every(item => typeof item === 'string'))) + && ['category', 'description', 'fix', 'confidence'].every(key => optionalText(value[key]))); + const validFinding = (value: SemgrepRawFinding): boolean => { + if (!isRecord(value)) return false; + const id = value.check_id ?? value.rule_id; + if (typeof value.path !== 'string' || value.path.length === 0 || typeof id !== 'string' || id.length === 0 + || !validPosition(value.start) || !validPosition(value.end) + || !optionalText(value.message) || !validSeverity(value.severity) || !validMetadata(value.metadata)) return false; + const extra: unknown = value.extra; + return extra === undefined || (isRecord(extra) + && ['message', 'lines', 'fix'].every(key => optionalText(extra[key])) + && validSeverity(extra.severity) && validMetadata(extra.metadata)); + }; + const results = rawResults.filter(validFinding); + const malformed = rawResults.length - results.length; + const errors: string[] = []; + if (parsed.errors?.length) errors.push(`Semgrep reported ${parsed.errors.length} analysis error(s).`); + if (malformed) errors.push(`Semgrep returned ${malformed} malformed finding(s).`); return { - success: true, + success: errors.length === 0, + status: errors.length === 0 ? 'completed' : 'partial', findings: results.map(r => ({ check_id: r.check_id || r.rule_id || 'unknown', path: r.path, @@ -242,7 +281,7 @@ function parseSemgrepOutput(stdout: string): SemgrepResult { end: { line: r.end?.line || r.start?.line || 1, col: r.end?.col || 1 }, extra: { message: r.extra?.message || r.message || 'Security issue detected', - severity: (r.extra?.severity || r.severity || 'WARNING') as 'ERROR' | 'WARNING' | 'INFO', + severity: (r.extra?.severity || r.severity || 'WARNING') as SemgrepSeverity, lines: r.extra?.lines || '', metadata: { cwe: r.extra?.metadata?.cwe || r.metadata?.cwe, @@ -257,11 +296,12 @@ function parseSemgrepOutput(stdout: string): SemgrepResult { })), errors, }; - } catch (error) { + } catch { return { success: false, + status: 'failed', findings: [], - errors: [`Failed to parse semgrep output: ${toErrorMessage(error)}`], + errors: ['Invalid Semgrep output.'], }; } } @@ -306,12 +346,19 @@ export async function runSemgrepWithRules( * Map semgrep severity to standard severity */ export function mapSemgrepSeverity( - severity: 'ERROR' | 'WARNING' | 'INFO' + severity: SemgrepSeverity ): 'critical' | 'high' | 'medium' | 'low' { const mapping: Record = { ERROR: 'high', WARNING: 'medium', INFO: 'low', + CRITICAL: 'critical', + HIGH: 'high', + MEDIUM: 'medium', + LOW: 'low', + // Retain the existing non-risky INFO mapping for deprecated categories. + EXPERIMENT: 'low', + INVENTORY: 'low', }; return mapping[severity] || 'medium'; } diff --git a/src/mcp/handlers/domain-handler-configs.ts b/src/mcp/handlers/domain-handler-configs.ts index c445a8f5b..a01c289fb 100644 --- a/src/mcp/handlers/domain-handler-configs.ts +++ b/src/mcp/handlers/domain-handler-configs.ts @@ -41,6 +41,9 @@ import { type RiskDecision, } from '../../contracts/verdicts.js'; +import type { SecurityScanEvidence } from '../../domains/security-compliance/scan-evidence.js'; +import type { SecurityCoverage } from '../../domains/security-compliance/interfaces.js'; + const SUPPORTED_LANGUAGES = Object.keys(DEFAULT_FRAMEWORKS) as SupportedLanguage[]; /** @@ -110,9 +113,21 @@ export interface SecurityScanResult { medium: number; low: number; topVulnerabilities: unknown[]; + findings?: unknown[]; recommendations: string[]; duration: number; savedFiles?: string[]; + informational?: number; + evidence?: SecurityScanEvidence; + limitations?: readonly string[]; + coverage?: SecurityCoverage; + filesScanned?: number; + jstsFilesScanned?: number; + otherFilesScanned?: number; + deepAnalysisPerformed?: boolean; + analysisDepth?: string; + scanTypes?: { sast: boolean; dast: boolean }; + note?: string; } export interface ContractValidateResult { @@ -668,26 +683,51 @@ export const securityScanConfig: DomainHandlerConfig ({ - sast: params.sast !== false, - dast: params.dast || false, - compliance: params.compliance || [], - target: params.target || '.', - routingTier: routingResult?.decision.tier, - useAgentBooster: routingResult?.useAgentBooster, - compiledContext: routingResult?.compiledContext, - }), + mapToPayload: (params, routingResult) => { + for (const name of ['sast', 'dast'] as const) { + if (params[name] !== undefined && typeof params[name] !== 'boolean') { + throw new Error(`${name} must be a boolean`); + } + } + return { + sast: params.sast !== false, + dast: params.dast || false, + compliance: params.compliance || [], + target: params.target || '.', + targetUrl: params.targetUrl, + routingTier: routingResult?.decision.tier, + useAgentBooster: routingResult?.useAgentBooster, + compiledContext: routingResult?.compiledContext, + }; + }, mapToResult: (taskId, data, duration, savedFiles) => ({ taskId, - status: 'completed', + // Legacy task results without receipts cannot establish scan completeness. + status: (data.evidence as SecurityScanEvidence | undefined)?.completeness === 'complete' ? 'completed' + : (data.evidence as SecurityScanEvidence | undefined)?.completeness === 'partial' ? 'partial' + : data.evidence ? 'unavailable' : 'unverified', vulnerabilities: (data.vulnerabilities as number) || 0, critical: (data.critical as number) || 0, high: (data.high as number) || 0, medium: (data.medium as number) || 0, low: (data.low as number) || 0, + informational: (data.informational as number) || 0, topVulnerabilities: (data.topVulnerabilities as unknown[]) || [], + findings: data.findings as unknown[] | undefined, recommendations: (data.recommendations as string[]) || [], + evidence: data.evidence as SecurityScanEvidence | undefined, + limitations: (data.limitations as string[]) || ['Execution receipts unavailable; scan completeness is unverified.'], + ...(data.evidence ? { + filesScanned: data.filesScanned as number | undefined, + jstsFilesScanned: data.jstsFilesScanned as number | undefined, + otherFilesScanned: data.otherFilesScanned as number | undefined, + coverage: data.coverage as SecurityCoverage | undefined, + } : {}), + deepAnalysisPerformed: Boolean(data.evidence && data.deepAnalysisPerformed === true), + analysisDepth: data.analysisDepth as string | undefined, + scanTypes: data.scanTypes as { sast: boolean; dast: boolean } | undefined, + note: data.note as string | undefined, duration, savedFiles, }), diff --git a/src/mcp/protocol-server.ts b/src/mcp/protocol-server.ts index 7176df1d1..30a3b2fc0 100644 --- a/src/mcp/protocol-server.ts +++ b/src/mcp/protocol-server.ts @@ -1083,12 +1083,13 @@ export class MCPProtocolServer { this.registerTool({ definition: { name: 'security_scan_comprehensive', - description: 'Run SAST and/or DAST security scans with vulnerability classification. Example: security_scan_comprehensive({ target: "src/", sast: true })', + description: 'Run security scans with execution evidence and explicit coverage limitations. Complete means the declared required scope ran, not that all vulnerabilities are absent. Example: security_scan_comprehensive({ target: "src/", sast: true })', category: 'domain', parameters: [ { name: 'sast', type: 'boolean', description: 'Run SAST scan', default: true }, { name: 'dast', type: 'boolean', description: 'Run DAST scan', default: false }, - { name: 'target', type: 'string', description: 'Target to scan' }, + { name: 'target', type: 'string', description: 'Source file or directory for SAST' }, + { name: 'targetUrl', type: 'string', description: 'URL for a requested DAST scan' }, ], }, handler: (params) => handleSecurityScan(params as unknown as Parameters[0]), diff --git a/src/mcp/types.ts b/src/mcp/types.ts index 1bc3ea185..9e4923756 100644 --- a/src/mcp/types.ts +++ b/src/mcp/types.ts @@ -268,6 +268,8 @@ export interface SecurityScanParams { dast?: boolean; compliance?: string[]; target?: string; + /** URL for explicitly requested dynamic scanning. */ + targetUrl?: string; } /** diff --git a/tests/integration/security-scan-execution-receipts.test.ts b/tests/integration/security-scan-execution-receipts.test.ts new file mode 100644 index 000000000..10ae91a31 --- /dev/null +++ b/tests/integration/security-scan-execution-receipts.test.ts @@ -0,0 +1,289 @@ +/** Registered security scan results must describe work that actually executed. */ +import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; +import { mkdtemp, writeFile, readFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { createServer, type Server } from 'node:http'; +import type { MCPProtocolServer } from '../../src/mcp/protocol-server.js'; +import type { SecurityScannerService } from '../../src/domains/security-compliance/services/security-scanner.js'; +import type { ToolResult } from '../../src/mcp/types.js'; + +// Keep the registered handler, executor, discovery, kernel memory, and built-in +// scanner real. Only external Semgrep availability and routing advice are mocked. +vi.mock('../../src/mcp/services/task-router.js', () => ({ + getTaskRouter: vi.fn(async () => { throw new Error('Offline routing fixture'); }), +})); +vi.mock('../../src/domains/security-compliance/services/semgrep-integration.js', () => ({ + isSemgrepAvailable: vi.fn(async () => false), + runSemgrepWithRules: vi.fn(async () => { throw new Error('External Semgrep must not run'); }), + convertSemgrepFindings: vi.fn(() => []), +})); + +type ScanResponse = { + savedFiles?: string[]; findings?: unknown[]; status: string; vulnerabilities: number; filesScanned: number; + deepAnalysisPerformed: boolean; topVulnerabilities: Array<{ type: string }>; + coverage?: { filesScanned: number }; limitations: string[]; recommendations: string[]; + evidence: { + completeness: string; requestedFiles: number; + files: Array<{ path: string; status: string; sourceDigest?: string; engineId: string }>; + engines: Array<{ id: string; requested: boolean; status: string; errors: string[] }>; + discovery?: { status: string; issues: Array<{ path: string; reason: string }> }; + }; +}; + +describe('registered security_scan_comprehensive execution receipts', () => { + let server: MCPProtocolServer; + let embedder: Server; + let root: string; + let scanner: SecurityScannerService; + let core: typeof import('../../src/mcp/handlers/core-handlers.js'); + let resetCache: () => void; + const originalCwd = process.cwd(); + + async function callTool(name: string, args: Record): Promise> { + const response = await server['handleRequest']({ + jsonrpc: '2.0', id: 1, method: 'tools/call', params: { name, arguments: args }, + }) as { content: Array<{ text: string }> }; + return JSON.parse(response.content[0].text) as ToolResult; + } + + async function source(name: string, content = 'export const answer = 42;\n'): Promise { + const dir = await mkdtemp(join(root, name)); + await writeFile(join(dir, 'example.ts'), content); + return dir; + } + + beforeAll(async () => { + root = await mkdtemp(join(tmpdir(), 'aqe-security-receipts-')); + process.chdir(root); + // Set every path before production imports. The entire test uses a new root. + vi.stubEnv('TMPDIR', root); + vi.stubEnv('AQE_PROJECT_ROOT', root); + vi.stubEnv('AQE_MEMORY_BACKEND', 'memory'); + vi.stubEnv('AQE_SESSION_CACHE', 'on'); + vi.stubEnv('AQE_LOOP_DETECTION_ENABLED', 'false'); + vi.stubEnv('AQE_LLM_ROUTER_DISABLED', 'true'); + vi.stubEnv('AQE_TRAJECTORY_JUDGE', '0'); + vi.stubEnv('AQE_LEARNING_ENABLED', 'false'); + embedder = createServer((request, response) => { + let body = ''; + request.on('data', chunk => { body += chunk; }); + request.on('end', () => { + const { input } = JSON.parse(body); + const inputs = Array.isArray(input) ? input : [input]; + response.setHeader('Content-Type', 'application/json'); + response.end(JSON.stringify({ data: inputs.map((_, index) => ({ + index, embedding: [1, ...Array(383).fill(0)], + })) })); + }); + }); + await new Promise(resolve => embedder.listen(0, '127.0.0.1', resolve)); + vi.stubEnv('AQE_EMBEDDER_ENDPOINT', `http://127.0.0.1:${(embedder.address() as { port: number }).port}`); + const { createMCPProtocolServer } = await import('../../src/mcp/protocol-server.js'); + core = await import('../../src/mcp/handlers/core-handlers.js'); + ({ resetSessionCache: resetCache } = await import('../../src/optimization/session-cache.js')); + server = createMCPProtocolServer(); + const fleet = await callTool('fleet_init', { memoryBackend: 'memory', maxAgents: 2 }); + expect(fleet.success).toBe(true); + const { getTaskExecutor } = await import('../../src/mcp/handlers/handler-factory.js'); + scanner = getTaskExecutor().getSecurityScanner(); + }, 60000); + + beforeEach(() => { + resetCache(); + core.getFleetState().queen!.getDomainBreakerRegistry()?.resetAll(); + // Configure the real producer, without replacing its results or scan method. + Object.assign((scanner as unknown as { config: Record }).config, { + defaultRuleSets: ['owasp-top-10', 'cwe-sans-25'], enableSemgrep: false, enableLLMAnalysis: false, + }); + }); + + afterAll(async () => { + resetCache?.(); + await server?.stop(); + if (embedder) await new Promise(resolve => embedder.close(() => resolve())); + process.chdir(originalCwd); + vi.unstubAllEnvs(); + }); + + it('preserves an unavailable discovery outcome instead of a completed clean scan', async () => { + const result = await callTool('security_scan_comprehensive', { target: join(root, 'absent'), sast: true }); + expect(result.success).toBe(true); + expect(result.data).toMatchObject({ status: 'unavailable', filesScanned: 0, deepAnalysisPerformed: false, + evidence: { completeness: 'none', discovery: { status: 'failed' } } }); + expect(result.data!.evidence.discovery!.issues.length).toBeGreaterThan(0); + expect(result.data!.recommendations).toEqual(['No findings reported; requested analysis is incomplete or unverified. Inspect execution receipts.']); + }); + + it('preserves real completed clean receipts and their source digests', async () => { + const target = await source('clean-'); + const result = await callTool('security_scan_comprehensive', { target, sast: true }); + expect(result.success).toBe(true); + expect(result.data).toMatchObject({ status: 'completed', vulnerabilities: 0, filesScanned: 1, + deepAnalysisPerformed: true, evidence: { completeness: 'complete', requestedFiles: 1 } }); + expect(result.data!.evidence.files).toEqual(expect.arrayContaining([ + expect.objectContaining({ path: join(target, 'example.ts'), status: 'analyzed', sourceDigest: expect.stringMatching(/^[a-f0-9]{64}$/) }), + ])); + }); + + it('retains findings and a failed SAST outcome when the real scanner rejects its rule configuration', async () => { + const target = await source('partial-', 'eval(userInput);\n'); + (scanner as unknown as { config: { defaultRuleSets: string[] } }).config.defaultRuleSets = ['unknown-fixture-rules']; + const result = await callTool('security_scan_comprehensive', { target, sast: true }); + expect(result.success).toBe(true); + expect(result.data).toMatchObject({ status: 'partial', deepAnalysisPerformed: false, + evidence: { completeness: 'partial' } }); + expect(result.data!.vulnerabilities).toBeGreaterThan(0); + expect(result.data!.topVulnerabilities.some(v => v.type.includes('eval/exec'))).toBe(true); + expect(result.data!.evidence.engines).toEqual(expect.arrayContaining([ + expect.objectContaining({ status: 'failed', errors: expect.arrayContaining([expect.stringContaining('No valid rule sets')]) }), + ])); + }); + + it('does no source scanning when both scan modes are disabled', async () => { + const target = await source('disabled-', 'eval(userInput);\n'); + const result = await callTool('security_scan_comprehensive', { target, sast: false, dast: false }); + expect(result.success).toBe(true); + expect(result.data).toMatchObject({ status: 'unavailable', vulnerabilities: 0, filesScanned: 0, + deepAnalysisPerformed: false, evidence: { completeness: 'none' } }); + expect(result.data!.evidence.engines.every(engine => !engine.requested)).toBe(true); + }); + + it('reports requested DAST without a URL as not-run without unrelated source discovery', async () => { + const result = await callTool('security_scan_comprehensive', { + target: join(root, 'not-a-source-target'), sast: false, dast: true, + }); + expect(result.success).toBe(true); + expect(result.data).toMatchObject({ status: 'unavailable', filesScanned: 0, + evidence: { completeness: 'none' } }); + expect(result.data!.evidence.engines).toEqual(expect.arrayContaining([ + expect.objectContaining({ id: 'dast', requested: true, status: 'not-run' }), + ])); + expect(result.data!.evidence.discovery).toBeUndefined(); + }); + + it('exposes an unavailable optional external engine without erasing completed built-in work', async () => { + const target = await source('optional-'); + (scanner as unknown as { config: { enableSemgrep: boolean } }).config.enableSemgrep = true; + const result = await callTool('security_scan_comprehensive', { target, sast: true }); + expect(result.success).toBe(true); + expect(result.data).toMatchObject({ status: 'completed', evidence: { completeness: 'complete' } }); + expect(result.data!.evidence.engines).toEqual(expect.arrayContaining([ + expect.objectContaining({ id: 'semgrep', status: 'unavailable' }), + ])); + expect(result.data!.limitations.length).toBeGreaterThan(0); + }); + + it('does not credit a legacy DAST success without a receipt establishing execution', async () => { + const result = await callTool('security_scan_comprehensive', { + sast: false, dast: true, targetUrl: 'not-a-valid-url', + }); + expect(result.success).toBe(true); + expect(result.data).toMatchObject({ status: 'unavailable', filesScanned: 0, + evidence: { completeness: 'none' } }); + expect(result.data!.evidence.engines).toEqual(expect.arrayContaining([ + expect.objectContaining({ id: 'dast', status: 'unverified' }), + ])); + expect(result.data!.limitations).toContain('dast returned no execution receipt; coverage is unverified.'); + }); + + it('treats a legacy task result without receipts as unverified', async () => { + const { securityScanConfig } = await import('../../src/mcp/handlers/domain-handler-configs.js'); + const result = securityScanConfig.mapToResult('legacy-fixture', { + vulnerabilities: 0, filesScanned: 100, deepAnalysisPerformed: true, + }, 1); + expect(result.status).toBe('unverified'); + expect(result.deepAnalysisPerformed).toBe(false); + expect(result.filesScanned).toBeUndefined(); + expect(result.coverage).toBeUndefined(); + }); + + + it('does not credit an explicitly requested unsupported file as analyzed', async () => { + const target = join(root, 'unsupported.bin'); + await writeFile(target, 'eval(userInput);'); + const result = await callTool('security_scan_comprehensive', { target, sast: true }); + expect(result.success).toBe(true); + expect(result.data).toMatchObject({ status: 'unavailable', vulnerabilities: 0, filesScanned: 0, + deepAnalysisPerformed: false, evidence: { completeness: 'none', requestedFiles: 1 } }); + expect(result.data!.evidence.files).toEqual(expect.arrayContaining([ + expect.objectContaining({ path: target, engineId: 'generic-patterns', status: 'unsupported' }), + ])); + expect(result.data!.recommendations.join(' ')).not.toContain('maintain current security practices'); + }); + + it('credits actual generic analysis of Python without claiming language-specific SAST', async () => { + const target = join(root, 'generic.py'); + await writeFile(target, 'print("hello")\n'); + const result = await callTool('security_scan_comprehensive', { target, sast: true }); + expect(result.success).toBe(true); + expect(result.data).toMatchObject({ status: 'completed', filesScanned: 1, deepAnalysisPerformed: false }); + expect(result.data!.evidence.engines).toEqual(expect.arrayContaining([ + expect.objectContaining({ id: 'generic-patterns', status: 'completed' }), + expect.objectContaining({ id: 'sast', status: 'not-run' }), + ])); + }); + + it('executes a fresh scan when source changes under identical public arguments', async () => { + const target = await source('changed-'); + const args = { target, sast: true }; + const first = await callTool('security_scan_comprehensive', args); + expect(first.data?.vulnerabilities).toBe(0); + const firstDigest = first.data!.evidence.files.find(file => file.engineId === 'generic-patterns')!.sourceDigest; + await writeFile(join(target, 'example.ts'), 'eval(userInput);\n'); + const second = await callTool('security_scan_comprehensive', args); + expect(second.data!.vulnerabilities).toBeGreaterThan(0); + expect(second.data!.evidence.files.find(file => file.engineId === 'generic-patterns')!.sourceDigest).not.toBe(firstDigest); + }); + + it.each(['false', 0, null])('rejects a non-boolean SAST flag (%j)', async (sast) => { + const result = await callTool('security_scan_comprehensive', { sast }); + expect(result.success).toBe(false); + expect(result.error).toContain('sast must be a boolean'); + expect(result.data).toBeUndefined(); + }); + + + it('marks requested compliance checks not-run without claiming they were performed', async () => { + const target = await source('compliance-'); + const result = await callTool('security_scan_comprehensive', { + target, sast: true, compliance: ['soc2'], + }); + expect(result.success).toBe(true); + expect(result.data).toMatchObject({ status: 'partial', evidence: { completeness: 'partial' } }); + expect(result.data!.evidence.engines).toEqual(expect.arrayContaining([ + expect.objectContaining({ id: 'compliance', requested: true, status: 'not-run' }), + ])); + }); + + it('preserves incomplete execution and every finding in saved JSON, Markdown, and SARIF', async () => { + // Enable only result-file writes after fleet initialization. Any incidental + // storage remains confined to this suite's fresh cwd/AQE_PROJECT_ROOT. + vi.stubEnv('AQE_MEMORY_BACKEND', 'sqlite'); + try { + const lines = Array.from({ length: 12 }, (_, i) => `const password_${i} = "synthetic-fixture-${i}";`).join('\n'); + const target = await source('saved-partial-', lines); + (scanner as unknown as { config: { defaultRuleSets: string[] } }).config.defaultRuleSets = ['unknown-fixture-rules']; + const result = await callTool('security_scan_comprehensive', { target, sast: true }); + expect(result.success).toBe(true); + expect(result.data?.status).toBe('partial'); + expect(result.data!.vulnerabilities).toBeGreaterThan(10); + const files = result.data!.savedFiles!; + expect(files).toHaveLength(3); + const saved = JSON.parse(await readFile(files.find(file => file.endsWith('_scan.json'))!, 'utf8')); + expect(saved.status).toBe('partial'); + expect(saved.findings).toHaveLength(result.data!.vulnerabilities); + const sarif = JSON.parse(await readFile(files.find(file => file.endsWith('.sarif'))!, 'utf8')); + expect(sarif.runs[0].invocations[0].executionSuccessful).toBe(false); + expect(sarif.runs[0].properties.securityScan.evidence.completeness).toBe('partial'); + expect(sarif.runs[0].results).toHaveLength(result.data!.vulnerabilities); + const markdown = await readFile(files.find(file => file.endsWith('_report.md'))!, 'utf8'); + expect(markdown).toContain('**Execution:** partial'); + expect(markdown).toContain('## Execution receipts'); + expect(markdown).toContain('## All findings'); + } finally { + vi.stubEnv('AQE_MEMORY_BACKEND', 'memory'); + } + }); + +}); diff --git a/tests/integration/security/vulnerability-detection.test.ts b/tests/integration/security/vulnerability-detection.test.ts index 9d909b489..31884fcfa 100644 --- a/tests/integration/security/vulnerability-detection.test.ts +++ b/tests/integration/security/vulnerability-detection.test.ts @@ -3,6 +3,9 @@ * Tests that perform real vulnerability detection with actual code patterns */ +import { mkdtemp, rm, writeFile } from 'node:fs/promises'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import { SecurityScannerService } from '../../../src/domains/security-compliance/services/security-scanner'; import type { MemoryBackend, VectorSearchResult } from '../../../src/kernel/interfaces'; @@ -305,15 +308,19 @@ describe('Security Scanner Integration', () => { let scanner: SecurityScannerService; let memory: MemoryBackend; let detector: CodeVulnerabilityDetector; + let fixture: string; - beforeEach(() => { + beforeEach(async () => { + fixture = await mkdtemp(join(tmpdir(), 'aqe-vulnerability-detection-')); + await Promise.all(['app.ts', 'safe.ts', 'vulnerable.ts'].map(name => writeFile(join(fixture, name), 'const safe = true;\n'))); memory = createMockMemoryBackend(); - scanner = new SecurityScannerService(memory); + scanner = new SecurityScannerService(memory, { enableSemgrep: false, enableLLMAnalysis: false }); detector = new CodeVulnerabilityDetector(); }); - afterEach(() => { + afterEach(async () => { vi.restoreAllMocks(); + await rm(fixture, { recursive: true, force: true }); }); describe('SQL Injection Detection', () => { @@ -520,8 +527,8 @@ describe('Security Scanner Integration', () => { describe('SecurityScannerService Integration', () => { it('should scan multiple files and return results', async () => { const files = [ - createMockFilePath('/src/vulnerable.ts'), - createMockFilePath('/src/safe.ts'), + createMockFilePath(join(fixture, 'vulnerable.ts')), + createMockFilePath(join(fixture, 'safe.ts')), ]; const result = await scanner.scanFiles(files); @@ -534,13 +541,15 @@ describe('Security Scanner Integration', () => { }); it('should scan with OWASP Top 10 rules', async () => { - const files = [createMockFilePath('/src/app.ts')]; + const files = [createMockFilePath(join(fixture, 'app.ts'))]; const result = await scanner.scanWithRules(files, ['owasp-top-10']); expect(result.success).toBe(true); if (result.success) { - expect(result.value.coverage.rulesApplied).toBeGreaterThan(40); + expect(result.value.coverage.rulesApplied).toBeGreaterThan(0); + expect(result.value.evidence?.engines[0].ruleIds).toEqual(expect.arrayContaining(['sqli-string-concat', 'xss-innerhtml'])); + expect(result.value.coverage.rulesApplied).toBe(new Set(result.value.evidence?.engines[0].ruleIds).size); } }); @@ -558,7 +567,7 @@ describe('Security Scanner Integration', () => { }); it('should run full combined SAST and DAST scan', async () => { - const files = [createMockFilePath('/src/app.ts')]; + const files = [createMockFilePath(join(fixture, 'app.ts'))]; const result = await scanner.runFullScan(files, 'https://example.com'); @@ -574,7 +583,7 @@ describe('Security Scanner Integration', () => { }); it('should store scan results in memory', async () => { - const files = [createMockFilePath('/src/app.ts')]; + const files = [createMockFilePath(join(fixture, 'app.ts'))]; await scanner.scanFiles(files); diff --git a/tests/unit/cli/ci-output.test.ts b/tests/unit/cli/ci-output.test.ts index 773c4ea4b..6ffdc6846 100644 --- a/tests/unit/cli/ci-output.test.ts +++ b/tests/unit/cli/ci-output.test.ts @@ -89,7 +89,9 @@ describe('toSARIF', () => { // Invocations expect(run.invocations).toBeInstanceOf(Array); - expect(run.invocations[0].executionSuccessful).toBe(true); + // Finding-only legacy results do not attest that the scan completed. + expect(run.invocations[0].executionSuccessful).toBe(false); + expect(run.properties.securityScan.status).toBe('unverified'); }); it('should map severity levels correctly', () => { diff --git a/tests/unit/cli/commands/security-evidence.test.ts b/tests/unit/cli/commands/security-evidence.test.ts new file mode 100644 index 000000000..3756861bf --- /dev/null +++ b/tests/unit/cli/commands/security-evidence.test.ts @@ -0,0 +1,238 @@ +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; +import type { CLIContext } from '../../../../src/cli/handlers/interfaces.js'; + +vi.mock('../../../../src/mcp/tools/security-compliance/visual-security.js', () => ({ + VisualSecurityTool: class { + async invoke() { + return { success: true, data: { urlSecurity: { valid: true, issues: [] }, piiExposure: { scanned: true, found: false }, summary: 'URL control' } }; + } + }, +})); + +describe('security command execution evidence', () => { + let createSecurityCommand: typeof import('../../../../src/cli/commands/security.js').createSecurityCommand; + let root: string; + let file: string; + let stdout: string[]; + let runSASTScan: ReturnType; + let runComplianceCheck: ReturnType; + let cleanupAndExit: ReturnType; + const originalRoot = process.env.AQE_PROJECT_ROOT; + const originalBackend = process.env.AQE_MEMORY_BACKEND; + + beforeAll(async () => { + root = mkdtempSync(path.join(tmpdir(), 'aqe-security-cli-')); + process.env.AQE_PROJECT_ROOT = root; + process.env.AQE_MEMORY_BACKEND = 'memory'; + file = path.join(root, 'clean.ts'); + writeFileSync(file, 'export const safe = 1;\n'); + ({ createSecurityCommand } = await import('../../../../src/cli/commands/security.js')); + }); + + afterAll(() => { + if (originalRoot === undefined) delete process.env.AQE_PROJECT_ROOT; + else process.env.AQE_PROJECT_ROOT = originalRoot; + if (originalBackend === undefined) delete process.env.AQE_MEMORY_BACKEND; + else process.env.AQE_MEMORY_BACKEND = originalBackend; + rmSync(root, { recursive: true, force: true }); + }); + + beforeEach(() => { + stdout = []; + vi.spyOn(console, 'log').mockImplementation((...args) => stdout.push(args.join(' '))); + vi.spyOn(console, 'error').mockImplementation(() => {}); + runSASTScan = vi.fn().mockResolvedValue({ success: true, value: completeScan() }); + runComplianceCheck = vi.fn(); + cleanupAndExit = vi.fn(async () => undefined); + }); + + afterEach(() => vi.restoreAllMocks()); + + function evidence(completeness: 'complete' | 'partial' | 'none' = 'complete') { + return { + schemaVersion: 1, completeness, requestedFiles: 1, requestedPaths: [file], duplicateInputs: 0, + files: [{ path: file, status: 'analyzed', sourceDigest: 'a'.repeat(64), readLines: 2, analyzedLines: 2, engineId: 'patterns' }], + engines: [{ id: 'patterns', requested: true, required: true, status: 'completed', scope: 'requested-files', analyzedFiles: 1, ruleIds: ['injection'], ruleCoverage: 'known', errors: [], limitations: [] }], + limitations: completeness === 'complete' ? [] : ['An intended engine did not complete.'], + }; + } + + function completeScan() { + return { vulnerabilities: [], coverage: { filesScanned: 1, linesScanned: 2, rulesApplied: 1 }, evidence: evidence() }; + } + + async function execute(args: string[] = ['--sast', '--format', 'json'], domainAvailable = true) { + const context = { kernel: { getDomainAPIAsync: vi.fn(async () => domainAvailable ? { runSASTScan, runComplianceCheck } : undefined) } } as unknown as CLIContext; + const command = createSecurityCommand(context, cleanupAndExit as unknown as (code: number) => Promise, async () => true); + await command.parseAsync(['--target', root, ...args], { from: 'user' }); + return stdout.join('\n'); + } + + it('returns a nonzero exit and structured failure when SAST fails', async () => { + runSASTScan.mockResolvedValue({ success: false, error: new Error('scanner unavailable') }); + const output = JSON.parse(await execute()); + expect(cleanupAndExit).toHaveBeenCalledExactlyOnceWith(1); + expect(output).toMatchObject({ status: 'failed', checks: [{ name: 'SAST', status: 'failed' }] }); + expect(JSON.stringify(output)).toContain('SAST scan failed.'); + }); + + it.each(['json', 'markdown', 'sarif', 'text'])('does not leak provider exception content into %s', async format => { + runSASTScan.mockRejectedValue(new Error('PRIVATE_SOURCE_AND_CREDENTIAL_SENTINEL')); + const output = await execute(['--sast', '--format', format]); + expect(output).not.toContain('PRIVATE_SOURCE_AND_CREDENTIAL_SENTINEL'); + expect(output).toContain('SAST scan failed.'); + expect(cleanupAndExit).toHaveBeenCalledExactlyOnceWith(1); + }); + + it('retains a bounded errno without exposing the returned error message', async () => { + runSASTScan.mockResolvedValue({ success: false, error: Object.assign(new Error('PRIVATE_ERROR_SENTINEL'), { code: 'EACCES' }) }); + const output = JSON.parse(await execute()); + expect(output.checks[0].reason).toBe('SAST scan failed (EACCES).'); + expect(JSON.stringify(output)).not.toContain('PRIVATE_ERROR_SENTINEL'); + expect(cleanupAndExit).toHaveBeenCalledExactlyOnceWith(1); + }); + + it('actually runs the advertised default SAST scan', async () => { + await execute(['--format', 'json']); + expect(runSASTScan).toHaveBeenCalledOnce(); + expect(cleanupAndExit).toHaveBeenCalledExactlyOnceWith(0); + }); + + it('passes FilePath values to the real domain API contract', async () => { + await execute(); + const files = runSASTScan.mock.calls[0][0]; + expect(files).toHaveLength(1); + expect(files[0].value).toBe(file); + expect(files[0].extension).toBe('ts'); + }); + + it('preserves complete producer evidence and measured coverage in JSON', async () => { + const output = JSON.parse(await execute()); + expect(output).toMatchObject({ status: 'complete', coverage: completeScan().coverage, evidence: evidence() }); + expect(cleanupAndExit).toHaveBeenCalledExactlyOnceWith(0); + }); + + it('does not call a legacy result without receipts verified or clean', async () => { + runSASTScan.mockResolvedValue({ success: true, value: { vulnerabilities: [], coverage: { filesScanned: 999, linesScanned: 9999, rulesApplied: 99 } } }); + const output = JSON.parse(await execute()); + expect(output.status).toBe('unverified'); + expect(output).not.toHaveProperty('coverage'); + expect(cleanupAndExit).toHaveBeenCalledExactlyOnceWith(1); + }); + + it.each(['text', 'markdown'])('does not render legacy counts as analyzed evidence in %s', async format => { + runSASTScan.mockResolvedValue({ success: true, value: { vulnerabilities: [], coverage: { filesScanned: 999, linesScanned: 9999, rulesApplied: 99 } } }); + const output = await execute(['--sast', '--format', format]); + expect(output).toContain('unverified'); + expect(output).not.toContain('999'); + expect(cleanupAndExit).toHaveBeenCalledExactlyOnceWith(1); + }); + + it('retains findings and incomplete evidence from a partial scan', async () => { + const finding = { severity: 'low', type: 'Information exposure', file, line: 1, message: 'Partial evidence finding' }; + runSASTScan.mockResolvedValue({ success: true, value: { ...completeScan(), vulnerabilities: [finding], evidence: evidence('partial') } }); + const output = JSON.parse(await execute()); + expect(output).toMatchObject({ status: 'partial', evidence: { completeness: 'partial' }, vulnerabilities: [finding] }); + expect(cleanupAndExit).toHaveBeenCalledExactlyOnceWith(1); + }); + + it('reports the unimplemented DAST command as not run in JSON', async () => { + const output = JSON.parse(await execute(['--dast', '--format', 'json'])); + expect(output).toMatchObject({ status: 'not-run', checks: [{ name: 'DAST', status: 'not-run' }] }); + expect(runSASTScan).not.toHaveBeenCalled(); + expect(cleanupAndExit).toHaveBeenCalledExactlyOnceWith(1); + }); + + it('keeps an executed clean scan at exit zero', async () => { + await execute(); + expect(cleanupAndExit).toHaveBeenCalledExactlyOnceWith(0); + }); + + it('records an unavailable domain as a structured failed scan', async () => { + const output = JSON.parse(await execute(undefined, false)); + expect(output.status).toBe('failed'); + expect(runSASTScan).not.toHaveBeenCalled(); + expect(cleanupAndExit).toHaveBeenCalledExactlyOnceWith(1); + }); + + it('does not turn failed source discovery into an empty clean scan', async () => { + const output = JSON.parse(await execute(['--sast', '--format', 'json', '--target', path.join(root, 'missing')])); + expect(output).toMatchObject({ status: 'failed', discovery: { status: 'failed' } }); + expect(output.discovery.issues).not.toHaveLength(0); + expect(runSASTScan).not.toHaveBeenCalled(); + expect(cleanupAndExit).toHaveBeenCalledExactlyOnceWith(1); + }); + + it('reports an empty discovered scope as not run', async () => { + const empty = path.join(root, 'empty'); + mkdirSync(empty); + const output = JSON.parse(await execute(['--sast', '--format', 'json', '--target', empty])); + expect(output).toMatchObject({ status: 'not-run', discovery: { status: 'complete' } }); + expect(runSASTScan).not.toHaveBeenCalled(); + expect(cleanupAndExit).toHaveBeenCalledExactlyOnceWith(1); + }); + + it('keeps optional unavailable engine evidence without failing completed required analysis', async () => { + const receipt = evidence(); + const optionalEngine = { id: 'semgrep', requested: true, required: false, status: 'unavailable', scope: 'parent-directory', ruleCoverage: 'unknown', errors: ['NOT_INSTALLED'], limitations: ['Optional engine unavailable'] }; + runSASTScan.mockResolvedValue({ success: true, value: { ...completeScan(), evidence: { ...receipt, engines: [...receipt.engines, optionalEngine] } } }); + const output = JSON.parse(await execute()); + expect(output.status).toBe('complete'); + expect(output.evidence.engines[1]).toEqual(optionalEngine); + expect(cleanupAndExit).toHaveBeenCalledExactlyOnceWith(0); + }); + + it.each([['high', 1], ['medium', 2], ['low', 0]] as const)('preserves the %s severity exit code after a complete scan', async (severity, code) => { + runSASTScan.mockResolvedValue({ success: true, value: { ...completeScan(), vulnerabilities: [{ severity, type: 'Injection', file, line: 1, message: 'Control finding' }] } }); + await execute(); + expect(cleanupAndExit).toHaveBeenCalledExactlyOnceWith(code); + }); + + it('converts actual domain vulnerability locations and messages into SARIF', async () => { + runSASTScan.mockResolvedValue({ success: true, value: { ...completeScan(), vulnerabilities: [{ id: 'finding-1', severity: 'high', title: 'SQL injection', category: 'injection', description: 'Unsanitized query', location: { file, line: 12 } }] } }); + const output = JSON.parse(await execute(['--sast', '--format', 'sarif'])); + expect(output.runs[0].results[0]).toMatchObject({ level: 'error', message: { text: 'Unsanitized query' }, locations: [{ physicalLocation: { artifactLocation: { uri: file }, region: { startLine: 12 } } }] }); + expect(output.runs[0].invocations[0].executionSuccessful).toBe(true); + expect(output.runs[0].properties.securityScan.evidence).toEqual(evidence()); + expect(cleanupAndExit).toHaveBeenCalledExactlyOnceWith(1); + }); + + it('preserves incomplete execution receipts in Markdown', async () => { + runSASTScan.mockResolvedValue({ success: true, value: { ...completeScan(), evidence: evidence('partial') } }); + const output = await execute(['--sast', '--format', 'markdown']); + expect(output).toContain('**Execution:** partial'); + expect(output).toContain('"sourceDigest": "' + 'a'.repeat(64) + '"'); + expect(output).toContain('An intended engine did not complete.'); + expect(cleanupAndExit).toHaveBeenCalledExactlyOnceWith(1); + }); + + it('does not print a clean completion after incomplete SAST', async () => { + runSASTScan.mockResolvedValue({ success: true, value: { ...completeScan(), evidence: evidence('partial') } }); + const output = await execute(['--sast']); + expect(output).toContain('Security analysis: partial'); + expect(output).toContain('requested analysis is incomplete or unverified'); + expect(output).not.toContain('No vulnerabilities found'); + expect(cleanupAndExit).toHaveBeenCalledExactlyOnceWith(1); + }); + + it('retains a later failed compliance framework instead of reporting the first pass as all compliant', async () => { + runComplianceCheck + .mockResolvedValueOnce({ success: true, value: { standardId: 'gdpr', violations: [], passedRules: ['privacy-policy'], skippedRules: [] } }) + .mockResolvedValueOnce({ success: false, error: new Error('SOC2 unavailable') }); + const output = JSON.parse(await execute(['--compliance', 'gdpr,soc2', '--format', 'json'])); + expect(runSASTScan).not.toHaveBeenCalled(); + expect(output).toMatchObject({ status: 'partial', compliance: { compliant: false }, checks: [{ name: 'Compliance:gdpr', status: 'complete' }, { name: 'Compliance:soc2', status: 'failed' }] }); + expect(JSON.stringify(output)).toContain('Compliance check failed.'); + expect(cleanupAndExit).toHaveBeenCalledExactlyOnceWith(1); + }); + + it('keeps the separate URL validation path unchanged', async () => { + const output = JSON.parse(await execute(['--url-validate', 'https://example.test', '--format', 'json'])); + expect(output.summary).toBe('URL control'); + expect(runSASTScan).not.toHaveBeenCalled(); + expect(cleanupAndExit).toHaveBeenCalledExactlyOnceWith(0); + }); +}); diff --git a/tests/unit/cli/security-evidence-output.test.ts b/tests/unit/cli/security-evidence-output.test.ts new file mode 100644 index 000000000..37761fd6e --- /dev/null +++ b/tests/unit/cli/security-evidence-output.test.ts @@ -0,0 +1,32 @@ +import { describe, expect, it } from 'vitest'; +import { securityToMarkdown, toSARIF, type SecurityScanResult } from '../../../src/cli/utils/ci-output.js'; + +describe('security output execution dispositions', () => { + const finding = { severity: 'high', type: 'Injection', file: 'source.ts', line: 4, message: 'Actual finding' }; + + it.each(['partial', 'none', 'failed', 'not-run', 'unverified'] as const)('retains %s in SARIF without discarding findings', status => { + const result: SecurityScanResult = { vulnerabilities: [finding], target: '.', scanType: 'SAST', status, checks: [{ name: 'SAST', status }] }; + const run = JSON.parse(toSARIF(result)).runs[0]; + expect(run.invocations[0].executionSuccessful).toBe(false); + expect(run.invocations[0].toolExecutionNotifications[0].message.text).toContain(status); + expect(run.properties.securityScan).toMatchObject({ status, checks: result.checks }); + expect(run.results[0].message.text).toBe(finding.message); + expect(securityToMarkdown(result)).toContain(`**Execution:** ${status}`); + }); + + it('preserves declared complete execution independently from high-severity findings', () => { + const result: SecurityScanResult = { vulnerabilities: [finding], target: '.', scanType: 'SAST', status: 'complete', checks: [{ name: 'SAST', status: 'complete' }] }; + const run = JSON.parse(toSARIF(result)).runs[0]; + expect(run.invocations[0].executionSuccessful).toBe(true); + expect(run.results[0].level).toBe('error'); + }); + + it('leaves receipt-less legacy reports explicitly unverified', () => { + const result: SecurityScanResult = { vulnerabilities: [], target: '.', scanType: 'SAST', coverage: { filesScanned: 999, linesScanned: 9999, rulesApplied: 99 } }; + const run = JSON.parse(toSARIF(result)).runs[0]; + expect(run.invocations[0].executionSuccessful).toBe(false); + expect(run.properties.securityScan).not.toHaveProperty('coverage'); + expect(securityToMarkdown(result)).toContain('**Execution:** unverified'); + expect(securityToMarkdown(result)).not.toContain('999'); + }); +}); diff --git a/tests/unit/coordination/security-audit-evidence.test.ts b/tests/unit/coordination/security-audit-evidence.test.ts new file mode 100644 index 000000000..b249c951a --- /dev/null +++ b/tests/unit/coordination/security-audit-evidence.test.ts @@ -0,0 +1,130 @@ +import { describe, expect, it, vi } from 'vitest'; +import { SecurityAuditProtocol, type SecurityAuditConfig } from '../../../src/coordination/protocols/security-audit.js'; +import { ok, err, type Result } from '../../../src/shared/types/index.js'; +import type { EventBus, MemoryBackend, AgentCoordinator } from '../../../src/kernel/interfaces.js'; + +import type { DASTResult } from '../../../src/domains/security-compliance/interfaces.js'; + +const summary = { critical: 0, high: 0, medium: 0, low: 0, informational: 0, totalFiles: 1, scanDurationMs: 1 }; +function setup(enableSecretScan = true, config: Partial = {}) { + const memory = { set: vi.fn().mockResolvedValue(undefined) } as unknown as MemoryBackend; + const events = { publish: vi.fn().mockResolvedValue(undefined) } as unknown as EventBus; + const agents = { + canSpawn: () => true, spawn: vi.fn().mockResolvedValue(ok('isolated-agent')), + stop: vi.fn().mockResolvedValue(ok(undefined)), + } as unknown as AgentCoordinator; + const protocol = new SecurityAuditProtocol(events, memory, agents, { + enableSecretScan, enableDAST: false, complianceStandards: [], sendNotifications: false, ...config, + }); + vi.spyOn(protocol, 'scanVulnerabilities').mockResolvedValue(ok({ + scanId: 'fixture', vulnerabilities: [], summary, + coverage: { filesScanned: 1, linesScanned: 1, rulesApplied: 1 }, + evidence: { + schemaVersion: 1, completeness: 'complete', requestedFiles: 1, requestedPaths: ['/fixture.ts'], + duplicateInputs: 0, files: [{ path: '/fixture.ts', engineId: 'patterns', status: 'analyzed', readLines: 1, analyzedLines: 1, sourceDigest: 'a'.repeat(64) }], + engines: [{ id: 'patterns', requested: true, required: true, status: 'completed', scope: 'requested-files', analyzedFiles: 1, ruleIds: ['fixture-rule'], ruleCoverage: 'known', errors: [], limitations: [] }], limitations: [], + }, + })); + vi.spyOn(protocol, 'scanDependencies').mockResolvedValue(ok({ vulnerabilities: [], outdatedPackages: [], summary })); + vi.spyOn(protocol, 'validateCompliance').mockResolvedValue(ok([])); + return { protocol, memory, events }; +} + +describe('security audit execution evidence', () => { + it('does not approve when no audit has run', async () => { + const { protocol } = setup(false); + expect((await protocol.generateReport()).deploymentDecision.allowed).toBe(false); + }); + + it('never reports the unimplemented secret scanner as a measured clean scan', async () => { + const { protocol } = setup(); + const result = await protocol.auditSecrets(); + expect(result.success).toBe(false); + if (!result.success) expect(result.error.message).toMatch(/unavailable|not implemented/i); + }); + + it('does not turn an unavailable secret check into deployment approval', async () => { + const { protocol, memory } = setup(); + const result = await protocol.execute('manual'); + expect(result.success).toBe(true); // Keep usable findings in the audit receipt. + if (!result.success) return; + expect(result.value.deploymentDecision.allowed).toBe(false); + expect(result.value.deploymentDecision.blockingIssues.join(' ')).toMatch(/secret/i); + expect(result.value.recommendations.join(' ')).not.toContain('Security posture is good'); + expect(memory.set).toHaveBeenCalled(); + }); + + it('preserves a verified healthy control when secret checking is not requested', async () => { + const { protocol } = setup(false); + const result = await protocol.execute('manual'); + expect(result.success).toBe(true); + if (result.success) expect(result.value.deploymentDecision.allowed).toBe(true); + }); + + it.each(['partial', 'none'] as const)('does not approve %s SAST coverage', async (completeness) => { + const { protocol } = setup(false); + vi.mocked(protocol.scanVulnerabilities).mockResolvedValue(ok({ + scanId: 'fixture', vulnerabilities: [], summary, + coverage: { filesScanned: 0, linesScanned: 0, rulesApplied: 0 }, + evidence: { schemaVersion: 1, completeness, requestedFiles: 1, requestedPaths: ['/fixture.ts'], duplicateInputs: 0, files: [], engines: [], limitations: [] }, + })); + const result = await protocol.execute('manual'); + expect(result.success).toBe(true); + if (result.success) expect(result.value.deploymentDecision.allowed).toBe(false); + }); + + it.each(['scanDependencies', 'validateCompliance'] as const)('does not ignore a failed %s check', async (method) => { + const { protocol } = setup(false, method === 'validateCompliance' ? { complianceStandards: ['gdpr'] } : {}); + vi.mocked(protocol[method]).mockResolvedValue(err(new Error('fixture failure'))); + const result = await protocol.execute('manual'); + expect(result.success).toBe(true); + if (result.success) expect(result.value.deploymentDecision.allowed).toBe(false); + }); + + it('does not approve requested DAST with no execution receipt', async () => { + const { protocol } = setup(false, { enableDAST: true, targetUrl: 'https://fixture.invalid' }); + vi.spyOn(protocol as unknown as { runDASTScan: () => Promise> }, 'runDASTScan').mockResolvedValue(ok({ scanId: 'fixture', vulnerabilities: [], summary, crawledUrls: [] })); + const result = await protocol.execute('manual'); + expect(result.success).toBe(true); + if (result.success) { + expect(result.value.deploymentDecision.allowed).toBe(false); + expect(result.value.incompleteChecks?.join(' ')).toMatch(/DAST coverage is unverified/); + } + }); + + it.each(['gdpr', 'unknown-standard'])('does not approve placeholder %s compliance', async standard => { + const { protocol } = setup(false, { complianceStandards: [standard] }); + vi.mocked(protocol.validateCompliance).mockRestore(); + const result = await protocol.execute('manual'); + expect(result.success).toBe(true); + if (result.success) { + expect(result.value.complianceReports).toHaveLength(1); + expect(result.value.deploymentDecision.allowed).toBe(false); + expect(result.value.incompleteChecks?.join(' ')).toMatch(/compliance.*verified/); + } + }); + + it('honors the pre-release trigger requiring secrets even when disabled for daily scans', async () => { + const { protocol } = setup(false); + const result = await protocol.execute('pre-release'); + expect(result.success).toBe(true); + if (result.success) expect(result.value.deploymentDecision.blockingIssues.join(' ')).toMatch(/Secret scan/); + }); + + it('does not require unrequested SAST, DAST, or secret checks on dependency-update', async () => { + const { protocol } = setup(true, { enableDAST: true }); + const result = await protocol.execute('dependency-update'); + expect(protocol.scanVulnerabilities).not.toHaveBeenCalled(); + expect(protocol.validateCompliance).not.toHaveBeenCalled(); + expect(result.success).toBe(true); + if (result.success) expect(result.value.deploymentDecision.allowed).toBe(true); + }); + + it('retains failed SAST as incomplete rather than fabricating zero-risk approval', async () => { + const { protocol } = setup(false); + vi.mocked(protocol.scanVulnerabilities).mockResolvedValue(err(new Error('fixture engine unavailable'))); + const result = await protocol.execute('manual'); + expect(result.success).toBe(true); + if (result.success) expect(result.value.deploymentDecision.allowed).toBe(false); + }); +}); diff --git a/tests/unit/domains/security-compliance/scanners/scan-evidence.test.ts b/tests/unit/domains/security-compliance/scanners/scan-evidence.test.ts new file mode 100644 index 000000000..21da51101 --- /dev/null +++ b/tests/unit/domains/security-compliance/scanners/scan-evidence.test.ts @@ -0,0 +1,183 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { mkdtemp, rm, writeFile } from 'node:fs/promises'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; +import { createHash } from 'node:crypto'; +import { FilePath } from '../../../../../src/shared/value-objects/index.js'; +import { SASTScanner } from '../../../../../src/domains/security-compliance/services/scanners/sast-scanner.js'; +import { DEFAULT_CONFIG } from '../../../../../src/domains/security-compliance/services/scanners/scanner-types.js'; +import { ALL_SECURITY_PATTERNS, BUILT_IN_RULE_SETS } from '../../../../../src/domains/security-compliance/services/scanners/security-patterns.js'; +import type { MemoryBackend } from '../../../../../src/kernel/interfaces.js'; +import { isSemgrepAvailable, runSemgrepWithRules } from '../../../../../src/domains/security-compliance/services/semgrep-integration.js'; + +vi.mock('../../../../../src/domains/security-compliance/services/semgrep-integration.js', async (importOriginal) => ({ + ...await importOriginal(), + isSemgrepAvailable: vi.fn(), + runSemgrepWithRules: vi.fn(), +})); + +describe('SAST execution evidence', () => { + let fixture: string; + beforeEach(async () => { + vi.clearAllMocks(); + fixture = await mkdtemp(join(tmpdir(), 'aqe-scan-evidence-')); + vi.mocked(isSemgrepAvailable).mockResolvedValue(false); + vi.mocked(runSemgrepWithRules).mockResolvedValue({ success: true, status: 'completed', findings: [], errors: [] }); + }); + afterEach(async () => { await rm(fixture, { recursive: true, force: true }); }); + + const scanner = (enableSemgrep = false) => new SASTScanner( + { ...DEFAULT_CONFIG, enableLLMAnalysis: false, enableSemgrep }, + { set: vi.fn().mockResolvedValue(undefined) } as unknown as MemoryBackend, + ); + const scan = async (paths: string[], enableSemgrep = false) => { + const result = await scanner(enableSemgrep).scanWithRules(paths.map(FilePath.create), ['owasp-top-10']); + expect(result.success).toBe(true); + if (!result.success) throw result.error; + return result.value; + }; + + it('does not credit an unreadable requested file as analyzed coverage', async () => { + const result = await scan([join(fixture, 'missing.ts')]); + expect(result.coverage).toMatchObject({ filesScanned: 0, linesScanned: 0, rulesApplied: 0 }); + expect(result.summary.totalFiles).toBe(0); + expect(result.evidence).toMatchObject({ completeness: 'none', requestedFiles: 1, files: [{ status: 'unreadable', readLines: 0, analyzedLines: 0 }] }); + }); + + it('records readable unsupported input without analysis credit', async () => { + const file = join(fixture, 'schema.sql'); + await writeFile(file, 'SELECT * FROM users;\n'); + const result = await scan([file]); + expect(result.coverage).toMatchObject({ filesScanned: 0, linesScanned: 0, rulesApplied: 0 }); + expect(result.evidence?.files[0]).toMatchObject({ status: 'unsupported', readLines: 2, analyzedLines: 0 }); + }); + + it('distinguishes unavailable Semgrep from completed pattern analysis', async () => { + const file = join(fixture, 'clean.ts'); + await writeFile(file, 'const safe = true;\n'); + const result = await scan([file], true); + expect(result.evidence).toMatchObject({ completeness: 'complete' }); + expect(result.evidence?.engines).toEqual(expect.arrayContaining([ + expect.objectContaining({ id: 'patterns', status: 'completed', analyzedFiles: 1 }), + expect.objectContaining({ id: 'semgrep', requested: true, status: 'unavailable' }), + ])); + }); + + it('binds successful clean pattern analysis to exact source and executed rules', async () => { + const file = join(fixture, 'clean.ts'); + const content = 'const safe = true;\n'; + await writeFile(file, content); + const result = await scan([file]); + const categories = new Set(BUILT_IN_RULE_SETS.find(rule => rule.id === 'owasp-top-10')!.categories); + const ruleIds = ALL_SECURITY_PATTERNS.filter(pattern => categories.has(pattern.category)).map(pattern => pattern.id); + expect(result.coverage.rulesApplied).toBe(new Set(ruleIds).size); + expect(result.evidence).toMatchObject({ completeness: 'complete' }); + expect(result.evidence?.files[0]).toMatchObject({ path: file, status: 'analyzed', sourceDigest: createHash('sha256').update(content).digest('hex'), readLines: 2, analyzedLines: 2 }); + expect(result.evidence?.engines).toEqual(expect.arrayContaining([expect.objectContaining({ id: 'semgrep', requested: false, status: 'disabled' })])); + }); + + it('retains a finding from a completed file alongside an unreadable input', async () => { + const file = join(fixture, 'vulnerable.ts'); + await writeFile(file, 'db.query("SELECT * FROM users WHERE id = " + userId + "");'); + const result = await scan([file, join(fixture, 'missing.ts')]); + expect(result.vulnerabilities.length).toBeGreaterThan(0); + expect(result.coverage.filesScanned).toBe(1); + expect(result.evidence).toMatchObject({ completeness: 'partial', requestedFiles: 2 }); + expect(result.evidence?.files.map(receipt => receipt.status)).toEqual(['analyzed', 'unreadable']); + }); + + it('deduplicates normalized paths without doubling findings or coverage', async () => { + const file = join(fixture, 'vulnerable.ts'); + await writeFile(file, 'document.write(userInput);'); + const result = await scan([file, join(fixture, 'subdir', '..', 'vulnerable.ts')]); + const single = await scan([file]); + expect(result.coverage).toEqual(single.coverage); + expect(result.vulnerabilities).toHaveLength(single.vulnerabilities.length); + expect(result.evidence).toMatchObject({ requestedFiles: 1, requestedPaths: [file], duplicateInputs: 1 }); + }); + + it('records a changed source digest when the same path changes', async () => { + const file = join(fixture, 'changed.ts'); + await writeFile(file, 'const value = 1;'); + const first = await scan([file]); + await writeFile(file, 'const value = 2;'); + const second = await scan([file]); + expect(first.evidence?.files[0].sourceDigest).not.toBe(second.evidence?.files[0].sourceDigest); + expect(first.evidence?.engines[0].rulesetDigest).toBe(second.evidence?.engines[0].rulesetDigest); + }); + + it('does not infer requested-file coverage from successful directory scanning', async () => { + vi.mocked(isSemgrepAvailable).mockResolvedValue(true); + const file = join(fixture, 'clean.ts'); + await writeFile(file, 'const safe = true;'); + const result = await scan([file], true); + const semgrep = result.evidence?.engines.find(engine => engine.id === 'semgrep'); + expect(semgrep).toMatchObject({ status: 'completed', scope: 'parent-directory', required: false, ruleCoverage: 'unknown' }); + expect(semgrep?.analyzedFiles).toBeUndefined(); + expect(semgrep?.ruleIds).toBeUndefined(); + expect(semgrep?.limitations.length).toBeGreaterThan(0); + expect(result.coverage.filesScanned).toBe(1); + }); + + it('retains partial Semgrep findings without counting findings as executed rules', async () => { + vi.mocked(isSemgrepAvailable).mockResolvedValue(true); + const file = join(fixture, 'clean.ts'); + await writeFile(file, 'const safe = true;'); + vi.mocked(runSemgrepWithRules).mockResolvedValue({ + success: false, status: 'partial', errors: ['fixture engine failure'], + findings: [{ check_id: 'fixture.security', path: file, start: { line: 1, col: 1 }, end: { line: 1, col: 2 }, + extra: { message: 'Fixture finding', severity: 'WARNING', lines: '' } }], + }); + const result = await scan([file], true); + const control = await scan([file]); + expect(result.vulnerabilities).toHaveLength(1); + expect(result.coverage.rulesApplied).toBe(control.coverage.rulesApplied); + expect(result.evidence?.engines.find(engine => engine.id === 'semgrep')).toMatchObject({ status: 'partial', required: false }); + expect(result.evidence?.engines.find(engine => engine.id === 'semgrep')?.errors).not.toContain('fixture engine failure'); + }); + + it('preserves pattern findings after a thrown optional engine error', async () => { + vi.mocked(isSemgrepAvailable).mockResolvedValue(true); + vi.mocked(runSemgrepWithRules).mockRejectedValue(new Error('private source contents')); + const file = join(fixture, 'vulnerable.ts'); + await writeFile(file, 'document.write(userInput);'); + const result = await scan([file], true); + expect(result.vulnerabilities.length).toBeGreaterThan(0); + expect(result.evidence?.engines.find(engine => engine.id === 'semgrep')).toMatchObject({ status: 'failed', errors: ['Semgrep execution failed.'] }); + expect(JSON.stringify(result.evidence)).not.toContain('private source contents'); + }); + + it('keeps a shared parent directory instead of widening sibling-file scope', async () => { + vi.mocked(isSemgrepAvailable).mockResolvedValue(true); + const first = join(fixture, 'first.ts'); + const second = join(fixture, 'second.ts'); + await writeFile(first, 'const first = 1;'); + await writeFile(second, 'const second = 2;'); + await scan([first, second], true); + expect(runSemgrepWithRules).toHaveBeenCalledWith(fixture, ['owasp-top-10']); + }); + + + it('does not turn a legacy finding-only adapter result into completed engine evidence', async () => { + vi.mocked(isSemgrepAvailable).mockResolvedValue(true); + vi.mocked(runSemgrepWithRules).mockResolvedValue({ success: true, findings: [], errors: [] }); + const file = join(fixture, 'clean.ts'); + await writeFile(file, 'const safe = true;'); + const result = await scan([file], true); + expect(result.evidence?.engines.find(engine => engine.id === 'semgrep')).toMatchObject({ status: 'unverified' }); + expect(result.evidence?.engines.find(engine => engine.id === 'semgrep')?.limitations).toContain('Legacy Semgrep adapter returned no execution disposition.'); + }); + + + it.each([['unknown'], ['owasp-top-10', 'unknown']])('rejects unknown requested rule sets without claiming a running scan: %j', async (...rules) => { + const active = new Map(); + const instance = new SASTScanner({ ...DEFAULT_CONFIG, enableSemgrep: true }, + { set: vi.fn().mockResolvedValue(undefined) } as unknown as MemoryBackend, undefined, active); + const result = await instance.scanWithRules([FilePath.create(join(fixture, 'unused.ts'))], rules); + expect(result.success).toBe(false); + expect(active.size).toBe(0); + expect(isSemgrepAvailable).not.toHaveBeenCalled(); + expect(runSemgrepWithRules).not.toHaveBeenCalled(); + }); + +}); diff --git a/tests/unit/domains/security-compliance/security-discovery.test.ts b/tests/unit/domains/security-compliance/security-discovery.test.ts new file mode 100644 index 000000000..81c8db8ac --- /dev/null +++ b/tests/unit/domains/security-compliance/security-discovery.test.ts @@ -0,0 +1,75 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import * as fs from 'node:fs/promises'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { discoverSecurityFiles } from '../../../../src/domains/security-compliance/scan-discovery.js'; + +vi.mock('node:fs/promises', async (importOriginal) => ({ + ...await importOriginal(), +})); + +describe('security discovery evidence', () => { + let root: string; + beforeEach(async () => { root = await fs.mkdtemp(path.join(os.tmpdir(), 'aqe-security-discovery-')); }); + afterEach(async () => { vi.restoreAllMocks(); await fs.rm(root, { recursive: true, force: true }); }); + + it('reports a missing target as failed discovery, not a clean empty scan', async () => { + const result = await discoverSecurityFiles(path.join(root, 'missing')); + expect(result.files).toEqual([]); + expect(result.discovery).toMatchObject({ status: 'failed', issues: [{ reason: 'ENOENT' }] }); + }); + + it('retains an explicit unsupported file for scanner classification', async () => { + const file = path.join(root, 'README.txt'); + await fs.writeFile(file, 'No language-specific analysis is available.'); + expect(await discoverSecurityFiles(file)).toMatchObject({ files: [file], discovery: { status: 'complete' } }); + }); + + it('distinguishes a complete empty inventory from failed discovery', async () => { + expect(await discoverSecurityFiles(root)).toMatchObject({ files: [], discovery: { status: 'complete', issues: [] } }); + }); + + it('preserves discovered siblings when a subtree is unreadable without exposing raw errors', async () => { + await fs.mkdir(path.join(root, 'denied')); + await fs.writeFile(path.join(root, 'safe.ts'), 'export const safe = true;'); + const original = fs.readdir; + vi.spyOn(fs, 'readdir').mockImplementation(async (...args: Parameters) => { + if (String(args[0]) === path.join(root, 'denied')) { + throw Object.assign(new Error('credential-shaped-value-that-must-not-escape'), { code: 'EACCES' }); + } + return original(...args); + }); + const result = await discoverSecurityFiles(root); + expect(result.files).toEqual([path.join(root, 'safe.ts')]); + expect(result.discovery).toMatchObject({ status: 'partial', issues: [{ path: path.join(root, 'denied'), reason: 'EACCES' }] }); + expect(JSON.stringify(result)).not.toContain('credential-shaped'); + }); + + it('marks a file limit as partial instead of claiming a complete manifest', async () => { + for (const name of ['c.ts', 'a.py', 'b.js']) await fs.writeFile(path.join(root, name), 'fixture'); + const result = await discoverSecurityFiles(root, { maxFiles: 2 }); + expect(result.files).toEqual([path.join(root, 'a.py'), path.join(root, 'b.js')]); + expect(result.discovery).toMatchObject({ status: 'partial', issues: [{ reason: 'FILE_LIMIT' }] }); + }); + + it('marks an omitted deep subtree as partial', async () => { + await fs.mkdir(path.join(root, 'deep', 'deeper'), { recursive: true }); + await fs.writeFile(path.join(root, 'deep', 'deeper', 'hidden.ts'), 'fixture'); + const result = await discoverSecurityFiles(root, { maxDepth: 1 }); + expect(result.discovery).toMatchObject({ status: 'partial', issues: [{ reason: 'DEPTH_LIMIT' }] }); + }); + + it('reports policy exclusions and does not traverse observed symbolic links', async () => { + await fs.mkdir(path.join(root, 'node_modules')); + await fs.writeFile(path.join(root, 'node_modules', 'dependency.ts'), 'fixture'); + await fs.writeFile(path.join(root, 'source.ts'), 'fixture'); + await fs.symlink(path.join(root, 'source.ts'), path.join(root, 'alias.ts')); + const result = await discoverSecurityFiles(root); + expect(result.files).toEqual([path.join(root, 'source.ts')]); + expect(result.discovery.status).toBe('complete'); + expect(result.discovery.excludedPaths).toEqual([path.join(root, 'alias.ts'), path.join(root, 'node_modules')]); + expect(await discoverSecurityFiles(path.join(root, 'alias.ts'))).toMatchObject({ + files: [], discovery: { status: 'failed', issues: [{ reason: 'SYMLINK_TARGET' }] }, + }); + }); +}); diff --git a/tests/unit/domains/security-compliance/security-scanner.test.ts b/tests/unit/domains/security-compliance/security-scanner.test.ts index 8aacc19d9..9348e0630 100644 --- a/tests/unit/domains/security-compliance/security-scanner.test.ts +++ b/tests/unit/domains/security-compliance/security-scanner.test.ts @@ -3,6 +3,9 @@ * Tests for SAST/DAST scanning and vulnerability detection */ +import { mkdtemp, rm, writeFile } from 'node:fs/promises'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; import { describe, it, expect, beforeEach, afterEach, vi, type Mock } from 'vitest'; import { SecurityScannerService } from '../../../../src/domains/security-compliance/services/security-scanner'; import type { MemoryBackend } from '../../../../src/kernel/interfaces'; @@ -37,22 +40,26 @@ const createMockFilePath = (path: string): FilePath => ({ describe('SecurityScannerService', () => { let service: SecurityScannerService; let mockMemory: MemoryBackend; + let fixture: string; - beforeEach(() => { + beforeEach(async () => { + fixture = await mkdtemp(join(tmpdir(), 'aqe-scanner-service-')); + await Promise.all(['app.ts', 'utils.ts'].map(name => writeFile(join(fixture, name), 'const safe = true;\n'))); mockMemory = createMockMemoryBackend(); - service = new SecurityScannerService(mockMemory); + service = new SecurityScannerService(mockMemory, { enableSemgrep: false, enableLLMAnalysis: false }); }); - afterEach(() => { + afterEach(async () => { vi.restoreAllMocks(); + await rm(fixture, { recursive: true, force: true }); }); describe('SAST Scanning', () => { describe('scanFiles', () => { it('should scan files and return SAST results', async () => { const files = [ - createMockFilePath('/src/app.ts'), - createMockFilePath('/src/utils.ts'), + createMockFilePath(join(fixture, 'app.ts')), + createMockFilePath(join(fixture, 'utils.ts')), ]; const result = await service.scanFiles(files); @@ -76,7 +83,7 @@ describe('SecurityScannerService', () => { }); it('should use default rule sets when scanning', async () => { - const files = [createMockFilePath('/src/app.ts')]; + const files = [createMockFilePath(join(fixture, 'app.ts'))]; const result = await service.scanFiles(files); @@ -87,7 +94,7 @@ describe('SecurityScannerService', () => { }); it('should store scan results in memory', async () => { - const files = [createMockFilePath('/src/app.ts')]; + const files = [createMockFilePath(join(fixture, 'app.ts'))]; await service.scanFiles(files); @@ -99,7 +106,7 @@ describe('SecurityScannerService', () => { describe('scanWithRules', () => { it('should scan with specific rule sets', async () => { - const files = [createMockFilePath('/src/app.ts')]; + const files = [createMockFilePath(join(fixture, 'app.ts'))]; const result = await service.scanWithRules(files, ['owasp-top-10']); @@ -110,7 +117,7 @@ describe('SecurityScannerService', () => { }); it('should return error for invalid rule sets', async () => { - const files = [createMockFilePath('/src/app.ts')]; + const files = [createMockFilePath(join(fixture, 'app.ts'))]; const result = await service.scanWithRules(files, ['invalid-ruleset']); @@ -121,14 +128,19 @@ describe('SecurityScannerService', () => { }); it('should combine multiple rule sets', async () => { - const files = [createMockFilePath('/src/app.ts')]; + const files = [createMockFilePath(join(fixture, 'app.ts'))]; const result = await service.scanWithRules(files, ['owasp-top-10', 'cwe-sans-25']); expect(result.success).toBe(true); if (result.success) { - // Combined rules from both sets - expect(result.value.coverage.rulesApplied).toBeGreaterThan(40); + // Overlapping rule sets execute each actual pattern once. + const singleSet = await service.scanWithRules(files, ['owasp-top-10']); + expect(singleSet.success).toBe(true); + if (!singleSet.success) throw singleSet.error; + expect(result.value.coverage.rulesApplied).toBe(singleSet.value.coverage.rulesApplied); + expect(result.value.coverage.rulesApplied).toBeGreaterThan(0); + expect(result.value.evidence?.engines[0].ruleIds).toEqual(expect.arrayContaining(['sqli-string-concat', 'xss-innerhtml'])); } }); }); @@ -344,7 +356,7 @@ describe('SecurityScannerService', () => { }); it('should return correct status for active scan', async () => { - const files = [createMockFilePath('/src/app.ts')]; + const files = [createMockFilePath(join(fixture, 'app.ts'))]; const scanResult = await service.scanFiles(files); if (scanResult.success) { @@ -358,7 +370,7 @@ describe('SecurityScannerService', () => { describe('Full Scan', () => { describe('runFullScan', () => { it('should run SAST scan only when no URL provided', async () => { - const files = [createMockFilePath('/src/app.ts')]; + const files = [createMockFilePath(join(fixture, 'app.ts'))]; const result = await service.runFullScan(files); @@ -371,7 +383,7 @@ describe('SecurityScannerService', () => { }); it('should run both SAST and DAST when URL provided', async () => { - const files = [createMockFilePath('/src/app.ts')]; + const files = [createMockFilePath(join(fixture, 'app.ts'))]; const targetUrl = 'https://example.com'; const result = await service.runFullScan(files, targetUrl); @@ -384,7 +396,7 @@ describe('SecurityScannerService', () => { }); it('should combine summaries from SAST and DAST', async () => { - const files = [createMockFilePath('/src/app.ts')]; + const files = [createMockFilePath(join(fixture, 'app.ts'))]; const targetUrl = 'https://example.com'; const result = await service.runFullScan(files, targetUrl); @@ -401,7 +413,7 @@ describe('SecurityScannerService', () => { }); it('should not fail full scan if DAST fails', async () => { - const files = [createMockFilePath('/src/app.ts')]; + const files = [createMockFilePath(join(fixture, 'app.ts'))]; // Full scan should complete even if DAST portion has issues const result = await service.runFullScan(files, 'https://example.com'); diff --git a/tests/unit/domains/security-compliance/semgrep-evidence.test.ts b/tests/unit/domains/security-compliance/semgrep-evidence.test.ts new file mode 100644 index 000000000..39ed5e8a6 --- /dev/null +++ b/tests/unit/domains/security-compliance/semgrep-evidence.test.ts @@ -0,0 +1,144 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { runSemgrep, convertSemgrepFindings } from '../../../../src/domains/security-compliance/services/semgrep-integration.js'; + +const boundary = vi.hoisted(() => ({ execute: vi.fn() })); +vi.mock('child_process', () => ({ + execFile: Object.assign(vi.fn(), { [Symbol.for('nodejs.util.promisify.custom')]: boundary.execute }), +})); + +describe('Semgrep execution outcome evidence', () => { + beforeEach(() => { + boundary.execute.mockReset(); + boundary.execute.mockImplementation(async (_command: string, args: string[]) => { + if (args[0] === '--version') return { stdout: '1.99.0\n', stderr: '' }; + return { stdout: JSON.stringify({ results: [], errors: [] }), stderr: '' }; + }); + }); + + const output = (stdout: string) => boundary.execute.mockImplementation(async (_command: string, args: string[]) => ( + args[0] === '--version' ? { stdout: '1.99.0\n', stderr: '' } : { stdout, stderr: '' } + )); + + it('rejects syntactically valid output without a findings collection', async () => { + output('{}'); + const result = await runSemgrep({ target: '/isolated-fixture' }); + expect(result.success).toBe(false); + expect(result.status).toBe('failed'); + }); + + it('does not call a scan complete when Semgrep reports execution errors', async () => { + output(JSON.stringify({ results: [], errors: [{ message: 'fixture parse failure' }] })); + const result = await runSemgrep({ target: '/isolated-fixture' }); + expect(result.success).toBe(false); + expect(result.status).toBe('partial'); + expect(result.errors).toHaveLength(1); + }); + + it('preserves a nonzero execution failure even with parseable output', async () => { + boundary.execute.mockImplementation(async (_command: string, args: string[]) => { + if (args[0] === '--version') return { stdout: '1.99.0\n', stderr: '' }; + throw Object.assign(new Error('fixture process failed'), { code: 2, stdout: JSON.stringify({ results: [], errors: [] }) }); + }); + const result = await runSemgrep({ target: '/isolated-fixture' }); + expect(result.success).toBe(false); + expect(result.status).toBe('failed'); + }); + + it('records a legitimate zero-finding run as completed', async () => { + const result = await runSemgrep({ target: '/isolated-fixture' }); + expect(result).toMatchObject({ success: true, status: 'completed', findings: [], errors: [], version: '1.99.0' }); + }); + + it('reports absence separately from malformed output or clean execution', async () => { + boundary.execute.mockRejectedValue(Object.assign(new Error('not installed'), { code: 'ENOENT' })); + const result = await runSemgrep({ target: '/isolated-fixture' }); + expect(result).toMatchObject({ success: false, status: 'unavailable', findings: [] }); + expect(boundary.execute).toHaveBeenCalledTimes(1); + }); + + it.each(['not json', 'null', '[]', '{"results":{}}'])('rejects malformed output %s', async (stdout) => { + output(stdout); + const result = await runSemgrep({ target: '/isolated-fixture' }); + expect(result).toMatchObject({ success: false, status: 'failed', findings: [] }); + }); + + it('retains valid findings when other results or engine work fail', async () => { + output(JSON.stringify({ results: [ + { check_id: 'fixture.rule', path: '/isolated-fixture/a.ts', start: { line: 1, col: 1 }, extra: { message: 'Fixture finding' } }, + { check_id: 'missing.path' }, + ], errors: [{ message: 'private source contents' }] })); + const result = await runSemgrep({ target: '/isolated-fixture' }); + expect(result).toMatchObject({ success: false, status: 'partial' }); + expect(result.findings).toHaveLength(1); + expect(result.findings[0].check_id).toBe('fixture.rule'); + expect(result.errors.join(' ')).not.toContain('private source contents'); + }); + + it('retains findings but rejects a killed process as completed analysis', async () => { + boundary.execute.mockImplementation(async (_command: string, args: string[]) => { + if (args[0] === '--version') return { stdout: '1.99.0\n', stderr: '' }; + throw Object.assign(new Error('timeout'), { code: 1, killed: true, + stdout: JSON.stringify({ results: [{ check_id: 'fixture.rule', path: '/isolated-fixture/a.ts' }], errors: [] }) }); + }); + const result = await runSemgrep({ target: '/isolated-fixture' }); + expect(result).toMatchObject({ success: false, status: 'failed' }); + expect(result.findings).toHaveLength(1); + }); + + it('preserves an exit-one findings result without treating exit two as equivalent', async () => { + boundary.execute.mockImplementation(async (_command: string, args: string[]) => { + if (args[0] === '--version') return { stdout: '1.99.0\n', stderr: '' }; + throw Object.assign(new Error('findings'), { code: 1, + stdout: JSON.stringify({ results: [{ check_id: 'fixture.rule', path: '/isolated-fixture/a.ts' }], errors: [] }) }); + }); + expect(await runSemgrep({ target: '/isolated-fixture' })).toMatchObject({ success: true, status: 'completed' }); + }); + + + it.each([ + { check_id: '' }, + { start: { line: 'not-a-number' } }, + { start: { line: 0 } }, + { extra: { message: {} } }, + { extra: { metadata: { owasp: [123] } } }, + { extra: { metadata: { references: 'not-an-array' } } }, + { metadata: { category: 123 } }, + { extra: { severity: 'NOT_A_SEVERITY' } }, + ])('isolates malformed finding fields without losing valid findings: %j', async (malformed) => { + output(JSON.stringify({ results: [ + { check_id: 'valid.rule', path: '/isolated-fixture/a.ts', extra: { message: 'Valid finding' } }, + { check_id: 'bad.rule', path: '/isolated-fixture/b.ts', ...malformed }, + ], errors: [] })); + const result = await runSemgrep({ target: '/isolated-fixture' }); + expect(result).toMatchObject({ success: false, status: 'partial' }); + expect(result.findings).toHaveLength(1); + expect(result.findings[0].check_id).toBe('valid.rule'); + expect(() => convertSemgrepFindings(result.findings)).not.toThrow(); + }); + + + it('keeps verbose diagnostics separate from successful analysis status', async () => { + boundary.execute.mockImplementation(async (_command: string, args: string[]) => { + if (args[0] === '--version') return { stdout: '1.99.0\n', stderr: '' }; + return { stdout: JSON.stringify({ results: [], errors: [] }), stderr: 'private diagnostic contents' }; + }); + const result = await runSemgrep({ target: '/isolated-fixture', verbose: true }); + expect(result).toMatchObject({ success: true, status: 'completed', errors: [], + diagnostics: ['Semgrep emitted diagnostic output.'] }); + expect(JSON.stringify(result)).not.toContain('private diagnostic contents'); + }); + + + it.each([ + ['ERROR', 'high'], ['WARNING', 'medium'], ['INFO', 'low'], + ['CRITICAL', 'critical'], ['HIGH', 'high'], ['MEDIUM', 'medium'], ['LOW', 'low'], + ['EXPERIMENT', 'low'], ['INVENTORY', 'low'], + ])('preserves supported Semgrep severity %s as %s', async (severity, expected) => { + output(JSON.stringify({ results: [{ check_id: 'valid.rule', path: '/isolated-fixture/a.ts', + extra: { severity, message: 'A supported severity' } }], errors: [] })); + const result = await runSemgrep({ target: '/isolated-fixture' }); + expect(result).toMatchObject({ success: true, status: 'completed' }); + expect(convertSemgrepFindings(result.findings)[0].severity).toBe(expected); + }); + +}); From c04870091b1ac58a0ed345da7f6e2b8a4d056f1b Mon Sep 17 00:00:00 2001 From: Rudy Celekli Date: Mon, 21 Sep 2026 01:46:07 -0400 Subject: [PATCH 2/5] fix(ci): skip report comments for fork pull requests (cherry picked from commit 0686b7cf5185f6f855c33f629bcdcbaf2c7fb832) --- .github/workflows/mcp-tools-test.yml | 3 +- .github/workflows/optimized-ci.yml | 3 +- .github/workflows/skill-validation.yml | 2 + tests/unit/scripts/fork-pr-comments.test.ts | 49 +++++++++++++++++++++ 4 files changed, 55 insertions(+), 2 deletions(-) create mode 100644 tests/unit/scripts/fork-pr-comments.test.ts diff --git a/.github/workflows/mcp-tools-test.yml b/.github/workflows/mcp-tools-test.yml index a0a409e7d..64c0abced 100644 --- a/.github/workflows/mcp-tools-test.yml +++ b/.github/workflows/mcp-tools-test.yml @@ -229,7 +229,8 @@ jobs: - name: Create summary comment uses: actions/github-script@v9 - if: github.event_name == 'pull_request' + # Fork tokens cannot write PR comments; keep reports and test gates running. + if: github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository with: script: | const fs = require('fs'); diff --git a/.github/workflows/optimized-ci.yml b/.github/workflows/optimized-ci.yml index ab346fb95..ffd0d52bd 100644 --- a/.github/workflows/optimized-ci.yml +++ b/.github/workflows/optimized-ci.yml @@ -412,7 +412,8 @@ jobs: path: ci-metrics.md retention-days: 30 - name: Comment on PR - if: github.event_name == 'pull_request' + # Fork tokens cannot write PR comments; keep reports and test gates running. + if: github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository uses: actions/github-script@v9 with: script: | diff --git a/.github/workflows/skill-validation.yml b/.github/workflows/skill-validation.yml index 8b3c9de2e..d3cad5fb2 100644 --- a/.github/workflows/skill-validation.yml +++ b/.github/workflows/skill-validation.yml @@ -451,6 +451,8 @@ jobs: cat report.md - name: Comment on PR + # Fork tokens cannot write PR comments; keep the Tier 3 gate running. + if: github.event.pull_request.head.repo.full_name == github.repository uses: actions/github-script@v9 with: script: | diff --git a/tests/unit/scripts/fork-pr-comments.test.ts b/tests/unit/scripts/fork-pr-comments.test.ts new file mode 100644 index 000000000..f6b620c7e --- /dev/null +++ b/tests/unit/scripts/fork-pr-comments.test.ts @@ -0,0 +1,49 @@ +import { readFileSync } from 'node:fs'; +import { resolve } from 'node:path'; +import { runInNewContext } from 'node:vm'; +import { parse } from 'yaml'; +import { describe, expect, it } from 'vitest'; + +type Step = { name?: string; if?: string }; +type Job = { if?: string; steps: Step[] }; +const root = resolve(import.meta.dirname, '../../..'); +const workflows = [ + ['optimized-ci.yml', 'dashboard', 'Comment on PR'], + ['mcp-tools-test.yml', 'mcp-summary', 'Create summary comment'], + ['skill-validation.yml', 'report', 'Comment on PR'], +] as const; + +// These workflow guards use comparisons and boolean operators shared by +// JavaScript and Actions expressions. Exercise the actual YAML conditions. +function allowed(condition: string | undefined, event: string, headRepo?: string): boolean { + if (!condition) return true; + return Boolean(runInNewContext(condition, { + always: () => true, + github: { + event_name: event, + repository: 'upstream/agentic-qe', + event: headRepo ? { pull_request: { head: { repo: { full_name: headRepo } } } } : {}, + }, + })); +} + +describe.each(workflows)('%s PR reporting permissions', (file, jobName, stepName) => { + const workflow = parse(readFileSync(resolve(root, '.github/workflows', file), 'utf8')); + const job: Job = workflow.jobs[jobName]; + const comment = job.steps.find((step) => step.name === stepName)!; + + it('keeps the reporting job available but skips writes for fork PRs', () => { + expect(comment).toBeDefined(); + expect(allowed(job.if, 'pull_request', 'contributor/agentic-qe')).toBe(true); + expect(allowed(comment.if, 'pull_request', 'contributor/agentic-qe')).toBe(false); + }); + + it('retains comments for same-repository PRs', () => { + expect(allowed(job.if, 'pull_request', 'upstream/agentic-qe')).toBe(true); + expect(allowed(comment.if, 'pull_request', 'upstream/agentic-qe')).toBe(true); + }); + + it.each(['push', 'workflow_dispatch'])('does not post a PR comment on %s', (event) => { + expect(allowed(job.if, event) && allowed(comment.if, event)).toBe(false); + }); +}); From ad03c60a446cdeae1a3bd9191613b440fd2df26f Mon Sep 17 00:00:00 2001 From: Rudy Celekli Date: Mon, 21 Sep 2026 17:53:47 -0400 Subject: [PATCH 3/5] chore(security): keep receipt imports separate from quality gate changes --- src/mcp/handlers/domain-handler-configs.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/mcp/handlers/domain-handler-configs.ts b/src/mcp/handlers/domain-handler-configs.ts index a01c289fb..2385fa6c0 100644 --- a/src/mcp/handlers/domain-handler-configs.ts +++ b/src/mcp/handlers/domain-handler-configs.ts @@ -29,6 +29,9 @@ import { ChaosTestParams, } from '../types'; import { MetricsCollector } from '../metrics'; +import type { SecurityScanEvidence } from '../../domains/security-compliance/scan-evidence.js'; +import type { SecurityCoverage } from '../../domains/security-compliance/interfaces.js'; + import { DEFAULT_FRAMEWORKS, FRAMEWORK_TO_LANGUAGE, @@ -41,9 +44,6 @@ import { type RiskDecision, } from '../../contracts/verdicts.js'; -import type { SecurityScanEvidence } from '../../domains/security-compliance/scan-evidence.js'; -import type { SecurityCoverage } from '../../domains/security-compliance/interfaces.js'; - const SUPPORTED_LANGUAGES = Object.keys(DEFAULT_FRAMEWORKS) as SupportedLanguage[]; /** From 45811d9c4f781e0ad817df9b141cb8d6e2d41ca4 Mon Sep 17 00:00:00 2001 From: Rudy Celekli Date: Mon, 21 Sep 2026 17:43:48 -0400 Subject: [PATCH 4/5] fix(ci): preserve runner exits and generate valid coverage reports (cherry picked from commit 480c054e3a1f1c74f36a0c189e3401970dbf04fd) --- .github/workflows/mcp-tools-test.yml | 18 ++--- scripts/ci-vitest-run.sh | 50 ++----------- tests/unit/scripts/ci-vitest-run.test.ts | 89 ++++++++++++++++++++++++ vitest.config.ts | 2 +- 4 files changed, 100 insertions(+), 59 deletions(-) create mode 100644 tests/unit/scripts/ci-vitest-run.test.ts diff --git a/.github/workflows/mcp-tools-test.yml b/.github/workflows/mcp-tools-test.yml index 64c0abced..138cd4c5f 100644 --- a/.github/workflows/mcp-tools-test.yml +++ b/.github/workflows/mcp-tools-test.yml @@ -147,21 +147,11 @@ jobs: - run: npm run build - name: Run MCP integration tests - run: | - timeout 480 npm run test:mcp:integration; EXIT=$? - if [ $EXIT -eq 124 ] && [ -f junit.xml ]; then - FAILURES=$(grep -c '/dev/null || echo "0") - if [ "$FAILURES" = "0" ]; then - echo "::warning::Vitest hung after tests passed (exit 124). Treating as success." - exit 0 - fi - fi - exit $EXIT + run: bash scripts/ci-vitest-run.sh tests/integration/mcp/ env: - NODE_OPTIONS: '--max-old-space-size=1024' - # C3: was `continue-on-error: true`, which let real failures pass as - # green. Removed so this job is an actual gate. The exit-124 hang - # tolerance above still absorbs the known vitest-hang flake. + NODE_OPTIONS: '--max-old-space-size=1024 --expose-gc' + CI_VITEST_TIMEOUT: '480' + # Preserve runner errors and timeouts even when a partial report exists. - name: Generate test report uses: dorny/test-reporter@v1 diff --git a/scripts/ci-vitest-run.sh b/scripts/ci-vitest-run.sh index 32260b6ec..7a1ddd706 100755 --- a/scripts/ci-vitest-run.sh +++ b/scripts/ci-vitest-run.sh @@ -1,50 +1,12 @@ #!/usr/bin/env bash -# CI wrapper for vitest that handles process hangs gracefully. -# -# Problem: vitest completes all tests but hangs due to open handles -# (SQLite connections, HNSW models, timers). The `timeout` command -# kills it with exit code 124, which CI treats as failure even though -# all tests passed. -# -# Solution: Capture vitest output via tee. When timeout kills vitest, -# check the captured output for the "Test Files X passed" summary -# line that vitest prints after all tests complete. junit.xml cannot -# be used because vitest writes it only on clean exit, and the killed -# process leaves it as 0 bytes. +# Bound CI test execution without changing Vitest's exit status. +# A passing test summary does not prove coverage/report generation or cleanup +# completed. Runner errors and timeouts must remain failures. # # Usage: scripts/ci-vitest-run.sh [vitest args...] TIMEOUT_SECONDS="${CI_VITEST_TIMEOUT:-480}" -OUTFILE=$(mktemp /tmp/vitest-output.XXXXXX) - -# --foreground: send signal only to the child process, not the process -# group. Without this, timeout kills this wrapper script too. -# Pipe through tee to capture output while still displaying it. -timeout --foreground "$TIMEOUT_SECONDS" npx vitest run "$@" 2>&1 | tee "$OUTFILE" -# PIPESTATUS[0] is timeout's exit code, not tee's -EXIT=${PIPESTATUS[0]} - -if [ "$EXIT" -eq 0 ]; then - rm -f "$OUTFILE" - exit 0 -fi - -# Check captured output for vitest's test summary. -# Vitest prints "Test Files X passed" after all tests complete, -# before the process hangs. If this line exists with no failures, -# tests passed and the exit code is from the timeout kill. -if grep -q "Test Files.*passed" "$OUTFILE" 2>/dev/null; then - if grep -q "Test Files.*failed" "$OUTFILE" 2>/dev/null; then - echo "::error::Some test files failed." - rm -f "$OUTFILE" - exit "$EXIT" - fi - echo "" - echo "::warning::Vitest process hung after all tests passed (exit $EXIT). Treating as success." - rm -f "$OUTFILE" - exit 0 -fi -echo "::error::Vitest was killed before tests completed (exit $EXIT)." -rm -f "$OUTFILE" -exit "$EXIT" +# --foreground sends the timeout signal to the child rather than this wrapper's +# process group. exec preserves the runner's status, including timeout exit 124. +exec timeout --foreground "$TIMEOUT_SECONDS" npx vitest run "$@" diff --git a/tests/unit/scripts/ci-vitest-run.test.ts b/tests/unit/scripts/ci-vitest-run.test.ts new file mode 100644 index 000000000..fa25cad4a --- /dev/null +++ b/tests/unit/scripts/ci-vitest-run.test.ts @@ -0,0 +1,89 @@ +import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join, resolve } from 'node:path'; +import { spawnSync } from 'node:child_process'; +import { afterEach, describe, expect, it } from 'vitest'; + +const wrapper = resolve(import.meta.dirname, '../../../scripts/ci-vitest-run.sh'); +// The wrapper is used by Ubuntu CI. Stock macOS and Windows do not ship all +// of its shell tools; qualify those local runs without letting Linux CI skip. +const timeoutVersion = spawnSync('timeout', ['--version'], { encoding: 'utf8' }); +const shellToolsAvailable = existsSync('/bin/bash') && timeoutVersion.status === 0 + && timeoutVersion.stdout.includes('GNU coreutils'); +if (process.platform === 'linux' && !shellToolsAvailable) { + throw new Error('CI Vitest wrapper tests require /bin/bash and GNU timeout on Linux; this CI prerequisite must not be skipped.'); +} +const skipReason = !shellToolsAvailable ? ' (requires Bash and GNU timeout on this platform)' : ''; +const fixtures: string[] = []; + +afterEach(() => { + for (const fixture of fixtures.splice(0)) { + rmSync(fixture, { recursive: true, force: true }); + } +}); + +function runRunner(exitCode: number, options: { hang?: boolean; summary?: boolean } = {}) { + const fixture = mkdtempSync(join(tmpdir(), 'aqe-ci-verdict-')); + fixtures.push(fixture); + const argsPath = join(fixture, 'args.txt'); + writeFileSync(join(fixture, 'npx'), `#!/bin/sh +printf '%s\n' "$@" > "$AQE_TEST_ARGS" +if [ "$AQE_TEST_SUMMARY" = 'true' ]; then + printf ' Test Files 1 passed (1)\n Tests 18 passed (18)\n' +fi +if [ "$AQE_TEST_HANG" = 'true' ]; then + exec sleep 15 +fi +if [ "$AQE_TEST_EXIT" != '0' ]; then + printf 'Unhandled Error: coverage report generation failed\n' >&2 +fi +exit "$AQE_TEST_EXIT" +`, { mode: 0o755 }); + + const result = spawnSync('/bin/bash', [wrapper, 'tests/fixture with spaces.test.ts', '--coverage'], { + cwd: fixture, + encoding: 'utf8', + timeout: 8000, + env: { + ...process.env, + PATH: `${fixture}:${process.env.PATH}`, + TMPDIR: fixture, + AQE_PROJECT_ROOT: fixture, + AQE_TEST_ARGS: argsPath, + AQE_TEST_EXIT: String(exitCode), + AQE_TEST_SUMMARY: String(options.summary ?? true), + AQE_TEST_HANG: String(options.hang ?? false), + CI_VITEST_TIMEOUT: '5', + }, + }); + return { ...result, args: readFileSync(argsPath, 'utf8').trim().split('\n') }; +} + +describe.skipIf(process.platform !== 'linux' && !shellToolsAvailable)(`CI Vitest runner exit status${skipReason}`, () => { + it('passes a successful runner and forwards arguments without splitting', () => { + const result = runRunner(0); + expect(result.error).toBeUndefined(); + expect(result.status).toBe(0); + expect(result.stdout).toContain('18 passed'); + expect(result.args).toEqual(['vitest', 'run', 'tests/fixture with spaces.test.ts', '--coverage']); + }); + + it.each([1, 2, 124, 137, 143, 255])('preserves exit %i after a passing test summary', (code) => { + const result = runRunner(code); + expect(result.error).toBeUndefined(); + expect(result.status).toBe(code); + expect(result.stdout + result.stderr).toContain('coverage report generation failed'); + expect(result.stdout + result.stderr).not.toContain('Treating as success'); + }); + + it('preserves failures before the test summary', () => { + expect(runRunner(1, { summary: false }).status).toBe(1); + }); + + it('fails an actual timeout even after all tests report passing', () => { + const result = runRunner(0, { hang: true }); + expect(result.error).toBeUndefined(); + expect(result.stdout).toContain('18 passed'); + expect(result.status).toBe(124); + }); +}); diff --git a/vitest.config.ts b/vitest.config.ts index 3fd5fea8e..93e359754 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -38,7 +38,7 @@ export default defineConfig({ ], coverage: { provider: 'v8', - reporter: ['text', 'json', 'html', 'junit'], + reporter: ['text', 'json', 'json-summary', 'html'], include: ['src/**/*.ts'], exclude: ['src/**/*.d.ts', 'src/**/index.ts'], }, From 3aec246eba1a6dbb01e004a30b4068080416cb08 Mon Sep 17 00:00:00 2001 From: Rudy Celekli Date: Mon, 21 Sep 2026 17:45:26 -0400 Subject: [PATCH 5/5] test(ci): bound local shell capability detection (cherry picked from commit 8152bc578fecaa58142cf4793ae8175e117ee493) --- tests/unit/scripts/ci-vitest-run.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/scripts/ci-vitest-run.test.ts b/tests/unit/scripts/ci-vitest-run.test.ts index fa25cad4a..fe5161af8 100644 --- a/tests/unit/scripts/ci-vitest-run.test.ts +++ b/tests/unit/scripts/ci-vitest-run.test.ts @@ -7,7 +7,7 @@ import { afterEach, describe, expect, it } from 'vitest'; const wrapper = resolve(import.meta.dirname, '../../../scripts/ci-vitest-run.sh'); // The wrapper is used by Ubuntu CI. Stock macOS and Windows do not ship all // of its shell tools; qualify those local runs without letting Linux CI skip. -const timeoutVersion = spawnSync('timeout', ['--version'], { encoding: 'utf8' }); +const timeoutVersion = spawnSync('timeout', ['--version'], { encoding: 'utf8', timeout: 2000 }); const shellToolsAvailable = existsSync('/bin/bash') && timeoutVersion.status === 0 && timeoutVersion.stdout.includes('GNU coreutils'); if (process.platform === 'linux' && !shellToolsAvailable) {