diff --git a/.abcd/work/DECISIONS.md b/.abcd/work/DECISIONS.md index 4903325..ed7365b 100644 --- a/.abcd/work/DECISIONS.md +++ b/.abcd/work/DECISIONS.md @@ -255,3 +255,16 @@ Architecture-shaping decisions graduate to an ADR under AGENTS.md current state). The /dev/null TTY-gate nitpick is recorded but unfixed (a true isatty check needs a dependency); the AGENTS.md fence's dangling brief link is an upstream abcd defect, not fixable here. +- 2026-07-29 — Bug-hunt round 9: four hunters, eight adversarial refuters + (two per dimension), 27 confirmed findings (11 substantive, 16 nitpick), + 3 candidates refuted, 1 skipped as recorded in round 8. Headlines: stray + positional arguments no longer silently swallow the flags after them; the + capture endpoint refuses records merge cannot read (shared validator); + the offset sidecar persists before the conversion's rename so no refusal + destroys a record-origin audio.wav; an unauthenticated gh falls back to + the verified checksum instead of a false provenance refusal; optional + dependency failures skip instead of aborting the installer; CHANGELOG + gains its Unreleased section; reference pages corrected against the code + (endpoint preconditions, evidence cap, sessionEnd bound, ffmpeg's real + consumers, the demo page's CDN fetch disclosed in privacy). Vendoring + rrweb (offline capture) needs a dependency decision — reported, not done. diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index ee551f1..5ee7c7f 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -220,9 +220,10 @@ jobs: dist/SHA256SUMS - name: Create the GitHub Release and upload the tarballs + SHA256SUMS - # --verify-tag: the tag must already exist (it triggered this run) and point - # at the checked-out commit. --generate-notes lists the merged PRs since the - # previous tag. + # --verify-tag: the tag must already exist in the remote (it triggered this + # run) — that is all gh checks; the moved-tag protection is the checkout of + # github.sha above, which pins what gets built regardless of where the tag + # points by now. --generate-notes lists the merged PRs since the previous tag. run: gh release create "$TAG" dist/testimony_*.tar.gz dist/SHA256SUMS --verify-tag --title "$TAG" --generate-notes env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} @@ -258,6 +259,12 @@ jobs: if: ${{ !cancelled() }} run: | set -euo pipefail + # This step runs under !cancelled(), so the env-recording step may have + # failed before writing these; report that instead of dying on set -u. + if [ -z "${DEFAULT_BRANCH_START_SHA:-}" ] || [ -z "${DEFAULT_BRANCH:-}" ]; then + echo "tripwire inconclusive: the default-branch tip was never recorded (an earlier step failed first)." + exit 0 + fi end="$(gh api "repos/${GITHUB_REPOSITORY}/git/ref/heads/${DEFAULT_BRANCH}" --jq '.object.sha')" if [ "$end" = "${DEFAULT_BRANCH_START_SHA}" ]; then echo "default branch unchanged across the release job (${end}); the job pushed nothing." diff --git a/CHANGELOG.md b/CHANGELOG.md index b6682cd..d3eb42e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,70 @@ leading `v`. Before v1.0.0, minor releases may make breaking changes; each one is called out in a **Breaking** section. +## [Unreleased] + +### Fixed + +Invocation contract (from the round-8 bug-hunt): + +- Every usage error now exits 2, as the CLI reference documents: a missing + required `-session`, a disallowed flag combination (`analyze -out` with + `-ingest`), and a non-finite `report -window` were reported at the runtime + status 1, indistinguishable to a script from a session that could not be + read. +- `report -window NaN`/`±Inf` is refused instead of silently rendering a + report whose events are detached from (or all filed under) the speech they + accompanied. +- `transcribe -audio` resolves the audio→session offset before the conversion + mutates the session, so a refused run no longer destroys a record-origin + `audio.wav`; an explicit `-offset` now rewrites an existing sidecar. +- `report` renders a hand-ordered (or hand-edited) `timeline.jsonl` in time + order instead of trusting the file's line order. +- `install.sh` installs the current release: its version pin had been left at + `v0.1.0`, handing new users a three-release-old binary, and the release + workflow now gates the pin against the tag so it cannot go stale again. The + installer also renders single-option prompts correctly and names the actual + cause when a release download fails. + +Invocation contract (round 9): + +- A stray positional argument is refused as a usage error; it used to be + silently swallowed together with every flag after it, so the command ran + with defaults at exit 0. +- The remaining invalid-flag-value paths exit 2: `review`'s + `-finding`/`-verdict` pairing and verdict syntax, an unknown + `transcribe -engine`, and a malformed capture `-addr` (which also no longer + creates a session directory before refusing). + +Capture integrity (round 9): + +- `POST /api/interactions` refuses with 400 any record `merge` would refuse — + a non-object body, or a missing/implausible `t` or missing `kind` — instead + of persisting with 204 a line that later fails the whole session's merge. +- A refused capture write and a deliberately wider (non-loopback) bind are + reported on stderr; the page posts via `sendBeacon`, so the terminal is the + only place a refusal can surface. +- The audio offset sidecar is persisted before the conversion's rename + replaces `audio.wav`, so a refused sidecar write leaves the session + byte-for-byte as it was found, and a failed rename rolls the sidecar back. +- A recorder that exits on its own mid-session still validates the artefacts + the other recorders left and prints the next-command block. + +Installer (round 9): + +- An unauthenticated (or attestation-incapable) `gh` no longer refuses the + install as a false provenance failure: it falls back to the verified + checksum, exactly like no `gh`; a verification `gh` actually performed and + rejected still refuses, and gh's own message is shown instead of being + swallowed. +- An optional-dependency failure (an unreachable ffmpeg or uv host, a failed + unpack or brew install) skips its step with guidance instead of aborting + the whole installer with the child's raw exit code and a leaked temp + directory; Ctrl+C stops the installer instead of being read as "skip"; + `--help` works through the documented pipe invocation; `--dir`/`--version` + without a value are refused cleanly; the whisper.cpp model recipe + downloads into a directory `-model NAME` actually searches. + ## [0.4.0] - 2026-07-24 A second robustness pass over the same capture → analysis pipeline, closing the @@ -231,6 +295,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.4.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 diff --git a/README.md b/README.md index 7971fd6..6937395 100644 --- a/README.md +++ b/README.md @@ -20,16 +20,18 @@ Raw audio and video never leave your machine; only derived text is analysed. See ## Install One line, no admin rights required (binary goes to `~/.local/bin`; the download -is verified against the release's published checksums and, when the GitHub CLI -is installed, its SLSA build-provenance attestation): +is verified against the release's published checksums and, when an +authenticated GitHub CLI (`gh`) is available, its SLSA build-provenance +attestation): ```sh curl -fsSL https://raw.githubusercontent.com/REPPL/Testimony/main/install.sh | sh ``` -The installer then offers to set up what `transcribe` needs — ffmpeg and a local -ASR engine (WhisperX or whisper.cpp) — via Homebrew where available, or as -user-local installs for machines without admin rights. Prefer to read before you +The installer then offers to set up the pipeline's two dependencies — ffmpeg, +which `record`'s capture (and `transcribe -audio`'s conversion) needs, and a +local ASR engine (WhisperX or whisper.cpp) for `transcribe` — via Homebrew +where available, or as user-local installs for machines without admin rights. Prefer to read before you run (sensible), or pass flags: ```sh @@ -84,7 +86,9 @@ Each session is one folder of small, inspectable files: ``` sessions// manifest.json # app, participant, tasks, t0_epoch_ms (the shared clock anchor) - audio.wav # 16 kHz mono extract (local only) + audio.wav # 16 kHz mono ASR input, captured or converted (local only) + audio.offset.json # audio→session offset for an external recording (local only) + screen.mp4 # screen capture, with record -video (local only) events.rrweb.jsonl # raw rrweb stream (archival) interactions.jsonl # normalised interaction events transcript.jsonl # time-aligned utterances diff --git a/docs/explanation/how-alignment-works.md b/docs/explanation/how-alignment-works.md index f77d19e..bd5f2d9 100644 --- a/docs/explanation/how-alignment-works.md +++ b/docs/explanation/how-alignment-works.md @@ -16,7 +16,7 @@ The voice recording is the awkward stream. A transcription engine reports times session_time = audio_time + offset ``` -`testimony transcribe` derives the offset automatically when it can: it reads the recording's embedded creation timestamp (the `creation_time` tag recorders such as QuickTime write) and subtracts the manifest's `t0_epoch_ms`. If the tag is missing or unreadable, it falls back to 0 — correct whenever you start recording at the moment the session starts. Either way, it prints the offset and its provenance, so the value is never silent. +Which offset applies depends on where the audio came from. A session captured with `testimony record` needs none: capture starts at `t0`, so the offset is 0 by construction and no derivation is attempted. For an external recording (`transcribe -audio`), the offset is derived automatically when it can be: `transcribe` reads the recording's embedded creation timestamp (the `creation_time` tag recorders such as QuickTime write) and subtracts the manifest's `t0_epoch_ms`; if the tag is missing or unreadable, it falls back to 0 — correct whenever you start recording at the moment the session starts. The derived value is then persisted beside the audio in `audio.offset.json`, so a later re-run over the converted `audio.wav` reuses it instead of silently assuming 0. Whatever the path, the command prints the offset and its provenance, so the value is never silent. Derivation can be wrong — a recorder may stamp the wrong moment, or omit the tag. That is why sessions begin with a spoken marker: saying "session start" aloud at t0 plants a phrase that appears in the transcript at a known session time (roughly zero). If the report looks misaligned, the marker's transcript time reveals the true offset, and an explicit `-offset` overrides derivation entirely. A belt-and-braces anchor, recoverable after the fact. diff --git a/docs/explanation/privacy.md b/docs/explanation/privacy.md index e41e8fa..5083335 100644 --- a/docs/explanation/privacy.md +++ b/docs/explanation/privacy.md @@ -8,7 +8,7 @@ The rule is simple: **raw recordings stay local; only derived text is ever analy - Your voice recording and any screen recording remain files on your machine. No part of the pipeline uploads them anywhere. - Speech recognition runs locally — the transcription engines execute on your own hardware, and no part of your session is sent anywhere. The one network request transcription can make is the engine fetching its own model files from their publisher the first time it needs them; your recording plays no part in it, and once the models are on disk the step runs offline. -- The capture server listens on your machine and writes to a local session directory. +- The capture server listens on your machine and writes to a local session directory. One disclosure sits alongside it: the demo page loads its session-replay recorder (rrweb) from a public CDN, so the participant's browser makes one request to that third party — which thereby sees an IP address and the time of the session, though never any session content. Offline, or with the CDN blocked, the demo still works and still captures the interaction stream; only the archival replay stream stays empty. - What the pipeline produces for analysis is derived text: the transcript, the normalised event stream, the merged timeline, and the report. These are small, readable files you can inspect line by line before sharing them with anyone — or with any analysis tool. The distinction matters because the derived text is a much narrower disclosure than the recording it came from. A transcript contains what was said; the audio contains a voiceprint. An event stream says a button was clicked; a screen recording shows everything else that was visible at the time. When an analysis layer (local or cloud) enters the picture, it sits on the far side of this boundary: it sees only the text you choose to give it, never the raw audio or video. If your setting demands it, a fully local analysis path keeps even the derived text on the machine. diff --git a/docs/how-to/instrument-your-own-app.md b/docs/how-to/instrument-your-own-app.md index dcad005..31e1f85 100644 --- a/docs/how-to/instrument-your-own-app.md +++ b/docs/how-to/instrument-your-own-app.md @@ -15,7 +15,7 @@ This creates a fresh session directory (with `manifest.json` anchoring the sessi | `POST /api/interactions` | one normalised interaction (a single JSON object) | `interactions.jsonl` (one line per request) | | `POST /api/events` | a batch of raw rrweb events (a JSON array) | `events.rrweb.jsonl` (one line per array element) | -Both endpoints accept POST only (anything else returns 405) and return `204 No Content` on success. `/api/interactions` caps the body at 4 MiB — the readable JSONL line limit, since one request becomes one line — and rejects invalid JSON with 400; `/api/events` caps the batch body at 8 MiB and rejects anything that is not a JSON array with 400. A body over its cap gets 413, as does a batch element that would itself exceed the 4 MiB line limit. +Both endpoints accept POST only (anything else returns 405) and return `204 No Content` on success. `/api/interactions` caps the body at 4 MiB — the readable JSONL line limit, since one request becomes one line — and rejects with 400 anything the merge step could not read back: invalid JSON, a body that is not a JSON object, or an object missing the required `t` or `kind` from the table below; `/api/events` caps the batch body at 8 MiB and rejects anything that is not a JSON array with 400. A body over its cap gets 413, as does a batch element that would itself exceed the 4 MiB line limit. To defend the evidence against cross-origin forgery (CSRF) and DNS-rebinding, the write endpoints require `Content-Type: application/json`, a loopback `Host`, and — when present — a same-origin `Origin`. Post from your app's own origin (see the proxy in step 5) and always set the JSON content type, as the snippet below does. Each accepted body is re-encoded to a single line, so one request is always exactly one JSONL record. @@ -36,7 +36,7 @@ Each interaction is one JSON object. The fields the pipeline consumes: | Field | Type | Meaning | |---|---|---| | `t` | integer, **required** | event time in epoch milliseconds (`Date.now()`) | -| `kind` | string | event kind, e.g. `"click"` or `"input"` | +| `kind` | string, **required** | event kind, e.g. `"click"` or `"input"` | | `selector` | string | CSS-like anchor, ideally `[data-testid=...]` | | `text` | string | short human-readable label of the element | | `value` | string | new value for input events | diff --git a/docs/reference/cli.md b/docs/reference/cli.md index cd1bb94..5b54925 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -12,7 +12,7 @@ Running `testimony` with no command, or with an unknown command, prints the usag |---|---| | 0 | success | | 1 | runtime error — the message is printed to stderr as `testimony: ` | -| 2 | usage error — no command, an unknown command, an unparseable or invalid flag value, a missing required flag, or a flag combination that is not allowed | +| 2 | usage error — no command, an unknown command, a stray positional argument (no command takes one), an unparseable or invalid flag value, a missing required flag, or a flag combination that is not allowed | ## `testimony demo` @@ -27,12 +27,14 @@ testimony demo [-addr :8737] [-out sessions] | `-addr` | `:8737` | listen address (a bare `:port` binds loopback `127.0.0.1` only) | | `-out` | `sessions` | root directory for new session folders | -Behaviour: creates a new session directory named after the current time (`YYYY-MM-DD_HHMMSS`) under the `-out` root, writes `manifest.json` (participant `P1`, `t0_epoch_ms` set to now), serves the demo page at `/`, and appends captured events via two endpoints: +Behaviour: creates a new session directory named after the current time (`YYYY-MM-DD_HHMMSS`) under the `-out` root, writes `manifest.json` (app `testimony demo`, participant `P1`, one seeded default task, `t0_epoch_ms` set to now), serves the demo page at `/`, and appends captured events via two endpoints: - `POST /api/interactions` — one JSON object per request, appended as one line of `interactions.jsonl`. - `POST /api/events` — a JSON array per request, each element appended as one line of `events.rrweb.jsonl`. -Both accept POST only (405 otherwise), return 204 on success, and 400 on malformed bodies. `POST /api/interactions` limits the body to 4 MiB — the readable JSONL line limit, since one request becomes one line — and `POST /api/events` limits the batch body to 8 MiB; a body over its limit is refused with 413, as is a batch element that would itself exceed the 4 MiB line limit. The command blocks until interrupted (`Ctrl+C`). +Both accept POST only (405 otherwise) and require `Content-Type: application/json` (415 otherwise), a loopback `Host`, and — when an `Origin` header is present — a loopback origin (403 otherwise): the guard against cross-site and DNS-rebinding forgery of the unauthenticated write endpoints. They return 204 on success and 400 on malformed bodies; `/api/interactions` also refuses with 400 any record `merge` would refuse — a body that is not a JSON object, or one missing the required `t` (a positive epoch-millisecond time on a plausible session clock) or `kind`. `POST /api/interactions` limits the body to 4 MiB — the readable JSONL line limit, since one request becomes one line — and `POST /api/events` limits the batch body to 8 MiB; a body over its limit is refused with 413, as is a batch element that would itself exceed the 4 MiB line limit. Every refused capture write is logged to stderr, the operator's only signal, since the page posts via `sendBeacon`, which surfaces no status. The command blocks until interrupted (`Ctrl+C`). + +The loopback `Host` requirement means an explicit non-loopback `-addr` host (e.g. `0.0.0.0:8737`) serves the page to other devices but refuses their capture posts; the command warns at startup that such a bind serves the page only. ## `testimony transcribe` @@ -112,7 +114,7 @@ testimony record [-out sessions] [-app NAME] [-participant P1] [-commit HASH] | `-app` | *(empty)* | application under test; with `-demo`, defaults to the demo app | | `-participant` | `P1` | participant pseudonym | | `-commit` | *(empty)* | build/commit hash under test | -| `-task` | *(none)* | a task the participant will attempt; repeat the flag for several tasks | +| `-task` | *(none)* | a task the participant will attempt; repeat the flag for several tasks. With `-demo` and no `-task`, the demo app's default task is seeded, matching a standalone `demo` session | | `-video` | off | also capture the screen to `screen.mp4` (needs Screen Recording permission) | | `-no-video` | — | explicitly disable screen capture; this is the default, and it wins when both `-video` and `-no-video` are given | | `-demo` | off | also serve the instrumented demo app into the same session directory | @@ -120,7 +122,7 @@ testimony record [-out sessions] [-app NAME] [-participant P1] [-commit HASH] Behaviour: creates a new session directory named after the current time (`YYYY-MM-DD_HHMMSS`) under the `-out` root and writes `manifest.json` (app, participant, tasks, commit, `t0_epoch_ms` set to now) through the same code path as `demo`. On macOS it captures the default microphone to `audio.wav` (16 kHz mono PCM, the canonical ASR input — no re-conversion needed downstream) and, with `-video`, the screen to `screen.mp4`. Audio-only is the default; `-video` opts in. With `-demo` it also serves the demo app so one command captures voice and clicks into the same directory. -The command blocks until interrupted (`Ctrl+C`). On SIGINT/SIGTERM it sends each recorder an interrupt so it finalises its container, waits up to five seconds, and hard-kills only on timeout. It then validates each recorder's artefact — `audio.wav`, and `screen.mp4` with `-video` — and prints the exact next commands with the real session directory: with a usable `audio.wav` in place it offers `transcribe` → `merge` → `report` without `-audio`, because the recording is already present. +The command blocks until interrupted (`Ctrl+C`). On SIGINT/SIGTERM — and on SIGHUP, so closing the terminal window mid-session finalises exactly like Ctrl+C — it sends each recorder an interrupt so it finalises its container, waits up to five seconds, and hard-kills only on timeout. It then validates each recorder's artefact — `audio.wav`, and `screen.mp4` with `-video` — and prints the exact next commands with the real session directory: with a usable `audio.wav` in place it offers `transcribe` → `merge` → `report` without `-audio`, because the recording is already present. If a recorder leaves no usable artefact — most often because its macOS permission was never granted, so it blocked on the prompt and captured nothing — the command names the missing file, points at the exact System Settings pane (Privacy & Security → Microphone, or → Screen Recording), appends the recorder's output, and exits with status 1. When there is no `audio.wav`, the next-command block omits the bare `transcribe` line and instead keeps `merge` and `report` (interactions may still be captured) and explains how to get audio: re-run `record` after granting the permission, or transcribe an external recording with `-audio FILE`. A recorder that instead exits on its own before it is asked to stop is reported the same way, distinguishing a start-up permissions denial from an unexpected mid-session stop. On platforms other than macOS, capture is unavailable — the command still writes a valid manifest and session directory, states what was skipped, and exits 0. @@ -139,11 +141,11 @@ testimony analyze -session DIR -ingest FILE # validate the answer → find | `-out` | *(stdout)* | emit mode: write the request to `FILE` instead of stdout | | `-ingest` | *(off)* | ingest mode: validate the answer JSON at `FILE` (or `-` for stdin) into `findings.jsonl` | -`analyze` runs in exactly one mode: emit (no `-ingest`) or ingest (`-ingest`). Combining `-out` and `-ingest` is an error. Both modes read `manifest.json` and `timeline.jsonl`, hinting to run `merge` first when the timeline is missing. +`analyze` runs in exactly one mode: emit (no `-ingest`) or ingest (`-ingest`). Combining `-out` and `-ingest` is an error. Emit reads `manifest.json` and `timeline.jsonl`; ingest reads `timeline.jsonl` only. Both hint to run `merge` first when the timeline is missing. Emit behaviour: writes a single self-contained prompt — the rubric version header (`testimony-analysis/v1`), the second-coder stance, two-pass instructions (segment coding, then session synthesis), the rubric body (five `type` definitions, the `1..4` severity scale, the evidence hard-constraints), the session context (app, participant, tasks), the timeline lines inline, and the required output shape with a worked example. Nothing in the session directory is mutated. The timeline is emitted whole (v1 does not chunk by task boundary; the manifest carries no task timestamps). With `-out FILE` the prompt goes to a file and the command prints `wrote `; otherwise it prints to stdout. -Ingest behaviour: reads the answer from `FILE` (or stdin when `-`), accepting a top-level object with a `findings` array (optionally a `rubric`, which must be a known version) or a bare array. Ingest is the sole validation boundary and never trusts the model. Each finding is decoded with unknown fields disallowed, then checked against every schema rule (see [session directory reference](session-directory.md#findingsjsonl)): id format and uniqueness, `t` within the session, the `type` and `severity` enums, non-empty `evidence` with every id real and at least one spoken `utt-*` anchor, a `quote` that is a verbatim substring of one *cited* evidence utterance, and any `ui` selector/route matching a real event. Validation is transactional — all errors are reported at once and nothing is written on any failure. On success every finding is forced to `status: unverified`, `findings.jsonl` is written, and the command prints `validated N findings → (all unverified)`. An answer with no findings (a bare `[]`, `{"findings":[]}`, or a truncated file) is refused rather than written, so it cannot erase a prior `findings.jsonl`. Ingest refuses to overwrite a `findings.jsonl` that already holds verdict records — counting any `kind:"verdict"` line, even one whose value is outside the closed enum. +Ingest behaviour: reads the answer from `FILE` (or stdin when `-`), accepting a top-level object with a `findings` array (optionally a `rubric`, which must be a known version) or a bare array. Ingest is the sole validation boundary and never trusts the model. Each finding is decoded with unknown fields disallowed, then checked against every schema rule (see [session directory reference](session-directory.md#findingsjsonl)): id format and uniqueness, `t` within the session, the `type` and `severity` enums, non-empty `evidence` of at most 64 ids with every id real and at least one spoken `utt-*` anchor, a `quote` that is a verbatim substring of one *cited* evidence utterance, and any `ui` selector/route matching a real event. Validation is transactional — all errors are reported at once and nothing is written on any failure. On success every finding is forced to `status: unverified`, `findings.jsonl` is written, and the command prints `validated N findings → (all unverified)`. An answer with no findings (a bare `[]`, `{"findings":[]}`, or a truncated file) is refused rather than written, so it cannot erase a prior `findings.jsonl`. Ingest refuses to overwrite a `findings.jsonl` that already holds verdict records — counting any `kind:"verdict"` line, even one whose value is outside the closed enum. ## `testimony review` diff --git a/docs/reference/session-directory.md b/docs/reference/session-directory.md index 473b344..879322e 100644 --- a/docs/reference/session-directory.md +++ b/docs/reference/session-directory.md @@ -87,7 +87,7 @@ Written by `transcribe` only when the audio came from an external recording (a ` ## `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. +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. The demo page loads the rrweb recorder from a public CDN, so on a machine without network access (or with the CDN blocked) the file is created but stays empty — the session still captures `interactions.jsonl`, which carries the evidence the pipeline consumes. ## `timeline.jsonl` @@ -106,7 +106,7 @@ Event payload (`src: "event"`): `kind`, plus `selector`, `text`, `value`, and `r ```json {"t":19.2,"src":"event","id":"ev-003","payload":{"kind":"click","route":"#general","selector":"[data-testid=save-btn]","text":"Save"}} -{"t":16,"src":"speech","id":"utt-003","payload":{"speaker":"P1","t1":21,"text":"Now I expect this save button to confirm somehow."}} +{"t":16,"src":"speech","id":"utt-003","payload":{"speaker":"P1","t1":21,"text":"Now I expect this save button to confirm somehow.","words":[{"w":"Now","t":17.6},{"w":"I","t":17.92}]}} ``` ## `findings.jsonl` @@ -120,12 +120,12 @@ Ingest validates every finding against the merged timeline and is the sole valid | Field | Type | Required | Meaning | |---|---|---|---| | `id` | string | yes | `F-NNN`, zero-padded (`^F-\d{3}$`); unique within the file | -| `t` | number | yes | finding time, session-relative seconds; within `[sessionStart, sessionEnd]` (the earliest and latest timeline entry times; `sessionStart` is `0` unless the timeline holds negative-time utterances from a recording predating `t0`) | +| `t` | number | yes | finding time, session-relative seconds; within `[sessionStart, sessionEnd]` (`sessionStart` is `0` unless the timeline holds negative-time utterances from a recording predating `t0`; `sessionEnd` is the latest moment on the timeline — the end (`t1`) of the last utterance when speech closes the session, else the latest entry time) | | `type` | string | yes | one of `bug`, `friction`, `inconsistency`, `preference`, `idea` | | `severity` | integer | yes | usability-severity scale `1..4`: cosmetic, minor, major, blocker | | `mode` | string | no | `A` or `B`, default `A`; only Mode A is produced today | | `quote` | string | yes | a verbatim substring of the `text` of one *cited* evidence utterance — no normalisation, never joined across utterances | -| `evidence` | array of strings | yes | non-empty; every id exists in `timeline.jsonl`; at least one `utt-*` (a spoken anchor) | +| `evidence` | array of strings | yes | non-empty, at most 64 ids; every id exists in `timeline.jsonl`; at least one `utt-*` (a spoken anchor) | | `ui` | object | no | `{selector?, route?}`; when present, each must match a real timeline event's `selector`/`route` | | `status` | string | yes | always `"unverified"` on ingest | diff --git a/docs/tutorials/getting-started.md b/docs/tutorials/getting-started.md index 2cde3e1..6fbc3b1 100644 --- a/docs/tutorials/getting-started.md +++ b/docs/tutorials/getting-started.md @@ -23,8 +23,12 @@ needs ffmpeg. The one yes/no confirmation inside the whisperx setup (installing its `uv` tool) is the exception: there `y` accepts. The installer verifies the `testimony` binary against the release's published -`SHA256SUMS` and, when the GitHub CLI (`gh`) is installed, against its SLSA -build-provenance attestation too, refusing to install on a mismatch. The +`SHA256SUMS` and, when an authenticated GitHub CLI (`gh`) is available, against +its SLSA build-provenance attestation too, refusing to install whenever that +verification does not confirm the build. A `gh` that cannot attempt the verification — +not authenticated, or too old to know attestations — is told apart from a +failed verification: the installer says so and proceeds on the checksum alone, +exactly as it does with no `gh` at all. The dependencies carry weaker guarantees: the **local** ffmpeg build for macOS is checked against a pinned publisher key only when `gpg` is on your machine, and it warns you when it installs one unverified; `uv` and whisperx are fetched from diff --git a/install.sh b/install.sh index 98cc7cc..85e5c5d 100644 --- a/install.sh +++ b/install.sh @@ -21,13 +21,15 @@ # # Trust model. The binary install downloads the platform tarball AND the release's # published SHA256SUMS, and verifies the tarball against it (integrity — the bytes -# are exactly what the release published). When `gh` (the GitHub CLI) is available -# it ALSO runs `gh attestation verify` against the release workflow's SLSA -# build-provenance (authenticity — cryptographic proof the tarball was built by -# REPPL/Testimony's own release.yml, the strong anchor). Without gh it proceeds on -# the checksum alone and prints a note that installing gh enables provenance -# verification. No per-release hash is pinned in this script: the checksums are -# fetched from the release itself and the attestation binds them to the workflow. +# are exactly what the release published). When an AUTHENTICATED `gh` (the GitHub +# CLI) is available it ALSO runs `gh attestation verify` against the release +# workflow's SLSA build-provenance (authenticity — cryptographic proof the tarball +# was built by REPPL/Testimony's own release.yml, the strong anchor). A gh that +# cannot attempt the verification — absent, unauthenticated, or too old to know +# attestations — means the install proceeds on the checksum alone, with a note +# saying so; only a verification gh performs and rejects refuses the install. +# No per-release hash is pinned in this script: the checksums are fetched from +# the release itself and the attestation binds them to the workflow. # Everything installs into user-owned locations by default; sudo is never invoked. set -eu @@ -116,7 +118,14 @@ install_binary() { base="https://github.com/$REPO/releases/download/$VERSION" tmp=$(mktemp -d "${TMPDIR:-/tmp}/testimony-install.XXXXXX") - trap 'rm -rf "$tmp"' EXIT INT TERM + # Both traps sweep every mktemp dir the script can hold ($tmp here; $tmp2, + # $gnupg, $uvd in the dependency stage — unset expansions vanish), so an + # interrupt mid-dependency leaks nothing. A caught INT/TERM must also STOP + # the script: a trap that only cleans up returns control after the handler, + # so Ctrl+C at a dependency prompt read was swallowed into the safe-default + # answer and the run carried on. + trap 'rm -rf ${tmp:+"$tmp"} ${tmp2:+"$tmp2"} ${gnupg:+"$gnupg"} ${uvd:+"$uvd"}' EXIT + trap 'rm -rf ${tmp:+"$tmp"} ${tmp2:+"$tmp2"} ${gnupg:+"$gnupg"} ${uvd:+"$uvd"}; trap - EXIT; exit 130' INT TERM say "Downloading $tarball ..." # A bad --version (or a platform the release never published) surfaces from @@ -131,7 +140,7 @@ install_binary() { # Integrity: verify the tarball against the release's published SHA256SUMS. # No hash is pinned in this script — it is fetched from the release itself. say "Downloading SHA256SUMS ..." - fetch "$base/SHA256SUMS" "$tmp/SHA256SUMS" + fetch "$base/SHA256SUMS" "$tmp/SHA256SUMS" || die "could not download SHA256SUMS from $base" want=$(awk -v f="$tarball" '$2 == f {print $1}' "$tmp/SHA256SUMS") [ -n "$want" ] || die "no entry for $tarball in SHA256SUMS" got=$(sha256_of "$tmp/$tarball") @@ -147,13 +156,31 @@ Refusing to install." # (which an attacker who replaced BOTH the tarball and SHA256SUMS could forge). # --signer-workflow binds acceptance to release.yml specifically, not any # workflow in the repo. Without gh, proceed on the checksum with a printed note. - if have gh; then + # + # A gh that CANNOT ATTEMPT the verification is the checksum-only case, not a + # provenance failure: `gh attestation verify` refuses to run unauthenticated + # (exit 4, before any verification), and a gh predating the attestation + # command or --signer-workflow fails the same way — treating those as + # "attestation FAILED" made a freshly `brew install`ed gh strictly worse + # than no gh at all, refusing a tarball whose checksum had just verified. + # Only a verification gh actually performed and rejected refuses the + # install, and gh's own output is shown instead of being swallowed. + if have gh && ! gh auth status --hostname github.com >/dev/null 2>&1; then + say "NOTE: 'gh' is installed but not authenticated — installed on the checksum alone." + say " 'gh attestation verify' needs an authenticated gh; run 'gh auth login'" + say " (or set GH_TOKEN) and re-run to also verify SLSA build-provenance." + elif have gh; then say "Verifying SLSA build-provenance attestation with gh ..." if gh attestation verify "$tmp/$tarball" \ --repo "$REPO" \ - --signer-workflow "$REPO/.github/workflows/release.yml" >/dev/null 2>&1; then + --signer-workflow "$REPO/.github/workflows/release.yml" >"$tmp/attest.log" 2>&1; then say "Provenance verified: built by $REPO/.github/workflows/release.yml" + elif grep -qiE 'unknown (command|flag)' "$tmp/attest.log"; then + say "NOTE: this gh cannot verify attestations (it lacks 'attestation verify" + say " --signer-workflow') — installed on the checksum alone. Update gh" + say " (https://cli.github.com) and re-run to also verify build provenance." else + sed 's/^/ gh: /' "$tmp/attest.log" >&2 die "attestation verification FAILED for $tarball gh could not confirm this tarball was built by $REPO's release workflow. Refusing to install." @@ -178,21 +205,24 @@ Refusing to install." } # --- dependencies ----------------------------------------------------------- -# transcribe needs: ffmpeg, plus one ASR engine (WhisperX preferred, whisper.cpp -# works too). demo/merge/report need nothing. Local options never require admin -# rights; brew needs a Homebrew install but not sudo on default setups. +# record's capture needs ffmpeg (so does transcribe -audio's conversion; a bare +# transcribe needs none), and transcribe needs one ASR engine (WhisperX +# preferred, whisper.cpp works too). demo/merge/report need nothing. Local +# options never require admin rights; brew needs a Homebrew install but not +# sudo on default setups. dep_ffmpeg() { if have ffmpeg; then say "ffmpeg: already installed ($(command -v ffmpeg))"; return; fi say "" - say "ffmpeg is required by 'testimony transcribe' (audio conversion)." + say "ffmpeg is required by 'testimony record' (audio/screen capture) and by" + say "'testimony transcribe -audio' (converting an external recording)." if have brew; then c=$(choose "Install ffmpeg via" "brew" "local") else c=$(choose "Install ffmpeg (no Homebrew found)" "local" "local") fi case "$c" in - brew) brew install ffmpeg ;; + brew) brew install ffmpeg || err "brew install ffmpeg failed; skipping ffmpeg (later: brew install ffmpeg)" ;; local) install_ffmpeg_local ;; skip) say "Skipped. Later: brew install ffmpeg (or re-run this installer)" ;; esac @@ -207,13 +237,22 @@ install_ffmpeg_local() { # evermeet.cx publishes a GPG signature (.sig) per build; verify it # against the PINNED publisher key ($EVERMEET_FPR) when gpg is # available, and refuse on a bad or wrong-key signature. + # Every fetch/unpack below is guarded with the err-skip-return + # convention the parse failure already uses: ffmpeg is an OPTIONAL + # dependency, and under `set -eu` an unguarded failure aborted the + # whole installer with the child's raw exit code — skipping the ASR + # step and the closing guidance, and leaking $tmp2 (the EXIT trap + # covers only install_binary's $tmp). say "Fetching static ffmpeg build (evermeet.cx) ..." - fetch "https://evermeet.cx/ffmpeg/info/ffmpeg/release" "$tmp2/info.json" + fetch "https://evermeet.cx/ffmpeg/info/ffmpeg/release" "$tmp2/info.json" \ + || { err "could not reach evermeet.cx; skipping ffmpeg"; rm -rf "$tmp2"; return; } u=$(sed -n 's/.*"zip":{"url":"\([^"]*\)".*/\1/p' "$tmp2/info.json" | head -1) [ -n "$u" ] || { err "could not parse evermeet.cx response; skipping ffmpeg"; rm -rf "$tmp2"; return; } - fetch "$u" "$tmp2/ffmpeg.zip" + fetch "$u" "$tmp2/ffmpeg.zip" \ + || { err "ffmpeg download failed; skipping ffmpeg"; rm -rf "$tmp2"; return; } if have gpg; then - fetch "$u.sig" "$tmp2/ffmpeg.zip.sig" + fetch "$u.sig" "$tmp2/ffmpeg.zip.sig" \ + || { err "could not fetch the ffmpeg signature; refusing this unverifiable build"; rm -rf "$tmp2"; return; } # Import ONLY the pinned publisher key into a throwaway keyring, # then verify against it. --auto-key-retrieve is never used: it # would fetch whatever key the (attacker-supplied) signature @@ -237,14 +276,17 @@ install_ffmpeg_local() { say "WARNING: gpg not found — installing this ffmpeg build unverified" say " (its signature is at $u.sig)." fi - (cd "$tmp2" && unzip -q ffmpeg.zip) - install -m 0755 "$tmp2/ffmpeg" "$INSTALL_DIR/ffmpeg" + (cd "$tmp2" && unzip -q ffmpeg.zip) \ + || { err "could not unpack ffmpeg; skipping ffmpeg"; rm -rf "$tmp2"; return; } + install -m 0755 "$tmp2/ffmpeg" "$INSTALL_DIR/ffmpeg" \ + || { err "could not install ffmpeg into $INSTALL_DIR; skipping ffmpeg"; rm -rf "$tmp2"; return; } ;; linux) arch=$(uname -m) case "$arch" in x86_64) ja=amd64 ;; aarch64|arm64) ja=arm64 ;; *) err "no static ffmpeg for $arch"; rm -rf "$tmp2"; return ;; esac say "Fetching static ffmpeg build (johnvansickle.com) ..." - fetch "https://johnvansickle.com/ffmpeg/releases/ffmpeg-release-${ja}-static.tar.xz" "$tmp2/ffmpeg.tar.xz" + fetch "https://johnvansickle.com/ffmpeg/releases/ffmpeg-release-${ja}-static.tar.xz" "$tmp2/ffmpeg.tar.xz" \ + || { err "ffmpeg download failed; skipping ffmpeg"; rm -rf "$tmp2"; return; } fetch "https://johnvansickle.com/ffmpeg/releases/ffmpeg-release-${ja}-static.tar.xz.md5" "$tmp2/ffmpeg.md5" || true if [ -s "$tmp2/ffmpeg.md5" ] && have md5sum; then (cd "$tmp2" && sed 's| .*ffmpeg-release.*| ffmpeg.tar.xz|' ffmpeg.md5 | md5sum -c -) \ @@ -253,8 +295,10 @@ install_ffmpeg_local() { else say "WARNING: could not verify the static ffmpeg build; installing unverified." fi - tar -xJf "$tmp2/ffmpeg.tar.xz" -C "$tmp2" - install -m 0755 "$tmp2"/ffmpeg-*-static/ffmpeg "$INSTALL_DIR/ffmpeg" + tar -xJf "$tmp2/ffmpeg.tar.xz" -C "$tmp2" \ + || { err "could not unpack ffmpeg; skipping ffmpeg"; rm -rf "$tmp2"; return; } + install -m 0755 "$tmp2"/ffmpeg-*-static/ffmpeg "$INSTALL_DIR/ffmpeg" \ + || { err "could not install ffmpeg into $INSTALL_DIR; skipping ffmpeg"; rm -rf "$tmp2"; return; } ;; esac rm -rf "$tmp2" @@ -282,8 +326,10 @@ dep_asr() { # a local attacker on a shared host pre-plant a symlink or win # the write→exec race and run their own code as the user. uvd=$(mktemp -d "${TMPDIR:-/tmp}/testimony-uv.XXXXXX") - fetch "https://astral.sh/uv/install.sh" "$uvd/uv-install.sh" - sh "$uvd/uv-install.sh" + fetch "https://astral.sh/uv/install.sh" "$uvd/uv-install.sh" \ + || { err "could not download the uv installer; skipping whisperx (later: uv tool install whisperx)"; rm -rf "$uvd"; return; } + sh "$uvd/uv-install.sh" \ + || { err "uv installation failed; skipping whisperx (later: uv tool install whisperx)"; rm -rf "$uvd"; return; } rm -rf "$uvd" # uv lands in ~/.local/bin; make it visible to this run. PATH="$HOME/.local/bin:$PATH"; export PATH @@ -292,31 +338,56 @@ dep_asr() { return fi fi - uv tool install whisperx + uv tool install whisperx \ + || { err "whisperx installation failed; later: uv tool install whisperx (or: pipx install whisperx)"; return; } say "whisperx installed (user-local). First run downloads its models." ;; whisper.cpp) - brew install whisper-cpp + brew install whisper-cpp \ + || { err "brew install whisper-cpp failed; later: brew install whisper-cpp"; return; } say "" - say "whisper.cpp needs a ggml model. Download once (~1.5 GB), user-local:" - say " mkdir -p ~/.local/share/testimony/models && curl -fL -o ~/.local/share/testimony/models/ggml-large-v3-turbo.bin \\" + say "whisper.cpp needs a ggml model. Download once (~1.5 GB), user-local," + say "into a directory '-model NAME' searches:" + say " mkdir -p ~/.cache/whisper.cpp && curl -fL -o ~/.cache/whisper.cpp/ggml-large-v3-turbo.bin \\" say " https://huggingface.co/ggerganov/whisper.cpp/resolve/main/ggml-large-v3-turbo.bin" - say "Then: testimony transcribe -engine whispercpp -model ~/.local/share/testimony/models/ggml-large-v3-turbo.bin ..." + say "Then: testimony transcribe -engine whispercpp ... (-model large-v3-turbo is the default)" ;; skip) say "Skipped. Later: uv tool install whisperx or brew install whisper-cpp" ;; esac } -usage() { sed -n '2,20p' "$0" 2>/dev/null || say "see script header"; } +# The help text is embedded rather than sed-extracted from "$0": through the +# documented pipe invocation $0 is the shell's own argv[0] ("sh", or a path +# like /bin/sh whose bytes sed would happily print), not this script. +usage() { + cat <<'EOF' +testimony installer — https://github.com/REPPL/Testimony + +Usage (one line): + curl -fsSL https://raw.githubusercontent.com/REPPL/Testimony/main/install.sh | sh + +Passing flags through a pipe: + curl -fsSL .../install.sh | sh -s -- --yes --dir "$HOME/bin" + +Flags: + -d, --dir DIR install directory (default: ~/.local/bin, or $TESTIMONY_INSTALL_DIR + when set — no admin rights needed) + -y, --yes non-interactive: accept dependency installs (brew if present, + otherwise the local, admin-free option) + --no-deps install the binary only; print dependency guidance and exit + --version V install release V instead of the default + -h, --help this text +EOF +} main() { while [ $# -gt 0 ]; do case "$1" in - -d|--dir) INSTALL_DIR="$2"; shift 2 ;; + -d|--dir) [ $# -ge 2 ] || die "--dir needs a value (try --help)"; INSTALL_DIR="$2"; shift 2 ;; -y|--yes) ASSUME_YES=1; shift ;; --no-deps) NO_DEPS=1; shift ;; - --version) VERSION="$2"; shift 2 ;; + --version) [ $# -ge 2 ] || die "--version needs a value (try --help)"; VERSION="$2"; shift 2 ;; -h|--help) usage; exit 0 ;; *) die "unknown flag: $1 (try --help)" ;; esac @@ -327,7 +398,7 @@ main() { if [ "$NO_DEPS" = 1 ]; then say "" - say "Dependencies skipped (--no-deps). 'testimony transcribe' needs ffmpeg + whisperx or whisper.cpp." + say "Dependencies skipped (--no-deps). 'testimony record' needs ffmpeg; 'testimony transcribe' needs whisperx or whisper.cpp (and ffmpeg with -audio)." exit 0 fi diff --git a/internal/cli/cli.go b/internal/cli/cli.go index 497e25e..3563057 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -27,7 +27,7 @@ const usage = `testimony — usability evidence, on the record Usage: testimony record [-out sessions] [-app NAME] [-participant P1] [-task ...] managed capture: session dir + manifest, start recorders, run until Ctrl+C - [-video|-no-video] [-demo [-addr :8737]] + [-commit HASH] [-video|-no-video] [-demo [-addr :8737]] testimony demo [-addr :8737] [-out sessions] serve the instrumented demo app, capture a session testimony transcribe -session DIR [-audio FILE] transcribe a voice recording into transcript.jsonl (reuses the session's audio.wav when -audio is omitted) [-engine auto|whisperx|whispercpp] [-model large-v3-turbo] [-language en] [-offset SECONDS] @@ -39,6 +39,7 @@ Usage: testimony review -session DIR interactively record verdicts on unverified findings (TTY-gated) testimony review -session DIR -finding F-NNN -verdict confirmed|rejected|duplicate-of-F-NNN testimony version + testimony help A session directory is described in docs/reference/session-directory.md. ` @@ -57,6 +58,15 @@ func Run(args []string) int { addr := fs.String("addr", ":8737", "listen address") out := fs.String("out", "sessions", "root directory for new session folders") fs.Parse(rest) + if err := rejectArgs(fs); err != nil { + return usageErr(err) + } + // Refuse a malformed address here, where wrong invocations exit 2 — + // reported from Serve it took the runtime status, after Run had already + // created a session directory for a server that could never bind. + if err := demo.CheckAddr(*addr); err != nil { + return usageErr(fmt.Errorf("demo: %w", err)) + } if err := demo.Run(*addr, *out); err != nil { return fail(err) } @@ -66,6 +76,9 @@ func Run(args []string) int { fs := flag.NewFlagSet("merge", flag.ExitOnError) dir := fs.String("session", "", "session directory") fs.Parse(rest) + if err := rejectArgs(fs); err != nil { + return usageErr(err) + } if *dir == "" { return usageErr(fmt.Errorf("merge: -session is required")) } @@ -82,6 +95,9 @@ func Run(args []string) int { dir := fs.String("session", "", "session directory") window := fs.Float64("window", 2.5, "utterance↔event join window, seconds") fs.Parse(rest) + if err := rejectArgs(fs); err != nil { + return usageErr(err) + } if *dir == "" { return usageErr(fmt.Errorf("report: -session is required")) } @@ -119,6 +135,14 @@ func Run(args []string) int { demoFlag := fs.Bool("demo", false, "also serve the instrumented demo app into the session") addr := fs.String("addr", ":8737", "demo server listen address (with -demo)") fs.Parse(rest) + if err := rejectArgs(fs); err != nil { + return usageErr(err) + } + if *demoFlag { + if err := demo.CheckAddr(*addr); err != nil { + return usageErr(fmt.Errorf("record: %w", err)) + } + } if err := record.Run(record.Options{ Out: *out, App: *app, @@ -146,9 +170,18 @@ func Run(args []string) int { vad := fs.String("vad", "auto", "(whisperx) VAD method: auto, silero, or pyannote (auto picks silero; pyannote trips newer torch's weights_only load)") offset := fs.Float64("offset", 0, "audio→session clock offset in seconds (default: derived from the recording's creation time)") fs.Parse(rest) + if err := rejectArgs(fs); err != nil { + return usageErr(err) + } if *dir == "" { return usageErr(fmt.Errorf("transcribe: -session is required")) } + // An unknown engine name is a wrong invocation (exit 2) — reported from + // detectEngine it took the runtime status a script could not tell from a + // genuinely missing engine binary. + if err := transcribe.CheckEngine(*engine); err != nil { + return usageErr(fmt.Errorf("transcribe: %w", err)) + } offsetSet := false fs.Visit(func(f *flag.Flag) { if f.Name == "offset" { @@ -180,6 +213,9 @@ func Run(args []string) int { out := fs.String("out", "", "write the emitted request to FILE instead of stdout") ingest := fs.String("ingest", "", "validate answer JSON at FILE (or \"-\" for stdin) into findings.jsonl") fs.Parse(rest) + if err := rejectArgs(fs); err != nil { + return usageErr(err) + } if *dir == "" { return usageErr(fmt.Errorf("analyze: -session is required")) } @@ -234,13 +270,32 @@ func Run(args []string) int { finding := fs.String("finding", "", "non-interactive: the finding to judge (F-NNN)") verdict := fs.String("verdict", "", "non-interactive: confirmed | rejected | duplicate-of-F-NNN") fs.Parse(rest) + if err := rejectArgs(fs); err != nil { + return usageErr(err) + } if *dir == "" { return usageErr(fmt.Errorf("review: -session is required")) } + f, v := strings.TrimSpace(*finding), strings.TrimSpace(*verdict) + // The -finding/-verdict pairing and the verdict's syntax are invocation + // facts, so they are refused here at the usage status — reported from + // review.Run they exited 1, and only after the findings load, so a wrong + // flag on a session with no findings.jsonl was misreported as that. + if f != "" && v == "" { + return usageErr(fmt.Errorf("review: -verdict is required with -finding")) + } + if v != "" && f == "" { + return usageErr(fmt.Errorf("review: -finding is required with -verdict")) + } + if v != "" { + if _, _, err := review.ParseVerdictFlag(v); err != nil { + return usageErr(fmt.Errorf("review: %w", err)) + } + } if err := review.Run(review.Options{ Dir: *dir, - Finding: strings.TrimSpace(*finding), - Verdict: strings.TrimSpace(*verdict), + Finding: f, + Verdict: v, In: os.Stdin, Out: os.Stdout, IsTTY: isCharDevice(os.Stdin), @@ -251,10 +306,18 @@ func Run(args []string) int { return 0 case "version": + // version and help parse no flags, so the shared rejectArgs guard never + // sees their leftovers; the same no-positional contract applies. + if len(rest) > 0 { + return usageErr(fmt.Errorf("version: unexpected argument %q (the command takes no positional arguments)", rest[0])) + } fmt.Println("testimony", Version) return 0 case "help", "-h", "--help": + if len(rest) > 0 { + return usageErr(fmt.Errorf("help: unexpected argument %q (the command takes no positional arguments)", rest[0])) + } fmt.Print(usage) return 0 @@ -264,6 +327,18 @@ func Run(args []string) int { } } +// rejectArgs refuses leftover positional arguments after flag parsing. Flag +// parsing stops at the first non-flag argument, so a stray positional silently +// discarded every flag that followed it and the command ran with defaults at +// exit 0 — an invocation the operator never gave. No command takes positional +// arguments (docs/reference/cli.md), so a leftover is a usage error. +func rejectArgs(fs *flag.FlagSet) error { + if fs.NArg() > 0 { + return fmt.Errorf("%s: unexpected argument %q (the command takes no positional arguments)", fs.Name(), fs.Arg(0)) + } + return nil +} + // printErr writes an operator-facing error in the one shape every command uses. func printErr(err error) { fmt.Fprintln(os.Stderr, "testimony:", err) diff --git a/internal/cli/cli_test.go b/internal/cli/cli_test.go index 3722763..f5a9b78 100644 --- a/internal/cli/cli_test.go +++ b/internal/cli/cli_test.go @@ -132,6 +132,89 @@ func TestReportRejectsNonFiniteWindow(t *testing.T) { } } +// TestStrayPositionalIsAUsageError pins the other half of the invocation +// contract: no command takes positional arguments (docs/reference/cli.md), and +// flag parsing stops at the first non-flag argument, so a stray positional +// silently discarded every flag after it — `report -session S junk -window X` +// rendered with the default window at exit 0, and `transcribe ... recording.m4a +// -offset 99` dropped the operator's offset. A leftover argument must refuse +// the run as a usage error before any work starts. `demo` is exercised through +// the same shared guard but not run here: on the pre-fix path it blocks +// serving until interrupted. +func TestStrayPositionalIsAUsageError(t *testing.T) { + dir := miniSession(t) + cases := [][]string{ + {"merge", "-session", dir, "junk"}, + {"report", "-session", dir, "junk", "-window", "NaN"}, + {"transcribe", "-session", dir, "junk", "-offset", "99"}, + {"analyze", "-session", dir, "junk", "-out", "x", "-ingest", "-"}, + {"review", "-session", dir, "junk", "-finding", "F-001", "-verdict", "confirmed"}, + {"record", "-out", t.TempDir(), "junk", "-participant", "P9"}, + {"version", "junk"}, + {"help", "junk"}, + } + for _, args := range cases { + var code int + stderr := captureStderr(t, func() { code = Run(args) }) + if code != 2 { + t.Errorf("%v: exit %d, want 2 (usage error)", args, code) + } + if want := `unexpected argument "junk"`; !strings.Contains(stderr, want) { + t.Errorf("%v: want %q on stderr, got %q", args, want, stderr) + } + } + if _, err := os.Stat(filepath.Join(dir, session.ReportFile)); !os.IsNotExist(err) { + t.Errorf("report with a stray positional rendered a report anyway (err=%v)", err) + } +} + +// TestInvalidFlagValuesExitTwo pins exit 2 for the usage errors that were still +// reported at the runtime status: the -finding/-verdict pairing, an invalid +// -verdict value, an unknown -engine, and a malformed capture -addr. Reported +// from inside the packages they took exit 1, so a script could not tell a +// mistyped flag from a session that genuinely could not be read. Validation +// must also precede any work: `demo -addr bogus` used to create a session +// directory before refusing the address. +func TestInvalidFlagValuesExitTwo(t *testing.T) { + dir := miniSession(t) + demoOut := t.TempDir() + cases := []struct { + args []string + want string + }{ + {[]string{"review", "-session", dir, "-finding", "F-001"}, "review: -verdict is required with -finding"}, + {[]string{"review", "-session", dir, "-verdict", "confirmed"}, "review: -finding is required with -verdict"}, + {[]string{"review", "-session", dir, "-finding", "F-001", "-verdict", "bogus"}, `review: invalid verdict "bogus"`}, + {[]string{"transcribe", "-session", dir, "-engine", "bogus"}, `transcribe: unknown engine "bogus"`}, + {[]string{"demo", "-addr", "bogus", "-out", demoOut}, `demo: invalid capture address "bogus"`}, + {[]string{"record", "-demo", "-addr", "bogus", "-out", t.TempDir()}, `record: invalid capture address "bogus"`}, + } + for _, c := range cases { + var code int + stderr := captureStderr(t, func() { code = Run(c.args) }) + if code != 2 { + t.Errorf("%v: exit %d, want 2 (usage error)", c.args, code) + } + if !strings.Contains(stderr, c.want) { + t.Errorf("%v: want %q on stderr, got %q", c.args, c.want, stderr) + } + } + if entries, err := os.ReadDir(demoOut); err != nil || len(entries) != 0 { + t.Errorf("demo -addr bogus created a session directory before refusing (entries=%d, err=%v)", len(entries), err) + } +} + +// TestUsageListsEveryFlagAndCommand pins the top-level usage text against the +// documented invocation surface: record's -commit flag and the help command +// are part of docs/reference/cli.md but were absent from `testimony help`. +func TestUsageListsEveryFlagAndCommand(t *testing.T) { + for _, want := range []string{"-commit HASH", "testimony help"} { + if !strings.Contains(usage, want) { + t.Errorf("usage text does not mention %q", want) + } + } +} + // TestMissingSessionIsAUsageError pins the exit-status contract of // docs/reference/cli.md: a wrong invocation exits 2 and a runtime failure of a // well-formed command exits 1. A missing required -session was reported as a diff --git a/internal/demo/demo.go b/internal/demo/demo.go index dfa8909..da18362 100644 --- a/internal/demo/demo.go +++ b/internal/demo/demo.go @@ -23,6 +23,7 @@ import ( "time" "github.com/REPPL/Testimony/internal/session" + "github.com/REPPL/Testimony/internal/timeline" ) //go:embed assets/index.html @@ -32,6 +33,7 @@ type server struct { mu sync.Mutex interactions *os.File rawEvents *os.File + t0 int64 // manifest t0_epoch_ms, anchoring the interaction shape check } // DefaultApp is the app-under-test name a demo session records. @@ -133,6 +135,18 @@ func Serve(addr, dir string) (*http.Server, error) { if err != nil { return nil, err } + // The interaction shape check needs the manifest's t0 anchor, and every + // caller creates the session (and so the manifest) before serving into it. + // Loading it here also refuses a session whose anchor merge could never + // use, before any capture is accepted against it. + man, err := session.LoadManifest(dir) + if err != nil { + return nil, err + } + t0, err := man.T0() + if err != nil { + return nil, err + } open := func(name string) (*os.File, error) { return session.OpenFileNoFollow(filepath.Join(dir, name), os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0o644) } @@ -145,7 +159,7 @@ func Serve(addr, dir string) (*http.Server, error) { inter.Close() return nil, err } - s := &server{interactions: inter, rawEvents: raw} + s := &server{interactions: inter, rawEvents: raw, t0: t0} mux := http.NewServeMux() mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { @@ -162,6 +176,16 @@ func Serve(addr, dir string) (*http.Server, error) { raw.Close() return nil, err } + // A deliberately wider bind serves the page to other devices, but allowWrite + // still pins capture posts to loopback clients — lifting that pin would + // reopen the CSRF/DNS-rebinding surface the guard exists for. The operator + // must hear the consequence up front: their clients post via sendBeacon, + // which surfaces no status to the page, so without this line the first + // signal that a remote participant's session recorded nothing was merge + // counting 0 events. + if !loopbackHost(bind) { + fmt.Fprintf(os.Stderr, "testimony demo: warning: bound to %s, but capture posts are accepted from loopback clients only — a page opened from another device is served yet records nothing\n", bind) + } // The two stream files use direct O_APPEND writes (no buffering), so their // data is durable without an explicit Close; the OS reclaims them on exit, // as before. Not closing them on Shutdown avoids racing an in-flight write. @@ -189,7 +213,7 @@ func (s *server) handleRawEvents(w http.ResponseWriter, r *http.Request) { func (s *server) appendLines(w http.ResponseWriter, r *http.Request, f *os.File, batch bool) { if r.Method != http.MethodPost { - http.Error(w, "POST only", http.StatusMethodNotAllowed) + refuseWrite(w, r, "method "+r.Method, "POST only", http.StatusMethodNotAllowed) return } if !allowWrite(w, r) { @@ -209,11 +233,11 @@ func (s *server) appendLines(w http.ResponseWriter, r *http.Request, f *os.File, } body, err := io.ReadAll(io.LimitReader(r.Body, maxBody+1)) if err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + refuseWrite(w, r, fmt.Sprintf("unreadable body: %v", err), err.Error(), http.StatusBadRequest) return } if int64(len(body)) > maxBody { - http.Error(w, "request body too large", http.StatusRequestEntityTooLarge) + refuseWrite(w, r, "body over the size limit", "request body too large", http.StatusRequestEntityTooLarge) return } @@ -221,17 +245,17 @@ func (s *server) appendLines(w http.ResponseWriter, r *http.Request, f *os.File, if batch { var msgs []json.RawMessage if err := json.Unmarshal(body, &msgs); err != nil { - http.Error(w, "expected JSON array", http.StatusBadRequest) + refuseWrite(w, r, "body is not a JSON array", "expected JSON array", http.StatusBadRequest) return } for _, m := range msgs { line, err := compactLine(m) if err != nil { - http.Error(w, "invalid JSON", http.StatusBadRequest) + refuseWrite(w, r, "invalid JSON in a batch element", "invalid JSON", http.StatusBadRequest) return } if tooLongForJSONL(line) { - http.Error(w, "record exceeds the readable JSONL line limit", http.StatusRequestEntityTooLarge) + refuseWrite(w, r, "batch element over the JSONL line limit", "record exceeds the readable JSONL line limit", http.StatusRequestEntityTooLarge) return } lines = append(lines, line) @@ -239,11 +263,23 @@ func (s *server) appendLines(w http.ResponseWriter, r *http.Request, f *os.File, } else { line, err := compactLine(body) if err != nil { - http.Error(w, "invalid JSON", http.StatusBadRequest) + refuseWrite(w, r, "invalid JSON", "invalid JSON", http.StatusBadRequest) return } + // The write side must respect the read side's shape invariant too, not + // just its line-length one: merge refuses an interactions.jsonl record + // that is not an object carrying the required t and kind, so accepting + // one here (204) would durably persist a line that fails the whole + // session's merge after the participant has gone. This is the single- + // record interaction path only; a raw-event batch is archival and no + // reader constrains its element shape. if tooLongForJSONL(line) { - http.Error(w, "record exceeds the readable JSONL line limit", http.StatusRequestEntityTooLarge) + refuseWrite(w, r, "record over the JSONL line limit", "record exceeds the readable JSONL line limit", http.StatusRequestEntityTooLarge) + return + } + if err := timeline.CheckInteraction(line, s.t0); err != nil { + msg := fmt.Sprintf("interaction %v", err) + refuseWrite(w, r, msg, msg, http.StatusBadRequest) return } lines = append(lines, line) @@ -327,23 +363,34 @@ func tooLongForJSONL(line []byte) bool { // returns false when the request must be refused. func allowWrite(w http.ResponseWriter, r *http.Request) bool { if !loopbackHost(r.Host) { - http.Error(w, "unexpected Host", http.StatusForbidden) + refuseWrite(w, r, fmt.Sprintf("unexpected Host %q", r.Host), "unexpected Host", http.StatusForbidden) return false } if o := r.Header.Get("Origin"); o != "" { u, err := url.Parse(o) if err != nil || !loopbackHost(u.Host) { - http.Error(w, "cross-origin request rejected", http.StatusForbidden) + refuseWrite(w, r, fmt.Sprintf("cross-origin Origin %q", o), "cross-origin request rejected", http.StatusForbidden) return false } } if !isJSONContentType(r.Header.Get("Content-Type")) { - http.Error(w, "Content-Type must be application/json", http.StatusUnsupportedMediaType) + refuseWrite(w, r, fmt.Sprintf("Content-Type %q", r.Header.Get("Content-Type")), "Content-Type must be application/json", http.StatusUnsupportedMediaType) return false } return true } +// refuseWrite answers a refused capture request and logs the refusal to the +// operator's terminal, for the same reason a failed persist is logged: the +// page posts via sendBeacon, which cannot report a status back, so stderr is +// the only signal that capture posts are being refused — by the forgery +// guard, a malformed or mis-shaped record, or an over-long body. Every +// refusal path answers through this one helper so none can go silent again. +func refuseWrite(w http.ResponseWriter, r *http.Request, reason, status string, code int) { + fmt.Fprintf(os.Stderr, "testimony demo: capture write refused (%s) from %s\n", reason, r.RemoteAddr) + http.Error(w, status, code) +} + // loopbackHost reports whether hostport names the local machine: the literal // "localhost" or any loopback IP. Used to pin the Host/Origin to loopback. func loopbackHost(hostport string) bool { @@ -385,11 +432,21 @@ func DisplayURL(addr string) string { return "http://" + net.JoinHostPort(host, port) } +// CheckAddr validates a capture listen address without binding it, so the CLI +// can refuse a malformed -addr as a usage error before any session directory +// is created. Serve keeps applying the same rule when it binds. +func CheckAddr(addr string) error { + _, err := listenAddr(addr) + return err +} + // listenAddr binds the capture server to loopback by default: a bare ":8737" // (empty host) becomes "127.0.0.1:8737", so the unauthenticated write endpoints // are not published to the LAN even though the banner prints "localhost". An -// operator who deliberately wants a wider bind can still pass an explicit host -// (e.g. "0.0.0.0:8737"). +// operator who deliberately passes an explicit host (e.g. "0.0.0.0:8737") gets +// the wider bind, but it serves the PAGE only: allowWrite keeps refusing +// capture posts from non-loopback clients, and Serve warns about exactly that +// at startup. // // An addr that does not parse into host and port is refused outright rather // than passed through to net.Listen. Passing it through defeated the very diff --git a/internal/demo/demo_test.go b/internal/demo/demo_test.go index 4679cd3..3b631a3 100644 --- a/internal/demo/demo_test.go +++ b/internal/demo/demo_test.go @@ -2,6 +2,7 @@ package demo import ( "errors" + "io" "net/http" "net/http/httptest" "os" @@ -12,6 +13,31 @@ import ( "github.com/REPPL/Testimony/internal/session" ) +// captureStderr runs fn with os.Stderr redirected to a pipe and returns +// everything it wrote there, so a test can assert on the operator-facing +// signal the capture server emits (its clients post via sendBeacon, so stderr +// is the only place a refusal ever surfaces). +func captureStderr(t *testing.T, fn func()) string { + t.Helper() + r, w, err := os.Pipe() + if err != nil { + t.Fatalf("pipe: %v", err) + } + old := os.Stderr + os.Stderr = w + read := make(chan string, 1) + go func() { + b, _ := io.ReadAll(r) + read <- string(b) + }() + fn() + os.Stderr = old + w.Close() + got := <-read + r.Close() + return got +} + // newTestServer builds a server writing into a fresh temp session directory, // mirroring Serve's stream files. func newTestServer(t *testing.T) (*server, string) { @@ -27,7 +53,20 @@ func newTestServer(t *testing.T) (*server, string) { inter := open(session.InteractionsFile) raw := open(session.RawEventsFile) t.Cleanup(func() { inter.Close(); raw.Close() }) - return &server{interactions: inter, rawEvents: raw}, dir + // t0 anchors the interaction shape check; 1 keeps the toy `"t":1` records + // these tests post at a session-relative time of 0. + return &server{interactions: inter, rawEvents: raw, t0: 1}, dir +} + +// manifestDir builds a temp session directory holding a manifest with a usable +// t0 anchor, which Serve now loads for the interaction shape check. +func manifestDir(t *testing.T) string { + t.Helper() + dir := t.TempDir() + if err := session.SaveManifest(dir, session.Manifest{Session: "s", App: "a", Participant: "P1", T0EpochMS: 1}); err != nil { + t.Fatalf("SaveManifest: %v", err) + } + return dir } // jsonPost builds a POST that passes the loopback/JSON guard by default; hdr @@ -120,7 +159,7 @@ func TestServeRefusesUnparseableAddr(t *testing.T) { // it alive for ever and the shutdown waited on it, leaving 'testimony record' // hanging after Ctrl+C instead of finalising the session. func TestServeBoundsRequestTimeouts(t *testing.T) { - srv, err := Serve(":0", t.TempDir()) + srv, err := Serve(":0", manifestDir(t)) if err != nil { t.Fatalf("Serve: %v", err) } @@ -324,6 +363,104 @@ func TestWriteEndpointGuard(t *testing.T) { } } +// TestWiderBindWarnsCaptureStaysLoopback pins the operator signal for the +// advertised wider bind: an explicit non-loopback host serves the page to +// other devices, but allowWrite still pins capture posts to loopback clients, +// so every remote post is refused and both streams stay empty. Pre-fix nothing +// said so anywhere — the operator learned only when merge counted 0 events. +func TestWiderBindWarnsCaptureStaysLoopback(t *testing.T) { + dir := manifestDir(t) + var srv *http.Server + var err error + stderr := captureStderr(t, func() { srv, err = Serve("0.0.0.0:0", dir) }) + if err != nil { + t.Skipf("cannot bind 0.0.0.0 here: %v", err) + } + defer Shutdown(srv) + if want := "capture posts are accepted from loopback clients only"; !strings.Contains(stderr, want) { + t.Errorf("wider bind printed no warning; want %q on stderr, got %q", want, stderr) + } + + // A loopback bind stays quiet — the warning must not cry wolf. + quiet := captureStderr(t, func() { + s2, err := Serve(":0", manifestDir(t)) + if err != nil { + t.Fatalf("Serve: %v", err) + } + Shutdown(s2) + }) + if strings.Contains(quiet, "loopback clients only") { + t.Errorf("loopback bind printed the wider-bind warning: %q", quiet) + } +} + +// TestRefusedCaptureWriteIsLogged pins the refusal signal: EVERY refused +// capture post answers an error status the sendBeacon client cannot surface, +// so each refusal — the forgery guard's, a mis-shaped record's, an over-long +// body's — must reach the operator's terminal, as a failed persist already +// does. The 400 shape refusals went silent when they were first added. +func TestRefusedCaptureWriteIsLogged(t *testing.T) { + cases := []struct { + name string + body string + hdr map[string]string + code int + }{ + {"rebound host", `{"t":1,"kind":"click"}`, map[string]string{"Host": "evil.example:8737"}, http.StatusForbidden}, + {"shape refusal", `{"t":1}`, nil, http.StatusBadRequest}, + {"non-object body", `[1,2,3]`, nil, http.StatusBadRequest}, + {"over-long record", jsonRecordOfSize(t, session.MaxJSONLLine), nil, http.StatusRequestEntityTooLarge}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + s, _ := newTestServer(t) + stderr := captureStderr(t, func() { + w := httptest.NewRecorder() + s.handleInteraction(w, jsonPost("/api/interactions", c.body, c.hdr)) + if w.Code != c.code { + t.Fatalf("status = %d, want %d", w.Code, c.code) + } + }) + if want := "capture write refused"; !strings.Contains(stderr, want) { + t.Errorf("refused write logged nothing; want %q on stderr, got %q", want, stderr) + } + }) + } +} + +// TestInteractionRefusedWhenMergeWouldRefuseIt is the write/read shape +// regression: the endpoint accepted any JSON value — an array, a bare string, +// null, a number, an object missing the required t or kind — with 204, and the +// record was durably persisted for merge to refuse later, breaking the whole +// session (docs/reference/cli.md promises one JSON object per request and 400 +// on malformed bodies). The write side must refuse exactly what the reader +// cannot take back. +func TestInteractionRefusedWhenMergeWouldRefuseIt(t *testing.T) { + cases := map[string]string{ + "array": `[1,2,3]`, + "string": `"hello"`, + "null": `null`, + "number": `42`, + "missing t": `{"kind":"click"}`, + "missing kind": `{"t":1}`, + "non-positive t": `{"t":0,"kind":"click"}`, + "absurd t": `{"t":9000000000000000000,"kind":"click"}`, + } + for name, body := range cases { + t.Run(name, func(t *testing.T) { + s, dir := newTestServer(t) + w := httptest.NewRecorder() + s.handleInteraction(w, jsonPost("/api/interactions", body, nil)) + if w.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400", w.Code) + } + if lines := fileLines(t, filepath.Join(dir, session.InteractionsFile)); len(lines) != 0 { + t.Fatalf("refused record still wrote %d lines: %q", len(lines), lines) + } + }) + } +} + // jsonRecordOfSize builds a single valid, whitespace-free interaction JSON // object of exactly n bytes, padding its text field. Because it carries no // insignificant whitespace, json.Compact leaves it byte-for-byte, so its stored @@ -415,7 +552,7 @@ func TestOversizedBatchRecordIsRefusedWhole(t *testing.T) { // TestServeRefusesSymlinkStream ensures the capture server will not open its // stream files through a pre-planted symlink (arbitrary-file append). func TestServeRefusesSymlinkStream(t *testing.T) { - dir := t.TempDir() + dir := manifestDir(t) outside := filepath.Join(t.TempDir(), "victim") if err := os.WriteFile(outside, []byte("keep\n"), 0o644); err != nil { t.Fatal(err) diff --git a/internal/record/record.go b/internal/record/record.go index 3722210..1e987d8 100644 --- a/internal/record/record.go +++ b/internal/record/record.go @@ -186,6 +186,36 @@ func Run(opts Options) error { atStartup := time.Since(dead.started) < startupWindow stopAll(children) stopDemo(srv) + // An early exit is reported the same way as a recorder that produced + // nothing (docs/reference/cli.md) — which includes validating what the + // OTHER recorders left behind and printing the next-command block. A + // session whose recorder died mid-way still holds whatever was captured + // up to that point (a partial audio.wav is transcribable), and without + // this the operator got the classification but no word on whether the + // artefacts are usable or what to run next. + // + // The dead recorder is excluded from the artefact sweep: its story is + // the classification returned below, and classifyMissingOutput's + // stayed-blocked-on-the-prompt narrative is disproved by the very exit + // that brought us here — running both printed two mutually exclusive + // diagnoses (and the stderr tail twice) in one run. Its artefact still + // counts towards the Next block. + others := make([]*liveChild, 0, len(children)) + for _, c := range children { + if c != dead { + others = append(others, c) + } + } + audioReady, problems := finaliseOutputs(dir, others) + if dead.stream == streamMicrophone { + if fi, err := os.Stat(expectedOutput(dir, dead.stream)); err == nil && fi.Size() > 0 { + audioReady = true + } + } + for _, p := range problems { + fmt.Fprintf(opts.Log, "\n%s\n", p) + } + fmt.Fprintf(opts.Log, "\n%s\n", nextCommands(dir, audioReady)) return errors.New(classifyRecorderExit(dead.stream, dead.err, dead.stderr.tail(), atStartup)) } diff --git a/internal/record/record_test.go b/internal/record/record_test.go index 3240e33..ad0a8b5 100644 --- a/internal/record/record_test.go +++ b/internal/record/record_test.go @@ -450,6 +450,85 @@ func TestRunInstallsSignalHandlerBeforeSpawning(t *testing.T) { } } +// TestEarlyRecorderExitStillFinalisesAndPrintsNext pins the promise of +// docs/reference/cli.md that a recorder exiting on its own is "reported the +// same way" as one that produced nothing — which includes validating the +// artefacts actually left behind and printing the next-command block. Pre-fix +// the recorder-exit branch returned straight after stopping the others, so a +// session whose recorder died mid-way still held a usable audio.wav and +// captured interactions, and the operator got no word on whether they could +// transcribe it or what to run next. +func TestEarlyRecorderExitStillFinalisesAndPrintsNext(t *testing.T) { + origNotify, origStart := notifyContext, startRecordersFn + t.Cleanup(func() { notifyContext, startRecordersFn = origNotify, origStart }) + + // Never cancelled: Run must reach the recorder-exit branch, not the Ctrl+C one. + notifyContext = func() (context.Context, context.CancelFunc) { + return context.WithCancel(context.Background()) + } + startRecordersFn = func(dir string, streams []string, _ io.Writer) ([]*liveChild, error) { + // The recorder leaves a usable audio.wav behind and then dies on its own + // (a device disconnect mid-session). + if err := os.WriteFile(filepath.Join(dir, session.AudioFile), []byte("RIFF...."), 0o644); err != nil { + t.Fatal(err) + } + mic := newLiveChild(streamMicrophone, newFakeProc(syscall.SIGINT), &lockedBuffer{}) + _ = mic.p.Signal(syscall.SIGINT) + return []*liveChild{mic}, nil + } + + var log bytes.Buffer + err := Run(Options{Out: t.TempDir(), GOOS: "darwin", Log: &log}) + if err == nil { + t.Fatal("a recorder exiting on its own must make Run exit non-zero") + } + out := log.String() + if !strings.Contains(out, "Next:") { + t.Fatalf("early recorder exit printed no next-command block: %q", out) + } + if !strings.Contains(out, "testimony transcribe") { + t.Fatalf("the usable audio.wav was not offered for transcription: %q", out) + } +} + +// TestEarlyRecorderExitDoesNotDoubleDiagnose pins the single-story rule for +// the early-exit path: the dead recorder's account is the classification the +// command exits with, so classifyMissingOutput — whose text asserts the +// recorder "stayed blocked on the permission prompt", the narrative the exit +// itself disproves — must not also run for it. Printing both gave the +// operator two mutually exclusive diagnoses (and the stderr tail twice) in +// one run. +func TestEarlyRecorderExitDoesNotDoubleDiagnose(t *testing.T) { + origNotify, origStart := notifyContext, startRecordersFn + t.Cleanup(func() { notifyContext, startRecordersFn = origNotify, origStart }) + + notifyContext = func() (context.Context, context.CancelFunc) { + return context.WithCancel(context.Background()) + } + startRecordersFn = func(dir string, streams []string, _ io.Writer) ([]*liveChild, error) { + // The recorder dies on its own having captured nothing at all. + mic := newLiveChild(streamMicrophone, newFakeProc(syscall.SIGINT), &lockedBuffer{}) + _ = mic.p.Signal(syscall.SIGINT) + return []*liveChild{mic}, nil + } + + var log bytes.Buffer + err := Run(Options{Out: t.TempDir(), GOOS: "darwin", Log: &log}) + if err == nil { + t.Fatal("a recorder exiting on its own must make Run exit non-zero") + } + out := log.String() + if strings.Contains(out, "stayed blocked on the permission prompt") { + t.Fatalf("the dead recorder was also reported through classifyMissingOutput, contradicting its own exit: %q", out) + } + if !strings.Contains(out, "Next:") { + t.Fatalf("the next-command block must still print: %q", out) + } + if strings.Contains(out, "testimony transcribe -session") && !strings.Contains(out, "-audio") { + t.Fatalf("with no audio captured, bare transcribe must not be offered: %q", out) + } +} + // TestRunClassifiesStartupExitDespiteSlowStop proves that a recorder which dies // inside the start-up window is still diagnosed as a permissions denial even // when the stop path that follows outlasts that window. The pre-fix code diff --git a/internal/timeline/timeline.go b/internal/timeline/timeline.go index 6f815ac..7831e81 100644 --- a/internal/timeline/timeline.go +++ b/internal/timeline/timeline.go @@ -4,6 +4,7 @@ package timeline import ( + "encoding/json" "errors" "fmt" "io/fs" @@ -198,33 +199,8 @@ const maxUtteranceSeconds = 1e9 func checkedInteractions(path string, t0EpochMS int64, raw []rawInteraction) ([]Interaction, error) { out := make([]Interaction, 0, len(raw)) for i, r := range raw { - if r.T == nil { - return nil, fmt.Errorf("%s: interaction %d is missing t; cannot place it on the session clock", path, i+1) - } - // t is epoch milliseconds, so a value at or below zero anchors the event at - // or before 1 January 1970 — no capture produces that, exactly the reasoning - // session.Manifest.T0 refuses a non-positive anchor on. Refusing it here also - // forecloses the extreme: a t near math.MinInt64 makes (t - t0EpochMS) wrap on - // signed overflow in BuildEntries and plant the event millions of years after - // session start, inflating the report's span while Merge still exits 0. - if *r.T <= 0 { - return nil, fmt.Errorf("%s: interaction %d has t %d; an epoch-millisecond time must be positive", path, i+1, *r.T) - } - // Bound the resulting session-relative magnitude, the epoch-ms twin of - // checkedUtterances' |t0| ≤ maxUtteranceSeconds check. The sign check above - // rules out the negative extreme, but a huge positive t (up to MaxInt64) still - // yields rel = (t − t0)/1000 of ~9e15 s — no session time — which merge writes - // while exiting 0, then report's end() reports as the session span and clock() - // renders as the broken "--:--", and it also inflates analyze.indexTimeline's - // idx.end so a finding may be anchored anywhere up to it. checkedUtterances - // refuses the same absurd magnitude on the speech side; without this the - // interaction side was the asymmetric gap, admitting a session-relative time - // its own twin rejects. t0 is the manifest anchor Merge resolved for this call. - if rel := float64(*r.T-t0EpochMS) / 1000.0; math.Abs(rel) > maxUtteranceSeconds { - return nil, fmt.Errorf("%s: interaction %d has t %d, a session-relative time of %gs that exceeds %g in magnitude", path, i+1, *r.T, rel, maxUtteranceSeconds) - } - if r.Kind == "" { - return nil, fmt.Errorf("%s: interaction %d is missing kind; an event must name what happened", path, i+1) + if err := checkInteraction(r, t0EpochMS); err != nil { + return nil, fmt.Errorf("%s: interaction %d %s", path, i+1, err) } out = append(out, Interaction{ T: *r.T, @@ -238,6 +214,61 @@ func checkedInteractions(path string, t0EpochMS int64, raw []rawInteraction) ([] return out, nil } +// checkInteraction enforces the required fields on one decoded interaction — +// the rules docs/reference/session-directory.md states — phrased so a caller +// can prefix the record's position ("interaction 3 …"). +// +// t is epoch milliseconds, so a value at or below zero anchors the event at +// or before 1 January 1970 — no capture produces that, exactly the reasoning +// session.Manifest.T0 refuses a non-positive anchor on. Refusing it also +// forecloses the extreme: a t near math.MinInt64 makes (t - t0EpochMS) wrap on +// signed overflow in BuildEntries and plant the event millions of years after +// session start, inflating the report's span while Merge still exits 0. +// +// The session-relative magnitude is bounded too — the epoch-ms twin of +// checkedUtterances' |t0| ≤ maxUtteranceSeconds check. The sign check +// rules out the negative extreme, but a huge positive t (up to MaxInt64) still +// yields rel = (t − t0)/1000 of ~9e15 s — no session time — which merge writes +// while exiting 0, then report's end() reports as the session span and clock() +// renders as the broken "--:--", and it also inflates analyze.indexTimeline's +// idx.end so a finding may be anchored anywhere up to it. checkedUtterances +// refuses the same absurd magnitude on the speech side; without this the +// interaction side was the asymmetric gap, admitting a session-relative time +// its own twin rejects. t0EpochMS is the manifest anchor the caller resolved. +// +// A record with no kind is refused because it would join the timeline naming +// no observed action. +func checkInteraction(r rawInteraction, t0EpochMS int64) error { + if r.T == nil { + return errors.New("is missing t; cannot place it on the session clock") + } + if *r.T <= 0 { + return fmt.Errorf("has t %d; an epoch-millisecond time must be positive", *r.T) + } + if rel := float64(*r.T-t0EpochMS) / 1000.0; math.Abs(rel) > maxUtteranceSeconds { + return fmt.Errorf("has t %d, a session-relative time of %gs that exceeds %g in magnitude", *r.T, rel, maxUtteranceSeconds) + } + if r.Kind == "" { + return errors.New("is missing kind; an event must name what happened") + } + return nil +} + +// CheckInteraction validates one interactions.jsonl record — a single JSON +// object carrying the fields docs/reference/session-directory.md marks +// required — against the same rules Merge enforces. It exists for the capture +// write side: every reader refuses these records, so an endpoint that accepted +// one would durably persist a line that later fails the whole session's merge. +// t0EpochMS is the manifest anchor the session-relative bound is taken +// against. The error is phrased to follow the word "interaction". +func CheckInteraction(line []byte, t0EpochMS int64) error { + var r rawInteraction + if err := json.Unmarshal(line, &r); err != nil { + return errors.New("is not a JSON interaction object") + } + return checkInteraction(r, t0EpochMS) +} + // checkedUtterances enforces the one transcript field whose absence cannot be // caught later — t0 — and converts the accepted records for BuildEntries. It is // the speech-side counterpart of checkedInteractions, and it exists because diff --git a/internal/transcribe/engine.go b/internal/transcribe/engine.go index bf6f6ce..db617c8 100644 --- a/internal/transcribe/engine.go +++ b/internal/transcribe/engine.go @@ -39,6 +39,24 @@ func detectEngine(pref string) (engine, bin string, err error) { } return EngineWhisperCpp, p, nil default: - return "", "", fmt.Errorf("unknown engine %q: want auto, whisperx, or whispercpp", pref) + if err := CheckEngine(pref); err != nil { + return "", "", err + } + // Unreachable while CheckEngine and the cases above agree on the valid + // set; a drift between them must fail loudly here rather than hand Run + // an empty engine that matches no adapter and writes an empty + // transcript at exit 0. + return "", "", fmt.Errorf("engine %q is valid but not wired to a binary", pref) + } +} + +// CheckEngine validates an -engine value without touching PATH, so the CLI can +// refuse an unknown name as a usage error before any work starts. detectEngine +// keeps applying the same rule when it resolves a binary. +func CheckEngine(pref string) error { + switch pref { + case "", EngineAuto, EngineWhisperX, EngineWhisperCpp: + return nil } + return fmt.Errorf("unknown engine %q: want auto, whisperx, or whispercpp", pref) } diff --git a/internal/transcribe/ffmpeg.go b/internal/transcribe/ffmpeg.go index 8760baa..3256d5b 100644 --- a/internal/transcribe/ffmpeg.go +++ b/internal/transcribe/ffmpeg.go @@ -45,8 +45,11 @@ func checkExternalAudio(in string) error { } // convertAudio produces the canonical ASR input — 16 kHz mono PCM WAV — from -// the original recording via an ffmpeg subprocess. -func convertAudio(in, out string) error { +// the original recording via an ffmpeg subprocess. beforeFinalise, when +// non-nil, runs after the conversion succeeded but before the rename replaces +// out, so a caller can insert its own last refusal (the offset sidecar write) +// while the session is still byte-for-byte as it was found. +func convertAudio(in, out string, beforeFinalise func() error) error { if err := checkExternalAudio(in); err != nil { return err } @@ -63,7 +66,7 @@ func convertAudio(in, out string) error { // treat as the whole recording. return atomicConvert(out, func(tmpPath string) error { return convertRunner(ffmpeg, in, tmpPath) - }) + }, beforeFinalise) } // convertRunner runs the actual ffmpeg conversion into the temp path. A var @@ -87,7 +90,9 @@ var convertRunner = func(ffmpeg, in, tmpPath string) error { // removed, so a failed conversion never leaves a truncated file that a later run would // mistake for the whole recording. The temp shares out's directory so the rename stays // on one filesystem and is atomic. The producer receives the temp path. -func atomicConvert(out string, produce func(tmpPath string) error) error { +// beforeFinalise, when non-nil, is the caller's last refusal: it runs +// immediately before the rename, and its error aborts with out untouched. +func atomicConvert(out string, produce func(tmpPath string) error, beforeFinalise func() error) error { tmp, err := os.CreateTemp(filepath.Dir(out), ".audio-*.wav") if err != nil { return fmt.Errorf("audio convert: create temp: %w", err) @@ -111,6 +116,11 @@ func atomicConvert(out string, produce func(tmpPath string) error) error { if err := os.Chmod(tmpPath, 0o644&^os.FileMode(um)); err != nil { return fmt.Errorf("audio convert: finalise %s: %w", out, err) } + if beforeFinalise != nil { + if err := beforeFinalise(); err != nil { + return err + } + } if err := os.Rename(tmpPath, out); err != nil { return fmt.Errorf("audio convert: finalise %s: %w", out, err) } diff --git a/internal/transcribe/transcribe.go b/internal/transcribe/transcribe.go index 5447f0c..de253cb 100644 --- a/internal/transcribe/transcribe.go +++ b/internal/transcribe/transcribe.go @@ -97,12 +97,6 @@ func Run(opts Options) (int, error) { if err != nil { return 0, err } - if external { - if err := convertAudio(opts.Audio, wav); err != nil { - return 0, err - } - } - // An external recording's audio.wav is NOT captured at t0, and disk bytes // cannot distinguish it from a record-origin audio.wav. Persist the offset // beside audio.wav so a later bare `transcribe` (a re-run with a different @@ -122,7 +116,63 @@ func Run(opts Options) (int, error) { return 0, err } } - if persist { + if external { + // The sidecar is persisted BETWEEN the conversion producing its temp + // file and the rename that replaces audio.wav, for the same reason the + // offset resolution above runs first: the sidecar write is the last + // remaining refusal, and refusing after the rename destroyed the + // record-origin capture at exit 1 with no offset recorded. Written in + // this order, a refused sidecar aborts with the session byte-for-byte + // as it was found; and if the rename itself then fails, the sidecar is + // rolled back so the session never claims a persisted offset for audio + // that was never converted — which would prime every later bare run to + // shift a record-origin capture at exit 0. + // + // The prior-bytes capture is bounded like every other untrusted read on + // the session surface: a received session can ship an arbitrarily large + // file at this name, and an unbounded ReadAll buffered it whole before + // the conversion even started. Three prior states drive the rollback: + // absent (remove the sidecar this run wrote), captured (restore the + // prior bytes), and present-but-uncapturable — over the cap, or a read + // the no-follow guard refuses — where the rollback must NOT remove: + // deleting a sidecar this run did not create would relabel external + // audio as record-origin, the exact silent shift the sidecar prevents. + sidecar := filepath.Join(opts.SessionDir, session.AudioOffsetFile) + var prior []byte + priorExists := false + priorCaptured := false + if f, rerr := session.OpenFileNoFollowRead(sidecar); rerr == nil { + priorExists = true + if b, rerr := io.ReadAll(io.LimitReader(f, maxOffsetSidecarBytes+1)); rerr == nil && len(b) <= maxOffsetSidecarBytes { + prior, priorCaptured = b, true + } + f.Close() + } else if !errors.Is(rerr, os.ErrNotExist) { + priorExists = true + } + wrote := false + convErr := convertAudio(opts.Audio, wav, func() error { + if werr := writeOffsetSidecar(opts.SessionDir, offset, provenance); werr != nil { + return fmt.Errorf("persist audio offset: %w", werr) + } + wrote = true + return nil + }) + if convErr != nil { + if wrote { + switch { + case priorCaptured: + _ = session.WriteFileNoFollow(sidecar, prior, 0o644) + case !priorExists: + _ = os.Remove(sidecar) + } + // priorExists && !priorCaptured: the sidecar this run wrote stays — + // restoring is impossible and removing would erase the one durable + // fact still true of the session, that audio.wav is external. + } + return 0, convErr + } + } else if persist { if err := writeOffsetSidecar(opts.SessionDir, offset, provenance); err != nil { return 0, fmt.Errorf("persist audio offset: %w", err) } diff --git a/internal/transcribe/transcribe_test.go b/internal/transcribe/transcribe_test.go index 0f27781..d738c35 100644 --- a/internal/transcribe/transcribe_test.go +++ b/internal/transcribe/transcribe_test.go @@ -524,6 +524,115 @@ func assertSessionUntouched(t *testing.T, dir, wav string, converted *bool) { } } +// TestSidecarRefusalDoesNotDestroyAudio closes the residual gap in the +// resolve-before-convert invariant: resolution was hoisted ahead of the +// conversion, but the sidecar persist still ran after it, so a refused sidecar +// write — a directory or symlink planted at audio.offset.json in a received +// session, ENOSPC — exited 1 with the record-origin audio.wav already replaced +// by the converted external recording and no offset recorded. The persist now +// happens before the rename that replaces audio.wav, so this refusal too +// leaves the session byte-for-byte as it found it. +func TestSidecarRefusalDoesNotDestroyAudio(t *testing.T) { + fakeTools(t) + dir, wav := seedSession(t, session.Manifest{Session: "s", T0EpochMS: 1_700_000_000_000}) + stubConvert(t) + if err := os.Mkdir(filepath.Join(dir, session.AudioOffsetFile), 0o755); err != nil { + t.Fatal(err) + } + + _, err := Run(Options{SessionDir: dir, Audio: externalRecording(t), Engine: EngineWhisperX, Log: io.Discard}) + if err == nil || !strings.Contains(err.Error(), "persist audio offset") { + t.Fatalf("want the sidecar persist refusal, got %v", err) + } + if b, rerr := os.ReadFile(wav); rerr != nil || string(b) != recordOriginAudio { + t.Errorf("the record-origin %s was replaced by a run refused at the sidecar: %q (err=%v)", session.AudioFile, b, rerr) + } + entries, err := os.ReadDir(dir) + if err != nil { + t.Fatal(err) + } + if len(entries) != 3 { // manifest.json, audio.wav, the planted directory + t.Errorf("a refused run left extra files behind: %v", entries) + } +} + +// TestRenameFailureRollsBackSidecar pins the other half of the persist-before- +// rename ordering: when the rename itself fails after the sidecar was written, +// the sidecar must not survive — a session carrying a persisted offset for an +// audio.wav that was never converted primes every later bare run to apply that +// offset to record-origin audio at exit 0. +func TestRenameFailureRollsBackSidecar(t *testing.T) { + fakeTools(t) + dir, wav := seedSession(t, session.Manifest{Session: "s", T0EpochMS: 1_700_000_000_000}) + old := convertRunner + t.Cleanup(func() { convertRunner = old }) + convertRunner = func(_, _, tmpPath string) error { + if err := os.WriteFile(tmpPath, []byte("RIFF converted external recording"), 0o644); err != nil { + return err + } + // Sabotage the finalise: a non-empty directory at audio.wav makes the + // rename fail after the conversion (and the sidecar write) succeeded. + if err := os.Remove(wav); err != nil { + return err + } + if err := os.Mkdir(wav, 0o755); err != nil { + return err + } + return os.WriteFile(filepath.Join(wav, "occupied"), []byte("x"), 0o644) + } + + _, err := Run(Options{SessionDir: dir, Audio: externalRecording(t), Engine: EngineWhisperX, Log: io.Discard}) + if err == nil { + t.Fatal("a failed finalise must fail the run") + } + if _, statErr := os.Stat(filepath.Join(dir, session.AudioOffsetFile)); !os.IsNotExist(statErr) { + t.Errorf("the sidecar survived a failed finalise (err=%v); the session claims an offset for audio that was never converted", statErr) + } +} + +// TestRenameFailureKeepsUncapturablePriorSidecar pins the third rollback +// state: a prior sidecar too large to capture (the bounded read refuses it — +// a received session can ship anything at this name) must survive a failed +// finalise as whatever this run wrote, never be removed. Removing a sidecar +// the run did not create would relabel external audio as record-origin — the +// silent shift the sidecar exists to prevent. The bound itself is the point: +// the capture read stops at the cap instead of buffering an arbitrarily large +// attacker-authored file. +func TestRenameFailureKeepsUncapturablePriorSidecar(t *testing.T) { + fakeTools(t) + dir, wav := seedSession(t, session.Manifest{Session: "s", T0EpochMS: 1_700_000_000_000}) + // An over-cap prior: readable, regular, but past maxOffsetSidecarBytes. + huge := make([]byte, maxOffsetSidecarBytes+2) + for i := range huge { + huge[i] = 'x' + } + if err := os.WriteFile(filepath.Join(dir, session.AudioOffsetFile), huge, 0o644); err != nil { + t.Fatal(err) + } + old := convertRunner + t.Cleanup(func() { convertRunner = old }) + convertRunner = func(_, _, tmpPath string) error { + if err := os.WriteFile(tmpPath, []byte("RIFF converted external recording"), 0o644); err != nil { + return err + } + if err := os.Remove(wav); err != nil { + return err + } + if err := os.Mkdir(wav, 0o755); err != nil { + return err + } + return os.WriteFile(filepath.Join(wav, "occupied"), []byte("x"), 0o644) + } + + _, err := Run(Options{SessionDir: dir, Audio: externalRecording(t), Engine: EngineWhisperX, Log: io.Discard}) + if err == nil { + t.Fatal("a failed finalise must fail the run") + } + if _, statErr := os.Stat(filepath.Join(dir, session.AudioOffsetFile)); statErr != nil { + t.Errorf("the sidecar was removed on rollback despite a pre-existing one the run could not capture: %v", statErr) + } +} + // TestRunRefusesNonRegularExternalAudio is the hang regression on the offset // derivation. The refusals that make -audio safe to open — the accepted-container // check and the regular-file check — lived inside convertAudio, so hoisting the @@ -717,7 +826,7 @@ func TestConvertAudioIntegration(t *testing.T) { } out := filepath.Join(dir, session.AudioFile) - if err := convertAudio(in, out); err != nil { + if err := convertAudio(in, out, nil); err != nil { t.Fatalf("convertAudio: %v", err) } fi, err := os.Stat(out) @@ -730,7 +839,7 @@ func TestConvertAudioIntegration(t *testing.T) { t.Fatalf("conversion left temp files behind: %v", temps) } - if err := convertAudio(filepath.Join(dir, "voice.mp3"), out); err == nil { + if err := convertAudio(filepath.Join(dir, "voice.mp3"), out, nil); err == nil { t.Fatal("unsupported extension must error") } @@ -744,7 +853,7 @@ func TestConvertAudioIntegration(t *testing.T) { t.Fatal(err) } failOut := filepath.Join(failDir, session.AudioFile) - if err := convertAudio(corrupt, failOut); err == nil { + if err := convertAudio(corrupt, failOut, nil); err == nil { t.Fatal("a corrupt input must fail the conversion") } if _, statErr := os.Stat(failOut); !os.IsNotExist(statErr) { @@ -770,7 +879,7 @@ func TestAtomicConvertHonoursUmask(t *testing.T) { out := filepath.Join(dir, session.AudioFile) if err := atomicConvert(out, func(tmpPath string) error { return os.WriteFile(tmpPath, []byte("RIFF....complete"), 0o644) - }); err != nil { + }, nil); err != nil { t.Fatalf("atomicConvert: %v", err) } fi, err := os.Stat(out) @@ -815,7 +924,7 @@ func TestConvertAudioRoutesThroughTemp(t *testing.T) { return errors.New("ffmpeg: killed by signal") } - if err := convertAudio(in, out); err == nil { + if err := convertAudio(in, out, nil); err == nil { t.Fatal("convertAudio must surface the conversion failure") } if producedAt == "" { @@ -850,7 +959,7 @@ func TestAtomicConvertLeavesNoPartialOnFailure(t *testing.T) { t.Fatalf("seed partial temp: %v", werr) } return errors.New("ffmpeg: killed by signal") - }) + }, nil) if err == nil { t.Fatal("atomicConvert must surface the producer's error") } @@ -866,7 +975,7 @@ func TestAtomicConvertLeavesNoPartialOnFailure(t *testing.T) { // A succeeding producer renames its temp into place and leaves no temp. if err := atomicConvert(out, func(tmpPath string) error { return os.WriteFile(tmpPath, []byte("RIFF....complete"), 0o644) - }); err != nil { + }, nil); err != nil { t.Fatalf("atomicConvert on success: %v", err) } if b, rerr := os.ReadFile(out); rerr != nil || string(b) != "RIFF....complete" { @@ -917,7 +1026,7 @@ func TestConvertAudioRefusesSymlinkOutput(t *testing.T) { // The guard must fire specifically on the symlink, before the ffmpeg PATH // lookup — otherwise (with ffmpeg present) the victim would be overwritten, // and on a machine without ffmpeg the error would merely be "not found". - err := convertAudio(in, out) + err := convertAudio(in, out, nil) if err == nil || !strings.Contains(err.Error(), "is a symlink") { t.Fatalf("want symlink refusal, got %v", err) } @@ -945,7 +1054,7 @@ func TestConvertAudioRefusesFIFOOutput(t *testing.T) { } done := make(chan error, 1) - go func() { done <- convertAudio(in, out) }() + go func() { done <- convertAudio(in, out, nil) }() select { case err := <-done: @@ -970,7 +1079,7 @@ func TestConvertAudioRefusesFIFOInput(t *testing.T) { } done := make(chan error, 1) - go func() { done <- convertAudio(in, filepath.Join(dir, session.AudioFile)) }() + go func() { done <- convertAudio(in, filepath.Join(dir, session.AudioFile), nil) }() select { case err := <-done: @@ -991,7 +1100,7 @@ func TestConvertAudioRefusesFIFOInput(t *testing.T) { } // A symlinked recording is legitimate, so it must get past the input guard // and fail (if at all) only later, on the ffmpeg lookup or the conversion. - if err := convertAudio(link, filepath.Join(dir, session.AudioFile)); err != nil && + if err := convertAudio(link, filepath.Join(dir, session.AudioFile), nil); err != nil && strings.Contains(err.Error(), "not a regular file") { t.Fatalf("symlink to a regular recording must be accepted, got %v", err) }