4.2 KiB
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 — FIXED
File: agent/agent.py:69-138
Fix: Added error logging on parse failures, flexible indent detection (2+ spaces), CRLF normalization, boolean case-insensitive (true/True/yes/1), parameter type validation with warnings.
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 (low risk)
Files: src/firecracker-vm.ts:140-164, src/agent-manager.ts:319-332
Two different implementations. The ChildProcess version uses SIGTERM→SIGKILL, the PID version uses polling. Both work, different contexts (owned vs adopted processes).
9. killall python3 hardcoded — FIXED
Files: src/agent-manager.ts
Fix: Replaced killall python3 with pkill -f 'agent.py'. Targets the specific script, not all python3 processes.
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 — FIXED
File: src/agent-manager.ts
Fix: Added error logging to cleanup catch blocks in listAgents and reconcileAgents. Remaining bare catches are intentional best-effort cleanup (umount, rmdir, unlink).
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 — FIXED
File: agent/agent.py:266
Fix: Reduced to 380 chars to stay within IRC's 512-byte line limit.
19. Thread safety on _last_response_time — FIXED
File: agent/agent.py:485
Fix: Added _cooldown_lock (threading.Lock) around cooldown check-and-set.
Summary
| Status | Count |
|---|---|
| Fixed | 13 |
| False alarm | 1 |
| Open (medium) | 2 |
| Open (low) | 4 |