Skip to content

fix: flow defaults no longer overwrite values the model set - #392

Open
markthomaschurch wants to merge 2 commits into
OWASP:masterfrom
markthomaschurch:fix/flow-defaults-fill-only
Open

markthomaschurch wants to merge 2 commits into
OWASP:masterfrom
markthomaschurch:fix/flow-defaults-fill-only

Conversation

@markthomaschurch

Copy link
Copy Markdown
Contributor

Fixes #391

The dataflow defaults applied by TM.process() now fill in only the attributes a model left unset, instead of overwriting the ones it set, as they did before the move to pydantic (#320).

  • pytm/base.py: new set_default(model, attr, value), which sets the attribute unless it's in model.model_fields_set (given to the constructor or assigned since), ignoring values the attribute doesn't accept. A value it fills in counts as set, so the first default wins, as before Refactor models to Pydantic #320.
  • pytm/element.py, pytm/base.py (Controls), pytm/data.py: the three identical _safeset() methods use it.
  • tests/test_flow_defaults.py (new): explicit values are kept on forward and response flows, on data and for levels, including explicit values equal to the type's default; unset values still come from the sink/source; a flow declared as encrypted HTTPS gets no DE01/CR08 finding. The seven tests with explicit values fail on master.

pytest tests → 279 passed (pydantic 2.12.5 and 2.14.3).

🤖 Generated with Claude Code

During process(), _apply_defaults() copies values from a dataflow's
source and sink onto the dataflow and its data: protocol and ports,
controls.isEncrypted, authenticatesDestination and
checksDestinationRevocation, Data.isSourceEncryptedAtRest and
isDestEncryptedAtRest, and levels. They are meant to fill in what the
model left out. Before the move to pydantic (cfc38f8) attributes could
only be set once, so _safeset() could only fill gaps; since then it is a
plain setattr, and every default replaces the model's value. A flow
declared as encrypted HTTPS on port 443 was evaluated as unencrypted with
no protocol (false DE01, CR08 and similar findings), data explicitly not
encrypted at rest was flipped to encrypted, and reports and diagrams
showed the overwritten values. The example model's own protocols and
ports (HTTP, MySQL, 80, 3306, ...) were replaced by "" and -1.

Fill in only attributes that aren't set, using pydantic's
model_fields_set (constructor arguments and assignments). A value filled
in by a default counts as set, so the first default wins, as before the
move to pydantic. The three identical _safeset() methods (Element,
Controls, Data) now share one helper, base.set_default().

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

This branch has not been deployed

No deployments
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.

Dataflow defaults overwrite values the model set explicitly

1 participant