Repository: SFDO-Tooling/CumulusCI
Pull request #3422 (2022-11-01 22:11:53)

Switch back to using pickle for config files temporarily
- Revert "Use JSON instead of Pickle (#3390)"
- Version 3.68.1



Patch:
diff --git a/HISTORY.rst b/HISTORY.rst
index c1678919b..8b6a98293 100644
--- a/HISTORY.rst
+++ b/HISTORY.rst
@@ -2,6 +2,12 @@
 History
 =======
 
+3.68.1 (2022-11-01)
+-------------------
+Critical Changes 🎉
+
+- Revert a change in parsing config files which was incompatible with some configs
+
 3.68.0 (2022-10-31)
 -------------------
 
diff --git a/cumulusci/core/config/base_config.py b/cumulusci/core/config/base_config.py
index c6984e794..130ef3ac9 100644
--- a/cumulusci/core/config/base_config.py
+++ b/cumulusci/core/config/base_config.py
@@ -1,8 +1,6 @@
-import json
 import logging
 import os
 import warnings
-from datetime import date, datetime
 from functools import lru_cache
 from typing import Any, Dict, Optional
 
@@ -12,14 +10,7 @@ STRICT_GETATTR = os.environ.get("STRICT_GETATTR")
 CHECK_CONFIG_TYPES = os.environ.get("CHECK_CONFIG_TYPES")
 
 
-def load_dates(x):
-    if isinstance(x, datetime):
-        return {"$type": "datetime", "$value": x.isoformat()}
-    elif isinstance(x, date):
-        return {"$type": "date", "$value": x.isoformat()}
-
-
-class BaseConfig:
+class BaseConfig(object):
     """BaseConfig provides a common interface for nested access for all Config objects in CCI."""
 
     defaults = {}
@@ -53,9 +44,6 @@ class BaseConfig:
         """Subclasses may override this method to initialize :py:attr:`~config`"""
         pass
 
-    def _serialize(self) -> bytes:
-        return json.dumps(self.config, default=load_dates).encode("utf-8")
-
     @classmethod
     def _allowed_names(cls) -> Dict[str, type]:
         return getattr(cls, "__annotations__", {})
diff --git a/cumulusci/core/keychain/encrypted_file_project_keychain.py b/cumulusci/core/keychain/encrypted_file_project_keychain.py
index 66c365f6e..854df2ac5 100644
--- a/cumulusci/core/keychain/encrypted_file_project_keychain.py
+++ b/cumulusci/core/keychain/encrypted_file_project_keychain.py
@@ -1,9 +1,9 @@
 import base64
 import json
 import os
+import pickle
 import sys
 import typing as T
-from datetime import date, datetime
 from pathlib import Path
 from shutil import rmtree
 
