Fix critical bugs, security vulnerability, and code quality issues

.github/workflows/main.yml

@@ -1,51 +1,41 @@
-# This is a basic workflow to help you get started with Actions
-
 name: Unittests
 
-# Controls when the action will run.
 on:
-  # Triggers the workflow on push or pull request events but only for the master branch
   push:
     branches: [ master, develop ]
   pull_request:
     branches: [ master, develop ]
-
-  # Allows you to run this workflow manually from the Actions tab
   workflow_dispatch:
 
-# A workflow run is made up of one or more jobs that can run sequentially or in parallel
 jobs:
-
-  # This workflow contains a single job called "build"
   build:
     environment: Master
-    # The type of runner that the job will run on
     runs-on: ubuntu-latest
 
-    # Steps represent a sequence of tasks that will be executed as part of the job
+    strategy:
+      matrix:
+        python-version: ["3.9", "3.11", "3.12"]
+
     steps:
-      # Checks-out your repository under $GITHUB_WORKSPACE, so your job can access it
-      - uses: actions/checkout@v2
-      - run: touch .env
-      - run: docker-compose pull
+      - uses: actions/checkout@v4
 
-      # In this step, this action saves a list of existing images,
-      # the cache is created without them in the post run.
-      # It also restores the cache if it exists.
-      - uses: satackey/action-docker-layer-caching@v0.0.11
-        # Ignore the failure of a step and avoid terminating the job.
-        continue-on-error: true
+      - name: Set up Python ${{ matrix.python-version }}
+        uses: actions/setup-python@v5
+        with:
+          python-version: ${{ matrix.python-version }}
+
+      - name: Install Poetry
+        run: pip install poetry
 
-      - run: docker-compose build
+      - name: Install dependencies
+        run: poetry install --only main
 
-      # Runs a single command using the runners shell
       - name: Run Unit Tests
-        run: docker-compose run web poetry run python -m unittest
-      - name: Build and publish to pypi
-        if: github.ref == 'refs/heads/master'
-        uses: JRubics/poetry-publish@v1.13
+        run: poetry run python -m unittest discover -s valley/tests -v
+
+      - name: Build and publish to PyPI
+        if: github.ref == 'refs/heads/master' && matrix.python-version == '3.11'
+        uses: JRubics/poetry-publish@v2.0
         with:
           pypi_token: ${{ secrets.PYPI_TOKEN }}
           ignore_dev_requirements: "yes"
-      - name: Remove .env file
-        run: rm .env

requirements.txt

@@ -1 +1 @@
-
+# Dependencies are managed via Poetry. To install, run: poetry install

valley/declarative.py

@@ -1,4 +1,3 @@
-import collections
 from typing import Any, Dict, List, Type
 
 class DeclaredVars(object):
@@ -62,7 +61,7 @@ def __new__(cls, name: str, bases: tuple, attrs: Dict[str, Any]) -> Type:
         return new_class
 
     @classmethod
-    def __prepare__(mcls, cls: str, bases: tuple) -> collections.OrderedDict:
+    def __prepare__(mcls, cls: str, bases: tuple) -> Dict[str, Any]:
         """
         Prepares the class namespace.
 
@@ -71,6 +70,6 @@ def __prepare__(mcls, cls: str, bases: tuple) -> collections.OrderedDict:
             bases (tuple): A tuple of base classes.
 
         Returns:
-            collections.OrderedDict: An ordered dictionary for the class namespace.
+            Dict[str, Any]: A dictionary for the class namespace (insertion-ordered in Python 3.7+).
         """
-        return collections.OrderedDict()
+        return {}

valley/properties.py

@@ -63,7 +63,7 @@ def validate(self, value: Any, key: str) -> None:
         Raises:
             ValidationException: If the value does not pass the validation checks.
         """
-        if not value and not isinstance(self.get_default_value(), type(None)):
+        if value is None and self.get_default_value() is not None:
             value = self.get_default_value()
         for validator in self.validators:
             validator.validate(value, key)
@@ -90,8 +90,8 @@ def get_db_value(self, value: Any) -> Any:
         Returns:
             Any: The converted value suitable for database storage.
         """
