Skip to content

fix: bug-hunt round — security hardening, lifecycle and TUI state fixes - #154

Merged
jkyberneees merged 28 commits into
mainfrom
fix/bug-hunt-audit
Oct 12, 2026
Merged

jkyberneees merged 28 commits into
mainfrom
fix/bug-hunt-audit

Conversation

@jkyberneees

Copy link
Copy Markdown
Contributor

Bug-hunt round across every package: functional and security defects found by parallel audits, each fixed with a regression test, then put through several rounds of adversarial review and an end-to-end run against a real odek serve.

Security

  • Terminal escape injection: remaining raw sinks now sanitized (panel status line quoting HTTP bodies, sub-agent result status, plan step status, session/run ids). sanitize() no longer lets invalid UTF-8 (raw 0x9b/0x9d 8-bit CSI/OSC) through its fast path and strips more invisibles (U+061C, U+180E, U+2061–2064, U+FFF9–FFFB, tag block).
  • Path traversal: export filenames map wire session ids to [A-Za-z0-9._-].
  • Token leak on redirect: authenticated REST clients refuse redirects (Go copies X-Odek-Ws-Token / X-Session-Token across hosts).
  • Unbounded reads: every REST JSON decode and the release-metadata decode are size-capped; /attach rejects FIFOs and devices (/dev/zero froze the UI and blocked quit) and vets the opened descriptor.
  • Symlink exfiltration: persisted attachments are stored resolved and never silently re-attached once a symlink appears along the path.
  • Permissions / PID reuse: ~/.bodek is 0700; leader signalling goes through Process.Signal; the watchdog identifies the parent by start time.

Process lifecycle

  • A failing log sink no longer kills odek serve via SIGPIPE; WaitDelay bounds Stop when a grandchild holds stderr; stderr partial lines are capped.
  • A second Ctrl+C / SIGTERM can abort a slow shutdown, and the orphan watchdog stays armed until odek exits.

State / data integrity

  • Tokens whose save failed survive the next merge; a settings file broken mid-run is not overwritten; workspace saves persist under the lock.
  • update.Newer: a release now outranks its own pseudo-version / -next builds (the existing test asserting the opposite is deliberately flipped).

TUI correctness

  • Armed actions and selections stay on their target across refreshes (runs tab, /queue drain, memory facts incl. duplicate text, sessions list/search/delete, palette).
  • Session resume is refused while a turn runs (documented in README/AGENTS.md); --resume with a persisted queue no longer loses the resume — sends wait until it lands, and /new / explicit ^R supersede it.
  • One approval-expiry tick chain at a time; jobs/plan replies for a session already left are dropped; key sentinels and Alt chords are never typed into hidden inputs; expired poll timeout clamped; tool arguments shown without HTML escaping; /attach outcome notes paint immediately.

Behaviour changes to note

  • Redirects on authenticated REST calls now fail with a status error instead of being followed (e.g. an http→https proxy hop).
  • WebSocket frame writes have a 60s deadline (very large attachment prompts on very slow links could hit it; reconnect recovers).
  • Commit-installed builds now see the upgrade hint for the release they predate.
  • Subdivision flag emoji (tag characters) render as a plain black flag.
  • Attachment names are the real file's basename; legacy persisted paths under a symlinked directory (e.g. macOS /tmp) are skipped once with a note.

Verification

  • make fmt, make vet (darwin, linux, windows), make lint (0 issues), make test (race) all green; every commit vets on its own.
  • New regression tests fail on the pre-fix code (the FIFO one hangs it).
  • E2E against odek v2.34.2 in an isolated HOME via a pty driver: streaming, queueing, cancel, resume refusal mid-turn, --resume with a persisted queue, /attach guards, clean quit/SIGTERM with no orphaned odek serve, and hostile escape bytes delivered through a real tool result rendered inert.

Not addressed (need a decision)

  • Release checksums are unsigned (corruption check only).
  • Concurrent bodek instances share one fixed server-log path.
  • ^U under an approval scrolls instead of clearing the draft (contradicts AGENTS.md).
  • A bare a/d/t can answer an approval that arrived in the same frame, before it was painted.
  • Transient notes are not shown on the empty first-run home screen (pre-existing).

🤖 Generated with Claude Code

jkyberneees and others added 28 commits October 12, 2026 15:00
A failing log sink (full disk) surfaced to os/exec, whose copy goroutine
closed the stderr pipe and killed odek on SIGPIPE: the scan writer now
swallows sink errors. WaitDelay bounds Wait when a grandchild that left
the process group holds fd 2, so Stop can no longer hang forever, and
the leader-pid fallback is skipped once the child is reaped (its pid may
be recycled). Unterminated stderr lines are capped at 64 KiB in both the
token scanner and the startup ring buffer.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A recycled parent pid kept the guard waiting forever after bodek was
killed, orphaning odek serve. The parent now gets the same start-time
token check as the target.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The signal channel stayed registered after the first SIGINT/SIGTERM, so
later signals were swallowed while Stop waited up to 30s.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
save fell back to the startup baseline when the file became unreadable
or unparsable, then replaced it — clobbering the user's hand edits, the
case persistDisabled already guards at startup. It now refuses.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Go pseudo-versions (and goreleaser -next snapshots) name the next patch:
v0.1.3-0.<ts>-<hash> is a commit after v0.1.2, so the v0.1.3 release is
newer. Equal cores read as up to date, hiding the upgrade. The existing
test asserting the opposite is flipped deliberately.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Seven decodes read resp.Body without the maxJSONBytes cap getJSON
documents, so a misbehaving server could stream until bodek ran out of
memory.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Go strips Authorization on a cross-host redirect but copies custom
headers, so X-Odek-Ws-Token and X-Session-Token (and a replayed POST
body on 307/308) reached whatever host a proxy redirected to. odek's API
never redirects; the 3xx now surfaces as a status error.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A peer that stopped reading parked send forever while holding wmu,
wedging every later approval, cancel, and ping behind it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
mergeLocked adopted the on-disk map wholesale and re-applied only the
current id, so a token whose persist failed (full disk, read-only dir)
was erased for good by the next successful Set. Mutations not yet on
disk are now tracked and replayed after the merge.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A first save from /theme created the directory 0755, exposing the names
of the token and workspace stores that share it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Save and Patch wrote a snapshot after unlocking, so interleaved writes
could land out of order on disk — the reorder tokens already fixed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The panel status line (which can quote raw HTTP error bodies), sub-agent
result status, and shortID'd session/run ids reached the terminal
without sanitize(); shortID also cut mid-rune. sanitize's fast path let
invalid UTF-8 through, so lone 0x9b/0x9d bytes (8-bit CSI/OSC) survived,
and it missed U+061C, U+180E, U+2061-2064, U+FFF9-FFFB and the tag block.
Export filenames now map a wire id to [A-Za-z0-9._-] so it cannot
traverse out of the working directory.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A FIFO blocked the Update goroutine forever (and, persisted, on every
launch in that cwd); /dev/zero reports size 0 and read without bound.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Several lists are replaced under a live selection or armed gate, so a
keypress could act on a row the operator never chose:
- runs tab: a poll re-indexed rows under a/D/t/c (now anchored by id;
  reopening drops the stale list)
- /queue: a head drain left the delete armed on the shifted index
- memory: a refresh that dropped the confirmed fact retargeted y
- sessions: a late list/search page or delete reply moved the armed
  delete onto another session
- palette: late session rows re-ranked the highlighted entry

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Resuming mid-turn swapped the transcript without resetting curIdx, so
the running turn's remaining frames streamed into an arbitrary replayed
message and its approval cards stayed up. Resume is refused while busy
(the sessions panel says why), and a resume now also resets the
turn-scoped focus, find, copy, approval and clarify state.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Every wire batch called approvalSweep while an approval was pending and
each tick re-armed its own chain, so a long wait under streaming frames
built hundreds of 1s chains, each refreshing the view.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
An in-flight jobs snapshot from the previous session became the new
session's baseline (wrong rows, stop with a foreign job id, lost exit
notes), and a late plan 404 for the old session disabled the plan strip
before the stale-reply guard ran.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Disarming a confirm forwarded any rune key to the composer, so Alt
decisions, pastes and the shift+enter/shift+delete sentinels landed as
text — under an approval this also killed the bare-key path. Only plain
typing falls through now (clarify gets it in its own buffer), and the
shift+delete sentinel is a no-op on the approval, clarify, popup,
palette and panel surfaces instead of splicing in its name.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
An EINTR retry past the deadline passed a negative timeout to poll(2),
which blocks indefinitely: a held ESC or partial sequence then waited
for the next keystroke instead of its 10ms budget.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
json.MarshalIndent escaped <, > and & to \u003c etc. in the inspector
and in alt+i copies.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Stop retired the watchdog before signalling and waiting. With a second
Ctrl+C now restoring the default signal action, bodek could die mid-Stop
with nothing left to escalate a hung odek serve, orphaning it. The
watchdog is retired in a defer once the child is gone; it stays inert
while bodek is alive.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The reaper flags the child only after Wait returns, up to WaitDelay
after wait4 freed the pid, so the raw kill(pid) fallback could hit a
recycled process. Process.Signal reports ErrProcessDone from the moment
the child is reaped.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With --resume and a persisted queue, the connect-time session frame
drained the queue before SessionDetail returned; the turn went busy and
the resume was refused, so the saved session was lost. Queue drains,
composer submits and /queue Enter now wait for the startup resume, and
every outcome releases the hold. A resume generation drops replies made
stale by /new or by a later explicit ^R pick.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The armed-confirm check after a refresh matched on text alone, so the
same fact under user and env (or two episodes with one summary) could
move y onto a row never confirmed. The gate now re-finds the row by
kind, text, session and hash, or disarms.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The REST plan snapshot's status reached the terminal raw when a step
was opened, an escape-injection sink the sanitize sweep missed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Persisted attachments are re-read at startup without the operator
choosing them. An agent with workspace write access could swap one (or
a parent directory) for a symlink to a host secret, which then went out
with the next prompt. attachFile now stages and persists the fully
resolved path, restore skips any path that no longer resolves to
itself, and readCapped opens O_NONBLOCK and vets the descriptor so a
path swapped for a FIFO after the stat cannot block the UI.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The note (staged, or why not) was pushed into state without a redraw,
so it stayed invisible until an unrelated event repainted.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jkyberneees
jkyberneees merged commit ea1a789 into main Oct 12, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant