Skip to content

Conversation

@RAJVEER42
Copy link
Contributor

This PR adds a second-layer safeguard for dangerous shell commands prefixed with CONFIRM:.

✅ What’s Changed

  • After confirming with "yes", users are required to re-type the command to proceed.
  • Execution is aborted if the typed command does not match exactly.
  • Non-destructive or direct commands are unaffected.

📋 Why This Is Important

This extra check helps prevent accidental deletions or irreversible changes due to misclicks or habit fatigue.

Please review and feel free to suggest improvements!

@RAJVEER42
Copy link
Contributor Author

Hi @Kirti-Rathi 👋,

Thank you so much for your guidance and the detailed review on the previous implementation — it really helped clarify the intended changes and improved the solution.

I've now pushed the updated code implementing the second-layer safeguard for CONFIRM: commands. It ensures that users must both confirm with "yes" and re-type the exact command before any dangerous action is executed. I've also removed the redundant code, kept the original logic intact as requested, and ensured everything remains backward-compatible.

Please feel free to review the PR at your convenience. I'd love your feedback on whether everything looks good now or if there’s anything else you’d like adjusted. 😊

Thanks again!

Best regards,
@RAJVEER42

Copy link
Owner

@Kirti-Rathi Kirti-Rathi left a comment

Choose a reason for hiding this comment

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

Pls replace all command[8:] with command[9:] b/c the prefixed white space is leading to false failed command verification message.
Everything else works fine except for this very minor issue.

@RAJVEER42
Copy link
Contributor Author

@hi @Kirti-Rathi ,

I've made the required change in the safeguard-exec-confirmation branch — specifically replaced all instances of command[8:] with command[9:] for better accuracy in command parsing.

Please review the commit and let me know if anything else needs to be updated.

Copy link
Owner

@Kirti-Rathi Kirti-Rathi left a comment

Choose a reason for hiding this comment

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

Good work!

@Kirti-Rathi Kirti-Rathi merged commit ae9c11f into Kirti-Rathi:main Jun 22, 2025
@RAJVEER42
Copy link
Contributor Author

Hey @Kirti-Rathi 👋,

Just wanted to say a big thank you for all your help and patience throughout my contribution . I really appreciate you guiding me, correcting my mistakes, and giving me the chance to learn and grow 💪.

Also, thanks a ton for merging my pull request — it means a lot! ✨

It’s been awesome working with you, and I truly hope we get to collaborate again in the future! 🚀

Cheers,

@RAJVEER42 RAJVEER42 deleted the safeguard-exec-confirmation branch June 22, 2025 10:01
@Kirti-Rathi Kirti-Rathi added Intermediate Requires some familiarity with the codebase, and may involve logic changes. SSoC25 Part of Social Summer of Code 2025. Tag for recognition. labels Jul 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Intermediate Requires some familiarity with the codebase, and may involve logic changes. SSoC25 Part of Social Summer of Code 2025. Tag for recognition.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants