From c691c3e052ae69d0a5e1436bbdeccb8d8e08afbf Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 03:12:14 +0900 Subject: [PATCH 01/22] test(zotero): require offline finalization CLI --- crates/conceptweave-zotero/src/main.rs | 36 ++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/crates/conceptweave-zotero/src/main.rs b/crates/conceptweave-zotero/src/main.rs index 102a879a..1a3080a8 100644 --- a/crates/conceptweave-zotero/src/main.rs +++ b/crates/conceptweave-zotero/src/main.rs @@ -183,11 +183,14 @@ mod tests { let worksheet = "/tmp/conceptweave-zotero-worksheet.json"; assert_eq!( parse_output_request(vec!["--worksheet", report, worksheet]), - Ok((Some(report.to_owned()), worksheet.to_owned())) + Ok(OutputRequest::Worksheet { + report: report.to_owned(), + worksheet: worksheet.to_owned(), + }) ); assert_eq!( parse_output_request(vec![report]), - Ok((None, report.to_owned())) + Ok(OutputRequest::Report(report.to_owned())) ); assert_eq!(parse_output_request(Vec::<&str>::new()), Err(USAGE)); assert!(parse_output_request(vec!["--worksheet"]).is_err()); @@ -196,6 +199,35 @@ mod tests { assert!(parse_output_request(vec!["--worksheet", report, report]).is_err()); } + #[test] + fn finalization_mode_requires_four_distinct_artifact_paths() { + let report = "/tmp/report.json"; + let worksheet = "/tmp/worksheet.json"; + let approval = "/tmp/approval.json"; + let output = "/tmp/golden.json"; + assert_eq!( + parse_output_request(vec![ + "--finalize", + report, + worksheet, + approval, + output, + ]), + Ok(OutputRequest::Finalize { + report: report.to_owned(), + worksheet: worksheet.to_owned(), + approval: approval.to_owned(), + output: output.to_owned(), + }) + ); + assert!( + parse_output_request(vec!["--finalize", report, worksheet, approval]).is_err() + ); + assert!( + parse_output_request(vec!["--finalize", report, worksheet, approval, report]).is_err() + ); + } + #[test] fn failed_private_output_is_removed_for_retry() { let output = unique_temp_path("failed-output"); From ffa115018f957842a7b9f102e5d975ded806b930 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 03:14:54 +0900 Subject: [PATCH 02/22] feat(zotero): finalize reviewed artifacts offline --- crates/conceptweave-zotero/src/main.rs | 271 +++++++++++++++++++++---- 1 file changed, 236 insertions(+), 35 deletions(-) diff --git a/crates/conceptweave-zotero/src/main.rs b/crates/conceptweave-zotero/src/main.rs index 1a3080a8..eccee646 100644 --- a/crates/conceptweave-zotero/src/main.rs +++ b/crates/conceptweave-zotero/src/main.rs @@ -1,15 +1,36 @@ #![forbid(unsafe_code)] #![cfg_attr(coverage_nightly, feature(coverage_attribute))] -use conceptweave_zotero::{build_steward_review_worksheet, read_local_snapshot}; +use conceptweave_zotero::{ + ClassificationReport, GoldenSetApproval, StewardReviewWorksheet, + build_steward_review_worksheet, read_local_snapshot, reviewed_golden_set_from_worksheet, +}; +use serde::de::DeserializeOwned; +use std::collections::BTreeSet; use std::env; use std::fs::{self, File, OpenOptions}; -use std::io::{self, BufWriter, Write}; +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"; +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 MAX_ARTIFACT_BYTES: u64 = 16 * 1024 * 1024; + +#[derive(Debug, PartialEq, Eq)] +enum OutputRequest { + Report(String), + Worksheet { + report: String, + worksheet: String, + }, + Finalize { + report: String, + worksheet: String, + approval: String, + output: String, + }, +} -fn parse_output_request(args: I) -> Result<(Option, String), &'static str> +fn parse_output_request(args: I) -> Result where I: IntoIterator, S: Into, @@ -26,9 +47,39 @@ where if report == worksheet { return Err("report and worksheet output paths must differ"); } - (Some(report), worksheet) + OutputRequest::Worksheet { report, worksheet } + } else if first == "--finalize" { + let report = args + .next() + .ok_or("--finalize requires four artifact paths")?; + let worksheet = args + .next() + .ok_or("--finalize requires four artifact paths")?; + let approval = args + .next() + .ok_or("--finalize requires four artifact paths")?; + let output = args + .next() + .ok_or("--finalize requires four artifact paths")?; + if BTreeSet::from([ + report.as_str(), + worksheet.as_str(), + approval.as_str(), + output.as_str(), + ]) + .len() + != 4 + { + return Err("finalization artifact paths must differ"); + } + OutputRequest::Finalize { + report, + worksheet, + approval, + output, + } } else { - (None, first) + OutputRequest::Report(first) }; if args.next().is_some() { return Err("unexpected extra argument"); @@ -36,6 +87,71 @@ where Ok(request) } +#[cfg_attr(coverage_nightly, coverage(off))] +fn read_private_json(raw: &str) -> io::Result { + let path = PathBuf::from(raw); + if !path.is_absolute() { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + "review input must be an absolute path in the system temp directory", + )); + } + let parent = path + .parent() + .ok_or_else(|| io::Error::new(io::ErrorKind::InvalidInput, "review input has no parent"))?; + if !allowed_output_parents().contains(&parent.canonicalize()?) { + return Err(io::Error::new( + io::ErrorKind::PermissionDenied, + "review input must be a direct child of the system temp directory", + )); + } + let path_metadata = fs::symlink_metadata(&path)?; + if path_metadata.file_type().is_symlink() || !path_metadata.is_file() { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + "review input must be a regular file", + )); + } + let file = File::open(&path)?; + let opened_metadata = file.metadata()?; + #[cfg(unix)] + { + use std::os::unix::fs::{MetadataExt, PermissionsExt}; + if opened_metadata.dev() != path_metadata.dev() + || opened_metadata.ino() != path_metadata.ino() + || opened_metadata.nlink() != 1 + || opened_metadata.permissions().mode() & 0o777 != 0o600 + { + return Err(io::Error::new( + io::ErrorKind::PermissionDenied, + "review input identity or permissions are unsafe", + )); + } + } + #[cfg(not(unix))] + return Err(io::Error::new( + io::ErrorKind::Unsupported, + "private review input requires a Unix platform", + )); + if opened_metadata.len() > MAX_ARTIFACT_BYTES { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + "review input exceeds the artifact size limit", + )); + } + let mut content = Vec::with_capacity(opened_metadata.len() as usize); + file.take(MAX_ARTIFACT_BYTES + 1) + .read_to_end(&mut content)?; + if content.len() as u64 > MAX_ARTIFACT_BYTES { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + "review input grew beyond the artifact size limit", + )); + } + serde_json::from_slice(&content) + .map_err(|error| io::Error::new(io::ErrorKind::InvalidData, error)) +} + fn write_private_output(path: &Path, content: &[u8]) -> io::Result<()> { write_private_output_with(path, content, write_all_and_flush) } @@ -148,27 +264,44 @@ fn create_report_file_with( #[cfg_attr(coverage_nightly, coverage(off))] /// Reads one Zotero snapshot and writes its sensitive local proposal report. fn main() -> Result<(), Box> { - let (report_output, output) = parse_output_request(env::args().skip(1))?; - let output = validate_output_path(&output)?; - let report_output = report_output - .as_deref() - .map(validate_output_path) - .transpose()?; - let report = read_local_snapshot()?; - if report.zotero_version.starts_with("9.") { - eprintln!("Zotero 9 Local API is read-only; writing local proposal output only"); - } - if let Some(report_output) = report_output { - let worksheet = build_steward_review_worksheet(&report)?; - let report_content = serde_json::to_vec_pretty(&report)?; - let worksheet_content = serde_json::to_vec_pretty(&worksheet)?; - write_private_output(&report_output, &report_content)?; - if let Err(error) = write_private_output(&output, &worksheet_content) { - let _ = fs::remove_file(report_output); - return Err(error.into()); + match parse_output_request(env::args().skip(1))? { + OutputRequest::Report(output) => { + let output = validate_output_path(&output)?; + let report = read_local_snapshot()?; + if report.zotero_version.starts_with("9.") { + eprintln!("Zotero 9 Local API is read-only; writing local proposal output only"); + } + write_private_output(&output, &serde_json::to_vec_pretty(&report)?)?; + } + OutputRequest::Worksheet { report, worksheet } => { + let report_output = validate_output_path(&report)?; + let worksheet_output = validate_output_path(&worksheet)?; + let report = read_local_snapshot()?; + if report.zotero_version.starts_with("9.") { + eprintln!("Zotero 9 Local API is read-only; writing local proposal output only"); + } + let worksheet = build_steward_review_worksheet(&report)?; + write_private_output(&report_output, &serde_json::to_vec_pretty(&report)?)?; + if let Err(error) = + write_private_output(&worksheet_output, &serde_json::to_vec_pretty(&worksheet)?) + { + let _ = fs::remove_file(report_output); + return Err(error.into()); + } + } + OutputRequest::Finalize { + report, + worksheet, + approval, + output, + } => { + let output = validate_output_path(&output)?; + let report: ClassificationReport = read_private_json(&report)?; + let worksheet: StewardReviewWorksheet = read_private_json(&worksheet)?; + let approval: GoldenSetApproval = read_private_json(&approval)?; + let golden = reviewed_golden_set_from_worksheet(&report, &worksheet, approval)?; + write_private_output(&output, &serde_json::to_vec_pretty(&golden)?)?; } - } else { - write_private_output(&output, &serde_json::to_vec_pretty(&report)?)?; } Ok(()) } @@ -206,13 +339,7 @@ mod tests { let approval = "/tmp/approval.json"; let output = "/tmp/golden.json"; assert_eq!( - parse_output_request(vec![ - "--finalize", - report, - worksheet, - approval, - output, - ]), + parse_output_request(vec!["--finalize", report, worksheet, approval, output,]), Ok(OutputRequest::Finalize { report: report.to_owned(), worksheet: worksheet.to_owned(), @@ -220,12 +347,86 @@ mod tests { output: output.to_owned(), }) ); + assert!(parse_output_request(vec!["--finalize"]).is_err()); + assert!(parse_output_request(vec!["--finalize", report]).is_err()); + assert!(parse_output_request(vec!["--finalize", report, worksheet]).is_err()); + assert!(parse_output_request(vec!["--finalize", report, worksheet, approval]).is_err()); assert!( - parse_output_request(vec!["--finalize", report, worksheet, approval]).is_err() + parse_output_request(vec!["--finalize", report, worksheet, approval, report]).is_err() ); assert!( - parse_output_request(vec!["--finalize", report, worksheet, approval, report]).is_err() + parse_output_request(vec![ + "--finalize", + report, + worksheet, + approval, + output, + "extra", + ]) + .is_err() + ); + } + + #[cfg(unix)] + #[test] + fn private_json_input_is_owner_only_regular_bounded_and_valid() { + use std::os::unix::fs::{PermissionsExt, symlink}; + + let valid = unique_temp_path("valid-input"); + let _ = fs::remove_file(&valid); + fs::write(&valid, br#"{"accepted":true}"#).unwrap(); + fs::set_permissions(&valid, fs::Permissions::from_mode(0o600)).unwrap(); + let parsed: serde_json::Value = read_private_json(valid.to_str().unwrap()).unwrap(); + assert_eq!(parsed["accepted"], true); + assert!(read_private_json::("relative.json").is_err()); + assert!(read_private_json::("/").is_err()); + assert!( + read_private_json::( + env::current_dir() + .unwrap() + .join("input.json") + .to_str() + .unwrap() + ) + .is_err() ); + + let directory = env::temp_dir().join(format!( + "conceptweave-zotero-{}-input-directory", + std::process::id() + )); + let _ = fs::remove_dir_all(&directory); + fs::create_dir(&directory).unwrap(); + assert!(read_private_json::(directory.to_str().unwrap()).is_err()); + fs::remove_dir(directory).unwrap(); + + let link = unique_temp_path("input-link"); + let _ = fs::remove_file(&link); + symlink(&valid, &link).unwrap(); + assert!(read_private_json::(link.to_str().unwrap()).is_err()); + fs::remove_file(link).unwrap(); + + fs::set_permissions(&valid, fs::Permissions::from_mode(0o644)).unwrap(); + assert!(read_private_json::(valid.to_str().unwrap()).is_err()); + fs::set_permissions(&valid, fs::Permissions::from_mode(0o600)).unwrap(); + + let hardlink = unique_temp_path("input-hardlink"); + let _ = fs::remove_file(&hardlink); + fs::hard_link(&valid, &hardlink).unwrap(); + assert!(read_private_json::(valid.to_str().unwrap()).is_err()); + fs::remove_file(hardlink).unwrap(); + + fs::write(&valid, b"not-json").unwrap(); + assert!(read_private_json::(valid.to_str().unwrap()).is_err()); + fs::remove_file(valid).unwrap(); + + let oversized = unique_temp_path("oversized-input"); + let _ = fs::remove_file(&oversized); + let file = File::create(&oversized).unwrap(); + file.set_len(MAX_ARTIFACT_BYTES + 1).unwrap(); + fs::set_permissions(&oversized, fs::Permissions::from_mode(0o600)).unwrap(); + assert!(read_private_json::(oversized.to_str().unwrap()).is_err()); + fs::remove_file(oversized).unwrap(); } #[test] From 6b8d20696c3e76926fb9ea233490fb90277c9a96 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 03:17:16 +0900 Subject: [PATCH 03/22] docs(zotero): record offline review finalization --- docs/PRD.md | 1 + docs/TRD.md | 1 + docs/UML.md | 4 +++- docs/adr/0006-zotero-research-intake.md | 2 ++ docs/product-technical-gap-baseline.md | 2 +- 5 files changed, 8 insertions(+), 2 deletions(-) diff --git a/docs/PRD.md b/docs/PRD.md index 62884c9f..d341c2cf 100644 --- a/docs/PRD.md +++ b/docs/PRD.md @@ -69,6 +69,7 @@ The Zotero 10+ adapter can accept a caller-owned API key and server identity at Evaluate classifier quality only against a steward-reviewed local golden set whose governance receipt is externally verified and bound to the canonical SHA-256 digest of the complete Zotero classification report plus its item-key/item-version coordinates. Abstention is a prediction outcome, never an approved truth label. Evaluation emits the verified library revision, rule revision, opaque snapshot digest, 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. A full-reclassification completion result additionally requires exactly one non-abstention steward label for every top-level bibliographic item; a sampled golden set remains valid for quality measurement but cannot prove completion. A local steward worksheet must bind the library version, rule revision, complete raw-snapshot digest, every observed parent/child item revision, and one editable 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. Invalid or duplicate report identity cannot produce a worksheet. 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 and the new golden-set output must use distinct owner-only local artifact paths; invalid, oversized, linked, or shared inputs fail closed. Every successful classification report includes aggregate evidence for snapshot coverage, proposal coverage, provenance completeness, abstentions, duplicate candidates, disposition totals, and zero unreported failures. diff --git a/docs/TRD.md b/docs/TRD.md index 741d1638..8662d1a1 100644 --- a/docs/TRD.md +++ b/docs/TRD.md @@ -71,6 +71,7 @@ A successful classification report carries an `audit_summary` whose snapshot, bi The review worksheet is a deterministic item-key-ordered projection of the report. `conceptweave-zotero --worksheet /tmp/REPORT.json /tmp/WORKSHEET.json` reads one live snapshot and creates both owner-only outputs as a bound pair; failed output removes partial files so the same paths can be retried. It binds the library version, rule revision, raw-snapshot digest, complete parent/child item coordinates, item proposal, abstention reason, and an initially empty decision for every bibliographic item. Construction rejects blank or duplicate snapshot identity, mismatched item revisions, and inconsistent observed, bibliographic, proposal, provenance, abstention, duplicate, failure, or disposition counts. It deliberately omits bibliographic text and matched evidence; stewards consult the paired owner-only report by item key. The finalization function consumes the filled worksheet plus a governance approval receipt. It rejects blank authority metadata, coordinate drift, duplicate or unknown decision keys, missing decisions, abstention as approved truth, and tampered proposal/reason pairs. Its output reuses the existing reviewed-golden-set evaluator; finalization itself does not verify external authority. The owner-only report uses owned JSON values and supports lossless deserialization. A serialize/deserialize roundtrip must preserve the report-derived worksheet exactly, allowing later offline finalization against the original snapshot rather than another live Zotero read. +`conceptweave-zotero --finalize /tmp/REPORT.json /tmp/WORKSHEET.json /tmp/APPROVAL.json /tmp/GOLDEN.json` performs that offline transition. All four paths must be distinct. 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. The local report can contain titles, tags, matched metadata, and abstention abstracts. It is sensitive steward-review material, remains outside the repository, and is not a publication artifact. diff --git a/docs/UML.md b/docs/UML.md index 8b14ad73..d1f177b4 100644 --- a/docs/UML.md +++ b/docs/UML.md @@ -56,7 +56,9 @@ sequenceDiagram Intake->>Report: write proposals and evidence Intake->>Report: derive snapshot-bound decision worksheet without bibliographic text Report->>Steward: review dispositions and merge candidates - Steward->>Intake: verified labels for every bibliographic item + 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 Intake-->>Steward: aggregate completion evidence or incomplete-review failure Steward->>Intake: verified canonical-item decisions Intake->>Report: before/after/rollback identity manifest diff --git a/docs/adr/0006-zotero-research-intake.md b/docs/adr/0006-zotero-research-intake.md index 4f58c9f6..9ed50109 100644 --- a/docs/adr/0006-zotero-research-intake.md +++ b/docs/adr/0006-zotero-research-intake.md @@ -28,6 +28,8 @@ To make full review executable without copying sensitive text again, ConceptWeav The report is a losslessly deserializable owner-only artifact. Evidence field names, matched phrases, and the rule revision use owned values so an offline process can reconstruct the exact canonical worksheet from the saved report; it must not reread a mutable Zotero library to finalize an earlier review. +The CLI finalizes an original report, completed worksheet, and approval receipt into the reviewed golden set without another Zotero read. Each artifact path 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 a validation failure instead of silently substituting current library state. + The reader fails closed above 50,000 items or 256 MiB of cumulative response bodies, while retaining the 8 MiB per-page bound, finite request timeouts, redirect denial, total-count checks, snapshot-version checks, and duplicate-key detection. Pagination, consistency, resource-budget, and provider-contract behavior are separated from the narrow `ureq` transport so deterministic tests exercise the production reader core rather than excluding the entire reader from coverage. Report output is restricted to a new direct child of the operating system temporary directory. Relative paths, nested paths, existing files, and symlinks are rejected before write; the file is opened with create-new semantics so a path swap cannot cause repository or arbitrary-file overwrite. The buffered writer is explicitly flushed and a final filesystem error fails the command. Reports stay local and are never committed. diff --git a/docs/product-technical-gap-baseline.md b/docs/product-technical-gap-baseline.md index c5cca9c6..68f52dc5 100644 --- a/docs/product-technical-gap-baseline.md +++ b/docs/product-technical-gap-baseline.md @@ -54,7 +54,7 @@ The steward workload now has a deterministic local worksheet contract rather tha Filled worksheets now have a fail-closed conversion into the existing reviewed golden-set contract. The conversion checks exact approval coordinates, complete unique decision identity, snapshot membership and item revision, proposal/abstention consistency, and rejects missing or abstention truth labels before the external authority verifier can run. The live completion KPI remains 0/3,715 until real steward decisions and an externally verified approval receipt exist. -The paired owner-only report is now losslessly deserializable: its rule revision and evidence vocabulary use owned JSON values, and roundtrip verification proves the report reconstructs the same canonical worksheet. This removes the need to reread a mutable Zotero library when the completed worksheet is later finalized. A CLI import/finalization command remains the next operator-facing Gap. +The paired owner-only report is losslessly deserializable: its rule revision and evidence vocabulary use owned JSON values, and roundtrip verification proves the report reconstructs the same canonical worksheet. The CLI now finalizes the original report, completed worksheet, and approval receipt offline into a create-new owner-only golden set without rereading mutable Zotero state. It requires four distinct paths and rejects non-temporary, linked, non-regular, non-`0600`, or over-16 MiB inputs before the report-bound validation runs. The next measurable Gap is the real steward-labeling campaign itself: 3,715 valid decisions and an externally verified receipt are still absent, so the completion KPI remains 0/3,715. The completion KPI is now executable: a sampled golden set may measure classifier quality, but it cannot prove that the library was fully reclassified. The full-review boundary returns aggregate completion evidence only when externally verified, snapshot-bound, non-abstention labels cover every unique classified bibliographic item exactly once. The current live baseline therefore remains incomplete at 0/3,715 approved labels rather than treating 57 deterministic proposals or a future sample as steward truth. From 72db3e46dd36e160e226c4ff6abeb76bce46ecdc Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 03:28:43 +0900 Subject: [PATCH 04/22] test(zotero): reject aliased finalization inputs --- .../tests/finalization_artifact_identity.rs | 95 +++++++++++++++++++ 1 file changed, 95 insertions(+) create mode 100644 crates/conceptweave-zotero/tests/finalization_artifact_identity.rs diff --git a/crates/conceptweave-zotero/tests/finalization_artifact_identity.rs b/crates/conceptweave-zotero/tests/finalization_artifact_identity.rs new file mode 100644 index 00000000..7ba485ad --- /dev/null +++ b/crates/conceptweave-zotero/tests/finalization_artifact_identity.rs @@ -0,0 +1,95 @@ +#![cfg(unix)] + +use conceptweave_zotero::{ + Disposition, GoldenSetApproval, ItemData, ZoteroItem, build_steward_review_worksheet, + classify_snapshot, +}; +use std::fs; +use std::os::unix::fs::PermissionsExt; +use std::process::Command; + +#[test] +fn finalization_rejects_distinct_path_spellings_for_one_input_file() { + let item = ZoteroItem { + key: "ITEM".into(), + version: 7, + data: ItemData { + item_type: "journalArticle".into(), + title: "ontology alignment".into(), + abstract_note: "review context".into(), + doi: "10.1000/example".into(), + parent_item: String::new(), + collections: vec!["COLLECTION".into()], + tags: vec![], + }, + }; + let report = classify_snapshot("9.0.6".into(), None, 42, vec![item]); + let mut worksheet = build_steward_review_worksheet(&report).unwrap(); + for decision in &mut worksheet.decisions { + decision.reviewed_disposition = Some(Disposition::OutOfScope); + } + let approval = GoldenSetApproval { + receipt_id: "receipt_1".into(), + reviewer_subject: "steward_1".into(), + library_version: worksheet.library_version, + rule_revision: worksheet.rule_revision.clone(), + snapshot_digest: worksheet.snapshot_digest.clone(), + snapshot_items: worksheet.snapshot_items.clone(), + }; + + // These structs intentionally accept additional owner-only fields. Without checking file + // identity, one JSON object can therefore be accepted as all three finalization inputs. + let mut combined = serde_json::to_value(&report).unwrap(); + let object = combined.as_object_mut().unwrap(); + object.insert( + "decisions".into(), + serde_json::to_value(&worksheet.decisions).unwrap(), + ); + object.insert( + "receipt_id".into(), + serde_json::to_value(&approval.receipt_id).unwrap(), + ); + object.insert( + "reviewer_subject".into(), + serde_json::to_value(&approval.reviewer_subject).unwrap(), + ); + + let temp = std::env::temp_dir().canonicalize().unwrap(); + let filename = format!( + "conceptweave-zotero-finalize-alias-{}-input.json", + std::process::id() + ); + let input = temp.join(&filename); + let output = temp.join(format!( + "conceptweave-zotero-finalize-alias-{}-output.json", + std::process::id() + )); + let _ = fs::remove_file(&input); + let _ = fs::remove_file(&output); + fs::write(&input, serde_json::to_vec(&combined).unwrap()).unwrap(); + fs::set_permissions(&input, fs::Permissions::from_mode(0o600)).unwrap(); + + let report_path = input.to_str().unwrap().to_owned(); + let worksheet_path = format!("{}/./{}", temp.display(), filename); + let approval_path = format!("{}//{}", temp.display(), filename); + assert_ne!(report_path, worksheet_path); + assert_ne!(worksheet_path, approval_path); + + let status = Command::new(env!("CARGO_BIN_EXE_conceptweave-zotero")) + .args([ + "--finalize", + &report_path, + &worksheet_path, + &approval_path, + output.to_str().unwrap(), + ]) + .status() + .unwrap(); + + let _ = fs::remove_file(&input); + let _ = fs::remove_file(&output); + assert!( + !status.success(), + "finalization must reject three path spellings that resolve to one input artifact" + ); +} From b8a3cdba998e5cceac964276c160e6de65d4c46c Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 03:33:56 +0900 Subject: [PATCH 05/22] fix(zotero): bind distinct finalization artifacts --- crates/conceptweave-zotero/src/main.rs | 89 +++++++++++++++++--------- 1 file changed, 60 insertions(+), 29 deletions(-) diff --git a/crates/conceptweave-zotero/src/main.rs b/crates/conceptweave-zotero/src/main.rs index eccee646..2a94d5fa 100644 --- a/crates/conceptweave-zotero/src/main.rs +++ b/crates/conceptweave-zotero/src/main.rs @@ -87,8 +87,13 @@ where Ok(request) } -#[cfg_attr(coverage_nightly, coverage(off))] -fn read_private_json(raw: &str) -> io::Result { +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] +struct ArtifactIdentity { + device: u64, + inode: u64, +} + +fn read_private_json(raw: &str) -> io::Result<(T, ArtifactIdentity)> { let path = PathBuf::from(raw); if !path.is_absolute() { return Err(io::Error::new( @@ -114,8 +119,16 @@ fn read_private_json(raw: &str) -> io::Result { } let file = File::open(&path)?; let opened_metadata = file.metadata()?; - #[cfg(unix)] + #[cfg(not(unix))] { + let _ = (path_metadata, opened_metadata, file); + return Err(io::Error::new( + io::ErrorKind::Unsupported, + "private review input requires a Unix platform", + )); + } + #[cfg(unix)] + let identity = { use std::os::unix::fs::{MetadataExt, PermissionsExt}; if opened_metadata.dev() != path_metadata.dev() || opened_metadata.ino() != path_metadata.ino() @@ -127,29 +140,36 @@ fn read_private_json(raw: &str) -> io::Result { "review input identity or permissions are unsafe", )); } + ArtifactIdentity { + device: opened_metadata.dev(), + inode: opened_metadata.ino(), + } + }; + #[cfg(unix)] + { + if opened_metadata.len() > MAX_ARTIFACT_BYTES { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + "review input exceeds the artifact size limit", + )); + } + let mut content = Vec::with_capacity(opened_metadata.len() as usize); + file.take(MAX_ARTIFACT_BYTES + 1) + .read_to_end(&mut content)?; + if content.len() as u64 > MAX_ARTIFACT_BYTES { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + "review input grew beyond the artifact size limit", + )); + } + let parsed = serde_json::from_slice(&content) + .map_err(|error| io::Error::new(io::ErrorKind::InvalidData, error))?; + Ok((parsed, identity)) } - #[cfg(not(unix))] - return Err(io::Error::new( - io::ErrorKind::Unsupported, - "private review input requires a Unix platform", - )); - if opened_metadata.len() > MAX_ARTIFACT_BYTES { - return Err(io::Error::new( - io::ErrorKind::InvalidData, - "review input exceeds the artifact size limit", - )); - } - let mut content = Vec::with_capacity(opened_metadata.len() as usize); - file.take(MAX_ARTIFACT_BYTES + 1) - .read_to_end(&mut content)?; - if content.len() as u64 > MAX_ARTIFACT_BYTES { - return Err(io::Error::new( - io::ErrorKind::InvalidData, - "review input grew beyond the artifact size limit", - )); - } - serde_json::from_slice(&content) - .map_err(|error| io::Error::new(io::ErrorKind::InvalidData, error)) +} + +fn label_input(name: &str, error: io::Error) -> io::Error { + io::Error::new(error.kind(), format!("{name}: {error}")) } fn write_private_output(path: &Path, content: &[u8]) -> io::Result<()> { @@ -296,9 +316,19 @@ fn main() -> Result<(), Box> { output, } => { let output = validate_output_path(&output)?; - let report: ClassificationReport = read_private_json(&report)?; - let worksheet: StewardReviewWorksheet = read_private_json(&worksheet)?; - let approval: GoldenSetApproval = read_private_json(&approval)?; + 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))?; + let (approval, approval_identity): (GoldenSetApproval, _) = + read_private_json(&approval).map_err(|error| label_input("approval", error))?; + if BTreeSet::from([report_identity, worksheet_identity, approval_identity]).len() != 3 { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + "finalization inputs must be distinct files", + ) + .into()); + } let golden = reviewed_golden_set_from_worksheet(&report, &worksheet, approval)?; write_private_output(&output, &serde_json::to_vec_pretty(&golden)?)?; } @@ -376,7 +406,8 @@ mod tests { let _ = fs::remove_file(&valid); fs::write(&valid, br#"{"accepted":true}"#).unwrap(); fs::set_permissions(&valid, fs::Permissions::from_mode(0o600)).unwrap(); - let parsed: serde_json::Value = read_private_json(valid.to_str().unwrap()).unwrap(); + let (parsed, _): (serde_json::Value, _) = + read_private_json(valid.to_str().unwrap()).unwrap(); assert_eq!(parsed["accepted"], true); assert!(read_private_json::("relative.json").is_err()); assert!(read_private_json::("/").is_err()); From 5299219c1049701b80d9e9f743499b6d7adaeae3 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 03:35:39 +0900 Subject: [PATCH 06/22] test(zotero): cover private artifact validation --- crates/conceptweave-zotero/src/main.rs | 157 +++++++++++++++++++------ 1 file changed, 121 insertions(+), 36 deletions(-) diff --git a/crates/conceptweave-zotero/src/main.rs b/crates/conceptweave-zotero/src/main.rs index 2a94d5fa..931c69ab 100644 --- a/crates/conceptweave-zotero/src/main.rs +++ b/crates/conceptweave-zotero/src/main.rs @@ -117,8 +117,7 @@ fn read_private_json(raw: &str) -> io::Result<(T, ArtifactI "review input must be a regular file", )); } - let file = File::open(&path)?; - let opened_metadata = file.metadata()?; + let (file, opened_metadata) = open_with_metadata(&path)?; #[cfg(not(unix))] { let _ = (path_metadata, opened_metadata, file); @@ -128,46 +127,67 @@ fn read_private_json(raw: &str) -> io::Result<(T, ArtifactI )); } #[cfg(unix)] - let identity = { - use std::os::unix::fs::{MetadataExt, PermissionsExt}; - if opened_metadata.dev() != path_metadata.dev() - || opened_metadata.ino() != path_metadata.ino() - || opened_metadata.nlink() != 1 - || opened_metadata.permissions().mode() & 0o777 != 0o600 - { - return Err(io::Error::new( - io::ErrorKind::PermissionDenied, - "review input identity or permissions are unsafe", - )); - } - ArtifactIdentity { - device: opened_metadata.dev(), - inode: opened_metadata.ino(), - } - }; + let identity = validate_opened_identity(&path_metadata, &opened_metadata)?; #[cfg(unix)] { - if opened_metadata.len() > MAX_ARTIFACT_BYTES { - return Err(io::Error::new( - io::ErrorKind::InvalidData, - "review input exceeds the artifact size limit", - )); - } - let mut content = Vec::with_capacity(opened_metadata.len() as usize); - file.take(MAX_ARTIFACT_BYTES + 1) - .read_to_end(&mut content)?; - if content.len() as u64 > MAX_ARTIFACT_BYTES { - return Err(io::Error::new( - io::ErrorKind::InvalidData, - "review input grew beyond the artifact size limit", - )); - } - let parsed = serde_json::from_slice(&content) - .map_err(|error| io::Error::new(io::ErrorKind::InvalidData, error))?; + let parsed = read_bounded_json(&mut { file }, opened_metadata.len())?; Ok((parsed, identity)) } } +#[cfg_attr(coverage_nightly, coverage(off))] +fn open_with_metadata(path: &Path) -> io::Result<(File, fs::Metadata)> { + let file = File::open(path)?; + let metadata = file.metadata()?; + Ok((file, metadata)) +} + +#[cfg(unix)] +fn validate_opened_identity( + path_metadata: &fs::Metadata, + opened_metadata: &fs::Metadata, +) -> io::Result { + use std::os::unix::fs::{MetadataExt, PermissionsExt}; + + if (opened_metadata.dev(), opened_metadata.ino()) != (path_metadata.dev(), path_metadata.ino()) + || opened_metadata.nlink() != 1 + || opened_metadata.permissions().mode() & 0o777 != 0o600 + { + return Err(io::Error::new( + io::ErrorKind::PermissionDenied, + "review input identity or permissions are unsafe", + )); + } + Ok(ArtifactIdentity { + device: opened_metadata.dev(), + inode: opened_metadata.ino(), + }) +} + +fn read_bounded_json( + reader: &mut dyn Read, + advertised_len: u64, +) -> io::Result { + if advertised_len > MAX_ARTIFACT_BYTES { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + "review input exceeds the artifact size limit", + )); + } + let mut content = Vec::with_capacity(advertised_len as usize); + reader + .take(MAX_ARTIFACT_BYTES + 1) + .read_to_end(&mut content)?; + if content.len() as u64 > MAX_ARTIFACT_BYTES { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + "review input grew beyond the artifact size limit", + )); + } + serde_json::from_slice(&content) + .map_err(|error| io::Error::new(io::ErrorKind::InvalidData, error)) +} + fn label_input(name: &str, error: io::Error) -> io::Error { io::Error::new(error.kind(), format!("{name}: {error}")) } @@ -406,11 +426,27 @@ mod tests { let _ = fs::remove_file(&valid); fs::write(&valid, br#"{"accepted":true}"#).unwrap(); fs::set_permissions(&valid, fs::Permissions::from_mode(0o600)).unwrap(); + + fs::set_permissions(&valid, fs::Permissions::from_mode(0o000)).unwrap(); + assert!(read_private_json::(valid.to_str().unwrap()).is_err()); + fs::set_permissions(&valid, fs::Permissions::from_mode(0o600)).unwrap(); let (parsed, _): (serde_json::Value, _) = read_private_json(valid.to_str().unwrap()).unwrap(); assert_eq!(parsed["accepted"], true); assert!(read_private_json::("relative.json").is_err()); assert!(read_private_json::("/").is_err()); + assert!( + read_private_json::( + unique_temp_path("missing-input").to_str().unwrap() + ) + .is_err() + ); + assert!( + read_private_json::( + "/tmp/conceptweave-zotero-missing-directory/input.json" + ) + .is_err() + ); assert!( read_private_json::( env::current_dir() @@ -458,6 +494,55 @@ mod tests { fs::set_permissions(&oversized, fs::Permissions::from_mode(0o600)).unwrap(); assert!(read_private_json::(oversized.to_str().unwrap()).is_err()); fs::remove_file(oversized).unwrap(); + + let identity_left = unique_temp_path("identity-left"); + let identity_right = unique_temp_path("identity-right"); + fs::write(&identity_left, b"{}").unwrap(); + fs::write(&identity_right, b"{}").unwrap(); + fs::set_permissions(&identity_left, fs::Permissions::from_mode(0o600)).unwrap(); + fs::set_permissions(&identity_right, fs::Permissions::from_mode(0o600)).unwrap(); + assert!( + validate_opened_identity( + &fs::metadata(&identity_left).unwrap(), + &fs::metadata(&identity_right).unwrap() + ) + .is_err() + ); + fs::remove_file(identity_left).unwrap(); + fs::remove_file(identity_right).unwrap(); + } + + #[test] + fn bounded_json_reader_and_input_labels_cover_failures() { + struct FailingReader; + impl Read for FailingReader { + fn read(&mut self, _: &mut [u8]) -> io::Result { + Err(io::Error::other("injected read failure")) + } + } + + let oversized = + read_bounded_json::(&mut io::empty(), MAX_ARTIFACT_BYTES + 1) + .unwrap_err(); + assert_eq!(oversized.kind(), io::ErrorKind::InvalidData); + + let grown = read_bounded_json::( + &mut io::repeat(b' ').take(MAX_ARTIFACT_BYTES + 1), + 0, + ) + .unwrap_err(); + assert_eq!(grown.kind(), io::ErrorKind::InvalidData); + + let read_failure = + read_bounded_json::(&mut FailingReader, 0).unwrap_err(); + assert_eq!(read_failure.kind(), io::ErrorKind::Other); + + let labeled = label_input( + "worksheet", + io::Error::new(io::ErrorKind::PermissionDenied, "unsafe"), + ); + assert_eq!(labeled.kind(), io::ErrorKind::PermissionDenied); + assert_eq!(labeled.to_string(), "worksheet: unsafe"); } #[test] From 89e1dffda95e4582c438fb919a5beba01d0ecfaf Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 03:37:36 +0900 Subject: [PATCH 07/22] docs(zotero): bind finalization file identity --- docs/PRD.md | 2 +- docs/TRD.md | 2 +- docs/adr/0006-zotero-research-intake.md | 2 +- docs/product-technical-gap-baseline.md | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/PRD.md b/docs/PRD.md index d341c2cf..7962564f 100644 --- a/docs/PRD.md +++ b/docs/PRD.md @@ -69,7 +69,7 @@ The Zotero 10+ adapter can accept a caller-owned API key and server identity at Evaluate classifier quality only against a steward-reviewed local golden set whose governance receipt is externally verified and bound to the canonical SHA-256 digest of the complete Zotero classification report plus its item-key/item-version coordinates. Abstention is a prediction outcome, never an approved truth label. Evaluation emits the verified library revision, rule revision, opaque snapshot digest, 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. A full-reclassification completion result additionally requires exactly one non-abstention steward label for every top-level bibliographic item; a sampled golden set remains valid for quality measurement but cannot prove completion. A local steward worksheet must bind the library version, rule revision, complete raw-snapshot digest, every observed parent/child item revision, and one editable 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. Invalid or duplicate report identity cannot produce a worksheet. 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 and the new golden-set output must use distinct owner-only local artifact paths; invalid, oversized, linked, or shared inputs fail closed. +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. Every successful classification report includes aggregate evidence for snapshot coverage, proposal coverage, provenance completeness, abstentions, duplicate candidates, disposition totals, and zero unreported failures. diff --git a/docs/TRD.md b/docs/TRD.md index 8662d1a1..1c4cba60 100644 --- a/docs/TRD.md +++ b/docs/TRD.md @@ -71,7 +71,7 @@ A successful classification report carries an `audit_summary` whose snapshot, bi The review worksheet is a deterministic item-key-ordered projection of the report. `conceptweave-zotero --worksheet /tmp/REPORT.json /tmp/WORKSHEET.json` reads one live snapshot and creates both owner-only outputs as a bound pair; failed output removes partial files so the same paths can be retried. It binds the library version, rule revision, raw-snapshot digest, complete parent/child item coordinates, item proposal, abstention reason, and an initially empty decision for every bibliographic item. Construction rejects blank or duplicate snapshot identity, mismatched item revisions, and inconsistent observed, bibliographic, proposal, provenance, abstention, duplicate, failure, or disposition counts. It deliberately omits bibliographic text and matched evidence; stewards consult the paired owner-only report by item key. The finalization function consumes the filled worksheet plus a governance approval receipt. It rejects blank authority metadata, coordinate drift, duplicate or unknown decision keys, missing decisions, abstention as approved truth, and tampered proposal/reason pairs. Its output reuses the existing reviewed-golden-set evaluator; finalization itself does not verify external authority. The owner-only report uses owned JSON values and supports lossless deserialization. A serialize/deserialize roundtrip must preserve the report-derived worksheet exactly, allowing later offline finalization against the original snapshot rather than another live Zotero read. -`conceptweave-zotero --finalize /tmp/REPORT.json /tmp/WORKSHEET.json /tmp/APPROVAL.json /tmp/GOLDEN.json` performs that offline transition. All four paths must be distinct. 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 --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. The local report can contain titles, tags, matched metadata, and abstention abstracts. It is sensitive steward-review material, remains outside the repository, and is not a publication artifact. diff --git a/docs/adr/0006-zotero-research-intake.md b/docs/adr/0006-zotero-research-intake.md index 9ed50109..6698f328 100644 --- a/docs/adr/0006-zotero-research-intake.md +++ b/docs/adr/0006-zotero-research-intake.md @@ -28,7 +28,7 @@ To make full review executable without copying sensitive text again, ConceptWeav The report is a losslessly deserializable owner-only artifact. Evidence field names, matched phrases, and the rule revision use owned values so an offline process can reconstruct the exact canonical worksheet from the saved report; it must not reread a mutable Zotero library to finalize an earlier review. -The CLI finalizes an original report, completed worksheet, and approval receipt into the reviewed golden set without another Zotero read. Each artifact path 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 a validation failure instead of silently substituting current library state. +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 reader fails closed above 50,000 items or 256 MiB of cumulative response bodies, while retaining the 8 MiB per-page bound, finite request timeouts, redirect denial, total-count checks, snapshot-version checks, and duplicate-key detection. Pagination, consistency, resource-budget, and provider-contract behavior are separated from the narrow `ureq` transport so deterministic tests exercise the production reader core rather than excluding the entire reader from coverage. diff --git a/docs/product-technical-gap-baseline.md b/docs/product-technical-gap-baseline.md index 68f52dc5..26b3f4fc 100644 --- a/docs/product-technical-gap-baseline.md +++ b/docs/product-technical-gap-baseline.md @@ -54,7 +54,7 @@ The steward workload now has a deterministic local worksheet contract rather tha Filled worksheets now have a fail-closed conversion into the existing reviewed golden-set contract. The conversion checks exact approval coordinates, complete unique decision identity, snapshot membership and item revision, proposal/abstention consistency, and rejects missing or abstention truth labels before the external authority verifier can run. The live completion KPI remains 0/3,715 until real steward decisions and an externally verified approval receipt exist. -The paired owner-only report is losslessly deserializable: its rule revision and evidence vocabulary use owned JSON values, and roundtrip verification proves the report reconstructs the same canonical worksheet. The CLI now finalizes the original report, completed worksheet, and approval receipt offline into a create-new owner-only golden set without rereading mutable Zotero state. It requires four distinct paths and rejects non-temporary, linked, non-regular, non-`0600`, or over-16 MiB inputs before the report-bound validation runs. The next measurable Gap is the real steward-labeling campaign itself: 3,715 valid decisions and an externally verified receipt are still absent, so the completion KPI remains 0/3,715. +The paired owner-only report is losslessly deserializable: its rule revision and evidence vocabulary use owned JSON values, and roundtrip verification proves the report reconstructs the same canonical worksheet. The CLI now finalizes the original report, completed worksheet, and approval receipt offline into a create-new owner-only golden set without rereading mutable Zotero state. It requires four distinct path arguments, proves the three opened inputs have distinct Unix device/inode identities, and rejects non-temporary, linked, non-regular, non-`0600`, or over-16 MiB inputs before the report-bound validation runs. The next measurable Gap is the real steward-labeling campaign itself: 3,715 valid decisions and an externally verified receipt are still absent, so the completion KPI remains 0/3,715. The completion KPI is now executable: a sampled golden set may measure classifier quality, but it cannot prove that the library was fully reclassified. The full-review boundary returns aggregate completion evidence only when externally verified, snapshot-bound, non-abstention labels cover every unique classified bibliographic item exactly once. The current live baseline therefore remains incomplete at 0/3,715 approved labels rather than treating 57 deterministic proposals or a future sample as steward truth. From 276e2a4031458f291d34d0c4f51c71b56f762639 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 03:44:36 +0900 Subject: [PATCH 08/22] docs(zotero): document private artifact functions --- crates/conceptweave-zotero/src/main.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/crates/conceptweave-zotero/src/main.rs b/crates/conceptweave-zotero/src/main.rs index 931c69ab..5cb3e7da 100644 --- a/crates/conceptweave-zotero/src/main.rs +++ b/crates/conceptweave-zotero/src/main.rs @@ -30,6 +30,7 @@ enum OutputRequest { }, } +/// Parses one mutually exclusive report, worksheet, or finalization request. fn parse_output_request(args: I) -> Result where I: IntoIterator, @@ -93,6 +94,7 @@ struct ArtifactIdentity { inode: u64, } +/// Opens, validates, bounds, and deserializes one owner-only review artifact. fn read_private_json(raw: &str) -> io::Result<(T, ArtifactIdentity)> { let path = PathBuf::from(raw); if !path.is_absolute() { @@ -136,6 +138,7 @@ fn read_private_json(raw: &str) -> io::Result<(T, ArtifactI } #[cfg_attr(coverage_nightly, coverage(off))] +/// Opens a review input once and returns metadata from the opened handle. fn open_with_metadata(path: &Path) -> io::Result<(File, fs::Metadata)> { let file = File::open(path)?; let metadata = file.metadata()?; @@ -143,6 +146,7 @@ fn open_with_metadata(path: &Path) -> io::Result<(File, fs::Metadata)> { } #[cfg(unix)] +/// Proves the opened Unix file matches the checked path and owner-only contract. fn validate_opened_identity( path_metadata: &fs::Metadata, opened_metadata: &fs::Metadata, @@ -164,6 +168,7 @@ fn validate_opened_identity( }) } +/// Reads JSON without allowing the input to exceed or grow past the artifact limit. fn read_bounded_json( reader: &mut dyn Read, advertised_len: u64, @@ -188,20 +193,24 @@ fn read_bounded_json( .map_err(|error| io::Error::new(io::ErrorKind::InvalidData, error)) } +/// Preserves an input error kind while naming the rejected artifact. fn label_input(name: &str, error: io::Error) -> io::Error { io::Error::new(error.kind(), format!("{name}: {error}")) } +/// Writes one create-new owner-only artifact and removes a failed partial write. 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)?; writer.flush() } +/// Runs the private-output boundary with an injectable writer for failure testing. fn write_private_output_with( path: &Path, content: &[u8], From 00e328515b70b9317bb135ba90894b45c44037a4 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 16:05:57 +0900 Subject: [PATCH 09/22] test(research): bind synthetic offline approval to proposals Signed-off-by: Seongho Bae --- .../tests/finalization_artifact_identity.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/crates/conceptweave-zotero/tests/finalization_artifact_identity.rs b/crates/conceptweave-zotero/tests/finalization_artifact_identity.rs index 7ba485ad..75de1ef4 100644 --- a/crates/conceptweave-zotero/tests/finalization_artifact_identity.rs +++ b/crates/conceptweave-zotero/tests/finalization_artifact_identity.rs @@ -11,6 +11,7 @@ use std::process::Command; #[test] fn finalization_rejects_distinct_path_spellings_for_one_input_file() { let item = ZoteroItem { + source_record: None, key: "ITEM".into(), version: 7, data: ItemData { @@ -34,6 +35,7 @@ fn finalization_rejects_distinct_path_spellings_for_one_input_file() { library_version: worksheet.library_version, rule_revision: worksheet.rule_revision.clone(), snapshot_digest: worksheet.snapshot_digest.clone(), + proposal_digest: conceptweave_zotero::classification_proposal_digest(&report), snapshot_items: worksheet.snapshot_items.clone(), }; @@ -53,6 +55,10 @@ fn finalization_rejects_distinct_path_spellings_for_one_input_file() { "reviewer_subject".into(), serde_json::to_value(&approval.reviewer_subject).unwrap(), ); + object.insert( + "proposal_digest".into(), + serde_json::to_value(&approval.proposal_digest).unwrap(), + ); let temp = std::env::temp_dir().canonicalize().unwrap(); let filename = format!( From 4c0c8f0f879c6bc4826fce27e3e27149b9272285 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 21:45:25 +0900 Subject: [PATCH 10/22] test(zotero): reproduce original private JSON diagnostic disclosure --- crates/conceptweave-zotero/src/main.rs | 41 +++++ .../tests/private_json_diagnostics_cli.rs | 146 ++++++++++++++++++ 2 files changed, 187 insertions(+) create mode 100644 crates/conceptweave-zotero/tests/private_json_diagnostics_cli.rs diff --git a/crates/conceptweave-zotero/src/main.rs b/crates/conceptweave-zotero/src/main.rs index 5cb3e7da..953557ec 100644 --- a/crates/conceptweave-zotero/src/main.rs +++ b/crates/conceptweave-zotero/src/main.rs @@ -534,6 +534,10 @@ mod tests { read_bounded_json::(&mut io::empty(), MAX_ARTIFACT_BYTES + 1) .unwrap_err(); assert_eq!(oversized.kind(), io::ErrorKind::InvalidData); + assert_eq!( + oversized.to_string(), + "review input exceeds the artifact size limit" + ); let grown = read_bounded_json::( &mut io::repeat(b' ').take(MAX_ARTIFACT_BYTES + 1), @@ -541,10 +545,15 @@ mod tests { ) .unwrap_err(); assert_eq!(grown.kind(), io::ErrorKind::InvalidData); + assert_eq!( + grown.to_string(), + "review input grew beyond the artifact size limit" + ); let read_failure = read_bounded_json::(&mut FailingReader, 0).unwrap_err(); assert_eq!(read_failure.kind(), io::ErrorKind::Other); + assert_eq!(read_failure.to_string(), "injected read failure"); let labeled = label_input( "worksheet", @@ -554,6 +563,38 @@ mod tests { assert_eq!(labeled.to_string(), "worksheet: unsafe"); } + #[test] + fn private_json_diagnostic_hides_unknown_field_names() { + #[derive(Debug, serde::Deserialize)] + #[serde(deny_unknown_fields)] + struct StrictReviewInput {} + + // Original metadata types permit additional fields. This strict test-only type + // exercises the shared reader's confidentiality contract for future callers. + let error = read_bounded_json::( + &mut br#"{"synthetic-private-field-sentinel":true}"#.as_slice(), + 0, + ) + .unwrap_err(); + assert_eq!(error.kind(), io::ErrorKind::InvalidData); + assert_eq!(error.to_string(), "review input is invalid"); + assert_eq!( + label_input("worksheet", error).to_string(), + "worksheet: review input is invalid" + ); + } + + #[test] + fn private_json_diagnostic_hides_rejected_enum_values() { + let error = read_bounded_json::( + &mut br#""synthetic-private-enum-sentinel""#.as_slice(), + 0, + ) + .unwrap_err(); + assert_eq!(error.kind(), io::ErrorKind::InvalidData); + assert_eq!(error.to_string(), "review input is invalid"); + } + #[test] fn failed_private_output_is_removed_for_retry() { let output = unique_temp_path("failed-output"); diff --git a/crates/conceptweave-zotero/tests/private_json_diagnostics_cli.rs b/crates/conceptweave-zotero/tests/private_json_diagnostics_cli.rs new file mode 100644 index 00000000..6a25e028 --- /dev/null +++ b/crates/conceptweave-zotero/tests/private_json_diagnostics_cli.rs @@ -0,0 +1,146 @@ +#![cfg(unix)] + +use conceptweave_zotero::{ + Disposition, GoldenSetApproval, ItemData, ReviewedGoldenSet, ZoteroItem, + build_steward_review_worksheet, classification_proposal_digest, classify_snapshot, +}; +use std::fs::{self, OpenOptions}; +use std::io::Write; +use std::os::unix::fs::{OpenOptionsExt, PermissionsExt}; +use std::path::PathBuf; +use std::process::{Command, Output}; + +/// Owns only synthetic, uniquely named direct-temp-child artifacts for one CLI case. +struct FinalizationFiles { + paths: [PathBuf; 4], + inputs: [Vec; 3], +} + +impl FinalizationFiles { + fn new(case_name: &str, mutate: impl FnOnce(&mut [serde_json::Value; 3])) -> Self { + let report = classify_snapshot( + "9.0.6".into(), + None, + 42, + vec![ZoteroItem { + source_record: None, + key: "SYNTHETIC".into(), + version: 7, + data: ItemData { + item_type: "journalArticle".into(), + title: "synthetic ontology alignment".into(), + abstract_note: String::new(), + doi: String::new(), + parent_item: String::new(), + collections: vec![], + tags: vec![], + }, + }], + ); + let mut worksheet = build_steward_review_worksheet(&report).unwrap(); + worksheet.decisions[0].reviewed_disposition = Some(Disposition::AlignmentVersioning); + let approval = GoldenSetApproval { + receipt_id: "synthetic-receipt".into(), + reviewer_subject: "synthetic-steward".into(), + library_version: worksheet.library_version, + rule_revision: worksheet.rule_revision.clone(), + snapshot_digest: worksheet.snapshot_digest.clone(), + proposal_digest: classification_proposal_digest(&report), + snapshot_items: worksheet.snapshot_items.clone(), + }; + let mut values = [ + serde_json::to_value(report).unwrap(), + serde_json::to_value(worksheet).unwrap(), + serde_json::to_value(approval).unwrap(), + ]; + mutate(&mut values); + let inputs = values.map(|value| serde_json::to_vec(&value).unwrap()); + let paths = ["report", "worksheet", "approval", "golden"].map(|role| { + std::env::temp_dir().join(format!( + "conceptweave-pr29-diagnostics-{}-{case_name}-{role}.json", + std::process::id() + )) + }); + assert!(!paths[3].exists()); + for (path, bytes) in paths.iter().zip(&inputs) { + let mut file = OpenOptions::new() + .write(true) + .create_new(true) + .mode(0o600) + .open(path) + .unwrap(); + file.set_permissions(fs::Permissions::from_mode(0o600)) + .unwrap(); + file.write_all(bytes).unwrap(); + } + Self { paths, inputs } + } + + fn run(&self) -> Output { + Command::new(env!("CARGO_BIN_EXE_conceptweave-zotero")) + .arg("--finalize") + .args(&self.paths) + .output() + .unwrap() + } + + fn assert_private_rejection(&self, role: &str) { + let output = self.run(); + assert!(!output.status.success()); + assert!(output.stdout.is_empty()); + assert!(!self.paths[3].exists()); + for (path, original) in self.paths.iter().zip(&self.inputs) { + assert_eq!(&fs::read(path).unwrap(), original); + } + let stderr = String::from_utf8(output.stderr).unwrap(); + assert!( + stderr.contains(&format!("{role}: review input is invalid")), + "{stderr}" + ); + assert!(!stderr.contains("synthetic-private"), "{stderr}"); + } +} + +impl Drop for FinalizationFiles { + fn drop(&mut self) { + for path in &self.paths { + let _ = fs::remove_file(path); + } + } +} + +#[test] +fn finalization_keeps_valid_synthetic_inputs_and_private_output_compatible() { + let files = FinalizationFiles::new("valid", |_| {}); + let output = files.run(); + assert!(output.status.success(), "{:?}", output.stderr); + let golden: ReviewedGoldenSet = + serde_json::from_slice(&fs::read(&files.paths[3]).unwrap()).unwrap(); + let approval: GoldenSetApproval = serde_json::from_slice(&files.inputs[2]).unwrap(); + assert_eq!(golden.approval, approval); + assert_eq!( + golden.labels[0].expected_disposition, + Disposition::AlignmentVersioning + ); + assert_eq!( + fs::metadata(&files.paths[3]).unwrap().permissions().mode() & 0o777, + 0o600 + ); +} + +#[test] +fn finalization_diagnostic_hides_private_worksheet_enum_value() { + let files = FinalizationFiles::new("enum", |values| { + values[1]["decisions"][0]["reviewed_disposition"] = + serde_json::json!("synthetic-private-enum-sentinel"); + }); + files.assert_private_rejection("worksheet"); +} + +#[test] +fn finalization_diagnostic_hides_private_approval_scalar_value() { + let files = FinalizationFiles::new("scalar", |values| { + values[2]["library_version"] = serde_json::json!("synthetic-private-scalar-sentinel"); + }); + files.assert_private_rejection("approval"); +} From 25d4a780c2195411151964b133f38cd47b29384c Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 21:46:13 +0900 Subject: [PATCH 11/22] fix(zotero): keep private JSON parse diagnostics source-free --- crates/conceptweave-zotero/src/main.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/conceptweave-zotero/src/main.rs b/crates/conceptweave-zotero/src/main.rs index 953557ec..8c2465cf 100644 --- a/crates/conceptweave-zotero/src/main.rs +++ b/crates/conceptweave-zotero/src/main.rs @@ -168,7 +168,7 @@ fn validate_opened_identity( }) } -/// Reads JSON without allowing the input to exceed or grow past the artifact limit. +/// Reads bounded JSON without exposing rejected field names or values in diagnostics. fn read_bounded_json( reader: &mut dyn Read, advertised_len: u64, @@ -190,7 +190,7 @@ fn read_bounded_json( )); } serde_json::from_slice(&content) - .map_err(|error| io::Error::new(io::ErrorKind::InvalidData, error)) + .map_err(|_| io::Error::new(io::ErrorKind::InvalidData, "review input is invalid")) } /// Preserves an input error kind while naming the rejected artifact. From cdf8e1232c409cc9d90bc442fb41586384877d89 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 21:47:02 +0900 Subject: [PATCH 12/22] test(zotero): reproduce original metadata output size gap --- crates/conceptweave-zotero/src/main.rs | 50 ++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/crates/conceptweave-zotero/src/main.rs b/crates/conceptweave-zotero/src/main.rs index 8c2465cf..28e889f0 100644 --- a/crates/conceptweave-zotero/src/main.rs +++ b/crates/conceptweave-zotero/src/main.rs @@ -595,6 +595,56 @@ mod tests { assert_eq!(error.to_string(), "review input is invalid"); } + #[cfg(unix)] + #[test] + fn private_output_limit_accepts_exactly_the_readable_artifact_boundary() { + use std::os::unix::fs::PermissionsExt; + + let output = unique_temp_path("exact-output-limit"); + assert!(!output.exists()); + let mut content = vec![b' '; MAX_ARTIFACT_BYTES as usize]; + content[..2].copy_from_slice(b"{}"); + write_private_output(&output, &content).unwrap(); + let metadata = fs::metadata(&output).unwrap(); + let restored = read_private_json::(output.to_str().unwrap()); + fs::remove_file(output).unwrap(); + assert_eq!(metadata.len(), MAX_ARTIFACT_BYTES); + assert_eq!(metadata.permissions().mode() & 0o777, 0o600); + assert_eq!(restored.unwrap().0, serde_json::json!({})); + } + + #[cfg(unix)] + #[test] + fn private_output_limit_rejects_oversize_before_creating_or_touching_a_file() { + let output = unique_temp_path("oversized-output-limit"); + let existing = unique_temp_path("existing-output-limit"); + assert!(!output.exists()); + assert!(!existing.exists()); + write_private_output(&existing, b"original").unwrap(); + let content = vec![b' '; MAX_ARTIFACT_BYTES as usize + 1]; + let rejected = write_private_output(&output, &content); + let touched = write_private_output(&existing, &content); + let created = output.exists(); + let preserved = fs::read(&existing).unwrap(); + // Clean synthetic artifacts even when the RED implementation creates the file. + if created { + fs::remove_file(output).unwrap(); + } + fs::remove_file(existing).unwrap(); + assert!( + !created, + "oversized metadata must be rejected before creation" + ); + assert_eq!(preserved, b"original"); + for error in [rejected.unwrap_err(), touched.unwrap_err()] { + assert_eq!(error.kind(), io::ErrorKind::InvalidData); + assert_eq!( + error.to_string(), + "metadata output exceeds the artifact size limit" + ); + } + } + #[test] fn failed_private_output_is_removed_for_retry() { let output = unique_temp_path("failed-output"); From 0837c6f7ee051d674a99a5132cdcf8beb759a81e Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 21:47:27 +0900 Subject: [PATCH 13/22] fix(zotero): bound metadata output before private file creation --- crates/conceptweave-zotero/src/main.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/crates/conceptweave-zotero/src/main.rs b/crates/conceptweave-zotero/src/main.rs index 28e889f0..41fc3ee5 100644 --- a/crates/conceptweave-zotero/src/main.rs +++ b/crates/conceptweave-zotero/src/main.rs @@ -198,8 +198,14 @@ fn label_input(name: &str, error: io::Error) -> io::Error { io::Error::new(error.kind(), format!("{name}: {error}")) } -/// Writes one create-new owner-only artifact and removes a failed partial write. +/// Writes one bounded create-new owner-only artifact and removes a failed partial write. fn write_private_output(path: &Path, content: &[u8]) -> io::Result<()> { + if content.len() as u64 > MAX_ARTIFACT_BYTES { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + "metadata output exceeds the artifact size limit", + )); + } write_private_output_with(path, content, write_all_and_flush) } From 79facfc875dfe72c78680d98fce89b813bc216a1 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 21:49:59 +0900 Subject: [PATCH 14/22] test(zotero): preserve owned rule revision in original receipt fixture Restore the clone-only intent of 1ca8a798b6a183c3dd823f416e396d6a4bb6149a. PR #28 merge 1623aadcb3680490c477b263681c6aa066dc3b5c retained an unnecessary into() after clone(); current PR #29 inherits that strict-Clippy failure. Later descendants already keep the clone-only expression. No receipt evidence or behavior changes. --- .../tests/classification_write_receipt_contract.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/conceptweave-zotero/tests/classification_write_receipt_contract.rs b/crates/conceptweave-zotero/tests/classification_write_receipt_contract.rs index fcb97d15..d35f844e 100644 --- a/crates/conceptweave-zotero/tests/classification_write_receipt_contract.rs +++ b/crates/conceptweave-zotero/tests/classification_write_receipt_contract.rs @@ -56,7 +56,7 @@ fn reviewed(report: &conceptweave_zotero::ClassificationReport) -> ReviewedClass server_id: report.server_id.clone(), zotero_version: report.zotero_version.clone(), library_version: report.library_version, - rule_revision: report.rule_revision.clone().into(), + rule_revision: report.rule_revision.clone(), snapshot_digest: report.snapshot_digest.clone(), snapshot_items: report.snapshot_items.clone(), changes: vec![ From fec97acb06b55bcb9c1e7dfe9d2d942bf0f5e9d2 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 21:55:50 +0900 Subject: [PATCH 15/22] test(zotero): simplify synthetic overflow artifact cleanup --- crates/conceptweave-zotero/src/main.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/conceptweave-zotero/src/main.rs b/crates/conceptweave-zotero/src/main.rs index 41fc3ee5..69ede19d 100644 --- a/crates/conceptweave-zotero/src/main.rs +++ b/crates/conceptweave-zotero/src/main.rs @@ -633,9 +633,7 @@ mod tests { let created = output.exists(); let preserved = fs::read(&existing).unwrap(); // Clean synthetic artifacts even when the RED implementation creates the file. - if created { - fs::remove_file(output).unwrap(); - } + let _ = fs::remove_file(output); fs::remove_file(existing).unwrap(); assert!( !created, From 7c2c0d19bfc8a63a4babf5bd4c0d0d956831dad9 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 7 Sep 2026 00:33:36 +0900 Subject: [PATCH 16/22] test(zotero): expose offline artifact opening gaps --- crates/conceptweave-zotero/src/main.rs | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/crates/conceptweave-zotero/src/main.rs b/crates/conceptweave-zotero/src/main.rs index 3712325d..782dff93 100644 --- a/crates/conceptweave-zotero/src/main.rs +++ b/crates/conceptweave-zotero/src/main.rs @@ -390,6 +390,28 @@ fn main() -> Result<(), Box> { mod tests { use super::*; + #[test] + #[cfg(unix)] + fn opening_private_input_refuses_a_symlink_before_reading() { + use std::os::unix::fs::symlink; + let target = unique_temp_path("open-target"); + let link = unique_temp_path("open-symlink"); + write_private_output(&target, b"{}").unwrap(); + symlink(&target, &link).unwrap(); + let result = open_with_metadata(&link); + fs::remove_file(&link).unwrap(); + fs::remove_file(&target).unwrap(); + assert!(result.is_err()); + } + + #[test] + fn private_input_rejects_a_nameless_child_of_the_validated_parent() { + let path = env::temp_dir().join(".."); + let error = read_private_json::(path.to_str().unwrap()).unwrap_err(); + assert_eq!(error.kind(), io::ErrorKind::InvalidInput); + assert_eq!(error.to_string(), "review input has no file name"); + } + #[test] fn worksheet_mode_is_explicit_and_rejects_ambiguous_arguments() { let report = "/tmp/conceptweave-zotero-report.json"; From f97ef462fd30d9d6640f47ddcd46ab8a679b137f Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 05:05:20 +0900 Subject: [PATCH 17/22] fix(zotero): pin validated private artifact parent (cherry picked from commit b06f54a53f457ee002dffccdb6f18e66fef0c3c7) --- crates/conceptweave-zotero/src/main.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/crates/conceptweave-zotero/src/main.rs b/crates/conceptweave-zotero/src/main.rs index 782dff93..0434a0a7 100644 --- a/crates/conceptweave-zotero/src/main.rs +++ b/crates/conceptweave-zotero/src/main.rs @@ -106,20 +106,25 @@ fn read_private_json(raw: &str) -> io::Result<(T, ArtifactI let parent = path .parent() .ok_or_else(|| io::Error::new(io::ErrorKind::InvalidInput, "review input has no parent"))?; - if !allowed_output_parents().contains(&parent.canonicalize()?) { + let resolved_parent = parent.canonicalize()?; + if !allowed_output_parents().contains(&resolved_parent) { return Err(io::Error::new( io::ErrorKind::PermissionDenied, "review input must be a direct child of the system temp directory", )); } - let path_metadata = fs::symlink_metadata(&path)?; + let file_name = path.file_name().ok_or_else(|| { + io::Error::new(io::ErrorKind::InvalidInput, "review input has no file name") + })?; + let validated_path = resolved_parent.join(file_name); + let path_metadata = fs::symlink_metadata(&validated_path)?; if path_metadata.file_type().is_symlink() || !path_metadata.is_file() { return Err(io::Error::new( io::ErrorKind::InvalidInput, "review input must be a regular file", )); } - let (file, opened_metadata) = open_with_metadata(&path)?; + let (file, opened_metadata) = open_with_metadata(&validated_path)?; #[cfg(not(unix))] { let _ = (path_metadata, opened_metadata, file); From 630d258b17811e56a02c6e3aff27d22d1d908090 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 04:18:11 +0900 Subject: [PATCH 18/22] fix(zotero): refuse symlink artifact opens (cherry picked from commit 7ccbbbe6857113d3d9fb5fd102ce9c9839b7975b) --- Cargo.lock | 1 + crates/conceptweave-zotero/Cargo.toml | 1 + crates/conceptweave-zotero/src/main.rs | 9 ++++++++- 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index ad534722..5a7db392 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -37,6 +37,7 @@ version = "0.1.0" name = "conceptweave-zotero" version = "0.1.0" dependencies = [ + "libc", "serde", "serde_json", "sha2", diff --git a/crates/conceptweave-zotero/Cargo.toml b/crates/conceptweave-zotero/Cargo.toml index 3f492bbe..11eb58ca 100644 --- a/crates/conceptweave-zotero/Cargo.toml +++ b/crates/conceptweave-zotero/Cargo.toml @@ -8,6 +8,7 @@ license.workspace = true description = "Read-only Zotero research classification for ConceptWeave" [dependencies] +libc = "0.2" serde = { version = "1", features = ["derive"] } serde_json = "1" sha2 = "0.10" diff --git a/crates/conceptweave-zotero/src/main.rs b/crates/conceptweave-zotero/src/main.rs index 0434a0a7..ee68d60b 100644 --- a/crates/conceptweave-zotero/src/main.rs +++ b/crates/conceptweave-zotero/src/main.rs @@ -145,7 +145,14 @@ fn read_private_json(raw: &str) -> io::Result<(T, ArtifactI #[cfg_attr(coverage_nightly, coverage(off))] /// Opens a review input once and returns metadata from the opened handle. fn open_with_metadata(path: &Path) -> io::Result<(File, fs::Metadata)> { - let file = File::open(path)?; + #[cfg(unix)] + use std::os::unix::fs::OpenOptionsExt; + + let mut options = OpenOptions::new(); + options.read(true); + #[cfg(unix)] + options.custom_flags(libc::O_NOFOLLOW); + let file = options.open(path)?; let metadata = file.metadata()?; Ok((file, metadata)) } From bce7efa2587bce058b4205a287bacdce32351014 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 7 Sep 2026 00:36:13 +0900 Subject: [PATCH 19/22] test(zotero): reproduce blocking raced FIFO input --- crates/conceptweave-zotero/src/main.rs | 35 ++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/crates/conceptweave-zotero/src/main.rs b/crates/conceptweave-zotero/src/main.rs index ee68d60b..97b34b0d 100644 --- a/crates/conceptweave-zotero/src/main.rs +++ b/crates/conceptweave-zotero/src/main.rs @@ -424,6 +424,41 @@ mod tests { assert_eq!(error.to_string(), "review input has no file name"); } + #[test] + #[cfg(unix)] + fn replaced_fifo_does_not_block_before_identity_rejection() { + let target = unique_temp_path("fifo-replacement"); + let retained = unique_temp_path("fifo-retained"); + write_private_output(&target, b"{}").unwrap(); + let checked_metadata = fs::symlink_metadata(&target).unwrap(); + fs::rename(&target, &retained).unwrap(); + assert!( + std::process::Command::new("mkfifo") + .arg(&target) + .status() + .unwrap() + .success() + ); + let (sender, receiver) = std::sync::mpsc::channel(); + let opened_path = target.clone(); + std::thread::spawn(move || { + let result = open_with_metadata(&opened_path).map(|(_, metadata)| metadata); + let _ = sender.send(result); + }); + let result = receiver.recv_timeout(std::time::Duration::from_secs(2)); + fs::remove_file(&target).unwrap(); + fs::remove_file(&retained).unwrap(); + let opened_metadata = result + .expect("opening a raced FIFO must not wait for a writer") + .unwrap(); + assert_eq!( + validate_opened_identity(&checked_metadata, &opened_metadata) + .unwrap_err() + .kind(), + io::ErrorKind::PermissionDenied + ); + } + #[test] fn worksheet_mode_is_explicit_and_rejects_ambiguous_arguments() { let report = "/tmp/conceptweave-zotero-report.json"; From ddf3f62413db4b6314ece54cc75b1f1daef1d1fc Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 7 Sep 2026 00:36:45 +0900 Subject: [PATCH 20/22] fix(zotero): refuse blocking artifact opens --- crates/conceptweave-zotero/src/main.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/conceptweave-zotero/src/main.rs b/crates/conceptweave-zotero/src/main.rs index 97b34b0d..c12bdcd5 100644 --- a/crates/conceptweave-zotero/src/main.rs +++ b/crates/conceptweave-zotero/src/main.rs @@ -143,7 +143,7 @@ fn read_private_json(raw: &str) -> io::Result<(T, ArtifactI } #[cfg_attr(coverage_nightly, coverage(off))] -/// Opens a review input once and returns metadata from the opened handle. +/// 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)] use std::os::unix::fs::OpenOptionsExt; @@ -151,7 +151,7 @@ fn open_with_metadata(path: &Path) -> io::Result<(File, fs::Metadata)> { let mut options = OpenOptions::new(); options.read(true); #[cfg(unix)] - options.custom_flags(libc::O_NOFOLLOW); + options.custom_flags(libc::O_NOFOLLOW | libc::O_NONBLOCK); let file = options.open(path)?; let metadata = file.metadata()?; Ok((file, metadata)) From ebe9663410af5708f4471b03ce603cd35da13c43 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 7 Sep 2026 00:37:14 +0900 Subject: [PATCH 21/22] docs(zotero): trace offline artifact opening repairs --- docs/TRD.md | 2 ++ docs/adr/0006-zotero-research-intake.md | 10 ++++++++++ 2 files changed, 12 insertions(+) diff --git a/docs/TRD.md b/docs/TRD.md index db4510cb..03e14cb2 100644 --- a/docs/TRD.md +++ b/docs/TRD.md @@ -117,6 +117,8 @@ It also rejects a blank worksheet `proposal_digest` as `InvalidReview` and compa 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. +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. + The local report can contain titles, tags, matched metadata, and abstention abstracts. It is sensitive steward-review material, remains outside the repository, and is not a publication artifact. The report is local JSON and contains proposals rather than governance decisions. On supported Unix platforms, CLI output is restricted to a new owner-readable/writable (`0600`) direct child of canonical `/tmp` or the operating system temporary directory; exact permissions are restored after umask application, and other platforms fail closed. Relative paths, nested paths, existing paths, and symlinks are rejected, and create-new file semantics prevent overwrite/path-swap writes. Reviewed collection/tag changes can produce a pure local plan whose default mode is dry-run. The plan requires exact report and item preconditions, complete before/after/rollback arrays, externally verified authority, and preserved Zotero tag types; its fields are externally read-only after validation. Zotero 9 execute mode fails closed. Every receipt copies the plan's review, authority, server, Zotero version, library, rule, snapshot and proposal coordinates; dry-run reports every operation as not attempted and makes no Local API call. Execute mode preflights every item before the first write, advances the library precondition only from a directly verified write response, stops on the first adapter or response failure, and re-reads that item through the same boundary as observation only. Failed writes remain indeterminate regardless of observed metadata; no inverse is issued for them. Prior directly verified operations retain their inverse coordinates. The API key remains adapter-owned and absent from serializable structures. diff --git a/docs/adr/0006-zotero-research-intake.md b/docs/adr/0006-zotero-research-intake.md index 16f3cec8..3c4d72a9 100644 --- a/docs/adr/0006-zotero-research-intake.md +++ b/docs/adr/0006-zotero-research-intake.md @@ -44,6 +44,16 @@ 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. +### 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. + +The original reader canonicalized a parent for admission but subsequently opened the original pathname; the open helper also followed final-component symlinks before checking device/inode. RED `7c2c0d1` proves that direct symlink opening succeeds and records the existing nameless-input error precedence; the latter is a validation-path regression, not proof of unauthorized data access. We reuse canonical owner fixes `b06f54a53f457ee002dffccdb6f18e66fef0c3c7` and `7ccbbbe6857113d3d9fb5fd102ce9c9839b7975b` as `f97ef46` and `630d258`, preserving their attribution. The reader uses the checked canonical parent plus file name for metadata and opening, and Unix opening uses `O_NOFOLLOW` before handle-based identity checks. The existing locked libc dependency supplies the native flag; no replacement filesystem abstraction is added. + +We reject relying only on a pre-open symlink check because a pathname can change between inspection and opening. We reject copying the later full-text CLI into this earlier owner: this command finalizes metadata only and grants neither full-text decision provenance nor external approval nor Zotero write authority. OS open failure remains an error; existing private-mode, single-link, byte-limit and source-binding checks remain required. These changes remain Proposed until exact-head protected checks, independent approval and release. + +The follow-up premortem identified a separate FIFO replacement: `O_NOFOLLOW` does not prevent an open from waiting for a pipe writer before identity validation. No existing owner fix was found. RED `bce7efa` replaces a checked regular unit-test artifact with an actual FIFO and observes a two-second test timeout. `ddf3f62` adds the existing native `O_NONBLOCK` flag at the same open boundary; the open returns without a writer and existing device/inode validation rejects the replacement. This is a filesystem-open safety rule, not a model/application timeout. Regular-file private reads and size limits remain unchanged. Root and later extracted private readers must inherit it; do not transfer this head's test evidence to them. + The reader fails closed above 50,000 items or 256 MiB of cumulative response bodies, while retaining the 8 MiB per-page bound, finite request timeouts, redirect denial, total-count checks, snapshot-version checks, and duplicate-key detection. Pagination, consistency, resource-budget, and provider-contract behavior are separated from the narrow `ureq` transport so deterministic tests exercise the production reader core rather than excluding the entire reader from coverage. In the context of reading every bibliographic source before classification, facing individually timely pages that can cumulatively hold a run open for days, we decided for a five-minute monotonic admission/completion budget in the existing reader and against rejecting legitimate short pages or adding another transport, to bound accepted work without excluding papers, accepting that an already-started request or classification computation can finish after the limit before its result is rejected. This is an application read limit, not a model timeout, hard process-cancellation deadline, wall-clock/suspend guarantee or atomic snapshot claim. Each page is checked before fetch and after return, and the complete report is checked before return. The stdlib clock has a private deterministic test seam; public APIs, provider timeouts and data/byte ceilings are unchanged. The [deadline doctoring](../doctoring/zotero_metadata_deadline.md) records the original review, RED/GREEN, alternatives and exact verification. This amendment remains Proposed and grants no Zotero mutation authority. From e21f14fbb4954762ffc97521af9a6cdd9982c630 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 7 Sep 2026 00:38:12 +0900 Subject: [PATCH 22/22] docs(zotero): record offline finalization boundary evidence --- docs/product-technical-gap-baseline.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/docs/product-technical-gap-baseline.md b/docs/product-technical-gap-baseline.md index a695640e..92a62a8a 100644 --- a/docs/product-technical-gap-baseline.md +++ b/docs/product-technical-gap-baseline.md @@ -6,6 +6,16 @@ This file records code-current product and technical gaps. Exact PR/check/run co ## September 6 source inventory checkpoint +### 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. + +RED `7c2c0d1` compiled with two failures: direct symlink opening succeeded; nameless-input validation took the old error path. The second is not an unauthorized-read claim. Canonical fixes `b06f54a` and `7ccbbbe` were reused with attribution as `f97ef46` and `630d258`, pinning the admitted parent and refusing symlink opens. A separate review found no existing FIFO repair. RED `bce7efa` replaced a checked unit-test regular file with an actual FIFO and timed out after 2.01 seconds. `ddf3f62` adds `O_NONBLOCK` at the existing opening boundary; existing device/inode validation then rejects the replacement without waiting for a writer. Independent bounded read-only review found no additional defect. No new abstraction or unpinned dependency was introduced. + +Final source `ddf3f62` passes 193 tests/28 suites including three doctests, strict Clippy, warnings-denied rustdoc, formatting, CI contract and diff checks. Unchanged coverage passes 318/318 reported functions, 2,778/2,778 normalized regions and 494/494 normalized branches. Raw 3,798/3,860 lines, 5,873/5,980 regions and 449/494 branches are not 100%. Logs: `/tmp/conceptweave-pr29-{baseline,integrated,opening-red,fifo-red,verified,clippy-verified,rustdoc-verified,coverage-verified}.log`. TRD and Proposed ADR0006 trace alternatives, retained failures and consumer obligations. + +Root/later private readers must inherit FIFO protection and the source-scope cascade. Authentic decisions and independent approvals remain 0/3,715 plus four unresolved standalone sources. Unit fixtures do not count as real research or approval. Native Visual Inspection was retried and the Mac remains locked, so no fresh screen evidence exists. Keep Draft; no hosted GREEN, protected merge, release or Zotero mutation is claimed. + ### September 7 PR28 metadata roundtrip continuity repair Normal merge `13b5529` preserves original PR28 `ba6b3dfc71cf89ed4c57b85da0dd9ca5f983efee` and PR27 `fd5ef23c1c23fa36ef106400e15c9438eaa5cd41`. RED `97fc490` compiled with four passing and two failing roundtrip tests: inconsistent retained parent coordinates passed shared admission, while valid pending orphan metadata failed worksheet restoration. `1157b1d` centralizes parent validation and removes divergent worksheet checks; `e102894` removes an implied evaluator guard and preserves coherent-mutation stale-approval rejection. Independent read-only review found no further production defect in the bounded repair.