ENA Webin - Move credentials to requirements #7605
Conversation
|
|
||
| ## Invoke Webin-CLI with computed flags. | ||
| ena-webin-cli -context genome -manifest "\$manifest" -userName "\$webin_id" -password "\$password" -centerName "\$center_name" -inputDir "./fasta" $test_flag $action_flag -outputDir $outputs_dir >> '$webin_cli_log' 2>&1 || true; | ||
| ena-webin-cli -context genome -manifest "\$manifest" -userName "$webin_id" -password "$password" -centerName "\$center_name" -inputDir "./fasta" $test_flag $action_flag -outputDir $outputs_dir >> '$webin_cli_log' 2>&1 || true; |
There was a problem hiding this comment.
If we use "$webin_id" and "$password" on the command line the values are visible to other users. Since this seems to be the only way to provide username and password to the program my suggestion would be to ask upstream to allow input via a file (e.g. json or whatever they prefer) or environment variables.
Also this might be a possible attack vector (since username and password are not sanitized) in particular if double quotes are used.
However, for now the PR provides an improvement and we may proceed (a bump would be required).
Wondering if || true is the right thing to do here? Intuitively I would remove it...
There was a problem hiding this comment.
Passing the env variables directly into the ena-webin-cli command would also solve the problem for now, right? Then the values will not be visible
There was a problem hiding this comment.
Yes, but does ena-webin-cli support this already?
There was a problem hiding this comment.
-userName, -username=USER
Webin submission account name or e-mail address.
-password=PASSWORD Webin submission account password.
-passwordFile=FILE File containing the Webin submission account
password.
-passwordEnv=VAR Environment variable containing the Webin submission
account password.
For the password yes.
There was a problem hiding this comment.
Then lets take this for now and ask for the same for user. We can then add this as soon as it is possible.
| echo "Submitting manifest \$manifest" >> "$webin_cli_log" 2>&1; | ||
|
|
||
| ## Invoke Webin-CLI with computed flags. | ||
| ena-webin-cli -context genome -manifest "\$manifest" -userName "\$webin_id" -password "\$password" -centerName "\$center_name" -inputDir "./fasta" $test_flag $action_flag -outputDir $outputs_dir >> '$webin_cli_log' 2>&1 || true; |
There was a problem hiding this comment.
The\ in "$webin_id" and "$password" was done to make it work for both Conda and Singularity installations. #7340 it was quite a debugging to get it working.
There was a problem hiding this comment.
Thanks for mentioning this, I will have a look!
There was a problem hiding this comment.
The \ just makes $webin_id" a bash variable. Otherwise the cheetah interpreter would try to replace it with the value of a Galaxy parameter (or output) named webin_id`.
Independent of this, the problem with bash variables in commands is the the content of the variables will be exposed in the process list (to all users of the system where the tool is running),
|
Great work and thanks a lot for the pro active approach! I think this was very much needed from a security standpoint. |
|
Yes profile 25.1 will prevent intsallation. |
3 participants
FOR CONTRIBUTOR: