amadan
App review fixes: security, cross-thread correctness, UI polish (#18)
* server: bind reactions/edits to their thread (close cross-thread IDOR) react/unreact/edit gated on thread membership but acted on a client-supplied message_id without checking the message lived in that thread. Since message ids are sequential, a member of one thread could toggle reactions on — and read back the counts of — messages in private threads they're not in. Add messageInThread() and require it in the edit and react/unreact handlers. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * web: composer hugs the bottom — padding-bottom = safe-area inset only Drop the max(12px, …) floor so there's no fixed bottom padding on desktop (env() is 0); on mobile the safe-area inset alone clears the Home indicator. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * web: stop cross-thread bleed on fast thread switches Guards checked activeId/msgEls before an await but not after, so switching threads mid-decrypt could render a message (or apply an edit) into the wrong thread's feed and corrupt its msgEls/msgData. Re-check after every decrypt: - appendMessage bails if activeId changed during decrypt - the WS edit handler re-validates activeId + msgEls after decrypt - openThread gets a generation token (abandons a slow history load if you've switched away) and now clears per-thread composer state on switch (myReactions, staged attachment, half-typed text) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * web: rework message hover actions + restore burst grouping - .msg now shrinks to the bubble width (align-self), so a 'them' message's hover icons sit right beside the bubble instead of way off at the 78% edge. - Replace the three separately-positioned hover icons + floating timestamp with a single full-height .msg-actions strip beside the bubble: the hover zone spans the whole message height (move straight from anywhere in the bubble to an icon), and the icons + exact time sit in a row so they never overlap. - Restore the .msg.cont tight grouping for consecutive same-sender messages, so quick-succession bursts aren't spaced like separate messages. - Empty state: 'Tap ＋' → 'Tap the compose button' (the ＋ is the attach button now). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * web: restore the drag-and-drop 'Drop a photo to send' overlay Drag-and-drop upload still worked but the visual cue was lost in the rewrite (the .drop-overlay CSS was orphaned). Re-add the overlay element to the thread view, give .thread-view position:relative so it can cover the pane, and toggle it on dragover/dragleave/drop (relatedTarget check avoids child-flicker). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * web: tap 'N members' in the group header to open the member list Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> ---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
5 files changed,
+91
−67
db.go+9 −0hub.go+4 −2web/pages/index.html+1 −0web/static/chat.js+49 −15web/static/style.css+28 −50
diff --git a/db.go b/db.go| index 175ebe8..477c7a1 100644 |
| --- a/db.go |
| +++ b/db.go |
| @@ -681,6 +681,15 @@ func (db *DB) threadMember(threadID, userID string) bool { |
| return db.sql.QueryRow("SELECT 1 FROM thread_members WHERE thread_id = ? AND user_id = ?", threadID, userID).Scan(&x) == nil |
| } |
| +// messageInThread reports whether message id lives in threadID. Binds a |
| +// reaction/edit to the thread the client names (membership is checked |
| +// separately), so a member of one thread can't react to or edit messages in a |
| +// thread they don't belong to. |
| +func (db *DB) messageInThread(id int64, threadID string) bool { |
| + var x int |
| + return db.sql.QueryRow("SELECT 1 FROM messages WHERE id = ? AND thread_id = ?", id, threadID).Scan(&x) == nil |
| +} |
| + |
| // wrappedKeyFor returns a member's wrapped thread key (to unwrap client-side). |
| func (db *DB) wrappedKeyFor(threadID, userID string) (string, bool, error) { |
| var w string |
diff --git a/hub.go b/hub.go| index 2e9b1ee..158f7cc 100644 |
| --- a/hub.go |
| +++ b/hub.go |
| @@ -248,7 +248,8 @@ func (s *Server) handleWS(w http.ResponseWriter, r *http.Request) { |
| go s.pushNewMessage(m.Blob, in.ThreadID, token) |
| case "edit": |
| - if in.MessageID <= 0 || in.Blob == "" || len(in.Blob) > maxBlobBytes { |
| + if in.MessageID <= 0 || in.Blob == "" || len(in.Blob) > maxBlobBytes || |
| + !s.db.messageInThread(in.MessageID, in.ThreadID) { |
| continue |
| } |
| m, ok, err := s.db.editMessage(in.MessageID, in.Blob, userID, editWindow) |
| @@ -263,7 +264,8 @@ func (s *Server) handleWS(w http.ResponseWriter, r *http.Request) { |
| s.broadcastToThread(in.ThreadID, out) |
| case "react", "unreact": |
| - if in.MessageID <= 0 || !allowedReactions[in.Emoji] { |
| + if in.MessageID <= 0 || !allowedReactions[in.Emoji] || |
| + !s.db.messageInThread(in.MessageID, in.ThreadID) { |
| continue |
| } |
| if in.Type == "react" { |
diff --git a/web/pages/index.html b/web/pages/index.html| index e529029..c79284a 100644 |
| --- a/web/pages/index.html |
| +++ b/web/pages/index.html |
| @@ -67,6 +67,7 @@ |
| </div> |
| </header> |
| <div class="feed" id="feed"></div> |
| + <div class="drop-overlay hidden" id="drop-overlay">Drop a photo to send</div> |
| <div class="typing hidden" id="typing" aria-live="polite"></div> |
| <div class="composer-reply hidden" id="composer-reply"></div> |
| <div class="composer-edit hidden" id="composer-edit"></div> |
diff --git a/web/static/chat.js b/web/static/chat.js| index b8bff23..3b1bc11 100644 |
| --- a/web/static/chat.js |
| +++ b/web/static/chat.js |
| @@ -441,9 +441,19 @@ function renderApp() { |
| fileInput.value = ""; |
| }; |
| const pane = document.getElementById("thread-view"); |
| - pane.addEventListener("dragover", (e) => e.preventDefault()); |
| + const dropOverlay = document.getElementById("drop-overlay"); |
| + pane.addEventListener("dragover", (e) => { |
| + e.preventDefault(); |
| + if (dropOverlay) dropOverlay.classList.remove("hidden"); |
| + }); |
| + pane.addEventListener("dragleave", (e) => { |
| + // Hide only when the cursor truly leaves the pane — dragleave also fires when |
| + // crossing onto a child element, so check relatedTarget. |
| + if (dropOverlay && !pane.contains(e.relatedTarget)) dropOverlay.classList.add("hidden"); |
| + }); |
| pane.addEventListener("drop", (e) => { |
| e.preventDefault(); |
| + if (dropOverlay) dropOverlay.classList.add("hidden"); |
| const f = e.dataTransfer.files && e.dataTransfer.files[0]; |
| if (f) addAttachment(f); |
| }); |
| @@ -463,7 +473,7 @@ function renderThreadList() { |
| if (!threads.length) { |
| const empty = document.createElement("p"); |
| empty.className = "thread-empty-note"; |
| - empty.textContent = "No chats yet. Tap ＋ to start one."; |
| + empty.textContent = "No chats yet. Tap the compose button to start one."; |
| list.appendChild(empty); |
| return; |
| } |
| @@ -616,13 +626,15 @@ function refreshSeparator(el) { |
| // from one person is labelled once. Call after refreshSeparator (it reads whether |
| // a .time-sep now sits directly above). |
| function refreshSenderName(el) { |
| - const name = el.querySelector(":scope > .name"); |
| - if (!name) return; |
| const prevSib = el.previousElementSibling; |
| const afterSeparator = !!prevSib && prevSib.classList.contains("time-sep"); |
| const prev = prevMsgOf(el); |
| const sameSender = !!prev && prev.dataset.sender === el.dataset.sender; |
| - name.style.display = !afterSeparator && sameSender ? "none" : ""; |
| + // Continuation of a same-sender run (no separator between) → tighter top gap |
| + // (.cont), so quick-succession messages aren't spaced like separate bursts. |
| + el.classList.toggle("cont", sameSender && !afterSeparator); |
| + const name = el.querySelector(":scope > .name"); |
| + if (name) name.style.display = !afterSeparator && sameSender ? "none" : ""; |
| } |
| function showList() { |
| @@ -660,9 +672,12 @@ function refreshAvatars() { |
| if (activeId) paintHeaderAvatar(activeId); |
| } |
| +let openSeq = 0; // bumped each openThread so a slow history load bails if you switch away |
| + |
| async function openThread(id) { |
| stopTyping(); // stop beating into the thread we're leaving |
| resetTyping(); // clear any "X is typing…" carried over from another thread |
| + const myOpen = ++openSeq; |
| activeId = id; |
| unread[id] = false; |
| document.getElementById("thread-empty").classList.add("hidden"); |
| @@ -674,6 +689,9 @@ async function openThread(id) { |
| const sub = document.getElementById("thread-sub"); |
| const n = t ? (t.members || []).length : 0; |
| sub.textContent = isGroup ? `${n} member${n === 1 ? "" : "s"}` : ""; |
| + // Tapping "N members" opens the member list (groups only). |
| + sub.onclick = isGroup ? () => showMembers() : null; |
| + sub.style.cursor = isGroup ? "pointer" : ""; |
| document.getElementById("thread-menu-wrap").style.display = isGroup ? "" : "none"; |
| paintHeaderAvatar(id); |
| document.getElementById("tt").classList.add("show-thread"); |
| @@ -683,17 +701,25 @@ async function openThread(id) { |
| feed.innerHTML = ""; |
| msgEls = {}; |
| msgData = {}; |
| + myReactions.clear(); // per-thread highlight; repopulated from each message's `mine` |
| + clearPending(); // don't carry a staged photo (encrypted to the old thread's key) over |
| cancelEdit(); |
| cancelReply(); |
| + const composerText = document.getElementById("text"); |
| + if (composerText) { |
| + composerText.value = ""; // drop any half-typed text from the previous thread |
| + composerText.dispatchEvent(new Event("input")); |
| + } |
| try { |
| const r = await ( |
| await fetch(`/api/messages?token=${encodeURIComponent(session.token)}&thread=${encodeURIComponent(id)}`) |
| ).json(); |
| + if (myOpen !== openSeq) return; // switched threads while loading — abandon this load |
| for (const m of r.messages || []) await appendMessage(id, m, false); |
| } catch (e) { |
| console.error("load messages", e); |
| } |
| - feed.scrollTop = feed.scrollHeight; |
| + if (myOpen === openSeq) feed.scrollTop = feed.scrollHeight; |
| if (!isCoarse) document.getElementById("text").focus(); |
| } |
| @@ -707,6 +733,7 @@ async function appendMessage(threadId, m, scroll = true) { |
| } catch { |
| return; // can't decrypt — skip |
| } |
| + if (threadId !== activeId) return; // switched threads mid-decrypt — don't bleed into the new feed |
| const feed = document.getElementById("feed"); |
| const mine = data.name === session.name; |
| const t = threads.find((x) => x.id === threadId); |
| @@ -761,21 +788,25 @@ async function appendMessage(threadId, m, scroll = true) { |
| if (isCoarse) { |
| attachSwipeReply(wrap, replyMeta); |
| } else { |
| - content.appendChild(buildHoverReact(m.id)); |
| - content.appendChild(buildHoverReply(replyMeta)); |
| + // Desktop: a full-height strip beside the bubble with the action icons and the |
| + // exact time, all revealed on hover (see .msg-actions). Full height so the |
| + // pointer can travel from anywhere in the bubble straight to an icon. |
| + const actions = document.createElement("div"); |
| + actions.className = "msg-actions"; |
| + actions.appendChild(buildHoverReact(m.id)); |
| + actions.appendChild(buildHoverReply(replyMeta)); |
| const remaining = m.created_at * 1000 + EDIT_WINDOW_MS - Date.now(); |
| if (mine && data.text && remaining > 0) { |
| const editBtn = buildHoverEdit(m.id); |
| - content.appendChild(editBtn); |
| + actions.appendChild(editBtn); |
| setTimeout(() => editBtn.remove(), remaining); // drop it when the window lapses |
| } |
| + const timeEl = document.createElement("span"); |
| + timeEl.className = "msg-time"; |
| + timeEl.textContent = formatTime(data.ts || m.created_at * 1000); |
| + actions.appendChild(timeEl); |
| + content.appendChild(actions); |
| } |
| - // Exact time, revealed on hover (desktop); the day/time context otherwise lives |
| - // in the separators. |
| - const timeEl = document.createElement("div"); |
| - timeEl.className = "msg-time"; |
| - timeEl.textContent = formatTime(data.ts || m.created_at * 1000); |
| - content.appendChild(timeEl); |
| wrap.appendChild(content); |
| // Reactions: track which are mine (server's per-message flag), render the chips |
| // below the bubble (outside .content), and wire the press-hold / double-tap picker. |
| @@ -1361,6 +1392,9 @@ function connectWS() { |
| if (msg.thread_id === activeId && msgEls[msg.id]) { |
| try { |
| const nd = await decrypt(threadKeys[msg.thread_id], msg.blob); |
| + // Re-validate after the await: a fast thread switch could have cleared |
| + // msgEls or changed the active thread while we were decrypting. |
| + if (msg.thread_id !== activeId || !msgEls[msg.id]) return; |
| const bubble = msgEls[msg.id].querySelector(".bubble"); |
| if (bubble) { |
| bubble.textContent = ""; |
diff --git a/web/static/style.css b/web/static/style.css| index 3b7dc99..418dda8 100644 |
| --- a/web/static/style.css |
| +++ b/web/static/style.css |
| @@ -223,9 +223,10 @@ input:focus, textarea:focus { outline: 2px solid var(--accent); border-color: tr |
| .msg { |
| display: flex; |
| flex-direction: column; |
| + align-self: flex-start; /* shrink to the bubble width (me overrides to flex-end) */ |
| max-width: 78%; |
| margin-top: 10px; |
| - position: relative; /* anchors the swipe-to-reply icon */ |
| + position: relative; /* anchors hover affordances + the swipe-to-reply icon */ |
| touch-action: pan-y; /* vertical scroll stays native; we handle horizontal swipes */ |
| } |
| /* continuation of a same-sender, same-minute run — tighter gap (see refreshGrouping) */ |
| @@ -282,35 +283,24 @@ input:focus, textarea:focus { outline: 2px solid var(--accent); border-color: tr |
| /* persistent "Edited" label under an edited bubble */ |
| .edited-tag { font-size: 11px; color: var(--muted); margin: 1px 6px 0; } |
| -/* exact per-message time, revealed on hover (desktop) just beyond the hover |
| - reply/edit icons so they don't overlap */ |
| -.msg-time { |
| +/* Hover affordances live in a full-height strip beside the bubble: the hover zone |
| + spans the whole message height, so you can move straight from anywhere in the |
| + bubble to an icon (no need to find the vertical centre), and the action icons + |
| + the exact time sit in a row so they never overlap. Revealed on hover (desktop). */ |
| +.msg-actions { |
| position: absolute; |
| - top: 50%; |
| - transform: translateY(-50%); |
| - font-size: 11px; |
| - color: var(--muted); |
| - white-space: nowrap; |
| + top: 0; |
| + bottom: 0; |
| + display: flex; |
| + align-items: center; |
| + gap: 2px; |
| opacity: 0; |
| pointer-events: none; |
| transition: opacity 0.12s; |
| } |
| -.msg.them .msg-time { left: 100%; margin-left: 34px; } |
| -.msg.me .msg-time { right: 100%; margin-right: 62px; } |
| -@media (hover: hover) { |
| - .msg:hover .msg-time { opacity: 1; } |
| -} |
| - |
| -/* desktop hover react/reply/edit icons — absolutely positioned just outside the |
| - bubble so they reveal on hover without shifting any content. They abut the |
| - bubble (no gap) so the pointer can travel from bubble to icon without losing |
| - the hover. */ |
| -.react-hover, |
| -.reply-hover, |
| -.edit-hover { |
| - position: absolute; |
| - top: 50%; |
| - transform: translateY(-50%); |
| +.msg.them .msg-actions { left: 100%; padding-left: 4px; } /* just right of the bubble */ |
| +.msg.me .msg-actions { right: 100%; padding-right: 4px; flex-direction: row-reverse; } /* just left */ |
| +.msg-actions button { |
| background: transparent; |
| color: var(--muted); |
| padding: 4px 6px; |
| @@ -318,27 +308,17 @@ input:focus, textarea:focus { outline: 2px solid var(--accent); border-color: tr |
| display: inline-flex; |
| align-items: center; |
| line-height: 1; |
| - opacity: 0; |
| - pointer-events: none; |
| - transition: opacity 0.12s; |
| } |
| -/* them: icons to the right of the bubble; me: to the left. React sits beside the |
| - reply icon; edit (your own in-window messages only) sits just beyond them. */ |
| -.reply-hover { right: -30px; } |
| -.react-hover { right: -58px; } |
| -.msg.me .reply-hover { right: auto; left: -30px; } |
| -.msg.me .react-hover { right: auto; left: -58px; } |
| -.msg.me .edit-hover { right: auto; left: -86px; } |
| -.react-hover svg, |
| -.reply-hover svg, |
| -.edit-hover svg { width: 18px; height: 18px; } |
| -.react-hover:hover, |
| -.reply-hover:hover, |
| -.edit-hover:hover { background: var(--bubble-them); color: var(--accent); } |
| +.msg-actions button:hover { background: var(--bubble-them); color: var(--accent); } |
| +.msg-actions button svg { width: 18px; height: 18px; } |
| +.msg-actions .msg-time { |
| + font-size: 11px; |
| + color: var(--muted); |
| + white-space: nowrap; |
| + margin: 0 4px; |
| +} |
| @media (hover: hover) { |
| - .msg:hover .react-hover, |
| - .msg:hover .reply-hover, |
| - .msg:hover .edit-hover { opacity: 1; pointer-events: auto; } |
| + .msg:hover .msg-actions { opacity: 1; pointer-events: auto; } |
| } |
| /* message content (text bubble + any attachments) */ |
| @@ -671,12 +651,10 @@ input:focus, textarea:focus { outline: 2px solid var(--accent); border-color: tr |
| display: flex; |
| gap: 8px; |
| align-items: flex-end; |
| - /* Bottom inset is dynamic, not fixed+inset: on desktop env() is 0 so it falls |
| - back to 12px (matching the 12px top — symmetric); on mobile it becomes the |
| - safe-area inset itself, clearing the Home indicator / Safari swipe bar |
| - without stacking an extra 12px on top of it. */ |
| + /* The input hugs the bottom: no fixed bottom padding (env() is 0 on desktop), |
| + just the safe-area inset on mobile to clear the Home indicator / swipe bar. */ |
| padding: 12px 16px; |
| - padding-bottom: max(12px, env(safe-area-inset-bottom, 0px)); |
| + padding-bottom: env(safe-area-inset-bottom, 0px); |
| border-top: 1px solid var(--border); |
| background: var(--panel); |
| } |
| @@ -906,7 +884,7 @@ input:focus, textarea:focus { outline: 2px solid var(--accent); border-color: tr |
| .empty-art { font-size: 42px; } |
| /* min-width:0 lets these panes shrink below their content so a long unbroken |
| URL wraps inside a bubble instead of widening the whole column. */ |
| -.thread-view { flex: 1; display: flex; flex-direction: column; min-height: 0; min-width: 0; width: 100%; } |
| +.thread-view { flex: 1; display: flex; flex-direction: column; min-height: 0; min-width: 0; width: 100%; position: relative; } |
| .thread-head { display: flex; align-items: center; gap: 6px; } |
| .thread-head-title { flex: 1; min-width: 0; display: flex; flex-direction: column; gap: 1px; } |
| .thread-title { font-weight: 700; font-size: 16px; line-height: 1.2; overflow: hidden; text-overflow: ellipsis; white-space: nowrap; } |