-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Add --sandbox
flag to prevent dynamic loading of other files/data.
#3092
base: master
Are you sure you want to change the base?
Conversation
Some use cases for `jq` may involve accepting untrusted input. See discussion in jqlang#1361 for some security considerations that may be relevant for those use cases. This commit adds a `--sandbox` flag which is meant to mitigate one category of security issue with untrusted input: features of jq which are meant to let the jq filter access files/data other than the direct input data given to the CLI. Specifically, the new `--sandbox` flag blocks the implicit loading of `$HOME/.jq`, and also blocks the use of `import` and `include` for loading other `jq` files. If other features are added to `jq` in the future which allow for reading files/data as part of the filter syntax, it is intended that the `--sandbox` flag would also gate access to those.
Should this flag block use of env? |
Good point, block i think. Would Any special consideration around the |
I will have a crack at updating the PR to make Regarding the |
I've updated the PR to clear the Let me know if there is anything else. |
The |
In a security-sensitive environment where the `--sandbox` flag can be used to mitigate some categories of threats from untrusted filter code and/or untrusted JSON data, it is also desirable to prevent leaking environment variable values (which often can include secrets in some environments). This commit does so by updating the behavior of `--sandbox` to also clear the environment variables seen by the jq filter code in the `$ENV` value and `env` builtin.
I've pushed an amended commit that fixes the copy/paste mistake in the test echo strings noticed by @emanuele6. Regarding There's possibly an argument for saying that For |
@itchyny @wader @emanuele6 - can any of you give this PR a deeper review and approve it (or give further comments on things to be improved)? |
I am not personally particularly interested in this feature as it is implemented, so I can't comment much on it. |
Sorry for waiting, but I think we need more discussion on this topic. Using |
Good points @itchyny. It would be great to hear what @nicowilliams thinks and how it would affect other future sandbox efforts. One thing might be to very clearly document what |
If there's a fear that Naming is hard, but perhaps something like The main point here is that |
In the interests of making some progress on this feature, which in some way or another has been discussed since at least March 2017, I'd like to propose that --sandbox be added as an experimental feature, or at least one with explicit indications that certain details are subject to change. A related thought is that, since there are so many different considerations and criteria regarding "security" and "sandbox", the "sandbox" flag could refer to a JSON object that specifies which "security flags" are to be enabled. E.g. {"now": false, "env": false} would mean that access to |
Are there any other existing examples in Seems more idiomatic to have separate flags that control separate features. Again, I'm only interested in disabling the features that allow for arbitrary file access from within the filter, so if I need to rename this flag to only mention the file restriction, I'm happy to rename it. Other flags could be added for controlling other features. I just need guidance from an authoritative maintainer on what the flag name and desired behavior should be, and I can adapt the PR to match. |
It's been about 6 months, so I'm checking in again. Any authoritative maintainer available to give direction on what changes are needed to get this across the finish line? |
Some use cases for
jq
may involve accepting untrusted input. See discussion in #1361 for some security considerations that may be relevant for those use cases.This commit adds a
--sandbox
flag which is meant to mitigate one category of security issue with untrusted input: features of jq which are meant to let the jq filter access files/data other than the direct input data given to the CLI.Specifically, the new
--sandbox
flag blocks the implicit loading of$HOME/.jq
, and also blocks the use ofimport
andinclude
for loading otherjq
files.If other features are added to
jq
in the future which allow for reading files/data as part of the filter syntax, it is intended that the--sandbox
flag would also gate access to those.