diff --git a/crates/mind-body/src/attachment.rs b/crates/mind-body/src/attachment.rs index 68b0a8b..8e1103d 100644 --- a/crates/mind-body/src/attachment.rs +++ b/crates/mind-body/src/attachment.rs @@ -8,6 +8,8 @@ //! operator's, not hers); every open below it is relative to a held descriptor and refuses a symlink, so a shell that swaps //! `attachments/` for a link between two calls finds the next call refusing it. The bytes go to a temporary name in that directory //! and are linked to the final name, which fails where a rename would replace her file; a crash leaves only the temporary. +//! +//! The file_received receipt means only that the mouth fetched and stored the file for her. It does not prove the file reached her sandbox. The receipt stays inside the mouth's fetch, so a failed save (Exists, WriteFailed, RepoTree, a Load skip) leaves the receipt standing; she is told the save failed in her own turn. use std::path::{Path, PathBuf}; @@ -82,6 +84,8 @@ fn write(sandbox: &Path, name: &str, bytes: &[u8]) -> Result<(), SaveFault> { Ok(()) | Err(Errno::NOENT) => {} Err(_) => return Err(SaveFault::WriteFailed), } + #[cfg(test)] + tests::between_unlink_and_create(); let file = fs::openat(&dir, temp.as_str(), OFlags::WRONLY | OFlags::CREATE | OFlags::EXCL | OFlags::NOFOLLOW | OFlags::CLOEXEC, Mode::RUSR | Mode::WUSR) .map_err(|_| SaveFault::WriteFailed)?; let landed = (|| { diff --git a/crates/mind-body/src/attachment/tests.rs b/crates/mind-body/src/attachment/tests.rs index c32dac6..7d69f10 100644 --- a/crates/mind-body/src/attachment/tests.rs +++ b/crates/mind-body/src/attachment/tests.rs @@ -26,6 +26,19 @@ pub(super) fn crash_after(written: usize) -> bool { fire } +thread_local! { + static GAP: std::cell::RefCell>> = const { std::cell::RefCell::new(None) }; +} + +/// The seam between the unlink of the temporary's name and its create: where her shell could plant something at that name. +pub(super) fn between_unlink_and_create() { + GAP.with(|gap| { + if let Some(plant) = gap.borrow().as_ref() { + plant(); + } + }); +} + pub(super) fn crashed() -> bool { CRASHED.get() } @@ -305,3 +318,44 @@ fn the_saved_file_is_mode_0600_and_the_directory_0700() { assert_eq!(std::fs::metadata(world.sandbox.path().join(path)).unwrap().permissions().mode() & 0o7777, 0o600); assert_eq!(std::fs::metadata(world.attachments()).unwrap().permissions().mode() & 0o7777, 0o700); } + +#[test] +fn a_file_or_link_planted_at_the_temporary_name_after_its_unlink_is_refused_and_never_written_through() { + let world = World::new(); + let image = world.put(ROOM, PNG, "png"); + let victim = world.sandbox.path().join("victim"); + let part = world.attachments().join(format!(".{}.png.part", image.sha256)); + for plant_a_link in [false, true] { + std::fs::write(&victim, b"keep me").unwrap(); + let (victim_path, part_path) = (victim.clone(), part.clone()); + let plant: Box = Box::new(move || { + if plant_a_link { + symlink(&victim_path, &part_path).unwrap(); + } else { + // A regular file of her own: an create without O_EXCL would open it and truncate. + std::fs::write(&part_path, b"hers").unwrap(); + } + }); + GAP.with(|gap| *gap.borrow_mut() = Some(plant)); + let saved = world.save(&image); + GAP.with(|gap| *gap.borrow_mut() = None); + assert_eq!(saved, Err(SaveFault::WriteFailed), "link: {plant_a_link}"); + assert_eq!(std::fs::read(&victim).unwrap(), b"keep me", "nothing was written through the planted name (link: {plant_a_link})"); + if !plant_a_link { + assert_eq!(std::fs::read(&part).unwrap(), b"hers", "her file is not truncated"); + } + assert!(!world.attachments().join(format!("{}.png", image.sha256)).exists(), "no final name from a refused create"); + let _ = std::fs::remove_file(&part); + } +} + +#[test] +fn a_link_to_a_same_length_regular_file_at_the_final_name_is_exists_not_saved() { + let world = World::new(); + let image = world.put(ROOM, PNG, "png"); + let outside = world.sandbox.path().join("outside.bin"); + std::fs::write(&outside, PNG).unwrap(); + std::fs::create_dir(world.attachments()).unwrap(); + symlink(&outside, world.attachments().join(format!("{}.png", image.sha256))).unwrap(); + assert_eq!(world.save(&image), Err(SaveFault::Exists), "a link is not a file in her sandbox, whatever it points at"); +} diff --git a/crates/mind-body/src/persona/tests.rs b/crates/mind-body/src/persona/tests.rs index a9dcaf7..60f50f3 100644 --- a/crates/mind-body/src/persona/tests.rs +++ b/crates/mind-body/src/persona/tests.rs @@ -9185,3 +9185,72 @@ fn both_notices_survive_a_reply_with_open_direct_and_save_attachment() { assert_eq!(rig.chat.fetches.borrow()[0].request_id, "s1-1-a"); assert_eq!(rig.chat.opens.borrow()[0].request_id, "s1-1-d"); } + +#[test] +fn a_revised_reply_with_an_open_and_a_save_sends_them_under_different_request_ids() { + use crate::chat_client::{FetchOutcome as F, ReceiptMode as R}; + let store = tempfile::tempdir().unwrap(); + let both = |extra: &str| { + let mut reply: serde_json::Value = serde_json::from_str(&opening(&[("swarm", extra)], "@ann:chat", "Come and see")).unwrap(); + reply["save_attachment"] = json!({ "room": "swarm", "event": "$pic" }); + reply.to_string() + }; + let rig = Rig::new(vec![], &[]); + rig.clock.step.set(0); + let image = stored_image(store.path(), "!swarm:chat", PNG_BYTES); + *rig.chat.fetch_answer.borrow_mut() = FetchAnswer::Receipt(crate::chat_client::ChatFetchReceipt { request_id: String::new(), outcome: F::Stored, room_id: Some("!swarm:chat".into()), image: Some(image), receipt: Some(R::Posted) }); + let config = Config { image_store: Some(store.path().to_path_buf()), ..on() }; + let (ask, answer) = (delivery(1, "swarm", "@ann:chat", "who owns the stairs?"), delivery(2, "swarm", "@libby:chat", "the stairs are mine")); + arrives(&rig, &ask); + with_inbox(&rig, &config, vec![ask.clone()], |inbox| { + inbox.start_line().unwrap(); + inbox.echoed(); + arrives(&rig, &answer); + inbox.result(&reply_result(&both("I own the stairs")), true, false).unwrap(); + assert!(inbox.poll().unwrap().pending); + inbox.tell(false).unwrap(); + inbox.echoed(); + inbox.result(&reply_result(&both("then the cellar is mine")), true, false).unwrap(); + let (opens, fetches) = (rig.chat.opens.borrow().clone(), rig.chat.fetches.borrow().clone()); + assert_eq!((opens.len(), fetches.len()), (1, 1)); + assert_ne!(opens[0].request_id, fetches[0].request_id, "two documents never share an id"); + assert!(opens[0].request_id.contains("-r1-") && opens[0].request_id.ends_with("-d"), "{}", opens[0].request_id); + assert!(fetches[0].request_id.contains("-r1-") && fetches[0].request_id.ends_with("-a"), "{}", fetches[0].request_id); + }); +} + +#[test] +fn every_failed_save_is_told_to_her_in_her_own_next_turn_and_leaves_no_file() { + use crate::chat_client::{FetchOutcome as F, ReceiptMode as R}; + type Prepare = Box; + let cases: [(&str, Prepare, &str); 5] = [ + ("RepoTree", Box::new(|sandbox, _| std::fs::create_dir_all(sandbox.join(".git")).unwrap()), "git work tree"), + ("OutsideSandbox", Box::new(|sandbox, _| std::os::unix::fs::symlink("/", sandbox.join("attachments")).unwrap()), "link or not a folder"), + ("Exists", Box::new(|sandbox, sha| { + std::fs::create_dir_all(sandbox.join("attachments")).unwrap(); + std::fs::write(sandbox.join("attachments").join(format!("{sha}.png")), b"hers").unwrap(); + }), "a different file already holds that name"), + ("WriteFailed", Box::new(|sandbox, sha| std::fs::create_dir_all(sandbox.join("attachments").join(format!(".{sha}.png.part"))).unwrap()), "could not be written into your sandbox"), + ("Load", Box::new(|_, _| {}), "the stored image cannot be read"), + ]; + for (word, prepare, told) in cases { + let (rig, store, image) = fetching(F::Stored, Some(R::Posted), &[saving("swarm", "$pic"), says(&[])]); + let (deliveries, pushed) = (rig.chat.deliveries.clone(), Cell::new(false)); + *rig.model.during.borrow_mut() = Some(Box::new(move || { + if !pushed.replace(true) { + deliveries.borrow_mut().push(delivery(2, "swarm", "@ann:chat", "any news?")); + } + })); + std::fs::create_dir_all(rig.sandbox()).unwrap(); + prepare(&rig.sandbox(), &image.sha256); + if word == "Load" { + std::fs::remove_file(store.path().join(&image.path)).unwrap(); + } + assert_eq!(failure_of(rig.go_with(&Config { image_store: Some(store.path().to_path_buf()), ..on() })), None, "{word}: told, never a failed step"); + let line = input_of(&rig, 1); + assert!(line.contains("Your attachment was not saved") && line.contains(told), "{word}: {line}"); + assert!(!line.contains("The room shows you received it"), "{word}: {line}"); + assert!(rig.trace.0.borrow()[0].outputs.iter().any(|item| item.text.starts_with("save_attachment swarm $pic: ") && item.text.contains(word)), "{word}"); + assert!(!rig.sandbox().join("attachments").join(format!("{}.png", image.sha256)).exists() || word == "Exists", "{word}"); + } +} diff --git a/crates/mind-body/tests/chat_rudp.rs b/crates/mind-body/tests/chat_rudp.rs index 874eb33..f38dc38 100644 --- a/crates/mind-body/tests/chat_rudp.rs +++ b/crates/mind-body/tests/chat_rudp.rs @@ -132,7 +132,7 @@ fn serve(key: &str, bind: &str, pending_kind: &'static str, pending_bytes: &'sta PENDING_REQUEST_V1 if pending_kind != PENDING && for_eureka(&chat_client::decode::(&document.payload).unwrap().persona) => Some(answer(pending_kind, "eureka", pending_bytes)), SEND_REQUEST if for_eureka(&chat_client::decode::(&document.payload).unwrap().persona) => Some(answer(SEND_RECEIPT, &document.record_key, SEND_RECEIPT_BYTES)), OPEN_DIRECT_REQUEST if for_eureka(&chat_client::decode::(&document.payload).unwrap().persona) => Some(answer(open.0, open.1.unwrap_or(&document.record_key), open.2)), - FETCH_REQUEST if for_eureka(&chat_client::decode::(&document.payload).unwrap().persona) => Some(answer(FETCH_RECEIPT, &document.record_key, FETCH_POSTED_BYTES)), + FETCH_REQUEST if for_eureka(&chat_client::decode::(&document.payload).unwrap().persona) => Some(if open.0 == REFUSAL { answer(open.0, open.1.unwrap_or(&document.record_key), open.2) } else { answer(FETCH_RECEIPT, &document.record_key, FETCH_POSTED_BYTES) }), _ => None, }; if let Some(reply) = reply { @@ -437,3 +437,29 @@ fn the_four_field_v3_crosses_real_rudp_with_its_gaps_and_the_three_field_one_wit let quiet = fetch(four_field(PENDING_TAIL_BYTES, &["#swarm:gamecult.org"])); assert!(!quiet.tail_gaps.contains(&"#operator:gamecult.org".to_string()), "a room the mouth did not name is not a gap, however few lines it has"); } + +#[test] +fn a_mouth_that_does_not_serve_the_fetch_kind_refuses_it_and_the_fetch_is_known_not_sent_at_once() { + let mouth = opening_with((REFUSAL, Some("eureka"), UNKNOWN_KIND_BYTES)); + let chat = client(&mouth, KEY, "eureka", Duration::from_secs(20)); + let request = chat_client::decode::(FETCH_REQUEST_BYTES).unwrap(); + let started = std::time::Instant::now(); + let error = chat.fetch(&request).unwrap_err(); + assert!(started.elapsed() < Duration::from_secs(5), "the refusal ends the wait: {:?}", started.elapsed()); + assert_eq!(error.downcast_ref::(), Some(&MouthRefused(Refusal::UnknownKind)), "{error}"); +} + +#[test] +fn a_fetch_refusal_of_another_reason_or_naming_another_persona_is_not_heard_as_not_sent() { + let request = chat_client::decode::(FETCH_REQUEST_BYTES).unwrap(); + // The fixture refusal is persona "ghost", UnknownPersona: a refusal for this Persona for a reason other than the kind. + let mouth = opening_with((REFUSAL, Some("ghost"), REFUSAL_BYTES)); + let error = client(&mouth, KEY, "ghost", Duration::from_secs(20)).fetch(&request).unwrap_err(); + assert!(error.downcast_ref::().is_none(), "only UnknownKind is a known not-sent: {error}"); + assert!(error.to_string().contains("UnknownPersona"), "{error}"); + // UnknownKind keyed by this persona but naming another. + let other = chat_client::encode(&ChatRefusal { persona: "someone-else".into(), refusal: Refusal::UnknownKind }).unwrap(); + let mouth = opening_with((REFUSAL, Some("eureka"), Box::leak(other.into_boxed_slice()))); + let error = client(&mouth, KEY, "eureka", Duration::from_secs(20)).fetch(&request).unwrap_err(); + assert!(error.downcast_ref::().is_none(), "the payload names another Persona: {error}"); +}