rbrooks/Quest-Board

Watch

Fork 0

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

New issue

Closed

opened 2026-08-25 20:44:47 +00:00 by claude-bot · 2 comments

claude-bot commented

2026-08-25 20:44:47 +00:00

Collaborator

Severity: MEDIUM

Found in the August 2026 session lifecycle review (#319).

The privacy-erasure feature that redacts a departing member's lines from transcripts and deletes their quote highlights identifies "their" lines by string-matching a set of display/character names — so two members who happen to share a name have their lines conflated, meaning erasing the departing member can permanently redact and delete the staying member's dialogue and quotes too, with no way to undo it since the scrub is designed to be idempotent and irreversible.

Evidence

webapp/backend/app/tasks/reminder_tasks.py:2896-2903 (run_member_erasure) — builds labels: set[str] from member.character_name and user.display_name only; no per-line speaker identity beyond string equality against these labels.
webapp/backend/app/tasks/reminder_tasks.py:2974-2980 — quote highlights are deleted via SessionHighlight.speaker.in_(list(labels)), which matches by the same bare string set, campaign-wide.
Because transcript redaction and the highlight delete both key off string membership in labels, two members sharing a display name or character name ("Alex") cause the staying member's transcript lines to be redacted to [removed] and their quote highlights permanently deleted along with the departing member's — and since the scrub is explicitly idempotent/one-way, there's no re-run that fixes it once source data has already been erased.
There is also an under-delete direction of the same root cause: speakers.json labels are captured from the Discord display name at recording time (bot/questboard_bot/cogs/recording.py, the display-name resolution around the WAV-conversion loop), which can drift from the app's current display_name, so old sessions can retain a departed member's lines under a name that no longer matches — a privacy gap rather than a data-loss bug, but the same root cause.

Failure scenario
A campaign has two players named "Alex" — one who has since left the group and requests erasure, and one who is still an active player. The erasure job builds its label set from the leaver's display/character name, matches it against every session's transcript and highlights by string, and redacts and deletes the staying Alex's lines and quotes too, with no way to tell after the fact which Alex's content was actually removed.

Proposed fix
Disambiguate by a verified, stable identity — the Discord platform link (PlatformLink.platform_user_id), which is already looked up for the audio-skip decision in the same function — rather than free-text name matching, wherever the transcript format allows it (this is a strong argument for persisting per-segment rows with a track_owner_id, which would make this exact and trivial). Until segment rows exist, at minimum warn/block the erasure flow when the resolved label set is ambiguous (matches more than one active member in the campaign) rather than proceeding silently.

Acceptance criteria

Erasure no longer identifies a member's lines/quotes purely by string match against a name that could belong to more than one campaign member.
The erasure flow detects and surfaces (blocks or warns on) an ambiguous label set before performing an irreversible scrub.
Regression test: two members sharing a display name in the same campaign; erasing one leaves the other's transcript lines and highlights intact.

**Severity: MEDIUM** Found in the August 2026 session lifecycle review (#319). The privacy-erasure feature that redacts a departing member's lines from transcripts and deletes their quote highlights identifies "their" lines by string-matching a set of display/character names — so two members who happen to share a name have their lines conflated, meaning erasing the departing member can permanently redact and delete the staying member's dialogue and quotes too, with no way to undo it since the scrub is designed to be idempotent and irreversible. **Evidence** - `webapp/backend/app/tasks/reminder_tasks.py:2896-2903` (`run_member_erasure`) — builds `labels: set[str]` from `member.character_name` and `user.display_name` only; no per-line speaker identity beyond string equality against these labels. - `webapp/backend/app/tasks/reminder_tasks.py:2974-2980` — quote highlights are deleted via `SessionHighlight.speaker.in_(list(labels))`, which matches by the same bare string set, campaign-wide. - Because transcript redaction and the highlight delete both key off string membership in `labels`, two members sharing a display name or character name ("Alex") cause the staying member's transcript lines to be redacted to `[removed]` and their quote highlights permanently deleted along with the departing member's — and since the scrub is explicitly idempotent/one-way, there's no re-run that fixes it once source data has already been erased. - There is also an under-delete direction of the same root cause: `speakers.json` labels are captured from the Discord display name at recording time (`bot/questboard_bot/cogs/recording.py`, the display-name resolution around the WAV-conversion loop), which can drift from the app's current `display_name`, so old sessions can retain a departed member's lines under a name that no longer matches — a privacy gap rather than a data-loss bug, but the same root cause. **Failure scenario** A campaign has two players named "Alex" — one who has since left the group and requests erasure, and one who is still an active player. The erasure job builds its label set from the leaver's display/character name, matches it against every session's transcript and highlights by string, and redacts and deletes the staying Alex's lines and quotes too, with no way to tell after the fact which Alex's content was actually removed. **Proposed fix** Disambiguate by a verified, stable identity — the Discord platform link (`PlatformLink.platform_user_id`), which is already looked up for the audio-skip decision in the same function — rather than free-text name matching, wherever the transcript format allows it (this is a strong argument for persisting per-segment rows with a `track_owner_id`, which would make this exact and trivial). Until segment rows exist, at minimum warn/block the erasure flow when the resolved label set is ambiguous (matches more than one active member in the campaign) rather than proceeding silently. **Acceptance criteria** - [ ] Erasure no longer identifies a member's lines/quotes purely by string match against a name that could belong to more than one campaign member. - [ ] The erasure flow detects and surfaces (blocks or warns on) an ambiguous label set before performing an irreversible scrub. - [ ] Regression test: two members sharing a display name in the same campaign; erasing one leaves the other's transcript lines and highlights intact.

claude-bot added this to the v4.1.0 - Data Durability & Recovery milestone

2026-08-25 20:44:47 +00:00

claude-bot added the

enhancement

backend

labels

2026-08-25 20:44:47 +00:00

claude-bot commented

2026-08-29 01:21:27 +00:00

Author

Collaborator

Raising an interaction found while sweeping for inline audio deletion during #427, because these two compound.

erase_member_recordings deletes the member's per-speaker WAV immediately and irreversibly — wav.unlink() in reminder_tasks.py, no trash, no grace:

wav = audio_dir / f"{discord_user_id}.wav"
if wav.is_file():
    wav.unlink()

On its own that is defensible and probably correct: erasure is deliberate destruction at a data subject's request, and a 7-day grace on a "delete my data" action has its own privacy problem. I deliberately left it alone in #427 rather than applying the trash-and-grace rule there, because it is a genuine policy question rather than an oversight.

But this issue is that erasure can scrub the wrong person. Combine the two and a mis-targeted erasure destroys an innocent member's audio with no recovery path at all — while every other deletion route in the system now carries a minimum 7-day window. That asymmetry is worth a deliberate decision rather than being left implicit.

Three options, roughly:

Confirm the target before destroying. Cheapest, and the natural fit for this issue's disambiguation work — if the target is unambiguous the deletion is safe, so fixing the ambiguity fixes this too.
Give erasure the same grace as everything else. Simple and consistent, but weakens the erasure guarantee, and "we still have your audio for 7 days" is a poor answer to a deletion request.
Two-phase. Scrub the transcript and attribution immediately (the part that matters for privacy in practice), hold only the raw WAV briefly. More work, and arguably splitting hairs.

No change made — flagging it so the decision is recorded here alongside the disambiguation work, rather than discovered later.

Raising an interaction found while sweeping for inline audio deletion during #427, because these two compound. `erase_member_recordings` deletes the member's per-speaker WAV **immediately and irreversibly** — `wav.unlink()` in `reminder_tasks.py`, no trash, no grace: ```python wav = audio_dir / f"{discord_user_id}.wav" if wav.is_file(): wav.unlink() ``` On its own that is defensible and probably correct: erasure is deliberate destruction at a data subject's request, and a 7-day grace on a "delete my data" action has its own privacy problem. I deliberately left it alone in #427 rather than applying the trash-and-grace rule there, because it is a genuine policy question rather than an oversight. **But this issue is that erasure can scrub the wrong person.** Combine the two and a mis-targeted erasure destroys an innocent member's audio with no recovery path at all — while every other deletion route in the system now carries a minimum 7-day window. That asymmetry is worth a deliberate decision rather than being left implicit. Three options, roughly: - **Confirm the target before destroying.** Cheapest, and the natural fit for this issue's disambiguation work — if the target is unambiguous the deletion is safe, so fixing the ambiguity fixes this too. - **Give erasure the same grace as everything else.** Simple and consistent, but weakens the erasure guarantee, and "we still have your audio for 7 days" is a poor answer to a deletion request. - **Two-phase.** Scrub the transcript and attribution immediately (the part that matters for privacy in practice), hold only the raw WAV briefly. More work, and arguably splitting hairs. No change made — flagging it so the decision is recorded here alongside the disambiguation work, rather than discovered later.

claude-bot referenced this issue

2026-08-29 05:58:03 +00:00

v4.0.1 — retention safety and secrets at rest #435

claude-bot referenced this issue

2026-08-29 06:35:31 +00:00

[Backend] set_admin --email crashes instead of disambiguating when two identities share an email #436

rbrooks referenced this issue from a commit

2026-09-01 00:45:52 +00:00

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

claude-bot referenced this issue from a pull request that will close it,

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

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

claude-bot closed this issue

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

claude-bot referenced this issue 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 commented

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

Author

Collaborator

Fixed in #475 (73b33e0). Closing.

Criterion	Outcome
Erasure no longer identifies lines/quotes purely by an ambiguous name	Met
The flow detects and surfaces an ambiguous label set before scrubbing	Met — warned and recorded, not blocked; see below
Regression test: two members sharing a name; erasing one leaves the other intact	Met, for both transcripts and quotes

The evidence had partly decayed

The transcript half was already fixed before this was picked up: an earlier pass rebuilt scrubbing around transcript_segments.track_owner_id (#335), which is exactly the "persisting per-segment rows with a track_owner_id would make this exact and trivial" this issue argued for. That path was already correct and is untouched.

What remained was everything the owner key cannot reach:

Pre-#335 sessions, which have no rows, so names are the only tool available.
Quote highlights, which record a bare speaker name and have no owner column at all — seen_labels seeded straight from the raw display and character names, so the failure scenario was live here regardless of when the session was recorded.

Warn rather than block, and under-erase rather than over-erase

The issue offered "blocks or warns". Blocking the whole erasure over one shared name would refuse a legitimate privacy request because of an unrelated collision, so instead everything unambiguous or owner-attributable is erased exactly, and only the undecidable part is withheld — with ambiguous_labels and quotes_withheld in the audit context and warnings in the log, so an incomplete erasure says so rather than looking finished.

The direction matters: a withheld quote is a gap a person can still close by hand; a wrongly deleted one cannot be recovered. This issue's own framing agrees — it calls the under-delete direction "a privacy gap rather than a data-loss bug".

The WAV question from the comment above is resolved

The comment flagged that erase_member_recordings unlinks audio immediately with no grace, and that combined with mis-targeting that is unrecoverable. It turns out not to have been 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 own preferred option — "if the target is unambiguous the deletion is safe" — is satisfied because that path was never ambiguous. No change was needed, and the immediate-deletion policy stands as deliberate.

Two defects the mutation check found in my own work

Recording these because both would have shipped looking correct:

The regression test for this issue's 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.
The ambiguous-label filter was applied in two places five lines apart, and 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 an ambiguous label re-enters via the owner rows rather than the seed set.

12 new tests; 7 mutations all caught; full backend suite 1558 passed.

Fixed in #475 (`73b33e0`). Closing. | Criterion | Outcome | |---|---| | Erasure no longer identifies lines/quotes purely by an ambiguous name | Met | | The flow detects and surfaces an ambiguous label set before scrubbing | Met — warned and recorded, not blocked; see below | | Regression test: two members sharing a name; erasing one leaves the other intact | Met, for both transcripts and quotes | ## The evidence had partly decayed The transcript half was already fixed before this was picked up: an earlier pass rebuilt scrubbing around `transcript_segments.track_owner_id` (#335), which is exactly the "persisting per-segment rows with a track_owner_id would make this exact and trivial" this issue argued for. That path was already correct and is untouched. What remained was everything the owner key cannot reach: - **Pre-#335 sessions**, which have no rows, so names are the only tool available. - **Quote highlights**, which record a bare speaker *name* and have **no owner column at all** — `seen_labels` seeded straight from the raw display and character names, so the failure scenario was live here regardless of when the session was recorded. ## Warn rather than block, and under-erase rather than over-erase The issue offered "blocks or warns". Blocking the whole erasure over one shared name would refuse a legitimate privacy request because of an unrelated collision, so instead everything unambiguous or owner-attributable is erased exactly, and only the undecidable part is withheld — with `ambiguous_labels` and `quotes_withheld` in the audit context and warnings in the log, so an incomplete erasure says so rather than looking finished. The direction matters: a withheld quote is a gap a person can still close by hand; a wrongly deleted one cannot be recovered. This issue's own framing agrees — it calls the under-delete direction "a privacy gap rather than a data-loss bug". ## The WAV question from the comment above is resolved The comment flagged that `erase_member_recordings` unlinks audio immediately with no grace, and that combined with mis-targeting that is unrecoverable. It turns out not to have been 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 own preferred option — "if the target is unambiguous the deletion is safe" — is satisfied because that path was never ambiguous. No change was needed, and the immediate-deletion policy stands as deliberate. ## Two defects the mutation check found in my own work Recording these because both would have shipped looking correct: 1. **The regression test for this issue's 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. 2. **The ambiguous-label filter was applied in two places five lines apart**, and 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 an ambiguous label re-enters via the owner rows rather than the seed set. 12 new tests; 7 mutations all caught; full backend suite 1558 passed.