@@ -26,7 +26,6 @@ from cumulusci.core.exceptions import (
 from cumulusci.core.keychain import BaseProjectKeychain
 from cumulusci.core.keychain.base_project_keychain import DEFAULT_CONNECTED_APP_NAME
 from cumulusci.core.utils import import_class, import_global
-from cumulusci.utils.pickle import restricted_loads
 from cumulusci.utils.yaml.cumulusci_yml import ScratchOrg
 
 DEFAULT_SERVICES_FILENAME = "DEFAULT_SERVICES.json"
@@ -34,7 +33,7 @@ DEFAULT_SERVICES_FILENAME = "DEFAULT_SERVICES.json"
 # The file permissions that we want set on all
 # .org and .service files. Equivalent to -rw-------
 SERVICE_ORG_FILE_MODE = 0o600
-OS_FILE_FLAGS = os.O_WRONLY | os.O_CREAT | os.O_TRUNC
+OS_FILE_FLAGS = os.O_WRONLY | os.O_CREAT
 if sys.platform.startswith("win"):
     # O_BINARY only available on Windows
     OS_FILE_FLAGS |= os.O_BINARY
@@ -44,6 +43,10 @@ BS = 16
 backend = default_backend()
 
 
+def pad(s):
+    return s + (BS - len(s) % BS) * chr(BS - len(s) % BS).encode("ascii")
+
+
 scratch_org_class = os.environ.get("CUMULUSCI_SCRATCH_ORG_CLASS")
 if scratch_org_class:
     scratch_org_factory = import_global(scratch_org_class)  # pragma: no cover
@@ -51,22 +54,6 @@ else:
     scratch_org_factory = ScratchOrgConfig
 
 
-def extract_dates(x):
-    if x.get("$type") == "date":
-        return date.fromisoformat(x["$value"])
-    if x.get("$type") == "datetime":
-        return datetime.fromisoformat(x["$value"])
-    assert "$type" not in x, f"Unknown $type: {x['$type']}"
-    return x
-
-
-def safe_load_json_or_pickle(data):
-    try:
-        return json.loads(data, object_hook=extract_dates)
-    except ValueError:
-        return restricted_loads(data)
-
-
 """
 EncryptedFileProjectKeychain
 ----------------------------
@@ -125,10 +112,10 @@ class EncryptedFileProjectKeychain(BaseProjectKeychain):
         return cipher, iv
 
     def _encrypt_config(self, config):
-        json_obj = config._serialize()
-        encryptor_value = json_obj + (BS - len(json_obj) % BS) * b" "
+        pickled = pickle.dumps(config.config, protocol=2)
+        pickled = pad(pickled)
         cipher, iv = self._get_cipher()
-        return base64.b64encode(iv + cipher.encryptor().update(encryptor_value))
+        return base64.b64encode(iv + cipher.encryptor().update(pickled))
 
     def _decrypt_config(self, config_class, encrypted_config, extra=None, context=None):
         if self.key:
@@ -140,17 +127,17 @@ class EncryptedFileProjectKeychain(BaseProjectKeychain):
             encrypted_config = base64.b64decode(encrypted_config)
             iv = encrypted_config[:16]
             cipher, iv = self._get_cipher(iv)
-            decrypted = cipher.decryptor().update(encrypted_config[16:])
+            pickled = cipher.decryptor().update(encrypted_config[16:])
             try:
-                config_obj = safe_load_json_or_pickle(decrypted)
-            except Exception as exc:
+                unpickled = pickle.loads(pickled, encoding="bytes")
+            except Exception:
                 raise KeychainKeyNotFound(
                     f"Unable to decrypt{' ' + context if context else ''}. "
                     "It was probably stored using a different CUMULUSCI_KEY."
-                ) from exc
+                )
             # Convert bytes created in Python 2
             config_dict = {}
-            for k, v in config_obj.items():
+            for k, v in unpickled.items():
                 if isinstance(k, bytes):
                     k = k.decode("utf-8")
                 if isinstance(v, bytes):
@@ -178,7 +165,7 @@ class EncryptedFileProjectKeychain(BaseProjectKeychain):
         if self.key:
             org_bytes = self._encrypt_config(config)
         else:
-            org_bytes = config._serialize()
+            org_bytes = pickle.dumps(config.config)
 
         assert org_bytes is not None, "org_bytes should have a value"
         return org_bytes
@@ -380,7 +367,7 @@ class EncryptedFileProjectKeychain(BaseProjectKeychain):
                 context=f"org config ({name})",
             )
         else:
-            config = safe_load_json_or_pickle(config)
+            config = pickle.loads(config)
             org = self._construct_config(OrgConfig, [config, name, self])
 
         return org
@@ -605,7 +592,7 @@ class EncryptedFileProjectKeychain(BaseProjectKeychain):
         elif self.key and not config_encrypted:
             config = self._encrypt_config(service_config)
         else:
-            config = service_config._serialize()
+            config = pickle.dumps(service_config.config)
 
         self.services[service_type][alias] = config
 
@@ -656,7 +643,7 @@ class EncryptedFileProjectKeychain(BaseProjectKeychain):
                 context=f"service config ({service_type}:{alias})",
             )
         else:
-            config = safe_load_json_or_pickle(config)
+            config = pickle.loads(config)
             org = self._construct_config(ConfigClass, [config, alias, self])
 
         return org
diff --git a/cumulusci/utils/pickle.py b/cumulusci/utils/pickle.py
deleted file mode 100644
index 7bb0ebd9e..000000000
--- a/cumulusci/utils/pickle.py
+++ /dev/null
@@ -1,44 +0,0 @@
-"""Tools for safely saving and loading pickles using an AllowedList"""
-
-import io
-import pickle
-import typing as T
-import warnings
-
-
-class Type_Cannot_Be_Used_With_Random_Reference(T.NamedTuple):
-    """This type cannot be unpickled."""
-
-    module: str
-    name: str
-
-
-_SAFE_CLASSES = {
-    ("decimal", "Decimal"),
-    ("datetime", "date"),
-    ("datetime", "datetime"),
-    ("datetime", "timedelta"),
-    ("datetime", "timezone"),
-}
-
-
-class RestrictedUnpickler(pickle.Unpickler):
-    """Safe unpickler with an allowed-list"""
-
-    count = 0
-
-    def find_class(self, module, name):
-        # Only allow safe classes from builtins.
-        if (module, name) in _SAFE_CLASSES:
-            return super().find_class(module, name)
-        else:
-            # Return a "safe" object that does nothing.
-            if RestrictedUnpickler.count < 10:
-                warnings.warn(f"Cannot save and refer to {module}, {name}")
-                RestrictedUnpickler.count += 1
-            return lambda *args: Type_Cannot_Be_Used_With_Random_Reference(module, name)
-
-
-def restricted_loads(data):
-    """Helper function analogous to pickle.loads()."""
-    return RestrictedUnpickler(io.BytesIO(data)).load()
diff --git a/cumulusci/version.txt b/cumulusci/version.txt
index 2d09e4284..cadc8491d 100644
--- a/cumulusci/version.txt
+++ b/cumulusci/version.txt
@@ -1,1 +1,1 @@
-3.68.0
+3.68.1
diff --git a/docs/history.md b/docs/history.md
index 71f04a2d7..e605b5781 100644
--- a/docs/history.md
+++ b/docs/history.md
@@ -1,5 +1,9 @@
 # History
 
+## 3.68.1 (2022-11-01)
+
+-   Revert a change in parsing config files which was incompatible with some configs
+
 ## 3.68.0 (2022-10-31)
 
 Critical Changes 🎉


Test patch:
diff --git a/cumulusci/core/keychain/tests/test_encrypted_file_project_keychain.py b/cumulusci/core/keychain/tests/test_encrypted_file_project_keychain.py
index 0b4f33e1f..73a7c2809 100644
--- a/cumulusci/core/keychain/tests/test_encrypted_file_project_keychain.py
+++ b/cumulusci/core/keychain/tests/test_encrypted_file_project_keychain.py
@@ -1,6 +1,7 @@
 import datetime
 import json
 import os
+import pickle
 import re
 import sys
 import tempfile
@@ -73,20 +74,11 @@ class TestEncryptedFileProjectKeychain:
     #               Orgs                  #
     #######################################
 
-    def test_set_and_get_org_with_dates__global(self, keychain, org_config):
+    def test_set_and_get_org__global(self, keychain, org_config):
         org_config.global_org = True
-
-        custom_datetime = datetime.datetime.now()
-        custom_date = datetime.datetime.now().date()
-        org_config.config["custom_datetime"] = custom_datetime
-        org_config.config["custom_date"] = custom_date
-
         keychain.set_org(org_config, True)
         assert list(keychain.orgs.keys()) == ["test"]
-        config = keychain.get_org("test").config
-        assert config == org_config.config
-        assert config["custom_datetime"] == custom_datetime
-        assert config["custom_date"] == custom_date
+        assert keychain.get_org("test").config == org_config.config
 
     def test_get_org__with_config_properly_overridden(
         self, keychain, scratch_org_config
@@ -120,7 +112,7 @@ class TestEncryptedFileProjectKeychain:
 
         filepath = Path(keychain.project_local_dir, "test.org")
         with open(filepath, "rb") as f:
-            assert json.load(f) == org_config.config
+            assert pickle.load(f) == org_config.config
 
     def test_set_org__should_not_save_when_environment_project_keychain_set(
         self, keychain, org_config