fix(bridges): R2 unguessable key + 30-day lifecycle + TG tables + Whisper budget#32
Merged
Merged
Conversation
…ention gate, reverse-map backfill Round-4 audit of the new code from tonight's 12 PRs (#21-#30). Three real bugs found and fixed in this PR. **#1 (HIGH) — R2 archive basename can leak inline env-var values** `archiveBasename("Bash", { command: "OPENAI_API_KEY=sk-xxx node srv.js" })` used the first whitespace-delimited token (`OPENAI_API_KEY=sk-xxx`) as the R2 object's basename, which lands in the *public* r2.dev URL the bridge emits as `📎 View full output`. Any inline env-var assignment in front of a Bash command would publish the value to anyone who reads the chat. Fix: walk tokens skipping `KEY=value` env vars + `nohup`/`time`/`sudo`/ `exec`/`env` prefixes; defensively strip any trailing `=…` even if the regex missed something; cap the final basename at 40 chars. New test exercises the exact leak vector. **#2 (MED) — mention gate skipped over E2EE and voice content** `shouldHandleMessage(roomId, ev)` was reading body/m.mentions from the OUTER ev.content. For E2EE rooms that's only the megolm ciphertext (no body/mentions). For voice messages it's `"Voice message.ogg"` (not the transcript). Result: in E2EE groups the bot stayed silent on explicit mentions; in any group with voice messages the bot couldn't trigger on the "Jada" wake-word even when the transcript started with it. Fix: introduce `mentionContent` alongside `body`/`msgtype`. For plain events it's `ev.content`; for E2EE it's `tryDecrypt`'s inner content; for voice it's a synthetic `{ msgtype: "m.text", body: transcript }`. Then call `shouldHandleMessage(roomId, { content: mentionContent })`. **#3 (MED) — cross-room guard backward-compat hole** PR #29's guard only checks the `replica:{id}` reverse mapping; the 47 existing room mappings that pre-date the guard have no reverse, so the check is a no-op for them. Until each room respawns its replica (could be days), the contamination guard does nothing. Fix: opportunistic backfill on every successful follow-up — if the reverse mapping is absent, write it (this room owns this replica). Race-safe: a colliding peer's NEXT follow-up sees mismatch and respawns. Same TTL as the forward mapping. **Validation** - 173/173 tests pass (+8 new — archive basename security suite) - Typecheck clean - Deployed `43e13059` Co-Authored-By: itsablabla <itsablabla@users.noreply.github.com> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…sper budget Sweeps the remaining audit notes from the night's work — 3 fixes, both bridges. **#1 R2 archive: unguessable key + 30-day lifecycle** The sha256-derived archive key let an attacker who could *guess* the content (e.g. the bytes of a public /etc/os-release dump) reconstruct the public r2.dev URL without ever seeing it. In E2EE rooms where the URL is the only thing crossing the encryption boundary, that's a real leak. Switched to a 128-bit random prefix (`crypto.getRandomValues(16)` → hex). Loses cross-room dedup, gains genuine unguessability. Also set a 30-day lifecycle rule on the `replicas-output-archive` bucket via the CF R2 API so objects auto-prune. Cost stays bounded as the bridge runs indefinitely. **#2 TG markdown tables → pre-formatted text** Telegram's HTML parse_mode doesn't support `<table>`, but it does support `<pre>`. Added a pre-pass to `markdown.ts` that detects GFM tables (header row + `|---|---|` separator), computes column widths, emits the same data inside a fenced code block with `│` / `─┼` box- drawing separators. The existing fenced-code path picks it up unchanged. 2 new tests. **#3 Whisper cost guardrail** A determined sender could fire 25-minute voice messages back-to-back to burn OpenAI credits at $0.006/min × 25 = $0.15/message. With mirror-mode they'd ALSO trigger TTS replies. Added a per-room rolling 60-min budget tracked in DO storage: when the room has already used > 30 minutes of Whisper time this hour, new voice msgs are skipped (same UX as the no-API-key path). Budget ledger keyed by `voice-budget:{roomId}` with entries pruned to the rolling window on every check. Bounded storage. **Validation** - Matrix: 173/173 ✅ · typecheck clean · deployed `159cbb42` - Telegram: 99/99 ✅ (+2 table tests) · typecheck clean · deployed `a9d1165f` - R2 lifecycle verified live via CF API GET Co-Authored-By: itsablabla <itsablabla@users.noreply.github.com> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
Capy auto-review is paused for this organization because the monthly auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Sweeps remaining audit notes from the night's work. 3 fixes across both bridges.
#1 R2 archive: unguessable key + 30-day lifecycle
The sha256-derived archive key let an attacker who could guess the content reconstruct the public r2.dev URL without ever seeing it. In E2EE rooms where the URL is the only thing crossing the encryption boundary, that's a real leak.
Switched to a 128-bit random prefix (
crypto.getRandomValues(16)→ hex). Loses cross-room dedup, gains genuine unguessability.Also set a 30-day lifecycle rule on
replicas-output-archivevia the CF R2 API. Cost bounded as the bridge runs indefinitely.#2 TG markdown tables → pre-formatted text
Telegram's HTML doesn't support
<table>but does support<pre>. Added pre-pass that detects GFM tables, computes column widths, emits same data inside a fenced code block with│/─┼box-drawing separators. 2 new tests.#3 Whisper cost guardrail
A determined sender could fire 25-minute voice messages back-to-back to burn OpenAI credits at $0.006/min × 25 = $0.15/message. With mirror-mode they'd ALSO trigger TTS replies. Added per-room rolling 60-min budget tracked in DO storage: when the room has already used > 30 min of Whisper time this hour, new voice msgs are skipped.
Ledger keyed by
voice-budget:{roomId}with entries pruned to the rolling window. Bounded storage.Validation
159cbb42a9d1165fTest plan
Workspace · Slack Thread