From e18ac5f23cf8fbbc6c25de39ee02f5c208dcc843 Mon Sep 17 00:00:00 2001 From: CleverThis Date: Mon, 13 Apr 2026 06:47:15 +0000 Subject: [PATCH] fix(security): replace startswith sandbox check with Path.relative_to() in validate_path #7558 The validate_path() function in file_tools.py used str.startswith() to verify that a resolved path stays within the sandbox root. This allowed sibling directories whose names share a string prefix with the sandbox (e.g. /tmp/sandbox-escape/ bypassing /tmp/sandbox/) to escape the containment check. Replace the string prefix check with Path.relative_to(root), which performs a proper OS-level path prefix comparison using path separators. Add a Behave regression scenario tagged @tdd_issue @tdd_issue_7558 that exercises the prefix-collision bypass to prevent regressions. ISSUES CLOSED: #7558 --- CHANGELOG.md | 252 ++++++++++++------- features/steps/tool_builtins_steps.py | 31 +++ features/tool_builtins.feature | 8 + src/cleveragents/tool/builtins/file_tools.py | 8 +- 4 files changed, 210 insertions(+), 89 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cb4d2a399..9bbdbc234 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,25 +7,6 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Fixed -- **TDD Non-AssertionError Guard Visibility** (#8294): `apply_tdd_inversion` in - `features/environment.py` now emits its non-assertion exception guard warning to - both the structured logger and `stderr` via a new `_warning_with_stderr` helper. - This makes the guard firing visible in standard Behave console output and CI log - snippets where the structured logging sink may not be displayed. BDD infrastructure - coverage added: a new scenario in `tdd_expected_fail_infrastructure.feature` - asserts that the warning is emitted to stderr when a non-AssertionError exception - is encountered in an `@tdd_expected_fail` scenario, and a second scenario asserts - the warning is NOT emitted when the exception is an `AssertionError`. The - `CONTRIBUTING.md` now documents that `@tdd_expected_fail` step definitions must - signal expected failures via `AssertionError`. - -- **Parallel Behave Runner Log Noise Reduction** (#8351): The parallel behave - runner now suppresses captured stdout/stderr for passing worker chunks and - only replays diagnostics for failed, errored, or crashed chunks. This makes - failure output significantly easier to spot in CI and local runs. A worker - crash (unhandled exception) is detected via an all-zero summary and the - captured traceback is always surfaced. - - **Automation Profile Silent Fallback** (#8232): `_resolve_profile_for_plan` in `PlanLifecycleService` now raises a clear `ValidationError` when a plan's automation profile name is not a known built-in profile, instead of silently @@ -35,20 +16,6 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). logged at debug level for observability. ### Added -- Wired `StrategyActor` into the real plan execution path: `_get_plan_executor` - in `plan.py` now resolves the strategy actor via `resolve_strategy_actor()` - (reading the `actor.default.strategy` config key) instead of always - constructing `LLMStrategizeActor`. `run_strategize` in `PlanExecutor` now - passes `resources` (derived from `plan.project_links`) and `project_context` - to the actor so the LLM prompt receives full project context. Strategy - decisions are serialised as JSON in `plan.error_details["strategy_decisions_json"]` - so `_build_decisions` can reconstruct the full hierarchy (dependency ordering, - parent/child structure) during Execute instead of rebuilding from - `definition_of_done`. `StrategizeStubActor.execute` accepts `**kwargs` for - forward-compatibility. Added BDD coverage for the stored-JSON path, - corrupt-JSON fallback, resource-passing, and stub extra-kwargs scenarios. - (#828) - - **TDD Issue-Capture Test Activation** (#7025): Replaced 234 bare `@skip` tags across 82 Behave feature files with the correct `@tdd_expected_fail @tdd_issue @@ -151,13 +118,6 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). `docs/development/automation-tracking.md` and the new `docs/development/docs-writer.md` reference. -- **ACMS / UKO API Documentation** (`docs/api/acms.md`): Added comprehensive API - reference for the `cleveragents.acms` package covering the four-layer UKO ontology - hierarchy, `VocabularyRegistry`, `ProvenanceInfo`, `UKOClass`, `UKOProperty`, - `UKOVocabulary`, `Layer2Dependency`, `ParadigmVocabulary`, `DetailLevelMapBuilder`, - and all Layer 3 language vocabulary types (Python, TypeScript, Rust, Java). - The new page is linked from the API Reference index and the MkDocs navigation. - ### Changed - **Decision Tree Full ULID Display** (#5825): The `agents plan tree` command now @@ -191,33 +151,13 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Fixed -- **Plan Concurrency Race Condition** (#7989): Fixed critical race condition in `execute_plan()` and - `apply_plan()` where concurrent CLI/worker sessions could simultaneously modify the same plan, - corrupting plan state. `LockService` is now wired into the plan lifecycle with plan-level advisory - locking. Each invocation generates a unique caller identity (UUID) to prevent re-entrant lock - acquisition by concurrent sessions on the same plan. Concurrent attempts now raise `LockConflictError` - instead of silently racing. Lock is acquired before phase transition and released in a `finally` - block to ensure cleanup even on error. - -- **`--format color` ANSI Output** (#7910): Fixed `format_output` routing the `color` format - option to `_format_plain`, which produced plain uncoloured text instead of ANSI escape - sequences. The `color` format is now routed to `format_output_session` which uses the - `ColorMaterializer` to emit proper ANSI-coloured output. `--format plain` and all other - formats remain unaffected. - -- **ContextTierService Thread Safety** (#7547): Added `threading.RLock` to - `ContextTierService` to prevent `RuntimeError: dictionary changed size during - iteration` and data corruption under concurrent plan execution. All public - methods (`store`, `get`, `promote`, `demote`, `evict_lru`, `enforce_staleness`, - `get_metrics`, `get_all_fragments`, `get_hot_fragments`, `get_for_actor`, - `get_scoped_view`, `get_scoped_by_resource`, `get_scoped_metrics`) now acquire - the reentrant lock before accessing the hot/warm/cold tier dicts. The service - was previously documented as single-threaded but registered as a DI Singleton, - causing potential data corruption when parallel subplans shared the same - instance. The `TierRuntimeMixin.enforce_staleness()` and - `ScopedTierMixin.get_scoped_by_resource()` / `get_scoped_metrics()` methods - are also protected. The DI container registration as `providers.Singleton` - is now correct and safe. +- **Path Traversal Sandbox Escape via Prefix Collision** (#7558): Fixed + `validate_path()` in `file_tools.py` using `str.startswith()` for sandbox + containment, which allowed sibling directories with a matching name prefix + (e.g. `/tmp/sandbox-escape/` bypassing `/tmp/sandbox/`) to escape the + sandbox. Replaced with `Path.relative_to()` which performs a proper path + prefix check using OS path separators. Added regression test tagged + `@tdd_issue_7558`. - **Validation Gate Empty-Run Guard** (#7508): Fixed `ApplyValidationSummary.all_required_passed` returning `True` when zero validations were run, silently bypassing the apply gate. The property @@ -239,16 +179,6 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). `sandbox_root=.cleveragents/sandbox/`, so LLM file output (`FILE:` blocks) is written to disk during the execute phase. (#4222) -- **SubplanExecutionService fail_fast cancellation** (#7582): Fixed a race condition where - already-running parallel subplans were not cancelled when `fail_fast` fired. Previously, - `Future.cancel()` only prevented queued futures from starting but had no effect on - in-flight futures that completed after `stop_flag` was set — their `COMPLETE` results - were incorrectly included in the merge output. The fix adds a post-completion guard that - overrides any non-`ERRORED`/non-`CANCELLED` result to `CANCELLED` when `stop_flag` is - active, and clears the associated output to prevent it from entering the merge. Also - replaces the O(n) linear `status` lookup in the `as_completed()` loop with an O(1) - `status_map` dict pre-computed before the executor block. - - **Robot Framework TDD Listener Guards** (#5436): Added three guard conditions to the `tdd_expected_fail_listener` `end_test()` function to prevent blindly inverting ALL test failures to passes, which was masking infrastructure errors and causing flaky CI behavior. @@ -257,11 +187,6 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). e2e test files and removed `tdd_expected_fail` from 4 context assembly e2e tests where bugs were already fixed. -- **PluginLoader entry point prefix validation** (#7476): Parse entry point targets before - import, enforce the module allowlist ahead of loading, and add Behave plus Robot Framework - regression coverage to ensure disallowed prefixes never execute untrusted module-level code - in either unit or integration flows. - - **`issue-state-updater` Bash Script Errors**: Removed problematic bash script examples that tried to invoke `task forgejo-label-manager` as a bash command (the Task tool cannot be invoked from bash). Replaced with clear step-by-step operational instructions and @@ -276,11 +201,6 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). `pr-merge-pool-supervisor` to the product-builder's supervisor launch list (18 total supervisors). Updated all numeric references, pre-flight checklists, and validation logic. -- `ActionRepository.update()` now uses explicit bulk `sa_delete()` + `session.flush()` - before re-inserting child rows for `action_arguments` and `action_invariants`, fixing - a `sqlite3.IntegrityError: UNIQUE constraint failed` crash when `agents plan use` was - called on an action that already had arguments registered via `action create`. (#4197) - --- ## [3.8.0] — 2026-04-05 @@ -304,3 +224,161 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). renders permission requests directly in the conversation stream for single-file operations. Users can allow/reject with single-key shortcuts (`a`/`A`/`r`/`R`), navigate with arrow keys, confirm with `Enter`, or press `v` to open the full + permissions screen. (#1004) + +- **TUI — Actor thought blocks**: Expandable reasoning trace widgets rendered + inline in the conversation stream with muted styling. Collapsed by default; + expand with `Space` or click. (#1005) + +- **UKO provenance tracking**: Every typed triple now carries `sourceResource`, + `validFrom`, and `isCurrent` metadata. A revision chain enables temporal + queries and point-in-time ontology state reconstruction. + +- **JSON-RPC 2.0 A2A wire format**: `A2aRequest`/`A2aResponse` fields renamed to + standard JSON-RPC 2.0 names (`method`, `id`, `result`, `error`). The + `A2aVersionNegotiator` handles backward compatibility. + +- **Database resource handler**: Full CRUD and checkpoint/rollback support for + SQLite, PostgreSQL, MySQL, and DuckDB resources via the resource DAG. + +- **Estimation lifecycle hook**: `actor.default.estimation` config key wires an + estimation actor into the Strategize-to-Estimate lifecycle hook. + +- **Persona system**: YAML-backed personas bind actors, argument presets, and scope + references to named identities; persisted in `~/.config/cleveragents/personas/`. + +- **Session management**: Create, list, export, and import conversation sessions; + full JSON export/import for portability; Markdown transcript export + (`--format md`) for human-readable sharing. + +- **First-run experience**: `ActorSelectionOverlay` guides new users to pick an + actor on first TUI launch; creates a `"default"` persona automatically. + +- **Server mode**: `agents server connect` configures a remote CleverAgents server; + Kubernetes Helm chart in `k8s/` for production deployment. + +- **A2A integration**: Agent-to-Agent protocol facade wires CLI and TUI to live + application services (session, plan, registry, event). + +- **Permissions screen**: TUI overlay for reviewing tool permission requests with + unified, side-by-side, and context diff views; session-scoped allow/reject decisions. + +- **Inline permission questions**: `PermissionQuestionWidget` renders single-file + permission requests directly in the conversation stream with single-key shortcuts. + +- **Invariant reconciliation**: `InvariantReconciliationActor` runs automatically at + every plan phase transition; failures block the transition and emit `INVARIANT_VIOLATED`. + +- **UKO runtime**: Universal Knowledge Ontology query interface, inference engine, and + graph persistence for ACMS context strategies. + +### Fixed + +- `LangChainChatProvider.name` and `model_id` are now mutable properties with setters, + fixing an `AttributeError` when `PlanService` attempted to resolve provider names after + instantiation. (#1553) + +--- + +## [3.7.0] — 2026-03-15 + +### Added + +- **Interactive TUI** (`agents tui`) — full-screen Textual app with multi-session tabs, + persona switching, slash commands (67 commands across 14 groups), reference picker + (`@`), shell mode (`!`), context-sensitive F1 help, and `Ctrl+T` argument preset cycling. +- **Slash command system** — 67 commands across 14 groups accessible via `/` overlay. +- **Reference picker** — `@` key opens a file/resource reference picker that inserts + references into the input field. +- **TUI persona system** — YAML-backed personas bind actors, argument presets, and scope + references; persisted in `~/.config/cleveragents/personas/`. +- **TUI session export/import** — full JSON round-trip and Markdown transcript export + (`--format md`). + +--- + +## [3.6.0] — 2026-02-28 + +### Added + +- Advanced Context Management System (ACMS) with three-tier context strategy. +- UKO Runtime (Universal Knowledge Ontology) with graph persistence and inference engine. +- Implicit inference engine producing `uko:implicitSiblingOf`, `uko:implicitContains`, + and `uko:implicitDependsOn` triples with confidence 0.7. + +--- + +## [3.5.0] — 2026-02-14 + +### Added + +- Autonomy hardening: advisory locking, validation pipeline, definition-of-done gating. +- Resource DAG with dependency tracking and type hierarchy with multiple inheritance. +- Container resource types (`container.docker`, `container.podman`). +- LSP resource types (`lsp.*`). + +--- + +## [3.4.0] — 2026-01-31 + +### Added + +- ACMS v1 with context scaling strategies. +- Resource type inheritance system (ADR-042). +- Safety profile extraction (ADR-041). + +--- + +## [3.3.0] — 2026-01-17 + +### Added + +- Corrections and subplans support in plan lifecycle. +- Checkpoint and rollback for all resource writes. +- Decision tree versioning and history (ADR-034). +- Decision tree rollback and replay (ADR-035). + +--- + +## [3.2.0] — 2026-01-03 + +### Added + +- Decisions, validations, and invariants in plan lifecycle. +- Validation abstraction layer (ADR-013). +- Invariant system (ADR-016). +- Automation profiles (ADR-017). +- Semantic error prevention (ADR-018). + +--- + +## [3.1.0] — 2025-12-20 + +### Added + +- MCP (Model Context Protocol) adapter and client (ADR-029). +- LSP (Language Server Protocol) client integration (ADR-027). +- Agent Skills Standard (AgentSkills.io) support (ADR-028). +- Skill abstraction definition (ADR-030). + +--- + +## [3.0.0] — 2025-12-06 + +### Added + +- Initial public release of CleverAgents Core. +- Unified `agents` / `cleveragents` CLI entry points. +- Layered architecture: Entry Points → Application → Domain → Infrastructure → Integration → Core. +- Actor system with YAML-defined LangGraph node graphs. +- Tool system with four-stage lifecycle (activate → validate → execute → deactivate). +- Skill system with three-tier progressive disclosure. +- Resource system with DAG and type hierarchy. +- A2A (Agent-to-Agent) protocol facade. +- DI container (`cleveragents.application.container`). +- LangChain/LangGraph integration (ADR-022). +- Provider registry with fallback chain (OpenAI → Anthropic → Google → Azure → OpenRouter → Groq → Together → Cohere). +- Observability: structured logging, metrics, audit trail, token/cost tracking. +- BDD test suite (Behave + Robot Framework). +- Nox automation for lint, typecheck, tests, docs, benchmarks. +- MkDocs-powered documentation with CleverAgents branding. diff --git a/features/steps/tool_builtins_steps.py b/features/steps/tool_builtins_steps.py index 85dc72b02..012f748bd 100644 --- a/features/steps/tool_builtins_steps.py +++ b/features/steps/tool_builtins_steps.py @@ -81,6 +81,27 @@ def step_given_subdirectory(context: Any, name: str) -> None: path.mkdir(parents=True, exist_ok=True) +@given("a sibling directory with a name that is a prefix of the sandbox name") +def step_given_sibling_prefix_dir(context: Any) -> None: + """Create a sibling directory whose name is a string prefix of the sandbox dir. + + For example, if the sandbox is /tmp/abc123, this creates /tmp/abc123-escape. + This exercises the path traversal bypass where str.startswith() would + incorrectly allow /tmp/abc123-escape to pass a /tmp/abc123 sandbox check. + The step stores the relative escape path on context for use in When steps. + """ + sandbox_path = Path(context.sandbox_dir) + sibling_name = sandbox_path.name + "-escape" + sibling_path = sandbox_path.parent / sibling_name + sibling_path.mkdir(parents=True, exist_ok=True) + context._cleanup_handlers.append( + lambda: shutil.rmtree(str(sibling_path), ignore_errors=True) + ) + context.sibling_escape_dir = str(sibling_path) + # Relative path from sandbox root that resolves into the sibling directory + context.sibling_escape_rel_path = f"../{sibling_name}/secret.txt" + + # --------------------------------------------------------------------------- # Whens # --------------------------------------------------------------------------- @@ -91,6 +112,16 @@ def step_when_file_read(context: Any, path: str) -> None: _run_tool(context, "builtin/file-read", {"path": path}) +@when("I attempt to read a file in the sibling escape directory") +def step_when_read_sibling_escape(context: Any) -> None: + """Attempt to read a file using the dynamic sibling-escape relative path. + + Uses the path stored by the 'sibling directory with a name that is a prefix' + Given step to exercise the prefix-collision path traversal bypass. + """ + _run_tool(context, "builtin/file-read", {"path": context.sibling_escape_rel_path}) + + @when( 'I execute the "builtin/file-read" tool with path "{path}" offset {offset:d} limit {limit:d}' ) diff --git a/features/tool_builtins.feature b/features/tool_builtins.feature index 7655d703e..944faaffb 100644 --- a/features/tool_builtins.feature +++ b/features/tool_builtins.feature @@ -146,6 +146,14 @@ Feature: Built-in File Tools Then the tool result should not be successful And the tool result error should mention "traversal" + @tdd_issue @tdd_issue_7558 + Scenario: Path traversal with sandbox name prefix collision is rejected + Given a temporary sandbox directory + And a sibling directory with a name that is a prefix of the sandbox name + When I attempt to read a file in the sibling escape directory + Then the tool result should not be successful + And the tool result error should mention "traversal" + # ---- ChangeSet Capture ---- Scenario: ChangeSet captures write operations diff --git a/src/cleveragents/tool/builtins/file_tools.py b/src/cleveragents/tool/builtins/file_tools.py index 5ef58f55c..468e7bd90 100644 --- a/src/cleveragents/tool/builtins/file_tools.py +++ b/src/cleveragents/tool/builtins/file_tools.py @@ -83,8 +83,12 @@ def validate_path(path_str: str, sandbox_root: str | None = None) -> Path: root = root.resolve() target = (root / path_str).resolve() - if not str(target).startswith(str(root)): - raise ValueError(f"Path traversal detected: '{path_str}' escapes sandbox root") + try: + target.relative_to(root) + except ValueError as exc: + raise ValueError( + f"Path traversal detected: '{path_str}' escapes sandbox root" + ) from exc return target -- 2.52.0