feat!: require user to opt-in to read unsafe pins

Author: machow

pins/boards.py

@@ -50,6 +50,7 @@ def __init__(
         fs: IFileSystem,
         versioned=True,
         meta_factory=MetaFactory(),
+        allow_insecure_read: "bool | None" = None,
     ):
         self.board = str(board)
         self.fs = fs
@@ -59,6 +60,7 @@ def __init__(
             raise NotImplementedError()
 
         self.versioned = versioned
+        self.allow_insecure_read = allow_insecure_read
 
     def pin_exists(self, name: str) -> bool:
         """Determine if a pin exists.
@@ -201,7 +203,10 @@ def pin_read(self, name, version: Optional[str] = None, hash: Optional[str] = No
         pin_name = self.path_to_pin(name)
 
         return load_data(
-            meta, self.fs, self.construct_path([pin_name, meta.version.version])
+            meta,
+            self.fs,
+            self.construct_path([pin_name, meta.version.version]),
+            allow_insecure_read=self.allow_insecure_read,
         )
 
     def pin_write(
@@ -603,7 +608,9 @@ def pin_download(self, name, version=None, hash=None) -> Sequence[str]:
         meta = self.pin_meta(name, version)
 
         if isinstance(meta, MetaRaw):
-            return load_data(meta, self.fs, None)
+            return load_data(
+                meta, self.fs, None, allow_insecure_read=self.allow_insecure_read
+            )
 
         raise NotImplementedError("TODO: allow download beyond MetaRaw.")
 

pins/config.py

@@ -2,11 +2,30 @@
 import os
 
 PINS_NAME = "pins-py"
+PINS_ENV_DATA_DIR = "PINS_DATA_DIR"
+PINS_ENV_CACHE_DIR = "PINS_CACHE_DIR"
+PINS_ENV_INSECURE_READ = "PINS_ALLOW_INSECURE_READ"
 
 
 def get_data_dir():
-    return os.environ.get("PINS_DATA_DIR", appdirs.user_data_dir(PINS_NAME))
+    return os.environ.get(PINS_ENV_DATA_DIR, appdirs.user_data_dir(PINS_NAME))
 
 
 def get_cache_dir():
-    return os.environ.get("PINS_CACHE_DIR", appdirs.user_cache_dir(PINS_NAME))
+    return os.environ.get(PINS_ENV_CACHE_DIR, appdirs.user_cache_dir(PINS_NAME))
+
+
+def get_allow_insecure_read(flag):
+    if flag is None:
+        env_var = os.environ.get(PINS_ENV_INSECURE_READ, "0")
+        try:
+            env_int = int(env_var)
+        except ValueError:
+            raise ValueError(
+                f"{PINS_ENV_INSECURE_READ} must be '0' or '1', but was set to "
+                f"{repr(env_var)}."
+            )
+
+        flag = bool(env_int)
+
+    return flag

pins/drivers.py

@@ -2,14 +2,23 @@
 
 from pathlib import Path
 
+from .config import get_allow_insecure_read, PINS_ENV_INSECURE_READ
 from .meta import Meta
-
+from .errors import PinsInsecureReadError
 
 # TODO: move IFileSystem out of boards, to fix circular import
 # from .boards import IFileSystem
 
 
-def load_data(meta: Meta, fs, path_to_version: "str | None" = None):
+UNSAFE_TYPES = frozenset(["joblib"])
+
+
+def load_data(
+    meta: Meta,
+    fs,
+    path_to_version: "str | None" = None,
+    allow_insecure_read: "bool | None" = None,
+):
     """Return loaded data, based on meta type.
     Parameters
     ----------
@@ -21,6 +30,14 @@ def load_data(meta: Meta, fs, path_to_version: "str | None" = None):
         A filepath used as the parent directory the data to-be-loaded lives in.
     """
     # TODO: extandable loading with deferred importing
+    if meta.type in UNSAFE_TYPES and not get_allow_insecure_read(allow_insecure_read):
+        raise PinsInsecureReadError(
+            f"Reading pin type {meta.type} is NOT secure. Set the allow_insecure_read=True "
+            f"when creating the board, or the {PINS_ENV_INSECURE_READ}=1 environment variable.\n"
+            "See:\n"
+            "  * https://docs.python.org/3/library/pickle.html \n"
+            "  * https://scikit-learn.org/stable/modules/model_persistence.html#security-maintainability-limitations"
+        )
 
     # Check that only a single file name was given
     fnames = [meta.file] if isinstance(meta.file, str) else meta.file

pins/errors.py

@@ -4,3 +4,7 @@ class PinsError(Exception):
 
 class PinsVersionError(PinsError):
     pass
+
+
+class PinsInsecureReadError(PinsError):
+    pass

pins/tests/test_boards.py

@@ -1,10 +1,11 @@
 import fsspec
+import os
 import uuid
 import pandas as pd
 import pytest
 
-from pins.tests.helpers import DEFAULT_CREATION_DATE
-from pins.errors import PinsError
+from pins.tests.helpers import DEFAULT_CREATION_DATE, rm_env
+from pins.errors import PinsError, PinsInsecureReadError
 from pins.meta import MetaRaw
 
 from datetime import datetime, timedelta
@@ -111,15 +112,44 @@ def test_board_pin_write_rsc_index_html(board, tmp_dir2, snapshot):
     "obj, type_", [(df, "csv"), (df, "joblib"), ({"a": 1, "b": [2, 3]}, "joblib")]
 )
 def test_board_pin_write_type(board, obj, type_, request):
-    meta = board.pin_write(obj, "test_pin", type=type_, title="some title")
-    dst_obj = board.pin_read("test_pin")
+    with rm_env("PINS_ALLOW_INSECURE_READ"):
+        os.environ["PINS_ALLOW_INSECURE_READ"] = "1"
+        meta = board.pin_write(obj, "test_pin", type=type_, title="some title")
+        dst_obj = board.pin_read("test_pin")
 
-    assert meta.type == type_
+        assert meta.type == type_
 
-    if isinstance(obj, pd.DataFrame):
-        assert obj.equals(dst_obj)
+        if isinstance(obj, pd.DataFrame):
+            assert obj.equals(dst_obj)
 
-    obj == dst_obj
+        obj == dst_obj
+
+
+def test_board_pin_read_insecure_fail_default(board):
+    board.pin_write({"a": 1}, "test_pin", type="joblib", title="some title")
+    with pytest.raises(PinsInsecureReadError) as exc_info:
+        board.pin_read("test_pin")
+
+    assert "joblib" in exc_info.value.args[0]
+
+
+def test_board_pin_read_insecure_fail_board_flag(board):
+    # board flag prioritized over env var
+    with rm_env("PINS_ALLOW_INSECURE_READ"):
+        os.environ["PINS_ALLOW_INSECURE_READ"] = "1"
+        board.allow_insecure_read = False
+        board.pin_write({"a": 1}, "test_pin", type="joblib", title="some title")
+        with pytest.raises(PinsInsecureReadError):
+            board.pin_read("test_pin")
+
+
+def test_board_pin_read_insecure_succeed_board_flag(board):
+    # board flag prioritized over env var
+    with rm_env("PINS_ALLOW_INSECURE_READ"):
+        os.environ["PINS_ALLOW_INSECURE_READ"] = "0"
+        board.allow_insecure_read = True
+        board.pin_write({"a": 1}, "test_pin", type="joblib", title="some title")
+        board.pin_read("test_pin")
 
 
 # pin_delete ==================================================================

pins/tests/test_config.py

@@ -0,0 +1,36 @@
+import os
+import pytest
+
+from pins.tests.helpers import rm_env
+from pins import config
+
+
+@pytest.fixture
+def env_unset():
+    with rm_env(
+        config.PINS_ENV_DATA_DIR,
+        config.PINS_ENV_CACHE_DIR,
+        config.PINS_ENV_INSECURE_READ,
+    ):
+        yield
+
+
+def test_allow_insecure_read_no_env(env_unset):
+    assert config.get_allow_insecure_read(True) is True
+    assert config.get_allow_insecure_read(False) is False
+
+
+def test_allow_insecure_read_env_1(env_unset):
+    os.environ[config.PINS_ENV_INSECURE_READ] = "1"
+
+    assert config.get_allow_insecure_read(True) is True
+    assert config.get_allow_insecure_read(False) is False
+    assert config.get_allow_insecure_read(None) is True
+
+
+def test_allow_insecure_read_env_0(env_unset):
+    os.environ[config.PINS_ENV_INSECURE_READ] = "0"
+
+    assert config.get_allow_insecure_read(True) is True
+    assert config.get_allow_insecure_read(False) is False
+    assert config.get_allow_insecure_read(None) is False