fix(tools): symlink-safe exclusive creation for all spill/cache writers

Spill files (terminal overflow, hook context, web_extract full text,
subagent summaries) were written with plain open()/write_text into
predictable directories. A pre-planted symlink at any of those paths
redirected the write onto an arbitrary user-owned file, and raw
pre-redaction terminal/hook spills landed world-readable under the
default umask.

New tools/spill_safety.py helpers create files with
O_CREAT|O_EXCL|O_NOFOLLOW (a link-shaped path fails the write instead of
following it) and overwrite via lstat-checked unlink + exclusive
re-create, so even the redaction rewrite cannot be diverted. Private
tier (0o700 dir / 0o600 file) covers raw terminal and hook spills;
cache/web and cache/delegation keep umask perms because those dirs are
bind-mounted into remote backends that must read them.

Pattern borrowed from DeepSeek Harness dsh-spill-local (MIT):
private root + exclusive owner-only opens for spill artifacts.
This commit is contained in:
Teknium 2026-08-13 10:56:22 -07:00
parent 6def7ce1df
commit 005dfcbfcc
7 changed files with 276 additions and 8 deletions

View File

@ -0,0 +1,114 @@
"""Symlink-refusal and permission tests for tools.spill_safety.
The bug class: spill/cache writers used ``open(path, "w")`` /
``Path.write_text`` in predictable directories, which follows a pre-planted
symlink and redirects the write onto an arbitrary user-owned file. Every
helper must refuse the link (never write through it) while keeping normal
writes byte-identical.
"""
import os
import stat
import sys
import pytest
from tools.spill_safety import (
ensure_spill_dir,
open_exclusive,
write_text_exclusive,
)
posix_only = pytest.mark.skipif(sys.platform == "win32", reason="POSIX perms/symlinks")
def test_write_creates_file_with_content(tmp_path):
target = tmp_path / "spill.txt"
write_text_exclusive(target, "hello\n")
assert target.read_text(encoding="utf-8") == "hello\n"
@posix_only
def test_private_file_is_0600(tmp_path):
target = tmp_path / "spill.txt"
write_text_exclusive(target, "secret", private=True)
assert stat.S_IMODE(os.lstat(target).st_mode) == 0o600
@posix_only
def test_private_dir_is_0700_and_tightened(tmp_path):
d = tmp_path / "spills"
d.mkdir(mode=0o755)
ensure_spill_dir(d, private=True)
assert stat.S_IMODE(os.lstat(d).st_mode) == 0o700
def test_ensure_spill_dir_refuses_symlinked_leaf(tmp_path):
victim = tmp_path / "victim-dir"
victim.mkdir()
link = tmp_path / "spills"
link.symlink_to(victim)
with pytest.raises(OSError):
ensure_spill_dir(link)
def test_refuses_planted_symlink(tmp_path):
"""The core attack: symlink at the spill path must fail, not redirect."""
victim = tmp_path / "victim.txt"
victim.write_text("original")
target = tmp_path / "spill.txt"
target.symlink_to(victim)
with pytest.raises(OSError):
write_text_exclusive(target, "attacker-controlled")
assert victim.read_text() == "original"
def test_refuses_dangling_symlink(tmp_path):
target = tmp_path / "spill.txt"
target.symlink_to(tmp_path / "does-not-exist.txt")
with pytest.raises(OSError):
write_text_exclusive(target, "x")
assert not (tmp_path / "does-not-exist.txt").exists()
def test_overwrite_removes_symlink_not_its_target(tmp_path):
victim = tmp_path / "victim.txt"
victim.write_text("original")
target = tmp_path / "spill.txt"
target.symlink_to(victim)
write_text_exclusive(target, "redacted copy", overwrite=True)
# Link replaced by a real file; the link's target untouched.
assert not target.is_symlink()
assert target.read_text(encoding="utf-8") == "redacted copy"
assert victim.read_text() == "original"
def test_overwrite_replaces_regular_file(tmp_path):
target = tmp_path / "spill.txt"
target.write_text("raw")
write_text_exclusive(target, "redacted", overwrite=True)
assert target.read_text(encoding="utf-8") == "redacted"
def test_overwrite_refuses_directory(tmp_path):
target = tmp_path / "spill.txt"
target.mkdir()
with pytest.raises(OSError):
write_text_exclusive(target, "x", overwrite=True)
assert target.is_dir()
def test_exclusive_create_fails_on_existing_without_overwrite(tmp_path):
target = tmp_path / "spill.txt"
target.write_text("first")
with pytest.raises(OSError):
write_text_exclusive(target, "second")
assert target.read_text() == "first"
def test_open_exclusive_streaming_write(tmp_path):
target = tmp_path / "spill.log"
with open_exclusive(target, errors="replace") as fh:
fh.write("chunk1")
fh.write("chunk2")
assert target.read_text(encoding="utf-8") == "chunk1chunk2"

