From ef1741393cb9e1f34374518071244f489194384f Mon Sep 17 00:00:00 2001 From: Val Alexander Date: Tue, 7 Jul 2026 01:58:07 -0500 Subject: [PATCH 1/4] feat(store): task artifact deletion, retention, and audit-scan surface (#12) delete_tasks_for_installation purges a tenant's tasks/attempts/deliveries on uninstall; expire_terminal_tasks(_before) drops terminal tasks past a retention horizon while never touching in-flight work; all_stored_text returns every adapter-generated durable free-text value as the scan surface for the redaction guarantee (user-authored issue/comment content is excluded). Signed-off-by: Val Alexander --- crates/store/src/lib.rs | 182 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 182 insertions(+) diff --git a/crates/store/src/lib.rs b/crates/store/src/lib.rs index 9df1fe3..3878191 100644 --- a/crates/store/src/lib.rs +++ b/crates/store/src/lib.rs @@ -770,6 +770,100 @@ impl Store { let cutoff = (chrono::Utc::now() - chrono::Duration::days(retention_days as i64)).to_rfc3339(); self.expire_memory_activity_before(&cutoff).await } + + /// Purges all task artifacts for an installation — attempts, tasks, and the + /// delivery records — on uninstall (issue #12). Returns rows removed. + pub async fn delete_tasks_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()?; + // Defer FK checks to commit so the multi-table cascade (attempts → + // tasks → deliveries) needn't be perfectly ordered; the whole set is + // consistent once committed. + tx.execute_batch("PRAGMA defer_foreign_keys = ON")?; + let attempts = tx.execute( + "DELETE FROM task_attempts + WHERE task_id IN (SELECT id FROM tasks WHERE installation_id = ?1)", + params![installation_id], + )?; + let tasks = tx.execute( + "DELETE FROM tasks WHERE installation_id = ?1", + params![installation_id], + )?; + let deliveries = tx.execute( + "DELETE FROM webhook_deliveries WHERE installation_id = ?1", + params![installation_id], + )?; + tx.commit()?; + Ok(attempts + tasks + deliveries) + }) + .await + .expect("store task panicked") + } + + /// Deletes terminal (completed / failed / superseded) tasks and their + /// attempts older than `cutoff` (RFC 3339) — task retention (issue #12). + /// In-flight (`queued` / `running`) tasks are never expired. + pub async fn expire_terminal_tasks_before(&self, cutoff: &str) -> Result { + let conn = self.conn.clone(); + let cutoff = cutoff.to_string(); + tokio::task::spawn_blocking(move || { + let mut conn = conn.lock().expect("store mutex poisoned"); + let tx = conn.transaction()?; + tx.execute_batch("PRAGMA defer_foreign_keys = ON")?; + let expiring = "SELECT id FROM tasks \ + WHERE state IN ('completed','failed','superseded') AND updated_at < ?1"; + let attempts = tx.execute( + &format!("DELETE FROM task_attempts WHERE task_id IN ({expiring})"), + params![cutoff], + )?; + let tasks = tx.execute( + "DELETE FROM tasks \ + WHERE state IN ('completed','failed','superseded') AND updated_at < ?1", + params![cutoff], + )?; + tx.commit()?; + Ok(attempts + tasks) + }) + .await + .expect("store task panicked") + } + + /// Expires terminal tasks older than `retention_days` from now. + pub async fn expire_terminal_tasks(&self, retention_days: u32) -> Result { + let cutoff = (chrono::Utc::now() - chrono::Duration::days(retention_days as i64)).to_rfc3339(); + self.expire_terminal_tasks_before(&cutoff).await + } + + /// Every adapter-generated free-text value the store durably retains — the + /// scan surface for the redaction guarantee (issue #12): no raw token or + /// secret may survive here. Deliberately excludes user-authored content + /// (issue/comment bodies in `tasks.kind`), which may legitimately quote + /// token-shaped strings and is never the adapter's to redact. + pub async fn all_stored_text(&self) -> Result> { + let conn = self.conn.clone(); + tokio::task::spawn_blocking(move || { + let conn = conn.lock().expect("store mutex poisoned"); + let mut out = Vec::new(); + for sql in [ + "SELECT summary FROM tasks WHERE summary IS NOT NULL", + "SELECT detail FROM task_attempts WHERE detail IS NOT NULL", + "SELECT routing FROM webhook_deliveries", + "SELECT target FROM memory_activity", + "SELECT target FROM memory_revocations", + ] { + let mut stmt = conn.prepare(sql)?; + let rows = stmt + .query_map([], |row| row.get::<_, String>(0))? + .collect::, _>>()?; + out.extend(rows); + } + Ok(out) + }) + .await + .expect("store task panicked") + } } /// One recorded memory operation for the inspect/audit trail (issue #6). @@ -1431,6 +1525,94 @@ mod queue_tests { assert!(store.claim_next(&Default::default()).await.unwrap().is_none()); } + async fn finish_completed(store: &Store, task_id: &str, summary: &str, detail: Option<&str>) { + store + .finish( + task_id, + Terminal { + state: TerminalState::Completed, + result_status: Some("success".to_string()), + branch: None, + pr_number: None, + summary: Some(summary.to_string()), + detail: detail.map(str::to_string), + }, + ) + .await + .expect("finish"); + } + + #[tokio::test] + async fn delete_tasks_for_installation_purges_that_tenants_artifacts() { + let store = Store::open_in_memory().expect("open"); + enqueue(&store, "d1", &fix_task("t1")).await; // installation 1 + // Installation 2: the delivery and task share the installation (as they + // always do from one webhook payload). + let other = Task { + installation_id: 2, + ..fix_task("t2") + }; + store + .record_delivery( + Delivery { + installation_id: Some(2), + ..delivery("d2") + }, + Routing::Task(&other), + ) + .await + .expect("enqueue"); + // Claim + finish t1 so it has an attempt row too. + store.claim_next(&Default::default()).await.unwrap(); + finish_completed(&store, "t1", "done", Some("some detail")).await; + + let removed = store.delete_tasks_for_installation(1).await.unwrap(); + assert!(removed >= 2, "task + delivery (+ attempt) removed: {removed}"); + + // Installation 1 is gone; installation 2 survives and is still claimable. + assert!(store + .delivery_routing("d1") + .await + .unwrap() + .is_none()); + let claimed = store.claim_next(&Default::default()).await.unwrap().expect("t2 survives"); + assert_eq!(claimed.id, "t2"); + } + + #[tokio::test] + async fn task_retention_expires_terminal_tasks_but_not_in_flight() { + let store = Store::open_in_memory().expect("open"); + enqueue(&store, "d1", &fix_task("done")).await; + enqueue(&store, "d2", &fix_task("queued")).await; + // Make the first terminal; leave the second queued. + store.claim_next(&Default::default()).await.unwrap(); + finish_completed(&store, "done", "s", None).await; + + // A cutoff in the future expires the terminal task, never the queued one. + let removed = store + .expire_terminal_tasks_before("2999-01-01T00:00:00+00:00") + .await + .unwrap(); + // The terminal task plus its attempt row are removed. + assert_eq!(removed, 2); + let claimed = store.claim_next(&Default::default()).await.unwrap().expect("queued survives"); + assert_eq!(claimed.id, "queued"); + } + + #[tokio::test] + async fn all_stored_text_surfaces_adapter_generated_fields() { + let store = Store::open_in_memory().expect("open"); + enqueue(&store, "d1", &fix_task("t1")).await; + store.claim_next(&Default::default()).await.unwrap(); + finish_completed(&store, "t1", "SUMMARY_MARKER", Some("DETAIL_MARKER")).await; + + let text = store.all_stored_text().await.unwrap(); + assert!(text.iter().any(|t| t.contains("SUMMARY_MARKER"))); + assert!(text.iter().any(|t| t.contains("DETAIL_MARKER"))); + // The delivery routing is adapter-generated and included. + assert!(text.iter().any(|t| t.starts_with("task:"))); + } + #[tokio::test] async fn finish_reaches_terminal_state_and_closes_the_attempt() { let store = Store::open_in_memory().expect("open"); From a527ca49f793c9a5e80263bfeabfd6c5acc2a4b1 Mon Sep 17 00:00:00 2001 From: Val Alexander Date: Tue, 7 Jul 2026 02:02:38 -0500 Subject: [PATCH 2/4] feat(config,webhook,server): task retention + purge task artifacts on uninstall (#12) [storage] task_retention_days drives a periodic server sweep of terminal task history; the installation.deleted webhook now purges task artifacts alongside memory and records an audited delete_on_uninstall entry noting how much was removed. Signed-off-by: Val Alexander --- config/example.toml | 1 + crates/config/src/lib.rs | 5 +++++ crates/server/src/main.rs | 20 +++++++++++++++++ crates/webhook/src/routes.rs | 43 +++++++++++++++++++++++++++++++----- 4 files changed, 63 insertions(+), 6 deletions(-) diff --git a/config/example.toml b/config/example.toml index 830507f..4609715 100644 --- a/config/example.toml +++ b/config/example.toml @@ -23,6 +23,7 @@ max_retries = 2 # Retries for infra errors (exit code 2) # Durable adapter state: webhook deliveries (idempotency) and task records. # SQLite file — keep it on a persistent volume; parents are created at start. path = "data/coven-github.db" +# task_retention_days = 90 # expire terminal task history after N days (issue #12); omit to keep forever # ── Task API auth (issue #3) ──────────────────────────────────────────────── # Gate GET /api/github/tasks. "open" = unauthenticated (local development diff --git a/crates/config/src/lib.rs b/crates/config/src/lib.rs index e55274d..c504d26 100644 --- a/crates/config/src/lib.rs +++ b/crates/config/src/lib.rs @@ -247,12 +247,17 @@ pub struct StorageConfig { /// SQLite database path; parent directories are created at startup. #[serde(default = "default_storage_path")] pub path: PathBuf, + /// Days to retain terminal task history before a periodic sweep deletes it + /// (issue #12). Absent = keep indefinitely. In-flight tasks are never + /// expired. + pub task_retention_days: Option, } impl Default for StorageConfig { fn default() -> Self { Self { path: default_storage_path(), + task_retention_days: None, } } } diff --git a/crates/server/src/main.rs b/crates/server/src/main.rs index 00dd1b0..348be73 100644 --- a/crates/server/src/main.rs +++ b/crates/server/src/main.rs @@ -132,6 +132,26 @@ async fn main() -> Result<()> { }); } + // Task-history retention sweep (issue #12): expire terminal tasks + // older than the configured horizon; in-flight work is never touched. + if let Some(retention_days) = config.storage.task_retention_days { + let sweep_store = store.clone(); + tokio::spawn(async move { + let mut ticker = + tokio::time::interval(std::time::Duration::from_secs(6 * 3600)); + loop { + ticker.tick().await; + match sweep_store.expire_terminal_tasks(retention_days).await { + Ok(0) => {} + Ok(expired) => { + tracing::info!(expired, "expired terminal task rows past retention") + } + Err(e) => tracing::error!("task retention sweep failed: {e:#}"), + } + } + }); + } + // Build router. let state = AppState { config: config.clone(), diff --git a/crates/webhook/src/routes.rs b/crates/webhook/src/routes.rs index da2c8c7..f954bea 100644 --- a/crates/webhook/src/routes.rs +++ b/crates/webhook/src/routes.rs @@ -511,12 +511,36 @@ pub async fn handle_webhook( { 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:#}"), - } + // Purge both memory (issue #6) and task artifacts (issue + // #12) for the departing tenant. Idempotent: a redelivery + // finds nothing left to remove. + let memory = state + .store + .delete_memory_for_installation(id) + .await + .unwrap_or_else(|e| { + error!("failed to purge memory on uninstall: {e:#}"); + 0 + }); + let tasks = state + .store + .delete_tasks_for_installation(id) + .await + .unwrap_or_else(|e| { + error!("failed to purge task artifacts on uninstall: {e:#}"); + 0 + }); + info!(installation_id = id, memory, tasks, "purged tenant data on uninstall"); + // Audit what was deleted (issue #12). + let _ = state + .store + .record_api_read( + &format!("installation:{id}"), + &id.to_string(), + "delete_on_uninstall", + &format!("memory:{memory},tasks:{tasks}"), + ) + .await; } (StatusCode::OK, Json(json!({"ok": true}))).into_response() } @@ -1747,6 +1771,13 @@ mod delivery_idempotency_tests { .len(), 1 ); + + // The purge (memory + task artifacts) is audited (issue #12). + let audit = state.store.api_audit_entries().await.unwrap(); + assert!( + audit.iter().any(|(_, _, action, _)| action == "delete_on_uninstall"), + "the uninstall purge must leave an audit record: {audit:?}" + ); } #[tokio::test] From 8de3c29f5dc74480ce2615ca1d0ab095757ce5fe Mon Sep 17 00:00:00 2001 From: Val Alexander Date: Tue, 7 Jul 2026 02:04:36 -0500 Subject: [PATCH 3/4] test(worker): prove no raw token survives in durable stores (#12) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An end-to-end guarantee test: a runtime result smeared with a token is sanitized as the worker does, persisted through store.finish, then every adapter-generated durable text value (task summary, attempt detail, delivery routing, memory targets) is scanned — asserting no raw token and no token pattern survives, with a negative control proving the scanner is not vacuous. Signed-off-by: Val Alexander --- crates/worker/src/lib.rs | 106 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 106 insertions(+) diff --git a/crates/worker/src/lib.rs b/crates/worker/src/lib.rs index 88ff611..32be180 100644 --- a/crates/worker/src/lib.rs +++ b/crates/worker/src/lib.rs @@ -3251,3 +3251,109 @@ mod publication_gate_tests { ); } } + +#[cfg(test)] +mod audit_redaction_tests { + use super::*; + use coven_github_api::{ + CommitInfo, ReviewResult, SessionResult, SessionStatus, Task, TaskKind, + HEADLESS_CONTRACT_VERSION, + }; + use coven_github_store::{Delivery, Routing, Store, Terminal, TerminalState}; + + const TOKEN: &str = "ghs_realTOKENrealTOKENrealTOKEN01"; + + fn result_smeared_with_token() -> SessionResult { + SessionResult { + contract_version: HEADLESS_CONTRACT_VERSION.to_string(), + status: SessionStatus::Success, + branch: Some(format!("cody/leak-{TOKEN}")), + commits: vec![CommitInfo { + sha: "abc".to_string(), + message: format!("commit mentioning {TOKEN}"), + }], + files_changed: vec![], + summary: format!("summary with {TOKEN} inside"), + pr_body: format!("pr body quoting {TOKEN}"), + review: ReviewResult::none(), + exit_reason: None, + memory_used: None, + } + } + + #[tokio::test] + async fn no_raw_token_survives_in_durable_stores() { + // Sanity: the raw result really does carry the token, so the assertions + // below are not vacuous — and the pattern scanner detects it. + let raw = result_smeared_with_token(); + assert!(raw.summary.contains(TOKEN)); + assert_ne!(redact::redact(&raw.summary, &[]), raw.summary); + + // The worker sanitizes the envelope before anything is persisted. + let mut result = result_smeared_with_token(); + redact::sanitize_result(&mut result, &[TOKEN]); + + let store = Store::open_in_memory().unwrap(); + let task = Task { + id: "t1".to_string(), + installation_id: 1, + repo_owner: "OpenCoven".to_string(), + repo_name: "demo".to_string(), + familiar_id: "cody".to_string(), + commander: None, + kind: TaskKind::FixIssue { + issue_number: 42, + issue_title: "t".to_string(), + issue_body: "b".to_string(), + }, + }; + store + .record_delivery( + Delivery { + delivery_id: "d1".to_string(), + event: "issues".to_string(), + action: Some("assigned".to_string()), + installation_id: Some(1), + repo: Some("OpenCoven/demo".to_string()), + payload_hash: "h".to_string(), + }, + Routing::Task(&task), + ) + .await + .unwrap(); + store.claim_next(&Default::default()).await.unwrap(); + // Persist terminal state exactly as the worker does: summary/branch from + // the sanitized envelope, detail through redact. + store + .finish( + "t1", + Terminal { + state: TerminalState::Completed, + result_status: Some("success".to_string()), + branch: result.branch.clone(), + pr_number: None, + summary: Some(result.summary.clone()), + detail: Some(redact::redact(&format!("error leaked {TOKEN}"), &[TOKEN])), + }, + ) + .await + .unwrap(); + + // Scan every adapter-generated durable text field. + let stored = store.all_stored_text().await.unwrap(); + assert!(!stored.is_empty()); + for value in &stored { + assert!(!value.contains(TOKEN), "raw token leaked into store: {value}"); + assert_eq!( + redact::redact(value, &[]), + *value, + "a token pattern survived into a durable artifact: {value}" + ); + } + // Redaction actually happened (not just an empty store). + assert!( + stored.iter().any(|v| v.contains(redact::REDACTED)), + "expected redacted markers in the stored artifacts" + ); + } +} From abd294c67e2e0a1ef59e9841d98b63c8dd20cb67 Mon Sep 17 00:00:00 2001 From: Val Alexander Date: Tue, 7 Jul 2026 02:06:48 -0500 Subject: [PATCH 4/4] docs: data retention, artifact classes, and audit reference (#12) New docs/data-retention.md documents artifact classes and what is retained vs never retained, redaction, the audit-event tables, deletion/retention semantics, and self-hosted vs hosted differences; linked from the security launch gate. Signed-off-by: Val Alexander --- docs/data-retention.md | 87 ++++++++++++++++++++++++++++++++++++++++++ docs/security.md | 2 +- 2 files changed, 88 insertions(+), 1 deletion(-) create mode 100644 docs/data-retention.md diff --git a/docs/data-retention.md b/docs/data-retention.md new file mode 100644 index 0000000..bc28261 --- /dev/null +++ b/docs/data-retention.md @@ -0,0 +1,87 @@ +# Data retention, artifacts, and audit (issue #12) + +What `coven-github` durably retains, what it never retains, how it is redacted, +and how it is deleted — for both self-hosted operators and hosted OpenCoven. +Companion to [Security Model](security.md) and the +[Durable task store](durable-task-store.md). + +## Principle + +Retain enough to debug, bill, support, and satisfy customer trust — never raw +secrets, credentialed URLs, private repository contents, or unfiltered agent +transcripts. Everything the adapter writes to durable storage passes through +redaction first, and a test (`no_raw_token_survives_in_durable_stores`) scans +every adapter-generated stored field to prove no token or secret pattern +survives. + +## Artifact classes + +| Class | Retained? | Where | Notes | +|---|---|---|---| +| `task_metadata` | Yes (tenant-scoped) | `tasks` | id, installation, repo, familiar, kind, state, timestamps. | +| `publication_metadata` | Yes | `tasks` | branch, PR number, Check Run URL. | +| `agent_result` | Yes, **after redaction** | `tasks.summary`, `task_attempts.detail` | The result envelope is `sanitize_result`-scrubbed before any persist/publish. | +| `delivery_metadata` | Yes | `webhook_deliveries` | delivery id (idempotency), event/action, installation, repo, **payload hash only** — never the body. | +| `audit_events` | Yes | `api_audit`, table states below | See [Audit events](#audit-events). | +| `memory_activity` | Opt-in, retention-limited | `memory_activity` | Per-installation; see [issue #6](memory-contract.md). | +| `logs` / `transcripts` | Not persisted by the adapter | — | Streamed/redacted only; durable transcripts are out of scope until opt-in retention is designed. | +| `repo_checkout` | **Never** after task cleanup | ephemeral workspace | Workspace is deleted after every task. Container-isolated cleanup is issue #5. | +| `tokens` / `secrets` | **Never** | — | The brief is tokenless (#4); results, comments, Check Runs, and stored fields are redacted. | + +## Redaction + +`sanitize_result` scrubs every free-text field of the result envelope (summary, +PR body, branch, commit messages, review findings and evidence) before the +adapter stores or publishes anything; error detail written to +`task_attempts.detail`, Check Run summaries, and status comments passes through +`redact` too. Redaction replaces exact live token values **and** GitHub token +patterns (`ghs_`/`ghp_`/`gho_`/`ghu_`/`ghr_`/`github_pat_`) and +`x-access-token:` URL credentials. User-authored content (issue/comment bodies) +is deliberately *not* pattern-scrubbed — a maintainer may legitimately quote a +token-shaped string — so it is excluded from the durable audit-scan surface. + +## Audit events + +| Event | Recorded in | +|---|---| +| Webhook received / routed / ignored | `webhook_deliveries.routing` (`task:` / `ignored:`) | +| Task queued / claimed / running / terminal | `tasks.state` + `task_attempts` | +| API read (task list, memory inspect, usage) | `api_audit` | +| Memory read/write decisions | `memory_activity` (with the adapter's accept/reject verdict) | +| Memory revocation | `memory_revocations` | +| Tenant data deletion on uninstall | `api_audit` (`delete_on_uninstall`, with counts) | + +Installation-scoped tokens are minted per repository and role (#4); their +**permission class** is deterministic from the role, and the token **value** is +never logged or stored. + +## Deletion and retention + +- **Delete on uninstall** — an `installation` `deleted` webhook purges that + tenant's memory (activity + revocations) and task artifacts (tasks, + attempts, deliveries), and records an audited `delete_on_uninstall` entry. + Idempotent: a redelivery finds nothing left. +- **Task-history retention** — `[storage] task_retention_days` runs a periodic + server sweep that deletes terminal (completed/failed/superseded) tasks and + their attempts past the horizon. In-flight tasks are never expired. +- **Memory retention** — `[memory] retention_days` sweeps memory audit rows; + revocations are never expired (that would un-revoke memory). See #6. +- **On-demand revoke** — `POST /api/github/memory/revoke` (tenant-scoped). + +## Self-hosted vs hosted + +| | Self-hosted | Hosted OpenCoven | +|---|---|---| +| Store | Local SQLite; operator owns the volume and retention | Managed, tenant-isolated | +| Task API auth | `open` mode allowed for local dev | Tenant-scoped tokens, fail-closed (#3) | +| Retention | Optional (`task_retention_days` / `retention_days`) | Set by tier/policy | +| Memory | Operator-managed, off by default | Opt-in, per-installation, revocable | +| Worker isolation | May run on host | Container-isolated per task (#5) | + +## Not yet covered + +- Container-scoped artifact handling — `repo_checkout` cleanup guarantees and + container log retention — lands with hosted worker isolation (#5). +- A unified append-only audit-event log (the tables above already provide the + equivalent records; a single stream is a possible future consolidation). +- Durable, opt-in agent transcripts with their own redaction/retention policy. diff --git a/docs/security.md b/docs/security.md index 799a4fd..75a2403 100644 --- a/docs/security.md +++ b/docs/security.md @@ -108,4 +108,4 @@ Before accepting paid hosted customers, the service should have: - Worker timeout enforcement. - Containerized or sandboxed worker execution. - Secret redaction tests. -- A documented data retention policy. +- A documented data retention policy — see [Data retention, artifacts, and audit](data-retention.md).