Skip to content

fix(agent): neutralize : in cancellation registry keys to prevent delimiter collision#586

Open
chuenchen309 wants to merge 1 commit into
nashsu:mainfrom
chuenchen309:fix/agent-cancel-key-delimiter-collision
Open

fix(agent): neutralize : in cancellation registry keys to prevent delimiter collision#586
chuenchen309 wants to merge 1 commit into
nashsu:mainfrom
chuenchen309:fix/agent-cancel-key-delimiter-collision

Conversation

@chuenchen309

Copy link
Copy Markdown
Contributor

Problem

cancel_key(project_id, session_id, run_id) joins the three fields with a
literal "::" delimiter, but normalize_key() only neutralized path
separators (\ and /), not ::

fn normalize_key(value: &str) -> String {
    value.replace(['\\', '/'], "_")
}

session_id / run_id are unrestricted Option<String> fields (no charset
validation, unlike agent/session.rs's sanitize_session_id) and are
reachable from the local HTTP API's POST /projects/:id/chat body and the
agent_chat / agent_start_turn_stream Tauri commands. When a field contains
the substring "::", two distinct (session_id, run_id) pairs collapse onto
the same registry key — e.g. (session_id="a::b", run_id="c") and
(session_id="a", run_id="b::c") both key to "p1::a::b::c" — so cancelling
one turn cross-cancels an unrelated turn's cancellation token. The no-run_id
prefix-search path in cancel() has the same exposure.

Fix

Extend normalize_key to also replace :_, matching its existing
\ / /_ pattern, so the :: delimiter can no longer appear inside a
field value.

Test

Added cancellation_registry_does_not_collide_on_embedded_delimiter to the
existing #[cfg(test)] mod tests, asserting the two colliding pairs above stay
independent.

…elimiter collision

cancel_key() joins project_id/session_id/run_id with `::`, but normalize_key()
only escaped `\` and `/`. Since session_id/run_id are unrestricted strings
from the HTTP API body and Tauri commands, a `::`-containing id in one field
can collide with a shifted split across two fields — e.g. session_id "a::b" +
run_id "c" produces the same registry key as session_id "a" + run_id "b::c".
This cross-cancels or cross-drops an unrelated turn's cancellation token.

Verified against the real production cancel.rs (compiled + all 5 tests run
directly via rustc, working around this environment's missing GTK/pkg-config
system libs which block a full `cargo test` of the Tauri crate): the new
regression test fails pre-fix and passes post-fix, and the 4 pre-existing
tests are unaffected.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.

1 participant