From 64a601a85ae9a079a99bca805301f687f52d1ca2 Mon Sep 17 00:00:00 2001 From: robert Date: Thu, 28 May 2026 09:59:40 -0300 Subject: [PATCH] feat: initial agent-reviewer meta-agent A2A agent that audits another deployed agent's source pre-deploy. Reads the target Gitea repo via a short-lived read-only token minted through ctx.mint_gitea_token(), runs an inner DeepAgents graph backed by GiteaBackend, and emits a typed ReviewReport. Skill bundles: - security-anti-patterns (credentials, eval, sandbox bypass, egress) - a2apack-best-practices (class shape, decorators, types, timeouts) - grant-scope-evaluation (declared vs actual workspace scope) Tools: - sandbox_a2a_card : round-trips the project through microsandbox - sandbox_ruff_check: objective static checks - submit_review_report: typed final report 7 smoke tests pass; agent card loads cleanly via `a2a card`. --- .gitignore | 6 + Dockerfile | 13 ++ README.md | 48 +++++ a2a.yaml | 6 + agent.py | 171 ++++++++++++++++ agent_reviewer/__init__.py | 6 + agent_reviewer/builder.py | 140 +++++++++++++ agent_reviewer/config.py | 46 +++++ .../skills/a2apack-best-practices/SKILL.md | 75 +++++++ .../skills/grant-scope-evaluation/SKILL.md | 74 +++++++ .../skills/security-anti-patterns/SKILL.md | 58 ++++++ agent_reviewer/tools.py | 190 ++++++++++++++++++ deploy/20-deployment.yaml | 83 ++++++++ pyproject.toml | 3 + requirements.txt | 7 + tests/__init__.py | 0 tests/test_agent.py | 139 +++++++++++++ 17 files changed, 1065 insertions(+) create mode 100644 .gitignore create mode 100644 Dockerfile create mode 100644 README.md create mode 100644 a2a.yaml create mode 100644 agent.py create mode 100644 agent_reviewer/__init__.py create mode 100644 agent_reviewer/builder.py create mode 100644 agent_reviewer/config.py create mode 100644 agent_reviewer/skills/a2apack-best-practices/SKILL.md create mode 100644 agent_reviewer/skills/grant-scope-evaluation/SKILL.md create mode 100644 agent_reviewer/skills/security-anti-patterns/SKILL.md create mode 100644 agent_reviewer/tools.py create mode 100644 deploy/20-deployment.yaml create mode 100644 pyproject.toml create mode 100644 requirements.txt create mode 100644 tests/__init__.py create mode 100644 tests/test_agent.py diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..b6f6151 --- /dev/null +++ b/.gitignore @@ -0,0 +1,6 @@ +__pycache__/ +*.pyc +.venv/ +.pytest_cache/ +node_modules/ +.DS_Store diff --git a/Dockerfile b/Dockerfile new file mode 100644 index 0000000..fbc29e5 --- /dev/null +++ b/Dockerfile @@ -0,0 +1,13 @@ +FROM registry.a2acloud.io/a2a/a2a-pack-base:latest + +WORKDIR /app + +COPY requirements.txt . +RUN pip install --no-cache-dir -r requirements.txt +COPY . . + +ENV A2A_ENTRYPOINT=agent:AgentReviewer +ENV PORT=8000 +EXPOSE 8000 + +CMD a2a run --entrypoint "$A2A_ENTRYPOINT" --host 0.0.0.0 --port 8000 diff --git a/README.md b/README.md new file mode 100644 index 0000000..9c84dd6 --- /dev/null +++ b/README.md @@ -0,0 +1,48 @@ +# agent-reviewer + +Pre-deploy audit agent. Reads another agent's source from Gitea, applies +three skill bundles (security, A2A Pack best practices, grant scope), and +returns a typed `ReviewReport`. + +## Skill: `review` + +``` +review(agent_name: str, ref: str = "main", owner: str | None = None) -> ReviewReport +``` + +Workflow: + +1. Mint a short-lived **read-only** Gitea token via the control plane. +2. Spin up a DeepAgents graph backed by a `GiteaBackend` pointed at + `{owner}/{agent_name}@{ref}`. +3. Read the source, consult the bundled skills, run sandbox checks + (`a2a card`, `ruff`), produce findings. +4. Release the Gitea token in `finally`. + +## Output + +`ReviewReport`: + +```python +class Finding(BaseModel): + severity: "critical" | "warning" | "info" + category: "security" | "correctness" | "ergonomics" | "policy" | "scope" + message: str + file: str | None + line: int | None + suggestion: str | None + +class ReviewReport(BaseModel): + ok: bool # True iff zero critical findings + agent_name: str + ref: str + summary: str + findings: list[Finding] +``` + +## Operational notes + +- Read-only by design. Never writes to the target repo, never deploys. +- Uses the `meta-agent-reader` Gitea service user via the platform's + `POST /v1/platform/gitea-token` endpoint. +- Caller must run through the orchestrator (cp_jwt is forwarded). diff --git a/a2a.yaml b/a2a.yaml new file mode 100644 index 0000000..eefb0e7 --- /dev/null +++ b/a2a.yaml @@ -0,0 +1,6 @@ +name: agent-reviewer +version: 0.1.0 +entrypoint: agent:AgentReviewer +description: Pre-deploy audit of an A2A agent's source — security, scope, ergonomics. +expose: + public: false diff --git a/agent.py b/agent.py new file mode 100644 index 0000000..0c7ae71 --- /dev/null +++ b/agent.py @@ -0,0 +1,171 @@ +"""agent-reviewer — pre-deploy audit of an A2A agent's source.""" +from __future__ import annotations + +import asyncio +import re +from contextlib import suppress +from typing import Any + +from pydantic import BaseModel + +from a2a_pack import ( + A2AAgent, LLMProvisioning, NoAuth, Pricing, RunContext, skill, +) + +from agent_reviewer import ReviewerContext, build_reviewer_graph +from agent_reviewer.tools import ReviewReport + + +class ReviewerConfig(BaseModel): + pass + + +DEEPAGENTS_RECURSION_LIMIT = 500 + + +class AgentReviewer(A2AAgent[ReviewerConfig, NoAuth]): + name = "agent-reviewer" + description = ( + "Audit an A2A agent's source before deploy. Reads the target " + "repo from Gitea, applies security / best-practice / grant-scope " + "skill bundles, and returns a structured ReviewReport." + ) + version = "0.1.0" + + config_model = ReviewerConfig + auth_model = NoAuth + tools_used = ("deepagents", "a2a-pack", "litellm", "microsandbox", "gitea") + llm_provisioning = LLMProvisioning.PLATFORM_OR_CALLER_PROVIDED + wants_cp_jwt = True + pricing = Pricing( + price_per_call_usd=0.05, + caller_pays_llm=False, + notes=( + "Reads target source via a short-lived, repo-scoped Gitea " + "token minted on the caller's behalf. Never deploys or writes." + ), + ) + + @skill( + description=( + "Audit an agent's source. Pass the target ``agent_name`` " + "(kebab-case) and optional ``ref`` (defaults to ``main``). " + "Returns a ReviewReport with severity-categorized findings." + ), + tags=["reviewer", "audit", "meta"], + stream=True, + timeout_seconds=900, + cost_class="medium", + ) + async def review( + self, + ctx: RunContext[NoAuth], + agent_name: str, + ref: str = "main", + owner: str | None = None, + ) -> dict[str, Any]: + if not _is_valid_slug(agent_name): + return {"error": f"invalid agent_name {agent_name!r} — use kebab-case"} + if not ctx.cp_jwt or not ctx.cp_url: + return {"error": "no cp_jwt; this agent must run through the orchestrator"} + + await ctx.emit_progress(f"requesting read-only Gitea token for {agent_name}@{ref}") + try: + mint = await ctx.mint_gitea_token( + agent_name, + scope="read", + owner=owner, + ttl_seconds=900, + purpose=f"reviewer:{agent_name}@{ref}", + ) + except RuntimeError as exc: + return {"error": f"gitea token mint failed: {exc}"} + + token_name = mint["token_name"] + gitea_token = mint["token"] + gitea_owner = mint["owner"] + await ctx.emit_progress( + f"minted token {token_name} (read on {gitea_owner}/{agent_name})" + ) + + creds = ctx.llm + await ctx.emit_progress(f"llm: {creds.model} via {creds.source}") + + completion_box: dict[str, ReviewReport] = {} + graph = build_reviewer_graph( + ReviewerContext( + agent_name=agent_name, + ref=ref, + gitea_token=gitea_token, + gitea_owner=gitea_owner, + llm_base_url=creds.base_url, + llm_api_key=creds.api_key, + llm_model=creds.model, + llm_temperature_mode=creds.temperature_mode, + llm_temperature=creds.temperature, + llm_extra_body=creds.extra_body, + ), + completion_box=completion_box, + ) + + user_msg = ( + f"Review the agent at ``{gitea_owner}/{agent_name}`` ref ``{ref}``. " + "Read the source, consult the bundled skills, run the sandbox checks, " + "then call ``submit_review_report`` exactly once with the structured " + "report." + ) + + async def _heartbeat() -> None: + while True: + await asyncio.sleep(20) + await ctx.emit_progress(f"reviewer still working on {agent_name!r}") + + heartbeat_task = asyncio.create_task(_heartbeat()) + last_reply = "" + try: + async for event in graph.astream_events( + {"messages": [{"role": "user", "content": user_msg}]}, + version="v2", + config={"recursion_limit": DEEPAGENTS_RECURSION_LIMIT}, + ): + kind = event.get("event") + tname = event.get("name") or "" + data = event.get("data") or {} + if kind == "on_tool_start": + await ctx.emit_progress(f"→ {tname}") + elif kind == "on_tool_end": + await ctx.emit_progress(f" {tname} ✓") + elif kind == "on_chain_end" and not event.get("parent_ids"): + out = data.get("output") + if isinstance(out, dict): + for msg in out.get("messages") or []: + content = ( + msg.get("content") + if isinstance(msg, dict) + else getattr(msg, "content", None) + ) + if isinstance(content, str) and content.strip(): + last_reply = content + except Exception as exc: # noqa: BLE001 + return {"error": f"reviewer graph failed: {type(exc).__name__}: {exc}"} + finally: + heartbeat_task.cancel() + with suppress(asyncio.CancelledError): + await heartbeat_task + with suppress(Exception): + await ctx.release_gitea_token(token_name) + + report = completion_box.get("report") + if report is None: + return { + "ok": False, + "agent_name": agent_name, + "ref": ref, + "warning": "reviewer finished without submitting a structured report", + "reply": last_reply[:2000], + } + return report.model_dump() + + +def _is_valid_slug(name: str) -> bool: + return bool(re.match(r"^[a-z][a-z0-9-]{1,62}$", name)) diff --git a/agent_reviewer/__init__.py b/agent_reviewer/__init__.py new file mode 100644 index 0000000..53a2593 --- /dev/null +++ b/agent_reviewer/__init__.py @@ -0,0 +1,6 @@ +"""agent-reviewer — pre-deploy audit of A2A agent source.""" +from __future__ import annotations + +from .builder import ReviewerContext, build_reviewer_graph + +__all__ = ["ReviewerContext", "build_reviewer_graph"] diff --git a/agent_reviewer/builder.py b/agent_reviewer/builder.py new file mode 100644 index 0000000..7149701 --- /dev/null +++ b/agent_reviewer/builder.py @@ -0,0 +1,140 @@ +"""Inner deepagents graph for the reviewer. + +Reads the target agent's source from Gitea (via :class:`GiteaBackend`), +consults the three skill bundles bundled with this agent, and emits a +typed :class:`ReviewReport` through the ``submit_review_report`` tool. +""" +from __future__ import annotations + +from dataclasses import dataclass +from typing import Any + +from a2a_pack import GiteaBackend +from deepagents import create_deep_agent +from langchain_openai import ChatOpenAI + +from .config import Settings, load_settings +from .tools import ReviewReport, ToolContext, build_tools + + +REVIEWER_SKILL_SOURCE = "/.agent-reviewer/skills/" + + +@dataclass(frozen=True) +class ReviewerContext: + agent_name: str + ref: str + gitea_token: str + gitea_owner: str + settings: Settings | None = None + llm_base_url: str | None = None + llm_api_key: str | None = None + llm_model: str | None = None + llm_temperature_mode: str | None = None + llm_temperature: float | None = None + llm_extra_body: dict[str, Any] | None = None + + +SYSTEM_PROMPT = """\ +You are an A2A agent reviewer. You audit deployed agents before they ship. + +Your job: read the source of one agent project in a Gitea repository, +identify security, correctness, ergonomics, and scope issues, then emit a +single structured review via ``submit_review_report``. + +You have packaged DeepAgents skills loaded from ``/.agent-reviewer/skills/``: + + - ``security-anti-patterns`` — credential leakage, unsafe execution, + sandbox bypass, egress concerns. Consult first. + - ``a2apack-best-practices`` — class structure, @skill decorator + contract, type hints, timeouts, error handling. + - ``grant-scope-evaluation`` — declared vs actual workspace scope, + write-prefix coverage, scope expansion correctness. + +Workflow: + +1. List files at the repo root with ``list_files`` (or use ``glob`` for + patterns). Read ``a2a.yaml``, ``agent.py``, and ``requirements.txt`` + first. Then any imported modules under the package directory. +2. Apply each skill bundle to the source. Tabulate findings with + ``severity`` (critical|warning|info) and ``category`` (security| + correctness|ergonomics|policy|scope). Be strict about CRITICAL, + conservative about INFO — false positives erode trust. +3. Call ``sandbox_a2a_card`` to verify the project actually loads. An + exit_code != 0 is itself a CRITICAL correctness finding; cite the + stderr line that broke things. +4. Call ``sandbox_ruff_check`` for objective static issues. Map S-prefix + security rules to security/warning, other rules to ergonomics/info. +5. Once you have your full list, call ``submit_review_report`` exactly + once with the JSON-encoded ReviewReport. ``ok=True`` iff zero CRITICAL. + +Discipline: + + - One review = one ``submit_review_report`` call. Multiple submissions + only on schema rejection; fix and resubmit. + - Every finding must cite ``file`` (and ``line`` when the static check + or AST scan gave you one). + - Don't invent findings to look thorough. An empty findings list with + ``ok=True`` is the right answer for a clean agent. + - Don't deploy, don't push commits, don't modify files. You are + read-only against the target repo. +""" + + +def build_reviewer_graph( + ctx: ReviewerContext, + *, + completion_box: dict[str, ReviewReport], +) -> Any: + settings = ctx.settings or load_settings() + backend = GiteaBackend( + gitea_url=settings.gitea_internal, + owner=ctx.gitea_owner, + repo=ctx.agent_name, + ref=ctx.ref, + token=ctx.gitea_token, + ) + + async def _fetch_source() -> dict[str, bytes]: + tree = await backend._load_tree() # type: ignore[attr-defined] + files: dict[str, bytes] = {} + for entry in tree: + if entry.is_dir: + continue + try: + files[entry.path] = await backend._read_bytes(entry.path) # type: ignore[attr-defined] + except Exception: # noqa: BLE001 + continue + return files + + tools = build_tools( + ToolContext( + settings=settings, + agent_name=ctx.agent_name, + ref=ctx.ref, + fetch_source=_fetch_source, + completion_box=completion_box, + ) + ) + + model_kwargs: dict[str, Any] = { + "model": ctx.llm_model or settings.litellm_model, + "base_url": ctx.llm_base_url or (settings.litellm_url + "/v1"), + "api_key": ctx.llm_api_key or settings.litellm_key, + "stream_usage": True, + } + if ctx.llm_temperature_mode != "omit": + model_kwargs["temperature"] = ( + ctx.llm_temperature if ctx.llm_temperature is not None else 0.0 + ) + if ctx.llm_extra_body: + model_kwargs["extra_body"] = dict(ctx.llm_extra_body) + model = ChatOpenAI(**model_kwargs) + + return create_deep_agent( + model=model, + tools=tools, + system_prompt=SYSTEM_PROMPT, + backend=backend, + skills=[REVIEWER_SKILL_SOURCE], + ) diff --git a/agent_reviewer/config.py b/agent_reviewer/config.py new file mode 100644 index 0000000..3e7ca36 --- /dev/null +++ b/agent_reviewer/config.py @@ -0,0 +1,46 @@ +"""Runtime config — env-driven.""" +from __future__ import annotations + +import os +from dataclasses import dataclass + + +@dataclass(frozen=True) +class Settings: + sandbox_url: str + sandbox_timeout_s: int + sandbox_token: str | None + gitea_internal: str + gitea_public: str + litellm_url: str + litellm_key: str + litellm_model: str + cp_url: str + image: str + + +def load_settings() -> Settings: + return Settings( + sandbox_url=( + os.environ.get("A2A_SANDBOX_URL") + or os.environ.get("SANDBOX_URL", "http://sandbox.sandbox.svc.cluster.local:8000") + ), + sandbox_timeout_s=int( + os.environ.get("A2A_SANDBOX_TIMEOUT_S") + or os.environ.get("SANDBOX_TIMEOUT_S", "600") + ), + sandbox_token=os.environ.get("A2A_SANDBOX_TOKEN") or os.environ.get("SANDBOX_TOKEN"), + gitea_internal=os.environ.get( + "A2A_GITEA_INTERNAL", "http://gitea-http.gitea.svc.cluster.local:3000" + ), + gitea_public=os.environ.get("A2A_GITEA_PUBLIC", "http://gitea.a2acloud.io"), + litellm_url=os.environ.get( + "A2A_LITELLM_URL", "http://litellm.llm.svc.cluster.local:4000" + ), + litellm_key=os.environ.get("A2A_LITELLM_KEY", ""), + litellm_model=os.environ.get("A2A_LITELLM_MODEL", "gpt-5.5"), + cp_url=os.environ.get( + "A2A_CP_URL", "http://control-plane.control-plane.svc.cluster.local" + ), + image=os.environ.get("AGENT_REVIEWER_SANDBOX_IMAGE", "python:3.11-slim"), + ) diff --git a/agent_reviewer/skills/a2apack-best-practices/SKILL.md b/agent_reviewer/skills/a2apack-best-practices/SKILL.md new file mode 100644 index 0000000..3c2188d --- /dev/null +++ b/agent_reviewer/skills/a2apack-best-practices/SKILL.md @@ -0,0 +1,75 @@ +--- +name: a2apack-best-practices +description: Shape of a well-formed A2A Pack agent. Use when reviewing agent.py for class structure, @skill decorators, type hints, error handling, idempotency, and timeouts. +--- +# A2A Pack Best Practices + +Use this skill when reviewing the *shape* of an agent — not its security +posture (that's `security-anti-patterns`) and not its grants (that's +`grant-scope-evaluation`). + +## Class contract + +* The class must inherit from `A2AAgent[ConfigT, AuthT]` with concrete + generic parameters. Missing parameters → `warning`. +* Class variables `name`, `description`, `version` must be set. Missing → + `warning`. `description` shorter than ~30 chars → `info`. +* `config_model` and `auth_model` must reference declared Pydantic + models (or `NoAuth`). A mismatched generic and class var → `warning`. + +## @skill decorators + +* Every public skill should declare `description`. Missing → `warning`. +* `timeout_seconds` should be set explicitly. Default is short; long-running + skills that omit it will time out in production. Missing on a skill that + does file work, LLM calls, or sandbox exec → `warning`. +* `stream=True` is required if the skill calls `ctx.emit_progress`, + `ctx.ask`, `ctx.collect`, or `ctx.request_scope`. Missing while the body + does emit → `critical`. +* `idempotent=True` matters for skills that can be safely retried. Skills + that write files, mutate state, or charge money → `info` if not set + explicitly (either direction is fine, just be intentional). + +## Type hints + +* Every skill parameter must have a concrete type hint (no `Any`, no missing + annotation). The Card's `input_schema` is built from these. Missing → `warning`. +* Return type should be `dict[str, Any]`, a concrete Pydantic model, or `str`. + Returning untyped values surprises callers → `info`. + +## Error handling + +* Skills should return structured error dicts (e.g. `{"error": "..."}`), not + raise unhandled exceptions for *expected* failure modes (bad input, missing + workspace, timeouts). Bare `raise` for invalid input → `info`. +* `try/except Exception` that swallows the error without logging or returning + → `warning`. + +## ctx.workspace / ctx.sandbox usage + +* If the skill calls `ctx.workspace_backend()`, the class must declare + `workspace_access = WorkspaceAccess.dynamic(...)`. Mismatch → `critical`. +* Subprocesses that produce user-visible files must run through + `ctx.workspace_shell` or `ctx.workspace_python`, not raw + `asyncio.create_subprocess_exec`. Already covered in + `security-anti-patterns` but worth a second look here. + +## Recursion limits + +* If the skill invokes a DeepAgents graph via `ainvoke` or `astream_events`, + it must pass `config={"recursion_limit": 500}` to match agent-builder. + Missing → `warning`. + +## Manifest (a2a.yaml) + +* `name` must match the agent's `name` class var. Mismatch → `critical`. +* `entrypoint` must point at a real `module:Class`. Stub or broken → `critical`. +* `version` should be semantic (major.minor.patch). Bad shape → `info`. + +## Severity rubric (specific to this skill) + +* `critical` — breaks the runtime contract (stream/emit mismatch, manifest + doesn't import, name disagreement). +* `warning` — works today but degrades the developer or caller experience + (no timeout, no type hint, bare raise). +* `info` — style nudges. diff --git a/agent_reviewer/skills/grant-scope-evaluation/SKILL.md b/agent_reviewer/skills/grant-scope-evaluation/SKILL.md new file mode 100644 index 0000000..fb92bb2 --- /dev/null +++ b/agent_reviewer/skills/grant-scope-evaluation/SKILL.md @@ -0,0 +1,74 @@ +--- +name: grant-scope-evaluation +description: Evaluate whether a skill's declared workspace grants match what its code actually does. Use when reviewing @skill grant_* parameters against ctx.workspace, ctx.sandbox, and file-writing patterns in the skill body. +--- +# Grant Scope Evaluation + +A2A grants are the platform's principle-of-least-authority lever. Every +public skill declares the scope it intends to use; the platform mints a +matching grant; the runtime enforces it at the FUSE layer. + +Your job is to flag mismatches between **declared** and **actual** scope. + +## What the decorator declares + +Look at the `@skill(...)` call. The relevant fields: + +* `grant_mode` — `read_only`, `read_write_overlay`, or absent. +* `grant_allow_patterns` — fnmatch globs the skill may *read*. +* `grant_deny_patterns` — fnmatch globs explicitly denied (overrides allow). +* `grant_outputs_prefix` — single prefix the skill writes outputs under. +* `grant_write_prefixes` — multiple write prefixes (more flexible than + `outputs_prefix`). +* `grant_ttl_seconds` — how long the grant lives. +* `allow_scope_expansion` — whether `ctx.request_scope` may be called. + +## What the skill body actually does + +Read the skill body and tabulate: + +1. **Reads** — every `ctx.workspace.read(...)` / `read_bytes`, and every + built-in DeepAgents `read_file` / `grep` / `glob`. Each read implies a + path or glob that must be matched by `grant_allow_patterns`. +2. **Writes** — every `ctx.workspace.write(...)`, `write_artifact`, + `ctx.workspace_shell` / `workspace_python` invocation that produces + files. Each write implies a path that must be under + `grant_outputs_prefix` or one of `grant_write_prefixes`. +3. **Scope expansion** — any call to `ctx.request_scope(...)`. If present + without `allow_scope_expansion=True` → `critical` (will raise at + runtime). + +## Common findings + +* **Over-broad declared scope** (`info`/`warning`): grants the agent + more than it uses, e.g. `grant_allow_patterns=("**",)` when the body + only reads `agents/{name}/**`. Tighten the declaration. +* **Under-declared writes** (`critical`): body writes to a path not under + any declared write prefix → FUSE returns `EACCES` and the skill fails. +* **Under-declared reads** (`warning`): body reads paths not in allow + patterns → reads return "not found" even when the file exists. +* **Missing `stream=True`** combined with `allow_scope_expansion=True` + (`critical`): scope expansion requires the SSE stream. +* **Long TTL on a fast skill** (`info`): `grant_ttl_seconds=3600` on a + 10-second skill is sloppy. Match TTL to expected runtime + buffer. + +## Path template variables + +Decorators support `{name}` and similar template variables that the +runtime substitutes from skill args. When evaluating coverage: + +* Treat `agents/{name}/**` as "any path matching that template after the + caller's `name` arg is substituted". +* Flag templates referencing args that don't exist on the skill (`critical`). + +## Cross-skill consistency + +If the agent has multiple skills writing to overlapping prefixes, they +should declare consistent patterns. Divergence is `warning`. + +## Severity rubric + +* `critical` — would fail at runtime (writes hit `EACCES`, scope expansion + without `stream`, template references missing arg). +* `warning` — works but mismatched (over-broad scope, divergent siblings). +* `info` — tighten-the-declaration suggestions. diff --git a/agent_reviewer/skills/security-anti-patterns/SKILL.md b/agent_reviewer/skills/security-anti-patterns/SKILL.md new file mode 100644 index 0000000..ba0b015 --- /dev/null +++ b/agent_reviewer/skills/security-anti-patterns/SKILL.md @@ -0,0 +1,58 @@ +--- +name: security-anti-patterns +description: Common security mistakes in A2A agent source. Use when reviewing agent.py, a2a.yaml, or any imported module for credential leakage, unsafe code execution, network egress, or sandbox bypass. +--- +# Security Anti-Patterns in A2A Agents + +Review the agent's source for these high-confidence red flags. Each finding +must cite a file path and (when available) a line number. Severity guidance +is below — be strict about `critical`, conservative about `info`. + +## Credential leakage (critical) + +* Hard-coded API keys, tokens, passwords. Even in comments. Patterns to grep: + `sk-`, `gho_`, `AKIA`, `xoxb-`, `Bearer `, `password=`, `api_key="..."`. +* Reading provider keys directly: `os.environ["OPENAI_API_KEY"]`, + `A2A_LITELLM_KEY`, master keys. The agent **must** use `ctx.llm` instead. +* Echoing secrets into logs, `emit_progress`, error messages, or returned dicts. +* Writing secrets to the workspace as files. + +## Unsafe execution (critical) + +* `eval(...)`, `exec(...)`, `compile(...)` on caller-supplied content. +* `subprocess.run`/`Popen` with `shell=True` and a string built from caller input. +* `os.system(...)` with any caller input. +* `pickle.load`/`pickle.loads` from caller-supplied bytes. +* Running caller-supplied code *outside* `ctx.sandbox`. Any subprocess spawned + in the agent container is unsandboxed. + +## Sandbox bypass (critical) + +* Calling `asyncio.create_subprocess_exec` for work that should produce + durable files — those run in the agent container, not the sandbox. Use + `ctx.workspace_shell` or `ctx.workspace_python`. +* Writing files outside the agent's declared `grant_write_prefixes`. The + sandbox enforces this at the FUSE layer, but unscoped writes from the + agent container itself are still a smell. + +## Network egress (warning, critical if combined with credential exfil) + +* `httpx`/`requests`/`urllib` calls to arbitrary caller-supplied URLs without + declaring egress policy. Check `runtime = AgentRuntime(egress=...)`. +* Webhooks or callbacks that send caller data to a third party not declared + in the agent card. + +## Resource shape mismatches (warning) + +* Long-running shell commands without `timeout` set. +* Unbounded loops over caller-supplied collections. +* Reading large files entirely into memory when streaming would do. + +## Severity rubric + +* `critical` — exploitable today, or directly violates the platform contract. +* `warning` — would fail a careful review; ship-blocker depending on context. +* `info` — style or hardening suggestion; does not block deploy. + +When in doubt, prefer `warning` over `critical`. Reviewers that cry wolf get +ignored. Reviewers that miss credential leakage get fired. diff --git a/agent_reviewer/tools.py b/agent_reviewer/tools.py new file mode 100644 index 0000000..8028cdd --- /dev/null +++ b/agent_reviewer/tools.py @@ -0,0 +1,190 @@ +"""Tools exposed to the inner DeepAgents graph. + +The reviewer relies heavily on the LLM reading source through the +GiteaBackend (built-in DeepAgents `read_file`, `grep`, `glob`) plus the +skill bundles for judgment. Tools here cover what the model cannot do +on its own: round-trip the project through a sandbox to verify ``a2a +card`` succeeds, run ``ruff`` for objective static issues, and submit a +typed final report. +""" +from __future__ import annotations + +import base64 +import io +import json +import tarfile +import textwrap +from dataclasses import dataclass +from typing import Any, Literal + +import httpx +from langchain_core.tools import tool +from pydantic import BaseModel, Field + +from .config import Settings + +_FINDING_LITERAL = Literal["critical", "warning", "info"] +_CATEGORY_LITERAL = Literal["security", "correctness", "ergonomics", "policy", "scope"] + + +class Finding(BaseModel): + severity: _FINDING_LITERAL + category: _CATEGORY_LITERAL + message: str + file: str | None = None + line: int | None = None + suggestion: str | None = None + + +class ReviewReport(BaseModel): + ok: bool = Field( + ..., description="True iff there are zero critical findings." + ) + agent_name: str + ref: str + summary: str + findings: list[Finding] = Field(default_factory=list) + + +@dataclass(frozen=True) +class ToolContext: + settings: Settings + agent_name: str + ref: str + fetch_source: Any # async callable: () -> dict[str, bytes] + completion_box: dict[str, ReviewReport] + + +def build_tools(ctx: ToolContext) -> list[Any]: + """Construct LangChain tools bound to ``ctx``.""" + + sandbox_url = ctx.settings.sandbox_url.rstrip("/") + sandbox_timeout = ctx.settings.sandbox_timeout_s + sandbox_headers: dict[str, str] = {} + if ctx.settings.sandbox_token: + sandbox_headers["Authorization"] = f"Bearer {ctx.settings.sandbox_token}" + + async def _tarball() -> bytes: + files: dict[str, bytes] = await ctx.fetch_source() + buf = io.BytesIO() + with tarfile.open(fileobj=buf, mode="w:gz") as tf: + for path, data in files.items(): + info = tarfile.TarInfo(name=path) + info.size = len(data) + tf.addfile(info, io.BytesIO(data)) + return buf.getvalue() + + async def _run_in_sandbox(script: str) -> dict[str, Any]: + tar_b64 = base64.b64encode(await _tarball()).decode() + payload = { + "command": script, + "image": ctx.settings.image, + "stdin_b64": tar_b64, + "timeout_seconds": sandbox_timeout, + } + async with httpx.AsyncClient(timeout=sandbox_timeout + 30) as client: + resp = await client.post( + f"{sandbox_url}/v1/run_shell", + json=payload, + headers=sandbox_headers, + ) + if resp.status_code >= 400: + return {"error": f"sandbox {resp.status_code}: {resp.text[:300]}"} + return resp.json() + + @tool + async def sandbox_a2a_card() -> dict[str, Any]: + """Round-trip the agent project through a sandbox. + + Extracts the Gitea source into ``/workspace``, installs requirements, + and runs ``a2a card`` to verify the manifest + class load cleanly. + + Returns ``{"ok": True, "card": {...}}`` on success, or + ``{"ok": False, "exit_code": int, "stdout": str, "stderr": str}`` on + any failure. Treat exit_code != 0 as a CRITICAL finding. + """ + script = textwrap.dedent( + """ + set -e + mkdir -p /workspace/project + cd /workspace/project + tar -xzf - 2>/dev/null + pip install --quiet -r requirements.txt 2>&1 | tail -5 + a2a card + """ + ).strip() + out = await _run_in_sandbox(script) + if out.get("exit_code", 0) != 0: + return { + "ok": False, + "exit_code": out.get("exit_code"), + "stdout": (out.get("stdout") or "")[-2000:], + "stderr": (out.get("stderr") or "")[-2000:], + } + try: + card = json.loads(out.get("stdout") or "{}") + except json.JSONDecodeError: + return { + "ok": False, + "exit_code": 0, + "error": "a2a card stdout was not valid JSON", + "stdout": (out.get("stdout") or "")[-2000:], + } + return {"ok": True, "card": card} + + @tool + async def sandbox_ruff_check() -> dict[str, Any]: + """Run ``ruff check`` on the agent source in a sandbox. + + Returns ``{"exit_code": int, "findings": [...]}`` where each entry + carries ``code``, ``message``, ``file``, ``line``. Treat any finding + as at least an INFO; security-related codes (S-prefix) should be + WARNING or CRITICAL depending on the rule. + """ + script = textwrap.dedent( + """ + set -e + mkdir -p /workspace/project + cd /workspace/project + tar -xzf - 2>/dev/null + pip install --quiet ruff 2>&1 | tail -3 + ruff check --output-format=json . || true + """ + ).strip() + out = await _run_in_sandbox(script) + stdout = out.get("stdout") or "" + try: + findings = json.loads(stdout) if stdout.strip() else [] + except json.JSONDecodeError: + findings = [] + return { + "exit_code": out.get("exit_code", 0), + "stderr": (out.get("stderr") or "")[-1000:], + "findings": findings[:200], + } + + @tool + async def submit_review_report(report_json: str) -> dict[str, Any]: + """Submit the final structured review report. Call exactly once. + + ``report_json`` must parse to ``{ok, agent_name, ref, summary, + findings: [{severity, category, message, file?, line?, suggestion?}]}``. + Use lowercase severities (critical|warning|info) and categories + (security|correctness|ergonomics|policy|scope). + + Returns ``{"accepted": True}`` on success or ``{"accepted": False, + "error": "..."}`` on schema failure — fix the JSON and call again. + """ + try: + raw = json.loads(report_json) + report = ReviewReport.model_validate(raw) + except Exception as exc: # noqa: BLE001 + return {"accepted": False, "error": f"{type(exc).__name__}: {exc}"} + report = report.model_copy(update={ + "agent_name": ctx.agent_name, + "ref": ctx.ref, + }) + ctx.completion_box["report"] = report + return {"accepted": True} + + return [sandbox_a2a_card, sandbox_ruff_check, submit_review_report] diff --git a/deploy/20-deployment.yaml b/deploy/20-deployment.yaml new file mode 100644 index 0000000..89b44d6 --- /dev/null +++ b/deploy/20-deployment.yaml @@ -0,0 +1,83 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: agent-reviewer + namespace: agents + labels: + app: agent-reviewer + a2a/managed-by: control-plane +spec: + replicas: 1 + revisionHistoryLimit: 1 + selector: + matchLabels: + app: agent-reviewer + strategy: + type: RollingUpdate + rollingUpdate: + maxSurge: 1 + maxUnavailable: 0 + progressDeadlineSeconds: 900 + template: + metadata: + labels: + app: agent-reviewer + spec: + enableServiceLinks: false + terminationGracePeriodSeconds: 30 + containers: + - name: agent + image: registry.a2acloud.io/agents/agent-reviewer:latest + imagePullPolicy: Always + ports: + - containerPort: 8000 + name: http1 + protocol: TCP + env: + - name: SANDBOX_URL + value: http://sandbox.sandbox.svc.cluster.local:8000 + - name: A2A_SANDBOX_URL + value: http://sandbox.sandbox.svc.cluster.local:8000 + - name: SANDBOX_TIMEOUT_S + value: "600" + - name: A2A_GRANT_VERIFYING_KEY + valueFrom: + secretKeyRef: {name: platform-secrets, key: grant_verifying_key} + - name: A2A_GITEA_INTERNAL + value: http://gitea-http.gitea.svc.cluster.local:3000 + - name: A2A_GITEA_PUBLIC + value: http://gitea.a2acloud.io + - name: A2A_CP_URL + value: http://control-plane.control-plane.svc.cluster.local + readinessProbe: + httpGet: {path: /healthz, port: 8000} + initialDelaySeconds: 8 + periodSeconds: 5 + failureThreshold: 3 + timeoutSeconds: 1 + livenessProbe: + httpGet: {path: /healthz, port: 8000} + initialDelaySeconds: 25 + periodSeconds: 15 + resources: + requests: {cpu: 200m, memory: 384Mi} + limits: {cpu: "1", memory: 768Mi} +--- +apiVersion: v1 +kind: Service +metadata: + name: agent-reviewer + namespace: agents + annotations: + argocd.argoproj.io/sync-options: ServerSideApply=false,Replace=true + labels: + app: agent-reviewer + a2a/managed-by: control-plane +spec: + selector: + app: agent-reviewer + ports: + - name: http + port: 80 + targetPort: 8000 + protocol: TCP diff --git a/pyproject.toml b/pyproject.toml new file mode 100644 index 0000000..68f242a --- /dev/null +++ b/pyproject.toml @@ -0,0 +1,3 @@ +[tool.pytest.ini_options] +asyncio_mode = "auto" +testpaths = ["tests"] diff --git a/requirements.txt b/requirements.txt new file mode 100644 index 0000000..a42aefd --- /dev/null +++ b/requirements.txt @@ -0,0 +1,7 @@ +a2a-pack>=0.1.34 +httpx>=0.27 +deepagents>=0.5.0 +langchain>=0.3 +langchain-openai>=0.2 +langgraph>=0.6 +pydantic>=2.6 diff --git a/tests/__init__.py b/tests/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/test_agent.py b/tests/test_agent.py new file mode 100644 index 0000000..454bcb5 --- /dev/null +++ b/tests/test_agent.py @@ -0,0 +1,139 @@ +"""Smoke tests for agent-reviewer: card, schema, validation behaviors.""" +from __future__ import annotations + +import json +import sys +from pathlib import Path + +import pytest + +ROOT = Path(__file__).resolve().parent.parent +if str(ROOT) not in sys.path: + sys.path.insert(0, str(ROOT)) + +from agent import AgentReviewer # noqa: E402 +from agent_reviewer.tools import Finding, ReviewReport, ToolContext, build_tools # noqa: E402 +from agent_reviewer.config import load_settings # noqa: E402 + + +def test_class_metadata() -> None: + assert AgentReviewer.name == "agent-reviewer" + assert AgentReviewer.version + assert AgentReviewer.wants_cp_jwt is True + + +def test_review_skill_registered() -> None: + spec = AgentReviewer._skills["review"] # type: ignore[attr-defined] + assert spec.stream is True + assert spec.policy.timeout_seconds == 900 + # Reviewer is read-only — no write grants declared. + assert not spec.policy.grant_write_prefixes + assert spec.policy.grant_outputs_prefix is None + + +def test_review_report_round_trips() -> None: + report = ReviewReport( + ok=False, + agent_name="hello", + ref="main", + summary="one critical finding", + findings=[ + Finding( + severity="critical", + category="security", + message="hardcoded API key", + file="agent.py", + line=42, + suggestion="read ctx.llm.api_key instead", + ) + ], + ) + dumped = report.model_dump() + assert dumped["ok"] is False + again = ReviewReport.model_validate(dumped) + assert again.findings[0].severity == "critical" + + +def test_invalid_severity_rejected() -> None: + with pytest.raises(Exception): + Finding( + severity="catastrophic", # type: ignore[arg-type] + category="security", + message="x", + ) + + +@pytest.mark.asyncio +async def test_submit_review_report_writes_completion_box() -> None: + box: dict = {} + tools = build_tools( + ToolContext( + settings=load_settings(), + agent_name="hello", + ref="main", + fetch_source=lambda: {}, # unused for submit + completion_box=box, + ) + ) + submit = next(t for t in tools if t.name == "submit_review_report") + result = await submit.ainvoke( + { + "report_json": json.dumps( + { + "ok": True, + "agent_name": "ignored", + "ref": "ignored", + "summary": "clean", + "findings": [], + } + ) + } + ) + assert result == {"accepted": True} + assert "report" in box + # Reviewer overrides agent_name / ref from ctx, not from the LLM. + assert box["report"].agent_name == "hello" + assert box["report"].ref == "main" + + +@pytest.mark.asyncio +async def test_submit_review_report_rejects_bad_json() -> None: + box: dict = {} + tools = build_tools( + ToolContext( + settings=load_settings(), + agent_name="hello", + ref="main", + fetch_source=lambda: {}, + completion_box=box, + ) + ) + submit = next(t for t in tools if t.name == "submit_review_report") + result = await submit.ainvoke({"report_json": "{not json"}) + assert result["accepted"] is False + assert "error" in result + assert "report" not in box + + +@pytest.mark.asyncio +async def test_submit_review_report_rejects_bad_schema() -> None: + box: dict = {} + tools = build_tools( + ToolContext( + settings=load_settings(), + agent_name="hello", + ref="main", + fetch_source=lambda: {}, + completion_box=box, + ) + ) + submit = next(t for t in tools if t.name == "submit_review_report") + result = await submit.ainvoke( + { + "report_json": json.dumps( + {"ok": True, "summary": "missing agent_name"} + ) + } + ) + assert result["accepted"] is False + assert "report" not in box