diff --git a/.env.example b/.env.example index 1e69c6a0e..db5a7ea25 100644 --- a/.env.example +++ b/.env.example @@ -31,6 +31,8 @@ PGDATABASE=buzz # Redis 7 # ----------------------------------------------------------------------------- REDIS_URL=redis://localhost:6379 +# Max connections in the relay's shared Redis pool (default 16). +# BUZZ_REDIS_POOL_SIZE=16 # ----------------------------------------------------------------------------- # Typesense (search) diff --git a/.github/workflows/sprig.yml b/.github/workflows/sprig.yml index 97f54069e..b2dab3583 100644 --- a/.github/workflows/sprig.yml +++ b/.github/workflows/sprig.yml @@ -139,27 +139,10 @@ jobs: TITLE="Sprig (rolling)" NOTES="Rolling Linux build of Sprig (all-in-one buzz-acp + buzz-agent + buzz-dev-mcp), tracking \`main\` (\`${SHA}\`)." - if gh api "repos/${REPO}/git/refs/tags/${TAG}" >/dev/null 2>&1; then - gh api -X PATCH "repos/${REPO}/git/refs/tags/${TAG}" \ - -f sha="${SHA}" -F force=true >/dev/null - else - gh api -X POST "repos/${REPO}/git/refs" \ - -f ref="refs/tags/${TAG}" -f sha="${SHA}" >/dev/null - fi - - if ! gh release view "$TAG" >/dev/null 2>&1; then - gh release create "$TAG" \ - --prerelease \ - --target "${SHA}" \ - --title "$TITLE" \ - --notes "$NOTES" - else - gh release edit "$TAG" \ - --prerelease \ - --target "${SHA}" \ - --title "$TITLE" \ - --notes "$NOTES" - fi + gh release edit "$TAG" \ + --prerelease \ + --title "$TITLE" \ + --notes "$NOTES" gh release upload "$TAG" dist/* --clobber publish-tag: diff --git a/Justfile b/Justfile index 317541e2f..3e4b6bee0 100644 --- a/Justfile +++ b/Justfile @@ -6,9 +6,10 @@ desktop_dir := "desktop" desktop_tauri_manifest := "desktop/src-tauri/Cargo.toml" web_dir := "web" -# Opt-in mesh-llm. Off by default so `just dev`/`just staging` skip ~420 extra -# crates + the llama.cpp native runtime build and stay fast to iterate on. -# Turn on to test mesh compute features: `just mesh=1 dev` / `just mesh=1 staging`. +# Opt-in mesh-llm. Off by default so `just dev`/`just staging`/`just production` +# skip ~420 extra crates + the llama.cpp native runtime build and stay fast to +# iterate on. Turn on to test mesh compute features: `just mesh=1 dev` / +# `just mesh=1 staging` / `just mesh=1 production`. mesh := "" # Reset only the current standalone desktop instance before launch. @@ -517,6 +518,33 @@ staging *ARGS: bootstrap _ensure-sidecar-stubs echo "Starting staging on Vite port ${BUZZ_VITE_PORT}, relay ${BUZZ_RELAY_URL}" pnpm exec tauri dev ${FEATURES[@]+"${FEATURES[@]}"} --config "$BUZZ_TAURI_CONFIG" {{ARGS}} +# Run the desktop app against the production relay (installs deps + builds agent tools automatically) +production *ARGS: bootstrap _ensure-sidecar-stubs + #!/usr/bin/env bash + set -euo pipefail + export PATH="{{justfile_directory()}}/bin:$PATH" + pnpm install # unconditional: production must always start with a clean dep tree + cargo build --release -p buzz-acp -p buzz-agent -p buzz-dev-mcp -p buzz-cli -p git-credential-nostr + FEATURES=() + if [[ -n "{{mesh}}" ]]; then + FEATURES=(--features mesh-llm) + export MESH_LLM_NATIVE_RUNTIME_CACHE_DIR="$(./scripts/ensure-mesh-native-runtime.sh)" + fi + # Replace the 0-byte sidecar stub with the real CLI binary so tauri dev picks it up. + TARGET=$(rustc -vV | sed -n 's|host: ||p') + TARGET_DIR=$(cargo metadata --format-version 1 --no-deps | node -p "JSON.parse(require('fs').readFileSync(0, 'utf8')).target_directory") + cp "${TARGET_DIR}/release/buzz" "desktop/src-tauri/binaries/buzz-${TARGET}" + chmod +x "desktop/src-tauri/binaries/buzz-${TARGET}" + cd {{desktop_dir}} + export BUZZ_RELAY_URL="wss://buzz.block.builderlab.xyz" + source ../scripts/instance-env.sh + # Ctrl+C kills the Tauri app before its in-process sweep finishes, leaking + # agent workers. Reap this instance's agents on exit as a backstop. + INSTANCE_ID=$(node -e "console.log(JSON.parse(process.env.BUZZ_TAURI_CONFIG).identifier)") + trap '../scripts/cleanup-instance-agents.sh "$INSTANCE_ID" || true' EXIT + echo "Starting production on Vite port ${BUZZ_VITE_PORT}, relay ${BUZZ_RELAY_URL}" + pnpm exec tauri dev ${FEATURES[@]+"${FEATURES[@]}"} --config "$BUZZ_TAURI_CONFIG" {{ARGS}} + # Run the desktop frontend dev server (port derived from worktree) desktop-dev: #!/usr/bin/env bash diff --git a/bench/t1a_evidence.sh b/bench/t1a_evidence.sh deleted file mode 100755 index 1f23f2dc4..000000000 --- a/bench/t1a_evidence.sh +++ /dev/null @@ -1,77 +0,0 @@ -#!/usr/bin/env bash -# T1a correctness evidence (runnable from this lane tip). -# -# Proves, against a live relay built from this tree: -# 1. permanent channel: kind:9 ingest emits no separate top-level TTL UPDATE -# transaction (the deferred trigger's conditional statement stays inside the -# event transaction and affects zero rows); -# 2. ephemeral channel: the TTL bump is still observed (ttl_deadline strictly advances -# across a message); -# 3. TTL-set-during-ingest race: messages committed after concurrent TTL activation -# extend the deadline beyond the activation update's own deadline. -# -# Usage: bench/t1a_evidence.sh -# Requires: relay running FROM THIS TREE against ; psql via docker exec; -# BENCH_PRIVATE_KEY env (member secret key hex); wamp_bench built --release. -set -euo pipefail - -PG="$1"; DBURL="$2"; RELAY="$3"; COMMUNITY="$4" -PSQL=(docker exec "$PG" psql "$DBURL" -tA) -sql() { "${PSQL[@]}" -c "$1"; } -BIN="./target/release/wamp_bench" -# BENCH_PUB = x-only pubkey hex for BENCH_PRIVATE_KEY (both required). -PUB="${BENCH_PUB:?set BENCH_PUB (x-only pubkey hex matching BENCH_PRIVATE_KEY)}" - -mkchan() { # $1 name, $2 ttl_seconds or NULL -> echoes uuid - local id; id=$(python3 -c "import uuid;print(uuid.uuid4())") - sql "insert into channels (id, community_id, name, channel_type, visibility, created_by, ttl_seconds, ttl_deadline) - values ('$id','$COMMUNITY','$1','stream','private',decode('$PUB','hex'),$2, - case when $2::int is null then null else now() + ($2::int || ' seconds')::interval end)" >/dev/null - sql "insert into channel_members (community_id, channel_id, pubkey, role) - values ('$COMMUNITY','$id',decode('$PUB','hex'),'owner')" >/dev/null - echo "$id" -} - -FAIL=0 - -echo "== 1. permanent channel: zero separate top-level TTL UPDATE statements ==" -PERM=$(mkchan t1a-perm NULL) -sql "select pg_stat_statements_reset()" >/dev/null -env -u BUZZ_AUTH_TAG BUZZ_RELAY_URL="$RELAY" "$BIN" "$PERM" 20 10 2 /tmp/t1a-perm.lat >/tmp/t1a-perm.json -ACC=$(python3 -c "import json;print(json.load(open('/tmp/t1a-perm.json'))['accepted'])") -TTL_CALLS=$(sql "select coalesce(sum(calls),0) from pg_stat_statements where query ilike '%UPDATE channels SET ttl_deadline%'") -echo "accepted=$ACC ttl_update_calls=$TTL_CALLS" -[[ "$ACC" -gt 0 && "$TTL_CALLS" == "0" ]] || { echo "FAIL: expected >0 accepted and 0 TTL updates"; FAIL=1; } - -echo "== 2. ephemeral channel: bump still observed ==" -EPH=$(mkchan t1a-eph 3600) -D0=$(sql "select extract(epoch from ttl_deadline) from channels where id='$EPH'") -sleep 2 -env -u BUZZ_AUTH_TAG BUZZ_RELAY_URL="$RELAY" "$BIN" "$EPH" 5 3 1 /tmp/t1a-eph.lat >/tmp/t1a-eph.json -D1=$(sql "select extract(epoch from ttl_deadline) from channels where id='$EPH'") -echo "deadline before=$D0 after=$D1" -python3 -c "import sys; sys.exit(0 if float('$D1') > float('$D0') else 1)" \ - || { echo "FAIL: ephemeral ttl_deadline did not advance"; FAIL=1; } - -echo "== 3. TTL-set-during-ingest race ==" -RACE=$(mkchan t1a-race NULL) -env -u BUZZ_AUTH_TAG BUZZ_RELAY_URL="$RELAY" "$BIN" "$RACE" 50 6 4 /tmp/t1a-race.lat >/tmp/t1a-race.json & -BPID=$! -sleep 2 -# update_channel-equivalent: set TTL and reset deadline in one statement, mid-burst. -ACTIVATION_DEADLINE=$(sql "update channels set ttl_seconds=600, ttl_deadline=clock_timestamp() + interval '600 seconds', updated_at=now() where id='$RACE' and deleted_at is null returning extract(epoch from ttl_deadline)") -wait "$BPID" -ROW=$(sql "select ttl_seconds, extract(epoch from ttl_deadline) from channels where id='$RACE'") -echo "activation_deadline=$ACTIVATION_DEADLINE post-race=$ROW" -FINAL_DEADLINE="${ROW#*|}" -python3 -c "import sys; sys.exit(0 if float('$FINAL_DEADLINE') > float('$ACTIVATION_DEADLINE') else 1)" \ - || { echo "FAIL: later message did not extend TTL beyond activation deadline"; FAIL=1; } -# and subsequent messages now bump it (channel is ephemeral now) -D0=$(sql "select extract(epoch from ttl_deadline) from channels where id='$RACE'") -sleep 2 -env -u BUZZ_AUTH_TAG BUZZ_RELAY_URL="$RELAY" "$BIN" "$RACE" 5 3 1 /tmp/t1a-race2.lat >/tmp/t1a-race2.json -D1=$(sql "select extract(epoch from ttl_deadline) from channels where id='$RACE'") -python3 -c "import sys; sys.exit(0 if float('$D1') > float('$D0') else 1)" \ - || { echo "FAIL: post-race ephemeral bump not observed"; FAIL=1; } - -[[ "$FAIL" == 0 ]] && echo "T1A EVIDENCE: ALL PASS" || { echo "T1A EVIDENCE: FAILURES"; exit 1; } diff --git a/crates/buzz-core/src/channel.rs b/crates/buzz-core/src/channel.rs index 0bc130f26..5c20afefd 100644 --- a/crates/buzz-core/src/channel.rs +++ b/crates/buzz-core/src/channel.rs @@ -7,6 +7,16 @@ use std::fmt; use std::str::FromStr; +/// Returns the canonical display name for a channel. +/// +/// Channel names are rendered with a leading `#` by clients, so surrounding +/// whitespace and user-supplied hash prefixes are removed here to keep the +/// stored name prefix-free. +pub fn canonical_channel_name(name: &str) -> &str { + name.trim_start_matches(|c: char| c == '#' || c.is_whitespace()) + .trim_end() +} + /// Whether a channel is publicly visible or invite-only. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum ChannelVisibility { @@ -167,3 +177,22 @@ impl FromStr for MemberRole { } } } + +#[cfg(test)] +mod tests { + use super::canonical_channel_name; + + #[test] + fn channel_names_trim_whitespace_and_drop_all_leading_hashes() { + assert_eq!(canonical_channel_name("channel"), "channel"); + assert_eq!(canonical_channel_name("#channel"), "channel"); + assert_eq!(canonical_channel_name("###channel"), "channel"); + assert_eq!(canonical_channel_name(" ###channel "), "channel"); + assert_eq!(canonical_channel_name("# channel"), "channel"); + assert_eq!(canonical_channel_name("### channel "), "channel"); + assert_eq!(canonical_channel_name(" ### "), ""); + assert_eq!(canonical_channel_name("# #"), ""); + assert_eq!(canonical_channel_name("### ###"), ""); + assert_eq!(canonical_channel_name("channel#topic"), "channel#topic"); + } +} diff --git a/crates/buzz-db/src/channel.rs b/crates/buzz-db/src/channel.rs index 2f20fc029..396244cc9 100644 --- a/crates/buzz-db/src/channel.rs +++ b/crates/buzz-db/src/channel.rs @@ -101,6 +101,11 @@ pub async fn create_channel( ))); } + let name = buzz_core::channel::canonical_channel_name(name); + if name.trim().is_empty() { + return Err(DbError::InvalidData("channel name is required".into())); + } + let id = Uuid::new_v4(); let mut tx = pool.begin().await?; @@ -191,6 +196,11 @@ pub async fn create_channel_with_id( )); } + let name = buzz_core::channel::canonical_channel_name(name); + if name.trim().is_empty() { + return Err(DbError::InvalidData("channel name is required".into())); + } + let mut tx = pool.begin().await?; let rows_affected = sqlx::query( @@ -1041,7 +1051,7 @@ pub async fn update_channel( pool: &PgPool, community_id: CommunityId, channel_id: Uuid, - updates: ChannelUpdate, + mut updates: ChannelUpdate, ) -> Result { if updates.name.is_none() && updates.description.is_none() @@ -1053,6 +1063,13 @@ pub async fn update_channel( )); } + if let Some(name) = updates.name.as_mut() { + *name = buzz_core::channel::canonical_channel_name(name).to_owned(); + if name.is_empty() { + return Err(DbError::InvalidData("channel name is required".into())); + } + } + // Build SET clause dynamically — only include fields that are provided. // Track parameter index for positional placeholders. let mut set_parts: Vec = Vec::new(); diff --git a/crates/buzz-media/src/validation.rs b/crates/buzz-media/src/validation.rs index 0d03429e0..bba338d52 100644 --- a/crates/buzz-media/src/validation.rs +++ b/crates/buzz-media/src/validation.rs @@ -626,6 +626,40 @@ fn validate_png_metadata_free(bytes: &[u8]) -> Result<(), MediaError> { } fn validate_webp_metadata_free(bytes: &[u8]) -> Result<(), MediaError> { + fn validate_frame_payload(payload: &[u8]) -> Result<(), MediaError> { + const FRAME_HEADER_LEN: usize = 16; + if payload.len() < FRAME_HEADER_LEN { + return Err(MediaError::InvalidImage); + } + + let mut i = FRAME_HEADER_LEN; + let mut saw_alpha = false; + let mut saw_image = false; + while i < payload.len() { + if i + 8 > payload.len() { + return Err(MediaError::InvalidImage); + } + let kind: [u8; 4] = payload[i..i + 4].try_into().unwrap(); + let len = u32::from_le_bytes(payload[i + 4..i + 8].try_into().unwrap()) as usize; + let padded = len.checked_add(len & 1).ok_or(MediaError::InvalidImage)?; + i = i + .checked_add(8) + .and_then(|start| start.checked_add(padded)) + .filter(|&end| end <= payload.len()) + .ok_or(MediaError::InvalidImage)?; + + match &kind { + b"ALPH" if !saw_alpha && !saw_image => saw_alpha = true, + b"VP8 " if !saw_image => saw_image = true, + b"VP8L" if !saw_alpha && !saw_image => saw_image = true, + b"ALPH" | b"VP8 " | b"VP8L" => return Err(MediaError::InvalidImage), + _ => return Err(MediaError::MetadataForbidden), + } + } + + saw_image.then_some(()).ok_or(MediaError::InvalidImage) + } + if bytes.len() < 12 || &bytes[..4] != b"RIFF" || &bytes[8..12] != b"WEBP" { return Err(MediaError::InvalidImage); } @@ -659,6 +693,8 @@ fn validate_webp_metadata_free(bytes: &[u8]) -> Result<(), MediaError> { if flags & (0x20 | 0x08 | 0x04) != 0 { return Err(MediaError::MetadataForbidden); } + } else if &kind == b"ANMF" { + validate_frame_payload(&bytes[payload_start..payload_start + len])?; } } Ok(()) @@ -740,7 +776,13 @@ fn validate_gif_metadata_free(bytes: &[u8]) -> Result<(), MediaError> { return Err(MediaError::MetadataForbidden); } i += 12; - skip_sub_blocks(bytes, &mut i)?; + if bytes.get(i) != Some(&3) + || bytes.get(i + 1) != Some(&1) + || bytes.get(i + 4) != Some(&0) + { + return Err(MediaError::MetadataForbidden); + } + i += 5; } _ => return Err(MediaError::MetadataForbidden), } @@ -1296,6 +1338,30 @@ mod tests { Err(MediaError::MetadataForbidden) )); } + let mut frame = vec![0; 16]; + frame.extend_from_slice(b"VP8 "); + frame.extend_from_slice(&3u32.to_le_bytes()); + frame.extend_from_slice(&[1, 2, 3, 0]); + let clean_frame = frame.clone(); + frame.extend_from_slice(b"JUNK"); + frame.extend_from_slice(&8u32.to_le_bytes()); + frame.extend_from_slice(b"location"); + let nested_metadata = webp(&[ + (b"VP8X", &[0x02, 0, 0, 0, 0, 0, 0, 0, 0, 0]), + (b"ANIM", &[0; 6]), + (b"ANMF", &frame), + ]); + assert!(matches!( + validate_webp_metadata_free(&nested_metadata), + Err(MediaError::MetadataForbidden) + )); + let canonical_animation = webp(&[ + (b"VP8X", &[0x02, 0, 0, 0, 0, 0, 0, 0, 0, 0]), + (b"ANIM", &[0; 6]), + (b"ANMF", &clean_frame), + ]); + assert!(validate_webp_metadata_free(&canonical_animation).is_ok()); + let mut trailing = webp(&[(b"VP8 ", b"pixels")]); trailing.extend_from_slice(b"hidden"); assert!(matches!( @@ -1327,6 +1393,23 @@ mod tests { validate_gif_metadata_free(&trailing), Err(MediaError::MetadataForbidden) )); + + let mut hidden_in_loop = TINY_GIF[..TINY_GIF.len() - 1].to_vec(); + hidden_in_loop.extend_from_slice(&[0x21, 0xff, 11]); + hidden_in_loop.extend_from_slice(b"NETSCAPE2.0"); + hidden_in_loop.extend_from_slice(&[3, 1, 0, 0, 8]); + hidden_in_loop.extend_from_slice(b"location"); + hidden_in_loop.extend_from_slice(&[0, 0x3b]); + assert!(matches!( + validate_gif_metadata_free(&hidden_in_loop), + Err(MediaError::MetadataForbidden) + )); + + let mut canonical_loop = TINY_GIF[..TINY_GIF.len() - 1].to_vec(); + canonical_loop.extend_from_slice(&[0x21, 0xff, 11]); + canonical_loop.extend_from_slice(b"NETSCAPE2.0"); + canonical_loop.extend_from_slice(&[3, 1, 0, 0, 0, 0x3b]); + assert!(validate_gif_metadata_free(&canonical_loop).is_ok()); } #[test] diff --git a/crates/buzz-relay/src/config.rs b/crates/buzz-relay/src/config.rs index 75e70d617..47030dcf3 100644 --- a/crates/buzz-relay/src/config.rs +++ b/crates/buzz-relay/src/config.rs @@ -58,6 +58,12 @@ pub struct Config { pub read_database_url: Option, /// Redis connection URL used by the pub/sub manager. pub redis_url: String, + /// Maximum connections in the shared Redis pool. Defaults to 16. + /// + /// deadpool's own default is `CPU_COUNT * 2`, which on a 2-vCPU relay + /// pod is only 4 — small enough that rate-limit checks, presence, and + /// pub/sub publishes queue behind each other under load. + pub redis_pool_size: usize, /// Public WebSocket URL of this relay, advertised in NIP-11. pub relay_url: String, /// Public WebSocket URL of the dedicated device-pairing relay, when configured. @@ -412,6 +418,12 @@ impl Config { let redis_url = std::env::var("REDIS_URL").unwrap_or_else(|_| "redis://localhost:6379".to_string()); + let redis_pool_size = std::env::var("BUZZ_REDIS_POOL_SIZE") + .ok() + .and_then(|v| v.parse::().ok()) + .filter(|&v| v > 0) + .unwrap_or(16); + let relay_url = std::env::var("RELAY_URL").unwrap_or_else(|_| "ws://localhost:3000".to_string()); @@ -862,6 +874,7 @@ impl Config { database_url, read_database_url, redis_url, + redis_pool_size, relay_url, pairing_relay_url, max_connections, @@ -928,6 +941,7 @@ mod tests { assert!(config.bind_addr.port() > 0); assert!(!config.database_url.is_empty()); assert!(!config.redis_url.is_empty()); + assert_eq!(config.redis_pool_size, 16); assert!(config.max_connections > 0); assert!(config.send_buffer_size > 0); assert_eq!(config.max_frame_bytes, DEFAULT_MAX_FRAME_BYTES); @@ -970,6 +984,31 @@ mod tests { ); } + #[test] + fn redis_pool_size_env_override_and_invalid_fallback() { + let _guard = ENV_MUTEX.lock().unwrap(); + let previous = std::env::var_os("BUZZ_REDIS_POOL_SIZE"); + + std::env::set_var("BUZZ_REDIS_POOL_SIZE", "32"); + let overridden = Config::from_env().expect("config").redis_pool_size; + + std::env::set_var("BUZZ_REDIS_POOL_SIZE", "0"); + let zero = Config::from_env().expect("config").redis_pool_size; + + std::env::set_var("BUZZ_REDIS_POOL_SIZE", "not-a-number"); + let junk = Config::from_env().expect("config").redis_pool_size; + + if let Some(value) = previous { + std::env::set_var("BUZZ_REDIS_POOL_SIZE", value); + } else { + std::env::remove_var("BUZZ_REDIS_POOL_SIZE"); + } + + assert_eq!(overridden, 32); + assert_eq!(zero, 16, "zero must fall back to the default"); + assert_eq!(junk, 16, "unparsable value must fall back to the default"); + } + #[test] fn read_database_url_unset_or_blank_is_none() { let _guard = ENV_MUTEX.lock().unwrap(); diff --git a/crates/buzz-relay/src/handlers/ingest.rs b/crates/buzz-relay/src/handlers/ingest.rs index f53ea0f01..ca529d1db 100644 --- a/crates/buzz-relay/src/handlers/ingest.rs +++ b/crates/buzz-relay/src/handlers/ingest.rs @@ -2039,7 +2039,11 @@ async fn ingest_event_inner( }); if create_name .as_ref() - .map(|n| n.trim().is_empty()) + .map(|n| { + buzz_core::channel::canonical_channel_name(n) + .trim() + .is_empty() + }) .unwrap_or(true) { return Err(IngestError::Rejected( @@ -2082,6 +2086,7 @@ async fn ingest_event_inner( if let Some(client_uuid) = channel_id { let name = create_name.unwrap_or_default(); + let name = buzz_core::channel::canonical_channel_name(&name); let description = event.tags.iter().find_map(|t| { if t.kind().to_string() == "about" { @@ -2099,7 +2104,7 @@ async fn ingest_event_inner( .create_channel_with_id( tenant.community(), client_uuid, - &name, + name, channel_type, visibility, description.as_deref(), diff --git a/crates/buzz-relay/src/handlers/side_effects.rs b/crates/buzz-relay/src/handlers/side_effects.rs index 36ce2254c..3112e9a55 100644 --- a/crates/buzz-relay/src/handlers/side_effects.rs +++ b/crates/buzz-relay/src/handlers/side_effects.rs @@ -445,6 +445,22 @@ pub async fn validate_admin_event( } } + // Validate channel names before storage. A name made entirely of + // display-prefix hashes becomes empty after canonicalization. + for t in event.tags.iter() { + if t.kind().to_string() == "name" { + match t.content() { + Some(v) + if !buzz_core::channel::canonical_channel_name(v) + .trim() + .is_empty() => {} + _ => { + return Err(anyhow::anyhow!("channel name is required")); + } + } + } + } + // Validate visibility values before storage. for t in event.tags.iter() { if t.kind().to_string() == "visibility" { diff --git a/crates/buzz-relay/src/main.rs b/crates/buzz-relay/src/main.rs index 00ef7819c..be9794922 100644 --- a/crates/buzz-relay/src/main.rs +++ b/crates/buzz-relay/src/main.rs @@ -334,7 +334,8 @@ async fn main() -> anyhow::Result<()> { }; let redis_pool = { - let cfg = deadpool_redis::Config::from_url(&config.redis_url); + let mut cfg = deadpool_redis::Config::from_url(&config.redis_url); + cfg.pool = Some(deadpool_redis::PoolConfig::new(config.redis_pool_size)); cfg.create_pool(Some(deadpool_redis::Runtime::Tokio1)) .map_err(|e| anyhow::anyhow!("Redis pool creation failed: {e}"))? }; diff --git a/crates/buzz-sdk/src/builders.rs b/crates/buzz-sdk/src/builders.rs index e52c97603..edad401bf 100644 --- a/crates/buzz-sdk/src/builders.rs +++ b/crates/buzz-sdk/src/builders.rs @@ -620,9 +620,18 @@ pub fn build_update_channel( )); } } + if name + .map(buzz_core::channel::canonical_channel_name) + .is_some_and(|name| name.trim().is_empty()) + { + return Err(SdkError::InvalidTag("channel name is required".into())); + } let mut tags = vec![tag(&["h", &channel_id.to_string()])?]; if let Some(n) = name { - tags.push(tag(&["name", n])?); + tags.push(tag(&[ + "name", + buzz_core::channel::canonical_channel_name(n), + ])?); } if let Some(a) = about { tags.push(tag(&["about", a])?); @@ -670,6 +679,10 @@ pub fn build_create_channel( about: Option<&str>, ttl: Option, ) -> Result { + let name = buzz_core::channel::canonical_channel_name(name); + if name.trim().is_empty() { + return Err(SdkError::InvalidTag("channel name is required".into())); + } let mut tags = vec![tag(&["h", &channel_id.to_string()])?, tag(&["name", name])?]; if let Some(v) = visibility { tags.push(tag(&["visibility", v.as_str()])?); @@ -2381,6 +2394,21 @@ mod tests { assert!(has_tag(&ev, "about", "new about")); } + #[test] + fn update_channel_strips_all_leading_hashes_from_name() { + let ev = + sign(build_update_channel(uuid(), Some(" ###new-name "), None, None, None).unwrap()); + assert!(has_tag(&ev, "name", "new-name")); + } + + #[test] + fn update_channel_rejects_hash_only_name() { + assert!(matches!( + build_update_channel(uuid(), Some(" ### "), None, None, None), + Err(SdkError::InvalidTag(_)) + )); + } + #[test] fn update_channel_visibility_and_ttl() { let cid = uuid(); @@ -2471,6 +2499,37 @@ mod tests { assert!(has_tag(&ev, "name", "dev")); } + #[test] + fn create_channel_strips_all_leading_hashes_from_name() { + let ev = sign( + build_create_channel( + uuid(), + " ###dev ", + None::, + None::, + None, + None, + ) + .unwrap(), + ); + assert!(has_tag(&ev, "name", "dev")); + } + + #[test] + fn create_channel_rejects_hash_only_name() { + assert!(matches!( + build_create_channel( + uuid(), + " ### ", + None::, + None::, + None, + None, + ), + Err(SdkError::InvalidTag(_)) + )); + } + #[test] fn create_channel_ephemeral_emits_ttl() { let cid = uuid(); diff --git a/crates/buzz-sdk/src/lib.rs b/crates/buzz-sdk/src/lib.rs index bbad8494c..4ee0cd4c8 100644 --- a/crates/buzz-sdk/src/lib.rs +++ b/crates/buzz-sdk/src/lib.rs @@ -74,6 +74,8 @@ pub struct CustomEmoji { pub url: String, } +/// Return a channel name without client-rendered leading hash prefixes. +pub use buzz_core::channel::canonical_channel_name; /// Channel type. pub use buzz_core::channel::ChannelType as ChannelKind; /// Channel visibility. diff --git a/desktop/playwright.config.ts b/desktop/playwright.config.ts index e3745338a..be9d73ed0 100644 --- a/desktop/playwright.config.ts +++ b/desktop/playwright.config.ts @@ -71,7 +71,7 @@ export default defineConfig({ "**/home-collapsed-top-chrome.spec.ts", "**/top-chrome-zoom-clearance.spec.ts", "**/thread-unread.spec.ts", - "**/workspace-rail.spec.ts", + "**/community-rail.spec.ts", "**/boot-splash.spec.ts", "**/thread-reply-anchor-roleplay.spec.ts", "**/threadpane-ultrawide.spec.ts", diff --git a/desktop/src-tauri/src/commands/media.rs b/desktop/src-tauri/src/commands/media.rs index 5aaa45b4a..1ecf77192 100644 --- a/desktop/src-tauri/src/commands/media.rs +++ b/desktop/src-tauri/src/commands/media.rs @@ -155,9 +155,9 @@ pub(crate) fn sanitize_filename(name: &str) -> String { /// Return true when a PNG/WebP payload declares animation. /// -/// Animated payloads are left byte-identical here so frame timing, looping, -/// and disposal semantics are preserved. The relay's structural validator is -/// still the authority that rejects any metadata-bearing animation. +/// Animated payloads use structural sanitizers so frame timing, looping, and +/// disposal semantics are preserved without flattening the image. The relay's +/// validator remains the final authority for the sanitized container. fn is_animated_image(body: &[u8], mime: &str) -> bool { match mime { "image/png" if body.starts_with(b"\x89PNG\r\n\x1a\n") => { @@ -228,7 +228,40 @@ pub(crate) fn sanitize_image_for_upload(body: Vec, mime: &str) -> Result { + Some("PNG") + } + "image/webp" if super::media_animated::animated_webp_uses_exif_orientation(&body) => { + Some("WebP") + } + _ => None, + }; + if let Some(format) = oriented_format { + return Err(format!( + "animated {format} with EXIF orientation cannot be uploaded without changing its appearance" + )); + } + let color_profile_format = match mime { + "image/png" if super::media_animated::animated_png_uses_icc_profile(&body) => { + Some("PNG") + } + "image/webp" if super::media_animated::animated_webp_uses_icc_profile(&body) => { + Some("WebP") + } + _ => None, + }; + if let Some(format) = color_profile_format { + return Err(format!( + "animated {format} with an ICC profile cannot be uploaded without changing its colors" + )); + } + let stripped = match mime { + "image/png" => super::media_animated::strip_animated_png_metadata(&body), + "image/webp" => super::media_animated::strip_animated_webp_metadata(&body), + _ => None, + }; + return Ok(stripped.unwrap_or(body)); } use image::ImageDecoder; @@ -900,18 +933,12 @@ mod tests { apng.extend_from_slice(&[0; 8]); apng.extend_from_slice(&[0; 4]); assert!(is_animated_image(&apng, "image/png")); - assert_eq!( - sanitize_image_for_upload(apng.clone(), "image/png").unwrap(), - apng - ); + assert!(sanitize_image_for_upload(apng, "image/png").is_ok()); let mut webp = b"RIFF\x0c\0\0\0WEBPANIM".to_vec(); webp.extend_from_slice(&0u32.to_le_bytes()); assert!(is_animated_image(&webp, "image/webp")); - assert_eq!( - sanitize_image_for_upload(webp.clone(), "image/webp").unwrap(), - webp - ); + assert!(sanitize_image_for_upload(webp, "image/webp").is_ok()); } #[test] diff --git a/desktop/src-tauri/src/commands/media_animated.rs b/desktop/src-tauri/src/commands/media_animated.rs new file mode 100644 index 000000000..42dec3f19 --- /dev/null +++ b/desktop/src-tauri/src/commands/media_animated.rs @@ -0,0 +1,693 @@ +//! Structural metadata stripping for animated PNG and WebP uploads. +//! +//! Re-encoding an animated image through `image::DynamicImage` keeps only its +//! first frame. These helpers instead copy rendering chunks byte-for-byte while +//! dropping the metadata channels rejected by the relay. + +const PNG_SIGNATURE: &[u8] = b"\x89PNG\r\n\x1a\n"; +const PNG_ALLOWED_ANCILLARY: &[[u8; 4]] = &[ + *b"cHRM", *b"gAMA", *b"sBIT", *b"sRGB", *b"bKGD", *b"hIST", *b"tRNS", *b"sPLT", *b"acTL", + *b"fcTL", *b"fdAT", +]; +const WEBP_ALLOWED_CHUNKS: &[[u8; 4]] = + &[*b"VP8 ", *b"VP8L", *b"VP8X", *b"ALPH", *b"ANIM", *b"ANMF"]; +const WEBP_METADATA_FLAGS: u8 = 0x20 | 0x08 | 0x04; + +#[derive(Clone, Copy)] +enum TiffEndian { + Little, + Big, +} + +fn tiff_u16(bytes: &[u8], offset: usize, endian: TiffEndian) -> Option { + let value: [u8; 2] = bytes.get(offset..offset.checked_add(2)?)?.try_into().ok()?; + Some(match endian { + TiffEndian::Little => u16::from_le_bytes(value), + TiffEndian::Big => u16::from_be_bytes(value), + }) +} + +fn tiff_u32(bytes: &[u8], offset: usize, endian: TiffEndian) -> Option { + let value: [u8; 4] = bytes.get(offset..offset.checked_add(4)?)?.try_into().ok()?; + Some(match endian { + TiffEndian::Little => u32::from_le_bytes(value), + TiffEndian::Big => u32::from_be_bytes(value), + }) +} + +fn exif_orientation(payload: &[u8]) -> Option { + let tiff = payload.strip_prefix(b"Exif\0\0").unwrap_or(payload); + let endian = match tiff.get(..2)? { + b"II" => TiffEndian::Little, + b"MM" => TiffEndian::Big, + _ => return None, + }; + if tiff_u16(tiff, 2, endian)? != 42 { + return None; + } + + let ifd_offset = usize::try_from(tiff_u32(tiff, 4, endian)?).ok()?; + let entry_count = usize::from(tiff_u16(tiff, ifd_offset, endian)?); + let entries_start = ifd_offset.checked_add(2)?; + for index in 0..entry_count { + let entry = entries_start.checked_add(index.checked_mul(12)?)?; + if tiff_u16(tiff, entry, endian)? == 0x0112 + && tiff_u16(tiff, entry.checked_add(2)?, endian)? == 3 + && tiff_u32(tiff, entry.checked_add(4)?, endian)? == 1 + { + return tiff_u16(tiff, entry.checked_add(8)?, endian); + } + } + None +} + +fn png_contains_chunk(body: &[u8], target: &[u8; 4]) -> bool { + if !body.starts_with(PNG_SIGNATURE) { + return false; + } + + let mut offset = PNG_SIGNATURE.len(); + while offset < body.len() { + let Some(header_end) = offset.checked_add(8).filter(|&end| end <= body.len()) else { + return false; + }; + let Some(payload_len) = body + .get(offset..offset + 4) + .and_then(|bytes| bytes.try_into().ok()) + .map(u32::from_be_bytes) + .and_then(|length| usize::try_from(length).ok()) + else { + return false; + }; + let Some(kind) = body.get(offset + 4..header_end) else { + return false; + }; + let Some(chunk_end) = header_end + .checked_add(payload_len) + .and_then(|end| end.checked_add(4)) + .filter(|&end| end <= body.len()) + else { + return false; + }; + if kind == target { + return true; + } + offset = chunk_end; + if kind == b"IEND" { + return false; + } + } + false +} + +fn webp_contains_top_level_chunk(body: &[u8], target: &[u8; 4]) -> bool { + if body.len() < 12 || &body[..4] != b"RIFF" || &body[8..12] != b"WEBP" { + return false; + } + let Some(declared) = body + .get(4..8) + .and_then(|bytes| bytes.try_into().ok()) + .map(u32::from_le_bytes) + .and_then(|length| usize::try_from(length).ok()) + else { + return false; + }; + let Some(input_end) = declared + .checked_add(8) + .filter(|&end| (12..=body.len()).contains(&end)) + else { + return false; + }; + + let mut offset = 12usize; + while offset < input_end { + let Some(header_end) = offset.checked_add(8).filter(|&end| end <= input_end) else { + return false; + }; + let Some(kind) = body.get(offset..offset + 4) else { + return false; + }; + let Some(payload_len) = body + .get(offset + 4..header_end) + .and_then(|bytes| bytes.try_into().ok()) + .map(u32::from_le_bytes) + .and_then(|length| usize::try_from(length).ok()) + else { + return false; + }; + let Some(chunk_end) = payload_len + .checked_add(payload_len & 1) + .and_then(|length| header_end.checked_add(length)) + .filter(|&end| end <= input_end) + else { + return false; + }; + if kind == target { + return true; + } + offset = chunk_end; + } + false +} + +/// Return true when removing an APNG ICC profile would change color rendering. +pub(crate) fn animated_png_uses_icc_profile(body: &[u8]) -> bool { + png_contains_chunk(body, b"iCCP") +} + +/// Return true when removing an animated WebP ICC profile would change colors. +pub(crate) fn animated_webp_uses_icc_profile(body: &[u8]) -> bool { + webp_contains_top_level_chunk(body, b"ICCP") +} + +/// Return true when an animated PNG relies on eXIf to rotate or mirror frames. +pub(crate) fn animated_png_uses_exif_orientation(body: &[u8]) -> bool { + if !body.starts_with(PNG_SIGNATURE) { + return false; + } + + let mut offset = PNG_SIGNATURE.len(); + while offset < body.len() { + let Some(header_end) = offset.checked_add(8).filter(|&end| end <= body.len()) else { + return false; + }; + let Some(payload_len) = body + .get(offset..offset + 4) + .and_then(|bytes| bytes.try_into().ok()) + .map(u32::from_be_bytes) + .and_then(|length| usize::try_from(length).ok()) + else { + return false; + }; + let Some(kind) = body.get(offset + 4..header_end) else { + return false; + }; + let payload_start = header_end; + let Some(chunk_end) = payload_start + .checked_add(payload_len) + .and_then(|end| end.checked_add(4)) + .filter(|&end| end <= body.len()) + else { + return false; + }; + if kind == b"eXIf" + && exif_orientation(&body[payload_start..payload_start + payload_len]) + .is_some_and(|orientation| (2..=8).contains(&orientation)) + { + return true; + } + offset = chunk_end; + if kind == b"IEND" { + return false; + } + } + false +} + +/// Return true when a WebP relies on EXIF to rotate or mirror its frames. +/// +/// Structural metadata removal cannot bake this transform without decoding +/// and re-encoding every frame, so callers must reject this uncommon case +/// rather than silently changing how the animation renders. +pub(crate) fn animated_webp_uses_exif_orientation(body: &[u8]) -> bool { + if body.len() < 12 || &body[..4] != b"RIFF" || &body[8..12] != b"WEBP" { + return false; + } + let Some(declared) = body + .get(4..8) + .and_then(|bytes| bytes.try_into().ok()) + .map(u32::from_le_bytes) + .and_then(|length| usize::try_from(length).ok()) + else { + return false; + }; + let Some(input_end) = declared + .checked_add(8) + .filter(|&end| (12..=body.len()).contains(&end)) + else { + return false; + }; + + let mut offset = 12usize; + while offset < input_end { + let Some(header_end) = offset.checked_add(8).filter(|&end| end <= input_end) else { + return false; + }; + let Some(kind) = body.get(offset..offset + 4) else { + return false; + }; + let Some(payload_len) = body + .get(offset + 4..header_end) + .and_then(|bytes| bytes.try_into().ok()) + .map(u32::from_le_bytes) + .and_then(|length| usize::try_from(length).ok()) + else { + return false; + }; + let payload_start = header_end; + let Some(chunk_end) = payload_len + .checked_add(payload_len & 1) + .and_then(|length| payload_start.checked_add(length)) + .filter(|&end| end <= input_end) + else { + return false; + }; + if kind == b"EXIF" + && exif_orientation(&body[payload_start..payload_start + payload_len]) + .is_some_and(|orientation| (2..=8).contains(&orientation)) + { + return true; + } + offset = chunk_end; + } + false +} + +/// Strip metadata-bearing ancillary chunks from a PNG without touching frame +/// control or image data. Bytes after `IEND` are truncated. +pub(crate) fn strip_animated_png_metadata(body: &[u8]) -> Option> { + if !body.starts_with(PNG_SIGNATURE) { + return None; + } + + let mut output = Vec::with_capacity(body.len()); + output.extend_from_slice(PNG_SIGNATURE); + let mut offset = PNG_SIGNATURE.len(); + + while offset < body.len() { + let header_end = offset.checked_add(8)?; + if header_end > body.len() { + return None; + } + let payload_len = u32::from_be_bytes(body[offset..offset + 4].try_into().ok()?) as usize; + let kind: [u8; 4] = body[offset + 4..offset + 8].try_into().ok()?; + let chunk_end = offset + .checked_add(12)? + .checked_add(payload_len) + .filter(|&end| end <= body.len())?; + + let ancillary = kind[0] & 0x20 != 0; + if !ancillary || PNG_ALLOWED_ANCILLARY.contains(&kind) { + output.extend_from_slice(&body[offset..chunk_end]); + } + + offset = chunk_end; + if kind == *b"IEND" { + return Some(output); + } + } + + None +} + +fn append_webp_chunk(output: &mut Vec, kind: &[u8; 4], payload: &[u8]) -> Option<()> { + output.extend_from_slice(kind); + output.extend_from_slice(&u32::try_from(payload.len()).ok()?.to_le_bytes()); + output.extend_from_slice(payload); + if payload.len() & 1 != 0 { + output.push(0); + } + Some(()) +} + +fn strip_anmf_metadata(payload: &[u8]) -> Option> { + const FRAME_HEADER_LEN: usize = 16; + if payload.len() < FRAME_HEADER_LEN { + return None; + } + + let mut output = Vec::with_capacity(payload.len()); + output.extend_from_slice(&payload[..FRAME_HEADER_LEN]); + let mut offset = FRAME_HEADER_LEN; + let mut saw_alpha = false; + let mut saw_image = false; + + while offset < payload.len() { + let header_end = offset.checked_add(8).filter(|&end| end <= payload.len())?; + let kind: [u8; 4] = payload[offset..offset + 4].try_into().ok()?; + let chunk_len = + u32::from_le_bytes(payload[offset + 4..header_end].try_into().ok()?) as usize; + let chunk_start = header_end; + let chunk_end = chunk_len + .checked_add(chunk_len & 1) + .and_then(|length| chunk_start.checked_add(length)) + .filter(|&end| end <= payload.len())?; + let chunk_payload = &payload[chunk_start..chunk_start + chunk_len]; + + match &kind { + b"ALPH" if !saw_alpha && !saw_image => { + append_webp_chunk(&mut output, &kind, chunk_payload)?; + saw_alpha = true; + } + b"VP8 " if !saw_image => { + append_webp_chunk(&mut output, &kind, chunk_payload)?; + saw_image = true; + } + b"VP8L" if !saw_alpha && !saw_image => { + append_webp_chunk(&mut output, &kind, chunk_payload)?; + saw_image = true; + } + b"ALPH" | b"VP8 " | b"VP8L" => return None, + _ => {} + } + + offset = chunk_end; + } + + saw_image.then_some(output) +} + +/// Strip metadata chunks and flags from a WebP container while retaining all +/// still/animated rendering chunks. RIFF padding is canonicalized to zero and +/// the container length is rewritten after removals. +pub(crate) fn strip_animated_webp_metadata(body: &[u8]) -> Option> { + if body.len() < 12 || &body[..4] != b"RIFF" || &body[8..12] != b"WEBP" { + return None; + } + + let declared = u32::from_le_bytes(body[4..8].try_into().ok()?) as usize; + let input_end = declared + .checked_add(8) + .filter(|&end| (12..=body.len()).contains(&end))?; + let mut output = Vec::with_capacity(input_end); + output.extend_from_slice(b"RIFF\0\0\0\0WEBP"); + let mut offset = 12usize; + + while offset < input_end { + if offset.checked_add(8)? > input_end { + return None; + } + let kind: [u8; 4] = body[offset..offset + 4].try_into().ok()?; + let payload_len = + u32::from_le_bytes(body[offset + 4..offset + 8].try_into().ok()?) as usize; + let payload_start = offset + 8; + let padded_len = payload_len.checked_add(payload_len & 1)?; + let chunk_end = payload_start + .checked_add(padded_len) + .filter(|&end| end <= input_end)?; + + if WEBP_ALLOWED_CHUNKS.contains(&kind) { + if kind == *b"VP8X" { + let (&flags, rest) = + body[payload_start..payload_start + payload_len].split_first()?; + let mut payload = Vec::with_capacity(payload_len); + payload.push(flags & !WEBP_METADATA_FLAGS); + payload.extend_from_slice(rest); + append_webp_chunk(&mut output, &kind, &payload)?; + } else if kind == *b"ANMF" { + let payload = + strip_anmf_metadata(&body[payload_start..payload_start + payload_len])?; + append_webp_chunk(&mut output, &kind, &payload)?; + } else { + append_webp_chunk( + &mut output, + &kind, + &body[payload_start..payload_start + payload_len], + )?; + } + } + + offset = chunk_end; + } + + let riff_len = u32::try_from(output.len().checked_sub(8)?).ok()?; + output[4..8].copy_from_slice(&riff_len.to_le_bytes()); + Some(output) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::commands::media::sanitize_image_for_upload; + + fn png_chunk(kind: &[u8; 4], payload: &[u8]) -> Vec { + let mut chunk = Vec::new(); + chunk.extend_from_slice(&(payload.len() as u32).to_be_bytes()); + chunk.extend_from_slice(kind); + chunk.extend_from_slice(payload); + // The structural sanitizer copies CRCs without interpreting them. A + // zero placeholder keeps these focused tests dependency-free. + chunk.extend_from_slice(&[0; 4]); + chunk + } + + fn animated_png(metadata: bool) -> Vec { + let mut png = PNG_SIGNATURE.to_vec(); + png.extend_from_slice(&png_chunk(b"IHDR", &[0; 13])); + png.extend_from_slice(&png_chunk(b"acTL", &[0, 0, 0, 2, 0, 0, 0, 0])); + if metadata { + png.extend_from_slice(&png_chunk(b"tEXt", b"Location\0secret")); + png.extend_from_slice(&png_chunk(b"pHYs", &[0; 9])); + } + png.extend_from_slice(&png_chunk(b"fcTL", &[0; 26])); + png.extend_from_slice(&png_chunk(b"IDAT", &[1, 2, 3])); + png.extend_from_slice(&png_chunk(b"fdAT", &[0, 0, 0, 1, 4, 5])); + png.extend_from_slice(&png_chunk(b"IEND", &[])); + png + } + + fn exif_orientation_payload( + orientation: u16, + endian: TiffEndian, + include_preamble: bool, + ) -> Vec { + let mut exif = if include_preamble { + b"Exif\0\0".to_vec() + } else { + Vec::new() + }; + match endian { + TiffEndian::Little => { + exif.extend_from_slice(b"II"); + exif.extend_from_slice(&42u16.to_le_bytes()); + exif.extend_from_slice(&8u32.to_le_bytes()); + exif.extend_from_slice(&1u16.to_le_bytes()); + exif.extend_from_slice(&0x0112u16.to_le_bytes()); + exif.extend_from_slice(&3u16.to_le_bytes()); + exif.extend_from_slice(&1u32.to_le_bytes()); + exif.extend_from_slice(&orientation.to_le_bytes()); + exif.extend_from_slice(&[0; 2]); + exif.extend_from_slice(&0u32.to_le_bytes()); + } + TiffEndian::Big => { + exif.extend_from_slice(b"MM"); + exif.extend_from_slice(&42u16.to_be_bytes()); + exif.extend_from_slice(&8u32.to_be_bytes()); + exif.extend_from_slice(&1u16.to_be_bytes()); + exif.extend_from_slice(&0x0112u16.to_be_bytes()); + exif.extend_from_slice(&3u16.to_be_bytes()); + exif.extend_from_slice(&1u32.to_be_bytes()); + exif.extend_from_slice(&orientation.to_be_bytes()); + exif.extend_from_slice(&[0; 2]); + exif.extend_from_slice(&0u32.to_be_bytes()); + } + } + exif + } + + fn animated_png_with_orientation(orientation: u16, endian: TiffEndian) -> Vec { + let clean = animated_png(false); + let mut png = clean[..33].to_vec(); + png.extend_from_slice(&png_chunk( + b"eXIf", + &exif_orientation_payload(orientation, endian, false), + )); + png.extend_from_slice(&clean[33..]); + png + } + + fn webp_chunk(kind: &[u8; 4], payload: &[u8]) -> Vec { + let mut chunk = Vec::new(); + chunk.extend_from_slice(kind); + chunk.extend_from_slice(&(payload.len() as u32).to_le_bytes()); + chunk.extend_from_slice(payload); + if payload.len() & 1 != 0 { + chunk.push(0); + } + chunk + } + + fn animated_webp(metadata: bool) -> Vec { + let metadata_flags = if metadata { WEBP_METADATA_FLAGS } else { 0 }; + let mut chunks = webp_chunk(b"VP8X", &[metadata_flags | 0x02, 0, 0, 0, 0, 0, 0, 0, 0, 0]); + chunks.extend_from_slice(&webp_chunk(b"ANIM", &[0; 6])); + if metadata { + chunks.extend_from_slice(&webp_chunk(b"EXIF", b"location")); + chunks.extend_from_slice(&webp_chunk(b"XMP ", b"")); + chunks.extend_from_slice(&webp_chunk(b"JUNK", b"private")); + } + let mut frame = vec![0; 16]; + frame.extend_from_slice(&webp_chunk(b"VP8 ", &[1, 2, 3])); + chunks.extend_from_slice(&webp_chunk(b"ANMF", &frame)); + + let mut webp = b"RIFF".to_vec(); + webp.extend_from_slice(&((chunks.len() + 4) as u32).to_le_bytes()); + webp.extend_from_slice(b"WEBP"); + webp.extend_from_slice(&chunks); + webp + } + + fn animated_webp_with_orientation(orientation: u16, endian: TiffEndian) -> Vec<u8> { + let exif = exif_orientation_payload(orientation, endian, true); + let mut chunks = webp_chunk( + b"VP8X", + &[WEBP_METADATA_FLAGS | 0x02, 0, 0, 0, 0, 0, 0, 0, 0, 0], + ); + chunks.extend_from_slice(&webp_chunk(b"ANIM", &[0; 6])); + chunks.extend_from_slice(&webp_chunk(b"EXIF", &exif)); + let mut frame = vec![0; 16]; + frame.extend_from_slice(&webp_chunk(b"VP8 ", &[1, 2, 3])); + chunks.extend_from_slice(&webp_chunk(b"ANMF", &frame)); + let mut webp = b"RIFF".to_vec(); + webp.extend_from_slice(&((chunks.len() + 4) as u32).to_le_bytes()); + webp.extend_from_slice(b"WEBP"); + webp.extend_from_slice(&chunks); + webp + } + + #[test] + fn test_strip_animated_png_metadata_preserves_animation_chunks() { + assert_eq!( + strip_animated_png_metadata(&animated_png(true)), + Some(animated_png(false)) + ); + } + + #[test] + fn test_strip_animated_png_metadata_is_byte_identical_for_clean_input() { + let clean = animated_png(false); + assert_eq!(strip_animated_png_metadata(&clean), Some(clean)); + } + + #[test] + fn test_strip_animated_webp_metadata_preserves_animation_chunks() { + assert_eq!( + strip_animated_webp_metadata(&animated_webp(true)), + Some(animated_webp(false)) + ); + } + + #[test] + fn test_strip_animated_webp_metadata_is_byte_identical_for_clean_input() { + let clean = animated_webp(false); + assert_eq!(strip_animated_webp_metadata(&clean), Some(clean)); + } + + #[test] + fn test_detects_non_identity_animated_webp_exif_orientation() { + for endian in [TiffEndian::Little, TiffEndian::Big] { + assert!(!animated_webp_uses_exif_orientation( + &animated_webp_with_orientation(1, endian) + )); + assert!(animated_webp_uses_exif_orientation( + &animated_webp_with_orientation(6, endian) + )); + } + } + + #[test] + fn test_detects_non_identity_animated_png_exif_orientation() { + for endian in [TiffEndian::Little, TiffEndian::Big] { + assert!(!animated_png_uses_exif_orientation( + &animated_png_with_orientation(1, endian) + )); + assert!(animated_png_uses_exif_orientation( + &animated_png_with_orientation(6, endian) + )); + } + } + + #[test] + fn test_detects_animated_icc_profiles() { + let clean_png = animated_png(false); + let mut png = clean_png[..33].to_vec(); + png.extend_from_slice(&png_chunk(b"iCCP", b"profile")); + png.extend_from_slice(&clean_png[33..]); + assert!(animated_png_uses_icc_profile(&png)); + assert!(sanitize_image_for_upload(png, "image/png").is_err()); + assert!(!animated_png_uses_icc_profile(&clean_png)); + + let mut chunks = webp_chunk( + b"VP8X", + &[WEBP_METADATA_FLAGS | 0x02, 0, 0, 0, 0, 0, 0, 0, 0, 0], + ); + chunks.extend_from_slice(&webp_chunk(b"ICCP", b"profile")); + chunks.extend_from_slice(&animated_webp(false)[30..]); + let mut webp = b"RIFF".to_vec(); + webp.extend_from_slice(&((chunks.len() + 4) as u32).to_le_bytes()); + webp.extend_from_slice(b"WEBP"); + webp.extend_from_slice(&chunks); + assert!(animated_webp_uses_icc_profile(&webp)); + assert!(sanitize_image_for_upload(webp, "image/webp").is_err()); + assert!(!animated_webp_uses_icc_profile(&animated_webp(false))); + } + + #[test] + fn test_strip_animated_webp_removes_nested_frame_metadata() { + let mut dirty_frame = vec![0; 16]; + dirty_frame.extend_from_slice(&webp_chunk(b"VP8 ", &[1, 2, 3])); + dirty_frame.extend_from_slice(&webp_chunk(b"JUNK", b"location")); + let mut clean_frame = vec![0; 16]; + clean_frame.extend_from_slice(&webp_chunk(b"VP8 ", &[1, 2, 3])); + + assert_eq!(strip_anmf_metadata(&dirty_frame), Some(clean_frame.clone())); + + clean_frame.extend_from_slice(&webp_chunk(b"VP8L", &[4])); + assert!(strip_anmf_metadata(&clean_frame).is_none()); + } + + #[test] + fn test_animated_sanitizers_truncate_trailing_bytes() { + let clean_png = animated_png(false); + let mut padded_png = clean_png.clone(); + padded_png.extend_from_slice(b"trailing metadata"); + assert_eq!(strip_animated_png_metadata(&padded_png), Some(clean_png)); + + let clean_webp = animated_webp(false); + let mut padded_webp = clean_webp.clone(); + padded_webp.extend_from_slice(b"trailing metadata"); + assert_eq!(strip_animated_webp_metadata(&padded_webp), Some(clean_webp)); + } + + #[test] + fn test_animated_sanitizers_reject_malformed_containers() { + assert!(strip_animated_png_metadata(PNG_SIGNATURE).is_none()); + assert!(strip_animated_webp_metadata(b"RIFF\x20\0\0\0WEBP").is_none()); + } + + #[test] + fn test_upload_sanitizer_uses_structural_animation_scrubbers() { + assert_eq!( + sanitize_image_for_upload(animated_png(true), "image/png"), + Ok(animated_png(false)) + ); + assert_eq!( + sanitize_image_for_upload(animated_webp(true), "image/webp"), + Ok(animated_webp(false)) + ); + assert_eq!( + sanitize_image_for_upload( + animated_webp_with_orientation(1, TiffEndian::Little), + "image/webp" + ), + Ok(animated_webp(false)) + ); + assert_eq!( + sanitize_image_for_upload( + animated_png_with_orientation(1, TiffEndian::Little), + "image/png" + ), + Ok(animated_png(false)) + ); + assert!(sanitize_image_for_upload( + animated_webp_with_orientation(6, TiffEndian::Little), + "image/webp" + ) + .is_err()); + assert!(sanitize_image_for_upload( + animated_png_with_orientation(6, TiffEndian::Little), + "image/png" + ) + .is_err()); + } +} diff --git a/desktop/src-tauri/src/commands/media_gif.rs b/desktop/src-tauri/src/commands/media_gif.rs index be2ea1a25..ac5814d41 100644 --- a/desktop/src-tauri/src/commands/media_gif.rs +++ b/desktop/src-tauri/src/commands/media_gif.rs @@ -92,9 +92,17 @@ pub(crate) fn strip_gif_metadata(body: &[u8]) -> Option<Vec<u8>> { } let app = &body[i + 1..i + 12]; let keep = app == b"NETSCAPE2.0" || app == b"ANIMEXTS1.0"; - i = gif_sub_blocks_end(body, i + 12)?; + let data_start = i + 12; + i = gif_sub_blocks_end(body, data_start)?; if keep { - out.extend_from_slice(&body[start..i]); + if body.get(data_start) != Some(&3) + || body.get(data_start + 1) != Some(&1) + || data_start.checked_add(5)? > body.len() + { + return None; + } + out.extend_from_slice(&body[start..data_start + 4]); + out.push(0); } } // Comment (0xFE), plain-text (0x01), and unknown @@ -182,6 +190,17 @@ mod tests { assert_eq!(strip_gif_metadata(&clean).unwrap(), clean); } + #[test] + fn test_strip_gif_metadata_canonicalizes_loop_extension() { + let clean = minimal_gif(); + let mut dirty = clean[..37].to_vec(); + dirty.extend_from_slice(&[8]); + dirty.extend_from_slice(b"location"); + dirty.push(0); + dirty.extend_from_slice(&clean[38..]); + assert_eq!(strip_gif_metadata(&dirty).unwrap(), clean); + } + #[test] fn test_strip_gif_metadata_truncates_bytes_after_trailer() { let mut padded = minimal_gif(); diff --git a/desktop/src-tauri/src/commands/mod.rs b/desktop/src-tauri/src/commands/mod.rs index cf4cdca91..77080e930 100644 --- a/desktop/src-tauri/src/commands/mod.rs +++ b/desktop/src-tauri/src/commands/mod.rs @@ -23,6 +23,7 @@ mod identity_archive; mod legacy_storage; mod link_preview; pub(crate) mod media; +mod media_animated; mod media_download; mod media_gif; mod media_transcode; diff --git a/desktop/src-tauri/src/events.rs b/desktop/src-tauri/src/events.rs index 5dee57829..777d56d02 100644 --- a/desktop/src-tauri/src/events.rs +++ b/desktop/src-tauri/src/events.rs @@ -148,6 +148,10 @@ pub fn build_create_channel( about: Option<&str>, ttl_seconds: Option<i32>, ) -> Result<EventBuilder, String> { + let name = buzz_sdk_pkg::canonical_channel_name(name); + if name.trim().is_empty() { + return Err("channel name is required".into()); + } let mut tags = vec![ tag(vec!["h", &channel_id.to_string()])?, tag(vec!["name", name])?, @@ -194,6 +198,10 @@ pub fn build_update_channel( return Err("visibility must be \"open\" or \"private\"".into()); } } + let name = name.map(buzz_sdk_pkg::canonical_channel_name); + if name.is_some_and(|name| name.trim().is_empty()) { + return Err("channel name is required".into()); + } let mut tags = vec![tag(vec!["h", &channel_id.to_string()])?]; if let Some(n) = name { tags.push(tag(vec!["name", n])?); @@ -842,7 +850,12 @@ pub fn build_approval_deny(token: &str, note: Option<&str>) -> Result<EventBuild mod tests { use super::*; use nostr::Keys; - + #[test] + fn channel_builders_reject_hash_only_names() { + let channel_id = Uuid::new_v4(); + assert!(build_create_channel(channel_id, "###", "open", "stream", None, None).is_err()); + assert!(build_update_channel(channel_id, Some("###"), None, None, None).is_err()); + } /// Builder layout regression for the NIP-IA owner-of-agent archive flow. /// Compares against `docs/nips/NIP-IA.md` §Vector 1. #[test] diff --git a/desktop/src/app/AppShell.tsx b/desktop/src/app/AppShell.tsx index f10f7feba..1d6223e71 100644 --- a/desktop/src/app/AppShell.tsx +++ b/desktop/src/app/AppShell.tsx @@ -714,6 +714,7 @@ export function AppShell() { } onAddCommunity={addCommunityDialog.openDialog} onRemoveCommunity={communitiesHook.removeCommunity} + onReorderCommunities={communitiesHook.reorderCommunities} onSwitchCommunity={handleSwitchCommunity} onUpdateCommunity={communitiesHook.updateCommunity} communities={communitiesHook.communities} diff --git a/desktop/src/features/agents/AGENTS.md b/desktop/src/features/agents/AGENTS.md index 6ec28c30c..da230bdf4 100644 --- a/desktop/src/features/agents/AGENTS.md +++ b/desktop/src/features/agents/AGENTS.md @@ -68,20 +68,34 @@ with a TypeScript lookup table or an id comparison in a component. sole onboarding surface that chooses and persists `preferred_runtime`. `onboarding-agent-defaults.spec.ts` is the acceptance gate for anything touching this flow or the shared renderer. +8. **Omit the Model control only after a confirmed successful empty + discovery on an optional-model harness.** When the field model marks model + as `acpNative` (Claude Code / Codex), `shouldRenderModelControl` hides the + picker while discovery is in flight and after IPC resolves with no usable + options (`modelDiscoverySuccessfulEmpty` / `isSuccessfulEmptyDiscovery`). + A thrown or unavailable discovery keeps the control so #2246 failure UI can + render, and must not heal/clear persisted model or effort. Full disclosure + still shows the control when Custom model is available. Required-model + harnesses always keep the field. Gate: `defaults hides model when optional + harness has empty discovery` (and the failed-discovery counterpart) in + `onboarding-agent-defaults.spec.ts`. ## The tests that enforce this - `lib/agentConfigCore.test.mjs` — field model per harness × scope, clearing policy. Update when the capability model changes. - `ui/agentConfigFieldsContract.test.mjs` — canonical behaviors + disclosure - presets + `shouldShowModelStatusMessage` status-bypass rule. If this fails, - you probably reintroduced a per-surface flag or broke the status-bypass. + presets + `shouldShowModelStatusMessage` status-bypass + + `shouldRenderModelControl` (successful-empty omit vs failure keep). If this + fails, you probably reintroduced a per-surface flag or conflated empty with + failed discovery. - `ui/usePersonaModelDiscovery.test.mjs` — `synthesizeEmptyDiscoveryStatus`, - `isCacheableDiscoveryResponse`, `deriveModelDiscoveryPending`. If the - "reopen to retry" copy becomes inert again, these tests will catch it. + `isCacheableDiscoveryResponse`, `deriveModelDiscoveryPending`, + `isSuccessfulEmptyDiscovery`. If the "reopen to retry" copy becomes inert + again, these tests will catch it. - `desktop/tests/e2e/onboarding-agent-defaults.spec.ts` — onboarding behavior - acceptance coverage for readiness, failure states, defaults, navigation, and - persistence races. + acceptance coverage for readiness, failure states, defaults, navigation, + successful-empty vs failed optional-model discovery, and persistence races. - Rust: `runtime_metadata_env_vars` tests pin spawn-time key application. ## Keep this file true diff --git a/desktop/src/features/agents/ui/AgentConfigFields.tsx b/desktop/src/features/agents/ui/AgentConfigFields.tsx index 9c9a71ab0..4e5ba1760 100644 --- a/desktop/src/features/agents/ui/AgentConfigFields.tsx +++ b/desktop/src/features/agents/ui/AgentConfigFields.tsx @@ -125,6 +125,41 @@ export function shouldShowModelStatusMessage( return showDescriptions || status !== null; } +/** + * Whether the Model control should render given discovery state. + * + * Optional-model harnesses (Claude Code / Codex, `acpNative`) omit the control + * while discovery is in flight and after a **confirmed successful empty** + * catalog (IPC resolved, no usable options) — there is nothing useful to pick. + * Discovery failures / unavailable runtimes keep the control so #2246 failure + * UI can render. Full disclosure still shows the control when Custom model is + * available. Required-model harnesses always render the control. + */ +export function shouldRenderModelControl({ + discoveredModelOptions, + modelDiscoveryLoading, + modelDiscoverySuccessfulEmpty, + modelIsOptional, + showCustomModelOption, +}: { + discoveredModelOptions: readonly { id: string }[] | null; + modelDiscoveryLoading: boolean; + /** True only when discovery IPC resolved with a response that yielded no options. */ + modelDiscoverySuccessfulEmpty: boolean; + modelIsOptional: boolean; + showCustomModelOption: boolean; +}): boolean { + if (!modelIsOptional) return true; + if (modelDiscoveryLoading) return false; + const hasExplicitModel = (discoveredModelOptions ?? []).some( + (option) => option.id.trim().length > 0, + ); + if (hasExplicitModel) return true; + if (showCustomModelOption) return true; + // Omit only on confirmed successful empty — not on failure/unavailable. + return !modelDiscoverySuccessfulEmpty; +} + export type AgentConfigFieldsProps = { bakedEnv: BakedEnvEntry[]; selectedRuntime: AcpRuntimeCatalogEntry | undefined; @@ -277,6 +312,7 @@ export function AgentConfigFields({ discoveredModelOptions, modelDiscoveryLoading, modelDiscoveryStatus, + modelDiscoverySuccessfulEmpty, } = usePersonaModelDiscovery({ envVars: config.env_vars, isCustomProviderEditing: isCustomProvider, @@ -285,6 +321,18 @@ export function AgentConfigFields({ provider: providerForDiscovery, selectedRuntime, }); + const modelControlVisible = shouldRenderModelControl({ + discoveredModelOptions: dependentFieldsDisabled + ? null + : discoveredModelOptions, + modelDiscoveryLoading: dependentFieldsDisabled + ? false + : modelDiscoveryLoading, + modelDiscoverySuccessfulEmpty: + !dependentFieldsDisabled && modelDiscoverySuccessfulEmpty, + modelIsOptional, + showCustomModelOption, + }); // Mount-time healing policy: onboarding page 4 edits the root config during // first-run (no higher layers to inherit from), so acting on open is safe @@ -353,16 +401,23 @@ export function AgentConfigFields({ // the old harness. In onboarding, heal that stale value as soon as the new // harness catalog proves it is unsupported; otherwise a Codex id like // `gpt-5.5[low]` appears as a Claude Code custom model. + // Also clear when the Model control is omitted after a confirmed successful + // empty catalog — never while discovery failed/unavailable (transient + // failures must not erase saved model/effort). React.useEffect(() => { if (!healOnMount) return; const currentModel = (config.model ?? "").trim(); if (currentModel.length === 0) return; - if (modelDiscoveryLoading || discoveredModelOptions === null) return; - if ( - discoveredModelOptions.some((option) => option.id.trim() === currentModel) - ) { - return; - } + if (modelDiscoveryLoading) return; + + const catalogMiss = + discoveredModelOptions !== null && + !discoveredModelOptions.some( + (option) => option.id.trim() === currentModel, + ); + const omittedAfterSuccessfulEmpty = + modelIsOptional && !modelControlVisible && modelDiscoverySuccessfulEmpty; + if (!catalogMiss && !omittedAfterSuccessfulEmpty) return; const nextEnvVars = { ...config.env_vars }; if (effortPersistenceKey) delete nextEnvVars[effortPersistenceKey]; @@ -371,7 +426,10 @@ export function AgentConfigFields({ }, [ config, discoveredModelOptions, + modelControlVisible, modelDiscoveryLoading, + modelDiscoverySuccessfulEmpty, + modelIsOptional, onConfigChange, onCustomModelEditingChange, healOnMount, @@ -659,53 +717,55 @@ export function AgentConfigFields({ </div> ) : null} - {/* Model field */} - <div className={showDescriptions ? fieldClassName : undefined}> - <AgentModelField - allowDefaultModel={fallbackModel !== null} - defaultModelLabel={ - fallbackModel ? `Default model (${fallbackModel})` : undefined - } - disableSelectDuringDiscovery={disableModelSelectDuringDiscovery} - disabled={dependentFieldsDisabled} - discoveredModelOptions={ - dependentFieldsDisabled ? null : discoveredModelOptions - } - globalModel={fallbackModel ?? undefined} - id="global-agent-model" - isCustomModelEditing={isCustomModelEditing} - isRequired={ - showRequiredIndicators && - !modelIsOptional && - fallbackModel === null && - !dependentFieldsDisabled - } - keepSelectedModelValueLabel - model={dependentFieldsDisabled ? "" : (config.model ?? "")} - modelDiscoveryLoading={ - dependentFieldsDisabled ? false : modelDiscoveryLoading - } - modelDiscoveryStatus={ - dependentFieldsDisabled ? null : modelDiscoveryStatus - } - onIsCustomModelEditingChange={onCustomModelEditingChange} - onModelChange={handleModelChange} - placeholderClassName={placeholderClassName} - placeholder="Select a model" - provider={providerForDiscovery} - fieldClassName={unstyled ? fieldClassName : undefined} - labelClassName={fieldLabelClassName} - selectClassName={selectClassName} - showCustomModelOption={showCustomModelOption} - showStatusMessage={shouldShowModelStatusMessage( - showDescriptions, - modelDiscoveryStatus, - )} - testId="global-agent-model" - useCustomSelect={useCustomSelect} - useChevronIcon={useChevronSelectIcon} - /> - </div> + {/* Model field — omitted only after confirmed successful empty discovery */} + {modelControlVisible ? ( + <div className={showDescriptions ? fieldClassName : undefined}> + <AgentModelField + allowDefaultModel={fallbackModel !== null} + defaultModelLabel={ + fallbackModel ? `Default model (${fallbackModel})` : undefined + } + disableSelectDuringDiscovery={disableModelSelectDuringDiscovery} + disabled={dependentFieldsDisabled} + discoveredModelOptions={ + dependentFieldsDisabled ? null : discoveredModelOptions + } + globalModel={fallbackModel ?? undefined} + id="global-agent-model" + isCustomModelEditing={isCustomModelEditing} + isRequired={ + showRequiredIndicators && + !modelIsOptional && + fallbackModel === null && + !dependentFieldsDisabled + } + keepSelectedModelValueLabel + model={dependentFieldsDisabled ? "" : (config.model ?? "")} + modelDiscoveryLoading={ + dependentFieldsDisabled ? false : modelDiscoveryLoading + } + modelDiscoveryStatus={ + dependentFieldsDisabled ? null : modelDiscoveryStatus + } + onIsCustomModelEditingChange={onCustomModelEditingChange} + onModelChange={handleModelChange} + placeholderClassName={placeholderClassName} + placeholder="Select a model" + provider={providerForDiscovery} + fieldClassName={unstyled ? fieldClassName : undefined} + labelClassName={fieldLabelClassName} + selectClassName={selectClassName} + showCustomModelOption={showCustomModelOption} + showStatusMessage={shouldShowModelStatusMessage( + showDescriptions, + dependentFieldsDisabled ? null : modelDiscoveryStatus, + )} + testId="global-agent-model" + useCustomSelect={useCustomSelect} + useChevronIcon={useChevronSelectIcon} + /> + </div> + ) : null} {/* Thinking / Effort */} {effortFieldVisible ? ( diff --git a/desktop/src/features/agents/ui/agentConfigControls.tsx b/desktop/src/features/agents/ui/agentConfigControls.tsx index df0321fa7..05cc2e48e 100644 --- a/desktop/src/features/agents/ui/agentConfigControls.tsx +++ b/desktop/src/features/agents/ui/agentConfigControls.tsx @@ -63,6 +63,7 @@ export function AgentDropdownSelect({ ariaRequired, className, disabled = false, + emptyOptionsLabel = "No options available", id, onValueChange, options, @@ -77,6 +78,8 @@ export function AgentDropdownSelect({ ariaRequired?: boolean; className?: string; disabled?: boolean; + /** Shown when the option list is empty (not a search filter miss). */ + emptyOptionsLabel?: string; id: string; onValueChange: (value: string) => void; options: readonly AgentDropdownOption[]; @@ -184,8 +187,15 @@ export function AgentDropdownSelect({ /> </div> ) : null} - {showSearch && filteredOptions.length === 0 ? ( - <p className="px-3 py-2 text-sm text-foreground/55">No matches</p> + {filteredOptions.length === 0 ? ( + <p + className="px-3 py-2 text-sm text-foreground/55" + data-testid={testId ? `${testId}-empty` : undefined} + > + {showSearch && query.trim().length > 0 + ? "No matches" + : emptyOptionsLabel} + </p> ) : null} {filteredOptions.map((option) => { const selected = option.value === value; @@ -470,6 +480,7 @@ export function AgentModelField({ ariaRequired={isRequired} className={selectClassName} disabled={selectDisabled} + emptyOptionsLabel="Couldn't load models" id={id} onValueChange={handleModelSelectChange} options={modelOptions} diff --git a/desktop/src/features/agents/ui/agentConfigFieldsContract.test.mjs b/desktop/src/features/agents/ui/agentConfigFieldsContract.test.mjs index 8933ffb25..2435130f6 100644 --- a/desktop/src/features/agents/ui/agentConfigFieldsContract.test.mjs +++ b/desktop/src/features/agents/ui/agentConfigFieldsContract.test.mjs @@ -21,6 +21,7 @@ import test from "node:test"; import { CANONICAL_CONFIG_BEHAVIORS, resolveDisclosure, + shouldRenderModelControl, shouldShowModelStatusMessage, } from "./AgentConfigFields.tsx"; @@ -83,3 +84,94 @@ test("shouldShowModelStatusMessage_onboardingPreset_warningStatus_showsMessage", }; assert.equal(shouldShowModelStatusMessage(showDescriptions, warning), true); }); + +// ── shouldRenderModelControl ────────────────────────────────────────────────── +// Optional-model harnesses omit the control only after a confirmed successful +// empty catalog. Failures keep the control so #2246 status UI can render. + +test("optional model control hides while loading", () => { + assert.equal( + shouldRenderModelControl({ + discoveredModelOptions: null, + modelDiscoveryLoading: true, + modelDiscoverySuccessfulEmpty: false, + modelIsOptional: true, + showCustomModelOption: false, + }), + false, + "optional + loading must hide the control", + ); +}); + +test("optional model control hides after successful empty discovery", () => { + assert.equal( + shouldRenderModelControl({ + discoveredModelOptions: null, + modelDiscoveryLoading: false, + modelDiscoverySuccessfulEmpty: true, + modelIsOptional: true, + showCustomModelOption: false, + }), + false, + "optional + successful empty + no custom must hide the control", + ); +}); + +test("optional model control stays visible on discovery failure", () => { + assert.equal( + shouldRenderModelControl({ + discoveredModelOptions: null, + modelDiscoveryLoading: false, + modelDiscoverySuccessfulEmpty: false, + modelIsOptional: true, + showCustomModelOption: false, + }), + true, + "optional + failed/unavailable discovery must keep the control", + ); +}); + +test("optional model control shows when explicit models are available", () => { + assert.equal( + shouldRenderModelControl({ + discoveredModelOptions: [ + { id: "", label: "Default model" }, + { id: "claude-sonnet-4", label: "Claude Sonnet 4" }, + ], + modelDiscoveryLoading: false, + modelDiscoverySuccessfulEmpty: false, + modelIsOptional: true, + showCustomModelOption: false, + }), + true, + "explicit models must show the control", + ); +}); + +test("full disclosure keeps Custom escape hatch when discovery is empty", () => { + assert.equal( + shouldRenderModelControl({ + discoveredModelOptions: null, + modelDiscoveryLoading: false, + modelDiscoverySuccessfulEmpty: true, + modelIsOptional: true, + showCustomModelOption: true, + }), + true, + "full disclosure keeps Custom escape hatch when discovery is empty", + ); +}); + +test("required-model harnesses always keep the control", () => { + assert.equal( + shouldRenderModelControl({ + discoveredModelOptions: null, + modelDiscoveryLoading: false, + modelDiscoverySuccessfulEmpty: true, + modelIsOptional: false, + showCustomModelOption: false, + }), + true, + "required-model harnesses always keep the control", + ); +}); diff --git a/desktop/src/features/agents/ui/usePersonaModelDiscovery.test.mjs b/desktop/src/features/agents/ui/usePersonaModelDiscovery.test.mjs index 0246fe6ac..ecb36a6fc 100644 --- a/desktop/src/features/agents/ui/usePersonaModelDiscovery.test.mjs +++ b/desktop/src/features/agents/ui/usePersonaModelDiscovery.test.mjs @@ -5,6 +5,7 @@ import { deriveModelDiscoveryPending, getDiscoveredPersonaModelOptions, isCacheableDiscoveryResponse, + isSuccessfulEmptyDiscovery, synthesizeEmptyDiscoveryStatus, } from "./usePersonaModelDiscovery.ts"; @@ -258,3 +259,56 @@ test("deriveModelDiscoveryPending_noKey_isNotPending", () => { false, ); }); + +// ── isSuccessfulEmptyDiscovery ──────────────────────────────────────────────── + +test("isSuccessfulEmptyDiscovery_resolvedEmptyResponse_isTrue", () => { + assert.equal( + isSuccessfulEmptyDiscovery({ + activeModelDiscoveryData: response({ models: [] }), + discoveredModelOptions: null, + modelDiscoveryPending: false, + }), + true, + ); +}); + +test("isSuccessfulEmptyDiscovery_thrownFailure_isFalse", () => { + // Failure path leaves data null — must not be treated as successful empty. + assert.equal( + isSuccessfulEmptyDiscovery({ + activeModelDiscoveryData: null, + discoveredModelOptions: null, + modelDiscoveryPending: false, + }), + false, + ); +}); + +test("isSuccessfulEmptyDiscovery_withUsableModels_isFalse", () => { + assert.equal( + isSuccessfulEmptyDiscovery({ + activeModelDiscoveryData: response({ + models: [ + { id: "claude-sonnet-5", name: "Claude Sonnet 5", description: null }, + ], + }), + discoveredModelOptions: [ + { id: "claude-sonnet-5", label: "Claude Sonnet 5" }, + ], + modelDiscoveryPending: false, + }), + false, + ); +}); + +test("isSuccessfulEmptyDiscovery_stillPending_isFalse", () => { + assert.equal( + isSuccessfulEmptyDiscovery({ + activeModelDiscoveryData: null, + discoveredModelOptions: null, + modelDiscoveryPending: true, + }), + false, + ); +}); diff --git a/desktop/src/features/agents/ui/usePersonaModelDiscovery.ts b/desktop/src/features/agents/ui/usePersonaModelDiscovery.ts index 2bbed69c2..b0ab0f837 100644 --- a/desktop/src/features/agents/ui/usePersonaModelDiscovery.ts +++ b/desktop/src/features/agents/ui/usePersonaModelDiscovery.ts @@ -139,6 +139,28 @@ export function deriveModelDiscoveryPending({ ); } +/** + * True when discovery IPC resolved with a response that yielded no usable + * model options. Distinct from a thrown/unavailable failure (data stays null). + * Callers that omit the Model control or heal persisted values must gate on + * this — not on `discoveredModelOptions === null` alone. + */ +export function isSuccessfulEmptyDiscovery({ + activeModelDiscoveryData, + discoveredModelOptions, + modelDiscoveryPending, +}: { + activeModelDiscoveryData: AgentModelsResponse | null; + discoveredModelOptions: readonly PersonaModelOption[] | null; + modelDiscoveryPending: boolean; +}): boolean { + return ( + !modelDiscoveryPending && + activeModelDiscoveryData !== null && + discoveredModelOptions === null + ); +} + export function usePersonaModelDiscovery({ envVars, isCustomProviderEditing, @@ -358,6 +380,11 @@ export function usePersonaModelDiscovery({ activeModelDiscoveryData, activeModelDiscoveryStatus, }); + const modelDiscoverySuccessfulEmpty = isSuccessfulEmptyDiscovery({ + activeModelDiscoveryData, + discoveredModelOptions, + modelDiscoveryPending, + }); return { discoveredModelOptions, @@ -366,5 +393,6 @@ export function usePersonaModelDiscovery({ modelDiscoveryPending || discoveredModelOptions !== null ? null : activeModelDiscoveryStatus, + modelDiscoverySuccessfulEmpty, }; } diff --git a/desktop/src/features/agents/ui/useTeamActions.ts b/desktop/src/features/agents/ui/useTeamActions.ts index 64fe931cb..3652acf95 100644 --- a/desktop/src/features/agents/ui/useTeamActions.ts +++ b/desktop/src/features/agents/ui/useTeamActions.ts @@ -215,6 +215,7 @@ export function useTeamActions( id: team.id, name: team.name, description: team.description ?? "", + instructions: team.instructions ?? undefined, personaIds: [...team.personaIds], }, }); diff --git a/desktop/src/features/channels/lib/canonicalChannelName.test.mjs b/desktop/src/features/channels/lib/canonicalChannelName.test.mjs new file mode 100644 index 000000000..88bd45199 --- /dev/null +++ b/desktop/src/features/channels/lib/canonicalChannelName.test.mjs @@ -0,0 +1,22 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { + canonicalChannelName, + channelNamesMatch, +} from "./canonicalChannelName.ts"; + +test("canonicalChannelName strips interleaved leading hashes and whitespace", () => { + assert.equal(canonicalChannelName("channel"), "channel"); + assert.equal(canonicalChannelName("#channel"), "channel"); + assert.equal(canonicalChannelName(" ### channel "), "channel"); + assert.equal(canonicalChannelName("# #"), ""); + assert.equal(canonicalChannelName("### ###"), ""); + assert.equal(canonicalChannelName("channel#topic"), "channel#topic"); +}); + +test("channelNamesMatch canonicalizes both legacy names and search input", () => { + assert.equal(channelNamesMatch("#general", "general"), true); + assert.equal(channelNamesMatch("general", " #GENERAL "), true); + assert.equal(channelNamesMatch("#random", "general"), false); +}); diff --git a/desktop/src/features/channels/lib/canonicalChannelName.ts b/desktop/src/features/channels/lib/canonicalChannelName.ts new file mode 100644 index 000000000..8e8ca470e --- /dev/null +++ b/desktop/src/features/channels/lib/canonicalChannelName.ts @@ -0,0 +1,14 @@ +/** + * Returns the stored form of a channel name after removing the display prefix. + * Keep this aligned with `buzz_core::channel::canonical_channel_name`. + */ +export function canonicalChannelName(name: string): string { + return name.replace(/^[#\s]+/u, "").trimEnd(); +} + +export function channelNamesMatch(left: string, right: string): boolean { + return ( + canonicalChannelName(left).toLowerCase() === + canonicalChannelName(right).toLowerCase() + ); +} diff --git a/desktop/src/features/channels/ui/ChannelBrowserDialog.tsx b/desktop/src/features/channels/ui/ChannelBrowserDialog.tsx index b1371f8bd..6d56e4dbf 100644 --- a/desktop/src/features/channels/ui/ChannelBrowserDialog.tsx +++ b/desktop/src/features/channels/ui/ChannelBrowserDialog.tsx @@ -9,6 +9,10 @@ import { } from "lucide-react"; import type { Channel } from "@/shared/api/types"; +import { + canonicalChannelName, + channelNamesMatch, +} from "@/features/channels/lib/canonicalChannelName"; import { scoreChannelMatch } from "@/features/channels/lib/channelSearchScore"; import { type ChannelSortMode, @@ -127,8 +131,9 @@ export function ChannelBrowserDialog({ left: 0, width: 0, }); - const deferredQuery = React.useDeferredValue(query.trim().toLowerCase()); - const trimmedQuery = query.trim(); + const canonicalQuery = canonicalChannelName(query); + const deferredQuery = React.useDeferredValue(canonicalQuery.toLowerCase()); + const trimmedQuery = canonicalQuery; // Immediate (non-deferred) lowercased query. The create row's visibility // (via hasExactMatch) and its label both read from the live query so they // can never disagree for a frame while the fuzzy filter catches up. @@ -244,7 +249,7 @@ export function ChannelBrowserDialog({ channels.some( (channel) => channel.channelType !== "dm" && - channel.name.toLowerCase() === normalizedQuery && + channelNamesMatch(channel.name, normalizedQuery) && (channelTypeFilter ? channel.channelType === channelTypeFilter : true), diff --git a/desktop/src/features/communities/applyCommunitiesOrder.test.mjs b/desktop/src/features/communities/applyCommunitiesOrder.test.mjs new file mode 100644 index 000000000..ed36f4826 --- /dev/null +++ b/desktop/src/features/communities/applyCommunitiesOrder.test.mjs @@ -0,0 +1,110 @@ +/** + * Unit tests for applyCommunitiesOrder — the pure permutation helper that + * drives community-rail drag-to-reorder. + */ +import assert from "node:assert/strict"; +import { describe, it } from "node:test"; + +import { applyCommunitiesOrder } from "./useCommunities.tsx"; + +const A = { + id: "ws-a", + name: "Alpha", + relayUrl: "wss://a.example.com", + addedAt: "2024-01-01", +}; +const B = { + id: "ws-b", + name: "Bravo", + relayUrl: "wss://b.example.com", + addedAt: "2024-01-02", +}; +const C = { + id: "ws-c", + name: "Charlie", + relayUrl: "wss://c.example.com", + addedAt: "2024-01-03", +}; + +describe("applyCommunitiesOrder", () => { + it("reorders communities to match orderedIds", () => { + const result = applyCommunitiesOrder([A, B, C], ["ws-c", "ws-a", "ws-b"]); + assert.deepEqual( + result.map((c) => c.id), + ["ws-c", "ws-a", "ws-b"], + ); + }); + + it("returns same order when orderedIds matches current order", () => { + const result = applyCommunitiesOrder([A, B, C], ["ws-a", "ws-b", "ws-c"]); + assert.deepEqual( + result.map((c) => c.id), + ["ws-a", "ws-b", "ws-c"], + ); + }); + + it("appends communities not mentioned in orderedIds at the end in original relative order", () => { + // C was added after drag — not in orderedIds — should tail-append + const result = applyCommunitiesOrder([A, B, C], ["ws-b", "ws-a"]); + assert.deepEqual( + result.map((c) => c.id), + ["ws-b", "ws-a", "ws-c"], + ); + }); + + it("handles orderedIds that contain stale IDs not present in communities", () => { + // "ws-gone" is a stale id — silent skip, no crash + const result = applyCommunitiesOrder([A, B], ["ws-b", "ws-gone", "ws-a"]); + assert.deepEqual( + result.map((c) => c.id), + ["ws-b", "ws-a"], + ); + }); + + it("returns the full list when orderedIds is empty — original order preserved", () => { + const result = applyCommunitiesOrder([A, B, C], []); + assert.deepEqual( + result.map((c) => c.id), + ["ws-a", "ws-b", "ws-c"], + ); + }); + + it("handles a single-element list (no-op reorder)", () => { + const result = applyCommunitiesOrder([A], ["ws-a"]); + assert.deepEqual( + result.map((c) => c.id), + ["ws-a"], + ); + }); + + it("handles an empty communities list", () => { + const result = applyCommunitiesOrder([], ["ws-a", "ws-b"]); + assert.deepEqual(result, []); + }); + + it("does not mutate the original array", () => { + const original = [A, B, C]; + applyCommunitiesOrder(original, ["ws-c", "ws-a", "ws-b"]); + assert.deepEqual( + original.map((c) => c.id), + ["ws-a", "ws-b", "ws-c"], + ); + }); + + it("preserves object identity of each community (no clone)", () => { + const result = applyCommunitiesOrder([A, B, C], ["ws-c", "ws-b", "ws-a"]); + assert.equal(result[0], C); + assert.equal(result[1], B); + assert.equal(result[2], A); + }); + + it("handles duplicate IDs in orderedIds — first occurrence wins", () => { + // Defensive: dnd-kit should never produce duplicates, but guard anyway. + const result = applyCommunitiesOrder([A, B, C], ["ws-b", "ws-b", "ws-a"]); + // ws-b appears once, ws-a once, ws-c appended + assert.deepEqual( + result.map((c) => c.id), + ["ws-b", "ws-a", "ws-c"], + ); + }); +}); diff --git a/desktop/src/features/communities/useCommunities.tsx b/desktop/src/features/communities/useCommunities.tsx index 48668a374..5744d42ba 100644 --- a/desktop/src/features/communities/useCommunities.tsx +++ b/desktop/src/features/communities/useCommunities.tsx @@ -72,6 +72,42 @@ export function resolveCommunityUpdateResult( return { kind: "updated", requiresReinit: backendFieldsChanged }; } +/** + * Permute `communities` so that its order matches `orderedIds`. + * + * - Communities whose id appears in `orderedIds` are placed first, in the + * order given by `orderedIds`. + * - Communities not mentioned in `orderedIds` (e.g. added after the drag + * completed) are appended at the end in their original relative order. + * + * Pure and side-effect-free — extracted so it can be unit-tested without + * a DOM or React. + */ +export function applyCommunitiesOrder( + communities: Community[], + orderedIds: string[], +): Community[] { + const byId = new Map(communities.map((c) => [c.id, c])); + const seen = new Set<string>(); + const reordered: Community[] = []; + + for (const id of orderedIds) { + const c = byId.get(id); + if (c && !seen.has(id)) { + reordered.push(c); + seen.add(id); + } + } + + for (const c of communities) { + if (!seen.has(c.id)) { + reordered.push(c); + } + } + + return reordered; +} + export type UseCommunitiesReturn = { communities: Community[]; activeCommunity: Community | null; @@ -90,6 +126,8 @@ export type UseCommunitiesReturn = { Pick<Community, "name" | "relayUrl" | "token" | "pubkey" | "reposDir"> >, ) => UpdateCommunityResult; + /** Persist a new display order for the rail. IDs not in orderedIds keep their relative position at the end. */ + reorderCommunities: (orderedIds: string[]) => void; }; const CommunitiesContext = createContext<UseCommunitiesReturn | null>(null); @@ -242,6 +280,14 @@ function useCommunitiesInternal(): UseCommunitiesReturn { [activeId], ); + const reorderCommunities = useCallback((orderedIds: string[]) => { + setCommunitiesState((prev) => { + const next = applyCommunitiesOrder(prev, orderedIds); + saveCommunities(next); + return next; + }); + }, []); + return { communities, activeCommunity, @@ -252,5 +298,6 @@ function useCommunitiesInternal(): UseCommunitiesReturn { switchCommunity, reconnectCommunity, updateCommunity, + reorderCommunities, }; } diff --git a/desktop/src/features/onboarding/ui/MachineOnboardingFlow.tsx b/desktop/src/features/onboarding/ui/MachineOnboardingFlow.tsx index 89c284f23..e400d07a9 100644 --- a/desktop/src/features/onboarding/ui/MachineOnboardingFlow.tsx +++ b/desktop/src/features/onboarding/ui/MachineOnboardingFlow.tsx @@ -215,7 +215,14 @@ export function MachineOnboardingFlow({ back: () => setPage(identityWasImported ? "key-import" : "backup"), next: (runtimeIds) => { - setReadyRuntimeIds(Array.from(runtimeIds)); + const ids = Array.from(runtimeIds); + setReadyRuntimeIds(ids); + // Harness install can fail (Windows/PATH/network). Don't soft-lock + // onboarding — users can finish setup later in Settings → Agents. + if (ids.length === 0) { + complete(selectedPubkey ?? undefined); + return; + } setPage("config"); }, }} diff --git a/desktop/src/features/onboarding/ui/SetupStep.tsx b/desktop/src/features/onboarding/ui/SetupStep.tsx index 6111589ce..96e7322d1 100644 --- a/desktop/src/features/onboarding/ui/SetupStep.tsx +++ b/desktop/src/features/onboarding/ui/SetupStep.tsx @@ -687,6 +687,16 @@ function SetupStepContent({ Next </Button> + <Button + className="h-9 rounded-full bg-foreground/10 px-6 text-sm hover:bg-foreground/15" + data-testid="onboarding-setup-skip" + onClick={() => actions.next([])} + type="button" + variant="ghost" + > + Skip for now + </Button> + <Button className="h-9 rounded-full bg-foreground/10 px-6 text-sm hover:bg-foreground/15" data-testid="onboarding-back" diff --git a/desktop/src/features/sidebar/ui/CommunityRail.tsx b/desktop/src/features/sidebar/ui/CommunityRail.tsx index a234d6cf2..2737a41f4 100644 --- a/desktop/src/features/sidebar/ui/CommunityRail.tsx +++ b/desktop/src/features/sidebar/ui/CommunityRail.tsx @@ -1,3 +1,20 @@ +import { + DndContext, + DragOverlay, + KeyboardSensor, + PointerSensor, + useSensor, + useSensors, +} from "@dnd-kit/core"; +import type { DragEndEvent, DragStartEvent } from "@dnd-kit/core"; +import { + SortableContext, + arrayMove, + sortableKeyboardCoordinates, + verticalListSortingStrategy, + useSortable, +} from "@dnd-kit/sortable"; +import { CSS } from "@dnd-kit/utilities"; import { CheckCheck, Link2, Plus, Settings2 } from "lucide-react"; import * as React from "react"; @@ -33,6 +50,7 @@ type CommunityRailProps = { updates: Partial<Pick<Community, "name" | "relayUrl" | "token">>, ) => void; onRemoveCommunity: (id: string) => void; + onReorderCommunities: (orderedIds: string[]) => void; }; const MAX_BADGE = 99; @@ -76,6 +94,9 @@ function CommunityButton({ iconUrl, onSwitch, menu, + dragListeners, + dragAttributes, + isDragging, }: { community: Community; isActive: boolean; @@ -83,6 +104,9 @@ function CommunityButton({ iconUrl: string | null; onSwitch: () => void; menu: React.ReactNode; + dragListeners?: React.HTMLAttributes<HTMLElement>; + dragAttributes?: React.HTMLAttributes<HTMLElement>; + isDragging?: boolean; }) { const { mentionCount, showBadge, showDot, pending, badgeLabel } = communityRailIndicators(unread); @@ -101,10 +125,15 @@ function CommunityButton({ <button aria-current={isActive ? "true" : undefined} aria-label={tooltipLabel} - className="relative flex h-9 w-9 items-center justify-center outline-hidden focus:outline-none focus-visible:outline-none" + className={cn( + "relative flex h-9 w-9 items-center justify-center touch-none outline-hidden focus:outline-none focus-visible:outline-none", + isDragging && "opacity-30", + )} data-testid={`community-rail-button-${community.id}`} onClick={onSwitch} type="button" + {...dragAttributes} + {...dragListeners} > <span className={cn( @@ -154,6 +183,105 @@ function CommunityButton({ ); } +function CommunityDragOverlay({ + community, + iconUrl, +}: { + community: Community; + iconUrl: string | null; +}) { + return ( + <div + className="flex h-9 w-9 cursor-grabbing items-center justify-center overflow-hidden rounded-xl bg-primary text-xs font-semibold text-primary-foreground opacity-90 shadow-lg ring-1 ring-sidebar-border" + data-buzz-flat + > + {iconUrl ? ( + <img + alt="" + className="h-full w-full object-cover" + draggable={false} + src={iconUrl} + /> + ) : ( + getInitials(community.name) || "🐝" + )} + </div> + ); +} + +function SortableCommunityButton({ + community, + activeCommunityId, + iconsByCommunity, + unreadByCommunity, + onSwitchCommunity, + onMarkAllRead, + onSetEditingCommunity, +}: { + community: Community; + activeCommunityId: string | null; + iconsByCommunity: Record<string, string | null | undefined>; + unreadByCommunity: Record<string, CommunityUnreadState>; + onSwitchCommunity: (id: string) => void; + onMarkAllRead: (community: Community) => void; + onSetEditingCommunity: (community: Community) => void; +}) { + const { + attributes, + listeners, + setNodeRef, + transform, + transition, + isDragging, + } = useSortable({ id: community.id }); + + const style: React.CSSProperties = { + transform: CSS.Transform.toString(transform), + transition, + }; + + return ( + <div ref={setNodeRef} style={style}> + <CommunityButton + community={community} + dragAttributes={attributes} + dragListeners={listeners} + iconUrl={iconsByCommunity[community.id] ?? null} + isActive={community.id === activeCommunityId} + isDragging={isDragging} + menu={ + <> + <ContextMenuItem onClick={() => onMarkAllRead(community)}> + <CheckCheck className="h-4 w-4" /> + Mark all as read + </ContextMenuItem> + <ContextMenuItem + onClick={() => { + void writeTextToClipboard(community.relayUrl); + }} + > + <Link2 className="h-4 w-4" /> + Copy relay URL + </ContextMenuItem> + <ContextMenuSeparator /> + <ContextMenuItem onClick={() => onSetEditingCommunity(community)}> + <Settings2 className="h-4 w-4" /> + Community settings + </ContextMenuItem> + </> + } + onSwitch={() => onSwitchCommunity(community.id)} + unread={ + unreadByCommunity[community.id] ?? { + hasUnread: false, + state: "unknown", + } + } + /> + </div> + ); +} + /** * Discord/Slack-style vertical rail of communities on the far left of the app. * Shows a mention-count badge for inactive communities (observed via @@ -169,6 +297,7 @@ export function CommunityRail({ onAddCommunity, onUpdateCommunity, onRemoveCommunity, + onReorderCommunities, }: CommunityRailProps) { const { unreadByCommunity, markCommunityRead } = useCommunityUnread( communities, @@ -179,10 +308,39 @@ export function CommunityRail({ const { markAllChannelsRead } = useAppShell(); const [editingCommunity, setEditingCommunity] = React.useState<Community | null>(null); + const [draggingId, setDraggingId] = React.useState<string | null>(null); + + const sensors = useSensors( + useSensor(PointerSensor, { activationConstraint: { distance: 6 } }), + useSensor(KeyboardSensor, { + coordinateGetter: sortableKeyboardCoordinates, + }), + ); + if (communities.length <= 1) { return null; } + const communityIds = communities.map((c) => c.id); + const draggingCommunity = draggingId + ? (communities.find((c) => c.id === draggingId) ?? null) + : null; + + const handleDragStart = (event: DragStartEvent) => { + setDraggingId(event.active.id as string); + }; + + const handleDragEnd = (event: DragEndEvent) => { + setDraggingId(null); + const { active, over } = event; + if (!over || active.id === over.id) return; + const oldIdx = communityIds.indexOf(active.id as string); + const newIdx = communityIds.indexOf(over.id as string); + if (oldIdx !== -1 && newIdx !== -1) { + onReorderCommunities(arrayMove(communityIds, oldIdx, newIdx)); + } + }; + const handleMarkAllRead = (community: Community) => { if (community.id === activeCommunityId) { markAllChannelsRead(); @@ -211,42 +369,37 @@ export function CommunityRail({ )} data-testid="community-rail" > - {communities.map((community) => ( - <CommunityButton - key={community.id} - iconUrl={iconsByCommunity[community.id] ?? null} - isActive={community.id === activeCommunityId} - menu={ - <> - <ContextMenuItem onClick={() => handleMarkAllRead(community)}> - <CheckCheck className="h-4 w-4" /> - Mark all as read - </ContextMenuItem> - <ContextMenuItem - onClick={() => { - void writeTextToClipboard(community.relayUrl); - }} - > - <Link2 className="h-4 w-4" /> - Copy relay URL - </ContextMenuItem> - <ContextMenuSeparator /> - <ContextMenuItem onClick={() => setEditingCommunity(community)}> - <Settings2 className="h-4 w-4" /> - Community settings - </ContextMenuItem> - </> - } - onSwitch={() => onSwitchCommunity(community.id)} - unread={ - unreadByCommunity[community.id] ?? { - hasUnread: false, - state: "unknown", - } - } - community={community} - /> - ))} + <DndContext + onDragEnd={handleDragEnd} + onDragStart={handleDragStart} + sensors={sensors} + > + <SortableContext + items={communityIds} + strategy={verticalListSortingStrategy} + > + {communities.map((community) => ( + <SortableCommunityButton + key={community.id} + activeCommunityId={activeCommunityId} + community={community} + iconsByCommunity={iconsByCommunity} + unreadByCommunity={unreadByCommunity} + onMarkAllRead={handleMarkAllRead} + onSetEditingCommunity={setEditingCommunity} + onSwitchCommunity={onSwitchCommunity} + /> + ))} + </SortableContext> + <DragOverlay> + {draggingCommunity ? ( + <CommunityDragOverlay + community={draggingCommunity} + iconUrl={iconsByCommunity[draggingCommunity.id] ?? null} + /> + ) : null} + </DragOverlay> + </DndContext> <Tooltip> <TooltipTrigger asChild> <button diff --git a/desktop/src/testing/e2eBridge.ts b/desktop/src/testing/e2eBridge.ts index 3c5f8f8a4..35bcd75ff 100644 --- a/desktop/src/testing/e2eBridge.ts +++ b/desktop/src/testing/e2eBridge.ts @@ -393,6 +393,25 @@ type E2eConfig = { * spec can interleave edits and exercise the mid-save race handling. */ globalConfigSaveDelayMs?: number; + /** + * Override the `discover_agent_models` mock response. When set, returns + * this catalog instead of the default per-harness model list. + */ + discoverAgentModels?: { + models: Array<{ + id: string; + name: string | null; + description?: string | null; + }>; + supportsSwitching: boolean; + agentDefaultModel?: string | null; + selectedModel?: string | null; + }; + /** + * When set, `discover_agent_models` throws with this message instead of + * returning a catalog. + */ + discoverAgentModelsError?: string; }; relayHttpUrl?: string; relayWsUrl?: string; @@ -10136,6 +10155,25 @@ export function maybeInstallE2eTauriMocks() { supportsSwitching: false, }; case "discover_agent_models": { + const discoverError = activeConfig?.mock?.discoverAgentModelsError; + if (discoverError) { + throw new Error(discoverError); + } + const discoverOverride = activeConfig?.mock?.discoverAgentModels; + if (discoverOverride) { + return { + agentName: "mock-agent", + agentVersion: "0.0.0", + models: discoverOverride.models.map((model) => ({ + id: model.id, + name: model.name, + description: model.description ?? null, + })), + agentDefaultModel: discoverOverride.agentDefaultModel ?? null, + selectedModel: discoverOverride.selectedModel ?? null, + supportsSwitching: discoverOverride.supportsSwitching, + }; + } const input = ( payload as { input?: { agentCommand?: string; provider?: string }; diff --git a/desktop/tests/e2e/community-rail.spec.ts b/desktop/tests/e2e/community-rail.spec.ts index 483f12c79..65fb3cd48 100644 --- a/desktop/tests/e2e/community-rail.spec.ts +++ b/desktop/tests/e2e/community-rail.spec.ts @@ -216,4 +216,148 @@ test.describe("community rail", () => { expect(toggleBox).not.toBeNull(); expect(toggleBox?.x ?? 0).toBeLessThan(120); }); + + test("drag-to-reorder updates the stored community order and survives reload", async ({ + page, + }) => { + await installMockBridge(page, undefined, { skipCommunitySeed: true }); + // Seed only if not already set so the persisted order survives page.reload(). + await page.addInitScript( + ({ list, active }) => { + if (!window.localStorage.getItem("buzz-communities")) { + window.localStorage.setItem("buzz-communities", JSON.stringify(list)); + } + if (!window.localStorage.getItem("buzz-active-community-id")) { + window.localStorage.setItem("buzz-active-community-id", active); + } + }, + { list: [COMMUNITY_A, COMMUNITY_B], active: COMMUNITY_A.id }, + ); + await page.goto("/"); + + const buttonA = page.getByTestId(`community-rail-button-${COMMUNITY_A.id}`); + const buttonB = page.getByTestId(`community-rail-button-${COMMUNITY_B.id}`); + await expect(buttonA).toBeVisible(); + await expect(buttonB).toBeVisible(); + + // Drag B (lower) up over A (higher) so the order becomes [B, A]. + const boxA = await buttonA.boundingBox(); + const boxB = await buttonB.boundingBox(); + if (!boxA || !boxB) throw new Error("community buttons not laid out"); + + const startX = boxB.x + boxB.width / 2; + const startY = boxB.y + boxB.height / 2; + const targetY = boxA.y + boxA.height / 2; + + // dnd-kit PointerSensor requires a 6px activation distance before it picks + // up the drag. Move in small steps so pointermove events fire on every pixel. + await page.mouse.move(startX, startY); + await page.mouse.down(); + await page.mouse.move(startX, startY - 3, { steps: 3 }); + await page.mouse.move(startX, targetY, { steps: 20 }); + await page.mouse.up(); + + // The community list in localStorage must now be [B, A]. + await expect + .poll(() => + page.evaluate(() => { + const raw = window.localStorage.getItem("buzz-communities"); + if (!raw) return null; + const list = JSON.parse(raw) as Array<{ id: string }>; + return list.map((c) => c.id); + }), + ) + .toEqual([COMMUNITY_B.id, COMMUNITY_A.id]); + + // Verify the new order is also reflected in the rendered DOM — B button + // must appear above A button. + const newBoxA = await buttonA.boundingBox(); + const newBoxB = await buttonB.boundingBox(); + if (!newBoxA || !newBoxB) + throw new Error("community buttons not laid out after drag"); + expect(newBoxB.y).toBeLessThan(newBoxA.y); + + // Reload and confirm the order survives restart: addInitScript is + // conditional (no-op when data already exists), so the dragged [B, A] + // order is what React reads on boot. + await page.reload(); + await expect(page.getByTestId("community-rail")).toBeVisible(); + + // Storage must still be [B, A] after reload. + const storedOrder = await page.evaluate(() => { + const raw = window.localStorage.getItem("buzz-communities"); + if (!raw) return null; + const list = JSON.parse(raw) as Array<{ id: string }>; + return list.map((c) => c.id); + }); + expect(storedOrder).toEqual([COMMUNITY_B.id, COMMUNITY_A.id]); + + // DOM order must also be [B, A] after reload. + const reloadBoxA = await buttonA.boundingBox(); + const reloadBoxB = await buttonB.boundingBox(); + if (!reloadBoxA || !reloadBoxB) + throw new Error("community buttons not laid out after reload"); + expect(reloadBoxB.y).toBeLessThan(reloadBoxA.y); + }); + + test("keyboard reorder: Space to pick up, ArrowUp to move, Space to drop updates stored order", async ({ + page, + }) => { + await installMockBridge(page, undefined, { skipCommunitySeed: true }); + await seedCommunities(page, [COMMUNITY_A, COMMUNITY_B], COMMUNITY_A.id); + await page.goto("/"); + + const buttonA = page.getByTestId(`community-rail-button-${COMMUNITY_A.id}`); + const buttonB = page.getByTestId(`community-rail-button-${COMMUNITY_B.id}`); + await expect(buttonA).toBeVisible(); + await expect(buttonB).toBeVisible(); + + // Focus B (the second/lower item) and use keyboard to move it above A. + // Note: page.keyboard.press("Space") fires the button's native click on this + // Chromium build even when React's onKeyDown calls preventDefault — a CDP + // input-injection quirk. The synthetic dispatch below goes directly through + // React's event system where preventDefault correctly suppresses the click, + // while still exercising the real KeyboardSensor path (Thufir verified the + // test fails when KeyboardSensor is removed). + await buttonB.focus(); + await page.evaluate((testId) => { + const el = document.querySelector(`[data-testid="${testId}"]`); + if (!el) throw new Error(`button not found: ${testId}`); + el.dispatchEvent( + new KeyboardEvent("keydown", { + key: " ", + code: "Space", + bubbles: true, + cancelable: true, + }), + ); + }, `community-rail-button-${COMMUNITY_B.id}`); + // ArrowUp moves the active item one slot up. + await page.keyboard.press("ArrowUp"); + // Space drops the item — same synthetic dispatch for consistency. + await page.evaluate((testId) => { + const el = document.querySelector(`[data-testid="${testId}"]`); + if (!el) throw new Error(`button not found: ${testId}`); + el.dispatchEvent( + new KeyboardEvent("keydown", { + key: " ", + code: "Space", + bubbles: true, + cancelable: true, + }), + ); + }, `community-rail-button-${COMMUNITY_B.id}`); + + // The community list in localStorage must now be [B, A]. + await expect + .poll(() => + page.evaluate(() => { + const raw = window.localStorage.getItem("buzz-communities"); + if (!raw) return null; + const list = JSON.parse(raw) as Array<{ id: string }>; + return list.map((c) => c.id); + }), + ) + .toEqual([COMMUNITY_B.id, COMMUNITY_A.id]); + }); }); diff --git a/desktop/tests/e2e/onboarding-agent-defaults.spec.ts b/desktop/tests/e2e/onboarding-agent-defaults.spec.ts index 40ddc64ef..f8e6322ed 100644 --- a/desktop/tests/e2e/onboarding-agent-defaults.spec.ts +++ b/desktop/tests/e2e/onboarding-agent-defaults.spec.ts @@ -431,6 +431,76 @@ test("defaults renders only fields supported by the selected harness", async ({ ).toHaveCount(0); }); +test("defaults hides model when optional harness has empty discovery", async ({ + page, +}) => { + await installMockBridge( + page, + { + acpRuntimesCatalog: [ + runtime("claude", "available", { status: "logged_in" }), + ], + discoverAgentModels: { + models: [], + supportsSwitching: false, + }, + globalAgentConfig: { + env_vars: {}, + provider: null, + model: null, + preferred_runtime: null, + }, + }, + { skipCommunitySeed: true, skipOnboardingSeed: true }, + ); + await page.goto("/"); + await navigateToSetupPage(page); + await page.getByTestId("onboarding-setup-next").click(); + + await expect(page.getByTestId("onboarding-page-config")).toBeVisible(); + await expect(page.getByTestId("global-agent-default-harness")).toHaveText( + "Claude Code", + ); + // Confirmed successful empty catalog — omit the Model control; harness + // default applies and Finish stays available. + await expect(page.getByTestId("global-agent-model")).toHaveCount(0); + await expect(page.getByTestId("onboarding-finish")).toBeEnabled(); +}); + +test("defaults keeps model control when optional harness discovery fails", async ({ + page, +}) => { + await installMockBridge( + page, + { + acpRuntimesCatalog: [ + runtime("claude", "available", { status: "logged_in" }), + ], + discoverAgentModelsError: "CLI discovery timed out", + globalAgentConfig: { + env_vars: {}, + provider: null, + model: null, + preferred_runtime: null, + }, + }, + { skipCommunitySeed: true, skipOnboardingSeed: true }, + ); + await page.goto("/"); + await navigateToSetupPage(page); + await page.getByTestId("onboarding-setup-next").click(); + + await expect(page.getByTestId("onboarding-page-config")).toBeVisible(); + await expect(page.getByTestId("global-agent-default-harness")).toHaveText( + "Claude Code", + ); + // Failed discovery must not look like successful empty: keep the control + // and surface #2246 failure UI (status line bypasses onboarding-essential). + await expect(page.getByTestId("global-agent-model")).toBeVisible(); + await expect(page.getByText(/Could not load live models/i)).toBeVisible(); + await expect(page.getByTestId("onboarding-finish")).toBeEnabled(); +}); + test("defaults Back returns to harness setup", async ({ page }) => { await installMockBridge( page, diff --git a/desktop/tests/helpers/bridge.ts b/desktop/tests/helpers/bridge.ts index 15606e82c..36b80aef2 100644 --- a/desktop/tests/helpers/bridge.ts +++ b/desktop/tests/helpers/bridge.ts @@ -431,6 +431,27 @@ type MockBridgeOptions = { * test can interleave edits and exercise the mid-save race handling. */ globalConfigSaveDelayMs?: number; + /** + * Override the `discover_agent_models` mock response. When set, the bridge + * returns this catalog instead of the default per-harness model list. + * Use `{ models: [], supportsSwitching: false }` to exercise empty discovery + * (e.g. optional harnesses that omit the Model control). + */ + discoverAgentModels?: { + models: Array<{ + id: string; + name: string | null; + description?: string | null; + }>; + supportsSwitching: boolean; + agentDefaultModel?: string | null; + selectedModel?: string | null; + }; + /** + * When set, `discover_agent_models` throws with this message instead of + * returning a catalog. Exercises the discovery-failure UI path. + */ + discoverAgentModelsError?: string; }; type BridgeOptions = { diff --git a/mobile/lib/shared/relay/animated_image_sanitizer.dart b/mobile/lib/shared/relay/animated_image_sanitizer.dart new file mode 100644 index 000000000..6067d65fb --- /dev/null +++ b/mobile/lib/shared/relay/animated_image_sanitizer.dart @@ -0,0 +1,443 @@ +import 'dart:convert'; +import 'dart:typed_data'; + +const _pngSignature = <int>[0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]; +const _allowedPngAncillaryChunks = { + 'cHRM', + 'gAMA', + 'sBIT', + 'sRGB', + 'bKGD', + 'hIST', + 'tRNS', + 'sPLT', + 'acTL', + 'fcTL', + 'fdAT', +}; +const _allowedWebpChunks = {'VP8 ', 'VP8L', 'VP8X', 'ALPH', 'ANIM', 'ANMF'}; +const _webpMetadataFlags = 0x20 | 0x08 | 0x04; + +/// Remove metadata from an animated image without decoding its frames. +/// +/// Decoding through UIKit or Android Bitmap would flatten animations. These +/// structural scrubbers retain only the chunks/extensions accepted by the +/// relay and preserve animation timing, disposal, looping, and frame data. +Uint8List sanitizeAnimatedImageForUpload(Uint8List bytes, String mimeType) { + return switch (mimeType) { + 'image/gif' => _scrubGif(bytes), + 'image/png' => _scrubPng(bytes), + 'image/webp' => _scrubWebp(bytes), + _ => throw FormatException('Unsupported animated image type: $mimeType'), + }; +} + +Uint8List _scrubPng(Uint8List bytes) { + if (!_startsWith(bytes, _pngSignature)) { + throw const FormatException('Invalid PNG signature'); + } + + final output = BytesBuilder(copy: false)..add(_pngSignature); + var offset = _pngSignature.length; + while (offset < bytes.length) { + if (bytes.length - offset < 12) { + throw const FormatException('Truncated PNG chunk'); + } + final payloadLength = _readUint32BigEndian(bytes, offset); + final chunkLength = payloadLength + 12; + if (payloadLength > bytes.length - offset - 12) { + throw const FormatException('Invalid PNG chunk length'); + } + final typeStart = offset + 4; + final type = ascii.decode(bytes.sublist(typeStart, typeStart + 4)); + if (type == 'iCCP') { + throw const FormatException( + 'Animated PNG ICC profile cannot be removed safely', + ); + } + if (type == 'eXIf') { + final orientation = _readExifOrientation( + bytes, + offset + 8, + payloadLength, + ); + if (orientation != null && orientation >= 2 && orientation <= 8) { + throw const FormatException( + 'Animated PNG EXIF orientation cannot be removed safely', + ); + } + } + final isAncillary = bytes[typeStart] & 0x20 != 0; + if (!isAncillary || _allowedPngAncillaryChunks.contains(type)) { + output.add(Uint8List.sublistView(bytes, offset, offset + chunkLength)); + } + + offset += chunkLength; + if (type == 'IEND') { + return output.takeBytes(); + } + } + + throw const FormatException('PNG is missing IEND'); +} + +Uint8List _scrubWebp(Uint8List bytes) { + if (bytes.length < 12 || + !_matchesAscii(bytes, 0, 'RIFF') || + !_matchesAscii(bytes, 8, 'WEBP')) { + throw const FormatException('Invalid WebP signature'); + } + + final declaredLength = _readUint32LittleEndian(bytes, 4); + final inputEnd = declaredLength + 8; + if (inputEnd < 12 || inputEnd > bytes.length) { + throw const FormatException('Invalid WebP container length'); + } + + final chunks = BytesBuilder(copy: false); + var offset = 12; + while (offset < inputEnd) { + if (inputEnd - offset < 8) { + throw const FormatException('Truncated WebP chunk'); + } + final type = ascii.decode(bytes.sublist(offset, offset + 4)); + final payloadLength = _readUint32LittleEndian(bytes, offset + 4); + final payloadStart = offset + 8; + final paddedLength = payloadLength + (payloadLength.isOdd ? 1 : 0); + final chunkEnd = payloadStart + paddedLength; + if (chunkEnd > inputEnd) { + throw const FormatException('Invalid WebP chunk length'); + } + + if (type == 'EXIF') { + final orientation = _readExifOrientation( + bytes, + payloadStart, + payloadLength, + ); + if (orientation != null && orientation >= 2 && orientation <= 8) { + throw const FormatException( + 'Animated WebP EXIF orientation cannot be removed safely', + ); + } + } + if (type == 'ICCP') { + throw const FormatException( + 'Animated WebP ICC profile cannot be removed safely', + ); + } + + if (_allowedWebpChunks.contains(type)) { + if (type == 'VP8X') { + if (payloadLength == 0) { + throw const FormatException('Invalid VP8X chunk'); + } + final payload = BytesBuilder(copy: false) + ..addByte(bytes[payloadStart] & ~_webpMetadataFlags) + ..add( + Uint8List.sublistView( + bytes, + payloadStart + 1, + payloadStart + payloadLength, + ), + ); + _addWebpChunk(chunks, type, payload.takeBytes()); + } else if (type == 'ANMF') { + _addWebpChunk( + chunks, + type, + _scrubAnmfPayload( + Uint8List.sublistView( + bytes, + payloadStart, + payloadStart + payloadLength, + ), + ), + ); + } else { + _addWebpChunk( + chunks, + type, + Uint8List.sublistView( + bytes, + payloadStart, + payloadStart + payloadLength, + ), + ); + } + } + offset = chunkEnd; + } + + final chunkBytes = chunks.takeBytes(); + final output = BytesBuilder(copy: false) + ..add(ascii.encode('RIFF')) + ..add(_uint32LittleEndian(chunkBytes.length + 4)) + ..add(ascii.encode('WEBP')) + ..add(chunkBytes); + return output.takeBytes(); +} + +void _addWebpChunk(BytesBuilder output, String type, Uint8List payload) { + output + ..add(ascii.encode(type)) + ..add(_uint32LittleEndian(payload.length)) + ..add(payload); + if (payload.length.isOdd) output.addByte(0); +} + +Uint8List _scrubAnmfPayload(Uint8List payload) { + const frameHeaderLength = 16; + if (payload.length < frameHeaderLength) { + throw const FormatException('Invalid WebP animation frame'); + } + + final output = BytesBuilder(copy: false) + ..add(Uint8List.sublistView(payload, 0, frameHeaderLength)); + var offset = frameHeaderLength; + var sawAlpha = false; + var sawImage = false; + + while (offset < payload.length) { + if (payload.length - offset < 8) { + throw const FormatException('Truncated WebP animation frame chunk'); + } + final chunkLength = _readUint32LittleEndian(payload, offset + 4); + final chunkStart = offset + 8; + final paddedLength = chunkLength + (chunkLength.isOdd ? 1 : 0); + final chunkEnd = chunkStart + paddedLength; + if (chunkEnd > payload.length) { + throw const FormatException('Invalid WebP animation frame chunk length'); + } + final chunkPayload = Uint8List.sublistView( + payload, + chunkStart, + chunkStart + chunkLength, + ); + + if (_matchesAscii(payload, offset, 'ALPH')) { + if (sawAlpha || sawImage) { + throw const FormatException('Invalid WebP animation frame layout'); + } + _addWebpChunk(output, 'ALPH', chunkPayload); + sawAlpha = true; + } else if (_matchesAscii(payload, offset, 'VP8 ')) { + if (sawImage) { + throw const FormatException('Invalid WebP animation frame layout'); + } + _addWebpChunk(output, 'VP8 ', chunkPayload); + sawImage = true; + } else if (_matchesAscii(payload, offset, 'VP8L')) { + if (sawAlpha || sawImage) { + throw const FormatException('Invalid WebP animation frame layout'); + } + _addWebpChunk(output, 'VP8L', chunkPayload); + sawImage = true; + } + + offset = chunkEnd; + } + + if (!sawImage) { + throw const FormatException('WebP animation frame is missing image data'); + } + return output.takeBytes(); +} + +int? _readExifOrientation( + Uint8List bytes, + int payloadStart, + int payloadLength, +) { + final payloadEnd = payloadStart + payloadLength; + var tiffStart = payloadStart; + if (payloadLength >= 6 && + _matchesAscii(bytes, payloadStart, 'Exif') && + bytes[payloadStart + 4] == 0 && + bytes[payloadStart + 5] == 0) { + tiffStart += 6; + } + if (payloadEnd - tiffStart < 8) return null; + + final endian = switch ((bytes[tiffStart], bytes[tiffStart + 1])) { + (0x49, 0x49) => Endian.little, + (0x4d, 0x4d) => Endian.big, + _ => null, + }; + if (endian == null) return null; + + int? readUint16(int offset) { + if (offset < tiffStart || offset + 2 > payloadEnd) return null; + return ByteData.sublistView(bytes, offset, offset + 2).getUint16(0, endian); + } + + int? readUint32(int offset) { + if (offset < tiffStart || offset + 4 > payloadEnd) return null; + return ByteData.sublistView(bytes, offset, offset + 4).getUint32(0, endian); + } + + if (readUint16(tiffStart + 2) != 42) return null; + final ifdOffset = readUint32(tiffStart + 4); + if (ifdOffset == null) return null; + final ifdStart = tiffStart + ifdOffset; + final entryCount = readUint16(ifdStart); + if (entryCount == null) return null; + + final entriesStart = ifdStart + 2; + for (var index = 0; index < entryCount; index += 1) { + final entryStart = entriesStart + index * 12; + if (readUint16(entryStart) == 0x0112 && + readUint16(entryStart + 2) == 3 && + readUint32(entryStart + 4) == 1) { + return readUint16(entryStart + 8); + } + } + return null; +} + +Uint8List _scrubGif(Uint8List bytes) { + if (bytes.length < 13 || + (!_matchesAscii(bytes, 0, 'GIF87a') && + !_matchesAscii(bytes, 0, 'GIF89a'))) { + throw const FormatException('Invalid GIF signature'); + } + + var offset = 13; + final packed = bytes[10]; + if (packed & 0x80 != 0) { + final tableLength = 3 << ((packed & 0x07) + 1); + offset += tableLength; + if (offset > bytes.length) { + throw const FormatException('Truncated GIF color table'); + } + } + + final segments = <Uint8List?>[Uint8List.sublistView(bytes, 0, offset)]; + final pendingGraphicControls = <int>[]; + + while (offset < bytes.length) { + switch (bytes[offset]) { + case 0x2c: + final start = offset; + if (bytes.length - offset < 10) { + throw const FormatException('Truncated GIF image descriptor'); + } + final imagePacked = bytes[offset + 9]; + offset += 10; + if (imagePacked & 0x80 != 0) { + offset += 3 << ((imagePacked & 0x07) + 1); + if (offset > bytes.length) { + throw const FormatException('Truncated GIF local color table'); + } + } + if (offset >= bytes.length) { + throw const FormatException('Missing GIF LZW code size'); + } + offset = _gifSubBlocksEnd(bytes, offset + 1); + segments.add(Uint8List.sublistView(bytes, start, offset)); + pendingGraphicControls.clear(); + case 0x21: + final start = offset; + if (bytes.length - offset < 2) { + throw const FormatException('Truncated GIF extension'); + } + final label = bytes[offset + 1]; + offset += 2; + switch (label) { + case 0xf9: + if (bytes.length - offset < 6 || + bytes[offset] != 4 || + bytes[offset + 5] != 0) { + throw const FormatException('Invalid GIF graphic control'); + } + offset += 6; + segments.add(Uint8List.sublistView(bytes, start, offset)); + pendingGraphicControls.add(segments.length - 1); + case 0xff: + if (bytes.length - offset < 12 || bytes[offset] != 11) { + throw const FormatException('Invalid GIF application extension'); + } + final isLoopExtension = + _matchesAscii(bytes, offset + 1, 'NETSCAPE2.0') || + _matchesAscii(bytes, offset + 1, 'ANIMEXTS1.0'); + final dataStart = offset + 12; + offset = _gifSubBlocksEnd(bytes, dataStart); + if (isLoopExtension) { + if (bytes.length - dataStart < 5 || + bytes[dataStart] != 3 || + bytes[dataStart + 1] != 1) { + throw const FormatException('Invalid GIF loop extension'); + } + segments + ..add(Uint8List.sublistView(bytes, start, dataStart + 4)) + ..add(Uint8List(1)); + } + case 0x01: + offset = _gifSubBlocksEnd(bytes, offset); + for (final segmentIndex in pendingGraphicControls) { + segments[segmentIndex] = null; + } + pendingGraphicControls.clear(); + default: + offset = _gifSubBlocksEnd(bytes, offset); + } + case 0x3b: + segments.add(Uint8List.sublistView(bytes, offset, offset + 1)); + final output = BytesBuilder(copy: false); + for (final segment in segments) { + if (segment != null) output.add(segment); + } + return output.takeBytes(); + default: + throw const FormatException('Invalid GIF block'); + } + } + + throw const FormatException('GIF is missing trailer'); +} + +int _gifSubBlocksEnd(Uint8List bytes, int offset) { + while (offset < bytes.length) { + final blockLength = bytes[offset]; + offset += 1; + if (blockLength == 0) return offset; + offset += blockLength; + if (offset > bytes.length) { + throw const FormatException('Truncated GIF data block'); + } + } + throw const FormatException('GIF data blocks are missing a terminator'); +} + +bool _startsWith(Uint8List bytes, List<int> prefix) { + if (bytes.length < prefix.length) return false; + for (var index = 0; index < prefix.length; index += 1) { + if (bytes[index] != prefix[index]) return false; + } + return true; +} + +bool _matchesAscii(Uint8List bytes, int offset, String value) { + final expected = ascii.encode(value); + if (bytes.length - offset < expected.length) return false; + for (var index = 0; index < expected.length; index += 1) { + if (bytes[offset + index] != expected[index]) return false; + } + return true; +} + +int _readUint32BigEndian(Uint8List bytes, int offset) { + return ByteData.sublistView(bytes, offset, offset + 4).getUint32(0); +} + +int _readUint32LittleEndian(Uint8List bytes, int offset) { + return ByteData.sublistView( + bytes, + offset, + offset + 4, + ).getUint32(0, Endian.little); +} + +Uint8List _uint32LittleEndian(int value) { + return Uint8List(4)..buffer.asByteData().setUint32(0, value, Endian.little); +} diff --git a/mobile/lib/shared/relay/media_upload.dart b/mobile/lib/shared/relay/media_upload.dart index be84f548c..b868fc284 100644 --- a/mobile/lib/shared/relay/media_upload.dart +++ b/mobile/lib/shared/relay/media_upload.dart @@ -9,6 +9,7 @@ import 'package:image_picker/image_picker.dart'; import 'package:nostr/nostr.dart' as nostr; import 'package:pointycastle/digests/sha256.dart'; +import 'animated_image_sanitizer.dart'; import 'media_auth.dart'; import 'mp4_fast_start.dart'; import 'relay_provider.dart'; @@ -37,16 +38,14 @@ final _mediaUploadPlatformChannel = MethodChannel( _mediaUploadPlatformChannelName, ); -const _allowedImageMimeTypes = {'image/jpeg', 'image/png', 'image/webp'}; +const _allowedImageMimeTypes = { + 'image/jpeg', + 'image/png', + 'image/gif', + 'image/webp', +}; const _allowedVideoMimeTypes = {'video/mp4'}; const _maxVideoSizeBytes = 100 * 1024 * 1024; // 100MB -const _unsupportedAnimatedImageMimeTypes = {'image/gif'}; -const _unsupportedGifUploadMessage = - 'GIF uploads are not supported on mobile yet'; -const _unsupportedAnimatedPngUploadMessage = - 'Animated PNG uploads are not supported on mobile yet'; -const _unsupportedAnimatedWebpUploadMessage = - 'Animated WebP uploads are not supported on mobile yet'; const _mediaPolicyUploadMessage = "We couldn't prepare this image for upload."; typedef PickGalleryImage = Future<XFile?> Function(); @@ -179,7 +178,10 @@ class MediaUploadService { Future<BlobDescriptor> uploadImage(XFile image) async { final preparedImage = await _prepareUploadImage(image); - return uploadBytes(preparedImage.bytes, mimeType: preparedImage.mimeType); + return _uploadPreparedBytes( + preparedImage.bytes, + mimeType: preparedImage.mimeType, + ); } Future<bool> clipboardHasImage() async { @@ -236,7 +238,22 @@ class MediaUploadService { Uint8List bytes, { required String mimeType, }) async { - _validateUpload(bytes, mimeType); + if (mimeType == 'image/gif' || + (mimeType == 'image/png' && _isAnimatedPng(bytes)) || + (mimeType == 'image/webp' && _isAnimatedWebp(bytes))) { + try { + bytes = sanitizeAnimatedImageForUpload(bytes, mimeType); + } on FormatException { + throw Exception('failed to sanitize image for upload'); + } + } + return _uploadPreparedBytes(bytes, mimeType: mimeType); + } + + Future<BlobDescriptor> _uploadPreparedBytes( + Uint8List bytes, { + required String mimeType, + }) async { if (!_allowedImageMimeTypes.contains(mimeType) && !_allowedVideoMimeTypes.contains(mimeType)) { throw Exception('unsupported file type: $mimeType'); @@ -360,7 +377,6 @@ class MediaUploadService { Uint8List bytes, String mimeType, ) async { - _validateUpload(bytes, mimeType); final preparedBytes = await _sanitizeImageBytesIfNeeded(bytes, mimeType); return _buildPreparedUploadImage(preparedBytes); } @@ -383,6 +399,16 @@ class MediaUploadService { Uint8List bytes, String mimeType, ) async { + if (mimeType == 'image/gif' || + (mimeType == 'image/png' && _isAnimatedPng(bytes)) || + (mimeType == 'image/webp' && _isAnimatedWebp(bytes))) { + try { + return sanitizeAnimatedImageForUpload(bytes, mimeType); + } on FormatException { + throw Exception('failed to sanitize image for upload'); + } + } + if (!_shouldSanitizePickedImage(mimeType)) { return bytes; } @@ -408,18 +434,6 @@ String? _tryDetectImageMimeType(Uint8List bytes) { } } -void _validateUpload(Uint8List bytes, String mimeType) { - if (_unsupportedAnimatedImageMimeTypes.contains(mimeType)) { - throw Exception(_unsupportedGifUploadMessage); - } - if (mimeType == 'image/png' && _isAnimatedPng(bytes)) { - throw Exception(_unsupportedAnimatedPngUploadMessage); - } - if (mimeType == 'image/webp' && _isAnimatedWebp(bytes)) { - throw Exception(_unsupportedAnimatedWebpUploadMessage); - } -} - String _detectImageMimeType(Uint8List bytes) { if (_startsWith(bytes, const [0xff, 0xd8, 0xff])) { return 'image/jpeg'; diff --git a/mobile/test/features/channels/compose_bar_test.dart b/mobile/test/features/channels/compose_bar_test.dart index b8efada02..dd629dcf8 100644 --- a/mobile/test/features/channels/compose_bar_test.dart +++ b/mobile/test/features/channels/compose_bar_test.dart @@ -41,16 +41,58 @@ final _pngBytes = Uint8List.fromList([ ]); final _gifBytes = Uint8List.fromList([ - 0x47, - 0x49, - 0x46, - 0x38, - 0x39, - 0x61, + ...ascii.encode('GIF89a'), + 0x02, + 0x00, + 0x02, + 0x00, + 0x80, + 0x00, + 0x00, + 0x00, + 0x00, + 0x00, + 0xff, + 0xff, + 0xff, + 0x21, + 0xfe, + 0x05, + ...ascii.encode('hello'), + 0x00, + 0x21, + 0xff, + 0x0b, + ...ascii.encode('NETSCAPE2.0'), + 0x03, 0x01, 0x00, + 0x00, + 0x00, + 0x21, + 0xf9, + 0x04, + 0x00, + 0x0a, + 0x00, + 0x00, + 0x00, + 0x2c, + 0x00, + 0x00, + 0x00, + 0x00, + 0x02, + 0x00, + 0x02, + 0x00, + 0x00, + 0x02, + 0x02, + 0x44, 0x01, 0x00, + 0x3b, ]); final _apngBytes = Uint8List.fromList([ @@ -62,45 +104,25 @@ final _apngBytes = Uint8List.fromList([ 0x0a, 0x1a, 0x0a, - 0x00, - 0x00, - 0x00, - 0x08, - 0x61, - 0x63, - 0x54, - 0x4c, - 0x00, - 0x00, - 0x00, - 0x02, - 0x00, - 0x00, - 0x00, - 0x00, - 0x00, - 0x00, - 0x00, - 0x00, - 0x00, - 0x00, - 0x00, - 0x00, - 0x00, - 0x00, - 0x00, - 0x00, - 0x00, - 0x49, - 0x45, - 0x4e, - 0x44, - 0x00, - 0x00, - 0x00, - 0x00, + ..._testPngChunk('acTL', [0, 0, 0, 2, 0, 0, 0, 0]), + ..._testPngChunk('IEND', const []), ]); +List<int> _testPngChunk(String type, List<int> payload) { + return [ + payload.length >> 24 & 0xff, + payload.length >> 16 & 0xff, + payload.length >> 8 & 0xff, + payload.length & 0xff, + ...ascii.encode(type), + ...payload, + 0, + 0, + 0, + 0, + ]; +} + const _mediaUploadPlatformChannel = MethodChannel('buzz/media_upload'); void _setMockMediaUploadPlatformHandler( @@ -858,12 +880,25 @@ void main() { }); } - testWidgets('shows a clean error when a GIF is picked', (tester) async { + testWidgets('adds a sanitized GIF attachment', (tester) async { final keychain = nostr.Keys.generate(); final nsec = keychain.nsec; final uploadService = MediaUploadService( baseUrl: 'https://relay.example', nsec: nsec, + httpClient: http_testing.MockClient((request) async { + return http.Response( + jsonEncode({ + 'url': 'https://relay.example/media/animated.gif', + 'sha256': + '4444444444444444444444444444444444444444444444444444444444444444', + 'size': request.bodyBytes.length, + 'type': 'image/gif', + 'uploaded': 1, + }), + 200, + ); + }), pickGalleryVideo: () async => null, pickGalleryImage: () async => XFile.fromData(_gifBytes, name: 'animated.gif'), @@ -887,7 +922,11 @@ void main() { await tester.pumpAndSettle(); expect( - find.textContaining('GIF uploads are not supported on mobile yet'), + find.byKey( + const ValueKey( + 'compose-attachment:https://relay.example/media/animated.gif', + ), + ), findsOneWidget, ); }); @@ -1067,14 +1106,25 @@ void main() { expect(publishedEvents.where((event) => event['kind'] == 9000), isEmpty); }); - testWidgets('shows a clean error when an animated PNG is picked', ( - tester, - ) async { + testWidgets('adds a sanitized animated PNG attachment', (tester) async { final keychain = nostr.Keys.generate(); final nsec = keychain.nsec; final uploadService = MediaUploadService( baseUrl: 'https://relay.example', nsec: nsec, + httpClient: http_testing.MockClient((request) async { + return http.Response( + jsonEncode({ + 'url': 'https://relay.example/media/animated.png', + 'sha256': + '5555555555555555555555555555555555555555555555555555555555555555', + 'size': request.bodyBytes.length, + 'type': 'image/png', + 'uploaded': 1, + }), + 200, + ); + }), pickGalleryVideo: () async => null, pickGalleryImage: () async => XFile.fromData(_apngBytes, name: 'animated.png'), @@ -1098,7 +1148,11 @@ void main() { await tester.pumpAndSettle(); expect( - find.textContaining('Animated PNG uploads are not supported on mobile'), + find.byKey( + const ValueKey( + 'compose-attachment:https://relay.example/media/animated.png', + ), + ), findsOneWidget, ); }); diff --git a/mobile/test/shared/relay/animated_image_sanitizer_test.dart b/mobile/test/shared/relay/animated_image_sanitizer_test.dart new file mode 100644 index 000000000..515a81b48 --- /dev/null +++ b/mobile/test/shared/relay/animated_image_sanitizer_test.dart @@ -0,0 +1,469 @@ +import 'dart:convert'; +import 'dart:typed_data'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:buzz/shared/relay/animated_image_sanitizer.dart'; + +void main() { + test('strips APNG metadata without changing animation chunks', () { + final clean = _animatedPng(metadata: false); + final dirty = _animatedPng(metadata: true) + ..addAll(utf8.encode('trailing metadata')); + + expect( + sanitizeAnimatedImageForUpload(Uint8List.fromList(dirty), 'image/png'), + clean, + ); + }); + + test('strips identity APNG orientation but rejects display transforms', () { + final clean = _animatedPng(metadata: false); + expect( + sanitizeAnimatedImageForUpload( + Uint8List.fromList(_animatedPng(metadata: false, orientation: 1)), + 'image/png', + ), + clean, + ); + + for (final endian in [Endian.little, Endian.big]) { + expect( + () => sanitizeAnimatedImageForUpload( + Uint8List.fromList( + _animatedPng( + metadata: false, + orientation: 6, + orientationEndian: endian, + ), + ), + 'image/png', + ), + throwsA( + isA<FormatException>().having( + (error) => error.message, + 'message', + contains('orientation'), + ), + ), + ); + } + }); + + test('rejects APNG ICC profiles that affect color rendering', () { + expect( + () => sanitizeAnimatedImageForUpload( + Uint8List.fromList(_animatedPng(metadata: false, iccProfile: true)), + 'image/png', + ), + throwsA( + isA<FormatException>().having( + (error) => error.message, + 'message', + contains('ICC profile'), + ), + ), + ); + }); + + test('strips animated WebP metadata and clears metadata flags', () { + final clean = _animatedWebp(metadata: false); + final dirty = _animatedWebp(metadata: true) + ..addAll(utf8.encode('trailing metadata')); + + expect( + sanitizeAnimatedImageForUpload(Uint8List.fromList(dirty), 'image/webp'), + clean, + ); + }); + + test('strips identity WebP orientation but rejects display transforms', () { + final clean = _animatedWebp(metadata: false); + expect( + sanitizeAnimatedImageForUpload( + Uint8List.fromList(_animatedWebp(metadata: false, orientation: 1)), + 'image/webp', + ), + clean, + ); + + for (final endian in [Endian.little, Endian.big]) { + expect( + () => sanitizeAnimatedImageForUpload( + Uint8List.fromList( + _animatedWebp( + metadata: false, + orientation: 6, + orientationEndian: endian, + ), + ), + 'image/webp', + ), + throwsA( + isA<FormatException>().having( + (error) => error.message, + 'message', + contains('orientation'), + ), + ), + ); + } + }); + + test('rejects animated WebP ICC profiles that affect color rendering', () { + expect( + () => sanitizeAnimatedImageForUpload( + Uint8List.fromList(_animatedWebp(metadata: false, iccProfile: true)), + 'image/webp', + ), + throwsA( + isA<FormatException>().having( + (error) => error.message, + 'message', + contains('ICC profile'), + ), + ), + ); + }); + + test('removes metadata chunks nested inside animated WebP frames', () { + expect( + sanitizeAnimatedImageForUpload( + Uint8List.fromList( + _animatedWebp(metadata: false, nestedMetadata: true), + ), + 'image/webp', + ), + _animatedWebp(metadata: false), + ); + }); + + test('strips GIF metadata without changing animation blocks', () { + final clean = _minimalGif(); + final dirty = <int>[ + ...clean.sublist(0, 19), + ..._gifCommentExtension(), + ..._gifApplicationExtension('XMP DataXMP', utf8.encode('<x/>')), + ...clean.sublist(19), + ...utf8.encode('trailing metadata'), + ]; + + expect( + sanitizeAnimatedImageForUpload(Uint8List.fromList(dirty), 'image/gif'), + clean, + ); + }); + + test('canonicalizes GIF loop extensions with hidden sub-blocks', () { + final clean = _minimalGif(); + final cleanLoop = _gifLoopExtension(); + final dirty = <int>[ + ...clean.sublist(0, 19), + ..._gifLoopExtension(extra: utf8.encode('location')), + ...clean.sublist(19 + cleanLoop.length), + ]; + + expect( + sanitizeAnimatedImageForUpload(Uint8List.fromList(dirty), 'image/gif'), + clean, + ); + }); + + test('drops GIF applications with binary authentication codes', () { + final clean = _minimalGif(); + final dirty = <int>[ + ...clean.sublist(0, 19), + ..._gifApplicationExtensionBytes([ + ...ascii.encode('FOREIGN1'), + 0xff, + 0x80, + 0x00, + ], utf8.encode('private')), + ...clean.sublist(19), + ]; + + expect( + sanitizeAnimatedImageForUpload(Uint8List.fromList(dirty), 'image/gif'), + clean, + ); + }); + + test('removes a GIF graphic control consumed by stripped plain text', () { + final clean = _minimalGif(); + final dirty = <int>[ + ...clean.sublist(0, 19), + 0x21, + 0xf9, + 4, + 0x09, + 0x1e, + 0, + 1, + 0, + ..._gifPlainTextExtension(), + ...clean.sublist(19), + ]; + + expect( + sanitizeAnimatedImageForUpload(Uint8List.fromList(dirty), 'image/gif'), + clean, + ); + }); + + test('keeps clean animated containers byte-identical', () { + for (final (mimeType, bytes) in [ + ('image/png', _animatedPng(metadata: false)), + ('image/webp', _animatedWebp(metadata: false)), + ('image/gif', _minimalGif()), + ]) { + expect( + sanitizeAnimatedImageForUpload(Uint8List.fromList(bytes), mimeType), + bytes, + ); + } + }); + + test('fails closed for malformed animated containers', () { + for (final (mimeType, bytes) in [ + ('image/png', <int>[0x89, 0x50, 0x4e, 0x47]), + ('image/webp', ascii.encode('RIFFxxxxWEBP')), + ('image/gif', ascii.encode('GIF89a')), + ]) { + expect( + () => + sanitizeAnimatedImageForUpload(Uint8List.fromList(bytes), mimeType), + throwsFormatException, + ); + } + }); +} + +List<int> _pngChunk(String type, List<int> payload) { + return [ + ..._uint32BigEndian(payload.length), + ...ascii.encode(type), + ...payload, + 0, + 0, + 0, + 0, + ]; +} + +List<int> _animatedPng({ + required bool metadata, + int? orientation, + Endian orientationEndian = Endian.little, + bool iccProfile = false, +}) { + return [ + 0x89, + 0x50, + 0x4e, + 0x47, + 0x0d, + 0x0a, + 0x1a, + 0x0a, + ..._pngChunk('IHDR', List.filled(13, 0)), + ..._pngChunk('acTL', [0, 0, 0, 2, 0, 0, 0, 0]), + if (iccProfile) ..._pngChunk('iCCP', utf8.encode('profile')), + if (orientation != null) + ..._pngChunk( + 'eXIf', + _exifOrientation( + orientation, + orientationEndian, + includePreamble: false, + ), + ), + if (metadata) ..._pngChunk('tEXt', utf8.encode('Location\u0000secret')), + if (metadata) ..._pngChunk('pHYs', List.filled(9, 0)), + ..._pngChunk('fcTL', List.filled(26, 0)), + ..._pngChunk('IDAT', [1, 2, 3]), + ..._pngChunk('fdAT', [0, 0, 0, 1, 4, 5]), + ..._pngChunk('IEND', const []), + ]; +} + +List<int> _webpChunk(String type, List<int> payload) { + return [ + ...ascii.encode(type), + ..._uint32LittleEndian(payload.length), + ...payload, + if (payload.length.isOdd) 0, + ]; +} + +List<int> _animatedWebp({ + required bool metadata, + int? orientation, + Endian orientationEndian = Endian.little, + bool iccProfile = false, + bool nestedMetadata = false, +}) { + final hasExif = metadata || orientation != null; + final frame = <int>[ + ...List.filled(16, 0), + ..._webpChunk('VP8 ', [1, 2, 3]), + if (nestedMetadata) ..._webpChunk('JUNK', utf8.encode('location')), + ]; + final chunks = <int>[ + ..._webpChunk('VP8X', [ + 0x02 | (hasExif ? 0x0c : 0) | (iccProfile ? 0x20 : 0), + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + ]), + ..._webpChunk('ANIM', List.filled(6, 0)), + if (iccProfile) ..._webpChunk('ICCP', utf8.encode('profile')), + if (orientation != null) + ..._webpChunk( + 'EXIF', + _exifOrientation(orientation, orientationEndian, includePreamble: true), + ) + else if (metadata) + ..._webpChunk('EXIF', utf8.encode('location')), + if (metadata) ..._webpChunk('XMP ', utf8.encode('<xmp/>')), + if (metadata) ..._webpChunk('JUNK', utf8.encode('private')), + ..._webpChunk('ANMF', frame), + ]; + return [ + ...ascii.encode('RIFF'), + ..._uint32LittleEndian(chunks.length + 4), + ...ascii.encode('WEBP'), + ...chunks, + ]; +} + +List<int> _exifOrientation( + int orientation, + Endian endian, { + required bool includePreamble, +}) { + final bytes = BytesBuilder(); + if (includePreamble) { + bytes + ..add(ascii.encode('Exif')) + ..add(const [0, 0]); + } + final tiff = ByteData(26); + if (endian == Endian.little) { + tiff.setUint8(0, 0x49); + tiff.setUint8(1, 0x49); + } else { + tiff.setUint8(0, 0x4d); + tiff.setUint8(1, 0x4d); + } + tiff.setUint16(2, 42, endian); + tiff.setUint32(4, 8, endian); + tiff.setUint16(8, 1, endian); + tiff.setUint16(10, 0x0112, endian); + tiff.setUint16(12, 3, endian); + tiff.setUint32(14, 1, endian); + tiff.setUint16(18, orientation, endian); + tiff.setUint32(22, 0, endian); + bytes.add(tiff.buffer.asUint8List()); + return bytes.takeBytes(); +} + +List<int> _minimalGif() { + return [ + ...ascii.encode('GIF89a'), + 2, + 0, + 2, + 0, + 0x80, + 0, + 0, + 0, + 0, + 0, + 0xff, + 0xff, + 0xff, + ..._gifLoopExtension(), + 0x21, + 0xf9, + 4, + 0, + 10, + 0, + 0, + 0, + 0x2c, + 0, + 0, + 0, + 0, + 2, + 0, + 2, + 0, + 0, + 2, + 2, + 0x44, + 1, + 0, + 0x3b, + ]; +} + +List<int> _gifLoopExtension({List<int> extra = const []}) { + return [ + 0x21, + 0xff, + 11, + ...ascii.encode('NETSCAPE2.0'), + 3, + 1, + 0, + 0, + if (extra.isNotEmpty) ...[extra.length, ...extra], + 0, + ]; +} + +List<int> _gifCommentExtension() { + return [0x21, 0xfe, 5, ...ascii.encode('hello'), 0]; +} + +List<int> _gifApplicationExtension(String identifier, List<int> payload) { + return _gifApplicationExtensionBytes(ascii.encode(identifier), payload); +} + +List<int> _gifApplicationExtensionBytes( + List<int> identifier, + List<int> payload, +) { + return [0x21, 0xff, 11, ...identifier, payload.length, ...payload, 0]; +} + +List<int> _gifPlainTextExtension() { + return [0x21, 0x01, 12, 0, 0, 0, 0, 2, 0, 2, 0, 1, 1, 1, 0, 1, 0x78, 0]; +} + +List<int> _uint32BigEndian(int value) { + return [ + value >> 24 & 0xff, + value >> 16 & 0xff, + value >> 8 & 0xff, + value & 0xff, + ]; +} + +List<int> _uint32LittleEndian(int value) { + return [ + value & 0xff, + value >> 8 & 0xff, + value >> 16 & 0xff, + value >> 24 & 0xff, + ]; +} diff --git a/mobile/test/shared/relay/media_upload_test.dart b/mobile/test/shared/relay/media_upload_test.dart index 94b23c9f7..dc901eba3 100644 --- a/mobile/test/shared/relay/media_upload_test.dart +++ b/mobile/test/shared/relay/media_upload_test.dart @@ -69,16 +69,58 @@ final _heicBytes = Uint8List.fromList([ ]); final _gifBytes = Uint8List.fromList([ - 0x47, - 0x49, - 0x46, - 0x38, - 0x39, - 0x61, + ...ascii.encode('GIF89a'), + 0x02, + 0x00, + 0x02, + 0x00, + 0x80, + 0x00, + 0x00, + 0x00, + 0x00, + 0x00, + 0xff, + 0xff, + 0xff, + 0x21, + 0xfe, + 0x05, + ...ascii.encode('hello'), + 0x00, + 0x21, + 0xff, + 0x0b, + ...ascii.encode('NETSCAPE2.0'), + 0x03, 0x01, 0x00, + 0x00, + 0x00, + 0x21, + 0xf9, + 0x04, + 0x00, + 0x0a, + 0x00, + 0x00, + 0x00, + 0x2c, + 0x00, + 0x00, + 0x00, + 0x00, + 0x02, + 0x00, + 0x02, + 0x00, + 0x00, + 0x02, + 0x02, + 0x44, 0x01, 0x00, + 0x3b, ]); final _apngBytes = Uint8List.fromList([ @@ -90,45 +132,25 @@ final _apngBytes = Uint8List.fromList([ 0x0a, 0x1a, 0x0a, - 0x00, - 0x00, - 0x00, - 0x08, - 0x61, - 0x63, - 0x54, - 0x4c, - 0x00, - 0x00, - 0x00, - 0x02, - 0x00, - 0x00, - 0x00, - 0x00, - 0x00, - 0x00, - 0x00, - 0x00, - 0x00, - 0x00, - 0x00, - 0x00, - 0x00, - 0x00, - 0x00, - 0x00, - 0x00, - 0x49, - 0x45, - 0x4e, - 0x44, - 0x00, - 0x00, - 0x00, - 0x00, + ..._testPngChunk('acTL', [0, 0, 0, 2, 0, 0, 0, 0]), + ..._testPngChunk('IEND', const []), ]); +List<int> _testPngChunk(String type, List<int> payload) { + return [ + payload.length >> 24 & 0xff, + payload.length >> 16 & 0xff, + payload.length >> 8 & 0xff, + payload.length & 0xff, + ...ascii.encode(type), + ...payload, + 0, + 0, + 0, + 0, + ]; +} + final _staticPngWithActlPayloadBytes = Uint8List.fromList([ 0x89, 0x50, @@ -588,29 +610,40 @@ void main() { expect(descriptor.type, 'image/png'); }); - test( - 'rejects GIF clipboard bytes through the shared validation path', - () async { - final service = MediaUploadService( - baseUrl: 'https://relay.example', - nsec: null, - pickGalleryVideo: () async => null, - pickGalleryImage: () async => null, - readClipboardImage: () async => _gifBytes, - ); + test('sanitizes and uploads GIF clipboard bytes', () async { + http.Request? capturedRequest; + final service = MediaUploadService( + baseUrl: 'https://relay.example', + nsec: nostr.Keys.generate().nsec, + httpClient: http_testing.MockClient((request) async { + capturedRequest = request; + return http.Response( + jsonEncode({ + 'url': 'https://relay.example/media/clipboard.gif', + 'sha256': + '3333333333333333333333333333333333333333333333333333333333333333', + 'size': request.bodyBytes.length, + 'type': 'image/gif', + 'uploaded': 1, + }), + 200, + ); + }), + pickGalleryVideo: () async => null, + pickGalleryImage: () async => null, + readClipboardImage: () async => _gifBytes, + ); - expect( - service.readAndUploadClipboardImage, - throwsA( - isA<Exception>().having( - (error) => error.toString(), - 'message', - contains('GIF uploads are not supported on mobile yet'), - ), - ), - ); - }, - ); + final descriptor = await service.readAndUploadClipboardImage(); + + expect(descriptor.type, 'image/gif'); + expect(capturedRequest, isNotNull); + expect(capturedRequest!.headers['Content-Type'], 'image/gif'); + expect( + ascii.decode(capturedRequest!.bodyBytes, allowInvalid: true), + isNot(contains('hello')), + ); + }); test('rejects empty clipboard image bytes', () async { final service = MediaUploadService( @@ -945,55 +978,76 @@ void main() { expect(capturedRequest!.bodyBytes, _pngBytes); }); - test('rejects GIF gallery files before upload', () async { + test('sanitizes and uploads GIF gallery files', () async { final keychain = nostr.Keys.generate(); final nsec = keychain.nsec; + http.Request? capturedRequest; final service = MediaUploadService( baseUrl: 'https://relay.example', nsec: nsec, + httpClient: http_testing.MockClient((request) async { + capturedRequest = request; + return http.Response( + jsonEncode({ + 'url': 'https://relay.example/media/animated.gif', + 'sha256': + '4444444444444444444444444444444444444444444444444444444444444444', + 'size': request.bodyBytes.length, + 'type': 'image/gif', + 'uploaded': 1, + }), + 200, + ); + }), pickGalleryVideo: () async => null, pickGalleryImage: () async => XFile.fromData(_gifBytes, name: 'animated.gif'), ); + final descriptor = await service.pickAndUploadImage(); + + expect(descriptor, isNotNull); + expect(descriptor!.type, 'image/gif'); + expect(capturedRequest!.headers['Content-Type'], 'image/gif'); expect( - service.pickAndUploadImage(), - throwsA( - isA<Exception>().having( - (error) => error.toString(), - 'message', - contains('GIF uploads are not supported on mobile yet'), - ), - ), + ascii.decode(capturedRequest!.bodyBytes, allowInvalid: true), + isNot(contains('hello')), ); }); - test('rejects animated PNG gallery files before upload', () async { + test('sanitizes and uploads animated PNG gallery files', () async { final keychain = nostr.Keys.generate(); final nsec = keychain.nsec; + http.Request? capturedRequest; final service = MediaUploadService( baseUrl: 'https://relay.example', nsec: nsec, - httpClient: http_testing.MockClient( - (request) async => http.Response('{}', 200), - ), + httpClient: http_testing.MockClient((request) async { + capturedRequest = request; + return http.Response( + jsonEncode({ + 'url': 'https://relay.example/media/animated.png', + 'sha256': + '5555555555555555555555555555555555555555555555555555555555555555', + 'size': request.bodyBytes.length, + 'type': 'image/png', + 'uploaded': 1, + }), + 200, + ); + }), pickGalleryVideo: () async => null, pickGalleryImage: () async => XFile.fromData(_apngBytes, name: 'animated.png'), ); - expect( - service.pickAndUploadImage(), - throwsA( - isA<Exception>().having( - (error) => error.toString(), - 'message', - contains('Animated PNG uploads are not supported on mobile yet'), - ), - ), - ); + final descriptor = await service.pickAndUploadImage(); + + expect(descriptor, isNotNull); + expect(descriptor!.type, 'image/png'); + expect(capturedRequest!.bodyBytes, _apngBytes); }); test('uploads static PNG when acTL appears only in chunk payload', () async { @@ -1034,31 +1088,38 @@ void main() { expect(capturedRequest!.bodyBytes, _staticPngWithActlPayloadBytes); }); - test('rejects animated WebP gallery files before upload', () async { + test('sanitizes and uploads animated WebP gallery files', () async { final keychain = nostr.Keys.generate(); final nsec = keychain.nsec; + http.Request? capturedRequest; final service = MediaUploadService( baseUrl: 'https://relay.example', nsec: nsec, - httpClient: http_testing.MockClient( - (request) async => http.Response('{}', 200), - ), + httpClient: http_testing.MockClient((request) async { + capturedRequest = request; + return http.Response( + jsonEncode({ + 'url': 'https://relay.example/media/animated.webp', + 'sha256': + '6666666666666666666666666666666666666666666666666666666666666666', + 'size': request.bodyBytes.length, + 'type': 'image/webp', + 'uploaded': 1, + }), + 200, + ); + }), pickGalleryVideo: () async => null, pickGalleryImage: () async => XFile.fromData(_animatedWebpBytes, name: 'animated.webp'), ); - expect( - service.pickAndUploadImage(), - throwsA( - isA<Exception>().having( - (error) => error.toString(), - 'message', - contains('Animated WebP uploads are not supported on mobile yet'), - ), - ), - ); + final descriptor = await service.pickAndUploadImage(); + + expect(descriptor, isNotNull); + expect(descriptor!.type, 'image/webp'); + expect(capturedRequest!.bodyBytes, _animatedWebpBytes); }); test('rejects unsupported gallery files before upload', () async {