feat(agents): expose sandboxed file helpers to inline code #103
@@ -9,6 +9,16 @@ and this project adheres to [Clever Semantic Versioning](https://www.w3.org/subm
|
||||
|
||||
### Added
|
||||
|
||||
- **Sandboxed `read_file`/`write_file` Helpers for Inline Code (issue #93)** (`agents/file_access.py`, `agents/tool.py`, `docs/index.md`): Exposes two injected local callables — `read_file` and `write_file` — inside inline-code tool bodies (§4.5.2) when the agent isn't running with `safe_mode` (or the invocation opts into `_unsafe_mode`), so inline code can read and write file contents dynamically (a path computed at runtime, several files chosen by the code, or a read-modify-write cycle) without the previous static `file_read → inline → file_write` wiring.
|
||||
|
||||
`read_file(path, max_chars=None, offset=0)` returns the raw decoded file contents — honoring the same `offset`/`max_chars` character-windowing as the `file_read` tool (ADR-2033), and returning the content itself rather than the `[FILE_READ_SUCCESS]` envelope. `write_file(path, content, mode="w")` writes via the `file_write` modes (`w`/`a`/`insert`, §4.5.5) and returns the number of characters written. Both helpers confine every access to the sandbox root: a path beginning with `~`, containing a `..` traversal component, or whose resolved real path (symlinks followed) falls outside the sandbox root is rejected with a `ValueError` naming the offending path. `write_file` performs a write only when the invocation context contains `_unsafe_mode: true` (raising `ValueError` otherwise); `read_file` does not require it, preserving the read/write privilege split. When neither condition holds, neither name is bound, so referencing it raises `NameError`, and the §13.2.1 built-in table (no `open`, `os`, `io`, `pathlib`, or file descriptors) is unchanged; the §13.2.2 expression sandbox gains no file access.
|
||||
|
||||
Internally, the `file_read`/`file_write` validation and I/O are extracted into a single shared core (`FileAccessCore`, `SandboxRootPolicy`) that backs both the built-in tools and the injected helpers — including the `w`/`a`/`insert` write mechanics themselves, not only their validation — so containment, windowing, and write-mode semantics have exactly one implementation. The shared containment resolves symlinks (`realpath`), closing a symlink-escape gap in the built-in tools as well. The injected callables are plain closures over the core, not bound methods, so inline code cannot reach the (intentionally unconfined-at-its-own-level) core through a `__self__` reference.
|
||||
|
||||
See `docs/adr/ADR-2035-inline-code-sandboxed-file-access.md` for the full set of design decisions (sanctioned helpers vs. raw `open()`, raw-value returns, the containment boundary and `ValueError` model, and the mode gating), reflected in Actor Configuration Standard revision 1.2.0.
|
||||
|
||||
**Module:** `src/cleveractors/agents/file_access.py` (`SandboxRootPolicy`, `FileAccessCore`, `InlineFileHelperFactory`, `coerce_read_window`), `src/cleveractors/agents/tool.py` (`ToolAgent._execute_python_code`, `ToolAgent._file_read_tool`, `ToolAgent._file_write_tool`). BDD: `features/inline_code_file_helpers.feature`, `features/sandbox_root_policy.feature`. Robot: `robot/inline_code_file_helpers.robot`.
|
||||
|
||||
- **Skill Package Schema and Agent-Side Skill Loading (issue #88)** (`agents/skills.py`, `agents/skill_schema.py`, `agents/skill_resolution.py`, `agents/factory.py`, `agents/llm.py`, `agents/tool.py`, `agents/llm_tools.py`, `runtime.py`, `runtime_dispatch.py`, `core/application.py`): Adds a normative Skill package schema (Package Registry Standard §16) aligned with the open [agentskills.io](https://agentskills.io/specification) Agent Skills format, and a new optional `skills` field on `type: llm` agent configs that resolves, validates, and loads Skill packages through the existing `cleveractors.registry` client.
|
||||
|
||||
A `skill`-type package is a single YAML mapping with required `name`, `description`, and `instructions`, plus optional `license`, `compatibility`, `metadata`, `allowed_tools`, and `resources` (bundled files, UTF-8 or base64-encoded, validated as well-formed at load time) — see `docs/actor-registry-standard.md` §16 for the full schema. `SkillLoader` resolves each `skills` reference (via `registry:`, `ID:`, or `local:` schemes, mirroring the existing template package-reference resolution path in `cleveractors.templates.base._resolve_package_ref`), validates it with `SkillValidator` (rejecting duplicate skill names across a `skills` list) and returns an augmented config with a discovery catalogue appended to `system_prompt`, a synthesized `skill` activation tool appended to `tools` (recognizing an already-declared `skill` tool in either shorthand or normalized OpenAI function-calling form, so it is never duplicated), and the resolved catalogue under an internal `_loaded_skills` key. A skill's content-addressed `package_id` is identical regardless of which reference scheme resolved it, and any reference that cannot be resolved — including an `ID:` reference absent from the local store's identity map — fails with a clear registry error rather than a misleading validation error. Both `SkillLoader`/`SkillReferenceResolver` and `AgentFactory` (`acreate_agent`) expose matching sync and async resolution paths, so a `registry:` reference resolves correctly whether agent creation runs from plain sync code or from the async runtime dispatch layer; every reference in a `skills` list shares one `PackageContentResolver` per agent-creation call — built unconditionally regardless of whether a local store or registry API key is configured, and closed once that call completes, including when agent creation itself fails. `Skill.to_context_dict()` surfaces `license`, `compatibility`, `metadata`, and `allowed_tools` alongside `name`/`description`/`instructions`/`resources`; base64-encoded resources that are not valid UTF-8 (genuine binary assets) decode safely instead of raising.
|
||||
|
||||
@@ -0,0 +1,47 @@
|
||||
"""ASV benchmarks for the sandbox-confined file-access cores (ADR-2035).
|
||||
|
||||
Measures the performance of the hot paths shared by the built-in
|
||||
``file_read`` / ``file_write`` tools and the inline-code ``read_file`` /
|
||||
``write_file`` helpers:
|
||||
|
||||
- sandbox-root containment resolution (``SandboxRootPolicy.resolve``)
|
||||
- windowed reads (``FileAccessCore.read_window``) at full-file and
|
||||
offset/max_chars-bounded sizes
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import shutil
|
||||
import tempfile
|
||||
|
||||
from cleveractors.agents.file_access import FileAccessCore, SandboxRootPolicy
|
||||
|
||||
|
||||
class FileAccessBenchmark:
|
||||
"""Benchmark the containment and windowing hot paths."""
|
||||
|
||||
def setup(self) -> None:
|
||||
"""Create a real sandbox with a sizable file to read."""
|
||||
self._sandbox = tempfile.mkdtemp()
|
||||
self._policy = SandboxRootPolicy(self._sandbox)
|
||||
self._core = FileAccessCore(self._policy)
|
||||
self._path = os.path.join(self._sandbox, "data.txt")
|
||||
with open(self._path, "w", encoding="utf-8") as handle:
|
||||
handle.write("0123456789" * 5000)
|
||||
|
||||
def teardown(self) -> None:
|
||||
"""Remove the sandbox."""
|
||||
shutil.rmtree(self._sandbox, ignore_errors=True)
|
||||
|
||||
def time_resolve_within_root(self) -> None:
|
||||
"""Measure a successful containment resolution."""
|
||||
self._policy.resolve("data.txt")
|
||||
|
||||
def time_read_full_window(self) -> None:
|
||||
"""Measure a full-file windowed read."""
|
||||
self._core.read_window(self._path, None, 0)
|
||||
|
||||
def time_read_bounded_window(self) -> None:
|
||||
"""Measure an offset/max_chars-bounded windowed read."""
|
||||
self._core.read_window(self._path, 8000, 4000)
|
||||
@@ -0,0 +1,383 @@
|
||||
# ADR-2035: Sandboxed `read_file`/`write_file` Helpers for Inline Code — Specification Extension
|
||||
|
||||
**Status:** Approved
|
||||
|
||||
**Date:** 2026-08-04
|
||||
|
||||
**Author:** Luis Mendes (CoreRasurae)
|
||||
|
||||
**Issue:** #93 — feat(agents): expose sandboxed file helpers to inline code
|
||||
|
||||
---
|
||||
|
||||
## Context
|
||||
|
||||
Inline-code tools (§4.5.2) — e.g. a `Python_exec`-style `type: tool` agent
|
||||
whose tool definition carries a `code` body — execute in the restricted
|
||||
inline-code sandbox built by `ToolAgent._execute_python_code`
|
||||
(`cleveractors.agents.tool`, at commit 46bff0d). That sandbox exposes the
|
||||
**Restricted Built-ins for Inline Code** set of §13.2.1 (type constructors,
|
||||
iteration/aggregation utilities, `isinstance`/`type`, `locals`, `print`, the
|
||||
`ValueError`/`IndexError`/`KeyError`/`TypeError` categories) plus a single
|
||||
injected module, `json` (`safe_globals["json"] = __import__("json")`), and the
|
||||
four local variables `input_data` / `message` / `context` / `result`.
|
||||
|
||||
Crucially, that set contains **no filesystem access whatsoever**: `open()` is
|
||||
not in the §13.2.1 built-in table, and §13.2.3 prohibits "performing arbitrary
|
||||
I/O against the filesystem" *unconditionally* — in both safe and unsafe mode.
|
||||
Today `_execute_python_code` even turns a `NameError` for `open` into a
|
||||
diagnostic steering the author toward "the file_read and file_write tools for
|
||||
file operations." That is conformant with the current standard, but it means an
|
||||
inline-code body cannot read or write file contents dynamically from within a
|
||||
single code body. The only sanctioned path is **static composition** — wiring a
|
||||
built-in `file_read` tool (§4.5.1) into the inline tool as `input_data`, and a
|
||||
separate `file_write` tool after it. Static composition cannot express dynamic
|
||||
access (a path computed at runtime), multi-file access (reading N files chosen
|
||||
by the code), or a read-modify-write cycle inside one body.
|
||||
|
||||
Meanwhile the built-in tools already own exactly the validated I/O this gap
|
||||
needs:
|
||||
|
||||
- `ToolAgent._file_read_tool` (`cleveractors.agents.tool`) performs shell-command
|
||||
detection, safe-mode `..`/absolute-path rejection, and — per ADR-2033 — the
|
||||
`offset`/`max_chars` character-windowing that returns
|
||||
`content[offset:window_end]`, wrapped in the `[FILE_READ_SUCCESS]` envelope
|
||||
destined for an LLM.
|
||||
- `ToolAgent._file_write_tool` (with helpers `_validate_file_write_args`,
|
||||
`_validate_file_path_safety`, `_prepare_append_content`,
|
||||
`_handle_insert_position`) performs the `_unsafe_mode`-gated,
|
||||
containment-checked `w`/`a`/`insert` writes of §4.5.5, returning a
|
||||
human-readable success sentence.
|
||||
|
||||
This ADR decides **how** to close the inline-code gap. The central design
|
||||
question is not *whether* inline code should reach the filesystem in unsafe mode
|
||||
— the issue already scopes that to unsafe mode — but *through what surface*:
|
||||
raw `open()`/`os`/`pathlib` (widening §13.2.1), or a narrow pair of sanctioned
|
||||
callables layered over the already-validated tool cores. It also resolves the
|
||||
subsidiary questions that follow from that choice: what the helpers return
|
||||
(raw content vs. the LLM envelope), what containment boundary they enforce and
|
||||
which exception type a violation raises, how the read/write privilege split is
|
||||
preserved, and which specification sections must change in lockstep with the
|
||||
code.
|
||||
|
||||
---
|
||||
|
||||
## Decision
|
||||
|
||||
### D-1: Sanctioned helper functions, not raw `open()`
|
||||
|
||||
**What:** In unsafe mode the inline-code sandbox gains exactly **two** injected
|
||||
local callables — `read_file` and `write_file` — and nothing else. `open`, the
|
||||
`os` / `io` / `pathlib` modules, and raw file descriptors remain absent. The
|
||||
§13.2.1 built-in table is **not** widened.
|
||||
|
||||
**Why:** The capability inline code actually needs is "read this file's
|
||||
contents" and "write these contents to that file" — not the full, unbounded
|
||||
`open()` surface (arbitrary modes, file-descriptor manipulation, `os.*`
|
||||
traversal of the whole filesystem). Injecting two purpose-built callables, in
|
||||
the exact manner `json` is already injected (a name in the execution namespace,
|
||||
not a new entry in `__builtins__`), grants precisely that capability and no
|
||||
more. It keeps the normative §13.2.1 built-in table unchanged — so the
|
||||
regression surface for "what builtins can inline code see" stays fixed — while
|
||||
still giving authors dynamic, multi-file, read-modify-write access. It also
|
||||
concentrates all filesystem policy in one auditable place (the shared cores of
|
||||
D-2) rather than scattering it across whatever an author might do with a raw
|
||||
`open()` handle.
|
||||
|
||||
**Why injected locals, not builtins:** `read_file`/`write_file` are host-provided
|
||||
facilities bound to host state (the sandbox root, the invocation context), just
|
||||
like `json` is a host-provided module. Modelling them as injected locals — the
|
||||
established §4.5.2(2)/(3) mechanism — means their presence is trivially
|
||||
conditioned on mode (D-5) by simply not binding the names, which yields the
|
||||
required `NameError` in safe mode for free.
|
||||
|
||||
### D-2: Single validation surface — reuse the existing tool cores
|
||||
|
||||
**What:** The validation-and-I/O logic currently embedded in `_file_read_tool`
|
||||
and `_file_write_tool` is extracted into reusable **cores** that back *both* the
|
||||
built-in tools *and* the injected helpers. There is exactly one implementation
|
||||
of: path containment, `offset`/`max_chars` read windowing, and `w`/`a`/`insert`
|
||||
write semantics. The built-in tools become thin adapters that call a core and
|
||||
then format the `[FILE_READ_SUCCESS]` / "Successfully wrote…" strings; the
|
||||
helpers call the *same* core and return its raw result (D-3).
|
||||
|
||||
**Why:** The acceptance criteria require the helpers to honor "the same
|
||||
semantics as the `file_read` tool" (offset/max_chars) and "the `file_write`
|
||||
core (§4.5.5 modes)". If the helpers reimplemented that logic, the two surfaces
|
||||
would inevitably drift — a containment fix or a mode fix applied to one and
|
||||
forgotten on the other would open a security or correctness gap on the
|
||||
untouched surface. A single validation surface makes "confined to the sandbox
|
||||
root" and "preserve the read/write privilege split" provable properties of one
|
||||
code path rather than an invariant that must be manually re-checked on two.
|
||||
This is the Facade/shared-core relationship already implicit in the codebase
|
||||
(the tools *are* the only file I/O today); the extraction just makes the core
|
||||
independently callable.
|
||||
|
||||
### D-3: Signatures and return values — raw content and counts, not LLM envelopes
|
||||
|
||||
**What:** The injected callables have these exact signatures and returns:
|
||||
|
||||
- `read_file(path, max_chars=None, offset=0) -> str` — returns the **raw decoded
|
||||
file contents** for the requested window, reusing the ADR-2033
|
||||
`offset`/`max_chars` character-windowing (`content[offset:window_end]`,
|
||||
`window_end = char_count if max_chars is None else min(char_count, offset +
|
||||
max_chars)`). It returns the *content itself*, **not** the
|
||||
`[FILE_READ_SUCCESS]…[FILE_CONTENT_START]…[FILE_CONTENT_END]` envelope, and
|
||||
emits no `TRUNCATED` / `MORE_CONTENT_AT_OFFSET` / `NO_MORE_CONTENT_AT_OFFSET`
|
||||
markers.
|
||||
- `write_file(path, content, mode="w") -> int` — writes via the `file_write`
|
||||
core honoring the §4.5.5 `w` / `a` / `insert` modes, and returns the
|
||||
**number of characters written** (`len(content)`) as an `int`, **not** the
|
||||
"Successfully wrote/appended/inserted …" sentence.
|
||||
|
||||
**Why return raw values, not the tool strings:** The `[FILE_READ_SUCCESS]`
|
||||
envelope and the "Successfully wrote…" sentence exist to communicate *to an
|
||||
LLM* — they are affordances of the tool-calling protocol (§4.5.7), carrying
|
||||
continuation hints and file metadata a model needs to decide its next call.
|
||||
An inline-code body is a *program*, not a model: it wants the bytes it asked
|
||||
for and an integer it can compare, and it would otherwise have to *parse the
|
||||
markers back out* of the envelope to use the content — re-introducing exactly
|
||||
the brittle header-parsing ADR-2033's Consequences warned against. Returning
|
||||
`str` content and an `int` count matches how `json.loads`/`json.dumps` (the
|
||||
sandbox's only other injected facility) behave: they return Python values, not
|
||||
narration. Pagination is still fully expressible — the caller drives it by
|
||||
passing `offset`/`max_chars` and observing `len()` of the returned window —
|
||||
without needing the header markers.
|
||||
|
||||
**Why `insert` mode has no `position` parameter in the helper signature:** The
|
||||
signature the issue fixes is `write_file(path, content, mode="w")`. `insert`
|
||||
without an explicit position defaults to `"end"` per §4.5.5 (equivalent to
|
||||
append at the end). Exposing the full `position` matrix through the helper is
|
||||
deferred as a non-goal of this issue; the core still supports it for the tool.
|
||||
|
||||
### D-4: Sandbox-root containment — realpath resolution, `ValueError` on escape
|
||||
|
||||
**What:** Both helpers confine all access to the **sandbox root**, defined as
|
||||
the resolved real path of the sandbox's working directory
|
||||
(`os.path.realpath(os.getcwd())` under the current sandbox strategy — the same
|
||||
process-CWD boundary `_validate_file_path_safety` already treats as the root).
|
||||
A path is admitted **iff** its fully resolved real path — with `..` collapsed
|
||||
and **symlinks followed** — is equal to the sandbox root or a descendant of it.
|
||||
The following are rejected, each raising **`ValueError`** whose message names
|
||||
the offending path:
|
||||
|
||||
- any path containing a `..` traversal component;
|
||||
- any path beginning with `~` (home-directory expansion);
|
||||
- any absolute or relative path whose resolved real path falls outside the
|
||||
sandbox root — **including a symlink that itself lives inside the root but
|
||||
points outside it** (symlink-escape).
|
||||
|
||||
**Why `realpath` (symlinks followed), strengthening the existing check:** The
|
||||
current `_validate_file_path_safety` computes `os.path.abspath` and compares
|
||||
`startswith(working_dir + os.sep)`. `abspath` normalises `..` but does **not**
|
||||
resolve symlinks, so a symlink placed inside the root pointing outside it passes
|
||||
the `startswith` test yet reads/writes outside the root. The acceptance criteria
|
||||
explicitly demand rejecting "any path whose resolved real path (symlinks
|
||||
followed) falls outside the sandbox root," so the shared containment core
|
||||
resolves via `realpath` before the boundary comparison. Because the core is
|
||||
shared (D-2), this closes the symlink-escape gap for the built-in tools too — a
|
||||
strict strengthening, never a relaxation, of today's boundary.
|
||||
|
||||
**Why `ValueError`, not `ExecutionError`:** A containment violation surfaces
|
||||
*inside the running inline-code body*, and that body can only catch exception
|
||||
categories the §13.2.1 table exposes to it — `ValueError` is one; the host's
|
||||
`ExecutionError` is not. Raising `ValueError` lets an inline author write
|
||||
`try: read_file(p) except ValueError: …` using the sandbox's own vocabulary,
|
||||
whereas an `ExecutionError` would be an opaque, uncatchable escape from the
|
||||
sandbox's error model. The built-in-tool adapters continue to translate core
|
||||
failures into `ExecutionError` for the tool-calling protocol; only the
|
||||
helper-facing surface raises `ValueError`.
|
||||
|
||||
### D-5: Mode gating — injection in unsafe mode, `write_file` also gated on `_unsafe_mode`
|
||||
|
||||
**What:**
|
||||
|
||||
- The helpers are injected into the inline namespace when the agent's
|
||||
`safe_mode` (§4.5) is `false`, or the invocation context carries
|
||||
`_unsafe_mode: true` — the same two conditions §4.5.4 already uses to relax
|
||||
`file_read`/`file_write`'s own restrictions. When neither holds, the names
|
||||
are simply never bound, so referencing `read_file` or `write_file` raises
|
||||
`NameError`, exactly as any other undefined name would.
|
||||
- `write_file` performs a write **only when the invocation context contains
|
||||
`_unsafe_mode: true`**; absent that, it raises `ValueError` (per D-4's
|
||||
helper-facing error model) and performs no write. This mirrors the existing
|
||||
`_file_write_tool` gate (which raises when `_unsafe_mode` is absent), and
|
||||
preserves the §4.5.4 / §13.3 rule that *writes always require `_unsafe_mode`
|
||||
regardless of host mode*.
|
||||
- `read_file` does **not** require `_unsafe_mode`. Within an already-unsafe host
|
||||
(the only place the helper exists at all), reads are permitted subject only to
|
||||
the D-4 containment boundary — preserving the existing read/write privilege
|
||||
split, where reads are less restricted than writes.
|
||||
- The §13.2.2 **expression** sandbox (`transform.fn`, and the bridge
|
||||
`custom_predicate` / `state_extractor` / `state_flattener` fields) is
|
||||
**unchanged** and gains no file access. `read_file`/`write_file` are injected
|
||||
only into the §13.2.1 inline-code namespace, never into expression
|
||||
evaluation.
|
||||
|
||||
**Why two gates for write, one for read:** There are two orthogonal switches
|
||||
already established by §4.5/§4.5.4 for the pre-existing tools: the agent's own
|
||||
`safe_mode` configuration and the per-invocation `_unsafe_mode` context flag;
|
||||
either being in its relaxed state is enough for the helpers to exist at all.
|
||||
The context flag is *additionally* layered on top as the per-invocation opt-in
|
||||
that governs *writes specifically*. Collapsing the two into one would either
|
||||
let a write happen without the explicit per-invocation opt-in (unsafe) or
|
||||
forbid reads that either knob has already sanctioned (needlessly restrictive).
|
||||
Keeping `read_file` off the `_unsafe_mode` gate reproduces the exact asymmetry
|
||||
the tools already implement: `_file_read_tool` never checks `_unsafe_mode` for
|
||||
a relative in-root read, while `_file_write_tool` refuses without it.
|
||||
|
||||
**Relationship to §13.1's host-level unsafe mode:** §13.1 describes a coarser,
|
||||
host-level switch (typically a CLI/config flag) that controls whether the host
|
||||
injects `_unsafe_mode` into invocation contexts in the first place; it is not
|
||||
itself threaded into an agent's `safe_mode` field automatically. The gate above
|
||||
reuses the same per-agent/per-invocation signals the pre-existing `file_read`/
|
||||
`file_write` tools already key off — it does not introduce a new, independent
|
||||
host-mode check.
|
||||
|
||||
### D-6: Specification amendments applied in the same change
|
||||
|
||||
**What:** This ADR mandates the following amendments to `docs/index.md`, applied
|
||||
in the **same commit** as the implementing code (per the project's
|
||||
same-commit documentation rule). This is the amendment set the issue's
|
||||
acceptance criteria refer to as "ADR-2035 D-6":
|
||||
|
||||
1. **§13.2.3 (Prohibited Capabilities)** — reworded so the blanket prohibition
|
||||
on filesystem I/O admits a single, explicitly-scoped exception: the sanctioned
|
||||
`read_file`/`write_file` helpers injected into the *inline-code* sandbox
|
||||
(§13.2.1) in unsafe mode. The prohibition remains absolute for the §13.2.2
|
||||
expression sandbox and for every other form of I/O (network beyond
|
||||
`http_request`, subprocess, dynamic import/eval, host introspection).
|
||||
2. **§4.5.2 (Inline Tools)** — the list of injected local facilities is extended
|
||||
to document `read_file`/`write_file` alongside `json`, with their signatures,
|
||||
return values, containment behavior, and mode gating.
|
||||
3. **§13.2.1 (Restricted Built-ins for Inline Code)** — a note clarifying that
|
||||
the built-in *table itself is unchanged* (no `open`, no `os`/`io`/`pathlib`,
|
||||
no file descriptors) and that filesystem access is available *only* through
|
||||
the two injected helpers, not through any built-in.
|
||||
4. **§13.3 (Filesystem Boundaries)** — a cross-reference recording that the
|
||||
helpers are bound by the same boundary as the tools (`..`/`~` rejection,
|
||||
sandbox-root containment) and that `write_file` requires `_unsafe_mode` just
|
||||
as `file_write` does.
|
||||
|
||||
**Why in the same change:** These are not documentation of a future intention;
|
||||
they are the normative statement of the behavior this commit introduces. Under
|
||||
the project's continuous-documentation rule, spec and code that describe the
|
||||
same behavior ship together, so the spec never describes a state the code has
|
||||
not yet reached (or vice versa). Per the standard ADR process, these amendments
|
||||
are applied to `docs/index.md` only **after** this ADR is accepted.
|
||||
|
||||
---
|
||||
|
||||
## Consequences
|
||||
|
||||
### Positive
|
||||
|
||||
- **Closes the dynamic/multi-file gap:** inline code can now compute a path at
|
||||
runtime, read several files chosen by the code, and perform read-modify-write
|
||||
cycles inside one body — none of which static `file_read → inline → file_write`
|
||||
composition can express.
|
||||
- **No new built-in surface:** the §13.2.1 table is untouched; the only new
|
||||
capability is two narrowly-scoped, host-controlled callables, so the
|
||||
"what can inline code do" audit surface grows by exactly two well-defined
|
||||
functions rather than the entire `open()`/`os` API.
|
||||
- **One validation surface:** containment, windowing, and write-mode semantics
|
||||
live in a single core (D-2); a fix to either applies to both the tools and the
|
||||
helpers automatically.
|
||||
- **Stronger containment everywhere:** the realpath-based boundary (D-4) closes
|
||||
a symlink-escape gap that the previous `abspath`-only check left open — for the
|
||||
built-in tools as well, since the core is shared.
|
||||
- **Programmatic ergonomics:** helpers return `str` content and an `int` count,
|
||||
matching `json`'s value-returning model, so inline authors never parse the
|
||||
LLM-facing envelope markers back out.
|
||||
- **Privilege split preserved:** reads stay less restricted than writes; writes
|
||||
still demand the explicit `_unsafe_mode` per-invocation opt-in.
|
||||
|
||||
### Negative / Risks
|
||||
|
||||
- **A sanctioned filesystem exception now exists in §13.2.3.** The prohibition
|
||||
is no longer categorical for the inline-code sandbox; it is now "no filesystem
|
||||
I/O *except* the two named helpers, in unsafe mode, within the sandbox root."
|
||||
This is a deliberate, bounded widening of capability, mitigated by the
|
||||
containment boundary (D-4) and the double gate on writes (D-5), but it is a
|
||||
larger attack surface than "no filesystem access at all."
|
||||
- **Containment correctness now rests on `realpath` behavior.** Symlink
|
||||
resolution and root-descendant comparison must be exactly right; a bug here is
|
||||
a sandbox-escape, so it must be covered by explicit escape-attempt tests
|
||||
(`..`, absolute-out-of-root, `~`, and symlink-out-of-root).
|
||||
- **Full-file read cost inherited (ADR-2033 D-6):** `read_file` reads the whole
|
||||
file before slicing the window, so paginating a very large file re-reads it on
|
||||
each call — an accepted, documented trade-off carried over unchanged, not
|
||||
introduced here.
|
||||
- **Helper `insert` positional coverage is partial:** `write_file`'s signature
|
||||
omits `position`, so `insert` via the helper defaults to end-insertion; full
|
||||
positional insert remains available only through the built-in tool.
|
||||
|
||||
### Follow-up Required
|
||||
|
||||
- **Expose `position` for `insert` through the helper** — only if a concrete use
|
||||
case needs mid-file insertion from inline code; deferred as a non-goal here.
|
||||
- **Configurable sandbox root** — this ADR fixes the root at the process CWD (the
|
||||
boundary the code already uses). A future sandbox strategy that relocates the
|
||||
root (e.g. a git-worktree or copy-on-write sandbox) should thread an explicit
|
||||
root through the shared core rather than reading `os.getcwd()` directly.
|
||||
|
||||
---
|
||||
|
||||
## Alternatives Considered
|
||||
|
||||
### A-1: Add `open()` (and/or `os`/`io`/`pathlib`) to the §13.2.1 built-ins
|
||||
|
||||
**Rejected because:** this grants far more than the "read a file / write a file"
|
||||
capability the issue calls for — arbitrary modes, raw file descriptors, and the
|
||||
whole `os` traversal surface — and it disperses filesystem policy across
|
||||
whatever an author does with a raw handle instead of concentrating it in one
|
||||
validated core (D-2). It would also mutate the normative §13.2.1 table, enlarging
|
||||
the regression surface the acceptance criteria explicitly want frozen ("the
|
||||
§13.2.1 built-in table is unchanged"). Two sanctioned callables give the needed
|
||||
capability with a fraction of the surface.
|
||||
|
||||
### A-2: Have the helpers return the `[FILE_READ_SUCCESS]` envelope / success sentence
|
||||
|
||||
**Rejected because:** those strings are LLM-facing protocol artifacts (§4.5.7),
|
||||
carrying continuation hints a *program* does not want. An inline body would have
|
||||
to parse the markers back out to obtain the content — re-introducing the exact
|
||||
brittle header-parsing ADR-2033 warned about. Returning raw `str`/`int` (D-3)
|
||||
matches the `json` facility's value-returning model and makes pagination
|
||||
expressible via `offset`/`max_chars` + `len()` without any marker parsing.
|
||||
|
||||
### A-3: Raise `ExecutionError` (as the tools do) on containment/`_unsafe_mode` violations
|
||||
|
||||
**Rejected because:** `ExecutionError` is a host type not present in the §13.2.1
|
||||
error-category table, so inline code cannot catch it — a violation would be an
|
||||
opaque, uncatchable escape from the sandbox's own error model. `ValueError`
|
||||
(D-4) is in that table, letting authors handle a rejected path with the
|
||||
sandbox's native vocabulary. The tool adapters keep translating core failures to
|
||||
`ExecutionError` for the protocol; only the helper-facing surface raises
|
||||
`ValueError`.
|
||||
|
||||
### A-4: Gate `read_file` on `_unsafe_mode` as well (symmetric with `write_file`)
|
||||
|
||||
**Rejected because:** it would break the read/write privilege split the tools
|
||||
already implement — `_file_read_tool` permits an in-root relative read without
|
||||
`_unsafe_mode`, while `_file_write_tool` demands it. Reads are only reachable at
|
||||
all inside an already-unsafe host; requiring a second opt-in for them would be
|
||||
strictly more restrictive than the equivalent tool, for no security gain the
|
||||
containment boundary (D-4) does not already provide.
|
||||
|
||||
### A-5: Reimplement read/write logic directly in the helpers (no shared core)
|
||||
|
||||
**Rejected because:** it creates two independent copies of containment,
|
||||
windowing, and write-mode logic that will drift — a containment fix applied to
|
||||
the tool and forgotten on the helper (or vice versa) silently opens a gap on the
|
||||
untouched surface. Extracting one shared core (D-2) makes "confined to the
|
||||
sandbox root" and "preserves the read/write split" properties of a single code
|
||||
path, provable once rather than audited twice.
|
||||
|
||||
### A-6: A copy-on-write / snapshot filesystem for inline code instead of a live-path boundary
|
||||
|
||||
**Rejected because:** it solves a different problem (isolating *mutations* for
|
||||
rollback) at far greater implementation cost, and still needs a root boundary to
|
||||
decide what the snapshot covers. The issue asks for confined live access reusing
|
||||
the existing validated cores, not a new sandbox filesystem strategy; a snapshot
|
||||
layer, if ever wanted, is an orthogonal future ADR (see the configurable-root
|
||||
follow-up).
|
||||
+17
-7
@@ -1,6 +1,6 @@
|
||||
# The Actor Configuration Standard
|
||||
|
||||
**Version:** 1.1.0
|
||||
**Version:** 1.2.0
|
||||
**Status:** Normative
|
||||
|
||||
---
|
||||
@@ -517,12 +517,17 @@ When `code` is provided, the tool executes the given code body in a sandboxed en
|
||||
|
||||
1. Restrict the available built-in functions and types to the **Restricted Built-ins for Inline Code** set defined in §13.2.1.
|
||||
2. Expose a JSON parsing and serialization facility under the name `json`, providing at minimum the operations `loads` (parse a JSON string), `dumps` (serialize a value to JSON), and an error category `JSONDecodeError` signaled when parsing fails.
|
||||
3. Provide the following local variables to the tool body:
|
||||
3. When the agent's `safe_mode` (§4.5) is `false`, or the invocation context contains `_unsafe_mode: true`, expose exactly two additional injected local callables providing sandbox-confined filesystem access (§13.2.3, §13.3) — the same two conditions §4.5.4 already uses to relax `file_read`/`file_write`'s own restrictions:
|
||||
- `read_file(path, max_chars=None, offset=0)` — returns the raw decoded file contents as a string, applying the same `offset`/`max_chars` character-windowing as the `file_read` tool (§4.5.1). It returns the content itself, not the `[FILE_READ_SUCCESS]` envelope, and does not require `_unsafe_mode`.
|
||||
- `write_file(path, content, mode="w")` — writes `content` using the `file_write` modes (§4.5.5) and returns the number of characters written as an integer. It MUST perform a write only when the invocation context contains `_unsafe_mode: true`.
|
||||
|
||||
Both callables MUST confine access to the sandbox root: a `path` beginning with `~`, containing a `..` traversal component, or whose resolved real path (symlinks followed) falls outside the sandbox root MUST be rejected with a `ValueError` naming the path (a `write_file` call without `_unsafe_mode` MUST likewise raise `ValueError`). When neither condition above holds, neither name is bound, so referencing it raises `NameError`. These helpers are exposed only to inline-code bodies, never to the expression sandbox (§13.2.2).
|
||||
4. Provide the following local variables to the tool body:
|
||||
- `input_data`: the raw input message (string).
|
||||
- `message`: an alias for `input_data`.
|
||||
- `context`: the active context dictionary (passed by reference; modifications propagate back to the caller).
|
||||
- `result`: initially `null`; the tool body sets this to its return value.
|
||||
4. When `result` is `null` after the tool body completes, fall back to one of (in order): `output`, `response`, `answer`, defaulting to the empty string if none are set.
|
||||
5. When `result` is `null` after the tool body completes, fall back to one of (in order): `output`, `response`, `answer`, defaulting to the empty string if none are set.
|
||||
|
||||
Inline-code tools MUST be enabled only when the host is operating in unsafe mode (see §13).
|
||||
|
||||
@@ -2400,7 +2405,7 @@ When a compliant implementation does honor these fields, `verbose: true` SHOULD
|
||||
|
||||
### 9.6 `version`
|
||||
|
||||
Implementations MAY accept a `version` parameter. This field is informational only and MAY be used to select compatibility behaviors. The current version of this standard is `"1.1.0"`.
|
||||
Implementations MAY accept a `version` parameter. This field is informational only and MAY be used to select compatibility behaviors. The current version of this standard is `"1.2.0"`.
|
||||
|
||||
---
|
||||
|
||||
@@ -2729,6 +2734,8 @@ When an inline-code tool body is executed, the following built-in facilities MUS
|
||||
| Error categories | `ValueError`, `IndexError`, `KeyError`, `TypeError` |
|
||||
| Module access | A facility exposing only the JSON parsing/serialization module under the name `json` |
|
||||
|
||||
This built-in table is exhaustive and unchanged by the file-access helpers: `open`, the `os` / `io` / `pathlib` modules, and raw file descriptors remain absent. Filesystem access from inline code is available only through the injected `read_file` / `write_file` callables of §4.5.2 (unsafe mode only), never through a built-in.
|
||||
|
||||
#### 13.2.2 Restricted Built-ins for Expression Evaluation
|
||||
|
||||
When an inline **expression** (such as the `transform.fn` parameter or the bridge `custom_predicate`/`state_extractor`/`state_flattener` fields) is evaluated, only the following minimal set of built-in facilities MUST be available:
|
||||
@@ -2742,13 +2749,13 @@ When an inline **expression** (such as the `transform.fn` parameter or the bridg
|
||||
|
||||
Neither sandbox MAY expose any of the following:
|
||||
|
||||
- Performing arbitrary I/O against the filesystem, network, or system.
|
||||
- Performing arbitrary I/O against the filesystem, network, or system. The sole sanctioned exception is the inline-code `read_file` / `write_file` helpers of §4.5.2: in unsafe mode the inline-code sandbox (§13.2.1) MAY expose those two sandbox-root-confined callables and nothing wider. This exception does not extend to the expression sandbox (§13.2.2), which MUST expose no filesystem access.
|
||||
- Dynamic evaluation or compilation of arbitrary source code.
|
||||
- Introspection of the host or surrounding execution environment.
|
||||
- Dynamic import of modules other than those explicitly listed.
|
||||
- Subprocess spawning or signaling.
|
||||
|
||||
Any attempt to expose these capabilities MUST be refused at configuration load time or at invocation time.
|
||||
Any attempt to expose these capabilities, beyond the single sanctioned filesystem exception above, MUST be refused at configuration load time or at invocation time.
|
||||
|
||||
### 13.3 Filesystem Boundaries
|
||||
|
||||
@@ -2762,6 +2769,8 @@ In unsafe mode (the host is in unsafe mode AND `_unsafe_mode: true` is in the in
|
||||
- `file_read` permits absolute paths.
|
||||
- `file_write` is enabled but still refuses `..` traversal and home-directory expansion (`~`).
|
||||
|
||||
The inline-code `read_file` / `write_file` helpers (§4.5.2) are bound by this same boundary: they reject `..` traversal and `~` expansion and confine every access to the sandbox root (resolved real path, symlinks followed). As with `file_write`, `write_file` performs a write only when the invocation context contains `_unsafe_mode: true`; `read_file`, like `file_read`, does not require it.
|
||||
|
||||
### 13.4 Network Boundaries
|
||||
|
||||
The `http_request` tool MAY be invoked in either mode. Outbound network access SHOULD be restricted according to host policy; that policy is outside the scope of this standard.
|
||||
@@ -3962,7 +3971,7 @@ Compliant implementations SHOULD signal the following error categories with the
|
||||
|
||||
## 21. Versioning of This Standard
|
||||
|
||||
This document is **Version 1.1.0** of the Actor Configuration Standard. Future revisions:
|
||||
This document is **Version 1.2.0** of the Actor Configuration Standard. Future revisions:
|
||||
|
||||
- A **patch** version (1.0.x) corrects errors and clarifies semantics without changing required behavior.
|
||||
- A **minor** version (1.x.0) adds new optional features without breaking conformance of existing implementations.
|
||||
@@ -3976,3 +3985,4 @@ Conformance MUST be declared against a specific version of this standard. Implem
|
||||
|---------|--------|
|
||||
| 1.0.0 | Initial version. |
|
||||
| 1.1.0 | §5.4: `content_not_contains` Honored By extended from `Stream router, conditional nodes, bridge router` to `All subsystems`, adding pure-graph edges. Minor revision — restores the symmetric contains/not-contains treatment `content_contains` already had, without breaking conformance of configurations that do not use `content_not_contains` on pure-graph edges. See `docs/adr/ADR-2034-content-not-contains-all-subsystems.md`. |
|
||||
| 1.2.0 | §4.5.2, §13.2.1, §13.2.3, §13.3: added the sanctioned inline-code `read_file`/`write_file` helpers, injected into the inline-code sandbox in unsafe mode and confined to the sandbox root. Minor revision — adds an optional capability without changing the §13.2.1 built-in table or breaking conformance of configurations that do not use the helpers. See `docs/adr/ADR-2035-inline-code-sandboxed-file-access.md`. |
|
||||
|
||||
@@ -0,0 +1,126 @@
|
||||
Feature: Inline-code sandboxed file helpers (read_file / write_file)
|
||||
Inline-code tool bodies gain two injected helpers, read_file and write_file,
|
||||
when the host is in unsafe mode (ADR-2035). They reuse the file_read/file_write
|
||||
validated cores, return raw values rather than the LLM envelope, and confine
|
||||
every access to the sandbox root.
|
||||
|
||||
Scenario: The helpers are absent in safe mode
|
||||
Given an inline-code tool agent in safe mode
|
||||
When the inline code tries to read "notes.txt"
|
||||
Then the tool execution fails because read_file is not available
|
||||
|
||||
Scenario: write_file writes content and read_file reads it back in unsafe mode
|
||||
Given an inline-code tool agent in unsafe mode granting write access
|
||||
When the inline code writes "hello from inline" to "notes.txt" and reads it back
|
||||
Then the tool result should be "hello from inline"
|
||||
|
||||
Scenario: write_file returns the number of characters written
|
||||
Given an inline-code tool agent in unsafe mode granting write access
|
||||
When the inline code writes "abcde" to "count.txt" and returns the write count
|
||||
Then the tool result should be "5"
|
||||
|
||||
Scenario: read_file honors offset and max_chars windowing
|
||||
Given a sandbox file "data.txt" containing "0123456789"
|
||||
And an inline-code tool agent in unsafe mode granting write access
|
||||
When the inline code reads "data.txt" with offset 2 and max_chars 3
|
||||
Then the tool result should be "234"
|
||||
|
||||
Scenario: read_file reading at or beyond end of file yields empty content
|
||||
Given a sandbox file "short.txt" containing "abc"
|
||||
And an inline-code tool agent in unsafe mode granting write access
|
||||
When the inline code reads "short.txt" with offset 10 and max_chars 5
|
||||
Then the tool result should be empty
|
||||
|
||||
Scenario: read_file with a negative max_chars matches the file_read tool's windowing exactly
|
||||
# Both surfaces share one windowing implementation (FileAccessCore.read_window,
|
||||
# ADR-2035 D-2), so a negative max_chars produces the same backward Python
|
||||
# slice (content[offset:offset+max_chars]) on both — this locks in that
|
||||
# parity rather than changing the pre-existing ADR-2033 max_chars coercion.
|
||||
Given a sandbox file "data.txt" containing "0123456789"
|
||||
And an inline-code tool agent in unsafe mode granting write access
|
||||
When the inline code reads "data.txt" with offset 0 and max_chars -3
|
||||
Then the tool result should be "0123456"
|
||||
|
||||
Scenario: write_file appends a Markdown section with separating spacing
|
||||
Given a sandbox file "doc.txt" containing "intro"
|
||||
And an inline-code tool agent in unsafe mode granting write access
|
||||
When the inline code appends "# Section" to "doc.txt" and reads it back
|
||||
Then the tool result should be "intro\n\n# Section"
|
||||
|
||||
Scenario: write_file inserts content and read_file confirms it
|
||||
Given a sandbox file "list.txt" containing "a\nb\n"
|
||||
And an inline-code tool agent in unsafe mode granting write access
|
||||
When the inline code inserts "z" into "list.txt" and reads it back
|
||||
Then the tool result should be "a\nb\nz\n"
|
||||
|
||||
Scenario: read_file does not require write access
|
||||
Given a sandbox file "readable.txt" containing "visible"
|
||||
And an inline-code tool agent in unsafe mode without write access
|
||||
When the inline code tries to read "readable.txt"
|
||||
Then the tool result should be "visible"
|
||||
|
||||
Scenario: write_file refuses to write without _unsafe_mode in the context
|
||||
Given an inline-code tool agent in unsafe mode without write access
|
||||
When the inline code tries to write "blocked" to "denied.txt"
|
||||
Then the caught error should mention "unsafe mode"
|
||||
And no file "denied.txt" should exist in the sandbox
|
||||
|
||||
Scenario: write_file rejects a non-string content value
|
||||
Given an inline-code tool agent in unsafe mode granting write access
|
||||
When the inline code tries to write a non-string value to "bad.txt"
|
||||
Then the caught error should mention "content must be a string"
|
||||
|
||||
Scenario: write_file rejects an invalid write mode
|
||||
Given an inline-code tool agent in unsafe mode granting write access
|
||||
When the inline code performs an invalid-mode write of "x" to "modes.txt" with mode "bad"
|
||||
Then the caught error should mention "Invalid mode"
|
||||
|
||||
Scenario: read_file rejects a negative offset
|
||||
Given a sandbox file "data.txt" containing "0123456789"
|
||||
And an inline-code tool agent in unsafe mode granting write access
|
||||
When the inline code reads "data.txt" with a negative offset
|
||||
Then the caught error should mention "offset must be non-negative"
|
||||
|
||||
Scenario: read_file rejects a non-integer max_chars
|
||||
Given an inline-code tool agent in unsafe mode granting write access
|
||||
When the inline code reads a file with a non-integer max_chars
|
||||
Then the caught error should mention "max_chars must be an integer"
|
||||
|
||||
Scenario: read_file rejects a non-integer offset
|
||||
Given an inline-code tool agent in unsafe mode granting write access
|
||||
When the inline code reads a file with a non-integer offset
|
||||
Then the caught error should mention "offset must be an integer"
|
||||
|
||||
Scenario: read_file rejects an empty path
|
||||
Given an inline-code tool agent in unsafe mode granting write access
|
||||
When the inline code reads an empty path
|
||||
Then the caught error should mention "non-empty string"
|
||||
|
||||
Scenario Outline: The helpers reject paths that escape the sandbox root
|
||||
Given an inline-code tool agent in unsafe mode granting write access
|
||||
When the inline code tries to read "<path>"
|
||||
Then the caught error should mention "<path>"
|
||||
|
||||
Examples:
|
||||
| path |
|
||||
| ../escape.txt |
|
||||
| ~/secret.txt |
|
||||
| /etc/hostname |
|
||||
|
||||
Scenario: The helpers reject a symlink that escapes the sandbox root
|
||||
Given an inline-code tool agent in unsafe mode granting write access
|
||||
And a symlink "link.txt" in the sandbox pointing outside the sandbox root
|
||||
When the inline code tries to read "link.txt"
|
||||
Then the caught error should mention "link.txt"
|
||||
|
||||
Scenario: write_file confines writes to the sandbox root
|
||||
Given an inline-code tool agent in unsafe mode granting write access
|
||||
When the inline code tries to write "x" to "../escape.txt"
|
||||
Then the caught error should mention "../escape.txt"
|
||||
|
||||
Scenario: write_file refuses to write through a symlink escaping the sandbox root
|
||||
Given an inline-code tool agent in unsafe mode granting write access
|
||||
And a symlink "wlink.txt" in the sandbox pointing outside the sandbox root
|
||||
When the inline code tries to write "leak" to "wlink.txt"
|
||||
Then the caught error should mention "wlink.txt"
|
||||
And the escaped symlink target should still contain "outside the root"
|
||||
@@ -0,0 +1,52 @@
|
||||
Feature: Sandbox-root containment policy (security)
|
||||
The SandboxRootPolicy (ADR-2035 D-4) is the single admission rule guarding
|
||||
the inline-code read_file/write_file helpers. These scenarios exercise it
|
||||
directly, with crafted roots, to prove that no path outside the sandbox root
|
||||
can be reached and that legitimate in-root paths are still admitted.
|
||||
|
||||
Scenario: A relative path inside the root is admitted
|
||||
Given a sandbox root policy
|
||||
When I resolve the relative path "notes.txt"
|
||||
Then resolution is admitted and stays inside the root
|
||||
|
||||
Scenario: A nested relative path inside the root is admitted
|
||||
Given a sandbox root policy
|
||||
When I resolve the relative path "sub/dir/notes.txt"
|
||||
Then resolution is admitted and stays inside the root
|
||||
|
||||
Scenario: An absolute path inside the root is admitted
|
||||
Given a sandbox root policy
|
||||
When I resolve an absolute path that is inside the root
|
||||
Then resolution is admitted and stays inside the root
|
||||
|
||||
Scenario: A sibling directory sharing the root's name prefix is rejected
|
||||
Given a sandbox root policy with a sibling directory sharing its name prefix
|
||||
When I resolve an absolute path into the sibling directory
|
||||
Then resolution is rejected as outside the root
|
||||
|
||||
Scenario: A symlink inside the root that targets outside is rejected
|
||||
Given a sandbox root policy
|
||||
And a symlink "escape" inside the root targeting a path outside it
|
||||
When I resolve the relative path "escape"
|
||||
Then resolution is rejected as outside the root
|
||||
|
||||
Scenario: A symlink inside the root that targets inside is admitted
|
||||
Given a sandbox root policy
|
||||
And a symlink "loop" inside the root targeting another path inside it
|
||||
When I resolve the relative path "loop"
|
||||
Then resolution is admitted and stays inside the root
|
||||
|
||||
Scenario Outline: Paths that attempt to escape the root are rejected
|
||||
Given a sandbox root policy
|
||||
When I resolve the raw path "<path>"
|
||||
Then resolution is rejected
|
||||
|
||||
Examples:
|
||||
| path |
|
||||
| ../escape.txt |
|
||||
| sub/../../escape |
|
||||
| .. |
|
||||
| ~/secret |
|
||||
| ~root/secret |
|
||||
| ..\\escape.txt |
|
||||
| /etc/hostname |
|
||||
@@ -0,0 +1,274 @@
|
||||
"""Step implementations for the inline-code read_file/write_file helpers (ADR-2035)."""
|
||||
|
||||
import os
|
||||
import tempfile
|
||||
from pathlib import Path
|
||||
|
||||
from behave import given, then, when
|
||||
from features.steps._coverage_utils import _run
|
||||
|
||||
from cleveractors.agents.tool import ToolAgent
|
||||
from cleveractors.core.exceptions import ExecutionError
|
||||
|
||||
|
||||
def _unescape(text: str) -> str:
|
||||
"""Interpret literal ``\\n`` / ``\\t`` in a Gherkin scalar as real whitespace."""
|
||||
return text.replace("\\n", "\n").replace("\\t", "\t")
|
||||
|
||||
|
||||
def _ensure_sandbox(context):
|
||||
"""Create the per-scenario sandbox directory once, resolving symlinks."""
|
||||
if getattr(context, "sandbox", None) is None:
|
||||
context.sandbox = Path(tempfile.mkdtemp()).resolve()
|
||||
return context.sandbox
|
||||
|
||||
|
||||
def _exec_inline(context, code: str) -> None:
|
||||
"""Run *code* as the sole inline tool of an agent, rooted at the sandbox.
|
||||
|
||||
The process working directory is switched to the sandbox for the duration
|
||||
so the helpers' sandbox root (the CWD) is the sandbox, then restored.
|
||||
"""
|
||||
_ensure_sandbox(context)
|
||||
agent = ToolAgent(
|
||||
"inline",
|
||||
{"tools": [{"name": "t", "code": code}], "safe_mode": context.safe_mode},
|
||||
)
|
||||
old_cwd = os.getcwd()
|
||||
os.chdir(context.sandbox)
|
||||
try:
|
||||
context.result = _run(agent.process_message("", dict(context.helper_ctx)))
|
||||
context.error = None
|
||||
except ExecutionError as exc:
|
||||
context.result = None
|
||||
context.error = str(exc)
|
||||
finally:
|
||||
os.chdir(old_cwd)
|
||||
|
||||
|
||||
# ── Given ────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
@given("an inline-code tool agent in safe mode")
|
||||
def step_agent_safe(context):
|
||||
context.safe_mode = True
|
||||
context.helper_ctx = {}
|
||||
|
||||
|
||||
@given("an inline-code tool agent in unsafe mode granting write access")
|
||||
def step_agent_unsafe_write(context):
|
||||
context.safe_mode = False
|
||||
context.helper_ctx = {"_unsafe_mode": True}
|
||||
|
||||
|
||||
@given("an inline-code tool agent in unsafe mode without write access")
|
||||
def step_agent_unsafe_no_write(context):
|
||||
context.safe_mode = False
|
||||
context.helper_ctx = {}
|
||||
|
||||
|
||||
@given('a sandbox file "{name}" containing "{content}"')
|
||||
def step_sandbox_file(context, name, content):
|
||||
sandbox = _ensure_sandbox(context)
|
||||
(sandbox / name).write_text(_unescape(content), encoding="utf-8")
|
||||
|
||||
|
||||
@given('a symlink "{name}" in the sandbox pointing outside the sandbox root')
|
||||
def step_sandbox_symlink(context, name):
|
||||
sandbox = _ensure_sandbox(context)
|
||||
outside = Path(tempfile.mkdtemp()).resolve()
|
||||
target = outside / "secret.txt"
|
||||
target.write_text("outside the root", encoding="utf-8")
|
||||
os.symlink(target, sandbox / name)
|
||||
context.symlink_target = target
|
||||
|
||||
|
||||
# ── When ─────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
@when('the inline code tries to read "{name}"')
|
||||
def step_try_read(context, name):
|
||||
_exec_inline(
|
||||
context,
|
||||
f"try:\n"
|
||||
f" result = read_file({name!r})\n"
|
||||
f"except ValueError as e:\n"
|
||||
f' result = "ERR:" + str(e)\n',
|
||||
)
|
||||
|
||||
|
||||
@when('the inline code writes "{content}" to "{name}" and reads it back')
|
||||
def step_write_read(context, content, name):
|
||||
_exec_inline(
|
||||
context,
|
||||
f"write_file({name!r}, {content!r})\nresult = read_file({name!r})\n",
|
||||
)
|
||||
|
||||
|
||||
@when('the inline code writes "{content}" to "{name}" and returns the write count')
|
||||
def step_write_count(context, content, name):
|
||||
_exec_inline(
|
||||
context,
|
||||
f"n = write_file({name!r}, {content!r})\nresult = str(n)\n",
|
||||
)
|
||||
|
||||
|
||||
@when(
|
||||
'the inline code reads "{name}" with offset {offset:d} and max_chars {max_chars:d}'
|
||||
)
|
||||
def step_read_window(context, name, offset, max_chars):
|
||||
_exec_inline(
|
||||
context,
|
||||
f"try:\n"
|
||||
f" result = read_file({name!r}, {max_chars}, {offset})\n"
|
||||
f"except ValueError as e:\n"
|
||||
f' result = "ERR:" + str(e)\n',
|
||||
)
|
||||
|
||||
|
||||
@when('the inline code appends "{content}" to "{name}" and reads it back')
|
||||
def step_append_read(context, content, name):
|
||||
_exec_inline(
|
||||
context,
|
||||
f'write_file({name!r}, {content!r}, "a")\nresult = read_file({name!r})\n',
|
||||
)
|
||||
|
||||
|
||||
@when('the inline code inserts "{content}" into "{name}" and reads it back')
|
||||
def step_insert_read(context, content, name):
|
||||
_exec_inline(
|
||||
context,
|
||||
f'write_file({name!r}, {content!r}, "insert")\nresult = read_file({name!r})\n',
|
||||
)
|
||||
|
||||
|
||||
@when('the inline code tries to write "{content}" to "{name}"')
|
||||
def step_try_write(context, content, name):
|
||||
_exec_inline(
|
||||
context,
|
||||
f"try:\n"
|
||||
f" write_file({name!r}, {content!r})\n"
|
||||
f' result = "WROTE"\n'
|
||||
f"except ValueError as e:\n"
|
||||
f' result = "ERR:" + str(e)\n',
|
||||
)
|
||||
|
||||
|
||||
@when('the inline code tries to write a non-string value to "{name}"')
|
||||
def step_try_write_nonstr(context, name):
|
||||
_exec_inline(
|
||||
context,
|
||||
f"try:\n"
|
||||
f" write_file({name!r}, 123)\n"
|
||||
f' result = "WROTE"\n'
|
||||
f"except ValueError as e:\n"
|
||||
f' result = "ERR:" + str(e)\n',
|
||||
)
|
||||
|
||||
|
||||
@when(
|
||||
'the inline code performs an invalid-mode write of "{content}" to "{name}" '
|
||||
'with mode "{mode}"'
|
||||
)
|
||||
def step_try_write_mode(context, content, name, mode):
|
||||
_exec_inline(
|
||||
context,
|
||||
f"try:\n"
|
||||
f" write_file({name!r}, {content!r}, {mode!r})\n"
|
||||
f' result = "WROTE"\n'
|
||||
f"except ValueError as e:\n"
|
||||
f' result = "ERR:" + str(e)\n',
|
||||
)
|
||||
|
||||
|
||||
@when('the inline code reads "{name}" with a negative offset')
|
||||
def step_read_negative(context, name):
|
||||
_exec_inline(
|
||||
context,
|
||||
f"try:\n"
|
||||
f" result = read_file({name!r}, None, -5)\n"
|
||||
f"except ValueError as e:\n"
|
||||
f' result = "ERR:" + str(e)\n',
|
||||
)
|
||||
|
||||
|
||||
@when("the inline code reads a file with a non-integer max_chars")
|
||||
def step_read_bad_maxchars(context):
|
||||
_exec_inline(
|
||||
context,
|
||||
"try:\n"
|
||||
' result = read_file("data.txt", "x")\n'
|
||||
"except ValueError as e:\n"
|
||||
' result = "ERR:" + str(e)\n',
|
||||
)
|
||||
|
||||
|
||||
@when("the inline code reads a file with a non-integer offset")
|
||||
def step_read_bad_offset(context):
|
||||
_exec_inline(
|
||||
context,
|
||||
"try:\n"
|
||||
' result = read_file("data.txt", None, "y")\n'
|
||||
"except ValueError as e:\n"
|
||||
' result = "ERR:" + str(e)\n',
|
||||
)
|
||||
|
||||
|
||||
@when("the inline code reads an empty path")
|
||||
def step_read_empty_path(context):
|
||||
_exec_inline(
|
||||
context,
|
||||
"try:\n"
|
||||
' result = read_file("")\n'
|
||||
"except ValueError as e:\n"
|
||||
' result = "ERR:" + str(e)\n',
|
||||
)
|
||||
|
||||
|
||||
# ── Then ─────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
@then('the tool result should be "{expected}"')
|
||||
def step_result_equals(context, expected):
|
||||
assert context.error is None, f"unexpected error: {context.error}"
|
||||
assert context.result == _unescape(expected), (
|
||||
f"expected {_unescape(expected)!r}, got {context.result!r}"
|
||||
)
|
||||
|
||||
|
||||
@then("the tool result should be empty")
|
||||
def step_result_empty(context):
|
||||
assert context.error is None, f"unexpected error: {context.error}"
|
||||
assert context.result == "", f"expected empty result, got {context.result!r}"
|
||||
|
||||
|
||||
@then("the tool execution fails because read_file is not available")
|
||||
def step_read_file_absent(context):
|
||||
assert context.error is not None, "expected a NameError-driven failure"
|
||||
assert "read_file" in context.error and "not available" in context.error, (
|
||||
f"unexpected error message: {context.error}"
|
||||
)
|
||||
|
||||
|
||||
@then('the caught error should mention "{fragment}"')
|
||||
def step_caught_mentions(context, fragment):
|
||||
assert context.error is None, f"unexpected hard error: {context.error}"
|
||||
assert isinstance(context.result, str) and context.result.startswith("ERR:"), (
|
||||
f"expected a caught ValueError, got {context.result!r}"
|
||||
)
|
||||
assert _unescape(fragment) in context.result, (
|
||||
f"expected {fragment!r} in {context.result!r}"
|
||||
)
|
||||
|
||||
|
||||
@then('no file "{name}" should exist in the sandbox')
|
||||
def step_no_file(context, name):
|
||||
assert not (context.sandbox / name).exists(), f"{name} should not have been written"
|
||||
|
||||
|
||||
@then('the escaped symlink target should still contain "{expected}"')
|
||||
def step_symlink_target_intact(context, expected):
|
||||
actual = context.symlink_target.read_text(encoding="utf-8")
|
||||
assert actual == _unescape(expected), (
|
||||
f"symlink target was modified: expected {expected!r}, got {actual!r}"
|
||||
)
|
||||
@@ -0,0 +1,98 @@
|
||||
"""Step implementations for the SandboxRootPolicy containment security tests (ADR-2035)."""
|
||||
|
||||
import os
|
||||
import tempfile
|
||||
from pathlib import Path
|
||||
|
||||
from behave import given, then, when
|
||||
|
||||
from cleveractors.agents.file_access import SandboxRootPolicy
|
||||
|
||||
|
||||
@given("a sandbox root policy")
|
||||
def step_policy(context):
|
||||
context.root = Path(tempfile.mkdtemp()).resolve()
|
||||
context.policy = SandboxRootPolicy(str(context.root))
|
||||
context.resolved = None
|
||||
context.error = None
|
||||
|
||||
|
||||
@given("a sandbox root policy with a sibling directory sharing its name prefix")
|
||||
def step_policy_sibling(context):
|
||||
base = Path(tempfile.mkdtemp()).resolve()
|
||||
root = base / "box"
|
||||
root.mkdir()
|
||||
sibling = base / "box-evil"
|
||||
sibling.mkdir()
|
||||
(sibling / "secret.txt").write_text("outside", encoding="utf-8")
|
||||
context.root = root
|
||||
context.policy = SandboxRootPolicy(str(root))
|
||||
context.sibling_file = str(sibling / "secret.txt")
|
||||
context.resolved = None
|
||||
context.error = None
|
||||
|
||||
|
||||
@given('a symlink "{name}" inside the root targeting a path outside it')
|
||||
def step_symlink_outside(context, name):
|
||||
outside = Path(tempfile.mkdtemp()).resolve()
|
||||
target = outside / "secret.txt"
|
||||
target.write_text("outside", encoding="utf-8")
|
||||
os.symlink(target, context.root / name)
|
||||
|
||||
|
||||
@given('a symlink "{name}" inside the root targeting another path inside it')
|
||||
def step_symlink_inside(context, name):
|
||||
target = context.root / "real.txt"
|
||||
target.write_text("inside", encoding="utf-8")
|
||||
os.symlink(target, context.root / name)
|
||||
|
||||
|
||||
def _resolve(context, path):
|
||||
try:
|
||||
context.resolved = context.policy.resolve(path)
|
||||
context.error = None
|
||||
except ValueError as exc:
|
||||
context.resolved = None
|
||||
context.error = str(exc)
|
||||
|
||||
|
||||
@when('I resolve the relative path "{path}"')
|
||||
def step_resolve_relative(context, path):
|
||||
_resolve(context, path)
|
||||
|
||||
|
||||
@when('I resolve the raw path "{path}"')
|
||||
def step_resolve_raw(context, path):
|
||||
_resolve(context, path)
|
||||
|
||||
|
||||
@when("I resolve an absolute path that is inside the root")
|
||||
def step_resolve_absolute_inside(context):
|
||||
_resolve(context, str(context.root / "inside.txt"))
|
||||
|
||||
|
||||
@when("I resolve an absolute path into the sibling directory")
|
||||
def step_resolve_sibling(context):
|
||||
_resolve(context, context.sibling_file)
|
||||
|
||||
|
||||
@then("resolution is admitted and stays inside the root")
|
||||
def step_admitted(context):
|
||||
assert context.error is None, f"unexpectedly rejected: {context.error}"
|
||||
root = str(context.root)
|
||||
assert context.resolved == root or context.resolved.startswith(root + os.sep), (
|
||||
f"{context.resolved!r} escaped root {root!r}"
|
||||
)
|
||||
|
||||
|
||||
@then("resolution is rejected as outside the root")
|
||||
def step_rejected_outside(context):
|
||||
assert context.error is not None, f"expected rejection, got {context.resolved!r}"
|
||||
assert "outside the sandbox root" in context.error, (
|
||||
f"unexpected rejection message: {context.error}"
|
||||
)
|
||||
|
||||
|
||||
@then("resolution is rejected")
|
||||
def step_rejected(context):
|
||||
assert context.error is not None, f"expected rejection, got {context.resolved!r}"
|
||||
@@ -9,6 +9,7 @@ from unittest.mock import AsyncMock, MagicMock, patch
|
||||
from behave import given, then, when
|
||||
from features.steps._coverage_utils import _catch, _run
|
||||
|
||||
from cleveractors.agents.file_access import FileAccessCore
|
||||
from cleveractors.agents.tool import ToolAgent
|
||||
from cleveractors.core.exceptions import AgentCreationError, ExecutionError
|
||||
|
||||
@@ -328,6 +329,20 @@ def step_fr_missing(context):
|
||||
)
|
||||
|
||||
|
||||
@when("I call file read tool on a symlink escaping the working directory")
|
||||
def step_fr_symlink_escape(context):
|
||||
outside = Path(tempfile.mkdtemp())
|
||||
(outside / "secret.txt").write_text("outside", encoding="utf-8")
|
||||
a = ToolAgent("tc", {"tools": ["file_read"]})
|
||||
old = os.getcwd()
|
||||
os.chdir(str(context.td))
|
||||
os.symlink(outside / "secret.txt", context.td / "link.txt")
|
||||
try:
|
||||
context.error = _run(_catch(a._file_read_tool, {"file": "link.txt"}, None))
|
||||
finally:
|
||||
os.chdir(old)
|
||||
|
||||
|
||||
@when("I call file read tool with empty args")
|
||||
def step_fr_empty(context):
|
||||
a = ToolAgent("tc", {"tools": ["file_read"]})
|
||||
@@ -581,10 +596,9 @@ def step_ps_val_error(context):
|
||||
|
||||
@when("I prepare append content for a section")
|
||||
def step_ap_section(context):
|
||||
a = ToolAgent("tc", {"tools": ["file_write"]})
|
||||
ap = context.td / "ap.txt"
|
||||
ap.write_text("existing\n")
|
||||
context.result = a._prepare_append_content(str(ap), "# Section")
|
||||
context.result = FileAccessCore.prepare_append_content(str(ap), "# Section")
|
||||
|
||||
|
||||
@then("the content should start with a newline")
|
||||
@@ -595,47 +609,45 @@ def step_ap_newline(context):
|
||||
# ── handle insert ────────────────────────────────────────────────────
|
||||
|
||||
|
||||
def _ins_agent(context):
|
||||
def _ins_path(context):
|
||||
ip = context.td / "ins.txt"
|
||||
ip.write_text("a\nb\nc\n")
|
||||
a = ToolAgent("tc", {"tools": ["file_write"]})
|
||||
return a, str(ip)
|
||||
return str(ip)
|
||||
|
||||
|
||||
@when("I insert content at the end of a file")
|
||||
def step_in_end(context):
|
||||
a, ip = _ins_agent(context)
|
||||
lines, _ = a._handle_insert_position(ip, "end_c", "end")
|
||||
ip = _ins_path(context)
|
||||
lines, _ = FileAccessCore.handle_insert_position(ip, "end_c", "end")
|
||||
context.result = lines
|
||||
|
||||
|
||||
@when("I insert content at the start of a file")
|
||||
def step_in_start(context):
|
||||
a, ip = _ins_agent(context)
|
||||
lines, _ = a._handle_insert_position(ip, "start_c", "start")
|
||||
ip = _ins_path(context)
|
||||
lines, _ = FileAccessCore.handle_insert_position(ip, "start_c", "start")
|
||||
context.result = lines
|
||||
|
||||
|
||||
@when("I insert content at line {n} of a file")
|
||||
def step_in_line(context, n):
|
||||
a, ip = _ins_agent(context)
|
||||
lines, _ = a._handle_insert_position(ip, "l2", int(n))
|
||||
ip = _ins_path(context)
|
||||
lines, _ = FileAccessCore.handle_insert_position(ip, "l2", int(n))
|
||||
context.result = lines
|
||||
|
||||
|
||||
@when("I insert content with an invalid position")
|
||||
def step_in_invalid(context):
|
||||
a, ip = _ins_agent(context)
|
||||
ip = _ins_path(context)
|
||||
try:
|
||||
a._handle_insert_position(ip, "x", "middle")
|
||||
except ExecutionError as e:
|
||||
FileAccessCore.handle_insert_position(ip, "x", "middle")
|
||||
except ValueError as e:
|
||||
context.error = str(e)
|
||||
|
||||
|
||||
@when("I insert content into a nonexistent file")
|
||||
def step_in_new(context):
|
||||
a = ToolAgent("tc", {"tools": ["file_write"]})
|
||||
lines, _ = a._handle_insert_position("/n/x.txt", "new", "end")
|
||||
lines, _ = FileAccessCore.handle_insert_position("/n/x.txt", "new", "end")
|
||||
context.result = lines
|
||||
|
||||
|
||||
@@ -854,8 +866,7 @@ def step_append_no_newline(context):
|
||||
fp = os.path.join(td, "no_trailing.txt")
|
||||
with open(fp, "w") as f:
|
||||
f.write("existing")
|
||||
a = ToolAgent("tc", {"tools": ["file_write"]})
|
||||
prefix = a._prepare_append_content(fp, "# Section")
|
||||
prefix = FileAccessCore.prepare_append_content(fp, "# Section")
|
||||
context.result = prefix
|
||||
|
||||
|
||||
@@ -868,15 +879,13 @@ def step_append_double_newline(context):
|
||||
fp = os.path.join(td, "double_nl.txt")
|
||||
with open(fp, "w") as f:
|
||||
f.write("existing\n\n")
|
||||
a = ToolAgent("tc", {"tools": ["file_write"]})
|
||||
prefix = a._prepare_append_content(fp, "# Section")
|
||||
prefix = FileAccessCore.prepare_append_content(fp, "# Section")
|
||||
context.result = prefix
|
||||
|
||||
|
||||
@when("I prepare append content for missing file")
|
||||
def step_append_missing(context):
|
||||
a = ToolAgent("tc", {"tools": ["file_write"]})
|
||||
prefix = a._prepare_append_content("/nonexistent/path.txt", "# Section")
|
||||
prefix = FileAccessCore.prepare_append_content("/nonexistent/path.txt", "# Section")
|
||||
context.result = prefix
|
||||
|
||||
|
||||
|
||||
@@ -133,6 +133,11 @@ Feature: Tool Agent Coverage Gaps
|
||||
When I call file read tool with empty args
|
||||
Then the file read raises requires a file path
|
||||
|
||||
Scenario: File read tool rejects a symlink escaping the sandbox root in safe mode
|
||||
Given a tool agent coverage test environment
|
||||
When I call file read tool on a symlink escaping the working directory
|
||||
Then the file read raises Unsafe file path blocked in safe mode
|
||||
|
||||
Scenario: File write tool writes content successfully
|
||||
Given a tool agent coverage test environment
|
||||
When I call file write tool with valid content in unsafe mode
|
||||
|
||||
@@ -0,0 +1,103 @@
|
||||
"""Robot Framework library for the inline-code file-helper integration tests.
|
||||
|
||||
Drives a real :class:`~cleveractors.agents.tool.ToolAgent` inline-code tool
|
||||
against a real temporary directory — no mocks, no LLM — exercising the
|
||||
sandbox-confined ``read_file`` / ``write_file`` helpers end to end (ADR-2035).
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import os
|
||||
import shutil
|
||||
import tempfile
|
||||
from pathlib import Path
|
||||
|
||||
from cleveractors.agents.tool import ToolAgent
|
||||
from cleveractors.core.exceptions import ExecutionError
|
||||
|
||||
|
||||
class InlineFileHelperTestLib:
|
||||
"""Keywords for integration-style tests of the inline file helpers."""
|
||||
|
||||
def __init__(self) -> None:
|
||||
self._sandbox: Path | None = None
|
||||
self._old_cwd: str | None = None
|
||||
self._write_ctx: dict[str, object] = {}
|
||||
self._safe_mode: bool = False
|
||||
self._result: str | None = None
|
||||
self._error: str | None = None
|
||||
|
||||
# -- lifecycle -------------------------------------------------------
|
||||
|
||||
def start_unsafe_inline_agent_with_write_access(self) -> None:
|
||||
"""Create a real sandbox directory and enter it, granting write access."""
|
||||
self._sandbox = Path(tempfile.mkdtemp()).resolve()
|
||||
self._old_cwd = os.getcwd()
|
||||
os.chdir(self._sandbox)
|
||||
self._safe_mode = False
|
||||
self._write_ctx = {"_unsafe_mode": True}
|
||||
self._result = None
|
||||
self._error = None
|
||||
|
||||
def clean_up_sandbox(self) -> None:
|
||||
"""Restore the working directory and remove the sandbox."""
|
||||
if self._old_cwd is not None:
|
||||
os.chdir(self._old_cwd)
|
||||
self._old_cwd = None
|
||||
if self._sandbox is not None:
|
||||
shutil.rmtree(self._sandbox, ignore_errors=True)
|
||||
self._sandbox = None
|
||||
|
||||
# -- actions ---------------------------------------------------------
|
||||
|
||||
def inline_code_writes_and_reads_back(self, content: str, filename: str) -> str:
|
||||
"""Run inline code that writes *content* to *filename* then reads it back."""
|
||||
code = f"write_file({filename!r}, {content!r})\nresult = read_file({filename!r})\n"
|
||||
return self._run(code)
|
||||
|
||||
def inline_read_is_rejected_for(self, path: str) -> str:
|
||||
"""Run inline code that reads *path*, catching the containment ValueError."""
|
||||
code = (
|
||||
"try:\n"
|
||||
f" result = read_file({path!r})\n"
|
||||
"except ValueError as exc:\n"
|
||||
' result = "ERR:" + str(exc)\n'
|
||||
)
|
||||
return self._run(code)
|
||||
|
||||
# -- assertions ------------------------------------------------------
|
||||
|
||||
def file_in_sandbox_should_contain(self, filename: str, expected: str) -> None:
|
||||
"""Assert the on-disk sandbox file holds exactly *expected*."""
|
||||
assert self._sandbox is not None, "sandbox not started"
|
||||
actual = (self._sandbox / filename).read_text(encoding="utf-8")
|
||||
assert actual == expected, f"expected {expected!r} on disk, got {actual!r}"
|
||||
|
||||
def result_should_equal(self, expected: str) -> None:
|
||||
assert self._error is None, f"unexpected error: {self._error}"
|
||||
assert self._result == expected, f"expected {expected!r}, got {self._result!r}"
|
||||
|
||||
def result_should_report_rejection_of(self, path: str) -> None:
|
||||
assert self._error is None, f"unexpected hard error: {self._error}"
|
||||
assert self._result is not None and self._result.startswith("ERR:"), (
|
||||
f"expected a caught ValueError, got {self._result!r}"
|
||||
)
|
||||
assert path in self._result, f"expected {path!r} named in {self._result!r}"
|
||||
|
||||
# -- internals -------------------------------------------------------
|
||||
|
||||
def _run(self, code: str) -> str:
|
||||
agent = ToolAgent(
|
||||
"inline",
|
||||
{"tools": [{"name": "t", "code": code}], "safe_mode": self._safe_mode},
|
||||
)
|
||||
try:
|
||||
self._result = asyncio.run(
|
||||
agent.process_message("", dict(self._write_ctx))
|
||||
)
|
||||
self._error = None
|
||||
except ExecutionError as exc:
|
||||
self._result = None
|
||||
self._error = str(exc)
|
||||
return self._result or (self._error or "")
|
||||
@@ -0,0 +1,23 @@
|
||||
*** Settings ***
|
||||
Documentation Integration tests for the inline-code sandboxed file helpers
|
||||
... (ADR-2035). A real ToolAgent runs an inline-code tool against
|
||||
... a real temporary directory — no mocks, no LLM — reading and
|
||||
... writing an actual file within the sandbox and rejecting an
|
||||
... attempt to escape the sandbox root.
|
||||
Library InlineFileHelperTestLib.py
|
||||
Test Setup Start Unsafe Inline Agent With Write Access
|
||||
Test Teardown Clean Up Sandbox
|
||||
|
||||
*** Test Cases ***
|
||||
Inline Code Writes And Reads A Real File Within The Sandbox
|
||||
[Documentation] write_file persists to disk and read_file returns the
|
||||
... exact bytes back, all inside the sandbox root.
|
||||
${result}= Inline Code Writes And Reads Back persisted to disk report.txt
|
||||
Result Should Equal persisted to disk
|
||||
File In Sandbox Should Contain report.txt persisted to disk
|
||||
|
||||
Inline Code Cannot Escape The Sandbox Root
|
||||
[Documentation] A parent-traversal read is rejected with a ValueError
|
||||
... naming the offending path.
|
||||
${result}= Inline Read Is Rejected For ../escape.txt
|
||||
Result Should Report Rejection Of ../escape.txt
|
||||
@@ -0,0 +1,441 @@
|
||||
"""Sandbox-confined file access cores shared by the built-in ``file_read`` /
|
||||
``file_write`` tools and the inline-code ``read_file`` / ``write_file`` helpers.
|
||||
|
||||
This module realises ADR-2035's *single validation surface* decision (D-2):
|
||||
containment, ``offset``/``max_chars`` read windowing (ADR-2033), and the
|
||||
``w``/``a``/``insert`` write semantics (§4.5.5) each have exactly one
|
||||
implementation here, so the two public surfaces — the LLM-facing tools and the
|
||||
inline-code helpers — cannot drift apart.
|
||||
|
||||
Design (see ``docs/adr/ADR-2035-inline-code-sandboxed-file-access.md``):
|
||||
|
||||
* :class:`SandboxRootPolicy` — the containment boundary (D-4). A path is
|
||||
admitted iff its fully resolved real path (``..`` collapsed, symlinks
|
||||
followed) is the sandbox root or a descendant of it. It is a *Specification*
|
||||
object: it answers "is this path admissible?" and nothing else.
|
||||
* :class:`FileAccessCore` — a *Facade* over the windowed read and moded write
|
||||
mechanics. It performs I/O only; it does not decide policy.
|
||||
* :class:`InlineFileHelperFactory` — a *Factory* that binds the two inline-code
|
||||
callables (``read_file`` / ``write_file``) to a core and an invocation
|
||||
context, applying the mode gating of D-5.
|
||||
|
||||
The helper-facing surface raises :class:`ValueError` on every rejection so an
|
||||
inline-code body can catch it with the sandbox's own error vocabulary
|
||||
(§13.2.1); the tool adapters translate core failures into ``ExecutionError``
|
||||
for the tool-calling protocol.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
from dataclasses import dataclass
|
||||
from typing import Any, Callable, Literal, Optional, Union
|
||||
|
||||
__all__ = [
|
||||
"PositionType",
|
||||
"WriteMode",
|
||||
"FileReadResult",
|
||||
"FileWriteResult",
|
||||
"SandboxRootPolicy",
|
||||
"FileAccessCore",
|
||||
"InlineFileHelperFactory",
|
||||
"coerce_read_window",
|
||||
]
|
||||
|
||||
PositionType = Union[None, int, Literal["start", "end"]]
|
||||
WriteMode = Literal["w", "a", "insert"]
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class FileReadResult:
|
||||
"""Outcome of a windowed file read (ADR-2033 semantics).
|
||||
|
||||
Attributes:
|
||||
content: The decoded window ``content[offset:window_end]``.
|
||||
char_count: Total character count of the whole file.
|
||||
line_count: Total line count of the whole file.
|
||||
window_end: Character position where the returned window stops.
|
||||
truncated: True when ``max_chars`` stopped the window before EOF.
|
||||
at_eof_beyond: True when ``offset`` landed at or beyond EOF
|
||||
(``offset > 0 and offset >= char_count``); ``content`` is empty.
|
||||
"""
|
||||
|
||||
content: str
|
||||
char_count: int
|
||||
line_count: int
|
||||
window_end: int
|
||||
truncated: bool
|
||||
at_eof_beyond: bool
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class FileWriteResult:
|
||||
"""Outcome of a moded write (§4.5.5 semantics).
|
||||
|
||||
Attributes:
|
||||
chars_written: Number of characters written (``len(content)``).
|
||||
insert_location: 1-indexed line the content was inserted before.
|
||||
``None`` for ``w``/``a`` modes, where no line position applies.
|
||||
"""
|
||||
|
||||
chars_written: int
|
||||
insert_location: Optional[int] = None
|
||||
|
||||
|
||||
def coerce_read_window(max_chars: Any, offset: Any) -> tuple[Optional[int], int]:
|
||||
"""Coerce and validate the ``max_chars`` / ``offset`` read-window pair.
|
||||
|
||||
Mirrors the ``int()`` coercion the ``file_read`` tool established
|
||||
(ADR-2033 D-4): an integer-valued float is truncated, only genuinely
|
||||
non-numeric input is rejected, and ``offset`` must be non-negative.
|
||||
|
||||
Args:
|
||||
max_chars: Optional character cap (``None`` means "to EOF").
|
||||
offset: Character position to start from (``None`` treated as ``0``).
|
||||
|
||||
Returns:
|
||||
The coerced ``(max_chars, offset)`` pair.
|
||||
|
||||
Raises:
|
||||
ValueError: If either value is non-numeric, or ``offset`` is negative.
|
||||
"""
|
||||
coerced_max: Optional[int] = None
|
||||
if max_chars is not None:
|
||||
try:
|
||||
coerced_max = int(max_chars)
|
||||
except (TypeError, ValueError) as exc:
|
||||
raise ValueError(
|
||||
f"max_chars must be an integer, got {max_chars!r}"
|
||||
) from exc
|
||||
|
||||
try:
|
||||
coerced_offset = 0 if offset is None else int(offset)
|
||||
except (TypeError, ValueError) as exc:
|
||||
raise ValueError(f"offset must be an integer, got {offset!r}") from exc
|
||||
|
||||
if coerced_offset < 0:
|
||||
raise ValueError(f"offset must be non-negative, got {coerced_offset}")
|
||||
|
||||
return coerced_max, coerced_offset
|
||||
|
||||
|
||||
class SandboxRootPolicy:
|
||||
"""The sandbox-root containment boundary (ADR-2035 D-4).
|
||||
|
||||
A path is admitted iff its fully resolved real path — ``..`` collapsed and
|
||||
symlinks followed — is the sandbox root or a descendant of it. Raw ``..``
|
||||
traversal components and ``~`` home-directory expansion are rejected
|
||||
outright, before resolution.
|
||||
|
||||
The root defaults to the process working directory at construction time,
|
||||
matching the boundary the ``file_write`` tool already enforces. Follows the
|
||||
*Specification* pattern: it encapsulates one reusable, composable admission
|
||||
rule and owns no I/O.
|
||||
"""
|
||||
|
||||
def __init__(self, root: Optional[str] = None) -> None:
|
||||
"""Initialise the policy.
|
||||
|
||||
Args:
|
||||
root: Sandbox root directory. Defaults to the current working
|
||||
directory. The value is resolved to a real path (symlinks
|
||||
followed) so containment comparisons are stable.
|
||||
"""
|
||||
self._root: str = os.path.realpath(root if root else os.getcwd())
|
||||
|
||||
def is_within(self, resolved_path: str) -> bool:
|
||||
"""Return whether an already-resolved real path lies within the root.
|
||||
|
||||
Args:
|
||||
resolved_path: An ``os.path.realpath`` result to test.
|
||||
|
||||
Returns:
|
||||
True if the path is the root itself or a descendant of it.
|
||||
"""
|
||||
return resolved_path == self._root or resolved_path.startswith(
|
||||
self._root + os.sep
|
||||
)
|
||||
|
||||
def resolve(self, path: str) -> str:
|
||||
"""Resolve ``path`` against the root, enforcing containment.
|
||||
|
||||
Args:
|
||||
path: A relative or absolute path supplied by inline code.
|
||||
|
||||
Returns:
|
||||
The resolved real path, guaranteed to be within the sandbox root.
|
||||
|
||||
Raises:
|
||||
ValueError: Naming the offending path, when it is empty/non-string,
|
||||
begins with ``~``, contains a ``..`` traversal component, or
|
||||
resolves (symlinks followed) outside the sandbox root.
|
||||
"""
|
||||
if not isinstance(path, str) or not path:
|
||||
raise ValueError(f"file path must be a non-empty string, got {path!r}")
|
||||
if path.startswith("~"):
|
||||
raise ValueError(f"home-directory paths ('~') are not allowed: {path!r}")
|
||||
if self._has_parent_traversal(path):
|
||||
raise ValueError(f"parent traversal ('..') is not allowed: {path!r}")
|
||||
|
||||
base = path if os.path.isabs(path) else os.path.join(self._root, path)
|
||||
resolved = os.path.realpath(base)
|
||||
if not self.is_within(resolved):
|
||||
raise ValueError(f"path resolves outside the sandbox root: {path!r}")
|
||||
return resolved
|
||||
|
||||
@staticmethod
|
||||
def _has_parent_traversal(path: str) -> bool:
|
||||
"""Return whether any path component is a ``..`` parent reference."""
|
||||
components = path.replace("\\", "/").split("/")
|
||||
return ".." in components
|
||||
|
||||
|
||||
class FileAccessCore:
|
||||
"""Facade over the windowed-read and moded-write file mechanics.
|
||||
|
||||
Backs both the built-in tools and the inline-code helpers so the read
|
||||
windowing (ADR-2033) and the ``w``/``a``/``insert`` write semantics
|
||||
(§4.5.5) exist in exactly one place (ADR-2035 D-2). This class performs I/O
|
||||
only; containment is the responsibility of :class:`SandboxRootPolicy`, and
|
||||
the callers decide which paths reach it.
|
||||
"""
|
||||
|
||||
def __init__(self, root_policy: Optional[SandboxRootPolicy] = None) -> None:
|
||||
"""Initialise the core.
|
||||
|
||||
Args:
|
||||
root_policy: Containment policy used by the inline-code helpers.
|
||||
Defaults to a policy rooted at the current working directory.
|
||||
"""
|
||||
self._root_policy: SandboxRootPolicy = root_policy or SandboxRootPolicy()
|
||||
|
||||
@property
|
||||
def root_policy(self) -> SandboxRootPolicy:
|
||||
"""The containment policy backing helper path resolution."""
|
||||
return self._root_policy
|
||||
|
||||
def read_window(
|
||||
self, path: str, max_chars: Optional[int], offset: int
|
||||
) -> FileReadResult:
|
||||
"""Read a UTF-8 file and return the ``[offset, offset+max_chars)`` window.
|
||||
|
||||
Args:
|
||||
path: Path to an existing, readable file (already validated by the
|
||||
caller — this method performs no containment check).
|
||||
max_chars: Character cap for the window, or ``None`` to read to EOF.
|
||||
offset: Character position to start the window at.
|
||||
|
||||
Returns:
|
||||
A :class:`FileReadResult` describing the window and the whole file.
|
||||
|
||||
Raises:
|
||||
OSError: If the file cannot be opened or read.
|
||||
UnicodeDecodeError: If the file is not valid UTF-8.
|
||||
"""
|
||||
with open(path, "r", encoding="utf-8") as handle:
|
||||
content = handle.read()
|
||||
|
||||
char_count = len(content)
|
||||
line_count = content.count("\n") + 1
|
||||
|
||||
# offset == 0 always falls through, even for an empty file, so the
|
||||
# no-offset path stays byte-for-byte identical to the pre-offset
|
||||
# behaviour (ADR-2033 D-4).
|
||||
if offset > 0 and offset >= char_count:
|
||||
return FileReadResult(
|
||||
content="",
|
||||
char_count=char_count,
|
||||
line_count=line_count,
|
||||
window_end=offset,
|
||||
truncated=False,
|
||||
at_eof_beyond=True,
|
||||
)
|
||||
|
||||
window_end = (
|
||||
char_count if max_chars is None else min(char_count, offset + max_chars)
|
||||
)
|
||||
truncated = max_chars is not None and window_end < char_count
|
||||
return FileReadResult(
|
||||
content=content[offset:window_end],
|
||||
char_count=char_count,
|
||||
line_count=line_count,
|
||||
window_end=window_end,
|
||||
truncated=truncated,
|
||||
at_eof_beyond=False,
|
||||
)
|
||||
|
||||
def write(
|
||||
self,
|
||||
path: str,
|
||||
content: str,
|
||||
mode: WriteMode = "w",
|
||||
position: PositionType = None,
|
||||
) -> FileWriteResult:
|
||||
"""Write ``content`` to ``path`` honouring the §4.5.5 write modes.
|
||||
|
||||
This is the single implementation of the write mechanics — both the
|
||||
``file_write`` tool and the ``write_file`` inline helper call this
|
||||
method rather than each opening/writing the file independently
|
||||
(ADR-2035 D-2).
|
||||
|
||||
Args:
|
||||
path: Destination path (already validated by the caller — this
|
||||
method performs no containment check).
|
||||
content: The content to write.
|
||||
mode: ``w`` (overwrite), ``a`` (append), or ``insert``.
|
||||
position: Insert position for ``insert`` mode (see §4.5.5).
|
||||
|
||||
Returns:
|
||||
A :class:`FileWriteResult` with the characters written and, for
|
||||
``insert`` mode, the 1-indexed insertion line.
|
||||
|
||||
Raises:
|
||||
ValueError: If ``mode`` is not one of the defined values.
|
||||
OSError: If the file cannot be written.
|
||||
"""
|
||||
if mode == "w":
|
||||
with open(path, "w", encoding="utf-8") as handle:
|
||||
handle.write(content)
|
||||
return FileWriteResult(chars_written=len(content))
|
||||
|
||||
if mode == "a":
|
||||
to_append = self.prepare_append_content(path, content)
|
||||
with open(path, "a", encoding="utf-8") as handle:
|
||||
handle.write(to_append)
|
||||
return FileWriteResult(chars_written=len(content))
|
||||
|
||||
if mode == "insert":
|
||||
lines, location = self.handle_insert_position(path, content, position)
|
||||
with open(path, "w", encoding="utf-8") as handle:
|
||||
handle.writelines(lines)
|
||||
return FileWriteResult(chars_written=len(content), insert_location=location)
|
||||
|
||||
raise ValueError(
|
||||
f"Invalid mode {mode!r}. Use 'w' (write), 'a' (append), or 'insert'."
|
||||
)
|
||||
|
||||
@staticmethod
|
||||
def prepare_append_content(path: str, content: str) -> str:
|
||||
"""Prepare append content, adding section spacing for Markdown headings.
|
||||
|
||||
Only content that looks like a document section (starts with ``#``)
|
||||
receives a separating blank line, so simple appends stay unaffected.
|
||||
"""
|
||||
if not content.lstrip().startswith("#"):
|
||||
return content
|
||||
|
||||
try:
|
||||
with open(path, "r", encoding="utf-8") as handle:
|
||||
existing = handle.read()
|
||||
except FileNotFoundError:
|
||||
return content
|
||||
|
||||
if existing and not existing.endswith("\n\n"):
|
||||
prefix = "\n" if existing.endswith("\n") else "\n\n"
|
||||
else:
|
||||
prefix = ""
|
||||
return prefix + content
|
||||
|
||||
@staticmethod
|
||||
def handle_insert_position(
|
||||
path: str, content: str, position: PositionType
|
||||
) -> tuple[list[str], int]:
|
||||
"""Compute the line list and 1-indexed location for an ``insert`` write."""
|
||||
try:
|
||||
with open(path, "r", encoding="utf-8") as handle:
|
||||
lines = handle.readlines()
|
||||
except FileNotFoundError:
|
||||
lines = []
|
||||
|
||||
formatted = content if content.endswith("\n") else content + "\n"
|
||||
|
||||
if position is None or position == "end":
|
||||
lines.append(formatted)
|
||||
return lines, len(lines)
|
||||
if position == "start":
|
||||
lines.insert(0, formatted)
|
||||
return lines, 1
|
||||
if isinstance(position, int):
|
||||
line_idx = max(0, min(position - 1, len(lines)))
|
||||
lines.insert(line_idx, formatted)
|
||||
return lines, line_idx + 1
|
||||
|
||||
raise ValueError(
|
||||
f"Invalid position {position!r}. Use 'start', 'end', or a line number."
|
||||
)
|
||||
|
||||
|
||||
class InlineFileHelperFactory:
|
||||
"""Builds the ``read_file`` / ``write_file`` callables injected into inline
|
||||
code in unsafe mode (ADR-2035 D-1, D-5).
|
||||
|
||||
The factory captures whether the invocation context opted into writes
|
||||
(``_unsafe_mode``); ``read_file`` is always permitted (subject to
|
||||
containment), while ``write_file`` refuses without that opt-in — preserving
|
||||
the read/write privilege split. Every rejection is a :class:`ValueError` so
|
||||
inline code can catch it with the sandbox's own vocabulary.
|
||||
|
||||
``build()`` returns closures rather than bound methods: inline code that
|
||||
reaches for ``read_file.__self__`` finds nothing, so the unconfined
|
||||
:class:`FileAccessCore` (whose own methods perform no containment check —
|
||||
that is the caller's responsibility) is not reachable through the returned
|
||||
callables. This is defense-in-depth only — the inline sandbox's
|
||||
``__import__`` builtin already makes arbitrary filesystem access possible
|
||||
by other means — but it closes the specific, cleaner bypass path a bound
|
||||
method would otherwise expose.
|
||||
"""
|
||||
|
||||
def __init__(self, core: FileAccessCore, *, write_enabled: bool) -> None:
|
||||
"""Initialise the factory.
|
||||
|
||||
Args:
|
||||
core: The shared file-access core to delegate I/O to.
|
||||
write_enabled: Whether the invocation context contained
|
||||
``_unsafe_mode: true`` (gates ``write_file``).
|
||||
"""
|
||||
self._core = core
|
||||
self._write_enabled = write_enabled
|
||||
|
||||
def build(self) -> dict[str, Callable[..., Any]]:
|
||||
"""Return the injectable ``{name: callable}`` mapping."""
|
||||
core = self._core
|
||||
write_enabled = self._write_enabled
|
||||
|
||||
def read_file(
|
||||
path: str, max_chars: Optional[int] = None, offset: int = 0
|
||||
) -> str:
|
||||
"""Return the raw decoded contents of ``path`` within the sandbox
|
||||
root, honouring the ADR-2033 ``offset``/``max_chars`` windowing.
|
||||
Returns the content itself — not the ``[FILE_READ_SUCCESS]``
|
||||
envelope.
|
||||
|
||||
Raises:
|
||||
ValueError: On containment violation or invalid window
|
||||
arguments.
|
||||
"""
|
||||
resolved = core.root_policy.resolve(path)
|
||||
coerced_max, coerced_offset = coerce_read_window(max_chars, offset)
|
||||
return core.read_window(resolved, coerced_max, coerced_offset).content
|
||||
|
||||
def write_file(path: str, content: str, mode: WriteMode = "w") -> int:
|
||||
"""Write ``content`` to ``path`` within the sandbox root; return
|
||||
the number of characters written.
|
||||
|
||||
Raises:
|
||||
ValueError: When the context did not opt into writes
|
||||
(``_unsafe_mode``), on containment violation, when
|
||||
``content`` is not a string, or on an invalid mode.
|
||||
"""
|
||||
if not write_enabled:
|
||||
raise ValueError(
|
||||
"write_file requires unsafe mode "
|
||||
"('_unsafe_mode: true' in the invocation context)"
|
||||
)
|
||||
if not isinstance(content, str):
|
||||
raise ValueError(
|
||||
f"content must be a string, got {type(content).__name__}"
|
||||
)
|
||||
resolved = core.root_policy.resolve(path)
|
||||
return core.write(resolved, content, mode).chars_written
|
||||
|
||||
return {"read_file": read_file, "write_file": write_file}
|
||||
+83
-124
@@ -12,11 +12,16 @@ import logging
|
||||
import os
|
||||
import re
|
||||
import subprocess # nosec B404: intentional subprocess for tool execution
|
||||
from typing import Any, List, Literal, Optional, Union
|
||||
from typing import Any, List, Optional
|
||||
|
||||
import aiohttp
|
||||
|
||||
from cleveractors.agents.base import Agent
|
||||
from cleveractors.agents.file_access import (
|
||||
FileAccessCore,
|
||||
InlineFileHelperFactory,
|
||||
SandboxRootPolicy,
|
||||
)
|
||||
from cleveractors.agents.llm_tools import SKILL_TOOL_NAME
|
||||
from cleveractors.core.exceptions import AgentCreationError, ExecutionError
|
||||
from cleveractors.templates.renderer import TemplateRenderer
|
||||
@@ -456,6 +461,23 @@ class ToolAgent(Agent):
|
||||
# Add json module for convenience
|
||||
safe_globals["json"] = __import__("json") # type: ignore[assignment]
|
||||
|
||||
# Inject the sandbox-confined file helpers when this agent's safe_mode
|
||||
# is false, or the invocation context opts into _unsafe_mode (ADR-2035
|
||||
# D-1/D-5; §4.5.2 point 3) — the same two conditions §4.5.4 already
|
||||
# uses to relax file_read/file_write. When neither holds, the names
|
||||
# are never bound, so referencing read_file/write_file raises
|
||||
# NameError — which _execute_python_code surfaces as a helpful
|
||||
# ExecutionError below.
|
||||
ctx_unsafe = isinstance(context, dict) and bool(
|
||||
context.get("_unsafe_mode", False)
|
||||
)
|
||||
helpers_enabled = (not self.safe_mode) or ctx_unsafe
|
||||
if helpers_enabled:
|
||||
helpers = InlineFileHelperFactory(
|
||||
self._make_file_core(), write_enabled=ctx_unsafe
|
||||
).build()
|
||||
safe_globals.update(helpers)
|
||||
|
||||
try:
|
||||
# Execute the code in a shared environment so helper functions see locals
|
||||
exec_env = dict(safe_globals)
|
||||
@@ -617,6 +639,15 @@ class ToolAgent(Agent):
|
||||
except Exception as e:
|
||||
raise ExecutionError(f"HTTP request failed: {e}") from e
|
||||
|
||||
def _make_file_core(self) -> FileAccessCore:
|
||||
"""Build a file-access core rooted at the current working directory.
|
||||
|
||||
A fresh core is created per operation so the sandbox root tracks the
|
||||
process CWD at call time (Behave/Robot suites ``chdir`` into temp
|
||||
directories), matching the tool's long-standing dynamic-root behavior.
|
||||
"""
|
||||
return FileAccessCore(SandboxRootPolicy())
|
||||
|
||||
async def _file_read_tool(
|
||||
self, args: dict[str, Any], context: Optional[dict[str, Any]]
|
||||
) -> str:
|
||||
@@ -674,13 +705,18 @@ class ToolAgent(Agent):
|
||||
f"grep, find, ls, cat. Use file_read only for file/directory paths."
|
||||
)
|
||||
|
||||
unsafe_ctx = bool(context and context.get("_unsafe_mode", False))
|
||||
if self.safe_mode:
|
||||
if ".." in filepath:
|
||||
raise ExecutionError("Unsafe file path blocked in safe mode")
|
||||
if filepath.startswith("/") and not (
|
||||
context and context.get("_unsafe_mode", False)
|
||||
):
|
||||
if filepath.startswith("/") and not unsafe_ctx:
|
||||
raise ExecutionError("Unsafe file path blocked in safe mode")
|
||||
if not unsafe_ctx and not os.path.isabs(filepath):
|
||||
# Close the symlink-escape gap (ADR-2035 D-4): reject a
|
||||
# relative path that resolves (symlinks followed) outside the
|
||||
# sandbox root even when it carries no literal '..' component.
|
||||
if not SandboxRootPolicy().is_within(os.path.realpath(filepath)):
|
||||
raise ExecutionError("Unsafe file path blocked in safe mode")
|
||||
|
||||
max_chars = args.get("max_chars")
|
||||
if max_chars is not None:
|
||||
@@ -717,39 +753,30 @@ class ToolAgent(Agent):
|
||||
result_parts.append(f" {marker} {entry}{size_str}")
|
||||
return "\n".join(result_parts)
|
||||
|
||||
with open(filepath, "r", encoding="utf-8") as f:
|
||||
content = f.read()
|
||||
|
||||
char_count = len(content)
|
||||
line_count = content.count("\n") + 1
|
||||
|
||||
# offset == 0 always falls through here, even for an empty file,
|
||||
# so the no-offset path stays byte-for-byte identical to the
|
||||
# pre-offset behavior (ADR-2033 D-4).
|
||||
if offset > 0 and offset >= char_count:
|
||||
header = (
|
||||
f"[FILE_READ_SUCCESS]📄 File: {filepath} | "
|
||||
f"Lines: {line_count} | Size: {char_count} chars | "
|
||||
f"NO_MORE_CONTENT_AT_OFFSET: {offset}"
|
||||
)
|
||||
return f"{header}\n[FILE_CONTENT_START]\n[FILE_CONTENT_END]"
|
||||
|
||||
window_end = (
|
||||
char_count if max_chars is None else min(char_count, offset + max_chars)
|
||||
# Delegate the offset/max_chars windowing to the shared core so the
|
||||
# tool and the inline-code read_file helper cannot drift (ADR-2035
|
||||
# D-2; ADR-2033 windowing semantics).
|
||||
read_result = self._make_file_core().read_window(
|
||||
filepath, max_chars, offset
|
||||
)
|
||||
truncated = max_chars is not None and window_end < char_count
|
||||
content = content[offset:window_end]
|
||||
|
||||
header = (
|
||||
f"[FILE_READ_SUCCESS]📄 File: {filepath} | "
|
||||
f"Lines: {line_count} | Size: {char_count} chars"
|
||||
f"Lines: {read_result.line_count} | "
|
||||
f"Size: {read_result.char_count} chars"
|
||||
)
|
||||
if truncated:
|
||||
if read_result.at_eof_beyond:
|
||||
header += f" | NO_MORE_CONTENT_AT_OFFSET: {offset}"
|
||||
return f"{header}\n[FILE_CONTENT_START]\n[FILE_CONTENT_END]"
|
||||
|
||||
if read_result.truncated:
|
||||
header += (
|
||||
f" | TRUNCATED to {max_chars} chars"
|
||||
f" | MORE_CONTENT_AT_OFFSET: {window_end}"
|
||||
f" | MORE_CONTENT_AT_OFFSET: {read_result.window_end}"
|
||||
)
|
||||
return f"{header}\n[FILE_CONTENT_START]\n{content}\n[FILE_CONTENT_END]"
|
||||
return (
|
||||
f"{header}\n[FILE_CONTENT_START]\n"
|
||||
f"{read_result.content}\n[FILE_CONTENT_END]"
|
||||
)
|
||||
except FileNotFoundError:
|
||||
parent_dir = os.path.dirname(filepath) or "."
|
||||
raise ExecutionError(
|
||||
@@ -812,10 +839,12 @@ class ToolAgent(Agent):
|
||||
logger.error("Directory traversal pattern detected in: %s", filepath)
|
||||
raise ExecutionError("Unsafe file path blocked in safe mode")
|
||||
|
||||
# Check if path escapes working directory
|
||||
path_escapes_working_dir = (
|
||||
not absolute_path.startswith(working_dir + os.sep)
|
||||
and absolute_path != working_dir
|
||||
# Check if path escapes the sandbox root. Resolution follows symlinks
|
||||
# (realpath) via the shared containment policy so a symlink that
|
||||
# escapes the root is rejected too (ADR-2035 D-4) — a strict
|
||||
# strengthening of the previous abspath-only check.
|
||||
path_escapes_working_dir = not SandboxRootPolicy().is_within(
|
||||
os.path.realpath(normalized_path)
|
||||
)
|
||||
|
||||
if not unsafe_mode:
|
||||
@@ -849,71 +878,17 @@ class ToolAgent(Agent):
|
||||
logger.error("Home directory expansion blocked for %s", filepath)
|
||||
raise ExecutionError("Home directory paths (~) are not allowed")
|
||||
|
||||
def _prepare_append_content(self, filepath: str, content: str) -> str:
|
||||
"""Prepare content for append mode with proper spacing for sections."""
|
||||
# Only add spacing if content looks like a document section (starts with #)
|
||||
# This prevents breaking simple append operations
|
||||
is_section = content.lstrip().startswith("#")
|
||||
|
||||
if not is_section:
|
||||
# Simple append without spacing
|
||||
return content
|
||||
|
||||
try:
|
||||
with open(filepath, "r", encoding="utf-8") as f:
|
||||
existing_content = f.read()
|
||||
|
||||
# Add spacing for section-based content
|
||||
if existing_content and not existing_content.endswith("\n\n"):
|
||||
if existing_content.endswith("\n"):
|
||||
prefix = "\n" # Has one newline, add one more
|
||||
else:
|
||||
prefix = "\n\n" # No newline at all, add two
|
||||
else:
|
||||
prefix = ""
|
||||
except FileNotFoundError:
|
||||
prefix = "" # File doesn't exist yet
|
||||
|
||||
return prefix + content
|
||||
|
||||
def _handle_insert_position(
|
||||
self,
|
||||
filepath: str,
|
||||
content: str,
|
||||
position: Union[None, int, Literal["start", "end"]],
|
||||
) -> tuple[list[str], int]:
|
||||
"""Handle insert mode positioning logic."""
|
||||
try:
|
||||
with open(filepath, "r", encoding="utf-8") as f:
|
||||
existing_lines = f.readlines()
|
||||
except FileNotFoundError:
|
||||
existing_lines = []
|
||||
|
||||
# Ensure content ends with newline
|
||||
formatted_content = content if content.endswith("\n") else content + "\n"
|
||||
|
||||
# Determine insertion position
|
||||
if position is None or position == "end":
|
||||
existing_lines.append(formatted_content)
|
||||
insert_location = len(existing_lines)
|
||||
elif position == "start":
|
||||
existing_lines.insert(0, formatted_content)
|
||||
insert_location = 1
|
||||
elif isinstance(position, int):
|
||||
line_idx = max(0, min(position - 1, len(existing_lines)))
|
||||
existing_lines.insert(line_idx, formatted_content)
|
||||
insert_location = line_idx + 1
|
||||
else:
|
||||
raise ExecutionError(
|
||||
f"Invalid position '{position}'. Use 'start', 'end', or line number."
|
||||
)
|
||||
|
||||
return existing_lines, insert_location
|
||||
|
||||
async def _file_write_tool(
|
||||
self, args: dict[str, Any], context: Optional[dict[str, Any]]
|
||||
) -> str:
|
||||
"""File writing tool with support for write, append, and insert modes."""
|
||||
"""File writing tool with support for write, append, and insert modes.
|
||||
|
||||
Delegates the actual write mechanics to
|
||||
:meth:`FileAccessCore.write` (ADR-2035 D-2) — this method only
|
||||
validates inputs/path safety and formats the result message; the
|
||||
``w``/``a``/``insert`` semantics live in exactly one place, shared
|
||||
with the inline-code ``write_file`` helper.
|
||||
"""
|
||||
filepath = args.get("file", "")
|
||||
content = args.get("content", "")
|
||||
mode = args.get("mode", "w") # w=write, a=append, insert=insert at pos
|
||||
@@ -931,38 +906,22 @@ class ToolAgent(Agent):
|
||||
self._validate_file_path_safety(filepath, unsafe_mode)
|
||||
|
||||
try:
|
||||
if mode == "w":
|
||||
# Standard write (overwrite)
|
||||
with open(filepath, "w", encoding="utf-8") as f:
|
||||
f.write(content)
|
||||
return f"Successfully wrote {len(content)} characters to {filepath}"
|
||||
|
||||
if mode == "a":
|
||||
# Append mode with proper spacing
|
||||
content_to_append = self._prepare_append_content(filepath, content)
|
||||
with open(filepath, "a", encoding="utf-8") as f:
|
||||
f.write(content_to_append)
|
||||
return f"Successfully appended {len(content)} characters to {filepath}"
|
||||
|
||||
if mode == "insert":
|
||||
# Insert mode at specified position
|
||||
existing_lines, insert_location = self._handle_insert_position(
|
||||
filepath, content, position
|
||||
)
|
||||
with open(filepath, "w", encoding="utf-8") as f:
|
||||
f.writelines(existing_lines)
|
||||
return (
|
||||
f"Successfully inserted {len(content)} characters "
|
||||
f"at line {insert_location} in {filepath}"
|
||||
)
|
||||
|
||||
raise ExecutionError(
|
||||
f"Invalid mode '{mode}'. Use 'w' (write), 'a' (append), or 'insert'."
|
||||
)
|
||||
result = self._make_file_core().write(filepath, content, mode, position)
|
||||
except Exception as e:
|
||||
logger.error("File write failed for %s: %s", filepath, e)
|
||||
raise ExecutionError(f"File write failed: {e}") from e
|
||||
|
||||
if mode == "a":
|
||||
return (
|
||||
f"Successfully appended {result.chars_written} characters to {filepath}"
|
||||
)
|
||||
if mode == "insert":
|
||||
return (
|
||||
f"Successfully inserted {result.chars_written} characters "
|
||||
f"at line {result.insert_location} in {filepath}"
|
||||
)
|
||||
return f"Successfully wrote {result.chars_written} characters to {filepath}"
|
||||
|
||||
async def _progress_bar_tool(
|
||||
self, args: dict[str, Any], context: Optional[dict[str, Any]]
|
||||
) -> str:
|
||||
|
||||
Reference in New Issue
Block a user