test(audio): ffprobe output past a cap is unreadable, not buffered (#49), red

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

DECISIONS.md

@@ -1531,7 +1531,7 @@ deferred. The scope cut — chat first, no rich analysis, no video — is what k
   #33 specifies, with the serving endpoint shown **(recommended, matches the issue)**; (B)
   disabled by default, a failed iguana job fails honestly and the user retries. *Blocks:*
   the production default only; the adapter ships either way.
-- Verify Tapir's ingress `proxy-body-size` against the 200 MB cap (`infra`).
+- ~~Verify Tapir's ingress `proxy-body-size` against the 200 MB cap (`infra`).~~ **Resolved 2026-10-03:** real unauthenticated bodies of 2, 30, 120 and 210 MiB all pass NPM (TLS, piguard) and Traefik to tapir (a 302 to login); there is no body limit in that range. Uploads were enabled in production the same day, local engines only (infra#520).
 - Per-transcript deletion is not built; account deletion cascades. A private-content store
   without a delete button is a gap worth a ticket, not a silent omission.
 

Taskfile.yml

@@ -34,7 +34,7 @@ tasks:
   lint:
     desc: golangci-lint (installed by CI; skipped locally if absent).
     cmds:
-      - 'command -v golangci-lint >/dev/null 2>&1 && golangci-lint run ./... || echo "golangci-lint not installed — skipping (CI installs it)"'
+      - 'if command -v golangci-lint >/dev/null 2>&1; then golangci-lint run ./...; else echo "golangci-lint not installed — skipping (CI installs it)"; fi'
 
   test:
     desc: Run all tests. -p 1 serializes packages so the embedded-postgres tests

docs/sprints/audio-upload-followups-2026-10.md

@@ -0,0 +1,88 @@
+# Sprint: audio upload follow-ups (Sweep Sprint, 2026-10)
+
+Method: Sweep Sprint (phase 1 parallel read-only research per issue: **done**; phase 2 sequential
+execution, one issue per commit: **not started, waits for the owner's go**). Closing oath below.
+
+Milestones: [v0.31.1 - security and robustness](https://git.d-ma.be/mathias/tapir/milestone/10),
+[v0.32.0 - audio upload follow-ups](https://git.d-ma.be/mathias/tapir/milestone/11).
+
+Context: v0.31.0 shipped audio upload (ADR-033/034/035) and is live in production behind auth at
+`/uploads`, with the Berget cloud fallback on for audio (infra#522). A real 63.7 min upload
+transcribed in 2m53s with no errors. The phase-1 investigation found two pre-existing or new
+weaknesses an authenticated user can reach, so the first release is a hotfix.
+
+## Oath (closing gates, every clause must hold; blocked = descoped and tracked, never "done")
+
+| # | Clause | Evidence |
+|---|--------|----------|
+| SG1 | Every issue in the sprint is closed with a linked commit, or moved out with a stated reason | issue list of both milestones |
+| SG2 | Each fix has a test that was seen red on main first, named after the mechanism it observes | red/green output in the issue |
+| SG3 | `task check` AND `golangci-lint run ./...` exit 0 (checked by `$?`, not by grep); `task check` does not gate lint until #41 | gate output |
+| SG4 | Control code (#48, #49, the upload cap in #47) has a recorded adversarial review verified by execution, verdict in the issue | gatekeeper verdict comment |
+| SG5 | Docs current in the same commit: ADR-033/035 text, feature file, use-case coverage test | diff |
+| SG6 | Each release tagged (v0.31.1, v0.32.0), pushed, deployed, and observed live (pod running the new image, no restart loop, a real upload still works) | `kubectl`/log + one upload |
+| SG7 | Learnings written to the brain; deferred work filed as issues; decisions that are the owner's recorded as DECISION NEEDED | brain ids + issue numbers |
+
+## Work order
+
+### v0.31.1 (security and robustness; hotfix, do first)
+
+1. **#41 lint gate.** `task check` swallows lint findings (`|| echo`). Make the gate fail on findings,
+   fix what it surfaces. First, so every later gate means something. (Decision D1.)
+2. **#48 cross-origin POST protection.** `http.NewCrossOriginProtection()` around `Router()`.
+   Red: foreign-origin POST to `/account/delete`, `/uploads`, `/uploads/{id}/chat` is refused.
+3. **#49 ffprobe/ffmpeg limits.** `-max_streams 32` + narrower probe output; probe stdout cap;
+   probe deadline. Three commits.
+4. **#47 concurrent-upload cap (remaining item).** Bound the spool: reservation per accepted upload
+   against the 6Gi emptyDir so concurrent uploads cannot evict the pod.
+5. **#50 SIGTERM interim.** Handle SIGTERM; record the interrupted job at once; fix the false
+   comment. Full drain recorded as a limitation. (Decision D2.)
+
+Release v0.31.1: one adversarial review round over 2-5, then tag, push, observe.
+
+### v0.32.0 (follow-ups)
+
+6. **#52** label a cloud-served transcript in plain words; document the empty-transcript trigger.
+7. **#53** bound the LLM client's response body read.
+8. **#51** tracker: save timeout, then app-clock heartbeat.
+9. **#37** nav link to uploads and per-recording delete (queued/transcribing: refuse until it
+   ends). (Decision D3.)
+10. **#36** chat default model after measurement. (Decision D4.)
+11. **#38** measurements (Berget length limit, large models on production iguana). (D5, D6.)
+12. **#46** remainder of the hardening list after the items above are split out.
+13. **#45** evaluate Berget as the reconciler: a question, not work. (D8.)
+
+## DECISION NEEDED (owner's call; recommendation first)
+
+| # | Question | Recommendation | Blocks |
+|---|----------|----------------|--------|
+| D1 | #41: should any linter be allowed to opt out of the gate? | No opt-out; fix findings | #41 |
+| D2 | #50: full SIGTERM drain, or record as a limitation and ship the interim fix? | Limitation + interim; a drain costs 4-6 h and downtime on every deploy at current volume | #50 |
+| D3 | #37: delete of a queued/transcribing recording | Refuse until it ends | #37 |
+| D4 | #36: make a 31B model the chat default? | Only after measurement, conditional | #36 |
+| D5 | #38: Berget length measurement needs a throwaway key | Yes, throwaway key | #38 |
+| D6 | #38: load large models on production iguana | Yes, in a quiet window with the owner present | #38 |
+| D7 | #46: where the ffmpeg tarball lives | Do nothing now; file an issue | none |
+| D8 | #45: evaluate Berget as the reconciler | No; stay local-only | #45 |
+| D9 | Who judges ADR-034's second blind check | The owner; do not reuse the one-off Opus authorisation | ADR-034 |
+| D10 | When to enable reconcile in production | Needs `iguana/pianissimo` in the key pin and env vars; owner present | ADR-034 |
+
+## Dropped or reduced, with reasons
+
+- **#46 item 1 (`alis`/`dref` external reference):** checked, not a problem. Under the application's
+  flags ffmpeg skips the external track and writes a 78-byte WAV.
+- **#46 item 2 / #47 item 2:** merged into #49.
+- **#47 item 3 (full drain):** reduced to #50 plus a recorded limitation.
+- **#46 `Beat` taking the tracker mutex:** incorrect in #46; only `logStill` does. Corrected in #51.
+
+## Human-present items (do not run unattended)
+
+- Production iguana model loads (#38, D6).
+- Enabling reconcile in production (D10).
+- Any key change or `apply` in infra (infra#523, infra#524 are open and not part of this sprint).
+
+## Release plan
+
+SemVer: v0.31.1 patch (fixes, no new behaviour); v0.32.0 minor (nav link, delete, cloud labelling).
+Every push to tapir main redeploys the pod and fails any job in flight: push when no upload is
+running (check the pod log first).

internal/adapters/audio/ffmpeg_test.go

@@ -109,3 +109,20 @@ printf data > "$last"`)
 	require.NoError(t, err)
 	assert.NotContains(t, " "+strings.ReplaceAll(string(raw), "\n", " ")+" ", " -t ")
 }
+
+func TestFFmpegToWAV_CapsTheStreamCountBeforeTheInput(t *testing.T) {
+	// ffmpeg also allocated ~570 MB opening the 900-stream file (#49).
+	rec := filepath.Join(t.TempDir(), "argv")
+	bin := stub(t, `printf '%s\n' "$@" > `+rec+`
+for a; do last="$a"; done
+printf data > "$last"`)
+
+	err := (&audio.FFmpeg{Bin: bin}).ToWAV(context.Background(), "/spool/in", filepath.Join(t.TempDir(), "o.wav"))
+
+	require.NoError(t, err)
+	args := argvOf(t, rec)
+	ms, in := indexOf(args, "-max_streams"), indexOf(args, "-i")
+	require.GreaterOrEqual(t, ms, 0, "argv: %v", args)
+	assert.Equal(t, "32", args[ms+1])
+	assert.Less(t, ms, in, "an input option placed after -i is applied to the output")
+}

internal/adapters/audio/ffprobe.go

@@ -30,9 +30,22 @@ type FFprobe struct {
 // formats. "mov" covers .mp4/.m4a/.3gp; "matroska" covers .webm.
 const AllowedDemuxers = "wav,mov,mp4,m4a,3gp,mp3,ogg,matroska,webm,flac,aac,opus,aiff,amr,asf"
 
+// MaxStreams bounds how many streams a file may declare. A 1.3 MB file declaring 900 of
+// them cost ~570 MB of RSS in each of ffprobe and ffmpeg (#49), against a 512 Mi pod;
+// real recordings have a handful.
+const MaxStreams = "32"
+
 // inputRestrictions are ffmpeg/ffprobe INPUT options and must precede the input.
-// Local files and pipes only: no network, no other protocols.
-var inputRestrictions = []string{"-protocol_whitelist", "file,pipe", "-format_whitelist", AllowedDemuxers}
+// Local files and pipes only: no network, no other protocols, and a bounded stream count.
+var inputRestrictions = []string{
+	"-protocol_whitelist", "file,pipe",
+	"-format_whitelist", AllowedDemuxers,
+	"-max_streams", MaxStreams,
+}
+
+// probeEntries is exactly what ParseProbe reads. -show_format/-show_streams print every
+// tag too, and a 120 MB WAV made 491 MB of JSON that Output() buffers.
+const probeEntries = "format=duration:stream=codec_type:stream_disposition=attached_pic"
 
 // killGrace bounds how long Probe waits for a cancelled ffprobe's pipes to close.
 const killGrace = 2 * time.Second
@@ -48,7 +61,7 @@ func (f *FFprobe) Probe(ctx context.Context, path string) (uploads.Probe, error)
 		bin = "ffprobe"
 	}
 	args := append([]string{"-v", "error"}, inputRestrictions...)
-	args = append(args, "-print_format", "json", "-show_format", "-show_streams", path)
+	args = append(args, "-print_format", "json", "-show_entries", probeEntries, path)
 	cmd := exec.CommandContext(ctx, bin, args...)
 	cmd.WaitDelay = killGrace
 	out, err := cmd.Output()

internal/adapters/audio/ffprobe_test.go

@@ -184,3 +184,53 @@ func TestAllowedDemuxers_ExcludeThePlaylistFormats(t *testing.T) {
 		assert.Contains(t, strings.Split(audio.AllowedDemuxers, ","), good)
 	}
 }
+
+func TestFFprobeProbe_CapsTheStreamCountBeforeTheInput(t *testing.T) {
+	// A 1.3 MB file declaring 900 streams cost ~570 MB of RSS in ffprobe (#49) against
+	// a 512 Mi pod. -max_streams is an input option: it must precede the path.
+	bin, argvFile := argvStub(t)
+
+	_, err := (&audio.FFprobe{Bin: bin}).Probe(context.Background(), "/spool/upload-1")
+
+	require.NoError(t, err)
+	args := argvOf(t, argvFile)
+	ms := indexOf(args, "-max_streams")
+	require.GreaterOrEqual(t, ms, 0, "argv: %v", args)
+	assert.Equal(t, "32", args[ms+1])
+	assert.Less(t, ms, len(args)-1, "must come before the path, which stays last")
+}
+
+func TestFFprobeProbe_AsksOnlyForTheFieldsParseProbeReads(t *testing.T) {
+	// -show_format/-show_streams print every tag: a 120 MB WAV produced 491 MB of JSON
+	// that cmd.Output() buffers in Go (#49).
+	bin, argvFile := argvStub(t)
+
+	_, err := (&audio.FFprobe{Bin: bin}).Probe(context.Background(), "/spool/upload-1")
+
+	require.NoError(t, err)
+	args := argvOf(t, argvFile)
+	assert.Equal(t, -1, indexOf(args, "-show_format"), "argv: %v", args)
+	assert.Equal(t, -1, indexOf(args, "-show_streams"), "argv: %v", args)
+	se := indexOf(args, "-show_entries")
+	require.GreaterOrEqual(t, se, 0, "argv: %v", args)
+	assert.Equal(t, "format=duration:stream=codec_type:stream_disposition=attached_pic", args[se+1])
+}
+
+func TestFFprobeProbe_OutputPastTheCapIsUnreadableNotBuffered(t *testing.T) {
+	// ffprobe's stdout is attacker-influenced (tags, 491 MB observed). Past the cap the
+	// probe is stopped and the file is reported as unreadable (#49).
+	bin := stub(t, `head -c 200000 /dev/zero | tr '\0' 'a'`)
+
+	_, err := (&audio.FFprobe{Bin: bin, MaxOutput: 4096}).Probe(context.Background(), "/x")
+
+	assert.ErrorIs(t, err, uploads.ErrUnreadable)
+}
+
+func TestFFprobeProbe_OutputWithinTheCapIsParsed(t *testing.T) {
+	bin := stub(t, `echo '{"streams":[{"codec_type":"audio"}],"format":{"duration":"90"}}'`)
+
+	got, err := (&audio.FFprobe{Bin: bin, MaxOutput: 4096}).Probe(context.Background(), "/x")
+
+	require.NoError(t, err)
+	assert.True(t, got.HasAudio)
+}

internal/web/crossorigin_test.go

@@ -0,0 +1,137 @@
+package web_test
+
+// Observes the cross-origin gate around Router (#48): SameSite=Lax does not stop a POST
+// from a sibling *.d-ma.be origin, so the router itself must refuse a non-safe request
+// whose Origin or Sec-Fetch-Site says it did not come from this site's own pages.
+
+import (
+	"context"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// foreign is a header set a browser sends for a form post made by another page.
+var foreign = map[string]map[string]string{
+	"sibling subdomain, Sec-Fetch-Site": {"Origin": "https://evil.d-ma.be", "Sec-Fetch-Site": "same-site"},
+	"other site, Sec-Fetch-Site":        {"Origin": "https://evil.example", "Sec-Fetch-Site": "cross-site"},
+	"sibling subdomain, Origin only":    {"Origin": "https://evil.d-ma.be"},
+	"opaque origin":                     {"Origin": "null"},
+}
+
+// own is a header set for the app's own pages (or a non-browser client): must pass.
+var own = map[string]map[string]string{
+	"same-origin Sec-Fetch-Site": {"Sec-Fetch-Site": "same-origin", "Origin": "http://example.com"},
+	"matching Origin only":       {"Origin": "http://example.com"},
+	"no browser headers":         {},
+}
+
+func withHeaders(req *http.Request, h map[string]string) *http.Request {
+	for k, v := range h {
+		req.Header.Set(k, v)
+	}
+	return req
+}
+
+func userCount(t *testing.T) int {
+	t.Helper()
+	var n int
+	require.NoError(t, rawPool(t).QueryRow(context.Background(), `SELECT count(*) FROM users WHERE id = $1`, userID).Scan(&n))
+	return n
+}
+
+func TestCrossOrigin_ARefusedDeleteAccountPostLeavesTheAccountInPlace(t *testing.T) {
+	for name, h := range foreign {
+		t.Run(name, func(t *testing.T) {
+			app, _ := newAccountApp(t)
+			resetDB(t, rawPool(t))
+			seedConnection(t, app, "youtube", "ada@channel", "ref")
+
+			rec := do(t, app, withHeaders(httptest.NewRequest(http.MethodPost, "/account/delete", nil), h))
+
+			assert.Equal(t, http.StatusForbidden, rec.Code)
+			assert.Equal(t, 1, userCount(t), "the account must survive a refused post")
+		})
+	}
+}
+
+func TestCrossOrigin_ASameOriginOrHeaderlessDeleteAccountPostIsServed(t *testing.T) {
+	for name, h := range own {
+		t.Run(name, func(t *testing.T) {
+			app, _ := newAccountApp(t)
+			resetDB(t, rawPool(t))
+			seedConnection(t, app, "youtube", "ada@channel", "ref")
+
+			rec := do(t, app, withHeaders(httptest.NewRequest(http.MethodPost, "/account/delete", nil), h))
+
+			assert.Equal(t, http.StatusSeeOther, rec.Code)
+			assert.Equal(t, "/auth/logout", rec.Header().Get("Location"))
+		})
+	}
+}
+
+func TestCrossOrigin_ARefusedUploadPostSubmitsNoJob(t *testing.T) {
+	for name, h := range foreign {
+		t.Run(name, func(t *testing.T) {
+			app, sub := newUploadApp(t, audioOK, 200<<20)
+
+			rec := do(t, app, withHeaders(uploadReq(t, "m", "m.wav", []byte("RIFF....WAVE")), h))
+
+			assert.Equal(t, http.StatusForbidden, rec.Code)
+			assert.Empty(t, sub.jobs, "a refused post must not enqueue transcription")
+		})
+	}
+}
+
+func TestCrossOrigin_ARefusedUploadChatPostCallsNoModel(t *testing.T) {
+	for name, h := range foreign {
+		t.Run(name, func(t *testing.T) {
+			app, fc := newUploadChatApp(t)
+			id := seedUpload(t, userID, "Roadmap sync", "done", meetingText)
+
+			req := chatPost("/uploads/"+id+"/chat", url.Values{"message": {"hej"}})
+			rec := do(t, app, withHeaders(req, h))
+
+			assert.Equal(t, http.StatusForbidden, rec.Code)
+			assert.Zero(t, fc.callCount)
+		})
+	}
+}
+
+func TestCrossOrigin_AForeignPostIsRefusedOnEachListedStateChangingPath(t *testing.T) {
+	routes := []string{
+		"/account/disconnect/youtube",
+		"/account/summarize-mode",
+		"/register",
+		"/paste",
+		"/v/abc/action",
+		"/v/abc/summarize",
+		"/v/abc/retry-now",
+		"/v/abc/chat",
+	}
+	for _, path := range routes {
+		t.Run(path, func(t *testing.T) {
+			app := newApp(t)
+			req := httptest.NewRequest(http.MethodPost, path, strings.NewReader(""))
+			req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+			rec := do(t, app, withHeaders(req, foreign["sibling subdomain, Sec-Fetch-Site"]))
+			assert.Equal(t, http.StatusForbidden, rec.Code)
+		})
+	}
+}
+
+// A login redirect comes back to /auth/callback as a cross-site GET navigation, and
+// health probes carry no headers: safe methods must never be refused on origin grounds.
+func TestCrossOrigin_ACrossSiteGETIsNotRefused(t *testing.T) {
+	app := newApp(t)
+	for _, path := range []string{"/healthz", "/welcome"} {
+		req := httptest.NewRequest(http.MethodGet, path, nil)
+		req.Header.Set("Sec-Fetch-Site", "cross-site")
+		assert.Equal(t, http.StatusOK, do(t, app, req).Code, path)
+	}
+}

internal/web/handlers.go

@@ -229,7 +229,12 @@ func (a *App) Router() http.Handler {
 	// resolved user_id. /register lives inside the auth guard but is exempt from
 	// the registration gate (you must be able to reach it before you have a user).
 	root.Handle("/", a.Auth.Middleware(a.registrationGate(app)))
-	return root
+
+	// The session cookie is SameSite=Lax, which still travels on a POST made from a
+	// sibling *.d-ma.be page (same site, different origin). Refuse any non-safe request
+	// whose Origin or Sec-Fetch-Site says it did not come from this origin (#48). It wraps
+	// everything, so a route added later is covered without remembering to ask.
+	return http.NewCrossOriginProtection().Handler(root)
 }
 
 // handleWelcome renders the public landing page (/welcome). It is mounted outside

test/lintgate/lintgate_test.go

@@ -0,0 +1,108 @@
+package lintgate
+
+// Observes the exit status of `task lint` (the lint step of `task check`), not the lint
+// rules themselves. It runs the repo's real Taskfile in a scratch module so a change to the
+// lint command that stops it failing on findings (#41) turns this red.
+
+import (
+	"bytes"
+	"errors"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+const scratchWithFinding = `package main
+
+import "os"
+
+func main() { os.Remove("x") }
+`
+
+func scratchModule(t *testing.T) string {
+	t.Helper()
+	taskfile, err := os.ReadFile(filepath.Join("..", "..", "Taskfile.yml"))
+	if err != nil {
+		t.Fatalf("read Taskfile.yml: %v", err)
+	}
+	dir := t.TempDir()
+	for name, body := range map[string]string{
+		"Taskfile.yml": string(taskfile),
+		"go.mod":       "module scratch\n\ngo 1.21\n",
+		"main.go":      scratchWithFinding,
+	} {
+		if err := os.WriteFile(filepath.Join(dir, name), []byte(body), 0o644); err != nil {
+			t.Fatalf("write %s: %v", name, err)
+		}
+	}
+	return dir
+}
+
+// hasSkipNotice reports whether the skip notice was printed as output. Task echoes the command
+// line first, and that line contains the same words, so a substring match would be vacuous.
+func hasSkipNotice(out string) bool {
+	for _, line := range strings.Split(out, "\n") {
+		if strings.HasPrefix(line, "golangci-lint not installed") {
+			return true
+		}
+	}
+	return false
+}
+
+// runLint runs `task lint` in dir with the given PATH and returns its combined output and exit code.
+func runLint(t *testing.T, dir, path string) (string, int) {
+	t.Helper()
+	taskBin, err := exec.LookPath("task")
+	if err != nil {
+		t.Skip("task not installed")
+	}
+	cmd := exec.Command(taskBin, "lint")
+	cmd.Dir = dir
+	cmd.Env = append(os.Environ(), "PATH="+path)
+	var out bytes.Buffer
+	cmd.Stdout, cmd.Stderr = &out, &out
+	err = cmd.Run()
+	var exitErr *exec.ExitError
+	switch {
+	case err == nil:
+		return out.String(), 0
+	case errors.As(err, &exitErr):
+		return out.String(), exitErr.ExitCode()
+	default:
+		t.Fatalf("run task lint: %v", err)
+		return "", -1
+	}
+}
+
+func TestTaskLint_ExitsNonZeroWhenGolangciLintReportsAFinding(t *testing.T) {
+	if _, err := exec.LookPath("golangci-lint"); err != nil {
+		t.Skip("golangci-lint not installed")
+	}
+	out, code := runLint(t, scratchModule(t), os.Getenv("PATH"))
+	if code == 0 {
+		t.Fatalf("task lint exited 0 on a module with an unchecked error; output:\n%s", out)
+	}
+	if hasSkipNotice(out) {
+		t.Fatalf("task lint blamed a missing binary for a lint failure; output:\n%s", out)
+	}
+}
+
+func TestTaskLint_SkipsWithANoticeWhenGolangciLintIsAbsent(t *testing.T) {
+	taskBin, err := exec.LookPath("task")
+	if err != nil {
+		t.Skip("task not installed")
+	}
+	bin := t.TempDir() // a PATH holding task and nothing else
+	if err := os.Symlink(taskBin, filepath.Join(bin, "task")); err != nil {
+		t.Fatalf("symlink task: %v", err)
+	}
+	out, code := runLint(t, scratchModule(t), bin)
+	if code != 0 {
+		t.Fatalf("task lint exited %d without golangci-lint on PATH; output:\n%s", code, out)
+	}
+	if !hasSkipNotice(out) {
+		t.Fatalf("expected the skip notice; output:\n%s", out)
+	}
+}