From a3094dea2b3c72f662bd1ffd3655e911ce081e76 Mon Sep 17 00:00:00 2001 From: CleverThis Date: Fri, 10 Apr 2026 18:05:10 +0000 Subject: [PATCH] docs(spec): clarify path containment, datetime, and plugin security contracts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- docs/specification.md | 57 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/docs/specification.md b/docs/specification.md index dbe7bd381..f0803345d 100644 --- a/docs/specification.md +++ b/docs/specification.md @@ -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 = ` 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):** 
-- Plans table: the fundamental unit of orchestration
@@ -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.