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`.
This commit is contained in:
75
agent_reviewer/skills/a2apack-best-practices/SKILL.md
Normal file
75
agent_reviewer/skills/a2apack-best-practices/SKILL.md
Normal file
@@ -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.
|
||||
74
agent_reviewer/skills/grant-scope-evaluation/SKILL.md
Normal file
74
agent_reviewer/skills/grant-scope-evaluation/SKILL.md
Normal file
@@ -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.
|
||||
58
agent_reviewer/skills/security-anti-patterns/SKILL.md
Normal file
58
agent_reviewer/skills/security-anti-patterns/SKILL.md
Normal file
@@ -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.
|
||||
Reference in New Issue
Block a user