Skip to content

Commit 2baa72b

Browse files
authored
feat: audit, artifact retention, and redaction controls — #12 (non-container core) (#60)
* 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 <bunsthedev@gmail.com> * 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 <bunsthedev@gmail.com> * test(worker): prove no raw token survives in durable stores (#12) 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 <bunsthedev@gmail.com> * 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 <bunsthedev@gmail.com> --------- Signed-off-by: Val Alexander <bunsthedev@gmail.com>
1 parent fd2ea35 commit 2baa72b

8 files changed

Lines changed: 439 additions & 7 deletions

File tree

config/example.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ max_retries = 2 # Retries for infra errors (exit code 2)
3939
# Durable adapter state: webhook deliveries (idempotency) and task records.
4040
# SQLite file — keep it on a persistent volume; parents are created at start.
4141
path = "data/coven-github.db"
42+
# task_retention_days = 90 # expire terminal task history after N days (issue #12); omit to keep forever
4243

4344
# ── Task API auth (issue #3) ────────────────────────────────────────────────
4445
# Gate GET /api/github/tasks. "open" = unauthenticated (local development

crates/config/src/lib.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,12 +247,17 @@ pub struct StorageConfig {
247247
/// SQLite database path; parent directories are created at startup.
248248
#[serde(default = "default_storage_path")]
249249
pub path: PathBuf,
250+
/// Days to retain terminal task history before a periodic sweep deletes it
251+
/// (issue #12). Absent = keep indefinitely. In-flight tasks are never
252+
/// expired.
253+
pub task_retention_days: Option<u32>,
250254
}
251255

252256
impl Default for StorageConfig {
253257
fn default() -> Self {
254258
Self {
255259
path: default_storage_path(),
260+
task_retention_days: None,
256261
}
257262
}
258263
}

crates/server/src/main.rs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,26 @@ async fn main() -> Result<()> {
132132
});
133133
}
134134

135+
// Task-history retention sweep (issue #12): expire terminal tasks
136+
// older than the configured horizon; in-flight work is never touched.
137+
if let Some(retention_days) = config.storage.task_retention_days {
138+
let sweep_store = store.clone();
139+
tokio::spawn(async move {
140+
let mut ticker =
141+
tokio::time::interval(std::time::Duration::from_secs(6 * 3600));
142+
loop {
143+
ticker.tick().await;
144+
match sweep_store.expire_terminal_tasks(retention_days).await {
145+
Ok(0) => {}
146+
Ok(expired) => {
147+
tracing::info!(expired, "expired terminal task rows past retention")
148+
}
149+
Err(e) => tracing::error!("task retention sweep failed: {e:#}"),
150+
}
151+
}
152+
});
153+
}
154+
135155
// Build router.
136156
let state = AppState {
137157
config: config.clone(),

crates/store/src/lib.rs

Lines changed: 182 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -770,6 +770,100 @@ impl Store {
770770
let cutoff = (chrono::Utc::now() - chrono::Duration::days(retention_days as i64)).to_rfc3339();
771771
self.expire_memory_activity_before(&cutoff).await
772772
}
773+
774+
/// Purges all task artifacts for an installation — attempts, tasks, and the
775+
/// delivery records — on uninstall (issue #12). Returns rows removed.
776+
pub async fn delete_tasks_for_installation(&self, installation_id: u64) -> Result<usize> {
777+
let conn = self.conn.clone();
778+
tokio::task::spawn_blocking(move || {
779+
let mut conn = conn.lock().expect("store mutex poisoned");
780+
let tx = conn.transaction()?;
781+
// Defer FK checks to commit so the multi-table cascade (attempts →
782+
// tasks → deliveries) needn't be perfectly ordered; the whole set is
783+
// consistent once committed.
784+
tx.execute_batch("PRAGMA defer_foreign_keys = ON")?;
785+
let attempts = tx.execute(
786+
"DELETE FROM task_attempts
787+
WHERE task_id IN (SELECT id FROM tasks WHERE installation_id = ?1)",
788+
params![installation_id],
789+
)?;
790+
let tasks = tx.execute(
791+
"DELETE FROM tasks WHERE installation_id = ?1",
792+
params![installation_id],
793+
)?;
794+
let deliveries = tx.execute(
795+
"DELETE FROM webhook_deliveries WHERE installation_id = ?1",
796+
params![installation_id],
797+
)?;
798+
tx.commit()?;
799+
Ok(attempts + tasks + deliveries)
800+
})
801+
.await
802+
.expect("store task panicked")
803+
}
804+
805+
/// Deletes terminal (completed / failed / superseded) tasks and their
806+
/// attempts older than `cutoff` (RFC 3339) — task retention (issue #12).
807+
/// In-flight (`queued` / `running`) tasks are never expired.
808+
pub async fn expire_terminal_tasks_before(&self, cutoff: &str) -> Result<usize> {
809+
let conn = self.conn.clone();
810+
let cutoff = cutoff.to_string();
811+
tokio::task::spawn_blocking(move || {
812+
let mut conn = conn.lock().expect("store mutex poisoned");
813+
let tx = conn.transaction()?;
814+
tx.execute_batch("PRAGMA defer_foreign_keys = ON")?;
815+
let expiring = "SELECT id FROM tasks \
816+
WHERE state IN ('completed','failed','superseded') AND updated_at < ?1";
817+
let attempts = tx.execute(
818+
&format!("DELETE FROM task_attempts WHERE task_id IN ({expiring})"),
819+
params![cutoff],
820+
)?;
821+
let tasks = tx.execute(
822+
"DELETE FROM tasks \
823+
WHERE state IN ('completed','failed','superseded') AND updated_at < ?1",
824+
params![cutoff],
825+
)?;
826+
tx.commit()?;
827+
Ok(attempts + tasks)
828+
})
829+
.await
830+
.expect("store task panicked")
831+
}
832+
833+
/// Expires terminal tasks older than `retention_days` from now.
834+
pub async fn expire_terminal_tasks(&self, retention_days: u32) -> Result<usize> {
835+
let cutoff = (chrono::Utc::now() - chrono::Duration::days(retention_days as i64)).to_rfc3339();
836+
self.expire_terminal_tasks_before(&cutoff).await
837+
}
838+
839+
/// Every adapter-generated free-text value the store durably retains — the
840+
/// scan surface for the redaction guarantee (issue #12): no raw token or
841+
/// secret may survive here. Deliberately excludes user-authored content
842+
/// (issue/comment bodies in `tasks.kind`), which may legitimately quote
843+
/// token-shaped strings and is never the adapter's to redact.
844+
pub async fn all_stored_text(&self) -> Result<Vec<String>> {
845+
let conn = self.conn.clone();
846+
tokio::task::spawn_blocking(move || {
847+
let conn = conn.lock().expect("store mutex poisoned");
848+
let mut out = Vec::new();
849+
for sql in [
850+
"SELECT summary FROM tasks WHERE summary IS NOT NULL",
851+
"SELECT detail FROM task_attempts WHERE detail IS NOT NULL",
852+
"SELECT routing FROM webhook_deliveries",
853+
"SELECT target FROM memory_activity",
854+
"SELECT target FROM memory_revocations",
855+
] {
856+
let mut stmt = conn.prepare(sql)?;
857+
let rows = stmt
858+
.query_map([], |row| row.get::<_, String>(0))?
859+
.collect::<std::result::Result<Vec<_>, _>>()?;
860+
out.extend(rows);
861+
}
862+
Ok(out)
863+
})
864+
.await
865+
.expect("store task panicked")
866+
}
773867
}
774868

775869
/// One recorded memory operation for the inspect/audit trail (issue #6).
@@ -1431,6 +1525,94 @@ mod queue_tests {
14311525
assert!(store.claim_next(&Default::default()).await.unwrap().is_none());
14321526
}
14331527

