Skip to content

feat: add confirmation steps to any write operation#1092

Open
RafaelGSS wants to merge 1 commit into
mainfrom
add-confirmation-to-git-node-security
Open

feat: add confirmation steps to any write operation#1092
RafaelGSS wants to merge 1 commit into
mainfrom
add-confirmation-to-git-node-security

Conversation

@RafaelGSS

Copy link
Copy Markdown
Member

One concern that @aduh95 shared with me is that git node security * performs a lot of things in the background without asking for permissions. This PR aims to add a security confirmation before any write operation.

@RafaelGSS RafaelGSS requested a review from aduh95 June 10, 2026 15:37
const allowed = await cli.prompt(message, { defaultAnswer: false });
if (!allowed) {
cli.info(`Aborted: ${action}.`);
process.exit(0);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let's not call process.exit, that's an anti-pattern (refs #1089)

Comment on lines +38 to +42
const lines = [`Allow action: ${action}?`];
if (detail) {
lines.push('', detail);
}
const message = lines.join('\n');

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit

Suggested change
const lines = [`Allow action: ${action}?`];
if (detail) {
lines.push('', detail);
}
const message = lines.join('\n');
let message = `Allow action: ${action}?`;
if (detail) {
message += `\n\n${detail}`;
}

Comment on lines +107 to +111
await runSecurityGitCommand(
cli,
['add', filePath],
`This stages ${filePath} for the security release commit.`
);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think it makes sense to prompt for this, as this is a local-only operation, and non destructive.

Comment thread lib/security-release/security-release.js
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.

2 participants