Reproduction steps:
- Extract a clean station-v0.71 tarball, sys.path.insert(0, "workbench").
- from module_sandbox import install_restrictions, scoped_handler, SandboxViolation
- install_restrictions({}, {"network": ["api.example.com"]}, slug="testmod")
- Idiom A -- a handler that spawns
threading.Thread(target=worker), where
worker() calls urllib.request.urlopen("http://127.0.0.1:1/"). Wrap the
handler with scoped_handler("testmod", handler) and call it.
- Idiom B -- a handler that instead spawns a `class AttackThread(threading.Thread):
def run(self): <same urlopen call> and does AttackThread().start()`.
Wrap with scoped_handler the same way and call it.
Expected: both idioms are "a module handler's thread doing network I/O" and
should be treated identically by the sandbox -- either both raise
SandboxViolation, or neither does.
Actual:
threading.Thread(target=worker) -> ('BLOCKED', "module 'testmod' tried urllib.request.urlopen to '127.0.0.1'")
class AttackThread(Thread): def run() -> ('NOT_BLOCKED_network_error', "URLError(ConnectionRefusedError(111, 'Connection refused'))")
Idiom B's urlopen call reaches the real socket layer -- it only fails with
ConnectionRefused because nothing is listening on the test port; against a
real off-allowlist host it would succeed, identical to the sandbox not
existing at all.
Root cause: workbench/module_sandbox.py, _install_thread_context_propagation()
(~line 145-176). The patch replaces threading.Thread.start andthreading.Thread.run at the class level:
def _start(self):
if _ACTIVE_SANDBOX.get() is not None and getattr(self, "_rc_sandbox_ctx", None) is None:
self._rc_sandbox_ctx = contextvars.copy_context()
return _real_start(self)
def _run(self):
ctx = getattr(self, "_rc_sandbox_ctx", None)
return ctx.run(_real_run, self) if ctx is not None else _real_run(self)
_t.Thread.start, _t.Thread.run = _start, _run
CPython's Thread._bootstrap_inner() invokes the running thread's work viaself.run() -- ordinary dynamic dispatch. When a caller uses thetarget= idiom, self.run resolves to the (now-patched) baseThread.run, whose default body calls self._target(...) -- so the
patched _run (and its ctx.run(...) wrapping) is exactly what executes,
and propagation works. But when a caller subclasses Thread and overridesrun(), self.run resolves to the SUBCLASS's method via normal MRO
lookup -- the patched Thread.run is shadowed and never called at all.
The captured _rc_sandbox_ctx is set correctly on start() (which is not
overridden in the repro's AttackThread), but nothing ever callsctx.run(...) to apply it, so the overridden run() executes in whatever
context the new OS thread happens to start with -- default None, same as
before the fix. This is the same underlying gap the fix was written to
close (a ContextVar not propagating into a thread a module spawns),
reopened by a different, equally standard way of using threading.Thread.
Subclassing Thread and overriding run() is documented, idiomatic stdlib
usage (see threading.Thread's own docstring), not an obscure pattern.
Suggested fix: don't rely on intercepting the default run() body. WrapThread.start to apply the captured context around the dispatch toself.run directly, regardless of whether run is the base implementation
or an override, e.g. by wrapping _bootstrap_inner/_bootstrap instead ofrun, or by wrapping start() so it runs the ENTIRE thread body (whateverself.run resolves to at call time) inside ctx.run(...):
def _start(self):
if _ACTIVE_SANDBOX.get() is not None and getattr(self, "_rc_sandbox_ctx", None) is None:
self._rc_sandbox_ctx = contextvars.copy_context()
return _real_start(self)
def _bootstrap_inner_wrapped(self):
ctx = getattr(self, "_rc_sandbox_ctx", None)
real_run = self.run # resolves polymorphically at call time, AFTER _rc_sandbox_ctx is set
if ctx is not None:
return ctx.run(real_run)
return real_run()
# then have run (not start) dispatch through this, or hook
# _bootstrap_inner itself rather than the class-level run slot.
(The exact mechanics need care since _bootstrap_inner is a private
CPython implementation detail across versions -- the actionable point is:
whatever is hooked must run AFTER polymorphic self.run resolution, not
replace the base class's run slot, since a subclass override is invisible
to a class-level Thread.run patch.)