diff --git a/THREAT_MODEL.md b/THREAT_MODEL.md index db08f60..4b3df9e 100644 --- a/THREAT_MODEL.md +++ b/THREAT_MODEL.md @@ -30,7 +30,8 @@ Source artifacts, imported ontologies, provider responses, model outputs, web-re 7. tenant/workspace evidence disclosure; 8. SSRF, DNS rebinding, unsafe redirects, or unbounded external retrieval; 9. dependency/provider compromise or unexpected retention; -10. write-back without reviewed before/after/rollback evidence and exact preconditions. +10. write-back without reviewed before/after/rollback evidence and exact preconditions; +11. same-host filesystem races replacing a checked owner-only review artifact path with a symlink before the file descriptor is opened. ## Zotero 10+ Local API transport boundary @@ -64,6 +65,14 @@ Neither path may reinterpret `Zotero-Server-ID` as cryptographic peer authentica - attachments and bibliographic source records are not deleted by classification write-back; - descendant integration evidence never back-proves an unresolved predecessor contract. +## Owner-only review artifact filesystem boundary + +Saved report, worksheet, approval, progress, and golden-set artifacts are sensitive local review material. Path policy alone is not the security boundary. A direct-temp-child path may be checked as a regular non-symlink and still be replaced by a symlink before a later symlink-following open. + +The opened file descriptor must therefore be obtained with a Unix final-component no-follow primitive such as `O_NOFOLLOW` or an equivalent safe abstraction. After open, ConceptWeave still verifies that the opened device/inode matches the checked regular file, link count is one, mode is exactly `0600`, and the bounded-read contract is satisfied. A second pathname check is not an equivalent repair because it leaves another check/open race window. + +PR #30 commit `9733d28` reproduced the inode-preserving final-component symlink swap and left the no-follow contract RED. Commit `7ccbbbe` repairs the shared input-open helper with Unix `O_NOFOLLOW`; focused security and artifact-identity tests then pass. Offline review/finalization remains acceptance-gated until this repair has terminal protected checks and independent approval on one unchanged exact head. + ## Release gate A capability is not release-ready while a valid security finding lacks a deterministic test or equivalent machine-verifiable contract, while required exact-head checks are non-terminal, or while the implemented transport cannot satisfy the advertised security claim. Documentation must describe residual risk without upgrading provider guarantees by inference. diff --git a/crates/conceptweave-zotero/src/lib.rs b/crates/conceptweave-zotero/src/lib.rs index b8d68e3..15ddf1d 100644 --- a/crates/conceptweave-zotero/src/lib.rs +++ b/crates/conceptweave-zotero/src/lib.rs @@ -1139,6 +1139,30 @@ pub struct StewardReviewWorksheet { pub decisions: Vec, } +/// Aggregate-only checkpoint for a human steward review campaign. +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub struct StewardReviewProgress { + /// Zotero library revision bound to the original report. + pub library_version: u64, + /// Classifier revision whose proposals are being reviewed. + pub rule_revision: String, + /// Opaque immutable snapshot identity. + pub snapshot_digest: String, + /// Opaque binding to the proposals and retained metadata used for these counts. + pub proposal_digest: String, + /// Number of bibliographic decisions required for completion. + pub total_count: usize, + /// Number of non-abstention decisions supplied by a steward. + pub decided_count: usize, + /// Number of decisions still blank. + pub remaining_count: usize, + /// Number of source records whose ancestry still requires resolution. + pub pending_source_count: usize, + /// Whether nonempty decision slots are filled and no source remains pending. + /// This is local preparation status, never independent approval or an applied write. + pub complete: bool, +} + /// A classification report cannot safely produce a review worksheet. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum WorksheetError { @@ -1193,6 +1217,68 @@ pub fn build_steward_review_worksheet( }) } +/// Validates an in-progress worksheet and returns privacy-safe aggregate progress. +pub fn assess_steward_review_progress( + report: &ClassificationReport, + worksheet: &StewardReviewWorksheet, +) -> Result { + let expected = build_steward_review_worksheet(report)?; + validate_steward_review_worksheet_against(&expected, worksheet)?; + if worksheet + .decisions + .iter() + .any(|decision| decision.reviewed_disposition == Some(Disposition::NeedsStewardReview)) + { + return Err(WorksheetError::InvalidReport); + } + let decided_count = worksheet + .decisions + .iter() + .filter(|decision| decision.reviewed_disposition.is_some()) + .count(); + let total_count = worksheet.decisions.len(); + let remaining_count = total_count - decided_count; + Ok(StewardReviewProgress { + library_version: worksheet.library_version, + rule_revision: worksheet.rule_revision.clone(), + snapshot_digest: worksheet.snapshot_digest.clone(), + proposal_digest: worksheet.proposal_digest.clone(), + total_count, + decided_count, + remaining_count, + pending_source_count: report.pending_source_item_keys.len(), + complete: total_count > 0 + && remaining_count == 0 + && report.pending_source_item_keys.is_empty(), + }) +} + +fn validate_steward_review_worksheet_against( + expected: &StewardReviewWorksheet, + worksheet: &StewardReviewWorksheet, +) -> Result<(), WorksheetError> { + if worksheet.library_version != expected.library_version + || worksheet.rule_revision != expected.rule_revision + || worksheet.snapshot_digest != expected.snapshot_digest + || worksheet.proposal_digest != expected.proposal_digest + || worksheet.snapshot_items != expected.snapshot_items + || worksheet.decisions.len() != expected.decisions.len() + || worksheet + .decisions + .iter() + .zip(&expected.decisions) + .any(|(decision, expected)| { + decision.item_key != expected.item_key + || decision.item_version != expected.item_version + || decision.proposed_disposition != expected.proposed_disposition + || decision.abstention_reason != expected.abstention_reason + }) + { + return Err(WorksheetError::InvalidReport); + } + Ok(()) +} + /// One steward-reviewed expected disposition in a local golden set. #[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize)] pub struct GoldenLabel { @@ -1293,16 +1379,11 @@ pub fn reviewed_golden_set_from_worksheet( { return Err(EvaluationError::SnapshotMismatch); } - + if validate_steward_review_worksheet_against(&expected, worksheet).is_err() { + return Err(EvaluationError::InvalidReview); + } let mut labels = Vec::with_capacity(worksheet.decisions.len()); - for (decision, expected_decision) in worksheet.decisions.iter().zip(expected.decisions) { - if decision.item_key != expected_decision.item_key - || decision.item_version != expected_decision.item_version - || decision.proposed_disposition != expected_decision.proposed_disposition - || decision.abstention_reason != expected_decision.abstention_reason - { - return Err(EvaluationError::InvalidReview); - } + for decision in &worksheet.decisions { let expected_disposition = decision .reviewed_disposition .ok_or(EvaluationError::IncompleteReview)?; diff --git a/crates/conceptweave-zotero/src/main.rs b/crates/conceptweave-zotero/src/main.rs index c12bdcd..e016e8e 100644 --- a/crates/conceptweave-zotero/src/main.rs +++ b/crates/conceptweave-zotero/src/main.rs @@ -3,7 +3,8 @@ use conceptweave_zotero::{ ClassificationReport, GoldenSetApproval, StewardReviewWorksheet, - build_steward_review_worksheet, read_local_snapshot, reviewed_golden_set_from_worksheet, + assess_steward_review_progress, build_steward_review_worksheet, read_local_snapshot, + reviewed_golden_set_from_worksheet, }; use serde::de::DeserializeOwned; use std::collections::BTreeSet; @@ -12,7 +13,7 @@ use std::fs::{self, File, OpenOptions}; use std::io::{self, BufWriter, Read, Write}; use std::path::{Path, PathBuf}; -const USAGE: &str = "usage: conceptweave-zotero /tmp/REPORT.json | --worksheet /tmp/REPORT.json /tmp/WORKSHEET.json | --finalize /tmp/REPORT.json /tmp/WORKSHEET.json /tmp/APPROVAL.json /tmp/GOLDEN.json"; +const USAGE: &str = "usage: conceptweave-zotero /tmp/REPORT.json | --worksheet /tmp/REPORT.json /tmp/WORKSHEET.json | --review-progress /tmp/REPORT.json /tmp/WORKSHEET.json /tmp/PROGRESS.json | --finalize /tmp/REPORT.json /tmp/WORKSHEET.json /tmp/APPROVAL.json /tmp/GOLDEN.json"; const MAX_ARTIFACT_BYTES: u64 = 16 * 1024 * 1024; #[derive(Debug, PartialEq, Eq)] @@ -22,6 +23,11 @@ enum OutputRequest { report: String, worksheet: String, }, + ReviewProgress { + report: String, + worksheet: String, + output: String, + }, Finalize { report: String, worksheet: String, @@ -30,7 +36,7 @@ enum OutputRequest { }, } -/// Parses one mutually exclusive report, worksheet, or finalization request. +/// Parses one mutually exclusive report, worksheet, review-progress, or finalization request. fn parse_output_request(args: I) -> Result where I: IntoIterator, @@ -49,6 +55,24 @@ where return Err("report and worksheet output paths must differ"); } OutputRequest::Worksheet { report, worksheet } + } else if first == "--review-progress" { + let report = args + .next() + .ok_or("--review-progress requires three artifact paths")?; + let worksheet = args + .next() + .ok_or("--review-progress requires three artifact paths")?; + let output = args + .next() + .ok_or("--review-progress requires three artifact paths")?; + if BTreeSet::from([report.as_str(), worksheet.as_str(), output.as_str()]).len() != 3 { + return Err("review progress artifact paths must differ"); + } + OutputRequest::ReviewProgress { + report, + worksheet, + output, + } } else if first == "--finalize" { let report = args .next() @@ -142,7 +166,6 @@ fn read_private_json(raw: &str) -> io::Result<(T, ArtifactI } } -#[cfg_attr(coverage_nightly, coverage(off))] /// Opens without following a symlink or waiting for a raced FIFO, then reads handle metadata. fn open_with_metadata(path: &Path) -> io::Result<(File, fs::Metadata)> { #[cfg(unix)] @@ -153,8 +176,7 @@ fn open_with_metadata(path: &Path) -> io::Result<(File, fs::Metadata)> { #[cfg(unix)] options.custom_flags(libc::O_NOFOLLOW | libc::O_NONBLOCK); let file = options.open(path)?; - let metadata = file.metadata()?; - Ok((file, metadata)) + file.metadata().map(|metadata| (file, metadata)) } #[cfg(unix)] @@ -237,7 +259,6 @@ fn write_private_output(path: &Path, content: &[u8]) -> io::Result<()> { write_private_output_with(path, content, write_all_and_flush) } -#[cfg_attr(coverage_nightly, coverage(off))] /// Writes and flushes the complete serialized artifact. fn write_all_and_flush(writer: &mut BufWriter, content: &[u8]) -> io::Result<()> { writer.write_all(content)?; @@ -258,7 +279,6 @@ fn write_private_output_with( result } -#[cfg_attr(coverage_nightly, coverage(off))] /// Returns canonical directories in which a sensitive report may be created. fn allowed_output_parents() -> Vec { let mut parents = vec![ @@ -371,6 +391,26 @@ fn main() -> Result<(), Box> { write_private_output(&report_output, &report_content)?; write_private_output(&worksheet_output, &worksheet_content)?; } + OutputRequest::ReviewProgress { + report, + worksheet, + output, + } => { + let output = validate_output_path(&output)?; + let (report, report_identity): (ClassificationReport, _) = + read_private_json(&report).map_err(|error| label_input("report", error))?; + let (worksheet, worksheet_identity): (StewardReviewWorksheet, _) = + read_private_json(&worksheet).map_err(|error| label_input("worksheet", error))?; + if report_identity == worksheet_identity { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + "review progress inputs must be distinct files", + ) + .into()); + } + let progress = assess_steward_review_progress(&report, &worksheet)?; + write_private_output(&output, &serde_json::to_vec_pretty(&progress)?)?; + } OutputRequest::Finalize { report, worksheet, @@ -516,6 +556,37 @@ mod tests { ); } + #[test] + fn review_progress_mode_requires_three_distinct_artifact_paths() { + let report = "/tmp/report.json"; + let worksheet = "/tmp/worksheet.json"; + let output = "/tmp/progress.json"; + assert_eq!( + parse_output_request(vec!["--review-progress", report, worksheet, output]), + Ok(OutputRequest::ReviewProgress { + report: report.to_owned(), + worksheet: worksheet.to_owned(), + output: output.to_owned(), + }) + ); + assert!(parse_output_request(vec!["--review-progress"]).is_err()); + assert!(parse_output_request(vec!["--review-progress", report]).is_err()); + assert!(parse_output_request(vec!["--review-progress", report, worksheet]).is_err()); + assert!( + parse_output_request(vec!["--review-progress", report, worksheet, report]).is_err() + ); + assert!( + parse_output_request(vec![ + "--review-progress", + report, + worksheet, + output, + "extra" + ]) + .is_err() + ); + } + #[cfg(unix)] #[test] fn private_json_input_is_owner_only_regular_bounded_and_valid() { @@ -534,6 +605,7 @@ mod tests { assert_eq!(parsed["accepted"], true); assert!(read_private_json::("relative.json").is_err()); assert!(read_private_json::("/").is_err()); + assert!(read_private_json::("/tmp/..").is_err()); assert!( read_private_json::( unique_temp_path("missing-input").to_str().unwrap() @@ -824,6 +896,12 @@ mod tests { assert_eq!(fs::read(&output).unwrap(), b"complete"); assert!(write_private_output(&output, b"replacement").is_err()); fs::remove_file(output).unwrap(); + + let read_only = unique_temp_path("read-only-writer"); + fs::write(&read_only, b"input").unwrap(); + let mut writer = BufWriter::with_capacity(1, File::open(&read_only).unwrap()); + assert!(write_all_and_flush(&mut writer, b"content").is_err()); + fs::remove_file(read_only).unwrap(); } fn unique_temp_path(suffix: &str) -> PathBuf { diff --git a/crates/conceptweave-zotero/tests/finalization_artifact_identity.rs b/crates/conceptweave-zotero/tests/finalization_artifact_identity.rs index 75de1ef..ecd4950 100644 --- a/crates/conceptweave-zotero/tests/finalization_artifact_identity.rs +++ b/crates/conceptweave-zotero/tests/finalization_artifact_identity.rs @@ -9,7 +9,7 @@ use std::os::unix::fs::PermissionsExt; use std::process::Command; #[test] -fn finalization_rejects_distinct_path_spellings_for_one_input_file() { +fn artifact_commands_reject_distinct_path_spellings_for_one_input_file() { let item = ZoteroItem { source_record: None, key: "ITEM".into(), @@ -91,6 +91,15 @@ fn finalization_rejects_distinct_path_spellings_for_one_input_file() { ]) .status() .unwrap(); + let progress_status = Command::new(env!("CARGO_BIN_EXE_conceptweave-zotero")) + .args([ + "--review-progress", + &report_path, + &worksheet_path, + output.to_str().unwrap(), + ]) + .status() + .unwrap(); let _ = fs::remove_file(&input); let _ = fs::remove_file(&output); @@ -98,4 +107,8 @@ fn finalization_rejects_distinct_path_spellings_for_one_input_file() { !status.success(), "finalization must reject three path spellings that resolve to one input artifact" ); + assert!( + !progress_status.success(), + "progress must reject two path spellings that resolve to one input artifact" + ); } diff --git a/crates/conceptweave-zotero/tests/private_artifact_coverage_contract.rs b/crates/conceptweave-zotero/tests/private_artifact_coverage_contract.rs new file mode 100644 index 0000000..e1e6d20 --- /dev/null +++ b/crates/conceptweave-zotero/tests/private_artifact_coverage_contract.rs @@ -0,0 +1,16 @@ +#[test] +fn private_artifact_helpers_are_not_excluded_from_owned_coverage() { + let source = include_str!("../src/main.rs"); + for helper in ["fn write_all_and_flush", "fn allowed_output_parents"] { + let position = source + .find(helper) + .unwrap_or_else(|| panic!("missing production helper: {helper}")); + let prefix = &source[..position]; + let window_start = prefix.len().saturating_sub(240); + let declaration_context = &prefix[window_start..]; + assert!( + !declaration_context.contains("#[cfg_attr(coverage_nightly, coverage(off))]"), + "{helper} must remain inside owned coverage rather than bypass the 100% production gate" + ); + } +} diff --git a/crates/conceptweave-zotero/tests/private_artifact_parent_path_contract.rs b/crates/conceptweave-zotero/tests/private_artifact_parent_path_contract.rs new file mode 100644 index 0000000..bfb7839 --- /dev/null +++ b/crates/conceptweave-zotero/tests/private_artifact_parent_path_contract.rs @@ -0,0 +1,21 @@ +#[test] +fn private_artifact_open_does_not_reuse_the_untrusted_raw_path_after_parent_validation() { + let source = include_str!("../src/main.rs"); + let start = source + .find("fn read_private_json") + .expect("missing private artifact reader"); + let end = source[start..] + .find("fn open_with_metadata") + .map(|offset| start + offset) + .expect("missing private artifact open helper"); + let reader = &source[start..end]; + + assert!( + !reader.contains("fs::symlink_metadata(&path)"), + "path metadata must be checked through a path rebuilt from the validated canonical parent" + ); + assert!( + !reader.contains("open_with_metadata(&path)"), + "the opened path must be rebuilt from the validated canonical parent so a replaced parent symlink cannot redirect the read" + ); +} diff --git a/crates/conceptweave-zotero/tests/private_input_open_security.rs b/crates/conceptweave-zotero/tests/private_input_open_security.rs new file mode 100644 index 0000000..8544347 --- /dev/null +++ b/crates/conceptweave-zotero/tests/private_input_open_security.rs @@ -0,0 +1,78 @@ +#[cfg(unix)] +#[test] +fn checked_path_can_be_swapped_to_a_symlink_that_preserves_inode_identity() { + use std::fs; + use std::os::unix::fs::{MetadataExt, PermissionsExt, symlink}; + use std::time::{SystemTime, UNIX_EPOCH}; + + let nonce = SystemTime::now() + .duration_since(UNIX_EPOCH) + .expect("system clock must be after the Unix epoch") + .as_nanos(); + let original = std::env::temp_dir().join(format!( + "conceptweave-zotero-{}-{nonce}-checked.json", + std::process::id() + )); + let moved = std::env::temp_dir().join(format!( + "conceptweave-zotero-{}-{nonce}-moved.json", + std::process::id() + )); + + let _ = fs::remove_file(&original); + let _ = fs::remove_file(&moved); + fs::write(&original, br#"{"review":"private"}"#).unwrap(); + fs::set_permissions(&original, fs::Permissions::from_mode(0o600)).unwrap(); + + let checked = fs::symlink_metadata(&original).unwrap(); + assert!(!checked.file_type().is_symlink()); + fs::rename(&original, &moved).unwrap(); + symlink(&moved, &original).unwrap(); + + let followed = fs::File::open(&original).unwrap().metadata().unwrap(); + assert_eq!( + (checked.dev(), checked.ino()), + (followed.dev(), followed.ino()) + ); + assert_eq!(followed.nlink(), 1); + assert_eq!(followed.permissions().mode() & 0o777, 0o600); + + fs::remove_file(&original).unwrap(); + fs::remove_file(&moved).unwrap(); +} + +#[test] +fn owner_only_input_open_must_not_follow_final_component_symlinks() { + let source = include_str!("../src/main.rs"); + let open_start = source + .find("fn open_with_metadata") + .expect("private artifact open helper must remain present"); + let open_tail = &source[open_start..]; + let open_end = open_tail.find("\n#[cfg(unix)]").unwrap_or(open_tail.len()); + let open_body = &open_tail[..open_end]; + + assert!( + !open_body.contains("File::open(path)"), + "checked-path metadata followed by File::open is vulnerable to a final-component symlink swap" + ); + assert!( + source.contains("O_NOFOLLOW") + || source.contains("no_follow") + || source.contains("nofollow") + || source.contains("follow_links(false)"), + "the opened private-artifact handle must be obtained with an explicit no-follow primitive" + ); +} + +#[test] +fn no_follow_private_input_open_must_remain_inside_security_coverage() { + let source = include_str!("../src/main.rs"); + let open_start = source + .find("fn open_with_metadata") + .expect("private artifact open helper must remain present"); + let attribute_window = &source[open_start.saturating_sub(160)..open_start]; + + assert!( + !attribute_window.contains("coverage(off)"), + "the O_NOFOLLOW private-input security boundary must not be removed from owned coverage" + ); +} diff --git a/crates/conceptweave-zotero/tests/steward_review_progress.rs b/crates/conceptweave-zotero/tests/steward_review_progress.rs new file mode 100644 index 0000000..1af121e --- /dev/null +++ b/crates/conceptweave-zotero/tests/steward_review_progress.rs @@ -0,0 +1,185 @@ +use conceptweave_zotero::{ + Disposition, ItemData, WorksheetError, ZoteroItem, assess_steward_review_progress, + build_steward_review_worksheet, classify_snapshot, +}; + +fn classification_report() -> conceptweave_zotero::ClassificationReport { + classify_snapshot( + "9.0.6".into(), + None, + 42, + vec![ + ZoteroItem { + source_record: None, + key: "B".into(), + version: 8, + data: ItemData { + item_type: "book".into(), + title: "unknown vocabulary".into(), + abstract_note: "private review context".into(), + doi: String::new(), + parent_item: String::new(), + collections: vec!["PRIVATE_COLLECTION".into()], + tags: vec![], + }, + }, + ZoteroItem { + source_record: None, + key: "A".into(), + version: 7, + data: ItemData { + item_type: "journalArticle".into(), + title: "ontology learning".into(), + abstract_note: String::new(), + doi: "10.1000/private".into(), + parent_item: String::new(), + collections: vec![], + tags: vec![], + }, + }, + ], + ) +} + +#[test] +fn progress_is_exact_aggregate_only_and_fail_closed() { + let report = classification_report(); + let mut worksheet = build_steward_review_worksheet(&report).unwrap(); + + let blank = assess_steward_review_progress(&report, &worksheet).unwrap(); + assert_eq!(blank.total_count, 2); + assert_eq!(blank.decided_count, 0); + assert_eq!(blank.remaining_count, 2); + assert!(!blank.complete); + + worksheet.decisions[0].reviewed_disposition = Some(Disposition::Generation); + let partial = assess_steward_review_progress(&report, &worksheet).unwrap(); + assert_eq!(partial.decided_count, 1); + assert_eq!(partial.remaining_count, 1); + assert!(!partial.complete); + let serialized = serde_json::to_string(&partial).unwrap(); + assert!(!serialized.contains("PRIVATE")); + assert!(!serialized.contains("10.1000")); + assert!(!serialized.contains("item_key")); + assert!(!serialized.contains("reviewer")); + assert!(!serialized.contains("receipt")); + + worksheet.decisions[1].reviewed_disposition = Some(Disposition::OutOfScope); + let complete = assess_steward_review_progress(&report, &worksheet).unwrap(); + assert_eq!(complete.decided_count, 2); + assert_eq!(complete.remaining_count, 0); + assert!(complete.complete); + + let mut invalid = worksheet.clone(); + invalid.decisions[0].reviewed_disposition = Some(Disposition::NeedsStewardReview); + assert_eq!( + assess_steward_review_progress(&report, &invalid), + Err(WorksheetError::InvalidReport) + ); + + let mut tampered = worksheet.clone(); + tampered.decisions.swap(0, 1); + assert_eq!( + assess_steward_review_progress(&report, &tampered), + Err(WorksheetError::InvalidReport) + ); + + let mut missing = worksheet; + missing.decisions.pop(); + assert_eq!( + assess_steward_review_progress(&report, &missing), + Err(WorksheetError::InvalidReport) + ); + + let canonical = build_steward_review_worksheet(&report).unwrap(); + let mut invalid_report = classification_report(); + invalid_report.rule_revision.clear(); + assert_eq!( + assess_steward_review_progress(&invalid_report, &canonical), + Err(WorksheetError::InvalidReport) + ); + let mut shifted = canonical.clone(); + shifted.library_version += 1; + assert_eq!( + assess_steward_review_progress(&report, &shifted), + Err(WorksheetError::InvalidReport) + ); + let mut shifted = canonical.clone(); + shifted.rule_revision.push_str("-changed"); + assert_eq!( + assess_steward_review_progress(&report, &shifted), + Err(WorksheetError::InvalidReport) + ); + let mut shifted = canonical.clone(); + shifted.snapshot_digest.push_str("-changed"); + assert_eq!( + assess_steward_review_progress(&report, &shifted), + Err(WorksheetError::InvalidReport) + ); + let mut shifted = canonical; + shifted.snapshot_items.pop(); + assert_eq!( + assess_steward_review_progress(&report, &shifted), + Err(WorksheetError::InvalidReport) + ); + + let empty_report = classify_snapshot("9.0.6".into(), None, 42, vec![]); + let empty_worksheet = build_steward_review_worksheet(&empty_report).unwrap(); + let empty = assess_steward_review_progress(&empty_report, &empty_worksheet).unwrap(); + assert_eq!(empty.total_count, 0); + assert!(!empty.complete); +} + +#[test] +fn progress_rejects_stale_or_blank_content_binding() { + let mut report = classification_report(); + let worksheet = build_steward_review_worksheet(&report).unwrap(); + let mut blank = worksheet.clone(); + blank.proposal_digest.clear(); + assert_eq!( + assess_steward_review_progress(&report, &blank), + Err(WorksheetError::InvalidReport) + ); + report.classified_items[0] + .title + .push_str(" changed context"); + assert_eq!( + assess_steward_review_progress(&report, &worksheet), + Err(WorksheetError::InvalidReport) + ); +} + +#[test] +fn progress_preserves_pending_source_scope_and_opaque_identity() { + let source = ZoteroItem { + source_record: None, + key: "PRIVATE_SOURCE".into(), + version: 1, + data: ItemData { + item_type: "attachment".into(), + title: "private source".into(), + abstract_note: String::new(), + doi: String::new(), + parent_item: String::new(), + collections: vec![], + tags: vec![], + }, + }; + let paper: ZoteroItem = serde_json::from_value(serde_json::json!({ + "key": "PAPER", "version": 1, + "data": {"itemType": "book", "title": "ontology learning"} + })) + .unwrap(); + let report = classify_snapshot("9.0.6".into(), None, 42, vec![source, paper]); + let mut worksheet = build_steward_review_worksheet(&report).unwrap(); + worksheet.decisions[0].reviewed_disposition = Some(Disposition::Generation); + let progress = assess_steward_review_progress(&report, &worksheet).unwrap(); + let json = serde_json::to_value(&progress).unwrap(); + assert_eq!(json["pending_source_count"], 1); + assert_eq!(json["proposal_digest"], worksheet.proposal_digest); + assert!(!progress.complete); + assert_eq!(progress.total_count, 1); + assert_eq!(progress.decided_count, 1); + assert_eq!(progress.remaining_count, 0); + assert!(!json.to_string().contains("PRIVATE_SOURCE")); +} diff --git a/docs/PRD.md b/docs/PRD.md index 150f7c6..90e0c65 100644 --- a/docs/PRD.md +++ b/docs/PRD.md @@ -89,6 +89,7 @@ The Zotero 10+ adapter can accept a caller-owned API key and server identity at A local steward worksheet must bind the library version, rule revision, complete raw-snapshot digest, current proposal-and-retained-source digest, every observed parent/child item revision, and one blank decision slot per classified bibliographic item. It repeats item identity, proposal, and abstention reason only; titles, abstracts, tags, collections, and matched evidence remain in the separate sensitive report. Shared inventory validation rejects omitted source records, hidden pending relationships and inconsistent identity before construction. Valid unresolved sources do not prevent starting review, but prevent claiming completion. Old worksheets without the content binding require regeneration, never automatic approval backfill. After every decision is filled, worksheet finalization must verify the governance receipt coordinates, unique item identities and revisions, proposal/abstention consistency, and non-abstention truth labels before producing a reviewed golden set. Missing decisions remain incomplete and cannot reach external approval verification. Operators must be able to finalize the saved report, completed worksheet, and approval receipt offline without rereading mutable Zotero state. Every input must be a distinct owner-only file identity, not merely a differently spelled path, and the new golden-set output must use a separate path; invalid, oversized, linked, or shared inputs fail closed. +During the human review campaign, operators must be able to validate a partially completed worksheet against its original report and persist aggregate progress without an approval receipt. Progress binds current proposal and retained metadata identity and reports bibliographic total, decided and remaining counts alongside unresolved source count. Local preparation is complete only for a nonempty fully decided worksheet with no unresolved sources. Filled paper decisions do not hide pending attachments, notes or disconnected ancestry. Progress never claims correctness, independent approval, applied reclassification or publication authority. An empty campaign is not complete. The worksheet's own required content identity must match the current report independently of the supplied receipt. Blank identity is invalid; a stale or replaced identity is a snapshot mismatch. Conversion only prepares input for independent verification. Unresolved sources can remain in locally prepared review data, but prevent whole-library completion; refreshing local digests cannot renew an independently issued approval. Evaluate classifier quality only against a steward-reviewed local golden set whose governance receipt is externally verified and binds both the complete source/classifier-input snapshot and every current proposal field, in addition to the item-key/item-version coordinates. Same-version changes to unmodeled provider metadata, absent/default fields, classifier inputs, predictions or supporting evidence must invalidate the corresponding binding. Evaluation recomputes proposal identity before contacting governance; a locally changed digest cannot renew an approval. Legacy unbound approvals require reissuance, never automatic backfill. Abstention is a prediction outcome, never an approved truth label. Evaluation emits the verified library revision, rule revision, opaque snapshot and proposal digests, and aggregate counts for exact matches, abstentions, and per-disposition true-positive/predicted/expected totals; it must not copy Zotero keys, reviewer identity, or bibliographic text into the result. diff --git a/docs/TRD.md b/docs/TRD.md index 03e14cb..5452a79 100644 --- a/docs/TRD.md +++ b/docs/TRD.md @@ -116,6 +116,7 @@ The finalization function consumes the filled worksheet plus a governance approv It also rejects a blank worksheet `proposal_digest` as `InvalidReview` and compares that field with the freshly built expected worksheet as `SnapshotMismatch`, retaining existing cardinality and approval error precedence. The converter does not accept an updated receipt as proof that old decisions reviewed changed content. A caller can construct self-consistent unverified data, so the evaluator still authenticates the entire reviewed set against independent evidence. Pending sources are admitted for preparation but rejected by complete evaluation before governance is contacted. Later extracted worksheet validators must preserve this comparison. The owner-only report uses owned JSON values and supports lossless deserialization of its defined metadata projection, not the original provider record or full-text capture. Unknown provider fields and raw capture bytes are not serialized; retain captures separately. Repeated serialize/deserialize roundtrips preserve report bytes, the report-derived worksheet and proposal digest, allowing offline finalization against the stored snapshot identity rather than another live Zotero read. Shared report validation binds every retained key, version and parent coordinate; classified sources are top-level, while valid unresolved orphan and cyclic metadata remains pending. Deserialization establishes structure, not approval: changing serialized title or evidence under an unchanged source digest must still fail both finalization and evaluation against the original receipt before external verification. `conceptweave-zotero --finalize /tmp/REPORT.json /tmp/WORKSHEET.json /tmp/APPROVAL.json /tmp/GOLDEN.json` performs that offline transition. All four path arguments must differ, and the three opened inputs must also have distinct Unix device/inode identities so alternate spellings cannot collapse artifacts. Inputs must be regular, single-link files with exact `0600` permissions, no larger than 16 MiB, and direct children of a canonical system temporary directory; the golden set uses the existing create-new `0600` output boundary. Finalization reads no Zotero state and applies the existing report-bound worksheet and approval validation before writing. +`conceptweave-zotero --review-progress /tmp/REPORT.json /tmp/WORKSHEET.json /tmp/PROGRESS.json` reuses private artifact admission and canonical worksheet comparison. Its input paths and opened device/inode identities must differ. Blank decisions are accepted; missing, extra, reordered, shifted or tampered decisions fail. Shared comparison requires the recomputed proposal digest, rejecting blank or stale binding without backfill. Output contains library/rule/snapshot/proposal coordinates, bibliographic total/decided/remaining counts, `pending_source_count` and `complete`. Completion requires nonempty fully decided slots and zero recomputed pending sources. This is local preparation only; no authority verifier or Zotero call runs. Existing finalization error precedence remains unchanged. Offline input admission pins the checked canonical parent plus file name for metadata and opening. Unix opening refuses final-component symlinks with `O_NOFOLLOW` and uses `O_NONBLOCK` so a raced FIFO cannot wait for a writer before device/inode validation rejects the replacement; a pre-open pathname check alone is insufficient. Regular-file reads remain bounded as before. Paired export retains the shared canonical-destination check before capture and serializes both artifacts before writing. Failures may leave private partial files and never trigger pathname cleanup or implicit buffer-flush retry. Finalized metadata remains unverified until the independent whole-set approval boundary succeeds. diff --git a/docs/UML.md b/docs/UML.md index 7b3ac2b..8e6d744 100644 --- a/docs/UML.md +++ b/docs/UML.md @@ -59,6 +59,8 @@ sequenceDiagram Note over Report,Steward: Pending sources prevent a whole-library completion claim; inventory is not approval Intake->>Report: derive snapshot-bound decision worksheet without bibliographic text Report->>Steward: review dispositions and merge candidates + Steward->>Intake: save partially completed worksheet + Intake->>Report: validate original binding; emit aggregate progress only Steward->>Intake: completed worksheet + approval receipt Intake->>Report: offline finalization against the original saved report Report-->>Steward: reviewed golden set or fail-closed validation error diff --git a/docs/adr/0006-zotero-research-intake.md b/docs/adr/0006-zotero-research-intake.md index 3c4d72a..deacedd 100644 --- a/docs/adr/0006-zotero-research-intake.md +++ b/docs/adr/0006-zotero-research-intake.md @@ -44,6 +44,7 @@ Older worksheets require regeneration, not a fabricated digest or approval. The The CLI finalizes an original report, completed worksheet, and approval receipt into the reviewed golden set without another Zotero read. Each path argument and each opened input device/inode identity must be distinct. Inputs remain direct temporary-directory children, regular single-link files, exact owner-only `0600`, and bounded to 16 MiB; output retains create-new `0600` semantics. This keeps sensitive review material local and makes snapshot drift or aliased artifacts a validation failure instead of silently substituting current library state. +The same offline boundary may emit an aggregate progress checkpoint for a partial worksheet. The checkpoint revalidates every immutable coordinate and proposal field, counts only human-supplied non-abstention decisions, contains no item or reviewer identity, and treats zero required decisions as incomplete. It is operational coverage evidence, not an approval receipt or semantic-quality result. ### Proposed offline input continuity amendment — September 7 PR29 normal merge `4845200` retains offline finalization and inherits PR28 source binding plus report/worksheet preservation. Worksheet destinations reuse canonical-pair validation before capture; both artifacts serialize before either write, and second-write failure does not unlink the first artifact. This is sequential local output, not an atomic pair or approval issuance. @@ -119,6 +120,12 @@ GREEN source `f6735b585022aac1c8ceff86c150d9b64fd77ec2` passed 68 tests across 1 ## Alternatives considered +### Proposed review-progress scope amendment — September 7 + +PR30 exposes incremental local progress and extracts shared worksheet comparison. Normal merge `7251238` inherits repaired source identity, finalization and private input boundaries while preserving the child's coverage improvements. The extracted comparator omitted the required proposal digest, allowing old worksheet progress after changed content. RED `4099434` compiled with one passing and two failing tests: blank binding was admitted and pending/content identity fields were missing. `bee32f4` adds one shared digest comparison and includes the opaque proposal binding and pending count in existing aggregate output. `b2b0ef4` verifies one filled paper decision plus an unresolved source stays incomplete, preserving bibliographic counts and excluding the private source key. + +We reject snapshot-only identity because restored proposal and retained metadata can change under that coordinate. We reject treating nonbibliographic sources as paper decisions because their disposition is not inferred truth. We report both scopes, require zero pending sources for local completion, and retain separate approval and write gates. No new digest, source-text export or authority mechanism is added. Existing early finalization checks preserve error precedence. The stricter completion meaning corrects consumers that equated filled slots with complete source scope; later consumers must inherit it before claiming preparation completion. Protected review and release remain pending. + ### September 6 duplicate source-scope admission (Proposed) PR #12 already binds exact candidate membership and complete item revisions in `ReviewedDuplicateMergeSet`; the prior audit-owner concern about unbound duplicate authority therefore does not describe this consumer. Its real remaining gap was that retained metadata and inventory were absent from its receipt, and external verification preceded local decision checks. RED `4656d6b` reproduced missing legacy scope binding, malformed inventory accepted, and altered standalone evidence reaching governance. diff --git a/docs/product-technical-gap-baseline.md b/docs/product-technical-gap-baseline.md index 92a62a8..4b8c346 100644 --- a/docs/product-technical-gap-baseline.md +++ b/docs/product-technical-gap-baseline.md @@ -6,6 +6,14 @@ This file records code-current product and technical gaps. Exact PR/check/run co ## September 6 source inventory checkpoint +### September 7 PR30 content-bound progress repair + +Original PR30 `a11e889d1680ab4d91f3565e3debf7ed0f10ba23` passed 156 tests/32 suites. Normal merge `7251238` retains it and PR29 `e21f14fbb4954762ffc97521af9a6cdd9982c630`; integrated tests passed 200/32. Child private-helper coverage improvements remain, combined with parent FIFO/source/approval/output safety. RED `4099434` compiled with one passing and two failing progress tests: blank proposal binding was accepted and pending/content identity fields were absent. `bee32f4` compares the existing digest in the shared worksheet validator and adds opaque proposal identity plus pending count to aggregate progress. `b2b0ef4` proves filled bibliographic slots do not hide an unresolved standalone source. Independent bounded review found no collateral finalization error-precedence or privacy regression. + +Final source `b2b0ef4` passes 202 tests/32 suites including three doctests, strict Clippy, warnings-denied rustdoc, formatting, CI contract and diff checks. The unchanged coverage gate passes 328/328 reported functions, 2,905/2,905 normalized regions and 518/518 normalized branches. Raw 3,923/3,985 lines, 6,065/6,172 regions and 473/518 branches are not 100%. Logs: `/tmp/conceptweave-pr30-{baseline,integrated,progress-red,verified,clippy,rustdoc,coverage}.log`. PRD/TRD/Proposed ADR0006 explicitly distinguish bibliographic slot counts, pending source scope, local preparation, independent approval and applied reclassification. No new digest, dependency or authority issuer was added. + +Root and later consumers must inherit content binding, pending completion semantics and prior FIFO protection. Actual decisions and independent approvals remain 0/3,715 plus four unresolved standalone sources. Synthetic unit fixtures do not count as research review. Native Visual Inspection was retried, but the Mac is locked and no fresh screenshot exists. Keep Draft; no real Zotero write, hosted GREEN, protected merge or release is claimed. + ### September 7 PR29 offline finalization continuity repair Original PR29 `f73705e15f1236fa8bd34fec032bc78d9b57760c` passed 149 tests/28 suites. Normal merge `4845200` retains it and repaired PR28 `63eb0f116408372675f132b9836fe7be4bdd7134`; integrated tests passed 190/28. Offline finalization remains local unverified metadata output. The request enum retains parent canonical-pair admission before capture, both serializations before writes and no pathname cleanup on failure. Complete source/worksheet/approval binding comes from the inherited shared validation, not a new authority issuer.