diff --git a/crates/oa-core/src/persistence.rs b/crates/oa-core/src/persistence.rs index 57484010..e7bc3f0b 100644 --- a/crates/oa-core/src/persistence.rs +++ b/crates/oa-core/src/persistence.rs @@ -3,6 +3,9 @@ use serde::{Deserialize, Serialize}; use std::{io::Write, path::Path}; use thiserror::Error; mod json_codec; +#[cfg(test)] +#[path = "persistence/source_save_tests.rs"] +mod source_save_tests; #[path = "migration_v2/mod.rs"] pub(crate) mod v2; pub(crate) use v2::adapter::migrate as migrate_v2_limited; @@ -948,19 +951,54 @@ fn save_checked_profile( expected: Option<&[u8]>, profile: json_codec::Profile, depth_128: bool, +) -> Result, FileError> { + save_checked_profile_with_source(path, p, expected, profile, depth_128, |project| { + crate::language::format(project).into_bytes() + }) +} +// Keep the source writer replaceable only at this private seam so regressions +// can prove that malformed or lossy output never reaches filesystem staging. +fn save_checked_profile_with_source( + path: &Path, + p: &Project, + expected: Option<&[u8]>, + profile: json_codec::Profile, + depth_128: bool, + source: impl FnOnce(&Project) -> Vec, ) -> Result, FileError> { validate_project(p)?; - let mut bytes = if path + let is_source = path .extension() - .is_some_and(|ext| ext.eq_ignore_ascii_case("arcadia")) - { - crate::language::format(p).into_bytes() + .is_some_and(|ext| ext.eq_ignore_ascii_case("arcadia")); + let mut bytes = if is_source { + source(p) } else { json_codec::encode(p, profile, depth_128)? }; if !bytes.ends_with(b"\n") { bytes.push(b'\n'); } + if is_source { + // Check exactly the final bytes, including their newline, through the + // public reader before creating a temp file or replacing a destination. + let decoded = decode_with_report_limited( + &bytes, + crate::language::ParseLimits { + max_source_bytes: profile.bytes, + ..crate::language::ParseLimits::UNLIMITED + }, + )?; + if decoded.project != *p { + return Err(FileError::Invalid( + "Generated source does not reproduce the complete requested project.".into(), + )); + } + if decoded.migration.is_some() { + return Err(FileError::Invalid( + "Generated source unexpectedly requires migration.".into(), + )); + } + } let parent = path .parent() .filter(|p| !p.as_os_str().is_empty()) diff --git a/crates/oa-core/src/persistence/source_save_tests.rs b/crates/oa-core/src/persistence/source_save_tests.rs new file mode 100644 index 00000000..73906adf --- /dev/null +++ b/crates/oa-core/src/persistence/source_save_tests.rs @@ -0,0 +1,145 @@ +//! Containment while the complete source8 adapter remains unavailable. +use super::*; +use crate::analysis::{Definition, DefinitionBody, QuantityAlgebra, QuantityKindDefinition}; +use crate::analysis_numeric::Dimension; +use std::fs; + +const ORDINARY_SOURCE7: &str = concat!( + "arcadia 7;\n", + "project saved \"Saved model\" id \"4210b9f2-7046-47f0-aa3b-153f626a579d\" {\n", + " description \"Retain café and exact identity\";\n", + "}", +); + +fn ordinary() -> Project { + let decoded = decode_with_report(ORDINARY_SOURCE7.as_bytes()).unwrap(); + assert!(decoded.migration.is_some()); + decoded.project +} + +fn directory_entries(path: &Path) -> Vec { + let mut names = fs::read_dir(path) + .unwrap() + .map(|entry| entry.unwrap().file_name()) + .collect::>(); + names.sort(); + names +} + +// A missing parent makes a mistakenly early NamedTempFile creation observable +// as Io, even when the temp would otherwise be removed again on error. +fn rejected_output( + project: &Project, + output: &[u8], + profile: json_codec::Profile, + check: impl Fn(&FileError), +) { + let root = tempfile::tempdir().unwrap(); + let existing = root.path().join("existing.arcadia"); + let sentinel = b"external bytes must survive\r\n"; + fs::write(&existing, sentinel).unwrap(); + for path in [ + existing.clone(), + root.path().join("new.arcadia"), + root.path().join("missing-parent/new.arcadia"), + ] { + let before = directory_entries(root.path()); + let expected = (path == existing).then_some(sentinel.as_slice()); + let error = + save_checked_profile_with_source(&path, project, expected, profile, false, |_| { + output.to_vec() + }) + .unwrap_err(); + check(&error); + assert_eq!(directory_entries(root.path()), before); + assert_eq!(fs::read(&existing).unwrap(), sentinel); + if path != existing { + assert!(!path.exists()); + } + } +} + +#[test] +fn malformed_generated_source_rejects_before_any_filesystem_staging() { + rejected_output( + &ordinary(), + b"arcadia", + json_codec::Profile::DOCUMENT, + |error| assert!(matches!(error, FileError::Text(_)), "{error}"), + ); +} + +#[test] +fn parseable_source_omitting_a_definition_cannot_replace_the_whole_project() { + let mut project = ordinary(); + let id = Id::parse_str("188b911c-5f8b-431c-9eea-2d41f3953767").unwrap(); + project.definitions.insert( + id, + Definition { + id, + alias: "duration".into(), + name: "Duration".into(), + description: "Unused definitions are still authored model data.".into(), + body: DefinitionBody::QuantityKind(QuantityKindDefinition { + dimension: Dimension::new([0, 0, 1, 0, 0, 0, 0]).unwrap(), + algebra: QuantityAlgebra::Linear, + }), + }, + ); + validate_project(&project).unwrap(); + let decoded = decode(ORDINARY_SOURCE7.as_bytes()).unwrap(); + assert_eq!(decoded.id, project.id); + assert_eq!(decoded.name, project.name); + assert_eq!(decoded.elements, project.elements); + assert!(decoded.definitions.is_empty()); + assert_ne!(decoded, project); + rejected_output( + &project, + ORDINARY_SOURCE7.as_bytes(), + json_codec::Profile::DOCUMENT, + |error| { + assert!( + matches!(error, FileError::Invalid(message) + if message == "Generated source does not reproduce the complete requested project."), + "{error}" + ); + }, + ); +} + +#[test] +fn equal_project_from_generated_legacy_source_still_rejects_migration() { + rejected_output( + &ordinary(), + ORDINARY_SOURCE7.as_bytes(), + json_codec::Profile::DOCUMENT, + |error| { + assert!( + matches!(error, FileError::Invalid(message) + if message == "Generated source unexpectedly requires migration."), + "{error}" + ); + }, + ); +} + +#[test] +fn generated_source_admission_checks_final_newline_under_the_effective_byte_limit() { + let project = ordinary(); + let profile = json_codec::Profile::with_byte_limit(ORDINARY_SOURCE7.len()); + rejected_output(&project, ORDINARY_SOURCE7.as_bytes(), profile, |error| { + assert!(matches!(error, FileError::Limit(_)), "{error}"); + }); + let mut terminated = ORDINARY_SOURCE7.as_bytes().to_vec(); + terminated.push(b'\n'); + let profile = json_codec::Profile::with_byte_limit(terminated.len()); + // Existing final newline is not added twice: decoding now reaches the + // independent migration refusal, rather than exceeding this exact limit. + rejected_output(&project, &terminated, profile, |error| { + assert!( + matches!(error, FileError::Invalid(message) + if message == "Generated source unexpectedly requires migration."), + "{error}" + ); + }); +}