Reap findings whose location no longer holds the secret #1

Closed
opened 2026-09-02 15:50:06 +00:00 by grenade · 1 comment
Owner

The problem

A finding is keyed on (fingerprint, origin_path, origin_detail, byte_offset). Edit a file and every offset after the edit moves, so the next sweep inserts new rows beside the old ones rather than updating them. Nothing ever removes the old ones.

Observed on roosta: 14 stale rows across the two files touched during one session. Small today, but it grows with every edit, and it erodes the number at the top of nanny status — which is the number the whole tool is judged by.

Why the obvious fix is wrong

"Mark anything the last full sweep did not re-confirm as gone" conflates three different things:

  1. the secret genuinely is no longer there;
  2. the source was not read this sweep — skipped as too large, binary or unreadable, a permission that changed, a collector that failed;
  3. the source was read incrementally. This is the killer. Tail sources resume from a cursor, so a finding at offset 500 in a 40 MB transcript is never re-confirmed on any normal sweep. Reaping on "not re-confirmed" would resolve every real transcript spill on the machine.

nanny reporting an exposure as gone when it is not is the worst failure this codebase has. A reap needs a positive "I read all of this and it was not there", not the absence of a confirmation.

Design

Only reap where the whole container was read. Collectors report, per source, whether they read it from the beginning rather than resuming: whole-mode files always (and are skipped entirely when unchanged, in which case nothing is concluded), tail-mode files when rotation or truncation forced a rescan or --full cleared the cursor, the SQLite collector only when starting from an empty cursor. Sources not read in full this sweep are left alone.

Then two outcomes, which are genuinely different:

  • Superseded — the same fingerprint is still in the same file at a different offset. The finding did not go away, it moved. This is pure bookkeeping noise and the case behind all 14 observed rows. Merge into the successor, carrying first_seen and the operator's status and note where the successor is still untriaged, then delete the stale row. This makes triage survive a reformat, which it does not today.

  • Vanished — the fingerprint is gone from that file entirely. Record it; do not resolve it. A spill edited away is not a spill that never happened: the value already reached a model provider and the rotation may still be owed. So vanished_at is a timestamp column, orthogonal to status, and not a status of its own — a finding can be both acknowledged and vanished, and only the operator decides the first.

Re-confirmation clears vanished_at, so a file on a filesystem that was briefly unavailable heals itself rather than staying wrongly marked.

Also: a finding whose origin_path no longer exists is vanished. One stat per distinct path per sweep.

Scope

  • vanished_at on finding (migration)
  • ExcerptSink::note_complete_scan, defaulted to a no-op so a collector that cannot make the guarantee does not accidentally make it
  • FindingStore::reap
  • collectors report full reads; sweep reaps afterwards and reports counts
  • record() clears vanished_at on re-confirmation
  • CLI and TUI show vanished distinctly rather than hiding it

Non-goals

Deleting vanished findings. The record of a leak outlives the copy on disk, and it is the only thing that says a rotation is owed.

## The problem A finding is keyed on `(fingerprint, origin_path, origin_detail, byte_offset)`. Edit a file and every offset after the edit moves, so the next sweep *inserts* new rows beside the old ones rather than updating them. Nothing ever removes the old ones. Observed on roosta: 14 stale rows across the two files touched during one session. Small today, but it grows with every edit, and it erodes the number at the top of `nanny status` — which is the number the whole tool is judged by. ## Why the obvious fix is wrong "Mark anything the last full sweep did not re-confirm as gone" conflates three different things: 1. the secret genuinely is no longer there; 2. the source was not read this sweep — skipped as too large, binary or unreadable, a permission that changed, a collector that failed; 3. **the source was read incrementally.** This is the killer. Tail sources resume from a cursor, so a finding at offset 500 in a 40 MB transcript is never re-confirmed on any normal sweep. Reaping on "not re-confirmed" would resolve every real transcript spill on the machine. nanny reporting an exposure as gone when it is not is the worst failure this codebase has. A reap needs a positive "I read all of this and it was not there", not the absence of a confirmation. ## Design **Only reap where the whole container was read.** Collectors report, per source, whether they read it from the beginning rather than resuming: whole-mode files always (and are skipped entirely when unchanged, in which case nothing is concluded), tail-mode files when rotation or truncation forced a rescan or `--full` cleared the cursor, the SQLite collector only when starting from an empty cursor. Sources not read in full this sweep are left alone. **Then two outcomes, which are genuinely different:** - **Superseded** — the same fingerprint is still in the same file at a different offset. The finding did not go away, it moved. This is pure bookkeeping noise and the case behind all 14 observed rows. Merge into the successor, carrying `first_seen` and the operator's status and note where the successor is still untriaged, then delete the stale row. **This makes triage survive a reformat**, which it does not today. - **Vanished** — the fingerprint is gone from that file entirely. Record it; do not resolve it. A spill edited away is not a spill that never happened: the value already reached a model provider and the rotation may still be owed. So `vanished_at` is a timestamp column, orthogonal to `status`, and not a status of its own — a finding can be both acknowledged and vanished, and only the operator decides the first. Re-confirmation clears `vanished_at`, so a file on a filesystem that was briefly unavailable heals itself rather than staying wrongly marked. **Also:** a finding whose `origin_path` no longer exists is vanished. One stat per distinct path per sweep. ## Scope - `vanished_at` on `finding` (migration) - `ExcerptSink::note_complete_scan`, defaulted to a no-op so a collector that cannot make the guarantee does not accidentally make it - `FindingStore::reap` - collectors report full reads; sweep reaps afterwards and reports counts - `record()` clears `vanished_at` on re-confirmation - CLI and TUI show vanished distinctly rather than hiding it ## Non-goals Deleting vanished findings. The record of a leak outlives the copy on disk, and it is the only thing that says a rotation is owed.
Author
Owner

