fireclaw/REPORT.md

Fireclaw Code Review Report

Generated 2026-04-08. Full codebase analysis.

Critical

1. Shell injection in agent-manager.ts — FIXED

File: src/agent-manager.ts:120-131 Config JSON and persona text interpolated directly into shell commands via echo '${configJson}'. If persona contains single quotes, shell breaks or injects arbitrary commands. Fix: Replaced with tee via stdin. No shell interpolation.

2. IP pool has no real locking — FIXED

File: src/network.ts:202-222 openSync created a lock file but never acquired an actual flock. Under concurrency, two agents could allocate the same IP. Fix: Atomic writes via writeFileSync + renameSync. Removed fake lock.

3. --no-snapshot flag broken — FALSE ALARM

File: src/cli.ts:38 Commander parses --no-snapshot as { snapshot: false }. Verified: opts.snapshot === false is correct. No fix needed.

High

4. SKILL.md parser fragile — OPEN

File: agent/agent.py:69-138

• Requires exact 2-space/4-space indentation
• No error reporting when skills fail to load
• No validation of parameter types
• Case-sensitive boolean parsing (true works, True doesn't)
• Assumes Unix line endings

5. SSH host key verification disabled — DOCUMENTED

Files: src/ssh.ts:104, src/agent-manager.ts, src/overseer.ts hostVerifier: () => true and StrictHostKeyChecking=no everywhere. Acceptable on private bridge network (172.16.0.0/24) — VMs are ephemeral and host keys change on every boot. Conscious design decision, not an oversight.

6. Memory not fully reloaded after save_memory — FIXED

File: agent/agent.py:319-326 After save_memory, only MEMORY.md index was reloaded. Individual memory files were not re-read into system prompt. Fix: Extracted reload_memory() function that reloads index + all memory/*.md files.

Medium

7. SSH options duplicated — FIXED

Files: src/agent-manager.ts, src/overseer.ts Same SSH options array repeated 4+ times. Fix: Extracted SSH_OPTS constant in both files.

8. Process termination inconsistent — OPEN

Files: src/firecracker-vm.ts:140-164, src/agent-manager.ts:319-332 Two different implementations: one uses SIGTERM→wait→SIGKILL on ChildProcess, the other uses process.kill(pid, 0) polling. Should consolidate.

9. killall python3 hardcoded — OPEN

Files: src/agent-manager.ts:310, src/agent-manager.ts:430 Assumes agent is always a python3 process. Fragile if agent wrapping changes.

10. Test suite expects researcher template — FIXED

File: tests/test-suite.sh:105 Test asserts researcher template exists, but install script only created worker, coder, quick. Fix: Added researcher and creative templates to scripts/install.sh.

11. Bare exception handlers — OPEN

File: src/agent-manager.ts — 8+ catch {} blocks Swallow all errors silently. Makes debugging impossible.

12. agent.py monolithic (598 lines) — OPEN

File: agent/agent.py Handles IRC, skill discovery, tool execution, memory, config reload in one file. Functional but could benefit from splitting.

13. Unused writePool function — FIXED

File: src/network.ts:198 Left over after switching to atomicWritePool. Removed.

Low

14. Hardcoded network interface fallback

File: src/network.ts:56 — defaults to "eno2" if route parsing fails.

15. Predictable mount point names

File: src/agent-manager.ts:94 — uses Date.now() instead of crypto random.

16. No Firecracker binary hash verification

File: scripts/install.sh:115-124 — downloads binary without SHA256 check.

17. Ollama response size unbounded

File: agent/agent.py:338 — resp.read() with no size limit.

18. IRC message splitting at 400 chars

File: agent/agent.py:266 — IRC limit is 512 bytes including protocol overhead. Safe limit is ~380 chars.

19. Thread safety on _last_response_time

File: agent/agent.py:485 — global updated without lock, two rapid requests could both pass cooldown.

Summary

Status	Count
Fixed	7
False alarm	1
Open (medium+)	6
Open (low)	6