From 8d3e99bc0053cdd601ca33209ef0f5205f2f08fa Mon Sep 17 00:00:00 2001 From: John Coffey Date: Sun, 27 Sep 2026 18:32:45 -0700 Subject: [PATCH] Legal holds, step 4: freezing, release, and the audit log Placing or widening a hold freezes what's already archived in its scope and range, its old deadline noted; releasing one gives each item no other hold covers that deadline back, or release plus 30 days if later. One pass over the archive does both and changes nothing twice (LH-6, LH-10, LH-11). A held archived item can't be destroyed; restoring still can, and the hold is named only to callers who may see holds (LH-7). Audit records about a held account survive the purge (AU-7). Fixes the daily clean-up of expired archived items (UD-13), which never found any: the registry's unfiltered query reads an all-ids index that archived items aren't in. Items are now walked account by account, kept deleted accounts included. Expired items were still removed whenever their account's archive was read. --- crates/common/src/audit.rs | 11 ++- crates/common/src/hold.rs | 95 ++++++++++++++++++++++++- crates/features/src/hold/mod.rs | 35 +++++++++ crates/features/src/undelete/data.rs | 7 ++ crates/features/src/undelete/records.rs | 81 +++++++++++++++++++-- crates/jmap/src/inbuxa/legal_hold.rs | 5 ++ crates/jmap/src/inbuxa/undelete.rs | 27 +++++++ tests/src/system/legal_hold.rs | 88 ++++++++++++++++++++++- 8 files changed, 341 insertions(+), 8 deletions(-) diff --git a/crates/common/src/audit.rs b/crates/common/src/audit.rs index 1cfeb90..488154b 100644 --- a/crates/common/src/audit.rs +++ b/crates/common/src/audit.rs @@ -449,7 +449,16 @@ impl Server { pub async fn audit_purge(&self) -> trc::Result { let settings = log::settings(self.store()).await?; let cutoff = ms().saturating_sub(settings.keep_for_secs.saturating_mul(1000)); - log::purge(self.store(), cutoff, |_| false).await + // LH-6, AU-7: a record about a held account stays while it's held. + // Worked out before the purge, which can't wait on lookups. + let held = self.held_accounts().await?; + log::purge(self.store(), cutoff, |record| { + record + .target + .account_id + .is_some_and(|account_id| held.contains(&account_id)) + }) + .await } } diff --git a/crates/common/src/hold.rs b/crates/common/src/hold.rs index f92815e..c97dd56 100644 --- a/crates/common/src/hold.rs +++ b/crates/common/src/hold.rs @@ -10,7 +10,26 @@ //! there are few holds. use crate::Server; -use inbuxa_features::hold::{self, Hold, Keeping, Member}; +use ahash::AHashMap; +use inbuxa_features::{ + hold::{self, HELD_UNTIL, Hold, Keeping, Member, is_held_until}, + undelete::records, +}; +use registry::schema::{prelude::ObjectType, structs::ArchivedItem}; +use store::{registry::RegistryQuery, write::now}; +use trc::AddContext; +use types::id::Id; + +/// The grace a released item gets at least (LH-10): a release made in error +/// can be undone by placing a new hold within it. +const RELEASE_GRACE: u64 = 30 * 86_400; + +/// What a settle pass changed. +#[derive(Debug, Default, Clone, Copy, PartialEq, Eq)] +pub struct Settled { + pub frozen: usize, + pub released: usize, +} impl Server { /// The active holds covering `account_id`, through its own name, its @@ -45,6 +64,80 @@ impl Server { Ok(Keeping::new(retention, &self.holds_on(account_id).await?)) } + /// LH-6, LH-10, LH-11: brings the whole archive in line with the active + /// holds. An archived item a hold covers is frozen (no deadline), its + /// old deadline noted; a frozen one no hold covers any more gets that + /// deadline back, or release plus 30 days if later. Run after every + /// change to a hold; it changes nothing twice. + pub async fn settle_archive(&self) -> trc::Result { + let data = self.store(); + let registry = self.registry(); + let any_active = !hold::active(data).await?.is_empty(); + let now = now(); + let mut keeping: AHashMap> = AHashMap::new(); + let mut settled = Settled::default(); + for id in records::all(data, registry).await? { + let Some(item) = registry.object::(id).await? else { + continue; + }; + let account_id = item.account_id().document_id(); + if !keeping.contains_key(&account_id) { + // An account that's gone can't be placed in a domain or + // tenant any more: None, and its items are left as they are + let known = self.account(account_id).await.is_ok(); + let value = if known { Some(self.keeping(account_id).await?) } else { None }; + keeping.insert(account_id, value); + } + let until = item.archived_until().timestamp().max(0) as u64; + let held = is_held_until(until); + let covered = match keeping.get(&account_id).and_then(Option::as_ref) { + Some(keeping) => match &item { + ArchivedItem::Email(email) => { + keeping.covers(Some(email.received_at.timestamp().max(0) as u64)) + } + ArchivedItem::CalendarEvent(event) => keeping + .covers_event(event.start_time.map(|t| t.timestamp().max(0) as u64)), + _ => keeping.covers(None), + }, + // Gone: release only once no hold is active anywhere + None => held && any_active, + }; + if covered && !held { + hold::set_original_deadline(data, id.id(), Some(until)).await?; + records::set_deadline(data, registry, id, &item, HELD_UNTIL).await?; + settled.frozen += 1; + } else if !covered && held { + let original = hold::original_deadline(data, id.id()).await?.unwrap_or(0); + records::set_deadline(data, registry, id, &item, original.max(now + RELEASE_GRACE)) + .await?; + hold::set_original_deadline(data, id.id(), None).await?; + settled.released += 1; + } + } + Ok(settled) + } + + /// Every account an active hold covers now. Empty, without looking at + /// accounts, when nothing is held. + pub async fn held_accounts(&self) -> trc::Result> { + let mut held = ahash::AHashSet::new(); + if hold::active(self.store()).await?.is_empty() { + return Ok(held); + } + for id in self + .registry() + .query::>(RegistryQuery::new(ObjectType::Account)) + .await + .caused_by(trc::location!())? + { + let account_id = id.document_id(); + if self.is_held(account_id).await? { + held.insert(account_id); + } + } + Ok(held) + } + /// Whether any active hold covers `account_id` at all. pub async fn is_held(&self, account_id: u32) -> trc::Result { Ok(!self.holds_on(account_id).await?.is_empty()) diff --git a/crates/features/src/hold/mod.rs b/crates/features/src/hold/mod.rs index 4480dde..5815647 100644 --- a/crates/features/src/hold/mod.rs +++ b/crates/features/src/hold/mod.rs @@ -107,6 +107,7 @@ impl Keeping { const FEATURE: u8 = b'H'; const KIND_HOLD: u8 = b'h'; +const KIND_ORIGINAL: u8 = b'o'; /// How many times creating a hold retries when another node took its id. const CREATE_ATTEMPTS: usize = 5; @@ -367,6 +368,40 @@ fn key(id: u32) -> ValueKey { ValueKey::from(class(id)) } +fn original_class(item_id: u64) -> ValueClass { + let mut key = Vec::with_capacity(10); + key.push(FEATURE); + key.push(KIND_ORIGINAL); + key.extend_from_slice(&item_id.to_be_bytes()); + ValueClass::Any(AnyClass { + subspace: SUBSPACE_INBUXA, + key, + }) +} + +/// LH-10: an archived item's deadline from before a hold froze it, so a +/// release can give it back (or a later one). None for an item held from +/// its deletion, which never had one. +pub async fn original_deadline(data: &Store, item_id: u64) -> trc::Result> { + data.get_value::(ValueKey::from(original_class(item_id))) + .await + .caused_by(trc::location!()) +} + +/// Notes (`Some`) or forgets (`None`) an item's deadline from before it +/// was frozen. +pub async fn set_original_deadline(data: &Store, item_id: u64, until: Option) -> trc::Result<()> { + let mut batch = BatchBuilder::new(); + match until { + Some(until) => batch.set(original_class(item_id), until.to_be_bytes().to_vec()), + None => batch.clear(original_class(item_id)), + }; + data.write(batch.build_all()) + .await + .caused_by(trc::location!()) + .map(|_| ()) +} + /// One hold, released or not. pub async fn get(data: &Store, id: u32) -> trc::Result> { Ok(data diff --git a/crates/features/src/undelete/data.rs b/crates/features/src/undelete/data.rs index b170c98..de1a7e4 100644 --- a/crates/features/src/undelete/data.rs +++ b/crates/features/src/undelete/data.rs @@ -470,8 +470,15 @@ mod tests { size: 3, mailboxes: vec![1], keywords: vec![], + held_ranges: vec![(Some(10), None)], + otherwise_until: Some(20), }; let bytes = Json(¬e).serialize().unwrap(); assert_eq!(Json::::deserialize(&bytes).unwrap().0, note); + + // A note written before legal holds still reads, as not held + let old = br#"{"archived_at":1,"archived_until":2,"size":3,"mailboxes":[1],"keywords":[]}"#; + let read = Json::::deserialize(old).unwrap().0; + assert!(read.held_ranges.is_empty() && read.otherwise_until.is_none()); } } diff --git a/crates/features/src/undelete/records.rs b/crates/features/src/undelete/records.rs index eb2b410..e21bdc8 100644 --- a/crates/features/src/undelete/records.rs +++ b/crates/features/src/undelete/records.rs @@ -89,6 +89,54 @@ pub async fn insert( Ok(id) } +/// Moves an archived item's deadline, and its kept copy's with it: frozen +/// by a hold (LH-6) or given a real one on release (LH-10). Returns the +/// item as it now is. +pub async fn set_deadline( + data: &Store, + registry: &RegistryStore, + id: Id, + item: &ArchivedItem, + until: u64, +) -> trc::Result { + let account_id = item.account_id().document_id(); + let blob_hash = item.blob_id().hash.clone(); + let before = item.archived_until().timestamp() as u64; + let mut updated = item.clone(); + updated.set_archived_until(registry::types::datetime::UTCDateTime::from_timestamp(until as i64)); + + // The new link first, so the kept copy is never unlinked in between + let mut batch = BatchBuilder::new(); + batch + .with_account_id(account_id) + .set( + BlobOp::Link { + hash: blob_hash.clone(), + to: BlobLink::Temporary { until }, + }, + vec![], + ); + if before != until { + batch.clear(BlobOp::Link { + hash: blob_hash, + to: BlobLink::Temporary { until: before }, + }); + } + data::log_change(&mut batch, account_id, registry.assign_id(), id, Change::Updated); + data.write(batch.build_all()) + .await + .caused_by(trc::location!())?; + + let mut batch = BatchBuilder::new(); + batch.set(item_class(id.id()), updated.to_pickled_vec()); + registry + .store() + .write(batch.build_all()) + .await + .caused_by(trc::location!())?; + Ok(updated) +} + /// Removes an archived item and releases its kept copy: on restore (UD-9), /// on destroy (UD-12) and past its deadline (UD-13). pub async fn remove( @@ -184,15 +232,38 @@ pub async fn get( } } +/// Every archived item on the server, account by account. Items are +/// indexed by account only, so the registry's query without a filter, +/// which reads its all-ids index, finds none of them. +pub async fn all(data: &Store, registry: &RegistryStore) -> trc::Result> { + let mut accounts = registry + .query::>(RegistryQuery::new(ObjectType::Account)) + .await + .caused_by(trc::location!())? + .into_iter() + .map(|id| id.document_id()) + .collect::>(); + // Deleted accounts still kept have archived items too + accounts.extend(data::kept_accounts(data).await?.into_iter().map(|(id, _)| id)); + accounts.sort_unstable(); + accounts.dedup(); + let mut items = Vec::new(); + for account_id in accounts { + items.extend( + registry + .query::>(RegistryQuery::new(ObjectType::ArchivedItem).with_account(account_id)) + .await + .caused_by(trc::location!())?, + ); + } + Ok(items) +} + /// Removes every expired archived item on the server (UD-13), for the /// scheduled clean-up. pub async fn remove_expired(data: &Store, registry: &RegistryStore) -> trc::Result { let mut removed = 0; - for id in registry - .query::>(RegistryQuery::new(ObjectType::ArchivedItem)) - .await - .caused_by(trc::location!())? - { + for id in all(data, registry).await? { if let Some(item) = registry.object::(id).await? && is_expired(&item) { diff --git a/crates/jmap/src/inbuxa/legal_hold.rs b/crates/jmap/src/inbuxa/legal_hold.rs index bd9e065..2d2ccb5 100644 --- a/crates/jmap/src/inbuxa/legal_hold.rs +++ b/crates/jmap/src/inbuxa/legal_hold.rs @@ -405,6 +405,11 @@ pub async fn set( response.updated.append(id, None); } + // LH-6, LH-10, LH-11: the archive follows what's now held + if !response.created.is_empty() || !response.updated.is_empty() { + server.settle_archive().await?; + } + for id in request.unwrap_destroy().into_valid() { response.not_destroyed.append( id, diff --git a/crates/jmap/src/inbuxa/undelete.rs b/crates/jmap/src/inbuxa/undelete.rs index b633d98..930c7f7 100644 --- a/crates/jmap/src/inbuxa/undelete.rs +++ b/crates/jmap/src/inbuxa/undelete.rs @@ -298,6 +298,33 @@ pub(crate) async fn set(mut set: RegistrySetResponse<'_>) -> trc::Result + { + let mut why = "A legal hold applies to this item, so it can't be deleted.".to_string(); + if set + .access_token + .has_permission(registry::schema::enums::Permission::SysLegalHoldGet) + { + let names = set + .server + .holds_on(account_id) + .await? + .into_iter() + .map(|hold| hold.name) + .collect::>(); + if !names.is_empty() { + why = format!("Held by {}, so it can't be deleted.", names.join(", ")); + } + } + set.response + .not_destroyed + .append(id, SetError::forbidden().with_description(why)); + } Some(item) => { undelete::records::remove(data, registry, id, &item).await?; set.response.destroyed.push(id); diff --git a/tests/src/system/legal_hold.rs b/tests/src/system/legal_hold.rs index 12b139f..7e265c8 100644 --- a/tests/src/system/legal_hold.rs +++ b/tests/src/system/legal_hold.rs @@ -33,7 +33,9 @@ const USING: &[&str] = &[ impl Account { async fn hold_call(&self, method: &str, mut arguments: Value) -> (String, Value) { - arguments["accountId"] = self.id_string().into(); + if arguments.get("accountId").is_none() { + arguments["accountId"] = self.id_string().into(); + } let response = self.jmap_request(USING, json!([[method, arguments, "0"]])).await; let call = response .0 @@ -364,6 +366,90 @@ pub async fn test(test: &mut TestServer) { "LH-3: a contact wasn't kept whole: {kept:?}" ); + // LH-6: placing a hold freezes what's already archived; LH-7: frozen + // items can't be destroyed; LH-11: releasing one hold of two frees + // nothing; LH-10: releasing the last gives a real deadline back + admin + .registry_update_setting( + DataRetention { + archive_deleted_items_for: Some(registry::schema::prelude::Duration( + std::time::Duration::from_secs(30 * 86_400), + )), + ..Default::default() + }, + &[Property::ArchiveDeletedItemsFor], + ) + .await; + let frozen = admin + .create_user_account("frozen@example.com", "frozen-secret-9031", "Frozen", &[], vec![]) + .await; + let frozen_client = frozen.jmap_client().await; + let doomed = import(&frozen_client, "Deleted before the hold", None).await; + frozen_client.email_destroy(&doomed).await.unwrap(); + test.wait_for_tasks().await; + let archived = |items: Vec| { + items + .into_iter() + .find(|i| i["subject"] == "Deleted before the hold") + .unwrap_or_else(|| panic!("not archived")) + }; + let item = archived(frozen.archived_items().await); + assert!(!is_held(&item), "undelete's 30 days first: {item}"); + let item_id = item["id"].as_str().unwrap().to_string(); + + let response = admin + .hold_set(json!({"reason": "First matter", "create": { + "a": {"name": "Matter 7001", "scope": {"accounts": [frozen.id_string()]}}, + "b": {"name": "Matter 7002", "scope": {"accounts": [frozen.id_string()]}}}})) + .await; + let first = response["created"]["a"]["id"].as_str().unwrap().to_string(); + let second = response["created"]["b"]["id"].as_str().unwrap().to_string(); + assert!( + is_held(&archived(frozen.archived_items().await)), + "test 6, LH-6: the archived item wasn't frozen" + ); + + let (_, response) = frozen + .hold_call("x:ArchivedItem/set", json!({"destroy": [item_id]})) + .await; + assert_eq!( + response["notDestroyed"][item_id.as_str()]["type"], "forbidden", + "test 6, LH-7: the owner destroyed a held item: {response}" + ); + assert!( + !response.to_string().contains("Matter 70"), + "LH-7: the hold was named to someone who can't see holds: {response}" + ); + let (_, response) = admin + .hold_call( + "x:ArchivedItem/set", + json!({"accountId": frozen.id_string(), "destroy": [item_id]}), + ) + .await; + assert!( + response.to_string().contains("Matter 7001"), + "LH-7: the administrator isn't told which hold: {response}" + ); + + admin + .hold_set(json!({"reason": "First settled", "update": {first.as_str(): {"released": true}}})) + .await; + assert!( + is_held(&archived(frozen.archived_items().await)), + "test 9, LH-11: releasing one hold of two freed the item" + ); + admin + .hold_set(json!({"reason": "Second settled", "update": {second.as_str(): {"released": true}}})) + .await; + let item = archived(frozen.archived_items().await); + assert!(!is_held(&item), "LH-10: the last release left it held: {item}"); + let until = item["archivedUntil"].as_str().unwrap_or_default().to_string(); + let grace = chrono::Utc::now() + chrono::Duration::days(29); + assert!( + until > grace.format("%Y-%m-%dT%H:%M:%S").to_string(), + "test 8, LH-10: under 30 days of grace after release: {until}" + ); + // LH-10: release needs a reason, and a released hold stays, read-only let response = admin .hold_set(json!({"update": {hold_id.as_str(): {"released": true}}}))