Skip to content

Commit 155c278

Browse files
authored
fix(assistant): normalize avatar storage and identity (#558)
## Summary - Store user assistant avatars as managed filenames instead of local absolute paths, and repair legacy local path rows at startup. - Resolve conversation/team/cron assistant identity from current assistant definitions instead of mutable snapshot avatar/name fields. - Serve assistant avatar routes from managed definition values with cache-busting response URLs and no directory scan fallback. ## Related PRs - Frontend rendering changes: iOfficeAI/AionUi#3493 ## Test Plan - cargo fmt --all - cargo test -p aionui-app --test assistants_e2e avatar - cargo test -p aionui-conversation create_routes_asset_avatar_in_assistant_identity_through_backend - cargo test -p aionui-team tc_create_team_prefers_assistant_avatar_over_backend_logo - just push -u origin fix/assistant-avatar-source Co-authored-by: zk <>
1 parent 41c2c94 commit 155c278

18 files changed

Lines changed: 1629 additions & 577 deletions

File tree

crates/aionui-api-types/src/assistant.rs

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,62 @@ pub struct AssistantDetailResponse {
215215
pub preferences: AssistantPreferencesResponse,
216216
}
217217

218+
pub fn assistant_avatar_response_value(
219+
avatar_type: &str,
220+
avatar_value: Option<&str>,
221+
assistant_id: &str,
222+
) -> Option<String> {
223+
if matches!(avatar_type, "builtin_asset" | "user_asset") {
224+
return Some(format!("/api/assistants/{assistant_id}/avatar"));
225+
}
226+
227+
let value = avatar_value.map(str::trim).filter(|value| !value.is_empty())?;
228+
229+
match avatar_type {
230+
_ if is_unsupported_direct_avatar_value(value) => None,
231+
_ if is_local_avatar_value(value) => None,
232+
_ => Some(value.to_owned()),
233+
}
234+
}
235+
236+
pub fn assistant_avatar_response_value_with_version(
237+
avatar_type: &str,
238+
avatar_value: Option<&str>,
239+
assistant_id: &str,
240+
version: i64,
241+
) -> Option<String> {
242+
if matches!(avatar_type, "builtin_asset" | "user_asset") {
243+
return Some(format!("/api/assistants/{assistant_id}/avatar?v={version}"));
244+
}
245+
246+
assistant_avatar_response_value(avatar_type, avatar_value, assistant_id)
247+
}
248+
249+
pub fn is_local_avatar_value(value: &str) -> bool {
250+
let value = value.trim();
251+
if value.is_empty() {
252+
return false;
253+
}
254+
if value.starts_with("file://") {
255+
return true;
256+
}
257+
if value.starts_with("/api/") || value.starts_with("/assets/") {
258+
return false;
259+
}
260+
if value.starts_with("//") || value.contains("://") || value.starts_with("data:") {
261+
return false;
262+
}
263+
if value.as_bytes().get(1) == Some(&b':') && matches!(value.as_bytes().first(), Some(b'A'..=b'Z' | b'a'..=b'z')) {
264+
return true;
265+
}
266+
std::path::Path::new(value).is_absolute()
267+
}
268+
269+
fn is_unsupported_direct_avatar_value(value: &str) -> bool {
270+
let value = value.trim().to_ascii_lowercase();
271+
value.starts_with("http://") || value.starts_with("https://") || value.starts_with("data:")
272+
}
273+
218274
// ---------------------------------------------------------------------------
219275
// Request types
220276
// ---------------------------------------------------------------------------
@@ -345,6 +401,61 @@ mod tests {
345401
assert!(parsed.is_err());
346402
}
347403

404+
#[test]
405+
fn assistant_avatar_response_value_routes_asset_values_through_backend() {
406+
assert_eq!(
407+
assistant_avatar_response_value("user_asset", Some("data:image/svg+xml;base64,abc"), "custom-1").as_deref(),
408+
Some("/api/assistants/custom-1/avatar")
409+
);
410+
assert_eq!(
411+
assistant_avatar_response_value("user_asset", None, "custom-1").as_deref(),
412+
Some("/api/assistants/custom-1/avatar")
413+
);
414+
assert_eq!(
415+
assistant_avatar_response_value("user_asset", Some("https://example.invalid/avatar.png"), "custom-1")
416+
.as_deref(),
417+
Some("/api/assistants/custom-1/avatar")
418+
);
419+
}
420+
421+
#[test]
422+
fn assistant_avatar_response_value_with_version_routes_asset_values_through_backend() {
423+
assert_eq!(
424+
assistant_avatar_response_value_with_version("user_asset", Some("custom-1.png"), "custom-1", 1782714544060)
425+
.as_deref(),
426+
Some("/api/assistants/custom-1/avatar?v=1782714544060")
427+
);
428+
assert_eq!(
429+
assistant_avatar_response_value_with_version("emoji", Some("🧠"), "custom-1", 1782714544060).as_deref(),
430+
Some("🧠")
431+
);
432+
}
433+
434+
#[test]
435+
fn assistant_avatar_response_value_never_exposes_local_paths() {
436+
assert_eq!(
437+
assistant_avatar_response_value(
438+
"user_asset",
439+
Some("/Users/veryliu/.aionui/assistant-avatars/custom-1.jpg"),
440+
"custom-1",
441+
)
442+
.as_deref(),
443+
Some("/api/assistants/custom-1/avatar")
444+
);
445+
assert_eq!(
446+
assistant_avatar_response_value(
447+
"emoji",
448+
Some("file:///Users/veryliu/.aionui/assistant-avatars/custom-1.jpg"),
449+
"custom-1",
450+
),
451+
None
452+
);
453+
assert_eq!(
454+
assistant_avatar_response_value("emoji", Some("https://example.invalid/avatar.png"), "custom-1"),
455+
None
456+
);
457+
}
458+
348459
#[test]
349460
fn assistant_response_round_trip_snake_case() {
350461
let resp = AssistantResponse {

crates/aionui-api-types/src/lib.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,8 @@ pub use assistant::{
5656
AssistantDetailResponse, AssistantEngineResponse, AssistantPreferencesResponse, AssistantProfileResponse,
5757
AssistantPromptsResponse, AssistantResponse, AssistantRulesResponse, AssistantSource, AssistantStateResponse,
5858
CreateAssistantRequest, ImportAssistantsRequest, ImportAssistantsResult, ImportError, SetAssistantStateRequest,
59-
UpdateAssistantRequest,
59+
UpdateAssistantRequest, assistant_avatar_response_value, assistant_avatar_response_value_with_version,
60+
is_local_avatar_value,
6061
};
6162
pub use auth::{
6263
AuthStatusResponse, ChangePasswordRequest, LoginRequest, LoginResponse, PublicUser, QrLoginRequest,

crates/aionui-app/tests/assistants_e2e.rs

Lines changed: 25 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,24 @@ fn test_agent_row(id: &str, backend: Option<&str>, agent_type: AgentType, name:
117117
}
118118
}
119119

120+
fn assert_versioned_avatar_route(body: &Value, expected_path: &str) {
121+
assert_versioned_avatar_value(body["data"]["avatar"].as_str(), expected_path);
122+
}
123+
124+
fn assert_versioned_avatar_value(value: Option<&str>, expected_path: &str) {
125+
let avatar = value.expect("avatar must be a string");
126+
let (path, version) = avatar
127+
.split_once("?v=")
128+
.expect("assistant avatar route must include cache-busting version");
129+
130+
assert_eq!(path, expected_path);
131+
assert!(!version.is_empty(), "avatar route version must not be empty");
132+
assert!(
133+
version.chars().all(|ch| ch.is_ascii_digit()),
134+
"avatar route version must be numeric: {version}"
135+
);
136+
}
137+
120138
async fn insert_generated_bare_assistant(
121139
fx: &Fixture,
122140
assistant_id: &str,
@@ -449,9 +467,9 @@ async fn list_builtin_file_avatar_is_served_via_assistant_avatar_route() {
449467
.find(|assistant| assistant["id"] == "builtin-office")
450468
.expect("builtin-office missing from assistant list");
451469

452-
assert_eq!(
470+
assert_versioned_avatar_value(
453471
builtin_office["avatar"].as_str(),
454-
Some("/api/assistants/builtin-office/avatar")
472+
"/api/assistants/builtin-office/avatar",
455473
);
456474
}
457475

@@ -736,7 +754,7 @@ async fn create_user_avatar_from_local_file_is_served_via_assistant_avatar_route
736754
let resp = fx.app.clone().oneshot(req).await.unwrap();
737755
assert_eq!(resp.status(), StatusCode::CREATED);
738756
let body = body_json(resp).await;
739-
assert_eq!(body["data"]["avatar"], "/api/assistants/u-avatar/avatar");
757+
assert_versioned_avatar_route(&body, "/api/assistants/u-avatar/avatar");
740758

741759
let persisted_avatar = fx.user_data_dir.join("assistant-avatars/u-avatar.png");
742760
assert!(
@@ -783,7 +801,7 @@ async fn create_user_avatar_from_builtin_avatar_route_copies_builtin_asset() {
783801
let resp = fx.app.clone().oneshot(req).await.unwrap();
784802
assert_eq!(resp.status(), StatusCode::CREATED);
785803
let body = body_json(resp).await;
786-
assert_eq!(body["data"]["avatar"], "/api/assistants/u-avatar-from-builtin/avatar");
804+
assert_versioned_avatar_route(&body, "/api/assistants/u-avatar-from-builtin/avatar");
787805

788806
let persisted_avatar = fx.user_data_dir.join("assistant-avatars/u-avatar-from-builtin.png");
789807
assert!(
@@ -833,10 +851,7 @@ async fn create_user_avatar_from_absolute_builtin_avatar_route_copies_builtin_as
833851
let resp = fx.app.clone().oneshot(req).await.unwrap();
834852
assert_eq!(resp.status(), StatusCode::CREATED);
835853
let body = body_json(resp).await;
836-
assert_eq!(
837-
body["data"]["avatar"],
838-
"/api/assistants/u-avatar-from-builtin-absolute/avatar"
839-
);
854+
assert_versioned_avatar_route(&body, "/api/assistants/u-avatar-from-builtin-absolute/avatar");
840855

841856
let persisted_avatar = fx
842857
.user_data_dir
@@ -1270,7 +1285,7 @@ async fn avatar_builtin_returns_bytes_with_content_type() {
12701285
}
12711286

12721287
#[tokio::test]
1273-
async fn avatar_user_returns_bytes_after_file_planted() {
1288+
async fn avatar_user_ignores_planted_file_without_managed_value() {
12741289
let fx = fixture().await;
12751290
create_user(&fx, "u1", "A").await;
12761291
let avatars_dir = fx.user_data_dir.join("assistant-avatars");
@@ -1283,11 +1298,7 @@ async fn avatar_user_returns_bytes_after_file_planted() {
12831298
.oneshot(get_with_token("/api/assistants/u1/avatar", &fx.token))
12841299
.await
12851300
.unwrap();
1286-
assert_eq!(resp.status(), StatusCode::OK);
1287-
assert_eq!(
1288-
resp.headers().get("content-type").and_then(|v| v.to_str().ok()),
1289-
Some("image/svg+xml")
1290-
);
1301+
assert_eq!(resp.status(), StatusCode::NOT_FOUND);
12911302
}
12921303

12931304
#[tokio::test]

crates/aionui-assistant/src/routes.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,7 @@ async fn get_avatar(State(state): State<AssistantRouterState>, Path(id): Path<St
124124
Response::builder()
125125
.status(StatusCode::OK)
126126
.header(header::CONTENT_TYPE, content_type)
127+
.header(header::CACHE_CONTROL, "no-store")
127128
.body(Body::from(asset.bytes))
128129
.map_err(|e| ApiError::Internal(e.to_string()))
129130
}

0 commit comments

Comments
 (0)