Skip to content

fix(audio): sanitize upstream error bodies on /v1/audio/translations - #17844

Merged
diegosouzapw merged 3 commits into
cheaper-inference:release/v3.8.53from
AndresDiagoM:fix/audio-translations-upstream-error-sanitize
Oct 26, 2026
Merged

diegosouzapw merged 3 commits into
cheaper-inference:release/v3.8.53from
AndresDiagoM:fix/audio-translations-upstream-error-sanitize

Conversation

@AndresDiagoM

Copy link
Copy Markdown
Contributor

⚠️ base-red inherited: #17219

Closes #17843.

Summary

  • open-sse/handlers/audioTranslation.ts returned errorResponse(res.status, extractUpstreamErrorMessage(errText, res.status)). That local helper passed errText through verbatim whenever the body wasn't JSON or had no message field, so an HTML error page, stack trace or echoed credential reached the /v1/audio/translations client, and the error response had no CORS headers.
  • Fix: use the shared upstreamErrorResponse(res, errText) from open-sse/utils/audioResponse.ts, as audioSpeech.ts and audioTranscription.ts have since fix(backend): whole-repository audit findings for v3.8.52 — 78 items (14 critical · 21 high · 29 medium · 14 keep) #15159, and delete the local helper. The handler shrinks by 16 lines.

Client-visible change

  • Opaque body (HTML / plaintext): the client gets Upstream error (N), no longer the provider's text.
  • JSON body: the provider's own message (error.message, a string error, or message) is kept after sanitizeErrorMessage. Plain prose such as "too many requests" is unchanged. A JSON body with no message field now gives Upstream error (N) where it used to return the raw JSON text.
  • CORS headers are now set on upstream error responses.
  • Success paths are untouched.

Validation (TDD)

New tests/unit/audio-translations-upstream-error-sanitization.test.ts (2 tests, modelled on audio-upstream-error-sanitization-15159.test.ts). Run against the original handler:

✖ translation: an opaque HTML upstream error is not echoed to the client
  AssertionError: echoed the HTML body: {"error":{"message":"<html><body><pre>Error: boom",...}}
✖ translation: a JSON upstream error keeps its reason and carries CORS
  AssertionError: lost CORS on error response

With the fix, all tests/unit/audio-*.test.ts pass, 152/152, including the existing audio-translations-route.test.ts JSON 429 case and the #15159 suite. typecheck:core, eslint on the changed files, check-file-size, and check-complexity-ratchets --base-ref origin/release/v3.8.53 (new-code mode, both OK) all pass.

cheaper-inference#15159 moved audioSpeech and audioTranscription onto the shared upstreamErrorResponse helper, but the translation handler kept a local extractUpstreamErrorMessage that returned the raw upstream body when it was not JSON. An HTML error page, stack trace or echoed token therefore reached the client, and the error response carried no CORS headers. Use upstreamErrorResponse and drop the local helper.
@diegosouzapw

Copy link
Copy Markdown
Collaborator

Thanks a lot @AndresDiagoM, this is a clean, well-scoped fix. You spotted exactly the sibling that #15159 missed, and routing /v1/audio/translations through the shared upstreamErrorResponse (and dropping the local extractUpstreamErrorMessage) is the right call: opaque HTML or plaintext bodies now become Upstream error (N), JSON reasons are kept after sanitization, and CORS is back on error responses. I also appreciate that the tests are modelled on the #15159 suite and plant a credential, an absolute path and an HTML body to prove nothing leaks (Hard Rule 12). I verified the red/green myself on head 27ca64e: with the fix both new tests pass, and with only the handler hunk reverted both fail (echoed the HTML body and lost CORS on error response); the existing audio-translations-route tests (8/8) also pass.

What we will take care of in-place on your branch (you keep full credit): adding the changelog fragment changelog.d/fixes/17844-audio-translations-upstream-error-sanitization.md, which is the one thing missing per changelog.d/README.md. Nothing is needed from you, unless you would like to add it yourself, in which case a one-line - **fix(audio):** ... (#17844 — thanks @AndresDiagoM) bullet is all it takes.

One optional follow-up for a separate PR if you are interested: the fetch-failure catch in audioTranslation.ts and audioTranscription.ts still returns the 500 without CORS headers (the message itself is already sanitized by errorResponse). Not a blocker here. CI on this head was cancelled/superseded rather than red, and the base is currently red per #17219, so we will re-run before landing. Thanks again for the contribution!

@AndresDiagoM
AndresDiagoM force-pushed the fix/audio-translations-upstream-error-sanitize branch from 5da1890 to c29a6eb Compare October 22, 2026 13:39
@AndresDiagoM

Copy link
Copy Markdown
Contributor Author

Thanks for the detailed verification.

The fragment was already on the branch (changelog.d/fixes/17844-audio-translations-upstream-error-sanitize.md, pushed right after opening the PR), but GitHub never synced that push onto the PR, so the file list stayed at the fix commit. I re-pushed the fragment commit with a fresh SHA (c29a6eb59a) and the PR now shows it, so nothing to add on your side. The same thing had happened on #17842; both are resynced now.

On the fetch-failure catch in audioTranslation.ts / audioTranscription.ts returning 500 without CORS: happy to take that as a separate PR.

…lations

Carried over from cheaper-inference#17941: a JSON upstream message with a credential and a
server path loses both but keeps its reason.

Co-authored-by: skygunner <84759+skygunner@users.noreply.github.com>
@AndresDiagoM

Copy link
Copy Markdown
Contributor Author

@diegosouzapw I carried the hostile-JSON case from #17941 over myself in 8c523be (Co-authored-by skygunner), so there's nothing left to do there. Skygunner's safe-JSON 429 case was already covered by the existing "keeps its reason and carries CORS" test, so I didn't add it twice. The changelog fragment is unchanged, so there's still only one entry.

@diegosouzapw
diegosouzapw merged commit b423973 into cheaper-inference:release/v3.8.53 Oct 26, 2026
42 of 57 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(api): /v1/audio/translations echoes opaque upstream error bodies and drops CORS (missed by #15159)

2 participants