feat: media expansion surfaces — edit mask, upscale, background removal, interpolation, diarization, meshgen (ADR-0020) #14
@@ -15,9 +15,9 @@ type BackgroundRemovalRequest struct {
|
||||
Net string
|
||||
|
||||
// OnlyMask returns the black/white segmentation mask instead of the
|
||||
// cutout — white where the subject is. Useful as an inpainting mask
|
||||
// (EditRequest.Mask semantics after inversion: the mask marks the
|
||||
// FOREGROUND, an inpaint mask marks the region to REPAINT).
|
||||
// cutout — WHITE marks the SUBJECT. To inpaint (repaint) the subject,
|
||||
// use it directly as EditRequest.Mask; to repaint the BACKGROUND,
|
||||
|
|
||||
// invert it first (EditRequest.Mask is white-means-repaint).
|
||||
OnlyMask bool
|
||||
}
|
||||
|
||||
|
||||
@@ -7,7 +7,6 @@ import (
|
||||
"fmt"
|
||||
"io"
|
||||
"mime"
|
||||
"mime/multipart"
|
||||
"net/http"
|
||||
"net/url"
|
||||
"strings"
|
||||
@@ -119,28 +118,19 @@ func (m *transcriptionModel) Transcribe(ctx context.Context, req audio.Transcrip
|
||||
return nil, fmt.Errorf("%w: transcription requires audio bytes", llm.ErrUnsupported)
|
||||
}
|
||||
|
||||
var buf bytes.Buffer
|
||||
w := multipart.NewWriter(&buf)
|
||||
fw, err := w.CreateFormFile("file", transcriptionFilename(req))
|
||||
buf, formType, err := buildMultipart("build transcription form",
|
||||
filePart{field: "file", filename: transcriptionFilename(req.Filename, req.MIME), data: req.Audio},
|
||||
[]formField{
|
||||
{"model", m.id, true},
|
||||
{"response_format", "json", true},
|
||||
{"language", req.Language, false},
|
||||
{"prompt", req.Prompt, false},
|
||||
})
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("llama-swap: build transcription form: %w", err)
|
||||
}
|
||||
if _, err := fw.Write(req.Audio); err != nil {
|
||||
return nil, fmt.Errorf("llama-swap: build transcription form: %w", err)
|
||||
}
|
||||
if err := writeFormFields(w, "build transcription form", []formField{
|
||||
{"model", m.id, true},
|
||||
{"response_format", "json", true},
|
||||
{"language", req.Language, false},
|
||||
{"prompt", req.Prompt, false},
|
||||
}); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if err := w.Close(); err != nil {
|
||||
return nil, fmt.Errorf("llama-swap: build transcription form: %w", err)
|
||||
}
|
||||
|
||||
raw, _, err := m.p.doRaw(ctx, http.MethodPost, "/v1/audio/transcriptions", m.id, w.FormDataContentType(), &buf, maxResponseBytes)
|
||||
raw, _, err := m.p.doRaw(ctx, http.MethodPost, "/v1/audio/transcriptions", m.id, formType, buf, maxResponseBytes)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
@@ -158,11 +148,11 @@ func (m *transcriptionModel) Transcribe(ctx context.Context, req audio.Transcrip
|
||||
// headers), else one derived from the MIME subtype ("audio.mp3"), else
|
||||
// "audio". MIME parameters ("audio/ogg; codecs=opus") are stripped before
|
||||
// matching.
|
||||
func transcriptionFilename(req audio.TranscriptionRequest) string {
|
||||
if name := sanitizeFilename(req.Filename); name != "" {
|
||||
func transcriptionFilename(filename, mimeType string) string {
|
||||
if name := sanitizeFilename(filename); name != "" {
|
||||
return name
|
||||
}
|
||||
mt := strings.ToLower(strings.TrimSpace(req.MIME))
|
||||
mt := strings.ToLower(strings.TrimSpace(mimeType))
|
||||
if parsed, _, err := mime.ParseMediaType(mt); err == nil {
|
||||
mt = parsed
|
||||
}
|
||||
|
||||
@@ -79,7 +79,7 @@ func (m *diarizationModel) Diarize(ctx context.Context, req audio.DiarizationReq
|
||||
}
|
||||
|
||||
body, contentType, err := buildMultipart("build diarization form",
|
||||
filePart{field: "audio_file", filename: diarizationFilename(req), data: req.Audio},
|
||||
filePart{field: "audio_file", filename: transcriptionFilename(req.Filename, req.MIME), data: req.Audio},
|
||||
nil)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
@@ -111,12 +111,3 @@ func (m *diarizationModel) Diarize(ctx context.Context, req audio.DiarizationReq
|
||||
}
|
||||
return res, nil
|
||||
}
|
||||
|
||||
// diarizationFilename mirrors transcriptionFilename for the diarization
|
||||
// request shape (same untrusted-metadata sanitization rules).
|
||||
func diarizationFilename(req audio.DiarizationRequest) string {
|
||||
return transcriptionFilename(audio.TranscriptionRequest{
|
||||
Filename: req.Filename,
|
||||
MIME: req.MIME,
|
||||
})
|
||||
}
|
||||
|
||||
@@ -22,6 +22,12 @@ import (
|
||||
"gitea.stevedudenhoeffer.com/steve/majordomo/videogen"
|
||||
)
|
||||
|
||||
// maxImageResponseBytes caps the raw-image bodies from the upscale and
|
||||
// background-removal endpoints. The 64MB maxResponseBytes is a JSON cap;
|
||||
// a 4x-upscaled PNG legitimately exceeds it. Bounded (not video-sized)
|
||||
// because a single still image past this is an upstream bug, not data.
|
||||
const maxImageResponseBytes = 256 << 20
|
||||
|
||||
// --- upscale ---
|
||||
|
||||
// UpscaleModel implements imagegen.UpscaleProvider against the mediautils
|
||||
@@ -62,7 +68,7 @@ func (m *upscaleModel) Upscale(ctx context.Context, req imagegen.UpscaleRequest,
|
||||
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, maxImageResponseBytes)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
@@ -112,23 +118,31 @@ func (m *backgroundModel) RemoveBackground(ctx context.Context, req imagegen.Bac
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
gitea-actions
commented
🟠 Empty Content-Type bypasses non-image payload validation in singleImageResult error-handling · flagged by 1 model 🪰 Gadfly · advisory 🟠 **Empty Content-Type bypasses non-image payload validation in singleImageResult**
_error-handling · flagged by 1 model_
<sub>🪰 Gadfly · advisory</sub>
|
||||
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, maxImageResponseBytes)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return singleImageResult(m.p.name, m.id, "background removal", raw, respType)
|
||||
|
gitea-actions
commented
🟠 singleImageResult validation bypassed by image/ Content-Type header* correctness · flagged by 1 model
🪰 Gadfly · advisory 🟠 **singleImageResult validation bypassed by image/* Content-Type header**
_correctness · flagged by 1 model_
- `provider/llamaswap/mediautil.go:125` — `singleImageResult` accepts non-image payloads when the upstream sends a misleading `Content-Type: image/*` header, and then `sniffImageMIME` returns a fabricated `image/png` MIME type for those bytes. **Impact:** A misconfigured proxy or hostile upstream that returns an error page with `Content-Type: image/png` (or any `image/*`) will bypass the “non-image payload” defense and be wrapped as a valid `imagegen.Result` with a false `image/png` MIME. **Fix:…
<sub>🪰 Gadfly · advisory</sub>
|
||||
}
|
||||
|
||||
// singleImageResult wraps one raw image body into an imagegen.Result,
|
||||
// rejecting empty or non-image payloads (a proxy error page must not become
|
||||
// "the image").
|
||||
// singleImageResult wraps one raw image body into an imagegen.Result.
|
||||
// Acceptance requires positive evidence of image-ness — sniffed magic
|
||||
|
gitea-actions
commented
🔴 singleImageResult: missing Content-Type header bypasses image-validation check, sniffImageMIME lies and returns image/png for any payload correctness, error-handling, maintainability, security · flagged by 6 models
🪰 Gadfly · advisory 🔴 **singleImageResult: missing Content-Type header bypasses image-validation check, sniffImageMIME lies and returns image/png for any payload**
_correctness, error-handling, maintainability, security · flagged by 6 models_
`provider/llamaswap/mediautil.go:129`: ```go mimeType := sniffImageMIME(raw) // always returns "image/png" for non-image bytes if ct := strings.TrimSpace(contentType); ct != "" && // gate: skipped when header absent !strings.HasPrefix(ct, "image/") && !strings.HasPrefix(http.DetectContentType(raw), "image/") { return nil, &llm.APIError{...} } return &imagegen.Result{Images: []llm.ImagePart{{MIME: mimeType, Data: raw}}}, nil ```
<sub>🪰 Gadfly · advisory</sub>
|
||||
// bytes or a declared image/* Content-Type — so a proxy error page with
|
||||
// an empty Content-Type can never become "the image" (sniffImageMIME's
|
||||
// PNG default is a labelling fallback, not a validator).
|
||||
func singleImageResult(provider, model, verb string, raw []byte, contentType string) (*imagegen.Result, error) {
|
||||
if len(raw) == 0 {
|
||||
return nil, &llm.APIError{Provider: provider, Model: model, Message: verb + " response contained no image"}
|
||||
}
|
||||
mimeType := sniffImageMIME(raw)
|
||||
if ct := strings.TrimSpace(contentType); ct != "" && !strings.HasPrefix(ct, "image/") && !strings.HasPrefix(http.DetectContentType(raw), "image/") {
|
||||
return nil, &llm.APIError{Provider: provider, Model: model, Message: fmt.Sprintf("%s response is not an image (Content-Type %q)", verb, ct)}
|
||||
sniffed := http.DetectContentType(raw)
|
||||
declared := strings.TrimSpace(contentType)
|
||||
if !strings.HasPrefix(sniffed, "image/") && !strings.HasPrefix(declared, "image/") {
|
||||
return nil, &llm.APIError{Provider: provider, Model: model,
|
||||
Message: fmt.Sprintf("%s response is not an image (Content-Type %q, sniffed %q)", verb, declared, sniffed)}
|
||||
}
|
||||
mimeType := sniffed
|
||||
if !strings.HasPrefix(mimeType, "image/") {
|
||||
mimeType = declared
|
||||
}
|
||||
return &imagegen.Result{Images: []llm.ImagePart{{MIME: mimeType, Data: raw}}}, nil
|
||||
}
|
||||
|
||||
@@ -251,3 +251,32 @@ func TestUpstreamPathRejectsSeparators(t *testing.T) {
|
||||
t.Errorf("path prefix wrong: %q", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestUpscaleRejectsEmptyContentTypeNonImage(t *testing.T) {
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.Header()["Content-Type"] = nil // suppress auto-detection: NO Content-Type at all
|
||||
_, _ = w.Write([]byte("502 bad gateway"))
|
||||
}))
|
||||
defer srv.Close()
|
||||
|
||||
p := New(WithBaseURL(srv.URL), WithHTTPClient(srv.Client()))
|
||||
up, _ := p.UpscaleModel("mediautils")
|
||||
_, err := up.Upscale(context.Background(),
|
||||
imagegen.UpscaleRequest{Image: imagegen.Image{Data: pngFixture(t)}})
|
||||
var apiErr *llm.APIError
|
||||
if !errors.As(err, &apiErr) {
|
||||
t.Fatalf("err = %v, want APIError for headerless non-image body", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestUpstreamPathRejectsDotDot(t *testing.T) {
|
||||
if _, err := upstreamPath("..", "/x"); err == nil {
|
||||
t.Error("model '..' accepted")
|
||||
}
|
||||
if _, err := upstreamPath("m", "/v1/audio?path=../../api/models/unload"); err == nil {
|
||||
t.Error("dot-dot rest accepted")
|
||||
}
|
||||
if _, err := upstreamPath("m", "/v1/audio?path=https://evil.example/x"); err == nil {
|
||||
t.Error("absolute-URL rest accepted")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -102,7 +102,7 @@ func (m *meshModel) Generate(ctx context.Context, req meshgen.Request, opts ...m
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("llama-swap: encode mesh request: %w", err)
|
||||
}
|
||||
raw, _, err := m.p.doRaw(ctx, http.MethodPost, path, m.id, "application/json", bytes.NewReader(encoded), maxMeshResponseBytes)
|
||||
raw, respType, err := m.p.doRaw(ctx, http.MethodPost, path, m.id, "application/json", bytes.NewReader(encoded), maxMeshResponseBytes)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
@@ -110,8 +110,16 @@ func (m *meshModel) Generate(ctx context.Context, req meshgen.Request, opts ...m
|
||||
return nil, &llm.APIError{Provider: m.p.name, Model: m.id, Message: "mesh response contained no data"}
|
||||
}
|
||||
// A JSON body on the success path is the server reporting a soft error
|
||||
// (or an API drift) — never hand it back as "the mesh".
|
||||
if first := strings.TrimLeft(string(raw[:min(len(raw), 64)]), " \t\r\n"); strings.HasPrefix(first, "{") || strings.HasPrefix(first, "[") {
|
||||
// (or an API drift) — never hand it back as "the mesh". Two signals:
|
||||
// the declared Content-Type, and a whitespace-tolerant peek at the
|
||||
|
gitea-actions
commented
🟠 JSON-vs-mesh body check only inspects first 64 bytes; a JSON error body with >64 leading whitespace bytes bypasses the guard and is returned as mesh bytes correctness, maintainability, security · flagged by 2 models
🪰 Gadfly · advisory 🟠 **JSON-vs-mesh body check only inspects first 64 bytes; a JSON error body with >64 leading whitespace bytes bypasses the guard and is returned as mesh bytes**
_correctness, maintainability, security · flagged by 2 models_
- **`provider/llamaswap/mesh.go:114` — JSON-vs-mesh detection only inspects the first 64 bytes.** Line 114 reads `raw[:min(len(raw), 64)]` then trims whitespace and checks for `{`/`[`. A JSON soft-error body with >64 bytes of leading whitespace (or 64 bytes of garbage then `{`) sails past the guard and is returned as "the mesh." This is the exact class the check exists to defend against (queue-full detail pages becoming "the mesh"). Low-to-medium severity given the upstream is host-built/trusted…
<sub>🪰 Gadfly · advisory</sub>
|
||||
// leading bytes (512 covers any indented error envelope; a binary STL
|
||||
// header theoretically CAN start with '{', but a real one also won't
|
||||
// be all-whitespace-then-brace).
|
||||
if strings.Contains(strings.ToLower(respType), "application/json") {
|
||||
return nil, &llm.APIError{Provider: m.p.name, Model: m.id,
|
||||
Message: "mesh response is JSON, not mesh bytes: " + truncateForError(raw)}
|
||||
}
|
||||
if first := strings.TrimLeft(string(raw[:min(len(raw), 512)]), " \t\r\n"); strings.HasPrefix(first, "{") || strings.HasPrefix(first, "[") {
|
||||
return nil, &llm.APIError{Provider: m.p.name, Model: m.id,
|
||||
|
gitea-actions
commented
🟡 truncateForError shadows predeclared identifier cap maintainability · flagged by 3 models
🪰 Gadfly · advisory 🟡 **truncateForError shadows predeclared identifier cap**
_maintainability · flagged by 3 models_
- `provider/llamaswap/mesh.go:124` — `truncateForError` shadows the predeclared identifier `cap` (`const cap = 500`). Rename the constant to `limit` or `maxLen` to avoid confusing readers and tooling.
<sub>🪰 Gadfly · advisory</sub>
|
||||
Message: "mesh response is JSON, not mesh bytes: " + truncateForError(raw)}
|
||||
}
|
||||
@@ -120,9 +128,9 @@ func (m *meshModel) Generate(ctx context.Context, req meshgen.Request, opts ...m
|
||||
|
||||
// truncateForError bounds a payload quoted into an error message.
|
||||
func truncateForError(b []byte) string {
|
||||
const cap = 500
|
||||
if len(b) > cap {
|
||||
return string(b[:cap]) + "..."
|
||||
const maxErrLen = 500
|
||||
if len(b) > maxErrLen {
|
||||
return string(b[:maxErrLen]) + "..."
|
||||
}
|
||||
return string(b)
|
||||
}
|
||||
|
||||
@@ -21,12 +21,19 @@ func upstreamPath(model, rest string) (string, error) {
|
||||
if strings.TrimSpace(model) == "" {
|
||||
return "", fmt.Errorf("llama-swap: upstream call requires a model id")
|
||||
}
|
||||
if strings.ContainsAny(model, "/?#") {
|
||||
if strings.ContainsAny(model, "/?#") || strings.Contains(model, "..") {
|
||||
|
gitea-actions
commented
🔴 Percent-encoded path separators (%2F, %3F, %23) bypass model-id validation, enabling path traversal through the upstream server's URL decoding security · flagged by 2 models
🪰 Gadfly · advisory 🔴 **Percent-encoded path separators (%2F, %3F, %23) bypass model-id validation, enabling path traversal through the upstream server's URL decoding**
_security · flagged by 2 models_
- **`provider/llamaswap/upstream.go:24` — Percent-encoded path separator bypass in `upstreamPath` (HIGH):** The function rejects literal `/`, `?`, `#` in the model id via `strings.ContainsAny(model, "/?#")`, but does not reject their percent-encoded forms (`%2F`, `%3F`, `%23`). Go's `http.NewRequestWithContext` (called at `llamaswap.go:205`) preserves already-encoded sequences in the URL path, so a model id like `foo%2Fbar` would be sent as `/upstream/foo%2Fbar/...` on the wire. A receiving serv…
<sub>🪰 Gadfly · advisory</sub>
|
||||
return "", fmt.Errorf("llama-swap: invalid model id %q for upstream call (contains a path separator)", model)
|
||||
}
|
||||
if !strings.HasPrefix(rest, "/") {
|
||||
rest = "/" + rest
|
||||
}
|
||||
// rest may embed SERVER-SUPPLIED components (e.g. ACE-Step's result
|
||||
// file URL) — refuse dot-dot segments and absolute-URL smuggling so a
|
||||
// hostile/buggy upstream cannot redirect the follow-up request at
|
||||
// another proxy endpoint (/api/models/unload, ...).
|
||||
if strings.Contains(rest, "..") || strings.Contains(rest, "://") {
|
||||
return "", fmt.Errorf("llama-swap: invalid upstream path %q (dot-dot or scheme)", rest)
|
||||
}
|
||||
return "/upstream/" + model + rest, nil
|
||||
}
|
||||
|
||||
|
||||
🟡 OnlyMask doc comment's 'after inversion' phrasing is ambiguous about inversion direction
maintainability · flagged by 1 model
imagegen/background.go:19—OnlyMaskdoc comment's "after inversion" phrasing is ambiguous about inversion direction. The comment (lines 17-20) reads "EditRequest.Mask semantics after inversion: the mask marks the FOREGROUND, an inpaint mask marks the region to REPAINT." The intended meaning (foreground mask → invert → inpaint repaint region) is correct but requires a reader to stop and parse which direction the inversion goes. Minor doc nit; stating the direction explicitly would help…🪰 Gadfly · advisory