fix(git-node): adjust security --cleanup (#901)

Those bugs I found while doing the last security release. I can confirm this patch fixes it.
nodejs · Jan 23, 2025 · 3cb5255 · 3cb5255
1 parent cce9b81
commit 3cb5255
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 18 deletions.
diff --git a/lib/prepare_security.js b/lib/prepare_security.js
@@ -52,24 +52,27 @@ export default class PrepareSecurityRelease extends SecurityRelease {
     await this.closeAndRequestDisclosure(vulnerabilityJSON.reports);
 
     this.cli.info('Closing pull requests');
-    // For now, close the ones with vN.x label
-    await this.closePRWithLabel(this.getAffectedVersions(vulnerabilityJSON));
+    // For now, close the ones with Security Release label
+    await this.closePRWithLabel('Security Release');
 
-    const updateFolder = this.cli.prompt(
+    const updateFolder = await this.cli.prompt(
       // eslint-disable-next-line max-len
       `Would you like to update the next-security-release folder to ${vulnerabilityJSON.releaseDate}?`,
       { defaultAnswer: true });
     if (updateFolder) {
-      const newFolder = this.updateReleaseFolder(vulnerabilityJSON.releaseDate);
+      this.updateReleaseFolder(
+        vulnerabilityJSON.releaseDate.replaceAll('/', '-')
+      );
+      const securityReleaseFolder = path.join(process.cwd(), 'security-release');
       commitAndPushVulnerabilitiesJSON(
-        newFolder,
+        securityReleaseFolder,
         'chore: change next-security-release folder',
         { cli: this.cli, repository: this.repository }
       );
     }
     this.cli.info(`Merge pull request with:
         - git checkout main
-        - git merge --squash ${NEXT_SECURITY_RELEASE_BRANCH}
+        - git merge ${NEXT_SECURITY_RELEASE_BRANCH} --no-ff -m "chore: add latest security release"
         - git push origin main`);
     this.cli.ok('Done!');
   }
@@ -306,16 +309,17 @@ export default class PrepareSecurityRelease extends SecurityRelease {
       labels = [labels];
     }
 
-    const url = 'https://github.com/nodejs-private/node-private/pulls';
+    const url = 'https://github.com/nodejs-private/node-private/pull';
     this.cli.startSpinner('Closing GitHub Pull Requests...');
     // At this point, GitHub does not provide filters through their REST API
-    const prs = this.req.getPullRequest(url);
+    const prs = await this.req.getPullRequest(url);
     for (const pr of prs) {
-      if (pr.labels.some((l) => labels.includes(l))) {
-        this.cli.updateSpinner(`Closing Pull Request: ${pr.id}`);
-        await this.req.closePullRequest(pr.id);
+      if (pr.labels.some((l) => labels.includes(l.name))) {
+        this.cli.updateSpinner(`Closing Pull Request: ${pr.number}`);
+        await this.req.closePullRequest(pr.number,
+          { owner: 'nodejs-private', repo: 'node-private' });
       }
     }
-    this.cli.startSpinner('Closed GitHub Pull Requests.');
+    this.cli.stopSpinner('Closed GitHub Pull Requests.');
   }
 }
diff --git a/lib/request.js b/lib/request.js
@@ -109,14 +109,15 @@ export default class Request {
     return this.json(url, options);
   }
 
-  async closePullRequest({ owner, repo }) {
-    const url = `https://api.github.com/repos/${owner}/${repo}/pulls`;
+  async closePullRequest(id, { owner, repo }) {
+    const url = `https://api.github.com/repos/${owner}/${repo}/pulls/${id}`;
     const options = {
       method: 'POST',
       headers: {
         Authorization: `Basic ${this.credentials.github}`,
         'User-Agent': 'node-core-utils',
-        Accept: 'application/vnd.github+json'
+        Accept: 'application/vnd.github+json',
+        'Content-Type': 'application/json'
       },
       body: JSON.stringify({
         state: 'closed'
@@ -230,7 +231,8 @@ export default class Request {
       headers: {
         Authorization: `Basic ${this.credentials.h1}`,
         'User-Agent': 'node-core-utils',
-        Accept: 'application/json'
+        Accept: 'application/json',
+        'Content-Type': 'application/json'
       },
       body: JSON.stringify({
         data: {
@@ -252,11 +254,13 @@ export default class Request {
       headers: {
         Authorization: `Basic ${this.credentials.h1}`,
         'User-Agent': 'node-core-utils',
-        Accept: 'application/json'
+        Accept: 'application/json',
+        'Content-Type': 'application/json'
       },
       body: JSON.stringify({
         data: {
           attributes: {
+            message: 'Requesting disclosure',
             // default to limited version
             substate: 'no-content'
           }

diff --git a/lib/security-release/security-release.js b/lib/security-release/security-release.js
@@ -236,7 +236,7 @@ export class SecurityRelease {
   updateReleaseFolder(releaseDate) {
     const folder = path.join(process.cwd(),
       NEXT_SECURITY_RELEASE_FOLDER);
-    const newFolder = path.join(process.cwd(), releaseDate);
+    const newFolder = path.join(process.cwd(), 'security-release', releaseDate);
     fs.renameSync(folder, newFolder);
     return newFolder;
   }