feat(git-node): select security release PRs from affectedVersions

components/git/release.js

@@ -14,10 +14,6 @@ const PROMOTE = 'promote';
 const RELEASERS = 'releasers';
 
 const releaseOptions = {
-  filterLabel: {
-    describe: 'Labels separated by "," to filter security PRs',
-    type: 'string'
-  },
   'gpg-sign': {
     describe: 'GPG-sign commits, will be passed to the git process',
     alias: 'S'
@@ -51,7 +47,9 @@ const releaseOptions = {
   },
   security: {
     describe: 'Demarcate the new security release as a security release. ' +
-              'Optionally provide path to security-release repository for CVE auto-population',
+              'Optionally provide the path to vulnerabilities.json (or the ' +
+              'security-release repository) to cherry-pick the reports and ' +
+              'dependency updates for this release line and auto-populate CVEs',
     type: 'string',
     coerce: (arg) => {
       // If --security=path is used, return the path

components/git/security.js

@@ -12,6 +12,7 @@ import SecurityBlog from '../../lib/security_blog.js';
 import SecurityAnnouncement from '../../lib/security-announcement.js';
 import { forceRunAsync } from '../../lib/run.js';
 import { getAffectedVersionLines } from '../../lib/security-release/security-release.js';
+import { getPullRequestURLForLine } from '../../lib/prepare_release.js';
 
 export const command = 'security [options]';
 export const describe = 'Manage an in-progress security release or start a new one.';
@@ -305,9 +306,12 @@ async function applySecurityPatches(cli) {
   const { releaseDate, reports, dependencies } = await fetchVulnerabilitiesDotJSON(cli, req);
 
   let patchedVersion;
-  for (const { affectedVersions, prURL, title } of Object.values(dependencies).flat()) {
-    if (!getAffectedVersionLines(affectedVersions).includes(`${nodeMajorVersion}.x`)) continue;
-    cli.separator(`Taking care of ${title}...`);
+  const releaseLine = `${nodeMajorVersion}.x`;
+  for (const [name, dep] of Object.entries(dependencies ?? {})) {
+    const prURL = getPullRequestURLForLine(
+      dep.affectedVersions, releaseLine, dep.prURL);
+    if (!prURL) continue;
+    cli.separator(`Taking care of ${name}...`);
     if (await skipIfExisting(cli, prURL)) continue;
 
     const argv = parsePRFromURL(prURL);

lib/prepare_release.js

@@ -6,18 +6,48 @@ import { replaceInFile } from 'replace-in-file';
 
 import { forceRunAsync, runAsync, runSync } from './run.js';
 import { writeJson, readJson } from './file.js';
-import Request from './request.js';
-import auth from './auth.js';
 import {
   getEOLDate,
   getStartLTSBlurb,
   updateTestProcessRelease
 } from './release/utils.js';
 import CherryPick from './cherry_pick.js';
 import Session from './session.js';
+import { getAffectedVersionLines } from './security-release/security-release.js';
 
 const isWindows = process.platform === 'win32';
 
+// Parse a GitHub Pull Request URL into its owner, repo and number.
+// Returns null when the URL does not look like a PR URL.
+export function parsePullRequestURL(url) {
+  const match = /github\.com\/([^/]+)\/([^/]+)\/pull\/(\d+)/.exec(url || '');
+  if (!match) return null;
+  return { owner: match[1], repo: match[2], number: Number(match[3]) };
+}
+
+// Resolve the PR URL that fixes a given release line from a report or
+// dependency `affectedVersions` entry.
+// Supports the current schema, where `affectedVersions` maps a release line to
+// its PR URL (e.g. `{ '22.x': 'https://.../pull/1' }`), and the legacy schema,
+// where `affectedVersions` is an array of lines paired with a single `prURL`.
+export function getPullRequestURLForLine(affectedVersions, line, legacyPrURL) {
+  if (
+    affectedVersions &&
+    typeof affectedVersions === 'object' &&
+    !Array.isArray(affectedVersions)
+  ) {
+    return affectedVersions[line] || null;
+  }
+
+  if (Array.isArray(affectedVersions) && legacyPrURL) {
+    return getAffectedVersionLines(affectedVersions).includes(line)
+      ? legacyPrURL
+      : null;
+  }
+
+  return null;
+}
+
 export default class ReleasePreparation extends Session {
   constructor(argv, cli, dir) {
     super(cli, dir);
@@ -32,101 +62,137 @@ export default class ReleasePreparation extends Session {
     this.defaultReleaseDate = argv.releaseDate ?? new Date().toISOString().slice(0, 10);
     this.ltsCodename = '';
     this.date = '';
-    this.filterLabels = argv.filterLabel && argv.filterLabel.split(',');
     this.newVersion = argv.newVersion;
   }
 
   get branch() {
     return this.isSecurityRelease ? this.releaseBranch : this.stagingBranch;
   }
 
-  warnForNonMergeablePR(pr) {
+  // Resolve the path to the security release vulnerabilities.json.
+  // `--security` may point either directly at a vulnerabilities.json file or at
+  // the root of the security-release repository, in which case the well-known
+  // sub-path is appended.
+  getVulnerabilitiesJSONPath() {
+    if (!this.securityReleaseRepo) return null;
+    if (this.securityReleaseRepo.endsWith('.json')) {
+      return this.securityReleaseRepo;
+    }
+    return path.join(
+      this.securityReleaseRepo,
+      'security-release',
+      'next-security-release',
+      'vulnerabilities.json'
+    );
+  }
+
+  loadVulnerabilitiesJSON() {
     const { cli } = this;
+    const vulnPath = this.getVulnerabilitiesJSONPath();
 
-    cli.warn(`PR#${pr.number} - ${pr.title} is not 'MERGEABLE'.
-      Status: ${pr.mergeable}`);
+    if (!vulnPath) {
+      cli.error('No vulnerabilities.json path provided. Pass the path to ' +
+        'vulnerabilities.json (or the security-release repository) via ' +
+        '`--security=<path>`.');
+      return null;
+    }
+
+    if (!existsSync(vulnPath)) {
+      cli.error(`vulnerabilities.json not found at ${vulnPath}.`);
+      return null;
+    }
+
+    try {
+      cli.startSpinner(`Reading vulnerabilities.json from ${vulnPath}..`);
+      const vulnData = JSON.parse(readFileSync(vulnPath, 'utf-8'));
+      cli.stopSpinner(`Done reading vulnerabilities.json from ${vulnPath}`);
+      return vulnData;
+    } catch (err) {
+      cli.error(`Failed to read vulnerabilities.json: ${err.message}`);
+      return null;
+    }
   }
 
-  async getOpenPRs(filterLabels) {
-    const credentials = await auth({ github: true });
-    const request = new Request(credentials);
-    const data = await request.gql('PRs', {
-      owner: this.owner,
-      repo: this.repo,
-      labels: filterLabels
-    });
-    return data.repository.pullRequests.nodes;
+  // Build the list of PRs to cherry-pick for a given release line from both the
+  // reports and the dependency updates, using their `affectedVersions` mapping.
+  getSecurityCherryPickTargets(vulnData, line) {
+    const targets = [];
+
+    for (const report of vulnData.reports ?? []) {
+      const url = getPullRequestURLForLine(
+        report.affectedVersions, line, report.prURL);
+      if (url) {
+        targets.push({
+          url,
+          cveIds: report.cveIds?.length ? report.cveIds : null,
+          label: report.title || report.id
+        });
+      }
+    }
+
+    for (const [name, dep] of Object.entries(vulnData.dependencies ?? {})) {
+      const url = getPullRequestURLForLine(
+        dep.affectedVersions, line, dep.prURL);
+      if (url) {
+        targets.push({ url, cveIds: null, label: `dependency: ${name}` });
+      }
+    }
+
+    return targets;
+  }
+
+  upstreamForPR({ owner, repo }) {
+    if (owner === 'nodejs-private') {
+      return `https://${this.username}:${this.config.token}` +
+        `@github.com/${owner}/${repo}.git`;
+    }
+    return `https://github.com/${owner}/${repo}.git`;
   }
 
-  async cherryPickSecurityPRs(filterLabels) {
+  async cherryPickSecurityPRs() {
     const { cli } = this;
+    const line = `${this.versionComponents.major}.x`;
 
-    const prs = await this.getOpenPRs(filterLabels);
-    if (prs.length === 0) {
-      cli.warn(`There are no PRs available in ${this.owner}/${this.repo}`);
+    const vulnData = this.loadVulnerabilitiesJSON();
+    if (!vulnData) {
+      cli.error('Cannot proceed with a security release without a valid ' +
+        'vulnerabilities.json.');
       return false;
     }
 
-    const vulnCveMap = new Map();
-    if (this.isSecurityRelease && this.securityReleaseRepo) {
-      const vulnPath = path.join(
-        this.securityReleaseRepo,
-        'security-release',
-        'next-security-release',
-        'vulnerabilities.json'
-      );
-
-      if (!existsSync(vulnPath)) {
-        cli.error(`vulnerabilities.json not found at ${vulnPath}. ` +
-          'Skipping CVE auto-population.');
-        cli.warn('PRs will require manual CVE-ID entry.');
-      } else {
-        try {
-          cli.startSpinner(`Reading vulnerabilities.json from ${vulnPath}..`);
-          const vulnData = JSON.parse(readFileSync(vulnPath, 'utf-8'));
-          cli.stopSpinner(`Done reading vulnerabilities.json from ${vulnPath}`);
-
-          if (vulnData.reports && Array.isArray(vulnData.reports)) {
-            vulnData.reports.forEach(report => {
-              if (report.prURL && report.cveIds && report.cveIds.length > 0) {
-                vulnCveMap.set(report.prURL, report.cveIds);
-              }
-            });
-          }
-          cli.ok(`Loaded ${vulnCveMap.size} CVE mappings from vulnerabilities.json`);
-        } catch (err) {
-          cli.error(`Failed to read vulnerabilities.json: ${err.message}`);
-          cli.warn('Continuing without CVE auto-population.');
-        }
-      }
+    const targets = this.getSecurityCherryPickTargets(vulnData, line);
+    if (targets.length === 0) {
+      cli.warn(`No PRs found in vulnerabilities.json for release line ${line}.`);
+      return false;
     }
 
-    for (const pr of prs) {
-      if (pr.mergeable !== 'MERGEABLE') {
-        this.warnForNonMergeablePR(pr);
-      }
+    cli.info(`Cherry-picking ${targets.length} PR(s) for ${line} from ` +
+      'vulnerabilities.json');
 
-      // Look up CVE-IDs from vulnerabilities.json
-      const prUrl = `https://github.com/${this.owner}/${this.repo}/pull/${pr.number}`;
-      const cveIds = vulnCveMap.get(prUrl);
+    for (const target of targets) {
+      const pr = parsePullRequestURL(target.url);
+      if (!pr) {
+        cli.warn(`Skipping invalid PR URL for ${target.label}: ${target.url}`);
+        continue;
+      }
 
-      if (!cveIds || cveIds.length === 0) {
-        cli.warn(`No CVE-IDs found in vulnerabilities.json for ${prUrl}`);
+      if (!target.cveIds) {
+        cli.warn(`No CVE-IDs found in vulnerabilities.json for ${target.url}`);
       }
 
+      cli.info(`Cherry-picking ${target.label} (${target.url})`);
       const cp = new CherryPick(pr.number, this.dir, cli, {
-        owner: this.owner,
-        repo: this.repo,
+        owner: pr.owner,
+        repo: pr.repo,
         gpgSign: this.gpgSign,
-        upstream: this.isSecurityRelease ? `https://${this.username}:${this.config.token}@github.com/${this.owner}/${this.repo}.git` : this.upstream,
+        upstream: this.upstreamForPR(pr),
         lint: false,
         includeCVE: true,
-        cveIds: cveIds || null,
-        vulnCveMap
+        cveIds: target.cveIds
       });
       const success = await cp.start();
       if (!success) {
-        cli.warn('The cherry-pick has failed. PR-ID: ' + pr.number);
+        cli.warn(`The cherry-pick has failed. PR: ${target.url}`);
         return false;
       }
     }
@@ -147,7 +213,7 @@ export default class ReleasePreparation extends Session {
       { defaultAnswer: this.runBranchDiff });
     if (runBranchDiff) {
       if (isSecurityRelease) {
-        const success = await this.cherryPickSecurityPRs(this.filterLabels);
+        const success = await this.cherryPickSecurityPRs();
         if (!success) {
           cli.error('Aborting security release preparation.');
           return;

lib/prepare_security.js

@@ -17,7 +17,6 @@ import {
   writeSecurityFile,
   SecurityRelease
 } from './security-release/security-release.js';
-import _ from 'lodash';
 
 function relativeDate(date) {
   const days = Math.floor((Date.now() - date) / (1000 * 60 * 60 * 24));
@@ -261,8 +260,7 @@ export default class PrepareSecurityRelease extends SecurityRelease {
     const reports = reportSelectionMode === 'include-all'
       ? await this.includeAllTriagedReports(excludedReports)
       : await this.chooseReports(excludedReports);
-    const depUpdates = await this.getDependencyUpdates();
-    const deps = _.groupBy(depUpdates, 'name');
+    const deps = await this.getDependencyUpdates();
 
     // create the vulnerabilities.json file in the security-release repo
     const filePath = await this.createVulnerabilitiesJSON(reports, deps, releaseDate);
@@ -558,8 +556,11 @@ export default class PrepareSecurityRelease extends SecurityRelease {
     process.exit(1);
   }
 
+  // Collect dependency updates using the same schema as the reports: each
+  // dependency maps every affected release line to the PR that lands it, e.g.
+  //   { undici: { affectedVersions: { '22.x': '<prURL>', '24.x': '<prURL>' } } }
   async getDependencyUpdates() {
-    const deps = [];
+    const deps = {};
     const updates = await this.cli.prompt('Are there dependency updates in this security release?',
       {
         defaultAnswer: true,
@@ -583,20 +584,24 @@ export default class PrepareSecurityRelease extends SecurityRelease {
         });
 
       const versions = await this.cli.prompt(
-        'Which release line does this dependency update affect?', {
+        'Which release line(s) does this dependency update PR affect? ' +
+        '(comma-separated)', {
           defaultAnswer: supportedVersions,
           questionType: 'input'
         });
 
       try {
         const res = await this.req.getPullRequest(dep);
-        const { html_url, title } = res;
-        deps.push({
-          name,
-          url: html_url,
-          title,
-          affectedVersions: versions.split(',').map((v) => v.replace('v', '').trim())
-        });
+        const { html_url: prURL } = res;
+        const affectedVersions = versions
+          .split(',')
+          .map((v) => v.replace('v', '').trim())
+          .filter(Boolean);
+
+        deps[name] ??= { affectedVersions: {} };
+        for (const line of affectedVersions) {
+          deps[name].affectedVersions[line] = prURL;
+        }
         this.cli.separator();
       } catch (error) {
         this.cli.error('Invalid PR url. Please provide a valid PR url.');

lib/security_blog.js

@@ -287,9 +287,13 @@ export default class SecurityBlog extends SecurityRelease {
     if (Object.keys(dependencyUpdates).length === 0) return '';
     let template = '\nThis security release includes the following dependency' +
       ' updates to address public vulnerabilities:\n';
-    for (const [dependency, { versions, affectedVersions }] of Object.entries(dependencyUpdates)) {
-      const releaseLines = getAffectedVersionLines(affectedVersions);
-      template += `- ${dependency} (${versions.join(', ')}) on ${releaseLines.join(', ')}\n`;
+    for (const [dependency, entry] of Object.entries(dependencyUpdates)) {
+      const releaseLines = getAffectedVersionLines(entry.affectedVersions);
+      const versions = Array.isArray(entry.versions)
+        ? entry.versions.map((v) => (typeof v === 'string' ? v : v.version))
+        : [];
+      const versionSuffix = versions.length ? ` (${versions.join(', ')})` : '';
+      template += `- ${dependency}${versionSuffix} on ${releaseLines.join(', ')}\n`;
     }
     return template;
   }