-        if not value:
-            return
+        if value is None:
+            return None
         return value
 
     def get_python_value(self, value: Any) -> Any:
@@ -116,7 +116,7 @@ class StringProperty(BaseProperty):
 
     def get_validators(self) -> None:
         """
-        Initialize the validators for the CharProperty.
+        Initialize the validators for the StringProperty.
         """
         super().get_validators()
         self.validators.append(StringValidator())
@@ -172,6 +172,10 @@ def get_validators(self) -> None:
         """
         super().get_validators()
         self.validators.append(FloatValidator())
+        if 'min_value' in self.kwargs:
+            self.validators.append(MinValueValidator(self.kwargs['min_value']))
+        if 'max_value' in self.kwargs:
+            self.validators.append(MaxValueValidator(self.kwargs['max_value']))
 
 
 class BooleanProperty(BaseProperty):
@@ -188,15 +192,16 @@ def get_validators(self) -> None:
         super().get_validators()
         self.validators.append(BooleanValidator())
 
-    def get_default_value(self) -> bool:
+    def get_default_value(self) -> Optional[bool]:
         """
         Get the default value of the BooleanProperty.
 
         Returns:
-            bool: The default boolean value.
+            Optional[bool]: The default boolean value, or None if unset.
         """
-        default = self.default_value
-        return bool(default)
+        if self.default_value is None:
+            return None
+        return bool(self.default_value)
 
 
 class DateProperty(BaseProperty):
@@ -342,33 +347,34 @@ def get_db_value(self, value: Any) -> Any:
         if self.return_type == 'single':
             if not self.return_prop:
                 raise ValueError('ForeignProperty requires the return_prop argument if return_type is "single"')
-            return value._data[self.return_prop]
+            return value.to_dict()[self.return_prop]
         if self.return_type == 'dict':
-            return value._data
+            return value.to_dict()
         if self.return_type == 'json':
             return json.dumps(value, cls=ValleyEncoder)
-        else:
-            return value
+        return value
 
 
 class ForeignListProperty(ListProperty):
-    def __init__(self, foreign_class: Type, **kwargs: Any):
+    def __init__(self, foreign_class: Type, return_type: str = 'list', **kwargs: Any):
         """
         Initialize a ForeignListProperty.
 
         Args:
             foreign_class (Type): The class of the foreign object.
+            return_type (str): How to serialize items — 'list' (default), 'json', or 'raw'.
             **kwargs: Additional keyword arguments.
         """
         self.foreign_class = foreign_class
+        self.return_type = return_type
         super().__init__(**kwargs)
 
     def get_validators(self) -> None:
         """
         Get validators for the foreign list property, adding ForeignListValidator to the list.
         """
         super().get_validators()
-        self.validators.insert(len(self.validators), ForeignListValidator(self.foreign_class))
+        self.validators.append(ForeignListValidator(self.foreign_class))
 
     def get_db_value(self, value: Any) -> Any:
         """
@@ -383,11 +389,10 @@ def get_db_value(self, value: Any) -> Any:
         if not value:
             return None
         if self.return_type == 'list':
-            return [obj._data for obj in value]
+            return [obj.to_dict() for obj in value]
         if self.return_type == 'json':
             return json.dumps(value, cls=ValleyEncoder)
-        else:
-            return value
+        return value
 
 
 class MultiProperty(BaseProperty):

valley/schema.py

@@ -154,3 +154,4 @@ class DeclarativeVariablesMetaclass(DVM):
 
 class Schema(BaseSchema, metaclass=DeclarativeVariablesMetaclass):
     BUILTIN_DOC_ATTRS = []
+    _create_error_dict = True

valley/tests/examples/example_schemas.py

@@ -21,7 +21,7 @@ class NameSchema(valley.Schema):
     _create_error_dict = True
     name = valley.StringProperty(required=True)
 
-    def __unicode__(self):
+    def __str__(self):
         return self.name
 
 

valley/tests/test_declarative.py

@@ -1,4 +1,3 @@
-import collections
 import unittest
 
 from valley.schema import BaseSchema
@@ -43,9 +42,9 @@ def test_metaclass_new_instance(self):
         self.assertIn('field2', instance._base_properties)
 
     def test_metaclass_prepare_namespace(self):
