rbrooks/Quest-Board

Watch

Fork 0

fix(privacy): stop erasure scrubbing a member who shares a name (#407) #475

Merged

claude-bot merged 1 commit from fix/407-erasure-ambiguity into main

2026-09-01 02:36:39 +00:00

claude-bot commented

2026-09-01 00:46:13 +00:00

Contributor

Closes #407.

What was left

Most of the transcript half was already fixed. An earlier pass rebuilt transcript scrubbing around transcript_segments.track_owner_id (#335), which settles attribution exactly for anything recorded since. That is untouched here. The issue's cited evidence had decayed accordingly — it describes labels driving everything, which is no longer true for rows-backed sessions.

What the owner key cannot reach was still live:

Sessions from before #335 have no rows, so names are the only tool.
Quote highlights record a bare speaker name and have no owner column at all, so seen_labels seeded straight from the raw display/character names — #407's exact scenario, regardless of rows.

The fix

_ambiguous_labels finds the names a departing member shares with somebody still in the campaign (display names and every character they ran, compared case- and whitespace-insensitively, the way a person would). Those names are withheld from every name-based decision. The authoritative owner key is untouched and still erases exactly.

Under-erasing is the right way to fail here. Withholding leaves a gap someone can close by hand; deleting the wrong person's quote cannot be undone. The audit context now carries ambiguous_labels and quotes_withheld, and both log warnings — so an incomplete erasure says so rather than looking like one that finished.

The WAV question from the earlier comment

The comment on this issue flagged that erase_member_recordings unlinks audio immediately with no grace, and that combined with mis-targeting that is unrecoverable.

It turned out not to be at risk. The filename is keyed on the verified PlatformLink Discord id, never a name, so it always deleted exactly the requesting member's track. The comment's preferred option — "if the target is unambiguous the deletion is safe" — is satisfied because that path was never ambiguous. Recorded in the commit so the question is not reopened from scratch.

Two things the mutation check caught in my own work

Worth flagging because both would have shipped looking fine:

My regression test for the headline scenario was vacuous. It used an [MM:SS] timestamp where _TRANSCRIPT_LINE_RE requires [HH:MM:SS], so the scrub matched nothing and the test passed whether or not the guard existed. Fixed, with the format noted inline.
I had filtered ambiguous labels in two places five lines apart — seeding seen_labels from the safe set and subtracting at the delete. Each hid the other: removing either left every test green. Collapsed to a single filter at the point of deletion, which a test now pins, including the path where the ambiguous label re-enters via the owner rows rather than the seed.

Verification

12 new tests. 7 mutations, all caught: disabling the check entirely, feeding ambiguous names to either the transcript scrub or the quote delete, recording nothing in the audit trail, case/whitespace-sensitive comparison, comparing a member against themselves, and ignoring character names.

Full backend suite 1558 passed (including the 12 pre-existing erasure tests, untouched); ruff check and ruff format --check clean.

🤖 Generated with Claude Code

Closes #407. ## What was left **Most of the transcript half was already fixed.** An earlier pass rebuilt transcript scrubbing around `transcript_segments.track_owner_id` (#335), which settles attribution exactly for anything recorded since. That is untouched here. The issue's cited evidence had decayed accordingly — it describes `labels` driving everything, which is no longer true for rows-backed sessions. What the owner key cannot reach was still live: - **Sessions from before #335** have no rows, so names are the only tool. - **Quote highlights** record a bare speaker *name* and have **no owner column at all**, so `seen_labels` seeded straight from the raw display/character names — #407's exact scenario, regardless of rows. ## The fix `_ambiguous_labels` finds the names a departing member shares with somebody still in the campaign (display names and every character they ran, compared case- and whitespace-insensitively, the way a person would). Those names are withheld from every name-based decision. The authoritative owner key is untouched and still erases exactly. **Under-erasing is the right way to fail here.** Withholding leaves a gap someone can close by hand; deleting the wrong person's quote cannot be undone. The audit context now carries `ambiguous_labels` and `quotes_withheld`, and both log warnings — so an incomplete erasure says so rather than looking like one that finished. ## The WAV question from the earlier comment The comment on this issue flagged that `erase_member_recordings` unlinks audio immediately with no grace, and that combined with mis-targeting that is unrecoverable. **It turned out not to be at risk.** The filename is keyed on the verified `PlatformLink` Discord id, never a name, so it always deleted exactly the requesting member's track. The comment's preferred option — "if the target is unambiguous the deletion is safe" — is satisfied because that path was never ambiguous. Recorded in the commit so the question is not reopened from scratch. ## Two things the mutation check caught in my own work Worth flagging because both would have shipped looking fine: 1. **My regression test for the headline scenario was vacuous.** It used an `[MM:SS]` timestamp where `_TRANSCRIPT_LINE_RE` requires `[HH:MM:SS]`, so the scrub matched nothing and the test passed whether or not the guard existed. Fixed, with the format noted inline. 2. **I had filtered ambiguous labels in two places five lines apart** — seeding `seen_labels` from the safe set *and* subtracting at the delete. Each hid the other: removing either left every test green. Collapsed to a single filter at the point of deletion, which a test now pins, including the path where the ambiguous label re-enters via the owner rows rather than the seed. ## Verification 12 new tests. **7 mutations, all caught:** disabling the check entirely, feeding ambiguous names to either the transcript scrub or the quote delete, recording nothing in the audit trail, case/whitespace-sensitive comparison, comparing a member against themselves, and ignoring character names. Full backend suite **1558 passed** (including the 12 pre-existing erasure tests, untouched); `ruff check` and `ruff format --check` clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

claude-bot added 1 commit

2026-09-01 00:46:13 +00:00

fix(privacy): stop erasure scrubbing a member who shares a name (#407 )

CI / Summarisation accuracy eval harness (stub provider) (pull_request) Successful in 47s

Details

CI / Bot/backend version sync (pull_request) Successful in 23s

Details

CI / Docker image build (pull_request) Successful in 27s

Details

CI / Backend lint (ruff) (pull_request) Successful in 57s

Details

CI / Bot tests and audit (pull_request) Successful in 1m33s

Details

CI / Frontend tests, audit, and build (pull_request) Successful in 3m16s

Details

CI / Backend migration, tests, and audit (pull_request) Successful in 5m54s

Details

e0340dd95a

Two players called Alex is an ordinary situation, not a corner case. Erasure
matched a departing member's content by their display name and character names,
and the scrub is deliberately irreversible — so erasing one Alex destroyed the
staying Alex's transcript lines and quote highlights, with nothing to restore
from and no way to tell afterwards whose content went.

Most of the transcript half was already fixed
---------------------------------------------
An earlier pass rebuilt transcript scrubbing around transcript_segments'
track_owner_id (#335), which settles attribution exactly for anything recorded
since. That is untouched here and still does the precise thing. What was left
was everything the owner key cannot reach:

- **Sessions from before #335**, which have no rows, so names are the only
  tool. Ambiguous names are now withheld from that fallback rather than used to
  redact somebody who might be the wrong person.
- **Quote highlights**, which record a bare speaker *name* and have no owner
  column at all. `seen_labels` seeded straight from the raw display/character
  names, so #407's exact scenario was live here regardless of rows.

Under-erasing is the right way to fail
--------------------------------------
Withholding leaves a gap someone can close by hand; deleting the wrong person's
quote cannot be undone. The audit context now carries `ambiguous_labels` and
`quotes_withheld`, and both are logged as warnings, so an incomplete erasure
says so instead of looking like one that finished.

The WAV deletion turned out not to be at risk. #407's comment flagged that
erase_member_recordings unlinks audio immediately with no grace, and that
combined with mis-targeting that is unrecoverable — but the filename is keyed
on the verified PlatformLink discord id, never a name, so it always deleted
exactly the requesting member's track. Recording that here so the question is
not reopened from scratch: the comment's preferred option ("if the target is
unambiguous the deletion is safe") is satisfied because that path was never
ambiguous.

Two things the mutation check caught in my own work
---------------------------------------------------
My regression test for the transcript case used an [MM:SS] timestamp where
_TRANSCRIPT_LINE_RE requires [HH:MM:SS]. It matched nothing, so it passed
whether or not the guard existed — a vacuous test for the issue's headline
scenario. Fixed, and the format is noted inline.

I had also filtered ambiguous labels in two places five lines apart, seeding
`seen_labels` from the safe set *and* subtracting at the delete. Each hid the
other: removing either left every test green. Collapsed to one filter at the
point of deletion, which a test now pins — including the path where the
ambiguous label re-enters via the owner rows rather than the seed.

Verification: 12 new tests, 7 mutations all caught, including disabling the
check, feeding ambiguous names to either the transcript scrub or the quote
delete, comparing names case-sensitively, comparing a member against
themselves, and ignoring character names. Full backend suite 1558 passed; ruff
clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

claude-bot merged commit 73b33e01fc into main

2026-09-01 02:36:39 +00:00

claude-bot deleted branch fix/407-erasure-ambiguity

2026-09-01 02:36:39 +00:00

claude-bot referenced this pull request from a commit

2026-09-01 02:36:41 +00:00

Merge PR #475: stop erasure scrubbing a member who shares a name (#407)

claude-bot referenced this pull request

2026-09-01 02:37:03 +00:00

[Backend] Disambiguate per-member erasure so it can't scrub the wrong person #407