1528+
async fn finish_completed(store: &Store, task_id: &str, summary: &str, detail: Option<&str>) {
1529+
store
1530+
.finish(
1531+
task_id,
1532+
Terminal {
1533+
state: TerminalState::Completed,
1534+
result_status: Some("success".to_string()),
1535+
branch: None,
1536+
pr_number: None,
1537+
summary: Some(summary.to_string()),
1538+
detail: detail.map(str::to_string),
1539+
},
1540+
)
1541+
.await
1542+
.expect("finish");
1543+
}
1544+
1545+
#[tokio::test]
1546+
async fn delete_tasks_for_installation_purges_that_tenants_artifacts() {
1547+
let store = Store::open_in_memory().expect("open");
1548+
enqueue(&store, "d1", &fix_task("t1")).await; // installation 1
1549+
// Installation 2: the delivery and task share the installation (as they
1550+
// always do from one webhook payload).
1551+
let other = Task {
1552+
installation_id: 2,
1553+
..fix_task("t2")
1554+
};
1555+
store
1556+
.record_delivery(
1557+
Delivery {
1558+
installation_id: Some(2),
1559+
..delivery("d2")
1560+
},
1561+
Routing::Task(&other),
1562+
)
1563+
.await
1564+
.expect("enqueue");
1565+
// Claim + finish t1 so it has an attempt row too.
1566+
store.claim_next(&Default::default()).await.unwrap();
1567+
finish_completed(&store, "t1", "done", Some("some detail")).await;
1568+
1569+
let removed = store.delete_tasks_for_installation(1).await.unwrap();
1570+
assert!(removed >= 2, "task + delivery (+ attempt) removed: {removed}");
1571+
1572+
// Installation 1 is gone; installation 2 survives and is still claimable.
1573+
assert!(store
1574+
.delivery_routing("d1")
1575+
.await
1576+
.unwrap()
1577+
.is_none());
1578+
let claimed = store.claim_next(&Default::default()).await.unwrap().expect("t2 survives");
1579+
assert_eq!(claimed.id, "t2");
1580+
}
1581+
1582+
#[tokio::test]
1583+
async fn task_retention_expires_terminal_tasks_but_not_in_flight() {
1584+
let store = Store::open_in_memory().expect("open");
1585+
enqueue(&store, "d1", &fix_task("done")).await;
1586+
enqueue(&store, "d2", &fix_task("queued")).await;
1587+
// Make the first terminal; leave the second queued.
1588+
store.claim_next(&Default::default()).await.unwrap();
1589+
finish_completed(&store, "done", "s", None).await;
1590+
1591+
// A cutoff in the future expires the terminal task, never the queued one.
1592+
let removed = store
1593+
.expire_terminal_tasks_before("2999-01-01T00:00:00+00:00")
1594+
.await
1595+
.unwrap();
1596+
// The terminal task plus its attempt row are removed.
1597+
assert_eq!(removed, 2);
1598+
let claimed = store.claim_next(&Default::default()).await.unwrap().expect("queued survives");
1599+
assert_eq!(claimed.id, "queued");
1600+
}
1601+
1602+
#[tokio::test]
1603+
async fn all_stored_text_surfaces_adapter_generated_fields() {
1604+
let store = Store::open_in_memory().expect("open");
1605+
enqueue(&store, "d1", &fix_task("t1")).await;
1606+
store.claim_next(&Default::default()).await.unwrap();
1607+
finish_completed(&store, "t1", "SUMMARY_MARKER", Some("DETAIL_MARKER")).await;
1608+
1609+
let text = store.all_stored_text().await.unwrap();
1610+
assert!(text.iter().any(|t| t.contains("SUMMARY_MARKER")));
1611+
assert!(text.iter().any(|t| t.contains("DETAIL_MARKER")));
1612+
// The delivery routing is adapter-generated and included.
1613+
assert!(text.iter().any(|t| t.starts_with("task:")));
1614+
}
1615+
14341616
#[tokio::test]
14351617
async fn finish_reaches_terminal_state_and_closes_the_attempt() {
14361618
let store = Store::open_in_memory().expect("open");

crates/webhook/src/routes.rs

Lines changed: 37 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -511,12 +511,36 @@ pub async fn handle_webhook(
511511
{
512512
Ok(Recorded::New) => {
513513
if let Some(id) = installation_id {
514-
match state.store.delete_memory_for_installation(id).await {
515-
Ok(purged) => {
516-
info!(installation_id = id, purged, "purged memory on uninstall")
517-
}
518-
Err(e) => error!("failed to purge memory on uninstall: {e:#}"),
519-
}
514+
// Purge both memory (issue #6) and task artifacts (issue
515+
// #12) for the departing tenant. Idempotent: a redelivery
516+
// finds nothing left to remove.
517+
let memory = state
518+
.store
519+
.delete_memory_for_installation(id)
520+
.await
521+
.unwrap_or_else(|e| {
522+
error!("failed to purge memory on uninstall: {e:#}");
523+
0
524+
});
525+
let tasks = state
526+
.store
527+
.delete_tasks_for_installation(id)
528+
.await
529+
.unwrap_or_else(|e| {
530+
error!("failed to purge task artifacts on uninstall: {e:#}");
531+
0
532+
});
533+
info!(installation_id = id, memory, tasks, "purged tenant data on uninstall");
534+
// Audit what was deleted (issue #12).
535+
let _ = state
536+
.store
537+
.record_api_read(
538+
&format!("installation:{id}"),
539+
&id.to_string(),
540+
"delete_on_uninstall",
541+
&format!("memory:{memory},tasks:{tasks}"),
542+
)
543+
.await;
520544
}
521545
(StatusCode::OK, Json(json!({"ok": true}))).into_response()
522546
}
@@ -1750,6 +1774,13 @@ mod delivery_idempotency_tests {
17501774
.len(),
17511775
1
17521776
);
1777+
1778+
// The purge (memory + task artifacts) is audited (issue #12).
1779+
let audit = state.store.api_audit_entries().await.unwrap();
1780+
assert!(
1781+
audit.iter().any(|(_, _, action, _)| action == "delete_on_uninstall"),
1782+
"the uninstall purge must leave an audit record: {audit:?}"
1783+
);
17531784
}
17541785

17551786
#[tokio::test]

0 commit comments

Comments
 (0)