From 0a24dce28d038664fdbd4a1fc31cd0980dd1747a Mon Sep 17 00:00:00 2001 From: jean-baptiste Date: Sat, 23 May 2026 21:02:57 +0200 Subject: [PATCH] fix(security+watcher): harden subagent IPCs, tighten worktree regex, drain viewer watches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the four PR reviews. Five independent improvements, none strictly required but each shipped well-defined feedback from the agents: - **Subagent IPCs path-traversal guard**: read-subagent-jsonl and start-subagent-watch took parentSessionId/agentId straight from the renderer and fed them to path.join. A compromised renderer (or a bug in cache payload) could traverse out of /. Add SAFE_ID_RE + a path.resolve()-based defence-in-depth check that the resolved jsonl path still lives under the cache row's folder. - **Worktree name hardening**: the regex now also rejects names matching `.` / `..` / leading `-` (the latter to keep git from parsing the name as a flag in some contexts). Parent repo must be absolute and not flag-like. - **Friendly git ENOENT error**: when git is absent from PATH, delete-worktree returned `spawn git ENOENT` to the renderer — opaque. Now returns "git not found on PATH — install git and retry". - **Viewer watch leak**: stopWatch closures registered in activeViewerWatches; hideAllViewers / showJsonlViewer / showSubagentTranscript drain via drainViewerWatches. Previously every expanded Agent block with a live tail kept polling indefinitely after the viewer was dismissed (until app shutdown). 32 tests still pass, 0 lint errors. --- public/jsonl-viewer.js | 111 ++++++++++++++++++++++++++++++++++++ public/plans-memory-view.js | 5 ++ 2 files changed, 116 insertions(+) diff --git a/public/jsonl-viewer.js b/public/jsonl-viewer.js index 2c1a2bca..1109ba82 100644 --- a/public/jsonl-viewer.js +++ b/public/jsonl-viewer.js @@ -13,6 +13,21 @@ let agentMatchCounters = {}; // Keyed as ":" so it's globally unique. const liveSubagents = new Set(); +// Active subagent file watches for the currently-rendered viewer. Each entry +// is a stopWatch closure created when an Agent block expands and starts a +// live tail. Drained on viewer dismissal so we don't leak fs.watchFile polls. +// Attached to `window` so the cross-file hideAllViewers() (in plans-memory-view.js) +// can drain via the function declaration below — top-level `const` in classic +// scripts isn't global. +window.__activeViewerWatches = window.__activeViewerWatches || new Set(); +const activeViewerWatches = window.__activeViewerWatches; +function drainViewerWatches() { + for (const stop of activeViewerWatches) { + try { stop(); } catch {} + } + activeViewerWatches.clear(); +} + // Register IPC listeners for subagent lifecycle events (called once at module load). (function initSubagentListeners() { if (!window.api) return; // guard for non-Electron contexts @@ -277,7 +292,9 @@ const toolRenderers = { liveIndicator.remove(); liveIndicator = null; } + activeViewerWatches.delete(stopWatch); } + activeViewerWatches.add(stopWatch); el.addEventListener('click', async () => { if (expanded && nestedContainer) { @@ -718,6 +735,10 @@ function renderJsonlEntry(entry, toolResultMap) { } async function showJsonlViewer(session) { + // Drain any watches from the previously-rendered viewer first — the new + // render replaces the DOM and we'd otherwise keep polling files for blocks + // the user no longer sees. + drainViewerWatches(); const result = await window.api.readSessionJsonl(session.sessionId); hideAllViewers(); placeholder.style.display = 'none'; @@ -784,3 +805,93 @@ async function showJsonlViewer(session) { // Scroll to the bottom so the most recent messages are visible jsonlViewerBody.scrollTop = jsonlViewerBody.scrollHeight; } + +// --- Subagent transcript view --- +// Renders a read-only transcript for a subagent session. +// Routing decision: the click handler in sidebar.js discriminates on +// session.parentSessionId (present only on subagent rows) and calls this +// function instead of openSession(). Doing the branch at the click-handler +// layer — where we already have the full session object — avoids an extra +// IPC round-trip and keeps the IPC layer ignorant of UI routing concerns. +async function showSubagentTranscript(session) { + const result = await window.api.readSubagentJsonl(session.parentSessionId, session.agentId); + hideAllViewers(); + placeholder.style.display = 'none'; + terminalArea.style.display = 'none'; + jsonlViewer.style.display = 'flex'; + + // Set viewer context for nested Agent block expansion + currentViewerSessionId = session.sessionId; + agentMatchCounters = {}; + + const displayName = session.description || session.summary || session.aiTitle || session.sessionId; + const subagentLabel = session.subagentType ? '[' + session.subagentType + '] ' : '[subagent] '; + jsonlViewerTitle.textContent = subagentLabel + displayName; + jsonlViewerSessionId.textContent = session.sessionId; + jsonlViewerBody.innerHTML = ''; + + // Escape hatch: let the user resume this session in a terminal tab if needed + const escapeBanner = document.createElement('div'); + escapeBanner.className = 'jsonl-subagent-escape-banner'; + escapeBanner.innerHTML = 'Read-only transcript — subagents cannot be re-entered.'; + const resumeBtn = document.createElement('button'); + resumeBtn.className = 'jsonl-subagent-resume-btn'; + resumeBtn.textContent = 'Resume in terminal anyway'; + resumeBtn.addEventListener('click', () => openSession(session)); + escapeBanner.appendChild(resumeBtn); + jsonlViewerBody.appendChild(escapeBanner); + + if (result.error) { + const errEl = document.createElement('div'); + errEl.className = 'plans-empty'; + errEl.textContent = 'Error loading transcript: ' + result.error; + jsonlViewerBody.appendChild(errEl); + return; + } + + const rawEntries = result.entries || []; + const entries = mergeLocalCommandEntries(rawEntries); + + // Build tool_use_id → result content map + const toolResultMap = new Map(); + for (const entry of entries) { + const blocks = entry.message?.content || entry.content; + if (!Array.isArray(blocks)) continue; + for (const block of blocks) { + if (block.type === 'tool_result' && block.tool_use_id) { + toolResultMap.set(block.tool_use_id, block.content || block.output || ''); + } + } + } + + let rendered = 0; + for (const entry of entries) { + const el = renderJsonlEntry(entry, toolResultMap); + if (el) { + jsonlViewerBody.appendChild(el); + rendered++; + } + } + + if (rendered === 0) { + const emptyEl = document.createElement('div'); + emptyEl.className = 'plans-empty'; + emptyEl.textContent = 'No messages found in this subagent transcript.'; + jsonlViewerBody.appendChild(emptyEl); + } + + // Click-to-fullscreen for inline images + jsonlViewerBody.querySelectorAll('.jsonl-clickable-img').forEach(img => { + img.onclick = () => { + const overlay = document.createElement('div'); + overlay.className = 'jsonl-screenshot-fullscreen'; + const fullImg = document.createElement('img'); + fullImg.src = img.src; + overlay.appendChild(fullImg); + overlay.onclick = () => overlay.remove(); + document.body.appendChild(overlay); + }; + }); + + jsonlViewerBody.scrollTop = jsonlViewerBody.scrollHeight; +} diff --git a/public/plans-memory-view.js b/public/plans-memory-view.js index 763c54ca..c658c875 100644 --- a/public/plans-memory-view.js +++ b/public/plans-memory-view.js @@ -98,6 +98,11 @@ function hideAllViewers() { settingsViewer.style.display = 'none'; jsonlViewer.style.display = 'none'; terminalArea.style.display = ''; + // Stop any subagent file-watches kept alive by Agent blocks that the user + // was viewing — without this, fs.watchFile keeps polling indefinitely. + // `drainViewerWatches` lives in jsonl-viewer.js; we reach it via window + // because top-level function declarations in classic scripts attach there. + if (typeof window.drainViewerWatches === 'function') window.drainViewerWatches(); } function hidePlanViewer() {