docs(spec): clarify path containment, datetime, and plugin security contracts
CI / benchmark-publish (pull_request) Has been skipped
CI / push-validation (pull_request) Successful in 17s
CI / build (pull_request) Successful in 23s
CI / typecheck (pull_request) Successful in 48s
CI / helm (pull_request) Successful in 53s
CI / lint (pull_request) Successful in 3m21s
CI / quality (pull_request) Successful in 3m42s
CI / integration_tests (pull_request) Successful in 4m3s
CI / e2e_tests (pull_request) Successful in 4m5s
CI / security (pull_request) Successful in 4m6s
CI / unit_tests (pull_request) Successful in 5m41s
CI / docker (pull_request) Successful in 1m33s
CI / coverage (pull_request) Successful in 10m22s
CI / status-check (pull_request) Successful in 1s
CI / benchmark-regression (pull_request) Successful in 57m14s
CI / push-validation (push) Successful in 20s
CI / helm (push) Successful in 21s
CI / quality (push) Successful in 41s
CI / typecheck (push) Successful in 48s
CI / security (push) Successful in 51s
CI / build (push) Successful in 1m0s
CI / lint (push) Successful in 3m18s
CI / benchmark-regression (push) Has been skipped
CI / e2e_tests (push) Successful in 3m40s
CI / unit_tests (push) Successful in 7m26s
CI / docker (push) Successful in 1m37s
CI / integration_tests (push) Successful in 9m33s
CI / coverage (push) Successful in 10m27s
CI / status-check (push) Successful in 2s
CI / benchmark-publish (push) Has been cancelled
CI / benchmark-publish (pull_request) Has been skipped
CI / push-validation (pull_request) Successful in 17s
CI / build (pull_request) Successful in 23s
CI / typecheck (pull_request) Successful in 48s
CI / helm (pull_request) Successful in 53s
CI / lint (pull_request) Successful in 3m21s
CI / quality (pull_request) Successful in 3m42s
CI / integration_tests (pull_request) Successful in 4m3s
CI / e2e_tests (pull_request) Successful in 4m5s
CI / security (pull_request) Successful in 4m6s
CI / unit_tests (pull_request) Successful in 5m41s
CI / docker (pull_request) Successful in 1m33s
CI / coverage (pull_request) Successful in 10m22s
CI / status-check (pull_request) Successful in 1s
CI / benchmark-regression (pull_request) Successful in 57m14s
CI / push-validation (push) Successful in 20s
CI / helm (push) Successful in 21s
CI / quality (push) Successful in 41s
CI / typecheck (push) Successful in 48s
CI / security (push) Successful in 51s
CI / build (push) Successful in 1m0s
CI / lint (push) Successful in 3m18s
CI / benchmark-regression (push) Has been skipped
CI / e2e_tests (push) Successful in 3m40s
CI / unit_tests (push) Successful in 7m26s
CI / docker (push) Successful in 1m37s
CI / integration_tests (push) Successful in 9m33s
CI / coverage (push) Successful in 10m27s
CI / status-check (push) Successful in 2s
CI / benchmark-publish (push) Has been cancelled
Three implementation contracts clarified in response to security/correctness bugs surfaced by the bug hunt pool: 1. Path containment: Sandbox path validation MUST use Path.is_relative_to() not string prefix comparison. String prefix allows /tmp/sandboxmalicious to pass a /tmp/sandbox root check. Canonical implementation provided. 2. Datetime handling: All stored ISO timestamp comparisons MUST parse back to timezone-aware datetime objects before comparing. String comparison of ISO timestamps is incorrect when timezone offsets differ in format. Canonical parse_utc_ts() pattern provided. 3. Plugin protocol validation: Protocol compliance MUST be checked structurally via issubclass() — never by instantiating the plugin class. Instantiation runs __init__ side effects before the plugin is approved. These are minor clarifications (implementation contracts, not architectural changes) added to the existing Security and Extensibility sections. Refs: BUG-HUNT issues #7336 (path traversal), #7341 (datetime comparison), #7331 (plugin instantiation)
This commit was merged in pull request #7362.
This commit is contained in:
@@ -45700,6 +45700,26 @@ The relational database follows a normalized design with foreign key constraints
|
||||
|
||||
4. **Optimistic concurrency control**: The `updated_at` timestamp column on mutable entities serves as a version check. Update operations include `WHERE updated_at = <previous_value>` to detect concurrent modifications. On conflict, the operation fails with an explicit error rather than silently overwriting.
|
||||
|
||||
5. **Datetime handling contract**: All timestamps stored in the database are UTC ISO 8601 strings (format: `YYYY-MM-DDTHH:MM:SS.ffffff`). All Python datetime comparisons involving stored timestamps MUST parse the stored string back to a timezone-aware `datetime` object before comparing — **never** compare ISO timestamp strings lexicographically. String comparison is incorrect because different timezone offset formats (`+00:00` vs `Z` vs `+05:30`) produce strings that are not lexicographically comparable even when representing the same moment. The canonical pattern is:
|
||||
|
||||
```python
|
||||
from datetime import datetime, UTC
|
||||
|
||||
def parse_utc_ts(ts_str: str) -> datetime:
|
||||
"""Parse a stored ISO timestamp string to a UTC-aware datetime."""
|
||||
dt = datetime.fromisoformat(str(ts_str))
|
||||
return dt if dt.tzinfo is not None else dt.replace(tzinfo=UTC)
|
||||
|
||||
# Correct: compare datetime objects
|
||||
if parse_utc_ts(stored_expires_at) >= datetime.now(UTC):
|
||||
...
|
||||
|
||||
# Wrong: string comparison (do not do this)
|
||||
# if stored_expires_at >= datetime.now(UTC).isoformat(): # BUG
|
||||
```
|
||||
|
||||
For database-level comparisons (e.g., `DELETE WHERE expires_at < ?`), pass the ISO string produced by `datetime.now(UTC).isoformat()` only when the database column was also written using the same Python UTC isoformat — ensuring consistent format. Prefer Python-side datetime parsing over relying on lexicographic string ordering.
|
||||
|
||||
**Core tables (SQLite DDL):**
|
||||
|
||||
<div class="highlight"><pre><code><span style="opacity: 0.7;">-- Plans table: the fundamental unit of orchestration</span>
|
||||
@@ -46205,6 +46225,21 @@ The sandbox is the primary safety mechanism preventing untested changes from rea
|
||||
3. **Atomic apply**: The Apply phase is an atomic operation — either all sandbox changes are committed to the real resources, or none are. For git-based resources, this uses git merge. For database resources, this uses transaction commit.
|
||||
4. **Sandbox cleanup**: Sandbox directories are cleaned up according to `sandbox.cleanup` policy. Sensitive data (API responses, LLM outputs) is purged from the sandbox on cleanup.
|
||||
|
||||
**Path containment implementation contract:**
|
||||
|
||||
All path containment checks (verifying that a resolved path is inside a sandbox root) MUST use `Path.is_relative_to(root)` (Python 3.9+) or equivalent path-semantic comparison — **never** string prefix comparison (`str(path).startswith(str(root))`). String prefix comparison is incorrect: a sandbox root of `/tmp/sandbox` would incorrectly pass `/tmp/sandboxmalicious/file.txt` because the string starts with `/tmp/sandbox`. The canonical implementation is:
|
||||
|
||||
```python
|
||||
def validate_path(path_str: str, sandbox_root: str | None = None) -> Path:
|
||||
root = Path(sandbox_root).resolve() if sandbox_root else Path.cwd().resolve()
|
||||
target = (root / path_str).resolve()
|
||||
if not (target == root or target.is_relative_to(root)):
|
||||
raise ValueError(f"Path traversal detected: '{path_str}' escapes sandbox root '{root}'")
|
||||
return target
|
||||
```
|
||||
|
||||
This invariant applies to all built-in tools, MCP tool adapters, and any infrastructure code that validates file paths against sandbox boundaries.
|
||||
|
||||
#### Prompt Injection Mitigation
|
||||
|
||||
In server mode, where user-provided content may flow into LLM prompts, CleverAgents implements several mitigations:
|
||||
@@ -46546,6 +46581,28 @@ The sandbox layer supports custom isolation strategies for specialized resource
|
||||
|
||||
Custom strategies are mapped to resource types via the resource type configuration's `sandbox_strategy` field, or globally via `sandbox.strategy` config.
|
||||
|
||||
#### Plugin Security Contract
|
||||
|
||||
The plugin loading system enforces a security boundary: module imports are restricted to a configurable prefix allowlist to prevent arbitrary code execution from untrusted configuration. However, **protocol validation must never instantiate the plugin class** to perform type checking. Instantiation during validation runs `__init__` side effects (network connections, file I/O, subprocess spawning) before the plugin is approved for use.
|
||||
|
||||
**Protocol compliance MUST be checked structurally** using `issubclass()` for `@runtime_checkable` protocols, or by inspecting declared methods — never by creating a live instance:
|
||||
|
||||
```python
|
||||
# Correct: structural check only — no instantiation
|
||||
@staticmethod
|
||||
def validate_protocol(klass: type[Any], protocol: type[Any]) -> bool:
|
||||
try:
|
||||
return issubclass(klass, protocol)
|
||||
except TypeError:
|
||||
return False
|
||||
|
||||
# Wrong: instantiates the class, running __init__ side effects (do not do this)
|
||||
# instance = klass() # BUG: arbitrary code execution during validation
|
||||
# return isinstance(instance, protocol)
|
||||
```
|
||||
|
||||
This contract applies to all plugin loaders in the Infrastructure layer (provider plugins, custom tool plugins, custom index backend plugins, custom sandbox strategy plugins, and ACMS extension plugins).
|
||||
|
||||
#### ACMS Extensions
|
||||
|
||||
The ACMS provides five categories of extension points: UKO analyzers, index backends, context strategies, UKO vocabularies, and Context Assembly Pipeline components.
|
||||
|
||||
Reference in New Issue
Block a user