diff --git a/CLAUDE.md b/CLAUDE.md index 6692162..b8ea2ed 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -104,6 +104,23 @@ Validate before mutating. The guard originally fired *after* the status changes had been written, which left a half-applied command — worse for the operator than either doing it or refusing. +## Reaping + +`ExcerptSink::note_complete_scan` is the only thing that licenses concluding a +secret is gone, and it defaults to a no-op so a collector cannot make the +guarantee by accident. Silence there costs a stale row; a wrong claim costs an +exposure reported as handled. + +Never reap on "not re-confirmed". Tail sources resume from a cursor and are +almost never re-confirmed; whole-mode sources are stat-skipped when unchanged +and are not read at all. Both look identical to "the secret is gone" from the +findings table alone, and both are covered by tests that will fail loudly if +someone tries. + +Vanishing is a timestamp, not a status. A finding can be acknowledged *and* +gone, only the operator sets the first, and vanishing must never resolve +anything — the leak outlives the copy on disk. + ## Exclusions `crates/nanny-core/src/exclude.rs` holds two different things that share one diff --git a/crates/nanny-cli/src/format.rs b/crates/nanny-cli/src/format.rs index 888d364..a1f5155 100644 --- a/crates/nanny-cli/src/format.rs +++ b/crates/nanny-cli/src/format.rs @@ -161,7 +161,15 @@ pub fn location(finding: &Finding, home: &std::path::Path, width: usize) -> Stri /// One finding as a table row. #[must_use] pub fn row(finding: &Finding, home: &std::path::Path, style: &Style, width: usize) -> String { - let where_ = location(finding, home, width); + let where_ = if finding.is_present() { + location(finding, home, width) + } else { + // Marked rather than hidden: the row still says a rotation may be owed. + format!( + "{} (gone)", + location(finding, home, width.saturating_sub(7)) + ) + }; format!( "{:>5} {:<8} {:<9} {:<9} {:<26} {where_}", finding.id.0, @@ -207,6 +215,8 @@ pub fn json(finding: &Finding) -> serde_json::Value { "first_seen": finding.first_seen.format(&Rfc3339).unwrap_or_default(), "last_seen": finding.last_seen.format(&Rfc3339).unwrap_or_default(), "occurrences": finding.occurrences, + "present": finding.is_present(), + "vanished_at": finding.vanished_at.map(|t| t.format(&Rfc3339).unwrap_or_default()), "status": finding.status.to_string(), "note": finding.note, }) @@ -247,6 +257,7 @@ mod tests { status: Status::Open, note: None, status_changed_at: None, + vanished_at: None, } } diff --git a/crates/nanny-cli/src/main.rs b/crates/nanny-cli/src/main.rs index 0a66fef..6fcbace 100644 --- a/crates/nanny-cli/src/main.rs +++ b/crates/nanny-cli/src/main.rs @@ -370,6 +370,16 @@ async fn scan(runtime: &Runtime, full: bool, json: bool, style: &Style) -> Resul ); } + if !report.reaped.is_empty() && !json { + println!( + "{}", + style.dim(&format!( + "reaped {} stale row(s) whose location shifted, {} no longer on disk", + report.reaped.superseded, report.reaped.vanished + )) + ); + } + // Counting only what is new: a scan that keeps failing because of a finding // already acknowledged would be a scan people stop running. exit_code(new.len()); @@ -428,6 +438,23 @@ async fn status(runtime: &Runtime, style: &Style) -> Result<()> { println!(" {:<14}{count}", harness.slug()); } + let gone = runtime + .store + .list(&Query::default()) + .await? + .iter() + .filter(|f| !f.is_present()) + .count(); + if gone > 0 { + println!(); + println!( + " {}", + style.dim(&format!( + "{gone} finding(s) no longer on disk — the copy is gone, the leak is not" + )) + ); + } + Ok(()) } @@ -511,6 +538,17 @@ async fn show(runtime: &Runtime, id: FindingId, style: &Style) -> Result<()> { ), } println!(" {:<12}{}", "status", style.status(f.status)); + if let Some(when) = f.vanished_at { + println!( + " {:<12}{}", + "on disk", + style.dim(&format!( + "no — gone from this file as of {}. The leak still happened; rotate if \ + it was ever real.", + format::when(when) + )) + ); + } println!(" {:<12}{}", "harness", f.harness.slug()); println!(" {:<12}{}", "rule", f.rule); println!(); diff --git a/crates/nanny-cli/src/tui.rs b/crates/nanny-cli/src/tui.rs index 694521a..00963e1 100644 --- a/crates/nanny-cli/src/tui.rs +++ b/crates/nanny-cli/src/tui.rs @@ -414,6 +414,23 @@ fn draw_detail(frame: &mut ratatui::Frame<'_>, area: Rect, app: &App, engine: &R lines.push(field("in", detail)); } + if let Some(when) = f.vanished_at { + lines.push(Line::from(vec![ + Span::styled( + format!("{:<10}", "on disk"), + Style::default().fg(Color::DarkGray), + ), + Span::styled( + format!("no — gone as of {}", format::when(when)), + Style::default().fg(Color::DarkGray), + ), + Span::styled( + " the leak still happened", + Style::default().fg(Color::Yellow), + ), + ])); + } + lines.push(field( "seen", &format!( diff --git a/crates/nanny-core/src/port.rs b/crates/nanny-core/src/port.rs index f80a7c6..8fcc33d 100644 --- a/crates/nanny-core/src/port.rs +++ b/crates/nanny-core/src/port.rs @@ -6,7 +6,7 @@ //! shown *not* to write secrets anywhere, and that is only a meaningful claim //! if the test can see everywhere it wrote. -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use async_trait::async_trait; use nanny_entities::{ @@ -29,6 +29,26 @@ pub trait ExcerptSink: Send { /// Returns an error only if the sink itself has failed — a storage write, /// say. Detection failures are not errors. async fn accept(&mut self, excerpt: Excerpt) -> crate::Result<()>; + + /// Declare that `path` was read **in full** this sweep — every byte of it, + /// from the beginning, not resumed from a cursor. + /// + /// This is the only thing that licenses concluding a secret is gone. The + /// absence of a confirmation does not: a tail source resumes from a cursor, + /// so a finding at offset 500 of a 40 MB transcript is never re-confirmed on + /// a normal sweep, and reaping on "not seen again" would resolve every real + /// transcript spill on the machine. + /// + /// Defaulted to a no-op so that a collector which cannot make the guarantee + /// does not make it by accident. Silence here costs a stale row; a wrong + /// claim costs an exposure reported as handled. + /// + /// # Errors + /// Returns an error only if the sink itself has failed. + async fn note_complete_scan(&mut self, path: &Path) -> crate::Result<()> { + let _ = path; + Ok(()) + } } /// What a sweep of one collector did. @@ -205,6 +225,31 @@ pub struct Recorded { pub is_new: bool, } +/// What a reap reconciled. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub struct Reaped { + /// Stale rows merged into a successor and removed — the same secret, still + /// in the same file, at a shifted offset. + pub superseded: usize, + /// Findings whose secret is no longer in the container at all. Recorded, + /// not resolved. + pub vanished: usize, +} + +impl Reaped { + /// Fold another reap's counts into these. + pub fn merge(&mut self, other: Self) { + self.superseded += other.superseded; + self.vanished += other.vanished; + } + + /// Whether anything was reconciled. + #[must_use] + pub const fn is_empty(&self) -> bool { + self.superseded == 0 && self.vanished == 0 + } +} + /// What to select when listing findings. #[derive(Debug, Clone, Default)] pub struct Query { @@ -335,6 +380,45 @@ pub trait SuppressionStore: Send + Sync { async fn suppression(&self, id: SuppressionId) -> crate::Result>; } +/// Reconciling findings whose location no longer holds the secret. +/// +/// Its own port rather than part of [`FindingStore`] because it is the one +/// operation whose bug reports an exposure as handled when it is not, and a +/// consumer that only reads findings has no business being able to retire them. +#[async_trait] +pub trait ReapStore: Send + Sync { + /// Reconcile findings in containers that were read in full this sweep. + /// + /// For each finding in one of `complete` whose `last_seen` predates + /// `sweep_started_at` — that is, the container was read cover to cover and + /// the secret was not at this location: + /// + /// * if the same fingerprint *is* present elsewhere in the same container, + /// the finding moved rather than went away. Merge it into its successor, + /// carrying `first_seen` and the operator's triage where the successor has + /// none, and delete the stale row. This is the bookkeeping noise an edit + /// creates, and merging is what makes a triage decision survive a + /// reformat. + /// * otherwise the secret is gone from that container. Stamp `vanished_at` + /// and leave `status` alone — vanishing is not resolving. + /// + /// Implementations must touch nothing in a container absent from `complete`. + /// + /// # Errors + /// Returns an error if the reconciliation fails. + async fn reap( + &self, + complete: &[PathBuf], + sweep_started_at: OffsetDateTime, + ) -> crate::Result; + + /// Mark findings whose container no longer exists on disk as vanished. + /// + /// # Errors + /// Returns an error if the read or write fails. + async fn reap_missing(&self) -> crate::Result; +} + /// Persistence for operator severity overrides scoped to a place. /// /// Separate from [`SuppressionStore`] because the two say different things. A diff --git a/crates/nanny-core/src/sweep.rs b/crates/nanny-core/src/sweep.rs index c0038f5..e562a89 100644 --- a/crates/nanny-core/src/sweep.rs +++ b/crates/nanny-core/src/sweep.rs @@ -4,6 +4,9 @@ //! Shared by the daemon and by `nanny scan`, so that a one-shot scan and a //! continuous watch cannot drift apart in what they detect. +use std::collections::BTreeSet; +use std::path::{Path, PathBuf}; + use async_trait::async_trait; use nanny_entities::{ DowngradeId, Downgrades, Excerpt, Finding, FindingId, Severity, Status, SuppressionId, @@ -13,7 +16,7 @@ use time::OffsetDateTime; use crate::port::{ Alerter, CollectStats, Collector, CursorStore, DowngradeStore, ExcerptSink, FindingStore, - Scope, SuppressionStore, + ReapStore, Reaped, Scope, SuppressionStore, }; use crate::scan::Scanner; @@ -29,6 +32,9 @@ pub struct SweepReport { pub suppressed: usize, /// Findings an operator downgrade lowered. pub downgraded: usize, + /// Stale rows retired because their container was read in full and the + /// secret was not there any more. + pub reaped: Reaped, /// Collectors that failed, by name. A sweep reports rather than aborts: /// one unreadable store must not hide what the others found. pub failures: Vec<(String, String)>, @@ -42,6 +48,12 @@ impl SweepReport { pub fn is_clean(&self) -> bool { self.new_findings.is_empty() && self.reconfirmed == 0 } + + /// Whether the sweep changed the recorded picture in any way. + #[must_use] + pub fn is_quiet(&self) -> bool { + self.is_clean() && self.reaped.is_empty() && self.suppressed == 0 + } } /// A sink that scans each excerpt and records what it finds. @@ -58,6 +70,9 @@ struct RecordingSink<'a, S: FindingStore + ?Sized> { /// the end rather than written per finding. hits: Vec, downgrade_hits: Vec, + /// Containers read cover to cover this sweep — the only ones about which + /// anything may be concluded from an absence. + complete: BTreeSet, } #[async_trait] @@ -109,6 +124,11 @@ impl ExcerptSink for RecordingSink<'_, S> { } Ok(()) } + + async fn note_complete_scan(&mut self, path: &Path) -> crate::Result<()> { + self.complete.insert(path.to_path_buf()); + Ok(()) + } } /// Run one sweep across every available collector in `scope`. @@ -125,7 +145,7 @@ pub async fn sweep( scanner: &Scanner, collectors: &[Box], cursors: &dyn CursorStore, - store: &(impl FindingStore + SuppressionStore + DowngradeStore + ?Sized), + store: &(impl FindingStore + SuppressionStore + DowngradeStore + ReapStore + ?Sized), scope: Scope, ) -> crate::Result { let started_at = OffsetDateTime::now_utc(); @@ -140,6 +160,7 @@ pub async fn sweep( let downgrades = store.downgrades().await?; let mut hits: Vec = Vec::new(); let mut downgrade_hits: Vec = Vec::new(); + let mut complete: BTreeSet = BTreeSet::new(); for collector in collectors { if !scope.includes(collector.cadence()) { @@ -164,6 +185,7 @@ pub async fn sweep( downgraded: 0, hits: Vec::new(), downgrade_hits: Vec::new(), + complete: BTreeSet::new(), }; match collector.collect(cursors, &mut sink).await { @@ -191,6 +213,7 @@ pub async fn sweep( report.downgraded += sink.downgraded; hits.extend(sink.hits); downgrade_hits.extend(sink.downgrade_hits); + complete.extend(sink.complete); } hits.sort_unstable(); @@ -202,6 +225,20 @@ pub async fn sweep( store.record_downgrade_hits(id, count).await?; } + // Reconcile only what was read cover to cover. A collector that failed + // part-way contributed whatever it did manage to read completely, and + // nothing about the rest — which is the right granularity: one unreadable + // transcript must not stop the other forty-three being tidied, and must not + // cause anything in it to be retired either. + let complete: Vec = complete.into_iter().collect(); + report.reaped = store.reap(&complete, started_at).await?; + + // A container that is gone takes its findings with it, and that needs no + // complete read to justify. Only worth the stat calls on a full sweep. + if scope == Scope::Full { + report.reaped.vanished += store.reap_missing().await?; + } + let finished_at = OffsetDateTime::now_utc(); report.finished_at = Some(finished_at); // Only a full sweep updates the liveness record: a reactive sweep skips the diff --git a/crates/nanny-data/migrations/0004_reap.sql b/crates/nanny-data/migrations/0004_reap.sql new file mode 100644 index 0000000..14ca59a --- /dev/null +++ b/crates/nanny-data/migrations/0004_reap.sql @@ -0,0 +1,15 @@ +-- When a finding's location no longer holds the secret. +-- +-- Findings are keyed on (fingerprint, path, detail, byte_offset). Edit a file +-- and every offset after the edit moves, so the next sweep inserts new rows +-- beside the old ones and nothing ever removes the old ones. +-- +-- Deliberately a timestamp and not a status. A finding can be both +-- acknowledged and vanished, and only the operator decides the first — making +-- "gone from disk" a status would let a sweep silently overwrite a triage +-- decision. It also would not be true: a spill edited away is not a spill that +-- never happened. The value already reached a model provider, and this row is +-- the only thing that still says a rotation may be owed. +ALTER TABLE finding ADD COLUMN vanished_at TEXT; + +CREATE INDEX finding_path_fingerprint_idx ON finding (origin_path, fingerprint); diff --git a/crates/nanny-data/src/collect/files.rs b/crates/nanny-data/src/collect/files.rs index 8da09d8..a1b4125 100644 --- a/crates/nanny-data/src/collect/files.rs +++ b/crates/nanny-data/src/collect/files.rs @@ -246,6 +246,10 @@ impl Collector for FileCollector { ); } + if outcome.complete { + sink.note_complete_scan(&path).await?; + } + for chunk in outcome.chunks { sink.accept(Excerpt { source: id.clone(), diff --git a/crates/nanny-data/src/collect/sqlite.rs b/crates/nanny-data/src/collect/sqlite.rs index 09b46a2..ba4dab8 100644 --- a/crates/nanny-data/src/collect/sqlite.rs +++ b/crates/nanny-data/src/collect/sqlite.rs @@ -129,6 +129,10 @@ impl Collector for SqliteCollector { _ => String::new(), }; let started_at = last_key.clone(); + // Only a scan from an empty cursor has seen every row, and so only + // that one licenses concluding a row is gone. Any other resumes + // past rows it never looked at. + let from_the_beginning = last_key.is_empty(); loop { let sql = format!( @@ -182,6 +186,10 @@ impl Collector for SqliteCollector { } } + if from_the_beginning { + sink.note_complete_scan(&self.path).await?; + } + if last_key != started_at { stats.advanced += 1; cursors.save(&id, &Cursor::Row { last_key }).await?; diff --git a/crates/nanny-data/src/read.rs b/crates/nanny-data/src/read.rs index bf6fe07..8b00dac 100644 --- a/crates/nanny-data/src/read.rs +++ b/crates/nanny-data/src/read.rs @@ -50,6 +50,13 @@ pub struct ReadOutcome { pub bytes: u64, /// Whether the source was skipped without being read. pub skipped: bool, + /// Whether every byte of the source was read this time, from offset zero. + /// + /// The licence to conclude a secret is no longer somewhere. Set only when + /// the read actually covered the whole container: a tail resumed from a + /// cursor did not, and neither did a whole-file read skipped on an + /// unchanged stat. + pub complete: bool, } /// Read the part of an append-only file after `cursor`. @@ -138,6 +145,10 @@ pub fn tail(path: &Path, cursor: Option<&Cursor>, config: &ScanConfig) -> Result }), bytes, skipped: false, + // Only when the tail happened to start at the beginning — a fresh + // source, or one whose cursor was invalidated by rotation, truncation + // or `--full`. + complete: start == 0, }) } @@ -211,6 +222,8 @@ pub fn whole(path: &Path, cursor: Option<&Cursor>, config: &ScanConfig) -> Resul }), bytes, skipped: false, + // A whole-mode read always covers the whole file. + complete: true, }) } @@ -364,6 +377,45 @@ mod tests { } } + #[test] + fn only_a_read_that_covered_everything_claims_to_be_complete() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("log"); + std::fs::write(&path, "first line\n").unwrap(); + + let first = tail(&path, None, &ScanConfig::default()).unwrap(); + assert!(first.complete, "a fresh source is read from the beginning"); + + let mut f = std::fs::OpenOptions::new() + .append(true) + .open(&path) + .unwrap(); + f.write_all(b"second line\n").unwrap(); + drop(f); + + let second = tail(&path, first.cursor.as_ref(), &ScanConfig::default()).unwrap(); + assert!( + !second.complete, + "a tail resumed from a cursor saw only part of the file, and must not \ + license concluding anything about the rest" + ); + + std::fs::write(&path, "replaced\n").unwrap(); + let third = tail(&path, second.cursor.as_ref(), &ScanConfig::default()).unwrap(); + assert!( + third.complete, + "truncation forces a read from zero, which is complete" + ); + + // Whole-mode reads cover everything, but a stat-skip covers nothing. + let whole_path = dir.path().join("memory.md"); + std::fs::write(&whole_path, "content\n").unwrap(); + let w1 = whole(&whole_path, None, &ScanConfig::default()).unwrap(); + assert!(w1.complete); + let w2 = whole(&whole_path, w1.cursor.as_ref(), &ScanConfig::default()).unwrap(); + assert!(!w2.complete, "a file that was not opened tells us nothing"); + } + #[test] fn a_tail_reads_only_what_is_new() { let dir = tempfile::tempdir().unwrap(); diff --git a/crates/nanny-data/src/store.rs b/crates/nanny-data/src/store.rs index d0b977c..81e9034 100644 --- a/crates/nanny-data/src/store.rs +++ b/crates/nanny-data/src/store.rs @@ -13,8 +13,8 @@ use async_trait::async_trait; use nanny_core::port::{ - CollectStats, CursorStore, DowngradeStore, FindingStore, Query, Recorded, Summary, - SuppressionStore, + CollectStats, CursorStore, DowngradeStore, FindingStore, Query, ReapStore, Reaped, Recorded, + Summary, SuppressionStore, }; use nanny_core::{Error, Result}; use nanny_entities::{ @@ -158,7 +158,11 @@ impl FindingStore for SqliteStore { severity = excluded.severity, rule_severity = excluded.rule_severity, context = excluded.context, - demotion_reason = excluded.demotion_reason + demotion_reason = excluded.demotion_reason, + -- Seeing it again proves it is there. A container on a + -- filesystem that was briefly unavailable heals itself rather + -- than staying wrongly marked as gone. + vanished_at = NULL RETURNING id, occurrences ", ) @@ -414,6 +418,10 @@ fn row_to_finding(row: &sqlx::sqlite::SqliteRow) -> Result { .get::, _>("status_changed_at") .map(|s| parse_stamp(&s)) .transpose()?, + vanished_at: row + .get::, _>("vanished_at") + .map(|s| parse_stamp(&s)) + .transpose()?, }) } @@ -652,3 +660,147 @@ impl DowngradeStore for SqliteStore { .cloned()) } } + +/// Reap implementation. +/// +/// Split out of the `FindingStore` impl because it is the one operation in this +/// file where being wrong means nanny reports an exposure as handled when it is +/// not, and it deserves to be read on its own. +impl SqliteStore { + /// Reconcile one container that was read in full. + async fn reap_one(&self, path: &str, since: &str) -> Result { + // Everything in this container that the complete read did not confirm. + // `last_seen` is stamped on every confirmation, so predating the sweep + // means "read the whole file, did not find it here". + let stale = sqlx::query( + "SELECT id, fingerprint, first_seen, status, note FROM finding \ + WHERE origin_path = ?1 AND last_seen < ?2 AND vanished_at IS NULL", + ) + .bind(path) + .bind(since) + .fetch_all(&self.pool) + .await + .map_err(|e| Error::Storage(format!("cannot read stale findings: {e}")))?; + + let mut reaped = Reaped::default(); + + for row in stale { + let id: i64 = row.get("id"); + let fingerprint: String = row.get("fingerprint"); + let first_seen: String = row.get("first_seen"); + let status: String = row.get("status"); + let note: Option = row.get("note"); + + // Did the same secret turn up elsewhere in this container during + // the same sweep? Then it moved rather than went away — an edit + // shifted every offset after it. Lowest id for determinism. + let successor = sqlx::query( + "SELECT id FROM finding \ + WHERE origin_path = ?1 AND fingerprint = ?2 AND last_seen >= ?3 AND id != ?4 \ + ORDER BY byte_offset, id LIMIT 1", + ) + .bind(path) + .bind(&fingerprint) + .bind(since) + .bind(id) + .fetch_optional(&self.pool) + .await + .map_err(|e| Error::Storage(format!("cannot look for a successor: {e}")))?; + + match successor { + Some(successor) => { + let successor_id: i64 = successor.get("id"); + + // Carry the history forward. `first_seen` is the honest one + // — the secret has been in this file since then, whatever + // reformatting has happened. Triage carries too, but only + // onto a successor nobody has decided about yet: a fresher + // decision must not be overwritten by an older one. + sqlx::query( + "UPDATE finding SET \ + first_seen = MIN(first_seen, ?1), \ + status = CASE WHEN status = 'open' THEN ?2 ELSE status END, \ + note = CASE WHEN status = 'open' THEN ?3 ELSE note END \ + WHERE id = ?4", + ) + .bind(&first_seen) + .bind(&status) + .bind(note.as_deref()) + .bind(successor_id) + .execute(&self.pool) + .await + .map_err(|e| Error::Storage(format!("cannot merge finding {id}: {e}")))?; + + sqlx::query("DELETE FROM finding WHERE id = ?1") + .bind(id) + .execute(&self.pool) + .await + .map_err(|e| { + Error::Storage(format!("cannot remove superseded finding {id}: {e}")) + })?; + + reaped.superseded += 1; + } + None => { + // Gone from this container. Recorded, not resolved: the + // value already reached wherever it was going. + sqlx::query("UPDATE finding SET vanished_at = ?1 WHERE id = ?2") + .bind(since) + .bind(id) + .execute(&self.pool) + .await + .map_err(|e| { + Error::Storage(format!("cannot mark finding {id} vanished: {e}")) + })?; + reaped.vanished += 1; + } + } + } + + Ok(reaped) + } +} + +#[async_trait] +impl ReapStore for SqliteStore { + async fn reap(&self, complete: &[PathBuf], sweep_started_at: OffsetDateTime) -> Result { + let since = stamp(sweep_started_at); + let mut reaped = Reaped::default(); + for path in complete { + reaped.merge(self.reap_one(&path.display().to_string(), &since).await?); + } + Ok(reaped) + } + + async fn reap_missing(&self) -> Result { + let rows = + sqlx::query("SELECT DISTINCT origin_path FROM finding WHERE vanished_at IS NULL") + .fetch_all(&self.pool) + .await + .map_err(|e| Error::Storage(format!("cannot list finding paths: {e}")))?; + + let now = stamp(OffsetDateTime::now_utc()); + let mut vanished = 0usize; + + for row in rows { + let path: String = row.get("origin_path"); + if Path::new(&path).exists() { + continue; + } + // The container is gone, so everything in it is. This is the one + // conclusion that needs no complete read to justify. + let result = sqlx::query( + "UPDATE finding SET vanished_at = ?1 \ + WHERE origin_path = ?2 AND vanished_at IS NULL", + ) + .bind(&now) + .bind(&path) + .execute(&self.pool) + .await + .map_err(|e| Error::Storage(format!("cannot mark {path} vanished: {e}")))?; + vanished += usize::try_from(result.rows_affected()).unwrap_or(0); + } + + Ok(vanished) + } +} diff --git a/crates/nanny-data/tests/end_to_end.rs b/crates/nanny-data/tests/end_to_end.rs index 5873212..5d1ce07 100644 --- a/crates/nanny-data/tests/end_to_end.rs +++ b/crates/nanny-data/tests/end_to_end.rs @@ -993,6 +993,282 @@ async fn a_marker_rule_is_flagged_so_the_cli_can_refuse_unscoped_suppression() { ); } +#[tokio::test] +async fn an_edit_that_shifts_offsets_merges_rather_than_duplicating() { + // The case behind the whole mechanism: editing a file moves every offset + // after the edit, so the next sweep inserts new rows beside the old ones. + let fixture = Fixture::new(); + let store = store().await; + let scanner = fixture.scanner(); + let collectors = fixture.collectors(); + + let memory = fixture + .home + .join(".claude/projects/-home-x-proj/memory/db.md"); + + nanny_core::sweep(&scanner, &collectors, &store, &store, Scope::Full) + .await + .expect("first sweep"); + let before = store + .list(&Query::default()) + .await + .expect("list") + .into_iter() + .find(|f| f.origin.path() == memory) + .expect("the memory finding"); + + // The operator triages it. + store + .set_status(&StatusChange { + id: before.id, + status: Status::Acknowledged, + note: Some("rotating on Friday".into()), + }) + .await + .expect("ack"); + + // Someone prepends a paragraph. The secret is unchanged; its offset is not. + let original = std::fs::read_to_string(&memory).expect("read"); + std::fs::write(&memory, format!("# Notes\n\nSome preamble.\n\n{original}")).expect("rewrite"); + + let second = nanny_core::sweep(&scanner, &collectors, &store, &store, Scope::Full) + .await + .expect("second sweep"); + + let after: Vec<_> = store + .list(&Query::default()) + .await + .expect("list") + .into_iter() + .filter(|f| f.origin.path() == memory) + .collect(); + + assert_eq!(after.len(), 1, "the finding moved; it did not become two"); + assert_eq!(second.reaped.superseded, 1); + assert_eq!(second.reaped.vanished, 0, "the secret is still in the file"); + + let moved = &after[0]; + assert!(moved.byte_offset > before.byte_offset, "it really did move"); + assert_eq!( + moved.status, + Status::Acknowledged, + "triage must survive a reformat, or nobody will triage" + ); + assert_eq!(moved.note.as_deref(), Some("rotating on Friday")); + assert_eq!( + moved.first_seen, before.first_seen, + "the history carries forward" + ); +} + +#[tokio::test] +async fn a_secret_removed_from_a_file_is_marked_gone_but_not_resolved() { + let fixture = Fixture::new(); + let store = store().await; + let scanner = fixture.scanner(); + let collectors = fixture.collectors(); + + let memory = fixture + .home + .join(".claude/projects/-home-x-proj/memory/db.md"); + nanny_core::sweep(&scanner, &collectors, &store, &store, Scope::Full) + .await + .expect("first sweep"); + + std::fs::write( + &memory, + "connection: postgres://svc@magrathea.internal:5432/app\n", + ) + .expect("scrub"); + + let second = nanny_core::sweep(&scanner, &collectors, &store, &store, Scope::Full) + .await + .expect("second sweep"); + assert_eq!(second.reaped.vanished, 1); + assert_eq!(second.reaped.superseded, 0); + + let finding = store + .list(&Query::default()) + .await + .expect("list") + .into_iter() + .find(|f| f.origin.path() == memory) + .expect("the row must survive: the leak happened even if the copy is gone"); + + assert!(!finding.is_present(), "it should be marked gone from disk"); + assert!(finding.vanished_at.is_some()); + assert_eq!( + finding.status, + Status::Open, + "vanishing must not resolve anything — a rotation may still be owed" + ); + assert!(finding.needs_attention()); +} + +#[tokio::test] +async fn a_tail_source_read_incrementally_is_never_reaped() { + // The failure that would matter most. Transcripts resume from a cursor, so + // a finding near the start of a 40 MB session is not re-confirmed on any + // normal sweep. Reaping on "not seen again" would retire every real + // transcript spill on the machine. + let fixture = Fixture::new(); + let store = store().await; + let scanner = fixture.scanner(); + let collectors = fixture.collectors(); + + let transcript = fixture + .home + .join(".claude/projects/-home-x-proj/session.jsonl"); + + nanny_core::sweep(&scanner, &collectors, &store, &store, Scope::Full) + .await + .expect("first sweep"); + let before = store + .list(&Query::default()) + .await + .expect("list") + .into_iter() + .find(|f| f.origin.path() == transcript) + .expect("transcript finding"); + assert!(before.is_present()); + + // Append, as a live session does. The tail resumes from the cursor and + // never re-reads the line the finding is on. + let mut existing = std::fs::read_to_string(&transcript).expect("read"); + existing.push_str("{\"type\":\"user\",\"text\":\"nothing interesting\"}\n"); + std::fs::write(&transcript, existing).expect("append"); + + for _ in 0..3 { + nanny_core::sweep(&scanner, &collectors, &store, &store, Scope::Full) + .await + .expect("sweep"); + } + + let after = store + .list(&Query::default()) + .await + .expect("list") + .into_iter() + .find(|f| f.origin.path() == transcript) + .expect("still there"); + assert!( + after.is_present(), + "an incremental read must never license concluding a secret is gone" + ); + assert_eq!(after.id, before.id); +} + +#[tokio::test] +async fn a_source_that_was_skipped_this_sweep_is_not_reaped() { + // Whole-mode files are skipped entirely when their stat is unchanged. That + // is "not looked at", not "not there", and must conclude nothing. + let fixture = Fixture::new(); + let store = store().await; + let scanner = fixture.scanner(); + let collectors = fixture.collectors(); + + nanny_core::sweep(&scanner, &collectors, &store, &store, Scope::Full) + .await + .expect("first"); + + // Nothing changes, so every whole-mode source is stat-skipped. + let second = nanny_core::sweep(&scanner, &collectors, &store, &store, Scope::Full) + .await + .expect("second"); + + assert_eq!(second.reaped.superseded, 0); + assert_eq!( + second.reaped.vanished, 0, + "skipping a file must conclude nothing about it" + ); + assert!( + store + .list(&Query::default()) + .await + .expect("list") + .iter() + .all(nanny_entities::Finding::is_present), + "a quiet sweep retired something it never read" + ); +} + +#[tokio::test] +async fn a_deleted_file_takes_its_findings_with_it() { + let fixture = Fixture::new(); + let store = store().await; + let scanner = fixture.scanner(); + let collectors = fixture.collectors(); + + let memory = fixture + .home + .join(".claude/projects/-home-x-proj/memory/db.md"); + nanny_core::sweep(&scanner, &collectors, &store, &store, Scope::Full) + .await + .expect("first"); + std::fs::remove_file(&memory).expect("delete"); + + let second = nanny_core::sweep(&scanner, &collectors, &store, &store, Scope::Full) + .await + .expect("second"); + assert!(second.reaped.vanished >= 1); + + let finding = store + .list(&Query::default()) + .await + .expect("list") + .into_iter() + .find(|f| f.origin.path() == memory) + .expect("the record outlives the file"); + assert!(!finding.is_present()); +} + +#[tokio::test] +async fn a_secret_that_comes_back_is_no_longer_marked_gone() { + // A filesystem that was briefly unavailable, or a change reverted. Seeing + // the secret again proves it is there, so the mark has to clear itself. + let fixture = Fixture::new(); + let store = store().await; + let scanner = fixture.scanner(); + let collectors = fixture.collectors(); + + let memory = fixture + .home + .join(".claude/projects/-home-x-proj/memory/db.md"); + let original = std::fs::read_to_string(&memory).expect("read"); + + nanny_core::sweep(&scanner, &collectors, &store, &store, Scope::Full) + .await + .expect("first"); + std::fs::write(&memory, "nothing here\n").expect("scrub"); + nanny_core::sweep(&scanner, &collectors, &store, &store, Scope::Full) + .await + .expect("second"); + + let gone = store + .list(&Query::default()) + .await + .expect("list") + .into_iter() + .find(|f| f.origin.path() == memory) + .expect("finding"); + assert!(!gone.is_present()); + + std::fs::write(&memory, original).expect("restore"); + nanny_core::sweep(&scanner, &collectors, &store, &store, Scope::Full) + .await + .expect("third"); + + let back = store + .list(&Query::default()) + .await + .expect("list") + .into_iter() + .find(|f| f.origin.path() == memory) + .expect("finding"); + assert!(back.is_present(), "re-confirmation must clear the mark"); + assert!(back.vanished_at.is_none()); +} + #[tokio::test] async fn clearing_cursors_makes_the_next_sweep_read_everything_again() { let fixture = Fixture::new(); diff --git a/crates/nanny-entities/src/finding.rs b/crates/nanny-entities/src/finding.rs index 90be18f..4c71576 100644 --- a/crates/nanny-entities/src/finding.rs +++ b/crates/nanny-entities/src/finding.rs @@ -186,9 +186,35 @@ pub struct Finding { /// Operator note attached at the last status change. pub note: Option, pub status_changed_at: Option, + /// When a sweep that read this container in full failed to find the secret + /// here any more. + /// + /// Deliberately a timestamp and not a [`Status`]. A finding can be both + /// acknowledged and vanished, and only the operator decides the first — + /// making "gone from disk" a status would let a sweep silently overwrite a + /// triage decision. It also would not be true: a spill edited away is not a + /// spill that never happened. The value already reached a model provider, + /// and this row is the only thing that still says a rotation may be owed. + pub vanished_at: Option, } impl Finding { + /// Whether the secret is still at this location, as far as the last + /// complete read of the container could tell. + #[must_use] + pub const fn is_present(&self) -> bool { + self.vanished_at.is_none() + } + + /// Whether this finding still represents an exposure nobody has dealt with. + /// + /// Vanishing does not enter into it. A leaked credential that has since been + /// edited out of a transcript is still leaked. + #[must_use] + pub const fn needs_attention(&self) -> bool { + self.status.is_unresolved() + } + /// Whether this finding's severity was lowered by where it was found, and /// the reason if so. /// @@ -209,9 +235,12 @@ impl Finding { /// Sort key for the UI: unresolved first, then most serious, then most /// recent. #[must_use] - pub fn triage_key(&self) -> (bool, u8, i64) { + pub fn triage_key(&self) -> (bool, bool, u8, i64) { ( !self.status.is_unresolved(), + // Still on disk sorts above gone: it is the one you can still act + // on locally, even though both may need a rotation. + !self.is_present(), self.severity.rank(), -self.last_seen.unix_timestamp(), ) diff --git a/readme.md b/readme.md index 01045cb..0e34242 100644 --- a/readme.md +++ b/readme.md @@ -205,6 +205,40 @@ Set `matches_marker = true` on any local rule whose capture group cannot tell on instance from another. For rules that do capture the secret, `--everywhere` prints the reach it is about to take before taking it. +### When a finding's location stops holding the secret + +Findings are keyed on `(fingerprint, path, detail, byte_offset)`. Edit a file and +every offset after the edit moves, so without reconciliation the next sweep +inserts new rows beside the old ones and the count at the top of `nanny status` +drifts away from reality. + +Reaping is deliberately conservative, because the failure mode is nanny +reporting an exposure as handled when it is not. **The absence of a confirmation +proves nothing** — tail sources resume from a cursor, so a finding at offset 500 +of a 40 MB transcript is never re-confirmed on a normal sweep. What licenses a +conclusion is a positive "I read all of this container and it was not there": +whole-mode files that were actually opened, tail files whose cursor was +invalidated by rotation, truncation or `--full`, and the database only when +scanned from an empty cursor. + +Given that, two outcomes: + +- **Superseded** — the same secret is still in the same file at a new offset. It + moved; it did not go away. The stale row is merged into its successor and + removed, carrying `first_seen` and the operator's triage where the successor + has none. **A decision survives a reformat**, which matters more than the row + count: triage that evaporates when someone runs a formatter is triage nobody + does twice. +- **Vanished** — the secret is gone from the container entirely, or the container + itself is gone. Recorded, **not resolved**. A spill edited away is not a spill + that never happened: the value already reached a model provider, and the row is + the only thing that still says a rotation may be owed. So `vanished_at` is a + timestamp orthogonal to `status`, findings show as `(gone)` rather than + disappearing, and an untriaged one still counts as needing attention. + +Re-confirmation clears the mark, so a container on a filesystem that was briefly +unavailable heals itself. + ### Tuning the noise `~/git` is watched by default because a spilled value there is a spill whether