Harden sandbox file access against symlink TOCTOU (CWE-367) with symlink-safe open #105

New issue

Closed

opened 2026-08-04 14:27:09 +00:00 by CoreRasurae · 0 comments

CoreRasurae commented

2026-08-04 14:27:09 +00:00

Member

Copy link

Metadata

Commit Message: feat(agents): symlink-safe sandbox file access closing TOCTOU window
Branch: feature/m1-symlink-safe-file-access (provisional — align the milestone number with the milestone assigned at triage)

Background and context

ADR-2035 gave inline code the sandbox-confined read_file/write_file helpers,
backed by the shared cores in cleveractors.agents.file_access
(SandboxRootPolicy, FileAccessCore) introduced in PR #103. Containment is
enforced by SandboxRootPolicy.resolve, which computes the fully resolved real
path (os.path.realpath, symlinks followed) and checks it is within the sandbox
root at validation time. The helper/tool then opens that resolved path via
FileAccessCore.read_window / FileAccessCore.write (plain open(...)).

Because validation and the subsequent open() are two separate steps operating
on a path string, a time-of-check/time-of-use (TOCTOU) race (CWE-367)
remains: an attacker who can write into a directory inside the sandbox root
can, between resolve() and open(), replace a path component with a symlink
pointing outside the root. open() re-traverses the path components at open
time and follows the swapped-in symlink, so the read or write can land outside
the sandbox root even though the pre-checked real path was inside it.

This window is inherited unchanged from the pre-existing file_write tool — it
is not introduced by ADR-2035 — and it is explicitly recorded as a residual
limitation in the ADR-2035 Consequences. Exploitation requires an attacker who
already holds write access to a parent directory within the sandbox and can win
a race against the operation, so the practical bar is high; this issue is
defence-in-depth hardening rather than a reported failure.

Current behavior

SandboxRootPolicy.resolve(path) validates os.path.realpath(path) against
the root, then returns the resolved path string.
FileAccessCore.read_window / FileAccessCore.write open that string with a
plain open(...), which re-resolves symlinks in every path component at open
time.
A symlink introduced into a validated path (final or intermediate component)
after the check but before the open is followed, allowing access
outside the sandbox root. There is no O_NOFOLLOW, openat/dir_fd, or
post-open inode re-verification.

Expected behavior

File opens are confined so that no path component can be a symlink that escapes
the sandbox root at open time, closing the TOCTOU window. Candidate
approaches (to be decided in an ADR if the chosen mechanism is architecturally
significant):

Open relative to a directory file descriptor for the sandbox root using
os.open(..., dir_fd=...) walked component-by-component with O_NOFOLLOW
(and O_DIRECTORY for intermediates), so no symlink is ever followed out of
the root; or
Open first, then re-verify containment on the opened descriptor
(os.fstat device/inode compared against a root the descriptor is confined
to) before any read/write, rejecting on mismatch.

Legitimate in-root reads and writes (including the existing w/a/insert
modes) MUST continue to behave exactly as today.

Acceptance criteria

An open whose final path component is a symlink to a target outside
the sandbox root fails without reading or writing, even when the symlink
is created after SandboxRootPolicy.resolve has validated the path.
An open where an intermediate parent component is swapped to an
out-of-root symlink after validation likewise fails without reading or
writing.
Legitimate in-root reads and writes continue to succeed with unchanged
results, including all file_write modes (w, a, insert).
The hardening is applied in the single shared core, so both the built-in
file_read/file_write tools and the inline read_file/write_file
helpers are protected (no divergence).
No raw file descriptor or new filesystem primitive is exposed to inline
code; the §13.2.1 built-in table remains unchanged.
Coverage remains >= 97% via nox -s coverage_report.

Supporting information

Weakness class: CWE-367 (TOCTOU). Related file-race classes: CWE-59 (link
following).
Introduced/handled in: src/cleveractors/agents/file_access.py
(SandboxRootPolicy.resolve, FileAccessCore.read_window,
FileAccessCore.write), with the tool adapters in
cleveractors.agents.tool (ToolAgent._file_read_tool,
ToolAgent._file_write_tool).
Origin: docs/adr/ADR-2035-inline-code-sandboxed-file-access.md (Consequences
section records this residual TOCTOU window); shipped in PR #103 for issue #93.
If the open strategy changes the normative filesystem-boundary semantics of
§13.3, an ADR + a spec revision (see §21 of docs/index.md) is required
before implementation.

Subtasks

Decide the symlink-safe open strategy (openat/dir_fd + O_NOFOLLOW
walk vs. open-then-fstat re-verification); write an ADR if the mechanism
is architecturally significant.
Implement the symlink-safe open in FileAccessCore for reads and for all
write modes (w/a/insert), keeping a single confinement mechanism
shared by tools and helpers.
Tests (Behave): post-validation symlink swap on final and intermediate
components is refused; legitimate in-root access still succeeds.
Tests (Robot): integration exercising a real symlink-swap attempt is
refused end to end.
Update docs/index.md §13.3 (via ADR + revision bump) if boundary
semantics change.
Update CHANGELOG with a user-facing entry.
Verify coverage >= 97% via nox -s coverage_report.
Run nox (all default sessions), fix any errors.

Definition of Done

This issue is complete when:

All subtasks above are completed and checked off.
A Git commit is created where the first line matches the Commit Message in
Metadata exactly.
The commit is pushed to the branch matching the Branch in Metadata exactly.
The commit is submitted as a PR to master, reviewed, and merged.

