You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Snakemake version
Snakemake 7.3.8
Wrappers 1.3.2 (master branch)
Note the Snakemake and snakemake-wrappers version for which you experience the bug.
Always make sure to try out the latest version of both before creating a new bug report.
Describe the bug
Hello,
We are wondering if the developers have considered publishing security recommendations or caveats for the wrapper usage where the wrapper is automatically retrieved from github? It is generally considered a poor practice to directly run code retrieved from a URL, particularly in an automated or programmatic fashion where the contents can change.
This feedback is less about concerns for Snakemake's code integrity (but security compromises are always possible); and more about the possible influence over poor user decisions by indicating that this kind of "always pull in remote code dependencies" pattern is acceptable and secure.
For example, giving a user a command to "curl -s [some url] | bash" as part of a routine operation, versus retrieving the file and running a known good copy. Even if the user hasn't reviewed the script's contents - this at least gives consistency and and ensurance that the contents will not change -- possibly maliciously.
Have the developers considered changing the default recommendation/default strategy to be cloning the repository?
Have the developers considered publishing recommendations that productionized code/environments should use a local copy of the wrapper?
Cloning the repository or specific wrapper directory has other benefits for performance and reliability, especially if github is having issues.
Even if the user is specifying the version tag to be retrieved, this is subject to abuse or error to point to broken or malicious code
Since the version tag is specified in the wrapper path for remote retrieval, this seems to be solely a usability move -- instead of the benefit of automatically retrieving the latest updates; in which case a local cached copy is the ultimate in stability.
We understand that merges to master and changes to tags or other metadata come with the usual levels of review, however this does not alleviate the risk to users of problems upstream from infrastructure, operational, or security (credential compromise, or other) influence/risks/unforeseen circumstances
If the above questions and statements seem reasonable and we are not overlooking some published docs/recommendations or other misunderstanding, our suggestions pertain mostly to documentation changes:
Update documentation to recommend cloning the repository/directory of the necessary wrappers as a default strategy, but we understand if this is seen as a significant detriment to snakemake-wrappers' usability for new users
At least publish caveats of relying on remote retrieval, that there is not a support/availability guarantee and you [the user] understand you are relying on a 3rd party resource when otherwise your data and resources are local
There should be a strong recommendation that productionized code/pipelines do not rely on hosted wrappers
The documentation be transparent about why the developers consider remote retrieval safe for development/non-production use, such as reference to the contribution and code review guidelines in use. This helps differentiate Snakemake's practices from pulling python, bash, or other code dynamically from non-reviewed/non-controlled sources (like some smaller project's manually maintained website on a Wordpress instance that could more easily be compromised without review/alert)
A non-documentation change could be to have the code cache the wrapper on the client if it is pulled from a specific version tag or commit hash. This would provide many of the benefits while reducing dependency on the remote hosting. Additionally, since a version tag or commit hash means the file contents are not expected to change, there is no need for the cache to have aging logic or other long-term maintenance built in -- it can get retrieved once and used forever.
Thank you for your consideration. If you are open to changes in at least the documentation, I am happy to contribute a PR that reflects a reasonable outcome. Logs
If applicable, any terminal output to help explain your problem.
Minimal example
Add a minimal example for reproducing the bug.
Additional context
Add any other context about the problem here.
The text was updated successfully, but these errors were encountered:
Snakemake version
Snakemake 7.3.8
Wrappers 1.3.2 (master branch)
Note the Snakemake and snakemake-wrappers version for which you experience the bug.
Always make sure to try out the latest version of both before creating a new bug report.
Describe the bug
Hello,
We are wondering if the developers have considered publishing security recommendations or caveats for the wrapper usage where the wrapper is automatically retrieved from github? It is generally considered a poor practice to directly run code retrieved from a URL, particularly in an automated or programmatic fashion where the contents can change.
This feedback is less about concerns for Snakemake's code integrity (but security compromises are always possible); and more about the possible influence over poor user decisions by indicating that this kind of "always pull in remote code dependencies" pattern is acceptable and secure.
For example, giving a user a command to "curl -s [some url] | bash" as part of a routine operation, versus retrieving the file and running a known good copy. Even if the user hasn't reviewed the script's contents - this at least gives consistency and and ensurance that the contents will not change -- possibly maliciously.
In particular, upon reviewing the documentation at https://snakemake-wrappers.readthedocs.io/en/latest/ , I have the following observations and/or questions:
We understand that merges to master and changes to tags or other metadata come with the usual levels of review, however this does not alleviate the risk to users of problems upstream from infrastructure, operational, or security (credential compromise, or other) influence/risks/unforeseen circumstances
If the above questions and statements seem reasonable and we are not overlooking some published docs/recommendations or other misunderstanding, our suggestions pertain mostly to documentation changes:
Update documentation to recommend cloning the repository/directory of the necessary wrappers as a default strategy, but we understand if this is seen as a significant detriment to snakemake-wrappers' usability for new users
At least publish caveats of relying on remote retrieval, that there is not a support/availability guarantee and you [the user] understand you are relying on a 3rd party resource when otherwise your data and resources are local
There should be a strong recommendation that productionized code/pipelines do not rely on hosted wrappers
The documentation be transparent about why the developers consider remote retrieval safe for development/non-production use, such as reference to the contribution and code review guidelines in use. This helps differentiate Snakemake's practices from pulling python, bash, or other code dynamically from non-reviewed/non-controlled sources (like some smaller project's manually maintained website on a Wordpress instance that could more easily be compromised without review/alert)
A non-documentation change could be to have the code cache the wrapper on the client if it is pulled from a specific version tag or commit hash. This would provide many of the benefits while reducing dependency on the remote hosting. Additionally, since a version tag or commit hash means the file contents are not expected to change, there is no need for the cache to have aging logic or other long-term maintenance built in -- it can get retrieved once and used forever.
Thank you for your consideration. If you are open to changes in at least the documentation, I am happy to contribute a PR that reflects a reasonable outcome.
Logs
If applicable, any terminal output to help explain your problem.
Minimal example
Add a minimal example for reproducing the bug.
Additional context
Add any other context about the problem here.
The text was updated successfully, but these errors were encountered: