Security option for library path (#4925)

Co-authored-by: Greg Finley <gregory.finley@gmail.com>

docs/source/configuration.rst

@@ -11,7 +11,9 @@ must be done via a file, because it otherwise gets slightly complicated.
 For details of what's available on the command line check out
 the :ref:`cliref`.
 
-Configuration files
+.. _`config-files`:
+
+Configuration Files
 -------------------
 
 For file based configuration *SQLFluff* will look for the following
@@ -656,6 +658,8 @@ projects. In particular it provides mock objects for:
 .. _`dbt`: https://www.getdbt.com/
 .. _`github`: https://www.github.com/sqlfluff/sqlfluff
 
+.. _jinja_library_templating:
+
 Library Templating
 """"""""""""""""""
 

docs/source/production.rst

@@ -1,9 +1,68 @@
-Production Usage
-================
+.. _production-use:
+
+Production Usage & Security
+===========================
 
 SQLFluff is designed to be used both as a utility for developers but also to
 be part of `CI/CD`_ pipelines.
 
+.. _security:
+
+Security Considerations
+-----------------------
+
+A full list of `Security Advisories is available on GitHub <https://github.com/sqlfluff/sqlfluff/security/advisories>`_.
+
+Given the context of how SQLFluff is designed to be used, there are three
+different tiers of access which users may have access to manipulate how the
+tool functions in a secure environment.
+
+#. *Users may have edit access to the SQL code which is being linted*. While
+   SQLFluff does not execute the SQL itself, in the process of the
+   :ref:`templating step <templater>` (in particular via jinja or dbt),
+   certain macros may have the ability to execute arbitrary SQL code (e.g.
+   the `dbt run_query macro`_). For the Jinja templater, SQLFluff uses the
+   `Jinja2 SandboxedEnvironment`_ to limit the execution on unsafe code. When
+   looking to further secure this situation, see below for ways to limit the
+   ability of users to import other libraries.
+
+#. *Users may have edit access to the SQLFluff :ref:`config-files`*. In some
+   (perhaps, many) environments, the users who can edit SQL files may also
+   be able to access and edit the :ref:`config-files`. It's important to note
+   that because of :ref:`in_file_config`, that users who can edit SQL files
+   which are designed to be linted, will also have access to the vast majority
+   of any configuration options available in :ref:`config-files`. This means
+   that there is minimal additional protection from restricting access to
+   :ref:`config-files` for users who already have access to edit the linting
+   target files (as described above).
+
+#. *Users may have access to change how SQLFluff is invoked*. SQLFluff can
+   be invoked either as a command line too or via the python API. Typically
+   the method is fixed for a given application. When thinking about how to
+   restrict the ability of users to call unsecure code, SQLFluff aims to
+   provide options at the point of invocation. In particular, as described
+   above, the primary risk vector for SQLFluff is the macro environment
+   as described in :ref:`templateconfig`. To restrict users being able to
+   bring arbitrary python methods into sqlfluff via the ``library_path``
+   configuration value (see :ref:`jinja_library_templating`), we recommend
+   that for secure environments you override this config value either by
+   providing an ``override`` option to the :class:`FluffConfig` object if
+   using the Python API or via the ``--library-path`` CLI option:
+
+   To disable this option entirely via the CLI:
+
+   .. code-block:: bash
+
+      $ sqlfluff lint my_path --library-path none
+
+   To disable this option entirely via the python API:
+
+   .. literalinclude:: ../../examples/04_config_overrides.py
+      :language: python
+
+.. _`Jinja2 SandboxedEnvironment`: https://jinja.palletsprojects.com/en/3.0.x/sandbox/#jinja2.sandbox.SandboxedEnvironment
+.. _`dbt run_query macro`: https://docs.getdbt.com/reference/dbt-jinja-functions/run_query
+
 Using SQLFluff on a whole sql codebase
 --------------------------------------
 
@@ -21,34 +80,34 @@ more.
 
 .. _diff-quality:
 
-Using SQLFluff on changes using `diff-quality`
-----------------------------------------------
+Using SQLFluff on changes using ``diff-quality``
+------------------------------------------------
 
 For projects with large amounts of (potentially imperfect) SQL code, the full
 SQLFluff output could be very large, which can be distracting -- perhaps the CI
 build for a one-line SQL change shouldn't encourage the developer to fix lots
 of unrelated quality issues.
 
 To support this use case, SQLFluff integrates with a quality checking tool
-called `diff-quality`. By running SQLFluff using `diff-quality` (rather than
-running it directly), you can limit the the output to the new or modified SQL
-in the branch (aka pull request or PR) containing the proposed changes.
+called ``diff-quality``. By running SQLFluff using ``diff-quality`` (rather
+than running it directly), you can limit the the output to the new or modified
+SQL in the branch (aka pull request or PR) containing the proposed changes.
 
 Currently, ``diff-quality`` requires that you are using ``git`` for version
 control.
 
-NOTE: Installing SQLFluff automatically installs the `diff_cover` package that
-provides the `diff-quality` tool.
+NOTE: Installing SQLFluff automatically installs the ``diff_cover`` package
+that provides the ``diff-quality`` tool.
 
-Adding `diff-quality` to your builds
-^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+Adding ``diff-quality`` to your builds
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 In your CI build script:
 
 1. Set the current working directory to the ``git`` repository containing the
 SQL code to be checked.
 
-2. Run `diff-quality`, specifying SQLFluff as the underlying tool:
+2. Run ``diff-quality``, specifying SQLFluff as the underlying tool:
 
 .. code-block:: text
 
@@ -72,18 +131,19 @@ The output will look something like:
     -------------
 
 These messages are basically the same as those provided directly by SQLFluff,
-although the format is a little different. Note that `diff-quality` only lists
+although the format is a little different. Note that ``diff-quality`` only lists
 the line _numbers_, not the character position. If you need the character
 position, you will need to run SQLFluff directly.
 
-For more information on `diff-quality`, see the
+For more information on ``diff-quality``, see the
 `documentation <https://diff-cover.readthedocs.io/en/latest/>`_. It covers topics
 such as:
 
 * Generating HTML reports
 * Controlling which branch to compare against (i.e. to determine new/changed
   lines). The default is `origin/master`.
-* Configuring `diff-quality` to return an error code if the quality is too low
+* Configuring ``diff-quality`` to return an error code if the quality is
+  too low.
 * Troubleshooting
 
 .. _using-pre-commit:

examples/04_config_overrides.py

@@ -0,0 +1,20 @@
+"""This is an example of providing config overrides."""
+
+from sqlfluff.core import Linter, FluffConfig
+
+sql = "SELECT 1\n"
+
+
+config = FluffConfig(
+    overrides={
+        "dialect": "snowflake",
+        # NOTE: We explicitly set the string "none" here rather
+        # than a None literal so that it overrides any config
+        # set by any config files in the path.
+        "library_path": "none",
+    }
+)
+
+linted_file = Linter(config=config).lint_string(sql)
+
+assert linted_file.get_violations() == []

src/sqlfluff/cli/click_deprecated_option.py

@@ -92,7 +92,7 @@ def process(value: Any, state: ParsingState) -> None:
             finally:
                 del frame
 
-            if opt in deprecated:  # type: ignore
+            if opt in deprecated:
                 msg = (
                     f"DeprecationWarning: The option {opt!r} is deprecated, "
                     f"use {preferred!r}."

src/sqlfluff/cli/commands.py

@@ -309,6 +309,16 @@ def core_options(f: Callable) -> Callable:
         default=None,
         help="Set this flag to ignore inline noqa comments.",
     )(f)
+    f = click.option(
+        "--library-path",
+        default=None,
+        help=(
+            "Override the `library_path` value from the [sqlfluff:templater:jinja]"
+            " configuration value. Set this to 'none' to disable entirely."
+            " This overrides any values set by users in configuration files or"
+            " inline directives."
+        ),
+    )(f)
     return f
 
 
@@ -382,8 +392,15 @@ def get_config(
     from_root_kwargs = {}
     if "require_dialect" in kwargs:
         from_root_kwargs["require_dialect"] = kwargs.pop("require_dialect")
+    library_path = kwargs.pop("library_path", None)
     # Instantiate a config object (filtering out the nulls)
     overrides = {k: kwargs[k] for k in kwargs if kwargs[k] is not None}
+    if library_path is not None:
+        # Check for a null value
+        if library_path.lower() == "none":
+            library_path = None  # Set an explicit None value.
+        # Set the global override
+        overrides["library_path"] = library_path
     try:
         return FluffConfig.from_root(
             extra_config_path=extra_config_path,

src/sqlfluff/core/templaters/jinja.py

@@ -130,7 +130,8 @@ def _extract_macros_from_config(self, config, env, ctx):
         return macro_ctx
 
     def _extract_libraries_from_config(self, config):
-        library_path = config.get_section(
+        # If a more global library_path is set, let that take precedence.
+        library_path = config.get("library_path") or config.get_section(
             (self.templater_selector, self.name, "library_path")
         )
         if not library_path:

test/cli/commands_test.py

@@ -545,6 +545,29 @@ def test__cli__command_lint_parse(command):
             ),
             1,
         ),
+        # Test overriding library path when it doesn't cause an issue
+        (
+            (
+                lint,
+                ["test/fixtures/cli/passing_a.sql", "--library-path", "none"],
+            ),
+            0,
+        ),
+        # Test overriding library path when it DOES cause an issue
+        # (because macros won't be found).
+        (
+            (
+                # Render because that's the step where the issue will
+                # occur.
+                render,
+                [
+                    "test/fixtures/templater/jinja_r_library_in_macro/jinja.sql",
+                    "--library-path",
+                    "none",
+                ],
+            ),
+            1,
+        ),
         # Test render fail
         (
             (

test/fixtures/templater/jinja_m_libraries_module/.sqlfluff

@@ -1,2 +1,3 @@
-[sqlfluff:templater:jinja]
+[sqlfluff]
+# Test setting the library_path via the global setting (not via jinja config)
 library_path=libs