Harden presence titles using server-owned metadata#11
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughClient stops sending article titles in presence messages. The server now derives and caches titles from metadata using canonicalized slugs and a KV namespace. ChangesTitle Resolution Shift
Sequence DiagramsequenceDiagram
participant Client
participant PresenceDO
participant ARTICLES_KV
Client->>PresenceDO: {t:"r", s: slug}
PresenceDO->>PresenceDO: canonicalize slug via slugify
PresenceDO->>ARTICLES_KV: resolveTitle(slug)
ARTICLES_KV-->>PresenceDO: {title?: string}
PresenceDO->>PresenceDO: cache title in DO titles map
PresenceDO->>PresenceDO: broadcast presence with ti from cache
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
|
Thanks for taking this; I filed a follow-up issue to keep broader XSS hardening explicit: #12 |
Why
Implemented a security-focused presence hardening patch so the live "Currently Being Consulted" panel can’t be driven by untrusted title text from users.
What changed
ti/title fields from presence websocket messages.slugifybefore storing per-socket location.title) with slug fallback.Why this was necessary
I observed that a user could navigate with crafted input and affect the sidebar text, making the panel a likely vector for content poisoning. The previous approach trusted client-provided title values, so hostile names could appear in live UI panels that are otherwise read as authoritative app state.
Note
This patch focused on presence-title trust boundaries. I intentionally left broader input/output hardening (e.g., full comment/comment-thread XSS review) for follow-up, since that should likely be handled in a separate PR.
Summary by CodeRabbit