cleveragents/cleveractors-core
0
0
Fork 0
Code Issues 20 Pull Requests 6 Actions 5 Packages Projects Releases Wiki Activity

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

New Issue
Open
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
Copy Source

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
State
Unverified
Type
Task
Priority
Medium
 labels 2026-08-04 14:27:10 +00:00
CoreRasurae added
State
Verified
Type
Feature
 and removed
State
Unverified
Type
Task
 labels 2026-08-04 18:29:23 +00:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
auto/blocked-by-deps
PR blocked by an open issue dependency. Operator must close the dep (or remove the dependency link) before the merge driver can act. Auto-cleared by merge_drive when no open deps remain.
 auto/ci-timeout
Most recent merge cycle hit CI timeout. Driver excludes this PR while last merge_cycle row is < 30 min old; label persists thereafter as visible history.
 auto/claimed-implementer
Currently being processed by an implementer worker.
 auto/claimed-merge
Currently being processed by the merge driver.
 auto/claimed-reviewer
Currently being processed by a reviewer worker.
 auto/driver-down
Merge driver heartbeat stale; pipeline halted. Closed automatically on next clean tick.
 auto/invariant-violation
Detected master commit violating the strict merge invariant. Tracked as an issue (not a PR label); kept here for label completeness.
 auto/last-attempt-tier-0
In-cycle escalation: most recent attempt ran at the Tier 0 slot (`tier-0`). Slot's model defined in .opencode/models/tiers.yaml.
 auto/last-attempt-tier-1
In-cycle escalation: most recent attempt ran at the Tier 1 slot (`tier-1`). Slot's model defined in .opencode/models/tiers.yaml.
 auto/last-attempt-tier-2
In-cycle escalation: most recent attempt ran at the Tier 2 slot (`tier-2`). Slot's model defined in .opencode/models/tiers.yaml. Gated behind IMPLEMENTER_ESCALATION_TIER2_ENABLED.
 auto/last-attempt-tier-min
In-cycle escalation: most recent attempt ran at the Tier -1 slot (`tier-min`). Slot's model defined in .opencode/models/tiers.yaml. Suffix is ``-min`` (not ``--1``) so the Forgejo UI reads naturally.
 Automation Tracking
Tracking issues used by the AI Automation system for agents to communicate and report.
 auto/needs-conflict-resolution
Rebase conflict needs LLM conflict-resolver.
 auto/needs-implementer
Failing CI needs implementer attention.
 auto/postmortem
Documenting a driver incident or rollback.
 auto/ready-to-merge
Reviewer has APPROVED this PR and no later REQUEST_CHANGES is outstanding. The merge driver requires this label to even consider a PR for merging. Set by the reviewer worker on APPROVE; cleared on REQUEST_CHANGES.
 auto/restart-throttled
Train repeatedly lost master-tempo races. Driver excludes via merge_cycle until cooldown elapses; label persists as visible history.
 auto/revert
Revert PR backing out an invariant violation. Fast-tracked through the merge driver.
 auto/sentinel
Sentinel PR duplicated from upstream into a personal fork by tools/duplicate_prs_to_fork.py for pipeline testing. Lives only in the fork; the canonical pipeline never sees it.
 auto/stale-inactivity
No implementer activity for N days. Flagged for human review. Auto-cleared on next push to head branch.
 auto/unstable
Repeatedly fails on current master (>= 3 ci-fail-on-rebased-sha releases in 12 h). Excluded from driver until human triage.
 Blocked
