Skip to content

fix: resolve bare names with the same precedence in the restricted and trusted evaluators - #695

Open
fgmacedo wants to merge 5 commits into
mainfrom
fgmacedo/uml-w2d_names
Open

fgmacedo wants to merge 5 commits into
mainfrom
fgmacedo/uml-w2d_names

Conversation

@fgmacedo

@fgmacedo fgmacedo commented Oct 14, 2026 •

Copy link
Copy Markdown
Owner

Why

With a datamodel item value = 1 and send("go", value=5), cond: "value == 5" was True when the document was loaded with the default restricted evaluator and False with trusted=True (probe from the #692 review, bug B8). The restricted evaluator looked a bare name up in the event kwargs first; the trusted one let model.__dict__ win and could not see properties, class attributes or slots at all. The docs did not state the precedence; the maintainer chose "datamodel/model wins".

Refs #692

What changes

  • A bare name resolves to a datamodel variable or a public model attribute (instance attribute, property, class attribute, slot) before a same-named send() keyword argument, under both trusted=False and trusted=True.
  • The payload stays reachable as _event.data.<name>, and a payload key still resolves bare when the model has no attribute of that name.
  • Restricted mode: only the lookup order changed; private, dunder and engine-protected names still never resolve off the model, and engine objects are still withheld from the kwargs.
  • Trusted mode now also resolves properties, class attributes and slots, except methods and non-instance attributes named like a Python builtin (len, type, ...), which keep their previous behavior.
  • docs/io/formats.md (Guards) documents the precedence with a doctest.
  • docs/releases/3.3.0.md: a bug-fix entry and three upgrade bullets (restricted precedence, catch-all __getattr__ models, trusted mode now resolving properties, class attributes and slots).

Evidence

  • tests/io/test_name_resolution.py: every case runs under both evaluators (restricted/trusted fixture): datamodel-only, kwarg-only, model-only (instance attribute, property, class attribute, slots), datamodel+kwarg, model+kwarg for each model kind, a value written by assign then shadowing the payload, _event.data reaching the payload, plus trusted-only checks for methods and builtin-named class attributes. Against the base commit d5282e0, 13 of these fail.
  • Full suite after rebasing onto main (with fix: enter history states by default without running the incoming effect twice #693, feat: accept x- extension keys in native documents and run the OMG PSSM 1.0 test suite #697, fix: report errors raised while an invocation processes its done.invoke #698), through the pre-commit hook (--cov-fail-under=100, branch): 3001 passed, 26 skipped, 126 xfailed; ruff, mypy, pyright clean.
  • W3C SCXML + tests/io/test_security.py: 587 passed, 42 xfailed before and after, same xfail list.
  • Review probe (cond: "value == 5", datamodel value: 1, send("go", value=5)): before, s1 restricted vs s2 trusted; after, s2 in both modes.
  • Adversarial review, three rounds by fresh agents with no shared context:
    1. Material: trusted only read model.__dict__, so properties, class attributes and slots resolved differently between modes. Fixed (_TrustedNamespace), with the model-kind matrix added.
    2. Material for trusted users: class attributes, properties or a catch-all __getattr__ could shadow builtins such as len. Fixed and tested; also verified eval honors __getitem__ on a dict subclass on Python 3.10 to 3.14.
    3. Nothing material; the docs paragraph was extended to name the builtin exception.

Not verified

  • Known divergence kept on purpose: model methods are not resolved by bare name under trusted=True. A restricted guard calls a bare method name (guard dialect), so a method-vs-kwarg collision and bare-method guards still differ between modes. Follow-up.
  • Under trusted=True, a non-instance model attribute named like a builtin (id, type, ...) resolves to the payload or the builtin, while restricted mode resolves it to the model.
  • Behavior change for restricted documents: a model with a catch-all __getattr__ (proxy, Mock-like) now shadows every bare payload name; _event.data.<name> still reaches it.
  • A walrus assignment inside a trusted guard to a name that is a model instance attribute no longer reads back its own value.

@fgmacedo fgmacedo added this to the 3.3.0 milestone Oct 14, 2026
@codecov

codecov Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (ec5d817) to head (cc0daf8).

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #695   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           53        53           
  Lines         5900      5918   +18     
  Branches       939       942    +3     
=========================================
+ Hits          5900      5918   +18     
Flag Coverage Δ
unittests 100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

… evaluators

Refs #692

Signed-off-by: Fernando Macedo <fgmacedo@gmail.com>
…restricted mode

The restricted evaluator looked a bare name up in the event keyword arguments
before the model, while the trusted evaluator let the model win. A document
with a datamodel variable named like a send() keyword argument took a
different transition depending on trusted=. Both now resolve the datamodel
(or bound model) first; the payload stays reachable as _event.data.<name>.

Refs #692

Signed-off-by: Fernando Macedo <fgmacedo@gmail.com>
…e in trusted mode

The trusted evaluator only read the model's instance __dict__, so a property,
a class attribute or a slot resolved in restricted mode but fell through to
the event payload (or NameError) under trusted=True. It now uses the same
boundary as the restricted evaluator, model first, then the call kwargs.
Model methods stay out of the trusted bare-name lookup: the restricted guard
dialect calls them, which eval cannot reproduce per name.

Refs #692

Signed-off-by: Fernando Macedo <fgmacedo@gmail.com>
…expressions

Resolving non-instance model attributes by bare name let a class attribute,
a property or a catch-all __getattr__ shadow builtins such as len or type
under trusted=True, which broke expressions that worked before.

Refs #692

Signed-off-by: Fernando Macedo <fgmacedo@gmail.com>
@fgmacedo
fgmacedo force-pushed the fgmacedo/uml-w2d_names branch from 516608a to 7111277 Compare October 16, 2026 22:24
…sions

Signed-off-by: Fernando Macedo <fgmacedo@gmail.com>
@fgmacedo
fgmacedo force-pushed the fgmacedo/uml-w2d_names branch from 7111277 to cc0daf8 Compare October 16, 2026 22:26
@sonarqubecloud

Copy link
Copy Markdown

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.

1 participant