fix(ai): never send a control character in a custom inference model slug - #14794
Draft
warp-agent-staging[bot] wants to merge 1 commit into
Draft
fix(ai): never send a control character in a custom inference model slug#14794warp-agent-staging[bot] wants to merge 1 commit into
warp-agent-staging[bot] wants to merge 1 commit into
Conversation
Filling in the optional "Model alias" on a Custom Inference endpoint could
result in the request carrying a model name with a trailing carriage return
(`glm-5.2:cloud\r`), which the provider rejects as an unknown model.
Two things combine to produce this:
- `ApiKeyManager::custom_model_providers_for_request` forwarded
`CustomEndpointModel.name` to the wire as `CustomModel.slug` verbatim, so
anything in the stored name reaches the provider.
- winit reports `"\r"` as `KeyEvent::text` for Enter. When the corresponding
key event goes unhandled, the winit event loop falls back to dispatching
`TypedCharacters { chars }`, and `EditorView::user_insert` inserts it into
the focused buffer verbatim. Pressing Enter to move from "Model name" to
"Model alias" therefore leaves an invisible `\r` in the name. macOS is
unaffected because Cocoa routes those keys to `doCommandBySelector:`, which
is a no-op for us.
Fixes it on both sides:
- Sanitize the model identifier (strip control characters, trim surrounding
whitespace) when building the request payload and when persisting an
endpoint, so already-corrupted configurations heal on the next request.
- Single-line editors now drop typed line breaks, since a single-line buffer
can never legitimately hold one.
Fixes #14782
Co-Authored-By: Warp Agent <agent@warp.dev>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
On a Custom Inference endpoint, filling in the optional Model alias could cause the request to carry a model name with a trailing carriage return (
glm-5.2:cloud\r). The provider then rejects it as an unknown model. The character is invisible everywhere it is displayed and is not whitespace, so atrim()-style guard passes it through.Root cause
Two separate things combine. I verified both in the code; the reporter's CRLF-serialization hypothesis turned out to be wrong (secure storage writes raw bytes on all platforms and does no line-ending translation).
1. The wire payload was never sanitized.
ApiKeyManager::custom_model_providers_for_request()incrates/ai/src/api_keys.rsforwardedCustomEndpointModel.nametoCustomModel.slugverbatim, so anything in the stored name reached the provider unchanged.2. A
\rcan get into the field in the first place, via key input — not serialization.In
crates/warpui/src/windowing/winit/event_loop/mod.rs(~L1299–1321), the winit event loop capturesKeyEvent::textand, when the resultingKeyDownis not handled, falls back to dispatchingTypedCharacters { chars }. winit reports"\r"as the text for Enter.EditorElement::typed_characters(app/src/editor/view/element.rs:615) routes that toEditorView::user_insert, which inserted it into the focused buffer with no control-character filtering.That matches the reported symptom precisely: the corruption only appears once the user fills in the alias, because the natural way to get from Model name to Model alias is to press Enter, which leaves the stray
\rbehind in the name field before focus moves on.It also explains why this is filed as Windows-only: on macOS, Cocoa routes Enter/Tab/etc. to
doCommandBySelector:, which is a deliberate no-op incrates/warpui/src/platform/mac/objc/host_view.m:420, so control characters never reachinsertText:. The same winit path exists on Linux, so this most likely reproduces there too — I was not able to exercise either platform's GUI to confirm (see Testing below).Changes
crates/ai/src/api_keys.rssanitize_custom_model_field()strips Unicode control characters and trims surrounding whitespace.CustomEndpointModel::request_slug()is the sanitized name, andcustom_model_providers_for_request()now uses it. Models whose name sanitizes to nothing are skipped, as empty names already were.build_custom_endpoint), so nothing dirty is persisted going forward. Sanitizing on the request path as well means an already-corrupted configuration heals on the next request without the user having to re-edit anything.app/src/editor/view/mod.rsEditorView::user_insert()drops\r/\nfrom typed text when the editor is single-line. A single-line buffer can never legitimately hold a line break, so this closes the entry point for every single-line settings field, not just this one. Paste (user_initiated_insert) is deliberately left alone.The alias remains a display-only label; nothing about it reaches the wire.
Linked Issue
Fixes #14782
factory-auto-implement.Testing
Automated tests added:
crates/ai/src/api_keys_tests.rs(5 new)custom_model_slug_strips_trailing_carriage_return— the exact reported case: nameglm-5.2:cloud\rwith an alias set must serialize asglm-5.2:cloud, and the slug must contain no control characters.custom_model_slug_strips_control_characters_and_surrounding_whitespace—\r\n, tabs,DEL(U+007F) andESC(U+001B).models_that_sanitize_to_nothing_are_skippedrequest_slug_leaves_clean_names_untouchedsaved_custom_endpoint_models_are_sanitized— coversadd_custom_endpoint/save_custom_endpointpersisting clean values, including an alias that is nothing but control characters being dropped rather than persisted as a blank picker label.app/src/editor/view/mod_tests.rs(3 new)test_single_line_editor_drops_typed_line_breakstest_single_line_editor_keeps_text_around_a_line_breaktest_multi_line_editor_still_accepts_typed_newlines(guards against over-reach)Exact commands and results:
The
warpbuilds above were run withCARGO_PROFILE_DEV_DEBUG=0 -j 4; linking the full test binary with debuginfo OOM-killed the sandbox.Honest notes on what is not verified
The 3 failures in the broad
ai:: settings_view::run are unrelated to this change, and I checked each against the unmodified tree:server::server_api::ai::tests::ambient_agent_headers_for_task_overrides_existing_cloud_agent_header— confirmed failing on the unmodified tree. It shells out tonscand gets an access-denied on an ambient agent workload token; it needs credentials this sandbox does not have.ai::agent_sdk::api_key::tests::resolve_api_key_identifier_errors_for_ambiguous_name_matches— confirmed failing on the unmodified tree. This is about Warp SDK API-key name resolution, an entirely differentapi_keymodule from the AI-provider keys touched here.ai::blocklist::block::secret_redaction::test::test_detect_secrets_no_regexes_configured— passes when run in isolation on the unmodified tree. It only failed inside the large sweep, so it looks order-dependent on shared secret-redaction regex state rather than caused by this PR. I did not re-run the fullai:: settings_view::sweep on the unmodified tree to prove that conclusively.No manual GUI testing. I could not run
./script/runin this environment, and I have no Windows machine, so the end-to-end repro (add a custom model, press Enter into the alias field, send a request) is unverified. The Windows-specific mechanism is inferred from the winit/Cocoa input paths cited above rather than observed.The data-layer sanitization is the part that is fully covered by tests and is sufficient on its own to fix the reported symptom, including for users whose stored configuration is already corrupted. The editor-side guard is the narrower root-cause fix and is covered by unit tests, but its real-world effect on Windows is not directly verified.
Endpoint URL and API key fields are not sanitized here. A control character in the URL fails the modal's existing
validate_urlcheck, and I did not want to widen the blast radius of an API-key value. Worth a follow-up if it turns out to bite.I have manually tested my changes locally with
./script/runAgent Mode
CHANGELOG-BUG-FIX: Fixed Custom Inference requests sending a model name with a stray carriage return when a Model alias was set, which caused providers to reject the model as not found.
Conversation: https://staging.warp.dev/conversation/b219d4d3-d14c-4f70-9600-2cf54f909452
Run: https://oz.staging.warp.dev/runs/019fd618-2f76-77aa-bf7f-64a3d6bc4b7e
This PR was generated with Oz.