Skip to content

Commit 37fc838

Browse files
feat(core): harden hosted memory operations (OpenCoven#102 OpenCoven#103 OpenCoven#109 OpenCoven#111)
Fixes OpenCoven#102. Fixes OpenCoven#103. Fixes OpenCoven#109. Fixes OpenCoven#111. - enforce secret scanning before hosted memory writes and sync - key hosted team-memory sync by structured tenant/repo scope - replace server-wins pull with conflict handling - add retention, deletion, and redaction controls Signed-off-by: Timothy Wayne Gregg <Timothy.Gregg@complete.tech>
1 parent 0b78521 commit 37fc838

7 files changed

Lines changed: 635 additions & 23 deletions

File tree

docs/configuration.md

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -376,6 +376,9 @@ Frontmatter fields:
376376
| `source` | Provenance source kind, for example `manual`, `github_pr`, `github_pr_review`, or `session_memory_extraction`. |
377377
| `source_ref` | Source reference such as `owner/repo#123`, a commit SHA, or another non-secret audit handle. |
378378
| `expires_at` | Optional expiry date in `YYYY-MM-DD` format. Expired hosted memory is ignored. |
379+
| `retention_class` | Optional lifecycle class such as `standard`, `short_lived`, `security`, or `legal_hold`. |
380+
| `redacted_at` | Marks content as redacted. Hosted review keeps the metadata visible but replaces the body with a redaction stub. |
381+
| `deleted_at` | Marks memory as deleted. Hosted review excludes deleted entries from prompt loading. |
379382
| `created_at`, `created_by`, `session_id`, `transcript_ref`, `confidence` | Optional provenance fields for audit and review artifacts. |
380383

381384
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
384387
with memory ids and provenance metadata; findings that rely on memory should
385388
include those ids in `memory_refs`.
386389

390+
Hosted sync and persistence boundaries run high-confidence secret scanning
391+
before writing or uploading memory. Entries with detected secret patterns are
392+
blocked by default. Logs and review candidates include only pattern labels and
393+
reason codes, not matched secret values. False positives should be handled by
394+
redacting or editing the memory entry before retrying sync.
395+
396+
Hosted team-memory pull is conflict-aware. Local changes are preserved when
397+
both local and remote content changed since the last known server checksum; a
398+
conflict record is written for operator review instead of overwriting local
399+
memory. Hosted team-memory sync also sends tenant, installation, repo, and
400+
domain scope metadata so the server can authorize the full tuple.
401+
387402
### @include directives
388403

389404
AGENTS.md files support `@include` to pull in content from other files:
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
# Hosted Review Phase 5 PR Notes
2+
3+
## Linked issues
4+
5+
Fixes #102.
6+
Fixes #103.
7+
Fixes #111.
8+
Fixes #109.
9+
10+
## Summary
11+
12+
- Enforces high-confidence secret scanning before hosted session-memory persistence or candidate creation, settings sync upload, team-memory upload, and team-memory pull apply.
13+
- Blocks secret-bearing memory by default and records only scanner labels/reason codes, never matched secret values.
14+
- Adds structured hosted team-memory sync scope with tenant id, installation id, repo id, repo full name, and domain metadata.
15+
- Replaces server-wins team-memory pull application with conflict-aware handling that preserves local changes and writes conflict records for both-changed cases.
16+
- Adds lifecycle metadata support for `retention_class`, `redacted_at`, and `deleted_at`.
17+
- 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.
18+
- Documents retention, redaction, secret scanning, and conflict workflows.
19+
20+
## Test evidence
21+
22+
- `cargo fmt --all -- --check`
23+
- `cargo check --workspace`
24+
- `cargo clippy --workspace --all-targets -- -D warnings`
25+
- `cargo test -p claurst-core --lib claudemd --quiet`
26+
- `cargo test -p claurst-core --lib team_memory_sync --quiet`
27+
- `cargo test -p claurst-core --lib settings_sync --quiet`
28+
- `cargo test -p claurst-core --lib memdir --quiet`
29+
- `cargo test -p claurst-query session_memory --quiet`
30+
- `cargo test -p claurst-core --lib --quiet`
31+
- `cargo test --workspace --quiet`
32+
33+
## Risk notes
34+
35+
- 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.
36+
- Conflict records preserve local and remote memory text for operator review, so downstream tooling should apply the same access controls as memory storage.
37+
- Secret scanning is intentionally high-confidence and blocks by default; false positives require operator edit/redaction before retry.

src-rust/crates/core/src/claudemd.rs

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,12 @@ pub struct MemoryFrontmatter {
5252
#[serde(default)]
5353
pub expires_at: Option<String>,
5454
#[serde(default)]
55+
pub retention_class: Option<String>,
56+
#[serde(default)]
57+
pub redacted_at: Option<String>,
58+
#[serde(default)]
59+
pub deleted_at: Option<String>,
60+
#[serde(default)]
5561
pub created_at: Option<String>,
5662
#[serde(default)]
5763
pub created_by: Option<String>,
@@ -191,6 +197,13 @@ pub fn parse_frontmatter(content: &str) -> (MemoryFrontmatter, &str) {
191197
"source" => fm.source = Some(strip_frontmatter_value(&val).to_string()),
192198
"source_ref" => fm.source_ref = Some(strip_frontmatter_value(&val).to_string()),
193199
"expires_at" => fm.expires_at = Some(strip_frontmatter_value(&val).to_string()),
200+
"retention_class" => {
201+
fm.retention_class = Some(strip_frontmatter_value(&val).to_string())
202+
}
203+
"redacted_at" => {
204+
fm.redacted_at = Some(strip_frontmatter_value(&val).to_string())
205+
}
206+
"deleted_at" => fm.deleted_at = Some(strip_frontmatter_value(&val).to_string()),
194207
"created_at" => fm.created_at = Some(strip_frontmatter_value(&val).to_string()),
195208
"created_by" => fm.created_by = Some(strip_frontmatter_value(&val).to_string()),
196209
"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
356369
return false;
357370
}
358371

372+
if file.frontmatter.deleted_at.is_some() {
373+
return false;
374+
}
375+
359376
if matches!(
360377
file.frontmatter.visibility,
361378
Some(MemoryVisibility::SecurityPrivate)
@@ -394,7 +411,11 @@ pub fn memory_id(file: &MemoryFileInfo) -> String {
394411
}
395412

396413
pub fn format_memory_file_for_prompt(file: &MemoryFileInfo, hosted: bool) -> String {
397-
let body = file.content.trim();
414+
let body = if hosted && file.frontmatter.redacted_at.is_some() {
415+
"[REDACTED: memory content removed; retain metadata for audit]"
416+
} else {
417+
file.content.trim()
418+
};
398419
if !hosted {
399420
return body.to_string();
400421
}
@@ -757,6 +778,39 @@ mod tests {
757778
assert!(files.is_empty());
758779
}
759780

781+
#[test]
782+
fn hosted_review_excludes_deleted_memory() {
783+
let project = tempfile::tempdir().unwrap();
784+
std::fs::write(
785+
project.path().join("AGENTS.md"),
786+
"---\ntrust: maintainer_approved\nvisibility: public_review\ndeleted_at: 2026-01-01T00:00:00Z\n---\ndeleted memory",
787+
)
788+
.unwrap();
789+
790+
let files =
791+
load_all_memory_files_with_options(project.path(), &MemoryLoadOptions::hosted_review());
792+
793+
assert!(files.is_empty());
794+
}
795+
796+
#[test]
797+
fn hosted_review_redacts_memory_content_in_prompt() {
798+
let project = tempfile::tempdir().unwrap();
799+
std::fs::write(
800+
project.path().join("AGENTS.md"),
801+
"---\nid: mem_redacted\ntrust: maintainer_approved\nvisibility: public_review\nredacted_at: 2026-01-01T00:00:00Z\n---\noriginal sensitive detail",
802+
)
803+
.unwrap();
804+
805+
let options = MemoryLoadOptions::hosted_review();
806+
let files = load_all_memory_files_with_options(project.path(), &options);
807+
let prompt = build_memory_prompt_with_options(&files, &options);
808+
809+
assert!(prompt.contains("id=\"mem_redacted\""));
810+
assert!(prompt.contains("[REDACTED: memory content removed"));
811+
assert!(!prompt.contains("original sensitive detail"));
812+
}
813+
760814
#[test]
761815
fn hosted_review_excludes_security_private_memory_by_default() {
762816
let project = tempfile::tempdir().unwrap();

src-rust/crates/core/src/memdir.rs

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -410,6 +410,26 @@ fn hosted_memory_path(scope: &HostedReviewScope) -> PathBuf {
410410
.join("memory")
411411
}
412412

413+
pub fn hosted_memory_path_for_scope(scope: &HostedReviewScope) -> PathBuf {
414+
hosted_memory_path(scope)
415+
}
416+
417+
pub fn delete_hosted_memory_for_scope(scope: &HostedReviewScope) -> std::io::Result<()> {
418+
let path = hosted_memory_path(scope);
419+
if path.exists() {
420+
std::fs::remove_dir_all(path)?;
421+
}
422+
Ok(())
423+
}
424+
425+
pub fn redact_memory_file(path: &Path, reason: &str) -> std::io::Result<()> {
426+
let timestamp = chrono::Utc::now().to_rfc3339();
427+
let stub = format!(
428+
"---\nredacted_at: {timestamp}\nretention_class: security\nsource: redaction\n---\n\n[REDACTED: {reason}]\n"
429+
);
430+
std::fs::write(path, stub)
431+
}
432+
413433
/// Sanitize an arbitrary string into a directory-name-safe component.
414434
/// Matches `sanitizePath` used inside `getAutoMemPath` in `paths.ts`.
415435
pub fn sanitize_path_component(s: &str) -> String {
@@ -1029,4 +1049,46 @@ mod tests {
10291049
assert_ne!(first, branch);
10301050
assert!(branch.to_string_lossy().contains("branch-feature_review"));
10311051
}
1052+
1053+
#[test]
1054+
fn hosted_memory_delete_removes_scope_directory() {
1055+
let home = tempfile::tempdir().unwrap();
1056+
let _lock = crate::coven_shared::COVEN_HOME_ENV_LOCK
1057+
.lock()
1058+
.unwrap_or_else(|err| err.into_inner());
1059+
let original_test_home = std::env::var("COVEN_CODE_TEST_HOME").ok();
1060+
std::env::set_var("COVEN_CODE_TEST_HOME", home.path());
1061+
let scope = crate::hosted_review::HostedReviewScope::new(
1062+
"tenant-delete".to_string(),
1063+
"install-delete".to_string(),
1064+
"repo-delete".to_string(),
1065+
"OpenCoven/coven-code".to_string(),
1066+
);
1067+
let path = hosted_memory_path_for_scope(&scope);
1068+
std::fs::create_dir_all(&path).unwrap();
1069+
std::fs::write(path.join("MEMORY.md"), "delete me").unwrap();
1070+
1071+
delete_hosted_memory_for_scope(&scope).unwrap();
1072+
1073+
match original_test_home {
1074+
Some(value) => std::env::set_var("COVEN_CODE_TEST_HOME", value),
1075+
None => std::env::remove_var("COVEN_CODE_TEST_HOME"),
1076+
}
1077+
1078+
assert!(!path.exists());
1079+
}
1080+
1081+
#[test]
1082+
fn redact_memory_file_preserves_audit_stub_without_original_content() {
1083+
let tmp = tempfile::tempdir().unwrap();
1084+
let path = tmp.path().join("MEMORY.md");
1085+
std::fs::write(&path, "secret incident detail").unwrap();
1086+
1087+
redact_memory_file(&path, "operator request").unwrap();
1088+
1089+
let content = std::fs::read_to_string(&path).unwrap();
1090+
assert!(content.contains("redacted_at:"));
1091+
assert!(content.contains("[REDACTED: operator request]"));
1092+
assert!(!content.contains("secret incident detail"));
1093+
}
10321094
}

src-rust/crates/core/src/settings_sync.rs

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
// and values are the UTF-8 file contents (JSON or Markdown).
1515

1616
use crate::hosted_review::{hosted_project_id, HostedReviewScope};
17+
use crate::team_memory_sync::scan_for_secrets;
1718
use anyhow::Result;
1819
use serde::Deserialize;
1920
use serde_json::Value;
@@ -288,6 +289,8 @@ impl SettingsSyncManager {
288289
///
289290
/// Compares with existing remote entries and only uploads changed keys.
290291
pub async fn upload(&self, local_entries: HashMap<String, String>) -> Result<()> {
292+
let local_entries = filter_entries_with_secrets(local_entries, "Settings sync");
293+
291294
// Fetch current remote state for diff
292295
let remote_entries = match self.download().await? {
293296
Some(data) => data.memory_files,
@@ -352,6 +355,31 @@ impl SettingsSyncManager {
352355
}
353356
}
354357

358+
fn filter_entries_with_secrets(
359+
entries: HashMap<String, String>,
360+
context: &str,
361+
) -> HashMap<String, String> {
362+
entries
363+
.into_iter()
364+
.filter_map(|(key, value)| {
365+
let secrets = scan_for_secrets(&value);
366+
if secrets.is_empty() {
367+
return Some((key, value));
368+
}
369+
370+
let labels: Vec<&str> = secrets.iter().map(|m| m.label.as_str()).collect();
371+
warn!(
372+
"{}: blocking {:?} from upload: detected {} ({} secret pattern(s))",
373+
context,
374+
key,
375+
labels.join(", "),
376+
labels.len(),
377+
);
378+
None
379+
})
380+
.collect()
381+
}
382+
355383
// ---------------------------------------------------------------------------
356384
// Apply result
357385
// ---------------------------------------------------------------------------
@@ -543,6 +571,20 @@ mod tests {
543571
assert!(data.memory_files.is_empty());
544572
}
545573

574+
#[test]
575+
fn filter_entries_with_secrets_blocks_secret_values() {
576+
let mut entries = HashMap::new();
577+
let secret = format!("ghp_{}", "A".repeat(36));
578+
entries.insert(SYNC_KEY_USER_MEMORY.to_string(), format!("token={secret}"));
579+
entries.insert("safe.md".to_string(), "# Safe".to_string());
580+
581+
let filtered = filter_entries_with_secrets(entries, "test");
582+
583+
assert!(filtered.contains_key("safe.md"));
584+
assert!(!filtered.contains_key(SYNC_KEY_USER_MEMORY));
585+
assert!(!filtered.values().any(|value| value.contains(&secret)));
586+
}
587+
546588
#[test]
547589
fn test_retry_delay_progression() {
548590
assert_eq!(retry_delay(1), Duration::from_secs(1));

0 commit comments

Comments
 (0)