Skip to content

feat(kiro): wire up new file-based session format (~May 2026) - #268

Closed
Sean10 wants to merge 2 commits into
vakovalskii:mainfrom
Sean10:feat/kiro-cli-file-format
Closed

feat(kiro): wire up new file-based session format (~May 2026)#268
Sean10 wants to merge 2 commits into
vakovalskii:mainfrom
Sean10:feat/kiro-cli-file-format

Conversation

@Sean10

@Sean10 Sean10 commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Follow-up to #230. Part 1 (maxBuffer + ToolUse) already landed in f2b77f9; 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 → skipped

The 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 about kiro-cli, so opening a session returned "Session file not found" and the new found.format === 'kiro-cli' branches were never reached.

Changes (addressing review points)

  1. Index the new files_buildSessionFileIndex() now scans ~/.kiro/sessions/cli/*.jsonl and registers them as { format: 'kiro-cli', sessionId }, so findSessionFile() resolves them and every downstream consumer (detail, preview, search, replay, export) works end-to-end.
  2. Path-traversal fixloadKiroCliDetail() validates sessionId as a strict UUID before path.join, since it takes untrusted request input once wired. The scanner uses the same strict UUID pattern.
  3. Fixture-based tests — new test/kiro-cli-session.test.js covers 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
  • New end-to-end test fails without the index wiring and passes with it

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 vakovalskii left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 (readFileSync of 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 path

or 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.
@Sean10

Sean10 commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the careful review — good catch on the symlink follow. Fixed in 16e14ef.

The UUID check only blocked ../ traversal; readFileSync still followed symlinks. Guarded both read sites with lstat + isSymbolicLink, matching the existing Claude reader (if (entry.isSymbolicLink()) continue;):

  • scanKiroCliSessionsreaddirSync(..., { withFileTypes: true }) skips symlinked metadata; the .jsonl is lstat'd so a symlinked events file reports has_detail: false / file_size: 0.
  • loadKiroCliDetaillstat the .jsonl and refuse anything that isn't a regular file before reading.
  • _buildSessionFileIndex — skip symlinked .jsonl too, for defense-in-depth (so it never resolves to { format: 'kiro-cli' }).

Added two tests that craft UUID-named symlinks pointing at out-of-tree files whose content would surface if followed (LEAKED-TITLE in metadata, LEAKED-CONTENT in events). I verified they fail with the guards removed and pass with them in place, so they actually bite rather than passing vacuously. Full suite: 209 pass, 2 skipped (win32-only), 0 fail — and re-confirmed against real data (125 CLI sessions still scan, legitimate sessions read fine).

@vakovalskii

Copy link
Copy Markdown
Owner

Проверил локально: ветка сливается в main без конфликтов, полный прогон node --test test/*.test.js — 253 passed / 0 failed вместе с #272#276.

Единственное, что мешает: CI на ветке не запускался (no checks reported on 'feat/kiro-cli-file-format'). Пните ветку пустым коммитом или ребейзните на свежий main, чтобы workflow прогнался — тогда заберу в релиз-кандидат 7.16.0.

Спасибо за переработку части 2 под ревью 🙏

vakovalskii added a commit that referenced this pull request Jul 30, 2026
…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>
@vakovalskii

Copy link
Copy Markdown
Owner

@Sean10 вошло в 7.16.0 через релиз-кандидат #278 — релиз опубликован: https://github.com/vakovalskii/codbash/releases/tag/v7.16.0

Закрываю как включённое (в main коммиты пришли squash'ем от RC-ветки, поэтому автозакрытия не случилось). CI на вашей ветке так и не прогнался, но я проверил локально в общем стеке: тесты 253 passed / 0 failed, включая test/kiro-cli-session.test.js.

Спасибо за переработку части 2 под ревью — Kiro теперь читает пофайловый формат из ~/.kiro/sessions/cli во всех местах (detail, preview, поиск, replay, экспорт) 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants