patches/Restricted: review RestrictionMutator patch

Document (from my guess of why we have this behavior) why we have such a
patch, also cover our expected behavior in a minimal test.

Keep refusing methods ending in __roles__, so that we can run original
AccessControl test suite, also because it seems safer and because
allowing or not to define methods ending in __roles__ should not affect
our cases.

bt5/erp5_core_test/TestTemplateItem/portal_components/test.erp5.testRestrictedPythonSecurity.py

@@ -402,6 +402,20 @@ class TestRestrictedPythonSecurity(ERP5TypeTestCase):
         expected=[("a", 1), ("b", 2)]
     )
 
+  def test_lax_name(self):
+    self.createAndRunScript(
+        textwrap.dedent('''\
+          def _function():
+            pass
+          class SimpleObject:
+            def __init__(self):
+              self.attribute = 1
+            def _method(self):
+              _variable = 1
+          return SimpleObject().attribute
+          '''),
+        expected=1
+    )
 
 def test_suite():
   suite = unittest.TestSuite()

product/ERP5Type/patches/Restricted.py

@@ -16,11 +16,25 @@ import sys
 import types
 
 from RestrictedPython.RestrictionMutator import RestrictionMutator
+_MARKER = []
+def checkNameLax(self, node, name=_MARKER):
+  """Verifies that a name being assigned is safe.
 
-# Unsafe attributes on protected objects are already disallowed at execution
-# and we don't want to maintain a duplicated list of exceptions.
-RestrictionMutator.checkName = RestrictionMutator.checkAttrName = \
-    lambda *args, **kw: None
+  In ERP5 we are much more lax that than in Zope's original restricted
+  python and allow to using names starting with _, because we rely on
+  runtime checks to prevent access to forbidden attributes from objects.
+
+  We don't allow defining attributes ending with __roles__ though.
+  """
+  if name is _MARKER:
+    # we use same implementation for checkName and checkAttrName which access
+    # the name in different ways ( see RestrictionMutator 3.6.0 )
+    name = node.attrname
+  if name.endswith('__roles__'):
+    self.error(node, '"%s" is an invalid variable name because '
+                     'it ends with "__roles__".' % name)
+
+RestrictionMutator.checkName = RestrictionMutator.checkAttrName = checkNameLax
 
 
 from Acquisition import aq_acquire