Add authed STT proxy route POST /v1/stt/transcribe (#8854 step 1) - #9011
Conversation
There was a problem hiding this comment.
2 issues found across 9 files
Confidence score: 4/5
- In
docs/doc/developer/backend/transcription.mdx, the/v1/stt/transcribeerror table is missing401and429, so integrators may mis-handle auth failures or throttling even though those cases are expected—add both responses to the documented error list before merging. - In
backend/routers/stt.py,_MAX_UPLOAD_BYTESduplicates the upload-size policy already defined inrouters/chat.py, which can drift over time and create inconsistent limits across endpoints—centralize this limit in one shared constant (or import the existing one) before merging.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="backend/routers/stt.py">
<violation number="1" location="backend/routers/stt.py:31">
P2: This new `_MAX_UPLOAD_BYTES` constant duplicates the existing `_MAX_PCM_BODY_BYTES = 200_000_000` policy from `routers/chat.py` for the same purpose. The inline comment even acknowledges the mirroring, which is a sign the duplication should probably be centralized. Consider extracting the shared upload-size limit into a common constants module so both routers import the same value, avoiding configuration drift if the limit ever changes.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
|
||
| # Mirrors _MAX_PCM_BODY_BYTES in routers/chat.py. Parakeet enforces its own | ||
| # duration cap (PARAKEET_MAX_FILE_DURATION); this only bounds backend memory. | ||
| _MAX_UPLOAD_BYTES = 200_000_000 |
There was a problem hiding this comment.
P2: This new _MAX_UPLOAD_BYTES constant duplicates the existing _MAX_PCM_BODY_BYTES = 200_000_000 policy from routers/chat.py for the same purpose. The inline comment even acknowledges the mirroring, which is a sign the duplication should probably be centralized. Consider extracting the shared upload-size limit into a common constants module so both routers import the same value, avoiding configuration drift if the limit ever changes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/routers/stt.py, line 31:
<comment>This new `_MAX_UPLOAD_BYTES` constant duplicates the existing `_MAX_PCM_BODY_BYTES = 200_000_000` policy from `routers/chat.py` for the same purpose. The inline comment even acknowledges the mirroring, which is a sign the duplication should probably be centralized. Consider extracting the shared upload-size limit into a common constants module so both routers import the same value, avoiding configuration drift if the limit ever changes.</comment>
<file context>
@@ -0,0 +1,139 @@
+
+# Mirrors _MAX_PCM_BODY_BYTES in routers/chat.py. Parakeet enforces its own
+# duration cap (PARAKEET_MAX_FILE_DURATION); this only bounds backend memory.
+_MAX_UPLOAD_BYTES = 200_000_000
+
+# How long a request may wait for an upstream slot before failing fast with
</file context>
Review feedback on BasedHardware#9011: the error contract advertised bearer auth and per-user rate limiting but omitted 401 and 429 (and the 400 empty-file case) from the error list. Matches the house style in docs/doc/developer/api/overview.mdx, which documents both for the authed Developer API.
Git-on-my-level
left a comment
There was a problem hiding this comment.
Thanks @K3N4Y — this is a strong, well-thought-out security improvement. I verified the core claim against the codebase: backend/parakeet/main.py's /v2/transcribe has no auth guard (no Depends/Authorization), and the existing internal caller (utils/stt/pre_recorded.py) hits it with no auth headers, so putting Omi auth + per-UID rate limiting in front of it is the right direction and aligns with issue #8854 step 1.
What I liked:
- Resource isolation: dedicated
get_stt_proxy_client()(4 conns, keep-alive off) +get_stt_proxy_semaphore()so bulk user uploads can't starve the listen pipeline's latency-sensitive internal STT pool — correct Lane 1 discipline. - Bounded RAM residency: the body is only buffered after a slot is held (max 4 resident bodies); waiters stay in starlette's disk spool.
- Triple size enforcement:
Content-Length+file.size+ bounded read as defense-in-depth, including a test for the chunked / no-Content-Lengthcase. - Filename sanitization: path separators, dot-prefixed, and overlong names never reach parakeet's temp path.
- Clean error mapping: only client-actionable 413/503 are forwarded; everything else becomes a generic 502 with no upstream body leakage.
- Comprehensive tests (auth, config guard, payload validation, chunked uploads, filename sanitization, slot exhaustion, passthrough, error mapping).
Not formally approving — this needs a human maintainer's call because it touches auth, model/provider routing, and a 200MB user-upload surface, and it's the first step of a larger plan (#8854). Flagging two minor items (also noted by cubic):
docs/.../transcription.mdxerror table omits401and429._MAX_UPLOAD_BYTESduplicates_MAX_PCM_BODY_BYTESfromrouters/chat.py— worth centralizing into a shared constant.
On the AGENTS.md / backend/AGENTS.md changes: they look accurate and are a helpful steer for future coding/review agents (clients must use the authed proxy; the new get_stt_proxy_client() is listed in Lane 1). The "clients must not call parakeet directly" line formalizes a product decision a maintainer should confirm before it becomes agent guidance. Tagging security-review, needs-maintainer-review, docs-accuracy for a maintainer.
kodjima33
left a comment
There was a problem hiding this comment.
Authed STT proxy route — backend feature/new capability. Approve only.
|
Re-review note (automated, GLM-5.2):
Not formally approving — this is step 1 of a multi-step plan and touches auth, provider/model routing, and a 200MB user-upload surface, so it needs a human maintainer's call before merge. Flagging for a gpt-5.5 security/auth pass as well. Thanks @K3N4Y for a well-thought-out security improvement. |
58f10f7 to
74f5ded
Compare
Clients currently reach the parakeet GPU service directly at parakeet.omiapi.com with no auth in front of it. This fronts parakeet's /v2/transcribe with the standard Omi auth guard plus per-UID rate limiting, so the mobile app's "Omi Parakeet" provider can migrate off the direct URL — step 1 of the parakeet cleanup plan in BasedHardware#8854. - routers/stt.py: async proxy via the shared STT httpx client and semaphore, 200MB body cap, upstream filename sanitization, 413/503 detail forwarding, other upstream failures mapped to 502 - rate_limit_config.py: stt:transcribe policy (60/h, mirrors voice:transcribe) - tests/unit/test_stt_router.py: 19 unit tests, registered in test.sh - AGENTS.md: parakeet service-map note about the authed proxy
Fixes from an independent agent review of the new /v1/stt/transcribe route: - Bound memory for chunked uploads: the Content-Length pre-check is skipped when the header is absent, and file.read() loaded the whole spooled body into RAM before the size check. Read at most _MAX_UPLOAD_BYTES + 1 bytes instead, and pre-check file.size too. - Isolate the proxy from the listen pipeline: dedicated get_stt_proxy_client() / get_stt_proxy_semaphore() (4 connections) so bulk user uploads can't starve VAD / speaker-embedding / speech profile calls sharing the internal STT pool; fail fast with 503 when no upstream slot frees up within 30s. - Give the route a 330s path timeout (HTTP_STT_TRANSCRIBE_TIMEOUT): the default 120s POST timeout would kill long transcriptions that the 300s upstream budget allows. - Don't 500 on non-dict JSON error bodies from an intermediate LB; coerce forwarded detail to str. - Truncate forwarded filenames to 64 chars (parakeet embeds them in its temp path; overlong names ENAMETOOLONG into an opaque 502). - Move the stt:transcribe rate policy out of the voice block. - Tests for all of the above (25 now), including a chunked-body upload that pins the bounded-read enforcement. - Document the endpoint in docs/doc/developer/backend/transcription.mdx.
- Buffer the upload only after acquiring an upstream slot: resident RAM is now capped at 4 concurrent bodies; waiters keep theirs in starlette's disk spool. Previously every waiter pinned up to 200MB. - Fix the size-enforcement attribution: starlette populates file.size for every multipart part (chunked included), so that pre-check is the primary cap. The bounded read stays as defense-in-depth and is now genuinely pinned by a direct-call test with size unavailable. - Disable keep-alive on get_stt_proxy_client (same rationale as the auth/tts clients): a stale kept-alive socket surfaces as RuntimeError (500) instead of a clean 502; per-request handshake is noise next to GPU transcription time. - Bump the route timeout 330s -> 350s for auth/spool-read headroom over the 30s slot wait + 300s upstream budget. - Preserve short file extensions when truncating forwarded filenames. - Fix http_client.py header docstring (no longer 4 clients) and list all Lane-1 clients in backend/AGENTS.md. Tests: 28 unit tests pass, async/import-purity scanners clean, E2E happy path + auth rejection re-verified against the reordered flow.
Review feedback on BasedHardware#9011: the error contract advertised bearer auth and per-user rate limiting but omitted 401 and 429 (and the 400 empty-file case) from the error list. Matches the house style in docs/doc/developer/api/overview.mdx, which documents both for the authed Developer API.
Rebase port of the PR's original AGENTS.md service-map note onto the new lean backend/AGENTS.md structure.
74f5ded to
8c435a8
Compare
…cy manifest Adds POST /v1/stt/transcribe with firebase auth, the stt:transcribe rate limit, and the path-timeout override so the Public Developer API contract and Hygiene manifest checks pass.
What
Adds
POST /v1/stt/transcribe— an authenticated backend proxy in front of theparakeet GPU service's
/v2/transcribe. This is step 1 of the parakeet cleanupplan laid out in #8854 ("Add new authed backend proxy route for mobile app").
Why
When a user selects the "Omi Parakeet" STT provider, the mobile app currently
posts audio buffers directly to
parakeet.omiapi.com/v1/transcribewith noauthentication of any kind (
app/lib/models/stt_provider.dart:533builds therequest with no Authorization header, and the parakeet service has no auth
guard of its own). That leaves a GPU transcription service effectively open to
the public at that path.
This route puts the standard Omi stack in front of it:
get_current_user_uid(Firebase ID token), same dependency as every other authed endpoint.stt:transcribepolicy (60/h per UID, mirrorsvoice:transcribe).get_stt_proxy_client()/get_stt_proxy_semaphore()(4 connections, keep-alive off) so bulk uploads can never starve the listen
pipeline's internal STT pool (VAD, speaker embedding, speech profile). Requests
fail fast with 503 if no upstream slot frees up within 30s, and the body is only
buffered into RAM after a slot is held (max 4 resident bodies; waiters stay in
the multipart disk spool).
file.sizepre-checks, plus abounded read as defense-in-depth), and upstream filename sanitization — parakeet
builds its temp path from the client filename, so path separators, dot-prefixed
and overlong names are never forwarded.
(model loading / overloaded) are forwarded with their detail; any other upstream
failure maps to a generic 502 with nothing leaked.
HTTP_STT_TRANSCRIBE_TIMEOUT) to cover the 300supstream budget; the 120s POST default would kill long transcriptions.
The response mirrors parakeet
/v2/transcribeverbatim(
{"text", "segments", "detected_language"}), so migrating the app is aURL + auth-header swap.
Follow-ups (out of scope, per the #8854 sequencing)
omiParakeetprovider to this route — needs a dynamic authheader (Firebase tokens expire hourly;
SchemaBasedSttProvidertakes staticheaders at socket creation), so it's a separate app-side change.
/v1/transcribeexposure after the soak period.Testing
backend/tests/unit/test_stt_router.py, registered intest.sh):auth required, config guard, empty/oversized payloads (including a chunked body
with no Content-Length and a direct-call test pinning the bounded read when
file.sizeis unavailable), filename sanitization matrix, slot exhaustion → 503,response passthrough,
diarizeforwarding, upstream 413/503 forwarded (dict,non-dict JSON and HTML bodies), upstream 500 → 502 with no body leak, network
error → 502.
and exercised it with
curl+ a real WAV — 401 without auth, 200 with exactJSON passthrough,
diarize=falseforwarded, multipart filename../../etc/evil.wavarriving upstream asevil.wav.backend/test.shrun;scan_async_blockers.pyandscan_import_time_side_effects.pyclean.Refs #8854