diff --git a/app/lifespan.py b/app/lifespan.py index d05d059..47f8ba5 100644 --- a/app/lifespan.py +++ b/app/lifespan.py @@ -8,6 +8,7 @@ from app.workers import start_workers, stop_workers from apps.utils.docker.observe import shutdown_stats_pool +from apps.utils.docker.render_budget import shutdown_render_pool logger = logging.getLogger("greffer") @@ -62,3 +63,4 @@ async def lifespan(app: FastAPI) -> AsyncIterator[None]: await stop_workers(tasks) finally: shutdown_stats_pool() + shutdown_render_pool() diff --git a/app/settings.py b/app/settings.py index 4593306..9e470e9 100644 --- a/app/settings.py +++ b/app/settings.py @@ -369,6 +369,55 @@ def _heartbeat_interval_in_range(cls, v): # allowlist. greffer_compose_extra_allowed_keys: str = "" + # compose-containment Feature 1b (render budget). Sizes in BYTES, the + # timeout in seconds; 0 disables that dimension. The memory cap is what + # protects the shared controller from compile-time allocation + # blow-ups — disabling it on a multi-tenant node is the operator + # choosing the risk. render_budget.py reads the env directly (the + # log-rotation precedent); these fields document and validate. + greffer_render_max_input: int = 1048576 + greffer_render_max_output: int = 4194304 + greffer_render_mem_bytes: int = 268435456 + greffer_render_timeout_seconds: float = 5.0 + + @field_validator("greffer_render_max_input", mode="before") + @classmethod + def _coerce_render_input(cls, v): + try: + n = int(v) + except (TypeError, ValueError): + return 1048576 + return max(n, 0) + + @field_validator("greffer_render_max_output", mode="before") + @classmethod + def _coerce_render_output(cls, v): + try: + n = int(v) + except (TypeError, ValueError): + return 4194304 + return max(n, 0) + + @field_validator("greffer_render_mem_bytes", mode="before") + @classmethod + def _coerce_render_mem(cls, v): + try: + n = int(v) + except (TypeError, ValueError): + return 268435456 + return max(n, 0) + + @field_validator("greffer_render_timeout_seconds", mode="before") + @classmethod + def _coerce_render_timeout(cls, v): + try: + n = float(v) + except (TypeError, ValueError): + return 5.0 + if not math.isfinite(n): + return 5.0 # inf breaks Future.result; nan times out instantly + return max(n, 0.0) + @field_validator("greffer_instance_log_max_file", mode="before") @classmethod def _coerce_log_max_file(cls, v): diff --git a/apps/utils/docker/compose.py b/apps/utils/docker/compose.py index 1e434cd..a93d56e 100644 --- a/apps/utils/docker/compose.py +++ b/apps/utils/docker/compose.py @@ -3,9 +3,7 @@ import json import logging from datauri import DataURI -from jinja2 import StrictUndefined from jinja2.exceptions import SecurityError, TemplateError, UndefinedError -from jinja2.sandbox import ImmutableSandboxedEnvironment import docker import subprocess import os @@ -19,84 +17,24 @@ # calls. client = docker.from_env(max_pool_size=32) +from apps.utils.docker.render_budget import ( + RenderBudgetExceeded, + render_bounded, +) from apps.utils.docker.volume import docker_copy_file_into_volume, docker_create_volume, docker_is_volume_exist logger = logging.getLogger(__name__) -# Strict Jinja environment for rendering baked `file`/`json` destination -# contents (feature: baked-config-files). Unlike the lenient ``Template`` -# used for the compose body, a missing/typo'd variable here MUST raise -# rather than render to empty string: a baked Keycloak realm with an empty -# ``{{ config.OIDC_RP_CLIENT_SECRET }}`` is a silent security failure, so we -# fail the deploy loudly instead. ``autoescape=False`` because these are -# config files (JSON/conf), not HTML, and we must not HTML-escape values. -# -# SANDBOXED: the template string is catalog-author-controlled, and the catalog -# is community-extensible, so we must assume it can be hostile. A plain -# Environment would allow server-side template injection -> RCE on this worker -# (which holds the instance's minted secrets, the greffer's manager token, and -# Docker socket access), e.g. -# ``{{ cycler.__init__.__globals__.os.popen('id').read() }}``. SandboxedEnvironment -# blocks attribute traversal to unsafe objects while still rendering the -# legitimate ``{{ config.X }}`` / ``{{ instance_url }}`` references. A -# SecurityError surfaces as a clean ConfigRenderError (-> 422), like any other -# render failure. -# -# StrictUndefined makes a missing/typo'd variable raise rather than render empty -# (a baked secret silently becoming '' is a security failure). The catalog -# validator is a SEPARATE, author-facing layer that rejects StrictUndefined -# *bypass idioms* (``config.get('X')`` / ``| default``) and integration refs; -# it is NOT an SSTI gate — the sandbox is what stops injection. -# ``autoescape=False`` because these are config files (JSON/conf), not HTML. -_FILE_RENDER_ENV = ImmutableSandboxedEnvironment( - undefined=StrictUndefined, autoescape=False, keep_trailing_newline=True -) # globals hardened below, once _harden is defined - -# The compose BODY render. Sandboxed for the same reason as the file render -# above, but with the LENIENT undefined the body has always had -- see the -# comment in ``create_compose``. Split from _FILE_RENDER_ENV so neither -# policy can be changed by accident while editing the other. -# -# IMMUTABLE, not merely sandboxed. A plain SandboxedEnvironment blocks the -# attribute walk to the interpreter but still permits mutating calls on the -# objects passed to ``render()`` -- and those are the LIVE deployment dicts. -# ``{{ volumes.update({"x": {"value": "/"}}) }}`` renders as empty output -# while adding a volume, and ``create_volumes_then_copy_files`` then runs -# ``docker container create -v /:/root`` (volume.py) and copies attacker -# content into the host filesystem. Blocking dunder traversal alone leaves -# that write primitive open, so both envs refuse mutation. -def _harden(env): - """Drop Jinja's default globals that re-open what the sandbox closes. - - ImmutableSandboxedEnvironment refuses a MUTATING call on an immutable - target -- but it decides that from the object the method is bound to, so - an UNBOUND call slips past: ``dict`` is a Jinja global, and - ``{{ dict.update(volumes.x, {"value": "/"}) }}`` checks ``dict`` (the - class) rather than the live mapping, renders clean, and rewrites the - context. Verified: the bound form raises SecurityError while the unbound - form succeeded and set a volume's value to "/", which - create_volumes_then_copy_files then mounts as ``-v /:/root``. - - ``cycler``/``joiner``/``namespace`` are the documented first hops of the - classic escape chains, and ``lipsum``/``range`` are of no use to a compose - file. The catalog uses none of them -- verified across every pinned entry - -- so removing them costs nothing and shrinks the surface to what a - compose actually needs. - - What this does NOT do is bound resource use. Removing ``range`` closes - one way to allocate, not the class: ``*`` and ``**`` are not intercepted, - so ``{{ "A" * (10 ** 8) }}`` is sandbox-legal and renders a 100 MB - compose. The sandbox stops code execution; an input/output/time budget - around the render is the second half of Feature 1 in the root - docs/features/compose-containment/epic.md, scoped there to the custom - (untrusted) compose path.""" - for unsafe in ('dict', 'range', 'lipsum', 'cycler', 'joiner', 'namespace'): - env.globals.pop(unsafe, None) - return env - - -_COMPOSE_RENDER_ENV = _harden(ImmutableSandboxedEnvironment(autoescape=False)) -_harden(_FILE_RENDER_ENV) +# The hardened render environments moved to render_budget.py (compose- +# containment Feature 1b) so the render CHILD — which imports that module — +# never imports the docker client (on spawn platforms compose.py's module +# scope performs a live daemon handshake). Re-exported here for the existing +# call sites and tests. +from apps.utils.docker.render_budget import ( # noqa: F401 (re-export) + _COMPOSE_RENDER_ENV, + _FILE_RENDER_ENV, + render_json_bounded, +) class ConfigRenderError(Exception): @@ -316,7 +254,10 @@ def _render_baked_file(raw, greffon_info, dest_name): explicitly for clarity).""" try: text = raw.decode('utf-8') if isinstance(raw, bytes) else raw - return _FILE_RENDER_ENV.from_string(text).render(**greffon_info) + # Feature 1b: budgeted like the body render. RenderBudgetExceeded is + # a ValueError, so the tuple below already converts it to the same + # clean ConfigRenderError 422. + return render_bounded('file', text, greffon_info) except (UndefinedError, TemplateError, SecurityError, UnicodeDecodeError, TypeError, ValueError) as exc: # TypeError/ValueError: e.g. `{{ x | tojson }}` on an undefined x raises @@ -332,6 +273,10 @@ def _render_json_value(value, greffon_info, dest_name): caller ``json.dumps`` the result). Rendering leaves rather than the serialized string means ``json.dumps`` escapes the substituted values, so a minted secret containing a quote or backslash can't corrupt the JSON.""" + if value is not None and not isinstance(value, (str,)): + # One child round-trip for the whole tree (per-leaf submission was + # quadratic: every leaf re-pickled the full context). + return render_json_bounded(value, greffon_info) if isinstance(value, str): return _render_baked_file(value, greffon_info, dest_name) if isinstance(value, dict): @@ -715,7 +660,12 @@ def create_compose(compose, greffon_info): # reachable. The compose file is only written after the render succeeds. dumped = yaml.dump(compose) try: - compose_file = _COMPOSE_RENDER_ENV.from_string(dumped).render(**greffon_info) + # Feature 1b: the render runs under the budget (input/output caps, + # child-process memory + time bound) — see render_budget.py. A cap + # violation re-raises below as its own bounded message. + compose_file = render_bounded('body', dumped, greffon_info) + except RenderBudgetExceeded as exc: + raise ConfigRenderError(str(exc)) from exc except Exception as exc: logger.error("compose render failed for %s: %s: %s", greffon_info.get('id'), type(exc).__name__, exc) diff --git a/apps/utils/docker/render_budget.py b/apps/utils/docker/render_budget.py new file mode 100644 index 0000000..a576d5f --- /dev/null +++ b/apps/utils/docker/render_budget.py @@ -0,0 +1,454 @@ +"""The render budget (compose-containment Feature 1b — the second half of +Feature 1, which the sandbox deliberately deferred). + +The sandbox (greffer#136) stops code execution and mutation; it does NOT +bound resource use. ``*``/``**`` are not intercepted, so a sandbox-legal +``{{ "A" * (10 ** 9) }}`` allocates ~1 GB — and Jinja CONSTANT-FOLDS it at +COMPILE time, before any output exists, so an output cap alone cannot catch +it. This module bounds the whole render, compile included, with three caps +plus isolation: + +- input: the template SOURCE (bytes) is capped before anything runs; +- output: the rendered string is accumulated via ``generate()`` and aborted + the moment it exceeds its cap (a blow-up must pass through here); +- memory: the render runs in a dedicated ONE-WORKER child process whose + ``RLIMIT_AS`` is capped, so a compile-time fold or any other + allocation blow-up kills the CHILD (a MemoryError we convert to a + clean refusal), never the ``--workers 1`` controller; +- time: the parent waits with a timeout; on expiry the pool is dropped and + rebuilt (a wedged worker must not poison the next render). + +Isolation is the load-bearing half: caps are checked in the child, where a +violation is an exception, not an OOM of the process that holds the manager +token. + +The caller-facing contract: ``render_bounded()`` returns the rendered string +or raises ``RenderBudgetExceeded`` — a ``ValueError`` SUBCLASS, so the +existing ``except`` tuples at both render call sites (baked files catch +ValueError; the body catches Exception) already map it to their +``ConfigRenderError`` 422s without new wiring. + +Env knobs (read per call; renders are rare): ``GREFFER_RENDER_MAX_INPUT`` +(default ``1048576``), ``GREFFER_RENDER_MAX_OUTPUT`` (default ``4194304``), +``GREFFER_RENDER_MEM_BYTES`` (default ``268435456``; also pins the child at +pool build — a change rebuilds the pool), ``GREFFER_RENDER_TIMEOUT_SECONDS`` +(default ``5``). ``0`` disables that dimension — the operator's call, but +note BOTH foot-guns: without the memory guard a compile-time fold can +kill the controller's sibling work, and without the timeout a wedged +render holds an anyio threadpool token (shared with start/stop) +indefinitely. +""" +from __future__ import annotations + +import concurrent.futures +import concurrent.futures.process +import logging +import math +import os +import sys +import threading + +from jinja2 import StrictUndefined +from jinja2.sandbox import ImmutableSandboxedEnvironment +logger = logging.getLogger("greffer") + +_DEFAULT_INPUT = 1048576 # 1 MiB — compose bodies are tens of KB +_DEFAULT_OUTPUT = 4194304 # 4 MiB +_DEFAULT_MEM = 268435456 # 256 MiB +_DEFAULT_TIMEOUT = 5.0 + + +class RenderBudgetExceeded(ValueError): + """The render exceeded a cap. Message is a short machine code plus the + cap — never rendered content (which may be the very thing that blew the + budget). ValueError so the existing ConfigRenderError mappings catch it. + """ + + def __init__(self, code: str, cap): + self.code = code + self.cap = cap + super().__init__(f"render_{code} (cap: {cap})") + + def __reduce__(self): + # Exceptions raised in the child are PICKLED back to the parent; + # the default pickling replays args (the formatted message), which + # __init__ cannot accept. Round-trip through the constructor args. + return (self.__class__, (self.code, self.cap)) + + +def _knob(name: str, default: int, as_float: bool = False): + raw = os.getenv(name) + if raw is None or not str(raw).strip(): + return float(default) if as_float else default + try: + value = float(raw) if as_float else int(raw) + if value < 0 or (as_float and not math.isfinite(value)): + # inf makes Future.result raise OverflowError and nan times + # out immediately — either way a typo'd knob breaks every + # render instead of falling back (codex review). + raise ValueError + except (TypeError, ValueError): + logger.warning("invalid %s=%r; using %s", name, raw, default) + return float(default) if as_float else default + return value + + +# Strict Jinja environment for rendering baked `file`/`json` destination +# contents (feature: baked-config-files). Unlike the lenient ``Template`` +# used for the compose body, a missing/typo'd variable here MUST raise +# rather than render to empty string: a baked Keycloak realm with an empty +# ``{{ config.OIDC_RP_CLIENT_SECRET }}`` is a silent security failure, so we +# fail the deploy loudly instead. ``autoescape=False`` because these are +# config files (JSON/conf), not HTML, and we must not HTML-escape values. +# +# SANDBOXED: the template string is catalog-author-controlled, and the catalog +# is community-extensible, so we must assume it can be hostile. A plain +# Environment would allow server-side template injection -> RCE on this worker +# (which holds the instance's minted secrets, the greffer's manager token, and +# Docker socket access), e.g. +# ``{{ cycler.__init__.__globals__.os.popen('id').read() }}``. SandboxedEnvironment +# blocks attribute traversal to unsafe objects while still rendering the +# legitimate ``{{ config.X }}`` / ``{{ instance_url }}`` references. A +# SecurityError surfaces as a clean ConfigRenderError (-> 422), like any other +# render failure. +# +# StrictUndefined makes a missing/typo'd variable raise rather than render empty +# (a baked secret silently becoming '' is a security failure). The catalog +# validator is a SEPARATE, author-facing layer that rejects StrictUndefined +# *bypass idioms* (``config.get('X')`` / ``| default``) and integration refs; +# it is NOT an SSTI gate — the sandbox is what stops injection. +# ``autoescape=False`` because these are config files (JSON/conf), not HTML. +_FILE_RENDER_ENV = ImmutableSandboxedEnvironment( + undefined=StrictUndefined, autoescape=False, keep_trailing_newline=True +) # globals hardened below, once _harden is defined + +# The compose BODY render. Sandboxed for the same reason as the file render +# above, but with the LENIENT undefined the body has always had -- see the +# comment in ``create_compose``. Split from _FILE_RENDER_ENV so neither +# policy can be changed by accident while editing the other. +# +# IMMUTABLE, not merely sandboxed. A plain SandboxedEnvironment blocks the +# attribute walk to the interpreter but still permits mutating calls on the +# objects passed to ``render()`` -- and those are the LIVE deployment dicts. +# ``{{ volumes.update({"x": {"value": "/"}}) }}`` renders as empty output +# while adding a volume, and ``create_volumes_then_copy_files`` then runs +# ``docker container create -v /:/root`` (volume.py) and copies attacker +# content into the host filesystem. Blocking dunder traversal alone leaves +# that write primitive open, so both envs refuse mutation. +def _harden(env): + """Drop Jinja's default globals that re-open what the sandbox closes. + + ImmutableSandboxedEnvironment refuses a MUTATING call on an immutable + target -- but it decides that from the object the method is bound to, so + an UNBOUND call slips past: ``dict`` is a Jinja global, and + ``{{ dict.update(volumes.x, {"value": "/"}) }}`` checks ``dict`` (the + class) rather than the live mapping, renders clean, and rewrites the + context. Verified: the bound form raises SecurityError while the unbound + form succeeded and set a volume's value to "/", which + create_volumes_then_copy_files then mounts as ``-v /:/root``. + + ``cycler``/``joiner``/``namespace`` are the documented first hops of the + classic escape chains, and ``lipsum``/``range`` are of no use to a compose + file. The catalog uses none of them -- verified across every pinned entry + -- so removing them costs nothing and shrinks the surface to what a + compose actually needs. + + What this does NOT do is bound resource use. Removing ``range`` closes + one way to allocate, not the class: ``*`` and ``**`` are not intercepted, + so ``{{ "A" * (10 ** 8) }}`` is sandbox-legal and renders a 100 MB + compose. The sandbox stops code execution; an input/output/time budget + around the render is the second half of Feature 1 in the root + docs/features/compose-containment/epic.md, scoped there to the custom + (untrusted) compose path.""" + for unsafe in ('dict', 'range', 'lipsum', 'cycler', 'joiner', 'namespace'): + env.globals.pop(unsafe, None) + return env + + +_COMPOSE_RENDER_ENV = _harden(ImmutableSandboxedEnvironment(autoescape=False)) +_harden(_FILE_RENDER_ENV) + + +# --- the child --------------------------------------------------------------- + +_CHILD_MEM_LIMIT = 0 # set by the pool initializer in the child process + + +def _child_init(mem_bytes: int) -> None: + if mem_bytes: + try: + import resource + + resource.setrlimit(resource.RLIMIT_AS, (mem_bytes, mem_bytes)) + except (ImportError, OSError, ValueError): + # macOS does not support RLIMIT_AS (setrlimit raises or is + # ignored). The child then relies on the input/output/time caps; + # production (alpine Linux) gets the full guard. + pass + # PDEATHSIG (Linux): if the PARENT dies, this child must not survive it + # churning a full core. + try: + import ctypes + + # CDLL(None) resolves against the PROCESS itself: works on glibc + # and musl (alpine) alike — "libc.so.6" does not exist on musl, + # which silently no-oped the guard in production (verification + # round finding). + libc = ctypes.CDLL(None, use_errno=True) + prctl = getattr(libc, "prctl", None) + if prctl is not None: + prctl(1, 9, 0, 0, 0) # PR_SET_PDEATHSIG, SIGKILL + except (OSError, ValueError, AttributeError): + # macOS has no prctl at all (dlsym raises AttributeError — which + # once crashed the child INITIALIZER and turned every render into + # child_died); musl/glibc Linux both resolve via CDLL(None). + pass + + +def _child_render(which: str, source: str, context: dict) -> str: + """Runs IN THE CHILD. which is 'body' or 'file'; the environments live + in compose.py, imported lazily so a forked child pays nothing and a + spawned one reuses the exact hardened envs (no drift to a local copy). + """ + # The envs are MODULE-LOCAL (moved from compose.py exactly so this + # child never imports the docker client — on spawn platforms that + # import performs a live daemon handshake, which turned a budget + # refusal into a DockerException 500 and re-paid 0.5s on every pool + # rebuild). + env = _COMPOSE_RENDER_ENV if which == "body" else _FILE_RENDER_ENV + max_out = _knob("GREFFER_RENDER_MAX_OUTPUT", _DEFAULT_OUTPUT) + template = env.from_string(source) # compile-time folds happen HERE, + # under the child's RLIMIT_AS — the case no output cap can catch. + try: + if not max_out: + return template.render(**context) + chunks = [] + total = 0 + for chunk in template.generate(**context): + # BYTES, matching the knob's documented unit (counting + # characters let a 1000-astral-char render pass a "1000 byte" + # cap as 4000). + total += len(chunk.encode("utf-8", "surrogatepass")) + if total > max_out: + raise RenderBudgetExceeded("output_too_large", max_out) + chunks.append(chunk) + return "".join(chunks) + except RenderBudgetExceeded: + raise + except Exception as exc: + # The exception is PICKLED to the parent: any implementation that + # echoes its input into the message (a future filter edge; the + # current set was swept and never does) would ship a giant string + # across the boundary and re-allocate it in the parent. Re-raise + # with a bounded message; if the type cannot be reconstructed with + # one argument, fall back to the budget refusal. + trimmed = str(exc)[:8192] + # Construct BEFORE raising: a raise inside the try would be caught + # by this very except and convert every real error into + # render_child_error (found by the sandbox suite: SecurityError + # must survive as SecurityError for the caller's isinstance checks). + try: + rebuilt = type(exc)(trimmed) + except Exception: + raise RenderBudgetExceeded("child_error", 8192) from exc + raise rebuilt from None + + +def _child_render_json(tree, context): + """Runs IN THE CHILD: render every STRING LEAF of a json-destination + value in one job. Per-leaf submission re-pickled the entire context + (cert material included) for every leaf — quadratic on real realm + documents (codex review: a 5k-leaf doc went 0.6s -> 10.4s).""" + if isinstance(tree, str): + max_in = _knob("GREFFER_RENDER_MAX_INPUT", _DEFAULT_INPUT) + if max_in and len(tree.encode("utf-8", "surrogatepass")) > max_in: + raise RenderBudgetExceeded("input_too_large", max_in) + return _child_render("file", tree, context) + if isinstance(tree, dict): + return {k: _child_render_json(v, context) for k, v in tree.items()} + if isinstance(tree, list): + return [_child_render_json(v, context) for v in tree] + return tree + + +def render_json_bounded(tree, context) -> object: + """Budgeted render of a json-destination value's string leaves, in one + child round-trip (input cap applies per leaf via the child render).""" + src_len = sum( + len(leaf.encode("utf-8", "surrogatepass")) + for leaf in _iter_string_leaves(tree)) + max_in = _knob("GREFFER_RENDER_MAX_INPUT", _DEFAULT_INPUT) + if max_in and src_len > max_in * 100: + # A sane guard for pathological leaf counts; each leaf is itself + # input-capped in the child. + raise RenderBudgetExceeded("input_too_large", max_in * 100) + mem = _knob("GREFFER_RENDER_MEM_BYTES", _DEFAULT_MEM) + timeout = _knob("GREFFER_RENDER_TIMEOUT_SECONDS", _DEFAULT_TIMEOUT, + as_float=True) + return _submit_budgeted( + lambda pool: pool.submit(_child_render_json, tree, context), + mem, timeout) + + +def _iter_string_leaves(tree): + if isinstance(tree, str): + yield tree + elif isinstance(tree, dict): + for v in tree.values(): + yield from _iter_string_leaves(v) + elif isinstance(tree, list): + for v in tree: + yield from _iter_string_leaves(v) + + +# --- the pool ---------------------------------------------------------------- + +_pool: concurrent.futures.ProcessPoolExecutor | None = None +_pool_mem = -1 +_pool_lock = threading.Lock() + + +def _get_pool(mem_bytes: int) -> concurrent.futures.ProcessPoolExecutor: + global _pool, _pool_mem + if _pool is not None and _pool_mem == mem_bytes: + return _pool + with _pool_lock: + if _pool is not None and _pool_mem == mem_bytes: + return _pool + if _pool is not None: + _drop_pool_locked() + # Pin FORK on Linux explicitly (the platform default, but now a + # decision): the child inherits the parent's address space, so the + # RLIMIT_AS cap effectively budgets 'parent VSZ at fork + render'. + # Measured headroom on the prod image: ~106 MB inherited vs the + # 256 MB default. A runtime swap to big-stack glibc would shrink + # that headroom and make LEGITIMATE renders fail memory_exceeded — + # re-measure if the base image changes. Spawn (macOS dev) is + # correct too: this module imports no docker client, so the child + # performs no daemon handshake. + import multiprocessing + + ctx = (multiprocessing.get_context("fork") + if sys.platform == "linux" else + multiprocessing.get_context()) + _pool = concurrent.futures.ProcessPoolExecutor( + max_workers=1, initializer=_child_init, initargs=(mem_bytes,), + mp_context=ctx) + _pool_mem = mem_bytes + return _pool + + +def _drop_pool_locked() -> None: + global _pool, _pool_mem + pool, _pool, _pool_mem = _pool, None, -1 + if pool is not None: + # wait=False + cancel: the pool is discarded precisely because its + # worker is presumed wedged or dead. shutdown() alone neither kills + # nor abandons a BUSY worker — a timed-out render would keep + # churning a full core with its result discarded, accumulate one + # leaked child per wedge, and (via the executor's atexit join) stall + # interpreter exit for as long as the churn lasts, wedging the + # watchdog restart path. Kill the discarded pool's processes so + # every wedge is bounded. + # Capture the worker handles BEFORE shutdown: shutdown() may + # clear _processes synchronously (codex review), after which the + # kill loop would iterate nothing and the churning worker survives. + procs = getattr(pool, "_processes", None) or {} + procs = procs.values() if isinstance(procs, dict) else procs + procs = list(procs) + pool.shutdown(wait=False, cancel_futures=True) + for proc in procs: + try: + proc.kill() + except (OSError, ValueError, AttributeError): + pass + + +def shutdown_render_pool() -> None: + """Lifespan teardown hook (the shutdown_stats_pool pattern): non-daemon + executor workers must be told to exit or interpreter exit stalls.""" + with _pool_lock: + if _pool is not None: + _drop_pool_locked() + + +def _submit_budgeted(submit, mem: int, timeout: float): + """Shared engine: pick the pool, submit under the lock, map every + failure to a budget refusal. Retries are BOUNDED (a pathological pool + recycler must not pin a render — and its anyio token — forever). + Order matters below: BrokenProcessPool IS a RuntimeError subclass, and + CancelledError is Exception-derived but in neither production call + site's catch tuple — both previously escaped as bare errors. + """ + for _attempt in range(3): + pool = _get_pool(mem) + try: + with _pool_lock: + _check_pool_current(pool) + future = submit(pool) + return future.result(timeout=timeout or None) + except RenderBudgetExceeded: + raise + except concurrent.futures.process.BrokenProcessPool as exc: + # The child died hard (on an un-RLIMITed platform an allocation + # blow-up is exactly this shape). MUST precede except + # RuntimeError, of which it is a subclass — an earlier ordering + # made this clause dead code and let the raw error escape. + _drop_pool_and_raise(pool, "child_died", mem, exc) + except concurrent.futures.CancelledError as exc: + # Our future was PENDING when another caller's timeout dropped + # this pool (shutdown(cancel_futures=True)). Retry on the fresh + # pool; if this pool is (oddly) still current, refuse. + if _pool is not pool: + continue + raise RenderBudgetExceeded("cancelled", 0) from exc + except RuntimeError as exc: + # Submit raced a concurrent recycle (executor shutting down). + if _pool is pool: + raise + continue + except concurrent.futures.TimeoutError as exc: + _drop_pool_and_raise(pool, "timeout", timeout, exc) + except MemoryError as exc: + # Raised in the PARENT when the child hit its RLIMIT_AS and the + # MemoryError itself was pickled back. + _drop_pool_and_raise(pool, "memory_exceeded", mem, exc) + + +def render_bounded(which: str, source: str, context: dict) -> str: + """Render ``source`` under the budget; returns the rendered string. + + Raises RenderBudgetExceeded on any cap violation (input, output, memory, + timeout, or a dead child — all 'the render exceeded what the node will + give it'), re-raising the caller's existing 422 mapping. + """ + if which not in ("body", "file"): + # A typo'd call site must fail loudly, not silently render under the + # StrictUndefined file env (the 'else' branch of the env selection). + raise ValueError(f"unknown render kind: {which!r}") + max_in = _knob("GREFFER_RENDER_MAX_INPUT", _DEFAULT_INPUT) + if max_in and len(source.encode("utf-8", "surrogatepass")) > max_in: + raise RenderBudgetExceeded("input_too_large", max_in) + mem = _knob("GREFFER_RENDER_MEM_BYTES", _DEFAULT_MEM) + timeout = _knob("GREFFER_RENDER_TIMEOUT_SECONDS", _DEFAULT_TIMEOUT, + as_float=True) + + return _submit_budgeted( + lambda pool: pool.submit(_child_render, which, source, context), + mem, timeout) + + +def _check_pool_current(pool) -> None: + if _pool is not pool: + raise RuntimeError("pool recycled") + + +def _drop_pool_and_raise(pool, code: str, cap, exc: Exception) -> None: + # Recycle only the FAILED executor: another caller may have already + # replaced the global with a healthy pool, and dropping that one + # cancels unrelated in-flight renders as 500s (codex review). + with _pool_lock: + if _pool is pool: + _drop_pool_locked() + logger.warning("render budget: %s (cap %s) — pool recycled", code, cap) + raise RenderBudgetExceeded(code, cap) from exc diff --git a/tests/test_render_budget.py b/tests/test_render_budget.py new file mode 100644 index 0000000..142885f --- /dev/null +++ b/tests/test_render_budget.py @@ -0,0 +1,231 @@ +"""Tests for the render budget (compose-containment Feature 1b). + +The budget is the second half of Feature 1: the sandbox stops code +execution, this stops resource exhaustion — a sandbox-legal allocation +blow-up must fail as a clean 422-shaped refusal, never take down the +--workers 1 controller that holds the manager token.""" +from __future__ import annotations + +import os +import sys + +import pytest + +from apps.utils.docker import compose, render_budget +from apps.utils.docker.render_budget import RenderBudgetExceeded + + +def _ctx(): + return {"id": "t1", "ports": [], "config": {}, "smtp": {}} + + +@pytest.fixture(autouse=True) +def _fresh_pool(): + render_budget.shutdown_render_pool() + yield + render_budget.shutdown_render_pool() + + +class TestCaps: + def test_small_render_passes_through(self, monkeypatch): + monkeypatch.setenv("GREFFER_RENDER_MAX_INPUT", "100000") + monkeypatch.setenv("GREFFER_RENDER_MAX_OUTPUT", "100000") + out = render_budget.render_bounded( + "body", "services: {{ config.a }}", {"config": {"a": "x"}}) + assert out == "services: x" + + def test_input_cap_refuses_before_any_work(self, monkeypatch): + monkeypatch.setenv("GREFFER_RENDER_MAX_INPUT", "64") + monkeypatch.setenv("GREFFER_RENDER_MAX_OUTPUT", "0") + source = "{{ 'pad' * 100 }}" + "# padding to exceed the cap " * 4 + with pytest.raises(RenderBudgetExceeded) as e: + render_budget.render_bounded("body", source, _ctx()) + assert e.value.code == "input_too_large" + assert "pad" not in str(e.value) # bounded message, never content + + def test_output_cap_aborts_mid_stream(self, monkeypatch): + # The blow-up must pass through generate(); the cap aborts on the + # first oversized accumulation, in the CHILD (an output cap checked + # only after a full render() would let the allocation happen first). + monkeypatch.setenv("GREFFER_RENDER_MAX_INPUT", "1000000") + monkeypatch.setenv("GREFFER_RENDER_MAX_OUTPUT", "1024") + with pytest.raises(RenderBudgetExceeded) as e: + render_budget.render_bounded( + "body", "{{ 'x' * 100000 }}", _ctx()) + assert e.value.code == "output_too_large" + + def test_disabled_dimensions_are_off(self, monkeypatch): + monkeypatch.setenv("GREFFER_RENDER_MAX_INPUT", "0") + monkeypatch.setenv("GREFFER_RENDER_MAX_OUTPUT", "0") + out = render_budget.render_bounded( + "body", "{{ 'x' * 5000 }}", _ctx()) + assert out == "x" * 5000 + + def test_exception_pickles_across_the_process_boundary(self): + # The child raises; the parent must see RenderBudgetExceeded, not an + # unpickling failure (the __reduce__ contract). + import pickle + + roundtripped = pickle.loads(pickle.dumps( + RenderBudgetExceeded("output_too_large", 99))) + assert roundtripped.code == "output_too_large" + assert roundtripped.cap == 99 + + +class TestBlowups: + def test_compile_time_fold_is_bounded(self, monkeypatch): + # The case no output cap can catch: Jinja CONSTANT-FOLDS 'x' * 10**7 + # at COMPILE time, allocating ~10 MB before generate() yields a + # single chunk. With a small memory cap the CHILD takes the hit + # (MemoryError or hard death — either maps to a budget refusal) and + # the parent converts it. On non-RLIMIT platforms (macOS) the child + # may instead be killed -> child_died; both codes are the feature + # working, so accept either. + monkeypatch.setenv("GREFFER_RENDER_MAX_INPUT", "1000000") + monkeypatch.setenv("GREFFER_RENDER_MAX_OUTPUT", "1000000") + monkeypatch.setenv("GREFFER_RENDER_MEM_BYTES", str(64 * 1024 * 1024)) + with pytest.raises(RenderBudgetExceeded) as e: + render_budget.render_bounded( + "file", "{{ 'x' * (10 ** 8) }}", _ctx()) + # On RLIMIT platforms (production alpine, Linux CI) the fold dies at + # compile; without RLIMIT (macOS dev) the fold survives and the + # OUTPUT cap catches the first chunk. Both are the budget working. + assert e.value.code in ("memory_exceeded", "child_died", + "output_too_large") + + @pytest.mark.skipif(sys.platform == "darwin", + reason="RLIMIT_AS is not enforced on macOS") + def test_memory_cap_linux(self, monkeypatch): + monkeypatch.setenv("GREFFER_RENDER_MAX_INPUT", "1000000") + monkeypatch.setenv("GREFFER_RENDER_MAX_OUTPUT", "1000000") + monkeypatch.setenv("GREFFER_RENDER_MEM_BYTES", str(64 * 1024 * 1024)) + with pytest.raises(RenderBudgetExceeded) as e: + render_budget.render_bounded( + "file", "{{ 'x' * (10 ** 9) }}", _ctx()) + assert e.value.code == "memory_exceeded" + + def test_timeout_path_fires_kills_worker_and_recycles_pool(self, monkeypatch): + # A timeout so small the pool round-trip itself exceeds it: proves + # the timeout machinery fires, the exception is the budget refusal, + # the WORKER IS KILLED (a dropped pool must not leave a churning + # child — the review's P1-1: leaked full-core children accumulate + # and stall interpreter exit via the executor's atexit join), and + # the pool is rebuilt (next render succeeds). + import time + + assert render_budget.render_bounded("body", "ok", _ctx()) == "ok" + pool_before = render_budget._pool + procs = getattr(pool_before, "_processes", None) or {} + worker = next(iter((procs.values() if isinstance(procs, dict) + else procs)), None) + assert worker is not None + monkeypatch.setenv("GREFFER_RENDER_TIMEOUT_SECONDS", "0.0001") + with pytest.raises(RenderBudgetExceeded) as e: + render_budget.render_bounded("body", "ok", _ctx()) + assert e.value.code == "timeout" + monkeypatch.delenv("GREFFER_RENDER_TIMEOUT_SECONDS") + # The discarded pool's worker is dead within a bounded wait. + # (multiprocessing BaseProcess exposes is_alive(); poll() is + # subprocess-only.) + deadline = time.monotonic() + 5.0 + while time.monotonic() < deadline and worker.is_alive(): + time.sleep(0.05) + assert not worker.is_alive(), "timed-out worker still alive" + # Belt: the PID itself is gone (is_alive can be False for a zombie + # that shutdown() later reaps; the kill must have actually fired). + try: + os.kill(worker.pid, 0) + raise AssertionError("worker pid still signalable after kill") + except (ProcessLookupError, OSError): + pass + # A fresh pool serves the next render. + assert render_budget.render_bounded("body", "ok", _ctx()) == "ok" + assert render_budget._pool is not pool_before + + def test_hard_child_death_is_child_died_not_raw_broken_pool(self): + # Regression (found by the verification round): except RuntimeError + # sat ABOVE except BrokenProcessPool (a RuntimeError SUBCLASS), + # making the child_died clause dead code — an externally killed + # worker escaped as a raw BrokenProcessPool 500 on the baked/json + # paths instead of the designed refusal. + import signal + + assert render_budget.render_bounded("body", "ok", _ctx()) == "ok" + pool = render_budget._pool + procs = getattr(pool, "_processes", None) or {} + worker = next(iter((procs.values() if isinstance(procs, dict) + else procs))) + os.kill(worker.pid, signal.SIGKILL) + with pytest.raises(RenderBudgetExceeded) as e: + render_budget.render_bounded("body", "ok", _ctx()) + assert e.value.code in ("child_died", "cancelled") + + def test_cancelled_pending_future_does_not_escape(self, monkeypatch): + # Regression: a PENDING future cancelled by another caller's pool + # drop raised bare CancelledError (not in any catch tuple → 500 on + # the baked/json paths). The engine must convert or retry it. + import threading + + monkeypatch.setenv("GREFFER_RENDER_TIMEOUT_SECONDS", "0.0001") + errors = [] + + def _render(): + try: + render_budget.render_bounded("body", "ok", _ctx()) + except RenderBudgetExceeded: + errors.append("budget") + except Exception as exc: # noqa: BLE001 — the escape under test + errors.append(("ESCAPED", type(exc).__name__)) + + threads = [threading.Thread(target=_render) for _ in range(8)] + for t in threads: + t.start() + for t in threads: + t.join(timeout=30) + assert all(e == "budget" for e in errors), errors + + def test_unknown_render_kind_refused(self): + with pytest.raises(ValueError, match="unknown render kind"): + render_budget.render_bounded("bodyy", "ok", _ctx()) + + def test_multibyte_output_counts_bytes(self, monkeypatch): + # Regression: the cap counted CHARACTERS; astral-plane content + # passed a '1000 byte' cap as 4000 actual bytes. + monkeypatch.setenv("GREFFER_RENDER_MAX_INPUT", "100000") + monkeypatch.setenv("GREFFER_RENDER_MAX_OUTPUT", "1000") + with pytest.raises(RenderBudgetExceeded) as e: + render_budget.render_bounded("body", "{{ '\U0001F600' * 400 }}", _ctx()) + assert e.value.code == "output_too_large" + + +class TestWiring: + """The two production call sites map budget refusals to their existing + 422 shapes (ConfigRenderError), without new router wiring.""" + + def test_body_render_budget_maps_to_config_render_error(self, monkeypatch, tmp_path): + monkeypatch.setenv("GREFFON_PATH", str(tmp_path)) + monkeypatch.setenv("GREFFER_RENDER_MAX_INPUT", "8") + with pytest.raises(compose.ConfigRenderError, match="input_too_large"): + compose.create_compose( + {"services": {"a": {"image": "x"}}}, + {"id": "t1", "ports": [], "configurations": [], + "integrations": {}}) + + def test_baked_file_budget_maps_to_config_render_error(self, tmp_path, monkeypatch): + monkeypatch.setenv("GREFFON_PATH", str(tmp_path)) + monkeypatch.setenv("GREFFER_RENDER_MAX_OUTPUT", "64") + info = {"id": "t1", "ports": [], "configurations": [{ + "value": {"file": "data:application/octet-stream;base64,aGVsbG8="}, + "destinations": [{ + "type": "file", "name": "big.txt", "volume": "v", + "x-greffon-render": True}]}], + "volumes": {"v": {"name": "v", "value": "v", + "containers": {}, "files": []}}} + # The baked payload renders {{ 'x' * N }} -> output cap -> 422. + import base64 + payload = base64.b64encode(b"{{ 'x' * 10000 }}").decode() + info["configurations"][0]["value"]["file"] = ( + "data:application/octet-stream;base64," + payload) + with pytest.raises(compose.ConfigRenderError, + match="output_too_large"): + compose.apply_configuration(info, {})