Terminal degradation ladder + redacted forensic crash log (#144) - #215
Terminal degradation ladder + redacted forensic crash log (#144)#215OsherElhadad wants to merge 4 commits into
Conversation
…ve tail A classic `cap-evolve run` was completely silent for its whole duration: every phase ran under subprocess.run(capture_output=True), so a multi-hour hill-climb showed a blank terminal until it returned one JSON blob — and a hung run was indistinguishable from a working one. The only live view was the web dashboard, which leaves CI/ssh/headless/air-gapped users with nothing. - New core module `cap_evolve.eventstream` (stdlib only, zero new deps): the ONE place that reads a run's events.jsonl. read_new_events() (byte-offset incremental read, partial trailing line left unconsumed), follow_events() (blocking generator that waits for the file, stops on finalize/idle/signal), and format_event()/render_line() (one human-readable line per event, ANSI applied only by render_line). - `cap-evolve run --follow`: prints stage transitions, baseline, per-candidate accept/reject with candidate id + val + reason, budget warnings, optimizer errors, finalize, plus a cumulative cost/token meter. Progress goes to STDERR so stdout stays the machine-readable final JSON scripts parse. Runs on a daemon thread started before baseline creates the run dir, so the first events are never missed; it can never raise into the run. - `cap-evolve tail [run_dir]`: attaches to an existing or ongoing run (default: newest run_* under --base). Waits for the run dir to appear, so you can attach before the run creates it. --from-start replays history. - The dashboard's SSE route now imports read_new_events from the shared core helper instead of owning its own copy, so terminal and web read the same typed event stream and can never disagree. #118 (stall detection), #122 (replay), #138 and #144 build on this module. - Degrades cleanly with no TTY: plain text when piped, in CI, or under NO_COLOR. Closes #116
…ion, honest cost, stderr safety Review fixes for #191 (hub PR for #118/#122/#138/#144). - A malformed event no longer kills the follower thread. format_event is total (non-dict / bad `t` degrade to None or `--:--:--`), and cli.py reports on stderr instead of swallowing, so a dead follower is never mistaken for a silent run — the exact bug #116 exists to fix. - read_new_events returns only JSON objects, so no consumer (CLI or dashboard SSE) receives a bare 42/null/[1,2]; unreadable records surface as a log_corruption event instead of vanishing. - All rendered text is sanitised: C0/C1 controls and ESC sequences are stripped and newlines collapse, so an optimizer's stderr cannot set the window title, clear the screen, or forge a fake FINALIZE line. - The cost meter no longer double-counts: runner spend from `evaluate`, optimizer spend from `step`-likes, intake from `intake` — matching Spent.total_usd. Exposed as public accrue_totals so #138 does not fork the arithmetic. - --follow disables itself when stderr is unusable (`2>&-`) rather than letting progress corrupt the stdout JSON contract. - Hub API: follow_events yields a typed _follow_end sentinel naming its exit reason (stop_kind/idle/should_stop) for #118, should_stop now receives the last event, the 300s idle_timeout module default is gone, a shrunk file re-reads from 0 for #122, and format_event(skip_kinds=()) exposes bookkeeping kinds for #138. - tail exits 2 on an impossible run dir and 3 on an idle timeout with no events; --resume --follow no longer replays the whole prior log; use_color requires its stream; negative --idle-timeout is rejected.
…crash log Closes #144. Builds directly on #191 (issue #116), which landed the shared event tail, sanitize(), and the plain-on-non-TTY endpoints. The ladder is now an explicit contract, not two endpoints. eventstream.capability() names five rungs — full (TTY+colour), plain (NO_COLOR), dumb (TERM=dumb/unknown), pipe (non-TTY/CI), none (2>&- / closed) — each one detected by a single signal and each one tested. Only `full` may put an escape byte on the wire; the other four are byte-verified plain. use_color() is now exactly `capability() == "full"`, so there is one decision rather than two that can drift. `cap-evolve tail --ladder` prints the rung, so a CI job can assert its own output mode. Nothing at any rung repaints or addresses the terminal, which is how #144's height-budget concern is answered: append-only output has no layout to overflow on a resize. Non-UTF-8 robustness (assigned here by #191's review): emit() checks the stream's encoding up front and transliterates (± -> +/-, Δ -> d, — -> -) rather than raising UnicodeEncodeError inside the follower thread, which would take the live view dark. Pre-checking matters because CPython opens stderr with errors="backslashreplace": under PYTHONIOENCODING=ascii the write does NOT raise, it silently prints "\xb1". Technically ASCII, unreadable in practice. Forensic crash log: an unhandled exception in a subcommand or in the follow thread writes version, argv, python/platform, the terminal rung + encoding, the traceback and the last 25 events to the run dir (else ${XDG_CACHE_HOME:-~/.cache}/ cap-evolve/crashes/) and prints ONE line pointing at it. #191's review found the follower could die silently and take the run dark; that path now leaves evidence. Event payloads in the log are sanitised too — a crash log is `cat`ed as often as it is read. No secrets: the payload AND the user-visible crash line both go through the existing dashboard.redact, not a new scrubber. Writing the log is skipped entirely if redact is unavailable — no log beats a leaked key. Two real leaks were found and fixed while testing this: (1) main's redact lacked #193's ghp_/UUID shapes and its shape-independent pass over this process's secret-looking env values, so a bare high-entropy watsonx-style key survived — that hunk is ported byte-identical, so merging #193 in either order is a no-op; (2) the crash line printed at the user echoed the raw exception, leaking what the log had masked. Every CLI crash line now routes through one _safe_exc() helper (redact + sanitize), so no call site can forget either half. Left to #137 (CLI ergonomics): argument-parsing-level UTF-8 handling. This change owns the rendering/TUI half only.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| return fn(argv[1:]) | ||
| except (KeyboardInterrupt, SystemExit): | ||
| raise # Ctrl-C / an explicit exit code is not a crash | ||
| except BaseException as e: # noqa: BLE001 — #144: never exit with a bare traceback |
| for usd_key, tok_key in pairs: | ||
| try: | ||
| totals["usd"] = totals.get("usd", 0.0) + float(ev.get(usd_key) or 0.0) | ||
| except (TypeError, ValueError): |
| pass | ||
| try: | ||
| totals["tokens"] = totals.get("tokens", 0) + int(ev.get(tok_key) or 0) | ||
| except (TypeError, ValueError): |
🔬 EvidenceEvery command run verbatim, with full output. Zero-API throughout ( EnvironmentTest suite + compileallNew tests added by this PRRung 4 —
|
🔍 Review — PR #215CHANGES REQUESTED — one real credential leak reproduced end-to-end in the crash log AND on stderr (finding 1), plus a Blocking1. The PR's own leak test ( Reproduced in the real Consequence: the file the docs call "redacted, safe to attach to a bug report" ( Fix: widen the env-value pass to all env values that look like credential material, not just those under a secret-looking key — e.g. drop 2.
Confirmed in a real crash log: Note Fix: in 3. Consequence: the Non-blocking**4. The NO_COLOR spec is presence-based ("when present, regardless of its value") but explicitly excludes the empty string, so this is arguably conformant — however 5. 6. Issue #144 item 4 asks to "detect width (…reuse 7. 8. 9. Nits10. 11. 12. Crash-log leak auditMy own canaries, planted via four vectors simultaneously (argv space-separated, argv
Verdict: YES, one canary leaks — the bare high-entropy value, in both the log and on stderr (findings 1 and 2). Every shaped credential is masked; the shape-independent pass has an env-key blind spot. Also checked:
Ladder detection matrix
Only Is the redact port byte-identical to #193?NO. Three deltas — all comments/whitespace, zero functional: --- dashboard.py @ origin/feat/issue-121-doctor (#193)
+++ dashboard.py @ origin/feat/issue-144-tty-ladder (#215)
@@ -81,7 +81,7 @@
- # credential shapes that the length-based rules above miss.
+ # credential shapes the length-based rules above miss. (Also landing via #193.)
@@ -95,13 +95,14 @@
- that literally — the only shape-independent defense. Longest first so a value
- that contains another isn't half-masked.
+ that literally — the only shape-independent defence. Longest first so a value
+ that contains another isn't half-masked. (Also landing via #193; identical hunk.)
vals = {v for k, v in os.environ.items()
if v and len(v) >= 6 and _key_is_secret(k)}
return sorted(vals, key=len, reverse=True)
+
# KEY=secret / KEY: secret inside prose …( Semantically identical: YES — verified by AST comparison with docstrings normalised: Practical merge consequence: none for So the "either merge order is a no-op" claim is true in effect, false as literally stated. No divergent redactor results either way. Downgraded to a nit (10) rather than blocking — but please make it actually byte-identical so the claim in the PR body is checkable. Injection
The gap is that the allowlist covers control bytes and not control code points — _CTRL.update({c: None for c in (0x200B, 0x200E, 0x200F, *range(0x202A, 0x2030),
*range(0x2066, 0x206A), 0x2028, 0x2029, 0xFEFF)})Suggest filing it rather than blocking — but please don't leave it undocumented, since "sanitize handles untrusted text" is now asserted in three places. Merge-order noteMerged this branch into each sibling; all conflicts are one-line
Two corrections to the PR body: Recommendation: #191 → #118 → #193 → #215 → #137. #215 last among the Verification I re-ran233 passed, matching the claim (no #200 flake this run). #191's tests unmodified: confirmed — Rung transcripts, real
The pre-check-vs-catch claim — VERIFIED, and it matters. CPython opens stderr with So a ASCII fallback is readable, not garbage — Follower-death path — reproduced (forced Run continued, valid JSON on stdout, reason on disk — as claimed (and it surfaced finding 1). Exit 0 after follower death — I agree with the author, keep 0. The exit code answers "did the optimization succeed", and it did: the test number is real, the JSON contract is intact, and Second exception-print path — none found. Every exception that reaches a user in Docs — the ladder is documented as a five-rung contract table with the detection signal per rung ( |
|
❌ Automatic Labeling Failed An error occurred while trying to automatically label this pull request. Please check the workflow logs for details and add labels manually. |
…both crash records Review fixes for #215. Blocking: - dashboard.py: the shape-independent env pass only admitted values under a secret-LOOKING key, so a bare high-entropy credential exported as MODEL_ENDPOINT_SUFFIX was in neither the shape list nor the env list and reached the crash log (x4) and stderr (x1) verbatim. Admission is now OR-ed with a value-based rule (_looks_like_credential: length + no whitespace/separator + mixed character classes) that never consults the key name. Fifth denylist-shaped defence in this epic to fail; this one is value-based by design. - dashboard.py: `--api-key VALUE` (space-separated, the normal CLI form) was never masked, because redact walked argv element-wise and the separator is list adjacency, not a character. Handled inside redact's list branch so every list reaching an artifact is covered, plus a --flag-prefixed rule for prose. - eventstream.py: the crash filename was second-resolution and write_text truncated, so main()'s handler and the follow thread's -- which fire on the SAME failure -- silently destroyed each other's evidence. The name now carries pid+thread id and the file is created with O_EXCL. Non-blocking: - NO_COLOR is presence-based per no-color.org (empty-but-set now demotes). - FORCE_COLOR overrides TERM=dumb; NO_COLOR still wins over it. - sanitize also strips Unicode Cf/Zl/Zp (BiDi overrides, zero-widths, LS/PS): the allowlist covered control bytes, not control code points, so U+202E survived and could spoof a candidate id. - crash logs are 0600 and pruned to the newest 50. - KeyboardInterrupt writes a forensic log; its exit behaviour is unchanged. - deleted the dead retry loop in emit (unreachable after the _encodable pre-check). - record NO_COLOR/FORCE_COLOR as raw strings, not bool. - a dead follower now reports "follow": "stopped" inside the run's EXISTING final JSON object -- never a second document on stdout (#217). - docs: state the width/height scope reduction against #144 items 2 and 4 outright. Tests: the leak canaries now include innocent key names (MODEL_ENDPOINT_SUFFIX, DEPLOYMENT_ID, MY_FAVOURITE_STRING). The old test planted every canary under a key the regex already matched, so it only exercised the case that already worked -- the same defect pattern as #193's sk--prefixed canary. Also asserts benign env values (PATH, TERM, LANG) are not over-redacted, and that BiDi code points are stripped (written as escapes, so the test file is not itself a Trojan Source finding).
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| def _scrub_value(val: str) -> str: | ||
| out = _INLINE_KV_RE.sub(lambda m: m.group(1) + _REDACTED, val) | ||
| out = _FLAG_VALUE_RE.sub(lambda m: m.group(1) + _REDACTED, val) |
| def _scrub_value(val: str) -> str: | ||
| out = _INLINE_KV_RE.sub(lambda m: m.group(1) + _REDACTED, val) | ||
| out = _FLAG_VALUE_RE.sub(lambda m: m.group(1) + _REDACTED, val) | ||
| out = _INLINE_KV_RE.sub(lambda m: m.group(1) + _REDACTED, out) |
| try: | ||
| for stale in sorted(directory.glob("crash-*.json"))[:-keep]: | ||
| stale.unlink(missing_ok=True) | ||
| except OSError: |
🔧 Review fixesAll 3 blocking, 6 non-blocking and 3 nits addressed or explicitly declined. Commit The headline finding was right and it was worse than a style problem: a bare credential under an innocent key name reached a file the docs call "safe to attach to a bug report", so the docs were actively instructing users to leak it. Fixed at the root, and the test methodology that let it through is fixed too. Leak auditRe-ran your exact probe (
Note Why a value-based rule, not a key-name heuristicYou named the pattern precisely, so I took the option that doesn't guess at names. Weighing the three you listed: Scrub every env value present in the text (no filter). Rejected. Allowlist the crash-log fields. Right shape of defence, wrong layer. The payload is already effectively an allowlist (nine named fields, no free-form env dump). The leak was not an unexpected field; it was a credential inside Scrub all env values above an entropy/length threshold. ← chosen. The admission test is now OR-ed and the second arm never looks at the key name: def _looks_like_credential(value: str) -> bool:
if not 16 <= len(value) <= 512: return False
if any(c.isspace() or c in "/\\" for c in value): return False
return (any(c.islower() for c in value) and any(c.isupper() for c in value)
and any(c.isdigit() for c in value))A credential is an opaque single token: long, unbroken, mixed character classes. That is a property of the value, so Over-redaction is the safe direction here, so I checked the cost explicitly rather than asserting it — Marked with a 1. Innocently-named credential leak — FIXED (
|
… to stdout Review fixes for #137 (PR #214). B1 — the `cap-evolve <phase>` redirect printed a fixed `--run-dir <dir>` template that was wrong for 5 of the 8 phases: finalize also needs --project, and intake/implement-and-check/gate reject --run-dir entirely. A confidently wrong remediation at the SEAL step is worse than a bare 'unknown command'. _PHASE_SCRIPTS is now a name -> required-flags map derived from each script's real argparse, and the message also points at `--help` and the phase's SKILL.md. New test_phase_redirect_commands_are_runnable executes every rendered command and fails on any structural argparse rejection. B2 — _harden_utf8 reconfigured stderr, which defeated #215's eventstream._encodable() pre-check (it reads stderr.encoding to decide whether to transliterate +/-), silently shipping mojibake to an ASCII terminal on the merged tree. stderr is already opened errors=backslashreplace by CPython and belongs to #144's TUI ladder; stdout is opened strict and IS load-bearing (ASCII --help carries a right-arrow). Narrowed to (sys.stdout,). N1 — a whitespace-only docstring is truthy but strips to '', so .splitlines()[0] raised IndexError and took down every invocation including --help on 3.10-3.12 (requires-python >= 3.10). Uses next(iter(...), fallback). N2 — --dashboard-port had no validation and reached bind() as an uncaught OverflowError; because the launch precedes the --plan-only return, even a spend-nothing preview crashed. Now a 1-65535 check in the same pre-spend block. N3 — the documented-CLI absence-exclusion was line-wide, silently skipping 19 of the scanned sites (including four real `cap-evolve finalize` instructions) and disabling itself for any line containing a stray 'not'. Anchored to the text immediately before the backtick: 3 skips now, all genuine absence statements, and the bypass shape is caught. N4 — the e2e stdout-contract test spawned a real uvicorn and opened a browser, importing #200's port-contention flake. Passes --dashboard off. N5 — CONTRIBUTING.md now states the contract explicitly (stdout is exactly one JSON object on success AND failure) and carves out the two audited exceptions: `report --terminal` and finalize's TestSealError on an already-sealed run. Also re-homes #116/#118's `cap-evolve tail` exit-code prose into the module docstring as a merge note, so it is not lost when the deleted subcommand list is resolved.
Closes #144.
Builds directly on #191 (issue #116), which landed the shared event tail. This is
the robustness half: the ladder as an explicit contract, non-UTF-8 survival, and a
forensic crash log.
What #191 already covered vs what this adds
use_color(stream)— the colour seamcapability(stream)— five named rungs;use_coloris now exactlycapability == "full"NO_COLORhonouredfullmay emit an escape byte, byte-verified per rung2>&-)nonerung, named and testablesanitize()— escape-injection defencePYTHONIOENCODING=ascii/LC_ALL=Csurvival (assigned here by #191's review)cap-evolve tail --ladderscriptable rung read-outThe ladder
fullTERMnotdumb/unknown, noNO_COLORplainNO_COLORdumbTERM=dumb/unknownpipeod -cnone2>&-)An unset
TERMis deliberately not demoted — normal on a real TTY outside ashell profile;
dumbis the explicit opt-out.On height-budgeting (#144 item 2): nothing at any rung repaints the screen, moves
the cursor, or allocates a layout — output is append-only at every rung. There is no
live layout to overflow or double-render on a resize, which is the cheapest way to be
correct about resizes. No curses, no rich, zero new deps.
Non-UTF-8 locales
emit()checks the stream's encoding up front and transliterates (±→+/-,Δ→d,—→-). Pre-checking rather than catching matters: CPython opens stderrwith
errors="backslashreplace", so underPYTHONIOENCODING=asciithe write does notraise — it silently prints
\xb1. Technically ASCII, unreadable in practice.The crash log
Contents (
crash-<stamp>.json):cap_evolve_version,when,argv,python,platform,cwd,terminal(both rungs,TERM,NO_COLOR, encoding,PYTHONIOENCODING, locale),context(which code path),exception,traceback,recent_events(last 25). Lands next to the run, else${XDG_CACHE_HOME:-~/.cache}/cap-evolve/crashes/. One stderr line points at it; exitcode is non-zero. Follows #193's precedent of offloading verbose detail to a local
file rather than stdout.
No-leak proof. The payload and the crash line both go through the existing
dashboard.redact— not a new scrubber. Ifredactis unavailable the log is notwritten at all (no log beats a leaked key). Two real leaks were found and fixed
while testing:
main'sredactlacked Addcap-evolve doctorinstall/health diagnostic #193'sghp_/github_pat_/UUID shapes and itsshape-independent pass over this process's env values, so a bare high-entropy
watsonx-style key survived.
Correction (review): this PR originally claimed that hunk was ported
byte-identical to Add
cap-evolve doctorinstall/health diagnostic #193's. That was false as stated — there were threecomment/whitespace deltas. Precisely: it was AST-identical ignoring docstrings, and
dashboard.pyauto-merged clean either way, so the "either merge order is a no-op"effect held while the literal claim did not.
It is now moot: review found that Add
cap-evolve doctorinstall/health diagnostic #193's env pass only admitted values under asecret-looking key, so a bare credential exported as
MODEL_ENDPOINT_SUFFIXleaked into the crash log and onto stderr. This PR therefore hardens
redactpast Add
cap-evolve doctorinstall/health diagnostic #193 — admission is now OR-ed with a value-based rule that never consults thekey name — so the hunk is deliberately no longer a copy. Terminal degradation ladder + redacted forensic crash log (#144) #215 must land after
Add
cap-evolve doctorinstall/health diagnostic #193, because the leak fix lives here.The crash line printed at the user echoed the raw exception, leaking exactly what
the log had masked. Every CLI crash line now routes through one
_safe_exc()(redact + sanitize), so no call site can forget either half.
Multi-shape canary test asserts all four shapes: bare high-entropy, UUID,
ghp_,watsonx-style — not just an
sk-prefix. Correction (review): it planted everycanary under a key name the regex already matched, so it only exercised the case that
already worked — the same defect pattern as #193's
sk--prefixed canary. It now plantscanaries under innocent key names too (
MODEL_ENDPOINT_SUFFIX,DEPLOYMENT_ID,AUTH_HEADER_VALUE), and asserts benign env values (PATH,TERM,LANG) are notover-redacted.
Scope note (review): issue #144's items 2 and 4 (height budget, width detection) are
a deliberate scope reduction, not a satisfied requirement — both presuppose output
that takes over the screen, and every rung here is append-only. Stated in
GETTING_STARTED.md; a future repainting view needsdashboard._term_widthand thatitem re-opened.
Expected merge order
#191 → #118 → #193 → #215 → #214. #191 first (this branch is based on it; merge
probe: 0 conflicts). After #193, because this PR hardens the shared
redactpastit.
Correction (review): this PR previously said no
feat/issue-137-*branch existed.It does —
origin/feat/issue-137-cli-ergonomics, PR #214. Re-ran the probe: theonly textual conflict is
cli.py, and the resolution is take #214's side, since#214 deleted the one-line
usage:literal in favour of a generated listing.But the textual resolution is not sufficient, and this needs its own issue: #214
adds
_harden_utf8(), whichreconfigure(encoding="utf-8")s stdout/stderr at the topof
main(). That makes this PR's_encodable()see a UTF-8 stream underPYTHONIOENCODING=ascii, so transliteration never fires and±reaches an ASCIIterminal (
test_run_follow_survives_ascii_io_encodingfails in the merge). Two PRssolve the same problem incompatibly: #214 reconfigures the stream to accept the glyph,
#144 transliterates the glyph to fit the stream. Verified this pre-exists the review
fixes (identical failure against this PR's original head), so it is a design conflict,
not a regression. #214's own docstring says
ponytail: the CLI-level guard only; the TUI ladder is #144's job, which suggests_harden_utf8should yield to the ladder — butthat is for whoever rebases second to decide, not to silently pick here.
Verification
Full suite (baseline 179; #191 added ~37, this PR 17):
Rung 4 —
pipe(realcap-evolve run --followonexamples/toy_calc, mock optimizer, zero API)Zero escape bytes. stdout still parses:
test_reward: 1.0.Rungs 1–3 (real pty) and 5
Ladder read-out per rung:
PYTHONIOENCODING=ascii+LC_ALL=CCrash log + no-leak (4 planted canaries)
Zero canaries leak. The OSC attack embedded in the crash message is inert.
Dying follower mid-flight (run continues, evidence on disk):
with
"context": {"where": "follow-thread", "terminal_rung": "pipe"}, the traceback,and the last 3 events (
splits,evaluate,baseline) — diagnosable without a repro.The run still exited 0 with valid JSON.
Escape injection still inert (#191's three attacks,
color=Falseandcolor=True)Full commands + untruncated output in the
## 🔬 Evidencecomment below.