feat: add confirmation steps to any write operation #1092
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,7 +30,36 @@ export const PLACEHOLDERS = { | |
| downloads: '%DOWNLOADS%' | ||
| }; | ||
|
|
||
| function formatCommand(command, args) { | ||
| return [command, ...args].join(' '); | ||
| } | ||
|
|
||
| export async function confirmSecurityStep(cli, action, detail) { | ||
| const lines = [`Allow action: ${action}?`]; | ||
| if (detail) { | ||
| lines.push('', detail); | ||
| } | ||
| const message = lines.join('\n'); | ||
|
|
||
| const allowed = await cli.prompt(message, { defaultAnswer: false }); | ||
| if (!allowed) { | ||
| cli.info(`Aborted: ${action}.`); | ||
| process.exit(0); | ||
|
Contributor
There was a problem hiding this comment. Let's not call |
||
| } | ||
| } | ||
|
|
||
| export async function runSecurityGitCommand(cli, args, detail) { | ||
| const command = formatCommand('git', args); | ||
| await confirmSecurityStep(cli, `run \`${command}\``, detail); | ||
| return runSync('git', args); | ||
| } | ||
|
|
||
| export async function writeSecurityFile(cli, filePath, content, detail) { | ||
| await confirmSecurityStep(cli, `write \`${filePath}\``, detail); | ||
| return fs.writeFileSync(filePath, content); | ||
| } | ||
|
|
||
| export async function checkRemote(cli, repository) { | ||
| const remote = runSync('git', ['ls-remote', '--get-url', 'origin']).trim(); | ||
| const { owner, repo } = repository; | ||
| const securityReleaseOrigin = [ | ||
|
|
@@ -44,26 +73,42 @@ export function checkRemote(cli, repository) { | |
| } | ||
| } | ||
|
|
||
| export async function checkoutOnSecurityReleaseBranch(cli, repository) { | ||
| await checkRemote(cli, repository); | ||
| const currentBranch = runSync('git', ['branch', '--show-current']).trim(); | ||
| cli.info(`Current branch: ${currentBranch} `); | ||
|
|
||
| if (currentBranch !== 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.` | ||
| ); | ||
| cli.ok(`Checkout on branch: ${NEXT_SECURITY_RELEASE_BRANCH} `); | ||
| }; | ||
| } | ||
|
|
||
| export async function commitAndPushVulnerabilitiesJSON( | ||
| filePath, | ||
| commitMessage, | ||
| { cli, repository } | ||
| ) { | ||
| await checkRemote(cli, repository); | ||
|
|
||
| if (Array.isArray(filePath)) { | ||
| for (const currentPath of filePath) { | ||
| await runSecurityGitCommand( | ||
| cli, | ||
| ['add', currentPath], | ||
| `This stages ${currentPath} for the security release commit.` | ||
| ); | ||
| } | ||
| } else { | ||
| await runSecurityGitCommand( | ||
| cli, | ||
| ['add', filePath], | ||
| `This stages ${filePath} for the security release commit.` | ||
| ); | ||
|
Comment on lines
+107
to
+111
Contributor
There was a problem hiding this comment. I don't think it makes sense to prompt for this, as this is a local-only operation, and non destructive. |
||
| } | ||
|
|
||
| const staged = runSync('git', ['diff', '--name-only', '--cached']).trim(); | ||
|
|
@@ -72,15 +117,31 @@ export function commitAndPushVulnerabilitiesJSON(filePath, commitMessage, { cli, | |
| return; | ||
| } | ||
|
|
||
| await runSecurityGitCommand( | ||
| cli, | ||
| ['commit', '-m', commitMessage], | ||
| `This creates a local commit with message: ${commitMessage}` | ||
| ); | ||
|
aduh95 marked this conversation as resolved.
|
||
|
|
||
| try { | ||
| await runSecurityGitCommand( | ||
| cli, | ||
| ['push', '-u', 'origin', NEXT_SECURITY_RELEASE_BRANCH], | ||
| `This pushes the security release branch to origin/${NEXT_SECURITY_RELEASE_BRANCH}.` | ||
| ); | ||
| } catch (error) { | ||
| cli.warn('Rebasing...'); | ||
| // try to pull rebase and push again | ||
| await runSecurityGitCommand( | ||
| cli, | ||
| ['pull', 'origin', NEXT_SECURITY_RELEASE_BRANCH, '--rebase'], | ||
| `This rebases local changes on origin/${NEXT_SECURITY_RELEASE_BRANCH}.` | ||
| ); | ||
| await runSecurityGitCommand( | ||
| cli, | ||
| ['push', '-u', 'origin', NEXT_SECURITY_RELEASE_BRANCH], | ||
| `This retries pushing the security release branch to origin/${NEXT_SECURITY_RELEASE_BRANCH}.` | ||
| ); | ||
| } | ||
| cli.ok(`Pushed commit: ${commitMessage} to ${NEXT_SECURITY_RELEASE_BRANCH}`); | ||
| } | ||
|
|
@@ -150,6 +211,11 @@ export function promptDependencies(cli) { | |
| } | ||
|
|
||
| export async function createIssue(title, content, repository, { cli, req }) { | ||
| await confirmSecurityStep( | ||
| cli, | ||
| `create GitHub issue \`${repository.owner}/${repository.repo}: ${title}\``, | ||
| `This creates an issue in ${repository.owner}/${repository.repo}.` | ||
| ); | ||
| const data = await req.createIssue(title, content, repository); | ||
| if (data.html_url) { | ||
| cli.ok(`Created: ${data.html_url}`); | ||
|
|
@@ -252,20 +318,30 @@ export class SecurityRelease { | |
| NEXT_SECURITY_RELEASE_FOLDER, 'vulnerabilities.json'); | ||
| } | ||
|
|
||
| async updateReleaseFolder(releaseDate) { | ||
| const folder = path.join(process.cwd(), | ||
| NEXT_SECURITY_RELEASE_FOLDER); | ||
| const newFolder = path.join(process.cwd(), 'security-release', releaseDate); | ||
| await confirmSecurityStep( | ||
| this.cli, | ||
| `rename \`${folder}\` to \`${newFolder}\``, | ||
| 'This moves the next-security-release folder to the dated release folder.' | ||
| ); | ||
| fs.renameSync(folder, newFolder); | ||
| return newFolder; | ||
| } | ||
|
|
||
| async updateVulnerabilitiesJSON(content) { | ||
| try { | ||
| const vulnerabilitiesJSONPath = this.getVulnerabilitiesJSONPath(); | ||
| this.cli.startSpinner(`Updating vulnerabilities.json from ${vulnerabilitiesJSONPath}...`); | ||
| await writeSecurityFile( | ||
| this.cli, | ||
| vulnerabilitiesJSONPath, | ||
| JSON.stringify(content, null, 2), | ||
| 'This updates vulnerabilities.json with the latest security release data.' | ||
| ); | ||
| await commitAndPushVulnerabilitiesJSON(vulnerabilitiesJSONPath, | ||
| 'chore: updated vulnerabilities.json', | ||
| { cli: this.cli, repository: this.repository }); | ||
| this.cli.stopSpinner(`Done updating vulnerabilities.json from ${vulnerabilitiesJSONPath}`); | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit