Skip to content

Commit 6a43c80

Browse files
fix(audit): address PR review on audio body logging
Validated automated review findings (greptile, CodeRabbit) and applied the valid ones: - Drop /v1/audio/translations from the endpoint classifier, tests, and docs: the route is not registered or implemented, so classifying it was misleading (greptile P1). Rejected the conflicting suggestion to document it. - Move the response-writer truncation flag inside the non-audio branch so an oversized audio response is not marked ResponseBodyTooBigToHandle while its body is fully captured by the audio handler (CodeRabbit). Add a regression test. - Dashboard CSS: use the defined --text-muted variable instead of the undefined --muted (CodeRabbit). - Clarify the audioBodyMaxBytes comment: the cap is on raw bytes; base64 is ~4/3 larger, which matters on document stores with a hard per-record limit such as MongoDB's 16 MB (greptile P2). - Set LogAudioBodies explicitly in DefaultConfig; correct the documented placeholder shape to {__audio__, content_type, bytes, stored: false}; note the 10 MB cap in the CLAUDE.md reference (CodeRabbit). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 88fb970 commit 6a43c80

10 files changed

Lines changed: 62 additions & 18 deletions

File tree

CLAUDE.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ Full reference: `.env.template` and `config/config.yaml`
116116
- `ENABLED_PASSTHROUGH_PROVIDERS` (openai,anthropic,openrouter,zai,vllm: Comma-separated list of enabled passthrough providers)
117117
- **Storage:** `STORAGE_TYPE` (sqlite), `SQLITE_PATH` (data/gomodel.db), `POSTGRES_URL`, `MONGODB_URL`
118118
- **Models:** `MODELS_ENABLED_BY_DEFAULT` (true), `MODEL_OVERRIDES_ENABLED` (true), `KEEP_ONLY_ALIASES_AT_MODELS_ENDPOINT` (false), `CONFIGURED_PROVIDER_MODELS_MODE` (`fallback` or `allowlist`, default `fallback`; `allowlist` skips upstream `/models` for providers with configured lists); persisted overrides restrict/allow selectors with `user_paths`. When alias-only models listing is enabled, `GET /v1/models` returns only model aliases, not full concrete model specs, to operators.
119-
- **Audit logging:** `LOGGING_ENABLED` (false), `LOGGING_LOG_BODIES` (false), `LOGGING_LOG_AUDIO_BODIES` (false: refines `LOGGING_LOG_BODIES` for audio endpoints — base64 audio for `/v1/audio/speech` + dashboard playback, upload metadata for transcriptions; no effect unless `LOGGING_LOG_BODIES` is on, in which case audio-off records a placeholder), `LOGGING_LOG_HEADERS` (false), `LOGGING_RETENTION_DAYS` (30)
119+
- **Audit logging:** `LOGGING_ENABLED` (false), `LOGGING_LOG_BODIES` (false), `LOGGING_LOG_AUDIO_BODIES` (false: refines `LOGGING_LOG_BODIES` for audio endpoints — base64 audio for `/v1/audio/speech` (≤10 MB, else `too_large`) + dashboard playback, upload metadata for transcriptions; no effect unless `LOGGING_LOG_BODIES` is on, in which case audio-off records a placeholder), `LOGGING_LOG_HEADERS` (false), `LOGGING_RETENTION_DAYS` (30)
120120
- **Usage tracking:** `USAGE_ENABLED` (true), `ENFORCE_RETURNING_USAGE_DATA` (true), `USAGE_RETENTION_DAYS` (90)
121121
- **Dashboard live logs:**
122122
- `DASHBOARD_LIVE_LOGS_ENABLED` (true): keep enabled for low-latency dashboard previews; set false only when live streams are not needed or memory/socket usage must be minimized.

docs/advanced/audio-api.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ unless body logging is enabled:
9494
- **Body logging off** (`LOGGING_LOG_BODIES=false`) — no audio body is stored,
9595
regardless of this setting.
9696
- **Body logging on, audio off** (the default) — the audio response is recorded
97-
as a lightweight `{content_type, bytes}` placeholder; no audio bytes are stored.
97+
as a lightweight `{__audio__, content_type, bytes, stored: false}` placeholder; no audio bytes are stored.
9898
- **Body logging on, audio on**`/v1/audio/speech` stores its text input and the
9999
generated audio (base64, capped at 10 MB) so the **dashboard renders an inline
100100
player**, and `/v1/audio/transcriptions` stores upload metadata (filename, model,

docs/advanced/configuration.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ Storage is shared by audit logging, usage tracking, and future features like IAM
112112
dashboard can play it back, and `/v1/audio/transcriptions` stores upload metadata
113113
(never the raw audio bytes). It defaults to `false` because audio payloads are
114114
large and grow the audit store quickly. When body logging is on but this is off,
115-
audio responses are recorded as a lightweight `{content_type, bytes}` placeholder;
115+
audio responses are recorded as a lightweight `{__audio__, content_type, bytes, stored: false}` placeholder;
116116
when body logging is off, no audio body is stored at all.
117117
</Note>
118118

internal/admin/dashboard/static/css/dashboard.css

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2895,7 +2895,7 @@ textarea:focus {
28952895

28962896
.audit-audio-meta {
28972897
font-size: 12px;
2898-
color: var(--muted, #888);
2898+
color: var(--text-muted, #888);
28992899
}
29002900

29012901
.audit-audio-empty {
@@ -2910,7 +2910,7 @@ textarea:focus {
29102910

29112911
.audit-audio-note {
29122912
font-size: 12px;
2913-
color: var(--muted, #888);
2913+
color: var(--text-muted, #888);
29142914
}
29152915

29162916
.conversation-body-highlight {

internal/auditlog/audio_body.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,12 @@ import (
55
"strings"
66
)
77

8-
// audioBodyMaxBytes caps how much audio is embedded as base64 in an audit log
9-
// entry. Larger payloads are recorded as metadata-only placeholders so the
10-
// audit store does not balloon on long generations.
8+
// audioBodyMaxBytes caps how much *raw* audio is embedded as base64 in an audit
9+
// log entry; larger payloads are recorded as metadata-only placeholders so the
10+
// audit store does not balloon on long generations. Note the stored base64 is
11+
// ~4/3 of this (≈13.3 MB at the cap). On document stores with a hard per-record
12+
// ceiling (e.g. MongoDB's 16 MB BSON limit) a near-cap clip plus other entry
13+
// fields can approach that ceiling; lower this if you log audio to such a store.
1114
const audioBodyMaxBytes = 10 * 1024 * 1024
1215

1316
// AudioBodyLog is the audit representation of an audio request/response body.

internal/auditlog/auditlog.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,7 @@ func DefaultConfig() Config {
261261
return Config{
262262
Enabled: false,
263263
LogBodies: false,
264+
LogAudioBodies: false,
264265
LogHeaders: false,
265266
BufferSize: 1000,
266267
FlushInterval: 5 * time.Second,

internal/auditlog/auditlog_test.go

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -535,6 +535,47 @@ func TestMiddleware_SkipsStreamingResponseWriterCapture(t *testing.T) {
535535
}
536536
}
537537

538+
// TestMiddleware_AudioResponseNotMarkedTruncated verifies that an oversized audio
539+
// response — which trips the response writer's truncation flag — does NOT set
540+
// ResponseBodyTooBigToHandle on the audit entry. The audio handler captures the
541+
// body losslessly via its own path, so a truncation flag here would produce
542+
// conflicting metadata (a fully-stored body alongside a "too big" marker).
543+
func TestMiddleware_AudioResponseNotMarkedTruncated(t *testing.T) {
544+
e := echo.New()
545+
logger := &capturingLogger{cfg: Config{Enabled: true, LogBodies: true}}
546+
547+
req := httptest.NewRequest(http.MethodPost, "/v1/audio/speech", strings.NewReader(`{"model":"gpt-4o-mini-tts","input":"hi","voice":"alloy"}`))
548+
rec := httptest.NewRecorder()
549+
c := e.NewContext(req, rec)
550+
551+
oversized := bytes.Repeat([]byte{0xff}, int(MaxBodyCapture)+16)
552+
var capture *responseBodyCapture
553+
handler := Middleware(logger)(func(c *echo.Context) error {
554+
capture, _ = c.Response().(*responseBodyCapture)
555+
c.Response().Header().Set("Content-Type", "audio/mpeg")
556+
c.Response().WriteHeader(http.StatusOK)
557+
_, err := c.Response().Write(oversized)
558+
return err
559+
})
560+
561+
if err := handler(c); err != nil {
562+
t.Fatalf("handler returned error: %v", err)
563+
}
564+
if capture == nil || !capture.truncated {
565+
t.Fatal("expected the response writer to mark the oversized body as truncated")
566+
}
567+
if len(logger.entries) != 1 {
568+
t.Fatalf("len(entries) = %d, want 1", len(logger.entries))
569+
}
570+
entry := logger.entries[0]
571+
if entry.Data != nil && entry.Data.ResponseBodyTooBigToHandle {
572+
t.Error("audio response must not set ResponseBodyTooBigToHandle; the handler owns audio body capture")
573+
}
574+
if entry.Data != nil && entry.Data.ResponseBody != nil {
575+
t.Errorf("middleware must not store the audio response body, got %T", entry.Data.ResponseBody)
576+
}
577+
}
578+
538579
func TestMiddleware_PrefersWorkflowOverLegacyResolution(t *testing.T) {
539580
e := echo.New()
540581
logger := &capturingLogger{
@@ -920,7 +961,6 @@ func TestIsModelInteractionPath(t *testing.T) {
920961
{"batches prefix overmatch", "/v1/batcheship", false},
921962
{"audio speech", "/v1/audio/speech", true},
922963
{"audio transcriptions", "/v1/audio/transcriptions", true},
923-
{"audio translations", "/v1/audio/translations", true},
924964
{"models", "/v1/models", false},
925965
{"models with subpath", "/v1/models/gpt-4", false},
926966
{"health", "/health", false},

internal/auditlog/middleware.go

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -135,18 +135,20 @@ func Middleware(logger LoggerInterface) echo.MiddlewareFunc {
135135

136136
// Capture response body if enabled
137137
if cfg.LogBodies && responseCapture != nil && shouldCaptureResponseBody(c) && responseCapture.body.Len() > 0 {
138-
// Set truncation flag if response body exceeded limit
139-
if responseCapture.truncated {
140-
entry.Data.ResponseBodyTooBigToHandle = true
141-
}
142-
143138
bodyBytes := responseCapture.body.Bytes()
144139

145140
// Audio responses are binary; the audio handler captures them
146141
// losslessly as base64 (gated by LogAudioBodies) before this
147142
// runs. Skip here so we neither corrupt the bytes via UTF-8
148-
// coercion nor clobber the handler-set body.
143+
// coercion nor clobber the handler-set body — and do not apply
144+
// the writer's truncation flag, which would conflict with the
145+
// fully-stored audio body (the handler tracks its own size cap).
149146
if !IsAudioContentType(c.Response().Header().Get("Content-Type")) {
147+
// Set truncation flag if response body exceeded limit
148+
if responseCapture.truncated {
149+
entry.Data.ResponseBodyTooBigToHandle = true
150+
}
151+
150152
// Decompress if Content-Encoding header is present
151153
if contentEncoding := c.Response().Header().Get("Content-Encoding"); contentEncoding != "" {
152154
if decompressed, ok := decompressBody(bodyBytes, contentEncoding); ok {

internal/core/endpoints.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ func describeEndpointPath(path string) EndpointDescriptor {
124124
Dialect: "openai_compat",
125125
Operation: OperationAudioSpeech,
126126
}
127-
case path == "/v1/audio/transcriptions" || path == "/v1/audio/translations":
127+
case path == "/v1/audio/transcriptions":
128128
return EndpointDescriptor{
129129
ModelInteraction: true,
130130
Dialect: "openai_compat",

internal/core/endpoints_test.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ func TestDescribeEndpointPath(t *testing.T) {
2525
{path: "/v1/files/file_1", managed: true, dialect: "openai_compat", operation: OperationFiles, bodyMode: BodyModeNone, interaction: true},
2626
{path: "/v1/audio/speech", managed: false, dialect: "openai_compat", operation: OperationAudioSpeech, bodyMode: BodyModeJSON, interaction: true},
2727
{path: "/v1/audio/transcriptions", managed: false, dialect: "openai_compat", operation: OperationAudioTranscriptions, bodyMode: BodyModeMultipart, interaction: true},
28-
{path: "/v1/audio/translations", managed: false, dialect: "openai_compat", operation: OperationAudioTranscriptions, bodyMode: BodyModeMultipart, interaction: true},
2928
{path: "/p/openai/responses", managed: true, dialect: "provider_passthrough", operation: OperationProviderPassthrough, bodyMode: BodyModeOpaque, interaction: true},
3029
{path: "/v1/models", managed: false, dialect: "", operation: "", bodyMode: BodyModeNone, interaction: false},
3130
}
@@ -77,7 +76,6 @@ func TestDescribeEndpoint_UsesMethodForBodyMode(t *testing.T) {
7776
{method: http.MethodGet, path: "/v1/files/file_1", bodyMode: BodyModeNone},
7877
{method: http.MethodPost, path: "/v1/audio/speech", bodyMode: BodyModeJSON},
7978
{method: http.MethodPost, path: "/v1/audio/transcriptions", bodyMode: BodyModeMultipart},
80-
{method: http.MethodPost, path: "/v1/audio/translations", bodyMode: BodyModeMultipart},
8179
{method: http.MethodPost, path: "/v1/batches/batch_1/cancel", bodyMode: BodyModeNone},
8280
}
8381

0 commit comments

Comments
 (0)