Implemented in bee34ef.

Measured on roosta

A nanny scan --full over 60,882 sources / 979 MB:

reaped 45 stale row(s) whose location shifted, 1 no longer on disk
before after
findings 1330 1285
rows for nanny-core/src/scan.rs 52 13

The 39 rows that went from one file are exactly the case the issue describes: that file was edited repeatedly during the session, and every edit shifted the offsets of everything below it.

Triage survived — the two ignored findings are still ignored, the suppression has caught 14 and the downgrade has lowered 12, all through the merges.

The one vanished finding is honest rather than incidental: crates/nanny-data/tests/end_to_end.rs had a hard-coded fixture token replaced with a generated one, so that value genuinely is no longer in that file. It shows as open and on disk: no, not as resolved.

What the tests pin

Six new end-to-end tests. The two that earn their place assert the negative:

  • a_tail_source_read_incrementally_is_never_reaped — appends to a transcript across three sweeps and asserts the finding near the start is still present. This is the one that would have retired every real transcript spill on the machine.
  • a_source_that_was_skipped_this_sweep_is_not_reaped — a quiet sweep stat-skips every whole-mode source, and must conclude nothing about any of them.

Plus: an offset-shifting edit merges rather than duplicating and carries the operator's status and note forward; a scrubbed secret is marked gone but stays open; a deleted file takes its findings with it; and a restored secret clears its own mark.

Deviation from the plan

None of substance. One thing became clearer while building it: the merge carrying triage forward is worth more than the row-count reduction. Triage that evaporates when someone runs a formatter is triage nobody does a second time.

Implemented in bee34ef. ## Measured on roosta A `nanny scan --full` over 60,882 sources / 979 MB: ``` reaped 45 stale row(s) whose location shifted, 1 no longer on disk ``` | | before | after | | --- | --- | --- | | findings | 1330 | 1285 | | rows for `nanny-core/src/scan.rs` | 52 | 13 | The 39 rows that went from one file are exactly the case the issue describes: that file was edited repeatedly during the session, and every edit shifted the offsets of everything below it. Triage survived — the two `ignored` findings are still ignored, the suppression has caught 14 and the downgrade has lowered 12, all through the merges. The one vanished finding is honest rather than incidental: `crates/nanny-data/tests/end_to_end.rs` had a hard-coded fixture token replaced with a generated one, so that value genuinely is no longer in that file. It shows as `open` and `on disk: no`, not as resolved. ## What the tests pin Six new end-to-end tests. The two that earn their place assert the negative: - `a_tail_source_read_incrementally_is_never_reaped` — appends to a transcript across three sweeps and asserts the finding near the start is still present. This is the one that would have retired every real transcript spill on the machine. - `a_source_that_was_skipped_this_sweep_is_not_reaped` — a quiet sweep stat-skips every whole-mode source, and must conclude nothing about any of them. Plus: an offset-shifting edit merges rather than duplicating *and* carries the operator's status and note forward; a scrubbed secret is marked gone but stays `open`; a deleted file takes its findings with it; and a restored secret clears its own mark. ## Deviation from the plan None of substance. One thing became clearer while building it: the merge carrying triage forward is worth more than the row-count reduction. Triage that evaporates when someone runs a formatter is triage nobody does a second time.
Sign in to join this conversation.
No Label
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: grenade/nanny#1