Update REPORT.md with fix status
This commit is contained in:
75
REPORT.md
75
REPORT.md
@@ -4,24 +4,23 @@ Generated 2026-04-08. Full codebase analysis.
|
||||
|
||||
## Critical
|
||||
|
||||
### 1. Shell injection in agent-manager.ts
|
||||
### 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:** Use `tee` with stdin instead of echo.
|
||||
**Fix:** Replaced with `tee` via stdin. No shell interpolation.
|
||||
|
||||
### 2. IP pool has no real locking
|
||||
### 2. IP pool has no real locking — FIXED
|
||||
**File:** `src/network.ts:202-222`
|
||||
`openSync` creates a lock file but never acquires an actual `flock`. Under concurrency, two agents can allocate the same IP.
|
||||
**Fix:** Use `flock` syscall or atomic file operations.
|
||||
`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
|
||||
### 3. --no-snapshot flag broken — FALSE ALARM
|
||||
**File:** `src/cli.ts:38`
|
||||
Commander parses `--no-snapshot` as `{ snapshot: false }`. Code checks `opts.snapshot === false` but passes it as `noSnapshot`. The flag may not work as intended.
|
||||
**Fix:** Verify Commander's behavior and align the check.
|
||||
Commander parses `--no-snapshot` as `{ snapshot: false }`. Verified: `opts.snapshot === false` is correct. No fix needed.
|
||||
|
||||
## High
|
||||
|
||||
### 4. SKILL.md parser fragile
|
||||
### 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
|
||||
@@ -29,56 +28,72 @@ Commander parses `--no-snapshot` as `{ snapshot: false }`. Code checks `opts.sna
|
||||
- Case-sensitive boolean parsing (`true` works, `True` doesn't)
|
||||
- Assumes Unix line endings
|
||||
|
||||
### 5. SSH host key verification disabled
|
||||
### 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 but should be a conscious documented decision.
|
||||
`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
|
||||
### 6. Memory not fully reloaded after save_memory — FIXED
|
||||
**File:** `agent/agent.py:319-326`
|
||||
After save_memory, MEMORY.md index is reloaded but individual memory files are not re-read. System prompt uses stale memory until next VM restart.
|
||||
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 4 times
|
||||
**Files:** `src/agent-manager.ts:305,410`, `src/overseer.ts:215,253`
|
||||
Same `["-o", "StrictHostKeyChecking=no", "-o", "UserKnownHostsFile=/dev/null", "-o", "ConnectTimeout=3", "-i", sshKeyPath]` array repeated. Extract to shared constant.
|
||||
### 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
|
||||
### 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 use one pattern.
|
||||
Two different implementations: one uses SIGTERM→wait→SIGKILL on ChildProcess, the other uses `process.kill(pid, 0)` polling. Should consolidate.
|
||||
|
||||
### 9. killall python3 hardcoded
|
||||
### 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
|
||||
### 10. Test suite expects researcher template — FIXED
|
||||
**File:** `tests/test-suite.sh:105`
|
||||
Asserts `researcher` template exists, but `scripts/install.sh` only creates worker, coder, quick.
|
||||
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
|
||||
### 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)
|
||||
### 12. agent.py monolithic (598 lines) — OPEN
|
||||
**File:** `agent/agent.py`
|
||||
Handles IRC, skill discovery, tool execution, memory, config reload in one file. Could split into modules but works for now.
|
||||
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
|
||||
|
||||
### 13. Hardcoded network interface fallback
|
||||
### 14. Hardcoded network interface fallback
|
||||
**File:** `src/network.ts:56` — defaults to `"eno2"` if route parsing fails.
|
||||
|
||||
### 14. Predictable mount point names
|
||||
### 15. Predictable mount point names
|
||||
**File:** `src/agent-manager.ts:94` — uses `Date.now()` instead of crypto random.
|
||||
|
||||
### 15. No Firecracker binary hash verification
|
||||
### 16. No Firecracker binary hash verification
|
||||
**File:** `scripts/install.sh:115-124` — downloads binary without SHA256 check.
|
||||
|
||||
### 16. Ollama response size unbounded
|
||||
### 17. Ollama response size unbounded
|
||||
**File:** `agent/agent.py:338` — `resp.read()` with no size limit.
|
||||
|
||||
### 17. IRC message splitting at 400 chars
|
||||
### 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.
|
||||
|
||||
### 18. Thread safety on _last_response_time
|
||||
### 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 |
|
||||
|
||||
Reference in New Issue
Block a user