-        """Test if __prepare__ method of metaclass returns an OrderedDict."""
+        """Test if __prepare__ method of metaclass returns an ordered dict."""
         namespace = DeclarativeVariablesMetaclass.__prepare__('Test', (object,))
-        self.assertIsInstance(namespace, collections.OrderedDict)
+        self.assertIsInstance(namespace, dict)
 
 
 if __name__ == '__main__':

valley/tests/test_utils.py

@@ -5,6 +5,12 @@
 from valley.utils import import_util
 from valley.utils.json_utils import (ValleyEncoder, ValleyDecoder)
 
+ALLOWED_TYPES = {
+    'valley.tests.examples.example_schemas.Breed',
+    'valley.tests.examples.example_schemas.Dog',
+    'valley.tests.examples.example_schemas.Troop',
+}
+
 
 class UtilTest(unittest.TestCase):
     json_string = '{"dogs": [{"breed": {"name": "Cocker Spaniel",' \
@@ -27,8 +33,13 @@ def test_json_encoder(self):
         self.assertEqual(json.dumps(durham, cls=ValleyEncoder), self.json_string)
 
     def test_json_decoder(self):
-        new_troop = json.loads(self.json_string, cls=ValleyDecoder)
+        new_troop = json.loads(self.json_string, cls=ValleyDecoder, allowed_types=ALLOWED_TYPES)
         self.assertEqual(new_troop.name, durham.name)
         self.assertEqual(new_troop.primary_breed.name, durham.primary_breed.name)
         self.assertEqual(new_troop.dogs[0].name, durham.dogs[0].name)
         self.assertEqual(new_troop.dogs[1].name, durham.dogs[1].name)
+
+    def test_json_decoder_rejects_unlisted_type(self):
+        malicious_json = '{"_type": "os.system", "cmd": "echo pwned"}'
+        with self.assertRaises(ValueError):
+            json.loads(malicious_json, cls=ValleyDecoder, allowed_types=ALLOWED_TYPES)

valley/utils/json_utils.py

@@ -8,10 +8,13 @@ class ValleyEncoder(json.JSONEncoder):
     show_type = True
 
     def default(self, obj):
-        if not isinstance(obj, (list,dict,int,float,bool)):
+        if not isinstance(obj, (list, dict, int, float, bool)):
             obj_dict = obj.to_dict()
             if self.show_type:
-                obj_dict['_type'] = '{}.{}'.format(inspect.getmodule(obj).__name__, obj.__class__.__name__)
+                module = inspect.getmodule(obj)
+                if module is None:
+                    raise TypeError(f"Cannot determine module for {obj.__class__.__name__}; cannot encode type info.")
+                obj_dict['_type'] = f'{module.__name__}.{obj.__class__.__name__}'
             return obj_dict
         return super(ValleyEncoder, self).default(obj)
 
@@ -21,13 +24,27 @@ class ValleyEncoderNoType(ValleyEncoder):
 
 
 class ValleyDecoder(json.JSONDecoder):
-    def __init__(self, *args, **kwargs):
+    """
+    JSON decoder that reconstructs Valley Schema objects from encoded type information.
+
+    By default, only types explicitly registered in ``allowed_types`` may be
+    instantiated during decoding.  Pass ``allowed_types=None`` to disable the
+    allowlist check (not recommended for untrusted input).
+    """
+
+    def __init__(self, *args, allowed_types=None, **kwargs):
+        self.allowed_types = allowed_types  # set of fully-qualified type strings, or None to disable
         json.JSONDecoder.__init__(self, object_hook=self.object_hook, *args, **kwargs)
 
     def object_hook(self, obj):
         if '_type' not in obj:
             return obj
-        klass = import_util(obj['_type'])
+        type_name = obj['_type']
+        if self.allowed_types is not None and type_name not in self.allowed_types:
+            raise ValueError(
+                f"Refusing to deserialize type '{type_name}': not in the allowed_types allowlist. "
+                "Pass allowed_types=None to disable this check (unsafe for untrusted input)."
+            )
+        klass = import_util(type_name)
         obj.pop('_type')
-
         return klass(**obj)