From f3de91b01bc2a162d48d103365eb45b2413439fd Mon Sep 17 00:00:00 2001 From: Cesar-M-Diaz Date: Thu, 16 Jul 2026 15:42:28 +0200 Subject: [PATCH 1/6] fix: Make the dependency audit skill scalable, make all calls through the cjs script --- .../skills/audit-dependencies-script.test.ts | 730 ++++++++++++++++ skills/ns-audit-dependencies/SKILL.md | 62 +- .../audit-dependencies.cjs | 822 ++++++++++++++++++ .../collect-dependencies.cjs | 27 +- 4 files changed, 1592 insertions(+), 49 deletions(-) create mode 100644 packages/core/test/unit/skills/audit-dependencies-script.test.ts create mode 100644 skills/ns-audit-dependencies/audit-dependencies.cjs diff --git a/packages/core/test/unit/skills/audit-dependencies-script.test.ts b/packages/core/test/unit/skills/audit-dependencies-script.test.ts new file mode 100644 index 0000000..24128f7 --- /dev/null +++ b/packages/core/test/unit/skills/audit-dependencies-script.test.ts @@ -0,0 +1,730 @@ +import assert from 'node:assert/strict' +import { createRequire } from 'node:module' +import { mkdtemp, rm, writeFile } from 'node:fs/promises' +import os from 'node:os' +import path from 'node:path' +import test from 'node:test' + +const require = createRequire(import.meta.url) +const { classifyBatchFailure, fetchBatch, runAudit } = require('../../../../../skills/ns-audit-dependencies/audit-dependencies.cjs') as { + classifyBatchFailure: (error: unknown) => string + fetchBatch: ( + apiUrl: string, + token: string, + packages: Array<{ name: string, version: string }>, + options?: Record + ) => Promise>> + runAudit: (dir: string, options: Record) => Promise> +} + +async function createNpmProject (packageCount: number): Promise { + const projectDir = await mkdtemp(path.join(os.tmpdir(), 'nsolid-audit-')) + const packages: Record = { '': {} } + + for (let index = 0; index < packageCount; index++) { + packages[`node_modules/package-${index}`] = { version: `1.0.${index}` } + } + + await writeFile(path.join(projectDir, 'package.json'), JSON.stringify({})) + await writeFile(path.join(projectDir, 'package-lock.json'), JSON.stringify({ + lockfileVersion: 3, + packages + })) + + return projectDir +} + +test('audit helper only emits vulnerable dependency data', async () => { + const projectDir = await mkdtemp(path.join(os.tmpdir(), 'nsolid-audit-')) + + try { + await writeFile(path.join(projectDir, 'package.json'), JSON.stringify({ + dependencies: { 'vulnerable-dep': '1.0.0' } + })) + await writeFile(path.join(projectDir, 'package-lock.json'), JSON.stringify({ + lockfileVersion: 3, + packages: { + '': { dependencies: { 'vulnerable-dep': '1.0.0' } }, + 'node_modules/vulnerable-dep': { version: '1.0.0' }, + 'node_modules/clean-dep': { version: '2.0.0' } + } + })) + + const summary = await runAudit(projectDir, { + token: 'test-token', + fetchBatch: async (_apiUrl: string, _token: string, packages: Array<{ name: string, version: string }>) => { + return packages.map(pkg => ({ + ...pkg, + published: true, + description: pkg.name === 'clean-dep' ? 'SECRET_CLEAN_BLOAT' : undefined, + scores: pkg.name === 'vulnerable-dep' && pkg.version === '1.0.0' + ? [ + { + group: 'security', + pass: false, + severity: 'HIGH', + title: 'Known vulnerability CVE-2026-1234', + data: { + cve: 'CVE-2026-1234', + vulnerable: ['< 1.0.1', ' < 1.0.1 ', 42], + patched: '>= 1.0.1', + description: 'LARGE_RAW_ADVISORY' + } + }, + { + group: 'risk', + pass: false, + severity: 'CRITICAL', + title: 'Package has an install script', + data: { scripts: ['postinstall'], secret: 'RAW_RISK_DATA' } + }, + { + group: 'quality', + pass: false, + severity: 'MEDIUM', + title: 'README is too small', + data: { bytes: 12, secret: 'RAW_QUALITY_DATA' } + }, + { + group: 'compliance', + name: 'license', + pass: true, + severity: 'NONE', + title: 'License is compliant', + data: { spdx: 'MIT', secret: 'RAW_LICENSE_DATA' } + } + ] + : [{ group: 'security', pass: true, severity: 'NONE', title: 'Clean' }] + })) + } + }) as { + packages: Record + vulnerabilities: Record + remediation: Record + findings: Array<{ + name: string + severity: string + license: { pass: boolean, spdx: string } + moduleRisks: Array<{ severity: string, title: string }> + codeQuality: Array<{ severity: string, title: string }> + vulnerabilities: Array<{ id: string, vulnerable: string[], patched: string[] }> + remediation: { status: string, version: string, source: string, changeType: string } + }> + } + const output = JSON.stringify(summary) + + assert.deepEqual(summary.packages, { total: 2, direct: 1, transitive: 1, checked: 2, unchecked: 0 }) + assert.equal(summary.vulnerabilities.total, 1) + assert.equal(summary.vulnerabilities.affectedPackages, 1) + assert.equal(summary.findings.length, 1) + assert.equal(summary.findings[0].name, 'vulnerable-dep') + assert.equal(summary.findings[0].severity, 'HIGH') + assert.equal(summary.findings[0].vulnerabilities[0].id, 'CVE-2026-1234') + assert.deepEqual(summary.findings[0].vulnerabilities[0].vulnerable, ['< 1.0.1']) + assert.deepEqual(summary.findings[0].vulnerabilities[0].patched, ['>= 1.0.1']) + assert.deepEqual(summary.findings[0].remediation, { + status: 'ncm-verified', + version: '1.0.1', + source: 'patched-range-boundary', + changeType: 'patch' + }) + assert.equal(summary.remediation.verified, 1) + assert.deepEqual(summary.findings[0].license, { pass: true, spdx: 'MIT' }) + assert.deepEqual(summary.findings[0].moduleRisks, [{ + severity: 'CRITICAL', + title: 'Package has an install script' + }]) + assert.deepEqual(summary.findings[0].codeQuality, [{ + severity: 'MEDIUM', + title: 'README is too small' + }]) + assert.doesNotMatch(output, /clean-dep|SECRET_CLEAN_BLOAT|LARGE_RAW_ADVISORY|RAW_RISK_DATA|RAW_QUALITY_DATA|RAW_LICENSE_DATA/) + } finally { + await rm(projectDir, { recursive: true, force: true }) + } +}) + +test('audit helper verifies a concrete latest fallback when no boundary candidate exists', async () => { + const projectDir = await createNpmProject(1) + const requestedVersions: string[] = [] + + try { + const summary = await runAudit(projectDir, { + token: 'test-token', + fetchBatch: async (_apiUrl: string, _token: string, packages: Array<{ name: string, version: string }>) => { + requestedVersions.push(...packages.map(pkg => pkg.version)) + return packages.map(pkg => { + if (pkg.version === 'latest') { + return { name: pkg.name, version: '2.0.0', published: true, scores: [] } + } + return { + ...pkg, + published: true, + scores: [{ + group: 'security', + pass: false, + severity: 'HIGH', + title: 'Vulnerability without an exclusive upper boundary', + data: { vulnerable: ['<= 1.0.0'] } + }] + } + }) + } + }) as { + packages: Record + remediation: Record + findings: Array<{ remediation: Record }> + } + + assert.deepEqual(requestedVersions, ['1.0.0', 'latest']) + assert.equal(summary.packages.checked, 1) + assert.equal(summary.remediation.verified, 1) + assert.deepEqual(summary.findings[0].remediation, { + status: 'ncm-verified', + version: '2.0.0', + source: 'latest-fallback', + changeType: 'major' + }) + } finally { + await rm(projectDir, { recursive: true, force: true }) + } +}) + +test('audit helper never emits the literal latest as a remediation version', async () => { + const projectDir = await createNpmProject(1) + + try { + const summary = await runAudit(projectDir, { + token: 'test-token', + fetchBatch: async (_apiUrl: string, _token: string, packages: Array<{ name: string, version: string }>) => { + return packages.map(pkg => ({ + ...pkg, + published: true, + scores: pkg.version === 'latest' + ? [] + : [{ + group: 'security', + pass: false, + severity: 'HIGH', + title: 'Known vulnerability', + data: { vulnerable: ['<= 1.0.0'] } + }] + })) + } + }) as { remediation: { unresolved: number }, findings: Array<{ remediation: { status: string, version?: string } }> } + + assert.equal(summary.remediation.unresolved, 1) + assert.deepEqual(summary.findings[0].remediation, { status: 'unresolved' }) + assert.equal(summary.findings[0].remediation.version, undefined) + } finally { + await rm(projectDir, { recursive: true, force: true }) + } +}) + +test('remediation verification failure does not make an audited package unchecked', async () => { + const projectDir = await createNpmProject(1) + + try { + const summary = await runAudit(projectDir, { + token: 'test-token', + fetchBatch: async (_apiUrl: string, _token: string, packages: Array<{ name: string, version: string }>) => { + if (packages.some(pkg => pkg.version !== '1.0.0')) { + throw Object.assign(new Error('candidate service failure'), { status: 503 }) + } + return packages.map(pkg => ({ + ...pkg, + published: true, + scores: [{ + group: 'security', + pass: false, + severity: 'HIGH', + title: 'Known vulnerability', + data: { vulnerable: ['< 1.0.1'] } + }] + })) + } + }) as { + packages: Record + batchFailures: { total: number } + remediation: { verificationFailed: number, failures: { total: number } } + findings: Array<{ remediation: { status: string } }> + } + + assert.equal(summary.packages.checked, 1) + assert.equal(summary.packages.unchecked, 0) + assert.equal(summary.batchFailures.total, 0) + assert.equal(summary.remediation.verificationFailed, 1) + assert.equal(summary.remediation.failures.total, 2) + assert.equal(summary.findings[0].remediation.status, 'verification-failed') + } finally { + await rm(projectDir, { recursive: true, force: true }) + } +}) + +test('withdrawn-only findings do not trigger remediation verification', async () => { + const projectDir = await createNpmProject(1) + let requests = 0 + + try { + const summary = await runAudit(projectDir, { + token: 'test-token', + fetchBatch: async (_apiUrl: string, _token: string, packages: Array<{ name: string, version: string }>) => { + requests++ + return packages.map(pkg => ({ + ...pkg, + published: true, + scores: [{ + group: 'security', + pass: false, + severity: 'LOW', + title: 'Withdrawn advisory', + data: { vulnerable: ['< 1.0.1'] } + }] + })) + } + }) as { + remediation: { notRequired: number, candidateRequests: number } + findings: Array<{ remediation: Record }> + } + + assert.equal(requests, 1) + assert.equal(summary.remediation.notRequired, 1) + assert.equal(summary.remediation.candidateRequests, 0) + assert.deepEqual(summary.findings[0].remediation, { + status: 'not-required', + reason: 'withdrawn-only' + }) + } finally { + await rm(projectDir, { recursive: true, force: true }) + } +}) + +test('remediation candidate batches remain within the two-request concurrency limit', async () => { + const projectDir = await createNpmProject(205) + let active = 0 + let maximumActive = 0 + + try { + const summary = await runAudit(projectDir, { + token: 'test-token', + fetchBatch: async ( + _apiUrl: string, + _token: string, + packages: Array<{ name: string, version: string }>, + options: { phase?: string } + ) => { + if (options.phase === 'remediation') { + active++ + maximumActive = Math.max(maximumActive, active) + await new Promise(resolve => setTimeout(resolve, 5)) + active-- + return packages.map(pkg => ({ ...pkg, published: true, scores: [] })) + } + return packages.map(pkg => ({ + ...pkg, + published: true, + scores: [{ + group: 'security', + pass: false, + severity: 'HIGH', + title: 'Known vulnerability', + data: { vulnerable: ['< 2.0.0'] } + }] + })) + } + }) as { + remediation: { candidateRequests: number, candidatesChecked: number, verified: number } + } + + assert.equal(maximumActive, 2) + assert.equal(summary.remediation.candidateRequests, 205) + assert.equal(summary.remediation.candidatesChecked, 205) + assert.equal(summary.remediation.verified, 205) + } finally { + await rm(projectDir, { recursive: true, force: true }) + } +}) + +test('audit helper retries an omitted remediation candidate response once', async () => { + const projectDir = await createNpmProject(1) + let remediationRequests = 0 + + try { + const summary = await runAudit(projectDir, { + token: 'test-token', + fetchBatch: async ( + _apiUrl: string, + _token: string, + packages: Array<{ name: string, version: string }>, + options: { phase?: string } + ) => { + if (options.phase === 'remediation') { + remediationRequests++ + if (remediationRequests === 1) return [] + return packages.map(pkg => ({ ...pkg, published: true, scores: [] })) + } + return packages.map(pkg => ({ + ...pkg, + published: true, + scores: [{ + group: 'security', + pass: false, + severity: 'HIGH', + title: 'Known vulnerability', + data: { vulnerable: ['< 1.0.1'] } + }] + })) + } + }) as { + remediation: { + verified: number + recovery: { missingCandidateRetries: number } + failures: { total: number } + } + } + + assert.equal(remediationRequests, 2) + assert.equal(summary.remediation.verified, 1) + assert.equal(summary.remediation.recovery.missingCandidateRetries, 1) + assert.equal(summary.remediation.failures.total, 0) + } finally { + await rm(projectDir, { recursive: true, force: true }) + } +}) + +test('audit helper runs at most two batches concurrently', async () => { + const projectDir = await createNpmProject(205) + let active = 0 + let maximumActive = 0 + let batches = 0 + + try { + await runAudit(projectDir, { + token: 'test-token', + fetchBatch: async (_apiUrl: string, _token: string, packages: Array<{ name: string, version: string }>) => { + batches++ + active++ + maximumActive = Math.max(maximumActive, active) + await new Promise(resolve => setTimeout(resolve, 10)) + active-- + return packages.map(pkg => ({ ...pkg, published: true, scores: [] })) + } + }) + + assert.equal(batches, 3) + assert.equal(maximumActive, 2) + } finally { + await rm(projectDir, { recursive: true, force: true }) + } +}) + +test('audit helper reports progress after every completed batch', async () => { + const projectDir = await createNpmProject(205) + const progress: string[] = [] + + try { + await runAudit(projectDir, { + token: 'test-token', + onProgress: (message: string) => progress.push(message), + fetchBatch: async (_apiUrl: string, _token: string, packages: Array<{ name: string, version: string }>) => { + return packages.map(pkg => ({ ...pkg, published: true, scores: [] })) + } + }) + + assert.deepEqual(progress, [ + 'Audit progress: batches 1/3, packages 100/205\n', + 'Audit progress: batches 2/3, packages 200/205\n', + 'Audit progress: batches 3/3, packages 205/205\n' + ]) + } finally { + await rm(projectDir, { recursive: true, force: true }) + } +}) + +test('audit helper classifies failures without exposing raw errors', () => { + assert.equal(classifyBatchFailure(Object.assign(new Error('request failed'), { status: 401 })), 'authentication') + assert.equal(classifyBatchFailure(Object.assign(new Error('request failed'), { status: 429 })), 'rate-limit') + assert.equal(classifyBatchFailure(Object.assign(new Error('request failed'), { status: 503 })), 'server') + assert.equal(classifyBatchFailure(Object.assign(new Error('request failed'), { name: 'AbortError' })), 'timeout') + assert.equal(classifyBatchFailure(new Error('fetch failed', { cause: Object.assign(new Error('dns'), { code: 'ENOTFOUND' }) })), 'network') + assert.equal(classifyBatchFailure(Object.assign(new Error('bad response'), { code: 'NCM_INVALID_RESPONSE' })), 'invalid-response') + assert.equal(classifyBatchFailure(new Error('unexpected failure')), 'unknown') +}) + +test('batch requests retry HTTP 500 with jitter and honor Retry-After', async () => { + const packages = [{ name: 'package', version: '1.0.0' }] + const jitterDelays: number[] = [] + const retryAfterDelays: number[] = [] + let http500Attempts = 0 + let http503Attempts = 0 + const response = (status: number, retryAfter: string | null = null) => ({ + ok: false, + status, + headers: { get: () => retryAfter }, + json: async () => ({}) + }) + + await assert.rejects(fetchBatch('https://example.invalid', 'token', packages, { + fetch: async () => { + http500Attempts++ + return response(500) + }, + random: () => 0.5, + sleep: async (delay: number) => jitterDelays.push(delay) + })) + await assert.rejects(fetchBatch('https://example.invalid', 'token', packages, { + fetch: async () => { + http503Attempts++ + return response(503, '2') + }, + sleep: async (delay: number) => retryAfterDelays.push(delay) + })) + + assert.equal(http500Attempts, 3) + assert.deepEqual(jitterDelays, [250, 500]) + assert.equal(http503Attempts, 3) + assert.deepEqual(retryAfterDelays, [2000, 2000]) +}) + +test('audit helper reports packages recovered by a transport retry', async () => { + const projectDir = await createNpmProject(1) + let attempts = 0 + + try { + const summary = await runAudit(projectDir, { + token: 'test-token', + random: () => 0, + sleep: async () => {}, + fetch: async () => { + attempts++ + if (attempts === 1) { + return { + ok: false, + status: 500, + headers: { get: () => null }, + json: async () => ({}) + } + } + return { + ok: true, + status: 200, + headers: { get: () => null }, + json: async () => ({ + data: { + packageVersions: [{ name: 'package-0', version: '1.0.0', published: true, scores: [] }] + } + }) + } + } + }) as { + packages: Record + batchRecovery: Record + } + + assert.equal(attempts, 2) + assert.equal(summary.packages.checked, 1) + assert.equal(summary.batchRecovery.transportRetries, 1) + assert.equal(summary.batchRecovery.recoveredPackages, 1) + } finally { + await rm(projectDir, { recursive: true, force: true }) + } +}) + +test('audit helper recovers an exhausted batch by splitting it once', async () => { + const projectDir = await createNpmProject(100) + const progress: string[] = [] + const requestSizes: number[] = [] + + try { + const summary = await runAudit(projectDir, { + token: 'test-token', + onProgress: (message: string) => progress.push(message), + fetchBatch: async (_apiUrl: string, _token: string, packages: Array<{ name: string, version: string }>) => { + requestSizes.push(packages.length) + if (packages.length === 100) throw Object.assign(new Error('temporary failure'), { status: 503 }) + return packages.map(pkg => ({ ...pkg, published: true, scores: [] })) + } + }) as { + packages: Record + batchFailures: { total: number, byReason: Record } + batchRecovery: Record + } + + assert.deepEqual(requestSizes, [100, 50, 50]) + assert.equal(summary.packages.checked, 100) + assert.equal(summary.packages.unchecked, 0) + assert.deepEqual(summary.batchFailures, { total: 0, byReason: {} }) + assert.deepEqual(summary.batchRecovery, { + transportRetries: 0, + splitBatches: 1, + missingPackageRetries: 0, + recoveredPackages: 100 + }) + assert.deepEqual(progress, ['Audit progress: batches 1/1, packages 100/100\n']) + } finally { + await rm(projectDir, { recursive: true, force: true }) + } +}) + +test('audit helper leaves only an unrecovered split unchecked', async () => { + const projectDir = await createNpmProject(100) + let requestNumber = 0 + + try { + const summary = await runAudit(projectDir, { + token: 'test-token', + fetchBatch: async (_apiUrl: string, _token: string, packages: Array<{ name: string, version: string }>) => { + requestNumber++ + if (requestNumber === 1 || requestNumber === 3) { + throw Object.assign(new Error('server PRIVATE_RAW_FAILURE'), { status: 503 }) + } + return packages.map(pkg => ({ ...pkg, published: true, scores: [] })) + } + }) as { + packages: Record + batchFailures: { total: number, byReason: Record } + batchRecovery: Record + } + + assert.equal(summary.packages.checked, 50) + assert.equal(summary.packages.unchecked, 50) + assert.deepEqual(summary.batchFailures, { total: 1, byReason: { server: 1 } }) + assert.equal(summary.batchRecovery.recoveredPackages, 50) + assert.doesNotMatch(JSON.stringify(summary), /PRIVATE_RAW_FAILURE/) + } finally { + await rm(projectDir, { recursive: true, force: true }) + } +}) + +test('audit helper bounds an exhausted batch to seven transport attempts', async () => { + const projectDir = await createNpmProject(100) + let attempts = 0 + + try { + const summary = await runAudit(projectDir, { + token: 'test-token', + random: () => 0, + sleep: async () => {}, + fetch: async () => { + attempts++ + return { + ok: false, + status: 503, + headers: { get: () => null }, + json: async () => ({}) + } + } + }) as { + packages: Record + batchFailures: { total: number, byReason: Record } + batchRecovery: Record + } + + assert.equal(attempts, 7) + assert.equal(summary.packages.unchecked, 100) + assert.deepEqual(summary.batchFailures, { total: 2, byReason: { server: 2 } }) + assert.equal(summary.batchRecovery.transportRetries, 4) + assert.equal(summary.batchRecovery.splitBatches, 1) + } finally { + await rm(projectDir, { recursive: true, force: true }) + } +}) + +test('audit helper retries omitted responses but not unpublished packages', async () => { + const projectDir = await createNpmProject(10) + const requestSizes: number[] = [] + + try { + const summary = await runAudit(projectDir, { + token: 'test-token', + fetchBatch: async (_apiUrl: string, _token: string, packages: Array<{ name: string, version: string }>) => { + requestSizes.push(packages.length) + if (packages.length === 10) { + return packages.slice(0, 9).map((pkg, index) => ({ + ...pkg, + published: index !== 0, + scores: [] + })) + } + return packages.map(pkg => ({ ...pkg, published: true, scores: [] })) + } + }) as { + packages: Record + batchFailures: { total: number, byReason: Record } + batchRecovery: Record + } + + assert.deepEqual(requestSizes, [10, 1]) + assert.equal(summary.packages.checked, 9) + assert.equal(summary.packages.unchecked, 1) + assert.deepEqual(summary.batchFailures, { total: 0, byReason: {} }) + assert.equal(summary.batchRecovery.missingPackageRetries, 1) + assert.equal(summary.batchRecovery.recoveredPackages, 1) + } finally { + await rm(projectDir, { recursive: true, force: true }) + } +}) + +test('audit recovery remains within the two-request concurrency limit', async () => { + const projectDir = await createNpmProject(205) + const failedFullBatches = new Set() + let active = 0 + let maximumActive = 0 + + try { + await runAudit(projectDir, { + token: 'test-token', + fetchBatch: async (_apiUrl: string, _token: string, packages: Array<{ name: string, version: string }>) => { + active++ + maximumActive = Math.max(maximumActive, active) + await new Promise(resolve => setTimeout(resolve, 5)) + active-- + const key = packages[0].name + if (packages.length === 100 && !failedFullBatches.has(key)) { + failedFullBatches.add(key) + throw Object.assign(new Error('temporary server failure'), { status: 503 }) + } + return packages.map(pkg => ({ ...pkg, published: true, scores: [] })) + } + }) + + assert.equal(maximumActive, 2) + } finally { + await rm(projectDir, { recursive: true, force: true }) + } +}) + +test('audit helper preserves severity sorting and the 50-finding limit', async () => { + const projectDir = await createNpmProject(55) + + try { + const summary = await runAudit(projectDir, { + token: 'test-token', + fetchBatch: async (_apiUrl: string, _token: string, packages: Array<{ name: string, version: string }>) => { + return packages.map((pkg, index) => ({ + ...pkg, + published: true, + scores: [{ + group: 'security', + pass: false, + severity: index === packages.length - 1 ? 'CRITICAL' : 'LOW', + title: index === 0 ? 'Withdrawn advisory' : 'Security issue' + }] + })) + } + }) as { + vulnerabilities: { total: number, affectedPackages: number } + findings: Array<{ severity: string, vulnerabilities: Array<{ withdrawn?: boolean }> }> + truncatedFindings: number + } + + assert.equal(summary.vulnerabilities.total, 55) + assert.equal(summary.vulnerabilities.affectedPackages, 55) + assert.equal(summary.findings.length, 50) + assert.equal(summary.findings[0].severity, 'CRITICAL') + assert.equal(summary.findings.some(finding => finding.vulnerabilities.some(item => item.withdrawn)), true) + assert.equal(summary.truncatedFindings, 5) + } finally { + await rm(projectDir, { recursive: true, force: true }) + } +}) diff --git a/skills/ns-audit-dependencies/SKILL.md b/skills/ns-audit-dependencies/SKILL.md index 0ddd22f..50bc337 100644 --- a/skills/ns-audit-dependencies/SKILL.md +++ b/skills/ns-audit-dependencies/SKILL.md @@ -1,62 +1,48 @@ --- name: ns-audit-dependencies description: >- - Audits a local Node.js project dependency tree with NCM vulnerability and quality data. Use when the user asks for an npm audit-style security review, package CVEs, direct/transitive vulnerability report, dependency risk assessment, or remediation plan before upgrading. For vulnerabilities actually loaded in running N|Solid processes, use ns-analyze-vulnerabilities instead. + Audits a local Node.js project dependency tree with NCM vulnerability data. Use when the user asks for an npm audit-style security review, package CVEs, direct/transitive vulnerability report, dependency risk assessment, or remediation plan before upgrading. For vulnerabilities actually loaded in running N|Solid processes, use ns-analyze-vulnerabilities instead. --- -### 1. Collect Dependencies +### 1. Collect and Query -**If grounded audit data is already provided in the prompt** (a `## Audit Results` block injected by the host), stop at that data: use it exclusively and skip steps 2 (NCM queries) and 3 (N|Solid live enrichment). Do not re-fetch packages that are already covered, and do not continue into any live enrichment or re-fetch logic — the injected data is the complete source of truth. +**If grounded audit data is already provided in the prompt** (a `## Audit Results` block injected by the host), use it exclusively. Skip the bundled helper and live enrichment; the injected data is the complete source of truth. -**Otherwise (MCP-only / agent mode):** -- Run the bundled helper to extract all packages (use the absolute path of the directory where you read this SKILL.md): +**Otherwise (agent mode):** +- Run the bundled audit helper from the project root (use the absolute path of the directory where you read this SKILL.md): ``` - node "/collect-dependencies.cjs" + node "/audit-dependencies.cjs" --dir "$PWD" ``` - The script walks `package.json` and the lockfile (`package-lock.json`, `yarn.lock`, or `pnpm-lock.yaml`) and prints a JSON object: - ```json - { - "packageManager": "npm|yarn|pnpm", - "direct": 12, - "transitive": 84, - "batches": [[{"name":"express","version":"4.18.2","isDirect":true}, ...], ...] - } - ``` -- Detect the package manager from the `packageManager` field (used later for exact remediation commands). - -### 2. Query NCM for Vulnerabilities -- For each batch in `batches`, call `getPackageVersions` with that batch array. -- Collect all packages that have at least one vulnerability reported. -- For critical or high severity findings where you need more detail, call `getPackageQuality` on that specific package. -- Cap batch size at ≤ 100 packages per `getPackageVersions` call. - -### 3. Optional: Enrich with N|Solid Live Data -This is enrichment only; the NCM audit path above remains the source of truth. Check `information-dashboard` first. -- No connected agents → skip live enrichment. -- `vulnerabilities` — high-level security overview across connected processes. -- `application-packages` — use the exact `app` name from `information-dashboard`. -- `sbom` — use the exact `app` name; do not guess. - -### 4. Present an Audit Report +- The helper requires outbound HTTPS access to `api.ncm.nodesource.com`. If the current command sandbox is known to block network access, request scoped escalation for this exact helper command before starting it. Let the harness ask the user for approval; never request broad or full-access sandbox permissions. +- The helper parses the lockfile, queries NCM in batches, retries transient failures, splits an exhausted batch once, retries omitted responses once, discards clean-package responses, and prints one compact JSON summary. For active findings it also verifies range-boundary candidates with NCM, then tests `latest` only as a fallback candidate and emits the concrete returned version when clean. Treat the summary as authoritative. +- Large audits and internal recovery can remain active while the helper reports compact progress, retries, and recovery on stderr. Poll the same execution session until it exits and returns the final JSON on stdout. +- Do not launch `wait.cjs`, start a second audit, or restart an audit automatically. If the original execution session is lost or terminated before it returns JSON, report the audit as incomplete. +- Do not call `getPackageVersions` or `getPackageQuality` after the helper succeeds. Raw dependency lists and clean NCM responses must stay outside model context. +- Inspect `batchFailures` before reporting results. If a completed attempt checked zero packages, left every package unchecked, and has only terminal `network` failures, request scoped network approval and rerun the exact helper command once; do not retry without approval. For `authentication`, direct the user to run `nsolid-plugin setup --harness `. For other failures, report the affected packages as unchecked; never describe them as safe. +- If the helper fails before returning a summary, report the error. Do not fall back to model-driven per-package MCP calls unless the user explicitly requests that slower fallback. + +Do not add N|Solid live enrichment to this static audit. If the user explicitly asks which findings are loaded in running processes, run `ns-analyze-vulnerabilities` as a separate workflow. + +### 2. Present an Audit Report Emit the audit directly in chat as markdown with these sections: -**Summary** — total packages checked, total vulnerabilities found (by severity), and count of packages that could not be checked. +**Summary** — total packages checked, total vulnerabilities found (by severity), count of packages that could not be checked, aggregate terminal batch-failure reasons, and recovery counts from `batchRecovery` when any retries, splits, or omitted-response recovery occurred. Summarize `remediation` separately: verified targets, unresolved findings, verification failures, withdrawn-only findings requiring no action, and candidate-recovery/failure counts. A remediation failure does not make the installed package unchecked when its primary vulnerability scan succeeded. -**Prioritized Findings** — sorted critical → high → medium → low. For each vulnerable package include package name/version, severity/title from NCM, latest safe version if known, and whether it is direct or transitive. +**Prioritized Findings** — sorted critical → high → medium → low. Render one separate finding block for every returned package version; never combine packages or versions into a single bullet. Include direct/transitive status and list every returned vulnerability with its severity, title, ID, URL, `vulnerable` ranges, and `patched` ranges when present. Mark advisories with `withdrawn: true` as withdrawn without silently removing them. Include the package's license, module risks, and code-quality issues from the finding. Render its remediation status exactly: `ncm-verified` with version/source/change type, `unresolved`, `verification-failed`, or `not-required` for withdrawn-only findings. Describe `ncm-verified` as free of active NCM advisories at audit time, not absolutely safe. If `truncatedFindings` is greater than zero, state exactly how many package findings were omitted. -**Remediation Plan** — exact commands for the detected package manager (npm / yarn / pnpm), starting with critical/high issues. Note SemVer breaking changes and use overrides/resolutions for transitive deps the user cannot directly control. +**Remediation Plan** — give package-specific commands for each direct critical/high finding only when `remediation.status` is `ncm-verified`, using its exact `remediation.version` and the detected package manager (npm / yarn / pnpm). Never emit the literal `@latest`. For `unresolved`, report the ranges as upgrade-selection evidence and require candidate verification before pinning. For `verification-failed`, say verification could not complete without weakening the vulnerability finding. For transitive findings, provide the appropriate `why` command and instruct the user to upgrade the introducing parent; use overrides/resolutions only temporarily with an `ncm-verified` version. -**Breaking Change Notes** — flag major-version bumps required to fix vulnerabilities; remind the user to review official changelogs. +**Breaking Change Notes** — use `remediation.changeType` to flag major-version bumps required to fix vulnerabilities; remind the user to review official changelogs. **Rollback Guidance** — N|Solid backs up `package.json` and the lockfile to `.nsolid/backup/` before each upgrade. If an upgrade causes issues, the user can click "Rollback" in the post-upgrade notification or restore manually from that directory. -### 5. Write the Report to Disk +### 3. Write the Report to Disk - Ask the user if they want to save the report to disk. - If the user confirms, write the final audit report as a markdown file (`.md`) under `.nsolid/assets/` — for example `.nsolid/assets/dependency-audit-.md`. ## Guardrails - Never hallucinate CVE IDs, vulnerability titles, or severity levels. Use only what NCM returns. If a package cannot be checked, report it as unchecked rather than safe. +- Never turn `latest-fallback` into `@latest`; use only the concrete NCM-returned `remediation.version`. Never infer a fixed version from a vulnerable range when remediation is unresolved. - When host-provided audit data is in the prompt, analyze only that data — do not re-fetch. -- Cap each `getPackageVersions` call at ≤ 100 packages to avoid context overflow. -- If the total vulnerability count exceeds 50 packages, truncate the detailed findings list and note the total count so the user knows the scope. +- Present every finding returned by the helper even when the report is long. The helper enforces the 50-package detail limit; do not compress its returned findings further. - Rollback reminder is mandatory in every audit response. diff --git a/skills/ns-audit-dependencies/audit-dependencies.cjs b/skills/ns-audit-dependencies/audit-dependencies.cjs new file mode 100644 index 0000000..2c241c4 --- /dev/null +++ b/skills/ns-audit-dependencies/audit-dependencies.cjs @@ -0,0 +1,822 @@ +#!/usr/bin/env node +'use strict' + +const fs = require('fs') +const os = require('os') +const path = require('path') +const { collectDependencies } = require('./collect-dependencies.cjs') + +const DEFAULT_API_URL = 'https://api.ncm.nodesource.com' +const MAX_FINDINGS = 50 +const REQUEST_TIMEOUT_MS = 120000 +const MAX_RETRIES = 2 +const MAX_RECOVERY_RETRIES = 1 +const MAX_CONCURRENCY = 2 +const MAX_RECOVERY_DEPTH = 1 +const RETRY_BASE_DELAY_MS = 500 +const RETRY_MAX_DELAY_MS = 5000 +const RETRY_AFTER_MAX_DELAY_MS = 30000 +const REMEDIATION_BATCH_SIZE = 100 +const MAX_REMEDIATION_CANDIDATES_PER_FINDING = 10 + +const PACKAGE_VERSIONS_QUERY = ` + query getPackageVersions($packageVersions: [PackageVersionInput!]!) { + packageVersions(packageVersions: $packageVersions) { + name + version + published + scores { + group + name + pass + severity + title + data + } + } + } +` + +const SEVERITY_RANK = { + NONE: 0, + LOW: 1, + MEDIUM: 2, + HIGH: 3, + CRITICAL: 4 +} + +function parseArgs () { + const args = process.argv.slice(2) + let dir = process.cwd() + + for (let i = 0; i < args.length; i++) { + if (args[i] === '--dir' && args[i + 1]) dir = path.resolve(args[++i]) + } + + return { dir } +} + +function loadToken () { + if (process.env.NCM_TOKEN) return process.env.NCM_TOKEN + + const authPath = path.join(os.homedir(), '.agents', '.nodesource-auth.json') + let auth + try { + auth = JSON.parse(fs.readFileSync(authPath, 'utf8')) + } catch { + throw new Error('NodeSource credentials not found. Run: npx -y nsolid-plugin setup --harness ') + } + + if (typeof auth.serviceToken !== 'string' || auth.serviceToken.length === 0) { + throw new Error('Missing serviceToken in ~/.agents/.nodesource-auth.json') + } + + return auth.serviceToken +} + +function normalizeApiUrl (value) { + return value.replace(/\/+$/, '') +} + +function isRetryable (error) { + const message = String(error && error.message ? error.message : error).toLowerCase() + const status = error && error.status + return (error && error.name === 'AbortError') || + status === 429 || status === 500 || status === 502 || status === 503 || status === 504 || + /\b(429|500|502|503|504)\b|timed? ?out|econnreset|socket hang up|fetch failed/.test(message) +} + +function ncmError (message, code, status, retryAfterMs) { + const error = new Error(message) + error.code = code + if (status) error.status = status + if (Number.isFinite(retryAfterMs)) error.retryAfterMs = retryAfterMs + return error +} + +function parseRetryAfter (value, now = Date.now()) { + if (typeof value !== 'string' || value.trim() === '') return null + const seconds = Number(value) + if (Number.isFinite(seconds) && seconds >= 0) return seconds * 1000 + const date = Date.parse(value) + if (!Number.isFinite(date)) return null + return Math.max(0, date - now) +} + +function responseHeader (response, name) { + return response && response.headers && typeof response.headers.get === 'function' + ? response.headers.get(name) + : null +} + +function retryDelay (error, attempt, random) { + if (Number.isFinite(error && error.retryAfterMs)) { + return Math.min(error.retryAfterMs, RETRY_AFTER_MAX_DELAY_MS) + } + const ceiling = Math.min(RETRY_BASE_DELAY_MS * (2 ** attempt), RETRY_MAX_DELAY_MS) + return Math.floor(random() * ceiling) +} + +async function fetchBatch (apiUrl, token, packages, options = {}) { + let lastError + const fetchImpl = options.fetch || fetch + const sleep = options.sleep || (delay => new Promise(resolve => setTimeout(resolve, delay))) + const random = options.random || Math.random + const onRetry = options.onRetry || (() => {}) + const maxRetries = Number.isInteger(options.maxRetries) ? options.maxRetries : MAX_RETRIES + + for (let attempt = 0; attempt <= maxRetries; attempt++) { + const controller = new AbortController() + const timer = setTimeout(() => controller.abort(), REQUEST_TIMEOUT_MS) + + try { + const response = await fetchImpl(`${normalizeApiUrl(apiUrl)}/ncm2/api/v2/graphql`, { + method: 'POST', + headers: { + Authorization: `Bearer ${token}`, + 'Content-Type': 'application/json' + }, + body: JSON.stringify({ + query: PACKAGE_VERSIONS_QUERY, + variables: { packageVersions: packages } + }), + signal: controller.signal + }) + + let body + try { + body = await response.json() + } catch { + if (!response.ok) { + throw ncmError( + `NCM API returned HTTP ${response.status}`, + 'NCM_HTTP_ERROR', + response.status, + parseRetryAfter(responseHeader(response, 'retry-after')) + ) + } + throw ncmError('NCM API returned invalid JSON', 'NCM_INVALID_RESPONSE') + } + + if (!response.ok) { + throw ncmError( + `NCM API returned HTTP ${response.status}`, + 'NCM_HTTP_ERROR', + response.status, + parseRetryAfter(responseHeader(response, 'retry-after')) + ) + } + if (Array.isArray(body.errors) && body.errors.length > 0) { + throw ncmError(`NCM API error: ${body.errors.map(error => error.message).join('; ')}`, 'NCM_GRAPHQL_ERROR') + } + if (!Array.isArray(body.data && body.data.packageVersions)) { + throw ncmError('NCM API returned an invalid packageVersions response', 'NCM_INVALID_RESPONSE') + } + + return body.data.packageVersions + } catch (error) { + lastError = error + if (attempt === maxRetries || !isRetryable(error)) throw error + const delayMs = retryDelay(error, attempt, random) + onRetry({ + attempt: attempt + 2, + totalAttempts: maxRetries + 1, + reason: classifyBatchFailure(error), + delayMs + }) + await sleep(delayMs) + } finally { + clearTimeout(timer) + } + } + + throw lastError +} + +function packageKey (pkg) { + return `${pkg.name}@${pkg.version}` +} + +function compactStringList (value) { + const values = Array.isArray(value) ? value : typeof value === 'string' ? [value] : [] + return Array.from(new Set(values + .filter(item => typeof item === 'string') + .map(item => item.trim()) + .filter(Boolean))) +} + +function parseVersion (value) { + const match = String(value || '').trim().match(/^v?(\d+)\.(\d+)\.(\d+)(?:-([0-9A-Za-z.-]+))?$/) + if (!match) return null + return { + major: Number(match[1]), + minor: Number(match[2]), + patch: Number(match[3]), + prerelease: match[4] || '' + } +} + +function compareVersions (left, right) { + const a = parseVersion(left) + const b = parseVersion(right) + if (!a || !b) return String(left).localeCompare(String(right), undefined, { numeric: true }) + for (const field of ['major', 'minor', 'patch']) { + if (a[field] !== b[field]) return a[field] - b[field] + } + if (a.prerelease === b.prerelease) return 0 + if (!a.prerelease) return 1 + if (!b.prerelease) return -1 + return a.prerelease.localeCompare(b.prerelease, undefined, { numeric: true }) +} + +function versionChangeType (current, candidate) { + const from = parseVersion(current) + const to = parseVersion(candidate) + if (!from || !to) return 'unknown' + if (from.major !== to.major) return 'major' + if (from.minor !== to.minor) return 'minor' + if (from.patch !== to.patch || from.prerelease !== to.prerelease) return 'patch' + return 'none' +} + +function remediationCandidates (finding) { + const candidates = new Map() + const add = (version, source) => { + if (!parseVersion(version) || compareVersions(version, finding.version) <= 0) return + if (!candidates.has(version)) candidates.set(version, source) + } + + for (const vulnerability of finding.vulnerabilities.filter(item => !item.withdrawn)) { + for (const range of vulnerability.patched || []) { + const match = range.match(/(?:^|[\s|,(])>=?\s*v?(\d+\.\d+\.\d+(?:-[0-9A-Za-z.-]+)?)/) + if (match) add(match[1], 'patched-range-boundary') + } + for (const range of vulnerability.vulnerable || []) { + const pattern = /(?:^|[\s|,(])<(?![=])\s*v?(\d+\.\d+\.\d+(?:-[0-9A-Za-z.-]+)?)/g + let match + while ((match = pattern.exec(range)) !== null) add(match[1], 'vulnerable-range-boundary') + } + } + + return Array.from(candidates, ([version, source]) => ({ version, source })) + .sort((a, b) => compareVersions(a.version, b.version)) + .slice(0, MAX_REMEDIATION_CANDIDATES_PER_FINDING) +} + +function activeVulnerabilities (pkg) { + if (!pkg || !Array.isArray(pkg.scores)) return null + return pkg.scores.filter(score => { + return score && score.group === 'security' && score.pass === false && + !/\bwithdrawn\b/i.test(String(score.title || '')) + }) +} + +function compactVulnerability (score) { + const title = String(score.title || 'Untitled NCM vulnerability') + const result = { + severity: String(score.severity || 'NONE').toUpperCase(), + title + } + const data = score.data + if (data && typeof data === 'object') { + const id = typeof data.cve === 'string' ? data.cve : typeof data.id === 'string' ? data.id : null + if (id) result.id = id + if (typeof data.url === 'string') result.url = data.url + const vulnerable = compactStringList(data.vulnerable) + const patched = compactStringList(data.patched) + if (vulnerable.length > 0) result.vulnerable = vulnerable + if (patched.length > 0) result.patched = patched + } + if (/\bwithdrawn\b/i.test(title)) result.withdrawn = true + return result +} + +function compactAssessment (score, fallbackTitle) { + return { + severity: String(score.severity || 'NONE').toUpperCase(), + title: String(score.title || fallbackTitle) + } +} + +function compactLicense (score) { + const data = score.data + return { + pass: score.pass === true, + spdx: data && typeof data === 'object' && typeof data.spdx === 'string' + ? data.spdx + : null + } +} + +function classifyBatchFailure (error) { + const chain = [] + let current = error + while (current && chain.length < 5) { + chain.push(current) + current = current.cause + } + + const statuses = chain.map(item => Number(item.status)).filter(Number.isFinite) + const codes = chain.map(item => String(item.code || '').toUpperCase()) + const names = chain.map(item => String(item.name || '').toLowerCase()) + const message = chain.map(item => String(item.message || item)).join(' ').toLowerCase() + + if (statuses.includes(401) || statuses.includes(403) || + /unauthori[sz]ed|forbidden|invalid token|expired token|permission denied/.test(message)) { + return 'authentication' + } + if (statuses.includes(429) || /\b429\b|rate.?limit|too many requests/.test(message)) { + return 'rate-limit' + } + if (statuses.some(status => status >= 500 && status <= 599) || /\b5\d\d\b/.test(message)) { + return 'server' + } + if (names.includes('aborterror') || codes.some(code => ['ETIMEDOUT', 'ESOCKETTIMEDOUT'].includes(code)) || + /timed? ?out|timeout/.test(message)) { + return 'timeout' + } + if (codes.some(code => ['ENOTFOUND', 'EAI_AGAIN', 'ENETUNREACH', 'EHOSTUNREACH', 'ECONNREFUSED', 'ECONNRESET'].includes(code)) || + /fetch failed|socket hang up|network/.test(message)) { + return 'network' + } + if (codes.some(code => ['NCM_INVALID_RESPONSE', 'NCM_GRAPHQL_ERROR'].includes(code)) || + /invalid json|invalid packageversions response|graphql/.test(message)) { + return 'invalid-response' + } + return 'unknown' +} + +function highestSeverity (vulnerabilities) { + return vulnerabilities.reduce((highest, vulnerability) => { + return (SEVERITY_RANK[vulnerability.severity] || 0) > (SEVERITY_RANK[highest] || 0) + ? vulnerability.severity + : highest + }, 'NONE') +} + +async function runAudit (dir, options = {}) { + const collected = collectDependencies(dir) + const requested = collected.batches.flat() + const directByKey = new Map(requested.map(pkg => [packageKey(pkg), pkg.isDirect])) + const findings = [] + const failureReasons = {} + const recoveredPackageKeys = new Set() + const batchRecovery = { + transportRetries: 0, + splitBatches: 0, + missingPackageRetries: 0 + } + const remediationRecovery = { + transportRetries: 0, + splitBatches: 0, + missingCandidateRetries: 0 + } + const remediationFailureReasons = {} + let unchecked = 0 + let failedBatches = 0 + + const token = requested.length > 0 ? options.token || loadToken() : '' + const apiUrl = options.apiUrl || process.env.NCM_API || DEFAULT_API_URL + const requestBatch = options.fetchBatch || fetchBatch + const onProgress = options.onProgress || (() => {}) + const totalBatches = collected.batches.length + let nextBatch = 0 + let completedBatches = 0 + let processedPackages = 0 + + function writeStatus (message) { + process.stderr.write(message) + } + + function mergeResults (...groups) { + const merged = new Map() + for (const results of groups) { + for (const pkg of results) merged.set(packageKey(pkg), pkg) + } + return Array.from(merged.values()) + } + + async function requestPackages (packages, maxRetries) { + let retried = false + const response = await requestBatch(apiUrl, token, packages, { + maxRetries, + fetch: options.fetch, + sleep: options.sleep, + random: options.random, + onRetry: event => { + retried = true + batchRecovery.transportRetries++ + writeStatus(`Audit retry: ${event.reason}, attempt ${event.attempt}/${event.totalAttempts}\n`) + } + }) + if (retried && Array.isArray(response)) { + const requestedKeys = new Set(packages.map(packageKey)) + for (const pkg of response) { + if (pkg && typeof pkg.name === 'string' && typeof pkg.version === 'string' && + requestedKeys.has(packageKey(pkg))) { + recoveredPackageKeys.add(packageKey(pkg)) + } + } + } + return response + } + + async function fetchWithRecovery (packages, recoveryOptions = {}) { + const depth = recoveryOptions.depth || 0 + const isRecovery = recoveryOptions.isRecovery === true + const retryMissing = recoveryOptions.retryMissing !== false + const maxRetries = recoveryOptions.maxRetries === undefined + ? MAX_RETRIES + : recoveryOptions.maxRetries + const requestedKeys = new Set(packages.map(packageKey)) + let results + + try { + const response = await requestPackages(packages, maxRetries) + if (!Array.isArray(response)) { + throw ncmError('NCM API returned an invalid packageVersions response', 'NCM_INVALID_RESPONSE') + } + results = mergeResults(response.filter(pkg => { + return pkg && typeof pkg.name === 'string' && typeof pkg.version === 'string' && + requestedKeys.has(packageKey(pkg)) + })) + } catch (error) { + if (isRetryable(error) && depth < MAX_RECOVERY_DEPTH && packages.length > 1) { + const reason = classifyBatchFailure(error) + const midpoint = Math.ceil(packages.length / 2) + batchRecovery.splitBatches++ + writeStatus(`Audit recovery: splitting failed batch (${reason})\n`) + const left = await fetchWithRecovery(packages.slice(0, midpoint), { + depth: depth + 1, + isRecovery: true, + retryMissing: false, + maxRetries: MAX_RECOVERY_RETRIES + }) + const right = await fetchWithRecovery(packages.slice(midpoint), { + depth: depth + 1, + isRecovery: true, + retryMissing: false, + maxRetries: MAX_RECOVERY_RETRIES + }) + return { + results: mergeResults(left.results, right.results), + unresolved: [...left.unresolved, ...right.unresolved], + failures: [...left.failures, ...right.failures] + } + } + + return { + results: [], + unresolved: packages, + failures: [{ reason: classifyBatchFailure(error) }] + } + } + + const returnedKeys = new Set(results.map(packageKey)) + if (isRecovery) { + for (const key of returnedKeys) recoveredPackageKeys.add(key) + } + const missing = packages.filter(pkg => !returnedKeys.has(packageKey(pkg))) + + if (missing.length > 0 && retryMissing) { + batchRecovery.missingPackageRetries++ + writeStatus(`Audit recovery: retrying ${missing.length} omitted package response(s)\n`) + const retried = await fetchWithRecovery(missing, { + depth: MAX_RECOVERY_DEPTH, + isRecovery: true, + retryMissing: false, + maxRetries: MAX_RECOVERY_RETRIES + }) + return { + results: mergeResults(results, retried.results), + unresolved: retried.unresolved, + failures: retried.failures + } + } + + return { + results, + unresolved: missing, + failures: missing.length > 0 ? [{ reason: 'missing-response' }] : [] + } + } + + async function queryRemediationRequests (requests, mode) { + const resultByKey = new Map() + const failedKeys = new Set() + const requestKey = pkg => mode === 'latest' ? pkg.name : packageKey(pkg) + const totalBatches = Math.ceil(requests.length / REMEDIATION_BATCH_SIZE) + let nextBatch = 0 + let completedBatches = 0 + let processedCandidates = 0 + + function recordFailures (packages, reason) { + for (const pkg of packages) failedKeys.add(requestKey(pkg)) + remediationFailureReasons[reason] = (remediationFailureReasons[reason] || 0) + packages.length + } + + async function requestChunk (packages, requestOptions = {}) { + const depth = requestOptions.depth || 0 + const retryMissing = requestOptions.retryMissing !== false + let response + try { + response = await requestBatch(apiUrl, token, packages, { + phase: 'remediation', + maxRetries: depth === 0 ? MAX_RETRIES : MAX_RECOVERY_RETRIES, + fetch: options.fetch, + sleep: options.sleep, + random: options.random, + onRetry: event => { + remediationRecovery.transportRetries++ + writeStatus(`Audit remediation retry: ${event.reason}, attempt ${event.attempt}/${event.totalAttempts}\n`) + } + }) + if (!Array.isArray(response)) { + throw ncmError('NCM API returned an invalid packageVersions response', 'NCM_INVALID_RESPONSE') + } + } catch (error) { + if (isRetryable(error) && depth < MAX_RECOVERY_DEPTH && packages.length > 1) { + remediationRecovery.splitBatches++ + writeStatus(`Audit remediation recovery: splitting failed candidate batch (${classifyBatchFailure(error)})\n`) + const midpoint = Math.ceil(packages.length / 2) + await requestChunk(packages.slice(0, midpoint), { depth: depth + 1, retryMissing: false }) + await requestChunk(packages.slice(midpoint), { depth: depth + 1, retryMissing: false }) + return + } + recordFailures(packages, classifyBatchFailure(error)) + return + } + + const requestedKeys = new Set(packages.map(requestKey)) + for (const pkg of response) { + if (!pkg || typeof pkg.name !== 'string' || typeof pkg.version !== 'string') continue + const key = mode === 'latest' ? pkg.name : packageKey(pkg) + if (requestedKeys.has(key) && !resultByKey.has(key)) resultByKey.set(key, pkg) + } + + const missing = packages.filter(pkg => !resultByKey.has(requestKey(pkg))) + if (missing.length > 0 && retryMissing) { + remediationRecovery.missingCandidateRetries++ + writeStatus(`Audit remediation recovery: retrying ${missing.length} omitted candidate response(s)\n`) + await requestChunk(missing, { depth: MAX_RECOVERY_DEPTH, retryMissing: false }) + } else if (missing.length > 0) { + recordFailures(missing, 'missing-response') + } + } + + async function processRemediationBatch (batch) { + try { + await requestChunk(batch) + } finally { + completedBatches++ + processedCandidates += batch.length + writeStatus(`Audit remediation: batches ${completedBatches}/${totalBatches}, candidates ${processedCandidates}/${requests.length}\n`) + } + } + + async function remediationWorker () { + while (nextBatch < totalBatches) { + const start = nextBatch++ * REMEDIATION_BATCH_SIZE + await processRemediationBatch(requests.slice(start, start + REMEDIATION_BATCH_SIZE)) + } + } + + const workerCount = Math.min(MAX_CONCURRENCY, totalBatches) + await Promise.all(Array.from({ length: workerCount }, () => remediationWorker())) + return { resultByKey, failedKeys } + } + + async function verifyFindingRemediations () { + const states = findings.map(finding => ({ + finding, + candidates: remediationCandidates(finding), + hadResponse: false, + hadFailure: false, + selected: null + })) + const activeStates = states.filter(state => { + if (state.finding.vulnerabilities.some(item => !item.withdrawn)) return true + state.finding.remediation = { status: 'not-required', reason: 'withdrawn-only' } + return false + }) + + const boundaryRequests = [] + const seenBoundaries = new Set() + for (const state of activeStates) { + for (const candidate of state.candidates) { + const key = `${state.finding.name}@${candidate.version}` + if (seenBoundaries.has(key)) continue + seenBoundaries.add(key) + boundaryRequests.push({ name: state.finding.name, version: candidate.version }) + } + } + + let candidateRequests = 0 + let candidatesChecked = 0 + if (boundaryRequests.length > 0) { + candidateRequests += boundaryRequests.length + const boundaryResults = await queryRemediationRequests(boundaryRequests, 'exact') + candidatesChecked += boundaryResults.resultByKey.size + for (const state of activeStates) { + for (const candidate of state.candidates) { + const key = `${state.finding.name}@${candidate.version}` + const result = boundaryResults.resultByKey.get(key) + if (result) { + state.hadResponse = true + const active = activeVulnerabilities(result) + if (result.published !== false && active && active.length === 0) { + state.selected = { version: result.version, source: candidate.source } + break + } + } else if (boundaryResults.failedKeys.has(key)) { + state.hadFailure = true + } + } + } + } + + const latestRequests = [] + const seenLatest = new Set() + for (const state of activeStates.filter(item => !item.selected)) { + if (seenLatest.has(state.finding.name)) continue + seenLatest.add(state.finding.name) + latestRequests.push({ name: state.finding.name, version: 'latest' }) + } + + if (latestRequests.length > 0) { + candidateRequests += latestRequests.length + const latestResults = await queryRemediationRequests(latestRequests, 'latest') + candidatesChecked += latestResults.resultByKey.size + for (const state of activeStates.filter(item => !item.selected)) { + const result = latestResults.resultByKey.get(state.finding.name) + if (result) { + state.hadResponse = true + const active = activeVulnerabilities(result) + if (result.published !== false && parseVersion(result.version) && active && active.length === 0 && + compareVersions(result.version, state.finding.version) > 0) { + state.selected = { version: result.version, source: 'latest-fallback' } + } + } else if (latestResults.failedKeys.has(state.finding.name)) { + state.hadFailure = true + } + } + } + + const summary = { + candidateRequests, + candidatesChecked, + verified: 0, + unresolved: 0, + verificationFailed: 0, + notRequired: states.length - activeStates.length, + failures: { + total: Object.values(remediationFailureReasons).reduce((total, count) => total + count, 0), + byReason: Object.fromEntries(Object.entries(remediationFailureReasons).sort(([a], [b]) => a.localeCompare(b))) + }, + recovery: remediationRecovery + } + + for (const state of activeStates) { + if (state.selected) { + state.finding.remediation = { + status: 'ncm-verified', + version: state.selected.version, + source: state.selected.source, + changeType: versionChangeType(state.finding.version, state.selected.version) + } + summary.verified++ + } else if (!state.hadResponse && state.hadFailure) { + state.finding.remediation = { status: 'verification-failed' } + summary.verificationFailed++ + } else { + state.finding.remediation = { status: 'unresolved' } + summary.unresolved++ + } + } + + return summary + } + + async function processBatch (batch) { + try { + const packages = batch.map(({ name, version }) => ({ name, version })) + const outcome = await fetchWithRecovery(packages) + const results = outcome.results + unchecked += outcome.unresolved.length + for (const failure of outcome.failures) { + failedBatches++ + failureReasons[failure.reason] = (failureReasons[failure.reason] || 0) + 1 + } + + for (const pkg of results) { + const key = packageKey(pkg) + + if (pkg.published === false) { + unchecked++ + continue + } + + const scores = Array.isArray(pkg.scores) ? pkg.scores.filter(Boolean) : [] + const vulnerabilities = scores + .filter(score => score.group === 'security' && score.pass === false) + .map(compactVulnerability) + + if (vulnerabilities.length > 0) { + const licenseScore = scores.find(score => score.group === 'compliance' && score.name === 'license') + findings.push({ + name: pkg.name, + version: pkg.version, + direct: directByKey.get(key) === true, + severity: highestSeverity(vulnerabilities), + vulnerabilities, + license: licenseScore ? compactLicense(licenseScore) : null, + moduleRisks: scores + .filter(score => score.group === 'risk' && score.pass === false) + .map(score => compactAssessment(score, 'Untitled NCM module risk')), + codeQuality: scores + .filter(score => score.group === 'quality' && score.pass === false) + .map(score => compactAssessment(score, 'Untitled NCM quality issue')) + }) + } + } + } catch (error) { + const reason = classifyBatchFailure(error) + unchecked += batch.length + failedBatches++ + failureReasons[reason] = (failureReasons[reason] || 0) + 1 + process.stderr.write(`Warning: NCM batch failed (${reason}); ${batch.length} package(s) left unchecked\n`) + } finally { + completedBatches++ + processedPackages += batch.length + onProgress(`Audit progress: batches ${completedBatches}/${totalBatches}, packages ${processedPackages}/${requested.length}\n`) + } + } + + async function worker () { + while (nextBatch < totalBatches) { + const batch = collected.batches[nextBatch++] + await processBatch(batch) + } + } + + const workerCount = Math.min(MAX_CONCURRENCY, totalBatches) + await Promise.all(Array.from({ length: workerCount }, () => worker())) + + const remediation = await verifyFindingRemediations() + + findings.sort((a, b) => { + return (SEVERITY_RANK[b.severity] || 0) - (SEVERITY_RANK[a.severity] || 0) || + a.name.localeCompare(b.name) || a.version.localeCompare(b.version) + }) + + const bySeverity = { critical: 0, high: 0, medium: 0, low: 0, unknown: 0 } + let vulnerabilityCount = 0 + for (const finding of findings) { + for (const vulnerability of finding.vulnerabilities) { + const key = vulnerability.severity.toLowerCase() + bySeverity[key in bySeverity ? key : 'unknown']++ + vulnerabilityCount++ + } + } + + const visibleFindings = findings.slice(0, MAX_FINDINGS) + const byFailureReason = Object.fromEntries(Object.entries(failureReasons).sort(([a], [b]) => a.localeCompare(b))) + return { + packageManager: collected.packageManager, + packages: { + total: requested.length, + direct: collected.direct, + transitive: collected.transitive, + checked: Math.max(0, requested.length - unchecked), + unchecked + }, + vulnerabilities: { + total: vulnerabilityCount, + affectedPackages: findings.length, + bySeverity + }, + batchFailures: { + total: failedBatches, + byReason: byFailureReason + }, + batchRecovery: { + ...batchRecovery, + recoveredPackages: recoveredPackageKeys.size + }, + remediation, + findings: visibleFindings, + truncatedFindings: findings.length - visibleFindings.length + } +} + +if (require.main === module) { + const { dir } = parseArgs() + runAudit(dir, { onProgress: message => process.stderr.write(message) }) + .then(summary => process.stdout.write(JSON.stringify(summary) + '\n')) + .catch(error => { + process.stderr.write(`Error: ${error.message}\n`) + process.exitCode = 1 + }) +} + +module.exports = { classifyBatchFailure, compactVulnerability, fetchBatch, runAudit } diff --git a/skills/ns-audit-dependencies/collect-dependencies.cjs b/skills/ns-audit-dependencies/collect-dependencies.cjs index 47ba424..25b0c8a 100644 --- a/skills/ns-audit-dependencies/collect-dependencies.cjs +++ b/skills/ns-audit-dependencies/collect-dependencies.cjs @@ -141,7 +141,6 @@ function parsePnpm (content, directDeps) { let inPackages = false let currentName = '' let currentVersion = '' - let baseIndent = -1 const seen = new Set() const flush = () => { @@ -155,7 +154,6 @@ function parsePnpm (content, directDeps) { } currentName = '' currentVersion = '' - baseIndent = -1 } for (let i = 0; i < lines.length; i++) { @@ -185,7 +183,6 @@ function parsePnpm (content, directDeps) { flush() currentName = m[1] currentVersion = (m[2] || '').replace(/['"]$/, '') - baseIndent = line.search(/\S/) continue } @@ -227,13 +224,10 @@ function batch (arr, size) { // Main // --------------------------------------------------------------------------- -;(function main () { - const { dir } = parseArgs() - +function collectDependencies (dir) { const pkgJsonPath = path.join(dir, 'package.json') if (!fs.existsSync(pkgJsonPath)) { - process.stderr.write(`Error: package.json not found in ${dir}\n`) - process.exit(1) + throw new Error(`package.json not found in ${dir}`) } const directDeps = getDirectDeps(pkgJsonPath) @@ -292,12 +286,23 @@ function batch (arr, size) { const directCount = unique.filter(d => d.isDirect).length const transitiveCount = unique.length - directCount - const result = { + return { packageManager, direct: directCount, transitive: transitiveCount, batches: batch(unique, BATCH_SIZE) } +} + +if (require.main === module) { + try { + const { dir } = parseArgs() + const result = collectDependencies(dir) + process.stdout.write(JSON.stringify(result, null, 2) + '\n') + } catch (error) { + process.stderr.write(`Error: ${error.message}\n`) + process.exitCode = 1 + } +} - process.stdout.write(JSON.stringify(result, null, 2) + '\n') -})() +module.exports = { collectDependencies } From e729559c7d236c4a385a9e85efb88e6abae99dfd Mon Sep 17 00:00:00 2001 From: Cesar-M-Diaz Date: Fri, 17 Jul 2026 13:07:01 +0200 Subject: [PATCH 2/6] fix(audit): emit complete findings and harden NCM response handling - remove the 50-package findings limit - bound oversized NCM strings with truncation accounting - treat malformed score responses as unchecked - match resolved versions returned for latest requests - support SemVer build metadata in remediation checks - update audit instructions and regression coverage --- .../skills/audit-dependencies-script.test.ts | 354 +++++++++++++++++- skills/ns-audit-dependencies/SKILL.md | 8 +- .../audit-dependencies.cjs | 139 +++++-- 3 files changed, 446 insertions(+), 55 deletions(-) diff --git a/packages/core/test/unit/skills/audit-dependencies-script.test.ts b/packages/core/test/unit/skills/audit-dependencies-script.test.ts index 24128f7..dd5b738 100644 --- a/packages/core/test/unit/skills/audit-dependencies-script.test.ts +++ b/packages/core/test/unit/skills/audit-dependencies-script.test.ts @@ -144,6 +144,61 @@ test('audit helper only emits vulnerable dependency data', async () => { } }) +test('audit helper matches concrete NCM versions returned for latest dependency requests', async () => { + const projectDir = await mkdtemp(path.join(os.tmpdir(), 'nsolid-audit-')) + const primaryRequests: string[][] = [] + + try { + await writeFile(path.join(projectDir, 'package.json'), JSON.stringify({ + dependencies: { + 'star-dep': '*', + 'tag-dep': 'latest', + 'workspace-dep': 'workspace:*' + } + })) + + const summary = await runAudit(projectDir, { + token: 'test-token', + fetchBatch: async ( + _apiUrl: string, + _token: string, + packages: Array<{ name: string, version: string }>, + options: { phase?: string } + ) => { + if (options.phase !== 'remediation') primaryRequests.push(packages.map(pkg => pkg.version)) + return packages.map((pkg, index) => ({ + name: pkg.name, + version: `${options.phase === 'remediation' ? 2 : 1}.0.${index}`, + published: true, + scores: [{ + group: 'security', + pass: false, + severity: 'HIGH', + title: 'Known vulnerability' + }] + })) + } + }) as { + packages: Record + batchFailures: { total: number, byReason: Record } + batchRecovery: { missingPackageRetries: number } + findings: Array<{ name: string, version: string, direct: boolean }> + } + + assert.deepEqual(primaryRequests, [['latest', 'latest', 'latest']]) + assert.deepEqual(summary.packages, { total: 3, direct: 3, transitive: 0, checked: 3, unchecked: 0 }) + assert.deepEqual(summary.batchFailures, { total: 0, byReason: {} }) + assert.equal(summary.batchRecovery.missingPackageRetries, 0) + assert.deepEqual(summary.findings.map(({ name, version, direct }) => ({ name, version, direct })), [ + { name: 'star-dep', version: '1.0.0', direct: true }, + { name: 'tag-dep', version: '1.0.1', direct: true }, + { name: 'workspace-dep', version: '1.0.2', direct: true } + ]) + } finally { + await rm(projectDir, { recursive: true, force: true }) + } +}) + test('audit helper verifies a concrete latest fallback when no boundary candidate exists', async () => { const projectDir = await createNpmProject(1) const requestedVersions: string[] = [] @@ -190,6 +245,103 @@ test('audit helper verifies a concrete latest fallback when no boundary candidat } }) +test('audit helper accepts SemVer build metadata in latest remediation versions', async () => { + const projectDir = await createNpmProject(1) + + try { + for (const concreteVersion of ['2.0.0+build.1', '2.0.0-rc.1+build.1']) { + const summary = await runAudit(projectDir, { + token: 'test-token', + fetchBatch: async ( + _apiUrl: string, + _token: string, + packages: Array<{ name: string, version: string }>, + options: { phase?: string } + ) => { + if (options.phase === 'remediation') { + return packages.map(pkg => ({ name: pkg.name, version: concreteVersion, published: true, scores: [] })) + } + return packages.map(pkg => ({ + ...pkg, + published: true, + scores: [{ + group: 'security', + pass: false, + severity: 'HIGH', + title: 'Known vulnerability' + }] + })) + } + }) as { + remediation: { verified: number, unresolved: number } + findings: Array<{ remediation: Record }> + } + + assert.equal(summary.remediation.verified, 1) + assert.equal(summary.remediation.unresolved, 0) + assert.deepEqual(summary.findings[0].remediation, { + status: 'ncm-verified', + version: concreteVersion, + source: 'latest-fallback', + changeType: 'major' + }) + } + } finally { + await rm(projectDir, { recursive: true, force: true }) + } +}) + +test('audit helper calculates change type when the installed version has build metadata', async () => { + const projectDir = await mkdtemp(path.join(os.tmpdir(), 'nsolid-audit-')) + + try { + await writeFile(path.join(projectDir, 'package.json'), JSON.stringify({ + dependencies: { 'build-dep': '1.0.0+build.1' } + })) + await writeFile(path.join(projectDir, 'package-lock.json'), JSON.stringify({ + lockfileVersion: 3, + packages: { + '': { dependencies: { 'build-dep': '1.0.0+build.1' } }, + 'node_modules/build-dep': { version: '1.0.0+build.1' } + } + })) + + const summary = await runAudit(projectDir, { + token: 'test-token', + fetchBatch: async ( + _apiUrl: string, + _token: string, + packages: Array<{ name: string, version: string }>, + options: { phase?: string } + ) => { + if (options.phase === 'remediation') { + return packages.map(pkg => ({ ...pkg, published: true, scores: [] })) + } + return packages.map(pkg => ({ + ...pkg, + published: true, + scores: [{ + group: 'security', + pass: false, + severity: 'HIGH', + title: 'Known vulnerability', + data: { patched: ['>= 1.1.0'] } + }] + })) + } + }) as { findings: Array<{ remediation: Record }> } + + assert.deepEqual(summary.findings[0].remediation, { + status: 'ncm-verified', + version: '1.1.0', + source: 'patched-range-boundary', + changeType: 'minor' + }) + } finally { + await rm(projectDir, { recursive: true, force: true }) + } +}) + test('audit helper never emits the literal latest as a remediation version', async () => { const projectDir = await createNpmProject(1) @@ -261,6 +413,53 @@ test('remediation verification failure does not make an audited package unchecke } }) +test('invalid remediation scores cannot certify a candidate as clean', async () => { + const projectDir = await createNpmProject(1) + + try { + const summary = await runAudit(projectDir, { + token: 'test-token', + fetchBatch: async ( + _apiUrl: string, + _token: string, + packages: Array<{ name: string, version: string }>, + options: { phase?: string } + ) => { + if (options.phase === 'remediation') { + return packages.map(pkg => ({ ...pkg, published: true })) + } + return packages.map(pkg => ({ + ...pkg, + published: true, + scores: [{ + group: 'security', + pass: false, + severity: 'HIGH', + title: 'Known vulnerability', + data: { vulnerable: ['< 1.0.1'] } + }] + })) + } + }) as { + packages: Record + remediation: { + verified: number + verificationFailed: number + failures: { total: number, byReason: Record } + } + findings: Array<{ remediation: { status: string } }> + } + + assert.equal(summary.packages.checked, 1) + assert.equal(summary.remediation.verified, 0) + assert.equal(summary.remediation.verificationFailed, 1) + assert.deepEqual(summary.remediation.failures, { total: 2, byReason: { 'invalid-response': 2 } }) + assert.equal(summary.findings[0].remediation.status, 'verification-failed') + } finally { + await rm(projectDir, { recursive: true, force: true }) + } +}) + test('withdrawn-only findings do not trigger remediation verification', async () => { const projectDir = await createNpmProject(1) let requests = 0 @@ -665,6 +864,33 @@ test('audit helper retries omitted responses but not unpublished packages', asyn } }) +test('audit helper leaves packages with missing or non-array scores unchecked', async () => { + const projectDir = await createNpmProject(2) + + try { + const summary = await runAudit(projectDir, { + token: 'test-token', + fetchBatch: async (_apiUrl: string, _token: string, packages: Array<{ name: string, version: string }>) => { + return packages.map((pkg, index) => ({ + ...pkg, + published: true, + ...(index === 0 ? {} : { scores: {} }) + })) + } + }) as { + packages: Record + batchFailures: { total: number, byReason: Record } + findings: unknown[] + } + + assert.deepEqual(summary.packages, { total: 2, direct: 2, transitive: 0, checked: 0, unchecked: 2 }) + assert.deepEqual(summary.batchFailures, { total: 1, byReason: { 'invalid-response': 1 } }) + assert.deepEqual(summary.findings, []) + } finally { + await rm(projectDir, { recursive: true, force: true }) + } +}) + test('audit recovery remains within the two-request concurrency limit', async () => { const projectDir = await createNpmProject(205) const failedFullBatches = new Set() @@ -694,7 +920,7 @@ test('audit recovery remains within the two-request concurrency limit', async () } }) -test('audit helper preserves severity sorting and the 50-finding limit', async () => { +test('audit helper preserves severity sorting and emits every finding', async () => { const projectDir = await createNpmProject(55) try { @@ -704,26 +930,132 @@ test('audit helper preserves severity sorting and the 50-finding limit', async ( return packages.map((pkg, index) => ({ ...pkg, published: true, - scores: [{ - group: 'security', - pass: false, - severity: index === packages.length - 1 ? 'CRITICAL' : 'LOW', - title: index === 0 ? 'Withdrawn advisory' : 'Security issue' - }] + scores: [ + { + group: 'security', + pass: false, + severity: index === packages.length - 1 ? 'CRITICAL' : 'LOW', + title: index === 0 ? 'Withdrawn advisory' : 'Security issue' + }, + { + group: 'security', + pass: false, + severity: 'LOW', + title: 'Second security issue' + } + ] })) } }) as { vulnerabilities: { total: number, affectedPackages: number } findings: Array<{ severity: string, vulnerabilities: Array<{ withdrawn?: boolean }> }> - truncatedFindings: number } - assert.equal(summary.vulnerabilities.total, 55) + assert.equal(summary.vulnerabilities.total, 110) assert.equal(summary.vulnerabilities.affectedPackages, 55) - assert.equal(summary.findings.length, 50) + assert.equal(summary.findings.length, 55) assert.equal(summary.findings[0].severity, 'CRITICAL') assert.equal(summary.findings.some(finding => finding.vulnerabilities.some(item => item.withdrawn)), true) - assert.equal(summary.truncatedFindings, 5) + assert.equal('truncatedFindings' in summary, false) + } finally { + await rm(projectDir, { recursive: true, force: true }) + } +}) + +test('audit helper bounds NCM strings without dropping nested finding items', async () => { + const projectDir = await createNpmProject(1) + const long = 'x'.repeat(5000) + const vulnerableRanges = [`< 2.0.0 ${long}`, `< 3.0.0 ${long}`] + const patchedRanges = [`>= 4.0.0 ${long}`] + const latestVersion = `5.0.0-${long}` + const oversizedFields = [ + long, long, long, long, + ...vulnerableRanges, + ...patchedRanges, + long, long, long, long, + long, long, long, long, + long, + latestVersion + ] + + try { + const summary = await runAudit(projectDir, { + token: 'test-token', + fetchBatch: async ( + _apiUrl: string, + _token: string, + packages: Array<{ name: string, version: string }>, + options: { phase?: string } + ) => { + if (options.phase === 'remediation') { + return packages.map(pkg => pkg.version === 'latest' + ? { name: pkg.name, version: latestVersion, published: true, scores: [] } + : { + ...pkg, + published: true, + scores: [{ group: 'security', pass: false, severity: 'LOW', title: 'Still vulnerable' }] + }) + } + return packages.map(pkg => ({ + ...pkg, + published: true, + scores: [ + { + group: 'security', + pass: false, + severity: long, + title: long, + data: { cve: long, url: long, vulnerable: vulnerableRanges, patched: patchedRanges } + }, + { group: 'risk', pass: false, severity: long, title: long }, + { group: 'risk', pass: false, severity: long, title: long }, + { group: 'quality', pass: false, severity: long, title: long }, + { group: 'quality', pass: false, severity: long, title: long }, + { group: 'compliance', name: 'license', pass: true, data: { spdx: long } } + ] + })) + } + }) as { + ncmContentTruncation: { truncatedFields: number, truncatedCharacters: number } + findings: Array<{ + vulnerabilities: Array<{ + severity: string + title: string + id: string + url: string + vulnerable: string[] + patched: string[] + }> + license: { spdx: string } + moduleRisks: Array<{ severity: string, title: string }> + codeQuality: Array<{ severity: string, title: string }> + remediation: { status: string, version: string } + }> + } + + const finding = summary.findings[0] + assert.equal(finding.vulnerabilities.length, 1) + assert.equal(finding.vulnerabilities[0].vulnerable.length, 2) + assert.equal(finding.vulnerabilities[0].patched.length, 1) + assert.equal(finding.moduleRisks.length, 2) + assert.equal(finding.codeQuality.length, 2) + assert.equal(finding.remediation.status, 'ncm-verified') + for (const value of [ + finding.vulnerabilities[0].severity, + finding.vulnerabilities[0].title, + finding.vulnerabilities[0].id, + finding.vulnerabilities[0].url, + ...finding.vulnerabilities[0].vulnerable, + ...finding.vulnerabilities[0].patched, + ...finding.moduleRisks.flatMap(item => [item.severity, item.title]), + ...finding.codeQuality.flatMap(item => [item.severity, item.title]), + finding.license.spdx, + finding.remediation.version + ]) assert.equal(value.length, 4096) + assert.deepEqual(summary.ncmContentTruncation, { + truncatedFields: oversizedFields.length, + truncatedCharacters: oversizedFields.reduce((total, value) => total + value.length - 4096, 0) + }) } finally { await rm(projectDir, { recursive: true, force: true }) } diff --git a/skills/ns-audit-dependencies/SKILL.md b/skills/ns-audit-dependencies/SKILL.md index 50bc337..6e70480 100644 --- a/skills/ns-audit-dependencies/SKILL.md +++ b/skills/ns-audit-dependencies/SKILL.md @@ -10,7 +10,7 @@ description: >- **Otherwise (agent mode):** - Run the bundled audit helper from the project root (use the absolute path of the directory where you read this SKILL.md): - ``` + ```bash node "/audit-dependencies.cjs" --dir "$PWD" ``` - The helper requires outbound HTTPS access to `api.ncm.nodesource.com`. If the current command sandbox is known to block network access, request scoped escalation for this exact helper command before starting it. Let the harness ask the user for approval; never request broad or full-access sandbox permissions. @@ -26,9 +26,9 @@ Do not add N|Solid live enrichment to this static audit. If the user explicitly ### 2. Present an Audit Report Emit the audit directly in chat as markdown with these sections: -**Summary** — total packages checked, total vulnerabilities found (by severity), count of packages that could not be checked, aggregate terminal batch-failure reasons, and recovery counts from `batchRecovery` when any retries, splits, or omitted-response recovery occurred. Summarize `remediation` separately: verified targets, unresolved findings, verification failures, withdrawn-only findings requiring no action, and candidate-recovery/failure counts. A remediation failure does not make the installed package unchecked when its primary vulnerability scan succeeded. +**Summary** — total packages checked, total vulnerabilities found (by severity), count of packages that could not be checked, aggregate terminal batch-failure reasons, and recovery counts from `batchRecovery` when any retries, splits, or omitted-response recovery occurred. If `ncmContentTruncation.truncatedFields` is greater than zero, report both the number of oversized NCM fields and characters truncated. Summarize `remediation` separately: verified targets, unresolved findings, verification failures, withdrawn-only findings requiring no action, and candidate-recovery/failure counts. A remediation failure does not make the installed package unchecked when its primary vulnerability scan succeeded. -**Prioritized Findings** — sorted critical → high → medium → low. Render one separate finding block for every returned package version; never combine packages or versions into a single bullet. Include direct/transitive status and list every returned vulnerability with its severity, title, ID, URL, `vulnerable` ranges, and `patched` ranges when present. Mark advisories with `withdrawn: true` as withdrawn without silently removing them. Include the package's license, module risks, and code-quality issues from the finding. Render its remediation status exactly: `ncm-verified` with version/source/change type, `unresolved`, `verification-failed`, or `not-required` for withdrawn-only findings. Describe `ncm-verified` as free of active NCM advisories at audit time, not absolutely safe. If `truncatedFindings` is greater than zero, state exactly how many package findings were omitted. +**Prioritized Findings** — sorted critical → high → medium → low. Render one separate finding block for every returned package version; never combine packages or versions into a single bullet. Include direct/transitive status and list every returned vulnerability with its severity, title, ID, URL, `vulnerable` ranges, and `patched` ranges when present. Mark advisories with `withdrawn: true` as withdrawn without silently removing them. Include the package's license, module risks, and code-quality issues from the finding. Render its remediation status exactly: `ncm-verified` with version/source/change type, `unresolved`, `verification-failed`, or `not-required` for withdrawn-only findings. Describe `ncm-verified` as free of active NCM advisories at audit time, not absolutely safe. **Remediation Plan** — give package-specific commands for each direct critical/high finding only when `remediation.status` is `ncm-verified`, using its exact `remediation.version` and the detected package manager (npm / yarn / pnpm). Never emit the literal `@latest`. For `unresolved`, report the ranges as upgrade-selection evidence and require candidate verification before pinning. For `verification-failed`, say verification could not complete without weakening the vulnerability finding. For transitive findings, provide the appropriate `why` command and instruct the user to upgrade the introducing parent; use overrides/resolutions only temporarily with an `ncm-verified` version. @@ -44,5 +44,5 @@ Emit the audit directly in chat as markdown with these sections: - Never hallucinate CVE IDs, vulnerability titles, or severity levels. Use only what NCM returns. If a package cannot be checked, report it as unchecked rather than safe. - Never turn `latest-fallback` into `@latest`; use only the concrete NCM-returned `remediation.version`. Never infer a fixed version from a vulnerable range when remediation is unresolved. - When host-provided audit data is in the prompt, analyze only that data — do not re-fetch. -- Present every finding returned by the helper even when the report is long. The helper enforces the 50-package detail limit; do not compress its returned findings further. +- Present every finding returned by the helper even when the report is long; do not compress or omit findings. - Rollback reminder is mandatory in every audit response. diff --git a/skills/ns-audit-dependencies/audit-dependencies.cjs b/skills/ns-audit-dependencies/audit-dependencies.cjs index 2c241c4..c359ecd 100644 --- a/skills/ns-audit-dependencies/audit-dependencies.cjs +++ b/skills/ns-audit-dependencies/audit-dependencies.cjs @@ -7,7 +7,7 @@ const path = require('path') const { collectDependencies } = require('./collect-dependencies.cjs') const DEFAULT_API_URL = 'https://api.ncm.nodesource.com' -const MAX_FINDINGS = 50 +const MAX_NCM_STRING_LENGTH = 4096 const REQUEST_TIMEOUT_MS = 120000 const MAX_RETRIES = 2 const MAX_RECOVERY_RETRIES = 1 @@ -197,16 +197,38 @@ function packageKey (pkg) { return `${pkg.name}@${pkg.version}` } -function compactStringList (value) { +function matchingRequestKey (pkg, requestedKeys, requestedLatestNames) { + if (!pkg || typeof pkg.name !== 'string' || typeof pkg.version !== 'string') return null + const exactKey = packageKey(pkg) + if (requestedKeys.has(exactKey)) return exactKey + return requestedLatestNames.has(pkg.name) ? `${pkg.name}@latest` : null +} + +function truncateNcmString (value, truncation) { + if (value.length <= MAX_NCM_STRING_LENGTH) return value + + let end = MAX_NCM_STRING_LENGTH + const finalCodeUnit = value.charCodeAt(end - 1) + if (finalCodeUnit >= 0xD800 && finalCodeUnit <= 0xDBFF) end-- + const result = value.slice(0, end) + if (truncation) { + truncation.truncatedFields++ + truncation.truncatedCharacters += value.length - result.length + } + return result +} + +function compactStringList (value, truncation) { const values = Array.isArray(value) ? value : typeof value === 'string' ? [value] : [] return Array.from(new Set(values .filter(item => typeof item === 'string') .map(item => item.trim()) .filter(Boolean))) + .map(item => truncateNcmString(item, truncation)) } function parseVersion (value) { - const match = String(value || '').trim().match(/^v?(\d+)\.(\d+)\.(\d+)(?:-([0-9A-Za-z.-]+))?$/) + const match = String(value || '').trim().match(/^v?(\d+)\.(\d+)\.(\d+)(?:-([0-9A-Za-z.-]+))?(?:\+[0-9A-Za-z.-]+)?$/) if (!match) return null return { major: Number(match[1]), @@ -271,39 +293,40 @@ function activeVulnerabilities (pkg) { }) } -function compactVulnerability (score) { - const title = String(score.title || 'Untitled NCM vulnerability') +function compactVulnerability (score, truncation) { + const rawTitle = String(score.title || 'Untitled NCM vulnerability') + const title = truncateNcmString(rawTitle, truncation) const result = { - severity: String(score.severity || 'NONE').toUpperCase(), + severity: truncateNcmString(String(score.severity || 'NONE').toUpperCase(), truncation), title } const data = score.data if (data && typeof data === 'object') { const id = typeof data.cve === 'string' ? data.cve : typeof data.id === 'string' ? data.id : null - if (id) result.id = id - if (typeof data.url === 'string') result.url = data.url - const vulnerable = compactStringList(data.vulnerable) - const patched = compactStringList(data.patched) + if (id) result.id = truncateNcmString(id, truncation) + if (typeof data.url === 'string') result.url = truncateNcmString(data.url, truncation) + const vulnerable = compactStringList(data.vulnerable, truncation) + const patched = compactStringList(data.patched, truncation) if (vulnerable.length > 0) result.vulnerable = vulnerable if (patched.length > 0) result.patched = patched } - if (/\bwithdrawn\b/i.test(title)) result.withdrawn = true + if (/\bwithdrawn\b/i.test(rawTitle)) result.withdrawn = true return result } -function compactAssessment (score, fallbackTitle) { +function compactAssessment (score, fallbackTitle, truncation) { return { - severity: String(score.severity || 'NONE').toUpperCase(), - title: String(score.title || fallbackTitle) + severity: truncateNcmString(String(score.severity || 'NONE').toUpperCase(), truncation), + title: truncateNcmString(String(score.title || fallbackTitle), truncation) } } -function compactLicense (score) { +function compactLicense (score, truncation) { const data = score.data return { pass: score.pass === true, spdx: data && typeof data === 'object' && typeof data.spdx === 'string' - ? data.spdx + ? truncateNcmString(data.spdx, truncation) : null } } @@ -372,6 +395,10 @@ async function runAudit (dir, options = {}) { missingCandidateRetries: 0 } const remediationFailureReasons = {} + const ncmContentTruncation = { + truncatedFields: 0, + truncatedCharacters: 0 + } let unchecked = 0 let failedBatches = 0 @@ -388,6 +415,12 @@ async function runAudit (dir, options = {}) { process.stderr.write(message) } + function isDirectPackage (pkg) { + const exactKey = packageKey(pkg) + if (directByKey.has(exactKey)) return directByKey.get(exactKey) === true + return directByKey.get(`${pkg.name}@latest`) === true + } + function mergeResults (...groups) { const merged = new Map() for (const results of groups) { @@ -411,11 +444,10 @@ async function runAudit (dir, options = {}) { }) if (retried && Array.isArray(response)) { const requestedKeys = new Set(packages.map(packageKey)) + const requestedLatestNames = new Set(packages.filter(pkg => pkg.version === 'latest').map(pkg => pkg.name)) for (const pkg of response) { - if (pkg && typeof pkg.name === 'string' && typeof pkg.version === 'string' && - requestedKeys.has(packageKey(pkg))) { - recoveredPackageKeys.add(packageKey(pkg)) - } + const matchedKey = matchingRequestKey(pkg, requestedKeys, requestedLatestNames) + if (matchedKey) recoveredPackageKeys.add(matchedKey) } } return response @@ -429,6 +461,7 @@ async function runAudit (dir, options = {}) { ? MAX_RETRIES : recoveryOptions.maxRetries const requestedKeys = new Set(packages.map(packageKey)) + const requestedLatestNames = new Set(packages.filter(pkg => pkg.version === 'latest').map(pkg => pkg.name)) let results try { @@ -437,8 +470,7 @@ async function runAudit (dir, options = {}) { throw ncmError('NCM API returned an invalid packageVersions response', 'NCM_INVALID_RESPONSE') } results = mergeResults(response.filter(pkg => { - return pkg && typeof pkg.name === 'string' && typeof pkg.version === 'string' && - requestedKeys.has(packageKey(pkg)) + return matchingRequestKey(pkg, requestedKeys, requestedLatestNames) !== null })) } catch (error) { if (isRetryable(error) && depth < MAX_RECOVERY_DEPTH && packages.length > 1) { @@ -472,7 +504,9 @@ async function runAudit (dir, options = {}) { } } - const returnedKeys = new Set(results.map(packageKey)) + const returnedKeys = new Set(results + .map(pkg => matchingRequestKey(pkg, requestedKeys, requestedLatestNames)) + .filter(Boolean)) if (isRecovery) { for (const key of returnedKeys) recoveredPackageKeys.add(key) } @@ -548,10 +582,16 @@ async function runAudit (dir, options = {}) { } const requestedKeys = new Set(packages.map(requestKey)) + const invalidResponseKeys = new Set() for (const pkg of response) { if (!pkg || typeof pkg.name !== 'string' || typeof pkg.version !== 'string') continue const key = mode === 'latest' ? pkg.name : packageKey(pkg) - if (requestedKeys.has(key) && !resultByKey.has(key)) resultByKey.set(key, pkg) + if (!requestedKeys.has(key) || resultByKey.has(key)) continue + if (!Array.isArray(pkg.scores)) { + invalidResponseKeys.add(key) + continue + } + resultByKey.set(key, pkg) } const missing = packages.filter(pkg => !resultByKey.has(requestKey(pkg))) @@ -560,7 +600,10 @@ async function runAudit (dir, options = {}) { writeStatus(`Audit remediation recovery: retrying ${missing.length} omitted candidate response(s)\n`) await requestChunk(missing, { depth: MAX_RECOVERY_DEPTH, retryMissing: false }) } else if (missing.length > 0) { - recordFailures(missing, 'missing-response') + const invalid = missing.filter(pkg => invalidResponseKeys.has(requestKey(pkg))) + const omitted = missing.filter(pkg => !invalidResponseKeys.has(requestKey(pkg))) + if (invalid.length > 0) recordFailures(invalid, 'invalid-response') + if (omitted.length > 0) recordFailures(omitted, 'missing-response') } } @@ -625,7 +668,10 @@ async function runAudit (dir, options = {}) { state.hadResponse = true const active = activeVulnerabilities(result) if (result.published !== false && active && active.length === 0) { - state.selected = { version: result.version, source: candidate.source } + state.selected = { + version: truncateNcmString(result.version, ncmContentTruncation), + source: candidate.source + } break } } else if (boundaryResults.failedKeys.has(key)) { @@ -652,9 +698,13 @@ async function runAudit (dir, options = {}) { if (result) { state.hadResponse = true const active = activeVulnerabilities(result) - if (result.published !== false && parseVersion(result.version) && active && active.length === 0 && - compareVersions(result.version, state.finding.version) > 0) { - state.selected = { version: result.version, source: 'latest-fallback' } + const resultVersion = truncateNcmString(result.version) + if (result.published !== false && parseVersion(resultVersion) && active && active.length === 0 && + compareVersions(resultVersion, state.finding.version) > 0) { + state.selected = { + version: truncateNcmString(result.version, ncmContentTruncation), + source: 'latest-fallback' + } } } else if (latestResults.failedKeys.has(state.finding.name)) { state.hadFailure = true @@ -708,37 +758,47 @@ async function runAudit (dir, options = {}) { failureReasons[failure.reason] = (failureReasons[failure.reason] || 0) + 1 } + let invalidResponses = 0 for (const pkg of results) { - const key = packageKey(pkg) - if (pkg.published === false) { unchecked++ continue } - const scores = Array.isArray(pkg.scores) ? pkg.scores.filter(Boolean) : [] + if (!Array.isArray(pkg.scores)) { + unchecked++ + invalidResponses++ + continue + } + + const scores = pkg.scores.filter(Boolean) const vulnerabilities = scores .filter(score => score.group === 'security' && score.pass === false) - .map(compactVulnerability) + .map(score => compactVulnerability(score, ncmContentTruncation)) if (vulnerabilities.length > 0) { const licenseScore = scores.find(score => score.group === 'compliance' && score.name === 'license') findings.push({ name: pkg.name, version: pkg.version, - direct: directByKey.get(key) === true, + direct: isDirectPackage(pkg), severity: highestSeverity(vulnerabilities), vulnerabilities, - license: licenseScore ? compactLicense(licenseScore) : null, + license: licenseScore ? compactLicense(licenseScore, ncmContentTruncation) : null, moduleRisks: scores .filter(score => score.group === 'risk' && score.pass === false) - .map(score => compactAssessment(score, 'Untitled NCM module risk')), + .map(score => compactAssessment(score, 'Untitled NCM module risk', ncmContentTruncation)), codeQuality: scores .filter(score => score.group === 'quality' && score.pass === false) - .map(score => compactAssessment(score, 'Untitled NCM quality issue')) + .map(score => compactAssessment(score, 'Untitled NCM quality issue', ncmContentTruncation)) }) } } + if (invalidResponses > 0) { + failedBatches++ + failureReasons['invalid-response'] = (failureReasons['invalid-response'] || 0) + 1 + process.stderr.write(`Warning: NCM returned invalid scores for ${invalidResponses} package(s); left unchecked\n`) + } } catch (error) { const reason = classifyBatchFailure(error) unchecked += batch.length @@ -779,7 +839,6 @@ async function runAudit (dir, options = {}) { } } - const visibleFindings = findings.slice(0, MAX_FINDINGS) const byFailureReason = Object.fromEntries(Object.entries(failureReasons).sort(([a], [b]) => a.localeCompare(b))) return { packageManager: collected.packageManager, @@ -803,9 +862,9 @@ async function runAudit (dir, options = {}) { ...batchRecovery, recoveredPackages: recoveredPackageKeys.size }, + ncmContentTruncation, remediation, - findings: visibleFindings, - truncatedFindings: findings.length - visibleFindings.length + findings } } From d85e00557ff28ed6a3cca4f57afd9bd29478242b Mon Sep 17 00:00:00 2001 From: Cesar-M-Diaz Date: Fri, 17 Jul 2026 15:32:25 +0200 Subject: [PATCH 3/6] fix: make report deterministic --- .../audit-dependencies-renderer.test.ts | 121 ++++++++++ .../skills/audit-dependencies-script.test.ts | 56 ++++- skills/ns-audit-dependencies/SKILL.md | 49 ++-- .../audit-dependencies.cjs | 82 +++++-- .../render-audit-report.cjs | 210 ++++++++++++++++++ 5 files changed, 466 insertions(+), 52 deletions(-) create mode 100644 packages/core/test/unit/skills/audit-dependencies-renderer.test.ts create mode 100644 skills/ns-audit-dependencies/render-audit-report.cjs diff --git a/packages/core/test/unit/skills/audit-dependencies-renderer.test.ts b/packages/core/test/unit/skills/audit-dependencies-renderer.test.ts new file mode 100644 index 0000000..5392cf8 --- /dev/null +++ b/packages/core/test/unit/skills/audit-dependencies-renderer.test.ts @@ -0,0 +1,121 @@ +import assert from 'node:assert/strict' +import { createRequire } from 'node:module' +import test from 'node:test' + +const require = createRequire(import.meta.url) +const { renderAuditReport, validateAuditSummary } = require('../../../../../skills/ns-audit-dependencies/render-audit-report.cjs') as { + renderAuditReport: (summary: Record) => string + validateAuditSummary: (summary: Record) => Record +} + +function finding (index: number, vulnerabilityCount = 1): Record { + return { + name: index % 2 === 0 ? 'shared-package' : `package-${index}`, + version: `1.0.${index}`, + direct: index === 0, + severity: 'HIGH', + vulnerabilities: Array.from({ length: vulnerabilityCount }, (_, record) => ({ + severity: 'HIGH', + id: record < 2 ? 'GHSA-duplicate' : `GHSA-${index}-${record}`, + title: record < 2 ? 'Duplicate title' : `Issue ${record}`, + vulnerable: ['< 2.0.0'], + patched: ['>= 2.0.0'] + })), + license: { pass: true, spdx: 'MIT' }, + moduleRisks: [], + codeQuality: [], + remediation: index % 3 === 0 + ? { status: 'ncm-verified', version: '2.0.0', source: 'latest-fallback', changeType: 'major' } + : { status: 'unresolved' } + } +} + +function summaryFor (findings: Array>, uncheckedPackages: Array> = []): Record { + const vulnerabilityTotal = findings.reduce((total, item) => total + item.vulnerabilities.length, 0) + const verified = findings.filter(item => item.remediation.status === 'ncm-verified').length + const unresolved = findings.filter(item => item.remediation.status === 'unresolved').length + const uncheckedByReason: Record = {} + for (const item of uncheckedPackages) uncheckedByReason[item.reason] = (uncheckedByReason[item.reason] || 0) + 1 + return { + packageManager: 'pnpm', + packages: { + total: findings.length + uncheckedPackages.length, + direct: 1, + transitive: Math.max(0, findings.length - 1), + checked: findings.length, + unchecked: uncheckedPackages.length, + uncheckedByReason + }, + vulnerabilities: { + total: vulnerabilityTotal, + affectedPackages: findings.length, + bySeverity: { critical: 0, high: vulnerabilityTotal, medium: 0, low: 0, unknown: 0 } + }, + batchFailures: { total: 0, byReason: {} }, + batchRecovery: { transportRetries: 0, splitBatches: 0, missingPackageRetries: 0, recoveredPackages: 0 }, + ncmContentTruncation: { truncatedFields: 0, truncatedCharacters: 0 }, + remediation: { + candidateRequests: 0, + candidatesChecked: 0, + verified, + unresolved, + verificationFailed: 0, + notRequired: 0, + failures: { total: 0, byReason: {} }, + recovery: { transportRetries: 0, splitBatches: 0, missingCandidateRetries: 0 } + }, + uncheckedPackages, + findings + } +} + +test('renderer emits every finding beyond fifty and preserves package versions', () => { + const summary = summaryFor(Array.from({ length: 55 }, (_, index) => finding(index))) + const report = renderAuditReport(summary) + assert.equal((report.match(/^### Finding /gm) || []).length, 55) + assert.match(report, /Finding 55\/55/) + assert.match(report, /Rendered 55\/55 package-version findings, 55\/55 vulnerability records/) + assert.match(report, /shared-package@1\.0\.54/) +}) + +test('renderer reconciles a 36-finding and 57-record report including duplicate records', () => { + const findings = Array.from({ length: 36 }, (_, index) => finding(index, index < 21 ? 2 : 1)) + const report = renderAuditReport(summaryFor(findings)) + assert.equal((report.match(/^### Finding /gm) || []).length, 36) + assert.equal((report.match(/^ {2}- Record /gm) || []).length, 57) + assert.match(report, /Record 57\/57/) + assert.match(report, /Rendered 36\/36 package-version findings, 57\/57 vulnerability records/) + assert.equal((report.match(/GHSA-duplicate/g) || []).length, 57) +}) + +test('renderer lists every unchecked package and escapes untrusted Markdown', () => { + const unsafe = finding(0) + unsafe.vulnerabilities[0].title = '# injected\nheading | cell' + const summary = summaryFor([unsafe], [ + { name: 'z-package', version: '1.0.0', reason: 'server' }, + { name: 'a-package', version: '2.0.0', reason: 'missing-response' } + ]) + const report = renderAuditReport(summary) + assert.match(report, /a-package@2\.0\.0 — missing-response/) + assert.match(report, /z-package@1\.0\.0 — server/) + assert.doesNotMatch(report, /^# injected/m) + assert.match(report, /\\# injected heading \\| cell/) + assert.match(report, /2\/2 unchecked package versions/) +}) + +test('renderer withholds ambiguous upgrade commands and qualifies rollback behavior', () => { + const report = renderAuditReport(summaryFor([finding(0)])) + assert.match(report, /Command withheld because the owning manifest/) + assert.doesNotMatch(report, /pnpm add|npm install|yarn add|@latest/) + assert.match(report, /For manually executed package-manager commands, rely on version control/) +}) + +test('integrity validation rejects mismatched totals with a stable error code', () => { + const summary = summaryFor([finding(0)]) + summary.vulnerabilities.total = 2 + assert.throws(() => validateAuditSummary(summary), (error: any) => { + assert.equal(error.code, 'AUDIT_REPORT_INTEGRITY_ERROR') + assert.match(error.message, /vulnerability records/) + return true + }) +}) diff --git a/packages/core/test/unit/skills/audit-dependencies-script.test.ts b/packages/core/test/unit/skills/audit-dependencies-script.test.ts index dd5b738..bd9cb57 100644 --- a/packages/core/test/unit/skills/audit-dependencies-script.test.ts +++ b/packages/core/test/unit/skills/audit-dependencies-script.test.ts @@ -6,7 +6,7 @@ import path from 'node:path' import test from 'node:test' const require = createRequire(import.meta.url) -const { classifyBatchFailure, fetchBatch, runAudit } = require('../../../../../skills/ns-audit-dependencies/audit-dependencies.cjs') as { +const { classifyBatchFailure, fetchBatch, formatCliError, parseArgs, runAudit } = require('../../../../../skills/ns-audit-dependencies/audit-dependencies.cjs') as { classifyBatchFailure: (error: unknown) => string fetchBatch: ( apiUrl: string, @@ -14,9 +14,36 @@ const { classifyBatchFailure, fetchBatch, runAudit } = require('../../../../../s packages: Array<{ name: string, version: string }>, options?: Record ) => Promise>> + formatCliError: (error: Error & { code?: string }) => string + parseArgs: () => { dir: string, format: string } runAudit: (dir: string, options: Record) => Promise> } +test('audit helper preserves integrity error codes in CLI output', () => { + const error = Object.assign(new Error('summary totals differ'), { + code: 'AUDIT_REPORT_INTEGRITY_ERROR' + }) + assert.equal( + formatCliError(error), + 'Error: AUDIT_REPORT_INTEGRITY_ERROR: summary totals differ\n' + ) + assert.equal(formatCliError(new Error('ordinary failure')), 'Error: ordinary failure\n') +}) + +test('audit helper parses output formats and rejects unsupported values', () => { + const originalArgv = process.argv + try { + process.argv = ['node', 'audit-dependencies.cjs'] + assert.equal(parseArgs().format, 'json') + process.argv = ['node', 'audit-dependencies.cjs', '--format', 'markdown'] + assert.equal(parseArgs().format, 'markdown') + process.argv = ['node', 'audit-dependencies.cjs', '--format', 'html'] + assert.throws(() => parseArgs(), /Unsupported format: html/) + } finally { + process.argv = originalArgv + } +}) + async function createNpmProject (packageCount: number): Promise { const projectDir = await mkdtemp(path.join(os.tmpdir(), 'nsolid-audit-')) const packages: Record = { '': {} } @@ -113,7 +140,7 @@ test('audit helper only emits vulnerable dependency data', async () => { } const output = JSON.stringify(summary) - assert.deepEqual(summary.packages, { total: 2, direct: 1, transitive: 1, checked: 2, unchecked: 0 }) + assert.deepEqual(summary.packages, { total: 2, direct: 1, transitive: 1, checked: 2, unchecked: 0, uncheckedByReason: {} }) assert.equal(summary.vulnerabilities.total, 1) assert.equal(summary.vulnerabilities.affectedPackages, 1) assert.equal(summary.findings.length, 1) @@ -186,7 +213,7 @@ test('audit helper matches concrete NCM versions returned for latest dependency } assert.deepEqual(primaryRequests, [['latest', 'latest', 'latest']]) - assert.deepEqual(summary.packages, { total: 3, direct: 3, transitive: 0, checked: 3, unchecked: 0 }) + assert.deepEqual(summary.packages, { total: 3, direct: 3, transitive: 0, checked: 3, unchecked: 0, uncheckedByReason: {} }) assert.deepEqual(summary.batchFailures, { total: 0, byReason: {} }) assert.equal(summary.batchRecovery.missingPackageRetries, 0) assert.deepEqual(summary.findings.map(({ name, version, direct }) => ({ name, version, direct })), [ @@ -747,6 +774,7 @@ test('audit helper recovers an exhausted batch by splitting it once', async () = packages: Record batchFailures: { total: number, byReason: Record } batchRecovery: Record + uncheckedPackages: Array<{ name: string, version: string, reason: string }> } assert.deepEqual(requestSizes, [100, 50, 50]) @@ -783,10 +811,13 @@ test('audit helper leaves only an unrecovered split unchecked', async () => { packages: Record batchFailures: { total: number, byReason: Record } batchRecovery: Record + uncheckedPackages: Array<{ name: string, version: string, reason: string }> } assert.equal(summary.packages.checked, 50) assert.equal(summary.packages.unchecked, 50) + assert.equal(summary.uncheckedPackages.length, 50) + assert.ok(summary.uncheckedPackages.every(pkg => pkg.reason === 'server')) assert.deepEqual(summary.batchFailures, { total: 1, byReason: { server: 1 } }) assert.equal(summary.batchRecovery.recoveredPackages, 50) assert.doesNotMatch(JSON.stringify(summary), /PRIVATE_RAW_FAILURE/) @@ -817,6 +848,7 @@ test('audit helper bounds an exhausted batch to seven transport attempts', async packages: Record batchFailures: { total: number, byReason: Record } batchRecovery: Record + uncheckedPackages: Array<{ name: string, version: string, reason: string }> } assert.equal(attempts, 7) @@ -851,11 +883,15 @@ test('audit helper retries omitted responses but not unpublished packages', asyn packages: Record batchFailures: { total: number, byReason: Record } batchRecovery: Record + uncheckedPackages: Array<{ name: string, version: string, reason: string }> } assert.deepEqual(requestSizes, [10, 1]) assert.equal(summary.packages.checked, 9) assert.equal(summary.packages.unchecked, 1) + assert.deepEqual(summary.uncheckedPackages, [ + { name: 'package-0', version: '1.0.0', reason: 'unpublished' } + ]) assert.deepEqual(summary.batchFailures, { total: 0, byReason: {} }) assert.equal(summary.batchRecovery.missingPackageRetries, 1) assert.equal(summary.batchRecovery.recoveredPackages, 1) @@ -880,10 +916,22 @@ test('audit helper leaves packages with missing or non-array scores unchecked', }) as { packages: Record batchFailures: { total: number, byReason: Record } + uncheckedPackages: Array<{ name: string, version: string, reason: string }> findings: unknown[] } - assert.deepEqual(summary.packages, { total: 2, direct: 2, transitive: 0, checked: 0, unchecked: 2 }) + assert.deepEqual(summary.packages, { + total: 2, + direct: 2, + transitive: 0, + checked: 0, + unchecked: 2, + uncheckedByReason: { 'invalid-response': 2 } + }) + assert.deepEqual(summary.uncheckedPackages, [ + { name: 'package-0', version: '1.0.0', reason: 'invalid-response' }, + { name: 'package-1', version: '1.0.1', reason: 'invalid-response' } + ]) assert.deepEqual(summary.batchFailures, { total: 1, byReason: { 'invalid-response': 1 } }) assert.deepEqual(summary.findings, []) } finally { diff --git a/skills/ns-audit-dependencies/SKILL.md b/skills/ns-audit-dependencies/SKILL.md index 6e70480..4515cd8 100644 --- a/skills/ns-audit-dependencies/SKILL.md +++ b/skills/ns-audit-dependencies/SKILL.md @@ -4,45 +4,32 @@ description: >- Audits a local Node.js project dependency tree with NCM vulnerability data. Use when the user asks for an npm audit-style security review, package CVEs, direct/transitive vulnerability report, dependency risk assessment, or remediation plan before upgrading. For vulnerabilities actually loaded in running N|Solid processes, use ns-analyze-vulnerabilities instead. --- -### 1. Collect and Query +## Run the audit -**If grounded audit data is already provided in the prompt** (a `## Audit Results` block injected by the host), use it exclusively. Skip the bundled helper and live enrichment; the injected data is the complete source of truth. +If the prompt already contains host-provided audit data, use only that data. Treat it as complete only when it contains renderer-produced Markdown or an equivalent integrity-checked artifact. -**Otherwise (agent mode):** -- Run the bundled audit helper from the project root (use the absolute path of the directory where you read this SKILL.md): - ```bash - node "/audit-dependencies.cjs" --dir "$PWD" - ``` -- The helper requires outbound HTTPS access to `api.ncm.nodesource.com`. If the current command sandbox is known to block network access, request scoped escalation for this exact helper command before starting it. Let the harness ask the user for approval; never request broad or full-access sandbox permissions. -- The helper parses the lockfile, queries NCM in batches, retries transient failures, splits an exhausted batch once, retries omitted responses once, discards clean-package responses, and prints one compact JSON summary. For active findings it also verifies range-boundary candidates with NCM, then tests `latest` only as a fallback candidate and emits the concrete returned version when clean. Treat the summary as authoritative. -- Large audits and internal recovery can remain active while the helper reports compact progress, retries, and recovery on stderr. Poll the same execution session until it exits and returns the final JSON on stdout. -- Do not launch `wait.cjs`, start a second audit, or restart an audit automatically. If the original execution session is lost or terminated before it returns JSON, report the audit as incomplete. -- Do not call `getPackageVersions` or `getPackageQuality` after the helper succeeds. Raw dependency lists and clean NCM responses must stay outside model context. -- Inspect `batchFailures` before reporting results. If a completed attempt checked zero packages, left every package unchecked, and has only terminal `network` failures, request scoped network approval and rerun the exact helper command once; do not retry without approval. For `authentication`, direct the user to run `nsolid-plugin setup --harness `. For other failures, report the affected packages as unchecked; never describe them as safe. -- If the helper fails before returning a summary, report the error. Do not fall back to model-driven per-package MCP calls unless the user explicitly requests that slower fallback. +Otherwise, run the bundled helper from the project root using the absolute directory containing this `SKILL.md`: -Do not add N|Solid live enrichment to this static audit. If the user explicitly asks which findings are loaded in running processes, run `ns-analyze-vulnerabilities` as a separate workflow. +```bash +node "/audit-dependencies.cjs" --dir "$PWD" --format markdown +``` -### 2. Present an Audit Report -Emit the audit directly in chat as markdown with these sections: +The helper requires HTTPS access to `api.ncm.nodesource.com`. Request escalation scoped to this command when the sandbox blocks that access. It reports progress and recovery on stderr and emits the final, integrity-checked report on stdout. Poll the same session until it exits; do not start another audit or use model-driven per-package calls after the helper succeeds. -**Summary** — total packages checked, total vulnerabilities found (by severity), count of packages that could not be checked, aggregate terminal batch-failure reasons, and recovery counts from `batchRecovery` when any retries, splits, or omitted-response recovery occurred. If `ncmContentTruncation.truncatedFields` is greater than zero, report both the number of oversized NCM fields and characters truncated. Summarize `remediation` separately: verified targets, unresolved findings, verification failures, withdrawn-only findings requiring no action, and candidate-recovery/failure counts. A remediation failure does not make the installed package unchecked when its primary vulnerability scan succeeded. +If every package is unchecked solely because of terminal network failures, request scoped network approval and rerun the exact command once. For authentication failures, direct the user to run `nsolid-plugin setup --harness `. Otherwise report the audit as incomplete; never describe unchecked packages as safe. -**Prioritized Findings** — sorted critical → high → medium → low. Render one separate finding block for every returned package version; never combine packages or versions into a single bullet. Include direct/transitive status and list every returned vulnerability with its severity, title, ID, URL, `vulnerable` ranges, and `patched` ranges when present. Mark advisories with `withdrawn: true` as withdrawn without silently removing them. Include the package's license, module risks, and code-quality issues from the finding. Render its remediation status exactly: `ncm-verified` with version/source/change type, `unresolved`, `verification-failed`, or `not-required` for withdrawn-only findings. Describe `ncm-verified` as free of active NCM advisories at audit time, not absolutely safe. +## Present the report -**Remediation Plan** — give package-specific commands for each direct critical/high finding only when `remediation.status` is `ncm-verified`, using its exact `remediation.version` and the detected package manager (npm / yarn / pnpm). Never emit the literal `@latest`. For `unresolved`, report the ranges as upgrade-selection evidence and require candidate verification before pinning. For `verification-failed`, say verification could not complete without weakening the vulnerability finding. For transitive findings, provide the appropriate `why` command and instruct the user to upgrade the introducing parent; use overrides/resolutions only temporarily with an `ncm-verified` version. +Treat successful Markdown stdout as authoritative. Present it without rewriting, regrouping, reordering, or omitting findings. Keep progress-only stderr outside the report because its aggregates are already rendered. -**Breaking Change Notes** — use `remediation.changeType` to flag major-version bumps required to fix vulnerabilities; remind the user to review official changelogs. +If the helper reports `AUDIT_REPORT_INTEGRITY_ERROR`, state that the audit report is incomplete. Do not reconstruct a report from partial output. -**Rollback Guidance** — N|Solid backs up `package.json` and the lockfile to `.nsolid/backup/` before each upgrade. If an upgrade causes issues, the user can click "Rollback" in the post-upgrade notification or restore manually from that directory. - -### 3. Write the Report to Disk -- Ask the user if they want to save the report to disk. -- If the user confirms, write the final audit report as a markdown file (`.md`) under `.nsolid/assets/` — for example `.nsolid/assets/dependency-audit-.md`. +After presenting the complete report, ask whether the user wants it saved as Markdown under `.nsolid/assets/`. ## Guardrails -- Never hallucinate CVE IDs, vulnerability titles, or severity levels. Use only what NCM returns. If a package cannot be checked, report it as unchecked rather than safe. -- Never turn `latest-fallback` into `@latest`; use only the concrete NCM-returned `remediation.version`. Never infer a fixed version from a vulnerable range when remediation is unresolved. -- When host-provided audit data is in the prompt, analyze only that data — do not re-fetch. -- Present every finding returned by the helper even when the report is long; do not compress or omit findings. -- Rollback reminder is mandatory in every audit response. + +- This is a static dependency audit. Use `ns-analyze-vulnerabilities` separately only when the user asks which findings are loaded in running N|Solid processes. +- Never invent CVE IDs, severities, fixed versions, or package ownership. +- `ncm-verified` means free of active NCM advisories at audit time, not absolutely safe. +- Never convert a concrete `latest-fallback` result into `@latest`. +- JSON remains available for programmatic callers by omitting `--format markdown` or using `--format json`. diff --git a/skills/ns-audit-dependencies/audit-dependencies.cjs b/skills/ns-audit-dependencies/audit-dependencies.cjs index c359ecd..5dd2793 100644 --- a/skills/ns-audit-dependencies/audit-dependencies.cjs +++ b/skills/ns-audit-dependencies/audit-dependencies.cjs @@ -5,6 +5,7 @@ const fs = require('fs') const os = require('os') const path = require('path') const { collectDependencies } = require('./collect-dependencies.cjs') +const { renderAuditReport } = require('./render-audit-report.cjs') const DEFAULT_API_URL = 'https://api.ncm.nodesource.com' const MAX_NCM_STRING_LENGTH = 4096 @@ -48,12 +49,24 @@ const SEVERITY_RANK = { function parseArgs () { const args = process.argv.slice(2) let dir = process.cwd() + let format = 'json' for (let i = 0; i < args.length; i++) { if (args[i] === '--dir' && args[i + 1]) dir = path.resolve(args[++i]) + else if (args[i] === '--format' && args[i + 1]) format = args[++i] } - return { dir } + if (!['json', 'markdown'].includes(format)) { + throw new Error(`Unsupported format: ${format}. Expected json or markdown.`) + } + return { dir, format } +} + +function formatCliError (error) { + const code = error && typeof error.code === 'string' && error.code.length > 0 + ? `${error.code}: ` + : '' + return `Error: ${code}${error.message}\n` } function loadToken () { @@ -399,7 +412,7 @@ async function runAudit (dir, options = {}) { truncatedFields: 0, truncatedCharacters: 0 } - let unchecked = 0 + const uncheckedByKey = new Map() let failedBatches = 0 const token = requested.length > 0 ? options.token || loadToken() : '' @@ -421,6 +434,24 @@ async function runAudit (dir, options = {}) { return directByKey.get(`${pkg.name}@latest`) === true } + function originalRequest (pkg) { + const exact = requested.find(item => packageKey(item) === packageKey(pkg)) + if (exact) return exact + return requested.find(item => item.name === pkg.name && item.version === 'latest') || pkg + } + + function recordUnchecked (pkg, reason) { + const requestedPackage = originalRequest(pkg) + const key = packageKey(requestedPackage) + if (!uncheckedByKey.has(key)) { + uncheckedByKey.set(key, { + name: requestedPackage.name, + version: pkg && pkg.version && requestedPackage.version === 'latest' ? pkg.version : requestedPackage.version, + reason + }) + } + } + function mergeResults (...groups) { const merged = new Map() for (const results of groups) { @@ -499,7 +530,7 @@ async function runAudit (dir, options = {}) { return { results: [], - unresolved: packages, + unresolved: packages.map(pkg => ({ pkg, reason: classifyBatchFailure(error) })), failures: [{ reason: classifyBatchFailure(error) }] } } @@ -530,7 +561,7 @@ async function runAudit (dir, options = {}) { return { results, - unresolved: missing, + unresolved: missing.map(pkg => ({ pkg, reason: 'missing-response' })), failures: missing.length > 0 ? [{ reason: 'missing-response' }] : [] } } @@ -752,7 +783,7 @@ async function runAudit (dir, options = {}) { const packages = batch.map(({ name, version }) => ({ name, version })) const outcome = await fetchWithRecovery(packages) const results = outcome.results - unchecked += outcome.unresolved.length + for (const unresolved of outcome.unresolved) recordUnchecked(unresolved.pkg, unresolved.reason) for (const failure of outcome.failures) { failedBatches++ failureReasons[failure.reason] = (failureReasons[failure.reason] || 0) + 1 @@ -761,12 +792,12 @@ async function runAudit (dir, options = {}) { let invalidResponses = 0 for (const pkg of results) { if (pkg.published === false) { - unchecked++ + recordUnchecked(pkg, 'unpublished') continue } if (!Array.isArray(pkg.scores)) { - unchecked++ + recordUnchecked(pkg, 'invalid-response') invalidResponses++ continue } @@ -801,7 +832,7 @@ async function runAudit (dir, options = {}) { } } catch (error) { const reason = classifyBatchFailure(error) - unchecked += batch.length + for (const pkg of batch) recordUnchecked(pkg, reason) failedBatches++ failureReasons[reason] = (failureReasons[reason] || 0) + 1 process.stderr.write(`Warning: NCM batch failed (${reason}); ${batch.length} package(s) left unchecked\n`) @@ -840,6 +871,13 @@ async function runAudit (dir, options = {}) { } const byFailureReason = Object.fromEntries(Object.entries(failureReasons).sort(([a], [b]) => a.localeCompare(b))) + const uncheckedPackages = Array.from(uncheckedByKey.values()) + .sort((a, b) => a.reason.localeCompare(b.reason) || a.name.localeCompare(b.name) || a.version.localeCompare(b.version)) + const uncheckedByReason = {} + for (const pkg of uncheckedPackages) { + uncheckedByReason[pkg.reason] = (uncheckedByReason[pkg.reason] || 0) + 1 + } + const unchecked = uncheckedPackages.length return { packageManager: collected.packageManager, packages: { @@ -847,7 +885,8 @@ async function runAudit (dir, options = {}) { direct: collected.direct, transitive: collected.transitive, checked: Math.max(0, requested.length - unchecked), - unchecked + unchecked, + uncheckedByReason: Object.fromEntries(Object.entries(uncheckedByReason).sort(([a], [b]) => a.localeCompare(b))) }, vulnerabilities: { total: vulnerabilityCount, @@ -864,18 +903,27 @@ async function runAudit (dir, options = {}) { }, ncmContentTruncation, remediation, + uncheckedPackages, findings } } if (require.main === module) { - const { dir } = parseArgs() - runAudit(dir, { onProgress: message => process.stderr.write(message) }) - .then(summary => process.stdout.write(JSON.stringify(summary) + '\n')) - .catch(error => { - process.stderr.write(`Error: ${error.message}\n`) - process.exitCode = 1 - }) + let args + try { + args = parseArgs() + } catch (error) { + process.stderr.write(formatCliError(error)) + process.exitCode = 1 + } + if (args) { + runAudit(args.dir, { onProgress: message => process.stderr.write(message) }) + .then(summary => process.stdout.write(args.format === 'markdown' ? `${renderAuditReport(summary)}\n` : `${JSON.stringify(summary)}\n`)) + .catch(error => { + process.stderr.write(formatCliError(error)) + process.exitCode = 1 + }) + } } -module.exports = { classifyBatchFailure, compactVulnerability, fetchBatch, runAudit } +module.exports = { classifyBatchFailure, compactVulnerability, fetchBatch, formatCliError, parseArgs, runAudit } diff --git a/skills/ns-audit-dependencies/render-audit-report.cjs b/skills/ns-audit-dependencies/render-audit-report.cjs new file mode 100644 index 0000000..60fead5 --- /dev/null +++ b/skills/ns-audit-dependencies/render-audit-report.cjs @@ -0,0 +1,210 @@ +'use strict' + +const SEVERITIES = ['critical', 'high', 'medium', 'low', 'unknown'] +const REMEDIATION_KEYS = { + 'ncm-verified': 'verified', + unresolved: 'unresolved', + 'verification-failed': 'verificationFailed', + 'not-required': 'notRequired' +} + +function integrityError (message) { + const error = new Error(`Audit report integrity check failed: ${message}`) + error.code = 'AUDIT_REPORT_INTEGRITY_ERROR' + return error +} + +function stableCounts (values) { + return Object.fromEntries(Object.entries(values).sort(([a], [b]) => a.localeCompare(b))) +} + +function assertEqual (actual, expected, label) { + if (actual !== expected) throw integrityError(`${label}: expected ${expected}, received ${actual}`) +} + +function assertCounts (actual, expected, label) { + const keys = new Set([...Object.keys(actual || {}), ...Object.keys(expected || {})]) + for (const key of keys) assertEqual(actual[key] || 0, expected[key] || 0, `${label}.${key}`) +} + +function validateAuditSummary (summary) { + if (!summary || !Array.isArray(summary.findings) || !Array.isArray(summary.uncheckedPackages)) { + throw integrityError('missing findings or uncheckedPackages collection') + } + + const vulnerabilityCounts = Object.fromEntries(SEVERITIES.map(severity => [severity, 0])) + const remediationCounts = { verified: 0, unresolved: 0, verificationFailed: 0, notRequired: 0 } + let vulnerabilityTotal = 0 + + for (const finding of summary.findings) { + if (!Array.isArray(finding.vulnerabilities)) throw integrityError('finding has no vulnerability collection') + const key = REMEDIATION_KEYS[finding.remediation && finding.remediation.status] + if (!key) throw integrityError(`unknown remediation status for ${finding.name || 'unknown package'}`) + remediationCounts[key]++ + for (const vulnerability of finding.vulnerabilities) { + const severity = String(vulnerability.severity || '').toLowerCase() + vulnerabilityCounts[SEVERITIES.includes(severity) ? severity : 'unknown']++ + vulnerabilityTotal++ + } + } + + assertEqual(summary.findings.length, summary.vulnerabilities.affectedPackages, 'affected package versions') + assertEqual(vulnerabilityTotal, summary.vulnerabilities.total, 'vulnerability records') + assertCounts(vulnerabilityCounts, summary.vulnerabilities.bySeverity, 'severity') + assertCounts(remediationCounts, { + verified: summary.remediation.verified, + unresolved: summary.remediation.unresolved, + verificationFailed: summary.remediation.verificationFailed, + notRequired: summary.remediation.notRequired + }, 'remediation') + + const uncheckedCounts = {} + for (const pkg of summary.uncheckedPackages) { + uncheckedCounts[pkg.reason] = (uncheckedCounts[pkg.reason] || 0) + 1 + } + assertEqual(summary.packages.checked + summary.packages.unchecked, summary.packages.total, 'package coverage') + assertEqual(summary.uncheckedPackages.length, summary.packages.unchecked, 'unchecked package versions') + assertCounts(stableCounts(uncheckedCounts), summary.packages.uncheckedByReason, 'unchecked reason') + + return { + findings: summary.findings.length, + vulnerabilities: vulnerabilityTotal, + unchecked: summary.uncheckedPackages.length + } +} + +function escapeMarkdown (value) { + return String(value == null ? '' : value) + .split('') + .map(character => { + const code = character.charCodeAt(0) + return (code <= 8 || code === 11 || code === 12 || (code >= 14 && code <= 31) || code === 127) + ? '\uFFFD' + : character + }) + .join('') + .replace(/\\/g, '\\\\') + .replace(/([`*_{}[\]()<>#|])/g, '\\$1') + .replace(/\r\n?|\n/g, ' ') +} + +function valueOrNotReturned (value) { + return value == null || value === '' ? 'not returned' : escapeMarkdown(value) +} + +function listOrNotReturned (value) { + return Array.isArray(value) && value.length > 0 + ? value.map(escapeMarkdown).join('; ') + : 'not returned' +} + +function countsText (counts) { + const entries = Object.entries(counts || {}).filter(([, count]) => count > 0) + return entries.length > 0 ? entries.map(([key, count]) => `${count} ${escapeMarkdown(key)}`).join(', ') : 'none' +} + +function renderFinding (finding, index, findingTotal, recordState, recordTotal) { + const lines = [ + `### Finding ${index + 1}/${findingTotal}: ${escapeMarkdown(finding.name)}@${escapeMarkdown(finding.version)}`, + '', + `- Severity: ${escapeMarkdown(finding.severity)}`, + `- Dependency: ${finding.direct ? 'direct' : 'transitive'}`, + `- License: ${finding.license && finding.license.spdx ? escapeMarkdown(finding.license.spdx) : 'not returned'}`, + '- Vulnerability records:' + ] + + for (const vulnerability of finding.vulnerabilities) { + recordState.count++ + const withdrawn = vulnerability.withdrawn ? ' — withdrawn' : '' + lines.push(` - Record ${recordState.count}/${recordTotal} — ${escapeMarkdown(vulnerability.severity)} — ${valueOrNotReturned(vulnerability.id)} — ${escapeMarkdown(vulnerability.title)}${withdrawn}`) + lines.push(` - Withdrawn: ${vulnerability.withdrawn ? 'yes' : 'no'}`) + lines.push(` - URL: ${valueOrNotReturned(vulnerability.url)}`) + lines.push(` - Vulnerable ranges: ${listOrNotReturned(vulnerability.vulnerable)}`) + lines.push(` - Patched ranges: ${listOrNotReturned(vulnerability.patched)}`) + } + + lines.push('- Module risks:') + if (finding.moduleRisks.length === 0) lines.push(' - none returned') + for (const risk of finding.moduleRisks) lines.push(` - ${escapeMarkdown(risk.severity)} — ${escapeMarkdown(risk.title)}`) + lines.push('- Code-quality issues:') + if (finding.codeQuality.length === 0) lines.push(' - none returned') + for (const issue of finding.codeQuality) lines.push(` - ${escapeMarkdown(issue.severity)} — ${escapeMarkdown(issue.title)}`) + + const remediation = finding.remediation + if (remediation.status === 'ncm-verified') { + lines.push(`- Remediation: ncm-verified → ${escapeMarkdown(remediation.version)} (${escapeMarkdown(remediation.source)}, ${escapeMarkdown(remediation.changeType)})`) + } else if (remediation.status === 'not-required') { + lines.push(`- Remediation: not-required (${escapeMarkdown(remediation.reason || 'withdrawn-only')})`) + } else { + lines.push(`- Remediation: ${escapeMarkdown(remediation.status)}`) + } + return lines.join('\n') +} + +function renderRemediationPlan (summary) { + const lines = ['## Remediation Plan', ''] + const verified = summary.findings.filter(finding => finding.remediation.status === 'ncm-verified') + if (verified.length === 0) { + lines.push('No NCM-verified upgrade targets were found.') + } else { + for (const finding of verified) { + lines.push(`- ${escapeMarkdown(finding.name)}@${escapeMarkdown(finding.version)} → verified target ${escapeMarkdown(finding.name)}@${escapeMarkdown(finding.remediation.version)}.`) + lines.push(' Command withheld because the owning manifest and resolved direct version are not proven unambiguous; locate the declaration before upgrading.') + if (!finding.direct) lines.push(` Locate the introducing parent with \`${escapeMarkdown(summary.packageManager)} why ${escapeMarkdown(finding.name)}\`.`) + } + } + const unresolved = summary.findings.filter(finding => finding.remediation.status === 'unresolved').length + const failed = summary.findings.filter(finding => finding.remediation.status === 'verification-failed').length + if (unresolved > 0) lines.push(`- ${unresolved} finding(s) remain unresolved; use reported ranges as evidence, but verify a candidate with NCM before pinning.`) + if (failed > 0) lines.push(`- ${failed} finding(s) could not complete remediation verification; this does not weaken the vulnerability finding.`) + return lines.join('\n') +} + +function renderAuditReport (summary) { + const counts = validateAuditSummary(summary) + const severity = summary.vulnerabilities.bySeverity + const lines = [ + '## Summary', + '', + `- ${summary.packages.checked} of ${summary.packages.total} package versions checked; ${summary.packages.unchecked} unchecked.`, + `- ${summary.vulnerabilities.total} vulnerability records across ${summary.vulnerabilities.affectedPackages} affected package versions: ${severity.critical} critical, ${severity.high} high, ${severity.medium} medium, ${severity.low} low, ${severity.unknown} unknown.`, + `- Terminal batch failures: ${summary.batchFailures.total} (${countsText(summary.batchFailures.byReason)}). Recovery: ${summary.batchRecovery.transportRetries} transport retries, ${summary.batchRecovery.splitBatches} batch splits, ${summary.batchRecovery.missingPackageRetries} omitted-response retries, ${summary.batchRecovery.recoveredPackages} recovered package versions.`, + `- Remediation: ${summary.remediation.verified} NCM-verified, ${summary.remediation.unresolved} unresolved, ${summary.remediation.verificationFailed} verification failures, ${summary.remediation.notRequired} not required. Candidate failures: ${summary.remediation.failures.total} (${countsText(summary.remediation.failures.byReason)}).`, + `- Remediation recovery: ${summary.remediation.recovery.transportRetries} transport retries, ${summary.remediation.recovery.splitBatches} batch splits, ${summary.remediation.recovery.missingCandidateRetries} omitted-candidate retries.` + ] + if (summary.ncmContentTruncation.truncatedFields > 0) { + lines.push(`- NCM content truncation: ${summary.ncmContentTruncation.truncatedFields} fields and ${summary.ncmContentTruncation.truncatedCharacters} characters truncated.`) + } else { + lines.push('- No NCM field truncation occurred.') + } + lines.push('', '“NCM-verified” means free of active NCM advisories at audit time—not absolutely safe.') + lines.push('', 'This is a static dependency audit; it does not establish runtime loading, reachability, or exploitability.') + + if (summary.uncheckedPackages.length > 0) { + lines.push('', '## Coverage Gaps', '') + for (const pkg of summary.uncheckedPackages) { + lines.push(`- ${escapeMarkdown(pkg.name)}@${escapeMarkdown(pkg.version)} — ${escapeMarkdown(pkg.reason)}`) + } + } + + lines.push('', '## Prioritized Findings', '') + if (summary.findings.length === 0) lines.push('No vulnerable package versions were returned.') + const recordState = { count: 0 } + summary.findings.forEach((finding, index) => { + if (index > 0) lines.push('') + lines.push(renderFinding(finding, index, counts.findings, recordState, counts.vulnerabilities)) + }) + + lines.push('', renderRemediationPlan(summary), '', '## Breaking Change Notes', '') + const majors = summary.findings.filter(finding => finding.remediation.status === 'ncm-verified' && finding.remediation.changeType === 'major') + if (majors.length === 0) lines.push('No NCM-verified major-version remediation targets were found.') + else lines.push(`Major upgrades require changelog and compatibility review: ${majors.map(finding => `${escapeMarkdown(finding.name)} → ${escapeMarkdown(finding.remediation.version)}`).join(', ')}.`) + + lines.push('', '## Rollback Guidance', '') + lines.push('The N|Solid upgrade workflow backs up `package.json` and the lockfile under `.nsolid/backup/`. For manually executed package-manager commands, rely on version control or create a separate backup first.') + lines.push('', '## Report Integrity', '') + lines.push(`Rendered ${counts.findings}/${summary.vulnerabilities.affectedPackages} package-version findings, ${recordState.count}/${summary.vulnerabilities.total} vulnerability records, and ${counts.unchecked}/${summary.packages.unchecked} unchecked package versions.`) + return lines.join('\n') +} + +module.exports = { escapeMarkdown, renderAuditReport, validateAuditSummary } From 6df65349befac8c35cbfced0e387f88df362bbf7 Mon Sep 17 00:00:00 2001 From: Cesar-M-Diaz Date: Fri, 17 Jul 2026 16:26:50 +0200 Subject: [PATCH 4/6] fix: report details --- .../audit-dependencies-renderer.test.ts | 107 ++++++- .../render-audit-report.cjs | 291 ++++++++++++++---- 2 files changed, 340 insertions(+), 58 deletions(-) diff --git a/packages/core/test/unit/skills/audit-dependencies-renderer.test.ts b/packages/core/test/unit/skills/audit-dependencies-renderer.test.ts index 5392cf8..c78a419 100644 --- a/packages/core/test/unit/skills/audit-dependencies-renderer.test.ts +++ b/packages/core/test/unit/skills/audit-dependencies-renderer.test.ts @@ -32,8 +32,17 @@ function finding (index: number, vulnerabilityCount = 1): Record { function summaryFor (findings: Array>, uncheckedPackages: Array> = []): Record { const vulnerabilityTotal = findings.reduce((total, item) => total + item.vulnerabilities.length, 0) + const bySeverity = { critical: 0, high: 0, medium: 0, low: 0, unknown: 0 } + for (const item of findings) { + for (const vulnerability of item.vulnerabilities) { + const severity = String(vulnerability.severity || '').toLowerCase() + bySeverity[severity in bySeverity ? severity as keyof typeof bySeverity : 'unknown']++ + } + } const verified = findings.filter(item => item.remediation.status === 'ncm-verified').length const unresolved = findings.filter(item => item.remediation.status === 'unresolved').length + const verificationFailed = findings.filter(item => item.remediation.status === 'verification-failed').length + const notRequired = findings.filter(item => item.remediation.status === 'not-required').length const uncheckedByReason: Record = {} for (const item of uncheckedPackages) uncheckedByReason[item.reason] = (uncheckedByReason[item.reason] || 0) + 1 return { @@ -49,7 +58,7 @@ function summaryFor (findings: Array>, uncheckedPackages: Ar vulnerabilities: { total: vulnerabilityTotal, affectedPackages: findings.length, - bySeverity: { critical: 0, high: vulnerabilityTotal, medium: 0, low: 0, unknown: 0 } + bySeverity }, batchFailures: { total: 0, byReason: {} }, batchRecovery: { transportRetries: 0, splitBatches: 0, missingPackageRetries: 0, recoveredPackages: 0 }, @@ -59,8 +68,8 @@ function summaryFor (findings: Array>, uncheckedPackages: Ar candidatesChecked: 0, verified, unresolved, - verificationFailed: 0, - notRequired: 0, + verificationFailed, + notRequired, failures: { total: 0, byReason: {} }, recovery: { transportRetries: 0, splitBatches: 0, missingCandidateRetries: 0 } }, @@ -88,6 +97,55 @@ test('renderer reconciles a 36-finding and 57-record report including duplicate assert.equal((report.match(/GHSA-duplicate/g) || []).length, 57) }) +test('renderer leads with active risk and partitions a 36-finding, 57-record report', () => { + const activeSeverities = [ + ...Array(5).fill('CRITICAL'), + ...Array(15).fill('HIGH'), + ...Array(22).fill('MEDIUM'), + ...Array(9).fill('LOW') + ] + let activeRecord = 0 + const findings = Array.from({ length: 36 }, (_, index) => { + const vulnerabilityCount = index < 19 || index === 32 || index === 33 ? 2 : 1 + const item = finding(index, vulnerabilityCount) + if (index < 32) { + for (const vulnerability of item.vulnerabilities) vulnerability.severity = activeSeverities[activeRecord++] + } else { + const withdrawnSeverities = index < 34 ? ['HIGH', 'HIGH'] : [index === 34 ? 'MEDIUM' : 'LOW'] + item.vulnerabilities.forEach((vulnerability: Record, record: number) => { + vulnerability.severity = withdrawnSeverities[record] + vulnerability.withdrawn = true + }) + item.remediation = { status: 'not-required', reason: 'withdrawn-only' } + } + return item + }) + const report = renderAuditReport(summaryFor(findings)) + assert.match(report, /51 active vulnerability records across 32 actively affected package versions: 5 critical, 15 high, 22 medium, 9 low, 0 unknown/) + assert.match(report, /57 vulnerability records across 36 affected package versions, including 6 withdrawn records and 4 withdrawn-only package versions/) + assert.match(report, /Finding partition: 32\/32 active and 4\/4 withdrawn-only/) + assert.match(report, /Record partition: 51\/51 active and 6\/6 withdrawn/) + assert.ok(report.indexOf('## Active Findings') < report.indexOf('## Withdrawn-Only Findings')) +}) + +test('renderer prioritizes mixed findings by active severity and keeps withdrawn records', () => { + const mixed = finding(0, 2) + mixed.name = 'mixed-package' + mixed.vulnerabilities = [ + { severity: 'CRITICAL', title: 'Withdrawn critical', withdrawn: true }, + { severity: 'LOW', title: 'Active low' } + ] + mixed.remediation = { status: 'unresolved' } + const high = finding(1) + high.name = 'active-high' + high.remediation = { status: 'unresolved' } + const report = renderAuditReport(summaryFor([mixed, high])) + assert.ok(report.indexOf('Finding 1/2: active-high') < report.indexOf('Finding 2/2: mixed-package')) + assert.match(report, /Withdrawn critical — withdrawn/) + assert.match(report, /2 active vulnerability records/) + assert.match(report, /1 withdrawn record/) +}) + test('renderer lists every unchecked package and escapes untrusted Markdown', () => { const unsafe = finding(0) unsafe.vulnerabilities[0].title = '# injected\nheading | cell' @@ -101,15 +159,44 @@ test('renderer lists every unchecked package and escapes untrusted Markdown', () assert.doesNotMatch(report, /^# injected/m) assert.match(report, /\\# injected heading \\| cell/) assert.match(report, /2\/2 unchecked package versions/) + assert.equal((report.match(/NCM returned no matching package-version response after omitted-response recovery/g) || []).length, 1) + assert.match(report, /Unchecked package versions were not proven safe/) }) test('renderer withholds ambiguous upgrade commands and qualifies rollback behavior', () => { - const report = renderAuditReport(summaryFor([finding(0)])) - assert.match(report, /Command withheld because the owning manifest/) + const first = finding(0) + const second = finding(3) + second.direct = false + const report = renderAuditReport(summaryFor([first, second])) + assert.equal((report.match(/Command withheld because exact manifest ownership is not proven/g) || []).length, 1) + assert.match(report, /root declaration name match; resolved ownership unproven/) + assert.match(report, /no root declaration name match; may be transitive or workspace-owned/) + assert.match(report, /pnpm why shared-package/) + assert.match(report, /pnpm why package-3/) + assert.equal((report.match(/^\| HIGH \|/gm) || []).length, 2) assert.doesNotMatch(report, /pnpm add|npm install|yarn add|@latest/) assert.match(report, /For manually executed package-manager commands, rely on version control/) }) +test('renderer groups major breaking changes without losing installed versions', () => { + const viteSix = finding(0) + viteSix.name = 'vite' + viteSix.version = '6.4.2' + viteSix.remediation = { status: 'ncm-verified', version: '8.1.5', source: 'latest-fallback', changeType: 'major' } + const viteSeven = finding(1) + viteSeven.name = 'vite' + viteSeven.version = '7.3.2' + viteSeven.remediation = { status: 'ncm-verified', version: '8.1.5', source: 'latest-fallback', changeType: 'major' } + const report = renderAuditReport(summaryFor([viteSix, viteSeven])) + assert.match(report, /vite@6\.4\.2 and vite@7\.3\.2 → 8\.1\.5/) + assert.equal((report.match(/^- vite@/gm) || []).length, 1) +}) + +test('renderer output is stable for repeated renders', () => { + const summary = summaryFor([finding(0), finding(1)]) + assert.equal(renderAuditReport(summary), renderAuditReport(summary)) +}) + test('integrity validation rejects mismatched totals with a stable error code', () => { const summary = summaryFor([finding(0)]) summary.vulnerabilities.total = 2 @@ -119,3 +206,13 @@ test('integrity validation rejects mismatched totals with a stable error code', return true }) }) + +test('integrity validation rejects invalid active and withdrawn remediation partitions', () => { + const withdrawn = finding(0) + withdrawn.vulnerabilities[0].withdrawn = true + assert.throws(() => validateAuditSummary(summaryFor([withdrawn])), /withdrawn-only finding requires not-required remediation/) + + const active = finding(1) + active.remediation = { status: 'not-required', reason: 'withdrawn-only' } + assert.throws(() => validateAuditSummary(summaryFor([active])), /active finding is marked not-required/) +}) diff --git a/skills/ns-audit-dependencies/render-audit-report.cjs b/skills/ns-audit-dependencies/render-audit-report.cjs index 60fead5..7eff6c8 100644 --- a/skills/ns-audit-dependencies/render-audit-report.cjs +++ b/skills/ns-audit-dependencies/render-audit-report.cjs @@ -1,6 +1,7 @@ 'use strict' const SEVERITIES = ['critical', 'high', 'medium', 'low', 'unknown'] +const SEVERITY_RANK = { unknown: 0, low: 1, medium: 2, high: 3, critical: 4 } const REMEDIATION_KEYS = { 'ncm-verified': 'verified', unresolved: 'unresolved', @@ -8,12 +9,40 @@ const REMEDIATION_KEYS = { 'not-required': 'notRequired' } +const COVERAGE_GUIDANCE = { + authentication: 'NCM authentication failed. Refresh the configured NodeSource credentials before rerunning the audit.', + 'rate-limit': 'NCM rate limiting remained after recovery. Rerun later; these package versions were not checked.', + server: 'NCM server failures remained after recovery. Rerun later; these package versions were not checked.', + timeout: 'NCM requests timed out after recovery. Check connectivity and rerun; these package versions were not checked.', + network: 'Network failures remained after recovery. Restore NCM connectivity and rerun; these package versions were not checked.', + 'invalid-response': 'NCM returned an invalid package response. These package versions were not checked and the response contract should be investigated.', + 'missing-response': 'NCM returned no matching package-version response after omitted-response recovery. Confirm the exact version and NCM coverage; use the organization\'s internal security-review process for private or internal packages without NCM metadata.', + unpublished: 'NCM marked these package versions unpublished. Confirm the resolved versions and review their provenance before use.', + unknown: 'The audit ended without a classified response. Investigate the audit stderr and rerun after resolving the underlying failure.' +} + function integrityError (message) { const error = new Error(`Audit report integrity check failed: ${message}`) error.code = 'AUDIT_REPORT_INTEGRITY_ERROR' return error } +function emptySeverityCounts () { + return Object.fromEntries(SEVERITIES.map(severity => [severity, 0])) +} + +function severityKey (value) { + const key = String(value || '').toLowerCase() + return SEVERITIES.includes(key) ? key : 'unknown' +} + +function highestSeverity (vulnerabilities) { + return vulnerabilities.reduce((highest, vulnerability) => { + const severity = severityKey(vulnerability.severity) + return SEVERITY_RANK[severity] > SEVERITY_RANK[highest] ? severity : highest + }, 'unknown') +} + function stableCounts (values) { return Object.fromEntries(Object.entries(values).sort(([a], [b]) => a.localeCompare(b))) } @@ -27,31 +56,88 @@ function assertCounts (actual, expected, label) { for (const key of keys) assertEqual(actual[key] || 0, expected[key] || 0, `${label}.${key}`) } -function validateAuditSummary (summary) { - if (!summary || !Array.isArray(summary.findings) || !Array.isArray(summary.uncheckedPackages)) { - throw integrityError('missing findings or uncheckedPackages collection') - } +function compareFindings (left, right) { + return SEVERITY_RANK[right.renderedSeverity] - SEVERITY_RANK[left.renderedSeverity] || + String(left.finding.name).localeCompare(String(right.finding.name)) || + String(left.finding.version).localeCompare(String(right.finding.version)) +} - const vulnerabilityCounts = Object.fromEntries(SEVERITIES.map(severity => [severity, 0])) +function analyzeFindings (findings) { + const rawBySeverity = emptySeverityCounts() + const activeBySeverity = emptySeverityCounts() + const withdrawnBySeverity = emptySeverityCounts() const remediationCounts = { verified: 0, unresolved: 0, verificationFailed: 0, notRequired: 0 } - let vulnerabilityTotal = 0 + const activeFindings = [] + const withdrawnOnlyFindings = [] + let rawRecords = 0 + let activeRecords = 0 + let withdrawnRecords = 0 + + for (const finding of findings) { + if (!Array.isArray(finding.vulnerabilities) || finding.vulnerabilities.length === 0) { + throw integrityError(`finding has no vulnerability records for ${finding.name || 'unknown package'}`) + } + const remediationKey = REMEDIATION_KEYS[finding.remediation && finding.remediation.status] + if (!remediationKey) throw integrityError(`unknown remediation status for ${finding.name || 'unknown package'}`) + remediationCounts[remediationKey]++ - for (const finding of summary.findings) { - if (!Array.isArray(finding.vulnerabilities)) throw integrityError('finding has no vulnerability collection') - const key = REMEDIATION_KEYS[finding.remediation && finding.remediation.status] - if (!key) throw integrityError(`unknown remediation status for ${finding.name || 'unknown package'}`) - remediationCounts[key]++ + const active = [] for (const vulnerability of finding.vulnerabilities) { - const severity = String(vulnerability.severity || '').toLowerCase() - vulnerabilityCounts[SEVERITIES.includes(severity) ? severity : 'unknown']++ - vulnerabilityTotal++ + const severity = severityKey(vulnerability.severity) + rawBySeverity[severity]++ + rawRecords++ + if (vulnerability.withdrawn === true) { + withdrawnBySeverity[severity]++ + withdrawnRecords++ + } else { + active.push(vulnerability) + activeBySeverity[severity]++ + activeRecords++ + } } + + if (active.length > 0) { + if (finding.remediation.status === 'not-required') { + throw integrityError(`active finding is marked not-required for ${finding.name}`) + } + activeFindings.push({ finding, renderedSeverity: highestSeverity(active) }) + } else { + if (finding.remediation.status !== 'not-required') { + throw integrityError(`withdrawn-only finding requires not-required remediation for ${finding.name}`) + } + withdrawnOnlyFindings.push({ finding, renderedSeverity: highestSeverity(finding.vulnerabilities) }) + } + } + + activeFindings.sort(compareFindings) + withdrawnOnlyFindings.sort(compareFindings) + return { + rawBySeverity, + activeBySeverity, + withdrawnBySeverity, + remediationCounts, + activeFindings, + withdrawnOnlyFindings, + rawRecords, + activeRecords, + withdrawnRecords + } +} + +function validateAuditSummary (summary) { + if (!summary || !Array.isArray(summary.findings) || !Array.isArray(summary.uncheckedPackages)) { + throw integrityError('missing findings or uncheckedPackages collection') } + const analysis = analyzeFindings(summary.findings) assertEqual(summary.findings.length, summary.vulnerabilities.affectedPackages, 'affected package versions') - assertEqual(vulnerabilityTotal, summary.vulnerabilities.total, 'vulnerability records') - assertCounts(vulnerabilityCounts, summary.vulnerabilities.bySeverity, 'severity') - assertCounts(remediationCounts, { + assertEqual(analysis.rawRecords, summary.vulnerabilities.total, 'vulnerability records') + assertEqual(analysis.activeRecords + analysis.withdrawnRecords, analysis.rawRecords, 'record partition') + assertEqual(analysis.activeFindings.length + analysis.withdrawnOnlyFindings.length, summary.findings.length, 'finding partition') + assertCounts(analysis.rawBySeverity, summary.vulnerabilities.bySeverity, 'severity') + assertEqual(Object.values(analysis.activeBySeverity).reduce((total, count) => total + count, 0), analysis.activeRecords, 'active severity total') + assertEqual(Object.values(analysis.withdrawnBySeverity).reduce((total, count) => total + count, 0), analysis.withdrawnRecords, 'withdrawn severity total') + assertCounts(analysis.remediationCounts, { verified: summary.remediation.verified, unresolved: summary.remediation.unresolved, verificationFailed: summary.remediation.verificationFailed, @@ -67,8 +153,9 @@ function validateAuditSummary (summary) { assertCounts(stableCounts(uncheckedCounts), summary.packages.uncheckedByReason, 'unchecked reason') return { + ...analysis, findings: summary.findings.length, - vulnerabilities: vulnerabilityTotal, + vulnerabilities: analysis.rawRecords, unchecked: summary.uncheckedPackages.length } } @@ -103,20 +190,42 @@ function countsText (counts) { return entries.length > 0 ? entries.map(([key, count]) => `${count} ${escapeMarkdown(key)}`).join(', ') : 'none' } -function renderFinding (finding, index, findingTotal, recordState, recordTotal) { +function severityText (counts) { + return `${counts.critical} critical, ${counts.high} high, ${counts.medium} medium, ${counts.low} low, ${counts.unknown} unknown` +} + +function noun (count, singular, plural = `${singular}s`) { + return count === 1 ? singular : plural +} + +function dependencyScope (finding, short = false) { + if (finding.direct) { + return short + ? 'root-name match; ownership unresolved' + : 'root declaration name match; resolved ownership unproven' + } + return short + ? 'no root-name match; workspace/transitive unresolved' + : 'no root declaration name match; may be transitive or workspace-owned' +} + +function renderFinding (item, index, findingTotal, renderState, recordTotal) { + const { finding, renderedSeverity } = item const lines = [ `### Finding ${index + 1}/${findingTotal}: ${escapeMarkdown(finding.name)}@${escapeMarkdown(finding.version)}`, '', - `- Severity: ${escapeMarkdown(finding.severity)}`, - `- Dependency: ${finding.direct ? 'direct' : 'transitive'}`, + `- Severity: ${escapeMarkdown(renderedSeverity.toUpperCase())}`, + `- Dependency scope: ${dependencyScope(finding)}`, `- License: ${finding.license && finding.license.spdx ? escapeMarkdown(finding.license.spdx) : 'not returned'}`, '- Vulnerability records:' ] for (const vulnerability of finding.vulnerabilities) { - recordState.count++ + renderState.records++ + if (vulnerability.withdrawn) renderState.withdrawnRecords++ + else renderState.activeRecords++ const withdrawn = vulnerability.withdrawn ? ' — withdrawn' : '' - lines.push(` - Record ${recordState.count}/${recordTotal} — ${escapeMarkdown(vulnerability.severity)} — ${valueOrNotReturned(vulnerability.id)} — ${escapeMarkdown(vulnerability.title)}${withdrawn}`) + lines.push(` - Record ${renderState.records}/${recordTotal} — ${escapeMarkdown(vulnerability.severity)} — ${valueOrNotReturned(vulnerability.id)} — ${escapeMarkdown(vulnerability.title)}${withdrawn}`) lines.push(` - Withdrawn: ${vulnerability.withdrawn ? 'yes' : 'no'}`) lines.push(` - URL: ${valueOrNotReturned(vulnerability.url)}`) lines.push(` - Vulnerable ranges: ${listOrNotReturned(vulnerability.vulnerable)}`) @@ -141,33 +250,102 @@ function renderFinding (finding, index, findingTotal, recordState, recordTotal) return lines.join('\n') } -function renderRemediationPlan (summary) { +function whyCommand (packageManager, name) { + return `${packageManager} why ${name}` +} + +function renderRemediationPlan (summary, analysis) { const lines = ['## Remediation Plan', ''] - const verified = summary.findings.filter(finding => finding.remediation.status === 'ncm-verified') + const verified = analysis.activeFindings.filter(item => item.finding.remediation.status === 'ncm-verified') if (verified.length === 0) { lines.push('No NCM-verified upgrade targets were found.') } else { - for (const finding of verified) { - lines.push(`- ${escapeMarkdown(finding.name)}@${escapeMarkdown(finding.version)} → verified target ${escapeMarkdown(finding.name)}@${escapeMarkdown(finding.remediation.version)}.`) - lines.push(' Command withheld because the owning manifest and resolved direct version are not proven unambiguous; locate the declaration before upgrading.') - if (!finding.direct) lines.push(` Locate the introducing parent with \`${escapeMarkdown(summary.packageManager)} why ${escapeMarkdown(finding.name)}\`.`) + lines.push('| Severity | Installed | Verified target | Scope | Change | Verification | Next step |') + lines.push('| --- | --- | --- | --- | --- | --- | --- |') + for (const { finding, renderedSeverity } of verified) { + lines.push(`| ${escapeMarkdown(renderedSeverity.toUpperCase())} | ${escapeMarkdown(`${finding.name}@${finding.version}`)} | ${escapeMarkdown(finding.remediation.version)} | ${escapeMarkdown(dependencyScope(finding, true))} | ${escapeMarkdown(finding.remediation.changeType)} | ${escapeMarkdown(finding.remediation.source)} | ${escapeMarkdown(whyCommand(summary.packageManager, finding.name))} |`) } + lines.push('', 'Command withheld because exact manifest ownership is not proven. These versions are NCM-verified targets, not instructions to edit an arbitrary root manifest.') } - const unresolved = summary.findings.filter(finding => finding.remediation.status === 'unresolved').length - const failed = summary.findings.filter(finding => finding.remediation.status === 'verification-failed').length + const unresolved = analysis.activeFindings.filter(item => item.finding.remediation.status === 'unresolved').length + const failed = analysis.activeFindings.filter(item => item.finding.remediation.status === 'verification-failed').length if (unresolved > 0) lines.push(`- ${unresolved} finding(s) remain unresolved; use reported ranges as evidence, but verify a candidate with NCM before pinning.`) if (failed > 0) lines.push(`- ${failed} finding(s) could not complete remediation verification; this does not weaken the vulnerability finding.`) + return { markdown: lines.join('\n'), rows: verified.length } +} + +function renderBreakingChangeNotes (analysis) { + const lines = ['## Breaking Change Notes', ''] + const majors = analysis.activeFindings.filter(item => { + return item.finding.remediation.status === 'ncm-verified' && item.finding.remediation.changeType === 'major' + }) + if (majors.length === 0) { + lines.push('No NCM-verified major-version remediation targets were found.') + return { markdown: lines.join('\n'), sourceFindings: 0, renderedInstalledVersions: 0 } + } + + const groups = new Map() + for (const { finding } of majors) { + const key = `${finding.name}\u0000${finding.remediation.version}` + if (!groups.has(key)) groups.set(key, { name: finding.name, target: finding.remediation.version, installed: new Set() }) + groups.get(key).installed.add(finding.version) + } + const ordered = Array.from(groups.values()).sort((a, b) => { + return a.name.localeCompare(b.name) || a.target.localeCompare(b.target) + }) + let renderedInstalledVersions = 0 + for (const group of ordered) { + const installed = Array.from(group.installed).sort((a, b) => a.localeCompare(b, undefined, { numeric: true })) + renderedInstalledVersions += installed.length + const sources = installed.map(version => `${escapeMarkdown(group.name)}@${escapeMarkdown(version)}`).join(' and ') + lines.push(`- ${sources} → ${escapeMarkdown(group.target)}`) + } + lines.push('', 'Review official changelogs and test build, development tooling, and runtime behavior before merging major upgrades.') + return { markdown: lines.join('\n'), sourceFindings: majors.length, renderedInstalledVersions } +} + +function renderCoverageGaps (summary) { + if (summary.uncheckedPackages.length === 0) return '' + const lines = ['## Coverage Gaps', ''] + const packages = [...summary.uncheckedPackages].sort((a, b) => { + return a.reason.localeCompare(b.reason) || a.name.localeCompare(b.name) || a.version.localeCompare(b.version) + }) + for (const pkg of packages) { + lines.push(`- ${escapeMarkdown(pkg.name)}@${escapeMarkdown(pkg.version)} — ${escapeMarkdown(pkg.reason)}`) + } + lines.push('', 'Unchecked package versions were not proven safe.') + const reasons = Array.from(new Set(packages.map(pkg => pkg.reason))).sort() + for (const reason of reasons) { + const guidance = COVERAGE_GUIDANCE[reason] || COVERAGE_GUIDANCE.unknown + lines.push(`- ${escapeMarkdown(reason)}: ${guidance}`) + } + return lines.join('\n') +} + +function renderFindingSection (title, items, startIndex, totalFindings, renderState, recordTotal, partition) { + const lines = [`## ${title}`, ''] + if (items.length === 0) { + lines.push(title === 'Active Findings' + ? 'No active vulnerable package versions were returned.' + : 'No withdrawn-only package versions were returned.') + return lines.join('\n') + } + items.forEach((item, itemIndex) => { + if (itemIndex > 0) lines.push('') + renderState[partition]++ + lines.push(renderFinding(item, startIndex + itemIndex, totalFindings, renderState, recordTotal)) + }) return lines.join('\n') } function renderAuditReport (summary) { - const counts = validateAuditSummary(summary) - const severity = summary.vulnerabilities.bySeverity + const analysis = validateAuditSummary(summary) const lines = [ '## Summary', '', `- ${summary.packages.checked} of ${summary.packages.total} package versions checked; ${summary.packages.unchecked} unchecked.`, - `- ${summary.vulnerabilities.total} vulnerability records across ${summary.vulnerabilities.affectedPackages} affected package versions: ${severity.critical} critical, ${severity.high} high, ${severity.medium} medium, ${severity.low} low, ${severity.unknown} unknown.`, + `- ${analysis.activeRecords} active vulnerability ${noun(analysis.activeRecords, 'record')} across ${analysis.activeFindings.length} actively affected package ${noun(analysis.activeFindings.length, 'version')}: ${severityText(analysis.activeBySeverity)}.`, + `- The complete NCM response contained ${analysis.rawRecords} vulnerability ${noun(analysis.rawRecords, 'record')} across ${summary.vulnerabilities.affectedPackages} affected package ${noun(summary.vulnerabilities.affectedPackages, 'version')}, including ${analysis.withdrawnRecords} withdrawn ${noun(analysis.withdrawnRecords, 'record')} and ${analysis.withdrawnOnlyFindings.length} withdrawn-only package ${noun(analysis.withdrawnOnlyFindings.length, 'version')}.`, `- Terminal batch failures: ${summary.batchFailures.total} (${countsText(summary.batchFailures.byReason)}). Recovery: ${summary.batchRecovery.transportRetries} transport retries, ${summary.batchRecovery.splitBatches} batch splits, ${summary.batchRecovery.missingPackageRetries} omitted-response retries, ${summary.batchRecovery.recoveredPackages} recovered package versions.`, `- Remediation: ${summary.remediation.verified} NCM-verified, ${summary.remediation.unresolved} unresolved, ${summary.remediation.verificationFailed} verification failures, ${summary.remediation.notRequired} not required. Candidate failures: ${summary.remediation.failures.total} (${countsText(summary.remediation.failures.byReason)}).`, `- Remediation recovery: ${summary.remediation.recovery.transportRetries} transport retries, ${summary.remediation.recovery.splitBatches} batch splits, ${summary.remediation.recovery.missingCandidateRetries} omitted-candidate retries.` @@ -177,34 +355,41 @@ function renderAuditReport (summary) { } else { lines.push('- No NCM field truncation occurred.') } + lines.push('', 'Vulnerability records are package-version occurrences; the same advisory may appear for multiple installed versions or more than once in the NCM response.') + lines.push('', 'Root declaration matches are based on package names. The audit does not yet prove which manifest or workspace owns each resolved version.') lines.push('', '“NCM-verified” means free of active NCM advisories at audit time—not absolutely safe.') lines.push('', 'This is a static dependency audit; it does not establish runtime loading, reachability, or exploitability.') - if (summary.uncheckedPackages.length > 0) { - lines.push('', '## Coverage Gaps', '') - for (const pkg of summary.uncheckedPackages) { - lines.push(`- ${escapeMarkdown(pkg.name)}@${escapeMarkdown(pkg.version)} — ${escapeMarkdown(pkg.reason)}`) - } - } + const coverage = renderCoverageGaps(summary) + if (coverage) lines.push('', coverage) - lines.push('', '## Prioritized Findings', '') - if (summary.findings.length === 0) lines.push('No vulnerable package versions were returned.') - const recordState = { count: 0 } - summary.findings.forEach((finding, index) => { - if (index > 0) lines.push('') - lines.push(renderFinding(finding, index, counts.findings, recordState, counts.vulnerabilities)) - }) + const renderState = { + records: 0, + activeRecords: 0, + withdrawnRecords: 0, + activeFindings: 0, + withdrawnOnlyFindings: 0 + } + lines.push('', renderFindingSection('Active Findings', analysis.activeFindings, 0, analysis.findings, renderState, analysis.vulnerabilities, 'activeFindings')) + lines.push('', renderFindingSection('Withdrawn-Only Findings', analysis.withdrawnOnlyFindings, analysis.activeFindings.length, analysis.findings, renderState, analysis.vulnerabilities, 'withdrawnOnlyFindings')) + assertEqual(renderState.activeFindings, analysis.activeFindings.length, 'rendered active findings') + assertEqual(renderState.withdrawnOnlyFindings, analysis.withdrawnOnlyFindings.length, 'rendered withdrawn-only findings') + assertEqual(renderState.records, analysis.vulnerabilities, 'rendered vulnerability records') + assertEqual(renderState.activeRecords, analysis.activeRecords, 'rendered active records') + assertEqual(renderState.withdrawnRecords, analysis.withdrawnRecords, 'rendered withdrawn records') - lines.push('', renderRemediationPlan(summary), '', '## Breaking Change Notes', '') - const majors = summary.findings.filter(finding => finding.remediation.status === 'ncm-verified' && finding.remediation.changeType === 'major') - if (majors.length === 0) lines.push('No NCM-verified major-version remediation targets were found.') - else lines.push(`Major upgrades require changelog and compatibility review: ${majors.map(finding => `${escapeMarkdown(finding.name)} → ${escapeMarkdown(finding.remediation.version)}`).join(', ')}.`) + const remediation = renderRemediationPlan(summary, analysis) + assertEqual(remediation.rows, analysis.remediationCounts.verified, 'remediation table rows') + const breaking = renderBreakingChangeNotes(analysis) + assertEqual(breaking.sourceFindings, breaking.renderedInstalledVersions, 'breaking-change installed versions') + lines.push('', remediation.markdown, '', breaking.markdown) lines.push('', '## Rollback Guidance', '') lines.push('The N|Solid upgrade workflow backs up `package.json` and the lockfile under `.nsolid/backup/`. For manually executed package-manager commands, rely on version control or create a separate backup first.') lines.push('', '## Report Integrity', '') - lines.push(`Rendered ${counts.findings}/${summary.vulnerabilities.affectedPackages} package-version findings, ${recordState.count}/${summary.vulnerabilities.total} vulnerability records, and ${counts.unchecked}/${summary.packages.unchecked} unchecked package versions.`) + lines.push(`Rendered ${renderState.activeFindings + renderState.withdrawnOnlyFindings}/${summary.vulnerabilities.affectedPackages} package-version findings, ${renderState.records}/${summary.vulnerabilities.total} vulnerability records, and ${analysis.unchecked}/${summary.packages.unchecked} unchecked package versions.`) + lines.push(`Finding partition: ${renderState.activeFindings}/${analysis.activeFindings.length} active and ${renderState.withdrawnOnlyFindings}/${analysis.withdrawnOnlyFindings.length} withdrawn-only. Record partition: ${renderState.activeRecords}/${analysis.activeRecords} active and ${renderState.withdrawnRecords}/${analysis.withdrawnRecords} withdrawn.`) return lines.join('\n') } -module.exports = { escapeMarkdown, renderAuditReport, validateAuditSummary } +module.exports = { analyzeFindings, escapeMarkdown, renderAuditReport, validateAuditSummary } From a3f47bb295f67df1b93d22eea743b7a6679a5afb Mon Sep 17 00:00:00 2001 From: Cesar-M-Diaz Date: Fri, 17 Jul 2026 17:41:15 +0200 Subject: [PATCH 5/6] fix: auto save the detailed report and show the summary instead in chat --- .../skills/audit-dependencies-output.test.ts | 230 ++++++++++++++++++ .../audit-dependencies-renderer.test.ts | 54 +++- .../skills/audit-dependencies-script.test.ts | 16 +- skills/ns-audit-dependencies/SKILL.md | 16 +- .../audit-dependencies.cjs | 26 +- .../audit-report-output.cjs | 81 ++++++ .../render-audit-report.cjs | 157 +++++++++++- 7 files changed, 566 insertions(+), 14 deletions(-) create mode 100644 packages/core/test/unit/skills/audit-dependencies-output.test.ts create mode 100644 skills/ns-audit-dependencies/audit-report-output.cjs diff --git a/packages/core/test/unit/skills/audit-dependencies-output.test.ts b/packages/core/test/unit/skills/audit-dependencies-output.test.ts new file mode 100644 index 0000000..d614f2f --- /dev/null +++ b/packages/core/test/unit/skills/audit-dependencies-output.test.ts @@ -0,0 +1,230 @@ +import assert from 'node:assert/strict' +import { createRequire } from 'node:module' +import { mkdtemp, readFile, readdir, rm, stat } from 'node:fs/promises' +import os from 'node:os' +import path from 'node:path' +import test from 'node:test' + +const require = createRequire(import.meta.url) +const { createSavedAuditOutput } = require('../../../../../skills/ns-audit-dependencies/audit-report-output.cjs') as { + createSavedAuditOutput: ( + summary: Record, + projectDir: string, + options?: { now?: Date } + ) => Promise<{ executiveSummary: string, report: string, reportPath: string }> +} + +function auditSummary (): Record { + return { + packageManager: 'pnpm', + packages: { + total: 2, + direct: 1, + transitive: 1, + checked: 1, + unchecked: 1, + uncheckedByReason: { 'missing-response': 1 } + }, + vulnerabilities: { + total: 1, + affectedPackages: 1, + bySeverity: { critical: 0, high: 1, medium: 0, low: 0, unknown: 0 } + }, + batchFailures: { total: 0, byReason: {} }, + batchRecovery: { transportRetries: 0, splitBatches: 0, missingPackageRetries: 1, recoveredPackages: 0 }, + ncmContentTruncation: { truncatedFields: 0, truncatedCharacters: 0 }, + remediation: { + candidateRequests: 1, + candidatesChecked: 1, + verified: 1, + unresolved: 0, + verificationFailed: 0, + notRequired: 0, + failures: { total: 0, byReason: {} }, + recovery: { transportRetries: 0, splitBatches: 0, missingCandidateRetries: 0 } + }, + uncheckedPackages: [ + { name: '@internal/package', version: '1.0.0', reason: 'missing-response' } + ], + findings: [ + { + name: 'vulnerable-package', + version: '1.0.0', + direct: true, + severity: 'HIGH', + vulnerabilities: [ + { + severity: 'HIGH', + id: 'GHSA-test', + title: 'Test vulnerability', + vulnerable: ['< 1.0.1'], + patched: ['>= 1.0.1'] + } + ], + license: { pass: true, spdx: 'MIT' }, + moduleRisks: [], + codeQuality: [], + remediation: { + status: 'ncm-verified', + version: '1.0.1', + source: 'boundary', + changeType: 'patch' + } + } + ] + } +} + +function uncheckedAuditSummary (reasons: string[]): Record { + const uncheckedByReason: Record = {} + const uncheckedPackages = reasons.map((reason, index) => { + uncheckedByReason[reason] = (uncheckedByReason[reason] || 0) + 1 + return { name: `package-${index}`, version: `1.0.${index}`, reason } + }) + return { + packageManager: 'pnpm', + packages: { + total: reasons.length, + direct: 0, + transitive: reasons.length, + checked: 0, + unchecked: reasons.length, + uncheckedByReason + }, + vulnerabilities: { + total: 0, + affectedPackages: 0, + bySeverity: { critical: 0, high: 0, medium: 0, low: 0, unknown: 0 } + }, + batchFailures: { total: reasons.length, byReason: uncheckedByReason }, + batchRecovery: { transportRetries: 2, splitBatches: 1, missingPackageRetries: 0, recoveredPackages: 0 }, + ncmContentTruncation: { truncatedFields: 0, truncatedCharacters: 0 }, + remediation: { + candidateRequests: 0, + candidatesChecked: 0, + verified: 0, + unresolved: 0, + verificationFailed: 0, + notRequired: 0, + failures: { total: 0, byReason: {} }, + recovery: { transportRetries: 0, splitBatches: 0, missingCandidateRetries: 0 } + }, + uncheckedPackages, + findings: [] + } +} + +test('saved audit output publishes exact complete report bytes before linking it from the summary', async () => { + const projectDir = await mkdtemp(path.join(os.tmpdir(), 'nsolid audit output ')) + try { + const output = await createSavedAuditOutput(auditSummary(), projectDir, { + now: new Date('2026-07-17T14:35:22.123Z') + }) + + assert.equal(output.reportPath, path.join( + projectDir, + '.nsolid', + 'assets', + 'dependency-audit-2026-07-17T14-35-22-123Z.md' + )) + assert.equal(await readFile(output.reportPath, 'utf8'), output.report) + assert.ok(output.report.endsWith('\n')) + const linkedPath = output.reportPath.split(path.sep).join('/').replace(/[.*+?^${}()|[\]\\]/g, '\\$&') + assert.match(output.executiveSummary, new RegExp(linkedPath)) + if (process.platform !== 'win32') assert.equal((await stat(output.reportPath)).mode & 0o777, 0o600) + assert.deepEqual( + (await readdir(path.dirname(output.reportPath))).filter(name => name.endsWith('.tmp')), + [] + ) + } finally { + await rm(projectDir, { recursive: true, force: true }) + } +}) + +test('saved audit output never overwrites a colliding report name', async () => { + const projectDir = await mkdtemp(path.join(os.tmpdir(), 'nsolid-audit-output-')) + const now = new Date('2026-07-17T14:35:22.123Z') + try { + const first = await createSavedAuditOutput(auditSummary(), projectDir, { now }) + const second = await createSavedAuditOutput(auditSummary(), projectDir, { now }) + + assert.notEqual(first.reportPath, second.reportPath) + assert.match(second.reportPath, /-2\.md$/) + assert.equal(await readFile(first.reportPath, 'utf8'), first.report) + assert.equal(await readFile(second.reportPath, 'utf8'), second.report) + } finally { + await rm(projectDir, { recursive: true, force: true }) + } +}) + +test('integrity failures create no report artifact', async () => { + const projectDir = await mkdtemp(path.join(os.tmpdir(), 'nsolid-audit-output-')) + const invalid = auditSummary() + invalid.vulnerabilities.total = 2 + try { + await assert.rejects( + createSavedAuditOutput(invalid, projectDir), + (error: any) => error.code === 'AUDIT_REPORT_INTEGRITY_ERROR' + ) + await assert.rejects(readdir(path.join(projectDir, '.nsolid', 'assets')), { code: 'ENOENT' }) + } finally { + await rm(projectDir, { recursive: true, force: true }) + } +}) + +test('fully unchecked retryable transport failures create no report artifact', async () => { + const projectDir = await mkdtemp(path.join(os.tmpdir(), 'nsolid-audit-output-')) + try { + await assert.rejects( + createSavedAuditOutput(uncheckedAuditSummary(['network', 'timeout', 'rate-limit', 'server']), projectDir), + (error: any) => { + assert.equal(error.code, 'AUDIT_REPORT_RETRY_REQUIRED') + assert.match(error.message, /all 4 package versions were unchecked/i) + return true + } + ) + await assert.rejects(readdir(path.join(projectDir, '.nsolid', 'assets')), { code: 'ENOENT' }) + } finally { + await rm(projectDir, { recursive: true, force: true }) + } +}) + +test('a successful second attempt leaves exactly one saved report', async () => { + const projectDir = await mkdtemp(path.join(os.tmpdir(), 'nsolid-audit-output-')) + try { + await assert.rejects( + createSavedAuditOutput(uncheckedAuditSummary(['network']), projectDir), + (error: any) => error.code === 'AUDIT_REPORT_RETRY_REQUIRED' + ) + const successful = await createSavedAuditOutput(auditSummary(), projectDir) + assert.deepEqual(await readdir(path.dirname(successful.reportPath)), [path.basename(successful.reportPath)]) + } finally { + await rm(projectDir, { recursive: true, force: true }) + } +}) + +test('fully unchecked authentication failures create no report artifact', async () => { + const projectDir = await mkdtemp(path.join(os.tmpdir(), 'nsolid-audit-output-')) + try { + await assert.rejects( + createSavedAuditOutput(uncheckedAuditSummary(['authentication']), projectDir), + (error: any) => error.code === 'AUDIT_REPORT_AUTHENTICATION_REQUIRED' + ) + await assert.rejects(readdir(path.join(projectDir, '.nsolid', 'assets')), { code: 'ENOENT' }) + } finally { + await rm(projectDir, { recursive: true, force: true }) + } +}) + +test('partial network coverage still saves the incomplete report', async () => { + const projectDir = await mkdtemp(path.join(os.tmpdir(), 'nsolid-audit-output-')) + const summary = auditSummary() + summary.packages.uncheckedByReason = { network: 1 } + summary.uncheckedPackages[0].reason = 'network' + try { + const output = await createSavedAuditOutput(summary, projectDir) + assert.equal(await readFile(output.reportPath, 'utf8'), output.report) + } finally { + await rm(projectDir, { recursive: true, force: true }) + } +}) diff --git a/packages/core/test/unit/skills/audit-dependencies-renderer.test.ts b/packages/core/test/unit/skills/audit-dependencies-renderer.test.ts index c78a419..7ff0121 100644 --- a/packages/core/test/unit/skills/audit-dependencies-renderer.test.ts +++ b/packages/core/test/unit/skills/audit-dependencies-renderer.test.ts @@ -1,10 +1,13 @@ import assert from 'node:assert/strict' import { createRequire } from 'node:module' +import path from 'node:path' import test from 'node:test' const require = createRequire(import.meta.url) -const { renderAuditReport, validateAuditSummary } = require('../../../../../skills/ns-audit-dependencies/render-audit-report.cjs') as { +const { renderAuditReport, renderAuditSummary, reportLink, validateAuditSummary } = require('../../../../../skills/ns-audit-dependencies/render-audit-report.cjs') as { renderAuditReport: (summary: Record) => string + renderAuditSummary: (summary: Record, reportPath: string) => string + reportLink: (reportPath: string, pathApi?: typeof path) => string validateAuditSummary: (summary: Record) => Record } @@ -97,6 +100,55 @@ test('renderer reconciles a 36-finding and 57-record report including duplicate assert.equal((report.match(/GHSA-duplicate/g) || []).length, 57) }) +test('executive summary links the complete 36-finding and 57-record report without claiming chat rendered it', () => { + const findings = Array.from({ length: 36 }, (_, index) => finding(index, index < 21 ? 2 : 1)) + findings[1].vulnerabilities.forEach((vulnerability: Record) => { vulnerability.severity = 'CRITICAL' }) + const summary = summaryFor(findings) + const reportPath = path.resolve('project with spaces', '.nsolid', 'assets', 'dependency-audit.md') + const executiveSummary = renderAuditSummary(summary, reportPath) + const linkedPath = reportPath.split(path.sep).join('/').replace(/[.*+?^${}()|[\]\\]/g, '\\$&') + + assert.match(executiveSummary, /^## Executive Summary/m) + assert.match(executiveSummary, new RegExp(`\\[Open the complete dependency audit report\\]\\(<${linkedPath}>\\)`)) + assert.match(executiveSummary, /The underlying complete report reconciles 36\/36 package-version findings, 57\/57 vulnerability records/) + assert.match(executiveSummary, /package-1@1\.0\.1/) + assert.doesNotMatch(executiveSummary, /^ {2}- Record /m) + assert.doesNotMatch(executiveSummary, /Vulnerable ranges:/) + assert.doesNotMatch(executiveSummary, /Rendered 36\/36/) +}) + +test('executive summary groups verified targets and lists every unresolved and withdrawn-only package version', () => { + const first = finding(0) + first.vulnerabilities[0].severity = 'CRITICAL' + const second = finding(2) + second.vulnerabilities[0].severity = 'CRITICAL' + second.remediation = { ...first.remediation } + const unresolved = finding(1) + const withdrawn = finding(4) + withdrawn.vulnerabilities[0].withdrawn = true + withdrawn.remediation = { status: 'not-required', reason: 'withdrawn-only' } + const summary = summaryFor([first, second, unresolved, withdrawn], [ + { name: 'internal-one', version: '1.0.0', reason: 'missing-response' }, + { name: 'internal-two', version: '2.0.0', reason: 'missing-response' } + ]) + const reportPath = path.resolve('dependency-audit.md') + const executiveSummary = renderAuditSummary(summary, reportPath) + + assert.match(executiveSummary, /2 verified package-version findings are represented by 1 grouped upgrade action/) + assert.match(executiveSummary, /shared-package@1\.0\.0, shared-package@1\.0\.2/) + assert.match(executiveSummary, /Unresolved \(1\): package-1@1\.0\.1/) + assert.match(executiveSummary, /shared-package@1\.0\.4/) + assert.match(executiveSummary, /2 unchecked package versions were not proven safe: 2 missing-response/) + assert.equal(renderAuditSummary(summary, reportPath), executiveSummary) +}) + +test('report links normalize Windows paths and encode Markdown-significant filename characters', () => { + assert.equal( + reportLink('C:\\Users\\Example User\\project\\.nsolid\\assets\\audit#1?.md', path.win32), + '[Open the complete dependency audit report]()' + ) +}) + test('renderer leads with active risk and partitions a 36-finding, 57-record report', () => { const activeSeverities = [ ...Array(5).fill('CRITICAL'), diff --git a/packages/core/test/unit/skills/audit-dependencies-script.test.ts b/packages/core/test/unit/skills/audit-dependencies-script.test.ts index bd9cb57..64aafc1 100644 --- a/packages/core/test/unit/skills/audit-dependencies-script.test.ts +++ b/packages/core/test/unit/skills/audit-dependencies-script.test.ts @@ -15,7 +15,7 @@ const { classifyBatchFailure, fetchBatch, formatCliError, parseArgs, runAudit } options?: Record ) => Promise>> formatCliError: (error: Error & { code?: string }) => string - parseArgs: () => { dir: string, format: string } + parseArgs: () => { dir: string, format: string, saveReport: boolean } runAudit: (dir: string, options: Record) => Promise> } @@ -27,6 +27,14 @@ test('audit helper preserves integrity error codes in CLI output', () => { formatCliError(error), 'Error: AUDIT_REPORT_INTEGRITY_ERROR: summary totals differ\n' ) + assert.equal( + formatCliError(Object.assign(new Error('retry with network access'), { code: 'AUDIT_REPORT_RETRY_REQUIRED' })), + 'Error: AUDIT_REPORT_RETRY_REQUIRED: retry with network access\n' + ) + assert.equal( + formatCliError(Object.assign(new Error('configure credentials'), { code: 'AUDIT_REPORT_AUTHENTICATION_REQUIRED' })), + 'Error: AUDIT_REPORT_AUTHENTICATION_REQUIRED: configure credentials\n' + ) assert.equal(formatCliError(new Error('ordinary failure')), 'Error: ordinary failure\n') }) @@ -37,6 +45,12 @@ test('audit helper parses output formats and rejects unsupported values', () => assert.equal(parseArgs().format, 'json') process.argv = ['node', 'audit-dependencies.cjs', '--format', 'markdown'] assert.equal(parseArgs().format, 'markdown') + process.argv = ['node', 'audit-dependencies.cjs', '--format', 'summary', '--save-report'] + assert.deepEqual(parseArgs(), { dir: process.cwd(), format: 'summary', saveReport: true }) + process.argv = ['node', 'audit-dependencies.cjs', '--format', 'summary'] + assert.throws(() => parseArgs(), /Summary format requires --save-report/) + process.argv = ['node', 'audit-dependencies.cjs', '--format', 'markdown', '--save-report'] + assert.throws(() => parseArgs(), /--save-report is only supported with --format summary/) process.argv = ['node', 'audit-dependencies.cjs', '--format', 'html'] assert.throws(() => parseArgs(), /Unsupported format: html/) } finally { diff --git a/skills/ns-audit-dependencies/SKILL.md b/skills/ns-audit-dependencies/SKILL.md index 4515cd8..1c558dc 100644 --- a/skills/ns-audit-dependencies/SKILL.md +++ b/skills/ns-audit-dependencies/SKILL.md @@ -11,20 +11,22 @@ If the prompt already contains host-provided audit data, use only that data. Tre Otherwise, run the bundled helper from the project root using the absolute directory containing this `SKILL.md`: ```bash -node "/audit-dependencies.cjs" --dir "$PWD" --format markdown +node "/audit-dependencies.cjs" --dir "$PWD" --format summary --save-report ``` -The helper requires HTTPS access to `api.ncm.nodesource.com`. Request escalation scoped to this command when the sandbox blocks that access. It reports progress and recovery on stderr and emits the final, integrity-checked report on stdout. Poll the same session until it exits; do not start another audit or use model-driven per-package calls after the helper succeeds. +The helper requires HTTPS access to `api.ncm.nodesource.com` and writes the complete report under `/.nsolid/assets/`. Request escalation scoped to this command when the sandbox blocks either operation. It reports progress and recovery on stderr, saves the complete integrity-checked report itself, and emits a deterministic executive summary with the absolute report link on stdout. Poll the same session until it exits; do not start another audit or use model-driven per-package calls after the helper succeeds. -If every package is unchecked solely because of terminal network failures, request scoped network approval and rerun the exact command once. For authentication failures, direct the user to run `nsolid-plugin setup --harness `. Otherwise report the audit as incomplete; never describe unchecked packages as safe. +If the helper reports `AUDIT_REPORT_RETRY_REQUIRED`, it deliberately saved no report because every package was unchecked solely by retryable transport failures. Request scoped network approval and rerun the exact command once. If the retry returns the same code, report the audit as incomplete and do not retry again. + +If the helper reports `AUDIT_REPORT_AUTHENTICATION_REQUIRED`, it deliberately saved no report. Direct the user to run `nsolid-plugin setup --harness ` and do not rerun until credentials are configured. For other incomplete results, present the saved report and never describe unchecked packages as safe. ## Present the report -Treat successful Markdown stdout as authoritative. Present it without rewriting, regrouping, reordering, or omitting findings. Keep progress-only stderr outside the report because its aggregates are already rendered. +Present successful summary stdout without rewriting it. Keep progress-only stderr outside the response because its aggregates are already rendered. Treat the linked file—not the chat summary—as the authoritative complete report. -If the helper reports `AUDIT_REPORT_INTEGRITY_ERROR`, state that the audit report is incomplete. Do not reconstruct a report from partial output. +If the helper reports `AUDIT_REPORT_INTEGRITY_ERROR` or fails to save a publishable report, state that the audit report is incomplete. Do not reconstruct a report from partial output and do not save model-rewritten report text. -After presenting the complete report, ask whether the user wants it saved as Markdown under `.nsolid/assets/`. +When the user asks for details, read the exact linked report and answer from it without rerunning the audit. Rerun only when the user explicitly requests a fresh audit. Never overwrite or replace the saved report. ## Guardrails @@ -32,4 +34,4 @@ After presenting the complete report, ask whether the user wants it saved as Mar - Never invent CVE IDs, severities, fixed versions, or package ownership. - `ncm-verified` means free of active NCM advisories at audit time, not absolutely safe. - Never convert a concrete `latest-fallback` result into `@latest`. -- JSON remains available for programmatic callers by omitting `--format markdown` or using `--format json`. +- JSON remains available for programmatic callers by omitting `--format` or using `--format json`; the complete report remains available on stdout with `--format markdown`. diff --git a/skills/ns-audit-dependencies/audit-dependencies.cjs b/skills/ns-audit-dependencies/audit-dependencies.cjs index 5dd2793..e644ea6 100644 --- a/skills/ns-audit-dependencies/audit-dependencies.cjs +++ b/skills/ns-audit-dependencies/audit-dependencies.cjs @@ -4,6 +4,7 @@ const fs = require('fs') const os = require('os') const path = require('path') +const { createSavedAuditOutput } = require('./audit-report-output.cjs') const { collectDependencies } = require('./collect-dependencies.cjs') const { renderAuditReport } = require('./render-audit-report.cjs') @@ -50,16 +51,24 @@ function parseArgs () { const args = process.argv.slice(2) let dir = process.cwd() let format = 'json' + let saveReport = false for (let i = 0; i < args.length; i++) { if (args[i] === '--dir' && args[i + 1]) dir = path.resolve(args[++i]) else if (args[i] === '--format' && args[i + 1]) format = args[++i] + else if (args[i] === '--save-report') saveReport = true } - if (!['json', 'markdown'].includes(format)) { - throw new Error(`Unsupported format: ${format}. Expected json or markdown.`) + if (!['json', 'markdown', 'summary'].includes(format)) { + throw new Error(`Unsupported format: ${format}. Expected json, markdown, or summary.`) } - return { dir, format } + if (format === 'summary' && !saveReport) { + throw new Error('Summary format requires --save-report so its complete evidence is persisted.') + } + if (saveReport && format !== 'summary') { + throw new Error('--save-report is only supported with --format summary.') + } + return { dir, format, saveReport } } function formatCliError (error) { @@ -918,7 +927,16 @@ if (require.main === module) { } if (args) { runAudit(args.dir, { onProgress: message => process.stderr.write(message) }) - .then(summary => process.stdout.write(args.format === 'markdown' ? `${renderAuditReport(summary)}\n` : `${JSON.stringify(summary)}\n`)) + .then(async summary => { + if (args.format === 'summary') { + const output = await createSavedAuditOutput(summary, args.dir) + process.stdout.write(output.executiveSummary) + } else if (args.format === 'markdown') { + process.stdout.write(`${renderAuditReport(summary)}\n`) + } else { + process.stdout.write(`${JSON.stringify(summary)}\n`) + } + }) .catch(error => { process.stderr.write(formatCliError(error)) process.exitCode = 1 diff --git a/skills/ns-audit-dependencies/audit-report-output.cjs b/skills/ns-audit-dependencies/audit-report-output.cjs new file mode 100644 index 0000000..27dc467 --- /dev/null +++ b/skills/ns-audit-dependencies/audit-report-output.cjs @@ -0,0 +1,81 @@ +'use strict' + +const crypto = require('crypto') +const fs = require('fs') +const path = require('path') +const { renderAuditReport, renderAuditSummary, validateAuditSummary } = require('./render-audit-report.cjs') + +const RETRYABLE_TRANSPORT_REASONS = new Set(['network', 'rate-limit', 'server', 'timeout']) + +function reportTimestamp (now) { + const date = now instanceof Date ? now : new Date(now) + if (Number.isNaN(date.getTime())) throw new Error('Invalid audit report timestamp') + return date.toISOString().replace(/[:.]/g, '-') +} + +function auditOutputError (code, message) { + const error = new Error(message) + error.code = code + return error +} + +function allPackagesUncheckedFor (summary, allowedReasons) { + return summary.packages.total > 0 && + summary.packages.checked === 0 && + summary.packages.unchecked === summary.packages.total && + summary.uncheckedPackages.every(pkg => allowedReasons.has(pkg.reason)) +} + +function assertPublishableAuditSummary (summary) { + validateAuditSummary(summary) + if (allPackagesUncheckedFor(summary, RETRYABLE_TRANSPORT_REASONS)) { + const reasons = Array.from(new Set(summary.uncheckedPackages.map(pkg => pkg.reason))).sort() + throw auditOutputError( + 'AUDIT_REPORT_RETRY_REQUIRED', + `All ${summary.packages.total} package versions were unchecked due to retryable transport failures (${reasons.join(', ')}); no report was saved.` + ) + } + if (allPackagesUncheckedFor(summary, new Set(['authentication']))) { + throw auditOutputError( + 'AUDIT_REPORT_AUTHENTICATION_REQUIRED', + `All ${summary.packages.total} package versions were unchecked because NCM authentication failed; no report was saved.` + ) + } +} + +async function saveAuditReport (projectDir, markdown, options = {}) { + const now = options.now || new Date() + const assetsDir = path.resolve(projectDir, '.nsolid', 'assets') + const baseName = `dependency-audit-${reportTimestamp(now)}` + const contents = markdown.endsWith('\n') ? markdown : `${markdown}\n` + const tempPath = path.join(assetsDir, `.${baseName}-${process.pid}-${crypto.randomUUID()}.tmp`) + + await fs.promises.mkdir(assetsDir, { recursive: true, mode: 0o700 }) + try { + await fs.promises.writeFile(tempPath, contents, { encoding: 'utf8', flag: 'wx', mode: 0o600 }) + for (let collision = 0; ; collision++) { + const suffix = collision === 0 ? '' : `-${collision + 1}` + const reportPath = path.join(assetsDir, `${baseName}${suffix}.md`) + try { + await fs.promises.link(tempPath, reportPath) + return reportPath + } catch (error) { + if (!error || error.code !== 'EEXIST') throw error + } + } + } finally { + await fs.promises.unlink(tempPath).catch(error => { + if (!error || error.code !== 'ENOENT') throw error + }) + } +} + +async function createSavedAuditOutput (summary, projectDir, options = {}) { + assertPublishableAuditSummary(summary) + const report = `${renderAuditReport(summary)}\n` + const reportPath = await saveAuditReport(projectDir, report, options) + const executiveSummary = `${renderAuditSummary(summary, reportPath)}\n` + return { executiveSummary, report, reportPath } +} + +module.exports = { assertPublishableAuditSummary, createSavedAuditOutput, reportTimestamp, saveAuditReport } diff --git a/skills/ns-audit-dependencies/render-audit-report.cjs b/skills/ns-audit-dependencies/render-audit-report.cjs index 7eff6c8..6a160fa 100644 --- a/skills/ns-audit-dependencies/render-audit-report.cjs +++ b/skills/ns-audit-dependencies/render-audit-report.cjs @@ -1,5 +1,7 @@ 'use strict' +const path = require('path') + const SEVERITIES = ['critical', 'high', 'medium', 'low', 'unknown'] const SEVERITY_RANK = { unknown: 0, low: 1, medium: 2, high: 3, critical: 4 } const REMEDIATION_KEYS = { @@ -209,6 +211,159 @@ function dependencyScope (finding, short = false) { : 'no root declaration name match; may be transitive or workspace-owned' } +function remediationText (finding) { + const remediation = finding.remediation + if (remediation.status === 'ncm-verified') { + return `ncm-verified → ${escapeMarkdown(remediation.version)} (${escapeMarkdown(remediation.changeType)})` + } + if (remediation.status === 'not-required') { + return `not-required (${escapeMarkdown(remediation.reason || 'withdrawn-only')})` + } + return escapeMarkdown(remediation.status) +} + +function groupVerifiedActions (analysis) { + const groups = new Map() + for (const { finding, renderedSeverity } of analysis.activeFindings) { + if (finding.remediation.status !== 'ncm-verified') continue + const key = [ + finding.name, + finding.remediation.version, + finding.remediation.changeType, + finding.remediation.source + ].join('\u0000') + if (!groups.has(key)) { + groups.set(key, { + name: finding.name, + target: finding.remediation.version, + changeType: finding.remediation.changeType, + source: finding.remediation.source, + installed: new Set(), + severity: renderedSeverity + }) + } + const group = groups.get(key) + group.installed.add(finding.version) + if (SEVERITY_RANK[renderedSeverity] > SEVERITY_RANK[group.severity]) group.severity = renderedSeverity + } + return Array.from(groups.values()).sort((left, right) => { + return SEVERITY_RANK[right.severity] - SEVERITY_RANK[left.severity] || + left.name.localeCompare(right.name) || + left.target.localeCompare(right.target) || + left.changeType.localeCompare(right.changeType) || + left.source.localeCompare(right.source) + }) +} + +function reportLink (reportPath, pathApi = path) { + const absolutePath = pathApi.resolve(reportPath) + const target = absolutePath + .split(pathApi.sep).join('/') + .replace(/%/g, '%25') + .replace(//g, '%3E') + .replace(/#/g, '%23') + .replace(/\?/g, '%3F') + return `[Open the complete dependency audit report](<${target}>)` +} + +function renderAuditSummary (summary, reportPath) { + const analysis = validateAuditSummary(summary) + if (typeof reportPath !== 'string' || reportPath.length === 0 || !path.isAbsolute(reportPath)) { + throw integrityError('summary requires an absolute saved report path') + } + + const critical = analysis.activeFindings.filter(item => item.renderedSeverity === 'critical') + const verified = analysis.activeFindings.filter(item => item.finding.remediation.status === 'ncm-verified') + const verifiedActions = groupVerifiedActions(analysis) + const unresolved = analysis.activeFindings.filter(item => item.finding.remediation.status === 'unresolved') + const verificationFailed = analysis.activeFindings.filter(item => item.finding.remediation.status === 'verification-failed') + const lines = [ + '## Executive Summary', + '', + `- ${summary.packages.checked} of ${summary.packages.total} package versions checked; ${summary.packages.unchecked} unchecked.`, + `- ${analysis.activeRecords} active vulnerability ${noun(analysis.activeRecords, 'record')} across ${analysis.activeFindings.length} actively affected package ${noun(analysis.activeFindings.length, 'version')}: ${severityText(analysis.activeBySeverity)}.`, + `- ${analysis.withdrawnRecords} withdrawn ${noun(analysis.withdrawnRecords, 'record')} were retained in the complete report; ${analysis.withdrawnOnlyFindings.length} package ${noun(analysis.withdrawnOnlyFindings.length, 'version')} were withdrawn-only.`, + `- Remediation: ${summary.remediation.verified} NCM-verified, ${summary.remediation.unresolved} unresolved, ${summary.remediation.verificationFailed} verification failures, ${summary.remediation.notRequired} not required.`, + `- Terminal batch failures: ${summary.batchFailures.total} (${countsText(summary.batchFailures.byReason)}). Recovery restored ${summary.batchRecovery.recoveredPackages} package versions.` + ] + + if (summary.ncmContentTruncation.truncatedFields > 0) { + lines.push(`- NCM content truncation: ${summary.ncmContentTruncation.truncatedFields} fields and ${summary.ncmContentTruncation.truncatedCharacters} characters truncated.`) + } else { + lines.push('- No NCM field truncation occurred.') + } + + lines.push('', '## Critical Findings', '') + if (critical.length === 0) { + lines.push('No active critical package-version findings were returned.') + } else { + for (const { finding } of critical) { + const ids = Array.from(new Set(finding.vulnerabilities + .filter(vulnerability => vulnerability.withdrawn !== true) + .map(vulnerability => vulnerability.id || 'ID not returned'))) + lines.push(`- ${escapeMarkdown(finding.name)}@${escapeMarkdown(finding.version)} — ${ids.map(escapeMarkdown).join(', ')}; ${remediationText(finding)}.`) + } + } + + lines.push('', '## Verified Upgrade Actions', '') + if (verifiedActions.length === 0) { + lines.push('No NCM-verified upgrade targets were found.') + } else { + lines.push(`${verified.length} verified package-version ${noun(verified.length, 'finding')} ${verified.length === 1 ? 'is' : 'are'} represented by ${verifiedActions.length} grouped upgrade ${noun(verifiedActions.length, 'action')}.`) + lines.push('', '| Installed | Target | Change | Verification |') + lines.push('| --- | --- | --- | --- |') + for (const action of verifiedActions) { + const installed = Array.from(action.installed) + .sort((left, right) => left.localeCompare(right, undefined, { numeric: true })) + .map(version => `${action.name}@${version}`) + .join(', ') + lines.push(`| ${escapeMarkdown(installed)} | ${escapeMarkdown(action.target)} | ${escapeMarkdown(action.changeType)} | ${escapeMarkdown(action.source)} |`) + } + lines.push('', 'Run the package manager’s `why` command before changing a manifest; exact manifest ownership remains unproven.') + } + + lines.push('', '## Findings Requiring Follow-up', '') + if (unresolved.length === 0 && verificationFailed.length === 0) { + lines.push('No active findings remain unresolved or verification-failed.') + } else { + if (unresolved.length > 0) { + const packages = unresolved.map(({ finding }) => `${finding.name}@${finding.version}`).map(escapeMarkdown) + lines.push(`- Unresolved (${unresolved.length}): ${packages.join(', ')}.`) + } + if (verificationFailed.length > 0) { + const packages = verificationFailed.map(({ finding }) => `${finding.name}@${finding.version}`).map(escapeMarkdown) + lines.push(`- Verification failed (${verificationFailed.length}): ${packages.join(', ')}.`) + } + } + + lines.push('', '## Withdrawn-Only Findings', '') + if (analysis.withdrawnOnlyFindings.length === 0) { + lines.push('No withdrawn-only package-version findings were returned.') + } else { + const packages = analysis.withdrawnOnlyFindings + .map(({ finding }) => `${finding.name}@${finding.version}`) + .map(escapeMarkdown) + lines.push(`${analysis.withdrawnOnlyFindings.length} withdrawn-only package ${noun(analysis.withdrawnOnlyFindings.length, 'version')} require no remediation: ${packages.join(', ')}.`) + } + + lines.push('', '## Coverage Gaps', '') + if (summary.packages.unchecked === 0) { + lines.push('No package versions were left unchecked.') + } else { + lines.push(`${summary.packages.unchecked} unchecked package ${noun(summary.packages.unchecked, 'version')} were not proven safe: ${countsText(summary.packages.uncheckedByReason)}.`) + lines.push('The complete report contains every unchecked package-version identifier and reason.') + } + + lines.push('', '## Complete Report', '') + lines.push(reportLink(reportPath)) + lines.push('') + lines.push(`The underlying complete report reconciles ${analysis.findings}/${summary.vulnerabilities.affectedPackages} package-version findings, ${analysis.vulnerabilities}/${summary.vulnerabilities.total} vulnerability records, and ${analysis.unchecked}/${summary.packages.unchecked} unchecked package versions.`) + lines.push('This executive summary intentionally omits per-record evidence, ranges, URLs, module risks, code-quality details, and the full unchecked-package list; use the linked report for those details.') + lines.push('“NCM-verified” means free of active NCM advisories at audit time—not absolutely safe.') + return lines.join('\n') +} + function renderFinding (item, index, findingTotal, renderState, recordTotal) { const { finding, renderedSeverity } = item const lines = [ @@ -392,4 +547,4 @@ function renderAuditReport (summary) { return lines.join('\n') } -module.exports = { analyzeFindings, escapeMarkdown, renderAuditReport, validateAuditSummary } +module.exports = { analyzeFindings, escapeMarkdown, renderAuditReport, renderAuditSummary, reportLink, validateAuditSummary } From 5783f96bd75c313932892195b9e8c6d479e6620d Mon Sep 17 00:00:00 2001 From: Cesar-M-Diaz Date: Fri, 17 Jul 2026 18:17:35 +0200 Subject: [PATCH 6/6] fix: reference the summary format in the skill --- .../audit-dependencies-renderer.test.ts | 29 +++++++++++++++++++ skills/ns-audit-dependencies/SKILL.md | 14 ++++++++- .../render-audit-report.cjs | 6 ++++ 3 files changed, 48 insertions(+), 1 deletion(-) diff --git a/packages/core/test/unit/skills/audit-dependencies-renderer.test.ts b/packages/core/test/unit/skills/audit-dependencies-renderer.test.ts index 7ff0121..88d4929 100644 --- a/packages/core/test/unit/skills/audit-dependencies-renderer.test.ts +++ b/packages/core/test/unit/skills/audit-dependencies-renderer.test.ts @@ -142,6 +142,19 @@ test('executive summary groups verified targets and lists every unresolved and w assert.equal(renderAuditSummary(summary, reportPath), executiveSummary) }) +test('executive summary always emits the fixed section contract in order', () => { + const executiveSummary = renderAuditSummary(summaryFor([]), path.resolve('dependency-audit.md')) + assert.deepEqual(executiveSummary.match(/^## .+$/gm), [ + '## Executive Summary', + '## Critical Findings', + '## Verified Upgrade Actions', + '## Findings Requiring Follow-up', + '## Withdrawn-Only Findings', + '## Coverage Gaps', + '## Complete Report' + ]) +}) + test('report links normalize Windows paths and encode Markdown-significant filename characters', () => { assert.equal( reportLink('C:\\Users\\Example User\\project\\.nsolid\\assets\\audit#1?.md', path.win32), @@ -259,6 +272,22 @@ test('integrity validation rejects mismatched totals with a stable error code', }) }) +test('integrity validation rejects contradictory failure totals and reason counts', () => { + const batchMismatch = summaryFor([]) + batchMismatch.batchFailures = { total: 1, byReason: {} } + assert.throws( + () => validateAuditSummary(batchMismatch), + /batch failures: expected 1, received 0/ + ) + + const remediationMismatch = summaryFor([]) + remediationMismatch.remediation.failures = { total: 2, byReason: { server: 1 } } + assert.throws( + () => validateAuditSummary(remediationMismatch), + /remediation failures: expected 2, received 1/ + ) +}) + test('integrity validation rejects invalid active and withdrawn remediation partitions', () => { const withdrawn = finding(0) withdrawn.vulnerabilities[0].withdrawn = true diff --git a/skills/ns-audit-dependencies/SKILL.md b/skills/ns-audit-dependencies/SKILL.md index 1c558dc..398b668 100644 --- a/skills/ns-audit-dependencies/SKILL.md +++ b/skills/ns-audit-dependencies/SKILL.md @@ -22,7 +22,19 @@ If the helper reports `AUDIT_REPORT_AUTHENTICATION_REQUIRED`, it deliberately sa ## Present the report -Present successful summary stdout without rewriting it. Keep progress-only stderr outside the response because its aggregates are already rendered. Treat the linked file—not the chat summary—as the authoritative complete report. +Treat successful summary stdout as the complete final chat response. Return it verbatim, preserving its wording, heading order, line breaks, lists, tables, counts, and report link; only the final trailing newline may be omitted. Do not add a preface, conclusion, interpretation, save question, or follow-up offer. Keep progress-only stderr outside the response because its aggregates are already rendered. Treat the linked file—not the chat summary—as the authoritative complete report. + +The deterministic summary always uses this section order: + +1. `## Executive Summary` +2. `## Critical Findings` +3. `## Verified Upgrade Actions` +4. `## Findings Requiring Follow-up` +5. `## Withdrawn-Only Findings` +6. `## Coverage Gaps` +7. `## Complete Report` + +Do not recreate this structure from the saved report or use a model-authored summary template. If successful stdout does not follow this contract, report an output-integrity failure instead of repairing it. If the helper reports `AUDIT_REPORT_INTEGRITY_ERROR` or fails to save a publishable report, state that the audit report is incomplete. Do not reconstruct a report from partial output and do not save model-rewritten report text. diff --git a/skills/ns-audit-dependencies/render-audit-report.cjs b/skills/ns-audit-dependencies/render-audit-report.cjs index 6a160fa..ea4d3aa 100644 --- a/skills/ns-audit-dependencies/render-audit-report.cjs +++ b/skills/ns-audit-dependencies/render-audit-report.cjs @@ -49,6 +49,10 @@ function stableCounts (values) { return Object.fromEntries(Object.entries(values).sort(([a], [b]) => a.localeCompare(b))) } +function sumCounts (values) { + return Object.values(values || {}).reduce((total, count) => total + count, 0) +} + function assertEqual (actual, expected, label) { if (actual !== expected) throw integrityError(`${label}: expected ${expected}, received ${actual}`) } @@ -145,6 +149,8 @@ function validateAuditSummary (summary) { verificationFailed: summary.remediation.verificationFailed, notRequired: summary.remediation.notRequired }, 'remediation') + assertEqual(sumCounts(summary.batchFailures.byReason), summary.batchFailures.total, 'batch failures') + assertEqual(sumCounts(summary.remediation.failures.byReason), summary.remediation.failures.total, 'remediation failures') const uncheckedCounts = {} for (const pkg of summary.uncheckedPackages) {