View File

@ -2087,7 +2087,12 @@ def _spill_summary_to_file(task_index: int, summary: str) -> Optional[str]:
cache_dir.mkdir(parents=True, exist_ok=True)
ts = _dt.datetime.now().strftime("%Y%m%d_%H%M%S_%f")
path = cache_dir / f"subagent-summary-{task_index}-{ts}.txt"
path.write_text(summary, encoding="utf-8")
from tools.spill_safety import write_text_exclusive
# Exclusive symlink-refusing create; not private because
# cache/delegation is bind-mounted read-only into remote backends
# whose container UID must be able to read it.
write_text_exclusive(path, summary, private=False)
return str(path)
except Exception as exc:
logger.debug("Failed to spill subagent summary to file: %s", exc)

View File

@ -112,8 +112,15 @@ class _BoundedOutputCollector:
return
try:
if self._spill_fh is None:
self._spill_path.parent.mkdir(parents=True, exist_ok=True)
self._spill_fh = open(self._spill_path, "w", encoding="utf-8", errors="replace")
from tools.spill_safety import ensure_spill_dir, open_exclusive
# Raw pre-redaction output: private perms + symlink-refusing
# exclusive create (a planted link must fail the spill, never
# redirect the write).
ensure_spill_dir(self._spill_path.parent, private=True)
self._spill_fh = open_exclusive(
self._spill_path, private=True, errors="replace"
)
# Backfill everything retained so far so the file holds the
# stream from byte 0, not just from the overflow point.
backlog = "".join(self._head) + "".join(self._tail)

View File

