From 4c4cf9c36412fb104fa5ad3867741278039a5e22 Mon Sep 17 00:00:00 2001 From: RafaelGSS Date: Wed, 23 Sep 2026 10:33:45 -0300 Subject: [PATCH 1/4] fix(security): fetch all pages of triaged reports Follow HackerOne pagination and deduplicate overlapping reports. Reject failed pages and invalid pagination links instead of returning a partial candidate list. --- lib/request.js | 44 ++++++++++++++++--- test/unit/request.test.js | 89 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 126 insertions(+), 7 deletions(-) diff --git a/lib/request.js b/lib/request.js index 0bafd73f..5db022f9 100644 --- a/lib/request.js +++ b/lib/request.js @@ -193,22 +193,52 @@ export default class Request { } async getTriagedReports() { - const url = 'https://api.hackerone.com/v1/reports?filter[program][]=nodejs&filter[state][]=triaged'; + let url = 'https://api.hackerone.com/v1/reports?filter[program][]=nodejs&filter[state][]=triaged'; const options = { method: 'GET', + redirect: 'error', headers: { Authorization: `Basic ${this.credentials.h1}`, 'User-Agent': 'node-core-utils', Accept: 'application/json' } }; - const data = await this.json(url, options); - if (data?.errors) { - throw new Error( - `Request to fetch triaged reports failed with: ${JSON.stringify(data.errors)}` - ); + const reports = []; + const reportIds = new Set(); + const pages = new Set(); + let result; + + while (url) { + const pageUrl = new URL(url); + if (pageUrl.origin !== 'https://api.hackerone.com' || + pageUrl.pathname !== '/v1/reports' || pageUrl.username || pageUrl.password) { + throw new Error('Invalid HackerOne reports pagination URL'); + } + if (pages.has(pageUrl.href)) { + throw new Error('Repeated HackerOne reports pagination URL'); + } + pages.add(pageUrl.href); + + result = await this.json(url, options); + if (result?.errors?.length) { + throw new Error( + `Request to fetch triaged reports failed with: ${JSON.stringify(result.errors)}` + ); + } + if (!Array.isArray(result?.data)) { + throw new Error('Invalid HackerOne reports response: expected a data array'); + } + for (const report of result.data) { + if (reportIds.has(report.id)) continue; + reportIds.add(report.id); + reports.push(report); + } + + const next = result.links?.next; + url = next ? new URL(next, pageUrl).href : null; } - return data; + + return { ...result, data: reports }; } async getPrograms() { diff --git a/test/unit/request.test.js b/test/unit/request.test.js index d4d637c2..9fbbe608 100644 --- a/test/unit/request.test.js +++ b/test/unit/request.test.js @@ -31,6 +31,95 @@ describe('Request', () => { }); }); + describe('getTriagedReports', () => { + it('fetches every page and includes overlapping reports only once', async() => { + const request = createRequest({}); + request.credentials.h1 = 'h1-credentials'; + const calls = []; + const next = 'https://api.hackerone.com/v1/reports?page[number]=2'; + request.json = async(url, options) => { + calls.push(url); + assert.strictEqual(options.method, 'GET'); + assert.strictEqual(options.redirect, 'error'); + assert.strictEqual(options.headers.Authorization, 'Basic h1-credentials'); + return calls.length === 1 + ? { data: [{ id: '1' }, { id: '2' }], links: { next } } + : { data: [{ id: '2' }, { id: '3' }], links: { next: null } }; + }; + + const result = await request.getTriagedReports(); + + assert.strictEqual(calls.length, 2); + assert.strictEqual(calls[1], next); + assert.deepStrictEqual(result.data, [{ id: '1' }, { id: '2' }, { id: '3' }]); + assert.strictEqual(result.links.next, null); + }); + + it('accepts an empty list', async() => { + const request = createRequest({ data: [] }); + assert.deepStrictEqual(await request.getTriagedReports(), { data: [] }); + }); + + it('resolves relative next-page links', async() => { + const request = createRequest({}); + let calls = 0; + request.json = async(url) => { + if (++calls === 1) { + return { data: [{ id: '1' }], links: { next: '?page[number]=2' } }; + } + assert.strictEqual(url, 'https://api.hackerone.com/v1/reports?page[number]=2'); + return { data: [{ id: '2' }] }; + }; + assert.strictEqual((await request.getTriagedReports()).data.length, 2); + }); + + it('rejects a failed later page instead of returning an incomplete list', async() => { + const request = createRequest({}); + let calls = 0; + request.json = async() => ++calls === 1 + ? { data: [{ id: '1' }], links: { next: '?page[number]=2' } } + : { errors: [{ detail: 'Rate limit exceeded' }] }; + + await assert.rejects(request.getTriagedReports(), /Rate limit exceeded/); + }); + + it('rejects malformed responses instead of treating them as empty pages', async() => { + for (const response of [null, {}, { data: {} }]) { + const request = createRequest(response); + await assert.rejects(request.getTriagedReports(), /expected a data array/); + } + }); + + it('does not send credentials to an unrelated pagination endpoint', async() => { + for (const next of [ + 'https://example.com/v1/reports', + 'http://api.hackerone.com/v1/reports', + 'https://api.hackerone.com/v1/users', + 'https://user@api.hackerone.com/v1/reports' + ]) { + const request = createRequest({}); + let calls = 0; + request.json = async() => { + calls++; + return { data: [], links: { next } }; + }; + await assert.rejects(request.getTriagedReports(), /Invalid HackerOne reports pagination/); + assert.strictEqual(calls, 1); + } + }); + + it('rejects pagination loops', async() => { + const request = createRequest({}); + let calls = 0; + request.json = async(url) => { + calls++; + return { data: [{ id: '1' }], links: { next: url } }; + }; + await assert.rejects(request.getTriagedReports(), /Repeated HackerOne reports pagination/); + assert.strictEqual(calls, 1); + }); + }); + describe('query', () => { it('preserves detailed GraphQL errors', async() => { const variables = { owner: 'nodejs', repo: 'node', prid: 65130 }; From d3ab16d876f0056b63937c0f1e1ca8876bd6da2d Mon Sep 17 00:00:00 2001 From: RafaelGSS Date: Wed, 23 Sep 2026 10:38:59 -0300 Subject: [PATCH 2/4] refactor(security): separate release data preparation from the CLI Expose candidate retrieval and draft preparation without prompts or publication. Reuse one candidate snapshot in the CLI and normalize dates and release-line maps while preserving incomplete metadata for review. --- docs/git-node.md | 40 +++++- lib/prepare_security.js | 124 +++++------------- lib/security-release/preparation.js | 168 +++++++++++++++++++++++++ test/unit/security_preparation.test.js | 163 ++++++++++++++++++++++++ 4 files changed, 402 insertions(+), 93 deletions(-) create mode 100644 lib/security-release/preparation.js create mode 100644 test/unit/security_preparation.test.js diff --git a/docs/git-node.md b/docs/git-node.md index 4102f94b..5868031c 100644 --- a/docs/git-node.md +++ b/docs/git-node.md @@ -494,9 +494,43 @@ $ ncu-config --global set h1_username $H1_TOKEN ### `git node security --start` -This command creates the Next Security Issue in Node.js private repository -following the [Security Release Process][] document. -It will retrieve all the triaged HackerOne reports and add creates the `vulnerabilities.json`. +This command prepares `vulnerabilities.json` and can open the Next Security +Release pull request in `nodejs-private/security-release`, following the +[Security Release Process][] document. It retrieves all pages of triaged +HackerOne reports and uses the same candidate list for exclusions and selection. +The CLI prompts for the release date, report selection, and dependency updates +before writing the draft. Commit, push, and PR creation remain separate +confirmed steps. + +#### Preparing release data without the CLI + +`lib/security-release/preparation.js` exposes helpers that can be used by other +local tools: + +- `listSecurityReleaseCandidates(request)` accepts an authenticated NCU + `Request` and returns the triaged HackerOne report objects. A failed page + rejects the operation rather than returning an incomplete candidate list. + It does not fetch extra report history or make report-selection decisions. +- `buildIncludedTriagedReport(report, options)` converts a HackerOne report to + release metadata. Supply affected lines and patch authors explicitly; it + does not discover them or fetch a patch. +- `prepareSecurityRelease({ releaseDate, reports, dependencies })` accepts the + selected release report entries and dependency updates. It returns + `{ release, missingInformation }`, without requests, prompts, filesystem + writes, Git operations, or publication. + +The preparation helper accepts `TBD`, `YYYY/MM/DD`, or `YYYY-MM-DD` dates and +normalizes defined dates to `YYYY-MM-DD`. It accepts legacy affected-line arrays +and strings as well as PR maps, and writes maps keyed by release line. Existing +map URLs are preserved. A report's canonical PR is not automatically assigned +to every affected line: unknown backport URLs remain empty. Legacy dependency +updates retain their explicit association between their PR and affected lines. + +The result is an independent copy of the input. Duplicate report IDs, invalid +dates, and conflicting release-line mappings are rejected. Missing report +metadata is listed for follow-up; a draft with no reports can still contain +dependency updates. These checks prepare a draft, not a final-release approval. +The caller owns selection, human review, persistence, and publication. ### `git node security --apply-patches` diff --git a/lib/prepare_security.js b/lib/prepare_security.js index ae9f6fe2..2665a653 100644 --- a/lib/prepare_security.js +++ b/lib/prepare_security.js @@ -3,6 +3,15 @@ import path from 'node:path'; import auth from './auth.js'; import Request from './request.js'; import { parsePRFromURL } from './links.js'; +import { + buildIncludedTriagedReport, + getReportPRURL, + getMissingReportInformation, + groupMissingReportInformation, + listSecurityReleaseCandidates, + prepareSecurityRelease +} from './security-release/preparation.js'; + import { NEXT_SECURITY_RELEASE_BRANCH, NEXT_SECURITY_RELEASE_FOLDER, @@ -12,13 +21,18 @@ import { promptDependencies, getSupportedVersions, getReportSeverity, - getSummary, pickReport, confirmSecurityStep, writeSecurityFile, SecurityRelease } from './security-release/security-release.js'; +export { + buildIncludedTriagedReport, + getMissingReportInformation, + groupMissingReportInformation +} from './security-release/preparation.js'; + function relativeDate(date) { const days = Math.floor((Date.now() - date) / (1000 * 60 * 60 * 24)); if (days < 30) return days === 1 ? '1 day ago' : `${days} days ago`; @@ -82,75 +96,6 @@ export function getNextTuesdayReleaseDateChoices(fromDate = new Date(), count = return choices; } -function getReportPRURL(report) { - const customFieldValues = report.relationships.custom_field_values?.data ?? []; - return customFieldValues[0]?.attributes?.value ?? ''; -} - -export function buildIncludedTriagedReport(report, options = {}) { - const { - affectedVersions = '', - patchAuthors = [], - prURL = getReportPRURL(report) - } = options; - const { - id, - attributes: { title, cve_ids = [] }, - relationships: { reporter } - } = report; - const link = `https://hackerone.com/reports/${id}`; - const summaryContent = getSummary(report); - - return { - id, - title, - cveIds: cve_ids, - severity: getReportSeverity(report), - summary: summaryContent ?? '', - patchAuthors, - prURL, - affectedVersions: affectedVersions - .split(',') - .map((v) => v.replace('v', '').trim()) - .filter(Boolean), - link, - reporter: reporter?.data?.attributes?.username ?? '' - }; -} - -export function getMissingReportInformation(report) { - const missing = []; - - if (!report.severity?.rating) missing.push('severity rating'); - if (!report.severity?.cvss_vector_string) missing.push('CVSS vector'); - if (!report.severity?.weakness_id) missing.push('weakness ID'); - if (!report.summary) missing.push('team summary'); - if (!report.prURL) missing.push('PR URL'); - if (!report.patchAuthors?.length) missing.push('patch authors'); - if (!report.affectedVersions?.length) missing.push('affected versions'); - - return missing; -} - -export function groupMissingReportInformation(reports) { - const grouped = new Map(); - - for (const report of reports) { - for (const field of report.missing) { - const current = grouped.get(field) ?? []; - current.push(report); - grouped.set(field, current); - } - } - - return Array.from(grouped.entries()) - .map(([field, fieldReports]) => ({ - field, - reports: fieldReports - })) - .sort((a, b) => b.reports.length - a.reports.length); -} - export default class PrepareSecurityRelease extends SecurityRelease { title = 'Next Security Release'; @@ -172,14 +117,15 @@ export default class PrepareSecurityRelease extends SecurityRelease { const content = await this.buildDescription(releaseDate); if (createVulnerabilitiesJSON) { const reportSelectionMode = await this.promptReportSelectionMode(); + const candidates = await listSecurityReleaseCandidates(this.req); if (reportSelectionMode === 'review') { const showTriaged = await this.promptShowTriagedWithoutPR(); if (showTriaged) { - excludedReports = await this.showTriagedReportsWithoutPR(); + excludedReports = await this.showTriagedReportsWithoutPR(candidates); } } await this.startVulnerabilitiesJSONCreation( - releaseDate, content, excludedReports, reportSelectionMode); + releaseDate, content, excludedReports, reportSelectionMode, candidates); } this.cli.ok('Done!'); @@ -251,15 +197,16 @@ export default class PrepareSecurityRelease extends SecurityRelease { releaseDate, content, excludedReports = [], - reportSelectionMode = 'review' + reportSelectionMode = 'review', + candidates ) { // checkout on the next-security-release branch await checkoutOnSecurityReleaseBranch(this.cli, this.repository); // choose the reports to include in the security release const reports = reportSelectionMode === 'include-all' - ? await this.includeAllTriagedReports(excludedReports) - : await this.chooseReports(excludedReports); + ? await this.includeAllTriagedReports(excludedReports, candidates) + : await this.chooseReports(excludedReports, candidates); const deps = await this.getDependencyUpdates(); // create the vulnerabilities.json file in the security-release repo @@ -366,11 +313,11 @@ export default class PrepareSecurityRelease extends SecurityRelease { { defaultAnswer: true }); } - async showTriagedReportsWithoutPR() { + async showTriagedReportsWithoutPR(candidates) { this.cli.info('Fetching triaged reports without PR URL...'); - const reports = await this.req.getTriagedReports(); - const reportsWithoutPR = reports.data.filter( - (report) => !report.relationships.custom_field_values.data.length + const reports = candidates ?? await listSecurityReleaseCandidates(this.req); + const reportsWithoutPR = reports.filter( + (report) => !getReportPRURL(report) ); if (!reportsWithoutPR.length) { this.cli.ok('All triaged reports have a PR URL.'); @@ -408,12 +355,12 @@ export default class PrepareSecurityRelease extends SecurityRelease { return template; } - async chooseReports(excludedReports = []) { + async chooseReports(excludedReports = [], candidates) { this.cli.info('Getting triaged H1 reports...'); - const reports = await this.req.getTriagedReports(); + const reports = candidates ?? await listSecurityReleaseCandidates(this.req); const selectedReports = []; - for (const report of reports.data) { + for (const report of reports) { if (excludedReports.includes(report.id)) continue; const rep = await pickReport(report, { cli: this.cli, req: this.req }); if (!rep) continue; @@ -422,14 +369,14 @@ export default class PrepareSecurityRelease extends SecurityRelease { return selectedReports; } - async includeAllTriagedReports(excludedReports = []) { + async includeAllTriagedReports(excludedReports = [], candidates) { this.cli.info('Getting triaged H1 reports...'); - const reports = await this.req.getTriagedReports(); + const reports = candidates ?? await listSecurityReleaseCandidates(this.req); const supportedVersions = await getSupportedVersions(); const selectedReports = []; const missingInformation = []; - for (const report of reports.data) { + for (const report of reports) { if (excludedReports.includes(report.id)) continue; const reportData = await this.buildIncludedTriagedReport( @@ -498,11 +445,8 @@ export default class PrepareSecurityRelease extends SecurityRelease { async createVulnerabilitiesJSON(reports, dependencies, releaseDate) { this.cli.startSpinner('Creating vulnerabilities.json...'); - const fileContent = JSON.stringify({ - releaseDate, - reports, - dependencies - }, null, 2) + '\n'; + const { release } = prepareSecurityRelease({ releaseDate, reports, dependencies }); + const fileContent = JSON.stringify(release, null, 2) + '\n'; const folderPath = path.resolve(NEXT_SECURITY_RELEASE_FOLDER); const fullPath = path.join(folderPath, 'vulnerabilities.json'); diff --git a/lib/security-release/preparation.js b/lib/security-release/preparation.js new file mode 100644 index 00000000..37ee8c8c --- /dev/null +++ b/lib/security-release/preparation.js @@ -0,0 +1,168 @@ +import { getReportSeverity, getSummary } from './security-release.js'; + +// Request.getTriagedReports rejects failed pages rather than returning partial data. +// Selection, metadata edits, and all mutations belong to the caller. +export async function listSecurityReleaseCandidates(request) { + const { data } = await request.getTriagedReports(); + return data; +} + +export function getReportPRURL(report) { + const customFieldValues = report.relationships.custom_field_values?.data ?? []; + return customFieldValues[0]?.attributes?.value ?? ''; +} + +export function buildIncludedTriagedReport(report, options = {}) { + const { + affectedVersions = '', + patchAuthors = [], + prURL = getReportPRURL(report) + } = options; + const { + id, + attributes: { title, cve_ids = [] }, + relationships: { reporter } + } = report; + const link = `https://hackerone.com/reports/${id}`; + const summaryContent = getSummary(report); + + return { + id, + title, + cveIds: cve_ids, + severity: getReportSeverity(report), + summary: summaryContent ?? '', + patchAuthors, + prURL, + affectedVersions: affectedVersions + .split(',') + .map((v) => v.replace('v', '').trim()) + .filter(Boolean), + link, + reporter: reporter?.data?.attributes?.username ?? '' + }; +} + +export function getMissingReportInformation(report) { + const missing = []; + + if (!report.severity?.rating) missing.push('severity rating'); + if (!report.severity?.cvss_vector_string) missing.push('CVSS vector'); + if (!report.severity?.weakness_id) missing.push('weakness ID'); + if (!report.summary) missing.push('team summary'); + if (!report.prURL) missing.push('PR URL'); + if (!report.patchAuthors?.length) missing.push('patch authors'); + if (!Object.keys(report.affectedVersions ?? {}).length) missing.push('affected versions'); + + return missing; +} + +export function groupMissingReportInformation(reports) { + const grouped = new Map(); + + for (const report of reports) { + for (const field of report.missing) { + const current = grouped.get(field) ?? []; + current.push(report); + grouped.set(field, current); + } + } + + return Array.from(grouped.entries()) + .map(([field, fieldReports]) => ({ + field, + reports: fieldReports + })) + .sort((a, b) => b.reports.length - a.reports.length); +} + +function normalizeReleaseDate(value) { + if (value === 'TBD') return value; + if (typeof value !== 'string' || !/^\d{4}([/-])\d{2}\1\d{2}$/.test(value)) { + throw new Error('Release date must be YYYY-MM-DD, YYYY/MM/DD, or TBD'); + } + const date = value.replaceAll('/', '-'); + const timestamp = Date.parse(`${date}T00:00:00Z`); + if (!Number.isFinite(timestamp) || new Date(timestamp).toISOString().slice(0, 10) !== date) { + throw new Error('Invalid release date'); + } + return date; +} + +function normalizeAffectedVersions(value, fallbackPR = '') { + if (value == null || value === '') return {}; + if (typeof fallbackPR !== 'string') throw new Error('PR URL must be a string'); + let entries; + if (typeof value === 'string' || Array.isArray(value)) { + const lines = typeof value === 'string' ? value.split(',') : value; + entries = lines.map((line) => [line, fallbackPR]); + } else if (typeof value === 'object') { + entries = Object.entries(value); + } else { + throw new Error('Affected versions must be release lines or a PR map'); + } + + const result = new Map(); + for (const [line, prURL] of entries) { + if (typeof line !== 'string') throw new Error('Release line must be a string'); + const trimmed = line.trim(); + const match = /^v?(\d+)(?:\.x)?$/.exec(trimmed); + const normalized = trimmed === 'main' ? 'main' : match && `${match[1]}.x`; + if (!normalized) throw new Error(`Invalid release line: ${line}`); + if (typeof prURL !== 'string') throw new Error(`PR URL for ${normalized} must be a string`); + if (result.has(normalized) && result.get(normalized) !== prURL) { + throw new Error(`Conflicting PR URLs for ${normalized}`); + } + result.set(normalized, prURL); + } + return Object.fromEntries(result); +} + +// This function only builds data. It does not fetch reports, prompt, write files, +// change branches, commit, push, or create a pull request. +export function prepareSecurityRelease({ releaseDate, reports, dependencies = {} }) { + const date = normalizeReleaseDate(releaseDate); + if (!Array.isArray(reports)) throw new Error('Reports must be an array'); + if (!dependencies || typeof dependencies !== 'object' || Array.isArray(dependencies)) { + throw new Error('Dependencies must be an object'); + } + + const ids = new Set(); + const selected = reports.map((report) => { + if (!report || typeof report.id !== 'string' || !/^\d+$/.test(report.id)) { + throw new Error('Report ID must be a numeric string'); + } + if (ids.has(report.id)) throw new Error(`Duplicate report ID: ${report.id}`); + ids.add(report.id); + return { + ...structuredClone(report), + // A canonical patch is not evidence of a backport for every affected line. + affectedVersions: normalizeAffectedVersions(report.affectedVersions) + }; + }); + + const deps = Object.fromEntries(Object.entries(dependencies).map(([name, updates]) => { + const normalizeUpdate = (update) => { + if (!update || typeof update !== 'object' || Array.isArray(update)) { + throw new Error(`Invalid dependency update: ${name}`); + } + return { + ...structuredClone(update), + affectedVersions: normalizeAffectedVersions(update.affectedVersions, update.prURL ?? '') + }; + }; + const normalized = Array.isArray(updates) + ? updates.map(normalizeUpdate) + : normalizeUpdate(updates); + return [name, normalized]; + })); + + return { + release: { releaseDate: date, reports: selected, dependencies: deps }, + missingInformation: selected.flatMap((report) => { + const missing = getMissingReportInformation(report); + if (!missing.length) return []; + return [{ id: report.id, title: report.title, link: report.link, missing }]; + }) + }; +} diff --git a/test/unit/security_preparation.test.js b/test/unit/security_preparation.test.js new file mode 100644 index 00000000..8d51fac2 --- /dev/null +++ b/test/unit/security_preparation.test.js @@ -0,0 +1,163 @@ +import { describe, it } from 'node:test'; +import assert from 'node:assert'; + +import { + buildIncludedTriagedReport, + getMissingReportInformation, + listSecurityReleaseCandidates, + prepareSecurityRelease +} from '../../lib/security-release/preparation.js'; +import PrepareSecurityRelease from '../../lib/prepare_security.js'; + +function h1Report(id = '123') { + return { + id, + attributes: { title: 'Example report', cve_ids: [] }, + relationships: { custom_field_values: { data: [] } } + }; +} + +function report(id = '123') { + return buildIncludedTriagedReport(h1Report(id)); +} + +describe('security release preparation', () => { + it('lists candidates through the supplied read-only request', async() => { + const reports = [h1Report('1'), h1Report('2')]; + const request = { + async getTriagedReports() { + return { data: reports }; + } + }; + assert.deepStrictEqual(await listSecurityReleaseCandidates(request), reports); + }); + + it('propagates retrieval failures', async() => { + const request = { + async getTriagedReports() { + throw new Error('Second page unavailable'); + } + }; + await assert.rejects(listSecurityReleaseCandidates(request), /Second page unavailable/); + }); + + it('uses the supplied candidate snapshot for exclusions and selection', async() => { + const candidates = [h1Report('1'), h1Report('2')]; + const messages = []; + const cli = { + info() {}, + separator() {}, + promptCheckbox(message, choices) { + assert.deepStrictEqual(choices.map(({ value }) => value), ['1', '2']); + return ['2']; + }, + prompt(message) { + messages.push(message); + return false; + } + }; + const release = new PrepareSecurityRelease(cli); + release.req = { + getTriagedReports() { + assert.fail('Do not fetch a different candidate snapshot'); + } + }; + const excluded = await release.showTriagedReportsWithoutPR(candidates); + assert.deepStrictEqual(await release.chooseReports(excluded, candidates), []); + assert.strictEqual(messages.length, 1); + }); + + it('prepares only the selected reports without mutating input', () => { + const selected = report('2'); + selected.affectedVersions = ['v24.x', '22.x', '24.x']; + selected.prURL = 'https://github.com/nodejs-private/node-private/pull/12'; + const input = { releaseDate: '2026/10/06', reports: [selected] }; + const before = structuredClone(input); + const { release, missingInformation } = prepareSecurityRelease(input); + + assert.strictEqual(release.releaseDate, '2026-10-06'); + assert.deepStrictEqual(release.reports.map(({ id }) => id), ['2']); + assert.deepStrictEqual(release.reports[0].affectedVersions, { '24.x': '', '22.x': '' }); + assert.strictEqual(missingInformation[0].id, '2'); + assert.ok(missingInformation[0].missing.includes('team summary')); + release.reports[0].cveIds.push('CVE-2026-0001'); + release.reports[0].severity.rating = 'high'; + assert.deepStrictEqual(input, before); + }); + + it('preserves explicit per-line patches and supports main', () => { + const selected = report(); + selected.affectedVersions = { + main: 'https://github.com/nodejs-private/node-private/pull/1', + 'v24.x': 'https://github.com/nodejs-private/node-private/pull/2', + '22.x': '' + }; + const { release } = prepareSecurityRelease({ releaseDate: 'TBD', reports: [selected] }); + assert.strictEqual(release.releaseDate, 'TBD'); + assert.deepStrictEqual(release.reports[0].affectedVersions, { + main: selected.affectedVersions.main, + '24.x': selected.affectedVersions['v24.x'], + '22.x': '' + }); + assert.ok(!getMissingReportInformation(release.reports[0]).includes('affected versions')); + }); + + it('keeps unknown affected versions explicit', () => { + const { release, missingInformation } = prepareSecurityRelease({ + releaseDate: 'TBD', reports: [report()] + }); + assert.deepStrictEqual(release.reports[0].affectedVersions, {}); + assert.ok(missingInformation[0].missing.includes('affected versions')); + }); + + it('prepares dependency-only releases and preserves legacy PR associations', () => { + const prURL = 'https://github.com/nodejs/node/pull/1'; + const dependencies = { + undici: [{ title: 'Update undici', prURL, affectedVersions: ['v24.x', '22.x'] }], + openssl: { affectedVersions: { '24.x': prURL, '22.x': '' } } + }; + const input = { releaseDate: '2026-10-06', reports: [], dependencies }; + const { release, missingInformation } = prepareSecurityRelease(input); + assert.deepStrictEqual(release.reports, []); + assert.deepStrictEqual(missingInformation, []); + assert.deepStrictEqual(release.dependencies.undici[0].affectedVersions, { + '24.x': prURL, '22.x': prURL + }); + assert.deepStrictEqual(release.dependencies.openssl.affectedVersions, { + '24.x': prURL, '22.x': '' + }); + release.dependencies.undici[0].title = 'Changed'; + assert.strictEqual(dependencies.undici[0].title, 'Update undici'); + }); + + it('rejects duplicate reports and invalid draft inputs', () => { + assert.throws(() => prepareSecurityRelease({ + releaseDate: 'TBD', reports: [report('1'), report('1')] + }), /Duplicate report ID/); + assert.throws(() => prepareSecurityRelease({ + releaseDate: 'TBD', reports: [{ id: 'not-an-id' }] + }), /Report ID/); + assert.throws(() => prepareSecurityRelease({ releaseDate: 'TBD', reports: null }), /Reports/); + assert.throws(() => prepareSecurityRelease({ + releaseDate: 'TBD', reports: [], dependencies: [] + }), /Dependencies/); + }); + + it('rejects invalid dates instead of silently rolling them forward', () => { + for (const releaseDate of ['', undefined, 'soon', '2026-02-30', '2026-13-01', '2026/10-06']) { + assert.throws(() => prepareSecurityRelease({ releaseDate, reports: [] }), /[Rr]elease date/); + } + }); + + it('rejects invalid release lines and conflicting normalized mappings', () => { + for (const affectedVersions of [ + ['invalid'], [24], 24, { '24.x': null }, + { '24.x': 'one', 'v24.x': 'two' } + ]) { + const selected = { ...report(), affectedVersions }; + assert.throws(() => prepareSecurityRelease({ + releaseDate: 'TBD', reports: [selected] + }), /release line|Release line|Affected versions|PR URL/); + } + }); +}); From b4c754473ffb8514a3944a2aca3e0ef072b3d60d Mon Sep 17 00:00:00 2001 From: RafaelGSS Date: Wed, 23 Sep 2026 10:41:58 -0300 Subject: [PATCH 3/4] fix(security): preserve existing release branches and staged work Check out existing release branches without resetting them and reuse remote-tracking branches when available. Reject unrelated staged files before adding release changes to the index. --- docs/git-node.md | 8 +- lib/security-release/security-release.js | 34 ++++++- test/unit/security_git.test.js | 121 +++++++++++++++++++++++ 3 files changed, 160 insertions(+), 3 deletions(-) create mode 100644 test/unit/security_git.test.js diff --git a/docs/git-node.md b/docs/git-node.md index 5868031c..02db2797 100644 --- a/docs/git-node.md +++ b/docs/git-node.md @@ -500,7 +500,13 @@ Release pull request in `nodejs-private/security-release`, following the HackerOne reports and uses the same candidate list for exclusions and selection. The CLI prompts for the release date, report selection, and dependency updates before writing the draft. Commit, push, and PR creation remain separate -confirmed steps. +confirmed steps. An existing `next-security-release` branch is checked out +without resetting it. If only `origin/next-security-release` exists locally, +the command creates a tracking branch from that ref. Fetch first when remote +state may have changed. + +Security release commits reject unrelated staged changes before staging their +own files. Commit or unstage those changes before continuing. #### Preparing release data without the CLI diff --git a/lib/security-release/security-release.js b/lib/security-release/security-release.js index 26044f9d..a710c2ea 100644 --- a/lib/security-release/security-release.js +++ b/lib/security-release/security-release.js @@ -79,10 +79,24 @@ export async function checkoutOnSecurityReleaseBranch(cli, repository) { cli.info(`Current branch: ${currentBranch} `); if (currentBranch !== NEXT_SECURITY_RELEASE_BRANCH) { + const refs = new Set(runSync('git', [ + 'for-each-ref', '--format=%(refname)', + `refs/heads/${NEXT_SECURITY_RELEASE_BRANCH}`, + `refs/remotes/origin/${NEXT_SECURITY_RELEASE_BRANCH}` + ]).trim().split('\n')); + let args; + if (refs.has(`refs/heads/${NEXT_SECURITY_RELEASE_BRANCH}`)) { + args = ['checkout', NEXT_SECURITY_RELEASE_BRANCH]; + } else if (refs.has(`refs/remotes/origin/${NEXT_SECURITY_RELEASE_BRANCH}`)) { + args = ['checkout', '-b', NEXT_SECURITY_RELEASE_BRANCH, + '--track', `origin/${NEXT_SECURITY_RELEASE_BRANCH}`]; + } else { + args = ['checkout', '-b', NEXT_SECURITY_RELEASE_BRANCH]; + } await runSecurityGitCommand( cli, - ['checkout', '-B', NEXT_SECURITY_RELEASE_BRANCH], - `This checks out or recreates the ${NEXT_SECURITY_RELEASE_BRANCH} branch locally.` + args, + `This switches to ${NEXT_SECURITY_RELEASE_BRANCH} without resetting an existing branch.` ); cli.ok(`Checkout on branch: ${NEXT_SECURITY_RELEASE_BRANCH} `); } @@ -95,6 +109,22 @@ export async function commitAndPushVulnerabilitiesJSON( ) { await checkRemote(cli, repository); + const files = (Array.isArray(filePath) ? filePath : [filePath]) + .map((file) => path.resolve(file)); + const stagedFiles = runSync('git', ['diff', '--cached', '--name-only', '-z']) + .split('\0').filter(Boolean); + const unrelated = stagedFiles.filter((file) => { + const absolute = path.resolve(file); + return !files.some((target) => + absolute === target || absolute.startsWith(`${target}${path.sep}`)); + }); + if (unrelated.length) { + throw new Error( + 'Unrelated staged changes would be included in the security release commit: ' + + unrelated.join(', ') + '. Commit or unstage them before continuing.' + ); + } + if (Array.isArray(filePath)) { for (const currentPath of filePath) { runSync('git', ['add', currentPath]); diff --git a/test/unit/security_git.test.js b/test/unit/security_git.test.js new file mode 100644 index 00000000..46ced3d3 --- /dev/null +++ b/test/unit/security_git.test.js @@ -0,0 +1,121 @@ +import { describe, it } from 'node:test'; +import assert from 'node:assert'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { execFileSync } from 'node:child_process'; + +import { + checkoutOnSecurityReleaseBranch, + commitAndPushVulnerabilitiesJSON, + NEXT_SECURITY_RELEASE_REPOSITORY +} from '../../lib/security-release/security-release.js'; + +function git(...args) { + return execFileSync('git', args, { encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }).trim(); +} + +function repository(t) { + const previous = process.cwd(); + const directory = fs.mkdtempSync(path.join(os.tmpdir(), 'ncu-security-git-')); + t.after(() => { + process.chdir(previous); + fs.rmSync(directory, { recursive: true, force: true }); + }); + process.chdir(directory); + git('init', '-b', 'main'); + git('config', 'user.name', 'Test User'); + git('config', 'user.email', 'test@example.com'); + git('config', 'commit.gpgsign', 'false'); + git('config', 'core.hooksPath', path.join(directory, 'no-hooks')); + git('commit', '--allow-empty', '-m', 'Initial commit'); + git('remote', 'add', 'origin', 'https://github.com/nodejs-private/security-release.git'); + return directory; +} + +const cli = { info() {}, ok() {}, prompt: async() => true }; + +describe('security release git state', { concurrency: false }, () => { + it('creates a new release branch from the current HEAD', async(t) => { + repository(t); + const head = git('rev-parse', 'HEAD'); + await checkoutOnSecurityReleaseBranch(cli, NEXT_SECURITY_RELEASE_REPOSITORY); + assert.strictEqual(git('branch', '--show-current'), 'next-security-release'); + assert.strictEqual(git('rev-parse', 'HEAD'), head); + }); + + it('checks out an existing release branch without resetting it', async(t) => { + repository(t); + const releaseHead = git('rev-parse', 'HEAD'); + git('branch', 'next-security-release'); + git('commit', '--allow-empty', '-m', 'Unrelated work'); + assert.notStrictEqual(git('rev-parse', 'HEAD'), releaseHead); + + await checkoutOnSecurityReleaseBranch(cli, NEXT_SECURITY_RELEASE_REPOSITORY); + + assert.strictEqual(git('branch', '--show-current'), 'next-security-release'); + assert.strictEqual(git('rev-parse', 'HEAD'), releaseHead); + }); + + it('starts from an existing remote-tracking release branch', async(t) => { + repository(t); + const releaseHead = git('rev-parse', 'HEAD'); + git('update-ref', 'refs/remotes/origin/next-security-release', releaseHead); + git('commit', '--allow-empty', '-m', 'Unrelated work'); + + await checkoutOnSecurityReleaseBranch(cli, NEXT_SECURITY_RELEASE_REPOSITORY); + + assert.strictEqual(git('rev-parse', 'HEAD'), releaseHead); + assert.strictEqual( + git('rev-parse', '--abbrev-ref', '@{upstream}'), 'origin/next-security-release'); + }); + + it('does not switch branches when the checkout is declined', async(t) => { + repository(t); + await assert.rejects(checkoutOnSecurityReleaseBranch({ + ...cli, prompt: async() => false + }, NEXT_SECURITY_RELEASE_REPOSITORY), /Aborted/); + assert.strictEqual(git('branch', '--show-current'), 'main'); + assert.strictEqual(git('branch', '--list', 'next-security-release'), ''); + }); + + it('rejects unrelated staged work before staging the release file', async(t) => { + repository(t); + fs.writeFileSync('unrelated notes.txt', 'User work\n'); + git('add', 'unrelated notes.txt'); + fs.writeFileSync('vulnerabilities.json', '{}\n'); + const index = git('write-tree'); + const head = git('rev-parse', 'HEAD'); + await assert.rejects(commitAndPushVulnerabilitiesJSON( + 'vulnerabilities.json', 'Prepare release', + { cli, repository: NEXT_SECURITY_RELEASE_REPOSITORY } + ), /Unrelated staged changes/); + assert.strictEqual(git('write-tree'), index); + assert.strictEqual(git('rev-parse', 'HEAD'), head); + assert.strictEqual(fs.readFileSync('unrelated notes.txt', 'utf8'), 'User work\n'); + }); + + it('allows staged release files and stops at a declined commit', async(t) => { + repository(t); + fs.mkdirSync('security-release'); + fs.writeFileSync('security-release/vulnerabilities.json', '{}\n'); + git('add', 'security-release/vulnerabilities.json'); + const index = git('write-tree'); + const prompts = []; + await assert.rejects(commitAndPushVulnerabilitiesJSON( + 'security-release', 'Prepare release', { + cli: { + ...cli, + async prompt(message) { + prompts.push(message); + return false; + } + }, + repository: NEXT_SECURITY_RELEASE_REPOSITORY + } + ), /Aborted/); + assert.strictEqual(prompts.length, 1); + assert.match(prompts[0], /git commit/); + assert.strictEqual(git('write-tree'), index); + }); +}); From 937e4886991873c0546657c3da47b2db2fe26252 Mon Sep 17 00:00:00 2001 From: RafaelGSS Date: Wed, 23 Sep 2026 10:48:30 -0300 Subject: [PATCH 4/4] fix(security): refuse to overwrite an existing release draft Expose local draft persistence with an explicit repository path and exclusive file creation. Prepare data before changing branches and check for an existing draft again after checkout. --- docs/git-node.md | 13 ++++- lib/prepare_security.js | 41 ++++++++------- lib/security-release/draft.js | 32 ++++++++++++ test/unit/security_draft.test.js | 88 ++++++++++++++++++++++++++++++++ test/unit/security_git.test.js | 80 +++++++++++++++++++++++++++++ 5 files changed, 235 insertions(+), 19 deletions(-) create mode 100644 lib/security-release/draft.js create mode 100644 test/unit/security_draft.test.js diff --git a/docs/git-node.md b/docs/git-node.md index 02db2797..ed8e1460 100644 --- a/docs/git-node.md +++ b/docs/git-node.md @@ -506,7 +506,9 @@ the command creates a tracking branch from that ref. Fetch first when remote state may have changed. Security release commits reject unrelated staged changes before staging their -own files. Commit or unstage those changes before continuing. +own files. Commit or unstage those changes before continuing. `--start` refuses +to overwrite an existing draft, including one found after switching branches. +Use `--sync`, `--add-report`, or `--remove-report` to update that release. #### Preparing release data without the CLI @@ -538,6 +540,15 @@ metadata is listed for follow-up; a draft with no reports can still contain dependency updates. These checks prepare a draft, not a final-release approval. The caller owns selection, human review, persistence, and publication. +After reviewing the prepared data, local tools can use +`writeSecurityReleaseDraft(directory, release)` from +`lib/security-release/draft.js`. The directory is an explicit security-release +repository path. The helper validates the draft and creates +`security-release/next-security-release/vulnerabilities.json` with an exclusive +write, so an existing file cannot be overwritten. It does not change Git state +or publish anything. Review and authorization belong to the calling tool; the +CLI retains its directory and file-write confirmations. + ### `git node security --apply-patches` This command fetches the list of reports and the list of PRs labelled for the diff --git a/lib/prepare_security.js b/lib/prepare_security.js index 2665a653..b8ff5fb9 100644 --- a/lib/prepare_security.js +++ b/lib/prepare_security.js @@ -1,8 +1,12 @@ -import fs from 'node:fs'; import path from 'node:path'; import auth from './auth.js'; import Request from './request.js'; import { parsePRFromURL } from './links.js'; +import { + assertNewSecurityRelease, + getSecurityReleaseDraftPath, + writeSecurityReleaseDraft +} from './security-release/draft.js'; import { buildIncludedTriagedReport, getReportPRURL, @@ -14,7 +18,6 @@ import { import { NEXT_SECURITY_RELEASE_BRANCH, - NEXT_SECURITY_RELEASE_FOLDER, checkoutOnSecurityReleaseBranch, commitAndPushVulnerabilitiesJSON, validateDate, @@ -23,7 +26,6 @@ import { getReportSeverity, pickReport, confirmSecurityStep, - writeSecurityFile, SecurityRelease } from './security-release/security-release.js'; @@ -100,6 +102,7 @@ export default class PrepareSecurityRelease extends SecurityRelease { title = 'Next Security Release'; async start() { + assertNewSecurityRelease(process.cwd()); const credentials = await auth({ github: true, h1: true @@ -200,17 +203,22 @@ export default class PrepareSecurityRelease extends SecurityRelease { reportSelectionMode = 'review', candidates ) { - // checkout on the next-security-release branch - await checkoutOnSecurityReleaseBranch(this.cli, this.repository); + assertNewSecurityRelease(process.cwd()); // choose the reports to include in the security release const reports = reportSelectionMode === 'include-all' ? await this.includeAllTriagedReports(excludedReports, candidates) : await this.chooseReports(excludedReports, candidates); const deps = await this.getDependencyUpdates(); + const { release } = prepareSecurityRelease({ releaseDate, reports, dependencies: deps }); - // create the vulnerabilities.json file in the security-release repo - const filePath = await this.createVulnerabilitiesJSON(reports, deps, releaseDate); + // Prepare all data before changing Git state. An existing branch may contain + // a draft that was not present on the branch where this command started. + await checkoutOnSecurityReleaseBranch(this.cli, this.repository); + assertNewSecurityRelease(process.cwd()); + + const filePath = await this.createVulnerabilitiesJSON( + release.reports, release.dependencies, release.releaseDate); // review the vulnerabilities.json file const review = await this.promptReviewVulnerabilitiesJSON(); @@ -444,26 +452,23 @@ export default class PrepareSecurityRelease extends SecurityRelease { } async createVulnerabilitiesJSON(reports, dependencies, releaseDate) { - this.cli.startSpinner('Creating vulnerabilities.json...'); const { release } = prepareSecurityRelease({ releaseDate, reports, dependencies }); - const fileContent = JSON.stringify(release, null, 2) + '\n'; - - const folderPath = path.resolve(NEXT_SECURITY_RELEASE_FOLDER); - const fullPath = path.join(folderPath, 'vulnerabilities.json'); + const directory = process.cwd(); + const fullPath = getSecurityReleaseDraftPath(directory); + assertNewSecurityRelease(directory); await confirmSecurityStep( this.cli, - `create directory \`${folderPath}\``, + `create directory \`${path.dirname(fullPath)}\``, 'This creates the security release folder if it does not already exist.' ); - await fs.promises.mkdir(folderPath, { recursive: true }); - await writeSecurityFile( + await confirmSecurityStep( this.cli, - fullPath, - fileContent, + `write \`${fullPath}\``, 'This creates vulnerabilities.json for the next security release.' ); + this.cli.startSpinner('Creating vulnerabilities.json...'); + writeSecurityReleaseDraft(directory, release); this.cli.stopSpinner(`Created ${fullPath}`); - return fullPath; } diff --git a/lib/security-release/draft.js b/lib/security-release/draft.js new file mode 100644 index 00000000..38d04ba3 --- /dev/null +++ b/lib/security-release/draft.js @@ -0,0 +1,32 @@ +import fs from 'node:fs'; +import path from 'node:path'; + +import { NEXT_SECURITY_RELEASE_FOLDER } from './security-release.js'; +import { prepareSecurityRelease } from './preparation.js'; + +export function getSecurityReleaseDraftPath(directory) { + if (typeof directory !== 'string' || !directory.trim()) { + throw new Error('Security release repository directory is required'); + } + return path.resolve(directory, NEXT_SECURITY_RELEASE_FOLDER, 'vulnerabilities.json'); +} + +export function assertNewSecurityRelease(directory) { + const file = getSecurityReleaseDraftPath(directory); + if (fs.existsSync(file)) { + throw new Error( + `Security release draft already exists: ${file}. ` + + 'Use --sync, --add-report, or --remove-report to update the existing release.' + ); + } +} + +// Local persistence only. The caller handles review, Git, and publication. +export function writeSecurityReleaseDraft(directory, draft) { + const { release } = prepareSecurityRelease(draft); + const file = getSecurityReleaseDraftPath(directory); + assertNewSecurityRelease(directory); + fs.mkdirSync(path.dirname(file), { recursive: true }); + fs.writeFileSync(file, JSON.stringify(release, null, 2) + '\n', { flag: 'wx' }); + return file; +} diff --git a/test/unit/security_draft.test.js b/test/unit/security_draft.test.js new file mode 100644 index 00000000..50ff0769 --- /dev/null +++ b/test/unit/security_draft.test.js @@ -0,0 +1,88 @@ +import { describe, it } from 'node:test'; +import assert from 'node:assert'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; + +import { + assertNewSecurityRelease, + getSecurityReleaseDraftPath, + writeSecurityReleaseDraft +} from '../../lib/security-release/draft.js'; +import PrepareSecurityRelease from '../../lib/prepare_security.js'; + +function directory(t) { + const previous = process.cwd(); + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'ncu-security-draft-')); + t.after(() => { + process.chdir(previous); + fs.rmSync(dir, { recursive: true, force: true }); + }); + return dir; +} + +describe('security release draft persistence', () => { + it('writes a normalized draft only in the explicit repository directory', (t) => { + const dir = directory(t); + const file = writeSecurityReleaseDraft(dir, { + releaseDate: '2026/10/06', reports: [], dependencies: {} + }); + assert.strictEqual(file, path.join(dir, + 'security-release', 'next-security-release', 'vulnerabilities.json')); + assert.deepStrictEqual(JSON.parse(fs.readFileSync(file, 'utf8')), { + releaseDate: '2026-10-06', reports: [], dependencies: {} + }); + assert.deepStrictEqual(fs.readdirSync(dir), ['security-release']); + }); + + it('preserves an existing draft byte for byte', (t) => { + const dir = directory(t); + const draft = { releaseDate: 'TBD', reports: [] }; + const file = writeSecurityReleaseDraft(dir, draft); + const before = fs.readFileSync(file); + assert.throws(() => assertNewSecurityRelease(dir), /draft already exists/); + assert.throws(() => writeSecurityReleaseDraft(dir, { + releaseDate: '2026-10-06', reports: [] + }), /draft already exists/); + assert.deepStrictEqual(fs.readFileSync(file), before); + }); + + it('uses exclusive creation even if the file appears after preflight', (t) => { + const dir = directory(t); + const file = getSecurityReleaseDraftPath(dir); + const mkdir = fs.mkdirSync; + t.mock.method(fs, 'mkdirSync', (...args) => { + mkdir(...args); + fs.writeFileSync(file, 'Another session\n'); + }); + assert.throws(() => writeSecurityReleaseDraft(dir, { + releaseDate: 'TBD', reports: [] + }), { code: 'EEXIST' }); + assert.strictEqual(fs.readFileSync(file, 'utf8'), 'Another session\n'); + }); + + it('validates inputs before creating directories', (t) => { + const dir = directory(t); + assert.throws(() => writeSecurityReleaseDraft(dir, { + releaseDate: '2026-02-30', reports: [] + }), /Invalid release date/); + assert.deepStrictEqual(fs.readdirSync(dir), []); + assert.throws(() => getSecurityReleaseDraftPath(), /directory is required/); + }); + + it('does not write a draft when the file-write confirmation is declined', async(t) => { + const dir = directory(t); + const previous = process.cwd(); + t.after(() => process.chdir(previous)); + process.chdir(dir); + let prompts = 0; + const release = new PrepareSecurityRelease({ + async prompt() { + return ++prompts === 1; + } + }); + await assert.rejects(release.createVulnerabilitiesJSON([], {}, 'TBD'), /Aborted: write/); + assert.strictEqual(prompts, 2); + assert.deepStrictEqual(fs.readdirSync(dir), []); + }); +}); diff --git a/test/unit/security_git.test.js b/test/unit/security_git.test.js index 46ced3d3..c08efb98 100644 --- a/test/unit/security_git.test.js +++ b/test/unit/security_git.test.js @@ -5,6 +5,9 @@ import os from 'node:os'; import path from 'node:path'; import { execFileSync } from 'node:child_process'; +import PrepareSecurityRelease from '../../lib/prepare_security.js'; +import { writeSecurityReleaseDraft } from '../../lib/security-release/draft.js'; + import { checkoutOnSecurityReleaseBranch, commitAndPushVulnerabilitiesJSON, @@ -118,4 +121,81 @@ describe('security release git state', { concurrency: false }, () => { assert.match(prompts[0], /git commit/); assert.strictEqual(git('write-tree'), index); }); + + it('rejects an existing draft before prompting or fetching reports', async(t) => { + const dir = repository(t); + const file = writeSecurityReleaseDraft(dir, { releaseDate: 'TBD', reports: [] }); + const before = fs.readFileSync(file); + const release = new PrepareSecurityRelease({ + prompt() { assert.fail('Existing releases must not start again'); } + }); + await assert.rejects(release.start(), /draft already exists/); + assert.strictEqual(git('branch', '--show-current'), 'main'); + assert.deepStrictEqual(fs.readFileSync(file), before); + }); + + it('preserves Git state if draft preparation fails', async(t) => { + repository(t); + const release = new PrepareSecurityRelease(cli); + release.chooseReports = async() => []; + release.getDependencyUpdates = async() => { + throw new Error('Preparation interrupted'); + }; + await assert.rejects( + release.startVulnerabilitiesJSONCreation('TBD', 'Release'), /Preparation interrupted/); + assert.strictEqual(git('branch', '--show-current'), 'main'); + assert.strictEqual(git('branch', '--list', 'next-security-release'), ''); + assert.strictEqual(git('status', '--porcelain'), ''); + }); + + it('preserves a draft discovered on the existing release branch', async(t) => { + const dir = repository(t); + git('checkout', '-b', 'next-security-release'); + const file = writeSecurityReleaseDraft(dir, { releaseDate: 'TBD', reports: [] }); + const before = fs.readFileSync(file); + git('add', 'security-release'); + git('commit', '-m', 'Existing release'); + const head = git('rev-parse', 'HEAD'); + git('checkout', 'main'); + assert.ok(!fs.existsSync(file)); + + const release = new PrepareSecurityRelease(cli); + release.chooseReports = async() => []; + release.getDependencyUpdates = async() => ({}); + await assert.rejects( + release.startVulnerabilitiesJSONCreation('2026-10-06', 'Release'), /draft already exists/); + + assert.deepStrictEqual(fs.readFileSync(file), before); + assert.strictEqual(git('rev-parse', 'HEAD'), head); + assert.strictEqual(git('status', '--porcelain'), ''); + }); + + it('creates a local draft without committing when publication is declined', async(t) => { + const dir = repository(t); + fs.writeFileSync('user-notes.txt', 'Staged user work\n'); + git('add', 'user-notes.txt'); + const index = git('write-tree'); + const head = git('rev-parse', 'HEAD'); + const release = new PrepareSecurityRelease({ + ...cli, startSpinner() {}, stopSpinner() {} + }); + release.chooseReports = async() => []; + release.getDependencyUpdates = async() => ({ + undici: { affectedVersions: { '24.x': 'https://github.com/nodejs/node/pull/1' } } + }); + release.promptReviewVulnerabilitiesJSON = async() => false; + release.createPullRequest = async() => assert.fail('Do not publish a local draft'); + + await release.startVulnerabilitiesJSONCreation('2026/10/06', 'Release'); + + const file = path.join(dir, 'security-release/next-security-release/vulnerabilities.json'); + const draft = JSON.parse(fs.readFileSync(file, 'utf8')); + assert.strictEqual(draft.releaseDate, '2026-10-06'); + assert.deepStrictEqual(draft.reports, []); + assert.strictEqual(draft.dependencies.undici.affectedVersions['24.x'], + 'https://github.com/nodejs/node/pull/1'); + assert.strictEqual(git('write-tree'), index); + assert.strictEqual(git('rev-parse', 'HEAD'), head); + assert.strictEqual(git('branch', '--show-current'), 'next-security-release'); + }); });