Skip to content

Refactor models to Pydantic#320

Merged
colesmj merged 18 commits intoOWASP:masterfrom
NoodlesNZ:pydantic
Apr 15, 2026
Merged

Refactor models to Pydantic#320
colesmj merged 18 commits intoOWASP:masterfrom
NoodlesNZ:pydantic

Conversation

@NoodlesNZ
Copy link
Copy Markdown
Contributor

Replaces the custom var descriptor system with Pydantic v2 BaseModel. This simplifies validation and makes it easier to extend. The public API is unchanged, existing threat model scripts should work without modification.

Changes:

  • Split the monolithic pytm.py into per-class modules (element.py, actor.py, asset.py, dataflow.py, tm.py, etc.)
  • All model classes now use Field() for attribute definitions and validators for coercion
  • Added LLM element with supporting threat conditions
  • Removed sqlDump / --sqldump (dropped the pydal dependency)
  • Dropped Python 3.8 support; added 3.12–3.14

Bug fixes:

  • Threat conditions targeting base classes (e.g. Asset) now correctly use isinstance rather than string comparison
  • Finding used as an override stub no longer registers a phantom element in the global state
  • Legacy string assignment to a dataflow's data field was incorrectly classifying data as PUBLIC instead of UNKNOWN
  • Threat.apply() returned None instead of False on type mismatch

Added Tests:
Added tests/test_pydantic_models.py covering the new validation and coercion behaviour (DataSet, field validators, Threat legacy field mapping). All existing tests pass.

@colesmj colesmj assigned colesmj and unassigned colesmj Apr 11, 2026
@colesmj colesmj self-requested a review April 11, 2026 20:37
Copy link
Copy Markdown
Collaborator

@colesmj colesmj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for submitting this refactoring of the pytm base code; it was long overdue. The change to pydantic instead of the legacy var mechanism will make for cleaner code and also create some new possibilities for extending pytm.

No threats needed to change to support this refactor (e.g. the condition evals were all "safe" with the checks you added)?

Comment thread pytm/asset.py Outdated
Comment thread pytm/dataflow.py
Comment thread pytm/pytm.py
@NoodlesNZ
Copy link
Copy Markdown
Contributor Author

Thank you for submitting this refactoring of the pytm base code; it was long overdue. The change to pydantic instead of the legacy var mechanism will make for cleaner code and also create some new possibilities for extending pytm.

No threats needed to change to support this refactor (e.g. the condition evals were all "safe" with the checks you added)?

Thanks for the review and the comments! There are a bunch of changes I'd like to make but I wanted to keep it invisible to most end users. Maybe with more drastic changes this might become part of a 2.0 release?

No threats needed to change for now. I did think about moving the threat library to python code, but there was not a lot to gain other than using enums and classes (to save string matching).

@NoodlesNZ
Copy link
Copy Markdown
Contributor Author

I've made some changes to add docstrings to help address #294

@NoodlesNZ NoodlesNZ mentioned this pull request Apr 13, 2026
@colesmj colesmj self-requested a review April 13, 2026 19:54
Copy link
Copy Markdown
Collaborator

@colesmj colesmj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good changes, beneficial to all.

@NoodlesNZ
Copy link
Copy Markdown
Contributor Author

I'm just doing some final clean ups and adding a bit of test coverage. I'll try push the last changes later today. Thank you for the reviews :D

@NoodlesNZ
Copy link
Copy Markdown
Contributor Author

This should be all ready to go now

@NoodlesNZ NoodlesNZ mentioned this pull request Apr 14, 2026
@colesmj colesmj merged commit cfc38f8 into OWASP:master Apr 15, 2026
3 checks passed
@NoodlesNZ NoodlesNZ deleted the pydantic branch April 15, 2026 09:29
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