Skip to content

fix: make Data.__hash__ safe during pydantic deepcopy - #378

Merged
izar merged 3 commits into
OWASP:masterfrom
markthomaschurch:fix/data-hash-deepcopy
Nov 11, 2026
Merged

izar merged 3 commits into
OWASP:masterfrom
markthomaschurch:fix/data-hash-deepcopy

Conversation

@markthomaschurch

Copy link
Copy Markdown
Contributor

Fixes #377

Summary

TM.report() raised AttributeError: 'Data' object has no attribute 'name' for any model where one Data object is carried by more than one Dataflow.

The cause is a reference cycle, Data -> carriedBy -> Dataflow -> data (DataSet) -> Data. Pydantic v2's BaseModel.__deepcopy__ registers the new instance in the memo before copying its fields, so the DataSet copy hashed a Data with no attributes yet.

Changes

  • pytm/data.py: Data.__hash__ now reads its fields with getattr(..., None), so hashing a partially built instance cannot raise.
  • tests/test_pydantic_models.py: two regression tests:
    • test_deepcopy_with_dataflow_cycle: calls copy.deepcopy on a Data that is part of a carriedBy ↔ Dataflow.data cycle.
    • test_report_with_data_carried_by_multiple_flows: end-to-end test of check() → resolve() → report() with one Data on two flows.

Testing

  • Both new tests fail on master with the AttributeError and pass with this change.
  • Full suite: pytest tests → 263 passed.

Known limitation

In the copied graph, the copied Data is stored in the copied DataSet under the hash it had while its fields were empty. Iterating the set works, but in checks on the copy return False. Report rendering doesn't depend on this; details are in the linked issue.

🤖 Generated with Claude Code

TM.report() deep-copies model elements. When a Data object is carried
by more than one Dataflow there is a reference cycle
(Data -> carriedBy -> Dataflow -> data (DataSet) -> Data). Pydantic v2's
BaseModel.__deepcopy__ registers the new instance in the memo before its
fields are copied, so rebuilding the DataSet hashes a Data that has no
attributes yet and raises:

    AttributeError: 'Data' object has no attribute 'name'

Read the hashed fields with getattr() so hashing a partially built
instance does not raise, and add regression tests for deepcopy and
TM.report().

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@izar

izar commented Nov 11, 2026

Copy link
Copy Markdown
Collaborator

Failing under Python 3.12:

__________________ TestData.test_deepcopy_with_dataflow_cycle __________________
self = <tests.test_pydantic_models.TestData object at 0x7232cbd26480>
def test_deepcopy_with_dataflow_cycle(self):
# Data -> carriedBy -> Dataflow -> data (DataSet) -> same Data.
# Pydantic's deepcopy memoizes the new Data before its fields are
# copied, so the DataSet copy hashes a Data with no attributes yet.
token = Data("token", classification=Classification.SECRET)
flow = Dataflow(Actor("user"), Server("web"), "login", data=token)
token.carriedBy = [flow]

    copied = copy.deepcopy(token)

    assert copied is not token
    assert copied.name == "token"
    assert copied.classification == Classification.SECRET
    copied_flow = copied.carriedBy[0]
    assert copied_flow is not flow
  assert any(d is copied for d in copied_flow.data)

E assert False
E + where False = any(<generator object TestData.test_deepcopy_with_dataflow_cycle.. at 0x7232c9849ff0>)
tests/test_pydantic_models.py:357: AssertionError
=========================== short test summary info ============================
FAILED tests/test_pydantic_models.py::TestData::test_deepcopy_with_dataflow_cycle - assert False

  • where False = any(<generator object TestData.test_deepcopy_with_dataflow_cycle.. at 0x7232c9849ff0>)
    ======================== 1 failed, 262 passed in 3.71s =========================

izar
izar previously requested changes Nov 11, 2026

@izar izar left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Needs fix.

test_deepcopy_with_dataflow_cycle checked that the copied Dataflow's
data is the very same copy of the Data. That holds only from pydantic
2.14, whose __deepcopy__ memoizes the new instance before copying its
fields (which is also what triggers the hashing bug). With the
pydantic 2.12.5 in poetry.lock, the cycle produces a second copy, so
the test failed in CI.

Check instead that the flow carries a copy of the token, not the
original. The test still fails without the fix on pydantic 2.14 and
passes on 2.12, 2.13 and 2.14.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@markthomaschurch

Copy link
Copy Markdown
Contributor Author

Thanks for catching this. The failure comes from the pydantic version rather than Python 3.12: CI fails on every Python version because poetry.lock pins pydantic 2.12.5, and I had tested with 2.14.3.

The bug itself only happens from pydantic 2.14, whose BaseModel.__deepcopy__ puts the new instance in the memo before copying its fields. pyproject.toml allows pydantic>=2.10.0,<3.0.0, so a pip install of pytm today gets 2.14 and TM.report() raises the AttributeError. With 2.12 and 2.13 the cycle makes a second copy of the Data, and the test's d is copied check failed on that difference.

f9f6b91 changes the test to check that the flow carries a copy of the token rather than the original, which holds on every version. The test suite passes with pydantic 2.12.5, 2.13.5 and 2.14.3, and the regression test still fails without the fix on 2.14.3.

@izar
izar dismissed their stale review November 11, 2026 19:01

Fixed.

@izar
izar merged commit 00015d9 into OWASP:master Nov 11, 2026
5 checks passed
@markthomaschurch
markthomaschurch deleted the fix/data-hash-deepcopy branch November 12, 2026 15:06
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.

TM.report() raises AttributeError: 'Data' object has no attribute 'name' when Data is carried by multiple dataflows

2 participants