Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,18 @@ Changes
providing an ``__import__`` implementation hands the security boundary to the
import policy of the calling application.

- Every ast node now has an explicit ``visit_<node>`` method in
``RestrictingNodeTransformer``, following "Explicit is better than
implicit." (PEP 20). The nodes which were only denied implicitly by
``generic_visit`` (``AnnAssign``, the ``match`` statement and its patterns,
the type parameter and ``type`` statement nodes of Python 3.12+,
``FunctionType`` and ``TypeIgnore``) are now denied explicitly, and the
docstrings of all denying methods explain the reason and the security
implications. They are still denied, but no longer emit the warning
"... statement is not known to RestrictedPython". A new test fails for
every ast node of the running Python version without a ``visit_<node>``
method.


8.5 (2026-08-19)
----------------
Expand Down
108 changes: 56 additions & 52 deletions docs/contributing/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ For all commits, use ``tox`` to run tests and lint, and build the docs, before p

.. _new_python_version:

Preperations for a new Python version
Preparations for a new Python version
+++++++++++++++++++++++++++++++++++++

RestrictedPython should be updated for each new version of Python.
Expand All @@ -49,19 +49,11 @@ To do so:
* For each new **AST Node** or functionality:

* Add tests to ``/tests/``.
* Add a ``visit_<AST Node>`` to ``/src/RestrictedPython/transformer.py``.
* Add a ``visit_<AST Node>`` method to ``/src/RestrictedPython/transformer.py`` which either allows or denies the node, see :ref:`explicit_visitors`.
The test ``tests/transformer/test_explicit_deny.py`` fails as long as a node of the running Python version has no ``visit_<AST Node>`` method.

If the new AST Node should be enabled by default, with or without any modification, please add a ``visit_<AST Node>`` method such as the following:

.. code-block:: python

def visit_<AST Node>(self, node):
"""Allow `<AST Node>` expressions."""
... # modifications
return self.node_contents_visit(node)

All AST Nodes without an explicit ``visit_<AST Node>`` method, are denied by default.
So the usage of this expression and functionality is not allowed.
* Check existing nodes with new fields, too.
A new feature does not always come with a new node: lazy imports (:pep:`810`) only added the field ``is_lazy`` to ``Import`` and ``ImportFrom``, see :doc:`changes_from314to315`.

* Check the documentation for `inspect <https://docs.python.org/3/library/inspect.html>`_ and adjust the ``transformer.py:INSPECT_ATTRIBUTES`` list.
* Add a corresponding changelog entry.
Expand Down Expand Up @@ -173,59 +165,71 @@ With RestrictedPython 4.0 an API compatible rewrite has happened, which supports

Tests and documentation are distributed within released packages.

.. todo::
.. _explicit_visitors:

Every AST node has an explicit ``visit_<AST Node>`` method
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

RestrictedPython follows the Zen of Python (:pep:`20`):

Resolve discussion about how RestrictedPython should be treat new expressions / ``ast.Nodes``.
This belongs to :ref:`new_python_version`.
.. code-block:: pycon
:emphasize-lines: 5

**Option 1 - reduce maintenance burden (preferred by icemac)**
>>> import this
The Zen of Python, by Tim Peters

Beautiful is better than ugly.
Explicit is better than implicit.
Simple is better than complex.
Complex is better than complicated.
Flat is better than nested.
Sparse is better than dense.
Readability counts.
Special cases aren't special enough to break the rules.
Although practicality beats purity.
Errors should never pass silently.
Unless explicitly silenced.
In the face of ambiguity, refuse the temptation to guess.
There should be one-- and preferably only one --obvious way to do it.
Although that way may not be obvious at first unless you're Dutch.
Now is better than never.
Although never is often better than *right* now.
If the implementation is hard to explain, it's a bad idea.
If the implementation is easy to explain, it may be a good idea.
Namespaces are one honking great idea -- let's do more of those!

All AST Nodes without an explicit ``visit_<AST Node>`` method, are denied by default.
So the usage of this expression and functionality is not allowed.
Therefore **every AST node has an explicit** ``visit_<AST Node>`` **method** in ``RestrictingNodeTransformer``, also the denied ones.

*This is currently the promoted version.*
``generic_visit`` denies every node without a ``visit_<AST Node>`` method.
It is the safety net for nodes of a new Python version which nobody has reviewed yet, not the place where the decision about a known language feature is recorded.
A node which is denied only by ``generic_visit`` looks the same as a node nobody has looked at.
An explicit method records that somebody has reviewed the node and why it is denied, at the place where the next reviewer looks for it.
In security relevant code, that knowledge is worth more than the lines it takes.

**Option 2 - be as explicit as possible (preferred by loechel)**
A denied node gets a method like the following:

If the new AST Node should be disabled by default, add a ``visit_<AST Node>`` method such as the following:
.. code-block:: python

.. code-block:: python
def visit_<AST Node>(self, node):
"""Deny `<AST Node>` (<example>, <PEP>).

def visit_<AST Node>(self, node):
"""`<AST Node>` expression currently not allowed."""
self.not_allowed(node)
<Why it is denied: which guard or check it would bypass, which
security implications it has, or why it has not been reviewed yet.>
"""
self.not_allowed(node)

Please note, that for all AST Nodes without an explicit ``visit_<AST Node>`` method, a default applies which denies the usage of this expression and functionality.
As we try to be **as explicit as possible**, all language features should have a corresponding ``visit_<AST Node>``.
The docstring states the reason, not only the decision:

That follows the Zen of Python:
* Which guard (``_getattr_``, ``_getitem_``, ``_getiter_``, ``_write_``, ...) or check (``check_name``, import policy, ...) the node would bypass.
* Which security implications allowing it would have.
* If no concrete bypass is known, say so and say that a security review is still missing.

.. code-block:: pycon
:emphasize-lines: 5
Where the reason is a behavior of the interpreter, add a test which allows the node and shows the bypass (see ``tests/transformer/test_explicit_deny.py``).
If the interpreter changes, the test fails and the docstring gets reviewed.

>>> import this
The Zen of Python, by Tim Peters
An allowed node gets a method which calls ``self.node_contents_visit(node)``, with the modifications needed to guard it.

Beautiful is better than ugly.
Explicit is better than implicit.
Simple is better than complex.
Complex is better than complicated.
Flat is better than nested.
Sparse is better than dense.
Readability counts.
Special cases aren't special enough to break the rules.
Although practicality beats purity.
Errors should never pass silently.
Unless explicitly silenced.
In the face of ambiguity, refuse the temptation to guess.
There should be one-- and preferably only one --obvious way to do it.
Although that way may not be obvious at first unless you're Dutch.
Now is better than never.
Although never is often better than *right* now.
If the implementation is hard to explain, it's a bad idea.
If the implementation is easy to explain, it may be a good idea.
Namespaces are one honking great idea -- let's do more of those!
The test ``tests/transformer/test_explicit_deny.py`` enforces the rule: it fails as long as a node of the running Python version has no ``visit_<AST Node>`` method.


Technical Backgrounds - Links to External Documentation
Expand Down
Loading
Loading