---
name: security-review
description: Security review of an open-autonomy agent service — cryptographic key handling, dynamic code execution, ABCI authentication and replay, secret exposure, dependency supply chain, and deployment hardening
argument-hint: "[path/to/skill or packages/ or service/]"
disable-model-invocation: false
---

# Security Review Skill

You are an expert security reviewer for **open-autonomy** agent services. Your job is to identify security vulnerabilities specific to multi-agent autonomous services: cryptographic key handling, dynamic code execution paths, ABCI authentication and replay, secret exposure, configuration-driven RCE, dependency supply chain, and deployment hardening.

Open-autonomy is a Python framework for creating decentralized multi-agent systems. Source and docs: https://github.com/valory-xyz/open-autonomy

This skill is **complementary** to the other audit skills:

- `/audit-fsm` — FSM correctness and safety. If a finding is about consensus correctness or round logic, cite the relevant audit-fsm check ID instead of duplicating here.
- `/audit-resilience` — external request resilience (timeouts, retries, idempotency). If a finding is about HTTP-level robustness, cite that skill.

Security findings that overlap with those skills (e.g. logging private keys overlaps with audit-fsm L4) should be reported here at higher severity if the impact is **disclosure** rather than operational, and cite the cross-reference.

## How to Use Arguments

- If `$ARGUMENTS` is provided, audit only those paths (e.g. `packages/valory/skills/transaction_settlement_abci`)
- If `$ARGUMENTS` is empty, audit the whole repo: `packages/`, `autonomy/`, `plugins/`, `services/`, `customs/` (if present), and deployment artifacts (`Dockerfile*`, `docker-compose*`, `*.yaml` configs)
- Multiple paths can be space-separated

---

## Open-Autonomy Security Architecture Reference

This section encodes the threat model. Use it as ground truth — do not rely on external documentation.

### Cryptographic Key Lifecycle

Open-autonomy agents hold private keys for one or more chains. Default file locations:
- `ethereum_private_key.txt`
- `solana_private_key.txt`
- `cosmos_private_key.txt`

