add scripts from cli #680
add scripts from cli #680
Conversation
|
Added security warning message |
| |___/ | ||
| The --scripts flag is intended for developers to include google analytics etc. You could be opening yourself to a | ||
| security risk by including the --scripts flag. Make sure you trust the scripts that you are including. | ||
| """) |
There was a problem hiding this comment.
I would further make people type in a confirmation that they understand this message before spawning the browser.
There was a problem hiding this comment.
We have users who want to deploy in a docker-deployed environment, ie, scripted startup where they have control over the deployment script. I like Tony's suggestion, but we need to ensure that the confirmation can be scripting (eg, pipe yes to it, or whatever). Alternative to "|yes": provide a --I-attest-that-really-mean-to-inject-scripts CLI param that still prints the warning, but dispenses with the interactive prompt. @ttung - thoughts?
There was a problem hiding this comment.
I should be able to add this such that | yes works.
There was a problem hiding this comment.
Either way is fine. The browser being fired up is sufficient, I believe, for bad stuff to run, so I do think the confirmation is important, at least for the first time.
There was a problem hiding this comment.
Added confirm.
yes | cellxgene launch --scripts foo --scripts bar example-dataset/pbmc3k.h5ad bypasses the confirm as expected.
|
From Sara on the security team:
Looks ok, as long as we keep an eye on it. |
Fixes #678
Able to add scripts to the html from launch CLI with --scripts flag. This means it will be accessible from the CLI and from docker since docker runs the
cellxgeneCLI.I reformatted the html file in the process. I could make that a separate PR, but I think it looks nicer now 😁