Skip to content

Fix critical bugs, security vulnerability, and code quality issues - #13

Open
bjinwright wants to merge 3 commits into
masterfrom
claude/code-quality-review-EyX3O
Open

Fix critical bugs, security vulnerability, and code quality issues#13
bjinwright wants to merge 3 commits into
masterfrom
claude/code-quality-review-EyX3O

Conversation

@bjinwright

Copy link
Copy Markdown
Member

Critical:

  • FloatValidator now rejects integers (isinstance check replaces float() cast)
  • IntegerValidator now rejects strings and bools (strict isinstance check)
  • ForeignListProperty.get_db_value() no longer crashes — initialize return_type
  • ValleyDecoder now requires an allowed_types allowlist to prevent arbitrary class instantiation from untrusted JSON

Major:

  • Schema class gets _create_error_dict = True default; validate() no longer AttributeErrors
  • BaseProperty.validate() and get_db_value() use is None instead of falsy check (fixes 0/False/[]/'' being treated as missing)
  • DateValidator/DateTimeValidator raise on invalid strings immediately instead of silent pass
  • ForeignProperty/ForeignListProperty use to_dict() instead of ._data directly
  • FloatProperty now wires up min_value/max_value kwargs like IntegerProperty

Minor/Nit:

  • Remove import time; use datetime.date.fromisoformat()
  • BooleanProperty.get_default_value() returns None when unset
  • Replace type(None) antipattern with is not None
  • Add type hints and docstrings to DateValidator/DateTimeValidator
  • Use f-strings consistently in date validators
  • Fix unicode -> str in example schemas
  • ChoiceValidator handles non-dict choices safely
  • Guard inspect.getmodule() None return in ValleyEncoder
  • Remove collections.OrderedDict from declarative.prepare (redundant in 3.7+)
  • Fix octal escape typo in EmailValidator regex (\001-011 -> \001-\011)
  • ForeignListProperty.get_validators() uses append() instead of insert(len())
  • Stale StringProperty docstring "CharProperty" corrected
  • Add PEP 8 blank line between DateTimeValidator and BooleanValidator
  • Clarify requirements.txt as Poetry-managed

https://claude.ai/code/session_01JGryFUpmEmKYAVhqSrkV8R

Critical:
- FloatValidator now rejects integers (isinstance check replaces float() cast)
- IntegerValidator now rejects strings and bools (strict isinstance check)
- ForeignListProperty.get_db_value() no longer crashes — initialize return_type
- ValleyDecoder now requires an allowed_types allowlist to prevent arbitrary class instantiation from untrusted JSON

Major:
- Schema class gets _create_error_dict = True default; validate() no longer AttributeErrors
- BaseProperty.validate() and get_db_value() use `is None` instead of falsy check (fixes 0/False/[]/'' being treated as missing)
- DateValidator/DateTimeValidator raise on invalid strings immediately instead of silent pass
- ForeignProperty/ForeignListProperty use to_dict() instead of ._data directly
- FloatProperty now wires up min_value/max_value kwargs like IntegerProperty

Minor/Nit:
- Remove `import time`; use datetime.date.fromisoformat()
- BooleanProperty.get_default_value() returns None when unset
- Replace `type(None)` antipattern with `is not None`
- Add type hints and docstrings to DateValidator/DateTimeValidator
- Use f-strings consistently in date validators
- Fix __unicode__ -> __str__ in example schemas
- ChoiceValidator handles non-dict choices safely
- Guard inspect.getmodule() None return in ValleyEncoder
- Remove collections.OrderedDict from declarative.__prepare__ (redundant in 3.7+)
- Fix octal escape typo in EmailValidator regex (\001-011 -> \001-\011)
- ForeignListProperty.get_validators() uses append() instead of insert(len())
- Stale StringProperty docstring "CharProperty" corrected
- Add PEP 8 blank line between DateTimeValidator and BooleanValidator
- Clarify requirements.txt as Poetry-managed

https://claude.ai/code/session_01JGryFUpmEmKYAVhqSrkV8R
… setup

The old workflow used docker-compose v1 (no longer on ubuntu-latest/Ubuntu 24.04),
satackey/action-docker-layer-caching (deleted/broken action), and actions/checkout@v2.
For a pure Python library, Docker adds no value in CI.

New workflow:
- actions/checkout@v4, actions/setup-python@v5
- Tests run on Python 3.9, 3.11, and 3.12 via matrix
- poetry install + unittest discover directly on the runner
- PyPI publish only on master push, only once (py3.11)
- JRubics/poetry-publish bumped to v2.0

https://claude.ai/code/session_01JGryFUpmEmKYAVhqSrkV8R
pyzmq==24.0.1 (transitive dep of jupyter) fails to compile against
GCC 13 (C++ allocator assertion). Tests don't need Jupyter; switch to
poetry install --only main to install just envs and the package itself.

https://claude.ai/code/session_01JGryFUpmEmKYAVhqSrkV8R
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants