Harden the pipeline against untrusted sessions (v0.4.0)

.abcd/work/DECISIONS.md

@@ -186,3 +186,61 @@ Architecture-shaping decisions graduate to an ADR under
   it used the zero-value anchor and wrote a silently corrupt timeline (~55-year
   offsets, nonsense report duration) with exit 0. Transcript-only sessions are
   unaffected.
+- 2026-07-22 — Close the Ingest/AppendVerdict TOCTOU on `findings.jsonl`:
+  `Ingest` now runs its verdict-guard probe, `O_TRUNC`, and rewrite as one step
+  under a `LOCK_EX` advisory lock (`commitFindings`), matching the lock
+  `review.AppendVerdict` already holds. Previously the probe and the truncating
+  write were two lock-free opens, so a concurrent `testimony review` could commit
+  a verdict between them and have the re-ingest destroy it — the human-decision
+  record the guard exists to protect. Sibling sweep: `findings.jsonl` is the only
+  session file with both an appender and a truncating rewriter; the other
+  `WriteJSONL` sites (timeline, transcript) are truncate-only, no sibling.
+- 2026-07-22 — Bind each review verdict to the finding the analyst judged.
+  `review.AppendVerdict` now takes the shown finding and, under its existing
+  `LOCK_EX` on findings.jsonl, re-reads the current findings and refuses if the
+  targeted id is gone or now names a different finding (`verifyTarget`,
+  `analyze.SameIdentity`). Previously the walk validated the target only against
+  the in-memory snapshot from `analyze.Load`, so a concurrent `analyze -ingest`
+  (permitted until the first verdict exists) that restarts finding ids at F-001
+  could slide a different finding under the same id and misattribute the
+  operator's verdict — silent corruption of the precision record. Refactored
+  `analyze.Load` to expose `ParseRecords(io.Reader)` so the re-check reads through
+  the already-locked descriptor. Sibling sweep: `AppendVerdict` is the sole
+  verdict writer; both production callers now bind the judged finding.
+- 2026-07-22 — Guard the untrusted-time-magnitude class end to end. Utterance
+  t0/t1 (session-relative float64 seconds) now have a magnitude bound in
+  timeline.checkedUtterances (1e9 s), the speech-side twin of the interaction
+  t<=0 guard; report.clock and review.clock refuse non-finite / out-of-range
+  seconds before the float64→int conversion (rendering `--:--`), defending the
+  display sink for a hand-authored timeline.jsonl/findings.jsonl that bypasses
+  merge; and report's trailing standalone-event flush uses a +Inf sentinel
+  instead of a finite 1e18 that silently dropped events at/after it. Sibling
+  sweep: report.clock and review.clock were the two duplicated float→int time
+  sinks; both guarded.
+- 2026-07-22 — Validate findings against the SafeText form of the timeline.
+  EmitRequest shows the agent each timeline line through session.SafeText, but
+  analyze.validate indexed and compared the raw bytes, so an id/selector/route/
+  quote carrying a stripped Bidi_Control or control character could never be
+  matched by an honest verbatim-copied answer (fail-closed, hostile/hand-edited
+  transcripts and genuine RTL speech). indexTimeline now stores SafeText keys and
+  validate compares SafeText(quote/selector/route/id). SafeText is a no-op on
+  ordinary text. Resolves the selector/route/id sibling of the earlier
+  emit-quote asymmetry.
+- 2026-07-22 — Surface ffmpeg's own diagnostic when the avfoundation device
+  listing is empty (record.probeDevices): an ffmpeg built without avfoundation
+  exits non-zero as an *exec.ExitError with the cause in its output, previously
+  discarded and misreported as "no microphone found". Now a bounded output tail
+  is appended to the error.
+- 2026-07-22 — transcribe persists the audio→session offset for external
+  recordings in a sidecar (`audio.offset.json`) beside audio.wav. A bare re-run
+  (different model, reusing audio.wav) reads it back instead of silently assuming
+  offset 0 and shifting every utterance; a record-origin audio.wav has no sidecar
+  and stays offset 0; a present-but-unusable sidecar refuses with guidance rather
+  than guessing. New session-directory artefact, documented in
+  docs/reference/session-directory.md.
+- 2026-07-22 — record captures the microphone via avfoundation ":default" (the
+  system default input) instead of enumerated index 0, so a virtual audio driver
+  (BlackHole/Loopback/conferencing device) that enumerates first is no longer
+  silently recorded in the real mic's place; startRecorders logs the detected
+  audio-input roster so a surprising default stays visible. NOTE: the avfoundation
+  capture path is not exercised by CI — verify on macOS hardware before release.

CHANGELOG.md

@@ -8,7 +8,107 @@ leading `v`.
 Before v1.0.0, minor releases may make breaking changes; each one is
 called out in a **Breaking** section.
 
-## [Unreleased]
+## [0.4.0] - 2026-07-24
+
+A second robustness pass over the same capture → analysis pipeline, closing the
+defects a multi-round review surfaced after v0.3.0. Every fix carries a
+regression test; the review finished with two consecutive passes finding only
+nitpicks.
+
+### Fixed
+
+Evidence-record and validation integrity:
+
+- A failed write of `findings.jsonl` (a full disk part-way through) no longer
+  leaves a truncated fragment that bricks the file against its own recovery —
+  the re-ingest path could not even parse it to overwrite it. The write now rolls
+  back to an empty, re-ingestable file.
+- `report.md`, the shareable evidence artefact, no longer passes attacker-authored
+  text through as live Markdown: an image or link in a transcript quote (e.g. a
+  remote-image tracking beacon) is neutralised so it renders as literal text.
+- A finding whose verbatim quote consists only of stripped control/bidi
+  characters is rejected, rather than passing the quote check against an empty
+  string.
+- A hand-edited `findings.jsonl` carrying two findings with the same `id` is
+  refused on load, naming the line, instead of silently collapsing them under one
+  status in the report.
+- An interaction whose epoch-millisecond time is astronomically large is refused
+  at merge (matching the existing utterance-side bound), rather than producing a
+  session span the report can only render as `--:--`.
+
+Robustness against malformed or exchanged sessions:
+
+- `analyze -ingest FILE` and `analyze -out FILE` now go through the same
+  symlink/FIFO-refusing guard every other session-artefact read and write uses, so
+  a pointer into an untrusted session directory cannot redirect the write out of
+  the session or hang the read on a planted FIFO.
+- The session's own `audio.wav` is refused when it is a symlink, so a shared
+  session cannot redirect transcription at an out-of-session file whose words would
+  then land in the (re-shareable) `transcript.jsonl`.
+- A missing-vs-unreadable `audio.wav` is distinguished, so a permissions or
+  symlink-loop error is no longer misreported as "no audio, run record first".
+- A finding id embedded in a `review` error message is sanitised before it reaches
+  the terminal, closing an ANSI-escape path through a hand-authored id.
+- A derived or persisted audio offset beyond a plausible magnitude is refused at
+  `transcribe`, where the bad recording metadata enters, rather than one command
+  later at `merge`.
+
+Capture reliability (`record`/`transcribe`, macOS):
+
+- Device enumeration now runs under a timeout, so a wedged capture device or
+  driver no longer hangs `record` before it can be interrupted.
+- A recorder's captured stderr is bounded, so a device-stall log flood over a long
+  session cannot exhaust memory and orphan the recorders.
+- Voice-recording conversion writes to a temp file and renames on success, so an
+  interrupted or crashed `ffmpeg` never leaves a partial `audio.wav` a later run
+  would transcribe as if complete.
+- A recorder that had to be force-stopped (it missed the finalisation grace) is
+  flagged, so a truncated, unplayable `screen.mp4` is no longer reported as good.
+
+Further adversarial review passes over that same commit closed what it had
+itself missed:
+
+- Text sanitisation now strips every invisible Unicode format character (zero
+  width space, word joiner, BOM, soft hyphen, the tag block), not only the bidi
+  controls — closing the remaining gap between what a terminal or `report.md`
+  displays and the bytes actually recorded (invisible-text smuggling).
+- A finding quote that sanitises to whitespace alone is rejected like one that
+  sanitises to nothing, closing the remaining trivially-satisfied verbatim
+  check.
+- A recorder that finalised cleanly right at the shutdown grace boundary is no
+  longer misclassified as force-stopped — the flag now reflects whether the
+  escalation SIGKILL actually terminated it — so a complete, playable recording
+  is not reported as truncated with a spurious failure exit.
+- Device enumeration no longer relies on the enumeration child being killable:
+  a child that survives SIGKILL (a wedged kernel driver) is abandoned with an
+  actionable error instead of hanging `record` forever, and a listing that
+  completed just as the deadline fired is used rather than discarded. Both
+  enumeration paths are now covered by hermetic tests against a fake `ffmpeg`,
+  as are the force-stop classification, the derived-offset bound, the
+  missing-vs-unreadable audio split, the report code-span escape, the review
+  error-path sanitisation, and the atomic-conversion call-site wiring — fixes
+  whose tests previously stayed green when the fix was reverted.
+- A converted `audio.wav` is written with the operator's umask-masked mode, like
+  every other session artefact and the record-side `audio.wav`, rather than the
+  temp file's private `0600` or a flat `0644` wider than the umask allows.
+- The recorder shutdown no longer hangs on a wedged capture device: the wait
+  after the escalation `SIGKILL` is now bounded, so a child pinned in an
+  uninterruptible kernel wait is abandoned (and its artefact distrusted) instead
+  of stalling the whole sequential shutdown before `record` can finalise its
+  outputs and print the follow-up commands.
+- Device enumeration survives an unresponsive child rather than hanging on it:
+  the wait is structured so an expired deadline always takes effect even when the
+  child cannot be reaped, closing a residual hang the first timeout could not.
+- The bounded enumeration-output sink honours the `io.Writer` contract on
+  overflow, so a flood past its cap can no longer abort the capture of the
+  listing partway through.
+
+### Changed
+
+- **Behaviour:** `analyze -ingest` and `analyze -out` now require a regular file
+  and reject named pipes, shell process substitution (`-ingest <(…)`), and
+  `/dev/stdout`. Use `-ingest -` to read the answer from stdin, and omit `-out` to
+  write the request to stdout — the supported equivalents.
 
 ## [0.3.0] - 2026-07-22
 
@@ -131,6 +231,7 @@ Resource and process lifecycle:
 - A one-line installer and a checksummed release of static binaries for macOS
   and Linux.
 
-[Unreleased]: https://github.com/REPPL/Testimony/compare/v0.2.0...HEAD
+[0.4.0]: https://github.com/REPPL/Testimony/compare/v0.3.0...v0.4.0
+[0.3.0]: https://github.com/REPPL/Testimony/compare/v0.2.0...v0.3.0
 [0.2.0]: https://github.com/REPPL/Testimony/compare/v0.1.0...v0.2.0
 [0.1.0]: https://github.com/REPPL/Testimony/releases/tag/v0.1.0

docs/reference/session-directory.md

@@ -6,6 +6,7 @@ Every capture session lives in one directory (by default under `sessions/`):
 sessions/<timestamp>/
   manifest.json        # session metadata, including t0_epoch_ms (written by demo)
   audio.wav            # 16 kHz mono ASR input (written by transcribe; local only)
+  audio.offset.json    # audio→session offset for an external recording (written by transcribe; local only)
   events.rrweb.jsonl   # raw rrweb stream, archival (written by demo)
   interactions.jsonl   # normalised interaction events (written by demo)
   transcript.jsonl     # time-aligned utterances (written by transcribe)
@@ -74,6 +75,15 @@ One utterance per line. Times are session-relative seconds (audio time plus the
 {"id":"utt-003","t0":16.0,"t1":21.0,"speaker":"P1","text":"Now I expect this save button to confirm somehow.","words":[{"w":"Now","t":17.6},{"w":"I","t":17.92}]}
 ```
 
+## `audio.offset.json`
+
+Written by `transcribe` only when the audio came from an external recording (a `-audio FILE` that is not the session's own `audio.wav`), which is not captured at `t0`. It records the audio→session offset so a later bare `transcribe` (for example, a re-run with a different model that reuses `audio.wav`) recovers the same offset instead of assuming `0`. A session recorded with `testimony record` captures `audio.wav` at `t0` and has no sidecar; its offset is `0`. If the sidecar is present but unreadable or malformed, `transcribe` refuses rather than guess, and asks for an explicit `-audio` or `-offset`.
+
+| Field | Type | Required | Meaning |
+|---|---|---|---|
+| `offset_seconds` | number | yes | seconds added to every audio-clock time to place it on the session clock |
+| `provenance` | string | no | how the offset was obtained, for the operator |
+
 ## `events.rrweb.jsonl`
 
 One raw [rrweb](https://github.com/rrweb-io/rrweb) event per line, exactly as emitted by the recorder (DOM snapshots, incremental mutations, pointer movement). Archival only: nothing downstream reads it; it exists so full session replay stays possible later.

internal/analyze/analyze.go

@@ -16,7 +16,9 @@ import (
 	"bytes"
 	"encoding/json"
 	"fmt"
+	"io"
 	"path/filepath"
+	"reflect"
 	"regexp"
 
 	"github.com/REPPL/Testimony/internal/session"
@@ -92,11 +94,31 @@ func Load(dir string) ([]Finding, []Verdict, error) {
 		return nil, nil, err
 	}
 	defer f.Close()
+	return ParseRecords(f, path)
+}
 
+// ParseRecords splits a findings.jsonl stream into finding and verdict records,
+// applying the same rules Load documents: blank lines are skipped and a verdict
+// carrying an out-of-enum value is ignored rather than applied. name labels
+// errors. Load is ParseRecords over the on-disk file opened through the
+// no-follow guard; review.AppendVerdict reuses it to re-read the current
+// findings through its own already-locked descriptor, so the re-check and the
+// append observe the same file under one lock.
+func ParseRecords(r io.Reader, name string) ([]Finding, []Verdict, error) {
 	var findings []Finding
 	var verdicts []Verdict
-	sc := bufio.NewScanner(f)
-	sc.Buffer(make([]byte, 0, 64*1024), 4*1024*1024)
+	// Finding ids must be unique across the file. Ingest already rejects duplicates
+	// in an answer (validate), but a hand-edited or exchanged findings.jsonl can carry
+	// two findings sharing one id — and every id-keyed consumer then silently
+	// misbehaves: EffectiveStatus collapses them onto one status entry, so one verdict
+	// paints both findings in the report, and review's findByID only ever reaches the
+	// first. Refusing the file here, naming the offending line, turns that
+	// display-collapse into an honest error the operator can repair, and keeps the
+	// id-uniqueness EffectiveStatus/findByID assume true actually true on every path
+	// that loads findings.
+	seenID := map[string]bool{}
+	sc := bufio.NewScanner(r)
+	sc.Buffer(make([]byte, 0, 64*1024), session.MaxJSONLLine)
 	line := 0
 	for sc.Scan() {
 		line++
@@ -108,12 +130,12 @@ func Load(dir string) ([]Finding, []Verdict, error) {
 			Kind string `json:"kind"`
 		}
 		if err := json.Unmarshal(raw, &probe); err != nil {
-			return nil, nil, fmt.Errorf("%s:%d: %w", path, line, err)
+			return nil, nil, fmt.Errorf("%s:%d: %w", name, line, err)
 		}
 		if probe.Kind == "verdict" {
 			var v Verdict
 			if err := json.Unmarshal(raw, &v); err != nil {
-				return nil, nil, fmt.Errorf("%s:%d: %w", path, line, err)
+				return nil, nil, fmt.Errorf("%s:%d: %w", name, line, err)
 			}
 			// The verdict enum is closed (confirmed|rejected|duplicate). A verdict
 			// carrying any other value — a typo, an empty string, or a foreign
@@ -130,16 +152,31 @@ func Load(dir string) ([]Finding, []Verdict, error) {
 		}
 		var fnd Finding
 		if err := json.Unmarshal(raw, &fnd); err != nil {
-			return nil, nil, fmt.Errorf("%s:%d: %w", path, line, err)
+			return nil, nil, fmt.Errorf("%s:%d: %w", name, line, err)
+		}
+		if seenID[fnd.ID] {
+			return nil, nil, fmt.Errorf("%s:%d: duplicate finding id %q; each finding must have a unique id", name, line, fnd.ID)
 		}
+		seenID[fnd.ID] = true
 		findings = append(findings, fnd)
 	}
 	if err := sc.Err(); err != nil {
-		return nil, nil, fmt.Errorf("%s: %w", path, err)
+		return nil, nil, fmt.Errorf("%s: %w", name, err)
 	}
 	return findings, verdicts, nil
 }
 
+// SameIdentity reports whether a and b are the same finding — equal in every
+// field a human verdict is recorded against. Status is excluded: it is the one
+// field a verdict is meant to change, and Ingest launders it to "unverified" on
+// every written finding regardless. review uses it under the append lock to
+// confirm a verdict still targets the finding the analyst was shown, rather than
+// a different finding a concurrent re-ingest slid under the same id.
+func SameIdentity(a, b Finding) bool {
+	a.Status, b.Status = "", ""
+	return reflect.DeepEqual(a, b)
+}
+
 // EffectiveStatus maps each finding id to its effective status: every finding
 // starts "unverified"; verdict records are applied in file order and the last
 // one for an id wins. This single helper is used by both review (to pick the