security agent cleanup
This commit is contained in:
@@ -6,7 +6,7 @@ Invoked by the host as:
|
||||
|
||||
The tool arguments are read from stdin as a JSON object; the result is
|
||||
printed to stdout. Only the *action* tools (the ones that touch the
|
||||
filesystem, shell, or network) are dispatched here — the in-memory
|
||||
filesystem, shell, or network) are dispatched here: the in-memory
|
||||
planning tools stay on the host.
|
||||
"""
|
||||
|
||||
|
||||
@@ -5,13 +5,13 @@ ISO timestamp and session id. The log is flushed after every record so
|
||||
that a crash still leaves a complete trail. The file alone is enough
|
||||
to reconstruct what the agent did, in order.
|
||||
|
||||
§2.4: tool results are truncated to ``MAX_RESULT_BYTES`` in the log to
|
||||
tool results are truncated to ``MAX_RESULT_BYTES`` in the log to
|
||||
prevent unbounded growth; a SHA-256 and full length are stored alongside
|
||||
so the truncated entry is self-describing and tamper-evident.
|
||||
|
||||
§6.1: dedicated events for every decision step —
|
||||
dedicated events for every decision step,
|
||||
``log_permission_decision``, ``log_llm_request``, ``log_llm_response``,
|
||||
``log_session_abort`` — give full forensic replay without guessing.
|
||||
``log_session_abort``, give full forensic replay without guessing.
|
||||
"""
|
||||
|
||||
import hashlib
|
||||
@@ -66,7 +66,8 @@ class AuditLog:
|
||||
"event": event,
|
||||
}
|
||||
record.update(fields)
|
||||
self._fh.write(json.dumps(record, ensure_ascii=False, default=str) + "\n")
|
||||
self._fh.write(json.dumps(
|
||||
record, ensure_ascii=False, default=str) + "\n")
|
||||
self._fh.flush()
|
||||
|
||||
# -- convenience wrappers ------------------------------------------
|
||||
@@ -86,7 +87,8 @@ class AuditLog:
|
||||
args = json.loads(tc.function.arguments)
|
||||
except Exception:
|
||||
args = tc.function.arguments
|
||||
calls.append({"id": tc.id, "name": tc.function.name, "args": args})
|
||||
calls.append(
|
||||
{"id": tc.id, "name": tc.function.name, "args": args})
|
||||
self.log("assistant_message", content=content, tool_calls=calls)
|
||||
|
||||
def log_tool_result(
|
||||
@@ -116,8 +118,6 @@ class AuditLog:
|
||||
**_truncate_for_log(result),
|
||||
)
|
||||
|
||||
# -- §6.1 decision-step logging ----------------------------------
|
||||
|
||||
def log_permission_decision(
|
||||
self,
|
||||
tool: str,
|
||||
@@ -126,7 +126,7 @@ class AuditLog:
|
||||
allowed: bool,
|
||||
reason: str | None = None,
|
||||
) -> None:
|
||||
"""Record a standalone permission decision (§6.1).
|
||||
"""Record a standalone permission decision.
|
||||
|
||||
This is emitted *before* the tool runs (or is refused), so the
|
||||
audit trail shows the decision and its reason even if the
|
||||
@@ -148,7 +148,7 @@ class AuditLog:
|
||||
token_estimate: int,
|
||||
has_tools: bool,
|
||||
) -> None:
|
||||
"""Record that an LLM request is about to be sent (§6.1)."""
|
||||
"""Record that an LLM request is about to be sent."""
|
||||
self.log(
|
||||
"llm_request",
|
||||
model=model,
|
||||
@@ -165,7 +165,7 @@ class AuditLog:
|
||||
message_hash: str | None = None,
|
||||
tool_call_count: int = 0,
|
||||
) -> None:
|
||||
"""Record the LLM response metadata (§6.1).
|
||||
"""Record the LLM response metadata.
|
||||
|
||||
``message_hash`` is a SHA-256 of the assistant message content
|
||||
so the full conversation can be verified for forensic replay
|
||||
@@ -181,7 +181,7 @@ class AuditLog:
|
||||
)
|
||||
|
||||
def log_session_abort(self, reason: str) -> None:
|
||||
"""Record that the session was aborted (§6.3)."""
|
||||
"""Record that the session was aborted."""
|
||||
self.log("session_abort", reason=reason)
|
||||
|
||||
def close(self) -> None:
|
||||
|
||||
@@ -4,5 +4,5 @@ def ask_question(question: str) -> str:
|
||||
try:
|
||||
answer = input(" Your answer: ").strip()
|
||||
except EOFError:
|
||||
return "(no answer — EOF)"
|
||||
return "(no answer, EOF)"
|
||||
return answer if answer else "(no answer provided)"
|
||||
|
||||
@@ -265,7 +265,7 @@ def get_tool_schemas():
|
||||
"and cannot reasonably infer it from context. "
|
||||
"Ask one focused question at a time. "
|
||||
"Do not use this for progress updates or to confirm actions you can already "
|
||||
"take — only ask when you are genuinely blocked."
|
||||
"take, only ask when you are genuinely blocked."
|
||||
),
|
||||
"parameters": {
|
||||
"type": "object",
|
||||
|
||||
@@ -11,12 +11,12 @@ read-only to the tool. Network egress can be disabled entirely with
|
||||
The container is started once per session and reused for every action
|
||||
tool call (via ``docker exec``) to avoid per-call startup latency.
|
||||
In-memory planning tools (todo, scratchpad, ask_question) are **not**
|
||||
run in the container — their state would not survive between separate
|
||||
``docker exec`` processes — so they stay in-process on the host.
|
||||
run in the container, their state would not survive between separate
|
||||
``docker exec`` processes, so they stay in-process on the host.
|
||||
|
||||
This module requires a Docker-compatible container CLI on the host:
|
||||
Docker, or Podman (which is auto-detected when ``docker info`` does not
|
||||
work — note that a shell ``alias docker=podman`` is **not** enough,
|
||||
work, note that a shell ``alias docker=podman`` is **not** enough,
|
||||
because the agent invokes the binary directly via ``subprocess`` without
|
||||
a shell). The runtime can also be forced with the ``$AGENT_DOCKER``
|
||||
environment variable. On first use the ``agent-security-runner`` image
|
||||
@@ -132,7 +132,7 @@ class DockerSandbox:
|
||||
self.image = image
|
||||
self.network = network
|
||||
self.exec_timeout = float(exec_timeout)
|
||||
# the harness — not the model — controls the container env.
|
||||
# the harness, not the model, controls the container env.
|
||||
# Only an allowlist of vars is inherited from the host; secret-
|
||||
# looking env vars are stripped before the container ever starts.
|
||||
self.container_env = container_env or {}
|
||||
@@ -221,7 +221,7 @@ class DockerSandbox:
|
||||
def run_tool(self, name: str, args: dict) -> str:
|
||||
"""Execute *name* with *args* inside the container, return its output.
|
||||
|
||||
§4.3: enforces a per-call timeout (``self.exec_timeout``). A
|
||||
enforces a per-call timeout (``self.exec_timeout``). A
|
||||
timeout is reported back as a ``DockerSandboxError`` with a
|
||||
clear message so the LLM knows not to retry blindly.
|
||||
"""
|
||||
@@ -241,7 +241,7 @@ class DockerSandbox:
|
||||
raise DockerSandboxError(
|
||||
f"Tool '{name}' timed out after {self.exec_timeout:.0f}s. "
|
||||
"The command did not finish in the allowed time. Do not "
|
||||
"retry the same call — adjust the approach or ask the user."
|
||||
"retry the same call, adjust the approach or ask the user."
|
||||
)
|
||||
if proc.returncode != 0:
|
||||
err = (proc.stderr or proc.stdout or "").strip()
|
||||
|
||||
@@ -102,7 +102,7 @@ def todo_update(id, content=None, status=None) -> str:
|
||||
if item["status"] == "in_progress" and retries > 0:
|
||||
if retries >= RETRY_LIMIT:
|
||||
return (
|
||||
f"Updated to do item {id} to in_progress — "
|
||||
f"Updated to do item {id} to in_progress, "
|
||||
f"but this is retry {retries} of {
|
||||
RETRY_LIMIT} (retry limit reached). "
|
||||
f"Do not retry again. Escalate to the user instead."
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
"""Lightweight JSON-Schema validator for tool inputs (checklist §3.1, §3.3).
|
||||
"""Lightweight JSON-Schema validator for tool inputs.
|
||||
|
||||
We deliberately avoid a third-party dependency (``jsonschema`` /
|
||||
``pydantic``) so the host-side validation needs no new install and no
|
||||
@@ -12,7 +12,7 @@ JSON-Schema Draft 7 actually used by ``tools/registry.get_tool_schemas``:
|
||||
- ``minimum`` / ``maximum``
|
||||
- ``minLength`` / ``maxLength``
|
||||
|
||||
§3.3 (limit output scope) is enforced by the same schemas: bounds on
|
||||
limit output scope is enforced by the same schemas: bounds on
|
||||
``offset``, ``limit``, ``command`` length, ``content`` length, and a
|
||||
rejection of absolute-path glob patterns are baked into the schemas and
|
||||
therefore checked here.
|
||||
@@ -43,7 +43,7 @@ def _check_type(value: Any, expected: str) -> str | None:
|
||||
if not isinstance(value, str):
|
||||
return f"expected string, got {type(value).__name__}"
|
||||
elif expected == "integer":
|
||||
# bool is a subclass of int — reject it explicitly.
|
||||
# bool is a subclass of int, reject it explicitly.
|
||||
if isinstance(value, bool) or not isinstance(value, int):
|
||||
return f"expected integer, got {type(value).__name__}"
|
||||
elif expected == "boolean":
|
||||
@@ -58,7 +58,7 @@ def validate_args(args: dict[str, Any], schema: dict) -> tuple[bool, list[str]]:
|
||||
"""Validate *args* against a tool's JSON-Schema function spec.
|
||||
|
||||
``schema`` is the inner ``{"type": "object", "properties": ...}``
|
||||
dict — i.e. ``tool["function"]["parameters"]``.
|
||||
dict, i.e. ``tool["function"]["parameters"]``.
|
||||
|
||||
Returns ``(ok, errors)``.
|
||||
"""
|
||||
@@ -89,11 +89,11 @@ def validate_args(args: dict[str, Any], schema: dict) -> tuple[bool, list[str]]:
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Bounded schemas (§3.3 limit output scope)
|
||||
# Bounded schemas
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def bounded_schemas(raw_schemas: list[dict]) -> list[dict]:
|
||||
"""Return a copy of *raw_schemas* with §3.3 bounds injected.
|
||||
"""Return a copy of *raw_schemas* with bounds injected.
|
||||
|
||||
We mutate copies of the per-tool parameter schemas to add:
|
||||
|
||||
@@ -104,7 +104,7 @@ def bounded_schemas(raw_schemas: list[dict]) -> list[dict]:
|
||||
- ``run_bash.command``: minLength 1, maxLength 4096
|
||||
- ``webfetch.url``: maxLength 4096
|
||||
- ``glob_files.pattern``: reject absolute paths (pattern check
|
||||
implemented in ``_validate_value`` via a custom constraint —
|
||||
implemented in ``_validate_value`` via a custom constraint,
|
||||
here we add ``format: relative-path`` which our validator
|
||||
treats specially).
|
||||
|
||||
@@ -142,7 +142,8 @@ def bounded_schemas(raw_schemas: list[dict]) -> list[dict]:
|
||||
# would escape the working-dir scoping.
|
||||
props.setdefault("pattern", {})["format"] = "relative-path"
|
||||
|
||||
out.append({"type": "function", "function": {**fn, "parameters": params}})
|
||||
out.append({"type": "function", "function": {
|
||||
**fn, "parameters": params}})
|
||||
return out
|
||||
|
||||
|
||||
@@ -181,12 +182,15 @@ def _validate_value(value: Any, schema: dict, path: str) -> list[str]:
|
||||
|
||||
if schema.get("type") == "string":
|
||||
if "minLength" in schema and len(value) < schema["minLength"]:
|
||||
errs.append(f"{path}: length {len(value)} < minLength {schema['minLength']}")
|
||||
errs.append(f"{path}: length {len(value)} < minLength {
|
||||
schema['minLength']}")
|
||||
if "maxLength" in schema and len(value) > schema["maxLength"]:
|
||||
errs.append(f"{path}: length {len(value)} > maxLength {schema['maxLength']}")
|
||||
# §3.3 custom format: relative-path (reject absolute glob patterns).
|
||||
errs.append(f"{path}: length {len(value)} > maxLength {
|
||||
schema['maxLength']}")
|
||||
# custom format: relative-path (reject absolute glob patterns).
|
||||
if schema.get("format") == "relative-path" and value.startswith("/"):
|
||||
errs.append(f"{path}: absolute paths are not allowed here (must be relative)")
|
||||
errs.append(
|
||||
f"{path}: absolute paths are not allowed here (must be relative)")
|
||||
|
||||
if schema.get("type") == "integer":
|
||||
if "minimum" in schema and value < schema["minimum"]:
|
||||
|
||||
Reference in New Issue
Block a user