@ -208,12 +208,21 @@ def spill_if_oversized(
saved_path: Optional[str] = None
try:
spill_dir = _resolve_spill_dir(directory_override, session_id)
spill_dir.mkdir(parents=True, exist_ok=True)
from tools.spill_safety import ensure_spill_dir, write_text_exclusive
# Hook context may embed raw secrets: private dir/file perms, and an
# exclusive symlink-refusing create so a planted link can't redirect
# the write (predictable per-session directory).
ensure_spill_dir(spill_dir, private=True)
filename = f"{uuid.uuid4().hex}.txt"
spill_path = spill_dir / filename
# Write the raw text plus a trailing newline so tail readers
# (``tail -f``, editors) don't report "missing newline".
spill_path.write_text(text if text.endswith("\n") else text + "\n", encoding="utf-8")
write_text_exclusive(
spill_path,
text if text.endswith("\n") else text + "\n",
private=True,
)
saved_path = str(spill_path)
except Exception as exc:
logger.warning("hook output spill failed: %s", exc)

118
tools/spill_safety.py Normal file
View File

@ -0,0 +1,118 @@
"""Symlink-safe creation helpers for spill/cache files.
Spill files (oversized terminal output, hook context, subagent summaries,
web_extract full text) are written into predictable, world-discoverable
directories under ``~/.hermes``. A plain ``open(path, "w")`` /
``Path.write_text`` there follows a pre-planted symlink, letting any local
process that can write to the spill directory redirect our write onto an
arbitrary file owned by the user (``~/.bashrc``, ``authorized_keys``, ...).
Every helper here refuses symlinks by construction:
* New files are created with ``O_CREAT | O_EXCL``, which fails on ANY
existing path — including a dangling symlink — instead of following it.
* Overwrites first remove the existing path via ``lstat`` + ``unlink``
(deleting a link deletes the link, never its target), then re-create
exclusively. The check-then-create pair cannot be raced into following a
link because creation itself is exclusive.
Two privacy tiers:
* ``private=True`` (default) also forces ``0o700`` directories and
``0o600`` files — for spills that may hold raw, pre-redaction secrets
(terminal output, hook context).
* ``private=False`` keeps umask-default permissions — for cache dirs that
are bind-mounted into remote terminal backends (Docker/Modal/SSH via
``credential_files._CACHE_DIRS``), where a non-root container UID must
still be able to read them (public web content, delegation summaries).
Disk failures are the caller's concern: helpers raise ``OSError`` and the
call sites keep their existing best-effort handling.
"""
from __future__ import annotations
import os
import stat
from pathlib import Path
from typing import IO
__all__ = [
"ensure_spill_dir",
"open_exclusive",
"write_text_exclusive",
]
# O_NOFOLLOW is POSIX-only; harmless to omit on Windows since O_EXCL alone
# already refuses every pre-existing path there too.
_O_NOFOLLOW = getattr(os, "O_NOFOLLOW", 0)
def ensure_spill_dir(path: Path, *, private: bool = True) -> Path:
"""Create ``path`` (and parents) as a directory, refusing symlinks.
With ``private=True`` the leaf directory is created ``0o700`` and an
already-existing leaf is tightened to ``0o700``. Raises ``OSError`` if
the leaf exists and is not a real directory (e.g. a planted symlink).
"""
path = Path(path)
if private:
path.mkdir(mode=0o700, parents=True, exist_ok=True)
else:
path.mkdir(parents=True, exist_ok=True)
st = os.lstat(path)
if not stat.S_ISDIR(st.st_mode):
raise OSError(f"spill dir is not a directory (symlink?): {path}")
if private and stat.S_IMODE(st.st_mode) != 0o700:
os.chmod(path, 0o700)
return path
def open_exclusive(
path: Path,
*,
private: bool = True,
overwrite: bool = False,
encoding: str = "utf-8",
errors: str = "strict",
) -> IO[str]:
"""Open ``path`` for writing via exclusive create; never follows a link.
``overwrite=True`` first unlinks an existing path (``lstat``-checked so
only the link itself is ever removed, and real directories are refused),
then creates exclusively — so even the overwrite path cannot be
redirected through a symlink.
"""
path = Path(path)
if overwrite:
try:
st = os.lstat(path)
except FileNotFoundError:
pass
else:
if stat.S_ISDIR(st.st_mode):
raise OSError(f"refusing to overwrite a directory: {path}")
os.unlink(path)
mode = 0o600 if private else 0o666 # non-private honors umask
fd = os.open(path, os.O_WRONLY | os.O_CREAT | os.O_EXCL | _O_NOFOLLOW, mode)
try:
return os.fdopen(fd, "w", encoding=encoding, errors=errors)
except Exception:
os.close(fd)
raise
def write_text_exclusive(
path: Path,
text: str,
*,
private: bool = True,
overwrite: bool = False,
encoding: str = "utf-8",
errors: str = "strict",
) -> None:
"""``Path.write_text`` equivalent that refuses to follow symlinks."""
with open_exclusive(
path, private=private, overwrite=overwrite, encoding=encoding, errors=errors
) as fh:
fh.write(text)

View File

@ -3434,9 +3434,17 @@ def terminal_tool(
try:
_sp = Path(spill_file_path)
raw_spill = _sp.read_text(encoding="utf-8", errors="replace")
_sp.write_text(
from tools.spill_safety import write_text_exclusive
# Rewrite in place via lstat-checked unlink + exclusive
# create so the redacted copy can't be diverted through a
# symlink planted between the collector's write and now.
write_text_exclusive(
_sp,
redact_terminal_output(strip_ansi(raw_spill), command),
encoding="utf-8", errors="replace",
private=True,
overwrite=True,
errors="replace",
)
result_dict["output_total_chars"] = spill_total_chars
result_dict["full_output_path"] = spill_file_path

View File

@ -506,7 +506,14 @@ def _store_full_text(url: str, content: str) -> Optional[str]:
+ f"\n\n[... stored copy truncated at {MAX_STORED_TEXT_CHARS:,} chars "
f"of {len(content):,}; re-extract a more specific URL for the rest ...]"
)
path.write_text(content, encoding="utf-8")
from tools.spill_safety import write_text_exclusive
# Deterministic filename in a well-known dir: refuse symlinks via
# lstat-unlink + exclusive create. Re-extraction of the same URL
# legitimately overwrites (same slug-digest name). Not private:
# cache/web is bind-mounted into remote backends whose container UID
# must be able to read it, and content is fetched public text.
write_text_exclusive(path, content, private=False, overwrite=True)
return str(path)
except Exception as exc: # noqa: BLE001
logger.debug("Failed to store full web_extract text for %s: %s", url, exc)