diff --git a/src/control_plane.rs b/src/control_plane.rs index f7093c9..dbe5096 100644 --- a/src/control_plane.rs +++ b/src/control_plane.rs @@ -47,7 +47,9 @@ use crate::drivers::{ WriteLeasePort, }; use crate::incident::{CloseReason, IncidentCondition, IncidentStore, render_incidents}; -use crate::runner_evidence::{self, RunnerFailureRecord, runner_evidence_store_path}; +use crate::runner_evidence::{ + self, Kept, RunnerFailureRecord, runner_evidence_store_path, unreadable_store_path, +}; use crate::host_actuator::{ HostActuatorAccess, HostActuatorHub, HostActuatorRunnerDriver, HostActuatorWorkloadDriver, HostUnobservable, IdunnHostActuatorIdentity, SharedHostActuatorHub, spawn_hub_service, @@ -10695,18 +10697,27 @@ fn runner_failure_journal_line( transaction: &DeploymentTransaction, failure: &RunnerStepFailure, store_path: &Path, - recorded: &Result<()>, + recorded: &Result, ) -> String { let head = format!( "Idunn transaction {} (command {}, target {}) {failure}", transaction.transaction_id, transaction.command_id, transaction.target ); match recorded { - Ok(()) => format!( - "{head}; evidence in {}; read it with idunn status --command {}", - store_path.display(), - transaction.command_id - ), + Ok(kept) => { + let aside = match kept { + Kept::Stored => String::new(), + Kept::AfterSettingAsideAnUnreadableStore => format!( + "; an undecodable evidence store was set aside as {}", + unreadable_store_path(store_path).display() + ), + }; + format!( + "{head}; evidence in {}{aside}; read it with idunn status --command {}", + store_path.display(), + transaction.command_id + ) + } Err(error) => format!("{head}; evidence was not kept: {error:#}"), } } @@ -24963,6 +24974,38 @@ GAMECULT_RUNTIME_PRESENCE_IDENTITY = "/etc/gamecult/service/runtime-presence-ide Ok(()) } + #[cfg(unix)] + #[test] + fn an_undecodable_evidence_store_does_not_stop_the_next_failure_keeping_its_evidence() -> Result<()> { + let damage = format!("\u{c1} not a store {RUNNER_CANARY}-in-the-damage"); + let (world, _tools) = failing_runner_world()?; + let evidence = runner_evidence_store_path(&world.state_store); + std::fs::write(&evidence, damage.as_bytes())?; + let ended = tick_until_ended(&world)?; + // The transaction's own outcome is the runner failure, not the store. + let Some(TransactionCompletion::FailedBeforeFencing { error }) = &ended.completion else { + bail!("the damaged store changed the outcome: {:?} {:?}", ended.phase, ended.last_error); + }; + assert!(error.contains("runner step test-daemon on runner rust"), "{error}"); + assert!(!error.contains("damage"), "{error}"); + // The damage is kept aside; the failure's evidence is in a fresh store. + assert_eq!(std::fs::read(unreadable_store_path(&evidence))?, damage.as_bytes()); + let record = runner_evidence::for_transaction(&evidence, &ended.transaction_id)? + .context("the failure kept no evidence beside a damaged store")?; + assert_eq!(record.subject_id, "test-daemon"); + assert!(printed_status(&world)?.contains("failed step test-daemon")); + // The journal says it, naming the file and none of its content. + std::fs::write(&evidence, damage.as_bytes())?; + let line = world + .engine + .keep_runner_failure(&ended, &anyhow::Error::new(canary_failure(b"x"))) + .context("no journal line")?; + assert!(line.contains("an undecodable evidence store was set aside as"), "{line}"); + assert!(line.contains("runner-evidence.unreadable.cc"), "{line}"); + assert!(!line.contains("damage") && !line.contains("not a store"), "{line}"); + Ok(()) + } + #[cfg(unix)] #[test] fn an_unreadable_evidence_store_costs_only_its_lines() -> Result<()> { diff --git a/src/runner_evidence.rs b/src/runner_evidence.rs index d77961f..cb08e0a 100644 --- a/src/runner_evidence.rs +++ b/src/runner_evidence.rs @@ -182,10 +182,44 @@ fn snapshot(path: &Path) -> Result<(Vec, Vec PathBuf { + path.with_file_name("runner-evidence.unreadable.cc") +} + +/// An undecodable store (a corrupt file, or one a newer Idunn wrote) must not +/// stop later failures from keeping their evidence. Move it aside, root-only, +/// over the previous one. Nothing of its content is named: a decode error can +/// quote stored values. A store that cannot even be read is left in place. +fn set_aside_unreadable(path: &Path) -> Result<()> { + std::fs::read(path).context("the runner evidence store could not be read")?; + let aside = unreadable_store_path(path); + std::fs::rename(path, &aside) + .context("the runner evidence store is undecodable and could not be set aside")?; + keep_private(&aside) +} + /// The one writer. Upsert by transaction id; past `RUNNER_EVIDENCE_RECORDS` the -/// oldest records go in the same exchange. The file ends at mode 0600. -pub(crate) fn record(path: &Path, record: &RunnerFailureRecord) -> Result<()> { - let (envelopes, records) = snapshot(path)?; +/// oldest records go in the same exchange. The file ends at mode 0600. A store +/// that cannot be decoded is set aside first and the record opens a new one. +pub(crate) fn record(path: &Path, record: &RunnerFailureRecord) -> Result { + let (envelopes, records, how) = match snapshot(path) { + Ok((envelopes, records)) => (envelopes, records, Kept::Stored), + Err(_) => { + set_aside_unreadable(path)?; + (Vec::new(), Vec::new(), Kept::AfterSettingAsideAnUnreadableStore) + } + }; let mut kept: Vec<(u64, CultCacheEnvelope)> = envelopes .iter() .zip(&records) @@ -214,7 +248,8 @@ pub(crate) fn record(path: &Path, record: &RunnerFailureRecord) -> Result<()> { .into()); } } - keep_private(path) + keep_private(path)?; + Ok(how) } #[cfg(unix)] @@ -270,10 +305,22 @@ pub(crate) fn render(record: &RunnerFailureRecord) -> Vec { lines } +/// Format characters that reorder what a terminal shows: the set the mind's +/// Title rule refuses. +fn is_bidi_control(character: char) -> bool { + matches!( + character, + '\u{61c}' | '\u{200e}' | '\u{200f}' | '\u{202a}'..='\u{202e}' | '\u{2066}'..='\u{2069}' + ) +} + fn escape(text: &str) -> String { let mut escaped = String::with_capacity(text.len()); for character in text.chars() { - if character.is_control() && character != '\n' && character != '\t' { + if (character.is_control() || is_bidi_control(character)) + && character != '\n' + && character != '\t' + { escaped.push_str(&format!("\\u{{{:x}}}", u32::from(character))); } else { escaped.push(character); @@ -428,12 +475,162 @@ mod tests { } #[test] - fn a_foreign_or_corrupt_store_is_an_error_not_an_empty_store() -> Result<()> { + fn the_reader_names_an_unreadable_store_as_an_error() -> Result<()> { let temp = tempfile::tempdir()?; let path = runner_evidence_store_path(&temp.path().join("control.cc")); std::fs::write(&path, b"\xc1 not a store")?; assert!(for_transaction(&path, "tx-a").is_err()); - assert!(record(&path, &record_at("tx-a", 1)).is_err()); Ok(()) } + + fn files_beside(path: &Path) -> Result> { + let mut names: Vec = std::fs::read_dir(path.parent().context("no parent")?)? + .map(|entry| Ok(entry?.file_name().to_string_lossy().into_owned())) + .collect::>()?; + names.retain(|name| name.starts_with("runner-evidence") && !name.contains("lock")); + names.sort(); + Ok(names) + } + + /// A store a newer Idunn wrote: well-formed, but a schema this one does not know. + fn write_future_store(path: &Path) -> Result<()> { + let mut future = record_at("tx-future", 1); + future.schema_version = "idunn.runner_failure.v2".into(); + let envelope = typed_envelope( + &future.transaction_id, + RunnerFailureRecord::TYPE, + "idunn.runner_failure.v2", + &future, + 1, + )?; + let outcome = exchange_behind_private_lock(path, &[], &[envelope])?; + ensure!( + matches!(outcome, TryCompareExchangeSnapshotOutcome::Exchanged), + "the future store was not written" + ); + Ok(()) + } + + #[test] + fn a_corrupt_store_is_set_aside_and_the_next_failure_keeps_its_evidence() -> Result<()> { + let temp = tempfile::tempdir()?; + let path = runner_evidence_store_path(&temp.path().join("control.cc")); + let damage = b"\xc1 not a store \x00 damage"; + std::fs::write(&path, damage)?; + let kept = record(&path, &record_at("tx-a", 5))?; + assert_eq!(kept, Kept::AfterSettingAsideAnUnreadableStore); + // Kept, not deleted: the damage is where the journal says, root-only. + let aside = unreadable_store_path(&path); + assert_eq!(std::fs::read(&aside)?, damage); + assert_eq!(std::fs::metadata(&aside)?.permissions().mode() & 0o777, 0o600); + // The failure's evidence is in a fresh, private store. + assert_eq!( + for_transaction(&path, "tx-a")?.map(|stored| stored.transaction_id), + Some("tx-a".to_owned()) + ); + assert_eq!(std::fs::metadata(&path)?.permissions().mode() & 0o777, 0o600); + // A healthy store is not set aside again. + assert_eq!(record(&path, &record_at("tx-b", 6))?, Kept::Stored); + assert_eq!(std::fs::read(&aside)?, damage); + Ok(()) + } + + #[test] + fn a_future_version_store_is_set_aside_and_the_next_failure_keeps_its_evidence() -> Result<()> { + let temp = tempfile::tempdir()?; + let path = runner_evidence_store_path(&temp.path().join("control.cc")); + write_future_store(&path)?; + let before = std::fs::read(&path)?; + assert!(for_transaction(&path, "tx-future").is_err()); + assert_eq!( + record(&path, &record_at("tx-a", 5))?, + Kept::AfterSettingAsideAnUnreadableStore + ); + assert_eq!(std::fs::read(unreadable_store_path(&path))?, before); + assert!(for_transaction(&path, "tx-a")?.is_some()); + assert_eq!(for_transaction(&path, "tx-future")?, None); + Ok(()) + } + + #[test] + fn at_most_one_unreadable_store_is_kept() -> Result<()> { + let temp = tempfile::tempdir()?; + let path = runner_evidence_store_path(&temp.path().join("control.cc")); + let damages: [&[u8]; 3] = [b"\xc1 first", b"\xc1 second", b"\xc1 third"]; + for (index, damage) in damages.into_iter().enumerate() { + std::fs::write(&path, damage)?; + record(&path, &record_at(&format!("tx-{index}"), 5 + index as u64))?; + assert_eq!( + std::fs::read(unreadable_store_path(&path))?, + damage, + "the newest damage is the one kept" + ); + } + assert_eq!( + files_beside(&path)?, + vec![ + "runner-evidence.cc".to_owned(), + "runner-evidence.unreadable.cc".to_owned() + ] + ); + Ok(()) + } + + #[test] + fn a_store_that_cannot_be_read_is_left_in_place_and_the_write_fails() -> Result<()> { + let temp = tempfile::tempdir()?; + let path = runner_evidence_store_path(&temp.path().join("control.cc")); + // A directory where the store should be: unreadable, not undecodable. + std::fs::create_dir(&path)?; + assert!(record(&path, &record_at("tx-a", 1)).is_err()); + assert!(path.is_dir()); + assert!(!unreadable_store_path(&path).exists()); + Ok(()) + } + + #[test] + fn status_escapes_bidi_controls_in_tails_and_in_names() { + for bidi in [ + '\u{61c}', '\u{200e}', '\u{200f}', '\u{202a}', '\u{202b}', '\u{202c}', '\u{202d}', + '\u{202e}', '\u{2066}', '\u{2067}', '\u{2068}', '\u{2069}', + ] { + let mut stored = record_at("tx-a", 1); + let tail = format!("ok {bidi}gpj.exe\n"); + stored.stdout_tail = tail.clone().into_bytes(); + stored.stdout_total_bytes = tail.len() as u64; + stored.stderr_tail = tail.clone().into_bytes(); + stored.stderr_total_bytes = tail.len() as u64; + stored.subject_id = format!("step{bidi}x"); + stored.runner = format!("rust{bidi}"); + let text = render(&stored).join("\n"); + let escaped = format!("\\u{{{:x}}}", bidi as u32); + assert!(!text.contains(bidi), "U+{:04X} reached the terminal", bidi as u32); + // Escaped in every place it can appear: two tails, the subject, the runner. + assert_eq!(text.matches(&escaped).count(), 4, "{text}"); + } + // Format characters either side of the set pass. + let mut stored = record_at("tx-a", 1); + stored.stdout_tail = "caf\u{e9} \u{200d}\u{202f}\u{2065}\u{206a}\n".as_bytes().to_vec(); + stored.stdout_total_bytes = stored.stdout_tail.len() as u64; + let text = render(&stored).join("\n"); + assert!(text.contains("caf\u{e9} \u{200d}\u{202f}\u{2065}\u{206a}"), "{text}"); + } + + #[test] + fn status_escapes_the_c1_controls() { + for code in 0x80u32..=0x9f { + let character = char::from_u32(code).unwrap(); + let mut stored = record_at("tx-a", 1); + stored.stdout_tail = format!("a{character}b\n").into_bytes(); + stored.stdout_total_bytes = stored.stdout_tail.len() as u64; + let text = render(&stored).join("\n"); + assert!(!text.contains(character), "U+{code:04X} reached the terminal"); + assert!(text.contains(&format!("a\\u{{{code:x}}}b")), "U+{code:04X}: {text}"); + } + // U+00A0 is not a control character and passes. + let mut stored = record_at("tx-a", 1); + stored.stdout_tail = "a\u{a0}b\n".as_bytes().to_vec(); + stored.stdout_total_bytes = stored.stdout_tail.len() as u64; + assert!(render(&stored).join("\n").contains("a\u{a0}b")); + } }