Lap Tran
bc7daf8590
- **Restore 82 completed tasks** from tasks/complete/ back to tasks/ top level (all <7 days old per the cleanup policy; premature bulk archive was fixed). - **Dashboard: fix scroll-reset on auto-refresh** — renderBoard rebuilds the board via innerHTML every 2s, destroying each column-body's scrollTop. Now snapshots column-body scrollTop + board.scrollLeft + view.scrollTop before rebuild and restores after (matched by PHASE_GROUPS index). - **Dashboard UI additions** (pre-existing unstaged work): approval section cards, transition buttons, inline artifact editor (textarea for writing missing SPEC/VERDICT/etc from the detail modal). - **Bind ornith as Implement model** — config.md: Model explicit to omlx/Ornith-1.0-35B-4bit-mlx, context window 32768. Interactive autopilot already used ornith via opencode default; now explicit. - **Fix cleanup stub** — automaton-cleanup.sh had a stale --project arg pointing at a pytest temp dir (test isolation leak). Rewired to point at ~/.automaton. - **Fix plist-isolation test** — test asserted host plist doesn't exist, but a real install creates it. Now snapshots mtime before run, asserts unchanged after (only a write during the test counts as bleed). - **New Playwright smoke test** (tests/test_dashboard_ui.py) — 2 tests: board renders tasks, column scroll survives auto-refresh tick. Verified the test fails without the scroll fix (scrollTop resets to 0). Skipped via importorskip when playwright is absent (main CI stays green). - **Clarify SI loop scope in README** — new-project onboarding section documents the framework-scoped self-improvement loop and options (leave/pause/create project loop). - **CHANGELOG** documents all changes including the known model-divergence gap (mde tasks marked complete but per-role model binding was never implemented).
63 lines
5.7 KiB
Markdown
63 lines
5.7 KiB
Markdown
# Adversarial Bug Report: fix-harness-command-template
|
|
|
|
Attack the fix as a hostile user / harness would, looking for ways to escape substitution, break harness invocation, or corrupt state.
|
|
|
|
## Attack vectors tried
|
|
|
|
### A1 — Can a malicious `loop.json` `harness.command` element escape argv via shell metachars?
|
|
`subprocess.run` is invoked with a list (no `shell=True`). Each list element is passed verbatim as a single argv element to the OS. A `harness.command` like `["sh", "-c", "rm -rf /"]` would invoke `sh -c "rm -rf /"` as a literal argv element — but `rm -rf /` is still the *content* of the `-c` argument, so it DOES run `rm -rf /`. **This is config-trust, not a runtime escape**: the user controls `loop.json` and could equally well write any command. Pre-fix behavior was identical (custom commands were always honored). ACCEPTED.
|
|
|
|
### A2 — Can a hostile verifier prompt inject into `{prompt_content}` for the orchestrate role?
|
|
The implement role's stdout is captured as the artifact content. The verify role's prompt is built by `_resolve_prompt` which substitutes `{artifact_content}` from the implement output. If the implement role's stdout contains `"{prompt_content}"` or `{verdict}`, it becomes part of the verify prompt content (via `_resolve_prompt`'s content substitution), and the resulting `{prompt_content}` for the verify invocation includes that text. No security boundary violation — the implement role was already allowed to influence the verify prompt (v1 behavior). ACCEPTED.
|
|
|
|
### A3 — Can `{prompt_content}` be leaked via the orchestrator's stdout capture?
|
|
The orchestrator's stdout is written to `<loop>/outputs/tickN-orchestrate.json`. If the orchestrator echoes `{prompt_content}` (which contained sensitive task content), the content is recorded. This is intended behavior — the orchestrator is supposed to see the prompt context. ACCEPTED.
|
|
|
|
### A4 — Can a path traversal in `loop_path` corrupt the prompt file write?
|
|
`_resolve_prompt` writes to `<loop_path>/outputs/tickN-<role>-prompt.md` using `out_dir.mkdir(parents=True, exist_ok=True)` and a fixed filename. No user-controlled path component — `tick_num` is an int, `role` is internal. ACCEPTED.
|
|
|
|
### A5 — Does `Path(resolved_prompt).read_text()` ignore encoding errors?
|
|
No `encoding` arg uses platform default. A prompt file with invalid bytes for the default encoding raises `UnicodeDecodeError`, which is NOT caught by the `try/except OSError` (UnicodeDecodeError is a `ValueError`, not OSError). The exception propagates up and the tick crashes.
|
|
|
|
**Wait — this is a real bug.** Let me check:
|
|
- `_invoke_harness` does `try: prompt_content = Path(resolved_prompt).read_text() except OSError`.
|
|
- `UnicodeDecodeError` is a subclass of `ValueError`, NOT `OSError`.
|
|
- So a binary prompt file (or a UTF-16 file with BOM, or any non-default-encoding text) would crash the tick.
|
|
|
|
Pre-fix behavior: `{prompt}` was just the file PATH string. No read happened in `_invoke_harness`. So this is a NEW failure surface introduced by my change.
|
|
|
|
**Severity**: LOW — prompt files are written by `_resolve_prompt` itself (markdown, UTF-8). A user would have to drop a binary file at `<loop>/<prompt_ref>` to trigger it. But the framework should not crash on a misconfigured prompt file; it should fall back to empty prompt content and halt with `verifier_failed` (graceful).
|
|
|
|
**Fix recommendation**: broaden the except clause to `(OSError, UnicodeDecodeError)` or use `except Exception` for the read. Or pass `encoding="utf-8", errors="replace"` to `read_text()`.
|
|
|
|
I'll fix this inline before transitioning to doc_review. It's a small, contained hardening — the alternative (a crash mid-tick) violates the idempotence contract.
|
|
|
|
### A6 — Can a missing prompt file slip through silently on the create path?
|
|
If `loop_path is None` (no loop context), `_resolve_prompt` is skipped and `resolved_prompt = prompt_path` (the raw ref). Then `Path(resolved_prompt).read_text()` fails with OSError, `prompt_content = ""`. Default command becomes `["opencode", "run", "--dir", "<cwd>", ""]`. The spawned opencode runs with no prompt. This matches the documented fallback (D-H2 mentions the empty-prompt fast path). Accepted.
|
|
|
|
### A7 — Can two concurrent ticks both compute the same `{prompt_content}` and clobber?
|
|
`{prompt_content}` is computed locally in each tick process. No shared state. The temp file is written by `_resolve_prompt` to `<loop>/outputs/tickN-<role>-prompt.md` where `tickN` is the current iteration count. Two ticks with the same iteration count would write to the same temp file path — but that's the same TOCTOU covered by `add-state-loop-lock` (task 2; SPEC already written). Out of scope for this task.
|
|
|
|
## Bugs found
|
|
|
|
**One LOW bug (A5)**: `Path(resolved_prompt).read_text()` raises `UnicodeDecodeError` on non-default-encoding prompt files, which is not caught by the `except OSError` clause. Causes a tick crash instead of a graceful `verifier_failed` halt.
|
|
|
|
## Fix applied inline
|
|
|
|
Broadened the except clause to also catch `UnicodeDecodeError`. See `scripts/loop-runner.py` line 319 (the `try/except` around the prompt-file read). Added `UnicodeDecodeError` to the tuple; falls back to `""` on decode failure.
|
|
|
|
Not adding a separate test for this — it's a defensive code broadening, well-narrowed by the type information.
|
|
|
|
## Five loop-death modes — coverage unchanged
|
|
|
|
| Death | Defense | Affected by fix? |
|
|
|-------|---------|------------------|
|
|
| drift | `_gate_worktree_drift` (status.py) | No |
|
|
| runaway | `_gate_iterations` (status.py) | No |
|
|
| bad verifier | `_gate_score_plateau` (status.py) + `parse_verdict` | No |
|
|
| resource burn | `_gate_budget` (status.py) | No |
|
|
| undetected halt | R8 transition refusal + audit Cat-6 | No |
|
|
|
|
## Verdict
|
|
|
|
PASS — one LOW bug found (A5), fixed inline. Proceed to doc_review. |