fix: review findings — clone-route audio sniffing, zip caps, filename sanitization, WAV response caps

- speakWithReference now validates the response IS audio via the same
  audioResultMIME sniff the sfx/enhance surfaces use — a 2xx JSON soft
  error or HTML proxy page was previously wrapped up as audio/wav bytes.
- audioResultMIME moves to audio.go (next to speechMIME; it was defined
  in sfx.go but shared by enhance/clone) and learns the Ogg container
  normalization (application/ogg -> audio/ogg).
- Stems zip unpack gains entry-count (16) and total-decompressed (1GB)
  caps on top of the existing per-entry cap — the per-entry bound alone
  still let a many-entry bomb multiply up.
- sanitizeFilename drops NUL and both path separators too, so upload
  metadata can never smuggle directory structure to a file-writing shim.
- New maxAudioResponseBytes (256MB) for bodies that ARE one audio clip
  (clone, enhance, sfx): a long WAV legitimately passes the 64MB JSON
  cap.
- speakWithReference local renamed path -> upPath (naming parity with
  stems/enhance).

Co-Authored-By: Claude Fable 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01WWCQcYStWXBYUy5sZWnbLT

provider/llamaswap/audio.go

@@ -83,12 +83,12 @@ func (m *speechModel) Speak(ctx context.Context, req audio.SpeechRequest, opts .
 // speakWithReference performs zero-shot voice cloning via the upstream
 // passthrough clone route. The reference sample rides as the `voice_file`
 // part and the text as an `input` field; voice/format/speed still go on the
-// wire when set (upstreams ignore fields they don't understand). The route's
-// default container is WAV, so MIME resolution falls back to audio/wav (not
-// the JSON route's mp3) when neither the response header nor the requested
-// format decides.
+// wire when set (upstreams ignore fields they don't understand). The result
+// MIME comes from the response header or content sniffing (audioResultMIME,
+// same validation as the sfx/enhance surfaces) — a JSON soft error or an
+// HTML proxy page must never be wrapped up as audio bytes.
 func (m *speechModel) speakWithReference(ctx context.Context, req audio.SpeechRequest) (*audio.SpeechResult, error) {
-	path, err := upstreamPath(m.id, "/v1/audio/speech/upload")
+	upPath, err := upstreamPath(m.id, "/v1/audio/speech/upload")
 	if err != nil {
 		return nil, err
 	}
@@ -107,18 +107,17 @@ func (m *speechModel) speakWithReference(ctx context.Context, req audio.SpeechRe
 	if err != nil {
 		return nil, err
 	}
-	audioBytes, contentType, err := m.p.doRaw(ctx, http.MethodPost, path, m.id, formType, body, maxResponseBytes)
+	audioBytes, contentType, err := m.p.doRaw(ctx, http.MethodPost, upPath, m.id, formType, body, maxAudioResponseBytes)
 	if err != nil {
 		return nil, err
 	}
 	if len(audioBytes) == 0 {
 		return nil, &llm.APIError{Provider: m.p.name, Model: m.id, Message: "speech clone response contained no audio"}
 	}
-	mimeType := "audio/wav"
-	if mt := mimeFromContentType(contentType, "audio/"); mt != "" {
-		mimeType = mt
-	} else if req.Format != "" {
-		mimeType = speechMIME("", req.Format)
+	mimeType := audioResultMIME(contentType, audioBytes)
+	if mimeType == "" {
+		return nil, &llm.APIError{Provider: m.p.name, Model: m.id,
+			Message: fmt.Sprintf("speech clone response is not audio (Content-Type %q): %s", contentType, truncateForError(audioBytes))}
 	}
 	return &audio.SpeechResult{Audio: audioBytes, MIME: mimeType}, nil
 }
@@ -144,6 +143,28 @@ func speechMIME(contentType, format string) string {
 	}
 }
 
+// audioResultMIME resolves an audio body's MIME type: the response
+// Content-Type when it is a concrete audio type, else content sniffing
+// (RIFF/WAVE, Ogg and friends), else "" — the caller treats undetectable as
+// an upstream error, mirroring videoMIME. Sniffed "audio/wave" is normalized
+// to the conventional "audio/wav", and Ogg's container type
+// "application/ogg" to "audio/ogg". Shared by the clone, sfx, and enhance
+// surfaces, which all answer raw audio bodies.
+func audioResultMIME(contentType string, data []byte) string {
+	if mt := mimeFromContentType(contentType, "audio/"); mt != "" {
+		return mt
+	}
+	switch mt := http.DetectContentType(data); {
+	case mt == "audio/wave":
+		return "audio/wav"
+	case mt == "application/ogg":
+		return "audio/ogg"
+	case strings.HasPrefix(mt, "audio/"):
+		return mt
+	}
+	return ""
+}
+
 // TranscriptionModel implements audio.TranscriptionProvider, binding a
 // speech-to-text model served by llama-swap (routed to a whisper.cpp-style
 // upstream).
@@ -241,10 +262,13 @@ func transcriptionFilename(filename, mimeType string) string {
 }
 
 // sanitizeFilename strips characters that would corrupt or inject into the
-// multipart Content-Disposition header. Quotes and backslashes are escaped
-// by mime/multipart itself; CR/LF are not — they must go.
+// multipart Content-Disposition header, or smuggle directory structure to
+// the receiving side. Quotes and backslashes are escaped by mime/multipart
+// itself, but a file-writing shim decodes them right back — so CR/LF/NUL
+// and both path separators are dropped: an upload-metadata filename must
+// never traverse ("../x", "a/b", "C:\x").
 func sanitizeFilename(name string) string {
-	name = strings.NewReplacer("\r", "", "\n", "").Replace(name)
+	name = strings.NewReplacer("\r", "", "\n", "", "\x00", "", "/", "", "\\", "").Replace(name)
 	return strings.TrimSpace(name)
 }
 

provider/llamaswap/clone_translate_test.go

@@ -2,12 +2,14 @@ package llamaswap
 
 import (
 	"context"
+	"errors"
 	"io"
 	"net/http"
 	"net/http/httptest"
 	"testing"
 
 	"gitea.stevedudenhoeffer.com/steve/majordomo/audio"
+	"gitea.stevedudenhoeffer.com/steve/majordomo/llm"
 )
 
 func TestSpeakWithReferenceUsesCloneRoute(t *testing.T) {
@@ -54,7 +56,7 @@ func TestSpeakWithReferenceUsesCloneRoute(t *testing.T) {
 	}
 }
 
-func TestSpeakWithReferenceDefaultsToWavMIME(t *testing.T) {
+func TestSpeakWithReferenceSniffsHeaderlessWav(t *testing.T) {
 	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		if err := r.ParseMultipartForm(1 << 20); err != nil {
 			t.Fatalf("parse multipart: %v", err)
@@ -65,7 +67,7 @@ func TestSpeakWithReferenceDefaultsToWavMIME(t *testing.T) {
 			}
 		}
 		w.Header()["Content-Type"] = nil // no declared type at all
-		_, _ = w.Write([]byte("RAWAUDIO"))
+		_, _ = w.Write(wavFixture())
 	}))
 	defer srv.Close()
 
@@ -77,9 +79,43 @@ func TestSpeakWithReferenceDefaultsToWavMIME(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Speak: %v", err)
 	}
-	// The clone route's default container is wav, not the JSON route's mp3.
+	// Headerless RIFF sniffs audio/wave, normalized to the conventional wav.
 	if res.MIME != "audio/wav" {
-		t.Errorf("MIME = %q, want audio/wav fallback", res.MIME)
+		t.Errorf("MIME = %q, want sniffed audio/wav", res.MIME)
+	}
+}
+
+func TestSpeakWithReferenceRejectsNonAudioResponse(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		// A FastAPI-style 2xx soft error must not be wrapped up as audio.
+		w.Header().Set("Content-Type", "application/json")
+		_, _ = w.Write([]byte(`{"detail":"reference audio too short"}`))
+	}))
+	defer srv.Close()
+
+	p := New(WithBaseURL(srv.URL), WithHTTPClient(srv.Client()))
+	sm, _ := p.SpeechModel("chatterbox")
+	_, err := sm.Speak(context.Background(),
+		audio.SpeechRequest{Input: "hi"},
+		audio.WithReferenceAudio([]byte("REF"), "audio/wav"))
+	var apiErr *llm.APIError
+	if !errors.As(err, &apiErr) {
+		t.Fatalf("err = %v, want APIError for non-audio clone body", err)
+	}
+}
+
+func TestSanitizeFilenameStripsPathAndControlBytes(t *testing.T) {
+	for in, want := range map[string]string{
+		"song.mp3":                "song.mp3",
+		"../../etc/passwd":        "....etcpasswd",
+		"a\r\nContent-Type: evil": "aContent-Type: evil",
+		"..\\..\\boot.ini":        "....boot.ini",
+		"nul\x00byte.wav":         "nulbyte.wav",
+		"  /  ":                   "",
+	} {
+		if got := sanitizeFilename(in); got != want {
+			t.Errorf("sanitizeFilename(%q) = %q, want %q", in, got, want)
+		}
 	}
 }
 

