Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lib: support security prepare #665

Merged
merged 1 commit into from
Jul 13, 2023

Conversation

RafaelGSS
Copy link
Member

@RafaelGSS RafaelGSS commented Feb 9, 2023

I'm doing an experiment as part of my work on nodejs/security-wg#860 + upcoming security release.

I was talking with @BethGriggs and looks reasonable to generate the proposal PR for security releases using git node release --security --prepare 19.6.1.

Currently, git node land doesn't work for the private repo, so the workflow is:

  1. Create the proposal branch (based on vX.X instead of staging)
  2. Iterate open PRs (based on the given filter) and ask to amend them in the proposal
  3. Proceed with the regular workflow (adjust the node_version, changelogs, ...)

Example:

node-private (main) ../node-core-utils/bin/git-node.js release --prepare --security 20.2.1
   ⚠  Remote repository URL does not point to the expected repository nodejs/node
   ⚠  You are trying to create a new release proposal branch for v20, but you're checked out on main and not v20.x.
--------------------------------------------------------------------------------
   ℹ  Switch to `v20.x` with `git checkout v20.x` before proceeding.
--------------------------------------------------------------------------------node-private (main) git checkout v20.x 
Switched to branch 'v20.x'
Your branch is up to date with 'upstream/v20.x'.node-private (v20.x) ../node-core-utils/bin/git-node.js release --prepare --security 20.2.1
   ⚠  Remote repository URL does not point to the expected repository nodejs/node
⠙ Creating new proposal branch for 20.2.1Branch 'v20.2.1-proposal' set up to track remote branch 'v20.x' from 'origin'.
Switched to a new branch 'v20.2.1-proposal'
✔  Created new proposal branch for 20.2.1
✔  Done loading data for nodejs-private/node-private/pull/393
----------------------------------- PR info ------------------------------------
Title      SENSITIVE: INFORMATION (#393)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     tniessen:h1-XXXXX -> nodejs-private:main
Labels
Commits    1
 - sensitive: information
Committers 1
 - Tobias Nießen <tniessen@tnie.de>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs-private/node-private/pull/393
Fixes: X
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
--------------------------------------------------------------------------------
   ℹ  This PR was created on Thu, 23 Feb 2023 15:18:32 GMT
   ✔  Approvals: 3
   ✔  - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs-private/node-private/pull/393#pullrequestreview-1352725924
   ✔  - Robert Nagy (@ronag) (TSC): https://github.com/nodejs-private/node-private/pull/393#pullrequestreview-1311573538
   ✔  - Rafael Gonzaga (@RafaelGSS) (TSC): https://github.com/nodejs-private/node-private/pull/393#pullrequestreview-1311635021
   ✖  Last GitHub CI failed
   ✖  No Jenkins CI runs detected
--------------------------------------------------------------------------------
? Would you like to amend this PR to the proposal? Yes
⠙ Downloading patch for 393From github.com:nodejs-private/node-private
 * branch                    refs/pull/393/merge -> FETCH_HEAD
✔  Fetched commits as 745083abb4a7..88b780e6e074
--------------------------------------------------------------------------------
[v20.2.1-proposal d79ce3ef3a4] sensitive: information
 Author: Tobias Nießen <tniessen@tnie.de>
 Date: Thu Feb 23 15:13:16 2023 +0000
 2 files changed, 43 insertions(+)
   ✔  Patches applied
--------------------------------------------------------------------------------
? Git found no CVE-ID trailer in the original commit message. Please, provide the CVE-ID CVE-2023-TEST123
--------------------------------- New Message ----------------------------------
sesntive: information

REMOVED DESCRIPTION.

Fixes: X
PR-URL: https://github.com/nodejs-private/node-private/pull/393
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
CVE-ID: CVE-2023-TEST123
--------------------------------------------------------------------------------
? Use this message? Yes

Limitations that should be addressed in subsequent PRs:

  • Support multiple commits
  • Public PRs (backports)

@codecov
Copy link

codecov bot commented Feb 9, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.04 ⚠️

Comparison is base (51a3b24) 83.41% compared to head (baf8494) 83.38%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #665      +/-   ##
==========================================
- Coverage   83.41%   83.38%   -0.04%     
==========================================
  Files          37       37              
  Lines        4131     4158      +27     
==========================================
+ Hits         3446     3467      +21     
- Misses        685      691       +6     
Impacted Files Coverage Δ
lib/collaborators.js 100.00% <100.00%> (ø)
lib/utils.js 65.15% <100.00%> (ø)

... and 4 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@RafaelGSS RafaelGSS force-pushed the support-security-release-proposal branch from 17a048b to f045e77 Compare February 9, 2023 15:15
@RafaelGSS RafaelGSS force-pushed the support-security-release-proposal branch 2 times, most recently from 7276a4e to ca602be Compare May 20, 2023 20:07
@RafaelGSS RafaelGSS marked this pull request as ready for review May 20, 2023 20:40
@RafaelGSS
Copy link
Member Author

RafaelGSS commented May 20, 2023

@nodejs/releasers just updated this PR.

Now it iterates over open PRs in the private repo and adds them to the proposal branch -- with the metadata. If the PR doesn't contain CVE-ID metadata, it will ask to include it in the middle of the process. Could you please review it?

Copy link
Member

@ruyadorno ruyadorno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! A few details you might want to clean up before shipping (although I might just need clarification in a few places) 👍

Great work! Security releases are going to be much simpler 😊 🙏

lib/cherry_pick.js Outdated Show resolved Hide resolved
lib/cherry_pick.js Outdated Show resolved Hide resolved
lib/cherry_pick.js Outdated Show resolved Hide resolved
lib/cherry_pick.js Show resolved Hide resolved
lib/cherry_pick.js Outdated Show resolved Hide resolved
lib/prepare_release.js Outdated Show resolved Hide resolved
lib/prepare_release.js Outdated Show resolved Hide resolved
lib/prepare_release.js Outdated Show resolved Hide resolved
@RafaelGSS RafaelGSS force-pushed the support-security-release-proposal branch from ad33378 to baf8494 Compare July 13, 2023 17:05
@RafaelGSS RafaelGSS requested a review from ruyadorno July 13, 2023 17:05
@RafaelGSS RafaelGSS merged commit bdc9a6b into nodejs:main Jul 13, 2023
12 of 13 checks passed
@ruyadorno
Copy link
Member

🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants