Skip to content

First pass at converting threats to python classes#328

Draft
NoodlesNZ wants to merge 1 commit into
OWASP:masterfrom
NoodlesNZ:python-threats
Draft

First pass at converting threats to python classes#328
NoodlesNZ wants to merge 1 commit into
OWASP:masterfrom
NoodlesNZ:python-threats

Conversation

@NoodlesNZ

Copy link
Copy Markdown
Contributor

Related to #327 here's an implementation of using python classes to define threats. It adds backwards compatibility via _add_threats_from_json() in tm.py

Created as a draft to show how this is possible and gauge whether this is something that pytm wants to adopt.

@raphaelahrens

Copy link
Copy Markdown
Contributor

Why did you implement the Threats as classes and no just as objects?

Further this change should consider the use case from PR #263 , would this only be possible with the JSON format or would this mean that python modules have to be included?

izar pushed a commit that referenced this pull request Jul 5, 2026
)

Fixes #272.

DO04 ("XML Entity Expansion") targets Dataflow, but its condition also
reads target.handlesResources, which only exists on Asset subclasses,
not on a plain Dataflow. So for any XML dataflow the lookup raises
AttributeError, and the broad `except Exception: return False` in
Threat.apply swallows it into a False. The rule has been silently dead,
never firing on the flows it's meant to flag.

Per @raphaelahrens' suggestion in the issue, I dropped the `and
target.handlesResources is False` clause so the condition is just
`any(d.format == 'XML' for d in target.data)`, which matches the
threat's description. I also removed the `handlesResources = False` line
in test_DO04, since that assignment was injecting the missing attribute
and masking the bug.

Tested on Python 3.13: the issue repro now flags DO04, `pytest -k DO04`
passes, and the full suite (243 tests) is green.

Heads-up: #328 reworks the threat library into Python classes, so if it
lands first this'll need a small rebase. Happy to redo it whichever way
is easier. Thanks for taking a look.

Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
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