Skip to content

Commit bf990ac

Browse files
authored
feat(git-node): add git node security --apply-patches (#1050)
1 parent 254ebaa commit bf990ac

6 files changed

Lines changed: 244 additions & 8 deletions

File tree

components/git/security.js

Lines changed: 210 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,35 @@
1+
import auth from '../../lib/auth.js';
2+
import Request from '../../lib/request.js';
3+
import LandingSession from '../../lib/landing_session.js';
4+
import Session from '../../lib/session.js';
15
import CLI from '../../lib/cli.js';
6+
import { getMetadata } from '../metadata.js';
7+
import { checkCwd } from '../../lib/update-v8/common.js';
8+
import { parsePRFromURL } from '../../lib/links.js';
29
import PrepareSecurityRelease from '../../lib/prepare_security.js';
310
import UpdateSecurityRelease from '../../lib/update_security_release.js';
411
import SecurityBlog from '../../lib/security_blog.js';
512
import SecurityAnnouncement from '../../lib/security-announcement.js';
13+
import { forceRunAsync } from '../../lib/run.js';
614

715
export const command = 'security [options]';
816
export const describe = 'Manage an in-progress security release or start a new one.';
917

18+
const SECURITY_REPO = {
19+
owner: 'nodejs-private',
20+
repo: 'node-private',
21+
};
22+
1023
const securityOptions = {
1124
start: {
1225
describe: 'Start security release process',
1326
type: 'boolean'
1427
},
28+
'apply-patches': {
29+
describe: 'Start an interactive session to make local HEAD ready to create ' +
30+
'a security release proposal',
31+
type: 'boolean'
32+
},
1533
sync: {
1634
describe: 'Synchronize an ongoing security release with HackerOne',
1735
type: 'boolean'
@@ -59,6 +77,10 @@ export function builder(yargs) {
5977
'git node security --start',
6078
'Prepare a security release of Node.js'
6179
)
80+
.example(
81+
'git node security --apply-patches',
82+
'Fetch all the patches for an upcoming security release'
83+
)
6284
.example(
6385
'git node security --sync',
6486
'Synchronize an ongoing security release with HackerOne'
@@ -98,6 +120,9 @@ export function handler(argv) {
98120
if (argv.start) {
99121
return startSecurityRelease(cli, argv);
100122
}
123+
if (argv['apply-patches']) {
124+
return applySecurityPatches(cli, argv);
125+
}
101126
if (argv.sync) {
102127
return syncSecurityRelease(cli, argv);
103128
}
@@ -168,6 +193,191 @@ async function startSecurityRelease(cli) {
168193
return release.start();
169194
}
170195

196+
async function fetchVulnerabilitiesDotJSON(cli, req) {
197+
const { owner } = SECURITY_REPO;
198+
const repo = 'security-release';
199+
200+
cli.startSpinner(`Looking for Security Release PR on ${owner}/${repo}`);
201+
const { repository: { pullRequests: { nodes: { length, 0: pr } } } } =
202+
await req.gql('ListSecurityReleasePRs', { owner, repo });
203+
if (length !== 1) {
204+
cli.stopSpinner('Expected exactly one open Pull Request on the ' +
205+
`${owner}/${repo} repository, found ${length}`,
206+
cli.SPINNER_STATUS.FAILED);
207+
cli.setExitCode(1);
208+
return;
209+
}
210+
if (pr.files.nodes.length !== 1 || !pr.files.nodes[0].path.endsWith('vulnerabilities.json')) {
211+
cli.stopSpinner(
212+
`${owner}/${repo}#${pr.number} does not contain only vulnerabilities.json`,
213+
cli.SPINNER_STATUS.FAILED
214+
);
215+
cli.setExitCode(1);
216+
return;
217+
}
218+
cli.stopSpinner(`Found ${owner}/${repo}#${pr.number} by @${pr.author.login}`);
219+
cli.startSpinner('Fetching vulnerabilities.json...');
220+
const result = await req.json(
221+
`/repos/${owner}/${repo}/contents/${pr.files.nodes[0].path}?ref=${pr.headRefOid}`,
222+
{ headers: { Accept: 'application/vnd.github.raw+json' } }
223+
);
224+
cli.stopSpinner('Fetched vulnerabilities.json');
225+
return result;
226+
}
227+
228+
async function skipIfExisting(cli, prURL) {
229+
const existingCommit = await forceRunAsync('git',
230+
['--no-pager', 'log', 'HEAD', '--grep', `^PR-URL: ${prURL}$`, '--format=%h %s'],
231+
{ ignoreFailure: false, captureStdout: true });
232+
if (existingCommit.trim()) {
233+
cli.info(`${prURL} seems to already be on the current tree: ${existingCommit}`);
234+
return await cli.prompt('Do you want to skip it?', { defaultAnswer: true });
235+
}
236+
return false;
237+
}
238+
async function landingSession(cli, req, prURL, argv, cveIds = undefined) {
239+
const response = await cli.prompt('Do you want to land it on the current HEAD?',
240+
{ defaultAnswer: true });
241+
if (!response) {
242+
cli.info('Skipping');
243+
cli.warn('The resulting HEAD will not be ready for a release proposal');
244+
return true;
245+
}
246+
247+
if (!cli.hasDetachedHEAD) {
248+
// Moving to a detached HEAD, we don't want security patches to be pushed to the public repo.
249+
await forceRunAsync('git', ['checkout', '--detach'], { ignoreFailure: false });
250+
}
251+
cli.hasDetachedHEAD = (await forceRunAsync(
252+
'git', ['rev-parse', 'HEAD'],
253+
{ ignoreFailure: false, captureStdout: true })).trim();
254+
255+
const session = new LandingSession(cli, req, process.cwd(), {
256+
autorebase: true, oneCommitMax: false, ...argv
257+
});
258+
Object.defineProperty(session, 'tryResetBranch', {
259+
__proto__: null,
260+
value: Function.prototype,
261+
configurable: true,
262+
});
263+
const metadata = await getMetadata(session.argv, true, cli);
264+
if (argv?.backport) {
265+
metadata.metadata += `PR-URL: ${prURL}\n`;
266+
}
267+
if (cveIds?.length) {
268+
metadata.metadata += cveIds.map(cve => `CVE-ID: ${cve}\n`).join('');
269+
}
270+
await session.start(metadata);
271+
return false;
272+
}
273+
async function applySecurityPatches(cli) {
274+
const { nodeMajorVersion } = await checkCwd({ nodeDir: process.cwd() });
275+
const branch = `v${nodeMajorVersion}.x`;
276+
const credentials = await auth({
277+
github: true
278+
});
279+
const req = new Request(credentials);
280+
281+
cli.info('N.B.: if there are commits on the staging branch that need to be included in the ' +
282+
'security release, please rebase them manually and answer no to the following question');
283+
// Try reset to the public upstream
284+
const session = new Session(cli, process.cwd(), undefined, { branch });
285+
await session.tryResetBranch();
286+
287+
const { owner, repo } = SECURITY_REPO;
288+
const { releaseDate, reports, dependencies } = await fetchVulnerabilitiesDotJSON(cli, req);
289+
290+
let patchedVersion;
291+
for (const { affectedVersions, prURL, title } of Object.values(dependencies).flat()) {
292+
if (!affectedVersions.includes(`${nodeMajorVersion}.x`)) continue;
293+
cli.separator(`Taking care of ${title}...`);
294+
if (await skipIfExisting(cli, prURL)) continue;
295+
296+
const argv = parsePRFromURL(prURL);
297+
if (argv.owner === SECURITY_REPO.owner && argv.repo === SECURITY_REPO.repo) {
298+
await landingSession(cli, req, prURL, argv);
299+
continue;
300+
}
301+
302+
const existingCommits = (await forceRunAsync('git',
303+
['--no-pager', 'log', `${session.upstream}/v${nodeMajorVersion}.x-staging`,
304+
'--grep', `^PR-URL: ${prURL}$`,
305+
'--format=%H %h %s'],
306+
{ ignoreFailure: false, captureStdout: true })).trim().split('\n');
307+
if (existingCommits[0] === '') {
308+
cli.error(`${prURL} was not found on ${session.upstream}/v${nodeMajorVersion}.x-staging.`);
309+
cli.info('Please cherry-pick the adequate commits to the public staging branch.');
310+
cli.info('Skipping');
311+
cli.warn('The resulting HEAD will not be ready for a release proposal');
312+
continue;
313+
}
314+
cli.info(`The following commit(s) have been found on ${session.upstream}/v${nodeMajorVersion
315+
}.x-staging:\n${existingCommits.map(c => ` - ${c.slice(41)}`).join('\n')}`);
316+
if (!await cli.prompt('Cherry-pick those for the current proposal?')) {
317+
cli.info('Skipping');
318+
cli.warn('The resulting HEAD will not be ready for a release proposal');
319+
continue;
320+
}
321+
await forceRunAsync('git',
322+
['cherry-pick', ...existingCommits.map(c => c.slice(0, 40))],
323+
{ ignoreFailure: false });
324+
}
325+
326+
cli.startSpinner(`Fetching open PRs on ${owner}/${repo}...`);
327+
const { repository: { pullRequests: { nodes } } } = await req.gql('PRs', {
328+
owner, repo, labels: [branch],
329+
});
330+
cli.stopSpinner(`Fetched all PRs labeled for v${nodeMajorVersion}.x`);
331+
332+
for (const { affectedVersions, prURL, cveIds, patchedVersions } of reports) {
333+
if (!affectedVersions.includes(`${nodeMajorVersion}.x`)) continue;
334+
patchedVersion ??= patchedVersions?.find(v => v.startsWith(`${nodeMajorVersion}.`));
335+
cli.separator(`Taking care of ${cveIds.join(', ')}...`);
336+
337+
if (await skipIfExisting(cli, prURL)) continue;
338+
339+
let pr = nodes.find(({ url }) => url === prURL);
340+
if (!pr) {
341+
cli.info(
342+
`${prURL} is not labelled for v${nodeMajorVersion}.x, there might be a backport PR.`
343+
);
344+
345+
cli.startSpinner('Fetching PR title to find a match...');
346+
const { title } = await req.getPullRequest(prURL);
347+
pr = nodes.find((pr) => pr.title.endsWith(title));
348+
if (pr) {
349+
cli.stopSpinner(`Found ${pr.url}`);
350+
} else {
351+
cli.stopSpinner(`Did not find a match for "${title}"`, cli.SPINNER_STATUS.WARN);
352+
const prID = await cli.prompt(
353+
'Please enter the PR number to use:',
354+
{ questionType: cli.QUESTION_TYPE.NUMBER, defaultAnswer: NaN }
355+
);
356+
pr = nodes.find(({ number }) => number === prID);
357+
if (!pr) {
358+
cli.error(`${prID} is not in the list of PRs labelled for v${nodeMajorVersion}.x`);
359+
cli.info('The list of labelled PRs and vulnerabilities.json are fetched ' +
360+
'once at the start of the session; to refresh those, start a new NCU session');
361+
const response = await cli.prompt('Do you want to skip that CVE?',
362+
{ defaultAnswer: false });
363+
if (response) continue;
364+
throw new Error(`Found no patch for ${cveIds}`);
365+
}
366+
}
367+
}
368+
cli.ok(`${pr.url} is labelled for v${nodeMajorVersion}.x.`);
369+
370+
const backport = prURL !== pr.url;
371+
372+
await landingSession(cli, req, prURL, { prid: pr.number, backport, ...SECURITY_REPO }, cveIds);
373+
}
374+
cli.ok('All patches are on the local HEAD!');
375+
cli.info('You can now build and test, and create a proposal with the following commands:');
376+
cli.info(`git switch -C v${nodeMajorVersion}.x HEAD`);
377+
cli.info(`git node release --prepare --security --newVersion=${patchedVersion} ` +
378+
`--releaseDate=${releaseDate.replaceAll('/', '-')} --skipBranchDiff`);
379+
}
380+
171381
async function cleanupSecurityRelease(cli) {
172382
const release = new PrepareSecurityRelease(cli);
173383
return release.cleanup();

docs/git-node.md

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -453,7 +453,7 @@ pushing changes, creating issues, or updating HackerOne reports.
453453
It's necessary to set up `.ncurc` with HackerOne keys:
454454

455455
```console
456-
$ ncu-config --global set h1_token $H1_TOKEN
456+
$ ncu-config --global set -x h1_token
457457
$ ncu-config --global set h1_username $H1_TOKEN
458458
```
459459

@@ -467,6 +467,12 @@ This command creates the Next Security Issue in Node.js private repository
467467
following the [Security Release Process][] document.
468468
It will retrieve all the triaged HackerOne reports and add creates the `vulnerabilities.json`.
469469

470+
### `git node security --apply-patches`
471+
472+
This command fetches the list of reports and the list of PRs labelled for the
473+
release corresponding to the CWD, and match them in pair, and run a
474+
`git node land` session for each.
475+
470476
### `git node security --update-date=YYYY/MM/DD`
471477

472478
This command updates the `vulnerabilities.json` with target date of the security release.

lib/landing_session.js

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -116,8 +116,8 @@ export default class LandingSession extends Session {
116116

117117
// We fetched the commit that would result if we used `git merge`.
118118
// ^1 and ^2 refer to the PR base and the PR head, respectively.
119-
const [base, head] = await runAsync('git',
120-
['rev-parse', 'FETCH_HEAD^1', 'FETCH_HEAD^2'],
119+
const [base, head, rebaseHead] = await runAsync('git',
120+
['rev-parse', 'FETCH_HEAD^1', 'FETCH_HEAD^2', 'HEAD'],
121121
{ captureStdout: 'lines' });
122122
const commitShas = await runAsync('git',
123123
['rev-list', `${base}..${head}`],
@@ -136,7 +136,7 @@ export default class LandingSession extends Session {
136136
process.exit(1);
137137
}
138138

139-
const commitInfo = { base, head, shas: commitShas };
139+
const commitInfo = { base, head, shas: commitShas, rebaseHead };
140140
this.saveCommitInfo(commitInfo);
141141

142142
try {
@@ -233,12 +233,11 @@ export default class LandingSession extends Session {
233233
// so that it will perform everything automatically.
234234
cli.log(`There are ${subjects.length} commits in the PR. ` +
235235
'Attempting autorebase.');
236-
const { upstream, branch } = this;
237236
const assumeYes = this.cli.assumeYes ? '--yes' : '';
238237
const msgAmend = `-x "git node land --amend ${assumeYes}"`;
239238
try {
240239
await forceRunAsync('git',
241-
['rebase', ...this.gpgSign, `${upstream}/${branch}`,
240+
['rebase', ...this.gpgSign, commitInfo.rebaseHead,
242241
'--no-keep-empty', '-i', '--autosquash', msgAmend],
243242
{
244243
ignoreFailure: false,
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
query PR($owner: String!, $repo: String!) {
2+
repository(owner: $owner, name: $repo) {
3+
pullRequests(states: OPEN, first: 2, headRefName: "next-security-release", orderBy: {field: CREATED_AT, direction: DESC}) {
4+
nodes {
5+
number
6+
headRefOid
7+
author {
8+
login
9+
}
10+
11+
files(first: 2) {
12+
nodes {
13+
path
14+
}
15+
}
16+
}
17+
}
18+
}
19+
}

lib/session.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -345,8 +345,9 @@ export default class Session {
345345
}
346346

347347
getStrayCommits(verbose) {
348-
const { upstream, branch } = this;
349-
const ref = `${upstream}/${branch}...HEAD`;
348+
const { upstream, branch, cli } = this;
349+
const base = cli.hasDetachedHEAD || `${upstream}/${branch}`;
350+
const ref = `${base}...HEAD`;
350351
const gitCmd = verbose
351352
? ['log', '--oneline', '--reverse', ref]
352353
: ['rev-list', '--reverse', ref];

lib/update-v8/common.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,4 +32,5 @@ export async function checkCwd(ctx) {
3232
`node-dir: ${ctx.nodeDir}`
3333
);
3434
}
35+
return ctx;
3536
};

0 commit comments

Comments
 (0)