The read_file device guard now walks symlink hops before the file operation
layer, but that hop walk still interpreted relative paths against the Python
process cwd. In sessions where TERMINAL_CWD points at the task workspace, a
relative workspace symlink to a blocked alias such as /dev/../dev/stdin could
therefore miss the intermediate device target before later task-cwd resolution.
Anchor relative device checks to the task base before symlink-hop inspection so
the pre-I/O guard sees the same workspace path that read_file would otherwise
read. Absolute device paths and the existing final realpath fallback remain
unchanged.
Refs #10141
Refs #29158
The salvaged #19820 unifies the write_file guard under
_is_internal_file_tool_content with the message 'internal read_file
display text'. Two tests added to test_file_read_guards.py after the PR
branch point still asserted the old 'status text' wording. Update them
to match the new (correct, more general) message.
Security-hardening fix for the read_file device guard, not a new sandbox
boundary. The guard already rejects direct device paths and upstream now
has a resolved-path pass for workspace symlinks to blocked devices, but
its concrete-path helper still compared the expanded path before
normalization. That leaves residual alias cases where the dangerous path
is visible before final terminal-specific resolution, for example:
1. /dev/../dev/zero and /dev/./urandom should match the blocked-device
list as concrete paths, not only after final realpath;
2. /dev/stdin-style aliases can disappear once realpath follows them
to /proc/self/fd/0 and then to a tty path;
3. a user symlink to /dev/../dev/stdin exposes the dangerous
intermediate target before final resolution, but not necessarily
after it.
Normalize expanded paths before matching and inspect each symlink hop
before falling back to realpath. This preserves the existing /proc fd and
/proc pseudo-file guards while enforcing the intended security invariant:
model-supplied read paths must not reach blocking or infinite device
streams through spelling, normalization, or symlink-hop tricks.
Classification: security hardening / residual bypass fix for the
read_file device blocklist. This is defensive code at the file-tool
boundary, but it fixes a concrete denial-of-service class tracked as
security in #10141 and #29158.
Tests:
- normalized /dev/../dev/zero and /dev/./urandom aliases
- symlink to /dev/../dev/stdin blocked before realpath
- existing symlink-to-device and regular-symlink guards still pass
Fixes#10141Fixes#29158
Remove unused imports (F401) and duplicate/shadowed import
redefinitions (F811) across the codebase using ruff's safe
autofixes. No behavioral changes -- imports only.
- ~1400 safe autofixes applied across 644 files (net -1072 lines)
- __init__.py re-exports preserved (excluded from F401 removal so
public re-export surfaces stay intact)
- Re-exports that are imported or monkeypatched by tests but look
unused in their defining module are kept with explicit # noqa:
F401 (gateway/run.py load_dotenv; run_agent re-exports from
agent.message_sanitization, agent.context_compressor,
agent.retry_utils, agent.prompt_builder, agent.process_bootstrap,
agent.codex_responses_adapter)
- Unsafe F841 (unused-variable) fixes deliberately skipped -- those
can change behavior when the RHS has side effects
- ruff lints remain disabled in pyproject.toml (only PLW1514 is
selected); this is a one-time cleanup, not a config change
Verification:
- python -m compileall: clean
- pytest --collect-only: all 27161 tests collect (zero import errors)
- core entry points import clean (run_agent, model_tools, cli,
toolsets, hermes_state, batch_runner, gateway)
- static scan: every name any test imports directly from an edited
module still resolves
All four failures were broken by the security cluster (#10082 / #10133 /
#4609 / symlink-reject batch) merging on May 25. They were red on
origin/main HEAD when #32042 and #32061 ran, gating PRs that touched
unrelated code.
1) tests/hermes_cli/test_update_zip_symlink_reject.py
test_update_via_zip_accepts_normal_member called the real
_update_via_zip without sandboxing PROJECT_ROOT — so the function's
shutil.copytree() actually copied the fake README from the test ZIP
over the real repo's README.md, which then made
test_readme_mentions_powershell_installer fail in any test run that
happened to pick this test up earlier. Mock PROJECT_ROOT to an
isolated tmp_path / install_dir, stub subprocess so pip/uv reinstall
doesn't actually run, and assert the fake README lands in the
sandbox (not the real tree).
2) tests/tools/test_windows_native_support.py
test_readme_mentions_powershell_installer was the victim of (1) —
nothing wrong with the test itself, the fix in (1) clears it.
3) tests/tools/test_file_read_guards.py
test_proc_fd_other_not_blocked called _is_blocked_device('/proc/self/fd/3')
expecting False. But _is_blocked_device runs realpath() and on
pytest xdist workers fd 3 happens to be dup'd to /dev/urandom
(because the worker subprocess inherits open fds from pytest's
collection pipe machinery). Switch to the lower-level
_is_blocked_device_path which is the path-pattern check the test
actually means to exercise; realpath-resolution coverage already
lives in test_symlink_to_blocked_device_is_blocked.
4) tests/tools/test_transcription_tools.py
Module installed a faster_whisper stub via sys.modules without
setting __spec__, then later @pytest.mark.skipif called
importlib.util.find_spec('faster_whisper') which raises
'ValueError: __spec__ is None' for modules with a None spec attr.
Set __spec__ on the stub to a real ModuleSpec.
Validation: 195/195 green across the 4 affected files.
* fix: reject read_file symlinks to blocking devices
The read_file guard already refused direct device paths such as /dev/zero, but a workspace symlink resolving to one of those devices could still reach the shell-backed read path and hang on wc/head/sed. Keep the literal alias check and add a resolved-path pass so local symlinks to blocked device/fd endpoints are rejected before I/O.
Constraint: Preserve literal /dev/stdin handling before terminal-specific realpath resolution
Confidence: high
Scope-risk: narrow
Tested: pytest tests/tools/test_file_read_guards.py tests/tools/test_file_tools.py -q; python -m compileall tools/file_tools.py tests/tools/test_file_read_guards.py; git diff --check
Signed-off-by: WuKongAI-CMU <210765158+WuKongAI-CMU@users.noreply.github.com>
* Keep file guard tests off sensitive macOS temp paths
The branch now inherits a sensitive-path write guard from upstream main. On macOS, tempfile.mkdtemp() resolves under /private/var/folders, so the new write-path guard fired before the file read dedup assertions could exercise their intended behavior. The tests now create their scratch files inside the worktree temp checkout, outside those system-sensitive prefixes, without changing production behavior.
Constraint: Rebased branch must pass the expanded file read guard suite on macOS.
Rejected: Loosen the production sensitive-path prefix list | broader behavior change unrelated to this PR.
Confidence: high
Scope-risk: narrow
Tested: pytest tests/tools/test_file_read_guards.py -q
---------
Signed-off-by: WuKongAI-CMU <210765158+WuKongAI-CMU@users.noreply.github.com>
Co-authored-by: WuKongAI-CMU <210765158+WuKongAI-CMU@users.noreply.github.com>
The read_file tool and terminal cat can access /proc/self/environ to
recover all process env vars including secrets stripped by the subprocess
blocklist. Output redaction partially mitigates (catches known-format
tokens) but misses custom/proprietary key formats, especially when
values are printed without their key names.
Add /proc/*/environ, /proc/*/cmdline, and /proc/*/maps to the blocked
device paths in _is_blocked_device():
- /proc/*/environ: leaks full process env (API keys, tokens)
- /proc/*/cmdline: leaks command-line args (may contain passwords)
- /proc/*/maps: leaks memory layout (ASLR bypass for exploitation)
Legitimate /proc reads (cpuinfo, meminfo, uptime, version) remain
accessible — the check only blocks per-pid pseudo-files with known
sensitive suffixes.
Complements PR #4432 (PID namespace isolation for child processes)
which prevents children from reading the parent's /proc, but does not
prevent the parent process itself from being read via file tools.
Partially addresses #4427
Changes:
tools/file_tools.py | +6
tests/tools/test_file_read_guards.py | +18 -1
Co-authored-by: dsr-restyn <dsr-restyn@users.noreply.github.com>
read_file's dedup path returned a lightweight stub on re-reads of an
unchanged file, then returned early — so the consecutive-read loop
guard (hard block at count>=4) at the bottom of read_file_tool never
ran for stub-looped calls. Weaker tool-following models (local Qwen3.6
variants in the reported case) ignore the passive 'refer to earlier
result' hint and hammer the same read_file call until iteration budget
runs out.
Track per-key stub returns in task_data['dedup_hits'] and, on the
second stub for the same (path, offset, limit), return a hard BLOCKED
error mirroring the wording the real-read path already uses. A real
read, an intervening non-read tool call (notify_other_tool_call), or
reset_file_dedup (on context compression) all clear the counter so
the guard never stays engaged longer than the actual loop.
Closes#15759
The write_file guard added in #16223 used strict equality against the
internal dedup status message. In practice, the model sometimes
prepends a short note or appends a trailing comment before calling
write_file, which slipped past the strict check.
Broaden the heuristic: reject writes whose stripped content equals
the status message OR contains it and is <=2x its length. Short,
status-dominated writes are always corruption; legitimate docs that
quote the message verbatim are always much longer.
Adds two tests: one for the small-wrapper corruption shape, one
confirming large legitimate files that quote the status still write.
write_file_tool and patch_tool both call _update_read_timestamp to
refresh the staleness tracker after writing, but they never invalidate
the dedup cache entries for the written path. The dedup cache keys are
(resolved_path, offset, limit) → mtime tuples populated by read_file_tool.
On filesystems where a read and write land in the same mtime second (or
when mtime granularity is 1s), the cached and current mtime are equal,
so the dedup check incorrectly returns a 'File unchanged since last
read' stub — even though the file was just overwritten.
The agent then sees stale content (or a stale 'File not found' error)
and enters expensive error-recovery loops, burning API calls.
Fix: add _invalidate_dedup_for_path(filepath, task_id) that removes all
dedup entries whose resolved path matches the written file. Called from
_update_read_timestamp so both write_file_tool and patch_tool benefit
automatically. Scoped to the writing task_id — other tasks' caches are
not affected.
6 regression tests added covering:
- read→write→read within same mtime second (core #13144 scenario)
- invalidation across all offset/limit combinations
- isolation: writing file A does not invalidate file B's cache
- isolation: writing in task A does not invalidate task B's cache
- _invalidate_dedup_for_path safety on missing task / empty dedup
All 25 tests pass (19 existing + 6 new).
Fixes#13144
* feat(file_tools): harden read_file with size guard, dedup, and device blocking
Three improvements to read_file_tool to reduce wasted context tokens and
prevent process hangs:
1. Character-count guard: reads that produce more than 100K characters
(≈25-35K tokens across tokenisers) are rejected with an error that
tells the model to use offset+limit for a smaller range. The
effective cap is min(file_size, 100K) so small files that happen to
have long lines aren't over-penalised. Large truncated files also
get a hint nudging toward targeted reads.
2. File-read deduplication: when the same (path, offset, limit) is read
a second time and the file hasn't been modified (mtime unchanged),
return a lightweight stub instead of re-sending the full content.
Writes and patches naturally change mtime, so post-edit reads always
return fresh content. The dedup cache is cleared on context
compression — after compression the original read content is
summarised away, so the model needs the full content again.
3. Device path blocking: paths like /dev/zero, /dev/random, /dev/stdin
etc. are rejected before any I/O to prevent process hangs from
infinite-output or blocking-input devices.
Tests: 17 new tests covering all three features plus the dedup-reset-
on-compression integration. All 52 file-read tests pass (35 existing +
17 new). Full tool suite (2124 tests) passes with 0 failures.
* feat: make file_read_max_chars configurable, add docs
Add file_read_max_chars to DEFAULT_CONFIG (default 100K). read_file_tool
reads this on first call and caches for the process lifetime. Users on
large-context models can raise it; users on small local models can lower it.
Also adds a 'File Read Safety' section to the configuration docs
explaining the char limit, dedup behavior, and example values.