A ticket in a blocked state and unable to complete until some other task is completed first.
Bounty
$100
A bounty of $100 for any open-source contributor who provides a MR that solves this issue
Bounty
$1000
A bounty of $1000 for any open-source contributor who provides a MR that solves this issue
Bounty
$10000
A bounty of $10000 for any open-source contributor who provides a MR that solves this issue
Bounty
$20
A bounty of $20 for any open-source contributor who provides a MR that solves this issue
Bounty
$2000
A bounty of $2000 for any open-source contributor who provides a MR that solves this issue
Bounty
$250
A bounty of $250 for any open-source contributor who provides a MR that solves this issue
Bounty
$50
A bounty of $50 for any open-source contributor who provides a MR that solves this issue
Bounty
$500
A bounty of $500 for any open-source contributor who provides a MR that solves this issue
Bounty
$5000
A bounty of $5000 for any open-source contributor who provides a MR that solves this issue
Bounty
$750
A bounty of $750 for any open-source contributor who provides a MR that solves this issue
MoSCoW
Could have
Could have feature in order to satisfy the epic/legendary.
MoSCoW
Must have
Must have feature in order to satisfy the epic/legendary.
MoSCoW
Should have
Should have feature in order to satisfy the epic/legendary.
Needs Feedback
There are questions in the ticket that can not be completed until the project owner provides clarity.
Points
1
1 man-hours worth of work for an expert with no learning curve.
Points
13
13 man-hours worth of work for an expert with no learning curve.
Points
2
2 man-hours worth of work for an expert with no learning curve.
Points
21
21 man-hours worth of work for an expert with no learning curve.
Points
3
3 man-hours worth of work for an expert with no learning curve.
Points
34
34 man-hours worth of work for an expert with no learning curve.
Points
5
5 man-hours worth of work for an expert with no learning curve.
Points
55
55 man-hours worth of work for an expert with no learning curve.
Points
8
8 man-hours worth of work for an expert with no learning curve.
Points
88
88 man-hours worth of work for an expert with no learning curve.
Priority
Backlog
This ticket has backlogged priority and is not to be worked on yet
Priority
CI Blocker
Critical priority issue that blocks CI/CD pipeline and prevents PR merges
Priority
Critical
The priority is critical
Priority
High
The priority is high
Priority
Low
The priority is low
Priority
Medium
The priority is medium
Signed-off: Owner
When an epic or legendary is in review it must be signed off by owner, tech lead, and scrum master before being marked as completed.
 Signed-off: Scrum Master
When an epic or legendary is in review it must be signed off by owner, tech lead, and scrum master before being marked as completed.
 Signed-off: Tech Lead
When an epic or legendary is in review it must be signed off by owner, tech lead, and scrum master before being marked as completed.
 Spike
A ticket for learning a tool or technology that is needed to be able to do future planning and design.
State
Completed
The ticket has been fully implemented, completed, and merged with the source code. This label should only be applied once a ticket is closed.
State
Duplicate
A ticket that represents the same content as an existing ticket.
State
In Progress
A ticket that is actively being developed.
State
In Review
A ticket that has had some code completed to implement but is waiting to pass peer review and is not yet merged in.
State
Paused
This ticket's work started but wasn't finished. It's on hold (likely in a feature branch) and will be resumed later, either due to a blocker or a delay.
State
Unverified
All new tickets start in this state. A developer may set it to show the ticket is unverified. This means we haven't agreed to work on it. It will either move to a verified state or be closed as wontdo.
State
Verified
The issue has been verified by a developer as legitimate. It will be worked on and verified tickets are now considered part of the backlog.
State
Wont Do
This ticket has been decided it wont be done. This may mean the bug has been determined to not be real (cant verify) or the feature is one we have decided we dont want to adopt.
Type
Automation
Any edits or discussion about the AI automated coding system.
Type
Bug
Something that doesnt work as intended.
Type
Discussion
Anytime a ticket represents a discussion about a subject and doesnt fall into one of the other categories.
Type
Documentation
An error or improvement needed in the documentation.
Type
Epic
Any first tier epic. That is, an epic which contains only issues as children and will not have sub-epics.
Type
Feature
Some new functionality not present.
Type
Legendary
A type of Epic which will contain other Epics.
Type
Refactor
A code change that restructures existing code without changing its external behavior.
Type
Support
Someone needs help using the project.
Type
Task
A generic task that doesnt fit into the other type categories.
Type
Testing
Work exclusively focusing on fixing or expanding testing.
No Label
Priority
Medium
State
Verified
Type
Feature
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
aditya (Aditya Chhabra) aleenaumair (Aleena Umair) brent.edwards (Brent Edwards) CoreRasurae (Luis Mendes) drew (Drew Morris) eugen.thaci (Eugen Thaci) freemo (Jeffrey Phillips Freeman) HAL9000 (HAL 9000) HAL9001 (HAL9001) hamza.khyari (Hamza Khyari) hurui200320 (Rui Hu) justin.morris khird (Kyle Hird) org.cleveragents
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Blocks
#77 Epic: LLM Agent Runtime Stabilization — reliability, resource enforcement & correctness hardening
cleveragents/cleveractors-core
Reference: cleveragents/cleveractors-core#105
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?