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

git clone subprocess NO LONGER FAILS for SSH when key requires passphrase #1984

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

nicheeralal
Copy link

@nicheeralal nicheeralal commented Nov 27, 2023

This pull request addresses the issue described in #1653.

My solution here is to replace the subprocess.check_output() call with subprocess.run(), which allows the SSH passphrase prompt to be passed through to the user.

The subprocess.run() function is more flexible and can interact with the user when necessary, such as prompting for a passphrase.

In this revised function, subprocess.run is used with the check=False argument to avoid immediate failure. The return code and output are then manually checked and handled. This should allow the passphrase prompt to appear and be entered by the user.

I have tested this locally and passed all unit tests. Note, this change required me to modify some of the test cases. This is my process, which was done in a virtual environment:

  1. Made sure I was in the directory where you have cloned your fork of Cookiecutter with the modifications.
  2. Installed in Editable Mode: Use the following pip command to install your local Cookiecutter directory in editable mode:
    pip install -e .
  3. After running the installation command, I verified that my local version of Cookiecutter is installed by checking the installed packages:
    pip list
  4. Tested my change in the Cookiecutter code, with Git configured with an SSH key that has a passphrase.
    cookiecutter git+ssh://git@github.com/audreyr/cookiecutter-pypackage.git
  5. This command now utilized my modified Cookiecutter code. It prompts for the SSH passphrase as expected.

Thus the issue/s are addressed.

@ericof ericof self-requested a review November 27, 2023 17:45
@ericof ericof added this to the 3.0.0 milestone Nov 27, 2023
@ericof ericof added the bug This issue/PR relates to a bug. label Nov 27, 2023
…ns in the vcs.py module. The use of subprocess.run with shell=False is flagged by Bandit as a potential security issue. However, in this context, the inputs to these subprocess calls are strictly controlled and validated (through is_valid_url and is_valid_checkout). This significantly lowers the risk of malicious command injection, which is the primary concern with subprocess calls.
@nicheeralal nicheeralal changed the title git clone subprocess NO LONGER FAILS for SSH when key requires passphrase Modified test cases for test_clone.py Nov 28, 2023
@nicheeralal nicheeralal changed the title Modified test cases for test_clone.py git clone subprocess NO LONGER FAILS for SSH when key requires passphrase Nov 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue/PR relates to a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants