Make the sandbox name pattern per-supervisor, not a mutable global

pyisolate/supervisor.py

@@ -33,7 +33,12 @@
 
 logger = logging.getLogger(__name__)
 
-# Allowed sandbox name pattern: alphanumerics, hyphen, underscore
+# Allowed sandbox name pattern: alphanumerics, hyphen, underscore.
+#
+# ``NAME_PATTERN`` is the process-wide default a Supervisor falls back to when
+# it was not given its own. Prefer ``Supervisor(name_pattern=...)``: a module
+# global cannot express two supervisors with different rules, and rebinding it
+# at runtime races with concurrent spawns.
 DEFAULT_NAME_PATTERN = re.compile(r"^[A-Za-z0-9_-]+$")
 NAME_PATTERN = DEFAULT_NAME_PATTERN
 
@@ -187,7 +192,12 @@ def __init__(
         self,
         warm_pool: int = 0,
         rollout_mode: str = "dev",
+        name_pattern: Optional[re.Pattern[str]] = None,
     ):
+        # None means "use whatever the module-level default is at spawn time",
+        # which keeps the documented global override working for the
+        # process-wide supervisor without this instance owning that decision.
+        self._name_pattern = name_pattern
         self._sandboxes: Dict[str, SandboxThread] = {}
         # Process-backed sandboxes live in a parallel registry: they are not
         # SandboxThread instances, so the watchdog/warm-pool/cgroup machinery
@@ -282,6 +292,11 @@ def _recover_state(self) -> None:
         cgroup.cleanup_orphans(set())
         recovery.cleanup_temp_orphans(set())
 
+    @property
+    def name_pattern(self) -> "re.Pattern[str]":
+        """Pattern sandbox names must match, per supervisor."""
+        return self._name_pattern if self._name_pattern is not None else NAME_PATTERN
+
     def register_alert_handler(self, callback) -> None:
         """Subscribe to policy violation alerts."""
         self._alerts.register(callback)
@@ -311,7 +326,6 @@ def spawn(
         ``backend="microvm"`` are explicit boundary modes; they are reserved
         API choices and fail closed until native launchers are available.
         """
-        global NAME_PATTERN
         backend = _normalize_backend(backend)
         if backend == "microvm":
             # Dedicated fail-closed path: probe for a real VMM/KVM boundary and
@@ -323,7 +337,7 @@ def spawn(
             raise ValueError("Sandbox name must be non-empty string")
         if len(name) > 64:
             raise ValueError("Sandbox name too long")
-        pattern = NAME_PATTERN
+        pattern = self.name_pattern
         if pattern.fullmatch(name) is None:
             raise ValueError("Sandbox name contains invalid characters")
         self._cleanup()
@@ -454,9 +468,6 @@ def spawn(
                 raise
         # Remove references to any terminated sandboxes
         self._cleanup()
-        # Reset any temporary overrides of the name validation pattern to avoid
-        # leaking state across sandboxes.
-        NAME_PATTERN = DEFAULT_NAME_PATTERN
         return Sandbox(thread, self)
 
     def _apply_kernel_policy(self, cg_path: Any, policy: Any) -> None:

tests/test_metrics.py

@@ -91,13 +91,16 @@ def test_export_sandbox_order_is_stable():
             sb.close()
 
 
-def test_export_sanitizes_sandbox_name():
+def test_export_sanitizes_sandbox_name(monkeypatch):
     name = 'weird "sand\\box\nname'
     import re
 
     import pyisolate.supervisor as supervisor
 
-    supervisor.NAME_PATTERN = re.compile(r".+", re.DOTALL)
+    # monkeypatch restores the default when the test ends. This used to rely on
+    # Supervisor.spawn resetting the global itself, which meant production code
+    # carried a test affordance and raced with concurrent spawns.
+    monkeypatch.setattr(supervisor, "NAME_PATTERN", re.compile(r".+", re.DOTALL))
     sb = iso.spawn(name)
     try:
         sb.exec("post(1)")

tests/test_supervisor.py

@@ -421,3 +421,62 @@ def fail_update(*_args, **_kwargs):
         assert sup_replay._tenant_usage.get("acme", 0) == 0
     finally:
         sup_replay.shutdown()
+
+
+# -- sandbox name validation ------------------------------------------------
+
+
+def test_supervisor_accepts_a_custom_name_pattern():
+    import re
+
+    sup = iso.Supervisor(name_pattern=re.compile(r"^ok\.[a-z]+$"))
+    try:
+        sb = sup.spawn("ok.alpha")
+        sb.close()
+        with pytest.raises(ValueError, match="invalid characters"):
+            sup.spawn("nope!")
+    finally:
+        sup.shutdown()
+
+
+def test_name_pattern_is_per_supervisor_not_global():
+    import re
+
+    from pyisolate import supervisor as supervisor_mod
+
+    permissive = iso.Supervisor(name_pattern=re.compile(r".+"))
+    strict = iso.Supervisor()
+    try:
+        sb = permissive.spawn("has.dots.and.stuff")
+        sb.close()
+        # A second supervisor keeps the default rules: one supervisor's naming
+        # policy must not leak into another's.
+        with pytest.raises(ValueError, match="invalid characters"):
+            strict.spawn("has.dots.and.stuff")
+        assert supervisor_mod.NAME_PATTERN is supervisor_mod.DEFAULT_NAME_PATTERN
+    finally:
+        permissive.shutdown()
+        strict.shutdown()
+
+
+def test_spawn_does_not_rebind_the_module_level_pattern(monkeypatch):
+    # Regression: Supervisor.spawn used to `global NAME_PATTERN` and reset it to
+    # the default on every successful spawn, so a caller that installed a custom
+    # pattern silently lost it after one sandbox -- and two threads spawning
+    # concurrently raced over the value.
+    import re
+
+    from pyisolate import supervisor as supervisor_mod
+
+    custom = re.compile(r".+", re.DOTALL)
+    monkeypatch.setattr(supervisor_mod, "NAME_PATTERN", custom)
+    sup = iso.Supervisor()
+    try:
+        sb = sup.spawn("weird.name with spaces")
+        sb.close()
+        assert supervisor_mod.NAME_PATTERN is custom
+        # Still honored on the next spawn rather than reset behind the caller.
+        sb2 = sup.spawn("another weird one")
+        sb2.close()
+    finally:
+        sup.shutdown()