diff --git a/docs/configuration.md b/docs/configuration.md index 8df5b48..26630d8 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -376,6 +376,9 @@ Frontmatter fields: | `source` | Provenance source kind, for example `manual`, `github_pr`, `github_pr_review`, or `session_memory_extraction`. | | `source_ref` | Source reference such as `owner/repo#123`, a commit SHA, or another non-secret audit handle. | | `expires_at` | Optional expiry date in `YYYY-MM-DD` format. Expired hosted memory is ignored. | +| `retention_class` | Optional lifecycle class such as `standard`, `short_lived`, `security`, or `legal_hold`. | +| `redacted_at` | Marks content as redacted. Hosted review keeps the metadata visible but replaces the body with a redaction stub. | +| `deleted_at` | Marks memory as deleted. Hosted review excludes deleted entries from prompt loading. | | `created_at`, `created_by`, `session_id`, `transcript_ref`, `confidence` | Optional provenance fields for audit and review artifacts. | Local mode tolerates missing metadata for backward compatibility. Hosted review @@ -384,6 +387,18 @@ memory below the configured trust threshold. Tagged hosted memory is injected with memory ids and provenance metadata; findings that rely on memory should include those ids in `memory_refs`. +Hosted sync and persistence boundaries run high-confidence secret scanning +before writing or uploading memory. Entries with detected secret patterns are +blocked by default. Logs and review candidates include only pattern labels and +reason codes, not matched secret values. False positives should be handled by +redacting or editing the memory entry before retrying sync. + +Hosted team-memory pull is conflict-aware. Local changes are preserved when +both local and remote content changed since the last known server checksum; a +conflict record is written for operator review instead of overwriting local +memory. Hosted team-memory sync also sends tenant, installation, repo, and +domain scope metadata so the server can authorize the full tuple. + ### @include directives AGENTS.md files support `@include` to pull in content from other files: diff --git a/docs/shared/hosted-review-phase-5-pr.md b/docs/shared/hosted-review-phase-5-pr.md new file mode 100644 index 0000000..7040702 --- /dev/null +++ b/docs/shared/hosted-review-phase-5-pr.md @@ -0,0 +1,37 @@ +# Hosted Review Phase 5 PR Notes + +## Linked issues + +Fixes #102. +Fixes #103. +Fixes #111. +Fixes #109. + +## Summary + +- Enforces high-confidence secret scanning before hosted session-memory persistence or candidate creation, settings sync upload, team-memory upload, and team-memory pull apply. +- Blocks secret-bearing memory by default and records only scanner labels/reason codes, never matched secret values. +- Adds structured hosted team-memory sync scope with tenant id, installation id, repo id, repo full name, and domain metadata. +- Replaces server-wins team-memory pull application with conflict-aware handling that preserves local changes and writes conflict records for both-changed cases. +- Adds lifecycle metadata support for `retention_class`, `redacted_at`, and `deleted_at`. +- Excludes deleted hosted memory, redacts hosted prompt content for redacted entries, and adds helpers for deleting a hosted memory scope or redacting a memory file. +- Documents retention, redaction, secret scanning, and conflict workflows. + +## Test evidence + +- `cargo fmt --all -- --check` +- `cargo check --workspace` +- `cargo clippy --workspace --all-targets -- -D warnings` +- `cargo test -p claurst-core --lib claudemd --quiet` +- `cargo test -p claurst-core --lib team_memory_sync --quiet` +- `cargo test -p claurst-core --lib settings_sync --quiet` +- `cargo test -p claurst-core --lib memdir --quiet` +- `cargo test -p claurst-query session_memory --quiet` +- `cargo test -p claurst-core --lib --quiet` +- `cargo test --workspace --quiet` + +## Risk notes + +- Hosted server authorization still must be enforced server-side; the client now sends structured scope metadata but does not treat client-side path construction as an authorization boundary. +- Conflict records preserve local and remote memory text for operator review, so downstream tooling should apply the same access controls as memory storage. +- Secret scanning is intentionally high-confidence and blocks by default; false positives require operator edit/redaction before retry. diff --git a/src-rust/crates/core/src/claudemd.rs b/src-rust/crates/core/src/claudemd.rs index 3ca2846..625440e 100644 --- a/src-rust/crates/core/src/claudemd.rs +++ b/src-rust/crates/core/src/claudemd.rs @@ -52,6 +52,12 @@ pub struct MemoryFrontmatter { #[serde(default)] pub expires_at: Option, #[serde(default)] + pub retention_class: Option, + #[serde(default)] + pub redacted_at: Option, + #[serde(default)] + pub deleted_at: Option, + #[serde(default)] pub created_at: Option, #[serde(default)] pub created_by: Option, @@ -191,6 +197,13 @@ pub fn parse_frontmatter(content: &str) -> (MemoryFrontmatter, &str) { "source" => fm.source = Some(strip_frontmatter_value(&val).to_string()), "source_ref" => fm.source_ref = Some(strip_frontmatter_value(&val).to_string()), "expires_at" => fm.expires_at = Some(strip_frontmatter_value(&val).to_string()), + "retention_class" => { + fm.retention_class = Some(strip_frontmatter_value(&val).to_string()) + } + "redacted_at" => { + fm.redacted_at = Some(strip_frontmatter_value(&val).to_string()) + } + "deleted_at" => fm.deleted_at = Some(strip_frontmatter_value(&val).to_string()), "created_at" => fm.created_at = Some(strip_frontmatter_value(&val).to_string()), "created_by" => fm.created_by = Some(strip_frontmatter_value(&val).to_string()), "session_id" => fm.session_id = Some(strip_frontmatter_value(&val).to_string()), @@ -356,6 +369,10 @@ pub fn memory_file_allowed_for_options(file: &MemoryFileInfo, options: &MemoryLo return false; } + if file.frontmatter.deleted_at.is_some() { + return false; + } + if matches!( file.frontmatter.visibility, Some(MemoryVisibility::SecurityPrivate) @@ -394,7 +411,11 @@ pub fn memory_id(file: &MemoryFileInfo) -> String { } pub fn format_memory_file_for_prompt(file: &MemoryFileInfo, hosted: bool) -> String { - let body = file.content.trim(); + let body = if hosted && file.frontmatter.redacted_at.is_some() { + "[REDACTED: memory content removed; retain metadata for audit]" + } else { + file.content.trim() + }; if !hosted { return body.to_string(); } @@ -763,6 +784,39 @@ mod tests { assert!(files.is_empty()); } + #[test] + fn hosted_review_excludes_deleted_memory() { + let project = tempfile::tempdir().unwrap(); + std::fs::write( + project.path().join("AGENTS.md"), + "---\ntrust: maintainer_approved\nvisibility: public_review\ndeleted_at: 2026-01-01T00:00:00Z\n---\ndeleted memory", + ) + .unwrap(); + + let files = + load_all_memory_files_with_options(project.path(), &MemoryLoadOptions::hosted_review()); + + assert!(files.is_empty()); + } + + #[test] + fn hosted_review_redacts_memory_content_in_prompt() { + let project = tempfile::tempdir().unwrap(); + std::fs::write( + project.path().join("AGENTS.md"), + "---\nid: mem_redacted\ntrust: maintainer_approved\nvisibility: public_review\nredacted_at: 2026-01-01T00:00:00Z\n---\noriginal sensitive detail", + ) + .unwrap(); + + let options = MemoryLoadOptions::hosted_review(); + let files = load_all_memory_files_with_options(project.path(), &options); + let prompt = build_memory_prompt_with_options(&files, &options); + + assert!(prompt.contains("id=\"mem_redacted\"")); + assert!(prompt.contains("[REDACTED: memory content removed")); + assert!(!prompt.contains("original sensitive detail")); + } + #[test] fn hosted_review_excludes_security_private_memory_by_default() { let project = tempfile::tempdir().unwrap(); diff --git a/src-rust/crates/core/src/memdir.rs b/src-rust/crates/core/src/memdir.rs index daf234a..69828be 100644 --- a/src-rust/crates/core/src/memdir.rs +++ b/src-rust/crates/core/src/memdir.rs @@ -410,6 +410,26 @@ fn hosted_memory_path(scope: &HostedReviewScope) -> PathBuf { .join("memory") } +pub fn hosted_memory_path_for_scope(scope: &HostedReviewScope) -> PathBuf { + hosted_memory_path(scope) +} + +pub fn delete_hosted_memory_for_scope(scope: &HostedReviewScope) -> std::io::Result<()> { + let path = hosted_memory_path(scope); + if path.exists() { + std::fs::remove_dir_all(path)?; + } + Ok(()) +} + +pub fn redact_memory_file(path: &Path, reason: &str) -> std::io::Result<()> { + let timestamp = chrono::Utc::now().to_rfc3339(); + let stub = format!( + "---\nredacted_at: {timestamp}\nretention_class: security\nsource: redaction\n---\n\n[REDACTED: {reason}]\n" + ); + std::fs::write(path, stub) +} + /// Sanitize an arbitrary string into a directory-name-safe component. /// Matches `sanitizePath` used inside `getAutoMemPath` in `paths.ts`. pub fn sanitize_path_component(s: &str) -> String { @@ -1029,4 +1049,46 @@ mod tests { assert_ne!(first, branch); assert!(branch.to_string_lossy().contains("branch-feature_review")); } + + #[test] + fn hosted_memory_delete_removes_scope_directory() { + let home = tempfile::tempdir().unwrap(); + let _lock = crate::coven_shared::COVEN_HOME_ENV_LOCK + .lock() + .unwrap_or_else(|err| err.into_inner()); + let original_test_home = std::env::var("COVEN_CODE_TEST_HOME").ok(); + std::env::set_var("COVEN_CODE_TEST_HOME", home.path()); + let scope = crate::hosted_review::HostedReviewScope::new( + "tenant-delete".to_string(), + "install-delete".to_string(), + "repo-delete".to_string(), + "OpenCoven/coven-code".to_string(), + ); + let path = hosted_memory_path_for_scope(&scope); + std::fs::create_dir_all(&path).unwrap(); + std::fs::write(path.join("MEMORY.md"), "delete me").unwrap(); + + delete_hosted_memory_for_scope(&scope).unwrap(); + + match original_test_home { + Some(value) => std::env::set_var("COVEN_CODE_TEST_HOME", value), + None => std::env::remove_var("COVEN_CODE_TEST_HOME"), + } + + assert!(!path.exists()); + } + + #[test] + fn redact_memory_file_preserves_audit_stub_without_original_content() { + let tmp = tempfile::tempdir().unwrap(); + let path = tmp.path().join("MEMORY.md"); + std::fs::write(&path, "secret incident detail").unwrap(); + + redact_memory_file(&path, "operator request").unwrap(); + + let content = std::fs::read_to_string(&path).unwrap(); + assert!(content.contains("redacted_at:")); + assert!(content.contains("[REDACTED: operator request]")); + assert!(!content.contains("secret incident detail")); + } } diff --git a/src-rust/crates/core/src/settings_sync.rs b/src-rust/crates/core/src/settings_sync.rs index a1eacca..3bddef2 100644 --- a/src-rust/crates/core/src/settings_sync.rs +++ b/src-rust/crates/core/src/settings_sync.rs @@ -14,6 +14,7 @@ // and values are the UTF-8 file contents (JSON or Markdown). use crate::hosted_review::{hosted_project_id, HostedReviewScope}; +use crate::team_memory_sync::scan_for_secrets; use anyhow::Result; use serde::Deserialize; use serde_json::Value; @@ -288,6 +289,8 @@ impl SettingsSyncManager { /// /// Compares with existing remote entries and only uploads changed keys. pub async fn upload(&self, local_entries: HashMap) -> Result<()> { + let local_entries = filter_entries_with_secrets(local_entries, "Settings sync"); + // Fetch current remote state for diff let remote_entries = match self.download().await? { Some(data) => data.memory_files, @@ -352,6 +355,31 @@ impl SettingsSyncManager { } } +fn filter_entries_with_secrets( + entries: HashMap, + context: &str, +) -> HashMap { + entries + .into_iter() + .filter_map(|(key, value)| { + let secrets = scan_for_secrets(&value); + if secrets.is_empty() { + return Some((key, value)); + } + + let labels: Vec<&str> = secrets.iter().map(|m| m.label.as_str()).collect(); + warn!( + "{}: blocking {:?} from upload: detected {} ({} secret pattern(s))", + context, + key, + labels.join(", "), + labels.len(), + ); + None + }) + .collect() +} + // --------------------------------------------------------------------------- // Apply result // --------------------------------------------------------------------------- @@ -543,6 +571,20 @@ mod tests { assert!(data.memory_files.is_empty()); } + #[test] + fn filter_entries_with_secrets_blocks_secret_values() { + let mut entries = HashMap::new(); + let secret = format!("ghp_{}", "A".repeat(36)); + entries.insert(SYNC_KEY_USER_MEMORY.to_string(), format!("token={secret}")); + entries.insert("safe.md".to_string(), "# Safe".to_string()); + + let filtered = filter_entries_with_secrets(entries, "test"); + + assert!(filtered.contains_key("safe.md")); + assert!(!filtered.contains_key(SYNC_KEY_USER_MEMORY)); + assert!(!filtered.values().any(|value| value.contains(&secret))); + } + #[test] fn test_retry_delay_progression() { assert_eq!(retry_delay(1), Duration::from_secs(1)); diff --git a/src-rust/crates/core/src/team_memory_sync.rs b/src-rust/crates/core/src/team_memory_sync.rs index 785946d..6ed73fb 100644 --- a/src-rust/crates/core/src/team_memory_sync.rs +++ b/src-rust/crates/core/src/team_memory_sync.rs @@ -58,6 +58,54 @@ pub struct TeamMemoryData { pub etag: Option, } +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +#[serde(rename_all = "snake_case")] +pub enum PullConflictKind { + CleanApply, + LocalOnly, + RemoteOnly, + BothChanged, + RejectedUnsafePath, + RejectedSecret, +} + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +pub struct TeamMemoryPullConflict { + pub key: String, + pub kind: PullConflictKind, + pub local_checksum: Option, + pub base_checksum: Option, + pub remote_checksum: Option, + pub reason: String, +} + +#[derive(Debug, Clone, Default, Serialize, Deserialize)] +pub struct TeamMemoryPullResult { + pub applied: Vec, + pub conflicts: Vec, +} + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +pub struct HostedTeamMemoryScope { + pub tenant_id: String, + pub installation_id: String, + pub repo_id: String, + pub repo_full_name: String, + pub domain: String, +} + +impl HostedTeamMemoryScope { + pub fn from_scope(scope: &HostedReviewScope) -> Self { + Self { + tenant_id: scope.tenant_id.clone(), + installation_id: scope.installation_id.clone(), + repo_id: scope.repo_id.clone(), + repo_full_name: scope.repo_full_name.clone(), + domain: scope.domain_component(), + } + } +} + // --------------------------------------------------------------------------- // Checksum helper // --------------------------------------------------------------------------- @@ -123,6 +171,7 @@ pub struct TeamMemorySync { token: String, /// Local directory that mirrors the server's key namespace. team_dir: PathBuf, + hosted_scope: Option, } impl TeamMemorySync { @@ -132,6 +181,7 @@ impl TeamMemorySync { repo, token, team_dir, + hosted_scope: None, } } @@ -141,27 +191,39 @@ impl TeamMemorySync { token: String, team_dir: PathBuf, ) -> Self { - Self::new( + let mut sync = Self::new( api_base, hosted_team_memory_repo_key(scope), token, team_dir, - ) + ); + sync.hosted_scope = Some(HostedTeamMemoryScope::from_scope(scope)); + sync } pub fn repo_key(&self) -> &str { &self.repo } + pub fn hosted_scope(&self) -> Option<&HostedTeamMemoryScope> { + self.hosted_scope.as_ref() + } + // ----------------------------------------------------------------------- // Pull // ----------------------------------------------------------------------- - /// Pull all entries from the server. Server wins: overwrites local files. + /// Pull all entries from the server. /// /// Updates `state.last_known_etag` and `state.server_checksums` on success. /// Returns `Ok(())` on HTTP 404 (no remote data yet). pub async fn pull(&self, state: &mut SyncState) -> Result<()> { + self.pull_with_conflicts(state).await.map(|_| ()) + } + + /// Pull all entries from the server, preserving local changes when both + /// local and remote changed since the last known server checksum. + pub async fn pull_with_conflicts(&self, state: &mut SyncState) -> Result { let client = reqwest::Client::new(); let url = format!( "{}/api/claude_code/team_memory?repo={}", @@ -169,9 +231,18 @@ impl TeamMemorySync { urlencoding::encode(&self.repo), ); - let response = client - .get(&url) - .bearer_auth(&self.token) + let mut request = client.get(&url).bearer_auth(&self.token); + if let Some(scope) = &self.hosted_scope { + request = request.query(&[ + ("tenant_id", scope.tenant_id.as_str()), + ("installation_id", scope.installation_id.as_str()), + ("repo_id", scope.repo_id.as_str()), + ("repo_full_name", scope.repo_full_name.as_str()), + ("domain", scope.domain.as_str()), + ]); + } + + let response = request .send() .await .context("team memory pull: HTTP request failed")?; @@ -179,7 +250,7 @@ impl TeamMemorySync { let http_status = response.status(); if http_status.as_u16() == 404 { - return Ok(()); // No remote data yet + return Ok(TeamMemoryPullResult::default()); // No remote data yet } if !http_status.is_success() { @@ -196,32 +267,105 @@ impl TeamMemorySync { .await .context("team memory pull: failed to parse response JSON")?; - state.server_checksums.clear(); + self.apply_remote_entries(data.entries, state).await + } - for entry in &data.entries { - validate_memory_path(&entry.key) - .with_context(|| format!("server returned unsafe path: {:?}", entry.key))?; + async fn apply_remote_entries( + &self, + entries: Vec, + state: &mut SyncState, + ) -> Result { + let mut result = TeamMemoryPullResult::default(); + + for entry in &entries { + if let Err(err) = validate_memory_path(&entry.key) { + result.conflicts.push(TeamMemoryPullConflict { + key: entry.key.clone(), + kind: PullConflictKind::RejectedUnsafePath, + local_checksum: None, + base_checksum: state.server_checksums.get(&entry.key).cloned(), + remote_checksum: Some(entry.checksum.clone()), + reason: err.to_string(), + }); + continue; + } - state - .server_checksums - .insert(entry.key.clone(), entry.checksum.clone()); + let secrets = scan_for_secrets(&entry.content); + if !secrets.is_empty() { + let labels: Vec = secrets.into_iter().map(|m| m.label).collect(); + result.conflicts.push(TeamMemoryPullConflict { + key: entry.key.clone(), + kind: PullConflictKind::RejectedSecret, + local_checksum: None, + base_checksum: state.server_checksums.get(&entry.key).cloned(), + remote_checksum: Some(entry.checksum.clone()), + reason: format!( + "remote entry contains secret patterns: {}", + labels.join(", ") + ), + }); + continue; + } + + if entry.content.len() > MAX_FILE_SIZE_BYTES { + continue; + } let local_path = self.team_dir.join(&entry.key); + let local_content = tokio::fs::read_to_string(&local_path).await.ok(); + let local_checksum = local_content.as_deref().map(content_checksum); + let base_checksum = state.server_checksums.get(&entry.key).cloned(); + + let local_changed = match (&local_checksum, &base_checksum) { + (Some(local), Some(base)) => local != base, + (Some(_), None) => true, + (None, _) => false, + }; + let remote_changed = base_checksum.as_deref() != Some(entry.checksum.as_str()); + + if local_changed && remote_changed { + let conflict = TeamMemoryPullConflict { + key: entry.key.clone(), + kind: PullConflictKind::BothChanged, + local_checksum, + base_checksum, + remote_checksum: Some(entry.checksum.clone()), + reason: "local and remote changed since last pull".to_string(), + }; + self.write_conflict_record(&conflict, local_content.as_deref(), entry) + .await?; + result.conflicts.push(conflict); + continue; + } + + if local_changed && !remote_changed { + result.conflicts.push(TeamMemoryPullConflict { + key: entry.key.clone(), + kind: PullConflictKind::LocalOnly, + local_checksum, + base_checksum, + remote_checksum: Some(entry.checksum.clone()), + reason: "local changed and remote did not change".to_string(), + }); + continue; + } + if let Some(parent) = local_path.parent() { tokio::fs::create_dir_all(parent) .await .with_context(|| format!("create_dir_all for {:?}", parent))?; } + tokio::fs::write(&local_path, &entry.content) + .await + .with_context(|| format!("writing {:?}", local_path))?; - if entry.content.len() <= MAX_FILE_SIZE_BYTES { - tokio::fs::write(&local_path, &entry.content) - .await - .with_context(|| format!("writing {:?}", local_path))?; - } - // Files exceeding MAX_FILE_SIZE_BYTES are silently skipped (same behaviour as push) + state + .server_checksums + .insert(entry.key.clone(), entry.checksum.clone()); + result.applied.push(entry.key.clone()); } - Ok(()) + Ok(result) } // ----------------------------------------------------------------------- @@ -311,7 +455,11 @@ impl TeamMemorySync { urlencoding::encode(&self.repo), ); - let body = serde_json::json!({ "entries": batch }); + let body = if let Some(scope) = &self.hosted_scope { + serde_json::json!({ "entries": batch, "scope": scope }) + } else { + serde_json::json!({ "entries": batch }) + }; let mut req = client.put(&url).bearer_auth(&self.token).json(&body); @@ -346,6 +494,29 @@ impl TeamMemorySync { } } + async fn write_conflict_record( + &self, + conflict: &TeamMemoryPullConflict, + local_content: Option<&str>, + remote: &TeamMemoryEntry, + ) -> Result<()> { + let conflict_dir = self.team_dir.join(".conflicts"); + tokio::fs::create_dir_all(&conflict_dir).await?; + let safe_key = remote.key.replace('/', "__"); + let path = conflict_dir.join(format!("{safe_key}.json")); + let record = serde_json::json!({ + "conflict": conflict, + "local": local_content.unwrap_or(""), + "remote": { + "key": remote.key, + "checksum": remote.checksum, + "content": remote.content, + } + }); + tokio::fs::write(path, serde_json::to_string_pretty(&record)?).await?; + Ok(()) + } + /// Recursively scan `team_dir` for `.md` files, returning entries sorted by key. async fn scan_local_files(&self) -> Result> { let mut entries = Vec::new(); @@ -550,6 +721,155 @@ mod tests { assert_ne!(first_sync.repo_key(), second_sync.repo_key()); assert!(first_sync.repo_key().contains("installations/install-1")); assert!(second_sync.repo_key().contains("installations/install-2")); + assert_eq!( + first_sync.hosted_scope().unwrap().installation_id, + "install-1" + ); + assert_eq!(first_sync.hosted_scope().unwrap().domain, "default-branch"); + } + + #[tokio::test] + async fn pull_clean_remote_entry_applies_file() { + let tmp = TempDir::new().unwrap(); + let sync = TeamMemorySync::new( + "https://example.com".to_string(), + "r".to_string(), + "t".to_string(), + tmp.path().to_path_buf(), + ); + let content = "# Remote"; + let mut state = SyncState::default(); + let result = sync + .apply_remote_entries( + vec![TeamMemoryEntry { + key: "MEMORY.md".to_string(), + content: content.to_string(), + checksum: content_checksum(content), + }], + &mut state, + ) + .await + .unwrap(); + + assert_eq!(result.applied, vec!["MEMORY.md"]); + assert!(result.conflicts.is_empty()); + assert_eq!( + tokio::fs::read_to_string(tmp.path().join("MEMORY.md")) + .await + .unwrap(), + content + ); + } + + #[tokio::test] + async fn pull_local_only_change_is_not_overwritten() { + let tmp = TempDir::new().unwrap(); + tokio::fs::write(tmp.path().join("MEMORY.md"), "# Local") + .await + .unwrap(); + let sync = TeamMemorySync::new( + "https://example.com".to_string(), + "r".to_string(), + "t".to_string(), + tmp.path().to_path_buf(), + ); + let base = "# Base"; + let mut state = SyncState::default(); + state + .server_checksums + .insert("MEMORY.md".to_string(), content_checksum(base)); + let result = sync + .apply_remote_entries( + vec![TeamMemoryEntry { + key: "MEMORY.md".to_string(), + content: base.to_string(), + checksum: content_checksum(base), + }], + &mut state, + ) + .await + .unwrap(); + + assert_eq!(result.conflicts[0].kind, PullConflictKind::LocalOnly); + assert_eq!( + tokio::fs::read_to_string(tmp.path().join("MEMORY.md")) + .await + .unwrap(), + "# Local" + ); + } + + #[tokio::test] + async fn pull_both_changed_creates_conflict_record() { + let tmp = TempDir::new().unwrap(); + tokio::fs::write(tmp.path().join("MEMORY.md"), "# Local") + .await + .unwrap(); + let sync = TeamMemorySync::new( + "https://example.com".to_string(), + "r".to_string(), + "t".to_string(), + tmp.path().to_path_buf(), + ); + let mut state = SyncState::default(); + state + .server_checksums + .insert("MEMORY.md".to_string(), content_checksum("# Base")); + let result = sync + .apply_remote_entries( + vec![TeamMemoryEntry { + key: "MEMORY.md".to_string(), + content: "# Remote".to_string(), + checksum: content_checksum("# Remote"), + }], + &mut state, + ) + .await + .unwrap(); + + assert_eq!(result.conflicts[0].kind, PullConflictKind::BothChanged); + assert!(tmp + .path() + .join(".conflicts") + .join("MEMORY.md.json") + .exists()); + assert_eq!( + tokio::fs::read_to_string(tmp.path().join("MEMORY.md")) + .await + .unwrap(), + "# Local" + ); + } + + #[tokio::test] + async fn pull_remote_secret_is_rejected_without_writing_value() { + let tmp = TempDir::new().unwrap(); + let sync = TeamMemorySync::new( + "https://example.com".to_string(), + "r".to_string(), + "t".to_string(), + tmp.path().to_path_buf(), + ); + let secret = format!("ghp_{}", "A".repeat(36)); + let mut state = SyncState::default(); + let result = sync + .apply_remote_entries( + vec![TeamMemoryEntry { + key: "MEMORY.md".to_string(), + content: format!("token={secret}"), + checksum: content_checksum(&secret), + }], + &mut state, + ) + .await + .unwrap(); + + assert_eq!(result.conflicts[0].kind, PullConflictKind::RejectedSecret); + assert!(result.conflicts[0] + .reason + .contains("GitHub personal access token")); + assert!(!result.conflicts[0].reason.contains(&secret)); + assert!(!tmp.path().join("MEMORY.md").exists()); } // --- validate_memory_path --- diff --git a/src-rust/crates/query/src/session_memory.rs b/src-rust/crates/query/src/session_memory.rs index 5b0e015..bcdb9aa 100644 --- a/src-rust/crates/query/src/session_memory.rs +++ b/src-rust/crates/query/src/session_memory.rs @@ -19,6 +19,7 @@ use claurst_api::{ SystemPrompt, }; use claurst_core::hosted_review::{HostedReviewConfig, MemorySourceTrust, RuntimeMode}; +use claurst_core::team_memory_sync::scan_for_secrets; use claurst_core::types::{Message, Role}; use serde::{Deserialize, Serialize}; use serde_json::Value; @@ -545,6 +546,31 @@ impl SessionMemoryExtractor { .map(|memory| memory.with_source_trust(source_trust)) .collect(); + let secret_labels = memory_secret_labels(&trusted_memories); + if !secret_labels.is_empty() { + let label_text = secret_labels.join(", "); + let redaction_required = ExtractedMemory { + content: format!("Redaction required before memory review. Detected secret patterns: {label_text}."), + category: MemoryCategory::Constraint, + confidence: 1.0, + source_trust, + }; + let reason = format!("redaction-required:{}", label_text); + let candidates = candidate_store + .write_pending( + &[redaction_required], + provenance, + "hosted-review", + "durable-memory", + Some(&reason), + ) + .await?; + return Ok(MemoryPersistenceOutcome::CandidatesWritten { + count: candidates.len(), + reason, + }); + } + if hosted_config.allows_auto_memory_persistence() { Self::persist(&trusted_memories, target_path).await?; return Ok(MemoryPersistenceOutcome::DurableWritten { @@ -581,6 +607,18 @@ fn hosted_memory_candidate_reason(config: &HostedReviewConfig) -> String { ) } +fn memory_secret_labels(memories: &[ExtractedMemory]) -> Vec { + let mut labels = Vec::new(); + for memory in memories { + for finding in scan_for_secrets(&memory.content) { + if !labels.iter().any(|label| label == &finding.label) { + labels.push(finding.label); + } + } + } + labels +} + fn source_trust_label(trust: MemorySourceTrust) -> &'static str { match trust { MemorySourceTrust::SystemPolicy => "system-policy", @@ -960,6 +998,50 @@ MEMORY: code_pattern | 7 | Uses builder pattern"; ); } + #[tokio::test] + async fn hosted_secret_memory_writes_redaction_candidate_without_secret_value() { + let dir = tempfile::tempdir().unwrap(); + let target = dir.path().join(".coven-code").join("AGENTS.md"); + let candidate_store = MemoryCandidateStore::for_working_dir(dir.path()); + let secret = format!("ghp_{}", "A".repeat(36)); + let memories = vec![ExtractedMemory { + content: format!("Store token {secret} for later"), + category: MemoryCategory::Constraint, + confidence: 0.9, + source_trust: MemorySourceTrust::Unknown, + }]; + let config = HostedReviewConfig { + enabled: true, + allow_auto_memory_persistence: true, + memory_source_trust: MemorySourceTrust::MaintainerApproved, + ..Default::default() + }; + + let outcome = SessionMemoryExtractor::persist_with_policy( + &memories, + &target, + &candidate_store, + RuntimeMode::HostedReview, + &config, + "session:test-session;source:session-memory-extraction", + ) + .await + .unwrap(); + + assert!(matches!( + outcome, + MemoryPersistenceOutcome::CandidatesWritten { .. } + )); + assert!(!target.exists()); + let candidate_dir = dir.path().join(".coven-code").join("memory-candidates"); + let mut entries = fs::read_dir(&candidate_dir).await.unwrap(); + let entry = entries.next_entry().await.unwrap().unwrap(); + let candidate_text = fs::read_to_string(entry.path()).await.unwrap(); + assert!(candidate_text.contains("redaction-required")); + assert!(candidate_text.contains("GitHub personal access token")); + assert!(!candidate_text.contains(&secret)); + } + #[tokio::test] async fn hosted_maintainer_session_writes_durable_only_when_policy_allows() { let dir = tempfile::tempdir().unwrap();