feat: audit, artifact retention, and redaction controls — #12 (non-container core)#60
Merged
Merged
Conversation
#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>
… 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>
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>
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>
…etention Signed-off-by: Val Alexander <bunsthedev@gmail.com> # Conflicts: # crates/worker/src/lib.rs
Contributor
There was a problem hiding this comment.
Pull request overview
This PR advances issue #12 by adding durable-store redaction verification, task-artifact retention/deletion controls, and documentation describing data retention/audit semantics for self-hosted vs hosted operation (excluding container-isolation work deferred to #5).
Changes:
- Adds durable-store retention primitives: uninstall-time purging by installation and periodic expiry of terminal tasks via
[storage] task_retention_days. - Adds an end-to-end redaction test that persists sanitized artifacts and scans durable stored text for raw token values and token patterns.
- Documents artifact classes, redaction scope, audit surfaces, and retention/deletion behavior in a new data-retention doc (linked from
docs/security.md).
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/security.md | Links the security launch gate to the new data-retention documentation. |
| docs/data-retention.md | New documentation describing retained artifacts, redaction policy, audit surfaces, and retention semantics. |
| crates/worker/src/lib.rs | Adds an integration-style test proving token redaction survives persistence via Store::all_stored_text(). |
| crates/webhook/src/routes.rs | Extends uninstall handling to purge task artifacts and records an audit entry for deletion. |
| crates/store/src/lib.rs | Adds uninstall purge (delete_tasks_for_installation), terminal-task expiry, and all_stored_text() scan surface used for redaction proof. |
| crates/server/src/main.rs | Adds a periodic retention sweep that expires terminal tasks past the configured horizon. |
| crates/config/src/lib.rs | Introduces [storage] task_retention_days configuration. |
| config/example.toml | Documents the new task_retention_days option in the example config. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+794
to
+797
| let deliveries = tx.execute( | ||
| "DELETE FROM webhook_deliveries WHERE installation_id = ?1", | ||
| params![installation_id], | ||
| )?; |
Comment on lines
+849
to
+855
| 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", | ||
| ] { |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Advances #12 with everything achievable without worker container isolation. Per the ROADMAP #12 is marked as needing #5, but its core doesn't — the container-specific bits (
repo_checkoutcleanup, container logs) stay deferred to #5, so this advances #12 without closing it.What this delivers
all_stored_text()returns every adapter-generated durable free-text value (task summary, attempt detail, delivery routing, memory targets — user-authored issue/comment content excluded), and an end-to-end worker test smears a token through a result, sanitizes it as the worker does, persists viastore.finish, then scans all stored fields asserting no raw token and no token pattern survives — with a negative control so it isn't vacuous.delete_tasks_for_installation(tasks + attempts + deliveries),expire_terminal_tasks(_before)(never touches in-flight work), a[storage] task_retention_daysperiodic server sweep, and theinstallation.deletedwebhook now purges task artifacts alongside memory — recording an auditeddelete_on_uninstallentry with counts.docs/data-retention.mddocuments artifact classes, what's retained vs never retained, redaction, the audit-event tables, deletion/retention semantics, and self-hosted vs hosted differences (linked from the security launch gate).#12 acceptance criteria
durable stores never persist raw tokens ✅ (+ scan test) · task API redacted + tenant-scoped ✅ (from #3/#4, exercised) · audit records trigger/actor/install/repo/task/publication-links ✅ (existing tables + deletion audit) · retention deletes artifacts by installation + age ✅ · tests scan stored artifacts for token patterns ✅ · docs on self-hosted vs hosted retention ✅.
Deferred to #5 (container isolation)
repo_checkoutcleanup guarantees and container log retention — inherently tied to the container execution model. Also noted: a unified append-only audit-event stream (the current per-table records are equivalent) and opt-in durable transcripts.Local gates:
cargo check --all-targets+cargo clippy --all-targets -- -D warnings+cargo test --all(215 passed, 0 failed).🤖 Generated with Claude Code