## Metadata - **Commit Message:** `feat(agents): symlink-safe sandbox file access closing TOCTOU window` - **Branch:** `feature/m1-symlink-safe-file-access` (provisional — align the milestone number with the milestone assigned at triage) ## Background and context ADR-2035 gave inline code the sandbox-confined `read_file`/`write_file` helpers, backed by the shared cores in `cleveractors.agents.file_access` (`SandboxRootPolicy`, `FileAccessCore`) introduced in PR #103. Containment is enforced by `SandboxRootPolicy.resolve`, which computes the fully resolved real path (`os.path.realpath`, symlinks followed) and checks it is within the sandbox root **at validation time**. The helper/tool then opens that resolved path via `FileAccessCore.read_window` / `FileAccessCore.write` (plain `open(...)`). Because validation and the subsequent `open()` are two separate steps operating on a path string, a **time-of-check/time-of-use (TOCTOU) race** (CWE-367) remains: an attacker who can write into a directory *inside* the sandbox root can, between `resolve()` and `open()`, replace a path component with a symlink pointing outside the root. `open()` re-traverses the path components at open time and follows the swapped-in symlink, so the read or write can land outside the sandbox root even though the pre-checked real path was inside it. This window is inherited unchanged from the pre-existing `file_write` tool — it is not introduced by ADR-2035 — and it is explicitly recorded as a residual limitation in the ADR-2035 Consequences. Exploitation requires an attacker who already holds write access to a parent directory within the sandbox and can win a race against the operation, so the practical bar is high; this issue is defence-in-depth hardening rather than a reported failure. ## Current behavior - `SandboxRootPolicy.resolve(path)` validates `os.path.realpath(path)` against the root, then returns the resolved path string. - `FileAccessCore.read_window` / `FileAccessCore.write` open that string with a plain `open(...)`, which re-resolves symlinks in every path component at open time. - A symlink introduced into a validated path (final or intermediate component) **after** the check but **before** the open is followed, allowing access outside the sandbox root. There is no `O_NOFOLLOW`, `openat`/`dir_fd`, or post-open inode re-verification. ## Expected behavior File opens are confined so that no path component can be a symlink that escapes the sandbox root **at open time**, closing the TOCTOU window. Candidate approaches (to be decided in an ADR if the chosen mechanism is architecturally significant): - Open relative to a directory file descriptor for the sandbox root using `os.open(..., dir_fd=...)` walked component-by-component with `O_NOFOLLOW` (and `O_DIRECTORY` for intermediates), so no symlink is ever followed out of the root; or - Open first, then re-verify containment on the opened descriptor (`os.fstat` device/inode compared against a root the descriptor is confined to) before any read/write, rejecting on mismatch. Legitimate in-root reads and writes (including the existing `w`/`a`/`insert` modes) MUST continue to behave exactly as today. ## Acceptance criteria - [ ] An open whose **final** path component is a symlink to a target outside the sandbox root fails without reading or writing, even when the symlink is created after `SandboxRootPolicy.resolve` has validated the path. - [ ] An open where an **intermediate** parent component is swapped to an out-of-root symlink after validation likewise fails without reading or writing. - [ ] Legitimate in-root reads and writes continue to succeed with unchanged results, including all `file_write` modes (`w`, `a`, `insert`). - [ ] The hardening is applied in the single shared core, so both the built-in `file_read`/`file_write` tools and the inline `read_file`/`write_file` helpers are protected (no divergence). - [ ] No raw file descriptor or new filesystem primitive is exposed to inline code; the §13.2.1 built-in table remains unchanged. - [ ] Coverage remains >= 97% via `nox -s coverage_report`. ## Supporting information - Weakness class: CWE-367 (TOCTOU). Related file-race classes: CWE-59 (link following). - Introduced/handled in: `src/cleveractors/agents/file_access.py` (`SandboxRootPolicy.resolve`, `FileAccessCore.read_window`, `FileAccessCore.write`), with the tool adapters in `cleveractors.agents.tool` (`ToolAgent._file_read_tool`, `ToolAgent._file_write_tool`). - Origin: `docs/adr/ADR-2035-inline-code-sandboxed-file-access.md` (Consequences section records this residual TOCTOU window); shipped in PR #103 for issue #93. - If the open strategy changes the normative filesystem-boundary semantics of §13.3, an ADR + a spec revision (see §21 of `docs/index.md`) is required before implementation. ## Subtasks - [ ] Decide the symlink-safe open strategy (`openat`/`dir_fd` + `O_NOFOLLOW` walk vs. open-then-fstat re-verification); write an ADR if the mechanism is architecturally significant. - [ ] Implement the symlink-safe open in `FileAccessCore` for reads and for all write modes (`w`/`a`/`insert`), keeping a single confinement mechanism shared by tools and helpers. - [ ] Tests (Behave): post-validation symlink swap on final and intermediate components is refused; legitimate in-root access still succeeds. - [ ] Tests (Robot): integration exercising a real symlink-swap attempt is refused end to end. - [ ] Update `docs/index.md` §13.3 (via ADR + revision bump) if boundary semantics change. - [ ] Update CHANGELOG with a user-facing entry. - [ ] Verify coverage >= 97% via `nox -s coverage_report`. - [ ] Run `nox` (all default sessions), fix any errors. ## Definition of Done This issue is complete when: - All subtasks above are completed and checked off. - A Git commit is created where the first line matches the Commit Message in Metadata exactly. - The commit is pushed to the branch matching the Branch in Metadata exactly. - The commit is submitted as a PR to `master`, reviewed, and merged.

CoreRasurae added the

labels

2026-08-04 14:27:09 +00:00

CoreRasurae added a new dependency

2026-08-04 14:28:28 +00:00

#77 Epic: LLM Agent Runtime Stabilization — reliability, resource enforcement & correctness hardening