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

exp push: non-tty process prompted for git credentials (https) #9339

Closed
mattseddon opened this issue Apr 18, 2023 · 4 comments
Closed

exp push: non-tty process prompted for git credentials (https) #9339

mattseddon opened this issue Apr 18, 2023 · 4 comments
Assignees

Comments

@mattseddon
Copy link
Member

Bug Report

Description

Under certain conditions, an experiment being pushed to the remote using the VS Code extension will result in a non-tty process being prompted for git credentials (e.g Username for 'https://github.com':). This makes it appear to the user that the process has hung.

One example is as follows:

  1. Remote has not been accessed with DVC previously (i.e dvc exp push origin [exp-name] has not been run against the CLI in a terminal).
  2. Remote is set up to use https. E.g https://github.com:iterative/vscode-dvc-demo.git.

Reproduce

  1. Clone vscode-dvc-demo using https (git clone https://github.com/iterative/vscode-dvc-demo.git)
  2. dvc pull
  3. dvc exp run
  4. dvc exp push origin [new-exp] <= using the extension, i.e non-tty
  5. Process will be asked for username

Expected

The process fails will an appropriate error message.

Environment information

Output of dvc doctor:

dvc doctor
DVC version: 2.54.0 (pip)
-------------------------
Platform: Python 3.10.6 on macOS-13.3.1-arm64-arm-64bit
Subprojects:
        dvc_data = 0.47.1
        dvc_objects = 0.21.1
        dvc_render = 0.3.1
        dvc_task = 0.2.0
        scmrepo = 1.0.1
Supports:
        http (aiohttp = 3.8.4, aiohttp-retry = 2.8.3),
        https (aiohttp = 3.8.4, aiohttp-retry = 2.8.3),
        s3 (s3fs = 2023.3.0, boto3 = 1.24.59)
Cache types: reflink, hardlink, symlink
Cache directory: apfs on /dev/disk3s1s1
Caches: local
Remotes: s3
Workspace directory: apfs on /dev/disk3s1s1
Repo: dvc, git
Repo.site_cache_dir: /Library/Caches/dvc/repo/9b2d1355ef26cb2a38a704c54d1f3710

Additional Information (if any):

It would be good to expose an API for DVC's Git credentials helper so that we could trigger it from the extension.

@pmrowla
Copy link
Contributor

pmrowla commented Apr 18, 2023

vscode normally sets the GIT_ASKPASS env var, which is what git (and DVC) use to get interactive input if the env var is set (instead of prompting on a terminal).

There is a bug here if the process is hanging, scmrepo shouldn't be prompting if it is not in a tty, but also the extension should be setting GIT_ASKPASS to match what you get in the vscode terminal when you run DVC (so the prompt is just done through the vscode UI even in non-TTY processes)

$ env|grep GIT
GIT_ASKPASS=/Applications/Visual Studio Code.app/Contents/Resources/app/extensions/git/dist/askpass.sh
VSCODE_GIT_ASKPASS_NODE=/Applications/Visual Studio Code.app/Contents/Frameworks/Code Helper (Plugin).app/Contents/MacOS/Code Helper (Plugin)
VSCODE_GIT_ASKPASS_EXTRA_ARGS=--ms-enable-electron-run-as-node
VSCODE_GIT_ASKPASS_MAIN=/Applications/Visual Studio Code.app/Contents/Resources/app/extensions/git/dist/askpass-main.js
VSCODE_GIT_IPC_HANDLE=/var/folders/5q/_pk6g_s12dj2c4kp4mf_fnc40000gn/T/vscode-git-94f397ffed.sock

(DVC only cares about GIT_ASKPASS, not sure whether or not the actual vscode helper also needs the other env vars to be set?)

@pmrowla
Copy link
Contributor

pmrowla commented Apr 18, 2023

Also for reference, you can explicitly disable command line prompting in git (and dvc/scmrepo) by setting GIT_TERMINAL_PROMPT=0 in your env.

This only affects the CLI prompt, and does not affect use of 3rd party credential helpers that may or may not be interactive. It also does not affect the use of GIT_ASKPASS (if askpass is set, the helper will be called regardless of GIT_TERMINAL_PROMPT)

@mattseddon
Copy link
Member Author

vscode normally sets the GIT_ASKPASS env var, which is what git (and DVC) use to get interactive input if the env var is set (instead of prompting on a terminal).

There is a bug here if the process is hanging, scmrepo shouldn't be prompting if it is not in a tty, but also the extension should be setting GIT_ASKPASS to match what you get in the vscode terminal when you run DVC (so the prompt is just done through the vscode UI even in non-TTY processes)

$ env|grep GIT
GIT_ASKPASS=/Applications/Visual Studio Code.app/Contents/Resources/app/extensions/git/dist/askpass.sh
VSCODE_GIT_ASKPASS_NODE=/Applications/Visual Studio Code.app/Contents/Frameworks/Code Helper (Plugin).app/Contents/MacOS/Code Helper (Plugin)
VSCODE_GIT_ASKPASS_EXTRA_ARGS=--ms-enable-electron-run-as-node
VSCODE_GIT_ASKPASS_MAIN=/Applications/Visual Studio Code.app/Contents/Resources/app/extensions/git/dist/askpass-main.js
VSCODE_GIT_IPC_HANDLE=/var/folders/5q/_pk6g_s12dj2c4kp4mf_fnc40000gn/T/vscode-git-94f397ffed.sock

(DVC only cares about GIT_ASKPASS, not sure whether or not the actual vscode helper also needs the other env vars to be set?)

The script this points to is:

#!/bin/sh
VSCODE_GIT_ASKPASS_PIPE=`mktemp`
ELECTRON_RUN_AS_NODE="1" VSCODE_GIT_ASKPASS_PIPE="$VSCODE_GIT_ASKPASS_PIPE" VSCODE_GIT_ASKPASS_TYPE="https" "$VSCODE_GIT_ASKPASS_NODE" "$VSCODE_GIT_ASKPASS_MAIN" $VSCODE_GIT_ASKPASS_EXTRA_ARGS $*
cat $VSCODE_GIT_ASKPASS_PIPE
rm $VSCODE_GIT_ASKPASS_PIPE

Most of these variables could be generated but VSCODE_GIT_IPC_HANDLE definitely can't be and it is required by the VSCODE_GIT_ASKPASS_MAIN script. VS Code seems to be purposefully hiding them from extensions (probably for the reason that they could be used maliciously). The "expected" way for extensions to handle this is to ask the user for permission to access their GH account (the only Git Forge that has built-in support). The resulting session details can then be passed to processes that are being created.

Once iterative/scmrepo#234 gets fixed I think I'll start off by providing an error to the user and letting them know that if they push an experiment using the CLI that the issue will be fixed.

@pmrowla
Copy link
Contributor

pmrowla commented Apr 18, 2023

This is fixed in scmrepo==1.0.2, you may still want to explicitly set GIT_TERMINAL_PROMPT=0 in vscode either way for cases where the user does not have the latest scmrepo ver installed

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

No branches or pull requests

2 participants