mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-08 13:12:08 +00:00
fix(vision): bound the sandbox exec-read at the ingest cap
The container exec-read piped the whole file through base64 with no size guard — the 50MB cap was only enforced host-side AFTER the full payload had already streamed into host memory. A prompt-injected read of a huge container file (or /dev/zero) could balloon the gateway process. head -c (cap+1) bounds the read inside the sandbox; the +1 byte lets the host distinguish at-cap from over-cap and reject with SourceTooLarge. Input redirect replaces 'base64 --' (no argv exposure at all for leading-dash paths). Docker integration tests re-verified live.
This commit is contained in:
parent
6c068358e4
commit
0d27d2ed14
2 changed files with 35 additions and 5 deletions
|
|
@ -151,7 +151,7 @@ class TestNonLocalBackendConfinement:
|
|||
assert res.origin == "container"
|
||||
assert res.data == PNG
|
||||
assert b"HOST-PRIVATE-KEY" not in res.data
|
||||
assert "base64 --" in calls["cmd"] # option-injection-safe form
|
||||
assert "head -c" in calls["cmd"] and "< " in calls["cmd"] # bounded, redirect-safe form
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_non_cache_path_fails_closed_without_sandbox(self, tmp_path, monkeypatch):
|
||||
|
|
@ -192,8 +192,9 @@ class TestNonLocalBackendConfinement:
|
|||
|
||||
class TestExecReadSafety:
|
||||
@pytest.mark.asyncio
|
||||
async def test_exec_read_uses_option_terminator(self, tmp_path, monkeypatch):
|
||||
"""Leading-dash paths must not be parsed as base64 options."""
|
||||
async def test_exec_read_is_bounded_and_redirect_safe(self, tmp_path, monkeypatch):
|
||||
"""Leading-dash paths go through an input redirect (no argv exposure)
|
||||
and the read is size-bounded via head -c."""
|
||||
home = tmp_path / "hermes"
|
||||
isrc = _reload(monkeypatch, home)
|
||||
monkeypatch.setenv("TERMINAL_ENV", "docker")
|
||||
|
|
@ -207,7 +208,26 @@ class TestExecReadSafety:
|
|||
return_value=SimpleNamespace(execute=fake_execute)):
|
||||
await isrc.resolve_image_source(
|
||||
"/workspace/-i-etc-shadow.png", isrc.ResolveContext(task_id="t1"))
|
||||
assert "base64 -- " in captured["cmd"]
|
||||
assert f"head -c {isrc._MAX_INGEST_BYTES + 1} < " in captured["cmd"]
|
||||
assert "'-i-etc-shadow.png'" in captured["cmd"] or "-i-etc-shadow.png" in captured["cmd"]
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_exec_read_over_cap_rejected(self, tmp_path, monkeypatch):
|
||||
"""A sandbox file larger than the ingest cap is rejected, not embedded."""
|
||||
home = tmp_path / "hermes"
|
||||
isrc = _reload(monkeypatch, home)
|
||||
monkeypatch.setenv("TERMINAL_ENV", "docker")
|
||||
# head -c returns cap+1 bytes for an oversized file.
|
||||
over = base64.b64encode(b"\x89PNG\r\n\x1a\n" + b"\x00" * (isrc._MAX_INGEST_BYTES - 7)).decode()
|
||||
|
||||
def fake_execute(cmd, **kw):
|
||||
return {"returncode": 0, "output": over}
|
||||
|
||||
with patch("tools.image_source._get_active_env",
|
||||
return_value=SimpleNamespace(execute=fake_execute)):
|
||||
with pytest.raises(isrc.SourceTooLarge):
|
||||
await isrc.resolve_image_source(
|
||||
"/workspace/huge.png", isrc.ResolveContext(task_id="t1"))
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_exec_read_nonzero_returncode_raises(self, tmp_path, monkeypatch):
|
||||
|
|
|
|||
|
|
@ -277,16 +277,26 @@ async def _resolve_container_fallback(p: Path, ctx: ResolveContext, src: str) ->
|
|||
f"session is available to read it",
|
||||
src=src, origin="container")
|
||||
|
||||
# Bound the read INSIDE the sandbox: head -c caps at ingest-limit+1 bytes
|
||||
# so a huge file (or /dev/zero) can't stream unbounded base64 into host
|
||||
# memory — the +1 byte lets us distinguish "exactly at the cap" from
|
||||
# "over the cap" after decode. The input redirect (< path) avoids argv
|
||||
# entirely, so leading-dash paths can't be parsed as options; base64
|
||||
# -w0 is GNU-only, so pipe through tr -d for BusyBox.
|
||||
# env.execute is a blocking backend exec; keep it off the event loop so a
|
||||
# multi-MB base64 read doesn't stall every other coroutine.
|
||||
qp = shlex.quote(str(p))
|
||||
res = await asyncio.to_thread(
|
||||
env.execute, f"base64 -- {shlex.quote(str(p))} | tr -d '\\n'")
|
||||
env.execute,
|
||||
f"head -c {_MAX_INGEST_BYTES + 1} < {qp} | base64 | tr -d '\\n'")
|
||||
if res.get("returncode", 1) != 0:
|
||||
raise SourceNotFound(f"could not read '{p}' inside the sandbox", src=src, origin="container")
|
||||
try:
|
||||
data = base64.b64decode(res.get("output", ""), validate=True)
|
||||
except Exception as exc:
|
||||
raise NotAnImage(f"sandbox returned non-image data for '{p}': {exc}", src=src)
|
||||
if len(data) > _MAX_INGEST_BYTES:
|
||||
raise SourceTooLarge("image exceeds size limit", src=src, origin="container")
|
||||
return _finalize(data, "", "container", src)
|
||||
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue