diff --git a/crates/server/src/main.rs b/crates/server/src/main.rs index 4bdaf18..aff6bd0 100644 --- a/crates/server/src/main.rs +++ b/crates/server/src/main.rs @@ -11,7 +11,9 @@ use tower_http::trace::TraceLayer; use tracing_subscriber::EnvFilter; use coven_github_config::Config; -use coven_github_webhook::routes::{handle_webhook, list_memory, list_tasks, AppState}; +use coven_github_webhook::routes::{ + handle_webhook, list_memory, list_tasks, revoke_memory, AppState, +}; use coven_github_worker as worker; #[derive(Parser)] @@ -120,6 +122,7 @@ async fn main() -> Result<()> { .route("/webhook", post(handle_webhook)) .route("/api/github/tasks", get(list_tasks)) .route("/api/github/memory", get(list_memory)) + .route("/api/github/memory/revoke", post(revoke_memory)) .with_state(state) .layer(TraceLayer::new_for_http()); diff --git a/crates/store/src/lib.rs b/crates/store/src/lib.rs index cf0f09c..2a16387 100644 --- a/crates/store/src/lib.rs +++ b/crates/store/src/lib.rs @@ -15,7 +15,7 @@ use std::path::Path; use std::sync::{Arc, Mutex}; /// Current schema version, stored in `PRAGMA user_version`. -const SCHEMA_VERSION: i32 = 4; +const SCHEMA_VERSION: i32 = 5; /// Handle to the durable store. Cheap to clone; all clones share one writer /// connection. @@ -562,6 +562,74 @@ impl Store { } } +impl Store { + /// Records a memory revocation for an installation/repo (issue #6). The + /// target is a key or prefix; the worker refuses matching reads/writes and + /// forwards it to the runtime as a denial list. + pub async fn record_revocation( + &self, + installation_id: u64, + repo: &str, + target: &str, + ) -> Result<()> { + let conn = self.conn.clone(); + let (repo, target) = (repo.to_string(), target.to_string()); + tokio::task::spawn_blocking(move || { + let conn = conn.lock().expect("store mutex poisoned"); + conn.execute( + "INSERT INTO memory_revocations (at, installation_id, repo, target) + VALUES (?1, ?2, ?3, ?4)", + params![now_rfc3339(), installation_id, repo, target], + )?; + Ok(()) + }) + .await + .expect("store task panicked") + } + + /// Revoked targets for an installation/repo, for the policy denial list. + pub async fn revocations_for(&self, installation_id: u64, repo: &str) -> Result> { + let conn = self.conn.clone(); + let repo = repo.to_string(); + tokio::task::spawn_blocking(move || { + let conn = conn.lock().expect("store mutex poisoned"); + let mut stmt = conn.prepare( + "SELECT target FROM memory_revocations + WHERE installation_id = ?1 AND repo = ?2 + ORDER BY id", + )?; + let rows = stmt + .query_map(params![installation_id, repo], |row| row.get::<_, String>(0))? + .collect::, _>>()?; + Ok(rows) + }) + .await + .expect("store task panicked") + } + + /// Purges all memory records for an installation (issue #6 delete-on- + /// uninstall). Returns the number of activity + revocation rows removed. + pub async fn delete_memory_for_installation(&self, installation_id: u64) -> Result { + let conn = self.conn.clone(); + tokio::task::spawn_blocking(move || { + let mut conn = conn.lock().expect("store mutex poisoned"); + let tx = conn.transaction()?; + let a = tx.execute( + "DELETE FROM memory_activity WHERE installation_id = ?1", + params![installation_id], + )?; + let r = tx.execute( + "DELETE FROM memory_revocations WHERE installation_id = ?1", + params![installation_id], + )?; + tx.commit()?; + Ok(a + r) + }) + .await + .expect("store task panicked") + } +} + /// One recorded memory operation for the inspect/audit trail (issue #6). #[derive(Debug, Clone, PartialEq, Eq, serde::Serialize)] pub struct MemoryActivity { @@ -731,6 +799,24 @@ fn migrate(conn: &Connection) -> Result<()> { ) .context("failed to apply schema v4")?; } + if version < 5 { + // v5: memory revocations (issue #6). A revoked key/prefix is refused on + // future reads/writes and passed to the runtime as a denial list. + conn.execute_batch( + r#" + CREATE TABLE memory_revocations ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + at TEXT NOT NULL, + installation_id INTEGER NOT NULL, + repo TEXT NOT NULL, + target TEXT NOT NULL + ); + CREATE INDEX memory_revocations_scope + ON memory_revocations(installation_id, repo); + "#, + ) + .context("failed to apply schema v5")?; + } conn.pragma_update(None, "user_version", SCHEMA_VERSION)?; Ok(()) } @@ -883,6 +969,58 @@ mod tests { assert!(!all[0].at.is_empty(), "the store stamps the timestamp"); } + #[tokio::test] + async fn revocations_are_recorded_and_scoped() { + let store = Store::open_in_memory().unwrap(); + store + .record_revocation(1, "OpenCoven/billing", "repo/OpenCoven/billing/secrets/") + .await + .unwrap(); + store + .record_revocation(1, "OpenCoven/web", "repo/OpenCoven/web/x") + .await + .unwrap(); + + let billing = store.revocations_for(1, "OpenCoven/billing").await.unwrap(); + assert_eq!(billing, vec!["repo/OpenCoven/billing/secrets/"]); + // A different repo, and a different installation, are isolated. + assert!(store.revocations_for(1, "OpenCoven/other").await.unwrap().is_empty()); + assert!(store.revocations_for(2, "OpenCoven/billing").await.unwrap().is_empty()); + } + + #[tokio::test] + async fn delete_on_uninstall_purges_activity_and_revocations() { + let store = Store::open_in_memory().unwrap(); + store + .record_memory_activity(vec![memory_row(1, "OpenCoven/a", "repo/OpenCoven/a/x", "accepted")]) + .await + .unwrap(); + store.record_revocation(1, "OpenCoven/a", "repo/OpenCoven/a/x").await.unwrap(); + // Another installation is untouched. + store + .record_memory_activity(vec![memory_row(2, "OpenCoven/b", "repo/OpenCoven/b/y", "accepted")]) + .await + .unwrap(); + + let removed = store.delete_memory_for_installation(1).await.unwrap(); + assert_eq!(removed, 2, "one activity + one revocation"); + assert!(store + .list_memory(Some(ApiScope { installation_id: 1, repos: None })) + .await + .unwrap() + .is_empty()); + assert!(store.revocations_for(1, "OpenCoven/a").await.unwrap().is_empty()); + // Installation 2 survives. + assert_eq!( + store + .list_memory(Some(ApiScope { installation_id: 2, repos: None })) + .await + .unwrap() + .len(), + 1 + ); + } + #[tokio::test] async fn list_memory_is_scoped_to_installation_and_repo() { let store = Store::open_in_memory().unwrap(); diff --git a/crates/webhook/src/routes.rs b/crates/webhook/src/routes.rs index f86e1bf..9742f3e 100644 --- a/crates/webhook/src/routes.rs +++ b/crates/webhook/src/routes.rs @@ -181,6 +181,109 @@ pub async fn list_memory(State(state): State, headers: HeaderMap) -> i } } +/// Body of a memory revocation request (issue #6). +#[derive(serde::Deserialize)] +pub struct RevokeRequest { + /// `owner/name`. Must be within the caller's scope. + pub repo: String, + /// The memory key or prefix to revoke. + pub key: String, + /// Required for service/open callers; ignored for tenants (taken from the + /// token so a tenant can only ever revoke its own installation). + pub installation_id: Option, +} + +/// POST /api/github/memory/revoke — revoke a memory key/prefix for a repo +/// (issue #6). A tenant may only revoke within its own installation (and +/// repositories, when scoped); the operator service token may revoke any +/// installation named in the body. Every revocation is audited. +pub async fn revoke_memory( + State(state): State, + headers: HeaderMap, + Json(req): Json, +) -> impl IntoResponse { + let action = "revoke_memory"; + let (caller, installation_id) = match authorize_api(&state.config.api, &headers) { + ApiCaller::Denied => { + if let Err(e) = state + .store + .record_api_read("anonymous", "none", action, "denied") + .await + { + error!("api audit write failed: {e:#}"); + } + return ( + StatusCode::UNAUTHORIZED, + Json(json!({"ok": false, "error": "unauthorized"})), + ) + .into_response(); + } + ApiCaller::Tenant(tenant) => { + // A scoped tenant may only revoke within its own repositories. + if !tenant.repos.is_empty() && !tenant.repos.contains(&req.repo) { + let _ = state + .store + .record_api_read( + &format!("tenant:{}", tenant.installation_id), + &req.repo, + action, + "forbidden", + ) + .await; + return ( + StatusCode::FORBIDDEN, + Json(json!({"ok": false, "error": "repository not in scope"})), + ) + .into_response(); + } + ( + format!("tenant:{}", tenant.installation_id), + tenant.installation_id, + ) + } + // Operator-wide callers must name the installation explicitly. + ApiCaller::Service | ApiCaller::Open => match req.installation_id { + Some(id) => ("service".to_string(), id), + None => { + return ( + StatusCode::BAD_REQUEST, + Json(json!({"ok": false, "error": "installation_id required"})), + ) + .into_response() + } + }, + }; + + match state + .store + .record_revocation(installation_id, &req.repo, &req.key) + .await + { + Ok(()) => { + if let Err(e) = state + .store + .record_api_read(&caller, &req.repo, action, "ok") + .await + { + error!("api audit write failed: {e:#}"); + } + Json(json!({"ok": true, "revoked": req.key})).into_response() + } + Err(e) => { + error!("revocation failed: {e:#}"); + let _ = state + .store + .record_api_read(&caller, &req.repo, action, "error") + .await; + ( + StatusCode::INTERNAL_SERVER_ERROR, + Json(json!({"ok": false, "error": "revocation failed"})), + ) + .into_response() + } + } +} + /// Resolved identity of a task-API caller. enum ApiCaller<'a> { /// `open` mode: unauthenticated local development. @@ -332,6 +435,33 @@ pub async fn handle_webhook( }; } + // Installation removed → purge that tenant's durable memory records + // (issue #6, delete-on-uninstall). Idempotent via the delivery id. + if event_type == "installation" && payload.action.as_deref() == Some("deleted") { + let installation_id = payload.installation.as_ref().map(|i| i.id); + return match state + .store + .record_delivery(delivery, Routing::Ignored("installation_deleted")) + .await + { + Ok(Recorded::New) => { + if let Some(id) = installation_id { + match state.store.delete_memory_for_installation(id).await { + Ok(purged) => { + info!(installation_id = id, purged, "purged memory on uninstall") + } + Err(e) => error!("failed to purge memory on uninstall: {e:#}"), + } + } + (StatusCode::OK, Json(json!({"ok": true}))).into_response() + } + Ok(Recorded::Duplicate) => { + (StatusCode::OK, Json(json!({"ok": true, "duplicate": true}))).into_response() + } + Err(e) => storage_unavailable(e), + }; + } + // 4. Route event → task, then persist BEFORE acknowledging: GitHub must // only hear success once durable state exists, and a redelivered // delivery id must never dispatch twice (issue #2). @@ -1472,6 +1602,52 @@ mod delivery_idempotency_tests { assert!(state.store.task_states().await.unwrap().is_empty()); } + #[tokio::test] + async fn installation_deleted_purges_only_that_installations_memory() { + let state = app_state(); + let mem = |installation_id: u64| coven_github_store::MemoryActivity { + at: String::new(), + installation_id, + repo: "OpenCoven/demo".to_string(), + task_id: "t".to_string(), + op: "read".to_string(), + target: "repo/OpenCoven/demo/x".to_string(), + scope: "repo".to_string(), + outcome: "accepted".to_string(), + }; + state.store.record_memory_activity(vec![mem(7)]).await.unwrap(); + state.store.record_memory_activity(vec![mem(99)]).await.unwrap(); + state + .store + .record_revocation(7, "OpenCoven/demo", "repo/OpenCoven/demo/x") + .await + .unwrap(); + + let body = serde_json::json!({ "action": "deleted", "installation": { "id": 7 } }) + .to_string(); + let (status, _) = + call(&state, signed_headers("installation", Some("del-1"), &body), &body).await; + assert_eq!(status, StatusCode::OK); + + // Installation 7's memory and revocations are purged; 99 is untouched. + assert!(state + .store + .list_memory(Some(ApiScope { installation_id: 7, repos: None })) + .await + .unwrap() + .is_empty()); + assert!(state.store.revocations_for(7, "OpenCoven/demo").await.unwrap().is_empty()); + assert_eq!( + state + .store + .list_memory(Some(ApiScope { installation_id: 99, repos: None })) + .await + .unwrap() + .len(), + 1 + ); + } + #[tokio::test] async fn redelivered_delivery_id_never_dispatches_twice() { let state = app_state(); @@ -1746,6 +1922,77 @@ mod tenancy_tests { assert_eq!(json["memory"].as_array().unwrap().len(), 2); } + async fn revoke( + state: &AppState, + bearer: Option<&str>, + repo: &str, + key: &str, + ) -> (StatusCode, serde_json::Value) { + let mut headers = HeaderMap::new(); + if let Some(token) = bearer { + headers.insert( + "authorization", + format!("Bearer {token}").parse().expect("header"), + ); + } + let response = revoke_memory( + State(state.clone()), + headers, + Json(RevokeRequest { + repo: repo.to_string(), + key: key.to_string(), + installation_id: None, + }), + ) + .await + .into_response(); + let status = response.status(); + let bytes = axum::body::to_bytes(response.into_body(), usize::MAX) + .await + .expect("body"); + (status, serde_json::from_slice(&bytes).expect("json")) + } + + #[tokio::test] + async fn revoke_is_tenant_scoped_and_persists() { + let mut api = two_tenant_api(); + api.tenants[0].repos = vec!["OpenCoven/alpha".to_string()]; + let state = token_state(api); + + // In-scope revocation succeeds and is stored. + let (status, json) = revoke( + &state, + Some("tenant-one-0123456789abcdef"), + "OpenCoven/alpha", + "repo/OpenCoven/alpha/secrets/", + ) + .await; + assert_eq!(status, StatusCode::OK); + assert_eq!(json["ok"], true); + assert_eq!( + state + .store + .revocations_for(1, "OpenCoven/alpha") + .await + .unwrap(), + vec!["repo/OpenCoven/alpha/secrets/"] + ); + + // A repo outside the tenant's scope is forbidden. + let (status, _) = revoke( + &state, + Some("tenant-one-0123456789abcdef"), + "OpenCoven/beta", + "k", + ) + .await; + assert_eq!(status, StatusCode::FORBIDDEN); + + // No credential fails closed. + let (status, _) = revoke(&state, None, "OpenCoven/alpha", "k").await; + assert_eq!(status, StatusCode::UNAUTHORIZED); + } + #[tokio::test] async fn tenant_repo_scope_narrows_within_the_installation() { let mut api = two_tenant_api(); diff --git a/crates/worker/src/lib.rs b/crates/worker/src/lib.rs index 5629ddc..e5d2fb5 100644 --- a/crates/worker/src/lib.rs +++ b/crates/worker/src/lib.rs @@ -481,7 +481,13 @@ async fn execute_task_with_minter( number, ); let mut body = - final_status_body(config, &task.id, &published.result, published.opened_pr); + final_status_body( + config, + &task.id, + &published.result, + published.opened_pr, + &published.cited_memory, + ); if let Some(report) = &advisory { body = format!("{body}\n\n{report}"); } @@ -586,6 +592,9 @@ struct Published { /// at `current_sha`. Publication must mark the task superseded instead of /// presenting stale findings as current. stale: Option, + /// Memory entry ids the review read and the adapter accepted — cited on + /// the status surface for transparency (issue #6). + cited_memory: Vec, } /// Evidence of a mid-session head move on a reviewed PR. @@ -747,6 +756,19 @@ async fn run_and_publish( // review target and actor: a fork PR is untrusted content and can never // write durable memory, overriding even a maintainer trigger. let repo_full = format!("{}/{}", task.repo_owner, task.repo_name); + // Revoked memory for this repo (issue #6): the adapter refuses these on the + // result and passes them to the runtime so it stops surfacing them. + let denied = if config.memory.enabled_for(&repo_full) { + store + .revocations_for(task.installation_id, &repo_full) + .await + .unwrap_or_else(|e| { + warn!(task_id = %task.id, "failed to load memory revocations: {e:#}"); + Vec::new() + }) + } else { + Vec::new() + }; let memory_policy = memory::compute_policy(memory::PolicyInputs { enabled: config.memory.enabled_for(&repo_full), installation_id: task.installation_id, @@ -754,6 +776,7 @@ async fn run_and_publish( trust: memory::derive_trust(targets.head_is_fork, task.commander.is_some()), approval_required: config.memory.approval_required_for(&repo_full), retention_days: config.memory.retention_days, + denied, }); let brief = brief::build( task, @@ -838,6 +861,7 @@ async fn run_and_publish( // granted (issue #6). The runtime's self-report is not trusted on its own: // any read or write outside scope — including a fork PR that tried to write // durable memory — is refused here before it can be persisted. + let mut cited_memory: Vec = Vec::new(); if let (Some(policy), Some(used)) = (&memory_policy, &result.memory_used) { let rejections = memory::validate_memory_used(policy, used, |text| redact::redact(text, &[]) != text); @@ -855,6 +879,18 @@ async fn run_and_publish( if let Err(e) = store.record_memory_activity(activity).await { warn!(task_id = %task.id, "failed to record memory activity: {e:#}"); } + // Cite the reads the adapter accepted (not refused/revoked) so the + // review discloses which memory influenced it (issue #6). + cited_memory = used + .read + .iter() + .filter(|r| { + !rejections + .iter() + .any(|rj| rj.op == memory::MemoryOp::Read && rj.target == r.id) + }) + .map(|r| r.id.clone()) + .collect(); } // Stale-ref gate (issue #8): review findings are only valid for the head @@ -886,6 +922,7 @@ async fn run_and_publish( reviewed_sha: targets.head_sha.clone(), current_sha: refs.head_sha, }), + cited_memory, }); } } @@ -949,6 +986,7 @@ async fn run_and_publish( opened_pr, changed_files: review.map(|r| r.files).unwrap_or_default(), stale: None, + cited_memory, }) } @@ -1302,9 +1340,10 @@ fn final_status_body( task_id: &str, result: &SessionResult, opened_pr: Option, + cited_memory: &[String], ) -> String { let session = cave_session_url(config, task_id); - match result.status { + let body = match result.status { SessionStatus::NeedsInput => format!( "Status: needs input\n\n{}\n\nReply on this thread to continue. Session: {session}", result.summary @@ -1323,6 +1362,17 @@ fn final_status_body( result.summary ), }, + }; + // Disclose which memory entries influenced this review (issue #6). + if cited_memory.is_empty() { + body + } else { + let cited = cited_memory + .iter() + .map(|id| format!("`{id}`")) + .collect::>() + .join(", "); + format!("{body}\n\nMemory used: {cited}") } } @@ -1900,7 +1950,7 @@ mod disposition_tests { exit_reason: None, memory_used: None, }; - let done = final_status_body(&config, "task-42", &result, Some(17)); + let done = final_status_body(&config, "task-42", &result, Some(17), &[]); assert!(done.starts_with("Status: done")); assert!(done.contains("PR #17 opened")); assert!(done.contains("https://cave.example.test/sessions/task-42")); @@ -1908,10 +1958,21 @@ mod disposition_tests { !done.contains('✅') && !done.contains('→'), "status body should stay concise and actionable: {done}" ); + assert!(!done.contains("Memory used"), "no citation without memory"); + + // A review that read memory cites the entries it used (issue #6). + let cited = final_status_body( + &config, + "task-42", + &result, + Some(17), + &["repo/acme/billing/conventions/x".to_string()], + ); + assert!(cited.contains("Memory used: `repo/acme/billing/conventions/x`")); let mut needs_input = result.clone(); needs_input.status = SessionStatus::NeedsInput; - let waiting = final_status_body(&config, "task-42", &needs_input, None); + let waiting = final_status_body(&config, "task-42", &needs_input, None, &[]); assert!(waiting.starts_with("Status: needs input")); assert!(waiting.contains("Reply on this thread")); } diff --git a/crates/worker/src/memory.rs b/crates/worker/src/memory.rs index 066980b..3927c11 100644 --- a/crates/worker/src/memory.rs +++ b/crates/worker/src/memory.rs @@ -64,6 +64,12 @@ pub struct MemoryPolicy { pub approval_required: bool, #[serde(skip_serializing_if = "Option::is_none")] pub retention_days: Option, + /// Revoked memory keys/prefixes (issue #6). The adapter refuses to read or + /// write anything matching one of these, and passes them to the runtime so + /// it can stop surfacing them — but the adapter's refusal is the guarantee, + /// not the runtime's cooperation. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub denied: Vec, } /// Inputs the adapter derives before computing a policy. @@ -76,6 +82,8 @@ pub struct PolicyInputs<'a> { pub trust: TrustScope, pub approval_required: bool, pub retention_days: Option, + /// Revoked key prefixes for this installation/repo. + pub denied: Vec, } /// Computes the memory policy for one invocation, **deny-by-default**: returns @@ -100,6 +108,7 @@ pub fn compute_policy(input: PolicyInputs) -> Option { write_scopes, approval_required, retention_days: input.retention_days, + denied: input.denied, }) } @@ -140,6 +149,13 @@ fn approval_forced(trust: TrustScope) -> bool { matches!(trust, TrustScope::Collaborator) } +/// True when `target` falls under a revoked key/prefix. A denied entry matches +/// its exact key and everything beneath it, so revoking `repo/o/r/` clears the +/// whole repo namespace and revoking a full key clears just that entry. +fn is_revoked(policy: &MemoryPolicy, target: &str) -> bool { + policy.denied.iter().any(|prefix| target.starts_with(prefix)) +} + /// Why the adapter refused a memory read or write. #[derive(Debug, Clone, PartialEq, Eq)] pub struct MemoryRejection { @@ -173,6 +189,14 @@ pub fn validate_memory_used( } for entry in &used.read { + if is_revoked(policy, &entry.id) { + rejections.push(MemoryRejection { + op: MemoryOp::Read, + target: entry.id.clone(), + reason: "memory has been revoked".to_string(), + }); + continue; + } if let Some(reason) = check_access(policy, &entry.scope, &entry.id, &policy.read_scopes) { rejections.push(MemoryRejection { op: MemoryOp::Read, @@ -183,6 +207,14 @@ pub fn validate_memory_used( } for write in &used.proposed { + if is_revoked(policy, &write.key) { + rejections.push(MemoryRejection { + op: MemoryOp::Write, + target: write.key.clone(), + reason: "memory has been revoked".to_string(), + }); + continue; + } if let Some(reason) = check_access(policy, &write.scope, &write.key, &policy.write_scopes) { rejections.push(MemoryRejection { op: MemoryOp::Write, @@ -260,6 +292,7 @@ mod tests { trust, approval_required: false, retention_days: Some(365), + denied: vec![], } } @@ -320,6 +353,46 @@ mod tests { ); } + #[test] + fn revoked_memory_is_refused_for_read_and_write() { + let policy = compute_policy(PolicyInputs { + denied: vec!["repo/acme/billing/secrets/".to_string()], + ..inputs(TrustScope::Maintainer, true) + }) + .unwrap(); + + let used = used_with( + // Write under the revoked prefix. + vec![proposed("repo", "repo/acme/billing/secrets/api", "pending")], + // Read under the revoked prefix. + vec![MemoryEntryRef { + id: "repo/acme/billing/secrets/db".to_string(), + scope: "repo".to_string(), + }], + ); + let rejections = validate_memory_used(&policy, &used, never_secret); + assert_eq!(rejections.len(), 2, "both the read and write are revoked"); + assert!(rejections.iter().all(|r| r.reason.contains("revoked"))); + + // A non-revoked key under the same repo is still fine. + let ok = used_with( + vec![proposed("repo", "repo/acme/billing/conventions/x", "pending")], + vec![], + ); + assert!(validate_memory_used(&policy, &ok, never_secret).is_empty()); + } + + #[test] + fn revoked_prefixes_serialize_into_the_policy_for_the_runtime() { + let policy = compute_policy(PolicyInputs { + denied: vec!["repo/acme/billing/x".to_string()], + ..inputs(TrustScope::Maintainer, true) + }) + .unwrap(); + let value = serde_json::to_value(&policy).unwrap(); + assert_eq!(value["denied"][0], "repo/acme/billing/x"); + } + #[test] fn fork_review_overrides_actor_trust() { // A fork PR is untrusted content: even a maintainer command reviewing it diff --git a/docs/contracts/session-brief.schema.json b/docs/contracts/session-brief.schema.json index 95846d7..a74fd1b 100644 --- a/docs/contracts/session-brief.schema.json +++ b/docs/contracts/session-brief.schema.json @@ -134,7 +134,12 @@ "items": { "type": "string", "enum": ["repo", "tenant_shared", "familiar_global"] } }, "approval_required": { "type": "boolean" }, - "retention_days": { "type": ["integer", "null"] } + "retention_days": { "type": ["integer", "null"] }, + "denied": { + "type": "array", + "description": "Revoked memory keys/prefixes the runtime must not read or surface (issue #6). The adapter also refuses them on the result.", + "items": { "type": "string" } + } } } } diff --git a/docs/memory-contract.md b/docs/memory-contract.md index 13355c7..6d4ab46 100644 --- a/docs/memory-contract.md +++ b/docs/memory-contract.md @@ -311,14 +311,20 @@ values are sane. ## Phased PRs (coordinated with coven-code) 1. **Contract + schemas.** This doc, plus the additive `memory_policy` / - `memory_used` schema fields and Rust types, landed in lockstep with the - coven-code side that accepts/emits them. No behavior yet. + `memory_used` schema fields and Rust types. **Done** (adapter side; the + coven-code side accepts/emits them independently). 2. **Adapter policy + enforcement.** `[memory]` config, deny-by-default brief - stamping with trust gating, and result-envelope validation (reject - out-of-scope/unredacted/non-prefixed writes). Wires the #13 `remember` / - `forget` approval path. -3. **Inspect / revoke.** Cave surface over prefix-addressable keys (needs #3); - `delete_on_uninstall`; retention expiry. - -Each PR keeps `cargo check/clippy/test` green and lands separately mergeable; -Phase 1's schema change is bilateral and MUST NOT ship on one side alone. + stamping with trust gating (incl. fork-PR never-write), and result-envelope + validation. **Done.** +3. **Inspect / revoke.** Tenant-scoped `GET /api/github/memory` inspect over + prefix-addressable keys; `POST /api/github/memory/revoke` with adapter-side + enforcement (revoked keys refused on future reads/writes) plus a denial list + forwarded to the runtime; `delete_on_uninstall`; read citation on the review + surface. **Done**, except **retention expiry**, which needs a periodic sweep + (an operational enhancement, not a contract requirement) and remains a + follow-up. + +The adapter side is complete. What is inherently bilateral — the runtime +honoring the `denied` list and physically deleting revoked bytes — is the +coven-code counterpart; the adapter's refusal is the standalone guarantee, so a +revoked memory can never influence a review regardless of the runtime.