provider/llamaswap/enhance.go

@@ -45,7 +45,7 @@ func (m *speechEnhancerModel) Enhance(ctx context.Context, req audio.Enhancement
 	if err != nil {
 		return nil, err
 	}
-	raw, respType, err := m.p.doRaw(ctx, http.MethodPost, path, m.id, contentType, body, maxResponseBytes)
+	raw, respType, err := m.p.doRaw(ctx, http.MethodPost, path, m.id, contentType, body, maxAudioResponseBytes)
 	if err != nil {
 		return nil, err
 	}

provider/llamaswap/llamaswap.go

@@ -53,6 +53,12 @@ const maxResponseBytes = 64 << 20
 // can't allocate without limit.
 const maxVideoResponseBytes = 512 << 20
 
+// maxAudioResponseBytes caps bodies that ARE a single encoded audio clip
+// (voice clone, speech enhancement, sfx): a long uncompressed WAV
+// legitimately passes the 64MB JSON cap. Still bounded so a buggy upstream
+// can't allocate without limit.
+const maxAudioResponseBytes = 256 << 20
+
 // Provider is a llama-swap client. It satisfies llm.Provider (chat, delegated
 // to provider/openai) and imagegen.Provider (image generation), and exposes
 // llama-swap's management endpoints as concrete methods.

provider/llamaswap/sfx.go

@@ -84,7 +84,7 @@ func (m *sfxModel) Generate(ctx context.Context, req musicgen.Request, opts ...m
 	if err != nil {
 		return nil, fmt.Errorf("llama-swap: encode sfx request: %w", err)
 	}
-	raw, contentType, err := m.p.doRaw(ctx, http.MethodPost, path, m.id, "application/json", bytes.NewReader(encoded), maxResponseBytes)
+	raw, contentType, err := m.p.doRaw(ctx, http.MethodPost, path, m.id, "application/json", bytes.NewReader(encoded), maxAudioResponseBytes)
 	if err != nil {
 		return nil, err
 	}
@@ -98,21 +98,3 @@ func (m *sfxModel) Generate(ctx context.Context, req musicgen.Request, opts ...m
 	}
 	return &musicgen.Result{Audio: musicgen.Audio{Data: raw, MIME: mimeType}}, nil
 }
-
-// audioResultMIME resolves an audio body's MIME type: the response
-// Content-Type when it is a concrete audio type, else content sniffing
-// (RIFF/WAVE and friends), else "" — the caller treats undetectable as an
-// upstream error, mirroring videoMIME. Sniffed "audio/wave" is normalized
-// to the conventional "audio/wav".
-func audioResultMIME(contentType string, data []byte) string {
-	if mt := mimeFromContentType(contentType, "audio/"); mt != "" {
-		return mt
-	}
-	if mt := http.DetectContentType(data); strings.HasPrefix(mt, "audio/") {
-		if mt == "audio/wave" {
-			return "audio/wav"
-		}
-		return mt
-	}
-	return ""
-}

provider/llamaswap/stems.go

@@ -31,6 +31,15 @@ const maxStemsResponseBytes = 512 << 20
 // A single WAV stem of even a very long song stays far under this.
 const maxStemEntryBytes = 256 << 20
 
+// maxStemEntries caps how many stem entries are unpacked: Demucs emits at
+// most six, so anything past a small multiple of that is a hostile or
+// broken archive, not a result.
+const maxStemEntries = 16
+
+// maxStemsTotalBytes caps the AGGREGATE decompressed size across entries —
+// the per-entry bound alone would still let a many-entry bomb multiply up.
+const maxStemsTotalBytes = 1 << 30
+
 // StemSeparatorModel implements audio.StemSeparationProvider. The id selects
 // which upstream llama-swap loads.
 func (p *Provider) StemSeparatorModel(id string, opts ...audio.StemSeparatorModelOption) (audio.StemSeparator, error) {
@@ -95,15 +104,25 @@ func (m *stemSeparatorModel) SeparateStems(ctx context.Context, req audio.StemSe
 			Message: fmt.Sprintf("stems response is not a zip (Content-Type %q): %s", respType, truncateForError(raw))}
 	}
 	res := &audio.StemSeparationResult{}
+	var totalBytes int64
 	for _, f := range zr.File {
 		if f.FileInfo().IsDir() {
 			continue
 		}
+		if len(res.Stems) >= maxStemEntries {
+			return nil, &llm.APIError{Provider: m.p.name, Model: m.id,
+				Message: fmt.Sprintf("stems zip holds more than %d entries", maxStemEntries)}
+		}
 		data, err := readZipEntry(f)
 		if err != nil {
 			return nil, &llm.APIError{Provider: m.p.name, Model: m.id,
 				Message: fmt.Sprintf("stems zip entry %q: %v", f.Name, err)}
 		}
+		totalBytes += int64(len(data))
+		if totalBytes > maxStemsTotalBytes {
+			return nil, &llm.APIError{Provider: m.p.name, Model: m.id,
+				Message: fmt.Sprintf("stems zip decompresses past %d bytes", int64(maxStemsTotalBytes))}
+		}
 		// Entry name → stem name; extension → MIME. Entries may sit in a
 		// per-model directory ("htdemucs/vocals.mp3"), so use the base name.
 		base := path.Base(f.Name)

provider/llamaswap/stems_sfx_enhance_test.go

@@ -6,8 +6,10 @@ import (
 	"context"
 	"encoding/json"
 	"errors"
+	"fmt"
 	"net/http"
 	"net/http/httptest"
+	"strings"
 	"testing"
 
 	"gitea.stevedudenhoeffer.com/steve/majordomo/audio"
@@ -169,6 +171,27 @@ func TestSeparateStemsRejectsEmptyZip(t *testing.T) {
 	}
 }
 
+func TestSeparateStemsRejectsTooManyEntries(t *testing.T) {
+	entries := map[string]string{}
+	for i := 0; i <= maxStemEntries; i++ {
+		entries[fmt.Sprintf("stem%02d.wav", i)] = "X"
+	}
+	zipBody := stemsZipFixture(t, entries)
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/zip")
+		_, _ = w.Write(zipBody)
+	}))
+	defer srv.Close()
+
+	p := New(WithBaseURL(srv.URL), WithHTTPClient(srv.Client()))
+	ss, _ := p.StemSeparatorModel("audioutils")
+	_, err := ss.SeparateStems(context.Background(), audio.StemSeparationRequest{Audio: []byte("SONG")})
+	var apiErr *llm.APIError
+	if !errors.As(err, &apiErr) || !strings.Contains(apiErr.Message, "entries") {
+		t.Fatalf("err = %v, want APIError for over-long stems zip", err)
+	}
+}
+
 // wavFixture is a minimal RIFF/WAVE header so http.DetectContentType sniffs
 // audio/wave.
 func wavFixture() []byte {