Repository navigation
AIDA-2346: Keep spoilers hidden in reminder previews - #2261
Conversation
Co-authored-by: BuzzPRReviewAssistant1 <a581a7e8115222acdc32ffb14bfce55d48c24478d98e155f8eba126b69b83c7b@buzz.block.builderlab.xyz> Signed-off-by: Ravneet Arora <[email protected]>
loganj
left a comment
loganj
left a comment
There was a problem hiding this comment.
🤖 Larry and Auto: an independent adversarial review of 8a2030b. Source review only; we did not test the installed desktop app. Larry and Auto each reviewed the PR without seeing the other's findings. We reproduced every finding below in Vitest on this head.
Approving, on condition that the two must-fix items are fixed before merge. Both are small and well scoped:
- error, must-fix: OS message notifications still show a spoiler that crosses the 4096-character cut (inline on
content.ts:79). - error, must-fix: new reminders hide literal code pipes (inline on
RemindersPage.tsx:515).
Other items:
- question:
pluginNotificationTextnow gives every plugin body spoiler semantics. For example, a GitHub plugin bodyfix a || b || cbecomesfix a [Spoiler] c. Is that intended for plugin text that is not Buzz message Markdown? If not, apply the redaction only to message-derived text. - advice:
content.ts:67-77duplicates the renderer's delimiter-marker step (protectInlineContentinMessageMarkdown.tsx), with its own collision check. One owner inmarkdown-preparation.ts, which already ownsspoilerDelimiters, would mean one place to fix bugs like the next item. - advice, follow-up (also in the renderer, not a regression): an inline HTML declaration can swallow the marker.
hi <!x ||Rosebud||previews ashi <!x <>Rosebud||, andMessageMarkdownshows the same leak. Suggest one follow-up that fixes both paths. - advice, perf: a preview that contains
||now parses Markdown about four times. That is bounded to 4096 characters, butInboxPage.tsx:1163runs it unmemoized for each row on every render. Reuse theprepareMarkdownscan or memoize there. - advice: the Inbox row previews (
InboxPage.tsx:1163) now redact too. That is good, but the PR body and tests do not mention it. - advice, follow-up (pre-existing, outside AIDA-2346): these surfaces still show raw spoiler text:
- search message results (
useSearchMessages.ts:94) - search channel detail and the Solo preview (
channel.previewis rawrow.contentfrommembership.ts:53, used atSearchResults.tsx:976andSoloDestination.tsx:192) - the Buzz link hover card (
BuzzLinkPreview.tsx:159) - the audience row
aria-label(MessageRow.tsx:1392)
- search message results (
- preference: when the 4096 cut applies,
hideUnclosedhides every unclosed||in the message, not only the block at the cut. This hides extra text but never exposes any. - preference: the 200-character
shortTextcut can split the label itself ([Sp…).
Tests: the new tests prove the main claim. With the four source files reset to the merge base, 22 of them fail. Two gaps match the must-fix items: no test goes through the real notification input (a pre-cut previewContent), and no test saves a new reminder for a message with literal code pipes and then reads it back.
Fine as is: spoilers inside code spans, fences and links stay literal, the same as the renderer. Escaped pipes, entities, emphasis around delimiters, multi-line spoilers, headings, quotes, lists, |||x||| and HTML inside a spoiler also match the renderer, and none of them exposed spoiler text.
Checks: on this head, Auto ran the full Vitest suite (875 files, 15,477 passed, 21 skipped) and tsc --noEmit (clean). Larry ran the affected directories (105 files, 2,621 passed).
| for (const offset of [...prepared.spoilerDelimiters].reverse()) | ||
| source = source.slice(0, offset) + delimiter + source.slice(offset + 2); | ||
| const { tree, tooDeep } = scanMarkdown(source); | ||
| if (!tooDeep) remarkSpoilers(delimiter, content.length > end)(tree); |
There was a problem hiding this comment.
error, must-fix: this check never fires for OS message notifications. messageNotificationText passes previewContent, and session.ts:3392 already cut that with content.slice(0, 4096), so content.length > end is always false on that path.
Repro: "The answer is ||Rosebud. " + "filler ".repeat(700) + "||".
- Through
messageNotificationText, the body isThe answer is ||Rosebud. filler filler …. messagePreviewon the full text returnsThe answer is [Spoiler].
The "secret ".repeat(700) test passes uncut content straight to messagePreview, so it misses the real path.
Suggested fix: keep bounding in one place. Either stop the pre-cut in session.ts and let plainText do the cut, or have the producer send a truncation flag (slicing to 4097 would also work). Add a test that goes through messageNotificationText with a pre-cut body. This also makes the new buzz-help.md line about notifications true.
There was a problem hiding this comment.
🤖 Fixed in db17c75ff in follow-up PR #2285. #2261 merged before this follow-up could be published. The live session keeps its 4,096-character preview bound and carries previewTruncated when the source continues. messageNotificationText passes that provenance to the spoiler formatter, which hides an unfinished spoiler after a cut. Coverage now includes your pre-cut notification example and a signed live event through the actual session delivery path into messageNotificationText. An ordinary short unfinished delimiter remains literal.
| pending && reminder.notBefore !== undefined && reminder.notBefore <= now; | ||
| const preview = reminder.target?.preview | ||
| ? messagePreview(reminder.target.preview) | ||
| ? messagePreview(reminder.target.preview, { storedPreview: true }) |
There was a problem hiding this comment.
error, must-fix: this conservative read also hits every new reminder whose message has literal pipes. RemindDialog.tsx:49 saves messagePreview(content), and that output keeps literal || from code spans, escapes and entities. This PR's own test expects `||literal||` to become ||literal||.
Repro: the message run `a || b` now is saved as run a || b now. The reminder row and the due banner then both show run a [Spoiler].
The comment at content.ts:57-59 ("newly saved redacted previews contain no such delimiter") is therefore not true. The Risk section describes this only for old saved previews.
Suggested fix, verified by Auto: have RemindDialog save preview.replaceAll("||", "\\|\\|"). The conservative reader then finds no raw ||, so new rows read back as run a || b now, and old rows still take the conservative path. A version marker on the saved preview would also work.
There was a problem hiding this comment.
🤖 Fixed in db17c75ff in follow-up PR #2285. #2261 merged before this follow-up could be published. RemindDialog redacts real spoilers, then escapes all CommonMark ASCII punctuation in the stored preview, including pipes and backslashes. One Markdown conversion restores the literal text in rows and banners; old ambiguous previews retain conservative redaction. Added save → serialized storage → RemindersPage cases for code pipes, paired pipes, escapes, entities, backslashes and real spoilers, plus banner tests through NotificationsService. Both Chromium and WebKit preserve run a || b || c now after saving. The author explicitly scoped this change to the current app; older clients that display the stored preview directly may show escapes.
| (text) => text && plainText(text), | ||
| ) ?? "A reminder is waiting", | ||
| [ | ||
| plainText(reminder.target?.preview ?? "", { |
There was a problem hiding this comment.
advice: the due banner is now flattened twice. These calls run plainText, and then notify.submit runs pluginNotificationText, which flattens again. On main the note was flattened only once.
Effects:
- A note
\*urgent\*shows asurgent; on main it shows*urgent*. - A preview of
`a || b || c`becomesa [Spoiler] c.
Suggestion: redact the saved preview once, pass the note raw, and let the plugin-notification step do the single flatten. This interacts with the fix on RemindersPage.tsx:515.
There was a problem hiding this comment.
🤖 Fixed in db17c75ff in follow-up PR #2285. #2261 merged before this follow-up could be published. The reminder producer now removes only an ambiguous legacy spoiler tail before submission, and passes notes without flattening them. The selected source is converted once by pluginNotificationText in NotificationsService. plainText no longer applies Buzz spoiler semantics to generic plugin text. The real notification-service test covers an escaped urgent note, literal/paired pipes, and an old unfinished spoiler preview.
| let bounded = content.slice(0, end); | ||
| // Old flattened text cannot distinguish a real closing delimiter from | ||
| // literal pipes inside the hidden text. Hide everything after its first | ||
| // possible opener; newly saved redacted previews contain no such delimiter. |
There was a problem hiding this comment.
convention: this comment is not accurate. Newly saved previews can contain ||, because literal pipes from code, escapes and entities survive messagePreview (see the comment on RemindersPage.tsx:515). Update the comment along with the fix.
There was a problem hiding this comment.
🤖 Fixed in db17c75ff in follow-up PR #2285. #2261 merged before this follow-up could be published. Removed the inaccurate claim about new previews. The legacy-redaction helper now documents lost code/escape provenance, and the save boundary documents its punctuation encoding for one later Markdown conversion. Regression tests cover literal pipes surviving both row and notification reads.
|
🤖 Follow-up in db17c75ff addresses both must-fix items, the double-flattening regression, the inaccurate comment, and the plugin scope question. Published as follow-up PR #2285, because #2261 merged and its branch was deleted before this update could be pushed. The follow-up is based on that merge. Required TypeScript/related-unit and design checks passed on its head; the Chromium/WebKit workflows passed again after rebasing.
Disposition of the remaining nonblocking advice and preferences:
Before rebasing onto the merge of #2261: 109 reminder/formatter tests, 165 live-delivery tests and 129 Inbox tests passed; TypeScript passed. Chromium/WebKit component checks passed for both spoilers and new literal-pipe reminders. The full Vitest run reported 15,486 passed, 1 failed, 21 skipped. The failure was Human acceptance, the installed desktop app, OS banners and live relay persistence remain unverified. Browser evidence uses the actual components with local fixture storage. No new browser journeys were added or removed. |
…xecution-support-for-native-buzz-33ea2bea * origin/main: (74 commits) feat(host): harden byte lifecycle and add protected image upload (#2199) Updated onboarding (#2183) perf(relay): read each signed roster once in large channels (#2244) AIDA-2346: Keep spoilers hidden in reminder previews (#2261) fix: AIDA-2371 Keep sidebar selection pill aligned during resize (#2182) fix(relay): skip background rereads of profiles a read found absent (#2147) fix: prevent startup logo selection (#1823) fix(agents): send sign-in evidence with agent Git credentials (#2240) perf(mentions): stop scanning every member on each mention keystroke (#2246) fix(attachments): send image dimensions so previews don't shift (#2262) feat(shortcuts): add Slack-style hotkeys (#2170) perf(plugins): share host request clients across calls (#2243) fix(identity): keep federated sessions and agent grants in one keychain item (#2225) fix(profiles): evict the least recently requested profile (#2255) fix(unread): repair unread from the recovery tick only every 15 minutes (#2252) AIDA-2374: Use readable labels in chat titles (#2185) perf(profiles): keep verified names checked through a planned renewal (#2234) feat(plugins): let a plugin ask Buzz to review an install (#2059) fix(identity): sign FI builds in at first run through the native session (FOUNDATION: App.tsx, services.ts) (#2249) Place profile verification beside the verified name (#2224) ... Signed-off-by: Daniel Rouleau <[email protected]>
* commit 'c8e882165': (50 commits) Scope execution targets by harness session Read durable execution state before admission feat(host): harden byte lifecycle and add protected image upload (#2199) Updated onboarding (#2183) perf(relay): read each signed roster once in large channels (#2244) AIDA-2346: Keep spoilers hidden in reminder previews (#2261) fix: AIDA-2371 Keep sidebar selection pill aligned during resize (#2182) fix(relay): skip background rereads of profiles a read found absent (#2147) fix: prevent startup logo selection (#1823) fix(agents): send sign-in evidence with agent Git credentials (#2240) perf(mentions): stop scanning every member on each mention keystroke (#2246) fix(attachments): send image dimensions so previews don't shift (#2262) feat(shortcuts): add Slack-style hotkeys (#2170) perf(plugins): share host request clients across calls (#2243) fix(identity): keep federated sessions and agent grants in one keychain item (#2225) fix(profiles): evict the least recently requested profile (#2255) fix(unread): repair unread from the recovery tick only every 15 minutes (#2252) AIDA-2374: Use readable labels in chat titles (#2185) perf(profiles): keep verified names checked through a planned renewal (#2234) feat(plugins): let a plugin ask Buzz to review an install (#2059) ... Signed-off-by: Daniel Rouleau <[email protected]> Co-authored-by: Codex <[email protected]>
…ection * origin/main: (39 commits) Authorize local apps to use Buzz MCP tools (block#2211) feat(bestie): choose the private channels Bestie can read (block#2036) Polish message actions, spacing, and membership notices (block#2258) test(unread): prevent startup catch-up in receipt reserve journey (block#2274) test(search): wait for rejected join clearance to settle (block#2278) test(reactions): provide stable emoji media for icon rendering (block#2279) feat(profiles): reserve profile room for an open channel's roster (block#2269) feat(host): harden byte lifecycle and add protected image upload (block#2199) Updated onboarding (block#2183) perf(relay): read each signed roster once in large channels (block#2244) AIDA-2346: Keep spoilers hidden in reminder previews (block#2261) fix: AIDA-2371 Keep sidebar selection pill aligned during resize (block#2182) fix(relay): skip background rereads of profiles a read found absent (block#2147) fix: prevent startup logo selection (block#1823) fix(agents): send sign-in evidence with agent Git credentials (block#2240) perf(mentions): stop scanning every member on each mention keystroke (block#2246) fix(attachments): send image dimensions so previews don't shift (block#2262) feat(shortcuts): add Slack-style hotkeys (block#2170) perf(plugins): share host request clients across calls (block#2243) fix(identity): keep federated sessions and agent grants in one keychain item (block#2225) ... # Conflicts: # src/features/messages/ThreadPanel.tsx
## Why [AIDA-2346](https://linear.app/squareup/issue/AIDA-2346): a message hides `Rosebud` in `The answer is ||Rosebud||`, but its reminder exposes and saves the secret in plain text. ## What Replace spoiler content with `[Spoiler]` before previews reach reminder storage. Protect existing reminder previews and due notifications too. Open the original message to reveal the spoiler. ## How Reuse the message renderer's spoiler rules before text flattening and length limits. Preserve literal code and escaped pipes when reading original message source. Cover delimiter collisions, emphasis boundaries and closing delimiters beyond the source limit. Old saved previews lost formatting and may have lost closing delimiters. Hide their remainder from the first `||` onward, since literal pipes inside a spoiler cannot be distinguished safely. ## Risk The shared preview generator also affects message and plugin notifications. Conservative handling can hide extra text in saved previews with literal pipes. Previously stored bytes are not rewritten. ## Testing On rebased commit `8a2030bda17b722d06f74163e7fbc5a988078562`, exercised production `MessageMarkdown`, `RemindDialog` and `RemindersPage` in a local browser fixture: 1. Show `The answer is ||Rosebud||`; confirm the source spoiler is hidden. 2. Select **Set a reminder**, then **Set reminder**. 3. Confirm both the saved preview argument and reminder row contain `The answer is [Spoiler]`. 4. Reveal the original message; confirm the reminder stays redacted. Also checked old reminder rows in Chromium and WebKit, light/dark themes, 1000px/360px widths, including 200% text. No page errors or document horizontal overflow in those checks. Reused existing fixture components and in-memory storage; no browser journeys were added or removed. Parser and storage-boundary regression cases live in colocated unit/component tests. Required pre-push TypeScript, related unit tests and design checks passed on the rebased commit. No hooks were bypassed. Earlier local validation before rebasing: full Vitest run **15,361 passed, 21 skipped**; final four affected test files **102 passed**; TypeScript and production build passed. Independent review found no remaining blockers after fixes. These counts describe the earlier snapshot, not the rebased commit. Human acceptance, installed desktop behavior, native banners and live relay persistence/relaunch remain untested. This PR stays draft pending human testing. [View the local screenshot in Buzz](https://buzz.block.builderlab.xyz/media/4ac3dee43dc990ec5f86043ad5e70464ed6dc77bad6dc26e07554ca44b77ff86.png) (Buzz sign-in required). The screenshot is from the earlier validation; the same flow passed again on the rebased commit above. Native GitHub attachment upload is unavailable in the installed CLI, so the screenshot is linked rather than embedded. Generated with Codex Signed-off-by: Ravneet Arora <[email protected]> Co-authored-by: BuzzPRReviewAssistant1 <a581a7e8115222acdc32ffb14bfce55d48c24478d98e155f8eba126b69b83c7b@buzz.block.builderlab.xyz>
Why
AIDA-2346: a message hides
RosebudinThe answer is ||Rosebud||, but its reminder exposes and saves the secret in plain text.What
Replace spoiler content with
[Spoiler]before previews reach reminder storage. Protect existing reminder previews and due notifications too. Open the original message to reveal the spoiler.How
Reuse the message renderer's spoiler rules before text flattening and length limits. Preserve literal code and escaped pipes when reading original message source. Cover delimiter collisions, emphasis boundaries and closing delimiters beyond the source limit.
Old saved previews lost formatting and may have lost closing delimiters. Hide their remainder from the first
||onward, since literal pipes inside a spoiler cannot be distinguished safely.Risk
The shared preview generator also affects message and plugin notifications. Conservative handling can hide extra text in saved previews with literal pipes. Previously stored bytes are not rewritten.
Testing
On rebased commit
8a2030bda17b722d06f74163e7fbc5a988078562, exercised productionMessageMarkdown,RemindDialogandRemindersPagein a local browser fixture:The answer is ||Rosebud||; confirm the source spoiler is hidden.The answer is [Spoiler].Also checked old reminder rows in Chromium and WebKit, light/dark themes, 1000px/360px widths, including 200% text. No page errors or document horizontal overflow in those checks. Reused existing fixture components and in-memory storage; no browser journeys were added or removed. Parser and storage-boundary regression cases live in colocated unit/component tests.
Required pre-push TypeScript, related unit tests and design checks passed on the rebased commit. No hooks were bypassed.
Earlier local validation before rebasing: full Vitest run 15,361 passed, 21 skipped; final four affected test files 102 passed; TypeScript and production build passed. Independent review found no remaining blockers after fixes. These counts describe the earlier snapshot, not the rebased commit.
Human acceptance, installed desktop behavior, native banners and live relay persistence/relaunch remain untested. This PR stays draft pending human testing.
View the local screenshot in Buzz (Buzz sign-in required). The screenshot is from the earlier validation; the same flow passed again on the rebased commit above. Native GitHub attachment upload is unavailable in the installed CLI, so the screenshot is linked rather than embedded.
Generated with Codex