Loaded by `aea` core via `aea.crypto.wallet.Wallet` (and the underlying `CryptoStore`) at startup. Used for:
- Signing ABCI payloads (each agent's `sender` field on consensus payloads)
- Signing on-chain transactions to a Gnosis Safe (multi-sig) or directly to a contract
- Establishing ACN (Agent Communication Network) identity
- Tendermint validator keys (separate, in `priv_validator_key.json`)

**Threat surface:**
- **Build-time leakage**: key file in container image layer, build artifact, or git history → permanent compromise.
- **Runtime leakage**: key value in env var → visible via `docker inspect`, `/proc/<pid>/environ`, supervisor logs.
- **Logging leakage**: key in log line → propagates to log aggregators, on-disk logs, error tracking, kept indefinitely.
- **Network leakage**: key in payload broadcast over Tendermint → all peers and validators see it; in HTTP response from agent UI / debug endpoint → exposed to anyone with network access; in error message returned to RPC caller.
- **Key derivation leakage**: mnemonic / seed / entropy logged or returned.

A leaked agent key generally allows:
- Submitting valid ABCI payloads as that agent
- Co-signing Safe transactions (in a 1-of-N or quorum-met scenario)
- Exhausting the agent's gas budget on chain
- Impersonating the agent in ACN

### ABCI Authentication and Replay Model

ABCI payloads carry `sender: str` (agent address). The framework verifies via Tendermint signature that the payload was signed by the claimed sender. The round then verifies:
- `sender` is in the active participant set
- Payload type matches the round's `payload_class`
- Threshold rules (e.g. `CollectSameUntilThresholdRound` requires N agents to agree)

**What the framework DOES guarantee (do not re-report as findings against standard rounds):**
- **Cross-period replay protection on standard rounds.** `BaseTxPayload.round_count` is monotonic and never resets across periods. The standard collection rounds (`CollectSameUntilThresholdRound`, etc.) verify it in `process_payload()` (`packages/valory/skills/abstract_round_abci/base.py:1417`): `if payload.round_count != self.synchronized_data.round_count: raise ABCIAppInternalError(...)`. A payload from a closed period therefore cannot be re-submitted in a later period as long as the round's `process_payload` invokes the parent's check.

**What is NOT verified by the framework:**
- **Payload field semantics**: an agent reporting "I have 1 ETH" is not cross-checked against actual on-chain balance. Field-level validation is the round/behaviour author's responsibility.
- **Cross-period replay on custom rounds that bypass the parent check.** Any round that overrides `process_payload()` without invoking the standard `round_count` verification re-opens the replay surface. This is the surface H6 audits.
- **Source authenticity beyond AEA identity**: if an agent's key is stolen, the attacker is indistinguishable from the legitimate agent. There is no per-payload TOFU or out-of-band confirmation.
- **Payload class identity stability**: `_MetaPayload` keys payloads by `"{module}.{ClassName}"`. Renaming the payload class or moving its module breaks deserialization of persisted Tendermint state — and creates a window where two distinct payload types share a registry key during rollout. Cross-reference: `audit-fsm` M1 (`Payload Class Mismatch`) and T6 (`_MetaPayload.registry Not Saved/Restored`).

### IPFS-Loaded Strategy Model (`customs/`)

**Scope caveat.** `customs/` and `FILE_HASH_TO_STRATEGIES` are **downstream service patterns** (e.g. `valory-xyz/trader`, `valory-xyz/optimus`) — they do NOT exist in `open-autonomy` itself. Audits scoped to the framework repo will produce vacuous "no findings" output for this section and for C3. Apply this subsection only when auditing a downstream service that adopts the pattern.

Strategies under `customs/` (e.g. `kelly_criterion`, `fixed_bet`) are loaded by IPFS hash at runtime:

1. The hash is configured in `aea-config.yaml` / `service.yaml` — typically as a param like `FILE_HASH_TO_STRATEGIES` mapping IPFS hashes to strategy module names.
2. Code is fetched from IPFS, the content hash is verified against the configured hash, and the module is **executed in the agent process**.

**Trust boundary:** the configured hash is the only thing standing between the agent and arbitrary code execution. The strategy code runs with full agent privileges:
- Access to private keys via the agent's `Wallet` / `CryptoStore` (`aea.crypto.wallet`)
- Access to environment variables
- Network access to all configured endpoints (RPC, exchange APIs, IPFS)
- Ability to read/write `synchronized_data` (consensus-critical state)

**Threat surface:**
- A wrong hash (typo, misconfiguration, malicious config injection) → agent fetches and executes attacker-chosen code.
- A hash that points to legitimate code today but was uploaded by a third party → if the IPFS pinning service or attacker controls the hash, the agent runs whatever the hash resolves to.
- Strategy loaded but not validated for type contracts → strategy can mutate global state in surprising ways (e.g. monkeypatch `Web3.eth.send_raw_transaction`).

### Configuration / Env-Var Resolution

`skill.yaml`, `aea-config.yaml`, `service.yaml` support env-var interpolation:

```yaml
api_url: ${API_URL:str:https://default.example}
api_key: ${API_KEY:str:dummy}
```

Resolution happens at agent startup. Resolved values flow into `Params` / `Model` instances and are read at runtime by behaviours.

**Threat surface:**
- **`dummy` / empty defaults in production**: if the env var isn't set, the agent runs with the placeholder. Anonymous requests to authenticated endpoints, wrong addresses, "test" keys.
- **Env-vars in `docker-compose.yaml` checked into git**: anyone with read access to the repo has the secret.
- **Env-vars baked into container image layers**: `ENV API_KEY=...` in a Dockerfile persists in the image even if overridden at runtime.
- **Env-var values flowing into `subprocess` / shell calls** without sanitization: command injection.
- **Env-var values used as Python module paths** (`importlib.import_module(os.environ['MODULE'])`): RCE.

### Tendermint Network Surface

Each agent ships with a Tendermint sidecar:
- RPC port (default 26657) — used for ABCI queries, block info, broadcast
- P2P port (default 26656) — peer-to-peer consensus
- ABCI app socket (default 26658) — the port the agent's `abci` connection listens on (`packages/valory/connections/abci/connection.py:110` `DEFAULT_ABCI_PORT`); Tendermint dials in to deliver `CheckTx` / `DeliverTx` etc. A 0.0.0.0 bind here lets any peer drive the application directly, bypassing Tendermint
- gRPC port (default 9090, optional)

**Threat surface:**
- RPC exposed on `0.0.0.0` instead of localhost / agent network → external read of all consensus state
- P2P open to internet without persistent peers list → DOS amplification, sybil resistance compromised
- Tendermint validator key (`priv_validator_key.json`) misplaced or world-readable → consensus signature forgery

### Container / Deployment Threat Surface

`autonomy deploy build` produces docker-compose / k8s manifests. Common defaults:
- Tendermint and agent processes run as **root** in container unless overridden
- Keys mounted via volume from host
- Logs written to mounted volume
- Health/debug endpoints potentially exposed (e.g. Flask Tendermint monitor on port 8080)

**Threat surface:**
- Root container = container escape → host compromise has broader blast radius
- World-readable mounted key file
- Health endpoint that returns synced state, validator info, or agent address without auth
- Debug routes that expose internal state in production builds

### Dependency Supply Chain

Open-autonomy uses `poetry` / `pipenv` and `tomte` for tooling. License policy (per `CLAUDE.md`):
- Allowed: MIT, BSD, Apache 2.0
- Prohibited: GPL, LGPL, MPL

Third-party trading libraries (`py_clob_client`, `web3`, `httpx`, `requests`, exchange-specific clients) are full-trust runtime deps with full keychain access.

**Threat surface:**
- Pinned dep with a known but unpatched CVE
- Typosquat (e.g. `python-requests` vs `requests`, `crpytography` vs `cryptography`)
- Transitive dep without pinning → lockfile drift across CI / dev / prod
- Dev-only dep bleeding into prod (`bandit`, `safety` in runtime container)
- Compromised release of an upstream package (rare but high-impact)
- Unsigned source distribution where wheel is unavailable

The repo's `tox -e safety` allowlist already pins known-unfixed CVEs in transitive deps. Audit the allowlist itself.

---

## Audit Checklist

### Critical (C) — Immediate compromise (RCE, key disclosure, full agent takeover)

#### C1: Dynamic Code Execution on Untrusted Input

**What:** `eval`, `exec`, `compile`, or dynamic import where the input is not statically known. Untrusted input includes anything sourced from: HTTP request, ABCI payload, env var, file content, IPFS, external API response.

**Search patterns:**
- `eval(`, `exec(`, `compile(` — any usage in `packages/`, `customs/`, `autonomy/`
- `importlib.import_module(`, `__import__(` with non-literal arguments
- `pickle.load`, `pickle.loads` on data not produced by the same trust boundary (file written by user, network response, IPFS payload)
- `marshal.loads`, `shelve.open` on untrusted data
- `yaml.load(...)` without `Loader=SafeLoader` — pre-5.4 `FullLoader` had **CVE-2020-14343** (arbitrary object construction); PyYAML ≥ 6.0 raises `TypeError` on missing `Loader`. Always pass `Loader=SafeLoader` (or `yaml.safe_load`) explicitly — even on PyYAML 6 — to be robust against future loader-default changes
- `subprocess.*` with `shell=True` AND interpolated arguments (`f"cmd {var}"`, `"cmd " + var`, `cmd % var`)
- `os.system(` with interpolated arguments
- `Template(...).substitute(...)` where the template itself comes from untrusted input

**Bug example:**
```python
# BUG: payload field interpolated into shell
def process_strategy(self, payload):
    strategy = payload.strategy_name
    subprocess.run(f"python -m {strategy}", shell=True)  # RCE
```

**Fix:** never `eval`/`exec`; use `yaml.safe_load`; use `subprocess.run([cmd, arg1, arg2], shell=False)`; for pickle, sign and verify or replace with JSON.

**Severity escalation:** If the input source is an ABCI payload or IPFS content, this is **unconditionally Critical** — a single malicious / compromised peer can pivot to RCE on every agent.

#### C2: Private Key Disclosure

**What:** Any code path that emits, persists, or returns a private key, mnemonic, seed, or signing material outside the AEA crypto/wallet boundary (`aea.crypto.wallet.Wallet`, `Crypto.sign_message`, `CryptoStore`).

**Search patterns:**
- Variable name regex inside `logger.*`, `print(`, `return`, response builders, payload constructors:
  - `private_key`, `priv_key`, `pk`, `sk`, `secret`, `seed`, `mnemonic`, `entropy`, `signing_key`, `wallet.private_key`
- HTTP handlers that read from the agent's `Wallet` / `CryptoStore` and return values in the response body (debug endpoints in particular)
- Behaviours that interpolate signed material into `payload` fields broadcast over Tendermint
- Files written by the agent that contain key material (`open(..., 'w')` with key in body)
- Exception handlers that include the key in the error message (`raise ValueError(f"Bad key: {key}")`)
- `__repr__` / `__str__` of wallet / signer objects that includes the key

**Bug example:**
```python
# BUG: full HTTP response body logging includes Authorization header containing the agent key
self.context.logger.info(f"Sent: {request}, Got: {response.json()}")
# request includes signed message that derives from the private key
```

**Cross-reference:** audit-fsm L4 covers this at the operational level. /security-review escalates to Critical because the impact is permanent disclosure.

**Fix:** never log key material; redact in `__repr__`; constrain key access to a single module that exposes only `sign(...)` not `get_key()`; review every debug endpoint.

#### C3: IPFS Strategy Hash Trust Bypass

**What:** Code paths that load Python modules from IPFS without hash verification, OR that allow the configured hash to be derived at runtime from untrusted input.

**Search patterns:**
- IPFS fetch + `importlib` / `exec` / `compile` of fetched bytes
- `FILE_HASH_TO_STRATEGIES` (or similar param) populated from external source rather than static config
- `customs/` loader code that fetches and evaluates without hash check
- Hash configured via env var that defaults to placeholder (`dummy`, empty) — production agents may run with unverified strategy

**Threat:** the IPFS hash IS the trust boundary. Bypassing it = arbitrary code execution with full agent privileges (keys, network, on-chain authority).

**Fix:** statically configure hashes; verify content hash matches configured hash before executing; reject placeholder defaults in production builds.

#### C4: Unsafe Deserialization on ABCI / Network Boundary

**What:** Connection / handler code that deserializes inbound payloads using formats that allow code execution (`pickle`, `yaml.load`, `marshal`).

**Search patterns:** in `connections/*/connection.py`, `handlers.py`, ABCI message handlers:
- `pickle.loads(message.body)`, `pickle.loads(srr_message.payload)`
- `yaml.load(...)` without `Loader=SafeLoader`
- `marshal.loads(...)`
- Custom deserializers that `eval` field values

**Distinct from `audit-resilience` BP14.** BP14 audits whether the connection's `on_send` / `_route_request` catches `JSONDecodeError` from `json.loads(message.payload)` — i.e. a malformed-input robustness concern. C4 audits whether the deserializer itself is code-executing — i.e. an RCE concern. A connection can simultaneously have a BP14 finding (no exception handling) and a clean C4 (uses `json.loads`), or vice versa (uses `pickle.loads` inside a generous try-except → C4 finding without BP14). Different threat classes; report independently.

**Fix:** use JSON; if a binary format is needed, use protobuf / msgpack with strict schemas.

#### C5: Hardcoded Secrets in Source Tree

**What:** API keys, JWTs, mnemonics, private keys, OAuth tokens, AWS credentials, or webhook URLs with embedded tokens, committed in source.

**Tooling:** `tox -e gitleaks` runs in CI. /security-review re-runs and reviews:
- The current `gitleaks.toml` allowlist for over-broad patterns
- New files added in the audit scope
- Test fixtures that may have been accidentally promoted to runtime code paths

**Search patterns** beyond gitleaks defaults. Gitleaks's built-in rules already cover common SaaS / cloud token shapes (Slack, GitHub, GitLab, AWS, Stripe, JWT, OpenAI, etc.) — do NOT re-list those literals here, listing them would re-trigger gitleaks on this file. Audit-specific additions:
- `0x[a-fA-F0-9]{64}` — 32-byte hex **shape** in non-test files. **High false-positive rate**: the same form matches Ethereum transaction hashes, block hashes, Keccak-256 outputs, storage-slot keys, IPFS CID v1 binary fields, and any `bytes32` literal. Triage before flagging: require proximity to a variable name in `{key, secret, mnemonic, pk, sk, private}`, OR exclude matches adjacent to `{tx_hash, block_hash, ipfs_hash, topic, slot}`. Without this triage, the rule fires on every block-hash literal in tests and tooling AND lets reviewers dismiss real keys as "just a hash"
- `mnemonic = "..."`, mnemonic-shaped 12/15/18/21/24-word strings in source

**Severity:** Critical regardless of intent — once committed, must be assumed compromised even after removal (git history retention).

**Fix:** rotate the disclosed credential; remove from history (`git filter-repo`); replace with env-var or secrets-manager reference; add to `gitleaks.toml` as a tested negative.

#### C6: Command Injection via Shell-Subprocess Interpolation

**What:** `subprocess.*(..., shell=True)` or `os.system(...)` where any argument is composed from external input (env var, payload field, HTTP query param, file content).

**Search patterns:**
- `subprocess.run(`, `subprocess.call(`, `subprocess.Popen(` with `shell=True`
- `os.system(`, `os.popen(`
- `commands.getoutput(` (if Python 2 legacy), `pty.spawn(`

For each, check whether arguments are static literals or interpolated.

**Bug example:**
```python
# BUG: agent_id from HTTP request flows into shell
@app.post("/restart")
def restart(agent_id: str):
    subprocess.run(f"systemctl restart agent-{agent_id}", shell=True)
```

**Fix:** `shell=False` and pass argv as a list; use `shlex.quote` for unavoidable shell paths; validate via allowlist before interpolation.

---

### High (H) — High-impact issues short of immediate RCE

#### H1: Env-Var-Driven Authentication with Insecure Defaults

**What:** `service.yaml` / `aea-config.yaml` / `skill.yaml` with env-var-interpolated auth credentials whose default is `dummy`, empty string, or a placeholder. Production deployments depend on the env var being set; if it isn't, the agent runs unauthenticated or with shared known-bad credentials.

**Search patterns** in YAML configs:
- `${[A-Z_]+:str:dummy}`, `${[A-Z_]+:str:}`, `${[A-Z_]+:str:test}`, `${[A-Z_]+:str:placeholder}`
- Param names matching `*_api_key`, `*_token`, `*_secret`, `*_password`, `*_credential`
- Param names matching `*_url`, `*_endpoint` where placeholder is suspicious (`localhost`, `example.com`)

**Fix:** require env var (no default), or default to a value that **fails closed** (e.g. an unauthenticated client that only allows read-only ops); add a startup check that refuses to run with placeholder values in production mode.

#### H2: Hardcoded Secrets in Deployment Artifacts

**What:** Secrets in `docker-compose.yaml`, `Dockerfile*`, `kubernetes.yaml`, `helm/` charts, GitHub Actions workflows, or `.env*` files committed to the repo.

**Search patterns:**
- `Dockerfile*`: `ENV API_KEY=`, `ARG SECRET=`, `RUN echo "..." > /key`
- `docker-compose.yaml`: literal values in `environment:` blocks (vs `${VAR}` references)
- `.github/workflows/*.yml`: literal tokens (vs `${{ secrets.X }}`)
- `.env`, `.env.local`, `.env.production` committed to git (these should be in `.gitignore`)
- `kubernetes/*.yaml`: `data:` blocks with base64-encoded plaintext (Secret manifests)

**Fix:** use Docker secrets / Kubernetes Secret resources with mounted files; use GitHub Actions encrypted secrets; ensure `.env*` is gitignored; rotate any value found.

#### H3: Container Running as Root

**What:** Dockerfiles without a `USER` directive, or with `USER root`, leave the agent process running as root inside the container.

**Search patterns:** `Dockerfile*`, especially `deployments/Dockerfiles/*/Dockerfile`:
- Absence of `USER` directive (Docker default = root)
- `USER root` explicitly set
- `RUN chown -R root:root /app` patterns

**Why high not critical:** root-in-container isn't immediate RCE on the host, but combined with any container-escape CVE (e.g. cgroup vulnerabilities, kernel bugs), the blast radius is much larger than non-root.

**Fix:** create a non-root user (`adduser --system --no-create-home agent`), `USER agent`, ensure mounted key files are readable by that uid only.

#### H4: Tendermint RPC Exposed Beyond Local Network

**What:** Tendermint RPC port (default 26657) bound to `0.0.0.0` or exposed in `docker-compose.yaml` `ports:` section without auth.

**Search patterns:**
- `docker-compose*.yaml`: `ports:` entries that publish 26657 / 26656 to the host
- Tendermint config (`config.toml`): `laddr = "tcp://0.0.0.0:26657"` instead of `tcp://127.0.0.1:26657`
- Kubernetes Service manifests with `type: LoadBalancer` for Tendermint

**Threat:** Tendermint RPC exposes block data, validator info, mempool state, and (in some configurations) `broadcast_tx_*` endpoints that allow anyone to submit transactions.

**Fix:** bind to localhost or to the agent-internal network; use a network policy / firewall to restrict ingress; require auth proxy for any external RPC access.

#### H5: ABCI Payload Field Validation Gaps

**What:** Payload fields read by `end_block()` or `process_payload()` that are used in security-sensitive contexts (signing material, addresses, amounts, contract addresses, URLs) without explicit validation.

**Search patterns:**
- Round / behaviour code that reads `payload.<field>` and uses it for: `Web3.toChecksumAddress(...)`, `Account.sign_message(...)`, contract write calls, HTTP requests to user-controlled URLs
- Payload classes whose fields are typed as `str` or `Any` (no Pydantic / dataclass-validation hooks)

**Threat scenario:** a compromised agent submits a payload with a malicious URL, address, or amount. If the round accepts the payload without bounds-checking, the consensus value drives downstream actions across all agents.

**Fix:** validate at `check_payload()` (round side) and at payload construction (behaviour side); use strict type hints (`Address`, `URL`, `PositiveInt`); reject out-of-range values.

#### H6: Replay Protection Gaps in Custom Rounds

**What:** Standard collection rounds inherit `process_payload()` from the framework, which verifies `payload.round_count == self.synchronized_data.round_count` (`packages/valory/skills/abstract_round_abci/base.py:1417`). H6 audits the narrower residual surface: **custom rounds that override `process_payload()` without invoking the parent check** (e.g. via `super().process_payload(payload)` or an equivalent `round_count` assertion).

**How to check:**
1. Grep for round subclasses that define their own `process_payload` (`def process_payload(self, payload`).
2. For each, verify the override either:
   - Calls `super().process_payload(payload)`, OR
   - Performs an equivalent `if payload.round_count != self.synchronized_data.round_count: raise ...` check.
3. If neither, this round's payloads from a closed period can be replayed in any later period — flag.
4. For payloads that drive on-chain transactions, also verify the resulting tx includes chain-id, contract nonce, and Safe nonce (these are independent of the ABCI-level check).

**Threat:** an adversary captures a payload signed by a participant in period N and re-submits it in period M to re-trigger a state-changing action. Standard rounds reject this at `process_payload`; overriding rounds may not.

**Distinct from `audit-resilience` BP15.** BP15 protects against the same logical action being re-submitted via FSM retry / connection-layer retry (accidental, internal trigger). H6 protects against an old payload from a closed period being replayed on the consensus layer (adversarial, external trigger). Different attacker model, different mitigation surface — the mitigations partially overlap (idempotency on the action vs nonce/round_count on the payload) but require independent assessment.

**Fix:** in every custom `process_payload` override, either call `super().process_payload(payload)` first, or replicate the `round_count` check explicitly.

#### H7: Dependency CVEs

**What:** Run `tox -e safety` (which uses tomte's allowlist) against the resolved environment. Each unaddressed CVE in a runtime dep is a finding. `pip-audit` is optional second-source coverage but is **NOT pre-configured** in this repo — there is no `[testenv:pip-audit]` stanza in `tox.ini` and `make security` runs only `safety`, `bandit`, and `gitleaks`. If you want pip-audit cross-check, install it manually (`pip install pip-audit`) and run separately; note in the report.

**Process:**
1. Run `tox -e safety`. Optionally `pip install pip-audit && pip-audit -r <lockfile>` for second-source coverage.
2. For each CVE not in the allowlist, document: package, version, CVE ID, severity per upstream, exploitability in this codebase (does the agent reach the vulnerable code path?).
3. Audit the existing tomte allowlist (`-i 37524 -i 38038 ...` in `tox.ini`) — for each pinned CVE, check whether upstream has shipped a fix that allows the pin to be removed.

**Fix:** upgrade the dep; if upgrade is impossible, document the mitigation in the allowlist with a date and re-check quarterly; for unreachable CVEs, document why they don't apply.

---

### Medium (M) — Hardening gaps and weak validation

#### M1: World-Readable Key Files in Container

**What:** Mounted private-key files with permissive mode (`0644`, `0755`), or container-internal copies created without chmod.

**Search patterns:**
- `Dockerfile*`: `COPY ethereum_private_key.txt` without subsequent `RUN chmod 600`
- `docker-compose*.yaml`: volume mounts of key files without explicit `read_only: true`
- Setup scripts that `cp` keys without chmod 600
- Kubernetes Secret mounts with `defaultMode: 0644`

**Fix:** `chmod 600 *_private_key.txt` after copy / mount; use Kubernetes Secret `defaultMode: 0400`; mount read-only.

#### M2: Debug / Health Endpoints Exposing Internal State

**What:** Flask Tendermint monitor (port 8080), agent debug routes, or health endpoints that return more than `{"status": "ok"}` — e.g. agent address, validator info, signed-payload count, configured params, last block.

**Search patterns:**
- `flask.*route(`, `@app.route(`, `@app.get(`, `@app.post(` in `packages/`, `autonomy/`, `deployments/`
- `make_response(`, `jsonify(` returning rich structures
- Response bodies that include `synchronized_data`, `params`, `address`, `validator`

**Threat:** reconnaissance for a targeted attack. Validator addresses + chain → fund tracing; configured RPCs → MITM target list; agent address → on-chain identity disclosure.

**Fix:** strip non-essential fields from health responses; gate debug routes behind `DEBUG=true` env var; require auth for any internal-state endpoint.

#### M3: Weak Validation at HTTP Handler Trust Boundaries

**What:** HTTP handlers in `handlers.py` that accept request bodies without schema validation. Audit-resilience covers crash patterns; /security-review M3 covers exploitability:
- SQL injection in any backing store query (rare in agents but check)
- Path traversal in any handler that reads/writes files based on request input (`open(f"data/{request.name}.json")`)
- SSRF in any handler that makes outbound requests to URLs from the request (`requests.get(request.url)`)
- XXE if XML parsing is used without `defusedxml`

**Search patterns:**
- `open(` with interpolated path component
- `requests.get(...)` with URL from request
- `xml.etree.ElementTree.parse(`, `lxml.etree.parse(` without `defusedxml`
- f-strings or `.format()` building SQL queries

**Fix:** validate inputs against a whitelist; resolve and check filesystem paths (`Path(...).resolve().is_relative_to(allowed_root)`); disallow user-controlled outbound URLs or restrict to allowlisted hosts; use `defusedxml.ElementTree`.

#### M4: Cryptographic Weakness

**What:**
- MD5 / SHA1 used for security purposes (signatures, MACs, password hashing)
- Hardcoded IVs / nonces in encryption
- ECB mode block ciphers
- `random.*` for security-critical randomness (use `secrets`)
- Custom HMAC implementations (use `hmac.compare_digest` for comparison)

**Search patterns:**
- `hashlib.md5(`, `hashlib.sha1(` — flag if used in `verify`, `auth`, `sign`, `mac` contexts
- `Crypto.Cipher.AES.MODE_ECB`, `AES.new(..., AES.MODE_ECB, ...)`
- `random.randint(`, `random.choice(`, `os.urandom(` (the latter is OK, but check usage) where the value is used as a key, IV, nonce, or token
- `==` comparison of HMACs / signatures (timing attack)

**Fix:** SHA-256+ for hashing; AES-GCM / ChaCha20-Poly1305 for encryption; `secrets.token_bytes(32)` for tokens; `hmac.compare_digest` for comparison.

#### M5: Loose CORS / Missing Security Headers

**What:** HTTP services (Flask Tendermint monitor, agent UI) with `Access-Control-Allow-Origin: *` or missing security headers (`X-Content-Type-Options`, `X-Frame-Options`, `Strict-Transport-Security`, `Content-Security-Policy`).

**Search patterns:**
- `Access-Control-Allow-Origin` with `*`
- `flask_cors.CORS(app)` with default config (= permissive)
- Absence of header-setting middleware

**Fix:** restrict CORS to known origins; set security headers via middleware; for read-only endpoints, document the threat model explicitly.

#### M6: License Compliance Drift

**What:** Per `CLAUDE.md`, only MIT / BSD / Apache 2.0 are allowed; GPL / LGPL / MPL are prohibited. Dependency tree may include prohibited licenses transitively.

**Process:**
1. Run `pip-licenses --format=markdown` (or `tomte`'s license tooling if available) against the resolved environment.
2. Flag any dep with GPL / LGPL / MPL / AGPL / SSPL / proprietary license.
3. Audit copyleft creep in transitive deps.

**Fix:** replace the dep; add explicit exception with legal review; document in NOTICE / SBOM.

---

### Low (L) — Defense in depth and observability

#### L1: Missing Secrets-in-Logs Redaction Policy

C2 covers individual-call-site disclosure. L1 asks the systemic question: is there a redaction layer in the logging pipeline (formatter / `structlog` processor / `logging.config.dictConfig` filter) that masks secret-shaped values, or does every author bear the burden individually? If the latter, file as L1 — defense-in-depth gap.

#### L2: No Startup-Time Secret-Validation Check

**What:** Even with H1 fixed (no insecure defaults), there's value in a startup check that asserts every required secret env var is set and matches an expected shape (e.g. `API_KEY` must be 32+ chars, `PRIVATE_KEY` must be 64 hex chars).

**Fix:** add a `validate_secrets()` call at agent startup; fail fast with a clear error if a required secret is missing or malformed.

#### L3: Tendermint Persistent Peer List Hardcoding

**What:** `persistent_peers` configured statically rather than via service discovery. Not a vulnerability per se, but limits the network's resilience to peer churn.

**Fix:** documented as informational; consider DNS-based or registry-based peer discovery for production.

#### L4: SBOM / Build Provenance

**What:** Are container images built with reproducibility / SBOM attestation? Are wheels installed from a verified PyPI mirror? Is the build pipeline signed?

**Fix:** generate SBOM via `cyclonedx-py` or `syft`; sign images with `cosign`; pin to an internal index where possible.

#### L5: Audit-Log Coverage of Security-Sensitive Events

**What:** Are key operations (key-file load, on-chain tx submission, ABCI payload signing, strategy load, config-reload) recorded to a separate audit log distinct from operational logs? Without this, post-incident forensics is hard.

**Fix:** route security events to a dedicated logger / external audit sink; include timestamp, agent id, operation, outcome.

---

## False Positive Guidance

These patterns look suspicious but are **correct by design**. Do NOT report them:

### Test-fixture private keys
Test fixtures (`tests/`, `plugins/aea-test-autonomy/`, `conftest.py`) routinely include hardcoded test private keys for deterministic test setup. These are well-known throwaway keys used by Hardhat / Ganache / test Tendermint. Do NOT flag these as C5. Limit C5 to non-test code paths.

### Dummy / placeholder values in *test* config files
Test YAML configs (`tests/test_data/`, `aea-test-autonomy` fixtures) use placeholder URLs / keys / addresses for hermetic testing. Don't flag these as H1 — they aren't deployed.

### `pickle.loads` of internally-produced data
If a `pickle.loads` reads a file or buffer that the agent itself wrote in the same process or container (e.g. local cache restore), it is not RCE-exposed in the same way as network input. Reduce severity to High and note the trust assumption.

### `subprocess.run(..., shell=False)` with interpolated args
Argument interpolation is safe when `shell=False` because the shell isn't involved — argv is passed directly to `execve`. Don't flag these as C6. C6 requires `shell=True`.

### `random.*` for non-security uses
`random.choice` for jitter, log sampling, A/B test bucketing, etc. is fine. Limit M4 to uses where the output is a key, IV, nonce, token, password, or anything that protects a secret.

### Internal RPC ports without auth
Tendermint RPC bound to `127.0.0.1` or an internal Docker network without external publication is the framework's default and is OK. H4 only applies when the port is published to a host interface or routable network.

### `eval(` in framework metaprogramming
Some framework-internal code uses `eval` for limited purposes (e.g. `enum.Enum` value coercion in metaclass init). If the input is statically constructed inside the same module and not user-controlled, do not flag as C1. Verify the input source before flagging.

### Hardcoded contract addresses in `customs/` / package_overrides
Contract addresses are public on-chain. They are not secrets. Don't conflate with API keys.

---

## Methodology Guardrails

Two sets of guardrails apply: the **cross-skill** ones already documented in `/audit-fsm` and `/audit-resilience`, and the **security-specific** ones below.

**Apply the cross-skill guardrails from the sibling Methodology Guardrails sections without restating them here:**
- *Verify every sub-agent finding before accepting it* — read source, confirm severity, consolidate splits, demote liberally.
- *Force the data-type trace before flagging* — runtime type ≠ static annotation when JSON crosses the boundary.
- *Single-finding-per-bug* — one root cause = one report entry citing all relevant IDs.

These reduce duplication and prevent drift when the sibling skills evolve. Below are the guardrails that are genuinely specific to security review.

### Distinguish disclosure from exposure
- **Disclosure** = the secret has left the trust boundary (in a log, response, payload, file outside the agent dir).
- **Exposure** = the secret is reachable via a code path but not yet emitted (e.g. `Wallet.crypto_objects[<ledger>].entity` / `.private_key` is callable from a behaviour).

C2 is for disclosure. Exposure is M-level at most, often L (defense in depth). Don't over-escalate exposure to Critical.

### Trace every "untrusted input" claim to a source
For C1, C4, C6, M3 findings: name the **source** of the untrusted input (HTTP request body, ABCI payload field, env var, file content, IPFS payload). If you can't name it, downgrade or drop the finding.

### Dual-skill findings — cite the owning skill, demote here
If a finding fits `/audit-fsm` or `/audit-resilience` better, cite that skill's check ID as the primary and demote here. Examples:
- `logger.info(f'key={k}')` → cite `audit-fsm` L4 first; /security-review C2 adds only the disclosure framing.
- `requests.get(...)` with no timeout → `audit-resilience` BP8 / CC5 owns this; /security-review flags only if the missing timeout enables a security-relevant DoS or resource-exhaustion attack on a trust boundary.

---

## Coverage Limitations

This skill audits **agent-process security** in the open-autonomy codebase. The following are explicitly NOT covered — call this out in every report:

- **Smart-contract security** (reentrancy, gas griefing, signature replay on chain) — out of scope; covered by Solidity-specific tooling (Slither, Mythril, manual audit).
- **Cryptographic-library implementation review** — we trust `cryptography`, `eth_keys`, `web3.py` etc. as black boxes. CVEs in those libraries surface via H7.
- **Tendermint internals** — we audit configuration, not the consensus engine.
- **Network / load testing** under attack scenarios — this audit is static.
- **Penetration testing of deployed services** — out of scope; recommend a separate engagement.
- **Compliance** (SOC2, ISO 27001, GDPR) — this audit checks technical controls, not policy / process / documentation.
- **Insider threat / multi-party governance** — out of scope.
- **Physical security** of deployment hosts — out of scope.

Companion skills:
- `/audit-fsm` — FSM correctness and safety
- `/audit-resilience` — external request resilience

Include a "What this audit did not cover" section in every report.

---

## Analysis Procedure

### Step 0: Run Existing Security Tools

Before manual inspection, run the framework's tooling and capture the output:

```bash
# Bandit — static analysis for common Python security issues
tox -e bandit

# Safety — known CVEs in pinned dependencies (uses tomte allowlist)
tox -e safety

# Gitleaks — secrets scanning across history
tox -e gitleaks

# Or run all three in parallel
make security
```

**Process the output:**
- Bandit findings → cross-reference with the C/H/M checklist; many will already match (e.g. Bandit's `B602` ≈ C6, `B301` ≈ C4).
- Safety findings → record under H7 with package, version, CVE ID; check the tomte allowlist for explanations.
- Gitleaks findings → record under C5; for each, verify whether the secret is rotated.

**Tooling-unavailable case.** If the tox envs cannot run (network restrictions, missing tomte version), note in the report and proceed with manual checks. Do NOT block the audit on tool availability.

### Step 1: Static Discovery

Launch up to **3 parallel Explore agents**, dividing the codebase:

- **Agent 1 — code paths**: search `packages/`, `autonomy/`, `plugins/`, `customs/` for the C/H/M/L patterns above. For each match, capture file:line, the surrounding context, and the input source.
- **Agent 2 — configuration**: parse `aea-config.yaml`, every `service.yaml`, every `skill.yaml` under audit. Extract env-var-interpolated params; flag insecure defaults (H1) and any param with security-relevant naming.
- **Agent 3 — deployment**: read `Dockerfile*`, `docker-compose*.yaml`, `deployments/`, `kubernetes/` if present, and `.github/workflows/*.yml`. Flag root containers (H3), exposed RPC (H4), hardcoded secrets in deployment artifacts (H2), key-file permissions (M1).

Each agent should return a structured list of candidates with check ID, file:line, and a short justification.

### Step 2: Semantic Verification

For each candidate from Step 1, an analysis agent:
1. Reads the call site and surrounding context.
2. Traces the input source for any "untrusted input" claim.
3. Applies the False Positive Guidance.
4. Promotes / demotes severity based on the actual blast radius.
5. Produces a finding entry with code snippet, threat description, and concrete fix.

### Step 3: Cross-Cutting Synthesis

After per-finding analysis:
1. Build a "Secrets Inventory" table: every credential / key / token surface, its storage mechanism, its rotation policy.
2. Build a "Trust Boundaries" diagram-table: every input source that crosses a trust boundary (network, IPFS, env, file), with its validator (or absence thereof).
3. Build a "Dependency Risk" table: every runtime dep, its license, its CVE status.
4. Identify systemic patterns: e.g. "secrets are consistently set via env vars but no startup validation" → L2 finding cluster.

### Step 4: Generate Report

Merge all findings into the output format below.

---

## Output Format

Present the security review as follows:

```markdown
# Security Review

**Scope:** [list of audited paths / packages / services]
**Date:** [current date]
**Tools run:** bandit (Y/N), safety (Y/N), gitleaks (Y/N), pip-audit (Y/N — optional second-source; install manually)

## Executive Summary

- Critical findings: N
- High findings: N
- Medium findings: N
- Low findings: N

[2-3 sentences on the highest-impact findings and the systemic patterns they reveal]

## Critical Findings

### [C#]: [Title]
- **File:** `path/to/file.py:line`
- **Threat:** [what compromise this enables, who can trigger it]
- **Code:**
  ```python
  [problematic code snippet]
  ```
- **Fix:**
  ```python
  [corrected code snippet]
  ```
- **Cross-references:** [audit-fsm L4 / audit-resilience BP14 / etc., if applicable]

## High Findings
[same format]

## Medium Findings
[same format]

## Low Findings
[same format]

---

## Secrets Inventory

| Secret | Storage | Loaded by | Rotation policy |
|---|---|---|---|
| Ethereum private key | mounted volume | `aea.crypto.wallet.Wallet` | manual |
| ... |

## Trust Boundaries

| Boundary | Input source | Validator | Findings |
|---|---|---|---|
| ABCI payload | Tendermint network | `check_payload()` per round | H5 if missing |
| HTTP handler | external HTTP | per-handler `_validate_*` | M3 if missing |
| IPFS strategy | IPFS network | content-hash check | C3 if missing |
| Env-var config | host environment | none / startup check | H1 if defaulted |
| ... |

## Dependency Risk

| Package | Version | License | CVE status |
|---|---|---|---|
| ... |

## Tooling Output Summary

### Bandit
[counts by severity, top categories]

### Safety
[CVEs not in allowlist]

### Gitleaks
[hits, by file]

---

## What This Audit Did Not Cover

- Smart-contract security (run Slither / Mythril / manual contract audit)
- FSM correctness (run `/audit-fsm`)
- External request resilience (run `/audit-resilience`)
- Cryptographic-library internals (trust black boxes; CVEs surfaced via H7)
- Tendermint consensus internals
- Penetration testing of deployed services
- Compliance audits (SOC2 / ISO 27001 / GDPR)
- Insider threat / governance review
- [Add scope exclusions specific to this run]
```

If no findings at a severity level, include the section header with "No findings." underneath.