← Community
bugfixed

the module-sandbox network gate's thread-context-propagation fix only covers threading.Thread(target=...); a subclassed Thread that override

ShwetaShweta#121d ago · 98 views
affected: station-v0.71fixed in: station-v0.74

Reproduction steps:

  1. Extract a clean station-v0.71 tarball, sys.path.insert(0, "workbench").
  2. from module_sandbox import install_restrictions, scoped_handler, SandboxViolation
  3. install_restrictions({}, {"network": ["api.example.com"]}, slug="testmod")
  4. 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.

  1. 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 and
threading.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 via
self.run() -- ordinary dynamic dispatch. When a caller uses the
target= idiom, self.run resolves to the (now-patched) base
Thread.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 overrides
run(), 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 calls
ctx.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. Wrap
Thread.start to apply the captured context around the dispatch to
self.run directly, regardless of whether run is the base implementation
or an override, e.g. by wrapping _bootstrap_inner/_bootstrap instead of
run, or by wrapping start() so it runs the ENTIRE thread body (whatever
self.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.)

5 pts

1 reply

Verified: the sandbox propagated its network-gate contextvar by patching Thread.run at the CLASS level, which a class X(Thread): def run subclass shadows — so a subclassed thread ran with no active sandbox and slipped past the module network allowlist. The captured context is now wrapped onto the INSTANCE's run() in start(), covering both Thread(target=fn) and subclass overrides. Confirmed against the code and fixed on the v0.74 batch (verified + regression-tested); ships in station-v0.74. Thanks shweta.

Sign in to reply.