From bf2f8c3705e752c9f4d031268b8a6561fe674820 Mon Sep 17 00:00:00 2001
From: Tim McCormack <tmccormack@edx.org>
Date: Tue, 15 Apr 2025 14:16:56 -0400
Subject: [PATCH] fix: Don't let local codejail exec pollute darklaunched
 remote globals

We were running local exec before making the copy of globals_dict for
remote_exec, so remote exec has been getting a polluted version of the
globals.
---
 xmodule/capa/safe_exec/safe_exec.py            |  7 +++++--
 xmodule/capa/safe_exec/tests/test_safe_exec.py | 17 +++++++++++++++++
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/xmodule/capa/safe_exec/safe_exec.py b/xmodule/capa/safe_exec/safe_exec.py
index 29661f573b..b3a1b888f6 100644
--- a/xmodule/capa/safe_exec/safe_exec.py
+++ b/xmodule/capa/safe_exec/safe_exec.py
@@ -180,6 +180,11 @@ def safe_exec(
 
     else:
 
+        # Create a copy so the originals are not modified as part of this call.
+        # This has to happen before local exec is run, since globals are modified
+        # as a side effect.
+        darklaunch_globals = copy.deepcopy(globals_dict)
+
         # Decide which code executor to use.
         if unsafely:
             exec_fn = codejail_not_safe_exec
@@ -215,8 +220,6 @@ def safe_exec(
         # when in darklaunch mode.
         if is_codejail_in_darklaunch():
             try:
-                # Create a copy so the originals are not modified as part of this call.
-                darklaunch_globals = copy.deepcopy(globals_dict)
                 data = {
                     "code": code_prolog + LAZY_IMPORTS + code,
                     "globals_dict": darklaunch_globals,
diff --git a/xmodule/capa/safe_exec/tests/test_safe_exec.py b/xmodule/capa/safe_exec/tests/test_safe_exec.py
index 7bba31a34d..a45c5911a2 100644
--- a/xmodule/capa/safe_exec/tests/test_safe_exec.py
+++ b/xmodule/capa/safe_exec/tests/test_safe_exec.py
@@ -1,6 +1,7 @@
 """Test safe_exec.py"""
 
 
+import copy
 import hashlib
 import os
 import os.path
@@ -216,10 +217,21 @@ class TestCodeJailDarkLaunch(unittest.TestCase):
         # Both will attempt to read and write the 'overwrite' key.
         globals_dict = {'overwrite': 'original'}
 
+        local_globals = None
+        remote_globals = None
+
         def local_exec(code, globals_dict, **kwargs):
+            # Preserve what local exec saw
+            nonlocal local_globals
+            local_globals = copy.deepcopy(globals_dict)
+
             globals_dict['overwrite'] = 'mock local'
 
         def remote_exec(data):
+            # Preserve what remote exec saw
+            nonlocal remote_globals
+            remote_globals = copy.deepcopy(data['globals_dict'])
+
             data['globals_dict']['overwrite'] = 'mock remote'
             return (None, None)
 
@@ -245,6 +257,11 @@ class TestCodeJailDarkLaunch(unittest.TestCase):
         )
         assert results['raised'] is None
 
+        # Both arms should have only seen the original globals object, untouched
+        # by the other arm.
+        assert local_globals == {'overwrite': 'original'}
+        assert remote_globals == {'overwrite': 'original'}
+
     def test_remote_runs_even_if_local_raises(self):
         """Test that remote exec runs even if local raises."""
         def local_exec(code, globals_dict, **kwargs):