`repyportability` should not override Python builtins

[aaaaalbert]

repyportability exists so that Python code can make use of Repy modules. Therefore, the portability module must make the Repy API functions available in the global context. However, it is not necessary at this point to ensure performance isolation (i.e. non nannying) nor namespace separation (so that what would usually be sandboxed code cannot access the internals of the sandbox).

The current repyportability module does the former (see code), but has issues with the latter (see more code). In particular, lines 204-211 seem to allow all the builtins, but the program flow in safe.py indicates that builtins are replaced preferrably even if they are considered OK.

Patching this behavior should be as simple as

@@ -205,6 +205,8 @@ def initialize_safe_module():
     for builtin_type in dir(__builtins__):
       if builtin_type not in safe._BUILTIN_OK:
         safe._BUILTIN_OK.append(builtin_type)
+      if builtin_type in safe._BUILTIN_REPLACE:
+        del safe._BUILTIN_REPLACE[builtin_type]

[aaaaalbert]

On a related note, their unit tests indicate that RepyV2 and SeattleLibV2 run fine with the original type builtin.

[aaaaalbert]

It should be noted that from repyportability import * does not cause problems with the builtins; not even add_dy_support(locals()) (which looks like a hot contender for its mere existence, see #36). Problems only start when you create and then evaluate a VirtualNamespace, or dy_import a Repy module (which also uses VirtualNamespaces behind the scenes).

For your copy-and-paste pleasure:

type # The original

from repyportability import *
type # Still the original

add_dy_support(locals())
type # Original, unchanged

vn = createvirtualnamespace("pass", "NOP namespace")
type # Still unchanged

vn.evaluate({})
type # Now it is <function safe_type at 0x....>

[aaaaalbert]

Funnily enough, the patch outlined above needs to go to a different line because of duplication/dead code in repyportability:

186a187,188
>   if builtin_type in safe._BUILTIN_REPLACE:
>     del safe._BUILTIN_REPLACE[builtin_type]

With this patch, the type override code example still shows <type 'type'> after the VirtualNamespace has been evaluated.

[JustinCappos]

I'd prefer the caller specifically indicate how they want builtins handled. What you propose won't handle getattr, isinstance, etc. and may be called incorrectly by accident.

On Wed, Dec 14, 2016 at 6:53 AM, aaaaalbert notifications@github.com wrote:

repyportability exists so that Python code can make use of Repy modules. Therefore, the portability module must make the Repy API functions available in the global context. However, it is not necessary at this point to ensure performance isolation (i.e. non nannying) nor namespace separation (so that what would usually be sandboxed code cannot access the internals of the sandbox).

The current repyportability module does the former (see code https://github.com/SeattleTestbed/portability/blob/f3a798fb1400e92f865e29b9536dd4633177235a/repyportability.py#L50-L79), but has issues with the latter (see more code https://github.com/SeattleTestbed/portability/blob/f3a798fb1400e92f865e29b9536dd4633177235a/repyportability.py#L189-L214). In particular, lines 204-211 https://github.com/SeattleTestbed/portability/blob/f3a798fb1400e92f865e29b9536dd4633177235a/repyportability.py#L204-L211 seem to allow all the builtins, but the program flow in safe.py indicates https://github.com/SeattleTestbed/repy_v2/blob/3cc932995c89e291e91cb917a52f10f6ccf89820/safe.py#L536-L549 that builtins are replaced preferrably even if they are considered OK.

Patching this behavior should be as simple as

@@ -205,6 +205,8 @@ def initialize_safe_module(): for builtin_type in dir(builtins): if builtin_type not in safe._BUILTIN_OK: safe._BUILTIN_OK.append(builtin_type)+ if builtin_type in safe._BUILTIN_REPLACE:+ del safe._BUILTIN_REPLACE[builtin_type]

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/SeattleTestbed/portability/issues/35, or mute the thread https://github.com/notifications/unsubscribe-auth/AA0XDz8VKow_EvIoYe88fSKNjX_PdhVHks5rH9i3gaJpZM4LM1nN .

[lukpueh]

Ignoring the implementation details, isn't it possible to have different namespaces/contexts for each of:

• sandboxed code (A)
• sandbox code (B)
• any other code (C)

In C my namespace is what I expect it to be (e.g. pure Python, or Django, or whatever). If I call into sandbox code then I only get access to B and if that sandbox code further calls into (runs) sandboxed code then it will only have access to A. When I return eventually I'm back to C.

[lukpueh]

I confirm that above patch resolves the wrong type issue when running python manager.py runserver for Django 1.9/1.10. Furthermore I was able to successfully perform the tests as described in https://github.com/SeattleTestbed/custominstallerbuilder/pull/21#issue-193904809.