mirror of
https://github.com/agentscope-ai/ReMe.git
synced 2026-10-26 06:04:11 +00:00
Refine file sandbox limits and executor checks
This commit is contained in:
酱牛肉
parent
7df7f60f9c
commit
e6ddb2bb35
13 changed files with 199 additions and 90 deletions
|
|
@ -15,6 +15,7 @@ contributes the `auto_bash` job, which can be run with
|
|||
The job defaults to read-only command validation and a 60-second timeout. Set
|
||||
`is_read_only=false` to disable command validation. Supplying `read_only_paths`
|
||||
or `writable_paths` enables a file sandbox (Bubblewrap on Linux, Seatbelt on
|
||||
macOS), with sandbox paths identical to their host paths. Linux defaults to a
|
||||
4 GiB cgroup memory limit; macOS has no memory limit and warns when a nonzero
|
||||
`memory_limit_bytes` value is supplied.
|
||||
macOS), with sandbox paths identical to their host paths. On Linux, the
|
||||
`memory_limit_mib` cgroup limit defaults to 4096 MiB; 0 disables it. Supplying
|
||||
the limit requires a path policy. macOS has no memory limit and warns when a
|
||||
nonzero value is supplied.
|
||||
|
|
|
|||
|
|
@ -1,11 +1,13 @@
|
|||
"""Run allowlisted shell commands without an approval prompt."""
|
||||
|
||||
import asyncio
|
||||
|
||||
from agentscope.message import TextBlock, ToolResultState
|
||||
from agentscope.permission import PermissionBehavior, PermissionContext, PermissionDecision
|
||||
from agentscope.tool import Bash, ToolChunk
|
||||
|
||||
from reme.utils.file_sandbox import create_file_sandbox_backend
|
||||
from reme.steps.base_step import BaseStep
|
||||
from reme.steps.common.file_sandbox import file_sandbox_for_step
|
||||
|
||||
_READ_COMMANDS = {
|
||||
":": {},
|
||||
|
|
@ -53,7 +55,9 @@ unset GIT_EXTERNAL_DIFF GIT_TRACE GIT_TRACE_SETUP GIT_TRACE_PERFORMANCE
|
|||
unset GIT_TRACE_PACK_ACCESS GIT_TRACE_PACKET GIT_TRACE_CURL GIT_TRACE2 GIT_TRACE2_EVENT GIT_TRACE2_PERF
|
||||
export GIT_OPTIONAL_LOCKS=0 GIT_PAGER=/bin/cat GIT_NO_LAZY_FETCH=1 GIT_TERMINAL_PROMPT=0
|
||||
git() {
|
||||
if command git -c trace2.normalTarget=0 -c trace2.eventTarget=0 -c trace2.perfTarget=0 \
|
||||
# These metadata queries do not read file contents through Git filters.
|
||||
if [ "$1" != rev-parse ] && [ "$1" != ls-files ] && \
|
||||
command git -c trace2.normalTarget=0 -c trace2.eventTarget=0 -c trace2.perfTarget=0 \
|
||||
config --get-regexp '^filter\..*\.(clean|smudge|process)$' > /dev/null; then
|
||||
echo 'Permission denied: Git repositories with external filters require unrestricted mode.' >&2
|
||||
return 1
|
||||
|
|
@ -158,19 +162,26 @@ class AutoBash(Bash):
|
|||
if "$" in command or "`" in command:
|
||||
return False
|
||||
try:
|
||||
from safecmd import CmdSpec, validate # pylint: disable=import-outside-toplevel
|
||||
from safecmd.bashxtract import extract_commands, parse_bash # pylint: disable=import-outside-toplevel
|
||||
from safecmd.core import normalize_dest # pylint: disable=import-outside-toplevel
|
||||
from safecmd import CmdSpec # pylint: disable=import-outside-toplevel
|
||||
from safecmd.bashxtract import ( # pylint: disable=import-outside-toplevel
|
||||
check_types,
|
||||
collect_ops,
|
||||
collect_redirects,
|
||||
parse_bash,
|
||||
visit_stmts,
|
||||
)
|
||||
from safecmd.core import normalize_dest, validate_cmd # pylint: disable=import-outside-toplevel
|
||||
|
||||
if not _literal_shell(parse_bash(command), command):
|
||||
ast = parse_bash(command)
|
||||
check_types(ast)
|
||||
if not _literal_shell(ast, command):
|
||||
return False
|
||||
specs = {CmdSpec(name, denied=options.get("denied")) for name, options in _READ_COMMANDS.items()}
|
||||
validate(command, cmds=specs, dests=_SAFE_DESTINATIONS)
|
||||
commands, operators, redirects = extract_commands(command)
|
||||
if any(not _allowed_arguments(tokens) for tokens in commands):
|
||||
commands = visit_stmts(ast.get("Stmts", []), command)
|
||||
if any(not validate_cmd(tokens, specs) or not _allowed_arguments(tokens) for tokens in commands):
|
||||
return False
|
||||
return "=" not in operators and all(
|
||||
normalize_dest(destination) == "/dev/null" for _, destination in redirects
|
||||
return "=" not in collect_ops(ast) and all(
|
||||
normalize_dest(destination) in _SAFE_DESTINATIONS for _, destination in collect_redirects(ast, command)
|
||||
)
|
||||
except Exception: # pylint: disable=broad-exception-caught
|
||||
# Parser failures and missing shfmt must never allow execution.
|
||||
|
|
@ -178,7 +189,7 @@ class AutoBash(Bash):
|
|||
|
||||
async def check_read_only(self, tool_input: dict) -> bool:
|
||||
"""Report the same policy used by the execution guard."""
|
||||
return self.is_read_only and self._is_read_only(tool_input.get("command", ""))
|
||||
return self.is_read_only and await asyncio.to_thread(self._is_read_only, tool_input.get("command", ""))
|
||||
|
||||
async def check_permissions(self, tool_input: dict, _context: PermissionContext) -> PermissionDecision:
|
||||
"""Resolve permission automatically, without an ASK decision."""
|
||||
|
|
@ -194,7 +205,9 @@ class AutoBash(Bash):
|
|||
|
||||
async def call(self, command: str, description: str = "", timeout: int = 60000):
|
||||
"""Apply the read-only guard when configured, then run the command."""
|
||||
if self.is_read_only and (not isinstance(command, str) or not self._is_read_only(command)):
|
||||
if self.is_read_only and (
|
||||
not isinstance(command, str) or not await asyncio.to_thread(self._is_read_only, command)
|
||||
):
|
||||
yield ToolChunk(
|
||||
content=[TextBlock(text="Permission denied: only read-only shell commands are allowed.")],
|
||||
state=ToolResultState.ERROR,
|
||||
|
|
@ -215,9 +228,6 @@ class AutoBashStep(BaseStep):
|
|||
command = self.context.get("command", "")
|
||||
timeout = self.context.get("timeout", self.kwargs.get("timeout", 60000))
|
||||
is_read_only = self.context.get("is_read_only", self.kwargs.get("is_read_only", True))
|
||||
read_only_paths = self.context.get("read_only_paths", self.kwargs.get("read_only_paths"))
|
||||
writable_paths = self.context.get("writable_paths", self.kwargs.get("writable_paths"))
|
||||
memory_limit_bytes = self.context.get("memory_limit_bytes", self.kwargs.get("memory_limit_bytes"))
|
||||
if not isinstance(command, str) or not command.strip():
|
||||
self.context.response.success = False
|
||||
self.context.response.answer = "command is required"
|
||||
|
|
@ -232,16 +242,7 @@ class AutoBashStep(BaseStep):
|
|||
return self.context.response
|
||||
|
||||
try:
|
||||
backend = (
|
||||
create_file_sandbox_backend(
|
||||
cwd=self.workspace_path,
|
||||
read_only_paths=[] if read_only_paths is None else read_only_paths,
|
||||
writable_paths=[] if writable_paths is None else writable_paths,
|
||||
memory_limit_bytes=memory_limit_bytes,
|
||||
)
|
||||
if read_only_paths is not None or writable_paths is not None
|
||||
else None
|
||||
)
|
||||
backend = file_sandbox_for_step(self)
|
||||
except (TypeError, ValueError, RuntimeError) as exc:
|
||||
self.context.response.success = False
|
||||
self.context.response.answer = str(exc)
|
||||
|
|
|
|||
|
|
@ -30,10 +30,10 @@ application_defaults:
|
|||
type: array
|
||||
items: { type: string }
|
||||
description: "Writable host paths; sandbox paths match host paths"
|
||||
memory_limit_bytes:
|
||||
memory_limit_mib:
|
||||
type: integer
|
||||
minimum: 0
|
||||
description: "Linux cgroup memory limit in bytes (default 4 GiB); macOS warns and ignores nonzero values"
|
||||
description: "Linux cgroup memory limit in MiB (default 4096; 0 disables it); requires a path policy; macOS warns and ignores nonzero values"
|
||||
required:
|
||||
- command
|
||||
steps:
|
||||
|
|
|
|||
|
|
@ -1,7 +1,9 @@
|
|||
"""Shell job and its optional read-only execution guard."""
|
||||
|
||||
import asyncio
|
||||
import inspect
|
||||
import subprocess
|
||||
import threading
|
||||
from importlib.metadata import EntryPoint
|
||||
from pathlib import Path
|
||||
|
||||
|
|
@ -139,6 +141,16 @@ async def test_unsupported_platform_runs_without_paths_but_rejects_path_policy(t
|
|||
assert restricted.answer == "No file sandbox backend for unsupported"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize("limit", [0, 1, -1])
|
||||
async def test_step_rejects_memory_limit_without_path_policy(tmp_path, limit):
|
||||
"""An explicitly supplied limit cannot be ignored by host execution."""
|
||||
step = AutoBashStep(app_context=ApplicationContext(workspace_dir=str(tmp_path)))
|
||||
response = await step(command="echo should-not-run", is_read_only=False, memory_limit_mib=limit)
|
||||
assert response.success is False
|
||||
assert response.answer == "memory_limit_mib requires read_only_paths or writable_paths to enable the sandbox"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_step_configuration_can_disable_read_only(tmp_path):
|
||||
"""Step configuration applies when the job does not supply the flag."""
|
||||
|
|
@ -174,6 +186,28 @@ async def test_permission_decisions_never_ask(tmp_path):
|
|||
assert denied.behavior == PermissionBehavior.DENY
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_read_only_parsing_does_not_block_event_loop(tmp_path, monkeypatch):
|
||||
"""A slow synchronous parser runs in a worker thread during permission checks."""
|
||||
started = threading.Event()
|
||||
release = threading.Event()
|
||||
|
||||
def slow_parse(_command):
|
||||
started.set()
|
||||
release.wait(timeout=2)
|
||||
return True
|
||||
|
||||
tool = AutoBash(cwd=tmp_path)
|
||||
monkeypatch.setattr(tool, "_is_read_only", slow_parse)
|
||||
task = asyncio.create_task(tool.check_read_only({"command": "echo hello"}))
|
||||
try:
|
||||
assert await asyncio.wait_for(asyncio.to_thread(started.wait), timeout=1)
|
||||
assert not task.done()
|
||||
finally:
|
||||
release.set()
|
||||
assert await asyncio.wait_for(task, timeout=1)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_unrestricted_tool_allows_commands_without_read_only_check(tmp_path, monkeypatch):
|
||||
"""The unrestricted tool never invokes the read-only parser."""
|
||||
|
|
@ -244,6 +278,7 @@ def test_home_manifest_declares_step_and_job():
|
|||
assert job["steps"] == [{"backend": "auto_bash_step"}]
|
||||
assert job["parameters"]["properties"]["read_only_paths"]["type"] == "array"
|
||||
assert job["parameters"]["properties"]["writable_paths"]["type"] == "array"
|
||||
assert job["parameters"]["properties"]["memory_limit_mib"]["minimum"] == 0
|
||||
|
||||
|
||||
def test_home_plugin_registers_job_and_step(monkeypatch):
|
||||
|
|
@ -340,6 +375,36 @@ async def test_git_configured_helpers_cannot_write(tmp_path, helper):
|
|||
assert all(chunk.state != ToolResultState.ERROR for chunk in chunks)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_global_git_lfs_config_allows_metadata_queries(tmp_path, monkeypatch):
|
||||
"""A global filter alone must not block Git queries that never use it."""
|
||||
repo = tmp_path / "repo"
|
||||
repo.mkdir()
|
||||
subprocess.run(["git", "init", "-q", str(repo)], check=True, capture_output=True)
|
||||
(repo / "note.txt").write_text("hello\n", encoding="utf-8")
|
||||
subprocess.run(["git", "add", "note.txt"], cwd=repo, check=True, capture_output=True)
|
||||
|
||||
global_config = tmp_path / "global.gitconfig"
|
||||
subprocess.run(
|
||||
["git", "config", "--file", str(global_config), "filter.lfs.process", "/bin/false"],
|
||||
check=True,
|
||||
capture_output=True,
|
||||
)
|
||||
monkeypatch.setenv("GIT_CONFIG_GLOBAL", str(global_config))
|
||||
monkeypatch.setenv("GIT_CONFIG_NOSYSTEM", "1")
|
||||
tool = AutoBash(cwd=repo)
|
||||
|
||||
for command, expected in (("git rev-parse --git-dir", ".git"), ("git ls-files", "note.txt")):
|
||||
chunks = [chunk async for chunk in tool.call(command)]
|
||||
assert chunks
|
||||
assert all(chunk.state != ToolResultState.ERROR for chunk in chunks)
|
||||
assert expected in "".join(block.text for chunk in chunks for block in chunk.content)
|
||||
|
||||
chunks = [chunk async for chunk in tool.call("git diff")]
|
||||
assert chunks[-1].state == ToolResultState.ERROR
|
||||
assert "external filters" in chunks[-1].content[0].text
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_allow_and_denied_command_options_are_combined(tmp_path, monkeypatch):
|
||||
"""Allow constrains the first argument while denied still checks later flags."""
|
||||
|
|
|
|||
|
|
@ -142,10 +142,10 @@ jobs:
|
|||
python_timeout:
|
||||
type: number
|
||||
description: "Execution timeout in seconds; defaults to 60."
|
||||
memory_limit_bytes:
|
||||
memory_limit_mib:
|
||||
type: integer
|
||||
minimum: 0
|
||||
description: "Linux memory limit in bytes (default 4 GiB); macOS ignores nonzero values"
|
||||
description: "Linux memory limit in MiB (default 4096; 0 disables it); requires a path policy; macOS ignores nonzero values"
|
||||
required:
|
||||
- code
|
||||
steps:
|
||||
|
|
|
|||
23
reme/steps/common/file_sandbox.py
Normal file
23
reme/steps/common/file_sandbox.py
Normal file
|
|
@ -0,0 +1,23 @@
|
|||
"""Resolve an invocation's optional file sandbox from injected job parameters."""
|
||||
|
||||
from ...utils.file_sandbox import create_file_sandbox_backend
|
||||
from ...utils.file_sandbox.file_sandbox_backend import FileSandboxBackend
|
||||
from ..base_step import BaseStep
|
||||
|
||||
|
||||
def file_sandbox_for_step(step: BaseStep) -> FileSandboxBackend | None:
|
||||
"""Keep path and memory policy selection identical across executor steps."""
|
||||
assert step.context is not None
|
||||
read_only_paths = step.context.get("read_only_paths", step.kwargs.get("read_only_paths"))
|
||||
writable_paths = step.context.get("writable_paths", step.kwargs.get("writable_paths"))
|
||||
memory_limit_mib = step.context.get("memory_limit_mib", step.kwargs.get("memory_limit_mib"))
|
||||
if read_only_paths is None and writable_paths is None:
|
||||
if memory_limit_mib is not None:
|
||||
raise ValueError("memory_limit_mib requires read_only_paths or writable_paths to enable the sandbox")
|
||||
return None
|
||||
return create_file_sandbox_backend(
|
||||
cwd=step.workspace_path,
|
||||
read_only_paths=[] if read_only_paths is None else read_only_paths,
|
||||
writable_paths=[] if writable_paths is None else writable_paths,
|
||||
memory_limit_mib=memory_limit_mib,
|
||||
)
|
||||
|
|
@ -5,10 +5,10 @@ import sys
|
|||
from dataclasses import dataclass
|
||||
from typing import Any
|
||||
|
||||
from ...utils.file_sandbox import create_file_sandbox_backend
|
||||
from ...utils.file_sandbox.file_sandbox_backend import FileSandboxBackend
|
||||
from ..base_step import BaseStep
|
||||
from ...components import R
|
||||
from .file_sandbox import file_sandbox_for_step
|
||||
|
||||
DEFAULT_TIMEOUT = 60.0
|
||||
|
||||
|
|
@ -41,20 +41,8 @@ class PythonExecuteStep(BaseStep):
|
|||
self.context.response.answer = timeout_error
|
||||
return self.context.response
|
||||
|
||||
read_only_paths = self.context.get("read_only_paths", self.kwargs.get("read_only_paths"))
|
||||
writable_paths = self.context.get("writable_paths", self.kwargs.get("writable_paths"))
|
||||
memory_limit_bytes = self.context.get("memory_limit_bytes", self.kwargs.get("memory_limit_bytes"))
|
||||
try:
|
||||
backend = (
|
||||
create_file_sandbox_backend(
|
||||
cwd=self.workspace_path,
|
||||
read_only_paths=[] if read_only_paths is None else read_only_paths,
|
||||
writable_paths=[] if writable_paths is None else writable_paths,
|
||||
memory_limit_bytes=memory_limit_bytes,
|
||||
)
|
||||
if read_only_paths is not None or writable_paths is not None
|
||||
else None
|
||||
)
|
||||
backend = file_sandbox_for_step(self)
|
||||
except (TypeError, ValueError, RuntimeError) as exc:
|
||||
self.context.response.success = False
|
||||
self.context.response.answer = str(exc)
|
||||
|
|
|
|||
|
|
@ -7,12 +7,12 @@ from .path_bubble_backend import PathBubbleBackend
|
|||
from .path_seatbelt_backend import PathSeatbeltBackend
|
||||
|
||||
|
||||
def create_file_sandbox_backend(*, memory_limit_bytes: int | None = None, **kwargs) -> FileSandboxBackend:
|
||||
def create_file_sandbox_backend(*, memory_limit_mib: int | None = None, **kwargs) -> FileSandboxBackend:
|
||||
"""Construct the platform backend; never silently execute without isolation."""
|
||||
if sys.platform.startswith("linux"):
|
||||
limit = 4 * 1024**3 if memory_limit_bytes is None else memory_limit_bytes
|
||||
return PathBubbleBackend(memory_limit_bytes=limit, **kwargs)
|
||||
limit = 4096 if memory_limit_mib is None else memory_limit_mib
|
||||
return PathBubbleBackend(memory_limit_mib=limit, **kwargs)
|
||||
if sys.platform == "darwin":
|
||||
limit = 0 if memory_limit_bytes is None else memory_limit_bytes
|
||||
return PathSeatbeltBackend(memory_limit_bytes=limit, **kwargs)
|
||||
limit = 0 if memory_limit_mib is None else memory_limit_mib
|
||||
return PathSeatbeltBackend(memory_limit_mib=limit, **kwargs)
|
||||
raise RuntimeError(f"No file sandbox backend for {sys.platform}")
|
||||
|
|
|
|||
|
|
@ -88,7 +88,17 @@ class FileSandboxBackend(BackendBase):
|
|||
) -> ExecResult:
|
||||
"""Execute an argv list with a timeout inside the platform sandbox."""
|
||||
process = await self._start_process(command, cwd or self._cwd)
|
||||
communication = asyncio.create_task(process.communicate())
|
||||
return await self._communicate(process, timeout=timeout)
|
||||
|
||||
async def _communicate(
|
||||
self,
|
||||
process: asyncio.subprocess.Process,
|
||||
*,
|
||||
data: bytes | None = None,
|
||||
timeout: float | None = None,
|
||||
) -> ExecResult:
|
||||
"""Drain output and release process resources on success, timeout, or cancellation."""
|
||||
communication = asyncio.create_task(process.communicate() if data is None else process.communicate(data))
|
||||
try:
|
||||
stdout, stderr = await asyncio.wait_for(asyncio.shield(communication), timeout=timeout)
|
||||
except TimeoutError:
|
||||
|
|
@ -152,13 +162,6 @@ class FileSandboxBackend(BackendBase):
|
|||
self._cwd,
|
||||
stdin=asyncio.subprocess.PIPE,
|
||||
)
|
||||
communication = asyncio.create_task(process.communicate(data))
|
||||
try:
|
||||
_stdout, stderr = await asyncio.shield(communication)
|
||||
except BaseException:
|
||||
await self._stop_process(process, communication)
|
||||
raise
|
||||
finally:
|
||||
self._release_process(process)
|
||||
if process.returncode != 0:
|
||||
raise OSError(stderr.decode(errors="replace"))
|
||||
result = await self._communicate(process, data=data)
|
||||
if not result.ok():
|
||||
raise OSError(result.stderr.decode(errors="replace"))
|
||||
|
|
|
|||
|
|
@ -30,11 +30,11 @@ class PathBubbleBackend(FileSandboxBackend):
|
|||
cwd: str | os.PathLike[str],
|
||||
read_only_paths: list[str],
|
||||
writable_paths: list[str],
|
||||
memory_limit_bytes: int = 4 * 1024**3,
|
||||
memory_limit_mib: int = 4096,
|
||||
) -> None:
|
||||
if isinstance(memory_limit_bytes, bool) or not isinstance(memory_limit_bytes, int) or memory_limit_bytes < 0:
|
||||
raise ValueError("memory_limit_bytes must be a non-negative integer")
|
||||
self._memory_limit_bytes = memory_limit_bytes
|
||||
if isinstance(memory_limit_mib, bool) or not isinstance(memory_limit_mib, int) or memory_limit_mib < 0:
|
||||
raise ValueError("memory_limit_mib must be a non-negative integer")
|
||||
self._memory_limit_mib = memory_limit_mib
|
||||
self._process_cgroups: dict[int, Path] = {}
|
||||
super().__init__(cwd=cwd, read_only_paths=read_only_paths, writable_paths=writable_paths)
|
||||
|
||||
|
|
@ -73,7 +73,7 @@ class PathBubbleBackend(FileSandboxBackend):
|
|||
if shutil.which("bwrap") is None:
|
||||
raise RuntimeError("PathBubbleBackend requires bwrap on PATH")
|
||||
argv = self._bwrap_argv(command, cwd)
|
||||
group = self._create_cgroup() if self._memory_limit_bytes else None
|
||||
group = self._create_cgroup() if self._memory_limit_mib else None
|
||||
try:
|
||||
if group is not None:
|
||||
argv = [sys.executable, "-I", "-S", "-c", _CGROUP_EXEC, str(group / "cgroup.procs"), *argv]
|
||||
|
|
@ -104,7 +104,7 @@ class PathBubbleBackend(FileSandboxBackend):
|
|||
group = parent / f"reme-{uuid.uuid4().hex}"
|
||||
group.mkdir()
|
||||
try:
|
||||
(group / "memory.max").write_text(str(self._memory_limit_bytes), encoding="ascii")
|
||||
(group / "memory.max").write_text(str(self._memory_limit_mib * 1024**2), encoding="ascii")
|
||||
except BaseException:
|
||||
group.rmdir()
|
||||
raise
|
||||
|
|
@ -125,6 +125,3 @@ class PathBubbleBackend(FileSandboxBackend):
|
|||
group.rmdir()
|
||||
except OSError as cleanup_error:
|
||||
raise RuntimeError(f"could not remove sandbox cgroup {group}: {cleanup_error}") from exc
|
||||
|
||||
# Keep the existing helper name for callers that manage a process directly.
|
||||
_release_cgroup = _release_process
|
||||
|
|
|
|||
|
|
@ -20,13 +20,13 @@ class PathSeatbeltBackend(FileSandboxBackend):
|
|||
cwd: str | os.PathLike[str],
|
||||
read_only_paths: list[str],
|
||||
writable_paths: list[str],
|
||||
memory_limit_bytes: int = 0,
|
||||
memory_limit_mib: int = 0,
|
||||
) -> None:
|
||||
if isinstance(memory_limit_bytes, bool) or not isinstance(memory_limit_bytes, int) or memory_limit_bytes < 0:
|
||||
raise ValueError("memory_limit_bytes must be a non-negative integer")
|
||||
if memory_limit_bytes:
|
||||
if isinstance(memory_limit_mib, bool) or not isinstance(memory_limit_mib, int) or memory_limit_mib < 0:
|
||||
raise ValueError("memory_limit_mib must be a non-negative integer")
|
||||
if memory_limit_mib:
|
||||
warnings.warn(
|
||||
"PathSeatbeltBackend does not support memory_limit_bytes; the limit is ignored on macOS",
|
||||
"PathSeatbeltBackend does not support memory_limit_mib; the limit is ignored on macOS",
|
||||
RuntimeWarning,
|
||||
stacklevel=2,
|
||||
)
|
||||
|
|
|
|||
|
|
@ -18,7 +18,7 @@ def test_platform_selection_and_memory_defaults(tmp_path, monkeypatch):
|
|||
monkeypatch.setattr("reme.utils.file_sandbox.sys.platform", "linux")
|
||||
linux = create_file_sandbox_backend(cwd=tmp_path, read_only_paths=[], writable_paths=[])
|
||||
assert isinstance(linux, PathBubbleBackend)
|
||||
assert linux._memory_limit_bytes == 4 * 1024**3 # pylint: disable=protected-access
|
||||
assert linux._memory_limit_mib == 4096 # pylint: disable=protected-access
|
||||
|
||||
monkeypatch.setattr("reme.utils.file_sandbox.sys.platform", "darwin")
|
||||
mac = create_file_sandbox_backend(cwd=tmp_path, read_only_paths=[], writable_paths=[])
|
||||
|
|
@ -28,7 +28,7 @@ def test_platform_selection_and_memory_defaults(tmp_path, monkeypatch):
|
|||
cwd=tmp_path,
|
||||
read_only_paths=[],
|
||||
writable_paths=[],
|
||||
memory_limit_bytes=1024,
|
||||
memory_limit_mib=1024,
|
||||
)
|
||||
assert isinstance(limited, PathSeatbeltBackend)
|
||||
|
||||
|
|
|
|||
|
|
@ -56,15 +56,36 @@ def test_path_bubble_backend_adds_workspace_read_only(tmp_path):
|
|||
|
||||
@pytest.mark.parametrize("limit", [-1, True, 1.5, "4096"])
|
||||
def test_path_bubble_backend_rejects_invalid_memory_limit(tmp_path, limit):
|
||||
"""Memory limits must be non-negative whole bytes."""
|
||||
with pytest.raises(ValueError, match="memory_limit_bytes"):
|
||||
PathBubbleBackend(cwd=tmp_path, read_only_paths=[], writable_paths=[], memory_limit_bytes=limit)
|
||||
"""Memory limits must be non-negative whole MiB."""
|
||||
with pytest.raises(ValueError, match="memory_limit_mib"):
|
||||
PathBubbleBackend(cwd=tmp_path, read_only_paths=[], writable_paths=[], memory_limit_mib=limit)
|
||||
|
||||
|
||||
def test_one_mib_is_written_as_bytes_to_cgroup(tmp_path, monkeypatch):
|
||||
"""The public MiB limit is converted only at the Linux cgroup boundary."""
|
||||
proc_file = tmp_path / "proc-self-cgroup"
|
||||
proc_file.write_text("0::/\n", encoding="ascii")
|
||||
cgroup_root = tmp_path / "cgroup"
|
||||
cgroup_root.mkdir()
|
||||
original_path = Path
|
||||
|
||||
def sandbox_path(value):
|
||||
if value == "/proc/self/cgroup":
|
||||
return proc_file
|
||||
if value == "/sys/fs/cgroup":
|
||||
return cgroup_root
|
||||
return original_path(value)
|
||||
|
||||
monkeypatch.setattr("reme.utils.file_sandbox.path_bubble_backend.Path", sandbox_path)
|
||||
backend = PathBubbleBackend(cwd=tmp_path, read_only_paths=[], writable_paths=[], memory_limit_mib=1)
|
||||
group = backend._create_cgroup() # pylint: disable=protected-access
|
||||
assert (group / "memory.max").read_text(encoding="ascii") == str(1024**2)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_each_execution_gets_separate_memory_cgroup(tmp_path, monkeypatch):
|
||||
"""Each process receives its own cgroup before bubblewrap is executed."""
|
||||
backend = PathBubbleBackend(cwd=tmp_path, read_only_paths=[], writable_paths=[], memory_limit_bytes=1024)
|
||||
backend = PathBubbleBackend(cwd=tmp_path, read_only_paths=[], writable_paths=[], memory_limit_mib=1024)
|
||||
groups = []
|
||||
subprocess_options = []
|
||||
subprocess_commands = []
|
||||
|
|
@ -92,15 +113,15 @@ async def test_each_execution_gets_separate_memory_cgroup(tmp_path, monkeypatch)
|
|||
for argv, group in zip(subprocess_commands, groups):
|
||||
assert argv[:4] == (sys.executable, "-I", "-S", "-c")
|
||||
assert argv[5:7] == (str(group / "cgroup.procs"), "bwrap")
|
||||
backend._release_cgroup(first) # pylint: disable=protected-access
|
||||
backend._release_cgroup(second) # pylint: disable=protected-access
|
||||
backend._release_process(first) # pylint: disable=protected-access
|
||||
backend._release_process(second) # pylint: disable=protected-access
|
||||
assert not any(group.exists() for group in groups)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_zero_memory_limit_skips_cgroup(tmp_path, monkeypatch):
|
||||
"""A zero limit bypasses cgroup creation and child enrollment."""
|
||||
backend = PathBubbleBackend(cwd=tmp_path, read_only_paths=[], writable_paths=[], memory_limit_bytes=0)
|
||||
backend = PathBubbleBackend(cwd=tmp_path, read_only_paths=[], writable_paths=[], memory_limit_mib=0)
|
||||
options = {}
|
||||
|
||||
async def create_process(*_argv, **kwargs):
|
||||
|
|
@ -164,7 +185,7 @@ async def test_python_execute_uses_path_backend_when_paths_supplied(tmp_path, mo
|
|||
assert seen["command"] == [sys.executable, "-c", "print('ok')"]
|
||||
assert seen["cwd"] == str(tmp_path)
|
||||
assert seen["timeout"] == 60
|
||||
assert seen["backend"]._memory_limit_bytes == 4 * 1024**3 # pylint: disable=protected-access
|
||||
assert seen["backend"]._memory_limit_mib == 4096 # pylint: disable=protected-access
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
|
@ -173,16 +194,26 @@ async def test_python_execute_passes_custom_memory_limit(tmp_path, monkeypatch):
|
|||
seen = {}
|
||||
|
||||
async def fake_exec(self, _command, *, cwd=None, timeout=None): # pylint: disable=unused-argument
|
||||
seen["limit"] = self._memory_limit_bytes # pylint: disable=protected-access
|
||||
seen["limit"] = self._memory_limit_mib # pylint: disable=protected-access
|
||||
return ExecResult(exit_code=0, stdout=b"", stderr=b"")
|
||||
|
||||
monkeypatch.setattr(PathBubbleBackend, "exec_shell", fake_exec)
|
||||
step = PythonExecuteStep(app_context=ApplicationContext(workspace_dir=str(tmp_path)))
|
||||
response = await step(code="pass", read_only_paths=[], memory_limit_bytes=0)
|
||||
response = await step(code="pass", read_only_paths=[], memory_limit_mib=0)
|
||||
assert response.success is True
|
||||
assert seen["limit"] == 0
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize("limit", [0, 1, -1])
|
||||
async def test_python_execute_rejects_memory_limit_without_path_policy(tmp_path, limit):
|
||||
"""A requested memory policy must never be silently ignored."""
|
||||
step = PythonExecuteStep(app_context=ApplicationContext(workspace_dir=str(tmp_path)))
|
||||
response = await step(code="print('should not run')", memory_limit_mib=limit)
|
||||
assert response.success is False
|
||||
assert response.answer == "memory_limit_mib requires read_only_paths or writable_paths to enable the sandbox"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_step_rejects_invalid_mount_parameters(tmp_path):
|
||||
"""Malformed path injection cannot silently disable sandboxing."""
|
||||
|
|
@ -214,4 +245,4 @@ def test_job_hides_injected_path_policy():
|
|||
parameters = yaml.safe_load(config_path.read_text(encoding="utf-8"))["jobs"]["python_execute"]["parameters"]
|
||||
assert "read_only_paths" not in parameters["properties"]
|
||||
assert "writable_paths" not in parameters["properties"]
|
||||
assert "default" not in parameters["properties"]["memory_limit_bytes"]
|
||||
assert "default" not in parameters["properties"]["memory_limit_mib"]
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue