feat(kiro): wire up new file-based session format (~May 2026) - #268
feat(kiro): wire up new file-based session format (~May 2026)#268Sean10 wants to merge 2 commits into
Conversation
Kiro CLI moved to per-session files under ~/.kiro/sessions/cli/:
<uuid>.json — metadata (session_id, cwd, title, created_at, updated_at)
<uuid>.jsonl — events: Prompt / AssistantMessage / ToolResults
- Add KIRO_SESSIONS_DIR + scanKiroCliSessions() and loadKiroCliDetail()
- Index the .jsonl files in _buildSessionFileIndex so they resolve to
{ format: 'kiro-cli' }, wiring detail/preview/search/replay/export
end-to-end (previously listed but "Session file not found" on open)
- Validate the sessionId as a strict UUID in loadKiroCliDetail before
path.join to close a path-traversal vector on untrusted input
- Add fixture-based tests covering scan, detail, path-traversal
rejection, index resolution, and end-to-end wiring
Old SQLite-based Kiro sessions remain fully supported alongside.
vakovalskii
left a comment
There was a problem hiding this comment.
Thanks for this — the file-based Kiro reader works and merges cleanly (all tests green, incl. the new ones). One blocker before merge, from a security pass:
Symlink traversal when reading session files (HIGH). The <uuid>.jsonl filenames are correctly UUID-validated (so ../ traversal is blocked), but fs.readFileSync(path.join(KIRO_SESSIONS_DIR, f)) still follows symlinks. A symlink named <valid-uuid>.jsonl inside ~/.kiro/sessions/cli/ pointing to e.g. ~/.ssh/id_rsa would be read and surfaced through the dashboard (detail / export / search / replay). This matters because codbash can be bound to a LAN address, and it's inconsistent with how the rest of the codebase already defends against this — see the Claude reader (src/data.js, ~line 679: if (entry.isSymbolicLink()) continue;) and isSafeLaunchPath in projects.js.
Two read sites are affected:
- the session-list loader (
readFileSyncof the meta/.jsonl) - the detail loader (
readFileSync(jsonlPath, ...))
Fix direction: before each read, reject symlinks / enforce containment, e.g.
const st = fs.lstatSync(p);
if (st.isSymbolicLink() || !st.isFile()) continue; // or: return null in the detail pathor fs.realpathSync(p) and verify it still starts with KIRO_SESSIONS_DIR + path.sep. Prefer lstat + isSymbolicLink to match the existing pattern.
No command-injection or prototype-pollution issues found in the new code — just this. Happy to merge once the symlink guard is in. 🙏
…view) The <uuid> UUID check blocks ../ path traversal, but readFileSync still followed symlinks: a symlink named <valid-uuid>.jsonl (or .json) inside ~/.kiro/sessions/cli/ pointing at e.g. ~/.ssh/id_rsa would be read and exposed via the dashboard (detail/export/search/replay), especially when codbash binds to a LAN address. Guard both read sites with lstat + isSymbolicLink, matching the existing Claude reader (src/data.js: `if (entry.isSymbolicLink()) continue;`): - scanKiroCliSessions: readdir withFileTypes skips symlinked metadata; the .jsonl is lstat'd so a symlinked events file reports has_detail:false - loadKiroCliDetail: lstat the .jsonl and refuse non-regular files - _buildSessionFileIndex: skip symlinked .jsonl for defense-in-depth Tests craft UUID-named symlinks to out-of-tree files with content that would surface if followed (LEAKED-TITLE / LEAKED-CONTENT); verified they fail with the guards removed and pass with them in place.
|
Thanks for the careful review — good catch on the symlink follow. Fixed in 16e14ef. The UUID check only blocked
Added two tests that craft UUID-named symlinks pointing at out-of-tree files whose content would surface if followed ( |
|
Проверил локально: ветка сливается в Единственное, что мешает: CI на ветке не запускался ( Спасибо за переработку части 2 под ревью 🙏 |
…268) (#278) * feat(kiro): support new file-based session format (~May 2026) Kiro CLI moved to per-session files under ~/.kiro/sessions/cli/: <uuid>.json — metadata (session_id, cwd, title, created_at, updated_at) <uuid>.jsonl — events: Prompt / AssistantMessage / ToolResults - Add KIRO_SESSIONS_DIR + scanKiroCliSessions() and loadKiroCliDetail() - Index the .jsonl files in _buildSessionFileIndex so they resolve to { format: 'kiro-cli' }, wiring detail/preview/search/replay/export end-to-end (previously listed but "Session file not found" on open) - Validate the sessionId as a strict UUID in loadKiroCliDetail before path.join to close a path-traversal vector on untrusted input - Add fixture-based tests covering scan, detail, path-traversal rejection, index resolution, and end-to-end wiring Old SQLite-based Kiro sessions remain fully supported alongside. * feat: real in-app auto-update for the desktop app (electron-updater) The desktop app showed an "Update Now" banner that called POST /api/update (npm i -g codbash-app@latest + restart). In the packaged Electron app that never worked: it updated an unrelated npm-global copy while the app kept running its bundled server, so the restart landed back on the old version. Replace it with a real in-place update via electron-updater: - Desktop: check GitHub Releases (latest-mac.yml/latest.yml), Download on click, progress, then Restart to relaunch onto the new version. autoDownload=false so the user controls the download; allowDowngrade/allowPrerelease pinned false. - Add mac `zip` target (Squirrel.Mac can't apply a DMG) and a Windows `nsis` target; document the zip/blockmap publish flow in RELEASE.md. - The npm-CLI self-update path is unchanged; the server refuses /api/update with 400 when CODBASH_DESKTOP=1 (set by desktop/main.js) so it can't half-update. Hardening (from security review): - will-navigate guard pins the window to the local server origin, so the powerful updater IPC bridge can't be reached by an off-origin page. - Main-process validates state before download/install (renderer buttons are UX, not the security boundary) and validates the IPC sender frame. Correctness (from code review): - Event listeners attach at autoUpdater creation so no check result is lost to a boot race; a single initial check (renderer-triggered, reload-safe). - Periodic 6h re-check skips while downloading/downloaded so it can't wipe the "ready to restart" banner. - Error state offers Check-again + Open-download-page; download is idempotent against a fast double-click. Test: test/desktop-update-guard.test.js asserts /api/update → 400 in desktop mode. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat: detect missing project folders + offer GitHub re-clone When a registered project folder is deleted/moved on disk, the Projects launcher now flags it instead of failing with a confusing "invalid path". - projects.js: pathExists() on-disk check; cloneRepo hardening — anchored GitHub-remote regex + realpath-of-nearest-ancestor containment guard (closes a symlinked-parent escape reachable via the "folder missing" window) - server.js: GET /api/projects/manual returns `exists`; /api/launch returns {missing:true, remoteUrl, projectId} before the generic safety check; new POST /api/projects/reclone restores the folder at its original path, with a per-id in-flight guard (409) - frontend: missing tiles show a role="note" disclaimer + Re-clone/Remove; launch-time misses (app.js + detail.js resume) offer a re-clone dialog; shared anchored isGithubRemote(); a11y — dialog semantics, Escape close, focus-on-open, aria-busy, AA-contrast warning text - tests: pathExists + cloneRepo guardrails (symlink-ancestor, control chars); headless render check in scratchpad; end-to-end server smoke green * feat: launch a project (with or without an agent) from the Terminal tab Add a "+ Project" launcher popover to the Workspace/Terminal toolbar that mirrors the Projects-tab launch model but opens in-app terminals: - Terminal — open a plain terminal in the project folder (no agent) - Run <agent> — open a terminal and auto-run the preferred/last-used agent - Agent menu — pick a specific installed agent to launch in the folder Reuses openInWorkspace(), which auto-runs the agent only when the folder actually opens, so a missing folder never misfiles agent history into ~. Agent commands mirror the server's buildLaunchCommand (terminals.js); pi resolves per-machine (pi vs omp) via getPiCommand(). Missing folders shown disabled; includes filter, empty-state, Escape/outside-click close, popover teardown on view-detach, height clamp, and theme-aware styling. Pure frontend (workspace.js + styles.css) — covers the npm CLI and the desktop app from one codebase. No backend changes. * chore(desktop): automate latest-mac.yml regeneration after stapling Adds desktop/scripts/regenerate-latest-mac.js (+ `npm run refresh-update-feed`) so the release runbook no longer hand-edits latest-mac.yml. Generalises the idea from #271 (thanks @indapublic) to the zip+dmg feed: it parses electron-builder's own latest-mac.yml and refreshes sha512/size/blockMapSize only for entries whose bytes changed on disk (sha512 mismatch), then re-syncs the top-level sha512 to the `path` file. Schema-preserving and idempotent — the untouched .zip entries (the actual mac update artifact) are left as-is; only stapled DMG entries are recomputed. RELEASE.md step 2b now calls the script instead of the manual yaml edit. Pure transforms covered by test/desktop-update-feed.test.js. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: block cross-origin state-changing requests (CSRF gate on POST/PUT/DELETE) State-changing API routes (/api/focus, /api/launch, /api/bulk-delete, DELETE /api/session, /api/projects/clone, /api/convert, …) had no CSRF/Origin check, and the server JSON-parses the body regardless of Content-Type. A malicious page in the user's browser could therefore issue a no-preflight "simple" cross-origin POST to http://localhost:PORT/... and trigger side effects (raise/launch terminals, delete sessions, clone repos). Add a single gate, right after the existing loopback Host guard, that runs before any route: for POST/PUT/DELETE, when Origin (or, as a fallback, Referer) is present it must match our Host, else 403. Absent both headers = a non-browser client (curl, scripts, the desktop shell's same-origin fetch) → allowed; a browser CSRF attack always carries at least one on a mutating cross-site request. GET/static are never gated. The same-origin logic is extracted to src/origin-guard.js and shared by the WebSocket upgrade check in terminal.js, which previously had its own copy — so the two can no longer drift. Comparison is host+port, case-normalized (RFC 7230 Host is case-insensitive), scheme ignored (codbash serves plain HTTP). - src/origin-guard.js: checkSameOrigin (tri-state + reason) + isDisallowedCrossOrigin - src/server.js: import the gate; drop the inline helper - src/terminal.js: verifyUpgradeAuth uses the shared checkSameOrigin - test/csrf-origin-guard.test.js: 15 unit tests (Origin/Referer, case, IPv6, fail-closed) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat: Running Agents lists external terminals and focuses their real window The Workspace "Running agents" tree now surfaces agents that actually run in external native terminals (iTerm/Terminal.app/Warp/cmux) — including the ones codbash itself launches via /api/launch — and a click raises that real window instead of opening a blank in-app terminal. Why: 7.15.0 scoped Running Agents to codbash-pty descendants only. Side effect: an agent codbash launched into iTerm descends from iTerm, not a codbash pty, so it vanished from the list — codbash opened the window yet showed "nothing running". And clicking a row with no live pane spawned an empty shell, which is not the agent. A live agent's PTY can't be mirrored into the browser terminal (OS: one controlling terminal per process), so the honest action is to focus its real window; true "resume with history" only applies to stopped sessions. - data.js: `_scopeToCodbashAgents` (filter) -> `_tagCodbashAgents` (tag). Each /api/active entry gets `local` (true=codbash-pane descendant, false=external); nothing is dropped. Pure, testable `_tagLocalAgents(active, live, ppidOf)` extracted; immutable, depth-bounded, fails open as external. - workspace.js: tree shows `!local` (external) agents; `jumpToRunningAgent` POSTs /api/focus by validated pid; removed dead `_wsPaneForCwd`; no blank terminal fallback. - server.js: LAN-bind (--host=0.0.0.0) banner notes all-user /api/active scope. - Tests: test/running-agents-external.test.js (11) + SDD/BDD artifacts. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(kiro): reject symlinked session files before reading (security review) The <uuid> UUID check blocks ../ path traversal, but readFileSync still followed symlinks: a symlink named <valid-uuid>.jsonl (or .json) inside ~/.kiro/sessions/cli/ pointing at e.g. ~/.ssh/id_rsa would be read and exposed via the dashboard (detail/export/search/replay), especially when codbash binds to a LAN address. Guard both read sites with lstat + isSymbolicLink, matching the existing Claude reader (src/data.js: `if (entry.isSymbolicLink()) continue;`): - scanKiroCliSessions: readdir withFileTypes skips symlinked metadata; the .jsonl is lstat'd so a symlinked events file reports has_detail:false - loadKiroCliDetail: lstat the .jsonl and refuse non-regular files - _buildSessionFileIndex: skip symlinked .jsonl for defense-in-depth Tests craft UUID-named symlinks to out-of-tree files with content that would surface if followed (LEAKED-TITLE / LEAKED-CONTENT); verified they fail with the guards removed and pass with them in place. * release: 7.16.0 — in-app desktop updates, Terminal project launcher, external running agents, missing-folder recovery, CSRF gate Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: sean10 <sean10reborn@gmail.com> Co-authored-by: NovakPAai <novakpavela@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@Sean10 вошло в 7.16.0 через релиз-кандидат #278 — релиз опубликован: https://github.com/vakovalskii/codbash/releases/tag/v7.16.0 Закрываю как включённое (в Спасибо за переработку части 2 под ревью — Kiro теперь читает пофайловый формат из |
Follow-up to #230. Part 1 (maxBuffer +
ToolUse) already landed inf2b77f9; this PR is Part 2 reworked around @vakovalskii's review feedback.What & why
Kiro CLI moved to per-session files under
~/.kiro/sessions/cli/:<uuid>.json— metadata (session_id,cwd,title,created_at,updated_at)<uuid>.jsonl— events, one per line:Prompt→ user,AssistantMessage→ assistant,ToolResults→ skippedThe previous version of Part 2 listed these sessions but wasn't wired end-to-end — detail/preview/search/replay/export all resolve through
findSessionFile()/_buildSessionFileIndex(), which didn't know aboutkiro-cli, so opening a session returned "Session file not found" and the newfound.format === 'kiro-cli'branches were never reached.Changes (addressing review points)
_buildSessionFileIndex()now scans~/.kiro/sessions/cli/*.jsonland registers them as{ format: 'kiro-cli', sessionId }, sofindSessionFile()resolves them and every downstream consumer (detail, preview, search, replay, export) works end-to-end.loadKiroCliDetail()validatessessionIdas a strict UUID beforepath.join, since it takes untrusted request input once wired. The scanner uses the same strict UUID pattern.test/kiro-cli-session.test.jscovers scan, detail parsing, path-traversal rejection, index resolution, and full end-to-end wiring (detail/preview/replay/export).scanKiroCliSessions()+loadKiroCliDetail()carry the parsing; old SQLite-based Kiro sessions remain fully supported alongside the new format.Testing
node --test test/*.test.js→ 207 pass, 2 skipped (win32-only), 0 fail